refactor(mcp): align transport domain structure
- Group server, stdio, and transport implementation by ownership - Mirror those domains in engine tests and centralize shared MCP fixtures
This commit is contained in:
parent
a46ade4361
commit
9c9e4fae08
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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"]
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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<void>((resolve) => {
|
||||
setTimeout(resolve, 200)
|
||||
|
|
|
|||
|
|
@ -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}`)
|
||||
|
|
@ -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}`)
|
||||
|
|
@ -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}`)
|
||||
|
|
@ -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}`)
|
||||
|
|
@ -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<ReturnType<typeof createStdioRpcBridge>> = []
|
||||
const origSocketEnv = process.env.OPENPENCIL_MCP_SOCKET
|
||||
|
|
@ -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'
|
||||
|
|
@ -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'
|
||||
|
|
@ -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)
|
||||
|
|
@ -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.
|
||||
Loading…
Reference in a new issue