fix(chatd): persist interrupted tool call steps instead of losing them (#23011)

## Problem

When a chat is interrupted while tools are executing, the step content
(text, reasoning, tool calls, and partial tool results) was being lost.
Two gaps existed:

1. **During tool execution**: `executeTools` returns with error results
for interrupted tools, but the subsequent `PersistStep(ctx, ...)` fails
on the canceled context and returns `ErrInterrupted` without persisting
anything.

2. **PersistStep race**: If the context is canceled between the
post-tool interrupt check and the `PersistStep` call, the same loss
occurs.

This is inconsistent with how we handle stream interruptions (which
properly flush and persist partial content via `persistInterruptedStep`)
and how [coder/blink](https://github.com/coder/blink) handles
interruptions (always inserting the response message regardless of
execution phase).

## Fix

Two changes in `chatloop.go`:

- **Post-tool-execution interrupt check**: After `executeTools` returns,
check if the context was interrupted and route through
`persistInterruptedStep` (which uses `context.WithoutCancel` internally)
to save the accumulated content.

- **PersistStep fallback**: If `PersistStep` returns `ErrInterrupted`,
retry via `persistInterruptedStep` so partial content is not lost.

## Tests

- `TestRun_InterruptedDuringToolExecutionPersistsStep`: Verifies that
when a tool is blocked and the chat is interrupted, the step (text +
reasoning + tool call + tool error result) is persisted via the
interrupt-safe path.

- `TestRun_PersistStepInterruptedFallback`: Verifies that when
`PersistStep` itself returns `ErrInterrupted`, the step is retried via
the fallback path and content is saved.
This commit is contained in:
Kyle Carberry
2026-03-12 16:59:16 -04:00
committed by GitHub
parent 072e9a212f
commit 42c12176a0
2 changed files with 197 additions and 3 deletions
+21 -3
View File
@@ -324,8 +324,20 @@ func Run(ctx context.Context, opts RunOptions) error {
for _, tr := range toolResults {
result.content = append(result.content, tr)
}
}
// Check for interruption after tool execution.
// Tools that were canceled mid-flight produce error
// results via ctx cancellation. Persist the full
// step (assistant blocks + tool results) through
// the interrupt-safe path so nothing is lost.
if ctx.Err() != nil {
if errors.Is(context.Cause(ctx), ErrInterrupted) {
persistInterruptedStep(ctx, opts, &result)
return ErrInterrupted
}
return ctx.Err()
}
}
// Extract context limit from provider metadata.
contextLimit := extractContextLimit(result.providerMetadata)
if !contextLimit.Valid && opts.ContextLimitFallback > 0 {
@@ -334,15 +346,21 @@ func Run(ctx context.Context, opts RunOptions) error {
Valid: true,
}
}
// Persist the step — errors propagate directly.
// Persist the step. If persistence fails because
// the chat was interrupted between the previous
// check and here, fall back to the interrupt-safe
// path so partial content is not lost.
if err := opts.PersistStep(ctx, PersistedStep{
Content: result.content,
Usage: result.usage,
ContextLimit: contextLimit,
}); err != nil {
if errors.Is(err, ErrInterrupted) {
persistInterruptedStep(ctx, opts, &result)
return ErrInterrupted
}
return xerrors.Errorf("persist step: %w", err)
}
lastUsage = result.usage
lastProviderMetadata = result.providerMetadata