mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 20:05:46 -06:00
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
This commit is contained in:
committed by
GitHub
parent
68ba1ef641
commit
ccc4e69052
@@ -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
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<unknown>): SubagentOutpu
|
||||
previousAssistantCalledYield = false;
|
||||
continue;
|
||||
}
|
||||
const text = extractSubagentAssistantText(message).trim();
|
||||
const text = extractStoredAssistantText(message)?.trim();
|
||||
if (!text) {
|
||||
snapshot.waitingForContinuation = false;
|
||||
previousAssistantCalledYield = false;
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
|
||||
@@ -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 <invoke name="tool">payload</invoke></minimax:tool_call> ' +
|
||||
"[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<tool_result>{"output":"hidden"}</tool_result>\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 <think>secret</think> after";
|
||||
const result = sanitizeTextContent(input).trim();
|
||||
const result = extractStoredAssistantText({ role: "assistant", content: input })?.trim();
|
||||
expect(result).toBe("Before after");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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()]));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user