diff --git a/CLAUDE.md b/CLAUDE.md index 3888921b..df922d70 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,8 @@ WorkOS CLI for installing AuthKit integrations and managing WorkOS resources (or - **Auth**: Exits code 4 instead of opening browser. Resource commands (organization, user, role, permission, membership, invitation, session, event, feature-flag, org-domain, portal, webhook, config) use the dashboard session from a prior `workos auth login`; expired access tokens refresh silently while the stored refresh token is valid, so only a truly dead session exits 4. `WORKOS_API_KEY` applies only to `workos api` and the still-REST commands (`connection`, `directory`, `audit-log`, `api-key`, `vault`, plus the workflow/debug commands `seed`, `setup-org`, `onboard-user`, `verify-login`, `debug-sso`, `debug-sync`, `migrations`). - **Errors**: Structured JSON to stderr: `{ "error": { "code": "...", "message": "..." } }` - **Exit codes**: 0=success, 1=error, 2=cancelled, 4=auth required (follows `gh` CLI convention) -- **Headless flags**: `--no-branch`, `--no-commit`, `--create-pr`, `--no-git-check`. CI mode (`WORKOS_MODE=ci`) auto-continues past a dirty tree without `--no-git-check`; agent mode requires the flag. +- **Headless flags**: `--no-branch`, `--no-git-check`. CI mode (`WORKOS_MODE=ci`) auto-continues past a dirty tree without `--no-git-check`; agent mode requires the flag. +- **Installer Git policy**: Generated changes stay unstaged/uncommitted; pre-existing staging is preserved. No installer-controlled staging, commits, pushes, PR creation, or commit/PR text generation. `--commit`, `--no-commit`, and `--create-pr` are deprecated compatibility-only no-ops, with human-only notices. Branch prompts/`--no-branch` remain unchanged; branches do not isolate uncommitted work. Change reporting is scoped to `installDir` and may include pre-existing changes. ## JSON Output Conventions diff --git a/README.md b/README.md index e2e2932c..f2c51eda 100644 --- a/README.md +++ b/README.md @@ -546,14 +546,26 @@ workos install [options] --pm Package manager for the scaffolded app: npm, pnpm, yarn, bun --no-validate Skip post-installation validation --no-branch Skip branch creation (use current branch) - --no-commit Skip auto-commit after installation - --create-pr Auto-create pull request after installation + --commit / --no-commit Deprecated compatibility-only no-ops + --create-pr Deprecated compatibility-only no-op (never publishes) --no-git-check Skip git dirty working tree check --force-install Force install packages even if peer dependency checks fail --no-tui Use plain line-by-line output instead of the full-screen installer --debug Enable verbose logging ``` +The installer leaves generated changes unstaged and uncommitted, preserving any +previously staged work. It never stages, commits, pushes, opens a pull request, or +generates commit/PR text. Review the project and commit independently when ready. +The deprecated Git flags above (including boolean/negated forms) are accepted but +ignored; human runs show a notice, while JSON runs keep machine streams clean. +Branch creation and `--no-branch` are unchanged: uncommitted changes are not +isolated by creating a branch. Reported changed files may include pre-existing +work; inspection is scoped to `--install-dir`. Each of its two Git commands is +limited to 5 seconds and 1 MiB of buffered output. If inspection hits either +limit, it reports unknown changed files (not “no changes”); review the project +manually. Partial output is never presented as a complete file list. + **Full-screen installer:** In an interactive terminal of at least 80×24, `workos install` opens a full-screen view: a plain-English walkthrough of what it's doing, a task list beside it that follows the dashboard's AuthKit @@ -653,7 +665,7 @@ Mode resolution notes: In non-TTY, the installer streams progress as NDJSON (one JSON object per line): ```bash -workos install --api-key sk_test_xxx --client-id client_xxx --no-commit 2>/dev/null +workos install --api-key sk_test_xxx --client-id client_xxx 2>/dev/null # → {"type":"detection:complete","integration":"nextjs","timestamp":"..."} # → {"type":"agent:start","timestamp":"..."} # → {"type":"agent:progress","message":"...","timestamp":"..."} diff --git a/src/bin-readonly-installer.integration.spec.ts b/src/bin-readonly-installer.integration.spec.ts new file mode 100644 index 00000000..43c72bfc --- /dev/null +++ b/src/bin-readonly-installer.integration.spec.ts @@ -0,0 +1,288 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = fileURLToPath(new URL('..', import.meta.url)); +const fixture = join(root, 'src/test/readonly-installer.fixture.ts'); +let sandbox: string; +let project: string; +let env: NodeJS.ProcessEnv; +const git = (dir: string, ...args: string[]) => execFileSync('git', args, { cwd: dir, env, encoding: 'utf8' }); + +function repo(dir: string) { + mkdirSync(dir, { recursive: true }); + git(dir, 'init', '-q', '-b', 'main'); + writeFileSync(join(dir, 'package.json'), '{"name":"offline-project","scripts":{"dev":"vite"}}'); + writeFileSync(join(dir, 'tracked.ts'), 'original\n'); + writeFileSync(join(dir, 'staged.ts'), 'original\n'); + writeFileSync(join(dir, '.gitignore'), '.env*\n'); + git(dir, 'add', '.'); + git(dir, '-c', 'commit.gpgsign=false', 'commit', '-qm', 'fixture'); + writeFileSync(join(dir, 'staged.ts'), 'pre-existing staged work\n'); + git(dir, 'add', 'staged.ts'); + writeFileSync(join(dir, 'staged.ts'), 'pre-existing unstaged work\n'); + writeFileSync(join(dir, '.env.local'), 'WORKOS_API_KEY=sk_test_offline\nWORKOS_CLIENT_ID=client_offline\n'); +} + +beforeEach(() => { + sandbox = mkdtempSync(join(root, '.auth6733-test-')); + const home = join(sandbox, 'home'); + mkdirSync(home); + const gitConfig = join(home, 'gitconfig'); + writeFileSync(gitConfig, ''); + env = { + PATH: process.env.PATH, + HOME: home, + USERPROFILE: home, + TMPDIR: home, + TMP: home, + TEMP: home, + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: gitConfig, + GIT_AUTHOR_NAME: 'Offline Test', + GIT_AUTHOR_EMAIL: 'offline@example.test', + GIT_COMMITTER_NAME: 'Offline Test', + GIT_COMMITTER_EMAIL: 'offline@example.test', + WORKOS_TELEMETRY: 'false', + NO_COLOR: '1', + TERM: 'dumb', + TEST_EVIDENCE: join(sandbox, 'evidence.ndjson'), + }; + // Windows subprocess startup needs these OS paths; never inherit credentials. + for (const key of ['SystemRoot', 'WINDIR', 'COMSPEC', 'PATHEXT']) { + if (process.env[key]) env[key] = process.env[key]; + } + writeFileSync(env.TEST_EVIDENCE!, ''); + project = join(sandbox, 'project'); + repo(project); +}); + +afterEach(() => rmSync(sandbox, { recursive: true, force: true })); + +function run(args: string[], overrides: NodeJS.ProcessEnv = {}, cwd = project, expectedForbidden: string[] = []) { + const head = git(project, 'rev-parse', 'HEAD'); + const index = git(project, 'ls-files', '--stage'); + const result = spawnSync('bun', [fixture, ...args], { + cwd, + env: { ...env, ...overrides }, + encoding: 'utf8', + timeout: 20_000, + }); + expect(result.error).toBeUndefined(); + const evidence = readFileSync(env.TEST_EVIDENCE!, 'utf8') + .trim() + .split('\n') + .filter(Boolean) + .map((s) => JSON.parse(s)); + expect(evidence.filter((e) => e.kind === 'forbidden').map((e) => e.value)).toEqual(expectedForbidden); + expect(git(project, 'rev-parse', 'HEAD')).toBe(head); + // Git status can refresh stat metadata, but never the staged entries. + expect(git(project, 'ls-files', '--stage')).toBe(index); + expect(git(project, 'show', ':staged.ts')).toBe('pre-existing staged work\n'); + expect(git(project, 'diff', '--cached', '--name-only')).toBe('staged.ts\n'); + return { ...result, evidence, index }; +} + +const install = [ + 'install', + '--skip-auth', + '--api-key', + 'sk_test_offline', + '--client-id', + 'client_offline', + '--no-git-check', + '--no-branch', +]; +const events = (stdout: string) => + stdout + .trim() + .split('\n') + .map((s) => JSON.parse(s)); + +function expectSuccess(result: ReturnType) { + expect(result.status, result.stderr + result.stdout).toBe(0); + const complete = events(result.stdout).find((e) => e.type === 'complete'); + expect(complete.success).toBe(true); + expect(complete.files).toEqual(expect.arrayContaining(['generated.ts', 'tracked.ts', 'staged.ts'])); + expect(complete.changeDetection.state).toBe('changed'); + expect(complete.applicationSetup.reason).toContain('Offline fixture'); + expect(complete.nextSteps.join(' ')).toContain('uncommitted'); + expect(events(result.stdout).some((e) => e.type === 'validation:complete')).toBe(true); + expect(events(result.stdout).some((e) => /commit:|pr:|push:/.test(e.type))).toBe(false); + expect(git(project, 'status', '--porcelain')).toContain('?? generated.ts'); + expect(git(project, 'diff', '--name-only')).toContain('tracked.ts'); +} + +describe('installer leaves changes uncommitted through the real parser and orchestrator', () => { + it('records forbidden process calls even when swallowed, without relying on executable shims', () => { + const result = run([], { TEST_ENTRY: 'guard-probe' }, project, [ + 'execFileSync git add -A', + 'execFileSync git commit -m forbidden', + 'execFileSync git push', + 'execFileSync gh pr create', + 'execSync git status --porcelain=v1 && git push', + 'exec', + 'execFile', + 'spawn', + 'spawnSync', + 'fork', + ]); + expect(result.status, result.stderr).toBe(0); + expect(result.evidence.filter((e) => e.kind === 'command').length).toBe(4); + }); + + it.each( + [ + [], + ['--commit'], + ['--no-commit'], + ['--commit=true'], + ['--commit=false'], + ['--create-pr'], + ['--create-pr=true'], + ['--create-pr=false'], + ['--no-create-pr'], + ['--commit', 'false', '--create-pr', 'false'], + ['--direct', '--commit', '--create-pr'], + ['--commit', '--no-commit', '--create-pr'], + ['--no-commit', '--commit', '--no-create-pr'], + ].map((flags) => ({ flags })), + )('accepts legacy forms $flags without publication or model calls (JSON)', ({ flags }) => { + const result = run([...install, ...flags, '--json']); + expectSuccess(result); + expect(result.stderr).toBe(''); + // Normalization drops obsolete controls entirely, including omitted flags. + expect(result.evidence.find((e) => e.kind === 'agent-options').value).toEqual({ installDir: project }); + expect(git(project, 'branch', '--show-current')).toBe('main\n'); + }); + + it('documents compatibility-only flags without active defaults in human and machine help', () => { + const human = run(['install', '--help'], { TEST_HUMAN: '1' }); + expect(human.status).toBe(0); + expect(human.stdout).toContain('Deprecated no-op'); + expect(human.stdout).not.toContain('Auto-commit'); + const machine = run(['install', '--help', '--json']); + expect(machine.status).toBe(0); + const options = JSON.parse(machine.stdout).options; + for (const name of ['commit', 'create-pr']) { + const option = options.find((o: { name: string }) => o.name === name); + expect(option.description).toContain('Deprecated no-op'); + expect(option).not.toHaveProperty('default'); + } + expect(machine.stderr).toBe(''); + }); + + it('CI completes on a dirty tree and preserves automatic branch creation', () => { + const result = run( + [ + 'install', + '--api-key', + 'sk_test_offline', + '--client-id', + 'client_offline', + '--install-dir', + project, + '--create-pr', + ], + { WORKOS_MODE: 'ci' }, + ); + expectSuccess(result); + expect(result.stderr).toBe(''); + expect(git(project, 'branch', '--show-current')).toBe('feat/add-workos-authkit\n'); + }); + + it('runtime/programmatic legacy values cannot revive removed actions', () => { + expectSuccess(run([], { TEST_ENTRY: 'programmatic' })); + }); + + it.each(['install', 'default'])('human %s path finishes with only pre-install prompts', (entry) => { + const result = run(entry === 'install' ? ['install', '--commit', '--create-pr'] : ['--no-commit', '--create-pr'], { + TEST_HUMAN: '1', + }); + expect(result.status, result.stderr + result.stdout).toBe(0); + expect(result.stdout + result.stderr).toContain('Deprecated installer Git flags are ignored'); + expect(result.stdout).toContain('The installer leaves changes uncommitted'); + expect(result.evidence.filter((e) => e.kind === 'prompt').map((e) => e.value)).toEqual( + entry === 'default' + ? ['Run the AuthKit installer?', 'Continue anyway?', 'You are on main. Create a feature branch?'] + : ['Continue anyway?', 'You are on main. Create a feature branch?'], + ); + expect(git(project, 'branch', '--show-current')).toBe('feat/add-workos-authkit\n'); + }); + + it('does not warn when legacy options are omitted in human mode', () => { + const result = run(install, { TEST_HUMAN: '1' }); + expect(result.status, result.stderr + result.stdout).toBe(0); + expect(result.stdout + result.stderr).not.toContain('Deprecated installer Git flags'); + // Preserve the existing limitation: human CLI still asks the branch question + // with --no-branch; only the headless adapter consumes that option today. + expect(result.evidence.filter((e) => e.kind === 'prompt').map((e) => e.value)).toContain( + 'You are on main. Create a feature branch?', + ); + expect(git(project, 'branch', '--show-current')).toBe('feat/add-workos-authkit\n'); + }); + + it('reports installDir instead of the different repository in process cwd', () => { + const other = join(sandbox, 'other'); + repo(other); + writeFileSync(join(other, 'wrong-repository.txt'), 'not the target'); + const result = run([...install, '--install-dir', project, '--json'], {}, other); + expectSuccess(result); + const output = events(result.stdout); + expect(output.find((e) => e.type === 'postinstall:changes').files).not.toContain('wrong-repository.txt'); + expect(output.find((e) => e.type === 'complete').files).not.toContain('wrong-repository.txt'); + // Existing pre-install policy is intentionally unchanged in this ticket: + // its dirty-tree check still inspects process cwd, unlike post-install. + expect(output.find((e) => e.type === 'git:status').files).toContain('- wrong-repository.txt'); + }); + + it.each([ + { probe: 'timeout', detail: 'timed out after 5000 ms' }, + { probe: 'overflow', detail: 'exceeded the 1048576-byte output limit' }, + ])( + 'reports unknown changed files after a real Bun $probe, never unchanged or a partial list', + ({ probe, detail }) => { + const result = run([...install, '--json'], { TEST_INSPECTION: probe }); + expect(result.status, result.stderr + result.stdout).toBe(0); + expect(result.stderr).toBe(''); + const output = events(result.stdout); + expect(output.filter((e) => e.type.startsWith('postinstall:'))).toEqual([ + expect.objectContaining({ + type: 'postinstall:unavailable', + reason: 'error', + error: expect.stringContaining(detail), + }), + ]); + const complete = output.find((e) => e.type === 'complete'); + // Installation succeeded; inspection did not. Neither outcome hides the other. + expect(complete.success).toBe(true); + expect(complete.files).toEqual([]); + expect(complete.changeDetection).toEqual({ state: 'error', files: [], error: expect.stringContaining(detail) }); + expect(complete.changeDetection.error).toContain('changed files are unknown'); + expect(git(project, 'status', '--porcelain')).toContain('?? generated.ts'); + }, + 15_000, + ); + + it('reports fake agent failure without post-install success or publication', () => { + const result = run([...install, '--create-pr', '--json'], { TEST_AGENT: 'fail' }); + expect(result.status).toBe(1); + expect( + events(result.stdout) + .filter((e) => e.type === 'complete') + .every((e) => e.success === false), + ).toBe(true); + expect(result.stdout).not.toContain('postinstall:'); + expect(JSON.parse(result.stderr).error.message).toContain('Fake installation failed'); + }); + + it('cancels before the agent without asking any post-install questions', () => { + git(project, 'checkout', '-qb', 'existing-feature'); + const result = run(['install', '--no-branch', '--create-pr'], { TEST_HUMAN: '1', TEST_CANCEL: '1' }); + expect(result.stdout).toContain('cancelled'); + expect(result.evidence.some((e) => e.kind === 'agent-options')).toBe(false); + expect(result.evidence.filter((e) => e.kind === 'prompt').map((e) => e.value)).toEqual(['Continue anyway?']); + }); +}); diff --git a/src/bin.ts b/src/bin.ts index 0f032e2c..05bab488 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -176,6 +176,25 @@ const tuiOption = { }, } as const; +// Compatibility-only booleans: no defaults, so omission stays distinguishable. +const deprecatedInstallerGitOptions = { + commit: { + describe: 'Deprecated no-op (--commit/--no-commit); installer changes are left uncommitted', + type: 'boolean' as const, + }, + 'create-pr': { + describe: 'Deprecated no-op; the installer never pushes or creates pull requests', + type: 'boolean' as const, + }, +}; + +function warnDeprecatedInstallerGitFlags(argv: { commit?: boolean; createPr?: boolean }): void { + if (isJsonMode() || (argv.commit === undefined && argv.createPr === undefined)) return; + ui.log.warn( + 'Deprecated installer Git flags are ignored. The installer no longer commits, pushes, or creates pull requests. Review and commit changes yourself.', + ); +} + const installerOptions = { direct: { alias: 'D', @@ -246,16 +265,7 @@ const installerOptions = { describe: 'Create a new branch for changes (use --no-branch to skip)', type: 'boolean' as const, }, - commit: { - default: true, - describe: 'Auto-commit after installation (use --no-commit to skip)', - type: 'boolean' as const, - }, - 'create-pr': { - default: false, - describe: 'Auto-create pull request after installation', - type: 'boolean' as const, - }, + ...deprecatedInstallerGitOptions, 'git-check': { default: true, describe: 'Check for dirty working tree (use --no-git-check to skip)', @@ -3199,6 +3209,7 @@ async function runCli(): Promise { force: argv.force, router: argv.router, }); + warnDeprecatedInstallerGitFlags(argv); await resolveInstallCredentials(argv.apiKey, argv.installDir, argv.skipAuth, ensureAuthenticated); const { handleInstall } = await import('./commands/install.js'); await handleInstall(argv); @@ -3410,8 +3421,10 @@ async function runCli(): Promise { 'WorkOS AuthKit CLI', // `--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, ...tuiOption }), + (yargs) => + yargs.options({ ...insecureStorageOption, ...forceOption, ...tuiOption, ...deprecatedInstallerGitOptions }), async (argv) => { + warnDeprecatedInstallerGitFlags(argv); // Non-human modes: emit machine-readable command tree (JSON) or the // fully-configured parser help (human non-TTY edge) instead of prompting. if (!isPromptAllowed()) { diff --git a/src/lib/adapters/cli-adapter.ts b/src/lib/adapters/cli-adapter.ts index 2c10c615..a77d757a 100644 --- a/src/lib/adapters/cli-adapter.ts +++ b/src/lib/adapters/cli-adapter.ts @@ -164,17 +164,11 @@ export class CLIAdapter implements InstallerAdapter { // Post-install events this.subscribe('postinstall:changes', this.handlePostInstallChanges); - this.subscribe('postinstall:commit:prompt', this.handleCommitPrompt); - this.subscribe('postinstall:commit:generating', this.handleCommitGenerating); - this.subscribe('postinstall:commit:success', this.handleCommitSuccess); - this.subscribe('postinstall:commit:failed', this.handleCommitFailed); - this.subscribe('postinstall:pr:prompt', this.handlePrPrompt); - this.subscribe('postinstall:pr:generating', this.handlePrGenerating); - this.subscribe('postinstall:pr:pushing', this.handlePrPushing); - this.subscribe('postinstall:pr:success', this.handlePrSuccess); - this.subscribe('postinstall:pr:failed', this.handlePrFailed); - this.subscribe('postinstall:push:failed', this.handlePushFailed); - this.subscribe('postinstall:manual', this.handleManualInstructions); + this.subscribe('postinstall:nochanges', () => ui.log.info('No Git changes detected in the install directory.')); + this.subscribe('postinstall:unavailable', ({ reason, error }) => { + if (reason === 'not-git') ui.log.info('This project is not a Git working tree; review the files manually.'); + else ui.log.warn(`Could not inspect Git changes; review the files manually. ${error ?? ''}`); + }); } async stop(): Promise { @@ -704,81 +698,6 @@ export class CLIAdapter implements InstallerAdapter { // ===== Post-install Event Handlers ===== private handlePostInstallChanges = ({ files }: InstallerEvents['postinstall:changes']): void => { - this.debugLog(`Post-install: ${files.length} changed files detected`); - }; - - private handleCommitPrompt = async (): Promise => { - const confirmed = await this.withPromptActive(() => - ui.confirm({ - message: 'Commit the changes?', - initialValue: true, - }), - ); - - this.sendEvent({ - type: ui.isCancel(confirmed) || !confirmed ? 'COMMIT_DECLINED' : 'COMMIT_APPROVED', - }); - }; - - private handleCommitGenerating = (): void => { - this.spinner = ui.spinner(); - this.spinner.start('Generating commit message...'); - }; - - private handleCommitSuccess = ({ message }: InstallerEvents['postinstall:commit:success']): void => { - this.stopSpinner('Committed'); - ui.log.success(`Committed: ${chalk.dim(message)}`); - }; - - private handleCommitFailed = ({ error }: InstallerEvents['postinstall:commit:failed']): void => { - this.stopSpinner('Commit failed'); - ui.log.error(`Commit failed: ${error}`); - }; - - private handlePrPrompt = async (): Promise => { - const confirmed = await this.withPromptActive(() => - ui.confirm({ - message: 'Create a pull request?', - initialValue: true, - }), - ); - - this.sendEvent({ - type: ui.isCancel(confirmed) || !confirmed ? 'PR_DECLINED' : 'PR_APPROVED', - }); - }; - - private handlePrGenerating = (): void => { - this.spinner = ui.spinner(); - this.spinner.start('Generating PR description...'); - }; - - private handlePrPushing = (): void => { - if (this.spinner) { - this.spinner.message('Pushing to remote...'); - } else { - this.spinner = ui.spinner(); - this.spinner.start('Pushing to remote...'); - } - }; - - private handlePrSuccess = ({ url }: InstallerEvents['postinstall:pr:success']): void => { - this.stopSpinner('PR created'); - ui.log.success(`Pull request created: ${chalk.cyan(url)}`); - }; - - private handlePrFailed = ({ error }: InstallerEvents['postinstall:pr:failed']): void => { - this.stopSpinner('PR creation failed'); - ui.log.error(`PR creation failed: ${error}`); - }; - - private handlePushFailed = ({ error }: InstallerEvents['postinstall:push:failed']): void => { - this.stopSpinner('Push failed'); - ui.log.error(`Push failed: ${error}`); - }; - - private handleManualInstructions = ({ instructions }: InstallerEvents['postinstall:manual']): void => { - ui.log.info('GitHub CLI not found. Manual steps:'); - console.log(chalk.dim(instructions)); + this.debugLog(`Post-install: ${files.length} changed files detected (left uncommitted)`); }; } diff --git a/src/lib/adapters/headless-adapter.spec.ts b/src/lib/adapters/headless-adapter.spec.ts index e9ff6625..eabe6197 100644 --- a/src/lib/adapters/headless-adapter.spec.ts +++ b/src/lib/adapters/headless-adapter.spec.ts @@ -254,56 +254,21 @@ describe('HeadlessAdapter', () => { }); }); - describe('commit auto-resolution', () => { - it('auto-commits by default', async () => { + describe('read-only post-install reporting', () => { + it('reports changed, unchanged, non-Git and inspection failures without sending approvals', async () => { const adapter = createAdapter(); await adapter.start(); - - emitter.emit('postinstall:commit:prompt', {}); - - expect(mockWriteNDJSON).toHaveBeenCalledWith({ type: 'commit:auto' }); - expect(sendEvent).toHaveBeenCalledWith({ type: 'COMMIT_APPROVED' }); - await adapter.stop(); - }); - - it('skips commit with --no-commit flag', async () => { - const adapter = createAdapter({ noCommit: true }); - await adapter.start(); - - emitter.emit('postinstall:commit:prompt', {}); - - expect(mockWriteNDJSON).toHaveBeenCalledWith({ - type: 'commit:skipped', - reason: '--no-commit flag', - }); - expect(sendEvent).toHaveBeenCalledWith({ type: 'COMMIT_DECLINED' }); - await adapter.stop(); - }); - }); - - describe('PR auto-resolution', () => { - it('skips PR by default', async () => { - const adapter = createAdapter(); - await adapter.start(); - - emitter.emit('postinstall:pr:prompt', {}); - - expect(mockWriteNDJSON).toHaveBeenCalledWith({ - type: 'pr:skipped', - reason: '--create-pr not set', - }); - expect(sendEvent).toHaveBeenCalledWith({ type: 'PR_DECLINED' }); - await adapter.stop(); - }); - - it('creates PR with --create-pr flag', async () => { - const adapter = createAdapter({ createPr: true }); - await adapter.start(); - - emitter.emit('postinstall:pr:prompt', {}); - - expect(mockWriteNDJSON).toHaveBeenCalledWith({ type: 'pr:creating' }); - expect(sendEvent).toHaveBeenCalledWith({ type: 'PR_APPROVED' }); + emitter.emit('postinstall:changes', { files: ['existing.ts', 'generated.ts'] }); + emitter.emit('postinstall:nochanges', {}); + emitter.emit('postinstall:unavailable', { reason: 'not-git' }); + emitter.emit('postinstall:unavailable', { reason: 'error', error: 'inspection failed' }); + expect(mockWriteNDJSON.mock.calls.map(([event]) => event)).toEqual([ + { type: 'postinstall:changes', files: ['existing.ts', 'generated.ts'], count: 2 }, + { type: 'postinstall:nochanges' }, + { type: 'postinstall:unavailable', reason: 'not-git' }, + { type: 'postinstall:unavailable', reason: 'error', error: 'inspection failed' }, + ]); + expect(sendEvent).not.toHaveBeenCalled(); await adapter.stop(); }); }); diff --git a/src/lib/adapters/headless-adapter.ts b/src/lib/adapters/headless-adapter.ts index 6766beb4..16e31cc6 100644 --- a/src/lib/adapters/headless-adapter.ts +++ b/src/lib/adapters/headless-adapter.ts @@ -25,8 +25,6 @@ export interface HeadlessOptions { apiKey?: string; clientId?: string; noBranch?: boolean; - noCommit?: boolean; - createPr?: boolean; noGitCheck?: boolean; /** CI mode (WORKOS_MODE=ci, or --ci when headless): pipelines never stop for a dirty tree. */ ci?: boolean; @@ -112,16 +110,10 @@ export class HeadlessAdapter implements InstallerAdapter { this.subscribe('branch:prompt', this.handleBranchPrompt); this.subscribe('branch:created', this.handleBranchCreated); - // Post-install — auto-resolve + // Post-install this.subscribe('postinstall:changes', this.handlePostInstallChanges); - this.subscribe('postinstall:commit:prompt', this.handleCommitPrompt); - this.subscribe('postinstall:commit:success', this.handleCommitSuccess); - this.subscribe('postinstall:commit:failed', this.handleCommitFailed); - this.subscribe('postinstall:pr:prompt', this.handlePrPrompt); - this.subscribe('postinstall:pr:success', this.handlePrSuccess); - this.subscribe('postinstall:pr:failed', this.handlePrFailed); - this.subscribe('postinstall:push:failed', this.handlePushFailed); - this.subscribe('postinstall:manual', this.handleManualInstructions); + this.subscribe('postinstall:nochanges', () => writeNDJSON({ type: 'postinstall:nochanges' })); + this.subscribe('postinstall:unavailable', (result) => writeNDJSON({ type: 'postinstall:unavailable', ...result })); // Terminal events this.subscribe('complete', this.handleComplete); @@ -353,56 +345,12 @@ export class HeadlessAdapter implements InstallerAdapter { writeNDJSON({ type: 'branch:created', name: branch }); }; - // ===== Post-install (auto-resolve) ===== + // ===== Post-install ===== private handlePostInstallChanges = ({ files }: InstallerEvents['postinstall:changes']): void => { writeNDJSON({ type: 'postinstall:changes', files, count: files.length }); }; - private handleCommitPrompt = (): void => { - if (this.options.noCommit) { - writeNDJSON({ type: 'commit:skipped', reason: '--no-commit flag' }); - this.sendEvent({ type: 'COMMIT_DECLINED' }); - } else { - writeNDJSON({ type: 'commit:auto' }); - this.sendEvent({ type: 'COMMIT_APPROVED' }); - } - }; - - private handleCommitSuccess = ({ message }: InstallerEvents['postinstall:commit:success']): void => { - writeNDJSON({ type: 'commit:created', message }); - }; - - private handleCommitFailed = ({ error }: InstallerEvents['postinstall:commit:failed']): void => { - writeNDJSON({ type: 'commit:failed', error }); - }; - - private handlePrPrompt = (): void => { - if (this.options.createPr) { - writeNDJSON({ type: 'pr:creating' }); - this.sendEvent({ type: 'PR_APPROVED' }); - } else { - writeNDJSON({ type: 'pr:skipped', reason: '--create-pr not set' }); - this.sendEvent({ type: 'PR_DECLINED' }); - } - }; - - private handlePrSuccess = ({ url }: InstallerEvents['postinstall:pr:success']): void => { - writeNDJSON({ type: 'pr:created', url }); - }; - - private handlePrFailed = ({ error }: InstallerEvents['postinstall:pr:failed']): void => { - writeNDJSON({ type: 'pr:failed', error }); - }; - - private handlePushFailed = ({ error }: InstallerEvents['postinstall:push:failed']): void => { - writeNDJSON({ type: 'push:failed', error }); - }; - - private handleManualInstructions = ({ instructions }: InstallerEvents['postinstall:manual']): void => { - writeNDJSON({ type: 'postinstall:manual', instructions }); - }; - // ===== Terminal Events ===== private handleComplete = ({ success, summary, completion }: InstallerEvents['complete']): void => { @@ -419,6 +367,7 @@ export class HeadlessAdapter implements InstallerAdapter { devCommand: completion.devCommand, url: completion.url, files: completion.files, + ...(completion.changeDetection ? { changeDetection: completion.changeDetection } : {}), nextSteps: completion.nextSteps, ...(completion.applicationSetup ? { applicationSetup: completion.applicationSetup } : {}), } diff --git a/src/lib/adapters/tui-adapter.spec.ts b/src/lib/adapters/tui-adapter.spec.ts index f1a57341..3c9aa35e 100644 --- a/src/lib/adapters/tui-adapter.spec.ts +++ b/src/lib/adapters/tui-adapter.spec.ts @@ -134,10 +134,10 @@ describe('TuiAdapter', () => { it('cancels an open question on esc, the same as cancelling it in the plain CLI', async () => { await adapter.start(); - emitter.emit('postinstall:commit:prompt', {}); - await waitFor(() => expect(frame()).toContain('? Commit the changes?')); + emitter.emit('branch:prompt', { branch: 'main' }); + await waitFor(() => expect(frame()).toContain('Create a feature branch?')); stdin.press(KEY.escape); - await waitFor(() => expect(sendEvent).toHaveBeenCalledWith({ type: 'COMMIT_DECLINED' })); + await waitFor(() => expect(sendEvent).toHaveBeenCalledWith({ type: 'BRANCH_CANCEL' })); }); it('lets ctrl-c finish machine cancellation before the adapter stops', async () => { @@ -195,8 +195,77 @@ describe('TuiAdapter', () => { it('surfaces errors the installer prints in the walkthrough', async () => { await adapter.start(); - emitter.emit('postinstall:commit:failed', { error: 'nothing to commit' }); - await waitFor(() => expect(frame()).toContain('✗ Commit failed: nothing to commit')); + emitter.emit('postinstall:unavailable', { reason: 'error', error: 'inspection failed' }); + await waitFor(() => expect(frame()).toContain('inspection failed')); + }); + + it('drives the real successful machine through branch consent to finish, without commit/PR prompts', async () => { + const prompts = vi.spyOn(ui, 'confirm'); + const branch = vi.fn(async () => ({ branch: 'feat/add-workos-authkit' })); + const emitted = vi.spyOn(emitter, 'emit'); + const machine = installerMachine.provide({ + actors: { + checkWorkspace: fromPromise(async () => ({ scaffoldable: false, packageManager: 'npm', autoScaffold: false })), + detectIntegration: fromPromise(async () => ({ integration: 'nextjs' })), + checkGitStatus: fromPromise(async () => ({ isClean: true, files: [] })), + checkBranch: fromPromise(async () => ({ branch: 'main', isProtected: true })), + createBranch: fromPromise(branch), + configureEnvironment: fromPromise(async () => {}), + runAgent: fromPromise(async () => { + emitter.emit('validation:start', { framework: 'nextjs' }); + emitter.emit('validation:complete', { passed: true, issueCount: 0, durationMs: 1 }); + return { success: true, summary: 'Fake agent completed validation' }; + }), + detectChanges: fromPromise(async () => ({ state: 'changed', files: ['existing.ts', 'generated.ts'] })), + buildCompletion: fromPromise(async ({ input: { context } }) => ({ + integration: 'nextjs', + devCommand: 'npm run dev', + url: 'http://localhost:3000', + files: context.changedFiles ?? [], + changeDetection: context.changeDetection, + nextSteps: ['Review and commit independently'], + docsUrl: 'https://workos.com/docs', + dashboardUrl: 'https://dashboard.workos.com', + })), + }, + }); + const actor = createActor(machine, { + input: { + emitter, + options: { + installDir: '/work/my-app', + skipAuth: true, + apiKey: 'offline', + clientId: 'offline', + noCommit: false, + createPr: true, + }, + }, + }); + sendEvent.mockImplementation((event) => actor.send(event)); + try { + await adapter.start(); + actor.start(); + actor.send({ type: 'START' }); + await waitFor(() => expect(frame()).toContain('Create a feature branch?')); + stdin.press(KEY.enter); + await waitFor(() => expect(actor.getSnapshot().value).toBe('complete')); + await waitFor(() => expect(frame()).toContain('Current changed files: 2')); + expect(branch).toHaveBeenCalledOnce(); + expect(prompts).not.toHaveBeenCalled(); + expect(emitted.mock.calls.some(([name]) => /postinstall:(commit|pr|push)/.test(name))).toBe(false); + expect(frame()).not.toContain('Commit the changes?'); + expect(frame()).not.toContain('Create a pull request?'); + await adapter.stop(); + expect(afterExit()).toContain('generated.ts'); + expect(afterExit()).toContain('may include pre-existing changes'); + expect(afterExit()).toContain('Review and commit independently'); + expect(getUiHost()).toBeNull(); + expect(stdin.rawMode).toBe(false); + expect(console.log).toBe(originalLog); + } finally { + actor.stop(); + } }); it('leaves the alternate screen on stop and prints the plain completion summary', async () => { diff --git a/src/lib/agent-runner.ts b/src/lib/agent-runner.ts index 6e29d2f6..044b7a59 100644 --- a/src/lib/agent-runner.ts +++ b/src/lib/agent-runner.ts @@ -208,8 +208,8 @@ export async function runAgentInstaller(config: FrameworkConfig, options: Instal // Block success: an error-severity security finding that survived the // self-correction retries fails the install rather than shipping silently. // Throwing routes through the state machine's error state (success: false, - // non-zero exit) and skips the commit/PR steps, leaving the insecure code - // uncommitted for the user to inspect. + // non-zero exit), leaving the insecure code uncommitted for the user to + // inspect. if (security.blocking.length > 0) { analytics.capture(INSTALLER_INTERACTION_EVENT_NAME, { action: 'security gate blocked install', diff --git a/src/lib/ai-content.ts b/src/lib/ai-content.ts deleted file mode 100644 index ec6b6eb2..00000000 --- a/src/lib/ai-content.ts +++ /dev/null @@ -1,153 +0,0 @@ -import Anthropic from '@anthropic-ai/sdk'; -import { startCredentialProxy } from './credential-proxy.js'; -import { getAuthkitDomain, getCliAuthClientId, getConfig } from './settings.js'; -import { getLlmGatewayUrl } from '../utils/urls.js'; -import { getCredentials } from './credentials.js'; -import { logInfo, logError } from '../utils/debug.js'; - -export interface AiContentOptions { - /** Use direct Anthropic API instead of llm-gateway */ - direct?: boolean; -} - -/** - * Execute an API call through a short-lived credential proxy. - * Handles proxy lifecycle automatically. - */ -async function withProxy(fn: (client: Anthropic) => Promise): Promise { - const gatewayUrl = getLlmGatewayUrl(); - const creds = getCredentials(); - - if (!creds?.refreshToken) { - // No refresh token - use credentials directly (legacy mode) - logInfo('[ai-content] No refresh token, using credentials directly'); - const client = new Anthropic({ - baseURL: gatewayUrl, - apiKey: 'gateway', // SDK requires something, gateway uses Authorization header - defaultHeaders: creds?.accessToken ? { Authorization: `Bearer ${creds.accessToken}` } : undefined, - }); - return fn(client); - } - - // Start short-lived proxy - const proxy = await startCredentialProxy({ - upstreamUrl: gatewayUrl, - refresh: { - authkitDomain: getAuthkitDomain(), - clientId: getCliAuthClientId(), - refreshThresholdMs: getConfig().proxy.refreshThresholdMs, - }, - }); - - logInfo(`[ai-content] Started proxy at ${proxy.url}`); - - try { - const client = new Anthropic({ - baseURL: proxy.url, - apiKey: 'proxy', // SDK requires something, proxy handles real auth - }); - return await fn(client); - } finally { - await proxy.stop(); - logInfo('[ai-content] Stopped proxy'); - } -} - -/** - * Execute an API call directly to Anthropic (--direct mode). - */ -async function withDirect(fn: (client: Anthropic) => Promise): Promise { - // SDK reads ANTHROPIC_API_KEY from env automatically - const client = new Anthropic(); - return fn(client); -} - -/** - * Generate a concise commit message for the AuthKit integration. - * Falls back to a default message if AI generation fails. - */ -export async function generateCommitMessage( - integration: string, - files: string[], - options: AiContentOptions = {}, -): Promise { - const executor = options.direct ? withDirect : withProxy; - - try { - return await executor(async (client) => { - const response = await client.messages.create({ - model: 'claude-sonnet-4-20250514', - max_tokens: 100, - messages: [ - { - role: 'user', - content: `Generate a concise git commit message for adding WorkOS AuthKit to a ${integration} project. Changed files: ${files.slice(0, 10).join(', ')}. Use conventional commit format (feat:). One line only, under 72 chars.`, - }, - ], - }); - - const text = response.content[0]; - if (text.type === 'text') { - return text.text.trim(); - } - throw new Error('Unexpected response format'); - }); - } catch (error) { - logError('[ai-content] Failed to generate commit message:', error); - return `feat: add WorkOS AuthKit integration for ${integration}`; - } -} - -/** - * Generate a PR description for the AuthKit integration. - * Falls back to a default template if AI generation fails. - */ -export async function generatePrDescription( - integration: string, - files: string[], - commitMessage: string, - options: AiContentOptions = {}, -): Promise { - const executor = options.direct ? withDirect : withProxy; - - try { - return await executor(async (client) => { - const response = await client.messages.create({ - model: 'claude-sonnet-4-20250514', - max_tokens: 500, - messages: [ - { - role: 'user', - content: `Generate a GitHub PR description for: "${commitMessage}" - -Framework: ${integration} -Files changed: ${files.join(', ')} - -Include: -- Brief summary (2-3 sentences) -- Key changes bullet list -- Link to WorkOS AuthKit docs: https://workos.com/docs/user-management - -Keep it concise. Markdown format.`, - }, - ], - }); - - const text = response.content[0]; - if (text.type === 'text') { - return text.text.trim(); - } - throw new Error('Unexpected response format'); - }); - } catch (error) { - logError('[ai-content] Failed to generate PR description:', error); - return `## Summary -Added WorkOS AuthKit integration for ${integration}. - -## Changes -${files.map((f) => `- ${f}`).join('\n')} - -## Documentation -https://workos.com/docs/user-management`; - } -} diff --git a/src/lib/completion-data.spec.ts b/src/lib/completion-data.spec.ts index deaeed2d..a02d2230 100644 --- a/src/lib/completion-data.spec.ts +++ b/src/lib/completion-data.spec.ts @@ -99,7 +99,7 @@ describe('buildCompletionData', () => { expect(data.url).toBe('http://localhost:8080'); }); - it('handles empty changedFiles (--no-commit shape) without throwing', async () => { + it('handles an empty changedFiles list without throwing', async () => { writePackageJson({ scripts: { dev: 'next dev' }, dependencies: { next: '15.0.0' } }); const data = await buildCompletionData({ integration: 'nextjs', changedFiles: [], installDir }, baseDeps); diff --git a/src/lib/completion-data.ts b/src/lib/completion-data.ts index 1dc8a26d..9cb29a9f 100644 --- a/src/lib/completion-data.ts +++ b/src/lib/completion-data.ts @@ -27,6 +27,7 @@ export function applicationSetupNextSteps(setup: AuthkitApplicationSetup): strin export interface CompletionContext { integration: string; changedFiles?: string[]; + changeDetection?: import('./post-install.js').ChangeDetection; installDir: string; } @@ -88,11 +89,13 @@ export async function buildCompletionData(ctx: CompletionContext, deps: Completi devCommand, url, files, + ...(ctx.changeDetection ? { changeDetection: ctx.changeDetection } : {}), nextSteps: [ ...claim, ...(deps.applicationSetup ? applicationSetupNextSteps(deps.applicationSetup) : []), ...concrete, ...framework, + 'The installer leaves changes uncommitted. Review the project and commit when ready.', ], ...(deps.applicationSetup ? { applicationSetup: deps.applicationSetup } : {}), docsUrl: deps.docsUrl, diff --git a/src/lib/env-writer.ts b/src/lib/env-writer.ts index a7c67a38..98297745 100644 --- a/src/lib/env-writer.ts +++ b/src/lib/env-writer.ts @@ -131,8 +131,8 @@ function writeSecretFile(path: string, contents: string, existed: boolean): void * 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. + * leave an unignored secret on disk: a later user-run `git add -A` must not + * stage the live API key and claim token held in the env file. * * 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 diff --git a/src/lib/events.ts b/src/lib/events.ts index 75c0c845..7016a4c7 100644 --- a/src/lib/events.ts +++ b/src/lib/events.ts @@ -17,6 +17,8 @@ export interface CompletionData { url: string; /** Changed files (git-relative), full list — display cap lives in the renderer */ files: string[]; + /** Inspection outcome; files can include changes made before installation. */ + changeDetection?: import('./post-install.js').ChangeDetection; /** Composed concrete + framework next-step lines */ nextSteps: string[]; /** Per-framework docs URL */ @@ -137,19 +139,7 @@ export interface InstallerEvents { // Post-install events 'postinstall:changes': { files: string[] }; 'postinstall:nochanges': Record; - 'postinstall:commit:prompt': Record; - 'postinstall:commit:generating': Record; - 'postinstall:commit:committing': { message: string }; - 'postinstall:commit:success': { message: string }; - 'postinstall:commit:failed': { error: string }; - 'postinstall:pr:prompt': Record; - 'postinstall:pr:generating': Record; - 'postinstall:pr:pushing': Record; - 'postinstall:pr:creating': Record; - 'postinstall:pr:success': { url: string }; - 'postinstall:pr:failed': { error: string }; - 'postinstall:push:failed': { error: string }; - 'postinstall:manual': { instructions: string }; + 'postinstall:unavailable': { reason: 'not-git' | 'error'; error?: string }; } export type InstallerEventName = keyof InstallerEvents; @@ -232,19 +222,7 @@ const INSTALLER_EVENT_REGISTRY = { 'branch:skipped': true, 'postinstall:changes': true, 'postinstall:nochanges': true, - 'postinstall:commit:prompt': true, - 'postinstall:commit:generating': true, - 'postinstall:commit:committing': true, - 'postinstall:commit:success': true, - 'postinstall:commit:failed': true, - 'postinstall:pr:prompt': true, - 'postinstall:pr:generating': true, - 'postinstall:pr:pushing': true, - 'postinstall:pr:creating': true, - 'postinstall:pr:success': true, - 'postinstall:pr:failed': true, - 'postinstall:push:failed': true, - 'postinstall:manual': true, + 'postinstall:unavailable': true, } as const satisfies Record; export const INSTALLER_EVENT_NAMES = Object.keys(INSTALLER_EVENT_REGISTRY) as InstallerEventName[]; diff --git a/src/lib/installer-core.spec.ts b/src/lib/installer-core.spec.ts index 2cdc2465..fb67e290 100644 --- a/src/lib/installer-core.spec.ts +++ b/src/lib/installer-core.spec.ts @@ -13,6 +13,7 @@ import type { } from './installer-core.types.js'; import type { EnvFileInfo } from './credential-discovery.js'; import type { StagingCredentials } from './staging-api.js'; +import type { ChangeDetection } from './post-install.js'; // Shared mock actors for reuse across tests const baseMockActors = { @@ -39,6 +40,7 @@ const baseMockActors = { branch: input.name, })), configureEnvironment: fromPromise(async () => {}), + detectChanges: fromPromise(async () => ({ state: 'unchanged', files: [] })), runAgent: fromPromise(async () => ({ success: true, summary: 'Done!', @@ -305,6 +307,25 @@ describe('InstallerCore State Machine', () => { }); describe('full flow', () => { + it('reports an unexpected inspection rejection without claiming no changes or failing the installation', async () => { + const { actor, emitter } = createTestActor( + { skipAuth: true, apiKey: 'offline', clientId: 'offline' }, + { + detectChanges: fromPromise(async () => { + throw new Error('inspection rejected'); + }), + }, + ); + const inspection: unknown[] = []; + emitter.on('postinstall:nochanges', () => inspection.push('unchanged')); + emitter.on('postinstall:unavailable', (result) => inspection.push(result)); + actor.start(); + actor.send({ type: 'START' }); + await waitFor(actor, (s) => s.status === 'done', { timeout: 1000 }); + expect(actor.getSnapshot().value).toBe('complete'); + expect(inspection).toEqual([{ reason: 'error', error: 'Error: inspection rejected' }]); + actor.stop(); + }); it('retains pending application setup for the completion actor', async () => { const applicationSetup = { clientId: 'client_123', diff --git a/src/lib/installer-core.ts b/src/lib/installer-core.ts index 2d85a9d5..e94efdc6 100644 --- a/src/lib/installer-core.ts +++ b/src/lib/installer-core.ts @@ -14,11 +14,10 @@ import type { } from './installer-core.types.js'; import type { InstallerOptions } from '../utils/types.js'; import type { CompletionData } from './events.js'; +import type { ChangeDetection } from './post-install.js'; import type { DeviceAuthResult, DeviceAuthResponse } from './device-auth.js'; import type { StagingCredentials } from './staging-api.js'; import { InstallDeclinedError } from './installer-errors.js'; -import { getManualPrInstructions } from './post-install.js'; -import { hasGhCli } from '../utils/git-utils.js'; import { formatWorkOSCommand } from '../utils/command-invocation.js'; export const installerMachine = setup({ @@ -256,79 +255,28 @@ export const installerMachine = setup({ context.emitter.emit('complete', { success: false, ...(declineCode ? { summary: message } : {}) }); }, // Post-install actions - assignChangedFiles: assign({ - changedFiles: ({ event }) => { - const doneEvent = event as unknown as { output: { hasChanges: boolean; files: string[] } }; - return doneEvent.output?.files ?? []; - }, - }), - emitChangesDetected: ({ context }) => { - context.emitter.emit('postinstall:changes', { files: context.changedFiles ?? [] }); - }, - emitNoChanges: ({ context }) => { - context.emitter.emit('postinstall:nochanges', {}); - }, - emitCommitPrompt: ({ context }) => { - context.emitter.emit('postinstall:commit:prompt', {}); - }, - emitGeneratingCommitMessage: ({ context }) => { - context.emitter.emit('postinstall:commit:generating', {}); - }, - assignCommitMessage: assign({ - commitMessage: ({ event }) => { - const doneEvent = event as unknown as { output: string }; - return doneEvent.output; - }, - }), - emitCommitting: ({ context }) => { - context.emitter.emit('postinstall:commit:committing', { message: context.commitMessage ?? '' }); - }, - emitCommitSuccess: ({ context }) => { - context.emitter.emit('postinstall:commit:success', { message: context.commitMessage ?? '' }); - }, - emitCommitFailed: ({ context }) => { - const message = context.error?.message ?? 'Commit failed'; - context.emitter.emit('postinstall:commit:failed', { error: message }); - }, - emitPrPrompt: ({ context }) => { - context.emitter.emit('postinstall:pr:prompt', {}); - }, - emitGeneratingPrDescription: ({ context }) => { - context.emitter.emit('postinstall:pr:generating', {}); - }, - assignPrDescription: assign({ - prDescription: ({ event }) => { - const doneEvent = event as unknown as { output: string }; - return doneEvent.output; + assignChangeDetection: assign(({ event }) => { + const result = (event as unknown as { output: ChangeDetection }).output; + return { changedFiles: result.files, changeDetection: result }; + }), + assignChangeDetectionError: assign(({ event }) => ({ + changedFiles: [], + changeDetection: { + state: 'error' as const, + files: [] as [], + error: String((event as unknown as { error: unknown }).error), }, - }), - emitPushing: ({ context }) => { - context.emitter.emit('postinstall:pr:pushing', {}); - }, - emitPushFailed: ({ context }) => { - const message = context.error?.message ?? 'Push failed'; - context.emitter.emit('postinstall:push:failed', { error: message }); - }, - emitCreatingPr: ({ context }) => { - context.emitter.emit('postinstall:pr:creating', {}); - }, - assignPrUrl: assign({ - prUrl: ({ event }) => { - const doneEvent = event as unknown as { output: string }; - return doneEvent.output; - }, - }), - emitPrCreated: ({ context }) => { - context.emitter.emit('postinstall:pr:success', { url: context.prUrl ?? '' }); - }, - emitPrFailed: ({ context }) => { - const message = context.error?.message ?? 'PR creation failed'; - context.emitter.emit('postinstall:pr:failed', { error: message }); - }, - emitManualInstructions: ({ context }) => { - const branch = context.currentBranch ?? 'HEAD'; - const instructions = getManualPrInstructions(branch); - context.emitter.emit('postinstall:manual', { instructions }); + })), + emitChangeDetection: ({ context }) => { + const result = context.changeDetection; + if (result?.state === 'changed') context.emitter.emit('postinstall:changes', { files: result.files }); + else if (result?.state === 'unchanged') context.emitter.emit('postinstall:nochanges', {}); + else if (result?.state === 'not-git' || result?.state === 'error') { + context.emitter.emit('postinstall:unavailable', { + reason: result.state, + ...(result.state === 'error' ? { error: result.error } : {}), + }); + } }, emitComplete: ({ context }) => { const summary = context.agentSummary ?? 'WorkOS AuthKit installed successfully!'; @@ -344,8 +292,6 @@ export const installerMachine = setup({ gitIsClean: ({ context }) => context.gitIsClean === true, hasCredentials: ({ context }) => context.options.apiKey !== undefined && context.options.clientId !== undefined, hasIntegration: ({ context }) => context.integration !== undefined, - shouldSkipPostInstall: ({ context }) => context.options.noCommit === true, - hasGhCli: () => hasGhCli(), // Read from the actor's done event (output), not context: the // assignWorkspaceResult action has not run yet when guards are evaluated. notScaffoldable: ({ event }) => !(event as unknown as { output: WorkspaceCheckOutput }).output?.scaffoldable, @@ -409,27 +355,9 @@ export const installerMachine = setup({ throw new Error('createBranch not implemented - provide via machine.provide()'); }), // Post-install actors - detectChanges: fromPromise<{ hasChanges: boolean; files: string[] }, void>(async () => { + detectChanges: fromPromise(async () => { throw new Error('detectChanges not implemented - provide via machine.provide()'); }), - generateCommitMessage: fromPromise(async () => { - throw new Error('generateCommitMessage not implemented - provide via machine.provide()'); - }), - commitChanges: fromPromise(async () => { - throw new Error('commitChanges not implemented - provide via machine.provide()'); - }), - generatePrDescription: fromPromise< - string, - { integration: string; files: string[]; commitMessage: string; direct?: boolean } - >(async () => { - throw new Error('generatePrDescription not implemented - provide via machine.provide()'); - }), - pushBranch: fromPromise(async () => { - throw new Error('pushBranch not implemented - provide via machine.provide()'); - }), - createPr: fromPromise(async () => { - throw new Error('createPr not implemented - provide via machine.provide()'); - }), }, }).createMachine({ id: 'installer', @@ -1033,164 +961,23 @@ export const installerMachine = setup({ }, }, + // Post-install: record what changed so the completion summary can list the + // files. Changes are deliberately left uncommitted for the user to review — + // the installer never commits or opens PRs on its own. postInstall: { - initial: 'checking', + initial: 'detectingChanges', entry: [{ type: 'emitStateEnter', params: { state: 'postInstall' } }], states: { - checking: { - always: [ - { - target: '#installer.buildingCompletion', - guard: 'shouldSkipPostInstall', - }, - { target: 'detectingChanges' }, - ], - }, - detectingChanges: { invoke: { id: 'detectChanges', src: 'detectChanges', - onDone: [ - { - target: 'promptingCommit', - guard: ({ event }) => (event.output as { hasChanges: boolean; files: string[] }).hasChanges, - actions: ['assignChangedFiles', 'emitChangesDetected'], - }, - { - target: 'done', - actions: ['emitNoChanges'], - }, - ], - onError: { target: 'done' }, - }, - }, - - promptingCommit: { - entry: ['emitCommitPrompt'], - on: { - COMMIT_APPROVED: { target: 'generatingCommitMessage' }, - COMMIT_DECLINED: { target: 'done' }, - CANCEL: { target: '#installer.cancelled' }, - }, - }, - - generatingCommitMessage: { - entry: ['emitGeneratingCommitMessage'], - invoke: { - id: 'generateCommitMessage', - src: 'generateCommitMessage', - input: ({ context }) => ({ - integration: context.integration ?? 'project', - files: context.changedFiles ?? [], - direct: context.options.direct, - }), - onDone: { - target: 'committing', - actions: ['assignCommitMessage'], - }, - }, - }, - - committing: { - entry: ['emitCommitting'], - invoke: { - id: 'commitChanges', - src: 'commitChanges', - input: ({ context }) => ({ - message: context.commitMessage ?? '', - cwd: context.options.installDir, - }), - onDone: { - target: 'checkingGhCli', - actions: ['emitCommitSuccess'], - }, - onError: { - target: 'done', - actions: ['assignError', 'emitCommitFailed'], - }, - }, - }, - - checkingGhCli: { - always: [ - { - target: 'promptingPr', - guard: 'hasGhCli', - }, - { - target: 'showingManualInstructions', - }, - ], - }, - - promptingPr: { - entry: ['emitPrPrompt'], - on: { - PR_APPROVED: { target: 'generatingPrDescription' }, - PR_DECLINED: { target: 'done' }, - CANCEL: { target: '#installer.cancelled' }, - }, - }, - - generatingPrDescription: { - entry: ['emitGeneratingPrDescription'], - invoke: { - id: 'generatePrDescription', - src: 'generatePrDescription', - input: ({ context }) => ({ - integration: context.integration ?? 'project', - files: context.changedFiles ?? [], - commitMessage: context.commitMessage ?? '', - direct: context.options.direct, - }), - onDone: { - target: 'pushing', - actions: ['assignPrDescription'], - }, - }, - }, - - pushing: { - entry: ['emitPushing'], - invoke: { - id: 'pushBranch', - src: 'pushBranch', - input: ({ context }) => ({ cwd: context.options.installDir }), - onDone: { target: 'creatingPr' }, - onError: { - target: 'showingManualInstructions', - actions: ['assignError', 'emitPushFailed'], - }, - }, - }, - - creatingPr: { - entry: ['emitCreatingPr'], - invoke: { - id: 'createPr', - src: 'createPr', - input: ({ context }) => ({ - title: context.commitMessage ?? '', - body: context.prDescription ?? '', - cwd: context.options.installDir, - }), - onDone: { - target: 'done', - actions: ['assignPrUrl', 'emitPrCreated'], - }, - onError: { - target: 'done', - actions: ['assignError', 'emitPrFailed'], - }, + input: ({ context }) => ({ installDir: context.options.installDir }), + onDone: { target: 'done', actions: ['assignChangeDetection', 'emitChangeDetection'] }, + onError: { target: 'done', actions: ['assignChangeDetectionError', 'emitChangeDetection'] }, }, }, - showingManualInstructions: { - entry: ['emitManualInstructions'], - always: { target: 'done' }, - }, - done: { type: 'final', }, diff --git a/src/lib/installer-core.types.ts b/src/lib/installer-core.types.ts index f8270cfe..68d3c1fb 100644 --- a/src/lib/installer-core.types.ts +++ b/src/lib/installer-core.types.ts @@ -50,14 +50,9 @@ export interface InstallerMachineContext { currentBranch?: string; /** Whether current branch is protected */ isProtectedBranch?: boolean; - /** Files changed during agent execution (for post-install) */ + /** Current changed files, including any pre-existing changes (git-relative). */ changedFiles?: string[]; - /** AI-generated commit message */ - commitMessage?: string; - /** AI-generated PR description */ - prDescription?: string; - /** URL of created PR */ - prUrl?: string; + changeDetection?: import('./post-install.js').ChangeDetection; /** Summary message from agent execution */ agentSummary?: string; applicationSetup?: import('./authkit-application-setup.js').AuthkitApplicationSetup; @@ -102,12 +97,7 @@ export type InstallerMachineEvent = // Branch check events | { type: 'BRANCH_CREATE' } | { type: 'BRANCH_CONTINUE' } - | { type: 'BRANCH_CANCEL' } - // Post-install events - | { type: 'COMMIT_APPROVED' } - | { type: 'COMMIT_DECLINED' } - | { type: 'PR_APPROVED' } - | { type: 'PR_DECLINED' }; + | { type: 'BRANCH_CANCEL' }; /** * Output from the detection actor. diff --git a/src/lib/post-install.spec.ts b/src/lib/post-install.spec.ts new file mode 100644 index 00000000..a12206aa --- /dev/null +++ b/src/lib/post-install.spec.ts @@ -0,0 +1,226 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as childProcess from 'node:child_process'; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { createActor, fromPromise, waitFor } from 'xstate'; +import { detectChanges, type ChangeDetection } from './post-install.js'; +import { installerMachine } from './installer-core.js'; +import { createInstallerEventEmitter } from './events.js'; +import { buildCompletionData } from './completion-data.js'; + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, execFileSync: vi.fn(actual.execFileSync) }; +}); + +let dir: string; +const git = (...args: string[]) => childProcess.execFileSync('git', args, { cwd: dir, encoding: 'utf8' }); + +beforeEach(() => { + dir = mkdtempSync(join(process.cwd(), '.auth6733-git-')); + vi.stubEnv('HOME', dir); + vi.stubEnv('GIT_CEILING_DIRECTORIES', process.cwd()); + vi.stubEnv('GIT_CONFIG_NOSYSTEM', '1'); + const gitConfig = join(dir, 'empty-gitconfig'); + writeFileSync(gitConfig, ''); + vi.stubEnv('GIT_CONFIG_GLOBAL', gitConfig); + vi.stubEnv('GIT_AUTHOR_NAME', 'Offline Test'); + vi.stubEnv('GIT_AUTHOR_EMAIL', 'offline@example.test'); + vi.stubEnv('GIT_COMMITTER_NAME', 'Offline Test'); + vi.stubEnv('GIT_COMMITTER_EMAIL', 'offline@example.test'); +}); +afterEach(() => { + vi.mocked(childProcess.execFileSync).mockReset(); + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + rmSync(dir, { recursive: true, force: true }); +}); + +function init() { + git('init', '-q', '-b', 'main'); + writeFileSync(join(dir, 'tracked.ts'), 'original'); + git('add', '.'); + git('-c', 'commit.gpgsign=false', 'commit', '-qm', 'fixture'); +} + +describe('read-only change inspection', () => { + it('distinguishes a confirmed unchanged tree from a non-Git directory and a failed inspection', () => { + expect(detectChanges(dir)).toEqual({ state: 'not-git', files: [] }); + expect(detectChanges(join(dir, 'missing'))).toMatchObject({ state: 'error', files: [], error: expect.any(String) }); + init(); + expect(detectChanges(dir)).toEqual({ state: 'unchanged', files: [] }); + }); + + it('preserves tracked/untracked filenames and staged rename destinations, without touching the index', () => { + init(); + const renamed = 'renamed → file.ts'; + git('mv', 'tracked.ts', renamed); + const names = ['space name.ts', 'unicode-雪.ts']; + for (const name of names) writeFileSync(join(dir, name), 'new'); + mkdirSync(join(dir, 'nested')); + writeFileSync(join(dir, 'nested', 'untracked.ts'), 'new'); + const index = readFileSync(join(dir, '.git/index')); + const result = detectChanges(dir); + expect(result.state).toBe('changed'); + expect(result.files.sort()).toEqual([...names, renamed, 'nested/untracked.ts'].sort()); + expect(readFileSync(join(dir, '.git/index'))).toEqual(index); + expect(detectChanges(join(dir, 'nested'))).toEqual({ state: 'changed', files: ['nested/untracked.ts'] }); + }); + + it('parses exact NUL-delimited names including characters unavailable in Windows filenames', () => { + const files = [' space.ts', 'line\nbreak.ts', 'quote".ts', 'unicode-雪.ts', 'renamed → "file".ts']; + vi.spyOn(childProcess, 'execFileSync').mockImplementation((_cmd, args) => + args?.[0] === 'rev-parse' + ? 'true\n' + : files + .slice(0, -1) + .map((file) => `?? ${file}\0`) + .join('') + `R ${files.at(-1)}\0old\nname.ts\0`, + ); + expect(detectChanges(dir)).toEqual({ state: 'changed', files }); + }); + + it('does not call a status failure unchanged (and runs only read-only commands)', () => { + const exec = vi.spyOn(childProcess, 'execFileSync').mockImplementation((_cmd, args) => { + if (args?.[0] === 'rev-parse') return 'true\n'; + if (args?.[0] === 'status') throw new Error('permission denied'); + throw new Error('Forbidden execution'); + }); + expect(detectChanges(dir)).toEqual({ state: 'error', files: [], error: 'permission denied' }); + expect(exec.mock.calls.map(([cmd, args]) => [cmd, args])).toEqual([ + ['git', ['rev-parse', '--is-inside-work-tree']], + ['git', ['status', '--porcelain=v1', '-z', '--untracked-files=all', '--', '.']], + ]); + for (const [, , options] of exec.mock.calls) { + expect(options).toMatchObject({ + cwd: dir, + timeout: 5_000, + killSignal: 'SIGKILL', + maxBuffer: 1024 * 1024, + env: { GIT_OPTIONAL_LOCKS: '0' }, + }); + } + }); + + it('reports a large complete untracked list without summarizing or losing filenames', () => { + const files = Array.from({ length: 10_000 }, (_, i) => `generated/subdir/untracked-${i}-雪.ts`); + const output = files.map((file) => `?? ${file}\0`).join(''); + expect(Buffer.byteLength(output)).toBeLessThan(1024 * 1024); + vi.spyOn(childProcess, 'execFileSync').mockImplementation((_cmd, args) => + args?.[0] === 'rev-parse' ? 'true\n' : output, + ); + expect(detectChanges(dir)).toEqual({ state: 'changed', files }); + }); + + it.each([ + { code: 'ETIMEDOUT', detail: 'timed out after 5000 ms' }, + { code: 'ENOBUFS', detail: 'exceeded the 1048576-byte output limit' }, + ])('discards partial stdout on $code in either command', ({ code, detail }) => { + for (const phase of ['rev-parse', 'status']) { + const error = Object.assign(new Error('raw subprocess error'), { + code, + stdout: '?? partially-reported.ts\0', + stderr: 'not a git repository', + }); + vi.spyOn(childProcess, 'execFileSync').mockImplementation((_cmd, args) => { + if (args?.[0] === phase) throw error; + return 'true\n'; + }); + expect(detectChanges(dir)).toEqual({ + state: 'error', + files: [], + error: `Git change inspection ${detail}; changed files are unknown. Review the project manually.`, + }); + } + }); + + it('distinguishes an unavailable Git executable from a non-Git project', () => { + vi.spyOn(childProcess, 'execFileSync').mockImplementation(() => { + throw Object.assign(new Error('spawn git ENOENT'), { stderr: null }); + }); + expect(detectChanges(dir)).toEqual({ state: 'error', files: [], error: 'spawn git ENOENT' }); + }); +}); + +describe('real post-install state machine path', () => { + it.each([ + { state: 'changed', files: ['existing.ts', 'new.ts'] }, + { state: 'unchanged', files: [] }, + { state: 'not-git', files: [] }, + { state: 'error', files: [], error: 'inspection failed' }, + ] satisfies ChangeDetection[])( + 'completes honestly with $state, even with legacy noCommit/createPr values', + async (result) => { + const emitter = createInstallerEventEmitter(); + const emitted = vi.spyOn(emitter, 'emit'); + const detect = vi.fn(() => result); + const machine = installerMachine.provide({ + actors: { + checkWorkspace: fromPromise(async () => ({ + scaffoldable: false, + packageManager: 'npm', + autoScaffold: false, + })), + detectIntegration: fromPromise(async () => ({ integration: 'nextjs' })), + checkGitStatus: fromPromise(async () => ({ isClean: true, files: [] })), + checkBranch: fromPromise(async () => ({ branch: 'feature', isProtected: false })), + configureEnvironment: fromPromise(async () => {}), + runAgent: fromPromise(async () => ({ success: true, summary: 'Fake agent completed validation' })), + detectChanges: fromPromise(async ({ input }) => { + expect(input.installDir).toBe(dir); + return detect(); + }), + buildCompletion: fromPromise(async ({ input: { context } }) => + buildCompletionData( + { + integration: context.integration!, + installDir: context.options.installDir, + changedFiles: context.changedFiles, + changeDetection: context.changeDetection, + }, + { + resolveDevCommand: async () => ({ command: 'npm', args: ['run', 'dev'] }), + detectPort: () => 3000, + docsUrl: 'https://workos.com/docs', + dashboardUrl: 'https://dashboard.workos.com', + }, + ), + ), + }, + }); + const actor = createActor(machine, { + input: { + emitter, + options: { + installDir: dir, + skipAuth: true, + apiKey: 'offline', + clientId: 'offline', + noCommit: true, + createPr: true, + }, + }, + }); + actor.start(); + actor.send({ type: 'START' }); + await waitFor(actor, (s) => s.status === 'done', { timeout: 1000 }); + expect(actor.getSnapshot().value).toBe('complete'); + expect(detect).toHaveBeenCalledOnce(); + expect(actor.getSnapshot().context.completion?.changeDetection).toEqual(result); + const post = emitted.mock.calls.filter(([name]) => name.startsWith('postinstall:')); + expect(post).toEqual( + result.state === 'changed' + ? [['postinstall:changes', { files: result.files }]] + : result.state === 'unchanged' + ? [['postinstall:nochanges', {}]] + : [ + [ + 'postinstall:unavailable', + { reason: result.state, ...(result.state === 'error' ? { error: result.error } : {}) }, + ], + ], + ); + actor.stop(); + }, + ); +}); diff --git a/src/lib/post-install.ts b/src/lib/post-install.ts index d2cc9784..94e9a083 100644 --- a/src/lib/post-install.ts +++ b/src/lib/post-install.ts @@ -1,54 +1,66 @@ import { execFileSync } from 'node:child_process'; -import { writeFileSync, unlinkSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { getDefaultBranch, getUncommittedFiles } from '../utils/git-utils.js'; -export function detectChanges(): { hasChanges: boolean; files: string[] } { - const files = getUncommittedFiles(); - return { hasChanges: files.length > 0, files }; -} +export type ChangeDetection = + | { state: 'changed' | 'unchanged'; files: string[] } + | { state: 'not-git'; files: [] } + | { state: 'error'; files: []; error: string }; -export function stageAndCommit(message: string, cwd: string): void { - execFileSync('git', ['add', '-A'], { cwd, stdio: 'ignore' }); - execFileSync('git', ['commit', '-m', message], { cwd, stdio: 'ignore' }); -} +// Completion is best-effort: at most two commands, each bounded independently. +// Never parse partial stdout after a timeout or buffer overflow as a complete list. +const GIT_TIMEOUT_MS = 5_000; +const GIT_MAX_BUFFER = 1024 * 1024; -export function pushBranch(cwd: string): void { - execFileSync('git', ['push', '-u', 'origin', 'HEAD'], { cwd, stdio: 'pipe' }); +function inspectionError(error: unknown): ChangeDetection { + const code = (error as { code?: string } | null)?.code; + const message = + code === 'ETIMEDOUT' + ? `Git change inspection timed out after ${GIT_TIMEOUT_MS} ms; changed files are unknown. Review the project manually.` + : code === 'ENOBUFS' + ? `Git change inspection exceeded the ${GIT_MAX_BUFFER}-byte output limit; changed files are unknown. Review the project manually.` + : error instanceof Error + ? error.message + : String(error); + return { state: 'error', files: [], error: message }; } -export function createPullRequest(title: string, body: string, cwd: string): string { - const baseBranch = getDefaultBranch(); - const tmpFile = join(tmpdir(), `pr-body-${Date.now()}.md`); - writeFileSync(tmpFile, body, 'utf-8'); - - try { - return execFileSync('gh', ['pr', 'create', '--title', title, '--body-file', tmpFile, '--base', baseBranch], { - cwd, +/** Read the current tree, not an attribution of changes to the installer. Never touch the index. */ +export function detectChanges(installDir: string): ChangeDetection { + const git = (args: string[]) => + execFileSync('git', args, { + cwd: installDir, + encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], - }) - .toString() - .trim(); - } finally { - try { - unlinkSync(tmpFile); - } catch {} + timeout: GIT_TIMEOUT_MS, + killSignal: 'SIGKILL', + maxBuffer: GIT_MAX_BUFFER, + env: { ...process.env, LC_ALL: 'C', GIT_OPTIONAL_LOCKS: '0' }, + }); + try { + if (git(['rev-parse', '--is-inside-work-tree']).trim() !== 'true') { + return { state: 'not-git', files: [] }; + } + } catch (error) { + const { code, stderr } = (error ?? {}) as { code?: string; stderr?: string | null }; + // A resource limit is never evidence of a non-Git project, even if stderr + // happens to contain that phrase before the process is terminated. + if (code !== 'ETIMEDOUT' && code !== 'ENOBUFS' && (stderr ?? '').includes('not a git repository')) { + return { state: 'not-git', files: [] }; + } + return inspectionError(error); + } + try { + // NUL records preserve spaces, quotes, newlines and Unicode. Rename/copy + // records contain a second (source) path; report the destination only. + const records = git(['status', '--porcelain=v1', '-z', '--untracked-files=all', '--', '.']).split('\0'); + const files: string[] = []; + for (let i = 0; i < records.length; i++) { + const record = records[i]; + if (!record) continue; + files.push(record.slice(3)); + if (/[RC]/.test(record.slice(0, 2))) i++; + } + return { state: files.length ? 'changed' : 'unchanged', files }; + } catch (error) { + return inspectionError(error); } -} - -export function getManualPrInstructions(branch: string): string { - const baseBranch = getDefaultBranch(); - return ` -To create a PR manually: - -1. Push your branch: - git push -u origin ${branch} - -2. Create PR via GitHub: - https://github.com///compare/${baseBranch}...${branch} - -Or install the GitHub CLI: - https://cli.github.com/ -`.trim(); } diff --git a/src/lib/run-with-core.ts b/src/lib/run-with-core.ts index 9eee52a6..b8580036 100644 --- a/src/lib/run-with-core.ts +++ b/src/lib/run-with-core.ts @@ -49,11 +49,7 @@ import { createBranch as createGitBranch, branchExists, } from '../utils/git-utils.js'; -import { detectChanges, stageAndCommit, pushBranch as pushGitBranch, createPullRequest } from './post-install.js'; -import { - generateCommitMessage as generateCommitMessageAi, - generatePrDescription as generatePrDescriptionAi, -} from './ai-content.js'; +import { detectChanges } from './post-install.js'; import { assertSupportedNextJsRouter, getNextJsRouter, @@ -370,7 +366,7 @@ export async function runWithCore(options: InstallerOptions): Promise { // Headless (no prompts, structured output) is for MACHINE output only: JSON. // A prompt cannot render into a JSON stream, so any JSON run must be headless. // We deliberately do NOT route a human session with non-TTY stdin here: - // headless auto-approves branch/commit/scaffold, and applying those unattended + // headless auto-approves branch/scaffold, and applying those unattended // to a session the user never opted into would violate the "nothing is written // until you confirm" contract. Those sessions keep the CLIAdapter, which now // fails fast with a clear `prompt_unavailable` error on the first prompt @@ -391,8 +387,6 @@ export async function runWithCore(options: InstallerOptions): Promise { apiKey: augmentedOptions.apiKey, clientId: augmentedOptions.clientId, noBranch: augmentedOptions.noBranch, - noCommit: augmentedOptions.noCommit, - createPr: augmentedOptions.createPr, noGitCheck: augmentedOptions.noGitCheck, ci: augmentedOptions.ci, }, @@ -533,7 +527,14 @@ export async function runWithCore(options: InstallerOptions): Promise { buildCompletion: fromPromise( async ({ input }) => { - const { integration, changedFiles, options: installerOptions, credentials, applicationSetup } = input.context; + const { + integration, + changedFiles, + changeDetection, + options: installerOptions, + credentials, + applicationSetup, + } = input.context; if (!integration) return undefined; try { const registry = await getRegistry(); @@ -556,7 +557,7 @@ export async function runWithCore(options: InstallerOptions): Promise { activeEnv.clientId === credentials.clientId, ); return await buildCompletionData( - { integration, changedFiles, installDir: installerOptions.installDir }, + { integration, changedFiles, changeDetection, installDir: installerOptions.installDir }, { resolveDevCommand, detectPort, @@ -667,34 +668,7 @@ export async function runWithCore(options: InstallerOptions): Promise { }), // Post-install actors - detectChanges: fromPromise<{ hasChanges: boolean; files: string[] }, void>(async () => { - return detectChanges(); - }), - - generateCommitMessage: fromPromise( - async ({ input }) => { - return generateCommitMessageAi(input.integration, input.files, { direct: input.direct }); - }, - ), - - commitChanges: fromPromise(async ({ input }) => { - stageAndCommit(input.message, input.cwd); - }), - - generatePrDescription: fromPromise< - string, - { integration: string; files: string[]; commitMessage: string; direct?: boolean } - >(async ({ input }) => { - return generatePrDescriptionAi(input.integration, input.files, input.commitMessage, { direct: input.direct }); - }), - - pushBranch: fromPromise(async ({ input }) => { - pushGitBranch(input.cwd); - }), - - createPr: fromPromise(async ({ input }) => { - return createPullRequest(input.title, input.body, input.cwd); - }), + detectChanges: fromPromise(async ({ input }) => detectChanges(input.installDir)), }, }); diff --git a/src/lib/skills-assets.spec.ts b/src/lib/skills-assets.spec.ts index b270ca96..fa31425a 100644 --- a/src/lib/skills-assets.spec.ts +++ b/src/lib/skills-assets.spec.ts @@ -1,9 +1,21 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, utimesSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { describe, expect, it, vi } from 'vitest'; +import { afterAll, describe, expect, it, vi } from 'vitest'; import { BUNDLED_SKILLS_VERSION, getReference, getSkillsDir } from './skills-assets.js'; +// All imports of node:os in this worker see the same private temp root, even +// after resetModules(). Other specs and CLI processes keep their own cache. +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + const { mkdtempSync } = await import('node:fs'); + const { join } = await import('node:path'); + const root = mkdtempSync(join(actual.tmpdir(), 'workos-skills-spec-')); + return { ...actual, tmpdir: () => root }; +}); + +afterAll(() => rmSync(tmpdir(), { recursive: true, force: true })); + function extractionSuffix(): string { return process.platform === 'win32' ? '' : `-${process.getuid?.() ?? 0}`; } @@ -25,7 +37,12 @@ function tempFilesUnder(root: string): string[] { describe('embedded skills assets', () => { it('materializes the complete plugin tree to a real directory', async () => { + const { tmpdir: sharedTmpdir } = await vi.importActual('node:os'); + // This spec deletes extraction roots: never share them with other workers + // (e.g. doctor --fix copying these assets) or real CLI invocations. + expect(tmpdir()).not.toBe(sharedTmpdir()); const skillsDir = getSkillsDir(); + expect(skillsDir.startsWith(join(tmpdir(), 'workos-skills-'))).toBe(true); expect(skillsDir).toContain(`workos-skills-${BUNDLED_SKILLS_VERSION}`); expect(existsSync(join(skillsDir, 'workos', 'SKILL.md'))).toBe(true); expect(existsSync(join(skillsDir, 'workos-widgets', 'SKILL.md'))).toBe(true); @@ -68,8 +85,7 @@ describe('embedded skills assets', () => { }); describe('materializeFile concurrent extraction race', () => { - // The extraction root is version-keyed and shared with the other tests and - // real CLI runs on this machine; each test wipes it first so materializeFile's + // Wipe only this spec's private extraction root so materializeFile's // identical-target pre-check can't short-circuit before the mocked rename runs. const extractionRoot = join(tmpdir(), `workos-skills-${BUNDLED_SKILLS_VERSION}${extractionSuffix()}`); diff --git a/src/run.ts b/src/run.ts index e2729626..6afe8c8c 100644 --- a/src/run.ts +++ b/src/run.ts @@ -23,10 +23,13 @@ export type InstallerArgs = { inspect?: boolean; noValidate?: boolean; validate?: boolean; + /** @deprecated Ignored: changes are always left uncommitted. */ noCommit?: boolean; + /** @deprecated Ignored: changes are always left uncommitted. */ commit?: boolean; noBranch?: boolean; branch?: boolean; + /** @deprecated Ignored: the installer never publishes changes. */ createPr?: boolean; noGitCheck?: boolean; gitCheck?: boolean; @@ -71,9 +74,7 @@ function buildOptions(argv: InstallerArgs): InstallerOptions { redirectUri: merged.redirectUri, inspect: merged.inspect ?? false, noValidate: merged.noValidate ?? merged.validate === false, - noCommit: merged.noCommit ?? merged.commit === false, noBranch: merged.noBranch ?? merged.branch === false, - createPr: merged.createPr ?? false, noGitCheck: merged.noGitCheck ?? merged.gitCheck === false, direct: merged.direct ?? false, noTui: merged.noTui ?? merged.tui === false, diff --git a/src/test/readonly-installer.fixture.ts b/src/test/readonly-installer.fixture.ts new file mode 100644 index 00000000..b4a96846 --- /dev/null +++ b/src/test/readonly-installer.fixture.ts @@ -0,0 +1,194 @@ +// Bun subprocess fixture: real parser/orchestration, fake external boundaries. +// @ts-expect-error This subprocess runs on Bun; the project typechecks against Node types. +import { mock } from 'bun:test'; +import { appendFileSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import type { InstallerOptions } from '../utils/types.js'; + +const record = (kind: string, value: unknown) => + appendFileSync(process.env.TEST_EVIDENCE!, `${JSON.stringify({ kind, value })}\n`); +const forbidden = + (name: string) => + (..._args: unknown[]): never => { + record('forbidden', name); + throw new Error(`Forbidden external call: ${name}`); + }; + +// Intercept before importing CLI modules. Use real Git for inspection/branch +// creation, but never invoke a shell or rely on POSIX PATH executable shims. +const childProcess = await import('node:child_process'); +const realExecFileSync = childProcess.execFileSync; +const execFileSync = (( + file: string, + args: string[] = [], + options?: import('node:child_process').ExecFileSyncOptions, +) => { + record('command', { file, args }); + const readOnly = ['rev-parse', 'status'].includes(args[0]); + const branch = args.length === 3 && args[0] === 'checkout' && args[1] === '-b'; + if (file !== 'git' || (!readOnly && !branch) || options?.shell) { + forbidden(`execFileSync ${file} ${args.join(' ')}`)(); + } + if (process.env.TEST_INSPECTION && args.includes('--untracked-files=all')) { + // Real Bun child-process limits, with deterministic slow/large fake Git output. + const script = + process.env.TEST_INSPECTION === 'timeout' + ? `process.stdout.write('?? partial.ts\\0'); setInterval(() => {}, 1000);` + : `process.stdout.write(Array.from({ length: 80_000 }, (_, i) => '?? generated/untracked-' + i + '.ts\\0').join(''));`; + return realExecFileSync(process.execPath, ['--eval', script], options); + } + return realExecFileSync(file, args, options); +}) as typeof childProcess.execFileSync; +const execSync = ((command: string, options?: import('node:child_process').ExecSyncOptions) => { + if ( + !['git rev-parse --abbrev-ref HEAD', 'git rev-parse --is-inside-work-tree', 'git status --porcelain=v1'].includes( + command, + ) + ) + forbidden(`execSync ${command}`)(); + return execFileSync('git', command.split(' ').slice(1), options); +}) as typeof childProcess.execSync; +const guardedProcesses = { + ...childProcess, + execFileSync, + execSync, + exec: forbidden('exec'), + execFile: forbidden('execFile'), + spawn: forbidden('spawn'), + spawnSync: forbidden('spawnSync'), + fork: forbidden('fork'), +}; +mock.module('node:child_process', () => ({ ...guardedProcesses, default: guardedProcesses })); + +// Block both native keychain backends BEFORE loading any CLI modules. +mock.module('@napi-rs/keyring', () => ({ + Entry: class { + constructor() { + forbidden('native keyring')(); + } + }, +})); +mock.module('../lib/darwin-keychain.js', () => ({ + DarwinSecurityEntry: class { + constructor() { + forbidden('darwin keychain')(); + } + }, +})); +await import('./force-insecure-storage.js'); +globalThis.fetch = forbidden('fetch'); +mock.module('@anthropic-ai/sdk', () => ({ + default: class { + constructor() { + forbidden('Anthropic')(); + } + }, +})); +mock.module('@anthropic-ai/claude-agent-sdk', () => ({ query: forbidden('agent SDK') })); +mock.module('../lib/credential-proxy.js', () => ({ startCredentialProxy: forbidden('credential proxy') })); +mock.module('../lib/version-check.js', () => ({ checkForUpdates: async () => {} })); +mock.module('../lib/resolve-install-credentials.js', () => ({ + resolveInstallCredentials: async () => {}, + maybePickInstallEnvironment: async () => {}, + resolveStagingCredentials: forbidden('staging credentials'), +})); +mock.module('../commands/setup.js', () => ({ maybeRunSetupAfter: async () => {} })); +const credentials = await import('../lib/credentials.js'); +mock.module('../lib/credentials.js', () => ({ ...credentials, getAccessToken: () => 'offline-test-token' })); +const application = await import('../lib/authkit-application-setup.js'); +mock.module('../lib/authkit-application-setup.js', () => ({ + ...application, + configureAuthkitApplication: async (setup: object) => ({ + ...setup, + verified: false, + reason: 'Offline fixture: browser flows not tested.', + }), +})); +const config = { + metadata: { integration: 'vanilla-js', language: 'javascript', docsUrl: 'https://workos.com/docs' }, + environment: { requiresApiKey: false }, + ui: {}, +}; +mock.module('../lib/registry.js', () => ({ + getRegistry: async () => ({ + detectionOrder: () => [config], + get: () => ({ + config, + run: async (options: InstallerOptions) => { + record('agent-options', { + noCommit: options.noCommit, + createPr: options.createPr, + installDir: options.installDir, + }); + if (process.env.TEST_AGENT === 'fail') throw new Error('Fake installation failed'); + writeFileSync(join(options.installDir, 'generated.ts'), 'export const authkit = true;\n'); + writeFileSync(join(options.installDir, 'tracked.ts'), 'export const modified = true;\n'); + options.emitter?.emit('validation:start', { framework: 'vanilla-js' }); + options.emitter?.emit('validation:complete', { passed: true, issueCount: 0, durationMs: 1 }); + return 'Offline installation completed'; + }, + }), + }), +})); + +// Human parser paths still use the real CLI adapter; answer only its expected +// pre-install questions. Any commit/PR question is a hard test failure. +if (process.env.TEST_HUMAN === '1') { + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stderr, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'columns', { value: 79, configurable: true }); + const { default: ui, CANCEL } = await import('../utils/ui.js'); + ui.confirm = async ({ message }) => { + record('prompt', message); + if (!['Run the AuthKit installer?', 'Continue anyway?'].includes(message)) forbidden(message)(); + return process.env.TEST_CANCEL !== '1' || message !== 'Continue anyway?'; + }; + ui.select = async ({ message, signal }) => { + if (signal?.aborted) return CANCEL; + record('prompt', message); + if (!message.includes('Create a feature branch?')) forbidden(message)(); + return 'create' as never; + }; +} + +if (process.env.TEST_ENTRY === 'guard-probe') { + const guarded = await import('node:child_process'); + // These must all throw AND leave evidence even if a caller catches the error. + const probes = [ + () => guarded.execFileSync('git', ['add', '-A']), + () => guarded.execFileSync('git', ['commit', '-m', 'forbidden']), + () => guarded.execFileSync('git', ['push']), + () => guarded.execFileSync('gh', ['pr', 'create']), + () => guarded.execSync('git status --porcelain=v1 && git push'), + () => guarded.exec('git push'), + () => guarded.execFile('git', ['push']), + () => guarded.spawn('git', ['push']), + () => guarded.spawnSync('git', ['push']), + () => guarded.fork('forbidden.js'), + ]; + for (const probe of probes) { + try { + probe(); + } catch { + continue; + } + throw new Error('Process guard allowed a forbidden call'); + } +} else if (process.env.TEST_ENTRY === 'programmatic') { + const { setOutputMode } = await import('../utils/output.js'); + setOutputMode('json'); + const { runWithCore } = await import('../lib/run-with-core.js'); + await runWithCore({ + installDir: process.cwd(), + skipAuth: true, + apiKey: 'sk_test_offline', + clientId: 'client_offline', + noGitCheck: true, + noBranch: true, + noCommit: false, + createPr: true, + } as InstallerOptions); +} else { + await import('../bin.js'); +} diff --git a/src/tui/content/installer-content.json b/src/tui/content/installer-content.json index ee32f860..e8a8cab2 100644 --- a/src/tui/content/installer-content.json +++ b/src/tui/content/installer-content.json @@ -103,10 +103,12 @@ "failed": "A few things are worth a look (issues: {count}). They'll be listed when this screen closes." }, "app-urls:step": "Your app's routes are in place, so now I'm pointing WorkOS at them.", - "postinstall:changes": "Files changed: {count}.", - "postinstall:commit:success": "Committed: {message}", - "postinstall:pr:success": "Pull request opened: {url}", - "postinstall:manual": "The GitHub CLI isn't installed, so here's how to open the pull request yourself.", + "postinstall:changes": "Current changed files: {count} (may include pre-existing changes). Nothing was committed; review the project and commit when ready.", + "postinstall:nochanges": "No Git changes detected in the install directory.", + "postinstall:unavailable": { + "not-git": "This project is not a Git working tree; review the files manually. Nothing was committed.", + "error": "I couldn't inspect Git changes; review the files manually. Nothing was committed." + }, "complete": { "success": "All done. AuthKit is installed. Start your app and complete your first sign-up.", "setup-required": "The code is in. Some WorkOS settings need a look in the dashboard (listed above). Then start your app and complete your first sign-up.", diff --git a/src/tui/content/schema.ts b/src/tui/content/schema.ts index 79347790..d6af5f08 100644 --- a/src/tui/content/schema.ts +++ b/src/tui/content/schema.ts @@ -54,8 +54,6 @@ export const WALKTHROUGH_PARAMS: Partial> = { 'validation:complete': ['passed', 'failed'], + 'postinstall:unavailable': ['not-git', 'error'], complete: ['success', 'setup-required', 'failure', 'cancelled'], }; diff --git a/src/tui/model/run-model.spec.ts b/src/tui/model/run-model.spec.ts index 2aba13a1..9f802080 100644 --- a/src/tui/model/run-model.spec.ts +++ b/src/tui/model/run-model.spec.ts @@ -31,6 +31,7 @@ function actors(overrides: Record = {}) { })), checkGitStatus: fromPromise(async () => ({ isClean: true, files: [] })), configureEnvironment: fromPromise(async () => {}), + detectChanges: fromPromise(async () => ({ state: 'unchanged' as const, files: [] })), runAgent: fromPromise(async () => ({ success: true, summary: 'Done!', @@ -48,7 +49,6 @@ function options(overrides: Partial = {}): InstallerOptions { local: true, ci: false, skipAuth: false, - noCommit: true, emitter: null!, apiKey: 'sk_test_123', clientId: 'client_test_123', @@ -406,22 +406,22 @@ describe('run model: tips, prompt, status, notices', () => { let notified = 0; const unsubscribe = model.subscribe(() => notified++); - model.setPrompt({ kind: 'confirm', message: 'Commit the changes?', initialValue: true }); - expect(model.getSnapshot().prompt).toEqual({ kind: 'confirm', message: 'Commit the changes?', initialValue: true }); - model.setStatus('Generating commit message...'); + model.setPrompt({ kind: 'confirm', message: 'Continue anyway?', initialValue: true }); + expect(model.getSnapshot().prompt).toEqual({ kind: 'confirm', message: 'Continue anyway?', initialValue: true }); + model.setStatus('Inspecting project changes...'); const withStatus = model.getSnapshot(); - model.setStatus('Generating commit message...'); // unchanged: same snapshot + model.setStatus('Inspecting project changes...'); // unchanged: same snapshot expect(model.getSnapshot()).toBe(withStatus); - model.addNotice('error', 'Commit failed: nothing to commit'); + model.addNotice('error', 'Change inspection failed'); model.setPrompt(null); const snapshot = model.getSnapshot(); expect(snapshot.prompt).toBeNull(); - expect(snapshot.status).toBe('Generating commit message...'); + expect(snapshot.status).toBe('Inspecting project changes...'); expect(snapshot.walkthrough.at(-1)).toMatchObject({ kind: 'notice', tone: 'error', - text: 'Commit failed: nothing to commit', + text: 'Change inspection failed', }); expect(notified).toBeGreaterThan(0); @@ -475,8 +475,6 @@ describe('run model: tips, prompt, status, notices', () => { 'agent:tool': { kind: 'command', detail: 'ls' }, 'validation:complete': { passed: false, issueCount: 1, durationMs: 1 }, 'postinstall:changes': { files: ['a'] }, - 'postinstall:commit:success': { message: 'feat: add AuthKit' }, - 'postinstall:pr:success': { url: 'https://github.com/o/r/pull/1' }, }; expect(Object.keys(payloads).sort()).toEqual(Object.keys(WALKTHROUGH_PARAMS).sort()); diff --git a/src/tui/model/run-model.ts b/src/tui/model/run-model.ts index 5c1824b5..b0600082 100644 --- a/src/tui/model/run-model.ts +++ b/src/tui/model/run-model.ts @@ -169,8 +169,7 @@ const TONES: Partial> = { 'agent:retry': 'warning', 'agent:success': 'success', 'agent:failure': 'error', - 'postinstall:commit:success': 'success', - 'postinstall:pr:success': 'success', + 'postinstall:unavailable': 'warning', }; const MAX_COMMAND = 80; @@ -431,8 +430,7 @@ export function createRunModel(options: RunModelOptions): RunModel { }, 'postinstall:changes': ({ files }) => narrate('postinstall:changes', { count: files.length }), - 'postinstall:commit:success': ({ message }) => narrate('postinstall:commit:success', { message }), - 'postinstall:pr:success': ({ url }) => narrate('postinstall:pr:success', { url }), + 'postinstall:unavailable': ({ reason }) => narrate('postinstall:unavailable', {}, reason), complete: ({ success }) => { outcome = success ? 'success' : cancelled ? 'cancelled' : 'failure'; diff --git a/src/utils/git-utils.ts b/src/utils/git-utils.ts index 79e1b36c..a4936d53 100644 --- a/src/utils/git-utils.ts +++ b/src/utils/git-utils.ts @@ -37,50 +37,3 @@ export function branchExists(name: string): boolean { return false; } } - -/** - * Get the default branch from origin, falling back to main/master detection. - */ -export function getDefaultBranch(): string { - try { - const ref = execSync('git symbolic-ref refs/remotes/origin/HEAD', { - stdio: ['ignore', 'pipe', 'ignore'], - }) - .toString() - .trim(); - return ref.replace('refs/remotes/origin/', ''); - } catch { - if (branchExists('main')) return 'main'; - if (branchExists('master')) return 'master'; - return 'main'; - } -} - -/** - * Check if the GitHub CLI (gh) is available. - */ -export function hasGhCli(): boolean { - try { - execSync('gh --version', { stdio: 'ignore' }); - return true; - } catch { - return false; - } -} - -/** - * Get list of uncommitted/untracked files from git status. - */ -export function getUncommittedFiles(): string[] { - try { - const status = execSync('git status --porcelain', { - stdio: ['ignore', 'pipe', 'ignore'], - }).toString(); - return status - .split('\n') - .filter(Boolean) - .map((line) => line.slice(3)); - } catch { - return []; - } -} diff --git a/src/utils/help-json.ts b/src/utils/help-json.ts index 39ca3400..9ace0fcf 100644 --- a/src/utils/help-json.ts +++ b/src/utils/help-json.ts @@ -2120,17 +2120,15 @@ const commands: CommandSchema[] = [ { name: 'commit', type: 'boolean', - description: 'Auto-commit after installation (use --no-commit to skip)', + description: 'Deprecated no-op (--commit/--no-commit); installer changes are left uncommitted', required: false, - default: true, hidden: false, }, { name: 'create-pr', type: 'boolean', - description: 'Auto-create pull request after installation', + description: 'Deprecated no-op; the installer never pushes or creates pull requests', required: false, - default: false, hidden: false, }, { diff --git a/src/utils/summary-box.ts b/src/utils/summary-box.ts index 0296f4cb..01b1d1d8 100644 --- a/src/utils/summary-box.ts +++ b/src/utils/summary-box.ts @@ -22,7 +22,13 @@ export function renderCompletionSummary(success: boolean, summary?: string, comp // Code in, dashboard not yet: a warning, not a success. tone: setupPending ? 'warning' : 'success', title: setupPending ? 'App code installed; WorkOS setup required' : 'WorkOS AuthKit Installed', - items: [...shown, ...steps], + items: [ + ...(files.length + ? [{ type: 'pending' as const, text: 'Current changed files (may include pre-existing changes):' }] + : []), + ...shown, + ...steps, + ], footer: completion.docsUrl, }); } @@ -34,6 +40,7 @@ export function renderCompletionSummary(success: boolean, summary?: string, comp ...(summary ? [{ type: 'pending' as const, text: summary }] : []), { type: 'pending', text: 'Start dev server to test authentication' }, { type: 'pending', text: 'Visit WorkOS Dashboard to manage users' }, + { type: 'pending', text: 'Changes are left uncommitted. Review the project and commit when ready.' }, ], footer: 'https://workos.com/docs/authkit', }); diff --git a/src/utils/types.ts b/src/utils/types.ts index be36754f..32b67733 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -87,7 +87,7 @@ export type InstallerOptions = { noValidate?: boolean; /** - * Skip post-install commit and PR workflow + * @deprecated Ignored: installer changes are always left uncommitted. */ noCommit?: boolean; @@ -97,7 +97,7 @@ export type InstallerOptions = { noBranch?: boolean; /** - * Auto-create pull request after installation + * @deprecated Ignored: the installer never pushes or creates pull requests. */ createPr?: boolean;