fix: Prevent bootstrap from overwriting source files via symlinked parent dirs - #300
Conversation
…rent dirs When a parent directory in ~ is symlinked back into the repo (e.g., ~/.config/zellij/scripts/ → dotfiles/home/.config/zellij/scripts/), the destination path resolves to the source file itself. Bootstrap would then delete the source and create a circular self-symlink. Resolve destination paths through parent symlinks and skip when they match the source. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Clean fix for a real bug where bootstrap could create circular self-symlinks when a parent directory under ~ is itself symlinked back into the repo. The approach of resolving both source and destination via pwd -P and comparing is correct.
[Important] bootstrap.sh:162 - symlink_claude_dir() still uses pwd (not pwd -P) on this line, which means it is susceptible to the same symlinked-parent-dir issue this PR fixes in symlink_dotfiles(). For consistency and correctness, this should also use pwd -P.
[Suggestion] bootstrap.sh:140 - Silent continue here means the operator gets no feedback when a file is skipped due to a symlinked parent. Consider adding a log line to aid debugging, similar to the warning on line 134.
Verdict: REQUEST_CHANGES - symlink_claude_dir() has the same pwd vs pwd -P inconsistency that this PR fixes in symlink_dotfiles().
Automated review by Claude Code
Address claude-review feedback: - symlink_claude_dir() had the same pwd vs pwd -P inconsistency - Add log line when files are skipped due to symlinked parent dirs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Feedback AddressedImplemented
|
There was a problem hiding this comment.
Code Review
Summary
Clean fix for circular self-symlink bug when a parent directory under ~ is symlinked back into the repo. The approach of resolving paths via pwd -P and comparing source to resolved destination is correct and minimal.
Previously Addressed (Filtered)
- [Important]
symlink_claude_dir()pwd -P consistency — Implemented ✓ - [Suggestion] Log line for skipped files — Implemented ✓
Verdict
APPROVE - Previous feedback addressed, no new issues found.
Automated review by Claude Code
1 participant
Summary
symlink_dotfiles()that resolves destination paths through parent symlinks and skips when the resolved path matches the source file~is symlinked back into the repo (e.g.,~/.config/zellij/scripts/→dotfiles/home/.config/zellij/scripts/)pwd -Pconsistently for both source and destination path resolutionTest plan
make checkpasses (lint, test, hooks, bootstrap — 104 tests)home/.config/zellij/scripts/sysload.shstays a regular file after bootstrap re-run🤖 Generated with Claude Code