fix(delegate-task): replay sync retry session registration
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -15,7 +15,6 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
let resetToastManager: (() => void) | null = null
|
let resetToastManager: (() => void) | null = null
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
//#given - configure fast timing for all tests
|
|
||||||
const { __setTimingConfig } = require("./timing")
|
const { __setTimingConfig } = require("./timing")
|
||||||
__setTimingConfig({
|
__setTimingConfig({
|
||||||
POLL_INTERVAL_MS: 10,
|
POLL_INTERVAL_MS: 10,
|
||||||
@@ -24,7 +23,6 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
MAX_POLL_TIME_MS: 100,
|
MAX_POLL_TIME_MS: 100,
|
||||||
})
|
})
|
||||||
|
|
||||||
//#given - reset call tracking
|
|
||||||
removeTaskCalls = []
|
removeTaskCalls = []
|
||||||
addTaskCalls = []
|
addTaskCalls = []
|
||||||
deleteCalls = []
|
deleteCalls = []
|
||||||
@@ -32,7 +30,6 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
|
|
||||||
clearRequireCache("./sync-task")
|
clearRequireCache("./sync-task")
|
||||||
|
|
||||||
//#given - initialize real task toast manager (avoid global module mocks)
|
|
||||||
const { initTaskToastManager, _resetTaskToastManagerForTesting } = require("../../features/task-toast-manager/manager")
|
const { initTaskToastManager, _resetTaskToastManagerForTesting } = require("../../features/task-toast-manager/manager")
|
||||||
_resetTaskToastManagerForTesting()
|
_resetTaskToastManagerForTesting()
|
||||||
resetToastManager = _resetTaskToastManagerForTesting
|
resetToastManager = _resetTaskToastManagerForTesting
|
||||||
@@ -48,7 +45,6 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
removeTaskCalls.push(id)
|
removeTaskCalls.push(id)
|
||||||
})
|
})
|
||||||
|
|
||||||
//#given - mock subagentSessions
|
|
||||||
const { subagentSessions } = require("../../features/claude-code-session-state")
|
const { subagentSessions } = require("../../features/claude-code-session-state")
|
||||||
spyOn(subagentSessions, "add").mockImplementation((id: string) => {
|
spyOn(subagentSessions, "add").mockImplementation((id: string) => {
|
||||||
addCalls.push(id)
|
addCalls.push(id)
|
||||||
@@ -60,7 +56,6 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
//#given - reset timing after each test
|
|
||||||
const { __resetTimingConfig } = require("./timing")
|
const { __resetTimingConfig } = require("./timing")
|
||||||
__resetTimingConfig()
|
__resetTimingConfig()
|
||||||
|
|
||||||
@@ -516,11 +511,158 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
test("depth regression: blocks spawn when reserveSubagentSpawn throws depth limit error", async () => {
|
test("replays sync session side effects for retry-created sessions", async () => {
|
||||||
// This is a smoke test guarding against regressions where the depth limit
|
const mockClient = {
|
||||||
// would be silently bypassed (e.g. via a fallback path that hardcodes
|
session: {
|
||||||
// childDepth: 1).
|
create: async () => ({ data: { id: "ignored" } }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const { executeSyncTask } = require("./sync-task")
|
||||||
|
const createdSessions: string[] = []
|
||||||
|
const onSyncSessionCreated = mock(async (_event: unknown) => {})
|
||||||
|
|
||||||
|
const deps = {
|
||||||
|
createSyncSession: async () => {
|
||||||
|
const sessionID = createdSessions.length === 0 ? "ses_first" : "ses_second"
|
||||||
|
createdSessions.push(sessionID)
|
||||||
|
return { ok: true as const, sessionID }
|
||||||
|
},
|
||||||
|
sendSyncPrompt: async () => null,
|
||||||
|
pollSyncSession: async (_ctx: unknown, _client: unknown, input: { sessionID: string }) => {
|
||||||
|
return input.sessionID === "ses_first"
|
||||||
|
? "Forbidden: Selected provider is forbidden"
|
||||||
|
: null
|
||||||
|
},
|
||||||
|
fetchSyncResult: async (_client: unknown, sessionID: string) => ({ ok: true as const, textContent: `Result from ${sessionID}` }),
|
||||||
|
}
|
||||||
|
|
||||||
|
const metadataCalls: any[] = []
|
||||||
|
const mockCtx = {
|
||||||
|
sessionID: "parent-session",
|
||||||
|
callID: "call-123",
|
||||||
|
metadata: (input: any) => { metadataCalls.push(input) },
|
||||||
|
}
|
||||||
|
|
||||||
|
const mockExecutorCtx = {
|
||||||
|
client: mockClient,
|
||||||
|
directory: "/tmp",
|
||||||
|
onSyncSessionCreated,
|
||||||
|
modelFallbackControllerAccessor: {
|
||||||
|
setSessionFallbackChain: () => {},
|
||||||
|
clearSessionFallbackChain: () => {},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const args = {
|
||||||
|
prompt: "test prompt",
|
||||||
|
description: "test task",
|
||||||
|
category: "quick",
|
||||||
|
load_skills: [],
|
||||||
|
run_in_background: false,
|
||||||
|
command: null,
|
||||||
|
}
|
||||||
|
|
||||||
|
const initialModel = {
|
||||||
|
providerID: "genai-proxy-openai",
|
||||||
|
modelID: "gpt-5.4-mini",
|
||||||
|
variant: undefined,
|
||||||
|
}
|
||||||
|
const fallbackChain = [
|
||||||
|
{ providers: ["genai-proxy-openai"], model: "gpt-5.4-mini" },
|
||||||
|
{ providers: ["genai-proxy-aws"], model: "us.anthropic.claude-haiku-4-5-20251001-v1:0" },
|
||||||
|
]
|
||||||
|
|
||||||
|
const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, {
|
||||||
|
sessionID: "parent-session",
|
||||||
|
}, "sisyphus-junior", initialModel, undefined, undefined, fallbackChain, deps)
|
||||||
|
|
||||||
|
expect(result).toContain("Result from ses_second")
|
||||||
|
expect(onSyncSessionCreated.mock.calls.map((call: any[]) => call[0])).toEqual([
|
||||||
|
{ sessionID: "ses_first", parentID: "parent-session", title: "test task" },
|
||||||
|
{ sessionID: "ses_second", parentID: "parent-session", title: "test task" },
|
||||||
|
])
|
||||||
|
expect(addTaskCalls.map((task) => task.sessionID)).toEqual(["ses_first", "ses_second"])
|
||||||
|
expect(addTaskCalls.map((task) => task.id)).toEqual(["sync_ses_firs", "sync_ses_firs"])
|
||||||
|
})
|
||||||
|
|
||||||
|
test("publishes latest retry session metadata when final retry still fails", async () => {
|
||||||
|
const mockClient = {
|
||||||
|
session: {
|
||||||
|
create: async () => ({ data: { id: "ignored" } }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const { executeSyncTask } = require("./sync-task")
|
||||||
|
const createdSessions: string[] = []
|
||||||
|
|
||||||
|
const deps = {
|
||||||
|
createSyncSession: async () => {
|
||||||
|
const sessionID = createdSessions.length === 0 ? "ses_first" : "ses_second"
|
||||||
|
createdSessions.push(sessionID)
|
||||||
|
return { ok: true as const, sessionID }
|
||||||
|
},
|
||||||
|
sendSyncPrompt: async () => null,
|
||||||
|
pollSyncSession: async (_ctx: unknown, _client: unknown, input: { sessionID: string }) => {
|
||||||
|
return input.sessionID === "ses_first"
|
||||||
|
? "Forbidden: Selected provider is forbidden"
|
||||||
|
: "Final retry failed"
|
||||||
|
},
|
||||||
|
fetchSyncResult: async () => ({ ok: true as const, textContent: "unused" }),
|
||||||
|
}
|
||||||
|
|
||||||
|
const metadataCalls: any[] = []
|
||||||
|
const mockCtx = {
|
||||||
|
sessionID: "parent-session",
|
||||||
|
callID: "call-123",
|
||||||
|
metadata: (input: any) => { metadataCalls.push(input) },
|
||||||
|
}
|
||||||
|
|
||||||
|
const mockExecutorCtx = {
|
||||||
|
client: mockClient,
|
||||||
|
directory: "/tmp",
|
||||||
|
onSyncSessionCreated: null,
|
||||||
|
modelFallbackControllerAccessor: {
|
||||||
|
setSessionFallbackChain: () => {},
|
||||||
|
clearSessionFallbackChain: () => {},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const args = {
|
||||||
|
prompt: "test prompt",
|
||||||
|
description: "test task",
|
||||||
|
category: "quick",
|
||||||
|
load_skills: [],
|
||||||
|
run_in_background: false,
|
||||||
|
command: null,
|
||||||
|
}
|
||||||
|
|
||||||
|
const initialModel = {
|
||||||
|
providerID: "genai-proxy-openai",
|
||||||
|
modelID: "gpt-5.4-mini",
|
||||||
|
variant: undefined,
|
||||||
|
}
|
||||||
|
const fallbackChain = [
|
||||||
|
{ providers: ["genai-proxy-openai"], model: "gpt-5.4-mini" },
|
||||||
|
{ providers: ["genai-proxy-aws"], model: "us.anthropic.claude-haiku-4-5-20251001-v1:0" },
|
||||||
|
]
|
||||||
|
|
||||||
|
const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, {
|
||||||
|
sessionID: "parent-session",
|
||||||
|
}, "sisyphus-junior", initialModel, undefined, undefined, fallbackChain, deps)
|
||||||
|
|
||||||
|
expect(result).toBe("Final retry failed")
|
||||||
|
const finalMetadata = metadataCalls.at(-1)
|
||||||
|
expect(finalMetadata.metadata.sessionId).toBe("ses_second")
|
||||||
|
expect(finalMetadata.metadata.taskId).toBe("ses_second")
|
||||||
|
expect(finalMetadata.metadata.model).toEqual({
|
||||||
|
providerID: "genai-proxy-aws",
|
||||||
|
modelID: "us.anthropic.claude-haiku-4-5-20251001-v1:0",
|
||||||
|
variant: undefined,
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test("depth regression: blocks spawn when reserveSubagentSpawn throws depth limit error", async () => {
|
||||||
const mockClient = {
|
const mockClient = {
|
||||||
session: {
|
session: {
|
||||||
create: async () => ({ data: { id: "ses_test_12345678" } }),
|
create: async () => ({ data: { id: "ses_test_12345678" } }),
|
||||||
@@ -574,17 +716,10 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
expect(result).toContain("child depth 4")
|
expect(result).toContain("child depth 4")
|
||||||
expect(result).toContain("maxDepth=3")
|
expect(result).toContain("maxDepth=3")
|
||||||
expect(reserveSubagentSpawn).toHaveBeenCalledWith("parent-session")
|
expect(reserveSubagentSpawn).toHaveBeenCalledWith("parent-session")
|
||||||
// critical: createSyncSession must NOT have been called -- if it was,
|
|
||||||
// the depth guard was bypassed.
|
|
||||||
expect(addCalls.length).toBe(0)
|
expect(addCalls.length).toBe(0)
|
||||||
})
|
})
|
||||||
|
|
||||||
test("depth regression: does not silently fall back to childDepth: 1 when manager methods are present", async () => {
|
test("depth regression: does not silently fall back to childDepth: 1 when manager methods are present", async () => {
|
||||||
// Guards against the dangerous fallback path in sync-task.ts that
|
|
||||||
// hardcodes childDepth: 1 if reserveSubagentSpawn / assertCanSpawn are
|
|
||||||
// not functions. With a real manager present, the fallback must NOT be
|
|
||||||
// taken.
|
|
||||||
|
|
||||||
const mockClient = {
|
const mockClient = {
|
||||||
session: {
|
session: {
|
||||||
create: async () => ({ data: { id: "ses_test_12345678" } }),
|
create: async () => ({ data: { id: "ses_test_12345678" } }),
|
||||||
|
|||||||
@@ -40,12 +40,7 @@ export async function executeSyncTask(
|
|||||||
spawnReservation = await manager.reserveSubagentSpawn(parentContext.sessionID)
|
spawnReservation = await manager.reserveSubagentSpawn(parentContext.sessionID)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Depth guard. We must NOT silently fall back to childDepth: 1
|
// Only default to childDepth: 1 for legacy managers that cannot enforce spawn depth.
|
||||||
// when the manager is unavailable or lacks the spawn methods, because that
|
|
||||||
// would let subagents recurse without bound. The only safe fallback is
|
|
||||||
// when the manager genuinely cannot enforce limits (legacy SDK), in which
|
|
||||||
// case we still record childDepth: 1 but log a warning so regressions are
|
|
||||||
// visible.
|
|
||||||
let spawnContext: { rootSessionID: string; parentDepth: number; childDepth: number }
|
let spawnContext: { rootSessionID: string; parentDepth: number; childDepth: number }
|
||||||
if (spawnReservation?.spawnContext) {
|
if (spawnReservation?.spawnContext) {
|
||||||
spawnContext = spawnReservation.spawnContext
|
spawnContext = spawnReservation.spawnContext
|
||||||
@@ -79,29 +74,61 @@ export async function executeSyncTask(
|
|||||||
const sessionID = createSessionResult.sessionID
|
const sessionID = createSessionResult.sessionID
|
||||||
spawnReservation?.commit()
|
spawnReservation?.commit()
|
||||||
syncSessionID = sessionID
|
syncSessionID = sessionID
|
||||||
subagentSessions.add(sessionID)
|
|
||||||
syncSubagentSessions.add(sessionID)
|
|
||||||
setSessionAgent(sessionID, agentToUse)
|
|
||||||
executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionID, fallbackChain)
|
|
||||||
|
|
||||||
if (args.category) {
|
const registerSyncSession = async (newSessionID: string): Promise<void> => {
|
||||||
SessionCategoryRegistry.register(sessionID, args.category)
|
syncSessionID = newSessionID
|
||||||
}
|
subagentSessions.add(newSessionID)
|
||||||
|
syncSubagentSessions.add(newSessionID)
|
||||||
|
setSessionAgent(newSessionID, agentToUse)
|
||||||
|
executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(newSessionID, fallbackChain)
|
||||||
|
|
||||||
if (onSyncSessionCreated) {
|
if (args.category) {
|
||||||
log("[task] Invoking onSyncSessionCreated callback", { sessionID, parentID: parentContext.sessionID })
|
SessionCategoryRegistry.register(newSessionID, args.category)
|
||||||
try {
|
}
|
||||||
await onSyncSessionCreated({
|
|
||||||
sessionID,
|
if (onSyncSessionCreated) {
|
||||||
parentID: parentContext.sessionID,
|
log("[task] Invoking onSyncSessionCreated callback", { sessionID: newSessionID, parentID: parentContext.sessionID })
|
||||||
title: args.description,
|
try {
|
||||||
})
|
await onSyncSessionCreated({
|
||||||
} catch (error) {
|
sessionID: newSessionID,
|
||||||
log("[task] onSyncSessionCreated callback failed", { error: String(error) })
|
parentID: parentContext.sessionID,
|
||||||
|
title: args.description,
|
||||||
|
})
|
||||||
|
} catch (error) {
|
||||||
|
log("[task] onSyncSessionCreated callback failed", { error: String(error) })
|
||||||
|
}
|
||||||
|
await new Promise(r => setTimeout(r, 200))
|
||||||
}
|
}
|
||||||
await new Promise(r => setTimeout(r, 200))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const publishSyncMetadata = async (
|
||||||
|
currentSessionID: string,
|
||||||
|
currentModel: DelegatedModelConfig | undefined,
|
||||||
|
currentTaskId: string,
|
||||||
|
spawnDepth: number,
|
||||||
|
): Promise<void> => {
|
||||||
|
await publishToolMetadata(ctx, {
|
||||||
|
title: args.description,
|
||||||
|
metadata: {
|
||||||
|
prompt: args.prompt,
|
||||||
|
agent: agentToUse,
|
||||||
|
category: args.category,
|
||||||
|
...(args.requested_subagent_type !== undefined ? { requested_subagent_type: args.requested_subagent_type } : {}),
|
||||||
|
load_skills: args.load_skills,
|
||||||
|
description: args.description,
|
||||||
|
run_in_background: args.run_in_background,
|
||||||
|
taskId: currentSessionID,
|
||||||
|
sessionId: currentSessionID,
|
||||||
|
sync: true,
|
||||||
|
spawnDepth,
|
||||||
|
command: args.command,
|
||||||
|
model: resolveMetadataModel(currentModel, parentContext.model),
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
await registerSyncSession(sessionID)
|
||||||
|
|
||||||
taskId = `sync_${sessionID.slice(0, 8)}`
|
taskId = `sync_${sessionID.slice(0, 8)}`
|
||||||
const startTime = new Date()
|
const startTime = new Date()
|
||||||
|
|
||||||
@@ -117,26 +144,7 @@ export async function executeSyncTask(
|
|||||||
modelInfo,
|
modelInfo,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
await publishSyncMetadata(sessionID, categoryModel, taskId, spawnContext.childDepth)
|
||||||
const syncTaskMeta = {
|
|
||||||
title: args.description,
|
|
||||||
metadata: {
|
|
||||||
prompt: args.prompt,
|
|
||||||
agent: agentToUse,
|
|
||||||
category: args.category,
|
|
||||||
...(args.requested_subagent_type !== undefined ? { requested_subagent_type: args.requested_subagent_type } : {}),
|
|
||||||
load_skills: args.load_skills,
|
|
||||||
description: args.description,
|
|
||||||
run_in_background: args.run_in_background,
|
|
||||||
taskId: sessionID,
|
|
||||||
sessionId: sessionID,
|
|
||||||
sync: true,
|
|
||||||
spawnDepth: spawnContext.childDepth,
|
|
||||||
command: args.command,
|
|
||||||
model: resolveMetadataModel(categoryModel, parentContext.model),
|
|
||||||
},
|
|
||||||
}
|
|
||||||
await publishToolMetadata(ctx, syncTaskMeta)
|
|
||||||
|
|
||||||
const syncPromptInput = {
|
const syncPromptInput = {
|
||||||
sessionID,
|
sessionID,
|
||||||
@@ -225,17 +233,23 @@ export async function executeSyncTask(
|
|||||||
}
|
}
|
||||||
|
|
||||||
activeSessionID = retrySessionResult.sessionID
|
activeSessionID = retrySessionResult.sessionID
|
||||||
syncSessionID = retrySessionResult.sessionID
|
|
||||||
subagentSessions.add(activeSessionID)
|
|
||||||
syncSubagentSessions.add(activeSessionID)
|
|
||||||
setSessionAgent(activeSessionID, agentToUse)
|
|
||||||
executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(activeSessionID, fallbackChain)
|
|
||||||
|
|
||||||
if (args.category) {
|
|
||||||
SessionCategoryRegistry.register(activeSessionID, args.category)
|
|
||||||
}
|
|
||||||
|
|
||||||
effectiveCategoryModel = nextFallbackModel
|
effectiveCategoryModel = nextFallbackModel
|
||||||
|
await registerSyncSession(activeSessionID)
|
||||||
|
if (toastManager && taskId) {
|
||||||
|
toastManager.addTask({
|
||||||
|
id: taskId,
|
||||||
|
sessionID: activeSessionID,
|
||||||
|
description: args.description,
|
||||||
|
agent: agentToUse,
|
||||||
|
isBackground: false,
|
||||||
|
category: args.category,
|
||||||
|
skills: args.load_skills,
|
||||||
|
modelInfo,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
if (taskId) {
|
||||||
|
await publishSyncMetadata(activeSessionID, effectiveCategoryModel, taskId, spawnContext.childDepth)
|
||||||
|
}
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -246,7 +260,6 @@ export async function executeSyncTask(
|
|||||||
|
|
||||||
const duration = formatDuration(startTime)
|
const duration = formatDuration(startTime)
|
||||||
|
|
||||||
// 检测模型路由是否与父 session 不同,给用户可见的提示
|
|
||||||
const actualModelStr = effectiveCategoryModel
|
const actualModelStr = effectiveCategoryModel
|
||||||
? `${effectiveCategoryModel.providerID}/${effectiveCategoryModel.modelID}`
|
? `${effectiveCategoryModel.providerID}/${effectiveCategoryModel.modelID}`
|
||||||
: undefined
|
: undefined
|
||||||
@@ -260,24 +273,7 @@ export async function executeSyncTask(
|
|||||||
modelRoutingNote = `\nModel: ${actualModelStr}${args.category ? ` (category: ${args.category})` : ""}`
|
modelRoutingNote = `\nModel: ${actualModelStr}${args.category ? ` (category: ${args.category})` : ""}`
|
||||||
}
|
}
|
||||||
|
|
||||||
await publishToolMetadata(ctx, {
|
await publishSyncMetadata(activeSessionID, effectiveCategoryModel, taskId!, spawnContext.childDepth)
|
||||||
title: args.description,
|
|
||||||
metadata: {
|
|
||||||
prompt: args.prompt,
|
|
||||||
agent: agentToUse,
|
|
||||||
category: args.category,
|
|
||||||
...(args.requested_subagent_type !== undefined ? { requested_subagent_type: args.requested_subagent_type } : {}),
|
|
||||||
load_skills: args.load_skills,
|
|
||||||
description: args.description,
|
|
||||||
run_in_background: args.run_in_background,
|
|
||||||
taskId: activeSessionID,
|
|
||||||
sessionId: activeSessionID,
|
|
||||||
sync: true,
|
|
||||||
spawnDepth: spawnContext.childDepth,
|
|
||||||
command: args.command,
|
|
||||||
model: resolveMetadataModel(effectiveCategoryModel, parentContext.model),
|
|
||||||
},
|
|
||||||
})
|
|
||||||
|
|
||||||
return `Task completed in ${duration}.
|
return `Task completed in ${duration}.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user