diff --git a/specs/010-catalog-backstage/contracts/README.md b/specs/010-catalog-backstage/contracts/README.md index e9540505..a22d8149 100644 --- a/specs/010-catalog-backstage/contracts/README.md +++ b/specs/010-catalog-backstage/contracts/README.md @@ -173,3 +173,74 @@ repeated in each: 5. **ADR-0014 rung 1 only.** Maintainer reference verification MUST NOT be called external, third-party, or community. Only corpus *data* is third-party — never the validation. + +--- + +## §4. Carried-forward contract conflicts, and which side governs + +Feature 010 adopts spike 009's contracts **by reference**. Those files are a frozen +historical record and are **not edited by this feature**. Where a carried-forward +contract conflicts with another carried-forward contract, with an ADR, or with this +feature's own `data-model.md`, the governing reading is fixed here so that no +implementer has to re-derive it — and so that two implementers cannot derive it +differently. + +All four were found by implementation sessions reading the contracts against each +other, not by review of the contracts alone. + +### 4.1 Namespace character class — ADR-0015 governs + +`admissibility.md` §2 previously gave `metadata.namespace` the same character class as +`metadata.name`. **Corrected in this feature's own contract.** ADR-0015 binds +`metadata.namespace` through `CommonValidatorFunctions.isValidDnsLabel`, +`/^[a-z0-9]+(?:\-+[a-z0-9]+)*$/` — no uppercase, no `_`, no `.`. Discriminating +assertion: `validateNamespace('Default') === false`. + +### 4.2 Unrecognized top-level manifest field — `atomic-fail-closed.md` §4 governs + +`input-manifest.md` §1 calls an unrecognized top-level field an +*"unsupported manifest version"-class* rejection. `atomic-fail-closed.md` §4 explicitly +assigns that exact case to **`invalid-manifest-shape`**, and distinguishes it from +`unsupported-manifest-version`, "which presumes the manifest parsed correctly and has +the right shape but declares an unsupported *value*". + +**§4 governs**, as the later and more specific statement. `unsupported-manifest-version` +is reserved for a well-shaped manifest declaring an unsupported `manifestSchemaVersion`. + +### 4.3 Path-validation stage 1 trigger class — `invalid-manifest-shape` + +`input-manifest.md` §4.1 names stage 2's trigger class explicitly +(`incomplete-required-source`, "and the file is never opened") but names **none** for +stage 1, saying only "reject the manifest, non-zero". + +**Stage 1 failures are `invalid-manifest-shape`.** A lexically invalid path is a defect +in the manifest's own content, discovered before any filesystem access — which is +precisely what distinguishes it from stage 2. This resolves a silence rather than +overriding a statement. + +### 4.4 `requiredCapabilities` arity — `input-manifest.md` §2 governs + +`input-manifest.md` §2 defines the rejection precisely: triggered by **any string other +than `"pathOwnership"` appearing in the array**. `data-model.md` §1 types the field as +the one-element tuple `readonly ["pathOwnership"]`, which would additionally reject `[]` +and `["pathOwnership", "pathOwnership"]`. + +**§2's rejection rule governs.** An arity rejection the contract does not authorize must +not be invented, so `[]` and repeated entries do **not** trigger rejection. `data-model.md` +§1's tuple should be read as the expected shape, not as an additional validation rule. + +### 4.5 `isValidDnsSubdomain`'s per-label character class — composed, and flagged + +ADR-0015's table gives `isValidDnsSubdomain`'s **bounds** (≤253 total, each dot-separated +label ≤63) but never states what a label may **contain**. It does give `isValidDnsLabel`'s +predicate, in the namespace row of the same table. + +**Composition:** a subdomain is dot-separated labels, each satisfying the stated +`isValidDnsLabel` predicate, bounded as stated. This is the one place in this feature +where transcription from ADR-0015 was insufficient and an inference was required. It is +recorded here rather than left in source comments, and it reproduces all four facts +ADR-0015 records as *executed* against the pin: a 243-character prefix passes, 254 fails, +an over-63 label fails, and a two-separator value fails. + +This inference is a candidate for an ADR-0015 addendum. Until one exists, it is an +inference and is labelled as one. diff --git a/specs/010-catalog-backstage/contracts/admissibility.md b/specs/010-catalog-backstage/contracts/admissibility.md index 5600931d..7a83b6df 100644 --- a/specs/010-catalog-backstage/contracts/admissibility.md +++ b/specs/010-catalog-backstage/contracts/admissibility.md @@ -53,7 +53,27 @@ ADR-0015 and restated in `spec.md` FR-016: | `apiVersion` | `validateApiVersion` | — | — | | `kind` | `validateKind` | — | — | | `metadata.name` | `validateEntityName` | `[A-Za-z0-9]` plus `-`, `_`, `.` | ≤ 63 characters | -| `metadata.namespace` | `validateNamespace` | `[A-Za-z0-9]` plus `-`, `_`, `.` | ≤ 63 characters | +| `metadata.namespace` | `validateNamespace` | `[a-z0-9]` plus `-` only — **no uppercase, no `_`, no `.`** | ≤ 63 characters | + +> **The two rows are not the same class, and the difference is load-bearing.** +> An earlier revision of this table gave `metadata.namespace` the same class as +> `metadata.name`. That was wrong. +> [ADR-0015](../../../docs/adr/0015-validate-descriptors-against-backstage-field-formats-before-canonicalizing.md) +> binds `metadata.namespace` through +> `KubernetesValidatorFunctions.isValidNamespace` → `CommonValidatorFunctions.isValidDnsLabel`, +> whose predicate is `/^[a-z0-9]+(?:\-+[a-z0-9]+)*$/` — a DNS label, which admits +> no uppercase, no underscore and no dot, and which cannot start or end with a +> hyphen. `metadata.name` goes through `isValidObjectName`, +> `/^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$/`, which admits all three. +> +> The discriminating assertion is `validateNamespace('Default') === false`. An +> implementation written to the earlier summary would silently admit namespaces +> the pinned validator rejects, and would then canonicalize them — exactly the +> defect ADR-0015 exists to prevent. +> +> **ADR-0015's table is authoritative.** Where this summary and that table +> disagree, the table wins and this file is the defect. FR-016 says so directly: +> "exactly the four field validators in ADR-0015's table." A descriptor is **admissible** when all four predicates return true for it. It is **inadmissible** when any one of them returns false. diff --git a/specs/010-catalog-backstage/contracts/package-boundary.md b/specs/010-catalog-backstage/contracts/package-boundary.md index 0d4cd955..365ef767 100644 --- a/specs/010-catalog-backstage/contracts/package-boundary.md +++ b/specs/010-catalog-backstage/contracts/package-boundary.md @@ -125,13 +125,13 @@ no entry anywhere granting the consumer permission to be a non-adapter; it simpl The two guards in `scripts/check-deps.ts` that matter here (all line numbers read in this worktree): -- Lines 175–182: a non-adapter workspace declaring a dependency on an adapter is a violation +- The guard emitting `non-adapter workspace depends on an adapter package`: a non-adapter workspace declaring a dependency on an adapter is a violation with reason `non-adapter workspace depends on an adapter package`. -- Lines 196–204: a package declaring a dependency outside its allowed public surface is a +- The guard emitting ` declares a dependency outside its allowed public surface`: a package declaring a dependency outside its allowed public surface is a violation with reason ` declares a dependency outside its allowed public surface`. **The trap:** `allowedDependenciesFor()` returns `undefined` for any package it has no entry for -(`scripts/check-deps.ts:151`), and the second guard is then skipped entirely. A package with no +(`allowedDependenciesFor()` returns `undefined`), and the second guard is then skipped entirely. A package with no allowlist entry is **silently unconstrained** — it passes `check:deps` no matter what it declares. @@ -218,3 +218,16 @@ Repeated here per `contracts/README.md` §4: 4. **ADR-0014 rung 1 only.** Nothing here is external, third-party, or community validation. 5. **Genuine unknowns are marked.** This contract carries none of its own. The name `@adrkit/catalog-envelope` is a working name, not an unknown (§1). + + +## §5. Citation rule for this contract + +Guards in `scripts/check-deps.ts` are cited **by the reason string they emit**, never +by line number. Line numbers drift with any edit above them — feature 010 Phase A's own +allowlist additions shifted both guards by 41 lines, silently invalidating every citation +here and in `tasks.md` — while the reason strings are stable and are already asserted +verbatim by `scripts/check-deps.test.ts`. + +A line number is a reference that nothing checks. A reason string is a reference the +test suite checks on every run, so a citation that goes stale fails the build rather +than quietly misleading a reader. diff --git a/specs/010-catalog-backstage/data-model.md b/specs/010-catalog-backstage/data-model.md index 8f75b9f5..bbdb09ef 100644 --- a/specs/010-catalog-backstage/data-model.md +++ b/specs/010-catalog-backstage/data-model.md @@ -107,8 +107,10 @@ DescriptorDocument { sourcePath: string documentIndexInFile: integer // 0-based parseOutcome: "parsed" | "duplicate-yaml-key" | "yaml-parse-error" + rawApiVersion: unknown // pre-validation; type not assumed rawKind: unknown // pre-validation; type not assumed rawMetadata: unknown // pre-validation; type not assumed + raw: unknown // the whole document node, for annotation access } ``` @@ -117,10 +119,18 @@ fails to parse for a YAML syntax reason *other than* a duplicate key must not be reported under the duplicate-key outcome. They map to two distinct trigger classes in §8. -**`rawKind` / `rawMetadata` are deliberately `unknown`.** They hold whatever -the YAML node contained, before any admissibility or shape check. Typing them -optimistically would defeat §4, whose whole purpose is to decide whether they -are usable at all. +**`rawApiVersion` is required, not optional.** An earlier revision named only +`rawKind` and `rawMetadata`. A record built to that shape could not be +admissibility-checked at all, because §4 evaluates all four of ADR-0015's +validators and `isValidApiVersion` had no field to read. `raw` is carried for +the same reason at one remove: `adrkit.io/owned-paths` lives under +`metadata.annotations`, and ownership resolution needs the document node rather +than a pre-selected projection of it. + +**`rawApiVersion` / `rawKind` / `rawMetadata` are deliberately `unknown`.** They +hold whatever the YAML node contained, before any admissibility or shape check. +Typing them optimistically would defeat §4, whose whole purpose is to decide +whether they are usable at all. --- @@ -136,9 +146,18 @@ AdmissibilityResult { failedFields: readonly AdmissibilityField[] // empty iff admissible } -AdmissibilityField = "kind" | "metadata.name" | "metadata.namespace" | "spec.type" +AdmissibilityField = "apiVersion" | "kind" | "metadata.name" | "metadata.namespace" ``` +> **This union is exactly ADR-0015's four validator-table rows, and nothing else.** +> An earlier revision omitted `apiVersion` — which ADR-0015 requires and binds +> through `isValidApiVersion` — and added `spec.type`, which no validator in that +> table covers. It was wrong in both directions: a record built to the earlier +> union could not report the field that actually failed when `apiVersion` was +> malformed, and invited an implementation to validate a field the pinned commit +> never validates. FR-016's "exactly the four field validators in ADR-0015's +> table" is the governing wording. + **Ordering rule, and why it is load-bearing.** This check runs **before** canonicalization (§5). ADR-0015's decision is that admissibility is a *precondition of* canonicalization, not a sibling check. The concrete @@ -413,10 +432,37 @@ SnapshotEntityRecord { ownershipState: OwnershipState derivedPaths: readonly string[] sourceDocument: { sourcePath: string, documentIndexInFile: integer } - provenance: string + provenance: AnnotationProvenance } + +AnnotationProvenance = "upstream-authored" | "maintainer-overlay" ``` +**`provenance` describes the ANNOTATION, not the descriptor.** This distinction is the +whole point of the field and is easy to get backwards. Under **ADR-0020 clause 5** the +descriptors are upstream-authored in *both* cases — the clause requires them "authored +upstream and otherwise unmodified" — so a value meaning "the descriptor came from +upstream" would be true always and would distinguish nothing. + +| Value | Meaning | +|---|---| +| `upstream-authored` | The `adrkit.io/owned-paths` annotation was already present in the real upstream descriptor as found. | +| `maintainer-overlay` | The annotation was authored by us and overlaid onto an otherwise-unmodified upstream descriptor. | + +That is what makes **FR-043** satisfiable: clause 5's "only the corpus data is +third-party, never the validation" boundary becomes legible from the artifact itself. + +**It also doubles as a live adoption signal.** Zero third-party descriptors in the pinned +corpora carry the annotation, so every corpus today is `maintainer-overlay`; +`upstream-authored` only becomes reachable if real adoption occurs. The field is not dead +metadata. + +*Decided by the maintainer on 2026-08-05. An earlier revision typed this a bare `string` +with no frozen domain, while consumer validation step 2 required "a recognized +`provenance`" — an unsatisfiable pairing. Phase C correctly implemented non-empty-string +and flagged the gap rather than inventing a vocabulary that would have made the consumer +reject conformant generator output.* + This is the **serialized projection** of the generator's internal entity record, not that record field-for-field. `snapshot-envelope.md` §1 fixes the shape and its rationale: the identity projection carries `{ canonicalId, diff --git a/specs/010-catalog-backstage/spec.md b/specs/010-catalog-backstage/spec.md index 46fbb70a..e0c0b081 100644 --- a/specs/010-catalog-backstage/spec.md +++ b/specs/010-catalog-backstage/spec.md @@ -1108,9 +1108,15 @@ The following are explicitly excluded and MUST NOT be introduced by this feature `explicit-empty`, `annotation-absent`; no two are treated as equivalent; and no path is ever derived for an `annotation-absent` entity. -- **SC-006** *(annotation decode order)*: Each of the annotation's ordered decode/validate steps - produces its own distinct rejection reason when violated in isolation, and a non-string YAML - node is rejected by the string-scalar check **before** any JSON parse is attempted. +- **SC-006** *(annotation decode order)*: The annotation's **five** ordered decode/validate + steps are evaluated in order, and the **three** steps that can reject each produce their own + distinct rejection reason when violated in isolation — `annotation-value-not-a-string` + (step 2), `parse-error` (step 3), `wrong-shape` (step 4). A non-string YAML node is rejected + by the string-scalar check **before** any JSON parse is attempted. Step 1 (presence) does not + reject: an absent annotation is the legitimate `annotation-absent` ownership state. Step 5 + (per-pattern) does not produce an annotation-decode reason: it delegates to the glob dialect, + whose rejection reasons belong to that contract and are covered by SC-007. **Five steps, + three reasons — the counts are deliberately different and must not be conflated.** - **SC-007** *(glob dialect)*: Each of rules 1–14 in [`glob-dialect.md`](../009-catalog-binding-viability/contracts/glob-dialect.md) §3's fifteen diff --git a/specs/010-catalog-backstage/tasks.md b/specs/010-catalog-backstage/tasks.md index cacf1414..6a0b2f5a 100644 --- a/specs/010-catalog-backstage/tasks.md +++ b/specs/010-catalog-backstage/tasks.md @@ -218,11 +218,23 @@ ownership result, or produces an envelope. Phase A may run concurrently with Pha Depends: T001, T002 Contract: `package-boundary.md` §2, §4 -- [X] T009 [US9] **Observed failing.** Introduce a dependency edge from `@adrkit/core` - (then `@adrkit/cli`, then a `schema/`-owning package) onto the adapter; run - `bun run check:deps`; observe the failure and record the exact emitted reason - string; remove the edge; observe the pass. Retain the failing inputs as a - permanent negative case. +- [X] T009 [US9] **Observed failing.** Introduce a dependency edge from `@adrkit/core`, + then `@adrkit/cli`, then the `schema/` surface, onto the adapter; observe the + isolation check fail in each case and record the exact emitted reason string; + remove the edge; observe the pass. Retain the failing inputs as permanent + negative cases. + **The three surfaces are not observed by the same command, and the deposit MUST + name which command produced each failure.** `core` and `cli` fail + `bun run check:deps`. **`schema/` cannot**: it has no `package.json`, so + `readWorkspacePackages()` never visits it and `check:deps` returns exit 0 — and no + change to `check-deps.ts` fixes that without teaching it to scan sources rather + than manifests. That clause is held instead by `bunfig.toml`'s isolated linker, + observed via `bun run typecheck` failing with `TS2307: Cannot find module`. + SC-015 is discharged because it requires "the **isolation check**" to fail, not + `check:deps` specifically — but the deviation must be recorded, not smoothed over. + This is `package-boundary.md` §4's trap one level up: there, a *package* with no + allowlist entry is silently unconstrained; here, a *directory* with no manifest is + invisible outright. Both fail in the direction of a check that cannot fail. Files: `scripts/check-deps.test.ts`, `/negative-cases/dep-core-to-adapter/`. Barrier: BEFORE Discharges: SC-015 @@ -230,7 +242,7 @@ ownership result, or produces an envelope. Phase A may run concurrently with Pha - [X] T010 [US9] **Observed failing.** Add `@adrkit/catalog-backstage` to the consumer's dependencies; run `bun run check:deps`; observe the guard at - `scripts/check-deps.ts:175–182` emit `non-adapter workspace depends on an + `scripts/check-deps.ts` emits `non-adapter workspace depends on an adapter package`; record the exact string; remove; observe the pass. Files: `scripts/check-deps.test.ts`, `/negative-cases/dep-consumer-to-adapter/`. Barrier: BEFORE @@ -240,7 +252,7 @@ ownership result, or produces an envelope. Phase A may run concurrently with Pha - [X] T011 [US9] **Observed failing.** Add `@adrkit/catalog-envelope` to the adapter's dependencies; run `bun run check:deps`; observe the guard at - `scripts/check-deps.ts:196–204` emit ` declares a dependency outside its + `scripts/check-deps.ts` emits ` declares a dependency outside its allowed public surface`; record the exact string; remove; observe the pass. Files: `scripts/check-deps.test.ts`, `/negative-cases/dep-adapter-to-consumer/`. Barrier: BEFORE @@ -250,7 +262,7 @@ ownership result, or produces an envelope. Phase A may run concurrently with Pha - [X] T012 [US9] **Observed failing — closes the silent-unconstrained trap.** `allowedDependenciesFor()` returns `undefined` for any package with no entry - (`scripts/check-deps.ts:151`), and the allowed-surface guard is then skipped + (`allowedDependenciesFor()` returns `undefined`), and the allowed-surface guard is then skipped entirely — so a package with no entry passes `check:deps` no matter what it declares. The only proof T008's entries actually exist is to add a disallowed dependency to each new package and observe a violation. Do so for both packages @@ -277,7 +289,13 @@ with the audit recording its own hashes and its own PASS/FAIL. Phase B may run concurrently with Phase A and with nothing else. - [ ] T013 [US1] Create the tracked evidence tree — `/README.md`, - `/frozen-expectations/`, `/accept-corpus-freeze/`. + `/frozen-expectations/`, `/accept-corpus-freeze/`, and + `/negative-cases/`. + **`negative-cases/` is a SHARED, CROSS-PHASE tree.** Roughly twenty tasks spanning + phases A–G deposit into it, each owning its **own subdirectory** — which is what makes + it safe for concurrent worktree sessions, since different subdirectories never collide + on merge. Do not write an index of it: an enumeration goes stale on the next deposit, + which is the exact failure ADR-0016 exists to prevent. These artifacts must be **git-tracked**: R5 mechanism 2 depends on CI being able to re-derive their hashes, and ADR-0015 Condition of Acceptance 1 requires them to be inspectable in the repository. @@ -423,7 +441,13 @@ concurrently with Phase D. - [X] T027 [P] [US8] Author the envelope fixtures under `/test/fixtures/` — one malformed fixture per validation step (five), plus mutated-payload, stale, - foreign-repository, and valid. + foreign-repository, valid, and the **all-annotation-absent acceptance contrast + case**: **ten** in total. + The tenth is required by `snapshot-envelope.md` §7 row 1b — an otherwise-valid + envelope whose entities are *all* `annotation-absent` with `identityOnly: false`, + which MUST be **accepted**. It is the case step 5's wording exists to protect, and + without it the fixture set contains only rejections, which cannot prove the + validator does not **over**-reject. Barrier: BEFORE Discharges: none — enables FR-045…FR-049 Depends: T002 @@ -743,9 +767,15 @@ slices — **D2** (input boundary), **D1a** (admissibility and identity), **D1b* Contract: `owned-paths-annotation.md` §1 - [ ] T058 [US4] Implement the **five** ordered annotation decode steps at - `/src/ownership/annotation.ts`, each with its own distinct rejection - reason. Observe each of the five failing independently; record five distinct - reason strings; restore; observe the pass. + `/src/ownership/annotation.ts`. **Three** of the five can reject, each with + its own distinct reason: step 2 → `annotation-value-not-a-string`, step 3 → + `parse-error`, step 4 → `wrong-shape`. Observe each of those three failing + independently; record three distinct reason strings; restore; observe the pass. + **Step 1 (presence) does not reject** — an absent annotation is the legitimate + `annotation-absent` ownership state (`owned-paths-annotation.md` §1 step 1). + **Step 5 (per-pattern) does not produce an annotation-decode reason** — it delegates + to the glob dialect, whose reasons belong to that contract and are covered by SC-007. + Five steps, three reasons; do not conflate the counts. Files: `/src/ownership/annotation.ts`, `/test/annotation-decode.test.ts`, `/negative-cases/annotation-decode/`. @@ -783,8 +813,12 @@ slices — **D2** (input boundary), **D1a** (admissibility and identity), **D1b* Discharges: SC-005 Depends: T060 -- [ ] T062 [US4] SC-006 close-out: a consolidated test over the five annotation decode - steps, each rejecting at its own step with its own reason. +- [ ] T062 [US4] SC-006 close-out: a consolidated test over the **five** annotation decode + steps, asserting that the **three** rejecting steps each reject at their own step with + their own reason, and documenting why the other two do not — step 1 yields the + `annotation-absent` state rather than a rejection, and step 5 delegates to the glob + dialect (SC-007). The test MUST assert the five-step *ordering* as well as the three + reasons, so that a reordering which happened to preserve the reasons still fails. Files: `/test/sc-006.test.ts`. Barrier: BEFORE Discharges: SC-006 @@ -1200,6 +1234,14 @@ Phase F completes. scoped to what a pure validator predicate returns at the pinned commit `1121a4facd9e321179d0402c3f355e4a649e84d9`; (vi) only corpus **data** is described as third-party; the validation never is. + **The check MUST match claims, not vocabulary.** ADR-0014's terms are binding, so + the maximally honest phrasing — "**not** `reference-verified` (rung 2), **not** + `externally validated` (rung 3)" — necessarily contains the very strings a naive + grep would flag. A check that fails on bare occurrence punishes the documentation + that is being most honest and rewards silence. Match assertion patterns (a claim + *of* the status) and treat negations and prohibitions as conformant, then verify + the check itself on both a real claim and a real denial before trusting it + (ADR-0016). Files: `/honesty-close-out.md`, `scripts/check-honesty-close-out.test.ts`. Barrier: BEHIND