diff --git a/AGENTS.md b/AGENTS.md index 7371a1be0..65ed0df61 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,8 +129,8 @@ Release commits are the exception: keep using `Release v0.x.y`. - Registries (`registry*.ts`) assemble tool sets. Add new tools to the appropriate registry so AI chat, MCP, and CLI eval paths can see them. - AI adapter (`packages/core/src/tools/ai-adapter.ts`) converts ToolDefs to Vercel AI tools with valibot schemas. `src/app/ai/tools/index.ts` is a thin app wire that creates `FigmaAPI` from the active editor. - CLI commands in `packages/cli/src/commands/**` are not generated from ToolDefs; they own CLI UX, pagination, and agentfmt formatting. The `eval` command exposes ToolDef operations through `FigmaAPI`. -- MCP server code lives under `packages/mcp/src/`. MCP-only tools such as `open_file`, `new_document`, `save_file`, and `get_codegen_prompt` are registered in `tool/registration.ts` because they need server filesystem access or are not scene-graph tools. -- Local MCP transport discovery lives under `packages/mcp/src/transport/`: macOS/Linux prefer an owner-only Unix socket, Windows uses localhost TCP, and `mcp.json` advertises the active transport and token. Keep test discovery paths isolated from the user's runtime file. +- MCP server code lives under `packages/mcp/src/`. MCP-only tools such as `open_file`, `new_document`, `save_file`, and `get_codegen_prompt` are registered in `tool/registration.ts` because they need server filesystem access or are not scene-graph tools. Listener lifecycle and session ownership live under `src/server/`; the stdio client bridge lives under `src/stdio/`. +- Local MCP transport discovery lives under `packages/mcp/src/transport/`: macOS/Linux prefer an owner-only Unix socket, Windows uses localhost TCP, and `mcp.json` advertises the active transport and token. Keep transport tests grouped under `tests/engine/mcp/{server,stdio,transport}/`, shared MCP fixtures under `tests/helpers/mcp/`, and test discovery paths isolated from the user's runtime file. - `open_file` and `new_document` are only registered when `OPENPENCIL_MCP_ROOT` is set. Export tools can write files under that root when given a `path`; path checks must resolve symlinks before filesystem access. - Core codegen prompts live as markdown under `packages/core/src/tools/prompts/`; app chat/ACP prompts live under `src/app/ai/**` markdown files. - `FigmaAPI` (`packages/core/src/figma-api/`) is the execution target for tools and CLI eval. It is Figma Plugin API compatible and uses Symbols for hidden internals. diff --git a/bunfig.toml b/bunfig.toml index 6746ee7ae..5ddc325e3 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -2,6 +2,6 @@ # Preload discovery-file isolation for every `bun test` run so engine tests # never clobber the real ~/Library/Application Support/OpenPencil/mcp.json # (which would break a running OpenPencil desktop app). See -# tests/helpers/mcp-discovery-isolation.ts for the rationale. +# tests/helpers/mcp/discovery-isolation.ts for the rationale. [test] -preload = ["./tests/helpers/mcp-discovery-isolation.ts"] +preload = ["./tests/helpers/mcp/discovery-isolation.ts"] diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index a5cd47ad9..bac648467 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -13,7 +13,7 @@ import { createBrowserRpcBridge } from '#mcp/browser-rpc' import { MCP_CORS_HEADERS, MCP_CORS_METHODS, MCP_EXPOSED_HEADERS } from '#mcp/http-options' import type { RpcJsonObject } from '#mcp/json' import { preprocessRpc } from '#mcp/jsx-preprocess' -import { createMcpSessionManager } from '#mcp/mcp-sessions' +import { createMcpSessionManager } from '#mcp/server/sessions' import { registerTools } from '#mcp/tool/registration' import packageJson from '../package.json' with { type: 'json' } @@ -26,7 +26,7 @@ import { teardownListeners, tryStartTcp, tryWriteDiscovery -} from './lifecycle' +} from './server/lifecycle' export const MCP_VERSION: string = packageJson.version diff --git a/packages/mcp/src/lifecycle.ts b/packages/mcp/src/server/lifecycle.ts similarity index 100% rename from packages/mcp/src/lifecycle.ts rename to packages/mcp/src/server/lifecycle.ts diff --git a/packages/mcp/src/mcp-sessions.ts b/packages/mcp/src/server/sessions.ts similarity index 100% rename from packages/mcp/src/mcp-sessions.ts rename to packages/mcp/src/server/sessions.ts diff --git a/packages/mcp/src/stdio.ts b/packages/mcp/src/stdio.ts index 953561749..c6c5f1be2 100644 --- a/packages/mcp/src/stdio.ts +++ b/packages/mcp/src/stdio.ts @@ -3,7 +3,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import { MCP_VERSION, registerTools } from '#mcp/server' -import { createStdioRpcBridge } from '#mcp/stdio-bridge' +import { createStdioRpcBridge } from '#mcp/stdio/bridge' if (process.argv.includes('--help') || process.argv.includes('-h')) { process.stdout.write( diff --git a/packages/mcp/src/stdio-bridge.ts b/packages/mcp/src/stdio/bridge.ts similarity index 100% rename from packages/mcp/src/stdio-bridge.ts rename to packages/mcp/src/stdio/bridge.ts diff --git a/tests/engine/mcp/browser-rpc.test.ts b/tests/engine/mcp/browser-rpc.test.ts index d052bf3bb..6df4de98c 100644 --- a/tests/engine/mcp/browser-rpc.test.ts +++ b/tests/engine/mcp/browser-rpc.test.ts @@ -136,7 +136,7 @@ describe('BrowserRpcBridge reconnection', () => { servers.push(srv) } - test('pending requests from old browser are rejected on reconnect (Fix 1)', async () => { + test('rejects pending requests from a disconnected browser on reconnect', async () => { const pairA = await setupWsPair() track(pairA) const pairB = await setupWsPair() @@ -166,7 +166,7 @@ describe('BrowserRpcBridge reconnection', () => { expect(elapsed).toBeLessThan(5_000) }) - test('connection waiters survive handleClose and are NOT rejected (Fix 2)', async () => { + test('keeps connection waiters pending across browser reconnects', async () => { const pair = await setupWsPair() track(pair) @@ -179,8 +179,8 @@ describe('BrowserRpcBridge reconnection', () => { await registerBrowser(pair.serverWs, pair.clientWs, bridge) // Close the client side — this should trigger serverWs 'close' - // which calls bridge.handleClose(). Fix 2 ensures that connection - // waiters are NOT rejected here. + // which calls bridge.handleClose(). Connection waiters must remain + // pending here. pair.clientWs.close() await new Promise((resolve) => { setTimeout(resolve, 200) diff --git a/tests/engine/mcp/lifecycle-failure.test.ts b/tests/engine/mcp/server/cleanup.test.ts similarity index 99% rename from tests/engine/mcp/lifecycle-failure.test.ts rename to tests/engine/mcp/server/cleanup.test.ts index a71631b4b..9d5a9e2df 100644 --- a/tests/engine/mcp/lifecycle-failure.test.ts +++ b/tests/engine/mcp/server/cleanup.test.ts @@ -5,7 +5,7 @@ import { join } from 'node:path' import { startServer, type ServerHandle } from '#mcp/server' -import { socketRequest, type HealthResponse } from './helpers' +import { socketRequest, type HealthResponse } from '#tests/helpers/mcp/server' const isUnix = process.platform !== 'win32' const SOCKET_DIR = join(tmpdir(), `openpencil-test-lifecycle-${process.pid}`) diff --git a/tests/engine/mcp/server.test.ts b/tests/engine/mcp/server/index.test.ts similarity index 99% rename from tests/engine/mcp/server.test.ts rename to tests/engine/mcp/server/index.test.ts index b79bce59d..4107c0558 100644 --- a/tests/engine/mcp/server.test.ts +++ b/tests/engine/mcp/server/index.test.ts @@ -15,7 +15,7 @@ import { waitForBrowserRegistration, type HealthResponse, type MockBrowser -} from './helpers' +} from '#tests/helpers/mcp/server' const isUnix = process.platform !== 'win32' const SOCKET_DIR = join(tmpdir(), `openpencil-test-server-${process.pid}`) diff --git a/tests/engine/mcp/server-auth.test.ts b/tests/engine/mcp/server/security.test.ts similarity index 99% rename from tests/engine/mcp/server-auth.test.ts rename to tests/engine/mcp/server/security.test.ts index 95e93fb0b..97d5f2f2e 100644 --- a/tests/engine/mcp/server-auth.test.ts +++ b/tests/engine/mcp/server/security.test.ts @@ -11,7 +11,11 @@ import { SceneGraph } from '@open-pencil/scene-graph' import { startServer, paramToZod } from '#mcp/server' import type { DiscoveryInfo } from '#mcp/transport/discovery' -import { connectMockBrowser, waitForBrowserRegistration, type HealthResponse } from './helpers' +import { + connectMockBrowser, + waitForBrowserRegistration, + type HealthResponse +} from '#tests/helpers/mcp/server' const isUnix = process.platform !== 'win32' const SOCKET_DIR = join(tmpdir(), `openpencil-test-server-${process.pid}`) diff --git a/tests/engine/mcp/server-transport.test.ts b/tests/engine/mcp/server/transport.test.ts similarity index 99% rename from tests/engine/mcp/server-transport.test.ts rename to tests/engine/mcp/server/transport.test.ts index dd121ec03..3fa6f1873 100644 --- a/tests/engine/mcp/server-transport.test.ts +++ b/tests/engine/mcp/server/transport.test.ts @@ -19,7 +19,7 @@ import { waitForBrowserRegistration, type HealthResponse, type MockBrowser -} from './helpers' +} from '#tests/helpers/mcp/server' const isUnix = process.platform !== 'win32' const SOCKET_DIR = join(tmpdir(), `openpencil-test-server-${process.pid}`) diff --git a/tests/engine/mcp/stdio-auth.test.ts b/tests/engine/mcp/stdio/auth.test.ts similarity index 98% rename from tests/engine/mcp/stdio-auth.test.ts rename to tests/engine/mcp/stdio/auth.test.ts index 754183d49..d1009c6c0 100644 --- a/tests/engine/mcp/stdio-auth.test.ts +++ b/tests/engine/mcp/stdio/auth.test.ts @@ -5,7 +5,7 @@ import { createServer, type Server } from 'node:http' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { createStdioRpcBridge } from '#mcp/stdio-bridge' +import { createStdioRpcBridge } from '#mcp/stdio/bridge' const TEST_DIR = join(tmpdir(), `openpencil-test-stdio-auth-${process.pid}`) const TEST_SOCKET = join(TEST_DIR, 'mcp-test.sock') @@ -130,7 +130,7 @@ async function createBridgeAndWaitForReady( const isUnix = process.platform !== 'win32' -describe.skipIf(!isUnix)('Fix 4 - Auth token auto-discovery and transparent retry', () => { +describe.skipIf(!isUnix)('MCP stdio authentication', () => { let httpServer: Server | null = null let bridges: Array> = [] const origSocketEnv = process.env.OPENPENCIL_MCP_SOCKET diff --git a/tests/engine/mcp/stdio.test.ts b/tests/engine/mcp/stdio/index.test.ts similarity index 99% rename from tests/engine/mcp/stdio.test.ts rename to tests/engine/mcp/stdio/index.test.ts index 17d2da3db..21297a8a6 100644 --- a/tests/engine/mcp/stdio.test.ts +++ b/tests/engine/mcp/stdio/index.test.ts @@ -11,8 +11,11 @@ import { SceneGraph } from '@open-pencil/scene-graph' import { startServer, type ServerHandle } from '#mcp/server' import { expectDefined, getNodeOrThrow } from '#tests/helpers/assert' - -import { connectMockBrowser, waitForBrowserRegistration, type MockBrowser } from './helpers' +import { + connectMockBrowser, + waitForBrowserRegistration, + type MockBrowser +} from '#tests/helpers/mcp/server' const AUTH_TOKEN = 'test-stdio-token' const NO_DOCUMENT_AUTH_TOKEN = 'test-stdio-no-document-token' diff --git a/tests/engine/mcp/stdio-bridge-reconnect.test.ts b/tests/engine/mcp/stdio/reconnect.test.ts similarity index 99% rename from tests/engine/mcp/stdio-bridge-reconnect.test.ts rename to tests/engine/mcp/stdio/reconnect.test.ts index 896da7725..d663c07ed 100644 --- a/tests/engine/mcp/stdio-bridge-reconnect.test.ts +++ b/tests/engine/mcp/stdio/reconnect.test.ts @@ -5,7 +5,7 @@ import { createServer, type Server } from 'node:http' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { createStdioRpcBridge } from '#mcp/stdio-bridge' +import { createStdioRpcBridge } from '#mcp/stdio/bridge' import { getDiscoveryPath } from '#mcp/transport/paths' const isUnix = process.platform !== 'win32' diff --git a/tests/engine/mcp/discovery-isolation.test.ts b/tests/engine/mcp/test-support/discovery-isolation.test.ts similarity index 92% rename from tests/engine/mcp/discovery-isolation.test.ts rename to tests/engine/mcp/test-support/discovery-isolation.test.ts index 90c2e8f28..0fd46f5a1 100644 --- a/tests/engine/mcp/discovery-isolation.test.ts +++ b/tests/engine/mcp/test-support/discovery-isolation.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it } from 'bun:test' -import { isAliveFromKillError, isProcessAlive } from '#tests/helpers/mcp-discovery-isolation' +import { isAliveFromKillError, isProcessAlive } from '#tests/helpers/mcp/discovery-isolation' -describe('mcp-discovery-isolation > isProcessAlive', () => { +describe('MCP test discovery isolation', () => { it('returns true for the current (live) process', () => { // The stale-dir sweep must never treat the running process as dead. expect(isProcessAlive(process.pid)).toBe(true) diff --git a/tests/engine/mcp/transport.test.ts b/tests/engine/mcp/transport/paths.test.ts similarity index 99% rename from tests/engine/mcp/transport.test.ts rename to tests/engine/mcp/transport/paths.test.ts index ea1804eff..3d0dc1a21 100644 --- a/tests/engine/mcp/transport.test.ts +++ b/tests/engine/mcp/transport/paths.test.ts @@ -124,7 +124,7 @@ describe('transport/paths', () => { }) describe('getDiscoveryPath', () => { - // The test preload (tests/helpers/mcp-discovery-isolation.ts) sets + // The test preload (tests/helpers/mcp/discovery-isolation.ts) sets // OPENPENCIL_MCP_DISCOVERY_PATH to a per-process temp path. Each case // below saves/clears/restores both env vars so platform-default behavior // can be asserted independently of the preload. diff --git a/tests/helpers/mcp-discovery-isolation.ts b/tests/helpers/mcp/discovery-isolation.ts similarity index 100% rename from tests/helpers/mcp-discovery-isolation.ts rename to tests/helpers/mcp/discovery-isolation.ts diff --git a/tests/engine/mcp/helpers.ts b/tests/helpers/mcp/server.ts similarity index 100% rename from tests/engine/mcp/helpers.ts rename to tests/helpers/mcp/server.ts