diff --git a/src/bin.ts b/src/bin.ts index a41d3922..2e723e43 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -145,6 +145,20 @@ const insecureStorageOption = { }, } as const; +/** + * Shared override for the "AuthKit is already installed" preflight guard. + * + * Distinct from `--force-install` below, which only relaxes peer-dependency + * checks during package installation. + */ +const forceOption = { + force: { + default: false, + describe: 'Continue even if AuthKit is already installed in this project', + type: 'boolean' as const, + }, +} as const; + const installerOptions = { direct: { alias: 'D', @@ -251,6 +265,7 @@ const installerOptions = { describe: 'Next.js router to target when detection is ambiguous (app or pages)', type: 'string' as const, }, + ...forceOption, }; // Check for updates (blocks up to 500ms, skip in JSON/non-human modes to keep machine streams clean) @@ -2551,6 +2566,11 @@ async function runCli(): Promise { (yargs) => yargs.options(installerOptions), async (argv) => { await applyInsecureStorage(argv.insecureStorage); + // MUST run before credential resolution below: that provisions a WorkOS + // environment and writes its credentials into the project's env file, + // so a guard placed after it is no guard at all. + const preflight = await import('./lib/preflight-authkit.js'); + await preflight.assertNoExistingAuthKit({ installDir: argv.installDir ?? process.cwd(), force: argv.force }); await resolveInstallCredentials(argv.apiKey, argv.installDir, argv.skipAuth, ensureAuthenticated); const { handleInstall } = await import('./commands/install.js'); await handleInstall(argv); @@ -2750,6 +2770,9 @@ async function runCli(): Promise { (yargs) => yargs.options(installerOptions), async (argv) => { await applyInsecureStorage(argv.insecureStorage); + // Guard first, before credential resolution — see the `install` handler above. + const preflight = await import('./lib/preflight-authkit.js'); + await preflight.assertNoExistingAuthKit({ installDir: argv.installDir ?? process.cwd(), force: argv.force }); await resolveInstallCredentials(argv.apiKey, argv.installDir, argv.skipAuth, ensureAuthenticated); const { handleInstall } = await import('./commands/install.js'); await handleInstall({ ...argv, dashboard: true }); @@ -2758,7 +2781,9 @@ async function runCli(): Promise { .command( ['$0'], 'WorkOS AuthKit CLI', - (yargs) => yargs.options(insecureStorageOption), + // `--force` must be registered here too: this parser is .strict(), so + // `npx workos --force` would die as an unknown argument otherwise. + (yargs) => yargs.options({ ...insecureStorageOption, ...forceOption }), async (argv) => { // Non-human modes: emit machine-readable command tree (JSON) or the // fully-configured parser help (human non-TTY edge) instead of prompting. @@ -2782,6 +2807,10 @@ async function runCli(): Promise { } await applyInsecureStorage(argv.insecureStorage); + // After the confirm above (two prompts back to back is worse UX), but + // still before credential resolution touches the project. + const preflight = await import('./lib/preflight-authkit.js'); + await preflight.assertNoExistingAuthKit({ installDir: process.cwd(), force: argv.force }); await resolveInstallCredentials(undefined, undefined, false, ensureAuthenticated); const { handleInstall } = await import('./commands/install.js'); diff --git a/src/doctor/checks/sdk.ts b/src/doctor/checks/sdk.ts index b905e175..92b9dbda 100644 --- a/src/doctor/checks/sdk.ts +++ b/src/doctor/checks/sdk.ts @@ -18,7 +18,13 @@ const SDK_PACKAGES = [ 'workos', // very old legacy ] as const; -const AUTHKIT_PACKAGES = new Set([ +/** + * AuthKit SDKs only — deliberately excludes `@workos-inc/node` and legacy + * `workos`, which are present in plenty of projects that never installed + * AuthKit. Consumed by the install preflight guard (`lib/preflight-authkit.ts`), + * where matching the base SDK would block legitimate first-time installs. + */ +export const AUTHKIT_PACKAGES = new Set([ '@workos/authkit-tanstack-react-start', '@workos/authkit-sveltekit', '@workos-inc/authkit-nextjs', diff --git a/src/lib/adapters/cli-adapter.spec.ts b/src/lib/adapters/cli-adapter.spec.ts index 3d3981ab..07fd6654 100644 --- a/src/lib/adapters/cli-adapter.spec.ts +++ b/src/lib/adapters/cli-adapter.spec.ts @@ -366,6 +366,62 @@ describe('CLIAdapter', () => { }); }); + describe('error rendering', () => { + async function renderError(message: string): Promise { + await adapter.start(); + const ui = await import('../../utils/ui.js'); + + emitter.emit('error', { message, stack: undefined }); + + return [...vi.mocked(ui.default.log.error).mock.calls, ...vi.mocked(ui.default.log.info).mock.calls] + .map((c) => String(c[0])) + .join('\n'); + } + + // The gateway's generic 500 fires for deterministic request failures too, so + // this is the interactive copy the reporter hit: four more minutes, same 500. + it('does not advise waiting for the gateway generic 500', async () => { + const output = await renderError( + 'API Error: 500 {"error":{"type":"internal_error","message":"An unexpected error occurred"}}', + ); + + expect(output).not.toMatch(/temporarily unavailable/i); + expect(output).not.toMatch(/few minutes|try again shortly|wait a minute/i); + expect(output).toMatch(/could not complete this request/i); + }); + + // installer-core re-emits runAgent's already-rendered message, so the + // adapter classifies our own copy on the real path — not the raw SDK text. + it('keeps the deterministic copy when handed an already-rendered message', async () => { + const output = await renderError( + 'The AI service could not complete this request. The same request is likely to fail the same way, so waiting will not help. Re-run with --debug to see the underlying error.', + ); + + expect(output).not.toMatch(/temporarily unavailable/i); + + // The headline must be re-derived, not echoed: a pass-through of the + // already-rendered string would put all three sentences in log.error. + const ui = await import('../../utils/ui.js'); + expect(vi.mocked(ui.default.log.error).mock.calls[0]?.[0]).toBe( + 'The AI service could not complete this request.', + ); + }); + + it('still shows the transient copy for a real 503', async () => { + const output = await renderError('API Error: 503 Service Unavailable'); + + expect(output).toMatch(/temporarily unavailable/i); + expect(output).toMatch(/few minutes/i); + }); + + it('renders the raw message when nothing matches', async () => { + const output = await renderError('Something broke'); + + expect(output).toMatch(/Something broke/); + expect(output).toMatch(/--debug/); + }); + }); + describe('staging success copy', () => { it('device path announces a fresh environment without "retrieved"', async () => { await adapter.start(); diff --git a/src/lib/adapters/cli-adapter.ts b/src/lib/adapters/cli-adapter.ts index 4e975013..d8a52883 100644 --- a/src/lib/adapters/cli-adapter.ts +++ b/src/lib/adapters/cli-adapter.ts @@ -6,7 +6,7 @@ import chalk from 'chalk'; import { getConfig } from '../settings.js'; import { ProgressTracker } from '../progress-tracker.js'; import { renderCompletionSummary, renderBrandMark } from '../../utils/summary-box.js'; -import { formatWorkOSCommand } from '../../utils/command-invocation.js'; +import { classifyAgentFailure, describeAgentFailure } from '../failure-classifier.js'; /** * CLI adapter that renders wizard events via ui. @@ -548,41 +548,16 @@ export class CLIAdapter implements InstallerAdapter { this.stopSpinner('Failed', 1); // Rewrite raw API/SDK errors into user-friendly messages with a next step. - // Matching is word-boundary / code-based so 'author' doesn't read as 'auth' - // and 'Module not found' doesn't read as a missing-directory error. - const isServiceError = - /\b50[0-9]\b/.test(message) || /server_error|internal_error|overloaded|service.*unavailable/i.test(message); - const isRateLimit = /\b429\b/.test(message) || /\brate.?limit/i.test(message); - const isNetworkError = /ECONNREFUSED|ETIMEDOUT|ENOTFOUND|fetch failed/i.test(message); - const isProcessExit = /process exited with code/i.test(message); - const isAuthError = /\b(401|403|unauthorized|forbidden|authentication|authorization)\b/i.test(message); - const isMissingPath = /\bENOENT\b/.test(message); - - if (isServiceError) { - ui.log.error('The AI service is temporarily unavailable.'); - ui.log.info('This is usually resolved within a few minutes. Please try again shortly.'); - } else if (isRateLimit) { - ui.log.error('The AI service is currently rate-limited.'); - ui.log.info('Please wait a minute and try again.'); - } else if (isNetworkError) { - ui.log.error('Could not connect to the AI service.'); - ui.log.info('Check your internet connection and try again.'); - } else if (isProcessExit) { - ui.log.error('The AI agent process exited unexpectedly.'); - ui.log.info('Try running again. If this persists, run with --debug for details.'); - } else if (isAuthError) { - ui.log.error('Authentication failed.'); - ui.log.info(`Try running: ${formatWorkOSCommand('auth logout')} && ${formatWorkOSCommand('install')}`); - } else if (isMissingPath) { - ui.log.error(message); - ui.log.info('Make sure you are running this in your project directory.'); - } else { - // Unknown error: still give the user somewhere to go next. - ui.log.error(message); - ui.log.info( + // The verdict comes from the shared classifier so this path cannot drift + // from the headless adapter or from runAgent's own error message. + const { headline, detail } = describeAgentFailure(classifyAgentFailure(message)); + + ui.log.error(headline ?? message); + ui.log.info( + // No specific advice for this kind: still give the user somewhere to go next. + detail ?? `Re-run with ${chalk.cyan('--debug')} for details, or report it at ${chalk.cyan('https://github.com/workos/cli/issues')}`, - ); - } + ); if (stack && this.debug) { this.debugLog(stack); diff --git a/src/lib/adapters/headless-adapter.spec.ts b/src/lib/adapters/headless-adapter.spec.ts index ec1c2e6f..e053a996 100644 --- a/src/lib/adapters/headless-adapter.spec.ts +++ b/src/lib/adapters/headless-adapter.spec.ts @@ -455,6 +455,39 @@ describe('HeadlessAdapter', () => { }); await adapter.stop(); }); + + // The gateway's generic 500 branch also fires for request-shape failures, so + // agents and CI must not be told to wait it out: the retry fails identically. + it('codes the gateway generic 500 as deterministic, not a transient outage', async () => { + const adapter = createAdapter(); + await adapter.start(); + + emitter.emit('error', { message: 'API Error: 500 An unexpected error occurred', stack: undefined }); + + expect(mockWriteNDJSON).toHaveBeenCalledWith({ + type: 'error', + code: 'deterministic_error', + message: expect.stringContaining('likely to fail the same way'), + }); + expect(mockWriteNDJSON).not.toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringContaining('temporarily unavailable') }), + ); + await adapter.stop(); + }); + + it('still codes a real 503 as service_unavailable', async () => { + const adapter = createAdapter(); + await adapter.start(); + + emitter.emit('error', { message: 'API Error: 503 Service Unavailable', stack: undefined }); + + expect(mockWriteNDJSON).toHaveBeenCalledWith({ + type: 'error', + code: 'service_unavailable', + message: expect.stringContaining('temporarily unavailable'), + }); + await adapter.stop(); + }); }); describe('staging events', () => { diff --git a/src/lib/adapters/headless-adapter.ts b/src/lib/adapters/headless-adapter.ts index fe07c3dd..cdc4e547 100644 --- a/src/lib/adapters/headless-adapter.ts +++ b/src/lib/adapters/headless-adapter.ts @@ -2,6 +2,20 @@ import type { InstallerAdapter, AdapterConfig } from './types.js'; import type { InstallerEventEmitter, InstallerEvents } from '../events.js'; import { writeNDJSON } from '../../utils/ndjson.js'; import { ExitCode } from '../../utils/exit-codes.js'; +import { classifyAgentFailure, formatAgentFailure, type AgentFailureKind } from '../failure-classifier.js'; + +/** + * Structured `code` emitted per failure kind. These are part of the NDJSON + * contract, so the existing codes must not be renamed; kinds without a + * dedicated code stay on `installer_error` and stream the raw message. + */ +const ERROR_CODES: Partial> = { + service_outage: 'service_unavailable', + rate_limited: 'rate_limited', + deterministic: 'deterministic_error', + network: 'network_error', + process_exit: 'process_error', +}; /** * Options controlling headless adapter behavior. @@ -412,30 +426,18 @@ export class HeadlessAdapter implements InstallerAdapter { }; private handleError = ({ message, stack }: InstallerEvents['error']): void => { - const isServiceError = - /\b50[0-9]\b/.test(message) || /server_error|internal_error|overloaded|service.*unavailable/i.test(message); - const isRateLimit = /\b429\b/.test(message) || /rate.limit/i.test(message); - const isNetworkError = /ECONNREFUSED|ETIMEDOUT|ENOTFOUND|fetch failed/i.test(message); - const isProcessExit = /process exited with code/i.test(message); - - let code = 'installer_error'; - let displayMessage = message; - - if (isServiceError) { - code = 'service_unavailable'; - displayMessage = 'The AI service is temporarily unavailable. Please try again in a few minutes.'; - } else if (isRateLimit) { - code = 'rate_limited'; - displayMessage = 'The AI service is currently rate-limited. Please wait a minute and try again.'; - } else if (isNetworkError) { - code = 'network_error'; - displayMessage = 'Could not connect to the AI service. Check your internet connection and try again.'; - } else if (isProcessExit) { - code = 'process_error'; - displayMessage = 'The AI agent process exited unexpectedly. Try running again with --debug for details.'; - } + // The verdict comes from the shared classifier so JSON consumers and the + // interactive path cannot disagree about whether a retry is worth trying. + const kind = classifyAgentFailure(message); + const code = ERROR_CODES[kind]; - writeNDJSON({ type: 'error', code, message: displayMessage }); + writeNDJSON({ + type: 'error', + code: code ?? 'installer_error', + // Kinds with no dedicated code keep streaming the raw message — JSON + // consumers depend on it where we have nothing better to say. + message: code ? formatAgentFailure(kind, message) : message, + }); this.debugLog(stack ?? ''); }; } diff --git a/src/lib/agent-interface.spec.ts b/src/lib/agent-interface.spec.ts index bc8fc5ae..24fc5a03 100644 --- a/src/lib/agent-interface.spec.ts +++ b/src/lib/agent-interface.spec.ts @@ -300,12 +300,47 @@ describe('service unavailability handling', () => { }) as typeof emitter.emit; }); - it('detects is_error result with API 500 as SERVICE_UNAVAILABLE', async () => { + // The gateway's generic 500 branch also fires for request-shape failures, so + // this text is NOT evidence of an outage. Telling the user to wait a few + // minutes costs them another four-minute run that fails identically. + it('treats the gateway generic 500 as deterministic, not a transient outage', async () => { const apiErrorText = 'API Error: 500 {"error":{"type":"internal_error","message":"An unexpected error occurred"}}'; mockQuery.mockImplementation(createMockSDKResponse([{ text: apiErrorText, is_error: true }])); const result = await runAgent(makeAgentConfig(), 'Test prompt', makeOptions(), undefined, emitter); + expect(result.error).toBe(AgentErrorType.EXECUTION_ERROR); + expect(result.errorMessage).not.toMatch(/temporarily unavailable/); + expect(result.errorMessage).not.toMatch(/few minutes|try again shortly/i); + expect(result.errorMessage).toMatch(/likely to fail the same way/); + }); + + // The proxy answers its own socket timeout with a 504, so the message carries a + // 5xx that the old "any 5xx is transient" rule misread as an outage. + it("treats the proxy's own upstream_timeout as deterministic", async () => { + mockQuery.mockImplementation( + createMockSDKResponse([ + { + text: 'API Error: 504 {"error":"upstream_timeout","message":"Upstream server timed out"}', + is_error: true, + }, + ]), + ); + + const result = await runAgent(makeAgentConfig(), 'Test prompt', makeOptions(), undefined, emitter); + + expect(result.error).toBe(AgentErrorType.EXECUTION_ERROR); + expect(result.errorMessage).not.toMatch(/temporarily unavailable/); + expect(result.errorMessage).toMatch(/likely to fail the same way/); + }); + + it('detects a 503 as SERVICE_UNAVAILABLE', async () => { + mockQuery.mockImplementation( + createMockSDKResponse([{ text: 'API Error: 503 Service Unavailable', is_error: true }]), + ); + + const result = await runAgent(makeAgentConfig(), 'Test prompt', makeOptions(), undefined, emitter); + expect(result.error).toBe(AgentErrorType.SERVICE_UNAVAILABLE); expect(result.errorMessage).toMatch(/temporarily unavailable/); }); @@ -328,8 +363,7 @@ describe('service unavailability handling', () => { }); it('skips validation retries when service is unavailable', async () => { - const apiErrorText = 'API Error: 500 {"error":{"type":"internal_error","message":"An unexpected error occurred"}}'; - mockQuery.mockImplementation(createMockSDKResponse([{ text: apiErrorText, is_error: true }])); + mockQuery.mockImplementation(createMockSDKResponse([{ text: 'API Error: 503 overloaded_error', is_error: true }])); const validateAndFormat = vi.fn().mockResolvedValue('Still broken'); diff --git a/src/lib/agent-interface.ts b/src/lib/agent-interface.ts index 2db6c170..5e8d31ae 100644 --- a/src/lib/agent-interface.ts +++ b/src/lib/agent-interface.ts @@ -19,6 +19,7 @@ import type { InstallerEventEmitter } from './events.js'; import { startCredentialProxy, startClaimTokenProxy, type CredentialProxyHandle } from './credential-proxy.js'; import { getActiveEnvironment, isUnclaimedEnvironment } from './config-store.js'; import { getAuthkitDomain, getCliAuthClientId } from './settings.js'; +import { classifyAgentFailure, formatAgentFailure } from './failure-classifier.js'; import type { SDKMessage, SDKUserMessage, @@ -63,6 +64,9 @@ const SERVICE_UNAVAILABLE_PREFIX = '__SERVICE_UNAVAILABLE__'; /** Internal prefix used to tag rate-limit errors from handleSDKMessage */ const RATE_LIMITED_PREFIX = '__RATE_LIMITED__'; +/** Internal prefix used to tag failures that will recur on every retry */ +const DETERMINISTIC_PREFIX = '__DETERMINISTIC__'; + /** * Error types that can be returned from agent execution. * These correspond to the error signals that the agent emits. @@ -702,7 +706,7 @@ export async function runAgent( logError('AI service unavailable:', detail); return { error: AgentErrorType.SERVICE_UNAVAILABLE, - errorMessage: 'The AI service is temporarily unavailable. Please try again in a few minutes.', + errorMessage: formatAgentFailure('service_outage', detail), }; } if (sdkError.startsWith(RATE_LIMITED_PREFIX)) { @@ -710,7 +714,18 @@ export async function runAgent( logError('AI service rate-limited:', detail); return { error: AgentErrorType.SERVICE_UNAVAILABLE, - errorMessage: 'The AI service is currently rate-limited. Please wait a minute and try again.', + errorMessage: formatAgentFailure('rate_limited', detail), + }; + } + if (sdkError.startsWith(DETERMINISTIC_PREFIX)) { + const detail = sdkError.slice(DETERMINISTIC_PREFIX.length); + // A gateway 500 is not automatically an outage: its generic branch also + // fires for request-shape failures, which recur on every attempt. Say so + // instead of sending the user round another four-minute run. + logError('Agent failure will recur on retry:', detail); + return { + error: AgentErrorType.EXECUTION_ERROR, + errorMessage: formatAgentFailure('deterministic', detail), }; } logError('Agent SDK error:', sdkError); @@ -922,16 +937,19 @@ function handleSDKMessage( const resultText = typeof message.result === 'string' ? message.result : ''; logError('Agent result marked as error:', resultText); - // Detect rate limiting (429) — check before 5xx so it gets distinct messaging - if (/\b429\b/.test(resultText) || /rate.limit/i.test(resultText)) { - return `${RATE_LIMITED_PREFIX}${resultText}`; - } - - // Detect service unavailability (API 500, upstream outage) - if (/\b50[0-9]\b/.test(resultText) || /server_error|internal_error|overloaded/.test(resultText)) { - return `${SERVICE_UNAVAILABLE_PREFIX}${resultText}`; + // One shared classifier owns the transient-vs-deterministic decision + // (failure-classifier.ts). This only maps its verdict onto the prefix + // protocol runAgent reads back below. + switch (classifyAgentFailure(resultText)) { + case 'rate_limited': + return `${RATE_LIMITED_PREFIX}${resultText}`; + case 'service_outage': + return `${SERVICE_UNAVAILABLE_PREFIX}${resultText}`; + case 'deterministic': + return `${DETERMINISTIC_PREFIX}${resultText}`; + default: + return resultText || 'Agent execution failed'; } - return resultText || 'Agent execution failed'; } if (message.subtype === 'success') { diff --git a/src/lib/credential-proxy.spec.ts b/src/lib/credential-proxy.spec.ts new file mode 100644 index 00000000..3c251bc0 --- /dev/null +++ b/src/lib/credential-proxy.spec.ts @@ -0,0 +1,107 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import http from 'node:http'; +import type { AddressInfo } from 'node:net'; + +vi.mock('../utils/debug.js', () => ({ + logInfo: vi.fn(), + logWarn: vi.fn(), + logError: vi.fn(), +})); + +vi.mock('../utils/analytics.js', () => ({ + analytics: { capture: vi.fn() }, +})); + +vi.mock('./credentials.js', () => ({ + getCredentials: vi.fn(() => ({ + accessToken: 'access-token', + expiresAt: Date.now() + 3_600_000, + userId: 'user_x', + })), + updateTokens: vi.fn(), +})); + +vi.mock('./token-refresh-client.js', () => ({ + refreshAccessToken: vi.fn(), +})); + +vi.mock('./host-probe.js', () => ({ + observeHostFailure: vi.fn(), +})); + +import { startCredentialProxy, startClaimTokenProxy, type CredentialProxyHandle } from './credential-proxy.js'; + +/** + * The socket timeout is the only knob standing between a long agent turn and a + * spurious 504. The gateway aggregates streams internally, so a non-streaming + * client can sit silent for minutes; 120s used to kill exactly those requests. + * Both factories are covered because the unclaimed-environment install path + * goes through the claim-token proxy, not the credential proxy. + */ +const EXPECTED_TIMEOUT_MS = 600_000; + +/** A throwaway upstream so the proxied request completes instead of hanging. */ +async function startUpstream(): Promise<{ url: string; close: () => Promise }> { + const server = http.createServer((_req, res) => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end('{"ok":true}'); + }); + + await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve())); + const { port } = server.address() as AddressInfo; + + return { + url: `http://127.0.0.1:${port}`, + close: () => new Promise((resolve) => server.close(() => resolve())), + }; +} + +describe('credential proxy upstream timeout', () => { + let capturedOptions: http.RequestOptions[]; + let upstream: Awaited>; + let proxy: CredentialProxyHandle | null = null; + + beforeEach(async () => { + capturedOptions = []; + upstream = await startUpstream(); + + // Capture what the proxy asks the transport for, then let the real request + // run so the response pipeline (and the test's fetch) actually completes. + const realRequest = http.request; + vi.spyOn(http, 'request').mockImplementation(((...args: Parameters) => { + capturedOptions.push(args[0] as http.RequestOptions); + return realRequest(...args); + }) as typeof http.request); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await proxy?.stop(); + proxy = null; + await upstream.close(); + }); + + it('startCredentialProxy forwards with a 600s socket timeout', async () => { + proxy = await startCredentialProxy({ upstreamUrl: upstream.url }); + + const res = await fetch(`${proxy.url}/v1/messages`, { method: 'POST', body: '{}' }); + + expect(res.status).toBe(200); + expect(capturedOptions).toHaveLength(1); + expect(capturedOptions[0].timeout).toBe(EXPECTED_TIMEOUT_MS); + }); + + it('startClaimTokenProxy forwards with a 600s socket timeout', async () => { + proxy = await startClaimTokenProxy({ + upstreamUrl: upstream.url, + claimToken: 'claim_xyz', + clientId: 'client_x', + }); + + const res = await fetch(`${proxy.url}/v1/messages`, { method: 'POST', body: '{}' }); + + expect(res.status).toBe(200); + expect(capturedOptions).toHaveLength(1); + expect(capturedOptions[0].timeout).toBe(EXPECTED_TIMEOUT_MS); + }); +}); diff --git a/src/lib/credential-proxy.ts b/src/lib/credential-proxy.ts index 606e0b99..f2d98141 100644 --- a/src/lib/credential-proxy.ts +++ b/src/lib/credential-proxy.ts @@ -44,6 +44,16 @@ export interface CredentialProxyHandle { stop: () => Promise; } +/** + * Socket inactivity timeout for gateway requests. Node's `timeout` option is + * already an idle timer, so this is a value change, not a mechanism change. + * 600s matches the Anthropic SDK's own 10-minute non-streaming ceiling. It must + * stay well above a single agent turn: when the gateway aggregates a stream + * internally, a non-streaming client receives no bytes until the turn completes, + * and the old 120s cut those requests off mid-flight. + */ +const UPSTREAM_IDLE_TIMEOUT_MS = 600_000; + // Hop-by-hop headers that must not be forwarded by proxies (RFC 7230 §6.1) const HOP_BY_HOP_HEADERS = new Set([ 'connection', @@ -311,7 +321,7 @@ async function handleRequest( path: finalPath, method: req.method, headers, - timeout: 120_000, // 2 minute timeout + timeout: UPSTREAM_IDLE_TIMEOUT_MS, }; const transport = useHttps ? https : http; @@ -398,7 +408,7 @@ export async function startClaimTokenProxy(options: { path: finalPath, method: req.method, headers, - timeout: 120_000, + timeout: UPSTREAM_IDLE_TIMEOUT_MS, }, (proxyRes) => { res.writeHead(proxyRes.statusCode || 500, filterHeaders(proxyRes.headers)); diff --git a/src/lib/env-writer.spec.ts b/src/lib/env-writer.spec.ts index 3af0878c..1c8aef55 100644 --- a/src/lib/env-writer.spec.ts +++ b/src/lib/env-writer.spec.ts @@ -1,8 +1,14 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { existsSync, readFileSync, unlinkSync, writeFileSync, mkdtempSync } from 'node:fs'; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { writeEnvLocal } from './env-writer.js'; +import { writeCredentialsEnv, writeEnvLocal } from './env-writer.js'; + +/** POSIX permission bits of `path`. */ +const modeOf = (path: string): number => statSync(path).mode & 0o777; + +// Windows has no meaningful POSIX permission bits. +const itPosix = it.skipIf(process.platform === 'win32'); describe('writeEnvLocal', () => { let testDir: string; @@ -12,12 +18,7 @@ describe('writeEnvLocal', () => { }); afterEach(() => { - for (const file of ['.env.local', '.gitignore']) { - const filePath = join(testDir, file); - if (existsSync(filePath)) { - unlinkSync(filePath); - } - } + rmSync(testDir, { recursive: true, force: true }); }); it('creates .env.local when none exists', () => { @@ -34,6 +35,16 @@ describe('writeEnvLocal', () => { expect(content).toContain('WORKOS_REDIRECT_URI=http://localhost:3000/callback'); }); + it('writes a fresh file with no leading blank line', () => { + writeEnvLocal(testDir, { + WORKOS_CLIENT_ID: 'client_123', + WORKOS_COOKIE_PASSWORD: 'pw', + }); + + const content = readFileSync(join(testDir, '.env.local'), 'utf-8'); + expect(content).toBe('WORKOS_CLIENT_ID=client_123\nWORKOS_COOKIE_PASSWORD=pw\n'); + }); + it('generates cookie password when not provided', () => { writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_123', @@ -55,6 +66,16 @@ describe('writeEnvLocal', () => { expect(content).toContain('WORKOS_COOKIE_PASSWORD=my-existing-password'); }); + it('does not regenerate a cookie password already in the file', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, 'WORKOS_COOKIE_PASSWORD=already_in_file\n'); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_123' }); + + const content = readFileSync(envPath, 'utf-8'); + expect(content).toBe('WORKOS_COOKIE_PASSWORD=already_in_file\nWORKOS_CLIENT_ID=client_123\n'); + }); + it('merges with existing .env.local without overwriting', () => { const envPath = join(testDir, '.env.local'); writeFileSync(envPath, 'EXISTING_VAR=existing_value\nOTHER_VAR=other\n'); @@ -90,50 +111,339 @@ describe('writeEnvLocal', () => { writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_123', - WORKOS_REDIRECT_URI: 'http://localhost:3000/callback', + WORKOS_COOKIE_PASSWORD: 'pw', }); const content = readFileSync(envPath, 'utf-8'); - expect(content).toContain('WORKOS_CLIENT_ID=client_123'); + expect(content).toBe('WORKOS_CLIENT_ID=client_123\nWORKOS_COOKIE_PASSWORD=pw\n'); }); - it('handles comments in existing .env file', () => { - const envPath = join(testDir, '.env.local'); - writeFileSync(envPath, '# This is a comment\nEXISTING=value\n# Another comment\n'); - + it('includes API key when provided', () => { writeEnvLocal(testDir, { + WORKOS_API_KEY: 'sk_test_123', WORKOS_CLIENT_ID: 'client_123', WORKOS_REDIRECT_URI: 'http://localhost:3000/callback', }); - const content = readFileSync(envPath, 'utf-8'); - expect(content).toContain('EXISTING=value'); - expect(content).toContain('WORKOS_CLIENT_ID=client_123'); - // Comments are not preserved in simple parser, which is fine + const content = readFileSync(join(testDir, '.env.local'), 'utf-8'); + expect(content).toContain('WORKOS_API_KEY=sk_test_123'); }); - it('handles values containing equals sign', () => { - const envPath = join(testDir, '.env.local'); - writeFileSync(envPath, 'KEY_WITH_EQUALS=value=with=equals\n'); + describe('line preservation', () => { + it('rewrites values in place, preserving comments, blanks, and key order', () => { + const envPath = join(testDir, '.env.local'); + const original = [ + '# WorkOS credentials', + '', + 'WORKOS_CLIENT_ID=client_old', + 'WORKOS_API_KEY=sk_old', + 'WORKOS_COOKIE_PASSWORD=keep_me', + '', + '# Unrelated app config', + 'DATABASE_URL=postgres://localhost/dev', + '', + ].join('\n'); + writeFileSync(envPath, original); + + writeEnvLocal(testDir, { + WORKOS_CLIENT_ID: 'client_new', + WORKOS_API_KEY: 'sk_new', + }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + [ + '# WorkOS credentials', + '', + 'WORKOS_CLIENT_ID=client_new', + 'WORKOS_API_KEY=sk_new', + 'WORKOS_COOKIE_PASSWORD=keep_me', + '', + '# Unrelated app config', + 'DATABASE_URL=postgres://localhost/dev', + '', + ].join('\n'), + ); + }); - writeEnvLocal(testDir, { + it('appends genuinely new keys at the end and touches nothing else', () => { + const envPath = join(testDir, '.env.local'); + const original = '# top\n\nDATABASE_URL=postgres://localhost/dev\n\n# bottom\n'; + writeFileSync(envPath, original); + + writeEnvLocal(testDir, { + WORKOS_CLIENT_ID: 'client_123', + WORKOS_COOKIE_PASSWORD: 'pw', + }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + original + 'WORKOS_CLIENT_ID=client_123\nWORKOS_COOKIE_PASSWORD=pw\n', + ); + }); + + it('preserves a comments-only file', () => { + const envPath = join(testDir, '.env.local'); + const original = '# This is a comment\n# Another comment\n'; + writeFileSync(envPath, original); + + writeEnvLocal(testDir, { + WORKOS_CLIENT_ID: 'client_123', + WORKOS_COOKIE_PASSWORD: 'pw', + }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + original + 'WORKOS_CLIENT_ID=client_123\nWORKOS_COOKIE_PASSWORD=pw\n', + ); + }); + + it('preserves values containing an equals sign', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, 'KEY_WITH_EQUALS=value=with=equals\nWORKOS_COOKIE_PASSWORD=a=b=c\n'); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_123' }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + 'KEY_WITH_EQUALS=value=with=equals\nWORKOS_COOKIE_PASSWORD=a=b=c\nWORKOS_CLIENT_ID=client_123\n', + ); + }); + + it('writes a value containing an equals sign verbatim', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, 'WORKOS_COOKIE_PASSWORD=old\n'); + + writeEnvLocal(testDir, { + WORKOS_CLIENT_ID: 'client_123', + WORKOS_COOKIE_PASSWORD: 'a=b=c', + }); + + expect(readFileSync(envPath, 'utf-8')).toBe('WORKOS_COOKIE_PASSWORD=a=b=c\nWORKOS_CLIENT_ID=client_123\n'); + }); + + // The writer has to recognize every line shape `readProjectEnvCredentials` + // treats as already-configured. Where it doesn't, the old line survives and + // the new value lands beneath it — the file then holds the key twice with + // conflicting values, which is how a stale key outlives its own update. + it('updates a key written with spaces around the equals sign, without duplicating it', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, 'WORKOS_CLIENT_ID = client_old\nWORKOS_COOKIE_PASSWORD=pw\n'); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_new' }); + + expect(readFileSync(envPath, 'utf-8')).toBe('WORKOS_CLIENT_ID=client_new\nWORKOS_COOKIE_PASSWORD=pw\n'); + }); + + it('updates an `export `-prefixed key in place, keeping the prefix', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync( + envPath, + '# top\nexport WORKOS_CLIENT_ID=client_old\nWORKOS_COOKIE_PASSWORD=pw\nDATABASE_URL=postgres://localhost/dev\n', + ); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_new' }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + '# top\nexport WORKOS_CLIENT_ID=client_new\nWORKOS_COOKIE_PASSWORD=pw\nDATABASE_URL=postgres://localhost/dev\n', + ); + }); + + it('rewrites an indented key exactly once', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, ' WORKOS_CLIENT_ID=client_old\nWORKOS_COOKIE_PASSWORD=pw\n'); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_new' }); + + const content = readFileSync(envPath, 'utf-8'); + expect(content.match(/^\s*WORKOS_CLIENT_ID=/gm)).toHaveLength(1); + expect(content).toBe('WORKOS_CLIENT_ID=client_new\nWORKOS_COOKIE_PASSWORD=pw\n'); + }); + + it('leaves a commented-out assignment commented, appending the real one', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, '# WORKOS_CLIENT_ID=do_not_touch\nWORKOS_COOKIE_PASSWORD=pw\n'); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_new' }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + '# WORKOS_CLIENT_ID=do_not_touch\nWORKOS_COOKIE_PASSWORD=pw\nWORKOS_CLIENT_ID=client_new\n', + ); + }); + + it('adds exactly one trailing newline to a file that lacked one', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, '# top\nWORKOS_COOKIE_PASSWORD=pw'); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_123' }); + + expect(readFileSync(envPath, 'utf-8')).toBe('# top\nWORKOS_COOKIE_PASSWORD=pw\nWORKOS_CLIENT_ID=client_123\n'); + }); + + it('keeps CRLF line endings on touched, untouched, and appended lines', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, '# top\r\nWORKOS_CLIENT_ID=client_old\r\nDATABASE_URL=postgres://localhost/dev\r\n'); + + writeEnvLocal(testDir, { + WORKOS_CLIENT_ID: 'client_new', + WORKOS_COOKIE_PASSWORD: 'pw', + }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + '# top\r\nWORKOS_CLIENT_ID=client_new\r\nDATABASE_URL=postgres://localhost/dev\r\nWORKOS_COOKIE_PASSWORD=pw\r\n', + ); + }); + + it('keeps CRLF endings on a file that lacked a trailing newline', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, '# top\r\nWORKOS_COOKIE_PASSWORD=pw'); + + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_123' }); + + expect(readFileSync(envPath, 'utf-8')).toBe( + '# top\r\nWORKOS_COOKIE_PASSWORD=pw\r\nWORKOS_CLIENT_ID=client_123\r\n', + ); + }); + }); + + describe('file permissions', () => { + const envVars = { WORKOS_CLIENT_ID: 'client_123', - WORKOS_REDIRECT_URI: 'http://localhost:3000/callback', + WORKOS_COOKIE_PASSWORD: 'pw', + }; + + itPosix('mirrors the source file mode onto the backup', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, 'WORKOS_API_KEY=sk_live_secret\n'); + chmodSync(envPath, 0o600); + + writeEnvLocal(testDir, envVars); + + expect(modeOf(join(testDir, '.env.local.bak'))).toBe(0o600); }); - const content = readFileSync(envPath, 'utf-8'); - expect(content).toContain('KEY_WITH_EQUALS=value=with=equals'); + itPosix('creates a new env file as 0600', () => { + writeEnvLocal(testDir, envVars); + + expect(modeOf(join(testDir, '.env.local'))).toBe(0o600); + }); + + itPosix('leaves the permissions of an existing env file alone', () => { + const envPath = join(testDir, '.env.local'); + writeFileSync(envPath, 'DATABASE_URL=postgres://localhost/dev\n'); + chmodSync(envPath, 0o640); + + writeEnvLocal(testDir, envVars); + + expect(modeOf(envPath)).toBe(0o640); + }); + + itPosix('creates a new .env as 0600 outside the JS branch', () => { + writeCredentialsEnv(testDir, envVars); + + expect(modeOf(join(testDir, '.env'))).toBe(0o600); + }); }); - it('includes API key when provided', () => { - writeEnvLocal(testDir, { - WORKOS_API_KEY: 'sk_test_123', + describe('backup', () => { + const envVars = { WORKOS_CLIENT_ID: 'client_123', - WORKOS_REDIRECT_URI: 'http://localhost:3000/callback', + WORKOS_COOKIE_PASSWORD: 'pw', + }; + + it('does not create a backup when no env file exists', () => { + writeEnvLocal(testDir, envVars); + + expect(existsSync(join(testDir, '.env.local.bak'))).toBe(false); }); - const content = readFileSync(join(testDir, '.env.local'), 'utf-8'); - expect(content).toContain('WORKOS_API_KEY=sk_test_123'); + it('backs up the pre-CLI file before the first mutation', () => { + const envPath = join(testDir, '.env.local'); + const original = '# mine\nDATABASE_URL=postgres://localhost/dev\n'; + writeFileSync(envPath, original); + + writeEnvLocal(testDir, envVars); + + expect(readFileSync(join(testDir, '.env.local.bak'), 'utf-8')).toBe(original); + }); + + it('keeps the original backup across two writes in the same run', () => { + const envPath = join(testDir, '.env.local'); + const original = '# mine\nDATABASE_URL=postgres://localhost/dev\n'; + writeFileSync(envPath, original); + + writeEnvLocal(testDir, envVars); + writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_456', WORKOS_COOKIE_PASSWORD: 'pw' }); + + // The backup is the pre-CLI file, not the state after the first write + expect(readFileSync(join(testDir, '.env.local.bak'), 'utf-8')).toBe(original); + expect(readFileSync(envPath, 'utf-8')).toContain('WORKOS_CLIENT_ID=client_456'); + }); + + it('never overwrites a backup from an earlier run', () => { + const envPath = join(testDir, '.env.local'); + const backupPath = join(testDir, '.env.local.bak'); + writeFileSync(envPath, 'WORKOS_CLIENT_ID=client_from_run_one\n'); + writeFileSync(backupPath, '# pristine from run one\n'); + + writeEnvLocal(testDir, envVars); + + expect(readFileSync(backupPath, 'utf-8')).toBe('# pristine from run one\n'); + }); + + it('git-ignores the backup before writing it, in a project with no .gitignore', () => { + writeFileSync(join(testDir, '.env.local'), 'DATABASE_URL=postgres://localhost/dev\n'); + + writeEnvLocal(testDir, envVars); + + expect(existsSync(join(testDir, '.env.local.bak'))).toBe(true); + expect(readFileSync(join(testDir, '.gitignore'), 'utf-8')).toBe('.env.local.bak\n.env.local\n'); + }); + + it('git-ignores the backup in a project whose .gitignore only has .env*.local', () => { + // The Next.js default: .env*.local covers .env.local but NOT .env.local.bak, + // and `git add -A` would otherwise commit a live API key and claim token. + const gitignorePath = join(testDir, '.gitignore'); + writeFileSync(gitignorePath, 'node_modules\n.env*.local\n'); + writeFileSync(join(testDir, '.env.local'), 'DATABASE_URL=postgres://localhost/dev\n'); + + writeEnvLocal(testDir, envVars); + + expect(existsSync(join(testDir, '.env.local.bak'))).toBe(true); + expect(readFileSync(gitignorePath, 'utf-8')).toBe('node_modules\n.env*.local\n.env.local.bak\n'); + }); + + it('leaves .gitignore alone when .env* already covers the backup', () => { + const gitignorePath = join(testDir, '.gitignore'); + writeFileSync(gitignorePath, '.env*\n'); + writeFileSync(join(testDir, '.env.local'), 'DATABASE_URL=postgres://localhost/dev\n'); + + writeEnvLocal(testDir, envVars); + + expect(existsSync(join(testDir, '.env.local.bak'))).toBe(true); + expect(readFileSync(gitignorePath, 'utf-8')).toBe('.env*\n'); + }); + + it('leaves .gitignore alone when *.bak already covers the backup', () => { + const gitignorePath = join(testDir, '.gitignore'); + writeFileSync(gitignorePath, '*.bak\n.env.local\n'); + writeFileSync(join(testDir, '.env.local'), 'DATABASE_URL=postgres://localhost/dev\n'); + + writeEnvLocal(testDir, envVars); + + expect(existsSync(join(testDir, '.env.local.bak'))).toBe(true); + expect(readFileSync(gitignorePath, 'utf-8')).toBe('*.bak\n.env.local\n'); + }); + + it('does not write the backup when .gitignore cannot be updated', () => { + // .gitignore as a directory makes the read/write throw. Proves ordering: + // an un-ignorable backup must never reach disk. + mkdirSync(join(testDir, '.gitignore')); + const original = 'WORKOS_API_KEY=sk_live_secret\n'; + writeFileSync(join(testDir, '.env.local'), original); + + expect(() => writeEnvLocal(testDir, envVars)).toThrow(); + + expect(existsSync(join(testDir, '.env.local.bak'))).toBe(false); + // The env write happens after the backup, so the user's file is intact + expect(readFileSync(join(testDir, '.env.local'), 'utf-8')).toBe(original); + }); }); describe('gitignore handling', () => { @@ -214,3 +524,78 @@ describe('writeEnvLocal', () => { }); }); }); + +describe('writeCredentialsEnv', () => { + let testDir: string; + + beforeEach(() => { + testDir = mkdtempSync(join(tmpdir(), 'env-writer-cred-test-')); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + }); + + const envVars = { + WORKOS_API_KEY: 'sk_test_new', + WORKOS_CLIENT_ID: 'client_new', + }; + + it('delegates to .env.local when package.json is present', () => { + writeFileSync(join(testDir, 'package.json'), '{}\n'); + + writeCredentialsEnv(testDir, envVars); + + expect(existsSync(join(testDir, '.env.local'))).toBe(true); + expect(existsSync(join(testDir, '.env'))).toBe(false); + }); + + it('rewrites .env values in place, preserving comments, blanks, and order', () => { + const envPath = join(testDir, '.env'); + const original = [ + '# WorkOS', + '', + 'WORKOS_CLIENT_ID=client_old', + 'WORKOS_API_KEY=sk_old', + '', + '# Django', + 'DJANGO_SECRET_KEY=abc=def', + '', + ].join('\n'); + writeFileSync(envPath, original); + + writeCredentialsEnv(testDir, envVars); + + expect(readFileSync(envPath, 'utf-8')).toBe( + [ + '# WorkOS', + '', + 'WORKOS_CLIENT_ID=client_new', + 'WORKOS_API_KEY=sk_test_new', + '', + '# Django', + 'DJANGO_SECRET_KEY=abc=def', + '', + ].join('\n'), + ); + }); + + it('does not generate a cookie password outside the JS branch', () => { + writeCredentialsEnv(testDir, envVars); + + const content = readFileSync(join(testDir, '.env'), 'utf-8'); + expect(content).toBe('WORKOS_API_KEY=sk_test_new\nWORKOS_CLIENT_ID=client_new\n'); + }); + + it('backs up .env once and git-ignores the backup', () => { + const envPath = join(testDir, '.env'); + const original = '# mine\nDJANGO_SECRET_KEY=abc\n'; + writeFileSync(envPath, original); + + writeCredentialsEnv(testDir, envVars); + writeCredentialsEnv(testDir, { WORKOS_CLIENT_ID: 'client_third' }); + + expect(readFileSync(join(testDir, '.env.bak'), 'utf-8')).toBe(original); + expect(readFileSync(join(testDir, '.gitignore'), 'utf-8')).toBe('.env.bak\n.env\n'); + }); +}); diff --git a/src/lib/env-writer.ts b/src/lib/env-writer.ts index 9f07ad9d..a7c67a38 100644 --- a/src/lib/env-writer.ts +++ b/src/lib/env-writer.ts @@ -1,18 +1,17 @@ -import { existsSync, readFileSync, writeFileSync } from 'fs'; -import { join } from 'path'; +import { existsSync, readFileSync, statSync, writeFileSync } from 'fs'; +import { basename, join } from 'path'; import { parseEnvFile } from '../utils/env-parser.js'; const ENV_LOCAL_COVERING_PATTERNS = ['.env.local', '.env*.local', '.env*']; const ENV_COVERING_PATTERNS = ['.env', '.env*']; /** - * Ensure the given env filename is in .gitignore. + * Ensure the given filename is in .gitignore. * Creates .gitignore if it doesn't exist. - * No-ops if a covering pattern is already present. + * No-ops if one of `coveringPatterns` is already present. */ -function ensureGitignore(installDir: string, filename: '.env' | '.env.local'): void { +function ensureGitignore(installDir: string, filename: string, coveringPatterns: string[]): void { const gitignorePath = join(installDir, '.gitignore'); - const coveringPatterns = filename === '.env' ? ENV_COVERING_PATTERNS : ENV_LOCAL_COVERING_PATTERNS; if (!existsSync(gitignorePath)) { writeFileSync(gitignorePath, `${filename}\n`); @@ -50,37 +49,140 @@ function generateCookiePassword(): string { return Array.from(array, (byte) => byte.toString(16).padStart(2, '0')).join(''); } +// Captures the optional `export ` prefix (group 1) and the variable name (group 2) +// of an assignment line. Deliberately mirrors the patterns in `lib/project-env.ts`. +const ENV_ASSIGNMENT = /^[ \t]*(export[ \t]+)?([A-Za-z_][A-Za-z0-9_]*)[ \t]*=/; + +/** + * Set `vars` in `content` without disturbing comments, blank lines, or key order. + * Existing keys are rewritten in place; new keys are appended. + * + * An existing key is rewritten as `key=value` with no leading whitespace even if + * the original line was indented — indented env lines are vanishingly rare and + * preserving the indent adds branching for no real benefit. + * + * Key extraction recognizes exactly the line shapes `readProjectEnvCredentials` + * (`lib/project-env.ts`) recognizes — optional indentation, an optional `export ` + * prefix, and whitespace around `=`. The two must agree: a line the reader counts + * as configured but the writer fails to match would be left holding its old value + * while the new one is appended below it, so the file would carry the key twice + * with conflicting values. Only the text up to the first `=` is matched, so + * values containing `=` are irrelevant here. + * + * Duplicate keys in a malformed source file: only the first occurrence is + * rewritten (`parseEnvFile` reads last-wins, so a duplicate still shadows the + * update — a pre-existing pathology, not handled here). + * + * The file's dominant line terminator is detected once and reused, so a CRLF + * file is not silently rewritten to LF (which would turn a two-line change into + * a whole-file diff and leave mixed endings behind). + */ +function upsertEnvLines(content: string, vars: Record): string { + const eol = content.includes('\r\n') ? '\r\n' : '\n'; + const body = content.endsWith('\r\n') + ? content.slice(0, -2) + : content.endsWith('\n') + ? content.slice(0, -1) + : content; + const pending = new Map(Object.entries(vars)); + + // An empty body has no lines at all — splitting it would invent a blank one. + const lines = + body === '' + ? [] + : body.split(/\r?\n/).map((line) => { + // Blank lines and `#` comments cannot match: only spaces and tabs may + // precede the name, so no separate guard for them is needed. + const match = ENV_ASSIGNMENT.exec(line); + if (!match) return line; + const [, exportPrefix, key] = match; + if (!pending.has(key)) return line; + const value = pending.get(key)!; + pending.delete(key); + // Keep `export ` — dropping it would stop the var being exported when + // the file is sourced, which is why the author wrote it. + return `${exportPrefix ?? ''}${key}=${value}`; + }); + + for (const [key, value] of pending) { + lines.push(`${key}=${value}`); + } + + // Always exactly one trailing newline, matching the previous writer. + return lines.join(eol) + eol; +} + +/** + * Write a secrets-bearing file, birthing a NEW file as 0600. + * + * `mode` is only honored on create, so an existing file's permissions are the + * user's business and are left exactly as they are. + */ +function writeSecretFile(path: string, contents: string, existed: boolean): void { + writeFileSync(path, contents, existed ? {} : { mode: 0o600 }); +} + +/** + * Copy `envPath` to `{envPath}.bak` before the first CLI mutation. + * + * Stateless: existence of the backup IS the "already backed up" flag, so a + * second write in the same run — or a later run — never overwrites the original + * pre-CLI file. Deliberately not a module-level flag: this module is otherwise + * pure, and the writers have four call sites across the codebase. + * + * `ensureGitignore` runs BEFORE the write so a crash between the two cannot + * leave an unignored secret on disk: `stageAndCommit` runs `git add -A`, and the + * env file holds a live API key and claim token. + * + * The copy mirrors the source's permission bits: a `chmod 600 .env.local` must + * not gain a world-readable twin, and `.bak` sits outside the `.env*` glob most + * secret-scanning and editor-hiding tooling watches. The `existsSync` guard + * above guarantees this write is always a create, so `mode` is honored. + */ +function backupEnvFile(installDir: string, envPath: string): void { + if (!existsSync(envPath)) return; // nothing to back up + const backupPath = `${envPath}.bak`; + if (existsSync(backupPath)) return; // never overwrite an earlier backup + + const backupName = basename(backupPath); + ensureGitignore(installDir, backupName, [backupName, '.env*', '*.bak']); + writeFileSync(backupPath, readFileSync(envPath, 'utf-8'), { mode: statSync(envPath).mode & 0o777 }); +} + +/** Drop keys whose value is undefined so they never serialize as `KEY=undefined`. */ +function definedVars(envVars: Partial): Record { + const result: Record = {}; + for (const [key, value] of Object.entries(envVars)) { + if (value !== undefined) result[key] = value; + } + return result; +} + /** * Write environment variables to .env.local before agent runs. - * Merges with existing .env.local if present (new vars take precedence). - * Auto-generates WORKOS_COOKIE_PASSWORD if not provided. + * Upserts into an existing .env.local if present (new vars take precedence), + * preserving comments, blank lines, and key order. + * Auto-generates WORKOS_COOKIE_PASSWORD if not already set. */ export function writeEnvLocal(installDir: string, envVars: Partial): void { const envPath = join(installDir, '.env.local'); - // Read existing env if present - let existingEnv: Record = {}; - if (existsSync(envPath)) { - const content = readFileSync(envPath, 'utf-8'); - existingEnv = parseEnvFile(content); - } + backupEnvFile(installDir, envPath); - // Merge with new vars (new vars take precedence) - const merged = { ...existingEnv, ...envVars }; + const envExisted = existsSync(envPath); + const existingContent = envExisted ? readFileSync(envPath, 'utf-8') : ''; + const existingEnv = parseEnvFile(existingContent); - // Generate cookie password if not provided - if (!merged.WORKOS_COOKIE_PASSWORD) { - merged.WORKOS_COOKIE_PASSWORD = generateCookiePassword(); - } + const vars = definedVars(envVars); - // Write back - const content = Object.entries(merged) - .map(([key, value]) => `${key}=${value}`) - .join('\n'); + // Generate cookie password only when neither the caller nor the file has one + if (!vars.WORKOS_COOKIE_PASSWORD && !existingEnv.WORKOS_COOKIE_PASSWORD) { + vars.WORKOS_COOKIE_PASSWORD = generateCookiePassword(); + } - ensureGitignore(installDir, '.env.local'); + ensureGitignore(installDir, '.env.local', ENV_LOCAL_COVERING_PATTERNS); - writeFileSync(envPath, content + '\n'); + writeSecretFile(envPath, upsertEnvLines(existingContent, vars), envExisted); } /** @@ -91,6 +193,11 @@ export function writeEnvLocal(installDir: string, envVars: Partial): vo * * Used by pre-detection flows that write credentials before the framework * integration is known (unclaimed env provisioning). + * + * KEEP IN SYNC: `resolveProjectEnvPath` in ./project-env.ts mirrors the + * package.json test below so the no-clobber check reads the same file this + * writes. If they disagree, the installer can check one file and overwrite + * another — the original bug. */ export function writeCredentialsEnv(installDir: string, envVars: Partial): void { const hasPackageJson = existsSync(join(installDir, 'package.json')); @@ -100,18 +207,13 @@ export function writeCredentialsEnv(installDir: string, envVars: Partial = {}; - if (existsSync(envPath)) { - const content = readFileSync(envPath, 'utf-8'); - existingEnv = parseEnvFile(content); - } - const merged = { ...existingEnv, ...envVars }; - const content = Object.entries(merged) - .map(([key, value]) => `${key}=${value}`) - .join('\n'); + backupEnvFile(installDir, envPath); + + const envExisted = existsSync(envPath); + const existingContent = envExisted ? readFileSync(envPath, 'utf-8') : ''; - ensureGitignore(installDir, '.env'); + ensureGitignore(installDir, '.env', ENV_COVERING_PATTERNS); - writeFileSync(envPath, content + '\n'); + writeSecretFile(envPath, upsertEnvLines(existingContent, definedVars(envVars)), envExisted); } diff --git a/src/lib/failure-classifier.spec.ts b/src/lib/failure-classifier.spec.ts new file mode 100644 index 00000000..b70738de --- /dev/null +++ b/src/lib/failure-classifier.spec.ts @@ -0,0 +1,102 @@ +import { describe, it, expect } from 'vitest'; +import { + classifyAgentFailure, + describeAgentFailure, + formatAgentFailure, + type AgentFailureKind, +} from './failure-classifier.js'; + +/** + * The verbatim strings from the incident, plus the neighbouring shapes that + * must NOT reclassify. The ordering inside classifyAgentFailure is the entire + * fix: every `deterministic` row below also matches a transient pattern, so a + * careless reorder turns them all back into "try again in a few minutes". + */ +const CASES: Array<[message: string, expected: AgentFailureKind]> = [ + // --- The incident. Both of these carry 5xx-shaped text. --- + ['API Error: 500 {"error":{"type":"internal_error","message":"An unexpected error occurred"}}', 'deterministic'], + ['API Error: 500 An unexpected error occurred (request id: req_x)', 'deterministic'], + ['{"error":"upstream_timeout","message":"Upstream server timed out"}', 'deterministic'], + ['504 Upstream server timed out', 'deterministic'], + ['Streaming is strongly recommended for operations that may take longer than 10 minutes', 'deterministic'], + ['400 max_tokens: 64000 > 32000, which is the maximum allowed', 'deterministic'], + + // --- Genuinely transient. This copy is correct and must not regress. --- + ['503 Service Unavailable', 'service_outage'], + ['{"type":"overloaded_error","message":"Overloaded"}', 'service_outage'], + ['server_error: service overloaded', 'service_outage'], + ['API Error: 502 Bad Gateway', 'service_outage'], + + // --- Rate limiting, checked before any 5xx branch. --- + ['API Error: 429 rate_limit_exceeded', 'rate_limited'], + ['429 Too Many Requests', 'rate_limited'], + ['This organization has exceeded its rate limit', 'rate_limited'], + + // --- Everything else keeps its previous verdict. --- + ['fetch failed', 'network'], + ['connect ECONNREFUSED 127.0.0.1:8000', 'network'], + ['process exited with code 1', 'process_exit'], + ['API Error: 401 unauthorized', 'auth'], + ["ENOENT: no such file or directory, open '/proj/package.json'", 'missing_path'], + ['Some other failure', 'unknown'], + ['', 'unknown'], +]; + +describe('classifyAgentFailure', () => { + for (const [message, expected] of CASES) { + it(`classifies ${JSON.stringify(message.slice(0, 60))} as ${expected}`, () => { + expect(classifyAgentFailure(message)).toBe(expected); + }); + } + + it('prefers rate_limited when a message carries both 429 and 500', () => { + expect(classifyAgentFailure('API Error: 429 after retrying a 500 An unexpected error occurred')).toBe( + 'rate_limited', + ); + }); + + it('prefers deterministic when a message carries both the generic-500 text and internal_error', () => { + // This is the exact shape that used to fall into service_outage: the + // deterministic phrase and the transient token arrive in the same body. + expect(classifyAgentFailure('500 internal_error: An unexpected error occurred')).toBe('deterministic'); + }); + + it('does not read "author" as an auth failure or "Module not found" as a missing path', () => { + expect(classifyAgentFailure('author field is required in package.json')).toBe('unknown'); + expect(classifyAgentFailure('Module not found: Cannot resolve "@workos-inc/authkit-nextjs"')).toBe('unknown'); + }); +}); + +describe('describeAgentFailure', () => { + it('never advises waiting for a deterministic failure', () => { + const { headline, detail } = describeAgentFailure('deterministic'); + + expect(headline).not.toMatch(/temporarily unavailable/i); + expect(`${headline} ${detail}`).not.toMatch(/few minutes|try again shortly|wait a minute/i); + expect(detail).toMatch(/--debug/); + }); + + it('keeps the transient copy for a real outage', () => { + expect(describeAgentFailure('service_outage').headline).toMatch(/temporarily unavailable/i); + expect(describeAgentFailure('service_outage').detail).toMatch(/few minutes/i); + }); + + it('falls back to the raw message for kinds with no better copy', () => { + expect(describeAgentFailure('unknown').headline).toBeNull(); + expect(describeAgentFailure('missing_path').headline).toBeNull(); + expect(formatAgentFailure('unknown', 'Something broke')).toBe('Something broke'); + }); + + /** + * installer-core's emitError re-emits the *rendered* message onto the `error` + * event, so the adapters classify our own copy rather than the raw SDK text. + * If rendering does not round-trip, the deterministic verdict is lost between + * runAgent and the terminal — which is the bug this phase exists to fix. + */ + it.each(['deterministic', 'service_outage', 'rate_limited'] as const)( + 'round-trips its own rendered copy for %s', + (kind) => { + expect(classifyAgentFailure(formatAgentFailure(kind, 'raw'))).toBe(kind); + }, + ); +}); diff --git a/src/lib/failure-classifier.ts b/src/lib/failure-classifier.ts new file mode 100644 index 00000000..b2be5270 --- /dev/null +++ b/src/lib/failure-classifier.ts @@ -0,0 +1,144 @@ +/** + * Single source of truth for what an agent failure means and what to tell the + * user about it. + * + * Three call sites used to re-derive this independently — `handleSDKMessage` in + * `agent-interface.ts`, `CLIAdapter.handleError`, `HeadlessAdapter.handleError` — + * each with its own copy of a "5xx means transient" regex. All three were wrong + * about the same thing: the WorkOS LLM gateway returns a generic 500 for + * failures that are entirely deterministic (its `toApiErrorResponse` fallback + * fires whenever the Anthropic SDK throws a status-less error, including the + * `max_tokens` guard), so "try again in a few minutes" sent users round a + * four-minute loop that could never succeed. + */ + +import { formatWorkOSCommand } from '../utils/command-invocation.js'; + +export type AgentFailureKind = + /** Retry helps, after a wait. */ + | 'rate_limited' + /** Retry helps; a transient upstream problem. */ + | 'service_outage' + /** Retry does NOT help; same input, same failure. */ + | 'deterministic' + | 'network' + | 'process_exit' + | 'auth' + | 'missing_path' + | 'unknown'; + +/** + * Headline for the `deterministic` kind, kept as a named constant because + * `classifyAgentFailure` also matches it: `installer-core`'s `emitError` + * re-emits the *already rendered* message onto the `error` event, so the + * adapters classify our own copy. Rendering has to round-trip or the + * deterministic verdict is silently lost on the way to the terminal. + */ +const DETERMINISTIC_HEADLINE = 'The AI service could not complete this request.'; + +/** + * Classify an agent failure message. The key distinction is whether retrying + * can plausibly succeed. A gateway 500 is NOT automatically transient: the + * WorkOS LLM gateway returns a generic 500 for client-side SDK validation + * failures (`llm-gateway.controller.ts` `toApiErrorResponse`), which recur on + * every attempt. + * + * Ordering is the whole design. The deterministic patterns are tested *before* + * the generic 5xx branch; flip them and the gateway's generic-500 text falls + * back into `service_outage` exactly as it did before this function existed. + * + * Matching is word-boundary / code-based so `author` does not read as `auth` + * and `Module not found` does not read as a missing directory. + */ +export function classifyAgentFailure(message: string): AgentFailureKind { + // Rate limiting first — it is a 429, and it is the one case with specific advice. + if (/\b429\b/.test(message) || /\brate.?limit/i.test(message)) return 'rate_limited'; + + // Deterministic failures that WEAR a 5xx. Must stay above the generic 5xx branch. + // + // "An unexpected error occurred" is the gateway's generic-branch message + // (llm-gateway.controller.ts, toApiErrorResponse) and is the precise + // signature of "the SDK threw something with no status". + if (/an unexpected error occurred/i.test(message)) return 'deterministic'; + // The proxy's own 504 body (credential-proxy.ts). A request that blew the + // socket timeout will blow it again; that is not an outage. + if (/upstream_timeout|upstream server timed out/i.test(message)) return 'deterministic'; + // Request-shape rejections from the Anthropic SDK / gateway guard rails. + if (/streaming is (strongly recommended|required)|max_tokens/i.test(message)) return 'deterministic'; + // Our own rendered copy, so a re-classification downstream round-trips. + if (message.includes(DETERMINISTIC_HEADLINE)) return 'deterministic'; + + // Genuinely transient upstream states. + if (/overloaded/i.test(message) || /\b503\b/.test(message)) return 'service_outage'; + if (/server_error|internal_error|service.*unavailable/i.test(message)) return 'service_outage'; + if (/\b50[0-9]\b/.test(message)) return 'service_outage'; + + if (/ECONNREFUSED|ETIMEDOUT|ENOTFOUND|fetch failed/i.test(message)) return 'network'; + if (/process exited with code/i.test(message)) return 'process_exit'; + if (/\b(401|403|unauthorized|forbidden|authentication|authorization)\b/i.test(message)) return 'auth'; + if (/\bENOENT\b/.test(message)) return 'missing_path'; + return 'unknown'; +} + +/** + * User-facing copy per kind. Adapters render these; they do not re-derive them. + * + * A `null` headline means "there is nothing better to say than the raw message" — + * the caller renders the message it was given. A `null` detail means the caller + * supplies its own next step (the CLI adapter's `--debug` hint). + */ +export function describeAgentFailure(kind: AgentFailureKind): { headline: string | null; detail: string | null } { + switch (kind) { + case 'rate_limited': + return { + headline: 'The AI service is currently rate-limited.', + detail: 'Please wait a minute and try again.', + }; + case 'service_outage': + return { + headline: 'The AI service is temporarily unavailable.', + detail: 'This is usually resolved within a few minutes. Please try again shortly.', + }; + case 'deterministic': + // Neither "wait a few minutes" (it will fail identically) nor a flat "do + // not retry" (a real outage can wear this shape) — say what is true. + return { + headline: DETERMINISTIC_HEADLINE, + detail: + 'The same request is likely to fail the same way, so waiting will not help. Re-run with --debug to see the underlying error.', + }; + case 'network': + return { + headline: 'Could not connect to the AI service.', + detail: 'Check your internet connection and try again.', + }; + case 'process_exit': + return { + headline: 'The AI agent process exited unexpectedly.', + detail: 'Try running again. If this persists, run with --debug for details.', + }; + case 'auth': + return { + headline: 'Authentication failed.', + detail: `Try running: ${formatWorkOSCommand('auth logout')} && ${formatWorkOSCommand('install')}`, + }; + case 'missing_path': + return { + headline: null, + detail: 'Make sure you are running this in your project directory.', + }; + case 'unknown': + return { headline: null, detail: null }; + } +} + +/** + * Flatten a classified failure into one string, for the paths that carry a + * single error message (`runAgent`'s `errorMessage`, the headless NDJSON + * `message`) rather than a headline plus a hint. + */ +export function formatAgentFailure(kind: AgentFailureKind, message: string): string { + const { headline, detail } = describeAgentFailure(kind); + if (!headline) return message; + return detail ? `${headline} ${detail}` : headline; +} diff --git a/src/lib/installer-core.ts b/src/lib/installer-core.ts index 59d97d34..07e8e216 100644 --- a/src/lib/installer-core.ts +++ b/src/lib/installer-core.ts @@ -233,7 +233,11 @@ export const installerMachine = setup({ context.emitter.emit('complete', { success: false, summary: 'Installer cancelled by user' }); }, emitError: ({ context }) => { - const message = context.error?.message ?? 'An unexpected error occurred'; + // Must NOT be 'An unexpected error occurred': that is the LLM gateway's + // generic-500 signature, which the failure classifier reads as a + // deterministic AI failure. A git/fs/detection error reaching the error + // state without an assigned `error` would otherwise be blamed on the AI. + const message = context.error?.message ?? 'The installer failed for an unknown reason'; context.emitter.emit('error', { message, stack: context.error?.stack }); context.emitter.emit('complete', { success: false }); }, diff --git a/src/lib/preflight-authkit.spec.ts b/src/lib/preflight-authkit.spec.ts new file mode 100644 index 00000000..7aa3be2d --- /dev/null +++ b/src/lib/preflight-authkit.spec.ts @@ -0,0 +1,239 @@ +import { mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { CliExit } from '../utils/cli-exit.js'; +import { resetInteractionModeForTests, setInteractionMode } from '../utils/interaction-mode.js'; +import { setOutputMode } from '../utils/output.js'; + +// Mock the UI facade — the interactive branch prompts, which has no place in a unit test. +const mockConfirm = vi.fn(); +const mockUi = { + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), step: vi.fn(), success: vi.fn() }, + rows: vi.fn(), + confirm: (...args: unknown[]) => mockConfirm(...args), + // Mirrors the real facade: only the CANCEL symbol counts as a cancellation. + isCancel: (value: unknown) => typeof value === 'symbol', +}; +vi.mock('../utils/ui.js', () => ({ default: mockUi })); + +const { assertNoExistingAuthKit, detectExistingAuthKit } = await import('./preflight-authkit.js'); + +function writePackageJson(dir: string, deps: Record, devDeps?: Record): void { + writeFileSync( + join(dir, 'package.json'), + JSON.stringify({ name: 'fixture', dependencies: deps, devDependencies: devDeps ?? {} }), + ); +} + +describe('preflight-authkit', () => { + let testDir: string; + let errorSpy: ReturnType; + + beforeEach(() => { + testDir = mkdtempSync(join(tmpdir(), 'preflight-authkit-test-')); + vi.clearAllMocks(); + resetInteractionModeForTests(); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + resetInteractionModeForTests(); + setOutputMode('human'); + errorSpy.mockRestore(); + }); + + describe('detectExistingAuthKit', () => { + it('returns [] when the project has no package.json', () => { + expect(detectExistingAuthKit(testDir)).toEqual([]); + }); + + it('returns [] when package.json is malformed (parse failure must not block a run)', () => { + writeFileSync(join(testDir, 'package.json'), '{ not json'); + + expect(detectExistingAuthKit(testDir)).toEqual([]); + }); + + it('returns [] for a framework project with no WorkOS packages', () => { + writePackageJson(testDir, { next: '15.0.0', react: '19.0.0' }); + + expect(detectExistingAuthKit(testDir)).toEqual([]); + }); + + it('detects an installed AuthKit SDK with its version range', () => { + writePackageJson(testDir, { next: '15.0.0', '@workos-inc/authkit-nextjs': '^2.6.0' }); + + expect(detectExistingAuthKit(testDir)).toEqual([{ name: '@workos-inc/authkit-nextjs', version: '^2.6.0' }]); + }); + + it('detects AuthKit declared as a devDependency', () => { + writePackageJson(testDir, { next: '15.0.0' }, { '@workos-inc/authkit-js': '1.0.0' }); + + expect(detectExistingAuthKit(testDir)).toEqual([{ name: '@workos-inc/authkit-js', version: '1.0.0' }]); + }); + + // False-positive guard: the base SDK is in plenty of projects that have + // never installed AuthKit, and those must stay installable. + it('does NOT trip on @workos-inc/node alone', () => { + writePackageJson(testDir, { '@workos-inc/node': '^7.0.0' }); + + expect(detectExistingAuthKit(testDir)).toEqual([]); + }); + + it('does NOT trip on the legacy workos package alone', () => { + writePackageJson(testDir, { workos: '^4.0.0' }); + + expect(detectExistingAuthKit(testDir)).toEqual([]); + }); + + it('ignores @workos-inc/node when it sits alongside AuthKit', () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0', '@workos-inc/node': '^7.0.0' }); + + expect(detectExistingAuthKit(testDir)).toEqual([{ name: '@workos-inc/authkit-nextjs', version: '^2.6.0' }]); + }); + + it('lists every AuthKit SDK found, not just the first', () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0', '@workos-inc/authkit-react': '^1.2.0' }); + + expect(detectExistingAuthKit(testDir)).toEqual([ + { name: '@workos-inc/authkit-nextjs', version: '^2.6.0' }, + { name: '@workos-inc/authkit-react', version: '^1.2.0' }, + ]); + }); + }); + + describe('assertNoExistingAuthKit', () => { + it('is a no-op when no AuthKit is present', async () => { + writePackageJson(testDir, { next: '15.0.0' }); + + await expect(assertNoExistingAuthKit({ installDir: testDir })).resolves.toBeUndefined(); + expect(mockConfirm).not.toHaveBeenCalled(); + }); + + it('throws CliExit (exit 1) naming the packages and doctor in non-interactive mode', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + setInteractionMode({ mode: 'agent', source: 'env' }); + + const error = await assertNoExistingAuthKit({ installDir: testDir }).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(CliExit); + const exit = error as InstanceType; + expect(exit.exitCode).toBe(1); + expect(exit.context?.errorCode).toBe('authkit_already_installed'); + expect(exit.context?.reason).toBe('validation_error'); + + const printed = errorSpy.mock.calls.map((call) => String(call[0])).join('\n'); + expect(printed).toContain('@workos-inc/authkit-nextjs ^2.6.0'); + expect(printed).toContain('doctor'); + expect(printed).toContain('--force'); + // Never prompt in a non-interactive session. + expect(mockConfirm).not.toHaveBeenCalled(); + }); + + // --json on a real TTY: output mode is json while interaction mode stays + // human, so isPromptAllowed() alone is not enough of a gate. Prompting here + // would put plain text in the JSON stream and report prompt_unavailable. + it('refuses without prompting or printing under --json on a TTY (interaction mode human)', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + setInteractionMode({ mode: 'human', source: 'default' }); + setOutputMode('json'); + + const error = await assertNoExistingAuthKit({ installDir: testDir }).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(CliExit); + expect((error as InstanceType).context?.errorCode).toBe('authkit_already_installed'); + expect(mockConfirm).not.toHaveBeenCalled(); + expect(mockUi.log.warn).not.toHaveBeenCalled(); + expect(mockUi.log.info).not.toHaveBeenCalled(); + expect(mockUi.rows).not.toHaveBeenCalled(); + }); + + it('also refuses in CI mode', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-remix': '^1.0.0' }); + setInteractionMode({ mode: 'ci', source: 'ci_env' }); + + await expect(assertNoExistingAuthKit({ installDir: testDir })).rejects.toBeInstanceOf(CliExit); + }); + + it('returns without throwing when force is set in non-interactive mode', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + setInteractionMode({ mode: 'agent', source: 'env' }); + + await expect(assertNoExistingAuthKit({ installDir: testDir, force: true })).resolves.toBeUndefined(); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it('does not even read package.json when force is set', async () => { + // force short-circuits before detection, so a malformed manifest is irrelevant. + writeFileSync(join(testDir, 'package.json'), '{ not json'); + + await expect(assertNoExistingAuthKit({ installDir: testDir, force: true })).resolves.toBeUndefined(); + }); + + it('returns normally when an interactive user accepts', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + mockConfirm.mockResolvedValueOnce(true); + + await expect(assertNoExistingAuthKit({ installDir: testDir })).resolves.toBeUndefined(); + expect(mockConfirm).toHaveBeenCalledTimes(1); + expect(mockUi.log.warn).toHaveBeenCalledWith(expect.stringContaining('already installed')); + expect(mockUi.rows).toHaveBeenCalledWith([ + { key: '@workos-inc/authkit-nextjs', value: '^2.6.0', statusKind: 'muted' }, + ]); + // Names the exact file at risk — .env.local here, since package.json exists. + expect(mockUi.log.info).toHaveBeenCalledWith(expect.stringContaining(join(testDir, '.env.local'))); + }); + + // The consent copy must not promise an outcome that cannot happen: with a key + // already on disk, credential resolution refuses to provision and logs in. + it('promises credentials will be KEPT (not rewritten) when the project env already has a key', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + writeFileSync(join(testDir, '.env.local'), 'WORKOS_API_KEY=sk_existing\n'); + mockConfirm.mockResolvedValueOnce(true); + + await expect(assertNoExistingAuthKit({ installDir: testDir })).resolves.toBeUndefined(); + + const printed = mockUi.log.info.mock.calls.map((call) => String(call[0])).join('\n'); + expect(printed).toContain('will be kept'); + expect(printed).not.toContain('provision a new WorkOS environment'); + }); + + it('names the file the key actually lives in, even when it is not the write target', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + // JS project, so the write target is .env.local — but the real key is in .env. + writeFileSync(join(testDir, '.env'), 'WORKOS_API_KEY=sk_existing\n'); + mockConfirm.mockResolvedValueOnce(true); + + await expect(assertNoExistingAuthKit({ installDir: testDir })).resolves.toBeUndefined(); + + const printed = mockUi.log.info.mock.calls.map((call) => String(call[0])).join('\n'); + expect(printed).toContain(join(testDir, '.env')); + expect(printed).not.toContain(join(testDir, '.env.local')); + }); + + it('throws CliExit (exit 2) and writes nothing when an interactive user declines', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + mockConfirm.mockResolvedValueOnce(false); + + const error = await assertNoExistingAuthKit({ installDir: testDir }).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(CliExit); + const exit = error as InstanceType; + expect(exit.exitCode).toBe(2); + expect(exit.context?.reason).toBe('cancelled'); + // No env file, no .gitignore — the guard runs before anything is written. + expect(readdirSync(testDir)).toEqual(['package.json']); + }); + + it('throws CliExit (exit 2) when the prompt is cancelled', async () => { + writePackageJson(testDir, { '@workos-inc/authkit-nextjs': '^2.6.0' }); + mockConfirm.mockResolvedValueOnce(Symbol('workos.prompt.cancel')); + + const error = await assertNoExistingAuthKit({ installDir: testDir }).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(CliExit); + expect((error as InstanceType).exitCode).toBe(2); + }); + }); +}); diff --git a/src/lib/preflight-authkit.ts b/src/lib/preflight-authkit.ts new file mode 100644 index 00000000..c3f8d485 --- /dev/null +++ b/src/lib/preflight-authkit.ts @@ -0,0 +1,89 @@ +/** + * Install preflight: refuse to run over an existing AuthKit install. + * + * `workos install` provisions a fresh WorkOS environment and writes its + * credentials into the project's env file before the installer state machine + * exists. In a project that already has AuthKit wired up that is data loss, so + * this guard runs first — prompting on an interactive TTY and hard-failing in + * agent/CI/JSON mode, with `--force` as the escape hatch. + */ + +import { AUTHKIT_PACKAGES } from '../doctor/checks/sdk.js'; +import { formatWorkOSCommand } from '../utils/command-invocation.js'; +import { ExitCode, exitWithCode } from '../utils/exit-codes.js'; +import { isPromptAllowed } from '../utils/interaction-mode.js'; +import { exitWithError, isJsonMode } from '../utils/output.js'; +import { getPackageVersion, readPackageJson } from '../utils/package-json.js'; +import ui from '../utils/ui.js'; + +export interface DetectedAuthKitPackage { + name: string; + version: string; +} + +/** + * Every AuthKit SDK present in the project, not just the first — the warning + * should name everything it found. + * + * An unreadable or malformed package.json reads as "no AuthKit": a parse + * failure must not block a run. + */ +export function detectExistingAuthKit(installDir: string): DetectedAuthKitPackage[] { + const packageJson = readPackageJson(installDir); + if (!packageJson) return []; + + return [...AUTHKIT_PACKAGES] + .map((name) => ({ name, version: getPackageVersion(name, packageJson) })) + .filter((pkg): pkg is DetectedAuthKitPackage => !!pkg.version); +} + +/** + * Stop the install before anything is written when AuthKit is already present. + * + * Must be called BEFORE credential resolution — provisioning writes the env + * file, so a guard that runs afterwards is no guard at all. + */ +export async function assertNoExistingAuthKit(opts: { installDir: string; force?: boolean }): Promise { + if (opts.force) return; + + const found = detectExistingAuthKit(opts.installDir); + if (found.length === 0) return; + + const summary = found.map((pkg) => `${pkg.name} ${pkg.version}`).join(', '); + + // Non-interactive: never prompt, never write. `--json` on a real TTY leaves + // interaction mode human (so isPromptAllowed() is still true) while output is + // machine-readable, so it has to be excluded too — otherwise the prompt below + // writes plain text into the JSON stream and fails as `prompt_unavailable` + // instead of `authkit_already_installed`. + if (!isPromptAllowed() || isJsonMode()) { + exitWithError({ + code: 'authkit_already_installed', + message: + `AuthKit is already installed in ${opts.installDir} (${summary}). ` + + `Continuing would provision a new WorkOS environment and rewrite your env file. ` + + `Run \`${formatWorkOSCommand('doctor')}\` to inspect the existing install, or pass --force to override.`, + }); + } + + // Interactive: show what was found, then ask. Name the env file explicitly — + // it is exactly what is at risk. + const { readProjectEnvCredentials, resolveProjectEnvPath } = await import('./project-env.js'); + const projectEnv = readProjectEnvCredentials(opts.installDir); + ui.log.warn('AuthKit is already installed here'); + ui.rows(found.map((pkg) => ({ key: pkg.name, value: pkg.version, statusKind: 'muted' as const }))); + // Only promise a rewrite when one can actually happen: with a key already on + // disk, credential resolution refuses to provision and logs in instead, so + // continuing changes code only. + ui.log.info( + projectEnv.apiKey + ? `Your existing WorkOS credentials in ${projectEnv.apiKeyPath ?? resolveProjectEnvPath(opts.installDir)} will be kept — continuing changes code only.` + : `Continuing will provision a new WorkOS environment and rewrite ${resolveProjectEnvPath(opts.installDir)}.`, + ); + ui.log.info(`To inspect the existing install instead: ${formatWorkOSCommand('doctor')}`); + + const proceed = await ui.confirm({ message: 'Continue anyway?' }); + if (ui.isCancel(proceed) || !proceed) { + exitWithCode(ExitCode.CANCELLED, { code: 'cancelled', message: 'Install cancelled.' }); + } +} diff --git a/src/lib/project-env.ts b/src/lib/project-env.ts new file mode 100644 index 00000000..4e0e27fd --- /dev/null +++ b/src/lib/project-env.ts @@ -0,0 +1,96 @@ +/** + * Reader for the project's WorkOS env files. + * + * Two halves that must not be confused: + * - `resolveProjectEnvPath` is the WRITE target — the single file + * `writeCredentialsEnv` would create, so the write and the warning copy can + * never disagree. + * - `readProjectEnvCredentials` is the READ side — every file that counts as an + * existing WorkOS configuration, because provisioning must refuse whenever the + * project already has a key, wherever it happens to live. + * + * Kept separate from `run-with-core.ts` so the credential path does not pull + * xstate and the whole installer graph in behind it. + */ + +import { existsSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +/** + * Env files that count as an existing WorkOS configuration, in the precedence + * order frameworks load them (earlier wins). + * + * KEEP IN SYNC: `ENV_FILE_NAMES` in ./credential-discovery.ts, which already + * treats all four as WorkOS credential sources. A file that discovery reads a + * key out of must also block provisioning: `.env.local` outranks `.env` in + * Next.js / Vite / Remix / SvelteKit, so provisioning into `.env.local` over a + * project configured through `.env` silently points the app at an empty + * throwaway environment while the real key sits on disk looking correct. + */ +export const ENV_FILE_NAMES = ['.env.local', '.env.development.local', '.env.development', '.env'] as const; + +// Leading indentation and an `export ` prefix are both common in env files that +// people also `source`, so both are tolerated. A `#`-commented line can never +// match: only spaces and tabs are allowed before the name. +const WORKOS_API_KEY_PATTERN = /^[ \t]*(?:export[ \t]+)?WORKOS_API_KEY[ \t]*=[ \t]*["']?([^"'\s#]+)["']?/m; +const WORKOS_CLIENT_ID_PATTERN = /^[ \t]*(?:export[ \t]+)?WORKOS_CLIENT_ID[ \t]*=[ \t]*["']?([^"'\s#]+)["']?/m; + +export interface ProjectEnvCredentials { + apiKey?: string; + clientId?: string; + /** Absolute path of the file `apiKey` was read from, when one was found. */ + apiKeyPath?: string; +} + +/** + * The env file the CLI would write for this project. + * + * Mirrors `writeCredentialsEnv` (`lib/env-writer.ts`): `.env.local` for JS + * projects (package.json present), `.env` for everything else. If that rule + * changes, both sides must change together. + * + * This is the write target only — it is NOT the set of files that count as + * already-configured. Use `readProjectEnvCredentials` for that. + */ +export function resolveProjectEnvPath(installDir: string): string { + return existsSync(join(installDir, 'package.json')) ? join(installDir, '.env.local') : join(installDir, '.env'); +} + +/** + * WorkOS credentials already present in any of the project's env files. + * + * Scans `ENV_FILE_NAMES` in precedence order and takes the first value found + * for each key — the same per-key, first-file-wins merge the frameworks + * themselves perform, so what we read is what the app would see. + * + * Read failures are skipped per file — a malformed or unreadable env file should + * not crash the installer, it should just look unconfigured. + */ +export function readProjectEnvCredentials(installDir: string): ProjectEnvCredentials { + const found: ProjectEnvCredentials = {}; + + for (const fileName of ENV_FILE_NAMES) { + const envPath = join(installDir, fileName); + if (!existsSync(envPath)) continue; + + let content: string; + try { + content = readFileSync(envPath, 'utf-8'); + } catch { + continue; + } + + if (!found.apiKey) { + const apiKey = content.match(WORKOS_API_KEY_PATTERN)?.[1]; + if (apiKey) { + found.apiKey = apiKey; + found.apiKeyPath = envPath; + } + } + found.clientId ??= content.match(WORKOS_CLIENT_ID_PATTERN)?.[1]; + + if (found.apiKey && found.clientId) break; + } + + return found; +} diff --git a/src/lib/resolve-install-credentials.spec.ts b/src/lib/resolve-install-credentials.spec.ts index eb645483..5bc6026d 100644 --- a/src/lib/resolve-install-credentials.spec.ts +++ b/src/lib/resolve-install-credentials.spec.ts @@ -1,4 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; // Mock config-store const mockGetActiveEnvironment = vi.fn(); @@ -20,18 +23,35 @@ vi.mock('./unclaimed-env-provision.js', () => ({ tryProvisionUnclaimedEnv: (...args: unknown[]) => mockTryProvisionUnclaimedEnv(...args), })); +// Mock the UI facade — the no-clobber branch now explains itself out loud. +const mockUi = { + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), step: vi.fn(), success: vi.fn(), hint: vi.fn() }, +}; +vi.mock('../utils/ui.js', () => ({ default: mockUi })); + const { resolveInstallCredentials } = await import('./resolve-install-credentials.js'); +const { setOutputMode } = await import('../utils/output.js'); describe('resolveInstallCredentials', () => { const mockAuthenticate = vi.fn(); const originalEnv = process.env.WORKOS_API_KEY; + // The default install dir is process.cwd(), and credential resolution now + // reads the project's env file. Point cwd at an empty dir so these specs + // don't depend on whatever .env.local happens to sit in the repo root. + let emptyCwd: string; + let cwdSpy: ReturnType; beforeEach(() => { vi.clearAllMocks(); delete process.env.WORKOS_API_KEY; + emptyCwd = mkdtempSync(join(tmpdir(), 'resolve-install-credentials-cwd-')); + cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(emptyCwd); }); afterEach(() => { + cwdSpy.mockRestore(); + setOutputMode('human'); + rmSync(emptyCwd, { recursive: true, force: true }); if (originalEnv !== undefined) { process.env.WORKOS_API_KEY = originalEnv; } else { @@ -146,4 +166,149 @@ describe('resolveInstallCredentials', () => { expect(mockTryProvisionUnclaimedEnv).toHaveBeenCalledWith({ installDir: process.cwd() }); }); + + // Provisioning writes credentials into the project's env file. If a key is + // already there, provisioning would clobber it — the reported data-loss bug. + describe('project env file already has credentials', () => { + let projectDir: string; + + beforeEach(() => { + projectDir = mkdtempSync(join(tmpdir(), 'resolve-install-credentials-test-')); + mockGetActiveEnvironment.mockReturnValue(null); + mockTryProvisionUnclaimedEnv.mockResolvedValue(true); + }); + + afterEach(() => { + rmSync(projectDir, { recursive: true, force: true }); + }); + + it('skips provisioning and falls back to login when .env.local has WORKOS_API_KEY (JS project)', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.local'), 'WORKOS_API_KEY=sk_a\nWORKOS_CLIENT_ID=client_a\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).not.toHaveBeenCalled(); + expect(mockAuthenticate).toHaveBeenCalled(); + }); + + it('skips provisioning when .env has WORKOS_API_KEY and there is no package.json (non-JS project)', async () => { + writeFileSync(join(projectDir, '.env'), 'WORKOS_API_KEY=sk_a\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).not.toHaveBeenCalled(); + expect(mockAuthenticate).toHaveBeenCalled(); + }); + + it('does not authenticate when skipAuth is set and the project env already has a key', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.local'), 'WORKOS_API_KEY=sk_a\n'); + + await resolveInstallCredentials(undefined, projectDir, true, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).not.toHaveBeenCalled(); + expect(mockAuthenticate).not.toHaveBeenCalled(); + }); + + it('provisions when .env.local exists but carries no WorkOS keys', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.local'), 'DATABASE_URL=postgres://localhost/dev\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).toHaveBeenCalledWith({ installDir: projectDir }); + expect(mockAuthenticate).not.toHaveBeenCalled(); + }); + + it('provisions when the project has no env file at all', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).toHaveBeenCalledWith({ installDir: projectDir }); + }); + + // The read side must cover every file the CLI itself treats as a credential + // source (credential-discovery.ts), not just the write target. `.env.local` + // outranks `.env` in Next.js/Vite/Remix/SvelteKit, so provisioning here would + // point the app at an empty throwaway env while the real key sits in `.env`. + it('refuses to provision when a JS project keeps its key in .env and has no .env.local', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env'), 'WORKOS_API_KEY=sk_real\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).not.toHaveBeenCalled(); + expect(mockAuthenticate).toHaveBeenCalled(); + }); + + it('refuses to provision when the key is in .env.development.local', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.development.local'), 'WORKOS_API_KEY=sk_real\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).not.toHaveBeenCalled(); + expect(mockAuthenticate).toHaveBeenCalled(); + }); + + it('refuses to provision when the key is in .env.development', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.development'), 'WORKOS_API_KEY=sk_real\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).not.toHaveBeenCalled(); + expect(mockAuthenticate).toHaveBeenCalled(); + }); + + // `export`-prefixed and indented lines are common in env files people `source`. + it('refuses to provision for `export WORKOS_API_KEY=` syntax with leading whitespace', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.local'), ' export WORKOS_API_KEY="sk_real"\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).not.toHaveBeenCalled(); + expect(mockAuthenticate).toHaveBeenCalled(); + }); + + it('still provisions when no env file anywhere carries a WorkOS key', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.local'), 'DATABASE_URL=postgres://localhost/dev\n'); + writeFileSync(join(projectDir, '.env.development'), '# WORKOS_API_KEY=sk_commented_out\n'); + writeFileSync(join(projectDir, '.env'), 'NEXT_PUBLIC_URL=http://localhost:3000\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockTryProvisionUnclaimedEnv).toHaveBeenCalledWith({ installDir: projectDir }); + expect(mockAuthenticate).not.toHaveBeenCalled(); + }); + + // Exit 4 from the login that follows is byte-identical to a provisioning + // network failure, so the refusal has to say why out loud. + it('tells the user the existing key was found and kept, naming the file it is in', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env'), 'WORKOS_API_KEY=sk_real\n'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + const printed = mockUi.log.info.mock.calls.map((call) => String(call[0])).join('\n'); + // The file the key actually lives in, not the write target (.env.local here). + expect(printed).toContain(`${join(projectDir, '.env')} already has WORKOS_API_KEY`); + expect(printed).toContain('Signing you in'); + }); + + it('stays silent in JSON mode so the machine-readable stream is not corrupted', async () => { + writeFileSync(join(projectDir, 'package.json'), '{}'); + writeFileSync(join(projectDir, '.env.local'), 'WORKOS_API_KEY=sk_real\n'); + setOutputMode('json'); + + await resolveInstallCredentials(undefined, projectDir, undefined, mockAuthenticate); + + expect(mockUi.log.info).not.toHaveBeenCalled(); + expect(mockAuthenticate).toHaveBeenCalled(); + }); + }); }); diff --git a/src/lib/resolve-install-credentials.ts b/src/lib/resolve-install-credentials.ts index 744ac975..1c352f8e 100644 --- a/src/lib/resolve-install-credentials.ts +++ b/src/lib/resolve-install-credentials.ts @@ -39,9 +39,35 @@ export async function resolveInstallCredentials( return; } + const dir = installDir ?? process.cwd(); + + // The CLI has no credentials, but the project might: provisioning writes to + // the project's env file, so a key already sitting there means we must not + // provision. Fall back to login instead — the project has a key, we just + // have no gateway auth. + const { readProjectEnvCredentials, resolveProjectEnvPath } = await import('./project-env.js'); + const projectEnv = readProjectEnvCredentials(dir); + if (projectEnv.apiKey) { + const { logInfo } = await import('../utils/debug.js'); + logInfo('[resolve-install-credentials] Project env already has WORKOS_API_KEY — skipping provisioning'); + + // Say it out loud. This is the branch that actually fires, and without a + // line here the login that follows looks identical to a provisioning + // network failure — the user never learns their key was found and kept. + const { isJsonMode } = await import('../utils/output.js'); + if (!isJsonMode()) { + const ui = (await import('../utils/ui.js')).default; + const envPath = projectEnv.apiKeyPath ?? resolveProjectEnvPath(dir); + ui.log.info(`${envPath} already has WORKOS_API_KEY — keeping it.`); + if (!skipAuth) ui.log.info('Signing you in so the AI installer can run.'); + } + + if (!skipAuth) await authenticate(); + return; + } + // No existing credentials — try unclaimed env provisioning const { tryProvisionUnclaimedEnv } = await import('./unclaimed-env-provision.js'); - const dir = installDir ?? process.cwd(); const provisioned = await tryProvisionUnclaimedEnv({ installDir: dir }); if (!provisioned) { // Unclaimed env provisioning failed — fall back to login diff --git a/src/lib/run-with-core.ts b/src/lib/run-with-core.ts index 1d8e2f3c..64497344 100644 --- a/src/lib/run-with-core.ts +++ b/src/lib/run-with-core.ts @@ -1,7 +1,5 @@ import { createActor, fromPromise } from 'xstate'; import open from 'open'; -import { existsSync, readFileSync } from 'fs'; -import { join } from 'path'; import { installerMachine } from './installer-core.js'; import { createInstallerEventEmitter } from './events.js'; import type { CompletionData } from './events.js'; @@ -24,7 +22,7 @@ import type { } from './installer-core.types.js'; import { isScaffoldableEmptyDir, resolvePackageManager, runCreateNextApp } from './scaffold/index.js'; import type { Integration } from './constants.js'; -import { parseEnvFile } from '../utils/env-parser.js'; +import { readProjectEnvCredentials } from './project-env.js'; import { enableDebugLogs, initLogFile, logInfo, logError } from '../utils/debug.js'; import { getAccessToken, saveCredentials, getStagingCredentials, saveStagingCredentials } from './credentials.js'; @@ -64,24 +62,6 @@ async function runIntegrationInstallerFn(integration: Integration, options: Inst return mod.run(options); } -function readExistingCredentials(installDir: string): { apiKey?: string; clientId?: string } { - const envPath = join(installDir, '.env.local'); - if (!existsSync(envPath)) { - return {}; - } - - try { - const content = readFileSync(envPath, 'utf-8'); - const envVars = parseEnvFile(content); - return { - apiKey: envVars.WORKOS_API_KEY || undefined, - clientId: envVars.WORKOS_CLIENT_ID || undefined, - }; - } catch { - return {}; - } -} - async function detectIntegrationFn(options: Pick): Promise { const registry = await getRegistry(); const configs = registry.detectionOrder(); @@ -192,7 +172,7 @@ export async function runWithCore(options: InstallerOptions): Promise { const gatewayUrl = getTelemetryUrl(); analytics.setGatewayUrl(gatewayUrl); - const existingCreds = readExistingCredentials(options.installDir); + const existingCreds = readProjectEnvCredentials(options.installDir); const augmentedOptions: InstallerOptions = { ...options, apiKey: options.apiKey || existingCreds.apiKey, diff --git a/src/lib/unclaimed-env-provision.spec.ts b/src/lib/unclaimed-env-provision.spec.ts index 1014f18b..044cd5e1 100644 --- a/src/lib/unclaimed-env-provision.spec.ts +++ b/src/lib/unclaimed-env-provision.spec.ts @@ -246,6 +246,66 @@ describe('unclaimed-env-provision', () => { expect(result).toBe(false); }); + // Write-site enforcement of the no-clobber invariant: whatever the caller + // did or did not check, the function that writes refuses to overwrite + // credentials the project already has. + describe('refuses when the project env file already has WORKOS_API_KEY', () => { + it('returns false and never calls the provisioning API (JS project, .env.local)', async () => { + writeFileSync(join(testDir, 'package.json'), '{}'); + writeFileSync(join(testDir, '.env.local'), 'WORKOS_API_KEY=sk_existing\nWORKOS_CLIENT_ID=client_existing\n'); + + const result = await tryProvisionUnclaimedEnv({ installDir: testDir }); + + expect(result).toBe(false); + expect(mockProvisionUnclaimedEnvironment).not.toHaveBeenCalled(); + expect(mockSaveConfig).not.toHaveBeenCalled(); + // The existing credentials are still byte-for-byte intact. + expect(readFileSync(join(testDir, '.env.local'), 'utf-8')).toBe( + 'WORKOS_API_KEY=sk_existing\nWORKOS_CLIENT_ID=client_existing\n', + ); + expect(mockUi.log.warn).toHaveBeenCalledWith(expect.stringContaining('already has WORKOS_API_KEY')); + }); + + it('returns false for a non-JS project whose .env already has a key', async () => { + writeFileSync(join(testDir, '.env'), 'WORKOS_API_KEY=sk_existing\n'); + + const result = await tryProvisionUnclaimedEnv({ installDir: testDir }); + + expect(result).toBe(false); + expect(mockProvisionUnclaimedEnvironment).not.toHaveBeenCalled(); + expect(readFileSync(join(testDir, '.env'), 'utf-8')).toBe('WORKOS_API_KEY=sk_existing\n'); + }); + + // The write target and the file holding the key are not the same thing: a + // JS project writes `.env.local`, but the key may already sit in `.env`. + // Naming the write target would send the user to edit the wrong file. + it('names the file the key was found in, not the file it would have written', async () => { + writeFileSync(join(testDir, 'package.json'), '{}'); + writeFileSync(join(testDir, '.env'), 'WORKOS_API_KEY=sk_existing\n'); + + const result = await tryProvisionUnclaimedEnv({ installDir: testDir }); + + expect(result).toBe(false); + expect(mockUi.log.warn).toHaveBeenCalledWith( + `${join(testDir, '.env')} already has WORKOS_API_KEY — not provisioning a new environment.`, + ); + }); + + it('still provisions when the env file exists without a WorkOS key', async () => { + writeFileSync(join(testDir, 'package.json'), '{}'); + writeFileSync(join(testDir, '.env.local'), 'DATABASE_URL=postgres://localhost/dev\n'); + mockProvisionUnclaimedEnvironment.mockResolvedValueOnce(validProvisionResult); + + const result = await tryProvisionUnclaimedEnv({ installDir: testDir }); + + expect(result).toBe(true); + expect(mockProvisionUnclaimedEnvironment).toHaveBeenCalled(); + const content = readFileSync(join(testDir, '.env.local'), 'utf-8'); + expect(content).toContain('DATABASE_URL=postgres://localhost/dev'); + expect(content).toContain('WORKOS_API_KEY=sk_test_oneshot'); + }); + }); + it('writes redirect URI to .env.local when provided (JS project)', async () => { writeFileSync(join(testDir, 'package.json'), '{}'); mockProvisionUnclaimedEnvironment.mockResolvedValueOnce(validProvisionResult); diff --git a/src/lib/unclaimed-env-provision.ts b/src/lib/unclaimed-env-provision.ts index 8dbf0071..15a883a0 100644 --- a/src/lib/unclaimed-env-provision.ts +++ b/src/lib/unclaimed-env-provision.ts @@ -37,6 +37,22 @@ export async function tryProvisionUnclaimedEnv(options: UnclaimedEnvProvisionOpt try { logInfo('[unclaimed-env-provision] Attempting unclaimed environment provisioning'); + // No-clobber invariant, enforced by the function that does the writing: + // never overwrite credentials the project already has. Keyed on credentials + // only (never on AuthKit detection) so `install --force` still works. + // Checked before provisioning so no environment is created then abandoned. + const { readProjectEnvCredentials, resolveProjectEnvPath } = await import('./project-env.js'); + const projectEnv = readProjectEnvCredentials(options.installDir); + if (projectEnv.apiKey) { + logInfo('[unclaimed-env-provision] Refusing to provision: project env already has WORKOS_API_KEY'); + // Name the file the key was actually found in, not the file we would have + // written: the key can live in any of `ENV_FILE_NAMES`, and pointing at the + // write target sends people to edit a file that may not even exist. + const envPath = projectEnv.apiKeyPath ?? resolveProjectEnvPath(options.installDir); + ui.log.warn(`${envPath} already has WORKOS_API_KEY — not provisioning a new environment.`); + return false; + } + const result = await provisionUnclaimedEnvironment(); // Write .env.local first — if this fails, config stays clean (no orphan entries) diff --git a/src/lib/workos-management.spec.ts b/src/lib/workos-management.spec.ts new file mode 100644 index 00000000..a328ed38 --- /dev/null +++ b/src/lib/workos-management.spec.ts @@ -0,0 +1,326 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import type { EnvironmentConfig } from './config-store.js'; + +// Provenance is derived in-module from the config store, so the store is the +// only seam the tests need to drive. `isUnclaimedEnvironment` keeps its real +// (trivial) behavior so a fixture's `type` alone decides the branch. +const getActiveEnvironment = vi.fn<() => EnvironmentConfig | null>(() => null); + +vi.mock('./config-store.js', () => ({ + getActiveEnvironment: () => getActiveEnvironment(), + isUnclaimedEnvironment: (env: EnvironmentConfig) => env.type === 'unclaimed', +})); + +vi.mock('../utils/analytics.js', () => ({ + analytics: { capture: vi.fn(), captureException: vi.fn() }, +})); + +const { analytics } = await import('../utils/analytics.js'); +const ui = (await import('../utils/ui.js')).default; +const { autoConfigureWorkOSEnvironment } = await import('./workos-management.js'); + +const API_KEY = 'sk_test_123'; +const HOMEPAGE_ENDPOINT = 'https://api.workos.com/user_management/app_homepage_url'; +/** `autoConfigureWorkOSEnvironment(apiKey, integration, port)` — port drives every URL. */ +const PORT = 4343; +const BASE_URL = `http://localhost:${PORT}`; + +type FetchCall = { url: string; method: string }; + +function jsonResponse(status: number, body: unknown): Response { + return { + ok: status >= 200 && status < 300, + status, + json: async () => body, + } as unknown as Response; +} + +/** A response whose body is not JSON — `.json()` rejects, like the real thing. */ +function unparseableResponse(status: number): Response { + return { + ok: status >= 200 && status < 300, + status, + json: async () => { + throw new SyntaxError('Unexpected token < in JSON'); + }, + } as unknown as Response; +} + +/** + * Stub `fetch` for the three parallel calls `autoConfigureWorkOSEnvironment` + * makes. `homepage` decides what the newly added GET returns; the two POSTs + * always succeed so failures can only come from the path under test. + */ +function stubFetch(homepage: (method: string) => Response | Promise): { + calls: FetchCall[]; +} { + const calls: FetchCall[] = []; + const stub = vi.fn(async (url: string, init: { method: string }) => { + calls.push({ url, method: init.method }); + if (url === HOMEPAGE_ENDPOINT) return homepage(init.method); + return jsonResponse(201, {}); + }); + vi.stubGlobal('fetch', stub); + return { calls }; +} + +function homepageCalls(calls: FetchCall[], method: string): FetchCall[] { + return calls.filter((c) => c.url === HOMEPAGE_ENDPOINT && c.method === method); +} + +/** The rows array passed to the single `ui.rows` call. */ +function capturedRows(): Array<{ key: string; value: string; status?: string; statusKind?: string }> { + const spy = vi.mocked(ui.rows); + expect(spy).toHaveBeenCalledTimes(1); + return spy.mock.calls[0]![0]; +} + +function rowFor(key: string) { + const row = capturedRows().find((r) => r.key === key); + expect(row, `no "${key}" row was rendered`).toBeDefined(); + return row!; +} + +const unclaimedEnv: EnvironmentConfig = { + name: 'unclaimed-2', + apiKey: API_KEY, + type: 'unclaimed', + clientId: 'client_123', + claimToken: 'tok_123', +}; + +const claimedEnv: EnvironmentConfig = { + name: 'sandbox', + apiKey: API_KEY, + type: 'sandbox', +}; + +/** `Integration` is a plain string identifier (constants.ts:8). */ +const INTEGRATION = 'nextjs'; + +describe('workos-management', () => { + beforeEach(() => { + getActiveEnvironment.mockReset(); + getActiveEnvironment.mockReturnValue(null); + vi.mocked(analytics.capture).mockClear(); + vi.spyOn(ui, 'rows').mockImplementation(() => {}); + vi.spyOn(ui.log, 'step').mockImplementation(() => {}); + vi.spyOn(ui.log, 'success').mockImplementation(() => {}); + vi.spyOn(ui.log, 'info').mockImplementation(() => {}); + vi.spyOn(ui.log, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + describe('setHomepageUrl read-then-write', () => { + it('skips the PUT when the current homepage URL already matches', async () => { + const { calls } = stubFetch(() => jsonResponse(200, { url: BASE_URL })); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(homepageCalls(calls, 'GET')).toHaveLength(1); + expect(homepageCalls(calls, 'PUT')).toHaveLength(0); + expect(result?.homepageUrl).toEqual({ success: true, alreadyExists: true }); + expect(rowFor('Homepage URL')).toMatchObject({ status: 'already set', statusKind: 'muted' }); + }); + + it('issues the PUT when the current homepage URL differs', async () => { + const { calls } = stubFetch((method) => + method === 'GET' ? jsonResponse(200, { url: 'https://app.example.com' }) : jsonResponse(200, {}), + ); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(homepageCalls(calls, 'PUT')).toHaveLength(1); + expect(result?.homepageUrl).toEqual({ success: true, alreadyExists: false }); + expect(rowFor('Homepage URL')).toMatchObject({ value: BASE_URL, status: 'updated', statusKind: 'ok' }); + }); + + it('compares against the caller-supplied homepage URL, not the base URL', async () => { + const custom = 'https://staging.example.com'; + const { calls } = stubFetch(() => jsonResponse(200, { url: custom })); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT, { homepageUrl: custom }); + + expect(homepageCalls(calls, 'PUT')).toHaveLength(0); + expect(result?.homepageUrl.alreadyExists).toBe(true); + }); + + it.each([404, 500])('falls through to the PUT when the GET returns %i', async (status) => { + const { calls } = stubFetch((method) => + method === 'GET' ? jsonResponse(status, { message: 'nope' }) : jsonResponse(200, {}), + ); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(homepageCalls(calls, 'PUT')).toHaveLength(1); + expect(result?.homepageUrl.alreadyExists).toBe(false); + }); + + it('falls through to the PUT when the GET body is not JSON', async () => { + const { calls } = stubFetch((method) => (method === 'GET' ? unparseableResponse(200) : jsonResponse(200, {}))); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(homepageCalls(calls, 'PUT')).toHaveLength(1); + expect(result?.homepageUrl.alreadyExists).toBe(false); + }); + + it('falls through to the PUT when the GET rejects', async () => { + const { calls } = stubFetch((method) => { + if (method === 'GET') return Promise.reject(new TypeError('fetch failed')); + return jsonResponse(200, {}); + }); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(homepageCalls(calls, 'PUT')).toHaveLength(1); + expect(result?.homepageUrl.alreadyExists).toBe(false); + }); + + it('omits a request body on the GET so the read cannot be mistaken for a write', async () => { + const bodies: Array = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string, init: { method: string; body?: string }) => { + if (url === HOMEPAGE_ENDPOINT && init.method === 'GET') bodies.push(init.body); + return jsonResponse(200, { url: BASE_URL }); + }), + ); + + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(bodies).toEqual([undefined]); + }); + + it('declares Content-Type only on the requests that carry a body', async () => { + const seen: Array<{ method: string; contentType?: string }> = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string, init: { method: string; headers: Record }) => { + if (url === HOMEPAGE_ENDPOINT) { + seen.push({ method: init.method, contentType: init.headers['Content-Type'] }); + } + // Differing URL forces the PUT, so both methods are observed. + return jsonResponse(200, { url: 'https://app.example.com' }); + }), + ); + + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(seen).toEqual([ + { method: 'GET', contentType: undefined }, + { method: 'PUT', contentType: 'application/json' }, + ]); + }); + + it('warns instead of aborting when the PUT fails', async () => { + stubFetch((method) => + method === 'GET' ? jsonResponse(404, {}) : jsonResponse(403, { message: 'API key lacks permission' }), + ); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(result).toBeNull(); + expect(ui.log.warn).toHaveBeenCalledWith(expect.stringContaining('Could not configure WorkOS dashboard')); + expect(ui.rows).not.toHaveBeenCalled(); + }); + + it('reports homepage no-op vs. overwrite to analytics', async () => { + stubFetch(() => jsonResponse(200, { url: BASE_URL })); + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + expect(analytics.capture).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ homepageUrl: 'existed' }), + ); + + vi.mocked(analytics.capture).mockClear(); + vi.mocked(ui.rows).mockClear(); + stubFetch((method) => (method === 'GET' ? jsonResponse(404, {}) : jsonResponse(200, {}))); + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + expect(analytics.capture).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ homepageUrl: 'updated' }), + ); + }); + }); + + describe('credential provenance row', () => { + beforeEach(() => { + stubFetch(() => jsonResponse(200, { url: BASE_URL })); + }); + + it('renders Environment as the first row', async () => { + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(capturedRows()[0]?.key).toBe('Environment'); + expect(capturedRows().map((r) => r.key)).toEqual(['Environment', 'Redirect URI', 'CORS origin', 'Homepage URL']); + }); + + it('names an unclaimed environment and points at `env claim`', async () => { + getActiveEnvironment.mockReturnValue(unclaimedEnv); + + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + const value = rowFor('Environment').value; + expect(value).toContain('unclaimed'); + expect(value).toContain('unclaimed-2'); + expect(value).toContain('env claim'); + }); + + it('names a claimed active environment', async () => { + getActiveEnvironment.mockReturnValue(claimedEnv); + + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + const value = rowFor('Environment').value; + expect(value).toBe('your active environment (sandbox)'); + expect(value).not.toContain('env claim'); + }); + + it('falls back to the supplied-key wording with no active environment', async () => { + getActiveEnvironment.mockReturnValue(null); + + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(rowFor('Environment').value).toBe('the API key supplied to this run'); + }); + + it('does not name a stored environment whose key did not do the writes', async () => { + // `--api-key sk_live_prod...` bypasses the store: the writes landed in the + // supplied key's environment, not the stored active one. + getActiveEnvironment.mockReturnValue(unclaimedEnv); + + await autoConfigureWorkOSEnvironment('sk_live_prod_999', INTEGRATION, PORT); + + const value = rowFor('Environment').value; + expect(value).toBe('the API key supplied to this run'); + expect(value).not.toContain('unclaimed-2'); + expect(value).not.toContain('env claim'); + }); + + it('does not name a claimed stored environment whose key did not do the writes', async () => { + getActiveEnvironment.mockReturnValue(claimedEnv); + + await autoConfigureWorkOSEnvironment('sk_live_prod_999', INTEGRATION, PORT); + + const value = rowFor('Environment').value; + expect(value).toBe('the API key supplied to this run'); + expect(value).not.toContain('sandbox'); + }); + + it('degrades to the supplied-key wording when the config store throws', async () => { + getActiveEnvironment.mockImplementation(() => { + throw new Error('keyring locked'); + }); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT); + + expect(result).not.toBeNull(); + expect(rowFor('Environment').value).toBe('the API key supplied to this run'); + }); + }); +}); diff --git a/src/lib/workos-management.ts b/src/lib/workos-management.ts index be8cd6df..8d6ae1f1 100644 --- a/src/lib/workos-management.ts +++ b/src/lib/workos-management.ts @@ -1,15 +1,23 @@ import type { Integration } from './constants.js'; +import type { EnvironmentConfig } from './config-store.js'; import { INSTALLER_INTERACTION_EVENT_NAME } from './constants.js'; import { analytics } from '../utils/analytics.js'; +import { formatWorkOSCommand } from '../utils/command-invocation.js'; import ui from '../utils/ui.js'; +import { getActiveEnvironment, isUnclaimedEnvironment } from './config-store.js'; import { getCallbackPath } from './port-detection.js'; const WORKOS_API_BASE = 'https://api.workos.com'; +const HOMEPAGE_URL_ENDPOINT = '/user_management/app_homepage_url'; + +/** Provenance wording when no stored environment can be named. */ +const SUPPLIED_KEY_PROVENANCE = 'the API key supplied to this run'; + export interface AutoConfigResult { redirectUri: { success: boolean; alreadyExists: boolean }; corsOrigin: { success: boolean; alreadyExists: boolean }; - homepageUrl: { success: boolean }; + homepageUrl: { success: boolean; alreadyExists: boolean }; } interface FetchError { @@ -19,18 +27,20 @@ interface FetchError { } async function workosRequest( - method: 'POST' | 'PUT', + method: 'GET' | 'POST' | 'PUT', endpoint: string, apiKey: string, - body: Record, + body?: Record, ): Promise { return fetch(`${WORKOS_API_BASE}${endpoint}`, { method, headers: { Authorization: `Bearer ${apiKey}`, - 'Content-Type': 'application/json', + // Only declare a body's type when there is a body. A bodyless GET that + // claims `application/json` is malformed, and strict proxies may reject it. + ...(body ? { 'Content-Type': 'application/json' } : {}), }, - body: JSON.stringify(body), + ...(body ? { body: JSON.stringify(body) } : {}), }); } @@ -89,17 +99,69 @@ async function createCorsOrigin(apiKey: string, origin: string): Promise<{ succe } /** - * Set the app homepage URL in WorkOS. + * Set the app homepage URL in WorkOS, skipping the write when it already matches. + * + * The homepage URL is a single-valued setting, so an unconditional PUT silently + * overwrites whatever a logged-in user already had configured. Reading first + * makes the common case a no-op that reports itself honestly. + * + * A read failure is not fatal: fall through to the PUT, which is the old + * behavior, rather than abandoning configuration over a GET we just added. */ -async function setHomepageUrl(apiKey: string, url: string): Promise<{ success: boolean }> { - const response = await workosRequest('PUT', '/user_management/app_homepage_url', apiKey, { url }); +async function setHomepageUrl(apiKey: string, url: string): Promise<{ success: boolean; alreadyExists: boolean }> { + try { + const current = await workosRequest('GET', HOMEPAGE_URL_ENDPOINT, apiKey); + if (current.ok) { + const data = (await current.json()) as { url?: string } | null; + if (data?.url === url) { + return { success: true, alreadyExists: true }; + } + } + } catch { + // Read failed (endpoint missing, non-JSON body, network error) — fall + // through to the write so behavior is never worse than before. + } + + const response = await workosRequest('PUT', HOMEPAGE_URL_ENDPOINT, apiKey, { url }); if (!response.ok) { const error = await parseFetchError(response); throw new Error(error.message || `HTTP ${error.status}`); } - return { success: true }; + return { success: true, alreadyExists: false }; +} + +/** + * Where the credentials being used came from, so the rows below say *where* the + * writes landed and not just what was written. + * + * Derived locally: the WorkOS API exposes no environment identity this module + * can reach (`workosRequest` is GET/POST/PUT against user_management only, and + * the config store holds no environment id). `activeEnvironment.name` is a + * local label, so it is only ever a parenthetical, never an authoritative id. + * + * The stored active environment is only named when its key is the one that + * actually did the writes — `--api-key` (or `WORKOS_API_KEY`) bypasses the + * store entirely, and naming an untouched environment is exactly the confusion + * this row exists to prevent. + */ +function describeCredentialProvenance(apiKey: string): string { + let activeEnv: EnvironmentConfig | null = null; + try { + activeEnv = getActiveEnvironment(); + } catch { + // Keyring locked or unavailable — a label is not worth aborting over. + return SUPPLIED_KEY_PROVENANCE; + } + + if (!activeEnv || activeEnv.apiKey !== apiKey) return SUPPLIED_KEY_PROVENANCE; + + if (isUnclaimedEnvironment(activeEnv)) { + return `a new unclaimed environment (${activeEnv.name}) — run \`${formatWorkOSCommand('env claim')}\` to keep it`; + } + + return `your active environment (${activeEnv.name})`; } export interface AutoConfigOptions { @@ -148,12 +210,15 @@ export async function autoConfigureWorkOSEnvironment( port, redirectUri: redirectUri.alreadyExists ? 'existed' : 'created', corsOrigin: corsOrigin.alreadyExists ? 'existed' : 'created', + homepageUrl: homepageUrl.alreadyExists ? 'existed' : 'updated', }); // Aligned key/value feedback: value in accent, a dim status for "already - // existed" vs. a green status for a fresh create/update. + // existed" vs. a green status for a fresh create/update. The provenance row + // comes first — it is the context for the three rows below it. ui.log.success('WorkOS dashboard configured'); ui.rows([ + { key: 'Environment', value: describeCredentialProvenance(apiKey), statusKind: 'muted' }, { key: 'Redirect URI', value: callbackUrl, @@ -166,7 +231,12 @@ export async function autoConfigureWorkOSEnvironment( status: corsOrigin.alreadyExists ? 'already set' : 'created', statusKind: corsOrigin.alreadyExists ? 'muted' : 'ok', }, - { key: 'Homepage URL', value: homepageUrlValue, status: 'updated', statusKind: 'ok' }, + { + key: 'Homepage URL', + value: homepageUrlValue, + status: homepageUrl.alreadyExists ? 'already set' : 'updated', + statusKind: homepageUrl.alreadyExists ? 'muted' : 'ok', + }, ]); return results; diff --git a/src/utils/help-json.ts b/src/utils/help-json.ts index 5037c570..b057941a 100644 --- a/src/utils/help-json.ts +++ b/src/utils/help-json.ts @@ -1424,6 +1424,17 @@ const commands: CommandSchema[] = [ default: false, hidden: false, }, + // Distinct from --force-install above: this one overrides the + // "AuthKit is already installed" preflight guard, and the guard's own + // error tells agents to pass it, so it has to be discoverable here. + { + name: 'force', + type: 'boolean', + description: 'Continue even if AuthKit is already installed in this project', + required: false, + default: false, + hidden: false, + }, { name: 'dashboard', type: 'boolean',