Skip to content

Commit 5331122

Browse files
committed
feat(agent-runtime): Improve abort handling with PromptResult pattern and AbortError class
- Add custom AbortError class for robust abort detection (vs brittle string matching) - Refactor processFileBlock and handleLargeFile to return PromptResult<T> instead of throwing - Update write-file.ts handler to check result.aborted and throw AbortError at boundary - Fix E2E mocks to return promptSuccess() instead of raw strings - Add comprehensive tests for abort propagation in processFileBlock and loopAgentSteps - Add tests verifying AbortError results in cancelled status, regular errors in failed status All 407 agent-runtime tests pass. Manual CLI testing confirmed: - Ctrl+C shows [response interrupted] and no file corruption - Follow-up prompts work correctly after abort - Message history preserved
1 parent 59738d8 commit 5331122

File tree

9 files changed

+608
-130
lines changed

9 files changed

+608
-130
lines changed

common/src/util/__tests__/error-abort.test.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,32 @@ describe('AbortError class', () => {
6262
})
6363

6464
describe('isAbortError edge cases', () => {
65-
describe('exact message matching', () => {
65+
describe('message matching with startsWith', () => {
6666
it('returns true for exact ABORT_ERROR_MESSAGE', () => {
6767
const error = new Error(ABORT_ERROR_MESSAGE)
6868
expect(isAbortError(error)).toBe(true)
6969
})
7070

71-
it('returns false for message containing ABORT_ERROR_MESSAGE as substring', () => {
72-
const error = new Error(`Error: ${ABORT_ERROR_MESSAGE} by system`)
71+
it('returns true for message with suffix after ABORT_ERROR_MESSAGE (like AbortError with reason)', () => {
72+
// This is the format AbortError uses: 'Request aborted: reason'
73+
const error = new Error(`${ABORT_ERROR_MESSAGE}: timeout`)
74+
expect(isAbortError(error)).toBe(true)
75+
})
76+
77+
it('returns false for message with non-colon suffix after ABORT_ERROR_MESSAGE', () => {
78+
// Only 'Request aborted' or 'Request aborted: <reason>' should match
79+
// Other patterns like 'Request aborted by user' should NOT match
80+
const error = new Error(`${ABORT_ERROR_MESSAGE} due to user action`)
7381
expect(isAbortError(error)).toBe(false)
7482
})
7583

76-
it('returns false for message with prefix before ABORT_ERROR_MESSAGE', () => {
77-
const error = new Error(`Something failed: ${ABORT_ERROR_MESSAGE}`)
84+
it('returns false for message containing ABORT_ERROR_MESSAGE as substring (not prefix)', () => {
85+
const error = new Error(`Error: ${ABORT_ERROR_MESSAGE} by system`)
7886
expect(isAbortError(error)).toBe(false)
7987
})
8088

81-
it('returns false for message with suffix after ABORT_ERROR_MESSAGE', () => {
82-
const error = new Error(`${ABORT_ERROR_MESSAGE} due to timeout`)
89+
it('returns false for message with prefix before ABORT_ERROR_MESSAGE', () => {
90+
const error = new Error(`Something failed: ${ABORT_ERROR_MESSAGE}`)
8391
expect(isAbortError(error)).toBe(false)
8492
})
8593
})

common/src/util/error.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,37 @@ export function promptAborted(reason?: string): PromptAborted {
130130
*/
131131
export const ABORT_ERROR_MESSAGE = 'Request aborted'
132132

133+
/**
134+
* Custom error class for abort errors.
135+
* Use this class instead of generic Error for abort errors to ensure
136+
* robust detection via isAbortError() (checks error.name === 'AbortError').
137+
*/
138+
export class AbortError extends Error {
139+
constructor(reason?: string) {
140+
super(reason ? `${ABORT_ERROR_MESSAGE}: ${reason}` : ABORT_ERROR_MESSAGE)
141+
this.name = 'AbortError'
142+
}
143+
}
144+
133145
/**
134146
* Check if an error is an abort error.
135147
* Use this helper to detect abort errors in catch blocks.
136148
*
137149
* Detects both:
138-
* - Errors with message 'Request aborted' (thrown by our code via ABORT_ERROR_MESSAGE)
150+
* - Errors with message starting with 'Request aborted' (thrown by our code via AbortError)
139151
* - Native AbortError (thrown by fetch/AI SDK when AbortSignal is triggered)
140152
*/
141153
export function isAbortError(error: unknown): boolean {
142154
if (!(error instanceof Error)) {
143155
return false
144156
}
145-
// Check for our custom abort error message
146-
if (error.message === ABORT_ERROR_MESSAGE) {
157+
// Check for our custom abort error message:
158+
// - Exact match: 'Request aborted'
159+
// - With reason: 'Request aborted: <reason>' (from AbortError class)
160+
if (
161+
error.message === ABORT_ERROR_MESSAGE ||
162+
error.message.startsWith(`${ABORT_ERROR_MESSAGE}: `)
163+
) {
147164
return true
148165
}
149166
// Check for native AbortError (DOMException or Error with name 'AbortError')
@@ -161,11 +178,11 @@ export function isAbortError(error: unknown): boolean {
161178
* as exceptions. Callers should use `isAbortError()` in catch blocks to detect
162179
* and handle abort errors appropriately (e.g., rethrow instead of logging as errors).
163180
*
164-
* @throws {Error} When result.aborted is true. The error message is ABORT_ERROR_MESSAGE.
181+
* @throws {AbortError} When result.aborted is true.
165182
*/
166183
export function unwrapPromptResult<T>(result: PromptResult<T>): T {
167184
if (result.aborted) {
168-
throw new Error(ABORT_ERROR_MESSAGE)
185+
throw new AbortError(result.reason)
169186
}
170187
return result.value
171188
}

packages/agent-runtime/src/__tests__/loop-agent-steps.test.ts

Lines changed: 125 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
} from '@codebuff/common/testing/mock-modules'
77
import { setupDbSpies } from '@codebuff/common/testing/mocks/database'
88
import { getInitialSessionState } from '@codebuff/common/types/session-state'
9-
import { promptSuccess } from '@codebuff/common/util/error'
9+
import { AbortError, promptSuccess } from '@codebuff/common/util/error'
1010
import { assistantMessage, userMessage } from '@codebuff/common/util/messages'
1111
import db from '@codebuff/internal/db'
1212
import {
@@ -807,4 +807,128 @@ describe('loopAgentSteps - runAgentStep vs runProgrammaticStep behavior', () =>
807807
// Should have output set
808808
expect(result.agentState.output).toEqual({ result: 'done' })
809809
})
810+
811+
describe('abort handling', () => {
812+
it('should handle AbortError and finish with cancelled status', async () => {
813+
// Test that when an AbortError is thrown (e.g., from a tool handler),
814+
// loopAgentSteps catches it, finishes with 'cancelled' status, and returns
815+
// an error output indicating the run was cancelled.
816+
817+
const llmOnlyTemplate = {
818+
...mockTemplate,
819+
handleSteps: undefined,
820+
}
821+
822+
const localAgentTemplates = {
823+
'test-agent': llmOnlyTemplate,
824+
}
825+
826+
// Track finishAgentRun calls
827+
let finishAgentRunStatus: string | undefined
828+
const mockFinishAgentRun = mock(async (params: { status: string }) => {
829+
finishAgentRunStatus = params.status
830+
})
831+
832+
// Mock promptAiSdkStream to throw an AbortError (simulating user cancellation mid-stream)
833+
loopAgentStepsBaseParams.promptAiSdkStream = async function* () {
834+
// Yield some content first
835+
yield { type: 'text' as const, text: 'Starting work...\n' }
836+
// Then throw AbortError to simulate user cancellation
837+
throw new AbortError('User pressed Ctrl+C')
838+
}
839+
840+
const result = await loopAgentSteps({
841+
...loopAgentStepsBaseParams,
842+
agentType: 'test-agent',
843+
localAgentTemplates,
844+
finishAgentRun: mockFinishAgentRun,
845+
})
846+
847+
// Verify the output indicates cancellation
848+
expect(result.output.type).toBe('error')
849+
if (result.output.type === 'error') {
850+
expect(result.output.message).toBe('Run cancelled by user')
851+
}
852+
853+
// Verify finishAgentRun was called with 'cancelled' status
854+
expect(mockFinishAgentRun).toHaveBeenCalled()
855+
expect(finishAgentRunStatus).toBe('cancelled')
856+
})
857+
858+
it('should distinguish AbortError from other errors', async () => {
859+
// Test that non-abort errors are NOT treated as cancellations
860+
861+
const llmOnlyTemplate = {
862+
...mockTemplate,
863+
handleSteps: undefined,
864+
}
865+
866+
const localAgentTemplates = {
867+
'test-agent': llmOnlyTemplate,
868+
}
869+
870+
// Track finishAgentRun calls
871+
let finishAgentRunStatus: string | undefined
872+
const mockFinishAgentRun = mock(async (params: { status: string }) => {
873+
finishAgentRunStatus = params.status
874+
})
875+
876+
// Mock promptAiSdkStream to throw a regular error (not AbortError)
877+
loopAgentStepsBaseParams.promptAiSdkStream = async function* () {
878+
yield { type: 'text' as const, text: 'Starting...\n' }
879+
throw new Error('Network connection failed')
880+
}
881+
882+
const result = await loopAgentSteps({
883+
...loopAgentStepsBaseParams,
884+
agentType: 'test-agent',
885+
localAgentTemplates,
886+
finishAgentRun: mockFinishAgentRun,
887+
})
888+
889+
// Verify the output indicates an error (not cancellation)
890+
expect(result.output.type).toBe('error')
891+
if (result.output.type === 'error') {
892+
expect(result.output.message).toContain('Network connection failed')
893+
expect(result.output.message).not.toBe('Run cancelled by user')
894+
}
895+
896+
// Verify finishAgentRun was called with 'failed' status (not 'cancelled')
897+
expect(mockFinishAgentRun).toHaveBeenCalled()
898+
expect(finishAgentRunStatus).toBe('failed')
899+
})
900+
901+
it('should handle signal.aborted before loop starts', async () => {
902+
// Test that if signal is already aborted when loopAgentSteps is called,
903+
// it returns immediately with a cancelled message
904+
905+
const abortController = new AbortController()
906+
abortController.abort() // Abort immediately
907+
908+
const llmOnlyTemplate = {
909+
...mockTemplate,
910+
handleSteps: undefined,
911+
}
912+
913+
const localAgentTemplates = {
914+
'test-agent': llmOnlyTemplate,
915+
}
916+
917+
const result = await loopAgentSteps({
918+
...loopAgentStepsBaseParams,
919+
agentType: 'test-agent',
920+
localAgentTemplates,
921+
signal: abortController.signal,
922+
})
923+
924+
// Verify the output indicates cancellation
925+
expect(result.output.type).toBe('error')
926+
if (result.output.type === 'error') {
927+
expect(result.output.message).toBe('Run cancelled by user')
928+
}
929+
930+
// LLM should not have been called since we aborted before starting
931+
expect(llmCallCount).toBe(0)
932+
})
933+
})
810934
})

packages/agent-runtime/src/__tests__/main-prompt.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as analytics from '@codebuff/common/analytics'
22
import { TEST_USER_ID } from '@codebuff/common/old-constants'
33
import { createTestAgentRuntimeParams } from '@codebuff/common/testing/fixtures/agent-runtime'
4+
import { promptSuccess } from '@codebuff/common/util/error'
45
import {
56
AgentTemplateTypes,
67
getInitialSessionState,
@@ -105,13 +106,13 @@ describe('mainPrompt', () => {
105106
// Mock processFileBlock
106107
spyOn(processFileBlockModule, 'processFileBlock').mockImplementation(
107108
async (params) => {
108-
return {
109+
return promptSuccess({
109110
tool: 'write_file' as const,
110111
path: params.path,
111112
content: params.newContent,
112113
patch: undefined,
113114
messages: [],
114-
}
115+
})
115116
},
116117
)
117118

0 commit comments

Comments
 (0)