fix(acp): include late-discovered skills in initial command lists (#8387)

* fix(providers): wait for the ACP command inventory to settle

An ACP agent publishes its slash-command inventory in batches. MiniMax
Code sends its built-in commands first, then merges in every discovered
skill roughly 350ms later. `listCommands()` settled on the first batch,
so a session that had to discover its commands at creation time offered
the 10 built-in commands instead of the 396 the agent eventually
publishes. Every skill was missing from `/` until the session was
reopened and the cached inventory was read again.

Wait for the inventory to go quiet (600ms without another batch) instead
of for the first batch. An empty batch still settles immediately, and
`initialCommandsWaitTimeoutMs` (1500ms default) still bounds the whole
wait for providers that publish once or never publish at all.

* test(acp): verify late commands through the provider boundary

Exercise initial command discovery over real stdio, including a first batch cached before session creation returns. Keep the later skill and subsequent cached read in the same public session contract.

Co-authored-by: huazhongmin <peanut_hzm@163.com>

---------

Co-authored-by: huazhongmin <peanut_hzm@163.com>
Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
paseo-bot[bot]
2026-10-31 16:07:08 +00:00
committed by GitHub
co-authored by huazhongmin
parent e02fa9a5f8
commit eab9ac3b03
2 changed files with 118 additions and 52 deletions
@@ -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", () => {
@@ -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<void>; 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<void>((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<void> {
const deferred = this.commandsReadyDeferred;
if (!deferred) {
return;
}
let timer: ReturnType<typeof setTimeout> | null = null;
try {
await Promise.race([
deferred.promise,
new Promise<void>((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<void> {
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<void>((resolve) =>
setTimeout(resolve, Math.min(remaining, COMMAND_INVENTORY_POLL_MS)),
);
}
}
async listCommands(): Promise<AgentSlashCommand[]> {
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;