Skip to content

fix(gates): devkit biome gate must fail on warnings, not just errors - #371

Merged
norvalbv merged 1 commit into
mainfrom
fix/biome-gate-error-on-warnings
Aug 7, 2026
Merged

fix(gates): devkit biome gate must fail on warnings, not just errors#371
norvalbv merged 1 commit into
mainfrom
fix/biome-gate-error-on-warnings

Conversation

@norvalbv

@norvalbv norvalbv commented Aug 7, 2026

Copy link
Copy Markdown
Owner

What

devkit ship printed six biome findings into the gate log and passed the commit anyway (the v0.50.0 ship). The deterministic runner was working correctly — biome check . exits 0 when every diagnostic is warn-severity, and --extra "lint=bun run lint" is only ever as hard as the script it names.

devkit’s own lint script is now biome check --error-on-warnings ., and the six pre-existing warnings are cleared so the gate starts green.

The six that were passing

file rule
cli/__tests__/husky-block.test.mts:418,440 noTemplateCurlyInString
cli/__tests__/self-host.test.mts:147 noTemplateCurlyInString
gate-engine/coverage/__tests__/produce.test.mts:123 noNonNullAssertion
gate-engine/decisions/__tests__/recall-scoring.test.mts:255 noNonNullAssertion
skills/correctness/scripts/checklist.mjs:65 useOptionalChain

The three noTemplateCurlyInString sites assert literal POSIX-sh ${VAR:-default} text — suppressed with the same biome-ignore line this repo already uses at six other sites. The two non-null assertions are removed rather than suppressed: one becomes a real precondition check, the other hoists the fixture literal so no assertion is needed. The optional chain is the rule’s own fix.

Scope: devkit’s own gate only

The consumer emitter (cli/lib/install/package-json.mts) and the overlay biome gate (cli/lib/husky/husky-block.mts) keep a bare biome check on purpose. biome/base.jsonc holds noConsole at "warn" deliberately for consumers, while devkit’s root config turns it off across its whole authored surface — so the flag costs devkit nothing and would cost a consumer every console.log. Making consumers strict is a separate decision.

Regression test

cli/__tests__/self-host.test.mts now asserts the repo’s real package.json lint script carries --error-on-warnings, because the strictness lives in a script string that no hook-text assertion can reach.

Also in the diff

  • docs/decisions/gate-opt-out-is-visible-and-detectable.md — note recording the reverse failure mode: not a gate that silently opts out, but one that prints its findings and exits 0.
  • .devkit/skills-manifest.jsondevkit sync-skills refreshed the correctness-checklist hash and bumped a stale devkitRef v0.49.1 → v0.50.0.

Deliberately NOT in the diff

cli/lib/husky/husky-block.mts:35 still documents the gate as biome check ., which is now stale by one line. Correcting that comment was in the first attempt and the ship blocked: the file is 560 lines against a 510-line size baseline on origin/main (eslint/baselines/size-lines.json:7), so guard-size rejects any commit that touches it. That debt is pre-existing — it arrived via #360, #348 and #286 without a baseline refresh — and paying it (a file split, or a guard-size freeze that launders 50 lines) does not belong in this PR. The load-bearing documentation of the command lives in cli/lib/husky/self-host.mts, next to SELF_HOST_EXTRAS, and that one is updated.

Verification

biome check --error-on-warnings . → 0 · tsc -p tsconfig.json --noEmit → 0 · eslint cli gate-engine → 0 · vitest run → 3681 passed / 1 failed, the failure being cli/__tests__/review.test.mts “preserves timeout exit 124” returning 143 (SIGTERM) — a wall-clock-bounded test losing its 1s race under a loaded machine, unrelated to this diff.

## What

`devkit ship` printed six biome findings into the gate log and passed the commit anyway (the v0.50.0 ship). The deterministic runner was working correctly — `biome check .` exits **0** when every diagnostic is warn-severity, and `--extra "lint=bun run lint"` is only ever as hard as the script it names.

devkit’s own `lint` script is now `biome check --error-on-warnings .`, and the six pre-existing warnings are cleared so the gate starts green.

## The six that were passing

| file | rule |
| --- | --- |
| `cli/__tests__/husky-block.test.mts:418,440` | `noTemplateCurlyInString` |
| `cli/__tests__/self-host.test.mts:147` | `noTemplateCurlyInString` |
| `gate-engine/coverage/__tests__/produce.test.mts:123` | `noNonNullAssertion` |
| `gate-engine/decisions/__tests__/recall-scoring.test.mts:255` | `noNonNullAssertion` |
| `skills/correctness/scripts/checklist.mjs:65` | `useOptionalChain` |

The three `noTemplateCurlyInString` sites assert literal POSIX-sh `${VAR:-default}` text — suppressed with the same `biome-ignore` line this repo already uses at six other sites. The two non-null assertions are **removed** rather than suppressed: one becomes a real precondition check, the other hoists the fixture literal so no assertion is needed. The optional chain is the rule’s own fix.

## Scope: devkit’s own gate only

The consumer emitter (`cli/lib/install/package-json.mts`) and the overlay biome gate (`cli/lib/husky/husky-block.mts`) keep a bare `biome check` on purpose. `biome/base.jsonc` holds `noConsole` at `"warn"` deliberately for consumers, while devkit’s root config turns it off across its whole authored surface — so the flag costs devkit nothing and would cost a consumer every `console.log`. Making consumers strict is a separate decision.

## Regression test

`cli/__tests__/self-host.test.mts` now asserts the repo’s real `package.json` lint script carries `--error-on-warnings`, because the strictness lives in a script string that no hook-text assertion can reach.

## Also in the diff

- `docs/decisions/gate-opt-out-is-visible-and-detectable.md` — note recording the reverse failure mode: not a gate that silently opts out, but one that prints its findings and exits 0.
- `.devkit/skills-manifest.json` — `devkit sync-skills` refreshed the correctness-checklist hash and bumped a stale `devkitRef` v0.49.1 → v0.50.0.

## Deliberately NOT in the diff

`cli/lib/husky/husky-block.mts:35` still documents the gate as `biome check .`, which is now stale by one line. Correcting that comment was in the first attempt and the ship blocked: the file is **560 lines against a 510-line size baseline on `origin/main`** (`eslint/baselines/size-lines.json:7`), so `guard-size` rejects any commit that touches it. That debt is pre-existing — it arrived via #360, #348 and #286 without a baseline refresh — and paying it (a file split, or a `guard-size freeze` that launders 50 lines) does not belong in this PR. The load-bearing documentation of the command lives in `cli/lib/husky/self-host.mts`, next to `SELF_HOST_EXTRAS`, and that one is updated.

## Verification

`biome check --error-on-warnings .` → 0 · `tsc -p tsconfig.json --noEmit` → 0 · `eslint cli gate-engine` → 0 · `vitest run` → 3681 passed / 1 failed, the failure being `cli/__tests__/review.test.mts` “preserves timeout exit 124” returning 143 (SIGTERM) — a wall-clock-bounded test losing its 1s race under a loaded machine, unrelated to this diff.
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@norvalbv, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e8fcaef1-7fb3-4af1-b889-5daef8745ac4

📥 Commits

Reviewing files that changed from the base of the PR and between c7fd683 and 947d234.

⛔ Files ignored due to path filters (3)
  • dist/cli/lib/husky/self-host.mjs is excluded by !**/dist/**
  • dist/package.json is excluded by !**/dist/**
  • dist/skills/correctness/scripts/checklist.mjs is excluded by !**/dist/**
📒 Files selected for processing (11)
  • .claude/skills/correctness/scripts/checklist.mjs
  • .cursor/skills/correctness/scripts/checklist.mjs
  • .devkit/skills-manifest.json
  • cli/__tests__/husky-block.test.mts
  • cli/__tests__/self-host.test.mts
  • cli/lib/husky/self-host.mts
  • docs/decisions/gate-opt-out-is-visible-and-detectable.md
  • gate-engine/coverage/__tests__/produce.test.mts
  • gate-engine/decisions/__tests__/recall-scoring.test.mts
  • package.json
  • skills/correctness/scripts/checklist.mjs

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.

@norvalbv
norvalbv merged commit 6b9db54 into main Aug 7, 2026
1 of 2 checks passed
norvalbv added a commit that referenced this pull request Aug 9, 2026
## Summary
- construct the template-literal fixture from string fragments
- preserve the exact tested fixture bytes
- restore a clean full-repository Biome gate on origin/main

## Root cause
PR #370 added a deliberately literal `${sideEffect()}` fixture inside a single-quoted string. Biome correctly flags that representation as noTemplateCurlyInString, and PR #371 made warnings fatal, so every unrelated devkit ship became blocked.

## Validation
- focused diff-focus Vitest: 30 passed
- Biome check for the changed file: clean
- matcher preflight: 0 new candidates
- commit guard: PASS
- GitNexus: exercised review-cache path is high reach, but production code and fixture value are unchanged

Tracks autonomous report 741fde8b-e5ef-41d5-aaa6-df158dd673ce.
norvalbv added a commit that referenced this pull request Aug 9, 2026
* fix(review): make template fixture Biome-safe

## Summary
- construct the template-literal fixture from string fragments
- preserve the exact tested fixture bytes
- restore a clean full-repository Biome gate on origin/main

## Root cause
PR #370 added a deliberately literal `${sideEffect()}` fixture inside a single-quoted string. Biome correctly flags that representation as noTemplateCurlyInString, and PR #371 made warnings fatal, so every unrelated devkit ship became blocked.

## Validation
- focused diff-focus Vitest: 30 passed
- Biome check for the changed file: clean
- matcher preflight: 0 new candidates
- commit guard: PASS
- GitNexus: exercised review-cache path is high reach, but production code and fixture value are unchanged

Tracks autonomous report 741fde8b-e5ef-41d5-aaa6-df158dd673ce.

* fix(review): resolve checklist assets across providers (sc-1397)

## Summary
- resolve checklist scripts from the provider-projected root
- keep prompts, Bash allowlists, retries, and identities on the same root
- make the correctness skill and reviewer brief portable across Codex, Claude, and Cursor projections
- extract cascade and asset resolution helpers to respect the size ratchet
- add regression coverage for provider-specific checklist resolution

## Root cause
The correctness review runtime and generated assets assumed a .claude projection. Consumers with the valid .agents projection could therefore invoke a nonexistent checklist path and fail with MODULE_NOT_FOUND.

## Validation
- reproduced the failure shape in Frink and verified portable resolution selects .agents/skills/correctness/scripts/checklist.mjs
- review orchestration: 123 tests passed
- provider and asset suites: 106 tests passed
- full review cluster: 663 tests passed before the behavior-preserving extraction
- correctness review: all four lenses passed
- Biome, ESLint, git diff check, and size gate passed
- matcher preflight: 0 candidates; commit-guard passed

## Shared checkout note
The shared checkout TypeScript run is blocked only by unrelated concurrent duplicate imports in setup-manifest.mts and setup-runtime.mts. The isolated ship excludes those changes.

## Dependency
Stacked on #374, which fixes the pre-existing origin/main Biome warning that otherwise blocks every devkit ship.
@norvalbv norvalbv mentioned this pull request Aug 10, 2026
norvalbv added a commit that referenced this pull request Aug 10, 2026
Bump 0.50.0 -> 0.51.0 and rebuild dist from a clean origin/main worktree
(f817b75).

Minor, not patch: two feature PRs landed since 0.50.0 — sentry-additive
restages that keep earned verdicts plus a sentry judge cache (#370), and the
opt-in priorArtGate deny-once step-0 ordering component (#382). The rest of
the range is fixes (#371-#383).

The rebuild also picks up two dist modules that git had never seen. dist/ is
gitignored on working branches by design, so gate-engine/review/cascade/
consumer-assets.mjs and reviewer.mjs — the compiled output of the #376
checklist-asset refactor — existed only on disk. They are force-added here,
which is exactly the path `devkit release` takes for ignored dist output.
Without them the shipped runtime.mjs would import a module absent from the
tag.

Release smoke checks ran: built bin reports 0.51.0, dist/package.json agrees,
and dist/cli/lib/ship/ship-branch.sh still wires ship_read_stdin_body (the
sc-1419 stdin-hang fix survived the build).

Gates bypassed at the maintainer's request (--no-verify); the range was
already reviewed on its constituent PRs.

Co-authored-by: Claude Opus 5 <noreply@anthropic.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