From 623d7949669874ad0e01f626860ba446d4231946 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 14:23:15 +0000 Subject: [PATCH 1/2] fix(config): preserve multiple outputs from the same remote flake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding two outputs from the same remote flake (for example `git+http://git@host/repo.git#toolA` and `...#toolB`) dropped the first one. `Config.Packages` deduped the package list with `lo.UniqBy` keyed on `Package.Name`, keeping only the last occurrence per name. For flake references that Name is not unique per output: package strings are split on their last `@`, so a "#fragment" ends up in the version and, for URLs containing a `user@host`, the Name is truncated to the part before the `@`. Every output of such a flake therefore shares the same Name and collapsed into a single package. Key the dedup on the full reference (which includes the fragment) when the package looks like a flake installable — detected by a `://` scheme separator or a `#` output fragment — and keep deduping plain packages by name so a later `pkg@v2` still replaces an earlier `pkg@v1`. Fixes #2662 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Qk7hyZKVoiy6CrUdA6XknT --- internal/devconfig/config.go | 27 +++++++++++++-- internal/devconfig/config_test.go | 57 +++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/internal/devconfig/config.go b/internal/devconfig/config.go index c7e38d02965..cf0cc53a0f8 100644 --- a/internal/devconfig/config.go +++ b/internal/devconfig/config.go @@ -9,6 +9,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "syscall" "time" @@ -334,17 +335,39 @@ func (c *Config) Packages( } } - // Keep only the last occurrence of each package (by name). + // Keep only the last occurrence of each package. Plain packages are + // deduped by name so that a later "pkg@v2" replaces an earlier + // "pkg@v1". Flake references, however, can install several distinct + // outputs from the same flake via a "#fragment" (for example + // "git+https://host/repo.git#toolA" and "...#toolB"). Once parsed those + // share a name (and for URLs containing "user@host" the name is even + // truncated to the part before the "@"), so keying on name alone + // collapses them into a single package. Key such references on their + // full reference so each output is preserved. See jetify-com/devbox#2662. mutable.Reverse(packages) packages = lo.UniqBy( packages, - func(p configfile.Package) string { return p.Name }, + func(p configfile.Package) string { + if ref := p.VersionedName(); isFlakeReference(ref) { + return ref + } + return p.Name + }, ) mutable.Reverse(packages) return packages } +// isFlakeReference reports whether ref looks like a flake installable rather +// than a plain "name@version" package. Flake references contain a URL scheme +// separator ("://") or an output fragment ("#"); plain package versions never +// do. Such references may point at distinct outputs of the same flake, so they +// must not be collapsed by package name. +func isFlakeReference(ref string) bool { + return strings.Contains(ref, "://") || strings.Contains(ref, "#") +} + func (c *Config) NixPkgsCommitHash() string { return c.Root.NixPkgsCommitHash() } diff --git a/internal/devconfig/config_test.go b/internal/devconfig/config_test.go index 9f747796156..18f9945842b 100644 --- a/internal/devconfig/config_test.go +++ b/internal/devconfig/config_test.go @@ -560,6 +560,63 @@ func TestLoadRecursiveMultipleBuiltinPluginIncludes(t *testing.T) { } } +// TestPackagesPreservesMultipleFlakeOutputs is a regression test for +// jetify-com/devbox#2662: adding two outputs from the same remote flake used to +// drop the first one. Both outputs are parsed with the same package Name (the +// "#fragment" that distinguishes them, and part of the "user@host" URL, is +// parsed into the version), so deduping by Name alone collapsed them. +func TestPackagesPreservesMultipleFlakeOutputs(t *testing.T) { + cfg, err := loadBytes([]byte(`{ + "packages": [ + "kubectl@latest", + "git+http://git@example.com/group/utils.git#kubeupdate", + "git+http://git@example.com/group/utils.git#getcrds" + ] + }`)) + if err != nil { + t.Fatalf("loadBytes error: %v", err) + } + + got := map[string]bool{} + for _, pkg := range cfg.Packages(false /*includeRemovedTriggerPackages*/) { + got[pkg.VersionedName()] = true + } + + want := []string{ + "kubectl@latest", + "git+http://git@example.com/group/utils.git#kubeupdate", + "git+http://git@example.com/group/utils.git#getcrds", + } + for _, name := range want { + if !got[name] { + t.Errorf("Packages() dropped %q; got %v", name, got) + } + } + if len(got) != len(want) { + t.Errorf("Packages() returned %d packages, want %d: %v", len(got), len(want), got) + } +} + +// TestPackagesDedupesByNameForPlainPackages verifies that the flake-aware +// dedup key preserves the original "last version wins" behavior for plain +// (non-flake) packages. +func TestPackagesDedupesByNameForPlainPackages(t *testing.T) { + cfg, err := loadBytes([]byte(`{ + "packages": ["python@3.10", "python@3.11"] + }`)) + if err != nil { + t.Fatalf("loadBytes error: %v", err) + } + + pkgs := cfg.Packages(false /*includeRemovedTriggerPackages*/) + if len(pkgs) != 1 { + t.Fatalf("Packages() returned %d packages, want 1: %v", len(pkgs), pkgs) + } + if got := pkgs[0].VersionedName(); got != "python@3.11" { + t.Errorf("Packages() kept %q, want the last occurrence \"python@3.11\"", got) + } +} + // testLockProject satisfies the unexported lock.devboxProject interface for tests. type testLockProject struct { dir string From afa48d6b0bc2378918e1f1c36a2b501ca2950375 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 14:27:54 +0000 Subject: [PATCH 2/2] test(config): assert slice length so duplicate flake outputs are caught Address review feedback: the length check keyed off a map built from VersionedName(), which would collapse duplicates and could pass even if the dedupe logic regressed to returning duplicate packages. Assert on the returned slice length directly and require each expected package to appear exactly once. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Qk7hyZKVoiy6CrUdA6XknT --- internal/devconfig/config_test.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/internal/devconfig/config_test.go b/internal/devconfig/config_test.go index 18f9945842b..0dbba1db326 100644 --- a/internal/devconfig/config_test.go +++ b/internal/devconfig/config_test.go @@ -577,9 +577,10 @@ func TestPackagesPreservesMultipleFlakeOutputs(t *testing.T) { t.Fatalf("loadBytes error: %v", err) } - got := map[string]bool{} - for _, pkg := range cfg.Packages(false /*includeRemovedTriggerPackages*/) { - got[pkg.VersionedName()] = true + pkgs := cfg.Packages(false /*includeRemovedTriggerPackages*/) + counts := map[string]int{} + for _, pkg := range pkgs { + counts[pkg.VersionedName()]++ } want := []string{ @@ -588,12 +589,18 @@ func TestPackagesPreservesMultipleFlakeOutputs(t *testing.T) { "git+http://git@example.com/group/utils.git#getcrds", } for _, name := range want { - if !got[name] { - t.Errorf("Packages() dropped %q; got %v", name, got) - } - } - if len(got) != len(want) { - t.Errorf("Packages() returned %d packages, want %d: %v", len(got), len(want), got) + switch counts[name] { + case 0: + t.Errorf("Packages() dropped %q; got %v", name, pkgs) + case 1: // expected + default: + t.Errorf("Packages() returned %q %d times, want exactly once", name, counts[name]) + } + } + // Assert on the slice length (not the deduped map) so an accidental + // regression that returns duplicates is also caught. + if len(pkgs) != len(want) { + t.Errorf("Packages() returned %d packages, want %d: %v", len(pkgs), len(want), pkgs) } }