Skip to content

fix(container): s6 reconciler honors config.yaml multiplex_profiles (#85413) - #86114

Closed
a-yeyang wants to merge 1 commit into
NousResearch:mainfrom
a-yeyang:fix/s6-reconciler-config-multiplex
Closed

a-yeyang wants to merge 1 commit into
NousResearch:mainfrom
a-yeyang:fix/s6-reconciler-config-multiplex

Conversation

@a-yeyang

Copy link
Copy Markdown

Summary

Fixes #85413.

The container-boot s6 reconciler (hermes_cli/container_boot.py:reconcile_profile_gateways) decided whether to auto-start named per-profile gateway slots by reading multiplex routing only from the GATEWAY_MULTIPLEX_PROFILES environment variable:

multiplex_profiles = is_truthy_value(os.environ.get("GATEWAY_MULTIPLEX_PROFILES"))
...
should_start = (not multiplex_profiles and prior_state in _AUTOSTART_STATES)

A user who enabled multiplexing exactly as documented — setting multiplex_profiles: true in config.yaml alone, without also exporting the env var — got every named profile slot auto-started by s6. Each booted, hit the default multiplexer's double-bind guard (✗ The default gateway is running as a profile multiplexer and already serves profile '<name>'), exited, and was restarted in a loop, pegging a CPU core near 100%.

The gateway process itself already reads the documented precedence (env override → config.yaml → default) in gateway/config.py; only the boot-time reconciler diverged.

Fix

Resolve the effective flag through the same precedence the runtime uses, via a new _multiplex_profiles_enabled(hermes_home) helper that reuses gateway.config._env_multiplex_profiles_override():

  1. GATEWAY_MULTIPLEX_PROFILES truthy/falsy wins (operator override).
  2. A blank or unrecognized env value is treated as unset (not False), so a provisioned-but-empty Fly/Docker secret cannot silently shadow a config.yaml opt-in and reintroduce the crash-loop.
  3. config.yaml — both the top-level multiplex_profiles key and the nested gateway.multiplex_profiles form (written by hermes config set gateway.multiplex_profiles true) are honored.
  4. Default False.

Config resolution is fail-open: any error reading config.yaml falls back to disabled rather than wedging container boot (the gateway process surfaces the real config error later).

The change is scoped to the reconciler's read of the flag; the existing should_start logic, default-slot handling, and stale-runtime sweeping are untouched.

Behavior change

Env var config.yaml Before After
unset multiplex_profiles: true named slots auto-started → crash-loop named slots registered down ✅
unset nested gateway.multiplex_profiles: true auto-started → crash-loop registered down ✅
unset absent auto-started (correct) auto-started (unchanged)
true any multiplex (correct) multiplex (unchanged)
false true no-multiplex no-multiplex (env wins, unchanged)
"" (blank) true no-multiplex (bug: empty secret shadowed opt-in) multiplex (config opt-in stands) ✅

Tests

tests/hermes_cli/test_container_boot.py — added:

  • test_config_only_multiplex_registers_named_slot_down — the core [Bug]: s6 boot reconciler auto-starts per-profile gateways (crash-loop, 100% CPU) when multiplex_profiles enabled via config.yaml only #85413 repro: config-only opt-in keeps named slots down.
  • test_config_only_nested_gateway_multiplex_honored — nested gateway.multiplex_profiles form.
  • test_multiplex_disabled_by_default_autostarts_named_slot — no regression for the common non-multiplex deploy.
  • test_env_var_still_wins_over_config — operator override preserved.
  • test_env_var_false_overrides_config_true — env > config precedence.
  • test_blank_env_var_does_not_shadow_config_optin — empty secret falls through to config.
  • test_malformed_config_yaml_fails_open_to_disabled — malformed config never wedges boot.
  • test_multiplex_profiles_enabled_helper_precedence — direct unit coverage of the resolver.

All 13 tests in the file pass locally.

🤖 Generated with Claude Code

…ousResearch#85413)

The cont-init.d s6 reconciler decided whether to auto-start named
per-profile gateway slots by reading multiplex routing ONLY from the
GATEWAY_MULTIPLEX_PROFILES environment variable. A user who enabled
multiplexing exactly as documented — setting `multiplex_profiles: true`
in config.yaml alone, without also exporting the env var — got every
named profile slot auto-started by s6. Each booted, hit the default
multiplexer's double-bind guard ("already serves profile ..."), exited,
and was restarted in a loop, pegging a CPU core near 100%.

Resolve the effective flag through the same env-override -> config.yaml
-> default precedence the gateway process itself uses
(gateway/config.py), via a new `_multiplex_profiles_enabled(hermes_home)`
helper:

- GATEWAY_MULTIPLEX_PROFILES truthy/falsy wins (operator override).
- A blank/unrecognized env value is treated as unset (not False), so a
  provisioned-but-empty secret cannot silently shadow a config opt-in.
- Both the top-level `multiplex_profiles` key and the nested
  `gateway.multiplex_profiles` form are honored.
- Config resolution is fail-open: a malformed config.yaml falls back to
  disabled rather than wedging container boot.

Adds tests covering config-only opt-in (top-level and nested), env>config
precedence, blank-env fallthrough, malformed-config fail-open, and the
unchanged default-autostart path.

Co-Authored-By: Claude <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard backend/docker Docker container execution area/docker Docker image, Compose, packaging area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 14, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Related: #85437 fixes #85413 through the shared gateway config loader; this PR implements the same precedence explicitly. Maintainers should choose one approach.

@a-yeyang

Copy link
Copy Markdown
Author

Noting the related PR #85437 here so maintainers can compare the two approaches side-by-side.

#85437 fixes the same issue by importing load_gateway_config() inside reconcile_profile_gateways. That is simpler (2 lines) but introduces a runtime dependency of gateway.config into the boot reconciler. This PR instead keeps the reconciler self-contained by adding a small _multiplex_profiles_enabled() helper that replicates the same env > config.yaml > default precedence without pulling in gateway config loading.

Both approaches solve the duplicate-bind restart loop. The trade-offs:

#85437 #86114
Lines changed ~2 ~40 (helper + call site)
Runtime deps added to boot reconciler gateway.config none
Precedence logic reuses load_gateway_config() explicit, mirrors gateway behavior
Tests added 2 8 (covers env override, blank env, nested gateway: section, malformed yaml fail-open)
Risk any gateway.config import side-effect or heavy load at boot slight duplication of precedence logic

I am happy to close this in favor of #85437 if maintainers prefer the shorter diff, or keep this if the additional test coverage and lack of boot-time coupling to gateway.config are preferred. Either way the user-visible bug is the same.

@Enough1122

Copy link
Copy Markdown

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

fix(container): s6 reconciler honors config.yaml multiplex_profiles (#85413)

  1. hermes_cli/container_boot.py:_multiplex_profiles_enabled duplicates the env → config → default precedence implemented in gateway/config.py instead of reusing it. The docstring explicitly says it mirrors the runtime, but the two can drift: a future change to the gateway loader (a new env alias, another config key, profile-scoped overrides) would silently leave the s6 reconciler behind. A parity test that feeds identical fixtures to both resolvers and asserts equal results would lock them together.

  2. The helper imports the private _env_multiplex_profiles_override from gateway.config. Depending on a private symbol from the boot path couples s6 boot to the internal shape of the gateway module; if that helper is renamed or refactored, boot-time resolution breaks. A stable public accessor would be more robust.

  3. Fail-open on an unrecognized value: a typo like miltiplex_profiles: true (top-level key not found, nested section absent) silently resolves to "multiplexing disabled" and auto-starts named slots — the same condition that produced the [Bug]: s6 boot reconciler auto-starts per-profile gateways (crash-loop, 100% CPU) when multiplex_profiles enabled via config.yaml only #85413 crash loop, only now via a config typo instead of a missing env var. The warning path only fires on read errors; consider also warning when the key exists but has an unexpected type/value so a typo is not silent.

@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.
You are credited via Co-authored-by / in the PR body of #101242 as noted there. Your diagnosis/RCA is credited in #101242; the residual gap noted there is what the merged change fixes.

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/config Config system, migrations, profiles area/docker Docker image, Compose, packaging backend/docker Docker container execution comp/cli CLI entry point, hermes_cli/, setup wizard needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: s6 boot reconciler auto-starts per-profile gateways (crash-loop, 100% CPU) when multiplex_profiles enabled via config.yaml only

4 participants