fix: resolve self-root refs against their source file when bundling - #44
Open
vishkaty wants to merge 1 commit into
Open
fix: resolve self-root refs against their source file when bundling#44vishkaty wants to merge 1 commit into
vishkaty wants to merge 1 commit into
Conversation
Observed: when the bundler inlines a schema fragment extracted from
another file, a `$ref: "#"` inside that fragment was left as-is
(loader.rs local and remote paths; compose.rs inline_internal_refs
likewise handled only `#/$defs/...`). After bundling, `#` resolves
against the nearest enclosing $id, which belongs to the INLINING file,
so the fragment validates against the wrong schema root. In the
2026-04-08 set, checkout payment.instruments[*] (payment.json ->
types/payment_instrument.json#/$defs/selected_payment_instrument, whose
allOf[0] is {"$ref": "#"}) binds to payment.json instead of
payment_instrument.json: instruments missing every required field
(id, handler_id, type) pass, a valid instrument carrying an extra
property named "instruments" is rejected, and strict mode rejects every
valid instrument.
Expected: `#` inside an inlined fragment keeps meaning the root of the
file it was written in.
Fix: when inlined content is extracted from an external file, resolve
`$ref: "#"` by inlining a copy of that file's root schema at the site
($defs/$id/$schema stripped; internal refs still resolve against the
source file as before). Sibling keywords at the `#` site are bundled
first — they are subschemas written in the same source file — and,
because 2020-12 applies `$ref` in conjunction with its siblings, the
inlined root joins them through `allOf` rather than a key-wise merge
that would drop colliding keys; a sibling-free site is spliced
directly. If the inline cannot terminate — the source root itself
contains `$ref: "#"`, or resolving it cycles back into the fragment —
fall back to embedding the source file once as an $id'd $defs resource
and rewriting the ref to that id, which 2020-12 embedded-resource
rules resolve correctly (the rewritten `$ref` next to the remaining
siblings is native conjunction). Synthesized ids for $id-less files
are content-derived, so two different files that happen to share a
relative ref path cannot collide. Whole-file inlines already keep the
file's $id, which is exactly what `#` binds to; if such a file has no
$id a content-derived one is synthesized so `#` cannot escape to the
inlining document root. Applied to both loader paths (local and
remote) and to compose-time $defs extraction, where the inline attempt
is committed unconditionally — a discarded attempt would leak the
nested fallback's bookkeeping and emit a `$ref` to a resource that was
never materialized.
bundle_preserves_self_root_ref previously asserted only that the "#"
text survived bundling; in its own fixture that surviving "#" bound to
the outer document root, i.e. the assertion pinned the buggy behavior.
It now also validates payloads through the bundled schema, so it fails
whenever "#" stops resolving to its source file's root, while the
original textual assertion is kept.
Bundled output for the 2026-04-08 schema set is unchanged except the
selected_payment_instrument allOf branch, which now carries the inlined
payment_instrument.json root instead of a dangling "#".
Fixes Universal-Commerce-Protocol#43
damaz91
added
status:needs-triage
Contributor
|
This LGTM, but I'm not super familiar on how the ucp-schema tool works. @igrigorik do you mind taking a look (or re-assigning to someone with more familiarity with this repo)? |
damaz91
approved these changes
Aug 5, 2026
Author
|
Thanks @damaz91! Happy to give the short version, since the tool internals are the awkward part here. What the bundler does: to validate a payload it inlines the referenced schema files into one document. The bug was that a Why it is safe to take even without deep tool familiarity, in decreasing order of how much it should reassure you:
If it is easier, I am glad to hop on the schema-tool basics or walk through any single hunk — no rush on your end. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #43.
Observed
When the bundler inlines a referenced fragment, a
$ref: "#"inside it is left as is; after bundling, the nearest enclosing$idbelongs to the INLINING file, so the jsonschema crate resolves#to the wrong root (correctly, per 2020-12 embedded resource rules). In the 2026-04-08 UCP set this voids thepayment_instrumentbase schema on every checkout payment path: an instrument with none of the requiredid,handler_id,typevalidates as{"valid": true}, a valid instrument with an extra property namedinstrumentsis rejected, and strict mode rejects every valid instrument. Details and reproduction in #43.Change
#inside an inlined fragment now keeps meaning the root of the file it was written in:inline_self_root_ref(local and remote loader paths, and the compose extraction path) removes the$ref, bundles any sibling values in the source file context first, then attaches the resolved source root: a direct splice when the site carries no siblings, or anallOfentry when it does, so sibling keywords compose by conjunction per 2020-12 rather than key merging.#or refs back into the fragment), the fallback embeds the source file once as a$defsresource with its$idintact (content derived id when the file has none, so distinct files can never collide) and rewrites#to point at it.bundle_preserves_self_root_refnow asserts the resolved SEMANTICS through validation, not just the preserved text; its previous form pinned the buggy binding.Verification
#— nothing else changes.--schemaand the self describing base path): the false accept rejects with the three required property errors, the false reject and the strict mode rejections accept.#before compose extraction sees one; it is pinned by a unit test at the function level, which is where the contract lives and where a future pipeline reordering would resurrect it.Happy to split anything out or adjust the approach if you prefer one of the other directions sketched in #43.