From 4277078bc515914c48190721897a8fba92ed37c7 Mon Sep 17 00:00:00 2001 From: "B.K." Date: Mon, 20 Apr 2026 01:31:31 +0300 Subject: [PATCH] fix(subagent): include role, session key, and timing in error payloads (#68726) Merged via squash. Prepared head SHA: 55c756142f29ec707f80e186d2f6b98f9b8a4665 Co-authored-by: BKF-Gitty <263413630+BKF-Gitty@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + src/agents/subagent-announce-output.ts | 51 ++++++++++--- src/agents/subagent-announce.test.ts | 26 +++++++ .../subagent-registry-completion.test.ts | 33 ++++++++ src/agents/subagent-registry-completion.ts | 26 ++++++- src/agents/subagent-registry-helpers.test.ts | 54 +++++++++++++ src/agents/subagent-registry-helpers.ts | 20 +++-- .../subagent-registry-lifecycle.test.ts | 76 +++++++++++++++++-- src/agents/subagent-registry-lifecycle.ts | 13 +++- src/agents/subagent-registry-run-manager.ts | 23 ++++-- src/agents/subagent-registry.test.ts | 18 ++++- src/agents/tools/sessions-spawn-tool.test.ts | 65 ++++++++++++++++ src/agents/tools/sessions-spawn-tool.ts | 20 ++++- 13 files changed, 390 insertions(+), 36 deletions(-) create mode 100644 src/agents/subagent-registry-helpers.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d8325bf4a0..fd60b682b8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai - Matrix/allowlists: hot-reload `dm.allowFrom` and `groupAllowFrom` entries on inbound messages while keeping config removals authoritative, so Matrix allowlist changes no longer require a channel restart to add or revoke a sender. (#68546) Thanks @johnlanni. - BlueBubbles: always set `method` explicitly on outbound text sends (`"private-api"` when available, `"apple-script"` otherwise), and prefer Private API on macOS 26 even for plain text. Fixes silent delivery failure on macOS setups without Private API where an omitted `method` let BB Server fall back to version-dependent default behavior that silently drops the message (#64480), and the AppleScript `-1700` error on macOS 26 Tahoe plain text sends (#53159). (#69070) Thanks @xqing3. - Matrix/commands: recognize slash commands that are prefixed with the bot's Matrix mention, so room messages like `@bot:server /new` trigger the command path without requiring custom mention regexes. (#68570) Thanks @nightq and @johnlanni. +- Agents/subagents: include requested role and runtime timing on subagent failure payloads so parent agents can correlate failed or timed-out child work. (#68726) Thanks @BKF-Gitty. ## 2026.4.19-beta.2 diff --git a/src/agents/subagent-announce-output.ts b/src/agents/subagent-announce-output.ts index b40b105cfb2..9f55f491719 100644 --- a/src/agents/subagent-announce-output.ts +++ b/src/agents/subagent-announce-output.ts @@ -58,8 +58,37 @@ export type AgentWaitResult = { export type SubagentRunOutcome = { status: "ok" | "error" | "timeout" | "unknown"; error?: string; + startedAt?: number; + endedAt?: number; + elapsedMs?: number; }; +function readFiniteNumber(value: number | undefined): number | undefined { + return typeof value === "number" && Number.isFinite(value) ? value : undefined; +} + +export function withSubagentOutcomeTiming( + outcome: SubagentRunOutcome, + timing: { + startedAt?: number; + endedAt?: number; + }, +): SubagentRunOutcome { + const startedAt = readFiniteNumber(timing.startedAt) ?? readFiniteNumber(outcome.startedAt); + const endedAt = readFiniteNumber(timing.endedAt) ?? readFiniteNumber(outcome.endedAt); + const nextTiming: Pick = {}; + if (typeof startedAt === "number") { + nextTiming.startedAt = startedAt; + } + if (typeof endedAt === "number") { + nextTiming.endedAt = endedAt; + } + if (typeof startedAt === "number" && typeof endedAt === "number") { + nextTiming.elapsedMs = Math.max(0, endedAt - startedAt); + } + return { ...outcome, ...nextTiming }; +} + function extractToolResultText(content: unknown): string { if (typeof content === "string") { return sanitizeTextContent(content); @@ -297,20 +326,22 @@ export function applySubagentWaitOutcome(params: { startedAt: params.startedAt, endedAt: params.endedAt, }; - const waitError = typeof params.wait?.error === "string" ? params.wait.error : undefined; - if (params.wait?.status === "timeout") { - next.outcome = { status: "timeout" }; - } else if (params.wait?.status === "error") { - next.outcome = { status: "error", error: waitError }; - } else if (params.wait?.status === "ok") { - next.outcome = { status: "ok" }; - } - if (typeof params.wait?.startedAt === "number" && !next.startedAt) { + if (typeof params.wait?.startedAt === "number" && typeof next.startedAt !== "number") { next.startedAt = params.wait.startedAt; } - if (typeof params.wait?.endedAt === "number" && !next.endedAt) { + if (typeof params.wait?.endedAt === "number" && typeof next.endedAt !== "number") { next.endedAt = params.wait.endedAt; } + const waitError = typeof params.wait?.error === "string" ? params.wait.error : undefined; + let outcome = next.outcome; + if (params.wait?.status === "timeout") { + outcome = { status: "timeout" }; + } else if (params.wait?.status === "error") { + outcome = { status: "error", error: waitError }; + } else if (params.wait?.status === "ok") { + outcome = { status: "ok" }; + } + next.outcome = outcome ? withSubagentOutcomeTiming(outcome, next) : undefined; return next; } diff --git a/src/agents/subagent-announce.test.ts b/src/agents/subagent-announce.test.ts index 448fa3d3dbf..838cb1eb859 100644 --- a/src/agents/subagent-announce.test.ts +++ b/src/agents/subagent-announce.test.ts @@ -168,8 +168,34 @@ vi.mock("./subagent-announce-delivery.js", () => ({ })); vi.mock("./subagent-announce.registry.runtime.js", () => subagentRegistryRuntimeMock); +import { applySubagentWaitOutcome } from "./subagent-announce-output.js"; import { runSubagentAnnounceFlow } from "./subagent-announce.js"; +describe("subagent wait outcome timing", () => { + it.each([ + { wait: { status: "ok" }, expected: { status: "ok" } }, + { wait: { status: "timeout" }, expected: { status: "timeout" } }, + { + wait: { status: "error", error: "boom" }, + expected: { status: "error", error: "boom" }, + }, + ] as const)("adds timing to $wait.status outcomes", ({ wait, expected }) => { + const result = applySubagentWaitOutcome({ + wait, + outcome: undefined, + startedAt: 1_000, + endedAt: 1_250, + }); + + expect(result.outcome).toEqual({ + ...expected, + startedAt: 1_000, + endedAt: 1_250, + elapsedMs: 250, + }); + }); +}); + describe("subagent announce seam flow", () => { beforeEach(() => { agentSpy.mockClear(); diff --git a/src/agents/subagent-registry-completion.test.ts b/src/agents/subagent-registry-completion.test.ts index e171f230d9d..28b0fa98c10 100644 --- a/src/agents/subagent-registry-completion.test.ts +++ b/src/agents/subagent-registry-completion.test.ts @@ -49,6 +49,39 @@ describe("emitSubagentEndedHookOnce", () => { lifecycleMocks.runSubagentEnded.mockClear(); }); + it("treats timing differences as different only after both outcomes have timing", () => { + expect( + mod.runOutcomesEqual( + { status: "timeout", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, + { status: "timeout", startedAt: 1_000, endedAt: 2_500, elapsedMs: 1_500 }, + ), + ).toBe(false); + expect( + mod.runOutcomesEqual( + { status: "error", error: "boom", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, + { status: "error", error: "boom", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, + ), + ).toBe(true); + expect( + mod.runOutcomesEqual( + { status: "ok", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, + { status: "ok" }, + ), + ).toBe(true); + expect( + mod.shouldUpdateRunOutcome( + { status: "ok" }, + { status: "ok", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, + ), + ).toBe(true); + expect( + mod.shouldUpdateRunOutcome( + { status: "ok", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, + { status: "ok" }, + ), + ).toBe(false); + }); + it("records ended hook marker even when no subagent_ended hooks are registered", async () => { lifecycleMocks.getGlobalHookRunner.mockReturnValue({ hasHooks: () => false, diff --git a/src/agents/subagent-registry-completion.ts b/src/agents/subagent-registry-completion.ts index c3c7e02b507..da3ad3999b9 100644 --- a/src/agents/subagent-registry-completion.ts +++ b/src/agents/subagent-registry-completion.ts @@ -24,9 +24,31 @@ export function runOutcomesEqual( return false; } if (a.status === "error" && b.status === "error") { - return (a.error ?? "") === (b.error ?? ""); + if ((a.error ?? "") !== (b.error ?? "")) { + return false; + } } - return true; + if (!runOutcomeHasTiming(a) || !runOutcomeHasTiming(b)) { + return true; + } + return a.startedAt === b.startedAt && a.endedAt === b.endedAt && a.elapsedMs === b.elapsedMs; +} + +export function runOutcomeHasTiming(outcome: SubagentRunOutcome | undefined): boolean { + return ( + Number.isFinite(outcome?.startedAt) || + Number.isFinite(outcome?.endedAt) || + Number.isFinite(outcome?.elapsedMs) + ); +} + +export function shouldUpdateRunOutcome( + current: SubagentRunOutcome | undefined, + next: SubagentRunOutcome | undefined, +): boolean { + return ( + !runOutcomesEqual(current, next) || (!runOutcomeHasTiming(current) && runOutcomeHasTiming(next)) + ); } export function resolveLifecycleOutcomeFromRunOutcome( diff --git a/src/agents/subagent-registry-helpers.test.ts b/src/agents/subagent-registry-helpers.test.ts new file mode 100644 index 00000000000..25512b5a887 --- /dev/null +++ b/src/agents/subagent-registry-helpers.test.ts @@ -0,0 +1,54 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { reconcileOrphanedRun } from "./subagent-registry-helpers.js"; +import type { SubagentRunRecord } from "./subagent-registry.types.js"; + +function createRunEntry(overrides: Partial = {}): SubagentRunRecord { + return { + runId: "run-1", + childSessionKey: "agent:main:subagent:child", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "finish the task", + cleanup: "keep", + retainAttachmentsOnKeep: true, + createdAt: 500, + startedAt: 1_000, + ...overrides, + }; +} + +describe("reconcileOrphanedRun", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("preserves timing on orphaned error outcomes", () => { + vi.useFakeTimers(); + vi.setSystemTime(4_000); + const entry = createRunEntry(); + const runs = new Map([[entry.runId, entry]]); + const resumedRuns = new Set([entry.runId]); + + expect( + reconcileOrphanedRun({ + runId: entry.runId, + entry, + reason: "missing-session-id", + source: "resume", + runs, + resumedRuns, + }), + ).toBe(true); + + expect(entry.endedAt).toBe(4_000); + expect(entry.outcome).toEqual({ + status: "error", + error: "orphaned subagent run (missing-session-id)", + startedAt: 1_000, + endedAt: 4_000, + elapsedMs: 3_000, + }); + expect(runs.has(entry.runId)).toBe(false); + expect(resumedRuns.has(entry.runId)).toBe(false); + }); +}); diff --git a/src/agents/subagent-registry-helpers.ts b/src/agents/subagent-registry-helpers.ts index cd45e87c682..71aa8484749 100644 --- a/src/agents/subagent-registry-helpers.ts +++ b/src/agents/subagent-registry-helpers.ts @@ -11,9 +11,9 @@ import { import type { OpenClawConfig } from "../config/types.openclaw.js"; import { defaultRuntime } from "../runtime.js"; import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; -import { type SubagentRunOutcome } from "./subagent-announce-output.js"; +import { withSubagentOutcomeTiming } from "./subagent-announce-output.js"; import { SUBAGENT_ENDED_REASON_ERROR } from "./subagent-lifecycle-events.js"; -import { runOutcomesEqual } from "./subagent-registry-completion.js"; +import { shouldUpdateRunOutcome } from "./subagent-registry-completion.js"; import type { SubagentRunRecord } from "./subagent-registry.types.js"; import { getSubagentSessionRuntimeMs, @@ -219,11 +219,17 @@ export function reconcileOrphanedRun(params: { params.entry.endedAt = now; changed = true; } - const orphanOutcome: SubagentRunOutcome = { - status: "error", - error: `orphaned subagent run (${params.reason})`, - }; - if (!runOutcomesEqual(params.entry.outcome, orphanOutcome)) { + const orphanOutcome = withSubagentOutcomeTiming( + { + status: "error", + error: `orphaned subagent run (${params.reason})`, + }, + { + startedAt: params.entry.startedAt, + endedAt: params.entry.endedAt, + }, + ); + if (shouldUpdateRunOutcome(params.entry.outcome, orphanOutcome)) { params.entry.outcome = orphanOutcome; changed = true; } diff --git a/src/agents/subagent-registry-lifecycle.test.ts b/src/agents/subagent-registry-lifecycle.test.ts index b44ef3579ba..617ab6e7de6 100644 --- a/src/agents/subagent-registry-lifecycle.test.ts +++ b/src/agents/subagent-registry-lifecycle.test.ts @@ -62,11 +62,6 @@ vi.mock("./subagent-registry-cleanup.js", () => ({ resolveDeferredCleanupDecision: () => ({ kind: "give-up", reason: "retry-limit" }), })); -vi.mock("./subagent-registry-completion.js", () => ({ - runOutcomesEqual: (left: unknown, right: unknown) => - JSON.stringify(left) === JSON.stringify(right), -})); - vi.mock("./subagent-registry-helpers.js", () => ({ ANNOUNCE_COMPLETION_HARD_EXPIRY_MS: 30 * 60_000, ANNOUNCE_EXPIRY_MS: 5 * 60_000, @@ -238,6 +233,77 @@ describe("subagent registry lifecycle hardening", () => { ); }); + it("enriches registered-run outcomes with persisted timing before cleanup", async () => { + const persist = vi.fn(); + const runSubagentAnnounceFlow = vi.fn(async () => true); + const entry = createRunEntry({ + startedAt: 2_000, + expectsCompletionMessage: false, + }); + + const controller = createLifecycleController({ entry, persist, runSubagentAnnounceFlow }); + + await expect( + controller.completeSubagentRun({ + runId: entry.runId, + endedAt: 4_250, + outcome: { status: "timeout" }, + reason: SUBAGENT_ENDED_REASON_COMPLETE, + triggerCleanup: true, + }), + ).resolves.toBeUndefined(); + + const enrichedOutcome = { + status: "timeout" as const, + startedAt: 2_000, + endedAt: 4_250, + elapsedMs: 2_250, + }; + expect(entry.outcome).toEqual(enrichedOutcome); + expect(taskExecutorMocks.failTaskRunByRunId).toHaveBeenCalledWith( + expect.objectContaining({ + status: "timed_out", + }), + ); + expect(runSubagentAnnounceFlow).toHaveBeenCalledWith( + expect.objectContaining({ + startedAt: 2_000, + endedAt: 4_250, + outcome: enrichedOutcome, + }), + ); + expect(persist).toHaveBeenCalled(); + }); + + it("persists timing when a preexisting outcome matches without timing", async () => { + const persist = vi.fn(); + const entry = createRunEntry({ + startedAt: 2_000, + outcome: { status: "ok" }, + expectsCompletionMessage: false, + }); + + const controller = createLifecycleController({ entry, persist }); + + await expect( + controller.completeSubagentRun({ + runId: entry.runId, + endedAt: 4_250, + outcome: { status: "ok" }, + reason: SUBAGENT_ENDED_REASON_COMPLETE, + triggerCleanup: false, + }), + ).resolves.toBeUndefined(); + + expect(entry.outcome).toEqual({ + status: "ok", + startedAt: 2_000, + endedAt: 4_250, + elapsedMs: 2_250, + }); + expect(persist).toHaveBeenCalled(); + }); + it("does not wait for a completion reply when the run does not expect one", async () => { const entry = createRunEntry({ expectsCompletionMessage: false, diff --git a/src/agents/subagent-registry-lifecycle.ts b/src/agents/subagent-registry-lifecycle.ts index 7b84a8183ca..6d89a3265ca 100644 --- a/src/agents/subagent-registry-lifecycle.ts +++ b/src/agents/subagent-registry-lifecycle.ts @@ -9,6 +9,7 @@ import { setDetachedTaskDeliveryStatusByRunId, } from "../tasks/detached-task-runtime.js"; import { normalizeDeliveryContext } from "../utils/delivery-context.js"; +import { withSubagentOutcomeTiming } from "./subagent-announce-output.js"; import { captureSubagentCompletionReply, runSubagentAnnounceFlow, @@ -22,7 +23,7 @@ import { resolveCleanupCompletionReason, resolveDeferredCleanupDecision, } from "./subagent-registry-cleanup.js"; -import { runOutcomesEqual } from "./subagent-registry-completion.js"; +import { shouldUpdateRunOutcome } from "./subagent-registry-completion.js"; import { ANNOUNCE_COMPLETION_HARD_EXPIRY_MS, ANNOUNCE_EXPIRY_MS, @@ -589,8 +590,12 @@ export function createSubagentRegistryLifecycleController(params: { entry.endedAt = endedAt; mutated = true; } - if (!runOutcomesEqual(entry.outcome, completeParams.outcome)) { - entry.outcome = completeParams.outcome; + const outcome = withSubagentOutcomeTiming(completeParams.outcome, { + startedAt: entry.startedAt, + endedAt, + }); + if (shouldUpdateRunOutcome(entry.outcome, outcome)) { + entry.outcome = outcome; mutated = true; } if (entry.endedReason !== completeParams.reason) { @@ -607,7 +612,7 @@ export function createSubagentRegistryLifecycleController(params: { } safeFinalizeSubagentTaskRun({ entry, - outcome: completeParams.outcome, + outcome, }); try { diff --git a/src/agents/subagent-registry-run-manager.ts b/src/agents/subagent-registry-run-manager.ts index 1e1c0e83773..a5e2d605cb2 100644 --- a/src/agents/subagent-registry-run-manager.ts +++ b/src/agents/subagent-registry-run-manager.ts @@ -7,7 +7,7 @@ import { createRunningTaskRun } from "../tasks/detached-task-runtime.js"; import { type DeliveryContext, normalizeDeliveryContext } from "../utils/delivery-context.js"; import { waitForAgentRun } from "./run-wait.js"; import type { ensureRuntimePluginsLoaded as ensureRuntimePluginsLoadedFn } from "./runtime-plugins.js"; -import type { SubagentRunOutcome } from "./subagent-announce-output.js"; +import { type SubagentRunOutcome, withSubagentOutcomeTiming } from "./subagent-announce-output.js"; import { SUBAGENT_ENDED_OUTCOME_KILLED, SUBAGENT_ENDED_REASON_COMPLETE, @@ -15,7 +15,10 @@ import { SUBAGENT_ENDED_REASON_KILLED, type SubagentLifecycleEndedReason, } from "./subagent-lifecycle-events.js"; -import { emitSubagentEndedHookOnce, runOutcomesEqual } from "./subagent-registry-completion.js"; +import { + emitSubagentEndedHookOnce, + shouldUpdateRunOutcome, +} from "./subagent-registry-completion.js"; import { getSubagentSessionRuntimeMs, getSubagentSessionStartedAt, @@ -127,13 +130,17 @@ export function createSubagentRunManager(params: { mutated = true; } const waitError = typeof wait.error === "string" ? wait.error : undefined; - const outcome: SubagentRunOutcome = + const baseOutcome: SubagentRunOutcome = wait.status === "error" ? { status: "error", error: waitError } : wait.status === "timeout" ? { status: "timeout" } : { status: "ok" }; - if (!runOutcomesEqual(entry.outcome, outcome)) { + const outcome = withSubagentOutcomeTiming(baseOutcome, { + startedAt: entry.startedAt, + endedAt: entry.endedAt, + }); + if (shouldUpdateRunOutcome(entry.outcome, outcome)) { entry.outcome = outcome; mutated = true; } @@ -417,7 +424,13 @@ export function createSubagentRunManager(params: { continue; } entry.endedAt = now; - entry.outcome = { status: "error", error: reason }; + entry.outcome = withSubagentOutcomeTiming( + { status: "error", error: reason }, + { + startedAt: entry.startedAt, + endedAt: now, + }, + ); entry.endedReason = SUBAGENT_ENDED_REASON_KILLED; entry.cleanupHandled = true; entry.cleanupCompletedAt = now; diff --git a/src/agents/subagent-registry.test.ts b/src/agents/subagent-registry.test.ts index 2c8b2de16f0..ba62216c012 100644 --- a/src/agents/subagent-registry.test.ts +++ b/src/agents/subagent-registry.test.ts @@ -191,7 +191,12 @@ describe("subagent registry seam flow", () => { task: "finish the task", cleanup: "delete", roundOneReply: "final completion reply", - outcome: { status: "ok" }, + outcome: { + status: "ok", + startedAt: 111, + endedAt: 222, + elapsedMs: 111, + }, }), ); @@ -397,6 +402,17 @@ describe("subagent registry seam flow", () => { }); expect(updated).toBe(1); + const killedRun = mod + .listSubagentRunsForRequester("agent:main:main") + .find((entry) => entry.runId === "run-killed-init"); + const killedAt = Date.parse("2026-03-24T12:00:00Z"); + expect(killedRun?.outcome).toEqual({ + status: "error", + error: "manual kill", + startedAt: killedAt, + endedAt: killedAt, + elapsedMs: 0, + }); await waitForFast(() => { expect(mocks.ensureRuntimePluginsLoaded).toHaveBeenCalledWith({ config: { diff --git a/src/agents/tools/sessions-spawn-tool.test.ts b/src/agents/tools/sessions-spawn-tool.test.ts index 1c525c7bbf1..cca546ac46a 100644 --- a/src/agents/tools/sessions-spawn-tool.test.ts +++ b/src/agents/tools/sessions-spawn-tool.test.ts @@ -66,6 +66,7 @@ describe("sessions_spawn tool", () => { childSessionKey: "agent:main:subagent:1", runId: "run-subagent", }); + expect(result.details).not.toHaveProperty("role"); expect(hoisted.spawnSubagentDirectMock).toHaveBeenCalledWith( expect.objectContaining({ task: "build feature", @@ -84,6 +85,46 @@ describe("sessions_spawn tool", () => { expect(hoisted.spawnAcpDirectMock).not.toHaveBeenCalled(); }); + it.each([ + { status: "error" as const, error: "spawn failed" }, + { status: "forbidden" as const, error: "not allowed" }, + ])("adds requested role to forwarded subagent $status results", async (spawnResult) => { + hoisted.spawnSubagentDirectMock.mockResolvedValueOnce(spawnResult); + const tool = createSessionsSpawnTool({ + agentSessionKey: "agent:main:main", + }); + + const result = await tool.execute("call-role-error", { + task: "build feature", + agentId: "reviewer", + }); + + expect(result.details).toMatchObject({ + ...spawnResult, + role: "reviewer", + }); + }); + + it("does not add role to forwarded failures when agentId is absent", async () => { + hoisted.spawnSubagentDirectMock.mockResolvedValueOnce({ + status: "error", + error: "spawn failed", + }); + const tool = createSessionsSpawnTool({ + agentSessionKey: "agent:main:main", + }); + + const result = await tool.execute("call-no-role-error", { + task: "build feature", + }); + + expect(result.details).toMatchObject({ + status: "error", + error: "spawn failed", + }); + expect(result.details).not.toHaveProperty("role"); + }); + it("supports legacy timeoutSeconds alias", async () => { const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main", @@ -198,6 +239,30 @@ describe("sessions_spawn tool", () => { expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); }); + it("adds requested role to forwarded ACP failures", async () => { + hoisted.spawnAcpDirectMock.mockResolvedValueOnce({ + status: "forbidden", + error: "ACP disabled", + errorCode: "acp_disabled", + }); + const tool = createSessionsSpawnTool({ + agentSessionKey: "agent:main:main", + }); + + const result = await tool.execute("call-acp-role-error", { + runtime: "acp", + task: "investigate", + agentId: "codex", + }); + + expect(result.details).toMatchObject({ + status: "forbidden", + error: "ACP disabled", + errorCode: "acp_disabled", + role: "codex", + }); + }); + it("forwards ACP sandbox options and requester sandbox context", async () => { const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:subagent:parent", diff --git a/src/agents/tools/sessions-spawn-tool.ts b/src/agents/tools/sessions-spawn-tool.ts index fa05597c721..35b13c8710a 100644 --- a/src/agents/tools/sessions-spawn-tool.ts +++ b/src/agents/tools/sessions-spawn-tool.ts @@ -53,6 +53,16 @@ function summarizeError(err: unknown): string { return "error"; } +function addRoleToFailureResult( + result: T, + role: string | undefined, +): T | (T & { role: string }) { + if (!role || (result.status !== "error" && result.status !== "forbidden")) { + return result; + } + return { ...result, role }; +} + function resolveTrackedSpawnMode(params: { requestedMode?: "run" | "session"; threadRequested: boolean; @@ -201,10 +211,13 @@ export function createSessionsSpawnTool( }>) : undefined; + const roleContext = requestedAgentId ? { role: requestedAgentId } : {}; + if (streamTo && runtime !== "acp") { return jsonResult({ status: "error", error: `streamTo is only supported for runtime=acp; got runtime=${runtime}`, + ...roleContext, }); } @@ -212,6 +225,7 @@ export function createSessionsSpawnTool( return jsonResult({ status: "error", error: `resumeSessionId is only supported for runtime=acp; got runtime=${runtime}`, + ...roleContext, }); } @@ -222,6 +236,7 @@ export function createSessionsSpawnTool( status: "error", error: "attachments are currently unsupported for runtime=acp; use runtime=subagent or remove attachments", + ...roleContext, }); } const result = await spawnAcpDirect( @@ -304,10 +319,11 @@ export function createSessionsSpawnTool( error: `Failed to register ACP run: ${summarizeError(err)}. Cleanup was attempted, but the already-started ACP run may still finish in the background.`, childSessionKey, runId: childRunId, + ...roleContext, }); } } - return jsonResult(result); + return jsonResult(addRoleToFailureResult(result, requestedAgentId)); } const result = await spawnSubagentDirect( @@ -345,7 +361,7 @@ export function createSessionsSpawnTool( }, ); - return jsonResult(result); + return jsonResult(addRoleToFailureResult(result, requestedAgentId)); }, }; }