Skip to content

fix(install): guard link-mode conflicts#449

Open
fengjikui wants to merge 1 commit into
devinoldenburg:mainfrom
fengjikui:codex/guard-link-install-conflicts
Open

fix(install): guard link-mode conflicts#449
fengjikui wants to merge 1 commit into
devinoldenburg:mainfrom
fengjikui:codex/guard-link-install-conflicts

Conversation

@fengjikui

Copy link
Copy Markdown

Summary

  • Refuse to replace existing non-symlink paths during --link installs unless --force is provided.
  • Apply the same conflict check to --dry-run so destructive link-mode conflicts are visible before install.
  • Add regression coverage for preserving a user-owned plugins/goal-mode/ directory and for the explicit --force replacement path.

Why

opencode-goal-mode-install --link removed existing destinations before creating symlinks. If a user already had a real plugins/goal-mode/ directory under their OpenCode config, the installer could delete those files without requiring --force. Copy installs already treat conflicts conservatively, so link installs should do the same.

Validation

  • Reproduced the original behavior with a temp config containing plugins/goal-mode/keep.txt: install exited 0 and removed the file.
  • Verified the fix: --link exits 1 and preserves keep.txt without --force.
  • Verified --link --force replaces the directory with a symlink.
  • npm test (26 pass)
  • npm run lint (passes; existing warnings only in unrelated files)
  • git diff --check

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@devinoldenburg devinoldenburg left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix, this is definitely the right direction. One small thing before merging: prepareLinkDestination() uses existsSync(dest), which misses dangling symlinks because it follows the link. With a broken existing symlink, --link --force now skips cleanup and then fails with EEXIST when symlinkSync() runs. Could you switch that check to lstatSync() in a try/catch and treat only ENOENT as missing? A small regression test for a dangling symlink would be great too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants