Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions .changeset/archive-phantom-proposal-warnings.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fission-ai/openspec": patch
---

Fix `openspec archive` reporting phantom proposal warnings that `openspec validate` never showed. Requirement-level issues coming from the delta specs are no longer repeated in the "Proposal warnings in proposal.md" block, so a heading such as `### Documentation Requirements` inside a delta section no longer produces a scenario warning against a requirement that does not exist. Genuine proposal-level warnings, and blocking delta spec errors, are unchanged.
8 changes: 8 additions & 0 deletions openspec/specs/cli-archive/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,14 @@ The archive command SHALL validate changes before applying them to ensure data i
- **AND** only proceed if validation passes
- **AND** show validation errors if it fails

#### Scenario: Proposal warnings stay proposal-level

- **WHEN** archiving a change
- **THEN** the non-blocking proposal warnings SHALL report proposal-level issues only
- **AND** SHALL NOT repeat requirement-level issues reached through the delta specs
- **AND** requirement-level issues in delta specs SHALL be left to delta spec
validation, which blocks the archive when they are errors

#### Scenario: Force archive without validation

- **WHEN** executing `openspec archive change-name --no-validate`
Expand Down
18 changes: 15 additions & 3 deletions src/core/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,22 @@ export class ArchiveCommand {
try {
await fs.access(changeFile);
const changeReport = await validator.validateChange(changeFile);
// Proposal validation is informative only (do not block archive)
if (!changeReport.valid) {
// Proposal validation is informative only (do not block archive).
// Report proposal-level issues only. Requirement-level issues reached
// through `deltas.<n>.requirement(s)` belong to the delta specs, and
// the delta report below already reports them, naming the delta
// operation and requirement. Repeating them here was noisy and
// misleading (#498): the change parser records every requirement
// under both `requirement` and `requirements`, so a missing scenario
// was reported twice here and once by the delta report, and a stray
// non-`### Requirement:` header in a delta section surfaced as a
// phantom scenario warning against a requirement that does not exist.
const proposalIssues = changeReport.issues.filter(
(issue) => !/^deltas\.\d+\.requirements?\./.test(issue.path)
);
if (!changeReport.valid && proposalIssues.length > 0) {
console.log(chalk.yellow(`\nProposal warnings in proposal.md (non-blocking):`));
for (const issue of changeReport.issues) {
for (const issue of proposalIssues) {
const symbol = issue.level === 'ERROR' ? '⚠' : (issue.level === 'WARNING' ? '⚠' : 'ℹ');
console.log(chalk.yellow(` ${symbol} ${issue.message}`));
}
Expand Down
146 changes: 146 additions & 0 deletions test/core/archive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1548,4 +1548,150 @@ The system SHALL do the thing differently.
await expect(fs.access(changeDir)).resolves.not.toThrow();
});
});

describe('proposal warnings (#498)', () => {
const LONG_WHY =
'This change exists to document AI application patterns thoroughly for the team, which is long enough.';

async function createChange(
changeName: string,
why: string,
deltaSpec: string
): Promise<string> {
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
await fs.mkdir(path.join(changeDir, 'specs', 'docs'), { recursive: true });
await fs.writeFile(
path.join(changeDir, 'proposal.md'),
`# Proposal\n\n## Why\n${why}\n\n## What Changes\n- Add docs.\n`
);
await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task 1\n');
await fs.writeFile(path.join(changeDir, 'specs', 'docs', 'spec.md'), deltaSpec);
return changeDir;
}

function loggedLines(): string[] {
return (console.log as unknown as ReturnType<typeof vi.fn>).mock.calls.map(
(call) => String(call[0])
);
}

// A stray non-`### Requirement:` header inside a delta section used to be
// parsed as a requirement, so archive blamed a requirement that does not
// exist while `openspec validate` reported the change as valid (#498).
it('does not report phantom requirement warnings for a stray delta header', async () => {
const changeName = 'stray-header';
await createChange(
changeName,
LONG_WHY,
[
'# Docs Delta',
'',
'## ADDED Requirements',
'',
'### Documentation Requirements',
'',
'### Requirement: AI Application Documentation',
'Teams building AI applications SHALL document agent definitions.',
'',
'#### Scenario: Agent Definition Documentation',
'- **WHEN** a team ships an agent',
'- **THEN** the agent definition is documented',
'',
].join('\n')
);

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

const output = loggedLines().join('\n');
expect(output).not.toContain('Proposal warnings in proposal.md');
expect(output).not.toContain('Requirement must have at least one scenario');

// The change still archives, exactly as `validate` predicted.
const archives = await fs.readdir(path.join(tempDir, 'openspec', 'changes', 'archive'));
expect(archives).toEqual([expect.stringMatching(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`))]);
});

it('still reports genuine proposal-level warnings', async () => {
const changeName = 'short-why';
await createChange(
changeName,
'Short.',
[
'# Docs Delta',
'',
'## ADDED Requirements',
'',
'### Requirement: Real Requirement',
'The system SHALL do a thing.',
'',
'#### Scenario: It works',
'- **WHEN** invoked',
'- **THEN** it works',
'',
].join('\n')
);

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

const output = loggedLines().join('\n');
expect(output).toContain('Proposal warnings in proposal.md');
expect(output).toContain('Why section must be at least 50 characters');
});

// The filter is anchored to the dot-joined Zod paths
// (`deltas.<n>.requirement(s).…`). Rules in applyChangeRules use bracket
// notation (`deltas[<n>].description`) and describe simple deltas parsed
// from `## What Changes`, which are proposal-level. They must survive.
it('keeps proposal-level warnings about simple deltas from What Changes', async () => {
const changeName = 'simple-deltas';
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
await fs.mkdir(changeDir, { recursive: true });
await fs.writeFile(
path.join(changeDir, 'proposal.md'),
'# Proposal\n\n## Why\nShort.\n\n## What Changes\n- **docs:** add x\n'
);
await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task 1\n');

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

const output = loggedLines().join('\n');
expect(output).toContain('Proposal warnings in proposal.md');
expect(output).toContain('Delta description is too brief');
expect(output).toContain('ADDED Delta should include requirements');
});

// Real delta defects are still caught. A missing scenario used to be
// reported three times (twice as proposal warnings, once by the delta
// report) and is now reported once, by the delta report.
it('still blocks the archive on real delta requirement errors, reported once', async () => {
const changeName = 'bad-delta';
const changeDir = await createChange(
changeName,
LONG_WHY,
[
'# Docs Delta',
'',
'## ADDED Requirements',
'',
'### Requirement: Missing Scenario',
'The system SHALL do a thing.',
'',
].join('\n')
);

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

const lines = loggedLines();
const output = lines.join('\n');
expect(output).toContain('Validation errors in change delta specs');
expect(output).toContain('must include at least one scenario');
expect(output).not.toContain('Proposal warnings in proposal.md');
expect(
lines.filter((line) => line.includes('must include at least one scenario'))
).toHaveLength(1);

// The change was not archived.
await expect(fs.access(changeDir)).resolves.not.toThrow();
});
});
});
Loading