fix(compilers/openapi): keep format: password at every position

[fuad-daoud]

Summary

formatTable mapped string/password to ir.PrimString — the same kind type: string alone selects — so scalarTypeID returned the shared primitive and nothing recorded the format. The one position that worked was an inline property, where FillPropertyDetail read the literal; a $ref, array items, additionalProperties, patternProperties, prefixItems, parameters, content schemas and headers all lost it silently. grep -c password on the issue's output was 0.

A position whose format is password now hoists a Scalar of its own carrying Sensitive (whole-type redaction) and Encoding.Name "password" (the source token, verbatim), exactly as byte and an unknown format already do — a control run with format: hex-token in the same positions shows the pre-existing target shape. Property.Secret is set on every property and header whose type reaches that node: directly, through a nullable oneOf/anyOf, or along a Base chain (an allOf over one $ref, an alias component). MarkSecretUses does this once lowering is done, because a $ref names a component before that component is lowered.

grep -c password on the issue's document: 0 → 4, with no diagnostic at any severity (--fail-on warning exits 0).

Decisions

• Carrier: the node, plus the per-use field where one exists. The primitive is interned once per kind and shared by every declaration of that type, so a per-declaration fact cannot live on it; Sensitive + Encoding.Name on a hoisted Scalar is the same shape byte and unknown formats produce. Property.Secret is set beside it at a property or a header.
• password keys on the format, not on string, so format: password on a non-string type or a union variant states the same redaction instead of surviving only as a verbatim format string.
• No new diagnostic. Every scalar position now reaches a field. Two exceptions are recorded rather than fixed here. Beside enum/const, the position lowers to an Enum or a Literal, which keeps the format verbatim in Unmodeled with an info degraded-construct and is not Sensitive (openapi: an enum ignores its format although Enum.ValueType can carry it #607, ir: a Literal carries no value type or format #668). Beside a differing contentEncoding, Encoding.Name is the content encoding and the format is kept verbatim with the same info, while Sensitive still holds.
• Parameters have no per-use redaction field (ir.Parameter has none), so the parameter's type node is the carrier and #578 owns that field; the position says so in code.
• No IR schema or IRVersion change — comments and the normative OpenAPI row only.

Accepted behaviour changes

• A password property/header/parameter now owns a node, so its docs/xml/deprecation/examples land on the node while default and the value constraints stay on the carrier — measured identical to the byte and unknown-format controls.
• An allOf redeclaration of password over a plain {type: string} (and the reverse) is now a silent drop of the losing declaration, kept verbatim under openapi:conflicting-redeclaration, with no new error or warning. Measured in both declaration orders. The same order-dependent drop already happens on main for an unknown format (hex-token); openapi: an allOf redeclaration of a property with an unknown format is resolved by branch order #782 tracks it, and the code comments point there.
• When a $ref points at a property's schema by pointer, a password property that writes writeOnly, readOnly or default now follows openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750: an info degraded-construct is reported only when the reference lowers first, so morphic-harness reports the document order-dependent on diagnostics. That happens because the position now owns a node. format: hex-token already behaves this way on main, and it can't be fixed here without fixing openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750.

Test plan

• New conformance case secret-format.yaml + golden, asserting where the fact lands at every position: a named component, an inline property, a property via $ref, array items, additionalProperties, patternProperties, prefixItems, a header parameter schema and a query parameter via $ref, request and response content schemas, and a response header — plus the guard that a plain {type: string} still targets the shared t/prim/string and is not sensitive.
• TestFormatTable_EveryRowDistinguishesTheBareType pins the invariant the row broke (a pairing may only map to a kind its bare type does not); re-inserting the row reddens it, and removing it again leaves the tree byte-identical.
• compose_test.go reworked onto a format-free fixture, with the Secret OR-fold kept as its own focused case in both declaration orders.
• assertScalarFormat extended, and unwitnessed.golden.txt loses TypeCommon.Sensitive as the field becomes witnessed.
• go run ./cmd/morphic-harness testdata/conformance/openapi/secret-format.yaml → ok; go test ./internal/harness green; make gate green on the branch (all statements covered, golangci-lint 0 issues).
• The fixture also covers the wrapper spellings (nullable oneOf/anyOf, allOf over one $ref, an alias declared after its first use), a nullable response header, and length bounds at a component, at array items and at a parameter. Removing the MarkSecretUses call reddens it, and so does dropping a Sensitive Scalar's constraints.
• TestRedactionFormat_BesideContentEncoding and TestRedactionFormat_BesideEnumOrConstIsNotSensitive pin the contentEncoding path and the enum/const limit. The case no longer claims the custom-scalars matrix row, which it does not exercise.
• The corpus-test ordering change that an earlier commit carried is reverted here; the race it addressed is compilers/openapi: corpus reader races the golden writer under -update #817.
• A final whitespace commit ends the new fixture with a newline (the convention every other fixture follows) and regenerates the one-line source hash in its golden.

Closes #579.

[fuad-daoud]

Rebased onto main (07576ab) at 2026-09-30. The first CI run failed with a golden mismatch on the new fixture: #551 landed after this branch was cut and makes a shared primitive carry provenance.source: -1 (NoSource) where the golden said 0. The new golden's one primitive line was regenerated; go test ./... is green on the rebased branch and the gate is re-running. No production code changed in this rebase.

[OmarAlJarrah]

Reordering allOf branches now changes the merged field, and one order silently loses detail.

Repro: allOf of {properties: {token: {type: string}}} and {properties: {token: {type: string, format: password, minLength: 8, default: x, description: d, example: e}}}.

• main, either order: one property with default, constraints.minLength, docs.description, examples and secret.
• this branch, plain declaration first: type is t/prim/string, secret: true, and the default, constraints, description and example are gone from the property; the whole password declaration is parked under openapi:conflicting-redeclaration<ptr>.
• this branch, password declaration first: type is the hoisted t/anon/.../allOf/0/properties/token and the default and constraints are kept.
• no diagnostic in either order.

mergeVisibility documents that allOf gives branch order no meaning, and the order-invariance oracle does not reorder allOf branches, so the corpus sweep cannot see this. The comment here ("the format drops") understates it: it is every shape detail of the redeclaration that drops, and in the password-first order it is the plain declaration that is dropped instead.

Suggest treating a format-only hoist difference as a compatible narrowing (fold the details, pick the richer type independent of order) rather than leaving it to #446, or at minimum reporting it.

[OmarAlJarrah]

Three corrections to this comment.

• The PR body already lists this under accepted behaviour changes, and I should have acknowledged that. The trade-off is a regression only for password, which main folds because it resolves to the shared primitive.
• The asymmetry is not new for formats in general. An unknown format already behaves this way on main: hex-token beside a plain string gives t/prim/string with no default or bounds when the plain branch is first, keeps both when the format branch is first, and reports nothing in either order. That is now openapi: an allOf redeclaration of a property with an unknown format is resolved by branch order #782. byte is not part of it; it reports an incompatible-types warning.
• The in-code pointer to typesConflict reports a format-narrowed primitive as conflicting with its bare primitive #446 does not cover this. typesConflict reports a format-narrowed primitive as conflicting with its bare primitive #446 and openapi: an allOf that narrows a property's type is reported as a conflict and resolved by branch order #598 are about typesConflict reporting a conflict between primitive kinds that can both hold. Here typesConflict resolves the Scalar to string, reports nothing, and the shape fold still discards one side. Please point this comment at openapi: an allOf redeclaration of a property with an unknown format is resolved by branch order #782 instead.

I did not find this shape in a sweep of 157 real-world specs that use format: password, so the cost is the order dependence and the lost typed fields, not something it hits in practice.

[fuad-daoud]

Repointed to #782 in 291a221. The comment now says the later declaration is dropped whole, its shape details with it, and that which declaration that is depends on branch order. I left the fold itself to #782: it is the same mechanism that hex-token already hits on main, and fixing it only for password would leave the two disagreeing.

[OmarAlJarrah]

Property.Secret is still false for two common spellings of a reference to a redaction schema, even though the Sensitive node sits right behind them.

Probed with Pw: {type: string, format: password}:

• {oneOf: [{$ref: Pw}, {type: "null"}]} and the anyOf form: type is Pw (nullable), secret: false.
• {allOf: [{$ref: Pw}], description: ...} (the usual way to annotate a $ref): anonymous Model with Base: Pw, sensitive: false, secret: false.
• {$ref: Pw}: secret: true.
• An alias component Chain: {$ref: Pw} is sensitive: false as well, so a parameter or array item typed Chain has no redaction signal on its own type node.

The ir-design row says Property.Secret is set beside the node at a property or a header, so an emitter that builds a redacting String() from Property.Secret would emit these fields in clear.

[OmarAlJarrah]

Scoping this: nothing is lost. In each of these spellings the property's type still reaches the Sensitive node (the nullable ref targets Pw, the allOf form has Pw as its base), so a consumer that checks both homes sees the redaction. The gap only bites a consumer that reads Property.Secret alone.

I did not find it in a sweep of 157 real-world specs that use format: password: all 259 properties whose type reaches a sensitive node also have secret: true. I would treat this as a consistency fix rather than a data-loss bug.

[fuad-daoud]

Fixed in 291a221. MarkSecretUses now runs once lowering is done and sets Secret on every property and header whose type reaches a Sensitive node: directly, through the nullable target, or along a Base chain. Probed on the branch: nullable oneOf, nullable anyOf, allOf: [{$ref: Pw}] with a description, an alias component, and a nullable response header are all secret: true now. The fixture covers each of them, plus an alias declared after its first use. Removing the MarkSecretUses call reddens the conformance case.

[OmarAlJarrah]

Secret is derived here by re-reading the schema literal (the position's own format plus one referent hop), while Sensitive is derived from the lowered node. The two "homes a consumer reads as the same statement" diverge as soon as a wrapper (nullable union, allOf, alias component) sits between the use and the Scalar; see the other comment on this line.

Deriving Secret from the resolved TypeRef (follow the nullable-ref target and the Base chain, OR in the node's Sensitive) would keep a single derivation and close those gaps without enumerating spellings in FillPropertyDetail.

[OmarAlJarrah]

One more reason to derive Secret from the resolved type. The doc on redactionFormat says the constant is read by both the lowering arm and the per-use carrier, "so the two cannot drift". They can: the two share the predicate but not the schema they apply it to (the position's own format plus one $ref hop, versus the lowered node), which is what the cases in the comment above show. Deriving Secret from the node, or dropping that claim from the comment, would fix it.

[fuad-daoud]

Done in 291a221, with one constraint: the derivation can't happen inside FillPropertyDetail, because a $ref names a component's ID before that component is lowered, so at fill time there is often no node to read. It runs as a walk after lowering, following the nullable target and the Base chain. The fill-time read of the declaration's own format stays, because the allOf fold ORs it across redeclarations, and in the drop case of #782 it is the only record of the losing branch's redaction. The redactionFormat comment now names both writers and no longer claims they cannot drift.

[OmarAlJarrah]

The richer branch used to write format: password; it is removed here and the comment says it "writes no format on purpose". That input is the one that regressed (see the comment on merge.go): a redeclaration whose format hoists a node no longer folds its default, constraints, description and examples, and this test can no longer see it. TestAllOf_RedeclarationOrsSecret only asserts Secret.

Please keep a format-bearing richer branch here, or add a case asserting that default and constraints survive when the redeclaration hoists a node, so the behavior is pinned rather than exempted.

[OmarAlJarrah]

The doc on TestAllOf_RedeclarationOrsSecret says this is the interplay #446 owns. It is not; see the reply on merge.go. The mechanism is tracked in #782. Once that is fixed this fixture can write the format again, and the new case should assert the folded fields as well as Secret.

[fuad-daoud]

The doc on TestAllOf_RedeclarationOrsSecret now cites #782 instead of #446 and says the case should assert the folded fields once that is fixed. The comment in TestAllOf_ReconcileAccumulatesRicherDetailWhateverTheOrder points there too (291a221).

[OmarAlJarrah]

Hoisting a Scalar at a CarriedRef position moves the schema's description, deprecated, xml and examples off Property and onto an anonymous node.

Probe: a model property {type: string, format: password, description: "the pwd", deprecated: true, example: hunter2, xml: {name: foo}}.

• main: Property.Docs.description, Deprecation, XML and Examples are all set.
• this branch: all four are empty or absent on the property and present only on t/anon/.../properties/pw.

It matches the byte and unknown-format controls, but password is by far the most common hoisting format, and an emitter that documents or deprecates a field from Property.Docs/Property.Deprecation loses both for every password field. If the node-per-position carrier stays, FillPropertyDetail could keep these on the property the way it keeps constraints ("Constraints stay unconditional" above).

[OmarAlJarrah]

Retracting the suggested fix here. The diagnosis was right; the remedy is not.

An inline enum, object or unknown-format property already behaves this way on main: description and xml sit on the node the position owns, and Property.Docs is empty. That is the single-reader design from #114 and #116 (attachDeclaredAnnotations runs above the dispatch so no lowering destination can forget them), and the PR body lists the move under accepted behaviour changes. Keeping a second copy on the property in FillPropertyDetail would add back the second reader those issues removed, and would make password the odd one out.

I also checked that nothing is lost. With description, title, example, deprecated, xml, x-*, externalDocs, default and bounds written on a password schema at each of 58 schema positions, every one of them is still in the IR, only relocated. Whether an emitter should read Property.Docs or fall back to its type's docs is an IR-level question (#465 covers the neighbouring override rule), not something to settle in this PR.

[fuad-daoud]

Agreed with the retraction: the annotations stay on the node the position owns, as for every other hoisting position. No change here.

[OmarAlJarrah]

"a Scalar of its own at every scalar position" overclaims. format: password beside enum or const lowers to an Enum/Literal, which has no Scalar to carry Sensitive: the format is kept verbatim in Unmodeled with an info degraded-construct and Sensitive stays false (probed with {type: string, format: password, enum: [a]} and {type: string, format: password, const: abc}). Property.Secret is still set for an inline property, but nothing marks items, additionalProperties, parameter or content positions.

Either state the exception here (and in the PR description's "no source fact is unread"), or carry the redaction on those nodes too.

[OmarAlJarrah]

Two more places this row over-promises. The fixture's header comment says the same ("at every schema position").

• With contentEncoding beside the format, Encoding.Name is base64, not password: encodingName lowers the content encoding, keeps the format verbatim under Unmodeled and reports an info degraded-construct. Sensitive is still set. Checked on the branch with {type: string, format: password, contentEncoding: base64, contentMediaType: text/plain}. This path is new for password and no test pins it.
• The enum and const limit is already tracked: openapi: an enum ignores its format although Enum.ValueType can carry it #607 (an enum ignores its format) and ir: a Literal carries no value type or format #668 (a Literal carries no value type or format). Citing them in the row would make the limit findable.

[fuad-daoud]

Fixed in 291a221. The row now says "at a scalar position" and states both exceptions. Beside a differing contentEncoding, Encoding.Name is that encoding and the format is kept verbatim with a degraded-construct info. Beside enum/const, the position lowers to an Enum or Literal that keeps the format verbatim and is not Sensitive, citing #607 and #668. The fixture header says the same. TestRedactionFormat_BesideContentEncoding and TestRedactionFormat_BesideEnumOrConstIsNotSensitive pin both, the latter as a recorded limit rather than a goal. The PR body's "no source fact is unread" line is corrected as well.

[OmarAlJarrah]

hoistRedactionScalar is hoistFormatScalar plus common.Sensitive = true: same base ref, same scalarEncoding(..., format, ...), same schemaConstraints, same ir.Scalar literal. hoistFormatScalar already takes the base kind and the format verbatim, so this is a fourth near-identical hoister (byte, format, content, redaction).

Setting Sensitive inside hoistFormatScalar when format == redactionFormat, or giving the shared body a small option, would drop one copy and one schemaRecursion entry.

[OmarAlJarrah]

Related to the duplication: nothing pins the one thing this hoister is documented to do. hoistByteScalar's doc says every hoister carries the position's value constraints itself (invariant 2; #147 was this bug for byte and unknown formats), and the content-vocabulary hoister has TestContentVocabulary_KeepsTheBoundsWrittenBesideIt.

I assigned cons = nil in hoistRedactionScalar and every test in ./compilers/openapi/..., ./internal/archtest and ./ir/... stayed green. The fixture writes no bound on a password schema, and neither do the unit tests. A bounds case at a component and at a non-property position (array items, a parameter), like the content-vocabulary one, would catch it. If the hoisters are merged as suggested above, one shared test covers both.

[fuad-daoud]

Merged in 291a221. hoistFormatScalar sets Sensitive when the format is the redaction format, and hoistRedactionScalar and its recursion-allowlist entry are gone. On the bounds: the fixture now writes minLength on the Pw component, maxLength on array items and minLength on a header parameter, and requireLengthBounds asserts each one. Planting cons = nil for a Sensitive Scalar reddens the conformance case.

[OmarAlJarrah]

Two statements here do not hold for every parameter. "as any hoisting format does" is inaccurate, since byte and unknown-format Scalars do not set Sensitive. And "that node is the only carrier here" is only reachable directly when the parameter schema is the redaction schema itself: for $ref to an alias component (Chain: {$ref: Pw}) the parameter's type node is the alias, which has sensitive: false and reaches the Scalar only through Base. Worth saying so here and under #578.

[fuad-daoud]

Rewritten in 291a221. It no longer says "as any hoisting format does". It now says that through a wrapper (an alias component, or an allOf over one $ref) the parameter's own node is not Sensitive, and only the node its Base chain reaches is. #578 is still named as the owner of a parameter-level field.

[OmarAlJarrah]

The ordering fix for TestConformance / TestConformance_TableNamesEveryCorpusSpec is a separate defect with its own repro and verification, unrelated to format: password. Repo guidance is one logical change per PR; this would fit its own PR (or an issue), and it keeps the squashed history about the redaction change only.

[OmarAlJarrah]

The ordering fix also depends on declaration order: the new doc says the reader is "declared after" the writer. On this branch, deleting a golden and running go test ./compilers/openapi -run TestConformance -update -shuffle=on failed with the same "corpus specs and goldens disagree" in 5 of 6 runs; in default order it was green 4 of 4. The gate does not shuffle, so CI is unaffected, but a fix that does not rely on test order (one test that writes and then checks, or a reader that does not read goldens in update mode) would be sturdier. One more reason it belongs in its own PR.

[fuad-daoud]

Agreed, and the -shuffle=on result settles it. Reverted on this branch in c2bef74, so both tests are t.Parallel() again exactly as on main. The race and the order-independent fixes you suggested are in #817, so it can land in a PR of its own.

[OmarAlJarrah]

This row names custom-scalars as the matrix row the case witnesses, but the fixture only writes format: password, a known format that lowers to a redaction scalar rather than a custom scalar. scalar-format already witnesses that row. The matrix reader checks the key exists, not that the spec exercises it, so this over-claims coverage. Consider naming the row actually exercised or leaving the witness list empty with a reason.

[fuad-daoud]

Right, it named a row the spec does not exercise. The witness list is empty now (291a221). The matrix has no redaction row, which is the case conformanceCase's doc allows an empty list for.

[OmarAlJarrah]

Two things at this arm.

1. password with writeOnly, readOnly or default, the usual way to declare a password field, now follows the path in openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750. The position owns a node, so a $ref aimed at the property's schema reports degraded-construct for the keyword only when the reference lowers first. Repro: the openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750 document with the property p replaced by {type: string, format: password, writeOnly: true}. On main, go run ./cmd/morphic-harness reports both orders ok. On this branch, and on this branch merged with current main, it reports both order-dependent on diagnostics, the same as format: hex-token already does on main. It needs a $ref to a property's schema by pointer and only an info diagnostic differs, but it is worth a line on openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750 and in the PR body so the next reader finds it. It is not fixable here without fixing openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750.
2. The comment above says the non-string and union-variant cases "are lossless today only by accident". After this change they are handled on purpose, so "today" describes a state the branch removes. "Keyed on the format so a non-string type and a union variant state the same redaction" says it without the history.

[fuad-daoud]

1. Reproduced: with p: {type: string, format: password, writeOnly: true} in the openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750 document, morphic-harness reports order-dependent on diagnostics on the branch and ok on main. It is now under accepted behaviour changes in the PR body and in a comment on openapi: a $ref to a property's schema reports readOnly as kept on a node that lacks it #750, noting that hex-token already behaves this way.
2. Reworded in 291a221: "Keyed on the format rather than on the string type, so format: password on a non-string type and on a union variant states the same redaction." The test doc that said "lossless only by accident" lost that clause too.

[OmarAlJarrah]

Merge-time notes that are not tied to a line.

• Merge commit 713128d carries a co-author trailer naming an AI assistant and a session-link trailer. The repo asks for no LLM or session artifacts in commit messages, and the squash default here is COMMIT_MESSAGES, which pastes every branch commit body into the squashed commit. The branch cannot be rewritten, so set the squash body explicitly when merging.
• Two branch commit subjects are over the 72-character cap: d7650c9 (77) and ecbeb0b (80). The PR title is 70 characters with (#592) appended, so the squashed subject is within the cap.

[fuad-daoud]

Thanks for the merge-time notes. The squash body will be set explicitly at merge, so neither the merge commit's trailers nor the two long branch subjects (d7650c9, ecbeb0b) reach main; the PR title is the squashed subject and stays within the cap.