fix(gateway): give the routing index one store instead of the ambient one (salvage #99559) - #99770
Merged
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on db56cc2 — fix(gateway): _routing_db tolerates bare test instances (obj
|
alt-glitch
added
type/bug
teknium1
force-pushed
the
salvage/99559-routing-index-store
branch
from
August 31, 2026 21:17
75f2fad to
f671afd
Compare
…ient scope #88734 made SessionStore._db follow the ambient HERMES_HOME so a multiplexed profile's rows reach its own state.db. That is correct for the inbound message path, which installs the scope via _profile_runtime_scope. Nothing else does. _session_expiry_watcher (gateway/run.py) walks the single process-wide _entries dict — every profile's keys — and finalizes expired sessions with no scope installed, so _db resolved the ROOT store for rows that live under profiles/<name>/state.db. The scoped inbound path and the unscoped background path then maintained two copies of the same logical session whose end_reason drifted apart independently. Once they disagreed, the #54878 stale-routing guard read one copy while the routing index pointed at the other, and a live conversation was dropped and recreated — silently, since that branch only sets was_auto_reset when a reset policy also fired. Field evidence from a live two-profile install: session 20260814_234313 was end_reason=None in the root store but agent_close in the profile store, while 20260822_225807 was inverted. Both directions, which rules out a single mis-scoped writer. The owning profile is already encoded in the session key, so derive the store from it: _profile_home_for_key / _db_for_key, plus _db_for_session_id for the entry points addressed by session id. 40 self._db uses across 14 methods now resolve that way. No signature changed and no existing test was modified. _profile_home_for_key returns None when multiplexing is off, when the key carries the legacy agent:main namespace, or when the profile has no live directory, so single-profile installs resolve exactly where they always did. The explicit-path branch still goes through SessionDB.__init__ -> _ensure_test_isolation, keeping the live-DB guard over per-profile paths. Part of #66887. The routing-index half — _routing_scope() and the sessions.json mirror still pinned to one frozen sessions_dir while the handle moves — is left for a follow-up rather than mixed in here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback on the memo introduced with _profile_home_for_key. The problem is sharper than "no invalidation on profile deletion": caching the miss pinned a profile that appears AFTER the gateway started to the ambient store for the life of the process, which is the exact failure this helper exists to prevent. That is not hypothetical — an enrollment bridge can provision profiles/<name>/ at runtime, so a key is legitimately seen before its directory exists. Memoize hits only. A miss costs one profile_exists() stat and recurs only for profiles that genuinely do not exist, so the hot path for real profiles is still a dict hit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review P1. _profile_home_for_key() returned the same None for three different states — multiplexing off / legacy agent:main namespace, a named profile whose directory does not exist yet, and a resolution error — and _db_for_key() collapsed all of them to the ambient store. That recreated the very split this change removes. The enrollment bridge provisions profiles/<name>/ at runtime, so a key such as agent:fitness:telegram:dm:1 can legitimately be seen first: the first lookup landed in root state.db, and the next one, after provisioning, in profiles/fitness/state.db. One qualified session identity, two physical stores. The resolver-exception path fell open the same way. Ownership is now tri-state: - no named owner -> ambient DB (single-profile behavior intact) - named owner + home -> that profile's DB - named owner, unresolvable -> None, and a warning; never root Callers already treat a missing DB as "skip the mutation", which is the defer-don't-misroute behavior wanted here. _append_transcript_message is the one path reached with an id the entry-point guard did not check (the compression-child id), so it now raises explicitly and lets the caller's retry queue hold the row instead of relying on an AttributeError. Tests exercise the effect boundary rather than cache state: a named key before its profile exists leaves root untouched and lands only in the profile store once provisioned, and a resolver exception fails closed too. Both fail against the previous two-state behavior by returning a live SessionDB where None is required. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review P1 #2. _append_to_transcript_serialized() writes the compression continuation to child_id BEFORE publishing either _transcript_reroutes or the _entries update — that ordering is load-bearing for backlog order, so it must not move. At that moment nothing in the routing index points at the child, so _db_for_session_id(child_id) missed its scan and fell through to _db_for_key(None), i.e. the ambient store. The fail-closed guard did not fire because root is a live handle. The row therefore targeted root rather than the already-proven parent owner. With no child row there the append is rejected by the FOREIGN KEY constraint, the pending queue never drains and the reroute cannot advance; against a split-brain root the message would instead be written cross-profile. Record ownership before the mutation instead of moving the publication: a private _session_owner_hints map carries session_id -> owning key for ids whose owner is proven but not yet published, consulted by the new _owner_key_for_session_id() after the index scan misses, and dropped as soon as routing publishes. Signatures are unchanged, so the existing suites that stub _append_transcript_message keep working untouched; the map is read through getattr for stores built via object.__new__. The regression is physical rather than mocked: an ended compression parent and a live child that exist only in profiles/fitness/state.db, no active profile scope, append to the parent, then assert all four effects — the row lands on the child in the profile store, the pending queue drains, the reroute and the routing entry advance, and root state.db stays untouched. Without the hint it fails exactly as the review predicted, on "FOREIGN KEY constraint failed" against root. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… one Second half of #66887. _entries is a single flat dict holding every profile's keys, so the index it persists to has to be a single file — but it was read and written through _db, which resolves whichever profile scope is active. A whole-index rewrite during one profile's turn copied every other profile's routing rows into that profile's store, and startup, which runs unscoped, then loaded a different copy than the last writer produced. That is why the startup recovery pass never sees a secondary profile's crash marker, which is the half this issue's title names. mark_turn_active() persists through the single-entry fast path (state.db only, no sessions.json mirror), so a marker written during a profile's turn landed in that profile's store and _recover_unclean_sessions(), running with no scope, read a store that had never heard of it. The turn was silently never promoted to resume_pending. Capture the gateway's own home at construction — the store is built at startup before any profile scope exists — and route the index through it: _ensure_loaded_locked, _reconcile_recovered_routing_locked, _persist_routing_data and _save_entry now use _routing_db. A pinned handle still wins, so suites that install a fake or disable the DB are unaffected. _prune_stale_sessions_locked is the mixed case and is split accordingly: it now asks _db_for_key(key) whether each session ended, because that is a per-session question, while the index write stays on the single store. One ambient handle previously answered it for every profile at once, which could prune a live secondary-profile route on the strength of the root store's copy of that session. Regression as requested on the issue: mark a turn active under a secondary profile's scope, then build a fresh store with no scope and run recover_interrupted_turns(). It promotes exactly one turn to resume_pending here and promotes zero against the previous behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The #66887 fix pins the routing index to HERMES_HOME state.db; the fast-path harness still read entries through the ambient store. Point get_hermes_home at the test tmp so both are the same file, matching the new single-store contract.
Bare SessionStore instances built without __init__ lack _db_pinned, _routing_home, and the handle cache behind the _db property. Restore main's old getattr contract for them: report no DB and fall through to the sessions.json path instead of raising AttributeError.
teknium1
force-pushed
the
salvage/99559-routing-index-store
branch
from
August 31, 2026 21:49
f671afd to
db56cc2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Multiplexed gateways no longer scatter routing-index writes across whichever profile store is ambient — the index now lives in a single
_routing_homestore, which also fixes the startup recovery pass writing secondary-profile sessions into the wrong state.db (the remaining recovery half of #66887).Salvage of #99559 by @caya8205-2 — all five commits cherry-picked with authorship preserved (fork-branch CI couldn't dispatch, so this internal branch carries the identical rebased series). Root cause and mechanism are theirs; verified live in wave-2 with a two-profile repro (recovery wrote to the ambient store on main, correctly scoped with the fix).
Fixes #66887
Changes
_routing_homestore; session storage resolved from the key's profile, never ambient scope; fail closed on unresolvable profile homes; memoize only resolved homesValidation
_routing_homeLive repro: wave-2 two-profile harness — recovery-path routing write landed in the wrong store on main, scoped correctly with this series applied.
Infographic