feat(http): validate the /me and usage response shapes at runtime - #291
feat(http): validate the /me and usage response shapes at runtime#291Davidson3556 wants to merge 1 commit into
Conversation
Extends the TestSprite#266 validation layer to the account surfaces. `GET /me` is read by three commands through the generic `client.get`, all of which blind-cast the body: - `auth status`/`whoami` renders `m.scopes.join(', ')` and computes missing scopes via `m.scopes.includes(...)` with no guard, so a `/me` body without `scopes` crashed with a raw `TypeError: Cannot read properties of undefined (reading 'join')` (exit 1). Under `--output json` it was worse: the renderer never ran, so the CLI printed the partial identity and exited 0, and an agent reading `scopes` got undefined. - `usage` feeds `credits` / `creditsPerRun` into `Math.floor(credits / creditsPerRun)`, so a string balance reached the pre-flight arithmetic unchecked. - `doctor`'s connectivity probe reads a fully-optional projection whose local interface had already drifted from the stubbed schema (`v3Enabled` existed on the command side only). Adds `ME_RESPONSE_SCHEMA` and `USAGE_RESPONSE_SCHEMA`, wires the previously-unwired `ME_IDENTITY_SCHEMA` into `doctor`, and aliases doctor's `MeIdentity` to the schema's wire type so the two cannot drift again. Drift now surfaces as the standard typed INTERNAL envelope naming the mismatched field paths. Follows the TestSprite#266 policy unchanged: every object is `looseObject` so additive server fields pass through and reach `--output json` untouched; `env` is validated as an open string via `openWireLiteral` so a new deployment tier cannot hard-fail; only fields that every `/me` fixture in the suite supplies (`userId`, `keyId`, `scopes`, `env`) are required, and the genuinely absent-safe ones (`email`, `displayName`, `v3Enabled`, `credits`, `subPlan`, `creditsPerRun`) stay optional with no default. Refs TestSprite#277
|
✅ This PR is linked to an issue assigned to @Davidson3556 — thanks! The |
WalkthroughChangesGET /me response validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Approving the direction — this is the right implementation of #292 and it follows the policy #266 established: It sat 17 days fully green with no review, which was a pure review-bandwidth failure on our side, and in the meantime Splitting the account surfaces out as #292 so #277 could stay open for the remaining groups was exactly the right way to handle it. For anyone else reading: |
What does this PR do?
Extends the response-validation layer established by #266 to the account surfaces — the
GET /mereads and the usage projection. This is the first of the incremental groups #277 asks for (reads / writes / account surfaces); the test and project shapes are left for follow-ups.All three
/mecallers go through the genericclient.get, so all three still blind-cast the body:auth status/whoamiMeResponsem.scopes.join(', ')andm.scopes.includes(...)are unguarded → rawTypeError(exit 1) in text mode, and exit 0 with a partial identity under--output jsonusage/creditsUsageResponsecreditsreached theMath.floor(credits / creditsPerRun)pre-flight arithmeticdoctorMeIdentityv3Enabledexisted on the command side only)Changes:
ME_RESPONSE_SCHEMAandUSAGE_RESPONSE_SCHEMAinsrc/lib/response-schemas.ts.ME_IDENTITY_SCHEMAintodoctor, and adds the missingv3Enabledto it.doctor'sMeIdentityis now a type alias of the schema's wire type, so interface and schema cannot drift apart again.client.get('/me', { schema })call sites.initinherits validation throughrunWhoami, where the existing try/catch already degrades to a placeholder identity — so a drifted/menever fails the wholeinit.Policy compliance (#266, unchanged)
looseObject: additive server fields pass and are preserved, so--output jsonstays byte-faithful. Covered by a test that sends an unknownorgNamethroughwhoami --output json.envis validated as an open string viaopenWireLiteral, so a new deployment tier (sandbox, …) cannot hard-fail the CLI.userId,keyId,scopes,env. Every/mefixture in the suite supplies all four (auth.test.ts,init.test.ts,usage.test.ts,cli.subprocess.test.ts,test/mock-backend/fixtures.ts,lib/dry-run/samples.ts), andscopesis exactly the field whose absence crashes today.email,displayName,v3Enabled,credits,subPlan,creditsPerRun. These are the documented forward-compat fields the backend does not send yet; every renderer branch is already gated on presence, so absence must stay absence.doctordeliberately keeps the fully-optional projection rather than reusingME_RESPONSE_SCHEMA:OK_MEindoctor.test.tsis{ userId, keyId }with noscopes/env, and a connectivity probe must not fail on a partial identity.Deliberately not wired
The pre-write ping in
runConfigure(auth setup) discards the/mebody and only checks that the key was accepted. Validating there would let an unrelated/mefield change block credential setup, which is the one path a user needs when everything else is broken.Related issue
Closes #292 — the account-surfaces slice of #277.
#277 is an umbrella that lists seven shapes and says "one PR per coherent group of shapes is fine", so it stays open for the remaining groups rather than being closed by this PR. #292 scopes just the
/meand usage surfaces and is assigned to me.Type of change
Checklist
mainbranch.feat(...),fix(...),docs(...), …).npm run lintandnpm run format:checkpass.npm run typecheckpasses.npm testpasses and coverage stays at or above the 80% gate.README.md/DOCUMENTATION.mdwhere relevant. — n/a, no user-facing surface changes; the only visible difference is a crash becoming a typed envelope.Notes for reviewers
Before / after
Against a local server returning a
/mebody withoutscopes:Before (main) — text mode leaks an internal
TypeError:Before (main) — JSON mode is worse, it silently succeeds:
An agent reading
error.codesees nothing andscopesisundefined.After (this branch):
Gates
npm run typecheck,npm run lint,npm run format:check,npm test(2041 passed / 2 skipped, 58 files),npm run build,npm run test:e2e(60 passed / 1 skipped) — all green on this branch. The onlyformat:checkwarning is the untracked local.claude/settings.local.json, which is not part of this change.Follow-ups
Happy to take the remaining #277 groups in separate PRs: the read shapes (
CliTest,CliTestCode,CliProject), the write shapes (CliCreateTestResponse),CliFailureSummary, and the leftoveras Tcasts in thehttp.tshelpers. Also happy to split or reshape this one if you would rather see a single larger PR.AI usage
Written with AI assistance (Claude Code). I reviewed every line, chose the required-vs-optional split from the fixtures cited above, and verified the before/after behavior and all gates locally.