Skip to content

fix(gateway): bind session context at agent turn entry (cross-session identity misbinding) - #271

Merged
Kyzcreig merged 3 commits into
mainfrom
fix/session-context-turn-binding
Jul 10, 2026
Merged

Kyzcreig merged 3 commits into
mainfrom
fix/session-context-turn-binding

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

Summary

Fixes the live, thrice-reproduced cross-session identity misbinding: an interactive turn could execute with the COMPLETE coherent identity (HERMES_SESSION_CHAT_ID/THREAD_ID/KEY/ID/MESSAGE_ID) of a different live session, causing real misroutes (kanban notify-subscribe subscribed a foreign Discord thread's chat id).

Root cause

_run_agent never enforced its own identity contract. The normal message handler binds session vars, but the in-band queued-follow-up path RECURSES into _run_agent directly — bypassing the handler — so the follow-up turn inherits the outer/sibling turn's coherent binding, which then propagates into the worker thread via copy_context() and out to every tool subprocess.

Fix (spec: Momus-reviewed, 2 passes)

  • _run_agent now resets inherited state and binds source/session_key/session_id/event_message_id from its OWN args before local or proxy execution — before any worker copy_context().
  • Nested (recursive follow-up) calls restore the OUTER binding via ContextVar tokens (restore_session_vars), never reset-to-UNSET — the A→B→A regression proves outer identity survives inner exit.
  • Log-only WARNING when the bound key disagrees with the current turn's source-derived key at dispatch.
  • Pool residue ruled out: all gateway agent jobs run under copy_context().run; regression uses a single-worker executor across A→B→A.

Tests

  • 3 new tests FAILED pre-fix, pass post-fix; all assert on the WORKER-THREAD/SUBPROCESS surface (_run_in_executor_with_context → subprocess env view), not MainThread ContextVars (which read _UNSET while the live bug was active — the fake-green trap).
  • Mutation checks: removing the choke-point bind → 4 fail; removing token restoration → 1 fail (A observes B's full identity).
  • Targeted sweep: 574 passed / 0 failed across 21 session/queue/restart/profile/proxy/cron/cache files (py3.11). Ruff + py_compile clean.

Cache safety

No prompt/messages/tool-schema/cache-key behavior changed — context binding only.

Reset and bind every _run_agent invocation from its own turn arguments before executor context copying, then restore nested ContextVar scopes with tokens.

Add worker/subprocess regressions for direct, queued, steer, multiplex-profile, mismatch-warning, and A-to-B-to-A nesting paths. Verified 574 targeted tests and ruff.
@greptile-apps

greptile-apps Bot commented Jul 10, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR fixes a live cross-session identity misbinding: recursive queued follow-up turns invoked _run_agent directly, bypassing the top-level message handler that normally binds session ContextVars, causing a queued turn to execute with the full coherent identity (CHAT_ID / THREAD_ID / KEY / ID / MESSAGE_ID) of a sibling session.

  • _run_agent is now the identity enforcement choke point: it calls reset_session_vars() (now returns tokens) then _set_session_vars_for_source() from its own call arguments before delegating to _run_agent_inner, regardless of what the caller had already bound.
  • Nested calls are safe via LIFO token restoration: outer identity is saved via reset_tokens and session_tokens, restored in the correct reverse order in a two-level finally block so recursive A→B→A turns always exit with A's binding intact.
  • restore_session_vars (new) applies per-token try/except so a single bad token cannot abort restoration of remaining vars. Five new tests validated against the worker-thread / subprocess environment surface.

Confidence Score: 5/5

Safe to merge. The fix is narrowly scoped to ContextVar binding order in _run_agent and introduces no behavioral changes to prompt content, cache keys, or tool schemas.

The change closes a well-documented, thrice-reproduced misroute by adding reset→rebind→restore logic at a single choke point. Token LIFO ordering is correct for every affected var, the two-level finally ensures restoration fires even when set_session_vars_for_source raises, and the per-token try/except in restore_session_vars prevents a single bad token from leaving a mixed identity. Five targeted tests validate the worker-thread surface where the live bug was observable.

No files require special attention. All four changed files are internally consistent and well-covered by the new tests.

Important Files Changed

Filename Overview
gateway/run.py Core fix: _run_agent now enforces its own session identity contract via reset→rebind→restore; _set_session_env refactored to delegate to the new _set_session_vars_for_source; _run_agent_inner gains a warning-only mismatch detector before dispatch.
gateway/session_context.py reset_session_vars() now returns tokens (was void); set_session_vars() captures the CWD token; new restore_session_vars() applies per-token best-effort restoration with logging.
agent/runtime_cwd.py Adds reset_session_cwd() returning a Token so _SESSION_CWD participates in the token-based restore chain; purely additive.
tests/gateway/test_session_context_inheritance.py Five new tests covering full turn rebind, multiplex profile scope, warning-net, cross-session A→B→A recursive identity, and same-session steer fallback message-id anchor — all asserting on the worker-thread subprocess-env surface.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant H as Message Handler
    participant RA as _run_agent (A)
    participant RAI as _run_agent_inner (A)
    participant RB as _run_agent (B, queued)
    participant RBI as _run_agent_inner (B)
    participant W as Worker Thread
    H->>H: reset_session_vars()
    H->>H: _set_session_env(ctx_A)
    H->>RA: "_run_agent(source=A)"
    RA->>RA: "reset_session_vars() -> reset_tokens_A"
    RA->>RA: "_set_session_vars_for_source(A) -> session_tokens_A"
    RA->>RAI: _run_agent_inner(A)
    RAI->>W: copy_context().run() sees A identity
    RAI->>RB: "_run_agent(source=B) queued follow-up"
    RB->>RB: "reset_session_vars() -> reset_tokens_B"
    RB->>RB: "_set_session_vars_for_source(B) -> session_tokens_B"
    RB->>RBI: _run_agent_inner(B)
    RBI->>W: copy_context().run() sees B identity
    RBI-->>RB: return
    RB->>RB: restore_session_vars(session_tokens_B)
    RB->>RB: restore_session_vars(reset_tokens_B) restores A
    RB-->>RAI: return
    RAI->>W: copy_context().run() sees A identity again
    RAI-->>RA: return
    RA->>RA: restore_session_vars(session_tokens_A)
    RA->>RA: restore_session_vars(reset_tokens_A)
    RA-->>H: return
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant H as Message Handler
    participant RA as _run_agent (A)
    participant RAI as _run_agent_inner (A)
    participant RB as _run_agent (B, queued)
    participant RBI as _run_agent_inner (B)
    participant W as Worker Thread
    H->>H: reset_session_vars()
    H->>H: _set_session_env(ctx_A)
    H->>RA: "_run_agent(source=A)"
    RA->>RA: "reset_session_vars() -> reset_tokens_A"
    RA->>RA: "_set_session_vars_for_source(A) -> session_tokens_A"
    RA->>RAI: _run_agent_inner(A)
    RAI->>W: copy_context().run() sees A identity
    RAI->>RB: "_run_agent(source=B) queued follow-up"
    RB->>RB: "reset_session_vars() -> reset_tokens_B"
    RB->>RB: "_set_session_vars_for_source(B) -> session_tokens_B"
    RB->>RBI: _run_agent_inner(B)
    RBI->>W: copy_context().run() sees B identity
    RBI-->>RB: return
    RB->>RB: restore_session_vars(session_tokens_B)
    RB->>RB: restore_session_vars(reset_tokens_B) restores A
    RB-->>RAI: return
    RAI->>W: copy_context().run() sees A identity again
    RAI-->>RA: return
    RA->>RA: restore_session_vars(session_tokens_A)
    RA->>RA: restore_session_vars(reset_tokens_A)
    RA-->>H: return
Loading

Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/session-con..." | Re-trigger Greptile

Comment thread gateway/session_context.py Outdated
@Kyzcreig
Kyzcreig enabled auto-merge July 10, 2026 19:55
@Kyzcreig
Kyzcreig merged commit 3c9328c into main Jul 10, 2026
35 checks passed
@Kyzcreig
Kyzcreig deleted the fix/session-context-turn-binding branch July 10, 2026 20:01
Kyzcreig added a commit that referenced this pull request Jul 11, 2026
…tch net

Follow-up to #271 (turn-entry binding). Tonight's live evidence (2026-07-10,
runtime f61bcb6): an interactive turn ran its terminal subprocesses with a
COMPLETE foreign session identity while the #271 entry warning stayed silent
— so the corruption enters between the async-task bind and the subprocess
spawn, or the entry warning's placement can't see it.

RC3 diagnosis (spec 2026-07-10, §4): deterministically POISON a reused
size-1 worker thread with a foreign binding and assert on the REAL
subprocess-env surface (_make_run_env/_inject_session_context_env) from
inside the worker:
- gateway executor seam (_run_in_executor_with_context): ctx.run overrides
  poisoned thread — PROVEN
- tool worker seam (propagate_context_to_thread): ctx.run overrides
  poisoned daemon worker — PROVEN
Both wrapped seams are therefore RULED OUT as the live mechanism. The live
leak path remains UNEXPLAINED (honest); prime remaining suspect is the
shared LocalEnvironment snapshot / an unwrapped submit not yet found.

Runtime net (the actionable half): _warn_on_tool_submit_session_mismatch
at the tool-batch submit seam — compares the executing agent's
_gateway_session_key against the bound HERMES_SESSION_KEY at the exact
point tonight's leak was visible. Next occurrence logs
'Tool executor context mismatch' with both keys instead of silently
misrouting. + test.

3 new tests (2 bidirectional residue gates + warning test); 26 pass across
session-context + env-leak suites; 36 pass tool suites.
Kyzcreig added a commit that referenced this pull request Jul 11, 2026
…shot

THE 2026-07-10 cross-session misbinding mechanism — finally pinned by a
second live occurrence (a hermetic test's HERMES_HOME=/tmp/dw_e2e/home
surfacing in a different session's interactive shell).

Mechanism: the persistent env snapshot (export -p dump, re-sourced by
every subsequent command in the environment) outlives a single turn, and
the gateway's 'default' LocalEnvironment is SHARED across sessions:
1. Session B's turn runs a command; the end-of-command re-dump writes
   B's injected HERMES_SESSION_* (+ any HERMES_HOME export) into the
   snapshot.
2. Session A's next command: per-spawn injection puts A's identity into
   the Popen env — then the wrapped script SOURCES the snapshot, whose
   'declare -x HERMES_SESSION_KEY=B' lines CLOBBER the injection.
3. A's command ends; re-dump persists B's identity again —
   self-perpetuating until someone exports over it.

This explains every observation from the day's hunt: coherent complete
foreign identity in terminal subprocess env; ContextVars correct
(#271's and #281's warnings silent — they watch the ContextVar surface,
which is fine; only the sourced-over child env is poisoned); fresh
shells inheriting it immediately (they source the same snapshot);
cross-HOME contamination from hermetic tests.

Fix: pipe both snapshot dumps (init_session bootstrap + per-command
re-dump) through a grep -vE filter excluding HERMES_SESSION_* and
HERMES_HOME. These vars are per-spawn-injected
(_inject_session_context_env / _inject_context_hermes_home) and the
injection must be authoritative; everything else still persists (the
snapshot's purpose is unchanged).

Tests: tests/tools/test_env_snapshot_session_leak.py — 4 new: the exact
incident sequence through a REAL LocalEnvironment (B exports identity ->
snapshot clean -> A's command sees EMPTY), persistence-purpose preserved
(non-session vars still roundtrip), bootstrap dump filtered (gateway
process env can't seed), regex contract (both declare -x/export forms;
lookalikes kept). Mutation-checked: filter neutered => incident + boot
tests red. 3 existing test_base_environment assertions updated to the
new dump shape.

Known pre-existing red (NOT this change): test_concurrent_writes_never_
tear_the_snapshot fails 3/3 on clean fork/main baseline (hand-rolled
shell repro, does not exercise this code path).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant