From c8b958e573f647a850057f82f6b78c8a233cd136 Mon Sep 17 00:00:00 2001 From: Onur <2453968+osolmaz@users.noreply.github.com> Date: Sun, 1 Mar 2026 14:55:40 +0100 Subject: [PATCH] ACP: add hidden-boundary separator for hidden tool events --- src/auto-reply/reply/acp-projector.test.ts | 198 ++++++++++++++++++ src/auto-reply/reply/acp-projector.ts | 67 +++++- .../reply/acp-stream-settings.test.ts | 3 + src/auto-reply/reply/acp-stream-settings.ts | 11 + src/config/schema.help.ts | 2 + src/config/schema.labels.ts | 1 + src/config/types.acp.ts | 2 + src/config/zod-schema.ts | 8 + 8 files changed, 291 insertions(+), 1 deletion(-) diff --git a/src/auto-reply/reply/acp-projector.test.ts b/src/auto-reply/reply/acp-projector.test.ts index 8289800ca..0c58a5ce3 100644 --- a/src/auto-reply/reply/acp-projector.test.ts +++ b/src/auto-reply/reply/acp-projector.test.ts @@ -573,4 +573,202 @@ describe("createAcpReplyProjector", () => { expect(deliveries.length).toBe(1); expect(deliveries[0]?.text).toContain("Tool Call"); }); + + it("inserts a newline boundary before visible text after hidden tool updates by default", async () => { + const deliveries: Array<{ kind: string; text?: string }> = []; + const projector = createAcpReplyProjector({ + cfg: createCfg({ + acp: { + enabled: true, + stream: { + coalesceIdleMs: 0, + maxChunkChars: 256, + deliveryMode: "live", + }, + }, + }), + shouldSendToolSummaries: true, + deliver: async (kind, payload) => { + deliveries.push({ kind, text: payload.text }); + return true; + }, + }); + + await projector.onEvent({ type: "text_delta", text: "fallback.", tag: "agent_message_chunk" }); + await projector.onEvent({ + type: "tool_call", + tag: "tool_call", + toolCallId: "call_hidden_1", + status: "in_progress", + title: "Run test", + text: "Run test (in_progress)", + }); + await projector.onEvent({ type: "text_delta", text: "I don't", tag: "agent_message_chunk" }); + await projector.flush(true); + + const combinedText = deliveries + .filter((entry) => entry.kind === "block") + .map((entry) => entry.text ?? "") + .join(""); + expect(combinedText).toBe("fallback.\nI don't"); + }); + + it("supports hiddenBoundarySeparator=space", async () => { + const deliveries: Array<{ kind: string; text?: string }> = []; + const projector = createAcpReplyProjector({ + cfg: createCfg({ + acp: { + enabled: true, + stream: { + coalesceIdleMs: 0, + maxChunkChars: 256, + deliveryMode: "live", + hiddenBoundarySeparator: "space", + }, + }, + }), + shouldSendToolSummaries: true, + deliver: async (kind, payload) => { + deliveries.push({ kind, text: payload.text }); + return true; + }, + }); + + await projector.onEvent({ type: "text_delta", text: "fallback.", tag: "agent_message_chunk" }); + await projector.onEvent({ + type: "tool_call", + tag: "tool_call", + toolCallId: "call_hidden_2", + status: "in_progress", + title: "Run test", + text: "Run test (in_progress)", + }); + await projector.onEvent({ type: "text_delta", text: "I don't", tag: "agent_message_chunk" }); + await projector.flush(true); + + const combinedText = deliveries + .filter((entry) => entry.kind === "block") + .map((entry) => entry.text ?? "") + .join(""); + expect(combinedText).toBe("fallback. I don't"); + }); + + it("supports hiddenBoundarySeparator=none", async () => { + const deliveries: Array<{ kind: string; text?: string }> = []; + const projector = createAcpReplyProjector({ + cfg: createCfg({ + acp: { + enabled: true, + stream: { + coalesceIdleMs: 0, + maxChunkChars: 256, + deliveryMode: "live", + hiddenBoundarySeparator: "none", + }, + }, + }), + shouldSendToolSummaries: true, + deliver: async (kind, payload) => { + deliveries.push({ kind, text: payload.text }); + return true; + }, + }); + + await projector.onEvent({ type: "text_delta", text: "fallback.", tag: "agent_message_chunk" }); + await projector.onEvent({ + type: "tool_call", + tag: "tool_call", + toolCallId: "call_hidden_3", + status: "in_progress", + title: "Run test", + text: "Run test (in_progress)", + }); + await projector.onEvent({ type: "text_delta", text: "I don't", tag: "agent_message_chunk" }); + await projector.flush(true); + + const combinedText = deliveries + .filter((entry) => entry.kind === "block") + .map((entry) => entry.text ?? "") + .join(""); + expect(combinedText).toBe("fallback.I don't"); + }); + + it("does not duplicate newlines when previous visible text already ends with newline", async () => { + const deliveries: Array<{ kind: string; text?: string }> = []; + const projector = createAcpReplyProjector({ + cfg: createCfg({ + acp: { + enabled: true, + stream: { + coalesceIdleMs: 0, + maxChunkChars: 256, + deliveryMode: "live", + }, + }, + }), + shouldSendToolSummaries: true, + deliver: async (kind, payload) => { + deliveries.push({ kind, text: payload.text }); + return true; + }, + }); + + await projector.onEvent({ + type: "text_delta", + text: "fallback.\n", + tag: "agent_message_chunk", + }); + await projector.onEvent({ + type: "tool_call", + tag: "tool_call", + toolCallId: "call_hidden_4", + status: "in_progress", + title: "Run test", + text: "Run test (in_progress)", + }); + await projector.onEvent({ type: "text_delta", text: "I don't", tag: "agent_message_chunk" }); + await projector.flush(true); + + const combinedText = deliveries + .filter((entry) => entry.kind === "block") + .map((entry) => entry.text ?? "") + .join(""); + expect(combinedText).toBe("fallback.\nI don't"); + }); + + it("does not insert boundary separator for hidden non-tool status updates", async () => { + const deliveries: Array<{ kind: string; text?: string }> = []; + const projector = createAcpReplyProjector({ + cfg: createCfg({ + acp: { + enabled: true, + stream: { + coalesceIdleMs: 0, + maxChunkChars: 256, + deliveryMode: "live", + }, + }, + }), + shouldSendToolSummaries: true, + deliver: async (kind, payload) => { + deliveries.push({ kind, text: payload.text }); + return true; + }, + }); + + await projector.onEvent({ type: "text_delta", text: "A", tag: "agent_message_chunk" }); + await projector.onEvent({ + type: "status", + tag: "available_commands_update", + text: "available commands updated", + }); + await projector.onEvent({ type: "text_delta", text: "B", tag: "agent_message_chunk" }); + await projector.flush(true); + + const combinedText = deliveries + .filter((entry) => entry.kind === "block") + .map((entry) => entry.text ?? "") + .join(""); + expect(combinedText).toBe("AB"); + }); }); diff --git a/src/auto-reply/reply/acp-projector.ts b/src/auto-reply/reply/acp-projector.ts index ede78abca..320fc976c 100644 --- a/src/auto-reply/reply/acp-projector.ts +++ b/src/auto-reply/reply/acp-projector.ts @@ -5,6 +5,7 @@ import type { OpenClawConfig } from "../../config/config.js"; import { prefixSystemMessage } from "../../infra/system-message.js"; import type { ReplyPayload } from "../types.js"; import { + type AcpHiddenBoundarySeparator, isAcpTagVisible, resolveAcpProjectionSettings, resolveAcpStreamingConfig, @@ -15,6 +16,7 @@ import type { ReplyDispatchKind } from "./reply-dispatcher.js"; const ACP_BLOCK_REPLY_TIMEOUT_MS = 15_000; const TERMINAL_TOOL_STATUSES = new Set(["completed", "failed", "cancelled", "done", "error"]); +const HIDDEN_BOUNDARY_TAGS = new Set(["tool_call", "tool_call_update"]); export type AcpProjectedDeliveryMeta = { tag?: AcpSessionUpdateTag; @@ -56,6 +58,47 @@ function normalizeToolStatus(status: string | undefined): string | undefined { return normalized || undefined; } +function resolveHiddenBoundarySeparatorText(mode: AcpHiddenBoundarySeparator): string { + if (mode === "space") { + return " "; + } + if (mode === "newline") { + return "\n"; + } + if (mode === "paragraph") { + return "\n\n"; + } + return ""; +} + +function shouldInsertSeparator(params: { + separator: string; + previousTail: string | undefined; + nextText: string; +}): boolean { + if (!params.separator) { + return false; + } + if (!params.nextText) { + return false; + } + const firstChar = params.nextText[0]; + if (typeof firstChar === "string" && /\s/.test(firstChar)) { + return false; + } + const tail = params.previousTail ?? ""; + if (!tail) { + return false; + } + if (params.separator === " " && /\s$/.test(tail)) { + return false; + } + if ((params.separator === "\n" || params.separator === "\n\n") && tail.endsWith("\n")) { + return false; + } + return true; +} + function renderToolSummaryText(event: Extract): string { const detailParts: string[] = []; const title = event.title?.trim(); @@ -114,6 +157,8 @@ export function createAcpReplyProjector(params: { let lastStatusHash: string | undefined; let lastToolHash: string | undefined; let lastUsageTuple: string | undefined; + let lastVisibleOutputTail: string | undefined; + let pendingHiddenBoundary = false; const pendingToolDeliveries: BufferedToolDelivery[] = []; const toolLifecycleById = new Map(); @@ -124,6 +169,8 @@ export function createAcpReplyProjector(params: { lastStatusHash = undefined; lastToolHash = undefined; lastUsageTuple = undefined; + lastVisibleOutputTail = undefined; + pendingHiddenBoundary = false; pendingToolDeliveries.length = 0; toolLifecycleById.clear(); }; @@ -291,10 +338,21 @@ export function createAcpReplyProjector(params: { if (!isAcpTagVisible(settings, event.tag)) { return; } - const text = event.text; + let text = event.text; if (!text) { return; } + if ( + pendingHiddenBoundary && + shouldInsertSeparator({ + separator: resolveHiddenBoundarySeparatorText(settings.hiddenBoundarySeparator), + previousTail: lastVisibleOutputTail, + nextText: text, + }) + ) { + text = `${resolveHiddenBoundarySeparatorText(settings.hiddenBoundarySeparator)}${text}`; + } + pendingHiddenBoundary = false; if (emittedTurnChars >= settings.maxTurnChars) { await emitTruncationNotice(); return; @@ -304,6 +362,7 @@ export function createAcpReplyProjector(params: { if (accepted.length > 0) { chunker.append(accepted); emittedTurnChars += accepted.length; + lastVisibleOutputTail = accepted.slice(-1); drainChunker(false); } if (accepted.length < text.length) { @@ -333,6 +392,12 @@ export function createAcpReplyProjector(params: { } if (event.type === "tool_call") { + if (!isAcpTagVisible(settings, event.tag)) { + if (event.tag && HIDDEN_BOUNDARY_TAGS.has(event.tag)) { + pendingHiddenBoundary = true; + } + return; + } await emitToolSummary(event); return; } diff --git a/src/auto-reply/reply/acp-stream-settings.test.ts b/src/auto-reply/reply/acp-stream-settings.test.ts index 3cc63370c..2cb6207d5 100644 --- a/src/auto-reply/reply/acp-stream-settings.test.ts +++ b/src/auto-reply/reply/acp-stream-settings.test.ts @@ -10,6 +10,7 @@ describe("acp stream settings", () => { it("resolves stable defaults", () => { const settings = resolveAcpProjectionSettings(createAcpTestConfig()); expect(settings.deliveryMode).toBe("final_only"); + expect(settings.hiddenBoundarySeparator).toBe("newline"); expect(settings.repeatSuppression).toBe(true); expect(settings.maxTurnChars).toBe(24_000); expect(settings.maxMetaEventsPerTurn).toBe(64); @@ -22,6 +23,7 @@ describe("acp stream settings", () => { enabled: true, stream: { deliveryMode: "final_only", + hiddenBoundarySeparator: "space", repeatSuppression: false, maxTurnChars: 500, maxMetaEventsPerTurn: 7, @@ -33,6 +35,7 @@ describe("acp stream settings", () => { }), ); expect(settings.deliveryMode).toBe("final_only"); + expect(settings.hiddenBoundarySeparator).toBe("space"); expect(settings.repeatSuppression).toBe(false); expect(settings.maxTurnChars).toBe(500); expect(settings.maxMetaEventsPerTurn).toBe(7); diff --git a/src/auto-reply/reply/acp-stream-settings.ts b/src/auto-reply/reply/acp-stream-settings.ts index 55aaf339f..bbe51ae2f 100644 --- a/src/auto-reply/reply/acp-stream-settings.ts +++ b/src/auto-reply/reply/acp-stream-settings.ts @@ -6,6 +6,7 @@ const DEFAULT_ACP_STREAM_COALESCE_IDLE_MS = 350; const DEFAULT_ACP_STREAM_MAX_CHUNK_CHARS = 1800; const DEFAULT_ACP_REPEAT_SUPPRESSION = true; const DEFAULT_ACP_DELIVERY_MODE = "final_only"; +const DEFAULT_ACP_HIDDEN_BOUNDARY_SEPARATOR = "newline"; const DEFAULT_ACP_MAX_TURN_CHARS = 24_000; const DEFAULT_ACP_MAX_TOOL_SUMMARY_CHARS = 320; const DEFAULT_ACP_MAX_STATUS_CHARS = 320; @@ -25,9 +26,11 @@ export const ACP_TAG_VISIBILITY_DEFAULTS: Record = }; export type AcpDeliveryMode = "live" | "final_only"; +export type AcpHiddenBoundarySeparator = "none" | "space" | "newline" | "paragraph"; export type AcpProjectionSettings = { deliveryMode: AcpDeliveryMode; + hiddenBoundarySeparator: AcpHiddenBoundarySeparator; repeatSuppression: boolean; maxTurnChars: number; maxToolSummaryChars: number; @@ -65,6 +68,13 @@ function resolveAcpDeliveryMode(value: unknown): AcpDeliveryMode { return DEFAULT_ACP_DELIVERY_MODE; } +function resolveAcpHiddenBoundarySeparator(value: unknown): AcpHiddenBoundarySeparator { + if (value === "none" || value === "space" || value === "newline" || value === "paragraph") { + return value; + } + return DEFAULT_ACP_HIDDEN_BOUNDARY_SEPARATOR; +} + function resolveAcpStreamCoalesceIdleMs(cfg: OpenClawConfig): number { return clampPositiveInteger( cfg.acp?.stream?.coalesceIdleMs, @@ -87,6 +97,7 @@ export function resolveAcpProjectionSettings(cfg: OpenClawConfig): AcpProjection const stream = cfg.acp?.stream; return { deliveryMode: resolveAcpDeliveryMode(stream?.deliveryMode), + hiddenBoundarySeparator: resolveAcpHiddenBoundarySeparator(stream?.hiddenBoundarySeparator), repeatSuppression: clampBoolean(stream?.repeatSuppression, DEFAULT_ACP_REPEAT_SUPPRESSION), maxTurnChars: clampPositiveInteger(stream?.maxTurnChars, DEFAULT_ACP_MAX_TURN_CHARS, { min: 1, diff --git a/src/config/schema.help.ts b/src/config/schema.help.ts index 44ae8bab3..447230c96 100644 --- a/src/config/schema.help.ts +++ b/src/config/schema.help.ts @@ -176,6 +176,8 @@ export const FIELD_HELP: Record = { "When true (default), suppress repeated ACP status/tool projection lines in a turn while keeping raw ACP events unchanged.", "acp.stream.deliveryMode": "ACP delivery style: live streams projected output incrementally, final_only buffers all projected ACP output until terminal turn events.", + "acp.stream.hiddenBoundarySeparator": + "Separator inserted before next visible assistant text when hidden ACP tool lifecycle events occurred (none|space|newline|paragraph).", "acp.stream.maxTurnChars": "Maximum assistant text characters projected per ACP turn before truncation notice is emitted.", "acp.stream.maxToolSummaryChars": diff --git a/src/config/schema.labels.ts b/src/config/schema.labels.ts index f13359d1e..045db4ebc 100644 --- a/src/config/schema.labels.ts +++ b/src/config/schema.labels.ts @@ -371,6 +371,7 @@ export const FIELD_LABELS: Record = { "acp.stream.maxChunkChars": "ACP Stream Max Chunk Chars", "acp.stream.repeatSuppression": "ACP Stream Repeat Suppression", "acp.stream.deliveryMode": "ACP Stream Delivery Mode", + "acp.stream.hiddenBoundarySeparator": "ACP Stream Hidden Boundary Separator", "acp.stream.maxTurnChars": "ACP Stream Max Turn Chars", "acp.stream.maxToolSummaryChars": "ACP Stream Max Tool Summary Chars", "acp.stream.maxStatusChars": "ACP Stream Max Status Chars", diff --git a/src/config/types.acp.ts b/src/config/types.acp.ts index fd4efa74c..6a89713a5 100644 --- a/src/config/types.acp.ts +++ b/src/config/types.acp.ts @@ -14,6 +14,8 @@ export type AcpStreamConfig = { repeatSuppression?: boolean; /** Live streams chunks or waits for terminal event before delivery. */ deliveryMode?: "live" | "final_only"; + /** Separator inserted before visible text when hidden tool events occurred. */ + hiddenBoundarySeparator?: "none" | "space" | "newline" | "paragraph"; /** Maximum assistant text characters forwarded per turn. */ maxTurnChars?: number; /** Maximum visible characters for tool summary/meta lines. */ diff --git a/src/config/zod-schema.ts b/src/config/zod-schema.ts index 181bcc2ba..6b7f63034 100644 --- a/src/config/zod-schema.ts +++ b/src/config/zod-schema.ts @@ -341,6 +341,14 @@ export const OpenClawSchema = z maxChunkChars: z.number().int().positive().optional(), repeatSuppression: z.boolean().optional(), deliveryMode: z.union([z.literal("live"), z.literal("final_only")]).optional(), + hiddenBoundarySeparator: z + .union([ + z.literal("none"), + z.literal("space"), + z.literal("newline"), + z.literal("paragraph"), + ]) + .optional(), maxTurnChars: z.number().int().positive().optional(), maxToolSummaryChars: z.number().int().positive().optional(), maxStatusChars: z.number().int().positive().optional(),