fix(lock): don't rewrite plugin_version on shell/run/install

[mikeland73]

Summary

devbox shell/run/install overwrote plugin_version in devbox.lock with the version of the built-in plugin embedded in the running binary, so teams on different Devbox versions saw the lockfile flip back and forth (reported in #1371). Now those commands only fill in a missing plugin_version; add, rm, and update still overwrite it. The value is only read as an "is installed" flag, so keeping an older version is harmless.

How was it tested?

Added TestUpdateLockfileVersion and ran go test ./internal/plugin/ ./internal/devbox/. Built the binary and used the dlvhdr/gh-dash docs/ lockfile (nodejs plugin_version 0.0.2): devbox install and devbox run left it unchanged, devbox update nodejs bumped it to 0.0.5, and devbox install filled it in when the field was missing.

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

[mikeland73]

@claude ptal

[claude]

Claude finished @mikeland73's task in 1m 38s —— View job

---

Code Review

Todo list:

• Gather context (diff, changed files, CLAUDE.md)
• Understand the request (ptal = please take a look — code review)
• Review internal/plugin/plugin.go changes
• Review internal/devbox/packages.go changes
• Review internal/plugin/plugin_test.go changes
• Post review feedback

Summary

The fix looks correct and well-targeted. I traced the call chain end-to-end:

• devbox install/shell/run → Devbox.Install() (internal/devbox/devbox.go:345-350) and the ensure paths in devbox.go:349,585 → all call ensureStateIsUpToDate(ctx, ensure) → updateLockfile(mode, ...) computes overwrite := mode != ensure → false. ✅ Only fills a missing plugin_version.
• devbox add → Add() (internal/devbox/packages.go:163) uses the internal installMode enum value install (a different concept from the CLI install command, just an unfortunate naming overlap) → overwrite = true. ✅
• devbox rm → uninstall mode → overwrite = true. ✅
• devbox update (with or without --no-install) → update/noInstall modes → overwrite = true. ✅

That naming overlap between the CLI install command (which maps to ensure mode) and the installMode enum's install value (used by Add) tripped me up briefly during review — it's not a bug, but might be worth a one-line comment near the installMode const block for future readers, since the two "install"s mean different things.

plugin.go's UpdateLockfileVersion change itself is minimal and safe: it returns early if there's no lockfile entry (unchanged) or if a non-empty version already exists and overwrite is false. The doc comment on both UpdateLockfileVersion and the updateLockfile loop clearly explains the intent.

The new TestUpdateLockfileVersion in plugin_test.go covers the three relevant cases (missing → filled, existing + no overwrite → kept, existing + overwrite → replaced) using the existing fakeIncludable test helper — good, focused table-driven test.

Nit: internal/plugin/plugin_test.go:129-138 adds a lockProjectForTest type solely to construct a lock.File via lock.GetFile. That's reasonable given the lockfile API, no changes needed there.

I wasn't able to run go build/go test in this sandboxed environment (tool permissions block go build/go test invocations), so I relied on careful static tracing of all callers instead. The PR description states these tests were run locally and passed, and the logic traces cleanly, so I have high confidence in the change, but flagging this limitation per instructions.

No blocking issues found — this looks ready to merge.