fix(gateway): pass session messages to shutdown_memory_provider (#15165)#15481
fix(gateway): pass session messages to shutdown_memory_provider (#15165)#15481briandevans wants to merge 1 commit into
Conversation
…Research#15165) ``_cleanup_agent_resources`` previously invoked ``agent.shutdown_memory_provider()`` with no arguments, so every memory provider's ``on_session_end`` hook received an empty list. Providers with an early-return guard on empty input (Holographic, Hindsight) never extracted facts from the conversation, and users hit "抱歉,找不到相關的對話記錄" on the first turn after any gateway restart, session reset, or idle expiry. Forward ``agent._session_messages`` — the transcript the agent itself maintains and refreshes every turn via ``_persist_session`` — so providers see the actual conversation. Falls back to the legacy no-arg call whenever the attribute is absent or not a list (test stubs built via ``object.__new__`` or ``MagicMock``) to preserve backward compatibility with existing suites. ``AIAgent.shutdown_memory_provider`` already accepts ``messages: list = None`` (run_agent.py:4126), so this is a pure caller-side fix. Paths that use ``skip_memory=True`` temporary agents (memory flush, hygiene auto-compress, ``/compress``) are no-ops inside ``shutdown_memory_provider`` because ``self._memory_manager`` is None — no behaviour change for them. Covers Part A of the bug report. Part B (adding ``on_session_end`` to the Hindsight plugin) is a separate concern that would benefit from this fix landing first. Regression test added at ``tests/gateway/test_shutdown_memory_provider_messages.py`` covering: populated messages forwarded, empty list still forwarded, attribute missing falls back, non-list (MagicMock) falls back, provider exceptions don't block ``close()``, None agent no-op, and agent without ``shutdown_memory_provider`` tolerated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes gateway session teardown so memory providers receive the actual conversation transcript on shutdown (addressing #15165 Part A), enabling on_session_end hooks to extract and persist facts instead of early-returning on [].
Changes:
- Update
GatewayRunner._cleanup_agent_resourcesto passagent._session_messagesintoshutdown_memory_provider(...)when it’s a reallist, otherwise fall back to the legacy no-arg call for stub/mock compatibility. - Add a focused regression test suite covering populated/empty transcripts, missing/non-list
_session_messages, exception swallowing, and cleanup tolerance forNone/partial agents.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
gateway/run.py |
Forwards the agent’s session transcript into shutdown_memory_provider during cleanup to unblock provider on_session_end extraction. |
tests/gateway/test_shutdown_memory_provider_messages.py |
Adds regression coverage for transcript forwarding and backward-compatible fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Merged via #16571 — your commit was cherry-picked onto current main with authorship preserved (rebase-merge, commit 500774e). We added a follow-up commit fixing the same bug in cli.py's exit cleanup path (was reading the non-existent |
Summary
_cleanup_agent_resourcespreviously invokedagent.shutdown_memory_provider()with no arguments, so every memory provider'son_session_endhook received an empty list. Providers with an early-return guard on empty input (Holographic, Hindsight, etc.) never extracted facts from the conversation.agent._session_messages— the transcriptAIAgentmaintains and refreshes every turn via_persist_session— so providers see the actual conversation instead of[]._session_messagesis absent or not alist(test stubs built viaobject.__new__orMagicMock) to keep every existing suite green.The bug
Per #15165:
Users saw "抱歉,找不到相關的對話記錄" (roughly: "sorry, cannot find the corresponding conversation record") on the first turn after any gateway restart / idle expiry / session reset because no facts had ever been persisted.
The fix
AIAgent.shutdown_memory_provideralready acceptsmessages: list = None(run_agent.py:4126), so this is a pure caller-side change ingateway/run.py:_session_messagesis set onAIAgent.__init__(run_agent.py:1518) and refreshed at the end of everyrun_conversationturn (_persist_session, line 3264). By the time_cleanup_agent_resourcesfires, it holds the real transcript.isinstance(..., list)discrimination is deliberate — it protects againstMagicMockagents (whose attribute access auto-synthesises a child mock, notNone) falling through and passing a bogus object to the provider'sList[Dict]hook.try/except Exception: passwrap is inherited; providers that raise never preventclose()from running.skip_memory=Truetemporary agents (pre-reset memory flush, session hygiene auto-compress,/compress) are no-ops insideshutdown_memory_providerbecauseself._memory_managerisNone— so this change has no behaviour effect on them.Scope
Part A only. Part B of #15165 (adding
on_session_endto the Hindsight plugin) is a separate concern that benefits from this fix landing first — without Part A, a Hindsighton_session_endhook would still receive[].Test plan
tests/gateway/test_shutdown_memory_provider_messages.py— 7 cases:_session_messageslist is forwarded byte-for-byte_session_messagesfalls back to no-arg call (stub compatibility)MagicMockagent (non-list attribute) falls back to no-arg callclose()still runs afterwardNoneagent is a no-op (idle-sweep race tolerance)shutdown_memory_providermethod still getsclose()test_populated_messages_forwardedfails withExpected: mock([...]) / Actual: mock(); restored the fix → all 7 pass.tests/gateway/suite:3703 passed, 9 pre-existing failures(dingtalk, matrix with missingmautrix, one approve-deny test — all fail identically onmain, unrelated to this change).test_agent_cache.py,test_compress_command.py,test_background_command.py,test_flush_memory_stale_guard.py,test_session_boundary_hooks.py,test_compress_plugin_engine.py,test_clean_shutdown_marker.py.Related
/new//resetmemory commit, different code pathon_session_finalizenot fired on idle timeout (orthogonal)🤖 Generated with Claude Code