Skip to content

Commit fd9ad7b

Browse files
authored
fix(uncheck): second whole-package review: safer fixes, fewer false greens, faster tsc plans (#21)
* test(uncheck): harden the test harness against machine leaks and false greens * fix(uncheck): keep node_modules and links leaving the project out of file lists * fix(uncheck): skip pre-commit package lines with nothing staged or no folder, place hook lines around husky 4 and quoted setup * fix(uncheck): address review of C1 * fix(uncheck): address review of B1 * fix(uncheck): address review of A1 * fix(uncheck): report repositories git refuses in prepare, resolve a symlinked --cwd, test the hook lock deterministically * fix(uncheck): typecheck deletions, never pass a broken setup, keep report-only sherif read-only * fix(uncheck): address review of A2 * fix(uncheck): select the projects tsc itself would check - use the nearest tsconfig.json above a folder that has none, scoped to the folder's files - add the projects of workspace packages that depend on a selected one - check reference graphs whose projects would emit next to their sources with tsc -p --noEmit - pass config paths starting with - or @ to tsc with ./ - leave unreachable tsconfig paths to tsc instead of aborting the run - skip shared bases named tsconfig.json, select the extenders of a deleted base * docs(uncheck): the presets need oxfmt 0.43+, the first to take a boolean sortImports * fix(uncheck): address review of A3 - build the clean projects that -p checked ones reference first, so -p reads fresh declarations - look for a tsconfig.json above the folder only inside its git repository - test workspace dependents reached through references and peer or optional dependencies * fix(uncheck): agent hook keeps sherif report-only, remembers its own block and checks the agent's project * fix(uncheck): address review of A4 * fix(uncheck): staged stages what the fixers produced, keeps later edits and ends with the verdict * test(uncheck): align merged expectations: deletions run tsc, physical --cwd, verdict last * fix(uncheck): staged keeps edits saved during a conflicted run, survives a dying commit index and names what it skipped * fix(uncheck): prepare lifts the lines v0.0.3 appended after an exec, reads setup without comments, keeps lines in blocks * fix(uncheck): tsc builds git-ignored in-place projects with -b, checks solutions in the -p fallback, uses the config above a folder without its own, and names a missing typescript - R5: a references member that emits beside its sources stays in tsc -b when git ignores its would-be JavaScript - R8: an affected solution without sources in a -p graph is checked with -p, so a missing reference fails with TS6053 - R6: the config above cwd is used whenever cwd has no tsconfig.json of its own, next to the nested ones - R7: workspace dependents also come from the packages owning the given files, before the not-covered exit - R9: a deleted base extended by package name selects its extenders - R11/R17: no tsconfig.json fails even for unrelated files; a missing typescript reads 'not installed' - R19: README scopes deleted or moved files to the hooks * fix(uncheck): address review of S * fix(uncheck): address review of P * fix(uncheck): leave out links into node_modules, isolate tests from /etc/gitattributes and ancestor ignore files, wait for parallel prepares * fix(uncheck): agent hook re-arms its block once the checks pass, follows links, reads Copilot's sessionId and reports a repository git refuses * docs(uncheck): links into node_modules, a repeated send-back, refused repositories in every command * refactor(uncheck): simplify the review changes and plan tsc faster - the runner skips a check that plans no commands, replacing the guards in oxlint and oxfmt and the evenIfRequired flag; literal runs imply the fixes stay within the given files - shared helpers replace copies (fileKind, resolveFolders, isOutside, logLines, a batched git listing, one copyBack); rawDiff always ends with -- - the agent hook finds its repository with one git call and looks it up once when the agent works in its project folder - tsc reads each config and its extends chain once per run and asks git once which projects build in place, so planning a 300-package monorepo drops from about 2-5 s to under 0.2 s with the same plans - tests share their repeated constants and assertion helpers
1 parent 97b5666 commit fd9ad7b

57 files changed

Lines changed: 4628 additions & 776 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎packages/uncheck/README.md‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ uncheck needs Node 22.20 or later. Install only the tools you want: a check runs
4747
$ npx uncheck
4848
uncheck in /home/me/my-app
4949
○ sherif skipped, not installed
50-
▶ oxlint
50+
▶ oxlint --ignore-pattern=node_modules --no-error-on-unmatched-pattern
5151
✔ oxlint passed 67ms
52-
▶ oxfmt --check
52+
▶ oxfmt --check --no-error-on-unmatched-pattern
5353
Format issues found in above 2 files. Run without `--check` to fix.
5454
✘ oxfmt failed 65ms
5555
▶ tsc -p tsconfig.json --noEmit
@@ -83,9 +83,9 @@ npx uncheck src '!src/generated' # a directory, minus a part of it
8383
npx uncheck '!**/*.gen.ts' # everything except some files
8484
```
8585

86-
uncheck turns your paths into one file list that every tool gets, so they never disagree about what a path means. Directories and globs match the files git knows about (tracked, or new and not ignored), dot files included. A path that exists is never read as a glob, so `'app/[id]/page.tsx'` and `'app/(marketing)/**'` just work. A path that matches nothing fails the run, unless you pass `--no-error-on-unmatched-pattern`.
86+
uncheck turns your paths into one file list that every tool gets, so they never disagree about what a path means. Directories and globs match the files git knows about (tracked, or new and not ignored), dot files included, but never links that point outside the directory uncheck runs in or into `node_modules`. Without paths, each tool finds the files itself, and oxlint checks and fixes those links too: pass `.` to leave them out. A path that exists is never read as a glob, so `'app/[id]/page.tsx'` and `'app/(marketing)/**'` just work. An exclusion glob also leaves out the folders it matches, as in `.gitignore`: `'!**/generated'`. A path that matches nothing fails the run, unless you pass `--no-error-on-unmatched-pattern`.
8787

88-
tsc then checks only the projects that include one of the files, and sherif runs only when a `package.json` or `pnpm-workspace.yaml` is among them.
88+
tsc then checks only the projects that include one of the files, and the projects that depend on those. sherif runs only when a `package.json` or `pnpm-workspace.yaml` is among the files.
8989

9090
## Run it before every commit
9191

@@ -101,28 +101,32 @@ Add a `prepare` script, so every clone sets up the hook on install, and run it o
101101

102102
Every commit then runs `uncheck staged --fix`: it checks the staged files, fixes what oxlint and oxfmt can, and stages those fixes. A failing check blocks the commit, and `git commit --no-verify` skips the hook. No lint-staged or simple-git-hooks needed.
103103

104-
| `prepare` flag | Effect |
104+
To change the hook, put flags in the `prepare` script and run it again, such as `"prepare": "uncheck prepare --pre-commit --only=oxlint --only=oxfmt"`. Every install runs the script, so a flag passed only by hand is undone by the next install.
105+
106+
| Flag in the `prepare` script | Effect |
105107
| ------------------------------- | ---------------------------------------------------------------------- |
106108
| `--no-fix` | The hook only checks and never changes your files |
107109
| `--allow-empty` | The hook lets a commit through when the fixes undo every staged change |
108-
| `--only`, `--skip`, `--require` | Written into the hook command |
110+
| `--only`, `--skip`, `--require` | Pick the checks the hook runs |
109111

110-
Run `prepare` again with other flags to change the hook. What it guarantees:
112+
What the hook guarantees:
111113

112-
- **You commit what was checked.** After `git add -p`, the unstaged part of a file is set aside while the checks run and put back afterwards, even after Ctrl-C.
114+
- **Each staged file is checked as you staged it.** After `git add -p`, the unstaged part of a file is set aside while the checks run and put back afterwards, even after Ctrl-C.
113115
- **Nothing is lost.** If a fix clashes with your unstaged changes, every fix is undone and the commit stops. Stage the whole file, or stash the rest, and commit again.
114-
- **Only fixes to staged files are staged.** During a merge, only files that differ from the branch being merged in are checked.
116+
- **Only fixes to staged files are staged.** The fixes are staged once oxlint and oxfmt finish, so an edit you save while tsc runs stays unstaged. During a merge, only files that differ from the branch being merged in are checked.
115117
- **No empty commits.** If the fixes undo every staged change, the commit fails, unless you pass `--allow-empty`.
116118

117119
Good to know:
118120

119-
- tsc checks whole projects, so it can report errors in files you did not stage. `--only=oxlint --only=oxfmt` keeps the hook inside the commit.
121+
- tsc checks whole projects as they are on disk, so it can report errors in files you did not stage, and pass thanks to a file you forgot to `git add`. `--only=oxlint --only=oxfmt` keeps the hook inside the commit.
122+
- A hook line checks the staged files in its folder and below, so a line at the root already covers every package. A package's own line adds its checks, with its flags, on top.
120123
- sherif only reports in the hook, since its fixes reach beyond the commit. Run `npx uncheck --fix` for them.
121124
- A commit no selected check covers, such as a README change with `--only=tsc`, passes. Add `--require=tsc` to make it fail.
122125
- An existing hook is kept: uncheck adds one line after its setup (comments, `source`, `export`, variables) and before its commands. With husky 9 or Vite+, it writes the `pre-commit` file they run.
123-
- uncheck writes nothing, and says why, outside a git repository, when `core.hooksPath` comes from your global or system git config, or when the existing hook is not a shell script. Your install keeps working.
126+
- uncheck writes nothing, and says why, outside a git repository, when git refuses the repository, when `core.hooksPath` comes from your global or system git config, or when the existing hook is not a shell script. Your install keeps working.
124127
- The hook runs uncheck through your package manager (`pnpm exec`, `yarn run --silent`, `bunx --no-install` or `npx --no`), so a missing install fails instead of downloading uncheck.
125128
- Yarn 2+ does not run `prepare`. Use `postinstall` instead, and in a package you publish, turn it off while packing, for example with `"prepack": "pinst --disable"` and `"postpack": "pinst --enable"`.
129+
- Installs that leave out devDependencies (`npm ci --omit=dev`, `NODE_ENV=production`, `bun install --production`, `yarn workspaces focus --production`) still run `prepare` or `postinstall`, but without uncheck. Append `|| exit 0` so they pass: `"prepare": "uncheck prepare --pre-commit || exit 0"`.
126130

127131
## Run it after every agent turn
128132

@@ -138,33 +142,37 @@ npx uncheck hooks install # or pick them from a list
138142
| Cursor | `cursor` | `.cursor/hooks.json` |
139143
| GitHub Copilot | `copilot` | `.github/hooks/uncheck.json` |
140144

141-
Whenever the agent finishes a turn, the hook runs `uncheck hooks run --fix`. It checks the files changed since the last commit, fixes what it can, and when problems remain, sends the agent back to fix them. That happens at most once per turn, so an agent that cannot fix something is never stuck in a loop.
145+
Whenever the agent finishes a turn, the hook runs `uncheck hooks run --fix`. It checks the files changed since the last commit, fixes what oxlint and oxfmt can, and when problems remain, sends the agent back to fix them. It sends the agent back again only after the checks have passed in between, whatever other hooks do, so an agent that cannot fix something is never stuck in a loop.
142146

147+
- **sherif only reports** here, since its fixes reach beyond the agent's change: they move versions in other packages and run your install. Run `npx uncheck --fix` for them.
143148
- **Too slow?** Leave the typecheck to CI: `npx uncheck hooks install claude --only=oxlint --only=oxfmt`. Install again to change the flags.
149+
- **Outside git** and before the first commit, the hook checks the whole folder, and oxlint also fixes the files that links in it point to. In a repository git refuses, such as one another user owns, the hook checks nothing and shows git's message.
150+
- **Installed from a package?** The hook finds uncheck only where your package manager can run it, so add uncheck to the workspace root too when the agent may work outside that package.
144151
- **Your config is kept.** Other hooks and settings stay, and installing again only updates uncheck's entry. Comments in the file are lost when it is rewritten.
145152
- **Avoid double runs.** Cursor and Copilot CLI also run the hooks in `.claude/settings.json`, so add `cursor` or `copilot` next to `claude` only where they do not read that file.
146153
- **Copilot** reads `.github/hooks` only at the top of the repository, so install `copilot` from there.
147154

148155
## Monorepos
149156

150-
Run uncheck from the workspace root, the folder whose `package.json` has `workspaces` or that has a `pnpm-workspace.yaml`, to check the whole monorepo.
157+
Run uncheck from the workspace root, the folder whose `package.json` has `workspaces` or whose `pnpm-workspace.yaml` lists `packages`, to check the whole monorepo.
151158

152-
**sherif** checks the workspace as a whole, so it only runs at the root. Configure it in the `sherif` field of the root `package.json`, [as sherif documents](https://github.com/QuiiBz/sherif). With `--fix`, mismatched versions move to the highest one (unless you set `select`), and your install runs afterwards (unless you set `"noInstall": true`). When `CI` is set, sherif only reports.
159+
**sherif** checks the workspace as a whole, so it only runs at the root. Configure it in the `sherif` field of the root `package.json`, [as sherif documents](https://github.com/QuiiBz/sherif). With `--fix`, mismatched versions move to the highest one (unless you set `select`), and your install runs afterwards (unless you set `"noInstall": true`). When `CI` is set, sherif only reports. Leave out `"fix": true`, since uncheck decides when sherif fixes: a run that only reports fails while it is set.
153160

154161
**TypeScript.** uncheck finds every `tsconfig.json` and follows their `references`:
155162

156-
- Projects linked by `references` are built with one `tsc -b`, which writes what your configs ask for, such as declarations.
157-
- Every other project is checked with `tsc -p --noEmit`, a few at a time.
158-
- When only some files are checked, tsc runs just the projects that include them, and the projects that reference those. A changed tsconfig selects every project that extends it.
163+
- Projects linked by `references` are built with one `tsc -b`, which writes what your configs ask for, such as declarations. A project that sets none of `noEmit`, `emitDeclarationOnly`, `outDir` and `outFile` gets JavaScript next to its sources. When git ignores that JavaScript, the project builds in place on purpose and stays in `tsc -b`. Otherwise, as with Vite's `tsconfig.node.json`, it and the projects that reference it are checked with `tsc -p --noEmit` instead, after `tsc -b` builds the projects they reference. These cannot import each other: tsc reports TS6305.
164+
- Every other project is checked with `tsc -p --noEmit`, a few at a time. A `tsconfig.json` that other configs extend and that includes no files is a shared base, not a project.
165+
- When only some files are checked, tsc runs just the projects that include them, the projects that reference those, and the projects of the packages that depend on the files' packages. A changed tsconfig selects every project that extends it. In the hooks, a deleted or moved file also selects the projects that included or extended it.
166+
- In a folder without its own `tsconfig.json`, such as a package that shares the root one, uncheck also uses the nearest one above it in the same git repository, when it includes files of that folder.
159167

160-
**Hooks.** Each package that runs `uncheck prepare --pre-commit` gets its own line in the one pre-commit hook, with its own flags:
168+
**Hooks.** Each package that runs `uncheck prepare --pre-commit` gets its own line in the one pre-commit hook, with its own flags. A package's line runs only when the commit changes files in that package:
161169

162170
```sh
163171
#!/bin/sh
164172
# Written by `uncheck prepare`, run it again to change the command.
165173
pnpm exec uncheck staged --fix || exit 1
166-
(cd "packages/a" && pnpm exec uncheck staged --fix --only=oxlint) || exit 1
167-
(cd "packages/b" && pnpm exec uncheck staged --fix) || exit 1
174+
git --literal-pathspecs diff --cached --quiet -- "packages/a" || [ ! -d "packages/a" ] || (cd "packages/a" && pnpm exec uncheck staged --fix --only=oxlint) || exit 1
175+
git --literal-pathspecs diff --cached --quiet -- "packages/b" || [ ! -d "packages/b" ] || (cd "packages/b" && pnpm exec uncheck staged --fix) || exit 1
168176
```
169177

170178
An agent hook installed from a package folder checks only that package, wherever the agent moves to.
@@ -199,14 +207,16 @@ export default defineConfig({ ...middleapi })
199207
}
200208
```
201209

202-
The presets need oxlint 1.70+, oxfmt 0.41+ and TypeScript 5.6+. The tsconfig presets load no runtime types, so name yours: `"types": ["node"]` for Node.js, or `"lib": ["ES2022", "DOM", "DOM.Iterable"]` for browsers.
210+
The presets need oxlint 1.70+, oxfmt 0.43+ and TypeScript 5.6+. The tsconfig presets load no runtime types, so name yours: `"types": ["node"]` for Node.js, or `"lib": ["ES2022", "DOM", "DOM.Iterable"]` for browsers.
203211

204212
## Troubleshooting
205213

206214
**A path looks like a command, a flag or an exclusion.** Start it with `./`: `./staged`, `./-draft.ts`, `'./!notes.ts'`.
207215

208216
**`uncheck dist` says "No files match".** git ignores that folder, so it holds no project files. You can still name an ignored file directly.
209217

218+
**git refuses the repository ("detected dubious ownership").** git does not trust a repository another user owns, such as a checkout mounted into a container, so `prepare` writes no hook and `uncheck staged` and the agent hook check nothing. Run the `safe.directory` command `git status` prints there, then try again.
219+
210220
**A commit stops with "An earlier run left the unstaged versions of your files in …".** A pre-commit run was killed before it could put your unstaged changes back. Copy what your files are missing from the folder the message names, delete the folder, and commit again.
211221

212222
## Sponsors

‎packages/uncheck/src/checks/oxfmt.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,13 @@ export const oxfmt: Check = {
1414
return yield* Effect.fail(new NothingToCheck({ reason: 'not installed' }))
1515
}
1616

17-
const args = fix ? [] : ['--check']
17+
// A folder with nothing oxfmt handles, or a given file it does not (a .txt file), is no failure.
18+
const args = [...(fix ? [] : ['--check']), '--no-error-on-unmatched-pattern']
1819

1920
if (files === undefined) {
2021
return [{ bin, args }]
2122
}
2223

23-
// Given files may include ones oxfmt does not handle (a .txt file), which is not a failure.
24-
return argvBatches(files).map((batch) => ({
25-
bin,
26-
args: [...args, '--no-error-on-unmatched-pattern'],
27-
files: batch,
28-
}))
24+
return argvBatches(files).map((batch) => ({ bin, args, files: batch }))
2925
}),
3026
}

‎packages/uncheck/src/checks/oxlint.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,19 @@ export const oxlint: Check = {
1414
return yield* Effect.fail(new NothingToCheck({ reason: 'not installed' }))
1515
}
1616

17-
const args = fix ? ['--fix'] : []
17+
const args = [
18+
...(fix ? ['--fix'] : []),
19+
// oxlint walks into node_modules unless an ignore file says not to, and a fix there rewrites
20+
// installed packages, through pnpm's hard links even those of its shared store.
21+
...(files === undefined ? ['--ignore-pattern=node_modules'] : []),
22+
// A folder with nothing oxlint handles, or a given file it does not (a .md file), is no failure.
23+
'--no-error-on-unmatched-pattern',
24+
]
1825

1926
if (files === undefined) {
2027
return [{ bin, args }]
2128
}
2229

23-
// Given files may include ones oxlint does not handle (a Markdown file), which is not a failure.
24-
return argvBatches(files).map((batch) => ({
25-
bin,
26-
args: [...args, '--no-error-on-unmatched-pattern'],
27-
files: batch,
28-
}))
30+
return argvBatches(files).map((batch) => ({ bin, args, files: batch }))
2931
}),
3032
}

‎packages/uncheck/src/checks/sherif.ts‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,49 @@ import process from 'node:process'
22

33
import { Effect, FileSystem, Path, Predicate } from 'effect'
44

5-
import { NothingToCheck } from '../errors'
5+
import { CannotCheck, NothingToCheck } from '../errors'
66
import { readJson } from '../files'
77
import { resolveBin } from '../tool'
88
import type { Check } from '../types'
99

1010
const WORKSPACE_FILES = new Set(['package.json', 'pnpm-workspace.yaml'])
1111

12+
// pnpm also keeps its settings in pnpm-workspace.yaml, and sherif fails on one without packages.
13+
const DECLARES_PACKAGES = /^["']?packages["']?\s*:/m
14+
1215
export const sherif: Check = {
1316
name: 'sherif',
1417
fixes: 'workspace',
15-
plan: Effect.fn(function* ({ cwd, fix, files }) {
18+
plan: Effect.fn(function* ({ cwd, fix, files, deleted }) {
1619
const fs = yield* FileSystem.FileSystem
1720
const path = yield* Path.Path
1821

19-
if (files?.some((file) => WORKSPACE_FILES.has(path.basename(file))) === false) {
22+
const bin = yield* resolveBin('sherif', cwd)
23+
24+
if (bin === undefined) {
25+
return yield* Effect.fail(new NothingToCheck({ reason: 'not installed' }))
26+
}
27+
28+
if (
29+
files !== undefined &&
30+
![...files, ...deleted].some((file) => WORKSPACE_FILES.has(path.basename(file)))
31+
) {
2032
return yield* Effect.fail(
21-
new NothingToCheck({ reason: 'no package.json among the given files' }),
33+
new NothingToCheck({ reason: 'no package.json among the given files', unrelated: true }),
2234
)
2335
}
2436

25-
const [bin, manifest, pnpmWorkspace] = yield* Effect.all(
37+
const [manifest, pnpmWorkspace] = yield* Effect.all(
2638
[
27-
resolveBin('sherif', cwd),
2839
readJson(path.join(cwd, 'package.json')),
29-
fs.exists(path.join(cwd, 'pnpm-workspace.yaml')).pipe(Effect.orElseSucceed(() => false)),
40+
fs.readFileString(path.join(cwd, 'pnpm-workspace.yaml')).pipe(
41+
Effect.map((text) => DECLARES_PACKAGES.test(text)),
42+
Effect.orElseSucceed(() => false),
43+
),
3044
],
3145
{ concurrency: 'unbounded' },
3246
)
3347

34-
if (bin === undefined) {
35-
return yield* Effect.fail(new NothingToCheck({ reason: 'not installed' }))
36-
}
37-
3848
if (manifest === undefined) {
3949
return yield* Effect.fail(new NothingToCheck({ reason: 'no package.json found' }))
4050
}
@@ -44,6 +54,16 @@ export const sherif: Check = {
4454
}
4555

4656
if (!fix || process.env.CI !== undefined) {
57+
// sherif reads this setting on every run, and no flag turns it off.
58+
if (Predicate.hasProperty(manifest.sherif, 'fix') && manifest.sherif.fix === true) {
59+
return yield* Effect.fail(
60+
new CannotCheck({
61+
reason:
62+
'would fix files while uncheck only reports, remove "fix": true from the sherif field of package.json',
63+
}),
64+
)
65+
}
66+
4767
return [{ bin, args: [] }]
4868
}
4969

0 commit comments

Comments
 (0)