Skip to content

fix(cron): use shared listener port for multiplex fires - #84755

Open
bergusdz wants to merge 1 commit into
NousResearch:mainfrom
bergusdz:codex/fix-multiplex-cron-listener-port
Open

fix(cron): use shared listener port for multiplex fires#84755
bergusdz wants to merge 1 commit into
NousResearch:mainfrom
bergusdz:codex/fix-multiplex-cron-listener-port

Conversation

@bergusdz

Copy link
Copy Markdown

What does this PR do?

Managed Chronos fires for a secondary profile now use the default profile's shared api_server listener port when gateway multiplexing is enabled, while retaining the target profile in the /p/<profile>/ route.

_gateway_fire_endpoint() previously resolved the target profile's port first and detected multiplexing afterward. A secondary profile with API_SERVER_PORT=8701 therefore produced 127.0.0.1:8701/p/worker_alpha/..., even though multiplex architecture gives the literal default profile sole ownership of the listener (for example port 8642). The dashboard then received 503 and Chronos retried instead of executing the job.

Related Issue

Focused follow-up regression in #84339; no separate issue.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Tests (adding or improving test coverage)

Changes Made

  • hermes_cli/web_server.py: resolve multiplex mode first through canonical load_gateway_config() under literal-default profile scope, select the listener owner, then apply existing config -> environment -> 8642 port precedence.
  • tests/hermes_cli/test_cron_fire_dashboard.py: cover both process-env multiplexing and an active-secondary dashboard where multiplexing exists only in the literal default config.

Non-multiplex per-profile ports, the default bare route, callback authentication, forwarding, retry behavior, and cron storage are unchanged.

How to Test

  1. Negative control on current main: target profile .env port 8701 wins under multiplex instead of default listener port 8642 (1 failed).
  2. Active-secondary negative control: default config enables multiplexing but current code emits a bare URL instead of /p/worker_alpha/... (1 failed).
  3. python -m pytest tests/hermes_cli/test_cron_fire_dashboard.py tests/gateway/test_cron_fire_webhook.py tests/gateway/test_multiplex_api_server_routing.py -q -> 25 passed.
  4. python -m ruff check hermes_cli/web_server.py tests/hermes_cli/test_cron_fire_dashboard.py -> clean.

Duplicate and intent check

Checklist

Code

  • I've read the Contributing Guide
  • Commit uses Conventional Commits
  • Searched open and merged PRs/issues for duplicates
  • PR contains one focused change
  • Full pytest tests/ -q (targeted and neighboring suites above were run)
  • Added behavior regression tests
  • Tested on Windows 11

Documentation & Housekeeping

  • Documentation update: N/A (fixes implementation to match documented multiplex contract)
  • cli-config.yaml.example: N/A
  • CONTRIBUTING.md / AGENTS.md: N/A
  • Cross-platform impact considered; URL/config selection is platform-independent
  • Tool schema update: N/A

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 12, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

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

Observations:

  • The GATEWAY_MULTIPLEX_PROFILES env-var override was removed; multiplex is now derived only from the default profile's gateway.multiplex_profiles config. Deployments that toggled multiplex via the env var (on a worker profile or the process) will silently fall back to the target-profile port and fire cron at the wrong listener. If the env flag is still honored by gateway/config.py for the gateway itself, keep reading it here for parity, or call out the migration.
  • The whole multiplex-detection block is wrapped in try/except Exception: pass — if load_gateway_config() fails (e.g. malformed default profile config), the function silently uses the target-profile port, which is the wrong listener in multiplex mode. At least log a warning so the mismatch is diagnosable.
  • Test note: test_fire_endpoint_multiplex_uses_literal_default_from_secondary_process mocks _resolve_profile_dir to return the default home for every profile, so the real _config_profile_scope("default") home resolution from a worker process isn't exercised end-to-end — fine for a unit test, but the scope-switch logic is the riskiest part of this change and only has indirect coverage.

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/profiles Multi-profile isolation, HERMES_HOME scoping comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management 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.

3 participants