fix: handleMessage avoids [object Object] errors and enforces valid JSON-RPC error codes for thrown plain objects

[Copilot]

submit_pull_request_review was failing with -32603 [object Object] because the handleMessage catch block in mcp_server_core.cjs used String(e) for non-Error thrown values. safe_outputs_handlers.cjs throws plain objects — not Error instances — for validation errors, so String({code: -32602, message: "..."}) → [object Object].

The HTTP path (handleRequest) already handled this correctly; the stdio path did not.

Changes

• mcp_server_core.cjs — align handleMessage catch with handleRequest, and tighten JSON-RPC code handling:
  • propagate e.code only when it is a negative integer (and from an object)
  • use e.message when present
  • fall back to "Internal error" instead of serializing the thrown value

// beforeserver.replyError(id,-32603,einstanceofError ? e.message : String(e));// afterconstcode=e&&typeofe==="object"&&Number.isInteger(e.code)&&e.code<0 ? e.code : -32603;server.replyError(id,code,e&&e.message ? String(e.message) : "Internal error");

• mcp_server_core.test.cjs — expanded regression coverage for thrown plain-object errors:
  • valid negative integer code propagates code and message
  • missing code falls back to -32603
  • positive code is suppressed and falls back to -32603
  • non-integer negative code is suppressed and falls back to -32603
  • missing message falls back to "Internal error"

[github-actions]

Hey @Copilot 👋 — thanks for picking up the submit_pull_request_review failure from the Matt Pocock Skills Reviewer workflow! This is exactly the right process — the PR is aligned and the failure root cause (MCP internal error -32603 when submitting reviews) is a real infrastructure problem worth fixing.

A couple of things to address before this is ready for review:

• Add a description — once you've formed a plan, update the PR body with a summary of what's changing and why (e.g. which tool call is being fixed, what fallback or workaround is being introduced). The current body is a placeholder.
• Add tests — the fix will presumably touch workflow logic or safe-output handling; covering the happy path (review submitted successfully) and the fallback path (graceful degradation when submit_pull_request_review fails) in the relevant test file would make this much easier to merge confidently.

If you'd like a hand getting started:

Debug and fix the `submit_pull_request_review` failure in the mattpocock-skills-reviewer workflow.
The root cause: all four attempted approaches to submit a pull request review via MCP failed with Error [-32603]: [object Object].
Steps:
1. Read debug.md for the standard debugging approach.
2. Inspect the workflow definition at .github/workflows/mattpocock-skills-reviewer.md.
3. Identify the `submit_pull_request_review` call site and determine why it fails with -32603.
4. Implement a fix — either correct the call arguments or add a reliable fallback (e.g. post as a comment when the review submission fails).
5. Add or update tests in the relevant test file to cover both the success path and the fallback path.
6. Update the PR description with a clear summary of the change.

Generated by ✅ Contribution Check · 93.6 AIC · ⌖ 16.5 AIC · ⊞ 5.9K · ◷

[Copilot]

Pull request overview

This PR fixes stdio-transport MCP error responses so that when a tool handler throws a plain object (e.g., validation errors), the JSON-RPC response includes the intended error.code and error.message instead of -32603 [object Object].

Changes:

• Updated handleMessage (stdio path) to propagate a negative JSON-RPC error code from thrown objects and prefer e.message when present.
• Added tests covering thrown plain-object errors with and without an explicit error code.
• Introduced whitespace-only changes in an auto-generated maintenance workflow file (should be reverted).

Show a summary per file

File	Description
actions/setup/js/mcp_server_core.cjs	Adjusts stdio error handling to extract JSON-RPC error code/message from non-Error thrown values.
actions/setup/js/mcp_server_core.test.cjs	Adds regression tests ensuring thrown plain-object errors return the correct code/message and avoid [object Object].
.github/workflows/agentics-maintenance.yml	Contains whitespace-only edits (trailing-space # lines) in an auto-generated workflow header section.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/3 changed files
• Comments generated: 4

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #40715 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold). 3 files changed.

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

[github-actions]

test body

🧪 Test quality analysis by Test Quality Sentinel · 52.4 AIC · ⌖ 15.9 AIC · ⊞ 8.3K · ◷

[github-actions]

✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). Both new tests verify observable JSON-RPC error responses when a handler throws a plain object, directly covering the behavioral contract fixed in this PR. Note: a test inflation flag (49 test lines / 5 prod lines = 9.8×) deducted 10 pts but is non-blocking — the verbosity is justified by full MCP request/response cycle scaffolding.

[github-actions]

Skills-Based Review 🧠

Applied /diagnose and /tdd — approving with minor suggestions.

📋 Key Themes & Highlights

Key Themes

• Root cause correctly addressed: The String(e) coercion in the old handleMessage catch block was the real bug; the fix extracts e.code and e.message directly, mirroring the already-correct handleRequest path.
• Positive-code guard is important but untested: The code comment explicitly warns that subprocess exit codes (positive integers) must not be forwarded as JSON-RPC error codes. A third test case would lock in this behaviour.
• Subtle transport asymmetry: handleRequest falls back to "Internal error" for messageless throws; handleMessage now falls back to String(e). Both are defensible, but the divergence should be intentional.

Positive Highlights

• ✅ Fix is a surgical one-liner that correctly aligns handleMessage with handleRequest
• ✅ Two targeted regression tests reproduce both failure modes before asserting the fix
• ✅ Null guard (e && ...) on the code extraction is a nice defensive addition
• ✅ The code comment clearly explains why positive codes are rejected — good documentation

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 78.8 AIC · ⌖ 10.4 AIC · ⊞ 6.5K

[github-actions]

[/diagnose] The message fallback here (String(e)) diverges from how handleRequest handles the same situation (err.message || "Internal error"). For primitives like throw 42 the stdio path returns "42" while the HTTP path returns "Internal error" — a subtle per-transport inconsistency that can complicate debugging.

💡 Suggested alignment

handleRequest (line 840):

message: err.message||"Internal error",

handleMessage (this line):

e&&e.message ? String(e.message) : String(e)

Consider aligning both paths to the same fallback message, or add a comment explaining why the two transports intentionally differ. Either choice is fine, but the asymmetry should be deliberate.

[github-actions]

Non-blocking but fix is incomplete and the key guard has no test

The core bug ([object Object] in error responses for plain-object throws) is correctly fixed for the primary case (safe_outputs_handlers.cjs throwing { code, message }). Two issues prevent a clean approve:

⚠️ Issues found

1. Message fallback still produces [object Object] (line 991) — String(e) fires any time e.message is falsy, including absent or empty-string. The reference implementation (handleRequest) avoids this entirely with err.message || "Internal error". The stdio path should match.

2. Positive-code suppression untested (test file, line 521) — The comment on lines 987–989 explicitly calls out blocking positive subprocess exit codes as the motivation for e.code < 0. No test exercises throw { code: 1, message: "..." } → code should be -32603. If the guard is ever inverted or removed, nothing catches it.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• patchdiff.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
allowed:
- defaults
- "patchdiff.githubusercontent.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer · 109.1 AIC · ⌖ 7.39 AIC · ⊞ 5.1K

[github-actions]

Message fallback String(e) still produces [object Object] for objects without a message, diverging from handleRequest.

e && e.message ? is a truthiness check — it drops any thrown object whose message property is an empty string "" or absent, falling through to String(e) which produces [object Object] for any plain object. This is the exact symptom the PR claims to fix.

The reference implementation (handleRequest, line 840) correctly uses err.message || "Internal error", avoiding the object serialization entirely.

💡 Suggested fix

// Current (can still produce "[object Object]" when message is absent or "")server.replyError(id,code,e&&e.message ? String(e.message) : String(e));// Aligned with handleRequest:server.replyError(id,code,(e&&e.message) ? String(e.message) : "Internal error");

This matches handleRequest's err.message || "Internal error" exactly and ensures the stdio path never serializes a plain object into the message field.

[pelikhan]

@copilot Rub pr-finisher skill

[Copilot]

@copilot Rub pr-finisher skill

Ran pr-finisher flow and pushed follow-up fixes in 2bd26fc and 60cc5c5 (plus prior 2c921d5). Local validation is green (make fmt, make lint, make test-unit, make test, make recompile, and make agent-report-progress).