Repository navigation
fix(cli): refuse upgrade methods that cannot replace the running binary - #52149
Raed2180416 wants to merge 1 commit into
Conversation
`method()` selected a package manager purely by probing that manager's global package list. When a second copy of the CLI is registered elsewhere - a custom npm prefix, or a manually linked directory - the probe still matched, so `opencode upgrade` ran `install --global` against an unrelated copy, printed "Upgrade complete", and left the running binary untouched. Confirmed on a real install, where the running binary was 2.0.19 while `npm install --global @opencode/[email protected]` reported success and changed nothing about the binary that was actually executing. Now each candidate manager must also own the running executable: its global root has to contain the binary, otherwise it is skipped. When no manager qualifies, detection reports "unknown" and the upgrade is refused with an actionable message instead of silently doing nothing. An unreadable root preserves the previous behaviour rather than disabling upgrades outright.
github-actions
Bot
added
needs:compliance
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes #52152
Type of change
What does this PR do?
method()chose a package manager by probing only that manager's global package list. That list can match even when the running binary is a different copy. The upgrade then raninstall --globalagainst the other copy, so the command reported success while the executing binary stayed put.The invariant being violated: a package manager can only replace the OpenCode copy inside its own global root. So each candidate manager now also resolves its global root, and is only eligible if that root contains the running executable. If none qualify, detection returns
unknownand the existing actionable error is shown instead of a silent no-op.installedPackagebecameinstalled({ package, executable }) to carry the resolved path. The name is still derived by the same manifest check, so detection for correct single-install setups is unchanged.How did you verify your code works?
replacesExecutableis exported as a pure helper with unit tests inupdater.test.ts, covering: binary inside the root (accept), outside it (reject — the reported case), a sibling directory sharing the root string prefix (reject), the root itself (reject), empty/unavailable root (preserves behaviour), and untrimmed/relative roots.I also ran the helper against the real paths from the reproduction, to confirm the buggy layout is refused and the healthy layout still accepted (guarding against a false negative that would break normal upgrades):
Then end-to-end on a working install, after repointing the binary into the global prefix:
upgrade 2.0.18moved the running binary 2.0.19 → 2.0.18, and a bareupgrademoved it 2.0.18 → 2.0.19. Both now actually change the running version.Caveat, stated plainly: my machine ran out of disk partway through, so I could not run the full
packages/clisuite or the pre-pushcheckhook, since both need a workspace install. The helper tests were run in isolation and pass, and both changed files parse cleanly, but I am relying on CI for the authoritative verification.Screenshots / recordings
Not a UI change.
Checklist
Scope of "tested locally", so it is not overread: I ran the new
replacesExecutableunit tests in isolation (6 pass), parse-checked both changed files, exercised the helper against the real paths from the reproduction, and confirmed end-to-end on a working install thatupgrade 2.0.18moved the running binary 2.0.19 -> 2.0.18 and a bareupgrademoved it back to 2.0.19. I did not run the fullpackages/clisuite, because my machine ran out of disk and the workspace install could not complete. CI is the authoritative check for the full suite.