From a25c8e2527de015224889019ddb01390d2efb0e5 Mon Sep 17 00:00:00 2001 From: Lum1104 Date: Wed, 6 May 2026 22:59:54 +0800 Subject: [PATCH] =?UTF-8?q?fix(install):=20address=20Codex=20review=20?= =?UTF-8?q?=E2=80=94=20guard=20reparse=20deletes=20and=20robust=20uninstal?= =?UTF-8?q?l?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - install.ps1 (P1): refuse to delete real files/directories; only remove paths that are actual junctions/symlinks. Closes a data-loss path where Cmd-Uninstall would Remove-Item -Recurse a pre-existing user directory at ~/.understand-anything-plugin (Cmd-Install correctly skipped touching it, but uninstall did not). Cmd-Uninstall and Unlink-Skills now both go through Remove-Reparse, which uses DirectoryInfo.Delete() so a junction's target is never followed. - install.sh (P2): --uninstall no longer exits early when the checkout has been deleted. The per-skill unlinker falls back to scanning the target dir for stale symlinks pointing into the plugin tree, so users can clean up after manually rm -rf'ing the checkout. Co-Authored-By: Claude Opus 4.7 (1M context) --- install.ps1 | 63 +++++++++++++++++++++++++++++++++++++++++------------ install.sh | 33 ++++++++++++++++++---------- 2 files changed, 71 insertions(+), 25 deletions(-) diff --git a/install.ps1 b/install.ps1 index 428ba42..a066856 100644 --- a/install.ps1 +++ b/install.ps1 @@ -97,8 +97,34 @@ function Get-SkillNames { Get-ChildItem -Path $root -Directory | Select-Object -ExpandProperty Name } +function Test-IsReparse([string]$Path) { + if (-not (Test-Path $Path)) { return $false } + $item = Get-Item -LiteralPath $Path -Force + return ($item.LinkType -eq 'Junction' -or $item.LinkType -eq 'SymbolicLink') +} + +function Remove-Reparse([string]$Path) { + # Removes a junction/symlink without touching its target. Refuses to touch + # real files or directories so an existing user folder at the same path is + # never destroyed. + if (-not (Test-Path $Path)) { return $false } + $item = Get-Item -LiteralPath $Path -Force + if ($item.LinkType -eq 'Junction' -or $item.LinkType -eq 'SymbolicLink') { + $item.Delete() + return $true + } + Write-Warning "Refusing to delete $Path — it is a real file/directory, not a junction/symlink we created. Remove it manually if you intended to." + return $false +} + function New-Junction([string]$LinkPath, [string]$TargetPath) { - if (Test-Path $LinkPath) { Remove-Item -Force -Recurse $LinkPath } + if (Test-Path $LinkPath) { + if (Test-IsReparse $LinkPath) { + (Get-Item -LiteralPath $LinkPath -Force).Delete() + } else { + Write-Error "Refusing to overwrite $LinkPath — it is a real file/directory, not a junction. Move or remove it first." + } + } New-Item -ItemType Junction -Path $LinkPath -Target $TargetPath | Out-Null } @@ -125,16 +151,28 @@ function Link-Skills([string]$Target, [string]$Style) { } function Unlink-Skills([string]$Target, [string]$Style) { + if (-not (Test-Path $Target)) { return } switch ($Style) { 'per-skill' { - foreach ($skill in Get-SkillNames) { - $link = Join-Path $Target $skill - if (Test-Path $link) { Remove-Item -Force -Recurse $link } + $skillsRoot = Get-SkillsRoot + if (Test-Path $skillsRoot) { + foreach ($skill in Get-SkillNames) { + Remove-Reparse (Join-Path $Target $skill) | Out-Null + } + } else { + # Checkout is gone — scan the target dir for stale links pointing + # into our plugin tree so we can still clean up. + Get-ChildItem -LiteralPath $Target -Force | ForEach-Object { + if ($_.LinkType -eq 'Junction' -or $_.LinkType -eq 'SymbolicLink') { + if ($_.Target -match 'understand-anything-plugin[\\/]+skills[\\/]+') { + Remove-Reparse $_.FullName | Out-Null + } + } + } } } 'folder' { - $link = Join-Path $Target 'understand-anything' - if (Test-Path $link) { Remove-Item -Force -Recurse $link } + Remove-Reparse (Join-Path $Target 'understand-anything') | Out-Null } } } @@ -167,18 +205,15 @@ function Cmd-Install([string]$Id) { function Cmd-Uninstall([string]$Id) { $cfg = Resolve-Platform $Id - if (-not (Test-Path $RepoDir)) { - Write-Host "No checkout found at $RepoDir — nothing to unlink." - return - } Write-Host "→ Removing skill links for $Id" Unlink-Skills $cfg.Target $cfg.Style - if (Test-Path $PluginLink) { - Remove-Item -Force -Recurse $PluginLink + if (Remove-Reparse $PluginLink) { Write-Host " ✓ removed $PluginLink" } - Write-Host "`nThe checkout at $RepoDir was kept (other platforms may still use it)." - Write-Host "To remove it: Remove-Item -Recurse -Force '$RepoDir'" + if (Test-Path $RepoDir) { + Write-Host "`nThe checkout at $RepoDir was kept (other platforms may still use it)." + Write-Host "To remove it: Remove-Item -Recurse -Force '$RepoDir'" + } } function Cmd-Update { diff --git a/install.sh b/install.sh index 83ac192..b757160 100755 --- a/install.sh +++ b/install.sh @@ -136,15 +136,28 @@ link_skills() { unlink_skills() { local target="$1" style="$2" + [[ -d "$target" ]] || return 0 case "$style" in per-skill) - local skill - while IFS= read -r skill; do - rm -f "$target/$skill" - done < <(list_skills) + if [[ -d "$(skills_root)" ]]; then + local skill + while IFS= read -r skill; do + [[ -L "$target/$skill" ]] && rm -f "$target/$skill" + done < <(list_skills) + else + # Checkout is gone — scan the target dir for stale links pointing into + # our plugin tree so we can still clean up. + local link resolved + for link in "$target"/*; do + [[ -L "$link" ]] || continue + resolved="$(readlink "$link" 2>/dev/null || true)" + [[ "$resolved" == *"/understand-anything-plugin/skills/"* ]] || continue + rm -f "$link" + done + fi ;; folder) - rm -f "$target/understand-anything" + [[ -L "$target/understand-anything" ]] && rm -f "$target/understand-anything" ;; esac } @@ -186,18 +199,16 @@ cmd_uninstall() { target="$(printf '%s\n' "$row" | cut -d'|' -f2)" style="$(printf '%s\n' "$row" | cut -d'|' -f3)" - if [[ ! -d "$REPO_DIR" ]]; then - printf 'No checkout found at %s — nothing to unlink.\n' "$REPO_DIR" - exit 0 - fi printf -- '→ Removing skill links for %s\n' "$id" unlink_skills "$target" "$style" if [[ -L "$PLUGIN_LINK" ]]; then rm -f "$PLUGIN_LINK" printf ' ✓ removed %s\n' "$PLUGIN_LINK" fi - printf '\nThe checkout at %s was kept (other platforms may still use it).\n' "$REPO_DIR" - printf 'To remove it: rm -rf "%s"\n' "$REPO_DIR" + if [[ -d "$REPO_DIR" ]]; then + printf '\nThe checkout at %s was kept (other platforms may still use it).\n' "$REPO_DIR" + printf 'To remove it: rm -rf "%s"\n' "$REPO_DIR" + fi } cmd_update() {