Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/query/toolFailureLoopGuard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ test('user aborts, user rejections, and streaming fallback discards are ignored'
getMissingToolResultAbortMessage('hard-max-query-timeout'),
getMissingToolResultAbortMessage('background'),
getMissingToolResultAbortMessage('side-task-cancelled'),
getMissingToolResultAbortMessage('agent-summary-superseded'),
getMissingToolResultAbortMessage('memory-extraction-superseded'),
getMissingToolResultAbortMessage('parent-ended'),
getMissingToolResultAbortMessage('unknown-abort'),
'Streaming fallback - tool execution discarded',
Expand Down Expand Up @@ -227,6 +229,23 @@ test('reason-aware synthetic aborts are ignored through wrappers and memory hint
expect(decision.tripped).toBe(false)
})

test('expected side-task cancellation messages do not trip repeated failure guard', () => {
const state = createToolFailureLoopGuardState()
const message = getMissingToolResultAbortMessage(
'memory-extraction-superseded',
)

update(state, [toolUse('a', 'Read')], [toolResult('a', message)], 2)
const decision = update(
state,
[toolUse('b', 'Read')],
[toolResult('b', message)],
2,
)

expect(decision.tripped).toBe(false)
})

test('tool timeout messages still count as real tool failures', () => {
const state = createToolFailureLoopGuardState()
const timeoutMessage = getMissingToolResultAbortMessage('tool-timeout')
Expand Down
2 changes: 2 additions & 0 deletions src/query/toolFailureLoopGuard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const SYNTHETIC_ABORT_TOOL_RESULT_PREFIXES = [
getMissingToolResultAbortMessage('hard-max-query-timeout'),
getMissingToolResultAbortMessage('background'),
getMissingToolResultAbortMessage('side-task-cancelled'),
getMissingToolResultAbortMessage('agent-summary-superseded'),
getMissingToolResultAbortMessage('memory-extraction-superseded'),
getMissingToolResultAbortMessage('parent-ended'),
getMissingToolResultAbortMessage('unknown-abort'),
].map(message => message.toLowerCase())
Expand Down
98 changes: 98 additions & 0 deletions src/services/AgentSummary/agentSummary.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { afterEach, describe, expect, mock, test, vi } from 'bun:test'
import * as runAgentNs from '../../tools/AgentTool/runAgent.js'
import * as forkedAgentNs from '../../utils/forkedAgent.js'
import * as sessionStorageNs from '../../utils/sessionStorage.js'
import * as localAgentTaskNs from '../../tasks/LocalAgentTask/LocalAgentTask.js'
import * as debugNs from '../../utils/debug.js'

const realRunAgent = { ...runAgentNs }
const realForkedAgent = { ...forkedAgentNs }
const realSessionStorage = { ...sessionStorageNs }
const realLocalAgentTask = { ...localAgentTaskNs }
const realDebug = { ...debugNs }

describe('agent summary cancellation', () => {
afterEach(() => {
vi.useRealTimers()
vi.restoreAllMocks()
mock.restore()
mock.module('../../tools/AgentTool/runAgent.js', () => realRunAgent)
mock.module('../../utils/forkedAgent.js', () => realForkedAgent)
mock.module('../../utils/sessionStorage.js', () => realSessionStorage)
mock.module(
'../../tasks/LocalAgentTask/LocalAgentTask.js',
() => realLocalAgentTask,
)
mock.module('../../utils/debug.js', () => realDebug)
})

test('stopping an in-flight summary aborts with an expected side-task reason', async () => {
vi.useFakeTimers()

let summaryAbortController: AbortController | undefined
const debugLog = mock(
(_message: string, _options?: { level?: string }) => {},
)
const runForkedAgent = mock(
({ overrides }: { overrides?: { abortController?: AbortController } }) => {
summaryAbortController = overrides?.abortController
return new Promise(() => {})
},
)

mock.module('../../tools/AgentTool/runAgent.js', () => ({
filterIncompleteToolCalls: <T>(messages: T) => messages,
runAgent: async function* () {},
}))
mock.module('../../utils/forkedAgent.js', () => ({
...realForkedAgent,
runForkedAgent,
}))
mock.module('../../utils/sessionStorage.js', () => ({
...realSessionStorage,
getAgentTranscript: mock(async () => ({
messages: [{ type: 'user' }, { type: 'assistant' }, { type: 'user' }],
})),
}))
mock.module('../../tasks/LocalAgentTask/LocalAgentTask.js', () => ({
updateAgentSummary: mock(() => {}),
}))
mock.module('../../utils/debug.js', () => ({
...realDebug,
logForDebugging: debugLog,
}))

const { startAgentSummarization } = await import(
`./agentSummary.js?ts=${Date.now()}-${Math.random()}`
)
const { stop } = startAgentSummarization(
'task-1',
'agent-1' as never,
{} as never,
mock(() => {}) as never,
)

vi.advanceTimersByTime(30_000)
await Promise.resolve()
await Promise.resolve()

expect(runForkedAgent).toHaveBeenCalledTimes(1)
expect(summaryAbortController).toBeDefined()

stop()

expect(summaryAbortController!.signal.aborted).toBe(true)
expect(summaryAbortController!.signal.reason).toBe(
'agent-summary-superseded',
)
expect(
debugLog.mock.calls.some(([message, options]) => {
return (
String(message).includes('[AgentSummary] Stopping summarization') &&
String(message).includes('agent-summary-superseded') &&
(options as { level?: string } | undefined)?.level !== 'error'
)
}),
).toBe(true)
})
})
7 changes: 5 additions & 2 deletions src/services/AgentSummary/agentSummary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { createUserMessage } from '../../utils/messages.js'
import { getAgentTranscript } from '../../utils/sessionStorage.js'

const SUMMARY_INTERVAL_MS = 30_000
const SUMMARY_SUPERSEDED_ABORT_REASON = 'agent-summary-superseded'

function buildSummaryPrompt(previousSummary: string | null): string {
const prevLine = previousSummary
Expand Down Expand Up @@ -160,14 +161,16 @@ export function startAgentSummarization(
}

function stop(): void {
logForDebugging(`[AgentSummary] Stopping summarization for ${taskId}`)
logForDebugging(
`[AgentSummary] Stopping summarization for ${taskId}: ${SUMMARY_SUPERSEDED_ABORT_REASON}`,
)
stopped = true
if (timeoutId) {
clearTimeout(timeoutId)
timeoutId = null
}
if (summaryAbortController) {
summaryAbortController.abort()
summaryAbortController.abort(SUMMARY_SUPERSEDED_ABORT_REASON)
summaryAbortController = null
}
}
Expand Down
62 changes: 61 additions & 1 deletion src/services/api/claude.abortClassification.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { describe, expect, test } from 'bun:test'
import { APIUserAbortError } from '@anthropic-ai/sdk'
import { shouldCreateUserInterruptionMessage } from '../../utils/abortReasons.js'
import { getClaudeStreamingAbortLogMessage } from './claude.js'
import {
getClaudeExpectedSideTaskApiAbortLogMessage,
getClaudeStreamingAbortLogMessage,
} from './claude.js'
import { CannotRetryError } from './withRetry.js'

describe('Claude stream abort classification wiring', () => {
test('formats timeout abort logs as timeout aborts, not user aborts', () => {
Expand Down Expand Up @@ -39,6 +44,28 @@ describe('Claude stream abort classification wiring', () => {
expect(shouldCreateUserInterruptionMessage('streaming_fallback')).toBe(
false,
)
expect(
getClaudeStreamingAbortLogMessage(
{ reason: 'agent-summary-superseded' },
new Error('Request was aborted.'),
),
).toBe(
'Streaming aborted because agent summary was superseded: Request was aborted.',
)
expect(
getClaudeStreamingAbortLogMessage(
{ reason: 'memory-extraction-superseded' },
new Error('Request was aborted.'),
),
).toBe(
'Streaming aborted because memory extraction was superseded: Request was aborted.',
)
expect(shouldCreateUserInterruptionMessage('agent-summary-superseded')).toBe(
false,
)
expect(
shouldCreateUserInterruptionMessage('memory-extraction-superseded'),
).toBe(false)
})

test('keeps actual user aborts user-facing for stream logs and advisor gating', () => {
Expand All @@ -55,4 +82,37 @@ describe('Claude stream abort classification wiring', () => {
true,
)
})

test('labels expected side-task retry aborts without reclassifying user aborts', () => {
const expectedAbort = new AbortController()
expectedAbort.abort('agent-summary-superseded')
const retryAbort = new CannotRetryError(new APIUserAbortError(), {
model: 'test-model',
thinkingConfig: { type: 'disabled' },
})

expect(
getClaudeExpectedSideTaskApiAbortLogMessage(
expectedAbort.signal,
retryAbort,
),
).toBe(
'Expected side-task API abort (agent-summary-superseded): Request was aborted.',
)

const userAbort = new AbortController()
userAbort.abort()
expect(
getClaudeExpectedSideTaskApiAbortLogMessage(
userAbort.signal,
new APIUserAbortError(),
),
).toBeNull()
expect(
getClaudeExpectedSideTaskApiAbortLogMessage(
expectedAbort.signal,
new Error('boom'),
),
).toBeNull()
})
})
54 changes: 54 additions & 0 deletions src/services/api/claude.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ import {
import { getOauthAccountInfo } from '../../utils/auth.js'
import {
getStreamingAbortMessage,
isExpectedSideTaskAbortReason,
normalizeAbortReason,
shouldCreateUserInterruptionMessage,
} from '../../utils/abortReasons.js'
import {
Expand Down Expand Up @@ -812,6 +814,38 @@ export function getClaudeStreamingAbortLogMessage(
return getStreamingAbortMessage(signal.reason, errorMessage(streamingError))
}

export function getClaudeExpectedSideTaskApiAbortLogMessage(
signal: Pick<AbortSignal, 'aborted' | 'reason'>,
errorFromRetry: unknown,
): string | null {
if (!signal.aborted || !isExpectedSideTaskAbortReason(signal.reason)) {
return null
}
const error =
errorFromRetry instanceof CannotRetryError
? errorFromRetry.originalError
: errorFromRetry
if (!(error instanceof APIUserAbortError)) {
return null
}
return `Expected side-task API abort (${normalizeAbortReason(signal.reason)}): ${errorMessage(error)}`
}

function handleClaudeExpectedSideTaskApiAbort(
signal: Pick<AbortSignal, 'aborted' | 'reason'>,
errorFromRetry: unknown,
releaseStreamResources: () => void,
): boolean {
const expectedSideTaskAbortLogMessage =
getClaudeExpectedSideTaskApiAbortLogMessage(signal, errorFromRetry)
if (!expectedSideTaskAbortLogMessage) return false
if (!(errorFromRetry instanceof CannotRetryError)) {
logForDebugging(expectedSideTaskAbortLogMessage)
}
releaseStreamResources()
return true
}

/**
* Determines if an LSP tool should be deferred (tool appears with defer_loading: true)
* because LSP initialization is not yet complete.
Expand Down Expand Up @@ -2815,6 +2849,16 @@ async function* queryModel(
throw errorFromRetry
}

if (
handleClaudeExpectedSideTaskApiAbort(
signal,
errorFromRetry,
releaseStreamResources,
)
) {
return
}

if (signal.aborted) {
releaseStreamResources()
return
Expand Down Expand Up @@ -2917,6 +2961,16 @@ async function* queryModel(
throw fallbackError
}

if (
handleClaudeExpectedSideTaskApiAbort(
signal,
fallbackError,
releaseStreamResources,
)
) {
return
}

// Fallback also failed, handle as normal error
logForDebugging(
`Non-streaming fallback also failed: ${errorMessage(fallbackError)}`,
Expand Down
Loading