fix(memory): flush Hindsight's buffered turns when a session ends

[AP3X-Dev]

What does this PR do?

Stops the Hindsight memory provider silently dropping buffered turns when a session ends.

The causal chain. With retain_every_n_turns > 1, sync_turn() accumulates turns in _session_turns and only dispatches a retain at every Nth-turn boundary. Anything buffered past that boundary exists nowhere but RAM. Two gaps meant it was never drained:

1. No on_session_end. MemoryProvider.on_session_end (agent/memory_provider.py:251) is the base-class hook for exactly this, and the other bundled providers implement it (holographic, honcho, openviking, supermemory). Hindsight — the only one that batches — did not. Its sole flush point was on_session_switch, which fires only when session_id rotates. Sessions that end without rotating (desktop session.close, the WS orphan reaper, gateway session expiry) never reached it.
2. shutdown() never flushed. It set _shutting_down, then drained the writer queue — but the buffer was never enqueued, so only already-dispatched retains survived. The gateway and dashboard are killed by SIGTERM and do not run atexit, so every restart dropped the buffer.

The fix extracts the flush on_session_switch already performed into _flush_buffered_turns() and calls it from a new on_session_end() and from shutdown().

Why the helper clears the buffer. MemoryManager.commit_session_boundary_async (agent/memory_manager.py:927) deliberately delivers on_session_end strictly before on_session_switch, and its docstring names "double-ingest of the old turn buffer" as the hazard it is guarding against. Clearing on flush is what makes the hook safe to call from several paths: the switch then finds an empty buffer and no-ops.

Ordering in shutdown() is load-bearing. The helper refuses to enqueue once _shutting_down is set (the writer is draining/gone), so the flush runs before that flag goes up. Reversed, it would silently discard the very buffer it exists to save.

Why the new call sites use a watermark and the old one doesn't. sync_turn skips a retain when the turn count hasn't reached a boundary, so on_session_end/shutdown must not re-ship turns already persisted. The guard mirrors sync_turn's own gate (_turn_counter % retain_every_n_turns) rather than _last_retained_turn_count, because that watermark is advanced only on append-capable APIs (sync_turn, guarded by if update_mode == "append") — on legacy/overwrite it stays at 0 forever and cannot answer the question. On append the watermark is checked as well, since it is the more precise signal there.

on_session_switch passes respect_watermark=False and so keeps its previous whole-buffer behavior byte-for-byte. Tightening that path is #41911, which is already open — this PR deliberately does not touch it.

Related Issue

Fixes #88944

Scope note: the linked issue reports three defects. This PR fixes defect #3 (the provider-level flush) only, which is self-contained and testable. Defects #1 and #2 are process-lifecycle changes in gateway/run.py and the serve teardown, and they touch a file with several other open PRs — they belong in their own change.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• plugins/memory/hindsight/__init__.py
  • Extracted the inline flush in on_session_switch into _flush_buffered_turns(reason, respect_watermark), which now also clears the buffer.
  • Added on_session_end() implementing the base-class hook.
  • shutdown() flushes before setting _shutting_down.
• tests/plugins/memory/test_hindsight_provider.py — added TestSessionEndBufferFlush (4 tests).

How to Test

pytest tests/plugins/memory/test_hindsight_provider.py::TestSessionEndBufferFlush -q

4 passed on this branch. With plugins/memory/hindsight/__init__.py reverted to main, two fail:

FAILED ...::test_on_session_end_flushes_buffered_turns
FAILED ...::test_shutdown_flushes_buffered_turns
2 failed, 2 passed

• test_on_session_end_flushes_buffered_turns — buffers 2 turns with retain_every_n_turns=3, calls on_session_end, asserts both turns land under the original document_id. On main no retain is dispatched at all.
• test_shutdown_flushes_buffered_turns — one buffered turn with retain_every_n_turns=5, then shutdown(). On main the turn dies with the process.
• test_session_end_then_switch_does_not_double_ingest — asserts exactly one aretain_batch across the manager's end → switch ordering.
• test_no_flush_when_last_turn_hit_a_retain_boundary — turn count landing exactly on the boundary must not flush again.

The last two pass both with and without the fix by design — they are regression guards for the duplicate-ingest failure mode, not proof of the fix. I'm calling that out rather than implying all four bite.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — left unchecked deliberately. I have not run the whole suite green on this Windows box; it carries pre-existing failures unrelated to this change. What I did run:
  • tests/plugins/memory/ → 343 passed, 7 failed, 7 skipped. The identical 7 failures occur on clean main with my changes stashed (339 passed, 7 failed — the +4 is this PR's new tests). Same failure set, no regressions.
  • tests/agent/test_memory_session_switch.py → 4 passed
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Windows 11 (build 26100), Python 3.12.10

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — the new helper and hook carry the ordering and watermark rationale inline; no user-facing docs describe this path
• I've updated cli-config.yaml.example if I added/changed config keys — N/A, no config keys
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — pure in-process buffer/lifecycle logic, no file I/O, process, or shell surface. check-windows-footguns.py reports the same 9 pre-existing findings in these two files on main as on this branch; this change adds none.
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Behavior on main — the turns are buffered and then never persisted:

DEBUG plugins.memory.hindsight: sync_turn: buffered turn 1 (will retain at turn 3)
DEBUG plugins.memory.hindsight: sync_turn: buffered turn 2 (will retain at turn 3)
DEBUG plugins.memory.hindsight: Hindsight shutdown: stopping writer + waiting for background threads
   (no retain dispatched — turns 1 and 2 are gone)

With this change the same teardown emits:

DEBUG plugins.memory.hindsight: Hindsight flush-on-shutdown: bank=..., doc=..., mode=..., num_turns=2

[alt-glitch]

This was generated by AI during triage.

Related to #28845 and #88944. This newer implementation covers the same session-end flush gap while adding shutdown ordering and double-ingest safeguards; #28845 is stale/conflicting, so maintainers should consolidate on the broader patch.

[tbetcke]

I implemented locally a change that adds two keywords: "auto_retain" and "retain_at_session_end". The rationale is that I have configured the Hindsight plugin to only send session transcripts to Hindsight at the end of a session to not pollute the memory with intermediate decisions.

While not identical to your PR, my Hermes session made me aware of this PR as related. Would it be useful to add those two config options to your PR?