Multiplexed profiles no longer inherit another profile's terminal env in cron jobs and turns - #120307
Conversation
૮ >ﻌ< ა ci reviewran on 7cd737e — fix(terminal): persistent Docker keys a routed profile's cro debug infoCI timingsCI timings · View report · View jobWall time 6m17s vs 5m7s (+22.8%). 5 job(s) slower, 6 faster, 1 unchanged.
|
alt-glitch
added
type/bug
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 9ee31940ed1c15f94cac805fd26ad74315e615c4 against PR base d2ef7db751afcbee20120d124c41fe871c54b7b1 and current main 6b6c7f4a99f1407eca3b6c06d691a972a8d80323.
The two root-cause fixes are pointed at the right ownership boundaries. gateway.run now anchors the import-time env bridge to the process home rather than whichever routed ContextVar happened to import the module first, and the routed-home cache key removes the concrete LocalEnvironment cross-profile reuse that the new regression demonstrates. That is complementary to #119324 (cron binds the owning home/secret/terminal scope), #109703 (foreign cron .env restoration), and #119129/#119355 (launch-home/routed-home classification); this PR is not a duplicate of those branches.
There is one remaining class-level blocker in the terminal key classifier:
P1 — the same routed profile can still split into two persistent Docker identities depending only on whether the call has a session id. _SessionScope.docker_profile_scoped explicitly promises one long-lived persistent Docker container per profile across CLI/gateway/WebUI. Session-bound profile B resolves to profile:B, but the new no-session branch returns home:<B-home> before the profile-scoped branch is reached. _get_terminal_env() uses that classifier result as the environment-cache key and Docker task_id, so routed cron/background work for B can create/reuse a different persistent Docker environment from B's ordinary turns. Cross-profile leakage is fixed, but same-profile persistent-container continuity is broken. I left the concrete path inline.
Required closure: canonicalize routed no-session persistent-Docker work onto the same profile identity used by session-bound work (or an equivalent canonical key policy), and add a regression that proves both sides of the invariant: same-profile session-bound + no-session calls reuse one Docker key/container, while different routed profiles remain isolated. The current regression exercises the local backend and therefore cannot detect this split.
Verification state is also not merge-ready at this head. Exact-head CI run 35874259676 is failure (the Python 3.11 test job is the failing lane); exact-head Nix flake check 35874259109 and Docker 35874259130 are green. main has also advanced since this PR's base, so the eventual implementation needs a fresh exact-head/current-main green receipt rather than inheriting the branch's existing partial evidence.
I did not find another demonstrated defect worth inventing. Once the Docker identity is canonicalized and the exact current head is green against current main, the terminal side of this multiplex ownership class looks clean.
| return f"shared:{shared}" | ||
| if not session_key: | ||
| return "default" | ||
| return _routed_home_task_key() or "default" |
There was a problem hiding this comment.
P1 — keep routed no-session persistent-Docker calls on the same profile-scoped identity. For a session-bound routed profile, the classifier reaches the branch below and returns profile:<profile>; this new early return instead sends the same profile's cron/background call to home:<path>. _get_terminal_env() caches the DockerEnvironment under this key and passes it through as the Docker task_id, so profile B can now own two persistent containers solely because one call had a session id and the other did not. _SessionScope.docker_profile_scoped explicitly defines this mode as one long-lived container per profile across CLI/gateway/WebUI. Please canonicalize the no-session routed case to that same profile identity when persistent Docker is active, and add a regression proving same-profile session/no-session reuse while different routed profiles remain isolated.
1ef360a to
83819be
Compare
…rminal environment A multiplexed host runs every profile's cron jobs without a session key, and _resolve_container_task_id collapsed all of them onto the shared "default" environment. Profile B's cron tool calls therefore reused the LocalEnvironment the launch profile's job created: B's terminal subprocess saw the launch profile's .env residue and bridged TERMINAL_* (TERMINAL_CWD, backend policy). Found by tests/e2e/core/tenancy/test_two_tenant_gateway.py (C7 canary): alpha's cron env snapshot carried default's TENANT_MARKER and TERMINAL_CWD. Session-less work under a routed home override now keys home:<realpath>; the launch profile and single-profile processes keep "default". The unit test that pinned the shared key for a routed profile is updated.
… on the process home gateway.run bridges config.yaml into os.environ at import, keyed on get_hermes_home(). The Desktop backend (hermes serve) first imports it lazily from a session's agent build (tui_gateway.agent_callbacks._wire_callbacks), under that session's routed profile override, so whichever secondary profile built first latched its terminal.* (TERMINAL_CWD, backend...) and bridged settings into the launch process env for every later launch-profile turn and cron job. Found by tests/e2e/core/tenancy/test_two_tenant_desktop_backend.py (C7 canary): the default profile's cron env snapshot carried alpha's/beta's TERMINAL_CWD. Use get_process_hermes_home(); identical for a standalone gateway.
…dges the launch home Invariant for the import-time config bridge: a multiplexed backend's first import of gateway.run can happen inside a routed profile's session (hermes serve imports it lazily from an agent build), and the bridge must still write the launch home's agent.max_turns and terminal.cwd into the process env. Red on the previous gateway.run (HERMES_MAX_ITERATIONS came from the routed profile), green with get_process_hermes_home().
… its profile container Under persistent Docker, profile B's session-bound work keys profile:B but its session-less (cron) work keyed home:<path>, so the same profile ran two long-lived containers. The routed no-session branch now returns the same profile key as branch 3 when Docker is persistent (profile-scoped); other backends keep the per-home key.
83819be to
7cd737e
Compare
…wn_failure patterns #120307 and #120295 merged, so their cells are plain tests and the probe machinery in core/_pending_fixes.py has no consumer left (known_failure stays). torn-tree also excuses 'No module named' (a later release pair can die on a new module instead of a moved name); the gemini listing gate now requires the 401 from the models endpoint, so an outage or 5xx fails the cell.
…wn_failure patterns #120307 and #120295 merged, so their cells are plain tests and the probe machinery in core/_pending_fixes.py has no consumer left (known_failure stays). torn-tree also excuses 'No module named' (a later release pair can die on a new module instead of a moved name); the gemini listing gate now requires the 401 from the models endpoint, so an outage or 5xx fails the cell.
3 participants
On a multiplexed host (one
hermes gateway runor onehermes serveserving several profiles), a profile's cron jobs and turns now run in their own terminal environment and working directory. Before, they could pick up another profile'sTERMINAL_CWD,.envvalues and shell state.tools/terminal_tool.py):_resolve_container_task_idsent every task without a session key to the shared"default"environment. So profile B's cron tool calls reused theLocalEnvironmentthat the launch profile's job had created, including A's.envleftovers, its bridgedTERMINAL_*values and its shell. When a routed (non-launch) home override is active, that work now gets its own key,home:<realpath>. The launch profile and single-profile processes still use"default", so nothing changes for them. With persistent Docker (profile-scoped containers) the routed key isprofile:<name>, the same key that profile's session-bound work uses, so one profile never ends up with two long-lived containers (review finding from @andrexibiza).gateway.runimport (gateway/run.py): at import time the module does_hermes_home = get_hermes_home()and copiesconfig.yamlvalues intoos.environ.hermes serveimports this module lazily, from a session's agent build (tui_gateway.agent_callbacks) and under that session's profile override. Whichever secondary profile built first therefore wrote itsterminal.cwd,agent.max_turnsand the other bridged settings into the launch process env, and every later launch-profile turn and cron job inherited them. The bridge now readsget_process_hermes_home(). For a standalone gateway that returns the same home as before.Root cause: both paths treated "no session" or "first import" as meaning the launch profile, but under multiplex that code runs in whichever profile's context reached it first.
Live repro (the two-tenant C7 canary: a real multiplexed
gateway runorserve, 3 profiles, real cron ticks, real terminal subprocesses, loopback providers):origin/mainhermes gateway runcanaryprovider[alpha] … carries default's env_marker,… carries default's workdir,provider[beta] … carries default's env_markerhermes servecanary (*)default: tool subprocess (cron job) carries beta's workdir: 'TERMINAL_CWD=…/work-beta-…',provider[default] … carries beta's custom_prompt(*) On
origin/mainthe serve canary first stops at two other bugs the same lane found (session.createreports the launch model for a secondary profile;model.save_keywrites the launch.env). Those fixes land in their own PRs. For this A/B they were applied temporarily and not committed, so the red above is caused only by thegateway.runchange. The E2E files (tests/e2e/core/tenancy/) land separately in the E2E-suite PR and are not part of this PR.Tests
tests/gateway/test_config_env_bridge_authority.py::test_first_import_under_a_routed_override_bridges_the_process_home: importsgateway.runin a subprocess with a routed override set before the import, and asserts the launch home'smax_turnsandterminal.cwdreach the env. On base it is red with'222' == '111'; with the fix it is green.tests/tools/test_terminal_scope_multiplex.py::test_routed_turn_reads_every_terminal_consumer_from_profile. It expected the old shared key (_resolve_container_task_id(None) == "default") for a routed profile, which is exactly the leak, so it now expectshome:<realpath>. It is red on base (both params) and green with the fix.tests/tools/test_terminal_scope_multiplex.py::test_persistent_docker_routed_profile_keeps_one_container: two routed persistent-Docker profiles through the real_profile_runtime_scopeboundary. For each, the key with no session must equal the key with a session (profile:<name>), and the two profiles' keys must differ. Red on 9ee3194 ('home:…/profiles/bee' == 'profile:bee'), green on this head.scripts/run_tests.sh tests/gateway tests/tools: 18044 passed, 4 failed. All 4 files fail on unmodifiedorigin/maintoo (A/B done by swapping in the twoorigin/mainsource files):test_compression_failure_session_sync,test_api_server_active_work_drainandtest_session_hygiene_turnhold_adoptionfail identically on both, andtest_delegate_capacity_interruptfails on both with different params each run under load.ruff check,check_no_tmp_literals.pyandgit diff --checkare all clean.Duplicate sweep: no open or closed PR fixes either root cause. These related PRs work on the same "cron under multiplex" area through a different mechanism. This PR neither supersedes nor conflicts with them:
os.environafter a foreign-profile cron tick reloads.env."default", which the terminal half of this PR fixes._is_routed_homeis what this PR's routed-home check calls.Left open
gateway/run.pyreadsget_process_hermes_home(). Hosts that pin their process home (the WebUI mirror) should readget_routing_process_hermes_home(), but that helper landed onmainin c7c9c18 after this branch's base. Switching needs a rebase, so it is left as a follow-up.defaultprofile under a named launch profile with persistent Docker keys"default"for both session and cron work. That matches the existing session-keyed behavior and is not a change from this PR.Infographic