The persisted (DB-fallback) branch of _resume_target_allowed() compared only
sessions.user_id against source.user_id, but build_session_key() keys the
participant on `user_id_alt or user_id` (Signal/Feishu carry the canonical
participant in user_id_alt). The sessions table has no user_id_alt column, so a
per-user row a caller shares the user_id of — but not the user_id_alt — maps to a
DIFFERENT live session key, yet the row's user_id matched both participants:
a co-member could resume/enumerate another member's persisted per-user group or
no-chat_id DM session (IDOR, CWE-639).
The live-origin guard (_same_origin_chat) already compares user_id_alt; the
persisted fallback couldn't. Fail closed on both identity-bearing per-user
branches (non-DM per-user group, no-chat_id DM) whenever the caller carries a
user_id_alt. Shared group/thread sessions (no participant scoping) and DMs keyed
on a present chat_id are unaffected; callers keyed on user_id (e.g. Telegram)
still resume their own rows; admin --all override still applies.
Regression: tests/gateway/test_resume_command.py::
test_resume_persisted_fallback_fails_closed_on_user_id_alt.
Addresses egilewski follow-up on PR #52355: the persisted-row fallback required
row_uid == caller_uid for every identity-bearing caller, which wrongly blocked a
legitimately SHARED non-DM group session. With group_sessions_per_user=False,
build_session_key resolves every participant of a chat to one session key, so a
co-member (different user_id) in the same chat shares Bob's session — but the
guard returned "/resume blocked".
Mirror is_shared_multi_user_session() in the fallback, exactly as the live-origin
branch (_same_origin_chat) already does: for a non-DM caller, first require the
same platform + chat + thread provenance (unchanged — blank/mismatching chat
still fails closed), then allow without user-id equality when the session is
shared, and keep requiring the same owner for per-user group/thread sessions.
DM scoping is unchanged (always per-user).
Adds a regression: shared group → co-member allowed; per-user group → blocked;
different chat → blocked even when shared.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses egilewski/CodeRabbit follow-up on PR #52355: the identity-bearing
persisted fallback compared row_chat == caller_chat, which SUCCEEDS when both
normalize to "" — so a legacy row with no stored chat provenance could still be
resumed by a caller that also has no chat_id (probe: a group caller with
chat_id=None resuming a NULL-chat telegram row on matching user_id).
A non-DM session (group/channel/forum/thread) is keyed by chat_id in
build_session_key, so a blank chat on either side is NOT proof of same-chat.
Require both row and caller chat_id to be non-blank and equal for non-DM
callers; a legacy NULL-chat row (or a caller missing its chat_id) now fails
closed. DMs are unchanged: they are keyed on user_id, so a no-chat_id DM row
stays resumable by the same user (and a mismatching chat_id, when present, is
still rejected).
Adds the blank-caller-chat group probe and a DM no-chat_id same-user/other-user
regression.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the egilewski/CodeRabbit and teknium1 reviews on PR #52355.
1) Persisted-row chat scope (egilewski/CodeRabbit). The sessions table stored
only source + user_id, so an identity-bearing caller could resume/list an
INACTIVE persisted row that matched source+user_id but belonged to a
DIFFERENT chat (probe: same user moves `same_user_chat_b` into chat-a).
Persist the messaging origin and compare it:
- schema: sessions gains origin_chat_id / origin_thread_id (declarative
auto-migration via the existing column reconciler).
- SessionDB._insert_session_row accepts + writes the two columns.
- the gateway records them at every origin-bearing creation: both
SessionStore create paths (get_or_create_session + reset/switch) and the
/title path that materializes a store-only session into the DB.
- _resume_target_allowed's identity branch now also requires
origin_chat_id AND origin_thread_id to match the caller. Legacy rows with
NULL origin (created before this change) cannot prove chat origin and
fail closed — resume them via a live session or an admin --all override.
The /sessions listing inherits the fix (non-Matrix rows route through the
same helper).
2) DM key-contract mirror (teknium1). _same_origin_chat's DM branch only
compared user_id and allowed when either side was missing, diverging from
build_session_key (no-chat_id DM keys are built from user_id_alt or
user_id). It now: treats an equal non-blank chat_id as sufficient (the DM
key IS the chat_id when present), and otherwise compares the effective
participant id (user_id_alt or user_id), failing closed on a
missing/different participant so two no-chat_id DM origins are never
conflated.
Tests: add same-user/different-chat (e2e + unit) and chat-scope unit cases;
add DM no-chat_id / user_id_alt / no-identity / same-chat_id cases; update
existing fixtures to record origin_chat_id like the gateway does; make the
cross-room `/resume --all` listing test run as admin (cross-room listing is
admin-gated) and give the boundary-state resume runner a live same-origin so
its post-resume clearing assertions exercise an authorized resume.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses egilewski (Codex/CodeRabbit) follow-up on PR #52355: the no-identity
branch of _resume_target_allowed() returned True after only checking that the
row's source didn't mismatch the caller platform. The sessions table has no
chat_id, so same-platform alone is not ownership proof — a Telegram group
caller in chat-a with user_id=None could resume (and /sessions could list) a
persisted row owned by another chat/user (e.g. victim_chat_b_uid,
source=telegram, user_id=victim).
Fail closed: an identity-less caller can no longer bind to or enumerate a
persisted session by id/title. A legitimate same-chat resume of an ACTIVE
session still works via the live-origin branch (which compares chat_id), and an
operator can use the admin --all override. The listing path inherits the fix
because _resume_row_visible() routes non-Matrix rows through the same helper.
Adds an end-to-end no-identity probe (resume blocked) and a unit-level
persisted-fallback assertion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses egilewski (Codex) CR on PR #52355: the Matrix direct /resume <id>
guard (and the Matrix listing guard) used _same_matrix_room(), which compared
only platform + chat_id. But build_session_key() appends thread_id for every
chat type when present, and Matrix scopes the model's turn to the current
room/thread — so a live session in another thread of the SAME room is a
DIFFERENT session. A caller in thread A could resume a target whose live origin
was in thread B (switch_session fired on the victim session).
Add a thread_id equality check to _same_matrix_room so room scoping also
enforces the thread boundary. Non-threaded rooms have empty thread_id on both
sides ("" == ""), so existing room-level sharing is preserved unchanged; only
cross-thread access is newly blocked. This mirrors the thread handling already
in _same_origin_chat for the non-Matrix adapters.
Adds regressions replaying the reviewer's thread-a -> thread-b probe (direct
guard + listing path), plus same-thread-shared and thread-vs-no-thread cases.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses egilewski (Codex) CR on PR #52355: the persisted-row fallback in
_resume_target_allowed() skipped the platform/source check when sessions.source
was blank (the row_src guard only rejects a *mismatching* non-blank source),
then accepted the row on user_id equality alone. A legacy/malformed row with a
blank source but a matching user_id was therefore resumable — an identified
caller could bind to a transcript whose origin it can't prove.
Now an identity-bearing caller is allowed only when the row proves BOTH the
same owner (non-blank user_id match) AND the same platform/origin (non-blank
source match). A blank/legacy source fails closed, exactly like a missing
user_id. No-identity (single-user) callers are unaffected.
Adds a regression replaying the reviewer's blank-source same-uid probe.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
/resume resolved a persisted session id/title with no ownership check on any
adapter except Matrix, so an authorized caller could bind their gateway session
to another user's/room's transcript and read it. The titled-session listing and
numeric index were also globally enumerable on non-Matrix platforms, exposing
the ids and previews needed to target the IDOR.
Generalize the Matrix-only room guard to an adapter-agnostic ownership check
(live origin when active; DB row source + user_id for persisted-only sessions,
the only fields available), applied to the direct-id/title path and the
listing/numeric paths on every platform. An explicit admin --all override is
honored. The Matrix path is preserved unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The topic-mode helpers (_telegram_topic_mode_enabled,
_recover_telegram_topic_thread_id, _record/_sync_telegram_topic_binding,
_is_telegram_topic_lane/_root_lobby, _normalize_source_for_session_key,
_telegram_topic_new_header, _schedule_telegram_topic_title_rename, and the
base.py _apply_topic_recovery hook) each run a synchronous SessionDB read or
write. They reach the event loop through async handlers, so a contended
state.db froze the loop the same way the handoff watcher did.
These helpers already run off-loop in the run_sync thread-pool closure, so
they are proven thread-safe there. Rather than colour them async, loop-side
callers now invoke them via asyncio.to_thread(...); the executor callers are
unchanged. Inside the helpers the SessionDB handle is unwrapped to the sync
door (getattr(db, '_db', db)) since they always run on a worker thread, and
AIAgent construction + query_session_listing are handed the sync SessionDB
directly. base.py wraps its single _apply_topic_recovery call in to_thread.
The guard is now alias-aware (catches db = getattr(self, '_session_db', None);
db.method(...)) and enforces the offload contract: the offloaded sync helpers
may never be called bare on the loop. Sibling test fixtures wrap their injected
SessionDB in AsyncSessionDB to match how the gateway holds it.
/resume is a conversation boundary, but unlike /new it did not clear the
chat-keyed _session_model_overrides / _pending_model_notes. A /model switch
made in the previous session under the same chat session_key leaked into the
resumed conversation, running it on the wrong model.
Clear both maps for the session_key after the switch (mirroring /new), scoped
to that key so other chats' overrides are untouched. The cached-agent eviction
this leak also implied already landed via #6672.
Closes#10702.
- CLI: bracketed/quoted target resolves; mismatched single bracket passes through unchanged.
- Gateway: bracketed session ID resolves; bare untitled session ID resolves via get_session() fallback.
Fixes#6672
Memory providers now receive on_session_switch() whenever AIAgent.session_id
rotates mid-process — /resume, /branch, /reset, /new, and context
compression. Before this, providers that cached per-session state in
initialize() (Hindsight's _session_id, _document_id, accumulated
_session_turns, _turn_counter) kept writing into the old session's
record after the agent had moved on.
MemoryProvider ABC
------------------
- New optional hook on_session_switch(new_session_id, *,
parent_session_id='', reset=False, **kwargs) with no-op default for
backward compat. reset=True signals /reset or /new — providers should
flush accumulated per-session buffers. reset=False for /resume,
/branch, compression where the logical conversation continues.
MemoryManager
-------------
- on_session_switch() fans the hook out to every registered provider.
Isolated try/except per provider — one bad provider can't block others.
- Empty/None new_session_id is a no-op to avoid corrupting provider state
during shutdown paths.
run_agent.py
------------
- _sync_external_memory_for_turn now passes session_id=self.session_id
into sync_all() and queue_prefetch_all(). Providers with defensive
session_id updates in sync_turn (Hindsight already had this at
plugins/memory/hindsight/__init__.py:1199) now actually receive the
current id.
- Compression block at ~L8884 already notified the context engine of
the rollover; now also calls
_memory_manager.on_session_switch(reason='compression').
cli.py
------
- new_session() fires reset=True, reason='new_session' so providers
flush buffers.
- _handle_resume_command fires reset=False, reason='resume' with the
previous session as parent_session_id.
- _handle_branch_command fires reset=False, reason='branch' with the
parent session_id already captured for the DB parent link.
gateway/run.py
--------------
- _handle_resume_command now evicts the cached AIAgent, mirroring
/branch and /reset. The next message rebuilds a fresh agent whose
memory provider initialize() runs with the correct session_id —
matches the pattern the gateway already uses for provider state
cross-session transitions.
Hindsight reference implementation
----------------------------------
- plugins/memory/hindsight/__init__.py adds on_session_switch that:
updates _session_id, mints a fresh _document_id (prevents
vectorize-io/hindsight#1303 overwrite), and clears _session_turns /
_turn_counter / _turn_index so in-flight batches don't flush under
the new document id. parent_session_id only overwritten when provided
(avoids clobbering on a bare switch).
Tests
-----
- tests/agent/test_memory_session_switch.py: new dedicated file. ABC
default no-op, manager fan-out, failure isolation, empty-id no-op,
session_id propagation through sync_all/queue_prefetch_all, Hindsight
state transitions for every reset/non-reset case, parent preservation.
- tests/cli/test_branch_command.py: new test verifying /branch fires
the hook with correct parent_session_id + reset=False + reason.
- tests/gateway/test_resume_command.py: new test verifying /resume
evicts the cached agent.
- tests/run_agent/test_memory_sync_interrupted.py: updated existing
assertions to account for the session_id kwarg on sync_all and
queue_prefetch_all.
E2E verified (real imports, tmp HERMES_HOME):
- /resume: session_id updates, doc_id fresh, buffers cleared, parent set
- /branch: session_id forks, parent links to original
- /new: reset=True clears accumulated state
- compression: reason='compression' propagated, lineage preserved
- Empty id: no-op, state preserved
- Legacy provider without on_session_switch: no crash
Reported by @nicoloboschi (Hindsight maintainer); related scope-widening
comment by @kidonng extending coverage to compression.
The AIAgent.flush_memories pre-compression save, the gateway
_flush_memories_for_session, and everything feeding them are
obsolete now that the background memory/skill review handles
persistent memory extraction.
Problems with flush_memories:
- Pre-dates the background review loop. It was the only memory-save
path when introduced; the background review now fires every 10 user
turns on CLI and gateway alike, which is far more frequent than
compression or session reset ever triggered flush.
- Blocking and synchronous. Pre-compression flush ran on the live agent
before compression, blocking the user-visible response.
- Cache-breaking. Flush built a temporary conversation prefix
(system prompt + memory-only tool list) that diverged from the live
conversation's cached prefix, invalidating prompt caching. The
gateway variant spawned a fresh AIAgent with its own clean prompt
for each finalized session — still cache-breaking, just in a
different process.
- Redundant. Background review runs in the live conversation's
session context, gets the same content, writes to the same memory
store, and doesn't break the cache. Everything flush_memories
claimed to preserve is already covered.
What this removes:
- AIAgent.flush_memories() method (~248 LOC in run_agent.py)
- Pre-compression flush call in _compress_context
- flush_memories call sites in cli.py (/new + exit)
- GatewayRunner._flush_memories_for_session + _async_flush_memories
(and the 3 call sites: session expiry watcher, /new, /resume)
- 'flush_memories' entry from DEFAULT_CONFIG auxiliary tasks,
hermes tools UI task list, auxiliary_client docstrings
- _memory_flush_min_turns config + init
- #15631's headroom-deduction math in
_check_compression_model_feasibility (headroom was only needed
because flush dragged the full main-agent system prompt along;
the compression summariser sends a single user-role prompt so
new_threshold = aux_context is safe again)
- The dedicated test files and assertions that exercised
flush-specific paths
What this renames (with read-time backcompat on sessions.json):
- SessionEntry.memory_flushed -> SessionEntry.expiry_finalized.
The session-expiry watcher still uses the flag to avoid re-running
finalize/eviction on the same expired session; the new name
reflects what it now actually gates. from_dict() reads
'expiry_finalized' first, falls back to the legacy 'memory_flushed'
key so existing sessions.json files upgrade seamlessly.
Supersedes #15631 and #15638.
Tested: 383 targeted tests pass across run_agent/, agent/, cli/,
and gateway/ session-boundary suites. No behavior regressions —
background memory review continues to handle persistent memory
extraction on both CLI and gateway.
The _async_flush_memories() helper accepts (session_id) but both the
/new and /resume handlers passed two arguments (session_id, session_key).
The TypeError was silently swallowed at DEBUG level, so memory extraction
never ran when users typed /new or /resume.
One call site (the session expiry watcher) was already fixed in 9c96f669,
but /new and /resume were missed.
- gateway/run.py:3247 — remove stray session_key from /new handler
- gateway/run.py:4989 — remove stray session_key from /resume handler
- tests/gateway/test_resume_command.py:222 — update test assertion
Salvaged from PR #1470 by adavyas.
Core fix: Honcho tool calls in a multi-session gateway could route to
the wrong session because honcho_tools.py relied on process-global
state. Now threads session context through the call chain:
AIAgent._invoke_tool() → handle_function_call() → registry.dispatch()
→ handler **kw → _resolve_session_context()
Changes:
- Add _resolve_session_context() to prefer per-call context over globals
- Plumb honcho_manager + honcho_session_key through handle_function_call
- Add sync_honcho=False to run_conversation() for synthetic flush turns
- Pass honcho_session_key through gateway memory flush lifecycle
- Harden gateway PID detection when /proc cmdline is unreadable
- Make interrupt test scripts import-safe for pytest-xdist
- Wrap BibTeX examples in Jekyll raw blocks for docs build
- Fix thread-order-dependent assertion in client lifecycle test
- Expand Honcho docs: session isolation, lifecycle, routing internals
Dropped from original PR:
- Indentation change in _create_request_openai_client that would move
client creation inside the lock (causes unnecessary contention)
Co-authored-by: adavyas <adavyas@users.noreply.github.com>
Messaging users can now switch back to previously-named sessions:
- /resume My Project — resolves the title (with auto-lineage) and
restores that session's conversation history
- /resume (no args) — lists recent titled sessions to choose from
Adds SessionStore.switch_session() which ends the current session and
points the session entry at the target session ID so the old transcript
is loaded on the next message. Running agents are cleared on switch.
Completes the session naming feature from PR #720 for gateway users.
8 new tests covering: name resolution, lineage auto-latest, already-on-
session check, nonexistent names, agent cleanup, no-DB fallback, and
listing titled sessions.
