diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 660df82..613889f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,7 @@ jobs: cache: 'npm' - run: npm ci + - run: npm run build - run: npm test env: CI: true diff --git a/src/commands/auth.test.ts b/src/commands/auth.test.ts index d65e6cc..3ccc990 100644 --- a/src/commands/auth.test.ts +++ b/src/commands/auth.test.ts @@ -165,7 +165,7 @@ describe('runConfigure', () => { const { capture, deps } = makeCapture(); // Pre-existing dev profile — re-running configure interactively must keep it // (the internal dogfooding flow) without ever prompting for the endpoint. - writeProfile( + await writeProfile( 'dev', { apiKey: 'sk-old', apiUrl: 'https://api.example.com:8443' }, { path: credentialsPath }, @@ -350,7 +350,7 @@ describe('runConfigure', () => { it('dogfood-2026-05-25 — --from-env without TESTSPRITE_API_URL inherits existing profile api_url AND validates against it', async () => { const { capture, deps } = makeCapture(); // Pre-write an existing profile with a custom (non-default) endpoint. - writeProfile( + await writeProfile( 'default', { apiKey: 'sk-old', apiUrl: 'https://api.example.com' }, { path: credentialsPath }, @@ -398,7 +398,7 @@ describe('runConfigure', () => { it('dogfood-2026-05-25 — --endpoint-url flag overrides existing profile api_url', async () => { const { capture, deps } = makeCapture(); - writeProfile( + await writeProfile( 'default', { apiKey: 'sk-old', apiUrl: 'https://api.example.com' }, { path: credentialsPath }, @@ -428,7 +428,7 @@ describe('runConfigure', () => { it('dogfood-2026-05-25 — no advisory when inherited url equals DEFAULT_API_URL', async () => { const { capture, deps } = makeCapture(); // Existing profile has the default prod endpoint — no advisory needed. - writeProfile( + await writeProfile( 'default', { apiKey: 'sk-old', apiUrl: 'https://api.testsprite.com' }, { path: credentialsPath }, @@ -504,7 +504,7 @@ describe('runConfigure', () => { // An exported-but-empty env var (`export TESTSPRITE_API_URL=`) must not // short-circuit the `??` chain to an empty endpoint; it should fall through // to the existing profile's api_url. - writeProfile( + await writeProfile( 'default', { apiKey: 'sk-old', apiUrl: 'https://api.example.com:8443' }, { path: credentialsPath }, @@ -610,7 +610,7 @@ describe('runWhoami', () => { } it('calls GET /me using the configured profile and prints text output', async () => { - writeProfile( + await writeProfile( 'default', { apiKey: 'sk-stored', apiUrl: 'https://api.example.com' }, { path: credentialsPath }, @@ -633,7 +633,7 @@ describe('runWhoami', () => { }); it('emits JSON when --output json', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runWhoami( { profile: 'default', output: 'json', debug: false }, @@ -644,7 +644,7 @@ describe('runWhoami', () => { }); it('L1788: text output includes the resolved endpoint URL', async () => { - writeProfile( + await writeProfile( 'default', { apiKey: 'sk-stored', apiUrl: 'https://api.example.com' }, { path: credentialsPath }, @@ -672,7 +672,7 @@ describe('runWhoami', () => { }); it('L1788: JSON output does NOT add endpoint (raw /me envelope is passed through)', async () => { - writeProfile( + await writeProfile( 'default', { apiKey: 'sk-stored', apiUrl: 'https://api.example.com' }, { path: credentialsPath }, @@ -701,7 +701,7 @@ describe('runWhoami', () => { }); it('L1866: renders email + name in text mode when the backend supplies them', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const meWithEmail = new Response( JSON.stringify({ ...sampleMe, email: 'alice@example.com', displayName: 'Alice' }), @@ -718,7 +718,7 @@ describe('runWhoami', () => { }); it('L1866: omits email/name lines when the backend does not return them', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runWhoami( { profile: 'default', output: 'text', debug: false }, @@ -731,7 +731,7 @@ describe('runWhoami', () => { }); it('L1866: passes email through verbatim in JSON mode when present', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const meWithEmail = new Response(JSON.stringify({ ...sampleMe, email: 'alice@example.com' }), { status: 200, @@ -769,7 +769,7 @@ describe('runWhoami', () => { }); it('emits debug events to stderr when debug is enabled', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runWhoami( { profile: 'default', output: 'json', debug: true }, @@ -782,7 +782,7 @@ describe('runWhoami', () => { }); it('forwards server AUTH_INVALID with exit code 3', async () => { - writeProfile('default', { apiKey: 'sk-bad' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-bad' }, { path: credentialsPath }); const { deps } = makeCapture(); const errorBody = { error: { @@ -812,7 +812,7 @@ describe('runWhoami', () => { ...sampleMe, scopes: ['read:projects', 'read:tests'], }; - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const fetchImpl = makeFetch( new Response(JSON.stringify(readOnlyMe), { @@ -835,7 +835,7 @@ describe('runWhoami', () => { ...sampleMe, scopes: ['read:projects', 'read:tests', 'write:tests', 'run:tests'], }; - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const fetchImpl = makeFetch( new Response(JSON.stringify(fullMe), { @@ -856,7 +856,7 @@ describe('runWhoami', () => { ...sampleMe, scopes: ['read:projects', 'read:tests'], }; - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const fetchImpl = makeFetch( new Response(JSON.stringify(readOnlyMe), { @@ -878,8 +878,8 @@ describe('runWhoami', () => { describe('runLogout', () => { it('removes the profile and reports success', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); - writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runLogout( { profile: 'default', output: 'text', debug: false }, @@ -927,7 +927,7 @@ describe('createAuthCommand wiring', () => { }); it('remove deletes the active profile and exits 0', async () => { - writeProfile('default', { apiKey: 'sk-remove' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-remove' }, { path: credentialsPath }); const { deps } = makeCapture(); const auth = createAuthCommand({ ...deps, credentialsPath }); auth.exitOverride(); @@ -937,7 +937,7 @@ describe('createAuthCommand wiring', () => { }); it('deprecated `whoami` alias emits a deprecation notice pointing at `auth status`', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const auth = createAuthCommand({ ...deps, @@ -953,7 +953,7 @@ describe('createAuthCommand wiring', () => { }); it('whoami uses injected fetch and exits 0', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { deps } = makeCapture(); const fetchImpl = vi.fn( async () => @@ -970,7 +970,7 @@ describe('createAuthCommand wiring', () => { }); it('L1802: `status` alias resolves to the whoami action', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { deps } = makeCapture(); const fetchImpl = vi.fn( async () => @@ -987,7 +987,7 @@ describe('createAuthCommand wiring', () => { }); it('logout removes the profile', async () => { - writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath }); const { deps } = makeCapture(); const auth = createAuthCommand({ ...deps, credentialsPath }); auth.exitOverride(); diff --git a/src/commands/auth.ts b/src/commands/auth.ts index de1eda4..5eaeb9f 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -168,7 +168,7 @@ export async function runConfigure(opts: ConfigureOptions, deps: AuthDeps = {}): ); } - writeProfile(opts.profile, { apiKey, apiUrl }, { path: credentialsPath }); + await writeProfile(opts.profile, { apiKey, apiUrl }, { path: credentialsPath }); out.print({ profile: opts.profile, apiUrl, status: 'configured' }, data => { const d = data as { profile: string; apiUrl: string }; @@ -258,7 +258,7 @@ export async function runLogout(opts: CommonOptions, deps: AuthDeps = {}): Promi return; } - const removed = deleteProfile(opts.profile, { path: credentialsPath }); + const removed = await deleteProfile(opts.profile, { path: credentialsPath }); out.print({ profile: opts.profile, status: removed ? 'logged_out' : 'no_credentials' }, data => { const d = data as { profile: string; status: string }; return d.status === 'logged_out' diff --git a/src/commands/usage.test.ts b/src/commands/usage.test.ts index 28bdef3..3efe3bc 100644 --- a/src/commands/usage.test.ts +++ b/src/commands/usage.test.ts @@ -97,7 +97,7 @@ describe('runUsage — dry-run', () => { describe('runUsage — real path without credits (current backend)', () => { it('returns the /me response and emits a note about missing balance', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const result = await runUsage( { profile: 'default', output: 'text', debug: false }, @@ -116,7 +116,7 @@ describe('runUsage — real path without credits (current backend)', () => { }); it('text output includes identity fields even without credits', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runUsage( { profile: 'default', output: 'text', debug: false }, @@ -132,7 +132,7 @@ describe('runUsage — real path without credits (current backend)', () => { }); it('JSON output passes the raw /me response through (no credits key present)', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runUsage( { profile: 'default', output: 'json', debug: false }, @@ -150,7 +150,7 @@ describe('runUsage — real path without credits (current backend)', () => { describe('runUsage — real path with credits (future backend)', () => { it('renders balance block when credits + subPlan are present', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runUsage( { profile: 'default', output: 'text', debug: false }, @@ -173,7 +173,7 @@ describe('runUsage — real path with credits (future backend)', () => { }); it('does NOT emit the missing-balance note when credits are present', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runUsage( { profile: 'default', output: 'text', debug: false }, @@ -189,7 +189,7 @@ describe('runUsage — real path with credits (future backend)', () => { }); it('emits low-balance warning when credits < creditsPerRun', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const lowBalance: UsageResponse = { ...meWithCredits, credits: 1, creditsPerRun: 2 }; await runUsage( @@ -206,7 +206,7 @@ describe('runUsage — real path with credits (future backend)', () => { }); it('emits free-plan upgrade hint when subPlan is Free', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); const freePlan: UsageResponse = { ...meWithCredits, subPlan: 'Free', credits: 10 }; await runUsage( @@ -223,7 +223,7 @@ describe('runUsage — real path with credits (future backend)', () => { }); it('JSON output passes credits and subPlan through verbatim', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { capture, deps } = makeCapture(); await runUsage( { profile: 'default', output: 'json', debug: false }, @@ -249,7 +249,7 @@ describe('runUsage — error handling', () => { }); it('forwards server AUTH_INVALID with exit code 3', async () => { - writeProfile('default', { apiKey: 'sk-bad' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-bad' }, { path: credentialsPath }); const { deps } = makeCapture(); const errorBody = { error: { @@ -273,7 +273,7 @@ describe('runUsage — error handling', () => { }); it('re-maps INSUFFICIENT_CREDITS (rate_limited with credits sub-case) to exit 12', async () => { - writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); + await writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath }); const { deps } = makeCapture(); const creditError = { error: { diff --git a/src/lib/config.test.ts b/src/lib/config.test.ts index 463ddcc..ee6c5ef 100644 --- a/src/lib/config.test.ts +++ b/src/lib/config.test.ts @@ -21,8 +21,12 @@ describe('loadConfig', () => { expect(config.profile).toBe('default'); }); - it('honors TESTSPRITE_API_URL over the file', () => { - writeProfile('default', { apiUrl: 'https://from-file.example.com' }, { path: credentialsPath }); + it('honors TESTSPRITE_API_URL over the file', async () => { + await writeProfile( + 'default', + { apiUrl: 'https://from-file.example.com' }, + { path: credentialsPath }, + ); const config = loadConfig({ env: { TESTSPRITE_API_URL: 'https://from-env.example.com' }, credentialsPath, @@ -30,8 +34,8 @@ describe('loadConfig', () => { expect(config.apiUrl).toBe('https://from-env.example.com'); }); - it('honors TESTSPRITE_API_KEY over the file', () => { - writeProfile('default', { apiKey: 'sk-from-file' }, { path: credentialsPath }); + it('honors TESTSPRITE_API_KEY over the file', async () => { + await writeProfile('default', { apiKey: 'sk-from-file' }, { path: credentialsPath }); const config = loadConfig({ env: { TESTSPRITE_API_KEY: 'sk-from-env' }, credentialsPath, @@ -55,8 +59,8 @@ describe('loadConfig', () => { ).toBe('option-profile'); }); - it('option.endpointUrl overrides everything', () => { - writeProfile('default', { apiUrl: 'https://file' }, { path: credentialsPath }); + it('option.endpointUrl overrides everything', async () => { + await writeProfile('default', { apiUrl: 'https://file' }, { path: credentialsPath }); const config = loadConfig({ endpointUrl: 'https://flag', env: { TESTSPRITE_API_URL: 'https://env' }, @@ -65,8 +69,8 @@ describe('loadConfig', () => { expect(config.apiUrl).toBe('https://flag'); }); - it('falls back to credentials file when env is unset', () => { - writeProfile( + it('falls back to credentials file when env is unset', async () => { + await writeProfile( 'default', { apiKey: 'sk-file', apiUrl: 'https://from-file.example.com' }, { path: credentialsPath }, @@ -76,8 +80,8 @@ describe('loadConfig', () => { expect(config.apiUrl).toBe('https://from-file.example.com'); }); - it('reads the requested profile, not just default', () => { - writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); + it('reads the requested profile, not just default', async () => { + await writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); const config = loadConfig({ profile: 'dev', env: {}, credentialsPath }); expect(config.apiKey).toBe('sk-dev'); }); diff --git a/src/lib/credentials.test.ts b/src/lib/credentials.test.ts index ac038f3..7f206d8 100644 --- a/src/lib/credentials.test.ts +++ b/src/lib/credentials.test.ts @@ -1,6 +1,9 @@ +import type { ChildProcess } from 'node:child_process'; +import { spawn } from 'node:child_process'; import { mkdtempSync, statSync, readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { DEFAULT_PROFILE, @@ -104,26 +107,30 @@ describe('readCredentialsFile / readProfile', () => { }); describe('writeProfile', () => { - it('creates the file with mode 0600 and writes the profile', () => { - writeProfile(DEFAULT_PROFILE, { apiKey: 'sk-new' }, { path: credentialsPath }); + it('creates the file with mode 0600 and writes the profile', async () => { + await writeProfile(DEFAULT_PROFILE, { apiKey: 'sk-new' }, { path: credentialsPath }); expect(existsSync(credentialsPath)).toBe(true); const mode = statSync(credentialsPath).mode & 0o777; expect(mode).toBe(0o600); expect(readProfile(DEFAULT_PROFILE, { path: credentialsPath })).toEqual({ apiKey: 'sk-new' }); }); - it('preserves other profiles when updating one', () => { - writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); - writeProfile('dev', { apiKey: 'sk-dev', apiUrl: 'https://dev' }, { path: credentialsPath }); - writeProfile('default', { apiUrl: 'https://prod' }, { path: credentialsPath }); + it('preserves other profiles when updating one', async () => { + await writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); + await writeProfile( + 'dev', + { apiKey: 'sk-dev', apiUrl: 'https://dev' }, + { path: credentialsPath }, + ); + await writeProfile('default', { apiUrl: 'https://prod' }, { path: credentialsPath }); const file = readCredentialsFile({ path: credentialsPath }); expect(file.default).toEqual({ apiKey: 'sk-d', apiUrl: 'https://prod' }); expect(file.dev).toEqual({ apiKey: 'sk-dev', apiUrl: 'https://dev' }); }); - it('does not leak the api key into the on-disk file format aside from the value itself', () => { - writeProfile('default', { apiKey: 'sk-secret-12345' }, { path: credentialsPath }); + it('does not leak the api key into the on-disk file format aside from the value itself', async () => { + await writeProfile('default', { apiKey: 'sk-secret-12345' }, { path: credentialsPath }); const onDisk = readFileSync(credentialsPath, 'utf-8'); expect(onDisk).toContain('api_key = sk-secret-12345'); expect(onDisk.split('\n').filter(line => line.includes('sk-secret-12345'))).toHaveLength(1); @@ -131,21 +138,21 @@ describe('writeProfile', () => { }); describe('deleteProfile', () => { - it('returns false when the profile is missing', () => { - expect(deleteProfile('nope', { path: credentialsPath })).toBe(false); + it('returns false when the profile is missing', async () => { + expect(await deleteProfile('nope', { path: credentialsPath })).toBe(false); }); - it('removes the named profile and leaves others intact', () => { - writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); - writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); - expect(deleteProfile('dev', { path: credentialsPath })).toBe(true); + it('removes the named profile and leaves others intact', async () => { + await writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); + await writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); + expect(await deleteProfile('dev', { path: credentialsPath })).toBe(true); expect(readProfile('dev', { path: credentialsPath })).toBeUndefined(); expect(readProfile('default', { path: credentialsPath })).toEqual({ apiKey: 'sk-d' }); }); - it('leaves an empty file when the last profile is removed', () => { - writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); - expect(deleteProfile('default', { path: credentialsPath })).toBe(true); + it('leaves an empty file when the last profile is removed', async () => { + await writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); + expect(await deleteProfile('default', { path: credentialsPath })).toBe(true); expect(readCredentialsFile({ path: credentialsPath })).toEqual({}); expect(existsSync(credentialsPath)).toBe(true); }); @@ -171,6 +178,61 @@ describe('defaultCredentialsPath', () => { }); }); +const projectRoot = fileURLToPath(new URL('../..', import.meta.url)); +const credentialsWriteChild = join(projectRoot, 'test/helpers/credentials-write-child.mjs'); + +function waitForChild(child: ChildProcess): Promise { + return new Promise((resolve, reject) => { + let stderr = ''; + child.stderr?.on('data', chunk => { + stderr += String(chunk); + }); + child.on('error', reject); + child.on('close', code => { + if (code === 0) resolve(); + else reject(new Error(`child exited with code ${code}: ${stderr}`)); + }); + }); +} + +function spawnCredentialsWriter(profile: string, apiKey: string, path: string): ChildProcess { + return spawn(process.execPath, [credentialsWriteChild], { + env: { + ...process.env, + CRED_PROFILE: profile, + CRED_PATH: path, + CRED_API_KEY: apiKey, + }, + stdio: ['ignore', 'ignore', 'pipe'], + }); +} + +describe('credentials write lock', () => { + it('serializes cross-process writes so concurrent profile updates are not lost', async () => { + const children = [ + spawnCredentialsWriter('dev', 'sk-dev', credentialsPath), + spawnCredentialsWriter('staging', 'sk-staging', credentialsPath), + ]; + await Promise.all(children.map(waitForChild)); + + const file = readCredentialsFile({ path: credentialsPath }); + expect(file.dev).toEqual({ apiKey: 'sk-dev' }); + expect(file.staging).toEqual({ apiKey: 'sk-staging' }); + }); + + it('removes the lock file after writeProfile completes', async () => { + await writeProfile('default', { apiKey: 'sk-lock-cleanup' }, { path: credentialsPath }); + expect(existsSync(`${credentialsPath}.lock`)).toBe(false); + }); + + it('removes the lock file after deleteProfile completes', async () => { + await writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); + await writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); + expect(await deleteProfile('dev', { path: credentialsPath })).toBe(true); + expect(existsSync(`${credentialsPath}.lock`)).toBe(false); + }); +}); + describe('assertValidProfileName / profile-name guard', () => { it('accepts conventional profile names', () => { for (const name of ['default', 'dev', 'prod', 'ci-staging', 'team.qa', 'a_b', 'P1']) { @@ -197,15 +259,15 @@ describe('assertValidProfileName / profile-name guard', () => { } }); - it('writeProfile rejects a malformed name and does NOT create the file', () => { - expect(() => writeProfile('prod]', { apiKey: 'sk-1' }, { path: credentialsPath })).toThrow( - ApiError, - ); + it('writeProfile rejects a malformed name and does NOT create the file', async () => { + await expect( + writeProfile('prod]', { apiKey: 'sk-1' }, { path: credentialsPath }), + ).rejects.toThrow(ApiError); expect(existsSync(credentialsPath)).toBe(false); }); - it('readProfile and deleteProfile reject a malformed name', () => { + it('readProfile and deleteProfile reject a malformed name', async () => { expect(() => readProfile('a\nb', { path: credentialsPath })).toThrow(ApiError); - expect(() => deleteProfile('a\nb', { path: credentialsPath })).toThrow(ApiError); + await expect(deleteProfile('a\nb', { path: credentialsPath })).rejects.toThrow(ApiError); }); }); diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index c885c66..8b62702 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -5,6 +5,7 @@ import { readFileSync, renameSync, statSync, + unlinkSync, writeFileSync, } from 'node:fs'; import { homedir } from 'node:os'; @@ -138,30 +139,37 @@ export function readProfile( return file[profile]; } -export function writeProfile( +export async function writeProfile( profile: string, entry: ProfileEntry, options: CredentialsOptions = {}, -): void { +): Promise { assertValidProfileName(profile); const path = resolvePath(options); - const file = readCredentialsFile(options); - file[profile] = { ...file[profile], ...entry }; - writeCredentialsAtomic(path, file); + await withCredentialsLock(path, () => { + const file = readCredentialsFile(options); + file[profile] = { ...file[profile], ...entry }; + writeCredentialsAtomic(path, file); + }); } -export function deleteProfile(profile: string, options: CredentialsOptions = {}): boolean { +export async function deleteProfile( + profile: string, + options: CredentialsOptions = {}, +): Promise { assertValidProfileName(profile); const path = resolvePath(options); - const file = readCredentialsFile(options); - if (!(profile in file)) return false; - delete file[profile]; - if (Object.keys(file).length === 0) { - writeCredentialsAtomic(path, {}); - } else { - writeCredentialsAtomic(path, file); - } - return true; + return await withCredentialsLock(path, () => { + const file = readCredentialsFile(options); + if (!(profile in file)) return false; + delete file[profile]; + if (Object.keys(file).length === 0) { + writeCredentialsAtomic(path, {}); + } else { + writeCredentialsAtomic(path, file); + } + return true; + }); } export function ensureRestrictiveMode(path: string): void { @@ -181,3 +189,87 @@ function writeCredentialsAtomic(path: string, file: CredentialsFile): void { renameSync(tmp, path); ensureRestrictiveMode(path); } + +/** Max wall-clock wait when another process holds the credentials lock. */ +const CREDENTIALS_LOCK_MAX_WAIT_MS = 10_000; +/** Back-off between lock attempts. */ +const CREDENTIALS_LOCK_RETRY_MS = 25; +/** Reclaim a lock file when the holder pid is gone or the file is older than this. */ +const CREDENTIALS_LOCK_STALE_MS = 30_000; + +function credentialsLockPath(credentialsPath: string): string { + return `${credentialsPath}.lock`; +} + +/** + * Serialize read-modify-write on the credentials file across processes. + * `writeCredentialsAtomic` only makes the final rename atomic; without this + * lock, concurrent `writeProfile` / `deleteProfile` calls can each read the + * same snapshot and the last rename wins — silently dropping the other update. + */ +async function withCredentialsLock(credentialsPath: string, fn: () => T): Promise { + await acquireCredentialsLock(credentialsPath); + try { + return fn(); + } finally { + releaseCredentialsLock(credentialsPath); + } +} + +async function acquireCredentialsLock(credentialsPath: string): Promise { + const lockPath = credentialsLockPath(credentialsPath); + // Ensure the credentials directory exists before creating the lock file. + // writeCredentialsAtomic also mkdirs, but only after the lock is held. + mkdirSync(dirname(lockPath), { recursive: true, mode: 0o700 }); + const deadline = Date.now() + CREDENTIALS_LOCK_MAX_WAIT_MS; + while (Date.now() < deadline) { + try { + writeFileSync(lockPath, `${process.pid}\n${Date.now()}\n`, { flag: 'wx', encoding: 'utf8' }); + return; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code !== 'EEXIST') throw err; + if (isStaleCredentialsLock(lockPath)) { + try { + unlinkSync(lockPath); + } catch { + // Another waiter may have claimed or released the lock. + } + continue; + } + await new Promise(resolve => setTimeout(resolve, CREDENTIALS_LOCK_RETRY_MS)); + } + } + throw new Error(`Timed out acquiring credentials lock: ${lockPath}`); +} + +function releaseCredentialsLock(credentialsPath: string): void { + try { + unlinkSync(credentialsLockPath(credentialsPath)); + } catch { + // Lock already released or never acquired — teardown must not mask errors. + } +} + +function isStaleCredentialsLock(lockPath: string): boolean { + try { + const content = readFileSync(lockPath, 'utf8'); + const [pidLine = '', tsLine = ''] = content.split('\n'); + const pid = Number.parseInt(pidLine.trim(), 10); + const ts = Number.parseInt(tsLine.trim(), 10); + // Incomplete lock — another process may be creating it; never reclaim. + if (!Number.isFinite(pid) || pid <= 0 || !Number.isFinite(ts)) return false; + + const stat = statSync(lockPath); + if (Date.now() - stat.mtimeMs > CREDENTIALS_LOCK_STALE_MS) return true; + if (Date.now() - ts > CREDENTIALS_LOCK_STALE_MS) return true; + try { + process.kill(pid, 0); + return false; + } catch { + return true; + } + } catch { + return false; + } +} diff --git a/test/cli.subprocess.test.ts b/test/cli.subprocess.test.ts index 95521a3..1b6c08d 100644 --- a/test/cli.subprocess.test.ts +++ b/test/cli.subprocess.test.ts @@ -32,12 +32,8 @@ let baseUrl: string; let tmpHome: string; beforeAll(async () => { - // Always rebuild — `npm run build` is fast and a stale `dist/index.js` - // would silently mask ESM/import regressions in this suite. The - // existsSync skip we used to do here let `dist` rot under - // refactors and gave false-green on `project list` once - // already. - execFileSync('npm', ['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); + // `test/global-setup.mjs` builds dist/ once before workers start (skips when + // dist/index.js is newer than src/). CI also pre-builds before npm test. server = createServer((req: IncomingMessage, res: ServerResponse) => { const url = req.url ?? '/'; if (url.startsWith('/api/cli/v1/projects/')) { diff --git a/test/global-setup.mjs b/test/global-setup.mjs new file mode 100644 index 0000000..343c3a1 --- /dev/null +++ b/test/global-setup.mjs @@ -0,0 +1,32 @@ +import { execSync } from 'node:child_process'; +import { existsSync, readdirSync, statSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const repoRoot = fileURLToPath(new URL('..', import.meta.url)); +const distEntry = join(repoRoot, 'dist', 'index.js'); +const srcDir = join(repoRoot, 'src'); + +function newestMtimeMs(dir) { + let newest = 0; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const path = join(dir, entry.name); + if (entry.isDirectory()) { + newest = Math.max(newest, newestMtimeMs(path)); + } else if (entry.isFile()) { + newest = Math.max(newest, statSync(path).mtimeMs); + } + } + return newest; +} + +function needsBuild() { + if (!existsSync(distEntry)) return true; + return newestMtimeMs(srcDir) > statSync(distEntry).mtimeMs; +} + +/** Runs once before any test file worker — shared dist/ for subprocess suites. */ +export default async function globalSetup() { + if (!needsBuild()) return; + execSync('npm run build', { cwd: repoRoot, stdio: 'inherit' }); +} diff --git a/test/help.snapshot.test.ts b/test/help.snapshot.test.ts index 2448c91..35e121c 100644 --- a/test/help.snapshot.test.ts +++ b/test/help.snapshot.test.ts @@ -5,14 +5,14 @@ * * Lives under `test/` * (not `src/`) to mirror the existing subprocess test pattern — the - * snapshot runs the real built binary and therefore needs a build in - * `beforeAll`, the same way `test/cli.subprocess.test.ts` does. + * snapshot runs the real built binary; `test/global-setup.mjs` builds + * `dist/` once before any worker starts. */ import { execFileSync } from 'node:child_process'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { beforeAll, describe, expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; const __dirname = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = resolve(__dirname, '..'); @@ -45,10 +45,6 @@ const cases: Array<[string, string[]]> = [ ]; describe('--help snapshots', () => { - beforeAll(() => { - execFileSync('npm', ['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); - }); - for (const [name, args] of cases) { it(name, () => { const out = execFileSync('node', [BIN_PATH, ...args], { diff --git a/test/helpers/credentials-write-child.mjs b/test/helpers/credentials-write-child.mjs new file mode 100644 index 0000000..1b7e647 --- /dev/null +++ b/test/helpers/credentials-write-child.mjs @@ -0,0 +1,12 @@ +import { writeProfile } from '../../dist/lib/credentials.js'; + +const profile = process.env.CRED_PROFILE; +const credentialsPath = process.env.CRED_PATH; +const apiKey = process.env.CRED_API_KEY; + +if (!profile || !credentialsPath || !apiKey) { + console.error('CRED_PROFILE, CRED_PATH, and CRED_API_KEY are required'); + process.exit(1); +} + +await writeProfile(profile, { apiKey }, { path: credentialsPath }); diff --git a/vitest.config.ts b/vitest.config.ts index 18ea00f..4bac6b0 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -4,6 +4,7 @@ export default defineConfig({ test: { include: ['src/**/*.{test,spec}.ts', 'test/**/*.{test,spec}.ts'], exclude: ['test/dev-e2e/**', 'test/e2e/**', 'node_modules/**', 'dist/**'], + globalSetup: ['./test/global-setup.mjs'], coverage: { provider: 'v8', reporter: ['text', 'text-summary', 'json-summary', 'html'],