Skip to content

feat(adrs): enforce concise, forward-only code comments (GEN-004) - #496

Merged
rhuanbarreto merged 12 commits into
mainfrom
claude/spicy-booping-muffin
Jul 25, 2026
Merged

feat(adrs): enforce concise, forward-only code comments (GEN-004)#496
rhuanbarreto merged 12 commits into
mainfrom
claude/spicy-booping-muffin

Conversation

@rhuanbarreto

Copy link
Copy Markdown
Contributor

Summary

Adds GEN-004: Concise, Forward-Only Code Comments — comments must describe current behavior only (no history or relocation narration) and stay within 5 prose lines per contiguous run — and brings the whole repo to zero violations in the same change.

Ported from a sibling project's equivalent ADR, adapted to this repo (TypeScript-only scope, archgate-ignore suppression with mandatory reason, dual-layer enforcement).

Enforcement (both layers at error)

Layer Rules Mechanism
archgate check GEN-004/no-narration-in-comments, GEN-004/oversized-comment-blocks companion .rules.ts line heuristics, surfaced to agents via review-context
bun run lint archgate/no-narration-in-comments, archgate/oversized-comment-blocks new .archgate/lint/concise-comments.ts jsPlugin using real comment tokens (sourceCode.getAllComments()) — string literals can never false-positive

Exemptions: tests/** skips the size bound only; tests/fixtures/** is fully exempt. Delimiters, dividers, SPDX headers, and tool directives don't count toward the prose budget.

Backlog fixed to zero

  • 85 oversized comment blocks condensed across src/, lint/, .archgate/, .simple-release.js — deep rationale replaced with pointers to ARCH-019/020/022/023, agent-memory files, or upstream issues
  • 18 narration comments reworded to present tense
  • Displaced rationale re-homed: incident CLI-5 sequence added to ARCH-012 Context; upstream Inquirer.js #2123 added to ARCH-019 References
  • Generated rules.d.ts/scaffold template docs in rules-shim.ts also condensed (facts preserved)

Verification

  • bun run validate passes end-to-end (lint, typecheck, format, 1464 tests, 46/46 ADR rules, knip, build)
  • Fire-tested both layers with a throwaway violating file: correct file:line reports, clean after removal
  • Reviewer pass across all 5 domains: 0 violations, 0 warnings

rhuanbarreto and others added 7 commits July 15, 2026 19:39
`Bun.env.X = undefined` assigns the literal string "undefined" and leaves the
key present — it does not unset. The idiomatic capture-and-restore

    const original = Bun.env.HOME; ...; Bun.env.HOME = original;

therefore leaked HOME="undefined" whenever the variable was unset to begin
with, which is the normal case on Windows for HOME and GIT_CONFIG_GLOBAL.
Bun's test runner shares one process across test files, so the bogus value
escaped into every later test file and into every subprocess they spawned.

The observed symptom was a test that passed in isolation and failed in the
full suite: a leaked HOME="undefined" made `review-context` report zero
changed files. Bun.env and process.env are the same store, so both accessors
are affected.

Add `restoreEnv(key, original)` to tests/test-utils.ts, which deletes the key
when the captured original was undefined, and route every env restore under
tests/ through it. This also collapses the correct-but-verbose
`if (orig === undefined) delete ... else ...` guards that several files had
already grown independently.

Three restores were genuinely unguarded rather than merely verbose:
check-action.test.ts (CI), binary-upgrade.test.ts (HOME) and
init-project.test.ts (HOME).

vscode-settings.test.ts already had a local bulk helper implementing the
correct semantics; it is renamed restoreEnvAll and now delegates to the
shared helper rather than duplicating the logic.

Document the rule as a Do/Don't in ARCH-005.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
ARCH-005 documents that tests must restore environment variables via
restoreEnv(), but the rule was manual-enforcement only — and the preceding
commit showed why that is not enough: several files guarded one variable
correctly with an explicit undefined check while leaving HOME bare, and
vscode-settings.test.ts carried a docstring describing the exact bug that five
other files were leaking. The knowledge was present but inconsistently
applied, which is what a mechanical check catches and review does not.

The invariant is purely syntactic, so it belongs in the linter (the same
reasoning recorded in .archgate/lint/oxlint.ts). Follows lint/expect-expect.ts
as the model: a custom oxlint JS plugin enabled for tests/**/*.test.ts.

The rule flags `Bun.env.NAME = <identifier>` / `process.env.NAME = <identifier>`
only when that identifier was itself captured from an env read earlier in the
same file. Tracking the capture — rather than matching a naming convention
such as `original*` — is what distinguishes a restore from an override: both
are spelled `Bun.env.HOME = <identifier>`, so `Bun.env.HOME = tempDir` is
correctly left alone. This matters in practice: the real call sites named
their variables savedHome, savedXdg, savedDistro, savedInterop, savedAppData
and origCI as often as original*. Computed access (`Bun.env[key]`) is also
left alone, being the shape of the restoreEnv helper itself.

Verified in both directions: dogfooded against the repo (31 sites, 17 files,
no false positives on overrides), and fire-tested by reintroducing a bare
restore, which the rule flagged at the expected line.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Agent harnesses spill an oversized tool result to a file, at which point the
payload stops being readable inline — which defeats the purpose of emitting
JSON for an agent. Three commands crossed that threshold on a project with a
few dozen ADRs. Measured on a 38-ADR project (and this repo for
review-context):

  adr list --json              6314 B ->  3856 B  (-39%)
  check --json                25158 B ->   919 B  (-96%)
  review-context --run-checks 80755 B ->  8759 B  (-90%)

More importantly, each payload now scales with the number of findings rather
than the number of rules or ADRs, so it stops degrading as a project grows.

- adr list: emitted the parsed frontmatter verbatim, including `files` glob
  arrays that are useless for deciding what to read next. Now projects to the
  four fields the human table already renders; `adr show <id>` still has the
  rest.

- check --json: emitted an entry for every rule, including cleanly-passing
  ones whose entry only restates static ADR text (99% of the payload; the
  `description` field alone was 43%). Now carries only rules with something to
  report. The documented example in check.mdx already showed this shape, so
  this aligns the implementation with its own contract. `--verbose` restores
  the full list, matching reportConsole and the flag's documented meaning.

- review-context: each ADR's Decision and Do's/Don'ts prose was 78% of the
  payload. Now opt-in behind `--verbose`; the default identifies which ADRs
  apply and the consumer drills down with `adr show <id>`. checkSummary gets
  the same projection as check --json, via a shared resultsWithFindings()
  helper so the two cannot drift.

The filter predicate is "has nothing to report", not "passed": buildSummary
sets status "fail" only for error-severity violations, so a warning-only rule
is status "pass" with a non-empty violations[]. Filtering on status alone
would silently swallow every warning — a regression test covers this.

Record the convention as ARCH-003 key convention 7, with a Don't guarding
against that exact misimplementation. Docs updated in all three locales.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
…n lesson

- project_test_isolation_gotchas: the existing entry described the env-leak
  symptom and a consumer-side workaround (resetting git vars in the runCli
  call) but never identified the cause — the restore itself. Record that
  `env.X = undefined` assigns the string "undefined", that Bun.env and
  process.env are one store, and point at restoreEnv.
- project_rules_engine_internals: a reviewer sub-agent will misquote the file
  it is reviewing, not just the ADR it cites — it reported a Norwegian
  misspelling that was not in the file, where applying the "fix" would have
  introduced the error. Grep the claimed string before acting on it.
- MEMORY.md: index entries updated to match.

Also includes an in-flight edit to feedback_prefer_tests_over_adr_rules.md
(ADR rules that only assert implementation shape) authored outside this
change but present in the working tree.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…timeouts

reporter.test.ts exercises reportJSON directly, so nothing caught check.ts
passing the wrong verbose value through to it. The --adr filter test covers
verbose=true (it asserts per-rule entries are present), but a hardcoded true
would have shipped undetected. Add an end-to-end test asserting the default
omits a cleanly-passing rule while the counts still report it, and that
--verbose restores the entry. Fire-tested: hardcoding verbose=true in check.ts
fails exactly this test.

Also drop the `, 60000` per-test overrides from the two new review-context
tests. ARCH-005 permits an override solely to grant a genuinely slow test MORE
time than the global `--timeout 60000`; an override equal to the global grants
nothing.

Both raised in CodeRabbit review of #476.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Add GEN-004 mandating comments describe current behavior only (no
history/relocation narration) within 5 prose lines per contiguous run,
enforced at error severity by two complementary layers:

- archgate layer: companion GEN-004 .rules.ts (line heuristics) surfaced
  by archgate check / review-context
- oxlint layer: archgate/no-narration-in-comments and
  archgate/oversized-comment-blocks in .archgate/lint/concise-comments.ts
  using real comment tokens via sourceCode.getAllComments()

tests/** is exempt from the size bound (not from forward-only content);
tests/fixtures/** is fully exempt.

Fix the entire 103-violation backlog in the same change: 85 oversized
blocks condensed (deep rationale now points at ARCH-019/020/022/023,
agent-memory files, or upstream issues) and 18 narration comments
reworded to present tense. Capture displaced rationale in ARCH-012
(incident CLI-5 sequence) and ARCH-019 (upstream Inquirer.js #2123).

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Too many files!

This PR contains 141 files, which is 41 over the limit of 100.

To get a review, narrow the scope:
• coderabbit review --committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 14acb694-4a5f-4012-b00a-84675e78aec6

📥 Commits

Reviewing files that changed from the base of the PR and between 1c8a5c6 and 73a4bde.

📒 Files selected for processing (141)
  • .archgate/adrs/ARCH-005-testing-standards.rules.ts
  • .archgate/adrs/ARCH-007-cross-platform-subprocess-execution.rules.ts
  • .archgate/adrs/ARCH-008-typed-command-options.rules.ts
  • .archgate/adrs/ARCH-012-command-error-boundaries.rules.ts
  • .archgate/adrs/ARCH-015-cli-command-documentation-coverage.rules.ts
  • .archgate/adrs/ARCH-016-cli-subcommand-documentation-accuracy.rules.ts
  • .archgate/adrs/ARCH-022-ast-aware-rule-context.md
  • .archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts
  • .archgate/adrs/CI-001-pin-github-actions-by-hash.rules.ts
  • .archgate/adrs/GEN-002-docs-i18n.rules.ts
  • .archgate/adrs/GEN-003-tool-invocation-via-scripts.rules.ts
  • .archgate/adrs/GEN-004-concise-forward-only-code-comments.md
  • .archgate/adrs/GEN-004-concise-forward-only-code-comments.rules.ts
  • .archgate/lint/concise-comments.ts
  • .archgate/lint/oxlint.ts
  • .claude/agent-memory/archgate-developer/MEMORY.md
  • .claude/agent-memory/archgate-developer/feedback_reply_on_review_threads.md
  • .claude/agent-memory/archgate-developer/project_cli_perf_baselines.md
  • .oxlintrc.json
  • lint/no-bare-env-restore.ts
  • scripts/add-spdx-headers.ts
  • src/cli.ts
  • src/commands/adr/create.ts
  • src/commands/adr/import.ts
  • src/commands/adr/list.ts
  • src/commands/adr/sync.ts
  • src/commands/check.ts
  • src/commands/init.ts
  • src/commands/login.ts
  • src/commands/plugin/install.ts
  • src/commands/upgrade.ts
  • src/engine/ast-support.ts
  • src/engine/git-files.ts
  • src/engine/glob-utils.ts
  • src/engine/js-parser.ts
  • src/engine/loader.ts
  • src/engine/reporter.ts
  • src/engine/rule-scanner.ts
  • src/engine/runner.ts
  • src/engine/safe-path.ts
  • src/engine/source-positions.ts
  • src/engine/suppressions.ts
  • src/formats/rules.ts
  • src/helpers/adr-import.ts
  • src/helpers/adr-writer.ts
  • src/helpers/auth.ts
  • src/helpers/binary-upgrade.ts
  • src/helpers/claude-settings.ts
  • src/helpers/copilot-settings.ts
  • src/helpers/credential-store.ts
  • src/helpers/cursor-settings.ts
  • src/helpers/doctor.ts
  • src/helpers/editor-detect.ts
  • src/helpers/exit.ts
  • src/helpers/git.ts
  • src/helpers/init-project.ts
  • src/helpers/install-info.ts
  • src/helpers/login-flow.ts
  • src/helpers/opencode-settings.ts
  • src/helpers/output.ts
  • src/helpers/pack-recommend.ts
  • src/helpers/paths.ts
  • src/helpers/platform.ts
  • src/helpers/plugin-install.ts
  • src/helpers/project-config.ts
  • src/helpers/prompt.ts
  • src/helpers/registry.ts
  • src/helpers/repo.ts
  • src/helpers/rules-shim.ts
  • src/helpers/sentry.ts
  • src/helpers/session-context-copilot.ts
  • src/helpers/session-context-opencode.ts
  • src/helpers/session-context.ts
  • src/helpers/signup.ts
  • src/helpers/stack-detect.ts
  • src/helpers/telemetry-config.ts
  • src/helpers/telemetry.ts
  • src/helpers/update-check.ts
  • src/helpers/user-error.ts
  • src/helpers/vscode-settings.ts
  • tests/commands/adr.test.ts
  • tests/commands/adr/create.test.ts
  • tests/commands/adr/import.test.ts
  • tests/commands/adr/sync.test.ts
  • tests/commands/check-security.test.ts
  • tests/commands/check.test.ts
  • tests/commands/clean.test.ts
  • tests/commands/doctor.test.ts
  • tests/commands/init.test.ts
  • tests/commands/login.test.ts
  • tests/commands/plugin/url.test.ts
  • tests/commands/review-context.test.ts
  • tests/commands/session-context/claude-code.test.ts
  • tests/commands/session-context/copilot.test.ts
  • tests/commands/session-context/cursor.test.ts
  • tests/commands/session-context/opencode.test.ts
  • tests/commands/telemetry.test.ts
  • tests/commands/upgrade.test.ts
  • tests/engine/context.test.ts
  • tests/engine/git-files.test.ts
  • tests/engine/glob-utils.test.ts
  • tests/engine/loader.test.ts
  • tests/engine/reporter.test.ts
  • tests/engine/rule-scanner-escapes.test.ts
  • tests/engine/rule-scanner.test.ts
  • tests/formats/rules.test.ts
  • tests/helpers/auth.test.ts
  • tests/helpers/binary-upgrade.test.ts
  • tests/helpers/claude-settings.test.ts
  • tests/helpers/credential-store.test.ts
  • tests/helpers/cursor-settings.test.ts
  • tests/helpers/doctor.test.ts
  • tests/helpers/editor-detect.test.ts
  • tests/helpers/init-base-branch.test.ts
  • tests/helpers/init-project.test.ts
  • tests/helpers/login-flow.test.ts
  • tests/helpers/opencode-settings.test.ts
  • tests/helpers/pack-recommend.test.ts
  • tests/helpers/paths.test.ts
  • tests/helpers/platform.test.ts
  • tests/helpers/plugin-install-cleanup.test.ts
  • tests/helpers/plugin-install.test.ts
  • tests/helpers/project-config.test.ts
  • tests/helpers/rules-shim.test.ts
  • tests/helpers/session-context-copilot.test.ts
  • tests/helpers/session-context-cursor.test.ts
  • tests/helpers/session-context-opencode.test.ts
  • tests/helpers/session-context.test.ts
  • tests/helpers/stack-detect-frameworks.test.ts
  • tests/helpers/telemetry-config.test.ts
  • tests/helpers/telemetry.test.ts
  • tests/helpers/update-check.test.ts
  • tests/helpers/vscode-settings.test.ts
  • tests/integration/adr.test.ts
  • tests/integration/check.test.ts
  • tests/integration/clean.test.ts
  • tests/integration/cli-harness.ts
  • tests/integration/cli-perf.test.ts
  • tests/integration/import.test.ts
  • tests/integration/review-context.test.ts
  • tests/test-utils.ts

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

The changes add lean identity-only ADR list JSON, reportable-result filtering with verbose expansion, and optional ADR prose in review-context. They introduce safe environment restoration and lint enforcement for bare restores. A new GEN-004 ADR and two comment-content rules enforce concise, forward-only comments. Documentation, ADR guidance, tests, and implementation comments are updated accordingly.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR does not address linked issue #39, which is about the 0.8.1 release and CI publishing fixes. Either relink the PR to the correct issue for GEN-004 or change the code changes to satisfy the release/CI objectives in #39.
Out of Scope Changes check ⚠️ Warning Most changes implement GEN-004 comment enforcement, which is unrelated to the linked 0.8.1 release issue. Remove the unrelated GEN-004 changes from this PR or attach the correct issue that describes them.
Docstring Coverage ⚠️ Warning Docstring coverage is 78.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding GEN-004 enforcement for concise, forward-only code comments.
Description check ✅ Passed The description matches the changeset well and correctly describes the new comment rules, linting, exemptions, and fixes.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 24, 2026

Copy link
Copy Markdown

Deploying archgate-cli with  Cloudflare Pages  Cloudflare Pages

Latest commit: 73a4bde
Status: ✅  Deploy successful!
Preview URL: https://bdd8f525.archgate-cli.pages.dev
Branch Preview URL: https://claude-spicy-booping-muffin.archgate-cli.pages.dev

View logs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

🤖 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-012-command-error-boundaries.rules.ts:
- Around line 4-8: Update the ARCH-012 rule-source comment to remove the
historical “incident CLI-5” reference and prior failure narration, while
retaining only a concise description of the current ESTree check for top-level
awaited statements outside the action’s try block.

In @.archgate/adrs/GEN-004-concise-forward-only-code-comments.rules.ts:
- Around line 22-27: Update looksLikeComment() so generator method declarations
such as *method() are not classified as comment prose, while genuine
block-comment lines remain recognized; use block-comment state or token-aware
scanning as appropriate, and add a regression covering generator methods to
prevent oversized-block and narration checks from processing them.

In @.archgate/lint/concise-comments.ts:
- Around line 63-72: Update the pattern-checking loop in the concise-comments
lint rule to emit at most one diagnostic for each comment. Stop evaluating
patterns after the first match and report, while preserving the existing
location and message behavior.
- Around line 52-56: Update isWholeLine to require both the text before
comment.loc.start.column and the text after comment.loc.end.column on the same
line to contain only whitespace. Preserve the existing handling for missing line
text, and only classify comments as whole-line when neither surrounding segment
contains code.

In @.claude/agent-memory/archgate-developer/project_test_isolation_gotchas.md:
- Line 10: Rewrite the guidance in project_test_isolation_gotchas.md to remove
the dated incident and retrospective symptom narration. Preserve the restoreEnv
rule, the explanation that assigning undefined leaves a literal "undefined"
environment value, and the Bun.env/process.env shared-store behavior; express
the leak behavior and the grep-based detection pattern as concise present-tense,
forward-only guidance.

In `@lint/no-bare-env-restore.ts`:
- Around line 82-107: Update collectCapturedNames to retain each captured
environment key together with its lexical binding, rather than storing only
identifier names. Resolve the RHS binding for captured declarations and
assignments, then require restore assignments to reference the same binding and
the same env key. Preserve shadowed-binding behavior so unrelated variables such
as originalHome are not reported as restores.

In `@src/cli.ts`:
- Around line 83-86: Update the startup comment near the concurrent telemetry
initialization to describe only the current behavior: acknowledge that
installGit() is awaited before command parsing, so --help and --version still
incur the git cost, and remove the PR `#211` historical reference.

In `@src/engine/loader.ts`:
- Around line 97-98: Update the documentation for checkRuleSyntax() to describe
only its actual validation of a satisfies RuleSet occurrence, unless the
implementation is changed to verify that occurrence is attached to export
default. Do not claim that the syntax check validates export placement.

In `@src/engine/runner.ts`:
- Around line 92-96: Update the cache documentation near RunCaches.globResults
to describe cached glob results as copy-protected rather than immutable, noting
that glob() copies arrays before exposing them. Keep the existing explanation of
shared promise-based caching and readJSON’s exclusion unchanged.

In `@src/engine/suppressions.ts`:
- Around line 114-117: Update the documentation comment for the suppression
logic to state that missing-reason warnings are emitted only for suppressions
that match a violation, while unused suppressions without reasons are skipped.
Keep the existing descriptions of inline and file-level suppression behavior
unchanged.

In `@src/helpers/project-config.ts`:
- Around line 244-247: Update the JSDoc for the project path resolver near its
configuration description to state that an unset paths.rules uses the default
lint directory via defaults.lintDir, rather than co-locating rules with ADRs.
Keep the documented defaults for configurations with no paths unchanged.

In `@src/helpers/user-error.ts`:
- Around line 4-8: Update the documentation in user-error.ts to state that
unexpected errors are captured by handleCommandError except ExitPromptError
cancellation errors, which are rethrown without Sentry capture. Keep the
existing UserError and handleCommandError references intact.

In `@tests/commands/adr/list.test.ts`:
- Around line 151-154: Replace the fixture write using writeFileSync in
tests/commands/adr/list.test.ts:151-154 with awaited Bun.write, and update the
test flow as needed to support the await. Replace both fixture writes in
tests/integration/review-context.test.ts:171 and 204 with awaited Bun.write; no
other file I/O behavior needs to change.

In `@tests/helpers/auth.test.ts`:
- Around line 38-47: Update the cleanup comments describing restoreEnv in
tests/helpers/auth.test.ts lines 38-47 and
tests/helpers/credential-store.test.ts lines 35-41 to state only its current
delete-or-restore behavior: delete environment variables that were originally
absent and restore the original value otherwise. Remove descriptions of previous
leakage or failure; the restoreEnv calls themselves require no change.

In `@tests/helpers/telemetry-config.test.ts`:
- Around line 24-26: The comments in tests/helpers/telemetry-config.test.ts
lines 24-26 and tests/helpers/telemetry.test.ts lines 29-31 must use
forward-only wording describing the current mechanism: direct environment
assignment stringifies undefined, while restoreEnv() deletes keys that should be
unset. Rewrite both comments accordingly without referring to a prior leak.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d62e0689-3287-44f5-a74f-3c10d9e80ac2

📥 Commits

Reviewing files that changed from the base of the PR and between 0015542 and 1c8a5c6.

📒 Files selected for processing (99)
  • .archgate/adrs/ARCH-003-output-formatting.md
  • .archgate/adrs/ARCH-004-no-barrel-files.rules.ts
  • .archgate/adrs/ARCH-005-testing-standards.md
  • .archgate/adrs/ARCH-008-typed-command-options.rules.ts
  • .archgate/adrs/ARCH-012-command-error-boundaries.md
  • .archgate/adrs/ARCH-012-command-error-boundaries.rules.ts
  • .archgate/adrs/ARCH-015-cli-command-documentation-coverage.rules.ts
  • .archgate/adrs/ARCH-016-cli-subcommand-documentation-accuracy.rules.ts
  • .archgate/adrs/ARCH-019-inquirer-prompt-fix.md
  • .archgate/adrs/ARCH-022-ast-aware-rule-context.rules.ts
  • .archgate/adrs/GEN-004-concise-forward-only-code-comments.md
  • .archgate/adrs/GEN-004-concise-forward-only-code-comments.rules.ts
  • .archgate/lint/concise-comments.ts
  • .archgate/lint/oxlint.ts
  • .claude/agent-memory/archgate-developer/MEMORY.md
  • .claude/agent-memory/archgate-developer/feedback_concise_comments.md
  • .claude/agent-memory/archgate-developer/feedback_prefer_tests_over_adr_rules.md
  • .claude/agent-memory/archgate-developer/project_oxlint_gotchas.md
  • .claude/agent-memory/archgate-developer/project_rules_engine_internals.md
  • .claude/agent-memory/archgate-developer/project_test_isolation_gotchas.md
  • .oxlintrc.json
  • .simple-release.js
  • docs/public/llms-full.txt
  • docs/src/content/docs/nb/reference/cli/check.mdx
  • docs/src/content/docs/nb/reference/cli/review-context.mdx
  • docs/src/content/docs/pt-br/reference/cli/check.mdx
  • docs/src/content/docs/pt-br/reference/cli/review-context.mdx
  • docs/src/content/docs/reference/cli/check.mdx
  • docs/src/content/docs/reference/cli/review-context.mdx
  • lint/expect-expect.ts
  • lint/no-bare-env-restore.ts
  • src/cli.ts
  • src/commands/adr/import.ts
  • src/commands/adr/list.ts
  • src/commands/check.ts
  • src/commands/init.ts
  • src/commands/review-context.ts
  • src/engine/ast-support.ts
  • src/engine/context.ts
  • src/engine/git-files.ts
  • src/engine/glob-utils.ts
  • src/engine/js-parser.ts
  • src/engine/loader.ts
  • src/engine/reporter.ts
  • src/engine/rule-scanner.ts
  • src/engine/runner.ts
  • src/engine/source-positions.ts
  • src/engine/suppressions.ts
  • src/formats/rules.ts
  • src/helpers/binary-upgrade.ts
  • src/helpers/credential-store.ts
  • src/helpers/cursor-settings.ts
  • src/helpers/exit.ts
  • src/helpers/init-project.ts
  • src/helpers/install-info.ts
  • src/helpers/opencode-settings.ts
  • src/helpers/paths.ts
  • src/helpers/plugin-install.ts
  • src/helpers/project-config.ts
  • src/helpers/prompt.ts
  • src/helpers/registry.ts
  • src/helpers/repo-probe.ts
  • src/helpers/repo.ts
  • src/helpers/rules-shim.ts
  • src/helpers/sentry.ts
  • src/helpers/session-context-copilot.ts
  • src/helpers/session-context-opencode.ts
  • src/helpers/session-context.ts
  • src/helpers/telemetry-config.ts
  • src/helpers/telemetry.ts
  • src/helpers/user-error.ts
  • tests/commands/adr/list.test.ts
  • tests/commands/check-action.test.ts
  • tests/commands/clean.test.ts
  • tests/commands/review-context.test.ts
  • tests/commands/upgrade.test.ts
  • tests/engine/context.test.ts
  • tests/engine/reporter.test.ts
  • tests/helpers/auth.test.ts
  • tests/helpers/binary-upgrade.test.ts
  • tests/helpers/credential-store.test.ts
  • tests/helpers/exit.test.ts
  • tests/helpers/init-project.test.ts
  • tests/helpers/install-info.test.ts
  • tests/helpers/opencode-settings.test.ts
  • tests/helpers/output.test.ts
  • tests/helpers/platform.test.ts
  • tests/helpers/plugin-install-cleanup.test.ts
  • tests/helpers/plugin-install.test.ts
  • tests/helpers/sentry.test.ts
  • tests/helpers/session-context-copilot.test.ts
  • tests/helpers/session-context-opencode.test.ts
  • tests/helpers/telemetry-config.test.ts
  • tests/helpers/telemetry.test.ts
  • tests/helpers/update-check.test.ts
  • tests/helpers/vscode-settings.test.ts
  • tests/integration/check.test.ts
  • tests/integration/review-context.test.ts
  • tests/test-utils.ts

Comment thread .archgate/adrs/ARCH-012-command-error-boundaries.rules.ts Outdated
Comment thread .archgate/adrs/GEN-004-concise-forward-only-code-comments.rules.ts
Comment thread .archgate/lint/concise-comments.ts Outdated
Comment thread .archgate/lint/concise-comments.ts
Comment thread src/helpers/project-config.ts Outdated
Comment thread src/helpers/user-error.ts Outdated
Comment thread tests/commands/adr/list.test.ts
Comment thread tests/helpers/auth.test.ts
Comment thread tests/helpers/telemetry-config.test.ts Outdated
Resolve conflicts from main's engine work (ctx.ast() comments/base-rev,
findAstNodes, parse caching, rule-file sandbox hardening, lean payloads):

- take main's code for all conflicted engine/test files
- keep GEN-004 wiring in .oxlintrc.json
- re-apply the GEN-004 comment sweep to main's new code (47 reintroduced
  violations trimmed/reworded; security rationale compressed to pointers
  at ARCH-022/ARCH-024, with the tmpdir symlink-attack walkthrough
  re-homed into ARCH-022's context)

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Metric Value
Lines 91.3% (8165 / 8942)
Threshold 90% minimum — met
Platforms Linux + Windows

Full HTML report available in workflow artifacts.

Per-directory breakdown
Directory Coverage Lines
src/commands/ 89.0% 1959 / 2200
src/engine/ 93.9% 2048 / 2181
src/formats/ 98.7% 148 / 150
src/helpers/ 90.9% 4010 / 4411

Rule fixes (both enforcement layers kept in sync):
- looksLikeComment() no longer treats a generator declaration (`*method()`)
  as a JSDoc continuation line, which inflated comment runs and exposed
  generator names to the narration patterns
- isWholeLine() also requires whitespace after the comment, so
  `/* c */ const x = 1` stays out of block runs
- report one diagnostic per comment when both patterns match

Comment accuracy — three inaccuracies predate this branch and were
inherited by the condensed versions:
- loader.ts: the syntax check is a presence check, not a placement check
- user-error.ts: ExitPromptError is rethrown before Sentry capture
- project-config.ts: unset `paths.rules` falls back to `.archgate/lint/`,
  it does not co-locate with ADRs
- suppressions.ts: a reason-less suppression warns only once scope-matched
- cli.ts: name what --help/--version actually defer
- ARCH-012 rules: drop incident narration, cite the ADR
- credential-store/telemetry-config tests: forward-only wording

GEN-004 scope: exempt `.claude/agent-memory/**` from the forward-only
requirement — a memory entry's Why line is deliberately a past-tense
incident account, which is what makes 'move rationale to a memory file'
a real remedy.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@rhuanbarreto

Copy link
Copy Markdown
Contributor Author

Review feedback addressed (b187edc)

Thanks — the two functional-correctness findings on the new rules were real bugs, and three of the accuracy findings turned out to predate this branch.

Rule bugs fixed (both layers kept in sync)

  • Generator declarations counted as comment proselooksLikeComment() matched any line starting with *, so *method() joined comment runs and exposed generator names to the narration patterns. Now a leading * counts only when followed by whitespace, /, or end of line. Fire-tested: six consecutive generator methods (one named *previouslyNamed()) produce zero violations on both layers, while a six-line JSDoc block is still flagged on both.
  • Text after a block commentisWholeLine() now requires whitespace after loc.end.column too, so /* c */ const x = 1 stays out of block runs.
  • Duplicate diagnostics — a comment matching both the narration and relocation patterns now reports once.

Comment accuracy

Three of these were wrong before this branch and the condensed versions inherited the error — good catches:

  • project-config.ts: unset paths.rules falls back to .archgate/lint/; it does not co-locate with ADRs.
  • user-error.ts: ExitPromptError is rethrown at exit.ts:106 before Sentry capture, so the blanket "anything not a UserError" claim was false.
  • loader.ts: checkRuleSyntax() is a presence check over source text, not a placement check.
  • suppressions.ts: a reason-less suppression warns only once scope-matched — one that never matches is skipped by the unused-detection loop and warns nowhere.
  • cli.ts: reworded to name what --help/--version actually defer (SDK parse + repo_id resolution). installGit() is indeed awaited unconditionally.
  • ARCH-012 rules file: incident narration dropped, ADR cited instead. This one is a true GEN-004 violation that both layers miss — the phrasing matches no banned pattern, exactly the false-negative class the ADR documents under Negative Consequences.
  • credential-store.test.ts, telemetry-config.test.ts: reworded forward-only.

Declined, with reasons

  • Forward-only prose in .claude/agent-memory/** — declining, and I amended GEN-004's Scope to say so explicitly. A memory entry's **Why:** line is deliberately a past-tense incident account; that account is what lets a future agent judge edge cases instead of applying a rule blindly. Banning it there would also make "move deep rationale to an ADR or memory file" — this ADR's own prescribed remedy — a redirect to a second place the rationale is forbidden.
  • lint/no-bare-env-restore.ts false positives (captured stores only identifier names, so cross-key restores and shadowed bindings misreport) — real, but that rule shipped in Feature: opt-in shared helper modules for rule files, contained within .archgate/ #490 and is untouched here; this PR only trims its header comment. Worth a follow-up issue rather than widening a comment-only PR.
  • Bun.write in test fixtures — same reasoning: node:fs calls in tests/commands/adr/list.test.ts and tests/integration/review-context.test.ts come from main, not this branch.

bun run validate passes: lint, typecheck, format, 1597 tests, 46/46 ADR rules, knip, build.

…ests

Also record the review-thread reply convention in agent memory.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
…vious comments

GEN-004 was measuring the wrong thing: a flat prose bound made TSDoc tags
compete with the summary for budget, so an earlier sweep dissolved 8 @PARAM
tags and an @example into prose. Three changes fix that.

1. Structured documentation is exempt. A structural TSDoc tag (@PARAM,
   @returns, @throws, @example, @see, @typeparam, ...) opens a section that
   does not count toward the 5-line bound — its length tracks the API
   surface, not narrative. Prose containers (@remarks, @description,
   @notes, @todo, @fixme) still count, so relabelling narrative cannot buy
   budget. Implemented identically in both layers.

2. No directory-level carve-outs. The tests/** size exemption and the
   tests/fixtures/** full exemption are gone from .oxlintrc.json and the
   companion rules file; the ADR now records that any future exemption
   belongs in the ADR, not in lint config. Removing them surfaced only 10
   blocks and zero narration, because the tag exemption covers the reason
   test comments legitimately grow.

3. Obvious comments removed. A 12-agent sweep over 238 files deleted 248
   comments that restate the code they sit above, keeping every comment
   carrying a why, an invariant, a footgun, a security property, or a
   pointer. Verified by diffing ADR references, issue links, and TSDoc tags
   per file: no net loss anywhere except duplicate copies of one upstream
   bug link.

Also restores the tags the earlier sweep dissolved, syncs rules-shim.ts
(the user-facing rules.d.ts) with formats/rules.ts, and rescues the CLI
perf baselines into agent memory.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
#499 extracted safePath/isWithinRoot/resolveUserPath from runner.ts into
src/engine/safe-path.ts and hardened them against symlinked ancestor
directories; take main's side for that move.

The new file's 11-line symlink rationale exceeded GEN-004's bound. ARCH-024
explicitly scopes itself out of ctx.readFile/ctx.glob path sandboxing, so the
rationale moves to ARCH-022 clause 1, which already depends on safePath's
'no symlink escapes' guarantee but did not say what it covers. The comment
keeps the invariant plus @throws and a pointer.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@rhuanbarreto
rhuanbarreto merged commit 9a114b3 into main Jul 25, 2026
22 checks passed
@rhuanbarreto
rhuanbarreto deleted the claude/spicy-booping-muffin branch July 25, 2026 10:13
@archgatebot archgatebot Bot mentioned this pull request Jul 25, 2026
rhuanbarreto added a commit that referenced this pull request Jul 25, 2026
GEN-004 landed in #496 while this branch was open, capping contiguous
comment runs at 5 lines of prose. Ten blocks added here exceeded it.

Each is trimmed to current-behavior essentials, with the rationale moved
behind @see pointers to the ADR, the tests, or the type that carries the
contract — the remedy GEN-004's own fix advice recommends. The rules.ts and
rules-shim.ts JSDoc stay mirrored so ARCH-022's manual parity item holds.

No behavior change; llms-full.txt regenerated for the trimmed prose.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
rhuanbarreto pushed a commit that referenced this pull request Jul 26, 2026
# archgate

## [0.51.0](v0.50.0...v0.51.0)
(2026-07-26)

### Features

* **adrs:** enforce concise, forward-only code comments (GEN-004)
([#496](#496))
([9a114b3](9a114b3)),
references [#2123](https://github.com/archgate/cli/issues/2123)
* **adrs:** flag stray files at the repository root (GEN-005)
([#535](#535))
([6a6e765](6a6e765)),
closes [#514](#514), references
[#500](#500)
* **engine:** add ctx.readYAML and ctx.checkCase rule helpers
([#497](#497))
([c5d82c5](c5d82c5)),
closes [#490](#490), references
[#490](#490)
[#491](#491)
[#499](#499)
[#499](#499)
[#499](#499)
[#499](#499)
* report truncated ADR briefings, trim the ADR corpus 15.6%, add GEN-005
briefing budget ([#501](#501))
([a9dab40](a9dab40))

### Bug Fixes

* **docs:** relocate ADR content to clear briefing-budget warnings
([#531](#531))
([c7419b3](c7419b3))
* **docs:** restore pt-br diacritics and enforce locale content
integrity ([#523](#523))
([db39104](db39104)),
closes [#516](#516), references
[#231](#231)
* **engine:** allow symlinks that resolve inside the project root
([#500](#500))
([387bf15](387bf15))
* **engine:** reject rule-file reads through a symlinked ancestor
directory ([#499](#499))
([a555f9d](a555f9d)),
references [#497](#497)
[#491](#491)
[#497](#497)
[#497](#497)
[#497](#497)
[#497](#497)
* **engine:** scan top-level export declarations with a null source
([#493](#493))
([d07db03](d07db03)),
closes [#491](#491)
* **engine:** stop dropping AST nodes with exotic literal values
([#494](#494))
([0015542](0015542)),
closes [#493](#493)
[#493](#493)
* **lint:** resolve no-bare-env-restore by captured key and lexical
scope ([#524](#524))
([7094a3a](7094a3a)),
closes [#498](#498)
* **rules:** make ARCH-020 and ARCH-023 match ctx.ast() instead of raw
text ([#533](#533))
([ad5529b](ad5529b)),
closes [#513](#513), references
[#486](#486)
* **tests:** replace bun:test anti-patterns with idiomatic patterns
([#512](#512))
([bcb086f](bcb086f))

---
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant