From 78afb54bd55864070806fcbd556c02e875d2c259 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Sun, 8 Feb 2026 17:49:42 -0600 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20Fix=20orgs/projects=20comman?= =?UTF-8?q?ds=20using=20project-scoped=20token=20instead=20of=20user=20aut?= =?UTF-8?q?h?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `orgs` and `projects` commands used `loadConfig()` which picks up project-scoped tokens from `vizzly project link` mappings. These tokens are scoped to a single org, so the API only returned data for that org. Both commands now prefer the user auth token (from `vizzly login`) since listing orgs and projects are user-level operations. Falls back to config.apiKey when no user auth token exists. Also fixes CLAUDE.md incorrectly referencing vitest as the test runner — the CLI uses Node's built-in test runner (`node --test`). --- CLAUDE.md | 8 ++++---- src/commands/orgs.js | 8 ++++++-- src/commands/projects.js | 8 ++++++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f2a6450..45093e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ npm run clean # Remove dist directory npm run compile # Babel compile src to dist npm run types # Generate TypeScript declarations -npm test # Run all tests once (vitest) +npm test # Run all tests once (node --test) npm test:watch # Run tests in watch mode npm run test:reporter # Run Playwright visual regression tests for reporter UI npm run test:reporter:visual # Self-test: run reporter tests with Vizzly @@ -64,8 +64,8 @@ npm run format:check # Prettier check ### Testing Specific Files/Patterns ```bash -npx vitest run tests/unit/config-loader.spec.js # Single test file -npx vitest run tests/services/ # Directory +node --test tests/commands/builds.test.js # Single test file +node --test $(find tests/services -name '*.test.js') # Directory ``` ### Reporter Development @@ -198,7 +198,7 @@ The package provides multiple entry points for different use cases: ## Testing -- **Vitest** for unit/integration tests (`npm test`) +- **Node test runner** for unit/integration tests (`npm test`) - **Playwright** for reporter UI visual tests (`npm run test:reporter`) - Coverage thresholds: 75% lines/functions, 70% branches - Tests mirror `src/` structure in `tests/` diff --git a/src/commands/orgs.js b/src/commands/orgs.js index 2f1a2ba..85923e4 100644 --- a/src/commands/orgs.js +++ b/src/commands/orgs.js @@ -5,6 +5,7 @@ import { createApiClient } from '../api/client.js'; import { loadConfig } from '../utils/config-loader.js'; import { getApiUrl } from '../utils/environment-config.js'; +import { getAccessToken } from '../utils/global-config.js'; import * as output from '../utils/output.js'; /** @@ -22,7 +23,10 @@ export async function orgsCommand(_options = {}, globalOptions = {}) { try { let config = await loadConfig(globalOptions.config, globalOptions); - if (!config.apiKey) { + // Prefer user auth token for orgs — project-scoped tokens only see one org + let token = (await getAccessToken()) || config.apiKey; + + if (!token) { output.error( 'API token required. Use --token, set VIZZLY_TOKEN, or run "vizzly login"' ); @@ -32,7 +36,7 @@ export async function orgsCommand(_options = {}, globalOptions = {}) { let client = createApiClient({ baseUrl: config.apiUrl || getApiUrl(), - token: config.apiKey, + token, }); output.startSpinner('Fetching organizations...'); diff --git a/src/commands/projects.js b/src/commands/projects.js index b091512..6364969 100644 --- a/src/commands/projects.js +++ b/src/commands/projects.js @@ -5,6 +5,7 @@ import { createApiClient } from '../api/client.js'; import { loadConfig } from '../utils/config-loader.js'; import { getApiUrl } from '../utils/environment-config.js'; +import { getAccessToken } from '../utils/global-config.js'; import * as output from '../utils/output.js'; /** @@ -22,7 +23,10 @@ export async function projectsCommand(options = {}, globalOptions = {}) { try { let config = await loadConfig(globalOptions.config, globalOptions); - if (!config.apiKey) { + // Prefer user auth token for listing projects — project-scoped tokens only see one org + let token = (await getAccessToken()) || config.apiKey; + + if (!token) { output.error( 'API token required. Use --token, set VIZZLY_TOKEN, or run "vizzly login"' ); @@ -32,7 +36,7 @@ export async function projectsCommand(options = {}, globalOptions = {}) { let client = createApiClient({ baseUrl: config.apiUrl || getApiUrl(), - token: config.apiKey, + token, }); // Build query params From f772e01053f536e7c9141618fd6ef977044bbfdb Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Sun, 8 Feb 2026 18:00:50 -0600 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=A7=AA=20Address=20code=20review:=20e?= =?UTF-8?q?xpiration=20check,=20DI,=20and=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - getAccessToken() now validates token expiration before returning, preventing cryptic API errors from expired tokens - Refactor orgs/projects commands to accept deps for testability, matching the existing pattern used by builds command - Add 17 tests covering token priority, fallback behavior, JSON/human output, empty states, pagination, and org filtering --- src/commands/orgs.js | 34 ++-- src/commands/projects.js | 34 ++-- src/utils/global-config.js | 7 +- tests/commands/orgs.test.js | 224 ++++++++++++++++++++++++++ tests/commands/projects.test.js | 273 ++++++++++++++++++++++++++++++++ 5 files changed, 552 insertions(+), 20 deletions(-) create mode 100644 tests/commands/orgs.test.js create mode 100644 tests/commands/projects.test.js diff --git a/src/commands/orgs.js b/src/commands/orgs.js index 85923e4..0503baa 100644 --- a/src/commands/orgs.js +++ b/src/commands/orgs.js @@ -2,18 +2,32 @@ * Organizations command - List organizations the user has access to */ -import { createApiClient } from '../api/client.js'; -import { loadConfig } from '../utils/config-loader.js'; -import { getApiUrl } from '../utils/environment-config.js'; -import { getAccessToken } from '../utils/global-config.js'; -import * as output from '../utils/output.js'; +import { createApiClient as defaultCreateApiClient } from '../api/client.js'; +import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; +import { getApiUrl as defaultGetApiUrl } from '../utils/environment-config.js'; +import { getAccessToken as defaultGetAccessToken } from '../utils/global-config.js'; +import * as defaultOutput from '../utils/output.js'; /** * Organizations command implementation * @param {Object} options - Command options * @param {Object} globalOptions - Global CLI options + * @param {Object} deps - Dependencies for testing */ -export async function orgsCommand(_options = {}, globalOptions = {}) { +export async function orgsCommand( + _options = {}, + globalOptions = {}, + deps = {} +) { + let { + loadConfig = defaultLoadConfig, + createApiClient = defaultCreateApiClient, + getApiUrl = defaultGetApiUrl, + getAccessToken = defaultGetAccessToken, + output = defaultOutput, + exit = code => process.exit(code), + } = deps; + output.configure({ json: globalOptions.json, verbose: globalOptions.verbose, @@ -23,7 +37,8 @@ export async function orgsCommand(_options = {}, globalOptions = {}) { try { let config = await loadConfig(globalOptions.config, globalOptions); - // Prefer user auth token for orgs — project-scoped tokens only see one org + // 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 = (await getAccessToken()) || config.apiKey; if (!token) { @@ -31,7 +46,8 @@ export async function orgsCommand(_options = {}, globalOptions = {}) { 'API token required. Use --token, set VIZZLY_TOKEN, or run "vizzly login"' ); output.cleanup(); - process.exit(1); + exit(1); + return; } let client = createApiClient({ @@ -87,7 +103,7 @@ export async function orgsCommand(_options = {}, globalOptions = {}) { output.stopSpinner(); output.error('Failed to fetch organizations', error); output.cleanup(); - process.exit(1); + exit(1); } } diff --git a/src/commands/projects.js b/src/commands/projects.js index 6364969..683c1f3 100644 --- a/src/commands/projects.js +++ b/src/commands/projects.js @@ -2,18 +2,32 @@ * Projects command - List projects the user has access to */ -import { createApiClient } from '../api/client.js'; -import { loadConfig } from '../utils/config-loader.js'; -import { getApiUrl } from '../utils/environment-config.js'; -import { getAccessToken } from '../utils/global-config.js'; -import * as output from '../utils/output.js'; +import { createApiClient as defaultCreateApiClient } from '../api/client.js'; +import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; +import { getApiUrl as defaultGetApiUrl } from '../utils/environment-config.js'; +import { getAccessToken as defaultGetAccessToken } from '../utils/global-config.js'; +import * as defaultOutput from '../utils/output.js'; /** * Projects command implementation * @param {Object} options - Command options * @param {Object} globalOptions - Global CLI options + * @param {Object} deps - Dependencies for testing */ -export async function projectsCommand(options = {}, globalOptions = {}) { +export async function projectsCommand( + options = {}, + globalOptions = {}, + deps = {} +) { + let { + loadConfig = defaultLoadConfig, + createApiClient = defaultCreateApiClient, + getApiUrl = defaultGetApiUrl, + getAccessToken = defaultGetAccessToken, + output = defaultOutput, + exit = code => process.exit(code), + } = deps; + output.configure({ json: globalOptions.json, verbose: globalOptions.verbose, @@ -23,7 +37,8 @@ export async function projectsCommand(options = {}, globalOptions = {}) { try { let config = await loadConfig(globalOptions.config, globalOptions); - // Prefer user auth token for listing projects — project-scoped tokens only see one org + // 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 = (await getAccessToken()) || config.apiKey; if (!token) { @@ -31,7 +46,8 @@ export async function projectsCommand(options = {}, globalOptions = {}) { 'API token required. Use --token, set VIZZLY_TOKEN, or run "vizzly login"' ); output.cleanup(); - process.exit(1); + exit(1); + return; } let client = createApiClient({ @@ -109,7 +125,7 @@ export async function projectsCommand(options = {}, globalOptions = {}) { output.stopSpinner(); output.error('Failed to fetch projects', error); output.cleanup(); - process.exit(1); + exit(1); } } diff --git a/src/utils/global-config.js b/src/utils/global-config.js index 09455a7..87a5b02 100644 --- a/src/utils/global-config.js +++ b/src/utils/global-config.js @@ -195,11 +195,14 @@ export async function hasValidTokens() { } /** - * Get the access token from global config if available + * Get the access token from global config if valid and not expired * @returns {Promise} Access token or null */ export async function getAccessToken() { - const auth = await getAuthTokens(); + let valid = await hasValidTokens(); + if (!valid) return null; + + let auth = await getAuthTokens(); return auth?.accessToken || null; } diff --git a/tests/commands/orgs.test.js b/tests/commands/orgs.test.js new file mode 100644 index 0000000..1a69c00 --- /dev/null +++ b/tests/commands/orgs.test.js @@ -0,0 +1,224 @@ +import assert from 'node:assert'; +import { describe, it } from 'node:test'; +import { orgsCommand, validateOrgsOptions } from '../../src/commands/orgs.js'; + +/** + * Create mock output object that tracks calls + */ +function createMockOutput() { + let calls = []; + return { + calls, + configure: opts => calls.push({ method: 'configure', args: [opts] }), + error: (msg, err) => calls.push({ method: 'error', args: [msg, err] }), + startSpinner: msg => calls.push({ method: 'startSpinner', args: [msg] }), + stopSpinner: () => calls.push({ method: 'stopSpinner', args: [] }), + header: (cmd, mode) => calls.push({ method: 'header', args: [cmd, mode] }), + print: msg => calls.push({ method: 'print', args: [msg] }), + blank: () => calls.push({ method: 'blank', args: [] }), + hint: msg => calls.push({ method: 'hint', args: [msg] }), + labelValue: (label, value) => + calls.push({ method: 'labelValue', args: [label, value] }), + cleanup: () => calls.push({ method: 'cleanup', args: [] }), + data: obj => calls.push({ method: 'data', args: [obj] }), + getColors: () => ({ + bold: s => s, + dim: s => s, + }), + }; +} + +let mockOrgs = [ + { + id: 'org-1', + name: 'Vizzly', + slug: 'vizzly', + role: 'owner', + projectCount: 11, + created_at: '2024-01-01', + }, + { + id: 'org-2', + name: 'PitStop', + slug: 'pitstop', + role: 'member', + projectCount: 3, + created_at: '2024-02-01', + }, +]; + +describe('commands/orgs', () => { + describe('validateOrgsOptions', () => { + it('returns no errors', () => { + let errors = validateOrgsOptions({}); + assert.deepStrictEqual(errors, []); + }); + }); + + describe('orgsCommand', () => { + it('requires API token when no auth exists', async () => { + let output = createMockOutput(); + let exitCode = null; + + await orgsCommand( + {}, + {}, + { + loadConfig: async () => ({}), + getAccessToken: async () => null, + output, + exit: code => { + exitCode = code; + }, + } + ); + + assert.strictEqual(exitCode, 1); + assert.ok(output.calls.some(c => c.method === 'error')); + }); + + it('prefers user auth token over project token', async () => { + let output = createMockOutput(); + let capturedToken = null; + + await orgsCommand( + {}, + { json: true }, + { + loadConfig: async () => ({ + apiKey: 'project-token', + apiUrl: 'https://api.test', + }), + getAccessToken: async () => 'user-auth-token', + createApiClient: ({ token }) => { + capturedToken = token; + return { request: async () => ({ organizations: mockOrgs }) }; + }, + output, + exit: () => {}, + } + ); + + assert.strictEqual(capturedToken, 'user-auth-token'); + }); + + it('falls back to config.apiKey when no user auth token', async () => { + let output = createMockOutput(); + let capturedToken = null; + + await orgsCommand( + {}, + { json: true }, + { + loadConfig: async () => ({ + apiKey: 'env-token', + apiUrl: 'https://api.test', + }), + getAccessToken: async () => null, + createApiClient: ({ token }) => { + capturedToken = token; + return { request: async () => ({ organizations: mockOrgs }) }; + }, + output, + exit: () => {}, + } + ); + + assert.strictEqual(capturedToken, 'env-token'); + }); + + it('returns all orgs in JSON output', async () => { + let output = createMockOutput(); + + await orgsCommand( + {}, + { json: true }, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ organizations: mockOrgs }), + }), + output, + exit: () => {}, + } + ); + + let dataCall = output.calls.find(c => c.method === 'data'); + assert.ok(dataCall); + assert.strictEqual(dataCall.args[0].count, 2); + assert.strictEqual(dataCall.args[0].organizations.length, 2); + assert.strictEqual(dataCall.args[0].organizations[0].name, 'Vizzly'); + assert.strictEqual(dataCall.args[0].organizations[1].name, 'PitStop'); + }); + + it('displays orgs in human-readable format', async () => { + let output = createMockOutput(); + + await orgsCommand( + {}, + {}, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ organizations: mockOrgs }), + }), + output, + exit: () => {}, + } + ); + + let labelCall = output.calls.find( + c => c.method === 'labelValue' && c.args[0] === 'Count' + ); + assert.ok(labelCall); + assert.strictEqual(labelCall.args[1], '2'); + }); + + it('shows "via token" for token role', async () => { + let output = createMockOutput(); + let tokenOrg = [{ ...mockOrgs[0], role: 'token' }]; + + await orgsCommand( + {}, + {}, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ organizations: tokenOrg }), + }), + output, + exit: () => {}, + } + ); + + let printCalls = output.calls.filter(c => c.method === 'print'); + assert.ok(printCalls.some(c => c.args[0].includes('via token'))); + }); + + it('handles empty organizations', async () => { + let output = createMockOutput(); + + await orgsCommand( + {}, + {}, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ organizations: [] }), + }), + output, + exit: () => {}, + } + ); + + let printCalls = output.calls.filter(c => c.method === 'print'); + assert.ok( + printCalls.some(c => c.args[0].includes('No organizations found')) + ); + }); + }); +}); diff --git a/tests/commands/projects.test.js b/tests/commands/projects.test.js new file mode 100644 index 0000000..8fe049e --- /dev/null +++ b/tests/commands/projects.test.js @@ -0,0 +1,273 @@ +import assert from 'node:assert'; +import { describe, it } from 'node:test'; +import { + projectsCommand, + validateProjectsOptions, +} from '../../src/commands/projects.js'; + +/** + * Create mock output object that tracks calls + */ +function createMockOutput() { + let calls = []; + return { + calls, + configure: opts => calls.push({ method: 'configure', args: [opts] }), + error: (msg, err) => calls.push({ method: 'error', args: [msg, err] }), + startSpinner: msg => calls.push({ method: 'startSpinner', args: [msg] }), + stopSpinner: () => calls.push({ method: 'stopSpinner', args: [] }), + header: (cmd, mode) => calls.push({ method: 'header', args: [cmd, mode] }), + print: msg => calls.push({ method: 'print', args: [msg] }), + blank: () => calls.push({ method: 'blank', args: [] }), + hint: msg => calls.push({ method: 'hint', args: [msg] }), + labelValue: (label, value) => + calls.push({ method: 'labelValue', args: [label, value] }), + cleanup: () => calls.push({ method: 'cleanup', args: [] }), + data: obj => calls.push({ method: 'data', args: [obj] }), + getColors: () => ({ + bold: s => s, + dim: s => s, + }), + }; +} + +let mockProjects = [ + { + id: 'proj-1', + name: 'Frontend', + slug: 'frontend', + organizationName: 'Vizzly', + organizationSlug: 'vizzly', + buildCount: 100, + created_at: '2024-01-01', + updated_at: '2024-06-01', + }, + { + id: 'proj-2', + name: 'Dashboard', + slug: 'dashboard', + organizationName: 'PitStop', + organizationSlug: 'pitstop', + buildCount: 50, + created_at: '2024-02-01', + updated_at: '2024-06-01', + }, +]; + +describe('commands/projects', () => { + describe('validateProjectsOptions', () => { + it('returns no errors', () => { + let errors = validateProjectsOptions({}); + assert.deepStrictEqual(errors, []); + }); + }); + + describe('projectsCommand', () => { + it('requires API token when no auth exists', async () => { + let output = createMockOutput(); + let exitCode = null; + + await projectsCommand( + {}, + {}, + { + loadConfig: async () => ({}), + getAccessToken: async () => null, + output, + exit: code => { + exitCode = code; + }, + } + ); + + assert.strictEqual(exitCode, 1); + assert.ok(output.calls.some(c => c.method === 'error')); + }); + + it('prefers user auth token over project token', async () => { + let output = createMockOutput(); + let capturedToken = null; + + await projectsCommand( + {}, + { json: true }, + { + loadConfig: async () => ({ + apiKey: 'project-token', + apiUrl: 'https://api.test', + }), + getAccessToken: async () => 'user-auth-token', + createApiClient: ({ token }) => { + capturedToken = token; + return { + request: async () => ({ + projects: mockProjects, + pagination: { total: 2, hasMore: false }, + }), + }; + }, + output, + exit: () => {}, + } + ); + + assert.strictEqual(capturedToken, 'user-auth-token'); + }); + + it('falls back to config.apiKey when no user auth token', async () => { + let output = createMockOutput(); + let capturedToken = null; + + await projectsCommand( + {}, + { json: true }, + { + loadConfig: async () => ({ + apiKey: 'env-token', + apiUrl: 'https://api.test', + }), + getAccessToken: async () => null, + createApiClient: ({ token }) => { + capturedToken = token; + return { + request: async () => ({ + projects: mockProjects, + pagination: { total: 2, hasMore: false }, + }), + }; + }, + output, + exit: () => {}, + } + ); + + assert.strictEqual(capturedToken, 'env-token'); + }); + + it('returns all projects in JSON output', async () => { + let output = createMockOutput(); + + await projectsCommand( + {}, + { json: true }, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ + projects: mockProjects, + pagination: { total: 2, hasMore: false }, + }), + }), + output, + exit: () => {}, + } + ); + + let dataCall = output.calls.find(c => c.method === 'data'); + assert.ok(dataCall); + assert.strictEqual(dataCall.args[0].projects.length, 2); + assert.strictEqual(dataCall.args[0].projects[0].name, 'Frontend'); + assert.strictEqual( + dataCall.args[0].projects[1].organizationSlug, + 'pitstop' + ); + }); + + it('passes org filter as query param', async () => { + let output = createMockOutput(); + let capturedEndpoint = null; + + await projectsCommand( + { org: 'pitstop' }, + { json: true }, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async endpoint => { + capturedEndpoint = endpoint; + return { projects: [], pagination: { total: 0, hasMore: false } }; + }, + }), + output, + exit: () => {}, + } + ); + + assert.ok(capturedEndpoint.includes('organization=pitstop')); + }); + + it('displays projects in human-readable format', async () => { + let output = createMockOutput(); + + await projectsCommand( + {}, + {}, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ + projects: mockProjects, + pagination: { total: 2, hasMore: false }, + }), + }), + output, + exit: () => {}, + } + ); + + let labelCall = output.calls.find( + c => c.method === 'labelValue' && c.args[0] === 'Showing' + ); + assert.ok(labelCall); + assert.strictEqual(labelCall.args[1], '2 of 2'); + }); + + it('handles empty projects', async () => { + let output = createMockOutput(); + + await projectsCommand( + {}, + {}, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ projects: [], pagination: {} }), + }), + output, + exit: () => {}, + } + ); + + let printCalls = output.calls.filter(c => c.method === 'print'); + assert.ok(printCalls.some(c => c.args[0].includes('No projects found'))); + }); + + it('shows pagination hint when more results available', async () => { + let output = createMockOutput(); + + await projectsCommand( + {}, + {}, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + getAccessToken: async () => 'user-token', + createApiClient: () => ({ + request: async () => ({ + projects: mockProjects, + pagination: { total: 50, hasMore: true }, + }), + }), + output, + exit: () => {}, + } + ); + + let hintCalls = output.calls.filter(c => c.method === 'hint'); + assert.ok(hintCalls.some(c => c.args[0].includes('--offset'))); + }); + }); +});