fix: stop double JSON-encoding execute_sql and get_logs results - #339
fix: stop double JSON-encoding execute_sql and get_logs results#339radmirnovii wants to merge 1 commit into
execute_sql and get_logs results#339Conversation
…base#311) Results were JSON-encoded twice: once inside the untrusted-data wrapper and again when the output object was serialized into MCP text content, so backslashes in queried data reached the LLM quadrupled and corrupted round-trips of function definitions. Tools can now provide a `textContent` serializer whose text is sent verbatim, with the full output also returned as `structuredContent` for typed clients. execute_sql and get_logs use it, so results are encoded exactly once. Includes a regression test for the E'\\' round-trip.
barryroodt
left a comment
There was a problem hiding this comment.
Thanks for the contribution, and for digging into this. The double-encoding diagnosis is exactly right, and I appreciate that you traced it to the two JSON.stringify calls instead of patching backslash counts at the string level.
I checked it locally rather than just reading the diff: both suites pass at head (mcp-utils 13/13, supabase unit 192/192), and the merge-base encoding does fail your new test, so it's a real regression test. Re-creating the function from the returned definition and comparing the stored body was a nice touch.
One small request. README.md:113-114 says this server doesn't send structuredContent, and that AI SDK falls back to parsing JSON from content text. Both halves stop being true for execute_sql and get_logs with your change, and that note sits in the canonical AI SDK integration section, so an integrator reading it would get the opposite of what the server now does. If you're up for amending those two lines here, great. If you'd rather keep the PR to the fix, that's fine too and one of us can follow up with the docs.
I also have some non-blocking notes from a deeper read, an injectableTool cleanup plus a couple of coverage gaps. Happy to post them once this lands, or now if you'd prefer them together.
Fixes #311
Root cause
As diagnosed in #311 (comment),
execute_sqlresults were JSON-encoded twice:wrapWithUntrustedDataBoundaryembeds the query rows viaJSON.stringifyinside the<untrusted-data-*>prose wrapper.createMcpServer's tool-call handler then serializes the whole output object ({ result: "<wrapper text>" }) with a secondJSON.stringifyto produce the text content.A single backslash in queried data (e.g. the
E'\\'literal in a PL/pgSQL function body) reached the model as 8 backslashes. Models routinely collapse that to the wrong count when writing the definition back, corrupting round-trips of function definitions.get_logshad the same double encoding.Fix
Tools in
@supabase/mcp-utilscan now declare an optionaltextContent(output)serializer:JSON.stringify).structuredContent, so typed clients keep parsing outputs exactly as before — the AI SDK integration (supabaseMcpToolSchemas) validatesstructuredContentfirst and never falls back to parsing the prose text.execute_sqlandget_logsuse it to send the untrusted-data wrapper directly, so query data is now JSON-encoded exactly once. All other tools are unchanged.This is also a first concrete step toward the
outputSchema/structuredContentsupport mentioned in the issue:createMcpServernow emitsstructuredContent(currently only for tools that opt intotextContent); advertisingoutputSchemaintools/listand emittingstructuredContentuniversally can build on top of this.Behavior change
execute_sql/get_logstext content is now the untrusted-data wrapper itself (prose containing single-encoded JSON) instead of a JSON object with the wrapper double-encoded inside. The text is meant for model consumption and was never parseable into row data in one step before.structuredContent({ result: string }).tools/listoutput is untouched — no changes to tool names, descriptions, input schemas, or annotations (the frozen-field contract for ChatGPT snapshots is unaffected;outputSchemais not advertised intools/list).get_logs's internal output schema is corrected from{ result: z.unknown() }to{ result: z.string() }to match what it has always returned.Verification
E'\\', read it back viapg_get_functiondefthroughexecute_sql, assert the text contains the E-string encoded exactly once (4 backslashes, not 8), re-create the function from the returned definition, and assert the stored body is byte-identical. Also assertsstructuredContentmatches the text.mcp-utilstest coverstextContentverbatim text +structuredContentemission.Typecheck (
tsc --noEmit) andbiome checkpass in both packages.🤖 Generated with Claude Code