fix(subagent): include role, session key, and timing in error payloads (#68726)

Merged via squash.

Prepared head SHA: 55c756142f
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
This commit is contained in:
B.K.
2026-04-20 01:31:31 +03:00
committed by GitHub
parent f9a1875127
commit 4277078bc5
13 changed files with 390 additions and 36 deletions

View File

@@ -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

View File

@@ -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<SubagentRunOutcome, "startedAt" | "endedAt" | "elapsedMs"> = {};
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;
}

View File

@@ -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();

View File

@@ -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,

View File

@@ -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(

View File

@@ -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> = {}): 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);
});
});

View File

@@ -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;
}

View File

@@ -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,

View File

@@ -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 {

View File

@@ -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;

View File

@@ -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: {

View File

@@ -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",

View File

@@ -53,6 +53,16 @@ function summarizeError(err: unknown): string {
return "error";
}
function addRoleToFailureResult<T extends { status: string }>(
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));
},
};
}