fix(agent): persist recovered final responses

[WXBR]

Summary

Close non-interrupted recovered/fallback final_response turns with a durable assistant message before session persistence.

This prevents the persisted transcript from ending at a tool or user message after the caller has already received a completed assistant response. If that malformed tail is saved, the next turn can reload stale/incomplete history and appear to repeat or lose the visible final answer.

Relation to #46071 / #46053

This is the same persistence-loop bug family as #46071 / #46053, but it is not the same root cause.

• fix(agent): persist repaired-turn responses #46071 fixed _flush_messages_to_session_db() losing newly appended assistant messages when repair_message_sequence() shortened messages below conversation_history length.
• This PR covers the adjacent path where finalize_turn() has a non-empty final_response, but no closing assistant message has been appended to messages before _persist_session().

In other words, #46071 makes flushing robust once an assistant message exists. This PR ensures the recovered final response exists in the transcript before persistence.

Changes

• agent/turn_finalizer.py: before _persist_session(), append {role: "assistant", content: final_response} when the turn is not interrupted, final_response is non-empty, and the transcript tail is not already assistant.
• tests/agent/test_turn_finalizer_final_response_persistence.py: regression for a transcript ending at a tool result while final_response="Done." is returned.

Validation

Check	Result
RED: python -m pytest tests/agent/test_turn_finalizer_final_response_persistence.py::test_final_response_closes_tool_tail_before_persistence -q -o 'addopts=' on clean origin/main	failed as expected: tail persisted as tool
GREEN: same focused test after fix	pass
python -m pytest tests/agent/test_turn_finalizer_final_response_persistence.py tests/run_agent/test_identity_flush.py tests/run_agent/test_compression_persistence.py tests/run_agent/test_860_dedup.py -q -o 'addopts='	19 passed, 1 warning
python -m py_compile agent/turn_finalizer.py tests/agent/test_turn_finalizer_final_response_persistence.py	pass
git diff --check	pass

Duplicate check

Searched existing issues/PRs and code for exact matches using terms including turn_finalizer final_response, final_response assistant tail, final_response tool tail, pending tool result, and exact code/test markers. Found #46071/#46053 as related persistence repair work, but no exact duplicate for closing a recovered final_response in agent/turn_finalizer.py before persistence.

[liuhao1024]

Verification comment — clean review

Reviewed the diff (3 files: agent/turn_finalizer.py, new test file, 112 lines).

What was checked:

• Not a dead variable: final_response is the function parameter — always defined in scope.
• Guard correctness: if final_response and not interrupted correctly scopes the fix to non-interrupted turns with a real response. Interrupted turns should NOT have their response appended (the interruption message is the last visible content).
• Tail-role check: messages[-1].get("role") with a try/except fallback handles empty-messages edge case safely.
• No double-append risk: if the messages already end with an assistant message, the guard (_tail_role != "assistant") prevents duplication.
• Test regression scenario: the test constructs a transcript ending at a tool result, calls finalize_turn with final_response="Done.", and asserts both the return value and persisted_messages end with the assistant message. This directly validates the bug described in the PR — recovery/fallback paths that return a real response without closing the durable turn.

The fix is minimal (7 lines of logic) and correctly addresses the session-history corruption path: without it, a durable session can end at a tool/user message even though the caller received a completed assistant response, causing the next turn to reload malformed history.

[teknium1]

Thanks for the careful writeup and the RED/GREEN validation, @WXBR — the observation is real, but this is a close, because the fix direction here is one we've already evaluated and deliberately did not adopt.

The real bug was at the flush layer, not the transcript. The observable failure (a delivered assistant reply missing from state.db) was a SessionDB flush problem — positional-index flushing dropping the assistant tail, and stream-delivery flags marking turns already-delivered. We fixed it there:

• fix(agent): persist repaired-turn responses #46071 (merged) — identity-based SessionDB flushing so repair-shrunk history can't drop a newly appended assistant message.
• fix(gateway): close silent response loss after agent tool calls (#29346) #34336 / fix(agent): surface silent partial-stream recovery (salvage #41498) #53987 (merged) — scope the stream-delivery flags at tool boundaries and deliver recovered originals via the gateway (with a visible "send continue" explainer on partial_stream_recovery).

Appending a synthetic {role: assistant, content: final_response} in finalize_turn is the approach we declined. The identical mechanism (_ensure_final_response_in_messages(), targeting the same two break paths — fallback_prior_turn_content and partial_stream_recovery) was proposed in #31291 and closed as implemented_on_main, with the note: "The PR's proposed helper was not adopted… current main now fixes that persistence failure with stronger flush semantics." The origin issue #31269 was closed NOT_PLANNED, and #31314 is the same stale-open idea.

It also works against #53987 specifically: on partial_stream_recovery we intentionally leave response_was_previewed=False so the gateway sends the recovered fragment plus the "stream died mid-sentence, send continue" explainer. Backfilling that fragment into durable history as a complete assistant turn masks the truncation and pre-empts that continue-flow.

The tool-tail close via close_interrupted_tool_sequence was deliberately scoped to interrupt aborts (#48879, #52592) — the non-interrupted recovery paths were left to the flush-layer + gateway-delivery fixes above.

Closing as superseded by #46071 and covered by the flush-layer work on current main. Genuinely appreciate the diligence — the analysis was sound, it just landed on a mechanism we'd already moved past.

[teknium1]

Merged via PR #56279 — your commit was cherry-picked onto current main with your authorship preserved (commit 59e7e9d). Thanks for fixing this at the finalize_turn chokepoint rather than per-site; that's exactly why this approach was chosen over the other candidates in the cluster.

Live E2E confirmed the fix: recovery-path turn went from persisting [user, assistant, user, user] (delivered response dropped, broken alternation) to [user, assistant, user, assistant], with no duplicate on normal turns.