diff --git a/docs/adr/0079-record-display-name.md b/docs/adr/0079-record-display-name.md new file mode 100644 index 0000000000..c3edf88f36 --- /dev/null +++ b/docs/adr/0079-record-display-name.md @@ -0,0 +1,337 @@ +# ADR-0079: The record display-name contract — `nameField` is the one canonical primary-title pointer + +- **Status**: Accepted — implemented. **This file is a RETROACTIVE RECONSTRUCTION written 2026-08-08**, not a contemporaneous record; see [Provenance](#provenance-read-this-before-citing-this-file) before you cite it. +- **Decided**: 2026-06-28 (the decision), reconstructed 2026-08-08 (this file) +- **Reconstructed by**: #6634 +- **Implemented by**: [#2434](https://github.com/objectstack-ai/objectstack/pull/2434) (foundation — `nameField` + resolver), [#2458](https://github.com/objectstack-ai/objectstack/pull/2458) (designate-only provisioning at the registry seam), [#2463](https://github.com/objectstack-ai/objectstack/pull/2463) (the author-time gate) +- **Builds on**: [ADR-0078](./0078-no-silently-inert-metadata.md) (author-time completeness must not be cloud-only), [ADR-0061](./0061-record-search-architecture.md) (`$search` field resolution reads the display field) +- **Precedent for**: [ADR-0085](./0085-object-semantic-roles-over-surface-hint-blocks.md) (`compactLayout` → `highlightFields` follows this ADR's alias mechanics "exactly"), [ADR-0098](./0098-pinyin-search-companion-column.md) (the search companion column is fed by this ADR's resolved display field) +- **Consumers**: `@objectstack/spec` (`data/display-name.ts`, `data/object.zod.ts`, `data/search-fields.ts`, `ai/solution-blueprint.zod.ts`), `@objectstack/objectql` (registry materialization seam, `$search` expansion, search companion), `@objectstack/metadata-protocol` (REST `$searchFields` ingress gate), `@objectstack/lint` (`validate-record-title`, `data-model-rules` R9), `@objectstack/platform-objects` and the first-party plugins/services (41 explicit `nameField:` designations), the `objectstack-data` / `objectstack-formula` skills + +--- + +## Provenance — read this before citing this file + +**This file was written on 2026-08-08, roughly six weeks after the decision it +records, by reading the code that cites it.** It is a reconstruction. Treat its +*emphasis and phrasing* as this author's, and its *content* as evidence-backed +but second-hand. + +Three facts make that disclosure necessary rather than decorative: + +1. **The decision is real and contemporaneous; only this file is late.** It was + taken on 2026-06-28 and implemented across three PRs in two days + (#2434 / #2458 / #2463). Nothing here is being decided now. + +2. **A contemporaneous record exists — in a different repository.** #2434's + own description ends: *"ADR: `objectstack-ai/cloud` + `docs/adr/0079-record-display-name.md`."* The decision record was authored + in the sibling `cloud` repo while every consumer of the decision was built + here. That is why `docs/adr/0079-*` has never existed in this repo's + history — not a loss, a **cross-repo split**. This file deliberately reuses + the original's slug (`record-display-name`) so the two are recognisably one + record. + + ⚠️ **The reconstruction could not be checked against that original.** The + `cloud` repo was not reachable from the session that wrote this file. Where + this document and the cloud original differ, **the cloud original is the + decision** and this file is the bug — say so in an issue and this file gets + corrected. + +3. **Why write it here at all, rather than a pointer.** 77 files in *this* + repo cite `ADR-0079` (measurement in #6634), and every other `ADR-NNNN` they + cite resolves to `docs/adr/` *here*. A reader who follows the convention + lands on nothing. A pointer to a repo most readers of this one cannot open + is not an improvement on nothing; the decision has to be *readable* at the + end of the citation. + +**What is reconstructed vs. what is quoted.** Every clause in +[Decision](#decision) is traceable to a citing site, quoted or cited by +`file:line`. The [Context](#context) section is inference from the +implementation and the three PR descriptions. [What this +reconstruction does NOT settle](#what-this-reconstruction-does-not-settle) is +the honest residue: places where the 77 citing sites are silent, or where they +use one word for two different states. Those are **not** decided here. + +--- + +## Context + +A record's human name — what shows on a card, a lookup chip, a breadcrumb, an +approval notification, a search result — was, before this decision, a +render-time guess. Two object-level keys competed (`displayNameField`, a +render-only `titleFormat` template), neither was required, and consumers each +re-derived a title from whatever they found. + +That is the ADR-0078 failure shape applied to identity: an object with no +resolvable title parses, "renders", and reports success — and every record it +holds is anonymous. The cost is asymmetric for an AI author, which is the +argument ADR-0078 makes in general and which lands hardest here, because the +symptom (records displaying as raw IDs) appears only in a UI a build never +opens. + +The specific defects the implementation names: + +- **`titleFormat` is unqueryable by construction.** It is a render-only + template. `packages/lint/src/validate-record-title.ts:92` states the + consequence: *"titleFormat is a render-only template the server cannot return + or query"*. A title the server cannot return cannot be sorted on, searched, + or sent in a notification body — so a title expressed only as a template is + not a title, it is a client-side decoration. +- **A guessed title cannot be relied on by the layers underneath the UI.** + `$search` expansion, the REST `$searchFields` ingress gate, the pinyin search + companion column, and approval/notification display enrichment all need *one* + answer to "what is this record called", computed the same way at every seam. +- **Two spellings, no canon.** `displayNameField` existed; nothing said it was + the authority, and nothing stopped a third spelling appearing. + +--- + +## Decision + +### D1 — A record's title is a structural invariant, not a render-time hint + +Every object has exactly **one** primary title field, and it is a **real stored +field** — `text`-ish, `autonumber`, or a `formula` whose result type is `text`. +Not a template, not a client-side composition. + +> "A record's human title is a STRUCTURAL INVARIANT: every object has exactly +> one primary title field, which is a real STORED field (text / autonumber / +> formula whose result is text)." +> — `packages/spec/src/data/display-name.ts:6-8` + +### D2 — `nameField` is canonical; `displayNameField` is a deprecated alias + +`nameField` is the object-level pointer to the primary title field. +`displayNameField` is **accepted as a parse-time alias**, never as a second +contract: + +- `ObjectSchema.parse` / `.safeParse` / `.create()` copy `displayNameField` onto + `nameField` when `nameField` is absent + (`normalizeNameFieldAlias`, `packages/spec/src/data/object.zod.ts:2082-2095`, + installed on the parse path at `:2149-2158` and reached by `create()` at + `:2204`). +- **Both keys are preserved on the parsed output** — deliberately, for + cross-repo consumers and older tests that still read the old spelling + (`object.zod.ts:2084-2085`). +- The schema's own text marks the direction: `displayNameField` describes + itself as `[DEPRECATED → nameField]` (`object.zod.ts:1698`). + +This is the alias mechanic ADR-0085 later adopted verbatim: *"Mechanics follow +ADR-0079's `displayNameField → nameField` precedent exactly: `compactLayout` is +accepted as a parse-time alias, copied onto `highlightFields`, both preserved on +output, describe marks the old key deprecated."* +(`docs/adr/0085-object-semantic-roles-over-surface-hint-blocks.md:57`) + +### D3 — `titleFormat` is retired in favour of `nameField` + +`titleFormat` is deprecated and lint-warned, and an explicit `nameField` takes +precedence over it. It **still parses** — existing metadata keeps loading — so +the diagnostic is advisory, not an error: + +> "`title-format-retired` — flags an object that declares a `titleFormat`. That +> key is a render-only template the server can neither return nor query; +> ADR-0079 retires it in favour of `nameField`. The schema still parses it +> (existing metadata keeps loading), so this is advisory, not an error." +> — the #2463 changeset (`.changeset/adr-0079-record-title-gate.md`, since +> consumed by the release) + +**Migration is stated, not left to the author**: a single-field title becomes +`nameField: ''`; a **composite** title becomes a `formula` field with +`returnType: 'text'`, designated as the `nameField` +(`validate-record-title.ts:91-96`; the worked example is +`content/docs/data-modeling/formulas.mdx:94-102`). + +### D4 — One resolution order, shared by every consumer + + nameField ?? displayNameField ?? deterministic derivation + +An **explicit pointer is honored even when the field it names is not +title-eligible** — the author asserted it, and eligibility gates *derivation* +only (`display-name.ts:resolveDisplayField`). + +Derivation, restricted to title-eligible fields, is ranked: + +1. name-ish **exact**, in priority order — `name` > `title` > `subject` > + `label` > `full_name` > `display_name` (so `name` beats `title` regardless of + declaration order); +2. name-ish **affix** — `*_name` / `*_title` / `name_*`, by declaration order; +3. the **first** title-eligible field by declaration order. + +The two runtime seams that consume it compute it identically, and each says so +where it does: + +- `packages/objectql/src/engine.ts:5212` — *"[ADR-0079] `nameField` is the + canonical primary-title pointer; `displayNameField` is the deprecated alias + (still honored)"*, feeding `expandSearchToFilter`. +- `packages/metadata-protocol/src/protocol.ts:4793` — *"[ADR-0079] Same + precedence the engine's search expansion applies"*, feeding the REST + `$searchFields` ingress gate. + +That duplication is intentional and is the point: **one precedence, asserted at +both seams, so the request gate and the query engine cannot drift.** + +### D5 — Title eligibility is a fail-closed allowlist + +Eligible: `text`, `textarea`, `email`, `url`, `markdown`, `html`, `richtext`, +plus `formula` when its result type (`returnType`, or `valueType` for +cross-repo compatibility) is `text`. Everything else is ineligible, and an +**unknown/new field type is ineligible by default** — a positive allowlist, so +a field type added later cannot silently become a title +(`display-name.ts:TITLE_ELIGIBLE_TYPES` / `isTitleEligible`). + +Two judgement calls are recorded rather than left implicit: + +- **`email` is eligible, `phone` is not.** *"`phone` is deliberately excluded (a + phone number is not a title); `email` IS eligible (commonly the human handle + on identity-ish objects)"* (`display-name.ts:71-72`). +- **`autonumber` is a valid primary but is never *derived*.** *"an autonumber is + a valid primary only when an author points at it explicitly … not something we + silently pick"* (`display-name.ts:66-68`). + +### D6 — A record never renders as "Untitled"; the floor is `Record #` + +`resolveRecordDisplayName` returns the value at the resolved field, and falls +back to a stable `Record #` — *"NEVER a bare 'Untitled'"* +(`display-name.ts:13-14`). A view may override the object's choice for one +render via `viewTitleField` (e.g. a list view labelling rows by another column); +that override is per-render and does not change the object's title. + +### D7 — Provisioning is designate-only at the materialization seam + +`SchemaRegistry.registerObject` runs `provisionPrimary(schema, { synthesize: +false })` — for **owned** objects only, after `applySystemFields` +(`packages/objectql/src/registry.ts:1079-1090`). + +- Where a title-eligible field already exists, `nameField` is **designated** — + so it is reliably populated for normal / user-built / AI-built objects. +- Where nothing is eligible, the object is left **exactly as-is**. No `name` + column is synthesized here, because that is a schema migration on dozens of + title-less system tables. +- **Extensions must not redesignate the owner's title** — hence owned-only. + +The `synthesize: true` half of `provisionPrimary` exists and guarantees a +primary by adding a `name` text field, but is deliberately **not** wired at this +seam (#2434's "Staged (deliberate)" section, kept as the `TODO(ADR-0079)` that +#2458 then resolved in the designate-only direction). + +### D8 — The author-time gate is advisory by design + +`@objectstack/lint`'s `validate-record-title` reports two warnings — +`title-format-retired` (D3) and `title-unresolvable` (`objectTitleCompleteness` +returns `status: 'none'`) — and **never errors**: + +> "Both are warnings: the auto-provision transform and the id floor mean a green +> build never ships a fully title-less object." +> — `packages/lint/src/validate-record-title.ts:26-27` + +It runs on `os build` / `os validate` / `os lint`, the MCP authoring surface and +hand authoring — *not* only on the cloud graph-lint path. That is ADR-0078's +"not cloud-only" principle applied +(`validate-record-title.ts:12-14`, `packages/lint/src/authoring-rules.ts:668-678`). + +### D9 — There is exactly one title pointer; a second is not a tolerable alias + +`primaryField` was read as a title pointer by two lint rules and was **removed**, +not declared, in #6326. The reasoning is recorded at +`packages/lint/src/data-model-rules.ts:404-409`: + +> "The maintainer ruled remove, not declare: `nameField` is ADR-0079's one +> canonical title pointer and a second parallel pointer contradicts 'one Zod +> source per metadata type' (Prime Directive #7). Do not reintroduce it as a +> tolerated alias — a consumer-side `??` for a key the producer rejects is +> exactly the second de-facto contract Prime Directive #12 bans." + +Note the asymmetry with D2, because it is the whole distinction: a **producer- +side, parse-time, both-keys-preserved** alias with a stated deprecation +(`displayNameField`) is the sanctioned migration mechanic; a **consumer-side +`??`** for a key the schema rejects (`primaryField`) is a banned second +contract. Same-looking code, opposite verdicts. + +### D10 — Downstream: the name field leads the search set by ORDERING only + +Search-adjacent consumers read the resolved display field, but it does not +buy the field an exemption from their own exclusion rules. `$search` field +resolution leads with the display field **as ordering, never as membership** +(`packages/spec/src/data/search-fields.ts:75-92`, #4483) — the concrete failure +that forced the distinction being D7's designate-only pass setting `nameField: +'id'` on tables whose only textual column is the primary key, which had turned +`$search` into a substring scan over the primary key. The ADR-0098 pinyin +companion column likewise takes *only* the resolved display field as its source +(`packages/objectql/src/search-companion.ts:19`, `:104`). + +--- + +## Status of the surface today (2026-08-08) + +| Key | Spec status | Ledger (`packages/spec/liveness/object.json`) | +|:---|:---|:---| +| `nameField` | canonical, `.optional()` | `live` — "ADR-0079 canonical record-title pointer" | +| `displayNameField` | deprecated alias, still parsed and preserved | `live` — "still read by objectui RecordDetailView + `resolveDisplayField` back-compat" | +| `titleFormat` | deprecated, still parsed, lint-warned | `live` — "objectui (`{{record.field}}` interpolation)" | + +41 first-party objects across `platform-objects`, the plugins and the services +carry an explicit `nameField:` with an `[ADR-0079]` comment. + +--- + +## What this reconstruction does NOT settle + +These are places where the 77 citing sites are **silent, or use one word for two +states**. They are recorded as open, not resolved by this file. Deciding any of +them needs a maintainer, and — for the first three — probably the cloud original. + +1. **When, if ever, does `nameField` become required?** #2434 recorded "No hard + `.refine()` requiring a title (would reject existing metadata)" and + `object.zod.ts:1684-1685` still says *"Optional at the schema level for now + (a hard required-refine is staged)"*. Two months on, "staged" names no + trigger and no criterion. Nothing in the tree says what would make it fire. + +2. **When is the `displayNameField` alias retired?** Its own precedent argues + for a deadline and does not have one: ADR-0085 copied this alias mechanic for + `compactLayout`, ran it for *"one deprecation window"*, and retired it in + framework#2536. `displayNameField` has run since 2026-06-28 with no window + declared, and the ledger records a live cross-repo reader (objectui + `RecordDetailView`), so retiring it is a cross-repo change, not a local one. + +3. **"Retired" is used for a key that is still live.** D3's citations call + `titleFormat` *retired*; the liveness ledger records it `"status": "live"`, + the schema parses it, and objectui still interpolates it. Under ADR-0049's + enforce-or-remove vocabulary "retired" normally means *gone*. Here it means + *deprecated, lint-warned, still parsed, still read cross-repo*. The citations + are consistent with each other about the behaviour and inconsistent about the + word. + +4. **`code` is name-like to lint and not to spec.** `packages/lint/src/data- + model-rules.ts:36`'s `NAME_LIKE_FIELDS` includes `code`; spec's derivation + set (`display-name.ts:NAME_ISH_EXACT`) does not. So an object whose only + name-ish field is `code` passes lint's R9 "has a title face" check while + `resolveDisplayField` will not derive `code` as its title (it may still be + picked by tier 3, as the first title-eligible field — but by a different rule + and a different priority). Whether the two sets are meant to be the same set + is not stated anywhere; this reconstruction does not assume they are. + +--- + +## Alternatives considered + +Reconstructed from what the implementation rejected; not an exhaustive record of +the 2026-06-28 discussion. + +- **Keep `titleFormat` as the composite-title mechanism.** Rejected: a template + the server can neither return nor query cannot be sorted, searched, or put in + a notification. Composite titles are expressible without it, as a text formula + field designated `nameField` (D3), which the server *can* return and query. +- **Require `nameField` immediately (hard `.refine()`).** Rejected as staging, + not on the merits — it would reject metadata that already exists (#2434). See + open question 1. +- **Synthesize a `name` column wherever no title is derivable.** Rejected at the + registry seam: it is a DB migration on every title-less system table. The + capability is kept behind `provisionPrimary`'s `synthesize` option for + authoring-time use (D7). +- **Let each consumer derive a title for itself.** Rejected implicitly by + building one shared pure module and having both search seams assert they use + the same precedence (D4). The alternative is drift that shows up as a request + gate and a query engine disagreeing about what is searchable. +- **Accept `primaryField` as an additional tolerated alias.** Rejected + explicitly by the maintainer in #6326 (D9). diff --git a/scripts/check-adr-anchors.mjs b/scripts/check-adr-anchors.mjs index 4285176fc3..1356aa1a20 100644 --- a/scripts/check-adr-anchors.mjs +++ b/scripts/check-adr-anchors.mjs @@ -73,6 +73,35 @@ // by v4`), and the navigation damage a shared number does is the same whatever // they say — a grep does not read status either. // +// ## The third thing it checks: a cited ADR number RESOLVES to a record (#6634) +// +// The two checks above only ever look at ids listed in `adr-anchors.json` — 30-odd +// ids, hand-registered. Every other `ADR-NNNN` written in a comment, a doc or a +// changeset was unchecked, and the gap is not theoretical: `ADR-0079` was cited +// by **77 files** in this repo, 14 times inside `packages/spec` alone, while +// `docs/adr/0079-*` had never existed here at all. Nothing was wrong with what +// those 77 files SAID — the display-name contract they describe is the one the +// code implements — but a reader following PD #13's "grep the ADRs for the +// surface you are touching" reached nothing, and the number was silently +// squatted: a future unrelated ADR-0079 would have retroactively falsified all +// 77 citations at once. It survived the repo growing to 120 distinct cited +// numbers because no check ever asked the question. +// +// So: every `ADR-NNNN` in a tracked file must name a record under `docs/adr/`. +// Two things are deliberately NOT failures, for opposite reasons: +// +// - **Another repo's registry, named as such.** `ObjectUI ADR-0001` is +// objectui's first decision, not ours, and `docs/adr/` is not its home. This +// is handled STRUCTURALLY (a repo qualifier immediately before the id), like +// the `.vN` version rule above and unlike an allowlist — an author who cites +// a sibling repo has a spelling that is both correct to a reader and clean +// to the gate. Bare `ADR-0001`, meaning ours, still fails. +// - **A record that was withdrawn or deleted, cited as history.** ADR-0107 was +// withdrawn before it landed (#3735) and is cited by the changeset that +// withdrew it. Those numbers sit on `UNRESOLVED_ADR_CITATIONS` below — an +// explicit, shrink-only allowlist, audited in both directions like the +// collision list. +// // ## Adding an entry // // Add one when an accepted ADR's decision is realized in code that would look @@ -84,6 +113,7 @@ // node scripts/check-adr-anchors.mjs // node scripts/check-adr-anchors.mjs --self-test # verify the checker itself +import { execFileSync } from 'node:child_process'; import { readFileSync, readdirSync, existsSync } from 'node:fs'; import { join } from 'node:path'; @@ -116,6 +146,69 @@ const ADR_FILENAME = /^(\d{4})-([a-z0-9]+(?:-[a-z0-9]+)*)(?:\.v(\d+))?\.md$/; */ const NON_RECORD_FILES = new Set(['PRIORITIZATION.md']); +/** + * An ADR citation as written in prose or a code comment, with the word before it + * (if any) captured so a SIBLING REPO's registry can be recognised. + * + * `ADR-0090` → { qualifier: '', number: '0090' } + * `(ObjectUI ADR-0001)` → { qualifier: 'ObjectUI', number: '0001' } + * `see ADR-0090` → { qualifier: 'see', number: '0090' } + * + * Only qualifiers in {@link CROSS_REPO_QUALIFIERS} exempt a citation; every other + * preceding word (`see`, `cf.`, `per`) is noise and the id must still resolve. + */ +const ADR_CITATION = /(?:([A-Za-z0-9_./-]+)[ \t]+)?ADR-(\d{4})/g; + +/** + * Words that, immediately before an id, say "this number belongs to ANOTHER + * repository's decision registry". The two siblings this repo is developed + * alongside (see AGENTS.md); matched case-insensitively, and `objectstack-ai/x` + * counts as `x`. + * + * This is a structural escape hatch, not an allowlist: it is available to every + * future citation, it keeps the sentence honest for a human reader, and it + * cannot hide a BARE id that was meant to be ours. + */ +const CROSS_REPO_QUALIFIERS = new Set(['objectui', 'object-ui', 'cloud']); + +/** + * ⛔ SHRINK-ONLY. ADR numbers that are cited in this repo but have no record + * under `docs/adr/`, and legitimately so: the record was **withdrawn** or + * **deleted**, and the citations are history discussing that fact. + * + * **Adding an entry is not the fix for a red build.** If this gate just told you + * that the id you wrote resolves to nothing, the id is wrong or the record is + * missing — write the record, or cite the number that exists. Widening this list + * re-opens #6634 (77 files citing a decision no reader could reach) for every + * future reader of that number rather than for you once. + * + * Removing an entry is always welcome and the gate enforces it in BOTH + * directions: an entry whose number gains a record, or that nothing cites any + * more, fails as stale — so a number cannot be quietly re-used under cover of + * its own grandfather clause, which is the squatting half of #6634. + */ +const UNRESOLVED_ADR_CITATIONS = [ + { + number: '0001', + // Deleted 2026-02-11 (9da8e3e72) together with 0002-database-driven-metadata- + // storage.md and docs/adr/README.md, in the permission-protocol rewrite. + // Cited as history by `docs/adr/0002-...md` ("already discarded in v3.4's + // ADR-0001"). ⚠️ ARCHITECTURE.md still carries a markdown LINK to the deleted + // path — a genuinely broken pointer, not history, filed separately from + // #6634; this entry keeps the gate honest about the number, it does not + // bless that link. + why: 'record deleted 2026-02-11 (9da8e3e72); cited as history by ADR-0002', + }, + { + number: '0107', + // Withdrawn before it landed: #3700 was closed as not planned and 3bb382b67 + // (#3735) deleted the record. Cited by the changeset that withdrew it, by + // the audit whose D4 it recorded, and by this file's own comment above — + // all three discussing the withdrawal itself. + why: 'record withdrawn 2026-07-28 (#3735, 3bb382b67); cited by the withdrawal changeset and audit', + }, +]; + /** * ⛔ SHRINK-ONLY. The number collisions that already existed when this audit * was written (#5992). Every entry is a navigation tax being paid daily, kept @@ -268,6 +361,145 @@ function auditAdrDirectory(filenames, allowlist) { return { records, errors }; } +/** + * Parse every ADR citation out of one file's text. + * + * Pure over a string so the self-test can drive it with fixtures instead of + * planting files in the tree. + * + * @param {string} file path, used only in the returned rows + * @param {string} text + * @returns {{ file: string, number: string, qualifier: string }[]} + */ +function citationsIn(file, text) { + const out = []; + for (const m of text.matchAll(ADR_CITATION)) { + const raw = (m[1] ?? '').toLowerCase(); + // `objectstack-ai/cloud` qualifies as `cloud`; trailing punctuation dropped. + const qualifier = raw.replace(/^.*\//, '').replace(/[^a-z0-9-]+$/, ''); + out.push({ file, number: m[2], qualifier }); + } + return out; +} + +/** + * Audit that every cited ADR number names a record — the #6634 check. + * + * Pure over the citation rows and the record set, for the same reason + * {@link auditAdrDirectory} is pure over a filename list: the red paths below + * are exercised by the REAL function in `--self-test`, not by an imitation. + * + * @param {{ file: string, number: string, qualifier: string }[]} citations + * @param {Set} records numbers that name a real record + * @param {{ number: string, why: string }[]} allowlist + * @returns {string[]} errors + */ +function auditCitedNumbers(citations, records, allowlist) { + const errors = []; + const allowed = new Map(allowlist.map((e) => [e.number, e])); + /** number → sorted files citing it, for numbers that resolve to nothing. */ + const danglingFiles = new Map(); + + for (const { file, number, qualifier } of citations) { + if (CROSS_REPO_QUALIFIERS.has(qualifier)) continue; // another repo's registry + if (records.has(number)) continue; + if (!danglingFiles.has(number)) danglingFiles.set(number, new Set()); + danglingFiles.get(number).add(file); + } + + const nextFree = String(Math.max(0, ...[...records].map(Number)) + 1).padStart(4, '0'); + + for (const number of [...danglingFiles.keys()].sort()) { + if (allowed.has(number)) continue; + const files = [...danglingFiles.get(number)].sort(); + const shown = files.slice(0, 8); + errors.push( + `ADR-${number} is cited by ${files.length} file(s) but names no record under ${ADR_DIR}/ —\n` + + shown.map((f) => ` ${f}`).join('\n') + + (files.length > shown.length ? `\n … and ${files.length - shown.length} more` : '') + + '\n A citation is a promise that the decision is readable at the other end. PD #13 tells the ' + + 'next author to grep the ADRs for the id they found in the code; an id that resolves to nothing ' + + 'costs them the search and teaches them the citation was decoration.\n' + + ' It is also a squat: whoever later writes a real ADR-' + + `${number} retroactively falsifies all ${files.length} of those citation(s) at once (#6634, where ` + + 'one number had accumulated 77 of them).\n' + + ` Fix, in order of preference: (a) write the record — if the decision is real, ${ADR_DIR}/${number}-` + + `.md; (b) cite the number that exists; (c) if it is a SIBLING repo's decision, say so — ` + + `\`ObjectUI ADR-${number}\` / \`cloud ADR-${number}\` is recognised and skipped. A brand-new record ` + + `takes the next free number — ${nextFree} — not this one.`, + ); + } + + for (const { number, why } of allowlist) { + if (records.has(number)) { + errors.push( + `${SELF_PATH}: the UNRESOLVED_ADR_CITATIONS entry for ${number} is stale — ${ADR_DIR}/ now HAS a ` + + `record for that number (the entry said: ${why}).\n` + + ' Delete the entry, and check the existing citations still mean what the new record says: a ' + + 'number that gained a record after being cited as a dead one is exactly the squat #6634 was about.', + ); + } else if (!danglingFiles.has(number)) { + errors.push( + `${SELF_PATH}: the UNRESOLVED_ADR_CITATIONS entry for ${number} is stale — nothing cites ADR-${number} ` + + `any more (the entry said: ${why}).\n` + + ' Delete the entry. The allowlist is shrink-only: a grandfather clause outliving its citations ' + + 'silently re-licences the number.', + ); + } + } + + return errors; +} + +/** + * Every ADR citation in the repo's TRACKED text files. + * + * `git grep` rather than a directory walk: it is the tool that already knows + * what is tracked, what is ignored (`node_modules/`, `dist/`) and what is + * binary, and getting any of those three wrong is how a repo-wide scan becomes + * either slow or wrong. Collection is deliberately the only impure part — the + * judgement lives in {@link auditCitedNumbers}, which is pure and self-tested. + * + * @returns {{ file: string, number: string, qualifier: string }[]} + */ +function collectCitations() { + let out; + try { + // -I text files only · -o just the matches · -h no filename... except we + // need it, so: --no-color -n gives `file:line:match`, one per match. + // `--untracked` so a NEW file citing a nonexistent record goes red before it + // is committed, not after CI has it. Ignored paths (`node_modules/`, `dist/`) + // stay excluded either way. + out = execFileSync('git', ['grep', '--untracked', '-IonE', '([A-Za-z0-9_./-]+[ \t]+)?ADR-[0-9]{4}'], { + cwd: ROOT, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + } catch (e) { + // git grep exits 1 with no output when nothing matches — a repo with zero + // ADR citations is odd but not an error. + if (e && e.status === 1 && !e.stdout) return []; + console.error( + `check-adr-anchors: cannot scan for ADR citations — ${e && e.message ? e.message : e}\n` + + ' This check reads the tracked tree via `git grep`, so it must run inside the repo checkout.', + ); + process.exit(1); + } + + const citations = []; + for (const row of out.split('\n')) { + if (!row) continue; + // `path:lineno:matchtext` — the path may itself contain ':' on no platform + // we support, but the first two fields are known-shaped, so split from the + // left exactly twice. + const first = row.indexOf(':'); + const second = row.indexOf(':', first + 1); + if (first < 0 || second < 0) continue; + citations.push(...citationsIn(row.slice(0, first), row.slice(second + 1))); + } + return citations; +} + /** Anchor ids whose number names two decisions — reported, never failed. */ function ambiguousAnchorRefs(anchorList, allowlist) { const ambiguous = new Set(allowlist.map((e) => e.number)); @@ -297,6 +529,11 @@ const { records, errors: numberErrors } = auditAdrDirectory(adrFiles, KNOWN_NUMB const errors = [...numberErrors]; let checked = 0; +// #6634 — every `ADR-NNNN` written anywhere in the tracked tree resolves to a +// record, not just the ~30 ids registered in adr-anchors.json. +const citations = collectCitations(); +errors.push(...auditCitedNumbers(citations, records, UNRESOLVED_ADR_CITATIONS)); + for (const entry of anchors) { const { file, adrs, invariant } = entry ?? {}; @@ -357,7 +594,8 @@ if (errors.length) { } console.log( `check-adr-anchors: OK (${checked} anchored file(s), every governing ADR still referenced; ` + - `${records.size} decision number(s), each naming one decision or an allowlisted pair).`, + `${records.size} decision number(s), each naming one decision or an allowlisted pair; ` + + `${citations.length} citation(s) across ${new Set(citations.map((c) => c.file)).size} file(s) resolve).`, ); // Reported, never failed. These anchors are correct — the number they name is @@ -487,6 +725,113 @@ function selfTest() { ); } + // ── Cited numbers resolve (#6634) ──────────────────────────────────────── + // + // ⚠️ Fixture ids are BUILT, never written literally: this file is itself in + // the tracked tree the real scan reads, so a literal `ADR-` + four digits in + // a fixture would be collected as a genuine citation and fail the gate it is + // testing. `id('0202')` keeps the token out of the source. + { + const id = (n) => 'ADR-' + n; + const RECORDS = new Set(['0090', '0107']); + const rows = (text, file = 'src/x.ts') => citationsIn(file, text); + const cited = auditCitedNumbers; + + { + const e = cited(rows(`see ${id('0090')} for why`), RECORDS, []); + assert('citation-to-a-real-record-is-green', e.length === 0, `expected no errors, got:\n${joined(e)}`); + } + + { + const parsed = rows(`(ObjectUI ${id('0202')}) and bare ${id('0203')}`); + assert( + 'citation-parser-reads-the-qualifier', + parsed.length === 2 && parsed[0].qualifier === 'objectui' && parsed[1].qualifier === 'bare', + `expected [objectui, bare], got ${JSON.stringify(parsed)}`, + ); + assert( + 'citation-parser-reads-the-number', + parsed[0].number === '0202' && parsed[1].number === '0203', + `expected [0202, 0203], got ${JSON.stringify(parsed)}`, + ); + } + + { + // The whole point: an id nobody can follow. + const e = cited(rows(`governed by ${id('0202')}`, 'packages/spec/src/a.ts'), RECORDS, []); + assert('dangling-citation-is-red', e.length === 1, `expected exactly 1 error, got ${e.length}:\n${joined(e)}`); + const msg = joined(e); + assert('dangling-message-names-the-number', msg.includes('0202'), `message lacks the number:\n${msg}`); + assert( + 'dangling-message-names-the-citing-file', + msg.includes('packages/spec/src/a.ts'), + `message must point at the file to edit:\n${msg}`, + ); + assert( + 'dangling-message-names-the-next-free-number', + msg.includes('0108'), + `message must offer the next free number for a NEW record:\n${msg}`, + ); + assert( + 'dangling-message-offers-the-cross-repo-spelling', + /ObjectUI ADR-|cloud ADR-/.test(msg), + `an author citing a sibling repo must be told the recognised spelling:\n${msg}`, + ); + assert( + 'dangling-message-does-not-invite-allowlisting', + !/add .*UNRESOLVED_ADR_CITATIONS/i.test(msg), + `the remedy must be "write the record / cite a real one", never "widen the allowlist":\n${msg}`, + ); + } + + { + const e = cited(rows(`(ObjectUI ${id('0202')})`), RECORDS, []); + assert('cross-repo-qualified-citation-is-skipped', e.length === 0, `expected no errors, got:\n${joined(e)}`); + } + + { + const e = cited(rows(`objectstack-ai/cloud ${id('0202')}`), RECORDS, []); + assert('org-qualified-sibling-repo-is-skipped', e.length === 0, `expected no errors, got:\n${joined(e)}`); + } + + { + // A noise word is not a repo. This is the hole the structural rule + // would have if it exempted "any preceding word". + const e = cited(rows(`see ${id('0202')}`), RECORDS, []); + assert( + 'noise-word-does-not-exempt-a-bare-id', + e.length === 1, + `only a sibling-repo qualifier may exempt an id, got ${e.length}:\n${joined(e)}`, + ); + } + + { + const e = cited(rows(`withdrawn ${id('0202')}`), RECORDS, [{ number: '0202', why: 'synthetic' }]); + assert('allowlisted-dangling-citation-is-green', e.length === 0, `expected no errors, got:\n${joined(e)}`); + } + + { + // Stale, direction A — the number gained a record. This is the squat + // half of #6634: old citations now point at a decision they never meant. + const e = cited(rows(`see ${id('0107')}`), RECORDS, [{ number: '0107', why: 'synthetic' }]); + assert( + 'allowlist-entry-whose-number-gained-a-record-is-red', + e.length === 1 && /stale/.test(joined(e)) && joined(e).includes('now HAS a record'), + `an allowlisted dead number that came back must fail, got:\n${joined(e)}`, + ); + } + + { + // Stale, direction B — nothing cites it any more. + const e = cited(rows(`see ${id('0090')}`), RECORDS, [{ number: '0202', why: 'synthetic' }]); + assert( + 'allowlist-entry-nothing-cites-is-red', + e.length === 1 && /stale/.test(joined(e)), + `a grandfather clause outliving its citations must fail, got:\n${joined(e)}`, + ); + } + } + // ── Live tree: green as shipped, red under ablation ────────────────────── let liveFiles = null; try { @@ -515,6 +860,45 @@ function selfTest() { `ADR number ${n} is allowlisted but does not collide under ablation — the entry is stale`, ); } + + // ── The citation scan, over the real tree (#6634) ───────────────────── + const liveRecords = audit(liveFiles, KNOWN_NUMBER_COLLISIONS).records; + const liveCitations = collectCitations(); + + // A scan that reads nothing is a phantom gate, and it would fail SILENTLY + // — every citation resolving vacuously. Pin that it really walked the tree + // before trusting anything it says. + assert( + 'live-citation-scan-reads-the-tree', + liveCitations.length > 100 && new Set(liveCitations.map((c) => c.file)).size > 50, + `expected the scan to find citations across the repo, got ${liveCitations.length} in ` + + `${new Set(liveCitations.map((c) => c.file)).size} file(s) — the collector is not reading the tree`, + ); + + const citeGreen = auditCitedNumbers(liveCitations, liveRecords, UNRESOLVED_ADR_CITATIONS); + assert( + 'live-citations-are-green-today', + citeGreen.length === 0, + `every cited ADR number must resolve as shipped, got:\n${joined(citeGreen)}`, + ); + + // Ablation, predicted RED: drop the citation allowlist and each entry's + // number must surface. Green here means the entries are dead weight. + const citeRed = auditCitedNumbers(liveCitations, liveRecords, []); + const citeNumbers = UNRESOLVED_ADR_CITATIONS.map((c) => c.number); + assert( + 'citation-ablation-without-allowlist-is-red', + citeRed.length === citeNumbers.length, + `expected ${citeNumbers.length} dangling number(s) with the allowlist removed, got ${citeRed.length}:\n` + + joined(citeRed), + ); + for (const n of citeNumbers) { + assert( + `citation-ablation-reports-${n}`, + citeRed.some((e) => e.includes('ADR-' + n + ' is cited')), + `ADR-${n} is allowlisted as unresolved but resolves under ablation — the entry is stale`, + ); + } } } catch (e) { // Never let the harness itself be the message. @@ -526,6 +910,9 @@ function selfTest() { for (const f of failures) console.error(' • ' + f + '\n'); process.exit(1); } - console.log(`✓ check-adr-anchors --self-test: ${checked} assertions over the real auditAdrDirectory() path.`); + console.log( + `✓ check-adr-anchors --self-test: ${checked} assertions over the real auditAdrDirectory() / ` + + 'auditCitedNumbers() paths.', + ); process.exit(0); }