feat(http): validate the /me and usage response shapes at runtime#291
Open
Davidson3556 wants to merge 1 commit into
Open
feat(http): validate the /me and usage response shapes at runtime#291Davidson3556 wants to merge 1 commit into
Davidson3556 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.