From c1728b801c2936acad4d1ba1ca7298ffd7efabf2 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:08:31 +0000 Subject: [PATCH 1/2] fix(#2852): re-stage and retry when pre-commit hooks auto-fix files When a pre-commit hook auto-fixes files (e.g. gofmt, ruff format, prettier), it exits non-zero but leaves the corrected files as unstaged modifications. Previously, post-code.sh and post-fix.sh treated any non-zero exit as a hard failure, wasting the entire agent run. After a failed pre-commit run, check git diff for unstaged changes. If hooks auto-fixed files, re-stage them, amend the commit, and retry once. If the retry still fails or no unstaged changes exist, exit 1 as before. Co-Authored-By: Claude Opus 4.6 --- .../fullsend-repo/scripts/post-code-test.sh | 69 +++++++++++++++++++ .../fullsend-repo/scripts/post-code.sh | 22 ++++-- .../fullsend-repo/scripts/post-fix-test.sh | 69 +++++++++++++++++++ .../fullsend-repo/scripts/post-fix.sh | 20 +++++- 4 files changed, 174 insertions(+), 6 deletions(-) diff --git a/internal/scaffold/fullsend-repo/scripts/post-code-test.sh b/internal/scaffold/fullsend-repo/scripts/post-code-test.sh index ef1e942131..21fc75c46e 100644 --- a/internal/scaffold/fullsend-repo/scripts/post-code-test.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-code-test.sh @@ -694,6 +694,75 @@ run_signoff_test "signoff-variant-casing-passes" \ signed-off-by: bot " \ "pass" +# --------------------------------------------------------------------------- +# Test helper — reimplements the pre-commit auto-fix retry decision logic +# from post-code.sh section 5. Given a pre-commit exit code and whether +# unstaged changes exist, returns the action the script would take. +# --------------------------------------------------------------------------- +decide_precommit_retry() { + local precommit_rc="$1" # 0 = passed, 1 = failed + local has_unstaged="$2" # "yes" or "no" + local retry_precommit_rc="$3" # 0 = passed on retry, 1 = still fails (ignored if no retry) + + if [ "${precommit_rc}" -eq 0 ]; then + echo "pass:clean" + return 0 + fi + + # Pre-commit failed — check for auto-fixed files + if [ "${has_unstaged}" = "yes" ]; then + if [ "${retry_precommit_rc}" -eq 0 ]; then + echo "pass:auto-fixed" + else + echo "blocked:retry-failed" + fi + else + echo "blocked:no-auto-fix" + fi +} + +run_precommit_retry_test() { + local test_name="$1" + local precommit_rc="$2" + local has_unstaged="$3" + local retry_precommit_rc="$4" + local expected="$5" + + local actual + actual="$(decide_precommit_retry "${precommit_rc}" "${has_unstaged}" "${retry_precommit_rc}")" + + if [ "${actual}" != "${expected}" ]; then + echo "FAIL: ${test_name}" + echo " precommit_rc: '${precommit_rc}'" + echo " has_unstaged: '${has_unstaged}'" + echo " retry_precommit_rc: '${retry_precommit_rc}'" + echo " expected: '${expected}'" + echo " actual: '${actual}'" + FAILURES=$((FAILURES + 1)) + return + fi + + echo "PASS: ${test_name}" +} + +# --- Pre-commit auto-fix retry test cases --- + +# Pre-commit passes on first run → no retry needed +run_precommit_retry_test "precommit-passes-first-run" \ + "0" "no" "0" "pass:clean" + +# Pre-commit fails, hooks auto-fixed files, retry succeeds +run_precommit_retry_test "precommit-auto-fix-retry-succeeds" \ + "1" "yes" "0" "pass:auto-fixed" + +# Pre-commit fails, hooks auto-fixed files, retry still fails +run_precommit_retry_test "precommit-auto-fix-retry-fails" \ + "1" "yes" "1" "blocked:retry-failed" + +# Pre-commit fails, no unstaged changes (genuine failure) +run_precommit_retry_test "precommit-genuine-failure" \ + "1" "no" "0" "blocked:no-auto-fix" + # --- Summary --- echo "" diff --git a/internal/scaffold/fullsend-repo/scripts/post-code.sh b/internal/scaffold/fullsend-repo/scripts/post-code.sh index d48abb1d40..ece949aaaa 100755 --- a/internal/scaffold/fullsend-repo/scripts/post-code.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-code.sh @@ -309,10 +309,24 @@ if [ -f .pre-commit-config.yaml ]; then if pre-commit run --files "${changed_array[@]}"; then echo "Pre-commit passed — all hooks clean" else - echo "::error::BLOCKED — pre-commit hooks failed on agent's changes" >&2 - echo "::error::The agent's code does not pass the repo's pre-commit hooks." >&2 - echo "::error::Fix the issues and re-run, or update the pre-commit config." >&2 - exit 1 + if git diff --name-only | grep -q .; then + echo "::warning::Pre-commit hooks auto-fixed files — re-staging and retrying" + git add -u + git commit --amend --no-edit + if pre-commit run --files "${changed_array[@]}"; then + echo "Pre-commit passed after auto-fix re-stage" + else + echo "::error::BLOCKED — pre-commit hooks still fail after auto-fix" >&2 + echo "::error::The agent's code does not pass the repo's pre-commit hooks." >&2 + echo "::error::Fix the issues and re-run, or update the pre-commit config." >&2 + exit 1 + fi + else + echo "::error::BLOCKED — pre-commit hooks failed on agent's changes" >&2 + echo "::error::The agent's code does not pass the repo's pre-commit hooks." >&2 + echo "::error::Fix the issues and re-run, or update the pre-commit config." >&2 + exit 1 + fi fi else echo "::warning::pre-commit not available on runner — skipping authoritative check" diff --git a/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh b/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh index 7773b419dd..f09a3595e8 100644 --- a/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh @@ -73,6 +73,75 @@ run_push_retry_test "push-rejected" \ run_push_retry_test "push-unexpected-error" \ "1" "fatal: repository not found" "fail:unexpected-error" +# --------------------------------------------------------------------------- +# Test helper — reimplements the pre-commit auto-fix retry decision logic +# from post-fix.sh section 3. Given a pre-commit exit code and whether +# unstaged changes exist, returns the action the script would take. +# --------------------------------------------------------------------------- +decide_precommit_retry() { + local precommit_rc="$1" # 0 = passed, 1 = failed + local has_unstaged="$2" # "yes" or "no" + local retry_precommit_rc="$3" # 0 = passed on retry, 1 = still fails (ignored if no retry) + + if [ "${precommit_rc}" -eq 0 ]; then + echo "pass:clean" + return 0 + fi + + # Pre-commit failed — check for auto-fixed files + if [ "${has_unstaged}" = "yes" ]; then + if [ "${retry_precommit_rc}" -eq 0 ]; then + echo "pass:auto-fixed" + else + echo "blocked:retry-failed" + fi + else + echo "blocked:no-auto-fix" + fi +} + +run_precommit_retry_test() { + local test_name="$1" + local precommit_rc="$2" + local has_unstaged="$3" + local retry_precommit_rc="$4" + local expected="$5" + + local actual + actual="$(decide_precommit_retry "${precommit_rc}" "${has_unstaged}" "${retry_precommit_rc}")" + + if [ "${actual}" != "${expected}" ]; then + echo "FAIL: ${test_name}" + echo " precommit_rc: '${precommit_rc}'" + echo " has_unstaged: '${has_unstaged}'" + echo " retry_precommit_rc: '${retry_precommit_rc}'" + echo " expected: '${expected}'" + echo " actual: '${actual}'" + FAILURES=$((FAILURES + 1)) + return + fi + + echo "PASS: ${test_name}" +} + +# --- Pre-commit auto-fix retry test cases --- + +# Pre-commit passes on first run → no retry needed +run_precommit_retry_test "precommit-passes-first-run" \ + "0" "no" "0" "pass:clean" + +# Pre-commit fails, hooks auto-fixed files, retry succeeds +run_precommit_retry_test "precommit-auto-fix-retry-succeeds" \ + "1" "yes" "0" "pass:auto-fixed" + +# Pre-commit fails, hooks auto-fixed files, retry still fails +run_precommit_retry_test "precommit-auto-fix-retry-fails" \ + "1" "yes" "1" "blocked:retry-failed" + +# Pre-commit fails, no unstaged changes (genuine failure) +run_precommit_retry_test "precommit-genuine-failure" \ + "1" "no" "0" "blocked:no-auto-fix" + # --- Summary --- echo "" diff --git a/internal/scaffold/fullsend-repo/scripts/post-fix.sh b/internal/scaffold/fullsend-repo/scripts/post-fix.sh index 3695d4271f..5502b1d774 100644 --- a/internal/scaffold/fullsend-repo/scripts/post-fix.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-fix.sh @@ -220,8 +220,24 @@ if [ "${NO_PUSH}" = "false" ] && [ -f .pre-commit-config.yaml ]; then if pre-commit run --files "${changed_array[@]}"; then echo "Pre-commit passed — all hooks clean" else - echo "::error::BLOCKED — pre-commit hooks failed on agent's changes" >&2 - exit 1 + if git diff --name-only | grep -q .; then + echo "::warning::Pre-commit hooks auto-fixed files — re-staging and retrying" + git add -u + git commit --amend --no-edit + if pre-commit run --files "${changed_array[@]}"; then + echo "Pre-commit passed after auto-fix re-stage" + else + echo "::error::BLOCKED — pre-commit hooks still fail after auto-fix" >&2 + echo "::error::The agent's code does not pass the repo's pre-commit hooks." >&2 + echo "::error::Fix the issues and re-run, or update the pre-commit config." >&2 + exit 1 + fi + else + echo "::error::BLOCKED — pre-commit hooks failed on agent's changes" >&2 + echo "::error::The agent's code does not pass the repo's pre-commit hooks." >&2 + echo "::error::Fix the issues and re-run, or update the pre-commit config." >&2 + exit 1 + fi fi else echo "::warning::pre-commit not available — skipping authoritative check" From 186468d4d5f89d06737f6d088e14b1b44333b3a0 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Wed, 1 Jul 2026 13:30:07 -0400 Subject: [PATCH 2/2] fix(#2852): harden pre-commit auto-fix retry path Re-run gitleaks secret scan and signed-off-by check after amending the commit with auto-fixed files, closing the bypass window where hooks could inject unscanned content. Scope git-add to only hook-modified files instead of the entire tracked tree. Rebuild the changed file list from merge-base after amend so the retry pre-commit runs on the correct set. Add SYNC cross-reference comments between post-code.sh and post-fix.sh retry blocks. Assisted-by: Claude Signed-off-by: Wayne Sun --- .../fullsend-repo/scripts/post-code-test.sh | 35 ++++++++++++----- .../fullsend-repo/scripts/post-code.sh | 39 ++++++++++++++++++- .../fullsend-repo/scripts/post-fix-test.sh | 35 ++++++++++++----- .../fullsend-repo/scripts/post-fix.sh | 39 ++++++++++++++++++- 4 files changed, 124 insertions(+), 24 deletions(-) diff --git a/internal/scaffold/fullsend-repo/scripts/post-code-test.sh b/internal/scaffold/fullsend-repo/scripts/post-code-test.sh index 21fc75c46e..680470bf16 100644 --- a/internal/scaffold/fullsend-repo/scripts/post-code-test.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-code-test.sh @@ -700,9 +700,10 @@ signed-off-by: bot " \ # unstaged changes exist, returns the action the script would take. # --------------------------------------------------------------------------- decide_precommit_retry() { - local precommit_rc="$1" # 0 = passed, 1 = failed - local has_unstaged="$2" # "yes" or "no" - local retry_precommit_rc="$3" # 0 = passed on retry, 1 = still fails (ignored if no retry) + local precommit_rc="$1" # 0 = passed, 1 = failed + local has_unstaged="$2" # "yes" or "no" + local retry_precommit_rc="$3" # 0 = passed on retry, 1 = still fails (ignored if no retry) + local retry_has_unstaged="${4:-no}" # "yes" if retry left unstaged changes if [ "${precommit_rc}" -eq 0 ]; then echo "pass:clean" @@ -712,7 +713,11 @@ decide_precommit_retry() { # Pre-commit failed — check for auto-fixed files if [ "${has_unstaged}" = "yes" ]; then if [ "${retry_precommit_rc}" -eq 0 ]; then - echo "pass:auto-fixed" + if [ "${retry_has_unstaged}" = "yes" ]; then + echo "blocked:retry-left-unstaged" + else + echo "pass:auto-fixed" + fi else echo "blocked:retry-failed" fi @@ -727,17 +732,19 @@ run_precommit_retry_test() { local has_unstaged="$3" local retry_precommit_rc="$4" local expected="$5" + local retry_has_unstaged="${6:-no}" local actual - actual="$(decide_precommit_retry "${precommit_rc}" "${has_unstaged}" "${retry_precommit_rc}")" + actual="$(decide_precommit_retry "${precommit_rc}" "${has_unstaged}" "${retry_precommit_rc}" "${retry_has_unstaged}")" if [ "${actual}" != "${expected}" ]; then echo "FAIL: ${test_name}" - echo " precommit_rc: '${precommit_rc}'" - echo " has_unstaged: '${has_unstaged}'" - echo " retry_precommit_rc: '${retry_precommit_rc}'" - echo " expected: '${expected}'" - echo " actual: '${actual}'" + echo " precommit_rc: '${precommit_rc}'" + echo " has_unstaged: '${has_unstaged}'" + echo " retry_precommit_rc: '${retry_precommit_rc}'" + echo " retry_has_unstaged: '${retry_has_unstaged}'" + echo " expected: '${expected}'" + echo " actual: '${actual}'" FAILURES=$((FAILURES + 1)) return fi @@ -763,6 +770,14 @@ run_precommit_retry_test "precommit-auto-fix-retry-fails" \ run_precommit_retry_test "precommit-genuine-failure" \ "1" "no" "0" "blocked:no-auto-fix" +# Pre-commit passes but unstaged changes exist (e.g. hook wrote a log file) +run_precommit_retry_test "precommit-passes-with-unstaged" \ + "0" "yes" "0" "pass:clean" + +# Pre-commit fails, auto-fix retry passes, but retry left unstaged changes +run_precommit_retry_test "precommit-retry-passes-but-left-unstaged" \ + "1" "yes" "0" "blocked:retry-left-unstaged" "yes" + # --- Summary --- echo "" diff --git a/internal/scaffold/fullsend-repo/scripts/post-code.sh b/internal/scaffold/fullsend-repo/scripts/post-code.sh index ece949aaaa..a4f429ff91 100755 --- a/internal/scaffold/fullsend-repo/scripts/post-code.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-code.sh @@ -306,14 +306,49 @@ if [ -f .pre-commit-config.yaml ]; then if command -v pre-commit >/dev/null 2>&1; then mapfile -t changed_array <<< "${CHANGED_FILES}" + # SYNC: parallel retry block in post-fix.sh section 3 — keep structure + # in sync (variable names differ: CHANGED_FILES here vs + # BRANCH_CHANGED_FILES there; SCAN_RANGE scopes differ by design). if pre-commit run --files "${changed_array[@]}"; then echo "Pre-commit passed — all hooks clean" else - if git diff --name-only | grep -q .; then + # Single retry only — do not convert to a loop without adding a cap. + # Scope detection/staging to changed_array so hooks can't inject files + # outside the pre-commit scope into the commit. + if git diff --name-only -- "${changed_array[@]}" | grep -q .; then echo "::warning::Pre-commit hooks auto-fixed files — re-staging and retrying" - git add -u + echo "Auto-fixed files:" + git diff --name-only -- "${changed_array[@]}" | sed 's/^/ /' + git diff --name-only -z -- "${changed_array[@]}" | xargs -0 -r git add -- git commit --amend --no-edit + + echo "Re-running secret scan on amended commit..." + if ! gitleaks detect --source . --log-opts="${SCAN_RANGE}" --redact; then + echo "::error::BLOCKED — secret detected in amended commit after auto-fix" >&2 + exit 1 + fi + if git log --format='%b' "${SCAN_RANGE}" | grep -q '^Signed-off-by:'; then + echo "::error::BLOCKED — amended commit contains a Signed-off-by trailer" >&2 + exit 1 + fi + + if [ -n "${MERGE_BASE}" ]; then + CHANGED_FILES="$(git diff --name-only "${MERGE_BASE}..HEAD")" + else + CHANGED_FILES="$(git diff --name-only "origin/${TARGET_BRANCH}..HEAD" 2>/dev/null \ + || git diff --name-only HEAD~1..HEAD 2>/dev/null || true)" + fi + if [ -z "${CHANGED_FILES}" ]; then + echo "::error::BLOCKED — pre-commit hooks removed all changes; commit is now empty" >&2 + exit 1 + fi + mapfile -t changed_array <<< "${CHANGED_FILES}" if pre-commit run --files "${changed_array[@]}"; then + if git diff --name-only -- "${changed_array[@]}" | grep -q .; then + echo "::error::BLOCKED — retry pre-commit left additional unstaged changes" >&2 + echo "::error::Committed content would diverge from what pre-commit validated." >&2 + exit 1 + fi echo "Pre-commit passed after auto-fix re-stage" else echo "::error::BLOCKED — pre-commit hooks still fail after auto-fix" >&2 diff --git a/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh b/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh index f09a3595e8..687a2fa2f3 100644 --- a/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-fix-test.sh @@ -79,9 +79,10 @@ run_push_retry_test "push-unexpected-error" \ # unstaged changes exist, returns the action the script would take. # --------------------------------------------------------------------------- decide_precommit_retry() { - local precommit_rc="$1" # 0 = passed, 1 = failed - local has_unstaged="$2" # "yes" or "no" - local retry_precommit_rc="$3" # 0 = passed on retry, 1 = still fails (ignored if no retry) + local precommit_rc="$1" # 0 = passed, 1 = failed + local has_unstaged="$2" # "yes" or "no" + local retry_precommit_rc="$3" # 0 = passed on retry, 1 = still fails (ignored if no retry) + local retry_has_unstaged="${4:-no}" # "yes" if retry left unstaged changes if [ "${precommit_rc}" -eq 0 ]; then echo "pass:clean" @@ -91,7 +92,11 @@ decide_precommit_retry() { # Pre-commit failed — check for auto-fixed files if [ "${has_unstaged}" = "yes" ]; then if [ "${retry_precommit_rc}" -eq 0 ]; then - echo "pass:auto-fixed" + if [ "${retry_has_unstaged}" = "yes" ]; then + echo "blocked:retry-left-unstaged" + else + echo "pass:auto-fixed" + fi else echo "blocked:retry-failed" fi @@ -106,17 +111,19 @@ run_precommit_retry_test() { local has_unstaged="$3" local retry_precommit_rc="$4" local expected="$5" + local retry_has_unstaged="${6:-no}" local actual - actual="$(decide_precommit_retry "${precommit_rc}" "${has_unstaged}" "${retry_precommit_rc}")" + actual="$(decide_precommit_retry "${precommit_rc}" "${has_unstaged}" "${retry_precommit_rc}" "${retry_has_unstaged}")" if [ "${actual}" != "${expected}" ]; then echo "FAIL: ${test_name}" - echo " precommit_rc: '${precommit_rc}'" - echo " has_unstaged: '${has_unstaged}'" - echo " retry_precommit_rc: '${retry_precommit_rc}'" - echo " expected: '${expected}'" - echo " actual: '${actual}'" + echo " precommit_rc: '${precommit_rc}'" + echo " has_unstaged: '${has_unstaged}'" + echo " retry_precommit_rc: '${retry_precommit_rc}'" + echo " retry_has_unstaged: '${retry_has_unstaged}'" + echo " expected: '${expected}'" + echo " actual: '${actual}'" FAILURES=$((FAILURES + 1)) return fi @@ -142,6 +149,14 @@ run_precommit_retry_test "precommit-auto-fix-retry-fails" \ run_precommit_retry_test "precommit-genuine-failure" \ "1" "no" "0" "blocked:no-auto-fix" +# Pre-commit passes but unstaged changes exist (e.g. hook wrote a log file) +run_precommit_retry_test "precommit-passes-with-unstaged" \ + "0" "yes" "0" "pass:clean" + +# Pre-commit fails, auto-fix retry passes, but retry left unstaged changes +run_precommit_retry_test "precommit-retry-passes-but-left-unstaged" \ + "1" "yes" "0" "blocked:retry-left-unstaged" "yes" + # --- Summary --- echo "" diff --git a/internal/scaffold/fullsend-repo/scripts/post-fix.sh b/internal/scaffold/fullsend-repo/scripts/post-fix.sh index 5502b1d774..b0c3ee54ea 100644 --- a/internal/scaffold/fullsend-repo/scripts/post-fix.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-fix.sh @@ -216,15 +216,50 @@ if [ "${NO_PUSH}" = "false" ] && [ -f .pre-commit-config.yaml ]; then fi if command -v pre-commit >/dev/null 2>&1; then + # SYNC: parallel retry block in post-code.sh section 5 — keep structure + # in sync (variable names differ: BRANCH_CHANGED_FILES here vs + # CHANGED_FILES there; SCAN_RANGE scopes differ by design). mapfile -t changed_array <<< "${BRANCH_CHANGED_FILES}" if pre-commit run --files "${changed_array[@]}"; then echo "Pre-commit passed — all hooks clean" else - if git diff --name-only | grep -q .; then + # Single retry only — do not convert to a loop without adding a cap. + # Scope detection/staging to changed_array so hooks can't inject files + # outside the pre-commit scope into the commit. + if git diff --name-only -- "${changed_array[@]}" | grep -q .; then echo "::warning::Pre-commit hooks auto-fixed files — re-staging and retrying" - git add -u + echo "Auto-fixed files:" + git diff --name-only -- "${changed_array[@]}" | sed 's/^/ /' + git diff --name-only -z -- "${changed_array[@]}" | xargs -0 -r git add -- git commit --amend --no-edit + + echo "Re-running secret scan on amended commit..." + if ! gitleaks detect --source . --log-opts="${SCAN_RANGE}" --redact; then + echo "::error::BLOCKED — secret detected in amended commit after auto-fix" >&2 + exit 1 + fi + if git log --format='%b' "${SCAN_RANGE}" | grep -q '^Signed-off-by:'; then + echo "::error::BLOCKED — amended commit contains a Signed-off-by trailer" >&2 + exit 1 + fi + + if [ -n "${MERGE_BASE}" ]; then + BRANCH_CHANGED_FILES="$(git diff --name-only "${MERGE_BASE}..HEAD")" + else + BRANCH_CHANGED_FILES="$(git diff --name-only "origin/${TARGET_BRANCH}..HEAD" 2>/dev/null \ + || git diff --name-only HEAD~1..HEAD 2>/dev/null || true)" + fi + if [ -z "${BRANCH_CHANGED_FILES}" ]; then + echo "::error::BLOCKED — pre-commit hooks removed all changes; commit is now empty" >&2 + exit 1 + fi + mapfile -t changed_array <<< "${BRANCH_CHANGED_FILES}" if pre-commit run --files "${changed_array[@]}"; then + if git diff --name-only -- "${changed_array[@]}" | grep -q .; then + echo "::error::BLOCKED — retry pre-commit left additional unstaged changes" >&2 + echo "::error::Committed content would diverge from what pre-commit validated." >&2 + exit 1 + fi echo "Pre-commit passed after auto-fix re-stage" else echo "::error::BLOCKED — pre-commit hooks still fail after auto-fix" >&2