Skip to content

fix(run_agent): 7 bug fixes — db flush, interrupt, tool error handling, retry loop - #76

Closed
0xbyt4 wants to merge 1 commit into
NousResearch:mainfrom
0xbyt4:fix/run-agent-7-bugs
Closed

fix(run_agent): 7 bug fixes — db flush, interrupt, tool error handling, retry loop#76
0xbyt4 wants to merge 1 commit into
NousResearch:mainfrom
0xbyt4:fix/run-agent-7-bugs

Conversation

@0xbyt4

@0xbyt4 0xbyt4 commented Feb 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Systematic bug audit of run_agent.py identified and fixed 7 verified bugs. Each fix is minimal and targeted — no refactoring, no new features.

Critical

  • Off-by-one in _flush_messages_to_session_db — first new message after conversation history was skipped, never written to DB (start_idx = len(history) + 1 → len(history))
  • Stale interrupt flag on early returns — 3 interrupt return paths didn't call clear_interrupt(), causing next run_conversation() call to exit immediately

High

  • Uncaught handle_function_call exceptions — quiet mode had try/finally without except, normal mode had no try at all. An exception would crash the agent and break the API contract (missing tool response)

Medium

  • flush_memories identity check could wipe all messages — is not flush_msg cleanup used Python identity comparison; if reference was lost, the while loop would pop every message. Replaced with sentinel marker

Low

  • _hydrate_todo_store cleared global interrupt flag — unrelated _set_interrupt(False) call removed
  • Retry loop off-by-one — while retry_count <= max_retries allowed 7 retries instead of intended 6. Changed to <
  • Redundant import re in _clean_session_content (already imported at module level)

Changes

Single file: run_agent.py (+22, -10)

@0xbyt4
fix: resolve 7 bugs in run_agent.py from systematic audit
2fabb48 
- fix(db): remove off-by-one in _flush_messages_to_session_db that skipped first new message
- fix(interrupt): add clear_interrupt() to 3 early-return paths preventing stale flag on next run
- fix(tools): catch handle_function_call exceptions in both quiet and normal mode paths
- fix(todo): remove unrelated _set_interrupt(False) from _hydrate_todo_store
- fix(flush): replace fragile identity check with sentinel marker in flush_memories cleanup
- fix(retry): change retry loop from <= to < so max_retries=6 means 6 retries not 7
- chore: remove redundant local import re in _clean_session_content
@0xbyt4 0xbyt4 changed the title fix: resolve 7 bugs in run_agent.py from systematic audit fix(run_agent): 7 bug fixes — db flush, interrupt, tool error handling, retry loop Feb 26, 2026
teknium1 added a commit that referenced this pull request Feb 27, 2026
@teknium1
Cherry-pick 6 bug fixes from PR #76 and update documentation
c77f3da 
Code fixes (run_agent.py):
- Fix off-by-one in _flush_messages_to_session_db skipping one message per flush
- Add clear_interrupt() to 3 early-return paths preventing stale interrupt state
- Wrap handle_function_call in try/except so tool crashes don't kill the conversation
- Replace fragile `is` identity check with _flush_sentinel marker for memory flush cleanup
- Fix retry loop off-by-one (6 attempts not 7)
- Remove redundant inline `import re`
@teknium1

teknium1 commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Some of it was out of sync with other changes made earlier in the day, had to cherry pick from it. Thank you for identifying!

@teknium1 teknium1 closed this Feb 27, 2026
angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
@teknium1
Cherry-pick 6 bug fixes from PR NousResearch#76 and update documentation
d8a36b7 
Code fixes (run_agent.py):
- Fix off-by-one in _flush_messages_to_session_db skipping one message per flush
- Add clear_interrupt() to 3 early-return paths preventing stale interrupt state
- Wrap handle_function_call in try/except so tool crashes don't kill the conversation
- Replace fragile `is` identity check with _flush_sentinel marker for memory flush cleanup
- Fix retry loop off-by-one (6 attempts not 7)
- Remove redundant inline `import re`
@aresbotv1-beep

aresbotv1-beep commented May 24, 2026 •
edited
Loading

Copy link
Copy Markdown

Thanks for the audit here. We reviewed this against current main: the major fixes from this PR appear to have landed later through other changes/cherry-picks, including the DB flush off-by-one, tool-call exception handling, retry-loop bound, and interrupt cleanup paths.

Given the branch is now stale and the PR is closed, I’d leave this closed rather than revive it. The only item that may still be worth a separate focused follow-up is whether _hydrate_todo_store() should still call _set_interrupt(False) in current main; that should be verified against the newer interrupt/thread-scoping behavior before changing it.

Appreciate the original bug audit — it helped identify real issues.

waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
@teknium1
Cherry-pick 6 bug fixes from PR NousResearch#76 and update documentation
15300e5 
Code fixes (run_agent.py):
- Fix off-by-one in _flush_messages_to_session_db skipping one message per flush
- Add clear_interrupt() to 3 early-return paths preventing stale interrupt state
- Wrap handle_function_call in try/except so tool crashes don't kill the conversation
- Replace fragile `is` identity check with _flush_sentinel marker for memory flush cleanup
- Fix retry loop off-by-one (6 attempts not 7)
- Remove redundant inline `import re`
@thomasjustesq-dev thomasjustesq-dev mentioned this pull request Aug 4, 2026
Merged
2 tasks
Meraniya pushed a commit to Meraniya/hermes-agent that referenced this pull request Aug 6, 2026
@dizhaky @claude
docs(plans): note config-integrity-watchdog follow-ups (NousResearch#68…
b22b6f1 
…, NousResearch#69) and mark arc complete (NousResearch#76)

Session close-out audit found the Outcome section only mentioned PR NousResearch#67.
PR NousResearch#69 explicitly self-describes as "follow-up to NousResearch#67" (unsigned-commit
git fallback) and PR NousResearch#68 (AGENTS.md docs) is also a direct follow-up;
neither was recorded. Also clarifies that NousResearch#71/NousResearch#72 (cron ticker
heartbeat/stall fix) are an unrelated arc shipped the same day, not
part of this project.


Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh

Co-authored-by: Claude <noreply@anthropic.com>
sijav added a commit to sijav/sijav-agent that referenced this pull request Aug 6, 2026
@sijav
test(coverage): agent_runtime_helpers chip arh-cachepol2 (prompt-cach…
02c3748 
…e policy)

Whole-function owner tests for anthropic_prompt_cache_policy (cost-critical
should_cache/native-layout tree: _cache_disabled short-circuit, native-Anthropic,
OpenRouter/Portal Claude|Kimi envelope, Portal Qwen, third-party anthropic_messages
Claude, MiniMax provider|host, Alibaba-Qwen, anthropic-wire non-claude-non-minimax
fall-through, MoA aggregator resolution/no-agg/rt-fail, unmatched) and
plan_cache_sections_for_destination (non-caching strip vs caching plan).
Cluster (1999-2237) 100%.

Found tech-debt NousResearch#76 (MoA branch recurses with no self/depth guard -> potential
infinite recursion on a pathological aggregator.provider=='moa').
sijav added a commit to sijav/sijav-agent that referenced this pull request Aug 7, 2026
@sijav
fix(cache-policy): break nested-MoA recursion in anthropic_prompt_cac…
02e459e 
…he_policy (NousResearch#76)

A MoA preset whose aggregator slot is itself declared provider='moa'
(self-referential, or an A->B->A cycle across two presets) made the MoA branch
re-enter itself until the stack blew. Measured: 993 recursive frames, each one
re-running load_config() + resolve_moa_preset(). The resulting RecursionError
was then swallowed by the branch's defensive `except Exception`, so the
function silently returned (False, False) — i.e. prompt caching fully OFF,
exactly the total cache loss (85% -> 2% share) this MoA branch was written to
prevent, with no signal to the operator.

Nested MoA aggregators are not a supported configuration, so the branch now
refuses once, loudly (logger.warning naming the preset and telling the operator
to point the aggregator at a concrete provider), instead of recursing.

Verified: 993 resolve_moa_preset calls -> 1. Regression tests cover both the
self-referential preset and the two-preset cycle, asserting the call count
(the discriminator vs the old behaviour) and the warning. Region 2060-2220:
0 missing lines, 0 missing branches. 61 passed.
@trippydao trippydao mentioned this pull request Aug 9, 2026
Merged
trippydao added a commit to trippydao/hermes-agent that referenced this pull request Aug 9, 2026
@trippydao
kanban: human-approval gate for auto-decompose (auto_decompose_requir…
dfa2a32 
…e_approval)

Adds a config-gated hold between auto-decompose and worker spawn.

- kanban.auto_decompose_require_approval (default false): when true, the
  auto-decomposer still builds child graphs but calls decompose_task with
  auto_promote=False, leaving every child in 'todo' — no worker spawns
  until a human runs 'hermes kanban approve'.
- decompose_task() gains an auto_promote override (None=read config),
  so the auto path can force a hold while manual/CLI decompose keeps its
  configured behaviour.
- 'hermes kanban approve <id>' registered as a semantic alias of promote
  (same parser + handler) to make the human-in-the-loop intent explicit.
- Gateway _resolve_auto_decompose_settings returns (enabled, per_tick,
  require_approval) so the flag is re-read live each tick like NousResearch#49638.
- config_defaults + EN/ZH kanban docs updated; 6 new/updated tests.

Closes the 'auto-decomposer spawned live cluster work without sign-off'
failure (mike-infra NousResearch#76): fan-out can now be approval-gated so nothing
launches against a deployment without human go-ahead.
@trippydao trippydao mentioned this pull request Aug 10, 2026
Open
@kentawata kentawata mentioned this pull request Aug 10, 2026
Open
trippydao added a commit to trippydao/hermes-agent that referenced this pull request Aug 15, 2026
@trippydao
kanban: human-approval gate for auto-decompose (auto_decompose_requir…
2da5fd7 
…e_approval)

Adds a config-gated hold between auto-decompose and worker spawn.

- kanban.auto_decompose_require_approval (default false): when true, the
  auto-decomposer still builds child graphs but calls decompose_task with
  auto_promote=False, leaving every child in 'todo' — no worker spawns
  until a human runs 'hermes kanban approve'.
- decompose_task() gains an auto_promote override (None=read config),
  so the auto path can force a hold while manual/CLI decompose keeps its
  configured behaviour.
- 'hermes kanban approve <id>' registered as a semantic alias of promote
  (same parser + handler) to make the human-in-the-loop intent explicit.
- Gateway _resolve_auto_decompose_settings returns (enabled, per_tick,
  require_approval) so the flag is re-read live each tick like NousResearch#49638.
- config_defaults + EN/ZH kanban docs updated; 6 new/updated tests.

Closes the 'auto-decomposer spawned live cluster work without sign-off'
failure (mike-infra NousResearch#76): fan-out can now be approval-gated so nothing
launches against a deployment without human go-ahead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@0xbyt4 @teknium1 @aresbotv1-beep