[john-the-dev]

Priority

Level: high
Reason: P1 (master-DB row rmvcrcbjtbh8et0): after a usage-limit stop, ~92 false "restricted runtime failed" replies were posted into team rooms in one burst.

Closes

Refs #5673 (fixes its suggested fixes 2 and 3; fix 1, orphan-check honoring watcher claims, is not needed for this symptom — see below).

Summary

/startup's task-orphan-check archived the inbox (mv tasks/<id>.txt tasks/archive/) while the standby watcher was still running the worker-pool router on those same tasks. pool_route_handler.read_task then hit FileNotFoundError, guarded_main returned must-handle (4), and run_handler_now (src/watch-tasks-stream.sh, the verdict 0 / failed branch) called publish_terminal_failure, which wrote "I could not safely process this Team-tier task because the restricted runtime failed…" as each task's result, delivered to its room.

Fix: on a failed must-handle run, if the payload no longer exists, the watcher logs it, withdraws its pending worker-stage hold (abandon) and releases the claim, publishing nothing. Whoever moved the payload owns the task now (orphan-check archives Team-tier tasks with [no-send] and aggregates owner-tier ones into one DM). A genuine failure with the payload still in place publishes the terminal failure exactly as before; the timed-out branch is unchanged.

Why not the classify.py option: a "held when a standby claim exists" verdict only protects tasks claimed at the instant the classifier runs. The standby's sweep can still reach a task right after it is archived, which fails the same way, so the guard has to be where the failure is published.

Before / after evidence

New test tests/watch-tasks-stream-vanished-payload.test.py runs the real watcher with a handler that probes 4, then moves its own task file into tasks/archive/ mid-run and exits 4.

Parent b028ebbb (fix reverted, test kept):

  ok   vanished: the handler ran and the claim was released
  ok   vanished: the payload really was moved away mid-run
  FAIL vanished: no terminal-failure result is published
  ok   vanished: nothing is handed to the live core
  ok   control: the claim settles with the payload in place
  ok   control: a genuine must-handle failure still publishes its terminal failure
  ok   control: nothing is handed to the live core
FAILED — vanished: no terminal-failure result is published

HEAD:

  ok   vanished: the handler ran and the claim was released
  ok   vanished: the payload really was moved away mid-run
  ok   vanished: no terminal-failure result is published
  ok   vanished: nothing is handed to the live core
  ok   control: the claim settles with the payload in place
  ok   control: a genuine must-handle failure still publishes its terminal failure
  ok   control: nothing is handed to the live core
PASS — a vanished payload publishes no terminal failure

(__pycache__ cleared before each run.)

Test plan

Ran every tests/watch-tasks-stream-*.test.{py,sh} plus tests/task-dispatch-contract.test.py locally on macOS (bash 3.2):

• Pass on the branch: 27 suites, including handler-terminal-rc, dead-worker-reap, shutdown-leaves-inflight-claim, task-dispatch-contract, and the new test.
• watch-tasks-stream-pool-writer.test.sh: its structural count of record_worker_done "$filename" abandon sites goes from 1 to 2 (the new branch withdraws the pending hold too). Expectation and its comment updated; it passes.
• Fail identically on origin/main b028ebbb on this host, so they are pre-existing and not from this change: config-hot-reload (2/3 on both), fallback-receipt-scope (4/6 on both), interrupted-task-retry (8 failures on both), malformed-roster-row (same 3 FAILs on both). CI is the check for those.

Not done: a live post-restart round trip (block a core on a usage limit, let team tasks queue under the standby, run /startup). The change is one branch on the failure path of a must-handle run.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

Diff Coverage

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

No lines with coverage information in this diff.

[sonichi]

Verdict: approvable (reviewed at 7056fae5; comment-only review)

What I checked

1. The bug exists on main, and the test fails for the right reason. I reverted only src/watch-tasks-stream.sh to the parent and kept the new test. One check fails, vanished: no terminal-failure result is published. Its stderr shows the old branch firing: required Team handler failed for task-demo.txt (exit 4); publishing safe terminal failure. At HEAD all 7 checks pass, and tests/watch-tasks-stream-pool-writer.test.sh passes too.

2. How "already archived" is decided (src/watch-tasks-stream.sh:759-762).

   • The new branch is reached only when the handler failed, the claim is still ours, the verdict is must-handle and the run did not time out.
   • It does not decide "archived" as such. It decides "the payload is no longer at $task_path".
   • Moved by a legitimate claim: the worker-pool router delivers by sentinel and never moves the payload (pool_router.py, pool_delivery.py). resolve_inbox_entry.py returns the payload's own path. The old claim_task.py rename to .claimed-core-N no longer exists in the tree, and nothing else renames the payload as a claim.
   • Other movers I found: orphan-check (archives DONE tasks, aggregates deferred ones into the owner DM), the bridges' archive-after-delivery, the codex NO-REPLY mv in discord-bridge.py, and the voice-cancel unlink in task-bridge.ts. Each one deliberately takes the task over. "The mover owns it" holds for all of them.
   • Timing: the -e check runs after the handler exits and is not atomic with publishing. A move that lands after the check falls through to the old publish, so a race only ever errs toward the old behaviour.

3. No double delivery.

   • The branch writes no result, withdraws the pending hold (abandon) and releases the claim through the shared claim_settled=1 path.
   • The payload is gone from the inbox, so the watcher cannot dispatch it again.
   • If orphan-check later re-queues it (mv tasks/archive/<id>.txt tasks/), it gets one fresh run under a fresh claim and nothing was published the first time. At most one answer goes out.

4. CI: every check is green at 7056fae5 (the Windows jobs were skipped).

Non-blocking

• src/watch-tasks-stream.sh:760: the silence leaves only a stderr line. If a future mover (for example a manual cleanup or a workspace migration) moves a payload without answering, the task disappears with no durable trace. Consider also writing a receipt or an activity row so the owner can audit the "another owner took it" decisions.
• tests/watch-tasks-stream-vanished-payload.test.py does not assert that the abandon worker record was written. The pool-writer test covers it only by counting call sites. A one-line functional assert on the worker record after the vanished run would pin it.
• The PR body says orphan-check archives Team-tier tasks with [no-send]. Since v0.1.4, skills/task-orphan-check/SKILL.md folds Team/other-tier orphans into the aggregated owner DM. The conclusion still holds; only the description is out of date.

Worst-case disruption: a failed must-handle task whose payload was moved by something that does not answer it ends silently instead of with a refusal. Today every mover I found answers the task or deliberately silences it, so this is acceptable. The receipt above would mitigate it.

[liususan091219]

Ready to approve once the CLA check is present and green, and a write-access maintainer files an APPROVED review. The code is clean at 7056fae5.

What I verified

• Revert-to-fail: with only src/watch-tasks-stream.sh reverted to the parent, tests/watch-tasks-stream-vanished-payload.test.py fails vanished: no terminal-failure result is published. At HEAD all 7 checks pass, and tests/watch-tasks-stream-pool-writer.test.sh passes.
• The new test is behavioral: it runs the real watcher with a handler that moves its own payload mid-run, and it carries a payload-in-place control that still publishes. It does not re-implement the unit under test.
• Scope of the new branch (src/watch-tasks-stream.sh:759-762): it fires only on a failed must-handle run that did not time out, with the payload gone. The genuine-failure path (:767) and the timed-out path (:758) are unchanged. I agree with the existing comment review that a race only errs toward the old publish.
• CI is green (Windows skipped). There are no unresolved threads and no hardcoded paths.

Not yet met

1. license/cla is ABSENT at this head. The commit-status endpoint returns state: pending with zero contexts. That is not a pass. The commit email is the GitHub noreply form, so a CLA re-check should clear it.
2. There is no APPROVED review. The only review is a COMMENTED "approvable" from the owner, and a comment does not count toward the approval gate.

Non-blocking

• tests/watch-tasks-stream-pool-writer.test.sh:59-60 bumps a source-grep count (grep -c 'record_worker_done ... abandon') from 1 to 2. This is the REVIEW.md #14 shape. A functional assert that the vanished run writes the abandon worker record would pin the same thing by behavior.
• The vanished branch leaves only a stderr line. A durable receipt would let the owner audit "another owner took it".

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Agent-assisted review (Sutando qingyun-001) of the whole PR at 7056fae.

What I ran:

• tests/watch-tasks-stream-vanished-payload.test.py: 7/7 pass.
• tests/watch-tasks-stream-pool-writer.test.sh: passes.
• scripts/review-checks.sh: passes.
• I inverted the -e "$task_path" guard at src/watch-tasks-stream.sh:759. Both the vanished case and the control fail, so the test exercises the real guard.

Idempotency: the branch runs once per claim, writes no result, and the payload is gone, so it cannot be redispatched. If the payload reappears after the -e check, the code falls back to the existing publish path, never a second send.

Non-blocking, already noted by other reviewers: a failing record_worker_done abandon at :759-764 is swallowed (|| true), and the claim is released anyway. Only a stderr line records it. There is no double-publish risk.

Approving. The merge is the owner's call, after the post-restart round trip on an affected service.

[john-the-dev]

REVIEW.md §15 post-restart round trip (live witness), run on a second host (Rui's Sutando II, Codex core) on 2026-10-20 UTC.

Candidate 6a265e6eb7117f39f663a57011bda6aa2c388af3 = main 7f7f56a8 + #5736 7056fae5 + #5737 e08d0599 + #5768 a537f9c3 (ancestry verified). Built out of place with ENGINE_SUTANDO_DIR/ENGINE_SERVICES_OUT/ENGINE_MANIFEST_OUT bash build-services.sh, then staged with a bounded rollback runner.

Live round trips through the restarted gateway:

• 19:06:08: the candidate gateway restart completed (the watcher was restarted too).
• 19:06:38: a fresh task LIVE-WITNESS-572FE-CANDIDATE-2 arrived through it.
• 19:06:56: the normal result-file reply was delivered, and the peer (me) confirmed receipt.
• 19:07:27: the original engine was restored and the gateway restarted.
• 19:07:53: a fresh LIVE-WITNESS-572FE-RESTORED task arrived.
• 19:08:27: the reply was delivered and confirmed.
• Final status: core=running gateway=running availability=online gateway_state=connected. The restored files and manifest match the app bundle byte for byte.

Edge probes on the candidate, as controlled fixtures:

• tests/watch-tasks-stream-vanished-payload.test.py passes. A vanished payload publishes no terminal failure, and a genuine must-handle failure still does.
• src/remote-gateway-bridge.test.py passes, 352 checks green, including:
  • a room_upgraded 400 keeps the nudge held and notifies the owner exactly once across passes;
  • a failed notice is retried next pass without re-POSTing the DM;
  • one notice attempt is counted per pass;
  • an empty room '' or ' ' is refused.

Boundaries:

• The core session itself stayed running (the gateway and watcher were restarted), so this does not witness a full core bootstrap.
• The edge cases used fixture rooms and a simulated upgrade/notification hook. No real owner DM was upgraded and no real queued task was removed.

[john-the-dev]

@cla-assistant check