From a6d9926d1d179848b1b0d827edce88bfe634c1b0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 26 Apr 2026 02:58:19 +0100 Subject: [PATCH] fix: keep acp management commands local --- CHANGELOG.md | 3 + docs/tools/acp-agents.md | 6 + docs/tools/slash-commands.md | 5 + scripts/test-projects.test-support.mjs | 5 + src/auto-reply/reply/commands-acp.test.ts | 18 +++ src/auto-reply/reply/commands-acp.ts | 6 +- .../reply/dispatch-acp-command-bypass.test.ts | 131 +++++++++++++++++- .../reply/dispatch-acp-command-bypass.ts | 16 +++ test/scripts/test-projects.test.ts | 17 +++ 9 files changed, 199 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 720b203846e..9cf3fb31849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,9 @@ Docs: https://docs.openclaw.ai - Gateway/pairing: stop corrupt or unreadable device/node pairing stores from being treated as empty state, preserving `paired.json` for repair instead of overwriting approved pairings. Fixes #71873. Thanks @iret77. +- ACP: keep `/acp` management commands, plus local `/status` and `/unfocus`, + on the Gateway path inside ACP-bound threads so they are not consumed as ACP + prompt text. Fixes #66298. Thanks @kindomLee. - ACP: wait for the configured runtime backend to become healthy before startup identity reconciliation, avoiding transient acpx warnings during Gateway boot. Fixes #40566. diff --git a/docs/tools/acp-agents.md b/docs/tools/acp-agents.md index 0989ca40bc3..6d990f09f52 100644 --- a/docs/tools/acp-agents.md +++ b/docs/tools/acp-agents.md @@ -150,6 +150,10 @@ Notes: - `--bind here` only works on channels that advertise current-conversation binding; OpenClaw returns a clear unsupported message otherwise. Bindings persist across gateway restarts. - On Discord, `spawnAcpSessions` is only required when OpenClaw needs to create a child thread for `--thread auto|here` — not for `--bind here`. - If you spawn to a different ACP agent without `--cwd`, OpenClaw inherits the **target agent's** workspace by default. Missing inherited paths (`ENOENT`/`ENOTDIR`) fall back to the backend default; other access errors (e.g. `EACCES`) surface as spawn errors. +- Gateway management commands stay local in bound conversations. In + particular, `/acp ...` commands are handled by OpenClaw even when normal + follow-up text routes to the bound ACP session; `/status` and `/unfocus` also + stay local whenever command handling is enabled for that surface. ### Thread-bound sessions @@ -159,6 +163,8 @@ When thread bindings are enabled for a channel adapter, ACP sessions can be boun - Follow-up messages in that thread route to the bound ACP session. - ACP output is delivered back to the same thread. - Unfocus/close/archive/idle-timeout or max-age expiry removes the binding. +- `/acp close`, `/acp cancel`, `/acp status`, `/status`, and `/unfocus` are + Gateway commands, not prompts to the ACP harness. Thread binding support is adapter-specific. If the active channel adapter does not support thread bindings, OpenClaw returns a clear unsupported/unavailable message. diff --git a/docs/tools/slash-commands.md b/docs/tools/slash-commands.md index 41610f50780..eeaf42f858b 100644 --- a/docs/tools/slash-commands.md +++ b/docs/tools/slash-commands.md @@ -9,6 +9,11 @@ title: "Slash commands" Commands are handled by the Gateway. Most commands must be sent as a **standalone** message that starts with `/`. The host-only bash chat command uses `! ` (with `/bash ` as an alias). +When a conversation or thread is bound to an ACP session, normal follow-up text +routes to that ACP harness. Gateway management commands still stay local: +`/acp ...` always reaches the OpenClaw ACP command handler, and `/status` plus +`/unfocus` stay local whenever command handling is enabled for the surface. + There are two related systems: - **Commands**: standalone `/...` messages. diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index dc06c8fd6d5..a17c6835aa5 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -290,6 +290,11 @@ const SOURCE_TEST_TARGETS = new Map([ "src/auto-reply/reply/dispatch-from-config.test.ts", ], ], + ["src/auto-reply/reply/commands-acp.ts", ["src/auto-reply/reply/commands-acp.test.ts"]], + [ + "src/auto-reply/reply/dispatch-acp-command-bypass.ts", + ["src/auto-reply/reply/dispatch-acp-command-bypass.test.ts"], + ], ]); const GENERATED_CHANGED_TEST_TARGETS = new Set([ "src/canvas-host/a2ui/.bundle.hash", diff --git a/src/auto-reply/reply/commands-acp.test.ts b/src/auto-reply/reply/commands-acp.test.ts index f9405b3bd65..cd08849f580 100644 --- a/src/auto-reply/reply/commands-acp.test.ts +++ b/src/auto-reply/reply/commands-acp.test.ts @@ -1637,6 +1637,24 @@ describe("/acp command", () => { expect(result?.reply?.text).toContain("Removed 1 binding"); }); + it("handles /acp close in a bound thread when text commands are disabled", async () => { + mockBoundThreadSession(); + hoisted.sessionBindingUnbindMock.mockResolvedValue([ + createBoundThreadSession() as SessionBindingRecord, + ]); + + const result = await handleAcpCommand(createThreadParams("/acp close", baseCfg), false); + + expect(hoisted.closeMock).toHaveBeenCalledTimes(1); + expect(hoisted.sessionBindingUnbindMock).toHaveBeenCalledWith( + expect.objectContaining({ + targetSessionKey: defaultAcpSessionKey, + reason: "manual", + }), + ); + expect(result?.reply?.text).toContain("Removed 1 binding"); + }); + it("lists ACP sessions from the session store", async () => { hoisted.sessionBindingListBySessionMock.mockImplementation((key: string) => key === defaultAcpSessionKey ? [createBoundThreadSession(key) as SessionBindingRecord] : [], diff --git a/src/auto-reply/reply/commands-acp.ts b/src/auto-reply/reply/commands-acp.ts index bcd9328833b..d1b9368d036 100644 --- a/src/auto-reply/reply/commands-acp.ts +++ b/src/auto-reply/reply/commands-acp.ts @@ -85,11 +85,7 @@ const ACP_MUTATING_ACTIONS = new Set([ "reset-options", ]); -export const handleAcpCommand: CommandHandler = async (params, allowTextCommands) => { - if (!allowTextCommands) { - return null; - } - +export const handleAcpCommand: CommandHandler = async (params, _allowTextCommands) => { const normalized = params.command.commandBodyNormalized; if (!normalized.startsWith(COMMAND)) { return null; diff --git a/src/auto-reply/reply/dispatch-acp-command-bypass.test.ts b/src/auto-reply/reply/dispatch-acp-command-bypass.test.ts index 34bdc681133..e172d84ae8e 100644 --- a/src/auto-reply/reply/dispatch-acp-command-bypass.test.ts +++ b/src/auto-reply/reply/dispatch-acp-command-bypass.test.ts @@ -1,9 +1,18 @@ -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../config/config.js"; +import { setActivePluginRegistry } from "../../plugins/runtime.js"; +import { + createChannelTestPluginBase, + createTestRegistry, +} from "../../test-utils/channel-plugins.js"; import { shouldBypassAcpDispatchForCommand } from "./dispatch-acp-command-bypass.js"; import { buildTestCtx } from "./test-ctx.js"; describe("shouldBypassAcpDispatchForCommand", () => { + beforeEach(() => { + setActivePluginRegistry(createTestRegistry([])); + }); + it("returns false for plain-text ACP turns", () => { const ctx = buildTestCtx({ Provider: "discord", @@ -15,7 +24,7 @@ describe("shouldBypassAcpDispatchForCommand", () => { expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(false); }); - it("returns false for ACP slash commands", () => { + it("returns true for ACP slash commands", () => { const ctx = buildTestCtx({ Provider: "discord", Surface: "discord", @@ -24,9 +33,58 @@ describe("shouldBypassAcpDispatchForCommand", () => { BodyForAgent: "/acp cancel", }); + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(true); + }); + + it("returns true for native ACP slash commands", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandSource: "native", + CommandBody: "/acp close", + BodyForCommands: "/acp close", + BodyForAgent: "/acp close", + }); + + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(true); + }); + + it("returns false for ACP slash commands addressed to another bot", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/acp@otherbot cancel", + BodyForCommands: "/acp@otherbot cancel", + BodyForAgent: "/acp@otherbot cancel", + }); + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(false); }); + it("returns true for local status commands", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/status", + BodyForCommands: "/status", + BodyForAgent: "/status", + }); + + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(true); + }); + + it("returns true for local unfocus commands", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/unfocus", + BodyForCommands: "/unfocus", + BodyForAgent: "/unfocus", + }); + + expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(true); + }); + it("returns true for ACP reset-tail slash commands", () => { const ctx = buildTestCtx({ Provider: "discord", @@ -52,7 +110,25 @@ describe("shouldBypassAcpDispatchForCommand", () => { expect(shouldBypassAcpDispatchForCommand(ctx, {} as OpenClawConfig)).toBe(true); }); - it("returns false for slash commands when text commands are disabled", () => { + it("returns false for unrelated slash commands when text commands are disabled", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/foo cancel", + BodyForCommands: "/foo cancel", + BodyForAgent: "/foo cancel", + CommandSource: "text", + }); + const cfg = { + commands: { + text: false, + }, + } as OpenClawConfig; + + expect(shouldBypassAcpDispatchForCommand(ctx, cfg)).toBe(false); + }); + + it("returns true for ACP slash commands when text commands are disabled", () => { const ctx = buildTestCtx({ Provider: "discord", Surface: "discord", @@ -67,9 +143,58 @@ describe("shouldBypassAcpDispatchForCommand", () => { }, } as OpenClawConfig; + expect(shouldBypassAcpDispatchForCommand(ctx, cfg)).toBe(true); + }); + + it("returns false for local status commands when text commands are disabled on text-native surfaces", () => { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "discord", + plugin: createChannelTestPluginBase({ + id: "discord", + capabilities: { nativeCommands: true, chatTypes: ["direct"] }, + }), + source: "test", + }, + ]), + ); + + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/status", + BodyForCommands: "/status", + BodyForAgent: "/status", + CommandSource: "text", + }); + const cfg = { + commands: { + text: false, + }, + } as OpenClawConfig; + expect(shouldBypassAcpDispatchForCommand(ctx, cfg)).toBe(false); }); + it("returns true for native local status commands when text commands are disabled", () => { + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + CommandBody: "/status", + BodyForCommands: "/status", + BodyForAgent: "/status", + CommandSource: "native", + }); + const cfg = { + commands: { + text: false, + }, + } as OpenClawConfig; + + expect(shouldBypassAcpDispatchForCommand(ctx, cfg)).toBe(true); + }); + it("returns false for unauthorized bang-prefixed commands", () => { const ctx = buildTestCtx({ Provider: "discord", diff --git a/src/auto-reply/reply/dispatch-acp-command-bypass.ts b/src/auto-reply/reply/dispatch-acp-command-bypass.ts index 62c045d8bfd..0399e9cf409 100644 --- a/src/auto-reply/reply/dispatch-acp-command-bypass.ts +++ b/src/auto-reply/reply/dispatch-acp-command-bypass.ts @@ -27,6 +27,14 @@ function isResetCommandCandidate(text: string): boolean { return /^\/(?:new|reset)(?:\s|$)/i.test(text); } +function isAcpCommandCandidate(text: string): boolean { + return /^\/acp(?:\s|$)/i.test(text); +} + +function isLocalCommandCandidate(text: string): boolean { + return /^\/(?:status|unfocus)(?:\s|$)/i.test(text); +} + export function shouldBypassAcpDispatchForCommand( ctx: FinalizedMsgContext, cfg: OpenClawConfig, @@ -49,6 +57,14 @@ export function shouldBypassAcpDispatchForCommand( return true; } + if (isAcpCommandCandidate(normalized)) { + return true; + } + + if (isLocalCommandCandidate(normalized)) { + return allowTextCommands; + } + if (!normalized.startsWith("!")) { return false; } diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 26889c3c1f7..dfe59c7fb9c 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -302,6 +302,23 @@ describe("scripts/test-projects changed-target routing", () => { }); }); + it("routes ACP command source files to ACP command regression tests", () => { + expect( + resolveChangedTestTargetPlan([ + "src/auto-reply/reply/commands-acp.ts", + "src/auto-reply/reply/commands-acp.test.ts", + "src/auto-reply/reply/dispatch-acp-command-bypass.ts", + "src/auto-reply/reply/dispatch-acp-command-bypass.test.ts", + ]), + ).toEqual({ + mode: "targets", + targets: [ + "src/auto-reply/reply/commands-acp.test.ts", + "src/auto-reply/reply/dispatch-acp-command-bypass.test.ts", + ], + }); + }); + it("routes Google Meet CLI edits to the lightweight CLI tests", () => { expect(resolveChangedTestTargetPlan(["extensions/google-meet/src/cli.ts"])).toEqual({ mode: "targets",