fix(memory/hindsight): flush buffered turns on session exit

[bernardokyotoku]

Problem

When retain_every_n_turns > 1, sync_turn() only enqueues a retain every Nth turn. Turns 1..N-1 accumulate in _session_turns and are silently discarded on exit (Ctrl-D, /quit, /exit).

The plugin already handles this for session switches — on_session_switch() flushes buffered turns before clearing (it explicitly calls out this data-loss class in its docstring). But the exit path goes through on_session_end() + shutdown():

• on_session_end() — base class implementation is a no-op (Hindsight plugin does not override it)
• shutdown() — only drains already-queued retains from the writer thread; does not flush buffered-but-unqueued turns from _session_turns

Fix

Adds on_session_end() to HindsightMemoryProvider that flushes the unretained delta through the same writer queue. Since on_session_end is called before shutdown_all in the exit sequence (cli.py:1049 → run_agent.py:3141), the flush is enqueued first, then shutdown()'s sentinel follows — the writer processes the flush before exiting (FIFO).

Refactor

The flush logic is extracted into a shared _enqueue_buffered_flush() helper. Both on_session_end and on_session_switch now call it, removing 36 lines of duplication. on_session_switch shrinks from 127 to 82 lines.

The helper handles: metadata building, lineage tags, content serialization, the _flush closure, and writer-queue routing. The caller handles:

• Which turns to send: on_session_end sends only the unretained delta (avoids append-mode duplicates); on_session_switch sends all turns (it clears the buffer anyway)
• When to resolve document_id: on_session_switch must resolve before session rotation; on_session_end uses current state

Design choices

• No-op when retain_every_n_turns=1 (default): _session_turns is empty at exit because every turn was already retained via sync_turn. The first guard (if not self._session_turns: return) makes the patch zero-risk for the default config.
• Delta-only in append mode: on_session_end slices from _last_retained_turn_count to avoid duplicating turns the server already has. In overwrite mode (legacy servers), it resends everything.
• Best-effort: failures are caught and logged as warnings — same as on_session_switch. No crash or data corruption if the API is down.
• Does not clear _session_turns: the provider is being torn down, not rotated. No need to reset state that's about to be destroyed.

Verification

All 116 existing Hindsight plugin tests pass, including all 5 TestSessionSwitchBufferFlush tests that verify the refactored on_session_switch still flushes correctly.

116 passed in 1.76s

Not covered by this patch

The atexit hook (_atexit_shutdown, line 1143) calls shutdown() directly, bypassing on_session_end. This is a pre-existing safety-net limitation for unclean interpreter exits — fixing it would require registering the flush in the atexit handler, which is a separate change.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #36219 — same on_session_end() writer-queue flush of buffered _session_turns in plugins/memory/hindsight/__init__.py, fixing the same silent-data-loss bug (#36216) when retain_every_n_turns > 1. #36219 is the earliest open canonical fix. Competing open fix: #55046. This PR adds a shared _enqueue_buffered_flush() refactor, which a maintainer may prefer — but the core mechanism is the same.

[teknium1]

Thanks for targeting a real buffered-turn loss on shutdown. Current main leaves partial batches in plugins/memory/hindsight/__init__.py:1621-1629, and agent shutdown does call provider on_session_end() before teardown (run_agent.py:3323-3330).

Problems

• The new hook also runs during session rotation: agent/memory_manager.py:796-825 invokes on_session_end() immediately before on_session_switch(). The PR queues the append delta, while the current switch hook then queues all buffered turns (plugins/memory/hindsight/__init__.py:1819-1881). This duplicates retained data on append-capable Hindsight servers.
• The PR changes only the provider source and adds no regression test for either exit flushing or the end-then-switch boundary. Existing switch-flush tests begin at tests/plugins/memory/test_hindsight_provider.py:1196.

Suggested changes

• Coordinate the end and switch hooks so a buffered tail is queued exactly once at a rotation.
• Add exit and append-mode end-then-switch regression coverage.

Automated hermes-sweeper review.

[mlapida]

Heads up on an interaction with #41911 — this PR as written would reintroduce a duplicate-turn bug that PR fixes.

The flush added here snapshots the whole buffer:

old_turns = list(self._session_turns)

In append mode (update_mode='append', Hindsight >= 0.5.0) sync_turn has already shipped each turn past _last_retained_turn_count and advanced that watermark. Re-sending the full buffer therefore appends a second copy of every already-retained turn to the same document. #41911 fixes exactly this in on_session_switch; this hook has the same shape, so it inherits the same defect at session end.

I measured it on a local build carrying this patch — driving turns through sync_turn, then firing on_session_end, counting what reaches aretain_batch:

23 turns, retain_every_n_turns=5

                       shipped  unique  dupes
as written                  43      23     20
with delta slice            23      23      0

Turns 1–20 stored twice. With the default retain_every_n_turns=1 every turn has already been appended by the time the session ends, so it's a full duplicate of the session on each exit.

The fix mirrors #41911 — resolve the retain target first, then slice:

old_document_id, old_update_mode = self._resolve_retain_target(self._document_id)
if old_update_mode == "append":
    old_turns = self._session_turns[self._last_retained_turn_count:]
else:
    old_turns = list(self._session_turns)

Two things that matter alongside it:

• Skip the enqueue entirely when the delta is empty, otherwise an empty payload gets shipped whenever the watermark has caught up to the buffer — which is the common case at retain_every_n_turns=1.
• Keep message_count derived from the trimmed old_turns, not the full buffer, or the metadata overstates what the retain actually carried.

I verified the legacy/overwrite path must not be sliced — there each retain replaces the document, so the flush has to carry the whole session (23 turns → 73 shipped, 23 unique, all present). The if old_update_mode == "append" branch is load-bearing.

Since both hooks want byte-identical logic, it may be worth factoring the slice into a small shared helper rather than duplicating it, depending on which of these lands first.

Not a criticism of the hook itself — the data-loss problem it solves is real, unflushed turns between the last boundary and session end genuinely do vanish today. Just worth landing it with the slice so it doesn't trade a loss bug for a duplication bug.