diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66daf1ed..f48cd318 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,6 +98,13 @@ jobs: run: bun scripts/run-network-denied.ts -- sh -c 'bun run schema:emit && git diff --exit-code schema/adr.schema.json' - name: Verify committed Action bundle matches source (network denied) run: bun scripts/run-network-denied.ts -- git diff --exit-code packages/ci/dist + # MANIFEST.md's record inventory is a pure function of docs/adr/, and it drifted + # six records before anyone noticed (#131). A guard that merely reported the + # divergence would still leave a human to hand-edit the table — which is how it + # drifted — so the block is regenerated and the tree asserted unchanged, exactly + # like the schema emit above. Both halves wrapped, for the reason recorded there. + - name: Verify MANIFEST inventory matches the corpus (network denied) + run: bun scripts/run-network-denied.ts -- sh -c 'bun run emit:manifest && git diff --exit-code MANIFEST.md' - name: Verify dependency boundaries (adapters + toolkit confinement) run: bun scripts/run-network-denied.ts -- bun run check:deps - name: Verify frozen oracle hashes match (feature 010 Barrier B, R5 mechanism 2) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95ada2b4..d5db631f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,30 @@ Until `1.0.0`, minor releases may include breaking changes ### Added +- **`MANIFEST.md`'s decision-corpus inventory is generated, not hand-written.** + `bun run emit:manifest` renders the record table and the status counts from + `adr graph --format json` between stable markers, and `clean-clone-builds` + regenerates it and asserts the tree is unchanged — the same shape as the + schema emit and the committed Action bundle. The inventory had drifted six + records before it was noticed + ([#131](https://github.com/mbeacom/adrkit/issues/131)). No public CLI surface + was added: the CLI stays read-only and the writing lives in a repo-local + script ([#132](https://github.com/mbeacom/adrkit/issues/132)). Judgment prose + outside the markers stays hand-maintained. + +### Fixed + +- **`adr queue` no longer stays silent when a proposed record has a review + deadline but no routing tier.** `item.tier-absent` now fires whenever the tier + cannot be determined on a record that has entered review — a `review` block, + or a top-level `reviewBy`. The carve-out the spec actually states is + two-conditioned (both absent), but only the first condition was implemented, + so a `cross-team` record with `reviewBy` and no `review` block was listed with + `tier=None` and no finding at all + ([#111](https://github.com/mbeacom/adrkit/issues/111)). A `proposed` record + with neither remains `not-queued` and silent. Severity stays `info`, so no + exit code changes. + - **Trusted CI gates that the pull request cannot edit.** A new `.github/workflows/trusted-gates.yml` runs on `pull_request_target`, which GitHub executes from the repository's default branch — workflow file, diff --git a/MANIFEST.md b/MANIFEST.md index be1ea444..b2adc0c2 100644 --- a/MANIFEST.md +++ b/MANIFEST.md @@ -56,9 +56,62 @@ adrkit/ - The ADR corpus lives in [docs/adr/](docs/adr/), with `0000-template.md` plus numbered records. -- There are 36 files: the template plus 35 records, ids `0001`-`0035`, with 33 - accepted and 2 superseded records. - The schema source of truth lives in `packages/core/src/schema/adr.schema.ts`. - `schema/adr.schema.json` is generated from that source and hosted at the schema `$id` through the docs site. + +### Inventory + +The block below is generated from the corpus by `bun run emit:manifest`, and +`clean-clone-builds` fails if the committed copy has drifted from what the +corpus says. Do not hand-edit it; edit the records instead. Anything outside the +markers — including judgment about what is open or unfinished — stays +hand-written. + + + + + +There are 35 records, ids `0001`-`0035`, alongside +`0000-template.md`: 33 accepted, 2 superseded. + +| Id | Status | Title | +| --- | --- | --- | +| `0001` | accepted | Record architecture decisions as versioned markdown in git | +| `0002` | accepted | Type the frontmatter as a MADR superset with an affects matcher | +| `0003` | accepted | Ship as a Spec Kit extension plus a standalone CLI, not a competing harness | +| `0004` | accepted | Treat git as the source of truth and the database as a derived index | +| `0005` | superseded | Gate proposals with a deterministic-first evaluator and declarative escalation | +| `0006` | accepted | License Apache-2.0 with a DCO and develop in a single monorepo | +| `0007` | accepted | Isolate integrations as optional adapters and build only against public surfaces | +| `0008` | accepted | Migrate MADR corpora in place and treat all other imports as one-way with a re-import diff | +| `0009` | accepted | Pin affects resolution semantics and bind entity refs to pluggable catalogs | +| `0010` | accepted | Use Bun as the package manager and test runner while publishing Node-targeted artifacts | +| `0011` | accepted | Host the canonical JSON Schema at its $id on adrkit.dev | +| `0012` | accepted | Bind catalog entities to owned paths with an explicit annotation | +| `0013` | accepted | Reconcile adapter isolation and catalog binding with the offline snapshot generator | +| `0014` | accepted | Stage phase-landing evidence across a three-rung validation ladder | +| `0015` | accepted | Validate descriptors against Backstage field formats before canonicalizing identity | +| `0016` | accepted | Require every check to be observed failing before it counts as coverage | +| `0017` | accepted | Keep dependency audit scope explicit and release-scoped | +| `0018` | accepted | Adopt MCP SDK v2 and serve protocol revision 2026-07-28 dual-era | +| `0019` | accepted | Ship the Spec Kit extension, treating the spike's no-go as a measurement artifact | +| `0020` | accepted | Rescope SC-010 and authorize work toward the Backstage catalog adapter | +| `0021` | superseded | Resolve inbound source annotations without changing the schema | +| `0022` | accepted | Scan inbound markers in check and CI without giving them exit-code authority | +| `0023` | accepted | Read a marker only where the format hides it: fences and markdown prose | +| `0024` | accepted | Report the measured scan extent, not the window constant | +| `0025` | accepted | Ship badges as recipes over existing output, not a new CLI surface | +| `0026` | accepted | Identify the CI comment by the strongest author evidence the token allows | +| `0027` | accepted | Ratify the deterministic evaluator and bind calibration reporting to the first probabilistic pass | +| `0028` | accepted | Ship decision memory as a portable agent plugin, and omit the MCP wiring hosts cannot honor | +| `0029` | accepted | Scope Backstage publication as a downstream consumer, tiered on the entity-ownership mapping | +| `0030` | accepted | Keep extension surfaces that carry a dependency tree outside this repository | +| `0031` | accepted | Publish a narrow consumer SDK as the contract, and document the CLI JSON as its sibling | +| `0032` | accepted | Publish one lockstep OCI image after the coordinated release succeeds | +| `0033` | accepted | Select interactive graph presentation at the CLI boundary while preserving piped DOT | +| `0034` | accepted | Extend the portable agent plugin with decision backfill | +| `0035` | accepted | Execute the gates that certify a pull request from the default branch | + + diff --git a/docs/RELEASING.md b/docs/RELEASING.md index a3c29daa..c1eae80f 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -356,12 +356,13 @@ bootstrap described below. Leave *historical* statements alone ("expanded in v0.5.0" records when something happened and stays true). This drifts silently: v0.6.0 shipped with all three site surfaces still advertising v0.5.0, so the hosted docs were a release behind - for anyone reading them. Check `MANIFEST.md`'s inventory and its Verification - counts against the corpus in the same pass — it is hand-maintained - ([#131](https://github.com/mbeacom/adrkit/issues/131)) and drifts the same way; it - was missing ADR-0026 and understating the record and accepted counts by one when - v0.7.0 was cut. `ls docs/adr/*.md | wc -l` and - `grep -h '^status:' docs/adr/*.md | sort | uniq -c` give the numbers to compare. + for anyone reading them. `MANIFEST.md`'s decision-corpus inventory no longer + needs this pass: it is generated by `bun run emit:manifest` and + `clean-clone-builds` fails when the committed copy has drifted from the corpus + ([#131](https://github.com/mbeacom/adrkit/issues/131)). It used to be + hand-maintained, and was missing ADR-0026 and understating the record and + accepted counts by one when v0.7.0 was cut. Prose *outside* the generated + markers is still hand-written and still worth reading in this pass. 5. Merge the version change only after CI passes. 6. Create and push the matching annotated tag, such as `v0.3.0`. 7. Approve the protected `npm` environment deployment. diff --git a/package.json b/package.json index 6b850ae4..7eefd01c 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "check:freeze-hashes": "bun run scripts/check-freeze-hashes.ts", "check:doc-pins": "bun run scripts/check-doc-cli-versions.ts", "check:site-grammar": "bun run scripts/check-site-corpus-grammar.ts", + "emit:manifest": "bun run scripts/emit-manifest.ts", "check:dco": "bun run scripts/check-dco.ts", "check:gate-integrity": "bun run scripts/check-gate-integrity.ts", "check:ci-comment": "bun run scripts/check-ci-comment.ts", diff --git a/packages/ci/dist/queue-action.js b/packages/ci/dist/queue-action.js index 04b63e4b..e91c7040 100644 --- a/packages/ci/dist/queue-action.js +++ b/packages/ci/dist/queue-action.js @@ -46367,7 +46367,8 @@ function computeItemFindings(frontmatter) { const findings = []; const review = frontmatter.review; const queuedAt = review?.queuedAt; - if (review !== undefined && review.tier == null) { + const enteredReview = review !== undefined || frontmatter.reviewBy != null; + if (enteredReview && review?.tier == null) { findings.push({ code: "item.tier-absent", severity: "info", message: ITEM_MESSAGES.tierAbsent }); } if (frontmatter.reviewBy != null && queuedAt != null) { diff --git a/packages/core/src/queue/kernel.ts b/packages/core/src/queue/kernel.ts index 3e8588c0..7a762957 100644 --- a/packages/core/src/queue/kernel.ts +++ b/packages/core/src/queue/kernel.ts @@ -106,9 +106,24 @@ function computeItemFindings(frontmatter: AdrFrontmatter): ItemFinding[] { const review = frontmatter.review; const queuedAt = review?.queuedAt; - // No item finding is generated for the absence of `review` alone (spec §Edge cases): - // tier-absent fires only when a `review` block is present but omits `tier`. - if (review !== undefined && review.tier == null) { + // `item.tier-absent` fires whenever the routing tier cannot be determined on a + // record that has *entered* review — `review` present, or a top-level `reviewBy` + // deadline (#111). + // + // The spec's carve-out is two-conditioned and its justification is "not yet + // entered into the review workflow" (spec.md §Edge cases): `review` block **and** + // top-level `reviewBy` both absent. This previously tested only the first + // condition, so ADR-0022 — `blastRadius: cross-team`, `reviewBy: 2027-02-08`, no + // `review` block — sat in the queue with `tier=None` and no finding at all, and + // was ratified without a routing tier. That is the ADR-0016 shape: "could not + // determine" rendering identically to "nothing to report". The contract's own + // condition (contracts/kernel.md `item.tier-absent`) is simply "`review?.tier` is + // absent"; the carve-out is the only narrowing, and it does not reach a record + // carrying an explicit deadline. + // + // A `proposed` record with neither stays silent, exactly as the spec intends. + const enteredReview = review !== undefined || frontmatter.reviewBy != null; + if (enteredReview && review?.tier == null) { findings.push({ code: 'item.tier-absent', severity: 'info', message: ITEM_MESSAGES.tierAbsent }); } diff --git a/packages/core/test/ordering-contract.test.ts b/packages/core/test/ordering-contract.test.ts index 70c5b6e3..0e40a8a7 100644 --- a/packages/core/test/ordering-contract.test.ts +++ b/packages/core/test/ordering-contract.test.ts @@ -92,16 +92,39 @@ describe('sortFindings orders every tuple field by code unit', () => { }); }); -describe('no scanned module on the check --json path reaches for localeCompare', () => { +describe('no scanned module on a serialized-output path reaches for localeCompare', () => { // The same source-scan shape as the adapter's // `test/glob-order.test.ts` guard, widened to every core module that feeds // `CheckOutcome`: `check/`, `load/`, `markers/`, `ordering/`, and `validate/`. + // `queue/` is scanned for the same reason on its own contract rather than on + // `CheckOutcome`'s: QueueReport v1 promises byte-for-byte identical output for + // identical inputs (007-arb-queue SC-001), which a locale-dependent sort would + // break as a difference between machines. + // + // `graph/` is deliberately NOT scanned, for the same shape of reason as + // `affects/` and with the same honesty about what a clean run therefore means. + // `buildAdrGraph` still orders nodes and edges with `localeCompare`, and + // ADR-0033 clause 8 pins that: "The graph JSON shape remains exactly + // `{ nodes, edges }`, with existing node and edge fields, **historical locale + // ordering**, and missing-target omission unchanged." Migrating it is a change + // to an accepted decision, not a defect fix, so it is recorded on #115 rather + // than made here. `scripts/emit-manifest.ts` does not depend on that order: it + // re-sorts the nodes it reads with `compareCodeUnits` before rendering, so the + // `MANIFEST.md` no-diff gate is locale-independent regardless of how this + // resolves. // These are scanned as whole directories rather than as a file allowlist, so a // new module added to any of them is covered the day it lands — an allowlist // silently exempts new files, which is how `validate/index.ts` stayed unscanned // while `validate/findings.ts` was named individually. See the header for why // `affects/` is excluded. - const SCANNED_DIRS = ['src/check', 'src/load', 'src/markers', 'src/ordering', 'src/validate']; + const SCANNED_DIRS = [ + 'src/check', + 'src/load', + 'src/markers', + 'src/ordering', + 'src/queue', + 'src/validate', + ]; function tsFilesUnder(relativeDir: string): string[] { const root = join(import.meta.dir, '..', relativeDir); @@ -126,6 +149,7 @@ describe('no scanned module on the check --json path reaches for localeCompare', expect(scanned).toContain('src/check/index.ts'); expect(scanned).toContain('src/load/corpus.ts'); expect(scanned).toContain('src/markers/resolve.ts'); + expect(scanned).toContain('src/queue/kernel.ts'); expect(scanned).toContain('src/validate/findings.ts'); // `lintCorpus` lives here and produces the `records` `checkChanges` reads, so // its absence from the scan was the gap the directory walk closes. diff --git a/packages/core/test/queue/kernel.test.ts b/packages/core/test/queue/kernel.test.ts index e42e059e..1aeb4ad5 100644 --- a/packages/core/test/queue/kernel.test.ts +++ b/packages/core/test/queue/kernel.test.ts @@ -159,6 +159,55 @@ describe('buildQueueReport — item findings', () => { }); }); + /** + * #111 — the real regression, reconstructed. + * + * ADR-0022 sat in the queue as `proposed`, `blastRadius: cross-team`, with + * `reviewBy: 2027-02-08` and no `review` block: `tier=None`, `findings=[]`. It + * was ratified without a routing tier because the queue said nothing. The + * spec's carve-out requires `review` **and** `reviewBy` to *both* be absent + * ("not yet entered into the review workflow"); a record carrying an explicit + * deadline has entered it, and the spec's very next sentence gives that exact + * combination SLA treatment. + */ + test('item.tier-absent when reviewBy is present with no review block (the ADR-0022 shape)', () => { + const report = buildQueueReport({ + corpus: single({ deciders: ['@mbeacom'], reviewBy: '2027-02-08' }), + asOf: '2026-01-08', + }); + expect(report.items[0]?.tier).toBeNull(); + expect(report.items[0]?.deadlineDate).toBe('2027-02-08'); + expect(report.items[0]?.slaState).toBe('within-sla'); + expect(report.items[0]?.itemFindings).toContainEqual({ + code: 'item.tier-absent', + severity: 'info', + message: TIER_ABSENT_MESSAGE, + }); + }); + + test('the finding is info, so it never changes the queue exit code', () => { + // Completeness signal, not a blocker: `adr queue` exits non-zero only on + // error-severity corpus findings, and this is neither. + const report = buildQueueReport({ + corpus: single({ deciders: ['@mbeacom'], reviewBy: '2027-02-08' }), + asOf: '2026-01-08', + }); + expect(report.items[0]?.itemFindings.every((f) => f.severity === 'info')).toBe(true); + expect(report.corpusFindings.filter((f) => f.severity === 'error')).toEqual([]); + }); + + test('the carve-out survives: neither review nor reviewBy stays silent', () => { + // The genuine "proposed but not yet entered into the review workflow" state. + // Widening the finding to every tierless record would fire here, which the + // spec explicitly forbids. + const report = buildQueueReport({ + corpus: single({ deciders: ['@mbeacom'] }), + asOf: '2026-01-08', + }); + expect(report.items[0]?.slaState).toBe('not-queued'); + expect(report.items[0]?.itemFindings).toEqual([]); + }); + test('item.deciders-empty when queuedAt present and deciders empty', () => { const report = buildQueueReport({ corpus: single({ deciders: [], review: { tier: 'arb', queuedAt: '2026-01-01T00:00:00Z', slaDays: 14 } }), diff --git a/scripts/emit-manifest.test.ts b/scripts/emit-manifest.test.ts new file mode 100644 index 00000000..0d8141e2 --- /dev/null +++ b/scripts/emit-manifest.test.ts @@ -0,0 +1,283 @@ +/** + * `emit-manifest` — and the negative cases that make the gate mean something. + * + * The gate `clean-clone-builds` runs is `bun run emit:manifest && git diff + * --exit-code MANIFEST.md`. That composition has exactly one failure mode worth + * fearing: a generator that writes *nothing* leaves the diff clean, so the gate + * reports green while checking nothing — the ADR-0016 shape this repository + * keeps finding. So the cases below drive the degenerate marker states, the + * empty corpus, and a deliberately stale MANIFEST, and assert each is caught + * rather than passed over. + */ + +import { describe, expect, test } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { + MARKER_BEGIN, + MARKER_END, + describeIdRange, + describeStatusCounts, + parseGraphNodes, + renderInventoryBlock, + replaceGeneratedBlock, + type GraphNode, +} from './emit-manifest.ts'; + +const REPO_ROOT = join(import.meta.dir, '..'); + +const node = (id: string, status = 'accepted', title = `Title ${id}`): GraphNode => ({ id, title, status }); + +const THREE: GraphNode[] = [node('0001'), node('0002', 'superseded'), node('0003')]; + +function surround(block: string): string { + return `# Manifest\n\nPreamble.\n\n${block}\n\nTrailing prose.\n`; +} + +describe('describeIdRange', () => { + test('a contiguous numeric run is reported as a range', () => { + expect(describeIdRange(['0001', '0002', '0003'])).toBe('`0001`-`0003`'); + }); + + test('a gap yields no range rather than a wrong one', () => { + // The failure this prevents: `0001`-`0004` claiming a record that is not there. + expect(describeIdRange(['0001', '0002', '0004'])).toBeNull(); + }); + + test('ULID ids yield no range', () => { + // The grammar admits 26-character Crockford base32; "from X to Y" means + // nothing over those, so the sentence is omitted. + expect(describeIdRange(['01ARZ3NDEKTSV4RRFFQ69G5FAV', '01BX5ZZKBKACTAV9WEVGEMMVRZ'])).toBeNull(); + }); + + test('mixed widths yield no range', () => { + expect(describeIdRange(['0001', '00002'])).toBeNull(); + }); + + test('a single record is its own range', () => { + expect(describeIdRange(['0007'])).toBe('`0007`-`0007`'); + }); + + test('an empty corpus has no range', () => { + expect(describeIdRange([])).toBeNull(); + }); +}); + +describe('describeStatusCounts', () => { + test('counts every status the corpus carries', () => { + expect(describeStatusCounts(THREE)).toBe('2 accepted, 1 superseded'); + }); + + test('orders statuses by code unit, not by locale', () => { + // 'S' (0x53) sorts before 'a' (0x61) by code unit; a locale-aware + // comparison typically interleaves the cases instead. + const mixed = [node('0001', 'accepted'), node('0002', 'Superseded')]; + expect('S'.localeCompare('a')).toBeGreaterThan(0); + expect(describeStatusCounts(mixed)).toBe('1 Superseded, 1 accepted'); + }); +}); + +describe('renderInventoryBlock', () => { + const block = renderInventoryBlock(THREE); + + test('carries both markers and a row per record', () => { + expect(block.startsWith(MARKER_BEGIN)).toBe(true); + expect(block.trimEnd().endsWith(MARKER_END)).toBe(true); + expect(block).toContain('| `0001` | accepted | Title 0001 |'); + expect(block).toContain('| `0002` | superseded | Title 0002 |'); + expect(block).toContain('There are 3 records, ids `0001`-`0003`, alongside'); + expect(block).toContain('2 accepted, 1 superseded.'); + }); + + test('orders rows by code unit whatever order the graph supplied', () => { + const reversed = renderInventoryBlock([...THREE].reverse()); + const ids = [...reversed.matchAll(/^\| `(\d+)`/gmu)].map((match) => match[1]); + expect(ids).toEqual(['0001', '0002', '0003']); + expect(reversed).toBe(renderInventoryBlock(THREE)); + }); + + test('normalizes away the upstream locale ordering', () => { + // `buildAdrGraph` sorts with `localeCompare`, and ADR-0033 clause 8 pins + // that ordering, so the gate cannot rest on it: the same records would + // render in a different row order under a different ICU locale and + // `git diff --exit-code` would fail with nothing changed. The id grammar + // admits mixed-case ULIDs, where the two comparators genuinely disagree. + const upper = `A${'0'.repeat(25)}`; + const lower = `b${'0'.repeat(25)}`; + expect(upper.localeCompare(lower)).toBeLessThan(0); + + const localeOrder = [node(lower), node(upper)].sort((a, b) => a.id.localeCompare(b.id)); + expect(localeOrder.map((n) => n.id)).toEqual([upper, lower]); + + // And the reverse pair, where locale and code unit disagree outright. + const zeta = `Z${'0'.repeat(25)}`; + const alpha = `a${'0'.repeat(25)}`; + expect(zeta.localeCompare(alpha)).toBeGreaterThan(0); + + const rendered = renderInventoryBlock([node(alpha), node(zeta)]); + expect(rendered.indexOf(zeta)).toBeLessThan(rendered.indexOf(alpha)); + expect(rendered).toBe(renderInventoryBlock([node(zeta), node(alpha)])); + }); + + test('escapes a pipe in a title rather than breaking the table', () => { + const rendered = renderInventoryBlock([node('0001', 'accepted', 'Use auto|async|arb')]); + expect(rendered).toContain('| `0001` | accepted | Use auto\\|async\\|arb |'); + }); + + test('a multiline title stays on one row', () => { + // `title` is `z.string().min(3).max(120)`, which a YAML block scalar can + // satisfy with an embedded newline. A raw newline in a cell splits one row + // into two lines of markdown — and the gate would still pass, because the + // broken table is what regeneration reproduces. Same order as the queue + // formatter's `escapeCell`: CRLF -> LF, CR -> space, LF ->
. + const rendered = renderInventoryBlock([node('0001', 'accepted', 'First line\r\nsecond\rthird\nfourth')]); + expect(rendered).toContain('| `0001` | accepted | First line
second third
fourth |'); + + const rows = rendered.split('\n').filter((line) => line.startsWith('| `0001`')); + expect(rows).toHaveLength(1); + expect(rendered).not.toMatch(/\r/u); + }); + + test('a title cannot forge a marker on a line of its own', () => { + // The composition that closes the hole: newline normalization means a title + // can never *start* a line, so it can never become a standalone marker. + const rendered = renderInventoryBlock([ + node('0001', 'accepted', `A title\n${MARKER_END}\nand more`), + ]); + const forged = rendered + .split('\n') + .filter((line) => line.trim() === MARKER_END || line.trim() === MARKER_BEGIN); + expect(forged).toEqual([MARKER_BEGIN, MARKER_END]); + }); + + test('escapes a backtick, so a title cannot open a code span across the row', () => { + const rendered = renderInventoryBlock([node('0001', 'accepted', 'Use `adr queue`')]); + expect(rendered).toContain('| `0001` | accepted | Use \\`adr queue\\` |'); + }); + + test('refuses to publish an empty inventory', () => { + // Negative case: zero records is what a corpus the loader could not see + // looks like, and it would otherwise be committed as a truthful-looking "0". + expect(() => renderInventoryBlock([])).toThrow(/no records/u); + }); +}); + +describe('replaceGeneratedBlock', () => { + const block = renderInventoryBlock(THREE); + + test('replaces the marked region and leaves the surrounding prose alone', () => { + const before = surround(`${MARKER_BEGIN}\nstale\n${MARKER_END}`); + const after = replaceGeneratedBlock(before, block); + expect(after).toContain('Preamble.'); + expect(after).toContain('Trailing prose.'); + expect(after).not.toContain('stale'); + expect(after).toContain('| `0001` | accepted | Title 0001 |'); + }); + + test('is idempotent', () => { + const once = replaceGeneratedBlock(surround(`${MARKER_BEGIN}\n${MARKER_END}`), block); + expect(replaceGeneratedBlock(once, block)).toBe(once); + }); + + test('throws when the markers were deleted', () => { + // Negative case: without this, deleting the markers turns the CI gate into + // a no-op that still reports success. + expect(() => replaceGeneratedBlock('# Manifest\n\nNo markers here.\n', block)).toThrow( + /exactly one .*found 0 and 0/su, + ); + }); + + test('throws when only one marker survives', () => { + expect(() => replaceGeneratedBlock(surround(MARKER_BEGIN), block)).toThrow(/found 1 and 0/u); + }); + + test('throws when a marker is duplicated', () => { + const doubled = surround(`${MARKER_BEGIN}\n${MARKER_END}\n${MARKER_BEGIN}\n${MARKER_END}`); + expect(() => replaceGeneratedBlock(doubled, block)).toThrow(/found 2 and 2/u); + }); + + test('ignores a marker embedded in a table cell', () => { + // A record whose *title* contains the end marker renders it into a row + // inside the block. Counting the substring anywhere made that a second + // marker, so every later run threw `found 1 and 2` — one corpus record + // permanently disabling the generator that has to render it. + const poisoned = renderInventoryBlock([ + node('0001', 'accepted', `Ship the thing ${MARKER_END} carefully`), + ]); + const document = surround(poisoned); + + // Round-trips: the poisoned row survives, and regeneration is still stable. + expect(document).toContain(`Ship the thing ${MARKER_END} carefully`); + expect(replaceGeneratedBlock(document, poisoned)).toBe(document); + expect(replaceGeneratedBlock(document, block)).toContain('| `0002` | superseded | Title 0002 |'); + expect(replaceGeneratedBlock(document, block)).not.toContain('Ship the thing'); + }); + + test('ignores a marker mentioned in prose', () => { + // The section above the block explains the markers by name; prose that + // names one must not be mistaken for the delimiter itself. + const document = `# Manifest\n\nEdit outside ${MARKER_BEGIN} and ${MARKER_END} only.\n\n${MARKER_BEGIN}\n${MARKER_END}\n`; + expect(replaceGeneratedBlock(document, block)).toContain('| `0001` | accepted | Title 0001 |'); + expect(replaceGeneratedBlock(document, block)).toContain(`Edit outside ${MARKER_BEGIN} and`); + }); + + test('tolerates indentation, trailing whitespace, and CRLF around a real marker', () => { + const document = `# Manifest\r\n\r\n ${MARKER_BEGIN} \r\nstale\r\n${MARKER_END}\t\r\ntail\r\n`; + const replaced = replaceGeneratedBlock(document, block); + expect(replaced).not.toContain('stale'); + expect(replaced).toContain('| `0001` | accepted | Title 0001 |'); + expect(replaced).toContain('tail'); + }); + + test('throws when the markers are inverted', () => { + expect(() => replaceGeneratedBlock(surround(`${MARKER_END}\n${MARKER_BEGIN}`), block)).toThrow( + /appears before/u, + ); + }); +}); + +describe('parseGraphNodes', () => { + test('reads the nodes of a graph payload', () => { + const json = JSON.stringify({ nodes: [{ id: '0001', title: 'T', status: 'accepted' }], edges: [] }); + expect(parseGraphNodes(json)).toEqual([{ id: '0001', title: 'T', status: 'accepted' }]); + }); + + test('throws when the payload has no nodes array', () => { + expect(() => parseGraphNodes('{"edges":[]}')).toThrow(/no `nodes` array/u); + }); + + test('throws when a node is missing a field', () => { + // A renamed field would otherwise render as `undefined` in the table. + expect(() => parseGraphNodes('{"nodes":[{"id":"0001","title":"T"}]}')).toThrow(/node 0 is missing/u); + }); +}); + +describe('the committed MANIFEST.md is what the corpus says', () => { + const manifest = readFileSync(join(REPO_ROOT, 'MANIFEST.md'), 'utf8'); + + test('it carries exactly one marked block', () => { + // Guard on the guard: if the markers were removed, every assertion that + // follows — and the CI gate itself — would be inspecting nothing. + expect(manifest.split(MARKER_BEGIN).length - 1).toBe(1); + expect(manifest.split(MARKER_END).length - 1).toBe(1); + }); + + test('the block is populated, not an empty shell', () => { + const rows = [...manifest.matchAll(/^\| `\d+` \| \w+ \|/gmu)]; + expect(rows.length).toBeGreaterThan(30); + }); + + test('a stale MANIFEST is detected by regeneration', () => { + // The ADR-0016 negative case for the CI gate, run against the real file: + // drop a row from the committed inventory and confirm regeneration puts it + // back — i.e. that `git diff --exit-code` would have something to report. + const stale = manifest.replace(/^\| `0001` \|.*\n/mu, ''); + expect(stale).not.toBe(manifest); + + const regenerated = replaceGeneratedBlock( + stale, + manifest.slice(manifest.indexOf(MARKER_BEGIN), manifest.indexOf(MARKER_END) + MARKER_END.length), + ); + expect(regenerated).toBe(manifest); + }); +}); diff --git a/scripts/emit-manifest.ts b/scripts/emit-manifest.ts new file mode 100644 index 00000000..28794cfe --- /dev/null +++ b/scripts/emit-manifest.ts @@ -0,0 +1,272 @@ +/** + * Render `MANIFEST.md`'s decision-corpus inventory from the corpus itself. + * + * bun run emit:manifest + * + * # Why this exists + * + * The inventory — how many records there are, which ids, and how the statuses + * split — is a pure function of `docs/adr/`. It was hand-maintained, and in #131 + * it was found publishing six records' worth of contradictory governance state. + * A guard that merely *reports* the divergence would still leave a human to + * hand-edit the table, which is exactly how it drifted, so this generates the + * block and `clean-clone-builds` asserts the working tree is unchanged + * afterwards — the same shape as `schema:emit` + `git diff --exit-code`. + * + * # Why it is a repo-local script and not a CLI command + * + * For the reason `check-doc-cli-versions.ts` states in its own docblock, plus + * two more that are specific to writing: the public CLI's write surface is + * deliberately small (`new` and `migrate` only), and the Spec Kit adapter holds + * a *tested* invariant that hooks reach only non-writing commands. No new + * public surface is needed anyway — `adr graph --format json` already emits + * `id`, `title` and `status`, which is the whole of the derived state. So this + * is the established pattern: read-only CLI, redirected, gated (#132). + * + * # What is deliberately NOT generated + * + * Only the block between the markers. The repository tree diagram, the planning + * sources, and any judgment prose stay hand-written — they are editorial, not + * derived, and generating them would be the "evaluator theater" ADR-0005 warns + * about. Equally, nothing here claims a *file* count for `docs/adr/`: the graph + * cannot see a file the corpus grammar rejects, so a file count sourced from it + * would be the same silent under-report this script exists to end. `adr lint` + * is the check that fails on an unreadable record, and it already runs in + * `clean-clone-builds`. + * + * # The failure mode this script must not have + * + * A generator that writes nothing leaves `git diff` clean, so the gate reports + * green while checking nothing (ADR-0016). Every way of writing nothing is + * therefore an error here: absent markers, duplicated markers, markers in the + * wrong order, and an empty node list all throw rather than no-op. + */ + +import { readFileSync, writeFileSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { compareCodeUnits } from '../packages/core/src/ordering/index.ts'; + +const repoRoot = resolve(import.meta.dir, '..'); + +export const MARKER_BEGIN = ''; +export const MARKER_END = ''; + +/** The `nodes` shape of `adr graph --format json`; `edges` are not inventory. */ +export interface GraphNode { + readonly id: string; + readonly title: string; + readonly status: string; +} + +/** A contiguous run of zero-padded numeric ids, or `null` when there isn't one. */ +export function describeIdRange(ids: readonly string[]): string | null { + if (ids.length === 0) return null; + // Ids are `[0-9]{4,}` or a 26-character ULID. A ULID corpus, or a numeric one + // with a gap, gets no range sentence rather than a wrong one. + if (!ids.every((id) => /^[0-9]{4,}$/u.test(id))) return null; + + const width = ids[0]!.length; + if (!ids.every((id) => id.length === width)) return null; + + const numbers = ids.map(Number); + for (let index = 1; index < numbers.length; index += 1) { + if (numbers[index]! !== numbers[index - 1]! + 1) return null; + } + return `\`${ids[0]}\`-\`${ids[ids.length - 1]}\``; +} + +/** + * `32 accepted, 2 superseded` — every status the corpus actually carries, in + * code-unit order so a status this repository has never used still renders + * deterministically the day it appears. + */ +export function describeStatusCounts(nodes: readonly GraphNode[]): string { + const counts = new Map(); + for (const node of nodes) counts.set(node.status, (counts.get(node.status) ?? 0) + 1); + + return [...counts.entries()] + .sort(([a], [b]) => compareCodeUnits(a, b)) + .map(([status, count]) => `${count} ${status}`) + .join(', '); +} + +/** + * Escape a table-cell value, in `@adrkit/core`'s queue-formatter order + * (`packages/core/src/queue/format.ts` `escapeCell`): CRLF→LF, CR→space, + * LF→`
`, then `\`, `|`, and a backtick. + * + * The line-break half is not cosmetic. `title` is `z.string().min(3).max(120)`, + * which a YAML block scalar can satisfy with an embedded newline, and a raw + * newline in a cell splits one table row into two lines of markdown. That is + * how a record could otherwise emit a line that *is* a marker, poisoning the + * block it lives inside. Backslash is escaped before the characters whose + * escapes introduce one, so a literal `\` can never combine with an introduced + * escape. + */ +function tableCell(value: string): string { + return value + .replaceAll('\r\n', '\n') + .replaceAll('\r', ' ') + .replaceAll('\n', '
') + .replaceAll('\\', '\\\\') + .replaceAll('|', '\\|') + .replaceAll('`', '\\`'); +} + +/** + * The generated block, markers included. + * + * Nodes are re-sorted here with `compareCodeUnits` rather than trusted in the + * order `adr graph` supplied. That order is `localeCompare`'s, and ADR-0033 + * clause 8 pins it deliberately — "existing node and edge fields, **historical + * locale ordering**, and missing-target omission unchanged" — so it is not this + * script's to change, and equally not one a `git diff --exit-code` gate can rest + * on: `localeCompare` follows the runtime's ICU locale, so two contributors + * would regenerate different row orders from identical records and the gate + * would fail with nothing having changed. Today's corpus is all zero-padded + * numeric ids, which both comparators order identically, so this is inert now + * and load-bearing the day a mixed-case ULID lands. #115 tracks the upstream + * sort. + */ +export function renderInventoryBlock(nodes: readonly GraphNode[]): string { + if (nodes.length === 0) { + // A zero-record corpus is indistinguishable from a corpus the loader could + // not see (ADR-0016), and this repository always has records. + throw new Error( + '`adr graph --format json` reported no records. Refusing to publish an empty inventory — ' + + 'run `bun run adr lint` and check that docs/adr/ is readable.', + ); + } + + const ordered = [...nodes].sort((a, b) => compareCodeUnits(a.id, b.id)); + const ids = ordered.map((node) => node.id); + const range = describeIdRange(ids); + const plural = nodes.length === 1 ? 'record' : 'records'; + + const lines = [ + MARKER_BEGIN, + '', + '', + '', + `There are ${nodes.length} ${plural}${range === null ? '' : `, ids ${range}`}, alongside`, + '`0000-template.md`: ' + `${describeStatusCounts(nodes)}.`, + '', + '| Id | Status | Title |', + '| --- | --- | --- |', + ...ordered.map((node) => `| \`${tableCell(node.id)}\` | ${tableCell(node.status)} | ${tableCell(node.title)} |`), + '', + MARKER_END, + ]; + return lines.join('\n'); +} + +/** + * Replace the marked block in `text`. + * + * A marker counts **only as a standalone line**. Counting the substring + * anywhere was a real defect, not a hypothetical one: `title` is free text + * (`z.string().min(3).max(120)`), so a record could carry + * `` in its title, that title would be + * rendered into a table row *inside* the block, and every subsequent run would + * then throw `found 1 and 2` — a corpus record permanently disabling the + * generator that has to render it. Line-anchoring is what makes the marker a + * structural delimiter rather than a string the content can forge; + * {@link tableCell}'s newline handling is the other half, since without it a + * multiline title could put a marker on a line of its own. + * + * Every degenerate marker state throws: a silent no-op here would leave the + * `git diff --exit-code` gate green while generating nothing. + */ +export function replaceGeneratedBlock(text: string, block: string): string { + const begins = [...text.matchAll(standaloneMarkerPattern(MARKER_BEGIN))]; + const ends = [...text.matchAll(standaloneMarkerPattern(MARKER_END))]; + + if (begins.length !== 1 || ends.length !== 1) { + throw new Error( + `MANIFEST.md must contain exactly one ${MARKER_BEGIN} and one ${MARKER_END} ` + + `on lines of their own (found ${begins.length} and ${ends.length}). Without them nothing is ` + + 'generated and the no-diff gate would pass while checking nothing.', + ); + } + + const start = begins[0]!.index; + const end = ends[0]!.index + ends[0]![0].length; + if (start >= end) { + throw new Error(`${MARKER_END} appears before ${MARKER_BEGIN} in MANIFEST.md.`); + } + + return `${text.slice(0, start)}${block}${text.slice(end)}`; +} + +/** + * A marker occupying a whole line, tolerating indentation, trailing whitespace, + * and a CRLF line ending — but never matching one embedded in prose or in a + * table cell. + */ +function standaloneMarkerPattern(marker: string): RegExp { + return new RegExp(String.raw`^[ \t]*${escapeRegExp(marker)}[ \t]*(?=\r?\n|$)`, 'gmu'); +} + +function escapeRegExp(value: string): string { + return value.replaceAll(/[.*+?^${}()|[\]\\]/gu, String.raw`\$&`); +} + +/** Parse and validate `adr graph --format json` output. */ +export function parseGraphNodes(json: string): GraphNode[] { + const parsed: unknown = JSON.parse(json); + const nodes = (parsed as { nodes?: unknown } | null)?.nodes; + if (!Array.isArray(nodes)) { + throw new Error('`adr graph --format json` output has no `nodes` array.'); + } + + return nodes.map((node, index) => { + const { id, title, status } = (node ?? {}) as Record; + if (typeof id !== 'string' || typeof title !== 'string' || typeof status !== 'string') { + throw new Error(`graph node ${index} is missing a string id, title, or status.`); + } + return { id, title, status }; + }); +} + +/** Run the read-only CLI and return its `nodes`. */ +function readCorpusNodes(): GraphNode[] { + const result = Bun.spawnSync({ + cmd: ['bun', join(repoRoot, 'packages', 'cli', 'src', 'index.ts'), 'graph', '--format', 'json'], + cwd: repoRoot, + stdout: 'pipe', + stderr: 'pipe', + }); + + if (result.exitCode !== 0) { + // Exit 1 is a corpus error finding; the graph is still emitted, but an + // inventory generated from a corpus that fails its own lint is not one to + // commit. + throw new Error( + `adr graph --format json exited ${result.exitCode}:\n${new TextDecoder().decode(result.stderr).trim()}`, + ); + } + return parseGraphNodes(new TextDecoder().decode(result.stdout)); +} + +function main(): void { + const manifestPath = join(repoRoot, 'MANIFEST.md'); + const nodes = readCorpusNodes(); + const current = readFileSync(manifestPath, 'utf8'); + const next = replaceGeneratedBlock(current, renderInventoryBlock(nodes)); + + if (next === current) { + console.log(`emit-manifest: ok - MANIFEST.md already matches the corpus (${nodes.length} records)`); + return; + } + writeFileSync(manifestPath, next); + console.log(`emit-manifest: wrote MANIFEST.md inventory for ${nodes.length} records`); +} + +if (import.meta.main) { + try { + main(); + } catch (error) { + console.error(`emit-manifest: ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } +} diff --git a/specs/007-arb-queue/contracts/kernel.md b/specs/007-arb-queue/contracts/kernel.md index c5a5bd95..500f0f14 100644 --- a/specs/007-arb-queue/contracts/kernel.md +++ b/specs/007-arb-queue/contracts/kernel.md @@ -191,7 +191,14 @@ Three findings are generated in Step 4, checked independently for each item: ### `item.tier-absent` -**Condition**: `record.frontmatter.review?.tier` is absent (undefined or null). +**Condition**: +- `record.frontmatter.review?.tier` is absent (undefined or null), AND +- the record has entered review: `record.frontmatter.review` is present, OR + `record.frontmatter.reviewBy` is present + +A `proposed` record with neither a `review` block nor a `reviewBy` is `not-queued` and +generates no finding — the spec's two-conditioned carve-out. A record carrying an +explicit deadline is *not* covered by it (#111). ```typescript { diff --git a/specs/007-arb-queue/research.md b/specs/007-arb-queue/research.md index 5834f797..fac0cd7f 100644 --- a/specs/007-arb-queue/research.md +++ b/specs/007-arb-queue/research.md @@ -134,7 +134,7 @@ have queue-specific incompleteness or data inconsistencies. | Code | Severity | Generation Condition | |------|----------|----------------------| -| `item.tier-absent` | info | `review.tier` is absent (not set in frontmatter); routing tier cannot be determined | +| `item.tier-absent` | info | `review.tier` is absent (not set in frontmatter) AND the record has entered review — `review` is present, or top-level `reviewBy` is present; routing tier cannot be determined. A `proposed` record with neither is `not-queued` and stays silent (#111) | | `item.review-by-before-queued` | warn | `reviewBy` is present AND `review.queuedAt` is present AND the UTC calendar date of `reviewBy` is strictly before the UTC calendar date of `review.queuedAt` | | `item.deciders-empty` | info | `deciders` array is empty AND `review.queuedAt` is present (record is in the queue with no routing targets) | diff --git a/specs/007-arb-queue/spec.md b/specs/007-arb-queue/spec.md index 62867970..d3127e76 100644 --- a/specs/007-arb-queue/spec.md +++ b/specs/007-arb-queue/spec.md @@ -446,8 +446,12 @@ and not as a `QueueItem`. - **`review` block and top-level `reviewBy` both absent on a `proposed` record**: treated as `not-queued`. This is a valid, expected state for records that have been proposed but not yet entered into the review workflow. No item finding is generated - for absence of `review` alone. If `reviewBy` is present without a `review` block, its - explicit deadline still produces `within-sla`, `due`, or `overdue` under FR-005. + when `review` **and** `reviewBy` are both absent — including no `item.tier-absent`. + Both conditions are required: a record carrying an explicit `reviewBy` deadline has + entered the review workflow, so it is outside this carve-out and `item.tier-absent` + fires if its routing tier cannot be determined (#111). If `reviewBy` is present + without a `review` block, its explicit deadline still produces `within-sla`, `due`, + or `overdue` under FR-005. - **Corpus with no `proposed` records**: the queue produces an empty ordered `QueueItem` list with no item findings; the `corpusFindings` list may still be non-empty if