fix(engine): track consecutive action errors in orchestrator (#2325) - #2340
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67fb7e1921
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| max_consecutive_errors = config.get("max_consecutive_errors", 5) | ||
| consecutive_nudges = 0 | ||
| consecutive_errors = 0 | ||
| consecutive_action_errors = 0 |
There was a problem hiding this comment.
Restore action-error counter when resuming checkpointed runs
run_loop always initializes consecutive_action_errors to 0, so a resumed thread loses the accumulated failure streak even though __save_checkpoint__ now writes consecutive_action_errors into runtime_checkpoint metadata. In the approval/auth pause flow, this means repeated all-failed action batches can avoid both the nudge and terminal failure thresholds by resetting on each resume. I verified load_runtime_checkpoint only rehydrates persisted_state (not counters), so the new counter persistence is currently write-only and the loop-protection behavior is bypassed after pauses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request implements tracking for consecutive action errors within the orchestrator to identify and mitigate loops where tool calls consistently fail. It introduces a consecutive_action_errors counter, updates checkpointing mechanisms to persist this state, and adds logic to either nudge the agent with a system message or fail the thread when error thresholds are met. The Rust test suite is also enhanced with new helpers and comprehensive unit tests for the error-counting logic. Review feedback highlighted a critical potential crash involving None type arithmetic, a persistence issue where counters reset upon resuming from a pause, and recommended resetting all error counters when an action batch succeeds.
| max_consecutive_errors = config.get("max_consecutive_errors", 5) | ||
| consecutive_nudges = 0 | ||
| consecutive_errors = 0 | ||
| consecutive_action_errors = 0 |
There was a problem hiding this comment.
There are two significant issues in this initialization block:
-
Potential Crash (Critical): If
max_consecutive_errorsis not explicitly set in the thread configuration (which is the default state inThreadConfig),config.get("max_consecutive_errors", 5)will returnNonebecause the key exists in the dictionary with anullvalue from the Rust side. This will cause aTypeErrorwhen arithmetic is performed with this value later (e.g., at line 869:max_consecutive_errors + 2). -
Persistence Bug (High): The error counters are initialized to 0 every time
run_loopstarts, but they are never restored from thestateorconfigon resume. Since the orchestrator is re-instantiated after pauses (such as approval gates or authentication requests), these counters will reset to 0, allowing an agent to bypass the error thresholds simply by triggering a system pause.
max_consecutive_errors = config.get("max_consecutive_errors")
if max_consecutive_errors is None:
max_consecutive_errors = 5
consecutive_nudges = state.get("consecutive_nudges", 0)
consecutive_errors = state.get("consecutive_errors", 0)
consecutive_action_errors = state.get("consecutive_action_errors", 0)| __save_checkpoint__(state, { | ||
| "nudge_count": consecutive_nudges, | ||
| "consecutive_errors": consecutive_errors, | ||
| "consecutive_action_errors": consecutive_action_errors, | ||
| "compaction_count": state.get("compaction_count", 0), | ||
| }) |
There was a problem hiding this comment.
To ensure the error counters are correctly persisted across pauses and resumes, they should be updated in the state dictionary before saving the checkpoint. This allows the initialization block at the start of run_loop to restore them correctly.
| __save_checkpoint__(state, { | |
| "nudge_count": consecutive_nudges, | |
| "consecutive_errors": consecutive_errors, | |
| "consecutive_action_errors": consecutive_action_errors, | |
| "compaction_count": state.get("compaction_count", 0), | |
| }) | |
| state["consecutive_errors"] = consecutive_errors | |
| state["consecutive_action_errors"] = consecutive_action_errors | |
| state["consecutive_nudges"] = consecutive_nudges | |
| __save_checkpoint__(state, { | |
| "nudge_count": consecutive_nudges, | |
| "consecutive_errors": consecutive_errors, | |
| "consecutive_action_errors": consecutive_action_errors, | |
| "compaction_count": state.get("compaction_count", 0), | |
| }) |
| if batch_success_count > 0: | ||
| consecutive_action_errors = 0 | ||
| elif batch_error_count > 0: | ||
| consecutive_action_errors += 1 |
There was a problem hiding this comment.
There are two improvements needed here:
- Cross-tier Reset: When a Tier 0 action batch succeeds, we should also reset the Tier 1
consecutive_errorscounter. If the agent successfully executes a tool, it is no longer stuck in a failing loop, regardless of which tier the previous errors occurred in. - Persistence: Update the
statedictionary with the new counter values so they are preserved in the checkpoint and can be restored if the thread is paused and resumed.
if batch_success_count > 0:
consecutive_action_errors = 0
consecutive_errors = 0
consecutive_nudges = 0
elif batch_error_count > 0:
consecutive_action_errors += 1
state["consecutive_action_errors"] = consecutive_action_errors
state["consecutive_errors"] = consecutive_errors
state["consecutive_nudges"] = consecutive_nudges…ath (#2325) The Python orchestrator had no error counting for structured action calls (Tier 0), allowing threads to loop indefinitely on failing tool calls and complete "successfully" even when every tool call failed. This adds a consecutive_action_errors counter that increments when all actions in a batch fail, resets when any succeeds, injects a nudge at the threshold, and transitions to failed at threshold + 2. Also prefixes error outputs with [ACTION FAILED] for visibility and persists the counter in checkpoints. Closes #2325 Related: #2279, #2240 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
04694b9 to
6163153
Compare
…ath (nearai#2325) (nearai#2340) The Python orchestrator had no error counting for structured action calls (Tier 0), allowing threads to loop indefinitely on failing tool calls and complete "successfully" even when every tool call failed. This adds a consecutive_action_errors counter that increments when all actions in a batch fail, resets when any succeeds, injects a nudge at the threshold, and transitions to failed at threshold + 2. Also prefixes error outputs with [ACTION FAILED] for visibility and persists the counter in checkpoints. Closes nearai#2325 Related: nearai#2279, nearai#2240 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
consecutive_action_errorscounter to the Python orchestrator's Tier 0 (structured action call) path, matching the existingconsecutive_errorstracking on the Tier 1 (code execution) pathmax_consecutive_errorsthreshold telling the LLM to try a different approachfailedatmax_consecutive_errors + 2with an explanatory error message[ACTION FAILED]for visibility in the message historyCloses #2325
Related: #2279 (bot falsely claims success despite tool errors), #2240 (agent retries same failing tool call)
Test plan
cargo test -p ironclaw_engine-- all 376 tests passGenerated with Claude Code