fix: bound scan loop, narrow install-method types, and extend ARCH-021 to markdown code spans - #548
Conversation
The scan loop advanced its cursor from inside the body, so termination
depended on every path through the body remembering to advance it. The
advance now lives in the for-statement's increment slot, where no
`continue` can skip it, and `indexOf` from `found + 1` returns either
-1 or a strictly greater index — bounding the scan to source.length
iterations by construction.
An empty needle is the one input that never terminated: `indexOf("", n)`
clamps to source.length instead of returning -1, so the cursor stopped
advancing. Every caller anchors on a non-empty token today, so this was
latent rather than reachable; it is now guarded, since no anchor means
no position and the caller already falls back to line 0.
Closes #540
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
The cache and the return type were both plain `string`, which discarded the four values the function can actually produce and left a `!== ""` cache-hit guard that no code path could falsify. Typing both as the `InstallMethod` union restores that information for every caller and makes the redundant comparison unwritable rather than merely unreachable. The type is the enforcement here, not the linter: with `typeAware` on, `no-unnecessary-condition` flags a `string !== undefined` control in the same function but does not flag the narrowed union against `""`, so removing the guard on the strength of a clean lint run would not have been sound. `InstallMethod` is distinct from upgrade.ts's same-named type, which carries the command needed to perform an upgrade; this one only names the source. Closes #541 Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
CommonMark gives backslash escapes no meaning inside an inline code span, so a backslash written to keep a backtick inside a span does the opposite: the backtick closes the span and every later delimiter on the line re-pairs one position out. The result is valid markdown that renders wrongly, which format:check cannot see because the file is already in normal form. GEN-006 adds a companion rule that walks each line over its code-span delimiters and reports a backslash-escaped backtick found in text content. Walking spans rather than grepping for the byte pair is what lets it pass a span whose content legitimately ends in a backslash, such as a Windows path — a shape this repository's docs use for Windows paths on four lines. ARCH-007 and ARCH-010 are brought in line with the rule. Closes #515 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 (2)
📝 WalkthroughWalkthroughThis change adds ARCH-021 authored-text integrity guidance and a new Archgate rule set that checks ASCII-only PowerShell files and escaped backticks in Markdown and MDX text. It removes escaped inline-code formatting from ARCH-007 and ARCH-010 examples. It updates 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
Two facts no check can reach: a bun -e fixture loses one level of escaping on the way into the file, which turns a fire-test into a false negative that reads as a broken rule; and no-unnecessary-condition does not flag a narrowed literal union against a literal outside it, so a clean lint run is not evidence about a dead comparison either way. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Deploying archgate-cli with
|
| Latest commit: |
4b46ab3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://19d59926.archgate-cli.pages.dev |
| Branch Preview URL: | https://rhuanbarreto-verify-issues-5.archgate-cli.pages.dev |
Code Coverage
Full HTML report available in workflow artifacts. Per-directory breakdown
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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 @.archgate/adrs/GEN-006-markdown-code-span-integrity.rules.ts:
- Around line 77-79: GEN-006 requires the fence scanner around FENCE.test to
track the opening marker and length, then close only on the same marker with at
least that length and only spaces or tabs afterward; add regression cases for
shorter, opposite-marker, and text-suffixed runs. In
.archgate/adrs/GEN-006-markdown-code-span-integrity.rules.ts lines 77-79,
implement the stateful matching-fence logic. In
.archgate/adrs/GEN-006-markdown-code-span-integrity.md lines 77-78, retain the
“only suppress reports” guarantee only after matching closing-fence detection is
enforced.
In `@src/helpers/install-info.ts`:
- Around line 25-35: Restore detectInstallMethod() to return the installation
metadata object expected by the upgrade command, including type, protoCmd, cmd,
args, and manualHint; narrow only the type field to InstallMethod. Ensure its
cached value uses the same object shape and remains compatible with the
method.type-based dispatch in the upgrade flow.
🪄 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: 74adfc5c-3546-400c-9e9e-3ae3e0008975
📒 Files selected for processing (7)
.archgate/adrs/ARCH-007-cross-platform-subprocess-execution.md.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.md.archgate/adrs/GEN-006-markdown-code-span-integrity.md.archgate/adrs/GEN-006-markdown-code-span-integrity.rules.tssrc/engine/source-positions.tssrc/helpers/install-info.tstests/engine/source-positions.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)
.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-007-cross-platform-subprocess-execution.md.archgate/adrs/GEN-006-markdown-code-span-integrity.rules.ts.archgate/adrs/GEN-006-markdown-code-span-integrity.md.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.md
.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/GEN-006-markdown-code-span-integrity.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/GEN-006-markdown-code-span-integrity.rules.ts
src/**/!(*platform).ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)
src/**/!(*platform).ts: Insrc/TypeScript source files, do not readprocess.platformdirectly; usesrc/helpers/platform.ts(isWindows(),isMacOS(),isLinux(),isWSL(),getPlatformInfo()) for all platform detection.
Use the centralized platform helper instead of duplicating OS/WSL detection logic inline anywhere insrc/TypeScript source.
When behavior differs between Linux and Windows, account for WSL by usingisWSL()rather than assuming `
Files:
src/helpers/install-info.tssrc/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-018-lazy-load-heavy-dependencies.md)
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 dynamicimport(); for example, useimport type { PostHog } from "posthog-node".
SDKs that require early initialization may use eager-start/lazy-await: begin initialization before command registration and await the result at first use, such as in apreActionhook.
src/**/*.ts: In all TypeScript source files undersrc/, useBun.envfor every environment-variable read and write; never referenceprocess.env, including in comments.
AccessBun.envproperties directly; do not create wrapper functions around it or destructure it.
Use nullish coalescing for environment-variable defaults, such asBun.env.NODE_ENV ?? "production".
For truthy environment-flag checks, useBoolean(Bun.env.FLAG)only inline, as part of a larger expression, or assign it to a variable before using it as a sole condition; otherwise use an explicit defined-and-nonempty comparison.
src/**/*.ts: Everyinquirer.prompt(...)call must be wrapped inwithPromptFix(() => ...)imported fromsrc/helpers/prompt.ts; keep the wrapper adjacent to the prompt invocation so automated checks can detect it.
Do not callinquirer.prompt(...)directly or reimplement cursor/newline fixes at individual call sites; route all prompt behavior throughwithPromptFix().Every call to
Bun.Glob#scan()(glob.scan(...)) in source must pass{ dot: true }in its options object, including scans whose patterns do not explicitly target dot-directories. Do not usedot: false; intentionally excluded dotfiles must be filtered explicitly after scanning with a comment. Normalize scanned path separators withfile.replaceAll("\\", "/")when performing cross-platform path comparisons.
src/**/*.ts: Use `sty...
Files:
src/helpers/install-info.tssrc/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.test.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/install-info.tssrc/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.test.ts
src/engine/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md)
src/engine/**/*.ts: The rules engine must list files by matching globs in memory against the git-tracked file set, rather than walking the filesystem.
runChecksmust share per-run caches across rule contexts: cache glob results by pattern and tracked mode, cache file text by absolute path using promises, copy cached glob arrays before returning them, and do not cache mutablereadJSONresults.
Do not filter filesystem scan results against the tracked set as a substitute for in-memory matching, and do not hardcode ignore directories; Git's ignore-aware file listing is authoritative.
src/engine/**/*.ts: TS/JS AST parsing MUST reuse the shared in-processmeriyahparser primitive; scanner parsing andctx.ast()parsing must not duplicate inlineparseModule()implementations.
Python and Ruby AST parsing MUST use their standard-library AST facilities through guardedBun.spawnsubprocesses, without third-party parsers.
WithincreateRuleContext(), AST guardrails MUST run in order: path safety, language plausibility, interpreter availability probe, then guarded subprocess invocation. The probe must be cached percheckinvocation.
Rule execution MUST NOT directly accessBun.spawn,child_process, or other subprocess/filesystem primitives;ctx.ast()is the sole sanctioned door for AST subprocess work.
Python subprocesses MUST use the-Iisolation flag; subprocess arguments MUST be array-based, with no shell interpolation of paths or source contents.
ctx.ast()MUST throw on unavailable interpreters, parse failures, missing base revisions, or files absent at base; it must never returnnullor another sentinel.fileAtBase()is the only exception.
ast(path, language, { rev: "base" })andfileAtBase(path)MUST read the same merge-base commit used bychangedFiles; git operations remain insrc/engine/git-files.ts.
ast(..., { comments: true })MUST opt in to a rootcommentsarray of structuredCommentTokenvalues; comments must not appear ...
Files:
src/engine/source-positions.ts
tests/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)
In test files, use
_resetPlatformCache()to simulate different platforms instead of mocking or mutatingprocess.platformdirectly.
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/engine/source-positions.test.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/engine/source-positions.test.ts
tests/engine/**/*.test.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-022-ast-aware-rule-context.md)
Test AST base-revision parsing, comment extraction and opt-in behavior, cross-language handling, comment-only structural equivalence, character-offset conversion, and throw-versus-
nullsemantics.
Files:
tests/engine/source-positions.test.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: archgate/cli
Timestamp: 2026-08-05T10:57:30.662Z
Learning: Reviewers must interpret mixed prose and backticks according to CommonMark delimiter pairing rather than the author's apparent intent, and treat escaped-backtick violations as rendering defects.
📚 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-007-cross-platform-subprocess-execution.md.archgate/adrs/GEN-006-markdown-code-span-integrity.md.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.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-007-cross-platform-subprocess-execution.md.archgate/adrs/GEN-006-markdown-code-span-integrity.md.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.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-007-cross-platform-subprocess-execution.md.archgate/adrs/GEN-006-markdown-code-span-integrity.md.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.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-007-cross-platform-subprocess-execution.md.archgate/adrs/GEN-006-markdown-code-span-integrity.md.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.md
📚 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/install-info.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.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:
src/helpers/install-info.tssrc/engine/source-positions.tstests/engine/source-positions.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/engine/source-positions.test.ts
📚 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/source-positions.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/engine/source-positions.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/engine/source-positions.test.ts
🪛 LanguageTool
.archgate/adrs/GEN-006-markdown-code-span-integrity.md
[uncategorized] ~39-~39: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...L frontmatter, which is not rendered as markdown. A backslash that escapes another backs...
(MARKDOWN_NNP)
[style] ~49-~49: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd the space is stripped on render. - DO split a sentence instead, when a long...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~49-~49: To elevate your writing, try using a synonym here.
Context: ...stead, when a longer delimiter would be hard to read: give the inner command its own...
(HARD_TO)
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e the surrounding message as prose. - DO treat a `GEN-006/no-escaped-backtick-...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...it causes continues to end of line. - DO put illustrative examples of the brok...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e delimiters really are unbalanced. - DON'T extend the generated-file exemptio...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.23.2)
.archgate/adrs/GEN-006-markdown-code-span-integrity.md
[warning] 19-19: Spaces inside code span elements
(MD038, no-space-in-code)
🔇 Additional comments (4)
src/engine/source-positions.ts (1)
134-148: LGTM!tests/engine/source-positions.test.ts (1)
68-88: LGTM!.archgate/adrs/ARCH-007-cross-platform-subprocess-execution.md (1)
16-16: LGTM!Also applies to: 63-63
.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.md (1)
11-36: LGTM!Also applies to: 48-58
A fence closes only on its own marker, run at least as long as the opener, with nothing but whitespace after it. Treating every fence-like line as a toggle ended the block on a nested shorter run, exposing the literal content after it to the code-span scan and inventing a violation inside a fenced example. The failure direction is now suppression only, which is what GEN-006's Risks section claims. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
The escaped-backtick check and the ASCII-only PowerShell check enforce one invariant: authored text must not carry a sequence that a downstream reader decodes differently from the way it was written. Both are lexical line scans for a corruption that format:check, oxlint, and tsc are all structurally unable to see, so they belong to one decision rather than two. ARCH-021 widens to cover .ps1, .md, and .mdx and is retitled for the invariant it now states. A standalone ADR bought nothing here: rules are discovered through ADRs, so a second ADR was the price of a second rule, not a second decision. Closes #515 Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
rhuanbarreto
changed the title
|
Restructured on review feedback: The reasoning: a standalone ADR bought nothing here. Rules are discovered through ADRs, so a second ADR was the price of a second rule, not of a second decision — and the decision was already made. The escaped-backtick check and the existing ASCII-only PowerShell check enforce one invariant: authored text must not carry a sequence that a downstream reader decodes differently from the way it was written. Both are lexical line scans for a corruption
I confirmed the "rules without an ADR" shape is genuinely unavailable rather than merely unconventional: with the ADR removed and only the Re-verified after the move, four ways:
The two earlier findings are unaffected and remain resolved — the fence-pairing fix moved across intact, and the @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Changed files ==='
git diff --name-status "$(git merge-base HEAD origin/main)" HEAD 2>/dev/null || git diff --name-status HEAD~1 HEAD
printf '%s\n' '=== ARCH-021 candidates ==='
fd -a 'ARCH-021*' .archgate 2>/dev/null || true
printf '%s\n' '=== Rule loader candidates ==='
rg -n -C 3 '\bloadRuleAdrs\b|rules:\s*true|\.rules\.ts' src .archgate -g '*.ts' 2>/dev/null || true
printf '%s\n' '=== Source position changes ==='
git diff "$(git merge-base HEAD origin/main)" HEAD -- src/engine/source-positions.ts tests/engine/source-positions.test.ts 2>/dev/null || trueLength of output: 20516 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Rule file outline ==='
ast-grep outline .archgate/adrs/ARCH-021-authored-text-integrity.rules.ts --items all --view expanded || true
printf '%s\n' '=== ADR frontmatter and enforcement sections ==='
rg -n -C 8 '^(---|id:|title:|files:|rules:|## (Decision|Compliance|Enforcement|Scope))' \
.archgate/adrs/ARCH-021-authored-text-integrity.md
printf '%s\n' '=== Full companion rule ==='
cat -n .archgate/adrs/ARCH-021-authored-text-integrity.rules.ts
printf '%s\n' '=== Loader selection implementation ==='
sed -n '260,370p' src/engine/loader.ts
printf '%s\n' '=== Tests and references for ARCH-021 rules ==='
rg -n -C 5 'ascii-only-ps1|no-escaped-backtick-in-markdown|ARCH-021' \
tests .archgate -g '*.test.ts' -g '*.spec.ts' -g '*.ts' -g '*.md' 2>/dev/null || true
printf '%s\n' '=== Exact new-file diff ==='
git diff "$(git merge-base HEAD origin/main)" HEAD -- \
.archgate/adrs/ARCH-021-authored-text-integrity.md \
.archgate/adrs/ARCH-021-authored-text-integrity.rules.tsLength of output: 43906 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
def find_closing_run(line, start, length):
i = start
while i < len(line):
if line[i] != "`":
i += 1
continue
run = 0
while i + run < len(line) and line[i + run] == "`":
run += 1
if run == length:
return i
i += run
return -1
def first_escape_column(line):
i = 0
while i < len(line):
if line[i] == "\\":
if i + 1 < len(line) and line[i + 1] == "`":
return i + 1
i += 2
continue
if line[i] != "`":
i += 1
continue
run = 0
while i + run < len(line) and line[i + run] == "`":
run += 1
close = find_closing_run(line, i + run, run)
i = i + run if close == -1 else close + run
return None
cases = {
"ADR documented invalid form": r'`UserError("Run \`archgate init\` first")`',
"Windows path ending in a backslash": r'`C:\work\`',
"escape outside a span": r'prose \` text',
}
for name, line in cases.items():
print(f"{name}: column={first_escape_column(line)!r}; line={line}")
PYLength of output: 1528 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
def find_closing_run(line, start, length):
i = start
while i < len(line):
if line[i] != "`":
i += 1
continue
run = 0
while i + run < len(line) and line[i + run] == "`":
run += 1
if run == length:
return i
i += run
return -1
def first_escape_column(line):
i = 0
while i < len(line):
if line[i] == "\\":
if i + 1 < len(line) and line[i + 1] == "`":
return i + 1
i += 2
continue
if line[i] != "`":
i += 1
continue
run = 0
while i + run < len(line) and line[i + run] == "`":
run += 1
close = find_closing_run(line, i + run, run)
i = close + run if close != -1 else i + run
return None
line = r"`show \`literal tick`"
print(f"line={line}")
print(f"reported_backslash_column={first_escape_column(line)!r}")
PYLength of output: 1198
✏️ Learnings added
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.archgate/adrs/ARCH-021-authored-text-integrity.rules.ts (1)
49-77: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCheck escaped backticks before changing code-span state.
firstEscapeColumn()scansbackslash + backtickbefore opener handling, but it skips that pair at the start of a span because it then treats the backtick as the delimiter. A backslash cannot escape a backtick delimiter in CommonMark, so such lines should be reported before the next matching backtick closes the span.
.archgate/adrs/ARCH-021-authored-text-integrity.rules.ts#L49-L77: Report escaped backtick pairs before treating the following backtick as an opener, and add a regression such as`show \`literal tick`..archgate/adrs/ARCH-021-authored-text-integrity.md#L23: Remove the two backslash-backtick pairs from the explanatory prose outside the fenced example.🤖 Prompt for 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. In @.archgate/adrs/ARCH-021-authored-text-integrity.rules.ts around lines 49 - 77, Update firstEscapeColumn in .archgate/adrs/ARCH-021-authored-text-integrity.rules.ts (lines 49-77) to detect backslash-backtick pairs before treating the backtick as a code-span opener, while preserving delimiter scanning behavior; add a regression for `show \`literal tick`. In .archgate/adrs/ARCH-021-authored-text-integrity.md (line 23), remove the two backslash-backtick pairs from the explanatory prose outside the fenced example.Source: Learnings
🤖 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-021-authored-text-integrity.rules.ts:
- Around line 91-116: Update the line-scanning logic around firstEscapeColumn()
in ARCH-021-authored-text-integrity.rules.ts to preserve active code-span
delimiter state across physical lines, so escaped backticks inside multiline
spans are ignored until the matching close; add the requested double-backtick
multiline fixture with a later close. Update ARCH-021-authored-text-integrity.md
lines 89-90 to document that the rule must avoid both suppressing and inventing
reports for content inside multiline code spans.
---
Outside diff comments:
In @.archgate/adrs/ARCH-021-authored-text-integrity.rules.ts:
- Around line 49-77: Update firstEscapeColumn in
.archgate/adrs/ARCH-021-authored-text-integrity.rules.ts (lines 49-77) to detect
backslash-backtick pairs before treating the backtick as a code-span opener,
while preserving delimiter scanning behavior; add a regression for `show
\`literal tick`. In .archgate/adrs/ARCH-021-authored-text-integrity.md (line
23), remove the two backslash-backtick pairs from the explanatory prose outside
the fenced example.
🪄 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: 43d672ff-ca01-4636-8a76-4c99804a4641
📒 Files selected for processing (5)
.archgate/adrs/ARCH-021-ascii-only-powershell-scripts.md.archgate/adrs/ARCH-021-ascii-only-powershell-scripts.rules.ts.archgate/adrs/ARCH-021-authored-text-integrity.md.archgate/adrs/ARCH-021-authored-text-integrity.rules.ts.claude/agent-memory/archgate-developer/MEMORY.md
💤 Files with no reviewable changes (2)
- .archgate/adrs/ARCH-021-ascii-only-powershell-scripts.md
- .archgate/adrs/ARCH-021-ascii-only-powershell-scripts.rules.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (3)
.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-021-authored-text-integrity.md.archgate/adrs/ARCH-021-authored-text-integrity.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-021-authored-text-integrity.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-021-authored-text-integrity.rules.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: archgate/cli
Timestamp: 2026-08-05T11:57:29.098Z
Learning: Extract a helper such as `run(cmd, opts)` or `runGit(args, cwd)` when a module performs several subprocess calls with the same shape.
📚 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-021-authored-text-integrity.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-021-authored-text-integrity.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-021-authored-text-integrity.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-021-authored-text-integrity.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
🪛 LanguageTool
.archgate/adrs/ARCH-021-authored-text-integrity.md
[uncategorized] ~53-~53: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...s) $errs ``` No escaped backticks in markdown. Markdown and MDX files MUST NOT cont...
(MARKDOWN_NNP)
[uncategorized] ~66-~66: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... character to delete: the backslash for markdown, the offending character for .ps1 ##...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.23.2)
.archgate/adrs/ARCH-021-authored-text-integrity.md
[warning] 23-23: Spaces inside code span elements
(MD038, no-space-in-code)
🪛 OpenGrep (1.26.0)
.archgate/adrs/ARCH-021-authored-text-integrity.rules.ts
[ERROR] 27-27: 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 (1)
.claude/agent-memory/archgate-developer/MEMORY.md (1)
29-29: LGTM!Also applies to: 41-41
A CommonMark code span may cross a line break and close on a later line of the same block. Restarting the walk at each line read that span's continuation as text, so a backslash-backtick inside it was reported against literal content. The scan now threads the open backtick-run length from line to line and clears it at a blank line, since a span cannot outlive its block. A run left unmatched on its line is treated as still open rather than as text, which keeps the failure direction on suppression. ARCH-021's Negative bullet is restated to match: the check is block-scoped, and an unmatched run costs reports to the next blank line. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
|
Re the outside-diff finding in review Why the reordering can't workThe suggested regression case and a legitimate Windows path are the same bytes. In both, the span's last content character is a backslash and the delimiter that follows closes the span: Tokenized, the structure is identical — the character immediately before the closing delimiter is a backslash in each. Nothing lexical distinguishes them; the only difference is what the author meant, which the scanner cannot see. So reporting the first necessarily reports the second. MeasuredI implemented the suggestion (report a backslash-backtick pair before treating the following backtick as a delimiter) and ran it over the repository's markdown:
All seven are correct-rendering content: six are the Windows path rows in the opencode integration guide across three locales, and the seventh is ARCH-021's own line 23 — which is the second part of the finding. Those two spans there are deliberate: they quote the fragments that fall out of the mangled example, and they render correctly as code containing a trailing backslash. Removing them would make the explanation wrong. This is the alternative ARCH-021 already records as considered and rejected: "Flag every backslash-backtick byte pair — rejected. It cannot distinguish a mistaken escape from a code span whose content legitimately ends in a backslash, which a Windows path routinely does. A rule that fires on correct prose is a rule contributors learn to suppress." What I did changeThe finding is right that this shape goes unreported, and ARCH-021 did not say so. That silence was a real gap — a limit a later reader would have had to rediscover. #549 adds it to Consequences as a stated boundary:
The last clause is the coverage that matters in practice: the shape from #515 carries two escapes, so its second one lands in text and is caught. Verified — that case still reports, and the seven correct lines above still do not. 🤖 Addressed by Claude Code |
…549) Two documentation-only follow-ups from the review cycle on #548. No source, rule, or behaviour changes. ## 1. Host a new rule on the ADR that already states its invariant The correction on #548 was that a rule does not justify its own ADR. Rules are reachable only through ADRs — `loadRuleAdrs` walks ADRs with `rules: true` and resolves `<baseName>.rules.ts` by filename, so an orphan companion has no discovery path and silently never runs. That coupling makes a new ADR the _price_ of a new rule, not evidence that a new decision exists. Extends the existing "pick the right enforcement layer" memory rather than adding a file, since it is the same question one level deeper: having chosen the ADR-rule layer, which ADR. ## 2. Name the escape-at-close case ARCH-021 does not report A later review asked the markdown check to report an escaped backtick before treating the following backtick as a span delimiter, with `` `show \`literal tick` `` as the regression case. Declined on measurement — implementing it flags **7 lines across 244 markdown files, of which 0 are defects**: six Windows-path rows in the opencode integration guide across three locales, plus ARCH-021's own explanatory prose. The two shapes are byte-identical. In each, the span's last content character is a backslash and the delimiter that follows closes the span: ```text `show \`literal tick` <- the review's example `C:\Users\<username>\.config\` <- opencode-integration.mdx:27 ``` Nothing lexical separates them; only authorial intent does, which the scanner cannot see. ARCH-021 already records this as a considered-and-rejected alternative. But the finding was right that the ADR never _said_ the shape goes unreported — and that silence is why it reads as a bug rather than a boundary. Consequences now states it, along with the mitigation: the multi-escape form a mangled snippet almost always takes is still caught, because its later escapes land in text rather than inside a span. That is the shape the corruption in #515 actually had. ## Verification `bun run validate` exit 0 (2017 tests, 0 fail). `archgate check` 51/51, no briefing-budget warnings — the added text is in Consequences, which is not a briefed section, so ARCH-021's Decision and Do's and Don'ts are unchanged at 1686 and 1554 characters against the 2000 cap. The new bullet quotes its example in a double-backtick span, so it renders as the literal source and does not trip the rule it documents. --------- Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
# archgate ## [0.52.0](v0.51.0...v0.52.0) (2026-08-06) ### ⚠ BREAKING CHANGES * add --strict and --output <format> (SARIF), remove --json/--ci/--max-warnings from check (#536) ### Features * add --strict and --output <format> (SARIF), remove --json/--ci/--max-warnings from check ([#536](#536)) ([70b1ede](70b1ede)) * **dist:** distribute archgate via winget ([#552](#552)) ([93cb3a8](93cb3a8)), references [#544](#544) * **lint:** migrate to TypeScript 7 and adopt oxlint type-aware linting ([#534](#534)) ([29daad8](29daad8)), references [#529](#529) * **plugin:** install Copilot plugin declaratively, covering the desktop app ([#543](#543)) ([ae2b988](ae2b988)) ### Bug Fixes * bound scan loop, narrow install-method types, and extend ARCH-021 to markdown code spans ([#548](#548)) ([3793d80](3793d80)), closes [#541](#541) [#540](#540) [#515](#515), references [#541](#541) [#540](#540) [#515](#515) * **cli:** exit 0 quietly when the output pipe closes (EPIPE) ([#546](#546)) ([e7bfa19](e7bfa19)) * **hooks:** invoke hooks through package scripts instead of a shell ([#557](#557)) ([3b9e411](3b9e411)), references [#442](#442) [#441](#441) [#442](#442) --- This PR was generated with [simple-release](https://github.com/TrigenSoftware/simple-release). <details> <summary>📄 Cheatsheet</summary> <br> You can configure the bot's behavior through a pull request comment using the `!simple-release/set-options` command. ### Command Format ````md !simple-release/set-options ```json { "bump": {}, "publish": {} } ``` ```` ### Useful Parameters #### Bump | Parameter | Type | Description | |-----------|------|-------------| | `version` | `string` | Force set specific version | | `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type | | `prerelease` | `string` | Pre-release identifier (e.g., "alpha", "beta") | | `firstRelease` | `boolean` | Whether this is the first release | | `skip` | `boolean` | Skip version bump | | `byProject` | `Record<string, object>` | Per-project bump options for monorepos | #### Publish | Parameter | Type | Description | |-----------|------|-------------| | `skip` | `boolean` | Skip publishing | | `access` | `'public' \| 'restricted'` | Package access level | | `tag` | `string` | Tag for npm publication | ### Usage Examples #### Force specific version ````md !simple-release/set-options ```json { "bump": { "version": "2.0.0" } } ``` ```` #### Force major bump ````md !simple-release/set-options ```json { "bump": { "as": "major" } } ``` ```` #### Create alpha pre-release ````md !simple-release/set-options ```json { "bump": { "prerelease": "alpha" } } ``` ```` #### Publish with specific access and tag ````md !simple-release/set-options ```json { "bump": { "prerelease": "beta" }, "publish": { "access": "public", "tag": "beta" } } ``` ```` ### Custom Changelog Preamble You can add custom markdown to the top of the changelog (right after the version header) using the `!simple-release/set-preamble` command. The markdown after the command line becomes the preamble. ```md !simple-release/set-preamble ## What's new? - The website was completely redesigned - The new API gives you awesome possibilities ``` In a monorepo, pass the full package name after the command to target a single package's changelog. Wrap the name in backticks so GitHub keeps it as text instead of a mention: ```md !simple-release/set-preamble `@your-org/core` ## Core changes - New plugin system ``` Use one comment per package, plus one without a name for the whole release. ### Access Restrictions The commands can only be used by users with permissions: - repository owner - organization member - collaborator ### Notes - The last comment with `!simple-release/set-options` command takes priority - The last `!simple-release/set-preamble` comment per package takes priority - JSON must be valid, otherwise the `set-options` command will be ignored - Parameters apply only to the current release execution - The commands can be updated by editing the comment or adding a new one </details> <!-- Please do not edit this comment. simple-release-pull-request: true simple-release-branch-from: release simple-release-branch-to: main --> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 participant
Closes #541, closes #540, closes #515.
Each issue was re-verified against current
mainbefore implementing. Two held up as written, one held up in substance but not in its stated mechanism — details below.#541 —
detectInstallMethod's empty-string cache checkVerdict: valid. Every assignment in the function body sets one of four literals and none is
"", so the!== ""half of the cache-hit guard has no falsifying path. The three call sites (telemetry.ts,doctor.ts,sentry.ts) each widen the result into aRecord<string, unknown>, so nothing depended on thestringreturn type.Both the cache and the return type are now the
InstallMethodunion, and the redundant comparison is gone.The issue's third bullet asked whether
typescript/no-unnecessary-conditionalready flags this. It does not — before or after narrowing. Restoring the guard on top of the narrowed type and runningbun run lintpasses clean, while astring !== undefinedcontrol placed in the same function is flagged at once. So type-aware lint would not have caught this, and removing the guard on the strength of a clean lint run would not have been sound reasoning. The type is the enforcement here.InstallMethodis deliberately distinct fromupgrade.ts's same-named local type: that one is a discriminated union carrying the command needed to perform an upgrade, and its fourth member ispackage-managerrather thanglobal-pm. They are not interchangeable and are not unified.#540 — unbounded
for(;;)scan loop infindCodeOccurrencesVerdict: valid, and there is a concrete case the issue did not identify.
The issue's reasoning about the current shape is correct:
idx = found + 1against a fixed-length string bounds the loop. The rewrite moves the advance into theforstatement's increment slot, so no path through the body can skip it and termination is structural rather than a separate argument about the body.While verifying, one input was found that genuinely does not terminate: an empty needle.
String.prototype.indexOf("", n)clamps tosource.lengthrather than returning-1, so the cursor stops advancing and the loop spins forever. Demonstrated on the pre-change shape:This is latent, not reachable today. Every
searchTextproducer inrule-scanner.tsis non-empty by construction: fixed anchors (import(,import.meta,.constructor),from "<specifier>"(minimum seven characters), and identifier names. It is guarded rather than left to chance, since an anchorless violation has no position to map to and the caller already falls back to line 0.An explicit iteration cap was considered and not used — it bounds the symptom, whereas the guard plus the increment-slot advance makes termination provable from the loop's own shape.
Two regression tests added: the empty needle, and a needle overlapping at every offset.
#515 — escaped backticks nested in markdown inline code spans
Verdict: the invariant is valid and there was live corruption in the repository — but the issue's stated mechanism is stale, and its proposed detection algorithm cannot work as written.
Three corrections to the issue:
oxfmtno longer reproduces the corruption. On the pinned 0.60.0,oxfmt --writeover several constructed repros (including the exact shape the issue quotes, reproduced in the fenced block below) is byte-identical in and out.Pinning the rule's justification to formatter behaviour would leave it unenforced on every version where the bug is absent.
The real justification is tool-independent and permanent. CommonMark gives backslash escapes no meaning inside a code span (§6.1), and a span ends at the next backtick run of equal length (§6.3). So the "escaped" backtick still closes the span, the backslash renders as itself, and every later delimiter on the line re-pairs one position out. This is wrong on every renderer, with or without a formatter in the loop.
The detection algorithm proposed in the issue — "flag a
\`sequence occurring between the delimiters" — can never match. The escaped backtick is the closing delimiter; that is precisely the bug. A rule looking strictly between an opener and a closer finds nothing. The rule implemented here inverts it: walk and skip whole code spans, and flag a backslash-before-backtick found in the text between them.That inversion is also what makes it precise. A span whose content legitimately ends in a backslash — a Windows path such as the
.config\opencode\agents\examples in the opencode integration guide — has its backslash inside the span and is never reported.Live corruption found and repaired:
ARCH-010ARCH-007Both are repaired using the CommonMark idiom the rule recommends: a longer delimiter, with symmetric padding when the content itself ends in a backtick.
The rule lives in
ARCH-021, which is generalized rather than duplicated. The escaped-backtick check and the existing ASCII-only PowerShell check enforce one invariant — authored text must not carry a sequence a downstream reader decodes differently from the way it was written — and both are lexical line scans for a corruptionformat:check,oxlint, andtscare all structurally unable to see. ARCH-021 is retitled Authored Text Integrity, widened tofiles: ["**/*.ps1", "**/*.md", "**/*.mdx"], and now carries two rules:ARCH-021/ascii-only-ps1.ps1(unchanged)ARCH-021/no-escaped-backtick-in-markdown.md,.mdx(new)A standalone ADR bought nothing: rules are discovered through ADRs, so a second ADR was the price of a second rule, not of a second decision. Verified — with the ADR removed and only the
.rules.tson disk, the loader drops from 51 rules to 50, the rule never runs, and a real violation passes silently.CHANGELOG.mdis exempt — regenerated from commit messages on every release, matching theignorePatternsentry it already carries in.oxfmtrc.json. That exemption is a stated limitation in the ADR, not a silent carve-out.Verification
Fire-tested in both directions rather than relying on a green suite:
.md(ARCH-010) and a.mdx(opencode guide) each produce exactly one violation, at the column of the backslash to delete..mdxholds four Windows paths ending in a backslash inside code spans and stays clean. A probe using that identical single-escape shape was correctly not reported..ps1still tripsascii-only-ps1, unchanged.node:fsimport to the rule file is rejected by the rule scanner, confirming the boundary is enforced on it.bun run validatepasses from a fresh clone of this branch (exit 0) — the companion.rules.tsreaches its types through the gitignored, generated.archgate/rules.d.ts, so a long-lived working directory would mask a generation-ordering fault that CI would hit immediately.archgate check: 51/51, no briefing-budget warnings. ARCH-021's two briefed sections measure 1686 and 1554 characters against the 2000 cap, so widening the ADR did not push its agent briefing into truncation.No repo ADR companion rule carries a bespoke unit test —
archgate checkover the real corpus duringvalidateis the test — so this follows that convention rather than introducing a new one.