gateway.multiplex_profiles defaults to on, gated by a boot-time serve guard that shows in status - #112854
Conversation
…boot-time serve guard DEFAULT_CONFIG now ships gateway.multiplex_profiles: true. GatewayConfig keeps an UNSET flag as None so the boot can tell "the operator chose" from "the default applies"; every reader tests truthiness, so an undecided flag never multiplexes by accident. hermes_cli/gateway_multiplex_mode.py settles the unset default once per boot (called from load_gateway_config_for_runner and `gateway run --config`): the same preflight `hermes gateway migrate --multiplex` runs — default profile, >= 2 profiles, no secondary running its own gateway (live pid or installed unit), no duplicate-credential / port-binder blocker, migratable host. A refusal is a logged warning naming the blocker and the migrate one-liner; the gateway comes up standalone exactly as before. Explicit values (config.yaml, GATEWAY_MULTIPLEX_PROFILES) pass through verbatim; `--standalone` already pins false. Other processes stop guessing the verdict from the merged default: named_profile_served_by_running _multiplexer, the enroll warning, the dashboard listener guard, the cron-fire port resolver, container boot and the migration plan (_read_multiplex_flag) read the live gateway's served_profiles record first and the EXPLICIT flag second — so a per-profile fleet with the flag unset still reads as "not yet multiplexed" and the fold proceeds. Docs: multi-profile-gateways.md, multiplexing-gateway.md, hermes_cli/AGENTS.md.
The boot guard logged its refusal once and nothing else knew. A user whose default says multiplex but whose gateway serves one profile would read `hermes gateway status` and see a healthy gateway. The verdict now lands in gateway_state.json (multiplex_standalone_reason) and status prints it with both remedies; a boot that does multiplex clears it.
૮ >ﻌ< ა ci reviewran on 54bf9ef — gateway status shows why an unset-default gateway stayed sta debug infoCI timingsCI timings · View report · View jobWall time 6m10s vs 6m40s (-7.5%). 10 job(s) slower, 3 faster,
|
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 54bf9ef72d0af99d2998716729b58d1bec6652ab against its actual merge base 6cd2502629cf17eeac7c8dda466d04cb72096146 and current main 784d5c3f9c2cb77698d8a9d2e72b1d106a38ea88. The branch is 2 commits ahead / 25 behind; I compared the intervening main delta and it is path-disjoint from all 17 files in this PR, so I found no current-main collision that changes the conclusions below.
The overall direction is right: keeping UNSET distinct from an operator verdict is the correct shape, using the live gateway's served_profiles as runtime truth is materially better than making every CLI process guess from merged config, and reusing the migration preflight instead of blindly flipping the default is exactly the right class of safeguard. The status explanation is also useful settlement evidence rather than leaving the user with a healthy-looking standalone gateway and no reason.
I do not think the default-on transition is safe to merge at this head yet. I found two P1 authority failures in the new guard itself, and the rollout currently precedes several known open multiplex-isolation fixes that are complementary to this carrier.
P1 — the implicit-default preflight has a check→act race with standalone secondary startup
resolve_multiplex_mode() samples build_migration_plan() once and commits the result into this process's config. The named-profile start guard, however, now deliberately recognizes a default multiplexer only from (a) its live served_profiles record or (b) an explicit default-profile flag. That creates a real gap unique to the new implicit default:
- default gateway, flag unset:
implicit_multiplex_blocker()sees no standalone secondary and returns clear; - this process commits
multiplex_profiles=Truein memory; - before
GatewayAdapterLifecycleMixin._record_served_profiles()publishes the served set,hermes -p coder gateway runstarts; - coder's guard sees no authoritative served record yet;
explicit_multiplex_flag(default_root)is stillNone, so coder is allowed to continue; - the default boot does not re-run the fleet preflight and proceeds to build coder's adapters as a multiplexer.
The PID claim does not close this window: start_gateway() claims the default PID before adapters, but recorded_served_profiles() returns None while the live process has not yet written the served_profiles key, and the fallback intentionally refuses to infer multiplexing from the unset default. This recreates exactly the two-owner state the boot guard says it prevents — two pollers on one credential / competing port ownership — just through concurrency instead of pre-existing fleet state.
This needs one cross-entry-point ownership boundary, not another sample. A machine/fleet transition lease shared by default-multiplex boot and named standalone startup would be the strongest shape; an authoritative generation-bound multiplex_pending publication can also work if named starts respect it and the default revalidates before adapter acquisition. Either way, please add a deterministic barrier regression: pause the default after the preflight has cleared but before served-profile publication, start coder, then prove that one side must lose before either side can own coder's adapters.
P1 — an invalid old config token is promoted from safe-off to explicit-on and bypasses the guard
There are two coupled paths here. GatewayConfig.from_dict() changed the multiplex_profiles coercion fallback from False to True, and explicit_multiplex_flag() maps an unrecognized string to True as well. So an existing config such as:
gateway:
multiplex_profiles: truused to coerce false; on this head it becomes a non-None True. resolve_multiplex_mode() then classifies it as source=config and intentionally skips implicit_multiplex_blocker() entirely. A typo can therefore bypass the live-secondary, duplicate-credential and port-binder checks that make this default flip safe. The environment override already has the safer behavior: unrecognized tokens are warned and ignored rather than promoted to authority.
An unrecognized config scalar is not an operator grant. Preserve it as unset so the boot guard decides, or reject/warn and stay standalone; do not mint explicit multiplex authority from parse failure. Please cover malformed top-level and nested config values with a live secondary present so the regression proves the guard cannot be skipped.
Merge-order / interlock graph
This PR is the rollout carrier, not the closure carrier for every profile-isolation surface, and that distinction matters more once multiplexing becomes the default. The campaign tracker #109417 explicitly describes the residual sweep as not class closure. Since that status was written, current upstream has several deterministic, still-open profile-isolation carriers:
- #112593 (
poijygfdyy) — bounded Nous account fetches lose the routed profileContextin a bare executor and can read launch-profile auth/account state; - #112599 (
poijygfdyy) — the global async-delegation stale monitor can terminalize the origin profile's work in the launch profile's durable ledger; - #112824 (
paulybeebe) — named-profile Dashboard chat child env can inherit launch-profile credential/environment authority; its head has advanced since the earlier review and is still open; - #112845 (
poijygfdyy) — abandoned-turn reaper threads lose the served profile and can perform process-registry cleanup against the launch profile; - #112846 (
anhtahaylove) — the legacy mirror-session fallback uses an import-time launch-profile sessions index and can resolve a same-key delivery to another profile's session.
Those are complementary fixes, not duplicates of this PR and not work to absorb without attribution. But a default-on carrier changes their exposure from opt-in multiplex users to ordinary multi-profile installs. I would interlock the rollout behind the applicable isolation carriers (or explicitly disable the affected surfaces until they settle) rather than merge default-on first and use the real install as the detector.
The existing prerequisite lineage is good and should stay explicit: #112651 is the migration/compensation hardening, #112685 is the routed credential/scope carrier, and #112653 is the rename/delete lifecycle carrier preserving the contributor lineage from #111927/#112594/#112601/#112596. This PR builds on those rather than superseding them.
Verification / acceptance
Exact-head hosted evidence is genuinely green: CI 35090058614, Docker 35090058018, and Nix 35090058353 all succeeded on 54bf9ef, and the CI run contains 27 real jobs with Python tests, e2e, Windows/macOS lanes, ruff, ty-diff, attribution/common-ancestor and the required-check aggregate all settling successfully. That is strong producer/hosted evidence for the reviewed object.
The history is not every-surviving-commit green, though: the first surviving commit 7914d59a17bd84bf18f5c7c4637a61a64cf58c98 has no associated workflow runs. So the current branch satisfies exact-head acceptance but not per-commit acceptance.
I would also move the planned 11-profile real-install soak before merge on this exact head. For a configuration-default transition, "merge, then make the maintainer install the first real-home probe" reverses the acceptance boundary. The two counterexamples above are precisely the kind of startup/config-state behavior a real fleet can expose even when a 4,926-test multiplex selection is green.
Once the transition is fenced against concurrent secondary startup, malformed config cannot mint authority, the known isolation interlocks are settled/explicitly gated, and the surviving commits have acceptance receipts, the default-on architecture here is coherent.
| if current is not None: | ||
| return MultiplexDecision(bool(current), "config") | ||
| try: | ||
| blocker = implicit_multiplex_blocker() |
There was a problem hiding this comment.
P1 — the implicit-default guard is check-then-act, so a standalone secondary can start in the publication gap. implicit_multiplex_blocker() samples the fleet once, but the result is only stored in this process. Until _record_served_profiles() later publishes the live served set, a concurrent hermes -p coder gateway run sees an unset explicit flag and no authoritative served record, so its named-profile guard allows it to start. This default process never re-runs the preflight and can then build coder's adapters too. That recreates the exact double-poller / port-owner state this guard is supposed to prevent. Please make the decision→publication interval an ownership boundary shared with named-profile startup (fleet lease, or equivalent generation-bound pending intent + revalidation) and add a deterministic barrier test that pauses here, starts a secondary, and proves one side loses before adapter ownership.
There was a problem hiding this comment.
Agreed on the gap. The shape I'd lean toward is recording the verdict synchronously where resolve_multiplex_mode settles it — including the positive/pending case, not just the refusal — instead of only persisting into gateway_state.json on refusal. Then served_profiles is just the lazy confirmation of an already-recorded intent, and the secondary-side guard consults that record before falling back to the explicit flag. It also covers the reverse direction: the preflight snapshot can't see a mid-boot secondary either, but the shared record closes both.
| return None | ||
| if isinstance(value, str): | ||
| parsed = _bool_token(value) | ||
| return True if parsed is None else parsed |
There was a problem hiding this comment.
P1 — parse failure is being upgraded into explicit multiplex authority. An unrecognized config string (for example multiplex_profiles: tru) lands here as True; GatewayConfig.from_dict() also changed its fallback to _coerce_bool(..., True). On current main the same malformed value fell back false. On this head it becomes non-None/true, so resolve_multiplex_mode() treats it as an explicit operator choice and skips the live-secondary / duplicate-credential / port-binder preflight. The env override already does the safer thing: warn and ignore unknown tokens. Please keep an unrecognized config value unset (so the boot guard decides) or reject/warn and stay standalone; it must not become a grant. Cover malformed top-level + nested values with a live secondary in the regression.
There was a problem hiding this comment.
Seconded on this one. The env override already warns-and-ignores unknown tokens; making the config path do the same keeps it consistent. A malformed value quietly becoming a grant is exactly the failure mode the preflight was built to prevent.
MindDragonLabs
left a comment
There was a problem hiding this comment.
Post-merge review — three independent reviewers, same clone
Ran a three-way review over this PR at head 54bf9ef (fresh clone): Hermes (GLM-5.3), Codex (gpt-6-astra), and Kimi (k3), each working independently, then cross-checked every claim by hand. Tests run at head:
tests/hermes_cli/test_gateway_multiplex_mode.py+tests/gateway/test_multiplex_phase0.py— 20 passedtests/gateway/test_status.py+tests/hermes_cli/test_config.py— 204 passed, 6 skipped
Consensus: the design is sound. Unset stays None end-to-end, one boot-time resolver settles it, every production reader tests truthiness, the live record correctly outranks the raw flag for other processes, and the migration/auto-fold interplay survives. One real defect class slipped through, plus narrower ones. All three reviewers found the first one independently.
1. MAJOR — unrecognized config strings coerce to explicit true, skipping the guard
gateway/config.py:730 flips _coerce_bool(multiplex_profiles, False) → (..., True). So multiplex_profiles: "flase" / "ture" / "enabled" / "" now parses as an explicit operator opt-in: resolve_multiplex_mode returns source="config" and the entire preflight is skipped. Reproduced at head:
GatewayConfig.from_dict({'gateway': {'multiplex_profiles': 'flase'}}).multiplex_profiles -> True
'' -> True, 'yes please' -> True
Pre-PR the same typo parsed False (fail-closed). A typo on a fleet with live per-profile gateways silently double-binds bot tokens — the exact scenario the guard exists to prevent. Notably, the env-var path (_env_multiplex_profiles_override) already handles garbage the right way: warn + treat as unset. The config path should match: unrecognized string → warning + None so the guard decides. Same shape in explicit_multiplex_flag (gateway_multiplex_mode.py:49).
2. MAJOR — gateway run --config alt.yaml preflights the wrong config
The --config path passes the injected config to the resolver, but implicit_multiplex_blocker() → build_migration_plan() re-reads each home's normal config (gateway_migrate.py:426), not the launch config. Codex reproduced: an alternate launch config sharing a secondary's bot token passes the resolver, while checking the actual launch config yields a duplicate-credential blocker. Result: duplicate adapters parked at startup, a profile left without an adapter. Narrow (manual/testing path), but real.
3. MAJOR — decision record writes before gateway ownership is won (--config path only)
On the --config path, log_multiplex_decision (run.py:5472) runs before start_gateway's duplicate-instance guard (run.py:5273) and the PID claim (run.py:5339). A second --config start re-stamps the live gateway's status record — new pid/argv/start_time merged with the old process's running state and served_profiles — then exits. The normal (non---config) path is fine: the guard fires before GatewayRunner construction. Fix: hold the decision in memory and persist it after the PID claim.
4. MINOR — first-boot visibility window
record_multiplex_decision writes only multiplex_standalone_reason; served_profiles lands much later, after the adapter connect loop (run_adapters.py:899). During the first-ever multiplex boot (seconds; minutes with retries), every other process — secondary-start refusal, cron-fire port resolver, dashboard listener guard, enroll warning — reads "not multiplexing". Reboots of an established multiplexer are masked by the stale record, so this is first-boot-only. Fix: write served_profiles (or [] for guard refusals) together with the verdict.
5. MINOR — status headline is self-contradictory on named profiles
A named-profile gateway records its guard reason ("this is profile 'coder's own gateway…") in its own state file. hermes -p coder gateway status then prints "⚠ Serving the default profile only … this is profile 'coder's own gateway" — and the remedy line omits the -p qualifier a secondary needs. Also: the named-profile refusal is the designed topology but logs at WARNING every boot (gateway_multiplex_mode.py:143-147). Suggest INFO for that arm, like SINGLE_PROFILE_REASON.
6. MINOR — explicit_multiplex_flag ignores the managed overlay
The helper reads raw user YAML only; the dashboard listener guard previously read managed-aware load_gateway_config(). User true + managed false → helper says True, gateway says False → the dashboard can reject a valid standalone listener config. Managed-overlay installs only.
Nits
- Stale comment at
gateway/run.py:3410-3411still says--configconfigs keep an unset flag — the same PR settles it at 5472. test_gateway_multiplex_mode.py:75-76: thenamed_profile_served_by_running_multiplexerassertion is vacuous (returns at the pid guard before the tested fallback runs).- Unpinned arms: the garbage-string coercion above, s6/Windows refusals (
_host_supports_migrationfaked in the fixture), andrun.py:1802verdict inheritance on scoped reload.
Checked and found sound
- Every production reader tests truthiness; the only
is Trueis the deliberate s6 slot decision (container_boot.py:87). GATEWAY_MULTIPLEX_PROFILESis pre-existing (#60589) — not a new env path.- Process detection stays on canonical identity helpers; no argv-substring matching.
- Rollback pins explicit
false;hermes updateauto-fold still proceeds for per-profile fleets. - Function-level imports throughout; no cycles; no dead imports left.
Suggested priority: fix #1 first (small, high-value), #2+#3 together, then the rest.
| stt_enabled=_coerce_bool(stt_setting("stt_enabled", "enabled"), True), | ||
| stt_echo_transcripts=_coerce_bool(stt_setting("stt_echo_transcripts", "echo_transcripts"), True), | ||
| multiplex_profiles=_coerce_bool(multiplex_profiles, False), | ||
| multiplex_profiles=None if multiplex_profiles is None else _coerce_bool(multiplex_profiles, True), |
There was a problem hiding this comment.
This fallback flip (False → True) makes any unrecognized string an explicit opt-in: 'flase', 'ture', 'enabled', '' all parse as True and skip the boot guard entirely. Pre-PR they parsed False. The env-var path handles garbage correctly (warn + unset) — the config path should match. All three reviewers hit this independently; reproduced at head. Suggested: unrecognized → warn + None so the guard decides.
| return None | ||
| if isinstance(value, str): | ||
| parsed = _bool_token(value) | ||
| return True if parsed is None else parsed |
There was a problem hiding this comment.
Second site of the fail-open coercion: True if parsed is None else parsed turns an unrecognized token into explicit true. Same fix as gateway/config.py:730 — treat unparsable as unset and let the guard decide.
| config = GatewayConfig.from_dict(yaml.safe_load(f) or {}) | ||
| # Same boot-time verdict the loaded config gets when the file leaves the flag unset. | ||
| from hermes_cli.gateway_multiplex_mode import log_multiplex_decision, resolve_multiplex_mode | ||
| log_multiplex_decision(resolve_multiplex_mode(config)) |
There was a problem hiding this comment.
Two issues on the --config path: (1) resolve_multiplex_mode(config) settles the injected config's flag, but implicit_multiplex_blocker() → build_migration_plan() re-reads each home's normal config, not this launch config — a duplicate-credential alt config passes the guard, then gets parked at startup. (2) This write runs before the duplicate-instance guard at run.py:5273 and the PID claim at run.py:5339, so a refused second start re-stamps the live gateway's status record. Fix both together: pass the launch config into preflight, and persist the decision only after the PID claim.
| gateway serves one profile while the default says multiplex; any other verdict clears the field.""" | ||
| try: | ||
| from gateway.status import write_runtime_status | ||
| write_runtime_status(multiplex_standalone_reason=decision.reason if decision.source == "guard" else None) |
There was a problem hiding this comment.
This writes only multiplex_standalone_reason; served_profiles lands much later (after the adapter connect loop, run_adapters.py:899). During the first-ever multiplex boot, every other process reads "not multiplexing" — a secondary started in the window isn't refused, a cron fire targets a dead port. Consider writing served_profiles (or [] for guard refusals) here too.
| except Exception: | ||
| return | ||
| if reason: | ||
| print(f"⚠ Serving the default profile only (gateway.multiplex_profiles unset): {reason}") |
There was a problem hiding this comment.
On a named-profile gateway this prints "⚠ Serving the default profile only … this is profile 'coder's own gateway" — self-contradictory — and the remedy line lacks the -p <name> qualifier a secondary needs. The named-profile refusal is designed topology but logs at WARNING every boot; INFO (like SINGLE_PROFILE_REASON) + a profile-aware headline would read better.
| # it runs, else the DEFAULT profile's explicit config (plus the process-wide | ||
| # GATEWAY_MULTIPLEX_PROFILES override). An unset flag is decided by the gateway, not guessed here. | ||
| from hermes_cli.gateway_multiplex_mode import default_gateway_multiplexes | ||
| if not default_gateway_multiplexes(): |
There was a problem hiding this comment.
The new fallback reads raw user YAML only, while the dashboard previously used managed-aware load_gateway_config(). User true + managed false → helper answers True, gateway answers False, and the dashboard can reject a valid standalone listener config. A presence-preserving effective read that applies the managed overlay (without merging schema defaults) would keep both consistent.
gateway.multiplex_profilesnow defaults to on, gated by a boot-time serve guard so no existing install changes behaviour by surprise.Why a guard and not just a flip
profiles_to_serve(multiplex=True)serves default + every liveprofiles/*unconditionally. Only the migration CLI checked for a secondary that already runs its own standalone gateway, or two profiles sharing a bot token. A bare default flip would have bypassed those checks: on the next restart the default gateway grabs every profile's tokens while the standalone gateways still hold them.What this does
DEFAULT_CONFIG.gateway.multiplex_profiles = True.GatewayConfig.multiplex_profilesbecomesOptional[bool],None= unset; every reader tests truthiness so an undecided flag never multiplexes by accident.hermes_cli/gateway_multiplex_mode.py::resolve_multiplex_modesettles an unset flag once per boot (fromload_gateway_config_for_runnerandgateway run --config) by running the same preflight ashermes gateway migrate --multiplex: default profile, ≥2 profiles, migratable host, no secondary with a live pid or installed unit, no duplicate-credential / port-binder blocker. Refusal → gateway comes up standalone exactly as before, one WARNING naming the blocker and the migrate one-liner. Explicit values (config.yaml,GATEWAY_MULTIPLEX_PROFILES,--standalone) pass through verbatim; an on-diskfalseis honoured as a deliberate rollback, no config migration.gateway_state.jsongetsmultiplex_standalone_reason;hermes gateway statusprints "⚠ Serving the default profile only (gateway.multiplex_profiles unset): " with both remedies. Cleared by a boot that multiplexes.named_profile_served_by_running_multiplexer, the enroll warning, the dashboard listener guard, the cron-fire port resolver,container_bootandgateway_migrate._read_multiplex_flagread the live gateway'sserved_profilesrecord first and the explicit flag second, so a per-profile fleet with the flag unset still reads "not yet multiplexed" and thehermes updateauto-fold proceeds.Verification
tests/hermes_cli/test_gateway_multiplex_mode.py: quiet fleet → on; live secondary / installed unit / duplicate credential / single profile → guard refusal; explicit true/false/env untouched; migration plan not short-circuited; live record outranks raw flag; guard verdict recorded in runtime status and cleared on default.True;codergateway live (via the migrate seam) →False+ warning,hermes -p coder gateway runnot refused, migration plan eligible; explicit true/false honoured; single profile →False.tests/gateway/test_status.py+ phase0 + the new file: 98 passed.check_compat_pointersclean.Docs
multi-profile-gateways.md(default-on section, "Pinning the flag"),multiplexing-gateway.md,hermes_cli/AGENTS.md.Real-install probe
Maintainer's 11-profile install moves to this head after merge (
hermes update+ gateway restart) as the real-home soak; result goes to #109417.Tracking: #109417. Follows #112651 (migration P1s), #112685 (scope P1s), #112653 (lifecycle).