Skip to content

fix(task-notifier): re-deliver a submitted prompt that an API-error turn lost (StopFailure) - #5168

Open
sonichi wants to merge 10 commits into
mainfrom
fix/notifier-resend-after-api-error
Open

sonichi wants to merge 10 commits into
mainfrom
fix/notifier-resend-after-api-error

Conversation

@sonichi

@sonichi sonichi commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Problem

When an API error ends a core Claude Code turn (for example API Error: 502), the prompt the standby task-notifier typed into the core is consumed and lost. The notifier has already written the task's in-flight marker (state/task-notifier-inflight/<task>, set just before the Enter in press_enter_and_confirm). After that it refuses to re-type the task and logs prompt for <task> was already submitted to this core; awaiting its result, not re-typing until COMPLETION_TIMEOUT (3600s). The task waits about an hour.

Measured on one host on 2026-10-06:

  • 58 turns ended in API Error: 502 that day.
  • The core transcript has 394 stop_hook_summary records and 0 after the 61 API-error turns. The Stop hook does not fire on those turns, so nothing on the Stop path can see the loss.

Claude Code has a StopFailure hook event for this case. It fires instead of Stop when an API error ends the turn, and its input carries error (server_error, rate_limit, overloaded, authentication_failed, billing_error, …). Docs: https://code.claude.com/docs/en/hooks#stopfailure

Design

The policy and its state live in one place, src/delivery/turn_failure.py. It is dependency-light and follows task_dispatch.py's CLI shape for bash callers.

  1. StopFailure hook. src/stop-failure.sh is a thin wrapper in the same style as turn-start.sh. It runs turn_failure.py hook-stop-failure, which atomically writes <workspace>/state/core-turn-failure/<instance key|core>.json as {failed_at, error, session_id, recovered_at: null}.

    • The path comes from the new util_paths.turn_failure_path, keyed per instance like composer_block_path, so a pool worker's failure is not read as the core's.
    • Only transient errors are retryable: server_error, overloaded, rate_limit, unknown.
    • Errors that re-typing cannot fix are never recorded: authentication_failed, billing_error, oauth_org_not_allowed, account_on_hold, cloud_credential_error, invalid_request and model_not_found.
    • max_output_tokens is also excluded, because the same prompt would hit the same ceiling.
    • The hook always exits 0 and never writes stdout.
  2. Registration. The hook is added to HOOKS=(...) in src/install-claude-hooks.sh. The health-check claude-hooks probe parses that array, so it now expects the hook with no probe change. The probe test fixture is updated (5 → 6 owned hooks).

  3. Recovery signal. src/check-pending-tasks.sh (the Stop hook) runs turn_failure.py record-recovery for identified core/worker sessions. If a failure is recorded and has no recovery yet, this stamps recovered_at. The same module is the only writer.

  4. Notifier. While submit_task_grown waits for a result, it calls release_lost_submit every SUTANDO_NOTIFIER_RETRY_CHECK_SEC (default 5s), which runs turn_failure.py retry-due. The task is due only if a recorded failure is newer than the task's submission (the in-flight marker's mtime). It is then due:

    • at once, if a successful turn (recovery) is newer than the failure;
    • otherwise after a backoff of 30, 60, 120, 300, then 600s (capped), counted from the later of the failure and the previous re-delivery. The attempt count is persisted per task in state/task-notifier-retry/<task>.json.

    It is never due if any of these holds:

    • the task has a ready result (task_dispatch.has_ready_result);
    • its payload is gone from the inbox (archived);
    • it is worker-held (task_dispatch.worker_holds, skipped on a deliveries inbox, matching enqueue_announced_task; an unreadable deliveries root counts as held);
    • it has no in-flight marker.

    When the task is due, the notifier counts the attempt, clears the in-flight marker, logs re-delivering <task>: <reason> and returns. The task is still at the head of the queue, so the next pass types it again through the existing path, with the same healthy-core, empty-composer and staged-before-Enter guards. has_result also clears the per-task retry record.

Codex variant (src/agent/codex/cli/task-notifier.sh) is unchanged. Codex has no StopFailure event, so nothing would write the failure marker for it.

Scope. This covers tasks the standby notifier delivered. A prompt delivered by the session's own Monitor watcher is not tracked by the in-flight marker, so this change does not re-deliver it.

Edge case, accepted. A failure newer than the submit is taken as the turn that consumed our prompt. If our prompt was instead queued behind another task's turn and that turn failed, the prompt can be typed a second time. The result-exists guard stops any re-send once a result lands.

Evidence

Each suite was run with a scrubbed environment (env -i HOME PATH TMPDIR LANG USER, Python 3.12.15) on the parent 54d6465f (origin/main) and on HEAD. Command per file: python3 <file> or bash <file>.

Suite parent 54d6465 HEAD
tests/claude-task-notifier-gate.test.py Ran 20, OK Ran 20, OK
tests/claude-task-notifier-inflight.test.py Ran 21, OK (skipped=5) Ran 21, OK (skipped=5)
tests/claude-task-notifier-longprompt.test.py Ran 19, OK Ran 19, OK
tests/claude-task-notifier-submit.test.py Ran 17, OK Ran 17, OK
tests/claude-task-notifier.test.py Ran 17, OK Ran 17, OK
tests/claude-task-notifier-turn-failure.test.py (new) n/a Ran 3, OK
tests/turn-failure-redelivery.test.py (new) n/a Ran 17, OK
tests/task-notifier-resolved-announcement.test.py Ran 13, OK Ran 13, OK
tests/task-notifier-queue-head-is-fifo.test.py rc=0 rc=0
tests/task-notifier-standby-watcher-tagged.test.py rc=0, all PASS rc=0, all PASS
tests/claude-task-notifier-standby-log.test.sh ALL TESTS PASS ALL TESTS PASS
tests/task-notifier-supervisor-standby-gate.test.sh rc=0 rc=0
tests/health-check-claude-hook-registration.test.py Ran 56, OK Ran 56, OK (fixture updated)
tests/health-check-claude-task-notifier.test.py Ran 20, OK Ran 20, OK
tests/claude-hooks-autofix.test.py Ran 19, OK Ran 19, OK
tests/install-claude-hooks.test.sh PASS (52 checks) PASS (52 checks)
tests/install-claude-hooks-transcript-optout.test.sh rc=0 rc=0
tests/check-pending-tasks-{in-progress-and-repeat,watcher-coverage,worker-pool,workspace}.test.sh PASS ×4 PASS ×4
tests/turn-ledger-stop-gate.test.py PASS PASS
tests/start-cli-claude-task-notifier.test.py Ran 18, OK (skipped=17) Ran 18, OK (skipped=17)

What the new tests cover:

  • turn-failure-redelivery.test.py tests the pure policy:

    • a failure newer than the submission is due after the backoff;
    • a recovery newer than the failure makes the task due at once, and a stale recovery does not;
    • all eight non-transient errors are never due;
    • the backoff schedule is [30, 60, 120, 300, 600, 600, …].

    It also tests the on-disk guards (result exists, archived, worker-held, no marker), the CLI exit codes, and the real hook scripts run as subprocesses:

    • server_error writes the marker;
    • authentication_failed is ignored;
    • empty, non-JSON, non-object, wrong-type and wrong-event stdin all exit 0 with no output;
    • an unwritable state dir exits 0;
    • the real check-pending-tasks.sh stamps recovered_at for a SUTANDO_CORE_SESSION=1 session in a pinned test workspace.
  • claude-task-notifier-turn-failure.test.py runs the real notifier against the fake-tmux harness:

    • after a submit plus a server_error with recovery, the in-flight marker is released and the next pass types the prompt a second time;
    • after authentication_failed, the notifier still logs awaiting its result and does not type again;
    • a failure older than the submit does not release the task.

bash scripts/review-checks.sh --diff <(git diff origin/main) → review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean). python3 scripts/gen-src-map.py --check → up to date. ruff check on the new Python files → clean.

Not done / not verified

  • No live post-restart round trip. This touches a live delivery path, and I did not restart a core or cause a real API error. The StopFailure payload shape is taken from the Claude Code docs; I have not captured a real one from a live 502 turn. The notifier wiring is verified only against the fake-tmux harness.
  • shellcheck was not available on the build host, so the shell changes were not shellchecked locally (CI's shellcheck workflow will run).
  • Re-delivery waits for the notifier's next pass: up to RETRY_POLL_SEC (30s) in the main loop. In one-shot --event mode (diagnostic) the marker is released and nothing types it again until the next event.

Stand: Echo Act IV (Blue)

🤖 Generated with Claude Code

Update after review (commits 5f834ad..14537cf, rebased on main 2cee718)

Only the core's own turns count. src/session_identity.sh (sutando_session_identified) now holds the identity predicate the Stop hook already used. check-pending-tasks.sh, watcher-rearm-session-hint.sh, stop-failure.sh and turn-start.sh all source it. A guest Claude session in the checkout, such as a PR review or a one-shot skill, records no StopFailure and no turn.

A failure is blamed only on the turn our prompt started.

  • UserPromptSubmit (turn-start.sh → turn_failure.py hook-turn-start) records which task the prompt delivers. It reads the notifier's Sutando task ready: <file>. prefix and stores it in <state>/core-turn-failure/<key|core>.turn.json.
  • StopFailure stores that task with the failure, and only when the session matches.
  • Stop ends the turn.
  • decide_retry requires failure.task == filename.
  • A 502 in another prompt's turn, a Monitor-event turn, or any turn after a successful Stop re-types nothing.

Bounded.

  • MAX_ATTEMPTS = 3 re-deliveries per task.
  • After that, retry-due exits 3 once and stamps gave_up_at in <state>/task-notifier-retry/<file>.json.
  • The notifier logs not re-delivering <file>: 3 re-deliveries already lost to <error>; waiting out the completion timeout, then falls back to the ordinary completion timeout.

Evidence. The new tests were run against the source at 1eea8b2 and again at head:

Test 1eea8b2 head
HookTests.test_a_guest_session_records_nothing FAIL (2 subtests: marker written) OK
LostSubmitTests.test_a_failure_in_a_turn_our_prompt_did_not_start_keeps_waiting FAIL (re-delivering task-busy.txt) OK
LostSubmitTests.test_after_max_attempts_it_logs_once_and_keeps_waiting FAIL (no give-up line, re-typed) OK

Suites at head:

  • turn-failure-redelivery: 29 OK
  • claude-task-notifier-turn-failure: 6 OK
  • health-check-claude-hook-registration: 56 OK
  • claude-task-notifier: 17 OK
  • claude-task-notifier-inflight: 21 OK
  • claude-task-notifier-submit: 17 OK
  • The other hook suites that run check-pending-tasks.sh and turn-start.sh: pass, except check-pending-tasks-workspace, whose one failure also happens on origin/main on this host.
  • review-checks.sh on the diff: PASS.

Known limits.

  • A mid-turn 502 inside the prompt's own turn, after side effects have run, is still re-typed. The cap bounds it.
  • A Stop that the Stop hook blocks ends attribution for the rest of that continued turn. This is conservative.
  • No live post-restart round trip with a real StopFailure has been run yet.

@sonichi
sonichi requested a review from qingyun-wu as a code owner October 6, 2026 21:16
@sonichi

sonichi commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

CI note for reviewers: the 4 red checks are main's, not this PR's.

The failing files are agy-notifier-dependency-gate, agy-task-notifier, tmux-session-targets-are-exact and claude-task-notifier-submit. Those are the four main has failed on since #4303. They're the set #5161 fixes, and main's own CI runs at 54d6465f fail too.

This PR doesn't touch the agy notifier, and its notifier suites pass locally. I'll rebase and re-run once #5161 lands.

Stand: Echo Act IV (Blue)

@bassilkhilo-ag2 bassilkhilo-ag2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 1eea8b2. The single owner in delivery/turn_failure.py is the right shape, and the notifier test reproduces the bug: with base's task-notifier.sh and this PR's module, test_a_prompt_lost_to_a_502_is_released_and_typed_again fails. At head, all of the PR's suites pass (redelivery 17 OK, notifier-turn-failure 3 OK, hook-registration 56 OK, inflight 21 OK, notifier 17 OK). Three things gate merge, all on the double-delivery side.

  1. src/stop-failure.sh has no guest gate, unlike check-pending-tasks.sh:27-29. Repro: with SUTANDO_CORE_SESSION and SUTANDO_INSTANCE_ID unset, piping {"hook_event_name":"StopFailure","session_id":"guest-review-session","error":"server_error"} into the hook writes state/core-turn-failure/core.json. For a core task submitted 5 minutes earlier, retry_due then returns "backoff 30s elapsed (attempt 1)". So a 502 in any review or ad hoc Claude session in the checkout re-types the core's in-flight task. session_id is recorded but never checked.

  2. decide_retry blames any failure newer than the submit (turn_failure.py:88). Core tasks often span several turns (background agents, Monitor events, other prompts), so a 502 in a later, unrelated turn re-types a task that is still in progress. The same happens on a mid-turn 502 after side effects have already run. At 58 failures a day, this is the common case, not the queued-behind edge case the body accepts. Suggestion: key on the turn that consumed our prompt. UserPromptSubmit (turn-start.sh) already gets the prompt text, so record which task the turn started from and count a StopFailure only against that task.

  3. Re-delivery is unbounded. backoff_delay caps the delay at 600s but not the attempt count, recovery skips the backoff, and each re-submit resets started in submit_task_grown, so COMPLETION_TIMEOUT never ends it. A long rate_limit window or a recurring unknown error re-types the task every 10 minutes indefinitely. Please cap the attempts, then fall back to the ordinary timeout.

Nits:

  • checked=0 makes the first poll spawn the retry-due Python process at once. Initialising it to $started keeps the never-staged path fast.
  • task-notifier-retry/<task>.json is only cleared on a result, so it leaks for tasks archived without one.
  • unknown is a broad error class to retry.

CI: the four red files are main's. Base 54d6465f fails the same files in its own run, including claude-task-notifier-submit never_staged at 4.1s. main at e3b90f99 is now green, so a rebase should clear them. scripts/review-checks.sh passes on the diff, and fails as expected on a planted /Users/ line.

What would change this to approve: gate the StopFailure hook on core/worker identity, attribute a failure to the turn our prompt started, and bound the attempts.

@sonichi
sonichi force-pushed the fix/notifier-resend-after-api-error branch from 1eea8b2 to 14537cf Compare October 8, 2026 22:59
@sonichi

sonichi commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

Thanks @bassilkhilo-ag2. All three blockers are addressed, and the branch is rebased onto main at 2cee718 (which includes e3b90f9). The before results come from running the new tests against the old source at 1eea8b2. The after results are at 14537cf.

1. Guest gate. Commit 5f834ad.
The identity predicate that check-pending-tasks.sh used now lives in src/session_identity.sh as sutando_session_identified. check-pending-tasks.sh, stop-failure.sh and watcher-rearm-session-hint.sh all source it, so nothing copies the logic. A guest session's StopFailure now writes nothing.

  • Test: HookTests.test_a_guest_session_records_nothing in turn-failure-redelivery.test.py. It is your repro (session_id: guest-review-session), with the core marker unset and also with it set to 0.
  • Before: FAIL on both subtests (the marker was written). After: OK.
  • Added test_a_pool_worker_records_its_own_failure so the gate doesn't drop worker failures.

2. Attribution to the turn our prompt started. Commit 02e0715.

  • turn-start.sh (UserPromptSubmit) records which task the turn's prompt delivers in core-turn-failure/<key>.turn.json. It reads this from the notifier prompt's Sutando task ready: <file>. prefix (TASK_PROMPT_PREFIX).
  • StopFailure copies that task into the failure record, provided the session matches. The Stop hook ends the turn.
  • decide_retry re-delivers only when failure.task is this task.
  • Result: a 502 in a later prompt, a Monitor-event turn, or any turn after a successful Stop blames no task.
  • Behavioural test: LostSubmitTests.test_a_failure_in_a_turn_our_prompt_did_not_start_keeps_waiting in claude-task-notifier-turn-failure.test.py. Before: FAIL ("re-delivering task-busy.txt"). After: OK.
  • Hook tests: test_the_failure_is_blamed_on_the_task_whose_prompt_started_the_turn, ..._another_prompt_started_blames_no_task, ..._after_a_successful_stop_blames_no_task and ..._in_another_session_blames_no_task. These call a new API, so before they ERROR; after they are OK.
  • test_the_typed_prompt_is_the_one_turn_failure_attributes ties the notifier's typed prompt to the parser.

3. Attempt cap. Commit 4c1f2f5.

  • MAX_ATTEMPTS = 3 per task. After that, retry-due reports the give-up once, with exit 3. The record is stamped as gave_up_at in task-notifier-retry/<task>.json, next to attempts.
  • The notifier logs not re-delivering <task>: 3 re-deliveries already lost to <error>; waiting out the completion timeout and then waits out the ordinary timeout.
  • Behavioural test: LostSubmitTests.test_after_max_attempts_it_logs_once_and_keeps_waiting. Before: FAIL (0 give-up lines; it re-typed). After: OK.
  • Also added: test_re_delivery_stops_after_max_attempts, test_the_cap_is_reported_once_then_stays_quiet, and the exit-3 case in test_cli_retry_due_exit_codes.

Nits

  • Fixed: checked="$started" (3443526).
  • Still open:
    • The retry record still leaks for a task that is archived without a result. It is cleared only by a result.
    • unknown is still retryable. The cap now bounds it.

Counts at 14537cf, run with a scrubbed env and the bundled interpreter:

  • redelivery: 29 OK
  • notifier-turn-failure: 6 OK
  • hook-registration: 56 OK
  • notifier: 17 OK
  • inflight: 21 OK (5 skipped)
  • notifier-submit: 17 OK
  • turn-ledger-stop-gate, stop-hook-emits-valid-json, stop-hook-reason, worker-delivery-matches-the-hook, detached-consumer-stop-role and claude-hooks-autofix: all pass
  • check-pending-tasks-watcher-coverage, -worker-pool and -in-progress-and-repeat, watcher-rearm-session-hint, install-claude-hooks and sutando-config-hooks: all pass
  • check-pending-tasks-workspace.test.sh: 32 ok, 1 FAIL ("non-Git bundle + unmarked session -> guest"). It fails the same way on origin/main on this host, so it is not from this change.
  • git diff origin/main...HEAD | bash scripts/review-checks.sh: PASS.
  • gen-src-map --check: clean after regenerating.

Not yet verified live. There has been no post-restart round trip with a real StopFailure (a 502) on a running core. Remaining limits:

  • A Stop that the Stop hook blocks ends the turn's attribution, so a 502 later in that continued turn is not re-delivered. This is conservative.
  • A mid-turn 502 inside the prompt's own turn, after side effects have run, is still re-typed. The cap now bounds this.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Gate

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

Diff Coverage

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

  • src/delivery/turn_failure.py (100%)
  • src/util_paths.py (100%)

Summary

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

@bassilkhilo-ag2 bassilkhilo-ag2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 14537cf. All three blockers are fixed. I verified each by hand with all SUTANDO_* vars unset, so I'm dismissing my earlier CHANGES_REQUESTED.

  1. Guest gate: the earlier repro (guest-review-session StopFailure, server_error) now exits 0 and writes no state/core-turn-failure. Guest turn-start and Stop write nothing either. sutando_session_identified is the same predicate check-pending-tasks.sh used before.
  2. Attribution: a 502 in a later turn records task: null, and retry-due exits 1. The later turn here was a Monitor-event prompt that failed after the task prompt's own turn had ended with Stop. A prompt that quotes "Sutando task ready: " mid-text also records null. A failure in the prompt's own turn does record the task, and it becomes due after a successful Stop.
  3. Cap: with the clock stepped forward, attempts 1 to 3 are due (30/60/120s). Attempt 4 returns give_up once (exit 3) and then stays quiet, and the loop falls through to COMPLETION_TIMEOUT.

Suites passing locally:

  • redelivery 29, notifier-turn-failure 6, hook-registration 56
  • notifier, inflight, submit, stop-hook-*, turn-ledger-stop-gate
  • check-pending-tasks-* (all 4), watcher-rearm-session-hint, install-claude-hooks
  • review-checks.sh passes.

Two gates remain before approval:

  • "diff coverage >= 95% (python)" is red: src/delivery/turn_failure.py is at 63.8%. The missing lines are the hook and CLI entry points (_hook_payload, _hook_stop_failure, _main) plus record_turn_start and end_turn, which only the subprocess tests reach. In-process tests for those paths would cover them.
  • This changes the notifier's delivery wait loop, so REVIEW.md #15 needs a real post-restart round trip: either a core turn that ends in StopFailure, with the task re-delivered and answered, or a forced give-up at the cap.

Non-blocking: the retry record still leaks for a task archived without a result, and unknown is still retryable, though now bounded by the cap.

@bassilkhilo-ag2
bassilkhilo-ag2 dismissed their stale review October 8, 2026 23:17

All three blockers fixed at 14537cf (verified by hand); remaining gates are coverage and a live round trip, see the re-review comment.

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes at 8f357dc35d045ffa5fec14f1e43d87cdfce9319a, for one reason only: this changes the core's delivery loop, and it has no live post-restart round trip yet (REVIEW.md #15; CONTRIBUTING "Live path"). The code itself checks out.

What I ran (scrubbed env, this head):

  • turn-failure-redelivery 46 OK, claude-task-notifier-turn-failure 6 OK, health-check-claude-hook-registration 56 OK, claude-task-notifier 17 OK, claude-task-notifier-inflight 21 OK, check-pending-tasks-watcher-coverage PASS, watcher-rearm-session-hint PASS.
  • check-pending-tasks-workspace fails one case ("non-Git bundle + unmarked session -> guest"). The same case fails at the merge-base 2cee7187 on this host, so it is not this PR.
  • Mutations, each restored afterwards:
    • Disabling the release_lost_submit call in submit_task_grown fails test_a_prompt_lost_to_a_502_is_released_and_typed_again and the max-attempts test.
    • Making sutando_session_identified always true fails the three guest-session tests.
    • Dropping the failure.get("task") != filename check in _lost_to fails 4 attribution and cap tests.
    • So the tests pin the guest gate, turn attribution and the cap: bassilkhilo's three earlier blockers.
  • No source-grep tests. Coverage gate is 100% on the diff. CI and CLA are green, with no unresolved threads.

Blocker:

  1. Live witness missing. The body's own "Not done" says no core was restarted and no real StopFailure was observed. The payload shape comes from the docs, and the notifier wiring has only been exercised against fake tmux. Please run this head on a restarted core and post the transcript of either:

    • a turn that really ends in StopFailure (for example, point the core at a stub endpoint that returns 502 for one request), followed by the re-delivering <task> log line and the task's result landing; or
    • a forced give-up at the cap.

    Include the captured state/core-turn-failure/core.json, so the real payload's error and session_id fields are on record.

Non-blocking:

  • The retry record in state/task-notifier-retry/ still leaks for a task that is archived without a result. bassilkhilo noted this too.
  • Unverified by me: whether a Claude Code build that predates the StopFailure event tolerates the new key in settings.json. It is worth one line in the body.

Not ready to merge until the live round trip is posted. Everything else I checked is ready.

— Sutando (susans-MacBook-Pro)

@liususan091219
liususan091219 dismissed their stale review October 9, 2026 04:11

Withdrawn: the remaining ask was a live round-trip test, which the owner no longer treats as blocking (2026-10-09). A live test is still owed and noted, not a merge gate. — Sutando (susans-MacBook-Pro)

sonichi and others added 9 commits October 16, 2026 10:52
…urn lost

When an API error (502, overload) ends a core turn, Claude Code fires
StopFailure instead of Stop and the prompt the standby notifier typed is
gone, but its in-flight marker holds the task until the 3600s completion
timeout.

- src/stop-failure.sh (StopFailure hook) records a transient failure via
  src/delivery/turn_failure.py; auth/billing/request errors are ignored.
- check-pending-tasks.sh (Stop) stamps the next successful turn as recovery.
- task-notifier.sh asks `turn_failure.py retry-due` while waiting on a
  result: a failure newer than the submit releases the in-flight marker
  after a 30/60/120/300/600s backoff (attempts persisted per task), or at
  once when a later turn succeeded; never for a task with a result, gone
  from its inbox, worker-held, or with no marker.
- install-claude-hooks.sh registers the StopFailure hook; the health probe
  reads it from HOOKS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ol worker

A guest Claude session in the checkout (a PR review, a one-shot skill) shares the
workspace, so its 502 was recorded as the core's and re-typed the core's in-flight
task. The identity predicate the Stop hook already used moves to
src/session_identity.sh; check-pending-tasks.sh, watcher-rearm-session-hint.sh and
stop-failure.sh all source it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ompt started it

Any failure after the submit used to count, so a 502 in a later, unrelated turn
(another prompt, a Monitor event) re-typed a task that was still running.
UserPromptSubmit (turn-start.sh) now records which task the turn's prompt
delivers; StopFailure stores that task with the failure; Stop ends the turn; and
decide_retry re-delivers only when the failed turn's task is this task.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A long rate_limit window or a recurring unknown error re-typed the same task
every 10 minutes indefinitely, since each re-submit restarts the completion
timer. MAX_ATTEMPTS (3) per task now caps it: retry-due reports the give-up once
(exit 3), the notifier logs it, and the task keeps the ordinary completion
timeout. The give-up is stamped in the task's retry record beside its attempts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he submit

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d capped

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hook and CLI entry points were reached only through subprocesses, which
coverage does not measure, so diff coverage of turn_failure.py read 63.8%.
Call _main and the hook handlers directly with redirected stdio: every exit
code (0/1/2/3), usage errors, bad payloads, the session guard, the
unreadable-pool guard and the failed-write cleanup. Now 100%.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonichi
sonichi force-pushed the fix/notifier-resend-after-api-error branch from 8f357dc to 9167925 Compare October 16, 2026 18:09
@sonichi

sonichi commented Oct 16, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto origin/main at 76826102; new head 9167925a (was 8f357dc3, which sat on 2cee7187, 113 commits behind).

Conflicts, all in CHANGELOG-PENDING.md ## Fixed, none in code:

The other 7 commits applied cleanly. src/install-claude-hooks.sh, src/check-pending-tasks.sh, the notifier, turn_failure.py, util_paths.py and the hook-registration fixture needed no manual resolution. python3 scripts/gen-src-map.py --check: up to date. bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD): review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean).

Suites were run with env -i HOME PATH TMPDIR LANG USER, python3 <file> / bash <file>, one at a time:

Suite main 7682610 head 9167925
tests/claude-task-notifier-gate.test.py Ran 20, OK Ran 20, OK
tests/claude-task-notifier-inflight.test.py Ran 21, OK Ran 21, OK
tests/claude-task-notifier-longprompt.test.py Ran 19, OK Ran 19, OK
tests/claude-task-notifier-submit.test.py Ran 17, OK Ran 17, OK
tests/claude-task-notifier.test.py Ran 17, OK Ran 17, OK
tests/claude-task-notifier-turn-failure.test.py (new) n/a Ran 6, OK
tests/turn-failure-redelivery.test.py (new) n/a Ran 46, OK
tests/task-notifier-resolved-announcement.test.py Ran 13, OK Ran 13, OK
tests/task-notifier-queue-head-is-fifo.test.py rc=0 rc=0
tests/task-notifier-standby-watcher-tagged.test.py rc=0 rc=0
tests/claude-task-notifier-standby-log.test.sh ALL TESTS PASS ALL TESTS PASS
tests/task-notifier-supervisor-standby-gate.test.sh PASSED PASSED
tests/health-check-claude-hook-registration.test.py Ran 58, OK Ran 58, OK
tests/health-check-claude-task-notifier.test.py Ran 20, OK Ran 20, OK
tests/claude-hooks-autofix.test.py Ran 19, OK Ran 19, OK
tests/install-claude-hooks.test.sh PASS (52 checks) PASS (52 checks)
tests/install-claude-hooks-transcript-optout.test.sh rc=0 rc=0
tests/check-pending-tasks-{in-progress-and-repeat,watcher-coverage,worker-pool,workspace}.test.sh PASS x4 PASS x4
tests/turn-ledger-stop-gate.test.py rc=0 rc=0
tests/start-cli-claude-task-notifier.test.py Ran 7, OK (skipped=6) Ran 7, OK (skipped=6)
tests/watcher-rearm-session-hint.test.sh PASS PASS

check-pending-tasks-workspace now passes on main and at head on this host. The earlier failure seen on main no longer reproduces.

Still not done, unchanged from the body: no live post-restart round trip with a real StopFailure.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment at head 9167925a (merge-base 76826102). I found no code defect. It is not merge-ready for one reason: the live witness that liususan091219 asked for is still missing. Her CHANGES_REQUESTED was dismissed, but the last author comment says "Still not done … no live post-restart round trip with a real StopFailure". This changes the core's delivery wait loop, so REVIEW.md #15 applies.

What I verified myself (scrubbed env env -i HOME PATH TMPDIR LANG USER, this head):

  • turn-failure-redelivery 46 OK, claude-task-notifier-turn-failure 6 OK, health-check-claude-hook-registration 58 OK, claude-task-notifier-inflight 21 OK, claude-task-notifier 17 OK, check-pending-tasks-watcher-coverage PASS, watcher-rearm-session-hint PASS.

  • check-pending-tasks-workspace fails one case on this host: "non-Git bundle + unmarked session -> guest". It fails the same way on main 76826102, twice each, so it is not this PR.

  • Mutations of src/delivery/turn_failure.py, each run against turn-failure-redelivery.test.py and then restored. These differ from the ones liususan091219 ran.

    • Dropping the failed_at <= submitted_at check: 1 failure.
    • Recording and retrying non-retryable errors: 10 failures.
    • Ignoring the session mismatch in _hook_stop_failure: 2 failures.
    • Ignoring recovered_at: 1 failure, 1 error.

    So the submit-age, error-class, session and recovery rules are each pinned by a test.

  • I read the whole activated path: release_lost_submit, submit_task_grown, stop-failure.sh, turn-start.sh (stdin is now kept for the second reader by turn_ledger … </dev/null) and check-pending-tasks.sh. turn_failure_path keys on instance_scope_key, the same key composer_block_path uses, so the core and a worker do not share a record.

  • review-checks.sh --diff on the PR diff: PASS. A control diff with /Users/bob/x is flagged, so the scanner does fire.

Nits (non-blocking):

  • core-turn-failure/<key>.json holds one record per instance. Say task A's turn fails, and then an unrelated turn fails before A is re-delivered (a Monitor event, for example). The second failure overwrites A's record with task: null, and A falls back to the completion timeout. That is the safe direction, but one sentence in the module docstring would help.
  • The retry-record leak for a task archived without a result, and unknown being retryable, were already raised. I have nothing to add there.

Not verified: a real StopFailure payload, and how a Claude Code build that predates StopFailure treats the new hook key. This host runs Claude Code 2.1.301, and I did not test either.

What would change this to approve: the live witness described in liususan091219's review. That means a restarted core whose turn really ends in StopFailure, followed by re-delivering <task> and the result landing (or a forced give-up at the cap), with the captured state/core-turn-failure/core.json.

— sutando-qingyun-air

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting at 9167925a2. I'm not requesting changes, and I'm not approving yet because nobody has approved this PR. bassilkhilo-ag2's review and qingyun-wu's review at this head are both COMMENTs. My CHANGES_REQUESTED at 8f357dc35 was withdrawn on 2026-10-09. Its only ask was the live round trip, which my owner's policy names but does not block on.

What I ran at this head (env -i HOME PATH TMPDIR LANG USER)

  • tests/turn-failure-redelivery.test.py: OK.
  • tests/claude-task-notifier-turn-failure.test.py: OK.
  • tests/health-check-claude-hook-registration.test.py: OK.
  • Mutation: with the failure.get("task") != filename check removed from src/delivery/turn_failure.py:132, the redelivery suite has 4 failures. Restored afterwards. qingyun-wu's mutations at this head cover the submit-age, error-class, session and recovery rules.
  • The rebase onto 76826102 only resolved conflicts in CHANGELOG-PENDING.md. A dry merge with current main is clean, though the branch is 103 commits behind.

Owed, not blocking: a live post-restart round trip. That is a core turn that really ends in StopFailure, then re-delivering <task>, then the result landing (or a forced give-up at the cap), with the captured state/core-turn-failure/core.json on record.

What would make this approvable: one expert approval at this head from someone who owns the delivery/notifier area.

Non-blocking, already raised by others: the task-notifier-retry/ record leaks for a task archived without a result, and unknown errors are retried (bounded by the cap).

CI: 34 SUCCESS, 1 SKIPPED. license/cla: 1 context, success. No unresolved threads.

— Sutando (susans-MacBook-Pro)

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment at 9167925a2: this corrects my 2026-10-23 comment, which said a dry merge with main was clean. It conflicts with main now.

  • git merge-tree --write-tree against main a3efa9b6 (the branch is 226 commits behind) conflicts in CHANGELOG-PENDING.md and docs/src-map.md only. src/agent/claude/cli/task-notifier.sh, src/check-pending-tasks.sh and tests/health-check-claude-hook-registration.test.py auto-merge. GitHub reports CONFLICTING.
  • docs/src-map.md is generated: after resolving, re-run python3 scripts/gen-src-map.py rather than hand-merging it.

Unchanged at this head

  • No change-request condition fires. My test and mutation results from 2026-10-23 still describe this head; nothing on the branch has moved.
  • No approval yet: bassilkhilo-ag2's and qingyun-wu's reviews are COMMENTs.
  • CI: 34 SUCCESS, 1 SKIPPED. license/cla: 1 context, success. No unresolved threads.

What is missing for approval

  1. Resolve the two conflicts above and re-run the turn-failure suites at the new head.
  2. One expert approval at that head from someone who owns the delivery/notifier area.

Owed, not blocking: the live post-restart round trip (a real StopFailure turn, then re-delivering <task>, then the result landing).

— Sutando (susans-MacBook-Pro)

Conflicts: CHANGELOG-PENDING.md (main's released file plus this PR's own #5168 line) and docs/src-map.md (regenerated with scripts/gen-src-map.py).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonichi

sonichi commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Merged main into the branch (82496d3, a merge commit, no force-push):

Re-ran the turn-failure suites at the new head, with the Sutando env vars unset: claude-task-notifier-turn-failure OK, turn-failure-redelivery OK, health-check-claude-hook-registration OK, check-pending-tasks-watcher-coverage PASS, watcher-rearm-session-hint PASS. check-pending-tasks-workspace.test.sh fails one case ("non-Git bundle + unmarked session -> guest") on this host, and fails identically at 9167925 before the merge, so it comes from the host's environment, not the merge. CI will run it clean.

Still needed per @liususan091219's review: one approval at this head from someone who owns the delivery/notifier area.

(agent-assisted, Blue for Chi's seat)

@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 82496d3e8d005454b5b7e7d166ce3cc8effbea1c: update the existing notifier regression harness for the new dependencies and restore green CI.

Reviewed the full 17-file merge-base diff and the activated hook/notifier paths. The concrete regression is described inline at src/agent/claude/cli/task-notifier.sh:196 (also :859). With a scrubbed environment, tests/task-notifier-abandoned-inflight.test.py passes all 5 tests at base a3efa9b6f, but fails 4 Claude subtests at this head with unbound TURN_FAILURE_PY / RETRY_CHECK_SEC. Current CI independently reports 3 failures in that same suite. This is a harness integration defect, not evidence that normal notifier startup omits these variables.

Validation: turn-failure-redelivery passes 46 tests; health-check-claude-hook-registration passes 58. The six-case claude-task-notifier-turn-failure run completed with four passes and two 15-second subprocess timeouts; the cause of those local timeouts is unverified and is not an additional finding. The diff passes review-checks.sh, and a deliberately introduced host-path literal is rejected by the scanner.

Newest run per check name: 41 successful, 2 failed, 2 skipped; CLA succeeds. The failed Python shard contains the regression above, the aggregate check fails because of that shard, and diff coverage is skipped. Failing shard.

Worst-case user disruption is repeated side effects when a task's own turn fails after doing work. Task/turn attribution, result and ownership guards, and the three-redelivery cap limit exposure, but cannot undo prior side effects. The live post-restart round trip required by REVIEW.md criterion 15 is still explicitly unverified in the PR evidence. Before approval, provide a real StopFailure-to-redelivery-to-result witness (or a forced cap/give-up witness), as well as green regression CI. No live service was restarted for this review.

local filename="$1"
"$NOTIFIER_PY" "$DISPATCH_PY" has-result "$RESULTS_DIR" "$filename" || return 1
"$NOTIFIER_PY" "$DISPATCH_PY" inflight-clear "$INFLIGHT_DIR" "$filename" || true
"$NOTIFIER_PY" "$TURN_FAILURE_PY" retry-clear --state "$WORKSPACE_DIR/state" "$filename" 2>/dev/null || true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Update the extracted-function harness for the new retry dependencies

This new reference, together with RETRY_CHECK_SEC at line 859 and release_lost_submit, breaks tests/task-notifier-abandoned-inflight.test.py: its harness extracts the production functions but does not execute the top-level initializers or include the new helper. Under set -u, the existing completion/abandonment cases now exit on TURN_FAILURE_PY: unbound variable or RETRY_CHECK_SEC: unbound variable. The suite passes 5/5 on base a3efa9b6f; it fails at this head locally and in the newest Python CI shard. Update the harness to supply the retry module, interval and required environment/helper dependencies while keeping the abandonment assertions active, then rerun the suite and CI.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants