fix(hooks): support linked worktree commits - #159
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change centralizes Git hook installation through ChangesHooks and commit flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
PR Summary by QodoSupport pre-commit hooks in linked Git worktrees
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
36 rules 1. git add errors swallowed
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
|
||
| # --- WHATS_<verb> phase SSOT (drives sub-help + error arms) ------------------- | ||
| WHATS_check := fmt lint validate audit udeps coverage qlty all | ||
| WHATS_check := fmt lint validate audit udeps coverage qlty hooks all |
There was a problem hiding this comment.
1. hooks missing from command docs 📘 Rule violation ≡ Correctness
The Makefile adds hooks to the supported make check WHAT=... options, but the repository’s static command reference still omits it. Developers relying on AGENTS.md therefore receive an incomplete list of executable check phases.
Agent Prompt
## Issue description
The new `make check WHAT=hooks` phase is absent from the static command reference.
## Issue Context
`WHATS_check` now exposes `hooks`, while `AGENTS.md` presents an explicit list of supported `WHAT` values without it.
## Fix Focus Areas
- Makefile[39-39]
- AGENTS.md[143-150]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| common_dir="$(git -C "$repo" rev-parse --path-format=absolute --git-common-dir)" \ | ||
| || mcb_die "$EX_PREREQ" "nao foi possivel resolver o diretorio Git comum de '$repo'" | ||
| printf '%s/hooks\n' "$common_dir" |
There was a problem hiding this comment.
2. Configured hook paths ignored 🐞 Bug ≡ Correctness
mcb_git_hooks_dir always installs under <git-common-dir>/hooks, so repositories with a non-default core.hooksPath report successful installation into a directory Git does not execute. This silently disables the documented pre-commit enforcement for those repositories.
Agent Prompt
## Issue description
Hook installation constructs `<git-common-dir>/hooks`, ignoring repositories that configure an alternate `core.hooksPath`. Resolve the effective hooks location through Git so the installed hook is actually executed.
## Issue Context
The installer reports success after copying to the constructed directory, while project documentation describes the hook as mechanical enforcement. Extend tests to cover a configured hook path as well as primary and linked worktrees.
## Fix Focus Areas
- scripts/lib/mcb.sh[54-69]
- scripts/lib/tests/test-hooks.sh[17-35]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| udeps) command -v cargo-udeps >/dev/null 2>&1 || cargo install cargo-udeps; cargo +nightly udeps --workspace ;; \ | ||
| coverage) cargo tarpaulin --out Lcov --output-dir coverage --exclude-files 'crates/*/tests/integration/*' --exclude-files 'crates/*/tests/admin/*' --timeout 300 ;; \ | ||
| qlty) mkdir -p docs/reports; ./scripts/analyze_qlty.py --scan --check --summary --markdown docs/reports/qlty-check-REPORTS.md; ./scripts/analyze_qlty.py --scan --smells --summary --markdown docs/reports/qlty-smells-REPORTS.md ;; \ | ||
| hooks) bash scripts/lib/tests/test-hooks.sh ;; \ |
There was a problem hiding this comment.
3. Hook regression test skips ci 🐞 Bug ☼ Reliability
The new test is reachable only through WHAT=hooks; WHAT=all, which backs make ci, does not invoke it, and the CI workflow has no separate hooks-test step. Linked-worktree hook regressions can therefore merge without this test running.
Agent Prompt
## Issue description
The linked-worktree hook regression test is exposed as a standalone check but is absent from the canonical CI execution paths. Integrate it into `WHAT=all` or add an explicit CI step/job.
## Issue Context
`make ci` delegates to `make check WHAT=all`, whose dispatch arm excludes the hooks test. The current workflow invokes no standalone `WHAT=hooks` check.
## Fix Focus Areas
- makefiles/dispatch.mk[68-80]
- Makefile[68-68]
- .github/workflows/ci.yml[116-166]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![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 `@makefiles/dispatch.mk`:
- Around line 78-79: Update the `""|all)` branch in `makefiles/dispatch.mk` to
invoke the existing hook-validation target or command used by the `hooks)`
branch, ensuring `check WHAT=all` validates linked-worktree hook installation
while preserving the current formatting, lint, test, and MCB validation steps.
In `@scripts/lib/mcb.sh`:
- Around line 54-59: Update mcb_git_hooks_dir() to use Git’s effective hooks
directory from git rev-parse --git-path hooks instead of appending hooks to the
common directory, resolving relative paths against the repository/worktree root
as needed for worktrees. Extend the hook path coverage in
scripts/lib/tests/test-hooks.sh to verify configured relative and worktree-aware
paths; both affected sites require changes.
In `@scripts/lib/tests/test-hooks.sh`:
- Around line 5-14: Update the bootstrap Git commands in the fixture setup to
use an isolated Git configuration, including the git init and initial commit
flow. Ensure host settings such as core.hooksPath cannot affect repository
initialization or trigger unrelated hooks before install-hooks runs, while
preserving the existing fixture creation behavior.
🪄 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: 49d19f7d-7fd6-47ce-84e3-c53e79d56b90
📒 Files selected for processing (5)
Makefilemakefiles/dispatch.mkscripts/hooks/pre-commitscripts/lib/mcb.shscripts/lib/tests/test-hooks.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Security Audit
- GitHub Check: Validate Documentation
- GitHub Check: Lint (Rust 2024)
- GitHub Check: Generate Diagrams
🧰 Additional context used
📓 Path-based instructions (1)
Makefile
📄 CodeRabbit inference engine (AGENTS.md)
Makefile and makefiles/*.mk define canonical developer verbs. Trust these over ad-hoc commands.
Files:
Makefile
🔇 Additional comments (5)
scripts/lib/mcb.sh (1)
148-149: LGTM!makefiles/dispatch.mk (2)
206-206: LGTM!
251-254: LGTM!scripts/hooks/pre-commit (1)
2-3: LGTM!Makefile (1)
39-39: LGTM!
| hooks) bash scripts/lib/tests/test-hooks.sh ;; \ | ||
| ""|all) cargo fmt --all -- --check && $(MAKE) lint-impl && $(MAKE) test && bash $(MCB_SH) validate $(if $(filter 1,$(QUICK)),quick,full) ;; \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include hook validation in the standard check gate.
Line 78 adds the check, but Line 79’s all branch never invokes it; therefore the CI gate (check WHAT=all) does not validate linked-worktree hook installation.
Proposed fix
- ""|all) cargo fmt --all -- --check && $(MAKE) lint-impl && $(MAKE) test && bash $(MCB_SH) validate $(if $(filter 1,$(QUICK)),quick,full) ;; \
+ ""|all) cargo fmt --all -- --check && $(MAKE) lint-impl && $(MAKE) test && bash $(MCB_SH) validate $(if $(filter 1,$(QUICK)),quick,full) && $(MAKE) check WHAT=hooks ;; \As per coding guidelines, “Makefile and makefiles/*.mk define canonical developer verbs. Trust these over ad-hoc commands.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| hooks) bash scripts/lib/tests/test-hooks.sh ;; \ | |
| ""|all) cargo fmt --all -- --check && $(MAKE) lint-impl && $(MAKE) test && bash $(MCB_SH) validate $(if $(filter 1,$(QUICK)),quick,full) ;; \ | |
| hooks) bash scripts/lib/tests/test-hooks.sh ;; \ | |
| ""|all) cargo fmt --all -- --check && $(MAKE) lint-impl && $(MAKE) test && bash $(MCB_SH) validate $(if $(filter 1,$(QUICK)),quick,full) && $(MAKE) check WHAT=hooks ;; \ |
🤖 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 `@makefiles/dispatch.mk` around lines 78 - 79, Update the `""|all)` branch in
`makefiles/dispatch.mk` to invoke the existing hook-validation target or command
used by the `hooks)` branch, ensuring `check WHAT=all` validates linked-worktree
hook installation while preserving the current formatting, lint, test, and MCB
validation steps.
Source: Coding guidelines
| mcb_git_hooks_dir() { | ||
| local repo="${1:-$MCB_ROOT}" | ||
| local common_dir | ||
| common_dir="$(git -C "$repo" rev-parse --path-format=absolute --git-common-dir)" \ | ||
| || mcb_die "$EX_PREREQ" "nao foi possivel resolver o diretorio Git comum de '$repo'" | ||
| printf '%s/hooks\n' "$common_dir" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)mcb\.sh$|(^|/)test-hooks\.sh$|scripts/hooks/pre-commit' || true
echo "== mcb outline =="
ast-grep outline scripts/lib/mcb.sh --view expanded || true
echo "== relevant mcb snippets =="
cat -n scripts/lib/mcb.sh | sed -n '1,140p'
echo "== test-hooks snippet =="
cat -n scripts/lib/tests/test-hooks.sh | sed -n '1,90p'
echo "== searches for hooksPath or hooks-dir =="
rg -n "hooks-dir|git-hooks-dir|core\.hooksPath|hooksPath|mcb_git_hooks_dir|expected_hook|LINKED|PRIMARY" scripts || true
echo "== git version =="
git --versionRepository: marlonsc/mcb
Length of output: 12398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cd "$tmp"
echo "== default setup =="
git init -q primary
git -C primary commit --allow-empty -q -m nulo
git -C primary worktree add -q -b linked-test linked
printf 'default primary: %s\n' "$(git -C primary rev-parse --path-format=absolute --git-common-dir)/hooks"
printf 'default linked: %s\n' "$(git -C linked rev-parse --path-format=absolute --git-common-dir)"
git -C linked rev-parse --path-format=absolute --git-common-dir
git -C linked rev-parse --hook-dir | sed "s|^|$tmp/linked/|"
echo "== core.hooksPath relative setup =="
git -C primary config core.hooksPath ../custom-hooks
mkdir -p primary/custom-hooks
git -C primary commit --allow-empty -q -m nulo
printf 'custom primary rev-parse hook dir: %s\n' "$(git -C primary rev-parse --path-format=absolute --git-common-dir)"
printf 'custom primary git rev-parse --hook-dir: %s\n' "$(git -C primary rev-parse --hook-dir)"
git -C linked status --short >/dev/null
printf 'custom linked git rev-parse --hook-dir from linked: %s\n' "$(git -C linked rev-parse --hook-dir)"
printf 'custom linked path-format from linked: %s\n' "$(git -C linked rev-parse --path-format=absolute --git-common-dir)"
echo "== hook-dir vs common-dir output =="
git -C primary rev-parse --path-format=absolute --git-common-dir
git -C linked rev-parse --path-format=absolute --git-common-dir
git -C primary rev-parse --hook-dir
git -C linked rev-parse --hook-dir
echo "== Git hook-path sources / rev-parse docs excerpts =="
git rev-parse --help 2>&1 | sed -n '1,/GIT/ p' | rg -n "hook-dir|hooksPath|core\.hooksPath|--hook-dir|git dir|work tree" -C 2 || trueRepository: marlonsc/mcb
Length of output: 508
🌐 Web query:
Git documentation rev-parse --hook-dir core.hooksPath relative hook path
💡 Result:
To interact with the hooks directory in Git, you can use git rev-parse --git-path hooks, which is designed to respect the core.hooksPath configuration [1][2]. Key Technical Details: 1. Resolving the Hooks Path: The command git rev-parse --git-path hooks will output the path to the directory Git currently uses for hooks [1]. If core.hooksPath is set, this command will reflect that override [1][2]. By default, it returns a path relative to the current directory [3]. To ensure you always receive an absolute path, you can use the --path-format=absolute option: git rev-parse --path-format=absolute --git-path hooks [3]. 2. Understanding core.hooksPath: The core.hooksPath configuration allows you to point to a custom directory for hooks instead of the default $GIT_DIR/hooks [4]. When you define a relative path for core.hooksPath, it is interpreted as relative to the directory where the hooks are executed [5][4]. 3. Hook Execution Context: It is important to note that when Git runs a hook, it changes its working directory [6][7]. In a non-bare repository, it switches to the root of the working tree; in a bare repository, it switches to $GIT_DIR [6][7]. A notable exception occurs for hooks triggered during a push (such as pre-receive, update, and post-receive), which are always executed within $GIT_DIR [6][7]. Because relative core.hooksPath values are relative to the execution directory, this change of context is critical if your hook scripts rely on path-relative resources [5][4]. For managing hooks centrally, setting core.hooksPath globally is a common practice [5][4]. Alternatively, you can disable hooks entirely by setting core.hooksPath to /dev/null [4][8].
Citations:
- 1: https://stackoverflow.com/questions/14073053/find-path-to-git-hooks-directory-on-the-shell
- 2: git/git@9445b49
- 3: https://stackoverflow.com/questions/44855669/git-rev-parse-git-path-hooks-always-return-absolute-path
- 4: https://github.com/git/git/blob/master/Documentation/config/core.adoc
- 5: https://stackoverflow.com/questions/21873326/how-specify-new-path-for-git-hooks-directory
- 6: https://github.com/git/git/blob/3100fd55888ce742056cc998343c5fc9f89b2c55/Documentation/githooks.txt
- 7: https://git-scm.com/docs/githooks
- 8: https://github.com/git/git/blob/e9019fca/Documentation/config/core.adoc
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
export GIT_AUTHOR_NAME="MCB Test"
export GIT_AUTHOR_EMAIL="test@example.com"
export GIT_COMMITTER_NAME="MCB Test"
export GIT_COMMITTER_EMAIL="test@example.com"
cd "$tmp"
echo "== default setup =="
git init -q primary
git -C primary commit --allow-empty -q -m nulo
git -C primary worktree add -q -b linked-test linked
printf 'default primary hook dir: "%s"\n' "$(git -C primary rev-parse --path-format=absolute --git-path hooks)"
printf 'default linked hook dir: "%s"\n' "$(git -C linked rev-parse --path-format=absolute --git-path hooks)"
echo "== core.hooksPath relative setup =="
git -C primary config core.hooksPath ../custom-hooks
rm -rf primary/custom-hooks
mkdir -p primary/custom-hooks
git -C primary commit --allow-empty -q -m nulo
printf 'custom primary git-path hooks: "%s"\n' "$(git -C primary rev-parse --path-format=absolute --git-path hooks)"
printf 'custom linked git-path hooks: "%s"\n' "$(git -C linked rev-parse --path-format=absolute --git-path hooks)"
echo "== actual hook execution directory =="
printf 'custom primary actual hook dir: "%s"\n' "$(git -C primary rev-parse --path-format=absolute --hook-dir)"
printf 'custom linked actual hook dir: "%s"\n' "$(git -C linked rev-parse --path-format=absolute --hook-dir)"
echo "== current common-dir-based hook dir from code =="
common_primary="$(git -C primary rev-parse --path-format=absolute --git-common-dir)"
common_linked="$(git -C linked rev-parse --path-format=absolute --git-common-dir)"
printf 'primary common: "%s"\n' "$common_primary/hooks"
printf 'linked common: "%s"\n' "$common_linked/hooks"
printf 'primary linked common: "%s"\n' "$common_linked/hooks"Repository: marlonsc/mcb
Length of output: 811
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
export GIT_AUTHOR_NAME="MCB Test"
export GIT_AUTHOR_EMAIL="test@example.com"
export GIT_COMMITTER_NAME="MCB Test"
export GIT_COMMITTER_EMAIL="test@example.com"
cd "$tmp"
echo "== repository setup =="
git init -q primary
git -C primary commit --allow-empty -q -m nulo
git -C primary worktree add -q -b linked-test linked
echo "== core.hooksPath custom hooks setup =="
git -C primary config core.hooksPath ../custom-hooks
mkdir -p primary/custom-hooks
git -C primary commit --allow-empty -q -m nulo
echo "== Git path helpers for primary/worktree =="
printf 'primary --git-path hooks from primary: "%s"\n' "$(git -C primary rev-parse --path-format=absolute --git-path hooks)"
printf 'linked --git-path hooks from linked: "%s"\n' "$(git -C linked rev-parse --path-format=absolute --git-path hooks)"
echo "== current common-dir-based hook dir from code =="
common_primary="$(git -C primary rev-parse --path-format=absolute --git-common-dir)"
printf 'primary common: "%s"/hooks\n' "$common_primary"
common_linked="$(git -C linked rev-parse --path-format=absolute --git-common-dir)"
printf 'linked common: "%s"/hooks\n' "$common_linked"
echo "== hook dir option support =="
git rev-parse --help | sed -n '/git rev-parse/,/GIT/ p' | rg -- '--hook-dir|hooksPath|core\.hooksPath|--git-path' -C 2 || true
printf 'primary hook-dir from primary: "%s"\n' "$(git -C primary rev-parse --path-format=absolute --hook-dir)"
printf 'linked hook-dir from linked: "%s"\n' "$(git -C linked rev-parse --path-format=absolute --hook-dir)"Repository: marlonsc/mcb
Length of output: 602
Honor Git’s configured hooks path when installing hooks.
mcb_git_hooks_dir() currently writes to $git-common-dir/hooks, but when a repo sets core.hooksPath, Git uses its configured hook directory instead. Use the repository’s effective hook directory via git rev-parse --git-path hooks (and resolve relative values from the worktree root, accounting for worktrees) so installs don’t land where commits will not execute hooks.
📍 Affects 2 files
scripts/lib/mcb.sh#L54-L59(this comment)scripts/lib/tests/test-hooks.sh#L17-L35
🤖 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 `@scripts/lib/mcb.sh` around lines 54 - 59, Update mcb_git_hooks_dir() to use
Git’s effective hooks directory from git rev-parse --git-path hooks instead of
appending hooks to the common directory, resolving relative paths against the
repository/worktree root as needed for worktrees. Extend the hook path coverage
in scripts/lib/tests/test-hooks.sh to verify configured relative and
worktree-aware paths; both affected sites require changes.
| FIXTURE="$(mktemp -d)" | ||
| trap 'rm -rf "$FIXTURE"' EXIT | ||
|
|
||
| PRIMARY="$FIXTURE/primary" | ||
| LINKED="$FIXTURE/linked" | ||
|
|
||
| git init -q "$PRIMARY" | ||
| git -C "$PRIMARY" config user.email test@example.com | ||
| git -C "$PRIMARY" config user.name "MCB Test" | ||
| git -C "$PRIMARY" commit --allow-empty -q -m initial |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '1,80p' scripts/lib/tests/test-hooks.sh 2>/dev/null || true
echo
echo "== related hook/config references =="
rg -n "hooksPath|XDG|HOME|git init|git -C|hook|hook directory" scripts/lib/mcb.sh scripts/lib/tests/test-hooks.sh scripts 2>/dev/null | head -200
echo
echo "== git version =="
git --version 2>/dev/null || true
echo
echo "== inspect mcb hook installer snippets if present =="
fd -a 'mcb\.sh|test-hooks\.sh' scripts lib 2>/dev/null | sed 's#^\./##'
for f in scripts/lib/mcb.sh scripts/lib/tests/test-hooks.sh; do
if [ -f "$f" ]; then
echo "--- $f"
wc -l "$f"
fi
doneRepository: marlonsc/mcb
Length of output: 8156
🌐 Web query:
Git hook lookup core.hooksPath documentation
💡 Result:
In Git, the core.hooksPath configuration variable allows you to specify a custom directory for Git hooks, overriding the default location [1][2][3]. How it works: By default, Git searches for hooks in the $GIT_DIR/hooks directory (typically.git/hooks in a local repository) [1][3]. When you set core.hooksPath, Git instead looks for hook scripts within the directory you have specified [1][2]. For example, if you set this to /etc/git/hooks, Git will attempt to execute /etc/git/hooks/pre-receive when the pre-receive event is triggered, rather than looking in $GIT_DIR/hooks/pre-receive [1]. Key Details: - Path Flexibility: The path can be provided as either an absolute or relative path [1][2]. If a relative path is used, it is interpreted as relative to the directory where the hooks are executed [1][2]. - Disabling Hooks: Setting core.hooksPath to /dev/null effectively disables all hooks [2]. This can be useful on a per-command basis, for example: git -c core.hooksPath=/dev/null commit [2]. - Use Case: This configuration is primarily used to manage hooks centrally across multiple repositories, rather than maintaining individual hook scripts inside each repository's.git/hooks directory [1][2]. Note on Newer Git Versions: While core.hooksPath is the traditional method for defining hook directories, newer versions of Git have introduced a more flexible hook configuration system (accessible via git config hook..*), which allows you to define multiple commands for a single hook event and control their execution order [4]. Traditional hooks (those found in the directory specified by core.hooksPath) are executed after any hooks configured via this newer system [4].
Citations:
- 1: https://git-scm.com/docs/git-config/2.31.4
- 2: https://github.com/git/git/blob/master/Documentation/config/core.adoc
- 3: https://git-scm.com/docs/githooks.html
- 4: https://git-scm.com/docs/git-hook
Isolate the fixture from host Git configuration.
The bootstrap git commit runs in the current developer/runner environment, so host configuration such as core.hooksPath can change hook lookup or execute unrelated hooks before install-hooks writes the expected fixtures. Use a temporary clean Git config, such as git -c core.hooksPath=/dev/null ... or an isolated XDG_CONFIG_HOME, for the bootstrap initialization and commit.
🤖 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 `@scripts/lib/tests/test-hooks.sh` around lines 5 - 14, Update the bootstrap
Git commands in the fixture setup to use an isolated Git configuration,
including the git init and initial commit flow. Ensure host settings such as
core.hooksPath cannot affect repository initialization or trigger unrelated
hooks before install-hooks runs, while preserving the existing fixture creation
behavior.
| show) git show --stat $(or $(REF),HEAD) ;; \ | ||
| add) bash $(MCB_SH) files-safe "$(FILES)"; $(call require_var,FILES); git add $(FILES) ;; \ | ||
| commit) $(call require_var,MSG); bash $(MCB_SH) files-safe "$(FILES)"; [ -n "$(FILES)" ] && git add $(FILES) || true; $(call gate,commit); git commit -m "$(MSG)" ;; \ | ||
| commit) $(call require_var,MSG); bash $(MCB_SH) files-safe "$(FILES)"; $(call gate,commit); [ -n "$(FILES)" ] && git add $(FILES) || true; git commit -m "$(MSG)" ;; \ |
There was a problem hiding this comment.
1. git add errors swallowed 📘 Rule violation ☼ Reliability
The && git add $(FILES) || true chain converts a failed git add into success, allowing `git commit` to proceed with incomplete or unintended staged content. This silently hides a fallible operation in the modified commit path.
Agent Prompt
## Issue description
The commit dispatch silently ignores `git add` failures because `|| true` applies to the complete `test && git add` chain.
## Issue Context
An empty `FILES` value should remain valid, but a non-empty value whose `git add` fails must stop the command before `git commit` executes. Use an explicit conditional so only the empty-value case is skipped.
## Fix Focus Areas
- makefiles/dispatch.mk[206-206]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| define DISPATCH_SETUP | ||
| @case "$(WHAT)" in \ | ||
| hooks) cp scripts/hooks/pre-commit .git/hooks/pre-commit; chmod +x .git/hooks/pre-commit; echo "✓ pre-commit hook installed" ;; \ | ||
| hooks) bash $(MCB_SH) install-hooks ;; \ |
There was a problem hiding this comment.
2. Documented install breaks worktrees 🐞 Bug ≡ Correctness
The new worktree-safe installer is used by make setup, but the operational guide still instructs users to copy into .git/hooks. Because .git is a file in linked worktrees, users following those instructions cannot install the pre-commit hook there.
Agent Prompt
## Issue description
The operational guide still installs the pre-commit hook by copying into `.git/hooks`. That path is invalid in linked worktrees, where `.git` is a file; update both documented installation examples to invoke the new worktree-aware installer, preferably `make setup WHAT=hooks`.
## Issue Context
The setup dispatch now delegates to `mcb.sh install-hooks`, which resolves the shared Git hooks directory. The linked-worktree test confirms that `.git` is a file in that environment.
## Fix Focus Areas
- docs/operations/CI_RELEASE.md[52-60]
- docs/operations/CI_RELEASE.md[654-660]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit b081ab9 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 5 files
Confidence score: 2/5
- In
scripts/lib/mcb.sh, hook-path resolution/install failures can still return success, so callers may proceed as if setup worked and, in permissive environments, a bad path can lead to copying the hook to/pre-commit— make the install path resolution fail-closed and propagate non-zero exit codes. - In
scripts/lib/mcb.sh, repos usingcore.hooksPathcan report a successful install while Git runs hooks from a different location, so the pre-commit check is silently bypassed — resolve and target the effective hooks directory viagit rev-parse --git-path hooks. - In
scripts/lib/mcb.sh,make setup WHAT=hookscurrently replaces any existing pre-commit hook, which can remove user/project automation and regress local safeguards — preserve/chain the prior hook, archive it, or require explicit overwrite confirmation. - In
makefiles/dispatch.mk, the linked-worktree regression test is not covered by default checks/CI paths, so this behavior can regress unnoticed after merge — include thehookstest path incheck WHAT=all/standard test gates.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="makefiles/dispatch.mk">
<violation number="1" location="makefiles/dispatch.mk:78">
P2: The linked-worktree regression test is not exercised by any existing default or CI gate. The new `hooks` case is only reachable via an explicit `make check WHAT=hooks`; `check WHAT=all`, `make test`, and the commands in `.github/workflows/ci.yml` never select it. Wiring this phase into an automated CI/default gate would ensure the worktree installation behavior cannot regress unnoticed.</violation>
</file>
<file name="scripts/lib/mcb.sh">
<violation number="1" location="scripts/lib/mcb.sh:56">
P1: Repositories with `core.hooksPath` configured will report a successful installation, but Git will execute hooks from the configured path and skip this pre-commit hook. Resolve the effective path with `git rev-parse --git-path hooks`, which also preserves linked-worktree handling.</violation>
<violation number="2" location="scripts/lib/mcb.sh:65">
P1: Library callers can receive success after hook-path resolution or installation fails; with sufficient permissions, an invalid repo can even copy the hook to `/pre-commit` because the failed command substitution leaves `hooks_dir` empty. Propagate each command failure explicitly instead of relying on the dispatcher's `set -e`.</violation>
<violation number="3" location="scripts/lib/mcb.sh:67">
P2: Running `make setup WHAT=hooks` destroys any existing pre-commit hook, potentially removing unrelated project checks or user automation. Preserve/chain the existing hook, archive it, or refuse replacement unless it is already the managed MCB hook.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
| local common_dir | ||
| common_dir="$(git -C "$repo" rev-parse --path-format=absolute --git-common-dir)" \ | ||
| || mcb_die "$EX_PREREQ" "nao foi possivel resolver o diretorio Git comum de '$repo'" | ||
| printf '%s/hooks\n' "$common_dir" |
There was a problem hiding this comment.
P1: Repositories with core.hooksPath configured will report a successful installation, but Git will execute hooks from the configured path and skip this pre-commit hook. Resolve the effective path with git rev-parse --git-path hooks, which also preserves linked-worktree handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/lib/mcb.sh, line 56:
<comment>Repositories with `core.hooksPath` configured will report a successful installation, but Git will execute hooks from the configured path and skip this pre-commit hook. Resolve the effective path with `git rev-parse --git-path hooks`, which also preserves linked-worktree handling.</comment>
<file context>
@@ -51,6 +51,24 @@ mcb_retry() { local n="$1" s="$2"; shift 2; local t=1; while ! "$@"; do [ "$t" -
+mcb_git_hooks_dir() {
+ local repo="${1:-$MCB_ROOT}"
+ local common_dir
+ common_dir="$(git -C "$repo" rev-parse --path-format=absolute --git-common-dir)" \
+ || mcb_die "$EX_PREREQ" "nao foi possivel resolver o diretorio Git comum de '$repo'"
</file context>
| local common_dir | |
| common_dir="$(git -C "$repo" rev-parse --path-format=absolute --git-common-dir)" \ | |
| || mcb_die "$EX_PREREQ" "nao foi possivel resolver o diretorio Git comum de '$repo'" | |
| printf '%s/hooks\n' "$common_dir" | |
| local hooks_dir | |
| hooks_dir="$(git -C "$repo" rev-parse --path-format=absolute --git-path hooks)" \ | |
| || mcb_die "$EX_PREREQ" "nao foi possivel resolver o diretorio de hooks Git de '$repo'" | |
| printf '%s\n' "$hooks_dir" |
| hooks_dir="$(mcb_git_hooks_dir "$repo")" | ||
| mkdir -p "$hooks_dir" | ||
| cp "$MCB_ROOT/scripts/hooks/pre-commit" "$hooks_dir/pre-commit" | ||
| chmod +x "$hooks_dir/pre-commit" | ||
| mcb_ok "pre-commit hook installed at $hooks_dir/pre-commit" |
There was a problem hiding this comment.
P1: Library callers can receive success after hook-path resolution or installation fails; with sufficient permissions, an invalid repo can even copy the hook to /pre-commit because the failed command substitution leaves hooks_dir empty. Propagate each command failure explicitly instead of relying on the dispatcher's set -e.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/lib/mcb.sh, line 65:
<comment>Library callers can receive success after hook-path resolution or installation fails; with sufficient permissions, an invalid repo can even copy the hook to `/pre-commit` because the failed command substitution leaves `hooks_dir` empty. Propagate each command failure explicitly instead of relying on the dispatcher's `set -e`.</comment>
<file context>
@@ -51,6 +51,24 @@ mcb_retry() { local n="$1" s="$2"; shift 2; local t=1; while ! "$@"; do [ "$t" -
+mcb_install_hooks() {
+ local repo="${1:-$MCB_ROOT}"
+ local hooks_dir
+ hooks_dir="$(mcb_git_hooks_dir "$repo")"
+ mkdir -p "$hooks_dir"
+ cp "$MCB_ROOT/scripts/hooks/pre-commit" "$hooks_dir/pre-commit"
</file context>
| hooks_dir="$(mcb_git_hooks_dir "$repo")" | |
| mkdir -p "$hooks_dir" | |
| cp "$MCB_ROOT/scripts/hooks/pre-commit" "$hooks_dir/pre-commit" | |
| chmod +x "$hooks_dir/pre-commit" | |
| mcb_ok "pre-commit hook installed at $hooks_dir/pre-commit" | |
| hooks_dir="$(mcb_git_hooks_dir "$repo")" || return $? | |
| mkdir -p "$hooks_dir" || return $? | |
| cp "$MCB_ROOT/scripts/hooks/pre-commit" "$hooks_dir/pre-commit" || return $? | |
| chmod +x "$hooks_dir/pre-commit" || return $? | |
| mcb_ok "pre-commit hook installed at $hooks_dir/pre-commit" |
| udeps) command -v cargo-udeps >/dev/null 2>&1 || cargo install cargo-udeps; cargo +nightly udeps --workspace ;; \ | ||
| coverage) cargo tarpaulin --out Lcov --output-dir coverage --exclude-files 'crates/*/tests/integration/*' --exclude-files 'crates/*/tests/admin/*' --timeout 300 ;; \ | ||
| qlty) mkdir -p docs/reports; ./scripts/analyze_qlty.py --scan --check --summary --markdown docs/reports/qlty-check-REPORTS.md; ./scripts/analyze_qlty.py --scan --smells --summary --markdown docs/reports/qlty-smells-REPORTS.md ;; \ | ||
| hooks) bash scripts/lib/tests/test-hooks.sh ;; \ |
There was a problem hiding this comment.
P2: The linked-worktree regression test is not exercised by any existing default or CI gate. The new hooks case is only reachable via an explicit make check WHAT=hooks; check WHAT=all, make test, and the commands in .github/workflows/ci.yml never select it. Wiring this phase into an automated CI/default gate would ensure the worktree installation behavior cannot regress unnoticed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At makefiles/dispatch.mk, line 78:
<comment>The linked-worktree regression test is not exercised by any existing default or CI gate. The new `hooks` case is only reachable via an explicit `make check WHAT=hooks`; `check WHAT=all`, `make test`, and the commands in `.github/workflows/ci.yml` never select it. Wiring this phase into an automated CI/default gate would ensure the worktree installation behavior cannot regress unnoticed.</comment>
<file context>
@@ -75,6 +75,7 @@ define DISPATCH_CHECK
udeps) command -v cargo-udeps >/dev/null 2>&1 || cargo install cargo-udeps; cargo +nightly udeps --workspace ;; \
coverage) cargo tarpaulin --out Lcov --output-dir coverage --exclude-files 'crates/*/tests/integration/*' --exclude-files 'crates/*/tests/admin/*' --timeout 300 ;; \
qlty) mkdir -p docs/reports; ./scripts/analyze_qlty.py --scan --check --summary --markdown docs/reports/qlty-check-REPORTS.md; ./scripts/analyze_qlty.py --scan --smells --summary --markdown docs/reports/qlty-smells-REPORTS.md ;; \
+ hooks) bash scripts/lib/tests/test-hooks.sh ;; \
""|all) cargo fmt --all -- --check && $(MAKE) lint-impl && $(MAKE) test && bash $(MCB_SH) validate $(if $(filter 1,$(QUICK)),quick,full) ;; \
*) printf "ERRO: WHAT '%s' invalido. Validos: $(WHATS_check)\n" "$(WHAT)" >&2; exit 2 ;; \
</file context>
| local hooks_dir | ||
| hooks_dir="$(mcb_git_hooks_dir "$repo")" | ||
| mkdir -p "$hooks_dir" | ||
| cp "$MCB_ROOT/scripts/hooks/pre-commit" "$hooks_dir/pre-commit" |
There was a problem hiding this comment.
P2: Running make setup WHAT=hooks destroys any existing pre-commit hook, potentially removing unrelated project checks or user automation. Preserve/chain the existing hook, archive it, or refuse replacement unless it is already the managed MCB hook.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/lib/mcb.sh, line 67:
<comment>Running `make setup WHAT=hooks` destroys any existing pre-commit hook, potentially removing unrelated project checks or user automation. Preserve/chain the existing hook, archive it, or refuse replacement unless it is already the managed MCB hook.</comment>
<file context>
@@ -51,6 +51,24 @@ mcb_retry() { local n="$1" s="$2"; shift 2; local t=1; while ! "$@"; do [ "$t" -
+ local hooks_dir
+ hooks_dir="$(mcb_git_hooks_dir "$repo")"
+ mkdir -p "$hooks_dir"
+ cp "$MCB_ROOT/scripts/hooks/pre-commit" "$hooks_dir/pre-commit"
+ chmod +x "$hooks_dir/pre-commit"
+ mcb_ok "pre-commit hook installed at $hooks_dir/pre-commit"
</file context>
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes hooks installation in linked git worktrees and adds a test to pin the behavior. All changes are in development tooling (shell scripts, Makefile) with no production or operational impact.
Re-trigger cubic
1 participant
No description provided.