Merge pull request #4007 from PeterPonyu/feat/runtime-fallback-internal-abort
fix(runtime-fallback): preserve attemptCount when our own abort is the cause
This commit is contained in:
@@ -35,6 +35,16 @@ export function createAutoRetryHelpers(deps: HookDeps) {
|
|||||||
} = deps
|
} = deps
|
||||||
|
|
||||||
const abortSessionRequest = async (sessionID: string, source: string): Promise<void> => {
|
const abortSessionRequest = async (sessionID: string, source: string): Promise<void> => {
|
||||||
|
// Sources we trigger ourselves to swap in a fallback model. Marking the
|
||||||
|
// session lets handleSessionError tell our abort apart from a user stop
|
||||||
|
// so it doesn't wipe attemptCount and re-enter the retry loop.
|
||||||
|
if (
|
||||||
|
source === "session.status.retry-signal" ||
|
||||||
|
source === "message.updated.retry-signal" ||
|
||||||
|
source === "session.timeout"
|
||||||
|
) {
|
||||||
|
deps.internallyAbortedSessions.add(sessionID)
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
await ctx.client.session.abort({ path: { id: sessionID } })
|
await ctx.client.session.abort({ path: { id: sessionID } })
|
||||||
releasePromptAsyncReservation(sessionID, `runtime-fallback-abort:${source}`)
|
releasePromptAsyncReservation(sessionID, `runtime-fallback-abort:${source}`)
|
||||||
|
|||||||
@@ -39,6 +39,7 @@ function createDeps(): HookDeps {
|
|||||||
sessionAwaitingFallbackResult: new Set(),
|
sessionAwaitingFallbackResult: new Set(),
|
||||||
sessionFallbackTimeouts: new Map(),
|
sessionFallbackTimeouts: new Map(),
|
||||||
sessionStatusRetryKeys: new Map(),
|
sessionStatusRetryKeys: new Map(),
|
||||||
|
internallyAbortedSessions: new Set(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -162,4 +163,89 @@ describe("createEventHandler", () => {
|
|||||||
expect(clearCalls).toEqual([sessionID])
|
expect(clearCalls).toEqual([sessionID])
|
||||||
expect(abortCalls).toEqual([])
|
expect(abortCalls).toEqual([])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("#given a session we aborted ourselves (internal abort flag set) #when session.error fires with isAbort #then fallback retry state is preserved (issue #4006)", async () => {
|
||||||
|
// given - we just called abortSessionRequest("session.status.retry-signal");
|
||||||
|
// opencode will emit session.error{isAbort:true} as a consequence. The
|
||||||
|
// handler must recognize this as our own abort and NOT wipe attemptCount,
|
||||||
|
// otherwise the next session.status retry signal restarts the loop at 1.
|
||||||
|
const sessionID = "session-internal-abort"
|
||||||
|
const deps = createDeps()
|
||||||
|
const abortCalls: string[] = []
|
||||||
|
const clearCalls: string[] = []
|
||||||
|
const state = createFallbackState("opencode-go/glm-5.1")
|
||||||
|
state.currentModel = "github-copilot/claude-haiku-4.5"
|
||||||
|
state.fallbackIndex = 0
|
||||||
|
state.attemptCount = 1
|
||||||
|
state.pendingFallbackModel = "github-copilot/claude-haiku-4.5"
|
||||||
|
deps.sessionStates.set(sessionID, state)
|
||||||
|
deps.internallyAbortedSessions.add(sessionID)
|
||||||
|
const handler = createEventHandler(deps, createHelpers(deps, abortCalls, clearCalls))
|
||||||
|
|
||||||
|
// when
|
||||||
|
await handler({ event: { type: "session.error", properties: { sessionID, error: { name: "MessageAbortedError" } } } })
|
||||||
|
|
||||||
|
// then - state intact, attemptCount preserved
|
||||||
|
const preserved = deps.sessionStates.get(sessionID)
|
||||||
|
expect(preserved?.attemptCount).toBe(1)
|
||||||
|
expect(preserved?.currentModel).toBe("github-copilot/claude-haiku-4.5")
|
||||||
|
expect(preserved?.fallbackIndex).toBe(0)
|
||||||
|
// flag was consumed so a subsequent user abort still gets the reset path
|
||||||
|
expect(deps.internallyAbortedSessions.has(sessionID)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#given an external abort (no internal flag) #when session.error fires with isAbort #then state is still reset as a real cancellation", async () => {
|
||||||
|
// given - regression guard: user-initiated abort path must continue to
|
||||||
|
// wipe state. Only OUR internal aborts get the preservation treatment.
|
||||||
|
const sessionID = "session-external-abort"
|
||||||
|
const deps = createDeps()
|
||||||
|
const abortCalls: string[] = []
|
||||||
|
const clearCalls: string[] = []
|
||||||
|
const state = createFallbackState("opencode-go/glm-5.1")
|
||||||
|
state.currentModel = "github-copilot/claude-haiku-4.5"
|
||||||
|
state.attemptCount = 1
|
||||||
|
deps.sessionStates.set(sessionID, state)
|
||||||
|
// NB: internallyAbortedSessions is empty
|
||||||
|
const handler = createEventHandler(deps, createHelpers(deps, abortCalls, clearCalls))
|
||||||
|
|
||||||
|
// when
|
||||||
|
await handler({ event: { type: "session.error", properties: { sessionID, error: { name: "MessageAbortedError" } } } })
|
||||||
|
|
||||||
|
// then - state reset, behaviour matches pre-fix cancellation path
|
||||||
|
const reset = deps.sessionStates.get(sessionID)
|
||||||
|
expect(reset?.attemptCount).toBe(0)
|
||||||
|
expect(reset?.currentModel).toBe("opencode-go/glm-5.1")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#given two consecutive internal-abort cycles #when session.error fires each time #then attemptCount can progress past 1", async () => {
|
||||||
|
// given - the failure mode in issue #4006 manifested as attempt:1 looping
|
||||||
|
// forever because every cycle reset attemptCount. This test verifies the
|
||||||
|
// counter actually advances when the internal-abort flag is honored
|
||||||
|
// across multiple iterations.
|
||||||
|
const sessionID = "session-progressing-attempts"
|
||||||
|
const deps = createDeps()
|
||||||
|
const abortCalls: string[] = []
|
||||||
|
const clearCalls: string[] = []
|
||||||
|
const state = createFallbackState("opencode-go/glm-5.1")
|
||||||
|
state.attemptCount = 1
|
||||||
|
state.pendingFallbackModel = "github-copilot/claude-haiku-4.5"
|
||||||
|
deps.sessionStates.set(sessionID, state)
|
||||||
|
const handler = createEventHandler(deps, createHelpers(deps, abortCalls, clearCalls))
|
||||||
|
|
||||||
|
// iteration 1: internal abort -> session.error{isAbort:true}
|
||||||
|
deps.internallyAbortedSessions.add(sessionID)
|
||||||
|
await handler({ event: { type: "session.error", properties: { sessionID, error: { name: "MessageAbortedError" } } } })
|
||||||
|
expect(deps.sessionStates.get(sessionID)?.attemptCount).toBe(1)
|
||||||
|
|
||||||
|
// simulate the next retry signal advancing the counter
|
||||||
|
const advanced = deps.sessionStates.get(sessionID)!
|
||||||
|
advanced.attemptCount = 2
|
||||||
|
|
||||||
|
// iteration 2: another internal abort
|
||||||
|
deps.internallyAbortedSessions.add(sessionID)
|
||||||
|
await handler({ event: { type: "session.error", properties: { sessionID, error: { name: "MessageAbortedError" } } } })
|
||||||
|
|
||||||
|
// then - counter is at 2, not reset to 0
|
||||||
|
expect(deps.sessionStates.get(sessionID)?.attemptCount).toBe(2)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -138,6 +138,14 @@ export function createEventHandler(deps: HookDeps, helpers: AutoRetryHelpers) {
|
|||||||
const resolvedAgent = await helpers.resolveAgentForSessionFromContext(sessionID, agent)
|
const resolvedAgent = await helpers.resolveAgentForSessionFromContext(sessionID, agent)
|
||||||
|
|
||||||
if (isAbortError(error)) {
|
if (isAbortError(error)) {
|
||||||
|
// If we triggered this abort to swap in a fallback model, consume the
|
||||||
|
// flag and preserve state — wiping attemptCount here is what causes
|
||||||
|
// the infinite retry loop (issue #4006).
|
||||||
|
if (deps.internallyAbortedSessions.has(sessionID)) {
|
||||||
|
deps.internallyAbortedSessions.delete(sessionID)
|
||||||
|
log(`[${HOOK_NAME}] session.error matched internal abort; preserving retry state`, { sessionID, resolvedAgent })
|
||||||
|
return
|
||||||
|
}
|
||||||
cancelledSessions.add(sessionID)
|
cancelledSessions.add(sessionID)
|
||||||
resetRetryState(sessionID)
|
resetRetryState(sessionID)
|
||||||
log(`[${HOOK_NAME}] session.error matched cancellation; cleared retry state`, { sessionID, resolvedAgent })
|
log(`[${HOOK_NAME}] session.error matched cancellation; cleared retry state`, { sessionID, resolvedAgent })
|
||||||
|
|||||||
@@ -52,6 +52,7 @@ export function createRuntimeFallbackHook(
|
|||||||
sessionAwaitingFallbackResult: new Set(),
|
sessionAwaitingFallbackResult: new Set(),
|
||||||
sessionFallbackTimeouts: new Map(),
|
sessionFallbackTimeouts: new Map(),
|
||||||
sessionStatusRetryKeys: new Map(),
|
sessionStatusRetryKeys: new Map(),
|
||||||
|
internallyAbortedSessions: new Set(),
|
||||||
}
|
}
|
||||||
|
|
||||||
const helpers = factories.createAutoRetryHelpers(deps)
|
const helpers = factories.createAutoRetryHelpers(deps)
|
||||||
@@ -100,6 +101,7 @@ export function createRuntimeFallbackHook(
|
|||||||
deps.sessionAwaitingFallbackResult.clear()
|
deps.sessionAwaitingFallbackResult.clear()
|
||||||
deps.sessionFallbackTimeouts.clear()
|
deps.sessionFallbackTimeouts.clear()
|
||||||
deps.sessionStatusRetryKeys.clear()
|
deps.sessionStatusRetryKeys.clear()
|
||||||
|
deps.internallyAbortedSessions.clear()
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
@@ -74,4 +74,12 @@ export interface HookDeps {
|
|||||||
sessionAwaitingFallbackResult: Set<string>
|
sessionAwaitingFallbackResult: Set<string>
|
||||||
sessionFallbackTimeouts: Map<string, RuntimeFallbackTimeout>
|
sessionFallbackTimeouts: Map<string, RuntimeFallbackTimeout>
|
||||||
sessionStatusRetryKeys: Map<string, string>
|
sessionStatusRetryKeys: Map<string, string>
|
||||||
|
/**
|
||||||
|
* Sessions whose in-flight request was aborted by us (to swap in a fallback
|
||||||
|
* model), as opposed to a user-initiated stop. Consumed by
|
||||||
|
* handleSessionError so the resulting session.error{isAbort:true} does NOT
|
||||||
|
* reset attemptCount — that reset is what was driving the infinite retry
|
||||||
|
* loop (every cycle started over at attempt:1). See issue #4006.
|
||||||
|
*/
|
||||||
|
internallyAbortedSessions: Set<string>
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user