From da3f8bbc2dd4e7719d5758ff796872be0d7126cc Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Wed, 19 Aug 2026 05:33:29 +1000 Subject: [PATCH 1/9] feat(cli): warn when the connected daemon runs a different build --- cmd/gortex/proxy.go | 17 +++++++++++++++++ cmd/gortex/proxy_skew_test.go | 20 ++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 cmd/gortex/proxy_skew_test.go diff --git a/cmd/gortex/proxy.go b/cmd/gortex/proxy.go index b04bf14d5..bd51abfc8 100644 --- a/cmd/gortex/proxy.go +++ b/cmd/gortex/proxy.go @@ -119,6 +119,9 @@ func runProxy(ctx context.Context, surface *gortexmcp.ToolSurface) (ran bool, er } logProxyConnection(os.Stderr, client, false) + if warn := daemonSkewWarning(client.Ack.DaemonVersion, canonicalVersion()); warn != "" { + fmt.Fprintln(os.Stderr, "[gortex mcp] "+warn) + } if surface != nil && surface.Active() { fmt.Fprintf(os.Stderr, "[gortex mcp] tool surface restricted (preset %q)\n", surface.Preset()) } @@ -140,6 +143,20 @@ func runProxy(ctx context.Context, surface *gortexmcp.ToolSurface) (ran bool, er return true, nil } +// daemonSkewWarning returns a one-line stderr warning when the daemon +// reports a different build than this binary, or "" when they match, +// the daemon did not report a version, or this is a dev build (no +// injected identity — comparing against it would noise every dev run). +// Implements the documented intent in docs/versioning.md: the daemon +// exposes DaemonVersion so "clients can feature-gate or warn on +// mismatch"; this warns and continues — never gates. +func daemonSkewWarning(daemonVer, localVer string) string { + if daemonVer == "" || localVer == "" || localVer == "v0.0.0-dev" || daemonVer == localVer { + return "" + } + return fmt.Sprintf("warning: daemon %s != binary %s — run 'gortex daemon restart'", daemonVer, localVer) +} + func newProxyLogicalSessionID() string { var raw [16]byte if _, err := rand.Read(raw[:]); err == nil { diff --git a/cmd/gortex/proxy_skew_test.go b/cmd/gortex/proxy_skew_test.go new file mode 100644 index 000000000..fc0e19ed7 --- /dev/null +++ b/cmd/gortex/proxy_skew_test.go @@ -0,0 +1,20 @@ +package main + +import "testing" + +func TestDaemonSkewWarning(t *testing.T) { + cases := []struct{ name, daemonV, localV, want string }{ + {"equal versions", "v0.63.4+5f5fce2", "v0.63.4+5f5fce2", ""}, + {"daemon older", "v0.63.4+5f9ce2a", "v0.63.5+abcdef1", + "warning: daemon v0.63.4+5f9ce2a != binary v0.63.5+abcdef1 — run 'gortex daemon restart'"}, + {"daemon version empty", "", "v0.63.5+abcdef1", ""}, + {"local dev build", "v0.63.4+5f9ce2a", "v0.0.0-dev", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := daemonSkewWarning(tc.daemonV, tc.localV); got != tc.want { + t.Fatalf("daemonSkewWarning(%q, %q) = %q, want %q", tc.daemonV, tc.localV, got, tc.want) + } + }) + } +} From 0756a131fd0cf03878577ca3c87a36ebd54c2a8a Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Wed, 19 Aug 2026 05:38:38 +1000 Subject: [PATCH 2/9] fix(cli): direction-aware daemon skew remedy; pin daemon-newer case --- cmd/gortex/proxy.go | 99 +++++++++++++++++++++++++++++++++-- cmd/gortex/proxy_skew_test.go | 8 ++- 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/cmd/gortex/proxy.go b/cmd/gortex/proxy.go index bd51abfc8..8c84677ad 100644 --- a/cmd/gortex/proxy.go +++ b/cmd/gortex/proxy.go @@ -9,10 +9,13 @@ import ( "fmt" "os" "path/filepath" + "strconv" + "strings" "time" "github.com/zzet/gortex/internal/daemon" gortexmcp "github.com/zzet/gortex/internal/mcp" + semver "github.com/zzet/gortex/internal/version" ) // coldStartTools is the static core catalogue the proxy answers a cold-start @@ -147,14 +150,102 @@ func runProxy(ctx context.Context, surface *gortexmcp.ToolSurface) (ran bool, er // reports a different build than this binary, or "" when they match, // the daemon did not report a version, or this is a dev build (no // injected identity — comparing against it would noise every dev run). -// Implements the documented intent in docs/versioning.md: the daemon -// exposes DaemonVersion so "clients can feature-gate or warn on -// mismatch"; this warns and continues — never gates. +// The remedy is direction-aware: an older daemon should be restarted +// (a restart respawns it from this newer binary), while a newer daemon +// means this binary is the stale side and upgrading it is the fix — +// restarting would downgrade the daemon to this older build. When +// either side does not parse as semver, or only the build metadata +// differs (which SemVer precedence ignores), a generic remedy that +// covers both directions is emitted. Implements the documented intent +// in docs/versioning.md: the daemon exposes DaemonVersion so "clients +// can feature-gate or warn on mismatch"; this warns and continues — +// never gates. func daemonSkewWarning(daemonVer, localVer string) string { if daemonVer == "" || localVer == "" || localVer == "v0.0.0-dev" || daemonVer == localVer { return "" } - return fmt.Sprintf("warning: daemon %s != binary %s — run 'gortex daemon restart'", daemonVer, localVer) + base := fmt.Sprintf("warning: daemon %s != binary %s", daemonVer, localVer) + d, dErr := semver.Parse(daemonVer) + l, lErr := semver.Parse(localVer) + if dErr != nil || lErr != nil { + return base + " — run 'gortex daemon restart' or upgrade this binary" + } + switch compareSemver(d, l) { + case -1: // daemon older — restarting respawns it from this newer binary + return base + " — run 'gortex daemon restart' to upgrade the daemon" + case 1: // daemon newer — this binary is the stale side + return base + " — this binary is older than the running daemon — upgrade it (e.g. brew upgrade gortex)" + default: // same precedence, different build metadata + return base + " — run 'gortex daemon restart' or upgrade this binary" + } +} + +// compareSemver orders two Versions by SemVer 2.0.0 precedence: +// major.minor.patch numerically, then pre-release (absent outranks +// present; identifiers compared per comparePrereleaseIdent). Build +// metadata is ignored — per the spec it never affects precedence. +// Returns -1, 0, or +1. +func compareSemver(a, b semver.Version) int { + switch { + case a.Major != b.Major: + return compareInt(a.Major, b.Major) + case a.Minor != b.Minor: + return compareInt(a.Minor, b.Minor) + case a.Patch != b.Patch: + return compareInt(a.Patch, b.Patch) + case a.Prerelease == b.Prerelease: + return 0 + case a.Prerelease == "": + return 1 // the release outranks any of its pre-releases + case b.Prerelease == "": + return -1 + default: + return comparePrerelease(a.Prerelease, b.Prerelease) + } +} + +// comparePrerelease compares two dot-separated pre-release identifier +// lists per SemVer 2.0.0: identifier by identifier, and once every +// shared identifier is equal, the shorter list ranks below the longer. +func comparePrerelease(a, b string) int { + as := strings.Split(a, ".") + bs := strings.Split(b, ".") + for i := 0; i < len(as) && i < len(bs); i++ { + if c := comparePrereleaseIdent(as[i], bs[i]); c != 0 { + return c + } + } + return compareInt(len(as), len(bs)) +} + +// comparePrereleaseIdent compares one pre-release identifier pair: +// numeric identifiers compare numerically and rank below alphanumeric +// ones; alphanumeric identifiers compare in ASCII sort order. +func comparePrereleaseIdent(a, b string) int { + an, aErr := strconv.Atoi(a) + bn, bErr := strconv.Atoi(b) + switch { + case aErr == nil && bErr == nil: + return compareInt(an, bn) + case aErr == nil: + return -1 // numeric identifiers rank below alphanumeric + case bErr == nil: + return 1 + default: + return strings.Compare(a, b) + } +} + +// compareInt is the three-way integer compare the semver helpers use. +func compareInt(a, b int) int { + switch { + case a < b: + return -1 + case a > b: + return 1 + default: + return 0 + } } func newProxyLogicalSessionID() string { diff --git a/cmd/gortex/proxy_skew_test.go b/cmd/gortex/proxy_skew_test.go index fc0e19ed7..7655e24f3 100644 --- a/cmd/gortex/proxy_skew_test.go +++ b/cmd/gortex/proxy_skew_test.go @@ -6,7 +6,13 @@ func TestDaemonSkewWarning(t *testing.T) { cases := []struct{ name, daemonV, localV, want string }{ {"equal versions", "v0.63.4+5f5fce2", "v0.63.4+5f5fce2", ""}, {"daemon older", "v0.63.4+5f9ce2a", "v0.63.5+abcdef1", - "warning: daemon v0.63.4+5f9ce2a != binary v0.63.5+abcdef1 — run 'gortex daemon restart'"}, + "warning: daemon v0.63.4+5f9ce2a != binary v0.63.5+abcdef1 — run 'gortex daemon restart' to upgrade the daemon"}, + {"daemon newer", "v0.63.5+abcdef1", "v0.63.4+5f9ce2a", + "warning: daemon v0.63.5+abcdef1 != binary v0.63.4+5f9ce2a — this binary is older than the running daemon — upgrade it (e.g. brew upgrade gortex)"}, + {"same version different build", "v0.63.4+aaaaaaa", "v0.63.4+bbbbbbb", + "warning: daemon v0.63.4+aaaaaaa != binary v0.63.4+bbbbbbb — run 'gortex daemon restart' or upgrade this binary"}, + {"unparseable daemon version", "strawberry", "v0.63.5+abcdef1", + "warning: daemon strawberry != binary v0.63.5+abcdef1 — run 'gortex daemon restart' or upgrade this binary"}, {"daemon version empty", "", "v0.63.5+abcdef1", ""}, {"local dev build", "v0.63.4+5f9ce2a", "v0.0.0-dev", ""}, } From 872723efc4789082b26e1a65e95b9be4f01e5080 Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Wed, 19 Aug 2026 05:55:36 +1000 Subject: [PATCH 3/9] feat(daemon): self-report on-disk binary drift in status --- internal/daemon/proto.go | 14 ++ internal/daemon/server.go | 101 ++++++++++++-- internal/daemon/server_stale_test.go | 188 +++++++++++++++++++++++++++ 3 files changed, 295 insertions(+), 8 deletions(-) create mode 100644 internal/daemon/server_stale_test.go diff --git a/internal/daemon/proto.go b/internal/daemon/proto.go index f34cf3882..0436a9e8c 100644 --- a/internal/daemon/proto.go +++ b/internal/daemon/proto.go @@ -365,6 +365,20 @@ type StatusResponse struct { // every alive (spec, workspace) subprocess. Empty when no LSP // router is wired (`semantic.enabled: false` in `.gortex.yaml`). LSPRouter *LSPRouterStatus `json:"lsp_router,omitempty"` + + // BinaryStale is true when the file at the daemon's os.Executable() + // path no longer matches the image the daemon is running (size or + // mtime differ) — the signature of a package-manager upgrade that + // replaced the binary under a running daemon. Stat failures leave + // this false and BinaryChecked false (unknown, not fresh). + BinaryStale bool `json:"binary_stale,omitempty"` + // BinaryChecked reports whether the binary-drift probe actually ran: + // false means the daemon could not establish the on-disk state (no + // captured start identity, or the status-time stat failed) and the + // binary is unknown — not fresh. + BinaryChecked bool `json:"binary_checked,omitempty"` + // BinaryReplacedAtUnix is the on-disk file's mtime when stale. + BinaryReplacedAtUnix int64 `json:"binary_replaced_at_unix,omitempty"` } // LSPRouterStatus reflects one daemon's LSP-router state for the diff --git a/internal/daemon/server.go b/internal/daemon/server.go index e3b42d649..42521eb1f 100644 --- a/internal/daemon/server.go +++ b/internal/daemon/server.go @@ -76,6 +76,19 @@ type Server struct { started time.Time instanceID string // unique to this daemon process; exposed in handshake acks + // Binary-drift detection: size+mtime of the daemon's own executable + // captured at construction, so each status request can cheaply tell + // whether the on-disk image was replaced (brew upgrade, cp over the + // binary) while this process keeps running the old code. An empty + // binaryPath means the identity was never captured and status reports + // the binary state as unknown. binaryStatFn is swappable for tests. + binaryMu sync.Mutex + binaryPath string + binaryStartSize int64 + binaryStartMod int64 + binaryStatFn func(path string) (int64, int64, error) // size, mtime unix seconds, err + binaryLoggedStale bool + shutdown chan struct{} doneOnce sync.Once conns map[net.Conn]struct{} @@ -173,14 +186,83 @@ func New(socketPath, version string, logger *zap.Logger) *Server { if logger == nil { logger = zap.NewNop() } - return &Server{ - SocketPath: socketPath, - Version: version, - Logger: logger, - instanceID: newSessionID(), - sessions: NewSessionRegistry(), - shutdown: make(chan struct{}), - conns: make(map[net.Conn]struct{}), + s := &Server{ + SocketPath: socketPath, + Version: version, + Logger: logger, + instanceID: newSessionID(), + sessions: NewSessionRegistry(), + shutdown: make(chan struct{}), + conns: make(map[net.Conn]struct{}), + binaryStatFn: osStatIdentity, + } + // Capture the running image's identity so status requests can detect + // a later on-disk replace. Best-effort: a failure here leaves + // binaryPath empty and status reports the binary state as unknown. + if exe, err := os.Executable(); err == nil { + s.captureBinaryIdentity(exe) + } + return s +} + +// osStatIdentity is the production binary-identity probe: os.Stat reduced +// to the (size, mtime) pair drift detection compares. A named func rather +// than an inline closure so Server.binaryStatFn has a swappable default +// and tests can substitute a fake. +func osStatIdentity(path string) (int64, int64, error) { + fi, err := os.Stat(path) + if err != nil { + return 0, 0, err + } + return fi.Size(), fi.ModTime().Unix(), nil +} + +// captureBinaryIdentity stats path through binaryStatFn and records the +// result as the start-of-life identity. Any error leaves the identity +// uncaptured (binaryPath empty) — status then reports the binary state as +// unknown rather than guessing fresh. +func (s *Server) captureBinaryIdentity(path string) { + if s.binaryStatFn == nil { + s.binaryStatFn = osStatIdentity + } + size, mod, err := s.binaryStatFn(path) + if err != nil { + return + } + s.binaryMu.Lock() + s.binaryPath = path + s.binaryStartSize = size + s.binaryStartMod = mod + s.binaryMu.Unlock() +} + +// populateBinaryStatus stamps the binary-drift fields onto a status +// response: BinaryChecked reports whether the probe ran, BinaryStale +// whether the on-disk image no longer matches the one this process +// started from. The first stale detection logs the restart hint once — +// every subsequent status stays quiet, so a monitoring loop polling status +// cannot spam the daemon log. Stat failures (and a never-captured +// identity) leave both flags false: unknown, not fresh. +func (s *Server) populateBinaryStatus(st *StatusResponse) { + s.binaryMu.Lock() + defer s.binaryMu.Unlock() + if s.binaryPath == "" || s.binaryStatFn == nil { + return + } + size, mod, err := s.binaryStatFn(s.binaryPath) + if err != nil { + return + } + st.BinaryChecked = true + if size != s.binaryStartSize || mod != s.binaryStartMod { + st.BinaryStale = true + st.BinaryReplacedAtUnix = mod + if !s.binaryLoggedStale { + s.binaryLoggedStale = true + s.Logger.Warn(fmt.Sprintf( + "daemon: on-disk binary changed since start (%s) — run 'gortex daemon restart' to upgrade", + s.binaryPath)) + } } } @@ -734,6 +816,9 @@ func (s *Server) handleControl(ctx context.Context, _ *Session, req ControlReque } st.MCPSessions = rows } + // Binary-drift self-report: did the on-disk daemon image get + // replaced under this running process? See populateBinaryStatus. + s.populateBinaryStatus(&st) buf, _ := json.Marshal(st) return ControlResponse{OK: true, Result: buf} diff --git a/internal/daemon/server_stale_test.go b/internal/daemon/server_stale_test.go new file mode 100644 index 000000000..4fa120377 --- /dev/null +++ b/internal/daemon/server_stale_test.go @@ -0,0 +1,188 @@ +package daemon + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +// statusOnlyController satisfies just the Status slice of Controller. +// Embedding the nil interface keeps the stub one line: handleControl only +// touches Status for a ControlStatus request, so the unimplemented methods +// are never reached. +type statusOnlyController struct{ Controller } + +func (statusOnlyController) Status(context.Context) (StatusResponse, error) { + return StatusResponse{}, nil +} + +// fakeBinaryStat is a swappable binaryStatFn over a real temp file: the test +// flips fail to simulate the status-time stat error path. +type fakeBinaryStat struct { + fail bool +} + +func (f *fakeBinaryStat) stat(path string) (int64, int64, error) { + if f.fail { + return 0, 0, errors.New("stat failed") + } + return osStatIdentity(path) +} + +// newBinaryStaleTestServer builds a Server whose binary identity points at +// a temp file standing in for the daemon executable, captured through the +// fake statFn exactly the way New captures the real one. +func newBinaryStaleTestServer(t *testing.T, logger *zap.Logger) (*Server, *fakeBinaryStat, string) { + t.Helper() + dir := t.TempDir() + binPath := filepath.Join(dir, "gortex") + if err := os.WriteFile(binPath, []byte("fake daemon image"), 0o755); err != nil { + t.Fatal(err) + } + s := New(filepath.Join(dir, "sock"), "test", logger) + fake := &fakeBinaryStat{} + s.binaryStatFn = fake.stat + s.captureBinaryIdentity(binPath) + if s.binaryPath != binPath { + t.Fatalf("binary identity not captured: binaryPath=%q", s.binaryPath) + } + s.Controller = statusOnlyController{} + return s, fake, binPath +} + +// requestStatus drives one ControlStatus request through the real handler +// and decodes the StatusResponse it returns. +func requestStatus(t *testing.T, s *Server) StatusResponse { + t.Helper() + resp := s.handleControl(context.Background(), nil, ControlRequest{Kind: ControlStatus}) + if !resp.OK { + t.Fatalf("control status failed: %s: %s", resp.ErrorCode, resp.ErrorMsg) + } + var st StatusResponse + if err := json.Unmarshal(resp.Result, &st); err != nil { + t.Fatalf("unmarshal status result: %v", err) + } + return st +} + +func staleWarningCount(logs *observer.ObservedLogs) int { + n := 0 + for _, e := range logs.All() { + if strings.Contains(e.Message, "on-disk binary changed") { + n++ + } + } + return n +} + +// TestStatusBinaryDriftFreshThenStale walks the full lifecycle through the +// real ControlStatus handler: fresh at start, stale after an on-disk +// replace (mtime drift), the restart hint logged exactly once, and the +// stale flag stable on the poll that follows. +func TestStatusBinaryDriftFreshThenStale(t *testing.T) { + core, logs := observer.New(zap.WarnLevel) + s, _, binPath := newBinaryStaleTestServer(t, zap.New(core)) + + // Fresh: the on-disk image still matches the one captured at start. + st := requestStatus(t, s) + if !st.BinaryChecked || st.BinaryStale { + t.Fatalf("fresh status: BinaryChecked=%v BinaryStale=%v, want true/false", st.BinaryChecked, st.BinaryStale) + } + if st.BinaryReplacedAtUnix != 0 { + t.Fatalf("fresh status: BinaryReplacedAtUnix=%d, want 0", st.BinaryReplacedAtUnix) + } + if n := staleWarningCount(logs); n != 0 { + t.Fatalf("fresh status: %d stale warnings, want 0", n) + } + + // Replace the image under the running daemon: future mtime, same size. + future := time.Now().Add(2 * time.Hour) + if err := os.Chtimes(binPath, future, future); err != nil { + t.Fatal(err) + } + st = requestStatus(t, s) + if !st.BinaryChecked || !st.BinaryStale { + t.Fatalf("stale status: BinaryChecked=%v BinaryStale=%v, want true/true", st.BinaryChecked, st.BinaryStale) + } + if st.BinaryReplacedAtUnix != future.Unix() { + t.Fatalf("stale status: BinaryReplacedAtUnix=%d, want %d", st.BinaryReplacedAtUnix, future.Unix()) + } + if n := staleWarningCount(logs); n != 1 { + t.Fatalf("first stale detection: %d stale warnings, want 1", n) + } + if !s.binaryLoggedStale { + t.Fatal("first stale detection: binaryLoggedStale once-flag not set") + } + + // The next stale poll must not log again — a monitoring loop polling + // status must not spam the daemon log. + st = requestStatus(t, s) + if !st.BinaryChecked || !st.BinaryStale { + t.Fatalf("second stale status: BinaryChecked=%v BinaryStale=%v, want true/true", st.BinaryChecked, st.BinaryStale) + } + if n := staleWarningCount(logs); n != 1 { + t.Fatalf("second stale status: %d stale warnings, want still 1", n) + } +} + +// TestStatusBinaryDriftSizeChange exercises the size half of the identity: +// a grown image with the original mtime restored is still drift. +func TestStatusBinaryDriftSizeChange(t *testing.T) { + s, _, binPath := newBinaryStaleTestServer(t, nil) + startMod := s.binaryStartMod + + f, err := os.OpenFile(binPath, os.O_APPEND|os.O_WRONLY, 0o755) + if err != nil { + t.Fatal(err) + } + if _, err := f.WriteString("-v2-now-longer"); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + // Restore the captured mtime so only the size differs. + if err := os.Chtimes(binPath, time.Unix(startMod, 0), time.Unix(startMod, 0)); err != nil { + t.Fatal(err) + } + + var st StatusResponse + s.populateBinaryStatus(&st) + if !st.BinaryChecked || !st.BinaryStale { + t.Fatalf("size drift: BinaryChecked=%v BinaryStale=%v, want true/true", st.BinaryChecked, st.BinaryStale) + } +} + +// TestStatusBinaryStatErrorReportsUnknown: when the status-time stat fails, +// the daemon must report unknown (both flags false), never fresh. +func TestStatusBinaryStatErrorReportsUnknown(t *testing.T) { + s, fake, _ := newBinaryStaleTestServer(t, nil) + fake.fail = true + + var st StatusResponse + s.populateBinaryStatus(&st) + if st.BinaryChecked || st.BinaryStale { + t.Fatalf("stat error: BinaryChecked=%v BinaryStale=%v, want false/false (unknown, not fresh)", st.BinaryChecked, st.BinaryStale) + } +} + +// TestStatusBinaryUncapturedIdentityReportsUnknown: a Server whose startup +// capture failed (empty binaryPath) reports unknown on every status. +func TestStatusBinaryUncapturedIdentityReportsUnknown(t *testing.T) { + s := New(filepath.Join(t.TempDir(), "sock"), "test", nil) + s.binaryPath = "" // simulate the constructor-capture failure path + + var st StatusResponse + s.populateBinaryStatus(&st) + if st.BinaryChecked || st.BinaryStale { + t.Fatalf("uncaptured identity: BinaryChecked=%v BinaryStale=%v, want false/false", st.BinaryChecked, st.BinaryStale) + } +} From 594ee105c45a19de6f890c7b661a5464ade9f1e7 Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Wed, 19 Aug 2026 06:16:44 +1000 Subject: [PATCH 4/9] feat(cli): surface daemon skew and staleness in daemon status --- cmd/gortex/daemon.go | 14 ++++ cmd/gortex/daemon_status_render_test.go | 97 +++++++++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/cmd/gortex/daemon.go b/cmd/gortex/daemon.go index 36106c01c..2af88d4da 100644 --- a/cmd/gortex/daemon.go +++ b/cmd/gortex/daemon.go @@ -1250,6 +1250,20 @@ func renderDaemonHeader(w io.Writer, st daemon.StatusResponse) { }) t.AppendRow(table.Row{"daemon", st.Version}) t.AppendRow(table.Row{"pid", st.PID}) + // Upgrade-skew facts belong next to the daemon version they qualify. + // The cli row appears only when this binary's build differs from the + // daemon's — the same compare runProxy warns on at connect time — so + // a matching pair keeps the table exactly as terse as before. The + // binary row surfaces the daemon's own on-disk drift probe and only + // when it actually ran: an unchecked binary is unknown, not fresh, + // and must not be reported as either. + local := canonicalVersion() + if warn := daemonSkewWarning(st.Version, local); warn != "" { + t.AppendRow(table.Row{"cli", local + " (differs from daemon — see warning)"}) + } + if st.BinaryChecked && st.BinaryStale { + t.AppendRow(table.Row{"binary", "stale — on-disk image newer than running image; run 'gortex daemon restart'"}) + } t.AppendRow(table.Row{"socket", st.SocketPath}) t.AppendRow(table.Row{"uptime", formatDuration(time.Duration(st.UptimeSeconds) * time.Second)}) switch { diff --git a/cmd/gortex/daemon_status_render_test.go b/cmd/gortex/daemon_status_render_test.go index a20ed2215..490057871 100644 --- a/cmd/gortex/daemon_status_render_test.go +++ b/cmd/gortex/daemon_status_render_test.go @@ -258,3 +258,100 @@ func TestRenderDaemonHeader_ReadyAndEnriched_NoWarmupLabelChange(t *testing.T) { assert.Contains(t, out, "ready (warmup 5m0s)") assert.NotContains(t, out, "enrichment in progress") } + +// stubBuildVersion rewrites the ldflags-injected build identity for the +// duration of a subtest — the same `-X main.version` / `-X main.commit` +// seam goreleaser populates — so canonicalVersion() reports a chosen +// build and renderDaemonHeader's skew compare can be driven end-to-end. +func stubBuildVersion(t *testing.T, v, c string) { + t.Helper() + oldV, oldC := version, commit + version, commit = v, c + t.Cleanup(func() { version, commit = oldV, oldC }) +} + +// TestRenderDaemonHeader_SkewRow — the local-version row appears only +// when daemonSkewWarning(st.Version, canonicalVersion()) is non-empty, +// the same compare runProxy applies at connect time. Matching versions +// and dev builds (no injected identity, the v0.0.0-dev sentinel) must +// keep the table exactly as terse as it was. +func TestRenderDaemonHeader_SkewRow(t *testing.T) { + t.Run("skewed versions append the cli row", func(t *testing.T) { + stubBuildVersion(t, "0.63.3", "deadbee") + st := daemon.StatusResponse{Version: "v0.63.4+abc1234"} + // Precondition: the row is gated on this exact compare, so prove + // the gate is live before asserting the render honors it. + if daemonSkewWarning(st.Version, canonicalVersion()) == "" { + t.Fatalf("expected daemonSkewWarning(%q, %q) to be non-empty", + st.Version, canonicalVersion()) + } + var buf bytes.Buffer + renderDaemonHeader(&buf, st) + out := buf.String() + assert.Contains(t, out, "cli") + assert.Contains(t, out, "v0.63.3+deadbee (differs from daemon") + }) + + t.Run("matching versions omit the row", func(t *testing.T) { + stubBuildVersion(t, "0.63.4", "abc1234") + st := daemon.StatusResponse{Version: "v0.63.4+abc1234"} + if daemonSkewWarning(st.Version, canonicalVersion()) != "" { + t.Fatalf("expected daemonSkewWarning(%q, %q) to be empty", + st.Version, canonicalVersion()) + } + var buf bytes.Buffer + renderDaemonHeader(&buf, st) + assert.NotContains(t, buf.String(), "cli") + }) + + t.Run("dev build omits the row", func(t *testing.T) { + // Plain `go build` identity: canonicalVersion() reports the + // v0.0.0-dev sentinel, which daemonSkewWarning deliberately + // ignores so dev binaries never nag about skew. + stubBuildVersion(t, "0.0.0", "") + assert.Equal(t, "v0.0.0-dev", canonicalVersion()) + st := daemon.StatusResponse{Version: "v0.63.4+abc1234"} + var buf bytes.Buffer + renderDaemonHeader(&buf, st) + assert.NotContains(t, buf.String(), "cli") + }) +} + +// TestRenderDaemonHeader_BinaryRow — the daemon's self-reported +// on-disk-binary drift row appears only when the drift probe ran +// (BinaryChecked) and found the running image stale. An unchecked binary +// must never render as stale — unknown is not stale. +func TestRenderDaemonHeader_BinaryRow(t *testing.T) { + t.Run("stale binary appends the binary row", func(t *testing.T) { + st := daemon.StatusResponse{ + Version: "v0.63.4+abc1234", + BinaryChecked: true, + BinaryStale: true, + } + var buf bytes.Buffer + renderDaemonHeader(&buf, st) + out := buf.String() + assert.Contains(t, out, "binary") + assert.Contains(t, out, "stale — on-disk image newer than running image") + }) + + t.Run("fresh binary omits the row", func(t *testing.T) { + st := daemon.StatusResponse{ + Version: "v0.63.4+abc1234", + BinaryChecked: true, + } + var buf bytes.Buffer + renderDaemonHeader(&buf, st) + assert.NotContains(t, buf.String(), "binary") + }) + + t.Run("unchecked binary omits the row even if BinaryStale is set", func(t *testing.T) { + st := daemon.StatusResponse{ + Version: "v0.63.4+abc1234", + BinaryStale: true, + } + var buf bytes.Buffer + renderDaemonHeader(&buf, st) + assert.NotContains(t, buf.String(), "binary") + }) +} From 824c71ed002a98cc6737ddc0636582ff71b1e672 Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Wed, 19 Aug 2026 06:21:37 +1000 Subject: [PATCH 5/9] fix(cli): drop dangling 'see warning' tail from the cli status row --- cmd/gortex/daemon.go | 2 +- cmd/gortex/daemon_status_render_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/gortex/daemon.go b/cmd/gortex/daemon.go index 2af88d4da..5da3546c6 100644 --- a/cmd/gortex/daemon.go +++ b/cmd/gortex/daemon.go @@ -1259,7 +1259,7 @@ func renderDaemonHeader(w io.Writer, st daemon.StatusResponse) { // and must not be reported as either. local := canonicalVersion() if warn := daemonSkewWarning(st.Version, local); warn != "" { - t.AppendRow(table.Row{"cli", local + " (differs from daemon — see warning)"}) + t.AppendRow(table.Row{"cli", local + " (differs from daemon)"}) } if st.BinaryChecked && st.BinaryStale { t.AppendRow(table.Row{"binary", "stale — on-disk image newer than running image; run 'gortex daemon restart'"}) diff --git a/cmd/gortex/daemon_status_render_test.go b/cmd/gortex/daemon_status_render_test.go index 490057871..a22f74d90 100644 --- a/cmd/gortex/daemon_status_render_test.go +++ b/cmd/gortex/daemon_status_render_test.go @@ -289,7 +289,7 @@ func TestRenderDaemonHeader_SkewRow(t *testing.T) { renderDaemonHeader(&buf, st) out := buf.String() assert.Contains(t, out, "cli") - assert.Contains(t, out, "v0.63.3+deadbee (differs from daemon") + assert.Contains(t, out, "v0.63.3+deadbee (differs from daemon)") }) t.Run("matching versions omit the row", func(t *testing.T) { From c4676ab62b46c9f87220f49c0a91a9c6bff56167 Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Wed, 19 Aug 2026 08:39:40 +1000 Subject: [PATCH 6/9] =?UTF-8?q?test(cli):=20pin=20SemVer=20=C2=A711=20prer?= =?UTF-8?q?elease=20precedence;=20silence=20dev-daemon=20skew?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/gortex/proxy.go | 8 ++++--- cmd/gortex/proxy_skew_test.go | 45 ++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/cmd/gortex/proxy.go b/cmd/gortex/proxy.go index 8c84677ad..a482440bc 100644 --- a/cmd/gortex/proxy.go +++ b/cmd/gortex/proxy.go @@ -148,8 +148,10 @@ func runProxy(ctx context.Context, surface *gortexmcp.ToolSurface) (ran bool, er // daemonSkewWarning returns a one-line stderr warning when the daemon // reports a different build than this binary, or "" when they match, -// the daemon did not report a version, or this is a dev build (no -// injected identity — comparing against it would noise every dev run). +// the daemon did not report a version, or either side is a dev build +// (no injected identity — comparing against a dev build would noise +// every dev run, and a dev-built daemon cannot be "upgraded" by a +// restart, so the remedy advice would be wrong for it too). // The remedy is direction-aware: an older daemon should be restarted // (a restart respawns it from this newer binary), while a newer daemon // means this binary is the stale side and upgrading it is the fix — @@ -161,7 +163,7 @@ func runProxy(ctx context.Context, surface *gortexmcp.ToolSurface) (ran bool, er // can feature-gate or warn on mismatch"; this warns and continues — // never gates. func daemonSkewWarning(daemonVer, localVer string) string { - if daemonVer == "" || localVer == "" || localVer == "v0.0.0-dev" || daemonVer == localVer { + if daemonVer == "" || localVer == "" || localVer == "v0.0.0-dev" || daemonVer == "v0.0.0-dev" || daemonVer == localVer { return "" } base := fmt.Sprintf("warning: daemon %s != binary %s", daemonVer, localVer) diff --git a/cmd/gortex/proxy_skew_test.go b/cmd/gortex/proxy_skew_test.go index 7655e24f3..bd179172d 100644 --- a/cmd/gortex/proxy_skew_test.go +++ b/cmd/gortex/proxy_skew_test.go @@ -1,6 +1,10 @@ package main -import "testing" +import ( + "testing" + + semver "github.com/zzet/gortex/internal/version" +) func TestDaemonSkewWarning(t *testing.T) { cases := []struct{ name, daemonV, localV, want string }{ @@ -15,6 +19,7 @@ func TestDaemonSkewWarning(t *testing.T) { "warning: daemon strawberry != binary v0.63.5+abcdef1 — run 'gortex daemon restart' or upgrade this binary"}, {"daemon version empty", "", "v0.63.5+abcdef1", ""}, {"local dev build", "v0.63.4+5f9ce2a", "v0.0.0-dev", ""}, + {"daemon dev build", "v0.0.0-dev", "v0.63.5+abcdef1", ""}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -24,3 +29,41 @@ func TestDaemonSkewWarning(t *testing.T) { }) } } + +// TestCompareSemverPrecedence pins the SemVer 2.0.0 §11 precedence +// chain on compareSemver, which the release-only table above never +// exercised: the canonical ascending pre-release ladder (each adjacent +// pair in spec order), build metadata being ignored for precedence +// (§10), and numeric identifiers ranking below alphanumeric ones (§11). +// Each row asserts the comparator's exact sign (-1 / 0 / +1) in the +// argument order given. +func TestCompareSemverPrecedence(t *testing.T) { + cases := []struct { + name string + a, b string + want int + }{ + // §11's ascending example chain, adjacent pair by adjacent pair. + {"alpha lt alpha.1 (larger field set ranks higher)", "v1.0.0-alpha", "v1.0.0-alpha.1", -1}, + {"alpha.1 lt alpha.beta (numeric lt alphanumeric)", "v1.0.0-alpha.1", "v1.0.0-alpha.beta", -1}, + {"alpha.beta lt beta (ASCII sort)", "v1.0.0-alpha.beta", "v1.0.0-beta", -1}, + {"beta lt beta.2 (larger field set ranks higher)", "v1.0.0-beta", "v1.0.0-beta.2", -1}, + {"beta.2 lt beta.11 (numeric compare, not ASCII)", "v1.0.0-beta.2", "v1.0.0-beta.11", -1}, + {"beta.11 lt rc.1 (ASCII sort)", "v1.0.0-beta.11", "v1.0.0-rc.1", -1}, + {"rc.1 lt release (pre-release ranks below release)", "v1.0.0-rc.1", "v1.0.0", -1}, + {"release gt rc.1 (same pair, reversed)", "v1.0.0", "v1.0.0-rc.1", 1}, + // §10: build metadata MUST be ignored when determining precedence. + {"build metadata ignored", "v1.0.0+a", "v1.0.0+b", 0}, + // §11: numeric identifiers always rank below alphanumeric ones. + {"bare numeric ident lt alphanumeric ident", "v1.0.0-1", "v1.0.0-alpha", -1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + a := semver.MustParse(tc.a) + b := semver.MustParse(tc.b) + if got := compareSemver(a, b); got != tc.want { + t.Fatalf("compareSemver(%s, %s) = %d, want %d", tc.a, tc.b, got, tc.want) + } + }) + } +} From 352131a291356f8977e0e4a345ba490133c77b42 Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Thu, 20 Aug 2026 10:44:30 +1000 Subject: [PATCH 7/9] fix(mcp): skew remedy names 'gortex upgrade', not brew-only advice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review #4969353348 (r3810847325): 'brew upgrade gortex' only works for macOS brew installs — curl-installed and scoop users get advice that fails. 'gortex upgrade' detects the install method and runs the matching update, and it stops/starts the daemon around the swap itself. --- cmd/gortex/proxy.go | 12 ++++++++---- cmd/gortex/proxy_skew_test.go | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/cmd/gortex/proxy.go b/cmd/gortex/proxy.go index a482440bc..90a219fa1 100644 --- a/cmd/gortex/proxy.go +++ b/cmd/gortex/proxy.go @@ -155,7 +155,11 @@ func runProxy(ctx context.Context, surface *gortexmcp.ToolSurface) (ran bool, er // The remedy is direction-aware: an older daemon should be restarted // (a restart respawns it from this newer binary), while a newer daemon // means this binary is the stale side and upgrading it is the fix — -// restarting would downgrade the daemon to this older build. When +// restarting would downgrade the daemon to this older build. The +// upgrade remedy is 'gortex upgrade' rather than a package-manager +// specific command: it upgrades the way gortex was installed (brew, +// scoop, go install, or the install script) and restarts the daemon +// around the binary swap. When // either side does not parse as semver, or only the build metadata // differs (which SemVer precedence ignores), a generic remedy that // covers both directions is emitted. Implements the documented intent @@ -170,15 +174,15 @@ func daemonSkewWarning(daemonVer, localVer string) string { d, dErr := semver.Parse(daemonVer) l, lErr := semver.Parse(localVer) if dErr != nil || lErr != nil { - return base + " — run 'gortex daemon restart' or upgrade this binary" + return base + " — run 'gortex daemon restart' or 'gortex upgrade'" } switch compareSemver(d, l) { case -1: // daemon older — restarting respawns it from this newer binary return base + " — run 'gortex daemon restart' to upgrade the daemon" case 1: // daemon newer — this binary is the stale side - return base + " — this binary is older than the running daemon — upgrade it (e.g. brew upgrade gortex)" + return base + " — this binary is older than the running daemon — run 'gortex upgrade'" default: // same precedence, different build metadata - return base + " — run 'gortex daemon restart' or upgrade this binary" + return base + " — run 'gortex daemon restart' or 'gortex upgrade'" } } diff --git a/cmd/gortex/proxy_skew_test.go b/cmd/gortex/proxy_skew_test.go index bd179172d..f9438e653 100644 --- a/cmd/gortex/proxy_skew_test.go +++ b/cmd/gortex/proxy_skew_test.go @@ -12,11 +12,11 @@ func TestDaemonSkewWarning(t *testing.T) { {"daemon older", "v0.63.4+5f9ce2a", "v0.63.5+abcdef1", "warning: daemon v0.63.4+5f9ce2a != binary v0.63.5+abcdef1 — run 'gortex daemon restart' to upgrade the daemon"}, {"daemon newer", "v0.63.5+abcdef1", "v0.63.4+5f9ce2a", - "warning: daemon v0.63.5+abcdef1 != binary v0.63.4+5f9ce2a — this binary is older than the running daemon — upgrade it (e.g. brew upgrade gortex)"}, + "warning: daemon v0.63.5+abcdef1 != binary v0.63.4+5f9ce2a — this binary is older than the running daemon — run 'gortex upgrade'"}, {"same version different build", "v0.63.4+aaaaaaa", "v0.63.4+bbbbbbb", - "warning: daemon v0.63.4+aaaaaaa != binary v0.63.4+bbbbbbb — run 'gortex daemon restart' or upgrade this binary"}, + "warning: daemon v0.63.4+aaaaaaa != binary v0.63.4+bbbbbbb — run 'gortex daemon restart' or 'gortex upgrade'"}, {"unparseable daemon version", "strawberry", "v0.63.5+abcdef1", - "warning: daemon strawberry != binary v0.63.5+abcdef1 — run 'gortex daemon restart' or upgrade this binary"}, + "warning: daemon strawberry != binary v0.63.5+abcdef1 — run 'gortex daemon restart' or 'gortex upgrade'"}, {"daemon version empty", "", "v0.63.5+abcdef1", ""}, {"local dev build", "v0.63.4+5f9ce2a", "v0.0.0-dev", ""}, {"daemon dev build", "v0.0.0-dev", "v0.63.5+abcdef1", ""}, From 205b2e2e0301101644ae480e151b8d9b28c917ce Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Thu, 20 Aug 2026 10:55:40 +1000 Subject: [PATCH 8/9] refactor(version): move semver precedence compare into internal/version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review #4969353348 (r3810812621): the comparators outgrew the proxy — internal/version parsed semver but could not order it, so Compare fills a package gap instead of living as cmd/gortex-private helpers. The §11 precedence test moves with it (TestComparePrecedence); the skew remedy stays in cmd/gortex as CLI messaging. --- cmd/gortex/proxy.go | 72 +------------------------------ cmd/gortex/proxy_skew_test.go | 44 +------------------ internal/version/compare.go | 74 ++++++++++++++++++++++++++++++++ internal/version/compare_test.go | 40 +++++++++++++++++ 4 files changed, 116 insertions(+), 114 deletions(-) create mode 100644 internal/version/compare.go create mode 100644 internal/version/compare_test.go diff --git a/cmd/gortex/proxy.go b/cmd/gortex/proxy.go index 90a219fa1..4d74f23e5 100644 --- a/cmd/gortex/proxy.go +++ b/cmd/gortex/proxy.go @@ -9,8 +9,6 @@ import ( "fmt" "os" "path/filepath" - "strconv" - "strings" "time" "github.com/zzet/gortex/internal/daemon" @@ -176,7 +174,7 @@ func daemonSkewWarning(daemonVer, localVer string) string { if dErr != nil || lErr != nil { return base + " — run 'gortex daemon restart' or 'gortex upgrade'" } - switch compareSemver(d, l) { + switch semver.Compare(d, l) { case -1: // daemon older — restarting respawns it from this newer binary return base + " — run 'gortex daemon restart' to upgrade the daemon" case 1: // daemon newer — this binary is the stale side @@ -186,74 +184,6 @@ func daemonSkewWarning(daemonVer, localVer string) string { } } -// compareSemver orders two Versions by SemVer 2.0.0 precedence: -// major.minor.patch numerically, then pre-release (absent outranks -// present; identifiers compared per comparePrereleaseIdent). Build -// metadata is ignored — per the spec it never affects precedence. -// Returns -1, 0, or +1. -func compareSemver(a, b semver.Version) int { - switch { - case a.Major != b.Major: - return compareInt(a.Major, b.Major) - case a.Minor != b.Minor: - return compareInt(a.Minor, b.Minor) - case a.Patch != b.Patch: - return compareInt(a.Patch, b.Patch) - case a.Prerelease == b.Prerelease: - return 0 - case a.Prerelease == "": - return 1 // the release outranks any of its pre-releases - case b.Prerelease == "": - return -1 - default: - return comparePrerelease(a.Prerelease, b.Prerelease) - } -} - -// comparePrerelease compares two dot-separated pre-release identifier -// lists per SemVer 2.0.0: identifier by identifier, and once every -// shared identifier is equal, the shorter list ranks below the longer. -func comparePrerelease(a, b string) int { - as := strings.Split(a, ".") - bs := strings.Split(b, ".") - for i := 0; i < len(as) && i < len(bs); i++ { - if c := comparePrereleaseIdent(as[i], bs[i]); c != 0 { - return c - } - } - return compareInt(len(as), len(bs)) -} - -// comparePrereleaseIdent compares one pre-release identifier pair: -// numeric identifiers compare numerically and rank below alphanumeric -// ones; alphanumeric identifiers compare in ASCII sort order. -func comparePrereleaseIdent(a, b string) int { - an, aErr := strconv.Atoi(a) - bn, bErr := strconv.Atoi(b) - switch { - case aErr == nil && bErr == nil: - return compareInt(an, bn) - case aErr == nil: - return -1 // numeric identifiers rank below alphanumeric - case bErr == nil: - return 1 - default: - return strings.Compare(a, b) - } -} - -// compareInt is the three-way integer compare the semver helpers use. -func compareInt(a, b int) int { - switch { - case a < b: - return -1 - case a > b: - return 1 - default: - return 0 - } -} - func newProxyLogicalSessionID() string { var raw [16]byte if _, err := rand.Read(raw[:]); err == nil { diff --git a/cmd/gortex/proxy_skew_test.go b/cmd/gortex/proxy_skew_test.go index f9438e653..5bcc879fe 100644 --- a/cmd/gortex/proxy_skew_test.go +++ b/cmd/gortex/proxy_skew_test.go @@ -1,10 +1,6 @@ package main -import ( - "testing" - - semver "github.com/zzet/gortex/internal/version" -) +import "testing" func TestDaemonSkewWarning(t *testing.T) { cases := []struct{ name, daemonV, localV, want string }{ @@ -29,41 +25,3 @@ func TestDaemonSkewWarning(t *testing.T) { }) } } - -// TestCompareSemverPrecedence pins the SemVer 2.0.0 §11 precedence -// chain on compareSemver, which the release-only table above never -// exercised: the canonical ascending pre-release ladder (each adjacent -// pair in spec order), build metadata being ignored for precedence -// (§10), and numeric identifiers ranking below alphanumeric ones (§11). -// Each row asserts the comparator's exact sign (-1 / 0 / +1) in the -// argument order given. -func TestCompareSemverPrecedence(t *testing.T) { - cases := []struct { - name string - a, b string - want int - }{ - // §11's ascending example chain, adjacent pair by adjacent pair. - {"alpha lt alpha.1 (larger field set ranks higher)", "v1.0.0-alpha", "v1.0.0-alpha.1", -1}, - {"alpha.1 lt alpha.beta (numeric lt alphanumeric)", "v1.0.0-alpha.1", "v1.0.0-alpha.beta", -1}, - {"alpha.beta lt beta (ASCII sort)", "v1.0.0-alpha.beta", "v1.0.0-beta", -1}, - {"beta lt beta.2 (larger field set ranks higher)", "v1.0.0-beta", "v1.0.0-beta.2", -1}, - {"beta.2 lt beta.11 (numeric compare, not ASCII)", "v1.0.0-beta.2", "v1.0.0-beta.11", -1}, - {"beta.11 lt rc.1 (ASCII sort)", "v1.0.0-beta.11", "v1.0.0-rc.1", -1}, - {"rc.1 lt release (pre-release ranks below release)", "v1.0.0-rc.1", "v1.0.0", -1}, - {"release gt rc.1 (same pair, reversed)", "v1.0.0", "v1.0.0-rc.1", 1}, - // §10: build metadata MUST be ignored when determining precedence. - {"build metadata ignored", "v1.0.0+a", "v1.0.0+b", 0}, - // §11: numeric identifiers always rank below alphanumeric ones. - {"bare numeric ident lt alphanumeric ident", "v1.0.0-1", "v1.0.0-alpha", -1}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - a := semver.MustParse(tc.a) - b := semver.MustParse(tc.b) - if got := compareSemver(a, b); got != tc.want { - t.Fatalf("compareSemver(%s, %s) = %d, want %d", tc.a, tc.b, got, tc.want) - } - }) - } -} diff --git a/internal/version/compare.go b/internal/version/compare.go new file mode 100644 index 000000000..e7f69f62a --- /dev/null +++ b/internal/version/compare.go @@ -0,0 +1,74 @@ +package version + +import ( + "strconv" + "strings" +) + +// Compare orders two Versions by SemVer 2.0.0 precedence: +// major.minor.patch numerically, then pre-release (absent outranks +// present; identifiers compared per comparePrereleaseIdent). Build +// metadata is ignored — per the spec it never affects precedence. +// Returns -1, 0, or +1. +func Compare(a, b Version) int { + switch { + case a.Major != b.Major: + return compareInt(a.Major, b.Major) + case a.Minor != b.Minor: + return compareInt(a.Minor, b.Minor) + case a.Patch != b.Patch: + return compareInt(a.Patch, b.Patch) + case a.Prerelease == b.Prerelease: + return 0 + case a.Prerelease == "": + return 1 // the release outranks any of its pre-releases + case b.Prerelease == "": + return -1 + default: + return comparePrerelease(a.Prerelease, b.Prerelease) + } +} + +// comparePrerelease compares two dot-separated pre-release identifier +// lists per SemVer 2.0.0: identifier by identifier, and once every +// shared identifier is equal, the shorter list ranks below the longer. +func comparePrerelease(a, b string) int { + as := strings.Split(a, ".") + bs := strings.Split(b, ".") + for i := 0; i < len(as) && i < len(bs); i++ { + if c := comparePrereleaseIdent(as[i], bs[i]); c != 0 { + return c + } + } + return compareInt(len(as), len(bs)) +} + +// comparePrereleaseIdent compares one pre-release identifier pair: +// numeric identifiers compare numerically and rank below alphanumeric +// ones; alphanumeric identifiers compare in ASCII sort order. +func comparePrereleaseIdent(a, b string) int { + an, aErr := strconv.Atoi(a) + bn, bErr := strconv.Atoi(b) + switch { + case aErr == nil && bErr == nil: + return compareInt(an, bn) + case aErr == nil: + return -1 // numeric identifiers rank below alphanumeric + case bErr == nil: + return 1 + default: + return strings.Compare(a, b) + } +} + +// compareInt is the three-way integer compare the semver helpers use. +func compareInt(a, b int) int { + switch { + case a < b: + return -1 + case a > b: + return 1 + default: + return 0 + } +} diff --git a/internal/version/compare_test.go b/internal/version/compare_test.go new file mode 100644 index 000000000..fe5c6ec9d --- /dev/null +++ b/internal/version/compare_test.go @@ -0,0 +1,40 @@ +package version + +import "testing" + +// TestComparePrecedence pins the SemVer 2.0.0 §11 precedence chain on +// Compare: the canonical ascending pre-release ladder (each adjacent +// pair in spec order), build metadata being ignored for precedence +// (§10), and numeric identifiers ranking below alphanumeric ones (§11). +// Each row asserts the comparator's exact sign (-1 / 0 / +1) in the +// argument order given. +func TestComparePrecedence(t *testing.T) { + cases := []struct { + name string + a, b string + want int + }{ + // §11's ascending example chain, adjacent pair by adjacent pair. + {"alpha lt alpha.1 (larger field set ranks higher)", "v1.0.0-alpha", "v1.0.0-alpha.1", -1}, + {"alpha.1 lt alpha.beta (numeric lt alphanumeric)", "v1.0.0-alpha.1", "v1.0.0-alpha.beta", -1}, + {"alpha.beta lt beta (ASCII sort)", "v1.0.0-alpha.beta", "v1.0.0-beta", -1}, + {"beta lt beta.2 (larger field set ranks higher)", "v1.0.0-beta", "v1.0.0-beta.2", -1}, + {"beta.2 lt beta.11 (numeric compare, not ASCII)", "v1.0.0-beta.2", "v1.0.0-beta.11", -1}, + {"beta.11 lt rc.1 (ASCII sort)", "v1.0.0-beta.11", "v1.0.0-rc.1", -1}, + {"rc.1 lt release (pre-release ranks below release)", "v1.0.0-rc.1", "v1.0.0", -1}, + {"release gt rc.1 (same pair, reversed)", "v1.0.0", "v1.0.0-rc.1", 1}, + // §10: build metadata MUST be ignored when determining precedence. + {"build metadata ignored", "v1.0.0+a", "v1.0.0+b", 0}, + // §11: numeric identifiers always rank below alphanumeric ones. + {"bare numeric ident lt alphanumeric ident", "v1.0.0-1", "v1.0.0-alpha", -1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + a := MustParse(tc.a) + b := MustParse(tc.b) + if got := Compare(a, b); got != tc.want { + t.Fatalf("Compare(%s, %s) = %d, want %d", tc.a, tc.b, got, tc.want) + } + }) + } +} From 668dfa478159f3b916d9de0c480e6b42f1bfe261 Mon Sep 17 00:00:00 2001 From: Stephen Eaton Date: Thu, 20 Aug 2026 11:08:39 +1000 Subject: [PATCH 9/9] feat(doctor): surface CLI-vs-daemon version skew in the environment probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review #4969353348 (r3810888879): doctor is the command a confused user runs when versions feel wrong, so the handshake row now downgrades to a warning and prints the same remedy line the MCP proxy emits — computed by the same daemonSkewWarning, so the three surfaces cannot disagree. DoctorEnvironment carries cli_version and version_skew_warning for --json consumers; dev builds stay silent via the existing sentinel. --- cmd/gortex/doctor.go | 33 ++++++++++++++++----- cmd/gortex/doctor_env_test.go | 55 +++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 cmd/gortex/doctor_env_test.go diff --git a/cmd/gortex/doctor.go b/cmd/gortex/doctor.go index 895c2bc21..7948c20cd 100644 --- a/cmd/gortex/doctor.go +++ b/cmd/gortex/doctor.go @@ -102,13 +102,15 @@ func init() { // integration cannot check: that the `gortex` the editor will launch is the // one on PATH, and that the daemon it proxies to actually answers a handshake. type DoctorEnvironment struct { - BinaryOnPath bool `json:"binary_on_path"` - BinaryPath string `json:"binary_path,omitempty"` - BinaryError string `json:"binary_error,omitempty"` - DaemonRunning bool `json:"daemon_running"` - DaemonSocket string `json:"daemon_socket,omitempty"` - DaemonVersion string `json:"daemon_version,omitempty"` - DaemonError string `json:"daemon_error,omitempty"` + BinaryOnPath bool `json:"binary_on_path"` + BinaryPath string `json:"binary_path,omitempty"` + BinaryError string `json:"binary_error,omitempty"` + CLIVersion string `json:"cli_version,omitempty"` + DaemonRunning bool `json:"daemon_running"` + DaemonSocket string `json:"daemon_socket,omitempty"` + DaemonVersion string `json:"daemon_version,omitempty"` + DaemonError string `json:"daemon_error,omitempty"` + VersionSkewWarning string `json:"version_skew_warning,omitempty"` } // DoctorAgentReport is one agent's slice of the doctor output. @@ -211,6 +213,7 @@ func doctorExit(r doctorRuntime) error { // best-effort and never fail the command — doctor is a read-only diagnostic. func doctorEnvironment() DoctorEnvironment { out := DoctorEnvironment{DaemonSocket: daemon.SocketPath()} + out.CLIVersion = canonicalVersion() if p, err := exec.LookPath("gortex"); err == nil { out.BinaryOnPath = true out.BinaryPath = p @@ -231,6 +234,10 @@ func doctorEnvironment() DoctorEnvironment { defer c.Close() out.DaemonRunning = true out.DaemonVersion = c.Ack.DaemonVersion + // The same skew compare `gortex mcp` warns on at connect time and + // `daemon status` renders next to the daemon version — doctor is + // what a confused user runs first, so it must agree with both. + out.VersionSkewWarning = daemonSkewWarning(out.DaemonVersion, out.CLIVersion) return out } @@ -331,7 +338,17 @@ func printDoctorEnvironment(w io.Writer, env DoctorEnvironment) { if ver == "" { ver = "ok" } - fmt.Fprintf(w, " %s daemon handshake: %s (%s)\n", glyphCheck, ver, doctorPath(env.DaemonSocket)) + // A skewed pair downgrades the handshake row to a warning and + // appends the same one-line remedy the proxy prints — doctor, + // `daemon status`, and the MCP proxy must give one verdict. + glyph := glyphCheck + if env.VersionSkewWarning != "" { + glyph = glyphWarn + } + fmt.Fprintf(w, " %s daemon handshake: %s (%s)\n", glyph, ver, doctorPath(env.DaemonSocket)) + if env.VersionSkewWarning != "" { + fmt.Fprintf(w, " %s\n", env.VersionSkewWarning) + } } else { fmt.Fprintf(w, " %s daemon handshake: %s\n", glyphCross, env.DaemonError) } diff --git a/cmd/gortex/doctor_env_test.go b/cmd/gortex/doctor_env_test.go new file mode 100644 index 000000000..de5e48cce --- /dev/null +++ b/cmd/gortex/doctor_env_test.go @@ -0,0 +1,55 @@ +package main + +import ( + "bytes" + "testing" +) + +// TestPrintDoctorEnvironment_VersionSkew — the daemon handshake row +// carries ✓ when CLI and daemon agree and ! plus the skew remedy line +// when they don't, reusing daemonSkewWarning so doctor, `daemon +// status`, and the MCP proxy render one verdict. +func TestPrintDoctorEnvironment_VersionSkew(t *testing.T) { + t.Run("matching versions keep the check row", func(t *testing.T) { + env := DoctorEnvironment{ + DaemonRunning: true, + DaemonVersion: "v0.63.4+5f5fce2", + CLIVersion: "v0.63.4+5f5fce2", + DaemonSocket: "/tmp/gortex.sock", + } + if warn := daemonSkewWarning(env.DaemonVersion, env.CLIVersion); warn != "" { + t.Fatalf("precondition: expected no skew warning for matching versions, got %q", warn) + } + var buf bytes.Buffer + printDoctorEnvironment(&buf, env) + out := buf.String() + if !bytes.Contains([]byte(out), []byte("✓ daemon handshake: v0.63.4+5f5fce2")) { + t.Fatalf("expected check-marked handshake row, got:\n%s", out) + } + if bytes.Contains([]byte(out), []byte("warning:")) { + t.Fatalf("unexpected skew warning in matching-version render:\n%s", out) + } + }) + + t.Run("skewed versions warn with the remedy", func(t *testing.T) { + env := DoctorEnvironment{ + DaemonRunning: true, + DaemonVersion: "v0.63.4+5f9ce2a", + CLIVersion: "v0.63.5+abcdef1", + DaemonSocket: "/tmp/gortex.sock", + } + env.VersionSkewWarning = daemonSkewWarning(env.DaemonVersion, env.CLIVersion) + if env.VersionSkewWarning == "" { + t.Fatal("precondition: daemonSkewWarning must fire for skewed versions") + } + var buf bytes.Buffer + printDoctorEnvironment(&buf, env) + out := buf.String() + if !bytes.Contains([]byte(out), []byte("! daemon handshake: v0.63.4+5f9ce2a")) { + t.Fatalf("expected warn-marked handshake row, got:\n%s", out) + } + if !bytes.Contains([]byte(out), []byte("run 'gortex daemon restart' to upgrade the daemon")) { + t.Fatalf("expected the skew remedy line, got:\n%s", out) + } + }) +}