Remove husky pre-commit and pre-push hooks; integrate copilot pre-commit checks directly in hygiene function#308698
Conversation
…mit checks directly in hygiene function
There was a problem hiding this comment.
Pull request overview
This PR removes the Copilot extension’s Husky-based git hooks and instead triggers Copilot’s staged-file linting/formatting via the repo’s existing build/hygiene.ts pre-commit flow when Copilot files are staged.
Changes:
- Remove Husky setup (
prepare) and thehuskydevDependency fromextensions/copilot. - Delete Copilot’s Husky
pre-commit(lint-staged) andpre-push(git-lfs) hook scripts. - Add a Copilot-specific
lint-stagedinvocation insidebuild/hygiene.tswhen staged paths includeextensions/copilot/.
Show a summary per file
| File | Description |
|---|---|
| extensions/copilot/package.json | Removes Husky hook installation and the Husky dependency. |
| extensions/copilot/package-lock.json | Updates lockfile to reflect removal of Husky. |
| extensions/copilot/.husky/pre-push | Removes git-lfs pre-push guard hook. |
| extensions/copilot/.husky/pre-commit | Removes lint-staged pre-commit hook. |
| build/hygiene.ts | Runs lint-staged in extensions/copilot when Copilot files are staged. |
Copilot's findings
Files not reviewed (1)
- extensions/copilot/package-lock.json: Language not supported
- Files reviewed: 4/5 changed files
- Comments generated: 4
| // Run copilot pre-commit checks if copilot files are staged | ||
| if (some.some(f => f.startsWith('extensions/copilot/'))) { |
There was a problem hiding this comment.
lint-staged (via tsfmt/eslint) can rewrite files and update the git index. Because some is computed before running these checks, any files modified/restaged by lint-staged may be missing from the subsequent createGitIndexVinyls(some) hygiene run. Consider rerunning git diff --cached --name-only after lint-staged (or moving the lint-staged invocation before computing some).
| // Run copilot pre-commit checks if copilot files are staged | ||
| if (some.some(f => f.startsWith('extensions/copilot/'))) { | ||
| console.log('Running copilot pre-commit checks...'); | ||
| const result = cp.spawnSync('npx', ['lint-staged'], { |
There was a problem hiding this comment.
Spawning npx lint-staged without --no-install can cause npm to download packages when lint-staged isn't present locally, which is risky for offline/devbox scenarios and can introduce non-determinism. Consider invoking the local binary explicitly (or using npx --no-install) so this fails fast instead of attempting a network install.
| const result = cp.spawnSync('npx', ['lint-staged'], { | |
| const result = cp.spawnSync('npx', ['--no-install', 'lint-staged'], { |
| if (result.status !== 0) { | ||
| console.error('Copilot pre-commit checks failed'); |
There was a problem hiding this comment.
spawnSync can fail to start (e.g. missing npx on PATH), in which case result.status is null and result.error contains the reason. Right now this reports a generic failure; consider handling result.error / result.signal to surface a more actionable message for developers.
| if (result.status !== 0) { | |
| console.error('Copilot pre-commit checks failed'); | |
| if (result.error) { | |
| console.error('Copilot pre-commit checks failed to start'); | |
| console.error(result.error); | |
| process.exit(1); | |
| } | |
| if (result.signal) { | |
| console.error(`Copilot pre-commit checks terminated by signal ${result.signal}`); | |
| process.exit(1); | |
| } | |
| if (result.status !== 0) { | |
| console.error(`Copilot pre-commit checks failed with exit code ${result.status}`); |
| "scripts": { | ||
| "postinstall": "tsx ./script/postinstall.ts", | ||
| "prepare": "husky", | ||
| "vscode-dts:update": "node script/build/vscodeDtsUpdate.js", | ||
| "vscode-dts:check": "node script/build/vscodeDtsCheck.js", |
There was a problem hiding this comment.
Removing Husky here also removes the Copilot-specific pre-push git-lfs guard. Since extensions/copilot/.gitattributes enables LFS for *.sqlite, consider adding an alternative enforcement point (e.g. a repo-level pre-push check or a CI validation) so contributors without git-lfs don’t accidentally commit/push large sqlite blobs.
No description provided.