test: close phase 1-2 coverage gaps and enforce the mock.module ban - #550
Conversation
Covers the 18 source files listed in phases 1 and 2 of #522, raising line coverage from 91.3% to 96.0% (390 missed of 9,738, measured with the CI lcov math on a Windows-only run). Every gap is closed by adding tests; no file under src/ changed. New tests are mostly sibling files because oxlint caps a test file at 500 lines and several targets had no headroom. Raises the CI min-coverage gate from 90 to 95 in both places the workflow hardcodes it, and moves the ARCH-005 stated target to match so the ADR and the gate agree. Three defects surfaced while writing the tests: - pack-recommend.test.ts and adr/sync.test.ts replaced the first-party registry module via mock.module, which ARCH-005 forbids because the call is process-global, retroactive, and not undone by mock.restore(). Nothing had exercised the real shallowClone, so the violation stayed invisible until a new test did and received another file's mock. Both now use spyOn over an "import * as" namespace, as their sync-strict and sync-conflicts siblings already did. The same conversion applies to plugin/install.test.ts and plugin/url.test.ts, leaving the suite with no first-party mock.module. - review-context "respects --domain filter" asserted nothing: its only expect() sat inside a loop over a runtime-derived array that was always empty, because the temp project was never git init'd. It is now git-backed with an unfiltered control test, since asserting one domain would still pass vacuously if only one domain ever appeared. - update-check.test.ts re-imported its module through a cache-busting query-string specifier. Bun treats that as a separate module and attributes its execution to nothing, so the source reported 76% while the paths were in fact tested. Switching to the static import took it to 100% with no new tests. That file also assigned a mock globalThis.fetch with no restore, leaking it into every later test file. Adds lint/no-first-party-module-mock.ts so a new mock.module on a relative specifier carrying a src path segment fails lint instead of surfacing later as an unrelated flake. Third-party specifiers (inquirer, node:readline) are untouched. Each oxlint plugin file needs a meta.name no other plugin uses; a duplicate silently drops the later file's rules and the config then fails with "Rule not found in plugin". Records the vacuous-loop rule in the ARCH-025 Decision section and the new plugin in ARCH-005 Compliance. Both sections were chosen because the Do's and Don'ts of each ADR sit within 20 characters of the review-context briefing cap. Refs #522 Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
|
Warning Review limit reached
Next review available in: 37 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 (9)
📝 WalkthroughWalkthroughThe change raises the enforced total coverage threshold from 90% to 95% and adds an Oxlint rule against relative first-party 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: |
48cb495
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1a5061c8.archgate-cli.pages.dev |
| Branch Preview URL: | https://rhuanbarreto-issue-522-phase.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: 7
🤖 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 @.archgate/adrs/ARCH-005-testing-standards.md:
- Line 184: Update the ARCH-005 CI pipeline guidance to use the package script
entrypoint instead of the bare test command; change the CI contract text around
the CI pipeline bullet so it references bun run test --timeout 60000, matching
the workflow command expected by the testing standards. Keep the rest of the CI
timeout and merge-blocking behavior unchanged.
In `@lint/no-first-party-module-mock.ts`:
- Around line 26-41: Update staticPropertyName() to return the value of computed
string-literal properties, while preserving undefined for other computed
properties; this allows isMockModuleCallee() to recognize mock["module"]. In
tests/lint/no-first-party-module-mock.test.ts lines 60-108, add a mock["module"]
call using a relative src specifier and assert exactly one ARCH-005 violation.
In `@tests/commands/adr/domain/remove.test.ts`:
- Around line 21-24: Update the DomainRemoveJsonSchema definition to use
z.strictObject instead of z.object, ensuring unknown fields in the domain remove
JSON payload cause schema validation to fail while preserving the existing
domain and removed fields.
In `@tests/commands/review-context.test.ts`:
- Around line 254-262: Move the local rejectionMessage helper from
review-context.test.ts into tests/test-utils.ts, export it there, and update
review-context.test.ts to import and use the shared helper. Also replace the
duplicate implementations in registry.test.ts and registry-clone.test.ts with
imports from tests/test-utils.ts, preserving the existing rejection behavior.
In `@tests/commands/upgrade-dispatch.test.ts`:
- Around line 444-502: Strengthen both TTY download tests by capturing the
chunks passed to process.stderr.write and asserting that the progress-line clear
sequence is emitted, rather than merely checking that writeSpy was called. Apply
this to the success test and the failure test around makeProgram().parseAsync
and runUpgrade, preserving their existing success and error assertions.
In `@tests/helpers/plugin-install-cursor-hooks.test.ts`:
- Line 44: Rename and reorganize the split test files to mirror their source
modules: move the Cursor-hook cases from
tests/helpers/plugin-install-cursor-hooks.test.ts into the test layout for
src/helpers/plugin-install.ts, the shallow-clone cases from
tests/helpers/registry-clone.test.ts into the layout for
src/helpers/registry.ts, and the failure cases from
tests/helpers/session-context-opencode-errors.test.ts into the layout for
src/helpers/session-context-opencode.ts. Ensure each resulting file follows the
required module-name.test.ts convention and mirrors the src directory structure.
In `@tests/helpers/session-context-opencode-errors.test.ts`:
- Around line 36-44: Update the afterEach cleanup to call the existing
safeRmSync() helper for tempDir instead of wrapping rmSync() in a catch block.
Preserve the restoreEnv cleanup and ensure the temporary directory removal
remains performed after each test while allowing genuine cleanup failures to
surface.
🪄 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: b0ff263e-a7d5-4bd6-b1e3-fe72bf1987a3
📒 Files selected for processing (31)
.archgate/adrs/ARCH-005-testing-standards.md.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.md.claude/agent-memory/archgate-developer/MEMORY.md.github/workflows/code-pull-request.yml.oxlintrc.jsonlint/no-first-party-module-mock.tstests/commands/adr/create-interactive.test.tstests/commands/adr/domain/remove.test.tstests/commands/adr/import-interactive.test.tstests/commands/adr/sync-conflicts.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/commands/plugin/install.test.tstests/commands/plugin/url.test.tstests/commands/review-context.test.tstests/commands/upgrade-dispatch.test.tstests/commands/upgrade-plugins.test.tstests/engine/context.test.tstests/engine/loader-failure-modes.test.tstests/engine/reporter-diagnostics.test.tstests/helpers/credential-store.test.tstests/helpers/git.test.tstests/helpers/pack-recommend.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry-clone.test.tstests/helpers/registry.test.tstests/helpers/repo-probe.test.tstests/helpers/repo.test.tstests/helpers/session-context-opencode-errors.test.tstests/helpers/update-check.test.tstests/lint/no-first-party-module-mock.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 (15)
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 mirror the
src/structure; shared test fixtures belong undertests/fixtures/.
tests/**/*.ts: Use Bun's built-inbun:testrunner for all tests; do not importnode:test.
Keep filesystem writes inside isolatedmkdtempdirectories and remove temporary directories inafterEachorafterAll.
Test each module's public interface with descriptive assertions; do not reach into private internals.
Every runnable test must contain anexpect()assertion; usetest.skiportest.todofor placeholders rather than assertion-less or silently skipped tests.
Restore every captured environment variable withrestoreEnv(key, original); never restore with a bare assignment such asBun.env.X = original.
Mockos.homedir()usingimport * as modplusspyOn; do not change production code to readBun.env.HOMEmerely to support tests.
Mock first-party modules with namespacespyOnand restore the spies; do not usemock.module()for first-party modules or split production code into-implfiles to avoid mocking restrictions.
Do not hit the network in tests. For HTTP mocking, saveglobalThis.fetchbefore replacing it and restore it directly inafterEach;mock.restore()does not restore direct assignments.
Close external SDK instances, servers, clients, and transports inafterEachorafterAll, not in test bodies.
For temporary Git repositories, configure localuser.emailanduser.nameaftergit initand before committing.
Wrap inlinespyOnormockImplementationusage intry/finallysomockRestore()executes even when assertions fail.
Inject small threshold values into threshold tests instead of generating large fixtures, and do not use a per-test timeout shorter than the globalbun test --timeout 60000.
Useexpect()for 'does not throw' checks, such asexpect(() => fn()).not.toThrow()or `await ...
Files:
tests/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-005-testing-standards.md)
Name test files
<module-name>.test.tsand mirror thesrc/directory structure undertests/.
tests/**/*.test.ts: Usetest.each()ordescribe.each()for the same assertion logic across multiple independent inputs; do not register tests or perform independent assertions insidefor/.forEachloops.
Use array rows for positional test-case arguments and object rows for named fields when usingtest.each()ordescribe.each().
Use descriptivetest.each()/describe.each()title format placeholders such as%s,%p,%d, or$fieldso cases remain independently identifiable.
Assert derived facts with specific matchers and underlying values rather than collapsing boolean expressions into.toBe(true)or.toBe(false); use matchers such as.toEqual(),.toContain(),.toMatch(),.toHaveLength(), and.toBeInstanceOf().
ReplaceArray.isArray(x)assertions withexpect(x).toBeInstanceOf(Array), and replace.some()/.every()boolean assertions with an appropriate matcher or.find()followed by.toBeDefined()/.toBeUndefined().
When iterating over runtime-derived collections, assert the collection itself or its length; the loop must not contain the test's only expectation.
When converting a loop totest.each()ordescribe.each(), preserve every per-iteration assertion without dropping or merging assertions.
Files:
tests/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.ts
.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-005-testing-standards.md.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.md
lint/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-005-testing-standards.md)
Implement test lint plugins that detect assertion-less tests, bare environment restores, and first-party
mock.module()calls; plugin names must be unique.
Files:
lint/no-first-party-module-mock.ts
.github/workflows/*.yml
📄 CodeRabbit inference engine (.archgate/adrs/CI-001-pin-github-actions-by-hash.md)
.github/workflows/*.yml: Do not reference third-party Actions or reusable workflows by mutable tags, branches, or abbreviated SHAs; use a full commit SHA instead.
Version comments after pinned Action SHAs must use the exact release tag, such as# v2.4.3; floating major comments such as# v5are prohibited.
The SLSA reusable workflow underslsa-framework/slsa-github-generator/.github/workflows/*is an explicit exception and must be referenced by its required version tag rather than a SHA.
Local reusable workflow and composite action references, such asuses: ./.github/workflows/...anduses: ./.github/actions/..., must not be SHA-pinned.
Docker container references such asuses: docker://image:tagare exempt from this GitHub Action SHA-pinning requirement.
When adding or updating a pinned Action, resolve annotated tags to the underlying commit SHA, verify that the SHA matches the intended tag using a trusted source, and update the SHA and version comment together.
Before adding an Action to a workflow, audit the Action's required permissions.
For unsupported third-party SHA pinning, do not create silent exceptions; document the limitation in the exceptions list and update the enforcement allowlist before merging.
Files:
.github/workflows/code-pull-request.yml
.github/workflows/code-pull-request.yml
📄 CodeRabbit inference engine (.archgate/adrs/CI-002-validate-workflow-syntax-with-actionlint.md)
.github/workflows/code-pull-request.yml: Installactionlintby downloading an explicitly version-pinnedlinux_amd64release tarball, verifying its SHA-256 checksum withsha256sum -c, and extracting the binary; do not uselatest, an unverified download, or an installer script.
Theactionlintjob MUST invoke./actionlint -colorwithout restricting the path, so it scans the entire.github/workflows/directory.
Setpersist-credentials: falseon theactions/checkoutstep used by theactionlintjob.
Do not add areviewdog/action-actionlintwrapper Action; invoke the externally downloadedactionlintbinary directly in CI.
When upgradingactionlint, update the pinned version and SHA-256 together, sourcing the checksum from that release'sactionlint_<version>_checksums.txtasset.
Files:
.github/workflows/code-pull-request.yml
.github/workflows/*
📄 CodeRabbit inference engine (.archgate/adrs/GEN-003-tool-invocation-via-scripts.md)
.github/workflows/*: CI workflows must invoke linting, formatting, and validation through package scripts and must not directly invoke lint or formatting binaries.
Usebun run testrather than barebun testin CI, so package-script flags are preserved.
Files:
.github/workflows/code-pull-request.yml
.oxlintrc.json
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-005-testing-standards.md)
Register the Bun test hygiene, environment-restore, and first-party-mocking plugins through
jsPlugins; each plugin file must declare a uniquemeta.name.
Files:
.oxlintrc.json
tests/engine/**/*.test.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-022-ast-aware-rule-context.md)
AST behavior must be covered for base parsing, throw-versus-null semantics, comment extraction, opt-in comment absence, source locations, string awareness, Ruby character offsets, and normalized block-comment values.
Files:
tests/engine/context.test.tstests/engine/loader-failure-modes.test.tstests/engine/reporter-diagnostics.test.ts
tests/{commands/plugin/install,commands/plugin/url,helpers/editor-detect}.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
When adding an editor target, update tests that assert exact editor choice lists, including expected length and ID order.
Files:
tests/commands/plugin/url.test.tstests/commands/plugin/install.test.ts
🧠 Learnings (17)
📚 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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/commands/adr/domain/remove.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tslint/no-first-party-module-mock.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.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/commands/adr/sync-conflicts.test.tstests/helpers/registry-clone.test.tstests/commands/adr/create-interactive.test.tstests/commands/adr/import-interactive.test.tstests/helpers/session-context-opencode-errors.test.tstests/lint/no-first-party-module-mock.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/commands/upgrade-plugins.test.tstests/helpers/repo-probe.test.tstests/engine/context.test.tstests/commands/adr/domain/remove.test.tstests/engine/loader-failure-modes.test.tstests/helpers/credential-store.test.tstests/commands/adr/sync.test.tstests/commands/plugin/install-failures.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/engine/reporter-diagnostics.test.tstests/commands/upgrade-dispatch.test.tstests/commands/plugin/url.test.tstests/helpers/pack-recommend.test.tstests/commands/plugin/install.test.tstests/commands/review-context.test.ts
📚 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/registry-clone.test.tstests/helpers/session-context-opencode-errors.test.tstests/helpers/repo.test.tstests/helpers/plugin-install-cursor-hooks.test.tstests/helpers/registry.test.tstests/helpers/repo-probe.test.tstests/helpers/credential-store.test.tstests/helpers/git.test.tstests/helpers/update-check.test.tstests/helpers/pack-recommend.test.ts
📚 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-005-testing-standards.md.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.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-005-testing-standards.md.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.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-005-testing-standards.md.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.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-25T16:25:02.361Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/CI-002-validate-workflow-syntax-with-actionlint.md:36-49
Timestamp: 2026-07-25T16:25:02.361Z
Learning: For any GitHub Actions workflow that installs/downloads `actionlint` (e.g., by downloading the pinned tarball and setting `ACTIONLINT_SHA256`), do not hardcode the SHA256. Instead, source the pinned checksum from the `actionlint` release checksum manifest/manifest reference defined by the corresponding `archgate` ADR (CI-002), and verify the workflow’s referenced release asset name matches the ADR. Reconcile any mismatches between the ADR and the workflow so CI follows the documented checksum/asset wiring.
Applied to files:
.github/workflows/code-pull-request.yml
📚 Learning: 2026-07-25T23:21:11.504Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 512
File: tests/engine/runner-ast-cache.test.ts:82-82
Timestamp: 2026-07-25T23:21:11.504Z
Learning: For Bun/TS tests under tests/engine, it’s acceptable (per ARCH-025) to validate a runtime-sized collection produced by a single operation by looping over items and making direct assertions like `expect(item).toBe(...)` inside the loop. Treat this as an approved alternative to boolean-collapse assertions such as `expect(items.every(predicate)).toBe(true)`. Do NOT conflate this with prohibited “manual loops” that create independent test cases (e.g., calling `test(...)`/`it(...)` inside a loop); that pattern should still be flagged.
Applied to files:
tests/engine/context.test.tstests/engine/loader-failure-modes.test.tstests/engine/reporter-diagnostics.test.ts
🪛 ast-grep (0.45.0)
tests/engine/reporter-diagnostics.test.ts
[warning] 24-24: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${String.fromCodePoint(27)}\\[[0-9;]*m, "gu")
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🔇 Additional comments (26)
tests/engine/context.test.ts (1)
323-358: LGTM!Also applies to: 398-439
tests/engine/loader-failure-modes.test.ts (1)
1-275: LGTM!tests/engine/reporter-diagnostics.test.ts (1)
1-340: LGTM!.archgate/adrs/ARCH-005-testing-standards.md (1)
23-34: LGTM!Also applies to: 156-156, 169-169, 182-183, 185-185
.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.md (1)
38-38: LGTM!.claude/agent-memory/archgate-developer/MEMORY.md (1)
22-23: LGTM!.github/workflows/code-pull-request.yml (1)
277-277: LGTM!Also applies to: 297-297
.oxlintrc.json (1)
5-5: LGTM!Also applies to: 125-125
lint/no-first-party-module-mock.ts (1)
1-24: LGTM!Also applies to: 44-96
tests/lint/no-first-party-module-mock.test.ts (1)
1-58: LGTM!tests/commands/adr/sync.test.ts (1)
21-21: LGTM!Also applies to: 92-93, 106-107, 119-120
tests/commands/plugin/install.test.ts (1)
18-43: LGTM!Also applies to: 84-140
tests/commands/plugin/url.test.ts (1)
16-16: LGTM!Also applies to: 159-191
tests/helpers/pack-recommend.test.ts (1)
3-11: LGTM!Also applies to: 20-20, 266-270, 279-279, 299-299, 319-319, 339-339
tests/helpers/update-check.test.ts (1)
9-18: LGTM!Also applies to: 65-71, 82-83, 237-245, 254-255, 300-300
tests/helpers/credential-store.test.ts (1)
173-209: LGTM!Also applies to: 212-283
tests/helpers/git.test.ts (1)
3-16: LGTM!Also applies to: 98-189
tests/helpers/registry.test.ts (1)
4-25: LGTM!Also applies to: 281-376
tests/helpers/repo-probe.test.ts (1)
148-228: LGTM!tests/helpers/repo.test.ts (1)
3-26: LGTM!Also applies to: 126-163, 215-254
tests/commands/adr/create-interactive.test.ts (1)
1-171: LGTM!tests/commands/adr/import-interactive.test.ts (1)
1-221: LGTM!tests/commands/adr/sync-conflicts.test.ts (1)
1-359: LGTM!tests/commands/plugin/install-failures.test.ts (1)
1-265: LGTM!tests/commands/upgrade-plugins.test.ts (1)
1-164: LGTM!tests/commands/review-context.test.ts (1)
429-461: 📐 Maintainability & Code QualityNo change needed. This test exercises
review-context --run-checks --strictwithout--base, sobuildReviewContext()usesbase === undefined,runCheckspassesbase: undefined,baseRevisnull, andchangedFilesresolves to[]; the exit comes from strict check Summary findings, not git diff resolution.
- lint rule: recognise the computed form mock["module"], which previously slipped past the guard. A computed key that is not a string literal stays unflagged, since its name is not knowable statically. Two cases added. - upgrade-dispatch: both TTY tests are named "clears the progress line" but neither checked that it happened; one asserted only that stderr had been written to at all, the other did not assert on the stream. They now capture the written chunks and assert the clear-line and cursor-to-start sequences. - test-utils: hoist rejectionMessage, which had three byte-identical copies in review-context, registry, and registry-clone tests. - session-context-opencode-errors: use safeRmSync for cleanup. It retries EBUSY/EPERM/ENOTEMPTY, which is how Windows reports a SQLite handle that has not been released yet, and rethrows anything else, so a genuine cleanup failure surfaces instead of being swallowed. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
ARCH-005 described the CI contract as bare `bun test --timeout 60000`. GEN-003 requires tools be invoked through package scripts, and a bare `bun test` applies Bun's 5-second default rather than the 60-second global, which surfaces as timeouts the gate never sees. Both the Compliance bullet and Decision convention 8 now name the script. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
One-line refinement to an agent-memory entry, split out of #550 because it was learned after that PR merged. ## Why The entry already warned that a single-platform local coverage run is a floor rather than the CI figure. It did not say how to obtain the real one — which leaves the warning unactionable, so the natural next step is to plan from the local numbers anyway. That is actively misleading for per-file work. `src/helpers/platform.ts` reads as **64 missed lines** on a Windows-only run and **12** once both platforms are merged, because Linux covers the other branches. Planning from the local view means chasing ~52 lines that are already covered. Updating #522's Phase 3 list ran into exactly this — the original issue listed `platform.ts` in the long tail on the strength of a single-platform reading. ## What Adds the artifact-merge recipe: download both `coverage-linux` and `coverage-windows`, union the `DA:<line>,<hits>` records keyed by the path from `src/` onward, and count a line covered when the summed hits exceed zero. Verified — it reproduces the figure the coverage comment reports for #550 (9,414 / 9,748 = 96.6%) digit for digit, which is what makes the per-file numbers trustworthy enough to plan from. ## Scope `.claude/agent-memory/**` only. No source, tests, ADRs, or CI config. `archgate check` passes 51/51 and `oxfmt --check` is clean; the file is exempt from GEN-004 by convention, and I checked it carries no stray control or non-ASCII characters. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
… long tail (#553) Phase 3 of #522. Covers **333 of the 334** lines the merged Linux+Windows baseline reported uncovered — entirely from `tests/`, with **no production file modified**. Baseline: `9414/9748 = 96.6%` (reproduced from the #550 merge run's `coverage-linux` + `coverage-windows` artifacts, digit for digit). ## Scope | Area | Lines | Notes | | --- | ---: | --- | | `telemetry.ts` | 106 | the phase's policy call — see below | | Platform branches | 51 | `binary-upgrade.ts`, `vscode-settings.ts`, `platform.ts` | | Commands | 60 | `check.ts` (20), `adr-import`, `init-project`, and nine smaller | | Engine + session-context | 56 | `session-context.ts` (14), `ast-support.ts` (13), and eight smaller | | Helpers long tail | 61 | sixteen files, each ≤ 8 lines | ## The telemetry policy call, settled #522 left this open: mock the SDK, or keep the "validated via dashboard" policy and make it explicit with coverage-ignore comments. This takes the first option. The mechanism is worth stating, because it is why the file sat at 54%. `trackEvent` returns early when `NODE_ENV === "test"`, and that single guard is what left the property builders (`getStaticProperties`, `getCommonProperties`) and all three env detectors unexecuted — they are only ever reached *through* a capture. Lifting the guard is only safe once `posthog-node` is faked; otherwise the tests would queue real events against the production project. So the fake is a prerequisite, not a convenience. With it in place, the CI-provider and shell detectors are driven as `test.each` tables and asserted through the captured payload, and the constructor's `options.fetch` wrapper — otherwise unreachable — is invoked directly to cover the network-failure fallback. `mock.module` on a third-party specifier is permitted by ARCH-005 and by `lint/no-first-party-module-mock`. It is process-global and retroactive, which here means any later in-process `initTelemetry()` gets the fake instead of the real SDK — strictly safer than the status quo. The file header says so. ## `process.platform` is writable in Bun The testability seam this was expected to need in `vscode-settings.ts` turned out to be unnecessary. `process.platform` is a writable, configurable data property, so `Object.defineProperty` plus a cache reset reaches every branch of `platform.ts` — and therefore every caller of it — from any runner, with the descriptor restored per test. That matters beyond the diff: CI merges only Linux and Windows, so a `test.skipIf(platform !== "darwin")` test contributes to neither and moves the aggregate by zero. The macOS and WSL paths are now covered by tests that run everywhere. ## One line is not coverable, and the reason generalizes `runner.ts:348` is the `} catch {` token of a branch whose body **is** already exercised — the pre-existing tests emit `DA:350,1` beside `DA:348,0`. Bun emits a never-incrementing record for a `catch` clause line when the body has its own statements. A test was written to confirm it moved nothing, then removed rather than left in the suite as a redundant fixture. The same accounting explains a slice of the residual elsewhere. On Linux, Bun emits zero-hit records for blank lines, comments, and closing braces that Windows omits entirely, so the union pins them at zero permanently. All seven lines the baseline attributed to `prompt.ts` are of exactly this kind — a blank line, two comments, and four closing braces — which is why that file is untouched. **Literal 100% is therefore not attainable**, and the threshold below leaves headroom for it. ## Fixed in passing Two `describe` blocks in `binary-upgrade.test.ts` assigned `globalThis.fetch` directly but relied on `mock.restore()`, which does not undo a direct assignment (ARCH-005). Both now save and restore it. ## Two production defects found, deliberately not fixed here Both were verified empirically rather than inferred, and both are behavioural changes that do not belong in a coverage PR. Filed separately. 1. **`binary-upgrade.ts` can return an unextracted binary on Windows.** `Expand-Archive`'s error is non-terminating, so `powershell -NoProfile -Command "Expand-Archive ..."` exits `0` on a corrupt archive and `downloadReleaseBinary` hands back a path to a binary that was never extracted, instead of raising. 2. **The backslash normalization at `binary-upgrade.ts:236` can never fire.** GNU tar escapes backslashes in `-tzf` output, so a member stored as `..\evil` is listed as `..\evil` and normalizes to `..//evil`, matching none of the three guard conditions. ## Verification `bun run validate` clean: **2358 pass / 0 fail / 25 skip**, `archgate check` 51/51 with zero warnings, knip and build check clean. Refs #522 ## Ratchet `min-coverage` moves **95 -> 99.5**, in `code-pull-request.yml` (both the action input and the enforcing step, which compare with `awk` and so handle the decimal as a float) and in ARCH-005's stated target, so the ADR and the gate agree. The floor sits ~39 lines below the measured 99.9% rather than tracking it exactly, to leave room for the uncoverable residue described above. | Directory | Coverage | | --- | --- | | `src/commands/` | 100.0% (2358 / 2358) | | `src/engine/` | 100.0% (2593 / 2594) | | `src/formats/` | 100.0% (151 / 151) | | `src/helpers/` | 99.8% (4617 / 4626) | The ten remaining lines are exactly the ones named above: `runner.ts:348`, the seven `prompt.ts` artifacts, and `auth.ts:158` — a closing brace inside `pollForAccessToken`'s `if ("error" in data)` block that no input can reach, since `DeviceTokenResponseSchema` guarantees either `access_token` or `error` and every path through the block ends in `continue` or `throw`. --------- Signed-off-by: Rhuan Barreto <rhuan.barreto@gmail.com> Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Implements phases 1 and 2 of #522.
Result
Line coverage 91.3% → 96.00% (390 missed of 9,738), measured with the CI action's own lcov math (
sum(LH)/sum(LF)filtered tosrc/*) on a Windows-only run — the merged Linux+Windows figure CI computes can only be higher.All 18 targeted source files reach 100% line coverage, achieved entirely by adding tests. No file under
src/changed. That was a deliberate constraint: coverage bought by reshaping production code isn't coverage, and the issue's premise that these gaps were testable as-is held up.13 new test files, 13 modified. Most new files are siblings rather than additions, because oxlint caps a test file at 500 lines and several targets (
import.test.ts479,plugin-install.test.ts474,sync.test.ts469) had no headroom.min-coveragegoes 90 → 95 in both placescode-pull-request.ymlhardcodes it, and ARCH-005's stated target moves to match in all four places it appears — leaving the ADR at 90 while CI enforced 95 would be exactly the doc/enforcement drift ADRs exist to prevent.Three defects the tests surfaced
A
mock.module()leak that had been latent indefinitely.pack-recommend.test.tsandadr/sync.test.tsreplaced the first-party registry module wholesale — which ARCH-005 already forbids, becausemock.moduleis process-global, retroactive, and not undone bymock.restore(). Nothing in the suite had ever exercised the realshallowClone, so the violation was invisible. The moment a new test did, five tests failed with another file's mock.sync.test.tswas the worse of the two: it replaced the module with just two exports, stripping the rest for every later importer.Fixing it also raised coverage (95.56% → 95.97%) — the leak had been suppressing real execution, not just causing failures.
A test that asserted nothing.
review-context→respects --domain filterput its onlyexpect()inside a loop over a runtime-derived array that was always empty, because the temp project was nevergit init'd. Permanently green; would have stayed green if--domainbroke entirely. Note the repo's ownbun-test/expect-expectplugin cannot catch this — theexpect()is lexically present. Rebuilt git-backed with an unfiltered control test, since asserting["architecture"]alone would still pass vacuously if only one domain ever appeared.Cache-busting dynamic imports silently hide coverage.
update-check.test.tsre-imported its module via a?t=${Date.now()}specifier. Bun loads that as a separate module instance and attributes its execution to nothing, so the reporter showed 76% while the paths were genuinely tested. Removing it took the file to 100% with zero new tests. It was also assigning a mockglobalThis.fetchwith no restore, leaking into every later test file.New lint rule
lint/no-first-party-module-mock.ts(test-mocking/no-first-party-module-mock, scoped totests/**/*.test.ts) fails the build on anymock.module()whose specifier is relative and carries asrcpath segment.inquirerandnode:readlineare untouched — they have no namespace object to spy.The suite now contains zero first-party
mock.module()calls, which is what unblocked writing the rule at all.Fire-tested in both directions, since a clean lint run alone would only prove the rule never fires: it reports the first-party call and stays silent on the third-party ones in the same file, and the whole real
tests/tree lints clean.tests/lint/no-first-party-module-mock.test.ts(26 cases) pins both directions permanently.For the reviewer
lint/no-first-party-module-mock.tsmust land together — reverting one without the other leaves the ADR making a false claim.context.filename, and I preferred not to depend on an unverified API. Verified sound for this repo: every/src/specifier undertests/points at the real source tree, and no fixture directory is namedsrc. If someone later addstests/fixtures/src/, this would false-positive.meta.name. Reusingbun-testsilently dropped the new file's rules and failed the config withRule not found in plugin. Documented in ARCH-005 so the next plugin author doesn't lose time to it.review-contextbriefing cap and ARCH-025's has 20, so neither could take a new bullet. The vacuous-loop rule went into ARCH-025's Decision (a definitional refinement, and 1065 chars free) and the plugin into ARCH-005's Compliance, which isn't briefed at all.archgate checkconfirmsbriefingWarnings: [].Out of scope
MIN_COVERAGEliteral is duplicated across two workflow steps — deliberate, per the template-injection comment on the shell step.test.skipIf(platform !== "win32")tests remain incredential-store.test.ts. Phase 3 territory.Validation
bun run validate:coverageexits 0 — lint, typecheck, format, 2161 pass / 21 skip / 0 fail,archgate check51/51 with zero warnings, knip, and build check.