Merge pull request #3825 from tw-yshuang/fix/delegated-child-session-early-failure-fallback
fix(delegate-task): harden child-session first-prompt fallback recovery
This commit is contained in:
@@ -6,26 +6,31 @@ import { buildTaskPrompt } from "./prompt-builder"
|
||||
import { publishToolMetadata } from "../../features/tool-metadata-store"
|
||||
import { formatDetailedError } from "./error-formatting"
|
||||
import { getSessionTools } from "../../shared/session-tools-store"
|
||||
import { SessionCategoryRegistry } from "../../shared/session-category-registry"
|
||||
import { QUESTION_DENIED_SESSION_PERMISSION } from "../../shared/question-denied-session-permission"
|
||||
import { stripAgentListSortPrefix } from "../../shared/agent-display-names"
|
||||
import { buildTaskMetadataBlock } from "../../features/tool-metadata-store/task-metadata-contract"
|
||||
import { resolveMetadataModel } from "./resolve-metadata-model"
|
||||
import { registerDelegatedChildSessionBootstrap } from "../../shared/delegated-child-session-bootstrap"
|
||||
|
||||
function registerBackgroundSessionContext(args: {
|
||||
sessionId: string
|
||||
promptText: string
|
||||
fallbackChain?: FallbackEntry[]
|
||||
category?: string
|
||||
modelFallbackControllerAccessor?: ExecutorContext["modelFallbackControllerAccessor"]
|
||||
}): void {
|
||||
args.modelFallbackControllerAccessor?.setSessionFallbackChain(args.sessionId, args.fallbackChain)
|
||||
if (args.category) {
|
||||
SessionCategoryRegistry.register(args.sessionId, args.category)
|
||||
}
|
||||
registerDelegatedChildSessionBootstrap({
|
||||
sessionID: args.sessionId,
|
||||
promptText: args.promptText,
|
||||
fallbackChain: args.fallbackChain,
|
||||
category: args.category,
|
||||
modelFallbackControllerAccessor: args.modelFallbackControllerAccessor,
|
||||
})
|
||||
}
|
||||
|
||||
function continueSessionSetup(args: {
|
||||
taskID: string
|
||||
promptText: string
|
||||
manager: ExecutorContext["manager"]
|
||||
timing: ReturnType<typeof getTimingConfig>
|
||||
fallbackChain?: FallbackEntry[]
|
||||
@@ -55,6 +60,7 @@ function continueSessionSetup(args: {
|
||||
|
||||
registerBackgroundSessionContext({
|
||||
sessionId,
|
||||
promptText: args.promptText,
|
||||
fallbackChain: args.fallbackChain,
|
||||
category: args.category,
|
||||
modelFallbackControllerAccessor: args.modelFallbackControllerAccessor,
|
||||
@@ -144,6 +150,7 @@ export async function executeBackgroundTask(
|
||||
onAbort: () => {
|
||||
continueSessionSetup({
|
||||
taskID: task.id,
|
||||
promptText: effectivePrompt,
|
||||
manager,
|
||||
timing,
|
||||
fallbackChain,
|
||||
@@ -160,15 +167,6 @@ export async function executeBackgroundTask(
|
||||
return `Task failed to start (status: ${updatedTask.status}).\n\nTask ID: ${task.id}`
|
||||
}
|
||||
|
||||
if (sessionId) {
|
||||
registerBackgroundSessionContext({
|
||||
sessionId,
|
||||
fallbackChain,
|
||||
category: args.category,
|
||||
modelFallbackControllerAccessor: executorCtx.modelFallbackControllerAccessor,
|
||||
})
|
||||
}
|
||||
|
||||
const resolvedModel = resolveMetadataModel(categoryModel, parentContext.model)
|
||||
const metadata = {
|
||||
prompt: args.prompt,
|
||||
|
||||
@@ -58,6 +58,7 @@ export async function sendSyncPrompt(
|
||||
sessionID: string
|
||||
agentToUse: string
|
||||
args: DelegateTaskArgs
|
||||
promptText?: string
|
||||
systemContent: string | undefined
|
||||
categoryModel: DelegatedModelConfig | undefined
|
||||
directory: string
|
||||
@@ -69,7 +70,7 @@ export async function sendSyncPrompt(
|
||||
): Promise<string | null> {
|
||||
const allowTask = isPlanFamily(input.agentToUse)
|
||||
const tddEnabled = input.sisyphusAgentConfig?.tdd
|
||||
const effectivePrompt = buildTaskPrompt(input.args.prompt, input.agentToUse, tddEnabled)
|
||||
const effectivePrompt = input.promptText ?? buildTaskPrompt(input.args.prompt, input.agentToUse, tddEnabled)
|
||||
const tools = {
|
||||
task: allowTask,
|
||||
call_omo_agent: true,
|
||||
|
||||
@@ -29,6 +29,8 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
addCalls = []
|
||||
|
||||
clearRequireCache("./sync-task")
|
||||
const { clearAllDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
||||
clearAllDelegatedChildSessionBootstrap()
|
||||
|
||||
const { initTaskToastManager, _resetTaskToastManagerForTesting } = require("../../features/task-toast-manager/manager")
|
||||
_resetTaskToastManagerForTesting()
|
||||
@@ -62,6 +64,8 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
mock.restore()
|
||||
resetToastManager?.()
|
||||
resetToastManager = null
|
||||
const { clearAllDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
||||
clearAllDelegatedChildSessionBootstrap()
|
||||
})
|
||||
|
||||
test("cleans up toast and subagentSessions when fetchSyncResult returns ok: false", async () => {
|
||||
@@ -385,16 +389,35 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
}
|
||||
|
||||
const { executeSyncTask } = require("./sync-task")
|
||||
const { getDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
||||
const attemptedModels: Array<{ providerID: string; modelID: string; variant?: string } | undefined> = []
|
||||
const promptSessionIDs: string[] = []
|
||||
const pollSessionIDs: string[] = []
|
||||
const fetchSessionIDs: string[] = []
|
||||
const bootstrapSnapshots: Array<{ retryParts: Array<{ type: "text"; text: string }> } | undefined> = []
|
||||
let createSyncSessionCalls = 0
|
||||
const setSessionFallbackChain = mock(() => {})
|
||||
const clearSessionFallbackChain = mock(() => {})
|
||||
|
||||
const deps = {
|
||||
createSyncSession: async () => ({ ok: true, sessionID: "ses_test_12345678" }),
|
||||
sendSyncPrompt: async (_client: unknown, input: { categoryModel?: { providerID: string; modelID: string; variant?: string } }) => {
|
||||
createSyncSession: async () => {
|
||||
createSyncSessionCalls += 1
|
||||
return { ok: true as const, sessionID: "ses_test_12345678" }
|
||||
},
|
||||
sendSyncPrompt: async (_client: unknown, input: { sessionID: string; categoryModel?: { providerID: string; modelID: string; variant?: string } }) => {
|
||||
promptSessionIDs.push(input.sessionID)
|
||||
bootstrapSnapshots.push(getDelegatedChildSessionBootstrap(input.sessionID))
|
||||
attemptedModels.push(input.categoryModel)
|
||||
return attemptedModels.length === 1 ? "Initial failure" : null
|
||||
},
|
||||
pollSyncSession: async () => null,
|
||||
fetchSyncResult: async () => ({ ok: true as const, textContent: "Result" }),
|
||||
pollSyncSession: async (_ctx: unknown, _client: unknown, input: { sessionID: string }) => {
|
||||
pollSessionIDs.push(input.sessionID)
|
||||
return null
|
||||
},
|
||||
fetchSyncResult: async (_client: unknown, sessionID: string) => {
|
||||
fetchSessionIDs.push(sessionID)
|
||||
return { ok: true as const, textContent: "Result" }
|
||||
},
|
||||
}
|
||||
|
||||
const mockCtx = {
|
||||
@@ -407,6 +430,10 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
client: mockClient,
|
||||
directory: "/tmp",
|
||||
onSyncSessionCreated: null,
|
||||
modelFallbackControllerAccessor: {
|
||||
setSessionFallbackChain,
|
||||
clearSessionFallbackChain,
|
||||
},
|
||||
}
|
||||
|
||||
const args = {
|
||||
@@ -440,6 +467,14 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
{ providerID: "anthropic", modelID: "claude-opus-4-7", variant: "max" },
|
||||
{ providerID: "opencode-go", modelID: "kimi-k2.6", variant: undefined },
|
||||
])
|
||||
expect(setSessionFallbackChain).toHaveBeenCalledWith("ses_test_12345678", fallbackChain)
|
||||
expect(bootstrapSnapshots[0]?.retryParts[0]?.text).toContain("test prompt")
|
||||
expect(createSyncSessionCalls).toBe(1)
|
||||
expect(promptSessionIDs).toEqual(["ses_test_12345678", "ses_test_12345678"])
|
||||
expect(pollSessionIDs).toEqual(["ses_test_12345678"])
|
||||
expect(fetchSessionIDs).toEqual(["ses_test_12345678"])
|
||||
expect(clearSessionFallbackChain).toHaveBeenCalledWith("ses_test_12345678")
|
||||
expect(getDelegatedChildSessionBootstrap("ses_test_12345678")).toBeUndefined()
|
||||
})
|
||||
|
||||
test("#given fallback chain exhausted #when all retries fail #then returns final error", async () => {
|
||||
@@ -510,6 +545,110 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
])
|
||||
})
|
||||
|
||||
test("keeps concurrent delegated first-prompt fallback bootstrap isolated per session", async () => {
|
||||
const mockClient = {
|
||||
session: {
|
||||
create: async () => ({ data: { id: "ignored" } }),
|
||||
},
|
||||
}
|
||||
|
||||
const { executeSyncTask } = require("./sync-task")
|
||||
const { getDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
||||
const perSessionAttempts = new Map<string, number>()
|
||||
const bootstrapSnapshots: Array<{ sessionID: string; text: string | undefined }> = []
|
||||
const setSessionFallbackChain = mock(() => {})
|
||||
const clearSessionFallbackChain = mock(() => {})
|
||||
|
||||
const deps = {
|
||||
createSyncSession: async (_client: unknown, input: { description: string }) => {
|
||||
return {
|
||||
ok: true as const,
|
||||
sessionID: input.description === "alpha task" ? "ses_alpha" : "ses_beta",
|
||||
}
|
||||
},
|
||||
sendSyncPrompt: async (_client: unknown, input: { sessionID: string; categoryModel?: { providerID: string; modelID: string; variant?: string } }) => {
|
||||
const bootstrap = getDelegatedChildSessionBootstrap(input.sessionID)
|
||||
bootstrapSnapshots.push({
|
||||
sessionID: input.sessionID,
|
||||
text: bootstrap?.retryParts[0]?.text,
|
||||
})
|
||||
const currentAttempt = (perSessionAttempts.get(input.sessionID) ?? 0) + 1
|
||||
perSessionAttempts.set(input.sessionID, currentAttempt)
|
||||
return currentAttempt === 1 ? `Initial failure for ${input.sessionID}` : null
|
||||
},
|
||||
pollSyncSession: async () => null,
|
||||
fetchSyncResult: async (_client: unknown, sessionID: string) => ({ ok: true as const, textContent: `Result from ${sessionID}` }),
|
||||
}
|
||||
|
||||
const mockExecutorCtx = {
|
||||
client: mockClient,
|
||||
directory: "/tmp",
|
||||
onSyncSessionCreated: null,
|
||||
modelFallbackControllerAccessor: {
|
||||
setSessionFallbackChain,
|
||||
clearSessionFallbackChain,
|
||||
},
|
||||
}
|
||||
|
||||
const alphaArgs = {
|
||||
prompt: "alpha delegated prompt",
|
||||
description: "alpha task",
|
||||
category: "test",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
command: null,
|
||||
}
|
||||
const betaArgs = {
|
||||
prompt: "beta delegated prompt",
|
||||
description: "beta task",
|
||||
category: "test",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
command: null,
|
||||
}
|
||||
const mockCtx = {
|
||||
sessionID: "parent-session",
|
||||
callID: "call-123",
|
||||
metadata: () => {},
|
||||
}
|
||||
|
||||
const [alphaResult, betaResult] = await Promise.all([
|
||||
executeSyncTask(alphaArgs, mockCtx, mockExecutorCtx, { sessionID: "parent-session" }, "test-agent", {
|
||||
providerID: "anthropic",
|
||||
modelID: "claude-opus-4-7",
|
||||
variant: "max",
|
||||
}, undefined, undefined, [
|
||||
{ providers: ["anthropic"], model: "claude-opus-4-7", variant: "max" },
|
||||
{ providers: ["openai"], model: "gpt-5.4" },
|
||||
], deps),
|
||||
executeSyncTask(betaArgs, mockCtx, mockExecutorCtx, { sessionID: "parent-session" }, "test-agent", {
|
||||
providerID: "genai-proxy-openai",
|
||||
modelID: "gpt-5.4-mini",
|
||||
variant: undefined,
|
||||
}, undefined, undefined, [
|
||||
{ providers: ["genai-proxy-openai"], model: "gpt-5.4-mini" },
|
||||
{ providers: ["genai-proxy-aws"], model: "us.anthropic.claude-haiku-4-5-20251001-v1:0" },
|
||||
], deps),
|
||||
])
|
||||
|
||||
expect(alphaResult).toContain("Result from ses_alpha")
|
||||
expect(betaResult).toContain("Result from ses_beta")
|
||||
expect(bootstrapSnapshots.filter((snapshot) => snapshot.sessionID === "ses_alpha").every((snapshot) => snapshot.text?.includes("alpha delegated prompt"))).toBe(true)
|
||||
expect(bootstrapSnapshots.filter((snapshot) => snapshot.sessionID === "ses_beta").every((snapshot) => snapshot.text?.includes("beta delegated prompt"))).toBe(true)
|
||||
expect(setSessionFallbackChain).toHaveBeenCalledWith("ses_alpha", [
|
||||
{ providers: ["anthropic"], model: "claude-opus-4-7", variant: "max" },
|
||||
{ providers: ["openai"], model: "gpt-5.4" },
|
||||
])
|
||||
expect(setSessionFallbackChain).toHaveBeenCalledWith("ses_beta", [
|
||||
{ providers: ["genai-proxy-openai"], model: "gpt-5.4-mini" },
|
||||
{ providers: ["genai-proxy-aws"], model: "us.anthropic.claude-haiku-4-5-20251001-v1:0" },
|
||||
])
|
||||
expect(clearSessionFallbackChain).toHaveBeenCalledWith("ses_alpha")
|
||||
expect(clearSessionFallbackChain).toHaveBeenCalledWith("ses_beta")
|
||||
expect(getDelegatedChildSessionBootstrap("ses_alpha")).toBeUndefined()
|
||||
expect(getDelegatedChildSessionBootstrap("ses_beta")).toBeUndefined()
|
||||
})
|
||||
|
||||
test("cleans up toast and subagentSessions on successful completion", async () => {
|
||||
const mockClient = {
|
||||
session: {
|
||||
@@ -575,6 +714,7 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
})
|
||||
|
||||
test("retries sync session on retryable runtime session error using next fallback model", async () => {
|
||||
const { getDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
||||
const mockClient = {
|
||||
session: {
|
||||
create: async () => ({ data: { id: "ignored" } }),
|
||||
@@ -653,6 +793,8 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
])
|
||||
expect(result).toContain("Result from ses_second")
|
||||
expect(deleteCalls).toContain("ses_first")
|
||||
expect(getDelegatedChildSessionBootstrap("ses_first")).toBeUndefined()
|
||||
expect(getDelegatedChildSessionBootstrap("ses_second")).toBeUndefined()
|
||||
|
||||
const finalMetadata = metadataCalls[metadataCalls.length - 1]
|
||||
expect(finalMetadata.metadata.sessionId).toBe("ses_second")
|
||||
@@ -740,6 +882,7 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
})
|
||||
|
||||
test("publishes latest retry session metadata when final retry still fails", async () => {
|
||||
const { getDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
||||
const mockClient = {
|
||||
session: {
|
||||
create: async () => ({ data: { id: "ignored" } }),
|
||||
@@ -805,7 +948,9 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
}, "sisyphus-junior", initialModel, undefined, undefined, fallbackChain, deps)
|
||||
|
||||
expect(result).toBe("Final retry failed")
|
||||
const finalMetadata = metadataCalls[metadataCalls.length - 1]
|
||||
expect(getDelegatedChildSessionBootstrap("ses_first")).toBeUndefined()
|
||||
expect(getDelegatedChildSessionBootstrap("ses_second")).toBeUndefined()
|
||||
const finalMetadata = metadataCalls.at(-1)
|
||||
expect(finalMetadata.metadata.sessionId).toBe("ses_second")
|
||||
expect(finalMetadata.metadata.taskId).toBe("ses_second")
|
||||
expect(finalMetadata.metadata.model).toEqual({
|
||||
@@ -937,5 +1082,3 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
||||
expect(taskMeta.metadata.spawnDepth).toBe(3) // NOT 1 (the fallback value)
|
||||
})
|
||||
})
|
||||
|
||||
export {}
|
||||
|
||||
@@ -14,6 +14,11 @@ import { buildTaskMetadataBlock } from "../../features/tool-metadata-store/task-
|
||||
import { resolveMetadataModel } from "./resolve-metadata-model"
|
||||
import { shouldRetryError } from "../../shared/model-error-classifier"
|
||||
import type { ModelFallbackState } from "../../hooks/model-fallback/hook"
|
||||
import { buildTaskPrompt } from "./prompt-builder"
|
||||
import {
|
||||
clearDelegatedChildSessionBootstrap,
|
||||
registerDelegatedChildSessionBootstrap,
|
||||
} from "../../shared/delegated-child-session-bootstrap"
|
||||
|
||||
function shouldAttemptPollErrorRecovery(pollError: string): boolean {
|
||||
const trimmed = pollError.trim()
|
||||
@@ -62,6 +67,9 @@ export async function executeSyncTask(
|
||||
| undefined
|
||||
|
||||
try {
|
||||
const tddEnabled = executorCtx.sisyphusAgentConfig?.tdd
|
||||
const delegatedPromptText = buildTaskPrompt(args.prompt, agentToUse, tddEnabled)
|
||||
|
||||
if (typeof manager?.reserveSubagentSpawn === "function") {
|
||||
spawnReservation = await manager.reserveSubagentSpawn(parentContext.sessionID)
|
||||
}
|
||||
@@ -106,11 +114,13 @@ export async function executeSyncTask(
|
||||
subagentSessions.add(newSessionID)
|
||||
syncSubagentSessions.add(newSessionID)
|
||||
setSessionAgent(newSessionID, agentToUse)
|
||||
executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(newSessionID, fallbackChain)
|
||||
|
||||
if (args.category) {
|
||||
SessionCategoryRegistry.register(newSessionID, args.category)
|
||||
}
|
||||
registerDelegatedChildSessionBootstrap({
|
||||
sessionID: newSessionID,
|
||||
promptText: delegatedPromptText,
|
||||
fallbackChain,
|
||||
category: args.category,
|
||||
modelFallbackControllerAccessor: executorCtx.modelFallbackControllerAccessor,
|
||||
})
|
||||
|
||||
if (onSyncSessionCreated) {
|
||||
log("[task] Invoking onSyncSessionCreated callback", { sessionID: newSessionID, parentID: parentContext.sessionID })
|
||||
@@ -176,6 +186,7 @@ export async function executeSyncTask(
|
||||
sessionID,
|
||||
agentToUse,
|
||||
args,
|
||||
promptText: delegatedPromptText,
|
||||
systemContent,
|
||||
directory: createSessionResult.parentDirectory,
|
||||
toastManager,
|
||||
@@ -198,6 +209,7 @@ export async function executeSyncTask(
|
||||
const cleanupRetrySession = (currentSessionID: string): void => {
|
||||
subagentSessions.delete(currentSessionID)
|
||||
syncSubagentSessions.delete(currentSessionID)
|
||||
clearDelegatedChildSessionBootstrap(currentSessionID)
|
||||
executorCtx.modelFallbackControllerAccessor?.clearSessionFallbackChain(currentSessionID)
|
||||
SessionCategoryRegistry.remove(currentSessionID)
|
||||
}
|
||||
@@ -362,6 +374,7 @@ ${buildTaskMetadataBlock({
|
||||
if (syncSessionID) {
|
||||
subagentSessions.delete(syncSessionID)
|
||||
syncSubagentSessions.delete(syncSessionID)
|
||||
clearDelegatedChildSessionBootstrap(syncSessionID)
|
||||
executorCtx.modelFallbackControllerAccessor?.clearSessionFallbackChain(syncSessionID)
|
||||
SessionCategoryRegistry.remove(syncSessionID)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user