Repository navigation
feat: support structured tool outputs and cancellation - #6
jo-adithya wants to merge 2 commits into
Conversation
alexng353
left a comment
There was a problem hiding this comment.
[agent] Review at a207f33 (stacked on #5). Same eight-read pass. bun test and type-check pass. Integration was not run.
Must-fix: one tool's output schema can hide every tool in the plugin. The SDK advertises the Zod-derived JSON Schema as outputSchema (app.ts:158). If that schema doesn't compile in the client's validator (for example a regex containing \-, which is valid in JS but not under unicode-mode AJV), listTools() throws, and Futurity drops the whole server. Both Claude reads reproduced this with a real MCP client. Fix: in declaresOutput (app.ts:80), compile the schema with the MCP SDK's own validator, and fall back to text-only output if it fails.
Related: some outputs pass Zod on the server but fail the client's schema check (regex flags are dropped, local datetimes). The same compile-and-validate step catches these.
Should-fix
- Aborts on close need SDK >= 1.26. "Aborts on close" isn't true below that version, but the peer range is
^1.24.3. Raise the floor or document the limit. Three reads confirmed this. .overwrite()schemas run twice. A Zod schema using.overwrite()is applied twice per call, and the second pass then fails validation (app.ts:89). Reproduced.
Nit: tools without an input schema receive the SDK's entire request context as input. That predates this PR, but this PR rewrote the line and documented the quirk instead of fixing it. Pass {} instead.
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
a207f33 to
20a0a2a
Compare
20a0a2a to
07eac1f
Compare
Summary
Stack
Part 2 of 2; depends on #5. This PR targets feat/manifest-v3 so its diff contains only the tool-output changes. Merge #5 first, then retarget this PR to main if necessary.
Verification