Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-codex-bootstrap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fission-ai/openspec": patch
---

Return a nonzero exit status when `openspec update --force` cannot replace a legacy-only Codex installation.
30 changes: 24 additions & 6 deletions src/core/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ const { version: OPENSPEC_VERSION } = require('../../package.json');
type LegacyUpgradeResult = {
newlyConfiguredTools: string[];
workflowOverrides: Partial<Record<string, readonly (typeof ALL_WORKFLOWS)[number][]>>;
failedTools?: ToolFailure[];
deferredGlobalCleanup?: LegacyDetectionResult;
/**
* Tools whose skill generation was skipped because another tool already owns
Expand All @@ -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
Expand Down Expand Up @@ -185,6 +194,7 @@ export class UpdateCommand {
newlyConfiguredTools,
workflowOverrides: legacyWorkflowOverrides,
deferredGlobalCleanup,
failedTools: legacyUpgradeFailures = [],
} = legacyUpgrade;

// 5. Find configured tools
Expand All @@ -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.
Expand Down Expand Up @@ -269,6 +280,7 @@ export class UpdateCommand {
this.detectNewTools(resolvedProjectPath, configuredTools);
this.displayProfileNotes(resolvedProjectPath, configuredTools, desiredWorkflows, profile, delivery);
this.displaySetupNotes(configuredTools);
throwIfUpdateFailed(legacyUpgradeFailures);
return;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<void> {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -1377,14 +1388,21 @@ 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 });
}
}

if (newlyConfigured.length > 0) {
console.log();
}

return { newlyConfiguredTools: newlyConfigured, workflowOverrides, skippedSharedSkillTools };
return {
newlyConfiguredTools: newlyConfigured,
workflowOverrides,
skippedSharedSkillTools,
failedTools,
};
}
}
115 changes: 115 additions & 0 deletions test/core/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof import('../../src/utils/interactive.js')>();
return { ...actual, isInteractive: () => interactiveState.value };
});

// Shared mutable mock config state
const mockState = {
config: {
Expand Down Expand Up @@ -45,6 +62,23 @@ async function markCodexTarget(skillsDir: string): Promise<void> {
await fs.writeFile(path.join(skillsDir, '.openspec-target'), 'codex\n');
}

async function createLegacyCodexPrompt(): Promise<string> {
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;
Expand All @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
Loading