Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 7 additions & 0 deletions .changeset/fix-archive-exit-code.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@fission-ai/openspec": patch
---

### Bug Fixes

- **`archive` exits non-zero when blocked in human mode** — `openspec archive <change> -y` (and any non-`--json` invocation) no longer returns exit code 0 when validation fails and nothing is archived. The three blocking paths in human mode — delta-spec validation failure, spec rebuild failure, and rebuilt-spec validation failure — now set `process.exitCode = 1`, matching the existing `--json` behavior. Previously the command printed "Validation failed" (or "Aborted. No files were changed.") and exited 0, letting scripts and CI believe the archive succeeded. Aligns `archive` with the same exit-code guarantee already approved for `apply` instructions (#1250).
3 changes: 3 additions & 0 deletions src/core/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ export class ArchiveCommand {
}
console.log(chalk.red('\nValidation failed. Please fix the errors before archiving.'));
console.log(chalk.yellow('To skip validation (not recommended), use --no-validate flag.'));
process.exitCode = 1;
return null;
}
} else if (json) {
Expand Down Expand Up @@ -428,6 +429,7 @@ export class ArchiveCommand {
}
console.log(String(err.message || err));
console.log('Aborted. No files were changed.');
process.exitCode = 1;
return null;
}

Expand All @@ -451,6 +453,7 @@ export class ArchiveCommand {
else if (issue.level === 'WARNING') console.log(chalk.yellow(` ⚠ ${issue.message}`));
}
console.log('Aborted. No files were changed.');
process.exitCode = 1;
return null;
}
}
Expand Down
94 changes: 94 additions & 0 deletions test/core/archive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('ArchiveCommand', () => {
let tempDir: string;
let archiveCommand: ArchiveCommand;
const originalConsoleLog = console.log;
const originalExitCode = process.exitCode;
const originalXdgDataHome = process.env.XDG_DATA_HOME;

beforeEach(async () => {
Expand All @@ -38,13 +39,20 @@ describe('ArchiveCommand', () => {
// Suppress console.log during tests
console.log = vi.fn();

// Isolate process.exitCode so a failing run can't leak into the next
// test or skew the vitest process exit status.
process.exitCode = undefined;

archiveCommand = new ArchiveCommand();
});

afterEach(async () => {
// Restore console.log
console.log = originalConsoleLog;

// Restore process.exitCode (clear anything a test set)
process.exitCode = originalExitCode;

if (originalXdgDataHome === undefined) {
delete process.env.XDG_DATA_HOME;
} else {
Expand Down Expand Up @@ -826,6 +834,92 @@ E1 updated`);
});
});

describe('exit code on blocked archive (human mode)', () => {
// Regression for the silent-exit-0 bug: when archive is blocked in
// human mode it must set a non-zero exit code so scripts/CI can detect
// the failure, mirroring the JSON-mode behavior.
it('sets exit code 1 when delta spec validation fails', async () => {
const changeName = 'exit-delta-fail';
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
const changeSpecDir = path.join(changeDir, 'specs', 'bad-capability');
await fs.mkdir(changeSpecDir, { recursive: true });

// Delta spec missing required SHALL/MUST keyword -> validation error
const specContent = `# Bad Capability - Changes

## ADDED Requirements

### Requirement: Logging Feature

The system will log all events.

#### Scenario: Event recorded
- **WHEN** an event occurs
- **THEN** it is captured`;
await fs.writeFile(path.join(changeSpecDir, 'spec.md'), specContent);
await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task 1\n');

await archiveCommand.execute(changeName, { yes: true, skipSpecs: true });

expect(process.exitCode).toBe(1);
expect(console.log).toHaveBeenCalledWith(
expect.stringContaining('Validation failed')
);

// Change must NOT have been archived
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives.some(a => a.includes(changeName))).toBe(false);
});

it('sets exit code 1 when spec rebuild fails (MODIFIED on new spec)', async () => {
const changeName = 'exit-rebuild-fail';
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
const changeSpecDir = path.join(changeDir, 'specs', 'new-capability');
await fs.mkdir(changeSpecDir, { recursive: true });

// MODIFIED on a non-existent target spec aborts the rebuild
const specContent = `# New Capability - Changes

## ADDED Requirements

### Requirement: New Feature
New feature description.

## MODIFIED Requirements

### Requirement: Existing Feature
Modified content.`;
await fs.writeFile(path.join(changeSpecDir, 'spec.md'), specContent);

await archiveCommand.execute(changeName, { yes: true, noValidate: true });

expect(process.exitCode).toBe(1);
expect(console.log).toHaveBeenCalledWith('Aborted. No files were changed.');

const mainSpecPath = path.join(tempDir, 'openspec', 'specs', 'new-capability', 'spec.md');
await expect(fs.access(mainSpecPath)).rejects.toThrow();

const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives.some(a => a.includes(changeName))).toBe(false);
});

it('leaves exit code 0 on successful archive (no leak from prior test)', async () => {
const changeName = 'exit-ok';
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
await fs.mkdir(changeDir, { recursive: true });

await archiveCommand.execute(changeName, { yes: true });

expect(process.exitCode).toBeUndefined();

const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives.some(a => a.includes(changeName))).toBe(true);
});
});

describe('error handling', () => {
it('should throw error when openspec directory does not exist', async () => {
// Remove openspec directory
Expand Down