KeeperHub / KeeperHub/keeperhub

The lint gate enforces nothing: pnpm check exits 0 while Biome reports errors

Open
#2,026 2 comments 0 reactions 0 assignees View on GitHub
accepted
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.