Skip to content

fix(serve): announce READY sentinel on fd 1, not the redirected sys.stdout - #96284

Closed
kitsonk wants to merge 1 commit into
NousResearch:mainfrom
kitsonk:fix/serve-ready-sentinel-on-stdout
Closed

fix(serve): announce READY sentinel on fd 1, not the redirected sys.stdout#96284
kitsonk wants to merge 1 commit into
NousResearch:mainfrom
kitsonk:fix/serve-ready-sentinel-on-stdout

Conversation

@kitsonk

@kitsonk kitsonk commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #96282.

Since 6d4e851 (#94724 item 2) the serve startup path imports
tui_gateway.server (for the flush-on-SIGTERM handlers) before printing
the HERMES_(BACKEND|DASHBOARD)_READY port=<n> sentinel. That module
redirects sys.stdout to sys.stderr at import time (to keep stray prints
off the JSON-RPC protocol stream), so the sentinel now lands on stderr
while the Electron desktop spawn (apps/desktop/electron/backend-ready.ts,
waitForDashboardPort) watches child.stdout only.

Result: the backend is healthy and announces its port in ~2s, but the desktop
times out after 90s ("Timed out waiting for Hermes backend port announcement"),
SIGTERMs a perfectly good backend, and the renderer loops
refreshProfiles failed after 3 attempt(s).

Repro on current main:

$ python -m hermes_cli.main --profile default serve --host 127.0.0.1 --port 0 > out.log 2> err.log
$ cat out.log   # empty
$ cat err.log
HERMES_BACKEND_READY port=62788
  Hermes backend listening on 127.0.0.1:62788

Fix

Write the sentinel to the real stdout file descriptor — fd 1 is the pipe the
desktop reads and is untouched by the Python-level sys.stdout redirect —
with a print() fallback for exotic environments where fd 1 isn't writable
(e.g. closed):

os.write(1, f"{ready_token} port={actual_port}\n".encode())

Test plan

  • New regression test test_ready_sentinel_arrives_on_stdout_not_stderr
    in tests/hermes_cli/test_serve_port_in_use.py — spawns a real serve
    with stdout/stderr captured separately and asserts the sentinel
    arrives on stdout. The existing E2E tests merge stderr into stdout,
    which is exactly how the regression slipped past CI.
  • Verified the new test fails on unfixed main (reproduces the
    desktop's 90s-timeout failure mode) and passes with this patch.
  • Full tests/hermes_cli/test_serve_port_in_use.py suite passes with the
    patch (existing conflict/ephemeral-port contracts untouched).
  • End-to-end on macOS: desktop-spawned backend now announces on stdout
    and the desktop connects (reporter-verified).

…tdout

Since 6d4e851 the serve startup path imports tui_gateway.server (for the
flush-on-SIGTERM handlers) before the READY sentinel is printed. That module
redirects sys.stdout to sys.stderr at import time, so the
HERMES_(BACKEND|DASHBOARD)_READY port=<n> sentinel landed on stderr while the
Electron desktop spawn watches child.stdout only — the desktop timed out
after 90s and killed a perfectly healthy backend (issue NousResearch#96282).

Write the sentinel to the real stdout file descriptor (fd 1 is untouched by
the Python-level redirect), with a print() fallback.

Adds a regression test that captures stdout/stderr separately — the existing
E2E suite merges them, which is exactly how this slipped past CI.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P1 High — major feature broken, no workaround duplicate This issue or pull request already exists labels Aug 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #92631. Both patches repair the same READY-sentinel stdout routing regression that makes Desktop boot time out against a healthy backend.

@Forser

Forser commented Aug 27, 2026

Copy link
Copy Markdown

Local validation against current origin/main completed — no remote changes made.

PR head:
cd2c5bcd97defd9a7f2a82556a781c9b7c8fa09f

Current fetched origin/main:
a588685fb957a0c55de84d5bfba69ff44ebfb31b

Merge compatibility:
git merge-tree --write-tree origin/main refs/remotes/origin/pr/96284
completed cleanly with no conflicts.

I reproduced the failure on unfixed current main using the real hermes_cli.web_server.start_server(...) path with stdout/stderr captured separately:

stdout: empty
stderr:
HERMES_BACKEND_READY port=40145
Hermes backend listening on 127.0.0.1:40145

With PR #96284:

stdout:
HERMES_BACKEND_READY port=40625

stderr:
Hermes backend listening on 127.0.0.1:40625

This confirms the PR’s root-cause analysis: Python-level sys.stdout has been redirected to stderr, while Desktop watches the child’s actual stdout pipe. Writing READY to fd 1 restores the handshake contract.

Test results:

Base:
tests/hermes_cli/test_serve_port_in_use.py
8 passed

PR:
tests/hermes_cli/test_serve_port_in_use.py
9 passed

Additional:
tests/test_web_server.py
tests/tui_gateway/test_serve_exit_flush.py
12 passed, 3 skipped

python -m compileall -q hermes_cli/web_server.py
passed

git diff --check
passed

One separate nonblocking finding: BACKEND_PORT_IN_USE still goes to stderr on both base and the PR. That appears to be affected by the same stdout-redirection issue, but it is separate from the READY regression fixed here.

Full Electron launch validation was not completed because the local Linux Electron install is blocked by the Chrome SUID sandbox configuration (chrome-sandbox is not root-owned/mode 4755). I did not change system permissions or bypass the sandbox.

So: the READY handshake fix is validated at the real subprocess/stdout boundary, but full packaged-Desktop launch validation was not performed on this machine.

@KeyArgo

KeyArgo commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Corroboration: this same root cause was just filed separately as #96295 on Windows 11 (backend healthy, HERMES_BACKEND_READY present, but desktop stuck on 'Connecting…' and 'Timed out waiting for Hermes backend port announcement (90000ms)' after updating to v0.20.5). The fd-1 sentinel fix here resolves that report too, since the regression is cross-platform (the stdout redirect is platform-independent; the Windows desktop reads the same child.stdout). The separate-stdout/stderr regression test in this PR is exactly what would have caught the slip. Thanks for the clean minimal fix.

@Paramon

Paramon commented Aug 27, 2026

Copy link
Copy Markdown

Independently hit this on macOS 26.5.2 (arm64) right after today's v0.20.5 update — four 90s boot-timeout loops in desktop.log while every backend announced READY within ~1.2s (full evidence in #96282 (comment)).

Verified the stream split locally: serve with separated pipes puts the sentinel on stderr and leaves stdout empty; after redirecting the sentinel back to real stdout the desktop boots in ~2s.

This os.write(1, ...) variant is the right call over sys.__stdout__ — it also covers the pythonw.exe case where __stdout__ is None, and the OSError fallback keeps non-desktop launches intact. The separated-streams regression test closes the exact gap (stderr=subprocess.STDOUT masking) that let 6d4e851 slip through CI. LGTM from the affected-user side.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #96311 — your commit cherry-picked with authorship preserved (rebase-merge, commit c211a00 on main). Thanks for the rapid RCA + the split-stream regression test that closes the CI blind spot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop boot times out: HERMES_BACKEND_READY sentinel printed to stderr after 6d4e851d8 (tui_gateway.server stdout redirect)

6 participants