From cc1ac37880a7284b97408e2dc6b611d3d9d46843 Mon Sep 17 00:00:00 2001 From: LQ <1553977725@qq.com> Date: Wed, 24 Jun 2026 23:48:27 +0800 Subject: [PATCH 1/3] fix archive scenario drift for #1246 --- src/core/specs-apply.ts | 49 +++++++++++++++++++++++- test/core/archive.test.ts | 78 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) 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); From 178d2ab4c716ae6ea081acadd0c8cd46d6bb3f8a Mon Sep 17 00:00:00 2001 From: LQ <1553977725@qq.com> Date: Wed, 24 Jun 2026 23:52:06 +0800 Subject: [PATCH 2/3] fix archive scenario drift for #1246 --- .../design.md | 20 +++++++++++++++++++ .../proposal.md | 16 +++++++++++++++ .../tasks.md | 7 +++++++ 3 files changed, 43 insertions(+) create mode 100644 openspec/changes/fix-archive-modified-scenario-drift-1246/design.md create mode 100644 openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md create mode 100644 openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md diff --git a/openspec/changes/fix-archive-modified-scenario-drift-1246/design.md b/openspec/changes/fix-archive-modified-scenario-drift-1246/design.md new file mode 100644 index 000000000..b6663fb3f --- /dev/null +++ b/openspec/changes/fix-archive-modified-scenario-drift-1246/design.md @@ -0,0 +1,20 @@ +# Design + +## Decision + +Use an archive-time conflict gate instead of scenario-level auto-merge. + +## Rationale + +`MODIFIED` currently means a complete requirement block replacement. Automatically merging scenario lists would change that contract and make deliberate scenario removal ambiguous. A conservative guard preserves the existing replacement model while preventing the specific data-loss failure from Issue #1246. + +## Approach + +- Parse `#### Scenario:` headings from the current canonical requirement block and from the incoming modified block. +- During `MODIFIED` application, compare scenario headings by trimmed, case-sensitive name. +- If any current scenario heading is absent from the incoming block, throw a clear error before any spec write happens. +- Continue to prepare all spec updates before writing, preserving existing all-or-nothing behavior. + +## Trade-offs + +This does not solve all stale-base drift cases. It specifically blocks the high-impact scenario deletion case described in Issue #1246. A later base-fingerprint design can extend drift detection for broader body text or requirement removal changes. diff --git a/openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md b/openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md new file mode 100644 index 000000000..e8f155dd0 --- /dev/null +++ b/openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md @@ -0,0 +1,16 @@ +# Fix archive modified scenario drift + +## Why + +Issue #1246 reports that two active changes can both modify the same requirement from the same original base. Archiving the first change updates the canonical spec, but archiving the second change then replaces the whole requirement block and silently removes scenarios added by the first change. + +## What Changes + +- Add a conservative archive-time guard for `MODIFIED` requirement blocks. +- When the current canonical requirement contains scenario headings that the incoming modified block does not contain, abort instead of replacing the block. +- Keep existing whole-requirement replacement behavior when the incoming modified block includes all current scenarios. +- Add a regression test that archives two changes modifying the same requirement sequentially and verifies the second archive does not delete the first change's scenario. + +## Impact + +This prevents silent spec data loss without changing the delta file format. Users must refresh/rebase a stale `MODIFIED` requirement block before archiving if the canonical spec has gained scenarios since the change was authored. diff --git a/openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md b/openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md new file mode 100644 index 000000000..3e64cb611 --- /dev/null +++ b/openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md @@ -0,0 +1,7 @@ +## Tasks + +- [x] Analyze Issue #1246 and archive delta application behavior. +- [x] Add archive-time scenario drift guard for modified requirements. +- [x] Add regression coverage for sequential archives modifying the same requirement. +- [x] Run targeted and project validation. +- [ ] Commit only source and test changes. From c7e3800a65aa1427e34fa761da71152070d02f99 Mon Sep 17 00:00:00 2001 From: LQ <1553977725@qq.com> Date: Fri, 26 Jun 2026 00:02:07 +0800 Subject: [PATCH 3/3] remove local openspec change docs --- .../design.md | 20 ------------------- .../proposal.md | 16 --------------- .../tasks.md | 7 ------- 3 files changed, 43 deletions(-) delete mode 100644 openspec/changes/fix-archive-modified-scenario-drift-1246/design.md delete mode 100644 openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md delete mode 100644 openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md diff --git a/openspec/changes/fix-archive-modified-scenario-drift-1246/design.md b/openspec/changes/fix-archive-modified-scenario-drift-1246/design.md deleted file mode 100644 index b6663fb3f..000000000 --- a/openspec/changes/fix-archive-modified-scenario-drift-1246/design.md +++ /dev/null @@ -1,20 +0,0 @@ -# Design - -## Decision - -Use an archive-time conflict gate instead of scenario-level auto-merge. - -## Rationale - -`MODIFIED` currently means a complete requirement block replacement. Automatically merging scenario lists would change that contract and make deliberate scenario removal ambiguous. A conservative guard preserves the existing replacement model while preventing the specific data-loss failure from Issue #1246. - -## Approach - -- Parse `#### Scenario:` headings from the current canonical requirement block and from the incoming modified block. -- During `MODIFIED` application, compare scenario headings by trimmed, case-sensitive name. -- If any current scenario heading is absent from the incoming block, throw a clear error before any spec write happens. -- Continue to prepare all spec updates before writing, preserving existing all-or-nothing behavior. - -## Trade-offs - -This does not solve all stale-base drift cases. It specifically blocks the high-impact scenario deletion case described in Issue #1246. A later base-fingerprint design can extend drift detection for broader body text or requirement removal changes. diff --git a/openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md b/openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md deleted file mode 100644 index e8f155dd0..000000000 --- a/openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md +++ /dev/null @@ -1,16 +0,0 @@ -# Fix archive modified scenario drift - -## Why - -Issue #1246 reports that two active changes can both modify the same requirement from the same original base. Archiving the first change updates the canonical spec, but archiving the second change then replaces the whole requirement block and silently removes scenarios added by the first change. - -## What Changes - -- Add a conservative archive-time guard for `MODIFIED` requirement blocks. -- When the current canonical requirement contains scenario headings that the incoming modified block does not contain, abort instead of replacing the block. -- Keep existing whole-requirement replacement behavior when the incoming modified block includes all current scenarios. -- Add a regression test that archives two changes modifying the same requirement sequentially and verifies the second archive does not delete the first change's scenario. - -## Impact - -This prevents silent spec data loss without changing the delta file format. Users must refresh/rebase a stale `MODIFIED` requirement block before archiving if the canonical spec has gained scenarios since the change was authored. diff --git a/openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md b/openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md deleted file mode 100644 index 3e64cb611..000000000 --- a/openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md +++ /dev/null @@ -1,7 +0,0 @@ -## Tasks - -- [x] Analyze Issue #1246 and archive delta application behavior. -- [x] Add archive-time scenario drift guard for modified requirements. -- [x] Add regression coverage for sequential archives modifying the same requirement. -- [x] Run targeted and project validation. -- [ ] Commit only source and test changes.