diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 503edca6..01d5d92b 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -51,6 +51,7 @@ import { ModelUsage, Options, PermissionMode, + PermissionUpdate, Query, query, Settings, @@ -305,6 +306,56 @@ const ALLOW_BYPASS = !IS_ROOT || !!process.env.IS_SANDBOX; // message and without invoking the model. const LOCAL_ONLY_COMMANDS = new Set(["/context", "/heapdump", "/extra-usage"]); +// The Claude SDK persists local slash command invocations (e.g. `/model`) and +// their output as user messages in the session transcript, wrapping the +// payload in these XML-like markers that the CLI uses for its own display. +// The live prompt loop drops them; replay must strip them too or they leak +// into the UI on session/load. +const LOCAL_COMMAND_TAG_PATTERN = + /<(command-name|command-message|command-args|local-command-stdout|local-command-stderr)>[\s\S]*?<\/\1>/g; + +function stripMarkerTags(text: string): string { + return text.replace(LOCAL_COMMAND_TAG_PATTERN, ""); +} + +/** + * Return user-message content with local-command marker tags removed, or + * `null` if nothing meaningful remains (caller should skip the message). + * Preserves real prose that's mixed in alongside the markers — e.g. a + * message like `…hi` becomes `hi`. + */ +export function stripLocalCommandMetadata(content: unknown): unknown | null { + if (typeof content === "string") { + const stripped = stripMarkerTags(content); + return stripped.trim() === "" ? null : stripped; + } + if (!Array.isArray(content)) return content; + + const kept: unknown[] = []; + for (const block of content) { + if ( + block && + typeof block === "object" && + "type" in block && + (block as { type: unknown }).type === "text" && + "text" in block && + typeof (block as { text: unknown }).text === "string" + ) { + const stripped = stripMarkerTags((block as { text: string }).text); + if (stripped.trim() === "") continue; + kept.push({ ...(block as object), text: stripped }); + } else { + kept.push(block); + } + } + if (kept.length === 0) return null; + return kept; +} + +export function isLocalCommandMetadata(content: unknown): boolean { + return stripLocalCommandMetadata(content) === null; +} + const PERMISSION_MODE_ALIASES: Record = { auto: "auto", default: "default", @@ -343,6 +394,50 @@ export function resolvePermissionMode(defaultMode?: unknown): PermissionMode { return mapped; } +/** + * Builds the label for the "Always Allow" permission option so the user can see + * the exact scope they are committing to. Uses the SDK-provided suggestions + * when available (e.g. `Bash(npm test:*)`) and falls back to naming the whole + * tool so "Always Allow" is never a blank check without disclosure. + */ +export function describeAlwaysAllow( + suggestions: PermissionUpdate[] | undefined, + toolName: string, +): string { + if (!suggestions || suggestions.length === 0) { + return `Always Allow all ${toolName}`; + } + + const ruleLabels: string[] = []; + const directories: string[] = []; + + for (const update of suggestions) { + if (update.type === "addRules" && update.behavior === "allow") { + for (const rule of update.rules) { + ruleLabels.push( + rule.ruleContent ? `${rule.toolName}(${rule.ruleContent})` : `all ${rule.toolName}`, + ); + } + } else if (update.type === "addDirectories") { + directories.push(...update.directories); + } + } + + const parts: string[] = []; + if (ruleLabels.length > 0) { + parts.push(ruleLabels.join(", ")); + } + if (directories.length > 0) { + parts.push(`access to ${directories.join(", ")}`); + } + + if (parts.length === 0) { + return `Always Allow all ${toolName}`; + } + + return `Always Allow ${parts.join(" and ")}`; +} + // Implement the ACP Agent interface export class ClaudeAcpAgent implements Agent { sessions: { @@ -1225,9 +1320,17 @@ export class ClaudeAcpAgent implements Agent { const messages = await getSessionMessages(sessionId); for (const message of messages) { + // @ts-expect-error - untyped in SDK but we handle all of these + let content: unknown = message.message.content; + // @ts-expect-error - untyped in SDK but we handle all of these + if (message.message.role === "user") { + content = stripLocalCommandMetadata(content); + if (content === null) continue; + } + for (const notification of toAcpNotifications( // @ts-expect-error - untyped in SDK but we handle all of these - message.message.content, + content, // @ts-expect-error - untyped in SDK but we handle all of these message.message.role, sessionId, @@ -1257,6 +1360,7 @@ export class ClaudeAcpAgent implements Agent { canUseTool(sessionId: string): CanUseTool { return async (toolName, toolInput, { signal, suggestions, toolUseID }) => { + const alwaysAllowLabel = describeAlwaysAllow(suggestions, toolName); const supportsTerminalOutput = this.clientCapabilities?._meta?.["terminal_output"] === true; const session = this.sessions[sessionId]; if (!session) { @@ -1347,7 +1451,7 @@ export class ClaudeAcpAgent implements Agent { options: [ { kind: "allow_always", - name: "Always Allow", + name: alwaysAllowLabel, optionId: "allow_always", }, { kind: "allow_once", name: "Allow", optionId: "allow" }, diff --git a/src/lib.ts b/src/lib.ts index f3cc5426..fd93fbab 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -1,6 +1,8 @@ // Export the main agent class and utilities for library usage export { ClaudeAcpAgent, + isLocalCommandMetadata, + stripLocalCommandMetadata, runAcp, toAcpNotifications, streamEventToAcpNotifications, diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 6b0bd036..1174b1ea 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -24,7 +24,15 @@ import { toolUpdateFromToolResult, toolUpdateFromEditToolResponse, } from "../tools.js"; -import { toAcpNotifications, promptToClaude, ClaudeAcpAgent, claudeCliPath } from "../acp-agent.js"; +import { + toAcpNotifications, + promptToClaude, + isLocalCommandMetadata, + stripLocalCommandMetadata, + ClaudeAcpAgent, + claudeCliPath, + describeAlwaysAllow, +} from "../acp-agent.js"; import { Pushable } from "../utils.js"; import { query, SDKAssistantMessage } from "@anthropic-ai/claude-agent-sdk"; import { randomUUID } from "crypto"; @@ -1110,6 +1118,113 @@ describe("toolUpdateFromEditToolResponse", () => { }); }); +describe("stripLocalCommandMetadata", () => { + it("returns null for strings that are pure marker metadata", () => { + expect(stripLocalCommandMetadata("/model")).toBeNull(); + expect( + stripLocalCommandMetadata("out"), + ).toBeNull(); + expect( + stripLocalCommandMetadata("err"), + ).toBeNull(); + expect( + stripLocalCommandMetadata( + "/model\n model\n opus", + ), + ).toBeNull(); + }); + + it("returns the string unchanged for real content", () => { + expect(stripLocalCommandMetadata("hi")).toBe("hi"); + expect(stripLocalCommandMetadata("please run /model with args")).toBe( + "please run /model with args", + ); + }); + + // Regression: in the original bug report the entire /model preamble and + // the user's real "hi" prompt were concatenated into a single message. + // We want to strip the marker tags and preserve the real prose, not drop + // the whole message. + it("strips marker tags from mixed-content strings, preserving real prose", () => { + const mixed = + "/model\n model\n opus" + + "Set model to opus (claude-opus-4-7)" + + "/model\n model\n opus[1m]" + + "Set model to opus[1m] (claude-opus-4-7[1m])" + + "hi"; + const stripped = stripLocalCommandMetadata(mixed); + expect(typeof stripped).toBe("string"); + expect(stripped as string).not.toContain(""); + expect(stripped as string).not.toContain(""); + expect(stripped as string).not.toContain(""); + expect(stripped as string).not.toContain(""); + expect((stripped as string).trimEnd()).toMatch(/hi$/); + }); + + it("drops marker-only blocks from mixed arrays, keeping real blocks", () => { + const result = stripLocalCommandMetadata([ + { type: "text", text: "/model" }, + { type: "text", text: "ok" }, + { type: "text", text: "hi" }, + ]); + expect(result).toEqual([{ type: "text", text: "hi" }]); + }); + + it("returns null when every block is a marker", () => { + expect( + stripLocalCommandMetadata([ + { type: "text", text: "/model" }, + { type: "text", text: "ok" }, + ]), + ).toBeNull(); + }); + + it("strips tags inside a text block while keeping the trailing prose", () => { + const result = stripLocalCommandMetadata([ + { + type: "text", + text: "/modelokhi", + }, + ]); + expect(result).toEqual([{ type: "text", text: "hi" }]); + }); + + it("leaves non-text blocks alone", () => { + const image = { type: "image", source: { type: "base64", data: "", media_type: "image/png" } }; + const result = stripLocalCommandMetadata([ + { type: "text", text: "/model" }, + image, + ]); + expect(result).toEqual([image]); + }); + + it("handles null/undefined/non-container shapes", () => { + expect(stripLocalCommandMetadata(null)).toBeNull(); + expect(stripLocalCommandMetadata(undefined)).toBeUndefined(); + expect(stripLocalCommandMetadata({ arbitrary: "object" })).toEqual({ arbitrary: "object" }); + }); +}); + +describe("isLocalCommandMetadata", () => { + it("is true when stripping leaves nothing", () => { + expect(isLocalCommandMetadata("/model")).toBe(true); + expect( + isLocalCommandMetadata([{ type: "text", text: "/model" }]), + ).toBe(true); + }); + + it("is false when real content survives stripping", () => { + expect(isLocalCommandMetadata("hi")).toBe(false); + expect(isLocalCommandMetadata("/modelhi")).toBe(false); + expect( + isLocalCommandMetadata([ + { type: "text", text: "/model" }, + { type: "text", text: "hi" }, + ]), + ).toBe(false); + }); +}); + describe("escape markdown", () => { it("should escape markdown characters", () => { let text = "Hello *world*!"; @@ -1254,6 +1369,81 @@ describe("permission requests", () => { expect(Array.isArray(requestStructure.toolCall.content)).toBe(true); } }); + + describe("describeAlwaysAllow", () => { + it("falls back to naming the whole tool when no suggestions are provided", () => { + expect(describeAlwaysAllow(undefined, "Bash")).toBe("Always Allow all Bash"); + expect(describeAlwaysAllow([], "Read")).toBe("Always Allow all Read"); + }); + + it("includes the scoped rule content from a suggestion", () => { + const label = describeAlwaysAllow( + [ + { + type: "addRules", + rules: [{ toolName: "Bash", ruleContent: "npm test:*" }], + behavior: "allow", + destination: "session", + }, + ], + "Bash", + ); + expect(label).toBe("Always Allow Bash(npm test:*)"); + }); + + it("indicates a tool-wide rule when the suggestion has no ruleContent", () => { + const label = describeAlwaysAllow( + [ + { + type: "addRules", + rules: [{ toolName: "Read" }], + behavior: "allow", + destination: "session", + }, + ], + "Read", + ); + expect(label).toBe("Always Allow all Read"); + }); + + it("joins multiple rules and directory suggestions", () => { + const label = describeAlwaysAllow( + [ + { + type: "addRules", + rules: [ + { toolName: "Bash", ruleContent: "git status" }, + { toolName: "Bash", ruleContent: "git diff:*" }, + ], + behavior: "allow", + destination: "session", + }, + { + type: "addDirectories", + directories: ["/tmp/work"], + destination: "session", + }, + ], + "Bash", + ); + expect(label).toBe("Always Allow Bash(git status), Bash(git diff:*) and access to /tmp/work"); + }); + + it("ignores non-allow rules and falls back when nothing is left", () => { + const label = describeAlwaysAllow( + [ + { + type: "addRules", + rules: [{ toolName: "Bash", ruleContent: "rm -rf:*" }], + behavior: "deny", + destination: "session", + }, + ], + "Bash", + ); + expect(label).toBe("Always Allow all Bash"); + }); + }); }); describe("stop reason propagation", () => { diff --git a/src/tests/session-load.test.ts b/src/tests/session-load.test.ts index ba4c31de..7b3fb57e 100644 --- a/src/tests/session-load.test.ts +++ b/src/tests/session-load.test.ts @@ -145,4 +145,189 @@ describe.skipIf(!process.env.RUN_INTEGRATION_TESTS)("session load/resume lifecyc await agentB.dispose(); } }, 30000); + + // Regression test for https://github.com/zed-industries/claude-code-acp/issues/579 + // The client (Zed) renders its own local slash commands — e.g. `/model` — + // by injecting user-message prompts whose text is wrapped in + // `` / `` markers. The Claude SDK + // persists those messages verbatim in the session transcript. Without + // filtering, `loadSession` replays the markers as user_message_chunks + // ahead of the real prompt, leaking CLI internals into the transcript. + it("ACP: loadSession does not replay local-command metadata user messages", async () => { + const recordedUserChunks: string[] = []; + const client = { + sessionUpdate: async (notification: SessionNotification) => { + if (notification.update.sessionUpdate === "user_message_chunk") { + const content = notification.update.content; + if (content.type === "text") recordedUserChunks.push(content.text); + } + }, + requestPermission: async () => ({ outcome: { outcome: "cancelled" } }), + readTextFile: async () => ({ content: "" }), + writeTextFile: async () => ({}), + } as unknown as AgentSideConnection; + + const commandName = + "/model\n model\n opus"; + const commandStdout = + "Set model to opus (claude-opus-4-7)"; + + // Step 1: create a session via the SDK, push the marker user messages + // followed by a real "hi" prompt. We bypass the ACP agent's prompt loop + // so we're only testing what the loadSession replay path does, not how + // prompts get routed. + const sessionId = randomUUID(); + const input = new Pushable(); + const q = query({ + prompt: input, + options: { + systemPrompt: { type: "preset", preset: "claude_code" }, + sessionId, + settingSources: ["user", "project", "local"], + includePartialMessages: true, + }, + }); + await q.initializationResult(); + + // Zed renders its local slash commands by mixing the marker blocks into + // the user's prompt content array alongside the real text. The Claude + // SDK persists the message verbatim — including the marker blocks — + // which is what loadSession must filter when it replays. + input.push({ + type: "user", + message: { + role: "user", + content: [ + { type: "text", text: commandName }, + { type: "text", text: commandStdout }, + { type: "text", text: "hi" }, + ], + }, + session_id: sessionId, + parent_tool_use_id: null, + }); + + for await (const msg of q) { + if (msg.type === "result") break; + } + input.end(); + q.return(undefined); + + // Sanity check: the SDK transcript contains the metadata we're filtering. + const sdkMessages = await getSessionMessages(sessionId); + const sdkTexts = sdkMessages + .map((m) => { + const c = (m as { message?: { content?: unknown } }).message?.content; + if (typeof c === "string") return c; + if (Array.isArray(c)) + return c + .map((b) => + b && typeof b === "object" && "text" in b + ? String((b as { text: unknown }).text) + : "", + ) + .join(""); + return ""; + }) + .join("\n"); + expect(sdkTexts).toContain(""); + expect(sdkTexts).toContain(""); + expect(sdkTexts).toContain("hi"); + + // Step 2: load the session through the ACP agent and confirm the markers + // never reach the client as user_message_chunks, while the real "hi" + // prompt does. + const agent = new ClaudeAcpAgent(client); + try { + await agent.loadSession({ + sessionId, + cwd: process.cwd(), + mcpServers: [], + }); + } finally { + await agent.dispose(); + } + + for (const chunk of recordedUserChunks) { + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + } + expect(recordedUserChunks.some((c) => c.includes("hi"))).toBe(true); + }, 60000); + + // Second regression: the original issue showed the markers and the real + // "hi" prompt all concatenated into a single string ending with "hi". + // loadSession must strip the marker tags in-place rather than dropping the + // whole message. + it("ACP: loadSession strips marker tags from a concatenated single-string prompt", async () => { + const recordedUserChunks: string[] = []; + const client = { + sessionUpdate: async (notification: SessionNotification) => { + if (notification.update.sessionUpdate === "user_message_chunk") { + const content = notification.update.content; + if (content.type === "text") recordedUserChunks.push(content.text); + } + }, + requestPermission: async () => ({ outcome: { outcome: "cancelled" } }), + readTextFile: async () => ({ content: "" }), + writeTextFile: async () => ({}), + } as unknown as AgentSideConnection; + + // Exact shape from https://github.com/zed-industries/claude-code-acp/issues/579. + const concatenated = + "/model\n model\n opus" + + "Set model to opus (claude-opus-4-7)\n" + + "/model\n model\n opus[1m]" + + "Set model to opus[1m] (claude-opus-4-7[1m])" + + "hi"; + + const sessionId = randomUUID(); + const input = new Pushable(); + const q = query({ + prompt: input, + options: { + systemPrompt: { type: "preset", preset: "claude_code" }, + sessionId, + settingSources: ["user", "project", "local"], + includePartialMessages: true, + }, + }); + await q.initializationResult(); + + input.push({ + type: "user", + message: { role: "user", content: concatenated }, + session_id: sessionId, + parent_tool_use_id: null, + }); + for await (const msg of q) { + if (msg.type === "result") break; + } + input.end(); + q.return(undefined); + + const agent = new ClaudeAcpAgent(client); + try { + await agent.loadSession({ + sessionId, + cwd: process.cwd(), + mcpServers: [], + }); + } finally { + await agent.dispose(); + } + + expect(recordedUserChunks.length).toBeGreaterThan(0); + for (const chunk of recordedUserChunks) { + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + expect(chunk).not.toContain(""); + } + expect(recordedUserChunks.some((c) => c.includes("hi"))).toBe(true); + }, 60000); });