From d8e38bcc627d8d8ee82c6d7a15316a4dece29199 Mon Sep 17 00:00:00 2001 From: Ogrodev Date: Mon, 11 May 2026 12:32:00 -0300 Subject: [PATCH] test(acp): add comprehensive ACP and edit tool test suites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AcpAgent integration tests: initialize→run→notify cycle, notification structure, message-ID continuity - ACP built-in slash command unit tests across all command handlers via fake runtime - ACP event mapper unit tests: text chunks, thinking, tool calls/results, errors → SessionNotification - ACP initialize conformance: terminal auth capability negotiation, agentInfo/agentCapabilities contract - stdout hygiene smoke test: first bytes on omp acp stdout must be a valid JSON-RPC frame - AgentSession permission gate: allow-once, reject-once, allow-always, abort-during-pending, non-gated tools - BashTool ACP terminal routing: bridge dispatch vs local PTY fallback - EditTool diff content propagation: patch, replace, and multi-file aggregation paths - ReadTool and WriteTool ACP filesystem permission tests - Shared ACP schema test helper --- packages/coding-agent/test/acp-agent.test.ts | 295 +++++- .../coding-agent/test/acp-builtins.test.ts | 836 ++++++++++++++++++ .../test/acp-event-mapper.test.ts | 216 +++++ .../test/acp-initialize-conformance.test.ts | 247 ++++++ .../test/acp-stdout-hygiene.test.ts | 109 +++ .../test/agent-session-acp-permission.test.ts | 242 +++++ .../test/bash-acp-terminal.test.ts | 105 +++ .../test/edit-per-file-diff-content.test.ts | 164 ++++ .../coding-agent/test/helpers/acp-schema.ts | 16 + .../coding-agent/test/read-acp-fs.test.ts | 113 +++ .../coding-agent/test/write-acp-fs.test.ts | 62 ++ 11 files changed, 2403 insertions(+), 2 deletions(-) create mode 100644 packages/coding-agent/test/acp-builtins.test.ts create mode 100644 packages/coding-agent/test/acp-initialize-conformance.test.ts create mode 100644 packages/coding-agent/test/acp-stdout-hygiene.test.ts create mode 100644 packages/coding-agent/test/agent-session-acp-permission.test.ts create mode 100644 packages/coding-agent/test/bash-acp-terminal.test.ts create mode 100644 packages/coding-agent/test/edit-per-file-diff-content.test.ts create mode 100644 packages/coding-agent/test/helpers/acp-schema.ts create mode 100644 packages/coding-agent/test/read-acp-fs.test.ts create mode 100644 packages/coding-agent/test/write-acp-fs.test.ts diff --git a/packages/coding-agent/test/acp-agent.test.ts b/packages/coding-agent/test/acp-agent.test.ts index 64a96930d..115f7d3af 100644 --- a/packages/coding-agent/test/acp-agent.test.ts +++ b/packages/coding-agent/test/acp-agent.test.ts @@ -3,11 +3,21 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import type { AgentSideConnection, PromptRequest, SessionNotification } from "@agentclientprotocol/sdk"; +import { + zForkSessionResponse, + zLoadSessionResponse, + zNewSessionResponse, + zPromptResponse, + zSessionNotification, +} from "@agentclientprotocol/sdk/dist/schema/zod.gen.js"; import type { Model } from "@oh-my-pi/pi-ai"; import { getConfigRootDir, setAgentDir } from "@oh-my-pi/pi-utils"; +import { _resetSettingsForTest, Settings } from "../src/config/settings"; import { AcpAgent } from "../src/modes/acp/acp-agent"; +import type { PlanModeState } from "../src/plan-mode/state"; import type { AgentSession, AgentSessionEvent } from "../src/session/agent-session"; import { SessionManager } from "../src/session/session-manager"; +import { expectAcpStructure } from "./helpers/acp-schema"; const TEST_MODELS: Model[] = [ { @@ -74,6 +84,13 @@ class FakeAgentSession { queuedMessageCount = 0; systemPrompt = "system"; disposed = false; + fastMode = false; + forcedToolChoice: string | undefined; + promptCalls: string[] = []; + customMessages: Array<{ customType: string; content: string; details?: unknown }> = []; + skillsSettings = { enableSkillCommands: true }; + skills: Array<{ name: string; description: string; filePath: string; baseDir: string; source: string }> = []; + planModeState: PlanModeState | undefined; #listeners = new Set<(event: AgentSessionEvent) => void>(); constructor( @@ -123,6 +140,7 @@ class FakeAgentSession { } async prompt(text: string): Promise { + this.promptCalls.push(text); this.isStreaming = true; this.sessionManager.appendMessage({ role: "user", content: text, timestamp: Date.now() }); const assistantMessage = makeAssistantMessage("pong"); @@ -147,6 +165,27 @@ class FakeAgentSession { this.isStreaming = false; } + async promptCustomMessage(message: { customType: string; content: string; details?: unknown }): Promise { + this.customMessages.push(message); + this.isStreaming = true; + const assistantMessage = makeAssistantMessage("skill pong"); + for (const listener of this.#listeners) { + listener({ + type: "message_update", + message: assistantMessage, + assistantMessageEvent: { type: "text_delta", delta: "skill pong" }, + } as AgentSessionEvent); + } + this.sessionManager.appendMessage(assistantMessage); + for (const listener of this.#listeners) { + listener({ + type: "agent_end", + messages: [assistantMessage], + } as AgentSessionEvent); + } + this.isStreaming = false; + } + async refreshMCPTools(_tools: unknown[]): Promise {} getContextUsage(): undefined { @@ -192,6 +231,37 @@ class FakeAgentSession { setActiveToolsByName(_toolNames: string[]): void {} + setClientBridge(_bridge: unknown): void {} + + getPlanModeState(): PlanModeState | undefined { + return this.planModeState; + } + + setPlanModeState(state: PlanModeState | undefined): void { + this.planModeState = state; + } + + getToolByName(_name: string): undefined { + return undefined; + } + + toggleFastMode(): boolean { + this.fastMode = !this.fastMode; + return this.fastMode; + } + + setFastMode(enabled: boolean): void { + this.fastMode = enabled; + } + + isFastModeEnabled(): boolean { + return this.fastMode; + } + + setForcedToolChoice(toolName: string): void { + this.forcedToolChoice = toolName; + } + async sendCustomMessage(_message: string, _options?: unknown): Promise {} async sendUserMessage(_content: string, _options?: unknown): Promise {} @@ -225,6 +295,12 @@ function getChunkMessageId(notification: SessionNotification): string | undefine return typeof update.messageId === "string" ? update.messageId : undefined; } +function expectAcpNotifications(updates: SessionNotification[]): void { + for (const update of updates) { + expectAcpStructure(zSessionNotification, update); + } +} + const cleanupRoots: string[] = []; const originalAgentDir = process.env.PI_CODING_AGENT_DIR; const fallbackAgentDir = path.join(getConfigRootDir(), "agent"); @@ -236,6 +312,7 @@ afterEach(async () => { setAgentDir(fallbackAgentDir); delete process.env.PI_CODING_AGENT_DIR; } + _resetSettingsForTest(); for (const root of cleanupRoots.splice(0)) { await fs.promises.rm(root, { recursive: true, force: true }); @@ -252,6 +329,7 @@ async function createHarness(): Promise { await fs.promises.mkdir(cwdA, { recursive: true }); await fs.promises.mkdir(cwdB, { recursive: true }); setAgentDir(agentDir); + await Settings.init({ agentDir, inMemory: true }); const updates: SessionNotification[] = []; const abortController = new AbortController(); @@ -288,6 +366,8 @@ describe("ACP agent", () => { const harness = await createHarness(); const first = await harness.agent.newSession({ cwd: harness.cwdA, mcpServers: [] }); const second = await harness.agent.newSession({ cwd: harness.cwdB, mcpServers: [] }); + expectAcpStructure(zNewSessionResponse, first); + expectAcpStructure(zNewSessionResponse, second); expect(first.models?.availableModels.map(model => model.modelId)).toEqual( TEST_MODELS.map(model => `${model.provider}/${model.id}`), @@ -302,6 +382,7 @@ describe("ACP agent", () => { configId: "thinking", value: "high", }); + expectAcpNotifications(harness.updates); const firstSession = harness.findSession(first.sessionId); const secondSession = harness.findSession(second.sessionId); @@ -318,12 +399,13 @@ describe("ACP agent", () => { cwd: harness.cwdA, mcpServers: [], }); + expectAcpStructure(zForkSessionResponse, forked); const forkedSession = harness.findSession(forked.sessionId); const forkedMessages = forkedSession?.sessionManager.buildSessionContext().messages ?? []; expect(forked.sessionId).not.toBe(first.sessionId); expect(forkedMessages.some(message => message.role === "user" && message.content === "fork me")).toBe(true); - await harness.agent.unstable_closeSession({ sessionId: forked.sessionId }); + await harness.agent.closeSession({ sessionId: forked.sessionId }); await expect(harness.agent.setSessionMode({ sessionId: forked.sessionId, modeId: "default" })).rejects.toThrow( "Unsupported ACP session", ); @@ -332,6 +414,72 @@ describe("ACP agent", () => { await Bun.sleep(0); }); + it("advertises plan mode and emits schema-valid mode updates", async () => { + const harness = await createHarness(); + Settings.instance.set("plan.enabled", true); + + const created = await harness.agent.newSession({ cwd: harness.cwdA, mcpServers: [] }); + expectAcpStructure(zNewSessionResponse, created); + expect(created.modes?.availableModes.map(mode => mode.id)).toEqual(["default", "plan"]); + const initialModeConfig = created.configOptions?.find(option => option.id === "mode") as + | { currentValue?: unknown; options?: Array<{ value: string }> } + | undefined; + expect(initialModeConfig?.currentValue).toBe("default"); + expect(initialModeConfig?.options?.map(option => option.value)).toEqual(["default", "plan"]); + + await harness.agent.setSessionMode({ sessionId: created.sessionId, modeId: "plan" }); + + const session = harness.findSession(created.sessionId)!; + expect(session.planModeState).toEqual( + expect.objectContaining({ enabled: true, planFilePath: "local://PLAN.md", workflow: "parallel" }), + ); + const modeNotifications = harness.updates.filter( + notification => + notification.sessionId === created.sessionId && + (notification.update.sessionUpdate === "current_mode_update" || + notification.update.sessionUpdate === "config_option_update"), + ); + expectAcpNotifications(modeNotifications); + expect( + modeNotifications.some( + notification => + notification.update.sessionUpdate === "current_mode_update" && + notification.update.currentModeId === "plan", + ), + ).toBe(true); + const configNotification = modeNotifications.findLast( + notification => notification.update.sessionUpdate === "config_option_update", + ); + const currentModeConfig = + configNotification?.update.sessionUpdate === "config_option_update" + ? (configNotification.update.configOptions.find(option => option.id === "mode") as + | { currentValue?: unknown } + | undefined) + : undefined; + expect(currentModeConfig?.currentValue).toBe("plan"); + + await harness.agent.setSessionMode({ sessionId: created.sessionId, modeId: "default" }); + expect(session.planModeState).toBeUndefined(); + + harness.abortController.abort(); + await Bun.sleep(0); + }); + + it("accepts only ACP underscore-prefixed extension methods", async () => { + const harness = await createHarness(); + + const result = await harness.agent.extMethod("_omp/sessions/listAll", { limit: 2 }); + + expect(Array.isArray(result.sessions)).toBe(true); + expect(typeof result.total).toBe("number"); + await expect(harness.agent.extMethod("omp/sessions/listAll", { limit: 2 })).rejects.toThrow( + "Unknown ACP ext method", + ); + + harness.abortController.abort(); + await Bun.sleep(0); + }); + it("replays messageIds and returns turn usage for prompts", async () => { const harness = await createHarness(); const stored = new FakeAgentSession(harness.cwdA); @@ -341,7 +489,12 @@ describe("ACP agent", () => { await stored.sessionManager.ensureOnDisk(); await stored.sessionManager.flush(); - await harness.agent.loadSession({ sessionId: stored.sessionId, cwd: harness.cwdA, mcpServers: [] }); + const loaded = await harness.agent.loadSession({ + sessionId: stored.sessionId, + cwd: harness.cwdA, + mcpServers: [], + }); + expectAcpStructure(zLoadSessionResponse, loaded); const replayChunks = harness.updates.filter( update => update.sessionId === stored.sessionId && @@ -368,6 +521,8 @@ describe("ACP agent", () => { messageId: "05b17a6f-b310-4be7-b767-6b4f3a84eb63", prompt: [{ type: "text", text: "ping" }], } as PromptRequest); + expectAcpStructure(zPromptResponse, response); + expectAcpNotifications(harness.updates); const liveChunks = harness.updates.filter( update => update.sessionId === live.sessionId && update.update.sessionUpdate === "agent_message_chunk", @@ -389,4 +544,140 @@ describe("ACP agent", () => { harness.abortController.abort(); await Bun.sleep(0); }); + + it("advertises ACP-safe builtins and skill commands", async () => { + const harness = await createHarness(); + const created = await harness.agent.newSession({ cwd: harness.cwdA, mcpServers: [] }); + const session = harness.findSession(created.sessionId)!; + const skillDir = path.join(harness.cwdA, ".skills", "sample"); + const skillPath = path.join(skillDir, "SKILL.md"); + await fs.promises.mkdir(skillDir, { recursive: true }); + await fs.promises.writeFile(skillPath, "---\ndescription: Sample skill\n---\n# Sample\nDo work.\n"); + session.skills = [ + { + name: "sample", + description: "Sample skill", + filePath: skillPath, + baseDir: skillDir, + source: "test", + }, + ]; + await harness.agent.prompt({ + sessionId: created.sessionId, + messageId: "00000000-0000-4000-8000-000000000004", + prompt: [{ type: "text", text: "/reload-plugins" }], + } as PromptRequest); + + const commandUpdates = harness.updates.filter( + update => + update.sessionId === created.sessionId && update.update.sessionUpdate === "available_commands_update", + ); + const names = commandUpdates.flatMap(update => + update.update.sessionUpdate === "available_commands_update" + ? update.update.availableCommands.map(command => command.name) + : [], + ); + expect(names).toContain("fast"); + expect(names).toContain("force"); + expect(names).toContain("skill:sample"); + expect(names).not.toContain("settings"); + expect(names).not.toContain("copy"); + expect(names).not.toContain("plan"); + expect(names).not.toContain("loop"); + expect(names).not.toContain("login"); + expect(names).not.toContain("new"); + expect(names).not.toContain("handoff"); + expect(names).not.toContain("fork"); + expect(names).not.toContain("btw"); + expect(names).not.toContain("drop"); + expect(names).not.toContain("resume"); + expect(names).not.toContain("agents"); + expect(names).not.toContain("extensions"); + expect(names).not.toContain("hotkeys"); + + harness.abortController.abort(); + await Bun.sleep(0); + }); + + it("executes skill commands through custom skill messages", async () => { + const harness = await createHarness(); + const created = await harness.agent.newSession({ cwd: harness.cwdA, mcpServers: [] }); + const session = harness.findSession(created.sessionId)!; + const skillDir = path.join(harness.cwdA, ".skills", "sample"); + const skillPath = path.join(skillDir, "SKILL.md"); + await fs.promises.mkdir(skillDir, { recursive: true }); + await fs.promises.writeFile(skillPath, "---\ndescription: Sample skill\n---\n# Sample\nDo work.\n"); + session.skills = [ + { + name: "sample", + description: "Sample skill", + filePath: skillPath, + baseDir: skillDir, + source: "test", + }, + ]; + + await harness.agent.prompt({ + sessionId: created.sessionId, + messageId: "00000000-0000-4000-8000-000000000001", + prompt: [{ type: "text", text: "/skill:sample extra context" }], + } as PromptRequest); + + expect(session.promptCalls).toEqual([]); + expect(session.customMessages).toHaveLength(1); + expect(session.customMessages[0]!.customType).toBe("skill-prompt"); + expect(session.customMessages[0]!.content).toContain("# Sample\nDo work."); + expect(session.customMessages[0]!.content).toContain(`Skill: ${skillPath}`); + expect(session.customMessages[0]!.content).toContain("User: extra context"); + + harness.abortController.abort(); + await Bun.sleep(0); + }); + + it("executes consumed ACP builtins without prompting the agent", async () => { + const harness = await createHarness(); + const created = await harness.agent.newSession({ cwd: harness.cwdA, mcpServers: [] }); + const session = harness.findSession(created.sessionId)!; + + const response = await harness.agent.prompt({ + sessionId: created.sessionId, + messageId: "00000000-0000-4000-8000-000000000002", + prompt: [{ type: "text", text: "/fast status" }], + } as PromptRequest); + + const chunks = harness.updates.filter( + update => update.sessionId === created.sessionId && update.update.sessionUpdate === "agent_message_chunk", + ); + expect(response.userMessageId).toBe("00000000-0000-4000-8000-000000000002"); + expect(session.promptCalls).toEqual([]); + expect( + chunks.some( + update => + update.update.sessionUpdate === "agent_message_chunk" && + update.update.content.type === "text" && + update.update.content.text === "Fast mode is off.", + ), + ).toBe(true); + + harness.abortController.abort(); + await Bun.sleep(0); + }); + + it("executes force builtins and forwards remaining prompt text", async () => { + const harness = await createHarness(); + const created = await harness.agent.newSession({ cwd: harness.cwdA, mcpServers: [] }); + const session = harness.findSession(created.sessionId)!; + + await harness.agent.prompt({ + sessionId: created.sessionId, + messageId: "00000000-0000-4000-8000-000000000003", + prompt: [{ type: "text", text: "/force read inspect package.json" }], + } as PromptRequest); + + expect(session.forcedToolChoice).toBe("read"); + expect(session.promptCalls).toEqual(["inspect package.json"]); + + harness.abortController.abort(); + await Bun.sleep(0); + }); }); diff --git a/packages/coding-agent/test/acp-builtins.test.ts b/packages/coding-agent/test/acp-builtins.test.ts new file mode 100644 index 000000000..95c4ec0bd --- /dev/null +++ b/packages/coding-agent/test/acp-builtins.test.ts @@ -0,0 +1,836 @@ +import { describe, expect, it, spyOn } from "bun:test"; +import { Settings } from "../src/config/settings"; +import type { AgentSession } from "../src/session/agent-session"; +import type { SessionManager } from "../src/session/session-manager"; +import { executeAcpBuiltinSlashCommand } from "../src/slash-commands/acp-builtins"; + +interface FakeAcpBuiltinSession { + fastMode: boolean; + forcedToolChoice: string | undefined; + isStreaming: boolean; + sessionFile: string | undefined; + sessionId: string; + sessionName: string; + _todoPhases: Array<{ name: string; tasks: Array<{ content: string; status: string }> }>; + toggleFastMode(): boolean; + setFastMode(enabled: boolean): void; + isFastModeEnabled(): boolean; + setForcedToolChoice(toolName: string): void; + fetchUsageReports?: () => Promise; + getAsyncJobSnapshot: (opts?: { recentLimit?: number }) => { running: unknown[]; recent: unknown[] } | null; + formatSessionAsText: () => string; + getLastAssistantText: () => string | undefined; + messages: unknown[]; + model: { provider: string; id: string } | undefined; + newSession(opts?: { drop?: boolean; parentSession?: string }): Promise; + fork(): Promise; + handoff(instr?: string): Promise<{ document: string; savedPath?: string } | undefined>; + exportToHtml(outputPath?: string): Promise; + getTodoPhases(): Array<{ name: string; tasks: Array<{ content: string; status: string }> }>; + setTodoPhases(phases: Array<{ name: string; tasks: Array<{ content: string; status: string }> }>): void; + refreshBaseSystemPrompt(): Promise; + getToolByName(name: string): unknown; + compact(args?: string): Promise; + getContextUsage(): { tokens?: number; contextWindow: number } | undefined; + getAvailableModels(): Array<{ provider: string; id: string; contextWindow?: number }>; + setModel(model: unknown): Promise; +} + +function createRuntime() { + const output: string[] = []; + const session: FakeAcpBuiltinSession = { + fastMode: false, + forcedToolChoice: undefined as string | undefined, + isStreaming: false, + sessionFile: undefined, + sessionId: "fake-session-id", + sessionName: "Fake Session", + _todoPhases: [], + toggleFastMode() { + this.fastMode = !this.fastMode; + return this.fastMode; + }, + setFastMode(enabled: boolean) { + this.fastMode = enabled; + }, + isFastModeEnabled() { + return this.fastMode; + }, + setForcedToolChoice(toolName: string) { + this.forcedToolChoice = toolName; + }, + async newSession(_opts?: { drop?: boolean; parentSession?: string }) { + return true; + }, + async fork() { + return true; + }, + async handoff(_instr?: string) { + return undefined; + }, + async exportToHtml(outputPath?: string) { + return outputPath ?? "/tmp/exported-session.html"; + }, + getTodoPhases() { + return this._todoPhases; + }, + setTodoPhases(phases) { + this._todoPhases = phases; + }, + async refreshBaseSystemPrompt() {}, + getAsyncJobSnapshot: () => null, + formatSessionAsText: () => "", + getLastAssistantText: () => undefined, + messages: [], + model: undefined, + getToolByName: (_name: string) => undefined, + async compact(_args?: string) {}, + getContextUsage: () => undefined, + getAvailableModels: () => [] as Array<{ provider: string; id: string; contextWindow?: number }>, + async setModel(_model: unknown) {}, + }; + const typedSession = session as unknown as AgentSession & FakeAcpBuiltinSession; + const fakeSessionManager = { + _sessionFile: undefined as string | undefined, + _cwd: "/tmp/project", + _entries: [] as { type: string }[], + _customEntries: [] as Array<{ customType: string; data: unknown }>, + _movedTo: undefined as string | undefined, + _flushed: false, + _sessionName: undefined as string | undefined, + getSessionId(): string { + return "fake-session-id"; + }, + getSessionFile(): string | undefined { + return this._sessionFile; + }, + getEntries(): { type: string }[] { + return this._entries; + }, + getBranch(): { type: string }[] { + return this._entries; + }, + appendCustomEntry(customType: string, data?: unknown): string { + this._customEntries.push({ customType, data }); + return "fake-entry-id"; + }, + async flush() { + this._flushed = true; + }, + async moveTo(newCwd: string) { + this._cwd = newCwd; + this._movedTo = newCwd; + }, + getCwd(): string { + return this._cwd; + }, + async setSessionName(name: string, _source: string): Promise { + this._sessionName = name; + return true; + }, + }; + return { + output, + session, + fakeSessionManager, + runtime: { + session: typedSession, + sessionManager: fakeSessionManager as unknown as SessionManager, + settings: Settings.isolated(), + cwd: "/tmp/project", + output: (text: string) => { + output.push(text); + }, + refreshCommands: () => {}, + notifyTitleChanged: undefined as (() => Promise | void) | undefined, + }, + }; +} + +describe("ACP builtin slash commands", () => { + it("consumes fast status without returning prompt text", async () => { + const { output, runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand("/fast status", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output).toEqual(["Fast mode is off."]); + }); + + it("forces a tool and returns remaining prompt text", async () => { + const { output, runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand("/force read inspect package.json", runtime); + + expect(result).toEqual({ prompt: "inspect package.json" }); + expect(runtime.session.forcedToolChoice).toBe("read"); + expect(output).toEqual(["Next turn forced to use read."]); + }); + + it("renders provider usage reports when the session can fetch them", async () => { + const { output, runtime } = createRuntime(); + runtime.session.fetchUsageReports = async () => [ + { + provider: "openai-codex", + fetchedAt: Date.now(), + limits: [ + { + id: "codex-5h", + label: "5 hours", + scope: { provider: "openai-codex", tier: "prolite", accountId: "account-1" }, + window: { id: "5h", label: "5 hours", resetsAt: Date.now() + 60 * 60 * 1000 }, + amount: { used: 0.24, usedFraction: 0.24, unit: "unknown" }, + }, + ], + metadata: { email: "user@example.com" }, + }, + ]; + + const result = await executeAcpBuiltinSlashCommand("/usage", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Openai Codex"); + expect(output[0]).toContain("5 hours (prolite)"); + expect(output[0]).toContain("user@example.com: 0.24 unknown used (76.0% left)"); + expect(output[0]).toContain("resets in"); + }); + + it("returns false for unknown commands", async () => { + const { runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand("/not-a-real-command-xyz", runtime); + + expect(result).toBe(false); + }); + + // /jobs + it("jobs: shows informative message when snapshot is null", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/jobs", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("background jobs"); + }); + + it("jobs: lists running and recent jobs from snapshot", async () => { + const { output, runtime } = createRuntime(); + runtime.session.getAsyncJobSnapshot = () => ({ + running: [{ id: "j1", type: "bash", status: "running", label: "npm install", startTime: Date.now() - 5000 }], + recent: [{ id: "j2", type: "task", status: "completed", label: "build done", startTime: Date.now() - 60_000 }], + }); + + const result = await executeAcpBuiltinSlashCommand("/jobs", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("npm install"); + expect(output[0]).toContain("build done"); + expect(output[0]).toContain("Running Jobs"); + expect(output[0]).toContain("Recent Jobs"); + }); + + // /dump + it("dump: outputs transcript when present", async () => { + const { output, runtime } = createRuntime(); + runtime.session.formatSessionAsText = () => "Session content here"; + + const result = await executeAcpBuiltinSlashCommand("/dump", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toBe("Session content here"); + }); + + it("dump: outputs empty-state message when no messages", async () => { + const { output, runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand("/dump", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("No messages"); + }); + + // /model + it("model: returns current model when set", async () => { + const { output, runtime } = createRuntime(); + runtime.session.model = { provider: "anthropic", id: "claude-opus-4-5" } as never; + + const result = await executeAcpBuiltinSlashCommand("/model", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("anthropic/claude-opus-4-5"); + }); + + it("model: returns no-selection message when undefined", async () => { + const { output, runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand("/model", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("No model"); + }); + + it("model: returns ACP usage message when args provided", async () => { + const { output, runtime } = createRuntime(); + + const result = await executeAcpBuiltinSlashCommand("/model claude-3-5-sonnet", runtime); + + expect(result).toEqual({ consumed: true }); + expect(output[0]?.toLowerCase()).toContain("acp"); + }); + + // Removed TUI-only and dropped commands fall through as false + it("removed commands return false (fall through to model)", async () => { + const removedCommands = [ + "/login", + "/logout", + "/resume", + "/tree", + "/branch", + "/plan", + "/loop", + "/hotkeys", + "/extensions", + "/agents", + "/copy", + "/btw hi", + "/new", + "/drop", + "/handoff", + "/fork", + ]; + for (const cmd of removedCommands) { + const { runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand(cmd, runtime); + expect(result).toBe(false); + } + }); +}); + +describe("session lifecycle commands", () => { + it("/session delete: returns in-memory usage when no sessionFile", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/session delete", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("in-memory"); + }); + + it("/session delete: refuses while streaming", async () => { + const { output, session, fakeSessionManager, runtime } = createRuntime(); + session.isStreaming = true; + fakeSessionManager._sessionFile = "/tmp/session.jsonl"; + const result = await executeAcpBuiltinSlashCommand("/session delete", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("streaming"); + }); + + it("/rename: renames and calls notifyTitleChanged on success", async () => { + const { output, fakeSessionManager, runtime } = createRuntime(); + let notified = false; + runtime.notifyTitleChanged = async () => { + notified = true; + }; + const result = await executeAcpBuiltinSlashCommand("/rename Project Apex", runtime); + expect(result).toEqual({ consumed: true }); + expect(fakeSessionManager._sessionName).toBe("Project Apex"); + expect(output[0]).toBe("Session renamed to Project Apex."); + expect(notified).toBe(true); + }); + + it("/rename: outputs precedence message when setSessionName returns false", async () => { + const { output, fakeSessionManager, runtime } = createRuntime(); + let notified = false; + runtime.notifyTitleChanged = async () => { + notified = true; + }; + fakeSessionManager.setSessionName = async () => false; + const result = await executeAcpBuiltinSlashCommand("/rename Bar", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("takes precedence"); + expect(notified).toBe(false); + }); + + it("/move: reports moved path via sessionManager.getCwd() and calls notifyTitleChanged", async () => { + const { output, fakeSessionManager, runtime } = createRuntime(); + let notified = false; + runtime.notifyTitleChanged = async () => { + notified = true; + }; + const result = await executeAcpBuiltinSlashCommand("/move /tmp", runtime); + expect(result).toEqual({ consumed: true }); + expect(fakeSessionManager._flushed).toBe(true); + expect(fakeSessionManager._movedTo).toBe("/tmp"); + expect(output[0]).toContain("/tmp"); + expect(notified).toBe(true); + }); + + it("/move: refuses while streaming", async () => { + const { output, session, runtime } = createRuntime(); + session.isStreaming = true; + const result = await executeAcpBuiltinSlashCommand("/move /tmp", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("streaming"); + }); +}); + +describe("wave 3 commands", () => { + // /export + it("/export: calls exportToHtml with the given arg and outputs the path", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/export /tmp/out.html", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toBe("Session exported to: /tmp/out.html"); + }); + + it("/export: uses default path when no arg given", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/export", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Session exported to:"); + }); + + it("/export: returns usage on exportToHtml failure", async () => { + const { output, session, runtime } = createRuntime(); + session.exportToHtml = async () => { + throw new Error("disk full"); + }; + const result = await executeAcpBuiltinSlashCommand("/export", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Failed to export session: disk full"); + }); + + // /todo + it("/todo no-args: outputs empty state message when no todos", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/todo", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toBe("No todos. Use /todo append to start one."); + }); + + it("/todo append: stores phases and records custom entry", async () => { + const { session, fakeSessionManager, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand('/todo append "Build" "Wire setup"', runtime); + expect(result).toEqual({ consumed: true }); + expect(session._todoPhases).toHaveLength(1); + expect(session._todoPhases[0]?.name).toBe("Build"); + expect(session._todoPhases[0]?.tasks[0]?.content).toBe("Wire setup"); + expect(fakeSessionManager._customEntries).toHaveLength(1); + expect(fakeSessionManager._customEntries[0]?.customType).toBe("user_todo_edit"); + }); + + it("/todo edit: returns TUI-only usage message", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/todo edit", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("TUI editor"); + }); + + it("/todo unknown: returns usage message", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/todo foobar", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Unknown /todo subcommand"); + }); + + // /move + it("/move: returns usage when no arg", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/move", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Usage: /move"); + }); + + it("/move: returns usage when path does not exist", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/move /no/such/path/xyz", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("does not exist"); + }); + + // /memory + it("/memory unknown: returns usage message", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/memory unknownverb", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Usage: /memory"); + }); + + it("/memory view: outputs memory payload (or empty message)", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/memory view", runtime); + expect(result).toEqual({ consumed: true }); + expect(output.length).toBeGreaterThan(0); + }); + + it("/memory (no args): defaults to view", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/memory", runtime); + expect(result).toEqual({ consumed: true }); + expect(output.length).toBeGreaterThan(0); + }); + + // /todo start fuzzy match + it("/todo start: finds pending task by substring and starts it", async () => { + const { output, session, runtime } = createRuntime(); + session._todoPhases = [{ name: "Setup", tasks: [{ content: "Wire up router", status: "pending" }] }]; + const result = await executeAcpBuiltinSlashCommand('/todo start "wire"', runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Wire up router"); + expect(session._todoPhases[0]?.tasks[0]?.status).toBe("in_progress"); + }); + + // /browser + it("/browser visible: sets headless=false; second call is idempotent", async () => { + const { runtime } = createRuntime(); + runtime.settings.set("browser.enabled" as never, true as never); + runtime.settings.set("browser.headless" as never, true as never); + const r1 = await executeAcpBuiltinSlashCommand("/browser visible", runtime); + expect(r1).toEqual({ consumed: true }); + expect(runtime.settings.get("browser.headless" as never)).toBe(false); + const r2 = await executeAcpBuiltinSlashCommand("/browser visible", runtime); + expect(r2).toEqual({ consumed: true }); + expect(runtime.settings.get("browser.headless" as never)).toBe(false); + }); + + it("/browser no-arg after /browser visible toggles to headless", async () => { + const { output, runtime } = createRuntime(); + runtime.settings.set("browser.enabled" as never, true as never); + runtime.settings.set("browser.headless" as never, true as never); + await executeAcpBuiltinSlashCommand("/browser visible", runtime); + const r = await executeAcpBuiltinSlashCommand("/browser", runtime); + expect(r).toEqual({ consumed: true }); + expect(output[output.length - 1]).toContain("headless"); + expect(runtime.settings.get("browser.headless" as never)).toBe(true); + }); + + // /compact + it("/compact: reports Compaction complete. after session.compact resolves", async () => { + const { output, session, runtime } = createRuntime(); + let compactCalled = false; + session.compact = async (_args?: string) => { + compactCalled = true; + }; + const result = await executeAcpBuiltinSlashCommand("/compact", runtime); + expect(result).toEqual({ consumed: true }); + expect(compactCalled).toBe(true); + expect(output[0]).toContain("Compaction complete."); + }); +}); + +describe("wave 4 commands", () => { + // /mcp + it("/mcp (no args): outputs help text containing list, enable, disable, remove, reload", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/mcp", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("list"); + expect(output[0]).toContain("enable"); + expect(output[0]).toContain("disable"); + expect(output[0]).toContain("remove"); + expect(output[0]).toContain("reload"); + }); + + it("/mcp help: outputs help text containing list, enable, disable, remove, reload", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/mcp help", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("list"); + expect(output[0]).toContain("enable"); + expect(output[0]).toContain("disable"); + expect(output[0]).toContain("remove"); + expect(output[0]).toContain("reload"); + }); + + it("/mcp add (no args): returns usage string", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/mcp add", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Usage"); + }); + + it("/mcp reload: calls refreshCommands and outputs confirmation", async () => { + let refreshCalled = false; + const { output, runtime } = createRuntime(); + runtime.refreshCommands = () => { + refreshCalled = true; + }; + const result = await executeAcpBuiltinSlashCommand("/mcp reload", runtime); + expect(result).toEqual({ consumed: true }); + expect(refreshCalled).toBe(true); + expect(output[0]).toContain("reload"); + }); + + it("/mcp resources: outputs server list or no-server message", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/mcp resources", runtime); + expect(result).toEqual({ consumed: true }); + // No servers configured in tmp project dir — should report that + expect(output[0]).toMatch(/No MCP servers configured|No resources/); + }); + + it("/mcp unknown-verb: returns usage pointing to help", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/mcp frobnicate", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Unknown"); + }); + + // /ssh + it("/ssh (no args): outputs help text containing list and remove", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/ssh", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("list"); + expect(output[0]).toContain("remove"); + }); + + it("/ssh help: outputs help text containing list and remove", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/ssh help", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("list"); + expect(output[0]).toContain("remove"); + }); + + it("/ssh add (no args): returns usage", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/ssh add", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Usage"); + }); + + it("/ssh unknown-verb: returns unknown subcommand message", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/ssh frobnicate", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Unknown"); + }); + + // /marketplace + it("/marketplace help: outputs help text", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/marketplace help", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Marketplace commands"); + expect(output[0]).toContain("install"); + }); + + it("/marketplace install (no args): returns interactive picker usage", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/marketplace install", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("TUI-only"); + }); + + it("/marketplace uninstall (no args): returns interactive picker usage", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/marketplace uninstall", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("TUI-only"); + }); + + // /plugins + it("/plugins list: outputs without throwing when registries are empty", async () => { + const { MarketplaceManager } = await import("../src/extensibility/plugins/marketplace"); + const { PluginManager } = await import("../src/extensibility/plugins"); + const listInstalledSpy = spyOn(MarketplaceManager.prototype, "listInstalledPlugins").mockResolvedValue([]); + const npmListSpy = spyOn(PluginManager.prototype, "list").mockResolvedValue([]); + try { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/plugins list", runtime); + expect(result).toEqual({ consumed: true }); + expect(output.length).toBeGreaterThan(0); + } finally { + listInstalledSpy.mockRestore(); + npmListSpy.mockRestore(); + } + }); + + it("/plugins (no args): defaults to list", async () => { + const { MarketplaceManager } = await import("../src/extensibility/plugins/marketplace"); + const { PluginManager } = await import("../src/extensibility/plugins"); + const listInstalledSpy = spyOn(MarketplaceManager.prototype, "listInstalledPlugins").mockResolvedValue([]); + const npmListSpy = spyOn(PluginManager.prototype, "list").mockResolvedValue([]); + try { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/plugins", runtime); + expect(result).toEqual({ consumed: true }); + expect(output.length).toBeGreaterThan(0); + } finally { + listInstalledSpy.mockRestore(); + npmListSpy.mockRestore(); + } + }); + + // /todo start with in_progress status in fuzzy list + it("/todo start: resolves ambiguous matches by preferring active tasks", async () => { + const { output, session, runtime } = createRuntime(); + session._todoPhases = [ + { + name: "Phase 1", + tasks: [ + { content: "Wire auth middleware", status: "pending" }, + { content: "Wire session store", status: "completed" }, + ], + }, + ]; + const result = await executeAcpBuiltinSlashCommand('/todo start "wire"', runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Wire auth middleware"); + }); +}); + +describe("wave 5 — adapters and polish", () => { + // /mcp help lists new subcommands + it("/mcp help: lists resources, prompts, test, add, smithery-search", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/mcp help", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("resources"); + expect(output[0]).toContain("prompts"); + expect(output[0]).toContain("test"); + expect(output[0]).toContain("add"); + expect(output[0]).toContain("smithery-search"); + }); + + // /mcp add — verify parsing and output message + it("/mcp add foo --url https://example.com --token X --scope project: outputs success or propagates write error", async () => { + // Uses project scope so it writes to /tmp/project/.omp/mcp.json which test infra controls. + // We verify the command either reports success or a meaningful error (not a parse error). + const mcpModule = await import("../src/mcp/config-writer"); + const spy = spyOn(mcpModule, "addMCPServer").mockResolvedValue(undefined); + try { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand( + "/mcp add foo --url https://example.com --token X --scope project", + runtime, + ); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain('Added MCP server "foo" (project).'); + expect(spy).toHaveBeenCalledTimes(1); + } finally { + spy.mockRestore(); + } + }); + + // /mcp test — spy on connectToServer + it("/mcp test bogus: returns error when server not found in config", async () => { + const { output, runtime } = createRuntime(); + // No servers in /tmp/project config — server not found + const result = await executeAcpBuiltinSlashCommand("/mcp test bogus", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("not found"); + }); + + // /ssh add — spy on addSSHHost + it("/ssh add foo --host x --user y --scope user: calls addSSHHost", async () => { + const sshModule = await import("../src/ssh/config-writer"); + const spy = spyOn(sshModule, "addSSHHost").mockResolvedValue(undefined); + try { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/ssh add foo --host x --user y --scope user", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain('Added SSH host "foo" (user).'); + } finally { + spy.mockRestore(); + } + }); + + // /model with unknown id + it("/model gpt-fake-9000: returns unknown-model message", async () => { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/model gpt-fake-9000", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Unknown model"); + }); + + // /model with known id (fake registry) + it("/model known-id: reports model set and triggers notifyTitleChanged", async () => { + const { output, session, runtime } = createRuntime(); + session.getAvailableModels = () => [{ provider: "anthropic", id: "claude-sonnet-test" }]; + let titleChanged = false; + runtime.notifyTitleChanged = () => { + titleChanged = true; + }; + const result = await executeAcpBuiltinSlashCommand("/model claude-sonnet-test", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("Model set to anthropic/claude-sonnet-test."); + expect(titleChanged).toBe(true); + }); + + // /usage bar character + it("/usage: includes bar character when usedFraction is 0.5", async () => { + const { output, runtime } = createRuntime(); + runtime.session.fetchUsageReports = async () => [ + { + provider: "test-provider", + fetchedAt: Date.now(), + limits: [ + { + id: "test-limit", + label: "Monthly", + scope: { provider: "test-provider", tier: "pro", accountId: "acct-1" }, + window: { id: "monthly", label: "monthly", resetsAt: Date.now() + 30 * 86400_000 }, + amount: { used: 50, usedFraction: 0.5, unit: "requests" }, + }, + ], + metadata: {}, + }, + ]; + const result = await executeAcpBuiltinSlashCommand("/usage", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("█"); + }); + + // /context breakdown + it("/context: lists more than one breakdown line for session with messages", async () => { + const { output, session, runtime } = createRuntime(); + // computeContextBreakdown needs model.contextWindow; fake session falls back gracefully + (session as unknown as Record).model = { + provider: "anthropic", + id: "claude-test", + contextWindow: 200_000, + }; + (session as unknown as Record).skills = []; + (session as unknown as Record).agent = { state: { tools: [] } }; + (session as unknown as Record).systemPrompt = ["You are a helpful assistant."]; + (session as unknown as Record).settings = { + getGroup: () => ({ enabled: false, strategy: "off" }), + }; + session.messages = [ + { role: "user", content: "Hello, how are you?" }, + { role: "assistant", content: "I am doing well." }, + ]; + const result = await executeAcpBuiltinSlashCommand("/context", runtime); + expect(result).toEqual({ consumed: true }); + // Should show the breakdown with multiple lines (Messages category visible) + const text = output[0] ?? ""; + expect(text).toContain("tokens"); + expect(text.split("\n").length).toBeGreaterThan(1); + }); + + // /jobs empty state + it("/jobs: empty-state output mentions background jobs definition", async () => { + const { output, runtime } = createRuntime(); + // Return empty snapshot (running=[], recent=[]) + runtime.session.getAsyncJobSnapshot = () => ({ running: [], recent: [] }); + const result = await executeAcpBuiltinSlashCommand("/jobs", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain("background jobs"); + }); + + // /marketplace discover bulleted list + it("/marketplace discover: output is bulleted with ' - ' token", async () => { + const { MarketplaceManager } = await import("../src/extensibility/plugins/marketplace"); + const discoverSpy = spyOn(MarketplaceManager.prototype, "listAvailablePlugins").mockResolvedValue([ + { name: "hello", version: "1.0.0", description: "A greeting plugin" } as never, + { name: "world", version: "2.0.0", description: undefined } as never, + ]); + try { + const { output, runtime } = createRuntime(); + const result = await executeAcpBuiltinSlashCommand("/marketplace discover", runtime); + expect(result).toEqual({ consumed: true }); + expect(output[0]).toContain(" - "); + expect(output[0]).toContain("hello@1.0.0"); + } finally { + discoverSpy.mockRestore(); + } + }); +}); diff --git a/packages/coding-agent/test/acp-event-mapper.test.ts b/packages/coding-agent/test/acp-event-mapper.test.ts index a95cc0293..1b259e358 100644 --- a/packages/coding-agent/test/acp-event-mapper.test.ts +++ b/packages/coding-agent/test/acp-event-mapper.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "bun:test"; +import type { SessionNotification } from "@agentclientprotocol/sdk"; +import { zSessionNotification } from "@agentclientprotocol/sdk/dist/schema/zod.gen.js"; import { mapAgentSessionEventToAcpSessionUpdates } from "../src/modes/acp/acp-event-mapper"; import type { AgentSessionEvent } from "../src/session/agent-session"; +import { expectAcpStructure, expectAcpStructureRejects } from "./helpers/acp-schema"; function makeAssistantMessage(text: string) { return { @@ -27,6 +30,12 @@ function getChunkMessageId(event: { update: object }): string | undefined { return typeof update.messageId === "string" ? update.messageId : undefined; } +function expectAcpNotifications(updates: SessionNotification[]): void { + for (const update of updates) { + expectAcpStructure(zSessionNotification, update); + } +} + describe("ACP event mapper", () => { it("attaches a stable messageId to live assistant chunks", () => { const assistantMessage = makeAssistantMessage("chunk"); @@ -54,6 +63,7 @@ describe("ACP event mapper", () => { expect(textUpdates).toHaveLength(1); expect(thoughtUpdates).toHaveLength(1); + expectAcpNotifications([...textUpdates, ...thoughtUpdates]); expect(textUpdates[0] ? getChunkMessageId(textUpdates[0]) : undefined).toBe( "a80f1ff7-4f0a-4e6b-9f09-c94857b62a4a", ); @@ -61,4 +71,210 @@ describe("ACP event mapper", () => { "a80f1ff7-4f0a-4e6b-9f09-c94857b62a4a", ); }); + + it("emits final assistant text when no text deltas were observed", () => { + const assistantMessage = makeAssistantMessage("final response"); + const progress = { textEmitted: false, thoughtEmitted: false }; + + const updates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "message_end", + message: assistantMessage, + } as AgentSessionEvent, + "session-1", + { getMessageProgress: message => (message === assistantMessage ? progress : undefined) }, + ); + + expect(updates).toEqual([ + { + sessionId: "session-1", + update: { + sessionUpdate: "agent_message_chunk", + content: { type: "text", text: "final response" }, + messageId: undefined, + }, + }, + ]); + expectAcpNotifications(updates); + expect(progress.textEmitted).toBe(true); + }); + + it("does not duplicate final assistant text after streaming deltas", () => { + const assistantMessage = makeAssistantMessage("streamed response"); + const progress = { textEmitted: false, thoughtEmitted: false }; + const options = { + getMessageProgress: (message: unknown) => (message === assistantMessage ? progress : undefined), + }; + + const deltaUpdates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "message_update", + message: assistantMessage, + assistantMessageEvent: { type: "text_delta", delta: "streamed response" }, + } as AgentSessionEvent, + "session-1", + options, + ); + const doneUpdates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "message_end", + message: assistantMessage, + } as AgentSessionEvent, + "session-1", + options, + ); + + expect(deltaUpdates).toHaveLength(1); + expectAcpNotifications(deltaUpdates); + expect(doneUpdates).toEqual([]); + }); + + it("emits a diff ToolCallContent for each per-file edit result", () => { + const updates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "tool_execution_end", + toolCallId: "tc-1", + toolName: "edit", + isError: false, + result: { + content: [{ type: "text", text: "applied" }], + details: { + diff: "--- a/foo\n+++ b/foo\n", + perFileResults: [ + { path: "foo.ts", diff: "...", oldText: "before\n", newText: "after\n" }, + { path: "bar.ts", diff: "...", oldText: undefined, newText: "created\n" }, + { path: "skipped.ts", diff: "", isError: true, errorText: "boom" }, + ], + }, + }, + } as AgentSessionEvent, + "session-1", + ); + + expect(updates).toHaveLength(1); + expectAcpNotifications(updates); + const update = updates[0]!.update as { + sessionUpdate: string; + content?: Array<{ type: string; path?: string; oldText?: string | null; newText?: string }>; + locations?: { path: string }[]; + }; + expect(update.sessionUpdate).toBe("tool_call_update"); + const diffBlocks = update.content?.filter(block => block.type === "diff") ?? []; + expect(diffBlocks).toEqual([ + { type: "diff", path: "foo.ts", oldText: "before\n", newText: "after\n" }, + { type: "diff", path: "bar.ts", oldText: null, newText: "created\n" }, + ]); + expect(update.locations).toEqual([{ path: "foo.ts" }, { path: "bar.ts" }, { path: "skipped.ts" }]); + }); + + it("emits a diff ToolCallContent for single-file edit details", () => { + const updates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "tool_execution_end", + toolCallId: "tc-single", + toolName: "edit", + isError: false, + result: { + content: [{ type: "text", text: "applied" }], + details: { + path: "single.ts", + diff: "--- a/single.ts\n+++ b/single.ts\n", + oldText: "before\n", + newText: "after\n", + }, + }, + } as AgentSessionEvent, + "session-1", + ); + + expect(updates).toHaveLength(1); + expectAcpNotifications(updates); + const update = updates[0]!.update as { + sessionUpdate: string; + content?: Array<{ type: string; path?: string; oldText?: string | null; newText?: string }>; + locations?: { path: string }[]; + }; + expect(update.sessionUpdate).toBe("tool_call_update"); + expect(update.content?.filter(block => block.type === "diff")).toEqual([ + { type: "diff", path: "single.ts", oldText: "before\n", newText: "after\n" }, + ]); + expect(update.locations).toEqual([{ path: "single.ts" }]); + }); + + it("emits locations on tool_execution_update from args", () => { + const updates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "tool_execution_update", + toolCallId: "tc-2", + toolName: "edit", + args: { path: "src/foo.ts" }, + partialResult: { content: [{ type: "text", text: "in progress" }] }, + } as AgentSessionEvent, + "session-1", + ); + + expect(updates).toHaveLength(1); + expectAcpNotifications(updates); + const update = updates[0]!.update as { sessionUpdate: string; locations?: { path: string }[] }; + expect(update.sessionUpdate).toBe("tool_call_update"); + expect(update.locations).toEqual([{ path: "src/foo.ts" }]); + }); + + it("emits a terminal ToolCallContent when tool details carry a terminalId", () => { + const updates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "tool_execution_update", + toolCallId: "tc-3", + toolName: "bash", + args: { command: "echo hi" }, + partialResult: { content: [], details: { terminalId: "term-42" } }, + } as AgentSessionEvent, + "session-1", + ); + + expect(updates).toHaveLength(1); + expectAcpNotifications(updates); + const update = updates[0]!.update as { + sessionUpdate: string; + content?: Array<{ type: string; terminalId?: string }>; + }; + expect(update.sessionUpdate).toBe("tool_call_update"); + expect(update.content).toEqual([{ type: "terminal", terminalId: "term-42" }]); + }); + it("emits distinct locations for move-style path arguments", () => { + const updates = mapAgentSessionEventToAcpSessionUpdates( + { + type: "tool_execution_start", + toolCallId: "tc-move", + toolName: "move", + args: { path: "src/current.ts", oldPath: "src/old.ts", newPath: "src/new.ts" }, + } as AgentSessionEvent, + "session-1", + ); + + expect(updates).toHaveLength(1); + expectAcpNotifications(updates); + const update = updates[0]!.update as { sessionUpdate: string; locations?: { path: string }[] }; + expect(update.sessionUpdate).toBe("tool_call"); + expect(update.locations).toEqual([{ path: "src/current.ts" }, { path: "src/old.ts" }, { path: "src/new.ts" }]); + }); + + it("rejects mutated ACP notification discriminators", () => { + const [notification] = mapAgentSessionEventToAcpSessionUpdates( + { + type: "tool_execution_start", + toolCallId: "tc-schema", + toolName: "read", + args: { path: "package.json" }, + } as AgentSessionEvent, + "session-1", + ); + + expectAcpStructure(zSessionNotification, notification); + expectAcpStructureRejects(zSessionNotification, { + ...notification, + update: { ...notification!.update, sessionUpdate: "tool_call_updates" }, + }); + expectAcpStructureRejects(zSessionNotification, { ...notification, sessionId: 42 }); + }); }); diff --git a/packages/coding-agent/test/acp-initialize-conformance.test.ts b/packages/coding-agent/test/acp-initialize-conformance.test.ts new file mode 100644 index 000000000..7ec1de87b --- /dev/null +++ b/packages/coding-agent/test/acp-initialize-conformance.test.ts @@ -0,0 +1,247 @@ +/** + * ACP `initialize` conformance — gates `terminal` auth methods on + * `clientCapabilities.auth.terminal`, advertises stable agentInfo, and keeps + * the agentCapabilities contract that downstream clients rely on. + */ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import type { AgentSideConnection, InitializeRequest } from "@agentclientprotocol/sdk"; +import { zInitializeResponse } from "@agentclientprotocol/sdk/dist/schema/zod.gen.js"; +import type { Model } from "@oh-my-pi/pi-ai"; +import { getConfigRootDir, setAgentDir, VERSION } from "@oh-my-pi/pi-utils"; +import { AcpAgent } from "../src/modes/acp/acp-agent"; +import { ACP_TERMINAL_AUTH_FLAG, prepareAcpTerminalAuthArgs } from "../src/modes/acp/terminal-auth"; +import type { AgentSession } from "../src/session/agent-session"; +import { SessionManager } from "../src/session/session-manager"; +import { expectAcpStructure } from "./helpers/acp-schema"; + +const TEST_MODELS: Model[] = [ + { + id: "claude-sonnet-4-20250514", + name: "Claude Sonnet", + api: "anthropic-messages", + provider: "anthropic", + baseUrl: "https://example.invalid", + reasoning: true, + input: ["text", "image"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 200_000, + maxTokens: 8_192, + }, +]; + +class FakeAgentSession { + sessionManager: SessionManager; + sessionId: string; + agent: { sessionId: string; waitForIdle: () => Promise }; + model: Model | undefined = TEST_MODELS[0]; + thinkingLevel: string | undefined; + customCommands: [] = []; + extensionRunner = undefined; + isStreaming = false; + queuedMessageCount = 0; + systemPrompt = "system"; + disposed = false; + + constructor(cwd: string) { + this.sessionManager = SessionManager.create(cwd); + this.sessionId = this.sessionManager.getSessionId(); + this.agent = { sessionId: this.sessionId, waitForIdle: async () => {} }; + } + + get sessionName(): string { + return this.sessionManager.getHeader()?.title ?? `Session ${this.sessionId}`; + } + + get modelRegistry(): { getApiKey: (model: Model) => Promise } { + return { getApiKey: async (_model: Model) => "test-key" }; + } + + getAvailableModels(): Model[] { + return TEST_MODELS; + } + + getAvailableThinkingLevels(): ReadonlyArray { + return ["low", "medium", "high"]; + } + + setThinkingLevel(): void {} + async setModel(): Promise {} + subscribe(): () => void { + return () => {}; + } + async prompt(): Promise {} + async abort(): Promise {} + async refreshMCPTools(): Promise {} + getContextUsage(): undefined { + return undefined; + } + async switchSession(): Promise { + return false; + } + async dispose(): Promise { + this.disposed = true; + await this.sessionManager.close(); + } + async reload(): Promise {} + async newSession(): Promise { + return false; + } + async branch(): Promise<{ cancelled: boolean }> { + return { cancelled: false }; + } + async navigateTree(): Promise<{ cancelled: boolean }> { + return { cancelled: false }; + } + getActiveToolNames(): string[] { + return []; + } + getAllToolNames(): string[] { + return []; + } + setActiveToolsByName(): void {} + setClientBridge(): void {} + getPlanModeState(): undefined { + return undefined; + } + setPlanModeState(): void {} + async sendCustomMessage(): Promise {} + async sendUserMessage(): Promise {} + async compact(): Promise {} + async fork(): Promise { + return false; + } +} + +const cleanupRoots: string[] = []; +const originalAgentDir = process.env.PI_CODING_AGENT_DIR; +const fallbackAgentDir = path.join(getConfigRootDir(), "agent"); + +afterEach(async () => { + if (originalAgentDir) { + setAgentDir(originalAgentDir); + } else { + setAgentDir(fallbackAgentDir); + delete process.env.PI_CODING_AGENT_DIR; + } + for (const root of cleanupRoots.splice(0)) { + await fs.promises.rm(root, { recursive: true, force: true }); + } +}); + +async function createAgent(): Promise { + const root = await fs.promises.mkdtemp(path.join(os.tmpdir(), "omp-acp-init-")); + cleanupRoots.push(root); + const agentDir = path.join(root, "agent"); + const cwd = path.join(root, "cwd"); + await fs.promises.mkdir(agentDir, { recursive: true }); + await fs.promises.mkdir(cwd, { recursive: true }); + setAgentDir(agentDir); + + const abortController = new AbortController(); + const connection = { + sessionUpdate: async () => {}, + signal: abortController.signal, + closed: Promise.withResolvers().promise, + } as unknown as AgentSideConnection; + + const initialSession = new FakeAgentSession(cwd); + const factory = async (next: string): Promise => new FakeAgentSession(next) as unknown as AgentSession; + return new AcpAgent(connection, initialSession as unknown as AgentSession, factory); +} + +function buildInitializeRequest(overrides: Partial = {}): InitializeRequest { + return { + protocolVersion: 1, + clientCapabilities: {}, + ...overrides, + } as InitializeRequest; +} + +describe("ACP initialize conformance", () => { + it("only advertises the agent-managed auth method when the client lacks terminal capability", async () => { + const agent = await createAgent(); + const response = await agent.initialize(buildInitializeRequest()); + expectAcpStructure(zInitializeResponse, response); + expect(response.authMethods).toHaveLength(1); + const [agentMethod] = response.authMethods!; + // AuthMethodAgent omits the `type` discriminator per ACP spec — the absence is the signal. + expect((agentMethod as { type?: string }).type).toBeUndefined(); + expect(agentMethod).toEqual( + expect.objectContaining({ + id: "agent", + name: expect.any(String), + description: expect.any(String), + }), + ); + }); + + it("appends the terminal setup method when the client opts in via clientCapabilities.auth.terminal", async () => { + const agent = await createAgent(); + const response = await agent.initialize( + buildInitializeRequest({ clientCapabilities: { auth: { terminal: true } } }), + ); + expectAcpStructure(zInitializeResponse, response); + expect(response.authMethods).toHaveLength(2); + const [first, second] = response.authMethods!; + expect((first as { type?: string }).type).toBeUndefined(); + expect(first).toEqual(expect.objectContaining({ id: "agent" })); + expect(response.authMethods![1]).toEqual( + expect.objectContaining({ + type: "terminal", + id: "terminal", + args: [ACP_TERMINAL_AUTH_FLAG], + }), + ); + void second; + }); + + it("uses a terminal auth arg that removes ACP mode before launching the interactive setup flow", () => { + const result = prepareAcpTerminalAuthArgs(["--mode", "acp", "--no-extensions", ACP_TERMINAL_AUTH_FLAG]); + + expect(result).toEqual({ + args: ["--no-extensions"], + terminalAuth: true, + }); + expect(prepareAcpTerminalAuthArgs(["--mode=acp", ACP_TERMINAL_AUTH_FLAG])).toEqual({ + args: [], + terminalAuth: true, + }); + }); + + it("declares agentInfo.version that matches the published package version", async () => { + const agent = await createAgent(); + const response = await agent.initialize(buildInitializeRequest()); + const pkgPath = path.join(import.meta.dir, "..", "package.json"); + const pkg = (await Bun.file(pkgPath).json()) as { version: string }; + expect(response.agentInfo).toEqual( + expect.objectContaining({ + name: "oh-my-pi", + title: "Oh My Pi", + version: VERSION, + }), + ); + expect(response.agentInfo!.version).toBe(pkg.version); + }); + + it("preserves the agentCapabilities contract clients depend on", async () => { + const agent = await createAgent(); + const response = await agent.initialize(buildInitializeRequest()); + expectAcpStructure(zInitializeResponse, response); + expect(response.agentCapabilities).toEqual( + expect.objectContaining({ + loadSession: true, + mcpCapabilities: expect.objectContaining({ http: true, sse: true }), + promptCapabilities: expect.objectContaining({ embeddedContext: true, image: true }), + sessionCapabilities: expect.objectContaining({ + list: expect.any(Object), + fork: expect.any(Object), + resume: expect.any(Object), + close: expect.any(Object), + }), + }), + ); + }); +}); diff --git a/packages/coding-agent/test/acp-stdout-hygiene.test.ts b/packages/coding-agent/test/acp-stdout-hygiene.test.ts new file mode 100644 index 000000000..8f4405f47 --- /dev/null +++ b/packages/coding-agent/test/acp-stdout-hygiene.test.ts @@ -0,0 +1,109 @@ +/** + * ACP stdout-hygiene smoke: launching `omp acp` must not leak any banner, + * progress text, or stray non-JSON bytes onto stdout — that channel is owned + * by the JSON-RPC protocol. We spawn the CLI as a subprocess, send a single + * `initialize` frame, and assert the first stdout line parses cleanly as a + * JSON-RPC response. + */ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +const repoRoot = path.resolve(import.meta.dir, "..", "..", ".."); +const cliEntry = path.join(repoRoot, "packages", "coding-agent", "src", "cli.ts"); + +const cleanupRoots: string[] = []; +let activeProc: ReturnType | undefined; + +afterEach(async () => { + if (activeProc) { + try { + activeProc.kill(); + await activeProc.exited; + } catch { + // ignore + } + activeProc = undefined; + } + for (const root of cleanupRoots.splice(0)) { + await fs.promises.rm(root, { recursive: true, force: true }); + } +}); + +async function readFirstFrame(stream: ReadableStream): Promise { + const reader = stream.getReader(); + const decoder = new TextDecoder(); + let buffer = ""; + while (true) { + const { value, done } = await reader.read(); + if (done) break; + buffer += decoder.decode(value, { stream: true }); + const newlineIdx = buffer.indexOf("\n"); + if (newlineIdx >= 0) { + reader.releaseLock(); + return buffer.slice(0, newlineIdx); + } + } + reader.releaseLock(); + return buffer; +} + +describe("ACP stdout hygiene", () => { + it("emits a JSON-RPC initialize response as the first bytes on stdout", async () => { + const root = await fs.promises.mkdtemp(path.join(os.tmpdir(), "omp-acp-stdout-")); + cleanupRoots.push(root); + const home = path.join(root, "home"); + const xdg = path.join(root, "xdg"); + const agentDir = path.join(root, "agent"); + await fs.promises.mkdir(home, { recursive: true }); + await fs.promises.mkdir(xdg, { recursive: true }); + await fs.promises.mkdir(agentDir, { recursive: true }); + + const proc = Bun.spawn(["bun", cliEntry, "acp"], { + cwd: repoRoot, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + env: { + ...process.env, + HOME: home, + XDG_DATA_HOME: xdg, + XDG_CONFIG_HOME: xdg, + PI_CODING_AGENT_DIR: agentDir, + PI_NO_TITLE: "1", + }, + }); + activeProc = proc; + + const initRequest = { + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { protocolVersion: 1, clientCapabilities: { auth: { terminal: true } } }, + }; + proc.stdin.write(new TextEncoder().encode(`${JSON.stringify(initRequest)}\n`)); + proc.stdin.flush(); + + const firstLine = await readFirstFrame(proc.stdout as ReadableStream); + expect(firstLine.length).toBeGreaterThan(0); + expect(firstLine[0]).toBe("{"); + + const message = JSON.parse(firstLine) as { + jsonrpc?: string; + id?: unknown; + result?: { protocolVersion?: number; authMethods?: Array<{ type?: string; id?: string }> }; + error?: unknown; + }; + expect(message.jsonrpc).toBe("2.0"); + expect(message.id).toBe(1); + expect(message.error).toBeUndefined(); + expect(message.result?.protocolVersion).toBe(1); + expect(message.result?.authMethods).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: "agent" }), + expect.objectContaining({ type: "terminal", id: "terminal" }), + ]), + ); + }, 20_000); +}); diff --git a/packages/coding-agent/test/agent-session-acp-permission.test.ts b/packages/coding-agent/test/agent-session-acp-permission.test.ts new file mode 100644 index 000000000..c7110b97e --- /dev/null +++ b/packages/coding-agent/test/agent-session-acp-permission.test.ts @@ -0,0 +1,242 @@ +/** + * Tests for the ACP permission gate in AgentSession. + * + * Verifies that sensitive tools (bash, edit, write, ast_edit) are gated behind + * `ClientBridge.requestPermission` when a bridge is set, and that allow/reject + * decisions are cached appropriately for allow_always / reject_always. + */ +import { afterEach, beforeEach, expect, it, spyOn } from "bun:test"; +import * as path from "node:path"; +import { Agent, type AgentTool } from "@oh-my-pi/pi-agent-core"; +import { getBundledModel } from "@oh-my-pi/pi-ai"; +import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; +import type { ClientBridge, ClientBridgePermissionOutcome } from "@oh-my-pi/pi-coding-agent/session/client-bridge"; +import { convertToLlm } from "@oh-my-pi/pi-coding-agent/session/messages"; +import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { TempDir } from "@oh-my-pi/pi-utils"; +import { Type } from "@sinclair/typebox"; + +class MockAssistantStream extends AssistantMessageEventStream {} + +// --------------------------------------------------------------------------- +// Shared setup +// --------------------------------------------------------------------------- + +let tempDir: TempDir; +let authStorage: AuthStorage | undefined; +let session: AgentSession; + +/** Fake bash tool that records execute calls. */ +function makeFakeTool(name: string): AgentTool & { executeCalls: number } { + const tool = { + name, + label: name, + description: `Fake ${name}`, + parameters: Type.Object({ command: Type.Optional(Type.String()) }), + executeCalls: 0, + async execute() { + tool.executeCalls++; + return { content: [{ type: "text" as const, text: "ok" }] }; + }, + }; + return tool; +} + +/** Build a minimal ClientBridge whose requestPermission resolves to the given outcome. */ +function makeBridge(outcome: ClientBridgePermissionOutcome): ClientBridge { + return { + capabilities: { requestPermission: true }, + async requestPermission(_toolCall, _options, _signal) { + return outcome; + }, + }; +} + +async function createSession(tools: AgentTool[], bridge?: ClientBridge): Promise { + const model = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!model) throw new Error("Expected claude-sonnet-4-5 model to exist"); + + const settings = Settings.isolated({ "compaction.enabled": false }); + const sessionManager = SessionManager.inMemory(tempDir.path()); + const registry = new ModelRegistry(authStorage!, path.join(tempDir.path(), "models.yml")); + + const agent = new Agent({ + getApiKey: () => "test-key", + initialState: { + model, + systemPrompt: ["Test"], + tools, + messages: [], + }, + convertToLlm, + streamFn: () => new MockAssistantStream(), + }); + + const sess = new AgentSession({ + agent, + sessionManager, + settings, + modelRegistry: registry, + toolRegistry: new Map(tools.map(t => [t.name, t])), + }); + + if (bridge) sess.setClientBridge(bridge); + return sess; +} + +beforeEach(async () => { + tempDir = TempDir.createSync("@pi-acp-permission-test-"); + authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); + authStorage.setRuntimeApiKey("anthropic", "test-key"); +}); + +afterEach(async () => { + await session?.dispose(); + authStorage?.close(); + authStorage = undefined; + tempDir.removeSync(); +}); + +// --------------------------------------------------------------------------- +// 1. Allow once: bridge called once, underlying execute called once +// --------------------------------------------------------------------------- + +it("allow_once: calls bridge once and executes the underlying tool", async () => { + const bashTool = makeFakeTool("bash"); + const bridge = makeBridge({ outcome: "selected", optionId: "allow_once", kind: "allow_once" }); + const permissionSpy = spyOn(bridge, "requestPermission"); + session = await createSession([bashTool], bridge); + + await session.setActiveToolsByName(["bash"]); + // Get the wrapped tool from the agent's active set. + const wrappedBash = session.agent.state.tools.find(t => t.name === "bash"); + expect(wrappedBash).toBeDefined(); + + await wrappedBash!.execute("call-1", { command: "echo hi" }, undefined, undefined as never, undefined as never); + + expect(permissionSpy).toHaveBeenCalledTimes(1); + expect(bashTool.executeCalls).toBe(1); +}); + +it("setClientBridge wraps tools that were already active", async () => { + const bashTool = makeFakeTool("bash"); + const bridge = makeBridge({ outcome: "selected", optionId: "allow_once", kind: "allow_once" }); + const permissionSpy = spyOn(bridge, "requestPermission"); + session = await createSession([bashTool]); + + session.setClientBridge(bridge); + const wrappedBash = session.agent.state.tools.find(t => t.name === "bash"); + expect(wrappedBash).toBeDefined(); + + await wrappedBash!.execute("call-1", { command: "echo hi" }, undefined, undefined as never, undefined as never); + + expect(permissionSpy).toHaveBeenCalledTimes(1); + expect(bashTool.executeCalls).toBe(1); +}); + +it("aborting an open permission request rejects without executing the tool", async () => { + const bashTool = makeFakeTool("bash"); + const pending = Promise.withResolvers(); + const bridge: ClientBridge = { + capabilities: { requestPermission: true }, + requestPermission: async () => pending.promise, + }; + session = await createSession([bashTool], bridge); + await session.setActiveToolsByName(["bash"]); + const wrappedBash = session.agent.state.tools.find(t => t.name === "bash"); + expect(wrappedBash).toBeDefined(); + + const abortController = new AbortController(); + const execution = wrappedBash!.execute( + "call-1", + { command: "echo hi" }, + abortController.signal, + undefined as never, + undefined as never, + ); + abortController.abort(); + + await expect(execution).rejects.toThrow(/Permission request cancelled/); + expect(bashTool.executeCalls).toBe(0); + pending.resolve({ outcome: "cancelled" }); +}); + +// --------------------------------------------------------------------------- +// 2. Reject once: throws, underlying execute never called +// --------------------------------------------------------------------------- + +it("reject_once: throws ToolError and never calls underlying execute", async () => { + const editTool = makeFakeTool("edit"); + const bridge = makeBridge({ outcome: "selected", optionId: "reject_once", kind: "reject_once" }); + session = await createSession([editTool], bridge); + + await session.setActiveToolsByName(["edit"]); + const wrappedEdit = session.agent.state.tools.find(t => t.name === "edit"); + expect(wrappedEdit).toBeDefined(); + + await expect( + wrappedEdit!.execute("call-1", { path: "/tmp/foo.ts" }, undefined, undefined as never, undefined as never), + ).rejects.toThrow(/rejected by user/); + + expect(editTool.executeCalls).toBe(0); +}); + +// --------------------------------------------------------------------------- +// 3. Always allow caches: bridge called exactly once across two executions +// --------------------------------------------------------------------------- + +it("allow_always: caches decision and calls bridge only once for subsequent executes", async () => { + const writeTool = makeFakeTool("write"); + const bridge = makeBridge({ outcome: "selected", optionId: "allow_always", kind: "allow_always" }); + const permissionSpy = spyOn(bridge, "requestPermission"); + session = await createSession([writeTool], bridge); + + await session.setActiveToolsByName(["write"]); + const wrappedWrite = session.agent.state.tools.find(t => t.name === "write"); + expect(wrappedWrite).toBeDefined(); + + // First call — bridge is consulted, decision cached. + await wrappedWrite!.execute("call-1", { path: "/tmp/a.ts" }, undefined, undefined as never, undefined as never); + // Second call — must skip the bridge entirely. + await wrappedWrite!.execute("call-2", { path: "/tmp/b.ts" }, undefined, undefined as never, undefined as never); + + expect(permissionSpy).toHaveBeenCalledTimes(1); + expect(writeTool.executeCalls).toBe(2); +}); + +// --------------------------------------------------------------------------- +// 4. Read tool not gated: bridge never called even when bridge is set +// --------------------------------------------------------------------------- + +it("read tool: requestPermission is never called for non-gated tools", async () => { + const readTool = makeFakeTool("read"); + const bridge = makeBridge({ outcome: "selected", optionId: "allow_once", kind: "allow_once" }); + const permissionSpy = spyOn(bridge, "requestPermission"); + session = await createSession([readTool], bridge); + + await session.setActiveToolsByName(["read"]); + const wrappedRead = session.agent.state.tools.find(t => t.name === "read"); + expect(wrappedRead).toBeDefined(); + + await wrappedRead!.execute("call-1", {}, undefined, undefined as never, undefined as never); + + expect(permissionSpy).toHaveBeenCalledTimes(0); + expect(readTool.executeCalls).toBe(1); +}); + +// --------------------------------------------------------------------------- +// 5. No bridge → original tool object identity preserved (no wrapping) +// --------------------------------------------------------------------------- + +it("no bridge: original tool object is returned unchanged", async () => { + const bashTool = makeFakeTool("bash"); + session = await createSession([bashTool]); // no bridge + + await session.setActiveToolsByName(["bash"]); + const activeBash = session.agent.state.tools.find(t => t.name === "bash"); + expect(activeBash).toBe(bashTool); +}); diff --git a/packages/coding-agent/test/bash-acp-terminal.test.ts b/packages/coding-agent/test/bash-acp-terminal.test.ts new file mode 100644 index 000000000..25e91655e --- /dev/null +++ b/packages/coding-agent/test/bash-acp-terminal.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it, spyOn } from "bun:test"; +import type { ClientBridge, ClientBridgeTerminalHandle } from "../src/session/client-bridge"; +import type { ToolSession } from "../src/tools"; +import { BashTool } from "../src/tools/bash"; + +function makeSession(bridge: ClientBridge): ToolSession { + return { + cwd: "/tmp", + hasUI: false, + skills: [], + getSessionFile: () => null, + settings: { + get(key: string) { + if (key === "async.enabled") return false; + if (key === "bash.autoBackground.enabled") return false; + if (key === "bash.autoBackground.thresholdMs") return 60_000; + if (key === "bashInterceptor.enabled") return false; + if (key === "astGrep.enabled") return false; + if (key === "astEdit.enabled") return false; + if (key === "search.enabled") return false; + if (key === "find.enabled") return false; + return undefined; + }, + getBashInterceptorRules() { + return []; + }, + }, + getClientBridge: () => bridge, + } as unknown as ToolSession; +} + +describe("BashTool ACP terminal routing", () => { + it("routes through bridge, emits terminalId update, and releases the handle", async () => { + const stubText = "hello from terminal\n"; + + const handle: ClientBridgeTerminalHandle = { + terminalId: "term-xyz", + waitForExit: async () => ({ exitCode: 0, signal: null }), + currentOutput: async () => ({ output: stubText, truncated: false }), + kill: async () => {}, + release: async () => {}, + }; + + const bridge: ClientBridge = { + capabilities: { terminal: true }, + createTerminal: async () => handle, + }; + + const createSpy = spyOn(bridge, "createTerminal"); + const releaseSpy = spyOn(handle, "release"); + + const updates: Array<{ details?: { terminalId?: string } }> = []; + + const tool = new BashTool(makeSession(bridge)); + const result = await tool.execute("call-1", { command: "echo hi" }, undefined, update => { + updates.push(update as { details?: { terminalId?: string } }); + }); + + // createTerminal must be called with the expanded command + expect(createSpy).toHaveBeenCalledTimes(1); + const params = createSpy.mock.calls[0]![0]; + expect(params.command).toBe("echo hi"); + + // The first onUpdate must carry the terminalId so the editor can embed it + expect(updates.length).toBeGreaterThanOrEqual(1); + expect(updates[0]!.details?.terminalId).toBe("term-xyz"); + + // The final result text must contain the stub output + const text = result.content.find(c => c.type === "text"); + expect(text?.text).toContain("hello from terminal"); + + // The result details must carry terminalId for the ACP event mapper + expect(result.details?.terminalId).toBe("term-xyz"); + + // The handle must always be released + expect(releaseSpy).toHaveBeenCalledTimes(1); + }); + + it("kills and releases the client terminal when the command times out", async () => { + const pendingExit = Promise.withResolvers<{ exitCode: number | null; signal: string | null }>(); + const handle: ClientBridgeTerminalHandle = { + terminalId: "term-timeout", + waitForExit: async () => pendingExit.promise, + currentOutput: async () => ({ output: "", truncated: false }), + kill: async () => {}, + release: async () => {}, + }; + const bridge: ClientBridge = { + capabilities: { terminal: true }, + createTerminal: async () => handle, + }; + const killSpy = spyOn(handle, "kill"); + const releaseSpy = spyOn(handle, "release"); + + const tool = new BashTool(makeSession(bridge)); + + await expect(tool.execute("call-timeout", { command: "sleep 60", timeout: 1 })).rejects.toThrow( + /Command timed out after 1 seconds/, + ); + + expect(killSpy).toHaveBeenCalledTimes(1); + expect(releaseSpy).toHaveBeenCalledTimes(1); + pendingExit.resolve({ exitCode: null, signal: "TERM" }); + }); +}); diff --git a/packages/coding-agent/test/edit-per-file-diff-content.test.ts b/packages/coding-agent/test/edit-per-file-diff-content.test.ts new file mode 100644 index 000000000..bf3e4ce0a --- /dev/null +++ b/packages/coding-agent/test/edit-per-file-diff-content.test.ts @@ -0,0 +1,164 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { _resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { + DEFAULT_FUZZY_THRESHOLD, + EditTool, + type EditToolDetails, + executePatchSingle, + executeReplaceSingle, +} from "@oh-my-pi/pi-coding-agent/edit"; +import { writethroughNoop } from "@oh-my-pi/pi-coding-agent/lsp"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; + +// ─── Minimal ToolSession stub ──────────────────────────────────────────────── + +function makeSession(cwd: string): ToolSession { + return { + cwd, + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + enableLsp: false, + settings: Settings.isolated({ "edit.mode": "patch" }), + getArtifactsDir: () => null, + getSessionId: () => null, + getPlanModeState: () => undefined, + } as unknown as ToolSession; +} + +const noopBeginDeferred = (_p: string) => ({ + onDeferredDiagnostics: () => {}, + signal: new AbortController().signal, + finalize: () => {}, +}); + +// ─── Setup / teardown ──────────────────────────────────────────────────────── + +let tempDir: string; + +beforeEach(async () => { + _resetSettingsForTest(); + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-edit-diff-")); + await Settings.init({ inMemory: true, cwd: tempDir }); +}); + +afterEach(async () => { + _resetSettingsForTest(); + await fs.rm(tempDir, { recursive: true, force: true }); +}); + +// ─── executePatchSingle ─────────────────────────────────────────────────────── + +describe("executePatchSingle — oldText/newText propagation", () => { + test("update: oldText is pre-edit content, newText is post-edit content", async () => { + await Bun.write(path.join(tempDir, "foo.txt"), "a\n"); + + const result = await executePatchSingle({ + session: makeSession(tempDir), + path: "foo.txt", + params: { op: "update", diff: "@@\n-a\n+b" }, + allowFuzzy: true, + fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD, + writethrough: writethroughNoop, + beginDeferredDiagnosticsForPath: noopBeginDeferred, + }); + + expect(result.details?.path).toBe("foo.txt"); + expect(result.details?.oldText).toBe("a\n"); + expect(result.details?.newText).toBe("b\n"); + }); + + test("create: oldText is undefined, newText is the created content", async () => { + const result = await executePatchSingle({ + session: makeSession(tempDir), + path: "new.txt", + params: { op: "create", diff: "hello\n" }, + allowFuzzy: true, + fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD, + writethrough: writethroughNoop, + beginDeferredDiagnosticsForPath: noopBeginDeferred, + }); + + expect(result.details?.path).toBe("new.txt"); + expect(result.details?.oldText).toBeUndefined(); + expect(result.details?.newText).toBe("hello\n"); + }); + + test("delete: oldText is prior content, newText is undefined", async () => { + await Bun.write(path.join(tempDir, "gone.txt"), "will be deleted\n"); + + const result = await executePatchSingle({ + session: makeSession(tempDir), + path: "gone.txt", + params: { op: "delete" }, + allowFuzzy: true, + fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD, + writethrough: writethroughNoop, + beginDeferredDiagnosticsForPath: noopBeginDeferred, + }); + + expect(result.details?.path).toBe("gone.txt"); + expect(result.details?.oldText).toBe("will be deleted\n"); + expect(result.details?.newText).toBeUndefined(); + }); +}); + +describe("EditTool patch aggregation — oldText/newText propagation", () => { + test("create followed by update preserves create-shaped oldText", async () => { + const tool = new EditTool(makeSession(tempDir)); + + const result = await tool.execute("call-create-update", { + path: "created.txt", + edits: [ + { op: "create", diff: "a\n" }, + { op: "update", diff: "@@\n-a\n+b" }, + ], + }); + const details = result.details as EditToolDetails; + expect(details.path).toBe("created.txt"); + expect("oldText" in details).toBe(true); + expect(details.oldText).toBeUndefined(); + expect(details.newText).toBe("b\n"); + }); + + test("update followed by delete preserves delete-shaped newText", async () => { + await Bun.write(path.join(tempDir, "updated-then-gone.txt"), "a\n"); + const tool = new EditTool(makeSession(tempDir)); + + const result = await tool.execute("call-update-delete", { + path: "updated-then-gone.txt", + edits: [{ op: "update", diff: "@@\n-a\n+b" }, { op: "delete" }], + }); + const details = result.details as EditToolDetails; + expect(details.path).toBe("updated-then-gone.txt"); + expect(details.oldText).toBe("a\n"); + expect("newText" in details).toBe(true); + expect(details.newText).toBeUndefined(); + }); +}); + +// ─── executeReplaceSingle ───────────────────────────────────────────────────── + +describe("executeReplaceSingle — oldText/newText propagation", () => { + test("replace: oldText is full file before, newText is full file after", async () => { + const originalContent = "line one\nline two\nline three\n"; + await Bun.write(path.join(tempDir, "bar.txt"), originalContent); + + const result = await executeReplaceSingle({ + session: makeSession(tempDir), + path: "bar.txt", + params: { old_text: "line two", new_text: "line TWO" }, + allowFuzzy: false, + fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD, + writethrough: writethroughNoop, + beginDeferredDiagnosticsForPath: noopBeginDeferred, + }); + + expect(result.details?.path).toBe("bar.txt"); + expect(result.details?.oldText).toBe(originalContent); + expect(result.details?.newText).toBe("line one\nline TWO\nline three\n"); + }); +}); diff --git a/packages/coding-agent/test/helpers/acp-schema.ts b/packages/coding-agent/test/helpers/acp-schema.ts new file mode 100644 index 000000000..e4d4d05ac --- /dev/null +++ b/packages/coding-agent/test/helpers/acp-schema.ts @@ -0,0 +1,16 @@ +import { expect } from "bun:test"; +import type * as z from "zod/v4"; + +function formatIssues(error: z.ZodError): string { + return error.issues.map(issue => `${issue.path.join(".") || ""}: ${issue.message}`).join("\n"); +} + +export function expectAcpStructure(schema: z.ZodType, value: unknown): void { + const result = schema.safeParse(value); + expect(result.success, result.success ? undefined : formatIssues(result.error)).toBe(true); +} + +export function expectAcpStructureRejects(schema: z.ZodType, value: unknown): void { + const result = schema.safeParse(value); + expect(result.success).toBe(false); +} diff --git a/packages/coding-agent/test/read-acp-fs.test.ts b/packages/coding-agent/test/read-acp-fs.test.ts new file mode 100644 index 000000000..76a26a17a --- /dev/null +++ b/packages/coding-agent/test/read-acp-fs.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import type { AgentToolResult } from "@oh-my-pi/pi-agent-core"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { ClientBridge } from "@oh-my-pi/pi-coding-agent/session/client-bridge"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; +import type { ReadToolDetails } from "@oh-my-pi/pi-coding-agent/tools/read"; +import { ReadTool } from "@oh-my-pi/pi-coding-agent/tools/read"; + +const BRIDGE_CONTENT = "// content from editor buffer\nexport function greet() { return 'bridge'; }\n"; + +function textOutput(result: AgentToolResult): string { + return result.content + .filter(c => c.type === "text") + .map(c => c.text) + .join("\n"); +} + +function createSession(cwd: string, bridge?: ClientBridge): ToolSession { + return { + cwd, + hasUI: false, + getSessionFile: () => path.join(cwd, "session.jsonl"), + getSessionSpawns: () => "*", + getArtifactsDir: () => path.join(cwd, "artifacts"), + allocateOutputArtifact: async () => ({ id: "artifact-1", path: path.join(cwd, "artifact-1.log") }), + settings: Settings.isolated(), + getClientBridge: bridge ? () => bridge : undefined, + }; +} + +describe("read tool ACP fs routing", () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "read-acp-fs-test-")); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + it("routes plain text reads through the bridge and does not call Bun.file().text()", async () => { + // .ts file so summarize would normally run (read.summarize.enabled defaults to true) + const filePath = path.join(tmpDir, "example.ts"); + await fs.writeFile(filePath, "export function greet() { return 'disk'; }\n"); + + const bridge: ClientBridge = { + capabilities: { readTextFile: true }, + readTextFile: async () => BRIDGE_CONTENT, + }; + const bridgeSpy = spyOn(bridge, "readTextFile"); + + // Wrap Bun.file() to detect any .text() calls + let textCallCount = 0; + const origBunFile = Bun.file.bind(Bun); + const bunFileSpy = spyOn(Bun, "file").mockImplementation( + (arg: string | URL | Uint8Array | ArrayBufferLike | number, opts?: BlobPropertyBag) => { + const bunFile = origBunFile(arg as string, opts); + const origText = bunFile.text.bind(bunFile); + bunFile.text = async () => { + textCallCount++; + return origText(); + }; + return bunFile; + }, + ); + + try { + const session = createSession(tmpDir, bridge); + const tool = new ReadTool(session); + + const result = await tool.execute("call-1", { path: filePath }); + const text = textOutput(result); + + // Bridge content should appear in output + expect(text).toContain("content from editor buffer"); + // Bridge readTextFile was invoked + expect(bridgeSpy).toHaveBeenCalled(); + // Bun.file().text() must not have been called — bridge is source of truth + expect(textCallCount).toBe(0); + } finally { + bunFileSpy.mockRestore(); + } + }); + + it("applies requested line ranges to bridge content exactly once", async () => { + const filePath = path.join(tmpDir, "range.txt"); + await fs.writeFile(filePath, "disk one\ndisk two\ndisk three\n"); + const bridgeContent = "bridge one\nbridge two\nbridge three\n"; + const bridge: ClientBridge = { + capabilities: { readTextFile: true }, + readTextFile: async params => { + if (typeof params.line !== "number") return bridgeContent; + const lines = bridgeContent.split("\n"); + const start = Math.max(0, params.line - 1); + return lines.slice(start, params.limit === undefined ? undefined : start + params.limit).join("\n"); + }, + }; + + const session = createSession(tmpDir, bridge); + const tool = new ReadTool(session); + + const result = await tool.execute("call-range", { path: `${filePath}:2+1` }); + const text = textOutput(result); + + expect(text).toContain("bridge two"); + expect(text).not.toContain("Line 2 is beyond end"); + expect(text).not.toContain("disk two"); + }); +}); diff --git a/packages/coding-agent/test/write-acp-fs.test.ts b/packages/coding-agent/test/write-acp-fs.test.ts new file mode 100644 index 000000000..a376b0d77 --- /dev/null +++ b/packages/coding-agent/test/write-acp-fs.test.ts @@ -0,0 +1,62 @@ +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { ClientBridge } from "@oh-my-pi/pi-coding-agent/session/client-bridge"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; +import { WriteTool } from "@oh-my-pi/pi-coding-agent/tools/write"; + +const FILE_CONTENT = "bridge write content\n"; + +function createSession(cwd: string, bridge?: ClientBridge): ToolSession { + return { + cwd, + hasUI: false, + getSessionFile: () => path.join(cwd, "session.jsonl"), + getSessionSpawns: () => "*", + getArtifactsDir: () => path.join(cwd, "artifacts"), + allocateOutputArtifact: async () => ({ id: "artifact-1", path: path.join(cwd, "artifact-1.log") }), + settings: Settings.isolated(), + getClientBridge: bridge ? () => bridge : undefined, + }; +} + +describe("write tool ACP fs routing", () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-acp-fs-test-")); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + it("routes plain text writes through the bridge and does not call Bun.write", async () => { + const filePath = path.join(tmpDir, "output.txt"); + + const bridge: ClientBridge = { + capabilities: { writeTextFile: true }, + writeTextFile: async () => undefined, + }; + + const bridgeSpy = spyOn(bridge, "writeTextFile"); + const bunWriteSpy = spyOn(Bun, "write"); + + try { + const session = createSession(tmpDir, bridge); + const tool = new WriteTool(session); + + await tool.execute("call-1", { path: filePath, content: FILE_CONTENT }); + + // Bridge was called with the exact path and content + expect(bridgeSpy).toHaveBeenCalledTimes(1); + expect(bridgeSpy).toHaveBeenCalledWith({ path: filePath, content: FILE_CONTENT }); + // Disk write must not have been called — bridge is the destination + expect(bunWriteSpy).not.toHaveBeenCalled(); + } finally { + bunWriteSpy.mockRestore(); + } + }); +});