Repository navigation
feat(uncheck): implement the CLI with agent stop hooks and dogfood it - #1
Conversation
One `uncheck` command runs oxlint, oxfmt --check and tsc, each only when the project uses it, with --fix, per-tool --oxlint/--oxfmt/--tsc switches and paths forwarded to oxlint and oxfmt while tsc runs only the projects whose files/include/exclude take them. tsconfig references are built with tsc -b first, standalone projects are checked with tsc -p afterwards. `uncheck hooks` writes Claude Code, CodeBuddy, Cursor, Windsurf and Copilot hook configs, and `--hook` checks the files an agent just edited and hands remaining problems back to it. The repo now runs uncheck instead of eslint and is on TypeScript 7.
commit: |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two issues block the advertised behavior: the GitHub Copilot hook never finds the edited file (its payload carries toolArgs as a JSON string), and path-scoped typecheck skips projects referenced under a name other than tsconfig.json — which includes every --hook run in a solution-style repo. Details inline.
Reviewed changes
- CLI spine —
command.ts/bin.tsplan and run oxlint,oxfmt --checkand tsc, with per-step--<tool>auto/require/skip flags, forwarded paths, merged outcomes and exit code. - Typecheck planning —
typecheck.ts/tsconfig.tsdiscover configs throughgit ls-files, followreferences, compile tsc-stylefiles/include/excludematching and splittsc -broots fromtsc -pstandalone projects. - Agent hooks —
hooks.tswrites/merges five agents' hook configs;uncheck --hookchecks only edited files and returnsadditionalContextwhile exiting 0. - Repo dogfooding — eslint removed,
pnpm check/check:fixand lint-staged/CI now use uncheck,preparestubs the bin, TypeScript 7. - Tests — 31 unit and integration tests, including real-tool fixtures in
command.test.ts.
ℹ️ Removed ESLint still referenced by Dependabot
.github/dependabot.yml still defines an eslint group and excludes eslint, @antfu/eslint-config and eslint-plugin-* from the minor/patch group, but this PR removes every ESLint dependency. The group now matches nothing and the exclusion is dead config. Low impact, but it is part of the cleanup the rest of this PR does.
Technical details
# Dependabot config references removed ESLint packages
## Affected sites
- `.github/dependabot.yml:13-17` — `eslint` group patterns `eslint`, `@antfu/eslint-config`, `eslint-plugin-*`.
- `.github/dependabot.yml:20-23` — `exclude-patterns` listing the same packages.
- `package.json` — no ESLint dependency remains.
## Required outcome
Dependabot's grouping reflects the dependencies the repo actually has.
## Suggested approach (optional)
Drop the `eslint` group and remove the ESLint patterns from `exclude-patterns`; group `oxlint`/`oxfmt` if grouping is still wanted.ℹ️ Nitpicks
scripts/sync-sponsors.ts:206— the comment still says the blank line is removed by "eslint's markdown fixer", but this PR switched the follow-up command topnpm check:fix.typecheck.ts:104and:128—namesUncheckableExtensiontreats a directory whose last segment contains a dot (src/foo.bar) as a file and drops it; even unfiltered,path.extname(target)routes it toincludesFile, so such a directory can never select a project.hooks.ts:191—installHooktreats any occurrence of the substringuncheckas "already installed", so an unrelated mention (a permission, another hook, the package name) suppresses the merge and a stale command is never refreshed.command.ts:163/packages/uncheck/README.md:71— the comment and README say only Claude Code and CodeBuddy surface the returned context; GitHub Copilot'spostToolUsealso honorsadditionalContextper the hooks reference.
DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏
| return [] | ||
| } | ||
|
|
||
| const selected = yield* Effect.filter( |
There was a problem hiding this comment.
selectTsconfigs only considers discovered tsconfig.json files, so a path never selects a project whose config is referenced under another name — the very common tsconfig.json → tsconfig.app.json/tsconfig.node.json solution layout. With a path, tsc is silently skipped for those projects, and --hook always passes paths (command.ts:213), so the hook never typechecks them.
Technical details
# Path scoping ignores referenced configs not named `tsconfig.json`
## Affected sites
- `packages/uncheck/src/typecheck.ts:110-119` — `selectTsconfigs` filters only `entries`.
- `packages/uncheck/src/typecheck.ts:226` — discovery keeps only `path.basename(relative) === 'tsconfig.json'`, so `tsconfig.app.json` is never an entry.
- `packages/uncheck/src/command.ts:290-294` — no selected project ⇒ `tsc` reported skipped, not failed; the run can end with all-checks-passed while the edited file was never typechecked.
- `packages/uncheck/README.md:56` — a file is documented to select the projects whose inputs take it; for a solution root (`files: []`) that is only true of `tsconfig.json` itself.
## Required outcome
A path inside a referenced project's inputs selects and checks that project in both normal and hook mode.
## Suggested approach (optional)
Resolve selection over the full project graph: load the reference closure of the discovered entries and match each path against every config in it, not just the entries. (Discovering `tsconfig*.json` alone would also surface base configs that are not real projects.)
## Open questions for the human (optional)
Should a selected project that is referenced by a non-selected project also surface the referencing project's reference-integrity errors (missing/composite)? `planTypecheck` loads forward references only, which is a defensible product choice but differs from the README wording that referenced projects are built with `tsc -b`.Paths and globs are resolved by uncheck into one file list that every tool receives, so tools never disagree on what a pattern means; a pattern matching nothing fails unless --no-error-on-unmatched-pattern is given. Matching uses Node's path.matchesGlob, which sets the floor at Node 22. Each tool is its own module under steps/ (oxlint, oxfmt, tsc) built on one Step contract, registered in checks.ts. Output goes through Effect's Console with children always piped, per the Effect CLI guide. Agent hooks now run once per turn instead of after every edit: `uncheck hooks install` writes stop-event configs and `uncheck hooks run` checks the working-tree changes with every step, then sends the agent back at most once (exit 2 for Claude Code and CodeBuddy, follow-up message for Cursor, block decision for Copilot).
dinwwwh
changed the title
There was a problem hiding this comment.
Important
Two correctness issues in the new path resolver: path.matchesGlob is unavailable on the Node versions engines.node allows, and literal bracket path segments (Next.js dynamic routes) are classified as globs and dropped. Details inline. The prior Copilot toolArgs and coversDirectory concerns are addressed; the referenced-configs selection concern from the first review still stands and its thread is left open.
Reviewed changes
- Replaced the per-edit
--hookwith a per-turn stop-hook command tree (hooks install/hooks run), with per-family continue/block feedback and a once-per-turn loop guard. - Added
resolvePaths: paths, directories, globs and!patternexclusions now resolve to one concrete file list every step receives, with--no-error-on-unmatched-pattern. - Split the CLI into
checks.ts,step.ts,resolve.tsandsteps/, removingtools.ts,typecheck.ts,tsconfig.ts,ui.tsandextractHookPaths. - Reworked
tscto consume resolved files and select projects throughincludesFileonly, droppingcoversDirectory. - Routed output through Effect's
Consoleand centralized color detection, so hook mode and tests capture output by swapping the service. - Repo/CI dogfood: ESLint config and dependencies removed, CI matrix narrowed to Node 26/24/22.
ℹ️ Hook tests encode assumed payload shapes
hooks.test.ts and the hook-mode tests in command.test.ts feed payloads authored to match stopAgent/stopFeedback, so they cannot catch a wrong contract — exactly how the previous Copilot toolArgs bug shipped with green CI. Capturing one real stop payload per agent as a fixture would turn those tests into contract checks.
Technical details
# Agent stop-hook tests are self-referential
## Affected sites
- `packages/uncheck/src/hooks.test.ts:5-9` — asserts `hook_event_name: 'Stop'` / `'stop'` and `stopReason` from hand-written objects.
- `packages/uncheck/src/command.test.ts:409-455` — same, plus `loop_count`/`stop_hook_active` shapes.
- `packages/uncheck/src/hooks.ts:156-189` — `stopAgent`/`stopFeedback` are only as correct as those assumptions.
## Required outcome
Tests fail when a vendor changes the stop event name, the loop-count field, or the continue output, rather than only when the implementation changes.
## Suggested approach (optional)
Record a real `stop` / `Stop` / `agentStop` payload from each agent into a fixture and feed it verbatim. Cursor is the one to capture first: its docs list `hook_event_name` in the common input but never show the `stop` value.
## Open questions for the human (optional)
The GitHub Copilot `agentStop` hook file must live on the default branch to reach the cloud agent — is committing it part of the intended setup flow?ℹ️ Nitpicks
hooks.ts:38—show_output: trueis documented not to apply topost_cascade_response; either drop it or soften the README:73 claim that Windsurf "shows the report".files.ts:81— glob matching runs with dotfiles off, souncheck '**/*.ts'skips dotfiles while directory expansion viagit ls-files(uncheck .) includes them; worth documenting the difference.checks.ts:78— when every match is removed by!pattern,filesandunmatchedare both empty and the run exits 0 with "nothing to check", which reads against README:46 ("A path that matches nothing fails the run").
DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏
| } | ||
|
|
||
| /** Characters that make a pattern a glob rather than a plain path. */ | ||
| const GLOB_CHARACTERS = /[*?[\]{}()]/ |
There was a problem hiding this comment.
GLOB_CHARACTERS includes [ and ], so a literal path containing them is treated as a glob and never reaches the fs.stat branch. posix.matchesGlob('app/[id]/page.tsx', 'app/[id]/page.tsx') is false — [id] parses as a character class — so uncheck 'app/[id]/page.tsx' fails with "No files match", and hooks run (where allowUnmatched: true) silently drops the changed file.
Technical details
# Bracket path segments are classified as globs and match nothing
## Affected sites
- `packages/uncheck/src/files.ts:137` — `GLOB_CHARACTERS = /[*?[\]{}()]/` includes `[`/`]`.
- `packages/uncheck/src/files.ts:81-83` — the glob branch runs before `fs.stat`, so an existing `app/[id]/page.tsx` is never recognized as a file.
- `packages/uncheck/src/hooks.ts:119` — hook mode passes `allowUnmatched: true`, so the drop is silent.
## Required outcome
A path that names an existing file or directory is checked even when its name contains `[`, `]` or another glob metacharacter.
## Suggested approach (optional)
Probe `fs.stat` first and fall through to glob matching only when the path does not exist; or narrow `GLOB_CHARACTERS`, noting `[`/`]` are the only characters in the set that break a literal self-match (`*`, `?`, `{`, `}`, `(`, `)` were tested and are safe).| return 'claude' | ||
| } | ||
|
|
||
| if (payload.hook_event_name === 'stop') { |
There was a problem hiding this comment.
Cursor is classified by hook_event_name === 'stop'. Cursor's docs list hook_event_name in the common hook input but never show its value for stop (only "workspaceOpen"), and at least one field report says newer Cursor builds send { status, loop_count } without it. If the field is absent, stopAgent returns undefined and Cursor never receives a followup_message — a silently dead hook; if the value is the capitalized "Stop", it is misclassified as Claude and exits 2 instead. Worth confirming against a captured real payload.
…-skip
Checks are plain objects that yield their commands or fail with
NothingToCheck (skipped, or failed under --require) or CannotCheck
(always failed); the runner decides the outcome. Per-check flags are
replaced by repeatable --require and --skip, and --cwd sets the
directory to run in.
Every function that wrapped Effect.gen is an Effect.fn, tiny helpers
are inlined, casts are gone, and JSON is narrowed by hand. The package
is CLI-only: no index entry or exports, only the bin.
Layout: types.ts, errors.ts, style.ts, tool.ts, files.ts, run.ts,
checks/{oxlint,oxfmt,tsc}.ts, commands/{flags,uncheck}.ts and
commands/hooks/. Tests live in tests/ and drive the tsc check through
its plan instead of internals.
There was a problem hiding this comment.
ℹ️ No critical issues — one minor output regression inline.
Reviewed changes
- CLI reorganized —
command.ts/checks.ts/step.ts/steps/becamecommands/{uncheck,hooks,flags}.tsandchecks/{oxlint,oxfmt,tsc}.tsover a plain-objectCheckcontract, andsrc/index.tsplus the packageexportswere removed, leaving a bin-only package. - Check flags replaced — the per-tool boolean flags became repeatable
--require=<check>/--skip=<check>(with a contradiction error), and a new--cwdflag runs in another directory. - Stop hooks inlined —
stopAgent/stopFeedbackmoved intocommands/hooks/run.ts;hooks.test.tswas deleted and the suite moved topackages/uncheck/tests/. - Shared helpers moved —
ancestors/readJsonnow live infiles.tsand argument batching intool.ts; the run log no longer hides the internal--no-error-on-unmatched-patternflag (see inline).
DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏
| const { bin, args, files } = invocation | ||
| const tail = files === undefined ? [] : files.length <= 3 ? files : [`[${files.length} files]`] | ||
|
|
||
| return Console.log(`${dim('▶')} ${bold(bin.name)} ${dim([...args, ...tail].join(' '))}`.trimEnd()).pipe( |
There was a problem hiding this comment.
The refactor dropped the describeArgs helper that filtered the internal --no-error-on-unmatched-pattern flag out of the run log, so every path-scoped invocation now prints it (the integration tests were updated to expect it). This is user-facing output — consider filtering the flag here so the log shows only flags the user actually passed.
The uncheck command owns its flags, check registry and run loop; the hooks commands reuse them from there and the entry attaches hooks as a subcommand, so no root module knows about flags. Process execution moves to tool.ts next to bin resolution.
Like middleapi/orpc: the package's `prepare` stubs dist with jiti and `prepack` runs the real unbuild, so there is no `build` script and the root `prepare` only installs git hooks. Runtime dependencies become devDependencies and are inlined, so the published bin is self-contained.
6658777 to
fdd390d
Compare
1 participant
uncheckis now a working CLI: one command that runs oxlint,oxfmt --checkand tsc, each only when the project uses it, with--fixto apply lint fixes and formatting. Paths and globs are resolved byuncheckitself into one file list that every tool receives, so tools never disagree on what a directory or pattern means, and tsc runs only the projects whosefiles/include/exclude(withextends) take those files. Agent hooks run once per turn and send the agent back to fix what remains. The repo itself now runsuncheckinstead of eslint and is on TypeScript 7 and Node 22.Checks
git ls-files,referencesare followed to build the graph, roots are built withtsc -bso references come first, standalone projects runtsc -pafterwards, and cycles are reported. Config parsing is independent of the TypeScript version, since TypeScript 7 ships no compiler API.--no-error-on-unmatched-patternruns with whatever matched. Long file lists are batched under the platform argument limit.--require=<check>and--skip=<check>(repeatable) override auto-detection: a required check fails instead of being skipped when its tool is missing, and a check that applies but is broken (circular references, a tsconfig without typescript) always fails.--cwdruns in another directory.Agent hooks
uncheck hooks install [claude|codebuddy|cursor|windsurf|copilot]writes or merges each agent's stop-event config (interactive multi-select on a TTY).uncheck hooks run --fixruns every check on the files changed since the last commit, reports on stderr, and sends the agent back at most once per turn: exit code 2 for Claude Code and CodeBuddy, a follow-up message for Cursor, ablockdecision for Copilot; Windsurf sees the report. Running per turn rather than per edit keeps typechecks to one per turn.Structure
checks/on aCheckcontract: its plan yields the commands to run, or fails withNothingToCheckorCannotCheck, and theuncheckcommand decides between skipped and failed. Adding a tool is one file plus a registry entry.uncheckcommand owns its flags, the check registry and the run loop;hooks runreuses them, and the entry attacheshooksas a subcommand, so no root module knows about flags. Root modules aretypes,errors,style,tool(bin lookup, argv batching, process execution) andfiles.Effect.fn, there are no casts, and the package is CLI-only: no library entry orexports, just the bin.Consolewith child processes always piped, following the Effect CLI guide; hook mode and tests capture it by swapping theConsoleservice.Repo
pnpm check/check:fix, lint-staged and CI use uncheck, andpreparestubs the bin so it always runs the current source.@typescript/typescript6for unbuild's d.ts step;engines.nodeis>=22.20 <23 || >=24.8, the releases wherepath.matchesGlobis stable, and the CI matrix drops Node 20.Testing
include/exclude/filesmatching,extendsthrough packages and${configDir},allowJs, defaults), path resolution (files, directories, globs, negations, unmatched, ignored), plus integration tests that run the real tools in temp fixtures (fix mode, references, path scoping,--require/--skip, git discovery, hook install and merge, stop-hook feedback per agent family).pnpm checkpasses on the repo; the built bin and the stop hook were exercised end to end.