Replace prek with hk for pre-commit hooks#157
Conversation
📝 WalkthroughWalkthroughMigrates pre-commit tooling from prek to hk: adds mise hk tasks and native runner helpers, updates mise and Renovate configs, switches CI workflows to run mise hk:base (with conditional pnpm install), simplifies CLI prepare, and aligns docs/README. ChangesPrek to hk migration: hook system replacement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
gtbuchanan
force-pushed
the
hk-evaluation-81
branch
3 times, most recently
from
ee550e0 to
7b71326
Compare
Migrate the pre-commit runner from prek to hk (jdx), configured in Pkl. File-hygiene checks become native hk builtins, dropping the Python/venv layer; actionlint, renovate-config-validator, and eslint run through mise-managed tools. Hook installation moves to a mise postinstall hook (`hk install --mise`), so `gtb prepare` handles skill syncing only. CI runs `hk check` and caches the portable, version-keyed pkl package rather than prek's path-baked hook environments. hk's file batching is tuned for Unix ARG_MAX, so a full-repo `hk check --all` can overflow cmd.exe on Windows; the everyday changed-file paths (commits, CI's --from-ref/--to-ref) are unaffected (jdx/hk discussion #971). Closes #81 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gtbuchanan
force-pushed
the
hk-evaluation-81
branch
from
7b71326 to
5934883
Compare
- Trim self-explanatory and duplicate comments in hk.pkl and mise.toml (gitlink/160000 rationale, HK_MISE, postinstall, etc.). - Drop the Prerequisites parenthetical in CONTRIBUTING that duplicated the Pre-commit section's hook-install explanation. - Move the noisy renovate-CLI bi-monthly throttle from the repo-local .github/renovate.json into the shared default.json preset, matching the sibling pre-commit-hooks throttle (clears minimumReleaseAge so the 3-day quarantine doesn't skip a schedule window). - Add a default.json customManagers regex that keeps the hk version in hk.pkl's amends/import URLs in lockstep with the jdx/hk release. - Update the AGENTS.md Renovate-preset section to match, without hard-coding the throttle count. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Sort mise.toml sections alphabetically. - Realign the Termux/Android setup with the dotfiles: Termux-native mise can't install hk/pkl/actionlint (no Android aqua assets), so they're installed out of band and disabled in mise. - Trim redundant CONTRIBUTING prose (mise.lock checksum detail, the postinstall parenthetical, the hk-migrate/issue-81 paragraph). - Share the eslint check/fix args via a single eslintArgs in hk.pkl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mise runs file tasks through Git Bash, which leaves the mise-injected tool dirs in POSIX /c/ form. The native hk binary then can't find pkl to load hk.pkl, and its cmd.exe steps can't find System32 tools like findstr. (The commit-time git hook is unaffected: it runs hk via mise x from native git.exe, so hk gets a Windows-format PATH.) Add a generic mise-tasks/lib.sh run_native helper that resolves the binary on the POSIX PATH, then execs it with a cygpath-converted Windows PATH; it no-ops off Windows. Source it from hk:base/hk:all, forwarding all hk args, and switch their shebangs to bash so mise uses real Git Bash. Document lib.sh and hk/all in the structure tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 248-252: Update the documentation to correctly describe how
pre-commit triggers hk: instead of saying it runs "hk check
--from-ref/--to-ref", state that pre-commit.yml invokes the mise-tasks/hk/base
script which computes changed files via "git diff --name-only" (as noted in the
comment around mise-tasks/hk/base lines that mention "We diff ourselves") and
then runs "hk check $files" with that explicit file list; reference
pre-commit.yml and mise-tasks/hk/base and the hk check $files invocation so
readers know the repo diffs are computed locally rather than using hk's
--from-ref/--to-ref flags.
In `@default.json`:
- Around line 3-19: The extractVersionTemplate currently requires a leading "v"
which fails to match the currentValue captured by matchStrings (which yields
values like "1.46.0"); update extractVersionTemplate to accept an optional
leading "v" (so it matches both "v1.2.3" and "1.2.3") or alternatively change
the matchStrings entries to include the "v" in the captured currentValue; ensure
you modify the extractVersionTemplate and/or matchStrings so that the captured
group name (currentValue) can be normalized by extractVersionTemplate.
In `@mise-tasks/hk/base`:
- Line 33: The current invocation run_native hk "$cmd" "$@" $files suffers
word-splitting on $files and will break filenames with spaces; change the code
to pass files as a proper array and expand it with "${files[@]}" (or, if you
cannot convert callers, ensure files is a single quoted value and split on
newlines only), so update the call site (the run_native invocation) and any code
that constructs files to build a shell array (e.g., files+=(...)) and then call
run_native hk "$cmd" "$@" "${files[@]}".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c9589761-a8a1-4829-8be4-2289560a195d
⛔ Files ignored due to path filters (2)
hk.pklis excluded by!**/*.pklmise.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/pr.yml.github/workflows/pre-commit-seed.yml.github/workflows/pre-commit.yml.github/workflows/release.yml.pre-commit-config.yamlAGENTS.mdCONTRIBUTING.mdREADME.mddefault.jsonmise-tasks/hk/allmise-tasks/hk/basemise-tasks/lib.shmise.tomlpackages/cli/README.mdpackages/cli/src/commands/root/prepare.tspackages/cli/test/prepare.test.tspackages/eslint-config/skills/gtb-eslint-config/SKILL.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
gtbuchanan/tooling(manual)
💤 Files with no reviewable changes (2)
- .pre-commit-config.yaml
- .github/workflows/pre-commit-seed.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
For GitHub Actions workflow files, reference in-repo composite actions by full path
gtbuchanan/tooling/.github/actions/<name>@main`` instead of relative paths./Use GitHub Actions composite action
mise-setupas the first step of every job that needs mise tools to restore the mise data dir with arch-specific fallbackUse
turbo-runcomposite action to run turbo tasks, which skipspnpm installwhen turbo remote cache covers all tasksUse
pnpm-taskscomposite action to restore the pnpm store from cache, install dependencies, and run optional pnpm commandsUse
pnpm-resolve-pinnedcomposite action to resolve a package's exact version from the lockfile without install forpnpm dlxinvocations
Files:
.github/workflows/pr.yml.github/workflows/pre-commit.yml.github/workflows/release.yml
mise.toml
📄 CodeRabbit inference engine (AGENTS.md)
Set
idiomatic_version_file_enable_tools = ['pnpm']in mise configuration to share the pnpm version betweenpackageManagerinpackage.jsonand mise
Files:
mise.toml
README.md
📄 CodeRabbit inference engine (AGENTS.md)
When adding or removing a package, update the packages table in
README.mdand the structure tree
Files:
README.md
**/test/**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
test.slowtag or/**@module-tagslow */JSDoc for slow tests in Vitest, controlled via--tags-filterCLI optionWhen asserting on
CommandResult(exit code, stdout, stderr), useexpect(result).toMatchObject({ exitCode: 0 })instead ofexpect(result.exitCode).toBe(0)Generate incidental test data via
@gtbuchanan/test-utils/buildersor@faker-js/fakerdirectly for one-off primitives, capturing result in a local variable for assertionsUse
import * as build from '@gtbuchanan/test-utils/builders'and reference captured values in assertions instead of duplicate literals
Files:
packages/cli/test/prepare.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T18:45:16.930Z
Learning: Set `MISE_LOCKED=1` environment variable so `mise install` fails on `mise.toml` ↔ `mise.lock` drift
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T18:45:16.930Z
Learning: Prefer per-package Vitest configuration with `configurePackage()` over global configuration for most packages
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T18:45:16.930Z
Learning: Maintain separate ESLint plugin set bundled in `gtbuchanan/eslint-config` with two-plugin Markdown lint split for different file types
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T18:45:16.930Z
Learning: On Termux/Android, run `pnpm build`, `pnpm test:slow`, and `pnpm test:e2e` with `--concurrency=1` to avoid OOM
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T18:45:16.930Z
Learning: Keep reusable workflows (`workflow_call`-only) separate from pipeline workflows with direct triggers to ensure `inputs` context always populates
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T18:45:16.930Z
Learning: Define leaf job names in branch protection as unique and descriptive (e.g., 'Build', 'E2E Test', 'Pre-Commit Run', 'Dependency Review') rather than workflow or calling job names
📚 Learning: 2026-06-04T18:38:29.949Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 141
File: .github/workflows/changeset-check.yml:15-15
Timestamp: 2026-06-04T18:38:29.949Z
Learning: In this repository (gtbuchanan/tooling), GitHub Actions used in workflows are intentionally referenced by moving tags/branches (e.g., `actions/checkoutv*`, `actions/upload-artifactv*`, `actions/cachev*`, `actions/create-github-app-tokenv*`, and other third-party actions like `rharkor/caching-for-turbov*`), and should never be pinned to a commit SHA. There is no repo-wide SHA-pinning policy; therefore, do not flag these as “unpinned” or suggest SHA-pinning (zizmor’s default `unpinned-uses` rule does not apply here). For in-repo composite actions referenced by reusable workflows, the deliberate documented convention is to use `main` (e.g., `gtbuchanan/tooling/.github/actions/mise-setupmain`), matching how consumers reference the reusable workflows themselves (`.../.github/workflows/<name>main`). Do not change `main` references to SHAs to avoid hash bumps and the self-reference paradox.
Applied to files:
.github/workflows/pr.yml.github/workflows/pre-commit.yml.github/workflows/release.yml
📚 Learning: 2026-06-04T02:26:02.824Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 139
File: packages/cli/test/coverage-codecov-upload.test.ts:2-2
Timestamp: 2026-06-04T02:26:02.824Z
Learning: In tests within **/{test,e2e,__tests__}/**/*.{test,spec}.{ts,tsx,js,jsx}, follow the AGENTS.md faker convention: direct `faker-js/faker` usage is allowed only for one-off primitives when the produced value’s shape is exactly what faker returns (e.g., `faker.git.commitSha()`, `faker.string.uuid()`). Use `gtbuchanan/test-utils/builders` only when there is a domain-shaped value worth centralizing (e.g., scoped package names, semver ranges, GitHub URLs). Do not wrap a native faker generator in a builder for a plain primitive (e.g., a raw commit SHA), since that adds indirection without centralizing any domain shape.
Applied to files:
packages/cli/test/prepare.test.ts
🪛 Shellcheck (0.11.0)
mise-tasks/lib.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🪛 zizmor (1.25.2)
.github/workflows/pre-commit.yml
[error] 22-22: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔇 Additional comments (36)
.github/workflows/pre-commit.yml (5)
2-6: LGTM!
16-16: LGTM!
18-22: LGTM!
24-27: LGTM!
34-39: LGTM!.github/workflows/pr.yml (1)
22-23: LGTM!.github/workflows/release.yml (1)
19-22: LGTM!AGENTS.md (6)
94-115: LGTM!
118-128: LGTM!
215-215: LGTM!
246-252: LGTM!
299-310: LGTM!
331-331: LGTM!CONTRIBUTING.md (3)
5-10: LGTM!
65-79: LGTM!
91-124: LGTM!README.md (3)
11-17: LGTM!
81-81: LGTM!
153-155: LGTM!packages/eslint-config/skills/gtb-eslint-config/SKILL.md (1)
56-64: LGTM!mise.toml (4)
1-4: LGTM!
6-7: LGTM!
10-11: LGTM!
34-39: Validate pinned tool versions inmise.toml
actionlintv1.7.12,hkv1.46.0,pklv0.31.1: release tags exist; the GitHub security advisory query returned no vulnerabilities.npm:renovate43.213.1: npm version exists, butnpm auditcouldn’t run here (no lockfile), so vulnerability status remains inconclusive.node24.16.0: not checked—also validate existence and security advisories.default.json (1)
59-71: LGTM!mise-tasks/lib.sh (1)
1-17: LGTM!mise-tasks/hk/all (1)
1-16: LGTM!mise-tasks/hk/base (1)
1-32: LGTM!packages/cli/src/commands/root/prepare.ts (2)
12-20: LGTM!
23-23: LGTM!packages/cli/test/prepare.test.ts (4)
4-7: LGTM!Also applies to: 19-19
35-43: LGTM!
45-54: LGTM!
56-61: LGTM!packages/cli/README.md (2)
60-61: LGTM!
91-91: LGTM!
- Correct the pre-commit.yml description in AGENTS.md: it runs the hk:base task, not hk's --from-ref/--to-ref. - Accept an optional leading `v` in default.json's hk extractVersionTemplate so it normalizes the tag either way. - Split the hk:base file list on newlines only (IFS + set -f) so paths with spaces or glob chars survive instead of word-splitting. - Add a shellcheck shell=bash directive to mise-tasks/lib.sh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
AGENTS.md (1)
289-289: 🧹 Nitpick | 🔵 TrivialClarify
pre-commit.enabledrationale in the shareable preset docs.
default.jsonsets"pre-commit": { "enabled": true }, andAGENTS.mddescribes it as enabling Renovate to bump.pre-commit-config.yamlhooks; consider explicitly noting this is primarily for consumer repos that still include.pre-commit-config.yaml(rather than implying it’s a no-op for this repo due to hk migration).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` at line 289, Update the AGENTS.md shareable preset documentation to explicitly explain why default.json sets "pre-commit": { "enabled": true }: state that this flag enables Renovate to bump .pre-commit-config.yaml hooks for consumer repositories that still use pre-commit, and clarify that in this repo the setting is effectively a no-op due to the hk migration; reference the "pre-commit.enabled" key and default.json preset so readers understand the intended target (consumer repos) versus this repo's migration status.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@AGENTS.md`:
- Line 289: Update the AGENTS.md shareable preset documentation to explicitly
explain why default.json sets "pre-commit": { "enabled": true }: state that this
flag enables Renovate to bump .pre-commit-config.yaml hooks for consumer
repositories that still use pre-commit, and clarify that in this repo the
setting is effectively a no-op due to the hk migration; reference the
"pre-commit.enabled" key and default.json preset so readers understand the
intended target (consumer repos) versus this repo's migration status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 96d27b4a-77a9-4623-8821-39aedae731f2
📒 Files selected for processing (4)
AGENTS.mddefault.jsonmise-tasks/hk/basemise-tasks/lib.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
gtbuchanan/tooling(manual)
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CI / Codecov Upload
- GitHub Check: CI / E2E Test
🧰 Additional context used
📓 Path-based instructions (2)
default.json
📄 CodeRabbit inference engine (AGENTS.md)
Renovate preset (
default.json) must override these defaults:labels: ['dependencies'],minimumReleaseAge: '3 days',osvVulnerabilityAlerts: true,platformCommit: 'enabled',postUpdateOptions: ['pnpmDedupe'],pre-commit.enabled: true,rebaseWhen: 'conflicted',semanticCommits: 'disabled',timezone: 'America/Chicago'Renovate preset must include
packageRulesthrottles for fast-moving deps (renovatebot/pre-commit-hooks, renovate CLI) at bi-monthly schedule (1,15) with clearedminimumReleaseAgeRenovate preset must include a
customManagersregex entry to keep hk version inhk.pklpackage URLs in sync with the hk release tracked inmise.toml
Files:
default.json
**/*.{ts,tsx,js,jsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
ESLint suppressions must include a
--reason suffix explaining why the rule is suppressed
Files:
default.jsonAGENTS.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T19:55:04.979Z
Learning: Use the Repository pattern for data access
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T19:55:04.979Z
Learning: Use `pnpm build`, `pnpm test:slow`, and `pnpm test:e2e` with `--concurrency=1` on Termux to avoid OOM due to workspace resolution constraints
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T19:55:04.979Z
Learning: Source tests live in `test/` directory (unit + integration with coverage), while artifact tests live in `e2e/` directory (no source coverage, requires tarballs from pack)
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-06-09T19:55:04.979Z
Learning: Turbo cache invalidation occurs on any source changes in `packages/*` workspace packages due to root devDeps transitive reach; root-only edits (docs, `.github/`) still hit cache
🔇 Additional comments (8)
AGENTS.md (2)
248-252: Documentation improved but could be more precise.The text now correctly states that
pre-commit.ymlruns thehk:basemise task rather than incorrectly claiminghk check --from-ref/--to-ref. However, the past review comment requested more detail about the mechanism—thatmise-tasks/hk/basecomputes changed files viagit diff --name-onlyand passes them tohk check $files. The current phrasing "on PR changed files" is accurate but leaves the implementation details implicit.
11-17: LGTM!Also applies to: 94-115, 307-310
default.json (3)
9-9: LGTM!
59-71: LGTM!
3-8: LGTM!Also applies to: 10-18, 20-58, 72-89
mise-tasks/hk/base (2)
34-39: LGTM!
1-32: LGTM!mise-tasks/lib.sh (1)
1-18: LGTM!
1 participant
Migrates the pre-commit runner from prek to hk (jdx), configured in Pkl (
hk.pkl).What changed
hk.pkl(new) — file-hygiene checks via nativeBuiltins.*(no Python/venv layer),actionlint, tworenovate-config-validator --strictsteps, a per-OSforbid-submodulesguard, and apnpm exec eslintstep (non-modifyingcheckfor CI, autofixingfixon commit).check-illegal-windows-nameswas dropped (no builtin; redundant on a Windows-primary repo)..pre-commit-config.yamlremoved.prek; addedhk/pkl/actionlint(aqua, realmise.lockintegrity) +npm:renovate;HK_MISE=1;[hooks] postinstall = "hk install --mise";experimental = true(required for[hooks]).gtb prepare— now skill-sync only; hook installation moved to mise''spostinstall. Command + tests updated.pre-commit.ymlrunshk check --from-ref/--to-refon changed files. hk has no hook environments to build or cache, so the seed workflow is gone and the release pipeline is just CI + CD.npm:renovatetool.Windows notes
Two Windows-specific wrinkles, both handled inside the
hk:*mise tasks:cmd.exearg limit. hk passes matched files as argv and its auto-batching is Unix-ARG_MAX-tuned, so a full-repo run can overflowcmd.exe''s 8191-char limit. Thehk:alltask setsHK_BATCH=1, which flips thebatch = batchFilessteps inhk.pklon so hk chunks files (~one batch per core) under the limit. Commits andhk:baseleave it unset so the common path stays single-invocation (batching a small changeset just means repeated linter startup). Filed upstream: jdx/hk discussion #971./c/…form; native hk then can''t findpklto loadhk.pkl, and its cmd.exe steps can''t find System32 tools likefindstr.mise-tasks/lib.sh''srun_nativeresolves the binary on the POSIX PATH, then runs it with a cygpath-converted Windows PATH (a no-op off Windows). The commit-time git hook is unaffected — it runs hk viamise xfrom native git.exe.Verification
hk validatepasses; the hk pre-commit hook runs green (one invocation per step).pnpm checkgreen — typecheck + lint + fast tests (215 cli tests).Closes #81