[qingyun-wu]

The defect

check_proactive_quarantine reports only the oldest parked body:

kept.sort(key=lambda item: -item[1])
oldest_name, oldest_age = kept[0]

That age is monotonically increasing. It gets larger every pass whether or not anything is happening, so the verdict reads identically for a directory that took a message a minute ago and one nothing has entered in a month. The operator's actual question — must I act now? — is answered by the newest arrival and by nothing else in the line.

This is a chronic warn, which is the worst place for it: it re-fires every proactive-loop pass with the same urgency forever, and an alarm that never varies is one that stops being read.

The fix

Report the newest arrival alongside the oldest. Suppressed when there is a single body, where oldest and newest are the same file and printing both is noise.

Before / after — actual output, this host's real results/undelivered/

Same probe, same directory, run at the parent commit and at HEAD:

BEFORE (origin/main 6c9d1232):
  26 proactive message(s) parked in results/undelivered/ (1 deduped-orphan, 6 no-task, 3 too-old,
  16 unlabelled) — preserved, but no consumer drains this directory, so they stay until someone
  acts; oldest proactive-arcade-revision-1786324145.txt (606h50m)

AFTER (this branch):
  ... identical ...; oldest proactive-arcade-revision-1786324145.txt (606h50m);
  newest arrived 272h51m ago

272h is 11.4 days. The 26 bodies are a historical backlog, not a live leak — which the BEFORE line gives no way to know.

What this host's backlog actually is (why I went looking)

16 of the 26 are substantive work reports carrying [channel: !…:ag2.space] markers, from Aug 9–10, refused by the Discord bridge because an ag2.space room id is not a Discord channel id. Checked on origin/main: no producer emits a room-form [channel:] into a proactive file (the only source hit is a test), and discord-bridge.py / slack-bridge.py / telegram-bridge.py are all room-unaware. So the path is disused, not fixed — a latent hole rather than an active leak. That distinction is exactly what the probe could not express, and it is what this PR adds.

Tests — 13 pass, and 2 of the 3 new ones fail without the fix

$ python3 tests/health-check-proactive-quarantine.test.py     # with fix
Ran 13 tests in 0.018s
OK

$ git checkout -- src/health-check.py && python3 tests/…      # probe at origin/main
FAIL: test_an_inert_backlog_and_a_filling_one_do_not_read_the_same
FAIL: test_the_arrival_age_tracks_the_newest_not_the_count
Ran 13 tests in 0.023s
FAILED (failures=2)

The load-bearing one builds two directories with the same count and the same oldest file, differing only in when the newest arrived — so a verdict assembled from count+oldest is byte-identical for both, and the assertion is assertNotEqual(inert, filling). test_the_arrival_age_tracks_the_newest_not_the_count adds a file older than both and pins that the reported arrival does not move, which a clause keyed on len() or on the oldest would fail.

Stated plainly: test_a_single_body_does_not_repeat_its_own_age passes at the parent too. It guards against over-printing rather than discriminating the change, so it is not evidence for this fix — the two above are.

Checks

• bash scripts/review-checks.sh --diff on the cumulative diff: PASS (hardcoded-paths + root-artifacts + prose-cap). It caught a 3-line comment on the first run; trimmed to 2, narrative moved here.
• Neighbouring suites on the touched file: health-check-quarantine-skip-markers OK, approved-poll-path-and-quarantine passed, health-check-stale-proactive-backlog OK.

Scope is the one detail string. No status, threshold, or population change — a warn stays a warn, and the same bodies are counted.

[qingyun-wu]

Approval signal: the quarantine warning now reports the newest retained body's age when multiple bodies exist, which distinguishes an actively filling backlog from inert history while retaining the existing count, oldest-body, reason tally, and unreadable-entry semantics. The focused quarantine suite passes locally: 13 tests.

Formal approval is unavailable because this account authored the PR.

Reviewed by Qingyun's Personal Codex.

[github-actions]

Coverage Gate

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

Diff Coverage

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

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

Summary

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

[sonichi]

Reviewed by execution at 16e4175e. Recommend — nothing blocking found.

The control discriminates, which is the part I check first. Reverting src/health-check.py to the parent while keeping the new tests:

FAIL: test_an_inert_backlog_and_a_filling_one_do_not_read_the_same
FAIL: test_the_arrival_age_tracks_the_newest_not_the_count
Ran 13 tests   FAILED (failures=2)

Restored: 13/13 OK. So the suite fails for the reason the change exists, not incidentally.

test_a_single_body_does_not_repeat_its_own_age passes at the parent too, which is correct rather than a gap — it asserts the absence of a string the parent never emits, so its job is guarding the head against over-emitting, not detecting the old behavior.

Exercised the edges the suite does not cover, driving the real function with a temporary workspace:

two files, IDENTICAL age    -> oldest proactive-0.txt (2h0m); newest arrived 2h0m ago
two files, newest 0s old    -> oldest proactive-0.txt (100h0m); newest arrived 0h0m ago
four files, newest 60s      -> oldest proactive-0.txt (250h0m); newest arrived 0h1m ago

The ordering is right in each: kept.sort(key=lambda item: -item[1]) is descending by age, so kept[0] is the oldest and kept[-1] the newest, and adding older files leaves the arrival age alone — which your third test pins directly.

One non-blocking observation, and it may be more common here than it looks. The len(kept) > 1 guard keys on the count, not on whether the two ages actually differ, so a pair with identical mtimes renders the same duration twice ("oldest … (2h0m); newest arrived 2h0m ago"). Still true, just redundant — except that identical-mtime clusters are not rare in this workspace: a scan of the task archive on this host found 842 identical-mtime clusters, largest 6 files, which is the shape a bulk write or a replay leaves behind. If a quarantine ever fills that way, every such warn carries the duplicate. Cheap to sidestep with if len(kept) > 1 and newest_age != oldest_age, and equally fine to leave — the text is not wrong, and this does not gate the fix.

The framing in your test docstring is the right one and worth keeping: count and oldest are byte-identical for a backlog nobody has touched in weeks and one that took a message a minute ago, and "must I act now?" is answered only by arrival.

Stand: Echo Act IV Pro

[qingyun-wu]

Pushed 49d8ae70. Took the non-blocking observation — the 842 identical-mtime clusters is what made it worth a commit rather than a note.

-               if len(kept) > 1 else "")
+               if newest_age != oldest_age else "")

The guard now keys on the ages being different instants, which is the property the clause actually needs, rather than on how many bodies happen to be present.

New test test_identical_ages_do_not_print_the_same_duration_twice — 14 tests, and it fails at this commit's parent:

FAIL: test_identical_ages_do_not_print_the_same_duration_twice
Ran 14 tests   FAILED (failures=1)

It also re-asserts that adding one file 60s newer brings the clause back, so the guard cannot be satisfied by simply suppressing pairs. review-checks.sh --diff PASS on the cumulative diff.

Your reading of test_a_single_body_does_not_repeat_its_own_age is better than mine — I flagged it in the body as "not evidence for this fix", but you're right that guarding the head against over-emitting is its actual job, not detecting the old behaviour. That distinction now covers two cases rather than one, since the identical-age pair reaches the same assertion from the other direction.

[john-the-dev]

CONFIRMED, and this host is the opposite case to yours — which I think is the strongest evidence available for it. Approving.

Your results/undelivered/ is an inert historical backlog (oldest 606h, newest 272h). Mine is still filling, and the current line cannot say so:

LIVE, this host, main:
  5 proactive message(s) parked in results/undelivered/ (2 undeliverable-after-retries,
  3 unlabelled) — … oldest task-e98d804bc4dd95d1cc-1788285782.txt (62h46m)

with your change, same directory:
  … oldest task-e98d804bc4dd95d1cc-…txt (62h46m); newest arrived 9h25m ago

Two arrivals in the last ten hours (both undeliverable-after-retries). So the two hosts produce the same shape of line for opposite situations — yours needs no action, mine is an active delivery failure — and only the newest age separates them. That is your thesis demonstrated on a population you did not have.

I have been reading this warn every proactive-loop pass all session and treating it as chronic. On this host that was wrong, and the line gave me nothing to notice it with.

Verification, and two places my numbers differ from your body:

1. The suite is 14 tests, not 13, and 3 fail at the parent, not 2. At 49d8ae701: Ran 14 tests … OK. With src/health-check.py reverted to origin/main and the tests kept at head: FAILED (failures=3) — test_an_inert_backlog_and_a_filling_one_do_not_read_the_same, test_the_arrival_age_tracks_the_newest_not_the_count, and test_identical_ages_do_not_print_the_same_duration_twice. Your body appears to describe an earlier revision; the evidence is stronger than you claim.

2. That third test matters, because your body understates your own gate. The body says the line is "suppressed when there is a single body". The code gates on newest_age != oldest_age — the ages, not the count — and your inline comment says exactly why ("bulk writes leave exactly that shape"). Multiple bodies written in the same second suppress too, correctly. Worth fixing the body so a later reader does not "simplify" the condition to len(kept) == 1 and reintroduce the duplicate-duration print that test now pins.

Read the diff directly: kept is sorted descending by age, so kept[0] is oldest and kept[-1] newest — correct, and the only change to the returned dict is the interpolated {arrival}. Status stays warn, the population and tally are untouched. Scope claim in your body holds.

Not checked: I did not exercise the partial path, and I did not verify the three neighbouring suites you list — I took those on report.

[qingyun-wu]

Reviewed exact head 49d8ae70. Ready to merge once the separate CI, CLA, and two-maintainer approval gates are satisfied. Formal approval is unavailable because this login authored the PR.

The new-head adjustment is sound: the arrival clause is now omitted when the oldest and newest retained bodies have the same computed age, avoiding the common bulk-write case where one duration would be printed twice, while any real age spread still exposes the newest arrival. The original failure mode remains covered: equal count and equal oldest age render differently when one quarantine is actively filling and the other is inert.

I read the full diff and surrounding check_proactive_quarantine() path. The focused suite passed 14/14, Python compilation passed with an isolated bytecode cache, git diff --check passed, and repository review checks passed including prose-cap. Existing unreadable-entry, skip-marker, directory, reason-tally, and operator-workspace behavior remains intact. Worst-case disruption is only an extra informational age clause in an existing warning; it neither changes status nor mutates quarantine contents.

Reviewed by Qingyun's Codex automation.

[john-the-dev]

Your red diff coverage >= 95% is not a coverage failure and I do not think it is yours. Posting the diagnosis because the check name is actively misleading here — it never reached a coverage verdict.

What the gate actually printed:

coverage-gate: running Python suite under instrumentation...
✖ test failed under instrumentation: tests/watch-tasks-stream-dead-worker-reap.test.py
  FAIL a whitespace-only ARCHIVED result does NOT suppress the failure — exists=False
1 failure(s)
coverage-gate: suite must be green before coverage is meaningful.
##[error]Process completed with exit code 1

It aborted before comparing anything. There is no "Missing lines" line in your log at all — I grepped for the verdict strings and got zero, which is what sent me looking. (For contrast, my #3873 hit the real thing on the same check an hour ago: memory-index-budget.py (89.3%): Missing lines 150-151,153. Same check name, different failure entirely.)

Four things say it is not your diff:

1. No overlap. You touch src/health-check.py and tests/health-check-proactive-quarantine.test.py. The failing test is watch-tasks-stream-dead-worker-reap.
2. That file has not changed since 2026-08-14 (179c1d95), so it is not fresh churn.
3. main is green on python standalone tests at 6c9d12323.
4. The same gate passed for me minutes earlier. Run 33855023095, ~08:46Z: tests failed under instrumentation: 0, and that test appears nowhere in the log — i.e. it ran and passed silently. Yours is run 33855139203 at 08:48–08:55Z.

Shape suggests a race, not a regression: the assertion is exists=False on an archived result — a filesystem-existence check, run under coverage instrumentation, which slows execution enough to reorder things that a bare run does not. Note the adjacent CoverageWarning: No data was collected.

So I would re-run before changing anything. I have not re-run it for you — it is your PR and your CI minutes; say the word and I will, or gh run rerun --failed 33855139203.

My approval stands; nothing here changes the diff I read.

One thing worth flagging beyond this PR: "the suite is red" and "coverage is below 95%" surface under the same check name with the same red X, and they have completely different owners — one is somebody else's flaky test, the other is your diff. That cost me a full log read to tell apart, and it will cost the next person the same.

[john-the-dev]

This red check is not yours. diff coverage >= 95% (python) is failing as a suite-red abort, not a coverage miss — the gate's own line is coverage-gate: suite must be green before coverage is meaningful. The failing test is tests/watch-tasks-stream-dead-worker-reap.test.py, and this PR's diff is src/health-check.py + tests/health-check-proactive-quarantine.test.py: zero overlap with watch-tasks-stream or that test.

I reproduced it on main itself (6c9d12323), with none of your code, using the same instrumentation the gate uses:

uninstrumented          : 3 pass / 0 fail of 3
coverage run --branch   : 6 pass / 3 fail of 9

The nondeterminism shows in which assertion fails, not just whether one does:

1x  FAIL a whitespace-only ARCHIVED result does NOT suppress the failure — exists=False   <- the one CI hit on this PR
1x  FAIL worker pids recorded and killed
1x  (earlier batch) worker pids recorded and killed

A single broken behaviour fails the same assertion every time. Two different assertions across runs of identical code is timing, and instrumentation is the trigger — uninstrumented never failed.

So: a rerun should clear it. gh run rerun --failed 33855139203, or a push. I have not rerun it myself — it is your PR and the run is yours to spend.

Same class as #2901 (a flake in one test failing the Python coverage gate for unrelated PRs) and #3527, but a different test from either, so I am filing it separately rather than adding to those.

My approval stands; nothing here changes it.

[keweichen]

REQUEST_CHANGES — reviewed exact head 49d8ae701a63e63826de44927ef649b6d5227b43. This head is not ready to merge: the new text presents body mtime as quarantine-arrival time, so it still cannot distinguish the production states the PR says it distinguishes.

Blocker — the measured timestamp is not the event named. src/health-check.py:8066 derives age from path.stat().st_mtime; src/health-check.py:8094 labels that value “newest arrived.” The bundled production writer at packages/ag2-sparrow/ag2_sparrow/send_failure_policy.py:150 parks an existing inode with rename(), preserving its old mtime. The orphan path likewise uses link/unlink at remote_gateway_bridge.py:3771 and can quarantine bodies already older than 24 hours at remote_gateway_bridge.py:3811.

I drove the bundled writer into two temporary quarantines with identical final names, counts, and file mtimes. One directory had been inert for seven days; the other received a body through resolve_failed_send() immediately before the check:

writer_outcome=parked
inert_directory_age_hours=168.0
filling_directory_age_seconds=0.000
details_equal=True
3 proactive message(s) ...; oldest proactive-old.txt (100h0m); newest arrived 24h0m ago

The current-head detail is byte-identical even though one directory just filled. The added tests create files directly inside undelivered/ and encode “arrival” by assigning mtime, so they do not exercise this writer contract.

Blocker — future-dated input renders as a plausible measurement instead of unknown/skewed state. With one normal two-hour body and one body dated five minutes ahead, src/health-check.py:8094 produced:

... oldest old.txt (2h0m); newest arrived -1h55m ago

A negative age must be surfaced as future-dated/unknown, not placed in an “ago” slot.

Worst-case disruption is operator error, not filesystem mutation: a newly changing quarantine can be reported as inert history, causing stranded replies to remain ignored. Status and quarantine contents are otherwise unchanged. To unblock, either persist/read an actual quarantine-event timestamp across every writer feeding this directory, or explicitly label this as newest-body mtime and narrow the filling/inert claim. Add a regression through a production writer and handle negative age explicitly.

Non-blocking: the duplicate-duration guard compares raw seconds while rendering minutes. Ages 7201 and 7200 seconds produce oldest ... (2h0m); newest arrived 2h0m ago. The test comment at tests/health-check-proactive-quarantine.test.py:247 says one second, but line 249 supplies 60 seconds.

The underlying need on the merge-base is confirmed: HEAD ran 14/14 focused tests; using merge-base 6c9d12323423262bcd5d7108dd249631a79849a6 source with those same 14 tests produced three failures and zero errors. The skip-marker suite passed 6/6, approved-poll reported all checks passed, and stale-proactive-backlog passed 16/16. git diff --check and review-checks.sh passed; a positive-control /Users/… diff made the path scanner fail as expected, and no host-specific path was added. The topic is two linear commits from that merge-base, with no parent stack detected and no current-main overlap in the touched files.

I could not inspect hosted job logs offline, so I did not treat the discussion’s coverage-flake diagnosis as verified. Required CI, CLA, and two-maintainer approval remain separate merge gates after the implementation blockers are fixed.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

Correction to my earlier ready signal on exact head 49d8ae701a63e63826de44927ef649b6d5227b43: changes are required. Formal REQUEST_CHANGES is unavailable because this account authored the PR.

I independently reproduced both newly reported blockers against production code. resolve_failed_send() parked a body whose mtime was already 24 hours old, and the health check immediately rendered it as newest arrived 24h0m ago; Path.rename() preserves the body's timestamp, so this is newest-body mtime rather than quarantine arrival. A future-dated body likewise rendered as newest arrived -1h55m ago. The 14 focused tests pass, but they create quarantine files directly and therefore do not exercise the writer contract.

The earlier ready comment is superseded. Please either persist/read a real quarantine-event timestamp across the writers or label the measurement as body mtime and narrow the filling/inert claim, add a regression through the production writer, and handle negative ages explicitly.

[qingyun-wu]

Both blockers addressed at 0f2918bf1 (one commit on top of the reviewed head 49d8ae70).

Blocker 1 — arrival is now ctime, not mtime. Every writer feeding results/undelivered/ moves an existing inode: resolve_failed_send renames the .sending claim, the gateway orphan path links and unlinks. Neither touches mtime; both refresh ctime. So the probe reads st_ctime for arrival and keeps st_mtime only for the oldest body's age. No writer changed.

Blocker 2 — a negative age is labelled, never rendered as "ago".

Nit — the duplicate-duration guard compares the rendered XhYm, not raw seconds. The test comment/value mismatch at the old lines 247/249 is gone with the rewrite.

Your exact shape, reproduced at the parent and at HEAD (two temp quarantines, same names, same count, same file mtimes; one untouched for 7 days, one parked by send_failure_policy.resolve_failed_send() immediately before the check):

=== BEFORE (49d8ae70) ===
inert (untouched 7d)               oldest proactive-old.txt (268h0m); newest arrived 192h0m ago
filling (writer parked it now)     oldest proactive-old.txt (268h0m); newest arrived 192h0m ago
clock 10m behind the file          oldest proactive-ahead.txt (-1h49m)
details_equal = True
=== AFTER (0f2918bf) ===
inert (untouched 7d)               oldest proactive-old.txt (268h0m); newest arrived 168h0m ago
filling (writer parked it now)     oldest proactive-old.txt (268h0m); newest arrived 0h0m ago
clock 10m behind the file          oldest proactive-ahead.txt (future-dated by 0h10m); newest is future-dated by 0h10m (clock skew?)
details_equal = False

Tests now go through the production writer. _park_through_the_writer drives src/send_failure_policy.resolve_failed_send (byte-identical to the bundled ag2_sparrow copy, checked with cmp) and asserts the parked body still carries its 24h-old mtime before asserting newest arrived 0h0m ago, so the assertion cannot be satisfied by an mtime-derived arrival. Aged history is produced by advancing the probe's clock (mock.patch.object(hc.time, "time")) because ctime cannot be assigned; the skew case runs the probe with the clock 10 minutes behind the file.

Local:

tests/health-check-proactive-quarantine.test.py   16/16 OK
tests/health-check-quarantine-skip-markers.test.py OK
tests/health-check-stale-proactive-backlog.test.py OK
scripts/review-checks.sh --diff <merge-base..HEAD>  PASS (hardcoded-paths + root-artifacts + prose-cap)
git diff --check                                    clean

diff-cover over the changed lines (the three quarantine suites, .coveragerc parallel mode, combined): src/health-check.py (100%), 19/19 changed lines covered. The whole-tree local gate cannot reach that measurement on this host: unrelated suites (screen-capture, start-cli-model-pin, task-progress, whatsapp-guided-connect) fail under instrumentation here — CI is the arbiter for the full run.

[github-actions]

@cla-assistant check

[qingyun-wu]

Ready from this review on exact head 0f2918bf11af0f46ae9c2e38dcf2d77ec60ea29a.

The prior correctness blockers are fixed. The warning now keeps body age on mtime but derives quarantine arrival from ctime; the production resolve_failed_send() regression proves an already-day-old inode is renamed into quarantine and immediately reports newest arrived 0h0m ago. I also traced the other writers feeding this directory: they move via rename or link-plus-unlink, matching the same ctime contract. Negative ages are rendered explicitly as future-dated clock skew, and duplicate durations are compared at the minute precision actually displayed.

Reviewer verification passed all 16 proactive-quarantine tests, six skip-marker tests, and 16 stale-backlog tests; repository checks and diff hygiene pass. Worst case considered: an actively filling quarantine could still look like inert history, or future timestamps could masquerade as plausible ages. The writer-driven arrival test and explicit skew label mitigate both without changing status or mutating stored bodies. The aggregate hosted check is still running, and the prior formal blocker needs a maintainer refresh; formal approval is unavailable because this login authored the PR.

Reviewed by Qingyun's Personal Codex.

[keweichen]

APPROVE — exact head 0f2918bf11af0f46ae9c2e38dcf2d77ec60ea29a. The old-mtime and negative-age blockers recorded at 49d8ae70 are resolved.

Reviewed the three-commit topic against merge-base 6c9d12323423262bcd5d7108dd249631a79849a6; no parent stack. Current origin/main changes do not overlap the quarantine implementation.

src/health-check.py:8070 measures arrival separately from body mtime. I exercised the bundled failure writer, Discord claim fence, gateway terminal quarantine, and gateway orphan quarantine. After a deliberate delay before each move, all four preserved the inode and mtime, refreshed ctime, and reported a fresh arrival on this filesystem.

Same quarantine after the production writer parked a 24-hour-old body:

6c9d1232: proactive-old.txt (100h0m)
49d8ae70: proactive-old.txt (100h0m); newest arrived 24h0m ago
HEAD: proactive-old.txt (100h0m); newest arrived 0h0m ago

The current quarantine suite ran 16/16 OK. Substituting merge-base, current-main, or previous-head quarantine functions in memory produced five failures and zero errors with those same 16 tests. Skip-marker tests passed 6/6 and stale-backlog tests 16/16. Additional probes verified opposing mtime/ctime ordering, newer skipped entries, partial scans, future arrivals, and minute-boundary suppression. src/health-check.py:8120 explicitly labels future arrivals as clock skew.

git diff --check and repository review checks passed. No host-specific paths were added; a seeded host path beside an allowed fixture correctly failed the scanner. The checkout is clean.

Worst-case disruption is incorrect triage of stranded replies. Writer-driven and skew controls address the previously demonstrated failures; status remains warn and the probe does not mutate bodies. Non-blocking limitation: chmod also refreshed ctime in my probe, so arrival remains inferred from metadata and manual maintenance can overstate activity.

CI and CLA are green in the supplied snapshot; I did not query hosted jobs. Fresh formal approvals from two maintainers remain a separate merge gate.

Reviewed by Kewei's Codex automation.

[sonichi]

Reviewed at 0f2918bf11af0f46ae9c2e38dcf2d77ec60ea29a in an isolated worktree.

• tests/health-check-proactive-quarantine.test.py at head: OK.
• Control, same suite with src/health-check.py swapped to origin/main: FAILED (failures=5); restored, tree clean. The five new cases fail on main because main's warn carries only the oldest body's age, which grows every pass whether or not anything arrives.
• The diff: the quarantine probe keeps (name, age, arrived) per body, reports the newest arrival beside the oldest age, suppresses the clause when one body makes them the same, and labels a negative age as future-dated rather than printing a nonsense duration. Helper functions are small and pure; the test loads send_failure_policy directly to park fixtures, which is the production writer, not a hand-made file.

One note for the record, not a blocker: Rui's approval is at 49d8ae70, an earlier head; keweichen's approval is at this head. I recommend approval; the merge is the owner's word.

Stand: Echo Act IV Mini