fix(cache): honor the host-declared conversation key on the affinity-key path (salvage of #97158) - #97709
fix(cache): honor the host-declared conversation key on the affinity-key path (salvage of #97158)#97709kshitijk4poor wants to merge 2 commits into
Conversation
…key path Every conversation-affinity hint Hermes sends is derived from the PHYSICAL session id: prompt_cache_key on both OpenAI-wire transports, OpenRouter's and Nous Portal's sticky session_id, and xAI's x-grok-conv-id. A host that mints one physical session per RESPONSE re-keys all four on every reply, so the conversation never lands back on the routing bucket it just warmed (NousResearch#96811). Two hosts do exactly that. Hermes Studio's group chat mints gc_run_<room>_<profile>_<name>_<uuid4hex> per reply and destroys it after, and POST /v1/responses with client-managed history mints str(uuid4()) per request — while parsing X-Hermes-Session-Key one screen earlier and handing it to the agent. Hermes must not infer the logical conversation from the id's syntax: that rule merges independent client-supplied ids and Studio members truncated past its 96-character boundary (the NousResearch#79017 failure class). It does not have to. gateway_session_key is already the "stable per-chat key" built by gateway.session.build_session_key from that header, and branching deliberately does not key off it. The affinity path simply never consulted it. - agent/prompt_cache_scope.py: declared_conversation_scope() resolves the key into gwk_<sha256[:24]> and outranks the lineage walk (it is stable across rotation AND across per-response ids). Hashed because, unlike a session id, the key embeds platform/chat/user identifiers and leaves the process verbatim as a sticky id and as x-grok-conv-id. - agent/portal_tags.py: a separate ambient scope for ROUTING, published only when a host declared one. The providers read the attribution id when it is unset, so delegate trees keep sharing their parent's sticky key and every host that keeps one id per conversation is byte-identical to before. - hermes_state.py: is_explicit_fork_child() — the public view of the marker rules that keep /branch children, delegate subagents and tool children off their parent's chat key. Background-review forks clone the live runtime, so _persist_disabled excludes them for the same reason (NousResearch#79161). Refs NousResearch#96570 Fixes NousResearch#96811
The turn-lease timeout/interrupt paths return from inside the try block before set_affinity_scope() runs; the finally then read an unassigned local -> UnboundLocalError. This was the cause of the 4 red cross-process lease tests on PR NousResearch#97158's CI.
|
Status: converting to draft — @cervantesh's review on #97158 contains one blocker this salvage fixes and one it does not. Fixed here: the cleanup regression (blocker 2). Verified still present (blocker 1): the db.create_session('sess-A', source='telegram')
a = agent(session_id='sess-A', _gateway_session_key=K)
db.create_session('sess-B', source='telegram') # what /new produces — same chat key K
b = agent(session_id='sess-B', _gateway_session_key=K)
resolve_prompt_cache_scope(a) # gwk_e545a9eb80a9283345539b21
resolve_prompt_cache_scope(b) # gwk_e545a9eb80a9283345539b21 ← does NOT rotate
What a correct carrier needs (matching the review's criteria): stable across per-response physical ids within one logical conversation, rotating on Holding this salvage as draft until the lifecycle question is settled on #97158. The |
…r prompt cache affinity (NousResearch#96811) Unifies and finalizes host-declared conversation affinity caching across all supported transports (OpenAI, Codex, OpenRouter, Nous, Grok), building upon initial work in NousResearch#97158 and NousResearch#97709: - Consumes host-declared gateway_session_key and mixes conversation_epoch into gwk_<sha256(key:epoch)[:24]> - Advances conversation_epoch on explicit /new (SessionStore.reset_session) - Advances conversation_epoch monotonically on idle/daily policy auto-resets in get_or_create_session (preventing ABA rollback) - Adds public get_conversation_epoch() helper to SessionStore abstraction - Preserves physical-id isolation for /branch children, delegate subagents, tool-spawned sessions, and background-review forks (_persist_disabled) - Ensures safe _affinity_scope ContextVar propagation and explicit None-shadowing for nested child turns with safe try/finally - Adds 21 unit and integration tests covering stability, epoch rotation, fork exclusions, and persistence reload Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com> Co-authored-by: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Refs NousResearch#96811, NousResearch#97158, NousResearch#97709
|
Closing this salvage draft alongside #97158: the lifecycle blocker (declared chat-key scope does not rotate on |
The declared key is a per-CHAT identifier and outlives the conversation it names: reset_session() mints a fresh physical id on /new but keeps the key, and the idle/daily/suspended policy resets do the same. Hashing the key alone therefore mapped the conversation before a reset and the one after it onto one gwk_ scope -- the lifecycle violation @cervantesh raised on NousResearch#97158 and @kshitijk4poor reproduced on NousResearch#97709. No counter is introduced. The generation that must rotate is already durable: every one of those boundaries closes the outgoing row with an _RESET_END_REASONS end_reason, so SessionDB.latest_conversation_boundary reads the most recent one and declared_conversation_scope hashes 'key|generation'. That makes the carrier stable across a host's per-response physical ids -- a host that never resets writes no boundary, so every reply hashes the same value -- while rotating on every conversation replacement, /new and the policy auto-resets alike. ended_at only moves forward, so a retired generation can never be reused: no ABA. It also cannot drift from the rest of the codebase's notion of a conversation boundary, because find_latest_gateway_session_for_peer fences on the same set. The read is on the memoized resolution path, not per API call, and both lookups fail closed: an unqualified key would span a /new, so a DB error degrades to the physical-id scope. A SessionDB without the lookup keeps the previous behaviour. Refs NousResearch#96811
The declared key is a per-CHAT identifier and outlives the conversation it names: reset_session() mints a fresh physical id on /new but keeps the key, and the idle/daily/suspended policy resets do the same. Hashing the key alone therefore mapped the conversation before a reset and the one after it onto one gwk_ scope -- the lifecycle violation @cervantesh raised on #97158 and @kshitijk4poor reproduced on #97709. No counter is introduced. The generation that must rotate is already durable: every one of those boundaries closes the outgoing row with an _RESET_END_REASONS end_reason, so SessionDB.latest_conversation_boundary reads the most recent one and declared_conversation_scope hashes 'key|generation'. That makes the carrier stable across a host's per-response physical ids -- a host that never resets writes no boundary, so every reply hashes the same value -- while rotating on every conversation replacement, /new and the policy auto-resets alike. ended_at only moves forward, so a retired generation can never be reused: no ABA. It also cannot drift from the rest of the codebase's notion of a conversation boundary, because find_latest_gateway_session_for_peer fences on the same set. The read is on the memoized resolution path, not per API call, and both lookups fail closed: an unqualified key would span a /new, so a DB error degrades to the physical-id scope. A SessionDB without the lookup keeps the previous behaviour. Refs #96811
The declared key is a per-CHAT identifier and outlives the conversation it names: reset_session() mints a fresh physical id on /new but keeps the key, and the idle/daily/suspended policy resets do the same. Hashing the key alone therefore mapped the conversation before a reset and the one after it onto one gwk_ scope -- the lifecycle violation @cervantesh raised on NousResearch#97158 and @kshitijk4poor reproduced on NousResearch#97709. No counter is introduced. The generation that must rotate is already durable: every one of those boundaries closes the outgoing row with an _RESET_END_REASONS end_reason, so SessionDB.latest_conversation_boundary reads the most recent one and declared_conversation_scope hashes 'key|generation'. That makes the carrier stable across a host's per-response physical ids -- a host that never resets writes no boundary, so every reply hashes the same value -- while rotating on every conversation replacement, /new and the policy auto-resets alike. ended_at only moves forward, so a retired generation can never be reused: no ABA. It also cannot drift from the rest of the codebase's notion of a conversation boundary, because find_latest_gateway_session_for_peer fences on the same set. The read is on the memoized resolution path, not per API call, and both lookups fail closed: an unqualified key would span a /new, so a DB error degrades to the physical-id scope. A SessionDB without the lookup keeps the previous behaviour. Refs NousResearch#96811
Summary
Salvage of PR #97158 by @JoaoMarcos44 onto current
main, authorship preserved, plus a one-line fix for the CI failure the PR branch carried.Lets the host declare the logical conversation behind its physical session ids, and makes all four conversation-affinity surfaces consult that declaration:
prompt_cache_keyon both OpenAI-wire transports, the OpenRouter/Nous stickysession_id, and xAI'sx-grok-conv-id. Hosts that mint one physical session per RESPONSE (Hermes Studio group chat,POST /v1/responseswith client-managed history) re-keyed all four on every reply, so the conversation never landed back on the routing bucket it warmed (#96811, the cost half of #96570). The declared key is the already-in-handgateway_session_key(X-Hermes-Session-Key/build_session_key), hashed togwk_<sha256[:24]>so no platform/chat/user identifier reaches a provider. Explicit fork children (/branch, delegate, tool) and background-review forks keep their own scope (#79161); no id-syntax inference (#79017's failure class stays closed — see #96768's negative controls).Changes
agent/portal_tags.pyaffinity-scope ContextVar,agent/prompt_cache_scope.pydeclared_conversation_scope(memoized, fork-boundary-aware, degrades to physical id on DB error),hermes_state.pypublicis_explicit_fork_child, openrouter/nous profile fallback chains,run_agent.pyscope publication for turns and out-of-turn compaction, 365-line test fileaffinity_token = Nonealongsidetoken = Noneinrun_conversation— the turn-lease timeout/interrupt early paths return beforeset_affinity_scope()runs, and thefinallyreads the name unconditionally →UnboundLocalError. This was exactly the 4 red cross-process lease tests on the PR's CI; reproduced locally, red→green with this init.Review notes
gwk_is an unsalted hash of the chat key, so two installs serving the same chat share a sticky bucket — pre-existing property ofbuild_session_key, edge case.Validation
Closes #97158. Refs #96811, #96570.