Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/core/specs-apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,27 @@ export function buildSpecSkeleton(specFolderName: string, changeName: string): s
}

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);
// Multiplicity-aware: a name present N times in current and M times in
// incoming means max(0, N - M) instances are missing. Set membership would
// treat N>M as fully covered and let archive silently drop duplicates
// (residual #1246 / duplicate-scenario-name blind spot).
const remainingIncoming = new Map<string, number>();
for (const scenario of parseScenarioBlocks(incoming.raw)) {
const name = scenario.name;
remainingIncoming.set(name, (remainingIncoming.get(name) ?? 0) + 1);
}

const missing: string[] = [];
for (const scenario of parseScenarioBlocks(current.raw)) {
const name = scenario.name;
const remaining = remainingIncoming.get(name) ?? 0;
if (remaining > 0) {
remainingIncoming.set(name, remaining - 1);
} else {
missing.push(name);
}
}
return missing;
}

function parseScenarioBlocks(requirementRaw: string): ScenarioBlock[] {
Expand Down
65 changes: 65 additions & 0 deletions test/core/archive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,71 @@ The system SHALL support the shared rule.
expect(archives.some(a => a.includes(changeB))).toBe(false);
});

it('should abort MODIFIED that drops a duplicate-named scenario (issue #1246 multiplicity)', async () => {
// Residual blind spot after the original #1246 gate: findMissingCurrentScenarios
// used Set membership, so two current scenarios sharing a name were both
// considered "present" when the MODIFIED block kept only one of them.
const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'dup-scenario');
await fs.mkdir(mainSpecDir, { recursive: true });
const mainSpecPath = path.join(mainSpecDir, 'spec.md');
await fs.writeFile(
mainSpecPath,
`# dup-scenario Specification

## Purpose
Duplicate scenario names within one requirement.

## Requirements

### Requirement: Login
The system SHALL authenticate.

#### Scenario: Validate
- **WHEN** input is empty
- **THEN** reject

#### Scenario: Validate
- **WHEN** input is malformed
- **THEN** reject`
);

const changeName = 'drop-one-validate';
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
const changeSpecDir = path.join(changeDir, 'specs', 'dup-scenario');
await fs.mkdir(changeSpecDir, { recursive: true });
await fs.writeFile(
path.join(changeSpecDir, 'spec.md'),
`# Drop One Validate - Change

## MODIFIED Requirements

### Requirement: Login
The system SHALL authenticate.

#### Scenario: Validate
- **WHEN** input is empty
- **THEN** reject`
);

await archiveCommand.execute(changeName, { yes: true, noValidate: true });

const updated = await fs.readFile(mainSpecPath, 'utf-8');
// Spec must be untouched — both Validate scenarios preserved
expect((updated.match(/#### Scenario: Validate/g) || []).length).toBe(2);
expect(updated).toContain('malformed');
expect(console.log).toHaveBeenCalledWith(
expect.stringContaining(
'dup-scenario MODIFIED failed for header "### Requirement: Login" - current spec contains scenario(s) not present in the modified block: "Validate"'
)
);
expect(console.log).toHaveBeenCalledWith('Aborted. No files were changed.');

await expect(fs.access(changeDir)).resolves.not.toThrow();
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives.some(a => a.includes(changeName))).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);
Expand Down
Loading