Fix CLI install recovery from stale App Translocation symlinks#413
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens RepoPrompt CE’s CLI installation flow against stale user-space symlinks created while the app was running under macOS App Translocation, and improves install error reporting so conflicts name the actual offending path.
Changes:
- Extend
ManagedCLIPathPolicy.classifySymlinkto treat certain translocated (AppTranslocation) symlink targets as “managed stale” when their app-bundle-relative suffix matches an allowlisted managed destination. - Update
CLIPathInstaller.InstallError.pathExistsNotOursto carry the conflicting path and update throw sites accordingly. - Add focused XCTest coverage for the new translocation reclaim behavior, user-space symlink repair, and the improved error message.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Tests/RepoPromptTests/MCP/CLIPathInstallerTests.swift |
Adds regression tests for translocated-stale symlink reclaim/repair and for correct path reporting in pathExistsNotOurs. |
Sources/RepoPrompt/Infrastructure/Process/CLI/ManagedCLIPathPolicy.swift |
Adds App Translocation-aware managed-stale classification via suffix matching against known managed destinations. |
Sources/RepoPrompt/Infrastructure/Process/CLI/CLIPathInstaller.swift |
Threads the actual conflicting path through pathExistsNotOurs and updates throw sites to report accurate locations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
install() previously threw pathExistsNotOurs whenever ensureLocalSymlink returned false, so non-conflict failures (support-directory creation, atomic swap/validation) wrongly claimed a conflicting file exists. Addresses Copilot review feedback on PR repoprompt#413. Add InstallError.userSymlinkSetupFailed(path:) and a testable userSymlinkInstallError(userLink:bundledPath:) mapping that reports pathExistsNotOurs only when the user-space path is a genuine unmanaged occupant, and a setup-failure message otherwise. Co-Authored-By: Oz <oz-agent@warp.dev>
6c4f563 to
d424274
Compare
|
Please review the updated PR. Both original Copilot comments have been addressed in the squashed commit: (1) Substring check replaced with path-component match. (2) Added InstallError.userSymlinkSetupFailed and userSymlinkInstallError to distinguish genuine conflicts from other setup failures, with regression tests. |
The CE user-space CLI symlink created while the app runs under macOS App Translocation points at a randomized /AppTranslocation/<uuid>/... path. After the app moves to a stable location that target disappears and is absent from the managed-destination allowlist, so classifySymlink returned .unmanaged and ensureLocalSymlink refused to repair it. Changes: - Recognize a translocated destination as managed-stale (repairable) when its app-bundle-relative suffix matches a known managed destination, while leaving unrelated .app targets and plain foreign links unmanaged. - Make InstallError.pathExistsNotOurs carry the actual offending path so failures no longer always report /usr/local/bin/rpce-cli. - Add InstallError.userSymlinkSetupFailed so install() no longer conflates non-conflict setup failures (directory creation, atomic swap) with file conflicts. - Hardened AppTranslocation detection from substring to path-component match (Copilot review feedback). Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-Authored-By: Oz <oz-agent@warp.dev>
d424274 to
dea1c81
Compare
baron
left a comment
There was a problem hiding this comment.
Approved after maintainer review. Locally rebased onto current main (dea1c81 on 7360ea8), reviewed the CLI symlink/App Translocation fix, ran focused validation (CLIPathInstallerTests: 14 tests, 0 failures), dev-lint, and RepoPrompt product build. Claude Fable 5 High second opinion found no must-fix issues.
Summary
InstallError.pathExistsNotOurscarry the actual offending path instead of always reporting/usr/local/bin/rpce-cli.Symptom
Installing the
rpce-cliCLI (Settings → MCP → CLI Tools → Install) fails with:Install failed: A file already exists at /usr/local/bin/rpce-cli that wasn't created by RepoPrompteven though
/usr/local/bin/rpce-clidoes not exist. This reproduces for anyone who first launched the app from a quarantined location (Downloads/DMG, i.e. App Translocation) and later moved it to/Applications.Root cause
The install failure originates at the user-space symlink step, not the
/usr/local/binshim:CLIPathInstaller.install()finds the PATH shimnotInstalled, then callsCLISymlinkManagerUserSpace.ensureLocalSymlink(...).ensureLocalSymlinkclassifies the existing user-space symlink viaManagedCLIPathPolicy.classifySymlink(...)./private/var/folders/.../AppTranslocation/<uuid>/d/RepoPrompt CE.app/Contents/MacOS/repoprompt-mcp./Applications, that target disappears and is not on the managed-destination allowlist, soclassifySymlinkreturns.unmanaged.ensureLocalSymlinkrefuses to replace.unmanagedentries and returnsfalse;install()throws.pathExistsNotOurs.Second, independent bug:
InstallError.pathExistsNotOurshad no associated value and always formatted its message withinstallPath, so a user-space symlink (orclaude-rpcewrapper) conflict was reported as/usr/local/bin/rpce-cli, a path that may not exist.Fix
ManagedCLIPathPolicy.classifySymlink(...)now also treats a destination as managed-stale (repairable) when:/AppTranslocation/, and*.appcomponent to the end matches the same suffix of a known managed destination (e.g.RepoPrompt CE.app/Contents/MacOS/repoprompt-mcp).This keeps unrelated paths unmanaged (an unrelated translocated
*.app, or plain foreign symlinks stay.unmanaged), and leaves current/stale/normal classifications unchanged.InstallError.pathExistsNotOursnow carriespath: String; each throw site passes the path that actually conflicted (the PATH shim, the user-space symlink, or theclaude-rpcewrapper).Proof
Standalone harness that mirrors
classifySymlinkand toggles only the new translocation branch, run against real on-disk symlinks:Local reproduction confirmed the user-space symlink pointed at a deleted
AppTranslocationtarget while/usr/local/bin/rpce-clidid not exist, matching both the misleading message and the refusal to repair.Tests
Added to
CLIPathInstallerTests:testClassifierReclaimsStaleTranslocatedManagedTarget— stale translocated managed target is.managedStale; a non-matching translocated*.appsuffix stays.unmanaged.testUserSpaceManagerRepairsStaleTranslocatedSymlink—ensureLocalSymlinkrepairs the stale translocated link (direct regression for the reported bug).testPathExistsNotOursErrorReportsProvidedPath— the error message names the provided path.Validation notes
xcrun swiftc -parseon both edited source files: no syntax errors.make dev-test FILTER=CLIPathInstallerTests,make dev-lint, andmake dev-swift-buildcould not run in the contributor environment because it is Command Line Tools–only and hits the existingPreviewsMacrosplugin limitation (issue CLT-only builds fail despite doctor.sh passing — Xcode-only macro plugins #174); the XCTest target cannot compile there. These lanes should run in CI / on a full-Xcode machine, and the added tests are written to run there.Relationship to #278
Complementary and non-conflicting. #278 relocates the user-space symlink path in
MCPFilesystemIdentity.swift(a hard cutover) and would incidentally sidestep the symptom by targeting a fresh path; it does not fix the classifier logic or the misleading error text. This PR touches onlyManagedCLIPathPolicy.swiftandCLIPathInstaller.swift.Conversation: https://app.warp.dev/conversation/550683e7-eeb5-469a-894f-eb6f7360e3fc
Co-Authored-By: Oz oz-agent@warp.dev