From 4313b6f0f9ac410a85a9b4d78d277ce26d956793 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Thu, 9 Jul 2026 11:59:47 -0300 Subject: [PATCH 1/2] fix: detect and error on duplicate ADR IDs across files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When two ADR files share the same frontmatter `id`, downstream consumers silently lose one: Map.set overwrites in review-context, Array.find picks the first in adr show/update, and archgate check has no validation for it. Add duplicate ID detection at the two centralized parse points: - parseAllAdrs() in engine/loader.ts — protects adr list, check, review-context - findAdrFileById() in helpers/adr-writer.ts — protects adr show, adr update Both throw UserError with a clear message listing all colliding filenames. Closes #465 Claude-Session: https://claude.ai/code/session_01EsLnQfUpYouoTw42Y7va6K Signed-off-by: Rhuan Barreto --- src/engine/loader.ts | 33 +++++++++++++++++++++++++++++++- src/helpers/adr-writer.ts | 12 +++++++++++- tests/engine/loader.test.ts | 19 ++++++++++++++++++ tests/helpers/adr-writer.test.ts | 18 +++++++++++++++++ 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/engine/loader.ts b/src/engine/loader.ts index ce24893e..d7bae445 100644 --- a/src/engine/loader.ts +++ b/src/engine/loader.ts @@ -30,6 +30,7 @@ import type { ViolationDetail } from "../formats/rules"; import { logDebug } from "../helpers/log"; import { resolvedProjectPaths } from "../helpers/project-config"; import { ensureRulesShim } from "../helpers/rules-shim"; +import { UserError } from "../helpers/user-error"; import { scanRuleSource } from "./rule-scanner"; interface LoadedAdr { @@ -188,7 +189,37 @@ export function parseAllAdrs(projectRoot: string): Promise { }) ); - return parsed.filter((e): e is ParsedAdrEntry => e !== null); + const entries = parsed.filter((e): e is ParsedAdrEntry => e !== null); + + // Detect duplicate ADR IDs — two files sharing the same frontmatter id + // is an authoring error that causes silent data loss downstream (Map.set + // overwrites, Array.find picks the first, etc.). + const idToFiles = new Map(); + for (const entry of entries) { + const id = entry.adr.frontmatter.id; + const existing = idToFiles.get(id); + if (existing) { + existing.push(entry.file); + } else { + idToFiles.set(id, [entry.file]); + } + } + const duplicates = [...idToFiles.entries()].filter( + ([, files]) => files.length > 1 + ); + if (duplicates.length > 0) { + const details = duplicates + .map( + ([id, files]) => + `Duplicate ADR ID: ${id}\n${files.map((f) => ` - ${f}`).join("\n")}` + ) + .join("\n\n"); + throw new UserError( + `${details}\n\nEach ADR must have a unique id in its YAML frontmatter.` + ); + } + + return entries; })(); parsedAdrsCache.set(projectRoot, promise); diff --git a/src/helpers/adr-writer.ts b/src/helpers/adr-writer.ts index bee7bf66..f77abf2e 100644 --- a/src/helpers/adr-writer.ts +++ b/src/helpers/adr-writer.ts @@ -140,7 +140,17 @@ export async function findAdrFileById( }) ); - return results.find((adr) => adr?.frontmatter.id === id) ?? null; + const matches = results.filter((adr) => adr?.frontmatter.id === id); + if (matches.length > 1) { + const collidingFiles = files.filter((_file, i) => { + const parsed = results[i]; + return parsed?.frontmatter.id === id; + }); + throw new UserError( + `Duplicate ADR ID: ${id}\n${collidingFiles.map((f) => ` - ${f}`).join("\n")}\n\nEach ADR must have a unique id in its YAML frontmatter.` + ); + } + return matches[0] ?? null; } interface UpdateAdrResult { diff --git a/tests/engine/loader.test.ts b/tests/engine/loader.test.ts index d49b71d5..c2fa2f80 100644 --- a/tests/engine/loader.test.ts +++ b/tests/engine/loader.test.ts @@ -256,6 +256,25 @@ export default { invalid syntax here !!! } satisfies RuleSet; expect(first.value.adr.frontmatter.id).toBe("TEST-001"); }); + test("parseAllAdrs throws on duplicate ADR IDs", async () => { + const adrsDir = join(tempDir, ".archgate", "adrs"); + + // Two files with different names but same frontmatter id + writeFileSync( + join(adrsDir, "GEN-099-topic-a.md"), + `---\nid: GEN-099\ntitle: Topic A\ndomain: general\nrules: false\n---\n# Topic A\n` + ); + writeFileSync( + join(adrsDir, "GEN-099-topic-b.md"), + `---\nid: GEN-099\ntitle: Topic B\ndomain: general\nrules: false\n---\n# Topic B\n` + ); + + await expect(parseAllAdrs(tempDir)).rejects.toThrow("Duplicate ADR ID"); + await expect(parseAllAdrs(tempDir)).rejects.toThrow("GEN-099"); + await expect(parseAllAdrs(tempDir)).rejects.toThrow("GEN-099-topic-a.md"); + await expect(parseAllAdrs(tempDir)).rejects.toThrow("GEN-099-topic-b.md"); + }); + test("parseAllAdrs reads from custom directory", async () => { // Configure custom path const customAdrsDir = join(tempDir, "governance"); diff --git a/tests/helpers/adr-writer.test.ts b/tests/helpers/adr-writer.test.ts index 58e79a5c..34a602e6 100644 --- a/tests/helpers/adr-writer.test.ts +++ b/tests/helpers/adr-writer.test.ts @@ -202,6 +202,24 @@ describe("findAdrFileById", () => { expect(result!.body).toContain("Backend context."); }); + test("throws on duplicate ADR IDs", async () => { + writeFileSync( + join(tempDir, "GEN-099-topic-a.md"), + `---\nid: GEN-099\ntitle: Topic A\ndomain: general\nrules: false\n---\n# Topic A\n` + ); + writeFileSync( + join(tempDir, "GEN-099-topic-b.md"), + `---\nid: GEN-099\ntitle: Topic B\ndomain: general\nrules: false\n---\n# Topic B\n` + ); + + await expect(findAdrFileById(tempDir, "GEN-099")).rejects.toThrow( + "Duplicate ADR ID" + ); + await expect(findAdrFileById(tempDir, "GEN-099")).rejects.toThrow( + "GEN-099" + ); + }); + test("skips unparseable files gracefully", async () => { writeFileSync(join(tempDir, "BAD-001-broken.md"), "not valid frontmatter"); await createAdrFile(tempDir, { From be5dab248325e0c16c2b6d21e8d0cfbea3c4802f Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Thu, 9 Jul 2026 15:00:48 -0300 Subject: [PATCH 2/2] test: assert colliding filenames in findAdrFileById duplicate test Add filename assertions (GEN-099-topic-a.md, GEN-099-topic-b.md) to match the parseAllAdrs test coverage, ensuring a regression that drops filenames from the error message is caught here too. Claude-Session: https://claude.ai/code/session_01EsLnQfUpYouoTw42Y7va6K Signed-off-by: Rhuan Barreto --- tests/helpers/adr-writer.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/helpers/adr-writer.test.ts b/tests/helpers/adr-writer.test.ts index 34a602e6..f0429d4c 100644 --- a/tests/helpers/adr-writer.test.ts +++ b/tests/helpers/adr-writer.test.ts @@ -218,6 +218,12 @@ describe("findAdrFileById", () => { await expect(findAdrFileById(tempDir, "GEN-099")).rejects.toThrow( "GEN-099" ); + await expect(findAdrFileById(tempDir, "GEN-099")).rejects.toThrow( + "GEN-099-topic-a.md" + ); + await expect(findAdrFileById(tempDir, "GEN-099")).rejects.toThrow( + "GEN-099-topic-b.md" + ); }); test("skips unparseable files gracefully", async () => {