fix: run CLI new-session memory flush off-thread by Tosko4 · Pull Request #16454 · NousResearch/hermes-agent

Tosko4 · 2026-04-27T08:33:57Z

Summary

move CLI /new old-session memory extraction onto a daemon background thread
snapshot the outgoing conversation history and old session_id before rotating/clearing the session
let AIAgent.commit_memory_session(...) finalize an explicit old session id so async callers do not race with the new session id
keep session finalization/reset behavior otherwise unchanged
add regression coverage for async dispatch and explicit old-session finalization

Why

Starting a fresh CLI session should not block on potentially slow memory extraction from the previous session. Current main still calls commit_memory_session(...) synchronously at the top of HermesCLI.new_session(), and #32234 now reports the exact user-facing hang this PR was originally meant to avoid.

This keeps /new responsive while preserving best-effort old-session memory/context-engine extraction.

Fixes #32234.

Validation

python -m pytest tests/cli/test_cli_new_session.py tests/run_agent/test_commit_memory_session_context_engine.py -q --tb=short — 15 passed
./scripts/run_tests.sh tests/cli/test_cli_new_session.py tests/run_agent/test_commit_memory_session_context_engine.py -q --tb=short — 15 passed
python -m compileall -q cli.py run_agent.py tests/cli/test_cli_new_session.py tests/run_agent/test_commit_memory_session_context_engine.py
ruff check cli.py run_agent.py tests/cli/test_cli_new_session.py tests/run_agent/test_commit_memory_session_context_engine.py — passed (existing warning only: invalid historical # noqa on run_agent.py:107)
git diff --check

Notes

CLI host behavior only
Rebased onto current main (d577408f3)
Updated from the old version: flush_memories(...) is no longer part of the live AIAgent path, so this now offloads commit_memory_session(...) directly and pins the ended session id for the context-engine hook

Tosko4 · 2026-04-27T08:55:56Z

Context for reviewers: this is one of the small follow-up slices from the closed broad host-support PR #13370.

Teknium salvaged the compression-boundary signal into #16306, and the remaining host-side pieces from #13370 are being resubmitted as narrow, independent PRs instead of reviving the original large PR.

This slice is intentionally limited to its described scope; it does not vendor or bundle any external context-engine implementation.

Tosko4 · 2026-07-05T09:18:54Z

Re-reviewed this against latest main and I think it is still relevant, not just old backlog.

Why:

current HermesCLI.new_session() still calls commit_memory_session(...) synchronously before rotating the session
/new hangs the TUI/CLI on long sessions — synchronous memory extraction has no timeout #32234 now reports the same user-facing /new hang this PR was aimed at
the timeout-only follow-ups (fix(cli): add 15s timeout for memory extraction in /new #32293 / fix: add timeout protection to /new and /clear commands (Issue #32383) #32657) mask the wait, but this PR keeps the old-session extraction path while moving it off the CLI reset path

I rebased the PR onto current main (d577408f3) and updated the implementation for the newer memory/session-boundary code:

async worker now offloads commit_memory_session(...) directly
outgoing history is snapshotted before reset
old session_id is captured and passed through so the context-engine on_session_end hook finalizes the ended session, not the newly-created one
added regression coverage for both the async CLI dispatch and explicit old-session finalization

Validation:

targeted local tests: 15 passed
./scripts/run_tests.sh tests/cli/test_cli_new_session.py tests/run_agent/test_commit_memory_session_context_engine.py -q --tb=short: 15 passed
compileall / ruff / git diff --check: passed
CI is green on the rebased head

@Teknium can you take another look when you have a chance?

Tosko4 · 2026-07-05T09:40:19Z

Small ping with the correct handle: @teknium1 can you take another look when you have a chance?

I re-reviewed this against current main; it is still relevant because CLI /new still synchronously calls commit_memory_session(...), and #32234 now tracks the same user-facing hang. The PR has been rebased, updated for the current session-boundary/memory code, and CI is green on the rebased head.

kshitijk4poor · 2026-07-08T21:48:09Z

Thanks @Tosko4 — this fix is landing via #61139, which cherry-picks your commit with authorship preserved and builds on it.

Your diagnosis was exactly right: /new blocked for seconds on the synchronous end-of-session memory extraction (an auxiliary-LLM call). During review we found that a bare background thread races new_session()'s inline on_session_switch(reset=True) — providers key off internal _session_id state, so a late on_session_end could misattribute the old transcript to the new session. The salvage keeps your off-the-command-path design and routes both hooks as one serialized end→switch task on the memory manager's existing background worker, which preserves the ordering providers depend on while keeping /new non-blocking.

Closing this in favor of #61139 (auto-merge armed, rebase — your commit lands under your name). Appreciate the contribution!

Deep review of the cherry-picked #16454 found the ad-hoc flush thread raced new_session()'s inline on_session_switch(reset=True): memory providers key off internal _session_id state (MemoryManager.on_session_end takes no session id), so a late off-thread extraction ran against post-rotation bindings — misattributing the old transcript to the new session id, double-ingesting the old turn buffer (supermemory), or double-committing (openviking already async-finalizes in on_session_switch). Redesign: new MemoryManager.commit_session_boundary_async queues on_session_end + on_session_switch as ONE task on the manager's existing single-worker background executor (the same worker sync_all already uses). This preserves the strict end→switch ordering providers depend on, serializes against per-turn syncs FIFO, keeps /new non-blocking, and degrades to inline (pre-#16454 behavior) when the executor is unavailable. No ad-hoc threads; no per-provider changes needed. The context-engine on_session_end half stays synchronous in _launch_session_boundary_memory_flush (cheap, must land before reset_session_state rebinds the engine). Exit durability: _run_cleanup calls the manager's existing flush_pending(timeout=10) barrier before shutdown, so '/new then quit' doesn't drop the queued extraction (shutdown_all's own drain is ~5s and cancels queued tasks). Bounded well inside the 30s exit watchdog. Tests: ordering invariant with slow (LLM-like) extraction, FIFO serialization vs sync_all, switch-fires-even-if-end-raises, no-provider no-op, CLI snapshot handoff + inline-switch fallback, sync engine boundary, cleanup flush_pending.

Deep review of the cherry-picked NousResearch#16454 found the ad-hoc flush thread raced new_session()'s inline on_session_switch(reset=True): memory providers key off internal _session_id state (MemoryManager.on_session_end takes no session id), so a late off-thread extraction ran against post-rotation bindings — misattributing the old transcript to the new session id, double-ingesting the old turn buffer (supermemory), or double-committing (openviking already async-finalizes in on_session_switch). Redesign: new MemoryManager.commit_session_boundary_async queues on_session_end + on_session_switch as ONE task on the manager's existing single-worker background executor (the same worker sync_all already uses). This preserves the strict end→switch ordering providers depend on, serializes against per-turn syncs FIFO, keeps /new non-blocking, and degrades to inline (pre-NousResearch#16454 behavior) when the executor is unavailable. No ad-hoc threads; no per-provider changes needed. The context-engine on_session_end half stays synchronous in _launch_session_boundary_memory_flush (cheap, must land before reset_session_state rebinds the engine). Exit durability: _run_cleanup calls the manager's existing flush_pending(timeout=10) barrier before shutdown, so '/new then quit' doesn't drop the queued extraction (shutdown_all's own drain is ~5s and cancels queued tasks). Bounded well inside the 30s exit watchdog. Tests: ordering invariant with slow (LLM-like) extraction, FIFO serialization vs sync_all, switch-fires-even-if-end-raises, no-provider no-op, CLI snapshot handoff + inline-switch fallback, sync engine boundary, cleanup flush_pending.

…search#31597) Redesign of the original patch against the current boundary lifecycle, per the hermes-sweeper review on PR NousResearch#31609: - compression: snapshot is captured before compress(), but the review is spawned only after the abort / no-op guards — an aborted compression retains the transcript and never pays the review cost - reset now covers all three surfaces: CLI HermesCLI.new_session, TUI _reset_session_agent, gateway _handle_reset_command - session_end covers CLI exit (bounded join), TUI _finalize_session, and gateway session expiry - CLI /new: the provider end→switch ordering contract (NousResearch#16454) is untouched — the review runs on its own fork with its own snapshot copy, gated centrally in maybe_spawn_boundary_review()

@url

…kes) (#438) Three wall-clock flakes hit in a single day. They share one defect: the test proves a CONCURRENCY or NON-BLOCKING property by measuring elapsed real time, which makes the OS scheduler part of the assertion. Under load the inequality flips with nothing wrong in the code under test. The worst one blocked the merge queue: tests/gateway/test_session_hygiene.py failed `assert elapsed < 2.0` with 2.005265276999978 -- five milliseconds -- while sitting at the head of a queue for a PR touching zero gateway files. Profiling that test shows the bound was not even measuring the behavior it protects: ~1.5s of the 2.3s window is models_dev.fetch_models_dev -> _save_disk_cache -> atomic_json_write (~600k json-encoder calls), i.e. one-time provider-metadata cache work. Instrumented runs on an IDLE box measured 1.40s, 1.87s and 2.39s -- the 2.0 threshold sits inside the natural distribution, so the failure was not an outlier. This applies the pattern from #426 (barrier, not stopwatch) to the rest of the family: - tests/gateway/test_session_hygiene.py -- ordering witness: an Event set in a finally on every worker exit path, asserted UNSET when the handler returns. - tests/agent/test_context_refs_concurrent.py -- asyncio.Barrier(3): all three @url: fetches must be in flight at once before any may return. - tests/tools/test_mcp_tool.py -- threading.Barrier(3) for parallel shutdown. - tests/plugins/memory/test_mem0_rerank_guard.py -- alert_returned witness replaces `elapsed < 0.1` across a 16-thread pool. - tests/agent/test_memory_boundary_commit.py -- assert the provider recorded NOTHING yet, proving /new was not gated on the slow extraction. Every conversion keeps a finite barrier/wait timeout (10-30s) so a genuine regression fails fast instead of hanging, but that is orders of magnitude above real rendezvous latency, so it is not itself a timing assertion. RED-PROVEN: each was verified by breaking the behavior it protects and confirming the new assertion fails by name -- removing the wait_for timeout wiring, replacing asyncio.gather with a serial loop (x2), making observe() join the alert thread, and running the boundary commit inline (the pre-NousResearch#16454 blocking bug). Two of the five first drafts could NOT fail and were caught and strengthened by that exercise. All source mutations reverted; this diff is test-only. Side effect: the barrier forms delete fixed sleeps, so test_mcp_tool shutdown drops ~1s -> ~0.1s and the mem0 rerank test 7.0s -> 1.7s. Deliberately NOT changed: hang-guards. `assert ev.wait(timeout=5)` asserts the EVENT, not the duration; lower bounds like `assert elapsed >= 0.04` prove an injected wait happened and get MORE reliable under load; and ceilings an order of magnitude above the hang they guard fail only on real regressions. The rule applied: convert when load can cause failure, leave alone when only a real regression can. Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>

Deep review of the cherry-picked NousResearch#16454 found the ad-hoc flush thread raced new_session()'s inline on_session_switch(reset=True): memory providers key off internal _session_id state (MemoryManager.on_session_end takes no session id), so a late off-thread extraction ran against post-rotation bindings — misattributing the old transcript to the new session id, double-ingesting the old turn buffer (supermemory), or double-committing (openviking already async-finalizes in on_session_switch). Redesign: new MemoryManager.commit_session_boundary_async queues on_session_end + on_session_switch as ONE task on the manager's existing single-worker background executor (the same worker sync_all already uses). This preserves the strict end→switch ordering providers depend on, serializes against per-turn syncs FIFO, keeps /new non-blocking, and degrades to inline (pre-NousResearch#16454 behavior) when the executor is unavailable. No ad-hoc threads; no per-provider changes needed. The context-engine on_session_end half stays synchronous in _launch_session_boundary_memory_flush (cheap, must land before reset_session_state rebinds the engine). Exit durability: _run_cleanup calls the manager's existing flush_pending(timeout=10) barrier before shutdown, so '/new then quit' doesn't drop the queued extraction (shutdown_all's own drain is ~5s and cancels queued tasks). Bounded well inside the 30s exit watchdog. Tests: ordering invariant with slow (LLM-like) extraction, FIFO serialization vs sync_all, switch-fires-even-if-end-raises, no-provider no-op, CLI snapshot handoff + inline-switch fallback, sync engine boundary, cleanup flush_pending.

alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard tool/memory Memory tool and memory providers labels Apr 27, 2026

Tosko4 force-pushed the fix/cli-new-session-background-memory-flush branch 4 times, most recently from 537954d to ae53e42 Compare May 1, 2026 20:20

Tosko4 force-pushed the fix/cli-new-session-background-memory-flush branch from ae53e42 to b5aaf8a Compare May 14, 2026 20:54

Tosko4 force-pushed the fix/cli-new-session-background-memory-flush branch 3 times, most recently from 21851b6 to 831cbd6 Compare July 5, 2026 09:09

Tosko4 force-pushed the fix/cli-new-session-background-memory-flush branch 2 times, most recently from 2522c57 to 7d87972 Compare July 5, 2026 09:20

fix: run CLI new-session memory flush off-thread

3e82a86

Tosko4 force-pushed the fix/cli-new-session-background-memory-flush branch from 7d87972 to 3e82a86 Compare July 5, 2026 09:30

kshitijk4poor mentioned this pull request Jul 8, 2026

fix(cli): run /new session-boundary memory extraction off the command path (#16454 salvage) #61139

Merged

kshitijk4poor closed this Jul 8, 2026

bumsoft mentioned this pull request Jul 13, 2026

feat(memory): trigger background review at session boundaries (#31597) #31609

Open

19 tasks

This was referenced Jul 26, 2026

test: replace wall-clock assertions with deterministic signals (wall-clock flake family) ANG-Ventures/hermes-agent#438

Merged

test: prove concurrency and non-blocking with barriers/witnesses, not wall-clock bounds #71913

Closed

Navigation Menu

Uh oh!

fix: run CLI new-session memory flush off-thread - #16454

fix: run CLI new-session memory flush off-thread#16454
Tosko4 wants to merge 1 commit into
NousResearch:mainfrom
Tosko4:fix/cli-new-session-background-memory-flush

Tosko4 commented Apr 27, 2026 •

edited

Loading

Uh oh!

Tosko4 commented Apr 27, 2026

Uh oh!

Tosko4 commented Jul 5, 2026 •

edited

Loading

Uh oh!

Tosko4 commented Jul 5, 2026

Uh oh!

kshitijk4poor commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Tosko4 commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Validation

Notes

Uh oh!

Tosko4 commented Apr 27, 2026

Uh oh!

Tosko4 commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tosko4 commented Jul 5, 2026

Uh oh!

kshitijk4poor commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Tosko4 commented Apr 27, 2026 •

edited

Loading

Tosko4 commented Jul 5, 2026 •

edited

Loading