fix: --output json emits valid JSON instead of util.inspect#89
fix: --output json emits valid JSON instead of util.inspect#89shawnduggan wants to merge 2 commits intosteipete:mainfrom
Conversation
When `wrapped.json()` returns null (e.g. when structuredContent has fields other than .json/.data like qmd's .results), the json output mode fell through to printRaw() which uses util.inspect() — producing output with unquoted keys, single quotes, and [Object] placeholders that is not valid JSON. Two fixes: 1. tryParseJson now returns structured objects directly when they don't have .json/.data wrappers (they are already parsed data) 2. printCallOutput json case falls back to JSON.stringify(raw) before printRaw, ensuring valid JSON output Fixes steipete#80
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e499d05ae1
ℹ️ 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".
src/result-utils.ts
Outdated
| // Return the object itself — it is already a parsed structure (e.g. structuredContent | ||
| // with a results array from MCP servers like qmd). | ||
| return value; |
There was a problem hiding this comment.
Keep auto output from treating any structured object as JSON
Returning every object from tryParseJson makes wrapped.json() succeed for responses like { structuredContent: { text: "..." } }, and printCallOutput's auto mode checks json() before markdown()/text() (src/cli/output-utils.ts), so default calls now print JSON objects instead of the human-readable text/markdown they previously emitted. This widens the fix beyond --output json and regresses default CLI behavior for servers that include both content and non-wrapper structuredContent fields.
Useful? React with 👍 / 👎.
Returning every object from tryParseJson makes wrapped.json() succeed
for responses like { structuredContent: { text: "..." } }, causing
auto mode to print JSON instead of human-readable text/markdown.
The output-utils.ts fix (JSON.stringify fallback in the json case) is
sufficient on its own — it only affects --output json without changing
default CLI behavior.
mcporter 0.7.3's --output json emits util.inspect() instead of valid JSON, causing OpenClaw's qmd memory search to fail with a JSON parse error. Disable until fix lands (PR steipete/mcporter#89).
Issue steipete#89: When wrapped.json() returns null, fallback to JSON.stringify(raw) instead of util.inspect which produces invalid JSON. This ensures --output json always outputs valid JSON that can be parsed.
|
+1 |
Summary
mcporter call <server.tool> --output jsonoutputsutil.inspect()format (unquoted keys, single quotes,[Object]placeholders) instead of valid JSON whenwrapped.json()returns null.This breaks programmatic consumers like OpenClaw's qmd memory backend, which
JSON.parse()s stdout and getsSyntaxError: Expected property name or '}' in JSON at position 4.Root cause
In
printCallOutput(output-utils.ts), thejsoncase falls back toprintRaw()whenwrapped.json()returns null.printRaw()callsutil.inspect(raw, { depth: 2 }), which is not valid JSON.This happens when an MCP server returns
structuredContentwith fields other than.json/.data(e.g. qmd returns{ results: [...] }) —tryParseJsondoesn't recognize it,json()returns null, and the fallback emits inspect output.Fix
In the
jsonoutput case, tryJSON.stringify(raw)before falling back toprintRaw(). This ensures--output jsonalways emits valid JSON without affectingauto/text/markdownoutput modes.Test plan
result-utils.test.tstests pass (16/16)cli-output-utils.test.tstests pass (1/1)mcporter call qmd.search --args '{"query":"test"}' --output jsonnow emits parseable JSONFixes #80