diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 88142ec00..29bc1ed55 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -47,6 +47,11 @@ export interface SpecsApplyOutput { noChanges: boolean; } +interface ScenarioBlock { + name: string; + raw: string; +} + // ----------------------------------------------------------------------------- // Public API // ----------------------------------------------------------------------------- @@ -283,7 +288,8 @@ export async function buildUpdatedSpec( // MODIFIED for (const mod of plan.modified) { const key = normalizeRequirementName(mod.name); - if (!nameToBlock.has(key)) { + const currentBlock = nameToBlock.get(key); + if (!currentBlock) { throw new Error(`${specName} MODIFIED failed for header "### Requirement: ${mod.name}" - not found`); } // Replace block with provided raw (ensure header line matches key) @@ -293,6 +299,12 @@ export async function buildUpdatedSpec( `${specName} MODIFIED failed for header "### Requirement: ${mod.name}" - header mismatch in content` ); } + const missingScenarios = findMissingCurrentScenarios(currentBlock, mod); + if (missingScenarios.length > 0) { + throw new Error( + `${specName} MODIFIED failed for header "### Requirement: ${mod.name}" - current spec contains scenario(s) not present in the modified block: ${missingScenarios.map(name => `"${name}"`).join(', ')}. Refresh the change spec before archiving to avoid dropping scenarios.` + ); + } nameToBlock.set(key, mod); } @@ -376,6 +388,41 @@ export function buildSpecSkeleton(specFolderName: string, changeName: string): s return `# ${titleBase} Specification\n\n## Purpose\nTBD - created by archiving change ${changeName}. Update Purpose after archive.\n\n## Requirements\n`; } +function findMissingCurrentScenarios(current: RequirementBlock, incoming: RequirementBlock): string[] { + const incomingScenarioNames = new Set(parseScenarioBlocks(incoming.raw).map((scenario) => scenario.name)); + return parseScenarioBlocks(current.raw) + .filter((scenario) => !incomingScenarioNames.has(scenario.name)) + .map((scenario) => scenario.name); +} + +function parseScenarioBlocks(requirementRaw: string): ScenarioBlock[] { + const lines = requirementRaw.replace(/\r\n?/g, '\n').split('\n'); + const scenarios: ScenarioBlock[] = []; + let index = 0; + + while (index < lines.length) { + const headerMatch = lines[index].match(/^####\s*Scenario:\s*(.+)\s*$/); + if (!headerMatch) { + index++; + continue; + } + + const start = index; + const name = headerMatch[1].trim(); + index++; + while (index < lines.length && !/^####\s*Scenario:\s*(.+)\s*$/.test(lines[index])) { + index++; + } + + scenarios.push({ + name, + raw: lines.slice(start, index).join('\n').trimEnd(), + }); + } + + return scenarios; +} + /** * Apply all delta specs from a change to main specs. * diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 1d0f75e14..a40d16bbb 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -561,6 +561,84 @@ new text await expect(fs.access(changeDir)).resolves.not.toThrow(); }); + it('should abort stale MODIFIED blocks that would drop current scenarios (issue #1246)', async () => { + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'stale-modified'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecPath = path.join(mainSpecDir, 'spec.md'); + const baseSpec = `# stale-modified Specification + +## Purpose +Stale modified purpose. + +## Requirements + +### Requirement: Shared Rule +The system SHALL support the shared rule. + +#### Scenario: Existing behavior +- **WHEN** the original behavior runs +- **THEN** it succeeds`; + await fs.writeFile(mainSpecPath, baseSpec); + + const changeA = 'modify-shared-a'; + const changeADir = path.join(tempDir, 'openspec', 'changes', changeA); + const changeASpecDir = path.join(changeADir, 'specs', 'stale-modified'); + await fs.mkdir(changeASpecDir, { recursive: true }); + await fs.writeFile(path.join(changeASpecDir, 'spec.md'), `# Stale Modified - Change A + +## MODIFIED Requirements + +### Requirement: Shared Rule +The system SHALL support the shared rule. + +#### Scenario: Existing behavior +- **WHEN** the original behavior runs +- **THEN** it succeeds + +#### Scenario: Behavior from A +- **WHEN** change A behavior runs +- **THEN** it succeeds`); + + const changeB = 'modify-shared-b'; + const changeBDir = path.join(tempDir, 'openspec', 'changes', changeB); + const changeBSpecDir = path.join(changeBDir, 'specs', 'stale-modified'); + await fs.mkdir(changeBSpecDir, { recursive: true }); + await fs.writeFile(path.join(changeBSpecDir, 'spec.md'), `# Stale Modified - Change B + +## MODIFIED Requirements + +### Requirement: Shared Rule +The system SHALL support the shared rule. + +#### Scenario: Existing behavior +- **WHEN** the original behavior runs +- **THEN** it succeeds + +#### Scenario: Behavior from B +- **WHEN** change B behavior runs +- **THEN** it succeeds`); + + await archiveCommand.execute(changeA, { yes: true, noValidate: true }); + await archiveCommand.execute(changeB, { yes: true, noValidate: true }); + + const updated = await fs.readFile(mainSpecPath, 'utf-8'); + expect(updated).toContain('#### Scenario: Existing behavior'); + expect(updated).toContain('#### Scenario: Behavior from A'); + expect(updated).not.toContain('#### Scenario: Behavior from B'); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + 'stale-modified MODIFIED failed for header "### Requirement: Shared Rule" - current spec contains scenario(s) not present in the modified block: "Behavior from A"' + ) + ); + expect(console.log).toHaveBeenCalledWith('Aborted. No files were changed.'); + + await expect(fs.access(changeBDir)).resolves.not.toThrow(); + const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive'); + const archives = await fs.readdir(archiveDir); + expect(archives.some(a => a.includes(changeA))).toBe(true); + expect(archives.some(a => a.includes(changeB))).toBe(false); + }); + it('should abort with a structural error when target spec hides requirements outside ## Requirements', async () => { const changeName = 'hidden-requirement-target'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);