fix(gateway): bind profile cwd in multiplexed sessions - #70600
ExitMaster wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the multiplexed cwd leak to the session binding seam. The core premise remains valid on current main: GatewayRunner._set_session_env() at gateway/run.py:20237 passes no cwd= to set_session_vars(), while agent/runtime_cwd.py:60 then falls back to process-global TERMINAL_CWD.
Problems
- The new ContextVar binding would not cover direct process-env readers.
gateway/run.py:15307readsos.environ["TERMINAL_CWD"]during inbound@-reference preprocessing, before session binding atgateway/run.py:15641;gateway/run.py:16885does the same for the runtime footer. A routed profile can still use or display the primary profile cwd on those paths.
Suggested changes
- Route these consumers through
resolve_agent_cwd()or bind the routed cwd before preprocessing, then add regressions for@references and the enabled footer. - Salvage the focused change onto the current
_set_session_env()atgateway/run.py:20237; the method moved during1a3a9de630's TurnRunner extraction.
Automated hermes-sweeper review.
| @@ -17179,9 +17180,69 @@ def _set_session_env(self, context: SessionContext) -> list: | |||
| session_key=context.session_key, | |||
There was a problem hiding this comment.
This binds the resolver used by agent startup and terminal seeding, but gateway/run.py:15307 still reads process-global TERMINAL_CWD before this session binding for @-reference expansion, and the footer does so at line 16885. Please route those paths through the scoped resolver or establish this cwd before preprocessing, with coverage.
Verified production repro + completed rebase with the review gaps coveredI hit this exact bug in production on v0.19.1: a Telegram topic routed to a secondary profile ( I rebased your fix onto current main and addressed the two gaps from the sweeper review:
Verification (branch
The branch is a drop-in superset of this PR's commit — if it's easier for the maintainers, the completed branch can be pulled into this PR, or the additional hunks can be applied on top of yours. Either way the fix is now complete and ready to merge. |
|
Update: the completed branch now also includes two further fixes found while verifying in production — a ContextVar wipe in |
|
Just to clarify authorship on the record: #70600 is @ExitMaster's PR, and dyreckt's #80546 builds on it (same fix, plus the review gaps: runtime footer, |
|
(Note: the previous "Superseded by #80546" line was posted in error by an automated close attempt; #70600 remains open and is @ExitMaster's PR — dyreckt's #80546 builds on it and both stay open for maintainer consolidation.) |
|
Thanks for this — I hit the same bug and can confirm the root cause. Two gaps remain after this PR that I wanted to flag, since both bit me on a multiplexed gateway where the secondary profile is reached over the API server rather than a messaging adapter: 1. The API-server path is not covered. 2. File tools never consult the session cwd contextvar. Minimal repro: # ~/.hermes/config.yaml
gateway: { multiplex_profiles: true }
terminal: { cwd: /home/u/.hermes/workspace }
# ~/.hermes/profiles/second/config.yaml
terminal: { cwd: /home/u/.hermes/profiles/second/workspace }
disabled_toolsets: [terminal]Put a different Happy to open a separate issue for the file-tools half if you'd prefer to keep this PR scoped. |
In a multiplexed gateway, startup bridges only the primary profile's terminal.cwd to process-global TERMINAL_CWD. Sessions routed to a secondary profile resolved their cwd from that process-global latch, so the runtime footer, @-reference preprocessing, and the terminal tool all used the wrong profile's workspace — and whichever profile made the first terminal call after a restart stamped its workspace process-wide for every lane. - gateway/run.py: _set_session_env() resolves the routed profile's terminal.cwd and binds it through the session ContextVar; the runtime footer and @-reference preprocessor read the session-cwd override instead of os.environ["TERMINAL_CWD"]; _resolve_profile_home_for_source() falls back to the gateway process's own home rather than get_active_profile_name(), which can leak a previously routed turn's profile scope through copy_context() - agent/runtime_cwd.py: expose get_session_cwd_override() - tools/terminal_tool.py: seed the per-session cwd from the ContextVar override instead of the process-global TERMINAL_CWD latch - tests: multiplex profile cwd isolation, footer and @-reference coverage, unrouted-source home resolution under an inherited profile scope, and profile-resolution fallback tests repointed at the new gateway-home seam Builds on NousResearch#70600 and addresses its review gaps (@-reference preprocessor and runtime footer now honor the session-cwd override). Co-authored-by: ExitMaster <292490062+ExitMaster@users.noreply.github.com>
|
Thanks for this PR. Merged via #101242 (4a7f228) on current main — routed multiplex profiles get their own terminal cwd/backend/docker config; container boot honors config multiplex_profiles. #101242 won as the consolidated fix because it covers the whole multiplex-profile bug class in one change (with tests) rather than the single symptom addressed here; this PR is superseded by it. If anything from your original change is still missing on main >= 4a7f228, please open a fresh PR/issue against main and tag it. Thanks again. |
Summary
Fix multiplexed gateway sessions inheriting the primary profile's process-global terminal cwd instead of the routed profile's configured cwd.
Root cause
Gateway startup bridges only the active profile's
terminal.cwdinto process-globalTERMINAL_CWD. A routed secondary profile loaded its own context files, but terminal commands still inherited the primary process cwd. The terminal environment is also cached across sessions, so changing only initial environment configuration does not prevent cwd bleed.Changes
terminal.cwdinside the existing profile runtime scopeRegression coverage
cdstate preservationVerification
6 passed291 passedpy_compileruff checkgit diff --checkScope
This PR intentionally addresses the local terminal backend only. Profile-specific SSH/container backend configuration remains outside this change because those backends use different path semantics and process-global backend configuration.
Model used
OpenAI Codex
gpt-5.6-sol; the implementation was manually constrained by TDD, source inspection, and executable regression tests.