From e309a15d73270ad1a430211d306c4d3d64281c33 Mon Sep 17 00:00:00 2001 From: ql-wade Date: Sat, 7 Mar 2026 12:04:45 +0800 Subject: [PATCH] fix: suppress ACP NO_REPLY fragments in console output (#38436) --- CHANGELOG.md | 1 + src/commands/agent.acp.test.ts | 183 +++++++++++++++++++++++++++++++++ src/commands/agent.ts | 103 ++++++++++++++++--- 3 files changed, 275 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61c15908a..088f79dd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -222,6 +222,7 @@ Docs: https://docs.openclaw.ai - Google Chat/multi-account webhook auth fallback: when `channels.googlechat.accounts.default` carries shared webhook audience/path settings (for example after config normalization), inherit those defaults for named accounts while preserving top-level and per-account overrides, so inbound webhook verification no longer fails silently for named accounts missing duplicated audience fields. Fixes #38369. - Models/tool probing: raise the tool-capability probe budget from 32 to 256 tokens so reasoning models that spend tokens on thinking before returning a required tool call are less likely to be misclassified as not supporting tools. (#7521) Thanks @jakobdylanc. - Gateway/transient network classification: treat wrapped `...: fetch failed` transport messages as transient while avoiding broad matches like `Web fetch failed (404): ...`, preventing Discord reconnect wrappers from crashing the gateway without suppressing non-network tool failures. (#38530) Thanks @xinhuagu. +- ACP/console silent reply suppression: filter ACP `NO_REPLY` lead fragments and silent-only finals before `openclaw agent` logging/delivery so console-backed ACP sessions no longer leak `NO`/`NO_REPLY` placeholders. (#38436) Thanks @ql-wade. ## 2026.3.2 diff --git a/src/commands/agent.acp.test.ts b/src/commands/agent.acp.test.ts index cde0ab54a..d5dd4b8b7 100644 --- a/src/commands/agent.acp.test.ts +++ b/src/commands/agent.acp.test.ts @@ -7,6 +7,7 @@ import { AcpRuntimeError } from "../acp/runtime/errors.js"; import * as embeddedModule from "../agents/pi-embedded.js"; import type { OpenClawConfig } from "../config/config.js"; import * as configModule from "../config/config.js"; +import { onAgentEvent } from "../infra/agent-events.js"; import type { RuntimeEnv } from "../runtime.js"; import { agentCommand } from "./agent.js"; @@ -195,6 +196,188 @@ describe("agentCommand ACP runtime routing", () => { }); }); + it("suppresses ACP NO_REPLY lead fragments before emitting assistant text", async () => { + await withTempHome(async (home) => { + const storePath = path.join(home, "sessions.json"); + writeAcpSessionStore(storePath); + mockConfig(home, storePath); + + const assistantEvents: Array<{ text?: string; delta?: string }> = []; + const stop = onAgentEvent((evt) => { + if (evt.stream !== "assistant") { + return; + } + assistantEvents.push({ + text: typeof evt.data?.text === "string" ? evt.data.text : undefined, + delta: typeof evt.data?.delta === "string" ? evt.data.delta : undefined, + }); + }); + + const runTurn = vi.fn(async (paramsUnknown: unknown) => { + const params = paramsUnknown as { + onEvent?: (event: { type: string; text?: string; stopReason?: string }) => Promise; + }; + for (const text of ["NO", "NO_", "NO_RE", "NO_REPLY", "Actual answer"]) { + await params.onEvent?.({ type: "text_delta", text }); + } + await params.onEvent?.({ type: "done", stopReason: "stop" }); + }); + + mockAcpManager({ + runTurn: (params: unknown) => runTurn(params), + }); + + try { + await agentCommand({ message: "ping", sessionKey: "agent:codex:acp:test" }, runtime); + } finally { + stop(); + } + + expect(assistantEvents).toEqual([{ text: "Actual answer", delta: "Actual answer" }]); + + const logLines = vi.mocked(runtime.log).mock.calls.map(([first]) => String(first)); + expect(logLines.some((line) => line.includes("NO_REPLY"))).toBe(false); + expect(logLines.some((line) => line.includes("Actual answer"))).toBe(true); + }); + }); + + it("keeps silent-only ACP turns out of assistant output", async () => { + await withTempHome(async (home) => { + const storePath = path.join(home, "sessions.json"); + writeAcpSessionStore(storePath); + mockConfig(home, storePath); + + const assistantEvents: string[] = []; + const stop = onAgentEvent((evt) => { + if (evt.stream !== "assistant") { + return; + } + if (typeof evt.data?.text === "string") { + assistantEvents.push(evt.data.text); + } + }); + + const runTurn = vi.fn(async (paramsUnknown: unknown) => { + const params = paramsUnknown as { + onEvent?: (event: { type: string; text?: string; stopReason?: string }) => Promise; + }; + for (const text of ["NO", "NO_", "NO_RE", "NO_REPLY"]) { + await params.onEvent?.({ type: "text_delta", text }); + } + await params.onEvent?.({ type: "done", stopReason: "stop" }); + }); + + mockAcpManager({ + runTurn: (params: unknown) => runTurn(params), + }); + + try { + await agentCommand({ message: "ping", sessionKey: "agent:codex:acp:test" }, runtime); + } finally { + stop(); + } + + expect(assistantEvents).toEqual([]); + + const logLines = vi.mocked(runtime.log).mock.calls.map(([first]) => String(first)); + expect(logLines.some((line) => line.includes("NO_REPLY"))).toBe(false); + expect(logLines.some((line) => line.includes("No reply from agent."))).toBe(true); + }); + }); + + it("preserves repeated identical ACP delta chunks", async () => { + await withTempHome(async (home) => { + const storePath = path.join(home, "sessions.json"); + writeAcpSessionStore(storePath); + mockConfig(home, storePath); + + const assistantEvents: Array<{ text?: string; delta?: string }> = []; + const stop = onAgentEvent((evt) => { + if (evt.stream !== "assistant") { + return; + } + assistantEvents.push({ + text: typeof evt.data?.text === "string" ? evt.data.text : undefined, + delta: typeof evt.data?.delta === "string" ? evt.data.delta : undefined, + }); + }); + + const runTurn = vi.fn(async (paramsUnknown: unknown) => { + const params = paramsUnknown as { + onEvent?: (event: { type: string; text?: string; stopReason?: string }) => Promise; + }; + for (const text of ["b", "o", "o", "k"]) { + await params.onEvent?.({ type: "text_delta", text }); + } + await params.onEvent?.({ type: "done", stopReason: "stop" }); + }); + + mockAcpManager({ + runTurn: (params: unknown) => runTurn(params), + }); + + try { + await agentCommand({ message: "ping", sessionKey: "agent:codex:acp:test" }, runtime); + } finally { + stop(); + } + + expect(assistantEvents).toEqual([ + { text: "b", delta: "b" }, + { text: "bo", delta: "o" }, + { text: "boo", delta: "o" }, + { text: "book", delta: "k" }, + ]); + + const logLines = vi.mocked(runtime.log).mock.calls.map(([first]) => String(first)); + expect(logLines.some((line) => line.includes("book"))).toBe(true); + }); + }); + + it("re-emits buffered NO prefix when ACP text becomes visible content", async () => { + await withTempHome(async (home) => { + const storePath = path.join(home, "sessions.json"); + writeAcpSessionStore(storePath); + mockConfig(home, storePath); + + const assistantEvents: Array<{ text?: string; delta?: string }> = []; + const stop = onAgentEvent((evt) => { + if (evt.stream !== "assistant") { + return; + } + assistantEvents.push({ + text: typeof evt.data?.text === "string" ? evt.data.text : undefined, + delta: typeof evt.data?.delta === "string" ? evt.data.delta : undefined, + }); + }); + + const runTurn = vi.fn(async (paramsUnknown: unknown) => { + const params = paramsUnknown as { + onEvent?: (event: { type: string; text?: string; stopReason?: string }) => Promise; + }; + for (const text of ["NO", "W"]) { + await params.onEvent?.({ type: "text_delta", text }); + } + await params.onEvent?.({ type: "done", stopReason: "stop" }); + }); + + mockAcpManager({ + runTurn: (params: unknown) => runTurn(params), + }); + + try { + await agentCommand({ message: "ping", sessionKey: "agent:codex:acp:test" }, runtime); + } finally { + stop(); + } + + expect(assistantEvents).toEqual([{ text: "NOW", delta: "NOW" }]); + + const logLines = vi.mocked(runtime.log).mock.calls.map(([first]) => String(first)); + expect(logLines.some((line) => line.includes("NOW"))).toBe(true); + }); + }); + it("fails closed for ACP-shaped session keys missing ACP metadata", async () => { await withTempHome(async (home) => { const storePath = path.join(home, "sessions.json"); diff --git a/src/commands/agent.ts b/src/commands/agent.ts index fcbe593ec..10582521b 100644 --- a/src/commands/agent.ts +++ b/src/commands/agent.ts @@ -38,6 +38,7 @@ import { buildWorkspaceSkillSnapshot } from "../agents/skills.js"; import { getSkillsSnapshotVersion } from "../agents/skills/refresh.js"; import { resolveAgentTimeoutMs } from "../agents/timeout.js"; import { ensureAgentWorkspace } from "../agents/workspace.js"; +import { normalizeReplyPayload } from "../auto-reply/reply/normalize-reply.js"; import { formatThinkingLevels, formatXHighModelHint, @@ -47,6 +48,11 @@ import { type ThinkLevel, type VerboseLevel, } from "../auto-reply/thinking.js"; +import { + isSilentReplyPrefixText, + isSilentReplyText, + SILENT_REPLY_TOKEN, +} from "../auto-reply/tokens.js"; import { formatCliCommand } from "../cli/command-format.js"; import { resolveCommandSecretRefsViaGateway } from "../cli/command-secret-gateway.js"; import { getAgentRuntimeCommandSecretTargetIds } from "../cli/command-secret-targets.js"; @@ -148,6 +154,80 @@ function prependInternalEventContext( return [renderedEvents, body].filter(Boolean).join("\n\n"); } +function createAcpVisibleTextAccumulator() { + let pendingSilentPrefix = ""; + let visibleText = ""; + const startsWithWordChar = (chunk: string): boolean => /^[\p{L}\p{N}]/u.test(chunk); + + const resolveNextCandidate = (base: string, chunk: string): string => { + if (!base) { + return chunk; + } + if ( + isSilentReplyText(base, SILENT_REPLY_TOKEN) && + !chunk.startsWith(base) && + startsWithWordChar(chunk) + ) { + return chunk; + } + // Some ACP backends emit cumulative snapshots even on text_delta-style hooks. + // Accept those only when they strictly extend the buffered text. + if (chunk.startsWith(base) && chunk.length > base.length) { + return chunk; + } + return `${base}${chunk}`; + }; + + const mergeVisibleChunk = (base: string, chunk: string): { text: string; delta: string } => { + if (!base) { + return { text: chunk, delta: chunk }; + } + if (chunk.startsWith(base) && chunk.length > base.length) { + const delta = chunk.slice(base.length); + return { text: chunk, delta }; + } + return { + text: `${base}${chunk}`, + delta: chunk, + }; + }; + + return { + consume(chunk: string): { text: string; delta: string } | null { + if (!chunk) { + return null; + } + + if (!visibleText) { + const leadCandidate = resolveNextCandidate(pendingSilentPrefix, chunk); + const trimmedLeadCandidate = leadCandidate.trim(); + if ( + isSilentReplyText(trimmedLeadCandidate, SILENT_REPLY_TOKEN) || + isSilentReplyPrefixText(trimmedLeadCandidate, SILENT_REPLY_TOKEN) + ) { + pendingSilentPrefix = leadCandidate; + return null; + } + if (pendingSilentPrefix) { + pendingSilentPrefix = ""; + visibleText = leadCandidate; + return { + text: visibleText, + delta: leadCandidate, + }; + } + } + + const nextVisible = mergeVisibleChunk(visibleText, chunk); + visibleText = nextVisible.text; + return nextVisible.delta ? nextVisible : null; + }, + finalize(): string { + return visibleText.trim(); + }, + }; +} + function runAgentAttempt(params: { providerOverride: string; modelOverride: string; @@ -492,7 +572,7 @@ async function agentCommandInternal( }, }); - let streamedText = ""; + const visibleTextAccumulator = createAcpVisibleTextAccumulator(); let stopReason: string | undefined; try { const dispatchPolicyError = resolveAcpDispatchPolicyError(cfg); @@ -528,13 +608,16 @@ async function agentCommandInternal( if (!event.text) { return; } - streamedText += event.text; + const visibleUpdate = visibleTextAccumulator.consume(event.text); + if (!visibleUpdate) { + return; + } emitAgentEvent({ runId, stream: "assistant", data: { - text: streamedText, - delta: event.text, + text: visibleUpdate.text, + delta: visibleUpdate.delta, }, }); }, @@ -566,14 +649,10 @@ async function agentCommandInternal( }, }); - const finalText = streamedText.trim(); - const payloads = finalText - ? [ - { - text: finalText, - }, - ] - : []; + const normalizedFinalPayload = normalizeReplyPayload({ + text: visibleTextAccumulator.finalize(), + }); + const payloads = normalizedFinalPayload ? [normalizedFinalPayload] : []; const result = { payloads, meta: {