refactor(engine): derive the rules.d.ts shim from the real interface - #547
Conversation
`generateRulesDts()` transcribed `RuleContext` and its supporting types as text inside a template literal. Nothing in `bun run validate` compared that text to `src/formats/rules.ts`: `tsc` sees a string, and ARCH-022's `rulecontext-shim-parity` compared member names only, so a wrong parameter type or a stale JSDoc paragraph shipped to rule authors' editors unflagged. `src/helpers/rules-source.ts` inlines the text of `src/formats/rules.ts` as a Bun macro, and `toAmbientDeclarations()` turns its exports into ambient declarations. Signatures and JSDoc now travel verbatim. The macro reads synchronously because an async macro fails to parse under `bun build --compile --bytecode`. `rulecontext-shim-derived` replaces `rulecontext-shim-parity`, failing on the three ways the shim could stop being a derivation: a value export in `src/formats/rules.ts`, a shim that no longer reads it through the macro, and an ambient declaration transcribed by hand. `single-ast-method` drops to one surface, since the shim's declarations now follow by construction. Closes #511 Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
`check` regenerates the shim before rules run, so `generateRulesDts()`'s throw on a value export in `src/formats/rules.ts` aborts the run at exit 2 and the rule's matching branch can never execute. Drop the branch and say so in ARCH-022 rather than claim coverage the rule cannot deliver. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
The comment described the shim's former construction rather than the reason the test picks this member (GEN-004, forward-only prose). Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Capture what no check reaches: synchronous Bun macros as the only import form that satisfies tsc and `--compile --bytecode`, the review-context JSON capture mechanics, and the rule that a guard preempted by an earlier pipeline stage is dead governance. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Signed-off-by placeholder Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
The derivation tests assert about the shim; none of them show it. Commit the generated `rules.d.ts` as a snapshot so a PR diff carries the exact ambient types rule authors receive, and any interface edit surfaces as a readable hunk rather than a passing assertion. Bun fails a missing snapshot when `CI` is set, so the file cannot be deleted to route around the check. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
TypeScript's `include` globs skip dot-directories, so tsconfig's `.archgate/` entry resolved to zero files and the 30 companion `.rules.ts` files — the layer that enforces every ADR — were never typechecked. Name the path explicitly so they enter the program. Their ambient types arrive through a triple-slash reference to the gitignored, generated `.archgate/rules.d.ts`, so `typecheck` generates it first; without that a fresh clone fails with TS6053 and a cascade of implicit-`any`. `tests/fixtures` leaves the program: a checked-in ambient `.d.ts` there contributed its declarations globally, and `skipLibCheck` hid the resulting duplicate identifiers. Its `rules.d.ts` is deleted outright — a pack ships `.rules.ts` whose triple-slash resolves in the consumer project after `archgate adr import` writes the shim, so a pack never carries one. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
A released binary carries `src/formats/rules.ts` frozen from a build CI proved type-only, so the generator's guard is unreachable for end users and only an edited checkout trips it. Throwing a bare Error routed that to the exit-2 boundary, which captures to Sentry — filing a contributor's own edit as an archgate defect (CLI-8, the pattern ARCH-002 records as CLI-5). UserError keeps it at exit 1 with an actionable hint and no capture. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe RuleContext shim now derives ambient declarations from 🚥 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: |
bbf3761
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bd484aa4.archgate-cli.pages.dev |
| Branch Preview URL: | https://rhuanbarreto-github-issue-51.archgate-cli.pages.dev |
Code Coverage
Full HTML report available in workflow artifacts. Per-directory breakdown
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/helpers/rules-shim.ts`:
- Around line 27-43: Update toAmbientDeclarations to detect top-level import
declarations before generating the ambient shim and throw UserError for the
first such import. Match the existing value-export guard’s structure, but direct
authors to declare the type inline in src/formats/rules.ts; preserve the current
transformation for type-only source without imports.
In `@tests/helpers/rules-shim.test.ts`:
- Around line 53-62: Guard the signature lookup in the “carries a member's
signature and JSDoc verbatim” test by storing the signature index and asserting
it is greater than zero before computing the slice end. Keep the existing JSDoc
start assertion and containment check, but ensure a missing or changed readYAML
signature causes the test to fail rather than slicing an empty range.
🪄 Autofix
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: d024e067-a988-4287-a84b-a38bdfe57030
⛔ Files ignored due to path filters (1)
tests/helpers/__snapshots__/rules-shim.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
.archgate/adrs/ARCH-022-ast-aware-rule-context.md.archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts.claude/agent-memory/archgate-developer/MEMORY.mdpackage.jsonsrc/helpers/rules-shim.tssrc/helpers/rules-source.tstests/fixtures/fake-registry/packs/test-pack/rules.d.tstests/helpers/rules-shim.test.tstests/helpers/rules-source.test.tstsconfig.json
💤 Files with no reviewable changes (1)
- tests/fixtures/fake-registry/packs/test-pack/rules.d.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Smoke Test (Windows) / Windows
- GitHub Check: Lint, Test & Check
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (csharp)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (18)
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.
tests/**/*.ts: Use Bun's built-inbun testrunner for all tests; do not use Jest, Vitest, or custom assertions.
Mirror thesrc/directory structure intests/, and name test files<module-name>.test.ts.
Usemkdtempfor filesystem-test isolation, keep writes inside the temporary directory, and clean up temporary resources inafterEachorafterAll.
Test each module's public interface with descriptive names; do not test private internals.
Every runnable test must contain anexpect()assertion; usetest.skiportest.todofor placeholders and do not leave assertion-less or silently skipped tests.
Restore every captured environment variable withrestoreEnv(key, original)rather than assigning the captured value directly.
Mockos.homedir()via an imported module namespace andspyOn; do not overrideHOMEto control home-directory resolution. Environment overrides are valid only for code that readsBun.envat call time.
Mock first-party modules withimport * as modplusspyOn, restore them withmock.restore(), and never usemock.module()or an-implproduction split for first-party modules.
For HTTP mocking, saveglobalThis.fetchbefore replacing it and restore the direct assignment inafterEach; do not usemock.module("node:fetch").
Tests must not hit the network or touch real user-scope paths or other real state.
Wrap inlinespyOnormockImplementationlifecycles intry/finally, or manage them in hooks, somockRestore()always executes.
Close external SDK instances, servers, clients, and transports inafterEachorafterAll, not in test bodies.
Configuregit user.emailanduser.namelocally aftergit initand before committing in temporary repositories; never rely on global Git identity.
Inject small threshold values into threshold tests instead of generating thousands of file...
Files:
tests/helpers/rules-source.test.tstests/helpers/rules-shim.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/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-006-dependency-policy.md)
**/*.{ts,tsx}: Prefer Bun built-ins for file I/O, HTTP, globbing, testing, and subprocess execution; prefernode:built-in modules over npm alternatives when appropriate.
UseBun.spawnwith array-based arguments for all subprocess execution; do not useBun.$because it can hang on Windows.
Do not add npm packages for functionality already provided by Bun, such asglob,chalk, or utility libraries used for a single function.
Use Bun APIs such asBun.file()instead of Node.js-specific APIs such asfs.readFile()when Bun provides an equivalent.
Use relative imports with Bun's native module resolution; do not use TypeScript path aliases.Use TypeScript strict mode with ESNext and ES modules; derive schema types with
z.infer<>rather than defining separate interfaces.
Files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.md)
tests/**/*.test.ts: Usetest.each()for the same assertion logic against multiple independent inputs, anddescribe.each()when each input requires a group of related tests. Do not register tests or run independent assertions insidefor/.forEachloops.
Use array rows for positionaltest.each()arguments and object rows for named fields, with descriptive title placeholders such as%s,%p,%d, or$field.
Assert derived facts with specific matchers rather than collapsing booleans into.toBe(true)or.toBe(false): compare values directly with.toBe()/.toEqual(), use.toContain()or.toMatch()for membership and substrings,.toBeInstanceOf(Array)for array checks,.toHaveLength()for counts, and.find()with.toBeDefined()/.toBeUndefined()for predicate existence checks.
Do not precompute a boolean solely for assertion; assert directly on the underlying values so failures expose the expected and received values.
When converting a loop totest.each()ordescribe.each(), preserve every assertion that ran per iteration; do not drop or merge assertions across cases.
Files:
tests/helpers/rules-source.test.tstests/helpers/rules-shim.test.ts
{src,tests,lint,scripts,shims}/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/GEN-004-concise-forward-only-code-comments.md)
{src,tests,lint,scripts,shims}/**/*.ts: Project-authored TypeScript comments must be concise, describe current behavior only, and never narrate history, relocations, refactors, or how the code came to be.
A contiguous run of whole-line comments must contain at most five lines of narrative prose; longer rationale belongs in an ADR, agent-memory file, issue, or PR with a pointer. Tests and fixtures follow the same limit.
Use structural TSDoc tags such as@param,@returns,@throws,@example, and@seefor structured documentation; tagged sections are exempt from the five-line narrative bound, while@remarks,@description,@summary,@notes,@todo, and@fixmeremain counted as prose.
Files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
**/*.{js,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.archgate/adrs/GEN-003-tool-invocation-via-scripts.md)
Invoke linting, formatting, and validation through package scripts (
bun run lint,bun run format,bun run format:check, andbun run validate), rather than directly invoking tool binaries such asbunx prettier,bunx oxfmt,npx eslint, oroxlint.
Files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.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/rules-source.test.tspackage.jsonsrc/helpers/rules-source.tstests/helpers/rules-shim.test.tstsconfig.jsonsrc/helpers/rules-shim.ts
package.json
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-017-multi-ecosystem-distribution.md)
The root
package.jsonmust not define a top-levelmainfield, so the npm shim does not bundle the CLI entry point into the published tarball.
package.json: Keep production dependencies minimal and limited to the approved packages:@commander-js/extra-typings,inquirer, andzod. New production dependencies require justification, dependency-tree review, and maintainer approval.
KeepdevDependenciesminimal and restricted to development tooling such as linting, formatting, commit conventions, and type declarations.Treat
package.jsonversionas the canonical source of truth for the CLI version.
Files:
package.json
*
📄 CodeRabbit inference engine (.archgate/adrs/GEN-005-repository-root-contents-allowlist.md)
*: Any newly added root-level file must be added to the allowlist in the same change, with the applicable Decision criterion documented in the PR description or commit message.
Place one-off scripts, scratch files, and exploratory helpers inscripts/or a gitignored scratch directory, never directly in the repository root.
Use;or an EXITtrapfor temporary-file cleanup that must run regardless of command failure; do not chain the script and cleanup with&&.
Prefer explicit paths withgit addinstead of habitually usinggit add -Aorgit add .when throwaway files may exist.
Rungit status, or otherwise inspect staged paths explicitly, before committing when scratch files may exist nearby.
Files:
package.jsontsconfig.json
**/package.json
📄 CodeRabbit inference engine (.archgate/adrs/GEN-003-tool-invocation-via-scripts.md)
**/package.json: Everypackage.jsoncontaining lintable or formattable source code must definelint,format,format:check, andvalidatescripts.
Do not add JavaScript/TypeScript linting or formatting tools without defining corresponding package scripts; direct tool binaries are acceptable only inside package.json script bodies.
**/package.json: All production and development dependencies declared inpackage.jsonmust use an SPDX license compatible with Apache-2.0. Approved licenses are MIT, Apache-2.0, ISC, BSD-2-Clause, BSD-3-Clause, 0BSD, CC0-1.0, Unlicense, BlueOak-1.0.0, CC-BY-4.0, CC-BY-3.0, and Python-2.0. SPDXORexpressions are allowed when at least one alternative is approved.
Do not add dependencies with copyleft, source-available, proprietary,Custom,UNLICENSED, or missing license fields; this includes GPL, AGPL, LGPL, and SSPL licenses.
Prefer dependencies with clear SPDX license identifiers in theirpackage.jsonfiles and verify transitive dependency licenses before adding or updating dependencies.
TheLEGAL-002/no-copyleft-depsrule must inspect all direct and transitive packages installed undernode_modules/and fail validation for any package whose license is not on the approved allowlist.
Files:
package.json
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/helpers/rules-source.tssrc/helpers/rules-shim.ts
src/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.md)
src/**/*.ts: Useawait Bun.file(path).json()when reading JSON files in Bun TypeScript source code; do not useJSON.parse(await Bun.file(path).text())orJSON.parse(fs.readFileSync(path, "utf-8"))for file reads.
UseBun.JSONC.parse()when reading files that may contain comments, such astsconfig.json, instead of plainJSON.parse()on file contents.
ReserveJSON.parse()for parsing JSON strings from non-file sources such as API responses or string variables; do not use it as the default for reading JSON files.
src/**/*.ts: All subprocess execution in Archgate source files MUST useBun.spawnwith array-based arguments; do not useBun.$, import$frombun, or usenode:child_process.
Do not use shell features such as pipes, redirects, or globbing inBun.spawnarguments; execute commands directly with separate array arguments.
When capturing subprocess output, configurestdoutandstderras pipes, consume streams withnew Response(...).text(), drain stdout and stderr concurrently, and awaitproc.exited.
Usestdout: "inherit"andstderr: "inherit"for subprocesses whose output belongs on the terminal, and pass command working directories through thecwdoption.
Wrap CLI availability checks intry/catchand return a boolean when the command may not exist on the system.
UsePromise.allSettledfor concurrent subprocesses that may reject, inspecting results only after all processes have settled; do not usePromise.allwhen one rejection could abandon a still-running sibling.
Extract a reusable helper such asrun(cmd, opts)orrunGit(args, cwd)when a module makes several subprocess calls with the same shape.
src/**/*.ts: Heavy runtime dependencies such asinquirer,posthog-node, and@sentry/*must be loaded with dynamicimport()at their point of use, never through top-level static value imports.
Type-only imports for heavy dependencies are allowed, but runtime values must be obtained through dynamic `import()...
Files:
src/helpers/rules-source.tssrc/helpers/rules-shim.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: For user-scope editors, resolve paths using the editor's actual path helper; do not assume Windows conventions. For opencode, mirrorxdg-basedir, which falls back to~/.configon all platforms.
For opencode-gated behavior, useisOpencodeAvailable()rather thanisOpencodeCliAvailable()alone because the Desktop distribution has no CLI binary and shares the config directory.
For Copilot-gated behavior, useisCopilotAvailable()rather thanisCopilotCliAvailable()alone because desktop and CLI distributions share~/.copilot/.
Files:
src/helpers/rules-source.tssrc/helpers/rules-shim.ts
src/helpers/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-002-error-handling.md)
In helper files, use
logInfo()orlogWarn()instead of directconsole.log(),console.warn(), orconsole.info()calls, except for explicitly exempted helper files.
Files:
src/helpers/rules-source.tssrc/helpers/rules-shim.ts
tsconfig.json
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-006-dependency-policy.md)
Do not configure path aliases; use relative imports with Bun's native module resolution.
Files:
tsconfig.json
.archgate/adrs/**/*.{md,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Read relevant self-governance ADRs before architectural changes; ADRs use YAML frontmatter and companion
.rules.tsfiles exporting a plain object satisfyingRuleSet.
Files:
.archgate/adrs/ARCH-022-ast-aware-rule-context.md.archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts
.archgate/adrs/**/*.rules.ts
📄 CodeRabbit inference engine (.archgate/adrs/GEN-004-concise-forward-only-code-comments.md)
.archgate/adrs/**/*.rules.ts: Comments in.archgate/adrs/**/*.rules.tsmust be concise, forward-only, and limited to current behavior; historical or relocation narration is prohibited.
Changes to narration or relocation detection patterns in companion.rules.tsfiles must be synchronized with.archgate/lint/oxlint.ts, and both enforcement layers must continue to report violations at error severity.
Files:
.archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts
.archgate/{lint,adrs}/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/GEN-004-concise-forward-only-code-comments.md)
.archgate/{lint,adrs}/**/*.ts: A contiguous run of whole-line comments must contain at most five lines of narrative prose, including in lint and companion rule implementations.
Use the same synchronized structural-TSDoc exemption in Archgate TypeScript files; narrative must not be relabeled with prose-container tags to evade the limit.
Files:
.archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts
🧠 Learnings (18)
📚 Learning: 2026-07-15T22:55:51.978Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 476
File: tests/helpers/telemetry-config.test.ts:24-28
Timestamp: 2026-07-15T22:55:51.978Z
Learning: In this Bun/TypeScript codebase, when a unit under test spawns subprocesses via Bun.spawn (e.g., running `git credential ...`), prefer overriding relevant env vars (such as `HOME`, `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_NOSYSTEM`) using `process.env` in the test and restoring them with the test utility (e.g., `restoreEnv` from `tests/test-utils.ts`). Avoid relying on `spyOn(os, 'homedir')` for this purpose, because it only affects in-process calls and does not change the environment inherited by subprocesses; env-var overrides should be used for subprocess-level isolation and must be applied at call time.
Applied to files:
tests/helpers/rules-source.test.tstests/helpers/rules-shim.test.ts
📚 Learning: 2026-07-15T22:56:35.415Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 476
File: tests/commands/clean.test.ts:61-62
Timestamp: 2026-07-15T22:56:35.415Z
Learning: When reviewing tests that rely on src/helpers/paths.ts `internalPath()`, note that `internalPath()` intentionally reads `Bun.env.HOME ?? Bun.env.USERPROFILE` at call time and only uses `os.homedir()` if neither env var is set. Therefore, don’t suggest changing tests to `spyOn(os, "homedir")` for this behavior; instead, use per-test `Bun.env.HOME` / `Bun.env.USERPROFILE` overrides (as applicable) so the tests control `internalPath()`’s inputs.
Applied to files:
tests/helpers/rules-source.test.tstests/helpers/rules-shim.test.ts
📚 Learning: 2026-07-25T00:05:58.884Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: tests/helpers/auth.test.ts:38-46
Timestamp: 2026-07-25T00:05:58.884Z
Learning: When reviewing the Archgate CLI repository’s GEN-004 “concise forward-only narration” comments, don’t rely only on the automated phrase-based narration checks. Those checks can pass even when the comment wording describes historical/transfer semantics rather than current behavior (e.g., saying a prior restore “leaked” a value or a later subprocess “inherited it”). Manually verify that the comment describes the code’s current, forward behavior; flag or adjust comments that imply past/historical state transfer even if GEN-004 enforcement passes.
Applied to files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
📚 Learning: 2026-07-25T00:05:59.109Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: src/cli.ts:0-0
Timestamp: 2026-07-25T00:05:59.109Z
Learning: Code comments may include a concise issue/PR reference (per GEN-004) when it’s used to point readers to fuller rationale instead of inlining that rationale. During review, flag surrounding comment prose that reads like historical context or narrates refactors/relocations; a bare GEN-004-style reference is allowed and should not be flagged by itself.
Applied to files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
📚 Learning: 2026-08-04T19:58:05.877Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 543
File: src/helpers/copilot-user-settings.ts:0-0
Timestamp: 2026-08-04T19:58:05.877Z
Learning: In archgate/cli TypeScript code, use `Bun.file(path).exists()` only to check whether a file exists; it must not be used for directory existence checks. For helpers such as `isCopilotAvailable()` that need to detect a configuration directory, use an appropriate directory-aware check such as `existsSync` from `node:fs`.
Applied to files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
📚 Learning: 2026-08-05T06:56:33.435Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 546
File: tests/integration/stream-guards.test.ts:3-9
Timestamp: 2026-08-05T06:56:33.435Z
Learning: When reviewing GEN-004 comment-block limits in the Archgate CLI repository, count only narrative prose lines within a block comment. Do not count a closing delimiter such as `*/` as a prose line; for example, in `tests/integration/stream-guards.test.ts`, Lines 4–8 contain five prose lines while Line 9 contains only the delimiter.
Applied to files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
📚 Learning: 2026-07-25T15:44:40.668Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-005-testing-standards.md:0-0
Timestamp: 2026-07-25T15:44:40.668Z
Learning: In Archgate CLI test code governed by ARCH-007, only allow `Bun.$` in test suites that are explicitly restricted to a single platform. Any cross-platform test that runs on Linux, macOS, and Windows must avoid `Bun.$` and instead use array-based `Bun.spawn`. For shared git setup used by tests, import and use the `git()` helper from `tests/test-utils.ts` rather than duplicating git setup logic.
Applied to files:
tests/helpers/rules-source.test.tstests/helpers/rules-shim.test.ts
📚 Learning: 2026-07-25T22:03:14.216Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-002-error-handling.md:0-0
Timestamp: 2026-07-25T22:03:14.216Z
Learning: In Archgate boundary-wrapped CLI command actions (the handlers that rely on `handleCommandError()` for user-facing error output), expected-failure guards should signal user errors by throwing `new UserError(<message/details>)` rather than directly calling `logError()` followed by `exitWith(1)`. This keeps user-facing logging and the exit path centralized in `handleCommandError()`. For normal/computed command outcomes (e.g., `const exitCode = getExitCode(await runChecks(...))`), use `await exitWith(exitCode)` instead of calling `process.exit(exitCode)` so telemetry/Sentry flushing and outcome tagging still run.
Applied to files:
tests/helpers/rules-source.test.tssrc/helpers/rules-source.tstests/helpers/rules-shim.test.tssrc/helpers/rules-shim.ts
📚 Learning: 2026-07-25T23:21:49.190Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 512
File: tests/engine/git-files.test.ts:98-100
Timestamp: 2026-07-25T23:21:49.190Z
Learning: When reviewing archgate/cli for ARCH-006 (per its ADR frontmatter), only enforce the production-dependency policy scoped to package.json. Do not treat test-only refactors or relocated `node:fs` fixture writes as an ARCH-006 violation (since ARCH-006 does not govern test-file I/O API selection). If there’s a broader/test-wide refactor that would migrate fixture writing to `Bun.write()`, evaluate it separately under the appropriate in-scope rule.
Applied to files:
tests/helpers/rules-source.test.tstests/helpers/rules-shim.test.ts
📚 Learning: 2026-07-27T16:05:38.683Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 536
File: tests/commands/adr/sync-strict.test.ts:173-173
Timestamp: 2026-07-27T16:05:38.683Z
Learning: In this Bun + TypeScript repo, for rejected-promise assertions use the unawaited form: `expect(promise).rejects.toThrow(...)`. Do NOT add `await` to `expect(promise).rejects.toThrow(...)` (Bun’s types model this as `void`), because it will violate the type-aware oxlint rules `typescript(await-thenable)` and `typescript(no-confusing-void-expression)`. Only request an `await` if the repo adopts a typed, lint-compliant assertion helper or Bun’s typings change.
Applied to files:
tests/helpers/rules-source.test.tstests/helpers/rules-shim.test.ts
📚 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/rules-source.tssrc/helpers/rules-shim.ts
📚 Learning: 2026-07-11T13:03:15.386Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 467
File: .archgate/adrs/ARCH-011-consistent-project-root-resolution.md:0-0
Timestamp: 2026-07-11T13:03:15.386Z
Learning: For Markdown files formatted by oxfmt (especially ADRs), avoid inline code spans that contain escaped backticks, e.g. `\`...\`` inside a single `` `...` `` span. oxfmt may mis-parse these and, on re-format, can collapse spaces after later inline code spans on the same line, effectively removing any manually re-added spacing. Instead, rephrase the text so the message stays plain quoted text, and put any embedded command/fragment that needs code formatting (e.g., `archgate init`) in its own separate inline code span; keep surrounding punctuation/spacing outside the code span.
Applied to files:
.archgate/adrs/ARCH-022-ast-aware-rule-context.md
📚 Learning: 2026-07-25T16:24:51.133Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-003-output-formatting.md:0-0
Timestamp: 2026-07-25T16:24:51.133Z
Learning: In Archgate ADRs (.archgate/adrs/*.md), omit quantitative claims (e.g., token savings, benchmarks, performance deltas) unless they are backed by a reproducible measurement and supported by a single cited reference. If you cannot satisfy both (reproducible measurement + exactly one cited reference), describe the benefit qualitatively and tie it to the relevant policy/requirements instead of using numeric estimates.
Applied to files:
.archgate/adrs/ARCH-022-ast-aware-rule-context.md
📚 Learning: 2026-07-25T22:03:17.073Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-015-cli-command-documentation-coverage.md:17-18
Timestamp: 2026-07-25T22:03:17.073Z
Learning: When updating an ADR that documents rule discovery/enforcement behavior, ensure the ADR’s stated discovery contract matches the implementation in code. If the rule only discovers commands by scanning `src/commands/*.ts` and `src/commands/*/index.ts`, the ADR must not claim it also inspects command registration calls elsewhere (e.g., `src/cli.ts`). Any ADR language that changes the documented contract should be treated as a normative change to behavior and aligned with the corresponding implementation/issue, not as prose-only documentation compression.
Applied to files:
.archgate/adrs/ARCH-022-ast-aware-rule-context.md
📚 Learning: 2026-07-26T13:09:49.888Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 533
File: .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md:0-0
Timestamp: 2026-07-26T13:09:49.888Z
Learning: In archgate/cli rule ADRs, `ctx.scopedFiles` is computed from the ADR frontmatter `files` glob patterns before the rule context is constructed. For ARCH-020-style rules, ensure the ADR `files` frontmatter correctly scopes the allowed paths (e.g., `files: ["src/**/*.ts"]`); then rule-specific `.ts`/file filters should assume the incoming file list is already restricted and avoid re-applying the same path-prefix restriction inside individual rules.
Applied to files:
.archgate/adrs/ARCH-022-ast-aware-rule-context.md
📚 Learning: 2026-06-11T12:50:28.661Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 406
File: .claude/agent-memory/archgate-developer/feedback_prefer_tests_over_adr_rules.md:8-18
Timestamp: 2026-06-11T12:50:28.661Z
Learning: In `archgate/cli`, for markdown files under `.claude/agent-memory/`, follow the established convention: use YAML frontmatter (with a `name:` field used as the document title) and do not require a top-level `#` (H1) heading. During code review, do not flag missing first-line/first-top-level H1 headings (e.g., MD041) for these agent-memory files since markdownlint is not part of the repo’s `bun run validate` lint pipeline (oxlint/oxfmt only).
Applied to files:
.claude/agent-memory/archgate-developer/MEMORY.md
📚 Learning: 2026-07-25T00:05:20.592Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: .claude/agent-memory/archgate-developer/project_test_isolation_gotchas.md:10-10
Timestamp: 2026-07-25T00:05:20.592Z
Learning: When reviewing documentation/agent-memory entries under `.claude/agent-memory/**`, do not enforce GEN-004’s “forward-only” comment/narrative requirement. These entries are allowed to keep historical/past-tense incident narratives and dated markers (e.g., `Found YYYY-MM-DD`) because the context is intended to help future agents evaluate edge cases. Outside this scope, GEN-004’s forward-only rule should still apply.
Applied to files:
.claude/agent-memory/archgate-developer/MEMORY.md
📚 Learning: 2026-07-25T18:51:09.926Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 497
File: docs/src/content/docs/reference/rule-api.mdx:218-234
Timestamp: 2026-07-25T18:51:09.926Z
Learning: In archgate/cli, the `rulecontext-shim-parity` automation only checks RuleContext member-name parity between `src/formats/rules.ts` and the generated shim template in `src/helpers/rules-shim.ts`. It does NOT validate RuleContext method signatures or whether types referenced by the shim template actually resolve. Until the automation is extended, treat full RuleContext shim contract parity (including method signatures and referenced type resolution) as manual-review-critical whenever these RuleContext/shim files are changed.
Applied to files:
src/helpers/rules-shim.ts
🪛 OpenGrep (1.26.0)
src/helpers/rules-shim.ts
[ERROR] 28-28: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
.archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts
[ERROR] 280-282: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (10)
src/helpers/rules-source.ts (1)
1-20: LGTM!src/helpers/rules-shim.ts (1)
49-51: LGTM!tests/helpers/rules-shim.test.ts (1)
4-51: LGTM!Also applies to: 64-90
tests/helpers/rules-source.test.ts (1)
1-22: LGTM!.archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts (2)
23-27: LGTM!Also applies to: 275-299
236-268: 📐 Maintainability & Code QualityNo change needed.
src/helpers/rules-source.tsalready readssrc/formats/rules.tssynchronously and documents thebun build --compile --bytecodeconstraint..archgate/adrs/ARCH-022-ast-aware-rule-context.md (1)
10-15: LGTM!Also applies to: 92-96, 146-150
tsconfig.json (1)
35-39: LGTM!.claude/agent-memory/archgate-developer/MEMORY.md (1)
22-42: LGTM!package.json (1)
55-55: 📐 Maintainability & Code QualityNo changes needed.
Root scripts and CI use
bun run typecheck, which runs thescripts/ensure-rules-dts.tsprelude beforetsc --build.
The signature-parity test resolved two anchors but asserted only one. A
renamed or reformatted `readYAML` made `indexOf` return -1, so `end` fell
below `start`, `slice` returned "", and `toContain("")` passed against any
input — the test reported green at the moment the signature it pins had
drifted. ARCH-022 names it as the replacement for the removed member-parity
check, so the silent pass took the only signature and JSDoc guarantee with
it. Assert both anchors before slicing.
`toAmbientDeclarations()` rejected value exports but let a top-level import
through. An import makes the emitted `rules.d.ts` a module, so its
declarations stop being global and every `.rules.ts` loses its types —
surfacing as implicit-`any` across 30 unrelated files rather than at the
cause. Guard it with the same UserError contract.
Reported by CodeRabbit on #547.
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Closes #511.
The problem
generateRulesDts()restatedRuleContextand its supporting types as text inside a template literal. Nothing inbun run validatecompared that text tosrc/formats/rules.ts—tscsees a string, and ARCH-022'srulecontext-shim-paritycompared member names only. A wrong parameter type or a stale JSDoc paragraph shipped to rule authors' editors unflagged.The fix
src/helpers/rules-source.tsinlines the text ofsrc/formats/rules.tsas a Bun macro, andtoAmbientDeclarations()turns its exports into ambient declarations. Signatures and JSDoc travel verbatim, so drift is structurally impossible rather than review-dependent.The macro reads synchronously on purpose: an async macro passes
bun runand plainbun build, then failsbun build --compile --bytecodewith a parse error pointing at the call site rather than the macro. Anwith { type: "text" }import was the other candidate and is rejected bytsc(TS5097 + TS1192).The drift was real
Regenerating from the actual interface changed 24 JSDoc regions. Three lost meaning, not just formatting:
AstNode— a whole sentence missingRubyAstProgram— clause missingRubyAstNode— meaning invertedGovernance
rulecontext-shim-derivedreplacesrulecontext-shim-parity. It fails when the shim stops reading the source through the macro, or transcribes an ambient declaration by hand. It deliberately does not check thatsrc/formats/rules.tsstays type-only:checkregenerates the shim before rules run, so the generator's throw preempts any such branch — an unreachable check would claim coverage it cannot deliver. ARCH-022 says so explicitly.single-ast-methoddrops to one surface, since the shim's declarations now follow by construction.Also in this PR
The ADR companion rules were never typechecked. TypeScript's
includeglobs skip dot-directories, so tsconfig's.archgate/entry resolved to zero files — the 30.rules.tsfiles that enforce every ADR went unchecked. Naming the path explicitly brings them in at a cost of 0 type errors, and the checking is real: plantingctx.thisMethodDoesNotExist()now yieldsTS2339: Property … does not exist on type 'RuleContext'.Two things fell out of that:
typechecknow generates.archgate/rules.d.tsfirst. The rules files reach their ambient types through a triple-slash reference to that gitignored file, so a fresh clone previously failed a standalonebun run typecheckwith TS6053 plus a cascade of implicit-any. CI was safe only becausevalidatehappens to runlintfirst.tests/fixturesleaves the tsc program, andtests/fixtures/fake-registry/packs/test-pack/rules.d.tsis deleted. It was a 58-line stale copy of the 331-line shim, contributing its declarations globally whileskipLibCheckhid the resulting duplicate identifiers. A pack never carries one: it ships.rules.tswhose triple-slash resolves in the consumer project afterarchgate adr importwrites the shim.Sentry classification (CLI-8). A released binary carries
src/formats/rules.tsfrozen from a build CI proved type-only, so the generator's guard is unreachable for end users — only an edited checkout trips it. Throwing a bareErrorrouted that to the exit-2 boundary, which captures to Sentry, filing a contributor's own edit as an archgate defect.UserErrorkeeps it at exit 1 with an actionable hint and no capture. A test pins the class, since a message-only assertion passes either way.Review notes
tests/helpers/__snapshots__/rules-shim.test.ts.snapis the completerules.d.tsa governed project receives. Any future interface edit surfaces here as a readable hunk. Bun fails a missing snapshot whenCIis set, so deleting it routes around nothing.{@link}tags now survive into.archgate/rules.d.ts— better editor tooltips. Existing users' shims self-heal on the nextarchgate check;ensureRulesShimalready content-compares and rewrites.archgate initin a temp project with no access to the source tree. The emittedrules.d.tswas byte-identical to the in-repo derivation, confirming the macro embeds at bundle time rather than reading from disk.Deliberately out of scope
.archgate/lint/*.ts(2 files) stays outside the tsc program. It imports with a literal.tsextension that oxlint needs at runtime, so including it requiresallowImportingTsExtensionsor a resolver change.reference/rule-api.mdxlocales anddocs/public/llms-full.txtfrom engine: RuleContext shim is a hand-maintained copy that nothing verifies #511's surface table. Those are prose for humans, not derivable from the interface.Validation
bun run validateexit 0 — 1978 tests + 1 snapshot,archgate check50/50 with zero warnings.