fix(mcp): result-shaped output schemas for application CRUD tools (#1324)

[kylebernhardy]

Fixes #1324. Stacked on #1320 (base branch is fix/1317-mcp-application-500-and-conformance) — review/merge #1320 first.

Summary

The auto-generated MCP application-profile CRUD tools advertised the full-record outputSchema, but their handlers return a result envelope. The official @modelcontextprotocol/sdk client validates results against the advertised schema and rejected every call with -32600 ("has an output schema but did not return structured content"):

• create_* returned a bare primary key (scalar) → no structuredContent at all.
• update_*/patch_* returned {ok:true}, which the full-record schema (with required server-assigned fields like @updatedTime) doesn't satisfy.

So these tools were unusable from spec-conformant hosts (Claude Desktop, etc.). Harper's server doesn't validate outputSchema — only strict clients do — which is why #1320's round-trip used raw JSON-RPC.

Fix

Advertise what the handlers actually return, mirroring the existing delete(boolean)/search(omit) precedent:

Tool	Returns	outputSchema
create_*	{ id }	{ id: <pk type> }
update_*/patch_*	{ ok: true }	{ ok: boolean }
delete_*	{ deleted: <bool> }	{ deleted: boolean }

Handlers wrap their scalar return into the matching object so structuredContent is present. get_* keeps the full-record schema (it genuinely returns the record). The #1317 integration round-trip is flipped from raw JSON-RPC to the SDK client, which now validates the contract end-to-end.

Where to look

• components/mcp/tools/application.ts — isStructuredEnvelope() guard so a custom Resource that returns a structured object (typically with a static outputSchemas override) passes through unchanged rather than being re-wrapped. This guard is the main correctness surface; it's applied symmetrically to create/update/patch/delete.
• components/mcp/tools/schemas/derive.ts — deriveRecordSchema is now used by get_* only; create/update/patch/delete get result-shaped schemas. This also fixes the secondary bug where a freshly-created record could lack a required @updatedTime.

🤖 Generated by an LLM (Claude Opus 4.8).

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[claude]

Reviewed; no blockers found.

[kriszyp]

The integration test failures here are all artifact download errors across every shard — not test failures. The issue appears to be stale/poisoned artifact storage on this run. Could you push a new commit (or empty git commit --allow-empty) to trigger a fresh CI run with clean artifacts? The code looks fine otherwise.

[kylebernhardy]

@kriszyp thanks — tried that, but it didn't clear it. Pushed an empty commit to get a fresh run with clean artifacts; the build uploaded a fresh 161 MB artifact successfully (successfully finalized), yet every integration shard still failed downloading it — Unable to download artifact(s)… after 5 retries, dying ~27s into the Azure blob transfer. Repeated with full re-runs (regenerating the artifact each time) and by merging this branch's base in — same result every time.

So it isn't stale storage on the original run; it's the blob download failing. Tellingly, runs on main and on the base branch (#1320) during the same window downloaded their artifacts fine, so the artifact service is healthy in general — #1336's runs just keep landing on flaky blob hosts (productionresultssa2/13/14).

(The transaction.test.js #1114-dedup unit failure on the latest run is an unrelated pre-existing flake — v24 only; this PR only touches components/mcp/**.)

The code itself is validated — unit/build/lint green, and Codex + Gemini cross-model reviews both clean. The reliable unblock looks like landing #1320 (green, and your review items are addressed) and rebasing this onto main, which sidesteps the stuck stacked run entirely. Let me know if you'd prefer a different approach.

_{🤖 AI-generated (Claude), posted by Kyle.}

[kylebernhardy]

@kriszyp update — CI is fully green now. Here's how it resolved:

Artifact-download failures: these turned out to be intermittent Azure-blob degradation that this PR's stacked runs kept losing the coin flip on — builds uploaded a valid 161 MB artifact every time, but the download died ~27s in (after 5 retries), across fresh artifacts and multiple blob hosts, while main and #1320 runs in the same window downloaded fine. Once #1320 merged, I merged main into this branch so it runs on a clean (non-stacked) base — and the artifact step has passed cleanly since (30/30 integration shards green).

The more important find: with integration finally running, the #1324 SDK-client round-trip caught a real operational bug — update_/patch_ threw Cannot read properties of undefined (reading 'directURLMapping'). makeUpdateHandler was invoking the verb method detached from its class (const fn = ResourceClass.put; fn(...)), so the static Resource dispatcher ran with this === undefined. get_/create_/delete_ already called the method on the class; update was the lone exception, and the unit mocks never referenced this so they missed it. Fixed by calling ResourceClass.put!/patch! directly (this preserved) + a regression test with a this-dereferencing handler. The SDK round-trip now passes for all four verbs.

(The transient transaction.test.js #1114 and blob.test.js unit failures along the way were unrelated pre-existing flakes — different test/runtime each run — and cleared on re-run.)

Net: result-shaped output schemas (create→{id}, update/patch→{ok}, delete→{deleted}) + the this-binding fix; Codex + Gemini both clean. Ready for your review.

_{🤖 AI-generated (Claude), posted by Kyle.}

[kriszyp]

Output schema/handler contract correctly aligned for all CRUD tools; this-binding fix is a good catch.

Reviewed by claude-sonnet-4-6.