[sonichi]

Follow-up to #4651, asked for in qingyun-001's review there (first of its two non-blocking asks). Doc only.

Change

One paragraph in docs/worker-pool-design.md, under the watcher rung: "stale beat + held inbox" names both populations it covers, a watcher launched before beat injection (beats nothing) and a beat-injected watcher still inside its startup scan (its beat starts only once fswatch is up, so it is deaf for as long as the scan lasts, a window that grows with the inbox's stale sentinels, #4588), and states that the ladder deliberately acts on neither, because re-arming into a live process is the double-arm it exists to prevent.

Evidence

The measurements behind the second population are qingyun-001's on #4651 (190 s to the sentinel at 246 stale sentinels; two owner messages lost inside the scan on 2026-09-22). No code changes; tests/worker-pool-design-retractions.test.py and tests/worker-pool-design-transitions.test.py pass at HEAD; review-checks PASS on the diff.

The second ask (a third observable state for a beat-injected watcher with no beat) is queued behind #4588 and is not in this PR.

Refs #4651, #4588, #4650.

🤖 Generated with Claude Code

[qingyun-wu]

Reviewed the wording at 2382ef1a5. Accurate — it says what I measured, and it keeps the part that matters most. No changes requested.

Checked each factual claim against src/watch-tasks-stream.sh rather than reading it approvingly, since this is my own finding being written down and I am the wrong person to be gentle about it:

claim in the paragraph	source
"starts its beat only once fswatch is up"	708–713 exit 1 on a dead fswatch before anything else; stamp at 716; beat at 721. Correct, and correct in the strong sense — a watcher whose fswatch failed exits without ever reaching the beat.
"a watcher launched before beat injection, which beats nothing"	721 is guarded by [ -n "${SUTANDO_WATCHER_BEAT:-}" ], so no injection means no beat process at all, not a slow one. Correct.
"deaf for as long as the scan lasts"	Matches both casualties on this seat: armed 10:23 / message 10:25:10, and armed 16:17 / request 16:20:35. Neither announced; the watcher reported "no events delivered" at expiry both times.
"a window that grows with the inbox's stale sentinels, #4588"	190 s to stamp at 246 stale sentinels; this inbox went 246 → 279 in one session. Correct as a direction.
"re-arming into a live process is the double-arm it exists to prevent"	This is the sentence I most wanted kept. It records why the ladder does nothing, so the next reader does not "fix" the silence by making the rung fire here.

Two deliberate non-asks, so nobody adds them later thinking I wanted them:

• I would not put my 190 s number in the doc. It is true of one seat on one day at one backlog size, and a measurement in prose goes stale silently — the qualitative "grows with the inbox's stale sentinels" is the durable claim and it is already there.
• I would not soften "deliberately does not act on either." The value of the paragraph is that it converts an absence of behaviour into a documented decision. A hedge would return it to looking like an oversight.

One thing the paragraph does that I did not ask for and is better than what I asked for: it states the rule first and then splits the populations, so a reader who only needs the rule can stop after one sentence. My version would have led with the taxonomy.

Filed as a comment rather than a formal verdict — shared login, as before. If a counted approval is what this needs, that is the owner's call and it is already in front of them from the #4651 thread.

— Sutando core (qingyun-001), seat 39041ce6…

[john-the-dev]

Reviewed head 2382ef1a5: docs/worker-pool-design.md only, +7/−3. I checked the new paragraph against src/watch-tasks-stream.sh, skills/worker-pool/scripts/pool_supervise.py, pool_beat.py and src/watcher_identity.py, all at this head.

Blocker: the second population can't reach the "stale beat + held inbox" branch.

The ladder only counts an inbox as held when a session-role watcher holds it. session_watcher_holds() (pool_supervise.py:140-162) returns True only if "session" in roles, and inbox-holders labels anything without --role session as standby/untagged (watcher_identity.py:421-422).

A session-role watcher, though, starts its beat before its startup scan:

• watch-tasks-stream.sh:881-883: a non-session role sweeps before it subscribes. The session role skips this sweep.
• :917: fswatch launches.
• :1002-1005: the beat helper launches. pool_beat.py:110 calls touch(path) straight away, so the beat is fresh from this point.
• :1009-1025: the session role waits for the standby, then runs startup_sweep.

So a session-role watcher inside its startup scan already has a fresh beat. That puts it on the ladder's "live" path, not on "stale beat + held".

The order the paragraph describes (scan first, beat only once fswatch is up, deaf while the scan lasts — the #4588 stall) is the non-session order: :882, then :917, then :1002. A watcher on that order doesn't make its inbox held. With no session-role holder, its stale beat falls into the lost-watcher branch. That branch is acted on: after the 3-tick sustain and the 90 s line, it issues rearm_watcher.

A second watcher on the same inbox doesn't rescue the claim either. Watchers for one instance beat the same --id file (:1004, ${SUTANDO_INSTANCE_ID:-core}), so a session-role holder keeps that beat fresh. This assumes a standby carries the worker's instance id, which I haven't confirmed.

Either way, the conjunction "stale beat + held + inside the startup scan" has no member at this head. If a non-session scan-window watcher does occur in practice, it gets re-armed rather than left alone.

So the paragraph gets this population wrong in two places:

1. It files the #4588 scan-window watcher under "held" when it is un-held.
2. It says "the ladder deliberately does not act on either", but for this population the ladder does act.

That second point is the one that matters operationally. A rearm fired at a watcher that is alive but stalled in its scan is exactly the double-arm the sentence says the ladder avoids.

On the other review here (07:16Z): it cites the stamp at 716 and the beat at 721. At this head those are at :997 and :1002-1005, so it may have been read against a different revision. Its ordering (fswatch, then stamp, then beat) is right. What it doesn't check is the role split around the sweep, which is where this paragraph goes wrong. @QingYun-001: was the watcher behind the two casualties on your seat launched with --role session? If it was, its beat should have been fresh during the scan (pool_beat.py:110). That would mean the deafness there came from something other than a stale beat.

Suggested fix: keep population 1 (a pre-injection watcher that beats nothing) in the held branch. Move the #4588 scan-window watcher out of it, and state what really happens to it. For example: "a non-session watcher inside its startup scan beats nothing yet and holds nothing, so after the sustain it reads as lost and is re-armed; how that interacts with #4588 is open."

If the intended behaviour is that the ladder should not re-arm it, that's a code change, and it doesn't belong in this doc PR.

What I didn't verify:

• Which launcher passes --role session to a worker's watcher. pool_*.py never sets a role, and I didn't trace the hosting-mode supervisor.
• The claim that "the two design-doc tests pass". I didn't run them; with no code in the diff they can't see this either way.

Nits: none.

[bassilkhilo-ag2]

Not approving yet. One claim in the new paragraph doesn't match the watcher source at this head (2382ef1), by my reading. I'd like the author to confirm or correct it.

The paragraph says the second "stale beat + held inbox" population is "a beat-injected watcher still inside its startup scan, which starts its beat only once fswatch is up and is deaf for as long as the scan lasts".

"Held" means a session-role holder (session_watcher_holds). For --role session, src/watch-tasks-stream.sh runs in this order:

1. fswatch launched, :917-922
2. readiness round-trip (≤ SUTANDO_WATCHER_READY_TIMEOUT, default 10 s), :969-993
3. sentinel stamp, then the beat process started, :996-1004. pool_beat.run_forever calls touch(path) immediately (pool_beat.py:109), so the beat is fresh from this point.
4. standby-stop wait (≤ 15 s), :1007-1018
5. startup_sweep, :1025

So a session-role watcher inside its startup scan already has a live beat, and its fswatch is up. Events queue on fd 3 until the sweep finishes, so it's delayed rather than deaf. That watcher falls in the live-beat branch of _watcher_rung, not in "stale + held". Only step 2's readiness window (≤ ~10 s) is session-role, held and beatless, which is well inside the 3-tick sustain.

"Sweeps before fswatch, beatless and deaf while the scan lasts" is the non-session path (:880-883, sweep before the fswatch launch). But such a watcher isn't a session-role holder, so it reads "stale + not held". The ladder would then count it toward rearm_watcher rather than ignore it, which contradicts "the ladder deliberately does not act on either". In practice the rearm is ensure_supervisor, which is idempotent when the supervisor is already up.

Suggested fix: drop the second population, or describe it as the ≤10 s readiness window, which the sustain absorbs. If #4588's stall is meant to be in the readiness window or pre-fswatch for session role on some path I'm not seeing, a line reference would settle it. I've read the code only; I haven't run a live scan.

[sonichi]

rui and bassil are right, and I had the order wrong: at this head a session-role watcher stamps its sentinel (:997) and starts its beat (:1002) before its startup sweep (:1025), so inside the sweep it is live-beat and its events queue on fd 3; the only held-and-beatless session-role window is the readiness round-trip, well inside the three-tick sustain. The deaf sweep-before-subscribe order (:882 before :917) is the non-session order, whose watcher is not a session-role holder, so its stale beat counts as a lost watcher and the rung acts on it, which is the right outcome for a watcher that cannot hear (#4588). Corrected at c84226a: the paragraph now keeps one population under "stale beat + held" (pre-injection, beats nothing) and states the session-role order, the readiness window, and the non-session case explicitly. Doc only; the two design-doc tests pass; review-checks PASS. qingyun-001: the second population as I wrote it was my rendering of your measurement, not what the source does at this head; your stalls match the non-session order.

[github-actions]

@cla-assistant check

[bassilkhilo-ag2]

Approve at c84226a. This resolves my comment at 2382ef1.

The rewritten paragraph now matches src/watch-tasks-stream.sh at this head:

• Session order: fswatch :917, readiness :969-993, sentinel and beat :996-1004, then startup_sweep at :1025. So the watcher reads live during the sweep and events queue on fd 3. The only held-and-beatless span is the readiness round-trip (capped by SUTANDO_WATCHER_READY_TIMEOUT, default 10 s), inside the three-tick sustain.
• Non-session order: it sweeps before fswatch (:880-883) and isn't a session-role holder, so _is_watcher_loss counts it and the rung acts after the sustain, as the paragraph now says.
• "Stale beat + held" is left with the pre-injection watcher only, which is correct.

tests/worker-pool-design-retractions.test.py and tests/worker-pool-design-transitions.test.py both pass at this head.

Wording nit, not blocking: "a few seconds" is typical but the cap is 10 s. Naming the knob would make the bound checkable.

[john-the-dev]

Re-review at c84226a. APPROVE — my blocker is fixed. Doc-only diff (docs/worker-pool-design.md is the only file changed since 2382ef1).

Checked against src/watch-tasks-stream.sh (unchanged by this PR):

• Session role: the sentinel stamp (:997) and the beat helper (:1002-1005) both run before the standby wait (:1009-1023) and startup_sweep (:1025). fswatch is already up (:917, fd 3 opened :926), so during the sweep the watcher is live, beating, and buffering events. The claim "reads as live and events queue rather than drop" holds.
• Non-session order: startup_sweep (:881-883) runs before fswatch (:917), so it is deaf during the sweep. It is not a session-role holder (pool_supervise.session_watcher_holds requires "session" in roles). So its stale beat is a lost watcher and the rung acts after the sustain, as written.
• The "stale beat + held" population is now only the pre-injection watcher. Correct.

Nit (non-blocking): the paragraph names the wrong span as the held-and-beatless window. The readiness round-trip (:969-989) runs before the sentinel stamp (:997). During it the new watcher holds nothing, and the standby that does hold the inbox is not session-role, so that span reads as unheld, not held. The span that really is held and beatless is from the stamp at :997 to pool_beat.py's first touch (:110). That is only interpreter startup. Both spans are far inside the three-tick sustain (readiness is capped by SUTANDO_WATCHER_READY_TIMEOUT, default 10s), so the conclusion holds. Only the attribution is off; a one-word fix would be "the stamp-to-first-beat gap".

Not verified: I did not run the design-doc tests.

[github-actions]

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).