Skip to content

fix(server): configure structured-content text fallback - #2798

Merged
bokelley merged 3 commits into
mainfrom
configure-client-payload
Sep 1, 2026
Merged

fix(server): configure structured-content text fallback#2798
bokelley merged 3 commits into
mainfrom
configure-client-payload

Conversation

@bokelley

@bokelley bokelley commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make the MCP structured-content text mirror configurable globally or per negotiated client
  • apply the marked fallback at the transport edge so idempotency caches retain canonical responses
  • keep A2A artifacts clean and fail safe to mirroring when MCP/direct client identity is unavailable
  • preserve exact-string deduplication and tag SDK-generated blocks with _meta["adcp/mirrored-structured-content"]

Follow-up to #2794. The registry contract sync merged there is intentionally untouched here.

Verification

  • npm run typecheck
  • npm run build:lib
  • 11 focused policy, legacy MCP, modern MCP, direct replay, and A2A tests
  • targeted ESLint and Prettier checks

Known baseline issue

A broader local server-test run currently encounters upstream AdCP 3.2.0-beta.9 manifest drift: core tools are marked as added in stable 3.2.0, producing existing VERSION_UNSUPPORTED failures under the beta pin. The feature tests use the stable bundled 3.1.18 release to isolate this change.

Comment thread .changeset/fair-hosts-listen.md Outdated

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Comment (human reviewer recommended)

Comment — not auto-approvable due to changeset impact concern.

The reviewer needs a closer human look at the changeset classification. This PR moves the MCP structuredContent→text mirror out of the canonical/idempotency-cached response into a marked, per-client transport-edge decoration. All four edges (legacy tools/call, modern invoke, direct invoke, A2A) were verified to apply the fallback exactly once with no double-mirroring; canonical/cache objects stay clean via shallow copy; fail-safe and A2A-clean semantics hold; no fabrication.

Medium findings

  • .changeset/fair-hosts-listen.md — changeset is patch but the PR adds new public API (a config option plus five new exports); per the repo's changeset-vs-wire-impact rule this should be minor.

Note: The reviewer summary flags the understated changeset as a non-blocking Medium. Per the repo's own changeset-vs-wire-impact mandate, a changeset that understates impact can rise to high; however the reviewer classified this specific case as Medium (a minor-vs-patch bump for additive new exports, not a removed/renamed export or response-shape change), and no finding was entered at high/critical severity. With a single Medium finding and no escalation trigger, the decision table would normally fall through to approve — but review_decision is REVIEW_REQUIRED, so a human should confirm the changeset bump before merge. Surfacing for human review.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean subsequent pass, no blocking or medium findings.

This is a follow-up to a prior comment decision. The delta since the prior review is contained to four files and introduces no new findings:

  • .changeset/fair-hosts-listen.md — bumped patch→minor, correct for the additive new exports (changeset-vs-wire-impact satisfied).
  • Two accurate doc comments on the stateless legacy-HTTP fail-safe path.
  • One E2E test correctly validating the fail-safe (unknown client → predicate not reached → compatibility mirror retained), matching the shouldMirrorStructuredContent clientInfo-undefined branch.

Decision-rule trace (top-down):

  • Rows 1: no critical/high findings.
  • Row 2: gated_paths is false — not triggered despite review_decision: REVIEW_REQUIRED.
  • Rows 3–5: high_risk is false; no medium findings.
  • Row 6: prior decision was comment (not escalate), and no new critical/high/medium findings.
  • Row 7: no no-auto-approve team match.
  • Row 8: fewer than three medium findings (zero).
  • Row 9: approve.

No witness-not-translator, transport-client, changeset, or doc-link concerns surfaced.

@bokelley
bokelley merged commit 8446c55 into main Sep 1, 2026
37 checks passed
@bokelley
bokelley deleted the configure-client-payload branch September 1, 2026 01:46
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.

1 participant