Skip to content

fix(server): keep product format divergence nonfatal - #3691

Open
aleksUIX wants to merge 2 commits into
adcontextprotocol:mainfrom
aleksUIX:fix/server-product-format-divergence-3685
Open

aleksUIX wants to merge 2 commits into
adcontextprotocol:mainfrom
aleksUIX:fix/server-product-format-divergence-3685

Conversation

@aleksUIX

@aleksUIX aleksUIX commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Keep get_products successful when one product has divergent legacy and canonical format declarations. Treat format_options as authoritative, remove contradictory legacy routes and append FORMAT_DECLARATION_DIVERGENT with SDK provenance and product evidence. Preserve unrelated products and existing diagnostics, and deduplicate by code and field. Remap advisory fields after legacy product filtering and retain product identity when the affected product is omitted.

Validation: 283 packaged catalog, server and version-translation tests pass on Node 24 with the official runner, including strict 3.1 response validation and legacy 3.0 projection. Seven original regressions fail before the fix. Two additional regressions fail before the follow-up and verify advisory paths after 3.0 filtering. Build, typecheck and the patch changeset check pass. The full SDK suite was not completed locally: an earlier run hit a 60-second timeout in an unchanged type fixture, which passed in isolation after warming.

Hosted CI: Generated, Docs & Formatting fails because the live registry sync changes schemas/registry/registry.yaml and src/lib/registry/types.generated.ts. Both files are unchanged by this PR. The official sync generator reproduces the drift from pristine main inputs at ccfc06c.

Fixes #3685. Refs #3642.

source: 'sdk',
sdk_id: `@adcp/sdk@${LIBRARY_VERSION}`,
field: path.replace(/^\$\./, ''),
details: {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium: field is computed against pre-projection product indices, but toCanonicalOnlyResponse can drop a product and compact the output array without throwing. A legacy-only product whose format_ids are unmappable projects to empty format_options → keep=false (augment-response.ts:656), dropped with a LEGACY_FORMAT_ID_DROPPED_UNMAPPED diagnostic, which is explicitly filtered out of blockingDiagnostics (L1004). So a response like [A: legacy-only unmappable, B: divergent dual] drops A, moving B to output products[0], while B's advisory still carries field: 'products[1]' — an index that no longer exists. details.product_id stays authoritative; the field pointer silently lies. Re-map the advisory field after projection, or anchor the advisory on product_id only.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Nov 11, 2026

@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 — reject→normalize-and-report conversion of dual-emission format-declaration validation is sound and well-tested.

Checked: non-mutating behavior confirmed (deepEqual(response, before)), legacy-ref evidence survives canonical identity strip via reconciliation pulling from projected.response.errors, dedup by code+field, and enclosingProductId propagation to nested placements. Divergence throw removals are reported loudly via advisories — clears the witness-not-translator bar. Changeset (patch) is defensible for a recovery fix that keeps catalogs available for a previously-rejected case. Coverage is complete.

Medium findings

  • src/lib/server/decisioning/runtime/from-platform.ts:1727 — Advisory field uses pre-projection product index; projection can compact the array. Non-blocking: details.product_id remains authoritative and the triggering drop (LEGACY_FORMAT_ID_DROPPED_UNMAPPED) is itself non-blocking.

No Critical/High findings. One Medium with no escalation trigger (high_risk false, gated_paths false, no team gate, not data-loss/schema/infra category) — falls through to approve per row 9.

Medium findings

  • src/lib/server/decisioning/runtime/from-platform.ts:1727 — Advisory field uses pre-projection product index; projection can compact the array

@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 fix, prior Medium resolved.

This subsequent push addresses the prior Medium finding (from-platform.ts:1727 — advisory used pre-projection product index). The reviewer confirms asProductResponseForWire now builds originalToOutputIndex and remapDivergenceAdvisory rewrites the FORMAT_DECLARATION_DIVERGENT advisory products[N] field to the compacted output index after empty-format_ids products are dropped, collapsing to the products collection when the divergent product is itself dropped. Index source matches normalization order (toCanonicalOnlyResponse preserves order/count). No mutation; non-divergence errors untouched. Two new tests cover both omission cases, satisfying the changeset-vs-wire and test requirements.

No current findings. No team gate, gated_paths false, high_risk false. Prior blocking/Medium issue is gone and no new findings surfaced — net improvement. Falls through to row 9.

This branch has not been deployed

No deployments
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.

Server runtime refuses a whole get_products answer for one divergent product; AdCP asks for FORMAT_DECLARATION_DIVERGENT in errors[] instead

1 participant