fix: preserve gateway terminal error recovery after save failure

[franksong2702]

Thinking Path

• Gateway terminal-error settlement can build the correct final transcript even when its final Session.save() fails.
• The run journal then becomes the only durable copy of the current terminal error.
• Recovery must therefore consume the producer persistence signal, select the authoritative terminal event, prove that the event owns the requested session and run, and rebuild the current turn in user -> partial activity -> terminal error order.
• Historical errors with identical text must remain distinct, while repeated recovery of the same journal event must stay idempotent.

What Changed

• _settle_gateway_terminal_error() now records terminal_session_persisted: false on save failure, and records true plus the exact persisted session ID only after a successful save.
• Run-journal terminal selection is centralized so recovery uses the same latest-semantic-terminal rule as journal summaries; stream_end remains transport-only.
• Cold recovery validates the exact session_id, run_id, integer seq, and derived event_id envelope before consuming a terminal payload.
• Recovery only trusts an embedded gateway transcript when its nested session IDs match and its save marker does not prove successful persistence.
• Current-turn recovery restores partial, reasoning, and tool activity before appending the specific terminal error. Dedupe is keyed by journal event identity, with a turn-scoped fallback for legacy untagged rows.
• The same ordered path now covers non-empty, core-transcript, empty-sidecar, and delayed-journal retry recovery.
• Added production-composed regressions using the real gateway producer, real journal writer, cache eviction, and cold get_session() repair.

Why It Matters

Without this consumer path, a gateway save failure could leave the exact provider error only in the journal. Reload then replaced it with a generic interruption marker, selected an older error, or placed recovered partial output after the terminal row. This change preserves the actual current-turn outcome and keeps the recovered transcript chronologically valid.

Contract Routing

• Contract family: Live-to-Final terminal recovery and replay durability.
• Invariant: only a successful save for the exact session may suppress journal recovery.
• Invariant: only the authoritative terminal event for the exact session/run envelope may settle the recovered turn.
• Invariant: recovered visible activity precedes the terminal row, and journal-event identity provides idempotency.
• Scope remains the standalone backend split from Live Stream: preserve Worklog identity across active session reattach #6304. No reattach UI, journal pagination, deployment, or protocol-shape changes are included.

Verification

• Fail-first focused matrix before the consumer fix: 12 failed, 1 passed.
• ./scripts/test.sh tests/test_session_sidecar_repair.py -k "gateway_terminal_error_cold_recovery or gateway_terminal_error_rejects or gateway_terminal_error_uses_authoritative or late_gateway_terminal_journal" --tb=short -q -> 15 passed, 67 deselected.
• ./scripts/test.sh tests/test_session_sidecar_repair.py tests/test_run_journal.py tests/test_run_journal_routes.py tests/test_webui_gateway_chat_backend.py --tb=short -q -> 155 passed.
• Neighboring recovery matrix -> 187 passed.
• .venv/bin/python scripts/ruff_lint.py --diff origin/master -> 0 findings on added or modified lines.
• git diff --check -> clean.
• Local full suite -> 13,815 passed, 114 skipped, 1 xfailed, 2 xpassed; 10 unrelated environment-dependent failures remained in atomic setgid preservation, local Hermes Agent imports/model IDs, and TTS network-policy tests. None is in a changed module; hosted CI is the clean-environment gate.

Risks / Follow-ups

• Malformed or foreign terminal evidence fails closed instead of falling back to an older error.
• This does not generalize embedded-session recovery to arbitrary event types; it is intentionally limited to the gateway apperror producer contract.
• Gateway success and cancel settlement remain separate producers.

Model Used

OpenAI Codex (GPT-5 family; exact desktop model ID not exposed), with local test and GitHub CLI tooling.

[nesquena-hermes]

Summary

Reading api/gateway_chat.py at the PR head and on origin/master, plus the full changed test file, this is the correct producer-layer fix. The persistence claim now starts false and becomes true only after Session.save() returns normally. A partial write followed by an exception is therefore treated conservatively as unpersisted, which is the safe result for terminal recovery. I do not see a blocking issue in this change.

Code reference

The key ordering at api/gateway_chat.py:760-773 is sound:

terminal_session_persisted = False
try:
    session.save()
    terminal_session_persisted = True
except Exception:
    logger.debug("Failed to persist gateway terminal error settlement", exc_info=True)
error_payload["terminal_session_persisted"] = terminal_session_persisted
if terminal_session_persisted:
    error_payload["terminal_session_persisted_session_id"] = session.session_id

This avoids the unsafe intermediate state where the exact session ID could be present after a failed save. It also preserves the in-memory, redacted session payload at api/gateway_chat.py:766-769, so recovery still has material to work with while being told that the sidecar is not durable.

Both terminal-error call sites pass the returned payload through unchanged: the runs-API exception path at api/gateway_chat.py:960-975 and the legacy gateway terminal-error path at api/gateway_chat.py:1134-1149. Because put_gateway_event() journals the same payload before queue delivery at api/gateway_chat.py:859-876, the durability marker reaches both reconnect and live consumers rather than existing only in the HTTP response path.

Diagnosis / recommendation

The change is appropriately narrow and should be mergeable as written. The important downstream rule is to require both terminal_session_persisted is True and an exact terminal_session_persisted_session_id match before compacting or discarding terminal transcript material. Missing markers from older journals and explicit false values must remain non-authoritative.

Verification step

The regressions at tests/test_run_journal_routes.py:13-75 cover both outcomes: a forced OSError leaves the boolean false and omits the ID, while a real successful Session.save() sets both fields. CI is green across lint, browser smoke, lifecycle checks, and the Python matrix. A later consumer PR should add an integration case that journals this payload, replays it, and proves that the false marker prevents terminal compaction; that belongs with the consumer behavior rather than this producer-only split.

[nesquena-hermes]

First-round gate at 2f17d1c7 — SHIP ONLY WITH FIXES: the producer signal is correct, but nothing consumes it, so the data-loss window is still open

Head reviewed: 2f17d1c7 (already on current origin/master)
Base: origin/master
Full suite: 13,870 passed / 0 failed. The diff is genuinely additive (5/0 production lines, 67/0 test lines) and regresses nothing — existing browser and journal consumers tolerate the extra payload keys.

Thanks @franksong2702 — the diagnosis is right and the producer-boundary signal is a good building block: emitting terminal_session_persisted: false when Session.save() fails (and true + the exact session ID on success) at api/gateway_chat.py:761-772 is exactly the right thing to record. But an adversarial trace + a real recovery probe show the stated bug is not actually closed, because no production code reads the new flag:

🔴 Must fix — the fix is producer-only; the consumer that discards the transcript never checks the flag

api/models.py:2480 — after a gateway terminal-error save failure and a restart, recovery discards the specific terminal transcript and replaces it with a generic interruption marker. The journal faithfully preserves terminal_session_persisted: false, but recovery reduces the journal to terminal_state (models.py:2480-2490), clears pending state, and saves the generic recovery (models.py:3192) — without ever consulting terminal_session_persisted. I confirmed with a real sidecar+journal recovery probe: the journal recorded terminal_session_persisted: false and the exact terminal transcript, yet recovery removed that transcript while reporting "recovery complete."

Grep confirms it: terminal_session_persisted appears only in api/gateway_chat.py:770-772 and the producer-only tests. There is no consumer line anywhere in api/*.py or static/*.js that reads it. So today the field is emitted into the apperror event and dropped on the floor.

Fix:

1. api/models.py:2480 — during terminal-error recovery, inspect the terminal apperror payload: only treat its embedded session as durable when the flag is exactly true and terminal_session_persisted_session_id matches the recovered session. When the flag is false, absent, or mismatched, validate and materialize/merge the journal's embedded terminal transcript before clearing pending state — don't replace it with the generic interruption marker. That's the line that actually closes the data-loss window this PR is about.
2. tests/test_run_journal_routes.py — the two new tests only assert the producer emits false/true. Extend the failed-save test through actual journal append and cold sidecar recovery, asserting the specific gateway terminal transcript survives recovery — not just that the producer emitted the flag. A test that fails before the models.py fix and passes after is the contract here.

Summary

Nothing to undo — the producer half is correct and safe. It just needs the consumer wired (the whole point of the flag) plus an end-to-end recovery test that proves the transcript survives. Re-push with models.py:2480 reading the flag and I'll run a clean full gate.

[franksong2702]

Implemented the requested consumer-side recovery fix in ded12e4.

• Recovery now validates the journal/session identity and only treats terminal persistence as authoritative for an exact true marker plus matching session ID.
• For false, absent, or otherwise non-authoritative persistence, it materializes the validated embedded gateway error transcript and avoids replacing it with the generic interruption marker.
• Added an end-to-end sidecar/journal regression proving an unsaved gateway terminal error survives recovery.

Verification: ./scripts/test.sh tests/test_session_sidecar_repair.py tests/test_run_journal_routes.py tests/test_webui_gateway_chat_backend.py (127 passed); ruff diff clean; git diff --check clean.

[nesquena-hermes]

Re-gate at ded12e48c5 — changes requested: the marker is consumed now, but cold recovery can still select and order the wrong terminal error

Thanks for wiring the producer marker into api/models.py and adding a recovery test. The two original trust checks are now correct: only literal terminal_session_persisted is True plus the matching persisted session ID suppresses recovery.

The current consumer still has reproducible data-integrity problems on the exact head:

1. It can recover the wrong turn's error. _materialize_unsaved_gateway_terminal_error() walks journal events from the front and then walks the embedded full transcript from the front, returning on the first assistant _error row (api/models.py:2516-2560). A cold-sidecar probe with an older provider error followed by the current gateway error retained only the old error. The content-only session-global dedup at api/models.py:2548-2555 also collapses identical error text from two distinct turns.
2. It makes a partial output follow the terminal error. All three repair branches call _materialize_unsaved_gateway_terminal_error() before _append_journaled_partial_output() (api/models.py:3263-3269, 3324-3330, 3375-3381). The recovered transcript becomes [terminal error, partial output], so the error is no longer terminal. This reproduced through the real gateway terminal producer, real journal append, and cold sidecar reload, as well as both empty/core-sidecar branches.
3. It does not bind the consumed event to the requested journal owner or authoritative terminal event. The helper checks nested payload IDs only when present, but does not validate the envelope session_id, run_id, seq, or event_id. A foreign envelope stored under the target path was accepted when its nested IDs matched. It also accepts the first qualifying apperror, even when a later terminal event supersedes it.
4. The submitted test is not the requested cold-reload regression. It appends an event and calls _apply_core_sync_or_error_marker() on the same in-memory Session; it does not feed the exact producer payload through the journal and then evict/reload the stale sidecar through get_session() / _repair_stale_pending().

Mandatory threat scan was CLEAN. The changed regression files pass in the no-network sandbox (85 passed), but reviewer-owned sandbox probes reproduced the cases above. After removing the temporary probes, the changed files were rerun clean.

Required fix

• Select the authoritative/latest terminal event using the journal's existing terminal-order semantics, and validate its envelope against the requested session_id and stream_id before consuming the payload.
• Select the current terminal error at the current-turn boundary, not the first historical _error in the embedded transcript. Deduplicate by stream/turn identity, not error text across the whole session.
• Recover partial/reasoning/tool output first and reconcile the specific terminal error last in every repair branch.
• Add a production-composed regression that uses the real terminal producer payload, real journal writer, cache eviction, and cold sidecar repair. Cover prior/repeated errors, partial-before-error ordering, foreign/malformed identity, later-terminal supersession, and the non-empty/core/empty sidecar branches.

This is a partial convergence, but the reported recovery path can still lose or misattribute the current provider error, so it is not ship-ready yet.

[franksong2702]

Addressed the re-gate at dadffe0e02: authoritative terminal selection now reuses run-journal semantics; exact envelope ownership gates both completed and error recovery; partial activity is restored before the terminal row across all three cold-repair branches; and dedupe is event/turn-scoped. The production-composed cold-reload matrix and neighboring recovery suites pass, and all 21 checks on this head are green.

[nesquena-hermes]

Round-2 re-gate at dadffe0e — PASS. The consumer is wired and the data-loss window is closed.

Head: dadffe0e (on current origin/master) · Full suite: 13,885 passed / 0 failed · Codex: SAFE TO SHIP.

My Round-1 bounce was that the PR was producer-only: gateway_chat.py emitted terminal_session_persisted: false on a save failure, but nothing read it, so recovery still discarded the specific terminal transcript and wrote a generic interruption marker despite the flag. This rework (+72/-0 → +693/-55) wires the consumer exactly as requested:

• models.py:2566-2571 now reads terminal_session_persisted and terminal_session_persisted_session_id, and skips recovery only when persisted is True AND persisted_id == session.session_id; otherwise (false / absent / mismatched) it materializes the terminal-error transcript from the journal (_materialize_unsaved_gateway_terminal_error, 2627+).
• Idempotence is guarded: an existing message with the same _recovered_event_id short-circuits (2643-2648), and same-turn matching errors are marked recovered rather than duplicated (2650-2666).

Verified by the adversarial gate (each reproduced):

• Real gateway worker save-failure → restart recovery preserved the exact terminal error and partial output — no generic marker. The original data loss is closed.
• Repeated restart produced exactly one recovered error (idempotent).
• Persisted-ID match skips recovery; mismatch, absent, or None recovers — all three flag branches correct.
• Foreign run/session identities and malformed content fail closed.
• Latest valid assistant error is selected correctly.
• Healthy gateway sends, successful terminal saves, completed-journal recovery, lazy retry, and unrelated journal summaries all remain intact — no regression.

Resolving my prior CHANGES_REQUESTED. Solid work @franksong2702 — this is the fix-the-class version: the signal is now consumed at the decision point, and it fails safe on every uncertain branch.

Routing to Nathan for the merge decision (pure-backend reliability, gate-clean).