[codex] fix dflash malformed final recovery

[OmarB97]

Summary

• detect malformed non-empty final responses after tool calls when a model reports finish_reason=stop but ends with degenerate repeated punctuation
• retry once with a clean regeneration prompt, then use the configured fallback chain if the malformed final repeats
• keep malformed recovery scaffolding out of persisted transcripts
• tighten background skill review instructions so unverified foreground diagnoses are not saved as durable debugging rules

Root Cause

The dflash turn did not fail as an empty response or stream timeout. It returned visible text with finish_reason=stop, so Hermes treated the corrupted final as successful and triggered the background skill-review loop. The skill reviewer then learned from the foreground assistant's unsupported diagnosis.

Validation

• python3 -m pytest -q -o addopts='' tests/run_agent/test_malformed_final_recovery.py tests/run_agent/test_empty_response_recovery_persistence.py tests/run_agent/test_background_review.py::test_background_review_installs_auto_deny_approval_callback tests/run_agent/test_background_review.py::test_background_review_summary_is_attributed_to_self_improvement_loop tests/run_agent/test_partial_stream_finish_reason.py tests/run_agent/test_provider_fallback.py tests/run_agent/test_retry_status_buffer.py
• python3 -m compileall -q run_agent.py agent/conversation_loop.py agent/background_review.py tests/run_agent/test_malformed_final_recovery.py
• git diff --check

Fork mirror: OmarB97#49
Closed dirty mirror superseded by #49: OmarB97#48

Note: local pytest needed -o addopts='' because this shell lacks the project timeout plugin.

[OmarB97]

Added follow-up commit eb3ca72e to cover the 2026-05-31 14:04 PDT live dflash/TUI failure where a post-tool final ended on the open connector But with finish_reason=stop. Hermes now classifies that as a malformed final-response tail and regenerates instead of ending the turn.

Validation: python3 -m pytest tests/run_agent/test_malformed_final_recovery.py -q

[mxnstrexgl]

LGTM — automated review passed. No security, quality, or test coverage issues detected.

[OmarB97]

Refreshed this branch onto current upstream main as a single refresh commit (transplant — the branch predated the history replacement, so there was no merge base to rebase across). The change is identical to the original diff except for one conflict resolution in agent/conversation_loop.py: main added a one-shot _emit_pending_fallback_notice() at the successful-content point, and the recovery block now sits ahead of it so the notice still fires only on genuinely successful content — its successful-recovery vs terminal-failure semantics (buffered-trace flush discards the pending notice) hold unchanged for the new malformed-final paths. The other five files applied cleanly. Original head was ba929207270ed97efbd8dcb10f4a9a50dcadf396 for provenance; the branch's focused tests (test_malformed_final_recovery.py, test_empty_response_recovery_persistence.py) pass on the refreshed head, and checks are re-running.

[OmarB97]

Refreshed onto current upstream/main (was 106 commits behind). Clean rebase of the existing single-commit refresh (d06a7bae04) — zero conflicts, no hand edits needed; the 6-file/348-insertion diff is byte-identical to the prior refresh. Original PR head (pre-refresh): ba929207270ed97efbd8dcb10f4a9a50dcadf396. New head: 5a111cb3f9. Ran the two directly-touched suites locally (tests/run_agent/test_malformed_final_recovery.py, tests/run_agent/test_empty_response_recovery_persistence.py) — 10 passed. Force-pushed with lease; checks re-running.

[teknium1]

Thanks for the focused malformed-final recovery and the regression coverage. Current main still accepts non-empty post-tool finish_reason="stop" text through agent/conversation_loop.py:5089-5097, so the underlying failure mode remains relevant.

Problems

• The fallback branch at agent/conversation_loop.py:5118 calls _try_activate_fallback() but does not synchronize the already-built api_messages. _sync_failover_system_message() exists specifically for this case (agent/conversation_loop.py:497-520), and every current fallback path invokes it. try_activate_fallback() rewrites the cached prompt identity at agent/chat_completion_helpers.py:1638-1640; without the sync, the fallback request can retain the primary identity.
• tests/run_agent/test_malformed_final_recovery.py:139-182 verifies the fallback model and output but not the fallback request's system-message identity.

Suggested changes

• Mirror the adjacent empty-response fallback path (agent/conversation_loop.py:5027-5030) by syncing active_system_prompt and api_messages after fallback activation.
• Add an assertion for the fallback request's system prompt in the malformed-final fallback regression.

Automated hermes-sweeper review.

[teknium1]

Please synchronize the in-flight api_messages after this fallback, as every existing fallback branch does via _sync_failover_system_message(...). _try_activate_fallback() rewrites the cached Model/Provider identity, but this call block reuses the pre-fallback API message list; without the sync the fallback request can carry stale identity.

[teknium1]

Thanks for the deep investigation here — the root-cause analysis (visible degenerate text with finish_reason=stop slipping past the empty-response recovery and then poisoning the background skill review) is genuinely well done, and the engineering is careful: scaffolding kept out of persisted transcripts, single retry then fallback chain, good test coverage.

Closing on policy rather than quality. The trigger is a heuristic content classifier over model output (trailing repeated-punctuation runs, open connector words) — that's open-ended coverage for degenerate output from a specific weak model, and the set of ways a bad model mangles a final response grows forever, each variant needing a new branch. We hold the line that we don't add recovery layers that absorb bad model outputs: the existing empty-response recovery stays because "zero content" is an objective wire-level failure signal, but "this text looks degenerate" is a quality judgment we don't want the agent loop making. The right fix for dflash-shaped finals is upstream (model choice / provider), not detection heuristics in the conversation loop.

Two pieces of this PR are independently valuable and NOT covered by the policy close — you're welcome to resubmit them standalone:

1. The background-review prompt hardening (not learning durable "debugging rules" from unverified foreground hypotheses) — that's a real improvement to the self-improvement loop regardless of what triggered it.
2. The transcript-hygiene guarantee that recovery scaffolding never persists.

Appreciate the work — this was a high-quality PR that happens to sit on the wrong side of a deliberate line.