Skip to content

fix(gateway): tear down cached agents on hard stop - #10291

Closed
alpha60-a7x wants to merge 31 commits into
NousResearch:mainfrom
alpha60-a7x:wip/1b05d536
Closed

fix(gateway): tear down cached agents on hard stop#10291
alpha60-a7x wants to merge 31 commits into
NousResearch:mainfrom
alpha60-a7x:wip/1b05d536

Conversation

@alpha60-a7x

Copy link
Copy Markdown

Summary

  • tear down cached session agents when /stop hard-stops a running turn
  • tear down cached session agents when stale running-agent locks are evicted
  • add regression tests covering cached-agent teardown on /stop and stale-lock eviction

Verification

  • pytest -q tests/gateway/test_session_race_guard.py tests/gateway/test_gateway_shutdown.py tests/gateway/test_restart_drain.py
  • pytest -q tests/gateway/test_command_bypass_active_session.py tests/tools/test_zombie_process_cleanup.py tests/tools/test_delegate.py

Root cause

The gateway unlock path dropped _running_agents entries but left the cached AIAgent alive. That retained per-session tool resources across turns in the long-lived gateway process, matching the repeated OOM/stuck-drain behavior.

alpha60-a7x and others added 30 commits April 15, 2026 19:04
Re-add 'qqbot' and 'automation-templates' doc indexes to sidebars.ts
that were accidentally dropped in NousResearch#9931.
…gation (NousResearch#10462)

Follow-up to NousResearch#10459 (salvage of NousResearch#7527). The copy_context() fix propagates
ALL ContextVars into the cron worker thread, including credential_files.
This test verifies that skill-declared required_credential_files are
visible inside the worker thread, matching the existing env_passthrough
regression test.
…rch#9537)

- Populate watcher_* routing fields for watch-only processes (not just
  notify_on_complete), so watch-pattern events carry direct metadata
  instead of relying solely on session_key parsing fallback
- Extract _parse_session_key() helper to dedupe session key parsing
  at two call sites in gateway/run.py
- Add negative test proving cross-thread leakage doesn't happen
- Add edge-case tests for _build_process_event_source returning None
  (empty evt, invalid platform, short session_key)
- Add unit tests for _parse_session_key helper
…rence

_parse_session_key() now extracts the optional 6th part (thread_id) from
session keys, and _notify_active_sessions_of_shutdown uses _parsed.get()
instead of the removed 'parts' variable. Without this, shutdown notifications
silently failed (NameError caught by try/except) and forum topic routing
was lost.
…ousResearch#10457)

Gold #FFD700 has 1.4:1 contrast ratio on white — barely visible.
Replace with dark amber palette (#8B6508 primary, #7A5800 links)
that passes WCAG AA (5.3:1 and 6.5:1 respectively).

Changes:
- :root primary palette → dark amber tones for light mode
- Explicit light mode link colors (#7A5800 / #5A4100 hover)
- Light mode sidebar active state with amber accent
- Light mode table header/border styling
- Footer hover color split by theme (gold for dark, amber for light)

Dark mode is completely unchanged.

Reported by @AbrahamMat7632
- Change default OPENVIKING_ACCOUNT from 'root' to 'default'
- Add account and user config options to get_config_schema()
- Add session creation in initialize()
- Add reset_session() method
- Update docstring to reflect new default

This is a breaking change: existing users who relied on the 'root' account will need to either:
1. Set OPENVIKING_ACCOUNT=root in their environment, or
2. Migrate their data to the 'default' account

Future release will add support for OPENVIKING_ACCOUNT and OPENVIKING_USER in setup when API key is provided.

update desc for key setup
- Fix copy-paste bug: `self._agent = user` → `self._agent = agent`
  with new `agent` parameter in `_VikingClient.__init__`
- Read account/user/agent env vars in `initialize()` and pass them
  to all 4 `_VikingClient` instantiations so identity headers are
  consistently applied across health check, prefetch, sync, and
  memory write paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The OpenViking memory provider extracts memories when its session is
committed (POST /api/v1/sessions/{id}/commit).  Before this fix, the
CLI had two code paths that changed the active session_id without ever
committing the outgoing OpenViking session:

1. /new (new_session() in cli.py) — called flush_memories() to write
   MEMORY.md, then immediately discarded the old session_id.  The
   accumulated OpenViking session was never committed, so all context
   from that session was lost before extraction could run.

2. /compress and auto-compress (_compress_context() in run_agent.py) —
   split the SQLite session (new session_id) but left the OpenViking
   provider pointing at the old session_id with no commit, meaning all
   messages synced to OpenViking were silently orphaned.

The gateway already handles session commit on /new and /reset via
shutdown_memory_provider() on the cached agent; the CLI path did not.

Fix: introduce a lightweight session-transition lifecycle alongside
the existing full shutdown path:

- OpenVikingMemoryProvider.reset_session(new_session_id): waits for
  in-flight background threads, resets per-session counters, and
  creates the new OV session via POST /api/v1/sessions — without
  tearing down the HTTP client (avoids connection overhead on /new).

- MemoryManager.restart_session(new_session_id): calls reset_session()
  on providers that implement it; falls back to initialize() for
  providers that do not.  Skips the builtin provider (no per-session
  state).

- AIAgent.commit_memory_session(messages): wraps
  memory_manager.on_session_end() without shutdown — commits OV session
  for extraction but leaves the provider alive for the next session.

- AIAgent.reinitialize_memory_session(new_session_id): wraps
  memory_manager.restart_session() — transitions all external providers
  to the new session after session_id has been assigned.

Call sites:
- cli.py new_session(): commit BEFORE session_id changes, reinitialize
  AFTER — ensuring OV extraction runs on the correct session and the
  new session is immediately ready for the next turn.
- run_agent._compress_context(): same pattern, inside the
  if self._session_db: block where the session_id split happens.

/compress and auto-compress are functionally identical at this layer:
both call _compress_context(), so both are fixed by the same change.

Tests added to tests/agent/test_memory_provider.py:
- TestMemoryManagerRestartSession: reset_session() routing, builtin
  skip, initialize() fallback, failure tolerance, empty-manager noop.
- TestOpenVikingResetSession: session_id update, per-session state
  clear, POST /api/v1/sessions call, API failure tolerance, no-client
  noop.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace hasattr-forked OpenViking-specific paths with a proper base-class
hook. Collapse the two agent wrappers into a single rotate_memory_session
so callers don't orchestrate commit + rebind themselves.

- MemoryProvider: add on_session_reset(new_session_id) as a default no-op
- MemoryManager: on_session_reset fans out unconditionally (no hasattr,
  no builtin skip — base no-op covers it)
- OpenViking: rename reset_session -> on_session_reset; drop the explicit
  POST /api/v1/sessions (OV auto-creates on first message) and the two
  debug raise_for_status wrappers
- AIAgent: collapse commit_memory_session + reinitialize_memory_session
  into rotate_memory_session(new_sid, messages)
- cli.py / run_agent.py: replace hasattr blocks and the split calls with
  a single unconditional rotate_memory_session call; compression path
  now passes the real messages list instead of []
- tests: align with on_session_reset, assert reset does NOT POST /sessions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
OV transparently handles message history across /new and /compress: old
messages stay in the same session and extraction is idempotent, so there's
no need to rebind providers to a new session_id. The only thing the
session boundary actually needs is to trigger extraction.

- MemoryProvider / MemoryManager: remove on_session_reset hook
- OpenViking: remove on_session_reset override (nothing to do)
- AIAgent: replace rotate_memory_session with commit_memory_session
  (just calls on_session_end, no rebind)
- cli.py / run_agent.py: single commit_memory_session call at the
  session boundary before session_id rotates
- tests: replace on_session_reset coverage with routing tests for
  MemoryManager.on_session_end

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nning gateways (NousResearch#10200) (NousResearch#10470)

The _client_cache used event loop id() as part of the cache key, so
every new worker-thread event loop created a new entry for the same
provider config.  In long-running gateways where threads are recycled
frequently, this caused unbounded cache growth — each stale entry
held an unclosed AsyncOpenAI client with its httpx connection pool,
eventually exhausting file descriptors.

Fix: remove loop_id from the cache key and instead validate on each
async cache hit that the cached loop is the current, open loop.  If
the loop changed or was closed, the stale entry is replaced in-place
rather than creating an additional entry.  This bounds cache growth
to at most one entry per unique provider config.

Also adds a _CLIENT_CACHE_MAX_SIZE (64) safety belt with FIFO
eviction as defense-in-depth against any remaining unbounded growth.

Cross-loop safety is preserved: different event loops still get
different client instances (validated by existing test suite).

Closes NousResearch#10200
…substantive tool calls (NousResearch#10472)

The _last_content_with_tools fallback was firing indiscriminately for ALL
content+tool turns, including mid-task narration alongside substantive
tools (terminal, search_files, etc.).  This caused the agent to exit
the loop with 'I'll scan the directory...' as the final answer instead
of nudging the model to continue processing tool results.

The fix restricts the fallback to housekeeping-only turns (memory, todo,
skill_manage, session_search) where the content genuinely IS the final
answer.  When substantive tools are present, the existing post-tool
nudge mechanism now fires instead, prompting the model to continue.

Affected models: xiaomi/mimo-v2-pro, GLM-5, and other weaker models
that intermittently return empty after tool results.

Reported by user Renaissance on Discord.
…outs (NousResearch#10501)

Multiple gaps in activity tracking could cause the gateway's inactivity
timeout to fire while the agent is actively working:

1. Streaming wait loop had no periodic heartbeat — the outer thread only
   touched activity when the stale-stream detector fired (180-300s), and
   for local providers (Ollama) the stale timeout was infinity, meaning
   zero heartbeats. Now touches activity every 30s.

2. Concurrent tool execution never set the activity callback on worker
   threads (threading.local invisible across threads) and never set
   _current_tool. Workers now set the callback, and the concurrent wait
   uses a polling loop with 30s heartbeats.

3. Modal backend's execute() override had its own polling loop without
   any activity callback. Now matches _wait_for_process cadence (10s).
…usResearch#10174) (NousResearch#10507)

The on_memory_write bridge that notifies external memory providers
(ClawMem, retaindb, supermemory, etc.) of built-in memory writes was
only present in the concurrent tool execution path (_invoke_tool).
The sequential path (_execute_tool_calls_sequential) — which handles
all single tool calls, the common case — was missing it entirely.

This meant external memory providers silently missed every single-call
memory write, which is the vast majority of memory operations.

Fix: add the identical bridge block to the sequential path, right
after the memory_tool call returns.

Closes NousResearch#10174
…ig (NousResearch#10306, NousResearch#10216) (NousResearch#10509)

Two gateway fixes:

1. MessageDeduplicator.is_duplicate() now checks TTL at query time (NousResearch#10306)

   Previously, is_duplicate() returned True for any previously seen ID
   without checking its age — expired entries were only purged when cache
   size exceeded max_size.  On normal workloads that never overflow, message
   IDs stayed deduplicated forever instead of expiring after the TTL.

   Fix: check `now - timestamp < ttl` before returning True.  Expired
   entries are removed and treated as new messages.

2. Gateway --config flag now uses yaml.safe_load() (NousResearch#10216)

   The --config CLI flag in gateway/run.py main() used json.load() to
   parse config files.  YAML is the only documented config format and
   every other config loader uses yaml.safe_load().  A YAML config file
   passed via --config would crash with json.JSONDecodeError.

Closes NousResearch#10306
Closes NousResearch#10216
…tices (NousResearch#10510)

- Pastes uploaded by /debug now auto-delete after 1 hour via a detached
  background process that sends DELETE to paste.rs
- CLI: shows privacy notice listing what data will be uploaded
- Gateway: only uploads summary report (system info + log tails), NOT
  full log files containing conversation content
- Added 'hermes debug delete <url>' for immediate manual deletion
- 16 new tests covering auto-delete scheduling, paste deletion, privacy
  notices, and the delete subcommand

Addresses user privacy concern where /debug uploaded full conversation
logs to a public paste service with no warning or expiry.
…#10234) (NousResearch#10512)

Python's json.dumps() defaults to ensure_ascii=True, escaping non-ASCII
characters to \uXXXX sequences.  For CJK characters this inflates
token count 3-4x — a single Chinese character like '中' becomes
'\u4e2d' (6 chars vs 3 bytes, ~6 tokens vs ~1 token).

Since MCP tool results feed directly into the model's conversation
context, this silently multiplied API costs for Chinese, Japanese,
and Korean users.

Fix: add ensure_ascii=False to all 20 json.dumps calls in mcp_tool.py.
Raw UTF-8 is valid JSON per RFC 8259 and all downstream consumers
(LLM APIs, display) handle it correctly.

Closes NousResearch#10234
@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 labels Apr 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #12917 (clean up cached agents on shutdown) — overlapping root cause in _agent_cache / _running_agents lifecycle. This PR additionally covers /stop hard-stop and stale-lock eviction paths.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for this, and for the careful root-cause writeup — the leak class you're targeting is real, but it's already been addressed on current main, so I'm closing this as implemented.

What main does now for the cached-agent lifecycle:

  • /stop routes through _interrupt_and_clear_session()_release_running_agent_state(), which releases the running-agent lock, the active-session lease, and the busy-ack state for the session.
  • Cached AIAgent instances are torn down via _evict_cached_agent(), which soft-releases the LLM client pool (frees the httpx sockets/buffers promptly rather than waiting on GC) while guarding against tearing down an agent that's mid-turn, and runs the teardown on a daemon thread so it never blocks the cache lock. This is wired into the real conversation boundaries: /new, /reset, idle sweep, cache-cap enforcement, fallback-model eviction, and the compression-exhaustion auto-reset. (See _evict_cached_agent and the #29298 / #25315 leak fixes.)

The one deliberate difference from this PR: main does not hard-tear-down the cached agent on /stop. A /stop is meant to leave the session resumable, so the terminal sandbox / browser daemon are kept alive on purpose — the heavy LLM client sockets are what get freed. A hard teardown on every /stop (this PR's _force_teardown_session_agent) would force a full agent + MCP reinit on the very next message of a session the user explicitly wanted to keep, which is the same recreate→reinit loop class main works to avoid (#7130).

Note also that because the branch is well behind main, the PR diff shows ~39 files / +2196 — but almost all of that is other contributors' already-merged work that the stale base is replaying. Your own substantive commits all target behavior main has since solved independently.

Appreciate the contribution — closing as already-fixed on main.

@teknium1 teknium1 closed this Jun 29, 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants