fix(nix): check per-output binary-cache status in InputAddressedPathForOutput (#2921)

[mikeland73]

Summary

Fixes #2921.

Devbox 0.17.3 regressed so that devbox shellenv (and any command that regenerates the environment) fails during flake generation with:

Error: Package "dnsutils@latest" cannot be fetched from binary cache store

for multi-output packages (e.g. dnsutils, curl) when the requested output is available in the binary cache but not every one of the package's default outputs is.

Root cause

The error is raised by InputAddressedPathForOutput (internal/devpkg/package.go), called from the flake template at internal/shellgen/tmpl/flake.nix.tmpl:52 — exactly where the reported stack trace points.

That template line only runs inside {{ if $output.CacheURI }}, and CacheURI is set per output by GetOutputsWithCache → fetchNarInfoStatusOnce(name), which checks that single output.

But InputAddressedPathForOutput internally guarded on IsInBinaryCache(), which is the all-default-outputs check (areExpectedOutputsInCacheOnce(useDefaultOutputs) requires len(found) == len(defaultOutputs)).

So for a package whose requested output is cached while some other default output is not, the template committed to emitting a builtins.fetchClosure for the cached output, yet the stricter internal guard reported the whole package as not-cached and returned the error. The per-output check (template) and the all-outputs check (guard) disagreed. This is the only code path that produces that error message at that template line.

Fix

InputAddressedPathForOutput returns the input-addressed path for a single output, so its precondition should concern only that output. Switch its guard from IsInBinaryCache() to IsOutputInBinaryCache(output). This mirrors InstallableForOutput, which already gates the very same call on the per-output check:

func (p *Package) InstallableForOutput(output string) (string, error) {
    inCache, err := p.IsOutputInBinaryCache(output)   // per-output
    ...
    if inCache {
        installable, err := p.InputAddressedPathForOutput(output)

The plural InputAddressedPaths() is unchanged: it still guards on IsInBinaryCache() (it legitimately wants all default outputs) before iterating, so relaxing the singular guard cannot weaken it — each per-output check there is a subset of the all-outputs check the caller already passed.

The error message now names the specific output for clearer diagnostics.

How was it tested?

• go build ./... and go vet ./internal/devpkg/ — clean.
• go test ./internal/devpkg/ — passes.
• Verified by code analysis that this is the sole path producing "... cannot be fetched from binary cache store" at flake.nix.tmpl:52, and that the fix restores agreement between the template's per-output caching decision and the guard.

Note: the binary-cache resolution path gates on a Nix ≥ 2.17 install (sysInfoIfExists → nix.AtLeast), so an end-to-end reproduction requires a Nix environment with a partially-populated cache (as in the issue). I was unable to run that scenario in the sandbox used here; reviewers with a Nix toolchain can confirm against the issue's repro steps.

cc @sstarcher (issue reporter) — thanks for the detailed diagnosis and stack trace.

Community Contribution License

All community contributions in this pull request are licensed to the project
maintainers under the terms of the
Apache 2 License.

By creating this pull request, I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 License as stated in
the
Community Contribution License.

🤖 Generated with Claude Code

https://claude.ai/code/session_017rMbKrge1aJW6mNbGuPGDS

---

Generated by Claude Code

[Copilot]

Pull request overview

Fixes a regression in internal/devpkg.Package.InputAddressedPathForOutput where multi-output packages could fail flake generation even when the requested output is available in a binary cache, due to the function incorrectly requiring all default outputs to be cached.

Changes:

• Update InputAddressedPathForOutput(output) to guard using IsOutputInBinaryCache(output) instead of IsInBinaryCache().
• Improve the failure diagnostic by including the specific output name in the error message.
• Add an explanatory comment documenting the per-output vs all-outputs cache-check mismatch and linking it to issue #2921.

---

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

[sstarcher]

Anything to get movement here. We are blocked on upgrading devbox without this fix.

[savil]

This makes sense, but I don't think this comment should be added here in the source code. Outside of this specific issue's bug fix, we don't need this comment's information for understanding the functionality of this function.