From 513f2a9ea2e0709fec1f41d61785ce5d9654a535 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:19:39 +0200 Subject: [PATCH] List a generic ACP agent's slash commands in a new agent's / menu (#5411) * List a generic ACP agent's slash commands in a new agent's / menu Generic ACP agents advertise slash commands with available_commands_update after session/new. The new-agent / menu opens a short-lived session and asks for its commands right after session/new, so it always got an empty list, which the app keeps for the draft. Wait for the first batch, bounded by the existing initial-commands timeout, as the Cursor, Kiro, and TRAE clients do. Closes #4759 * Keep the generic ACP command test independent of scheduling * Wait for agent state writes before removing MCP test temp dirs The blocking send_agent_prompt timeout test removed its temp dir while AgentManager background tasks and AgentStorage writes were still in flight, which intermittently failed cleanup with ENOTEMPTY on Linux CI. Flush both before removing the dir in the tests that use real storage. --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com> --- .../src/server/agent/mcp-server.test.ts | 18 ++- .../generic-acp-agent.commands.test.ts | 108 ++++++++++++++++++ .../agent/providers/generic-acp-agent.test.ts | 1 + .../agent/providers/generic-acp-agent.ts | 4 +- 4 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 packages/server/src/server/agent/providers/generic-acp-agent.commands.test.ts diff --git a/packages/server/src/server/agent/mcp-server.test.ts b/packages/server/src/server/agent/mcp-server.test.ts index 4dd1726ec8..e695c84048 100644 --- a/packages/server/src/server/agent/mcp-server.test.ts +++ b/packages/server/src/server/agent/mcp-server.test.ts @@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { describe, expect, it, vi } from "vitest"; -import { realpathSync, rmSync } from "node:fs"; +import { realpathSync } from "node:fs"; import { access, mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; import { join, resolve as resolvePath } from "node:path"; import { tmpdir } from "node:os"; @@ -192,6 +192,16 @@ async function removeTempDir(path: string): Promise { await rm(path, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); } +async function removeAgentStateDir( + agentManager: AgentManager, + storage: AgentStorage, + path: string, +): Promise { + await agentManager.flush(); + await storage.flush(); + await removeTempDir(path); +} + type AgentManagerSpies = ReturnType; type AgentStorageSpies = ReturnType; @@ -3420,7 +3430,7 @@ describe("create_agent MCP tool", () => { expect(storedChild?.workspaceId).toBe("wks_parent"); expect(storedChild?.labels[PARENT_AGENT_ID_LABEL]).toBe(parent.id); } finally { - rmSync(workdir, { recursive: true, force: true }); + await removeAgentStateDir(agentManager, storage, workdir); } }); @@ -4012,7 +4022,7 @@ describe("send_agent_prompt MCP tool", () => { }); } finally { vi.useRealTimers(); - rmSync(workdir, { recursive: true, force: true }); + await removeAgentStateDir(agentManager, storage, workdir); } }); it("notifies the caller once when it prompts a created child that is still running", async () => { @@ -4072,7 +4082,7 @@ describe("send_agent_prompt MCP tool", () => { await new Promise((resolve) => setTimeout(resolve, 100)); expect(finishNotifications()).toHaveLength(1); } finally { - rmSync(workdir, { recursive: true, force: true }); + await removeAgentStateDir(agentManager, storage, workdir); } }); }); diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.commands.test.ts b/packages/server/src/server/agent/providers/generic-acp-agent.commands.test.ts new file mode 100644 index 0000000000..b53be4c5a3 --- /dev/null +++ b/packages/server/src/server/agent/providers/generic-acp-agent.commands.test.ts @@ -0,0 +1,108 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +import { describe, expect, test } from "vitest"; + +import { createTestLogger } from "../../../test-utils/test-logger.js"; +import { GenericACPAgentClient } from "./generic-acp-agent.js"; + +describe("GenericACPAgentClient slash commands", () => { + test("lists commands an agent advertises right after session/new", async () => { + await withFakeACPAgent("commands-after-session-new", async (command, cwd) => { + const client = new GenericACPAgentClient({ + logger: createTestLogger(), + command, + // Long enough that the update always lands inside the wait, however slowly + // the fake agent is scheduled. + initialCommandsWaitTimeoutMs: 60_000, + }); + const session = await client.createSession({ provider: "acp", cwd }); + try { + await expect(session.listCommands?.()).resolves.toEqual([ + { name: "review", description: "Review the diff", argumentHint: "", kind: "command" }, + ]); + } finally { + await session.close(); + } + }); + }); + + test("answers with no commands for an agent that never advertises any", async () => { + await withFakeACPAgent("silent", async (command, cwd) => { + const client = new GenericACPAgentClient({ + logger: createTestLogger(), + command, + initialCommandsWaitTimeoutMs: 50, + }); + const session = await client.createSession({ provider: "acp", cwd }); + try { + await expect(session.listCommands?.()).resolves.toEqual([]); + } finally { + await session.close(); + } + }); + }); +}); + +async function withFakeACPAgent( + mode: "commands-after-session-new" | "silent", + run: (command: [string, ...string[]], cwd: string) => Promise, +): Promise { + const testDir = await mkdtemp(path.join(tmpdir(), "paseo-acp-commands-")); + try { + const scriptPath = path.join(testDir, "fake-acp-agent.cjs"); + await writeFile(scriptPath, fakeACPAgentScript, "utf8"); + await run([process.execPath, scriptPath, mode], testDir); + } finally { + await rm(testDir, { recursive: true, force: true }); + } +} + +const fakeACPAgentScript = ` +const readline = require("node:readline"); + +const mode = process.argv[2]; +const rl = readline.createInterface({ input: process.stdin }); + +function write(message) { + process.stdout.write(JSON.stringify({ jsonrpc: "2.0", ...message }) + "\\n"); +} + +rl.on("line", (line) => { + const message = JSON.parse(line); + if (message.method === "initialize") { + write({ + id: message.id, + result: { + protocolVersion: message.params?.protocolVersion ?? 1, + agentCapabilities: { sessionCapabilities: { close: {} } }, + }, + }); + return; + } + + if (message.method === "session/new") { + write({ id: message.id, result: { sessionId: "session-1" } }); + if (mode === "commands-after-session-new") { + setTimeout(() => { + write({ + method: "session/update", + params: { + sessionId: "session-1", + update: { + sessionUpdate: "available_commands_update", + availableCommands: [{ name: "review", description: "Review the diff" }], + }, + }, + }); + }, 0); + } + return; + } + + if (message.id !== undefined) { + write({ id: message.id, result: {} }); + } +}); +`; diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.test.ts b/packages/server/src/server/agent/providers/generic-acp-agent.test.ts index 19a1e9f8ee..c266cc148b 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.test.ts @@ -62,6 +62,7 @@ describe("GenericACPAgentClient", () => { supportsRewindFiles: false, supportsRewindBoth: false, }, + waitForInitialCommands: true, }, ]); }); diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.ts b/packages/server/src/server/agent/providers/generic-acp-agent.ts index 3fcabb596e..c8f8960ea2 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.ts @@ -70,7 +70,9 @@ export class GenericACPAgentClient extends ACPAgentClient { }, defaultCommand: options.command, capabilities: buildGenericACPCapabilities(providerParams), - waitForInitialCommands: options.waitForInitialCommands, + // ACP agents advertise slash commands with available_commands_update after + // session/new, so the first listCommands() waits for that batch. + waitForInitialCommands: options.waitForInitialCommands ?? true, initialCommandsWaitTimeoutMs: options.initialCommandsWaitTimeoutMs, clientCapabilities: providerParams.clientCapabilities, clientCapabilityMeta: options.clientCapabilityMeta,