Skip to content

test(gateway): RC3 executor-residue gates + tool-submit context-mismatch warning net - #281

Merged
Kyzcreig merged 4 commits into
mainfrom
fix/executor-ctx-residue
Jul 11, 2026
Merged

Kyzcreig merged 4 commits into
mainfrom
fix/executor-ctx-residue

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

Follow-up to #271, driven by fresh live evidence (foreign session identity in subprocess env with the #271 entry warning silent).

What this proves: deterministic RC3 tests poison a reused size-1 worker thread with a foreign binding and assert on the real subprocess-env surface from inside the worker — both wrapped seams (gateway _run_in_executor_with_context, tool propagate_context_to_thread) correctly override residue. Ruled out.

What this adds: a log-only mismatch warning at the tool-batch submit seam (agent._gateway_session_key vs bound key) — the exact point tonight's leak was visible. Next live occurrence self-identifies with both keys.

Honest status: the live mechanism remains unexplained; diagnosis doc names the remaining suspects (shared LocalEnvironment snapshot, unwrapped submit). This PR ships the gates + the net, not a claimed fix.

…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
Kyzcreig enabled auto-merge July 10, 2026 23:38
@greptile-apps

greptile-apps Bot commented Jul 10, 2026 •

Copy link
Copy Markdown

Greptile Summary

Adds two deterministic RC3 tests that poison a size-1 pool thread with a foreign ContextVar binding and assert the gateway and tool-thread ctx.run wrappers correctly override that residue, ruling out both wrapped seams as the live leak mechanism. A log-only _warn_on_tool_submit_session_mismatch diagnostic is inserted at the tool-batch submit point in execute_tool_calls_concurrent, comparing the bound HERMES_SESSION_KEY against agent._gateway_session_key so the next live occurrence self-identifies.

  • agent/tool_executor.py gains _warn_on_tool_submit_session_mismatch: ImportError is silently skipped, unexpected exceptions now emit a debug-level trace (addressing prior review concern) rather than vanishing silently, and a warning fires only when the keys genuinely differ.
  • tests/gateway/test_session_context_inheritance.py adds three tests covering the gateway executor bidirectional residue override, the tool-thread wrapper bidirectional residue override, and the new warning function's mismatch branch.
  • Three docs/sync/ files are committed for traceability of the diagnosis, evidence pack, and change log.

Confidence Score: 5/5

Safe to merge. The production change is a log-only diagnostic that cannot affect tool dispatch correctness, and the new tests are hermetic.

The only production-code change is _warn_on_tool_submit_session_mismatch, which returns early or emits log output and never affects control flow. The new tests are isolated with the existing _isolate_session_context autouse fixture. No data path, session binding, or executor lifecycle is modified.

No files require special attention. The test file is the natural place to extend coverage but the gap does not affect production behavior.

Important Files Changed

Filename Overview
agent/tool_executor.py Adds _warn_on_tool_submit_session_mismatch diagnostic and wires it into execute_tool_calls_concurrent; exception handling is now visible (debug log) rather than silent, and the call site is correctly placed before the executor is created.
tests/gateway/test_session_context_inheritance.py Three new tests: two bidirectional residue-override probes (gateway executor and tool-thread wrapper), one warning-net unit. The mismatch warning test only exercises the mismatch branch; the matched-key complement is absent (flagged in previous review thread).
docs/sync/2026-07-10-ctx-residue-DIAGNOSIS.md Diagnosis document recording root-cause investigation, re-grounded line anchors, and mechanism description; no code changes.
docs/sync/2026-07-10-ctx-residue-CHANGES.md Changes summary listing code modifications, test results, and outstanding work; no code changes.
docs/sync/CTX-RESIDUE-BRIEF.md Evidence pack and build brief committed for traceability; no code changes.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant AT as Async Task
    participant GW as GatewayRunner._run_in_executor_with_context
    participant WT as Worker Thread
    participant TE as execute_tool_calls_concurrent
    participant WF as _warn_on_tool_submit_session_mismatch
    participant TW as Tool Worker Thread

    AT->>GW: copy_context() + ctx.run
    GW->>WT: ctx.run overrides residue
    WT->>TE: runnable_calls assembled
    TE->>WF: check bound key vs agent._gateway_session_key
    alt "bound != expected"
        WF-->>TE: logger.warning mismatch
    else "bound == expected"
        WF-->>TE: silent
    end
    TE->>TW: propagate_context_to_thread
    TW->>TW: subprocess env injected
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 AT as Async Task
    participant GW as GatewayRunner._run_in_executor_with_context
    participant WT as Worker Thread
    participant TE as execute_tool_calls_concurrent
    participant WF as _warn_on_tool_submit_session_mismatch
    participant TW as Tool Worker Thread

    AT->>GW: copy_context() + ctx.run
    GW->>WT: ctx.run overrides residue
    WT->>TE: runnable_calls assembled
    TE->>WF: check bound key vs agent._gateway_session_key
    alt "bound != expected"
        WF-->>TE: logger.warning mismatch
    else "bound == expected"
        WF-->>TE: silent
    end
    TE->>TW: propagate_context_to_thread
    TW->>TW: subprocess env injected
Loading

Reviews (3): Last reviewed commit: "fix(tool-executor): don't let the mismat..." | Re-trigger Greptile

Comment thread agent/tool_executor.py
Kyzcreig and others added 2 commits July 10, 2026 16:58
…(Greptile P2)

ImportError (expected: non-gateway contexts) returns quietly; any other
get_session_env failure now leaves a debug trace instead of silently
disabling the diagnostic — a monitoring net that can vanish without a
signal is the false-green class this whole PR exists to catch.
@Kyzcreig
Kyzcreig merged commit a25bbcc into main Jul 11, 2026
35 checks passed
@Kyzcreig
Kyzcreig deleted the fix/executor-ctx-residue branch July 11, 2026 00:14
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