From c6ce6bb410540e699e697c40065d291b76a4cc17 Mon Sep 17 00:00:00 2001 From: sean Date: Thu, 15 Oct 2026 19:26:43 +0100 Subject: [PATCH] fix: a box on a newer release than the app is no longer called outdated, nor downgraded by update all The build check compared build hashes only, so a laptop on 0.3.11 took a 0.3.22 box for outdated, and Update all (or automatic updates) would have put the older berthd on it. A box now says its release in /v1/info, and berth upgrade compares releases when both are known: a newer box isn't outdated, and upgrading it needs --force. Co-Authored-By: Claude Opus 5.5 --- cmd/berth/main.go | 4 +++- cmd/berth/upgrade.go | 30 ++++++++++++++++++++---- cmd/berth/upgrade_test.go | 24 ++++++++++++++++++++ docs/changelog.mdx | 8 +++++++ docs/reference/cli.mdx | 2 +- internal/agent/outdated.go | 5 +++- internal/box/upgrade.go | 5 +++- internal/box/upgrade_test.go | 6 +++++ internal/version/version.go | 39 ++++++++++++++++++++++++++++++++ internal/version/version_test.go | 21 +++++++++++++++++ 10 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 cmd/berth/upgrade_test.go diff --git a/cmd/berth/main.go b/cmd/berth/main.go index f0910aae..dd13341e 100644 --- a/cmd/berth/main.go +++ b/cmd/berth/main.go @@ -58,7 +58,9 @@ Boxes Machines on the tailnet that could be boxes berth boxes [--json] List paired boxes and whether they are online berth ping BOX [-c N] Check a box answers and still trusts you, and its latency - berth upgrade BOX [--check] [--json] Upgrade the box's daemon over berth (no SSH); --check only reports + berth upgrade BOX [--check] [--json] [--force] + Upgrade the box's daemon over berth (no SSH); --check only reports. + A box on a newer release is left alone; --force downgrades it berth kit add|apply|list|save … Set projects up the same way on every box; see berth kit help berth team show|setup|status|retry … Set a box up the way your team's are, from /.berth on GitHub (read with gh); see berth team help diff --git a/cmd/berth/upgrade.go b/cmd/berth/upgrade.go index ce2650bb..ff883249 100644 --- a/cmd/berth/upgrade.go +++ b/cmd/berth/upgrade.go @@ -9,17 +9,19 @@ import ( "time" "github.com/cosscom/shipyard/internal/box" + "github.com/cosscom/shipyard/internal/version" ) // upgrade replaces a box's daemon with the build shipped beside this berth, // over berth's own connection: no SSH, and agent sessions keep running. func upgrade(l laptop, args []string) error { - var check bool + var check, force bool fs, asJSON, err := flags("upgrade", args, func(fs *flag.FlagSet) { fs.BoolVar(&check, "check", false, "only report whether the box runs an older daemon") + fs.BoolVar(&force, "force", false, "put this berth's daemon on the box even when the box runs a newer release") }) if err != nil || fs.NArg() != 1 { - return errors.New("usage: berth upgrade BOX [--check [--json]]") + return errors.New("usage: berth upgrade BOX [--check [--json]] [--force]") } args = fs.Args() wc, err := l.boxClient(args[0]) @@ -43,13 +45,20 @@ func upgrade(l laptop, args []string) error { return err } next := box.BuildID(binary) + // A box on a newer release than this berth isn't outdated: this berth + // is. Builds that can't be placed (dev, or a box too old to say its + // release) are told apart by their bytes alone, as before. + newer := boxIsNewer(info.Version, version.Version) if check { if asJSON { - return printJSON(map[string]any{"box": args[0], "current": info.Build, "available": next, "outdated": next != info.Build}) + return printJSON(map[string]any{"box": args[0], "current": info.Build, "available": next, "outdated": next != info.Build && !newer, "newer": newer, "version": info.Version}) } - if next == info.Build { + switch { + case next == info.Build: fmt.Printf("%s runs this build (%s).\n", args[0], next) - } else { + case newer: + fmt.Printf("%s runs berthd %s, newer than this berth's %s: update this app, not the box.\n", args[0], info.Version, version.Version) + default: fmt.Printf("%s runs %s; this berth ships %s. Upgrade with: berth upgrade %s\n", args[0], info.Build, next, args[0]) } return nil @@ -58,6 +67,9 @@ func upgrade(l laptop, args []string) error { fmt.Printf("%s already runs this build (%s).\n", args[0], next) return nil } + if newer && !force { + return fmt.Errorf("%s runs berthd %s, newer than this berth's %s: update this app instead (--force puts the older daemon on it anyway)", args[0], info.Version, version.Version) + } fmt.Printf("Uploading berthd-%s-%s to %s…\n", info.OS, info.Arch, args[0]) if err := bc.Upgrade(ctx, binary); err != nil { return err @@ -72,3 +84,11 @@ func upgrade(l laptop, args []string) error { } return fmt.Errorf("%s did not come back on the new build in time; check it with berth ping %s", args[0], args[0]) } + +// boxIsNewer says whether a box's daemon is from a newer release than this +// berth's. Either one unplaced (a dev build, or a box too old to say its +// release) is not. +func boxIsNewer(box, mine string) bool { + c, ok := version.Compare(box, mine) + return ok && c > 0 +} diff --git a/cmd/berth/upgrade_test.go b/cmd/berth/upgrade_test.go new file mode 100644 index 00000000..ae597764 --- /dev/null +++ b/cmd/berth/upgrade_test.go @@ -0,0 +1,24 @@ +package main + +import "testing" + +// A laptop on an older release never takes a newer box for outdated, nor +// puts its older daemon on it (the app's Update all asks this). +func TestABoxOnANewerReleaseIsNotOutdated(t *testing.T) { + for _, c := range []struct { + box, mine string + newer bool + }{ + {"v0.3.22", "v0.3.11", true}, + {"v0.3.11", "v0.3.22", false}, + {"v0.3.22", "v0.3.22", false}, + // Unplaced: told apart by build alone, as before. + {"", "v0.3.22", false}, + {"dev", "v0.3.11", false}, + {"v0.3.22", "dev", false}, + } { + if got := boxIsNewer(c.box, c.mine); got != c.newer { + t.Errorf("boxIsNewer(%q, %q) = %v, want %v", c.box, c.mine, got, c.newer) + } + } +} diff --git a/docs/changelog.mdx b/docs/changelog.mdx index f9d50dc8..7f0fb8b5 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -9,9 +9,17 @@ upgrades with `berth upgrade BOX`, or by running the [install command](/getting-started/add-a-box#with-the-install-command) again. +## Unreleased +- **A box newer than the app is no longer called outdated.** An older + Shipyard used to offer to update a box that already ran a newer release, + and *Update all* (or updating boxes automatically) would put its older + `berthd` on it. Now a newer box is left alone, and `berth upgrade` refuses + to downgrade one unless you pass `--force`. This works once both the app + and the box are on this release or later. ## v0.3.22 + - **Mac terminal keys work in terminals.** ⇧↵ starts a new line in Claude Code instead of sending, ⌥← and ⌥→ move a word, ⌘← and ⌘→ go to the start and end of the line, and ⌘⌫ deletes back to its start. ⇧↵ needs diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 1374b5bb..302ea2ed 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -31,7 +31,7 @@ the `BOX/` prefix. | `berth discover [--network NET] [--json]` | Machines on the tailnet that could be boxes | | `berth boxes [--json]` | Paired boxes, whether they are online, and the [route](/concepts/laptop-agent#routes-to-a-box) each is reached by | | `berth ping BOX [-c N]` | Check a box answers and still trusts you: how long connecting took, then N−1 round trips on the open connection (its latency) | -| `berth upgrade BOX [--check] [--json]` | Upgrade the box's daemon over Shipyard's own connection (no SSH); `--check` only reports whether it is older | +| `berth upgrade BOX [--check] [--json] [--force]` | Upgrade the box's daemon over Shipyard's own connection (no SSH); `--check` only reports whether it is older. A box on a newer release than this Shipyard isn't outdated and is left alone (update the app instead); `--force` puts the older daemon on it anyway | | `berth edit BOX/PROJECT[/WT] [FILE[:LINE[:COL]]] [--in cursor\|vscode\|windsurf\|zed]` | Open a worktree, or a file at a line, in your editor (default: the first installed) | | `berth ssh-config [--write] [--json]` | Show (then write) the SSH hosts editors use: `berth-` | | `berth forget BOX` | Remove a box from this laptop | diff --git a/internal/agent/outdated.go b/internal/agent/outdated.go index 11892770..c1ae83d5 100644 --- a/internal/agent/outdated.go +++ b/internal/agent/outdated.go @@ -21,7 +21,10 @@ type OutdatedBox struct { Current string `json:"current,omitempty"` Available string `json:"available,omitempty"` Outdated bool `json:"outdated"` - Error string `json:"error,omitempty"` + // Newer is a box on a newer release than this berth: not outdated, + // and never downgraded by Update all. + Newer bool `json:"newer,omitempty"` + Error string `json:"error,omitempty"` } // outdatedFresh is how long an answer stands: each check reads the box's diff --git a/internal/box/upgrade.go b/internal/box/upgrade.go index 9caecbe3..e1f93c03 100644 --- a/internal/box/upgrade.go +++ b/internal/box/upgrade.go @@ -28,6 +28,9 @@ type Info struct { OS string `json:"os"` Arch string `json:"arch"` Build string `json:"build"` + // Version is the release it is ("v0.3.22"), or "dev": a laptop on an + // older release must not take it for outdated (berth upgrade). + Version string `json:"version,omitempty"` // User and Home are the account berthd runs as, which is the one to // log in as over SSH, for editors. User string `json:"user,omitempty"` @@ -73,7 +76,7 @@ func (u *SelfUpdate) info() (Info, error) { if err != nil { return Info{}, err } - return Info{OS: runtime.GOOS, Arch: runtime.GOARCH, Build: BuildID(b), Tools: Tools()}, nil + return Info{OS: runtime.GOOS, Arch: runtime.GOARCH, Build: BuildID(b), Version: version.Version, Tools: Tools()}, nil } // Install verifies binary and swaps it in for the running executable. The old diff --git a/internal/box/upgrade_test.go b/internal/box/upgrade_test.go index 2719174e..5ee12ef3 100644 --- a/internal/box/upgrade_test.go +++ b/internal/box/upgrade_test.go @@ -7,6 +7,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/cosscom/shipyard/internal/version" ) // daemonScript is a stand-in build that answers `id` with a fingerprint. @@ -70,4 +72,8 @@ func TestInfoNamesTheAccountToLogInAs(t *testing.T) { if i.User != me.Username || i.Home != home { t.Fatalf("info = %+v, want user %s home %s", i, me.Username, home) } + // Its release, so an older laptop can tell it isn't outdated. + if i.Version != version.Version { + t.Fatalf("info says version %q, want %q", i.Version, version.Version) + } } diff --git a/internal/version/version.go b/internal/version/version.go index 16f2e2bb..4cd2d825 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -13,6 +13,8 @@ import ( "fmt" "os" "runtime" + "strconv" + "strings" ) // Version is set at link time by release builds. @@ -35,3 +37,40 @@ func Line(program string) string { } return fmt.Sprintf("%s %s (build %s, %s/%s)", program, Version, build, runtime.GOOS, runtime.GOARCH) } + +// Compare orders two releases ("v0.3.22" against "v0.3.11"): -1, 0 or 1, as +// a is older, the same or newer. ok is false unless both are releases: a +// "dev" build, or a box too old to say its version, can't be placed. +func Compare(a, b string) (c int, ok bool) { + pa, oka := parse(a) + pb, okb := parse(b) + if !oka || !okb { + return 0, false + } + for i := range pa { + if pa[i] != pb[i] { + if pa[i] < pb[i] { + return -1, true + } + return 1, true + } + } + return 0, true +} + +// parse reads "vX.Y.Z" (or "X.Y.Z"). +func parse(v string) ([3]int, bool) { + var p [3]int + parts := strings.Split(strings.TrimPrefix(v, "v"), ".") + if len(parts) != 3 { + return p, false + } + for i, s := range parts { + n, err := strconv.Atoi(s) + if err != nil || n < 0 { + return p, false + } + p[i] = n + } + return p, true +} diff --git a/internal/version/version_test.go b/internal/version/version_test.go index 4d44d3d7..7489b659 100644 --- a/internal/version/version_test.go +++ b/internal/version/version_test.go @@ -17,3 +17,24 @@ func TestLineNamesTheReleaseAndTheBuild(t *testing.T) { t.Error("build IDs must tell builds apart, in 12 hex digits") } } + +func TestReleasesAreOrderedAndDevBuildsAreNot(t *testing.T) { + for _, c := range []struct { + a, b string + want int + ok bool + }{ + {"v0.3.22", "v0.3.11", 1, true}, + {"v0.3.11", "v0.3.22", -1, true}, + {"v0.3.9", "v0.3.10", -1, true}, + {"v1.0.0", "v0.99.99", 1, true}, + {"0.3.22", "v0.3.22", 0, true}, + {"dev", "v0.3.22", 0, false}, + {"", "v0.3.22", 0, false}, + {"v0.3", "v0.3.22", 0, false}, + } { + if got, ok := Compare(c.a, c.b); got != c.want || ok != c.ok { + t.Errorf("Compare(%q, %q) = %d %v, want %d %v", c.a, c.b, got, ok, c.want, c.ok) + } + } +}