fix: detect and error on duplicate ADR IDs across files#466
Conversation
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 <rhuan@barreto.work>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2d5bb151-dc2e-4de6-ae84-8fd72d631d48) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (11)
🧰 Additional context used📓 Path-based instructions (5)tests/**/*.test.ts📄 CodeRabbit inference engine (.archgate/adrs/ARCH-005-testing-standards.md)
Files:
**/*.{ts,tsx,js,jsx,mjs,cjs}📄 CodeRabbit inference engine (.archgate/adrs/ARCH-006-dependency-policy.md)
Files:
tests/**/*.ts📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)
Files:
{src,tests}/**/*.ts📄 CodeRabbit inference engine (.archgate/adrs/LEGAL-001-spdx-license-headers.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThis change adds duplicate ADR ID detection to two code paths. In 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying archgate-cli with
|
| Latest commit: |
be5dab2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b7baa2c6.archgate-cli.pages.dev |
| Branch Preview URL: | https://claude-unified-rolling-kite.archgate-cli.pages.dev |
Code Coverage
Full HTML report available in workflow artifacts. Per-directory breakdown
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/helpers/adr-writer.test.ts`:
- Around line 205-222: The duplicate ADR ID test for findAdrFileById only checks
the generic error text and ID, so it should also assert the colliding filenames
are included in the thrown message. Update the test in adr-writer.test.ts to
mirror the parseAllAdrs coverage by verifying the error mentions both
GEN-099-topic-a.md and GEN-099-topic-b.md, using the existing findAdrFileById
expectation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 286f0cc5-ab9c-4d2d-8011-996a2e6785bf
📒 Files selected for processing (4)
src/engine/loader.tssrc/helpers/adr-writer.tstests/engine/loader.test.tstests/helpers/adr-writer.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Smoke Test (Windows) / Windows
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (8)
tests/**/*.test.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-005-testing-standards.md)
tests/**/*.test.ts: Use Bun's built-in test runner (bun:test) for all test files, and place tests undertests/mirroring thesrc/directory structure with<module-name>.test.tsnaming.
Use temp directories (mkdtemp) for filesystem tests, and clean them up inafterEachorafterAll.
Close external SDK instances (servers, clients, transports, connections) inafterEachorafterAll, managing their lifecycle in hooks rather than inside individual test bodies.
When a test creates a temporary git repository and runsgit commit, configure localuser.emailanduser.nameimmediately aftergit init.
Test public module interfaces, not private implementation details.
Use descriptive test names that explain the expected behavior.
Every runnabletest()/it()must contain at least oneexpect()assertion; smoke tests must make the contract explicit withexpect(() => fn()).not.toThrow()orawait expect(promise).resolves.toBeUndefined().
Usetest.skip,test.skipIf, ortest.todofor intentionally empty or disabled tests; do not use barereturnor empty callbacks to skip work.
If the firstexpect()is being added to a previously assertion-less test file, addexpectto thebun:testimport.
When mockingfetchin tests, assign directly toglobalThis.fetchand restore the original or usemock.restore()afterward.
WrapspyOn()and inlinemockImplementation()usage intry/finally, or create and restore spies in hooks, somockRestore()always runs.
Only raise a per-test timeout above the globalbun test --timeout 60000; never set a shorter per-test timeout.
Mock first-party modules withimport * as modplusspyOn(mod, "fn"), notmock.module().
When a test needs to redirect user-scope paths, mockos.homedir()instead of relying onHOME/Bun.env.HOME; restore the spy in test hooks.
Do not depend on network access in unit tests.
Do not leave temp files after test runs.
Do not leave external SDK instances open after tests...
Files:
tests/helpers/adr-writer.test.tstests/engine/loader.test.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-006-dependency-policy.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use Bun built-ins for file I/O (Bun.file,Bun.write), HTTP, subprocess execution (Bun.spawn), globbing (Bun.Glob), and testing (bun:test).
Do not use Node.js-specific APIs when Bun alternatives exist; for example, useBun.file()instead offs.readFile()for simple reads.
Prefernode:built-in modules such asnode:util,node:path, andnode:fsover npm alternatives.
Do not use utility libraries for single functions, such as importing lodash for one helper likepick.
Do not use path aliases (tsconfig paths); use relative imports with Bun's native module resolution.
Do not install packages globally during development; usebunxfor one-off tools.
Files:
tests/helpers/adr-writer.test.tssrc/engine/loader.tssrc/helpers/adr-writer.tstests/engine/loader.test.ts
tests/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)
In test files, use
_resetPlatformCache()to simulate different platforms instead of mocking or mutatingprocess.platformdirectly.
Files:
tests/helpers/adr-writer.test.tstests/engine/loader.test.ts
{src,tests}/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/LEGAL-001-spdx-license-headers.md)
{src,tests}/**/*.ts: Every TypeScript source file insrc/andtests/must begin with// SPDX-License-Identifier: Apache-2.0followed by// Copyright 2026 Archgate.
If a TypeScript file has a shebang line (for example#!/usr/bin/env buninsrc/cli.ts), the SPDX license header must appear immediately after the shebang.
Use single-line//comments for the SPDX header; do not use block comments (/* */) or alternate license identifiers.
Files:
tests/helpers/adr-writer.test.tssrc/engine/loader.tssrc/helpers/adr-writer.tstests/engine/loader.test.ts
**
⚙️ CodeRabbit configuration file
**: # CLAUDE.mdArchgate is a CLI tool for AI governance via Architecture Decision Records (ADRs) — combining human-readable docs with machine-checkable rules. The CLI dogfoods itself via ADRs in
.archgate/adrs/. AI features are delivered as a Claude Code plugin (../plugins/claude-code), not via direct API calls.Technology Stack
- Runtime: Bun (>=1.2.21) — not Node.js compatible
- Language: TypeScript (strict mode, ESNext, ES modules)
- CLI framework: Commander.js (
@commander-js/extra-typings)- Linter: Oxlint | Formatter: Oxfmt | Dead exports: Knip | Commits: Conventional Commits
Commands
bun run src/cli.ts <command> # run CLI locally bun run lint # oxlint bun run typecheck # tsc --build bun run format # oxfmt --write bun run format:check # oxfmt --check bun run test # all tests (not bare `bun test` — picks up --timeout; see GEN-003) bun run knip # dead export detection bun run validate # MANDATORY: lint + typecheck + format:check + test + ADR check + knip + build check bun run build:check # verify build compiles (CI builds binaries via release workflow) bun run commit # conventional commit wizardValidation Gate
bun run validatemust pass before any task is considered complete. Fail-fast pipeline: lint → typecheck → format:check → test → ADR check → knip → build check. Mirrors CI in.github/workflows/code-pull-request.yml.Git Hooks (Git 2.54+)
Config-based hooks in
.githooksrun validation locally before commits and pushes:
- pre-commit: lint + typecheck + format:check (~15s)
- pre-push: full
bun run validate(~60s, mirrors CI)Activate once per clone:
git config --local include.path ../.githooksOpt out of a specific hook:
git config --local hook.<name>.enabled false. Skip all hooks for a single commit:git commit --no-verify.#...
Files:
tests/helpers/adr-writer.test.tssrc/engine/loader.tssrc/helpers/adr-writer.tstests/engine/loader.test.ts
⚙️ CodeRabbit configuration file
**: This project uses Archgate — an AI governance framework based on
Architecture Decision Records (ADRs). The ADRs in.archgate/adrs/
are the authoritative rules for this codebase. Each ADR has a companion
.rules.tsfile with automated checks that run viaarchgate check.When reviewing, you must:
- Treat ADR violations as blocking issues, not suggestions.
- Cite the specific ADR ID when flagging a violation (e.g., "Violates ARCH-006").
- Focus on semantic and contextual violations that automated rules cannot catch —
the.rules.tsfiles already cover syntactic/structural patterns.- If you are unsure whether something violates an ADR, flag it as a question
rather than approving it.
Files:
tests/helpers/adr-writer.test.tssrc/engine/loader.tssrc/helpers/adr-writer.tstests/engine/loader.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-002-error-handling.md)
src/**/*.ts: UselogError()fromsrc/helpers/log.tsfor user-facing errors instead of printing errors directly.
Exit with code1for expected failures such as missing config, invalid input, or validation violations.
Let unexpected errors crash naturally so they are treated as internal errors with exit code2.
Include actionable suggestions in user-facing error messages whenever possible.
Write user-facing errors to stderr vialogError(); do not send them to stdout.
WhenfindProjectRoot()returnsnullfor commands that do not require.archgate/, fall back toprocess.cwd()as the path key or working directory.
CatchExitPromptErrorfrom Inquirer in the top-level error boundary and treat it as user cancellation: exit with code130without logging an error or sending it to Sentry.
Do not useconsole.error()directly; uselogError()for consistent formatting.
Do not exit with codes other than0,1,2, or130.
Do not send user-cancellation errors such as InquirerExitPromptErrorto Sentry; filter them inbeforeSend.
src/**/*.ts: UsestyleText(format, text)fromnode:utilfor all terminal colors and formatting in CLI source files; do not use raw ANSI escape codes or third-party color libraries.
Commands that produce structured results and support--jsonmust emit machine-readable JSON to stdout with no colors or decorative formatting.
When a command supports--json, useformatJSON()fromsrc/helpers/output.tsfor JSON serialization, and passforcePretty: truewhen the user explicitly provided--json.
UseisAgentContext()fromsrc/helpers/output.tsto enable auto-JSON behavior for commands that support both human-readable and JSON output modes.
CLI output must not include emoji; use text symbols and colors instead.
Send normal command output to stdout withconsole.log(), and send errors, warnings, and debug messages to stderr vialogError(),logWarn(), andlogDebug().
Keep CLI output concise and scannabl...
Files:
src/engine/loader.tssrc/helpers/adr-writer.ts
src/{helpers,engine}/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-002-error-handling.md)
Do not use
console.log(),console.warn(), orconsole.info()directly in helper or engine files; uselogInfo()orlogWarn()instead. Command files are exempt because they are the I/O layer.
Files:
src/engine/loader.tssrc/helpers/adr-writer.ts
src/**/!(*platform).ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)
src/**/!(*platform).ts: Insrc/TypeScript source files, do not readprocess.platformdirectly; usesrc/helpers/platform.ts(isWindows(),isMacOS(),isLinux(),isWSL(),getPlatformInfo()) for all platform detection.
Use the centralized platform helper instead of duplicating OS/WSL detection logic inline anywhere insrc/TypeScript source.
When behavior differs between Linux and Windows, account for WSL by usingisWSL()rather than assuming `
Files:
src/engine/loader.tssrc/helpers/adr-writer.ts
🧠 Learnings (1)
📚 Learning: 2026-07-02T16:03:33.031Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 446
File: src/helpers/session-context-opencode.ts:81-100
Timestamp: 2026-07-02T16:03:33.031Z
Learning: For synchronous helper functions that use Bun’s sqlite sync API (i.e., they must remain synchronous), it’s acceptable to use `existsSync` from `node:fs` to check whether the SQLite database file exists. Avoid using `Bun.file(path).exists()` for this purpose because it’s async and would force the helper to become async (no equivalent synchronous Bun alternative). If the DB file is missing, throw/return a clear, actionable "No database found" error (per ARCH-006) rather than letting the sqlite open fail with a generic DB-open error.
Applied to files:
src/helpers/adr-writer.ts
🔇 Additional comments (3)
src/engine/loader.ts (1)
33-33: LGTM!Also applies to: 192-222
tests/engine/loader.test.ts (1)
259-277: LGTM!src/helpers/adr-writer.ts (1)
143-153: LGTM!
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 <rhuan@barreto.work>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a09959be-0716-489c-8f6d-2b857cac55cc) |
# archgate ## [0.48.1](v0.48.0...v0.48.1) (2026-07-09) ### Bug Fixes * **ci:** disable toolchain cache in release-binaries ([#462](#462)) ([a4b9829](a4b9829)) * detect and error on duplicate ADR IDs across files ([#466](#466)) ([991cc9e](991cc9e)), closes [#465](#465) --- This PR was generated with [simple-release](https://github.com/TrigenSoftware/simple-release). <details> <summary>📄 Cheatsheet</summary> <br> You can configure the bot's behavior through a pull request comment using the `!simple-release/set-options` command. ### Command Format ````md !simple-release/set-options ```json { "bump": {}, "publish": {} } ``` ```` ### Useful Parameters #### Bump | Parameter | Type | Description | |-----------|------|-------------| | `version` | `string` | Force set specific version | | `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type | | `prerelease` | `string` | Pre-release identifier (e.g., "alpha", "beta") | | `firstRelease` | `boolean` | Whether this is the first release | | `skip` | `boolean` | Skip version bump | | `byProject` | `Record<string, object>` | Per-project bump options for monorepos | #### Publish | Parameter | Type | Description | |-----------|------|-------------| | `skip` | `boolean` | Skip publishing | | `access` | `'public' \| 'restricted'` | Package access level | | `tag` | `string` | Tag for npm publication | ### Usage Examples #### Force specific version ````md !simple-release/set-options ```json { "bump": { "version": "2.0.0" } } ``` ```` #### Force major bump ````md !simple-release/set-options ```json { "bump": { "as": "major" } } ``` ```` #### Create alpha pre-release ````md !simple-release/set-options ```json { "bump": { "prerelease": "alpha" } } ``` ```` #### Publish with specific access and tag ````md !simple-release/set-options ```json { "bump": { "prerelease": "beta" }, "publish": { "access": "public", "tag": "beta" } } ``` ```` ### Access Restrictions The command can only be used by users with permissions: - repository owner - organization member - collaborator ### Notes - The last comment with `!simple-release/set-options` command takes priority - JSON must be valid, otherwise the command will be ignored - Parameters apply only to the current release execution - The command can be updated by editing the comment or adding a new one </details> <!-- Please do not edit this comment. simple-release-pull-request: true simple-release-branch-from: release simple-release-branch-to: main --> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 participant
Summary
parseAllAdrs()(engine/loader.ts) — protectsadr list,check, andreview-contextby throwing aUserErrorwhen two files share the same frontmatteridfindAdrFileById()(helpers/adr-writer.ts) — protectsadr showandadr updatewith the same errorCloses #465
Test plan
bun run validatepasses (lint, typecheck, format, 1440 tests, 43 ADR checks, knip, build)parseAllAdrs throws on duplicate ADR IDs— verifies error message includes ID and both filenamesfindAdrFileById throws on duplicate ADR IDs— verifies error on ambiguous lookupidand runarchgate adr list,adr show <id>,check— all should error with the colliding filenameshttps://claude.ai/code/session_01EsLnQfUpYouoTw42Y7va6K