From 102817bb98913c132491e48751e8dd6b0bc76cb1 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 15 Jul 2026 18:56:04 +0500 Subject: [PATCH 1/4] fix(cli): home heartbeat fetches one client, not the whole account (#338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare-CLI home screen showed a false "running — couldn't confirm it's connected to tracebloc" while `tb doctor` was fully green. Root cause: realHeartbeat → lookupClientStatus called api.Client.ListClients, which pages through EVERY client in the account (GET /edge-device/, up to maxListPages=100) just to read one client's status — all under the ~1.2s home-probe budget. On accounts with enough clients to need multiple pages, the list can't finish in time → context deadline → beatUnknown → the false "couldn't confirm". doctor is green because its Backend-auth check is a single GET /userinfo/ with no leash. Add api.Client.GetClient (GET /edge-device/{id}/ — the detail route PatchClientClusterID/RevokeClient already use) and switch lookupClientStatus to fetch the single active client directly (404 → not-found, not an error). One O(1) round-trip fits comfortably in the budget → honest Online/not-online. Removes the now-unused findClientByID (blocking deadcode gate). ListClients still backs list / use / create-collision. Fixes #338. Co-Authored-By: Claude Opus 4.8 --- internal/api/client.go | 27 +++++++++++++++++++ internal/api/client_get_test.go | 47 +++++++++++++++++++++++++++++++++ internal/cli/client.go | 33 ++++++++++------------- 3 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 internal/api/client_get_test.go diff --git a/internal/api/client.go b/internal/api/client.go index 15a95bd..d5ed292 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -505,6 +505,33 @@ func (c *Client) RevokeClient(ctx context.Context, id int) error { // against a misbehaving `next` chain, set well above any real account. const maxListPages = 100 +// GetClient fetches a single client by its dashboard id (GET /edge-device/{id}/). +// The detail route is the same one PatchClientClusterID/RevokeClient address, and +// returns one ProvisionedClient. This is the O(1) way to check ONE client's +// status — unlike ListClients, which pages through the whole account (the +// home-screen heartbeat must not do that under its ~1.2s budget, cli#338). +// A 404 returns (nil, nil) so the caller can distinguish "no such client" from +// a transport/backend error. +func (c *Client) GetClient(ctx context.Context, id int) (*ProvisionedClient, error) { + path := fmt.Sprintf("/edge-device/%d/", id) + url := c.BaseURL + path + status, raw, err := c.get(ctx, path) + if err != nil { + return nil, err + } + if status == http.StatusNotFound { + return nil, nil + } + if status < 200 || status >= 300 { + return nil, &APIError{StatusCode: status, Body: string(raw), URL: url} + } + var out ProvisionedClient + if err := json.Unmarshal(raw, &out); err != nil { + return nil, fmt.Errorf("decoding get-client response: %w", err) + } + return &out, nil +} + // ListClients returns ALL clients in the caller's account (GET /edge-device/). // The endpoint is DRF-paginated, so this follows `next` to the end — list, // `use `, and create-time collision detection must see every client, not diff --git a/internal/api/client_get_test.go b/internal/api/client_get_test.go new file mode 100644 index 0000000..d71728a --- /dev/null +++ b/internal/api/client_get_test.go @@ -0,0 +1,47 @@ +package api + +import ( + "context" + "net/http" + "testing" +) + +// TestGetClient covers the single-client detail fetch (GET /edge-device/{id}/) +// that backs the home-screen heartbeat (cli#338): a 2xx decodes one client, a +// 404 is (nil, nil) — "no such client", not an error — and any other non-2xx is +// an APIError. It also pins the path so the heartbeat can't regress to a list. +func TestGetClient(t *testing.T) { + t.Run("2xx decodes a single client + hits the detail path", func(t *testing.T) { + c := stubClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/edge-device/1070/" { + t.Errorf("path = %q, want /edge-device/1070/ (detail route, not the list)", r.URL.Path) + } + _, _ = w.Write([]byte(`{"id":1070,"first_name":"asad-macbook","status":1,"namespace":"asad-macbook-3"}`)) + }) + pc, err := c.GetClient(context.Background(), 1070) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pc == nil || pc.ID != 1070 || pc.Status != 1 { + t.Fatalf("GetClient = %+v, want id=1070 status=1", pc) + } + }) + + t.Run("404 -> (nil, nil), not an error", func(t *testing.T) { + c := stubClient(t, func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) }) + pc, err := c.GetClient(context.Background(), 999) + if pc != nil || err != nil { + t.Fatalf("404 must be (nil, nil); got (%+v, %v)", pc, err) + } + }) + + t.Run("non-2xx (500) -> APIError", func(t *testing.T) { + c := stubClient(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"detail":"boom"}`)) + }) + if _, err := c.GetClient(context.Background(), 1); err == nil { + t.Error("a 500 must return an error") + } + }) +} diff --git a/internal/cli/client.go b/internal/cli/client.go index 8a44919..cd1ff9d 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -864,30 +864,25 @@ func runClientStatus(ctx context.Context, p *ui.Printer, wait bool, timeout time } } -// lookupClientStatus finds the account client whose numeric id matches active and -// returns its backend status code. found=false means no such client (deleted, or -// signed into the wrong account). A list error is returned verbatim so --wait can -// treat it as transient and retry. +// lookupClientStatus fetches the active client directly and returns its backend +// status code. found=false means no such client (deleted, or signed into the +// wrong account). A lookup error is returned verbatim so --wait can treat it as +// transient and retry. Fetches the single client by id (GET /edge-device/{id}/) +// rather than listing the whole account — the home-screen heartbeat runs this +// under a ~1.2s budget, and paging every client blew it (cli#338). func lookupClientStatus(ctx context.Context, client *api.Client, active string) (status int, found bool, err error) { - clients, err := client.ListClients(ctx) + id, err := strconv.Atoi(active) if err != nil { - return 0, false, err + return 0, false, fmt.Errorf("active client id %q is not numeric: %w", active, err) } - if c := findClientByID(clients, active); c != nil { - return c.Status, true, nil + c, err := client.GetClient(ctx, id) + if err != nil { + return 0, false, err } - return 0, false, nil -} - -// findClientByID returns the account client whose numeric dashboard id equals id -// (the string form stored as the active-client pointer), or nil if none match. -func findClientByID(clients []api.ProvisionedClient, id string) *api.ProvisionedClient { - for i := range clients { - if strconv.Itoa(clients[i].ID) == id { - return &clients[i] - } + if c == nil { + return 0, false, nil // 404 — no such client } - return nil + return c.Status, true, nil } // EdgeDevice.status codes mirrored from the backend (metaApi User.py). From eb67e6628f1b940afffdb71f44bb0eccf75db4e2 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 15 Jul 2026 19:43:48 +0500 Subject: [PATCH 2/4] test(cli): point client-status stubs at the /edge-device/{id}/ detail route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lookupClientStatus now fetches the single client (GetClient) instead of ListClients, so the TestClientStatus_* backend stubs must serve GET /edge-device/5/ (single object) rather than GET /edge-device/ (array). Without this the status path decoded an empty body ('unexpected end of JSON input') and the --wait fail-fast tests (426/401/missing) looped to the 10m test timeout. Create/list stubs are unchanged — ListClients still backs those. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client_test.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index cb0b847..4ebfe2d 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -1223,8 +1223,8 @@ func setActiveClientID(t *testing.T, id string) { // reports the active client online, and says so. func TestClientStatus_WaitOnline_Exit0(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { - _, _ = w.Write([]byte(`[{"id":5,"first_name":"c","namespace":"c","status":1}]`)) // 1 = online + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","namespace":"c","status":1}`)) // 1 = online return } t.Errorf("unexpected %s %s", r.Method, r.URL.Path) @@ -1245,8 +1245,8 @@ func TestClientStatus_WaitOnline_Exit0(t *testing.T) { // so the loop never sleeps. func TestClientStatus_WaitTimeout_Exit1(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { - _, _ = w.Write([]byte(`[{"id":5,"first_name":"c","namespace":"c","status":0}]`)) // 0 = offline + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","namespace":"c","status":0}`)) // 0 = offline } }) setActiveClientID(t, "5") @@ -1265,8 +1265,8 @@ func TestClientStatus_WaitTimeout_Exit1(t *testing.T) { // TestClientStatus_OneShot: without --wait, report the current state and exit 0. func TestClientStatus_OneShot(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { - _, _ = w.Write([]byte(`[{"id":5,"first_name":"c","namespace":"c","status":2}]`)) // 2 = pending + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","namespace":"c","status":2}`)) // 2 = pending } }) setActiveClientID(t, "5") @@ -1297,7 +1297,7 @@ func TestClientStatus_NoActiveClient(t *testing.T) { // proves we didn't poll to exhaustion. func TestClientStatus_WaitFailsFastOn426(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { w.WriteHeader(http.StatusUpgradeRequired) // 426 _, _ = w.Write([]byte(`{"error":"upgrade_required","min_version":"1.2.3"}`)) } @@ -1320,8 +1320,8 @@ func TestClientStatus_WaitFailsFastOn426(t *testing.T) { // matching the one-shot path, rather than polling to the timeout. func TestClientStatus_WaitFailsFastOnMissingClient(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { - _, _ = w.Write([]byte(`[{"id":9,"first_name":"other","namespace":"other","status":1}]`)) // active id 5 absent + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { + w.WriteHeader(http.StatusNotFound) // active id 5 absent → 404 } }) setActiveClientID(t, "5") @@ -1340,8 +1340,8 @@ func TestClientStatus_WaitFailsFastOnMissingClient(t *testing.T) { // "unreachable". A 1ns timeout means the deadline passes on the first failure. func TestClientStatus_WaitTimeoutSurfacesListError(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { - w.WriteHeader(http.StatusInternalServerError) // persistent list failure + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { + w.WriteHeader(http.StatusInternalServerError) // persistent status-check failure } }) setActiveClientID(t, "5") @@ -1362,7 +1362,7 @@ func TestClientStatus_WaitTimeoutSurfacesListError(t *testing.T) { // the full timeout. A 10-minute timeout would hang the test if it didn't. func TestClientStatus_WaitFailsFastOn401(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { w.WriteHeader(http.StatusUnauthorized) // dead credential } }) @@ -1398,13 +1398,13 @@ func TestClientStatus_TimeoutWithoutWaitRejected(t *testing.T) { func TestClientStatus_WaitTimeoutClearsStaleError(t *testing.T) { calls := 0 withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { calls++ if calls == 1 { w.WriteHeader(http.StatusBadGateway) // one transient blip return } - _, _ = w.Write([]byte(`[{"id":5,"first_name":"c","namespace":"c","status":0}]`)) // then offline + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","namespace":"c","status":0}`)) // then offline } }) setActiveClientID(t, "5") @@ -1430,8 +1430,8 @@ func TestClientStatus_WaitTimeoutClearsStaleError(t *testing.T) { // during --wait exits quietly with code 130 — not a bare "Error: context canceled". func TestClientStatus_WaitCtrlCIsSilent(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { - _, _ = w.Write([]byte(`[{"id":5,"first_name":"c","namespace":"c","status":0}]`)) // offline + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","namespace":"c","status":0}`)) // offline } }) setActiveClientID(t, "5") From 4d633c77942cebe7bba3d9605e7cd4926f80bfe6 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 15 Jul 2026 19:51:23 +0500 Subject: [PATCH 3/4] test(cli): point delete status-guard stubs at /edge-device/{id}/ too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit delete's offboard guard also checks the client via lookupClientStatus, which now fetches GET /edge-device/{id}/. The TestDelete_* status stubs served the list route → the guard hit an unhandled path (empty body / 'unexpected GET /edge-device/5/') and the online/426/401 guards stopped firing. Redirect the status-guard GETs to /edge-device/5/ (single object). The --force revoke-fail tests skip the guard and only stub /revoke/, so they're untouched. Full internal/cli package green. Co-Authored-By: Claude Opus 4.8 --- internal/cli/delete_test.go | 66 ++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/internal/cli/delete_test.go b/internal/cli/delete_test.go index f70bb56..c8d89a7 100644 --- a/internal/cli/delete_test.go +++ b/internal/cli/delete_test.go @@ -137,9 +137,9 @@ func TestDelete_Yes_FullSequence(t *testing.T) { revokePath := "" withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": // Guard's status lookup: report the client OFFLINE so it doesn't block. - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): revokePath = r.URL.Path if got := r.Header.Get("Authorization"); got != "Bearer tok" { @@ -200,8 +200,8 @@ func TestDelete_Yes_FullSequence(t *testing.T) { func TestDelete_RevokeNon403_ContinuesTeardown(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusNotFound) // 404: stale pointer / backend predates /revoke default: @@ -248,8 +248,8 @@ func TestDelete_RevokeNon403_ContinuesTeardown(t *testing.T) { func TestDelete_RevokeNon403_Degraded_HonestClosing(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusNotFound) // 404: best-effort revoke fails default: @@ -329,8 +329,8 @@ func TestDelete_RevokeUnauthorized_FailsFast(t *testing.T) { func TestDelete_KeepData_SparesDataDir(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) default: @@ -375,8 +375,8 @@ func TestDelete_KeepData_SparesDataDir(t *testing.T) { func TestDelete_WipeFails_StillClearsPointer(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -407,8 +407,8 @@ func TestDelete_WipeFails_StillClearsPointer(t *testing.T) { func TestDelete_TeardownFailure_HonestClosing(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -441,7 +441,7 @@ func TestDelete_Guard426_FailsFast(t *testing.T) { if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke") { revoked = true } - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { w.WriteHeader(http.StatusUpgradeRequired) // 426 _, _ = w.Write([]byte(`{"error":"upgrade_required","min_version":"1.2.3"}`)) } @@ -472,7 +472,7 @@ func TestDelete_GuardAuthError_FailsFast(t *testing.T) { if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke") { revoked = true } - if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/" { w.WriteHeader(http.StatusUnauthorized) // 401 — token revoked/expired _, _ = w.Write([]byte(`{"detail":"invalid token"}`)) } @@ -500,8 +500,8 @@ func TestDelete_GuardAuthError_FailsFast(t *testing.T) { func TestDelete_SelfRemovalFails_HonestClosing(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -532,8 +532,8 @@ func TestDelete_SelfRemovalFails_HonestClosing(t *testing.T) { func TestDelete_KubeconfigContext_ReachHelm(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -559,9 +559,9 @@ func TestDelete_RunningJob_RefusesUnlessForce(t *testing.T) { newHandler := func() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": // status 1 = online (a running client). - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":1}]`)) + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":1}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -619,8 +619,8 @@ func TestDelete_RunningJob_RefusesUnlessForce(t *testing.T) { func TestDelete_ShowsRetainedAndLeftCopy(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -653,7 +653,7 @@ func TestDelete_TypedNameMismatch_Cancels(t *testing.T) { if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke") { revoked = true } - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) }) setActiveForDelete(t, "5", "gpu-box-01", "gpu-box-01") fn := &fakeNodeboot{executable: filepath.Join(t.TempDir(), "tracebloc")} @@ -675,8 +675,8 @@ func TestDelete_TypedNameMismatch_Cancels(t *testing.T) { func TestDelete_BrewManagedBinary_Hint(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -704,8 +704,8 @@ func TestDelete_BrewManagedBinary_Hint(t *testing.T) { func TestDelete_NoNamespace_WarnsAndSkipsUninstall(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -733,8 +733,8 @@ func TestDelete_NoNamespace_WarnsAndSkipsUninstall(t *testing.T) { func TestDelete_RevokeForbidden_OffboardCopy(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodGet && r.URL.Path == "/edge-device/admins/": _, _ = w.Write([]byte(`[]`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): @@ -814,8 +814,8 @@ func writeBinaryWithTBAlias(t *testing.T) string { func TestDelete_OwnTBAlias_Removed(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } @@ -839,8 +839,8 @@ func TestDelete_OwnTBAlias_Removed(t *testing.T) { func TestDelete_ForeignTBAlias_Left(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": - _, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`)) + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/": + _, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}`)) case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"): w.WriteHeader(http.StatusOK) } From c1f50ead8cb7e216f98e92f84f6ff034b6672d31 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 15 Jul 2026 19:56:11 +0500 Subject: [PATCH 4/4] fix(cli): non-numeric active client id is not-found, not a retryable error (Bugbot #338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lookupClientStatus returned a permanent error when ActiveClientID wasn't numeric. --wait only fail-fasts on 401/403/426/404, so it treated that parse error as transient and polled to the timeout — a regression vs the old ListClients+match path, which returned found=false (a non-numeric id can never match). Return (0, false, nil) so --wait fail-fasts on the missing client. Adds TestClientStatus_WaitFailsFastOnNonNumericID. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 8 +++++++- internal/cli/client_test.go | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index cd1ff9d..126175f 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -873,7 +873,13 @@ func runClientStatus(ctx context.Context, p *ui.Printer, wait bool, timeout time func lookupClientStatus(ctx context.Context, client *api.Client, active string) (status int, found bool, err error) { id, err := strconv.Atoi(active) if err != nil { - return 0, false, fmt.Errorf("active client id %q is not numeric: %w", active, err) + // A non-numeric active id can never match a backend client, so report it + // as not-found — exactly what the old ListClients+match path did — rather + // than a permanent error. A --wait loop fail-fasts on a missing client but + // treats errors as transient, so returning an error here would make it + // poll a permanent parse failure to the timeout (Bugbot: poll/retry loops + // must fail-fast on non-transient errors). + return 0, false, nil } c, err := client.GetClient(ctx, id) if err != nil { diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index 4ebfe2d..29e2ceb 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -1335,6 +1335,29 @@ func TestClientStatus_WaitFailsFastOnMissingClient(t *testing.T) { } } +// TestClientStatus_WaitFailsFastOnNonNumericID (Bugbot, #338 follow-up): a +// corrupt (non-numeric) active client id can never match a backend client, so +// --wait must fail fast on a missing client rather than retry a permanent parse +// error to the timeout. No backend call is expected (the id never parses). +func TestClientStatus_WaitFailsFastOnNonNumericID(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("no backend call expected for a non-numeric id, got %s %s", r.Method, r.URL.Path) + }) + setActiveClientID(t, "not-a-number") + // Long timeout: the test would hang if a non-numeric id were treated as a + // transient error instead of a missing client. + err := runClientStatus(context.Background(), ui.New(&bytes.Buffer{}), true, 10*time.Minute) + if got := ExitCodeFromError(err); got != 1 { + t.Fatalf("exit code = %d, want 1", got) + } + if err != nil && strings.Contains(err.Error(), "timed out") { + t.Errorf("a non-numeric id must fail fast, not time out: %v", err) + } + if err == nil || !strings.Contains(err.Error(), "isn't in your account") { + t.Errorf("want a missing-client error, got: %v", err) + } +} + // TestClientStatus_WaitTimeoutSurfacesListError (Bugbot #146-F): when every // status check fails, the timeout message must name the real error, not a bare // "unreachable". A 1ns timeout means the deadline passes on the first failure.