From 5a25bd24736416a0562618d3efa9f89b4453a4bb Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 24 Jun 2026 20:16:20 +0500 Subject: [PATCH 1/4] fix(cli): back off device-flow poll by 5s on slow_down (RFC 8628) (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On `slow_down`, runLogin bumped the poll interval by 1s (`interval++`). RFC 8628 §3.5 requires increasing it by 5s for that and all subsequent polls, so the CLI kept polling too aggressively after the server asked it to back off. Test captures the durations handed to the pollAfter seam: post-slow_down wait is now 10s (5+5), not 6s. Bugbot: "Device flow slow_down interval" (Medium, v0.4.0 RC, #107) Co-Authored-By: Claude Opus 4.8 --- internal/cli/auth.go | 4 +++- internal/cli/auth_test.go | 49 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 6f159653..836d4797 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -123,7 +123,9 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { case errors.Is(err, api.ErrAuthorizationPending): // not approved yet — keep polling case errors.Is(err, api.ErrSlowDown): - interval++ + // RFC 8628 §3.5: on slow_down the client MUST increase the poll + // interval by 5 seconds for this and all subsequent polls. + interval += 5 case errors.Is(err, api.ErrExpiredToken): return &exitError{code: 1, err: errors.New("the sign-in code expired — re-run `tracebloc login`")} case errors.Is(err, api.ErrAccessDenied): diff --git a/internal/cli/auth_test.go b/internal/cli/auth_test.go index 5be742b6..c4bba27a 100644 --- a/internal/cli/auth_test.go +++ b/internal/cli/auth_test.go @@ -87,6 +87,55 @@ func TestLogin_FullFlow(t *testing.T) { } } +// TestLogin_SlowDownBacksOffByFive pins RFC 8628 §3.5: on `slow_down` the poll +// interval must increase by 5 seconds, not 1. Captures the durations handed to +// the pollAfter seam. +func TestLogin_SlowDownBacksOffByFive(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + var polls int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/device/code": + _, _ = w.Write([]byte(`{"device_code":"dc","user_code":"X","verification_uri":"https://x/activate","expires_in":600,"interval":5}`)) + case "/device/token": + polls++ + if polls == 1 { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"slow_down"}`)) + return + } + _, _ = w.Write([]byte(`{"token":"cat_ok"}`)) + case "/userinfo/": + _, _ = w.Write([]byte(`{"email":"e@co","account":"A"}`)) + } + })) + t.Cleanup(srv.Close) + + origClient, origAfter := newAPIClient, pollAfter + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + var waits []time.Duration + pollAfter = func(d time.Duration) <-chan time.Time { + waits = append(waits, d) + ch := make(chan time.Time, 1) + ch <- time.Time{} + return ch + } + t.Cleanup(func() { newAPIClient = origClient; pollAfter = origAfter }) + + if _, err := runCmd(t, "login"); err != nil { + t.Fatalf("login: %v", err) + } + if len(waits) < 2 { + t.Fatalf("expected >=2 polls, got waits=%v", waits) + } + if waits[0] != 5*time.Second { + t.Errorf("first poll wait = %v, want 5s (server interval)", waits[0]) + } + if waits[1] != 10*time.Second { + t.Errorf("post-slow_down wait = %v, want 10s (interval+5 per RFC 8628), not 6s", waits[1]) + } +} + func TestLogin_BackendUnsupported(t *testing.T) { withTestBackend(t, func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) From a1e4eb0af111564965867923059d0519386b1d31 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 24 Jun 2026 20:16:20 +0500 Subject: [PATCH 2/4] fix(doctor): resolve CLIENT_ENV case-insensitively in backendHost (#106) backendHost switched on CLIENT_ENV case-sensitively, but the API client (api.ResolveEnv/BaseURL) lowercases env values. A non-lowercase CLIENT_ENV on the edge box (e.g. "DEV") fell through to the prod default, so `cluster doctor` probed api.tracebloc.io even when the cluster targeted dev/stg. Normalize with ToLower+TrimSpace before the switch. Test extends TestBackendHost with "DEV"/"Stg"/" dev " cases. Bugbot: "Doctor backend env casing" (Low, v0.4.0 RC, #107) Co-Authored-By: Claude Opus 4.8 --- internal/doctor/doctor.go | 5 ++++- internal/doctor/doctor_test.go | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index fdcc4ecd..d69ce9c8 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -288,7 +288,10 @@ func checkBackendEgress(ctx context.Context, env map[string]string, probe func(c // runtime's own mapping (controller.py). Unset/unknown defaults to prod, the // chart's CLIENT_ENV default. func backendHost(clientEnv string) string { - switch clientEnv { + // Normalize like the API client (api.ResolveEnv/BaseURL lower-case), so a + // non-lowercase CLIENT_ENV on the edge box doesn't fall through to prod and + // make the doctor probe the wrong backend. + switch strings.ToLower(strings.TrimSpace(clientEnv)) { case "dev": return "dev-api.tracebloc.io" case "stg": diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index 766fe933..7eaaa22b 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -232,6 +232,11 @@ func TestBackendHost(t *testing.T) { "prod": "api.tracebloc.io", "": "api.tracebloc.io", "weird": "api.tracebloc.io", + // Case/space-insensitive, matching the API client's env resolution — a + // non-lowercase CLIENT_ENV must not fall through to prod. + "DEV": "dev-api.tracebloc.io", + "Stg": "stg-api.tracebloc.io", + " dev ": "dev-api.tracebloc.io", } for in, want := range tests { if got := backendHost(in); got != want { From fb92d1eafda61a9c86d9d29ef5ed964c1036861e Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 24 Jun 2026 20:16:20 +0500 Subject: [PATCH 3/4] fix(api): guard the bare-array list decode to the first page (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ListClients attempted the unpaginated bare-array decode on every iteration. A bare array is only valid as the sole response (a paginated chain is a {next,results} object on every page), so a stray bare body mid-chain could silently end the loop and drop earlier pages. Guard the bare decode to pageNum 0. Test: a bare-array response still returns the full list. Found in the proactive RC review (low; latent — DRF doesn't mix shapes). Co-Authored-By: Claude Opus 4.8 --- internal/api/client.go | 12 ++++++++---- internal/api/client_test.go | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 5ff51cc9..f0e8187c 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -333,10 +333,14 @@ func (c *Client) ListClients(ctx context.Context) ([]ProvisionedClient, error) { if status < 200 || status >= 300 { return nil, &APIError{StatusCode: status, Body: string(raw), URL: reqURL} } - // Unpaginated deployment → a bare array; return it as-is. - var bare []ProvisionedClient - if err := json.Unmarshal(raw, &bare); err == nil { - return append(all, bare...), nil + // Unpaginated deployment → a bare array. Only valid as the sole response + // (a paginated chain is a `{next,results}` object on every page), so guard + // to page 0 — a stray bare body mid-chain must not silently end the loop. + if pageNum == 0 { + var bare []ProvisionedClient + if err := json.Unmarshal(raw, &bare); err == nil { + return bare, nil + } } var body struct { Next string `json:"next"` diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 8cad599f..e091b79c 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -234,3 +234,22 @@ func TestListClients_UnparseableNextLink_IsError(t *testing.T) { t.Fatal("expected an error on an unparseable next link, got nil (silent truncation)") } } + +// TestListClients_BareArrayUnpaginated covers the unpaginated deployment shape +// (a bare JSON array) — still returned as-is after the bare-decode was guarded +// to the first page. +func TestListClients_BareArrayUnpaginated(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`[{"id":1,"first_name":"a","namespace":"a"},{"id":2,"first_name":"b","namespace":"b"}]`)) + })) + defer srv.Close() + c := New("prod") + c.BaseURL = srv.URL + got, err := c.ListClients(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(got) != 2 || got[0].ID != 1 || got[1].ID != 2 { + t.Fatalf("want 2 clients [1,2] from a bare array, got %+v", got) + } +} From 87777ba32116915c0affd408c1979ccfbbfd271a Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 24 Jun 2026 20:16:21 +0500 Subject: [PATCH 4/4] fix(cli): exclude this cluster's own client from create collision check (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `client create` built the namespace-collision set from ALL clients, including the one already anchored to this cluster. On an idempotent re-run with the same --name, that bumped the derived slug (lab-one → lab-one-2) and showed it in the review — but the backend adopts on cluster_id and returns the original namespace, so the review contradicted the actual outcome. Skip the client whose cluster_id matches this cluster's anchor. Test: a re-run review no longer shows a bumped namespace. Found in the proactive RC review (low; cosmetic — backend state was correct). Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 7 +++++++ internal/cli/client_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/internal/cli/client.go b/internal/cli/client.go index c4d06a3f..328bbc32 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -167,6 +167,13 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien var existing []string if clients, lerr := client.ListClients(ctx); lerr == nil { for _, c := range clients { + // Skip the client already anchored to this cluster: a re-run adopts it + // (the backend keys on cluster_id), so its namespace isn't a collision. + // Counting it would bump the derived slug and show a namespace in the + // review that doesn't match the one actually adopted. + if clusterID != "" && c.ClusterID == clusterID { + continue + } if c.Namespace != "" { existing = append(existing, c.Namespace) } diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index bd79021d..dcdcc1cf 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -521,3 +521,31 @@ func TestClientCreate_NoClusterAnchorWarns(t *testing.T) { t.Errorf("no-anchor fallback should still print the credential, got:\n%s", out.String()) } } + +// TestClientCreate_ReRunReviewShowsAdoptedNamespace pins that on an idempotent +// re-run, the client already anchored to this cluster is excluded from collision +// detection — so the review shows the namespace that's actually adopted +// (lab-one), not a bumped lab-one-2. +func TestClientCreate_ReRunReviewShowsAdoptedNamespace(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": + // An existing client already anchored to THIS cluster (uid-1). + _, _ = w.Write([]byte(`[{"id":1,"first_name":"Lab One","username":"u-1","namespace":"lab-one","location":"DE","cluster_id":"uid-1"}]`)) + case r.Method == http.MethodPost && r.URL.Path == "/edge-device/": + w.WriteHeader(http.StatusOK) // adopt + _, _ = w.Write([]byte(`{"id":1,"first_name":"Lab One","username":"u-1","namespace":"lab-one","location":"DE","cluster_id":"uid-1"}`)) + } + }) + stubClusterID(t, "uid-1", nil) + confirmYes := true + pr := &fakePrompter{answers: map[string]string{}, confirm: &confirmYes} + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), pr, + clientCreateOpts{name: "Lab One", location: "DE"}); err != nil { + t.Fatalf("re-run create: %v", err) + } + if strings.Contains(out.String(), "lab-one-2") { + t.Errorf("review showed a bumped namespace — the cluster's own client wasn't excluded from collision detection:\n%s", out.String()) + } +}