KeeperHub / KeeperHub/keeperhub
The lint gate enforces nothing: pnpm check exits 0 while Biome reports errors
- Dominant language
- TypeScript
- Stars
- 24
- Forks
- 93
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 266
Description
## Reason
`pnpm check` runs `ultracite check`, which shells out to Biome, fails to parse its output, prints `Error: Failed to parse Biome output` followed by the raw JSON, and then **exits 0 regardless of what Biome found**.
Runnable reproduction on a clean `staging`:
```
printf '\nconst badlyFormatted={a:1, b:2};\nexport const _unused = badlyFormatted;\n' >> lib/http-status.ts
pnpm check; echo "exit: $?" # exit: 0 <-- the bug
node_modules/.bin/biome check .; echo "exit: $?" # exit: 1
```
What told me to expect otherwise: `.claude/hooks/pre-commit-checks.sh` blocks commits on a non-zero `pnpm check`, CLAUDE.md instructs contributors to run it before committing, and CI runs a `lint` job. All three are wired to an exit code that is always 0, so all three are decorative.
Cost: a full `biome check` on current `staging` reports **76 errors**. Every one of them landed through a green `lint` job. I hit this directly on #2022 — real formatting violations in my own change passed `pnpm check` and were only caught by invoking Biome per file.
## Scope
Covers the `check` and `fix` scripts, the Biome config, and clearing the existing backlog so the armed gate is green on merge. Those cannot be separated: arming the gate without clearing the backlog turns CI red for everyone on the next push.
The 76 errors break down as:
- **28** in untracked `.claude/` agent scratch. Biome has no `vcs` config so it does not honour `.gitignore`.
- **23** `lint/performance/noDelete`, every one of them `delete process.env.X` in test teardown.
- **25** whole-file `format` violations across 26 files.
Explicitly does **not** cover the ultracite 6.5.1 to 7.10.2 major upgrade. A major version can change rule sets and surface a fresh batch of errors, which would turn a mechanical cleanup into an open-ended one. Worth doing, separately, on purpose.
Also does not cover the 80 remaining `warning`-level diagnostics. They do not gate, and folding them in would triple the diff.
## Plan
1. Exclude `.claude` in `files.includes`, as `.worktrees` already is.
2. Disable `performance/noDelete` in the existing test-only `overrides` block. **Do not auto-fix these.** The rule's replacement is wrong for the only pattern we use it in: `process.env` coerces values to strings, so `process.env.X = undefined` leaves the key present holding the literal string `"undefined"` instead of unsetting it, and a test asserting the variable is absent would silently start seeing a truthy value.
3. Run the formatter over the remaining 25.
4. Point `check` at `biome check` and `fix` at `biome check --write`, dropping the ultracite wrapper that swallows the exit code.
5. Prove it: the same injected violation must exit non-zero.
Contributor guide
Research direction
Start by running `pnpm check` and `node_modules/.bin/biome check .` on the clean staging branch, then inspect the check and fix scripts, the Biome config, `.claude/hooks/pre-commit-checks.sh`, and CLAUDE.md. Apply the stated scope to the 76 errors without auto-fixing `process.env` teardown, and verify that the injected violation makes `pnpm check` exit non-zero while the backlog is cleared.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- ci-cd, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100