Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/commands/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,41 @@ describe('runWhoami', () => {
expect(printed).toEqual(sampleMe);
});

// Issue #277: `/me` is validated against ME_RESPONSE_SCHEMA at the client
// boundary, so wire drift becomes a typed envelope instead of a crash.
it('turns a /me body without scopes into a typed INTERNAL envelope, not a raw TypeError', async () => {
writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath });
const { deps } = makeCapture();
const withoutScopes: Record<string, unknown> = { ...sampleMe };
delete withoutScopes.scopes;
const drifted = new Response(JSON.stringify(withoutScopes), {
status: 200,
headers: { 'content-type': 'application/json' },
});
const rejection = await runWhoami(
{ profile: 'default', output: 'text', debug: false },
{ ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(drifted) },
).catch((error: unknown) => error);
// Before: `m.scopes.join(', ')` threw `TypeError: ... not a function`.
expect(rejection).toBeInstanceOf(ApiError);
expect(rejection).toMatchObject({ code: 'INTERNAL' });
expect(JSON.stringify((rejection as ApiError).getDetail('issues'))).toContain('scopes');
});

it('passes an additive /me field straight through to --output json', async () => {
writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath });
const { capture, deps } = makeCapture();
const additive = new Response(JSON.stringify({ ...sampleMe, orgName: 'Acme' }), {
status: 200,
headers: { 'content-type': 'application/json' },
});
await runWhoami(
{ profile: 'default', output: 'json', debug: false },
{ ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(additive) },
);
expect(JSON.parse(capture.stdout.join('')).orgName).toBe('Acme');
});

it('renders routing: v3 and the gap advisory when v3Enabled is true', async () => {
writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath });
const { capture, deps } = makeCapture();
Expand Down
3 changes: 2 additions & 1 deletion src/commands/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { emitDeprecationNotice } from '../lib/deprecate.js';
import type { OutputMode } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js';
import { promptSecret } from '../lib/prompt.js';
import { ME_RESPONSE_SCHEMA } from '../lib/response-schemas.js';
import { emitV3RoutingAdvisory, routingLabel } from '../lib/v3-advisory.js';

export interface MeResponse {
Expand Down Expand Up @@ -261,7 +262,7 @@ export async function runWhoami(opts: CommonOptions, deps: AuthDeps = {}): Promi
stderr: deps.stderr,
});

const me = await client.get<MeResponse>('/me');
const me = await client.get<MeResponse>('/me', { schema: ME_RESPONSE_SCHEMA });
out.print(me, data => {
const m = data as MeResponse;
const lines = [
Expand Down
17 changes: 10 additions & 7 deletions src/commands/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import { loadConfig } from '../lib/config.js';
import { ApiError, CLIError, localValidationError } from '../lib/errors.js';
import type { FetchImpl } from '../lib/http.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';
import type { MeIdentityWire } from '../lib/response-schemas.js';
import { ME_IDENTITY_SCHEMA } from '../lib/response-schemas.js';
import { isVerifySkillInstalled } from '../lib/skill-nudge.js';
import { emitV3RoutingAdvisory, routingLabel } from '../lib/v3-advisory.js';
import { VERSION } from '../version.js';
Expand All @@ -46,12 +48,13 @@ export interface DoctorReport {
warnings: number;
}

/** Minimal projection of `GET /me` we read for the connectivity detail. */
interface MeIdentity {
userId?: string;
keyId?: string;
v3Enabled?: boolean;
}
/**
* Minimal projection of `GET /me` we read for the connectivity detail.
*
* Aliased to the schema's wire type so the interface and
* {@link ME_IDENTITY_SCHEMA} cannot drift apart (issue #277).
*/
type MeIdentity = MeIdentityWire;

export interface DoctorDeps {
env?: NodeJS.ProcessEnv;
Expand Down Expand Up @@ -213,7 +216,7 @@ async function checkConnectivity(
fetchImpl: deps.fetchImpl,
stderr: deps.stderr,
});
const me = await client.get<MeIdentity>('/me');
const me = await client.get<MeIdentity>('/me', { schema: ME_IDENTITY_SCHEMA });
const who = me.userId ? ` (userId ${me.userId})` : '';
return {
check: { name, status: 'ok', detail: `reached GET /me, API key accepted${who}` },
Expand Down
17 changes: 17 additions & 0 deletions src/commands/usage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,23 @@ describe('runUsage — real path without credits (current backend)', () => {
expect(stderr).toContain('testsprite.com');
});

// Issue #277: the usage projection is validated at the client boundary, so a
// string balance surfaces as a typed envelope instead of silently reaching
// the `Math.floor(credits / creditsPerRun)` pre-flight arithmetic.
it('rejects a non-numeric credit balance with a typed INTERNAL envelope', async () => {
writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath });
const { deps } = makeCapture();
const rejection = await runUsage(
{ profile: 'default', output: 'text', debug: false },
{
...deps,
credentialsPath,
fetchImpl: makeFetch({ ...meWithoutCredits, credits: '100', creditsPerRun: 2 }),
},
).catch((error: unknown) => error);
expect(rejection).toMatchObject({ code: 'INTERNAL' });
});

it('text output includes identity fields even without credits', async () => {
writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath });
const { capture, deps } = makeCapture();
Expand Down
3 changes: 2 additions & 1 deletion src/commands/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { loadConfig } from '../lib/config.js';
import { resolvePortalBase } from '../lib/facade.js';
import type { FetchImpl } from '../lib/http.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';
import { USAGE_RESPONSE_SCHEMA } from '../lib/response-schemas.js';

/**
* Usage/balance response from `/me` (when the backend supplies it) or a future
Expand Down Expand Up @@ -117,7 +118,7 @@ export async function runUsage(opts: CommonOptions, deps: UsageDeps = {}): Promi
// /me is the only available source of credits/plan today.
// When the backend adds credits/subPlan to MeResponse (or adds /usage),
// this single get call is sufficient — no code change needed in the CLI.
const me = await client.get<UsageResponse>('/me');
const me = await client.get<UsageResponse>('/me', { schema: USAGE_RESPONSE_SCHEMA });

out.print(me, data => renderUsage(data as UsageResponse, portalBase));

Expand Down
102 changes: 101 additions & 1 deletion src/lib/response-schemas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
import { describe, expect, it } from 'vitest';
import * as v from 'valibot';
import { HttpClient } from './http.js';
import { RUN_RESPONSE_SCHEMA, TRIGGER_RUN_RESPONSE_SCHEMA } from './response-schemas.js';
import {
ME_IDENTITY_SCHEMA,
ME_RESPONSE_SCHEMA,
RUN_RESPONSE_SCHEMA,
TRIGGER_RUN_RESPONSE_SCHEMA,
USAGE_RESPONSE_SCHEMA,
} from './response-schemas.js';

const VALID_RUN = {
runId: 'run_1',
Expand Down Expand Up @@ -102,3 +108,97 @@ describe('TRIGGER_RUN_RESPONSE_SCHEMA', () => {
expect(parsed.success).toBe(true);
});
});

// ---------------------------------------------------------------------------
// Account surfaces (issue #277): GET /me and its usage projection.
// ---------------------------------------------------------------------------

const VALID_ME = {
userId: 'u_1',
keyId: 'k_1',
scopes: ['read:projects', 'read:tests'],
env: 'development',
};

describe('ME_RESPONSE_SCHEMA', () => {
it('accepts the minimal /me body and preserves unknown extra keys', () => {
const parsed = v.safeParse(ME_RESPONSE_SCHEMA, { ...VALID_ME, plan: 'Pro' });
expect(parsed.success).toBe(true);
if (parsed.success) {
expect((parsed.output as { plan?: string }).plan).toBe('Pro');
}
});

it('accepts an unknown env value (a new deployment tier must not hard-fail)', () => {
expect(v.safeParse(ME_RESPONSE_SCHEMA, { ...VALID_ME, env: 'sandbox' }).success).toBe(true);
});

it('leaves the absent-safe identity fields absent rather than defaulting them', () => {
const parsed = v.safeParse(ME_RESPONSE_SCHEMA, VALID_ME);
expect(parsed.success).toBe(true);
if (parsed.success) {
expect('email' in parsed.output).toBe(false);
expect('displayName' in parsed.output).toBe(false);
expect('v3Enabled' in parsed.output).toBe(false);
}
});

it('rejects a /me body without scopes, naming the path', () => {
const withoutScopes: Record<string, unknown> = { ...VALID_ME };
delete withoutScopes.scopes;
const parsed = v.safeParse(ME_RESPONSE_SCHEMA, withoutScopes);
expect(parsed.success).toBe(false);
if (!parsed.success) {
expect(parsed.issues.some(issue => v.getDotPath(issue) === 'scopes')).toBe(true);
}
});
});

describe('ME_IDENTITY_SCHEMA', () => {
it("accepts doctor's partial identity projection (connectivity must not fail on it)", () => {
expect(v.safeParse(ME_IDENTITY_SCHEMA, { userId: 'u-doc', keyId: 'k-doc' }).success).toBe(true);
});

it('carries v3Enabled through so the routing advisory still fires', () => {
const parsed = v.safeParse(ME_IDENTITY_SCHEMA, { ...VALID_ME, v3Enabled: true });
expect(parsed.success).toBe(true);
if (parsed.success) {
expect(parsed.output.v3Enabled).toBe(true);
}
});

it('rejects a wrongly-typed identity field', () => {
expect(v.safeParse(ME_IDENTITY_SCHEMA, { userId: 42 }).success).toBe(false);
});
});

describe('USAGE_RESPONSE_SCHEMA', () => {
it("accepts today's /me body, which carries no credits fields at all", () => {
const parsed = v.safeParse(USAGE_RESPONSE_SCHEMA, VALID_ME);
expect(parsed.success).toBe(true);
if (parsed.success) {
expect(parsed.output.credits).toBeUndefined();
// `scopes` is not part of the usage projection but must survive as an
// unknown extra key so `--output json` stays byte-faithful.
expect((parsed.output as { scopes?: string[] }).scopes).toEqual(VALID_ME.scopes);
}
});

it('accepts the future body with credits, plan and per-run cost', () => {
const parsed = v.safeParse(USAGE_RESPONSE_SCHEMA, {
...VALID_ME,
credits: 100,
subPlan: 'Standard',
creditsPerRun: 2,
});
expect(parsed.success).toBe(true);
});

it('rejects a non-numeric credit balance instead of rendering NaN math', () => {
const parsed = v.safeParse(USAGE_RESPONSE_SCHEMA, { ...VALID_ME, credits: '100' });
expect(parsed.success).toBe(false);
if (!parsed.success) {
expect(parsed.issues.some(issue => v.getDotPath(issue) === 'credits')).toBe(true);
}
});
});
91 changes: 80 additions & 11 deletions src/lib/response-schemas.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
/**
* Valibot schemas for the run-path wire shapes (issue #102).
* Valibot schemas for the API wire shapes (issue #102, extended by #277).
*
* `requestWithMeta` used to return `(await response.json()) as T` with zero
* runtime validation, so a drifted or partial server response surfaced as
* `undefined` output or an opaque TypeError deep inside a command. These
* schemas are wired (opt-in, via `RequestOptions.schema`) into the typed
* HttpClient helpers only: `triggerRun`, `triggerRunWithMeta`, `triggerRerun`,
* `triggerBatchRerun`, `triggerBatchRunFresh`, `getRun`, `listTestRuns`.
* The generic `get`/`post`/`put`/`patch`/`delete` paths stay schema-free.
* HttpClient helpers `triggerRun`, `triggerRunWithMeta`, `triggerRerun`,
* `triggerBatchRerun`, `triggerBatchRunFresh`, `getRun`, `listTestRuns`, and
* — for the account surfaces below — at the `client.get` call sites that own
* a `/me` or usage read. Every other generic `get`/`post`/`put`/`patch`/
* `delete` caller stays schema-free and opt-in.
*
* Resilience rules (additive server changes must never hard-fail the CLI):
*
Expand All @@ -32,6 +34,11 @@
* interface it mirrors, so schema/interface drift fails `tsc` in this file.
*/
import * as v from 'valibot';
// Type-only imports: erased at compile time, so pulling a command's wire
// interface into `lib/` adds no runtime edge (same pattern as `bundle.ts`,
// which type-imports `CliTestStep` from `commands/test.ts`).
import type { MeResponse } from '../commands/auth.js';
import type { UsageResponse } from '../commands/usage.js';
import type {
BatchRerunResponse,
BatchRunFreshResponse,
Expand All @@ -44,6 +51,9 @@ import type {
TriggerRunResponse,
} from './runs.types.js';

/** Deployment environment the bound key belongs to; open on the wire (rule 2). */
type AccountEnv = 'development' | 'staging' | 'production';

/**
* Compile-time literal union, runtime open string.
*
Expand Down Expand Up @@ -251,21 +261,80 @@ export const LIST_RUNS_RESPONSE_SCHEMA: v.GenericSchema<unknown, ListRunsRespons
// ---------------------------------------------------------------------------

/**
* Minimal `/me` identity core shared by its consumers. `doctor` reads a
* two-field optional projection (`MeIdentity` in commands/doctor.ts) while
* `auth whoami` reads the full `MeResponse` (commands/auth.ts); this schema
* validates the common identity core so it can guard either caller, and
* `looseObject` lets the full projection (scopes, env, email, ...) pass
* through untouched. Not wired into any typed helper yet: `/me` callers use
* the generic `get`, which stays schema-free in this change.
* Minimal `/me` identity core, as read by `doctor`'s connectivity check.
*
* `doctor` deliberately treats every field as optional: the check only needs
* "the key was accepted", and it decorates the detail line with the userId
* *when present* (`me.userId ? ...`). Fixture evidence for keeping it fully
* optional rather than reusing {@link ME_RESPONSE_SCHEMA}: `OK_ME` in
* `commands/doctor.test.ts` is `{ userId, keyId }` with no `scopes`/`env`, and
* a connectivity probe must not fail on a partial identity projection.
*
* `commands/doctor.ts` aliases its `MeIdentity` to this type so the two cannot
* drift (they already had: `v3Enabled` existed on the command side only).
*/
export interface MeIdentityWire {
userId?: string;
keyId?: string;
/** Authoritative per-user V3 routing bit; older backends omit it. */
v3Enabled?: boolean;
}

/** Mirrors `MeIdentity` (commands/doctor.ts): `GET /api/cli/v1/me` core. */
export const ME_IDENTITY_SCHEMA: v.GenericSchema<unknown, MeIdentityWire> = v.looseObject({
userId: v.optional(v.string()),
keyId: v.optional(v.string()),
v3Enabled: v.optional(v.boolean()),
});

/**
* Mirrors `MeResponse` (commands/auth.ts): the full `GET /me` projection read
* by `auth whoami` (and, through it, `init`).
*
* `scopes` is required and array-typed on purpose — this is the shape drift
* that actually bites today. `runWhoami` renders `m.scopes.join(', ')` and
* computes `missingScopes` via `m.scopes.includes(...)` with no guard, so a
* `/me` body without `scopes` crashes with a raw `TypeError` (exit 1) instead
* of a typed envelope. Every `/me` fixture in the suite supplies it
* (`auth.test.ts`, `init.test.ts`, `usage.test.ts`, `cli.subprocess.test.ts`,
* `test/mock-backend/fixtures.ts`), so requiring it matches observed wire
* reality; `email` / `displayName` / `v3Enabled` are the genuinely absent-safe
* ones and stay `v.optional` with no default (rule 3, optional branch).
*
* `init` calls this through `runWhoami` inside a try/catch that falls back to
* a placeholder identity, so a drifted `/me` degrades the setup summary
* instead of failing the whole `init`.
*/
export const ME_RESPONSE_SCHEMA: v.GenericSchema<unknown, MeResponse> = v.looseObject({
userId: v.string(),
keyId: v.string(),
scopes: v.array(v.string()),
env: openWireLiteral<AccountEnv>(),
email: v.optional(v.string()),
displayName: v.optional(v.string()),
v3Enabled: v.optional(v.boolean()),
});

// ---------------------------------------------------------------------------
// GET /me (usage projection)
// ---------------------------------------------------------------------------

/**
* Mirrors `UsageResponse` (commands/usage.ts): the credits/plan projection the
* `usage` command reads off the same `GET /me` body.
*
* `renderUsage` prints `userId`/`keyId`/`env` unconditionally as its "identity
* block", so those three are required; `credits`, `subPlan` and
* `creditsPerRun` are forward-compat fields the backend does not send today
* (see the BACKEND FOLLOW-UP note in usage.ts) and every renderer branch is
* gated on `!== undefined`, so they stay optional with no default. `scopes`
* rides along as an unknown extra key and is preserved by `looseObject`.
*/
export const USAGE_RESPONSE_SCHEMA: v.GenericSchema<unknown, UsageResponse> = v.looseObject({
userId: v.string(),
keyId: v.string(),
env: openWireLiteral<AccountEnv>(),
credits: v.optional(v.number()),
subPlan: v.optional(v.string()),
creditsPerRun: v.optional(v.number()),
});
Loading