[qingyun-wu]

Stack #4106 → #4107 → #4109 → #4108 → #4110 — this is layer 1 of 5. Parent: —; base branch main. Merge in that order; each PR's diff is exactly its own layer (restacked 2026-09-10 after review found #4108/#4110 vendoring other layers' files).

What this is

Stage one of the worker pool per docs/worker-pool-design.md: the delivery mechanism alone, no routing. Plan and checklist at #4105.

The design's sequencing rule is explicit about why this comes first:

first improve the single-core delivery mechanism alone ... verified against session exit, watcher restart, a missed notification and a stalled consumer; then add routing on top. Sequenced this way the pool release changes routing decisions and nothing underneath them.

What it implements

The recipient half of the specified layout:

tasks/<task-id>.json                         payload, immutable after admission
deliveries/<recipient-id>/<task-id>          sentinel, 0 bytes, unclaimed
deliveries/<recipient-id>/<task-id>.claimed  sentinel, 0 bytes, claimed

Creating a sentinel assigns — the router's job, deliberately not in this module. Renaming claims: os.rename is atomic and exclusive, so a loser sees OSError and walks away. No lock, no lease, no coordinator.

residue reads what a crash left behind from the filesystem alone, with no journal. sweep maps each state to exactly one action, and an unmapped state raises rather than falling through — that fallthrough is what hid the second defect below.

state	condition	sweep does
unclaimed	sentinel, payload live	ready
died-mid-work	claimed, no result	release to the same recipient
completed	result, no flag	leave for the owner to finish
finished	result + flag	retire the sentinel
stale-sentinel	sentinel, payload gone	drop it
undelivered / clean	no sentinel	not reachable from a sweep

This serves the core, not just workers

Per the design, "the core is a recipient with a watcher like any other". The core reads deliveries/core/ from the first install, so there is no protocol switch when the first worker appears or the last one leaves. That is why this lands useful before any worker exists.

The structural guarantee

New files only. src/watch-tasks-stream.sh is untouched — the core's executed code is byte-identical, and you can verify that from the diff rather than taking my word:

$ git diff --name-only origin/main...HEAD -- src/watch-tasks-stream.sh
(empty)

Defects the tests found that reading did not

• the id pattern accepted a dot, so a payload name dropped in a delivery folder parsed as a valid sentinel;
• a task with a result and a done-flag whose payload was not yet archived read as undelivered. Since nothing retired the sentinel on completion, the happy path would have handed finished work back as ready and re-run it — not an edge case;
• residue and sweep used one name for two states, which is what hid the second defect.

Evidence

$ python3 tests/pool-delivery.test.py
Ran 42 tests in 0.175s
OK

Mutations verified to fail them — each breaks a distinct test:

mutation	result
claim appends the suffix instead of substituting	caught
sweep ignores a completed task	caught
recipient folder hardcoded to core	caught
finished falls through to ready (the original bug)	caught by test_finished_work_is_retired_not_handed_back

scripts/review-checks.sh PASS on the cumulative diff.

Why draft

Stage one is not finished. The design names four verifications this does not yet exhibit: session exit, watcher restart, a missed notification, a stalled consumer. This is tested against a real temp filesystem, but not against a live recipient surviving a restart. Those are the next commits on this branch, not a follow-up PR.

Refs #4105, #4037

[qingyun-wu]

Stage-one verification: the four cases the design names

Run against a real workspace with a minted worker (uuid4().hex, per the id decision in #4037), not a unit-test temp dir.

session exit — worker claims, session dies holding it:

residue: died-mid-work

watcher restart — the reconciling sweep on the next boot:

sweep: {"ready":[],"released":["task-hello"],"completed":[],"retired":[],"stale":[]}
pending after restart: task-hello

Released to the SAME worker. No other party may take it.

missed notification — a sentinel created with nothing signalling; only the sweep finds it, and it retires the finished one in the same pass:

sweep: {"ready":["task-quiet"],"released":[],"completed":[],"retired":["task-hello"],"stale":[]}

completion ordering — result, then flag:

after result:  completed
after flag:    finished

cross-worker isolation — a second minted worker sees none of it:

worker-2 pending: []
worker-2 sweep:   {ready: [], released: [], completed: [], retired: [], stale: []}

A limitation this surfaced — not fixed here

stalled consumer — a claim two hours old with no result:

pass 1: {"released":["task-quiet"], ...}
pass 2: {"ready":["task-quiet"], ...}
pass 3: {"ready":["task-quiet"], ...}

Nothing is stolen — only the owning recipient can see that folder, and age authorises nothing (stale is not dead; release keys on abandoned, which the core sets). That part is correct.

But pass 1 released a claim that a live worker might still be holding. From the filesystem alone, died-mid-work and working-right-now are the same state. The design contains this by saying recovery happens at boot — "a worker reads its own folder at boot" — but this module does not enforce boot-only, so a sweep called mid-work releases the caller its own in-flight claim.

The fix is already named in #4037: incarnation_id. A claim recording its incarnation lets a boot sweep release only claims from a previous incarnation and leave the current one alone. That makes the sweep safe to run at any time instead of correct only by convention.

Not in this PR — it needs the incarnation field, which lands with the worker lifecycle. Filing it here so the constraint is visible rather than implicit.

[github-actions]

Coverage Gate

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

Diff Coverage

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

• src/pool_delivery.py (98.3%): Missing lines 217,251-252

Summary

• Total: 174 lines
• Missing: 3 lines
• Coverage: 98%

src/pool_delivery.py

Lines 213-221

  213     actions = {"ready": [], "released": [], "completed": [], "retired": [], "stale": []}
  214     for p in accepted(ws, recipient) + pending(ws, recipient):
  215         task_id = parse_sentinel(p.name)[0]
  216         if task_id in seen:
! 217             continue
  218         seen.add(task_id)
  219         state = residue(ws, recipient, task_id)
  220         if state == "pending":
  221             actions["ready"].append(task_id)

---

Lines 247-256

  247     if not p.is_file():
  248         return None
  249     try:
  250         return p.read_text(encoding="utf-8", errors="replace")
! 251     except OSError:
! 252         return None
  253 
  254 
  255 def _emit(task_id: str) -> None:
  256     # A dead consumer must end the reader rather than let events pile up unseen.

---

[sonichi]

Reviewed at f74ad3a8. One finding, verified by running it; the rest is scope.

The coverage gate is not cosmetic here — all three uncovered lines are the failure-mode branches. 77.0% vs the 95% bar, missing 175, 194, 210:

• 175 continue — the dedup when a task_id is in both claimed() and pending(). That overlap is the reason the seen set exists, and nothing exercises it.
• 194 raise AssertionError(f"unmapped residue state ...") — the fail-closed guard. It is the right shape, and no test has ever made it fire, so "the mapping is total" rests on inspection.
• 210 try: in _emit — below.

_emit does not do what its comment says, and I only found it because my first probe was wrong.

Line 209 reads "A dead consumer must end the reader rather than let events pile up unseen." It ends the reader — but not cleanly. Measured on the verbatim shape at this head:

5 emits,     head -1   ->  REACHED-END-NO-BREAK, exit 0, stderr clean
20000 emits, head -1   ->  EXCEPT-BRANCH-RAN
                           Exception ignored while flushing sys.stdout:
                           BrokenPipeError: [Errno 32] Broken pipe
                           exit 120

raise SystemExit(0) does not yield exit 0. The interpreter flushes sys.stdout on the way out, that flush fails on the dead pipe, and the process exits 120 with a BrokenPipeError on stderr. A supervisor gating on the exit status sees a failure where the code intends a clean stop.

Worth stressing why the first line of that table exists: five short lines fit inside the 64KB pipe buffer, so head exiting broke nothing and the probe returned a clean result. I nearly posted "the handler is fine" on the strength of a control that never produced a positive. The uncovered line and the inadequate probe are the same gap — you cannot see this branch without forcing the buffer past 64KB, which is also what a test for line 210 has to do.

Remedy, verified against the same 20000-emit case:

except BrokenPipeError:
    try: os.dup2(os.open(os.devnull, os.O_WRONLY), sys.stdout.fileno())
    except Exception: pass
    raise SystemExit(0)
# -> exit 0, stderr clean

Scope of this review, stated so you can price it: #4106 only, at f74ad3a8. I read #4107 (97.9%, missing 114/170), #4108 (98.6%, missing 25) and #4109 for coverage and size only — I have not reviewed their logic, and I am not implying they are clean. If you want the same treatment on #4108 (2607 lines, 23 files) say so and I will do it as its own pass.

I have not recorded a formal review action — that authority is the owner's on this repo.

Stand: Echo Act IV Pro

[qingyun-wu]

Changes requested (this GitHub identity owns the PR, so a formal REQUEST_CHANGES review is unavailable). The current 51 tests and added-path scan pass, but the executable watch loop strands recovered deliveries.

watch, unclaimed task-x at boot  -> DELIVERY: task-x
watch, claimed task-x at boot    -> no event; pending contains task-x

This ran the actual CLI against a scratch filesystem. sweep releases the interrupted claim, but main emits only ready and then includes the released task in announced. It is excluded from every later delta until another restart. The added tests at this head do not change that implementation. Announce recovered work as well, and add the interrupted-claim CLI control. Also use the shared ready-result predicate: empty and whitespace result files currently classify as completed just like a real answer, suppressing unfinished work.

Worst case is lost notification or premature completion of an owner task. The branch remains draft with its session-exit/watcher-restart/stalled-consumer acceptance still owed.

Reviewed at a781602f89dc166eee2b9245e933306839e35fae. No contributor branch or live runtime was changed.

[qingyun-wu]

Changes requested at f734a571cab97a1e4b7f99849d206d448d1495d8 (formal REQUEST_CHANGES is unavailable on my own PR). The .txt sentinel change does not resolve the two existing blockers; re-ran the actual watch CLI using the new names:

unclaimed task-x.txt at boot -> DELIVERY: task-x
claimed task-x.claimed at boot -> no event; pending = [task-x.txt]
result body empty / whitespace / done -> completed in all three cases

src/pool_delivery.py:242-244 still emits only the sweep's ready bucket and then marks released work as already announced. :151 still uses result-file existence instead of the shared readiness predicate. Include released work in startup notifications and delegate result readiness to its existing owner; add the interrupted-claim CLI and unready-result controls. These update the existing threads rather than creating duplicates.

51 tests and the added-line path scan pass; the scanner rejects a synthetic forbidden host path. Worst case remains stranded work or premature completion. The draft's real session-exit/restart/missed-event/stalled-consumer acceptance remains owed. CI and CLA are green in the inspected snapshot. No contributor branch or live runtime was changed.

[qingyun-wu]

Changes still required at 2153fa48. This head incorporates the merged current-track fix; the delivery implementation and its two open findings are unchanged. Re-read the full topic diff and reran the production CLI in an isolated workspace:

watch, initially unclaimed: DELIVERY: task-x
watch, initially claimed:   <no event>; pending=['task-x.txt']
result empty:               completed
result whitespace-only:     completed
result ready ('done'):      completed
pool-delivery.test.py:      51 tests, OK

The blockers remain src/pool_delivery.py:240-242 (boot recovery seeds a released task into announced without emitting it) and :151 (file existence substitutes for result readiness). Worst case: interrupted work stays undispatched or an empty result suppresses recovery. Emit recovered deliveries and use the shared ready-result predicate, with production-path regression tests. The four live recovery witnesses listed in the draft remain owed. No live service was changed; added-path scan passes. This is a COMMENTED review because this login authored the PR.

[qingyun-wu]

Changes still required at 2153fa48. This head incorporates the merged current-track fix; the delivery implementation and its two open findings are unchanged. Re-read the full topic diff and reran the production CLI in an isolated workspace:

watch, initially unclaimed: DELIVERY: task-x
watch, initially claimed:   <no event>; pending=['task-x.txt']
result empty:               completed
result whitespace-only:     completed
result ready ('done'):      completed
pool-delivery.test.py:      51 tests, OK

The blockers remain src/pool_delivery.py:240-242 (boot recovery seeds a released task into announced without emitting it) and :151 (file existence substitutes for result readiness). Worst case: interrupted work stays undispatched or an empty result suppresses recovery. Emit recovered deliveries and use the shared ready-result predicate, with production-path regression tests. The four live recovery witnesses listed in the draft remain owed. No live service was changed; added-path scan passes. This is a COMMENTED review because this login authored the PR.

[qingyun-wu]

Changes still required at 2153fa48. This head incorporates the merged current-track fix; the delivery implementation and its two open findings are unchanged. Re-read the full topic diff and reran the production CLI in an isolated workspace:

watch, initially unclaimed: DELIVERY: task-x
watch, initially claimed:   <no event>; pending=['task-x.txt']
result empty:               completed
result whitespace-only:     completed
result ready ('done'):      completed
pool-delivery.test.py:      51 tests, OK

The blockers remain src/pool_delivery.py:240-242 (boot recovery seeds a released task into announced without emitting it) and :151 (file existence substitutes for result readiness). Worst case: interrupted work stays undispatched or an empty result suppresses recovery. Emit recovered deliveries and use the shared ready-result predicate, with production-path regression tests. The four live recovery witnesses listed in the draft remain owed. No live service was changed; added-path scan passes. This is a COMMENTED review because this login authored the PR.

[qingyun-wu]

Changes still required at 2153fa48. This head incorporates the merged current-track fix; the delivery implementation and its two open findings are unchanged. Re-read the full topic diff and reran the production CLI in an isolated workspace:

watch, initially unclaimed: DELIVERY: task-x
watch, initially claimed:   <no event>; pending=['task-x.txt']
result empty:               completed
result whitespace-only:     completed
result ready ('done'):      completed
pool-delivery.test.py:      51 tests, OK

The blockers remain src/pool_delivery.py:240-242 (boot recovery seeds a released task into announced without emitting it) and :151 (file existence substitutes for result readiness). Worst case: interrupted work stays undispatched or an empty result suppresses recovery. Emit recovered deliveries and use the shared ready-result predicate, with production-path regression tests. The four live recovery witnesses listed in the draft remain owed. No live service was changed; added-path scan passes. This is a COMMENTED review because this login authored the PR.

[qingyun-wu]

Changes requested at 10ce35a7 (COMMENTED because this login authored the PR). The .txt/header-text payload change is correct: the handler-to-recipient probe now reads the admitted text and keeps the delivery ready. The earlier recovery blockers are unchanged:

• [P1] Announce recovered claims — src/pool_delivery.py:247-249. sweep releases an interrupted claim, but watch emits only ready and immediately marks every pending name as already announced. Running the real CLI with a real .txt payload:

  initially unclaimed -> DELIVERY: task-x
  initially claimed   -> <no event>; pending=['task-x.txt']
  

  Include the released deliveries in startup notification and pin the claimed-at-boot path through main/watch.

• [P2] Reuse result readiness — src/pool_delivery.py:151-159. An empty file, whitespace-only file and actual answer all still produce completed; placeholders prevent recovery. Delegate to delivery.readiness.read_ready_result and cover unready bodies instead of equating existence with completion.

Full three-file diff and surrounding lifecycle inspected; 51 tests pass and added-path checks pass with a rejected forbidden-path control. The new module has no default runtime wiring, so today's installs are unaffected. Once activated, the worst case is interrupted work silently stranded or falsely completed; the two repairs above and the acknowledged live acceptance cases are still needed. The live restart/consumer tests remain outstanding, as the draft body already states.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Changes requested at 5b619b64 (COMMENTED because this login authored the PR). The lane-ID repair works:

previous 10ce35a7 parse('task-local-hs~task-1.txt'): None
current 5b619b64: ('task-local-hs~task-1', False)
current suite: 52 tests, OK

The two existing recovery blockers remain at src/pool_delivery.py:249-251 and :153-161. Production CLI probes still give:

initially unclaimed: DELIVERY: task-x
initially claimed:   <no event>; pending=['task-x.txt']
empty result: completed; whitespace result: completed; real answer: completed

Emit the boot sweep's released deliveries before populating announced, and reuse the shared result-readiness predicate rather than file existence. Keep the existing threads; these are confirmations at the new head, not new findings.

Full diff and lifecycle reviewed; path scan and forbidden-path control pass as expected. The reported live happy-path run on #4105 supports lane handling but does not exercise either crash-recovery defect. On activation, the worst case is stranded interrupted work or premature completion; recovery-path tests and the outstanding live acceptance cases are still needed. No contributor branch or live runtime changes.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Changes requested at 3246495f. The new done-flag rule is correct: with a claim, payload and flag but no result, production residue changes from died-mid-work at 5b619b64 to finished; 53 tests pass.

The original two blockers remain: src/pool_delivery.py:251-253 still emits only boot-sweep ready, then marks released claims announced without emitting; :153 still treats an empty or whitespace-only result file as completed. The real CLI emits DELIVERY: task-x for initially unclaimed work, but emits nothing for initially claimed work despite pending=['task-x.txt']. Empty, whitespace and actual answers still all classify completed when no flag exists.

Emit released deliveries on startup and use the shared readiness predicate. Worst case is stranded interrupted work or premature completion. The new flag fix closes a different recovery case and does not resolve these threads. Full three-file diff reviewed; path scan passes; no contributor branch or live runtime changes. COMMENTED because this login authored the PR.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Changes requested at f6eed3ad. COMMENTED because the authenticated account authored this PR.

The rename to .accepted/accept() does not repair the two existing blockers:

• [P1] Boot recovery is silent (src/pool_delivery.py:256-258). The production watch CLI emits DELIVERY: task-x for an initially pending sentinel; after accepting that sentinel and restarting the CLI, it emits nothing although pending() contains task-x.txt. Announce the sweep's released deliveries before seeding announced, and cover this actual CLI path.
• [P2] Result existence is still treated as readiness (:159-169). Empty, whitespace-only and substantive result bodies all classify as completed. Use the shared delivery-readiness contract so an unfinished result cannot suppress recovery.

53 delivery tests pass, but the isolated failure probes above still reproduce. Worst case on activation: interrupted work remains silent or is mistaken for completed work. These two repairs remain required for approval.

Current diff, relevant surrounding code and existing threads reviewed. Added-path scan passes; an injected forbidden-path control fails. CI/CLA freshness checked: CLA green, hosted checks still running. No live runtime or contributor branch changed. Stack order: #4106 → #4107 → #4109 → #4108 → #4110; recheck after parent synchronization.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Why the legacy suffix and the flag-alone rule are one story

Both fixes in this PR came out of the same live pool, and they would have compounded. Recording the reasoning, because neither is obvious from its own diff.

The compounding path, had a live task hit the rename window:

1. the .accepted rename lands while 15 sentinels are already named .claimed
2. find() checks .txt and .accepted only          -> accepted work reads as never delivered
3. the router's both-names guard relies on find() -> it writes a second sentinel
4. residue() sees a fresh pending sentinel + a done flag
   WITHOUT the flag-alone rule: the gateway had already drained results/<id>.txt,
   so has_result is False -> "died-mid-work" -> sweep RELEASES it -> the worker
   accepts it again and answers the room twice
   WITH it:                    the flag is terminal -> "finished" -> archived, never re-run

So the flag-alone-is-terminal fix is the thing that would have contained a bug the legacy-suffix fix now prevents outright. Defence in depth that was not designed as such — worth keeping both.

What actually happened, stated precisely so nobody reads more into it: no live task went through the window. Verified from disk — the one task that got a duplicate sentinel had its payload archived at 23:58, before the window opened, and the router only routes what it finds in tasks/; the dispatch dir shows zero handler activity for that id. I created that sentinel by calling deliver_one() directly while checking an unrelated question. The hole is real and provable by construction; it did not fire in production.

A third finding from that same bad call: deliver_one() writes a sentinel without checking the payload exists. Harmless under the design — the router only runs on admitted tasks — but it is what let a hand-call create a sentinel pointing at nothing. Not fixed here; noting it.

Measurement for the finish gap (from the live worker's folder, 17 sentinels): every one returns residue finished, none retired, because finish does not exist and completion is done by hand. Step 3 — remove the sentinel — is exactly what would have left nothing in that folder for the rename to trip over. The litter and the hole share a cause.

[qingyun-wu]

Changes requested at b5d78521. COMMENTED because the authenticated account authored this PR.

The legacy .claimed compatibility repair is verified: the current 54-test suite fails one assertion against the previous delivery source and passes at this head. find, accepted and pending now recognize the old name correctly.

The two existing blockers remain on this head:

• [P1] Boot recovery stays silent (src/pool_delivery.py:258-260). Actual watch CLI: pending sentinel → DELIVERY: task-x; accepted sentinel → <no event>, although recovery leaves task-x.txt pending. Announce released deliveries before seeding announced; include legacy .claimed in that CLI regression matrix.
• [P2] File presence is not result readiness (src/pool_delivery.py:163-173). Empty, whitespace-only and substantive results all return completed. Use the shared readiness contract before suppressing recovery.

Worst case remains interrupted work silently stranded or mistaken for completed work. Both repairs are required for approval; the new compatibility test does not cover either failure.

Reviewed the current diff, surrounding code and existing threads. Added-path scan passes; an injected forbidden-path control fails. Current hosted checks and CLA are green. Stack order: #4106 → #4107 → #4109 → #4108 → #4110; recheck after parent updates. No contributor branch or live runtime changes.

Reviewed by Qingyun’s Codex automation.

[qingyun-wu]

Changes requested at 64f4f5406a3fb6d723a6a89fab4e50a38e9bd43f (self-author COMMENTED verdict).

This is an update from main: all three PR files and the imported readiness/workspace helper files are byte-identical to bbda4d79b. The 60 delivery tests pass, but the open wakeup race at src/pool_delivery.py:291 survives the integration.

I reran the production watch loop with a deterministic arrival during boot emission and a later arrival as a positive control:

current: emits [task-boot, task-control]; task-racing remains pending
seed announced from emitted boot ids only: emits [task-boot, task-racing, task-control]

Seed announced from the events actually emitted rather than a fresh directory listing, and pin this interleaving. No duplicate inline finding; the existing P1 remains actionable. Worst case is a delivered task sitting indefinitely without waking its recipient.

Full current three-file diff and surrounding shared-readiness path inspected. Path/root/prose scans pass with a forbidden-path rejecting control; the current CLA is green and Python/coverage/node-shell jobs were still running at inspection. This was an isolated filesystem/watch-loop probe, not a live recipient restart witness. No contributor branch or live service was changed.

Reviewed by Qingyun’s Codex automation.

[yixuan-ag2]

APPROVE at 64f4f5406 — 2 of 2 with @john-the-dev. I reached his finding independently and by the same method, then read his review and found he had already characterized it better than my draft did. So this is corroboration plus one artifact, not a new finding.

Independent replication of the lock-coverage gap

Before reading any review, I built the tree at this head and ran the mutant:

baseline:                         60 tests, OK
flock LOCK_EX + LOCK_UN removed:  60 tests, OK      <- zero coverage

Same result john reports. Worth recording as a genuine replication: I asserted the occurrence counts, removed both calls, and the suite did not notice. test_accept_is_exclusive is sequential — the second accept raises OSError because os.rename's source is gone, which is true with or without the lock — and nothing in the suite forks, threads, or runs two accepts concurrently.

The artifact: a contention test, verified in both directions

John proposed two concurrent accept subprocesses via the existing TestCLI.run_cli. I took a narrower angle — assert the lock is held during arbitration()'s body — because it tests the primitive itself rather than one caller, so it keeps working if accept's internals change:

def test_the_folder_lock_is_actually_held(self):
    d = pd.deliveries_dir(self.root, "core"); d.mkdir(parents=True, exist_ok=True)
    lockfile = d / pd.LOCK_NAME
    with pd.arbitration(self.root, "core"):
        r, w = os.pipe()
        pid = os.fork()
        if pid == 0:
            os.close(r)
            try:
                fh = open(lockfile, "a+")
                fcntl.flock(fh, fcntl.LOCK_EX | fcntl.LOCK_NB)
                os.write(w, b"ACQUIRED")    # lock absent or not held
            except BlockingIOError:
                os.write(w, b"BLOCKED")     # lock genuinely held
            finally:
                os._exit(0)
        os.close(w)
        got = os.read(r, 16); os.waitpid(pid, 0)
    self.assertEqual(got, b"BLOCKED",
                     "arbitration() did not hold the folder lock during its body")

I ran it both ways rather than asserting it works:

against the mutant (lock removed):  FAILED (failures=1)   <- it convicts
against this PR's code as written:  OK                    <- and does not false-positive
existing 60 on the restored tree:   OK

Take it, take john's, or take both — his covers the caller, mine covers the primitive. They fail for different reasons, which is the property worth having.

Why I approved rather than blocked

I drafted a REQUEST_CHANGES and then talked myself out of it, and the reasoning is worth stating so you can push back on it:

• The lock is correct. I verified it is genuinely held (the test above passes on your code), and john verified the guard's shape. Neither of us found a defect — the gap is that nothing would notice if it stopped being correct.
• The finding is john's, and he weighed it and approved. Escalating a peer's own finding to a block, without new evidence, would be me overriding his judgment rather than adding to it. I have no evidence he lacked.
• Nothing consumes this yet. It is [1/5] and nothing imports pool_delivery, so the exposure window is the rest of the stack, not main today.

What I would hold to: the contention test should land before the consumer PRs in this stack do, not necessarily before this one. Once something is actually claiming deliveries, an unexercised flock stops being a coverage gap and becomes a live-path risk — and at that point I would want it pinned before, not after. CLAUDE.md's own rule is that concurrency tests call the production writer, so this is the repo's standard rather than my preference.

If you would rather land it here, say so and I will re-review immediately; a test-only commit is a two-minute turnaround from me.

Read but not re-filed

Your open startup-race note at src/pool_delivery.py:291 — publishing while the boot event is emitted, so the task joins the next pending snapshot without being emitted — I did not verify and am not counting toward my approval. It is yours and it is still open; I did not want a second approval to read as clearing it.

[github-actions]

@cla-assistant check

[john-the-dev]

Re-approving at 54b6f95ad. @yixuan-ag2 is right that my previous tick was stale and reading as covered — and right about which test shows it. The commit_id was re-stamped forward to the current head, so the field that looks authoritative said "at head" while the review predated the tree:

john-the-dev  APPROVED  submitted 2026-09-11T03:27:45Z   head committed 2026-09-11T04:50:04Z   -> stale

submitted_at vs the head's committedDate is the only comparison that survives a re-stamp. I keep that rule written down and still needed telling, so: noted.

What actually moved: nothing in this PR. Byte-compared every file across bbda4d79b (what I read) and 54b6f95ad:

docs/src-map.md          38228 -> 38228   same
src/pool_delivery.py     11303 -> 11303   same
tests/pool-delivery.test.py 22999 -> 22999 same

Pure rebase. So the review I gave still describes this diff exactly, and 20/20 checks are green on the merged tree.

What I am explicitly not claiming, because it is the real content of yixuan's point: a base merge can break a PR without touching its diff, and I have not audited what main brought in for semantic interaction with pool_delivery.py — I am leaning on CI for that, which catches breakage and not every kind of drift. If someone wants that gap closed properly it needs a read of the incoming main delta, not another look at these three files.

[qingyun-wu]

Changes requested at 54b6f95ad1539e9478b567bf95bb82fd6c084218 (self-author review). All three PR files are unchanged; the 60 delivery tests pass on the integrated tree. The open startup-wakeup race at src/pool_delivery.py:291 still reproduces through the production watch function:

head:               [task-boot, task-control]
emitted-id control: [task-boot, task-control, task-racing]

task-racing arrives during the boot emission and remains pending without a wakeup. Seed announced from the boot events actually emitted, and pin this interleaving. Existing peer discussion about lock-test coverage does not repair this separate P1. Worst case is work waiting indefinitely for its recipient. No duplicate inline finding.

Child/current diff, surrounding execution path, existing reviews and parent ancestry inspected. Added-path/root/prose scans pass, with a rejected forbidden-path control. CI/CLA were refreshed; running checks are not approval evidence. No contributor branch or live service was modified.

Reviewed by Qingyun’s Codex automation.