diff --git a/src/agents/acp-spawn.test.ts b/src/agents/acp-spawn.test.ts index d1dbc39ac62..b31fa16dd11 100644 --- a/src/agents/acp-spawn.test.ts +++ b/src/agents/acp-spawn.test.ts @@ -99,12 +99,18 @@ const resolveAcpSpawnStreamLogPathSpy = vi.spyOn( const { spawnAcpDirect } = await import("./acp-spawn.js"); type SpawnRequest = Parameters[0]; type SpawnContext = Parameters[1]; +type SpawnResult = Awaited>; type AgentCallParams = { deliver?: boolean; channel?: string; to?: string; threadId?: string; }; +type CrossAgentWorkspaceFixture = { + workspaceRoot: string; + mainWorkspace: string; + targetWorkspace: string; +}; function replaceSpawnConfig(next: OpenClawConfig): void { const current = hoisted.state.cfg as Record; @@ -186,12 +192,68 @@ function createRequesterContext(overrides?: Partial): SpawnContext }; } +async function createCrossAgentWorkspaceFixture(options?: { + targetDirName?: string; + createTargetWorkspace?: boolean; +}): Promise { + const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acp-spawn-")); + const mainWorkspace = path.join(workspaceRoot, "main"); + const targetWorkspace = path.join(workspaceRoot, options?.targetDirName?.trim() || "claude-code"); + await fs.mkdir(mainWorkspace, { recursive: true }); + if (options?.createTargetWorkspace !== false) { + await fs.mkdir(targetWorkspace, { recursive: true }); + } + return { + workspaceRoot, + mainWorkspace, + targetWorkspace, + }; +} + +function configureCrossAgentWorkspaceSpawn(fixture: CrossAgentWorkspaceFixture): void { + replaceSpawnConfig({ + ...hoisted.state.cfg, + acp: { + ...hoisted.state.cfg.acp, + allowedAgents: ["codex", "claude-code"], + }, + agents: { + list: [ + { + id: "main", + default: true, + workspace: fixture.mainWorkspace, + }, + { + id: "claude-code", + workspace: fixture.targetWorkspace, + }, + ], + }, + }); +} + function findAgentGatewayCall(): { method?: string; params?: Record } | undefined { return hoisted.callGatewayMock.mock.calls .map((call: unknown[]) => call[0] as { method?: string; params?: Record }) .find((request) => request.method === "agent"); } +function expectFailedSpawn( + result: SpawnResult, + status?: "error" | "forbidden", +): Extract { + if (status) { + expect(result.status).toBe(status); + } else { + expect(result.status).not.toBe("accepted"); + } + if (result.status === "accepted") { + throw new Error("Expected ACP spawn to fail"); + } + return result; +} + function expectAgentGatewayCall(overrides: AgentCallParams): void { const agentCall = findAgentGatewayCall(); expect(agentCall?.params?.deliver).toBe(overrides.deliver); @@ -542,33 +604,9 @@ describe("spawnAcpDirect", () => { }); it("uses the target agent workspace for cross-agent ACP spawns when cwd is omitted", async () => { - const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acp-spawn-")); + const fixture = await createCrossAgentWorkspaceFixture(); try { - const mainWorkspace = path.join(workspaceRoot, "main"); - const targetWorkspace = path.join(workspaceRoot, "claude-code"); - await fs.mkdir(mainWorkspace, { recursive: true }); - await fs.mkdir(targetWorkspace, { recursive: true }); - - replaceSpawnConfig({ - ...hoisted.state.cfg, - acp: { - ...hoisted.state.cfg.acp, - allowedAgents: ["codex", "claude-code"], - }, - agents: { - list: [ - { - id: "main", - default: true, - workspace: mainWorkspace, - }, - { - id: "claude-code", - workspace: targetWorkspace, - }, - ], - }, - }); + configureCrossAgentWorkspaceSpawn(fixture); const result = await spawnAcpDirect( { @@ -586,41 +624,21 @@ describe("spawnAcpDirect", () => { expect.objectContaining({ sessionKey: expect.stringMatching(/^agent:claude-code:acp:/), agent: "claude-code", - cwd: targetWorkspace, + cwd: fixture.targetWorkspace, }), ); } finally { - await fs.rm(workspaceRoot, { recursive: true, force: true }); + await fs.rm(fixture.workspaceRoot, { recursive: true, force: true }); } }); it("falls back to backend default cwd when the inherited target workspace does not exist", async () => { - const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acp-spawn-")); + const fixture = await createCrossAgentWorkspaceFixture({ + targetDirName: "claude-code-missing", + createTargetWorkspace: false, + }); try { - const mainWorkspace = path.join(workspaceRoot, "main"); - const missingTargetWorkspace = path.join(workspaceRoot, "claude-code-missing"); - await fs.mkdir(mainWorkspace, { recursive: true }); - - replaceSpawnConfig({ - ...hoisted.state.cfg, - acp: { - ...hoisted.state.cfg.acp, - allowedAgents: ["codex", "claude-code"], - }, - agents: { - list: [ - { - id: "main", - default: true, - workspace: mainWorkspace, - }, - { - id: "claude-code", - workspace: missingTargetWorkspace, - }, - ], - }, - }); + configureCrossAgentWorkspaceSpawn(fixture); const result = await spawnAcpDirect( { @@ -642,39 +660,15 @@ describe("spawnAcpDirect", () => { }), ); } finally { - await fs.rm(workspaceRoot, { recursive: true, force: true }); + await fs.rm(fixture.workspaceRoot, { recursive: true, force: true }); } }); it("surfaces non-missing target workspace access failures instead of silently dropping cwd", async () => { - const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acp-spawn-")); + const fixture = await createCrossAgentWorkspaceFixture(); const accessSpy = vi.spyOn(fs, "access"); try { - const mainWorkspace = path.join(workspaceRoot, "main"); - const targetWorkspace = path.join(workspaceRoot, "claude-code"); - await fs.mkdir(mainWorkspace, { recursive: true }); - await fs.mkdir(targetWorkspace, { recursive: true }); - - replaceSpawnConfig({ - ...hoisted.state.cfg, - acp: { - ...hoisted.state.cfg.acp, - allowedAgents: ["codex", "claude-code"], - }, - agents: { - list: [ - { - id: "main", - default: true, - workspace: mainWorkspace, - }, - { - id: "claude-code", - workspace: targetWorkspace, - }, - ], - }, - }); + configureCrossAgentWorkspaceSpawn(fixture); accessSpy.mockRejectedValueOnce( Object.assign(new Error("permission denied"), { code: "EACCES" }), @@ -693,12 +687,13 @@ describe("spawnAcpDirect", () => { expect(result).toEqual({ status: "error", + errorCode: "cwd_resolution_failed", error: "permission denied", }); expect(hoisted.initializeSessionMock).not.toHaveBeenCalled(); } finally { accessSpy.mockRestore(); - await fs.rm(workspaceRoot, { recursive: true, force: true }); + await fs.rm(fixture.workspaceRoot, { recursive: true, force: true }); } }); @@ -1063,8 +1058,7 @@ describe("spawnAcpDirect", () => { }, ); - expect(result.status).toBe("error"); - expect(result.error).toContain("set `acp.defaultAgent`"); + expect(expectFailedSpawn(result, "error").error).toContain("set `acp.defaultAgent`"); }); it("fails fast when Discord ACP thread spawn is disabled", async () => { @@ -1094,8 +1088,7 @@ describe("spawnAcpDirect", () => { }, ); - expect(result.status).toBe("error"); - expect(result.error).toContain("spawnAcpSessions=true"); + expect(expectFailedSpawn(result, "error").error).toContain("spawnAcpSessions=true"); }); it("forbids ACP spawn from sandboxed requester sessions", async () => { @@ -1118,8 +1111,9 @@ describe("spawnAcpDirect", () => { }, ); - expect(result.status).toBe("forbidden"); - expect(result.error).toContain("Sandboxed sessions cannot spawn ACP sessions"); + expect(expectFailedSpawn(result, "forbidden").error).toContain( + "Sandboxed sessions cannot spawn ACP sessions", + ); expect(hoisted.callGatewayMock).not.toHaveBeenCalled(); expect(hoisted.initializeSessionMock).not.toHaveBeenCalled(); }); @@ -1136,8 +1130,7 @@ describe("spawnAcpDirect", () => { }, ); - expect(result.status).toBe("forbidden"); - expect(result.error).toContain('sandbox="require"'); + expect(expectFailedSpawn(result, "forbidden").error).toContain('sandbox="require"'); expect(hoisted.callGatewayMock).not.toHaveBeenCalled(); expect(hoisted.initializeSessionMock).not.toHaveBeenCalled(); }); @@ -1653,8 +1646,7 @@ describe("spawnAcpDirect", () => { }, ); - expect(result.status).toBe("error"); - expect(result.error).toContain("agent dispatch failed"); + expect(expectFailedSpawn(result, "error").error).toContain("agent dispatch failed"); expect(relayHandle.dispose).toHaveBeenCalledTimes(1); expect(relayHandle.notifyStarted).not.toHaveBeenCalled(); }); @@ -1673,8 +1665,7 @@ describe("spawnAcpDirect", () => { }, ); - expect(result.status).toBe("error"); - expect(result.error).toContain('streamTo="parent"'); + expect(expectFailedSpawn(result, "error").error).toContain('streamTo="parent"'); expect(hoisted.callGatewayMock).not.toHaveBeenCalled(); expect(hoisted.startAcpSpawnParentStreamRelayMock).not.toHaveBeenCalled(); }); diff --git a/src/agents/acp-spawn.ts b/src/agents/acp-spawn.ts index bb7e8814625..e28297a094a 100644 --- a/src/agents/acp-spawn.ts +++ b/src/agents/acp-spawn.ts @@ -94,16 +94,38 @@ export type SpawnAcpContext = { sandboxed?: boolean; }; -export type SpawnAcpResult = { - status: "accepted" | "forbidden" | "error"; +export const ACP_SPAWN_ERROR_CODES = [ + "acp_disabled", + "requester_session_required", + "runtime_policy", + "thread_required", + "target_agent_required", + "agent_forbidden", + "cwd_resolution_failed", + "thread_binding_invalid", + "spawn_failed", + "dispatch_failed", +] as const; +export type SpawnAcpErrorCode = (typeof ACP_SPAWN_ERROR_CODES)[number]; + +type SpawnAcpAcceptedResult = { + status: "accepted"; childSessionKey?: string; runId?: string; mode?: SpawnAcpMode; streamLogPath?: string; note?: string; - error?: string; }; +type SpawnAcpFailedResult = { + status: "forbidden" | "error"; + childSessionKey?: string; + error: string; + errorCode: SpawnAcpErrorCode; +}; + +export type SpawnAcpResult = SpawnAcpAcceptedResult | SpawnAcpFailedResult; + export const ACP_SPAWN_ACCEPTED_NOTE = "initial ACP task queued in isolated session; follow-ups continue in the bound thread."; export const ACP_SPAWN_SESSION_ACCEPTED_NOTE = @@ -369,6 +391,25 @@ function summarizeError(err: unknown): string { return "error"; } +function createAcpSpawnFailure(params: { + status: "forbidden" | "error"; + errorCode: SpawnAcpErrorCode; + error: string; + childSessionKey?: string; +}): SpawnAcpFailedResult { + return { + status: params.status, + errorCode: params.errorCode, + error: params.error, + ...(params.childSessionKey ? { childSessionKey: params.childSessionKey } : {}), + }; +} + +function isMissingPathError(error: unknown): boolean { + const code = error instanceof Error ? (error as NodeJS.ErrnoException).code : undefined; + return code === "ENOENT" || code === "ENOTDIR"; +} + async function resolveRuntimeCwdForAcpSpawn(params: { resolvedCwd?: string; explicitCwd?: string; @@ -383,8 +424,7 @@ async function resolveRuntimeCwdForAcpSpawn(params: { await fs.access(params.resolvedCwd); return params.resolvedCwd; } catch (error) { - const code = error instanceof Error ? (error as NodeJS.ErrnoException).code : undefined; - if (code === "ENOENT" || code === "ENOTDIR") { + if (isMissingPathError(error)) { return undefined; } throw error; @@ -870,18 +910,20 @@ export async function spawnAcpDirect( requesterSessionKey: ctx.agentSessionKey, }); if (!isAcpEnabledByPolicy(cfg)) { - return { + return createAcpSpawnFailure({ status: "forbidden", + errorCode: "acp_disabled", error: "ACP is disabled by policy (`acp.enabled=false`).", - }; + }); } const streamToParentRequested = params.streamTo === "parent"; const parentSessionKey = ctx.agentSessionKey?.trim(); if (streamToParentRequested && !parentSessionKey) { - return { + return createAcpSpawnFailure({ status: "error", + errorCode: "requester_session_required", error: 'sessions_spawn streamTo="parent" requires an active requester session context.', - }; + }); } let requestThreadBinding = params.thread === true; @@ -892,10 +934,11 @@ export async function spawnAcpDirect( sandbox: params.sandbox, }); if (runtimePolicyError) { - return { + return createAcpSpawnFailure({ status: "forbidden", + errorCode: "runtime_policy", error: runtimePolicyError, - }; + }); } const spawnMode = resolveSpawnMode({ @@ -903,10 +946,11 @@ export async function spawnAcpDirect( threadRequested: requestThreadBinding, }); if (spawnMode === "session" && !requestThreadBinding) { - return { + return createAcpSpawnFailure({ status: "error", + errorCode: "thread_required", error: 'mode="session" requires thread=true so the ACP session can stay bound to a thread.', - }; + }); } const requesterState = resolveAcpSpawnRequesterState({ @@ -926,18 +970,20 @@ export async function spawnAcpDirect( cfg, }); if (!targetAgentResult.ok) { - return { + return createAcpSpawnFailure({ status: "error", + errorCode: "target_agent_required", error: targetAgentResult.error, - }; + }); } const targetAgentId = targetAgentResult.agentId; const agentPolicyError = resolveAcpAgentPolicyError(cfg, targetAgentId); if (agentPolicyError) { - return { + return createAcpSpawnFailure({ status: "forbidden", + errorCode: "agent_forbidden", error: agentPolicyError.message, - }; + }); } const sessionKey = `agent:${targetAgentId}:acp:${crypto.randomUUID()}`; @@ -948,10 +994,19 @@ export async function spawnAcpDirect( requesterSessionKey: ctx.agentSessionKey, explicitWorkspaceDir: params.cwd, }); - const runtimeCwd = await resolveRuntimeCwdForAcpSpawn({ - resolvedCwd, - explicitCwd: params.cwd, - }); + let runtimeCwd: string | undefined; + try { + runtimeCwd = await resolveRuntimeCwdForAcpSpawn({ + resolvedCwd, + explicitCwd: params.cwd, + }); + } catch (error) { + return createAcpSpawnFailure({ + status: "error", + errorCode: "cwd_resolution_failed", + error: summarizeError(error), + }); + } let preparedBinding: PreparedAcpThreadBinding | null = null; if (requestThreadBinding) { @@ -964,10 +1019,11 @@ export async function spawnAcpDirect( groupId: ctx.agentGroupId, }); if (!prepared.ok) { - return { + return createAcpSpawnFailure({ status: "error", + errorCode: "thread_binding_invalid", error: prepared.error, - }; + }); } preparedBinding = prepared.binding; } @@ -1014,10 +1070,11 @@ export async function spawnAcpDirect( deleteTranscript: true, runtimeCloseHandle: initializedRuntime, }); - return { + return createAcpSpawnFailure({ status: "error", + errorCode: isSessionBindingError(err) ? "thread_binding_invalid" : "spawn_failed", error: isSessionBindingError(err) ? err.message : summarizeError(err), - }; + }); } const deliveryPlan = resolveAcpSpawnBootstrapDeliveryPlan({ @@ -1075,11 +1132,12 @@ export async function spawnAcpDirect( shouldDeleteSession: true, deleteTranscript: true, }); - return { + return createAcpSpawnFailure({ status: "error", + errorCode: "dispatch_failed", error: summarizeError(err), childSessionKey: sessionKey, - }; + }); } if (effectiveStreamToParent && parentSessionKey) {