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..f0429d4c 100644 --- a/tests/helpers/adr-writer.test.ts +++ b/tests/helpers/adr-writer.test.ts @@ -202,6 +202,30 @@ 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" + ); + 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 () => { writeFileSync(join(tempDir, "BAD-001-broken.md"), "not valid frontmatter"); await createAdrFile(tempDir, {