[sonichi]

What

Fixes a real, host-observable false-failure in tests/runtime-health.test.py introduced by #4365.

Before/after evidence

Before (this branch's HEAD reverted to the pre-fix version of just this file, via git show <parent>:tests/runtime-health.test.py, run on this live host with a genuinely fresh core heartbeat):

  FAIL real _core_running: false on bogus socket
  ok   real _gateway_running returns a bool (configured host)
  ok   _gateway_running: unconfigured host -> None
  ok   real _pane_text returns a str
  ok   real _resolve_workspace returns a path
  FAIL real derive: offline on bogus socket
  ...
FAIL — 2 failing

After (this PR's actual diff):

  ok   real _core_running: false on bogus socket
  ok   real _gateway_running returns a bool (configured host)
  ok   _gateway_running: unconfigured host -> None
  ok   real _pane_text returns a str
  ok   real _resolve_workspace returns a path
  ok   real derive: offline on bogus socket
  ...
PASS — runtime-health green

Why

#4365 (fix(runtime-health): probe the socket the core launched on, not a guessed default) changed _core_running() to resolve its tmux socket via the new _tmux_socket() helper, which — when SUTANDO_TMUX_SOCKET is unset — prefers the socket core_heartbeat.py recorded in a fresh <workspace>/state/cores/<host>.alive file, falling back to the module-level TMUX_SOCKET constant only when that record is absent, stale, or unreadable.

tests/runtime-health.test.py's "exercise the real probes in-process" section (section 6) injects its "no session" precondition by patching rh.TMUX_SOCKET directly. That was sufficient before #4365, when _core_running() read the constant directly. After #4365, on any host with a genuinely live core (a fresh .alive heartbeat file — which is the normal, common case for anyone actually running this test from inside a live session) _tmux_socket() ignores the patched constant entirely, resolves the real socket, finds a real session, and the two "bogus socket" assertions fail against a truthfully-running core.

I hit this directly: pulled #4365 into a live core's checkout, ran the full test suite as routine post-pull hygiene, and got 2 failures that reproduced identically before this fix and cleared after it, on the same host, same process.

Fix

Patch rh._tmux_socket itself (the function), in addition to the existing rh.TMUX_SOCKET patch (kept, since other code may still read the constant directly), and restore both in the finally block.

Testing

• python3 tests/runtime-health.test.py — PASS (was 2 failing before this fix, on this exact host)
• Mutation-tested: swapped in the pre-this-fix version of just this file (via git show <parent-sha>:tests/runtime-health.test.py, never git checkout), confirmed the same 2 failures reproduce identically, restored byte-identical, re-confirmed green.
• bash -n, git diff --check, scripts/review-checks.sh — all pass.

Single concern: this only touches the test's mock setup, not src/runtime-health.py itself.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 89%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

[qingyun-wu]

The test correction is narrowly right: _core_running() resolves its socket through _tmux_socket(), so replacing only TMUX_SOCKET left the in-process check exposed to a live host heartbeat record. Mocking and restoring the resolver itself makes the bogus-socket assertion hermetic. The 50-case runtime-health suite passes locally, and the non-Windows hosted suites are green.

The Windows job is red only on the pre-existing core-heartbeat.test.py/fcntl import problem; this one-file test change does not touch that path.

Reviewed by Qingyun's Personal Codex.

[keweichen]

REQUEST_CHANGES — exact head df98b77eb99130c0ae5b8afcfd2238a1577d0f6b is not ready to merge.

Blocker: tests/runtime-health.test.py:339-341 still assumes _core_running() delegates with rh.TMUX_SOCKET. Production delegates with _tmux_socket() (src/runtime-health.py:228-231), and this patch restores the resolver at line 294 before that assertion. A detached caller with no SUTANDO_TMUX_SOCKET and a fresh private-socket heartbeat therefore still gets a host-dependent false failure.

Controlled fresh-heartbeat repro against the production resolver:

parent-style section 6: socket=/tmp/private-live.sock core_running=True
HEAD section 6:         socket=/tmp/bogus.sock core_running=False
existing final assertion: actual socket=/tmp/private-live.sock
existing final expected:  socket=/tmp/default.sock
existing final assertion passes=False

This confirms both that the defect exists on the merge-base/current-main version and that the new section-6 mock fixes its two targeted assertions, but the later delegation assertion remains broken for the adjacent private-socket case. To unblock, pin _tmux_socket in that delegation test too, assert the injected socket, restore it in finally, and rerun with an unset environment plus a fresh private-socket heartbeat.

Worst-case user disruption is test-only: detached/private-socket installs can still report a false suite failure and block routine upgrade validation. Isolating every socket-dependent assertion from host heartbeat state mitigates it; no runtime behavior or state schema changes here.

Verification:

• Topic is one commit from merge-base e23d860591dc46dec40795f43b3d066c504f635a; no stack. Current origin/main is one unrelated commit ahead.
• git diff --check passed.
• scripts/review-checks.sh passed; a synthetic /Users/reviewer/private control failed as expected. No added host-specific path was found.
• core-health-verdict, runtime-health atomic-write (9 tests), and Station-signal (28 tests) suites passed.
• python3 tests/runtime-health.test.py could not be green in this sandbox because tmux is absent (shutil.which('tmux') = None), producing four existing tri-state None versus offline/false failures; the controlled resolver harness above supplies the discriminating before/after evidence.
• The adjacent core-heartbeat suite ran 26 tests with three writer-handoff failures in files unchanged by this topic; those are not the blocker above.
• The supplied status snapshot also has PowerShell + Windows runtime red. Network access was prohibited, so I could not inspect that job log; it remains a required merge-gate condition.

Reviewed by Kewei's Codex automation.

[sonichi]

Real finding, thanks — fixed. Pushed 09af1a8f2.

You're right that section 6's fix was incomplete. There's a third, separate assertion (section 7, _core_running: delegates to tmux_probe.has_session(...)) that also compares against the bare rh.TMUX_SOCKET constant, and it runs after section 6's finally block has already restored the real _tmux_socket. Reproduced your exact scenario directly — built the adversarial condition (fresh .alive heartbeat recording a different socket, SUTANDO_TMUX_SOCKET unset) and ran the assertion's logic in isolation:

PARENT (pre-this-fix) delegation assertion under adversarial condition: False
FIXED delegation assertion under the same condition: True

Fixed the same way as section 6: pin rh._tmux_socket to an injected value for this assertion too, assert against the injected value (not the constant), restore in the finally.

Also confirmed the 3 suites you named: core-health-verdict all-pass, runtime-health-atomic-writes 9/9, runtime-health-ag2space-station 28/28 (matches your count). core-heartbeat.test.py runs clean here (26/26) — the "writer-handoff failures" you saw must be sandbox-specific, as you suspected.

On the Windows job: confirmed it's unrelated to this fix — core-heartbeat.test.py failed there, which is main's still-open import fcntl bug (#4371, in review right now, not yet merged). This branch forked before that lands, so it inherits the red until #4371 merges and this rebases. Not something to fix here.

[qingyun-wu]

The follow-up correctly applies the same hermetic socket-resolver seam to the delegation assertion: _core_running() is now verified against an injected _tmux_socket() result rather than whichever live heartbeat record happens to exist on the host. The full runtime-health suite passes locally on 09af1a8f2. No remaining findings.

Reviewed by Qingyun's Personal Codex.

[github-actions]

@cla-assistant check

[keweichen]

APPROVE — exact head 2fe9d5e75458f2dbf5de8e3bbc281f5a73d89af2 has no implementation blocker and is ready to merge once required hosted checks are green and the two-maintainer gate is met. The prior REQUEST_CHANGES on df98b77e is resolved.

The defect exists on current origin/main (merge-base = 186c5ba4): _core_running() delegates through _tmux_socket() at src/runtime-health.py:228-231, and that resolver prefers a fresh heartbeat socket at lines 43-69. Replacing only rh.TMUX_SOCKET therefore does not establish the bogus-socket precondition. Current head fixes both affected sites: section 6 replaces and restores the resolver at tests/runtime-health.test.py:262-294, while the later delegation assertion independently injects, asserts, and restores it at lines 336-348.

Controlled fresh-heartbeat before/after evidence:

origin/main section 6: socket=/tmp/review-private-live.sock core_running=True passes=False
HEAD section 6: socket=/tmp/review-bogus.sock core_running=False health=offline passes=True
origin/main delegation: actual=/tmp/review-private-live.sock expected=/tmp/review-default.sock passes=False
HEAD delegation: actual=/tmp/review-delegation.sock expected=/tmp/review-delegation.sock passes=True

The harness also confirmed the original resolver identity was restored after both HEAD paths. The production runtime-health.py, tmux_probe.py, and core_heartbeat.py blobs are identical between origin/main and HEAD; the topic diff is one test file, +13/-3, with no stack. The merge commit did not alter the fixed test blob from 09af1a8f.

Verification:

• tests/tmux-probe.test.py: 19/19 passed.
• tests/core-health-verdict.test.py: all passed.
• tests/runtime-health-atomic-writes.test.py: 9/9 passed.
• tests/runtime-health-ag2space-station.test.py: 28/28 passed.
• python3 tests/runtime-health.test.py reached both changed assertions but reported four probe failures because this sandbox has no tmux (shutil.which('tmux') = None); the controlled harness above supplies the discriminating live/private-socket and bogus-socket observations.
• The broader unchanged suites had one core-input-watch no-tmux failure and three core-heartbeat writer-handoff failures; their relevant files are byte-identical to origin/main, so they are not caused by this topic.
• git diff --check and scripts/review-checks.sh passed. A synthetic /Users/reviewer/private control was rejected, proving the path scan fires. Added absolute paths are PID-scoped /tmp test fixtures only.

Worst-case disruption is test-only contamination from a leaked process-global resolver mock. Both changed regions restore the original in finally, and the harness confirmed restoration; no production path, schema, startup, bridge, or delivery behavior changes.

At review time the supplied current-head snapshot still had hosted jobs in progress. I could not inspect live CI under the no-network review scope, so required CI/CLA and two current-head maintainer approvals remain merge conditions.

Reviewed by Kewei's Codex automation.