Skip to content

After #20: adopt the SDK 1.x API so advertised inputSchema contracts are actually enforced #30

Description

@minerva-sky

Summary

#20 bumps @modelcontextprotocol/sdk from 0.6.1 to 1.30.0. I've verified that bump is safe to merge as-is (evidence in the PR thread: full stdio round-trip, identical results to the 0.6.1 baseline). This issue is about the thing the bump doesn't do.

The server survives a nine-major-version jump because it uses the low-level Server + setRequestHandler API, which 1.x left alone. What 1.x added is the higher-level McpServer / registerTool surface — and with it, declarative schema enforcement. Staying on the low-level API means the server keeps advertising input contracts it doesn't enforce.

The concrete gap

mcp/index.js advertises required on every tool. All six schemas are well-formed — type: "object", and every required key has a matching entry in properties:

setup_architecture:          required=[projectPath]
create_adr:                  required=[title,context,decision,consequences,projectPath]
list_architecture_members:   required=[projectPath]
get_architecture_status:     required=[projectPath]
configure_pragmatic_mode:    required=[projectPath]
get_implementation_guidance: required=[projectPath]

Nothing checks them. The low-level Server API validates the JSON-RPC envelope, not inputSchema. Calling a tool without its required argument, over a real stdio client:

tools/call get_architecture_status {}
  -> isError=true
     Error: The "path" argument must be of type string. Received undefined

That's path.join(undefined, ...) surfacing through the catch-all in _setupToolHandlers. It reproduces identically on 0.6.1, so it is not a regression and not a reason to hold #20 — it's just been invisible.

Two smaller things in the same handler, worth folding into one pass rather than three:

  1. Unknown tool: <name> is returned as a successful result with isError: true. An unrecognised tool name is a protocol-level error, not a tool-execution failure. isError is for "the tool ran and failed"; a client can't distinguish "you asked for something that doesn't exist" from "the operation didn't work."
  2. The catch-all stringifies error.message verbatim. Fine for the framework's own messages; less fine when the message is a raw Node ERR_INVALID_ARG_TYPE, which tells the caller nothing actionable.

Why it's worth doing

A well-behaved MCP client reads inputSchema and fills arguments correctly, which is why this hasn't bitten anyone. The failure mode is the sloppier caller: a hand-written client, a smaller model, a user driving the tools directly. They get a Node internals error instead of "projectPath is required," and there's nothing in the response to tell them which field they missed.

Proposal

Migrate the six tool registrations from setRequestHandler(CallToolRequestSchema, …) + switch to McpServer.registerTool(name, {inputSchema}, handler), with schemas expressed in zod. That gets validation, a generated inputSchema that cannot drift from the handler, and typed arguments — deleting the hand-written schema literals rather than adding a validation layer alongside them.

Cost: contained. Six registrations, one switch removed, the tool method bodies unchanged. mcp/index.js is 1265 lines; this touches roughly the 150 lines between _initServer and the end of _setupToolHandlers.

Risk: it changes wire behavior for malformed calls — a schema violation becomes a protocol error instead of isError: true text. That's the correct behavior and what a conforming client expects, but it is a behavior change, so it wants a version bump and your call on timing.

Sequencing: merge #20 first. This migration is only available once 1.x is the pinned version, and doing it as a separate change keeps the "did the bump break anything" question separate from "did the rewrite break anything."

Testable: #TBD-PR adds a protocol smoke test that spawns the server over stdio and asserts the advertised schemas are well-formed. Extending it to assert enforcement — that get_architecture_status {} is rejected with a schema error naming projectPath — is a few lines, and gives this migration a red test to turn green.

Fences

Within WORLD.md: the MCP server is a documented install path, and this is protocol correctness on the primary distribution channel, not new surface area. It adds no tools and no config format. It does change observable behavior for malformed calls, which makes it public-api-change — L0 — so it stays an issue rather than a PR. Say the word and I'll take the implementation.

Labels intended: loop:deps, status:analyzed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    loop:depsDependency / language upgrade loopstatus:analyzedAnalyzed, awaiting decision

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions