From ccc4e69052d15f95b09eb94f611fcbe4257b7079 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 24 Aug 2026 00:57:53 -0700 Subject: [PATCH] fix(subagents): hide private commentary from logs and completion announcements (#128584) * fix(subagents): hide commentary in logs and completion announcements * refactor(agents): remove obsolete test-only history sanitizer export --- config/assertion-safety-baseline.txt | 2 +- .../announce/subagent-announce-output.test.ts | 11 +++ .../announce/subagent-announce-output.ts | 19 +---- src/agents/tools/chat-history-text.ts | 2 +- src/agents/tools/sessions.test.ts | 10 +-- .../reply/commands-subagents-text.ts | 8 +- .../reply/commands-subagents.test.ts | 73 +++++++++++++++++++ 7 files changed, 96 insertions(+), 29 deletions(-) diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 5419ee10614e..cc21366662c7 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -2058,7 +2058,7 @@ src/agents/subagents/announce/subagent-announce-delivery-retry.ts 2 src/agents/subagents/announce/subagent-announce-delivery.runtime.ts 2 src/agents/subagents/announce/subagent-announce-delivery.ts 1 src/agents/subagents/announce/subagent-announce-origin.ts 2 -src/agents/subagents/announce/subagent-announce-output.ts 10 +src/agents/subagents/announce/subagent-announce-output.ts 8 src/agents/subagents/completion/subagent-completion-admission.store.ts 2 src/agents/subagents/registry/subagent-control-messaging.ts 2 src/agents/subagents/registry/subagent-control.ts 1 diff --git a/src/agents/subagents/announce/subagent-announce-output.test.ts b/src/agents/subagents/announce/subagent-announce-output.test.ts index 793794c18393..b381427be465 100644 --- a/src/agents/subagents/announce/subagent-announce-output.test.ts +++ b/src/agents/subagents/announce/subagent-announce-output.test.ts @@ -122,6 +122,17 @@ describe("readSubagentOutput", () => { expect(deps.callGateway).toHaveBeenCalledOnce(); }); + it.each([ + { phase: "commentary", expected: undefined }, + { phase: "final_answer", expected: "Visible subagent answer" }, + ])("respects the phase of scalar $phase output", async ({ phase, expected }) => { + installOutputDeps({ + messages: [{ role: "assistant", phase, content: "Visible subagent answer" }], + }); + + await expect(readSubagentOutput("agent:main:subagent:child")).resolves.toBe(expected); + }); + it.each([ { shape: "OpenAI top-level snake_case function call", diff --git a/src/agents/subagents/announce/subagent-announce-output.ts b/src/agents/subagents/announce/subagent-announce-output.ts index 3c5377f263ba..108fc8a8db49 100644 --- a/src/agents/subagents/announce/subagent-announce-output.ts +++ b/src/agents/subagents/announce/subagent-announce-output.ts @@ -12,7 +12,7 @@ import { isFastTestRuntimeEnv } from "../../../infra/env.js"; import { formatDurationCompact } from "../../../infra/format-time/format-duration.js"; import { buildAgentRunTerminalOutcomeFromWaitResult } from "../../agent-run-terminal-outcome.js"; import { wrapPromptDataBlock } from "../../sanitize-for-prompt.js"; -import { extractStoredAssistantText, sanitizeTextContent } from "../../tools/chat-history-text.js"; +import { extractStoredAssistantText } from "../../tools/chat-history-text.js"; import { isAnnounceSkip } from "../../tools/sessions-send-tokens.js"; import { resolveSubagentCompletionResultText } from "../completion/subagent-completion-result.js"; import { compareSubagentRunGeneration } from "../registry/subagent-run-generation.js"; @@ -117,21 +117,6 @@ export function withSubagentOutcomeTiming( return { ...outcome, ...nextTiming }; } -function extractSubagentAssistantText(message: unknown): string { - if (!message || typeof message !== "object") { - return ""; - } - const role = (message as { role?: unknown }).role; - if (role !== "assistant") { - return ""; - } - const content = (message as { content?: unknown }).content; - if (typeof content === "string") { - return sanitizeTextContent(content); - } - return extractStoredAssistantText(message) ?? ""; -} - function countAssistantToolCalls(message: unknown): number { if (!message || typeof message !== "object") { return 0; @@ -196,7 +181,7 @@ function summarizeSubagentOutputHistory(messages: Array): SubagentOutpu previousAssistantCalledYield = false; continue; } - const text = extractSubagentAssistantText(message).trim(); + const text = extractStoredAssistantText(message)?.trim(); if (!text) { snapshot.waitingForContinuation = false; previousAssistantCalledYield = false; diff --git a/src/agents/tools/chat-history-text.ts b/src/agents/tools/chat-history-text.ts index ca111a9e2c64..c91fe4ef1d57 100644 --- a/src/agents/tools/chat-history-text.ts +++ b/src/agents/tools/chat-history-text.ts @@ -22,7 +22,7 @@ export function stripToolMessages(messages: unknown[]): unknown[] { * Sanitize text content to strip tool call markers and thinking tags. * This ensures user-facing text doesn't leak internal tool representations. */ -export function sanitizeTextContent(text: string): string { +function sanitizeTextContent(text: string): string { return sanitizeAssistantVisibleTextWithProfile(text, "history"); } diff --git a/src/agents/tools/sessions.test.ts b/src/agents/tools/sessions.test.ts index 288b4e645c11..a1d58a373428 100644 --- a/src/agents/tools/sessions.test.ts +++ b/src/agents/tools/sessions.test.ts @@ -18,7 +18,7 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { GatewayClientRequestError } from "../../gateway/client.js"; import { withTestDir } from "../../test-helpers/temp-dir.js"; import { createTestRegistry } from "../../test-utils/channel-plugins.js"; -import { extractStoredAssistantText, sanitizeTextContent } from "./chat-history-text.js"; +import { extractStoredAssistantText } from "./chat-history-text.js"; const callGatewayMock = vi.fn(); const inProcessGatewayRequestMock = vi.fn((opts: unknown) => callGatewayMock(opts)); @@ -381,13 +381,13 @@ async function executeFireAndForgetA2AFrom( return flowParams; } -describe("sanitizeTextContent", () => { +describe("extractStoredAssistantText sanitization", () => { it("strips minimax tool call XML and downgraded markers", () => { // Session recall should not replay provider/tool markup as assistant text. const input = 'Hello payload ' + "[Tool Call: foo (ID: 1)] world"; - const result = sanitizeTextContent(input).trim(); + const result = extractStoredAssistantText({ role: "assistant", content: input })?.trim(); expect(result).toBe("Hello world"); expect(result).not.toContain("invoke"); expect(result).not.toContain("Tool Call"); @@ -395,14 +395,14 @@ describe("sanitizeTextContent", () => { it("strips tool_result XML via the shared assistant-visible sanitizer", () => { const input = 'Prefix\n{"output":"hidden"}\nSuffix'; - const result = sanitizeTextContent(input).trim(); + const result = extractStoredAssistantText({ role: "assistant", content: input })?.trim(); expect(result).toBe("Prefix\n\nSuffix"); expect(result).not.toContain("tool_result"); }); it("strips thinking tags", () => { const input = "Before secret after"; - const result = sanitizeTextContent(input).trim(); + const result = extractStoredAssistantText({ role: "assistant", content: input })?.trim(); expect(result).toBe("Before after"); }); }); diff --git a/src/auto-reply/reply/commands-subagents-text.ts b/src/auto-reply/reply/commands-subagents-text.ts index 7da74ad5b13c..878eab4a7de3 100644 --- a/src/auto-reply/reply/commands-subagents-text.ts +++ b/src/auto-reply/reply/commands-subagents-text.ts @@ -1,5 +1,5 @@ /** Text extraction helpers for subagent command output. */ -import { sanitizeTextContent } from "../../agents/tools/chat-history-text.js"; +import { extractStoredAssistantText } from "../../agents/tools/chat-history-text.js"; import { extractTextFromChatContent } from "../../shared/chat-content.js"; /** Minimal chat message shape used by subagent text extraction. */ @@ -13,9 +13,7 @@ export function extractSubagentMessageText( message: ChatMessage, ): { role: string; text: string } | null { const role = typeof message.role === "string" ? message.role : ""; - const shouldSanitize = role === "assistant"; - const text = extractTextFromChatContent(message.content, { - sanitizeText: shouldSanitize ? sanitizeTextContent : undefined, - }); + const content = role === "assistant" ? extractStoredAssistantText(message) : message.content; + const text = extractTextFromChatContent(content); return text ? { role, text } : null; } diff --git a/src/auto-reply/reply/commands-subagents.test.ts b/src/auto-reply/reply/commands-subagents.test.ts index 5d8db8fab9df..c950619f6be8 100644 --- a/src/auto-reply/reply/commands-subagents.test.ts +++ b/src/auto-reply/reply/commands-subagents.test.ts @@ -494,6 +494,79 @@ describe("subagents log", () => { }); }); + it.each([ + { + name: "hides signed commentary while retaining the final answer", + messages: [ + { + role: "assistant", + content: [ + { + type: "text", + text: "PRIVATE_COMMENTARY", + textSignature: JSON.stringify({ v: 1, phase: "commentary" }), + }, + { + type: "output_text", + text: "Visible final answer", + textSignature: JSON.stringify({ v: 1, phase: "final_answer" }), + }, + ], + }, + ], + expectedText: "Assistant: Visible final answer", + unexpectedText: "PRIVATE_COMMENTARY", + }, + { + name: "omits commentary-only history messages", + messages: [{ role: "assistant", phase: "commentary", content: "PRIVATE_COMMENTARY" }], + expectedText: "(no messages)", + unexpectedText: "PRIVATE_COMMENTARY", + }, + { + name: "does not revive legacy text when the signed final answer is empty", + messages: [ + { + role: "assistant", + content: [ + { type: "text", text: "PRIVATE_LEGACY" }, + { + type: "text", + text: " ", + textSignature: JSON.stringify({ v: 1, phase: "final_answer" }), + }, + ], + }, + ], + expectedText: "(no messages)", + unexpectedText: "PRIVATE_LEGACY", + }, + { + name: "renders persisted Responses output text", + messages: [ + { role: "assistant", content: [{ type: "output_text", text: "Persisted output" }] }, + ], + expectedText: "Assistant: Persisted output", + unexpectedText: "(no messages)", + }, + { + name: "renders persisted assistant input text", + messages: [ + { role: "assistant", content: [{ type: "input_text", text: "Persisted assistant input" }] }, + ], + expectedText: "Assistant: Persisted assistant input", + unexpectedText: "(no messages)", + }, + ])("$name", async ({ messages, expectedText, unexpectedText }) => { + callGatewayMock.mockResolvedValue({ messages }); + + const result = await handleSubagentsLogAction(buildLogContext(["1"], [makeRun()])); + const text = requireReplyText(result.reply); + + expect(text).toContain(expectedText); + expect(text).not.toContain(unexpectedText); + }); + it("uses the numeric token after the target as the history limit", async () => { await handleSubagentsLogAction(buildLogContext(["1", "5"], [makeRun()]));