[codex] Add progress outcome canary - #35702
Conversation
10b1a69 to
92e9fe7
Compare
92e9fe7 to
8e7d34c
Compare
8e7d34c to
4eaf17e
Compare
4eaf17e to
2fd7b8f
Compare
2fd7b8f to
a6e17f2
Compare
a6e17f2 to
fd0dfee
Compare
fd0dfee to
33d9162
Compare
|
Rebased this upstream mirror onto current main and repaired the CI fallout. Current head: fd0dfeebb was superseded by 33d9162b0, final head is 33d9162b0527d6593d69b209fb0ea567238fa4e7. Local verification passed: |
33d9162 to
991aebd
Compare
991aebd to
113c340
Compare
|
Refreshed onto upstream/main (rebase, live merge-base |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the remaining gap between per-tool repetition checks and a turn-level lack of outcome. The premise remains present on current main: agent/tool_guardrails.py:350-375 only tracks repeated results for the same idempotent call signature.
Problems
agent/conversation_loop.py:4888-4892appends a syntheticusermessage after tool execution. This violates the strict alternation invariant inAGENTS.md:88-91, which explicitly prohibits synthetic user messages mid-loop.agent/stall_retry.py:271-272independently constructs a synthetic assistant/user pair for retry, creating the same invalid in-turn transcript shape.agent/stall_retry.py:20-28adds user-facing non-secretHERMES_STALL_RETRY_*settings;AGENTS.md:102-107requires behavioral settings to useconfig.yamlinstead.- The 33-file diff also includes dflash retry/model switching, result compaction, timeout, MeshBoard routing, and file-tool work beyond the stated canary.
Suggested changes
- Attach the canary guidance to an existing tool result instead of appending a user message, and add an end-to-end role-alternation test.
- Remove or separately redesign the retry transcript mutation.
- Keep user-facing settings in
config.yamland split unrelated feature work from the canary.
Automated hermes-sweeper review.
| ) | ||
| if _progress_decision is not None: | ||
| agent._emit_status("Progress canary nudged the model to pick a concrete outcome") | ||
| messages.append({ |
There was a problem hiding this comment.
This appends a synthetic user turn after a tool batch in the active loop. AGENTS.md:88-91 explicitly prohibits synthetic user messages mid-loop to preserve role alternation and prompt-cache invariants. Attach the advisory to an existing tool-result message instead, and cover the resulting transcript shape end to end.
| visible = (stalled_content or "").strip() | ||
| if visible: | ||
| retry_messages.append({"role": "assistant", "content": visible}) | ||
| retry_messages.append({"role": "user", "content": _STALL_RETRY_NUDGE}) |
There was a problem hiding this comment.
This retry path independently creates a synthetic user turn in the current conversation payload. It has the same role-alternation problem as the canary injection; a retry mechanism must preserve the transcript roles rather than append an assistant/user pair mid-turn.
| (e.g. ``qwen3.6-27b-256k``). Default-off => zero change to existing behavior. | ||
|
|
||
| Env: | ||
| HERMES_STALL_RETRY_MODEL retry lane/model name (required to enable) |
There was a problem hiding this comment.
These are user-facing non-secret behavior controls. Project policy requires timeouts, thresholds, flags, and similar settings to live in config.yaml, not new HERMES_* environment variables. Keep the config mapping and remove these env overrides.
Refresh fix/progress-outcome-canary onto current upstream/main and reconcile conflicts across the local-model feature stack: - chat_completion_helpers: adopt the resolve_stream_stale_timeout() extraction but re-apply upstream's reasoning-model stale-timeout floor via max() at the call site (the extraction omits it); union the anthropic/openai stale-stream client rebuild with the no-chunk abort join + TimeoutError break. - turn_context/turn_finalizer: union the progress-outcome canary + stall retry summary hooks with upstream's consolidation-reset/cleanup-errors. - runtime_provider: carry the HERMES_LLM_BASE_URL stream-tap fallback through upstream's os.getenv -> _getenv (profile-scoped) migration. - file_tools search: merge the empty-result _hint with upstream's file_read/densify redaction + credential-omission _omitted block. - conversation_loop: stall-retry failure returns now surface the block's error text as final_response (satisfies the run_conversation final_response contract instead of literal None). - Drop the release AUTHOR_MAP duplicate and the skip_missing runner machinery whose _run_one_file support was removed upstream. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
113c340 to
8abfa3b
Compare
Summary
Root Cause
The search/read/terminal guardrails catch known local loop shapes, but an agent can still burn several tool rounds doing plausible-looking diagnostics without producing a concrete outcome. There was no higher-level assertion that a turn had made an edit, changed durable state, opened/updated workflow state, or deliberately changed strategy.
Validation
./scripts/run_tests.sh tests/run_agent/test_tool_call_guardrail_runtime.py tests/agent/test_tool_guardrails.py tests/tools/test_read_loop_detection.pypython -m ruff check agent/progress_outcome.py agent/agent_init.py agent/conversation_loop.py run_agent.py tests/run_agent/test_tool_call_guardrail_runtime.pygit -c core.fsmonitor=false diff --checkFork Mirror