Skip to content

fix(mcp-server): mirror tool payloads into content so rows reach the model - #4295

Merged
gilgardosh merged 4 commits into
mainfrom
fix/mcp-mirror-payload-into-content
Aug 27, 2026
Merged

fix(mcp-server): mirror tool payloads into content so rows reach the model#4295
gilgardosh merged 4 commits into
mainfrom
fix/mcp-mirror-payload-into-content

Conversation

@gilgardosh

Copy link
Copy Markdown
Collaborator

The symptom

Asking the connector for a counterparty's charges returned this, and only this:

Found 7 charge(s) across 2 businesses; showing 7 on page 1 of 1.

No ids, dates, amounts or business names — from any tool. The model couldn't work around it by
changing filters, because it had nothing to filter. Verified locally in Claude Desktop before and
after: full data now comes through.

Cause

Every list tool funnels through shapeListResult, which put the summary in content and the rows
only in structuredContent:

return {
  content: [{ type: 'text', text: summarize(shown, total, truncated) }],
  structuredContent: structured,   // ← the rows lived only here
};

Under MCP 2025-06-18, structuredContent is contractually meaningful only when a tool advertises an
outputSchema. None of ours ever have — outputSchema has never existed in this package. A client
is therefore free to ignore that field, and the spec correspondingly asks a server returning
structured content to also return it serialized in a TextContent block. We did neither, so every
tool's data rode on undefined-by-spec client behaviour. When the client stopped surfacing unschema'd
structured content, the rows stopped arriving.

Not a regression in this repo. shapeListResult's return statement was byte-identical to its
original from the package's first commit (b82802376); output.ts has been touched three times
ever, and both recent changes were purely additive. executeRegisteredTool (execute.ts:230) and
dispatchMcpRequest (handler.ts:238) pass the result through untouched. It was an original design
gap that only became visible when the assumption underneath it changed.

Confirmed empirically before fixing, by dumping a real result with a stubbed upstream:

content blocks: 1
does content mention CHARGE-AAA? false

Fix

A single mirroredResult(summary, structured) in src/tools/output.ts, which shapeListResult,
shapeWriteResult and toToolErrorResult all return through. Summary still leads, followed by the
serialized JSON; structuredContent is kept as-is for hosts that consume it directly.

Deliberately one function rather than a per-tool convention — the failure being fixed is exactly the
kind that drifts back one tool at a time. No tool handler changed; all nineteen already funnel
through those three functions.

Two less obvious things this restores:

  • accounter_list_business_memberships instructs the model to "Pass their memberBusinessId
    values"
    while those ids lived in the invisible field. Discovery that can't be acted on breaks the
    scoping workflow every other tool depends on.
  • Error payloads mirror too. A VALIDATION_ERROR's field-level issues never reached the model, so
    a rejected call said that it was wrong but never what to fix — and it would retry the same
    shape. accounter_explain_terminology was likewise returning the whole glossary into a field
    nothing read.

The 60KB budget is unchanged and still measures what the model consumes: fittingCount
binary-searches on JSON.stringify(structured), which is now exactly the mirrored text. The
JSON-RPC body roughly doubles — far under the 1MB transport cap. A client rendering both channels
sees the payload twice; that's the accepted cost of not depending on which one it reads.

Why the tests didn't catch it

They couldn't. Rows were asserted exclusively through result.structuredContent, and
content[0].text only ever for summary strings. The suite went 760 → 760 green across the
broken/fixed boundary.

tools/__tests__/mirroring-contract.test.ts closes that in two layers:

  1. A sweep over toolRegistry.list() asserting any structuredContent is carried by a content
    block. Verified it bites by temporarily reverting the fix — it fails per-tool with
    accounter_search_charges returned structuredContent that no content block carries.
  2. A source-level check that no file under src/tools/ builds a content array by hand. The first
    layer has a blind spot: with an empty upstream, most data tools return a mirrored error, so a
    new tool hand-rolling an unmirrored success could slip past it.

Verification

  • 785 tests pass (53 files); typecheck and prettier clean
  • Change is contained to mcp-server — nothing outside imports these functions
  • End-to-end through Claude Desktop against a local server: full charge data renders

Follow-up, deliberately not in this PR

Declaring outputSchema per tool would restore structuredContent as a second channel and is the
proper 2025-06-18 shape. It's left out because the fix no longer depends on it, and because
"servers MUST provide structured results that conform" means a schema drifting from the payload
turns working calls into client-side errors — 17 hand-written schemas of risk for redundancy we
don't currently need.

🤖 Generated with Claude Code

…e model

Every list tool put its summary line in `content` and the rows only in
`structuredContent`. Under MCP 2025-06-18 that field is contractually
meaningful only when a tool declares an `outputSchema` — none of ours do —
so a client may ignore it, and the spec asks servers returning structured
content to also serialize it into a TextContent block. We did neither, so
when the client stopped surfacing unschema'd structured content, every tool
returned counts with no rows behind them.

Adds a single `mirroredResult()` in `tools/output.ts` that `shapeListResult`,
`shapeWriteResult` and `toToolErrorResult` all return through. No handler
changed; all nineteen tools already funnel through those three functions.

Also restores two less obvious things: the `memberBusinessId` values that
`accounter_list_business_memberships` tells the model to pass, and the
field-level `issues` on a `VALIDATION_ERROR`.

The 60KB budget is unchanged and still measures what the model reads —
`fittingCount` searches on the same string that is now mirrored.

Guarded by `mirroring-contract.test.ts`: the suite previously passed green
while the connector was blind, since rows were only ever asserted through
`structuredContent`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gilgardosh
gilgardosh temporarily deployed to accounter-fullstack August 26, 2026 14:04 — with GitHub Actions Inactive
@gilgardosh
gilgardosh temporarily deployed to accounter-fullstack August 26, 2026 14:04 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@accounter/client 0.1.0-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/green-invoice-graphql 0.8.7-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/hashavshevet-mesh 0.2.13-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/israeli-vat-scraper 0.1.13-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/modern-poalim-scraper 0.11.0-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/payper-mesh 0.2.13-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/scraper-app 0.0.3-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/server 0.2.0-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/shaam-uniform-format-generator 0.2.7-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎
@accounter/shaam6111-generator 0.1.9-alpha-20260827074155-e9eda61bebae2bb5a0ed35f161555f186006f933 npm ↗︎ unpkg ↗︎

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The fix is contained and well-tested, with only a minor suggested robustness tweak to one test heuristic.

Pull request overview

Fixes MCP tool output shaping so that structured payload rows (and error details) reliably reach the model by mirroring JSON into content alongside structuredContent, aligning with MCP 2025-06-18 expectations for unschema’d structured results.

Changes:

  • Introduce mirroredResult() and route list/write shaping (and taxonomy errors) through it so payloads are visible in content.
  • Add targeted unit tests and a registry-wide contract test to prevent regressions where payloads are only emitted via structuredContent.
  • Document the mirroring contract and rationale in the mcp-server README and publish a patch changeset.
File summaries
File Description
packages/mcp-server/src/tools/output.ts Adds mirroredResult() and updates shaping helpers to mirror structured payload JSON into content.
packages/mcp-server/src/tools/tests/output.test.ts Adds unit tests asserting list/write shapers mirror structured payloads into content and keep the byte budget meaningful.
packages/mcp-server/src/tools/tests/mirroring-contract.test.ts Adds registry-wide tests enforcing that any structuredContent is also carried via a content text block.
packages/mcp-server/src/errors/taxonomy.ts Mirrors error payloads into content so validation issues/correlationId reach the model.
packages/mcp-server/README.md Documents the mirroring behavior and the contract test that enforces it.
.changeset/mcp-mirror-payload-into-content.md Publishes a patch changeset describing the symptom, cause, fix, and test coverage.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/mcp-server/src/tools/__tests__/mirroring-contract.test.ts
gilgardosh and others added 2 commits August 27, 2026 10:36
…bstring

The drift guard looked for the literal `content: [`, so `content:[` or
`content : [` would have slipped through. Prettier normalizes the spacing
today, but a guard against drift should not depend on prettier having run.

Verified by injecting a `content : [` result into a tool file: the regex
flags it, the substring check did not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gilgardosh
gilgardosh temporarily deployed to accounter-fullstack August 27, 2026 07:37 — with GitHub Actions Inactive
@gilgardosh
gilgardosh temporarily deployed to accounter-fullstack August 27, 2026 07:37 — with GitHub Actions Inactive
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gilgardosh
gilgardosh temporarily deployed to accounter-fullstack August 27, 2026 07:40 — with GitHub Actions Inactive
@gilgardosh
gilgardosh temporarily deployed to accounter-fullstack August 27, 2026 07:40 — with GitHub Actions Inactive
@gilgardosh
gilgardosh merged commit 2f42953 into main Aug 27, 2026
11 checks passed
@gilgardosh
gilgardosh deleted the fix/mcp-mirror-payload-into-content branch August 27, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants