Skip to content

fix(terminal): resolve terminal config per profile under multiplex - #94890

Closed
hashbender wants to merge 2 commits into
NousResearch:mainfrom
hashbender:salvage/profile-scoped-terminal-config
Closed

hashbender wants to merge 2 commits into
NousResearch:mainfrom
hashbender:salvage/profile-scoped-terminal-config

Conversation

@hashbender

Copy link
Copy Markdown

Summary

In a multiplex gateway every profile shares one process, but _get_env_config reads process-global os.environ — so every profile gets the SAME terminal backend, images, timeouts, and container resources regardless of its own config.yaml. Downstream, the system prompt describes the wrong backend, one profile's backend-probe output is served as another profile's environment hint, and the tool-availability TTL cache aliases across profiles.

Changes

  • tools/terminal_tool.py — _runtime_terminal_env(): during a profile-scoped turn (is_multiplex_active() and a bound secret scope), overlay the active profile's terminal config onto a private copy of the environment via the existing apply_terminal_config_to_env(env=..., config=load_config_readonly()) — os.environ is never mutated, and exported values the profile did not configure are preserved. Single-profile processes keep the historical _ensure_terminal_env_bridged() + os.environ path byte-for-byte. _get_env_config reads every TERMINAL_* value through this mapping, and _parse_env_var gains a keyword-only env parameter so numeric/JSON parsing reads the same snapshot.
  • agent/prompt_builder.py — build_environment_hints resolves the backend from _get_env_config()["env_type"] under multiplex (fail-soft to the env var), and _BACKEND_PROBE_CACHE is additionally keyed by the resolved profile home so one profile's probe output is never another profile's hint.
  • tools/registry.py — check_fn_cache_scope: an explicitly bound secret scope without multiplex (e.g. dashboard probe threads) now also scopes the availability cache, keyed by the resolved hermes home; multiplex-without-override still bypasses; failures still fail closed to bypass.

Validation

Check Result
New tests/tools/test_runtime_terminal_env.py (scoped overlay wins; os.environ untouched; unconfigured exports preserved; multiplex-without-scope and single-profile paths unchanged) + new profile-scoped availability-cache test + multiplexed-hint test (two profiles configuring ssh vs local, driven through the real gateway.run._profile_runtime_scope chain) + probe-cache profile-key test all pass
Touched + related suites: terminal_tool_requirements, parse_env_var, terminal_tool, prompt_builder, registry, terminal_env_bridge, terminal_config_env_sync, browser_extension_router 179 pass / 0 fail / 1 pre-existing skip
_get_env_config consumer sweep: container_cwd_sanitize, docker_network_config, file_tools_container_config ×2, gateway_cwd_contract, interrupted_command_cwd, modal_sandbox_fixes, ssh_environment, terminal_task_cwd, docker_session_isolation, terminal_degraded_mode 124 pass / 0 fail (5 environmental skips)

🤖 Generated with Claude Code

A multiplex gateway serves every profile from one process, but the
terminal tool read all of its settings from the process-global
os.environ (TERMINAL_*). Every profile therefore got the SAME backend,
images, and timeouts regardless of its own config.yaml; the system
prompt described the wrong backend for the turn; one profile's backend
probe output was cached and served as another profile's environment
hint; and the check_fn availability TTL cache aliased across profiles
whose scope was bound without multiplexing.

Four interlocking pieces close this:

- tools/terminal_tool.py: new _runtime_terminal_env() builds a private
  env mapping for a profile-scoped turn — a copy of os.environ overlaid
  with the active profile's terminal.* config via
  apply_terminal_config_to_env — so the profile's settings win without
  mutating global state, while exported values the profile did not
  configure are preserved. Single-profile processes keep the historical
  _ensure_terminal_env_bridged() + os.environ path byte-for-byte.
  _get_env_config() now reads every TERMINAL_* value through that
  mapping, and _parse_env_var grew a keyword-only env parameter for it.

- agent/prompt_builder.py: build_environment_hints() resolves the
  backend through _get_env_config() during a profile-scoped turn
  (fail-soft to the env var), so the prompt describes the profile's
  actual backend; _BACKEND_PROBE_CACHE keys now include the resolved
  profile home so one profile's probe output is never emitted into
  another profile's system prompt.

- tools/registry.py: check_fn_cache_scope() also scopes the
  availability cache by resolved hermes home when a secret scope is
  explicitly bound WITHOUT multiplexing (e.g. dashboard probe threads);
  multiplex-without-override still bypasses, and resolution failures
  still fail closed to bypass.

Covered by profile-isolation tests for the check_fn cache, the probe
cache, the multiplexed environment hint (two profiles, ssh vs local,
driven through the real _profile_runtime_scope chain), and direct
_runtime_terminal_env behavior on both the scoped and single-profile
paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/tools Tool registry, model_tools, toolsets tool/terminal Terminal execution and process management area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 25, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

The overlay design is the right shape: a private env copy per scoped turn (terminal_tool.py:1763-1799) avoids the obvious wrong fix (mutating os.environ per profile, which would race across multiplexed turns), keyed caches move to the profile boundary in both prompt_builder.py:1334-1337 and registry.py:331-344, and the fail directions are correct — scope resolution failure falls back to process env for reads, but cache-scope ambiguity bypasses caching instead of aliasing profiles. The three new tests cover the multiplexed, multiplex-unscoped, and legacy paths well.

Three points worth considering:

  1. The overlay doesn't see profile .env values (terminal_tool.py:1782-1795). Its base is dict(os.environ) plus terminal.* from config.yaml — but users commonly put TERMINAL_SSH_HOST/TERMINAL_DOCKER_IMAGE style settings in the profile's .env, not config.yaml. Under multiplex those keys were never in the process env, so the overlay can't carry them and the profile silently resolves backend config with them missing. If the intended contract is "config.yaml for terminal settings, .env only for credentials" it needs to be said explicitly; otherwise the base should come from the bound scope's resolved env, not the process env.

  2. Authority is only partially migrated. _get_env_config reads the overlay, but direct os.getenv("TERMINAL_*") readers elsewhere now disagree with it under multiplex — e.g. the _probe_remote_backend cwd_hint cache key (prompt_builder.py:1333) and any env-type branching outside _get_env_config. Profile-home keying masks part of the staleness, but two readers with different sources for the same setting is a standing source of "hint says ssh, shell runs docker" bugs. A grep audit for remaining direct readers, converting them to _get_env_config() or documenting why they must stay process-global, would finish the migration.

  3. Cache-scope hot paths just got more expensive in two directions (registry.py:336-343). Path(get_hermes_home()).expanduser().resolve() runs on every _check_fn_cached call for scope-bound processes — resolve() touches the filesystem for symlink resolution; memoize the computed key. And multiplex-without-override now returns CHECK_FN_CACHE_BYPASS, so a gateway that doesn't install home overrides per turn loses all check_fn caching and re-pays every provider's availability probe per check — correct, but worth a comment quantifying the cost so nobody "optimizes" the bypass back into aliasing later.

…er migration

Review follow-ups for the profile-scoped terminal config change:

- tools/terminal_tool.py: _runtime_terminal_env() now seeds the overlay
  with the bound secret scope's TERMINAL_*-prefixed entries between
  os.environ and the config.yaml overlay. A TERMINAL_* setting living
  only in the profile's .env travels in the scope (never in os.environ),
  so it was invisible to a scoped turn; the three-layer precedence
  (process env < profile .env < config.yaml terminal.*) keeps config
  authoritative, matching the bridge's config-over-stale-env semantics.
  Verified: build_profile_secret_scope() loads the entire profile .env
  into the scope, so arbitrary TERMINAL_* entries are present.

- tools/terminal_tool.py: converted the remaining direct
  os.getenv("TERMINAL_...") reads that feed per-profile decisions to the
  profile-aware view (_sudo_nopasswd_works, _maybe_reap_docker_orphans
  lifetime, _session_isolation_enabled, _docker_session_isolation_enabled,
  _docker_persistent_profile_scoped, _resolve_container_task_id shared
  key, terminal_tool degraded_mode). Deliberately process-global reads
  (import-time constants, _safe_getcwd's deleted-cwd emergency fallback,
  the __main__ diagnostic block) are now commented as such.

- agent/prompt_builder.py: _probe_remote_backend's cwd_hint cache-key
  component now resolves through _runtime_terminal_env in the same
  guarded block as the profile key (fail-soft to os.getenv), so a
  multiplexed turn's cache key can no longer disagree with the overlay
  its own probe config uses. The single-profile base read in
  build_environment_hints is commented as deliberate.

- tools/registry.py: memoized the Path.expanduser().resolve() of the
  hermes home in check_fn_cache_scope() (module-level dict keyed by the
  raw home string — bounded, homes are few), and documented the
  deliberate probe-re-run cost of the multiplex-without-override
  CHECK_FN_CACHE_BYPASS branch so it doesn't get "optimized" back.

- tests/tools/test_runtime_terminal_env.py: new test drives a real
  profile .env through build_profile_secret_scope into the overlay:
  .env-only keys carried, .env beats process env, config.yaml beats
  .env, and os.environ stays untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hashbender

Copy link
Copy Markdown
Author

Addressed in 526e252:

  1. Overlay now sees profile .env values — you found a real gap. Verified that build_profile_secret_scope() loads the entire profile .env (no credential-shape filter on .env entries), so the scope does carry arbitrary TERMINAL_* settings; _runtime_terminal_env now layers three sources with a precedence comment: os.environ < the bound scope's TERMINAL_*-prefixed entries < config.yaml terminal.* (config still wins, matching the bridge's config-over-stale-env semantics). Non-TERMINAL_* scope entries are never copied, so credentials can't leak into the terminal env mapping. New test drives a real on-disk .env through build_profile_secret_scope and pins all three layers plus os.environ being untouched.

  2. Hot-path cost — the home resolution is memoized (module-level map keyed by the raw home string; bounded by the fixed set of profile homes per process, deliberately never invalidated since a home re-resolving differently mid-process would mean the filesystem changed under a running gateway), and the multiplex-without-override bypass now carries a comment quantifying why re-paying availability probes is the price of not aliasing profiles.

  3. Reader migration finished for the touched files — beyond the cwd_hint cache key you flagged (now resolved through the profile-aware view, fail-soft), a sweep of the two touched files converted seven more per-profile-relevant direct reads (session-isolation and docker-persistence gates, shared-container-key resolution, sudo-probe env-type, docker-orphan lifetime, degraded-mode). The reads that deliberately stay process-global now carry one-line comments saying why (import-time constants that aren't config-bridged, the deleted-cwd emergency fallback, the __main__ diagnostics). One behavior note for review: functions that previously called _ensure_terminal_env_bridged() directly now skip the one-shot os.environ bridge during a scoped multiplex turn — intentional (a scoped turn shouldn't mutate process env), and the single-profile path is unchanged.

Suites re-run green: 144 passed across the adjudicated files, plus 123 across the suites covering every converted function; the two test_file_tools.py failures are the known pre-existing macOS /tmp→/private/tmp quirk, reproduced on the branch base without this diff.

@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

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.
Your PR is referenced in #101242's body as prior work on this bug.

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.

@teknium1 teknium1 closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants