Skip to content

Commit cef5447

Browse files
committed
fix: address friction log issues from Greg's workos install test
- Fix silent commit failure: track postInstallError in state machine context so emitComplete emits success: false when commit/push/PR fails - Capture pre-commit hook output: stageAndCommit now pipes stderr/stdout and includes hook output in error messages - Include WORKOS_COOKIE_PASSWORD and redirect URI in Vercel env uploads by reading .env.local after generation - Improve port detection: detect PORT=NNNN in dev scripts and .env files - Add existing env var conflict detection before Vercel upload prompt - Enhance agent spinner with file operation names and elapsed time - Add tests for post-install, port detection, and env var upload
1 parent 38c19ec commit cef5447

12 files changed

Lines changed: 433 additions & 14 deletions

File tree

src/lib/adapters/cli-adapter.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ export class CLIAdapter implements InstallerAdapter {
3232

3333
// Long-running agent update interval
3434
private agentUpdateInterval: NodeJS.Timeout | null = null;
35+
private agentStartTime: number | null = null;
36+
private lastFileOperation: string | null = null;
3537

3638
constructor(config: AdapterConfig) {
3739
this.emitter = config.emitter;
@@ -107,6 +109,8 @@ export class CLIAdapter implements InstallerAdapter {
107109
this.subscribe('config:complete', this.handleConfigComplete);
108110
this.subscribe('agent:start', this.handleAgentStart);
109111
this.subscribe('agent:progress', this.handleAgentProgress);
112+
this.subscribe('file:write', this.handleFileWrite);
113+
this.subscribe('file:edit', this.handleFileEdit);
110114
this.subscribe('validation:start', this.handleValidationStart);
111115
this.subscribe('validation:issues', this.handleValidationIssues);
112116
this.subscribe('validation:complete', this.handleValidationComplete);
@@ -161,6 +165,8 @@ export class CLIAdapter implements InstallerAdapter {
161165
clearInterval(this.agentUpdateInterval);
162166
this.agentUpdateInterval = null;
163167
}
168+
this.agentStartTime = null;
169+
this.lastFileOperation = null;
164170
};
165171

166172
private stopSpinner(message: string): void {
@@ -346,15 +352,18 @@ export class CLIAdapter implements InstallerAdapter {
346352
};
347353

348354
private handleAgentStart = (): void => {
355+
this.agentStartTime = Date.now();
356+
this.lastFileOperation = null;
349357
this.spinner = clack.spinner();
350358
this.spinner.start('Running AI agent...');
351359

352-
// Periodic status updates for long-running operations
353-
let dots = 0;
360+
// Periodic status updates with elapsed time
354361
this.agentUpdateInterval = setInterval(() => {
355-
dots = (dots + 1) % 4;
356-
const dotStr = '.'.repeat(dots + 1);
357-
this.spinner?.message(`Running AI agent${dotStr}`);
362+
const elapsed = Math.round((Date.now() - (this.agentStartTime ?? Date.now())) / 1000);
363+
const timeStr =
364+
elapsed >= 60 ? `${Math.floor(elapsed / 60)}m ${elapsed % 60}s` : `${elapsed}s`;
365+
const detail = this.lastFileOperation ?? 'Working';
366+
this.spinner?.message(`${detail} (${timeStr})`);
358367
}, 2000);
359368
};
360369

@@ -363,6 +372,18 @@ export class CLIAdapter implements InstallerAdapter {
363372
this.spinner?.message(message);
364373
};
365374

375+
private handleFileWrite = ({ path }: InstallerEvents['file:write']): void => {
376+
const shortPath = path.split('/').slice(-2).join('/');
377+
this.lastFileOperation = `Writing ${shortPath}`;
378+
this.spinner?.message(this.lastFileOperation);
379+
};
380+
381+
private handleFileEdit = ({ path }: InstallerEvents['file:edit']): void => {
382+
const shortPath = path.split('/').slice(-2).join('/');
383+
this.lastFileOperation = `Editing ${shortPath}`;
384+
this.spinner?.message(this.lastFileOperation);
385+
};
386+
366387
private handleValidationStart = (): void => {
367388
this.stopAgentUpdates();
368389
this.stopSpinner('Agent completed');

src/lib/agent-runner.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import { uploadEnvironmentVariablesStep } from '../steps/index.js';
1515
import { autoConfigureWorkOSEnvironment } from './workos-management.js';
1616
import { detectPort, getCallbackPath } from './port-detection.js';
1717
import { writeEnvLocal } from './env-writer.js';
18+
import { readFile, access } from 'node:fs/promises';
19+
import { join } from 'node:path';
20+
import { parseEnvFile } from '../utils/env-parser.js';
1821

1922
/**
2023
* Universal agent-powered wizard runner.
@@ -175,6 +178,25 @@ export async function runAgentInstaller(config: FrameworkConfig, options: Instal
175178
// Build environment variables from WorkOS credentials
176179
const envVars = config.environment.getEnvVars(apiKey, clientId);
177180

181+
// Supplement with additional vars from .env.local (cookie password, redirect URI)
182+
// These are generated by writeEnvLocal during configuration but not in getEnvVars.
183+
// We read .env.local rather than changing the shared getEnvVars interface (used by 15+ integrations).
184+
const envLocalPath = join(options.installDir, '.env.local');
185+
try {
186+
await access(envLocalPath);
187+
const envLocal = parseEnvFile(await readFile(envLocalPath, 'utf-8'));
188+
if (envLocal.WORKOS_COOKIE_PASSWORD) {
189+
envVars.WORKOS_COOKIE_PASSWORD = envLocal.WORKOS_COOKIE_PASSWORD;
190+
}
191+
for (const key of Object.keys(envLocal)) {
192+
if (key.endsWith('WORKOS_REDIRECT_URI') && !envVars[key]) {
193+
envVars[key] = envLocal[key];
194+
}
195+
}
196+
} catch {
197+
// .env.local doesn't exist yet — vars will be written by agent
198+
}
199+
178200
// Upload environment variables to hosting providers (if configured)
179201
let uploadedEnvVars: string[] = [];
180202
if (config.environment.uploadToHosting) {

src/lib/installer-core.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,9 @@ export const installerMachine = setup({
241241
const message = context.error?.message ?? 'Commit failed';
242242
context.emitter.emit('postinstall:commit:failed', { error: message });
243243
},
244+
assignPostInstallError: assign({
245+
postInstallError: ({ context }) => context.error?.message ?? 'Post-install step failed',
246+
}),
244247
emitPrPrompt: ({ context }) => {
245248
context.emitter.emit('postinstall:pr:prompt', {});
246249
},
@@ -282,8 +285,11 @@ export const installerMachine = setup({
282285
context.emitter.emit('postinstall:manual', { instructions });
283286
},
284287
emitComplete: ({ context }) => {
285-
const summary = context.agentSummary ?? 'WorkOS AuthKit installed successfully!';
286-
context.emitter.emit('complete', { success: true, summary });
288+
const hasPostInstallError = !!context.postInstallError;
289+
const summary = hasPostInstallError
290+
? `Installation completed but post-install failed: ${context.postInstallError}`
291+
: context.agentSummary ?? 'WorkOS AuthKit installed successfully!';
292+
context.emitter.emit('complete', { success: !hasPostInstallError, summary });
287293
},
288294
},
289295

@@ -936,7 +942,7 @@ export const installerMachine = setup({
936942
},
937943
onError: {
938944
target: 'done',
939-
actions: ['assignError', 'emitCommitFailed'],
945+
actions: ['assignError', 'assignPostInstallError', 'emitCommitFailed'],
940946
},
941947
},
942948
},
@@ -989,7 +995,7 @@ export const installerMachine = setup({
989995
onDone: { target: 'creatingPr' },
990996
onError: {
991997
target: 'showingManualInstructions',
992-
actions: ['assignError', 'emitPushFailed'],
998+
actions: ['assignError', 'assignPostInstallError', 'emitPushFailed'],
993999
},
9941000
},
9951001
},
@@ -1010,7 +1016,7 @@ export const installerMachine = setup({
10101016
},
10111017
onError: {
10121018
target: 'done',
1013-
actions: ['assignError', 'emitPrFailed'],
1019+
actions: ['assignError', 'assignPostInstallError', 'emitPrFailed'],
10141020
},
10151021
},
10161022
},

src/lib/installer-core.types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ export interface InstallerMachineContext {
5959
prUrl?: string;
6060
/** Summary message from agent execution */
6161
agentSummary?: string;
62+
/** Error from post-install steps (commit, push, PR) */
63+
postInstallError?: string;
6264
}
6365

6466
/**

src/lib/port-detection.spec.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { describe, it, expect, vi, beforeEach } from 'vitest';
2+
import * as fs from 'node:fs';
3+
import { detectPort } from './port-detection.js';
4+
5+
vi.mock('node:fs', async () => {
6+
const actual = await vi.importActual<typeof import('node:fs')>('node:fs');
7+
return {
8+
...actual,
9+
readFileSync: vi.fn(),
10+
};
11+
});
12+
13+
vi.mock('./settings.js', () => ({
14+
getConfig: () => ({
15+
frameworks: {},
16+
}),
17+
}));
18+
19+
const mockReadFileSync = vi.mocked(fs.readFileSync);
20+
21+
describe('port-detection', () => {
22+
beforeEach(() => {
23+
vi.clearAllMocks();
24+
});
25+
26+
describe('detectPort for nextjs', () => {
27+
it('detects port from -p flag in dev script', () => {
28+
mockReadFileSync.mockReturnValue(
29+
JSON.stringify({ scripts: { dev: 'next dev -p 4000' } }),
30+
);
31+
expect(detectPort('nextjs', '/test')).toBe(4000);
32+
});
33+
34+
it('detects port from --port flag in dev script', () => {
35+
mockReadFileSync.mockReturnValue(
36+
JSON.stringify({ scripts: { dev: 'next dev --port 5000' } }),
37+
);
38+
expect(detectPort('nextjs', '/test')).toBe(5000);
39+
});
40+
41+
it('detects port from --port= flag in dev script', () => {
42+
mockReadFileSync.mockReturnValue(
43+
JSON.stringify({ scripts: { dev: 'next dev --port=3123' } }),
44+
);
45+
expect(detectPort('nextjs', '/test')).toBe(3123);
46+
});
47+
48+
it('detects PORT=NNNN inline env var in dev script', () => {
49+
mockReadFileSync.mockReturnValue(
50+
JSON.stringify({ scripts: { dev: 'PORT=3123 next dev' } }),
51+
);
52+
expect(detectPort('nextjs', '/test')).toBe(3123);
53+
});
54+
55+
it('prefers -p flag over PORT= env var', () => {
56+
mockReadFileSync.mockReturnValue(
57+
JSON.stringify({ scripts: { dev: 'PORT=4000 next dev -p 5000' } }),
58+
);
59+
expect(detectPort('nextjs', '/test')).toBe(5000);
60+
});
61+
62+
it('falls back to .env file PORT when no script port', () => {
63+
// First call: package.json (no port in script)
64+
mockReadFileSync.mockReturnValueOnce(
65+
JSON.stringify({ scripts: { dev: 'next dev' } }),
66+
);
67+
// Second call: .env.local
68+
mockReadFileSync.mockReturnValueOnce('PORT=3123\nOTHER_VAR=foo');
69+
70+
expect(detectPort('nextjs', '/test')).toBe(3123);
71+
});
72+
73+
it('falls back to default 3000 when no port found anywhere', () => {
74+
// package.json - no port
75+
mockReadFileSync.mockReturnValueOnce(
76+
JSON.stringify({ scripts: { dev: 'next dev' } }),
77+
);
78+
// All .env files throw (don't exist)
79+
mockReadFileSync.mockImplementation(() => {
80+
throw new Error('ENOENT');
81+
});
82+
83+
expect(detectPort('nextjs', '/test')).toBe(3000);
84+
});
85+
});
86+
});

src/lib/port-detection.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ function parseViteConfigPort(configPath: string): number | null {
4646

4747
/**
4848
* Parse port from Next.js package.json scripts.
49-
* Next.js uses: "dev": "next dev -p 4000" or --port 4000
49+
* Checks for: -p 4000, --port 4000, --port=4000, PORT=4000
5050
*/
5151
function parseNextConfigPort(installDir: string): number | null {
5252
try {
@@ -60,12 +60,38 @@ function parseNextConfigPort(installDir: string): number | null {
6060
if (portMatch) {
6161
return parseInt(portMatch[1] || portMatch[2], 10);
6262
}
63+
64+
// Match: PORT=4000 (inline env var in script)
65+
const portEnvMatch = devScript.match(/PORT=(\d+)/);
66+
if (portEnvMatch) {
67+
return parseInt(portEnvMatch[1], 10);
68+
}
6369
} catch {
6470
// Can't read package.json
6571
}
6672
return null;
6773
}
6874

75+
/**
76+
* Parse PORT from .env files.
77+
* Checks .env.local, .env.development, .env in order.
78+
*/
79+
function parseEnvFilePort(installDir: string): number | null {
80+
const envFiles = ['.env.local', '.env.development', '.env'];
81+
for (const file of envFiles) {
82+
try {
83+
const content = fs.readFileSync(join(installDir, file), 'utf-8');
84+
const match = content.match(/^PORT=(\d+)/m);
85+
if (match) {
86+
return parseInt(match[1], 10);
87+
}
88+
} catch {
89+
// File doesn't exist
90+
}
91+
}
92+
return null;
93+
}
94+
6995
/**
7096
* Parse port from TanStack Start app.config.ts.
7197
* Uses Vinxi: server: { port: N }
@@ -121,5 +147,6 @@ export function detectPort(integration: Integration, installDir: string): number
121147
}
122148
}
123149

124-
return detectedPort ?? getDefaultPort(integration);
150+
// Fallback: check .env files for PORT before using framework default
151+
return detectedPort ?? parseEnvFilePort(installDir) ?? getDefaultPort(integration);
125152
}

0 commit comments

Comments
 (0)