fix: remove unstaging logic from patch application
Fix critical bug in patcher.py where patches failed to apply to staged files during pre-commit hooks. **Root Cause:** The apply_patch() function was unstaging files before applying patches: 1. File gets unstaged (git reset HEAD) 2. Patch tries to apply with --index flag 3. But patch was generated from STAGED content 4. Base state mismatch causes patch application to fail 5. Original changes get re-staged, AI changes are lost **The Fix:** Remove the unstaging logic entirely (lines 599-610, 639-641). - Patches are generated from staged content (git diff --cached) - The --index flag correctly applies to both working tree and index - No need to unstage first - that changes the base state **Changes:** - Deleted 19 lines of problematic unstaging code - Added clear comment explaining why unstaging is harmful - Simplified apply_patch() function **Impact:** - Patches now apply correctly during pre-commit hooks - Status changes (OPEN → READY_FOR_DESIGN) work properly - Gate creation (design_gate_writer) will trigger correctly - No behavior change for non-staged files **Testing:** - All 18 existing tests still pass - Bundle rebuilt and verified Discovered during end-to-end testing when AI-generated status promotion patches failed with "Failed to apply patch (strict and 3-way both failed)". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
22944a2a49
commit
e84b0757f8
|
|
@ -596,49 +596,36 @@ def apply_patch(repo_root: Path, patch_file: Path, patch_level: str, output_rel:
|
||||||
"""
|
"""
|
||||||
absolute_patch = patch_file.resolve()
|
absolute_patch = patch_file.resolve()
|
||||||
|
|
||||||
# Check if the output file is currently staged
|
# Note: Patches are generated from staged content (git diff --cached).
|
||||||
# If it is, unstage it so the patch applies to the working directory version
|
# The --index flag applies patches to both working tree and index, which is
|
||||||
was_staged = False
|
# what we want. Do NOT unstage files before applying - that changes the base
|
||||||
check_staged = run(
|
# state and causes patch application to fail.
|
||||||
["git", "diff", "--cached", "--name-only", "--", output_rel.as_posix()],
|
|
||||||
cwd=repo_root,
|
|
||||||
check=False
|
|
||||||
)
|
|
||||||
if check_staged.returncode == 0 and check_staged.stdout.strip():
|
|
||||||
# File is staged, unstage it temporarily
|
|
||||||
was_staged = True
|
|
||||||
run(["git", "reset", "HEAD", "--", output_rel.as_posix()], cwd=repo_root, check=False)
|
|
||||||
|
|
||||||
try:
|
# First, try a dry-run check with --check to see if the patch applies cleanly.
|
||||||
# First, try a dry-run check with --check to see if the patch applies cleanly.
|
check_args = ["git", "apply", patch_level, "--index", "--check", absolute_patch.as_posix()]
|
||||||
check_args = ["git", "apply", patch_level, "--index", "--check", absolute_patch.as_posix()]
|
if run(check_args, cwd=repo_root, check=False).returncode == 0:
|
||||||
if run(check_args, cwd=repo_root, check=False).returncode == 0:
|
# If it applies cleanly, perform the actual apply.
|
||||||
# If it applies cleanly, perform the actual apply.
|
run(["git", "apply", patch_level, "--index", absolute_patch.as_posix()], cwd=repo_root)
|
||||||
run(["git", "apply", patch_level, "--index", absolute_patch.as_posix()], cwd=repo_root)
|
return
|
||||||
|
|
||||||
|
# If direct apply --check fails, try 3-way merge with a check first.
|
||||||
|
three_way_check_args = ["git", "apply", patch_level, "--index", "--3way", "--recount", "--whitespace=nowarn", absolute_patch.as_posix()]
|
||||||
|
if run(three_way_check_args + ["--check"], cwd=repo_root, check=False).returncode == 0:
|
||||||
|
# If 3-way check passes, perform the actual 3-way apply.
|
||||||
|
run(three_way_check_args, cwd=repo_root)
|
||||||
|
return
|
||||||
|
|
||||||
|
# Special handling for new files: if the patch creates a new file ('--- /dev/null'),
|
||||||
|
# a simple 'git apply' might work, followed by 'git add'.
|
||||||
|
text = patch_file.read_text(encoding="utf-8")
|
||||||
|
if "--- /dev/null" in text:
|
||||||
|
# Try applying without --index and then explicitly add the file.
|
||||||
|
if run(["git", "apply", patch_level, absolute_patch.as_posix()], cwd=repo_root, check=False).returncode == 0:
|
||||||
|
run(["git", "add", "--", output_rel.as_posix()], cwd=repo_root)
|
||||||
return
|
return
|
||||||
|
|
||||||
# If direct apply --check fails, try 3-way merge with a check first.
|
# If all attempts fail, raise an error.
|
||||||
three_way_check_args = ["git", "apply", patch_level, "--index", "--3way", "--recount", "--whitespace=nowarn", absolute_patch.as_posix()]
|
raise PatchGenerationError("Failed to apply patch (strict and 3-way both failed)")
|
||||||
if run(three_way_check_args + ["--check"], cwd=repo_root, check=False).returncode == 0:
|
|
||||||
# If 3-way check passes, perform the actual 3-way apply.
|
|
||||||
run(three_way_check_args, cwd=repo_root)
|
|
||||||
return
|
|
||||||
|
|
||||||
# Special handling for new files: if the patch creates a new file ('--- /dev/null'),
|
|
||||||
# a simple 'git apply' might work, followed by 'git add'.
|
|
||||||
text = patch_file.read_text(encoding="utf-8")
|
|
||||||
if "--- /dev/null" in text:
|
|
||||||
# Try applying without --index and then explicitly add the file.
|
|
||||||
if run(["git", "apply", patch_level, absolute_patch.as_posix()], cwd=repo_root, check=False).returncode == 0:
|
|
||||||
run(["git", "add", "--", output_rel.as_posix()], cwd=repo_root)
|
|
||||||
return
|
|
||||||
|
|
||||||
# If all attempts fail, raise an error.
|
|
||||||
raise PatchGenerationError("Failed to apply patch (strict and 3-way both failed)")
|
|
||||||
finally:
|
|
||||||
# If the file was staged before, re-stage it now (whether patch succeeded or failed)
|
|
||||||
if was_staged:
|
|
||||||
run(["git", "add", "--", output_rel.as_posix()], cwd=repo_root, check=False)
|
|
||||||
|
|
||||||
|
|
||||||
def run(args: list[str], cwd: Path, check: bool = True) -> subprocess.CompletedProcess[str]:
|
def run(args: list[str], cwd: Path, check: bool = True) -> subprocess.CompletedProcess[str]:
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue