diff --git a/src/commands/project.test.ts b/src/commands/project.test.ts index 1dde21f..d034b0c 100644 --- a/src/commands/project.test.ts +++ b/src/commands/project.test.ts @@ -631,6 +631,79 @@ describe('createProjectCommand --page-size option parser', () => { }); }); +describe('project read response validation', () => { + it.each([ + ['missing name', { ...PROJECT_FIXTURE, name: undefined }], + ['invalid attribute list', { ...PROJECT_FIXTURE, testIdAttributes: 'data-testid' }], + ['invalid nullable URL', { ...PROJECT_FIXTURE, targetUrl: false }], + ])('rejects %s before printing a successful result', async (_label, body) => { + const out: string[] = []; + await expect( + runGet( + { profile: 'default', output: 'json', debug: false, projectId: PROJECT_FIXTURE.id }, + { ...makeCreds(), fetchImpl: makeFetch(() => ({ body })), stdout: line => out.push(line) }, + ), + ).rejects.toMatchObject({ code: 'INTERNAL', exitCode: 1 }); + expect(out).toEqual([]); + }); + + it.each([false, true])('validates list rows before output (single page: %s)', async single => { + const out: string[] = []; + let calls = 0; + const fetchImpl = makeFetch(() => { + calls += 1; + return { + body: + !single && calls === 1 + ? { items: [PROJECT_FIXTURE], nextToken: 'second-page' } + : { items: [{ ...PROJECT_FIXTURE, id: null }], nextToken: null }, + }; + }); + await expect( + runList( + { profile: 'default', output: 'text', debug: false, ...(single ? { pageSize: 1 } : {}) }, + { ...makeCreds(), fetchImpl, stdout: line => out.push(line) }, + ), + ).rejects.toMatchObject({ code: 'INTERNAL', exitCode: 1 }); + expect(calls).toBe(single ? 1 : 2); + expect(out).toEqual([]); + }); + + it('rejects a malformed cursor instead of returning it as a usable next page', async () => { + const out: string[] = []; + await expect( + runList( + { profile: 'default', output: 'json', debug: false, pageSize: 1 }, + { + ...makeCreds(), + fetchImpl: makeFetch(() => ({ body: { items: [PROJECT_FIXTURE], nextToken: 17 } })), + stdout: line => out.push(line), + }, + ), + ).rejects.toMatchObject({ code: 'INTERNAL', exitCode: 1 }); + expect(out).toEqual([]); + }); + + it('preserves new server fields and enum values without inventing absent optional fields', async () => { + const body = { + ...PROJECT_FIXTURE, + type: 'mobile', + createdFrom: 'import', + owner: { name: 'Team' }, + }; + const out: string[] = []; + const result = await runGet( + { profile: 'default', output: 'json', debug: false, projectId: PROJECT_FIXTURE.id }, + { ...makeCreds(), fetchImpl: makeFetch(() => ({ body })), stdout: line => out.push(line) }, + ); + expect(result).toEqual(body); + expect(JSON.parse(out[0]!)).toEqual(body); + expect(result).not.toHaveProperty('targetUrl'); + expect(result).not.toHaveProperty('testIdAttributes'); + expect(result).not.toHaveProperty('orgName'); + }); +}); + describe('runGet', () => { it('GETs /projects/{id} and prints the §6.1 fields in text mode', async () => { const { credentialsPath } = makeCreds(); diff --git a/src/commands/project.ts b/src/commands/project.ts index 0ecdda0..6e2b677 100644 --- a/src/commands/project.ts +++ b/src/commands/project.ts @@ -15,13 +15,14 @@ import type { FetchImpl, HttpClient } from '../lib/http.js'; import { globalShutdown, type ShutdownHandle } from '../lib/interrupt.js'; import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js'; import { readSecretFileGuarded } from '../lib/secret-file.js'; +import { CLI_PROJECT_LIST_SCHEMA, CLI_PROJECT_SCHEMA } from '../lib/project-response-schema.js'; import { assertNotLocal } from '../lib/target-url.js'; import { renderTextTable, resolveTextColumns, type TextTableColumn } from '../lib/text-table.js'; import { assertIdempotencyKey } from '../lib/validate.js'; import { - fetchSinglePage, paginate, validatePaginationFlags, + type FetchPageArgs, type Page, type PaginationFlags, } from '../lib/pagination.js'; @@ -155,22 +156,17 @@ export async function runList( // request — same shape AWS CLI ships. Otherwise auto-page. const useSinglePage = opts.pageSize !== undefined && opts.maxItems === undefined; + const fetchPage = async ({ pageSize, cursor }: FetchPageArgs) => + client.get>('/projects', { + query: { pageSize, cursor }, + schema: CLI_PROJECT_LIST_SCHEMA, + }); + let page: Page; if (useSinglePage) { - page = await fetchSinglePage( - client, - '/projects', - paginationFlags.pageSize!, - opts.startingToken, - ); + page = await fetchPage({ pageSize: paginationFlags.pageSize!, cursor: opts.startingToken }); } else { - page = await paginate( - async ({ pageSize, cursor }) => - client.get>('/projects', { - query: { pageSize, cursor }, - }), - paginationFlags, - ); + page = await paginate(fetchPage, paginationFlags); } out.print(page, data => { @@ -188,7 +184,9 @@ export async function runGet(opts: GetOptions, deps: ProjectDeps = {}): Promise< const out = makeOutput(opts.output, deps); const client = makeClient(opts, deps); - const project = await client.get(`/projects/${encodeURIComponent(opts.projectId)}`); + const project = await client.get(`/projects/${encodeURIComponent(opts.projectId)}`, { + schema: CLI_PROJECT_SCHEMA, + }); out.print(project, data => renderProjectText(data as CliProject)); return project; } diff --git a/src/commands/test.test.ts b/src/commands/test.test.ts index 55da144..ff44d4f 100644 --- a/src/commands/test.test.ts +++ b/src/commands/test.test.ts @@ -8,7 +8,7 @@ import { } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, expectTypeOf, it, vi } from 'vitest'; import type { Command } from 'commander'; import type { RunResponse } from '../lib/runs.types.js'; import { ApiError, InterruptError } from '../lib/errors.js'; @@ -1547,6 +1547,104 @@ describe('isPresignedCodeUrl', () => { }); describe('runCodeGet', () => { + it.each(['json', 'text'] as const)( + '%s mode rejects malformed test-code responses before writing output', + async output => { + const { credentialsPath } = makeCreds(); + const stdout: string[] = []; + const fetchImpl = makeFetch(() => ({ + body: { ...TEST_CODE_INLINE, code: { unexpected: 'do not emit this body' } }, + })); + await expect( + runCodeGet( + { profile: 'default', output, debug: false, testId: 'test_fe' }, + { + credentialsPath, + fetchImpl, + stdout: line => stdout.push(line), + rawStdout: chunk => { + stdout.push(chunk); + }, + }, + ), + ).rejects.toMatchObject({ code: 'INTERNAL', exitCode: 1 }); + expect(stdout).toEqual([]); + }, + ); + + it('rejects a missing code field without replacing an existing --out file', async () => { + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'cli-test-code-shape-')); + const target = join(dir, 'existing.json'); + writeFileSync(target, 'keep the original'); + const withoutCode: Record = { ...TEST_CODE_INLINE }; + delete withoutCode.code; + const fetchImpl = makeFetch(() => ({ body: withoutCode })); + await expect( + runCodeGet( + { profile: 'default', output: 'json', debug: false, testId: 'test_fe', out: target }, + { credentialsPath, fetchImpl }, + ), + ).rejects.toMatchObject({ code: 'INTERNAL', exitCode: 1 }); + expect(readFileSync(target, 'utf8')).toBe('keep the original'); + expect(readdirSync(dir)).toEqual(['existing.json']); + }); + + it('preserves additive fields and future code language/framework strings', async () => { + const { credentialsPath } = makeCreds(); + const body = { + ...TEST_CODE_INLINE, + language: 'ruby', + framework: 'rspec', + provenance: { source: 'future-server' }, + }; + const lines: string[] = []; + const result = await runCodeGet( + { profile: 'default', output: 'json', debug: false, testId: 'test_fe' }, + { credentialsPath, fetchImpl: makeFetch(() => ({ body })), stdout: line => lines.push(line) }, + ); + expectTypeOf(result.language).toEqualTypeOf(); + expectTypeOf(result.framework).toEqualTypeOf(); + expect(result).toEqual(body); + expect(JSON.parse(lines.join(''))).toEqual(body); + }); + + it('routes null code to the no-code response without dereferencing it', async () => { + const { credentialsPath } = makeCreds(); + const lines: string[] = []; + await runCodeGet( + { profile: 'default', output: 'text', debug: false, testId: 'test_fe' }, + { + credentialsPath, + fetchImpl: makeFetch(() => ({ body: { ...TEST_CODE_INLINE, code: null } })), + stderr: line => lines.push(line), + rawStdout: () => { + throw new Error('must not write an absent source body'); + }, + }, + ); + expect(lines).toContain('(no code generated yet — run the test first)'); + }); + + it('rejects an invalid auto-fetched codeVersion before sending code put', async () => { + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'cli-test-code-version-')); + const codeFile = join(dir, 'replacement.py'); + writeFileSync(codeFile, 'print("replacement")'); + const methods: string[] = []; + const fetchImpl = makeFetch((_url, init) => { + methods.push(init.method ?? 'GET'); + return { body: { ...TEST_CODE_INLINE, codeVersion: { unexpected: true } } }; + }); + await expect( + runCodePut( + { profile: 'default', output: 'json', debug: false, testId: 'test_fe', codeFile }, + { credentialsPath, fetchImpl, stderr: () => undefined }, + ), + ).rejects.toMatchObject({ code: 'INTERNAL', exitCode: 1 }); + expect(methods).toEqual(['GET']); + }); + it('JSON mode prints the §6.3 wire shape verbatim and skips the URL fetch', async () => { const { credentialsPath } = makeCreds(); const seen: string[] = []; diff --git a/src/commands/test.ts b/src/commands/test.ts index ec8954a..039323d 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -58,6 +58,7 @@ import { import { REQUEST_TIMEOUT_DEFAULT_MS, REQUEST_TIMEOUT_MAX_MS } from '../lib/http.js'; import type { FetchImpl } from '../lib/http.js'; import type { HttpClient } from '../lib/http.js'; +import { CLI_TEST_CODE_SCHEMA } from '../lib/response-schemas.js'; import { VERSION } from '../version.js'; import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js'; import { @@ -253,6 +254,13 @@ export interface CliTestCode { etag?: string | null; } +/** The standalone code endpoint also serves draft, legacy, and newer server values. */ +export type CliTestCodeRead = Omit & { + language: string; + framework?: string; + code: string | null; +}; + /** §6.4 TestStep wire shape. `null` is "not known", not "absent". */ export interface CliTestStep { testId: string; @@ -4607,7 +4615,10 @@ interface CodeGetOptions extends CommonOptions { * temp file is discarded and the user's pre-existing `--out` file, if * any, is left untouched. */ -export async function runCodeGet(opts: CodeGetOptions, deps: TestDeps = {}): Promise { +export async function runCodeGet( + opts: CodeGetOptions, + deps: TestDeps = {}, +): Promise { // Dry-run: no fetch, no fs. Print the canned shape to stdout and, if // the user passed `--out`, log on stderr what would have been written. // We deliberately do NOT validate the `--out` path here in dry-run — @@ -4617,15 +4628,23 @@ export async function runCodeGet(opts: CodeGetOptions, deps: TestDeps = {}): Pro const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); const out = makeOutput(opts.output, deps); const client = makeClient(opts, deps); - const code = await client.get(`/tests/${encodeURIComponent(opts.testId)}/code`); + const code = await client.get( + `/tests/${encodeURIComponent(opts.testId)}/code`, + { + schema: CLI_TEST_CODE_SCHEMA, + }, + ); if (opts.out !== undefined) { - const bytes = isPresignedCodeUrl(code.code) ? '' : `${code.code.length}`; + const bytes = + code.code !== null && isPresignedCodeUrl(code.code) + ? '' + : `${code.code?.length ?? 0}`; stderr(`[dry-run] would write code body (${bytes} bytes) to ${opts.out}`); } if (opts.output === 'json') { out.print(code); } else { - await out.writeChunk(code.code); + await out.writeChunk(code.code ?? ''); } return code; } @@ -4635,13 +4654,18 @@ export async function runCodeGet(opts: CodeGetOptions, deps: TestDeps = {}): Pro const client = makeClient(opts, deps); try { - const code = await client.get(`/tests/${encodeURIComponent(opts.testId)}/code`); + const code = await client.get( + `/tests/${encodeURIComponent(opts.testId)}/code`, + { + schema: CLI_TEST_CODE_SCHEMA, + }, + ); let wroteContent = false; if (opts.output === 'json') { out.print(code); wroteContent = true; - } else if (isPresignedCodeUrl(code.code)) { + } else if (code.code !== null && isPresignedCodeUrl(code.code)) { // Text mode: dump the source body. JSON consumers want the wire // shape; humans (and agents shelling out via `> file.ts`) want // ready-to-edit code. Stream chunk-wise so a multi-MB generated @@ -4817,7 +4841,12 @@ export async function runCodePut( requireNonEmpty('expected-version', opts.expectedVersion); ifMatch = opts.expectedVersion; } else { - const fetched = await client.get(`/tests/${encodeURIComponent(opts.testId)}/code`); + const fetched = await client.get( + `/tests/${encodeURIComponent(opts.testId)}/code`, + { + schema: CLI_TEST_CODE_SCHEMA, + }, + ); const cv = fetched.codeVersion; if (cv === null || cv === undefined) { // Server hasn't stamped a codeVersion yet (legacy row). Send `*` diff --git a/src/lib/project-response-schema.ts b/src/lib/project-response-schema.ts new file mode 100644 index 0000000..266579c --- /dev/null +++ b/src/lib/project-response-schema.ts @@ -0,0 +1,27 @@ +import * as v from 'valibot'; +import type { CliProject } from '../commands/project.js'; +import type { Page } from './pagination.js'; + +/** + * Project read fixtures in commands/project.test.ts and test/mock-backend/fixtures.ts + * establish these core fields. Optional fields must stay absent when omitted: + * in particular, an absent targetUrl means no answer, whereas null means unset. + * Follow response-schemas.ts: preserve added fields and accept future enum strings. + */ +export const CLI_PROJECT_SCHEMA: v.GenericSchema = v.looseObject({ + id: v.string(), + name: v.string(), + type: v.custom(value => typeof value === 'string'), + createdFrom: v.custom(value => typeof value === 'string'), + createdAt: v.string(), + updatedAt: v.string(), + orgId: v.optional(v.string()), + orgName: v.optional(v.string()), + targetUrl: v.optional(v.nullable(v.string())), + testIdAttributes: v.optional(v.nullable(v.array(v.string()))), +}); + +export const CLI_PROJECT_LIST_SCHEMA: v.GenericSchema> = v.looseObject({ + items: v.array(CLI_PROJECT_SCHEMA), + nextToken: v.nullable(v.string()), +}); diff --git a/src/lib/response-schemas.code.test.ts b/src/lib/response-schemas.code.test.ts new file mode 100644 index 0000000..545c120 --- /dev/null +++ b/src/lib/response-schemas.code.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest'; +import * as v from 'valibot'; +import { testCodeFixture, testCodeLargeFixture } from '../../test/mock-backend/fixtures.js'; +import { CLI_TEST_CODE_SCHEMA } from './response-schemas.js'; + +describe('CLI_TEST_CODE_SCHEMA', () => { + it.each([testCodeFixture, testCodeLargeFixture])('preserves a complete source fixture', body => { + expect(v.parse(CLI_TEST_CODE_SCHEMA, body)).toEqual(body); + }); + + it('accepts an omitted framework on the code-put legacy auto-fetch shape', () => { + const body = { testId: 'test_alpha', language: 'typescript', code: 'old', codeVersion: null }; + expect(v.parse(CLI_TEST_CODE_SCHEMA, body)).toEqual(body); + }); + + it('normalizes an absent legacy codeVersion to null without inventing an etag', () => { + const body = { testId: 'test_alpha', language: 'python', code: '' }; + const parsed = v.parse(CLI_TEST_CODE_SCHEMA, body); + expect(parsed.codeVersion).toBeNull(); + expect(parsed).not.toHaveProperty('etag'); + expect(parsed).not.toHaveProperty('framework'); + }); + + it('preserves explicit null etag and code from a draft row', () => { + const body = { ...testCodeFixture, code: null, etag: null }; + expect(v.parse(CLI_TEST_CODE_SCHEMA, body)).toEqual(body); + }); + + it.each([ + null, + [], + { ...testCodeFixture, testId: 7 }, + { ...testCodeFixture, language: false }, + { ...testCodeFixture, framework: [] }, + { ...testCodeFixture, code: undefined }, + { ...testCodeFixture, codeVersion: 7 }, + { ...testCodeFixture, etag: {} }, + ])('rejects an incompatible response shape', body => { + expect(v.safeParse(CLI_TEST_CODE_SCHEMA, body).success).toBe(false); + }); +}); diff --git a/src/lib/response-schemas.ts b/src/lib/response-schemas.ts index 35568cf..5910a84 100644 --- a/src/lib/response-schemas.ts +++ b/src/lib/response-schemas.ts @@ -1,13 +1,12 @@ /** - * Valibot schemas for the run-path wire shapes (issue #102). + * Valibot schemas for the run-path and test-code wire shapes (#102, #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 and selected command reads. Generic requests remain + * schema-free unless the caller supplies a schema. * * Resilience rules (additive server changes must never hard-fail the CLI): * @@ -47,6 +46,7 @@ import type { import type { CliTestListRunResponse } from './testlist.types.js'; import type { TunnelMintResponse, TunnelStatusResponse } from './tunnel.types.js'; import type { ConflictReason } from './conflict-reason.js'; +import type { CliTestCodeRead } from '../commands/test.js'; /** * Compile-time literal union, runtime open string. @@ -60,6 +60,23 @@ function openWireLiteral(): v.GenericSchema(value => typeof value === 'string'); } +/** + * GET /tests/{id}/code. The inline and presigned fixtures in + * test/mock-backend/fixtures.ts contain the required identity/source fields. + * The code-put auto-fetch fixtures in commands/test.test.ts omit framework; + * legacy codeVersion may be null/absent and already uses the explicit + * If-Match fallback. Keep etag absence distinct from an explicit null. + * A null code body is the draft/no-generated-code branch of runCodeGet. + */ +export const CLI_TEST_CODE_SCHEMA: v.GenericSchema = v.looseObject({ + testId: v.string(), + language: v.string(), + framework: v.optional(v.string()), + code: v.nullable(v.string()), + codeVersion: v.nullish(v.string(), null), + etag: v.optional(v.nullable(v.string())), +}); + // --------------------------------------------------------------------------- // GET /runs/{runId} // --------------------------------------------------------------------------- diff --git a/test/contract/p4-schema.test.ts b/test/contract/p4-schema.test.ts index cedea9c..dd6a536 100644 --- a/test/contract/p4-schema.test.ts +++ b/test/contract/p4-schema.test.ts @@ -301,7 +301,7 @@ describe('P4 schema contract — CLI runners return §6.x shapes', () => { { credentialsPath, stdout: () => undefined }, ); validateTestCode(code); - expect(code.code.startsWith('https://')).toBe(false); + expect(code.code?.startsWith('https://')).toBe(false); }); it('runCodeGet (presigned) returns a §6.3 TestCode with https code', async () => { @@ -311,7 +311,7 @@ describe('P4 schema contract — CLI runners return §6.x shapes', () => { { credentialsPath, stdout: () => undefined }, ); validateTestCode(code); - expect(code.code.startsWith('https://')).toBe(true); + expect(code.code?.startsWith('https://')).toBe(true); }); it('runSteps returns a §6.4 TestStepList', async () => {