docs: add refactor-verification guidance and the worktree hook trap #46
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,7 @@ This is the first thing to check, because everything else here is worthless with | |
|
|
||
| Before every commit, enter a review loop (same discipline as the plan review loop). Do NOT commit after a single pass — iterate until **3 consecutive review passes find zero issues**. Do NOT skip, shortcut, or batch this step. The goal is to land clean commits in the first place, so the history doesn't need fix-up commits. | ||
|
|
||
| **Review on Opus, as comprehensively as possible — CodeRabbit's lens is the floor, not the ceiling.** This review is judgement-heavy, so run it at Opus tier (the §1c local review loop and the plan-review gate are its analogues — both Opus per `CLAUDE.md` §2); escalate to the Fable reserve only for the hardest / highest-stakes money-path diffs. The six dimensions above are the baseline; then go wider than any single reviewer would. Review as CodeRabbit would (its Actionable / Nitpick categories, the project's CR config, recurring past CR findings) AND as a demanding staff engineer would, across at least: | ||
|
|
||
| - **Architecture & design fit** — does the change belong where it landed, follow the module's patterns, and avoid leaking abstractions? | ||
| - **Type design & invariants** — are illegal states unrepresentable, invariants expressed in types rather than asserted at runtime, encapsulation intact? | ||
|
|
@@ -53,7 +53,7 @@ For multi-concern or substantial diffs, fan out the specialised review agents in | |
|
|
||
| ### Each pass | ||
|
|
||
| Read the full staged diff (`git diff --cached`) and the relevant unstaged context, and systematically check all six dimensions: | ||
|
|
||
| - **Completeness**: Does the commit deliver what it claims? Nothing missing? All touched files consistent with the commit message? Tests updated for the changed behaviour? | ||
| - **Correctness**: Any logic errors, off-by-ones, wrong assumptions, broken invariants, stale references, type mismatches, leftover debug code, unused imports? | ||
|
|
@@ -93,6 +93,8 @@ If the review finds issues, fix them in the same staged changeset — do not com | |
|
|
||
| After committing, run a quick sanity scan (`git show HEAD`) to catch anything the pre-commit loop missed. If this finds issues, treat it as a process failure (the pre-commit loop should have caught them). Fix-forward in a new commit only when strictly necessary (e.g., pre-commit hook caught a legitimate issue that required the commit to land first). | ||
|
|
||
| **Hooks may silently not run in a worktree.** When a repo sets `core.hooksPath` to a gitignored, install-generated directory (husky's `.husky/_` is the common case), that path exists only where the install ran — usually the main checkout. Git skips hooks with no warning when it doesn't resolve, so commits from a worktree can quietly bypass lint, formatting and generated-artifact rebuilds. Since §1b puts non-trivial work in worktrees, check once per worktree (`git config core.hooksPath`, then confirm the directory exists) and run the checks by hand if it doesn't. Never paper over it with `--no-verify`. | ||
|
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: root=$(git rev-parse --show-toplevel)
hooks=$(git config --get core.hooksPath || git rev-parse --git-path hooks)
case "$hooks" in /*) ;; *) hooks="$root/$hooks" ;; esac
test -x "$hooks/pre-commit"Repository: LeanerCloud/dotclaude Length of output: 159 🏁 Script executed: printf '%s\n' '--- git-workflow.md ---'
sed -n '80,105p' git-workflow.md
printf '%s\n' '--- hook-related guidance ---'
rg -n -C 3 'core\.hooksPath|pre-commit|hooks|worktree' --glob '!node_modules' .
printf '%s\n' '--- repository state ---'
git config --show-origin --get core.hooksPath || true
git rev-parse --show-toplevel
git rev-parse --git-dir
git rev-parse --git-path hooksRepository: LeanerCloud/dotclaude Length of output: 50379 🌐 Web query:
💡 Result: The behavior of core.hooksPath, particularly regarding relative paths and git worktrees, is governed by how Git resolves directory paths during execution [1][2][3]. Core.hooksPath Behavior Git allows you to override the default hooks directory ($GIT_DIR/hooks) using the core.hooksPath configuration variable [1][2][3]. When set to a relative path, Git interprets this path relative to the current working directory from which the Git command is executed, not necessarily the repository root or the.git directory [1][3]. Before executing a hook, Git changes its working directory to the root of the working tree (in non-bare repositories) or to the $GIT_DIR (in bare repositories) [2][3]. Git Worktree Considerations Git worktrees often lead to complications when using relative paths for core.hooksPath [4][5][6]. Because Git resolves relative paths based on the current working directory, a path that works in the main repository may fail in a worktree if the working directory structure differs [4][6]. For instance, if core.hooksPath is set to a relative path like.hooks, and you run a Git command from inside a worktree, Git will look for.hooks relative to that specific worktree's root, which may not contain the expected hooks directory [5][6]. To ensure consistent behavior across worktrees, it is generally recommended to use absolute paths [4][5][6]. Git rev-parse --git-path hooks The command git rev-parse --git-path hooks is designed to respect the core.hooksPath configuration [7][8]. If core.hooksPath is set, git rev-parse --git-path hooks will return the configured path rather than the default $GIT_DIR/hooks location [7][9]. This command is a reliable way to check where Git expects to find hooks for the current context [7]. You can use the --path-format=(absolute|relative) option (available since Git v2.31) to force the output to be an absolute or relative path, as desired [9][10]. Executable Hook Behavior Regardless of where the hooks are located (via default or core.hooksPath), Git will only execute files that have the executable bit set [11]. If a hook file is found at the specified path but lacks the executable permission, Git will ignore it [11]. Citations:
🏁 Script executed: git --version
git help --no-man-viewer --no-info 'githooks' 2>/dev/null | sed -n '1,180p' || true
git help --no-man-viewer --no-info 'git-config' 2>/dev/null | rg -n -C 5 'core\.hooksPath|hooksPath' || true
git rev-parse --git-path --path-format=absolute hooks 2>/dev/null || trueRepository: LeanerCloud/dotclaude Length of output: 212 🏁 Script executed: set -u
printf '%s\n' '--- effective hook paths ---'
for value in '' '.hooks' '../shared-hooks' '/tmp/absolute-hooks'; do
if [ -n "$value" ]; then
printf 'core.hooksPath=%s\n' "$value"
git -c "core.hooksPath=$value" rev-parse --git-path hooks
git -c "core.hooksPath=$value" --path-format=absolute rev-parse --git-path hooks 2>&1 || true
else
printf '%s\n' 'core.hooksPath=<unset>'
git rev-parse --git-path hooks
git --path-format=absolute rev-parse --git-path hooks 2>&1 || true
fi
done
printf '%s\n' '--- config and repository roots ---'
git config --get core.hooksPath; printf 'config_exit=%s\n' "$?"
git rev-parse --show-toplevel
git rev-parse --git-dir
git rev-parse --git-common-dirRepository: LeanerCloud/dotclaude Length of output: 2303 Check the hook file and its executable bit, not only the directory. A present directory does not prove that 🤖 Prompt for AI Agents |
||
|
|
||
| ## Rate-limit handling — always run a retry cron, never stall | ||
|
|
||
| This is a global rule (see `CLAUDE.md` Core Principles): on every request, keep a retry cron running so any throttling is caught and retried automatically rather than stalling the work. When an operation is throttled — a `429` / `403 secondary rate limit` / "rate limit" / "usage limit" / "try again later" from the GitHub API, CodeRabbit, the model/API itself, or any CLI reporting a cooldown — do NOT abandon the work and do NOT block the session busy-waiting; let the standing cron catch it and retry. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Scope the “Subtractive only” rule.
Line 50 says to introduce an abstraction at the rule of three. The PR objective says to favor subtractive edits, not to forbid every new abstraction. State that deletion and inlining are preferred for behavior-preserving cleanup, while justified abstractions remain allowed when current callers require them.
Proposed wording
🤖 Prompt for AI Agents