From 15c6fcf974ec3a66e1d14721290ebd3ef7ed626a Mon Sep 17 00:00:00 2001 From: Mark Beacom Date: Wed, 26 Aug 2026 17:37:12 -0400 Subject: [PATCH 1/3] feat: generate the MANIFEST inventory and gate it against the corpus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MANIFEST.md's record inventory is a pure function of docs/adr/, and it was hand-maintained: 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 this generates the block and asserts the tree is unchanged afterwards — the same shape as the schema emit and the committed Action bundle. `scripts/emit-manifest.ts` renders the record table and status counts from `adr graph --format json` between stable markers; `bun run emit:manifest && git diff --exit-code MANIFEST.md` joins the existing guards in `clean-clone-builds`. No public CLI surface is added: the CLI stays read-only and hook-safe, and the writing is a repo-local script (#132). Prose outside the markers — including judgment about what is open — stays hand-written. Every way of generating nothing is an error rather than a silent no-op, since a generator that writes nothing leaves the diff clean and the gate green while checking nothing (ADR-0016): absent, duplicated, or inverted markers and an empty node list all throw. The generator re-sorts nodes with `compareCodeUnits` rather than trusting `buildAdrGraph`'s `localeCompare` order, which ADR-0033 clause 8 pins, so the no-diff gate cannot fail because two contributors ran it under different ICU locales. Also fires `item.tier-absent` when a proposed record has entered review but its routing tier cannot be determined (#111). The spec's carve-out is two-conditioned — `review` block *and* top-level `reviewBy` both absent, on the reasoning that the record has "not yet entered into the review workflow" — but only the first condition was implemented. ADR-0022 therefore sat in the queue as cross-team with `reviewBy: 2027-02-08`, `tier=None`, and no finding, and was ratified without a routing tier. spec.md, research.md, and contracts/kernel.md now state the same rule as the code. Severity stays `info`, so no exit code changes, and a proposed record with neither field stays silent. Observed failing before being relied on, per ADR-0016: the queue case against the pre-fix kernel, and the marker, empty-corpus, and stale-inventory cases as permanent negative tests. Closes #131 Closes #111 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Mark Beacom --- .github/workflows/ci.yml | 7 + CHANGELOG.md | 26 +++ MANIFEST.md | 56 ++++- docs/RELEASING.md | 13 +- package.json | 1 + packages/core/src/queue/kernel.ts | 21 +- packages/core/test/ordering-contract.test.ts | 28 ++- packages/core/test/queue/kernel.test.ts | 49 ++++ scripts/emit-manifest.test.ts | 219 +++++++++++++++++ scripts/emit-manifest.ts | 234 +++++++++++++++++++ specs/007-arb-queue/contracts/kernel.md | 9 +- specs/007-arb-queue/research.md | 2 +- specs/007-arb-queue/spec.md | 8 +- 13 files changed, 656 insertions(+), 17 deletions(-) create mode 100644 scripts/emit-manifest.test.ts create mode 100644 scripts/emit-manifest.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d501157..b642229e 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 19cf970a..4b277db1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,32 @@ Until `1.0.0`, minor releases may include breaking changes ## [Unreleased] +### 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. + ## [0.11.0] - 2026-08-26 ### Added diff --git a/MANIFEST.md b/MANIFEST.md index db22f209..1238571d 100644 --- a/MANIFEST.md +++ b/MANIFEST.md @@ -56,9 +56,61 @@ adrkit/ - The ADR corpus lives in [docs/adr/](docs/adr/), with `0000-template.md` plus numbered records. -- There are 35 files: the template plus 34 records, ids `0001`-`0034`, with 32 - 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 34 records, ids `0001`-`0034`, alongside +`0000-template.md`: 32 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 | + + diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 9b14092e..50c4da6f 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 23d2ab19..14d8f628 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:ci-comment": "bun run scripts/check-ci-comment.ts", "audit:gate": "bun run scripts/audit-gate.ts", 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..b6069055 --- /dev/null +++ b/scripts/emit-manifest.test.ts @@ -0,0 +1,219 @@ +/** + * `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('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('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..28431543 --- /dev/null +++ b/scripts/emit-manifest.ts @@ -0,0 +1,234 @@ +/** + * 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(', '); +} + +/** Markdown table cells cannot carry a raw `|`, and titles are free text. */ +function tableCell(value: string): string { + return value.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`. + * + * 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(new RegExp(escapeRegExp(MARKER_BEGIN), 'gu'))]; + const ends = [...text.matchAll(new RegExp(escapeRegExp(MARKER_END), 'gu'))]; + + if (begins.length !== 1 || ends.length !== 1) { + throw new Error( + `MANIFEST.md must contain exactly one ${MARKER_BEGIN} and one ${MARKER_END} ` + + `(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 + MARKER_END.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)}`; +} + +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 From bd54074c4f003e0016f50e2c7a6664271b42adf2 Mon Sep 17 00:00:00 2001 From: Mark Beacom Date: Wed, 26 Aug 2026 17:43:36 -0400 Subject: [PATCH 2/3] build: rebuild the committed queue Action bundle for the tier-absent fix `packages/ci/dist/queue-action.js` embeds the queue kernel, so the #111 change to `computeItemFindings` has to be rebuilt into it. Caught by the existing `git diff --exit-code packages/ci/dist` gate rather than by review, which is the gate working. Built with the repository-pinned Bun 1.3.14; a newer bundler rewrites unrelated runtime helpers throughout both bundles and would fail the same gate in CI. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Mark Beacom --- packages/ci/dist/queue-action.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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) { From 875282291338698890f1a522c33cad9702be096b Mon Sep 17 00:00:00 2001 From: Mark Beacom Date: Wed, 26 Aug 2026 17:54:45 -0400 Subject: [PATCH 3/3] fix: make the MANIFEST markers unforgeable by corpus content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found in review of #177, both of which let a *valid* ADR record break or corrupt the generated block it is rendered into. `replaceGeneratedBlock` counted marker substrings anywhere in the file. `title` is `z.string().min(3).max(120)` — free text long enough to hold `` — so a record carrying a marker in its title would have it rendered into a table row inside the block, and every later run would throw `found 1 and 2`. One corpus record could permanently disable the generator whose job is to render it, and the fix would have been to edit the record. Markers are now recognized only as standalone lines, tolerating indentation, trailing whitespace, and CRLF, so a marker in prose or in a table cell is content rather than a delimiter. `tableCell` escaped `\` and `|` but not CR/LF. A YAML block scalar satisfies the title schema with an embedded newline, and a raw newline splits one table row into two lines of markdown — which regeneration then reproduces byte-for-byte, so the no-diff gate stays green over a broken table, and a multiline title could place a marker on a line of its own. It now follows `@adrkit/core`'s queue formatter exactly (`packages/core/src/queue/format.ts` `escapeCell`): CRLF to LF, CR to space, LF to `
`, then `\`, `|`, and a backtick, with the backslash escaped first so it cannot combine with an introduced escape. The two halves compose: newline normalization means content can never start a line, and line-anchoring means an inline occurrence is never a marker. Observed failing before being relied on (ADR-0016): the five new cases fail against the pre-fix script — marker in a cell, marker in prose, multiline title, forged marker line, and backtick escaping — and pass after. A sixth asserts a genuinely indented, trailing-whitespace, CRLF marker is still recognized, so the new anchoring cannot silently become too strict. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Mark Beacom --- scripts/emit-manifest.test.ts | 64 +++++++++++++++++++++++++++++++++++ scripts/emit-manifest.ts | 52 ++++++++++++++++++++++++---- 2 files changed, 109 insertions(+), 7 deletions(-) diff --git a/scripts/emit-manifest.test.ts b/scripts/emit-manifest.test.ts index b6069055..0d8141e2 100644 --- a/scripts/emit-manifest.test.ts +++ b/scripts/emit-manifest.test.ts @@ -124,6 +124,37 @@ describe('renderInventoryBlock', () => { 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". @@ -165,6 +196,39 @@ describe('replaceGeneratedBlock', () => { 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, diff --git a/scripts/emit-manifest.ts b/scripts/emit-manifest.ts index 28431543..28794cfe 100644 --- a/scripts/emit-manifest.ts +++ b/scripts/emit-manifest.ts @@ -90,9 +90,27 @@ export function describeStatusCounts(nodes: readonly GraphNode[]): string { .join(', '); } -/** Markdown table cells cannot carry a raw `|`, and titles are free text. */ +/** + * 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('\\', '\\\\').replaceAll('|', '\\|'); + return value + .replaceAll('\r\n', '\n') + .replaceAll('\r', ' ') + .replaceAll('\n', '
') + .replaceAll('\\', '\\\\') + .replaceAll('|', '\\|') + .replaceAll('`', '\\`'); } /** @@ -145,23 +163,34 @@ export function renderInventoryBlock(nodes: readonly GraphNode[]): string { /** * 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(new RegExp(escapeRegExp(MARKER_BEGIN), 'gu'))]; - const ends = [...text.matchAll(new RegExp(escapeRegExp(MARKER_END), 'gu'))]; + 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} ` + - `(found ${begins.length} and ${ends.length}). Without them nothing is generated and the ` + - 'no-diff gate would pass while checking nothing.', + `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 + MARKER_END.length; + const end = ends[0]!.index + ends[0]![0].length; if (start >= end) { throw new Error(`${MARKER_END} appears before ${MARKER_BEGIN} in MANIFEST.md.`); } @@ -169,6 +198,15 @@ export function replaceGeneratedBlock(text: string, block: string): string { 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`\$&`); }