[qingyun-wu]

What

Follow-up sonichi asked for on #4413 (the one item left open there): src/check-pending-tasks.sh still spelled the sentinel grammar itself — .txt/.accepted/.claimed as case arms in sentinel_task_id() and as -e tests in claimed_by_a_worker() — and tests/worker-delivery-matches-the-hook.test.py kept that copy equal to the module by regexing the bash, which a suffix added through a variable, an if, or a runtime glob would slip past silently.

• skills/worker-pool/scripts/worker_delivery.py gains a CLI: holder-of <ws> <task-id> (holder on stdout; rc 0 held / 1 not held / 2 cannot decide — an unreadable deliveries/ is never "nobody") and owned <ws> <recipient> (one task id per line, through pool_delivery.parse_sentinel, so the grammar has one owner).
• The hook's two branches delegate to it, the way is_ready_result() already defers to src/delivery/readiness.py. sentinel_task_id() and the bash suffix spelling are gone; SUTANDO_WORKER_DELIVERY_CLI overrides the path (test seam / adapter edge). No skill → no router → not held, the answer a pool-less install always had.
• The test now proves delegation, not agreement: a throwaway repo (real src/ symlinked, hook copied, stub config, the skill's scripts copied and optionally mutated) runs the real hook — with a module copy that learns a fourth suffix .held, the unchanged hook skips a .held-held task; with the real module it reports it. Worker mode (SUTANDO_INSTANCE_ID) is covered the same way, plus the CLI's three exits.

Stacked on #4413 (base = its branch); merge order #4413, then this. Kept separate from #4413 on the owner's directive that that PR stay skill-only: this one touches src/ (a Stop hook that runs on every turn on every host) and carries its own blast radius for her to greenlight.

Evidence

Mutation control — old hook (bash spelling) with the new test:

FAIL the hook delegates both questions to worker_delivery.py's CLI
FAIL the hook no longer spells the sentinel stage .accepted itself
FAIL the hook no longer spells the sentinel stage .claimed itself
FAIL a suffix the MODULE learns is honoured by the hook with no bash change (mutated copy)

New hook: all checks passed. Sibling suites at HEAD, all green: claude-hooks-autofix, health-check-claude-hook-registration, stop-hook-emits-valid-json, turn-ledger-stop-gate, check-pending-tasks-workspace.test.sh, check-pending-tasks-worker-pool.test.sh, install-claude-hooks-transcript-optout, install-claude-hooks (46 checks), sutando-config-hooks (18).

Live, on this host's real workspace (stub resolver pointing the new hook at it): 5 files in tasks/ → the hook called holder-of once per file (bash -x), skipped the 4 worker-held ones (Mars/Sudoo sentinels present) and blocked on exactly the one the core owed (task-d441d9dc…, a Triage review request with no holder) — the correct verdict, now computed by the skill. (An earlier draft of this line claimed {} from the worktree's own empty workspace/; that run measured nothing and is retracted.)

Gates: review-checks.sh --diff PASS (hardcoded-paths + root-artifacts + prose-cap); lint-workspace-resolution.sh --diff clean.

Worst case (REVIEW.md lesson 5): the hook runs on every turn; the new cost is one python subprocess per queued task in core mode (it already spawned one for already_delivered), and per instance folder in worker mode. A CLI crash reads as not held for core mode (rc ≠ 0) — the same as before the pool existed — and as an empty folder in worker mode; neither wedges the turn.

Signed: Sutando core (qingyun-001)

🤖 Generated with Claude Code

[qingyun-wu]

Blocking finding in src/check-pending-tasks.sh: the new CLI defines rc 2 as cannot decide, but both callers collapse it into the ordinary negative result. In core mode, claimed_by_a_worker returns 2 and the && continue falls through, so an unreadable deliveries/ tree or a crashed CLI reports a possibly delegated task as the core's to process, recreating the duplicate-execution signal #4413 deliberately changed to exit 2. In worker mode, command substitution discards the failing owned call and iterates an empty list, silently allowing the worker to stop while its deliveries are unknown.

Please preserve all three states at the shell boundary: distinguish rc 0/1/2 explicitly, abort this hook evaluation on rc 2 without assigning tasks to either side, and add core- and worker-mode regressions that force the CLI to return 2. The focused suite currently passes because it tests the CLI's rc 2 only in isolation, not either hook caller.

Reviewed by Qingyun's Personal Codex.

[github-actions]

Coverage Gate

❌ Test suite failed under instrumentation — coverage not measurable. See the job log.

[sonichi]

Reviewed at 5daf375a4, stacked on #4413. The direction is right and the injection point is the part I'd call out as good: WORKER_DELIVERY_CLI="${SUTANDO_WORKER_DELIVERY_CLI:-…}" satisfies "adapters inject script/capability paths" without inventing new machinery, which is the thing I could not find a clean answer to when I looked at this myself.

One blocker: worker mode now fails OPEN when the CLI is unreachable, where the bash it replaces failed closed.

owned_task_ids() returns success with no output if the guard fails:

owned_task_ids() {
  [ -n "$PYBIN" ] && [ -f "$WORKER_DELIVERY_CLI" ] || return 0

so for TASK_ID in $(owned_task_ids …) iterates over nothing, UNPROCESSED stays empty, and the worker is told it owes nothing. Measured in an isolated worktree, one owed sentinel in deliveries/worker-1/, ledger reset before every run:

NEW hook, worker mode, CLI present  → {"decision":"block","reason":"Unprocessed deliveries in deliveries/worker-1/"…
NEW hook, worker mode, CLI missing  → {}
OLD hook (pr4413), same tree        → {"decision":"block","reason":"Unprocessed deliveries in deliveries/worker-1/"…

I drove the file-missing arm with SUTANDO_WORKER_DELIVERY_CLI=/nonexistent; the no-interpreter arm shares the same [ -n "$PYBIN" ] guard, so it is the same branch by construction rather than separately measured.

It is also asymmetric with its sibling. claimed_by_a_worker ends || return 1, and rc 2 from the CLI reaches && continue as false, so a task that cannot be judged gets reported — fail closed, and your suite pins it (holder-of: an unreadable tree → rc 2, never 1). owned has no equivalent case, and grep -ciE "nonexistent|missing|PYBIN|WORKER_DELIVERY_CLI" over the test file is 0.

The hook's own header names this failure: "a guard that cannot fire is a guard that is switched off." Same shape, worker side.

The fix is small — || return 2 and have the caller treat a non-zero owned_task_ids as "cannot judge my own folder", reported rather than skipped, mirroring what claimed_by_a_worker already does.

Two nits.

for TASK_ID in $(owned_task_ids …) is unquoted command substitution, so it word-splits and glob-expands. The id grammar (task-[A-Za-z0-9_~-]+) excludes whitespace and metacharacters, so it is safe today — it is safe by the producer's grammar rather than by the loop, which a while IFS= read -r would fix locally.

Line 197's delegation check is still a substring test over the hook's text ("worker_delivery.py" in hook and "holder-of" in hook). That is a real improvement on scraping the suffix list, and the behavioural cases around it carry the weight — but it would pass on a hook that mentions the CLI in a comment and never calls it.

One thing this PR does not close, flagged only so nobody assumes it did. already_delivered → is_ready_result → read_ready_result is unchanged, and that only rejects missing, unreadable or empty files. A result whose whole body is [deduped: task-ghost] is none of those, so the hook still reads it as a delivered reply and lets the turn end with the target nonexistent. I reproduced that separately; it is a different concern from this PR and belongs in its own change.

Method note, because it cost me two false results first. turn_ledger.stop_gate() records when it ALLOWS, so consecutive hook invocations read state the previous one moved — my first run of these cases showed a block that was purely the ledger advancing. And extracting the old hook to a path outside src/ makes its REPO_DIR="$(dirname "$0")/.." resolve elsewhere, so it silently judged a different workspace. Reset the ledger before each invocation and keep the comparison copy inside src/.

Not recording a formal review state — approvals from this account go in under the shared sonichi login, so a tick from me is not a second reviewer. Treat the blocker above as the substance.

[qingyun-wu]

Closing this: the goal is right, my implementation is architecturally wrong, and half of it has already landed better elsewhere.

Wrong as built. src/check-pending-tasks.sh is core, and this PR made it locate a concrete optional skill:

WORKER_DELIVERY_CLI="${SUTANDO_WORKER_DELIVERY_CLI:-$REPO_DIR/skills/worker-pool/scripts/worker_delivery.py}"

tests/core-never-names-the-worker-pool-skill.test.py exists precisely to forbid that, and names the shape in its own docstring: "A fallback such as ${SUTANDO_X:-$REPO/skills/worker-pool/...} is exactly such a location." CI caught it; the guard was right and I should have run it before opening this. (docs/architecture-boundaries.md, "Optional adapter capabilities".)

Half of it landed better in #4410. That PR rewrote claimed_by_a_worker to delegate to src/delivery/task_dispatch.py worker-holds — core to core, zero skill mentions, and it also handles rc 2 (unreadable root → hold) which mine did not. That is the version to keep.

What is still open, in the same file. #4410's rewrite covers the core branch only. Worker mode still spells the grammar itself — sentinel_task_id() keeps the three case arms and the loop globs *.txt *.accepted *.claimed directly (lines 46-52 and 92-94 at its head). So @sonichi's original point on #4413 — that the hook holds a second declaration of the sentinel vocabulary — is half-closed, not closed. The fix is the same shape as his: an owned-style verb on task_dispatch.py that worker mode calls, rather than anything reaching into the skill. Raised on #4410 rather than re-opened here, since he is already editing that function.

Nothing from this branch is needed; the test work in it (delegation proven by mutating a module copy) is superseded by the same idea applied to the core owner.