From c535b9f31de4cf43852ac6ec71e0618d622ff1bb Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Wed, 23 Sep 2026 00:20:31 -0500 Subject: [PATCH 1/5] =?UTF-8?q?=E2=9C=A8=20Discover=20and=20call=20the=20p?= =?UTF-8?q?ublic=20review=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expose compact schema discovery and generic JSON bodies and file downloads so agents can complete reviews using server contracts. Preserve JSON output and dedicated review retry behavior. --- src/api/client.js | 21 +- src/cli.js | 48 ++- src/commands/api.js | 116 +++++--- tests/commands/api.test.js | 582 +++++++++++++------------------------ 4 files changed, 334 insertions(+), 433 deletions(-) diff --git a/src/api/client.js b/src/api/client.js index d79a37be..fcab787d 100644 --- a/src/api/client.js +++ b/src/api/client.js @@ -76,6 +76,11 @@ export function createApiClient(options = {}) { * @returns {Promise} Parsed JSON response */ async function request(endpoint, fetchOptions = {}, isRetry = false) { + let { + responseType = 'json', + retryAuthentication = true, + ...httpOptions + } = fetchOptions; let url = buildApiUrl(baseUrl, endpoint); let headers = buildRequestHeaders({ @@ -88,7 +93,7 @@ export function createApiClient(options = {}) { let response; try { response = await fetch(url, { - ...fetchOptions, + ...httpOptions, headers, }); } catch (error) { @@ -106,6 +111,7 @@ export function createApiClient(options = {}) { // Handle 401 with token refresh if ( + retryAuthentication && shouldRetryWithRefresh( response.status, isRetry, @@ -134,6 +140,18 @@ export function createApiClient(options = {}) { }); } + if (responseType === 'response') return response; + if (response.status === 204 || httpOptions.method === 'HEAD') return null; + let contentType = response.headers?.get?.('content-type'); + if ( + contentType && + !/application\/(?:[\w.+-]+\+)?json\b/i.test(contentType) + ) { + throw new VizzlyError( + 'This response contains file data. Use api --output to download it.', + 'BINARY_RESPONSE' + ); + } return response.json(); } @@ -157,6 +175,7 @@ export function createApiClient(options = {}) { let refreshUrl = buildApiUrl(baseUrl, '/api/auth/cli/refresh'); let response = await fetch(refreshUrl, { method: 'POST', + redirect: 'error', headers: { 'Content-Type': 'application/json', 'User-Agent': userAgent, diff --git a/src/cli.js b/src/cli.js index b706e2c4..6f44fb05 100644 --- a/src/cli.js +++ b/src/cli.js @@ -2,7 +2,11 @@ import 'dotenv/config'; import { existsSync, statSync } from 'node:fs'; import { Option, program } from 'commander'; -import { apiCommand, validateApiOptions } from './commands/api.js'; +import { + apiCommand, + apiSchemaCommand, + validateApiOptions, +} from './commands/api.js'; import { baselinesCommand } from './commands/baselines.js'; import { buildsCommand, validateBuildsOptions } from './commands/builds.js'; import { @@ -1170,16 +1174,17 @@ Note: Baselines are stored locally in .vizzly/baselines/ during TDD mode. await baselinesCommand(options, globalOptions); }); -program +let api = program .command('api') .description('Make raw API requests (for power users)') .argument('', 'API endpoint (e.g., /api/sdk/builds)') .option( '-X, --method ', - 'HTTP method (GET or POST for build comments)', + 'HTTP method; discover supported requests with api schema', 'GET' ) - .option('-d, --data ', 'Request body (JSON)') + .option('-d, --data ', 'JSON body, @file, or @- for stdin') + .option('-o, --output ', 'Write response bytes to a new file') .option( '-H, --header
', 'Add header (key:value), can be repeated', @@ -1200,8 +1205,10 @@ Examples: $ vizzly api /api/sdk/builds/abc123/comments -X POST -d '{"content":"Looks good"}' $ vizzly api /api/sdk/builds/abc123/comments -X POST -d '{"content":"Nice!"}' -Note: POST is restricted to build comment endpoints. Use dedicated approve/reject commands for review decisions. -Most operations have dedicated commands (builds, comparisons, approve, etc.). +Discover operations: vizzly api schema, then vizzly api schema . +Use the method, version header, parameters and body described there. +JSON output places the API payload under data.response. Image downloads use --output. +Writes are never automatically retried after authentication failures. ` ) .action(async (endpoint, options) => { @@ -1216,6 +1223,29 @@ Most operations have dedicated commands (builds, comparisons, approve, etc.). await apiCommand(endpoint, options, globalOptions); }); +api + .command('schema [operation-id]') + .description( + 'Discover supported API operations and their request/response schemas' + ) + .option('--full', 'Download full OpenAPI (requires --output)') + .option( + '-q, --query ', + 'Schema view=request, response, or full', + (value, previous) => [...(previous || []), value] + ) + .option('-o, --output ', 'Write the schema to a new file') + .action(async (operationId, options) => { + options = { ...api.opts(), ...options }; + if (options.full && !options.output) { + reportValidationErrors([ + '--full requires --output to avoid dumping the entire schema.', + ]); + return; + } + await apiSchemaCommand(operationId, options, getGlobalOptions()); + }); + program .command('approve') .description('Approve a comparison') @@ -1510,7 +1540,11 @@ let commandNames = new Set(program.commands.map(command => command.name())); let nestedCommandNames = new Map( program.commands.map(command => [ command.name(), - new Set(command.commands.map(subcommand => subcommand.name())), + new Set( + command.registeredArguments.length + ? [] + : command.commands.map(subcommand => subcommand.name()) + ), ]) ); let normalizedArgv = normalizeJsonArgv(process.argv, commandNames); diff --git a/src/commands/api.js b/src/commands/api.js index 4b3ed295..5a526377 100644 --- a/src/commands/api.js +++ b/src/commands/api.js @@ -2,11 +2,16 @@ * API command - raw API access for power users */ +import { createWriteStream } from 'node:fs'; +import { readFile, unlink } from 'node:fs/promises'; +import { resolve } from 'node:path'; +import { Readable } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; import { createApiClient as defaultCreateApiClient } from '../api/index.js'; import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import * as defaultOutput from '../utils/output.js'; -let ALLOWED_POST_ENDPOINTS = [/^\/api\/sdk\/builds\/[^/]+\/comments$/]; +let API_METHODS = ['GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE']; function createApiCommandDeps(deps = {}) { return { @@ -39,10 +44,6 @@ export function normalizeApiMethod(method = 'GET') { return method.toUpperCase(); } -export function isAllowedPostEndpoint(endpoint) { - return ALLOWED_POST_ENDPOINTS.some(pattern => pattern.test(endpoint)); -} - export function parseApiHeaders(headerOption) { let headers = {}; let headerList = Array.isArray(headerOption) ? headerOption : [headerOption]; @@ -82,24 +83,15 @@ export function appendApiQuery(endpoint, queryOption) { export function validateApiRequest({ endpoint, method, hasData = false }) { let errors = []; - - if (method !== 'GET' && method !== 'POST') { - errors.push( - `Method ${method} not allowed. Use GET for queries or POST for build comments.` - ); - return errors; + if (!API_METHODS.includes(method)) { + errors.push(`Unsupported HTTP method: ${method}`); } - - if (method === 'POST' && !isAllowedPostEndpoint(endpoint)) { - errors.push( - `POST not allowed for ${endpoint}. Only build comment endpoints support POST.` - ); + if (hasData && ['GET', 'HEAD'].includes(method)) { + errors.push('Request data requires a method other than GET or HEAD.'); } - - if (hasData && method !== 'POST') { - errors.push('Request data requires --method POST.'); + if (/^[a-z][a-z\d+.-]*:/i.test(endpoint) || endpoint.startsWith('//')) { + errors.push('Use an API path on the configured Vizzly server.'); } - return errors; } @@ -107,7 +99,7 @@ export function buildApiRequest({ endpoint, options = {} }) { let normalizedEndpoint = normalizeApiEndpoint(endpoint); let method = normalizeApiMethod(options.method || 'GET'); let errors = validateApiRequest({ - endpoint: normalizedEndpoint, + endpoint, method, hasData: options.data !== undefined, }); @@ -119,7 +111,7 @@ export function buildApiRequest({ endpoint, options = {} }) { let headers = parseApiHeaders(options.header); let requestOptions = { method }; - if (options.data && method === 'POST') { + if (options.data !== undefined && !['GET', 'HEAD'].includes(method)) { headers['Content-Type'] = headers['Content-Type'] || 'application/json'; requestOptions.body = options.data; } @@ -161,16 +153,28 @@ export async function apiCommand( let allOptions = { ...globalOptions, ...options }; let config = await loadConfig(globalOptions.config, allOptions); - // Validate API token - if (!config.apiKey) { + // A linked project's upload credential must not hide an existing user login. + // Explicit file/env/--token credentials still take precedence. + let token = config.linkedProject + ? config.userToken || config.apiKey + : config.apiKey || config.userToken; + if (!token && !options.schemaDiscovery) { output.error( - 'API token required. Use --token or set VIZZLY_TOKEN environment variable' + 'Authentication required. Run vizzly login or set VIZZLY_TOKEN.' ); output.cleanup(); exit(1); return; } + if (options.data?.startsWith('@')) { + let source = options.data.slice(1); + let data = + source === '-' ? await readStdin() : await readFile(source, 'utf8'); + options = { ...options, data }; + } + if (options.data !== undefined) JSON.parse(options.data); + let { errors, method, normalizedEndpoint, requestOptions } = buildApiRequest({ endpoint, options }); @@ -179,14 +183,7 @@ export async function apiCommand( if (errors.length > 0) { output.error(errors[0]); - if (method === 'POST') { - output.hint( - 'Use GET for queries, or use dedicated commands (vizzly approve, vizzly reject, vizzly comment)' - ); - } - output.hint( - 'Most raw API use should stay read-only; prefer dedicated commands for mutations.' - ); + output.hint('Use vizzly api schema to discover supported operations.'); output.cleanup(); exit(1); return; @@ -197,11 +194,35 @@ export async function apiCommand( let client = createApiClient({ baseUrl: config.apiUrl, - token: config.apiKey, + token, command: 'api', + allowNoToken: Boolean(options.schemaDiscovery), }); - let response = await client.request(normalizedEndpoint, requestOptions); + let response = await client.request(normalizedEndpoint, { + ...requestOptions, + redirect: 'error', + retryAuthentication: ['GET', 'HEAD'].includes(method), + responseType: options.output ? 'response' : 'json', + }); + if (options.output) { + let path = resolve(options.output); + let file = createWriteStream(path, { flags: 'wx' }); + let created = false; + file.once('open', () => { + created = true; + }); + try { + await pipeline(response.body || Readable.from([]), file); + } catch (error) { + if (created) await unlink(path); + throw error; + } + response = { + file: path, + contentType: response.headers.get('content-type'), + }; + } output.stopSpinner(); // Output response @@ -267,11 +288,10 @@ export function validateApiOptions(endpoint, options = {}) { return errors; } - let normalizedEndpoint = normalizeApiEndpoint(endpoint); let method = normalizeApiMethod(options.method || 'GET'); errors.push( ...validateApiRequest({ - endpoint: normalizedEndpoint, + endpoint, method, hasData: options.data !== undefined, }) @@ -279,3 +299,25 @@ export function validateApiOptions(endpoint, options = {}) { return errors; } + +async function readStdin() { + let chunks = []; + for await (let chunk of process.stdin) chunks.push(chunk); + return Buffer.concat(chunks.map(chunk => Buffer.from(chunk))).toString( + 'utf8' + ); +} + +export async function apiSchemaCommand(operationId, options, globalOptions) { + let endpoint = '/api/sdk/schema'; + if (operationId) { + endpoint += `/${encodeURIComponent(operationId)}`; + if (options.full) + options = { ...options, query: [...(options.query || []), 'view=full'] }; + } else if (options.full) endpoint += '/openapi'; + return apiCommand( + endpoint, + { ...options, schemaDiscovery: true }, + globalOptions + ); +} diff --git a/tests/commands/api.test.js b/tests/commands/api.test.js index 5a0c55ca..475f815f 100644 --- a/tests/commands/api.test.js +++ b/tests/commands/api.test.js @@ -1,405 +1,211 @@ import assert from 'node:assert/strict'; -import { describe, it } from 'node:test'; +import { spawn } from 'node:child_process'; +import { once } from 'node:events'; +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { createServer } from 'node:http'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { after, before, test } from 'node:test'; import { - apiCommand, appendApiQuery, buildApiRequest, - isAllowedPostEndpoint, normalizeApiEndpoint, - normalizeApiMethod, parseApiHeaders, validateApiOptions, - validateApiRequest, } from '../../src/commands/api.js'; -function createMockOutput() { - let calls = []; - return { - calls, - configure: opts => calls.push({ method: 'configure', args: [opts] }), - data: value => calls.push({ method: 'data', args: [value] }), - error: (message, error) => - calls.push({ method: 'error', args: [message, error] }), - hint: message => calls.push({ method: 'hint', args: [message] }), - header: command => calls.push({ method: 'header', args: [command] }), - labelValue: (label, value) => - calls.push({ method: 'labelValue', args: [label, value] }), - blank: () => calls.push({ method: 'blank', args: [] }), - print: message => calls.push({ method: 'print', args: [message] }), - startSpinner: message => - calls.push({ method: 'startSpinner', args: [message] }), - stopSpinner: () => calls.push({ method: 'stopSpinner', args: [] }), - cleanup: () => calls.push({ method: 'cleanup', args: [] }), - }; -} - -function createApiHarness(response = { ok: true }) { - let output = createMockOutput(); - let clientConfig = null; - let request = null; - let exitCode = null; - - return { - output, - get clientConfig() { - return clientConfig; - }, - get request() { - return request; - }, - get exitCode() { - return exitCode; - }, - deps: { - loadConfig: async () => ({ - apiKey: 'token-123', - apiUrl: 'https://api.example.test', - }), - createApiClient: config => { - clientConfig = config; - return { - request: async (endpoint, options) => { - request = { endpoint, options }; - return response; - }, - }; - }, - output, - exit: code => { - exitCode = code; +let directory, server, origin; +let requests = []; +before(async () => { + directory = await mkdtemp(join(tmpdir(), 'vizzly-api-command-')); + server = createServer(async (req, res) => { + let chunks = []; + for await (let chunk of req) chunks.push(chunk); + let body = Buffer.concat(chunks).toString(); + requests.push({ + method: req.method, + url: req.url, + headers: req.headers, + body, + }); + if (req.url === '/api/image') { + res.setHeader('Content-Type', 'image/png'); + res.end(Buffer.from([137, 80, 78, 71, 13, 10, 26, 10])); + } else if (req.url === '/api/redirect') { + res.writeHead(302, { Location: `${origin}/api/secret` }); + res.end(); + } else if (req.url === '/api/unauthorized') { + res.writeHead(401, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'denied' })); + } else if (req.method === 'DELETE') { + res.writeHead(204); + res.end(); + } else { + res.setHeader('Content-Type', 'application/json'); + res.end( + JSON.stringify({ + items: ['one'], + received: body ? JSON.parse(body) : null, + }) + ); + } + }); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + origin = `http://127.0.0.1:${server.address().port}`; +}); +after(async () => { + await new Promise((resolve, reject) => + server.close(error => (error ? reject(error) : resolve())) + ); + await rm(directory, { recursive: true, force: true }); +}); +async function cli(args, input = '', environment = {}) { + let child = spawn( + process.execPath, + [resolve('src/cli.js'), ...args, '--json'], + { + cwd: directory, + env: { + ...process.env, + VIZZLY_HOME: join(directory, 'home'), + VIZZLY_TOKEN: 'vzt_test', + VIZZLY_API_URL: origin, + NO_COLOR: '1', + ...environment, }, - }, + stdio: ['pipe', 'pipe', 'pipe'], + } + ); + let stdout = '', + stderr = ''; + child.stdout.on('data', chunk => { + stdout += chunk; + }); + child.stderr.on('data', chunk => { + stderr += chunk; + }); + child.stdin.end(input); + let [code] = await once(child, 'close'); + return { + code, + stdout, + stderr, + json: stdout.trim() ? JSON.parse(stdout) : null, }; } - -describe('commands/api', () => { - describe('request helpers', () => { - it('normalizes endpoint and method inputs', () => { - assert.strictEqual(normalizeApiEndpoint('sdk/builds'), '/api/sdk/builds'); - assert.strictEqual( - normalizeApiEndpoint('/api/sdk/builds'), - '/api/sdk/builds' - ); - assert.strictEqual(normalizeApiMethod('post'), 'POST'); - assert.strictEqual(normalizeApiMethod(), 'GET'); - }); - - it('parses headers and query parameters without losing separators', () => { - assert.deepStrictEqual( - parseApiHeaders(['X-Test: alpha:beta', 'Accept: application/json']), - { - 'X-Test': 'alpha:beta', - Accept: 'application/json', - } - ); - assert.strictEqual( - appendApiQuery('/api/sdk/builds?existing=1', [ - 'branch=feature/a=b', - 'limit=5', - ]), - '/api/sdk/builds?existing=1&branch=feature%2Fa%3Db&limit=5' - ); - }); - - it('allows only selected POST endpoints', () => { - assert.strictEqual( - isAllowedPostEndpoint('/api/sdk/comparisons/cmp-1/approve'), - false - ); - assert.strictEqual( - isAllowedPostEndpoint('/api/sdk/comparisons/cmp-1/reject'), - false - ); - assert.strictEqual( - isAllowedPostEndpoint('/api/sdk/builds/build-1/comments'), - true - ); - assert.strictEqual(isAllowedPostEndpoint('/api/sdk/builds'), false); - }); - - it('builds GET and POST request options', () => { - assert.deepStrictEqual( - buildApiRequest({ - endpoint: 'sdk/builds', - options: { - query: ['limit=5'], - header: 'X-Test: yes', - }, - }), - { - errors: [], - method: 'GET', - normalizedEndpoint: '/api/sdk/builds?limit=5', - requestOptions: { - method: 'GET', - headers: { 'X-Test': 'yes' }, - }, - } - ); - - assert.deepStrictEqual( - buildApiRequest({ - endpoint: '/api/sdk/builds/build-1/comments', - options: { method: 'POST', data: '{"content":"LGTM"}' }, - }), - { - errors: [], - method: 'POST', - normalizedEndpoint: '/api/sdk/builds/build-1/comments', - requestOptions: { - method: 'POST', - body: '{"content":"LGTM"}', - headers: { 'Content-Type': 'application/json' }, - }, - } - ); - }); - - it('reports unsafe API requests', () => { - assert.deepStrictEqual( - validateApiRequest({ - endpoint: '/api/sdk/builds', - method: 'POST', - }), - [ - 'POST not allowed for /api/sdk/builds. Only build comment endpoints support POST.', - ] - ); - assert.deepStrictEqual( - validateApiRequest({ - endpoint: '/api/sdk/builds', - method: 'DELETE', - }), - [ - 'Method DELETE not allowed. Use GET for queries or POST for build comments.', - ] - ); - assert.deepStrictEqual( - validateApiRequest({ - endpoint: '/api/sdk/builds', - method: 'GET', - hasData: true, - }), - ['Request data requires --method POST.'] - ); - }); +test('generic request helpers retain parameter values and reject external origins', () => { + assert.equal(normalizeApiEndpoint('sdk/builds'), '/api/sdk/builds'); + assert.equal( + appendApiQuery('/api/builds', ['name=a=b']), + '/api/builds?name=a%3Db' + ); + assert.deepEqual(parseApiHeaders('X-Trace: a:b'), { 'X-Trace': 'a:b' }); + assert.equal( + buildApiRequest({ + endpoint: '/api/review', + options: { method: 'POST', data: '{}' }, + }).requestOptions.body, + '{}' + ); + assert.equal(validateApiOptions('/api/test', { method: 'PATCH' }).length, 0); + assert.equal(validateApiOptions('/api/test', { data: '{}' }).length, 1); + assert.equal(validateApiOptions('https://elsewhere.test/api').length, 1); +}); +test('actual CLI discovers schemas and preserves the JSON payload envelope', async () => { + let result = await cli(['api', 'schema']); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(requests.at(-1).url, '/api/sdk/schema'); + assert.deepEqual(result.json.data.response.items, ['one']); + result = await cli(['api', 'schema', 'sdk.listBuilds']); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(requests.at(-1).url, '/api/sdk/schema/sdk.listBuilds'); + result = await cli([ + 'api', + 'schema', + 'sdk.listBuilds', + '-q', + 'view=response', + ]); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal( + requests.at(-1).url, + '/api/sdk/schema/sdk.listBuilds?view=response' + ); +}); +test('CLI sends decisions using file or stdin JSON and arbitrary documented headers', async () => { + let file = join(directory, 'decision.json'); + await writeFile(file, '{"decision":"approved"}'); + let result = await cli([ + 'api', + '/api/review', + '-X', + 'POST', + '-d', + `@${file}`, + '-H', + 'X-Organization: team', + ]); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(requests.at(-1).headers['x-organization'], 'team'); + assert.equal(requests.at(-1).headers.authorization, 'Bearer vzt_test'); + assert.deepEqual(result.json.data.response.received, { + decision: 'approved', }); + result = await cli( + ['api', '/api/review', '-X', 'POST', '-d', '@-'], + '{"decision":"rejected"}' + ); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(result.json.data.response.received.decision, 'rejected'); +}); +test('image output writes real bytes without overwriting an existing file', async () => { + let path = join(directory, 'image.png'); + let result = await cli(['api', '/api/image', '--output', path]); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(result.json.data.response.file, path); + assert.deepEqual( + await readFile(path), + Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]) + ); + result = await cli(['api', '/api/image', '--output', path]); + assert.equal(result.code, 1); + assert.equal((await readFile(path)).length, 8); +}); +test('empty responses succeed; malformed bodies and HTTP errors fail without replay', async () => { + let result = await cli(['api', '/api/review', '-X', 'DELETE']); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(result.json.data.response, null); + let before = requests.length; + result = await cli(['api', '/api/review', '-X', 'POST', '-d', '{invalid']); + assert.equal(result.code, 1); + assert.equal(requests.length, before); + result = await cli(['api', '/api/unauthorized', '-X', 'POST', '-d', '{}']); + assert.equal(result.code, 1); + assert.equal(requests.length, before + 1); +}); +test('CLI refuses redirects and full-schema console dumps', async () => { + let before = requests.length; + let result = await cli(['api', '/api/redirect']); + assert.equal(result.code, 1); + assert.equal(requests.length, before + 1); + result = await cli(['api', 'schema', '--full']); + assert.equal(result.code, 1); + assert.equal(requests.length, before + 1); +}); - describe('validateApiOptions', () => { - it('validates endpoint and method options', () => { - assert.deepStrictEqual(validateApiOptions('/api/sdk/builds'), []); - assert.deepStrictEqual(validateApiOptions(''), ['Endpoint is required']); - assert.deepStrictEqual(validateApiOptions(' '), [ - 'Endpoint is required', - ]); - assert.deepStrictEqual( - validateApiOptions('/api/sdk/builds', { method: 'POST' }), - [ - 'POST not allowed for /api/sdk/builds. Only build comment endpoints support POST.', - ] - ); - assert.deepStrictEqual( - validateApiOptions('/api/sdk/builds', { method: 'PATCH' }), - [ - 'Method PATCH not allowed. Use GET for queries or POST for build comments.', - ] - ); - assert.deepStrictEqual( - validateApiOptions('/api/sdk/builds', { data: '{"ignored":true}' }), - ['Request data requires --method POST.'] - ); - }); +test('schema discovery works before login but ordinary data reads require credentials', async () => { + let result = await cli(['api', 'schema'], '', { + VIZZLY_TOKEN: '', + VIZZLY_HOME: join(directory, 'anonymous'), }); - - describe('apiCommand', () => { - it('performs a GET request and returns JSON output', async () => { - let harness = createApiHarness({ builds: [] }); - - await apiCommand( - 'sdk/builds', - { query: ['limit=5'] }, - { json: true }, - harness.deps - ); - - assert.deepStrictEqual(harness.clientConfig, { - baseUrl: 'https://api.example.test', - token: 'token-123', - command: 'api', - }); - assert.deepStrictEqual(harness.request, { - endpoint: '/api/sdk/builds?limit=5', - options: { method: 'GET' }, - }); - - let dataCall = harness.output.calls.find(call => call.method === 'data'); - assert.deepStrictEqual(dataCall.args[0], { - endpoint: '/api/sdk/builds?limit=5', - method: 'GET', - response: { builds: [] }, - }); - }); - - it('performs an allowed POST request with headers and body', async () => { - let harness = createApiHarness({ comment: { id: 'comment-1' } }); - - await apiCommand( - '/api/sdk/builds/build-1/comments', - { - method: 'POST', - data: '{"content":"LGTM"}', - header: 'X-Trace: trace-1', - }, - {}, - harness.deps - ); - - assert.deepStrictEqual(harness.request, { - endpoint: '/api/sdk/builds/build-1/comments', - options: { - method: 'POST', - body: '{"content":"LGTM"}', - headers: { - 'X-Trace': 'trace-1', - 'Content-Type': 'application/json', - }, - }, - }); - assert.ok( - harness.output.calls.some( - call => call.method === 'labelValue' && call.args[0] === 'Endpoint' - ) - ); - }); - - it('cleans up and exits when no API token is configured', async () => { - let output = createMockOutput(); - let exitCode = null; - - await apiCommand( - '/api/sdk/builds', - {}, - {}, - { - loadConfig: async () => ({ apiUrl: 'https://api.example.test' }), - output, - exit: code => { - exitCode = code; - }, - } - ); - - assert.strictEqual(exitCode, 1); - assert.ok(output.calls.some(call => call.method === 'error')); - assert.ok(output.calls.some(call => call.method === 'cleanup')); - }); - - it('blocks unsafe POST requests before creating a client', async () => { - let output = createMockOutput(); - let exitCode = null; - let createdClient = false; - - await apiCommand( - '/api/sdk/builds', - { method: 'POST', data: '{}' }, - {}, - { - loadConfig: async () => ({ - apiKey: 'token-123', - apiUrl: 'https://api.example.test', - }), - createApiClient: () => { - createdClient = true; - return {}; - }, - output, - exit: code => { - exitCode = code; - }, - } - ); - - assert.strictEqual(exitCode, 1); - assert.strictEqual(createdClient, false); - assert.ok(output.calls.some(call => call.method === 'error')); - assert.ok(output.calls.some(call => call.method === 'cleanup')); - }); - - it('blocks request data on GET before creating a client', async () => { - let output = createMockOutput(); - let exitCode = null; - let createdClient = false; - - await apiCommand( - '/api/sdk/builds', - { data: '{"silently":"dropped"}' }, - {}, - { - loadConfig: async () => ({ - apiKey: 'token-123', - apiUrl: 'https://api.example.test', - }), - createApiClient: () => { - createdClient = true; - return {}; - }, - output, - exit: code => { - exitCode = code; - }, - } - ); - - assert.strictEqual(exitCode, 1); - assert.strictEqual(createdClient, false); - assert.ok( - output.calls.some( - call => - call.method === 'error' && - call.args[0] === 'Request data requires --method POST.' - ) - ); - }); - - it('returns JSON failure details using normalized endpoint and method', async () => { - let output = createMockOutput(); - let exitCode = null; - let error = new Error('network failed'); - error.code = 'network_error'; - error.context = { status: 503 }; - - await apiCommand( - 'sdk/builds', - {}, - { json: true }, - { - loadConfig: async () => ({ - apiKey: 'token-123', - apiUrl: 'https://api.example.test', - }), - createApiClient: () => ({ - request: async () => { - throw error; - }, - }), - output, - exit: code => { - exitCode = code; - }, - } - ); - - assert.strictEqual(exitCode, 1); - let dataCall = output.calls.find(call => call.method === 'data'); - assert.deepStrictEqual(dataCall.args[0], { - endpoint: '/api/sdk/builds', - method: 'GET', - error: { - message: 'network failed', - code: 'network_error', - status: 503, - }, - }); - }); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(requests.at(-1).headers.authorization, undefined); + let before = requests.length; + result = await cli(['api', '/api/review'], '', { + VIZZLY_TOKEN: '', + VIZZLY_HOME: join(directory, 'anonymous'), }); + assert.equal(result.code, 1); + assert.equal(requests.length, before); }); From fa431649bcf218fb2183f41194be5e105361bc4c Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Wed, 23 Sep 2026 00:33:23 -0500 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=93=9D=20Show=20field=20discovery=20i?= =?UTF-8?q?n=20schema=20help?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/cli.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli.js b/src/cli.js index 6f44fb05..857e84a4 100644 --- a/src/cli.js +++ b/src/cli.js @@ -1231,7 +1231,7 @@ api .option('--full', 'Download full OpenAPI (requires --output)') .option( '-q, --query ', - 'Schema view=request, response, or full', + 'Schema view=request, fields, response, or full', (value, previous) => [...(previous || []), value] ) .option('-o, --output ', 'Write the schema to a new file') From e5cda70ed6c759fdcdf5b5b0fc99157f048b65fe Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Wed, 23 Sep 2026 06:45:52 -0500 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=90=9B=20Clean=20up=20interrupted=20A?= =?UTF-8?q?PI=20downloads=20reliably?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Open the exclusive destination before transferring bytes so early stream failures cannot leave an empty file behind. Exercise credential precedence, refresh/replay behavior, binary errors and cleanup through real CLI subprocesses and HTTP. --- src/commands/api.js | 17 +++--- tests/commands/api.test.js | 119 ++++++++++++++++++++++++++++++++++++- 2 files changed, 125 insertions(+), 11 deletions(-) diff --git a/src/commands/api.js b/src/commands/api.js index 5a526377..5e6bc592 100644 --- a/src/commands/api.js +++ b/src/commands/api.js @@ -2,8 +2,7 @@ * API command - raw API access for power users */ -import { createWriteStream } from 'node:fs'; -import { readFile, unlink } from 'node:fs/promises'; +import { open, readFile, unlink } from 'node:fs/promises'; import { resolve } from 'node:path'; import { Readable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; @@ -207,15 +206,15 @@ export async function apiCommand( }); if (options.output) { let path = resolve(options.output); - let file = createWriteStream(path, { flags: 'wx' }); - let created = false; - file.once('open', () => { - created = true; - }); + let file = await open(path, 'wx'); try { - await pipeline(response.body || Readable.from([]), file); + await pipeline( + response.body || Readable.from([]), + file.createWriteStream() + ); } catch (error) { - if (created) await unlink(path); + await file.close(); + await unlink(path); throw error; } response = { diff --git a/tests/commands/api.test.js b/tests/commands/api.test.js index 475f815f..62335668 100644 --- a/tests/commands/api.test.js +++ b/tests/commands/api.test.js @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { spawn } from 'node:child_process'; import { once } from 'node:events'; -import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { createServer } from 'node:http'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; @@ -28,7 +28,37 @@ before(async () => { headers: req.headers, body, }); - if (req.url === '/api/image') { + if (req.url === '/api/broken-image') { + res.writeHead(200, { + 'Content-Type': 'image/png', + 'Content-Length': '100', + Connection: 'close', + }); + res.end('partial'); + } else if (req.url === '/api/auth/cli/refresh') { + res.setHeader('Content-Type', 'application/json'); + res.end( + JSON.stringify({ + accessToken: 'refreshed-user', + refreshToken: 'new-refresh', + expiresIn: 900, + }) + ); + } else if ( + req.url === '/api/refreshable' && + req.headers.authorization !== 'Bearer refreshed-user' + ) { + res.writeHead(401, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'expired' })); + } else if (req.url === '/api/problem') { + res.writeHead(422, { 'Content-Type': 'application/problem+json' }); + res.end( + JSON.stringify({ + message: 'Invalid decision', + code: 'INVALID_DECISION', + }) + ); + } else if (req.url === '/api/image') { res.setHeader('Content-Type', 'image/png'); res.end(Buffer.from([137, 80, 78, 71, 13, 10, 26, 10])); } else if (req.url === '/api/redirect') { @@ -209,3 +239,88 @@ test('schema discovery works before login but ordinary data reads require creden assert.equal(result.code, 1); assert.equal(requests.length, before); }); + +async function credentialHome(name) { + let home = join(directory, name); + await mkdir(home); + await writeFile( + join(home, 'config.json'), + JSON.stringify({ + auth: { accessToken: 'logged-in-user', refreshToken: 'saved-refresh' }, + projectLink: { + active: 'fixture', + links: { + fixture: { apiUrl: origin, token: 'vzt_linked', storage: 'file' }, + }, + }, + }) + ); + return { VIZZLY_HOME: home, VIZZLY_TOKEN: '' }; +} +test('CLI prefers login over linked upload credentials and honors explicit tokens', async () => { + let environment = await credentialHome('credentials'); + for (let [args, env, expected] of [ + [[], {}, 'logged-in-user'], + [[], { VIZZLY_TOKEN: 'vzt_environment' }, 'vzt_environment'], + [ + ['--token', 'vzt_explicit'], + { VIZZLY_TOKEN: 'vzt_environment' }, + 'vzt_explicit', + ], + ]) { + let result = await cli(['api', '/api/review', ...args], '', { + ...environment, + ...env, + }); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(requests.at(-1).headers.authorization, `Bearer ${expected}`); + } +}); +test('CLI refreshes a read once but never replays generic writes', async () => { + let environment = await credentialHome('refresh'); + let before = requests.length; + let result = await cli(['api', '/api/refreshable'], '', environment); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.deepEqual( + requests.slice(before).map(({ method, url }) => [method, url]), + [ + ['GET', '/api/refreshable'], + ['POST', '/api/auth/cli/refresh'], + ['GET', '/api/refreshable'], + ] + ); + let saved = JSON.parse( + await readFile(join(environment.VIZZLY_HOME, 'config.json'), 'utf8') + ); + assert.equal(saved.auth.accessToken, 'refreshed-user'); + for (let method of ['POST', 'PUT', 'PATCH', 'DELETE']) { + before = requests.length; + result = await cli( + ['api', '/api/unauthorized', '-X', method], + '', + environment + ); + assert.equal(result.code, 1); + assert.equal(requests.length, before + 1); + } +}); +test('failed image downloads remove partial files and binary output requires a file', async () => { + let path = join(directory, 'partial.png'); + let result = await cli(['api', '/api/broken-image', '--output', path]); + assert.equal(result.code, 1); + await assert.rejects(readFile(path), { code: 'ENOENT' }); + result = await cli(['api', '/api/image']); + assert.equal(result.code, 1); + assert.match(result.json.data.error.message, /--output/); +}); +test('HEAD, structured errors, and explicit raw schema paths keep their HTTP behavior', async () => { + let result = await cli(['api', '/api/review', '-X', 'HEAD']); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(result.json.data.response, null); + result = await cli(['api', '/api/problem']); + assert.equal(result.code, 1); + assert.match(result.json.data.error.message, /Invalid decision/); + result = await cli(['api', '/api/schema']); + assert.equal(result.code, 0, result.stderr + result.stdout); + assert.equal(requests.at(-1).url, '/api/schema'); +}); From 107ce8a5927b36cd2e2275481ba49598deae0ec1 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Wed, 23 Sep 2026 13:51:41 -0500 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=90=9B=20Finish=20startup=20config=20?= =?UTF-8?q?save=20before=20reading=20API=20credentials?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Await the menubar PATH save before command dispatch so API authentication cannot read a partially written config or race with token refresh. --- src/cli.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli.js b/src/cli.js index 857e84a4..bfcc8b2d 100644 --- a/src/cli.js +++ b/src/cli.js @@ -1532,9 +1532,9 @@ program await whoamiCommand(options, globalOptions); }); -// Save user's PATH for menubar app (non-blocking, runs in background) -// This auto-configures the menubar app so it can find package runners/node -saveUserPath().catch(() => {}); +// Save PATH for the menubar app before commands read or update credentials +// in the same config file. +await saveUserPath().catch(() => {}); let commandNames = new Set(program.commands.map(command => command.name())); let nestedCommandNames = new Map( From 7db8213c41061652d5898195dbb7cd4fd4b4b628 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Wed, 23 Sep 2026 14:01:43 -0500 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=93=9D=20Teach=20the=20Vizzly=20skill?= =?UTF-8?q?=20schema=20discovery=20and=20API=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guide agents toward compact live request schemas, field selection, image downloads, and authorized review decisions without duplicating endpoint contracts. --- skills/vizzly/SKILL.md | 8 +++++- skills/vizzly/references/cli-context.md | 35 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/skills/vizzly/SKILL.md b/skills/vizzly/SKILL.md index eb9c2466..04995879 100644 --- a/skills/vizzly/SKILL.md +++ b/skills/vizzly/SKILL.md @@ -20,6 +20,12 @@ test workflow in charge of how the UI is exercised. ## Inspect And Verify +For cloud API queries, selectable fields, or an authorized review decision, +start with `vizzly api schema --json` and follow +[schema discovery](references/cli-context.md#query-the-cloud-api). +Discover request details as needed instead of loading the entire OpenAPI document. +The `context` commands below remain useful for local evidence and guided inspection. + 1. Choose the supplied cloud build or comparison when one is named. Otherwise, use current local evidence or find the relevant cloud build. 2. Request bounded JSON: @@ -57,7 +63,7 @@ test workflow in charge of how the UI is exercised. ## Load A Reference When Needed - [CLI context](references/cli-context.md): local and cloud evidence, build - discovery, drill-downs, images, and TDD lifecycle. + discovery, schema queries, review decisions, images, and TDD lifecycle. - [SDK capture](references/sdks.md): add or change screenshot capture code. - [Dynamic content](references/dynamic-content.md): investigate unstable content and screenshot-specific tolerances. diff --git a/skills/vizzly/references/cli-context.md b/skills/vizzly/references/cli-context.md index eb4c342b..27154b7e 100644 --- a/skills/vizzly/references/cli-context.md +++ b/skills/vizzly/references/cli-context.md @@ -4,6 +4,41 @@ Use the repository's established CLI invocation and existing authentication. If cloud authentication is unavailable, report the blocker. Do not start an interactive login unless setup is in scope. +## Query The Cloud API + +Discover the live review API instead of guessing endpoints or adding command flags: + +```bash +vizzly api schema --json +vizzly api schema --json +vizzly api schema -q view=fields --json +vizzly api schema -q view=response --json +``` + +The index lists available operations. The default operation view describes the +method, path, parameters, authentication, and example arguments. Request field +choices or response types only when needed. API JSON payloads are under +`data.response`. + +Call the discovered path using `vizzly api `, `-X` for its method, `-H` +for headers, and `-q` for query parameters. Send the discovered API version +header on data requests. Use `fields` to select just the evidence needed. +Follow the response's pagination values explicitly; the CLI fetches one page +per request. Keep cursors opaque and preserve the query they belong to. + +For a review, discover projects/builds, then inspect the build's screenshots, +comparisons, and image endpoints. Download images with `--output ` +and view baseline, current, and diff together. Use file output for large analysis +responses too. Existing files are not overwritten. Export the full schema only +when needed: `vizzly api schema --full --output --json`. + +Review decisions require an authorized task, user credentials, and the documented +organization header. Discover the decision operation's body before sending it +with `-d @` or `-d @-` for stdin. Generate a fresh `commandId` for each +intended decision, then read back the review state. Generic writes are not +automatically replayed; after an uncertain result, inspect state before retrying. +Treat schema examples as argument arrays, not shell scripts. + ## Choose The Evidence Use an ID supplied by the task. If no cloud build is supplied, list recent