fix(mcp): mark declared tool failures as errors

[maria-rcks]

adds integration coverage for mcp failure flags and output schemas using the official sdk validator. main already contains the production fix; this diff retains the regression coverage and validator dependency.

verified the actual codex provider/client path: t3_thread_read for a missing thread returned isError: true and the encoded OrchestratorMcpFailure with thread_not_found; the next call for an existing thread completed with the structured thread result. both persisted tool rows and the provider's report appeared in the actual client. production mcp, contracts and effect-patch source on running 321f9456 is identical to reviewed 50001d9; differences in those scoped paths are two existing test files only. other providers and failure kinds were not exercised live.

existing production-registration tests use the official mcp sdk validator for success, declared failures and parameter failures, and cover malformed output rejection and private defect redaction. the current integration suite, server typecheck and scoped lint passed. github status is checked separately.

closes #15566

implementation: codex; original model unrecorded. evidence: gpt-6.1-sol, codex harness.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3d7ec669-0d36-4f42-87ce-3b0dc4aeb4d1

📥 Commits

Reviewing files that changed from the base of the PR and between 4ee6bfd and 2255dcb.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (4)

• apps/server/package.json
• apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts
• apps/server/src/mcp/toolkits/core.test.ts
• patches/effect@4.0.0-rc.115.patch

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

---

📝 Walkthrough

Walkthrough

The patch changes MCP tool error flags and output schemas for returned failures. Server tests check failure flags and schema validation for tool results.

Changes

MCP tool failures

Layer / File(s)	Summary
Failure flags and output schemas patches/effect@4.0.0-rc.115.patch	Tool results now set isError from the failure status. When failureMode is "return" and the success schema is an object, the output schema includes the failure schema.
Failure behavior tests apps/server/package.json, apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts, apps/server/src/mcp/toolkits/core.test.ts	The server adds the MCP SDK dependency. Tests check failure flags, serialized failure content, and output-schema validity for returned failures and queue-list responses.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 2255d

Declared MCP tool failures are now flagged as errors and validate against the advertised output schema. No merge-blocking risk was identified in the supplied changes.

Architecture Summary

Architecture risk: 🔵 Low · up to 2255d

The change affects 2 systems.

Changed systems: apps/server, patches

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — apps/server (service) was modified; 3 changed files map to changed impact.
• observed — patches (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in apps/server/package.json: Added @modelcontextprotocol/sdk at version 1.29.0 to devDependencies.
• observed — Modified behavior in apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts: Added imports for AJV-based JSON Schema validation and the MCP JsonSchemaType type.
• observed — Modified behavior in apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts: Added an assertion that the thread pin operation does not return an error.
• observed — Modified behavior in apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts: Added checks that a normal queue-list response and a missing-thread error conform to the tool’s output schema, while malformed queue data does not. Also asserts that queue listing and thread reading for a missing thread return MCP errors.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The patches/effect@4.0.0-rc.115.patch changes also add HTTP session DELETE handling, cookie parsing fallback behavior, RPC request lifecycle hooks and chunk counts, and socket ping/pong hooks. The…	Remove the unrelated HTTP session, cookie, RPC lifecycle, and socket ping/pong changes from the patch. Keep changes that implement MCP failure flags and success/failure output schemas.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	For #15566, the MCP result patch maps declared failures to isError: true and retains isError: false for success. It advertises the union of success and failure schemas for returned failures. Tests…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Title check	✅ Passed	The title clearly states the primary change: marking declared MCP tool failures as errors.
Description check	✅ Passed	The description explains the failure-reporting problem, the regression coverage and validator dependency, links issue #15566, and gives detailed verification results, limits, and evidence. It does not…

Full details: Out of Scope Changes check

Explanation

The patches/effect@4.0.0-rc.115.patch changes also add HTTP session DELETE handling, cookie parsing fallback behavior, RPC request lifecycle hooks and chunk counts, and socket ping/pong hooks. These changes do not implement #15566’s MCP tool failure representation requirements.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[macroscopeapp]

Approvability

Verdict: Approved at 50001d9

Macroscope's review found this PR approvable — The changes are confined to ignored test and dependency-management paths, with no production implementation, product-default, deployment, security, billing, or authentication impact. Their runtime effect is limited to verification and development tooling.

Notes:

• No code objects were reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

Dismissing prior approval to re-evaluate 50001d9