Repository navigation
Conversation
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
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 |
A chmod that came with a content change passed validation but the worker only wrote file content, so the executable bit never reached the sub-PR. Read new mode / new file mode from the diff header and chmod the materialized file.
9d7fe0f to
fe6a533
Compare
…file-modes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
|
This change is already in Generated by Claude Code |
Problem
A
chmod +x(or-x) that arrives together with a content change passes validation, but the worker only ever wrote file content:materialize_group_filesreturns text and the worktree write iswrite_text. The mode bit never reached the sub-PR branch —git ls-treeon the sub-PR showed100644 run.shwhere the feature branch had100755. No error anywhere. (Hunk-less mode-only entries are rejected up front by #79; this handles the common case where the mode change comes with edits.)Fix
target_file_modes(parsed_diff, group)readsnew mode NNNNNN/new file mode NNNNNNfrom each assigned file's diff header (unidiff keeps them inpatch_info) and returns the mode for regular files only (mode & 0o170000 == 0o100000), so a symlink's120000is never masked to 000.mode & 0o777after writing it.Verified end to end in a real repo:
+xwith content,-xwith content, and a new executable file all land with the right mode in the committed sub-PR branch; stacked children and merge nodes (which re-materialise from the merge base) get it too; a symlink in the same group keeps its pre-existing (plain-file) behaviour instead of aborting the group.Tests
target_file_modes: mode+content, new executable,-x, unassigned/plain files ignored, deleted files ignored, symlink ignored. Worker: executable bit applied only to the file the diff marks.Local review: 5/5. 451 tests pass, ruff clean.
Follow-up: symlinks are still materialised as regular files (pre-existing); separate PR.