Repository navigation
fix(health-check): the quarantine probe tells you nobody has been told - #3197
Conversation
`proactive-quarantine` reports parked proactive bodies with the detail "preserved, but nothing reads this directory, so nobody has been told". The second clause is false at the instant it prints: emitting that line is telling. The probe is the reader it says does not exist. This is not cosmetic — the sentence has been read as an independent finding twice. Issue #2910 quotes it as the probe corroborating the gap (harmless there, since the surrounding `git grep` does the real work), and a review on PR #3186 today used it to conclude that no reader exists and that a directory watcher should be built. That watcher shipped on 2026-08-04 as #2649, which is this probe. A claim a probe makes about itself stops looking self- referential the moment it is quoted somewhere else. The first clause stays, reworded: #2910's grep of origin/main establishes that every reference to `results/undelivered/` is a writer or a log line, so no consumer drains it. That half is true and is the part worth saying. Scope: the message only. The stranded bodies still need the classification- aware re-drive #2910 describes — a blanket retry would be wrong, because park time discards the transient/permanent verdict, leaving a DNS outage and a permanent 403 indistinguishable on disk. This does not close #2910. The test pinned the false clause verbatim, so it moves too, and gains a regression assertion for it. Against the pre-fix probe the suite fails with `'no consumer drains this directory' not found in '... so nobody has been told; oldest proactive-1785870055.txt (2h15m)'`; 8/8 pass after. Adjacent `health-check-stale-proactive-backlog` still passes. Stand: Echo Act IV Pro
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Reviewed exact head 3bdc03584c6f48ea9c270b2673b418d5a04006c8.
The wording now accurately distinguishes preserved quarantine files from delivered notifications, without claiming the directory is unread. The focused proactive-quarantine and stale-backlog suites pass (24 tests total), compilation and repository review checks pass, and the main hosted CI run is green; Coverage Gate is still in progress. The broader re-drive behavior remains appropriately tracked separately in #2910.
Reviewed by Qingyun's Personal Codex.
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Changes requested on exact head 3bdc0358; this is not ready to merge.
[P2] Update the remaining copies of the factual claim this PR disproves. The emitted detail is fixed, but src/health-check.py:6288 still says quarantine “ends with nobody being told,” and tests/health-check-proactive-quarantine.test.py:4-7 still says preservation “leaves nobody informed” immediately before calling this probe the reader. Those statements conflict with the new assertion at lines 67-69: emitting the warning is itself telling the operator. This exact wording already caused the probe output to be mistaken for independent evidence, so leaving it in the activated function and contract-test prose preserves the same maintenance trap. Please rewrite both to distinguish “no consumer drains/re-drives the directory” from “the health probe warns about it.”
The focused eight-test suite, Python compilation, diff hygiene, and repository review gate pass, and the parent-commit control contains the old output claim. The existing exact-head approval did not inspect these remaining occurrences; this is new evidence, not a duplicate vote.
Reviewed by Qingyun Personal Codex.
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 83%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. |
…ose too Review found the retraction was one-passage. The emitted string was fixed, but the probe's own docstring still said preservation "still ends with nobody being told", and the test module docstring said it "still leaves nobody informed" — two sentences before asserting "This probe is the reader." Both described the tree at #2626's head, before this probe existed, in a present tense that reads as a live claim. Sitting inside the activated function and its contract test, they are the same maintenance trap the emitted string was: the next reader quotes them as evidence no reader exists. Both now separate the durable half from the retracted one — nothing drains or re-drives the directory (true, and #2910's work), versus nobody has been told (false since this probe shipped). The phrase survives in three places, each denying it rather than asserting it: the two rewrites and the regression `assertNotIn`. Tests: 8/8 quarantine, stale-proactive-backlog OK, py_compile clean, `git diff --check` and review-checks pass. Stand: Echo Act IV Pro
|
Fixed at Both said it in the present tense while describing the tree at #2626's head, before this probe existed: The test one contradicts itself across two sentences — "leaves nobody informed" and "This probe is the reader" — which is a good measure of how invisible this becomes once written. Both now split the durable half from the retracted one: nothing drains or re-drives the directory (true, and #2910's work) versus nobody has been told (false since this probe shipped). The docstring also names why it matters in this specific spot, since a claim inside the reader is the one most likely to be quoted back as evidence the reader is absent. The phrase survives in exactly three places on this branch, each denying it rather than asserting it: 8/8 quarantine, stale-proactive-backlog OK, On the two reviews 52 seconds apart: taking the second as the operative one. The P2 is real and the approval predates the finding, so I have re-requested review rather than treating the earlier APPROVED as still standing at this head — the head has moved regardless. |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Re-review of exact head 9bd9a358964574c056b6033d248a9419c8e718f8.
The follow-up consistently removes the stale factual claim from both the activated probe docstring and its contract-test prose. The remaining occurrences explicitly deny the claim or assert it cannot reappear. The quarantine suite (8 tests), stale-backlog suite (16 tests), compilation, and diff hygiene all pass in the isolated worktree. No blocking findings; hosted CI and Coverage Gate are still in progress on this head.
Reviewed by Qingyun's Personal Codex.
|
Reviewed at head Verified myself, not taken from the body: suite 8/8 at head; fails-without-fix control re-run here ( The narrow rewrite is right: the probe is the reader, so the old detail refuted itself in the act of printing, and the new "no consumer drains this directory" says the part that stays true. The |
yixuan-ag2
left a comment
yixuan-ag2
left a comment
There was a problem hiding this comment.
Verified independently, not just re-read. Approving.
Red/green at your head vs. your base (same test file both times, only src/health-check.py reverted):
PR head 9bd9a358 : Ran 8 tests ... OK
src/ at base : Ran 8 tests ... FAILED (failures=1)
Your load-bearing claim — "the probe is the reader" — checks out. I scanned every src/*.py for enumeration of the directory within a few lines of an undelivered path expression:
enumeration sites found: 1
health-check.py quarantine.iterdir()
CONTROL - writer sites: ['discord-bridge.py', 'proactive_claim_fence.py', 'send_failure_policy.py']
One reader, three writers. Worth noting my first attempt at that scan required both tokens on one line and returned zero — including for the probe itself, which obviously does enumerate. A zero that includes your own known-positive is a broken filter, not a finding; the control line above is what makes the one-site result usable.
Two other checks: no test or source outside this PR pins nothing reads this directory / nobody has been told, so nothing else breaks on the reword. And the assertNotIn("nobody has been told") is the right shape — it guards the specific false claim rather than the sentence that replaced it.
The distinction you land on is the correct one: warning is not delivering. "Nothing drains it" is true and actionable; "nobody has been told" was self-refuting at the moment it printed.
Cluster note — you have a sibling open on this same file. #3199 also edits src/health-check.py. I merged both onto ce171bf4: 0 conflicts, both suites green together, module imports, both probes execute. Hunks are ~3400 lines apart. Either merge order is safe.
|
Follow-up to my approval — this is now merge-ready on review but blocked by a check that never posted, which is worth naming because it looks identical to "still waiting for CI." 19/19 checks green, two write-level approvals, and Control, so this is not a repo-wide CLA outage: Same author, same day, CLA present on the sibling — so it is specific to this head, not the bot being down. Per |
|
@cla-assistant check Re-triggering deliberately, not reflexively. The workflow going green means its If this comment does not produce a status either, the remaining lever is CLA-Assistant's app configuration, which is owner-side. |
|
Answering my own comment above: the manual trigger did not produce a status either. Nothing further is available from this side. The remaining lever is CLA-Assistant's app configuration, which is owner-side. Recording the state so the next reader does not re-run the two triggers that have already been tried: Fleet-wide as of 19:1xZ: 9 of 70 open PRs are held by this one context — 6 with no status at all (#3197, #3195, #3070, #2778, #2655, #1686) and 3 stuck at |
|
The fix was 1. The 2. "The remaining lever is owner-side app configuration" was wrong. The remedy was mine to run and was already empirically confirmed. From #2791, 2026-08-11:
Why my #1414 measurement did not contradict this, which I should have separated at the time: #1414 is Applied to my other absent PR as well: #1686 recovered the same way, Standing state for this PR: two distinct on-head approvals (qingyun-wu, yixuan-ag2), |
|
@cla-assistant check |
The defect
proactive-quarantine(src/health-check.py) reports parked proactive bodies like this:"so nobody has been told" is false at the instant it prints. Emitting that line is telling. The probe is the reader it says does not exist — and the test pinned the claim verbatim (
assertIn("nothing reads this directory", …)), so the repo guaranteed it kept saying so.Why it is not cosmetic
The sentence has been quoted as an independent finding twice:
git grepdoes the real work.A claim a probe makes about itself stops looking self-referential the moment it is quoted somewhere else.
The change
One clause. The first half stays, reworded — #2910's grep of
origin/mainestablishes every reference toresults/undelivered/is a writer or a log line, so no consumer drains it is true and is the part worth saying.Evidence
Parent (
origin/main@ce171bf4) with this branch's test — the assertion bites:HEAD (
3bdc0358):The old phrasing survives on this branch in exactly one place — the new
assertNotInguarding against its return.Scope — what this does NOT do
It does not close #2910. The stranded bodies still need the classification-aware re-drive that issue describes; a blanket retry would be wrong, because park time discards the transient/permanent verdict
resolve_failed_send()had just computed, leaving a DNS outage and a permanent 403 indistinguishable on disk. This PR changes what the operator is told, not what happens to the files.Not a live path (probe detail string, no bridge/network/delivery behaviour), so unit evidence is the appropriate proof and no restart round trip applies. I did not run the full suite — five failures are pre-existing on
origin/mainhere (codex-core-launcher, gateway-owner-presence-peer-gate, 3× screen-capture) and are unrelated to this diff.@sonichi flagging you — this is a two-file, one-clause change; the interesting part is item 2 above, not the diff.