diff --git a/cmd/flue/relayui.go b/cmd/flue/relayui.go index ee9ccb3..50fc08f 100644 --- a/cmd/flue/relayui.go +++ b/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 } diff --git a/cmd/flue/relayui_test.go b/cmd/flue/relayui_test.go index 7a0af40..4b60f4f 100644 --- a/cmd/flue/relayui_test.go +++ b/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{ diff --git a/internal/daemon/relayui.go b/internal/daemon/relayui.go index 41ec439..b4a97e1 100644 --- a/internal/daemon/relayui.go +++ b/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