Skip to content

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

Merged
saadqbal merged 4 commits into
developfrom
fix/rc-review-findings
Jun 24, 2026
Merged

fix: proactive v0.4.0 RC review — fix 4 findings, document 3 non-issues (#106)#110
saadqbal merged 4 commits into
developfrom
fix/rc-review-findings

Conversation

@saadqbal

@saadqbal saadqbal commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

A proactive review of the whole developmain RC diff (4,142 lines / 31 files) to get ahead of Cursor Bugbot's incremental passes on #107. Combined Bugbot's outstanding findings with a fan-out review across the high-risk packages (auth, api, config, client, cluster, doctor, push, slug), then adversarially verified every candidate against the code. Rolls up under #106.

Fixed (4 — all verified real, each with a regression test)

# Sev Where Issue Source
1 Med cli/auth.go device-flow slow_down bumped the poll interval by 1s, not the 5s RFC 8628 §3.5 requires — kept polling too aggressively after the server said back off Bugbot
2 Low doctor/doctor.go backendHost matched CLIENT_ENV case-sensitively while the API client lowercases — a "DEV" env made cluster doctor probe prod on a dev/stg cluster Bugbot
3 Low api/client.go ListClients tried the bare-array decode on every page; a stray bare body mid-chain could silently end pagination early. Guarded to page 0 review
4 Low cli/client.go client create counted this cluster's own client in the namespace-collision set, so an idempotent re-run showed a bumped lab-one-2 in the review while the backend adopted lab-one review

Reviewed and deliberately NOT changed (3 — documented so they're consciously closed, not missed)

  • Poll loop doesn't put the deadline in its select (auth.go) — overshoot is bounded by one poll interval against a 10-min code TTL; negligible. Not worth restructuring.
  • PollToken treats a 2xx body as success (api/client.go) — the CLI is coded to the RFC 8628 + its own tested contract (poll errors arrive on 4xx; 2xx = token). Treating a 2xx error field as a poll sentinel could mask a real backend bug. Won't fix.
  • randHex ignores the crypto/rand.Read error (cli/client.go) — crypto/rand.Read is contractually non-failing on supported platforms (the code says so); adding error plumbing for an unreachable path is noise.

push/dataset/slug/teardown and the rest of doctor came back clean under a deep trace (incl. table-name traversal, DROP-TABLE quoting, teardown pod/uid targeting, slug collision, node-fit math).

Tests

4 new regression tests (slow_down back-off timing via the pollAfter seam; backendHost case-insensitivity; bare-array list; re-run review namespace). gofmt/vet/build clean; full go test -race -cover ./... green.

🤖 Generated with Claude Code


Note

Low Risk
Targeted correctness fixes in login polling, doctor probes, client listing, and create review text; behavior changes are narrow and covered by new tests.

Overview
Addresses four proactive RC review findings, each with a regression test.

Login (tracebloc login) — On OAuth device-flow slow_down, the poll interval now increases by 5 seconds (RFC 8628 §3.5), not 1 second, so the CLI backs off as the spec requires.

cluster doctorbackendHost normalizes CLIENT_ENV with trim + lower-case, aligned with the API client, so values like "DEV" or " dev " probe dev-api instead of defaulting to prod.

ListClients — Unpaginated bare JSON arrays are only accepted on page 0; a stray bare body later in a paginated chain can no longer end pagination early and truncate the client list.

client create — When deriving the namespace for the review step, clients already anchored to the current cluster are excluded from collision detection, so an idempotent re-run shows the adopted namespace (e.g. lab-one) instead of a bumped slug (lab-one-2).

Reviewed by Cursor Bugbot for commit 87777ba. Bugbot is set up for automated code reviews on this repo. Configure here.

saadqbal and others added 4 commits June 24, 2026 20:16
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…ck (#106)

`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 <noreply@anthropic.com>
@saadqbal saadqbal self-assigned this Jun 24, 2026
@saadqbal saadqbal merged commit 818db1c into develop Jun 24, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants