Skip to content

fix(gateway): trigger memory provider shutdown on /new and /reset - #7762

Closed
fancydirty wants to merge 1 commit into
NousResearch:mainfrom
fancydirty:fix/reset-shutdown-memory-provider
Closed

fix(gateway): trigger memory provider shutdown on /new and /reset#7762
fancydirty wants to merge 1 commit into
NousResearch:mainfrom
fancydirty:fix/reset-shutdown-memory-provider

Conversation

@fancydirty

Copy link
Copy Markdown
Contributor

Fixes #7759

Problem

The /new and /reset commands were not calling shutdown_memory_provider() on the cached agent before eviction. This caused OpenViking (and any memory provider that relies on session-end shutdown) to skip commit, leaving memories un-indexed until idle timeout or gateway shutdown.

Root Cause

In _handle_reset_command(), the code flushed memories and closed tool resources, but it never shut down the memory provider. The session expiry watcher already does this correctly.

Fix

Add the missing shutdown_memory_provider() call in _handle_reset_command(), matching the behavior already present in the session expiry watcher (lines ~1600–1625).

Verification

  • tests/gateway/test_session_reset_notify.py — 13 passed ✅
  • tests/gateway/test_gateway_inactivity_timeout.py — 8 passed ✅
  • tests/gateway/test_voice_command.py — 168 passed ✅

The /new and /reset commands were not calling shutdown_memory_provider()
on the cached agent before eviction. This caused OpenViking (and any
memory provider that relies on session-end shutdown) to skip commit,
leaving memories un-indexed until idle timeout or gateway shutdown.

Add the missing shutdown_memory_provider() call in _handle_reset_command(),
matching the behavior already present in the session expiry watcher.

Fixes NousResearch#7759
Copilot AI review requested due to automatic review settings April 11, 2026 14:26

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

This PR fixes a session-lifecycle gap in the gateway: /new and /reset now trigger the cached agent’s shutdown_memory_provider() before eviction, matching the existing session-expiry behavior so memory providers (e.g., OpenViking) can commit/index on session end.

Changes:

  • Invoke shutdown_memory_provider() on the cached agent during _handle_reset_command() before closing and evicting it.
  • Keep the reset flow non-blocking by leaving the transcript-based flush as a background task while still triggering provider shutdown immediately.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fancydirty

Copy link
Copy Markdown
Contributor Author

Hi maintainers — this is a small, targeted fix for a session-lifecycle gap. It adds the missing shutdown_memory_provider() call during /new and /reset so that memory providers like OpenViking can commit/index on session end.

  • Branch is up-to-date with main (no conflicts)
  • Verified locally:
    • tests/gateway/test_session_reset_notify.py — 13 passed
    • tests/gateway/test_gateway_inactivity_timeout.py — 8 passed
    • tests/gateway/test_voice_command.py — 168 passed

Would love a review when you have a moment. Thanks!

@ZaynJarvis

Copy link
Copy Markdown
Contributor

agree, these issues should be fixed.

@ZaynJarvis ZaynJarvis 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.

Review: Gateway /new Memory Commit Fix

Verdict: LGTM — ready to merge

Surgical 5-line fix.

In the gateway, /new /reset destroys the old agent entirely and creates a fresh one for the next session, so destroy should be save.

Checklist Pass

  • shutdown_memory_provider() called before close() in _handle_reset_command()
  • Wrapped in its own try/except Exception block — isolated from the close() error path
  • hasattr guard — non-OV agents are completely unaffected
  • Confirmed MERGEABLE by GitHub, 189 gateway tests pass

This is the fastest path to fixing commit timing for gateway /new and /reset. Recommend merging before the larger PRs (#9167, #8362) to reduce diff scope.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #9796. Your commit was cherry-picked onto current main with your authorship preserved in git log. Thanks for catching this gap, @fancydirty!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/new and /reset commands don't trigger OpenViking session commit

4 participants