Add controlled MCP action error envelope (roadmap #9) - #125
Conversation
Hardens every Pandora MCP tool response so an action never stream-breaks. - New pandora-mcp-action-envelope.runMcpAction wraps each tool call: - request_id on every response (success and failure); - error_code + safe message for controlled failures (no stack/HTML/raw); - 10s timeout guard -> controlled action_timeout JSON; - fallback_used flag; context reads fall back to latest context pack; - secret redaction on error messages (reuses memory-redaction-service); - deterministic payload cap at the action boundary. - Wire pandora-mcp-server to route all 13 tools through runMcpAction; get_memory_context / get_adaptive_context fall back to get_latest_context_pack. - Tests: thrown error -> controlled JSON; timeout -> controlled JSON; fallback_used=true; request_id on success+failure; payload cap enforced; no stack/secret leakage; happy-path fields preserved. No gated features touched (no embeddings/semantic/model/pruning). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03208de08a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const timeoutMs = opts.timeoutMs ?? MCP_ACTION_TIMEOUT_MS; | ||
| const maxPayloadChars = opts.maxPayloadChars ?? MCP_ACTION_MAX_PAYLOAD_CHARS; | ||
| try { | ||
| const data = await withActionTimeout(Promise.resolve().then(fn), timeoutMs); |
There was a problem hiding this comment.
Avoid timing out mutating tools without cancellation
When this wrapper is used for mutating tools such as capture_memory_event, create_session_digest, and candidate capture registered in lib/services/pandora-mcp-server.ts, a DB call that exceeds 10s is only raced: the original fn keeps running after this line returns an action_timeout. Since those paths insert/update rows without an AbortSignal or request idempotency guarantee, the caller can receive ok:false and retry while the first write later commits, producing unreported or duplicate memory mutations. Please either avoid this timeout for write tools or wire cancellation/idempotency before returning failure.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| function toContent(data: unknown): McpActionContent { | ||
| return { content: [{ type: "text", text: JSON.stringify(data, null, 2) }] }; |
There was a problem hiding this comment.
Cap the serialized content, not the compact JSON
The payload cap is measured with compact JSON.stringify, but this line returns pretty-printed JSON with extra whitespace. For nested or array-heavy context results near MCP_ACTION_MAX_PAYLOAD_CHARS, the value can pass capActionPayload and then grow past the action boundary when serialized here, so the MCP response can still be oversized. Serialize compactly or enforce the cap against the final text string.
Useful? React with 👍 / 👎.
What
Roadmap #9 — action error hardening. Every Pandora MCP tool response now goes through a controlled envelope so an action never stream-breaks or leaks.
Change
lib/services/pandora-mcp-action-envelope.ts—runMcpAction(fn, opts):request_idon every response (success and failure);{ ok:false, error_code, message, request_id }on failure — never HTML, stack traces, raw error objects, empty/malformed JSON;action_timeoutJSON;fallback_usedboolean; optional fallback path;memory-redaction-service);lib/services/pandora-mcp-server.ts— route all 13 tools throughrunMcpAction;get_memory_context/get_adaptive_contextfall back toget_latest_context_packif the primary path fails.Behavior preserved
The envelope spreads the tool's own result, so happy-path fields (and any existing controlled
ok:falsegate likemcp_capture_disabled) are unchanged;warningsare preserved and merged.Tests —
tests/unit/mcp-action-envelope.test.ts(8/8)thrown error → controlled JSON · timeout → controlled JSON ·
fallback_used=truewhen fallback succeeds · both-fail → controlled error ·request_idon success+failure · payload cap enforced · no stack/secret leakage (JWT redacted) · happy-path fields preserved · always valid JSON.Verification
first-reviewed-memory-fixture→spawnSync npm ENOENT) is the pre-existing sandbox-only flake, unrelated.Scope / safety
🤖 Generated with Claude Code