Skip to content

Fix devbox update not refreshing locked revs for flake refs#2825

Open
Lagoja wants to merge 3 commits intojetify-com:mainfrom
Lagoja:fix/update-refreshes-flake-refs
Open

Fix devbox update not refreshing locked revs for flake refs#2825
Lagoja wants to merge 3 commits intojetify-com:mainfrom
Lagoja:fix/update-refreshes-flake-refs

Conversation

@Lagoja
Copy link
Copy Markdown
Collaborator

@Lagoja Lagoja commented Apr 24, 2026

Problem Summary

When running devbox update on a package added via flake reference, the package stays pinned to the original commit hash when it was added (in other words, update doesn't work). The hash is only advanced when you remove and re-add the package.

devbox update now refreshes the locked commit of flake references, matching how it already works for versioned packages. Under the hood, it re-resolves the flake against the upstream source (with --refresh so nix doesn't hand back a stale cache) and rewrites the lockfile entry to the new commit. A broken ref still prints a warning instead of aborting the whole update, so one bad flake won't block the rest of the packages from updating.

Summary

  • devbox update on a flake ref (e.g. github:owner/repo#attr) now actually advances the locked rev. Previously the dispatch routed unversioned packages to attemptToUpgradeFlake (→ nix profile upgrade), which is a no-op once devbox's lockfile has pinned the rev — the only way to get a newer commit was to remove and re-add the package.
  • Flake refs now flow through the same FetchResolvedPackage pipeline as versioned nixpkgs packages, with a flake-aware branch in mergeResolvedPackageToLockfile that compares on Resolved (since flakes carry no Version).
  • A refresh bool is threaded through ResolveFlake / lockFlake / FetchResolvedPackage so the update path passes --refresh to nix flake metadata. Without this, nix's own eval/tarball cache can still serve stale results on the update path (observed: a 10-hour-old commit). Add, install, and outdated-check callers keep using the cache.
  • Forgiving error handling is preserved: one broken flake ref (deleted branch, renamed attr, network blip) warns and lets the rest of the update proceed — matches the contract from [update] Update flakes #1180 / [update] Ignore update flake error #1840.
  • attemptToUpgradeFlake, nix.ProfileUpgrade, and nixprofile.ProfileUpgrade are removed (no remaining callers).

User-visible message

Info: Updating github:numtide/llm-agents.nix#claude-code 92de4ac -> 8ff0f2a  (2026-04-24 → 2026-04-24)

Falls back gracefully for refs without a git rev (e.g. path:): shows the date range only. Falls back to rev-only if LastModified is missing.

Scope

Affects devbox update and devbox global update — they share this code path, and both were affected by the bug.

Test plan

  • go build ./... and go vet ./... clean
  • go test ./internal/... — all pass, including 6 new tests covering mergeResolvedFlakeToLockfile, shortRev, shortDate, and describeFlakeUpdate
  • go test ./testscripts -run TestScripts/update — both the pre-existing update.test (nixpkgs) and the new update_flake_local.test (local path: flake) pass
  • nix build .#default produces a working binary
  • Manual smoke: pre-seeded an older rev of github:numtide/llm-agents.nix#claude-code in devbox.lock, ran devbox update, and saw the rev advance with the expected message format
  • Reviewer: consider verifying with a real second flake ref in your own project to double-check the per-ref message format

`devbox update` on a flake ref (e.g. `github:owner/repo#attr`) never
moved the locked rev forward. The dispatch in `Update` routed
unversioned packages to `attemptToUpgradeFlake`, which ran
`nix profile upgrade` — a no-op once devbox's lockfile had already
pinned the rev. Re-adding the package worked only because the Add path
re-resolves via `FetchResolvedPackage`.

Route flake refs through the same lockfile-resolution pipeline as
versioned nixpkgs packages, and add a flake-aware branch to
`mergeResolvedPackageToLockfile` since flake refs have no Version.
Errors in the flake branch are non-fatal (warn + continue) to preserve
the forgiving contract from jetify-com#1180 / jetify-com#1840 — one broken ref should not
abort update for the rest.

Also thread a `refresh bool` through `ResolveFlake`, `lockFlake`, and
`FetchResolvedPackage` so the update path passes `--refresh` to
`nix flake metadata`. Without it, nix's own eval/tarball cache can
serve a stale result (observed: a 10-hour-old commit even after the
dispatch fix). Other callers (Add, install, outdated checks) pass
`false` and keep using the cache.

User-facing message:

    Info: Updating github:numtide/llm-agents.nix#claude-code 92de4ac -> 8ff0f2a  (2026-04-24 → 2026-04-24)

Affects both `devbox update` and `devbox global update` — same code
path.

Removes `attemptToUpgradeFlake` and its `nix.ProfileUpgrade` /
`nixprofile.ProfileUpgrade` helpers (no remaining callers).
@Lagoja Lagoja requested review from Copilot, gcurtis and mikeland73 and removed request for Copilot April 24, 2026 20:28
@Lagoja Lagoja marked this pull request as ready for review April 24, 2026 20:28
Splits the per-package dispatch loop out of Update() so golangci-lint's
revive/cognitive-complexity check passes (was 34, threshold 32). No
behavior change.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes devbox update for flake refs so that updating a flake reference (e.g. github:owner/repo#attr) actually advances the locked revision in devbox.lock, using a flake-aware lockfile merge path and a --refresh metadata resolution option to avoid stale Nix cache results.

Changes:

  • Route flake refs through the normal lockfile resolution/merge pipeline and compare updates via Resolved (and LastModified) instead of Version.
  • Thread a refresh boolean through flake resolution so devbox update can pass --refresh to nix flake metadata.
  • Remove the old nix profile upgrade-based flake update code path and add regression coverage for flake-update formatting/behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
testscripts/update/update_flake_local.test.txt Adds regression test ensuring local path: flake update doesn’t warn/fail.
internal/nix/nixprofile/upgrade.go Removes profile-upgrade helper (no longer used).
internal/nix/flake_update.go Removes ProfileUpgrade wrapper around nix profile upgrade.
internal/nix/flake.go Adds refresh parameter to ResolveFlake and passes --refresh when requested.
internal/lock/resolve.go Adds refresh plumbing to flake resolution (FetchResolvedPackage / lockFlake).
internal/lock/lockfile.go Updates call sites for new FetchResolvedPackage(pkg, refresh) signature.
internal/devbox/update_test.go Adds tests for flake lockfile merging and update message formatting helpers.
internal/devbox/update.go Updates flake update flow to rewrite lock entries; adds flake-specific merge and messaging.
internal/devbox/packages.go Updates outdated-check path for new FetchResolvedPackage signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/devbox/update.go Outdated
The lexicographic staleness check would trigger whenever
resolved.LastModified was empty (some nix error paths omit it), since
any non-empty string sorts after "". That wrongly blocked legit
updates when the existing lock had a populated timestamp. Skip the
guard unless both sides have a value — missing == unknown, not older.

Addresses Copilot review feedback on jetify-com#2825.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants