From b5ed7248038cd6be5ac3e99453bf4af36ce9479b Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 26 Aug 2026 11:29:51 -0500 Subject: [PATCH 1/6] feat(install): show the whole team in the environment picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picker offered only profiles this machine knows about, while the dashboard shows the team's full environment list — installs felt like they were choosing from a different world than the dashboard. The picker now merges in the team's environments (best-effort, via the existing session; offline/logged-out degrades to the local-only list). Environments without a local API key render as disabled rows naming the gap, and the intro note carries the recipe: Your team has 21 environments; 5 are ready on this machine — pick ... ○ rows need an API key here first: workos profile add --client-id ❯ cli-branding-smoke > Staging staging-3 · Sandbox ● active Nick's Team's Project > test12 staging · Sandbox Nick's Team's Project > Production ○ no API key on this machine They stay disabled because they must: the dashboard catalog has no operation that creates or reveals an sk_ secret, so the CLI cannot make an arbitrary environment installable on its own. Also prompts now when a single keyed profile coexists with other visible team environments (previously silent), and ui.select learns a disabled option flag. --- src/lib/resolve-install-credentials.spec.ts | 73 ++++++++++++++++++++ src/lib/resolve-install-credentials.ts | 76 +++++++++++++++++---- src/utils/ui.ts | 3 + 3 files changed, 140 insertions(+), 12 deletions(-) diff --git a/src/lib/resolve-install-credentials.spec.ts b/src/lib/resolve-install-credentials.spec.ts index 5aa7e500..b646d79f 100644 --- a/src/lib/resolve-install-credentials.spec.ts +++ b/src/lib/resolve-install-credentials.spec.ts @@ -43,6 +43,18 @@ vi.mock('./unclaimed-env-provision.js', () => ({ tryProvisionUnclaimedEnv: (...args: unknown[]) => mockTryProvisionUnclaimedEnv(...args), })); +// Session + team-environment discovery for the picker's disabled rows. +const mockRefreshIfExpired = vi.fn(); +vi.mock('./command-auth.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, refreshIfExpired: () => mockRefreshIfExpired() }; +}); +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(); @@ -69,6 +81,7 @@ describe('resolveInstallCredentials', () => { beforeEach(() => { vi.clearAllMocks(); mockGetConfig.mockReturnValue(null); + mockRefreshIfExpired.mockResolvedValue(null); delete process.env.WORKOS_API_KEY; emptyCwd = mkdtempSync(join(tmpdir(), 'resolve-install-credentials-cwd-')); cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(emptyCwd); @@ -439,6 +452,66 @@ describe('resolveInstallCredentials', () => { expect(mockSetActiveEnvironment).not.toHaveBeenCalled(); }); + it('lists team environments without a local key as disabled rows', async () => { + mockGetConfig.mockReturnValue(twoProfiles); + mockRefreshIfExpired.mockResolvedValue({ accessToken: 'tok' }); + 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 resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + + 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 names the gap and the recipe. + const note = String(mockUi.note.mock.calls[0][0]); + expect(note).toContain('3 environments'); + 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'] }, + }); + mockRefreshIfExpired.mockResolvedValue({ accessToken: 'tok' }); + mockFetchTeamEnvironments.mockResolvedValue([ + { id: 'env_prod', name: 'Production', sandbox: false, clientId: 'client_z', projectName: 'P' }, + ]); + mockSelect.mockResolvedValue('staging-3'); + + await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + + expect(mockSelect).toHaveBeenCalled(); + }); + + it('degrades to the local-only picker when team discovery fails', async () => { + mockGetConfig.mockReturnValue(twoProfiles); + mockRefreshIfExpired.mockRejectedValue(new Error('offline')); + mockSelect.mockResolvedValue('staging'); + + await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + + 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); diff --git a/src/lib/resolve-install-credentials.ts b/src/lib/resolve-install-credentials.ts index a1433d6a..e655e072 100644 --- a/src/lib/resolve-install-credentials.ts +++ b/src/lib/resolve-install-credentials.ts @@ -24,6 +24,13 @@ 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( activeEnv: EnvironmentConfig | null, @@ -41,32 +48,77 @@ 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. + // Requires a usable session; any failure degrades to the local-only picker. + let teamEnvironments: Array<{ + id: string; + name: string | null; + sandbox?: boolean | null; + clientId?: string | null; + projectName?: string | null; + }> = []; + try { + const { refreshIfExpired } = await import('./command-auth.js'); + const token = (await refreshIfExpired())?.accessToken; + if (token) { + const { fetchTeamEnvironments } = await import('./environment-target.js'); + teamEnvironments = await fetchTeamEnvironments(token); + } + } catch { + // Offline / logged out / flag-gated — the local-only picker still works. + } + + 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.`); + ui.note( + unavailable.length > 0 + ? `Your team has ${candidates.length + unavailable.length} environments; ${candidates.length} ${candidates.length === 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); 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, From 1a7fd9c525be403db5d926a7cc8e02c876f5a827 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 26 Aug 2026 11:38:46 -0500 Subject: [PATCH 2/6] feat(install): move the environment picker below the brand mark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picker ran in bin.ts pre-flight, so it rendered before the AuthKit brand mark — the flow read: question, THEN banner. It now runs in runWithCore between adapter.start() (banner) and actor.start() (machine), so the flow reads: banner → choose environment → install steps (branch prompt et al). Side benefits of pre-machine placement: - the machine's own auth check runs AFTER the pick, so switching to a profile that still needs a login is handled by the in-flow device auth - explicit credentials (--api-key / WORKOS_API_KEY) and headless runs skip the picker at the call site; the helper keeps its own guards handleInstall now rethrows CliExit so the picker's cancel (exit 2) surfaces as a clean cancel instead of being masked as installer_error. Picker specs retarget the exported helper directly. --- src/commands/install.ts | 5 +++++ src/lib/resolve-install-credentials.spec.ts | 24 +++++++++++---------- src/lib/resolve-install-credentials.ts | 4 ++-- src/lib/run-with-core.ts | 12 +++++++++++ 4 files changed, 32 insertions(+), 13 deletions(-) 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/resolve-install-credentials.spec.ts b/src/lib/resolve-install-credentials.spec.ts index b646d79f..2c3fe2be 100644 --- a/src/lib/resolve-install-credentials.spec.ts +++ b/src/lib/resolve-install-credentials.spec.ts @@ -66,7 +66,9 @@ 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', () => { @@ -382,7 +384,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 }>; @@ -403,7 +405,7 @@ describe('resolveInstallCredentials', () => { mockGetConfig.mockReturnValue(twoProfiles); mockSelect.mockResolvedValue('staging-3'); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSetActiveEnvironment).not.toHaveBeenCalled(); }); @@ -414,7 +416,7 @@ describe('resolveInstallCredentials', () => { environments: { 'staging-3': twoProfiles.environments['staging-3'] }, }); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSelect).not.toHaveBeenCalled(); }); @@ -424,7 +426,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(); @@ -435,7 +437,7 @@ describe('resolveInstallCredentials', () => { mockGetConfig.mockReturnValue(twoProfiles); setOutputMode('json'); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSelect).not.toHaveBeenCalled(); }); @@ -446,7 +448,7 @@ 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(); @@ -469,7 +471,7 @@ describe('resolveInstallCredentials', () => { }); 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; disabled?: string }>; @@ -496,7 +498,7 @@ describe('resolveInstallCredentials', () => { ]); mockSelect.mockResolvedValue('staging-3'); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + await maybePickInstallEnvironment(null, emptyCwd); expect(mockSelect).toHaveBeenCalled(); }); @@ -506,7 +508,7 @@ describe('resolveInstallCredentials', () => { mockRefreshIfExpired.mockRejectedValue(new Error('offline')); mockSelect.mockResolvedValue('staging'); - await resolveInstallCredentials(undefined, undefined, undefined, mockAuthenticate); + 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); @@ -516,7 +518,7 @@ describe('resolveInstallCredentials', () => { 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 e655e072..7725534a 100644 --- a/src/lib/resolve-install-credentials.ts +++ b/src/lib/resolve-install-credentials.ts @@ -32,7 +32,7 @@ import type { EnvironmentConfig } from './config-store.js'; * 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 { @@ -209,7 +209,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()); From 32fa867b58d80ad21ffccecb166c888b096a5b3e Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 26 Aug 2026 13:21:26 -0500 Subject: [PATCH 3/6] fix(install): scope picker team counts to the team and bound discovery Two review findings on the team-wide picker: - The 'Your team has N environments' note totaled candidates + unavailable, counting keyed profiles from another account or team as team members. The team total now comes from the fetched catalog itself; foreign profiles stay selectable but are no longer counted. - Best-effort discovery awaited the refresh client's 30s timeout (and the team fetch after it) before showing even the local-only picker or the single-profile silent path. The whole enrichment is now bounded at 3s and degrades to the local-only picker past that. Addresses PR #232 review. --- src/lib/resolve-install-credentials.spec.ts | 23 +++++++++- src/lib/resolve-install-credentials.ts | 50 ++++++++++++++------- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/lib/resolve-install-credentials.spec.ts b/src/lib/resolve-install-credentials.spec.ts index 2c3fe2be..b298f471 100644 --- a/src/lib/resolve-install-credentials.spec.ts +++ b/src/lib/resolve-install-credentials.spec.ts @@ -481,9 +481,13 @@ describe('resolveInstallCredentials', () => { 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 names the gap and the recipe. + // 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('3 environments'); + expect(note).toContain('2 environments'); + expect(note).toContain('1 is ready'); expect(note).toContain('workos profile add'); }); @@ -514,6 +518,21 @@ describe('resolveInstallCredentials', () => { expect(call.options.every((o) => !o.disabled)).toBe(true); }); + it('does not let a hung session refresh stall the picker', async () => { + mockGetConfig.mockReturnValue(twoProfiles); + // Expired session + dead endpoint: the refresh promise never settles. + // Discovery is bounded, so the picker still opens with local-only rows. + mockRefreshIfExpired.mockReturnValue(new Promise(() => {})); + mockSelect.mockResolvedValue('staging'); + + const start = Date.now(); + await maybePickInstallEnvironment(null, emptyCwd); + + expect(Date.now() - start).toBeLessThan(10_000); // bounded, not the 30s refresh timeout + const call = mockSelect.mock.calls[0][0] as { options: Array<{ disabled?: string }> }; + expect(call.options.every((o) => !o.disabled)).toBe(true); + }, 15_000); + it('cancel cancels the install (exit 2)', async () => { mockGetConfig.mockReturnValue(twoProfiles); mockSelect.mockResolvedValue(CANCEL); diff --git a/src/lib/resolve-install-credentials.ts b/src/lib/resolve-install-credentials.ts index 7725534a..7c57b07c 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 refresh client alone 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 @@ -51,24 +59,28 @@ export async function maybePickInstallEnvironment( if (candidates.length === 0) return activeEnv; // Best-effort: the rest of the team's environments, for the disabled rows. - // Requires a usable session; any failure degrades to the local-only picker. - let teamEnvironments: Array<{ - id: string; - name: string | null; - sandbox?: boolean | null; - clientId?: string | null; - projectName?: string | null; - }> = []; - try { - const { refreshIfExpired } = await import('./command-auth.js'); - const token = (await refreshIfExpired())?.accessToken; - if (token) { + // Any failure degrades to the local-only picker — including slowness: the + // refresh client alone waits 30s before giving up, and this enrichment must + // not stall installer startup (or the single-profile silent path) behind a + // slow or dead endpoint, so the whole discovery is bounded. + const discoverTeam = async (): Promise => { + try { + const { refreshIfExpired } = await import('./command-auth.js'); + const token = (await refreshIfExpired())?.accessToken; + if (!token) return []; const { fetchTeamEnvironments } = await import('./environment-target.js'); - teamEnvironments = await fetchTeamEnvironments(token); + return await fetchTeamEnvironments(token); + } catch { + return []; // Offline / logged out / flag-gated — the local-only picker still works. } - } catch { - // Offline / logged out / flag-gated — the local-only picker still works. - } + }; + const teamEnvironments = await Promise.race([ + discoverTeam(), + new Promise((resolve) => { + const timer = setTimeout(() => resolve([]), TEAM_DISCOVERY_TIMEOUT_MS); + timer.unref?.(); + }), + ]); const keyedClientIds = new Set(candidates.map(([, env]) => env.clientId).filter(Boolean)); const keyedEnvironmentIds = new Set(candidates.map(([, env]) => env.environmentId).filter(Boolean)); @@ -94,9 +106,13 @@ export async function maybePickInstallEnvironment( ...unavailable.map((env) => formatEnvironmentLabel(env).length), ); + // 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 ${candidates.length + unavailable.length} environments; ${candidates.length} ${candidates.length === 1 ? 'is' : 'are'} ready on this machine — pick the one this app should call home. + ? `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.`, ); From e4deac52881d5dc0873929a831968f91e54b569d Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 26 Aug 2026 13:50:24 -0500 Subject: [PATCH 4/6] fix(install): never refresh inside the environment picker The 3s discovery bound abandoned the losing promise without cancelling it, and a refresh outliving the picker can answer invalid_grant after the installer's auth flow writes a new session -- and invalid_grant clears the credential store. The picker now only uses a currently-valid stored token; an expired session degrades to the local-only picker and the machine's own auth check re-authenticates later. The bounded race now wraps only the read-only team fetch. Addresses PR #232 review. --- src/lib/resolve-install-credentials.spec.ts | 48 +++++++++++++++------ src/lib/resolve-install-credentials.ts | 25 ++++++----- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/lib/resolve-install-credentials.spec.ts b/src/lib/resolve-install-credentials.spec.ts index b298f471..6ae58755 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,12 +47,9 @@ vi.mock('./unclaimed-env-provision.js', () => ({ tryProvisionUnclaimedEnv: (...args: unknown[]) => mockTryProvisionUnclaimedEnv(...args), })); -// Session + team-environment discovery for the picker's disabled rows. -const mockRefreshIfExpired = vi.fn(); -vi.mock('./command-auth.js', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, refreshIfExpired: () => mockRefreshIfExpired() }; -}); +// 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(); @@ -83,7 +84,8 @@ describe('resolveInstallCredentials', () => { beforeEach(() => { vi.clearAllMocks(); mockGetConfig.mockReturnValue(null); - mockRefreshIfExpired.mockResolvedValue(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); @@ -456,7 +458,7 @@ describe('resolveInstallCredentials', () => { it('lists team environments without a local key as disabled rows', async () => { mockGetConfig.mockReturnValue(twoProfiles); - mockRefreshIfExpired.mockResolvedValue({ accessToken: 'tok' }); + 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' }, @@ -496,7 +498,7 @@ describe('resolveInstallCredentials', () => { activeEnvironment: 'staging-3', environments: { 'staging-3': twoProfiles.environments['staging-3'] }, }); - mockRefreshIfExpired.mockResolvedValue({ accessToken: 'tok' }); + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: Date.now() + 3_600_000 }); mockFetchTeamEnvironments.mockResolvedValue([ { id: 'env_prod', name: 'Production', sandbox: false, clientId: 'client_z', projectName: 'P' }, ]); @@ -509,7 +511,8 @@ describe('resolveInstallCredentials', () => { it('degrades to the local-only picker when team discovery fails', async () => { mockGetConfig.mockReturnValue(twoProfiles); - mockRefreshIfExpired.mockRejectedValue(new Error('offline')); + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: Date.now() + 3_600_000 }); + mockFetchTeamEnvironments.mockRejectedValue(new Error('offline')); mockSelect.mockResolvedValue('staging'); await maybePickInstallEnvironment(null, emptyCwd); @@ -518,21 +521,38 @@ describe('resolveInstallCredentials', () => { expect(call.options.every((o) => !o.disabled)).toBe(true); }); - it('does not let a hung session refresh stall the picker', async () => { + it('does not let a hung team fetch stall the picker', async () => { mockGetConfig.mockReturnValue(twoProfiles); - // Expired session + dead endpoint: the refresh promise never settles. + // Valid session + dead endpoint: the fetch promise never settles. // Discovery is bounded, so the picker still opens with local-only rows. - mockRefreshIfExpired.mockReturnValue(new Promise(() => {})); + mockGetCredentials.mockReturnValue({ accessToken: 'tok', expiresAt: Date.now() + 3_600_000 }); + mockFetchTeamEnvironments.mockReturnValue(new Promise(() => {})); mockSelect.mockResolvedValue('staging'); const start = Date.now(); await maybePickInstallEnvironment(null, emptyCwd); - expect(Date.now() - start).toBeLessThan(10_000); // bounded, not the 30s refresh timeout + 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); }, 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); diff --git a/src/lib/resolve-install-credentials.ts b/src/lib/resolve-install-credentials.ts index 7c57b07c..9c54d59b 100644 --- a/src/lib/resolve-install-credentials.ts +++ b/src/lib/resolve-install-credentials.ts @@ -13,8 +13,8 @@ import type { TeamEnvironment } from './environment-target.js'; /** * Upper bound on team-environment discovery before the picker falls back to - * local profiles. The refresh client alone can wait 30s on a dead endpoint; - * the picker (and the single-profile silent path) must not inherit that wait. + * 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; @@ -59,17 +59,22 @@ export async function maybePickInstallEnvironment( 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: the - // refresh client alone waits 30s before giving up, and this enrichment must - // not stall installer startup (or the single-profile silent path) behind a - // slow or dead endpoint, so the whole discovery is bounded. + // 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 discoverTeam = async (): Promise => { try { - const { refreshIfExpired } = await import('./command-auth.js'); - const token = (await refreshIfExpired())?.accessToken; - if (!token) return []; + 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(token); + return await fetchTeamEnvironments(creds.accessToken); } catch { return []; // Offline / logged out / flag-gated — the local-only picker still works. } From 8fb1f6bce9378251755691f4df43ea7ed0d6c061 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 26 Aug 2026 14:11:41 -0500 Subject: [PATCH 5/6] chore: formatting --- src/lib/resolve-install-credentials.spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib/resolve-install-credentials.spec.ts b/src/lib/resolve-install-credentials.spec.ts index 6ae58755..e00fabeb 100644 --- a/src/lib/resolve-install-credentials.spec.ts +++ b/src/lib/resolve-install-credentials.spec.ts @@ -67,9 +67,8 @@ const mockUi = { }; vi.mock('../utils/ui.js', () => ({ default: mockUi })); -const { resolveInstallCredentials, resolveStagingCredentials, maybePickInstallEnvironment } = 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', () => { From 22db1093c87777f66df2d989b837f533aad3c8e1 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 26 Aug 2026 14:22:03 -0500 Subject: [PATCH 6/6] fix(install): cancel team discovery's request when the race times out A lost race returned the local-only picker but left the dashboard request running: its socket and 30s abort timer keep the event loop alive, so a quickly finished (or cancelled) install lingered until the transport timeout. The timeout now aborts an AbortController threaded through fetchTeamEnvironments into the dashboard transport, which merges caller signals into its own controller. The hung-fetch test asserts the signal fires. Addresses PR #232 review. --- src/lib/dashboard-graphql.ts | 8 ++++++++ src/lib/environment-target.ts | 4 ++-- src/lib/resolve-install-credentials.spec.ts | 9 ++++++++- src/lib/resolve-install-credentials.ts | 15 ++++++++++++--- 4 files changed, 30 insertions(+), 6 deletions(-) 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 e00fabeb..a3969eeb 100644 --- a/src/lib/resolve-install-credentials.spec.ts +++ b/src/lib/resolve-install-credentials.spec.ts @@ -525,7 +525,11 @@ describe('resolveInstallCredentials', () => { // 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 }); - mockFetchTeamEnvironments.mockReturnValue(new Promise(() => {})); + let fetchSignal: AbortSignal | undefined; + mockFetchTeamEnvironments.mockImplementation((_token: string, signal?: AbortSignal) => { + fetchSignal = signal; + return new Promise(() => {}); + }); mockSelect.mockResolvedValue('staging'); const start = Date.now(); @@ -534,6 +538,9 @@ describe('resolveInstallCredentials', () => { 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 () => { diff --git a/src/lib/resolve-install-credentials.ts b/src/lib/resolve-install-credentials.ts index 9c54d59b..522755f9 100644 --- a/src/lib/resolve-install-credentials.ts +++ b/src/lib/resolve-install-credentials.ts @@ -68,24 +68,33 @@ export async function maybePickInstallEnvironment( // 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); + 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) => { - const timer = setTimeout(() => resolve([]), TEAM_DISCOVERY_TIMEOUT_MS); - timer.unref?.(); + 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));