fix(conversation-loop): recover when a leaked "(empty)" sentinel ends a turn

[samfoy]

What does this PR do?

A turn could end on a non-answer. When a model emitted a final response that began with Hermes' own "(empty)" sentinel followed by scratchpad shorthand, run_conversation treated it as a completed text turn — finish_reason=stop, shorthand persisted as the assistant message, task abandoned mid-flight.

Observed live on a Responses-API provider, one tool round after a PASS:

finish_reason : stop
content       : (empty) again risk. Need progress + tool. Already can. Use execute.

That ended a ~27-minute multi-step task, and the visible "answer" was the model's own planning note.

The recovery for this class already existed — the post-tool empty-response nudge. It was unreachable because the whole ladder is gated on the response being contentless:

if not agent._has_content_after_think_block(final_response):

The leak carried 67 characters, so it read as content and skipped every branch. A garbage-but-present final answer is strictly worse than an empty one: empty is recoverable, garbage is not.

This PR routes a leaked sentinel into that existing ladder. It adds no new recovery machinery.

Why the sentinel prefix is a safe signal. "(empty)" is not model text — Hermes injects it (_nudge_msg["content"], and the empty terminal). A model echoing it back has leaked a planning note. Measured against a real session DB of 3,828 non-trivial turn-ending assistant messages: (empty) appears as a prefix exactly once — this bug. [Tool loop warning, [System:, [IMPORTANT:, the interrupt scaffolding, [TRANSCRIPT and <untrusted_tool_result appear zero times, which is why the guard is deliberately narrow rather than a general shorthand heuristic. (Single-user data — indicative, not a population estimate.)

Related Issue

Fixes #83505

Adjacent, not duplicated — all three touch the same recovery ladder in conversation_loop.py, so whoever merges first will make the others rebase:

• fix: add _has_visible_content guard to thinking-prefill retry gate #64733 — _has_visible_content guard on the thinking-prefill retry gate
• fix(agent): nudge once before the housekeeping empty-follow-up shortcut #65674 — nudge once before the housekeeping empty-follow-up shortcut (issue [Bug]: Housekeeping-tool empty-follow-up shortcut wrongly ends the turn when a weak/quantized local model chokes instead of finishing #65600)
• fix(agent): surface prior-turn content as last-resort fallback #70668 — surface prior-turn content as last-resort fallback

Sibling issue #24933 (closed) fixed the commentary-phase form of the same scratchpad leak; this is the case where the note arrives stamped final_answer instead.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

agent/conversation_loop.py

• EMPTY_RESPONSE_SENTINEL = "(empty)" — module-level, shared with the injection sites so the guard cannot drift from them.
• _visible_text_for_sentinel_check(content) — returns only visible assistant prose from a str, a list of blocks, or a mapping. Drops thinking / reasoning / redacted_thinking / reasoning_content parts. Never raises: a bad shape yields "" so the guard simply does not fire (fails toward the pre-existing terminate behaviour rather than crashing a turn).
• _leaked_empty_sentinel — true when the visible text starts with the sentinel and the response is otherwise non-empty; OR-ed into the existing empty-recovery gate.
• The partial-stream recovery branch inside that gate is fenced with not _leaked_empty_sentinel. Without this the fix is a no-op with extra steps: the leaked text was streamed, so _current_streamed_assistant_text holds it and would be re-delivered verbatim.

tests/test_leaked_empty_sentinel_guard.py (new) — 17 tests / 23 subtests.

Two deliberate design choices, both learned the hard way in review:

• No mirrored oracle. The predicate is borrowed off the real class (_MinimalAgent._has_content_after_think_block = AIAgent._has_content_after_think_block, pinned with assertIs) and the guard expression is extracted from source with ast.unparse, then evaluated against the production helpers. An earlier revision defined its own copies and 7 of 9 tests passed with the fix reverted. A revert now fails at collection.
• Both directions asserted. The leak must recover; "Done.", "PASS", "OK", "42", markdown answers, and substring mentions (`(empty)` is the sentinel) must still end the turn.

Two behaviours documented in-code rather than changed, to keep this to one concern:

• Recovery is not guaranteed. It depends on the post-tool nudge still being available; with the nudge latch already consumed this turn the leak reaches the empty terminal and the user sees a bare (empty). Still better than shipping shorthand as an answer, and the comment now says so instead of claiming "must recover".
• No infinite-loop risk from the terminal's own "(empty)": it assigns and breaks immediately, so the guard is never re-evaluated.

How to Test

1. Reproduce the gate (no provider needed). The condition is a pure expression over final_response:

import re
def has_content(c):   # mirrors _has_content_after_think_block
    return bool(re.sub(r"<(think|thinking|reasoning)>.*?</\1>", "", c or "",
                       flags=re.S | re.I).strip())

leak = "(empty) again risk. Need progress + tool. Already can. Use execute."
print(has_content(leak))   # True -> old gate skipped -> turn terminated

2. Run the new module:

python -m pytest tests/test_leaked_empty_sentinel_guard.py -q
# 17 passed, 23 subtests passed

3. Confirm the tests are revert-sensitive (this is the part worth checking):

# remove the guard from agent/conversation_loop.py, then:
python -m pytest tests/test_leaked_empty_sentinel_guard.py -q
# ERROR at collection: `_leaked_empty_sentinel = ...` not found — the fix is missing

# or keep the guard but make _visible_text_for_sentinel_check skip the
# reasoning-exclusion, then:
# 6 subtests fail, each naming the (part_type, key) shape that regressed

4. Suite:

python -m pytest tests/run_agent/ -q

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits
• I searched for existing PRs to make sure this isn't a duplicate — closest are fix: add _has_visible_content guard to thinking-prefill retry gate #64733 / fix(agent): nudge once before the housekeeping empty-follow-up shortcut #65674 / fix(agent): surface prior-turn content as last-resort fallback #70668, listed above as adjacent
• My PR contains only changes related to this fix (2 files, no unrelated commits)
• I've run pytest tests/ -q and all tests pass — see note below
• I've added tests for my changes
• I've tested on my platform: Amazon Linux 2 (aarch64), Python 3.11.15

Note on the test checkbox, stated plainly rather than ticked: tests/run_agent/ reports 7 failures on this branch — and the identical 7 on a pristine origin/main worktree. I captured failing test IDs both ways in the same container and diffed them: 0 introduced, 0 fixed. All 7 are ModuleNotFoundError: anthropic, an artifact of my minimal test image installing only the dev extra. I have not run the entire tests/ tree, so I can't honestly claim all-green.

Verification ran in Docker against a throwaway clone (--network none, repo mounted read-only, HERMES_HOME redirected) because this repo is also my live agent install and an in-place run can be rewritten mid-suite.

Documentation & Housekeeping

• I've updated relevant documentation — N/A; rationale lives in code comments beside the guard
• I've updated cli-config.yaml.example if I added/changed config keys — N/A, no new config
• I've updated CONTRIBUTING.md / AGENTS.md — N/A, no architecture change
• I've considered cross-platform impact — pure Python string/dict handling, no OS-specific paths or APIs
• I've updated tool descriptions/schemas — N/A, no tool behaviour change

Screenshots / Logs

The real session row that motivated this, from the local session DB:

finish_reason      : stop
content len        : 206        <- an earlier instance: a progress sentence
codex_message_items:
   phase='final_answer' status='completed' text='The queue is broad and includes...'

and the one that leaked the sentinel:

finish_reason : stop
content       : (empty) again risk. Need progress + tool. Already can. Use execute.

Both were mid-task, both had just completed successful tool calls.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(conversation-loop): recover when a leaked "(empty)" sentinel ends a turn

1. The regression test reads production source from disk and ast.parse/evals the guard expression (tests/test_leaked_empty_sentinel_guard.py). Beyond conflicting with the repo's "never read source code in tests" hardline, it fails at import/collection time if the guard is refactored (renamed, inlined, extracted into a helper) — the whole test module breaks even when the behavior is correct. Since the file already imports _visible_text_for_sentinel_check and EMPTY_RESPONSE_SENTINEL from production, extracting the guard into a pure helper that both sides call would achieve the anti-drift goal without source inspection.

2. The documented false-positive edge: a legitimate answer that genuinely begins with "(empty)" (e.g. the user asked what a placeholder should be) is misrouted into recovery, and with the nudge latch already consumed this turn, the user sees a bare "(empty)". The 1/3804 measurement makes this acceptable, but a slightly stricter trigger — sentinel followed by whitespace and word characters, matching the observed scratchpad shape — would narrow it further.

3. _visible_text_for_sentinel_check returns "" on any exception, which is the safe direction but also silently swallows genuine shape-regression bugs in the flattening logic. A rate-limited warning log would make silent failures visible without spamming.

[samfoy]

cf74a7c730 is the new head, and the PR reports MERGEABLE with unchanged scope: 2 files, 318 insertions, and 3 deletions.

• The rebase from 5,682 commits behind main produced one additive conflict in agent/conversation_loop.py: upstream added _COMPRESSION_TIMEOUT_FINAL_RESPONSE, while this branch added EMPTY_RESPONSE_SENTINEL, _NON_VISIBLE_CONTENT_PART_TYPES, and _visible_text_for_sentinel_check.
• I kept both additions, placed upstream first, dropped no code, and verified references at lines 3143, 6624, and 9324 plus the guard call at 8399–8401.
• The patch remains byte-identical: both merge-base-to-tip stats report 2 files, 318 insertions, and 3 deletions, with only index hashes and @@ line numbers different.
• The regression test reports 17 passed with 23 subtests, while scripts/run_tests.sh reports 794 passed and 0 failed across 31 relevant files.
• Plain main reports 777 passed across 30 files, so this PR's test file accounts for the exact 17-test difference.
• The branch is three commits behind main again, but they touch neither PR file, merge-tree reports no conflict, and I can rebase again on request.