fix(config): keep git flake subdirectory refs as distinct inputs (#2704)

[mikeland73]

Summary

Fixes #2704.

Two git flake references that point at the same repository but different subdirectories (via the dir query parameter) are distinct inputs, but devbox deduplicated them and silently installed only one. For example:

"packages": [
  "git+ssh://git@gitlab.com/org/devbox-shims.git?dir=betteralign&ref=master&rev=17d2bedca4884176e0d08078aa42311053e531c2",
  "git+ssh://git@gitlab.com/org/devbox-shims.git?dir=recovergoroutine&ref=master&rev=593d44945851d912f0c2158c46cc76da445adc43"
]

Only dir=recovergoroutine ended up in devbox.lock and got installed.

Root cause

parseVersionedName (in internal/devconfig/configfile/packages.go) split every package string on @ to separate the name from the version. Git flake URLs legitimately contain an @ in their git@host authority, so both of the refs above parsed to the name git+ssh://git, with the rest of the URL (including the differentiating dir) dumped into the "version".

Config.Packages() then deduplicates packages by name (lo.UniqBy(p.Name) — used so that a version override wins over an included/plugin package). Because both refs shared the same parsed name, one of the two distinct inputs was dropped.

Fix

Treat flake references as unversioned in parseVersionedName, so the whole reference is used as the name. Flake refs aren't Devbox name@version packages, so this is also conceptually correct. This:

• keeps refs that differ only by query parameters (such as dir) distinct, and
• leaves versioned Devbox packages (python@3.10, emacsPackages.@@latest, …) and flake refs without an @ unchanged.

The flake-detection reuses the existing pkgtype.IsFlake helper (no import cycle — pkgtype does not depend on configfile). The package's VersionedName() output is unchanged for these refs, so no lockfile migration is needed.

Tests

• TestParseVersionedName: added git-ssh-flake-with-dir and git-ssh-flake-without-query cases.
• TestPackagesDoesNotDedupDistinctGitFlakes: new regression test asserting that Config.Packages() preserves both git flakes that differ only by dir.

ok  go.jetify.com/devbox/internal/devconfig/configfile
ok  go.jetify.com/devbox/internal/devconfig (TestPackagesDoesNotDedupDistinctGitFlakes PASS)

(The two pre-existing TestFindError permission sub-tests fail only when the suite runs as root and are unrelated to this change.)

cc @maxiem-ota-insight (issue author)

🤖 Generated with Claude Code

https://claude.ai/code/session_013x2JTaq6GPt4kX5bubQC2D

---

Generated by Claude Code

[Copilot]

Pull request overview

This PR fixes a bug where Devbox incorrectly deduplicated distinct git flake inputs that point to the same repo but different subdirectories (via ?dir=...), caused by @ in git@host being misinterpreted as the name@version delimiter during package parsing.

Changes:

• Update parseVersionedName to treat flake references as unversioned, preserving the full flake ref as the package name.
• Extend TestParseVersionedName coverage for git+ssh://git@... flake URLs (with and without query params).
• Add a regression test ensuring Config.Packages() does not deduplicate distinct git flake refs that differ only by dir.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
internal/devconfig/configfile/packages.go	Detect flake references early and avoid splitting on @, preventing incorrect deduplication.
internal/devconfig/configfile/packages_test.go	Adds test cases proving parseVersionedName preserves full git+ssh flake refs containing @.
internal/devconfig/config_test.go	Adds regression test asserting Config.Packages() preserves distinct git flake inputs differing by dir.

---

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

[mikeland73]

@claude please review this PR for correctness, and flag anything that would block merging.

[claude]

Claude finished @mikeland73's task in 2m 58s —— View job

---

Reviewing PR #2877

• Gather context (diff, repo setup)
• Read changed files
• Analyze correctness of fix
• Verify tests exist and reasoning holds
• Post final review

Note: I was unable to run go build/go test in this sandbox (the tool invocations were blocked by the environment's approval requirements), so this review is based on static analysis of the diff and surrounding code only. The PR author's report of go test ./internal/devconfig/... passing should be independently verified in CI.

Summary

This is a well-targeted, low-risk fix. I traced the root cause and confirmed it independently:

• Config.Packages() (internal/devconfig/config.go:339) dedupes by p.Name via lo.UniqBy — this is real and does exactly what the PR describes.
• searcher.ParseVersionedPackage (internal/searcher/parse.go:14) splits on the last @. For a git flake ref like git+ssh://git@gitlab.com/org/repo.git?dir=..., there's only one @ (in the git@host authority), so it splits into name="git+ssh://git", version="gitlab.com/org/repo.git?dir=...". Two refs differing only by dir produce the same name, so one is silently dropped. This matches the PR's root-cause writeup exactly.
• The fix in internal/devconfig/configfile/packages.go:372 short-circuits via pkgtype.IsFlake(versionedName) before the @-split, using the entire ref as the name (version "").

I checked a few things that could have made this risky, and they all check out:

1. No import cycle: internal/devpkg/pkgtype/flake.go only imports strings and go.jetify.com/devbox/nix/flake — it doesn't import configfile, so this is safe.
2. Consistency with existing flake detection: pkgtype.IsFlake is already the canonical check used elsewhere (internal/devbox/update.go:128,174, internal/lock/lockfile.go:84, internal/lock/resolve.go:36), so this doesn't introduce a second, potentially divergent heuristic.
3. No regression for plain name@version packages containing @: e.g. git@latest (a real, common Devbox package — the git CLI tool, exercised in internal/devbox/update_test.go:199 and internal/shellgen/generate_test.go:101). pkgtype.IsAmbiguous requires indirect flake refs to have a flake: prefix to be considered unambiguous, so git@latest (no scheme) is correctly rejected by IsFlake and still falls through to the normal @-split logic. Same reasoning protects python@3.10, emacsPackages.@@latest, etc. — verified against the existing test cases, which are all preserved.
4. VersionedName() round-trips identically: since Name now contains the full original string and Version is "", VersionedName() reconstructs the exact original ref (previously it was name + "@" + version reconstructing the same string via the split fields). So no lockfile/config migration is needed, as claimed.
5. pkgs.index(name, version) (used by Add/Remove/AddPlatforms/etc. in the same file) still works correctly since both sides of any comparison go through the same parseVersionedName, so add/remove/platform-set operations on git flakes remain internally consistent.

Tests

The new test cases are appropriate and directly exercise the bug:

• TestParseVersionedName's git-ssh-flake-with-dir / git-ssh-flake-without-query cover the unit-level parsing.
• TestPackagesDoesNotDedupDistinctGitFlakes in config_test.go is a good regression test that exercises the actual Config.Packages() dedup path end-to-end (not just the parser), which is the layer where the bug actually manifested.

Nits (non-blocking)

• None found. The change is minimal, well-scoped, and the comments in packages.go explain the non-obvious "why" (flake refs legitimately contain @) without restating the code.

Verdict

I don't see anything that should block merging. The fix is correct, narrowly scoped, backed by tests that reproduce the actual bug (not just the parsing symptom), and I couldn't construct a case where it regresses existing behavior.