fix(prompt-retry): preserve async holds without blocking validation fallbacks
This commit is contained in:
@@ -6,7 +6,7 @@ import {
|
|||||||
clearAllDelegatedChildSessionBootstrap,
|
clearAllDelegatedChildSessionBootstrap,
|
||||||
getDelegatedChildSessionBootstrap,
|
getDelegatedChildSessionBootstrap,
|
||||||
} from "../../shared/delegated-child-session-bootstrap"
|
} from "../../shared/delegated-child-session-bootstrap"
|
||||||
import { dispatchInternalPrompt } from "../../shared/prompt-async-gate"
|
import { dispatchInternalPrompt, releaseAllPromptAsyncReservationsForTesting } from "../../shared/prompt-async-gate"
|
||||||
import { clearSessionPromptParams, getSessionPromptParams } from "../../shared/session-prompt-params-state"
|
import { clearSessionPromptParams, getSessionPromptParams } from "../../shared/session-prompt-params-state"
|
||||||
import {
|
import {
|
||||||
getSessionAgent,
|
getSessionAgent,
|
||||||
@@ -26,6 +26,7 @@ afterAll(() => { mock.restore() })
|
|||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
clearBackgroundTaskRegistryForTesting()
|
clearBackgroundTaskRegistryForTesting()
|
||||||
|
releaseAllPromptAsyncReservationsForTesting()
|
||||||
})
|
})
|
||||||
|
|
||||||
const TASK_TTL_MS = 30 * 60 * 1000
|
const TASK_TTL_MS = 30 * 60 * 1000
|
||||||
|
|||||||
@@ -3,12 +3,14 @@ import {
|
|||||||
clearSessionPromptParams,
|
clearSessionPromptParams,
|
||||||
getSessionPromptParams,
|
getSessionPromptParams,
|
||||||
} from "../../shared/session-prompt-params-state"
|
} from "../../shared/session-prompt-params-state"
|
||||||
|
import { releaseAllPromptAsyncReservationsForTesting } from "../../shared/prompt-async-gate"
|
||||||
import { createTask, startTask } from "./spawner"
|
import { createTask, startTask } from "./spawner"
|
||||||
import type { BackgroundTask } from "./types"
|
import type { BackgroundTask } from "./types"
|
||||||
|
|
||||||
describe("background-agent spawner agent-not-found fallback", () => {
|
describe("background-agent spawner agent-not-found fallback", () => {
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
clearSessionPromptParams("session-fallback")
|
clearSessionPromptParams("session-fallback")
|
||||||
|
releaseAllPromptAsyncReservationsForTesting()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("retries with 'general' agent when promptAsync fails with Agent not found", async () => {
|
test("retries with 'general' agent when promptAsync fails with Agent not found", async () => {
|
||||||
|
|||||||
@@ -405,6 +405,67 @@ describe("promptWithModelSuggestionRetry", () => {
|
|||||||
expect(promptMock).toHaveBeenCalledTimes(1)
|
expect(promptMock).toHaveBeenCalledTimes(1)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("#given promptAsync throws after dispatch was attempted #when caller observes the error #then the post-dispatch hold remains reserved", async () => {
|
||||||
|
// given
|
||||||
|
const promptMock = mock().mockRejectedValueOnce(new Error("JSON Parse error: Unexpected EOF"))
|
||||||
|
const client = { session: { promptAsync: promptMock } }
|
||||||
|
const args = {
|
||||||
|
path: { id: "session-failed-async-hold" },
|
||||||
|
body: {
|
||||||
|
parts: [{ type: "text", text: "hello" }],
|
||||||
|
model: { providerID: "anthropic", modelID: "claude-sonnet-4" },
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
await expect(
|
||||||
|
promptWithModelSuggestionRetry(unsafeTestValue(client), args)
|
||||||
|
).rejects.toThrow("Unexpected EOF")
|
||||||
|
const second = await dispatchInternalPrompt({
|
||||||
|
mode: "async",
|
||||||
|
client,
|
||||||
|
sessionID: "session-failed-async-hold",
|
||||||
|
input: args,
|
||||||
|
source: "test:after-failed-async",
|
||||||
|
settleMs: 0,
|
||||||
|
postDispatchHoldMs: 0,
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(second).toEqual({ status: "reserved", reservedBy: "model-suggestion-retry" })
|
||||||
|
expect(promptMock).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#given promptAsync rejects before acceptance with an agent lookup error #when retried immediately #then the reservation is released", async () => {
|
||||||
|
// given
|
||||||
|
const promptMock = mock()
|
||||||
|
.mockRejectedValueOnce(new Error("Agent not found: missing-agent"))
|
||||||
|
.mockResolvedValueOnce(undefined)
|
||||||
|
const client = { session: { promptAsync: promptMock } }
|
||||||
|
const args = {
|
||||||
|
path: { id: "session-agent-preaccept-failure" },
|
||||||
|
body: {
|
||||||
|
agent: "missing-agent",
|
||||||
|
parts: [{ type: "text", text: "hello" }],
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
await expect(
|
||||||
|
promptWithModelSuggestionRetry(unsafeTestValue(client), args)
|
||||||
|
).rejects.toThrow("Agent not found")
|
||||||
|
await promptWithModelSuggestionRetry(unsafeTestValue(client), {
|
||||||
|
...args,
|
||||||
|
body: {
|
||||||
|
...args.body,
|
||||||
|
agent: "general",
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptMock).toHaveBeenCalledTimes(2)
|
||||||
|
})
|
||||||
|
|
||||||
it("should pass all body fields through to promptAsync", async () => {
|
it("should pass all body fields through to promptAsync", async () => {
|
||||||
// given a client where promptAsync succeeds
|
// given a client where promptAsync succeeds
|
||||||
const promptMock = mock().mockResolvedValueOnce(undefined)
|
const promptMock = mock().mockResolvedValueOnce(undefined)
|
||||||
|
|||||||
@@ -8,7 +8,6 @@ import {
|
|||||||
import {
|
import {
|
||||||
dispatchInternalPrompt,
|
dispatchInternalPrompt,
|
||||||
releasePromptAsyncReservation,
|
releasePromptAsyncReservation,
|
||||||
type InternalPromptDispatchResult,
|
|
||||||
} from "./prompt-async-gate"
|
} from "./prompt-async-gate"
|
||||||
|
|
||||||
type Client = ReturnType<typeof createOpencodeClient>
|
type Client = ReturnType<typeof createOpencodeClient>
|
||||||
@@ -34,6 +33,15 @@ function extractMessage(error: unknown): string {
|
|||||||
return String(error)
|
return String(error)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isAgentResolutionError(error: unknown): boolean {
|
||||||
|
const message = extractMessage(error)
|
||||||
|
return message.includes("Agent not found") || message.includes("agent.name")
|
||||||
|
}
|
||||||
|
|
||||||
|
function shouldReleaseReservationAfterFailedAsyncPrompt(error: unknown): boolean {
|
||||||
|
return parseModelSuggestion(error) !== null || isAgentResolutionError(error)
|
||||||
|
}
|
||||||
|
|
||||||
export function parseModelSuggestion(error: unknown): ModelSuggestionInfo | null {
|
export function parseModelSuggestion(error: unknown): ModelSuggestionInfo | null {
|
||||||
if (!error) return null
|
if (!error) return null
|
||||||
|
|
||||||
@@ -98,10 +106,9 @@ export async function promptWithModelSuggestionRetry(
|
|||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
const timeoutMs = options.timeoutMs ?? PROMPT_TIMEOUT_MS
|
const timeoutMs = options.timeoutMs ?? PROMPT_TIMEOUT_MS
|
||||||
const timeoutContext = createPromptTimeoutContext(args, timeoutMs)
|
const timeoutContext = createPromptTimeoutContext(args, timeoutMs)
|
||||||
let promptResult: InternalPromptDispatchResult | undefined
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
promptResult = await dispatchInternalPrompt({
|
const promptResult = await dispatchInternalPrompt({
|
||||||
mode: "async",
|
mode: "async",
|
||||||
client,
|
client,
|
||||||
sessionID: args.path.id,
|
sessionID: args.path.id,
|
||||||
@@ -125,7 +132,7 @@ export async function promptWithModelSuggestionRetry(
|
|||||||
if (timeoutContext.wasTimedOut()) {
|
if (timeoutContext.wasTimedOut()) {
|
||||||
throw new Error(`promptAsync timed out after ${timeoutMs}ms`)
|
throw new Error(`promptAsync timed out after ${timeoutMs}ms`)
|
||||||
}
|
}
|
||||||
if (promptResult?.status === "failed") {
|
if (shouldReleaseReservationAfterFailedAsyncPrompt(error)) {
|
||||||
releasePromptAsyncReservation(args.path.id, "model-suggestion-retry")
|
releasePromptAsyncReservation(args.path.id, "model-suggestion-retry")
|
||||||
}
|
}
|
||||||
throw error
|
throw error
|
||||||
|
|||||||
@@ -412,4 +412,46 @@ bunDescribe("sendSyncPrompt", () => {
|
|||||||
bunExpect(promptWithModelSuggestionRetry).toHaveBeenCalledTimes(1)
|
bunExpect(promptWithModelSuggestionRetry).toHaveBeenCalledTimes(1)
|
||||||
bunExpect(promptSyncWithModelSuggestionRetry).toHaveBeenCalledTimes(0)
|
bunExpect(promptSyncWithModelSuggestionRetry).toHaveBeenCalledTimes(0)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
bunTest("#given oracle promptSync fallback is blocked by the prompt gate #when async prompt reports EOF #then the original EOF error is preserved", async () => {
|
||||||
|
//#given
|
||||||
|
const { sendSyncPrompt } = require("./sync-prompt-sender")
|
||||||
|
|
||||||
|
const promptWithModelSuggestionRetry = bunMock(async () => {
|
||||||
|
throw new Error("JSON Parse error: Unexpected EOF")
|
||||||
|
})
|
||||||
|
const promptSyncWithModelSuggestionRetry = bunMock(async () => {
|
||||||
|
throw new Error("prompt skipped by gate: reserved")
|
||||||
|
})
|
||||||
|
|
||||||
|
const input = {
|
||||||
|
sessionID: "test-session",
|
||||||
|
agentToUse: "oracle",
|
||||||
|
args: {
|
||||||
|
description: "test task",
|
||||||
|
prompt: "test prompt",
|
||||||
|
run_in_background: false,
|
||||||
|
load_skills: [],
|
||||||
|
},
|
||||||
|
systemContent: undefined,
|
||||||
|
categoryModel: undefined,
|
||||||
|
toastManager: null,
|
||||||
|
taskId: undefined,
|
||||||
|
}
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = await sendSyncPrompt(
|
||||||
|
{ session: { promptAsync: bunMock(async () => ({ data: {} })) } },
|
||||||
|
input,
|
||||||
|
{
|
||||||
|
promptWithModelSuggestionRetry,
|
||||||
|
promptSyncWithModelSuggestionRetry,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
bunExpect(result).toContain("JSON Parse error")
|
||||||
|
bunExpect(result).not.toContain("prompt skipped by gate")
|
||||||
|
bunExpect(promptSyncWithModelSuggestionRetry).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -52,6 +52,11 @@ function isUnexpectedEofError(error: unknown): boolean {
|
|||||||
return lowered.includes("unexpected eof") || lowered.includes("json parse error")
|
return lowered.includes("unexpected eof") || lowered.includes("json parse error")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isPromptGateReservedError(error: unknown): boolean {
|
||||||
|
const message = error instanceof Error ? error.message : String(error)
|
||||||
|
return message.includes("promptAsync skipped by gate: reserved") || message.includes("prompt skipped by gate: reserved")
|
||||||
|
}
|
||||||
|
|
||||||
export function buildSyncPromptTools(agentToUse: string): Record<string, boolean> {
|
export function buildSyncPromptTools(agentToUse: string): Record<string, boolean> {
|
||||||
return {
|
return {
|
||||||
task: isPlanFamily(agentToUse),
|
task: isPlanFamily(agentToUse),
|
||||||
@@ -112,7 +117,9 @@ export async function sendSyncPrompt(
|
|||||||
await deps.promptSyncWithModelSuggestionRetry(client, routePromptSyncRetry(promptArgs, input.directory))
|
await deps.promptSyncWithModelSuggestionRetry(client, routePromptSyncRetry(promptArgs, input.directory))
|
||||||
return null
|
return null
|
||||||
} catch (oracleRetryError) {
|
} catch (oracleRetryError) {
|
||||||
promptError = oracleRetryError
|
if (!isPromptGateReservedError(oracleRetryError)) {
|
||||||
|
promptError = oracleRetryError
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -16,6 +16,10 @@ function toDelegatedModelConfig(fallback: NonNullable<ReturnType<typeof getNextR
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isPromptGateReservedError(error: string): boolean {
|
||||||
|
return error.includes("promptAsync skipped by gate: reserved") || error.includes("prompt skipped by gate: reserved")
|
||||||
|
}
|
||||||
|
|
||||||
export async function retrySyncPromptWithFallbacks(input: {
|
export async function retrySyncPromptWithFallbacks(input: {
|
||||||
sessionID: string
|
sessionID: string
|
||||||
initialError: string
|
initialError: string
|
||||||
@@ -63,6 +67,14 @@ export async function retrySyncPromptWithFallbacks(input: {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (isPromptGateReservedError(promptError)) {
|
||||||
|
return {
|
||||||
|
promptError: finalError,
|
||||||
|
categoryModel,
|
||||||
|
fallbackState,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
finalError = promptError
|
finalError = promptError
|
||||||
fallbackState.providerID = fallbackModel.providerID
|
fallbackState.providerID = fallbackModel.providerID
|
||||||
fallbackState.modelID = fallbackModel.modelID
|
fallbackState.modelID = fallbackModel.modelID
|
||||||
|
|||||||
@@ -514,6 +514,34 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
])
|
])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("#given sync prompt fallback is blocked by the prompt gate #when retrying prompt fallback #then preserves the original prompt error", async () => {
|
||||||
|
//#given
|
||||||
|
const { retrySyncPromptWithFallbacks } = require("./sync-task-fallback")
|
||||||
|
const sendPrompt = mock(async () => "promptAsync skipped by gate: reserved")
|
||||||
|
const initialModel = {
|
||||||
|
providerID: "anthropic",
|
||||||
|
modelID: "claude-opus-4-7",
|
||||||
|
variant: "max",
|
||||||
|
}
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = await retrySyncPromptWithFallbacks({
|
||||||
|
sessionID: "ses_gate_reserved",
|
||||||
|
initialError: "JSON Parse error: Unexpected EOF",
|
||||||
|
categoryModel: initialModel,
|
||||||
|
fallbackChain: [
|
||||||
|
{ providers: ["anthropic"], model: "claude-opus-4-7", variant: "max" },
|
||||||
|
{ providers: ["openai"], model: "gpt-5.4", variant: "medium" },
|
||||||
|
],
|
||||||
|
sendPrompt,
|
||||||
|
})
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result.promptError).toBe("JSON Parse error: Unexpected EOF")
|
||||||
|
expect(result.categoryModel).toEqual(initialModel)
|
||||||
|
expect(sendPrompt).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
|
||||||
test("cleans up toast and subagentSessions on successful completion", async () => {
|
test("cleans up toast and subagentSessions on successful completion", async () => {
|
||||||
const mockClient = {
|
const mockClient = {
|
||||||
session: {
|
session: {
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import { clearSkillCache } from "../../features/opencode-skill-loader/skill-cont
|
|||||||
import { __setTimingConfig, __resetTimingConfig } from "./timing"
|
import { __setTimingConfig, __resetTimingConfig } from "./timing"
|
||||||
import * as connectedProvidersCache from "../../shared/connected-providers-cache"
|
import * as connectedProvidersCache from "../../shared/connected-providers-cache"
|
||||||
import * as executor from "./executor"
|
import * as executor from "./executor"
|
||||||
|
import { releaseAllPromptAsyncReservationsForTesting } from "../../shared/prompt-async-gate"
|
||||||
|
|
||||||
const runtimeRequire = require as NodeJS.Require & { cache?: Record<string, unknown> }
|
const runtimeRequire = require as NodeJS.Require & { cache?: Record<string, unknown> }
|
||||||
|
|
||||||
@@ -78,6 +79,7 @@ describe("sisyphus-task", () => {
|
|||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
__resetTimingConfig()
|
__resetTimingConfig()
|
||||||
|
releaseAllPromptAsyncReservationsForTesting()
|
||||||
cacheSpy?.mockRestore()
|
cacheSpy?.mockRestore()
|
||||||
providerModelsSpy?.mockRestore()
|
providerModelsSpy?.mockRestore()
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user