roboomp
185615497c
ACP clients own MCP server configuration via session/new.mcpServers and AcpAgent#configureMcpServers. The ACP session factory previously left enableMCP at its default (true), so createAgentSession ran discoverAndLoadMCPTools on every session/new and the resulting host MCP tools landed in the session tool registry alongside the client-supplied ones. search_tool_bm25 then surfaced only the host tools. Force enableMCP: false on every session created through createAcpSessionFactory so on-disk discovery is bypassed in ACP mode. Non-ACP modes (omp interactive, print, RPC) keep auto-discovery. Fixes #1234
77 lines
2.9 KiB
TypeScript
77 lines
2.9 KiB
TypeScript
/**
|
|
* Regression test for issue #1234.
|
|
*
|
|
* `omp acp` must not auto-discover host `.mcp.json` servers when creating a
|
|
* session for an ACP client. MCP server ownership belongs entirely to the ACP
|
|
* client (`session/new.mcpServers` → `AcpAgent#configureMcpServers`); letting
|
|
* `createAgentSession` run on-disk discovery in parallel registers host MCP
|
|
* tools that shadow the client-supplied ones in the session tool registry.
|
|
*
|
|
* The contract enforced here is narrow on purpose: every call routed through
|
|
* the ACP session factory must reach `createAgentSession` with
|
|
* `enableMCP: false`, regardless of what `baseOptions` carries.
|
|
*/
|
|
|
|
import { 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 { Snowflake } from "@oh-my-pi/pi-utils";
|
|
import { ModelRegistry } from "../src/config/model-registry";
|
|
import { Settings } from "../src/config/settings";
|
|
import { createAcpSessionFactory } from "../src/main";
|
|
import type { CreateAgentSessionOptions, CreateAgentSessionResult } from "../src/sdk";
|
|
import type { AgentSession } from "../src/session/agent-session";
|
|
import { AuthStorage } from "../src/session/auth-storage";
|
|
|
|
describe("createAcpSessionFactory MCP isolation (issue #1234)", () => {
|
|
it("forces enableMCP=false even when baseOptions opts in", async () => {
|
|
const tempDir = path.join(os.tmpdir(), `pi-acp-mcp-isolation-${Snowflake.next()}`);
|
|
fs.mkdirSync(tempDir, { recursive: true });
|
|
const authStorage = await AuthStorage.create(path.join(tempDir, "auth.db"));
|
|
try {
|
|
const modelRegistry = new ModelRegistry(authStorage);
|
|
const settings = Settings.isolated({});
|
|
const fakeSession = {} as AgentSession;
|
|
const captured: CreateAgentSessionOptions[] = [];
|
|
const createSession = async (options: CreateAgentSessionOptions): Promise<CreateAgentSessionResult> => {
|
|
captured.push(options);
|
|
return {
|
|
session: fakeSession,
|
|
extensionsResult: {
|
|
extensions: [],
|
|
errors: [],
|
|
runner: undefined,
|
|
} as unknown as CreateAgentSessionResult["extensionsResult"],
|
|
setToolUIContext: () => {},
|
|
eventBus: {
|
|
emit: () => {},
|
|
on: () => () => {},
|
|
off: () => {},
|
|
} as unknown as CreateAgentSessionResult["eventBus"],
|
|
};
|
|
};
|
|
|
|
// baseOptions deliberately sets enableMCP=true to prove the factory ignores it.
|
|
const factory = createAcpSessionFactory({
|
|
baseOptions: { enableMCP: true } as CreateAgentSessionOptions,
|
|
settings,
|
|
sessionDir: path.join(tempDir, "sessions"),
|
|
authStorage,
|
|
modelRegistry,
|
|
parsedArgs: {},
|
|
rawArgs: [],
|
|
createSession,
|
|
});
|
|
|
|
const result = await factory(tempDir);
|
|
expect(result).toBe(fakeSession);
|
|
expect(captured).toHaveLength(1);
|
|
expect(captured[0].enableMCP).toBe(false);
|
|
} finally {
|
|
authStorage.close();
|
|
fs.rmSync(tempDir, { recursive: true, force: true });
|
|
}
|
|
});
|
|
});
|