From 6e6a8b25f3d6fe41cb34c089d5bf6b2aa9aec86b Mon Sep 17 00:00:00 2001 From: Mark Beacom Date: Sat, 10 Oct 2026 17:38:45 -0400 Subject: [PATCH 1/2] fix(core,evaluator): treat log-qualified relatesTo/conflictsWith as inert in lint, and cross-repository supersession as an error everywhere (#299) The schema accepts `:` references, but `adr lint` failed a qualified `relatesTo` as `dangling-relatesTo` (error) while the evaluator, the marker resolver, and MCP all treated it as inert. Under ADR-0059 clause 5 (which amends ADR-0031 clause 5 for this case), lint now reports a qualified `relatesTo` or `conflictsWith` as `federated-ref-unresolved` at `info`. Only the `relatesTo` case lowers an exit code; a dangling `conflictsWith` was already `warn`. A qualified reference never resolves to a local record with the same bare id. ADR-0059 clause 4 keeps cross-repository supersession a dangling-* error everywhere. Lint already failed it and now says why. `adr evaluate` treated a qualified `supersedes` as inert (or passing, with a snapshot) and now fails it as `no-orphan-refs.dangling-supersedes`, naming the clause; a reference qualified with the evaluator's own resolution log still counts as local. packages/ci/dist is rebuilt under linux/amd64 Bun 1.4.2 for the core change. Every new test was observed failing first (ADR-0016). ADR-0059 action item 2 is ticked. Signed-off-by: Mark Beacom --- CHANGELOG.md | 29 ++++++++ docs/RELEASING.md | 10 ++- ...nization-s-repositories-as-a-pinned-rea.md | 2 +- packages/ci/dist/index.js | 24 ++++++- packages/ci/dist/portal-action.js | 24 ++++++- packages/ci/dist/queue-action.js | 31 ++++++++- packages/cli/test/lint.test.ts | 65 ++++++++++++++++++ .../core/src/validate/corpus-invariants.ts | 37 +++++++++- packages/core/test/corpus-invariants.test.ts | 68 +++++++++++++++++++ .../evaluator/src/rules/no-orphan-refs.ts | 18 ++++- .../src/rules/supersession-consistent.ts | 6 +- .../evaluator/test/no-orphan-refs.test.ts | 47 ++++++++++++- .../test/supersession-consistent.test.ts | 23 +++++++ 13 files changed, 362 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e562662..3833be76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,35 @@ Until `1.0.0`, minor releases may include breaking changes ([ADR-0054](docs/adr/0054-publish-a-static-decision-portal-for-github-pages-built-outside-the-cli-from-the.md) action item 8, #294). +- **Behavior change: `adr lint` no longer fails on a log-qualified `relatesTo` + or `conflictsWith`.** A reference such as `relatesTo: ["payments:0012"]` names + a record in another decision log. The schema has always accepted it, but the + corpus gate reported it as `dangling-relatesTo` (`error`, exit `1`) or + `dangling-conflictsWith` (`warn`), while the evaluator, the marker resolver, + and the MCP server already treated it as inert. `adr lint` and `adr lint + --json` now report it as `federated-ref-unresolved` at `info`, so the + surfaces agree ([#299](https://github.com/mbeacom/adrkit/issues/299)). This is + the one non-additive change + [ADR-0059](docs/adr/0059-aggregate-decision-corpora-across-an-organization-s-repositories-as-a-pinned-rea.md) + clause 5 makes, and it amends + [ADR-0031](docs/adr/0031-publish-a-narrow-consumer-sdk-as-the-contract-and-document-the-cli-json-as-its-s.md) + clause 5's additive-only CLI JSON contract for exactly this case: the finding + is renamed and its severity lowered. Only the `relatesTo` case lowers an exit + code, from `1` to `0`; a dangling `conflictsWith` was already a warning. + Measured on a one-record fixture, that moves `adr lint` and `adr explain` + from exit `1` to `0`; `adr check` and `adr queue` already exited `0` there. + Everything that reads lint findings, the Actions included, sees the new code. + A qualified reference never resolves + against a local record with the same bare id. An unqualified dangling + reference is unchanged, and a log-qualified `supersedes` or `supersededBy` + stays a `dangling-*` error, now with a message saying cross-repository + supersession is not supported (ADR-0059 clause 4). `adr evaluate` now agrees: + a log-qualified `supersedes` fails `no-orphan-refs` as + `no-orphan-refs.dangling-supersedes` even when a snapshot of that log is + supplied (before, it was inert without a snapshot and passed when the snapshot + listed the id), and a log-qualified `supersededBy` names clause 4 in its + existing failure. + ## [0.19.0] - 2026-10-10 This release ships the decision portal: a third, build-only nested Action, diff --git a/docs/RELEASING.md b/docs/RELEASING.md index b1d3c7bf..fb66c333 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -81,9 +81,13 @@ The public surface is: Patch releases must be backward compatible: no removals, renames, exit-code reclassification, or incompatible output/type-shape changes. Minor releases may carry breaking changes before `1.0.0`, but the release notes must call them out -explicitly. Adding a runtime export is not breaking, but it intentionally expands -the API: update the matching surface test and mention the addition in the -release notes. Any `exports` map or runtime named-export change that is not +explicitly. The CLI JSON is additive-only under ADR-0031 clause 5, with one +amendment: under ADR-0059 clause 5, `adr lint --json` reports a log-qualified +`relatesTo` or `conflictsWith` (`payments:0012`) as `federated-ref-unresolved` +at `info`, not `dangling-relatesTo` (`error`) or `dangling-conflictsWith` +(`warn`), so only the qualified `relatesTo` case lowers an exit code. Adding a +runtime export is not breaking, but it intentionally expands the API: update +the matching surface test and mention the addition in the release notes. Any `exports` map or runtime named-export change that is not accompanied by a surface-test update is a release blocker. `QueueReport.totalItems` carries an obligation the surface tests cannot see. diff --git a/docs/adr/0059-aggregate-decision-corpora-across-an-organization-s-repositories-as-a-pinned-rea.md b/docs/adr/0059-aggregate-decision-corpora-across-an-organization-s-repositories-as-a-pinned-rea.md index 47969d10..114cbc8e 100644 --- a/docs/adr/0059-aggregate-decision-corpora-across-an-organization-s-repositories-as-a-pinned-rea.md +++ b/docs/adr/0059-aggregate-decision-corpora-across-an-organization-s-repositories-as-a-pinned-rea.md @@ -464,7 +464,7 @@ rung 3 is absent. 1. [ ] Define the manifest in Zod, emit `schema/federation.schema.json`, and add the manifest validator (unique `log` and `repo`, required `visibility`, no unknown keys). -2. [ ] Change `validateCorpusInvariants` per clause 5, with a CHANGELOG +2. [x] Change `validateCorpusInvariants` per clause 5, with a CHANGELOG behavior-change entry and a test observed failing first. 3. [ ] Add `packages/ci/federation/action.yml`: fetch per member, write the tree aside, write the sidecar last, swap, write the lock, and check diff --git a/packages/ci/dist/index.js b/packages/ci/dist/index.js index 02cf5cf9..8950feb0 100644 --- a/packages/ci/dist/index.js +++ b/packages/ci/dist/index.js @@ -39374,15 +39374,27 @@ function validateParsedAdr(parsed) { } // ../core/src/validate/corpus-invariants.ts function addReferenceFinding(findings, record, field, ref, severity) { + const crossLog = parseAdrRef(ref).log !== undefined; findings.push({ rule: `dangling-${field}`, severity, - message: `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, + message: crossLog ? `Reference "${ref}" in ${field} names another decision log; cross-repository supersession is not supported` : `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, path: record.path, id: record.frontmatter.id, field }); } +function addFederatedReferenceFinding(findings, record, field, ref, log) { + findings.push({ + rule: "federated-ref-unresolved", + severity: "info", + message: `Reference "${ref}" in ${field} names another decision log ("${log}") and is inert against this corpus`, + path: record.path, + id: record.frontmatter.id, + field, + pattern: ref + }); +} function validateCorpusInvariants(records) { const findings = []; const byId = new Map; @@ -39416,11 +39428,17 @@ function validateCorpusInvariants(records) { addReferenceFinding(findings, record, "supersededBy", record.frontmatter.supersededBy, "error"); } for (const ref of record.frontmatter.relatesTo) { - if (!ids.has(ref)) + const { log } = parseAdrRef(ref); + if (log !== undefined) + addFederatedReferenceFinding(findings, record, "relatesTo", ref, log); + else if (!ids.has(ref)) addReferenceFinding(findings, record, "relatesTo", ref, "error"); } for (const ref of record.frontmatter.conflictsWith) { - if (!ids.has(ref)) { + const { log } = parseAdrRef(ref); + if (log !== undefined) { + addFederatedReferenceFinding(findings, record, "conflictsWith", ref, log); + } else if (!ids.has(ref)) { addReferenceFinding(findings, record, "conflictsWith", ref, "warn"); } else if (record.frontmatter.status === "accepted") { findings.push({ diff --git a/packages/ci/dist/portal-action.js b/packages/ci/dist/portal-action.js index 6beaa977..47df156d 100644 --- a/packages/ci/dist/portal-action.js +++ b/packages/ci/dist/portal-action.js @@ -41128,15 +41128,27 @@ function validateParsedAdr(parsed) { } // ../core/src/validate/corpus-invariants.ts function addReferenceFinding(findings, record, field, ref, severity) { + const crossLog = parseAdrRef(ref).log !== undefined; findings.push({ rule: `dangling-${field}`, severity, - message: `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, + message: crossLog ? `Reference "${ref}" in ${field} names another decision log; cross-repository supersession is not supported` : `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, path: record.path, id: record.frontmatter.id, field }); } +function addFederatedReferenceFinding(findings, record, field, ref, log) { + findings.push({ + rule: "federated-ref-unresolved", + severity: "info", + message: `Reference "${ref}" in ${field} names another decision log ("${log}") and is inert against this corpus`, + path: record.path, + id: record.frontmatter.id, + field, + pattern: ref + }); +} function validateCorpusInvariants(records) { const findings = []; const byId = new Map; @@ -41170,11 +41182,17 @@ function validateCorpusInvariants(records) { addReferenceFinding(findings, record, "supersededBy", record.frontmatter.supersededBy, "error"); } for (const ref of record.frontmatter.relatesTo) { - if (!ids.has(ref)) + const { log } = parseAdrRef(ref); + if (log !== undefined) + addFederatedReferenceFinding(findings, record, "relatesTo", ref, log); + else if (!ids.has(ref)) addReferenceFinding(findings, record, "relatesTo", ref, "error"); } for (const ref of record.frontmatter.conflictsWith) { - if (!ids.has(ref)) { + const { log } = parseAdrRef(ref); + if (log !== undefined) { + addFederatedReferenceFinding(findings, record, "conflictsWith", ref, log); + } else if (!ids.has(ref)) { addReferenceFinding(findings, record, "conflictsWith", ref, "warn"); } else if (record.frontmatter.status === "accepted") { findings.push({ diff --git a/packages/ci/dist/queue-action.js b/packages/ci/dist/queue-action.js index 3b45b696..aa7f5526 100644 --- a/packages/ci/dist/queue-action.js +++ b/packages/ci/dist/queue-action.js @@ -37408,6 +37408,13 @@ var AdrFrontmatter = strictObject2({ }); // ../core/src/schema/emit.ts var ADR_SCHEMA_ID = `https://adrkit.dev/schema/adr/v${SCHEMA_VERSION}/adr.schema.json`; +// ../core/src/schema/ref.ts +function parseAdrRef(ref) { + const idx = ref.indexOf(":"); + if (idx <= 0) + return { id: ref }; + return { log: ref.slice(0, idx), id: ref.slice(idx + 1) }; +} // ../../node_modules/.bun/yaml@2.9.1/node_modules/yaml/dist/index.js var composer = require_composer(); var Document = require_Document(); @@ -37696,15 +37703,27 @@ function validateParsedAdr(parsed) { } // ../core/src/validate/corpus-invariants.ts function addReferenceFinding(findings, record, field, ref, severity) { + const crossLog = parseAdrRef(ref).log !== undefined; findings.push({ rule: `dangling-${field}`, severity, - message: `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, + message: crossLog ? `Reference "${ref}" in ${field} names another decision log; cross-repository supersession is not supported` : `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, path: record.path, id: record.frontmatter.id, field }); } +function addFederatedReferenceFinding(findings, record, field, ref, log) { + findings.push({ + rule: "federated-ref-unresolved", + severity: "info", + message: `Reference "${ref}" in ${field} names another decision log ("${log}") and is inert against this corpus`, + path: record.path, + id: record.frontmatter.id, + field, + pattern: ref + }); +} function validateCorpusInvariants(records) { const findings = []; const byId = new Map; @@ -37738,11 +37757,17 @@ function validateCorpusInvariants(records) { addReferenceFinding(findings, record, "supersededBy", record.frontmatter.supersededBy, "error"); } for (const ref of record.frontmatter.relatesTo) { - if (!ids.has(ref)) + const { log } = parseAdrRef(ref); + if (log !== undefined) + addFederatedReferenceFinding(findings, record, "relatesTo", ref, log); + else if (!ids.has(ref)) addReferenceFinding(findings, record, "relatesTo", ref, "error"); } for (const ref of record.frontmatter.conflictsWith) { - if (!ids.has(ref)) { + const { log } = parseAdrRef(ref); + if (log !== undefined) { + addFederatedReferenceFinding(findings, record, "conflictsWith", ref, log); + } else if (!ids.has(ref)) { addReferenceFinding(findings, record, "conflictsWith", ref, "warn"); } else if (record.frontmatter.status === "accepted") { findings.push({ diff --git a/packages/cli/test/lint.test.ts b/packages/cli/test/lint.test.ts index a7ffc85c..2c12b589 100644 --- a/packages/cli/test/lint.test.ts +++ b/packages/cli/test/lint.test.ts @@ -58,6 +58,71 @@ describe('adr lint CLI', () => { expect(result.stderr).toContain('superseded-requires-supersededBy'); }); + // ADR-0059 clause 5, amending ADR-0031 clause 5: a log-qualified relatesTo or + // conflictsWith is inert against a single corpus, so it no longer fails the gate. + test('exits 0 on a qualified relatesTo and reports it at info in --json', async () => { + const root = await resetTestDir(DIR_NAME); + await writeText( + join(root, 'docs/adr/0001-federated.md'), + recordMarkdown('0001').replace('relatesTo: []', 'relatesTo: ["payments:0012"]'), + ); + + const human = await runAdr(['lint', '--dir', join(root, 'docs/adr')]); + expect(human.exitCode).toBe(0); + expect(human.stdout).toContain('checked 1 records, 0 errors, 0 warnings'); + expect(human.stderr).toContain('federated-ref-unresolved'); + expect(human.stderr).not.toContain('dangling-relatesTo'); + + const json = await runAdr(['lint', '--json', '--dir', join(root, 'docs/adr')]); + expect(json.exitCode).toBe(0); + const report = JSON.parse(json.stdout) as { findings: Array> }; + expect(report.findings).toMatchObject([ + { rule: 'federated-ref-unresolved', severity: 'info', id: '0001', field: 'relatesTo', pattern: 'payments:0012' }, + ]); + }); + + test('reports a qualified conflictsWith at info instead of warn', async () => { + const root = await resetTestDir(DIR_NAME); + await writeText( + join(root, 'docs/adr/0001-federated.md'), + recordMarkdown('0001', undefined, 'conflictsWith: ["payments:0012"]\n'), + ); + + const result = await runAdr(['lint', '--json', '--dir', join(root, 'docs/adr')]); + expect(result.exitCode).toBe(0); + const report = JSON.parse(result.stdout) as { findings: Array> }; + expect(report.findings).toMatchObject([ + { rule: 'federated-ref-unresolved', severity: 'info', field: 'conflictsWith' }, + ]); + }); + + test('still exits 1 on a qualified supersedes, which clause 4 keeps out', async () => { + const root = await resetTestDir(DIR_NAME); + await writeText( + join(root, 'docs/adr/0001-federated.md'), + recordMarkdown('0001', undefined, 'supersedes: ["payments:0012"]\n'), + ); + + const result = await runAdr(['lint', '--dir', join(root, 'docs/adr')]); + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain('checked 1 records, 1 errors'); + expect(result.stderr).toContain('dangling-supersedes'); + expect(result.stderr).toContain('cross-repository supersession is not supported'); + }); + + test('still exits 1 on an unqualified dangling relatesTo', async () => { + const root = await resetTestDir(DIR_NAME); + await writeText( + join(root, 'docs/adr/0001-local.md'), + recordMarkdown('0001').replace('relatesTo: []', 'relatesTo: ["0012"]'), + ); + + const result = await runAdr(['lint', '--json', '--dir', join(root, 'docs/adr')]); + expect(result.exitCode).toBe(1); + const report = JSON.parse(result.stdout) as { findings: Array> }; + expect(report.findings).toMatchObject([{ rule: 'dangling-relatesTo', severity: 'error' }]); + }); + test('reports a missing explicit path as a finding instead of crashing', async () => { const root = await resetTestDir(DIR_NAME); diff --git a/packages/core/src/validate/corpus-invariants.ts b/packages/core/src/validate/corpus-invariants.ts index c0505562..a3695023 100644 --- a/packages/core/src/validate/corpus-invariants.ts +++ b/packages/core/src/validate/corpus-invariants.ts @@ -1,4 +1,5 @@ import type { Adr } from '../schema/adr.schema.ts'; +import { parseAdrRef } from '../schema/ref.ts'; import type { Finding, FindingSeverity } from './findings.ts'; function addReferenceFinding( @@ -8,16 +9,41 @@ function addReferenceFinding( ref: string, severity: FindingSeverity, ): void { + // ADR-0059 clause 4: a log-qualified supersession target stays a dangling error, and the + // message says why, so nobody reads it as a typo to fix in this corpus. + const crossLog = parseAdrRef(ref).log !== undefined; findings.push({ rule: `dangling-${field}`, severity, - message: `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, + message: crossLog + ? `Reference "${ref}" in ${field} names another decision log; cross-repository supersession is not supported` + : `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, path: record.path, id: record.frontmatter.id, field, }); } +/** + * A log-qualified `relatesTo` or `conflictsWith` (`payments:0012`) names a record in another + * decision log, which a single corpus cannot judge. ADR-0059 clause 5 (amending ADR-0031 + * clause 5) reports it at `info`, as the marker resolver's `marker-unresolvable`, the + * evaluator's `no-orphan-refs.federated-log-absent`, and MCP's `federated-log-unavailable` + * already treat the same input: inert here, not broken. It never resolves against a local + * record with the same bare id. + */ +function addFederatedReferenceFinding(findings: Finding[], record: Adr, field: string, ref: string, log: string): void { + findings.push({ + rule: 'federated-ref-unresolved', + severity: 'info', + message: `Reference "${ref}" in ${field} names another decision log ("${log}") and is inert against this corpus`, + path: record.path, + id: record.frontmatter.id, + field, + pattern: ref, + }); +} + export function validateCorpusInvariants(records: readonly Adr[]): Finding[] { const findings: Finding[] = []; const byId = new Map(); @@ -54,11 +80,16 @@ export function validateCorpusInvariants(records: readonly Adr[]): Finding[] { } for (const ref of record.frontmatter.relatesTo) { - if (!ids.has(ref)) addReferenceFinding(findings, record, 'relatesTo', ref, 'error'); + const { log } = parseAdrRef(ref); + if (log !== undefined) addFederatedReferenceFinding(findings, record, 'relatesTo', ref, log); + else if (!ids.has(ref)) addReferenceFinding(findings, record, 'relatesTo', ref, 'error'); } for (const ref of record.frontmatter.conflictsWith) { - if (!ids.has(ref)) { + const { log } = parseAdrRef(ref); + if (log !== undefined) { + addFederatedReferenceFinding(findings, record, 'conflictsWith', ref, log); + } else if (!ids.has(ref)) { addReferenceFinding(findings, record, 'conflictsWith', ref, 'warn'); } else if (record.frontmatter.status === 'accepted') { findings.push({ diff --git a/packages/core/test/corpus-invariants.test.ts b/packages/core/test/corpus-invariants.test.ts index 92492694..33eaa1f3 100644 --- a/packages/core/test/corpus-invariants.test.ts +++ b/packages/core/test/corpus-invariants.test.ts @@ -83,6 +83,74 @@ describe('corpus invariants', () => { ]); }); + describe('log-qualified references (ADR-0059 clauses 4 and 5)', () => { + test('a qualified relatesTo is federated-ref-unresolved at info, not a dangling error', () => { + const findings = validateCorpusInvariants([record('0001', { relatesTo: ['payments:0012'] })]); + expect(findings).toEqual([ + { + rule: 'federated-ref-unresolved', + severity: 'info', + message: + 'Reference "payments:0012" in relatesTo names another decision log ("payments") and is inert against this corpus', + path: 'docs/adr/0001-test.md', + id: '0001', + field: 'relatesTo', + pattern: 'payments:0012', + }, + ]); + }); + + test('a qualified conflictsWith is federated-ref-unresolved at info, not a dangling warning', () => { + const findings = validateCorpusInvariants([record('0001', { conflictsWith: ['payments:0012'] })]); + expect(findings).toMatchObject([ + { rule: 'federated-ref-unresolved', severity: 'info', field: 'conflictsWith', pattern: 'payments:0012' }, + ]); + }); + + test('a qualified conflictsWith on an accepted record never reads as a known local conflict', () => { + const findings = validateCorpusInvariants([ + record('0012'), + record('0001', { status: 'accepted', deciders: ['@mbeacom'], conflictsWith: ['payments:0012'] }), + ]); + expect(findings.map((finding) => [finding.rule, finding.severity])).toEqual([ + ['federated-ref-unresolved', 'info'], + ]); + }); + + test('a qualified reference never resolves to a local record with the same bare id', () => { + const findings = validateCorpusInvariants([ + record('0012'), + record('0001', { relatesTo: ['payments:0012'] }), + ]); + expect(findings).toMatchObject([{ rule: 'federated-ref-unresolved', severity: 'info', id: '0001' }]); + }); + + test('a qualified supersedes or supersededBy stays a dangling error', () => { + const findings = validateCorpusInvariants([ + record('0001', { status: 'superseded', supersededBy: 'payments:0012' }), + record('0002', { supersedes: ['payments:0013'] }), + ]); + expect(findings.map((finding) => [finding.rule, finding.severity])).toEqual([ + ['dangling-supersededBy', 'error'], + ['dangling-supersedes', 'error'], + ]); + for (const finding of findings) { + expect(finding.message).toContain('cross-repository supersession is not supported'); + } + }); + + test('an unqualified dangling reference is unchanged', () => { + const findings = validateCorpusInvariants([record('0001', { relatesTo: ['9999'], conflictsWith: ['9998'] })]); + expect(findings.map((finding) => [finding.rule, finding.severity])).toEqual([ + ['dangling-relatesTo', 'error'], + ['dangling-conflictsWith', 'warn'], + ]); + for (const finding of findings) { + expect(finding.message).toContain('does not resolve to a record in the corpus'); + } + }); + }); + test('clean corpus yields zero errors', () => { const findings = validateCorpusInvariants([ record('0001'), diff --git a/packages/evaluator/src/rules/no-orphan-refs.ts b/packages/evaluator/src/rules/no-orphan-refs.ts index 3de780c6..aa971cd2 100644 --- a/packages/evaluator/src/rules/no-orphan-refs.ts +++ b/packages/evaluator/src/rules/no-orphan-refs.ts @@ -3,7 +3,8 @@ * * Local `supersedes` / `relatesTo` targets must resolve. A federated ref (`:`) * resolves against a supplied federated-log snapshot; a federated ref whose log has NO - * snapshot is inert (`federated-log-absent`), never an orphan failure (C2). `supersededBy` + * snapshot is inert (`federated-log-absent`), never an orphan failure (C2), except that a + * `supersedes` naming another log is always `dangling-supersedes` (ADR-0059 clause 4). `supersededBy` * is owned by supersession-consistent and is not re-reported here. Exactly one aggregate * result (C11); status precedence keeps a dangling failure above a federated inert. */ @@ -16,6 +17,12 @@ import type { ReasonCode, RuleFinding, RuleResult } from '../types.ts'; type RefField = 'supersedes' | 'relatesTo'; +/** True when `ref` is qualified with a log other than the one being evaluated. */ +export function isCrossLog(ref: string, resolutionLog: string | undefined): boolean { + const { log } = parseAdrRef(ref); + return log !== undefined && log !== resolutionLog; +} + export function evaluateNoOrphanRefs(ctx: RuleContext): RuleResult { const resolutionLog = ctx.input.resolutionLog; @@ -54,7 +61,10 @@ export function evaluateNoOrphanRefs(ctx: RuleContext): RuleResult { field === 'supersedes' ? 'no-orphan-refs.dangling-supersedes' : 'no-orphan-refs.dangling-relates-to'; const lowerRule = field === 'supersedes' ? 'dangling-supersedes' : 'dangling-relatesTo'; for (const ref of refs) { - const outcome = classify(ref); + // ADR-0059 clause 4: a `supersedes` naming another log is refused, snapshot or not, + // so a federated log never makes cross-repository supersession resolve. + const crossLog = field === 'supersedes' && isCrossLog(ref, resolutionLog); + const outcome = crossLog ? 'dangling' : classify(ref); if (outcome === 'resolved') continue; if (outcome === 'federated-absent') { const finding: RuleFinding = { @@ -70,7 +80,9 @@ export function evaluateNoOrphanRefs(ctx: RuleContext): RuleResult { } const finding: RuleFinding = { reason: danglingReason, - message: `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, + message: crossLog + ? `Reference "${ref}" in ${field} names another decision log; cross-repository supersession is not supported (ADR-0059 clause 4)` + : `Reference "${ref}" in ${field} does not resolve to a record in the corpus`, adr: record.frontmatter.id, candidateAdr: record.frontmatter.id, relatedAdr: ref, diff --git a/packages/evaluator/src/rules/supersession-consistent.ts b/packages/evaluator/src/rules/supersession-consistent.ts index c7c4090f..831a4971 100644 --- a/packages/evaluator/src/rules/supersession-consistent.ts +++ b/packages/evaluator/src/rules/supersession-consistent.ts @@ -10,6 +10,7 @@ import { buildAdrGraph, type Adr } from '@adrkit/core'; import { aggregate, passResult, type SubResult } from './kernel.ts'; +import { isCrossLog } from './no-orphan-refs.ts'; import type { RuleContext } from './context.ts'; import type { RuleFinding, RuleResult } from '../types.ts'; @@ -66,7 +67,10 @@ export function evaluateSupersessionConsistent(ctx: RuleContext): RuleResult { danglingRefs.add(`${record.frontmatter.id}->${target}`); const finding: RuleFinding = { reason: 'supersession-consistent.dangling-superseded-by', - message: `supersededBy "${target}" does not resolve to a record in the corpus`, + // ADR-0059 clause 4: a log-qualified successor is never resolved, snapshot or not. + message: isCrossLog(target, ctx.input.resolutionLog) + ? `supersededBy "${target}" names another decision log; cross-repository supersession is not supported (ADR-0059 clause 4)` + : `supersededBy "${target}" does not resolve to a record in the corpus`, adr: record.frontmatter.id, candidateAdr: record.frontmatter.id, relatedAdr: target, diff --git a/packages/evaluator/test/no-orphan-refs.test.ts b/packages/evaluator/test/no-orphan-refs.test.ts index cdd61370..52f0a729 100644 --- a/packages/evaluator/test/no-orphan-refs.test.ts +++ b/packages/evaluator/test/no-orphan-refs.test.ts @@ -61,8 +61,8 @@ describe('no-orphan-refs', () => { expect(ruleResult(report, 'no-orphan-refs').reason).toBe('no-orphan-refs.dangling-relates-to'); }); - test('a federated ref with NO snapshot is inert (federated-log-absent), not an orphan', () => { - const proposal = record({ id: '0002', supersedes: ['payments:0012'] }, { path: 'docs/adr/0002-a.md' }); + test('a federated relatesTo with NO snapshot is inert (federated-log-absent), not an orphan', () => { + const proposal = record({ id: '0002', relatesTo: ['payments:0012'] }, { path: 'docs/adr/0002-a.md' }); const report = evaluateReport( baseInput({ corpus: corpusOf([proposal]), proposalPath: 'docs/adr/0002-a.md' }), ); @@ -71,4 +71,47 @@ describe('no-orphan-refs', () => { expect(result.reason).toBe('no-orphan-refs.federated-log-absent'); expect(report.outcome).toBe('ok'); }); + + // ADR-0059 clause 4: cross-repository supersession stays a dangling error everywhere, + // with or without a snapshot of the named log. + test('a log-qualified supersedes with NO snapshot fails as dangling-supersedes', () => { + const proposal = record({ id: '0002', supersedes: ['payments:0012'] }, { path: 'docs/adr/0002-a.md' }); + const report = evaluateReport( + baseInput({ corpus: corpusOf([proposal]), proposalPath: 'docs/adr/0002-a.md' }), + ); + const result = ruleResult(report, 'no-orphan-refs'); + expect(result.status).toBe('fail'); + expect(result.severity).toBe('error'); + expect(result.reason).toBe('no-orphan-refs.dangling-supersedes'); + expect(result.findings).toHaveLength(1); + expect(result.findings[0]?.message).toContain('ADR-0059 clause 4'); + expect(result.findings[0]?.lowerLevel).toMatchObject({ rule: 'dangling-supersedes', field: 'supersedes' }); + }); + + test('a log-qualified supersedes still fails when a snapshot of that log has the id', () => { + const proposal = record({ id: '0002', supersedes: ['payments:0012'] }, { path: 'docs/adr/0002-a.md' }); + const report = evaluateReport( + baseInput({ + corpus: corpusOf([proposal]), + proposalPath: 'docs/adr/0002-a.md', + federatedLogs: [{ log: 'payments', adrIds: ['0012'] }], + }), + ); + const result = ruleResult(report, 'no-orphan-refs'); + expect(result.status).toBe('fail'); + expect(result.reason).toBe('no-orphan-refs.dangling-supersedes'); + expect(result.findings[0]?.message).toContain('ADR-0059 clause 4'); + }); + + test('a supersedes qualified with the resolution log itself is a local reference', () => { + const proposal = record({ id: '0002', supersedes: ['here:0001'] }, { path: 'docs/adr/0002-a.md' }); + const target = record( + { id: '0001', status: 'superseded', supersededBy: '0002' }, + { path: 'docs/adr/0001-b.md' }, + ); + const report = evaluateReport( + baseInput({ corpus: corpusOf([proposal, target]), proposalPath: 'docs/adr/0002-a.md', resolutionLog: 'here' }), + ); + expect(ruleResult(report, 'no-orphan-refs').status).toBe('pass'); + }); }); diff --git a/packages/evaluator/test/supersession-consistent.test.ts b/packages/evaluator/test/supersession-consistent.test.ts index 8490a361..dc9a342e 100644 --- a/packages/evaluator/test/supersession-consistent.test.ts +++ b/packages/evaluator/test/supersession-consistent.test.ts @@ -71,4 +71,27 @@ describe('supersession-consistent', () => { expect(orphan.findings.some((f) => f.reason === 'supersession-consistent.dangling-superseded-by')).toBe(false); expect(orphan.status).not.toBe('fail'); }); + + // ADR-0059 clause 4: a log-qualified supersededBy stays the dangling failure, with or + // without a snapshot of the named log, and its message says why. + test('a log-qualified supersededBy fails as dangling, even with a snapshot of that log', () => { + const superseded = record( + { id: '0001', status: 'superseded', supersededBy: 'payments:0012' }, + { path: 'docs/adr/0001-a.md' }, + ); + const proposal = record({ id: '0003' }, { path: 'docs/adr/0003-c.md' }); + for (const federatedLogs of [undefined, [{ log: 'payments', adrIds: ['0012'] }]]) { + const report = evaluateReport( + baseInput({ + corpus: corpusOf([superseded, proposal]), + proposalPath: 'docs/adr/0003-c.md', + ...(federatedLogs ? { federatedLogs } : {}), + }), + ); + const result = ruleResult(report, 'supersession-consistent'); + expect(result.status).toBe('fail'); + expect(result.reason).toBe('supersession-consistent.dangling-superseded-by'); + expect(result.findings[0]?.message).toContain('ADR-0059 clause 4'); + } + }); }); From 36eff0125179d16367a7c007db23d04308fe8c28 Mon Sep 17 00:00:00 2001 From: Mark Beacom Date: Sat, 10 Oct 2026 17:47:45 -0400 Subject: [PATCH 2/2] fix(evaluator): read same-log-qualified supersession refs as local in every check Copilot found that a supersededBy qualified with the evaluator's own resolution log (here:0002 under resolutionLog here) was still dangling: supersession-consistent compared the raw ref against bare ids before the cross-log test mattered, and the reciprocity, pair, and cycle checks compared raw refs too. Refs qualified with the resolution log are now normalized to their bare id once, up front, so every check agrees with no-orphan-refs; a ref naming another log (or any qualified ref with no resolution log) stays raw and keeps the ADR-0059 clause 4 failure. Four new tests failed before the fix (ADR-0016). Signed-off-by: Mark Beacom --- .../src/rules/supersession-consistent.ts | 30 +++++-- .../test/supersession-consistent.test.ts | 88 +++++++++++++++++++ 2 files changed, 112 insertions(+), 6 deletions(-) diff --git a/packages/evaluator/src/rules/supersession-consistent.ts b/packages/evaluator/src/rules/supersession-consistent.ts index 831a4971..13bb68d6 100644 --- a/packages/evaluator/src/rules/supersession-consistent.ts +++ b/packages/evaluator/src/rules/supersession-consistent.ts @@ -8,7 +8,7 @@ * in the supersedes relation fail with `cycle`. Exactly one aggregate result (C11). */ -import { buildAdrGraph, type Adr } from '@adrkit/core'; +import { buildAdrGraph, parseAdrRef, type Adr } from '@adrkit/core'; import { aggregate, passResult, type SubResult } from './kernel.ts'; import { isCrossLog } from './no-orphan-refs.ts'; import type { RuleContext } from './context.ts'; @@ -47,7 +47,23 @@ function hasSupersedesCycle(records: readonly Adr[]): boolean { } export function evaluateSupersessionConsistent(ctx: RuleContext): RuleResult { - const records = ctx.corpusRecords; + const resolutionLog = ctx.input.resolutionLog; + // A ref qualified with the resolution log itself (`here:0002` under `here`) names a local + // record, as no-orphan-refs reads it, so every check below compares its bare id. A ref + // naming another log stays raw, never matches a bare id, and keeps the clause-4 failure. + const local = (ref: string): string => (isCrossLog(ref, resolutionLog) ? ref : parseAdrRef(ref).id); + const normalized: Adr[] = ctx.corpusRecords.map((record) => ({ + ...record, + frontmatter: { + ...record.frontmatter, + supersedes: record.frontmatter.supersedes.map(local), + ...(record.frontmatter.supersededBy !== undefined + ? { supersededBy: local(record.frontmatter.supersededBy) } + : {}), + }, + })); + const originals = new Map(normalized.map((record, index) => [record, ctx.corpusRecords[index]!])); + const records = normalized; const ids = new Set(records.map((record) => record.frontmatter.id)); const supersededByById = new Map(); for (const record of records) { @@ -65,15 +81,17 @@ export function evaluateSupersessionConsistent(ctx: RuleContext): RuleResult { const target = record.frontmatter.supersededBy; if (target && !ids.has(target)) { danglingRefs.add(`${record.frontmatter.id}->${target}`); + // Findings echo the ref as written, never the normalized id. + const written = originals.get(record)!.frontmatter.supersededBy ?? target; const finding: RuleFinding = { reason: 'supersession-consistent.dangling-superseded-by', // ADR-0059 clause 4: a log-qualified successor is never resolved, snapshot or not. - message: isCrossLog(target, ctx.input.resolutionLog) - ? `supersededBy "${target}" names another decision log; cross-repository supersession is not supported (ADR-0059 clause 4)` - : `supersededBy "${target}" does not resolve to a record in the corpus`, + message: isCrossLog(written, resolutionLog) + ? `supersededBy "${written}" names another decision log; cross-repository supersession is not supported (ADR-0059 clause 4)` + : `supersededBy "${written}" does not resolve to a record in the corpus`, adr: record.frontmatter.id, candidateAdr: record.frontmatter.id, - relatedAdr: target, + relatedAdr: written, recordPath: record.path, field: 'supersededBy', lowerLevel: { rule: 'dangling-supersededBy', path: record.path, id: record.frontmatter.id, field: 'supersededBy' }, diff --git a/packages/evaluator/test/supersession-consistent.test.ts b/packages/evaluator/test/supersession-consistent.test.ts index dc9a342e..ee96e442 100644 --- a/packages/evaluator/test/supersession-consistent.test.ts +++ b/packages/evaluator/test/supersession-consistent.test.ts @@ -94,4 +94,92 @@ describe('supersession-consistent', () => { expect(result.findings[0]?.message).toContain('ADR-0059 clause 4'); } }); + + // A ref qualified with the evaluator's own resolution log names a local record, so every + // supersession check reads it by its bare id (no-orphan-refs already does). + describe('refs qualified with the resolution log itself', () => { + test('a same-log supersededBy resolves and reciprocates → pass', () => { + const superseded = record( + { id: '0001', status: 'superseded', supersededBy: 'here:0002' }, + { path: 'docs/adr/0001-a.md' }, + ); + const successor = record({ id: '0002', supersedes: ['0001'] }, { path: 'docs/adr/0002-b.md' }); + const report = evaluateReport( + baseInput({ corpus: corpusOf([superseded, successor]), proposalPath: 'docs/adr/0002-b.md', resolutionLog: 'here' }), + ); + expect(ruleResult(report, 'supersession-consistent')).toMatchObject({ + status: 'pass', + reason: 'supersession-consistent.ok', + }); + }); + + test('a same-log supersedes reciprocates a bare supersededBy → pass', () => { + const superseded = record( + { id: '0001', status: 'superseded', supersededBy: '0002' }, + { path: 'docs/adr/0001-a.md' }, + ); + const successor = record({ id: '0002', supersedes: ['here:0001'] }, { path: 'docs/adr/0002-b.md' }); + const report = evaluateReport( + baseInput({ corpus: corpusOf([superseded, successor]), proposalPath: 'docs/adr/0002-b.md', resolutionLog: 'here' }), + ); + expect(ruleResult(report, 'supersession-consistent').status).toBe('pass'); + expect(ruleResult(report, 'no-orphan-refs').status).toBe('pass'); + }); + + test('a same-log supersedes that is not reciprocated fails as non-reciprocal', () => { + const target = record({ id: '0001', status: 'accepted', deciders: ['@a'] }, { path: 'docs/adr/0001-a.md' }); + const successor = record({ id: '0002', supersedes: ['here:0001'] }, { path: 'docs/adr/0002-b.md' }); + const report = evaluateReport( + baseInput({ corpus: corpusOf([target, successor]), proposalPath: 'docs/adr/0002-b.md', resolutionLog: 'here' }), + ); + expect(ruleResult(report, 'supersession-consistent').reason).toBe('supersession-consistent.non-reciprocal'); + }); + + test('a supersession cycle through same-log refs is detected', () => { + const a = record( + { id: '0001', status: 'superseded', supersededBy: 'here:0002', supersedes: ['here:0002'] }, + { path: 'docs/adr/0001-a.md' }, + ); + const b = record( + { id: '0002', status: 'superseded', supersededBy: 'here:0001', supersedes: ['here:0001'] }, + { path: 'docs/adr/0002-b.md' }, + ); + const proposal = record({ id: '0003' }, { path: 'docs/adr/0003-c.md' }); + const report = evaluateReport( + baseInput({ corpus: corpusOf([a, b, proposal]), proposalPath: 'docs/adr/0003-c.md', resolutionLog: 'here' }), + ); + const result = ruleResult(report, 'supersession-consistent'); + expect(result.status).toBe('fail'); + expect(result.findings.map((f) => f.reason)).toEqual(['supersession-consistent.cycle']); + }); + + test('a same-log supersededBy whose id is missing is dangling without the clause-4 wording', () => { + const superseded = record( + { id: '0001', status: 'superseded', supersededBy: 'here:0009' }, + { path: 'docs/adr/0001-a.md' }, + ); + const proposal = record({ id: '0003' }, { path: 'docs/adr/0003-c.md' }); + const report = evaluateReport( + baseInput({ corpus: corpusOf([superseded, proposal]), proposalPath: 'docs/adr/0003-c.md', resolutionLog: 'here' }), + ); + const result = ruleResult(report, 'supersession-consistent'); + expect(result.reason).toBe('supersession-consistent.dangling-superseded-by'); + expect(result.findings[0]?.relatedAdr).toBe('here:0009'); + expect(result.findings[0]?.message).not.toContain('ADR-0059'); + }); + + test('without a resolution log, a qualified supersededBy is another log and stays the clause-4 failure', () => { + const superseded = record( + { id: '0001', status: 'superseded', supersededBy: 'here:0002' }, + { path: 'docs/adr/0001-a.md' }, + ); + const successor = record({ id: '0002', supersedes: ['0001'] }, { path: 'docs/adr/0002-b.md' }); + const report = evaluateReport( + baseInput({ corpus: corpusOf([superseded, successor]), proposalPath: 'docs/adr/0002-b.md' }), + ); + const result = ruleResult(report, 'supersession-consistent'); + expect(result.status).toBe('fail'); + expect(result.findings.some((f) => f.message?.includes('ADR-0059 clause 4') === true)).toBe(true); + }); + }); });