fix: proactive v0.4.0 RC review — fix 4 findings, document 3 non-issues (#106)

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"`

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)
+	}
+}

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):

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)

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)
  }

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())
+	}
+}

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":

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 {