[qingyun-wu]

Stack #4106 → #4107 → #4109 → #4108 → #4110 — this is layer 5 of 5. Parent: #4108; base branch feat/spawn-worker-launcher. Deploy order: #4108 → #4110. Each PR's diff is exactly its own layer (restacked 2026-09-10; rebased 2026-09-11 onto #4108's head 45eba4c08, then again 2026-09-11 onto #4108's new head 4c8536c7c, and finally 2026-09-11 onto #4108's re-homed head 99c058d66 — see Re-homed onto skills/worker-pool at the bottom). This layer's router now lives in skills/worker-pool/; deploy order is PR A #4188 → #4108 → #4110.
Note: TestHeldByWorker in tests/pool-delivery.test.py collides with #4167's rename to held_by_other_instance() and will drop on the post-#4167 rebase.

What this is

5/5 — the last piece of the worker-pool series (#4105). The router pass: resolve one admitted task against the roster, write the deliveries.

Merge order

#4106  delivery reader     (merged into this branch)
#4109  bindings + roster   (base)
this   router pass

Base is feat/pool-bindings-roster; feat/pool-delivery-reader is merged in. It consumes both.

A role, not a process

This is a function the core's task watcher calls. There is no daemon here and there must not be one — a second process would re-watch tasks/, which the watcher already does across code hardened by many incidents.

Its whole input is the roster and one task. A test proves it by deleting the roster file and routing from an in-memory copy: no clock, no directory listing, no liveness probe inside the decision. That is what makes a pass replayable.

The rule it may never break: substitution

unknown target   -> fails the task BY NAME, delivers to nobody
non-live target  -> HOLDS the work, never re-aimed

A test asserts an unknown target creates no deliveries/core/ at all. Re-aiming work at a reachable recipient is how a task reaches someone the owner never addressed.

Double delivery, enforced from the writing side

deliver_one checks both sentinel names:

if pd.find(workspace, recipient, task_id) is not None:   # unclaimed OR .claimed
    return "already"

Checking only the unclaimed name recreates a sentinel for work already in flight and delivers it twice — the defect fixed in 1/5, now enforced where deliveries are written too. EEXIST is success: another pass won the race, and the delivery exists either way.

Two smaller decisions

One pass reads the roster once, so every task in it is decided against the same version and a recompile mid-pass cannot split the decision.

Ordering is urgent > normal > low, then oldest created_at. A task with no priority sorts as normal, not last — an absent field is a default, not a demotion.

Evidence

$ python3 tests/pool-router-pass.test.py
Ran 21 tests in 0.037s
OK

src/pool_router.py    43 stmts    0 miss    100%

mutation	result
check only the unclaimed name (the double-delivery bug)	fails 1
default to core when the roster is missing	fails 3
deliver to non-live targets anyway	fails 2
substitute instead of failing an unknown target	fails 1

The last uncovered branch was the real race — a sentinel appearing between the check and the open, where O_EXCL is what actually arbitrates. Covered by making the pre-check miss it, rather than by pre-creating the file, which the earlier check catches.

scripts/review-checks.sh PASS — it flagged a three-line comment block first, and the fix keeps # noqa: E402 inline where the directives still apply rather than collapsing them into one line that would silence nothing.

Composition

All five modules were exercised together against a scratch workspace: spawn a worker, bind a room, compile the roster, route, claim, complete. Payload never copied (sentinel 0 bytes, payload intact), the second worker saw an empty sweep, and completion retired the spent sentinel instead of handing it back.

Refs #4105, #4037

---

Payload guard (2026-09-10, 22ef1b6d0). deliver_one now returns "no-payload" and writes nothing when tasks/<task-id>.txt is absent, and route() collects those into a skipped list rather than folding them into already — a refusal must not read as a success. Motivating case, found the destructive way: a hand-call created a sentinel for a task whose payload had already been archived, which is finished work offered a second time. Contributed by the live pool worker; controls confirm 3 tests go red with the guard removed and a 4th with the refusal folded back into already.

---

Deployment order

Strict-header-only routing (89f649a5b) makes this router and the gateway
writer a coupled pair, so the order matters.

The gateway bridge still writes task: before source/channel_id —
_TASK_FIELDS in packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:1719
reads "task", "source", "channel_id", and the live task files on the pool
host have that shape:

$ grep -n '^task:\|^source:\|^channel_id:' <workspace>/tasks/archive/task-local-hs~task-c594afc20824a311cf.txt
5:task: There are CI errors in your PRs
6:source: ag2space
7:channel_id: !SWdwgcAevVVLHNWwJe:ag2space.local

The router no longer recovers routing metadata from
anything below task:, so until the writer converges to task-last every
gateway task routes to the core.

That is the safe direction — the core is a real recipient, not a dropped task —
but it is not the intended one, so:

Deploy this router together with, or after, the writer change. #4121 moves
source/channel_id above task: in _TASK_FIELDS. Rolling this branch out
first degrades the pool to core-only for gateway traffic; rolling #4121 out
first is harmless with either router.

---

Review fixes (2026-09-11, 64916421a)

Three threads on 65f30697d. Every tail below is the actual command output at
the parent commit and at 64916421a, run in a detached worktree.

1. The retry driver was unreachable on the unbound-event path

@keweichen [P2] at src/pool_route_handler.py:128. dispatch_task probes
first and queues a real run only on exit 0/4 (src/watch-tasks-stream.sh
393-416). An unbound task classifies 3, and the real run was the only
caller of retry_pass — so on a host whose traffic is all core-bound, marked
work stayed parked indefinitely.

The probe is the watcher's read-only question (test_the_probe_runs_no_retry_pass
pins that), so the driver cannot move there. The decline branch now runs the
same pass explicitly over --retry-pass; its stdout and exit code are
discarded, so a handler that does not know the verb costs the declined task
nothing. The extra process only ever runs when a handler is configured —
dispatch_task returns early when DISPATCH_DIR is empty.

New test lifts dispatch_task verbatim out of the production script with awk
and runs it against the real handler, stubbing only the emit/queue
collaborators — the same shape tests/watch-tasks-dispatch-activity.test.sh
already uses. Two-sided:

$ bash tests/pool-route-retry-driver-dispatch.test.sh     # parent watcher
  ok   an unbound event still goes to the core
  FAIL the deferred task is delivered on the unbound path
     no deliveries/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/task-old.txt
  FAIL the retry marker is consumed
     state/pool-route-retry/task-old survived
  ok   a bound event is queued for the real run
  ok   a bound event is not shown to the core
  ok   the probe alone delivers nothing
  ok   the probe alone consumes no marker
FAIL   (rc=1)

$ bash tests/pool-route-retry-driver-dispatch.test.sh     # 64916421a
  ok   an unbound event still goes to the core
  ok   the deferred task is delivered on the unbound path
  ok   the retry marker is consumed
  ok   a bound event is queued for the real run
  ok   a bound event is not shown to the core
  ok   the probe alone delivers nothing
  ok   the probe alone consumes no marker
PASS   (rc=0)

The last four lines are the controls that separate the fix from the bug: a
watcher that ran the pass unconditionally would break the bound branch, and a
probe that delivered would pass case 1 for the wrong reason.

2. An unwritable retry marker no longer loses recovery silently

@keweichen [P2] at ~161-170. _defer logged deferred WITHOUT marker and
returned 0. The marker is the only thing retry_pass enumerates, so the task
was recoverable only by the watcher's next startup sweep — and nothing said
so at the time.

The exit code cannot carry this: any non-zero from the real run takes the
fallback disposition in finish_handler_task and hands the task to the live
core, which is the one outcome the owner rule forbids. So it is announced on
stderr, which the handler-runner leaves inherited (only stdout is
>/dev/null) and the watcher's own log keeps.

$ python3 tests/pool-route-handler.test.py   # parent src, new tests
FAIL: test_an_unwritable_marker_says_recovery_is_off_instead_of_nothing
AssertionError: 'DEFERRED WITHOUT RETRY MARKER' not found in ''
Ran 38 tests   FAILED (failures=1)

$ python3 tests/pool-route-handler.test.py   # 64916421a
Ran 38 tests in 0.162s
OK

The assertion is on the literal string, not on h.UNMARKED_NOTICE, so the
parent fails on a silent stderr rather than on a name that does not exist
there yet. A third test is the control: a deferral that keeps its marker prints
nothing, so the notice names a real loss and not every deferral.

What this does not do: it does not add a second durable marker store. A
task deferred with no marker is still recovered by the watcher's initial sweep
(for f in "$TASKS_DIR"/*.txt; do dispatch_task "$f") and not before it; the
notice says exactly that rather than implying --retry-pass could find it.

3. --held can now say "I could not determine that"

@john-the-dev's note. The hook read any non-zero exit as not held, and
2>&1 >/dev/null removed the only channel separating a traceback from a clean
negative — so a broken lookup reported a worker's in-flight task as unprocessed
and told the core to answer it.

pool_delivery.py --held now exits 2 when the lookup raises, with the
exception on stderr. The hook branches on $?: 0 exempt, 1 report, anything
else UNKNOWN — not reported to the core, and named on stderr rather than
dropped silently. No interpreter still reads as 1, so the separate
no usable interpreter; queue not reported refusal below is unchanged.

$ bash tests/check-pending-tasks-held-unknown.test.sh    # parent hook
  ok   a readable tree still blocks on an undelivered task
  FAIL an undeterminable hold is not reported to the core
     hook said: {"decision":"block",...,"--- task-zz-unknown-48896.txt ---\n...
  FAIL the unknown is named on stderr
     stderr was:
  FAIL pool_delivery --held exits outside the 0/1 pair
     exit was 1
FAIL   (rc=1)

$ bash tests/check-pending-tasks-held-unknown.test.sh    # 64916421a
  ok   a readable tree still blocks on an undelivered task
  ok   an undeterminable hold is not reported to the core
  ok   the unknown is named on stderr
  ok   pool_delivery --held exits 2, outside the 0/1 pair
PASS   (rc=0)

The unreadable tree is produced with chmod 000 and the case skips itself if
the mode does not take (root). The first line is the control: a hook that
exempted everything would also stop reporting this task.

tests/pool-delivery.test.py gains the same answer twice — once with
held_by_worker patched to raise, once through a real subprocess so the hook's
$? branch is pinned against an exit code and not a patched function:

$ python3 tests/pool-delivery.test.py   # parent src, new tests
ERROR: test_a_lookup_that_crashes_is_unknown_not_a_denial        (PermissionError escapes main)
FAIL:  test_an_unreadable_deliveries_tree_is_unknown_through_the_cli
AssertionError: 1 unexpectedly found in (0, 1)
Ran 68 tests   FAILED (failures=1, errors=1)

$ python3 tests/pool-delivery.test.py   # 64916421a
Ran 68 tests in 0.828s
OK

Cross-PR collision with #4167 — deliberately not resolved here

@john-the-dev flagged it and I agree with the ordering. #4167 rewrites the
same block of src/check-pending-tasks.sh, has two approvals, and lands first
per the repo owner's "core-only seam changes first". Resolving it in this
branch now would only have to be undone.

#4110 will rebase onto main after #4167 merges and rework that hunk, with
the --held delegation plus the UNKNOWN branch above carried onto #4167's
version of the loop and the full check set rerun on the result. Until then the
hunk here is expected to conflict, and that conflict is not a review finding.

Full suite + gate at 64916421a

tests/pool-route-handler.test.py                 rc=0  Ran 38 tests  OK
tests/pool-router-pass.test.py                   rc=0  Ran 25 tests  OK
tests/pool-bindings-roster.test.py               rc=0  Ran 29 tests  OK
tests/pool-delivery.test.py                      rc=0  Ran 68 tests  OK
tests/state-paths-adoption.test.py               rc=0
tests/check-pending-tasks-worker-held.test.sh    rc=0  PASS
tests/check-pending-tasks-workspace.test.sh      rc=0  PASS
tests/check-pending-tasks-held-unknown.test.sh   rc=0  PASS
tests/pool-route-retry-driver-dispatch.test.sh   rc=0  PASS
tests/watch-tasks-dispatch-activity.test.sh      rc=0  PASS

$ git diff origin/feat/spawn-worker-launcher > /tmp/f4110.diff
$ bash scripts/review-checks.sh --diff /tmp/f4110.diff
prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)
GATE_RC=0

prose_exts is ['.py'], so the scanner reads no shell. Hand-counted: every
added in-body shell comment block is 2 lines or fewer. The two new shell test
files carry the usual defect/control/isolation file header, matching every
sibling shell test on this branch and on main.

---

Review fixes (2026-09-11, 6b632f09d)

@keweichen's two blockers at 64916421a, plus @yixuan-ag2's open nit. Every
tail below is the actual command output at the parent commit and at the new
head, run in a detached worktree; no live service was started or signalled.

(6b632f09d adds one test-only commit on top of 1a04349c6: the new CLI
branches were exercised only across a subprocess, which scores zero coverage,
so the same answers are asserted in-process too. It was force-pushed once, within
a minute, to undo an accidental .coveragerc deletion in dd0428f29 — that SHA
existed on the branch for about 60s and no review points at it.)

1. A failed retry-marker write no longer settles unrecoverable work

The ask. _defer returned 0 after failing to create the only marker
retry_pass() enumerates, so the watcher released its claim at
src/watch-tasks-stream.sh:277-279, the pass never saw the task, and the Stop
hook then reported a worker-addressed payload to the core
(handler rc=0; marker=False; delivery=False -> decision=block). Do not
report settled unless durable, non-fallback ownership or an automatically
discoverable recovery path exists.

The mechanism, both halves of kewei's "and/or".

• A second durable store the same pass enumerates. retry_dirs() returns
  state/pool-route-retry/ then tasks/.pool-route-retry/. The second shares
  no path component with the first below the workspace, so one obstructed
  directory cannot take retry offline; it is hidden, so the tasks/*.txt glob
  every producer and consumer uses never sees it. retry_pass reads both, which
  makes the marker discoverable before any repair, not only after one.
• A distinct exit code, for the case where NO store takes it. I checked what
  finish_handler_task supports first: it branches on the claim's disposition
  (must-handle / fallback / unrecognised), not on rc, and every non-zero rc ends
  in one of those branches — fallback publishes a worker's task to the live
  core, which the owner rule forbids. So the handler returns UNSETTLED (5) and
  the watcher maps that ONE code to "keep the claim, publish nothing": the task
  is neither settled nor handed over, and the next watcher start re-dispatches it.

The Stop hook stops spelling the marker path. It now asks the owner —
pool_route_handler.py --parked <id>, 0/1/2 exactly like --held. One
refinement over the --held shape: a store obstructed by a file is a
negative, not an unknown (nothing can be stored under a file), so the hook
keeps working on every other task while one path is broken; only a store that
genuinely cannot be read is UNKNOWN and therefore not reported.

Regression test, exactly the sequence kewei asked for — marker-write failure ->
storage repaired -> automatic retry delivers, and the hook never names the
parked task. Two-sided:

$ bash tests/pool-route-marker-failure-recovery.test.sh     # parent 64916421a
  ok   an obstructed store still exits 0 (the task stays the worker's)
  FAIL the marker lands in the second store
  FAIL the handler answers that the task is parked
  FAIL a parked task is not reported to the core
  ok   an unparked task is still reported (the hook did not go blind)
  FAIL the repaired task is delivered by the automatic retry pass
  FAIL the marker is consumed once delivered
  FAIL no store anywhere exits UNSETTLED
  ok   the loss is named on stderr
  ok   an unsettled run keeps the claim and tells no core
  ok   an ordinary handler failure still falls back to the core
FAIL

$ bash tests/pool-route-marker-failure-recovery.test.sh     # 1a04349c6
  (the same 11 lines, all ok)
PASS

Three of those lines are the controls that separate the fix from the bug: an
unparked task in the same workspace is still reported (a hook that exempted
everything would also stop reporting the parked one); an ordinary handler
failure still falls back to the core (so the new code did not simply disable the
fallback); and the last two ok at the parent are the pre-existing behaviour the
change must not have needed.

finish_handler_task is lifted verbatim out of the production watcher with
awk and run against stubbed claim/emit collaborators that record which branch
ran — the same shape tests/pool-route-retry-driver-dispatch.test.sh uses.

2. A launched worker now receives the task, not the sentinel

The ask. spawn_worker points the worker's watcher at deliveries/<worker>,
deliver_one writes a zero-byte sentinel there, and nothing called
pool_delivery.read_payload — so the worker was handed 0 bytes
(payload_bytes=61; watched_file_bytes=0). The Codex path also lost the
workspace: no SUTANDO_RESULTS_DIR, no forwarding of SUTANDO_WORKSPACE_DIR,
and task-notifier.sh derived results from the inbox's parent
(deliveries/results vs canonical results, matches=False).

Where the fix lives, and why. In watch-tasks-stream.sh's dispatch_task
— the one place an inbox event becomes a task path — with the kind of inbox
injected by the adapter that named it (SUTANDO_INBOX_KIND, set by
spawn_worker.plan()). Two rules decide this:

• The watcher must not infer that its inbox is a delivery folder; discovery
  of an optional capability belongs at the adapter edge (CLAUDE.md, "Optional
  capability discovery stays at the adapter edge"). It is told, it never sniffs
  the path.
• The sentinel -> payload grammar must have one owner. Resolving in the
  launchers instead would copy that grammar into the Claude launcher, the Codex
  launcher and every future one — the duplicated policy CLAUDE.md calls a defect
  in its own right. pool_delivery grows a payload verb (built on the same
  payload_path() that read_payload() uses) and both adapters ask it.

The emitted line still carries the bare task id, so a worker's core reads
<workspace>/tasks/<id>.txt exactly as the core does; only the resolution
changed, not the protocol.

Both launchers now forward the whole seam (SUTANDO_WORKSPACE_DIR,
SUTANDO_RESULTS_DIR, SUTANDO_INBOX_KIND), and task-notifier.sh derives
results, state, claims, fallback receipts, core-status and logs from the
workspace rather than from dirname "$TASKS_DIR". spawn_worker composes
neither folder itself any more — pool_delivery owns deliveries_dir and a new
results_dir, which is also what tests/state-paths-adoption.test.py asks for
(it fails on a hand-composed <workspace>/results).

Composition probe: routes a real task through the production router, launches
nothing, and runs the watcher's own read path over the delivery folder.

$ bash tests/pool-worker-inbox-composition.test.sh          # parent 64916421a
  ok   the production router delivered to the bound worker
  ok   the delivery is a 0-byte sentinel (the payload is never copied)
  ok   the event still names the task by its id
  FAIL the handler is given the canonical payload
       it was given: .../deliveries/aaaa…/task-body.txt
  FAIL the bytes the handler receives are the payload's
       got 0, payload is 68
  ok   an ordinary inbox resolves to the file the event named
  FAIL a sentinel with no payload dispatches nothing
  ok   the Claude-side watcher writes results to <workspace>/results
  FAIL the Codex notifier writes results to <workspace>/results
       got .../deliveries/results
  FAIL the two runtimes agree on the results path
       .../results vs .../deliveries/results
  FAIL the Codex prompt points the worker at the payload
FAIL

$ bash tests/pool-worker-inbox-composition.test.sh          # 1a04349c6
  (the same 11 lines, all ok — payload bytes 68, sentinel 0, matches=True)
PASS

dispatch_task, both results resolutions and the notifier's payload_file are
lifted verbatim from the production scripts, so this pins shipped text rather
than a restatement. Two controls are in the same run: the sentinel's own size is
asserted to be 0 (so "the handler got 68 bytes" cannot be true by accident), and
an ordinary core inbox is asserted unchanged (so the fix cannot be "always
rewrite the path"). The one parent-ok in the middle is honest: the Claude-side
watcher already resolved results correctly via #4108's SUTANDO_WORKSPACE_DIR;
only the Codex side lost them.

3. @yixuan-ag2's open items

The unknown_targets contract disagreement (#4109 vs here). You asked the
author to pick: the behaviour here is the one I am keeping — an unknown
target goes to the core, and the name it was addressed to is reported in
redirected, which is what makes it a recorded redirect rather than a silent
mis-route. The text that changes is #4109's unknown_targets docstring, now on
main; it is a one-line doc change and it does not belong in this layer's diff,
so it is a separate PR I have not filed yet rather than something quietly done
here.

The pool_router.py:15 fragment is fixed in 1a04349c6, and the docstring
now also names where the redirect is recorded.

redirected has no consumer yet — still true, still one hit (:112, the
line that constructs it). Unchanged by this push, and noted as a [5/5] property
rather than answered.

Still outstanding, deliberately

• No live post-restart round trip. The live router and the four live
  watchers on this host run from an older checkout; rolling them forward is the
  owner's call at a quiet moment, and this work was done in an isolated
  worktree. Everything above is deterministic-harness evidence plus
  parent-vs-head probes. The task -> worker -> canonical-result round trip
  stays a merge condition.
• Stack order. feat(pool): accept a stamped picker command, so prose becomes the fallback #4121 (task-last writer) remains the deployment prerequisite,
  and parent feat(pool) [4/5]: spawn a worker — identity, folder, tmux session, watcher #4108 lands first.
• The check-pending-tasks.sh hunk will rebase onto fix(watcher): the inbox and the workspace may be named by env; a worker-held task is not unprocessed #4167. fix(watcher): the inbox and the workspace may be named by env; a worker-held task is not unprocessed #4167 rewrites
  the same block, is 2-approved and lands first under the owner's "core-only
  seam changes first" (@john-the-dev's collision, which I agree with). After it
  merges this PR rebases onto main and reworks the hunk, carrying the --held
  delegation, the UNKNOWN branch and the new --parked delegation onto fix(watcher): the inbox and the workspace may be named by env; a worker-held task is not unprocessed #4167's
  version of the loop — whose grammar is held_by_other_instance/self_recipient,
  so the rebased hunk adopts that spelling rather than re-introducing
  held_by_worker. The full check set is rerun on the result. Until then the
  conflict is expected and is not a review finding.
• A worker's priority ordering reads sentinels, not payloads (Codex
  next_pending_task sorts tasks_dir.glob("*.txt") by header priority, and a
  sentinel has no headers). Pre-existing, unrelated to the two blockers, and
  invisible until a worker has more than one queued task; naming it rather than
  folding a third change into this push.

Full check set at 6b632f09d

tests/pool-route-handler.test.py                  rc=0  Ran 49 tests  OK
tests/pool-router-pass.test.py                    rc=0  Ran 25 tests  OK
tests/pool-bindings-roster.test.py                rc=0  Ran 29 tests  OK
tests/pool-delivery.test.py                       rc=0  Ran 77 tests  OK
tests/spawn-worker-launcher.test.py               rc=0  Ran 31 tests  OK
tests/state-paths-adoption.test.py                rc=0  all passed
tests/health-check-codex-task-notifier.test.py    rc=0  Ran 18 tests  OK
tests/codex-core-launcher.test.py                 rc=0  Ran 32 tests  OK
tests/watch-tasks-stream-dead-worker-reap.test.py        rc=0
tests/watch-tasks-stream-interrupted-task-retry.test.py  rc=0
tests/watch-tasks-stream-sentinel-ownership.test.py      rc=0
tests/watch-tasks-stream-fallback-receipt-scope.test.py  rc=0
tests/pool-route-marker-failure-recovery.test.sh  rc=0  PASS
tests/pool-worker-inbox-composition.test.sh       rc=0  PASS
tests/pool-route-retry-driver-dispatch.test.sh    rc=0  PASS
tests/check-pending-tasks-worker-held.test.sh     rc=0  PASS
tests/check-pending-tasks-held-unknown.test.sh    rc=0  PASS
tests/check-pending-tasks-workspace.test.sh       rc=0  PASS
tests/start-cli-worker-env-forwarded.test.sh      rc=0  PASS — 7 checks green
tests/start-cli-task-event-handler-env.test.sh    rc=0  3 passed, 0 failed
tests/start-cli-proxy-routing.test.sh             rc=0  PASS — 13 checks green
tests/start-cli-visible.test.sh                   rc=0  PASS
tests/start-cli-compat-shim.test.sh               rc=0  PASS
tests/watch-tasks-dispatch-activity.test.sh       rc=0  PASS
tests/watch-tasks-stream-shutdown-emit.test.sh    rc=0  PASS
tests/watch-tasks-stream-trap-exit.test.sh        rc=0  PASS
tests/task-emit-substitution.test.sh              rc=0  ALL PASS

$ git diff origin/feat/spawn-worker-launcher > /tmp/f4110b.diff
$ bash scripts/review-checks.sh --diff /tmp/f4110b.diff
prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)
GATE_RC=0

The restart-launching suites (start-cli-restart-inside-core-guard.test.sh,
start-cli-restart-verify.test.py) were not run: they drive a real restart
on this host.

prose_exts is ['.py'], so the scanner reads no shell. Hand-counted on the
cumulative diff: every added in-body shell comment block is 2 lines or fewer
(one 3-line block was trimmed before the final push); the two new shell test
files carry the usual defect/shape/control/isolation header, as every sibling
shell test on this branch and on main does.

Two of the python suites are only partly two-sided, said plainly: at the parent,
tests/pool-route-handler.test.py reports 2 behavioural failures and 10
errors that are AttributeError on names that do not exist there yet
(retry_dirs, parked, UNSETTLED), and the 4 new pool-delivery CLI tests
fail on an absent payload verb. Those errors prove the API is new, not that
behaviour changed — the behavioural evidence is the two shell tests above.

---

Rebase onto #4108's new head (2026-09-11, 449d2981a)

GitHub reported this PR DIRTY after #4108 moved to 45eba4c08. Rebased
--onto origin/feat/spawn-worker-launcher from the old base a25ec94c7 (the
#4108 head this branch was last stacked on), in a detached worktree; no live
service was started, signalled or restarted.

What moved DOWN into #4108 — dropped from this layer

#4108 absorbed the whole launcher/notifier/delivery-path seam, so this PR's
copies are gone rather than duplicated:

hunk	now owned by
Claude launcher SUTANDO_INBOX_KIND / SUTANDO_RESULTS_DIR forwarding	#4108
Codex launcher instance/tasks/workspace/inbox-kind/results forwarding, into both the core session and the notifier	#4108 (notifier_env_args)
task-notifier.sh workspace-derived results/claims/receipts/core-status/logs, plus payload_file	#4108
pool_delivery.results_dir() and the payload verb	#4108
spawn_worker.plan() composing neither folder itself	#4108
the payload verb's own unit tests (TestPayloadVerb equivalents)	#4108

Two conflicts, both resolved by taking #4108's version and re-applying only
this layer's delta on top:

• src/agent/codex/cli/start-cli.sh — feat(pool) [4/5]: spawn a worker — identity, folder, tmux session, watcher #4108 refactored the inline notifier-env
  block into notifier_env_args, which already forwards everything this layer
  used to add. Kept feat(pool) [4/5]: spawn a worker — identity, folder, tmux session, watcher #4108's call; this file's diff against the base is now 0
  lines, as are task-notifier.sh and spawn_worker.py.
• src/pool_delivery.py — command is nargs="?" (so --held needs no verb)
  and its choices keep feat(pool) [4/5]: spawn a worker — identity, folder, tmux session, watcher #4108's payload. held_by_worker / --held /
  HELD_UNKNOWN are the only additions left here.

What this layer KEEPS

• the watcher-side dispatch_task sentinel → payload resolution, keyed on
  SUTANDO_INBOX_KIND and asking feat(pool) [4/5]: spawn a worker — identity, folder, tmux session, watcher #4108's pool_delivery payload verb
• SUTANDO_TASK_EVENT_HANDLER forwarding (Claude launcher, +3 lines)
• held_by_worker / --held / HELD_UNKNOWN
• the second retry store tasks/.pool-route-retry/, UNSETTLED rc 5, --parked
• every router/handler test

The file set now

$ git diff origin/feat/spawn-worker-launcher --stat
 docs/src-map.md                                  |   2 +
 src/agent/claude/cli/start-cli.sh                |   3 +
 src/check-pending-tasks.sh                       |  32 ++
 src/pool_delivery.py                             |  44 +-
 src/pool_route_handler.py                        | 368 +++++++++++++++
 src/pool_router.py                               | 123 +++++
 src/watch-tasks-stream.sh                        |  32 +-
 tests/check-pending-tasks-held-unknown.test.sh   |  88 ++++
 tests/check-pending-tasks-worker-held.test.sh    | 115 +++++
 tests/pool-delivery.test.py                      |  89 +++-
 tests/pool-route-handler.test.py                 | 561 +++++++++++++++++++++++
 tests/pool-route-marker-failure-recovery.test.sh | 155 +++++++
 tests/pool-route-retry-driver-dispatch.test.sh   | 110 +++++
 tests/pool-router-pass.test.py                   | 267 +++++++++++
 tests/pool-worker-inbox-composition.test.sh      | 182 ++++++++
 tests/start-cli-task-event-handler-env.test.sh   |  35 ++
 16 files changed, 2202 insertions(+), 4 deletions(-)

Only this layer's own files. Structural checks on the result:

$ grep -c '^def results_dir' src/pool_delivery.py        # 1  (#4108's)
$ grep -c 'command == "payload"' src/pool_delivery.py    # 1  (#4108's)
$ grep -n 'results_dir\|deliveries_dir' src/spawn_worker.py
109:    delivery_dir = str(pd.deliveries_dir(workspace, worker_id))
132:                "SUTANDO_RESULTS_DIR": str(pd.results_dir(workspace)),
$ grep -n 'INBOX_KIND\|pool_delivery.py' src/watch-tasks-stream.sh
92:INBOX_KIND="${SUTANDO_INBOX_KIND:-tasks}"
402:    payload="$("$SUTANDO_PY_BIN" "$__REPO_ROOT/src/pool_delivery.py" \
403:      --workspace "$WORKSPACE_DIR" payload --sentinel "$filename")"

dispatch_task still calls #4108's payload verb; nothing is defined twice.

Checks at 449d2981a

Including #4108's own two suites, which is what proves the moved-down hunks
still satisfy this layer's contract:

tests/pool-route-handler.test.py                  rc=0  Ran 49 tests  OK
tests/pool-router-pass.test.py                    rc=0  Ran 25 tests  OK
tests/pool-bindings-roster.test.py                rc=0  Ran 29 tests  OK
tests/pool-delivery.test.py                       rc=0  Ran 73 tests  OK
tests/pool-route-marker-failure-recovery.test.sh  rc=0  PASS
tests/pool-worker-inbox-composition.test.sh       rc=0  PASS
tests/pool-route-retry-driver-dispatch.test.sh    rc=0  PASS
tests/check-pending-tasks-worker-held.test.sh     rc=0  PASS
tests/check-pending-tasks-held-unknown.test.sh    rc=0  PASS
tests/check-pending-tasks-workspace.test.sh       rc=0  PASS
tests/watch-tasks-dispatch-activity.test.sh       rc=0  PASS
tests/start-cli-worker-bootstrap.test.py          rc=0  Ran  5 tests  OK   (#4108's)
tests/spawn-worker-runtime-and-sentinel.test.py   rc=0  Ran 11 tests  OK   (#4108's)
tests/spawn-worker-launcher.test.py               rc=0  Ran 33 tests  OK   (#4108's)
tests/state-paths-adoption.test.py                rc=0  all passed

$ git diff origin/feat/spawn-worker-launcher > /tmp/rb4110.diff
$ bash scripts/review-checks.sh --diff /tmp/rb4110.diff
prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)
GATE_RC=0

tests/pool-worker-inbox-composition.test.sh is the one that matters most here:
it lifts dispatch_task, both results resolutions and the notifier's
payload_file verbatim out of the production scripts — which after this
rebase are #4108's copies — so its 11 green lines are evidence about the merged
pair, not about text this branch carries.

tests/pool-delivery.test.py drops from 77 to 73 tests: the 4 subprocess
payload-verb cases and the 5 in-process ones were removed with the verb, and
#4108's TestPayloadVerb covers the same answers. TestHeldByWorker is now this
file's entire contribution.

The restart-launching suites (start-cli-restart-inside-core-guard.test.sh,
start-cli-restart-verify.test.py) were not run: they drive a real restart
on this host.

prose_exts is ['.py'], so the gate reads no shell. Hand-counted on the
cumulative diff: every added in-body shell comment block is 2 lines or fewer
— 449d2981a trims one 3-line block in
tests/pool-worker-inbox-composition.test.sh whose third line restated the two
above it (every other section marker in that file is exactly two). The six
remaining >2-line runs are all file headers directly under the shebang, the
same shape every sibling shell test on this branch and on main carries.

The #4167 note above still stands: src/check-pending-tasks.sh is unchanged
by this rebase and still expects to be reworked onto #4167's version of the loop
once #4167 lands.

Re-homed onto skills/worker-pool (2026-09-11, 43a74e2d3)

Owner directive: non-core pool code moves to skills/worker-pool/. PR A (#4188)
moved pool_roster + worker_identity; #4108 re-homed the spawner and the
bootstrap gate at 99c058d66. This rebases onto that head and moves this
layer's own two modules and their suites the same way.

Only paths, imports, globs and seams changed. No routing rule, no exit code,
no assertion text. Every test that was green at 449d2981a is green here,
running from its new location against the same production code.

File → path

before	after
src/pool_router.py	skills/worker-pool/scripts/pool_router.py
src/pool_route_handler.py	skills/worker-pool/scripts/pool_route_handler.py
tests/pool-router-pass.test.py	skills/worker-pool/tests/pool-router-pass.test.py
tests/pool-route-handler.test.py	skills/worker-pool/tests/pool-route-handler.test.py
tests/pool-route-retry-driver-dispatch.test.sh	skills/worker-pool/tests/pool-route-retry-driver-dispatch.test.sh
tests/pool-route-marker-failure-recovery.test.sh	skills/worker-pool/tests/pool-route-marker-failure-recovery.test.sh
tests/pool-worker-inbox-composition.test.sh	skills/worker-pool/tests/pool-worker-inbox-composition.test.sh

Both moved scripts pick up the skill bootstrap every other script in this skill
uses — siblings on sys.path, the core's src/ at parents[3]:

sys.path.insert(0, str(Path(__file__).resolve().parent))
sys.path.insert(1, str(Path(__file__).resolve().parents[3] / "src"))

docs/src-map.md is regenerated and now carries no router entry — net zero
against the parent (git diff origin/feat/spawn-worker-launcher -- docs/src-map.md
is empty). skills/worker-pool/SKILL.md gains both scripts in its table, the
new seam, and drops #4110 from the pending list; manifest.json's description
follows. The handler declares no inline tool, so no tools block.

How the Stop hook reaches --parked now

It was a path — "$PYBIN" "$REPO_DIR/src/pool_route_handler.py" --parked — which
after the move would be a core file reaching into a skill. It now asks the
handler the pool's install already named, the same one the watcher probes, runs,
and drives --retry-pass on:

PARKED_RC=1
if [ -n "${SUTANDO_TASK_EVENT_HANDLER:-}" ] && [ -x "${SUTANDO_TASK_EVENT_HANDLER}" ]; then
  "$SUTANDO_TASK_EVENT_HANDLER" --workspace "$WORKSPACE" --parked "$TASK_ID" >/dev/null 2>&1
  PARKED_RC=$?
fi

Four answers, four dispositions — all four pinned by the new
tests/check-pending-tasks-parked-handler-seam.test.sh:

handler	rc	hook
not named (no pool installed)	—	reports the task — no router ran, so nothing could have parked it
named, says "not parked"	1	reports
named, says "parked"	0	exempt: the task is a worker's
named, cannot answer	2	exempt and named on stderr — unknown is not a negative

$ bash tests/check-pending-tasks-parked-handler-seam.test.sh
  ok   an unnamed handler does not exempt the task
  ok   "not parked" (exit 1) is reported
  ok   the hook asks --parked with the task id
  ok   a parked task (exit 0) is not reported to the core
  ok   an undeterminable park (exit 2) is not reported
  ok   the unknown is named on stderr
  ok   the hook names no handler file
PASS

The last line is the boundary assertion: the shipped hook is grepped for
pool_route_handler and skills/worker-pool and must hit neither.

What stayed core, and why

The line is who reads it with zero workers configured.

• src/pool_delivery.py — the delivery-record grammar. The watcher resolves
  a sentinel to its payload through its payload verb on every
  delivery-inbox event, and the Stop hook asks --held on every task. Both run
  with no skill installed.
• finish_handler_task's UNSETTLED rc-5 branch (src/watch-tasks-stream.sh)
  — a protocol number. Its comment no longer names a file.
• The --retry-pass call on a declined event and SUTANDO_TASK_EVENT_HANDLER
  forwarding in src/agent/claude/cli/start-cli.sh — the core's own env
  allowlist, gated on a variable the core never sets.
• held_by_worker / --held / HELD_UNKNOWN — pool_delivery's, unmoved.

The boundary grep in SKILL.md is widened to cover the two new names and comes
back empty:

$ git grep 'pool_roster\|pool_router\|pool_route_handler\|worker_identity\|spawn_worker\|worker_bootstrap' -- src scripts
$ echo $?
1

CI discovery

The Python step already scans skills (#4108, 0a9596250). The shell step
scanned tests only, so the three moved .test.sh files would have gone
unrun — a silently un-gated suite. It now uses the same optional-root form:

sh_roots=(tests); if [ -d skills ]; then sh_roots+=(skills); fi
...
done < <(find "${sh_roots[@]}" -name '*.test.sh' -not -path '*/node_modules/*' | sort)

find skills -name '*.test.sh' returns exactly those three, so no other suite
is newly gated by this change.

Checks at 43a74e2d3

skills/worker-pool/tests/pool-router-pass.test.py                  rc=0  Ran 25 tests  OK
skills/worker-pool/tests/pool-route-handler.test.py                rc=0  Ran 49 tests  OK
skills/worker-pool/tests/pool-route-retry-driver-dispatch.test.sh  rc=0  PASS  (7 ok)
skills/worker-pool/tests/pool-route-marker-failure-recovery.test.sh rc=0 PASS  (11 ok)
skills/worker-pool/tests/pool-worker-inbox-composition.test.sh     rc=0  PASS  (11 ok)
tests/check-pending-tasks-parked-handler-seam.test.sh              rc=0  PASS  (7 ok)   [new]
tests/check-pending-tasks-worker-held.test.sh                      rc=0  PASS  (9 ok)
tests/check-pending-tasks-held-unknown.test.sh                     rc=0  PASS  (4 ok)
tests/check-pending-tasks-workspace.test.sh                        rc=0  PASS  (7 ok)
tests/start-cli-task-event-handler-env.test.sh                     rc=0  3 passed, 0 failed
tests/pool-delivery.test.py                                        rc=0  Ran 74 tests  OK
tests/watch-tasks-dispatch-activity.test.sh                        rc=0  PASS
tests/watch-tasks-stream-dead-worker-reap.test.py                  rc=0  PASS
tests/watch-tasks-stream-fallback-receipt-scope.test.py            rc=0  5/5 passed
tests/watch-tasks-stream-interrupted-task-retry.test.py            rc=0  PASS
tests/watch-tasks-stream-sentinel-ownership.test.py                rc=0  PASS
tests/watch-tasks-stream-shutdown-emit.test.sh                     rc=0  all checks passed
tests/watch-tasks-stream-trap-exit.test.sh                         rc=0  PASSED
tests/ci-covers-every-python-test.test.py                          rc=0  Ran  5 tests  OK
tests/policy/runner-glob.test.py                                   rc=0  Ran  5 tests  OK
tests/src-map.test.py                                              rc=0  27 passed
tests/state-paths-adoption.test.py                                 rc=0  all passed
ruff check skills/worker-pool src tests                            rc=0  All checks passed!
python3 -m py_compile (both moved scripts + both moved .py suites)  rc=0

The two FAIL kill_workers readiness lines inside
watch-tasks-stream-dead-worker-reap.test.py are that suite's own deliberate
negative cases (it prints "the two 'FAIL kill_workers readiness' lines below are
DELIBERATE"); its exit code is 0.

Not run: anything that starts, signals or restarts a process on this host — this
box runs the live core, four watchers and two workers.

$ git diff $(git merge-base origin/feat/spawn-worker-launcher HEAD) > /tmp/rh4110.diff
$ bash scripts/review-checks.sh --diff /tmp/rh4110.diff
prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)
GATE_RC=0

prose_exts is ['.py'], so the gate reads no shell. Hand-counted on the
cumulative diff: one 3-line in-body block in src/watch-tasks-stream.sh and one
in tests/start-cli-task-event-handler-env.test.sh were trimmed to 2. The seven
remaining >2-line runs are all file headers directly under the shebang — the
same shape every sibling shell suite on this branch and on main carries.

Deploy order

PR A #4188  →  #4108  →  #4110 (this)  →  #4115  →  #4119 · #4120 · #4121 · #4162 · #4175 · #4176

Reviews at earlier heads

The reviews on 64916421a, 6b632f09d and 449d2981a are reviews of this
same behaviour at the old paths. Nothing they examined changed: the router
rules, the handler's exit codes, the retry stores (state/pool-route-retry/ and
tasks/.pool-route-retry/ — workspace state, reachable from a skill), the
marker logic and every assertion are byte-identical modulo the sys.path
bootstrap and the test-local $REPO/$SCRIPTS spellings. The one behavioural
delta is the hook's --parked seam above, which is new and separately pinned.

The #4167 note above still stands: src/check-pending-tasks.sh still expects
to be reworked onto #4167's version of the loop once #4167 lands — now including
the SUTANDO_TASK_EVENT_HANDLER call in place of the old path.

[github-actions]

Coverage Gate

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

Diff Coverage

Diff: origin/feat/spawn-worker-launcher...HEAD, staged and unstaged changes

• skills/worker-pool/scripts/pool_route_handler.py (96.6%): Missing lines 129-130,286-288,292-293
• skills/worker-pool/scripts/pool_router.py (100%)
• skills/worker-pool/tests/pool-route-handler.test.py (99.0%): Missing lines 204-205,491,535
• skills/worker-pool/tests/pool-router-pass.test.py (100%)
• src/pool_delivery.py (100%)

Summary

• Total: 843 lines
• Missing: 11 lines
• Coverage: 98%

skills/worker-pool/scripts/pool_route_handler.py

Lines 125-134

  125         d = _ws(workspace) / "logs"
  126         d.mkdir(parents=True, exist_ok=True)
  127         with open(d / "pool-route-handler.log", "a", encoding="utf-8") as fh:
  128             fh.write(time.strftime("%Y-%m-%dT%H:%M:%S ") + line.rstrip() + "\n")
! 129     except OSError:
! 130         pass
  131 
  132 
  133 def main(argv=None) -> int:
  134     p = argparse.ArgumentParser(add_help=False)

---

Lines 282-297

  282     try:
  283         out = rt.route(ws, task, roster)
  284     except rt.RouterRefused as e:
  285         return _defer(ws, task_id, f"refused: {e}")
! 286     except Exception:
! 287         _log(ws, f"{task_id}: crashed:\n" + traceback.format_exc())
! 288         return _defer(ws, task_id, "crashed (traceback logged)")
  289     if out.get("failed"):
  290         # Unreachable via the probe, which declines these; kept so a direct
  291         # caller cannot turn an unknown name into a delivery.
! 292         print(json.dumps(out), file=sys.stderr)
! 293         return DECLINE
  294     _drop_marker(ws, task_id)
  295     _log(ws, f"{task_id}: delivered={out.get('delivered')} "
  296              f"already={out.get('already')} skipped={out.get('skipped')} -> 0")
  297     return 0

---

skills/worker-pool/tests/pool-route-handler.test.py

Lines 200-209

  200 
  201         def loader(_ws):
  202             try:
  203                 last[0] = next(seq)
! 204             except StopIteration:
! 205                 pass
  206             return last[0]
  207         return unittest.mock.patch.object(h.pr, "load_roster", side_effect=loader)
  208 
  209     def test_a_roster_swapped_mid_run_does_not_change_the_delivery_set(self):

---

Lines 487-495

  487         blind.mkdir(parents=True)
  488         os.chmod(blind, 0o000)
  489         self.addCleanup(os.chmod, blind, 0o755)
  490         if os.access(blind, os.R_OK):
! 491             self.skipTest("the mode did not take effect (running as root?)")
  492         self.roster()
  493         self.task_file("task-1", channel_id="!room:x")
  494         d = h.retry_dirs(self.ws)[1]
  495         d.mkdir(parents=True, exist_ok=True)

---

Lines 531-539

  531         d.mkdir(parents=True)
  532         os.chmod(d, 0o000)
  533         self.addCleanup(os.chmod, d, 0o755)
  534         if os.access(d, os.R_OK):
! 535             self.skipTest("the mode did not take effect (running as root?)")
  536         self.assertEqual(h.main(["--workspace", str(self.ws), "--parked", "task-1"]),
  537                          h.PARKED_UNKNOWN)
  538         self.assertGreater(h.PARKED_UNKNOWN, 1)

---

[qingyun-wu]

Changes requested (formal REQUEST_CHANGES is unavailable on this identity's own PR). Reviewed the child layer against #4109 and its bundled #4106 delivery implementation; 21 router and 42 delivery tests plus the added-path scan pass.

Three blocking integration cases remain:

1. A router paused after find() can race another pass and a worker claim. When it resumes, O_EXCL recreates task-x beside task-x.claimed, and a second production pd.claim succeeds, replacing the first claim. Serialize/fence the whole transition; O_EXCL on one of two mutable names is insufficient.
2. The documented admitted shape source=room, channel_id=!abc:ag2.space resolves to core despite a room:!abc:ag2.space binding. The test supplies the prefixed binding key as channel_id, so it misses real envelope normalization. Define one source-key owner and test the documented envelope.
3. Every member of a set receives the same task id. pool_delivery.result_path is global by task id, and payload archival is global too: the first completion consumes the shared payload and multiple answers share one result path. Implement the specified per-recipient child identities/parent grouping before fan-out.

The race and source-key failures were exercised through the production router in scratch workspaces. The result-path collision is directly established by the router and delivery path functions; I did not run external task effects. Worst case: duplicate execution, wrong recipient or a lost sibling answer. Parent delivery/bindings findings also remain applicable.

Reviewed at 499b39f5c8a1d6a09f652eb33cc4a9ca009b89fb. No contributor branch or live runtime was changed.

[qingyun-wu]

Changes requested at be393f1f7644ea6d4f76682c63e9cee7f95780c0 (formal REQUEST_CHANGES is unavailable on my own PR). The new handler introduces two blocking composition failures, detailed inline:

explicit worker, good roster: handler probe=0; production dispatch queues it
same task, corrupt roster: probe=3; production dispatch emits CORE_EVENT:task-x.txt
handler accepts tasks/task-handler.txt: rc=0, sentinel created
recipient read_payload: None; sweep: stale=[task-handler], sentinel removed
JSON-payload control: payload readable; sweep ready=[task-x]

The dispatch probe executes the current shell function with scratch inputs and stubbed event/queue emitters; it is not a real agent round trip. A declined probe is a live-core event, so absent/corrupt routing state must not decline addressed work. The adapter and delivery reader also need one canonical payload contract; creating a .txt sentinel alone does not bridge .txt envelopes to the reader's .json payload.

The prior three findings remain: src/pool_router.py:53-60 still permits two successful production claims in the find/create/claim/recreate interleaving; :80 still routes source=room/channel_id=!x to core while the prefixed-channel control reaches the bound worker; :90-92 still fans out one global task/result identity. Reproduced the race and source-key failure at this head. The shared result-path collision follows directly from the unchanged path functions. Keep those existing threads open; do not copy routing policy into the handler to patch symptoms.

21 router, 51 delivery and 9 handler tests pass. Current parent roster code and bundled delivery code inspected; parent blockers remain. Child/cumulative added-path scans and hosted CI/CLA pass, but green tests miss these behaviors. Worst case is wrong-recipient execution, duplicate work or silently discarded delivery. Real post-restart end-to-end acceptance remains required. No contributor branch or live runtime was changed.

[qingyun-wu]

Changes requested at 0deacfa8 (COMMENTED because this login authored the PR). The inherited .txt payload mismatch is resolved: production handler returns 0, read_payload returns the admitted header text, and the recipient sweep reports ready with the sentinel intact. The revised unknown-name-to-core and non-live-target delivery rules match the current design; the old PR-body rules should be refreshed.

These existing blockers remain reproducible:

1. [P1] Duplicate claims — src/pool_router.py:53-58. Pause writer A after find(None); production writer B delivers and the recipient claims; resume A's exclusive create. It succeeds on the now-free pending name. A second production claim succeeds and overwrites the first .claimed path. The both-name lookup and publication/claim transitions need one shared arbitration contract; the current EEXIST test does not cover a claimant freeing the name.
2. [P1] Source-key mismatch — src/pool_router.py:79-80 and src/pool_route_handler.py:50. With binding room:!x, {source: room, channel_id: !x} routes to core; changing only the channel field to the synthetic room:!x reaches the worker. Derive the binding key from the actual admission shape in one shared owner.
3. [P1] Fan-out completion identity — src/pool_router.py:91-93. Each member still gets the same task id and therefore the same result path and payload. One finish can remove the other's unread payload; two result publications overwrite one reply. Define distinct delivery/result identity plus shared-payload retention and test both completion orders and restart.
4. [P1] Missing/corrupt roster declines to core — src/pool_route_handler.py:47-49. The same explicitly addressed task probes 0 with a valid roster and 3 without it; watch-tasks-stream.sh::dispatch_task emits the task to the core on 3. Missing routing state is still required to refuse the pass, unlike a known roster containing an unknown target. Preserve that distinction across probe and execution failures.

Full child and inherited diff inspected. Delivery 51, router 20, handler 9 and bindings 22 tests pass. Child and cumulative path scans pass; the forbidden-path control fails. Scratch merge with current roster parent 1785e19c is conflict-free. Merge parents #4106/#4109 first and rerun after synchronization; their unresolved blockers remain relevant. Worst case on activation is duplicate effects, wrong-recipient execution or lost replies. No live round trip was performed; these are isolated production-function probes.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Changes requested at db054bef (COMMENTED because this login authored the PR). Both new fixes are verified: lane-encoded sentinels parse, and a gateway-shaped task with source/channel fields below task: changes from probe=3 on 0deacfa8 to probe=0 at HEAD. The inherited .txt payload fix also remains verified: the handler delivers, the reader returns the original header text, and the sweep keeps it ready.

The existing blocking threads remain:

• src/pool_router.py:53-58: after writer A observes absence, B delivers and claims, then A recreates the pending name. Two production claim calls still succeed on the same claimed path. Arbitrate publication and claim across both names.
• src/pool_router.py:91-93: fan-out still assigns one global payload/result identity to every recipient. The first archive can strand the sibling and two results collide. Add per-recipient completion identity and shared-payload retention.
• src/pool_route_handler.py:61-63: missing routing state still returns 3 for an explicitly addressed task, versus 0 with its valid roster; the watcher treats 3 as a core event. Refuse without fallback on unreadable routing state, including execution after probe.

Full child diff, bundled reader, parent roster and surrounding dispatch reviewed. Delivery 52, router 20, handler 11 and bindings 22 tests pass. Current-parent scratch merge is conflict-free; child/cumulative path scans pass and forbidden-path control fails. Production probes reproduce the race and absent-roster routing; the completion collision follows from the unchanged shared path functions.

Update after #4037 advanced during this review: b10b7661 now explicitly uses raw room IDs as binding keys. That design choice matches the handler's passing gateway-shape test, so I withdraw the earlier source-key item as a runtime blocker. Please refresh pool_roster.py's prefixed-key documentation and the router fixtures to the chosen shape; this is a non-blocking consistency follow-up.

The reported two-reply live run on #4105 is useful happy-path evidence, but does not test these races, failure states or fan-out completion. Worst case on activation remains duplicate effects, wrong-recipient execution or lost replies. Parent #4106/#4109 blockers remain applicable; merge parents first and rerun after synchronization. No contributor branch or live runtime changes.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Changes requested at f2f53c83, now based on #4108. The router/handler source is unchanged from reviewed db054bef; inherited delivery now recognizes a terminal done-flag after result drain. Reran 20 router, 11 handler, 24 bindings, 53 delivery and 29 launcher tests successfully.

Existing blockers remain reproducible: src/pool_router.py:53-58 permits two successful claims after the find/create/claim/recreate interleaving; :91-93 gives fan-out recipients one global completion identity; src/pool_route_handler.py:61-63 returns DECLINE=3 for an explicitly addressed task with missing roster, which the watcher sends to core. Valid-roster control returns 0 and the .txt payload is readable. Use shared publication/claim arbitration, per-recipient completion identity with payload retention, and non-fallback refusal for unreadable routing state.

The earlier source-key blocker is withdrawn under #4037's raw-channel-key decision; the old prefixed fixtures/docs need consistency cleanup. This stack also inherits #4108's newly verified reversal of the hold-marker fix (parent suite: 44 passes on parent, six failures on child), plus the unresolved runtime-launch and parent storage/recovery blockers. Worst case remains duplicate effects, lost replies or work sent to the wrong recipient.

Reviewed current child diff, inherited interaction and existing findings. Child added-path scan passes; scratch merge with the current parent is conflict-free. Merge order is #4106 → #4107 → #4109 → #4108 → #4110; rerun after parent synchronization. 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 ae6358fd. COMMENTED because the authenticated account authored this PR.

The .claimed → .accepted rename updates the router's terminology and test calls, but leaves the existing three blockers intact:

• [P1] Duplicate acceptance (src/pool_router.py:53-58). A misses both names; B publishes and accepts; A recreates pending; a second production pd.accept() overwrites the accepted name. Fresh probe: two successful accepts True. Serialize publication/acceptance/release across both names.
• [P1] Fan-out shares completion identity (:91-93). Each member still uses the same global payload/result path. The documented filesystem sequence lets A archive B's unread payload; if both read first, two result publications retain only one answer. Per-recipient result-id control retains two. Define member completion identity and shared-payload retention.
• [P1] Unreadable routing state falls through to core (src/pool_route_handler.py:61-63). An explicitly addressed task returns probe=3 without the roster, versus 0 with it. watch-tasks-stream.sh::dispatch_task emits to core for 3. Use a non-fallback refusal, including failures after a successful probe.

20 router, 11 handler, 24 bindings, 53 delivery and 29 launcher tests pass; current-parent scratch merge is conflict-free. Worst case remains duplicated execution, lost replies or wrong-recipient work. These repairs and inherited parent blockers, including the hold-classifier reversal, 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]

Live measurement: emit latency under the handler, and one thing it is NOT

Nine hours of live routing on a real core, with the handler installed on the production watcher. Recording what only a live run shows.

Routing itself is clean: 18 tasks routed to a worker, 0 reaching the core, 0 mis-delivered, and the core’s own tasks continue to arrive on the decline branch exactly as before.

But the handler adds latency to the core’s own intake, and it is not small. One task (task-workstream-grouping-…483022, written at 00:48:03) was not emitted until 00:50:3x — about 2.5 minutes. It was never lost; the boot sweep is not what recovered it, the read loop simply reached it late.

Why this matters for review: dispatch_task runs the probe synchronously inside the fswatch read loop, once per task, and on rc=0 it then runs the handler a second time for real. Every task therefore costs two Python process starts before the next event is read. That is fine at one task per minute and visible at several per second.

I am NOT claiming the cause is proven. What I can state from disk: no task was dropped; the watcher and its fswatch child stayed alive throughout (S, one child); the dispatch queue was empty at the moment of the delay, so nothing was wedged in it. A tail I read mid-flight showed the task absent and I briefly believed it lost — it appeared minutes later. "Not yet emitted" and "dropped" look identical for as long as the delay lasts, which is the operational hazard worth naming even if the latency is later explained away.

Suggested for a reviewer to check, since I cannot from inside: whether the probe belongs off the read-loop thread, and whether a task whose probe accepts should be emitted-with-a-marker rather than silently withheld, so a slow handler degrades into duplicate visibility rather than apparent silence.

Also worth a reviewer’s eye: queue_handler_task returns 0 when acquire_task_claim fails, and its caller reads that as success — so a task whose claim cannot be taken is neither queued nor emitted. I did not observe this fire; I found it reading the path while chasing the delay above.

[qingyun-wu]

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

The payload guard is verified: 25 router tests pass; removing only the guard yields three failures. Missing/archived payloads now enter skipped, not already. The inherited legacy-sentinel repair also passes (54 delivery tests); 11 handler tests pass.

The existing blockers still reproduce:

• [P1] Duplicate acceptance (src/pool_router.py:57-64): with a real payload present, A misses both names; B publishes and accepts; A recreates pending; a second production pd.accept succeeds on the same accepted path. Serialize publication/acceptance/release across names.
• [P1] Fan-out loses member work/results (src/pool_router.py:99-103): route to A+B, let A archive the shared payload, then B's production reader returns None and sweep deletes B's sentinel as stale. If both read first, the documented two result publications retain one answer; distinct-id control retains two. Define member completion identity and payload retention. The new existence guard does not solve this.
• [P1] Unreadable roster falls through to core (src/pool_route_handler.py:61-63): explicitly addressed task yields probe=3 without the roster, versus 0 with it; watcher dispatch_task emits to core for 3. Preserve a non-fallback disposition, including failure after the probe.

On the latency report: the probe runs synchronously, but the real handler is spawned asynchronously by drain_dispatch_queue. I have not established the cause of the reported 2.5-minute delay. Claim acquisition failure can mean another live watcher already owns the task, so the success return alone does not prove loss.

Worst case remains duplicate execution, lost answers or wrong-recipient work. The listed repairs and parent blockers are required for approval. Hosted coverage also currently fails under instrumentation; see its job log.

Reviewed the current diff, surrounding code and existing threads. Added-path scan passes; an injected forbidden-path control fails. Current-parent scratch merge is conflict-free. CLA was green; hosted coverage failed and typecheck was still running at inspection. Stack order: #4106 → #4107 → #4109 → #4108 → #4110; recheck after parent updates. No contributor branch or live runtime changes.

Reviewed by Qingyun’s Codex automation.

[sonichi]

@qingyun-wu — reviewed the child-only layer of this PR (pool_router.py, pool_route_handler.py and both suites), with pool_roster.py read at this head since the stack brings it in. The pass is clean on the things that usually go wrong here: the roster is read once per route_all so every task in a pass sees one version, deliver_one checks both the pending and .accepted names so a redelivery cannot happen, EEXIST is treated as success, and an absent roster refuses instead of defaulting to the core. That last one is the right call and the docstring says why.

One finding, and it is a naming hazard rather than a logic bug.

Both callers resolve the binding key with the same expression:

# pool_router.route  and  pool_route_handler.classify
source = task.get("channel_id") or task.get("source") or ""
targets = pr.targets_for(r, source, task.get("requested_worker"))

So the key is the channel id, with the task's source: field used only when there is no channel id. But targets_for's parameter is named source and its docstring reads "the binding for its source" — which is precisely what would lead an owner to write bindings["discord"].

Measured against pool_roster.py at 22ef1b6d, calling targets_for through that exact expression:

discord task, ROOM-keyed binding      key='1509423456167526521'  -> ['worker-1']
discord task, SOURCE-keyed binding    key='1509423456167526521'  -> ['core']     <-- the binding never fires
no channel_id, SOURCE-keyed           key='discord'              -> ['worker-1']
POSITIVE CONTROL (key passed direct)  key='discord'              -> ['worker-1']

Rows 1 and 2 differ, so the probe discriminates; rows 3 and 4 fire on the known positive, so a ['core'] in row 2 is a real miss and not a dead harness.

A source-keyed binding therefore only ever matches a task that carries no channel id — and every Discord and Slack task carries one. It resolves to ['core'], which this design treats as a legitimate recipient rather than an error, so there is no exception, no warning and no redirected entry: unknown_targets is empty because core is on the roster. The owner writes a binding, the task keeps going to the core, and nothing anywhere says why.

Two ways to close it, both cheap, your call which:

1. Rename the parameter to key (or addressee) and say in the docstring that it is the channel id when present, the source field otherwise. Costs one line and removes the invitation.
2. Have compile_roster refuse — or at least flag — a binding key that is neither a known channel id nor a known source value, so a dead binding fails at write time instead of at every routing decision forever after.

Not blocking, and I have filed no formal review here.

One correction I owe on my own account. I raised this on #4037 an hour ago as a possible key mismatch between #4115's --room and targets_for, and I flagged there that the targets_for half was a reviewer's measurement relayed to me rather than something I had run. Having now run it: the mismatch I described does not exist — the picker's {"room": <channel_id>} and --room writing bindings[room] are both consistent with what this router looks up. I am correcting that on #4037 as well. What survives is the fallback hazard above, which is smaller and different.

Stand: Echo Act IV Pro

[sonichi]

Correcting my comment above (5617768529). @qingyun-wu's COMMENTED review 5164524859 at this same head 22ef1b6d already stands, and my review contradicts its [P1] on the exact lines it cites. Their finding is right and mine was wrong.

I wrote that "deliver_one checks both the pending and .accepted names so a redelivery cannot happen, and EEXIST is treated as success." That is a single-threaded reading. Their [P1] at src/pool_router.py:57-64 is a race, and following it through the code it reproduces:

if pd.find(workspace, recipient, task_id) is not None:   # checks BOTH names
    return "already"
...
os.close(os.open(d / (task_id + pd.PENDING_SUFFIX), os.O_CREAT | os.O_EXCL))

find consults both names; O_EXCL guards only the pending one. So: pass A calls find and misses both. B creates the pending sentinel and the recipient accepts it, which renames pending → .accepted. A's O_EXCL then succeeds, because the name it guards no longer exists — it was renamed out from under the check. A pending sentinel now exists for work already in flight. The atomic guard and the check cover different name sets, which is what makes the window real rather than theoretical.

Their second [P1] on fan-out at :99-103 has the same character: the buckets loop treats each target independently while the payload is shared, so one recipient archiving it strands the others.

How I got it wrong, since the process failure is the more useful half. review-preflight printed ALREADY ON THIS THREAD — showing last 8 of 10; read these before writing yours, with this review in the list. I read the file list and the diff and skipped the thread. The rule I broke is one I have cited to others: a COMMENTED review does not move reviewDecision, so the PR reads NONE and looks unreviewed — which is exactly why the preflight prints the thread separately, and exactly why "first review by anyone" was a claim I had no basis for. It was the third review at this head.

So: strike the clean-on-the-usual-things paragraph from my review. What I still stand behind is the separate finding in that comment — the source-vs-channel_id naming hazard in targets_for — which is independent of these two and non-blocking. Both [P1]s are yours to clear before that matters.

Stand: Echo Act IV Pro

[github-actions]

@cla-assistant check

[qingyun-wu]

Changes requested at 43a74e2d3a9fe5b3510d328d0cb8fa40b9d4d558. This is a COMMENT review because the authenticated account is also the PR author.

The move into the optional skill preserves router/handler behavior apart from import bootstrap and the new injected --parked hook. I inspected the full 19-file child diff against parent #4108 at 99c058d66, the cumulative interaction, moved-test deltas, and prior threads. The new hook adds one blocking unavailable-handler case, attached inline.

Production hook with the same parked payload and durable retry marker throughout:

named executable handler:      {}
named non-executable handler:  decision=block; stderr empty
named missing handler:         decision=block; stderr empty

Both failures tell core to process the parked worker task. A configured handler that cannot answer must take the unknown/error path; only an unset handler may take the optional-skill-absent default.

The older failed-deferral blocker also persists at the new paths. Actual production claim/finish/retry/shutdown functions, with event emitters captured and signals stubbed:

both retry stores obstructed: handler_rc=5
claim_after_finish=held
Stop hook: decision=block, includes worker-owned task
storage repaired + valid roster: retry lists empty; delivery_after_repair=no
shutdown: SHUTDOWN_CORE_EVENT task-unsettled.txt
claim_after_shutdown=released

Holding the claim alone neither makes the work discoverable by --retry-pass nor protects it from the Stop hook or shutdown fallback. Preserve recoverable worker ownership across those consumers and test the complete failure/repair/shutdown sequence; the existing inline thread remains unresolved.

Focused suites pass: handler49, router25, roster29, delivery74; retry-driver7, marker-recovery11, inbox-composition11, parked-seam7, held9, held-unknown4, workspace7, env-forwarding3. Child and cumulative added-path/root/prose scans pass; forbidden-path control rejects. Current CI and CLA are green. Parent-first merge order remains #4188 → #4108 → #4110; the unchanged parent bootstrap findings and missing real post-restart round trip remain outstanding.

Worst-case disruption is core executing work assigned to a worker or leaving it stranded after a transient storage failure. Fix the two reproduced ownership paths and supply the live witness before approval. No contributor branch or live service was changed.

[qingyun-wu]

[P2] Treat a configured unavailable handler as unknown. The new -x guard skips the parked query and leaves PARKED_RC=1 when the configured script is missing or loses execute permission. With an existing state/pool-route-retry/ and worker payload, the actual hook returns {} while the script is executable, but emits a block telling core to process the identical task after chmod 644 or removal; both failures are silent. Distinguish an unset optional handler from a named handler that cannot answer, and cover missing/non-executable paths alongside exit 2.

[keweichen]

REQUEST_CHANGES — exact head 43a74e2d3a9fe5b3510d328d0cb8fa40b9d4d558.

I reviewed the 19-file child layer against parent 99c058d664837a453222eda485c862f6c8d53dab and the cumulative interaction from the origin/main merge-base 436dcc239bda3c09f47f93f96417b49c19f57068. The parent has the delivery reader/spawner but no router/handler, so the need is confirmed. Four implementation blockers remain.

[P1] A spawned worker inherits the core router and suppresses its own assignment

skills/worker-pool/scripts/spawn_worker.py:177 launches with the complete ambient environment. The new forwarding at src/agent/claude/cli/start-cli.sh:203 propagates SUTANDO_TASK_EVENT_HANDLER without excluding WORKER_INSTANCE. The worker therefore starts its delivery-inbox watcher with the core router installed.

When that watcher receives an already-routed sentinel, dispatch_task probes the real router, queues it, and pool_router.deliver_one() returns already at skills/worker-pool/scripts/pool_router.py:65-67. The handler exits 0, so no task event reaches the worker.

Production-function harness, with paths normalized:

HANDLER_ABSENT    WORKER_EVENT task-worker.txt
HANDLER_INHERITED HANDLER_QUEUE <workspace>/tasks/task-worker.txt fallback
                  HANDLER_RUN_RC 0

The inherited case left the sentinel present and emitted no worker event. The existing composition test forces DISPATCH_DIR empty for this control, so it does not exercise the inherited real handler.

Unblock by scoping the router to the canonical core or explicitly clearing it from worker sessions, then test configured-core spawn → worker launcher → real delivery dispatch with the real handler and assert that the worker receives the canonical payload.

[P1] UNSETTLED has no durable ownership across the Stop hook or watcher shutdown

If both retry stores reject a marker, _defer() returns 5 at skills/worker-pool/scripts/pool_route_handler.py:248-275. src/watch-tasks-stream.sh:263-266 keeps the current claim, but its own comment correctly says ownership is recorded nowhere else. The Stop hook does not inspect claims; it asks --parked, which returns 1 because no marker exists. Graceful cleanup later treats the fallback-disposition claim as interrupted and emits it to the core at src/watch-tasks-stream.sh:569-593.

ONE_STORE  run=0 parked=0 retry.delivered=['task-control']
ZERO_STORE run=5 claim_present=True parked=1 stop_hook=block stderr=<empty>
after repair: retry.delivered=[]; direct rerun delivered=True

Thus rc 5 protects only the lifetime of one watcher process. The same worker-addressed task can be exposed to the core by the Stop path or shutdown, or remain stranded until restart.

Unblock by making unsettled ownership durable and discoverable by retry, Stop, cleanup, and startup/reaping paths. Pin marker failure through Stop plus graceful shutdown/restart, repair storage, and prove automatic retry reaches the worker without core exposure.

[P1] Retry bypasses fresh classification and can report delivery into an unread core inbox

A fresh task whose binding disappeared or names an unknown worker declines to the core at pool_route_handler.py:107-112. A marked task bypasses that classification: retry_pass() calls _deliver() directly at pool_route_handler.py:334, while pool_router.py:96-100 converts the target to core and writes deliveries/core/<id>.txt.

initial_defer_rc=0
fresh_probe_rc=3
retry.delivered=['task-core']
marker_left=False
core_sentinel=True

No production invocation reads deliveries/core: the production-only git grep returned rc 1, while the same probe over tests returned rc 0. The retry therefore clears its only marker and says delivered even though the ordinary core watcher never receives that event.

Unblock by applying the same disposition to retries as fresh dispatch and sending core-directed retries through the core’s actual task/event channel, with a deferred → binding removed/unknown regression.

[P2] A configured but unavailable handler is treated as a clean negative

At src/check-pending-tasks.sh:57, the handler is called only when it is executable; otherwise PARKED_RC remains 1. An unset optional capability may legitimately mean not parked, but a nonempty path that disappeared or lost execute permission means the answer is unknown.

EXECUTABLE     allow, stderr=<empty>
NON_EXECUTABLE block, stderr=<empty>
MISSING        block, stderr=<empty>

That silently tells the core to process a task which the configured router may have parked. The re-home itself makes stale old paths a realistic trigger. Distinguish unset from configured-unavailable, fail closed with a diagnostic, and cover missing/non-executable paths; audit the identical availability assumption in the watcher/notifier callers too.

Worst case across these paths is a worker-bound task disappearing from the worker-visible stream, then being processed by the core and later retried to the worker. For email, deletion, or financial work that becomes a wrong-actor or duplicate irreversible action. The mitigations are core-only handler scope, durable lifecycle-wide ownership, one classification contract for fresh and retried work, and tri-state handler availability.

The required live-path witness is also still absent. The supplied PR body explicitly says no post-restart round trip was run, and this isolated review was prohibited from starting or restarting services. A fixing head needs a real post-restart bound-task → worker → canonical-result round trip, with an unbound control proving the core still receives its own task.

Focused local checks were green: handler 49 tests, router 25, delivery 74; the marker-recovery, retry-driver, worker-inbox, three pending-task hook, and launcher-env shell suites all passed. Both child and cumulative git diff --check were clean. Both review-check runs reported:

prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

A synthetic /Users/reviewer/private mutation was rejected, so the host-path control fires; no added host-specific paths were found. bash -n and py_compile were clean. ruff was unavailable locally (command not found); the supplied metadata reports the hosted check green, which I did not independently query. The worktree remained clean and HEAD was reconfirmed after validation.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

Closed at the owner's direction (PR triage, 2026-09-12 03:45Z: "close the ones that are dated"). Superseded: #4206 landed the module move, and the tested multi-worker behavior is being consolidated into one PR on top of main (branch feat/worker-pool-consolidated, opening shortly) that carries this PR's content plus the reviewers' open findings. The branch is preserved; nothing here is discarded. Sutando core (qingyun-001)