Skip to content

Commit dbdc3a5

Browse files
fentasclaude
andauthored
fix(state): preserve 'version:' on b.yaml round-trip (--fix, oci:// tags) (#149)
## Summary \`BinaryList.MarshalYAML\` emitted \`LocalBinary.Enforced\` as YAML \`version:\` and ignored \`LocalBinary.Version\` entirely. Two visible consequences: 1. \`b install --add oci://docker@cli\` produced \`oci://docker: {}\` in \`b.yaml\` — the tag (\`cli\`) was loaded into \`Version\` (via \`version:\`) and never written back out. 2. \`b install --fix jq@v1.7\` wrote \`version: v1.7\`. On reload only \`Version\` was populated, so the \`--fix\` pin was silently lost; next \`b update\` could move jq off the pinned version. ## Fix Emit \`Version\` as \`version:\` and \`Enforced\` as \`enforced:\` in \`BinaryList.MarshalYAML\`. The unmarshal path already maps them to distinct YAML keys, so they now round-trip independently. ## Workflow Split into two commits for clarity: 1. \`test(state): failing regression\` — adds 4 sub-tests for \`MarshalYAML\` and a file-round-trip test, all currently failing. 2. \`fix(state): preserve 'version:' on b.yaml round-trip\` — implements the fix; new tests pass; two existing tests updated from the old buggy semantics to the correct ones. ## End-to-end reproduction Before: \`\`\` $ b install --add oci://docker@cli:/usr/local/bin/docker $ cat b.yaml binaries: oci://docker:/usr/local/bin/docker: {} # ← tag wiped \`\`\` After: \`\`\` binaries: oci://docker:/usr/local/bin/docker: version: cli \`\`\` ## Test plan - [x] \`TestBinaryListMarshalYAML_VersionRoundTrip\` (4 sub-tests: version-only, enforced-only, both-set, oci-tag-as-version). - [x] \`TestBinaryListMarshalYAML_VersionSurvivesFileRoundTrip\` — Load → Save → reload. - [x] \`go test ./...\` — all 39 packages pass. - [x] Manual e2e: \`b install --add oci://docker@cli:/usr/local/bin/docker\` now writes \`version: cli\` to b.yaml. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a09a756 commit dbdc3a5

2 files changed

Lines changed: 137 additions & 7 deletions

File tree

pkg/state/types.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,16 @@ func (list *BinaryList) MarshalYAML() (interface{}, error) {
345345
// Build the binary configuration
346346
config := make(map[string]string)
347347

348-
// Add version if enforced
348+
// Emit 'version:' — the requested version (loaded from YAML
349+
// 'version:' or set by 'b install --add <ref>@<tag>').
350+
if b.Version != "" {
351+
config["version"] = b.Version
352+
}
353+
354+
// Emit 'enforced:' — the '--fix' pin. Distinct YAML key from
355+
// 'version:' so both can round-trip independently.
349356
if b.Enforced != "" {
350-
config["version"] = b.Enforced
357+
config["enforced"] = b.Enforced
351358
}
352359

353360
// Add alias if specified

pkg/state/types_test.go

Lines changed: 128 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strings"
66
"testing"
77

8+
"github.com/fentas/b/pkg/binary"
89
"gopkg.in/yaml.v3"
910
)
1011

@@ -317,6 +318,8 @@ func TestCreateDefaultConfig(t *testing.T) {
317318

318319
func TestBinaryListMarshalYAML(t *testing.T) {
319320
list := BinaryList{
321+
// Enforced-only entries now serialize as 'enforced:' (not 'version:')
322+
// so --fix pins round-trip correctly instead of being lost.
320323
{Name: "jq", Enforced: "jq-1.7"},
321324
{Name: "envsubst", Alias: "renvsubst"},
322325
{Name: "kubectl", File: "/usr/local/bin/kubectl"},
@@ -333,13 +336,16 @@ func TestBinaryListMarshalYAML(t *testing.T) {
333336
t.Fatal("expected map result")
334337
}
335338

336-
// jq should have version
339+
// jq should have enforced (not version — those are distinct now)
337340
jqCfg, ok := m["jq"].(map[string]string)
338341
if !ok {
339342
t.Fatal("jq config should be map[string]string")
340343
}
341-
if jqCfg["version"] != "jq-1.7" {
342-
t.Errorf("jq version = %q, want %q", jqCfg["version"], "jq-1.7")
344+
if jqCfg["enforced"] != "jq-1.7" {
345+
t.Errorf("jq enforced = %q, want %q", jqCfg["enforced"], "jq-1.7")
346+
}
347+
if jqCfg["version"] != "" {
348+
t.Errorf("jq version = %q, want empty (Enforced shouldn't leak into version)", jqCfg["version"])
343349
}
344350

345351
// envsubst should have alias
@@ -361,6 +367,122 @@ func TestBinaryListMarshalYAML(t *testing.T) {
361367
}
362368
}
363369

370+
// TestBinaryListMarshalYAML_VersionRoundTrip is a regression test for
371+
// a bug where 'version: <x>' in b.yaml was silently dropped on save.
372+
// Unmarshal loads 'version:' into LocalBinary.Version, but the old
373+
// MarshalYAML only emitted Enforced, so a b.yaml round-trip lost the
374+
// version. The fix must emit Version as 'version:' and Enforced as
375+
// 'enforced:' (both are distinct YAML keys, not aliases).
376+
func TestBinaryListMarshalYAML_VersionRoundTrip(t *testing.T) {
377+
cases := []struct {
378+
name string
379+
in *binary.LocalBinary
380+
wantVersion string
381+
wantEnforced string
382+
}{
383+
{
384+
name: "version-only",
385+
in: &binary.LocalBinary{Name: "jq", Version: "jq-1.7"},
386+
wantVersion: "jq-1.7",
387+
wantEnforced: "",
388+
},
389+
{
390+
name: "enforced-only",
391+
in: &binary.LocalBinary{Name: "kubectl", Enforced: "v1.28.0"},
392+
wantVersion: "",
393+
wantEnforced: "v1.28.0",
394+
},
395+
{
396+
name: "both-set",
397+
in: &binary.LocalBinary{Name: "k9s", Version: "v0.32.0", Enforced: "v0.32.0"},
398+
wantVersion: "v0.32.0",
399+
wantEnforced: "v0.32.0",
400+
},
401+
{
402+
name: "oci-tag-as-version",
403+
in: &binary.LocalBinary{Name: "oci://docker", Version: "cli"},
404+
wantVersion: "cli",
405+
wantEnforced: "",
406+
},
407+
}
408+
409+
for _, tc := range cases {
410+
t.Run(tc.name, func(t *testing.T) {
411+
list := BinaryList{tc.in}
412+
result, err := list.MarshalYAML()
413+
if err != nil {
414+
t.Fatalf("MarshalYAML: %v", err)
415+
}
416+
root, ok := result.(map[string]interface{})
417+
if !ok {
418+
t.Fatalf("MarshalYAML returned %T, want map[string]interface{}", result)
419+
}
420+
raw, ok := root[tc.in.Name]
421+
if !ok {
422+
t.Fatalf("entry %q missing from MarshalYAML output: %v", tc.in.Name, root)
423+
}
424+
cfg, ok := raw.(map[string]string)
425+
if !ok {
426+
t.Fatalf("entry %q has type %T, want map[string]string (value=%v)", tc.in.Name, raw, raw)
427+
}
428+
gotVersion, gotEnforced := cfg["version"], cfg["enforced"]
429+
if gotVersion != tc.wantVersion {
430+
t.Errorf("'version:' = %q, want %q (cfg=%v)", gotVersion, tc.wantVersion, cfg)
431+
}
432+
if gotEnforced != tc.wantEnforced {
433+
t.Errorf("'enforced:' = %q, want %q (cfg=%v)", gotEnforced, tc.wantEnforced, cfg)
434+
}
435+
})
436+
}
437+
}
438+
439+
// TestBinaryListMarshalYAML_VersionSurvivesFileRoundTrip verifies that a
440+
// b.yaml written by user hand with 'version:' survives LoadConfig →
441+
// SaveConfig. This is the end-to-end regression for 'b install --add
442+
// oci://docker@cli' ending up as 'oci://docker: {}' because the tag was
443+
// dropped on save.
444+
func TestBinaryListMarshalYAML_VersionSurvivesFileRoundTrip(t *testing.T) {
445+
tmpDir := t.TempDir()
446+
configPath := tmpDir + "/b.yaml"
447+
448+
initial := `binaries:
449+
oci://docker:
450+
version: cli
451+
kubectl:
452+
version: v1.28.0
453+
`
454+
if err := os.WriteFile(configPath, []byte(initial), 0644); err != nil {
455+
t.Fatal(err)
456+
}
457+
458+
config, err := LoadConfigFromPath(configPath)
459+
if err != nil {
460+
t.Fatalf("LoadConfigFromPath: %v", err)
461+
}
462+
if err := SaveConfig(config, configPath); err != nil {
463+
t.Fatalf("SaveConfig: %v", err)
464+
}
465+
466+
out, err := os.ReadFile(configPath)
467+
if err != nil {
468+
t.Fatal(err)
469+
}
470+
got := string(out)
471+
472+
var saved struct {
473+
Binaries map[string]map[string]string `yaml:"binaries"`
474+
}
475+
if err := yaml.Unmarshal(out, &saved); err != nil {
476+
t.Fatalf("unmarshal saved yaml: %v\n%s", err, got)
477+
}
478+
if v := saved.Binaries["oci://docker"]["version"]; v != "cli" {
479+
t.Errorf("oci://docker.version = %q, want %q\nfull:\n%s", v, "cli", got)
480+
}
481+
if v := saved.Binaries["kubectl"]["version"]; v != "v1.28.0" {
482+
t.Errorf("kubectl.version = %q, want %q\nfull:\n%s", v, "v1.28.0", got)
483+
}
484+
}
485+
364486
func TestBinaryListGet(t *testing.T) {
365487
list := BinaryList{
366488
{Name: "jq"},
@@ -435,8 +557,9 @@ func TestBinaryListMarshalYAML_WithAsset(t *testing.T) {
435557
if !strings.Contains(s, "asset: argsh-so-*") {
436558
t.Errorf("marshal output missing asset field:\n%s", s)
437559
}
438-
if !strings.Contains(s, "version: v1.0.0") {
439-
t.Errorf("marshal output missing version field:\n%s", s)
560+
// Enforced now serializes as 'enforced:' not 'version:'.
561+
if !strings.Contains(s, "enforced: v1.0.0") {
562+
t.Errorf("marshal output missing enforced field:\n%s", s)
440563
}
441564
}
442565

0 commit comments

Comments
 (0)