The suppression predicate _transcript_already_advanced_past_pending()
required a run-journal terminal 'completed' state, but its caller in
_apply_core_sync_or_error_marker already returns early on that same
condition. The predicate branch was therefore unreachable dead code: it
could never suppress anything, and stale-cancel recovery could still
append a duplicate recovered user row, a journal clone and an
interruption marker for a turn that actually finished.
Derive the completion evidence from the crash-safe turn journal instead:
_turn_journal_records_completion() reports True when the exact-stream
'completed' event is the latest terminal state for the pending stream id.
The run journal's terminal write is not durable evidence for this path
(it is consumed by the caller's early return), so the turn journal is the
only still-observable source. A cancelled/crashed turn (interrupted)
keeps recovery active, and completion for another stream of the same
session never settles the pending one.
Also record the same completion event on the Gateway success writeback
path, so Gateway-backed runs have the equivalent completion persistence
instead of leaving the run journal's lost terminal write unrecoverable.
Regression coverage:
- test_turn_journal_helper_reports_completed_for_matching_stream
- test_turn_journal_helper_rejects_interrupted_turn
- test_advances_past_pending_from_turn_journal_without_run_journal_terminal
- test_full_recovery_suppresses_duplicates_on_turn_journal_completion
- test_full_recovery_appends_rows_on_turn_journal_completion_loss
- test_gateway_chat_worker_records_turn_journal_completion
Reverting the predicate gate turns the two new CORE regressions red;
reverting the Gateway write turns the Gateway regression red.
Re-review #7682 (nesquena-hermes 2026-09-22T19:18:00Z, CHANGES_REQUESTED).
`_transcript_already_advanced_past_pending` suppressed recovery on a
transcript-only heuristic: the first assistant row with final visible text
inside the pending turn's boundary. An interrupted multi-segment turn shaped
like
user -> assistant "Let me check the logs first." (unflagged interim prose)
-> tool -> tool ... (stream dies here)
passed the predicate as "finished" even though the interim sentence is not a
final answer and the stream was cut off mid-tool. `_apply_core_sync_or_error_marker`
then cleared pending state and returned early: the run journal was never
replayed and no interruption marker was added — a half-finished turn rendered
as complete, journaled tool output never appearing.
Fix: a genuine final answer inside the turn is now NECESSARY BUT NOT
SUFFICIENT. The suppression also requires durable same-stream terminal
evidence — `_run_journal_terminal_state(session, active_stream_id) == 'completed'`
(a `done`/`stream_end` event). Without it, fall through to the normal
journal-recovery path so the partial output is replayed and the interruption
is marked.
Regression tests (red/green verified — both fail with the gate removed):
- test_does_not_advance_when_unflagged_interim_prose_precedes_tools
- test_full_recovery_runs_journal_replay_for_unflagged_interim_prose
(drives `_apply_core_sync_or_error_marker`, asserts journal replay +
interruption marker present)
- test_does_advance_when_journal_marked_done_after_interim_prose (positive)
Existing `_FakeSession` fixtures updated to carry a session_id and write a
`done` journal event so the durable-evidence gate is satisfied for the
settled-turn tests.
Verification: test_issue6366 35 passed; anchor_scene + cancelled_turn
neighbours 91 passed. Red/green proven on both regressions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #6366 suppression was too broad and could destroy data in two ways,
both verified with full `_apply_core_sync_or_error_marker()` probes:
1. A repeated prompt could be silently discarded. Checkpoint matching
truncates timestamps to integer seconds and ignored
`_active_turn_token`, so a prior identical prompt from a *different*
stream could match the new pending turn — and the suppression branch
then cleared the only durable copy of the prompt, with no recovered
row and no error marker.
Identity is now bound to the pending stream's exact active-turn token
(`_transcript_user_row_is_pending_turn`). Integer-second timestamp
equality never overrides a token mismatch, and a row that carries no
token cannot be proven to be this turn. Legacy sessions with no token
on either side fall back to the strict content + timestamp + source +
attachments checkpoint.
2. Interrupted tool-running turns were treated as completed. Any
unflagged assistant row counted as settled, including empty rows,
tool-call-only rows, interim-progress rows and compaction rows, so
`:3600` bypassed journal replay and the interruption marker.
Completion now reuses the established final-answer semantics
(`_pending_turn_has_final_assistant_answer` ->
`_assistant_message_has_final_visible_text`) inside the matched
user-turn boundary, rejecting tool-only, empty, interim, journal-
recovered and compaction tails.
The suppression is deliberately asymmetric: a false negative leaves a
visible cosmetic duplicate the user can ignore, while a false positive
silently destroys a prompt or a response. Whenever identity or
completion cannot be positively proven, recovery runs instead of
suppression.
Tests: the 11 predicate unit cases are re-derived against the new
identity/completion helpers, plus full-recovery-path cases that exercise
`_apply_core_sync_or_error_marker()` itself — distinct same-second
repeated turns (token-bearing and legacy) proving the prompt survives,
and empty / tool-call-only / interim / compaction tails proving journal
replay and the interruption marker still run — plus a positive control
for the suppression branch itself.
ruff F401 flagged both imports; the test references
``_transcript_already_advanced_past_pending`` directly via the
``from api.models import (...)`` form below. Dropped the unused
``import pytest`` and the aliased module import.
Files: tests/test_issue6366_stale_cancel_recovery.py:23,25
Model: MiniMax-M3
Stale-cancel recovery in _apply_core_sync_or_error_marker
(line ~3497+) only inspects the transcript tail
(`session.messages[-1]`) when checking whether the durable
transcript already contains the pending turn. When the pending
turn's user row is older than the tail, the tail is a *newer*
settled assistant row that can never match the pending **user**
checkpoint, and the recovery path falls through into the append
branch. The cancel/recovery tail then appends, after the valid
final answer, a recovered user row, a `_partial` clone of the
old journal, and a generic no-response error that survives reload
in the durable transcript.
The fix adds `_transcript_already_advanced_past_pending()`
which scans the messages for any strictly-matching user row
(content + timestamp + source + attachments) and, when one is
found, looks for a later settled assistant row (no
`_partial` / `_error` / `_recovered`). When that is the
case, the helper returns True and the non-completed-journal
branch clears only the stale pending fields and returns
without appending any rows.
Strict match is required so that a *new* user turn that happens
to repeat the same prompt text (e.g. user re-sends the same
question) is not misclassified as a stale advance. The
regression guard is the existing
`test_repeated_pending_prompt_uses_current_checkpoint_identity`
which now passes alongside the new helper unit cases.
The helper is a pure read; running recovery twice therefore
produces the same outcome, satisfying the maintainer's
idempotence requirement.
Validation: 11/11 new tests pass (covers the strict-match
distinction, the partial/error/recovered skip, and idempotence);
124/124 related stale-pending / sidecar-repair / recovery tests
pass; py_compile clean.
happy5318