From d867d61eb3c4999ad3a899e8945b9e2d3aea2edc Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Wed, 29 Apr 2026 14:38:59 -0300 Subject: [PATCH] update: refresh install.toml commit + built_at from new binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After `banger update` swaps binaries, install.toml needs to reflect the just-installed identity. The previous code passed buildinfo.Current().{Commit,BuiltAt} into installmeta.UpdateBuildInfo — but buildinfo.Current() in the running CLI is the OLD pre-swap binary's identity (we're it), not the staged one. install.toml's version field got refreshed to target.Version while commit and built_at stayed pinned at the previous release. `banger doctor` compares the running CLI's three fields against install.toml's three fields and so raised a false-positive drift warning on every update. Fix: after the swap, exec /usr/local/bin/banger version, parse the three-line output, and write all three fields to install.toml. If the exec fails for any reason we fall back to the old behaviour (version + stale commit/built_at) with a warning, since install.toml drift is a doctor warning not a broken host — same posture as before for the failure path. The parser is split out (parseVersionOutput) and table-tested: happy path, whitespace-tolerance, missing-field rejection, empty input rejection, ignoring unrelated lines. Caught by running v0.1.0 → v0.1.1 live as the first end-to-end smoke test of the self-update flow, which was the whole point of that exercise. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 15 +++++- internal/cli/commands_update.go | 66 ++++++++++++++++++++--- internal/cli/commands_update_test.go | 79 ++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 8 deletions(-) create mode 100644 internal/cli/commands_update_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e753034..07aba77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,18 @@ changed between versions. ## [Unreleased] +## [v0.1.2] - 2026-04-29 + +### Fixed + +- `banger update` now writes the freshly-installed binary's commit + and built_at fields to `/etc/banger/install.toml`, not the running + CLI's. Previously install.toml's `version` was correct after an + update but `commit` + `built_at` still pointed at the pre-update + binary's identity, which made `banger doctor` raise a false-positive + "CLI/install drift" warning on every update. Caught by the v0.1.0 + → v0.1.1 live update smoke-test. + ## [v0.1.1] - 2026-04-29 ### Added @@ -125,6 +137,7 @@ root filesystem and network, and exits on demand. the swap rather than starting up against an incompatible store. - Linux only. amd64 only. KVM required. -[Unreleased]: https://git.thaloco.com/thaloco/banger/compare/v0.1.1...HEAD +[Unreleased]: https://git.thaloco.com/thaloco/banger/compare/v0.1.2...HEAD +[v0.1.2]: https://git.thaloco.com/thaloco/banger/releases/tag/v0.1.2 [v0.1.1]: https://git.thaloco.com/thaloco/banger/releases/tag/v0.1.1 [v0.1.0]: https://git.thaloco.com/thaloco/banger/releases/tag/v0.1.0 diff --git a/internal/cli/commands_update.go b/internal/cli/commands_update.go index 1c0ee3f..37ae9a2 100644 --- a/internal/cli/commands_update.go +++ b/internal/cli/commands_update.go @@ -198,13 +198,20 @@ func (d *deps) runUpdate(cmd *cobra.Command, opts runUpdateOpts) error { } // Finalise: refresh install metadata, drop backups, clean staging. - info := buildinfo.Current() - // We just installed `target.Version` — info.Version still reflects - // the OLD running binary (we're it). The new bangerd encodes its - // own version; for install.toml we record what we INSTALLED. - if err := installmeta.UpdateBuildInfo(installmeta.DefaultPath, target.Version, info.Commit, info.BuiltAt); err != nil { - // Don't fail the update for this — the install is healthy; - // install.toml drift is a doctor warning, not a broken host. + // Read the new binary's identity by exec'ing it; buildinfo.Current() + // reflects the OLD running CLI (we're it), so the commit + built_at + // have to come from the freshly-swapped /usr/local/bin/banger or + // install.toml ends up with mixed-version fields. + newInfo, err := readInstalledBuildinfo(ctx, targets.Banger) + if err != nil { + fmt.Fprintf(out, "warning: read installed buildinfo: %v\n", err) + // Fall back to the manifest version + the running binary's + // commit/built_at. install.toml drift is a doctor warning, + // not a broken host, so don't fail the update. + old := buildinfo.Current() + newInfo = buildinfo.Info{Version: target.Version, Commit: old.Commit, BuiltAt: old.BuiltAt} + } + if err := installmeta.UpdateBuildInfo(installmeta.DefaultPath, newInfo.Version, newInfo.Commit, newInfo.BuiltAt); err != nil { fmt.Fprintf(out, "warning: update install metadata: %v\n", err) } if err := updater.CleanupBackups(swap); err != nil { @@ -283,6 +290,51 @@ func sanityRunStaged(ctx context.Context, staged updater.StagedRelease, expected return nil } +// readInstalledBuildinfo execs the just-swapped banger binary, parses +// its three-line `version` output, and returns the parsed identity. +// Used to refresh install.toml after an update so the on-disk record +// reflects the binary that's actually installed — buildinfo.Current() +// in the running process is the OLD binary's identity, not the one we +// just put on disk. +// +// Output shape (from internal/cli/banger.go versionString): +// +// version: vX.Y.Z +// commit: +// built_at: +func readInstalledBuildinfo(ctx context.Context, bangerPath string) (buildinfo.Info, error) { + out, err := exec.CommandContext(ctx, bangerPath, "version").Output() + if err != nil { + return buildinfo.Info{}, fmt.Errorf("exec %s version: %w", bangerPath, err) + } + return parseVersionOutput(string(out)) +} + +// parseVersionOutput extracts the three identity fields from +// `banger version`. Split out of readInstalledBuildinfo so it can be +// unit-tested without exec'ing a real binary. +func parseVersionOutput(out string) (buildinfo.Info, error) { + var info buildinfo.Info + for _, line := range strings.Split(out, "\n") { + k, v, ok := strings.Cut(line, ":") + if !ok { + continue + } + switch strings.TrimSpace(k) { + case "version": + info.Version = strings.TrimSpace(v) + case "commit": + info.Commit = strings.TrimSpace(v) + case "built_at": + info.BuiltAt = strings.TrimSpace(v) + } + } + if info.Version == "" || info.Commit == "" || info.BuiltAt == "" { + return buildinfo.Info{}, fmt.Errorf("could not parse version/commit/built_at from %q", strings.TrimSpace(out)) + } + return info, nil +} + // runPostUpdateDoctor invokes `banger doctor` on the JUST-INSTALLED // CLI (not d.doctor — that's the in-process implementation; we want // to exercise the new binary end-to-end). diff --git a/internal/cli/commands_update_test.go b/internal/cli/commands_update_test.go new file mode 100644 index 0000000..7207008 --- /dev/null +++ b/internal/cli/commands_update_test.go @@ -0,0 +1,79 @@ +package cli + +import "testing" + +func TestParseVersionOutput(t *testing.T) { + cases := []struct { + name string + in string + wantVersion string + wantCommit string + wantBuilt string + wantErr bool + }{ + { + name: "happy path — three-line shape from banger version", + in: `version: v0.1.2 +commit: a0b5c7fa3ca95a37ba99b35280fc75e5647b59e8 +built_at: 2026-04-29T17:34:45Z +`, + wantVersion: "v0.1.2", + wantCommit: "a0b5c7fa3ca95a37ba99b35280fc75e5647b59e8", + wantBuilt: "2026-04-29T17:34:45Z", + }, + { + name: "tolerates extra whitespace around the values", + in: ` version : v0.1.2 + commit : abc123 + built_at : 2026-01-01T00:00:00Z`, + wantVersion: "v0.1.2", + wantCommit: "abc123", + wantBuilt: "2026-01-01T00:00:00Z", + }, + { + name: "missing commit field is rejected", + in: "version: v0.1.2\nbuilt_at: 2026-01-01T00:00:00Z\n", + wantErr: true, + }, + { + name: "empty input is rejected", + in: "", + wantErr: true, + }, + { + name: "unrelated lines are ignored", + in: `banger v0.1.2 +some other diagnostic line: with a colon +version: v0.1.2 +commit: abc +built_at: 2026-01-01T00:00:00Z +`, + wantVersion: "v0.1.2", + wantCommit: "abc", + wantBuilt: "2026-01-01T00:00:00Z", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := parseVersionOutput(tc.in) + if tc.wantErr { + if err == nil { + t.Fatalf("want error, got nil; parsed=%+v", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.Version != tc.wantVersion { + t.Errorf("Version: got %q, want %q", got.Version, tc.wantVersion) + } + if got.Commit != tc.wantCommit { + t.Errorf("Commit: got %q, want %q", got.Commit, tc.wantCommit) + } + if got.BuiltAt != tc.wantBuilt { + t.Errorf("BuiltAt: got %q, want %q", got.BuiltAt, tc.wantBuilt) + } + }) + } +}