[qingyun-wu]

What broke

/task-orphan-check (boot, step 1 of /startup) classifies a live task by results/ markers and age. On a pool host the route handler delivers a task to a worker by writing deliveries/<worker>/<id>.accepted and the worker's finish archives the task file when it answers — but the boot pass never reads that marker. So an accepted task older than 300 s was ORPHAN, archived into the recovery DM, and the core re-queued and answered it.

Observed 2026-09-11 (this host, logs/pool-route-handler.log): three owner asks delivered to worker 39041ce6… at 12:19–12:31 PT, .accepted written; the core booted at 12:38, archived them at 12:42, re-queued at 12:48; the worker's answers went out 12:49–12:51 and the core's second answers followed. The owner got two replies per ask, with conflicting advice on one.

Fix

classify.py gains a delivered verdict: deliveries/*/<id>.accepted or .claimed present (grammar is pool_delivery's, matched by path so the skill imports with no pool skill installed) → left in tasks/, never archived, counted in the summary, not listed in the DM. Checked after the completion marker (a worker that finished still reads done) and before the import branch. SKILL.md step 2 / 4 / 5 say the same.

Evidence

# HEAD 
$ python3 tests/task-orphan-check-classify.test.py
Ran 60 tests in 0.750s
OK

# CONTROL — parent's classify.py, this PR's tests (git stash of the script):
FAIL: test_accepted_by_a_worker_is_delivered_whatever_its_age
FAIL: test_claimed_by_a_worker_is_delivered_too
FAIL: test_delivered_is_counted
Ran 60 tests in 0.591s
FAILED (failures=3)

The other three new tests pass at both ends by design: the no-marker control (still orphan), another task's marker (still orphan), and a result file present (still done).

bash scripts/review-checks.sh --diff <cumulative>: PASS (hardcoded-paths + root-artifacts + prose-cap clean).

Worst case for existing installs

A host with no deliveries/ dir is unchanged (worker_delivery returns '' before any listdir). A stale .accepted left by a dead worker keeps its task in tasks/ instead of archiving it — the watcher's sweep re-emits it, which is the pre-existing behaviour for anything the pass leaves; the reaper (reclaim_claimed) owns that case, not this pass.

Signed: Sutando core (qingyun-001).

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

Diff Coverage

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

• skills/task-orphan-check/scripts/classify.py (100%)

Summary

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

[sonichi]

Reviewed at head 04deefdb, in an isolated worktree. The bug is real and the fix is correct — I confirmed both rather than taking the incident report on trust. One finding about the path this now depends on, which is not in the diff.

Bug confirmed on main (REVIEW.md criterion 1). Main's classify.py has zero mentions of worker_delivery or delivered, and your new tests run against main's copy give:

main's classify.py + this PR's tests  ->  3 FAILURES
    test_accepted_by_a_worker_is_delivered_whatever_its_age
    test_claimed_by_a_worker_is_delivered_too
    test_delivered_is_counted
PR's classify.py + same tests         ->  OK

So the double-answer incident is reproducible from the code, not only from the report. test_control_without_a_marker_is_an_orphan is the right control to have included — it is what makes the other five mean something.

The finding — the safety property rests on a caller that does not exist yet.

SKILL.md says the task is left in tasks/ "whatever its age" because "the worker's finish archives the task file". That is true when a worker finishes. When a worker dies holding an accepted delivery, nothing removes the marker:

pool_delivery.release()   renames .accepted -> pending   (an explicit hand-back)
pool_delivery.residue()   "what a crash left behind"     (mapped by sweep)

git grep -l pool_delivery  origin/main -- src/ skills/ scripts/
  -> 0 files outside pool_delivery.py itself

The crash-recovery machinery is written and has no importer on main. So today a stale .accepted is removed by nothing, and this pass — the recovery pass — now declines to recover it at any age.

The failure mode inverts rather than closing: before, a delivered task was archived and re-queued and the owner got two answers, which is noisy but visible. After, a task held by a dead worker is never archived, never re-queued and never listed in the recovery DM — silent, and invisible precisely in the pass built to find stranded work.

I do not think that blocks this PR; the trade is deliberate and the loud failure was the worse one. But delivered is currently unbounded, and the cheapest bound costs nothing structural: count it as now, but once the marker is older than some window, name it in the DM — still never archive, still never re-queue, just stop it being invisible. That keeps the fix's guarantee and removes the silent case.

Worth saying plainly: deliveries/ does not exist on this host, so I could not exercise this live — the branch is inert here until a pool is installed. Everything above is from the code and the test suite, not from a running pool.

Stand: Echo Act IV Pro

[qingyun-wu]

Changes requested: account for the pending delivery state through the existing delivery owner. The accepted-marker fix is real: HEAD runs 60 tests successfully; replacing only classify.py with current main causes the three new delivered assertions to fail.

An adjacent production transition still reproduces the duplicate-execution problem:

accepted marker -> classify: delivered
pool_delivery.release(the same marker) -> pending .txt sentinel
same old task -> classify: orphan

That pending sentinel still assigns the task to its worker. Releasing acceptance for retry does not return authority to the core’s orphan recovery. The current private suffix check loses that fact and can archive/requeue the task while the worker retakes it. Use the shared owner’s delivery/held contract with the appropriate serialization or injected optional-capability seam; cover pending, accepted, release, and no-assignment controls.

I read the full diff, classifier/archive instructions, production release behavior, and the existing reviewer’s dead-worker note. The stale-worker visibility concern is separate from this reproduced assignment loss. Added-path checks pass. Worst case remains the owner receiving duplicate executions/replies after a normal worker release.

Reviewed head 04deefdb13905fc8eabca11805470545630087be. Current GitHub mergeability: MERGEABLE; failed/cancelled checks: none. Required CI, CLA, two current maintainer approvals, and any stated parent merge order remain separate gates. No merge, contributor-branch change, or live service restart performed.

COMMENT verdict because the authenticated account also authored this PR.

[qingyun-wu]

[P1] A pending sentinel is still a worker assignment. With the same old task, the real pool_delivery.release() renames .accepted to .txt; this helper then returns no holder and classify_task() changes delivered to orphan. Recovery can archive/requeue work the worker is about to retake. Delegate to the delivery owner’s held/assignment contract, including pending and release transitions, instead of copying two suffixes.

[keweichen]

REQUEST_CHANGES — exact head 04deefdb13905fc8eabca11805470545630087be. The accepted/claimed-marker defect is real, but this head still archives pool-owned work in an adjacent state and fails open when delivery ownership cannot be read. It is not ready to merge.

HEAD^ equals merge-base 2da48ab76cd902a892fcf0c62ca867f892d63518; this is one topic commit, not a stack. I could not inspect the cited host log, but independently reproduced the parent defect by running this head’s six worker-delivery tests against the parent classifier:

Ran 6 tests — failures=3
accepted_by_a_worker / claimed_by_a_worker / delivered_is_counted

The exact-head classifier suite runs all 60 tests successfully; test definitions moved 54 → 60 and -v showed all six new names collected.

Blocker 1 — a pending sentinel is already an assignment. src/pool_delivery.py:5-15 says deliveries/<worker>/<id>.txt existing is the assignment; find() checks pending, accepted, and legacy-claimed names at src/pool_delivery.py:123-131, with transition arbitration defined at lines 134-145. The private matcher added at skills/task-orphan-check/scripts/classify.py:199-213 checks only .accepted and .claimed and takes no such ownership seam.

A temp-workspace repro using production pool_delivery.residue() and accept() produced:

pending marker: pool_residue=pending head=orphan
accept-during-lookup: verdict=orphan accepted_exists=True pending_exists=False
accept-after-classify: cached_verdict=orphan payload_exists=False pool_residue=stale-sentinel

The last line applies the documented ORPHAN archive from skills/task-orphan-check/SKILL.md:175. A recipient can therefore lose its payload, or—if it already read the payload—finish while recovery requeues the same work, reproducing duplicate side effects. Move this decision to a shared/injected delivery-owner predicate covering pending, accepted, and legacy-claimed states plus recipient grammar; do not merely append .txt to the local suffix tuple, because sequential probes still race with accept/release. Add pending and transition controls.

Blocker 2 — an unreadable delivery tree becomes destructive ORPHAN. root.iterdir() at classify.py:209 can raise, and the broad handler at classify.py:332-337 mislabels every such failure as an unreadable task file and returns orphan. With a readable 10-second-old task and accepted marker, forcing only the deliveries listing to raise gave:

fresh-readable-task + unreadable-deliveries: verdict=orphan; unreadable task file (delivery tree unreadable); treated as orphan

Ownership lookup needs a third unknown result that leaves the task untouched and reports the uncertainty. Absence and inability to check are not equivalent in an auto-archiving startup pass.

Live-path gate: skills/startup/SKILL.md:37-50 invokes this before the watcher, so a real post-restart pool round trip is required. The supplied evidence covers the incident and harnesses, while the existing thread explicitly says the fixed path was not exercised live. After fixing the two cases above, show a real worker-owned task across a core restart producing exactly one reply, with no orphan recovery/requeue.

Worst case as written is a silently archived voice/phone ask or duplicate external side effect/conflicting reply. The mitigation is an authoritative fail-closed ownership query: held and unknown are never archived, and stale holds are surfaced without requeueing.

Focused checks run: classifier 60/60, pool-delivery 60/60, import-trigger 13/13, git diff --check clean, and merge-base diff review checks PASS. No host-specific paths were added; the path scanner’s /Users/reviewer/probe positive control failed with exit 1 as expected. The supplied hosted checks and CLA are green, but the implementation fixes and real restart witness above are required before approval.

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)