Skip to content

fix(gateway): pass session messages to shutdown_memory_provider (#15165)#15481

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/gateway-shutdown-pass-session-messages
Closed

fix(gateway): pass session messages to shutdown_memory_provider (#15165)#15481
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/gateway-shutdown-pass-session-messages

Conversation

@briandevans

Copy link
Copy Markdown
Contributor

Summary

  • Fixes [Bug]: Gateway restart drops session memory — shutdown_memory_provider receives empty messages #15165 Part A_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, etc.) never extracted facts from the conversation.
  • Forward agent._session_messages — the transcript AIAgent maintains and refreshes every turn via _persist_session — so providers see the actual conversation instead of [].
  • Falls back to the legacy no-arg call whenever _session_messages is absent or not a list (test stubs built via object.__new__ or MagicMock) to keep every existing suite green.

The bug

Per #15165:

...any provider that tries to extract facts from the session's conversation gets an empty list. Providers like Holographic (on_session_end([])) have an early return guard... Every gateway restart wipes the session's conversational memory...

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_provider already accepts messages: list = None (run_agent.py:4126), so this is a pure caller-side change in gateway/run.py:

session_messages = getattr(agent, "_session_messages", None)
if isinstance(session_messages, list):
    agent.shutdown_memory_provider(session_messages)
else:
    agent.shutdown_memory_provider()
  • _session_messages is set on AIAgent.__init__ (run_agent.py:1518) and refreshed at the end of every run_conversation turn (_persist_session, line 3264). By the time _cleanup_agent_resources fires, it holds the real transcript.
  • isinstance(..., list) discrimination is deliberate — it protects against MagicMock agents (whose attribute access auto-synthesises a child mock, not None) falling through and passing a bogus object to the provider's List[Dict] hook.
  • The try/except Exception: pass wrap is inherited; providers that raise never prevent close() from running.
  • Paths using skip_memory=True temporary agents (pre-reset memory flush, session hygiene auto-compress, /compress) are no-ops inside shutdown_memory_provider because self._memory_manager is None — so this change has no behaviour effect on them.

Scope

Part A only. Part B of #15165 (adding on_session_end to the Hindsight plugin) is a separate concern that benefits from this fix landing first — without Part A, a Hindsight on_session_end hook would still receive [].

Test plan

  • New regression suite: tests/gateway/test_shutdown_memory_provider_messages.py — 7 cases:
    • Populated _session_messages list is forwarded byte-for-byte
    • Empty list is still explicitly forwarded (matches pre-fix observable behaviour)
    • Agent without _session_messages falls back to no-arg call (stub compatibility)
    • MagicMock agent (non-list attribute) falls back to no-arg call
    • Provider exception is swallowed — close() still runs afterward
    • None agent is a no-op (idle-sweep race tolerance)
    • Agent without shutdown_memory_provider method still gets close()
  • Regression guard verified: reverted the fix → test_populated_messages_forwarded fails with Expected: mock([...]) / Actual: mock(); restored the fix → all 7 pass.
  • Full tests/gateway/ suite: 3703 passed, 9 pre-existing failures (dingtalk, matrix with missing mautrix, one approve-deny test — all fail identically on main, unrelated to this change).
  • Related suites pass clean (100/100): 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

🤖 Generated with Claude Code

…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>
Copilot AI review requested due to automatic review settings April 25, 2026 02:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_resources to pass agent._session_messages into shutdown_memory_provider(...) when it’s a real list, 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 for None/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.

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery tool/memory Memory tool and memory providers labels Apr 25, 2026
@teknium1

Copy link
Copy Markdown
Contributor

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 conversation_history attr, now reads _session_messages like the gateway path). Thanks for the clean fix and the thorough test suite!

@teknium1 teknium1 closed this Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Gateway restart drops session memory — shutdown_memory_provider receives empty messages

4 participants