test(api): enforce race coverage for token cache

[rabesss]

Opened by my assistant.

Summary

Fixes #255.

• Add go test -race -count=1 -timeout 5m ./internal/client/... to CI
• Exercise distinct tokens and assert recorded Authorization headers and request errors
• Replace two external-network request-building tests with recording transports, preserving and strengthening request assertions

The race step is deliberately client-only; unrelated process timing issue #259 remains separate. No production code changes.

Validation

• Exact new client race CI command passes
• Temporary lock-free Go overlay removes only eight token mutex lock/unlock calls: both regression tests fail with three DATA RACE reports
• Real token tests pass 20 repetitions at each GOMAXPROCS 1, 2 and 4
• Independent reviewer repeated the race command and negative-control proof successfully
• Full vet, golangci-lint v2.12.1 (0 issues), and build with -buildvcs=false pass

Full go test ./... was attempted. All packages except the nine pre-existing player tests pass; this sandbox prohibits AF_UNIX sockets. Worktree builds require -buildvcs=false due to the environment's VCS lookup. Hosted CI and Security passed on the final head: https://github.com/rabesss/impartus-cli/actions/runs/37479525609. The latest title check also passes.

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[factory-droid]

Droid finished @rabesss's task —— View job

---

LGTM — no high-confidence actionable issues found.

0 inline comments posted

[pullfrog]

✅ No new issues found.

Reviewed changes — this run covered the full PR (1 commit): the new client race CI step, the strengthened token-cache race tests, and the two request-building tests converted to recording transports.

• CI race gate: new test-job step runs go test -race -count=1 -timeout 5m ./internal/client/... before the coverage pass, scoped client-only so #259 stays separate.
• Hermetic request-building tests: TestGetAuthorizedWithToken_NilToken and TestDoRequestWithTokenRequestBuilding swap real-network New(nil, nil) clients for recording transports, now asserting method, URL, and the exact Authorization header (plus that invalid requests never reach the transport).
• Strengthened race regressions: the fallback test alternates two different-length tokens and asserts every recorded Authorization header is one of the two complete bearers (count 1:1 with requests); the login test seeds a fallback token, checks every GetCourses error, and asserts the login token lands in both the client cache and cfg.Token.

Verified in both directions: the exact new CI command passes on this head (ok internal/client 4.551s), and a client_auth.go overlay with all eight tokenMu calls stripped makes both race tests fail under -race — the gate genuinely catches a missing lock, which is the whole point of #255.

  ^{｜ View workflow run ｜ Using glm-5.3 ｜ 𝕏}

[kilo-code-bot]

[SUGGESTION]: Exact-capacity buffered channel turns over-calls into a CI hang instead of an assertion failure

The buffer holds exactly the expected request count and the transport send blocks once it is full. assertTokenRaceHeaders's count check exists to catch a wrong number of requests, but if the client ever issues more than iterations requests (e.g. a future retry or redirect is added), the blocked send stalls GetCourses, wait.Wait() deadlocks, and this step burns the full 5-minute CI timeout instead of failing fast on the count mismatch. The same pattern exists in TestClientLoginAndFallbackRequestsShareTokenSafely (channel at line 86, handler send at line 95). Consider a non-blocking send that records drops, or an oversized buffer, so over-calls actually reach the count assertion.

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

[SUGGESTION]: Final-token assertion is silently coupled to the parity of iterations

This passes deterministically today only because iterations is even: the writer's last write is tokens[199%2] == secondToken. If iterations is ever changed to an odd value (e.g. while stress-tuning this constant), the last written token becomes firstToken and this assertion fails with a "want last written token" message that no longer matches the writer. Derive the expectation from the same expression the writer uses, e.g. wantLast := tokens[(iterations-1)%len(tokens)], and use it in both this comparison and the fatal message.

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	2

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
internal/client/token_race_test.go	30	Exact-capacity header channel makes over-count regressions hang CI for the full 5-minute timeout instead of failing on the count assertion (same pattern at lines 86/95)
internal/client/token_race_test.go	75	Final-token assertion silently depends on iterations being even; derive the expected last token from the writer's tokens[(iterations-1)%len(tokens)] expression

Files Reviewed (3 files)

• .github/workflows/ci.yml - 0 issues
• internal/client/streamutils_test.go - 0 issues
• internal/client/token_race_test.go - 2 issues

Fix these issues in Kilo Cloud

---

_{Reviewed by glm-5.3-flash · Input: 68.1K · Output: 17.5K · Cached: 371.8K}

_{Review guidance: REVIEW.md from base branch main}