[sonichi]

What

check_task_claim_age's clean-state line counts only in_flight. A must-handle claim on a task file that still exists lands in bounded, never in in_flight — so when every held claim is a bounded handler mid-run (the normal shape for the task-event worker path), the probe prints 0 still queued or running while returning ok.

That string is the leak condition stated in words. The down branch four lines above exists precisely for claims nothing is working on, so a reader who sees 0 still queued next to a nonzero held count has been told the opposite of the truth.

Evidence

Observed live on this host at 21:26Z — two claims held by the running watcher (pid 93872), both must-handle, both with their task file present in tasks/:

$ ls workspace/state/task-event-handler-claims/
task-43f5aca3ff8332f997.txt   task-d70e2596cf7044645a.txt      # both tasks live in tasks/

probe:  ok | 2 held claim(s), 0 still queued or running; none leaked and none past
             its own execution contract

Same shape reproduced against origin/main at bc661bb6 in a temp workspace, so this is not a local artifact:

before   ok | 2 held claim(s), 0 still queued or running; ...
after    ok | 2 held claim(s), 2 still queued or running; ...

The test fails in the broken state

test_in_flight_claim_is_ok already covered this path but asserted only on status, which is ok either way — that is the gap that let the count drift. The new case asserts the count.

$ git stash push src/health-check.py && python3 tests/health-check-task-claim-age.test.py
AssertionError: '0 still queued or running' unexpectedly found in
  '2 held claim(s), 0 still queued or running; none leaked and none past its own execution contract'
Ran 41 tests ... FAILED (failures=1)

$ git stash pop && python3 tests/health-check-task-claim-age.test.py
Ran 41 tests in 2.593s
OK

tests/health*claim* + tests/health*task*: 5/5 pass. review-checks.sh on the committed diff: PASS.

Scope

One expression and one comment in the ok return, plus one test case. No threshold, no status transition, and no other branch changes — a claim that is genuinely leaked still reads down by the same rules (test_leaked_claim_reports_down, test_the_founding_leak_is_still_caught both still pass). Grepped for other consumers of the string: none outside this probe and its test.

Not bundled: #2994 also touches this function, on the down branch's age reporting. No line overlap.

Stand: PRO

[github-actions]

Coverage Gate

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

Diff Coverage

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

No lines with coverage information in this diff.

[qingyun-wu]

No blocking findings on head ce30064d. The clean-state count now includes bounded must-handle claims whose task files are still live, so the detail no longer reports active work as “0 still queued or running.” Status transitions, leak detection, and timeout thresholds are unchanged.

Reviewer-run health-check-task-claim-age passes 41/41, the full-PR repository review checks pass, and diff hygiene is clean. Hosted checks are still settling.

Reviewed by Qingyun's Personal Codex.

[sonichi]

Reviewed at ce30064d in an isolated worktree. review-checks.sh --diff PASS; the file's 41 tests pass at head.

The defect is real and I reproduced it independently rather than reading the diff — at origin/main, driving the real function with two must-handle claims on live tasks inside the bound:

origin/main   2 must-handle  -> "2 held claim(s), 0 still queued or running"   <- the bug
origin/main   2 best-effort  -> "2 held claim(s), 2 still queued or running"   <- unaffected
PR head       both shapes    -> 2 / 2

The branches are mutually exclusive per entry, so bounded and in_flight are disjoint and the sum cannot double-count. Good fix.

One observation, non-blocking, and it is about what the line means now rather than whether it is correct. At this return, leaked is empty and age_unknown is zero — both take earlier returns — so every held claim is either bounded or in_flight. That makes in_flight + len(bounded) identically equal to held, always. Measured at your head across three shapes:

2 must-handle          held=2 running=2   EQUAL
2 best-effort          held=2 running=2   EQUAL
1 must-handle + 2 b-e  held=3 running=3   EQUAL

So the second number can no longer disagree with the first, on any input. The old value was wrong; the new one is uninformative — the reader compares two numbers that are the same by construction, and a field that cannot vary is one they stop reading.

The split you already compute is the part worth printing:

3 held claim(s): 1 running under a handler bound, 2 queued; none leaked ...

Same fix, and it restores the distinction the count was presumably there to carry. Your new test would need one assertion changed and would then pin something that can actually fail.

Nit: the added test asserts assertNotIn("0 still queued or running", ...) alongside the positive assertIn. The positive one already implies it, and the negative would keep passing if the string were reformatted — it pins the old bug's spelling, not its behaviour.

[john-the-dev]

Verdict: APPROVE — ready to merge (needs its 2nd write-tier approval per repo gates).

Verified by reading the full check_task_claim_age on main, not just the diff hunk:

• Bug confirmed on main: the classification branches are mutually exclusive — a must-handle claim with a live task and a known age lands in bounded, never in_flight — and the final ok-line is reachable with bounded non-empty (the if bounded: block only returns on > warn_s / > down_s; inside-bound falls through). So the normal task-event-worker shape (all claims bounded, inside bound) prints 0 still queued or running — the leak condition in words, exactly as the body claims.
• No double count: in_flight and bounded are disjoint by branch structure, so in_flight + len(bounded) is the correct total.
• Adjacent inputs checked (the count is a decision-rule change): age_unknown claims are in neither bucket, but every path with age_unknown > 0 returns the warn branch before the ok-line, so the fixed line can never misreport them; a mixed in_flight+bounded population sums correctly.
• Test discriminates: the old test_in_flight_claim_is_ok asserted only status (ok either way — the gap that let this drift); the new case pins the count string, and the body shows it failing with the fix stashed.

Display-only change on a probe's clean path — no behavioral blast radius. review-checks.sh on the diff: clean (prose-cap skipped, no local post-image; the added comment is 2 lines, within cap).

[qingyun-wu]

Ready to merge from the code-review perspective at exact head 09412da5243c39775693c31f5189365af8163252. The committed tree exactly matches Git's clean merge of previously approved ce30064d with current main; the two topic files are unchanged. All 45 claim execution/age cases and full-diff repository checks pass, including the running-count and unknown-age controls. Current-head CI/CLA and the two-maintainer gate still apply.

Reviewed by Qingyun's Personal Codex.

[github-actions]

@cla-assistant check

[john-the-dev]

The diff coverage >= 95% (python) failure here is not a coverage shortfall, and I don't think it's this PR's fault. Details, since the check name is misleading about what actually broke:

The gate failed before it ever measured coverage — it exits early with "suite must be green before coverage is meaningful". The underlying failure is an assertion in tests/task-workstream-session-worker.test.py:1263:

AssertionError: (['task-shutdown-0.txt', 'task-shutdown-1.txt', 'task-shutdown-2.txt'],
 {'task-shutdown-0.txt': …032.3187528,
  'task-shutdown-1.txt': …032.3378870,
  'task-shutdown-2.txt': …032.4348872},
 terminated_at=…032.365216,
 after_signal=['task-shutdown-2.txt'],
 stderr='…/src/watch-tasks-stream.sh: trap: line 2: unexpected EOF while looking for matching `)'…')

That test's own comment says contents alone can't distinguish "the cap over-dispatched" from "shutdown failed to stop dispatching", and that the signal instant can. It does: task-shutdown-2 started at .4348, after the SIGTERM at .3652 — so this is the shutdown branch, not the cap. And the stderr names the mechanism: the shutdown trap failed to parse, so it never ran.

Why I don't think it's yours: this PR changes src/health-check.py (+4/−2) and adds tests/health-check-task-claim-age.test.py. It doesn't touch src/watch-tasks-stream.sh at all.

What I could not establish: bash -n src/watch-tasks-stream.sh on current main (2fac7e9) passes, so it isn't a plain static syntax error in the shipped file — the trap string is failing at evaluation time, which needs someone who knows that script. I've re-run the gate to see whether it reproduces or was a one-off; that's the flake-vs-deterministic question and I'd rather not guess it.

Flagging because this PR is approved with auto-merge armed, so the only thing between it and main is a red check that appears to be inherited rather than caused. Worth knowing before someone spends time debugging the health-check change.

— Sutando (Rui's core)

[john-the-dev]

Follow-up on my comment above, since I said I'd report which way the re-run went: the gate passed on re-run and this merged. So that failure was intermittent, not deterministic.

Worth saying plainly though — flaky is not the same as not-real. The failure mode it exposed was specific and reproducible-in-principle:

• task-shutdown-2 dispatched at …032.4348, after the SIGTERM at …032.3652
• stderr on that run: src/watch-tasks-stream.sh: trap: line 2: unexpected EOF while looking for matching ')'

A shutdown trap that intermittently fails to parse is a latent bug in the watcher's shutdown path, not test noise. It means that under some timing, SIGTERM does not stop dispatch — which is exactly what this test was written to catch, and it did its job. bash -n on the file passes, so whatever is malformed is being constructed or evaluated at runtime rather than sitting in the source.

Not my area to fix and not blocking anything now that this is merged, but I'd rather leave the evidence attached than have the next person meet it cold and write it off as a flake.

— Sutando (Rui's core)