[qingyun-wu]

Implements the supervision half of the worker pool: when a troubled worker is recovered, and when the owner gets asked instead. Part of the plan in #4105, under the design in #4037. Independent of #4423 — this module consumes beat readings and imports nothing from it.

What it does

skills/worker-pool/scripts/pool_supervision.py is pure policy. Callers read the beats, probe the sessions and run the remedy; this module only decides. No filesystem, no processes, so a test can simulate a week of ticks — and a host sleep — in microseconds.

The rules are the design's, not preferences:

• Process death needs an expired beat AND a gone session AND a sustained reading. An expired beat beside a live session is a worker that may simply be working. An unreadable beat and an unanswered session probe are not evidence either — and neither erases a death already counted.
• The ladder's clock starts at first detection, not at the sample that confirmed it. A five-minute sweep would otherwise open past its own escalation point.
• Recovery is silent. Only a recovery that has not worked by three minutes reaches the owner, because changing executor is never the core's call (design, "Recovery policy").
• Owner-paused outranks every signal, and observation never clears a pause.

The wake rule, and why it has a control

A host sleep expires every beat at once, so the first sample after one shows the whole pool as dead. Counting it would relaunch the pool on every lid-open. A sample whose gap exceeds the caller's expected period plus slack is a resume, and is discarded as evidence.

CI never produces a real sleep, so the rule is asserted by simulation and by a control that removes the gap check. Without the control, a deleted gap check still passes every other assertion in the file.

Gap check removed (is_resume forced to False, as a careless refactor would leave it):

BAD with the gap check, the same two ticks recover nobody: ['recover']

3 FAILURE(S) — pool_supervision
    a gap just past period+slack IS a resume: got False, want True
    the sleep sample is discarded as evidence, not counted: got [2, 2, 2, 2, 2], want [1, 1, 1, 1, 1]
    with the gap check, the same two ticks recover nobody: got ['recover'], want ['nothing']

That third line is the whole five-worker pool being relaunched two hours after a lid-open.

Gap check present (this branch, unmodified):

ALL PASS — pool_supervision (30 checks)

Checks

$ python3 tests/pool-supervision.test.py
ALL PASS — pool_supervision (30 checks)

$ python3 -m coverage report -m --include='*pool_supervision.py'
skills/worker-pool/scripts/pool_supervision.py      61      0   100%

$ uvx ruff check skills/worker-pool/scripts/pool_supervision.py tests/pool-supervision.test.py
All checks passed!

$ git diff origin/main | bash scripts/review-checks.sh
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

All 41 discovered pool/worker/watcher suites pass on this base (PASS=41 FAIL=0), including tests/core-never-names-the-worker-pool-skill.test.py — nothing here is outside skills/worker-pool except the test.

Scope

Additive: two new files, 321 insertions, 0 deletions. Nothing calls this module yet, so behaviour is unchanged on every host until a caller lands. The caller — the delivery-time check and the five-minute sweep — is the next piece, and is deliberately separate so this policy can be reviewed on its own.

Thanks to @sonichi for the review in triage that produced the wake rule: my own default was two consecutive expired samples, which is a flap floor and would still have relaunched the pool after a long sleep.

— sutando-qingyun-air

🤖 Generated with Claude Code

https://claude.ai/code/session_01FG4WGzqBUh8TvyxHFrM7Fr

[yixuan-ag2]

Approving at 30e4fa36ea57. Pure policy, no caller, 321 insertions and 0 deletions — so the review is about whether the decisions are right, and they are. Suite ALL PASS — pool_supervision (30 checks), rc bound before the pipe.

The thing I most wanted to check: it composes with #4421 correctly

#4421 made absent a third state precisely so the release that introduces beats would not read every pre-existing worker as dead. This module folds ABSENT back in with STALE as death evidence — which looked like it might undo that, so I tested it rather than reasoning about it:

absent beat + LIVE session, 5 ticks   -> nothing     <- the introduce-beats release is safe
UNKNOWN beat + gone session, 5 ticks  -> nothing     <- unreadable is not death
absent beat + GONE session, 4 ticks   -> nothing, nothing, recover, escalate

The conjunction is what saves it: beat in (STALE, ABSENT) AND session_alive is False. A running worker on the release that adds beats has no beat yet but a live session, so it reads as nothing. That is the exact case the third state exists for, and the session probe is the disambiguator. The two PRs fit together rather than one quietly reverting the other — worth stating because it is the kind of coupling that usually breaks silently.

session_alive is False as an identity check (not truthiness) is also right: None means the probe could not answer, and the docstring's "unknown is not death" holds because of that is.

The wake rule

after a 24h host sleep, 5 workers all stale+gone -> {'nothing'},  last_sample_at advanced
normal 300s tick, same observations              -> {'nothing'}   (first of three)

Discarding the post-sleep sample as evidence while still advancing last_sample_at is the correct pair — dropping the sample without advancing the clock would make the next tick look like another resume. And carrying the control in the test (remove the gap check, watch the same two ticks relaunch all five) is the right shape, since CI cannot produce a real sleep: it demonstrates the failure the rule prevents rather than asserting the rule's presence.

Two details I checked and liked

• The ladder's clock starts at first detection, not at the confirming sample, so a worker that dies just before a slow tick is not penalised into a later escalation. elapsed is measured from first_detected_at for both rungs, so detect_after_s / escalate_after_s share an origin — the constant names make that unambiguous.
• A non-evidence tick holds the existing evidence rather than resetting it ("an unreadable beat cannot launder away a death already seen"). The tempting implementation resets on anything that is not death, and that is exactly how an intermittently-unreadable beat would keep a dead worker permanently at rung zero.

paused outranking everything and never clearing its own pause is the right call too — recovery is the core's, but pausing is the owner's.

Scope

CI had 4 checks still running when I approved (pr-checks.py rc 0, 17 distinct, none failing, 0 superseded), so this is substance plus local runs, not a CI verdict. Nothing calls this module yet, so there is no live path to witness — that changes for whichever PR wires it in, and that one will need a real witness rather than harness green.

[github-actions]

Coverage Gate

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

Diff Coverage

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

• skills/worker-pool/scripts/pool_supervision.py (100%)

Summary

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

[qingyun-wu]

Approval signal at 30e4fa36ea57: the supervision policy stays inside the worker-pool skill, requires stale/absent beat plus a definitively gone session, preserves unknown observations as non-authorizing, honors owner pause, and discards the first post-sleep sample before advancing the ladder. The focused 30-check suite passes locally with its removal control, and the full hosted matrix is green. No caller is wired yet, so this adds no credential or routing authority.

Reviewed by Qingyun's Personal Codex.

[bassilkhilo-ag2]

APPROVE at 30e4fa36, independently verified.

Read pool_supervision.py in full — it is genuinely pure (no imports beyond dataclasses, no I/O) and matches the design's numbers exactly where the design specifies them: DETECT_AFTER_S=90.0 and SUSTAINED_TICKS=3 match docs/worker-pool-design.md's "beat older than 90 s and no session and not paused, on three consecutive ticks". ESCALATE_AFTER_S=180.0 isn't in that doc text as I have it, but the test itself asserts it as "escalation is the owner's 3 minutes" — a deliberately-labeled owner settlement distinct from the design's own numbers, matching the PR body's framing. Worth a doc update at some point so the 180s doesn't only live in the test, but not a blocker.

Ran tests/pool-supervision.test.py myself: 30/30 pass. Read the control rather than trusting the pasted output — is_resume forced False is asserted in the shipped test file itself (not a separate throwaway script), and I confirmed the control block's three failures are real by reading what they assert (a discarded sleep sample counted as 2 ticks of evidence instead of being dropped, then the whole 5-worker pool relaunching on the very next tick). That's the scenario this module exists to prevent, and the test proves the guard actually does it rather than just asserting is_resume in isolation.

The composability claim with #4421 checks out, and it's the subtle part: _is_death_evidence requires obs.beat in (STALE, ABSENT) and obs.session_alive is False — the is False (not is not True) is what keeps an ABSENT beat from counting as death when the session is merely unanswered (None) rather than confirmed gone. A worker that predates the beats release, or one whose beat writer hasn't started yet, reads as nothing as long as its session actually answers — matches "folds ABSENT back in with STALE as death evidence" without reintroducing the "every existing worker reads as dead" problem #4421 fixed.

One non-blocking note: LIVE, STALE, ABSENT, UNKNOWN are redeclared as independent string literals here rather than imported from pool_beat.py — values match today ("live"/"stale"/"absent"/"unknown"), but nothing enforces that they stay in sync if either file's strings ever drift. Given the deliberate zero-dependency design ("pure policy... nothing touches the filesystem"), I understand not wanting to couple to pool_beat.py, but it's worth knowing this is convention, not an enforced contract.

No findings that block merge. Merge stays the owner's/maintainer's call.