Skip to content

fix(claude-sdk): preserve SDK loop on terminator delivery failure via SDK's native error pipeline - #159

Merged
dylanneve1 merged 2 commits into
mainfrom
fix/terminator-throws-on-delivery-failure
May 19, 2026
Merged

fix(claude-sdk): preserve SDK loop on terminator delivery failure via SDK's native error pipeline#159
dylanneve1 merged 2 commits into
mainfrom
fix/terminator-throws-on-delivery-failure

Conversation

@claudiusthebot

Copy link
Copy Markdown
Collaborator

Summary

When a turn-terminator tool (end_turn, strict react) failed to deliver β€” Telegram rejected for "Message too long", invalid chat_id, network blip β€” the PostToolBatch hook terminated the SDK loop anyway. The model saw the error in its tool result but had no turn left to react. End result: silent dropped turn, user sees nothing.

Canonical incident (2026-05-13 13:11Z, Pandario reply 226264): end-of-turn delivery of a 4326-char message hit Telegram's 4096 cap, bridge returned {ok:false, error:\"Message too long...\"}, hook fired regardless, user pinged the bot 24 min later asking what happened.

Supersedes #158 (closed). That PR took a content-sniffing approach in the hook (parse tool_response looking for \"ok\":false). Dylan flagged three real problems: frontend coupling (Telegram bridge shape baked into a generic SDK hook), schema drift (if the bridge envelope changes the check goes blind), false positives (any string containing \"ok\":false would trigger the recovery). This PR uses the SDK's native error pipeline instead.

How it works

  1. end_turn.execute and react.execute THROW on {ok:false} instead of returning the failure object silently. A new throwIfFailed helper raises Error(\"<tool> delivery failed: <bridge error>\"). The "what counts as a failure" decision now lives in the tool implementation, where the contract is owned.

  2. SDK observes the throw and fires PostToolUseFailure with a typed {tool_name, tool_input, tool_use_id, error, is_interrupt} payload. No string sniffing, no unknown parsing.

  3. New PostToolUseFailure hook records the failed tool_use_id in a per-session Set<string>. Ignores interrupts (is_interrupt: true) and non-terminator failures (send, etc).

  4. PostToolBatch hook consults the Set β€” if the terminator's tool_use_id was flagged, deletes the flag and returns {continue: true} to keep the SDK loop alive. Otherwise terminates as before (PR fix(claude-sdk): terminate SDK loop on end_turn (MCP-prefix match + PostToolBatch hook)Β #122's perf win preserved on the happy path).

  5. Hooks share state via closure β€” buildTurnTerminatorHooks() creates a fresh Set per buildSdkOptions() call. Concurrent chat sessions stay isolated.

Why this is better than #158

  • Frontend-agnostic: any frontend whose tools throw on delivery failure gets the same recovery behaviour. The hook never sees bridge envelope shapes.
  • Type-safe: PostToolUseFailureHookInput has named fields (tool_name, error, is_interrupt). No unknown parsing.
  • No false positives: a tool returning \"ok\":false\" somewhere in its payload no longer accidentally triggers the recovery path. Only a thrown exception counts.
  • Schema-drift resistant: changing the bridge envelope shape doesn't break the hook. The tool's execute() decides what's a failure, and the SDK propagates that.

Test plan

  • 9 new PostToolUseFailure + PostToolBatch coordination cases:
    • Failed end_turn β†’ loop preserved
    • Failed strict react β†’ loop preserved
    • Successful end_turn β†’ terminates as usual
    • is_interrupt: true ignored (not a real failure)
    • Non-terminator failure (send) ignored
    • Soft react (end_turn: false) failure ignored
    • Defensive: non-PostToolUseFailure events return {continue: true}
    • Flag consumed on first match (no stale-flag bug)
    • Per-session isolation (chat A failure doesn't preserve chat B)
  • 8 new messaging-tools cases:
    • end_turn throws on text path / buttons path / missing error field
    • end_turn success path unchanged (returns the bridge result)
    • react strict throws on {ok:false}
    • react soft (end_turn: false) also throws on {ok:false}
    • react success path unchanged
    • react still strips the end_turn param before bridging
  • All 33 existing PostToolBatch hook tests still pass.
  • npm run typecheck clean
  • npm run lint β€” no new warnings
  • npm run test β€” 2001/2014 pass, 1 pre-existing package.functional flake (verified same flake on main when running tests on a host where Talon daemon is up).

πŸ€– Generated with Claude Code

… SDK's native error pipeline

When a turn-terminator tool (`end_turn`, strict `react`) failed to deliver
(e.g. Telegram rejected `end_turn` for "Message too long", invalid chat_id,
network blip), the PostToolBatch hook terminated the SDK loop anyway β€” the
model saw the error in its tool result but had no turn left to react. End
result: silent dropped turn, user sees nothing.

Canonical incident (2026-05-13 13:11Z Pandario reply 226264): end-of-turn
delivery of a 4326-char message hit Telegram's 4096 cap, bridge returned
`{ok: false, error: "Message too long..."}`, hook fired regardless, turn
silently ended.

Supersedes #158 (content-sniffing approach was fragile β€” frontend-coupled,
schema-drift-vulnerable, false-positive-prone on responses that happened
to contain `"ok":false` substrings).

This PR uses the SDK's NATIVE error pipeline instead of inspecting bodies.

Implementation:

1. `end_turn.execute` and `react.execute` THROW when the bridge returns
   `{ok: false}` instead of returning the failure object silently. A new
   `throwIfFailed` helper wraps the bridge result and raises a typed
   `Error("<tool> delivery failed: <bridge error>")`. The "what counts
   as a failure" decision now lives in the tool implementation, where the
   contract is owned.

2. The SDK observes the throw and fires `PostToolUseFailure` with a typed
   `{tool_name, tool_input, tool_use_id, error, is_interrupt}` payload β€”
   no string sniffing, no `unknown` parsing.

3. New `PostToolUseFailure` hook records the failed `tool_use_id` in a
   per-session `Set<string>`. Ignores interrupts (`is_interrupt: true`)
   and non-terminator failures (e.g. `send`).

4. `PostToolBatch` hook now consults the Set β€” if the terminator's
   `tool_use_id` was flagged, it deletes the flag and returns
   `{continue: true}` to keep the SDK loop alive. Otherwise terminates
   as before (perf win from PR #122 preserved on the happy path).

5. The two hooks share state via closure β€” `buildTurnTerminatorHooks()`
   creates a fresh Set per `buildSdkOptions()` call, so concurrent chat
   sessions stay isolated.

Frontend-agnostic by design: any frontend whose tools throw on delivery
failure gets the same recovery behaviour. No bridge envelope shape is
baked into the SDK options layer.

Tests:
  - 9 new `PostToolUseFailure + PostToolBatch coordination` cases
    (terminator failure preserves loop, success terminates, interrupt
    ignored, non-terminator failure ignored, soft-react `end_turn:false`
    ignored, defensive non-failure events, flag-consumed-on-match,
    per-session isolation).
  - 8 new messaging-tools cases for `end_turn` / `react` throw behaviour
    (text path throws on {ok:false}, buttons path throws, generic
    message when error field missing, success path unchanged, react
    strict + soft both throw, react strips end_turn param).
  - All 33 existing PostToolBatch hook tests still pass.
  - 2001/2014 vitest pass β€” same pre-existing `package.functional` flake
    as PR #157 (irrelevant: running tests on a host where Talon daemon is
    already live).
  - typecheck clean, prettier clean, no new lint warnings.
@dylanneve1
dylanneve1 enabled auto-merge (squash) May 19, 2026 16:39
@dylanneve1
dylanneve1 merged commit 9fefe59 into main May 19, 2026
37 checks passed
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.

2 participants