From 500fed6d4a91efc2ee3beb1b2d437cb58e5691cb Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 31 Jul 2026 09:30:58 -0700 Subject: [PATCH] fix(agents): preserve bounded truthful subagent outcomes (#116932) Co-authored-by: Peter Steinberger --- src/agents/subagent-announce-output.test.ts | 148 ++++++++++++++++++++ src/agents/subagent-announce-output.ts | 95 +++++++++++-- src/agents/subagent-yield-output.ts | 30 ++-- src/agents/tools/subagents-tool.test.ts | 139 ++++++++++++++++++ src/agents/tools/subagents-tool.ts | 13 +- 5 files changed, 404 insertions(+), 21 deletions(-) diff --git a/src/agents/subagent-announce-output.test.ts b/src/agents/subagent-announce-output.test.ts index 198a1c1f4501..1a0cbab4b6f8 100644 --- a/src/agents/subagent-announce-output.test.ts +++ b/src/agents/subagent-announce-output.test.ts @@ -121,6 +121,54 @@ describe("readSubagentOutput", () => { expect(deps.callGateway).toHaveBeenCalledOnce(); }); + it.each([ + { + shape: "OpenAI top-level snake_case function call", + assistant: { + role: "assistant", + content: "Waiting for child completion.", + tool_calls: [{ type: "function", function: { name: "sessions_yield" } }], + }, + }, + { + shape: "top-level camelCase tool call", + assistant: { + role: "assistant", + content: "Waiting for child completion.", + toolCalls: [{ name: "sessions_yield" }], + }, + }, + { + shape: "nested content function call", + assistant: { + role: "assistant", + content: [ + { type: "text", text: "Waiting for child completion." }, + { type: "function_call", function: { name: "sessions_yield" } }, + ], + }, + }, + ])("does not expose a $shape yield turn as completion output", async ({ assistant }) => { + installOutputDeps({ + messages: [assistant, { role: "tool", content: '{"status":"yielded"}' }], + }); + + await expect(readSubagentOutput("agent:main:subagent:child")).resolves.toBeUndefined(); + }); + + it.each(["toolUse", "functionCall", "function_call"])( + "reports visible tool activity for provider-specific %s transcript blocks", + async (type) => { + installOutputDeps({ + messages: [{ role: "assistant", content: [{ type, name: "read" }] }], + }); + + await expect(readSubagentOutput("agent:main:subagent:child")).resolves.toBe( + "1 tool call(s) made without visible output.", + ); + }, + ); + it("returns final assistant output that arrives after a sessions_yield wait turn", async () => { installOutputDeps({ messages: [ @@ -321,6 +369,106 @@ describe("readSubagentOutput", () => { }); describe("buildChildCompletionFindings", () => { + it("hard-bounds each child result and the aggregate parent prompt", () => { + const findings = buildChildCompletionFindings( + Array.from({ length: 8 }, (_, index) => ({ + childSessionKey: `agent:main:subagent:${index}`, + task: `worker ${index}`, + createdAt: index, + completion: { resultText: "🚀".repeat(60_000) }, + outcome: { status: "ok" as const }, + })), + ); + + expect(findings).toBeDefined(); + expect(findings!.length).toBeLessThanOrEqual(4_096); + expect(findings).toContain("status: ok"); + expect(findings).toContain("[child result truncated]"); + expect(findings).toContain("additional child completion result"); + expect(findings).toContain(""); + for (const character of findings ?? "") { + const code = character.charCodeAt(0); + expect(character.length > 1 || code < 0xd800 || code > 0xdfff).toBe(true); + } + }); + + it("retains a later actionable failure when an earlier child exceeds the remaining budget", () => { + const findings = buildChildCompletionFindings([ + { + childSessionKey: "agent:main:subagent:first", + task: "first large result", + createdAt: 1, + completion: { resultText: "<".repeat(100_000) }, + outcome: { status: "ok" }, + }, + { + childSessionKey: "agent:main:subagent:second", + task: "second large result", + createdAt: 2, + completion: { resultText: "<".repeat(100_000) }, + outcome: { status: "ok" }, + }, + { + childSessionKey: "agent:main:subagent:failure", + task: "later actionable failure", + createdAt: 3, + completion: { resultText: "Permission required." }, + outcome: { status: "error", error: "Writable session authorization required." }, + }, + ]); + + expect(findings!.length).toBeLessThanOrEqual(4_096); + expect(findings).toContain("first large result"); + expect(findings).toContain("later actionable failure"); + expect(findings).toContain("status: error: Writable session authorization required."); + expect(findings).toContain("[1 additional child completion result omitted"); + }); + + it("prioritizes an oversized failed completion over an earlier oversized success", () => { + const findings = buildChildCompletionFindings([ + { + childSessionKey: "agent:main:subagent:success", + task: "earlier oversized success", + createdAt: 1, + completion: { resultText: "<".repeat(100_000) }, + outcome: { status: "ok" }, + }, + { + childSessionKey: "agent:main:subagent:failure", + task: "later oversized failure", + createdAt: 2, + completion: { resultText: "<".repeat(100_000) }, + outcome: { status: "error", error: "Writable session authorization required." }, + }, + ]); + + expect(findings!.length).toBeLessThanOrEqual(4_096); + expect(findings).toContain("later oversized failure"); + expect(findings).toContain("status: error: Writable session authorization required."); + expect(findings).not.toContain("earlier oversized success"); + expect(findings).toContain("[1 additional child completion result omitted"); + }); + + it("keeps escaped child data and oversized failure metadata inside the same hard cap", () => { + const findings = buildChildCompletionFindings([ + { + childSessionKey: "agent:main:subagent:child", + label: "L".repeat(20_000), + task: "child task", + createdAt: 1, + completion: { resultText: "<".repeat(100_000) }, + outcome: { status: "error", error: "E".repeat(20_000) }, + }, + ]); + + expect(findings).toBeDefined(); + expect(findings!.length).toBeLessThanOrEqual(4_096); + expect(findings).toContain("status: error:"); + expect(findings).toContain("<"); + expect(findings).toContain("[child result truncated]"); + expect(findings).toContain(""); + }); + it("does not convert ANNOUNCE_SKIP child completions into no-output findings", () => { const findings = buildChildCompletionFindings([ { diff --git a/src/agents/subagent-announce-output.ts b/src/agents/subagent-announce-output.ts index 4c718c4a541e..92d674237da5 100644 --- a/src/agents/subagent-announce-output.ts +++ b/src/agents/subagent-announce-output.ts @@ -4,6 +4,7 @@ * Reads child session output, detects waiting states, and formats completion findings for announcements. */ import { asFiniteNumber } from "@openclaw/normalization-core/number-coercion"; +import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; import type { SessionTranscriptRuntimeTarget } from "../config/sessions/session-accessor.js"; import { isFastTestRuntimeEnv } from "../infra/env.js"; @@ -28,6 +29,17 @@ import { extractAssistantText, sanitizeTextContent } from "./tools/chat-history- import { isAnnounceSkip, selectDeliverableSessionsReply } from "./tools/sessions-send-tokens.js"; const FAST_TEST_RETRY_INTERVAL_MS = 8; +const MAX_CHILD_COMPLETION_RESULT_CHARS = 512; +const MAX_CHILD_COMPLETION_FIELD_CHARS = 256; +const MAX_CHILD_COMPLETION_FINDINGS_CHARS = 4_096; +const CHILD_RESULT_TRUNCATION_NOTICE = "\n[child result truncated]"; +const ASSISTANT_TOOL_CALL_BLOCK_TYPES = new Set([ + "toolCall", + "tool_use", + "toolUse", + "functionCall", + "function_call", +]); type SubagentAnnounceOutputDeps = { callGateway: typeof callGateway; @@ -128,8 +140,7 @@ function countAssistantToolCalls(message: unknown): number { (block) => block && typeof block === "object" && - ((block as { type?: unknown }).type === "toolCall" || - (block as { type?: unknown }).type === "tool_use"), + ASSISTANT_TOOL_CALL_BLOCK_TYPES.has((block as { type?: string }).type ?? ""), ).length : 0; const toolCalls = @@ -358,14 +369,29 @@ function describeSubagentOutcome(outcome?: SubagentRunOutcome): string { } function formatChildResultData(resultText?: string | null): string { + const text = resultText?.trim() || "(no output)"; + const boundedText = + text.length > MAX_CHILD_COMPLETION_RESULT_CHARS + ? `${truncateUtf16Safe( + text, + MAX_CHILD_COMPLETION_RESULT_CHARS - CHILD_RESULT_TRUNCATION_NOTICE.length, + )}${CHILD_RESULT_TRUNCATION_NOTICE}` + : text; return ( wrapPromptDataBlock({ label: "Child result", - text: resultText?.trim() || "(no output)", + text: boundedText, + maxChars: MAX_CHILD_COMPLETION_RESULT_CHARS, }) || "Child result: (no output)" ); } +function truncateChildCompletionField(value: string): string { + return value.length > MAX_CHILD_COMPLETION_FIELD_CHARS + ? `${truncateUtf16Safe(value, MAX_CHILD_COMPLETION_FIELD_CHARS - 1)}…` + : value; +} + type ChildCompletionRow = { childSessionKey: string; task: string; @@ -386,6 +412,12 @@ type ChildCompletionRow = { outcome?: SubagentRunOutcome; }; +type ChildCompletionSection = { + index: number; + text: string; + actionable: boolean; +}; + function selectChildCompletionResultText(child: ChildCompletionRow): string | undefined { const primary = child.completion?.resultText ?? child.delivery?.payload?.frozenResultText; const fallback = @@ -429,7 +461,7 @@ export function buildChildCompletionFindings( : 0; }); - const sections: string[] = []; + const sections: ChildCompletionSection[] = []; for (const [index, child] of sorted.entries()) { const resultText = selectChildCompletionResultText(child); const outcome = describeSubagentOutcome(child.outcome); @@ -442,18 +474,61 @@ export function buildChildCompletionFindings( child.childSessionKey.trim() || `child ${index + 1}`; const displayIndex = sections.length + 1; - sections.push( - [`${displayIndex}. ${title}`, `status: ${outcome}`, formatChildResultData(resultText)].join( - "\n", - ), - ); + sections.push({ + index: displayIndex, + actionable: child.outcome?.status !== "ok", + text: [ + `${displayIndex}. ${truncateChildCompletionField(title)}`, + `status: ${truncateChildCompletionField(outcome)}`, + formatChildResultData(resultText), + ].join("\n"), + }); } if (sections.length === 0) { return undefined; } - return ["Child completion results:", "", ...sections].join("\n\n"); + // Escaping can expand bounded child text. Preserve failures before successes, + // keep rendered survivors chronological, and account for omitted completions. + const render = (visibleSections: string[], omittedCount = 0) => + [ + "Child completion results:", + "", + ...visibleSections, + ...(omittedCount > 0 + ? [ + `[${omittedCount} additional child completion result${omittedCount === 1 ? "" : "s"} omitted to fit the context budget.]`, + ] + : []), + ].join("\n\n"); + const allSections = sections.map((section) => section.text); + if (render(allSections).length <= MAX_CHILD_COMPLETION_FINDINGS_CHARS) { + return render(allSections); + } + const prioritizedSections = [ + ...sections.filter((section) => section.actionable), + ...sections.filter((section) => !section.actionable), + ]; + let visibleSections: ChildCompletionSection[] = []; + for (const section of prioritizedSections) { + const nextSections = [...visibleSections, section].toSorted( + (left, right) => left.index - right.index, + ); + const omittedCount = sections.length - nextSections.length; + if ( + render( + nextSections.map((entry) => entry.text), + omittedCount, + ).length <= MAX_CHILD_COMPLETION_FINDINGS_CHARS + ) { + visibleSections = nextSections; + } + } + return render( + visibleSections.map((section) => section.text), + sections.length - visibleSections.length, + ); } export function dedupeLatestChildCompletionRows( diff --git a/src/agents/subagent-yield-output.ts b/src/agents/subagent-yield-output.ts index 771091bee04c..f4a7806b9339 100644 --- a/src/agents/subagent-yield-output.ts +++ b/src/agents/subagent-yield-output.ts @@ -12,13 +12,13 @@ function readToolName(value: unknown): string | undefined { if (!record) { return undefined; } - return readTrimmedStringAlias(record, [ - "name", - "toolName", - "tool_name", - "functionName", - "function_name", - ]); + const aliases = ["name", "toolName", "tool_name", "functionName", "function_name"]; + const direct = readTrimmedStringAlias(record, aliases); + if (direct) { + return direct; + } + const nestedFunction = asOptionalRecord(record.function); + return nestedFunction ? readTrimmedStringAlias(nestedFunction, aliases) : undefined; } function isToolCallBlock(value: unknown): boolean { @@ -38,11 +38,21 @@ function isToolCallBlock(value: unknown): boolean { /** Returns true when an assistant message requested the sessions_yield tool. */ export function assistantCallsSessionsYield(message: unknown): boolean { const record = asOptionalRecord(message); - if (!record || record.role !== "assistant" || !Array.isArray(record.content)) { + if (!record || record.role !== "assistant") { return false; } - return record.content.some( - (block) => isToolCallBlock(block) && readToolName(block) === "sessions_yield", + if ( + Array.isArray(record.content) && + record.content.some( + (block) => isToolCallBlock(block) && readToolName(block) === "sessions_yield", + ) + ) { + return true; + } + return [record.toolCalls, record.tool_calls].some( + (toolCalls) => + Array.isArray(toolCalls) && + toolCalls.some((toolCall) => readToolName(toolCall) === "sessions_yield"), ); } diff --git a/src/agents/tools/subagents-tool.test.ts b/src/agents/tools/subagents-tool.test.ts index d1f74a8a3872..1ede25caa30a 100644 --- a/src/agents/tools/subagents-tool.test.ts +++ b/src/agents/tools/subagents-tool.test.ts @@ -1,6 +1,7 @@ // Subagents tool tests cover requester-scoped task listing and cancellation. import { describe, expect, it, vi } from "vitest"; import type { TaskRecord, TaskRuntime, TaskStatus } from "../../tasks/task-registry.types.js"; +import { TASK_STATUS_DETAIL_MAX_CHARS } from "../../tasks/task-status.js"; import { createSubagentsTool } from "./subagents-tool.js"; function task(params: { @@ -13,6 +14,8 @@ function task(params: { label?: string; progressSummary?: string; terminalSummary?: string; + terminalOutcome?: TaskRecord["terminalOutcome"]; + error?: string; }): TaskRecord { return { taskId: params.taskId, @@ -30,6 +33,8 @@ function task(params: { ...(params.label ? { label: params.label } : {}), ...(params.progressSummary ? { progressSummary: params.progressSummary } : {}), ...(params.terminalSummary ? { terminalSummary: params.terminalSummary } : {}), + ...(params.terminalOutcome ? { terminalOutcome: params.terminalOutcome } : {}), + ...(params.error ? { error: params.error } : {}), }; } @@ -155,6 +160,140 @@ describe("subagents tool", () => { expect(cancelTask).toHaveBeenCalledTimes(1); }); + it("preserves blocked terminal outcomes and actionable terminal failure reasons", async () => { + const tasks = [ + task({ + taskId: "blocked", + runtime: "acp", + status: "succeeded", + terminalOutcome: "blocked", + terminalSummary: "Writable session authorization required.", + }), + task({ + taskId: "failed", + runtime: "subagent", + status: "failed", + error: "Provider rejected the tool call.", + }), + task({ + taskId: "timed-out", + runtime: "cli", + status: "timed_out", + error: "Provider timed out before producing output.", + }), + ]; + const tool = createSubagentsTool({ + agentSessionKey: "agent:main:main", + config: {}, + listTasks: () => tasks, + }); + + const result = await tool.execute("list", { action: "list" }); + + expect(result.details).toMatchObject({ + status: "ok", + taskTotal: 3, + tasks: expect.arrayContaining([ + expect.objectContaining({ + taskId: "blocked", + status: "blocked", + terminalOutcome: "blocked", + terminalSummary: "Writable session authorization required.", + }), + expect.objectContaining({ + taskId: "failed", + status: "failed", + error: "Provider rejected the tool call.", + }), + expect.objectContaining({ + taskId: "timed-out", + status: "timed_out", + error: "Provider timed out before producing output.", + }), + ]), + }); + }); + + it("bounds terminal failure text with the canonical surrogate-safe task detail budget", async () => { + const tool = createSubagentsTool({ + agentSessionKey: "agent:main:main", + config: {}, + listTasks: () => [ + task({ + taskId: "oversized-error", + runtime: "subagent", + status: "failed", + error: "🚀".repeat(20_000), + }), + ], + }); + + const result = await tool.execute("list", { action: "list" }); + const [row] = (result.details as { tasks: Array<{ error?: string }> }).tasks; + + if (!row?.error) { + throw new Error("Expected a sanitized terminal task failure."); + } + expect(row.error.length).toBeLessThanOrEqual(TASK_STATUS_DETAIL_MAX_CHARS); + expect(row.error.endsWith("…")).toBe(true); + for (const character of row.error) { + const code = character.charCodeAt(0); + expect(character.length > 1 || code < 0xd800 || code > 0xdfff).toBe(true); + } + }); + + it("strips internal provider context and redacts raw approval denial details", async () => { + const internalContext = [ + "OpenClaw runtime context (internal):", + "This context is runtime-generated, not user-authored. Keep internal details private.", + "[Internal task completion event]", + "providerAuthorization: private-provider-context", + ].join("\n"); + const tool = createSubagentsTool({ + agentSessionKey: "agent:main:main", + config: {}, + listTasks: () => [ + task({ + taskId: "with-internal-context", + runtime: "subagent", + status: "failed", + error: `Permission denied by ACP runtime.\n<<>>\n${internalContext}\n<<>>`, + }), + task({ + taskId: "only-internal-context", + runtime: "subagent", + status: "failed", + error: internalContext, + }), + task({ + taskId: "approval-denied", + runtime: "acp", + status: "failed", + error: "Exec denied (gateway id=req-1, approval-timeout): bash -lc print-private-context", + }), + ], + }); + + const result = await tool.execute("list", { action: "list" }); + const rows = (result.details as { tasks: Array<{ taskId: string; error?: string }> }).tasks; + + expect(rows).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + taskId: "with-internal-context", + error: "Permission denied by ACP runtime.", + }), + expect.objectContaining({ + taskId: "approval-denied", + error: "Command did not run: approval timed out.", + }), + ]), + ); + expect(rows.find((row) => row.taskId === "only-internal-context")).not.toHaveProperty("error"); + expect(JSON.stringify(result.details)).not.toContain("private-provider-context"); + expect(JSON.stringify(result.details)).not.toContain("print-private-context"); + }); + it.each([0, 1.5])("rejects invalid recentMinutes value %s", async (recentMinutes) => { const tool = createSubagentsTool(); diff --git a/src/agents/tools/subagents-tool.ts b/src/agents/tools/subagents-tool.ts index 438d964ca4c6..3304204ac000 100644 --- a/src/agents/tools/subagents-tool.ts +++ b/src/agents/tools/subagents-tool.ts @@ -9,6 +9,7 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { listTaskRecordsUnsorted } from "../../tasks/runtime-internal.js"; import { cancelDetachedTaskRunById } from "../../tasks/task-executor.js"; import type { TaskRecord, TaskStatus } from "../../tasks/task-registry.types.js"; +import { TASK_STATUS_DETAIL_MAX_CHARS, sanitizeTaskStatusText } from "../../tasks/task-status.js"; import { optionalPositiveIntegerSchema, optionalStringEnum } from "../schema/typebox.js"; import { DEFAULT_RECENT_MINUTES, @@ -74,13 +75,23 @@ function listTreeTasks(tasks: TaskRecord[], rootSessionKey: string): TaskRecord[ } function mapTask(task: TaskRecord) { + // Task failures can contain hidden provider/runtime context; reuse the bounded status owner. + const error = sanitizeTaskStatusText(task.error, { + errorContext: true, + maxChars: TASK_STATUS_DETAIL_MAX_CHARS, + }); return { taskId: task.taskId, runtime: task.runtime, - status: STATUS_MAP[task.status], + status: + task.status === "succeeded" && task.terminalOutcome === "blocked" + ? "blocked" + : STATUS_MAP[task.status], ...(task.label ? { label: task.label } : {}), ...(task.progressSummary ? { progressSummary: task.progressSummary } : {}), ...(task.terminalSummary ? { terminalSummary: task.terminalSummary } : {}), + ...(task.terminalOutcome ? { terminalOutcome: task.terminalOutcome } : {}), + ...(error ? { error } : {}), }; }