diff --git a/.changeset/tidy-codex-bootstrap.md b/.changeset/tidy-codex-bootstrap.md new file mode 100644 index 0000000000..10c19576f5 --- /dev/null +++ b/.changeset/tidy-codex-bootstrap.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Return a nonzero exit status when `openspec update --force` cannot replace a legacy-only Codex installation. diff --git a/src/core/update.ts b/src/core/update.ts index 772950fbab..1f624a345d 100644 --- a/src/core/update.ts +++ b/src/core/update.ts @@ -89,6 +89,7 @@ const { version: OPENSPEC_VERSION } = require('../../package.json'); type LegacyUpgradeResult = { newlyConfiguredTools: string[]; workflowOverrides: Partial>; + failedTools?: ToolFailure[]; deferredGlobalCleanup?: LegacyDetectionResult; /** * Tools whose skill generation was skipped because another tool already owns @@ -98,6 +99,14 @@ type LegacyUpgradeResult = { skippedSharedSkillTools?: string[]; }; +type ToolFailure = { name: string; error: string }; + +function throwIfUpdateFailed(failedTools: readonly ToolFailure[]): void { + if (failedTools.length > 0) { + throw new Error(`OpenSpec update failed for: ${failedTools.map((tool) => tool.name).join(', ')}`); + } +} + /** * Checkout artifacts that are not real content drift: a UTF-8 BOM and the CRLF * line endings a Windows clone with `core.autocrlf` reintroduces on every @@ -185,6 +194,7 @@ export class UpdateCommand { newlyConfiguredTools, workflowOverrides: legacyWorkflowOverrides, deferredGlobalCleanup, + failedTools: legacyUpgradeFailures = [], } = legacyUpgrade; // 5. Find configured tools @@ -195,6 +205,7 @@ export class UpdateCommand { if (deferredGlobalCleanup) { await this.performDeferredGlobalPromptCleanup(resolvedProjectPath, deferredGlobalCleanup); } + throwIfUpdateFailed(legacyUpgradeFailures); if (declinedMigrations.length > 0) { // Not an unconfigured project — a configured one the user chose to // leave in its former directory. Saying "run init" would be wrong. @@ -269,6 +280,7 @@ export class UpdateCommand { this.detectNewTools(resolvedProjectPath, configuredTools); this.displayProfileNotes(resolvedProjectPath, configuredTools, desiredWorkflows, profile, delivery); this.displaySetupNotes(configuredTools); + throwIfUpdateFailed(legacyUpgradeFailures); return; } @@ -299,7 +311,7 @@ export class UpdateCommand { ); const updatedTools: string[] = []; const updatedToolIds: string[] = []; - const failedTools: Array<{ name: string; error: string }> = []; + const failedTools: ToolFailure[] = [...legacyUpgradeFailures]; const skillsInvocableCommandSkips: string[] = []; const zeroArtifactTools: string[] = []; let removedCommandCount = 0; @@ -526,9 +538,7 @@ export class UpdateCommand { if (restartHint) { console.log(chalk.dim(restartHint)); } - if (failedTools.length > 0) { - throw new Error(`OpenSpec update failed for: ${failedTools.map((tool) => tool.name).join(', ')}`); - } + throwIfUpdateFailed(failedTools); } private async syncCopilotCloudFiles(projectPath: string, configuredTools: string[]): Promise { @@ -1277,6 +1287,7 @@ export class UpdateCommand { // Create skills/commands for selected tools using effective profile+delivery. const newlyConfigured: string[] = []; const skippedSharedSkillTools: string[] = []; + const failedTools: ToolFailure[] = []; const workflowOverrides: LegacyUpgradeResult['workflowOverrides'] = {}; const arbitrationTools = [...new Set([...configuredTools, ...selectedTools])] .map((toolId) => AI_TOOLS.find((tool) => tool.value === toolId)) @@ -1377,7 +1388,9 @@ export class UpdateCommand { } } catch (error) { spinner.fail(`Failed to set up ${tool.name}`); - console.log(chalk.red(` ${error instanceof Error ? error.message : String(error)}`)); + const message = error instanceof Error ? error.message : String(error); + console.log(chalk.red(` ${message}`)); + failedTools.push({ name: tool.name, error: message }); } } @@ -1385,6 +1398,11 @@ export class UpdateCommand { console.log(); } - return { newlyConfiguredTools: newlyConfigured, workflowOverrides, skippedSharedSkillTools }; + return { + newlyConfiguredTools: newlyConfigured, + workflowOverrides, + skippedSharedSkillTools, + failedTools, + }; } } diff --git a/test/core/update.test.ts b/test/core/update.test.ts index 8541d43d46..ae0efd7d84 100644 --- a/test/core/update.test.ts +++ b/test/core/update.test.ts @@ -11,6 +11,23 @@ import path from 'path'; import fs from 'fs/promises'; import os from 'os'; +const { confirmMock, searchableMultiSelectMock, interactiveState } = vi.hoisted(() => ({ + confirmMock: vi.fn(), + searchableMultiSelectMock: vi.fn(), + interactiveState: { value: false }, +})); + +vi.mock('@inquirer/prompts', () => ({ confirm: confirmMock })); + +vi.mock('../../src/prompts/searchable-multi-select.js', () => ({ + searchableMultiSelect: searchableMultiSelectMock, +})); + +vi.mock('../../src/utils/interactive.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, isInteractive: () => interactiveState.value }; +}); + // Shared mutable mock config state const mockState = { config: { @@ -45,6 +62,23 @@ async function markCodexTarget(skillsDir: string): Promise { await fs.writeFile(path.join(skillsDir, '.openspec-target'), 'codex\n'); } +async function createLegacyCodexPrompt(): Promise { + const prompt = path.join(process.env.CODEX_HOME!, 'prompts', 'opsx-explore.md'); + await fs.mkdir(path.dirname(prompt), { recursive: true }); + await fs.writeFile(prompt, 'legacy prompt'); + return prompt; +} + +function failCodexSkillWrites(): void { + const originalWriteFile = FileSystemUtils.writeFile.bind(FileSystemUtils); + vi.spyOn(FileSystemUtils, 'writeFile').mockImplementation(async (filePath, content) => { + if (filePath.includes(`${path.sep}.agents${path.sep}`) && filePath.endsWith('SKILL.md')) { + throw new Error('EACCES: permission denied'); + } + return originalWriteFile(filePath, content); + }); +} + describe('UpdateCommand', () => { let testDir: string; let updateCommand: UpdateCommand; @@ -66,6 +100,9 @@ describe('UpdateCommand', () => { // Reset mock config to defaults resetMockConfig(); + interactiveState.value = false; + confirmMock.mockReset(); + searchableMultiSelectMock.mockReset(); // Clear all mocks before each test vi.restoreAllMocks(); @@ -1688,6 +1725,84 @@ metadata: }); describe('error handling', () => { + it('should report a failed legacy-only Codex bootstrap to automation', async () => { + setMockConfig({ featureFlags: {}, profile: 'core', delivery: 'commands' }); + + const prompt = await createLegacyCodexPrompt(); + failCodexSkillWrites(); + + await expect(new UpdateCommand({ force: true }).execute(testDir)).rejects.toThrow( + 'OpenSpec update failed for: Codex' + ); + await expect(fs.access(prompt)).resolves.toBeUndefined(); + await expect( + fs.access(path.join(testDir, '.agents', 'skills', 'openspec-explore', 'SKILL.md')) + ).rejects.toThrow(); + }); + + it('should refresh configured tools after a legacy Codex bootstrap fails', async () => { + setMockConfig({ featureFlags: {}, profile: 'core', delivery: 'commands' }); + + await createLegacyCodexPrompt(); + + const cursorCommand = path.join(testDir, '.cursor', 'commands', 'opsx-explore.md'); + await fs.mkdir(path.dirname(cursorCommand), { recursive: true }); + await fs.writeFile(cursorCommand, 'old'); + + failCodexSkillWrites(); + + await expect(new UpdateCommand({ force: true }).execute(testDir)).rejects.toThrow( + 'OpenSpec update failed for: Codex' + ); + expect(await fs.readFile(cursorCommand, 'utf-8')).not.toBe('old'); + }); + + it('should report a failed bootstrap when configured tools are already current', async () => { + setMockConfig({ featureFlags: {}, profile: 'core', delivery: 'commands' }); + await new InitCommand({ tools: 'cursor', force: true }).execute(testDir); + + await createLegacyCodexPrompt(); + + interactiveState.value = true; + confirmMock.mockResolvedValue(true); + searchableMultiSelectMock.mockResolvedValue(['codex']); + + failCodexSkillWrites(); + const consoleSpy = vi.spyOn(console, 'log'); + + await expect(new UpdateCommand().execute(testDir)).rejects.toThrow( + 'OpenSpec update failed for: Codex' + ); + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('All 1 tool(s) up to date')); + }); + + it('should report a failed bootstrap after declining an unrelated migration', async () => { + setMockConfig({ featureFlags: {}, profile: 'core', delivery: 'commands' }); + + const legacySkill = path.join( + testDir, + '.windsurf', + 'skills', + 'openspec-explore', + 'SKILL.md' + ); + await fs.mkdir(path.dirname(legacySkill), { recursive: true }); + await fs.writeFile(legacySkill, 'legacy skill'); + + await createLegacyCodexPrompt(); + + interactiveState.value = true; + confirmMock.mockResolvedValueOnce(false).mockResolvedValueOnce(true); + searchableMultiSelectMock.mockResolvedValue(['codex']); + + failCodexSkillWrites(); + + await expect(new UpdateCommand().execute(testDir)).rejects.toThrow( + 'OpenSpec update failed for: Codex' + ); + expect(await fs.readFile(legacySkill, 'utf-8')).toBe('legacy skill'); + }); + it('should preserve legacy Codex skills and prompts when canonical generation fails', async () => { const legacySkill = path.join( testDir,