fix archive scenario drift for #1246

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.

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.

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.

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.
  *

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);