Uh oh!
There was an error while loading. Please reload this page.
Fix update installing all packages instead of target - #2816
Conversation
… target When running `devbox update <pkg>`, packagesToInstallInStore() was unconditionally adding every package to the install list in update mode. This caused all packages to be re-installed with --refresh, not just the ones the user requested. Track which packages are being updated via a field on the Devbox struct and only force-install those in packagesToInstallInStore(). Other packages still go through the normal store-path check. Fixes#2653 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes devbox update <pkg> unintentionally (re)installing all configured packages by tracking the update targets on Devbox and only force-refreshing those packages during the store-install step.
Changes:
- Add
packagesBeingUpdatedtoDevboxand set it duringUpdate. - Change store-install selection so
updatemode force-installs only the targeted packages; other packages still follow the normal “missing store path” check. - Add helper
isBeingUpdatedto check whether a package is part of the current update target set.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/devbox/update.go | Records the update targets on the Devbox instance before running ensureStateIsUpToDate. |
| internal/devbox/packages.go | Adjusts packagesToInstallInStore to only force-refresh targeted packages; adds isBeingUpdated. |
| internal/devbox/devbox.go | Extends Devbox state with packagesBeingUpdated to support targeted refresh behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (d *Devbox) isBeingUpdated(pkg *devpkg.Package) bool { | ||
| for _, u := range d.packagesBeingUpdated { | ||
| if u.Raw == pkg.Raw { | ||
| return true | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
isBeingUpdated does a linear scan over d.packagesBeingUpdated for every package in packagesToInstallInStore. When updating all packages this becomes O(n²) and adds avoidable overhead. Consider representing the update targets as a map[string]struct{}/set keyed by pkg.Raw (or building such a set once) so membership checks are O(1).
| d.packagesBeingUpdated = inputs | ||
There was a problem hiding this comment.
This change fixes a user-visible regression in devbox update <pkg> behavior, but there isn't a test covering the new “only refresh targeted packages” logic. Please add a regression test that exercises Update with a single package in a multi-package config and asserts only that package is selected for store installation/refresh (e.g., by stubbing the store-install selection or asserting on the emitted install list).
Tests that isBeingUpdated correctly filters packages so that only the targeted packages are force-refreshed during `devbox update <pkg>`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
devbox update <pkg>causing all packages to be re-installed instead of just the specified oneDevboxstructpackagesToInstallInStore()now only force-installs the targeted packages; others go through the normal store-path checkFixes#2653
Test plan
devbox update <single-pkg>with multiple packages in devbox.json — only the target should be installeddevbox update(no args) — all packages should still be updated