-
Notifications
You must be signed in to change notification settings - Fork 12
fix(#577): disable in-repo git hooks inside sandbox #578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -261,6 +261,15 @@ the issue does not mention. | |
| echo "::notice::STEP 5: Create branch" | ||
| ``` | ||
|
|
||
| **Disable in-repo git hooks.** The sandbox is ephemeral and the post-script | ||
| runs hooks authoritatively on the runner. In-repo hooks are redundant inside | ||
| the sandbox and some (e.g., Husky with DCO) actively interfere by injecting | ||
| `Signed-off-by` trailers that the post-script rejects. Disable them: | ||
|
|
||
| ```bash | ||
| git config --global core.hooksPath /dev/null | ||
| ``` | ||
|
|
||
| If the `BRANCH_NAME` environment variable is set, use it: | ||
|
|
||
| ```bash | ||
|
|
@@ -761,11 +770,12 @@ Repeat until gitlint passes. Do not leave a commit that you know will | |
| fail CI. If gitlint is not available, manually verify that no line in | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] logic-error Step 10 states 'Git hooks are disabled in step 5 (core.hooksPath /dev/null)' as a blanket precondition, but step 4's existing-branch path ('Skip to step 9') bypasses step 5 entirely. When an agent reuses a branch, core.hooksPath is never set to /dev/null, making the step 10 assertion false for that code path. The --no-verify fallback is documented but framed as a rare edge case rather than the expected path. Suggested fix: Move 'git config --global core.hooksPath /dev/null' to step 3 (always runs), or add it to the step 4 existing-branch path before skipping to step 9. |
||
| the title or body exceeds the configured limits. | ||
|
|
||
| If a git hook fires during `git commit` and fails (e.g., the repo shipped | ||
| a `.git/hooks/pre-commit`), do NOT enter a fix-and-retry loop. You already | ||
| ran pre-commit in step 9b (which is the same check). Commit with | ||
| `--no-verify` to bypass the git hook and disclose the failure in the commit | ||
| message. The post-script runs an authoritative pre-commit on the runner. | ||
| Git hooks are disabled in step 5 (`core.hooksPath /dev/null`), so in-repo | ||
| hooks (including Husky and commitlint) should not fire during commit. If a | ||
| hook fires despite this (e.g., the repo's build process re-enabled hooks), | ||
| commit with `--no-verify` to bypass it and disclose the failure in the | ||
| commit message. The post-script runs an authoritative pre-commit on the | ||
| runner. | ||
|
|
||
| **Do not push the branch.** The post-script handles pushing, PR creation, | ||
| and failure reporting. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -193,6 +193,15 @@ Before writing any code, understand how this repository works: | |
| 1. Read `CLAUDE.md`, `CONTRIBUTING.md`, `AGENTS.md` if they exist. | ||
| 2. Discover test and lint commands from `Makefile`, `package.json`, etc. | ||
| 3. Check for linter config (`.golangci.yml`, `.pre-commit-config.yaml`, etc.). | ||
| 4. **Disable in-repo git hooks.** The sandbox is ephemeral and the | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] internal-consistency fix-review's step 8 (Commit) has no mention that hooks are disabled or a --no-verify fallback, unlike code-implementation's step 10 which was explicitly updated. The missing fallback guidance means an agent has no documented recovery if hooks fire despite the disable. Suggested fix: Add a brief note to fix-review step 8 analogous to code-implementation step 10: 'Hooks are disabled in step 3. If a hook fires despite this, commit with --no-verify.' |
||
| post-script runs hooks authoritatively on the runner. In-repo hooks | ||
| are redundant inside the sandbox and some (e.g., Husky with DCO) | ||
| actively interfere by injecting `Signed-off-by` trailers that the | ||
| post-script rejects. Disable them: | ||
|
|
||
| ```bash | ||
| git config --global core.hooksPath /dev/null | ||
| ``` | ||
|
|
||
| Determine: | ||
| - Test command (e.g., `make test`, `go test ./...`) | ||
|
|
||
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.
[low] formatting-consistency
The hook-disabling prose block is inserted between the STEP 5 progress marker and the branch creation commands, placing preparatory infrastructure before the step's primary operation.
Suggested fix: Move the hook-disabling section to appear after the branch creation commands or before the step marker.