fix: the update card stops demanding a redeploy the edge just hasn't caught up with

cmd/flue/relayui.go

@@ -49,6 +49,21 @@ type relayUIService struct {
 	// log hears what the deploy did — worker names and outcomes, never a
 	// token and never the secret. Nil is quiet, which is what tests want.
 	log *slog.Logger
+
+	// shippedOrigin and shippedStamp remember the last deploy this process
+	// completed: where it shipped and what stamp it shipped. They exist for
+	// the minutes right after a deploy — Cloudflare's API accepts a new
+	// Worker before every edge serves it, and until propagation finishes the
+	// relay's /api/health still answers with the previous stamp. A Status
+	// that trusted only the health read would put the update card back under
+	// the checkmarks the deploy just earned, and taking that offer redeploys
+	// identical bytes. Their own mutex, not mu: Status must render during a
+	// deploy, not queue behind one. Lost on a daemon restart, deliberately —
+	// by the time a daemon comes back, propagation is long done and the
+	// health read is telling the truth again.
+	shippedMu     sync.Mutex
+	shippedOrigin string
+	shippedStamp  string
 }
 
 func (s *relayUIService) logf() *slog.Logger {
@@ -140,7 +155,17 @@ func (s *relayUIService) Status(ctx context.Context) daemon.RelayUIStatus {
 	if w, err := updateWorkerName("", cfg); err == nil {
 		st.Worker = w
 	}
-	st.DeployedVersion = deployedVersion(ctx, cfg.Origin)
+	// This process's own deploy outranks the health read: right after one,
+	// the edge can keep serving the previous Worker — previous stamp and
+	// all — for seconds, occasionally longer, and believing /api/health in
+	// that window turns a finished deploy into an update offer. The memory
+	// declines to answer whenever it has gone stale (see shippedVersion),
+	// and the health read decides as before.
+	if stamp, ok := s.shippedVersion(cfg.Origin); ok {
+		st.DeployedVersion = stamp
+	} else {
+		st.DeployedVersion = deployedVersion(ctx, cfg.Origin)
+	}
 	return st
 }
 
@@ -171,6 +196,31 @@ func deployedVersion(ctx context.Context, origin string) string {
 	return health.Version
 }
 
+// recordShipped is the deploy paths' success line: this process just put its
+// own bytes behind origin, whatever /api/health says for the next while. A
+// deploy that failed must never reach here — it changed nothing at the edge,
+// and the card has to keep offering what the health read supports.
+func (s *relayUIService) recordShipped(origin string) {
+	s.shippedMu.Lock()
+	defer s.shippedMu.Unlock()
+	s.shippedOrigin, s.shippedStamp = origin, deployStamp()
+}
+
+// shippedVersion answers Status from memory, but only while the memory still
+// speaks for the question being asked: the same origin, carrying the stamp
+// this binary would ship again. Anything else is stale and must lose to a
+// live health read — a relay.json repointed or re-joined elsewhere names a
+// deploy this process never performed, and a binary whose own stamp moved on
+// has a genuinely newer build whose update card must not be swallowed.
+func (s *relayUIService) shippedVersion(origin string) (string, bool) {
+	s.shippedMu.Lock()
+	defer s.shippedMu.Unlock()
+	if s.shippedStamp == "" || s.shippedOrigin != origin || s.shippedStamp != deployStamp() {
+		return "", false
+	}
+	return s.shippedStamp, true
+}
+
 // resolveToken decides what credential a deploy runs with: the request's
 // token when one was typed, the stored one otherwise, a refusal when there is
 // neither. It reports whether the token came from the request — the ones that
@@ -328,6 +378,10 @@ func (s *relayUIService) Provision(ctx context.Context, req daemon.RelayUIDeploy
 	} else if started {
 		res.Steps = append(res.Steps, "daemon connecting to the relay")
 	}
+	// Remember what shipped and where: Status answers from this while the
+	// edge catches up, instead of trusting a health read that briefly still
+	// says the previous deploy.
+	s.recordShipped(res.Origin)
 	s.logf().Info("relay deployed from the UI", "worker", worker, "host", host, "restartNeeded", res.RestartNeeded)
 	return res, nil
 }
@@ -345,7 +399,10 @@ func joinCommand(host, secret string) string {
 // any existing pairing — the daemon serves exactly the origin it dials, so
 // every browser paired on the old one must pair again on the new address
 // (see runRelayAddress), and the second step line carries that truth to the
-// card.
+// card. The shipped-deploy memory keys on the origin, so a repoint sends
+// Status back to asking the relay itself: the Worker behind the new name
+// should be the same one, but that is the health read's fact to confirm,
+// not memory's to assume.
 func (s *relayUIService) SetAddress(ctx context.Context, address string) (daemon.RelayUIDeployResult, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -458,6 +515,9 @@ func (s *relayUIService) Update(ctx context.Context, req daemon.RelayUIDeployReq
 			steps = append(steps, "token stored for one-click updates")
 		}
 	}
+	// Same note as Provision's: the deploy succeeded, so Status may say so
+	// without waiting for the edge to agree.
+	s.recordShipped(cfg.Origin)
 	s.logf().Info("relay updated from the UI", "worker", worker)
 	return daemon.RelayUIDeployResult{Steps: steps, Origin: cfg.Origin}, nil
 }

cmd/flue/relayui_test.go

@@ -218,6 +218,140 @@ func TestRelayUIStatusReportsTheDeployedVersion(t *testing.T) {
 	}
 }
 
+// staleHealth is a relay edge mid-propagation: whatever was just deployed,
+// /api/health still answers with the previous deploy's stamp — which is what
+// a real relay does for seconds, occasionally longer, after every deploy.
+func staleHealth(t *testing.T) *httptest.Server {
+	t.Helper()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/api/health" {
+			http.NotFound(w, r)
+			return
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "version": "0.0.0-previous"})
+	}))
+	t.Cleanup(srv.Close)
+	return srv
+}
+
+// seedRelayAt writes the relay.json of a machine already joined to a relay
+// whose origin is the given server — the state the update card renders from.
+func seedRelayAt(t *testing.T, origin string) {
+	t.Helper()
+	if err := config.SaveRelay(config.Relay{
+		URL:       "wss://flue-relay.karn.workers.dev",
+		Secret:    "s",
+		Origin:    origin,
+		MachineID: "m-1",
+		Worker:    relayScriptName,
+	}); err != nil {
+		t.Fatalf("SaveRelay: %v", err)
+	}
+}
+
+// TestRelayUIStatusTrustsItsOwnDeployWhileTheEdgeCatchesUp is the repro that
+// earned the service its memory: deploy, watch every checkmark land, and the
+// card underneath still says "update the relay" — because Cloudflare's API
+// accepted the new Worker while the edge kept serving the old one, previous
+// stamp and all, and Status believed the edge. The user obliges and
+// redeploys identical bytes; time was the actual fix. After a successful
+// deploy the service's own memory answers instead.
+func TestRelayUIStatusTrustsItsOwnDeployWhileTheEdgeCatchesUp(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	f := newFakeCloudflare(t, oneAccount(), "karn")
+	stale := staleHealth(t)
+	seedRelayAt(t, stale.URL)
+	svc := uiService(f, &relayRuntime{running: true})
+
+	// Before this process has deployed anything, the health read decides —
+	// its differing stamp is what puts the update card up at all.
+	if st := svc.Status(context.Background()); st.DeployedVersion != "0.0.0-previous" {
+		t.Fatalf("deployed version before deploying = %q, want the health read's 0.0.0-previous", st.DeployedVersion)
+	}
+
+	if _, err := svc.Update(context.Background(), daemon.RelayUIDeployRequest{Token: setupToken}); err != nil {
+		t.Fatalf("Update: %v", err)
+	}
+
+	// The edge still answers the previous stamp; Status must not believe it.
+	if st := svc.Status(context.Background()); st.DeployedVersion != deployStamp() {
+		t.Fatalf("deployed version right after a successful deploy = %q, want this binary's %q", st.DeployedVersion, deployStamp())
+	}
+
+	// Provision remembers the same way: a first deploy's Status answers from
+	// memory too, without dialling the fresh workers.dev origin at all.
+	if _, err := svc.Provision(context.Background(), daemon.RelayUIDeployRequest{Token: setupToken}); err != nil {
+		t.Fatalf("Provision: %v", err)
+	}
+	if st := svc.Status(context.Background()); st.DeployedVersion != deployStamp() {
+		t.Fatalf("deployed version right after a provision = %q, want %q", st.DeployedVersion, deployStamp())
+	}
+}
+
+// TestRelayUIFailedUpdateLeavesTheHealthReadInCharge: a deploy the API
+// refused changed nothing at the edge, so it earns no memory — the card
+// keeps offering exactly what the health read supports.
+func TestRelayUIFailedUpdateLeavesTheHealthReadInCharge(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	f := newFakeCloudflare(t, oneAccount(), "karn")
+	f.reject["/scripts/"+relayScriptName] = "computer says no"
+	stale := staleHealth(t)
+	seedRelayAt(t, stale.URL)
+	svc := uiService(f, &relayRuntime{running: true})
+
+	if _, err := svc.Update(context.Background(), daemon.RelayUIDeployRequest{Token: setupToken}); err == nil {
+		t.Fatal("the rejected deploy reported success")
+	}
+	if st := svc.Status(context.Background()); st.DeployedVersion != "0.0.0-previous" {
+		t.Fatalf("deployed version after a failed deploy = %q, want the health read's 0.0.0-previous", st.DeployedVersion)
+	}
+}
+
+// TestRelayUIStaleShipMemoryLosesToTheHealthRead pins the two ways the
+// memory expires. A binary whose stamp changed no longer ships what the
+// memory says was shipped — in practice a rebuilt daemon, whose restart
+// drops the memory anyway; the guard states the invariant without leaning on
+// the restart. And an origin that moved on names a relay this process never
+// deployed to. Both must yield to the health read, so a genuinely newer
+// build still gets its update card.
+func TestRelayUIStaleShipMemoryLosesToTheHealthRead(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	f := newFakeCloudflare(t, oneAccount(), "karn")
+	stale := staleHealth(t)
+	seedRelayAt(t, stale.URL)
+	svc := uiService(f, &relayRuntime{running: true})
+
+	if _, err := svc.Update(context.Background(), daemon.RelayUIDeployRequest{Token: setupToken}); err != nil {
+		t.Fatalf("Update: %v", err)
+	}
+
+	// The rebuilt-binary shape: the memory holds a stamp this binary would
+	// not ship. (A test cannot rebuild itself, so it plants the mismatch.)
+	svc.shippedMu.Lock()
+	svc.shippedStamp = "dev-some-other-build"
+	svc.shippedMu.Unlock()
+	if st := svc.Status(context.Background()); st.DeployedVersion != "0.0.0-previous" {
+		t.Fatalf("deployed version under a stale stamp = %q, want the health read's 0.0.0-previous", st.DeployedVersion)
+	}
+
+	// The moved-origin shape, through the real path: deploy again (memory
+	// back in force), then repoint the address. The new origin answers no
+	// health read — port 1 refuses instantly — and the memory, keyed to the
+	// origin it shipped to, must not answer for it.
+	if _, err := svc.Update(context.Background(), daemon.RelayUIDeployRequest{Token: setupToken}); err != nil {
+		t.Fatalf("second Update: %v", err)
+	}
+	if st := svc.Status(context.Background()); st.DeployedVersion != deployStamp() {
+		t.Fatalf("deployed version after re-deploying = %q, want %q", st.DeployedVersion, deployStamp())
+	}
+	if _, err := svc.SetAddress(context.Background(), "wss://127.0.0.1:1"); err != nil {
+		t.Fatalf("SetAddress: %v", err)
+	}
+	if st := svc.Status(context.Background()); st.DeployedVersion != "" {
+		t.Fatalf("deployed version after repointing = %q, want empty: neither memory nor a health read can speak for the new origin", st.DeployedVersion)
+	}
+}
+
 // twoAccounts mirrors oneAccount for the picker tests.
 func twoAccounts() []cloudflare.Account {
 	return []cloudflare.Account{

internal/daemon/relayui.go

@@ -57,9 +57,13 @@ type RelayUIStatus struct {
 	// the sentence the UI shows instead of a button.
 	CanDeploy       bool   `json:"can_deploy"`
 	CanDeployReason string `json:"can_deploy_reason,omitempty"`
-	// Version is this binary's; DeployedVersion is what the relay's
-	// /api/health reported, empty when unreachable or unstamped. The UI
-	// offers an update when the two differ.
+	// Version is this binary's; DeployedVersion is what the relay serves.
+	// Normally that is what its /api/health reported — empty when
+	// unreachable or unstamped — except right after a deploy this daemon
+	// performed, when it is the stamp the daemon shipped: the edge keeps
+	// serving the previous Worker for a while after the API accepts a new
+	// one, and a health read taken in that window would re-offer an update
+	// that just succeeded. The UI offers an update when the two differ.
 	Version         string `json:"version"`
 	DeployedVersion string `json:"deployed_version,omitempty"`
 	// HasToken says a Cloudflare token is stored (config/cloudflare.json), so