Repository navigation
fix(global): preserve existing binaries when installation fails - #3119
Conversation
Capture binary files and ownership configs before changing them, then restore their original state on failure instead of removing every declared binary. Keep backups when restoration fails. Add regression coverage for existing files, replaced npm links, partial installs, and failed restoration.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 662a755ebb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Defer temporary directory creation until an existing path needs a backup so fresh global installations do not fail when temporary storage is unavailable. Cover packages without binaries and fresh binary installations with a read-only temporary directory.
Reject foreign regular files at new shim paths before activation and restore backups without comparing full file contents. Keep lazy disk backups so permissions survive rollback and failed restorations retain recoverable files. Cover permission restoration and preserve CLI coverage for existing files and npm ownership after installation failures.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 288eb4b897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Skip shim capture for protected stale bin names while retaining BinConfig backups. Add regression coverage for file identity preservation and config restoration after a later failure.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be182726c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Discard rollback entries added by a failed capture_bin call because no changes to those paths have started. Preserve earlier backups so prior changes still roll back. Add regression coverage for untouched hard links and restoration of earlier binaries and ownership configs.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
jong-kyung
added
test: e2e
2 participants
Summary
A failed
vp install -gcould delete existing executables while rolling back the installation, including files it never created. It could also lose an existing npm link and its ownership config after replacing the link.Rollback now restores the state captured before each change instead of inferring ownership from the package's declared binaries. If restoration fails, backups are retained and a warning identifies their location.