diff --git a/src/commands/install.ts b/src/commands/install.ts index e8e68625..8da40186 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -6,6 +6,7 @@ import { ExitCode, exitWithCode } from '../utils/exit-codes.js'; import { isCiMode } from '../utils/interaction-mode.js'; import type { ArgumentsCamelCase } from 'yargs'; import { InstallDeclinedError } from '../lib/installer-errors.js'; +import { CliExit } from '../utils/cli-exit.js'; import { maybeRunSetupAfter } from './setup.js'; /** @@ -37,6 +38,10 @@ export async function handleInstall(argv: ArgumentsCamelCase): Pr // (human/TTY-only, decline-respecting) and best-effort — never fails install. await maybeRunSetupAfter('install'); } catch (err) { + // Structured exits (e.g. the environment picker's cancel, exit 2) carry + // their own code and messaging — masking them as installer_error would + // turn a clean cancel into a failure. + if (err instanceof CliExit) throw err; if (err instanceof InstallDeclinedError) { // The integration already printed actionable guidance; exit non-zero // so scripts don't proceed as if AuthKit were installed. diff --git a/src/lib/dashboard-graphql.ts b/src/lib/dashboard-graphql.ts index 0295dddd..655ea9fd 100644 --- a/src/lib/dashboard-graphql.ts +++ b/src/lib/dashboard-graphql.ts @@ -70,6 +70,13 @@ export interface DashboardGraphqlOptions { * no environment header. */ environmentId?: string; + /** + * Optional caller cancellation, merged into the request's own abort + * controller so the socket dies with the caller's deadline instead of + * holding the event loop open until the transport timeout. (The install + * picker's bounded team discovery is the caller that needs this.) + */ + signal?: AbortSignal; } /** @@ -95,6 +102,7 @@ async function sendDashboardRequest( const url = `${getWorkOSApiUrl()}/graphql`; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS); + options.signal?.addEventListener('abort', () => controller.abort(), { once: true }); let res: Response; try { diff --git a/src/lib/environment-target.ts b/src/lib/environment-target.ts index 8597bbf9..366a3090 100644 --- a/src/lib/environment-target.ts +++ b/src/lib/environment-target.ts @@ -83,9 +83,9 @@ function remedies(): string { * whether that is fatal (`resolveEnvironmentTarget`) or best-effort * (`tryResolveProfileEnvironmentId`). */ -export async function fetchTeamEnvironments(token: string): Promise { +export async function fetchTeamEnvironments(token: string, signal?: AbortSignal): Promise { const op = getOperation('teamProjectsV2'); - const data = await dashboardGraphqlRequest(resolveExecutableDocument(op), { token }); + const data = await dashboardGraphqlRequest(resolveExecutableDocument(op), { token, signal }); const projects = data.currentTeam?.projectsV2 ?? []; return projects.flatMap((project) => (project.environments ?? []).map((env) => ({ ...env, projectName: project.name })), diff --git a/src/lib/resolve-install-credentials.spec.ts b/src/lib/resolve-install-credentials.spec.ts index 5aa7e500..a3969eeb 100644 --- a/src/lib/resolve-install-credentials.spec.ts +++ b/src/lib/resolve-install-credentials.spec.ts @@ -25,10 +25,14 @@ vi.mock('./config-store.js', async (importOriginal) => { const mockGetAccessToken = vi.fn(); const mockGetStagingCredentials = vi.fn(); const mockSaveStagingCredentials = vi.fn(); +const mockGetCredentials = vi.fn(); +const mockIsTokenExpired = vi.fn(); vi.mock('./credentials.js', () => ({ getAccessToken: () => mockGetAccessToken(), getStagingCredentials: () => mockGetStagingCredentials(), saveStagingCredentials: (...args: unknown[]) => mockSaveStagingCredentials(...args), + getCredentials: () => mockGetCredentials(), + isTokenExpired: (...args: unknown[]) => mockIsTokenExpired(...args), })); // Mock the staging API @@ -43,6 +47,15 @@ vi.mock('./unclaimed-env-provision.js', () => ({ tryProvisionUnclaimedEnv: (...args: unknown[]) => mockTryProvisionUnclaimedEnv(...args), })); +// Team-environment discovery for the picker's disabled rows. The picker only +// ever uses a currently-valid stored token — it never refreshes (a refresh +// outliving the discovery timeout can clear a newer session on invalid_grant). +const mockFetchTeamEnvironments = vi.fn(); +vi.mock('./environment-target.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, fetchTeamEnvironments: (...args: unknown[]) => mockFetchTeamEnvironments(...args) }; +}); + // Mock the UI facade — the no-clobber branch now explains itself out loud. const CANCEL = Symbol('cancel'); const mockSelect = vi.fn(); @@ -54,7 +67,8 @@ const mockUi = { }; vi.mock('../utils/ui.js', () => ({ default: mockUi })); -const { resolveInstallCredentials, resolveStagingCredentials } = await import('./resolve-install-credentials.js'); +const { resolveInstallCredentials, resolveStagingCredentials, maybePickInstallEnvironment } = + await import('./resolve-install-credentials.js'); const { setOutputMode } = await import('../utils/output.js'); describe('resolveInstallCredentials', () => { @@ -69,6 +83,8 @@ describe('resolveInstallCredentials', () => { beforeEach(() => { vi.clearAllMocks(); mockGetConfig.mockReturnValue(null); + mockGetCredentials.mockReturnValue(null); + mockIsTokenExpired.mockReturnValue(false); delete process.env.WORKOS_API_KEY; emptyCwd = mkdtempSync(join(tmpdir(), 'resolve-install-credentials-cwd-')); cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(emptyCwd); @@ -369,7 +385,7 @@ describe('resolveInstallCredentials', () => { mockGetConfig.mockReturnValue(twoProfiles); mockSelect.mockResolvedValue('staging'); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); const call = mockSelect.mock.calls[0][0] as { options: Array<{ value: string; label: string }>; @@ -390,7 +406,7 @@ describe('resolveInstallCredentials', () => { mockGetConfig.mockReturnValue(twoProfiles); mockSelect.mockResolvedValue('staging-3'); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSetActiveEnvironment).not.toHaveBeenCalled(); }); @@ -401,7 +417,7 @@ describe('resolveInstallCredentials', () => { environments: { 'staging-3': twoProfiles.environments['staging-3'] }, }); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSelect).not.toHaveBeenCalled(); }); @@ -411,7 +427,7 @@ describe('resolveInstallCredentials', () => { setInteractionMode({ mode: 'agent', source: 'env' }); try { mockGetConfig.mockReturnValue(twoProfiles); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSelect).not.toHaveBeenCalled(); } finally { resetInteractionModeForTests(); @@ -422,7 +438,7 @@ describe('resolveInstallCredentials', () => { mockGetConfig.mockReturnValue(twoProfiles); setOutputMode('json'); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSelect).not.toHaveBeenCalled(); }); @@ -433,17 +449,121 @@ describe('resolveInstallCredentials', () => { writeFileSync(join(emptyCwd, '.env'), 'WORKOS_API_KEY=sk_project\n'); mockGetConfig.mockReturnValue(twoProfiles); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSelect).not.toHaveBeenCalled(); expect(mockSetActiveEnvironment).not.toHaveBeenCalled(); }); + it('lists team environments without a local key as disabled rows', async () => { + mockGetConfig.mockReturnValue(twoProfiles); + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: Date.now() + 3_600_000 }); + mockFetchTeamEnvironments.mockResolvedValue([ + // Joined to the keyed 'staging-3' profile via clientId — not duplicated. + { id: 'env_staging', name: 'Staging', sandbox: true, clientId: 'client_b', projectName: 'cli-branding-smoke' }, + { id: 'env_prod', name: 'Production', sandbox: false, clientId: 'client_z', projectName: 'cli-branding-smoke' }, + ]); + mockGetConfig.mockReturnValue({ + ...twoProfiles, + environments: { + ...twoProfiles.environments, + 'staging-3': { ...twoProfiles.environments['staging-3'], clientId: 'client_b' }, + }, + }); + mockSelect.mockResolvedValue('staging'); + + await maybePickInstallEnvironment(null, emptyCwd); + + const call = mockSelect.mock.calls[0][0] as { + options: Array<{ value: string; label: string; disabled?: string }>; + }; + const disabledRows = call.options.filter((o) => o.disabled); + expect(disabledRows).toHaveLength(1); + expect(disabledRows[0].value).toBe('__unavailable__env_prod'); + expect(disabledRows[0].label).toContain('cli-branding-smoke > Production'); + expect(disabledRows[0].disabled).toContain('no API key on this machine'); + // The note counts the TEAM, not local profiles: 'staging' carries no + // clientId/environmentId and joins nothing in the team catalog (a + // foreign profile), so the team truth is 2 environments, 1 keyed here — + // not the 3/2 a naive local count would claim. Plus the recipe. + const note = String(mockUi.note.mock.calls[0][0]); + expect(note).toContain('2 environments'); + expect(note).toContain('1 is ready'); + expect(note).toContain('workos profile add'); + }); + + it('prompts even with a single keyed profile when the team has more environments', async () => { + mockGetConfig.mockReturnValue({ + activeEnvironment: 'staging-3', + environments: { 'staging-3': twoProfiles.environments['staging-3'] }, + }); + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: Date.now() + 3_600_000 }); + mockFetchTeamEnvironments.mockResolvedValue([ + { id: 'env_prod', name: 'Production', sandbox: false, clientId: 'client_z', projectName: 'P' }, + ]); + mockSelect.mockResolvedValue('staging-3'); + + await maybePickInstallEnvironment(null, emptyCwd); + + expect(mockSelect).toHaveBeenCalled(); + }); + + it('degrades to the local-only picker when team discovery fails', async () => { + mockGetConfig.mockReturnValue(twoProfiles); + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: Date.now() + 3_600_000 }); + mockFetchTeamEnvironments.mockRejectedValue(new Error('offline')); + mockSelect.mockResolvedValue('staging'); + + await maybePickInstallEnvironment(null, emptyCwd); + + const call = mockSelect.mock.calls[0][0] as { options: Array<{ disabled?: string }> }; + expect(call.options.every((o) => !o.disabled)).toBe(true); + }); + + it('does not let a hung team fetch stall the picker', async () => { + mockGetConfig.mockReturnValue(twoProfiles); + // Valid session + dead endpoint: the fetch promise never settles. + // Discovery is bounded, so the picker still opens with local-only rows. + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: Date.now() + 3_600_000 }); + let fetchSignal: AbortSignal | undefined; + mockFetchTeamEnvironments.mockImplementation((_token: string, signal?: AbortSignal) => { + fetchSignal = signal; + return new Promise(() => {}); + }); + mockSelect.mockResolvedValue('staging'); + + const start = Date.now(); + await maybePickInstallEnvironment(null, emptyCwd); + + expect(Date.now() - start).toBeLessThan(10_000); // bounded, not the 30s fetch timeout + const call = mockSelect.mock.calls[0][0] as { options: Array<{ disabled?: string }> }; + expect(call.options.every((o) => !o.disabled)).toBe(true); + // The abandoned request is cancelled too — its socket and abort timer + // must not hold the event loop open past CLI exit. + expect(fetchSignal?.aborted).toBe(true); + }, 15_000); + + it('never refreshes inside the picker — an expired session degrades to local-only rows', async () => { + // A refresh raced past the discovery timeout keeps running and can + // answer invalid_grant after the installer writes a new session, which + // clears the credential store. The picker must only use a valid token. + mockGetConfig.mockReturnValue(twoProfiles); + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: 0 }); + mockIsTokenExpired.mockReturnValue(true); + mockSelect.mockResolvedValue('staging'); + + await maybePickInstallEnvironment(null, emptyCwd); + + expect(mockFetchTeamEnvironments).not.toHaveBeenCalled(); + const call = mockSelect.mock.calls[0][0] as { options: Array<{ disabled?: string }> }; + expect(call.options.every((o) => !o.disabled)).toBe(true); + }); + it('cancel cancels the install (exit 2)', async () => { mockGetConfig.mockReturnValue(twoProfiles); mockSelect.mockResolvedValue(CANCEL); - await expect(resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate)).rejects.toMatchObject({ + await expect(maybePickInstallEnvironment(null, emptyCwd)).rejects.toMatchObject({ exitCode: 2, }); expect(mockSetActiveEnvironment).not.toHaveBeenCalled(); diff --git a/src/lib/resolve-install-credentials.ts b/src/lib/resolve-install-credentials.ts index a1433d6a..522755f9 100644 --- a/src/lib/resolve-install-credentials.ts +++ b/src/lib/resolve-install-credentials.ts @@ -9,6 +9,14 @@ * - Direct mode: not handled here (resolved in agent-interface.ts via ANTHROPIC_API_KEY) */ import type { EnvironmentConfig } from './config-store.js'; +import type { TeamEnvironment } from './environment-target.js'; + +/** + * Upper bound on team-environment discovery before the picker falls back to + * local profiles. The GraphQL fetch can wait 30s on a dead endpoint; the + * picker (and the single-profile silent path) must not inherit that wait. + */ +const TEAM_DISCOVERY_TIMEOUT_MS = 3_000; /** * When several stored profiles could serve this install, ask which WorkOS @@ -24,8 +32,15 @@ import type { EnvironmentConfig } from './config-store.js'; * persists via setActiveEnvironment so the installer and every later command * agree; cancel cancels the install (exit 2), matching the installer's other * prompts. + * + * The picker shows the WHOLE team, not just this machine: environments the + * session can see but that have no local API key render as disabled rows + * with the recipe to enable them. The dashboard catalog has no operation + * that creates or reveals an sk_ secret, so the CLI cannot make those rows + * selectable on its own — but hiding them is what made installs feel like + * they were choosing from a different list than the dashboard shows. */ -async function maybePickInstallEnvironment( +export async function maybePickInstallEnvironment( activeEnv: EnvironmentConfig | null, installDir: string, ): Promise { @@ -41,32 +56,99 @@ async function maybePickInstallEnvironment( const config = getConfig(); if (!config) return activeEnv; const candidates = Object.entries(config.environments).filter(([, env]) => env.apiKey); - if (candidates.length < 2) return activeEnv; + if (candidates.length === 0) return activeEnv; + + // Best-effort: the rest of the team's environments, for the disabled rows. + // Any failure degrades to the local-only picker — including slowness: a + // dead endpoint's 30s fetch timeout must not stall installer startup (or + // the single-profile silent path), so the fetch is bounded. Only a + // currently-valid token is used — never a refresh: the race abandons the + // loser without cancelling it, and a refresh outliving the picker can + // answer invalid_grant AFTER the installer's own auth flow wrote a new + // session, and invalid_grant clears the credential store. An expired + // session just degrades to the local-only picker; the machine's auth check + // re-authenticates later. + const discovery = new AbortController(); + const discoverTeam = async (): Promise => { + try { + const { getCredentials, isTokenExpired } = await import('./credentials.js'); + const creds = getCredentials(); + if (!creds || isTokenExpired(creds)) return []; + const { fetchTeamEnvironments } = await import('./environment-target.js'); + return await fetchTeamEnvironments(creds.accessToken, discovery.signal); + } catch { + return []; // Offline / logged out / flag-gated — the local-only picker still works. + } + }; + // A lost race must also CANCEL the request: an abandoned fetch keeps its + // socket and abort timer alive, and CLI exit waits on the event loop + // draining — otherwise a quick install lingers until the transport timeout. + let discoveryTimer: ReturnType | undefined; + const teamEnvironments = await Promise.race([ + discoverTeam(), + new Promise((resolve) => { + discoveryTimer = setTimeout(() => { + discovery.abort(); + resolve([]); + }, TEAM_DISCOVERY_TIMEOUT_MS); + discoveryTimer.unref?.(); + }), + ]); + clearTimeout(discoveryTimer); + + const keyedClientIds = new Set(candidates.map(([, env]) => env.clientId).filter(Boolean)); + const keyedEnvironmentIds = new Set(candidates.map(([, env]) => env.environmentId).filter(Boolean)); + const unavailable = teamEnvironments.filter( + (env) => !(env.clientId && keyedClientIds.has(env.clientId)) && !keyedEnvironmentIds.has(env.id), + ); + + // One usable profile and nothing else visible: the pick is forced — stay silent. + if (candidates.length < 2 && unavailable.length === 0) return activeEnv; const ui = (await import('../utils/ui.js')).default; const { ExitCode, exitWithCode } = await import('../utils/exit-codes.js'); const chalk = (await import('chalk')).default; + const { formatEnvironmentLabel } = await import('./environment-target.js'); // Lead with the environment (the thing being chosen); the profile key is // bookkeeping and rides dim in the metadata. Labels are column-aligned — // padEnd runs on the PLAIN name before any color, so ANSI codes never // skew the columns (see the env-list alignment bug). const displayFor = (key: string, env: EnvironmentConfig): string => profileEnvironmentLabel(env) ?? key; - const nameW = Math.max(...candidates.map(([key, env]) => displayFor(key, env).length)); + const nameW = Math.max( + ...candidates.map(([key, env]) => displayFor(key, env).length), + ...unavailable.map((env) => formatEnvironmentLabel(env).length), + ); - ui.note(`This machine knows ${candidates.length} WorkOS environments — pick the one this app should call home.`); + // Count the team, not this machine: candidates can include foreign profiles + // (a key from another account left on disk), which must not inflate the team + // totals. teamEnvironments.length IS the team total when discovery ran. + const teamReady = teamEnvironments.length - unavailable.length; + ui.note( + unavailable.length > 0 + ? `Your team has ${teamEnvironments.length} environments; ${teamReady} ${teamReady === 1 ? 'is' : 'are'} ready on this machine — pick the one this app should call home. +○ rows need an API key here first: workos profile add --client-id ` + : `This machine knows ${candidates.length} WorkOS environments — pick the one this app should call home.`, + ); const choice = await ui.select({ message: 'Which WorkOS environment should this install use?', - options: candidates.map(([key, env]) => { - const display = displayFor(key, env); - const type = env.type === 'sandbox' ? 'Sandbox' : env.type === 'unclaimed' ? 'Unclaimed' : 'Production'; - // Only show the profile key when it isn't already the display name. - const meta = [display === key ? null : key, type].filter(Boolean).join(' · '); - let label = `${display.padEnd(nameW)} ${chalk.dim(meta)}`; - if (key === config.activeEnvironment) label += ` ${chalk.green('● active')}`; - return { value: key, label }; - }), + options: [ + ...candidates.map(([key, env]) => { + const display = displayFor(key, env); + const type = env.type === 'sandbox' ? 'Sandbox' : env.type === 'unclaimed' ? 'Unclaimed' : 'Production'; + // Only show the profile key when it isn't already the display name. + const meta = [display === key ? null : key, type].filter(Boolean).join(' · '); + let label = `${display.padEnd(nameW)} ${chalk.dim(meta)}`; + if (key === config.activeEnvironment) label += ` ${chalk.green('● active')}`; + return { value: key, label }; + }), + ...unavailable.map((env) => ({ + value: `__unavailable__${env.id}`, + label: `${formatEnvironmentLabel(env).padEnd(nameW)} ${chalk.dim(env.sandbox ? 'Sandbox' : 'Production')}`, + disabled: '○ no API key on this machine', + })), + ], initialValue: config.activeEnvironment, }); if (ui.isCancel(choice)) exitWithCode(ExitCode.CANCELLED); @@ -157,7 +239,7 @@ export async function resolveInstallCredentials( try { const { getActiveEnvironment, isUnclaimedEnvironment } = await import('./config-store.js'); const { getAccessToken } = await import('./credentials.js'); - const activeEnv = await maybePickInstallEnvironment(getActiveEnvironment(), installDir ?? process.cwd()); + const activeEnv = getActiveEnvironment(); if (activeEnv?.apiKey) { // Has API key — but does it have gateway auth? diff --git a/src/lib/run-with-core.ts b/src/lib/run-with-core.ts index 63229ba4..6dd25116 100644 --- a/src/lib/run-with-core.ts +++ b/src/lib/run-with-core.ts @@ -533,6 +533,18 @@ export async function runWithCore(options: InstallerOptions): Promise { await adapter.start(); + // Environment pick lives AFTER the brand mark (adapter.start) and BEFORE the + // machine starts (branch prompt, detection, …), so the flow reads: banner → + // choose environment → install steps. Running pre-machine also means the + // machine's own auth check sees the PICKED profile — switching to a profile + // that still needs a login is handled by the in-flow device auth. Explicit + // credentials (flag/env var) and headless runs skip it; the helper itself + // guards JSON mode, project-owned keys, and single-profile configs. + if (!headlessMode && !augmentedOptions.apiKey && !process.env.WORKOS_API_KEY) { + const { maybePickInstallEnvironment } = await import('./resolve-install-credentials.js'); + await maybePickInstallEnvironment(getActiveEnvironment(), augmentedOptions.installDir); + } + analytics.configureAuthFromAvailableSources(); const mode = headlessMode ? 'headless' : augmentedOptions.dashboard ? 'tui' : 'cli'; analytics.sessionStart(mode, getVersion()); diff --git a/src/utils/ui.ts b/src/utils/ui.ts index 8d5f7578..ff26fcf7 100644 --- a/src/utils/ui.ts +++ b/src/utils/ui.ts @@ -357,6 +357,8 @@ interface SelectOption { value: T; label?: string; hint?: string; + /** Not selectable; a string renders as the reason next to the row. */ + disabled?: boolean | string; } interface SelectOptions { message: string; @@ -376,6 +378,7 @@ async function select(options: SelectOptions): Promise { value: o.value, name: o.label ?? String(o.value), description: o.hint, + disabled: o.disabled, })), default: options.initialValue, pageSize: options.maxItems,