Add MCP write tools: create / connect / update-docs / move (#19)#55
Merged
Conversation
Four thin-adapter MCP tools over existing services, plus a new moveNode service for reparenting (cycle prevention + incident-edge orphan reject). Each tool is a single-op write wrapped in db.$transaction; service errors translate through a generic toMcpWriteError that carries ConflictError.details as data.archDetails so an agent can self-correct on a duplicate Connection or a blocked move (ADR-0010 named pattern). No destructive tool is exposed. The moveNode service writes only parentId — non-cascading by design (ADR-0024). Cycle-creating moves reject as ValidationError; moves that would orphan an incident Connection reject as ConflictError with details.conflictingEdgeIds. A cross-scope FlowRoute falsification check was considered and shown to be unreachable under current routeFlow/connectNodes constraints (analyzed in ADR-0024); the placement for the check is named explicitly so a future constraint loosening lands locally. The catalog is plain data (WRITE_TOOLS in src/server/mcp/tool-catalog.ts), so the SDK registration loop and /llms.txt render from one source — #40 / #42's Flow / FlowRoute tools and #20's apply_graph batch tool plug in additively. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a moveNode reparent service with cycle and orphaning guards, a typed moveNode input schema and tests, MCP write-tool catalog and registration (four tools), MCP write error translation, and an updated /llms.txt discovery document exposing write-tool metadata and structured conflict details. ChangesMCP write tools feature
Sequence Diagram(s)sequenceDiagram
participant Agent
participant MCP_Tool
participant Authorization
participant Database
participant Service
Agent->>MCP_Tool: invoke move_component {id, parentId}
MCP_Tool->>MCP_Tool: validate schema
MCP_Tool->>Authorization: load and authorize node
Authorization-->>MCP_Tool: actor can write
MCP_Tool->>Database: start transaction
MCP_Tool->>Service: moveNode(db, actor, args)
Service->>Database: load node and parent
Service->>Database: compute moving subtree
Service->>Database: check incident edges
alt cycle detected
Service-->>MCP_Tool: ValidationError
else orphaning detected
Service-->>MCP_Tool: ConflictError with conflictingEdgeIds
else valid
Service->>Database: UPDATE node.parentId
Service-->>MCP_Tool: updated node
end
alt error
MCP_Tool->>MCP_Tool: toMcpWriteError mapping
MCP_Tool-->>Agent: McpError with ErrorCode, archDetails if CONFLICT
else success
MCP_Tool-->>Agent: {content: [{type: text, text: confirmation message}]}
end
MCP_Tool->>Database: commit transaction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/server/mcp/tool-catalog.ts (1)
45-54: ⚡ Quick winTrim field comments that restate the type names.
The per-field doc comments here are largely “what” comments; keep only rationale-level notes (why constraints/shape exist) to match project comment policy.
♻️ Suggested change
export interface McpWriteToolDescriptor<Schema extends z.ZodType> { - /** MCP tool name; what the agent calls. snake_case to match MCP convention. */ name: string; - /** Human/agent-facing one-liner (shown in `tools/list` and `llms.txt`). */ title: string; - /** Multi-paragraph agent guidance; carries the prompt-injection note. */ description: string; - /** Zod schema validating the tool's input. */ inputSchema: Schema; - /** Service-layer call; the registry wraps it in `db.$transaction`. */ + /** Kept as a callback so registration/discovery stay data-driven and in sync. */ invoke: ( db: Db, actor: Actor, args: z.input<Schema>, ) => Promise<ToolInvocationResult>; }As per coding guidelines:
Only write comments to explain why code exists, not what it does; avoid comments that restate the code or serve as coverage metrics.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/mcp/tool-catalog.ts` around lines 45 - 54, Remove or shorten the per-field doc comments on the MCP tool interface that merely restate the type (fields: name, title, description, inputSchema, invoke); keep only rationale-level notes explaining why a field exists or any non-obvious constraints (for example note that name is snake_case to match MCP convention or that invoke is wrapped in db.$transaction). Update the comments for name, title, description, inputSchema, and invoke to be minimal and explanatory (drop “what it is” phrasing that duplicates the type) so the docs explain intent/constraints not types.src/server/mcp/__tests__/errors.test.ts (1)
12-12: ⚡ Quick winUse
~/*alias for src-local imports in tests too.Please replace
../errorswith the~/*alias form to keep import style consistent with repo rules.♻️ Suggested change
-import { toMcpWriteError } from "../errors"; +import { toMcpWriteError } from "~/server/mcp/errors";As per coding guidelines:
Use the path alias ~/* to reference src/* throughout the codebase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/mcp/__tests__/errors.test.ts` at line 12, Replace the relative import of toMcpWriteError in the test file with the repo path-alias form; locate the import statement that currently reads `import { toMcpWriteError } from "../errors";` and change it to use the `~/*` alias (e.g. `import { toMcpWriteError } from "~server/mcp/errors";`) so tests follow the src-local import convention.src/server/mcp/handler.ts (1)
4-6: ⚡ Quick winSwitch src-local imports to
~/*aliases.Please convert these relative imports to alias imports for consistency with repo TS import rules.
♻️ Suggested change
import { db } from "~/server/db"; -import { makeVerifyMcpToken } from "./auth"; -import { registerArchitectureResources } from "./resources"; -import { registerArchitectureTools } from "./tools"; +import { makeVerifyMcpToken } from "~/server/mcp/auth"; +import { registerArchitectureResources } from "~/server/mcp/resources"; +import { registerArchitectureTools } from "~/server/mcp/tools";As per coding guidelines:
Use the path alias ~/* to reference src/* throughout the codebase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/mcp/handler.ts` around lines 4 - 6, Replace the three relative imports in handler.ts with project path-alias imports: change import sources "./auth", "./resources", and "./tools" to the src alias paths (e.g. use ~/server/mcp/auth, ~/server/mcp/resources, and ~/server/mcp/tools) while keeping the imported symbols makeVerifyMcpToken, registerArchitectureResources, and registerArchitectureTools unchanged so the module resolution follows the repository's ~/* convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTEXT.md`:
- Around line 504-505: The paragraph referencing the MCP tool contains "`#40` /
`#42`" which Markdown can treat as headings; update the MCP tool paragraph (the
line containing "`#40` / `#42`" near the mention of `apply_graph` and `/llms.txt`)
to prevent heading parsing by escaping the hashes or rendering them as inline
code (e.g., replace "`#40`" and "`#42`" with escaped hashes or backticked text) so
they appear as plain text rather than Markdown headers.
In `@src/lib/schemas.ts`:
- Around line 211-213: Update the moveNodeInput docstring to remove the outdated
statement that moves are rejected for "falsify a refinement FlowRoute" and
instead document the actual enforced behavior per ADR-0024: state that
moveNodeInput rejects moves that would orphan an Incident or Connection with
ValidationError, and that ConflictError is used only for the currently enforced
conflict cases; remove or reword any clause claiming FlowRoute-falsification as
a reject path. Locate the wording in the moveNodeInput docstring and replace the
incorrect clause so the description matches the current service enforcement.
In `@src/server/architecture/node.service.ts`:
- Around line 866-867: The current read uses db.node.findUnique which can return
a soft-deleted ("tombstoned") row; change the lookup to enforce live-row
semantics by excluding soft-deleted records — replace db.node.findUnique(...)
with db.node.findFirst({ where: { id: node.id, deletedAt: null } }) or, if your
soft-delete flag is different, add the appropriate where clause (e.g., deleted:
false) so that the subsequent if (!current) truly represents not-found rather
than a tombstone.
In `@src/server/mcp/tools.ts`:
- Around line 3-7: The module currently accepts a typed PrismaClient parameter
and uses relative imports; change it to import and use the singleton DB instance
from ~/server/db instead of taking a PrismaClient arg, and convert all relative
imports (e.g., actorFromAuthInfo, toMcpWriteError, WRITE_TOOLS) to use the path
alias ~/server/...; update any function signatures that accept a PrismaClient to
remove that parameter and reference the singleton DB inside those functions, and
ensure error handling via toMcpWriteError still works with the singleton DB
usage.
---
Nitpick comments:
In `@src/server/mcp/__tests__/errors.test.ts`:
- Line 12: Replace the relative import of toMcpWriteError in the test file with
the repo path-alias form; locate the import statement that currently reads
`import { toMcpWriteError } from "../errors";` and change it to use the `~/*`
alias (e.g. `import { toMcpWriteError } from "~server/mcp/errors";`) so tests
follow the src-local import convention.
In `@src/server/mcp/handler.ts`:
- Around line 4-6: Replace the three relative imports in handler.ts with project
path-alias imports: change import sources "./auth", "./resources", and "./tools"
to the src alias paths (e.g. use ~/server/mcp/auth, ~/server/mcp/resources, and
~/server/mcp/tools) while keeping the imported symbols makeVerifyMcpToken,
registerArchitectureResources, and registerArchitectureTools unchanged so the
module resolution follows the repository's ~/* convention.
In `@src/server/mcp/tool-catalog.ts`:
- Around line 45-54: Remove or shorten the per-field doc comments on the MCP
tool interface that merely restate the type (fields: name, title, description,
inputSchema, invoke); keep only rationale-level notes explaining why a field
exists or any non-obvious constraints (for example note that name is snake_case
to match MCP convention or that invoke is wrapped in db.$transaction). Update
the comments for name, title, description, inputSchema, and invoke to be minimal
and explanatory (drop “what it is” phrasing that duplicates the type) so the
docs explain intent/constraints not types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d70eeac6-1916-42f0-8475-7b49d13ccdff
📒 Files selected for processing (11)
CONTEXT.mddocs/adr/0024-movenode-reparent-reject-orphaning.mdsrc/app/llms.txt/route.tssrc/lib/schemas.tssrc/server/architecture/__tests__/node.service.test.tssrc/server/architecture/node.service.tssrc/server/mcp/__tests__/errors.test.tssrc/server/mcp/errors.tssrc/server/mcp/handler.tssrc/server/mcp/tool-catalog.tssrc/server/mcp/tools.ts
Comment on lines
+3
to
+7
| import type { PrismaClient } from "../../../generated/prisma/client"; | ||
| import { actorFromAuthInfo } from "./auth"; | ||
| import { toMcpWriteError } from "./errors"; | ||
| import { WRITE_TOOLS } from "./tool-catalog"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Align DB access and imports with TypeScript repository rules.
This adapter should use the singleton DB and ~/* imports, rather than taking a typed client parameter and using relative src paths.
♻️ Suggested change
import { type McpServer } from "`@modelcontextprotocol/sdk/server/mcp.js`";
-import type { PrismaClient } from "../../../generated/prisma/client";
-import { actorFromAuthInfo } from "./auth";
-import { toMcpWriteError } from "./errors";
-import { WRITE_TOOLS } from "./tool-catalog";
+import { db } from "~/server/db";
+import { actorFromAuthInfo } from "~/server/mcp/auth";
+import { toMcpWriteError } from "~/server/mcp/errors";
+import { WRITE_TOOLS } from "~/server/mcp/tool-catalog";
@@
export function registerArchitectureTools(
- server: McpServer,
- db: PrismaClient,
+ server: McpServer,
): void {As per coding guidelines: Always access the database through the singleton at ~/server/db and Use the path alias ~/* to reference src/* throughout the codebase.
Also applies to: 30-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/mcp/tools.ts` around lines 3 - 7, The module currently accepts a
typed PrismaClient parameter and uses relative imports; change it to import and
use the singleton DB instance from ~/server/db instead of taking a PrismaClient
arg, and convert all relative imports (e.g., actorFromAuthInfo, toMcpWriteError,
WRITE_TOOLS) to use the path alias ~/server/...; update any function signatures
that accept a PrismaClient to remove that parameter and reference the singleton
DB inside those functions, and ensure error handling via toMcpWriteError still
works with the singleton DB usage.
Owner
Author
Fixes Applied SuccessfullyFixed 3 file(s) based on 3 of 4 CodeRabbit feedback item(s). Files modified:
Declined:
Commit: The latest autofix changes are on the |
This was referenced Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #19.
Summary
Four single-op MCP write tools —
create_component,connect_components,update_component_docs,move_component— over the existing service layer, plus a newmoveNodeservice for reparenting. Each tool is a thin adapter, wrapped indb.$transaction; the agent receives a short text confirmation with the affected row id (so it can chain calls without an intermediate read).moveNode(ADR-0024) writes onlyparentId— non-cascading by design. Cycle-creating moves reject asValidationError; moves that would orphan an active incident Connection reject asConflictErrorwithdetails.conflictingEdgeIds. The cross-scope refinement falsification case was considered and shown to be unreachable under currentrouteFlow/connectNodesconstraints — the analysis lives in ADR-0024, and the placement for a future reject is named.A new generic
toMcpWriteErrorkeeps NOT_FOUND/FORBIDDEN indistinguishable (read posture, ADR-0022) while preserving CONFLICT/BAD_REQUEST messages and carryingConflictError.detailsthrough asdata.archDetails— the AI-readable self-correction channel future Flow tools (#40 / #42) reuse unchanged.The tool catalog is plain data (
WRITE_TOOLSinsrc/server/mcp/tool-catalog.ts), so SDK registration,tools/list, and/llms.txtall render from one source.Scope boundary (held)
ConflictErrorDetailsso they plug in untouched).apply_graphbatch tool (MCP apply_graph batch tool #20).moveNodeis exercised by vitest and the MCP tool only.Verification
pnpm check— eslint + tsc clean.pnpm test— 256 / 256 (12 test files). 11 newmoveNodecases (happy paths, idempotent no-op, cycle, missing/soft-deleted/foreign parent, non-owner, incident-edge reject with structured details, soft-deleted edges ignored, descendants travel intact) + 8toMcpWriteErrorcases (non-disclosure parity, message preservation, details envelope, Zod, opaque internal, McpError passthrough).Docs (travel-with-code)
docs/adr/0024-movenode-reparent-reject-orphaning.md.CONTEXT.md— new MCP tool glossary entry (canonical write-surface term);Nodeentry reflects realized reparenting; status notes inAgentandMCP path.Test plan
/connectand confirmtools/listover the MCP Inspector enumerates exactly the four tools — no delete tool listed.create_componentround-trips through a read of thearchitecture://project/{projectId}resource (the acceptance-criterion round-trip).connect_componentsbetween two created Components; duplicate call returns aConflictErrorcarryingarchDetails.conflictingEdgeIds.update_component_docspersists;/p/{slug}shows the new docs.move_componentto a new parent succeeds; moving onto self or a descendant returns BAD_REQUEST; moving a Component with active Connections returns CONFLICT carryingarchDetails.conflictingEdgeIds./api/mcpreturns 401./llms.txtlists all four tools by name.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests