fix(agent): let guardrail hard-stop give the model one rebound before halting - #64363
fix(agent): let guardrail hard-stop give the model one rebound before halting#64363kyssta-exe wants to merge 1 commit into
Conversation
… halting When tool_loop_guardrails.hard_stop_enabled triggers a block, the synthetic tool result was already injected into messages but the conversation loop then fabricated an assistant message and broke the turn — the model never saw the error to self-correct. Now the first guardrail halt per turn resets the decision and continues the loop, giving the model exactly one rebound chance to see the tool error and change strategy. A second guardrail halt in the same turn triggers the real controlled halt + break. - Add _tool_guardrail_halt_count counter (agent_init, turn_context) - Change _set_tool_guardrail_halt to increment the counter - Conversation loop: first halt → continue (rebound), second → break Fixes NousResearch#64322
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real current-main failure: agent/conversation_loop.py:4843-4866 appends tool results and then immediately fabricates a halt response instead of issuing another model call.
Problems
run_agent.py:5643counts eachshould_haltcallback, not each rebound opportunity. The concurrent executor evaluates every call in a batch (agent/tool_executor.py:360-485). If two distinct calls are already blocked, both invoke_guardrail_block_result(), the count reaches two before the model has seen either result, and the proposedagent/conversation_loop.py:4751branch halts without the promised rebound.- The diff has no tests. Current coverage still asserts the old canned halt at
tests/run_agent/test_tool_call_guardrail_runtime.py:271-299, so the new first-rebound, second-halt, and multi-block-batch contracts are untested.
Suggested changes
- Count a completed guardrail-bearing batch/rebound cycle once, rather than each guardrail decision.
- Add runtime coverage for successful recovery, a later terminal halt, and multiple pre-blocked calls in one concurrent batch.
Current main has moved the nearby dispatch path since the PR base, including segmented batch dispatch in 271a9d8ec; salvage should apply the corrected semantics against that path.
Automated hermes-sweeper review.
| turn triggers the real exit break. | ||
| """ | ||
| if decision.should_halt: | ||
| self._tool_guardrail_halt_count += 1 |
There was a problem hiding this comment.
This increments once per blocked decision, not once per rebound opportunity. The concurrent executor evaluates every call in a batch before returning; two distinct already-blocked calls can make this 2 before the model has seen either synthetic result, so the loop takes the terminal branch without any rebound. Count a completed guardrail-bearing batch/loop cycle once instead.
SummaryOne PR, #64363, addresses #64322 by replacing the immediate synthetic halt after a first guardrail block with an intended model rebound before a later controlled halt. Its callback-level counter can nevertheless reach two within one concurrent batch, before the model receives a rebound opportunity. Related pull requests
Suggested consolidationKeep #64363 open with a salvage path, consistent with the contributor keep_open review: retain the first-block rebound behavior, change accounting to one completed guardrail-bearing batch/rebound cycle rather than individual callbacks, and add the requested runtime coverage. There are no duplicate PRs to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I64322(["issue #64322 (open)"])
P64363["PR #64363 (open)"]
P64363 -->|best fix| I64322
class I64322 open
class P64363 open
class P64363 best
class P64363 target
click I64322 "https://github.com/NousResearch/hermes-agent/issues/64322"
click P64363 "https://github.com/NousResearch/hermes-agent/pull/64363"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 6 kB of PR diffs, 6 kB of issue/PR text, 2 kB of discussion (2 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Fixes #64322
Problem
When
tool_loop_guardrails.hard_stop_enabledtriggers a block (repeated exact failure or idempotent no-progress), the synthetic tool result was already injected intomessagesbut the conversation loop immediately fabricated an assistant message like "I stopped retrying..." andbreakout of the tool loop — the model never saw the error to self-correct. The agent went silent mid-task, requiring manual user input.Fix
The first guardrail halt per turn now:
_tool_guardrail_halt_decisiontoNonecontinues the loop instead of breakingThe model sees the synthetic tool error in
messagesand gets exactly one rebound chance to change strategy. A second guardrail halt in the same turn (_tool_guardrail_halt_count >= 2) performs the real controlled halt + break (the original behavior).Changes
agent_init.py: add_tool_guardrail_halt_countfieldturn_context.py: reset counter per turnrun_agent.py:_set_tool_guardrail_haltincrements counter instead of one-shotconversation_loop.py: first halt → continue (rebound), second → breakVerification