From 5ee0c0401cd73cae73ea32c599167d220f56507d Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Tue, 3 Nov 2026 01:31:02 -0600 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Add=20account=20browsing=20and=20up?= =?UTF-8?q?load=20destination=20selection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep account reads independent of checkout upload credentials, recover saved user sessions, and make project discovery and destination selection explicit. Verify upload targets before cloud writes and exercise the workflows through the built CLI and real terminal prompts. --- README.md | 46 +- skills/vizzly/references/cli-context.md | 27 ++ src/adapters/cli/cli.js | 51 ++- src/adapters/cli/setup.js | 40 +- src/domains/api/api.commands.js | 27 +- src/domains/auth/auth-client.js | 18 +- src/domains/auth/auth-errors.js | 6 +- src/domains/auth/auth.normalizers.js | 12 +- src/domains/auth/auth.service.js | 92 +++- src/domains/auth/cloud-client.js | 73 +--- src/domains/auth/cloud-client.test.js | 19 +- src/domains/auth/credential-presentation.js | 20 + src/domains/auth/credential.rules.js | 46 ++ src/domains/auth/credential.rules.test.js | 41 ++ src/domains/auth/credential.service.js | 14 + src/domains/auth/login.commands.js | 5 +- src/domains/auth/whoami.commands.js | 2 + .../baselines/baseline-download.test.js | 7 +- src/domains/builds/build.service.js | 10 +- src/domains/builds/build.service.test.js | 18 +- src/domains/builds/builds.commands.js | 38 +- src/domains/builds/status.commands.js | 13 +- .../comparisons/comparisons.commands.js | 17 +- src/domains/config/config-loader.js | 12 +- src/domains/context/context.commands.js | 56 +-- src/domains/projects/discovery.serializers.js | 23 + src/domains/projects/discovery.service.js | 16 + src/domains/projects/orgs.commands.js | 64 +-- .../projects/project-picker.service.js | 73 ++++ .../projects/project-status.commands.js | 66 +++ src/domains/projects/project.commands.js | 66 ++- src/domains/projects/projects.commands.js | 73 ++-- .../projects/projects.commands.test.js | 6 +- .../projects/upload-destination.service.js | 59 +++ src/domains/review/review.commands.js | 12 + src/domains/review/review.commands.test.js | 339 --------------- src/domains/runs/run.commands.js | 16 + src/domains/setup/init.commands.js | 30 +- src/domains/tdd/tdd.service.js | 9 +- src/domains/updates/update-notifications.js | 4 +- src/domains/uploads/upload.commands.js | 25 +- src/shared/cloud-pagination.js | 83 ++++ .../terminal-prompts.js} | 2 +- src/shared/vizzly-error.js | 2 +- src/shared/vizzly-error.test.js | 2 +- tests/cli/projects/account-browsing.test.js | 409 ++++++++++++++++++ tests/cli/projects/discovery.test.js | 232 +++------- tests/cli/projects/project-picker.test.js | 129 ++++++ tests/cli/review/review-cli.test.js | 97 ++++- tests/cli/root-help.test.js | 4 +- tests/cli/runs/run-cli.test.js | 15 +- tests/cli/setup/init-guided.test.js | 148 ++----- tests/cli/uploads/upload-cli.test.js | 50 ++- tests/cli/uploads/upload-errors.test.js | 4 +- tests/helpers/cli-runner.js | 9 +- tests/helpers/terminal-runner.js | 120 +++++ 56 files changed, 1913 insertions(+), 984 deletions(-) create mode 100644 src/domains/auth/credential-presentation.js create mode 100644 src/domains/auth/credential.rules.js create mode 100644 src/domains/auth/credential.rules.test.js create mode 100644 src/domains/auth/credential.service.js create mode 100644 src/domains/projects/discovery.serializers.js create mode 100644 src/domains/projects/discovery.service.js create mode 100644 src/domains/projects/project-picker.service.js create mode 100644 src/domains/projects/project-status.commands.js create mode 100644 src/domains/projects/upload-destination.service.js create mode 100644 src/shared/cloud-pagination.js rename src/{domains/setup/setup-prompts.js => shared/terminal-prompts.js} (93%) create mode 100644 tests/cli/projects/account-browsing.test.js create mode 100644 tests/cli/projects/project-picker.test.js create mode 100644 tests/helpers/terminal-runner.js diff --git a/README.md b/README.md index d8961a0a..a304452d 100644 --- a/README.md +++ b/README.md @@ -76,9 +76,22 @@ If the configured port is busy, `tdd run` picks an available one. An explicit ### Review in the cloud +Sign in to discover projects and read their builds from any directory: + ```bash pnpm exec vizzly login -pnpm exec vizzly project link your-org/your-project +pnpm exec vizzly orgs +pnpm exec vizzly projects --org your-org +pnpm exec vizzly builds --project your-project --org your-org +``` + +Browsing doesn't change your checkout's upload destination. When you're ready +to upload, choose a project interactively or supply its qualified name: + +```bash +pnpm exec vizzly project link +# Or: pnpm exec vizzly project link your-org/your-project +pnpm exec vizzly project status pnpm exec vizzly run "pnpm test" ``` @@ -89,6 +102,34 @@ Replace `"pnpm test"` with your test command. If you linked with an older CLI, run `project link` once in each repository to save its project selection. Handwritten config may need a manual edit; the command prints what to add. Explicit `--token` and `VIZZLY_TOKEN` still work. +Before an upload, Vizzly checks the token's project and shows the destination. +If it conflicts with your checkout's configured project, fix the binding or +remove the token override before uploading. + +An explicit token can expose fewer projects than your user account. Use +`--auth user` to browse with your saved login while keeping that upload token: + +```bash +pnpm exec vizzly projects --auth user +pnpm exec vizzly builds --auth user --project another-project --org another-org +``` + +Cloud read commands accept `--auth auto|user|token`. The default, `auto`, uses +an explicit flag, environment, or config token first, then your saved login, +then the checkout's linked credential. `user` selects only your saved login; +`token` selects only an upload or explicit API credential. `--token` and +`--auth user` cannot be combined. User login and upload credentials stay separate. + +Lists show a page of accessible results and tell you how to fetch the next one. +Use `--limit` and `--offset` for scripts; JSON keeps paging details under +`pagination`. `orgs` and `projects` also report the credential source under `auth`. +A project token can see only its own project. Organization/project discovery +now honors explicit token settings consistently with other cloud reads. + +Vizzly refreshes an expired or nearly expired selected login automatically. +If the saved session is rejected, run `vizzly login` and retry your command. +Connection or server errors preserve your saved session and ask you to retry. +Submitted API writes are never automatically replayed after an auth failure. To upload screenshots you already have: @@ -218,7 +259,8 @@ with `pnpm exec` when Vizzly is installed locally. | --- | --- | | `vizzly init` | Walk through setup. Use `--quick` to create config without questions. | | `vizzly config [key]` | Show your current config. | -| `vizzly project link /` | Link this checkout to a cloud project. | +| `vizzly project link [org/project]` | Choose this checkout's upload project; omit the name for a picker. | +| `vizzly project status` | Show the configured and effective upload destinations. | | `vizzly orgs` | List your organizations. | | `vizzly projects` | List projects you can access. | | `vizzly login` | Sign in through the browser. | diff --git a/skills/vizzly/references/cli-context.md b/skills/vizzly/references/cli-context.md index 0af261f0..50224eb0 100644 --- a/skills/vizzly/references/cli-context.md +++ b/skills/vizzly/references/cli-context.md @@ -45,6 +45,33 @@ before trying again. Treat schema examples as argument arrays, not shell scripts ## Choose the evidence +Discover accessible organizations and projects before guessing a destination: + +```bash +vizzly orgs --auth user --json +vizzly projects --auth user --json +vizzly builds --auth user --project --org --json +``` + +These reads don't require linking a checkout. `--auth user` selects the saved +login even when the repo launcher exports a project token. A project token sees +only its own project. In `auto` mode, explicit flag/env/config credentials win +over a saved login; a saved login wins over an implicit linked upload credential. +Use `--auth token` to deliberately inspect the token's view. Don't combine +`--auth user` with `--token`. + +Follow `pagination.nextOffset` when `pagination.hasMore` is true. A list command +returns one logical page; large limits can require several bounded API requests. +If the next offset is null while more results exist, narrow your filters to stay +within the API's offset limit. An expired selected login is refreshed automatically; +a rejected session asks for login. Report the recovery action when interactive +login isn't authorized. Network failures preserve the session. + +`vizzly project status --json` describes configured and effective upload +destinations separately. Use `project link` only when setup or changing the +upload destination is authorized. It issues or reuses a project credential and +binds the checkout. Browsing another project must not change that binding. + Use the ID supplied by the task. If no cloud build is named, find recent builds and choose the one matching the branch, commit, or pull request: diff --git a/src/adapters/cli/cli.js b/src/adapters/cli/cli.js index a5f13e85..f22106e3 100644 --- a/src/adapters/cli/cli.js +++ b/src/adapters/cli/cli.js @@ -47,7 +47,7 @@ import { } from '../../domains/previews/preview.commands.js'; import { orgsCommand } from '../../domains/projects/orgs.commands.js'; import { projectLinkCommand } from '../../domains/projects/project.commands.js'; -import { validateProjectLinkOptions } from '../../domains/projects/project.validations.js'; +import { projectStatusCommand } from '../../domains/projects/project-status.commands.js'; import { projectsCommand, validateProjectsOptions, @@ -589,6 +589,12 @@ async function finishUpdateNotifications() { program.hook('preAction', async (_command, actionCommand) => { if (actionCommand.name() === 'init') return; + if ( + ['run', 'upload', 'login'].includes(actionCommand.name()) || + (actionCommand.parent?.name() === 'tdd' && actionCommand.name() === 'start') + ) { + await saveUserPath().catch(() => {}); + } updateOptions = actionCommand.optsWithGlobals(); let notification = await notifyUpdates(updateOptions); pendingUpdateCheck = notification?.check; @@ -978,6 +984,7 @@ program program .command('status') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Check the status of a build') .argument('', 'Build ID to check status for') .action(async (buildId, options) => { @@ -994,6 +1001,7 @@ program program .command('builds') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('List and query builds') .option('-b, --build ', 'Get a specific build by ID') .option('--branch ', 'Filter by branch') @@ -1035,6 +1043,7 @@ Examples: program .command('comparisons') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Query and search comparisons') .option('-b, --build ', 'Get comparisons for a specific build') .option('--id ', 'Get a specific comparison by ID') @@ -1078,6 +1087,7 @@ let contextCmd = program contextCmd .command('build') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Fetch build context') .argument('', 'Build ID to fetch context for') .option('--source ', 'Context source: auto, cloud, or local', 'auto') @@ -1118,6 +1128,7 @@ Deprecation: contextCmd .command('comparison') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Fetch a comparison context bundle') .argument('', 'Comparison ID to fetch context for') .option('--source ', 'Context source: auto, cloud, or local', 'auto') @@ -1163,6 +1174,7 @@ Deprecation: contextCmd .command('screenshot') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Fetch screenshot context and historical memory') .argument('', 'Screenshot name') .option('--source ', 'Context source: auto, cloud, or local', 'auto') @@ -1195,6 +1207,7 @@ Examples: contextCmd .command('similar') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Fetch project-scoped matches for a fingerprint hash') .argument('', 'Fingerprint hash to search for') .option('--source ', 'Context source: auto, cloud, or local', 'auto') @@ -1227,6 +1240,7 @@ Examples: contextCmd .command('review-queue') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Fetch pending review context for a project') .option('--source ', 'Context source: auto, cloud, or local', 'auto') .option('-p, --project ', 'Project scope for user auth lookups') @@ -1291,6 +1305,7 @@ Note: Baselines are stored locally in .vizzly/baselines/ during TDD mode. let api = program .command('api') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('Make raw API requests (for power users)') .argument('', 'API endpoint (e.g., /api/sdk/builds)') .option( @@ -1463,12 +1478,15 @@ Workflow: program .command('orgs') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('List organizations you have access to') + .option('--limit ', 'Maximum results to return (1-250)', Number, 50) + .option('--offset ', 'Skip first N results', Number, 0) .addHelpText( 'after', ` Examples: - $ vizzly orgs # List all organizations + $ vizzly orgs # List accessible organizations $ vizzly orgs --json # Output as JSON for scripting Note: Shows organizations from your user account (via vizzly login) @@ -1483,6 +1501,7 @@ or the single organization for a project token. program .command('projects') + .option('--auth ', 'Credential source: auto, user, token', 'auto') .description('List projects you have access to') .option('--org ', 'Filter by organization slug') .option('--limit ', 'Maximum results to return (1-250)', Number, 50) @@ -1491,14 +1510,17 @@ program 'after', ` Examples: - $ vizzly projects # List all projects + $ vizzly projects # List accessible projects $ vizzly projects --org my-company # Filter by organization $ vizzly projects --json # Output as JSON for scripting Workflow: 1. List orgs: vizzly orgs 2. List projects: vizzly projects --org - 3. Query builds: vizzly builds + 3. Query builds: vizzly builds --project --org + +Use --auth user to browse your account when VIZZLY_TOKEN selects a project. +Linking is needed only to choose a checkout upload destination. ` ) .action(async options => { @@ -1514,10 +1536,19 @@ Workflow: let projectCommand = program .command('project') - .description('Manage the project linked to this local checkout') + .description('Show or choose the upload destination') + .addHelpText( + 'after', + '\nDiscover: vizzly orgs, vizzly projects --auth user\nBrowse: vizzly builds --auth user --project --org \nChoose uploads: vizzly project link\nInspect uploads: vizzly project status\nBrowsing another project does not change this checkout’s upload destination.\n' + ) .showHelpAfterError('(Run vizzly project --help for available commands)') .showSuggestionAfterError(); +projectCommand + .command('status') + .description('Show configured and effective upload destinations') + .action(async options => projectStatusCommand(options, getGlobalOptions())); + projectCommand .command('link [selector]') .description('Link a Vizzly project for cloud uploads') @@ -1529,6 +1560,7 @@ projectCommand 'after', ` Examples: + $ vizzly project link # Choose from accessible projects $ vizzly project link vizzly/storybook $ vizzly project link --org vizzly --project storybook @@ -1539,11 +1571,6 @@ used for cloud uploads; your user login remains separate for review actions. .action(async (selector, options) => { let globalOptions = getGlobalOptions(); - const validationErrors = validateProjectLinkOptions(selector, options); - if (validationErrors.length > 0) { - await reportValidationErrors(validationErrors); - } - await projectLinkCommand(selector, options, globalOptions, commandDeps); }); @@ -1652,10 +1679,6 @@ program await whoamiCommand(options, globalOptions, commandDeps); }); -// Save PATH for the menubar app before commands read or update credentials -// in the same config file. -if (needsRuntimeBootstrap) await saveUserPath().catch(() => {}); - let commandNames = new Set(program.commands.map(command => command.name())); let nestedCommandNames = new Map( program.commands.map(command => [ diff --git a/src/adapters/cli/setup.js b/src/adapters/cli/setup.js index 3494c49e..1d399c54 100644 --- a/src/adapters/cli/setup.js +++ b/src/adapters/cli/setup.js @@ -1,6 +1,8 @@ import { spawn } from 'node:child_process'; import { fileURLToPath } from 'node:url'; +import { loadConfig } from '../../domains/config/config-loader.js'; import { projectLinkCommand } from '../../domains/projects/project.commands.js'; +import { pickProject } from '../../domains/projects/project-picker.service.js'; function runProcess(command, args) { return new Promise((resolve, reject) => { @@ -18,21 +20,29 @@ function runProcess(command, args) { }); } -export async function connectProject(project, options) { - let exitCode = 0; - let link = () => - projectLinkCommand(project, {}, options, { - exit: code => { - exitCode = code; - }, - }); - let result = await link(); - if (result?.reason === 'auth_required') { +export async function connectProject(options) { + let config = await loadConfig(options.config, options); + let selected; + try { + selected = await pickProject(config, options); + } catch (error) { + if (error.code !== 'LOGIN_REQUIRED') throw error; let cli = fileURLToPath(new URL('./cli.js', import.meta.url)); - await runProcess(process.execPath, [cli, 'login']); - result = await link(); + await runProcess(process.execPath, [ + cli, + 'login', + '--api-url', + config.apiUrl, + ]); + config = await loadConfig(options.config, options); + selected = await pickProject(config, options); } - let status = result?.linked ? 'linked' : 'failed'; - if (!result?.linked && exitCode === 1) status = 'auth_required'; - return { status, project }; + if (!selected) return { status: 'cancelled' }; + let project = `${selected.organizationSlug}/${selected.projectSlug}`; + let result = await projectLinkCommand(project, {}, options, { + exit: code => { + process.exitCode = code; + }, + }); + return { status: result?.linked ? 'linked' : 'failed', project }; } diff --git a/src/domains/api/api.commands.js b/src/domains/api/api.commands.js index 9fb338d2..714e0ba3 100644 --- a/src/domains/api/api.commands.js +++ b/src/domains/api/api.commands.js @@ -8,6 +8,8 @@ import { Readable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; import * as defaultOutput from '../../shared/output.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { resolveCredential } from '../auth/credential.service.js'; +import { showCredential } from '../auth/credential-presentation.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; let API_METHODS = ['GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE']; @@ -152,19 +154,16 @@ export async function apiCommand( let allOptions = { ...globalOptions, ...options }; let config = await loadConfig(globalOptions.config, allOptions); - // 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( - 'Authentication required. Run vizzly login or set VIZZLY_TOKEN.' - ); - output.cleanup(); - exit(1); - return; - } + let credentialConfig = + options.schemaDiscovery && (options.auth || 'auto') === 'auto' + ? { ...config, userToken: null } + : config; + let credential = resolveCredential(credentialConfig, { + ...allOptions, + command: 'api', + allowNoToken: Boolean(options.schemaDiscovery), + }); + let token = credential.token; if (options.data?.startsWith('@')) { let source = options.data.slice(1); @@ -194,6 +193,7 @@ export async function apiCommand( let client = createApiClient({ baseUrl: config.apiUrl, token, + credential, command: 'api', allowNoToken: Boolean(options.schemaDiscovery), }); @@ -237,6 +237,7 @@ export async function apiCommand( // Pretty print response for humans output.header('api'); + showCredential(output, credential, 'projects'); output.labelValue('Endpoint', normalizedEndpoint); output.labelValue('Method', method); output.blank(); diff --git a/src/domains/auth/auth-client.js b/src/domains/auth/auth-client.js index e25b0a6d..76d49527 100644 --- a/src/domains/auth/auth-client.js +++ b/src/domains/auth/auth-client.js @@ -1,4 +1,5 @@ import { getPackageVersion } from '../../shared/package-info.js'; +import { VizzlyError } from '../../shared/vizzly-error.js'; import { parseAuthError, parseAuthenticatedError } from './auth-errors.js'; import { buildAuthUserAgent, buildRequestHeaders } from './auth-headers.js'; @@ -50,10 +51,19 @@ export function createAuthClient(options = {}) { extra: fetchOptions.headers, }); - let response = await fetch(url, { - ...fetchOptions, - headers, - }); + let response; + try { + response = await fetch(url, { + redirect: 'error', + ...fetchOptions, + headers, + }); + } catch { + throw new VizzlyError( + `Unable to reach Vizzly at ${new URL(baseUrl).origin}. Try this command again when connected.`, + 'NETWORK_ERROR' + ); + } if (!response.ok) { let body = await parseResponseBody(response); diff --git a/src/domains/auth/auth-errors.js b/src/domains/auth/auth-errors.js index 17255cf7..7267e0a1 100644 --- a/src/domains/auth/auth-errors.js +++ b/src/domains/auth/auth-errors.js @@ -64,12 +64,14 @@ export function parseAuthenticatedError(status, body, endpoint) { if (status === 401) { return new AuthError( - 'Authentication token is invalid or expired. Please run "vizzly login" again.' + 'Authentication token is invalid or expired. Please run "vizzly login" again.', + { status } ); } return new VizzlyError( `API request failed: ${status}${errorText ? ` - ${errorText}` : ''} (${endpoint})`, - 'API_REQUEST_ERROR' + 'API_REQUEST_ERROR', + { status } ); } diff --git a/src/domains/auth/auth.normalizers.js b/src/domains/auth/auth.normalizers.js index 3c7117b0..38ecb39f 100644 --- a/src/domains/auth/auth.normalizers.js +++ b/src/domains/auth/auth.normalizers.js @@ -4,11 +4,19 @@ * @param {Object|null} existingUser - Existing user data to preserve * @returns {Object} Token data for storage */ -export function buildTokenData(response, existingUser = null) { +export function buildTokenData( + response, + existingUser = null, + now = Date.now() +) { return { accessToken: response.accessToken, refreshToken: response.refreshToken, - expiresAt: response.expiresAt, + expiresAt: + response.expiresAt || + (response.expiresIn != null + ? new Date(now + response.expiresIn * 1000).toISOString() + : undefined), user: response.user || existingUser, }; } diff --git a/src/domains/auth/auth.service.js b/src/domains/auth/auth.service.js index 07a31986..02244622 100644 --- a/src/domains/auth/auth.service.js +++ b/src/domains/auth/auth.service.js @@ -1,4 +1,5 @@ import * as output from '../../shared/output.js'; +import { AuthError, VizzlyError } from '../../shared/vizzly-error.js'; import { getApiUrl } from '../config/environment.js'; import { buildTokenData, @@ -210,12 +211,80 @@ export async function refresh(httpClient, tokenStore) { body: JSON.stringify(buildRefreshPayload(auth.refreshToken)), }); + if ( + typeof response.accessToken !== 'string' || + !response.accessToken || + typeof response.refreshToken !== 'string' || + !response.refreshToken + ) { + throw new VizzlyError( + 'Vizzly returned an invalid login refresh response. Your saved session was preserved.', + 'INVALID_AUTH_RESPONSE' + ); + } + // Preserve existing user data when refreshing await tokenStore.saveTokens(buildTokenData(response, auth.user)); return response; } +/** One selected login and one refresh budget for a command's requests. */ +export function createUserSession({ + baseUrl, + command = 'projects', + httpClient = createAuthClient({ baseUrl }), + tokenStore = createTokenStore(), +} = {}) { + let auth; + let refreshAttempt; + let loginRequired = () => { + let error = new AuthError( + `Your saved login has expired. Run vizzly login, then rerun vizzly ${command}.`, + { status: 401 } + ); + error.code = 'LOGIN_REQUIRED'; + return error; + }; + async function renew() { + if (refreshAttempt) return refreshAttempt; + refreshAttempt = (async () => { + auth = await tokenStore.getTokens(); + if (!auth?.refreshToken) throw loginRequired(); + try { + await refresh(httpClient, tokenStore); + } catch (error) { + if ([400, 401, 403].includes(error.context?.status)) { + throw loginRequired(); + } + if (error.context?.status) { + throw new VizzlyError( + `Unable to refresh your login (HTTP ${error.context.status}). Try this command again later.`, + 'AUTH_REFRESH_UNAVAILABLE', + error.context + ); + } + throw error; + } + auth = await tokenStore.getTokens(); + return auth.accessToken; + })(); + return refreshAttempt; + } + async function prepare() { + auth ||= await tokenStore.getTokens(); + if (!auth?.accessToken) throw loginRequired(); + if ( + auth.expiresAt && + !(Date.parse(auth.expiresAt) > Date.now() + 5 * 60 * 1000) + ) { + await renew(); + } + return auth.accessToken; + } + return { prepare, renew, loginRequired, canRefresh: () => !refreshAttempt }; +} + /** * Logout and revoke tokens * @param {Object} httpClient - HTTP client @@ -252,17 +321,20 @@ export async function logout(httpClient, tokenStore) { * @returns {Promise} User and organization data */ export async function whoami(httpClient, tokenStore) { - let auth = await tokenStore.getTokens(); - let validation = validateTokens(auth, 'accessToken'); - - if (!validation.valid) { - throw validation.error; + let session = createUserSession({ + httpClient, + tokenStore, + command: 'whoami', + }); + let token = await session.prepare(); + try { + return await httpClient.authenticatedRequest('/api/auth/cli/whoami', token); + } catch (error) { + if (error.context?.status !== 401) throw error; + if (!session.canRefresh()) throw session.loginRequired(); + token = await session.renew(); + return httpClient.authenticatedRequest('/api/auth/cli/whoami', token); } - - return httpClient.authenticatedRequest( - '/api/auth/cli/whoami', - auth.accessToken - ); } /** diff --git a/src/domains/auth/cloud-client.js b/src/domains/auth/cloud-client.js index 2f2a99bc..4e89446f 100644 --- a/src/domains/auth/cloud-client.js +++ b/src/domains/auth/cloud-client.js @@ -10,22 +10,16 @@ import { buildRequestHeaders, buildUserAgent, extractErrorBody, - isAuthError, parseApiError, } from '../../shared/cloud-protocol.js'; import { getPackageVersion } from '../../shared/package-info.js'; import { AuthError, VizzlyError } from '../../shared/vizzly-error.js'; -import { getAuthTokens, saveAuthTokens } from './token-store.js'; /** * Default API URL */ export const DEFAULT_API_URL = 'https://app.vizzly.dev'; -function isProjectToken(token) { - return typeof token === 'string' && token.startsWith('vzt_'); -} - /** Prefer the transport cause because Node's top-level fetch message is generic. */ function getNetworkFailureReason(error) { return ( @@ -58,6 +52,7 @@ export function createApiClient(options = {}) { options.sdkUserAgent || options.userAgent ); let allowNoToken = options.allowNoToken || false; + let session = options.credential?.session; // Validate token requirement if (!token && !allowNoToken) { @@ -77,9 +72,12 @@ export function createApiClient(options = {}) { async function request(endpoint, fetchOptions = {}, isRetry = false) { let { responseType = 'json', - retryAuthentication = true, + retryAuthentication = ['GET', 'HEAD'].includes( + fetchOptions.method || 'GET' + ), ...httpOptions } = fetchOptions; + if (session) token = await session.prepare(); let url = buildApiUrl(baseUrl, endpoint); let headers = buildRequestHeaders({ @@ -108,28 +106,14 @@ export function createApiClient(options = {}) { if (!response.ok) { let errorBody = await extractErrorBody(response); - // Handle 401 with token refresh - if ( - retryAuthentication && - response.status === 401 && - !isRetry && - !isProjectToken(token) - ) { - let auth = await getAuthTokens(); - let refreshed = - auth?.accessToken === token && auth.refreshToken - ? await attemptTokenRefresh(auth) - : null; - if (refreshed) { - token = refreshed; + if (response.status === 401) { + if (retryAuthentication && !isRetry && session?.canRefresh()) { + token = await session.renew(); return request(endpoint, fetchOptions, true); } - } - - // Auth error - if (isAuthError(response.status)) { + if (session) throw session.loginRequired(); throw new AuthError( - 'Invalid or expired API token. Run "vizzly project link /" or set VIZZLY_TOKEN.', + 'Invalid or expired API token. Replace the selected --token, VIZZLY_TOKEN, or linked project credential. For account access, use --auth user.', { status: response.status } ); } @@ -157,43 +141,6 @@ export function createApiClient(options = {}) { return response.json(); } - /** - * Attempt to refresh the access token - * @returns {Promise} New token or null if refresh failed - */ - async function attemptTokenRefresh(auth) { - try { - 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, - }, - body: JSON.stringify({ refreshToken: auth.refreshToken }), - }); - - if (!response.ok) return null; - - let data = await response.json(); - - // Save new tokens - await saveAuthTokens({ - accessToken: data.accessToken, - refreshToken: data.refreshToken, - expiresAt: - data.expiresAt || - new Date(Date.now() + data.expiresIn * 1000).toISOString(), - user: auth.user, - }); - - return data.accessToken; - } catch { - return null; - } - } - return { request, getBaseUrl: () => baseUrl, diff --git a/src/domains/auth/cloud-client.test.js b/src/domains/auth/cloud-client.test.js index 55dd2901..7c627bb1 100644 --- a/src/domains/auth/cloud-client.test.js +++ b/src/domains/auth/cloud-client.test.js @@ -6,6 +6,7 @@ import { tmpdir } from 'node:os'; import path from 'node:path'; import { after, before, describe, it } from 'node:test'; import { createApiClient } from './cloud-client.js'; +import { resolveCredential } from './credential.service.js'; import { getAuthTokens, saveAuthTokens } from './token-store.js'; let originalHome; @@ -49,7 +50,7 @@ describe('saved-token API refresh', () => { await saveAuthTokens({ accessToken: 'expired-access', refreshToken: 'saved-refresh', - expiresAt: '2020-01-01T00:00:00.000Z', + expiresAt: '2999-01-01T00:00:00.000Z', user: { id: 'user-1' }, }); @@ -87,6 +88,10 @@ describe('saved-token API refresh', () => { let client = createApiClient({ baseUrl: apiUrl, token: 'expired-access', + credential: resolveCredential({ + apiUrl, + userToken: 'expired-access', + }), }); let result = await client.request('/api/sdk/projects'); @@ -178,7 +183,7 @@ describe('saved-token API refresh', () => { }; await saveAuthTokens(saved); - for (let token of ['explicit-user-access', null]) { + for (let token of ['explicit-user-access', 'saved-user-access', null]) { let requests = []; await withApiServer( (request, response) => { @@ -239,9 +244,13 @@ describe('saved-token API refresh', () => { response.end(JSON.stringify({ error: 'unauthorized' })); }, async apiUrl => { - let client = createApiClient({ baseUrl: apiUrl, token: 'old-access' }); + let client = createApiClient({ + baseUrl: apiUrl, + token: 'old-access', + credential: resolveCredential({ apiUrl, userToken: 'old-access' }), + }); await assert.rejects(() => client.request('/api/sdk/projects'), { - code: 'AUTH_ERROR', + code: 'LOGIN_REQUIRED', }); assert.deepEqual(requests, [ { url: '/api/sdk/projects', authorization: 'Bearer old-access' }, @@ -254,7 +263,7 @@ describe('saved-token API refresh', () => { await assert.rejects( () => client.request('/api/sdk/projects', { retryAuthentication: false }), - { code: 'AUTH_ERROR' } + { code: 'LOGIN_REQUIRED' } ); assert.deepEqual(requests, [ { url: '/api/sdk/projects', authorization: 'Bearer new-access' }, diff --git a/src/domains/auth/credential-presentation.js b/src/domains/auth/credential-presentation.js new file mode 100644 index 00000000..a09e7747 --- /dev/null +++ b/src/domains/auth/credential-presentation.js @@ -0,0 +1,20 @@ +let sources = { + flag: '--token', + environment: 'VIZZLY_TOKEN', + config: 'configuration', + 'linked-project': 'linked project', + user: 'saved user login', + anonymous: 'anonymous access', +}; + +export function credentialSourceLabel(source) { + return sources[source] || source; +} + +export function showCredential(output, credential, command) { + output.hint(`Using ${credentialSourceLabel(credential.source)}`); + if (credential.kind === 'project') { + output.hint('This project token can access only its project.'); + output.hint(`Browse your account: vizzly ${command} --auth user`); + } +} diff --git a/src/domains/auth/credential.rules.js b/src/domains/auth/credential.rules.js new file mode 100644 index 00000000..782875bd --- /dev/null +++ b/src/domains/auth/credential.rules.js @@ -0,0 +1,46 @@ +import { VizzlyError } from '../../shared/vizzly-error.js'; + +export function selectCredential(config, options = {}) { + let mode = options.auth || 'auto'; + if (!['auto', 'user', 'token'].includes(mode)) { + throw new VizzlyError( + '--auth must be auto, user, or token.', + 'INVALID_AUTH' + ); + } + if ( + mode === 'user' && + (options.token || config.credentialSource === 'flag') + ) { + throw new VizzlyError( + 'Use either --auth user or --token, not both.', + 'INVALID_AUTH' + ); + } + let source = + config.credentialSource || + (config.linkedProject ? 'linked-project' : 'config'); + let useUser = + mode === 'user' || + (mode === 'auto' && + config.userToken && + (!config.apiKey || source === 'linked-project')); + let token = useUser ? config.userToken : config.apiKey; + if (!token && (!options.allowNoToken || mode !== 'auto')) { + throw new VizzlyError( + mode === 'token' + ? 'API token required. Set VIZZLY_TOKEN or link an upload project with vizzly project link /.' + : 'Login required. Run vizzly login, then rerun this command.', + 'LOGIN_REQUIRED' + ); + } + return { + token: token || null, + source: token ? (useUser ? 'user' : source) : 'anonymous', + kind: useUser ? 'user' : token?.startsWith('vzt_') ? 'project' : 'token', + }; +} + +export function credentialInfo(credential) { + return { source: credential.source, kind: credential.kind }; +} diff --git a/src/domains/auth/credential.rules.test.js b/src/domains/auth/credential.rules.test.js new file mode 100644 index 00000000..3c69dd76 --- /dev/null +++ b/src/domains/auth/credential.rules.test.js @@ -0,0 +1,41 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { credentialInfo, selectCredential } from './credential.rules.js'; + +test('credential selection honors deliberate sources and keeps account reads separate from uploads', () => { + let linked = { + apiKey: 'vzt_upload', + userToken: 'user-access', + credentialSource: 'linked-project', + }; + assert.deepEqual(credentialInfo(selectCredential(linked)), { + source: 'user', + kind: 'user', + }); + assert.equal(selectCredential(linked, { auth: 'token' }).token, 'vzt_upload'); + for (let source of ['flag', 'environment', 'config']) { + let config = { ...linked, credentialSource: source }; + assert.equal(selectCredential(config).token, 'vzt_upload'); + if (source !== 'flag') + assert.equal( + selectCredential(config, { auth: 'user' }).token, + 'user-access' + ); + } + assert.equal(selectCredential({ apiKey: 'vzt_ci' }).token, 'vzt_ci'); + assert.equal(selectCredential({ userToken: 'user-access' }).source, 'user'); + assert.throws( + () => selectCredential(linked, { token: 'explicit', auth: 'user' }), + { code: 'INVALID_AUTH' } + ); + assert.throws(() => selectCredential({}, { auth: 'user' }), { + code: 'LOGIN_REQUIRED', + }); + assert.throws(() => selectCredential({}, { auth: 'token' }), { + code: 'LOGIN_REQUIRED', + }); + assert.equal( + selectCredential({}, { allowNoToken: true }).source, + 'anonymous' + ); +}); diff --git a/src/domains/auth/credential.service.js b/src/domains/auth/credential.service.js new file mode 100644 index 00000000..dc0f6230 --- /dev/null +++ b/src/domains/auth/credential.service.js @@ -0,0 +1,14 @@ +import { createUserSession } from './auth.service.js'; +import { selectCredential } from './credential.rules.js'; + +/** Select one identity without refreshing during configuration discovery. */ +export function resolveCredential(config, options = {}) { + let credential = selectCredential(config, options); + if (credential.source === 'user') { + credential.session = createUserSession({ + baseUrl: config.apiUrl, + command: options.command, + }); + } + return credential; +} diff --git a/src/domains/auth/login.commands.js b/src/domains/auth/login.commands.js index 38be20eb..1394de92 100644 --- a/src/domains/auth/login.commands.js +++ b/src/domains/auth/login.commands.js @@ -188,7 +188,10 @@ export async function loginCommand( } output.blank(); - output.hint('You can now browse cloud builds and download baselines'); + output.hint('Discover your projects: vizzly projects --auth user'); + output.hint( + 'Choose this checkout’s upload destination: vizzly project link' + ); output.cleanup(); } catch (error) { diff --git a/src/domains/auth/whoami.commands.js b/src/domains/auth/whoami.commands.js index f5a64daa..7e166003 100644 --- a/src/domains/auth/whoami.commands.js +++ b/src/domains/auth/whoami.commands.js @@ -108,6 +108,7 @@ export async function whoamiCommand( let tokenStore = createTokenStore(); let response = await whoami(client, tokenStore); + auth = await getAuthTokens(); output.stopSpinner(); @@ -196,6 +197,7 @@ export async function whoamiCommand( output.data({ authenticated: false, error: error.message, + ...(error.code ? { code: error.code } : {}), }); } else { output.error('Authentication token is invalid or expired', error); diff --git a/src/domains/baselines/baseline-download.test.js b/src/domains/baselines/baseline-download.test.js index 47b8dc3f..2d12c7f0 100644 --- a/src/domains/baselines/baseline-download.test.js +++ b/src/domains/baselines/baseline-download.test.js @@ -116,7 +116,12 @@ describe('cloud baseline installation', () => { return; } if (new URL(req.url, origin).pathname === '/api/sdk/builds') { - res.end(JSON.stringify({ data: [{ id: 'latest-build' }] })); + res.end( + JSON.stringify({ + builds: [{ id: 'latest-build' }], + meta: { total: 1, limit: 1, offset: 0, hasMore: false }, + }) + ); return; } res.end(JSON.stringify(response)); diff --git a/src/domains/builds/build.service.js b/src/domains/builds/build.service.js index 3cc015b5..1aecabfc 100644 --- a/src/domains/builds/build.service.js +++ b/src/domains/builds/build.service.js @@ -1,7 +1,5 @@ -import { - buildEndpointWithParams, - buildQueryParams, -} from '../../shared/cloud-protocol.js'; +import { readCloudPage } from '../../shared/cloud-pagination.js'; +import { buildEndpointWithParams } from '../../shared/cloud-protocol.js'; import { buildBuildPayload } from './build.serializers.js'; /** @@ -40,9 +38,7 @@ export async function getBuildStatus(client, buildId) { * @returns {Promise} List of builds */ export async function getBuilds(client, filters = {}) { - let query = buildQueryParams(filters); - let endpoint = `/api/sdk/builds${query ? `?${query}` : ''}`; - return client.request(endpoint); + return readCloudPage(client, '/api/sdk/builds', 'builds', filters); } /** diff --git a/src/domains/builds/build.service.test.js b/src/domains/builds/build.service.test.js index a325a1ec..34611e84 100644 --- a/src/domains/builds/build.service.test.js +++ b/src/domains/builds/build.service.test.js @@ -23,7 +23,10 @@ describe('cloud builds', () => { body: request.url.endsWith('/status') ? status : request.url.includes('?branch=') - ? { builds: [build] } + ? { + builds: [build], + meta: { total: 1, limit: 50, offset: 0, hasMore: false }, + } : { build }, })); @@ -42,7 +45,16 @@ describe('cloud builds', () => { assert.deepEqual(await getBuildStatus(client, build.id), status); assert.deepEqual( await getBuilds(client, { branch: 'feature/cart', status: 'running' }), - { builds: [build] } + { + builds: [build], + pagination: { + total: 1, + limit: 50, + offset: 0, + hasMore: false, + nextOffset: null, + }, + } ); assert.deepEqual( requests.map(({ method, url, body }) => ({ method, url, body })), @@ -67,7 +79,7 @@ describe('cloud builds', () => { { method: 'GET', url: '/api/sdk/builds/build-1/status', body: null }, { method: 'GET', - url: '/api/sdk/builds?branch=feature%2Fcart&status=running', + url: '/api/sdk/builds?branch=feature%2Fcart&status=running&limit=50&offset=0', body: null, }, ] diff --git a/src/domains/builds/builds.commands.js b/src/domains/builds/builds.commands.js index fad94f45..3b28d9ce 100644 --- a/src/domains/builds/builds.commands.js +++ b/src/domains/builds/builds.commands.js @@ -6,6 +6,9 @@ import { formatBuildForJson } from './build.serializers.js'; import * as defaultOutput from '../../shared/output.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { credentialInfo } from '../auth/credential.rules.js'; +import { resolveCredential } from '../auth/credential.service.js'; +import { showCredential } from '../auth/credential-presentation.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { getBuild as defaultGetBuild, @@ -43,22 +46,17 @@ export async function buildsCommand( let allOptions = { ...globalOptions, ...options }; let config = await loadConfig(globalOptions.config, allOptions); - let token = config.apiKey || config.userToken; - - // Validate cloud auth - if (!token) { - output.error( - 'Authentication required. Use --token, set VIZZLY_TOKEN, or run "vizzly login"' - ); - exit(1); - return; - } - + let credential = resolveCredential(config, { + ...allOptions, + command: 'builds', + }); let client = createApiClient({ baseUrl: config.apiUrl, - token, + token: credential.token, + credential, command: 'builds', }); + if (!globalOptions.json) showCredential(output, credential, 'builds'); // If a specific build ID is provided, get that build if (options.build) { @@ -95,21 +93,19 @@ export async function buildsCommand( let response = await getBuilds(client, filters); output.stopSpinner(); - let builds = response.builds || []; - let pagination = response.pagination || { - total: builds.length, - hasMore: false, - }; + let { builds, pagination } = response; // JSON output if (globalOptions.json) { output.data({ + auth: credentialInfo(credential), builds: builds.map(b => formatBuildForJson(b)), pagination: { total: pagination.total, limit: filters.limit, offset: filters.offset, hasMore: pagination.hasMore, + nextOffset: pagination.nextOffset, }, }); output.cleanup(); @@ -174,9 +170,13 @@ export async function buildsCommand( // Pagination info if (pagination.total > builds.length) { + output.hint(`Showing ${builds.length} of ${pagination.total} builds.`); + } + if (pagination.hasMore) { output.hint( - `Showing ${builds.length} of ${pagination.total} builds. ` + - `Use --offset ${filters.offset + filters.limit} to see more.` + pagination.nextOffset !== null + ? `Use --offset ${pagination.nextOffset} to see more.` + : 'API paging limit reached. Narrow the project or branch filter.' ); } diff --git a/src/domains/builds/status.commands.js b/src/domains/builds/status.commands.js index a5ce9118..31103e5b 100644 --- a/src/domains/builds/status.commands.js +++ b/src/domains/builds/status.commands.js @@ -17,6 +17,7 @@ import { import { getAppBaseUrl } from '../../shared/api-url.js'; import * as defaultOutput from '../../shared/output.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { resolveCredential } from '../auth/credential.service.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { getPreviewInfo as defaultGetPreviewInfo } from '../previews/preview.service.js'; import { getBuildStatus as defaultGetBuildStatus } from './build.service.js'; @@ -40,10 +41,12 @@ function configureOutput(output, globalOptions) { }); } -function createStatusClient({ createApiClient, config }) { +function createStatusClient({ createApiClient, config, options }) { + let credential = resolveCredential(config, { ...options, command: 'status' }); return createApiClient({ baseUrl: config.apiUrl, - token: config.apiKey || config.userToken, + token: credential.token, + credential, command: 'status', }); } @@ -382,7 +385,11 @@ export async function statusCommand( // Get build details via functional API output.startSpinner('Fetching build status...'); - let client = createStatusClient({ createApiClient, config }); + let client = createStatusClient({ + createApiClient, + config, + options: allOptions, + }); let buildStatus = await getBuildStatus(client, buildId); // Also fetch preview info (if exists) diff --git a/src/domains/comparisons/comparisons.commands.js b/src/domains/comparisons/comparisons.commands.js index ea4ad093..91c9d8a1 100644 --- a/src/domains/comparisons/comparisons.commands.js +++ b/src/domains/comparisons/comparisons.commands.js @@ -5,6 +5,7 @@ import * as defaultOutput from '../../shared/output.js'; import { createWildcardMatcher } from '../../shared/patterns.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { resolveCredential } from '../auth/credential.service.js'; import { getBuild as defaultGetBuild } from '../builds/build.service.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { @@ -50,18 +51,14 @@ export async function comparisonsCommand( let allOptions = { ...globalOptions, ...options }; let config = await loadConfig(globalOptions.config, allOptions); - // Validate API token - if (!config.apiKey) { - output.error( - 'API token required. Use --token or set VIZZLY_TOKEN environment variable' - ); - exit(1); - return; - } - + let credential = resolveCredential(config, { + ...allOptions, + command: 'comparisons', + }); let client = createApiClient({ baseUrl: config.apiUrl, - token: config.apiKey, + token: credential.token, + credential, command: 'comparisons', }); diff --git a/src/domains/config/config-loader.js b/src/domains/config/config-loader.js index 0023c887..836136ea 100644 --- a/src/domains/config/config-loader.js +++ b/src/domains/config/config-loader.js @@ -1,6 +1,6 @@ import { resolve } from 'node:path'; import * as output from '../../shared/output.js'; -import { getAccessToken } from '../auth/token-store.js'; +import { getAuthTokens } from '../auth/token-store.js'; import { getProjectLink } from '../projects/project-link-store.js'; import { deepMerge } from './config.normalizers.js'; import { validateVizzlyConfigWithDefaults } from './config.validations.js'; @@ -40,6 +40,7 @@ export async function loadConfig(configPath = null, cliOverrides = {}) { // Merge validated file config mergeConfig(config, validatedFileConfig); + if (config.apiKey) config.credentialSource = 'config'; // 3. Override with environment variables (higher priority than fallbacks) let envApiKey = getApiToken(); @@ -51,6 +52,7 @@ export async function loadConfig(configPath = null, cliOverrides = {}) { if (envApiKey) { config.apiKey = envApiKey; + config.credentialSource = 'environment'; output.debug('config', 'using token from environment'); } if (envApiUrl !== 'https://app.vizzly.dev') config.apiUrl = envApiUrl; @@ -70,6 +72,7 @@ export async function loadConfig(configPath = null, cliOverrides = {}) { // 4. Apply CLI overrides (highest priority) if (cliOverrides.token) { + config.credentialSource = 'flag'; output.debug('config', 'using token from --token flag'); } @@ -94,6 +97,7 @@ export async function loadConfig(configPath = null, cliOverrides = {}) { }); if (linkedProject?.token) { config.apiKey = linkedProject.token; + config.credentialSource = 'linked-project'; config.linkedProject = { account: linkedProject.account, apiUrl: linkedProject.apiUrl, @@ -111,9 +115,9 @@ export async function loadConfig(configPath = null, cliOverrides = {}) { } // 6. Keep user auth separate from upload credentials. - let userToken = await getAccessToken(); - if (userToken) { - config.userToken = userToken; + let auth = await getAuthTokens(); + if (auth) { + config.userToken = auth.accessToken; output.debug('config', 'using user login for user-authenticated commands'); } diff --git a/src/domains/context/context.commands.js b/src/domains/context/context.commands.js index 1b864620..77b7fc39 100644 --- a/src/domains/context/context.commands.js +++ b/src/domains/context/context.commands.js @@ -2,6 +2,8 @@ import * as defaultOutput from '../../shared/output.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { resolveCredential } from '../auth/credential.service.js'; +import { showCredential } from '../auth/credential-presentation.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { readSession as defaultReadSession } from '../runs/session-store.js'; import { @@ -33,10 +35,6 @@ import { import { createLocalWorkspaceContextProvider as defaultCreateLocalWorkspaceContextProvider } from './local-workspace-provider.js'; import { resolveContextSource as defaultResolveContextSource } from './provider-resolver.js'; -function buildAuthErrorMessage() { - return 'Authentication required. Use --token, set VIZZLY_TOKEN, run "vizzly login", or link a project.'; -} - function hasExplicitCloudScope(options = {}, config = {}) { return Boolean( options.org || @@ -49,7 +47,8 @@ function hasExplicitCloudScope(options = {}, config = {}) { function createClient(config, createApiClient) { return createApiClient({ baseUrl: config.apiUrl, - token: config.apiKey || config.userToken, + token: config.readCredential.token, + credential: config.readCredential, command: 'context', }); } @@ -69,27 +68,6 @@ function attachContextSource(context, source) { return { ...context, source }; } -async function loadContextConfig(globalOptions, options, deps) { - let { - loadConfig = defaultLoadConfig, - requireApiKey = true, - output = defaultOutput, - exit = code => process.exit(code), - } = deps; - - let allOptions = { ...globalOptions, ...options }; - let config = await loadConfig(globalOptions.config, allOptions); - - if (requireApiKey && !config.apiKey && !config.userToken) { - output.error(buildAuthErrorMessage()); - output.cleanup(); - exit(1); - return null; - } - - return config; -} - function createCloudContextProvider(config, deps = {}) { let { createApiClient = defaultCreateApiClient, @@ -187,14 +165,12 @@ async function loadContextRuntime( createLocalWorkspaceContextProvider = defaultCreateLocalWorkspaceContextProvider, resolveContextSource = defaultResolveContextSource, output = defaultOutput, - exit = code => process.exit(code), + loadConfig = defaultLoadConfig, } = deps; - let config = await loadContextConfig(globalOptions, options, { - ...deps, - output, - exit, - requireApiKey: false, + let config = await loadConfig(globalOptions.config, { + ...globalOptions, + ...options, }); let requestedSource = options.source || 'auto'; let projectRoot = deps.projectRoot || process.cwd(); @@ -212,17 +188,21 @@ async function loadContextRuntime( } ); - if (source === 'cloud' && !config.apiKey && !config.userToken) { + if (source === 'cloud') { if ( + !config.apiKey && + !config.userToken && shouldExplainLocalSimilarityGap(requestedSource, command, localProvider) ) { throw buildLocalFingerprintCapabilityError(); } - - output.error(buildAuthErrorMessage()); - output.cleanup(); - exit(1); - return null; + config.readCredential = resolveCredential(config, { + ...globalOptions, + ...options, + command: 'context', + }); + if (!globalOptions.json) + showCredential(output, config.readCredential, 'projects'); } let provider = diff --git a/src/domains/projects/discovery.serializers.js b/src/domains/projects/discovery.serializers.js new file mode 100644 index 00000000..a3f0aeef --- /dev/null +++ b/src/domains/projects/discovery.serializers.js @@ -0,0 +1,23 @@ +export function serializeProject(project) { + return { + id: project.id, + name: project.name, + slug: project.slug, + organizationName: project.organizationName, + organizationSlug: project.organizationSlug, + buildCount: project.buildCount, + createdAt: project.created_at, + updatedAt: project.updated_at, + }; +} + +export function serializeOrganization(org) { + return { + id: org.id, + name: org.name, + slug: org.slug, + role: org.role, + projectCount: org.projectCount, + createdAt: org.created_at, + }; +} diff --git a/src/domains/projects/discovery.service.js b/src/domains/projects/discovery.service.js new file mode 100644 index 00000000..3f2d4e83 --- /dev/null +++ b/src/domains/projects/discovery.service.js @@ -0,0 +1,16 @@ +import { readCloudPage } from '../../shared/cloud-pagination.js'; + +export function discoverProjects(client, options = {}) { + let filters = {}; + if (options.org) filters.organization = options.org; + if (options.limit !== undefined) filters.limit = options.limit; + if (options.offset !== undefined) filters.offset = options.offset; + return readCloudPage(client, '/api/sdk/projects', 'projects', filters); +} + +export function discoverOrganizations(client, options = {}) { + return readCloudPage(client, '/api/sdk/organizations', 'organizations', { + limit: options.limit ?? 50, + offset: options.offset ?? 0, + }); +} diff --git a/src/domains/projects/orgs.commands.js b/src/domains/projects/orgs.commands.js index 80d5ca5e..85027f56 100644 --- a/src/domains/projects/orgs.commands.js +++ b/src/domains/projects/orgs.commands.js @@ -1,7 +1,12 @@ import * as defaultOutput from '../../shared/output.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { credentialInfo } from '../auth/credential.rules.js'; +import { resolveCredential } from '../auth/credential.service.js'; +import { showCredential } from '../auth/credential-presentation.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { getApiUrl as defaultGetApiUrl } from '../config/environment.js'; +import { serializeOrganization } from './discovery.serializers.js'; +import { discoverOrganizations } from './discovery.service.js'; /** * Organizations command - List organizations the user has access to @@ -13,11 +18,7 @@ import { getApiUrl as defaultGetApiUrl } from '../config/environment.js'; * @param {Object} globalOptions - Global CLI options * @param {Object} deps - Dependencies for testing */ -export async function orgsCommand( - _options = {}, - globalOptions = {}, - deps = {} -) { +export async function orgsCommand(options = {}, globalOptions = {}, deps = {}) { let { loadConfig = defaultLoadConfig, createApiClient = defaultCreateApiClient, @@ -35,27 +36,21 @@ export async function orgsCommand( try { let config = await loadConfig(globalOptions.config, globalOptions); - // Prefer user auth token for listing orgs (project tokens are org-scoped). - // Falls back to config.apiKey which may be: VIZZLY_TOKEN, --token flag, or project token. - let token = config.userToken || config.apiKey; - - if (!token) { - output.error( - 'API token required. Use --token, set VIZZLY_TOKEN, or run "vizzly login"' - ); - output.cleanup(); - exit(1); - return; - } - + let credential = resolveCredential(config, { + ...globalOptions, + ...options, + command: 'orgs', + }); let client = createApiClient({ baseUrl: config.apiUrl || getApiUrl(), - token, + token: credential.token, + credential, + command: 'orgs', }); output.startSpinner('Fetching organizations...'); - let response = await client.request('/api/sdk/organizations'); + let response = await discoverOrganizations(client, options); output.stopSpinner(); @@ -63,18 +58,14 @@ export async function orgsCommand( if (globalOptions.json) { output.data({ - organizations: orgs.map(org => ({ - id: org.id, - name: org.name, - slug: org.slug, - role: org.role, - projectCount: org.projectCount, - createdAt: org.created_at, - })), + organizations: orgs.map(serializeOrganization), + pagination: response.pagination, + auth: credentialInfo(credential), count: orgs.length, }); } else { output.header('orgs'); + showCredential(output, credential, 'orgs'); let colors = output.getColors(); @@ -89,11 +80,22 @@ export async function orgsCommand( output.print( ` ${colors.bold(org.name)} ${colors.dim(`@${org.slug}`)}` ); - output.print( - ` ${colors.dim(`${org.projectCount} projects · ${roleLabel}`)}` - ); + let details = [ + org.projectCount != null ? `${org.projectCount} projects` : null, + roleLabel, + ] + .filter(Boolean) + .join(' · '); + if (details) output.print(` ${colors.dim(details)}`); } } + if (response.pagination.hasMore) + output.hint( + response.pagination.nextOffset !== null + ? `Use --offset ${response.pagination.nextOffset} to see more` + : 'API paging limit reached.' + ); + output.hint('List projects: vizzly projects --org '); } output.cleanup(); diff --git a/src/domains/projects/project-picker.service.js b/src/domains/projects/project-picker.service.js new file mode 100644 index 00000000..30ee18fa --- /dev/null +++ b/src/domains/projects/project-picker.service.js @@ -0,0 +1,73 @@ +import * as output from '../../shared/output.js'; +import { createPrompts } from '../../shared/terminal-prompts.js'; +import { createApiClient } from '../auth/cloud-client.js'; +import { resolveCredential } from '../auth/credential.service.js'; +import { discoverProjects } from './discovery.service.js'; + +/** A paged selection; selecting a project does not link or issue credentials. */ +export async function pickProject(config, options = {}, prompts = null) { + let credential = resolveCredential(config, { + ...options, + auth: options.token ? 'auto' : 'user', + command: 'project link', + }); + let client = createApiClient({ + baseUrl: config.apiUrl, + token: credential.token, + credential, + command: 'project-link', + }); + let reader = prompts || createPrompts(); + let org = options.org; + let offset = 0; + try { + while (true) { + let page = await discoverProjects(client, { org, offset, limit: 10 }); + output.labelValue( + 'Upload projects', + `${page.projects.length} of ${page.pagination.total}` + ); + if (page.projects.length === 0) + output.hint('No accessible projects in this view.'); + page.projects.forEach((project, index) => { + output.print( + ` ${index + 1}. ${project.organizationSlug}/${project.slug} (${project.name})` + ); + }); + let actions = [ + page.pagination.nextOffset !== null ? 'n: next page' : null, + offset > 0 ? 'b: first page' : null, + 'o: filter organization', + 'q: cancel', + ].filter(Boolean); + output.hint(actions.join(' · ')); + let answer = await reader.ask('Choose project number or action'); + if (answer === 'q') return null; + if (answer === 'o') { + org = await reader.ask('Organization slug (empty for all)'); + offset = 0; + continue; + } + if (answer === 'b') { + offset = 0; + continue; + } + if (answer === 'n' && page.pagination.nextOffset !== null) { + offset = page.pagination.nextOffset; + continue; + } + let selected = /^\d+$/.test(answer) + ? page.projects[Number(answer) - 1] + : null; + if (selected) { + return { + organizationSlug: selected.organizationSlug, + projectSlug: selected.slug, + }; + } + output.hint('Enter a project number or one of the listed actions.'); + } + } finally { + if (!prompts) reader.close(); + } +} diff --git a/src/domains/projects/project-status.commands.js b/src/domains/projects/project-status.commands.js new file mode 100644 index 00000000..abfe4a63 --- /dev/null +++ b/src/domains/projects/project-status.commands.js @@ -0,0 +1,66 @@ +import * as output from '../../shared/output.js'; +import { credentialSourceLabel } from '../auth/credential-presentation.js'; +import { loadConfig } from '../config/config-loader.js'; +import { resolveUploadDestination } from './upload-destination.service.js'; + +function projectLabel(project) { + return project + ? `${project.organizationSlug}/${project.projectSlug}` + : 'none'; +} + +export async function projectStatusCommand(_options = {}, globalOptions = {}) { + output.configure({ json: globalOptions.json, color: !globalOptions.noColor }); + try { + let config = await loadConfig(globalOptions.config, globalOptions); + let destination; + try { + destination = await resolveUploadDestination(config); + } catch (error) { + if (error.code !== 'NETWORK_ERROR' && !(error.context?.status >= 500)) + throw error; + destination = { + apiUrl: config.apiUrl, + configured: config.project || null, + effective: null, + credential: config.apiKey ? { source: config.credentialSource } : null, + status: 'unverified', + matches: null, + error: { code: error.code, message: error.message }, + }; + } + if (globalOptions.json) output.data(destination); + else { + output.header('project'); + output.labelValue('API', destination.apiUrl); + output.labelValue( + 'Checkout project', + projectLabel(destination.configured) + ); + output.labelValue( + 'Upload destination', + destination.status === 'unverified' + ? 'unverified (unable to reach Vizzly)' + : projectLabel(destination.effective) + ); + output.labelValue( + 'Credential', + destination.credential + ? credentialSourceLabel(destination.credential.source) + : 'none' + ); + if (destination.matches === false) + output.warn( + 'The upload token does not match this checkout. Uploads will fail until corrected.' + ); + if (!destination.effective) + output.hint('Choose uploads: vizzly project link'); + output.hint('Browse projects: vizzly projects --auth user'); + } + output.cleanup(); + } catch (error) { + output.error('Failed to inspect upload destination', error); + output.cleanup(); + process.exitCode = 1; + } +} diff --git a/src/domains/projects/project.commands.js b/src/domains/projects/project.commands.js index a230282f..ab5c13f1 100644 --- a/src/domains/projects/project.commands.js +++ b/src/domains/projects/project.commands.js @@ -1,9 +1,14 @@ import { preflightChanges } from '../../shared/file-changes.js'; import * as defaultOutput from '../../shared/output.js'; +import { VizzlyError } from '../../shared/vizzly-error.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { resolveCredential } from '../auth/credential.service.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { getApiUrl as defaultGetApiUrl } from '../config/environment.js'; -import { parseProjectSelector } from './project.validations.js'; +import { + parseProjectSelector, + validateProjectLinkOptions, +} from './project.validations.js'; import { planProjectBinding as defaultPlanProjectBinding, writeProjectBinding as defaultWriteProjectBinding, @@ -12,6 +17,7 @@ import { getProjectLink as defaultGetProjectLink, saveProjectLink as defaultSaveProjectLink, } from './project-link-store.js'; +import { pickProject } from './project-picker.service.js'; export async function projectLinkCommand( selector, @@ -50,6 +56,43 @@ export async function projectLinkCommand( } catch (error) { configError = error; } + if ((selector || options.project) && (!organizationSlug || !projectSlug)) { + throw new VizzlyError( + validateProjectLinkOptions(selector, options).join(' '), + 'PROJECT_INPUT_REQUIRED' + ); + } + if (!organizationSlug || !projectSlug) { + if ( + globalOptions.json || + !process.stdin.isTTY || + !process.stdout.isTTY || + process.env.CI + ) { + throw new VizzlyError( + 'Choose an upload destination: vizzly project link /. Run vizzly projects --auth user to discover projects, or run project link interactively.', + 'PROJECT_INPUT_REQUIRED' + ); + } + if (configError) throw configError; + let selected = await pickProject(config, { + ...globalOptions, + ...options, + org: organizationSlug, + }); + if (!selected) { + output.hint('Project selection cancelled; no files changed.'); + output.cleanup(); + return { linked: false, reason: 'cancelled' }; + } + organizationSlug = selected.organizationSlug; + projectSlug = selected.projectSlug; + } + let errors = validateProjectLinkOptions( + `${organizationSlug}/${projectSlug}` + ); + if (errors.length) + throw new VizzlyError(errors.join(' '), 'PROJECT_INPUT_REQUIRED'); let apiUrl = config?.apiUrl || getApiUrl(); let project = { organizationSlug, projectSlug }; let binding = await planProjectBinding({ @@ -112,9 +155,15 @@ export async function projectLinkCommand( if (!linkedProject) { output.startSpinner(`Linking ${organizationSlug}/${projectSlug}...`); + let credential = resolveCredential(config, { + ...globalOptions, + auth: globalOptions.token ? 'auto' : 'user', + command: 'project link', + }); let client = createApiClient({ baseUrl: apiUrl, - token: userToken, + token: credential.token, + credential, command: 'project-link', }); let response = await client.request( @@ -187,16 +236,25 @@ export async function projectLinkCommand( `Repository project: ${linkedProject.organizationSlug}/${linkedProject.projectSlug}` ); output.hint( - `Cloud uploads and project-scoped context will use ${linkedProject.tokenPrefix}...` + 'Cloud uploads use this project credential. Browse other projects with --auth user.' ); output.hint(`Credential storage: ${linkedProject.storage}`); output.cleanup(); return result; } catch (error) { output.stopSpinner(); + if (error.code === 'PROMPT_CANCELLED') { + output.hint('Project selection cancelled; no files changed.'); + output.cleanup(); + exit(130); + return { linked: false, reason: 'cancelled' }; + } output.error('Failed to link project', error); output.cleanup(); exit(1); - return { linked: false, reason: 'link_failed' }; + return { + linked: false, + reason: error.code === 'LOGIN_REQUIRED' ? 'auth_required' : 'link_failed', + }; } } diff --git a/src/domains/projects/projects.commands.js b/src/domains/projects/projects.commands.js index 0d45f484..a9e78021 100644 --- a/src/domains/projects/projects.commands.js +++ b/src/domains/projects/projects.commands.js @@ -1,7 +1,12 @@ import * as defaultOutput from '../../shared/output.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { credentialInfo } from '../auth/credential.rules.js'; +import { resolveCredential } from '../auth/credential.service.js'; +import { showCredential } from '../auth/credential-presentation.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { getApiUrl as defaultGetApiUrl } from '../config/environment.js'; +import { serializeProject } from './discovery.serializers.js'; +import { discoverProjects } from './discovery.service.js'; /** * Projects command - List projects the user has access to @@ -35,58 +40,35 @@ export async function projectsCommand( try { let config = await loadConfig(globalOptions.config, globalOptions); - // Prefer user auth token for listing projects (project tokens are org-scoped). - // Falls back to config.apiKey which may be: VIZZLY_TOKEN, --token flag, or project token. - let token = config.userToken || config.apiKey; - - if (!token) { - output.error( - 'API token required. Use --token, set VIZZLY_TOKEN, or run "vizzly login"' - ); - output.cleanup(); - exit(1); - return; - } - + let credential = resolveCredential(config, { + ...globalOptions, + ...options, + command: 'projects', + }); let client = createApiClient({ baseUrl: config.apiUrl || getApiUrl(), - token, + token: credential.token, + credential, + command: 'projects', }); - // Build query params - let params = new URLSearchParams(); - if (options.org) params.set('organization', options.org); - if (options.limit) params.set('limit', String(options.limit)); - if (options.offset) params.set('offset', String(options.offset)); - - let queryString = params.toString(); - let endpoint = `/api/sdk/projects${queryString ? `?${queryString}` : ''}`; - output.startSpinner('Fetching projects...'); - let response = await client.request(endpoint); + let response = await discoverProjects(client, options); output.stopSpinner(); - let projects = response.projects || []; - let pagination = response.pagination || {}; + let { projects, pagination } = response; if (globalOptions.json) { output.data({ - projects: projects.map(p => ({ - id: p.id, - name: p.name, - slug: p.slug, - organizationName: p.organizationName, - organizationSlug: p.organizationSlug, - buildCount: p.buildCount, - createdAt: p.created_at, - updatedAt: p.updated_at, - })), + projects: projects.map(serializeProject), + auth: credentialInfo(credential), pagination, }); } else { output.header('projects'); + showCredential(output, credential, 'projects'); let colors = output.getColors(); @@ -106,16 +88,25 @@ export async function projectsCommand( output.print( ` ${colors.bold(project.name)} ${colors.dim(`@${project.organizationSlug}/${project.slug}`)}` ); - output.print(` ${colors.dim(`${project.buildCount} builds`)}`); + if (project.buildCount != null) + output.print(` ${colors.dim(`${project.buildCount} builds`)}`); } if (pagination.hasMore) { output.blank(); output.hint( - `Use --offset ${(options.offset || 0) + projects.length} to see more` + pagination.nextOffset !== null + ? `Use --offset ${pagination.nextOffset} to see more` + : 'API paging limit reached. Narrow the organization filter.' ); } } + output.hint( + 'Read builds: vizzly builds --project --org ' + ); + output.hint( + 'Choose an upload destination: vizzly project link /' + ); } output.cleanup(); @@ -146,9 +137,11 @@ export function validateProjectsOptions(options = {}) { if ( options.offset !== undefined && - (!Number.isInteger(options.offset) || options.offset < 0) + (!Number.isInteger(options.offset) || + options.offset < 0 || + options.offset > 10000) ) { - errors.push('--offset must be a non-negative integer'); + errors.push('--offset must be an integer between 0 and 10000'); } return errors; diff --git a/src/domains/projects/projects.commands.test.js b/src/domains/projects/projects.commands.test.js index 03d77a9e..8bf5c357 100644 --- a/src/domains/projects/projects.commands.test.js +++ b/src/domains/projects/projects.commands.test.js @@ -17,11 +17,11 @@ describe('validateProjectsOptions', () => { } }); - it('rejects malformed and negative offsets', () => { - for (let offset of [Number('1abc'), -1, 1.5]) { + it('rejects malformed and out-of-range offsets', () => { + for (let offset of [Number('1abc'), -1, 1.5, 10001]) { assert.ok( validateProjectsOptions({ offset }).includes( - '--offset must be a non-negative integer' + '--offset must be an integer between 0 and 10000' ) ); } diff --git a/src/domains/projects/upload-destination.service.js b/src/domains/projects/upload-destination.service.js new file mode 100644 index 00000000..8b5fdffb --- /dev/null +++ b/src/domains/projects/upload-destination.service.js @@ -0,0 +1,59 @@ +import { VizzlyError } from '../../shared/vizzly-error.js'; +import { createApiClient } from '../auth/cloud-client.js'; +import { credentialInfo, selectCredential } from '../auth/credential.rules.js'; + +/** Resolve the effective upload token, independently of the saved user login. */ +export async function resolveUploadDestination(config) { + let result = { + apiUrl: config.apiUrl, + configured: config.project || null, + effective: null, + credential: null, + status: 'unconfigured', + matches: null, + }; + if (!config.apiKey) return result; + let credential = selectCredential(config, { auth: 'token' }); + result.credential = credentialInfo(credential); + result.status = 'unverified'; + let client = createApiClient({ + baseUrl: config.apiUrl, + token: credential.token, + credential, + command: 'project-status', + }); + let context = await client.request('/api/sdk/token/context'); + if (!context.organization?.slug || !context.project?.slug) { + throw new VizzlyError( + 'This credential has no upload project. Run vizzly project link /.', + 'UPLOAD_PROJECT_REQUIRED' + ); + } + result.effective = { + organizationSlug: context.organization.slug, + projectSlug: context.project.slug, + }; + result.status = 'verified'; + result.matches = + !result.configured || + (result.configured.organizationSlug === result.effective.organizationSlug && + result.configured.projectSlug === result.effective.projectSlug); + return result; +} + +export async function requireUploadDestination(config) { + let destination = await resolveUploadDestination(config); + if (destination.status !== 'verified') { + throw new VizzlyError( + 'Choose an upload destination with vizzly project link or set VIZZLY_TOKEN.', + 'UPLOAD_PROJECT_REQUIRED' + ); + } + if (!destination.matches) { + throw new VizzlyError( + `Upload token targets ${destination.effective.organizationSlug}/${destination.effective.projectSlug}, but this checkout is configured for ${destination.configured.organizationSlug}/${destination.configured.projectSlug}. Remove the token override or link the intended project before uploading.`, + 'UPLOAD_PROJECT_MISMATCH' + ); + } + return destination; +} diff --git a/src/domains/review/review.commands.js b/src/domains/review/review.commands.js index 9858a7b8..0d115d0c 100644 --- a/src/domains/review/review.commands.js +++ b/src/domains/review/review.commands.js @@ -1,3 +1,4 @@ +import { createUserSession } from '../auth/auth.service.js'; /** * Review commands - approve, reject, and comment on comparisons/builds */ @@ -54,9 +55,18 @@ async function getReviewToken(config, getAccessToken) { } function createReviewClient({ createApiClient, config, command, token }) { + let credential = { + token, + source: token === config.userToken ? 'user' : 'config', + kind: 'user', + }; + if (credential.source === 'user') { + credential.session = createUserSession({ baseUrl: config.apiUrl, command }); + } return createApiClient({ baseUrl: config.apiUrl, token, + credential, command, }); } @@ -239,6 +249,8 @@ async function runComparisonDecision({ `/api/visual-review/builds/${locator.build_id}/comparisons/${comparisonId}/decision`, { method: 'POST', + // The server deduplicates this decision by the unchanged commandId. + retryAuthentication: true, headers: { ...organizationHeaders, 'Content-Type': 'application/json', diff --git a/src/domains/review/review.commands.test.js b/src/domains/review/review.commands.test.js index 681c2331..ac8b9312 100644 --- a/src/domains/review/review.commands.test.js +++ b/src/domains/review/review.commands.test.js @@ -1,88 +1,14 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import { - approveCommand, - commentCommand, createApprovalBody, createCommentBody, createRejectionBody, - rejectCommand, validateApproveOptions, validateCommentOptions, validateRejectOptions, } from './review.commands.js'; -function createMockOutput() { - let calls = []; - return { - calls, - configure: opts => calls.push({ method: 'configure', args: [opts] }), - data: obj => calls.push({ method: 'data', args: [obj] }), - labelValue: (label, value) => - calls.push({ method: 'labelValue', args: [label, value] }), - hint: message => calls.push({ method: 'hint', args: [message] }), - complete: message => calls.push({ method: 'complete', args: [message] }), - error: (message, error) => - calls.push({ method: 'error', args: [message, error] }), - startSpinner: message => - calls.push({ method: 'startSpinner', args: [message] }), - stopSpinner: () => calls.push({ method: 'stopSpinner', args: [] }), - cleanup: () => calls.push({ method: 'cleanup', args: [] }), - }; -} - -function createReviewHarness(response = {}) { - let output = createMockOutput(); - let clientCalls = []; - let clientConfig = null; - let exitCode = null; - - return { - output, - get clientCalls() { - return clientCalls; - }, - get clientConfig() { - return clientConfig; - }, - get exitCode() { - return exitCode; - }, - deps: { - loadConfig: async () => ({ - userToken: 'token-123', - apiUrl: 'https://api.example.test', - }), - getAccessToken: async () => null, - randomUUID: () => 'command-1', - createApiClient: config => { - clientConfig = config; - return { - request: async (endpoint, options) => { - clientCalls.push({ endpoint, options }); - if (endpoint === '/api/sdk/comparisons/comparison-1') { - return { - comparison: { id: 'comparison-1' }, - review_locator: { - comparison_id: 'comparison-1', - build_id: 'build-1', - organization_slug: 'acme', - project_slug: 'website', - }, - }; - } - return response; - }, - }; - }, - output, - exit: code => { - exitCode = code; - }, - }, - }; -} - describe('commands/review', () => { describe('validation', () => { it('validates approve inputs', () => { @@ -141,269 +67,4 @@ describe('commands/review', () => { ); }); }); - - describe('approveCommand', () => { - it('finds the review context and approves the comparison', async () => { - let harness = createReviewHarness({ - eventId: 'event-1', - idempotent: false, - review: { build: { state: 'approved' } }, - }); - - await approveCommand('comparison-1', {}, { json: true }, harness.deps); - - assert.deepStrictEqual(harness.clientConfig, { - baseUrl: 'https://api.example.test', - token: 'token-123', - command: 'approve', - }); - assert.deepStrictEqual(harness.clientCalls, [ - { - endpoint: '/api/sdk/comparisons/comparison-1', - options: undefined, - }, - { - endpoint: - '/api/visual-review/builds/build-1/comparisons/comparison-1/decision', - options: { - method: 'POST', - body: JSON.stringify({ - commandId: 'command-1', - decision: 'approved', - }), - headers: { - 'Content-Type': 'application/json', - 'X-Organization': 'acme', - }, - }, - }, - ]); - - let dataCall = harness.output.calls.find(call => call.method === 'data'); - assert.deepStrictEqual(dataCall.args[0], { - eventId: 'event-1', - idempotent: false, - review: { build: { state: 'approved' } }, - }); - }); - - it('sends an approval comment when provided', async () => { - let harness = createReviewHarness({ - eventId: 'event-1', - idempotent: false, - review: { build: { state: 'approved' } }, - }); - - await approveCommand( - 'comparison-1', - { comment: 'LGTM' }, - {}, - harness.deps - ); - - assert.deepStrictEqual(harness.clientCalls[1].options, { - method: 'POST', - body: JSON.stringify({ - commandId: 'command-1', - decision: 'approved', - annotation: 'LGTM', - }), - headers: { - 'Content-Type': 'application/json', - 'X-Organization': 'acme', - }, - }); - assert.ok( - harness.output.calls.some( - call => - call.method === 'complete' && call.args[0].includes('approved') - ) - ); - assert.ok( - harness.output.calls.some( - call => call.method === 'hint' && call.args[0] === 'Comment: "LGTM"' - ) - ); - }); - }); - - describe('rejectCommand', () => { - it('rejects a comparison with a required reason', async () => { - let harness = createReviewHarness({ - eventId: 'event-2', - idempotent: false, - review: { build: { state: 'rejected' } }, - }); - - await rejectCommand( - 'comparison-1', - { reason: 'Visual regression' }, - { json: true }, - harness.deps - ); - - assert.deepStrictEqual(harness.clientCalls, [ - { - endpoint: '/api/sdk/comparisons/comparison-1', - options: undefined, - }, - { - endpoint: - '/api/visual-review/builds/build-1/comparisons/comparison-1/decision', - options: { - method: 'POST', - body: JSON.stringify({ - commandId: 'command-1', - decision: 'rejected', - annotation: 'Visual regression', - }), - headers: { - 'Content-Type': 'application/json', - 'X-Organization': 'acme', - }, - }, - }, - ]); - - let dataCall = harness.output.calls.find(call => call.method === 'data'); - assert.deepStrictEqual(dataCall.args[0], { - eventId: 'event-2', - idempotent: false, - review: { build: { state: 'rejected' } }, - }); - }); - - it('exits before API work when reason is missing', async () => { - let harness = createReviewHarness(); - - await rejectCommand('comparison-1', {}, {}, harness.deps); - - assert.strictEqual(harness.exitCode, 1); - assert.deepStrictEqual(harness.clientCalls, []); - assert.ok(harness.output.calls.some(call => call.method === 'cleanup')); - }); - }); - - describe('commentCommand', () => { - it('creates a build comment and prints human-readable output', async () => { - let harness = createReviewHarness({ - comment: { id: 'comment-1', content: 'Looks good' }, - }); - - await commentCommand( - 'build-1', - 'Looks good', - { type: 'approval' }, - {}, - harness.deps - ); - - assert.deepStrictEqual(harness.clientConfig, { - baseUrl: 'https://api.example.test', - token: 'token-123', - command: 'comment', - }); - assert.deepStrictEqual(harness.clientCalls, [ - { - endpoint: '/api/sdk/builds/build-1/comments', - options: { - method: 'POST', - body: JSON.stringify({ - content: 'Looks good', - type: 'approval', - }), - headers: { 'Content-Type': 'application/json' }, - }, - }, - ]); - assert.ok( - harness.output.calls.some( - call => call.method === 'complete' && call.args[0] === 'Comment added' - ) - ); - assert.ok( - harness.output.calls.some( - call => - call.method === 'labelValue' && - call.args[0] === 'Message' && - call.args[1] === 'Looks good' - ) - ); - }); - - it('exits before API work when the message is blank', async () => { - let harness = createReviewHarness(); - - await commentCommand('build-1', ' ', {}, {}, harness.deps); - - assert.strictEqual(harness.exitCode, 1); - assert.deepStrictEqual(harness.clientCalls, []); - assert.ok(harness.output.calls.some(call => call.method === 'cleanup')); - }); - }); - - describe('error handling', () => { - it('cleans up and exits when no API token is configured', async () => { - let output = createMockOutput(); - let exitCode = null; - - await approveCommand( - 'comparison-1', - {}, - {}, - { - loadConfig: async () => ({ apiUrl: 'https://api.example.test' }), - getAccessToken: async () => null, - 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 === 'hint')); - assert.ok(output.calls.some(call => call.method === 'cleanup')); - }); - - it('returns JSON failure details when an API request fails', async () => { - let output = createMockOutput(); - let exitCode = null; - let error = new Error('API failed'); - error.code = 'server_error'; - - await commentCommand( - 'build-1', - 'Looks good', - {}, - { json: true }, - { - loadConfig: async () => ({ - userToken: 'token-123', - apiUrl: 'https://api.example.test', - }), - getAccessToken: async () => null, - 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], { - created: false, - buildId: 'build-1', - error: { message: 'API failed', code: 'server_error' }, - }); - assert.ok(output.calls.some(call => call.method === 'cleanup')); - }); - }); }); diff --git a/src/domains/runs/run.commands.js b/src/domains/runs/run.commands.js index e354c547..4c736492 100644 --- a/src/domains/runs/run.commands.js +++ b/src/domains/runs/run.commands.js @@ -12,6 +12,7 @@ import { import * as defaultOutput from '../../shared/output.js'; import { VizzlyError } from '../../shared/vizzly-error.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { credentialSourceLabel } from '../auth/credential-presentation.js'; import { createBuild as defaultCreateApiBuild, finalizeBuild as defaultFinalizeApiBuild, @@ -21,6 +22,7 @@ import { createBuildObject as defaultCreateBuildObject } from '../builds/build-i import { CONFIG_DEFAULTS } from '../config/config-defaults.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { getTokenContext as defaultGetTokenContext } from '../context/context.service.js'; +import { requireUploadDestination } from '../projects/upload-destination.service.js'; import { createUploader as defaultCreateUploader } from '../uploads/upload.service.js'; import { finalizeBuild as defaultFinalizeBuild, @@ -57,6 +59,10 @@ export async function resolveBuildDisplayUrl({ } let baseUrl = getAppBaseUrl(config.apiUrl); + if (config.uploadDestination?.effective) { + let { organizationSlug, projectSlug } = config.uploadDestination.effective; + return `${baseUrl}/${organizationSlug}/${projectSlug}/builds/${result.buildId}`; + } try { let client = createApiClient({ @@ -239,6 +245,14 @@ export async function runCommand( return { success: false, reason: 'no-api-key' }; } + if (config.apiKey) { + config.uploadDestination = await requireUploadDestination(config); + let destination = config.uploadDestination; + output.info( + `Upload destination: ${destination.effective.organizationSlug}/${destination.effective.projectSlug} (${credentialSourceLabel(destination.credential.source)})` + ); + } + // Collect git metadata and build info let configuredBuildName = config.build.name && config.build.name !== CONFIG_DEFAULTS.build.name @@ -416,6 +430,7 @@ export async function runCommand( }); let jsonResult = { + destination: config.uploadDestination, buildId: result.buildId, status: 'completed', url: displayUrl, @@ -568,6 +583,7 @@ export async function runCommand( } let jsonResult = { + destination: config.uploadDestination, buildId: result.buildId, status: buildResult.status, url: displayUrl, diff --git a/src/domains/setup/init.commands.js b/src/domains/setup/init.commands.js index a9b6c777..66b7a32e 100644 --- a/src/domains/setup/init.commands.js +++ b/src/domains/setup/init.commands.js @@ -1,31 +1,27 @@ import path from 'node:path'; import { applyChanges } from '../../shared/file-changes.js'; import * as output from '../../shared/output.js'; +import { createPrompts } from '../../shared/terminal-prompts.js'; import { discoverConfig } from '../config/config-discovery.js'; import { planConfig } from '../config/config-plan.js'; -import { validateProjectLinkOptions } from '../projects/project.validations.js'; import { planAgentSetup } from './agent-setup.js'; -import { createPrompts } from './setup-prompts.js'; async function guidedPlan(cwd, config, prompts) { output.hint(`Project: ${cwd}`); let guidance = await prompts.confirm( 'Add the Vizzly skill and AGENTS.md instructions?' ); - let cloudProject = null; - if ( - await prompts.confirm('Connect this repository to a Vizzly Cloud project?') - ) { - cloudProject = await prompts.ask('Project (organization/project)'); - let errors = validateProjectLinkOptions(cloudProject); - if (errors.length) throw new Error(errors.join(' ')); - } + let connectCloud = await prompts.confirm( + 'Connect this repository to a Vizzly Cloud project?' + ); let agents = await planAgentSetup(cwd, { skill: guidance, guidance }); let changes = [planConfig(config), ...agents.changes]; output.labelValue('Proposed changes', ''); output.list(changes.map(change => path.relative(cwd, change.path))); - if (cloudProject) { - output.hint(`Connect project: ${cloudProject}`); + if (connectCloud) { + output.hint( + 'After local setup, sign in and choose an accessible cloud project.' + ); output.hint( 'Local setup is saved first. If cloud linking fails, your local files stay in place.' ); @@ -35,7 +31,7 @@ async function guidedPlan(cwd, config, prompts) { code: 'INIT_CANCELLED', }); } - return { ...agents, changes, cloudProject }; + return { ...agents, changes, connectCloud }; } function agentOptions(options) { @@ -118,7 +114,8 @@ export async function init(options = {}, { connectProject } = {}) { plan = await guidedPlan(cwd, config, prompts); } } catch (error) { - if (error.code !== 'INIT_CANCELLED') throw error; + if (!['INIT_CANCELLED', 'PROMPT_CANCELLED'].includes(error.code)) + throw error; output.hint('Setup cancelled; no files changed.'); return { status: 'cancelled', mode, exitCode: 130 }; } finally { @@ -138,13 +135,12 @@ export async function init(options = {}, { connectProject } = {}) { ...(plan.agentGuidance ? { agentGuidance: plan.agentGuidance } : {}), exitCode: 0, }; - if (plan.cloudProject) { + if (plan.connectCloud) { try { - result.cloud = await connectProject(plan.cloudProject, options); + result.cloud = await connectProject(options); } catch (error) { result.cloud = { status: 'failed', - project: plan.cloudProject, error: error.message, }; } diff --git a/src/domains/tdd/tdd.service.js b/src/domains/tdd/tdd.service.js index 1d8aeac3..0f9c5473 100644 --- a/src/domains/tdd/tdd.service.js +++ b/src/domains/tdd/tdd.service.js @@ -369,7 +369,7 @@ export class TddService { limit: 1, }); - if (!builds.data || builds.data.length === 0) { + if (!builds.builds || builds.builds.length === 0) { output.warn(`No baseline builds found for ${environment}/${branch}`); output.info( 'Tip: Run a build in normal mode first to create baselines' @@ -377,11 +377,14 @@ export class TddService { return null; } - let apiResponse = await getTddBaselines(this.client, builds.data[0].id); + let apiResponse = await getTddBaselines( + this.client, + builds.builds[0].id + ); if (!apiResponse) { throw new Error( - `Build ${builds.data[0].id} not found or API returned null` + `Build ${builds.builds[0].id} not found or API returned null` ); } diff --git a/src/domains/updates/update-notifications.js b/src/domains/updates/update-notifications.js index fff6dd9b..e6f3dab0 100644 --- a/src/domains/updates/update-notifications.js +++ b/src/domains/updates/update-notifications.js @@ -11,13 +11,13 @@ import { getPackageName, getPackageVersion, } from '../../shared/package-info.js'; +import { createPrompts } from '../../shared/terminal-prompts.js'; import { agentGuidanceFlag, agentGuidancePath, bundledAgentSkill, planAgentSetup, } from '../setup/agent-setup.js'; -import { createPrompts } from '../setup/setup-prompts.js'; import { findCliUpdatePlan, runUpdateProcess } from './cli-update.js'; let day = 24 * 60 * 60 * 1000; @@ -121,7 +121,7 @@ async function offerUpdate(cwd, { cliVersion, guidanceFlag }) { } output.printErr(' Vizzly update applied.'); } catch (error) { - if (error.code !== 'INIT_CANCELLED') + if (error.code !== 'PROMPT_CANCELLED') output.printErr(` Could not apply the Vizzly update: ${error.message}`); } return updatedCli; diff --git a/src/domains/uploads/upload.commands.js b/src/domains/uploads/upload.commands.js index 08ce2a14..f436d2a7 100644 --- a/src/domains/uploads/upload.commands.js +++ b/src/domains/uploads/upload.commands.js @@ -9,10 +9,12 @@ import { } from '../../shared/git.js'; import * as defaultOutput from '../../shared/output.js'; import { createApiClient as defaultCreateApiClient } from '../auth/cloud-client.js'; +import { credentialSourceLabel } from '../auth/credential-presentation.js'; import { finalizeBuild as defaultFinalizeBuild } from '../builds/build.service.js'; import { CONFIG_DEFAULTS } from '../config/config-defaults.js'; import { loadConfig as defaultLoadConfig } from '../config/config-loader.js'; import { getTokenContext as defaultGetTokenContext } from '../context/context.service.js'; +import { requireUploadDestination } from '../projects/upload-destination.service.js'; import { writeSession as defaultWriteSession } from '../runs/session-store.js'; import { createUploader as defaultCreateUploader } from './upload.service.js'; @@ -25,6 +27,10 @@ import { createUploader as defaultCreateUploader } from './upload.service.js'; * @returns {Promise} Proper build URL */ export async function constructBuildUrl(buildId, apiUrl, apiToken, deps = {}) { + if (deps.destination?.effective) { + let { organizationSlug, projectSlug } = deps.destination.effective; + return `${getAppBaseUrl(apiUrl)}/${organizationSlug}/${projectSlug}/builds/${buildId}`; + } let { createApiClient = defaultCreateApiClient, getTokenContext = defaultGetTokenContext, @@ -94,6 +100,7 @@ export async function uploadCommand( let buildId = null; let config = null; + let destination = null; let uploadStartTime = Date.now(); try { @@ -112,6 +119,12 @@ export async function uploadCommand( return { success: false, reason: 'no-api-key' }; } + destination = await requireUploadDestination(config); + config.uploadDestination = destination; + output.info( + `Upload destination: ${destination.effective.organizationSlug}/${destination.effective.projectSlug} (${credentialSourceLabel(destination.credential.source)})` + ); + // Collect git metadata if not provided let configuredBuildName = config.build.name && config.build.name !== CONFIG_DEFAULTS.build.name @@ -226,11 +239,12 @@ export async function uploadCommand( result.buildId, config.apiUrl, config.apiKey, - deps + { ...deps, destination } ) : null); output.data({ + destination, buildId: result.buildId, url: buildUrl, stats: { @@ -266,7 +280,7 @@ export async function uploadCommand( result.buildId, config.apiUrl, config.apiKey, - deps + { ...deps, destination } )); output.blank(); output.labelValue('View', output.link('Results', buildUrl)); @@ -289,10 +303,11 @@ export async function uploadCommand( result.buildId, config.apiUrl, config.apiKey, - deps + { ...deps, destination } )); output.data({ + destination, buildId: result.buildId, status: buildResult.failedComparisons > 0 ? 'failed' : 'completed', url: waitBuildUrl, @@ -343,7 +358,7 @@ export async function uploadCommand( result.buildId, config.apiUrl, config.apiKey, - deps + { ...deps, destination } )); output.blank(); output.labelValue('View', output.link('Results', waitBuildUrl)); @@ -362,6 +377,7 @@ export async function uploadCommand( if (status >= 500) { if (globalOptions.json) { output.data({ + destination, buildId: null, status: 'skipped', message: 'Vizzly API unavailable - upload skipped', @@ -394,6 +410,7 @@ export async function uploadCommand( // JSON output for errors if (globalOptions.json) { output.data({ + destination, buildId: buildId || null, status: 'failed', error: { diff --git a/src/shared/cloud-pagination.js b/src/shared/cloud-pagination.js new file mode 100644 index 00000000..6fae15ea --- /dev/null +++ b/src/shared/cloud-pagination.js @@ -0,0 +1,83 @@ +import { VizzlyError } from './vizzly-error.js'; + +export let SDK_API_VERSION = '2026-09-12'; + +/** Read one CLI result window using bounded pages from the cloud API. */ +export async function readCloudPage(client, endpoint, key, filters = {}) { + let { limit = 50, offset = 0, ...query } = filters; + if ( + !Number.isInteger(limit) || + limit < 1 || + limit > 250 || + !Number.isInteger(offset) || + offset < 0 || + offset > 10000 + ) { + throw new VizzlyError( + 'Use --limit 1–250 and --offset 0–10000.', + 'INVALID_PAGE' + ); + } + let items = []; + let seen = new Set(); + let nextOffset = offset; + let meta; + do { + let pageLimit = Math.min(50, limit - items.length); + let params = new URLSearchParams(query); + params.set('limit', String(pageLimit)); + params.set('offset', String(nextOffset)); + let response = await client.request(`${endpoint}?${params}`, { + headers: { 'Vizzly-API-Version': SDK_API_VERSION }, + }); + meta = response.meta; + let page = response[key]; + if ( + !Array.isArray(page) || + !Number.isInteger(meta?.total) || + !Number.isInteger(meta?.limit) || + meta.limit <= 0 || + meta.limit > pageLimit || + meta.offset !== nextOffset || + typeof meta.hasMore !== 'boolean' + ) { + throw new VizzlyError( + 'Invalid cloud list response.', + 'INVALID_PAGE_RESPONSE' + ); + } + if ( + page.length > meta.limit || + page.some(item => typeof item.id !== 'string') + ) { + throw new VizzlyError( + 'Invalid cloud list items.', + 'INVALID_PAGE_RESPONSE' + ); + } + let before = items.length; + for (let item of page) { + if (!seen.has(item.id)) { + seen.add(item.id); + items.push(item); + } + } + nextOffset += meta.limit; + if (meta.hasMore && (page.length === 0 || items.length === before)) { + throw new VizzlyError( + 'Cloud results changed while paging. Run the command again.', + 'PAGE_CHANGED' + ); + } + } while (meta.hasMore && items.length < limit && nextOffset <= 10000); + return { + [key]: items, + pagination: { + total: meta.total, + limit, + offset, + hasMore: meta.hasMore, + nextOffset: meta.hasMore && nextOffset <= 10000 ? nextOffset : null, + }, + }; +} diff --git a/src/domains/setup/setup-prompts.js b/src/shared/terminal-prompts.js similarity index 93% rename from src/domains/setup/setup-prompts.js rename to src/shared/terminal-prompts.js index 1afcfee6..65cd17d0 100644 --- a/src/domains/setup/setup-prompts.js +++ b/src/shared/terminal-prompts.js @@ -7,7 +7,7 @@ export function createPrompts({ let reader = createInterface({ input, crlfDelay: Infinity, terminal: false }); let lines = reader[Symbol.asyncIterator](); let cancelled = () => - Object.assign(new Error('Setup cancelled.'), { code: 'INIT_CANCELLED' }); + Object.assign(new Error('Prompt cancelled.'), { code: 'PROMPT_CANCELLED' }); let interrupted = false; let onInterrupt = () => { interrupted = true; diff --git a/src/shared/vizzly-error.js b/src/shared/vizzly-error.js index 43428ef2..cf7eaf39 100644 --- a/src/shared/vizzly-error.js +++ b/src/shared/vizzly-error.js @@ -57,7 +57,7 @@ export class AuthError extends VizzlyError { } getUserMessage() { - return `Authentication error: ${this.message}. Please check your VIZZLY_TOKEN.`; + return `Authentication error: ${this.message}`; } } diff --git a/src/shared/vizzly-error.test.js b/src/shared/vizzly-error.test.js index 2f4adf79..0b7bb03a 100644 --- a/src/shared/vizzly-error.test.js +++ b/src/shared/vizzly-error.test.js @@ -73,7 +73,7 @@ describe('errors/vizzly-error', () => { Class: AuthError, name: 'AuthError', code: 'AUTH_ERROR', - messageIncludes: ['Authentication error', 'VIZZLY_TOKEN'], + messageIncludes: ['Authentication error'], }, { Class: NetworkError, diff --git a/tests/cli/projects/account-browsing.test.js b/tests/cli/projects/account-browsing.test.js new file mode 100644 index 00000000..d095ddd5 --- /dev/null +++ b/tests/cli/projects/account-browsing.test.js @@ -0,0 +1,409 @@ +import assert from 'node:assert/strict'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import { findJSONMessage, runCLI } from '../../helpers/cli-runner.js'; +import { createCloudServer } from '../../helpers/cloud-server.js'; + +let entrypoint = fileURLToPath( + new URL('../../../bin/vizzly.js', import.meta.url) +); +let userToken = 'synthetic-user-access'; +let projectToken = 'vzt_synthetic-upload'; +let projects = Array.from({ length: 260 }, (_, index) => ({ + id: `project-${index}`, + name: `Project ${index}`, + slug: `app-${index}`, + organizationSlug: index % 2 ? 'studio' : 'acme', +})); + +function page(items, query) { + let limit = Number(query.get('limit') || 10); + let offset = Number(query.get('offset') || 0); + assert.ok(limit <= 50); + return { + items: items.slice(offset, offset + limit), + meta: { + total: items.length, + limit, + offset, + hasMore: offset + limit < items.length, + page: Math.floor(offset / limit) + 1, + totalPages: Math.ceil(items.length / limit), + }, + }; +} + +async function fixture(t, { expired = false, binding = true } = {}) { + let cwd = await mkdtemp(path.join(tmpdir(), 'vizzly-account-')); + let home = path.join(cwd, 'state'); + await mkdir(home); + t.after(() => rm(cwd, { recursive: true, force: true })); + let configPath = path.join(cwd, '.vizzlyrc.json'); + let authPath = path.join(home, 'config.json'); + await writeFile( + authPath, + JSON.stringify({ + auth: { + accessToken: userToken, + refreshToken: 'synthetic-refresh', + expiresAt: expired ? '2020-01-01T00:00:00Z' : '2999-01-01T00:00:00Z', + user: { name: 'Fixture user' }, + }, + }) + ); + await writeFile( + configPath, + JSON.stringify( + binding + ? { + project: { organizationSlug: 'acme', projectSlug: 'app-0' }, + } + : {} + ) + ); + let state = { + refreshStatus: 200, + rejectUser: false, + failPage: null, + projectsStatus: 200, + }; + let { client, requests } = await createCloudServer(t, request => { + let url = new URL(request.url, 'http://fixture'); + let bearer = request.headers.authorization; + if (url.pathname === '/api/auth/cli/refresh') { + assert.ok( + ['synthetic-refresh', 'synthetic-rotated'].includes( + request.body.refreshToken + ) + ); + return { + status: state.refreshStatus, + body: + state.refreshStatus === 200 + ? { + accessToken: 'synthetic-refreshed', + refreshToken: 'synthetic-rotated', + expiresIn: 3600, + } + : { error: 'refresh rejected' }, + }; + } + if (state.rejectUser && bearer !== `Bearer ${projectToken}`) { + return { status: 401, body: { error: 'invalid user' } }; + } + if (url.pathname === '/api/auth/cli/whoami') { + return { body: { user: { name: 'Fixture user' }, organizations: [] } }; + } + if (url.pathname === '/api/sdk/projects') { + assert.equal(request.headers['vizzly-api-version'], '2026-09-12'); + if (state.projectsStatus !== 200) { + return { + status: state.projectsStatus, + body: { error: 'Project access denied' }, + }; + } + if (state.failPage === Number(url.searchParams.get('offset'))) { + return { status: 503, body: { error: 'offline' } }; + } + let visible = + bearer === `Bearer ${projectToken}` ? projects.slice(0, 1) : projects; + let org = url.searchParams.get('organization'); + if (org) visible = visible.filter(p => p.organizationSlug === org); + let { items, meta } = page(visible, url.searchParams); + return { body: { projects: items, meta } }; + } + if (url.pathname === '/api/sdk/organizations') { + let { items, meta } = page( + [ + { id: 'o-1', slug: 'acme', name: 'Acme' }, + { id: 'o-2', slug: 'studio', name: 'Studio' }, + ], + url.searchParams + ); + return { body: { organizations: items, meta } }; + } + if (url.pathname === '/api/sdk/builds') { + if ( + url.searchParams.get('project') === 'web' && + !url.searchParams.has('organization') + ) { + return { + status: 409, + body: { + error: + 'Project slug web is ambiguous. Specify --org acme or --org studio.', + }, + }; + } + assert.equal(url.searchParams.get('project'), 'app-1'); + assert.equal(url.searchParams.get('organization'), 'studio'); + assert.notEqual(bearer, `Bearer ${projectToken}`); + let { items, meta } = page(state.builds || [], url.searchParams); + return { body: { builds: items, meta } }; + } + if (url.pathname === '/api/sdk/token/context') { + return { + body: { organization: { slug: 'acme' }, project: { slug: 'app-0' } }, + }; + } + if (url.pathname === '/api/cli/app-1/link-token') { + return { + body: { + organization: { slug: 'studio' }, + project: { slug: 'app-1' }, + token: { + id: 't-1', + token: projectToken, + token_prefix: 'vzt_fixture', + }, + }, + }; + } + return { status: 404, body: { error: 'Unexpected fixture request' } }; + }); + let apiUrl = client.getBaseUrl(); + async function cli(args, env = {}) { + let result = await runCLI(['--no-color', ...args], { + cwd, + entrypoint, + env: { + VIZZLY_HOME: home, + VIZZLY_API_URL: apiUrl, + VIZZLY_DISABLE_KEYCHAIN: 'true', + ...env, + }, + }); + assert.doesNotMatch( + result.stdout + result.stderr, + /synthetic-user-access|vzt_synthetic-upload|synthetic-refresh|synthetic-rotated/ + ); + return result; + } + return { cwd, home, authPath, configPath, apiUrl, state, requests, cli }; +} + +let data = result => findJSONMessage(result.stdout, 'data')?.data; + +test('built CLI browses account projects with another upload destination and leaves bindings unchanged', async t => { + let f = await fixture(t); + let before = await readFile(f.configPath, 'utf8'); + let authBefore = await readFile(f.authPath, 'utf8'); + let env = { VIZZLY_TOKEN: projectToken }; + let restricted = await f.cli(['projects', '--json'], env); + assert.equal(restricted.code, 0, restricted.stderr); + assert.equal(data(restricted).projects.length, 1); + assert.equal(data(restricted).auth.source, 'environment'); + let all = await f.cli( + ['projects', '--auth', 'user', '--limit', '250', '--json'], + env + ); + assert.equal(all.code, 0, all.stderr); + assert.equal(data(all).projects.length, 250); + assert.equal(data(all).pagination.nextOffset, 250); + assert.equal(data(all).pagination.hasMore, true); + let last = await f.cli( + ['projects', '--auth', 'user', '--offset', '250', '--json'], + env + ); + assert.equal(data(last).projects.length, 10); + assert.equal(data(last).pagination.hasMore, false); + let builds = await f.cli( + [ + 'builds', + '--auth', + 'user', + '--project', + 'app-1', + '--org', + 'studio', + '--json', + ], + env + ); + assert.equal(builds.code, 0, builds.stderr); + let orgs = await f.cli(['orgs', '--auth', 'user', '--json'], env); + assert.equal(data(orgs).count, 2); + let status = await f.cli(['project', 'status', '--json'], env); + assert.equal(data(status).status, 'verified'); + assert.deepEqual(data(status).effective, { + organizationSlug: 'acme', + projectSlug: 'app-0', + }); + assert.equal(await readFile(f.configPath, 'utf8'), before); + assert.equal(await readFile(f.authPath, 'utf8'), authBefore); + assert.ok(f.requests.every(request => request.method === 'GET')); +}); + +test('explicit tokens, source selection, and noninteractive missing targets are deliberate', async t => { + let f = await fixture(t); + let explicit = await f.cli(['--token', projectToken, 'projects', '--json']); + assert.equal(data(explicit).projects.length, 1); + assert.equal(data(explicit).auth.source, 'flag'); + for (let args of [ + ['projects', '--auth', 'invalid'], + ['--token', projectToken, 'projects', '--auth', 'user'], + ['projects', '--auth', 'token'], + ['projects', '--offset', '10001', '--json'], + ['project', 'link', '--json'], + ['project', 'link', 'web', '--json'], + ]) { + let before = f.requests.length; + let result = await f.cli(args); + assert.equal(result.code, 1, result.stdout + result.stderr); + assert.equal(f.requests.length, before); + if (args.includes('10001')) assert.match(result.stderr, /0 and 10000/); + } + let human = await f.cli(['projects'], { VIZZLY_TOKEN: projectToken }); + assert.match(human.stdout + human.stderr, /only its project/); + assert.match(human.stdout + human.stderr, /--auth user/); + assert.doesNotMatch(human.stdout + human.stderr, /undefined/); +}); + +test('expired and near-expiry saved sessions recover without falling back to the upload token', async t => { + for (let expiresAt of [ + '2020-01-01T00:00:00Z', + new Date(Date.now() + 120000).toISOString(), + ]) { + let f = await fixture(t); + let auth = JSON.parse(await readFile(f.authPath, 'utf8')); + auth.auth.expiresAt = expiresAt; + await writeFile(f.authPath, JSON.stringify(auth)); + let result = await f.cli(['projects', '--auth', 'user', '--json'], { + VIZZLY_TOKEN: projectToken, + }); + assert.equal(result.code, 0, result.stderr); + assert.equal(f.requests[0].url, '/api/auth/cli/refresh'); + let saved = JSON.parse(await readFile(f.authPath, 'utf8')).auth; + assert.equal(saved.accessToken, 'synthetic-refreshed'); + assert.equal(saved.user.name, 'Fixture user'); + assert.ok(Date.parse(saved.expiresAt) > Date.now()); + assert.equal(f.requests.filter(r => r.url.includes('/refresh')).length, 1); + } +}); + +test('rejected and unavailable refreshes preserve session state and give different recovery advice', async t => { + for (let status of [401, 503]) { + let f = await fixture(t, { expired: true }); + f.state.refreshStatus = status; + let before = await readFile(f.authPath, 'utf8'); + let result = await f.cli(['projects', '--auth', 'user', '--json'], { + VIZZLY_TOKEN: projectToken, + }); + assert.equal(result.code, 1); + assert.equal(f.requests.length, 1); + assert.equal(await readFile(f.authPath, 'utf8'), before); + if (status === 401) + assert.match(result.stderr, /LOGIN_REQUIRED|vizzly login/); + else assert.doesNotMatch(result.stderr, /vizzly login/); + } +}); + +test('whoami reports refreshed expiry and a command never refreshes twice', async t => { + let f = await fixture(t, { expired: true }); + let who = await f.cli(['whoami', '--json']); + assert.equal(who.code, 0, who.stderr); + assert.ok(Date.parse(data(who).tokenExpiresAt) > Date.now()); + f.state.rejectUser = true; + f.requests.length = 0; + let result = await f.cli(['projects', '--auth', 'user', '--json']); + assert.equal(result.code, 1); + assert.equal(f.requests.filter(r => r.url.includes('/refresh')).length, 1); + assert.equal(f.requests.filter(r => r.method === 'GET').length, 2); +}); + +test('a later discovery page failure never returns a partial successful catalog', async t => { + let f = await fixture(t); + f.state.failPage = 50; + let result = await f.cli(['projects', '--limit', '100', '--json']); + assert.equal(result.code, 1); + assert.equal(data(result), undefined); +}); + +test('access denial and ambiguous project names do not refresh or change identity', async t => { + let f = await fixture(t); + f.state.projectsStatus = 403; + let denied = await f.cli(['projects', '--auth', 'user', '--json']); + assert.equal(denied.code, 1); + assert.doesNotMatch(denied.stderr, /vizzly login/); + let ambiguous = await f.cli(['builds', '--project', 'web', '--json']); + assert.equal(ambiguous.code, 1); + assert.match(ambiguous.stderr, /--org acme.*--org studio/); + assert.ok(f.requests.every(r => r.method === 'GET')); +}); + +test('build list output shows the page total and offers a next page only when one exists', async t => { + let f = await fixture(t); + f.state.builds = [ + { id: 'b-1', name: 'Checkout', status: 'completed', branch: 'main' }, + { id: 'b-2', name: 'Settings', status: 'completed', branch: 'main' }, + ]; + let args = [ + 'builds', + '--project', + 'app-1', + '--org', + 'studio', + '--limit', + '1', + ]; + let first = await f.cli(args); + assert.equal(first.code, 0, first.stderr); + assert.match(first.stdout, /Showing 1 of 2 builds/); + assert.match(first.stdout, /--offset 1/); + let last = await f.cli([...args, '--offset', '1']); + assert.equal(last.code, 0, last.stderr); + assert.match(last.stdout, /Settings/); + assert.doesNotMatch(last.stdout, /--offset|paging limit/); +}); + +test('offline project status keeps the checkout binding separate from an unverified destination', async t => { + let f = await fixture(t); + let result = await f.cli(['project', 'status', '--json'], { + VIZZLY_TOKEN: projectToken, + VIZZLY_API_URL: 'http://127.0.0.1:0', + }); + assert.equal(result.code, 0, result.stderr); + assert.equal(data(result).status, 'unverified'); + assert.equal(data(result).effective, null); + assert.deepEqual(data(result).configured, { + organizationSlug: 'acme', + projectSlug: 'app-0', + }); +}); + +test('linking chooses uploads, browsing stays read-only, and a mismatched override fails before writes', async t => { + let f = await fixture(t, { binding: false }); + let linked = await f.cli(['project', 'link', 'studio/app-1', '--json']); + assert.equal(linked.code, 0, linked.stderr); + let before = await readFile(f.configPath, 'utf8'); + let discovered = await f.cli(['projects', '--json']); + assert.equal(data(discovered).auth.source, 'user'); + let browsed = await f.cli([ + 'builds', + '--auth', + 'user', + '--project', + 'app-1', + '--org', + 'studio', + '--json', + ]); + assert.equal(browsed.code, 0, browsed.stderr); + assert.equal(await readFile(f.configPath, 'utf8'), before); + let screenshots = path.join(f.cwd, 'screenshots'); + await mkdir(screenshots); + await writeFile(path.join(screenshots, 'home.png'), 'synthetic-png'); + f.requests.length = 0; + let upload = await f.cli(['upload', screenshots, '--json'], { + VIZZLY_TOKEN: projectToken, + }); + assert.equal(upload.code, 1); + assert.match( + upload.stdout + upload.stderr, + /UPLOAD_PROJECT_MISMATCH|does not match|configured for/ + ); + assert.ok(f.requests.every(r => r.method === 'GET')); +}); diff --git a/tests/cli/projects/discovery.test.js b/tests/cli/projects/discovery.test.js index 724aa73b..442dd502 100644 --- a/tests/cli/projects/discovery.test.js +++ b/tests/cli/projects/discovery.test.js @@ -3,7 +3,7 @@ import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { describe, it } from 'node:test'; -import { findJSONMessage, runCLI } from '../../helpers/cli-runner.js'; +import { runCLI } from '../../helpers/cli-runner.js'; import { createCloudServer } from '../../helpers/cloud-server.js'; let projects = [ @@ -83,192 +83,76 @@ async function runDiscoveryCLI(workspace, apiUrl, args, env = {}) { }); } -function jsonData(result) { - return findJSONMessage(result.stdout, 'data')?.data; -} - describe('organization and project discovery CLI', () => { - it('uses saved user auth over environment and project credentials and returns JSON data', async t => { - let { client, requests } = await createCloudServer(t, ({ url }) => { - if (url.startsWith('/api/sdk/projects')) { - return { - body: { - projects, - pagination: { total: 8, hasMore: true }, + it('shows names, available counts, token role, and the next project page', async t => { + let { client } = await createCloudServer(t, ({ url }) => { + let query = new URL(url, 'http://fixture').searchParams; + let isProjects = url.includes('/projects'); + return { + body: { + ...(isProjects ? { projects } : { organizations }), + meta: { + total: isProjects ? 8 : 2, + limit: Number(query.get('limit')), + offset: Number(query.get('offset')), + hasMore: isProjects, }, - }; - } - return { body: { organizations } }; - }); - let workspace = await createWorkspace(t, { - config: { apiKey: 'vzt_project_credential' }, - accessToken: 'saved-user-credential', - }); - - let projectsResult = await runDiscoveryCLI( - workspace, - client.getBaseUrl(), - [ - '--json', - 'projects', - '--org', - 'pitstop', - '--limit', - '2', - '--offset', - '1', - ], - { VIZZLY_TOKEN: 'vzt_environment_credential' } - ); - let orgsResult = await runDiscoveryCLI(workspace, client.getBaseUrl(), [ - '--json', - 'orgs', - ]); - - assert.equal(projectsResult.code, 0, projectsResult.stderr); - assert.equal(orgsResult.code, 0, orgsResult.stderr); - assert.deepEqual( - requests.map(request => request.headers.authorization), - ['Bearer saved-user-credential', 'Bearer saved-user-credential'] - ); - assert.equal( - requests[0].url, - '/api/sdk/projects?organization=pitstop&limit=2&offset=1' - ); - assert.deepEqual(jsonData(projectsResult), { - projects: [ - { - id: 'project-1', - name: 'Frontend', - slug: 'frontend', - organizationName: 'Vizzly', - organizationSlug: 'vizzly', - buildCount: 100, - createdAt: '2026-01-01', - updatedAt: '2026-06-01', - }, - { - id: 'project-2', - name: 'Dashboard', - slug: 'dashboard', - organizationName: 'PitStop', - organizationSlug: 'pitstop', - buildCount: 50, - createdAt: '2026-02-01', - updatedAt: '2026-06-01', - }, - ], - pagination: { total: 8, hasMore: true }, - }); - assert.deepEqual(jsonData(orgsResult), { - organizations: [ - { - id: 'org-1', - name: 'Vizzly', - slug: 'vizzly', - role: 'owner', - projectCount: 11, - createdAt: '2026-01-01', }, - { - id: 'org-2', - name: 'PitStop', - slug: 'pitstop', - role: 'token', - projectCount: 3, - createdAt: '2026-02-01', - }, - ], - count: 2, + }; }); - }); - - it('falls back to a configured API key and exits when no credential exists', async t => { - let { client, requests } = await createCloudServer(t, ({ url }) => ({ - body: url.includes('/projects') - ? { projects, pagination: { total: 2, hasMore: false } } - : { organizations }, - })); - let fallbackWorkspace = await createWorkspace(t, { - config: { apiKey: 'vzt_fallback_credential' }, - }); - - let fallbackResult = await runDiscoveryCLI( - fallbackWorkspace, - client.getBaseUrl(), - ['--json', 'projects'] - ); - - assert.equal(fallbackResult.code, 0, fallbackResult.stderr); - assert.equal( - requests[0].headers.authorization, - 'Bearer vzt_fallback_credential' - ); - - let noAuthWorkspace = await createWorkspace(t); - let noAuthResult = await runDiscoveryCLI( - noAuthWorkspace, - client.getBaseUrl(), - ['orgs'] - ); - - assert.equal(noAuthResult.code, 1); - assert.match( - `${noAuthResult.stdout}\n${noAuthResult.stderr}`, - /API token required/ - ); - assert.equal(requests.length, 1); - }); - - it('shows names, counts, token role, and the next project page in human output', async t => { - let { client } = await createCloudServer(t, ({ url }) => ({ - body: url.includes('/projects') - ? { projects, pagination: { total: 8, hasMore: true } } - : { organizations }, - })); let workspace = await createWorkspace(t, { accessToken: 'saved-user-credential', }); - - let projectsResult = await runDiscoveryCLI(workspace, client.getBaseUrl(), [ + let result = await runDiscoveryCLI(workspace, client.getBaseUrl(), [ 'projects', + '--limit', + '2', ]); - let orgsResult = await runDiscoveryCLI(workspace, client.getBaseUrl(), [ - 'orgs', - ]); - - assert.equal(projectsResult.code, 0, projectsResult.stderr); - assert.match(projectsResult.stdout, /Frontend/); - assert.match(projectsResult.stdout, /Dashboard/); - assert.match(projectsResult.stdout, /Showing:\s+2 of 8/); - assert.match(projectsResult.stdout, /--offset 2/); - assert.equal(orgsResult.code, 0, orgsResult.stderr); - assert.match(orgsResult.stdout, /Vizzly/); - assert.match(orgsResult.stdout, /PitStop/); - assert.match(orgsResult.stdout, /Count:\s+2/); - assert.match(orgsResult.stdout, /via token/); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /Frontend/); + assert.match(result.stdout, /Dashboard/); + assert.match(result.stdout, /Showing:\s+2 of 8/); + assert.match(result.stdout, /--offset 2/); + assert.match(result.stdout, /builds --project/); + let orgs = await runDiscoveryCLI(workspace, client.getBaseUrl(), ['orgs']); + assert.equal(orgs.code, 0, orgs.stderr); + assert.match(orgs.stdout, /Vizzly/); + assert.match(orgs.stdout, /PitStop/); + assert.match(orgs.stdout, /Count:\s+2/); + assert.match(orgs.stdout, /via token/); }); - it('reports empty project and organization lists clearly', async t => { - let { client } = await createCloudServer(t, ({ url }) => ({ - body: url.includes('/projects') - ? { projects: [], pagination: { total: 0, hasMore: false } } - : { organizations: [] }, - })); + it('reports empty accessible lists and distinguishes an absent login', async t => { + let { client, requests } = await createCloudServer(t, ({ url }) => { + let query = new URL(url, 'http://fixture').searchParams; + return { + body: { + ...(url.includes('/projects') + ? { projects: [] } + : { organizations: [] }), + meta: { + total: 0, + limit: Number(query.get('limit')), + offset: Number(query.get('offset')), + hasMore: false, + }, + }, + }; + }); let workspace = await createWorkspace(t, { accessToken: 'saved-user-credential', }); - - let projectsResult = await runDiscoveryCLI(workspace, client.getBaseUrl(), [ - 'projects', - ]); - let orgsResult = await runDiscoveryCLI(workspace, client.getBaseUrl(), [ - 'orgs', - ]); - - assert.equal(projectsResult.code, 0, projectsResult.stderr); - assert.match(projectsResult.stdout, /No projects found/); - assert.equal(orgsResult.code, 0, orgsResult.stderr); - assert.match(orgsResult.stdout, /No organizations found/); + for (let command of ['projects', 'orgs']) { + let result = await runDiscoveryCLI(workspace, client.getBaseUrl(), [ + command, + ]); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /No (projects|organizations) found/); + } + let noAuth = await createWorkspace(t); + let result = await runDiscoveryCLI(noAuth, client.getBaseUrl(), ['orgs']); + assert.equal(result.code, 1); + assert.match(result.stderr, /Login required.*vizzly login/); + assert.equal(requests.length, 2); }); }); diff --git a/tests/cli/projects/project-picker.test.js b/tests/cli/projects/project-picker.test.js new file mode 100644 index 00000000..1a99792a --- /dev/null +++ b/tests/cli/projects/project-picker.test.js @@ -0,0 +1,129 @@ +import assert from 'node:assert/strict'; +import { once } from 'node:events'; +import { mkdir, readdir, readFile, writeFile } from 'node:fs/promises'; +import http from 'node:http'; +import path from 'node:path'; +import { + terminalFixture as fixture, + terminalTest as ptyTest, + runTerminal as wizard, +} from '../../helpers/terminal-runner.js'; + +async function projectPickerFixture(t) { + let cwd = await fixture(t); + let calls = []; + let projects = Array.from({ length: 11 }, (_, index) => ({ + id: `project-${index}`, + name: `App ${index}`, + slug: 'web', + organizationSlug: index === 10 ? 'studio' : `team-${index}`, + })); + let server = http.createServer(async (request, response) => { + for await (let _chunk of request) { + /* consume the external request */ + } + calls.push({ method: request.method, url: request.url }); + let url = new URL(request.url, 'http://fixture'); + response.setHeader('Content-Type', 'application/json'); + if (url.pathname === '/api/sdk/projects') { + let offset = Number(url.searchParams.get('offset')); + response.end( + JSON.stringify({ + projects: projects.slice(offset, offset + 10), + meta: { total: 11, limit: 10, offset, hasMore: offset === 0 }, + }) + ); + } else if (url.pathname === '/api/cli/web/link-token') { + assert.equal(request.headers['x-organization'], 'studio'); + response.end( + JSON.stringify({ + organization: { slug: 'studio' }, + project: { slug: 'web' }, + token: { + id: 'token-1', + token: 'vzt_picker_secret', + token_prefix: 'vzt_fixture', + }, + }) + ); + } else { + response.statusCode = 404; + response.end(JSON.stringify({ error: 'Unexpected fixture request' })); + } + }); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + t.after(() => new Promise(resolve => server.close(resolve))); + await mkdir(path.join(cwd, 'user-state')); + await writeFile( + path.join(cwd, 'user-state', 'config.json'), + JSON.stringify({ + auth: { + accessToken: 'fixture-picker-user', + expiresAt: '2999-01-01T00:00:00Z', + }, + }) + ); + let environment = { + VIZZLY_API_URL: `http://127.0.0.1:${server.address().port}`, + VIZZLY_NO_UPDATE_NOTIFIER: '1', + }; + return { cwd, calls, environment }; +} + +ptyTest( + 'project link reaches another page and binds the qualified selection in a real terminal', + { timeout: 30000 }, + async t => { + let { cwd, calls, environment } = await projectPickerFixture(t); + let result = await wizard( + t, + cwd, + [ + ['Choose project number or action', 'n\n'], + ['Choose project number or action', '1\n'], + ], + environment, + ['project', 'link'] + ); + assert.equal(result.code, 0, result.output); + assert.match(result.output, /Linked studio\/web/); + assert.doesNotMatch(result.output, /vzt_picker_secret/); + assert.deepEqual( + calls.map(call => call.url), + [ + '/api/sdk/projects?limit=10&offset=0', + '/api/sdk/projects?limit=10&offset=10', + '/api/cli/web/link-token', + ] + ); + let { default: config } = await import(path.join(cwd, 'vizzly.config.js')); + assert.deepEqual(config.project, { + organizationSlug: 'studio', + projectSlug: 'web', + }); + } +); + +ptyTest( + 'cancelling the project picker leaves credentials and checkout untouched', + { timeout: 30000 }, + async t => { + for (let answer of ['q\n', '\x03', '\x04']) { + let { cwd, calls, environment } = await projectPickerFixture(t); + let filename = path.join(cwd, 'user-state', 'config.json'); + let before = await readFile(filename, 'utf8'); + let result = await wizard( + t, + cwd, + [['Choose project number or action', answer]], + environment, + ['project', 'link'] + ); + assert.match(result.output, /cancelled/); + assert.deepEqual(await readdir(cwd), ['user-state']); + assert.equal(await readFile(filename, 'utf8'), before); + assert.ok(calls.every(call => call.method === 'GET')); + } + } +); diff --git a/tests/cli/review/review-cli.test.js b/tests/cli/review/review-cli.test.js index 1cb2b938..69d1cc7a 100644 --- a/tests/cli/review/review-cli.test.js +++ b/tests/cli/review/review-cli.test.js @@ -1,5 +1,5 @@ import assert from 'node:assert/strict'; -import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { createServer } from 'node:http'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -58,6 +58,8 @@ function reviewResponse(state) { function createReviewState() { return { + comments: [], + commentStatus: 200, annotation: null, buildId: 'build-1', comparisonId: 'comparison-1', @@ -144,6 +146,22 @@ async function withReviewApiServer(callback) { return; } + if ( + req.method === 'POST' && + req.url === `/api/sdk/builds/${state.buildId}/comments` + ) { + let body = await readJson(req); + state.comments.push(body); + sendJson( + res, + state.commentStatus, + state.commentStatus === 200 + ? { comment: { id: 'comment-1', ...body } } + : { error: 'Comment service unavailable' } + ); + return; + } + if (req.method === 'POST' && req.url === decisionPath) { await handleDecision(req, res, state); return; @@ -290,4 +308,81 @@ describe('commands/review CLI', () => { assert.strictEqual(state.history.length, 3); }); }); + + it('prints approval annotations and human comments, and reports comment failures without replay', async t => { + await withReviewApiServer(async ({ apiUrl, state }) => { + let { cwd, vizzlyHome } = createWorkspace(); + t.after(() => rmSync(cwd, { recursive: true, force: true })); + let env = { VIZZLY_API_URL: apiUrl, VIZZLY_HOME: vizzlyHome }; + let approved = await runCLI( + [ + '--no-color', + 'approve', + state.comparisonId, + '--comment', + 'Ready to ship', + ], + { cwd, env } + ); + assert.equal(approved.code, 0, approved.stderr); + assert.match(approved.stdout + approved.stderr, /approved/); + assert.match(approved.stdout + approved.stderr, /Ready to ship/); + let commented = await runCLI( + [ + '--no-color', + 'comment', + state.buildId, + 'Looks good', + '--type', + 'approval', + ], + { cwd, env } + ); + assert.equal(commented.code, 0, commented.stderr); + assert.match(commented.stdout + commented.stderr, /Comment added/); + assert.match(commented.stdout + commented.stderr, /Looks good/); + assert.deepEqual(state.comments, [ + { content: 'Looks good', type: 'approval' }, + ]); + state.commentStatus = 503; + let failed = await runCLI( + ['--json', 'comment', state.buildId, 'Follow up'], + { cwd, env } + ); + assert.equal(failed.code, 1); + assert.equal(parseResult(failed).created, false); + assert.match( + parseResult(failed).error.message, + /Comment service unavailable/ + ); + assert.equal(state.comments.length, 2); + assert.equal(state.refreshRequests, 0); + }); + }); + + it('rejects missing reasons, blank messages, and missing login before making review changes', async t => { + await withReviewApiServer(async ({ apiUrl, state }) => { + let { cwd, vizzlyHome } = createWorkspace(); + t.after(() => rmSync(cwd, { recursive: true, force: true })); + let env = { VIZZLY_API_URL: apiUrl, VIZZLY_HOME: vizzlyHome }; + for (let args of [ + ['reject', state.comparisonId], + ['comment', state.buildId, ' '], + ]) { + let result = await runCLI(['--no-color', ...args], { cwd, env }); + assert.equal(result.code, 1); + assert.match(result.stderr, /reason|message.*required/i); + } + rmSync(join(vizzlyHome, 'config.json')); + let result = await runCLI(['--no-color', 'approve', state.comparisonId], { + cwd, + env, + }); + assert.equal(result.code, 1); + assert.match(result.stderr, /User login required/); + assert.equal(state.decisionRequests.length, 0); + assert.equal(state.comments.length, 0); + assert.equal(state.refreshRequests, 0); + }); + }); }); diff --git a/tests/cli/root-help.test.js b/tests/cli/root-help.test.js index cc39fce8..702e59a2 100644 --- a/tests/cli/root-help.test.js +++ b/tests/cli/root-help.test.js @@ -9,11 +9,11 @@ describe('cli root help', () => { assert.strictEqual(result.code, 0); assert.match( result.stdout, - /Setup[\s\S]*project\s+Manage the project linked to this local checkout/ + /Setup[\s\S]*project\s+Show or choose the upload destination/ ); assert.doesNotMatch( result.stdout, - /Plugins[\s\S]*project\s+Manage the project linked to this local checkout/ + /Plugins[\s\S]*project\s+Show or choose the upload destination/ ); }); }); diff --git a/tests/cli/runs/run-cli.test.js b/tests/cli/runs/run-cli.test.js index 24f19368..3fa6750b 100644 --- a/tests/cli/runs/run-cli.test.js +++ b/tests/cli/runs/run-cli.test.js @@ -250,7 +250,13 @@ describe('commands/run CLI', () => { assert.strictEqual(payload.data.status, 'completed'); assert.strictEqual(payload.data.buildId, 'build-123'); assert.strictEqual(payload.data.comparisons.total, 0); - assert.equal(requests[0].json.build.metadata, undefined); + assert.equal( + requests.find( + request => + request.method === 'POST' && request.url === '/api/sdk/builds' + ).json.build.metadata, + undefined + ); assert.strictEqual( payload.data.contextCommand, 'vizzly api schema sdk.getBuildContext --json' @@ -258,11 +264,11 @@ describe('commands/run CLI', () => { assert.deepStrictEqual( requests.map(request => `${request.method} ${request.url}`), [ + 'GET /api/sdk/token/context', 'POST /api/sdk/builds', 'GET /api/sdk/builds/build-123', 'PUT /api/sdk/builds/build-123/status', 'GET /api/sdk/builds/build-123', - 'GET /api/sdk/token/context', ] ); }); @@ -310,7 +316,10 @@ describe('commands/run CLI', () => { ); assert.equal(result.code, 0, result.stderr + result.stdout); assert.deepEqual( - requests[0].json.build.metadata.comparison, + requests.find( + request => + request.method === 'POST' && request.url === '/api/sdk/builds' + ).json.build.metadata.comparison, scenario.expected ); }); diff --git a/tests/cli/setup/init-guided.test.js b/tests/cli/setup/init-guided.test.js index e5075a32..8556c10e 100644 --- a/tests/cli/setup/init-guided.test.js +++ b/tests/cli/setup/init-guided.test.js @@ -1,140 +1,27 @@ import assert from 'node:assert/strict'; import { once } from 'node:events'; -import { constants } from 'node:fs'; import { access, chmod, cp, mkdir, - mkdtemp, readdir, readFile, - realpath, rm, symlink, writeFile, } from 'node:fs/promises'; import http from 'node:http'; -import { createRequire } from 'node:module'; -import { tmpdir } from 'node:os'; import path from 'node:path'; -import { after, test } from 'node:test'; import { fileURLToPath } from 'node:url'; +import { + terminalFixture as fixture, + terminalTest as ptyTest, + runTerminal as wizard, +} from '../../helpers/terminal-runner.js'; let repository = fileURLToPath(new URL('../../..', import.meta.url)); -let cli = path.join(repository, 'src', 'adapters', 'cli', 'cli.js'); -// Use tui-driver's optional PTY backend directly so tests wait on output events -// instead of the visual driver's screen-stability polling. -let pty; -let ptyFixture; -try { - let resolve = createRequire(import.meta.resolve('tui-driver')); - let entry = resolve.resolve('node-pty'); - if (process.platform === 'darwin') { - let directory = path.dirname(path.dirname(entry)); - let helper = path.join( - 'prebuilds', - `darwin-${process.arch}`, - 'spawn-helper' - ); - try { - await access(path.join(directory, helper), constants.X_OK); - } catch (error) { - if (error.code === 'EACCES') { - // Some package caches lose this helper's executable bit. Repair an - // isolated dependency copy; tests must not modify installed modules. - ptyFixture = await mkdtemp(path.join(tmpdir(), 'vizzly-pty-backend-')); - let copy = path.join(ptyFixture, 'node_modules', 'node-pty'); - await cp(directory, copy, { recursive: true }); - await chmod(path.join(copy, helper), 0o755); - entry = path.join(copy, 'lib', 'index.js'); - } else if (error.code !== 'ENOENT') { - throw error; - } - } - } - pty = (await import(entry)).default; -} catch { - /* The optional native test dependency is unavailable. */ -} -after(async () => { - if (ptyFixture) await rm(ptyFixture, { recursive: true, force: true }); -}); -let ptyTest = pty ? test : test.skip; - -async function fixture(t) { - let cwd = await realpath( - await mkdtemp(path.join(tmpdir(), 'vizzly-guided-')) - ); - t.after(() => rm(cwd, { recursive: true, force: true })); - return cwd; -} - -function wizard( - t, - cwd, - responses, - environment = {}, - args = ['init'], - entry = cli, - observe = () => {} -) { - let env = { ...process.env, TERM: 'dumb', COLUMNS: '40', NO_COLOR: '1' }; - for (let name of Object.keys(env)) { - if ( - name.startsWith('VIZZLY_') || - [ - 'CI', - 'NODE_V8_COVERAGE', - 'FORCE_COLOR', - 'NO_UPDATE_NOTIFIER', - 'NODE_OPTIONS', - ].includes(name) - ) - delete env[name]; - } - env.VIZZLY_HOME = path.join(cwd, 'user-state'); - env.VIZZLY_DISABLE_KEYCHAIN = 'true'; - Object.assign(env, environment); - let child = pty.spawn(process.execPath, [entry, ...args, '--no-color'], { - cwd, - env, - name: 'dumb', - cols: 40, - rows: 24, - }); - let exited = false; - t.after(() => { - if (!exited) child.kill(); - }); - return new Promise((resolve, reject) => { - let output = ''; - let pending = ''; - let index = 0; - let receive = chunk => { - let value = chunk.toString(); - output += value; - pending += value; - observe(output); - while (index < responses.length) { - let [marker, answer] = responses[index]; - let position = pending.indexOf(marker); - if (position < 0) break; - pending = pending.slice(position + marker.length); - index++; - child.write(answer); - } - }; - child.onData(receive); - child.onExit(({ exitCode: code }) => { - exited = true; - if (index !== responses.length) - reject(new Error(`Wizard ended before prompt ${index}: ${output}`)); - else resolve({ code, output }); - }); - }); -} - +let cli = path.join(repository, 'bin', 'vizzly.js'); let noAgents = ['Add the Vizzly skill', 'n\n']; let noCloud = ['Connect this repository', 'n\n']; let apply = ['Apply this setup?', 'y\n']; @@ -782,6 +669,22 @@ ptyTest( } calls.push(request.url); response.setHeader('Content-Type', 'application/json'); + if (request.url.startsWith('/api/sdk/projects')) { + response.end( + JSON.stringify({ + projects: [ + { + id: 'p-1', + name: 'Fixture app', + slug: 'fixture-app', + organizationSlug: 'fixture-org', + }, + ], + meta: { total: 1, limit: 10, offset: 0, hasMore: false }, + }) + ); + return; + } response.end( JSON.stringify({ organization: { slug: 'fixture-org' }, @@ -809,13 +712,16 @@ ptyTest( [ noAgents, ['Connect this repository', 'y\n'], - ['Project (organization/project)', 'fixture-org/fixture-app\n'], apply, + ['Choose project number or action', '1\n'], ], { VIZZLY_API_URL: apiUrl } ); assert.equal(result.code, 0, result.output); - assert.deepEqual(calls, ['/api/cli/fixture-app/link-token']); + assert.deepEqual(calls, [ + '/api/sdk/projects?limit=10&offset=0', + '/api/cli/fixture-app/link-token', + ]); let source = await readFile(path.join(cwd, 'vizzly.config.js'), 'utf8'); assert.doesNotMatch(source, /fixture-private-token/); let { default: config } = await import(path.join(cwd, 'vizzly.config.js')); diff --git a/tests/cli/uploads/upload-cli.test.js b/tests/cli/uploads/upload-cli.test.js index b8012d1a..f348d56c 100644 --- a/tests/cli/uploads/upload-cli.test.js +++ b/tests/cli/uploads/upload-cli.test.js @@ -86,36 +86,44 @@ describe('commands/upload CLI', () => { assert.deepStrictEqual( requests.map(request => `${request.method} ${request.url}`), [ + 'GET /api/sdk/token/context', 'POST /api/sdk/builds', 'POST /api/sdk/check-shas', 'POST /api/sdk/upload', 'PUT /api/sdk/builds/build-123/status', - 'GET /api/sdk/token/context', ] ); - assert.strictEqual(requests[0].json.build.name, 'CLI upload'); - assert.strictEqual(requests[0].json.build.branch, 'main'); - assert.strictEqual(requests[0].json.build.commit_sha, 'abc123'); - assert.strictEqual( - requests[0].json.build.commit_message, - 'Upload screenshots' + let build = requests.find( + request => + request.method === 'POST' && request.url === '/api/sdk/builds' + ).json.build; + assert.strictEqual(build.name, 'CLI upload'); + assert.strictEqual(build.branch, 'main'); + assert.strictEqual(build.commit_sha, 'abc123'); + assert.strictEqual(build.commit_message, 'Upload screenshots'); + assert.strictEqual(build.environment, 'test'); + assert.equal(build.metadata?.comparison, undefined); + let shaRequest = requests.find( + request => request.url === '/api/sdk/check-shas' + ); + let uploadRequest = requests.find( + request => request.url === '/api/sdk/upload' ); - assert.strictEqual(requests[0].json.build.environment, 'test'); - assert.equal(requests[0].json.build.metadata?.comparison, undefined); + let finalRequest = requests.find(request => request.method === 'PUT'); // Keep the released preflight name; the additive filename identifies multipart captures. assert.deepStrictEqual( - requests[1].json.screenshots.map(({ name }) => name).sort(), + shaRequest.json.screenshots.map(({ name }) => name).sort(), ['home-chrome', 'settings-firefox'] ); assert.deepStrictEqual( - requests[1].json.screenshots.map(({ filename }) => filename).sort(), + shaRequest.json.screenshots.map(({ filename }) => filename).sort(), ['home-chrome.png', 'settings-firefox.png'] ); - assert.match(requests[2].contentType, /multipart\/form-data/); - assert.ok(requests[2].body.includes('home-chrome.png')); - assert.ok(requests[2].body.includes('settings-firefox.png')); - assert.strictEqual(requests[3].json.status, 'completed'); + assert.match(uploadRequest.contentType, /multipart\/form-data/); + assert.ok(uploadRequest.body.includes('home-chrome.png')); + assert.ok(uploadRequest.body.includes('settings-firefox.png')); + assert.strictEqual(finalRequest.json.status, 'completed'); }); }); @@ -137,9 +145,15 @@ describe('commands/upload CLI', () => { } ); assert.equal(result.code, 0, result.stderr + result.stdout); - assert.deepEqual(requests[0].json.build.metadata.comparison, { - threshold: 0, - }); + assert.deepEqual( + requests.find( + request => + request.method === 'POST' && request.url === '/api/sdk/builds' + ).json.build.metadata.comparison, + { + threshold: 0, + } + ); }); }); }); diff --git a/tests/cli/uploads/upload-errors.test.js b/tests/cli/uploads/upload-errors.test.js index 16874f33..63e3f8f5 100644 --- a/tests/cli/uploads/upload-errors.test.js +++ b/tests/cli/uploads/upload-errors.test.js @@ -24,7 +24,9 @@ for (let scenario of [ for await (let chunk of req) chunks.push(chunk); let body = chunks.length ? JSON.parse(Buffer.concat(chunks)) : {}; let result = {}; - if (req.url === '/api/sdk/builds' && req.method === 'POST') { + if (req.url === '/api/sdk/token/context') { + result = { organization: { slug: 'acme' }, project: { slug: 'web' } }; + } else if (req.url === '/api/sdk/builds' && req.method === 'POST') { result = { id: 'build-123' }; } else if ( req.url === '/api/sdk/builds/build-123' && diff --git a/tests/helpers/cli-runner.js b/tests/helpers/cli-runner.js index d1ed7c79..335c4e6f 100644 --- a/tests/helpers/cli-runner.js +++ b/tests/helpers/cli-runner.js @@ -45,7 +45,12 @@ function createCleanEnv(overrides = {}) { * @returns {Promise<{code: number, stdout: string, stderr: string}>} */ export async function runCLI(args, options = {}) { - let { cwd, env = {}, timeout = process.env.CI ? 15000 : 10000 } = options; + let { + cwd, + env = {}, + timeout = process.env.CI ? 15000 : 10000, + entrypoint = CLI_PATH, + } = options; // Create temp directory if not provided if (!cwd) { @@ -62,7 +67,7 @@ export async function runCLI(args, options = {}) { }); return new Promise((resolve, reject) => { - let child = spawn('node', [CLI_PATH, ...args], { + let child = spawn('node', [entrypoint, ...args], { stdio: ['pipe', 'pipe', 'pipe'], env: cleanEnv, cwd, diff --git a/tests/helpers/terminal-runner.js b/tests/helpers/terminal-runner.js new file mode 100644 index 00000000..8b6a7dd8 --- /dev/null +++ b/tests/helpers/terminal-runner.js @@ -0,0 +1,120 @@ +import { constants } from 'node:fs'; +import { access, chmod, cp, mkdtemp, realpath, rm } from 'node:fs/promises'; +import { createRequire } from 'node:module'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { after, test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +let cli = fileURLToPath(new URL('../../bin/vizzly.js', import.meta.url)); +// Use tui-driver's optional PTY backend directly so tests wait on output events +// instead of the visual driver's screen-stability polling. +let pty; +let ptyFixture; +try { + let resolve = createRequire(import.meta.resolve('tui-driver')); + let entry = resolve.resolve('node-pty'); + if (process.platform === 'darwin') { + let directory = path.dirname(path.dirname(entry)); + let helper = path.join( + 'prebuilds', + `darwin-${process.arch}`, + 'spawn-helper' + ); + try { + await access(path.join(directory, helper), constants.X_OK); + } catch (error) { + if (error.code === 'EACCES') { + // Some package caches lose this helper's executable bit. Repair an + // isolated dependency copy; tests must not modify installed modules. + ptyFixture = await mkdtemp(path.join(tmpdir(), 'vizzly-pty-backend-')); + let copy = path.join(ptyFixture, 'node_modules', 'node-pty'); + await cp(directory, copy, { recursive: true }); + await chmod(path.join(copy, helper), 0o755); + entry = path.join(copy, 'lib', 'index.js'); + } else if (error.code !== 'ENOENT') { + throw error; + } + } + } + pty = (await import(entry)).default; +} catch { + /* The optional native test dependency is unavailable. */ +} +after(async () => { + if (ptyFixture) await rm(ptyFixture, { recursive: true, force: true }); +}); +export let terminalTest = pty ? test : test.skip; + +export async function terminalFixture(t) { + let cwd = await realpath( + await mkdtemp(path.join(tmpdir(), 'vizzly-terminal-')) + ); + t.after(() => rm(cwd, { recursive: true, force: true })); + return cwd; +} + +export function runTerminal( + t, + cwd, + responses, + environment = {}, + args = ['init'], + entry = cli, + observe = () => {} +) { + let env = { ...process.env, TERM: 'dumb', COLUMNS: '40', NO_COLOR: '1' }; + for (let name of Object.keys(env)) { + if ( + name.startsWith('VIZZLY_') || + [ + 'CI', + 'NODE_V8_COVERAGE', + 'FORCE_COLOR', + 'NO_UPDATE_NOTIFIER', + 'NODE_OPTIONS', + ].includes(name) + ) + delete env[name]; + } + env.VIZZLY_HOME = path.join(cwd, 'user-state'); + env.VIZZLY_DISABLE_KEYCHAIN = 'true'; + Object.assign(env, environment); + let child = pty.spawn(process.execPath, [entry, ...args, '--no-color'], { + cwd, + env, + name: 'dumb', + cols: 40, + rows: 24, + }); + let exited = false; + t.after(() => { + if (!exited) child.kill(); + }); + return new Promise((resolve, reject) => { + let output = ''; + let pending = ''; + let index = 0; + let receive = chunk => { + let value = chunk.toString(); + output += value; + pending += value; + observe(output); + while (index < responses.length) { + let [marker, answer] = responses[index]; + let position = pending.indexOf(marker); + if (position < 0) break; + pending = pending.slice(position + marker.length); + index++; + child.write(answer); + } + }; + child.onData(receive); + child.onExit(({ exitCode: code }) => { + exited = true; + if (index !== responses.length) + reject(new Error(`Terminal ended before prompt ${index}: ${output}`)); + else resolve({ code, output }); + }); + }); +}