feat(codeflow-mcp): extract MCP package with test_tool and JSON-RPC server - #27
Conversation
…erver - Move `src/lib/blueprint/mcp.ts` to `packages/codeflow-mcp/src/index.ts` (listMcpTools, invokeMcpTool, extractTextFromMcpResult) - Add MCP server (`src/invoke/index.ts`) with working `test_tool` that prints paw and CF in ASCII art — proves isolation works - Add CLI (`src/bin/cli.ts`) with server start, tool list, tool invoke - Wire `src/app/api/mcp/invoke/route.ts` and `src/app/api/mcp/tools/route.ts` to import from `@abhinav2203/codeflow-mcp` - All 11 tests passing, type-check clean, build clean Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…erver - Move `src/lib/blueprint/mcp.ts` to `packages/codeflow-mcp/src/index.ts` (listMcpTools, invokeMcpTool, extractTextFromMcpResult) - Add MCP server (`src/invoke/index.ts`) with working `test_tool` that prints paw and CF in ASCII art — proves isolation works - Add CLI (`src/bin/cli.ts`) with server start, tool list, tool invoke - Wire `src/app/api/mcp/invoke/route.ts` and `src/app/api/mcp/tools/route.ts` to import from `@abhinav2203/codeflow-mcp` - All 11 tests passing, type-check clean, build clean Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (86)
📒 Files selected for processing (47)
📝 WalkthroughWalkthroughThis PR introduces two new npm packages extracted from the main codebase: Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request initiates the decomposition of the CodeFlow monorepo into modular packages, specifically implementing the @abhinav2203/codeflow-mcp and @abhinav2203/codeflow-store packages. The changes include core logic, CLI tools, and updated API routes to support this new architecture. Feedback identifies significant code duplication within the MCP package and the use of non-standard console.json methods in the implementation plans. Additionally, the reviewer recommends replacing unsafe type assertions with Zod schema validation across network and file system operations to ensure better type safety.
| interface JsonRpcRequest { | ||
| jsonrpc: "2.0"; | ||
| id: number | string | null; | ||
| method: string; | ||
| params?: Record<string, unknown>; | ||
| } | ||
|
|
||
| interface JsonRpcResponse { | ||
| jsonrpc: "2.0"; | ||
| id: number | string | null; | ||
| result?: unknown; | ||
| error?: { code: number; message: string }; | ||
| } | ||
|
|
||
| const TOOLS = [ | ||
| { | ||
| name: "test_tool", | ||
| description: "Prints a paw and 'CF' in ASCII art. Use to verify the MCP server is working.", | ||
| inputSchema: { type: "object", properties: {} }, | ||
| }, | ||
| ] as const; | ||
|
|
||
| function jsonRpcError(id: unknown, code: number, message: string): JsonRpcResponse { | ||
| return { jsonrpc: "2.0", id: id as string | number | null, error: { code, message } }; | ||
| } | ||
|
|
||
| function jsonRpcResult(id: unknown, result: unknown): JsonRpcResponse { | ||
| return { jsonrpc: "2.0", id: id as string | number | null, result }; | ||
| } | ||
|
|
||
| async function handleRequest(req: JsonRpcRequest): Promise<JsonRpcResponse> { | ||
| const { method, params, id } = req; | ||
|
|
||
| if (method === "tools/list") { | ||
| return jsonRpcResult(id, { tools: TOOLS }); | ||
| } | ||
|
|
||
| if (method === "tools/call") { | ||
| const name = (params as Record<string, unknown>)?.["name"] as string | undefined; | ||
| const args = ((params as Record<string, unknown>)?.["arguments"] as Record<string, unknown>) ?? {}; | ||
| if (!name) { | ||
| return jsonRpcError(id, -32602, "Missing tool name"); | ||
| } | ||
| if (name === "test_tool") { | ||
| return jsonRpcResult(id, { | ||
| content: [ | ||
| { | ||
| type: "text" as const, | ||
| text: [ | ||
| " ∧＿∧", | ||
| " (｡･ω･｡)", | ||
| " ／> <＼", | ||
| " ／< > ＼", | ||
| " | ∨ | |", | ||
| "", | ||
| " ┌──┐", | ||
| " │CF│", | ||
| " └──┘", | ||
| "", | ||
| "🐾 CodeFlow MCP server is alive!", | ||
| ].join("\n"), | ||
| }, | ||
| ], | ||
| }); | ||
| } | ||
| throw new Error(`Unknown tool: ${name}`); | ||
| } | ||
|
|
||
| return jsonRpcError(id, -32601, `Method not found: ${method}`); | ||
| } | ||
|
|
||
| function startHttpServer(port: number, host: string) { | ||
| const server = createServer(async (req, res) => { | ||
| if (req.method === "OPTIONS") { | ||
| res.writeHead(204, { | ||
| "Access-Control-Allow-Origin": "*", | ||
| "Access-Control-Allow-Methods": "GET, POST, OPTIONS", | ||
| "Access-Control-Allow-Headers": "Content-Type, authorization, x-api-key", | ||
| }); | ||
| res.end(); | ||
| return; | ||
| } | ||
| if (req.method !== "POST") { | ||
| res.writeHead(405); | ||
| res.end(); | ||
| return; | ||
| } | ||
|
|
||
| let body = ""; | ||
| for await (const chunk of req) { | ||
| body += chunk; | ||
| } | ||
|
|
||
| let request: JsonRpcRequest; | ||
| try { | ||
| request = JSON.parse(body); | ||
| } catch { | ||
| const err: JsonRpcResponse = { | ||
| jsonrpc: "2.0", id: null, error: { code: -32700, message: "Parse error" }, | ||
| }; | ||
| res.writeHead(200, { "Content-Type": "application/json", "Access-Control-Allow-Origin": "*" }); | ||
| res.end(JSON.stringify(err)); | ||
| return; | ||
| } | ||
|
|
||
| const response = await handleRequest(request); | ||
| res.writeHead(200, { "Content-Type": "application/json", "Access-Control-Allow-Origin": "*" }); | ||
| res.end(JSON.stringify(response)); | ||
| }); | ||
|
|
||
| server.listen(port, host, () => { | ||
| console.log(`[codeflow-mcp] MCP server running at http://${host}:${port}`); | ||
| console.log(`[codeflow-mcp] SSE endpoint: POST /`); | ||
| console.log(`[codeflow-mcp] Tools: test_tool`); | ||
| }); | ||
|
|
||
| return server; | ||
| } |
There was a problem hiding this comment.
There is significant code duplication between this file and src/invoke/index.ts. The entire HTTP server implementation, including handleRequest, jsonRpcError, jsonRpcResult, and the TOOLS definition, is replicated here. This makes the code harder to maintain, as any changes would need to be applied in two places.
To resolve this:
- Make
src/tools/index.tsthe single source of truth for theTOOLSconstant. - Update
src/invoke/index.tsto importTOOLSfromsrc/tools/index.ts. - Refactor the server logic in
src/invoke/index.tsinto an exportable function (e.g.,startHttpServer). - Update this CLI file to import and call
startHttpServerfor theserver startcommand, removing the duplicated server code.
| const TOOLS = [ | ||
| { | ||
| name: "test_tool", | ||
| description: "Prints a paw and 'CF' in ASCII art. Use to verify the MCP server is working.", | ||
| inputSchema: { | ||
| type: "object", | ||
| properties: {}, | ||
| }, | ||
| }, | ||
| ] as const; |
There was a problem hiding this comment.
The TOOLS constant is also defined in src/bin/cli.ts and src/tools/index.ts. To avoid duplication and ensure a single source of truth, this constant should be defined only in src/tools/index.ts and imported here.
| if (cmd === "tool" && args[0] === "list") { | ||
| const serverUrl = args[1] ?? "http://localhost:3001/mcp"; | ||
| const tools = await listMcpTools(serverUrl); | ||
| console.json({ tools }); |
There was a problem hiding this comment.
The console.json method is not a standard part of the Node.js console API and will cause a runtime error. To print a JSON object to the console, you should use console.log(JSON.stringify(yourObject, null, 2));.
| console.json({ tools }); | |
| console.log(JSON.stringify({ tools }, null, 2)); |
| const serverUrl = args[2] ?? "http://localhost:3001/mcp"; | ||
| const rawArgs = args[3] ?? "{}"; | ||
| const result = await invokeMcpTool(serverUrl, toolName, JSON.parse(rawArgs)); | ||
| console.json({ result }); |
There was a problem hiding this comment.
The console.json method is not a standard part of the Node.js console API and will cause a runtime error. To print a JSON object to the console, you should use console.log(JSON.stringify(yourObject, null, 2));.
| console.json({ result }); | |
| console.log(JSON.stringify({ result }, null, 2)); |
| headers: { "content-type": "application/json" }, | ||
| body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list", params: {} }), | ||
| }); | ||
| const data = await res.json() as JsonRpcResponse; |
There was a problem hiding this comment.
Using as for type assertion from an external data source like a network response is unsafe. If the response shape doesn't match JsonRpcResponse, it can lead to runtime errors. Since zod is a dependency in this package, it would be more robust to define a Zod schema for JsonRpcResponse and use it to parse and validate the response data. This ensures type safety.
| export const getApprovalRecord = async (approvalId: string): Promise<ApprovalRecord | null> => { | ||
| try { | ||
| const content = await fs.readFile(approvalPath(approvalId), "utf8"); | ||
| return JSON.parse(content) as ApprovalRecord; |
There was a problem hiding this comment.
Using as for type assertion when parsing JSON from a file is unsafe. If the file content doesn't match the ApprovalRecord type, it can lead to runtime errors. It's better to use a schema validation library like Zod to parse and validate the data, ensuring type safety. The persistedSessionSchema from @abhinav2203/codeflow-core is a good example of this pattern.
|
|
||
| const parseJsonFile = async <T>(filePath: string): Promise<T> => { | ||
| const { readFileSync } = await import("node:fs"); | ||
| return JSON.parse(readFileSync(filePath, "utf8")) as T; |
There was a problem hiding this comment.
This function uses an unsafe type assertion as T. When reading and parsing a JSON file, the content should be validated against a schema to ensure it conforms to the expected type T. Using a library like Zod for parsing would provide this type safety and prevent potential runtime errors from malformed JSON.
| ): Promise<GraphBranch | null> => { | ||
| try { | ||
| const content = await fs.readFile(branchPath(projectName, branchId), "utf8"); | ||
| return JSON.parse(content) as GraphBranch; |
There was a problem hiding this comment.
Unsafe type assertion as GraphBranch. Please use a Zod schema (like graphBranchSchema if it exists in @abhinav2203/codeflow-core/schema) to parse and validate the file content. This ensures type safety and prevents runtime errors if the file content is malformed.
| .map(async (entry) => { | ||
| try { | ||
| const content = await fs.readFile(path.join(dir, entry), "utf8"); | ||
| return JSON.parse(content) as GraphBranch; |
There was a problem hiding this comment.
Unsafe type assertion as GraphBranch. Please use a Zod schema to parse and validate the file content. This will prevent runtime errors from malformed JSON.
| ): Promise<ObservabilitySnapshot | null> => { | ||
| try { | ||
| const content = await fs.readFile(observabilityPath(projectName), "utf8"); | ||
| return JSON.parse(content) as ObservabilitySnapshot; |
There was a problem hiding this comment.
Unsafe type assertion as ObservabilitySnapshot. Please use a Zod schema to parse and validate the file content to ensure type safety and prevent runtime errors if the file content is malformed.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8978511c41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { exportBlueprintRequestSchema } from "@/lib/blueprint/schema"; | ||
| import { createRunPlan } from "@/lib/blueprint/plan"; | ||
| import { assessExportRisk } from "@/lib/blueprint/risk"; | ||
| import { assessExportRisk } from "@abhinav2203/codeflow-store/risk"; |
There was a problem hiding this comment.
This route now imports @abhinav2203/codeflow-store/*, but the root install metadata in this commit does not add @abhinav2203/codeflow-store or @abhinav2203/codeflow-mcp to package.json/package-lock.json and there is no workspace linking configured, so a clean install cannot resolve these modules and the Next.js build will fail with module-resolution errors when compiling these API routes.
Useful? React with 👍 / 👎.
| const nextSession = persistedSessionSchema.parse({ | ||
| sessionId: sessionId ?? existing?.sessionId ?? createSessionId(), | ||
| projectName: normalizedGraph.projectName, | ||
| updatedAt: new Date().toISOString(), | ||
| graph: normalizedGraph, |
There was a problem hiding this comment.
The new upsertSession implementation constructs nextSession without repoPath, which drops any previously stored repo path on every write; this regresses the export flow because src/app/api/export/route.ts reads loadLatestSession(...).repoPath to decide whether CodeRAG re-indexing can run, so after one session update future exports will silently skip re-indexing for that project.
Useful? React with 👍 / 👎.
| ], | ||
| }); | ||
| } | ||
| throw new Error(`Unknown tool: ${name}`); |
There was a problem hiding this comment.
For tools/call, unknown tool names currently throw, and the HTTP server path awaiting handleRequest does not catch that exception, so a single request for an unsupported tool terminates the CLI server process instead of returning a JSON-RPC error response; this makes the server brittle and causes client-side transport failures.
Useful? React with 👍 / 👎.
…atibility Implements three MCP transports per the MCP spec: - **stdio** (`codeflow-mcp stdio`): Claude Code CLI, Cursor, any stdio MCP client Reads/writes newline-delimited JSON-RPC. Handles initialize, tools/list, tools/call, notifications/initialized, and terminate protocol messages. - **HTTP POST** (`codeflow-mcp server start --port 3100`): Standard JSON-RPC request/response. Compatible with all HTTP MCP clients. GET / returns server info + supported transports. - **SSE** (`http://host:port/sse`): Server-Sent Events for streaming responses. Claude Desktop and Cursor use this for long-running tool results. Includes keep-alive pings every 30s. Updated CLI to expose all three transports with clear usage examples. All 11 tests passing, type-check clean, build clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Extracts CodeFlow’s MCP client/server logic into a standalone @abhinav2203/codeflow-mcp package and updates the Next.js API routes to consume the new package exports. This PR also introduces a new @abhinav2203/codeflow-store package and rewires export/approval routes to use it.
Changes:
- Added
packages/codeflow-mcpwith MCP client utilities, a JSON-RPC server implementation, CLI, and tests. - Added
packages/codeflow-storewith local filesystem-backed persistence (sessions/approvals/runs/checkpoints/etc.) plus CLI/tests, and updated app API routes to import from it. - Updated root
package.json(including moving/bumpingnext).
Reviewed changes
Copilot reviewed 33 out of 109 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/api/mcp/tools/route.ts | Switches tool listing route to import listMcpTools from @abhinav2203/codeflow-mcp. |
| src/app/api/mcp/invoke/route.ts | Switches tool invocation route to import invokeMcpTool from @abhinav2203/codeflow-mcp. |
| src/app/api/export/route.ts | Migrates export flow to use @abhinav2203/codeflow-store (risk/approval/checkpoint/run/session). |
| src/app/api/approvals/approve/route.ts | Migrates approval action to @abhinav2203/codeflow-store/approval. |
| packages/codeflow-store/vitest.config.ts | Adds Vitest config for the new codeflow-store package. |
| packages/codeflow-store/tsconfig.json | Adds TS config for building codeflow-store. |
| packages/codeflow-store/test-fixtures/sample-blueprint.json | Adds fixture JSON for store-related tests. |
| packages/codeflow-store/test-fixtures/minimal-blueprint.json | Adds minimal fixture JSON for store-related tests. |
| packages/codeflow-store/src/store/index.ts | Adds Zustand-based blueprint/workbench UI store. |
| packages/codeflow-store/src/shared/utils.ts | Adds filesystem path helpers for store roots and persisted objects. |
| packages/codeflow-store/src/shared/terminal-sessions.ts | Adds in-memory terminal session manager that spawns shells and captures output. |
| packages/codeflow-store/src/shared/run-command.ts | Adds a capped/timeout command runner utility. |
| packages/codeflow-store/src/shared/file-tree.ts | Adds recursive repo file scanner utility. |
| packages/codeflow-store/src/session/index.ts | Adds session persistence (latest + history) using PersistedSession schema validation. |
| packages/codeflow-store/src/session.test.ts | Adds tests for session ID generation and risk assessment behavior. |
| packages/codeflow-store/src/run/index.ts | Adds run record persistence. |
| packages/codeflow-store/src/risk/index.ts | Adds export risk assessment (fingerprint + risk factors). |
| packages/codeflow-store/src/observability/index.ts | Adds observability snapshot persistence + merge helper. |
| packages/codeflow-store/src/checkpoint/index.ts | Adds checkpoint creation by copying existing output dir. |
| packages/codeflow-store/src/branch/index.ts | Adds branch persistence APIs. |
| packages/codeflow-store/src/bin/cli.ts | Adds codeflow-store CLI for sessions/approvals/risk/branches/runs/observability. |
| packages/codeflow-store/src/approval/index.ts | Adds approval record creation/lookup/approve persistence. |
| packages/codeflow-store/package.json | Defines package exports/bin/scripts/deps for @abhinav2203/codeflow-store. |
| packages/codeflow-store/dist/store/index.js.map | Built artifact for store module. |
| packages/codeflow-store/dist/store/index.js | Built artifact for store module. |
| packages/codeflow-store/dist/store/index.d.ts.map | Built artifact typings map for store module. |
| packages/codeflow-store/dist/store/index.d.ts | Built artifact typings for store module. |
| packages/codeflow-store/dist/shared/utils.js.map | Built artifact for shared utils. |
| packages/codeflow-store/dist/shared/utils.js | Built artifact for shared utils. |
| packages/codeflow-store/dist/shared/utils.d.ts.map | Built artifact typings map for shared utils. |
| packages/codeflow-store/dist/shared/utils.d.ts | Built artifact typings for shared utils. |
| packages/codeflow-store/dist/shared/terminal-sessions.js.map | Built artifact for terminal sessions. |
| packages/codeflow-store/dist/shared/terminal-sessions.js | Built artifact for terminal sessions. |
| packages/codeflow-store/dist/shared/terminal-sessions.d.ts.map | Built artifact typings map for terminal sessions. |
| packages/codeflow-store/dist/shared/terminal-sessions.d.ts | Built artifact typings for terminal sessions. |
| packages/codeflow-store/dist/shared/run-command.js.map | Built artifact for run-command. |
| packages/codeflow-store/dist/shared/run-command.js | Built artifact for run-command. |
| packages/codeflow-store/dist/shared/run-command.d.ts.map | Built artifact typings map for run-command. |
| packages/codeflow-store/dist/shared/run-command.d.ts | Built artifact typings for run-command. |
| packages/codeflow-store/dist/shared/file-tree.js.map | Built artifact for file-tree. |
| packages/codeflow-store/dist/shared/file-tree.js | Built artifact for file-tree. |
| packages/codeflow-store/dist/shared/file-tree.d.ts.map | Built artifact typings map for file-tree. |
| packages/codeflow-store/dist/shared/file-tree.d.ts | Built artifact typings for file-tree. |
| packages/codeflow-store/dist/session/index.js.map | Built artifact for session module. |
| packages/codeflow-store/dist/session/index.js | Built artifact for session module. |
| packages/codeflow-store/dist/session/index.d.ts.map | Built artifact typings map for session module. |
| packages/codeflow-store/dist/session/index.d.ts | Built artifact typings for session module. |
| packages/codeflow-store/dist/run/index.js.map | Built artifact for run module. |
| packages/codeflow-store/dist/run/index.js | Built artifact for run module. |
| packages/codeflow-store/dist/run/index.d.ts.map | Built artifact typings map for run module. |
| packages/codeflow-store/dist/run/index.d.ts | Built artifact typings for run module. |
| packages/codeflow-store/dist/risk/index.js.map | Built artifact for risk module. |
| packages/codeflow-store/dist/risk/index.js | Built artifact for risk module. |
| packages/codeflow-store/dist/risk/index.d.ts.map | Built artifact typings map for risk module. |
| packages/codeflow-store/dist/risk/index.d.ts | Built artifact typings for risk module. |
| packages/codeflow-store/dist/observability/index.js.map | Built artifact for observability module. |
| packages/codeflow-store/dist/observability/index.js | Built artifact for observability module. |
| packages/codeflow-store/dist/observability/index.d.ts.map | Built artifact typings map for observability module. |
| packages/codeflow-store/dist/observability/index.d.ts | Built artifact typings for observability module. |
| packages/codeflow-store/dist/checkpoint/index.js.map | Built artifact for checkpoint module. |
| packages/codeflow-store/dist/checkpoint/index.js | Built artifact for checkpoint module. |
| packages/codeflow-store/dist/checkpoint/index.d.ts.map | Built artifact typings map for checkpoint module. |
| packages/codeflow-store/dist/checkpoint/index.d.ts | Built artifact typings for checkpoint module. |
| packages/codeflow-store/dist/branch/index.js.map | Built artifact for branch module. |
| packages/codeflow-store/dist/branch/index.js | Built artifact for branch module. |
| packages/codeflow-store/dist/branch/index.d.ts.map | Built artifact typings map for branch module. |
| packages/codeflow-store/dist/branch/index.d.ts | Built artifact typings for branch module. |
| packages/codeflow-store/dist/bin/cli.js.map | Built artifact for store CLI. |
| packages/codeflow-store/dist/bin/cli.js | Built artifact for store CLI. |
| packages/codeflow-store/dist/bin/cli.d.ts.map | Built artifact typings map for store CLI. |
| packages/codeflow-store/dist/bin/cli.d.ts | Built artifact typings for store CLI. |
| packages/codeflow-store/dist/approval/index.js.map | Built artifact for approval module. |
| packages/codeflow-store/dist/approval/index.js | Built artifact for approval module. |
| packages/codeflow-store/dist/approval/index.d.ts.map | Built artifact typings map for approval module. |
| packages/codeflow-store/dist/approval/index.d.ts | Built artifact typings for approval module. |
| packages/codeflow-mcp/vitest.config.ts | Adds Vitest config for the new codeflow-mcp package. |
| packages/codeflow-mcp/tsconfig.json | Adds TS config for building codeflow-mcp. |
| packages/codeflow-mcp/src/tools/index.ts | Adds MCP tool registry export. |
| packages/codeflow-mcp/src/invoke/index.ts | Adds MCP JSON-RPC HTTP server implementation + start/stop helpers. |
| packages/codeflow-mcp/src/index.ts | Adds MCP client functions (listMcpTools, invokeMcpTool, extractTextFromMcpResult). |
| packages/codeflow-mcp/src/index.test.ts | Adds unit tests for MCP client behavior. |
| packages/codeflow-mcp/src/bin/cli.ts | Adds codeflow-mcp CLI for server start/tool list/tool invoke. |
| packages/codeflow-mcp/scripts/wrap-cli.mjs | Adds build helper to ensure dist/bin exists for the CLI. |
| packages/codeflow-mcp/package.json | Defines package exports/bin/scripts/deps for @abhinav2203/codeflow-mcp. |
| packages/codeflow-mcp/dist/tools/index.js.map | Built artifact for tools registry. |
| packages/codeflow-mcp/dist/tools/index.js | Built artifact for tools registry. |
| packages/codeflow-mcp/dist/tools/index.d.ts.map | Built artifact typings map for tools registry. |
| packages/codeflow-mcp/dist/tools/index.d.ts | Built artifact typings for tools registry. |
| packages/codeflow-mcp/dist/invoke/index.js.map | Built artifact for invoke server. |
| packages/codeflow-mcp/dist/invoke/index.js | Built artifact for invoke server. |
| packages/codeflow-mcp/dist/invoke/index.d.ts.map | Built artifact typings map for invoke server. |
| packages/codeflow-mcp/dist/invoke/index.d.ts | Built artifact typings for invoke server. |
| packages/codeflow-mcp/dist/index.test.js.map | Built artifact for MCP client tests. |
| packages/codeflow-mcp/dist/index.test.js | Built artifact for MCP client tests. |
| packages/codeflow-mcp/dist/index.test.d.ts.map | Built artifact typings map for MCP client tests. |
| packages/codeflow-mcp/dist/index.test.d.ts | Built artifact typings for MCP client tests. |
| packages/codeflow-mcp/dist/index.js.map | Built artifact for MCP client. |
| packages/codeflow-mcp/dist/index.js | Built artifact for MCP client. |
| packages/codeflow-mcp/dist/index.d.ts.map | Built artifact typings map for MCP client. |
| packages/codeflow-mcp/dist/index.d.ts | Built artifact typings for MCP client. |
| packages/codeflow-mcp/dist/bin/cli.js.map | Built artifact for MCP CLI. |
| packages/codeflow-mcp/dist/bin/cli.js | Built artifact for MCP CLI. |
| packages/codeflow-mcp/dist/bin/cli.d.ts.map | Built artifact typings map for MCP CLI. |
| packages/codeflow-mcp/dist/bin/cli.d.ts | Built artifact typings for MCP CLI. |
| package.json | Updates dependency classification/version for Next.js. |
| docs/superpowers/plans/2026-04-21-codeflow-mcp-decomposition.md | Adds implementation plan for the codeflow-mcp decomposition. |
Files not reviewed (1)
- packages/codeflow-store/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const approvalPath = (approvalId: string): string => | ||
| path.join(getStoreRoot(), "approvals", `${approvalId}.json`); | ||
|
|
||
| export const runPath = (runId: string): string => | ||
| path.join(getStoreRoot(), "runs", `${runId}.json`); | ||
|
|
||
| export const checkpointPath = (checkpointId: string): string => | ||
| path.join(getStoreRoot(), "checkpoints", checkpointId); |
There was a problem hiding this comment.
approvalPath, runPath, and checkpointPath interpolate caller-provided IDs directly into a filesystem path. Since API routes accept approvalId as an arbitrary string, this allows path traversal via values like "../...". Sanitize these IDs (e.g., path.basename + equality check, or a strict UUID regex) before joining them into paths, similar to how branchPath validates branchId.
| server.listen(PORT, HOST, () => { | ||
| console.log(`[codeflow-mcp] MCP server running at http://${HOST}:${PORT}`); | ||
| console.log(`[codeflow-mcp] SSE endpoint: POST /`); | ||
| console.log(`[codeflow-mcp] Tools: test_tool`); | ||
| }); | ||
|
|
||
| export async function startServer(port = PORT, host = HOST) { | ||
| return new Promise<void>((resolve) => { | ||
| server.listen(port, host, resolve); | ||
| }); | ||
| } |
There was a problem hiding this comment.
This module starts listening (server.listen(...)) immediately at import time, but also exports startServer() which calls server.listen() again. Importing @abhinav2203/codeflow-mcp/invoke in any program will unexpectedly start a server and startServer() will throw if the server is already listening. Move the initial server.listen behind an explicit entrypoint check (e.g., only when executed as a script) and have startServer() be the single place that binds the server.
| import { assessExportRisk } from "@abhinav2203/codeflow-store/risk"; | ||
| import { createSandboxDir, syncSandboxToTarget, writeDiffManifest } from "@/lib/blueprint/sandbox"; | ||
| import { | ||
| createApprovalRecord, | ||
| getApprovalRecord | ||
| } from "@/lib/blueprint/approval-store"; | ||
| import { createCheckpointIfNeeded } from "@/lib/blueprint/checkpoint-store"; | ||
| import { createRunId, saveRunRecord } from "@/lib/blueprint/run-store"; | ||
| import { loadLatestSession, upsertSession } from "@/lib/blueprint/session-store"; | ||
| } from "@abhinav2203/codeflow-store/approval"; | ||
| import { createCheckpointIfNeeded } from "@abhinav2203/codeflow-store/checkpoint"; | ||
| import { createRunId, saveRunRecord } from "@abhinav2203/codeflow-store/run"; | ||
| import { loadLatestSession, upsertSession } from "@abhinav2203/codeflow-store/session"; |
There was a problem hiding this comment.
The PR description/title focuses on extracting @abhinav2203/codeflow-mcp, but this change also introduces and wires up a new @abhinav2203/codeflow-store package (risk/approval/session/run/checkpoint) and updates production API routes to depend on it. Please update the PR description (or split into separate PRs) so reviewers understand the additional scope and can assess the migration impact appropriately.
| child.on("close", (code) => { | ||
| session.status = session.status === "error" ? "error" : "exited"; | ||
| session.exitCode = code; | ||
| appendOutput(session, `\n[CodeFlow] Terminal exited with code ${code ?? "unknown"}.\\n`); |
There was a problem hiding this comment.
The close handler appends a string ending with \\n, which will render as a literal backslash+n in the stored output instead of a newline. Use a single \n so the terminal output formatting is consistent with the other messages in this module.
| "dependencies": { | ||
| "@abhinav2203/coderag": "^0.2.1", | ||
| "@monaco-editor/react": "^4.7.0", | ||
| "@react-three/drei": "^10.7.7", | ||
| "@react-three/fiber": "^9.5.0", | ||
| "@xyflow/react": "^12.10.1", | ||
| "cross-spawn": "^7.0.6", | ||
| "framer-motion": "^12.38.0", | ||
| "monaco-editor": "^0.55.1", | ||
| "next": "^16.1.6", | ||
| "opencode-ai": "^1.3.13", | ||
| "react": "^19.2.4", | ||
| "react-dom": "^19.2.4", | ||
| "react-rnd": "^10.4.13", | ||
| "three": "^0.183.2", | ||
| "ts-morph": "^27.0.2", | ||
| "typescript": "^5.9.3", | ||
| "zod": "^4.3.6" | ||
| }, | ||
| "devDependencies": { | ||
| "@playwright/test": "^1.59.1", | ||
| "@testing-library/jest-dom": "^6.9.1", | ||
| "@testing-library/react": "^16.3.2", | ||
| "@testing-library/user-event": "^14.6.1", | ||
| "@types/cross-spawn": "^6.0.6", | ||
| "@types/node": "^25.5.0", | ||
| "@types/react": "^19.2.14", | ||
| "@types/react-dom": "^19.2.3", | ||
| "eslint": "^9.39.4", | ||
| "eslint-config-next": "^16.2.1", | ||
| "jsdom": "^28.1.0", | ||
| "next": "^16.2.4", | ||
| "tinyexec": "^1.0.2", | ||
| "tsx": "^4.21.0", |
There was a problem hiding this comment.
next was moved from dependencies to devDependencies, but this repo has next dev/build/start scripts and likely needs Next.js in production installs. If the deployment uses npm ci --omit=dev (or equivalent), the app will fail to build/start due to missing next. Keep next in dependencies for an application package unless you can guarantee devDependencies are always installed in production.
| if (!resolvedOutputDir.startsWith(workspaceRoot)) { | ||
| factors.push({ | ||
| code: "outside-workspace", | ||
| message: `Output directory is outside the workspace root: ${resolvedOutputDir}.`, | ||
| score: 2 |
There was a problem hiding this comment.
The workspace containment check uses resolvedOutputDir.startsWith(workspaceRoot), which can produce false positives (e.g. /tmp/workspace-root2 starts with /tmp/workspace-root). Use a path-aware check (like path.relative(workspaceRoot, resolvedOutputDir) and ensure it doesn't start with .. or include an absolute path) to avoid misclassifying output directories in the risk report.
| // Re-export the tool list from the package index for use by API routes and CLI | ||
| export const TOOLS: McpTool[] = [ | ||
| { | ||
| name: "test_tool", | ||
| description: "Prints a paw and 'CF' in ASCII art. Use to verify the MCP server is working.", | ||
| inputSchema: { type: "object", properties: {} }, | ||
| }, |
There was a problem hiding this comment.
The comment says this re-exports the tool list from the package index, but it actually defines its own TOOLS array (and src/index.ts doesn’t export a tools list). Either update the comment or centralize the tool registry so src/tools/index.ts, src/invoke/index.ts, and the CLI all reference the same source of truth.
- Test handleJsonRpc for tools/list, initialize, tools/call, method-not-found - Test jsonRpcError and jsonRpcResult helper functions - Test TOOLS registry structure - Export handleJsonRpc, jsonRpcError, jsonRpcResult from invoke/index.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- src/tools/index.test.ts: 8 tests for TOOLS registry shape and McpTool validation - src/bin/cli.test.ts: 3 integration tests for CLI binary (usage, stdio round-trip, unknown cmd) - Fix TypeScript errors in cli.test.ts (vitest callback signature) - All 40 tests passing across 4 test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add vitest tests for approval, branch, checkpoint, observability, risk, run, and session modules with proper test isolation via synchronous cleanStore using fsSync.rmSync. Configure vitest to run in single-thread mode to avoid ENOTEMPTY errors from concurrent file operations on macOS. Add TESTING.md with blueprint generation docs and test patterns. Add .test-store to .gitignore. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@copilot apply changes based on the comments in this thread high There is significant code duplication between this file and src/invoke/index.ts. The entire HTTP server implementation, including handleRequest, jsonRpcError, jsonRpcResult, and the TOOLS definition, is replicated here. This makes the code harder to maintain, as any changes would need to be applied in two places. To resolve this: Make src/tools/index.ts the single source of truth for the TOOLS constant. Update src/invoke/index.ts to import TOOLS from src/tools/index.ts. Refactor the server logic in src/invoke/index.ts into an exportable function (e.g., startHttpServer). Update this CLI file to import and call startHttpServer for the server start command, removing the duplicated server code. @gemini-code-assist gemini-code-assist Bot 10 hours ago high The TOOLS constant is also defined in src/bin/cli.ts and src/tools/index.ts. To avoid duplication and ensure a single source of truth, this constant should be defined only in src/tools/index.ts and imported here.The console.json method is not a standard part of the Node.js console API and will cause a runtime error. To print a JSON object to the console, you should use console.log(JSON.stringify(yourObject, null, 2));. Suggested change console.json({ tools }); console.log(JSON.stringify({ tools }, null, 2));The console.json method is not a standard part of the Node.js console API and will cause a runtime error. To print a JSON object to the console, you should use console.log(JSON.stringify(yourObject, null, 2));. Suggested change console.json({ result }); console.log(JSON.stringify({ result }, null, 2)); @gemini-code-assist gemini-code-assist Bot 11 hours ago medium Using as for type assertion from an external data source like a network response is unsafe. If the response shape doesn't match JsonRpcResponse, it can lead to runtime errors. Since zod is a dependency in this package, it would be more robust to define a Zod schema for JsonRpcResponse and use it to parse and validate the response data. This ensures type safety. gemini-code-assist Bot 11 hours ago medium Similar to the other network call, using as for type assertion here is unsafe. Please use a Zod schema to parse and validate the JSON response to ensure type safety. medium Using as for type assertion when parsing JSON from a file is unsafe. If the file content doesn't match the ApprovalRecord type, it can lead to runtime errors. It's better to use a schema validation library like Zod to parse and validate the data, ensuring type safety. The persistedSessionSchema from @abhinav2203/codeflow-core is a good example of this pattern.medium This function uses an unsafe type assertion as T. When reading and parsing a JSON file, the content should be validated against a schema to ensure it conforms to the expected type T. Using a library like Zod for parsing would provide this type safety and prevent potential runtime errors from malformed JSON. gemini-code-assist Bot 11 hours ago medium Unsafe type assertion as GraphBranch. Please use a Zod schema (like graphBranchSchema if it exists in @abhinav2203/codeflow-core/schema) to parse and validate the file content. This ensures type safety and prevents runtime errors if the file content is malformed.medium Unsafe type assertion as GraphBranch. Please use a Zod schema to parse and validate the file content. This will prevent runtime errors from malformed JSON.Unsafe type assertion as ObservabilitySnapshot. Please use a Zod schema to parse and validate the file content to ensure type safety and prevent runtime errors if the file content is malformed. Declare extracted store/mcp packages as app dependencies This route now imports @abhinav2203/codeflow-store/*, but the root install metadata in this commit does not add @abhinav2203/codeflow-store or @abhinav2203/codeflow-mcp to package.json/package-lock.json and there is no workspace linking configured, so a clean install cannot resolve these modules and the Next.js build will fail with module-resolution errors when compiling these API routes. Useful? React with 👍 / 👎.Preserve repoPath when upserting persisted sessions The new upsertSession implementation constructs nextSession without repoPath, which drops any previously stored repo path on every write; this regresses the export flow because src/app/api/export/route.ts reads loadLatestSession(...).repoPath to decide whether CodeRAG re-indexing can run, so after one session update future exports will silently skip re-indexing for that project. Useful? React with 👍 / 👎.Return JSON-RPC error instead of crashing on unknown tool For tools/call, unknown tool names currently throw, and the HTTP server path awaiting handleRequest does not catch that exception, so a single request for an unsupported tool terminates the CLI server process instead of returning a JSON-RPC error response; this makes the server brittle and causes client-side transport failures. Useful? React with 👍 / 👎. |
- Import TOOLS in invoke/index.ts from tools/index.ts (single source of truth) - Add repoPath param to upsertSession and preserve it from existing session - Sanitize approvalId/runId/checkpointId in utils.ts to prevent path traversal - Fix \\n literal in terminal-sessions.ts:184 - Fix workspace containment check in risk/index.ts (use path.relative) - Move next from devDependencies to dependencies in root package.json Agent-Logs-Url: https://github.com/nehraa/CodeFlow/sessions/b3a099f7-fed2-485b-a959-fc125924cf11 Co-authored-by: nehraa <187715469+nehraa@users.noreply.github.com>
Addressed all the review feedback in commit
|
|
@copilot resolve the merge conflicts in this pull request |
feat(codeflow-mcp): extract MCP package with test_tool and JSON-RPC server
Summary
Extracts
@abhinav2203/codeflow-mcpas a standalone npm package perdocs/PACKAGE_DECOMPOSITION.md. Phase 1 (Foundation — no deps) in the package build order.Package name:
@abhinav2203/codeflow-mcpVersion:
0.1.0What changed
packages/codeflow-mcp/src/app/api/mcp/invoke/route.ts@abhinav2203/codeflow-mcpsrc/app/api/mcp/tools/route.ts@abhinav2203/codeflow-mcpPackage structure
MCP transports
codeflow-mcp stdiocodeflow-mcp server start --port 3100http://host:port/sseCurrent tool:
test_toolRight now the server only has
test_tool— it returns a paw print + "CF" ASCII art. Useful for verifying the transport works, nothing more. The real tools (blueprint ops, risk analysis, approvals) come in later phases once the broader CodeFlow system is built.Test coverage: 40 tests across 4 files
src/index.test.tssrc/invoke/index.test.tssrc/tools/index.test.tssrc/bin/cli.test.tsVerification
Test plan
npm run check— tsc --noEmit passesnpm run test— vitest run passes (40 tests, 4 files)npm run build— TypeScript compiles, dist/ producedinitialize+tools/call— working🤖 Generated with Claude Code