Skip to content

Commit 3ceef2d

Browse files
authored
fix(archive): allow REMOVED requirements when creating new spec files (#403) (#404)
When creating a new spec file, REMOVED requirements are now ignored with a warning instead of causing archive to fail. This enables refactoring scenarios where old fields are removed while documenting a capability for the first time. Fixes #403
1 parent 2c2599b commit 3ceef2d

File tree

2 files changed

+150
-7
lines changed

2 files changed

+150
-7
lines changed

src/core/archive.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { promises as fs } from 'fs';
22
import path from 'path';
3-
import { FileSystemUtils } from '../utils/file-system.js';
43
import { getTaskProgressForChange, formatTaskStatus } from '../utils/task-progress.js';
54
import { Validator } from './validation/validator.js';
65
import chalk from 'chalk';
@@ -447,15 +446,26 @@ export class ArchiveCommand {
447446

448447
// Load or create base target content
449448
let targetContent: string;
449+
let isNewSpec = false;
450450
try {
451451
targetContent = await fs.readFile(update.target, 'utf-8');
452452
} catch {
453-
// Target spec does not exist; only ADDED operations are permitted
454-
if (plan.modified.length > 0 || plan.removed.length > 0 || plan.renamed.length > 0) {
453+
// Target spec does not exist; MODIFIED and RENAMED are not allowed for new specs
454+
// REMOVED will be ignored with a warning since there's nothing to remove
455+
if (plan.modified.length > 0 || plan.renamed.length > 0) {
455456
throw new Error(
456-
`${specName}: target spec does not exist; only ADDED requirements are allowed for new specs.`
457+
`${specName}: target spec does not exist; only ADDED requirements are allowed for new specs. MODIFIED and RENAMED operations require an existing spec.`
457458
);
458459
}
460+
// Warn about REMOVED requirements being ignored for new specs
461+
if (plan.removed.length > 0) {
462+
console.log(
463+
chalk.yellow(
464+
`⚠️ Warning: ${specName} - ${plan.removed.length} REMOVED requirement(s) ignored for new spec (nothing to remove).`
465+
)
466+
);
467+
}
468+
isNewSpec = true;
459469
targetContent = this.buildSpecSkeleton(specName, changeName);
460470
}
461471

@@ -498,9 +508,15 @@ export class ArchiveCommand {
498508
for (const name of plan.removed) {
499509
const key = normalizeRequirementName(name);
500510
if (!nameToBlock.has(key)) {
501-
throw new Error(
502-
`${specName} REMOVED failed for header "### Requirement: ${name}" - not found`
503-
);
511+
// For new specs, REMOVED requirements are already warned about and ignored
512+
// For existing specs, missing requirements are an error
513+
if (!isNewSpec) {
514+
throw new Error(
515+
`${specName} REMOVED failed for header "### Requirement: ${name}" - not found`
516+
);
517+
}
518+
// Skip removal for new specs (already warned above)
519+
continue;
504520
}
505521
nameToBlock.delete(key);
506522
}

test/core/archive.test.ts

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,133 @@ Then expected result happens`;
127127
expect(updatedContent).toContain('#### Scenario: Basic test');
128128
});
129129

130+
it('should allow REMOVED requirements when creating new spec file (issue #403)', async () => {
131+
const changeName = 'new-spec-with-removed';
132+
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
133+
const changeSpecDir = path.join(changeDir, 'specs', 'gift-card');
134+
await fs.mkdir(changeSpecDir, { recursive: true });
135+
136+
// Create delta spec with both ADDED and REMOVED requirements
137+
// This simulates refactoring where old fields are removed and new ones are added
138+
const specContent = `# Gift Card - Changes
139+
140+
## ADDED Requirements
141+
142+
### Requirement: Logo and Background Color
143+
The system SHALL support logo and backgroundColor fields for gift cards.
144+
145+
#### Scenario: Display gift card with logo
146+
- **WHEN** a gift card is displayed
147+
- **THEN** it shows the logo and backgroundColor
148+
149+
## REMOVED Requirements
150+
151+
### Requirement: Image Field
152+
### Requirement: Thumbnail Field`;
153+
await fs.writeFile(path.join(changeSpecDir, 'spec.md'), specContent);
154+
155+
// Execute archive - should succeed with warning about REMOVED requirements
156+
await archiveCommand.execute(changeName, { yes: true, noValidate: true });
157+
158+
// Verify warning was logged about REMOVED requirements being ignored
159+
expect(console.log).toHaveBeenCalledWith(
160+
expect.stringContaining('Warning: gift-card - 2 REMOVED requirement(s) ignored for new spec (nothing to remove).')
161+
);
162+
163+
// Verify spec was created with only ADDED requirements
164+
const mainSpecPath = path.join(tempDir, 'openspec', 'specs', 'gift-card', 'spec.md');
165+
const updatedContent = await fs.readFile(mainSpecPath, 'utf-8');
166+
expect(updatedContent).toContain('# gift-card Specification');
167+
expect(updatedContent).toContain('### Requirement: Logo and Background Color');
168+
expect(updatedContent).toContain('#### Scenario: Display gift card with logo');
169+
// REMOVED requirements should not be in the final spec
170+
expect(updatedContent).not.toContain('### Requirement: Image Field');
171+
expect(updatedContent).not.toContain('### Requirement: Thumbnail Field');
172+
173+
// Verify change was archived successfully
174+
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
175+
const archives = await fs.readdir(archiveDir);
176+
expect(archives.length).toBeGreaterThan(0);
177+
expect(archives.some(a => a.includes(changeName))).toBe(true);
178+
});
179+
180+
it('should still error on MODIFIED when creating new spec file', async () => {
181+
const changeName = 'new-spec-with-modified';
182+
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
183+
const changeSpecDir = path.join(changeDir, 'specs', 'new-capability');
184+
await fs.mkdir(changeSpecDir, { recursive: true });
185+
186+
// Create delta spec with MODIFIED requirement (should fail for new spec)
187+
const specContent = `# New Capability - Changes
188+
189+
## ADDED Requirements
190+
191+
### Requirement: New Feature
192+
New feature description.
193+
194+
## MODIFIED Requirements
195+
196+
### Requirement: Existing Feature
197+
Modified content.`;
198+
await fs.writeFile(path.join(changeSpecDir, 'spec.md'), specContent);
199+
200+
// Execute archive - should abort with error message (not throw, but log and return)
201+
await archiveCommand.execute(changeName, { yes: true, noValidate: true });
202+
203+
// Verify error message mentions MODIFIED not allowed for new specs
204+
expect(console.log).toHaveBeenCalledWith(
205+
expect.stringContaining('new-capability: target spec does not exist; only ADDED requirements are allowed for new specs. MODIFIED and RENAMED operations require an existing spec.')
206+
);
207+
expect(console.log).toHaveBeenCalledWith('Aborted. No files were changed.');
208+
209+
// Verify spec was NOT created
210+
const mainSpecPath = path.join(tempDir, 'openspec', 'specs', 'new-capability', 'spec.md');
211+
await expect(fs.access(mainSpecPath)).rejects.toThrow();
212+
213+
// Verify change was NOT archived
214+
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
215+
const archives = await fs.readdir(archiveDir);
216+
expect(archives.some(a => a.includes(changeName))).toBe(false);
217+
});
218+
219+
it('should still error on RENAMED when creating new spec file', async () => {
220+
const changeName = 'new-spec-with-renamed';
221+
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
222+
const changeSpecDir = path.join(changeDir, 'specs', 'another-capability');
223+
await fs.mkdir(changeSpecDir, { recursive: true });
224+
225+
// Create delta spec with RENAMED requirement (should fail for new spec)
226+
const specContent = `# Another Capability - Changes
227+
228+
## ADDED Requirements
229+
230+
### Requirement: New Feature
231+
New feature description.
232+
233+
## RENAMED Requirements
234+
- FROM: \`### Requirement: Old Name\`
235+
- TO: \`### Requirement: New Name\``;
236+
await fs.writeFile(path.join(changeSpecDir, 'spec.md'), specContent);
237+
238+
// Execute archive - should abort with error message (not throw, but log and return)
239+
await archiveCommand.execute(changeName, { yes: true, noValidate: true });
240+
241+
// Verify error message mentions RENAMED not allowed for new specs
242+
expect(console.log).toHaveBeenCalledWith(
243+
expect.stringContaining('another-capability: target spec does not exist; only ADDED requirements are allowed for new specs. MODIFIED and RENAMED operations require an existing spec.')
244+
);
245+
expect(console.log).toHaveBeenCalledWith('Aborted. No files were changed.');
246+
247+
// Verify spec was NOT created
248+
const mainSpecPath = path.join(tempDir, 'openspec', 'specs', 'another-capability', 'spec.md');
249+
await expect(fs.access(mainSpecPath)).rejects.toThrow();
250+
251+
// Verify change was NOT archived
252+
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
253+
const archives = await fs.readdir(archiveDir);
254+
expect(archives.some(a => a.includes(changeName))).toBe(false);
255+
});
256+
130257
it('should throw error if change does not exist', async () => {
131258
await expect(
132259
archiveCommand.execute('non-existent-change', { yes: true })

0 commit comments

Comments
 (0)