fix(claude-sdk): terminate SDK loop on end_turn (MCP-prefix match + PostToolBatch hook)

[claudiusthebot]

Summary

Two-part fix for the typing-indicator-lingers-after-end_turn bug Dylan flagged in chat. The first half (commits ff08fe8 + 031a463 + d5ce30f) makes the existing turn-terminator detection actually match MCP-prefixed tool names. The second half (commits e0262e0 + 097b8b4) replaces the dropped qi.interrupt() approach with an SDK-sanctioned PostToolBatch hook that terminates the query loop cleanly without the abort race.

Background

PR #108 added a qi.interrupt() call after observing a turn-terminator tool, intending to abort the SDK loop instead of letting it do a wasted follow-up API call to "wrap up" after end_turn. The check sits in isTurnTerminator(toolName) against a set built from tool registry names — so the set contains "end_turn".

But tools served through MCP arrive at the SDK with a server prefix: mcp__telegram-tools__end_turn. The check used strict equality, so isTurnTerminator("mcp__telegram-tools__end_turn") returned false. The entire interrupt path was dead code in production. state.turnTerminated stayed false, qi.interrupt() never fired, and the SDK ran its full natural cycle including the unnecessary second API call.

captureDeliveredText had the same bug: it compared toolName === "end_turn" and toolName === "send" literally, so cross-tool dedup also never matched on the MCP-routed names.

Diagnosis

Diagnostic logging on a temp branch (now removed) confirmed:

[DIAG] terminator tool mcp__telegram-tools__end_turn observed at t=174109
[DIAG] typing cleared at t=177232  (+3123ms post-end_turn)
SDK result: numApiCalls=2

numApiCalls=2 — the second call is the wasted wrap-up after end_turn returned a tool result. That's exactly the 3s gap.

Fix part 1 — MCP prefix detection (ff08fe8, 031a463)

• New stripMcpPrefix() helper in src/core/tools/index.ts — strips mcp__<server>__ (non-greedy .+? match, first __ after mcp__ is the server-name boundary). No-op if no prefix matches.
• isTurnTerminator now checks both the raw name and the stripped form.
• captureDeliveredText in src/backend/claude-sdk/handler.ts normalizes via stripMcpPrefix before its equality checks.

Fix part 2 — Drop qi.interrupt() (d5ce30f)

After landing the prefix fix, qi.interrupt() started actually firing — and surfaced a different, worse bug. The SDK frequently emits multiple tool_use blocks in the same assistant message, fired as a parallel batch. end_turn itself is one MCP tool; siblings (e.g. mempalace_add_drawer, a react, etc.) might still be in-flight when end_turn resolves. interrupt() cancels their AbortController mid-flight, surfacing as MCP error -32001: AbortError in the SDK result and bubbling up to the user as "Something went wrong."

Removed the interrupt; accepted the ~2-3s typing lag as the temporary cost of not breaking turns. state.turnTerminated is still tracked so the flow-violation re-prompt path can skip its retry when the model explicitly ended its turn.

Fix part 3 — PostToolBatch hook (e0262e0, 097b8b4)

Restores the loop-termination behavior cleanly using the SDK's documented hook system instead of the racy interrupt.

The SDK exposes SyncHookJSONOutput.continue: false as the canonical way to terminate the query loop, and TerminalReason: 'hook_stopped' is a documented exit condition. The hook event we want is PostToolBatch — per the SDK type docs:

PostToolUse fires per-tool and may run concurrently for parallel tool calls; PostToolBatch fires exactly once with the full batch.

That's the magic property. The race that killed qi.interrupt() was sibling MCP tools getting their AbortControllers cancelled mid-flight. PostToolBatch fires AFTER every tool in the batch has resolved — by definition no in-flight siblings to race with.

Implementation in src/backend/claude-sdk/options.ts:

const turnTerminatorHook: HookCallback = async (input) => {
  if (input.hook_event_name !== "PostToolBatch") {
    return { continue: true };
  }
  const batch = input as PostToolBatchHookInput;
  const terminator = batch.tool_calls.find((tc) =>
    isTurnTerminator(tc.tool_name),
  );
  if (terminator) {
    log("agent", `PostToolBatch: terminating SDK loop on ${terminator.tool_name} (batch size: ${batch.tool_calls.length})`);
    return { continue: false, stopReason: "turn terminated by end_turn / send" };
  }
  return { continue: true };
};

Registered on the options:

hooks: {
  PostToolBatch: [{ hooks: [turnTerminatorHook] }],
},

The same isTurnTerminator helper from part 1 — it already strips the MCP prefix, so the hook works transparently for both bare and prefixed tool names.

Live verification

After the fix landed in /home/dylan/telegram-claude-agent/ and Talon was restarted:

{"level":30,"time":1778322236,"component":"agent","msg":"PostToolBatch: terminating SDK loop on mcp__telegram-tools__end_turn (batch size: 1)"}

Hook fires, SDK exits with TerminalReason: 'hook_stopped', no follow-up API call, no wasted tokens, no abort race. Confirmed working in production.

Tests

5 prefix-detection cases on top of PR #108's existing tests, plus 6 new cases covering the hook:

• isTurnTerminator passes on mcp__telegram-tools__end_turn, mcp__teams-tools__end_turn, mcp__some-server-name__end_turn
• isTurnTerminator still rejects mcp__telegram-tools__send, mcp__telegram-tools__react
• stripMcpPrefix strips correctly on real tool name shapes
• stripMcpPrefix is a no-op when no prefix matches (end_turn, Read, mcp__incomplete, not_mcp__server__tool)
• Hook is registered on options.hooks.PostToolBatch
• Hook returns continue: false for MCP-prefixed end_turn in the batch
• Hook returns continue: false for bare end_turn
• Hook returns continue: true when no terminator is in the batch
• Hook returns continue: true on empty batch
• Hook ignores non-PostToolBatch events defensively

vitest run → 1690/1691 pass. The 1 failure is package.functional.test.ts host-isolation gap (talon status test expects "Stopped" but Talon is running on the test host) — passes in CI, fails locally on the production VPS. Unrelated to this change.

Test plan

• tsc clean
• prettier --check clean on touched files
• oxlint clean on touched files
• full test suite passes (modulo unrelated pre-existing host-isolation gap)
• live smoke test on production: hook fires on every end_turn, log line confirmed, typing-indicator drops instantly

🤖 Generated with Claude Code

[claudiusthebot]

Heartbeat #169 addendum — found three improvements sitting uncommitted on this branch, committed them as 031a463:

1. telegram/handlers.ts — processAndReply onToolUse
The original PR fixed isTurnTerminator and captureDeliveredText in the SDK layer, but processAndReply in handlers.ts still did bare-name equality on the onToolUse callback. That meant:

• end_turn deliveries (arriving as mcp__telegram-tools__end_turn in production) were silently not captured in the daily log
• Only bare send was captured, which would also miss the prefixed form in production

Applied the same stripMcpPrefix pattern. Now both end_turn and send log to the daily diary regardless of how they arrive.

2. end-turn.test.ts — production wire-shape contract
Added a new describe block that tests isTurnTerminator + processAssistantMessage with actual SDK-emitted names (mcp__telegram-tools__end_turn, mcp__teams-tools__end_turn, etc.). Auto-derived from ALL_TOOLS.filter(t => t.endsTurn) × known MCP servers — adding a new endsTurn tool stays covered without manual additions. These are exactly the tests that would have caught the original bug.

3. handlers.test.ts — onToolUse matrix
Replaced the single bare-send test with 6 cases: MCP-prefixed send, MCP-prefixed end_turn, bare send, bare end_turn (defensive), non-text send (should NOT capture), react tool (should NOT capture). All six pass.

typecheck + format:check + vitest run all clean (143 tests in the two affected files). CI restarted on 031a463 — the Windows Functional flake (package.functional.test.ts) is pre-existing and unrelated to this change, same as on main.

[claudiusthebot]

Pushed d5ce30f — drops the qi.interrupt() call.

Reproduced on the live deployment: with v1 applied (prefix fix only), isTurnTerminator correctly matches MCP-prefixed names → qi.interrupt() actually fires → races with in-flight MCP tool dispatches in the same assistant message → MCP error -32001: AbortError → turns bomb.

Hits seen between 16:19–22:47 UTC: two group turns, the OpenRouter Model Monitor cron ("Job failed"), and Dylan's "What's up" DM all surfaced the AbortError.

Removed the interrupt call. state.turnTerminated is still tracked because the flow-violation re-prompt path uses it — just no more aborting in-flight MCP work. Net effect: ~2–3s of typing lag at end_turn (one wasted SDK follow-up call where the model says nothing) but no broken turns.

Prefix-stripping stays — the daily-log capture / dedup fixes are independent value.

Patch already applied to /home/dylan/telegram-claude-agent and tsc clean. Restart whenever for the live fix.