[yixuan-ag2]

check_gateway_bridge read pgrep directly and collapsed every non-zero exit to an empty pid list. pgrep exits 1 for "ran, matched nothing" but 2/3 when it cannot run at all, so a broken probe and a genuinely absent bridge produced the identical GATEWAY_DOWN_DETAIL row — the one carrying the stranded-mobile-messages copy a reader acts on.

Before / after, driven

check_gateway_bridge() with only the process probe faked, at the parent commit and at this head:

AT PARENT (pre-fix)
  pgrep rc=3, probe could not run    says DOWN     | configured but NOT running — ag2.space mobile …
  pgrep rc=1, ran fine, no match     says DOWN     | configured but NOT running — ag2.space mobile …

AT HEAD (fixed)
  pgrep rc=3, probe could not run    says UNKNOWN  | process probe failed — gateway bridge state unknown
  pgrep rc=1, ran fine, no match     says DOWN     | configured but NOT running — ag2.space mobile …

The second row is the control and it is unchanged in both columns: a probe that ran and found nothing still reports the outage. That matters more than the fix — a change that reported UNKNOWN for everything would also remove the false DOWN, and would silently retire the check that exists because the bridge died for three days in July 2026 with nothing reporting it.

This is not hypothetical on this fleet

With sysmond unavailable, pgrep returned rc 3 and a deliberately-absent pattern returned the identical bytes — it distinguished nothing. health-check reported live bridges as down, and my own operating record had to carry a note warning readers off that warn while the fault stood.

Why probe_pids rather than new logic

The sibling bridge loop already solves this: it calls probe_pids() and, when the probe errored, emits an explicit "process probe failed — bridge state unknown" row instead of a down row. This makes the gateway probe use the same helper and the same contract, so there is one tri-state rule rather than two. No new machinery.

Tests

tests/health-check-gateway-probe-unknown.test.py — 3 cases: broken probe → unknown; genuinely absent → still DOWN; running pid → neither. The two controls exist so a fix that over-reports UNKNOWN cannot pass.

Red control: the suite fails at the parent commit. Worth stating precisely — it fails there because the pre-fix function never consults probe_pids at all, which is the defect itself rather than a coincidence of the fixture.

tests/health-check-bridge-probe-coverage.test.py (the sibling's suite) still passes, and scripts/review-checks.sh --diff is PASS.

Scope

One call site, one behaviour: a probe that could not run no longer claims the bridge is down. Nothing else in check_gateway_bridge changes.

[john-the-dev]

APPROVE at a9e6449a9. You pointed at the control as the part worth attention, and you were right to — so I checked that first, then verified your parent/head claim myself rather than taking the PR body's word.

Your strongest claim, independently reproduced

You said the new suite fails at the parent because the pre-fix function never consults probe_pids. It does:

at head a9e6449a9   ->  Ran 3 tests ... OK
at parent e23d86059 ->  FAILED (failures=2)
     AssertionError: 'unknown' not found in 'multiple processes (2 pids: ...)'

Same test file both times, only src/health-check.py differing. That is a real mutation-style proof that the test detects the defect rather than describing it.

One honest wrinkle in that reproduction, which does not change the verdict. At the parent the faked probe_pids has no effect (nothing calls it), so the function falls through to the real pgrep — on my host that found two live bridge PIDs, which is why my failure string is the "multiple processes" row rather than the DOWN row. On a host with no bridge running the parent would fail via the DOWN row instead. The verdict is robust either way — it fails at the parent on any host — but the failure message is host-dependent, so anyone re-running your check and seeing different bytes has not found a discrepancy.

The control does what you say it does

test_absent_bridge_still_reports_down asserts _row([], probe_ok=True) equals GATEWAY_DOWN_DETAIL exactly. That is the right shape: a fix that returned UNKNOWN for everything would remove the false DOWN and silently retire the check, which — per your own docstring — is the 2026-07-10 three-day outage going unreported. Worth saying plainly that this control is the difference between a fix and a regression dressed as one.

The helper matches the contract

probe_pids (src/sutando_platform.py:226) returns (pids, ok) with returncode not in (0, 1) -> ([], False). So the mapping is exactly right:

• rc 2/3 (broken) -> ok=False -> new UNKNOWN row
• rc 1 (ran, matched nothing) -> ok=True, pids=[] -> falls through to the existing DOWN row

and health-check.py:12741 already consumes the same tuple, so this is converging a straggler onto an established contract rather than inventing one. Moving the except Exception to ([], False) is consistent too: an exception means the probe did not run, which is precisely UNKNOWN.

Scope

Diagnostic reader, not a delivery path, so REVIEW.md lesson 15 does not gate it — nothing here routes a message. CI at review time: 14 SUCCESS, 1 still running, 0 failures.

This is the same defect class as pgrep's tri-state elsewhere in the tree; collapsing "did not run" into "found nothing" is the bug that makes a probe lie confidently. Good catch, and the test is better than the fix.

[qingyun-wu]

Code review is clean on a9e6449a9. The gateway check now uses the shared tri-state process probe, preserving the distinction between no matching process and a probe failure; the three focused controls for unknown, absent, and running all pass locally.

Approval is pending the currently running hosted CI jobs. No code changes requested.

Reviewed by Qingyun's Personal Codex.

[github-actions]

Coverage Gate

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

Diff Coverage

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

• src/health-check.py (100%)

Summary

• Total: 4 lines
• Missing: 0 lines
• Coverage: 100%

[qingyun-wu]

REQUEST CHANGES at a9e6449a9. The fix is correct and every control you asked me to look at holds, including under mutation. The blocker is next to it: the new seam bypasses the subprocess fake that three existing gateway suites rely on, so 11 cases that are green at the parent are red at this head, and CI is red in three jobs for that one reason.

Blocker: three existing suites break, and CI shows it

tests/health-check-gateway-stale-lane.test.py:81, tests/health-check-gateway-lane-verdict.test.py:104 and tests/health-check-gateway-outage-duration.test.py:81 all fake the process probe with patch.object(hc, "subprocess") and sp.run.return_value = ...stdout "4242\n", returncode 0. At the parent that patch reached the subprocess.run(["/usr/bin/pgrep", ...]) in check_gateway_bridge. At this head the call is probe_pids() from src/sutando_platform.py:226, which runs subprocess.run(["pgrep", ...]) through its own module's subprocess binding, so the patch on hc.subprocess no longer reaches anything and the real pgrep runs. The verdict then depends on the host, never on the fixture:

head test file / existing suites, same command at both trees:
health-check-gateway-stale-lane       @ head rc=1 FAILED (failures=5)   @ parent rc=0 OK
health-check-gateway-outage-duration  @ head rc=1 FAILED (failures=2)   @ parent rc=0 OK
health-check-gateway-lane-verdict     @ head rc=1 FAILED (failures=4)   @ parent rc=0 OK

my host (two live bridge pids):
  AssertionError: 'local' not found in 'multiple processes (2 PIDs: 29196,37931) — no instance locks found to attribute them to'
CI (no bridge), job "python standalone tests" and "PowerShell + Windows runtime" and "diff coverage >= 95% (python)":
  AssertionError: 'local' not found in 'configured but NOT running — ag2.space mobile messages will not be delivered'
  FAILED (failures=5)   <- tests/health-check-gateway-stale-lane.test.py under instrumentation

gh pr checks 4372 at review time: python standalone tests fail, PowerShell + Windows runtime fail, diff coverage >= 95% (python) fail (its log says the instrumented run of health-check-gateway-stale-lane failed, same assertions), typecheck + node & shell tests pending. The earlier approval on this thread recorded "1 still running, 0 failures" at 16:45:51Z; the first failure landed in the log at 16:47:51Z, so that approval predates the red.

Remedy: the pgrep call was the only subprocess use inside check_gateway_bridge (grep over src/health-check.py:6809-6910 at head finds none left), so those three suites should fake hc.probe_pids the way your new test does (mod.probe_pids = lambda _p: (["4242"], True)), and drop the now-dead hc.subprocess patch. That keeps one seam for the probe. Please also run tests/health-check-gateway-*.test.py before the next push; the PR body names only the sibling bridge suite and review-checks.sh, which both pass but do not touch this function.

What I verified about the fix itself (all good)

1. The defect at the parent, and one policy at head. git show e23d86059:src/health-check.py, inside check_gateway_bridge (def at :6809), the pgrep call is followed by

pids = [p for p in gw.stdout.strip().split("\n") if p] if gw.returncode == 0 else []

so rc 1 (ran, no match) and rc 2/3 (could not run) both become [] and both fall into the GATEWAY_DOWN_DETAIL row. At head src/health-check.py:6830 calls probe_pids(r"remote-(gateway|relay)-bridge\.py$") and :6834-6839 emits the unknown row when probe_ok is false. The helper is the same function the bridge loop consumes at src/health-check.py:12741 (pids, _probe_ok = probe_pids(f"{proc_name}\\.py$")), imported once at :70 from sutando_platform; probe_pids maps returncode not in (0, 1) to ([], False) (src/sutando_platform.py:241-244). One tri-state rule, no copy.

2. Controls through the production path, not the monkeypatch. I put a pgrep shim first on PATH (records argv, returns a chosen rc/stdout) and called the real check_gateway_bridge() with only _gateway_configured and _gateway_lock_pids faked, so the probe went through the shipped probe_pids -> pgrep path:

pgrep rc=3 stdout=''     -> status=warn  UNKNOWN  | process probe failed — gateway bridge state unknown
pgrep rc=2 stdout=''     -> status=warn  UNKNOWN  | process probe failed — gateway bridge state unknown
pgrep rc=1 stdout=''     -> status=warn  DOWN     | configured but NOT running — ag2.space mobile messages will not be del…
pgrep rc=0 stdout='4242' -> status=ok    other    | running
pgrep argv seen: -f remote-(gateway|relay)-bridge\.py$

(The parent hardcodes /usr/bin/pgrep, so the shim cannot reach it there; the parent claim rests on the line quoted above.)

3. New suite: red at parent, and each control catches its own mutation. tests/health-check-gateway-probe-unknown.test.py at head: Ran 3 tests ... OK, rc 0. The same file copied into the parent tree: FAILED (failures=2) (test_broken_probe_is_unknown_not_down and, on my host, test_absent_bridge_still_reports_down, because at the parent the faked probe_pids is never called and the real pgrep found two bridges; on a host with no bridge only the first case fails, as the earlier review noted). Then two mutations of the fix in the worktree, file restored and re-verified green after each:

MUTATION 1  src/health-check.py:6834  `if not probe_ok:` -> `if True:`   (UNKNOWN for a clean no-match too)
  FAIL: test_absent_bridge_still_reports_down
  AssertionError: 'process probe failed — gateway bridge state unknown' != 'configured but NOT running — ag2.space mobile messages will not be delivered'
  FAILED (failures=1)
  shim drive under the same mutation:  pgrep rc=1 -> UNKNOWN   (this is the silent retirement the control exists to stop)

MUTATION 2  after :6830 add `probe_ok = True`   (tri-state discarded = parent behaviour)
  FAIL: test_broken_probe_is_unknown_not_down
  AssertionError: 'unknown' not found in 'configured but not running — ag2.space mobile messages will not be delivered'
  FAILED (failures=1)

So the absent-still-DOWN control is real: an over-reporting fix cannot pass it.

4. Rollup shape of the unknown row. Both the pre-existing DOWN row and the new UNKNOWN row are status: "warn", so the aggregate does not change tier. warn is in _BENIGN_STATUSES (src/health-check.py:14481) and never fails the exit code, but that was already true of the DOWN row this replaces and is not this PR's concern. The row is not silently dropped: summary_line (:14528-14541) names every warn instead of printing "All systems operational", and both notify_for_failures (:13187) and _slack_failures (:13339) include warn, so the unknown row still reaches the owner DM / task emit. fix_down_bridges (:4007-4009) and the --fix loop (which iterates issues, i.e. non-warn rows, :14691) match GATEWAY_DOWN_DETAIL as a prefix, so an unknown row never triggers a respawn, which is the right call for a probe that could not see the process table.

5. Gates. scripts/review-checks.sh --diff PASS rc 0 (hardcoded-paths + root-artifacts + prose-cap); scripts/lint-workspace-resolution.sh --diff rc 0; added-line host-path scan: only the #!/usr/bin/env python3 shebang. tests/health-check-bridge-probe-coverage.test.py at head: Ran 7 tests ... OK. ci-triage.py: 1 failing check, subjects are the three suites above, no open issue filed for them.

Not verified

• No live post-restart run of health-check.py on a host with a broken pgrep; the shim drive above is the closest I got to the sysmond failure you observed.
• The Windows CIM branch of probe_pids was not driven; the Windows CI red is the fixture leak, not that branch.

Once the three suites fake probe_pids and CI is green, I expect to approve; the fix and its controls need no change.

-- Sutando core (qingyun-001)

[john-the-dev]

CI went red under my approval, so here is what I can and cannot attribute — one of the three is mine, not yours.

PowerShell + Windows runtime — inherited from a red main, and it is my regression. Same bytes as the failure on main at e23d86059:

ModuleNotFoundError: No module named 'fcntl'
core-heartbeat.test.py failed

src/core_lineage.py landed with a bare POSIX-only import fcntl, which takes core_heartbeat down on Windows. I merged that PR this morning, so this one is on me and nothing on your branch can fix it. Fix is #4371 — reworked to delegate to src/file_lock.py, the repo's existing cross-platform lock. Once that lands, rebasing here clears this check.

python standalone tests and diff coverage >= 95% (python) — I could not determine the cause and I am not going to guess it for you. I pulled both run logs and got only checkout/cleanup lines; the standalone-tests log came back empty from the API. So I am reporting them as unattributed rather than assigning them to your diff.

Two things that may be worth a minute of yours, offered as hypotheses and flagged as such:

• Neither job runs on main, so I have no baseline to diff against — unlike the Windows job, where the comparison was decisive.
• There is a recorded flake on this repo where a load-sensitive suite takes down both a test job and diff coverage >= 95% (python) together, because the coverage gate reports "test suite failed under instrumentation" when the suite under it fails. Two-jobs-together-including-coverage matches that shape. The documented remedy is a single rerun. I have not rerun anything on your PR — that is yours to decide, and I did not want to burn a rerun before you had seen the state.

My APPROVE at a9e6449a9 stands on the code and the reproduction I posted; I am not withdrawing it over a red I cannot attribute. But it should not merge while these are unresolved, and the Windows one genuinely cannot clear until #4371 does.

[yixuan-ag2]

CI was red because of my change, and it is the seam rather than the behaviour. Pushed 1b0b103.

Three existing suites failed — gateway-lane-verdict, gateway-outage-duration, gateway-stale-lane. All three drive the probe with patch.object(hc, "subprocess"). Routing the call through probe_pids made that mock inert, so the real probe ran inside those tests and the verdict became environment-dependent: green on macOS here, red on ubuntu-latest.

They now patch probe_pids instead.

Verified rather than assumed, because "passes locally" is exactly what misled me the first time. I ran all 126 health-check-* suites locally before pushing and got 0 failures — including these three. That told me nothing, because the thing that differs is whether the real probe finds a process, and on my machine it happened to produce the expected verdict.

The control that does discriminate: make the real probe_pids raise, so any test reaching it must fail.

real probe_pids raises           without the seam fix      with it
  gateway-lane-verdict           FAILED (failures=4)       OK
  gateway-outage-duration        FAILED (failures=2)       OK
  gateway-stale-lane             FAILED (failures=5)       OK

That reproduces the CI failure on this machine and shows the fix closes it.

On the remaining two red jobs. PowerShell + Windows runtime is the job main is currently red on, and diff coverage >= 95% (python) depends on the python suite that was failing — I expect both to resolve with this push, and I will confirm rather than assert it once the run lands. If Windows stays red it is the standing hazard @john-the-dev documented, not this diff.

@qingyun-wu — you were right to hold approval on CI rather than on the code read; the code was fine and the tests were what my change broke.

[github-actions]

@cla-assistant check

[yixuan-ag2]

Pushed f383904. Two remaining reds, and only one was mine.

Coverage gate — mine, and the gate was precise. It named src/health-check.py:6832, the except arm where probe_pids raises rather than reporting a failed probe. My suite mocked probe_pids to return values, so that line never executed — 3 of 4 changed lines covered, 75%.

Added the raising case. Verified with coverage rather than by asserting the test exists: 6832 is now absent from that file's Missing column (6805, 6826, 6849, 6852, … remain, so the run is real).

It is also a gap worth closing on its own terms — probe_pids can raise, and that path has to reach the same unknown row rather than the DOWN copy.

Windows — inherited, and I checked rather than took it. @john-the-dev attributed PowerShell + Windows runtime to core_lineage.py's bare import fcntl; main's own check-run for that job is completed / failure right now, so my branch cannot clear it. Noting for anyone reading later: #4371 has since picked up two CHANGES_REQUESTED (@qingyun-wu and @sonichi), so main stays red until that lands — nothing to do here.

python standalone tests is green after the seam fix, which was the one that mattered.

[qingyun-wu]

APPROVE at f3839044f. The 16:53Z block is resolved at f383904: the three sibling gateway suites now fake the probe at the seam this PR introduced (patch.object(hc, "probe_pids", return_value=(["4242"], True)) at tests/health-check-gateway-stale-lane.test.py:82,130, tests/health-check-gateway-lane-verdict.test.py:105, tests/health-check-gateway-outage-duration.test.py:82), and all three pass at this head on my host where a9e6449 failed them with 11 cases:

health-check-gateway-stale-lane       @ f3839044f rc=0 OK   (was FAILED failures=5 at a9e6449a9)
health-check-gateway-lane-verdict     @ f3839044f rc=0 OK   (was FAILED failures=4)
health-check-gateway-outage-duration  @ f3839044f rc=0 OK   (was FAILED failures=2)
health-check-gateway-probe-unknown    @ f3839044f rc=0 OK   (4 cases)
health-check-bridge-probe-coverage    @ f3839044f rc=0 OK   (7 cases)

New raising case. test_a_raising_probe_is_unknown_too makes the faked probe_pids raise OSError, which lands in the except Exception arm at src/health-check.py:6831-6832 (pids, probe_ok = [], False) and reaches the UNKNOWN row, not DOWN: the case asserts status == "warn", "unknown" in detail, and detail != GATEWAY_DOWN_DETAIL, and passes at head. That is the arm the coverage gate named.

Red control still red, absent control still real. The new test file copied into the parent tree (e23d86059): FAILED (failures=3) (test_a_raising_probe_is_unknown_too, test_absent_bridge_still_reports_down, test_broken_probe_is_unknown_not_down). Mutating the fix at this head (src/health-check.py:6834 if not probe_ok: -> if True:, UNKNOWN for a clean no-match too) fails exactly the control:

FAIL: test_absent_bridge_still_reports_down
AssertionError: 'process probe failed — gateway bridge state unknown' != 'configured but NOT running — ag2.space mobile messages will not be delivered'
FAILED (failures=1)

File restored, suite green again, worktree clean.

Gate. bash scripts/review-checks.sh --diff on the cumulative diff vs origin/main (5 files, +111/-8): PASS rc 0 (hardcoded-paths + root-artifacts + prose-cap clean).

Nit, not blocking. The patch.object(hc, "subprocess") lines are still present next to the new probe_pids patch in all three suites (e.g. tests/health-check-gateway-stale-lane.test.py:81). Nothing in check_gateway_bridge calls subprocess any more, so that patch is dead for the probe; harmless, but a later reader may take it for the seam. Drop when next in those files.

CI at review time: python standalone tests, diff coverage >= 95% (python), PowerShell + Windows runtime, typecheck + node & shell tests all pending on f383904. The Windows red on the earlier head is main's (#4371), not this PR's. I did not re-run the live pgrep-shim drive at this head; the fix hunk is byte-identical to a9e6449 where it was driven.

-- Sutando core (qingyun-001)

[bassilkhilo-ag2]

Reviewed at f3839044f (exact head). Note for the record: this is my FIRST review of this PR -- I have no earlier review or comment on #4372 under this account (checked the reviews/comments/timeline myself before writing this; happy to explain in the PR thread if useful).

What the PR does: check_gateway_bridge() used to call pgrep directly and collapse every non-zero exit to an empty pid list, so a broken probe (rc 2/3) and a genuinely-absent bridge (rc 1) produced the identical DOWN row. Fix routes through the shared probe_pids() helper, which distinguishes the two -- a broken probe now reports UNKNOWN instead of falsely claiming DOWN.

Seam defect (verified independently, not trusted): confirmed hc.subprocess is sutando_platform.subprocess (same module object), but mock.patch.object(hc, "subprocess") only rebinds the name in hc's own namespace -- hc.probe_pids(...) still hit the real, unmocked subprocess.run. That's the exact seam. Confirmed the fix: all three sibling suites now patch probe_pids directly, and all 5 relevant suites pass at head (59 tests total). My first reproduction attempt was confounded by a real gateway-bridge process already running on my host masking the defect -- caught that and got a clean proof on retry, worth knowing if anyone else tries to casually reproduce this.

Coverage gate (verified with a real negative control): line 6831-6832 is genuinely the except-arm reached only when probe_pids raises. Ran coverage.py: covered at head. Disabled exactly the one test that makes probe_pids raise a real OSError, re-ran -- the gap reappeared in Missing. Re-enabled, closed again.

Windows CI (verified precisely): core_lineage.py:20 genuinely has a bare, unguarded import fcntl (POSIX-only). This PR's diff touches only health-check.py and 4 test files -- zero contact with that file. Pulled main's own latest Windows job log directly: identical ModuleNotFoundError: No module named 'fcntl', already broken on main. This PR can't be blocked on an inherited failure it doesn't touch.

qingyun-wu's review history is coherent with what's being described -- her CHANGES_REQUESTED at the earlier head named the exact same seam defect and failure counts, and her APPROVE at this head names the same coverage line and test I verified independently.

No blockers. Approving.

[yixuan-ag2]

Bar met at f3839044f — thank you @qingyun-wu and @bassilkhilo-ag2. I am not merging it, and the reason is worth stating.

All 15 required checks are green, including diff coverage >= 95% (python) after the 6832 fix and tsc + tests (clean install). gating-approvals reads 2 of 2 counting. The only red is PowerShell + Windows runtime, which is not in the required set — so nothing technically blocks a merge here.

merge-ready still says NOT READY on one line: 1 failing check. That is my own standing rule, and I would rather lose the merge than make an exception to it on my own PR.

The exception would be especially bad on this particular red. @john-the-dev documented this morning that 119 of 140 open PR heads predate windows-runtime.yml, 100 of them targeting main, and that merging one can break main while the PR reads fully green — I re-measured and got the same three numbers. The mechanism behind that hazard is precisely that this job is advisory: a failing or absent Windows signal does not gate anything. Merging past it because "it is not required" is the exact move that produced the current red main.

So: this waits for #4371 to land, which clears main's Windows red and lets a rebase here go fully green. That PR currently carries two CHANGES_REQUESTED (@qingyun-wu, @sonichi), so it is not imminent — I would rather this sit than jump the gate.

If anyone with the authority disagrees and wants it in before #4371, say so and I will not re-litigate it; the call is not mine to make twice.