fix(review): accept a husky-reclaimed hooksPath when the overlay is provably gated - #351
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe change adds overlay-specific hook-path validation, accepts verified Husky-reclaimed paths, and adds doctor repair logic. Repair updates local configuration only when the hook is valid and the checkout is not a linked worktree. Tests cover valid, malformed, missing, and shared configurations. ChangesOverlay hook lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReviewSetupManifest
participant overlayHooksPathRejection
participant globalHookWired
participant HuskyRunner
ReviewSetupManifest->>overlayHooksPathRejection: validate live core.hooksPath
overlayHooksPathRejection->>globalHookWired: verify generated init.sh block
globalHookWired-->>overlayHooksPathRejection: wiring result
overlayHooksPathRejection->>HuskyRunner: validate runner and hook chain
HuskyRunner-->>overlayHooksPathRejection: chain validation result
overlayHooksPathRejection-->>ReviewSetupManifest: acceptance or rejection
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
…rovably gated In an overlay-mode repo, husky's committed `prepare` resets `core.hooksPath` to `.husky/_` on **every install**, un-wiring overlay's `.devkit/hooks` pointer. Commits stay gated — the opt-in `~/.config/husky/init.sh` shim is sourced by husky's `_/h` before the committed hook and runs the overlay's gates — but `devkit review` hard-failed on the literal: ``` core.hooksPath is ".husky/_", expected .devkit/hooks — run 'devkit doctor --fix'. ``` So a single `pnpm install` broke `devkit review` in a repo whose commits were still fully gated, and the remedy it printed could not repair `core.hooksPath` at all — `doctor` only warned. In overlay mode `review-target.sh` **hardcodes** `-c core.hooksPath=.devkit/hooks` for its private gate run and derives the chain run from `dirname "$chain"`. The captured value reaches git only in the non-overlay branch. So in overlay mode the check was never a steering input — it was an assertion that the repo is gated, and after the reclaim plus the shim, it still is. Same class as sc-1329 (`devkit ship` unconditionally required `.husky/_` and rejected a valid standalone install); the fix there was *resolve, don't hardcode*. **Widen the assertion, don't drop it.** New `cli/lib/ship/review/overlay-hooks-path.mts` accepts `.devkit/hooks` outright and `.husky/_` only when every link of the surviving chain is proven **by content**: - the devkit block in the resolved `init.sh` is complete (both markers + the line that runs the hook) - `.husky/_/h` exists and sources `husky/init.sh` - `.husky/_/pre-commit` is executable and sources `_/h` - a committed `.husky/pre-commit` exists — husky 9.1.7's `_/h:6` runs `[ ! -f "$s" ] && exit 0` **before** sourcing `init.sh`, so without it the shim never fires and commits are genuinely ungated - the overlay chain resolves through `.husky` (an overlay installed before husky records `origHooksPath: ''` and chains to `.git/hooks`, which husky's runner would never execute) Any missing link still fails, naming the first broken one and the resolved `init.sh` path so a `HOME`/`XDG_CONFIG_HOME` mismatch is distinguishable from an uninstalled shim. **The frozen value is canonicalized.** Overlay always freezes `.devkit/hooks` — the value the gate run actually uses — and validates the live value through the predicate. This matters: `setup-runtime.mts` re-compares the frozen value against the live one throughout a review, and the live value is *transient* (both `git ci` and `doctor --fix` re-point it). Freezing the reclaimed `.husky/_` would let a concurrent `git ci` in another terminal abort an in-flight review. Canonicalizing also meant `setup-manifest-parse.mts` and the `review-target.sh` field guard needed no change at all. **`devkit doctor --fix` now re-points `core.hooksPath`**, so the remedy devkit prints is real. It refuses inside a linked worktree — that config lives in the shared `.git/config` and `.devkit/hooks` is relative, so writing it there would re-point sibling worktrees at a path most of them lack. 12 new tests: the acceptance matrix (shim absent, block truncated after its start marker, no committed `.husky/pre-commit`, non-executable stub, `_/h` not sourcing `init.sh`, chain not through `.husky`), a capture→verify round-trip, a **mid-review re-point** regression, and three `doctor --fix` cases including the linked-worktree refusal. - `tsc --noEmit` and `biome check` clean - 112 passed across the six affected suites; 3048 passed across the full unit project - `guard-decisions check overlay-self-heal` exits 0 - impact analysis LOW on `effectiveHooksPath`, `verifySource`, `runOverlayDoctor` New Target on `overlay-self-heal`, citing the `devkit-owned-hook-runner-delivery` "`--fix` is file-content-only" ruling in an `--evidence-change` — a reversible git *config* write differs from the git *index* write that ruling rejected, and this change removes the review-side coupling that record cited as the reason not to re-point. `devkit ship`'s dist-integrity preflight blocked this change on `dist/cli/lib/ship/review/shared/common.mjs` — an artifact that six already-tracked dist files import but which was never force-added when `shared/common.mts` was extracted (sc-1414). It is untracked on `main` today, so the tracked dist is currently unresolvable at that path. It is included here because the preflight is fail-closed and this PR is what surfaced it; it is not part of the feature.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
cli/__tests__/review-setup-manifest.test.mts (1)
515-517: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePin one diagnostic instead of accepting either message.
The alternation
/husky's runner would never reach it|exits before sourcing the shim/passes when production emits either string. A change from one message to the other goes undetected, and this test is the one that proves the "first broken link" diagnostic for the missing committed hook. Assert the exact message thatcaptureReviewSetupemits for this link.Every other test in this suite pins a single pattern, so this is the only assertion that does not fix the diagnostic it documents.
🤖 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 `@cli/__tests__/review-setup-manifest.test.mts` around lines 515 - 517, Update the captureReviewSetup assertion to match only the exact diagnostic emitted for the missing committed hook, removing the alternation between “husky's runner would never reach it” and “exits before sourcing the shim.” Preserve the existing toThrow assertion and pin the first broken-link message documented by this test.cli/__tests__/review-setup-runtime.test.mts (1)
33-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
HUSKY_RUNNER_Hinto the shared fixture module.This constant is byte-identical to
HUSKY_RUNNER_Hincli/__tests__/review-setup-manifest.test.mtslines 73-75. Both files already importgitandwritefromcli/__tests__/review-setup-fixture.mts, so export the constant from there and import it in both tests.The decision record names husky's runner contract as the revisit trigger, so a single definition keeps both suites moving together when husky changes that shape.
🤖 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 `@cli/__tests__/review-setup-runtime.test.mts` around lines 33 - 38, Move the duplicated HUSKY_RUNNER_H definition into the shared review-setup-fixture module and export it there. Update both review-setup-runtime.test.mts and review-setup-manifest.test.mts to import and reuse that shared constant, removing their local definitions while preserving the exact byte content.docs/decisions/overlay-self-heal.md (1)
81-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the overlay doctor modules in the Scope.
The ruling controls
doctor --fixre-pointing and linked-worktree refusal, but that implementation is incli/lib/doctor/overlay-doctor.mtsandcli/lib/doctor/hook-checks.mts, not the globs listed on the Scope line. Regenerate the decision record withguard-decisionsaddingcli/lib/doctor/**; do not hand-edit the generated file.🤖 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 `@docs/decisions/overlay-self-heal.md` at line 81, Regenerate the decision record using guard-decisions so the Scope includes cli/lib/doctor/** alongside the existing paths. Do not hand-edit docs/decisions/overlay-self-heal.md; ensure the generated scope covers overlay-doctor.mts and hook-checks.mts.Source: Learnings
🤖 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 `@cli/lib/overlay-global-hook.mts`:
- Around line 68-91: Update globalHookWired() to verify the complete generated
devkit block or the exact active shell command that invokes the overlay hook,
rather than accepting any occurrence of OVERLAY_HOOK_REL between MARK_START and
MARK_END. Ensure huskyReclaimRejection only treats the hook as wired when the
generated invocation is present and executable.
In `@cli/lib/ship/review/setup/overlay-hooks-path.mts`:
- Around line 95-109: Wrap the lstatSync inspection in both readIfFile and
isExecutableFile with error handling so filesystem failures do not propagate.
Return null from readIfFile and false from isExecutableFile for any lstatSync or
subsequent inspection error, while preserving the existing file-content and
executable-bit behavior on successful checks.
In `@eslint/baselines/size-lines.json`:
- Line 5: Update the size-lines baseline entries in size-lines.json to match
current file line counts, reducing cli/commands/doctor.mts from 528 to 527 and
correcting the same one-line surplus for cli/commands/init.mts and the affected
gate-engine entries. Do not add separate baseline entries for the extracted
doctor modules; keep them covered by the applicable cap.
---
Nitpick comments:
In `@cli/__tests__/review-setup-manifest.test.mts`:
- Around line 515-517: Update the captureReviewSetup assertion to match only the
exact diagnostic emitted for the missing committed hook, removing the
alternation between “husky's runner would never reach it” and “exits before
sourcing the shim.” Preserve the existing toThrow assertion and pin the first
broken-link message documented by this test.
In `@cli/__tests__/review-setup-runtime.test.mts`:
- Around line 33-38: Move the duplicated HUSKY_RUNNER_H definition into the
shared review-setup-fixture module and export it there. Update both
review-setup-runtime.test.mts and review-setup-manifest.test.mts to import and
reuse that shared constant, removing their local definitions while preserving
the exact byte content.
In `@docs/decisions/overlay-self-heal.md`:
- Line 81: Regenerate the decision record using guard-decisions so the Scope
includes cli/lib/doctor/** alongside the existing paths. Do not hand-edit
docs/decisions/overlay-self-heal.md; ensure the generated scope covers
overlay-doctor.mts and hook-checks.mts.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ed35c02-e274-4366-88c3-9238b03dfae7
⛔ Files ignored due to path filters (3)
dist/cli/lib/doctor/overlay-doctor.mjsis excluded by!**/dist/**dist/cli/lib/ship/review/setup/overlay-hooks-path.mjsis excluded by!**/dist/**dist/cli/lib/ship/review/shared/common.mjsis excluded by!**/dist/**
📒 Files selected for processing (14)
cli/__tests__/overlay.test.mtscli/__tests__/review-setup-manifest.test.mtscli/__tests__/review-setup-runtime.test.mtscli/commands/doctor.mtscli/lib/doctor/hook-checks.mtscli/lib/doctor/overlay-doctor.mtscli/lib/overlay-global-hook.mtscli/lib/ship/review/setup-manifest.mtscli/lib/ship/review/setup-profile.mtscli/lib/ship/review/setup-runtime.mtscli/lib/ship/review/setup/overlay-hooks-path.mtsdocs/decisions/INDEX.mddocs/decisions/overlay-self-heal.mdeslint/baselines/size-lines.json
norvalbv
force-pushed
the
fix/review-husky-reclaimed-hookspath
branch
from
August 5, 2026 22:26
592e766 to
63cadb5
Compare
Addresses the review on #351. globalHookWired compared a substring (the overlay hook path) between the markers, which `# .devkit/hooks/pre-commit` satisfies while husky runs nothing. It now matches this devkit's generated BLOCK byte-for-byte — the same standard reviewHookDrift already applies to the pre-commit block, and it needs no shell parsing. A block devkit did not generate reads as NOT wired, which is fail-closed; `devkit init --overlay --global-commit-gate` restores it (installGlobalHook is strip-then-reinsert). readIfFile and isExecutableFile called lstatSync outside their try blocks. `throwIfNoEntry: false` only silences ENOENT, so an unreadable parent (EACCES) or a symlink cycle (ELOOP) escaped as an unhandled error from predicates whose job is to return a diagnostic. Both now fail closed. size-lines.json: doctor.mts recorded 426 against an actual gate count of 422. The gate counts split('\n').length (size-disable.mts:138), i.e. wc -l + 1; audited by that method, every other entry is already exact.
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
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 `@cli/lib/doctor/hook-checks.mts`:
- Around line 52-53: Update the hook-repair flow around the local core.hooksPath
configuration to query the effective value with git config --get core.hooksPath
after writing .devkit/hooks. Return true only when the effective value is
exactly .devkit/hooks; otherwise return false so doctor --fix does not report
success while an overriding worktree value remains active.
- Around line 46-52: Update the main-checkout fix path in the hook-check logic
around the linked-worktree detection and git config command to inspect existing
per-worktree or sibling-scoped core.hooksPath values before rewriting shared
configuration. If any scoped value exists, refuse the change or clearly annotate
that manual reconciliation is required; only run git config --local when no
sibling-specific value would be overwritten.
In `@docs/decisions/INDEX.md`:
- Line 42: Update the decision generator’s hook-path rendering to wrap paths in
inline-code Markdown, preventing underscore paths from being interpreted as
emphasis. Regenerate docs/decisions/INDEX.md lines 42-42 and
docs/decisions/overlay-self-heal.md lines 71-78 so both current-ruling and
Target record hook paths use the corrected rendering.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3673eba6-2bb7-480b-bdf9-83b1800b6cbf
⛔ Files ignored due to path filters (4)
dist/cli/lib/doctor/overlay-doctor.mjsis excluded by!**/dist/**dist/cli/lib/overlay-global-hook.mjsis excluded by!**/dist/**dist/cli/lib/ship/review/setup/overlay-hooks-path.mjsis excluded by!**/dist/**dist/cli/lib/ship/review/shared/common.mjsis excluded by!**/dist/**
📒 Files selected for processing (14)
cli/__tests__/overlay.test.mtscli/__tests__/review-setup-manifest.test.mtscli/__tests__/review-setup-runtime.test.mtscli/commands/doctor.mtscli/lib/doctor/hook-checks.mtscli/lib/doctor/overlay-doctor.mtscli/lib/overlay-global-hook.mtscli/lib/ship/review/setup-manifest.mtscli/lib/ship/review/setup-profile.mtscli/lib/ship/review/setup-runtime.mtscli/lib/ship/review/setup/overlay-hooks-path.mtsdocs/decisions/INDEX.mddocs/decisions/overlay-self-heal.mdeslint/baselines/size-lines.json
🚧 Files skipped from review as they are similar to previous changes (10)
- cli/lib/ship/review/setup-profile.mts
- cli/lib/ship/review/setup-runtime.mts
- cli/tests/review-setup-runtime.test.mts
- cli/lib/overlay-global-hook.mts
- cli/tests/overlay.test.mts
- cli/lib/doctor/overlay-doctor.mts
- cli/lib/ship/review/setup/overlay-hooks-path.mts
- cli/lib/ship/review/setup-manifest.mts
- cli/commands/doctor.mts
- cli/tests/review-setup-manifest.test.mts
Comment on lines
+46
to
+52
| if (git('rev-parse', '--git-dir') !== git('rev-parse', '--git-common-dir')) { | ||
| console.log( | ||
| ' · linked worktree — core.hooksPath is shared with every other worktree, so --fix leaves it alone; re-point from the main checkout: git config --local core.hooksPath .devkit/hooks', | ||
| ); | ||
| return false; | ||
| } | ||
| git('config', '--local', 'core.hooksPath', '.devkit/hooks'); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
main="$tmp/main"
sibling="$tmp/sibling"
git init -q "$main"
git -C "$main" config user.email test@example.invalid
git -C "$main" config user.name test
git -C "$main" commit --allow-empty -qm init
git -C "$main" worktree add -q "$sibling"
mkdir -p "$main/.devkit/hooks"
printf '#!/bin/sh\n' > "$main/.devkit/hooks/pre-commit"
chmod +x "$main/.devkit/hooks/pre-commit"
git -C "$main" config --local core.hooksPath .devkit/hooks
test "$(git -C "$sibling" config --get core.hooksPath)" = '.devkit/hooks'
test ! -e "$sibling/.devkit/hooks/pre-commit"Repository: norvalbv/devkit
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file =="
wc -l cli/lib/doctor/hook-checks.mts || true
echo "== outline =="
ast-grep outline cli/lib/doctor/hook-checks.mts || true
echo "== relevant lines =="
sed -n '1,180p' cli/lib/doctor/hook-checks.mts
echo "== git/worktree related searches =="
rg -n "core\.hooksPath|git-common-dir|worktree|hookOk|linked|linked worktree|git\\('config" cli/lib/doctor/hook-checks.mtsRepository: norvalbv/devkit
Length of output: 13659
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
main="$tmp/main"
sibling="$tmp/sibling"
worktree="$tmp/worktree"
git init -q "$main"
git -C "$main" config user.email test@example.invalid
git -C "$main" config user.name test
git -C "$main" commit --allow-empty -qm init
git -C "$main" worktree add -q "$sibling"
mkdir -p "$sibling/.devkit/hooks"
printf '# sibling no-op\n' > "$sibling/.devkit/hooks/pre-commit"
chmod +x "$sibling/.devkit/hooks/pre-commit"
git -C "$main" worktree add -q "$worktree"
mkdir -p "$worktree/.devkit/hooks"
printf '# worktree has no-op\n' > "$worktree/.devkit/hooks/pre-commit"
chmod +x "$worktree/.devkit/hooks/pre-commit"
git -C "$main" config --local core.hooksPath .devkit/hooks
echo "== main config =="
git -C "$main" config --local --get core.hooksPath || true
echo "== sibling git dir/common dir =="
git -C "$sibling" rev-parse --git-dir
git -C "$sibling" rev-parse --git-common-dir
echo "== sibling core.hooksPath =="
git -C "$sibling" config --get core.hooksPath || true
echo "== sibling pre-commit exists =="
test -x "$sibling/.devkit/hooks/pre-commit" && echo sibling_precommit=exists || echo sibling_precommit=missing
echo "== worktree pre-commit exists =="
test -x "$worktree/.devkit/hooks/pre-commit" && echo worktree_precommit=exists || echo worktree_precommit=missing
echo "== git common dir hook file paths =="
find "$(git -C "$main" rev-parse --git-common-dir)" -path '*/hooks/pre-commit' -maxdepth 50 2>/dev/null | sort || trueRepository: norvalbv/devkit
Length of output: 492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
main="$tmp/main"
sibling="$tmp/sibling"
worktree="$tmp/worktree"
git init -q "$main"
git -C "$main" config user.email test@example.invalid
git -C "$main" config user.name test
git -C "$main" commit --allow-empty -qm init
git -C "$main" worktree add -q "$sibling"
mkdir -p "$sibling/.devkit/hooks"
printf '# sibling has hook\n' > "$sibling/.devkit/hooks/pre-commit"
chmod +x "$sibling/.devkit/hooks/pre-commit"
git -C "$main" config --local core.hooksPath .devkit/hooks
git -C "$main" worktree add -q "$worktree"
# worktree has no hook overlay
git -C "$main" config --local --unset core.hooksPath || true
test "$(git -C "$sibling" config --get core.hooksPath 2>/dev/null || true)" = '' || true
echo "== sibling hooksPath after unlink in main =="
git -C "$sibling" config --get core.hooksPath || trueRepository: norvalbv/devkit
Length of output: 198
Preserve sibling core.hooksPath values before changing shared config.
When running from the main checkout, git config --local core.hooksPath .devkit/hooks changes the shared .git/config value, not the sibling’s per-worktree pointer. A linked sibling with its own scoped core.hooksPath would inherit the shared value from that sibling checkout if it creates one later. Detect local/sibling scoped values and refuse/annotate instead of blindly rewriting the shared config.
🤖 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 `@cli/lib/doctor/hook-checks.mts` around lines 46 - 52, Update the
main-checkout fix path in the hook-check logic around the linked-worktree
detection and git config command to inspect existing per-worktree or
sibling-scoped core.hooksPath values before rewriting shared configuration. If
any scoped value exists, refuse the change or clearly annotate that manual
reconciliation is required; only run git config --local when no sibling-specific
value would be overwritten.
Comment on lines
+52
to
+53
| git('config', '--local', 'core.hooksPath', '.devkit/hooks'); | ||
| return true; // the caller reports the healed path on its own core.hooksPath line |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
repo="$tmp/repo"
git init -q "$repo"
git -C "$repo" config extensions.worktreeConfig true
git -C "$repo" config --worktree core.hooksPath .husky/_
git -C "$repo" config --local core.hooksPath .devkit/hooks
test "$(git -C "$repo" config --get core.hooksPath)" = '.husky/_'Repository: norvalbv/devkit
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'hook-checks\.mts$' . | sed 's#^\./##'
echo
echo "== file outline =="
ast-grep outline cli/lib/doctor/hook-checks.mts --view expanded || true
echo
echo "== relevant file sections =="
nl -ba cli/lib/doctor/hook-checks.mts | sed -n '1,220p'
echo
echo "== usages of core.hooksPath and runOverlayDoctor =="
rg -n "core\.hooksPath|runOverlayDoctor|git\(['\"]config" cli/lib/doctor -SRepository: norvalbv/devkit
Length of output: 1100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== hook-checks lines 1-260 =="
awk '{printf "%6d\t%s\n", NR, $0}' cli/lib/doctor/hook-checks.mts | sed -n '1,260p'
echo
echo "== doctor files with relevant symbols =="
rg -n "core\.hooksPath|runOverlayDoctor|run.*Overlay|core\.hooks|doctor --fix|fix" cli/lib/doctor -S
echo
echo "== repo files containing doctor command =="
rg -n "doctor|overlay|hooksPath|husky" cli -S --glob '!**/*.md' | sed -n '1,220p'
echo
echo "== behavioral probe of local vs worktree hooksPath =="
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
repo="$tmp/repo"
export GIT_TERMINAL_PROMPT=0
git init -q "$repo"
git -C "$repo" config extensions.worktreeConfig true
git -C "$repo" config --worktree core.hooksPath .husky/_
git -C "$repo" config --local core.hooksPath .devkit/hooks
printf 'local config:\n'
git -C "$repo" config --local --get core.hooksPath || true
printf 'effective get:\n'
git -C "$repo" config --get core.hooksPath
test "$(git -C "$repo" config --get core.hooksPath)" = '.husky/_' && echo WORKTREE_READ_OVERRIDDENRepository: norvalbv/devkit
Length of output: 48223
Verify the effective hooks path before reporting success.
git config --local core.hooksPath only writes the shared .git/config; a worktree-scoped value can still override it with git config --get core.hooksPath. Return failure when the effective value is not .devkit/hooks so doctor --fix does not report a healed path while Git still uses .husky/_.
🤖 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 `@cli/lib/doctor/hook-checks.mts` around lines 52 - 53, Update the hook-repair
flow around the local core.hooksPath configuration to query the effective value
with git config --get core.hooksPath after writing .devkit/hooks. Return true
only when the effective value is exactly .devkit/hooks; otherwise return false
so doctor --fix does not report success while an overriding worktree value
remains active.
| | [non-devkit-asset-collision-preserve](non-devkit-asset-collision-preserve.md) | a sync treats a name as the CONSUMER's (preserve, never clobber) iff it (1) exists under a target surface, (2) is NOT recorded in devkit's prior manifest, AND (3) its on-disk bytes DIVERGE from the bundle. Default everywhere is PRESERVE. `devkit init` interactive offers a per-asset `multiselect` (keyed `${kind}:${name}`) to adopt specific collisions; `--force` (package/standalone/overlay) and the standalone `sync-skills`/`sync-agents --force` adopt all. A preserved name is left off the manifest (devkit never claims a file it didn't write). devkit's OWN copies — manifest-owned, or unmanifested-but-byte-identical to the bundle — keep overwriting, so version-bump propagation and self-dogfood are intact. `clean`'s no-manifest fallback gains the same content/tracked guard so it never deletes a preserved untracked user asset. | the sync step (syncSkills/syncAgents/syncHookScripts) hardcoded `wr… | 2026-06-30 | | ||
| | [open-ended-reviewer-gold-slots](open-ended-reviewer-gold-slots.md) | The completeness bench scores the reviewer against per-case GOLD SLOTS (gaps that must surface, each with target severity) plus DECOYS (recorded decisions / out-of-scope items it must not flag), mapped by an LLM matcher that asks one forced-choice question per slot (never one holistic list-to-list call), votes majority-of-K, and is itself audited (committed labels, Cohen's kappa >= 0.7 to be trusted) and hashed into the baseline (matcherHash) so a matcher edit invalidates comparisons exactly like a gate edit. Headline metrics are gap recall (hard floor) and decoy false-flag rate (hard ceiling); severity calibration is warn-tier; the flip gate clusters by CASE because slots within a case share one reviewer transcript. | A prompt/model edit to the 18KB feature-completeness-reviewer brief… | 2026-07-05 | | ||
| | [overlay-self-heal](overlay-self-heal.md) | `devkit upgrade` handles overlay in a self-contained branch mirroring the self-host branch. It resolves the pin install-agnostically from config.json's `devkitRef` (falling back to the running CLI version), chases a newer PUBLISHED tag exactly like package mode (`fetchLatestTag` → `update` → `needsRerun`/`NEEDS_RERUN`) — where the "install" for overlay is `bun add -g` (the global CLI is node_modules' analog) — then re-syncs via `applyInit({ overlay:true, devkitRef:'v'+target, globalCommitGate: cfg.globalCommitGate })` and runs `doctor`. `globalCommitGate` is threaded from the recorded config so a re-sync never un-wires an opted-in machine-global shim (applyOverlay reads `plan.globalCommitGate`, NOT the file). Deliberately omitted vs package upgrade: the gate-reconcile multiselect and `computeMigration` — overlay's `guard.config.json`/lint configs are `writeIfAbsent` extend-configs, and the load-bearing refresh is the regenerated `.devkit/hooks/pre-commit`. | `devkit upgrade` bailed on an overlay repo (`overlay (local-only) r… | 2026-07-14 | | ||
| | [overlay-self-heal](overlay-self-heal.md) | The hooksPath literal is an ASSERTION in overlay mode, not an input — review-target.sh hardcodes -c core.hooksPath=.devkit/hooks for its private gate run — so widen the assertion rather than drop it. Review freezes the canonical .devkit/hooks and validates the LIVE value through an acceptance predicate: .devkit/hooks outright, .husky/_ only when every link of the surviving chain is proven by content (devkit block complete in the resolved init.sh, _/h sourcing husky/init.sh, an executable _/pre-commit sourcing _/h, a committed .husky/pre-commit, and a chain resolving through .husky). devkit doctor --fix additionally re-points core.hooksPath, refusing inside a linked worktree. | The 2026-06-29 Target closed the plain-commit gap with the global h… | 2026-08-05 | |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix hook-path rendering in the decision generator.
The generated documents contain bare underscore paths that trigger MD037 and can render as emphasis. Update the renderer to emit hook paths as inline code, then regenerate both files.
docs/decisions/INDEX.md#L42-L42: render the current-ruling hook paths as code.docs/decisions/overlay-self-heal.md#L71-L78: render the Target record hook paths as code.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 42-42: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
[warning] 42-42: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
[warning] 42-42: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
📍 Affects 2 files
docs/decisions/INDEX.md#L42-L42(this comment)docs/decisions/overlay-self-heal.md#L71-L78
🤖 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 `@docs/decisions/INDEX.md` at line 42, Update the decision generator’s
hook-path rendering to wrap paths in inline-code Markdown, preventing underscore
paths from being interpreted as emphasis. Regenerate docs/decisions/INDEX.md
lines 42-42 and docs/decisions/overlay-self-heal.md lines 71-78 so both
current-ruling and Target record hook paths use the corrected rendering.
Sources: Learnings, Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In an overlay-mode repo, husky's committed
prepareresetscore.hooksPathto.husky/_on everyinstall, un-wiring overlay's
.devkit/hookspointer. Commits stay gated — the opt-in~/.config/husky/init.shshim is sourced by husky's_/hbefore the committed hook and runs theoverlay's gates — but
devkit reviewhard-failed on the literal:So a single
pnpm installbrokedevkit reviewin a repo whose commits were still fully gated, andthe remedy it printed could not repair
core.hooksPathat all —doctoronly warned.Why the literal was the wrong check
In overlay mode
review-target.shhardcodes-c core.hooksPath=.devkit/hooksfor its privategate run and derives the chain run from
dirname "$chain". The captured value reaches git only inthe non-overlay branch. So in overlay mode the check was never a steering input — it was an assertion
that the repo is gated, and after the reclaim plus the shim, it still is.
Same class as sc-1329 (
devkit shipunconditionally required.husky/_and rejected a validstandalone install); the fix there was resolve, don't hardcode.
The fix
Widen the assertion, don't drop it. New
cli/lib/ship/review/overlay-hooks-path.mtsaccepts.devkit/hooksoutright and.husky/_only when every link of the surviving chain is proven bycontent:
init.shis complete (both markers + the line that runs the hook).husky/_/hexists and sourceshusky/init.sh.husky/_/pre-commitis executable and sources_/h.husky/pre-commitexists — husky 9.1.7's_/h:6runs[ ! -f "$s" ] && exit 0before sourcing
init.sh, so without it the shim never fires and commits are genuinely ungated.husky(an overlay installed before husky recordsorigHooksPath: ''and chains to.git/hooks, which husky's runner would never execute)Any missing link still fails, naming the first broken one and the resolved
init.shpath so aHOME/XDG_CONFIG_HOMEmismatch is distinguishable from an uninstalled shim.The frozen value is canonicalized. Overlay always freezes
.devkit/hooks— the value the gate runactually uses — and validates the live value through the predicate. This matters:
setup-runtime.mtsre-compares the frozen value against the live one throughout a review, and the live value is
transient (both
git cianddoctor --fixre-point it). Freezing the reclaimed.husky/_wouldlet a concurrent
git ciin another terminal abort an in-flight review. Canonicalizing also meantsetup-manifest-parse.mtsand thereview-target.shfield guard needed no change at all.devkit doctor --fixnow re-pointscore.hooksPath, so the remedy devkit prints is real. Itrefuses inside a linked worktree — that config lives in the shared
.git/configand.devkit/hooksis relative, so writing it there would re-point sibling worktrees at a path most of them lack.
Tests
12 new tests: the acceptance matrix (shim absent, block truncated after its start marker, no committed
.husky/pre-commit, non-executable stub,_/hnot sourcinginit.sh, chain not through.husky),a capture→verify round-trip, a mid-review re-point regression, and three
doctor --fixcasesincluding the linked-worktree refusal.
Verification
tsc --noEmitandbiome checkcleanguard-decisions check overlay-self-healexits 0effectiveHooksPath,verifySource,runOverlayDoctorDecision record
New Target on
overlay-self-heal, citing thedevkit-owned-hook-runner-delivery"--fixisfile-content-only" ruling in an
--evidence-change— a reversible git config write differs from thegit index write that ruling rejected, and this change removes the review-side coupling that record
cited as the reason not to re-point.
One incidental fix
devkit ship's dist-integrity preflight blocked this change ondist/cli/lib/ship/review/shared/common.mjs— an artifact that six already-tracked dist files importbut which was never force-added when
shared/common.mtswas extracted (sc-1414). It is untracked onmaintoday, so the tracked dist is currently unresolvable at that path. It is included here becausethe preflight is fail-closed and this PR is what surfaced it; it is not part of the feature.
Summary by CodeRabbit
New Features
doctor --fixcan restore valid local hook paths while protecting linked worktrees.Bug Fixes