fix(filemerge): preserve symlinked config files

[fjpalacios]

Linked Issue

Closes #306

PR Type

• Bug fix
• New feature
• Documentation only
• Code refactoring
• Maintenance/tooling
• Breaking change

Summary

• Resolves existing file symlinks before atomic writes so sync updates the target file without replacing the symlink.
• Allows uninstall cleaners to read symlinked managed files.
• Keeps removal conservative: deleting a symlinked managed file removes the link path, not the dotfiles target.

Changes

File	Change
internal/components/filemerge/writer.go	Resolve existing symlinks and perform atomic replacement on the resolved target path.
internal/components/filemerge/writer_test.go	Replace symlink rejection coverage with preservation/no-op tests.
internal/components/uninstall/cleaners.go	Follow symlinks when reading managed files.
internal/components/uninstall/cleaners_test.go	Cover symlink reads and conservative symlink removal behavior.

Test Plan

• go test ./internal/components/filemerge ./internal/components/uninstall
• git diff --check
• go test ./... — currently fails in internal/cli on this machine because TestEngramPathGuidanceDefault expects go/bin, while Go is installed under asdf (/home/fjpalacios/.asdf/installs/golang/1.25.5/bin). Packages touched by this PR pass.

Follow-up

The interactive/configurable uninstall symlink policy is intentionally deferred to #1078 so this bugfix stays under the 400-line review budget.

Notes

Maintainer note: I cannot add type:bug from this fork, so the PR still needs exactly one type:* label added by someone with label permissions.

Summary by CodeRabbit

• Bug Fixes

  • File updates now handle existing symlinks more safely, preserving the link while updating the intended target.
  • Reading managed files now follows symlinks instead of rejecting them.
  • Broken symlinks are reported as missing, and cleanup correctly removes only the symlink, not its target.

• Tests

  • Updated coverage for symlink behavior, including no-op writes, broken links, and platform-specific symlink permission handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 315a0445-a357-4073-86cd-d919f5687c5e

📥 Commits

Reviewing files that changed from the base of the PR and between 86e737b and 8b68794.

📒 Files selected for processing (4)

• internal/components/filemerge/writer.go
• internal/components/filemerge/writer_test.go
• internal/components/uninstall/cleaners.go
• internal/components/uninstall/cleaners_test.go

---

📝 Walkthrough

Walkthrough

WriteFileAtomic and readComparableFile now resolve symlinks before comparing and renaming, and readManagedFile now follows symlinks instead of rejecting them. The tests were updated to verify preserved links, target updates, broken-link behavior, and Windows symlink privilege skips.

Changes

Symlink resolution support

Layer / File(s)	Summary
WriteFileAtomic symlink resolution internal/components/filemerge/writer.go	WriteFileAtomic resolves the destination path before comparing existing contents, preparing the parent directory, and renaming; readComparableFile resolves symlink targets and reads the resolved file.
WriteFileAtomic symlink test coverage internal/components/filemerge/writer_test.go	Updates the symlink destination test to expect preserved links and target updates, and adds a no-op case for rewriting identical content through the same symlink.
readManagedFile symlink resolution internal/components/uninstall/cleaners.go	readManagedFile now resolves symlinks with filepath.EvalSymlinks, retargets the read to the resolved path, and reports resolution or stat failures on the resolved target.
Uninstall symlink test coverage internal/components/uninstall/cleaners_test.go	Adds Windows symlink-privilege skipping and replaces the rejection test with cases for reading through symlinks, broken symlinks, and removing only the symlink entry.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

• Gentle​man-Programming/gentle-ai#1078 — Also concerns uninstall symlink handling; this PR updates readManagedFile and related cleanup behavior for symlinked managed files.

Suggested labels: type:bug

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main symlink-preservation change.
Linked Issues check	✅ Passed	The PR addresses #306 by allowing sync/uninstall to work with symlinked files instead of rejecting them.
Out of Scope Changes check	✅ Passed	The code changes stay focused on symlink handling in file merge and uninstall paths, with matching test updates.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/components/filemerge/writer.go`:
- Around line 118-134: The resolveExistingSymlink helper in writer.go treats
dangling symlinks as hard errors, causing WriteFileAtomic to abort. Update
resolveExistingSymlink (and the WriteFileAtomic path that uses it) to explicitly
handle os.ModeSymlink targets that are broken by either treating them as
create-through paths or replacing the symlink with a regular file, and keep the
behavior consistent for all symlink-managed paths.

In `@internal/components/uninstall/cleaners_test.go`:
- Around line 286-308: Add a regression test in
TestReadManagedFile_FollowsSymlink coverage for a broken symlink case, using
readManagedFile and the rewriteJSONFile/os.IsNotExist contract. Create a
symlink, remove its target, then verify readManagedFile returns an error that
still satisfies os.IsNotExist through the PathError/ENOENT wrapping path, so
future changes to filepath.EvalSymlinks error handling don’t break the no-op
behavior in service.go.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2a5b3c67-fb38-474c-94f8-6dfc09e000e7

📥 Commits

Reviewing files that changed from the base of the PR and between 61bcfc4 and 86e737b.

📒 Files selected for processing (4)

• internal/components/filemerge/writer.go
• internal/components/filemerge/writer_test.go
• internal/components/uninstall/cleaners.go
• internal/components/uninstall/cleaners_test.go

[fjpalacios]

Addressed the CodeRabbit feedback in 8b68794:

• WriteFileAtomic now handles dangling symlinks by creating the missing target path while preserving the symlink.
• Added TestWriteFileAtomicCreatesDanglingSymlinkTarget.
• readManagedFile now preserves the os.IsNotExist contract for broken symlinks.
• Added TestReadManagedFile_BrokenSymlinkReturnsNotExist.

Validation:

• go test ./internal/components/filemerge ./internal/components/uninstall
• git diff --check

The PR remains under the 400-line review budget: 215 insertions / 16 deletions.