fix(agent): continue after recoverable tool loop guardrail

[OmarB97]

Summary

• Rebuilt this branch on current upstream main as a focused replacement for the old stacked PR.
• Keep explicit tool_reported_loop_block tool results inside the same user turn by appending model-visible redirect guidance in default soft-guardrail mode.
• Preserve hard-stop behavior when tool_loop_guardrails.hard_stop_enabled is true.
• Add a bounded action-preamble recovery for no-tool responses like "Let me inspect ...:" so the agent continues with the required tool call instead of treating the preamble as a final answer.

Review Follow-Up

• Old stacked diff is gone: this PR is now 7 files, +390/-4 on top of upstream main.
• agent/tool_guardrails.py is 531 lines.
• The unrelated stack items called out in review are not part of this diff: no HERMES_LLM_BASE_URL, no HERMES_SKIP_PROFILE_OVERRIDE, no dflash stream-timeout resolver, no progress canary, no retry_on_stall, and no chat() error fallback change.
• The tool_reported_loop_block path now respects hard_stop_enabled: default mode redirects/continues; hard-stop mode halts with the existing controlled guardrail response.

Verification

• python -m pytest tests/agent/test_action_preamble.py tests/agent/test_tool_guardrails.py tests/run_agent/test_tool_call_guardrail_runtime.py -q -o addopts= -> 32 passed, 1 warning.
• python -m pytest tests/agent/test_action_preamble.py tests/agent/test_tool_guardrails.py tests/run_agent/test_tool_call_guardrail_runtime.py tests/run_agent/test_streaming.py tests/agent/test_local_stream_timeout.py -q -o addopts= -> 108 passed, 1 warning.
• python -m py_compile agent/action_preamble.py agent/tool_guardrails.py agent/conversation_loop.py run_agent.py -> OK.
• git diff --check -> OK.

Task: hermes-dflash-post-retry-preamble-stall

Reviewer note: this overlaps with #37166 at the same no-tool-call decision point in run_conversation. If both land, the redirect-guardrail path here should be treated as canonical for guardrail-triggered loops, and #37166's stall retry for empty/stalled generations — happy to rebase whichever lands second.

[combatsheep]

LGTM — the recoverable stall/guardrail handling is well scoped, and the tests cover both the retry path and the hard-stop path.

[OmarB97]

Reviewer Notes (hermes-local-longctx-ko-mac)

Self-review blocked — reviewer and author are both OmarB97. Formal approve/request-changes requires a different GitHub identity. Leaving this comment with findings; the structured verdict follows below.

Findings Summary

Blocking (must fix before merge):

1. exceeds 1,000-line ceiling. Original was ~1000 lines; PR adds +646/-10 for ~1636 total. AGENTS.md code boundary rule is hard: No source file may exceed 1,000 lines. The new low-information detection, terminal usage error handling, and redirect logic should be extracted into or similar.

2. Massive scope creep. Original task: (a) make recoverable tool-loop guardrail non-terminal, (b) fix action-preamble stop detection. This PR adds 7+ major features: stall retry, progress outcome canary, tool guardrail redirects, dflash stream timeout, text result compaction, HERMES_LLM_BASE_URL proxy override, HERMES_SKIP_PROFILE_OVERRIDE, search tool zero-result hints, CLI Ctrl+C interrupt handling, timeout config for custom providers, chat() error resilience. Each deserves its own PR with focused review.

Concerns:
3. is a blanket override that redirects ALL provider traffic through a custom proxy. This is a security concern — any code path that sets this env var (including less-trusted MeshBoard launcher) can intercept inference traffic from all providers. Consider scoping it to explicitly opt-in providers only.

4. calls twice when dflash path is taken (once directly, once inside ). Cache the result.

5. is hardcoded in . Should be configurable via the guardrail config like other thresholds.

Suggestions:
6. The function forces for the retry call. This is fine for correctness but means the retry bypasses all streaming-specific error handling. Consider noting this trade-off in the docstring.

7. now falls back to when is missing. Callers can no longer distinguish model output from error messages — both return strings. Consider returning a typed wrapper or at least document this behavior change.

8. The handler in sets unconditionally, even when is False. The existing logic respects ; this inconsistency means tool-reported blocks always halt regardless of config. Intentional?

---

Recommendation: — split into focused PRs, extract past the 1K line limit, address scope concerns.

[OmarB97]

Reviewer Notes (hermes-local-longctx-ko-mac)

Self-review blocked — reviewer and author are both OmarB97. Formal approve/request-changes requires a different GitHub identity. Leaving this comment with findings; the structured verdict follows below.

Findings Summary

Blocking (must fix before merge):

1. tool_guardrails.py exceeds 1,000-line ceiling. Original was ~1000 lines; PR adds +646/-10 for ~1636 total. AGENTS.md code boundary rule is hard: "No source file may exceed 1,000 lines." The new low-information detection, terminal usage error handling, and redirect logic should be extracted into a separate module.

2. Massive scope creep. Original task: (a) make recoverable tool-loop guardrail non-terminal, (b) fix action-preamble stop detection. This PR adds 7+ major features: stall retry, progress outcome canary, tool guardrail redirects, dflash stream timeout, text result compaction, HERMES_LLM_BASE_URL proxy override, HERMES_SKIP_PROFILE_OVERRIDE, search tool zero-result hints, CLI Ctrl+C interrupt handling, timeout config for custom providers, chat() error resilience. Each deserves its own PR with focused review.

Concerns:

3. HERMES_LLM_BASE_URL is a blanket override that redirects ALL provider traffic through a custom proxy. This is a security concern — any code path that sets this env var (including less-trusted MeshBoard launcher) can intercept inference traffic from all providers. Consider scoping it to explicitly opt-in providers only.

4. resolve_stream_stale_timeout calls estimate_request_context_tokens(api_kwargs) twice when dflash path is taken (once directly, once inside _dflash_local_stale_timeout). Cache the result.

5. _tool_guardrail_continue_max = 2 is hardcoded in conversation_loop.py. Should be configurable via the guardrail config like other thresholds.

Suggestions:

6. The retry_on_stall function forces stream=False for the retry call. This is fine for correctness but means the retry bypasses all streaming-specific error handling. Consider noting this trade-off in the docstring.

7. chat() now falls back to result.get('error') when final_response is missing. Callers can no longer distinguish model output from error messages — both return strings. Consider returning a typed wrapper or at least document this behavior change.

8. The tool_reported_loop_block handler in after_call sets _halt_decision unconditionally, even when hard_stop_enabled is False. The existing before_call logic respects hard_stop_enabled; this inconsistency means tool-reported blocks always halt regardless of config. Intentional?

Recommendation: needs-changes — split into focused PRs, extract tool_guardrails.py past the 1K line limit, address scope concerns.

[OmarB97]

Follow-up pushed in dec03c6bc.

I rebuilt the branch from current upstream main rather than layering more changes onto the old stacked branch. That addresses the review scope issue directly: the PR is now 7 files, +390/-4, and agent/tool_guardrails.py is 531 lines.

Addressed the reviewer notes:

• Removed the unrelated stacked changes from this PR: no HERMES_LLM_BASE_URL, HERMES_SKIP_PROFILE_OVERRIDE, dflash stream-timeout resolver, progress canary, retry_on_stall, text-result compaction, search-hint changes, Ctrl+C changes, timeout config changes, or chat() error fallback.
• tool_reported_loop_block now respects hard_stop_enabled: default soft-guardrail mode appends Tool strategy redirect guidance and continues the same user turn; hard-stop mode still produces the controlled guardrail halt.
• Added focused action-preamble recovery for no-tool responses like Let me inspect ...: with a per-turn cap, plus a false-positive test for explanatory “Let me explain...” text.

Verification:

• python -m pytest tests/agent/test_action_preamble.py tests/agent/test_tool_guardrails.py tests/run_agent/test_tool_call_guardrail_runtime.py -q -o addopts= -> 32 passed, 1 warning
• python -m pytest tests/agent/test_action_preamble.py tests/agent/test_tool_guardrails.py tests/run_agent/test_tool_call_guardrail_runtime.py tests/run_agent/test_streaming.py tests/agent/test_local_stream_timeout.py -q -o addopts= -> 108 passed, 1 warning
• python -m py_compile agent/action_preamble.py agent/tool_guardrails.py agent/conversation_loop.py run_agent.py -> OK
• git diff --check -> OK

The remaining PR state is GitHub-side BLOCKED/checks-review gating; the branch itself is mergeable.

[teknium1]

Thanks for narrowing this to the redirect and no-tool recovery paths. The target behavior remains absent on current main: default non-Codex intent-ack continuation is off (agent/agent_runtime_helpers.py:2788), and file/search BLOCKED loop results have no dedicated redirect handling (agent/tool_guardrails.py:298-345).

Problems

• agent/conversation_loop.py:5110-5115 tags both recovery messages with _action_preamble_recovery, but run_agent.py:224-235 does not include that flag in _EPHEMERAL_SCAFFOLDING_FLAGS. The session flusher skips only recognized scaffolding, so this internal assistant/user pair would persist and replay on resume.
• agent/conversation_loop.py:5117 continues with the incomplete preamble still in final_response. Current main's analogous ack continuation explicitly clears it at agent/conversation_loop.py:5132 (commit f46e7647e); otherwise budget exhaustion can surface the preamble instead of using agent/turn_finalizer.py:82-96's summary fallback.

Suggested changes

• Register _action_preamble_recovery as ephemeral scaffolding and add a persistence regression test.
• Clear final_response before this continuation and add a max-iteration regression test.

Automated hermes-sweeper review.

[teknium1]

Blocking: this flag is not in run_agent.py:_EPHEMERAL_SCAFFOLDING_FLAGS, so _persist_session() will retain this internal assistant/user recovery pair. Add _action_preamble_recovery to that classifier and cover persistence after a successful recovery.

[teknium1]

Blocking: clear final_response before continuing. Current main does this for the analogous intent-ack continuation (agent/conversation_loop.py:5132, f46e7647e); otherwise exhausting the continuation budget returns the stale preamble instead of taking the summary fallback.