Skip to content

Commit fd9e069

Browse files
ixxieclaude
andcommitted
fix(lifecycle): sync returns its report — exit codes live at the CLI edge
Programmatic callers read the returned SyncReport instead of sniffing process.exitCode; a new silent option suppresses output for reuse as a gate. Unreadable metadata on a named sync now yields the same conflict entry the no-arg sweep reports, so CI parses one shape either way, and list rejects an unknown --status value instead of printing an empty list that reads as success. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4d72e3d commit fd9e069

5 files changed

Lines changed: 101 additions & 31 deletions

File tree

src/cli/index.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,10 @@ program
461461
.option('--json', 'Output as JSON (non-interactive)')
462462
.action(async (changeName?: string, options?: { check?: boolean; json?: boolean }) => {
463463
try {
464-
await new SyncCommand().execute(changeName, '.', options ?? {});
464+
const report = await new SyncCommand().execute(changeName, '.', options ?? {});
465+
if (!report.clean) {
466+
process.exitCode = 1;
467+
}
465468
} catch (error) {
466469
failWithError(error);
467470
process.exit(1);
@@ -476,7 +479,10 @@ program
476479
.option('--json', 'Output as JSON (non-interactive)')
477480
.action(async (changeName: string, options?: { json?: boolean }) => {
478481
try {
479-
await new ShipCommand().execute(changeName, '.', options ?? {});
482+
const report = await new ShipCommand().execute(changeName, '.', options ?? {});
483+
if (!report.clean) {
484+
process.exitCode = 1;
485+
}
480486
} catch (error) {
481487
failWithError(error);
482488
process.exit(1);

src/core/list.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ export class ListCommand {
118118
async execute(targetPath: string = '.', mode: 'changes' | 'specs' = 'changes', options: ListOptions = {}): Promise<void> {
119119
const { sort = 'recent', json = false, root } = options;
120120

121+
if (options.status && !LIFECYCLE_STATES.has(options.status)) {
122+
throw new Error(
123+
`Unknown status '${options.status}' — expected one of: ${[...LIFECYCLE_STATES].join(', ')}.`
124+
);
125+
}
126+
121127
if (mode === 'changes') {
122128
const changesDir = path.join(targetPath, 'openspec', 'changes');
123129

src/core/sync.ts

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import { resolveLifecycle } from './project-config.js';
1717
export interface SyncOptions {
1818
check?: boolean;
1919
json?: boolean;
20+
/** Suppress all output — programmatic callers read the returned report. */
21+
silent?: boolean;
2022
}
2123

2224
type PendingFold = {
@@ -57,22 +59,25 @@ export class SyncCommand {
5759
changeName: string | undefined,
5860
targetPath: string = '.',
5961
options: SyncOptions = {}
60-
): Promise<void> {
62+
): Promise<SyncReport> {
6163
const mode = resolveLifecycle(targetPath);
6264
const report: SyncReport = { mode, changes: [], clean: true };
6365

6466
if (mode !== 'status') {
6567
// Mode-aware by contract: under `lifecycle: archive` the archive command
6668
// owns the fold and there is no status field to gate on. Report and exit
6769
// 0 rather than misfiring on the default layout.
70+
if (options.silent) {
71+
return report;
72+
}
6873
if (options.json) {
6974
console.log(JSON.stringify(report, null, 2));
7075
} else {
7176
console.log(
7277
"This project uses `lifecycle: archive` (the default) — nothing to sync or gate. `openspec sync` applies under `lifecycle: status`; see openspec/config.yaml."
7378
);
7479
}
75-
return;
80+
return report;
7681
}
7782

7883
const changesDir = path.join(targetPath, 'openspec', 'changes');
@@ -95,23 +100,23 @@ export class SyncCommand {
95100
if (!options.check && state.report.state === 'unfolded') {
96101
for (const fold of state.folds) {
97102
await writeUpdatedSpec(fold.update, fold.rebuilt, fold.counts, {
98-
silent: options.json,
103+
silent: options.json || options.silent,
99104
});
100105
}
101106
state.report.state = 'folded';
102107
report.clean = report.changes.every((c) => c.state === 'folded');
103108
}
104109
}
105110

106-
if (options.json) {
107-
console.log(JSON.stringify(report, null, 2));
108-
} else {
109-
this.print(report, options);
111+
if (!options.silent) {
112+
if (options.json) {
113+
console.log(JSON.stringify(report, null, 2));
114+
} else {
115+
this.print(report, options);
116+
}
110117
}
111118

112-
if (!report.clean) {
113-
process.exitCode = 1;
114-
}
119+
return report;
115120
}
116121

117122
private async shippedChanges(
@@ -166,7 +171,22 @@ export class SyncCommand {
166171

167172
// An explicitly named change must be shipped before its deltas may touch
168173
// specs/. In check mode a non-shipped change is simply not gated.
169-
const metadata = readChangeMetadata(changeDir, projectRoot);
174+
// Unreadable metadata is the same conflict entry the no-arg sweep reports,
175+
// so CI sees one shape either way.
176+
let metadata;
177+
try {
178+
metadata = readChangeMetadata(changeDir, projectRoot);
179+
} catch (err) {
180+
return {
181+
report: {
182+
change: name,
183+
state: 'conflict',
184+
pending: [],
185+
error: err instanceof ChangeMetadataError ? err.message : String(err),
186+
},
187+
folds: [],
188+
};
189+
}
170190
if (metadata?.status !== 'shipped') {
171191
if (options.check) {
172192
return null;
@@ -240,7 +260,7 @@ export class ShipCommand {
240260
changeName: string,
241261
targetPath: string = '.',
242262
options: { json?: boolean } = {}
243-
): Promise<void> {
263+
): Promise<SyncReport> {
244264
const mode = resolveLifecycle(targetPath);
245265
if (mode !== 'status') {
246266
throw new Error(
@@ -265,6 +285,6 @@ export class ShipCommand {
265285
console.log(` ${changeName}: already shipped`);
266286
}
267287

268-
await new SyncCommand().execute(changeName, targetPath, { json: options.json });
288+
return new SyncCommand().execute(changeName, targetPath, { json: options.json });
269289
}
270290
}

test/core/list.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ describe('ListCommand', () => {
4848
expect(logOutput).toEqual(['No active changes found.']);
4949
});
5050

51+
it('rejects an unknown --status value instead of silently matching nothing', async () => {
52+
const changesDir = path.join(tempDir, 'openspec', 'changes');
53+
await fs.mkdir(changesDir, { recursive: true });
54+
55+
const listCommand = new ListCommand();
56+
57+
await expect(
58+
listCommand.execute(tempDir, 'changes', { status: 'bogus' })
59+
).rejects.toThrow(/Unknown status 'bogus'/);
60+
});
61+
5162
it('should not report a malformed openspec/changes path as empty', async () => {
5263
await fs.mkdir(path.join(tempDir, 'openspec'), { recursive: true });
5364
await fs.writeFile(path.join(tempDir, 'openspec', 'changes'), 'not a directory\n');

test/core/sync.test.ts

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -71,44 +71,72 @@ describe('SyncCommand', () => {
7171

7272
it('reports nothing to gate under lifecycle: archive', async () => {
7373
await scaffold({ lifecycle: 'archive', status: 'shipped' });
74-
await new SyncCommand().execute(undefined, tempDir, { check: true });
75-
expect(process.exitCode).toBeUndefined();
74+
const report = await new SyncCommand().execute(undefined, tempDir, { check: true });
75+
expect(report.clean).toBe(true);
76+
expect(report.mode).toBe('archive');
7677
expect(logs.join('\n')).toContain('lifecycle: archive');
7778
});
7879

7980
it('check fails on a shipped change whose delta is not folded', async () => {
8081
await scaffold({ lifecycle: 'status', status: 'shipped' });
81-
await new SyncCommand().execute(undefined, tempDir, { check: true });
82-
expect(process.exitCode).toBe(1);
82+
const report = await new SyncCommand().execute(undefined, tempDir, { check: true });
83+
expect(report.clean).toBe(false);
8384
expect(logs.join('\n')).toContain('add-oauth');
8485
expect(logs.join('\n')).toContain('auth');
8586
});
8687

8788
it('ignores proposed changes: their deltas stay out of specs/', async () => {
8889
await scaffold({ lifecycle: 'status', status: 'proposed' });
89-
await new SyncCommand().execute(undefined, tempDir, { check: true });
90-
expect(process.exitCode).toBeUndefined();
90+
const report = await new SyncCommand().execute(undefined, tempDir, { check: true });
91+
expect(report.clean).toBe(true);
9192
await expect(fs.access(targetSpec())).rejects.toThrow();
9293
});
9394

9495
it('folds a shipped change, then check passes and a re-run is a no-op', async () => {
9596
await scaffold({ lifecycle: 'status', status: 'shipped' });
9697

97-
await new SyncCommand().execute(undefined, tempDir, {});
98-
expect(process.exitCode).toBeUndefined();
98+
const fold = await new SyncCommand().execute(undefined, tempDir, {});
99+
expect(fold.clean).toBe(true);
99100
const folded = await fs.readFile(targetSpec(), 'utf-8');
100101
expect(folded).toContain('OAuth login');
101102

102-
process.exitCode = undefined;
103103
logs = [];
104-
await new SyncCommand().execute(undefined, tempDir, { check: true });
105-
expect(process.exitCode).toBeUndefined();
104+
const check = await new SyncCommand().execute(undefined, tempDir, { check: true });
105+
expect(check.clean).toBe(true);
106106

107107
await new SyncCommand().execute(undefined, tempDir, {});
108108
const refolded = await fs.readFile(targetSpec(), 'utf-8');
109109
expect(refolded).toBe(folded);
110110
});
111111

112+
it('silent mode emits nothing and still returns the report', async () => {
113+
await scaffold({ lifecycle: 'status', status: 'shipped' });
114+
const report = await new SyncCommand().execute(undefined, tempDir, {
115+
check: true,
116+
silent: true,
117+
});
118+
expect(report.clean).toBe(false);
119+
expect(logs).toEqual([]);
120+
});
121+
122+
it('reports unreadable metadata as the same conflict entry named or swept', async () => {
123+
await scaffold({ lifecycle: 'status', status: 'shipped' });
124+
await fs.writeFile(
125+
path.join(tempDir, 'openspec', 'changes', 'add-oauth', '.openspec.yaml'),
126+
'status: [unclosed\n'
127+
);
128+
129+
const swept = await new SyncCommand().execute(undefined, tempDir, { check: true, silent: true });
130+
const named = await new SyncCommand().execute('add-oauth', tempDir, { check: true, silent: true });
131+
132+
for (const report of [swept, named]) {
133+
expect(report.clean).toBe(false);
134+
expect(report.changes).toHaveLength(1);
135+
expect(report.changes[0].state).toBe('conflict');
136+
expect(report.changes[0].error).toBeTruthy();
137+
}
138+
});
139+
112140
it('refuses to fold an explicitly named change that is not shipped', async () => {
113141
await scaffold({ lifecycle: 'status', status: 'proposed' });
114142
await expect(
@@ -119,8 +147,8 @@ describe('SyncCommand', () => {
119147
it('ship flips status and folds in one step; re-ship is a no-op', async () => {
120148
await scaffold({ lifecycle: 'status', status: 'proposed' });
121149

122-
await new ShipCommand().execute('add-oauth', tempDir, {});
123-
expect(process.exitCode).toBeUndefined();
150+
const shipped = await new ShipCommand().execute('add-oauth', tempDir, {});
151+
expect(shipped.clean).toBe(true);
124152
const metadata = await fs.readFile(
125153
path.join(tempDir, 'openspec', 'changes', 'add-oauth', '.openspec.yaml'),
126154
'utf-8'
@@ -129,9 +157,8 @@ describe('SyncCommand', () => {
129157
const folded = await fs.readFile(targetSpec(), 'utf-8');
130158
expect(folded).toContain('OAuth login');
131159

132-
process.exitCode = undefined;
133-
await new ShipCommand().execute('add-oauth', tempDir, {});
134-
expect(process.exitCode).toBeUndefined();
160+
const reshipped = await new ShipCommand().execute('add-oauth', tempDir, {});
161+
expect(reshipped.clean).toBe(true);
135162
expect(await fs.readFile(targetSpec(), 'utf-8')).toBe(folded);
136163
});
137164

0 commit comments

Comments
 (0)