Skip to content

fix(mcp): declare returned fields in tool output schemas; add security policy and code of conduct - #575

Merged
Aymericr merged 3 commits into
mainfrom
chore/maintainer-housekeeping
Aug 4, 2026
Merged

fix(mcp): declare returned fields in tool output schemas; add security policy and code of conduct#575
Aymericr merged 3 commits into
mainfrom
chore/maintainer-housekeeping

Conversation

@Aymericr

@Aymericr Aymericr commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Two independent commits, both maintainer-side housekeeping.

1. MCP tools returned fields their own schemas did not declare

Fixes #566.

get_level_summary returns floorIndex and create_from_template returns saveSkipped, and neither was declared in the tool's outputSchema. The MCP SDK generates the JSON Schema with additionalProperties: false, so once a client has cached the schema from tools/list it rejects the response:

MCP error -32602: Structured content does not match the tool's output schema: data must NOT have additional properties

Every real host — Claude Desktop, Claude Code, Codex, Cursor — calls tools/list before tools/call, so both tools failed in production on an empty default scene while the whole suite stayed green.

Why the suite missed it. The SDK client only validates structuredContent against a tool's output schema once it has cached that schema from tools/list. No test called listTools() first, so validation never ran. @Till-JS diagnosed this precisely in #566 — thank you.

packages/mcp/src/tools/output-schema-contract.test.ts connects a client that lists before calling and exercises the read-only tools, so a payload field added without a matching schema entry fails CI from now on. Reverting either schema change makes it fail with the exact -32602 above.

#566 reported get_level_summary as the only offender out of 37 tools. That was right for the tools its sweep reached; create_from_template only surfaces on the save: true + no-store path, which needs a store-less bridge to hit.

2. Community health files

  • The README Discord badge pointed at an expired invite (Discord invite link is ded #552). Now uses the same non-expiring invite the hosted app already advertises.
  • No SECURITY.md existed, so there was no stated private channel for a vulnerability report — the only visible options were a public issue or discussion. Adds GitHub private vulnerability reporting plus security@pascal.app, with scope.
  • Adds Contributor Covenant 2.1, and a Contributing section in the README linking both files and the plugin guidance CONTRIBUTING.md already gives.

Verification

  • bun test packages/mcp/src — 299 pass, 0 fail (was 297 pass)
  • bun run check — 1582 files, clean
  • bun run check-types — 9/9 tasks pass

🤖 Generated with Claude Code


Note

Low Risk
Changes are schema declarations, tests, and documentation; MCP behavior is unchanged aside from fixing client-side schema validation failures.

Overview
MCP tool output schemas now declare fields that handlers already return, fixing -32602 validation failures in hosts that call tools/list before tools/call. get_level_summary adds floorIndex; create_from_template adds optional saveSkipped.

A new output-schema-contract.test.ts runs an MCP client that lists tools first and exercises read-only tools (plus the no-store save: true path for templates), so undeclared structuredContent fields fail CI.

Repo hygiene: next-env.d.ts is gitignored and the tracked copy under apps/ifc-converter is removed to stop Next dev/typegen churn. README updates the Discord invite and adds a Contributing section. New SECURITY.md (private reporting, scope) and CODE_OF_CONDUCT.md (Contributor Covenant 2.1).

Reviewed by Cursor Bugbot for commit f724952. Bugbot is set up for automated code reviews on this repo. Configure here.

Aymericr and others added 2 commits August 4, 2026 12:32
`get_level_summary` returned `floorIndex` and `create_from_template`
returned `saveSkipped`, neither of which was declared in the tool's
`outputSchema`. The MCP SDK generates the JSON Schema with
`additionalProperties: false`, so once a client has cached the schema from
`tools/list` it rejects the response:

    MCP error -32602: Structured content does not match the tool's output
    schema: data must NOT have additional properties

Every real host — Claude Desktop, Claude Code, Codex, Cursor — calls
`tools/list` before `tools/call`, so both tools failed in production on an
empty default scene while passing the whole test suite.

The suite missed it because the SDK client only validates
`structuredContent` once it has cached the schema, and no test called
`listTools()` first. `output-schema-contract.test.ts` connects a client
that lists before calling and exercises the read-only tools, so a payload
field added without a matching schema entry now fails CI.

Reported in #566, which found `get_level_summary`; `create_from_template`
turned up in the same sweep once the harness could see it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README's Discord badge pointed at an expired invite (#552); this uses
the same non-expiring invite the hosted app already advertises.

The repo had no SECURITY.md, so there was no stated way to report a
vulnerability privately — the only visible channels were public issues and
discussions. It now points at GitHub private reporting and
security@pascal.app, and says what is in scope.

Adds Contributor Covenant 2.1 as the code of conduct, and a Contributing
section in the README linking both files plus the plugin guidance that
CONTRIBUTING.md already gives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b3852a1. Configure here.

Comment thread apps/ifc-converter/next-env.d.ts Outdated
Next rewrites the route-types import in `next-env.d.ts` depending on whether
it was last touched by `next dev` (`.next/dev/types`) or `next typegen` /
`next build` (`.next/types`), so a tracked copy shows up as a spurious diff
in unrelated PRs. `apps/editor` already ignores it; move the rule to the root
ignore so it covers every Next app instead of being repeated per app.
@Aymericr

Aymericr commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Good catch from Bugbot on the apps/ifc-converter/next-env.d.ts churn — that was a next typegen side effect from running check-types locally.

Rather than just reverting the line, I fixed the cause: the file was tracked in apps/ifc-converter but ignored in apps/editor, which is why it churns for anyone who runs the typecheck. Next rewrites that import path depending on whether next dev or next typegen/next build touched it last. f724952 untracks it and moves the ignore rule to the root .gitignore so it covers every Next app instead of being duplicated per app. Verified Next regenerates it from scratch and check-types still passes 9/9.

@Aymericr
Aymericr merged commit b4abdd8 into main Aug 4, 2026
3 checks passed
@Aymericr
Aymericr deleted the chore/maintainer-housekeeping branch August 4, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_level_summary violates its own outputSchema (floorIndex) — fails on every MCP host that calls tools/list

1 participant