Uh oh!
There was an error while loading. Please reload this page.
fix(compilers/openapi): read x-* at every object that carries it - #345
Merged
Conversation
This was referenced Aug 9, 2026
Both sides grew a per-route wrapper around the path item's residue: this branch added applyPathItem for its servers and x-*, main added applyPathItemResidue for its servers and documentation. They are one mechanism, so the merged function is main's, carrying all three — which is the third construct its doc comment anticipated. The test helpers this branch's new encoding cases used moved into the openapitest package on main; the cases now call them there.
Three gaps found reviewing the extension sweep, all in what the tests claim rather than in what the compiler does. An encoding entry declaring only allowReserved or only x-* lowers to an empty PartEncoding, which partEncodings drops from Content.Encoding. Reading the entry has to precede that check or what it declared goes with it, and a comment says so, but moving the read below the check left the whole suite green: every encoding fixture also declares a style or a contentType, so none of them lowers an empty PartEncoding at all. The new case declares nothing else, and reddens on that swap with both values gone and no diagnostic. Four rows of assertEveryObjectKeepsItsExtensions named their carrier as a bare ".Unmodeled", which ends every path the value walk produces — all 39 in that fixture — so the carrier half of those rows admitted any map in the document. Pointing them at ".Operations[0].Unmodeled" makes a misplaced entry fail where the same wrong expectation passed before. ExtensionsUnder justified its keys only against unscoped ones, which does not cover the non-extension keys sharing its scopes — openapi:encoding/<part>/ allowReserved is written two functions away. The keys are distinct; the comment now gives the reason that actually holds.
The keys an emitter reads are contract, and this sweep added about twenty of them plus the scoping rule behind them without any of it reaching the normative document. §12 now states the grammar — format prefix, then the path from the carrier down to the object that wrote the entry, for the objects that lower to no node of their own — and why an unscoped key will not do: several such objects reach one map, where two of them writing x-id leaves a survivor chosen by lowering order, which is the order-dependence §4.3 forbids for minted IDs. The OpenAPI 3.x row enumerates the keys and drops two statements this work made untrue: that links reach Response.Unmodeled, which said nothing about the error branch they were being dropped on, and that x-* is simply preserved, which was the claim the sweep found unmet at twenty-one of twenty-eight objects. It also records allowReserved, whose entry has to be read before the encoding's emptiness is judged. carrierNarrows holds each row of assertEveryObjectKeepsItsExtensions to a carrier that picks out fewer than all the document's Unmodeled maps. Naming a bare ".Unmodeled" ends every path the value walk produces, so four rows were asserting nothing about placement; pointing them at the operation fixed those four, and this fails the next row spelled that way instead of passing it.
Two of the scopes an Unmodeled key is built from address an object the document names rather than one the compiler does: a form part, by schema property name, and a callback, by map key. Both interpolated that name into a "/"-delimited scope unescaped, so a name containing a separator read as two segments and two objects spelled one key between them. Form parts named "q" and "q/x-a" both write openapi:encoding/q/x-a/x-b, and which entry survives follows the order the two properties were declared in — one order keeps the first part's value, the other keeps the second, and neither reports anything. That is the loss this branch exists to stop, arrived at from the other side, and §4.3's rule against a minted node reusing a pointer reached by a different route. The callback scope had it identically, so both now take their scope from ids.Scope, which applies the escaping Ptr already applies to a pointer segment. Each site has its own case, because reverting one escape reddens only its own. The ids tests hold the escaping itself, including the property the whole thing exists for: a name that spells another's scope plus a segment must not produce that other's key. Also here, from the same review: carrierNarrows reported a carrier matching no map at all as one matching every map, naming the opposite defect to a reader chasing it, and §12 gave a reason for the keys being distinct that was untrue of the two scopes above and of pathItem, which no keyword spells.
Uh oh!
There was an error while loading. Please reload this page.
OmarAlJarrah added a commit
that referenced
this pull request
Aug 11, 2026
Three ways an undeclared key still reached the IR in no form at all, each leaving two documents that differ compiling to the same document. The Components Object took no census. It was excluded as one of the maps whose every key is a valid entry, which `paths`, `responses` and a callback are — each embeds a sequenced map — while Components is a fixed-field struct beside them, so a key it does not define is as undeclared as one anywhere else. It is a census site now, keyed under "components" on the carrier #345 already gives its extensions. A key written as an alias was reported by the parser under the name it resolves to, while the mapping still holds an alias node whose own value is the anchor; searching it raw found nothing, and a key with no node kept nothing and said nothing. RawChildNode now compares the resolved name, which is the name every caller asks by. A key holding a "/" spelled the scope of the object that path names, so a root key "info/contact/slack" was the same entry as the contact object's own "slack" and the second site to reach the carrier dropped its key in silence. The key is escaped as one segment, per the rule ids.Scope already records for the scopes a document chooses the segments of. Beside them: a key the raw mapping does not present at all is announced under openapi/unknown-key-unreachable instead of passed over. The one class that reaches it is a key merged in through a `<<`, which needs the merge-expanded view this package cannot reach today (#395); the bound now applies to what the census contributes rather than to keys another reader already kept, and tagUnknownSites skips a nil entry the way tagExtensions does.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
A vendor extension declared on most OpenAPI objects reached the IR in no form at
all: no field, no
Unmodeledentry, no diagnostic. Compiling a document thatwrites
x-*on every object it can showed only seven of the twenty-eightextension-bearing objects being read — the document root, a parameter, an
operation, a media type, a header, a security scheme, and a schema. Everything
else was dropped in silence, which invariant 2 forbids.
The two reported instances (a response's extensions, an encoding's) are two
sites of that one mechanism, so this reads them everywhere instead. Every object
below now keeps its
x-*:info, contact, license, externalDocs (root, tag, operation and schema), server,
server variable, tag, paths, path item, request body, encoding (and 3.2
itemEncoding), example, responses, response, error response, callback,components, oauth flows, oauth flow, xml, discriminator.
Most of them lower to a node with an
Unmodeledmap and their entries keystraight onto it. The rest — an info block, an encoding, a path item, a callback
— lower to no node of their own and ride on the nearest one that does. Several
can reach the same map, where
openapi:x-idfrom two objects is one key and thesurvivor would depend on which lowering ran last, so those entries are keyed
under the source path from the carrier down to the object that wrote them
(
openapi:info/contact/x-id,openapi:encoding/<part>/x-id). A scoped keycannot collide with an unscoped one, since only an
x-key reaches the readerand no scope begins with
x-.Two constructs beside the sweep, both mechanical:
linkswas preserved on a 2xx and dropped ona 4xx, so the same declaration survived or vanished on nothing but its status
code.
ir.Responseandir.ErrorCaseare two lowerings of one source object,so both now go through one helper and a construct added there reaches both.
allowReservedon an encoding. It has noir.PartEncodingfield althoughstyleandexplodebeside it do, and nothing read it: two documentsdiffering only in it compiled to a byte-identical IR. It is kept verbatim on
the owning content with
ReasonNoIRHomeand one info diagnostic, the shapepreserveHeaderSerializationalready uses for the pair beside it.Not swept, deliberately: a Link Object's own
x-*gets no entry of its own.Nothing is lost — the whole
linksmap is preserved verbatim on the response,extensions included — so a separate keyed entry would duplicate what is already
there. Unknown non-
x-keys and JSON Schema keywords beside a$refaredifferent problems and are untouched here.
RequestBodyandServerVariableexpose noGetExtensions()at speakeasyv1.24.0, so their exported
Extensionsfield is read directly; both accessesare nil-safe and the choice is noted at each site. A request body declaring no
content lowers to no payload and so has nowhere to carry its extensions — such a
body is invalid per OpenAPI, which makes
contentREQUIRED there; thatlimitation is stated in
lowerRequestBody.Test plan
testdata/conformance/openapi/extensions-x.yamlnow writesx-markon everyobject that admits one, with a value naming the object.
assertEveryObjectKeepsItsExtensionsderives the carriers from the IR valuegraph rather than naming them, so a row says both that the entry survived and
which
Unmodeledmap it landed on. Before the fix, 25 of its 30 rows failed.response-links.yamlgained a 4xx response carrying links, declared beforethe 201 so the fixture is in the order that used to be wrong.
multipart-encoding.yamlgainedallowReservedon its form encoding. Eachissue is therefore visible in its own capability fixture, not only in the
sweep.
ExtensionsUnder/ExtensionsAtininternal/annotation(including thetwo-objects-one-key case and the unserializable-site case), encoding
allowReservedandx-*at both the multipart anditemEncodingpositions ininternal/operation, with a control asserting an encoding that declaresneither records neither.
time the rows naming that site went red: all extension reads, the error-branch
links and response extensions, encoding
allowReserved, the schemasub-objects, the path item, the document-level sites, and the oauth flows.
extensions-xis exactly the newallowReservedentry, the 4xx links entry, and the source hashes. Six IRfields left the unwitnessed list.
gofmt,go vet,golangci-lint,go build, and coverageat 100%.
Closes#275
Closes#291