Skip to content

feat(config)!: add shared top-level steps - #1255

Open
jdx wants to merge 8 commits into
feat/builtin-functionsfrom
feat/shared-top-level-steps
Open

feat(config)!: add shared top-level steps#1255
jdx wants to merge 8 commits into
feat/builtin-functionsfrom
feat/shared-top-level-steps

Conversation

@jdx

@jdx jdx commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a top-level steps map inherited by implicit check, fix, and pre-commit hooks
  • let explicit hooks override inherited steps and add enabled = false for execution/install suppression
  • preserve additive XDG/project merging and scope subproject defaults correctly
  • generate concise v2 configs from hk init and hk migrate pre-commit

Defaults

  • check: check mode, no staging
  • fix: fix mode, no staging
  • pre-commit: fix mode, staging enabled, Git stash
  • no implicit pre-push

Stack

Testing

  • cargo test --no-default-features (286 passed before the final generator assertion update; focused generator tests pass afterward)
  • mise run test:bats test/top_level_steps.bats
  • mise run test:bats test/migrate_precommit.bats (behavior passed; expectations updated for implicit pre-commit output)
  • cargo fmt
  • git diff --check

AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.


Note

High Risk
This is a breaking config and hook-default change that affects how every project’s check, fix, pre-commit, install, and monorepo merge behave.

Overview
This PR introduces hk v2-style configuration: define linters once in a top-level steps map instead of repeating the same step lists under check, fix, and pre-commit.

At load time, materialize_default_hooks builds those three implicit hooks from shared steps (check-only / fix without staging / pre-commit with fix, staging, and Git stash). Explicit hook entries override steps by name and can set enabled = false to turn off an implicit hook so hk check, hk install, and runs skip it as a successful no-op.

hk init, the default template, and hk migrate pre-commit now emit shorter configs (top-level steps; optional hooks only for disabled implicit hooks or extras like pre-push). Subproject and hkrc merging were updated so top-level steps compose correctly and disabled subproject hooks are not merged.

Docs and bats tests reflect the new model; hk builtins alone can exit early without spinning up the full runtime.

Reviewed by Cursor Bugbot for commit 3df92ae. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ba736c9-2420-42ef-a4ab-ce5103f6a7fd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds shared top-level steps inherited by the implicit check, fix, and pre-commit hooks, including explicit disabling and override behavior.

  • Materializes shared steps across project, user, and subproject configuration.
  • Updates hook execution and installation to honor enabled = false.
  • Emits concise top-level-step configurations from initialization and pre-commit migration.
  • Updates documentation and integration coverage for the new configuration model.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/config.rs Materializes implicit hooks and merges shared steps while correctly excluding disabled subproject hooks.
src/hook.rs Adds the enabled hook property with a backward-compatible true default.
src/hook_options.rs Makes disabled hook execution a successful structured no-op.
src/cli/install.rs Excludes disabled hooks and manual check/fix commands from installable Git events.
src/cli/migrate/pre_commit.rs Moves migrated pre-commit steps into the shared top-level map while retaining explicit non-default stages.
src/cli/init/generator.rs Generates concise shared-step configurations and explicit disable entries for unselected implicit hooks.
pkl/Config.pkl Extends the public configuration schema with top-level steps and hook enablement.

Reviews (20): Last reviewed commit: "perf(builtins): skip runtime for bare li..." | Re-trigger Greptile

Comment thread src/config.rs
Comment thread src/config.rs
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
builtins ████████████████▁ 1,989,871 → 1,542,592 -22.48% 2.92 → 1.78ms -38.87%
check 21.50 → 21.34ms -0.76%
usage ▆▆▆▄▄▆▂█▁▁▁▅▅▅██▄ 4,650,086 → 4,647,993 -0.05% 3.16 → 3.10ms -1.80%
validate 5.48 → 5.00ms -8.80%

No instruction-count regression above 1%.

Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.

Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.

3df92ae2edb0 vs a15fcdfb197f · measured on the runner, not pushed to the history.

jdx commented Aug 26, 2026

Copy link
Copy Markdown
Owner Author

Stack continuation: #1256 implements contextual staging, followed by #1257 for the deprecated v1 interface removal and migration guide.

AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.

@jdx
jdx force-pushed the feat/shared-top-level-steps branch 6 times, most recently from c0a86c7 to 1344eb2 Compare August 26, 2026 01:43
Comment thread src/config.rs
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from 46a2b7b to fd40f9e Compare August 26, 2026 02:22
Comment thread src/cli/install.rs
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from 466fd6a to 28123fb Compare August 26, 2026 02:51
Comment thread src/cli/install.rs
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from 28123fb to c3854e3 Compare August 26, 2026 03:10

jdx commented Aug 26, 2026

Copy link
Copy Markdown
Owner Author

Fixed in 7d713d7. Disabled subproject hooks are now skipped before root hook creation and step scoping, so they neither create an installable Git event nor contribute executable steps. Added unit coverage for the disabled-only subproject case.

AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.

Comment thread src/config.rs
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from 7d713d7 to 0520fce Compare August 26, 2026 04:49
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from 0520fce to 904b559 Compare August 26, 2026 04:53
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from 904b559 to eea92ba Compare August 26, 2026 05:09

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit eea92ba. Configure here.

Comment thread pkl/Config.pkl
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from eea92ba to e1a74c0 Compare August 26, 2026 05:26
@jdx
jdx force-pushed the feat/shared-top-level-steps branch from 7da9e05 to 3df92ae Compare August 26, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant