Repository navigation
test(gateway): RC3 executor-residue gates + tool-submit context-mismatch warning net #281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
3c413ca
test(gateway): RC3 executor-residue gates + tool-submit context-misma…
Kyzcreig 69038cb
Merge branch 'main' into fix/executor-ctx-residue
Kyzcreig e2fd35d
Merge branch 'main' into fix/executor-ctx-residue
Kyzcreig 1b02867
fix(tool-executor): don't let the mismatch net self-disable silently …
Kyzcreig File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| # CTX residue changes | ||
|
|
||
| Date: 2026-07-10 | ||
|
|
||
| ## Header | ||
|
|
||
| No live-gateway restart was performed. I did not touch | ||
| `~/.hermes/hermes-agent` or `~/.hermes/runtime`. | ||
|
|
||
| Active-risk assessment: the branch contains the `_run_agent` choke-point fix | ||
| for the confirmed recursive/shortcut turn-entry class. The added RC3 tests did | ||
| not reproduce pool-thread residue through the wrapped gateway/tool seams. | ||
|
|
||
| ## Code changes | ||
|
|
||
| - Added a log-only tool-submit invariant check in | ||
| `agent/tool_executor.py`. | ||
| It compares the currently bound `HERMES_SESSION_KEY` against the executing | ||
| agent's `_gateway_session_key` immediately before concurrent tool jobs are | ||
| submitted. This is the runtime net requested for the worker/tool seam. | ||
| - Added worker-thread residue regressions in | ||
| `tests/gateway/test_session_context_inheritance.py`. | ||
| They poison a size-1 executor thread with a foreign ContextVar binding and | ||
| assert on the same subprocess-env surface the incident exposed. | ||
| - Added a focused unit for the new tool-submit warning. | ||
|
|
||
| ## Diagnosis summary | ||
|
|
||
| RC3 pool residue was ruled out for these two wrapped seams: | ||
|
|
||
| - `GatewayRunner._run_in_executor_with_context` uses `copy_context()` plus | ||
| `ctx.run`, so a recycled gateway pool thread's own ContextVars are not used. | ||
| - `tools.thread_context.propagate_context_to_thread` uses `copy_context()` plus | ||
| `ctx.run`, so a recycled tool worker's own ContextVars are not used. | ||
|
|
||
| The confirmed fix remains the `_run_agent` wrapper that binds from the current | ||
| call's `source`, `session_key`, `session_id`, and `event_message_id` before any | ||
| agent execution or executor snapshot, then restores by token on exit. | ||
|
|
||
| ## Gates run | ||
|
|
||
| - `scripts/run_tests.sh tests/gateway/test_session_context_inheritance.py` | ||
| passed: 14 tests. | ||
| - `scripts/run_tests.sh tests/tools/test_local_env_session_leak.py` | ||
| passed: 12 tests. | ||
| - `scripts/run_tests.sh tests/agent/test_close_interrupted_tool_sequence.py tests/agent/test_shell_hooks.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_approved_command_clean_slate.py` | ||
| passed: 92 tests. | ||
| - `scripts/run_tests.sh tests/tools/test_interrupt.py tests/agent/test_budget_grace_gate.py` | ||
| passed: 24 tests. | ||
| - `scripts/run_tests.sh tests/tools/test_tool_search.py` | ||
| passed: 39 tests. | ||
| - `python -m py_compile agent/tool_executor.py tests/gateway/test_session_context_inheritance.py` | ||
| passed. | ||
|
|
||
| ## Not completed | ||
|
|
||
| - I did not implement the requested `execute_code` live-agent namespace because | ||
| this checkout's `execute_code` path executes user code in subprocess/remote | ||
| environments, not an in-process namespace. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| # CTX residue diagnosis | ||
|
|
||
| Date: 2026-07-10 | ||
| Worktree: `/Users/alexgierczyk/.hermes/worktrees/ctx-residue` | ||
| Branch: `fix/executor-ctx-residue` | ||
|
|
||
| ## Result | ||
|
|
||
| Confirmed root cause in this checkout is the turn-entry / recursive `_run_agent` | ||
| binding class, not recycled executor-thread residue. | ||
|
|
||
| The current branch already contains the structural `_run_agent` wrapper fix for | ||
| that class. I added the missing RC3 worker-thread gates and they pass against | ||
| the fixed checkout, explicitly ruling out the two wrapped executor seams as the | ||
| active residue mechanism here: | ||
|
|
||
| - `tests/gateway/test_session_context_inheritance.py::test_gateway_executor_context_run_overrides_reused_thread_residue_bidirectionally` | ||
| - `tests/gateway/test_session_context_inheritance.py::test_tool_thread_context_wrapper_overrides_reused_thread_residue_bidirectionally` | ||
|
|
||
| Both tests poison a size-1 worker thread with a foreign binding, then assert on | ||
| the subprocess-env surface (`_make_run_env` / `_inject_session_context_env`) | ||
| from inside the reused worker. The gateway worker path observes the current | ||
| turn's binding because `GatewayRunner._run_in_executor_with_context` schedules | ||
| `ctx.run`. The tool-worker path observes the agent thread's binding because | ||
| `propagate_context_to_thread` snapshots the parent context and runs the target | ||
| under `ctx.run`. | ||
|
|
||
| ## Re-grounded anchors | ||
|
|
||
| - Handler entry reset: `gateway/run.py:10672-10685`. | ||
| This clears inherited task ContextVars before the pre-bind window. | ||
| - Top-level handler bind: `gateway/run.py:12926-12929`. | ||
| Normal message handling builds `SessionContext` and calls `_set_session_env`. | ||
| - Shared bind helper: `gateway/run.py:17223-17272`. | ||
| `_set_session_vars_for_source` binds source/chat/thread/user/session/message | ||
| identity and async-delivery capability. | ||
| - Gateway executor wrapper: `gateway/run.py:17279-17288`. | ||
| It captures `copy_context()` in the async task and schedules `ctx.run`. | ||
| - `_run_agent` choke point: `gateway/run.py:19282-19318`. | ||
| It resets then binds from this call's own `source`, `session_key`, | ||
| `session_id`, and `event_message_id` before `_run_agent_inner`. | ||
| - Existing entry warning: `gateway/run.py:19389-19401`. | ||
| It logs when the bound session key disagrees with the currently executing | ||
| turn's source-derived key. | ||
| - Recursive queued follow-up: `gateway/run.py:22392-22402`. | ||
| It re-enters `_run_agent`; the choke point above is what makes nested | ||
| follow-ups restore/bind correctly. | ||
| - Tool context wrapper: `tools/thread_context.py:64-105`. | ||
| It captures `contextvars.copy_context()` on the agent/tool-submit thread and | ||
| runs the tool target under that context. | ||
| - Tool submit warning added here: `agent/tool_executor.py:417-431`, | ||
| called at `agent/tool_executor.py:804-805`. | ||
|
|
||
| ## Mechanism | ||
|
|
||
| The live symptom was a coherent foreign identity in terminal subprocess env, | ||
| while the task-entry mismatch warning did not fire. The worker-thread surface | ||
| had to be tested directly because MainThread ContextVar reads are not a valid | ||
| gate for this failure class. | ||
|
|
||
| The deterministic residue tests show that a stale binding directly written onto | ||
| a reused worker thread does not leak through either wrapped seam: | ||
|
|
||
| 1. Gateway worker seam: `_run_in_executor_with_context` snapshots the current | ||
| async task after the turn bind, then calls `ctx.run` on the pool thread. | ||
| A poisoned pool thread is overridden for the duration of the job. | ||
| 2. Tool worker seam: `propagate_context_to_thread` snapshots the agent worker | ||
| thread at submit time and executes `_run_tool` under `ctx.run`. A poisoned | ||
| daemon worker is overridden for the duration of the tool job. | ||
|
|
||
| The confirmed bad path is therefore the caller-to-`_run_agent` path where | ||
| recursive or shortcut turns can bypass the top-level message handler's bind. | ||
| The fixed choke point is `_run_agent` itself. It is before `_run_agent_inner` | ||
| and before any `_run_in_executor_with_context` snapshot, satisfying the | ||
| bind-precedes-snapshot invariant. It uses token-based reset/restore semantics: | ||
| inner recursive exits restore the outer turn's binding rather than clearing it. | ||
|
|
||
| ## Failing-first tests for the confirmed path | ||
|
|
||
| Existing tests in this branch cover the confirmed mechanism and would fail if | ||
| the `_run_agent` rebinding wrapper were removed: | ||
|
|
||
| - `test_run_agent_rebinds_full_turn_context_before_inner_dispatch` | ||
| - `test_recursive_run_agent_rebinds_queued_cross_session_followup` | ||
| - `test_recursive_run_agent_rebinds_steer_fallback_message_anchor` | ||
| - `test_run_agent_composes_session_binding_with_profile_scope` | ||
|
|
||
| New RC3 tests cover the worker-thread residue suspicion and runtime net: | ||
|
|
||
| - `test_gateway_executor_context_run_overrides_reused_thread_residue_bidirectionally` | ||
| - `test_tool_thread_context_wrapper_overrides_reused_thread_residue_bidirectionally` | ||
| - `test_tool_submit_warns_on_agent_context_mismatch` | ||
|
|
||
| ## Execute-code probe note | ||
|
|
||
| The brief also requested exposing the live `agent` object to executed code when | ||
| `execute_code` runs in-process for the top-level agent. In this checkout, | ||
| `tools/code_execution_tool.py` runs user code through local subprocess sandbox | ||
| or remote environment dispatch; I found no in-process user-code namespace to | ||
| bind without changing the execution model. I did not add a fake in-process path. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| # Evidence pack + build brief — executor context residue (misbinding mech-B / RC3) | ||
|
|
||
| Date: 2026-07-10 evening · Author: Apollo · Worktree: ~/.hermes/worktrees/ctx-residue | ||
| Branch: fix/executor-ctx-residue (off fork/main @ f61bcb640, WITH #271 merged) | ||
| Parent spec: ~/.hermes/plans/2026-07-10_gateway-session-context-misbinding-spec.md | ||
| (READ IT FULLY — esp. §4 RC3, §5 B2 invariants. This brief is the RC3 branch it mandated.) | ||
|
|
||
| ## 0. What is already FIXED vs what is NOT | ||
|
|
||
| PR #271 (merged, deployed, running) fixed the TURN-ENTRY binding class: | ||
| `_run_agent` recursion paths now bind the correct per-turn context, with a | ||
| mismatch WARNING at ~gateway/run.py:19398 comparing the async task's bound | ||
| `HERMES_SESSION_KEY` against the turn's source-derived key. | ||
|
|
||
| TONIGHT'S FRESH EVIDENCE (post-#271, runtime f61bcb640, gateway pid 72309): | ||
| an interactive turn in session `agent:main:discord:thread:1525078186545909801:…` | ||
| (triggered by a REAL user message in that thread) executed its terminal-tool | ||
| subprocess with the COMPLETE identity of session | ||
| `agent:main:discord:thread:1525269534926442529:…`: | ||
| HERMES_SESSION_KEY/CHAT_ID/THREAD_ID=1525269534926442529 | ||
| HERMES_SESSION_MESSAGE_ID=1525277291276664964 (a ~16:05 message in the | ||
| FOREIGN thread), USER_NAME=Ace, chat_name of the foreign thread. | ||
| Two fresh persistent shells (pids 84668@16:08, 88531@16:12, 99737 parent | ||
| 80971@16:08:48) all carried it — so this is NOT stale-shell-env; the shells | ||
| were freshly spawned BY THIS TURN's tool calls and inherited the foreign | ||
| binding live. | ||
|
|
||
| CRITICAL: the #271 mismatch WARNING fired ZERO times this boot | ||
| (`grep -c 'Agent executor context mismatch' gateway.log` = 0). So the async | ||
| task WAS correctly bound at _run_agent entry. The foreign identity enters | ||
| BETWEEN the async-task bind and the tool-worker subprocess spawn. | ||
|
|
||
| Corroboration: the same turn's PROMPT contained a foreign session's restart | ||
| handoff (SGR-3A0BFCE6, an auth.json-lane session) — a delivery misroute | ||
| consistent with the same identity confusion. | ||
|
|
||
| ## 1. The seam to diagnose (RC3 outcome (b) — pool residue / unwrapped submit) | ||
|
|
||
| The propagation chain for a gateway turn: | ||
| async task (bound via _set_session_env / #271 choke point) | ||
| → GatewayRunner._run_in_executor_with_context (gateway/run.py ~17240): | ||
| copy_context() at call time → executor thread runs ctx.run(func) | ||
| → the AGENT WORKER THREAD runs the whole conversation loop under that | ||
| snapshot | ||
| → tool dispatch: agent/tool_executor.py:803 | ||
| executor.submit(propagate_context_to_thread(_run_tool), …) — copies the | ||
| AGENT WORKER THREAD's context to the tool worker | ||
| → tool worker spawns subprocess; tools/environments/local.py | ||
| _inject_session_context_env bridges ContextVars → child env | ||
| (ContextVar-authoritative when session_context_engaged()). | ||
|
|
||
| If the async task was bound correctly but the subprocess saw a foreign | ||
| identity, the corruption is inside this chain. Suspects, in order: | ||
|
|
||
| S1. A path that runs the conversation loop on a pool thread WITHOUT the | ||
| ctx.run wrapper (unwrapped executor.submit / to_thread / plain Thread) — | ||
| the pool thread then runs under ITS OWN thread-level contextvars, i.e. | ||
| residue from a previous job that SET vars at thread level. | ||
| Grep every executor.submit / run_in_executor / asyncio.to_thread in | ||
| gateway/run.py, agent/, tools/ and classify wrapped-vs-unwrapped. | ||
| KNOWN WRAPPED: gateway _run_in_executor_with_context, tool_executor:803, | ||
| async_delegation 233/425. Find the UNWRAPPED ones. | ||
|
|
||
| S2. A code path that calls set_session_vars / _CONTEXTVAR.set() ON a pool | ||
| thread directly (not via ctx.run isolation) — that write persists on the | ||
| thread across jobs (residue SOURCE). Grep set_session_vars call sites + | ||
| any .set( on the session ContextVars outside gateway/session_context.py. | ||
| Candidates: cron scheduler per-job set/reset (PR #88 pattern), | ||
| tui_gateway/slash_worker.py:121 (os.environ write path), | ||
| approval.py set_current_session_key, delegate_tool child binds, | ||
| _bind_child_cron_session. | ||
|
|
||
| S3. The FIFO / steer / queued-continuation drain: an event enqueued by | ||
| session A's turn but drained/executed inside session B's task or thread. | ||
| (My turn had queued steer notices; the foreign MESSAGE_ID was ~16:05, | ||
| close to my turn's start.) | ||
|
|
||
| S4. Streaming/tool-notification callbacks (notify_on_complete watchers, | ||
| process_registry watcher threads) that call back into agent machinery | ||
| with a stale captured context. | ||
|
|
||
| ## 2. Mandatory diagnosis-first protocol (spec §4) | ||
|
|
||
| 1. Build the failing test FIRST, on the surface that broke: a REAL | ||
| tool-shaped call through the worker-thread path asserting on the env dict | ||
| produced by _inject_session_context_env INSIDE the worker (grep spec §4 | ||
| B1 — MainThread ContextVar reads are execution-invalid for this gate). | ||
| 2. Reproduce the residue deterministically: two concurrent fake sessions A/B | ||
| through the real GatewayRunner dispatch path (hermetic temp HERMES_HOME, | ||
| see skill hermes-gateway-test-hermeticity + existing harness patterns in | ||
| tests/gateway/test_session_context_inheritance.py), N interleaved turns, | ||
| assert bidirectionally: B's worker sees B AND A's worker never sees B | ||
| (spec RC2). If S1/S2 is right, the failure will show within a bounded | ||
| number of interleavings on a size-1 executor (force thread reuse: | ||
| max_workers=1 makes residue deterministic). | ||
| 3. Write the diagnosis note (path → verified file:line → mechanism → | ||
| failing test name) at docs/sync/2026-07-10-ctx-residue-DIAGNOSIS.md | ||
| BEFORE the fix commit. If the repro does NOT fall out of S1–S4, STOP and | ||
| report honestly — do not fix blind. | ||
|
|
||
| ## 3. Fix contract (spec §5, B2 mode-independent invariants) | ||
|
|
||
| - Fix the residue CLASS: every executor job that can touch session | ||
| ContextVars runs under a fresh copied context (ctx.run), and/or | ||
| thread-level writes are reset in a finally (token discipline). Prefer the | ||
| smallest structural choke point (e.g. wrap the executor submit seam or | ||
| make DaemonThreadPoolExecutor context-propagating by default) over | ||
| patching N call sites — but ONLY if the bind-precedes-snapshot invariant | ||
| holds at that choke point. | ||
| - Token/nesting semantics per spec B2: inner exit restores OUTER binding, | ||
| never blanket reset. | ||
| - Keep the #271 mismatch WARNING; if diagnosis shows the corruption happens | ||
| AFTER that check, ADD a second cheap assert at the tool-executor submit | ||
| seam (log-only, compares worker context key vs the turn's expected key — | ||
| the net that would have caught tonight's case). | ||
| - No new env vars, no config knobs, no cache-breaking, no changes to | ||
| _inject_session_context_env's strip semantics (they are correct and | ||
| tested). | ||
| - ALSO (small, same PR — the probe surface): tools/code_execution_tool.py | ||
| runs in-process; expose the LIVE agent object to executed code as `agent` | ||
| in the execution namespace WHEN the tool executes in-process for the | ||
| top-level agent (it already has access to the process globals — this adds | ||
| a first-class handle instead of forcing hacks). Guard: only bind if the | ||
| executing context can resolve its owning AIAgent (tool_executor knows it); | ||
| never for sandboxed/subprocess modes. This gives operators a direct probe: | ||
| execute_code 'print(agent.context_compressor._recent_skews)'. Add a test. | ||
|
|
||
| ## 4. Gates (I certify, you don't self-certify) | ||
|
|
||
| - New failing-first tests fail on fork/main@f61bcb640, pass with the fix, | ||
| bidirectional + mutation-checked (revert fix → red). | ||
| - Full files green: tests/gateway/test_session_context_inheritance.py, | ||
| tests/tools/test_local_env_session_leak.py, tests/agent/ tool-executor | ||
| suites, plus your new file(s). | ||
| - py_compile on every touched file; scripts/run_tests.sh for suites. | ||
| - Do NOT commit — leave the worktree dirty; Apollo reviews, commits, lands. | ||
| - Write the diagnosis doc + a one-page CHANGES summary at | ||
| docs/sync/2026-07-10-ctx-residue-CHANGES.md. | ||
|
|
||
| ## 5. Boundaries | ||
|
|
||
| - Work ONLY in this worktree. Never touch ~/.hermes/hermes-agent (live | ||
| checkout) or ~/.hermes/runtime. No gateway restarts. No pushes. | ||
| - Tests hermetic: temp HERMES_HOME, no live state.db/sessions.json. | ||
| - The live gateway is running the code you're reading — flag anything you | ||
| find that's actively dangerous (vs latent) in the CHANGES doc header. |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.