diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index e902493dcb..7e50b32cf5 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -1,6 +1,6 @@ import { type ChildProcess, type ChildProcessWithoutNullStreams } from "node:child_process"; import { EventEmitter } from "node:events"; -import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -2634,6 +2634,72 @@ describe("ACPAgentSession slash commands", () => { }, ]); }); + + test("lists late command batches through the public ACP client", async () => { + const artifacts = path.resolve(".artifacts"); + await mkdir(artifacts, { recursive: true }); + const directory = await mkdtemp(path.join(artifacts, "acp-command-inventory-")); + const script = path.join(directory, "agent.mjs"); + await writeFile( + script, + ` +import { createInterface } from "node:readline"; +const send = (message) => process.stdout.write(JSON.stringify(message) + "\\n"); +const help = { name: "help", description: "Show available commands" }; +const skill = { name: "xlsx", description: "Work with spreadsheets", input: { hint: "[instructions]" } }; +const publish = (commands) => send({ + jsonrpc: "2.0", method: "session/update", params: { + sessionId: "inventory-session", update: { + sessionUpdate: "available_commands_update", availableCommands: commands, + }, + }, +}); +createInterface({ input: process.stdin }).on("line", (line) => { + const request = JSON.parse(line); + if (request.method === "initialize") { + send({ jsonrpc: "2.0", id: request.id, result: { protocolVersion: 1, agentCapabilities: {} } }); + } else if (request.method === "session/new") { + // The first batch is already cached when createSession returns. + publish([help]); + send({ jsonrpc: "2.0", id: request.id, result: { sessionId: "inventory-session" } }); + setTimeout(() => publish([help]), 100); + setTimeout(() => publish([help, skill]), 350); + } else if (request.id !== undefined) { + send({ jsonrpc: "2.0", id: request.id, error: { code: -32601, message: "Method not found" } }); + } +}); +`, + ); + const client = new GenericACPAgentClient({ + logger: createTestLogger(), + command: [process.execPath, script], + }); + try { + const session = await client.createSession({ provider: "acp", cwd: directory }); + try { + const commands = await session.listCommands!(); + expect(commands).toEqual([ + { + name: "help", + description: "Show available commands", + argumentHint: "", + kind: "command", + }, + { + name: "xlsx", + description: "Work with spreadsheets", + argumentHint: "[instructions]", + kind: "command", + }, + ]); + expect(await session.listCommands!()).toEqual(commands); + } finally { + await session.close(); + } + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); }); describe("ACPAgentSession", () => { diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index d2973d42c7..6831274d2d 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -337,6 +337,14 @@ const ACP_CLOSE_REQUEST_TIMEOUT_MS = 2_000; const ACP_IMPORT_HISTORY_LOAD_TIMEOUT_MS = 30_000; const ACP_IMPORT_HISTORY_BUDGET_MS = 60_000; +/** + * An ACP agent publishes its slash-command inventory incrementally. Wait this + * long without another batch before treating the inventory as complete. MiniMax + * publishes its skills about 350ms after its built-in commands. + */ +const COMMAND_INVENTORY_QUIET_MS = 600; +const COMMAND_INVENTORY_POLL_MS = 25; + function summarizeMalformedACPStdoutError(error: unknown): { type: string; message: string } { return { type: error instanceof Error ? error.name : typeof error, @@ -1765,7 +1773,8 @@ export class ACPAgentSession implements AgentSession, ACPClient { private lastActivityAt: string | null = null; private configOptions: SessionConfigOption[] = []; private cachedCommands: AgentSlashCommand[] = []; - private commandsReadyDeferred: { promise: Promise; resolve: () => void } | null = null; + /** When the last command batch arrived; null while nothing has been published. */ + private commandsUpdatedAt: number | null = null; private commandsReadySettled = false; private waitForInitialCommands: boolean; private initialCommandsWaitTimeoutMs: number; @@ -2063,58 +2072,39 @@ export class ACPAgentSession implements AgentSession, ACPClient { ]; } - private ensureCommandsReadyDeferred(): void { - if (this.commandsReadyDeferred || this.commandsReadySettled || this.cachedCommands.length > 0) { - return; - } - - let resolve!: () => void; - const promise = new Promise((r) => { - resolve = r; - }); - this.commandsReadyDeferred = { promise, resolve }; - } - private settleCommandsReady(): void { - if (this.commandsReadySettled) { - return; - } this.commandsReadySettled = true; - this.commandsReadyDeferred?.resolve(); - this.commandsReadyDeferred = null; } - private async waitForCommandsReady(): Promise { - const deferred = this.commandsReadyDeferred; - if (!deferred) { - return; - } - - let timer: ReturnType | null = null; - try { - await Promise.race([ - deferred.promise, - new Promise((resolve) => { - timer = setTimeout(resolve, this.initialCommandsWaitTimeoutMs); - }), - ]); - } finally { - if (timer) { - clearTimeout(timer); - } + /** + * Providers publish their command inventory incrementally. MiniMax sends its + * built-in commands twice, then a third batch about 350ms later that merges in + * every discovered skill. Returning after the first batch loses the skills. + * + * Wait for the inventory to go quiet rather than for the first batch. The + * quiet window must exceed the gap between a provider's own batches; 600ms + * covers MiniMax with margin. `initialCommandsWaitTimeoutMs` still bounds the + * whole wait for providers that publish once or never publish at all. + */ + private async waitForCommandsToSettle(): Promise { + const deadline = Date.now() + this.initialCommandsWaitTimeoutMs; + while (!this.commandsReadySettled && !this.closed) { + const remaining = deadline - Date.now(); + if (remaining <= 0) return; + const quietFor = this.commandsUpdatedAt === null ? 0 : Date.now() - this.commandsUpdatedAt; + if (quietFor >= COMMAND_INVENTORY_QUIET_MS) return; + await new Promise((resolve) => + setTimeout(resolve, Math.min(remaining, COMMAND_INVENTORY_POLL_MS)), + ); } } async listCommands(): Promise { - if (this.cachedCommands.length > 0) { - return this.cachedCommands; - } - if (!this.waitForInitialCommands || this.closed) { + if (this.commandsReadySettled || this.closed || !this.waitForInitialCommands) { return this.cachedCommands; } - this.ensureCommandsReadyDeferred(); - await this.waitForCommandsReady(); + await this.waitForCommandsToSettle(); this.settleCommandsReady(); return this.cachedCommands; } @@ -2745,14 +2735,12 @@ export class ACPAgentSession implements AgentSession, ACPClient { } } - // Cache an asynchronously-delivered slash-command batch and unblock any - // listCommands() call that is waiting on the initial batch. Used when a - // provider supplies an extensionCommandsParser whose result arrives after - // session/new (e.g. via a vendor extension notification). The ready gate is - // always settled — even for an empty batch — so a provider that legitimately - // reports no commands does not leave listCommands() blocked for the full - // initial-commands timeout. An optional sessionId scopes the batch to this - // session; notifications addressed to a different session are ignored. + // Cache an asynchronously-delivered slash-command batch from a provider that + // supplies an extensionCommandsParser (e.g. a vendor extension notification). + // An empty batch is the provider saying it has no commands, so it settles the + // waiter immediately instead of letting it wait out the quiet window. An + // optional sessionId scopes the batch to this session; notifications + // addressed to a different session are ignored. private applyResolvedCommands( commands: AgentSlashCommand[], options?: { sessionId?: string }, @@ -2766,7 +2754,13 @@ export class ACPAgentSession implements AgentSession, ACPClient { } if (commands.length > 0) { + this.commandsUpdatedAt = Date.now(); this.cachedCommands = commands; + // A non-empty batch may be followed by more. Let the quiet window decide + // when the inventory is complete rather than settling here; an empty + // batch is the provider saying it has nothing, which must still unblock + // the waiter immediately. + return; } this.settleCommandsReady(); } @@ -3150,13 +3144,19 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.handleUsageUpdate(update); return pendingUserEvents; case "available_commands_update": + this.commandsUpdatedAt = Date.now(); this.cachedCommands = update.availableCommands.map((command) => ({ name: command.name, description: command.description, argumentHint: command.input?.hint ?? "", kind: "command", })); - this.settleCommandsReady(); + // An empty batch means the agent has no commands and would otherwise + // wait out the whole quiet window. A non-empty batch may be one of + // several, so completion belongs to the quiet window. + if (update.availableCommands.length === 0) { + this.settleCommandsReady(); + } return pendingUserEvents; default: return pendingUserEvents;