From 63c7f1f57e45885508d9bbdf9d4d9971f417eee5 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 08:43:07 -0500 Subject: [PATCH 1/8] fix(archive): read a wrapped scenario bullet as one bullet A repository that wraps its prose at a column limit writes most scenario bullets over two lines. The retirement guard read the continuation line as content the merge could not account for, so `retire_capabilities` refused every such spec - and because the hint that names the marker is gated on that same count, an unmarked author got the bare "must have at least one requirement" abort and never learned the retirement path exists. A line indented to the content column of the item above it, with no blank line between, is part of that item. It is accounted for when the item was and already reported when it was not, so nothing is deleted unmentioned either way. A blank line still ends the item, so a note written below the scenarios is still the author's own however it is indented. Closes #1780 Co-Authored-By: Claude Opus 5 --- .changeset/wrapped-scenario-bullets-retire.md | 5 + src/core/specs-apply.ts | 43 ++++++- test/core/archive.test.ts | 106 ++++++++++++++++++ 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 .changeset/wrapped-scenario-bullets-retire.md diff --git a/.changeset/wrapped-scenario-bullets-retire.md b/.changeset/wrapped-scenario-bullets-retire.md new file mode 100644 index 0000000000..3344477e18 --- /dev/null +++ b/.changeset/wrapped-scenario-bullets-retire.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Fix `retire_capabilities` refusing any spec whose scenario bullets wrap onto a second line. The continuation line was counted as content the merge could not account for, which blocked the retirement and suppressed the hint that names the marker (#1780). diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 821a7b2ca6..260d3dc66c 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -618,6 +618,17 @@ function firstForeignTail(raw: string): { heading: string; raw: string } | undef return undefined; } +/** + * The column a line's content starts at, tabs expanded to a four-column stop. + * `prefix` is the text that precedes the content: a line's indentation, or a + * list item's indentation together with its marker. + */ +function contentColumn(prefix: string): number { + let column = 0; + for (const char of prefix) column += char === '\t' ? 4 - (column % 4) : 1; + return column; +} + /** * The non-blank lines of a spec that are not part of what a retirement is able * to name: the title, the `## Purpose` section, the `## Requirements` header, @@ -704,20 +715,50 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { // operational note below the last scenario be deleted unmentioned. let inScenarioBullets = false; let bulletsSeen = false; + // The content column of the list item the previous line opened or + // continued, or null when the last line was not part of one. A line + // indented to that column continues the item it sits under (#1780) - a + // repository that wraps its prose at a column limit writes most scenario + // bullets over two lines, and counting the second line as loose content + // made every such capability unretirable. Reset by a blank line, so an + // indented note written below the scenarios is still the author's own. + let listContentIndent: number | null = null; for (let index = 0; index < lines.length; index++) { const line = lines[index]; if (!line.trim()) { // Only a blank that follows actual bullets closes the run, so a blank // between a scenario header and its first bullet is not a boundary. if (bulletsSeen) inScenarioBullets = false; + listContentIndent = null; continue; } if (index === 0) continue; // the `### Requirement:` header itself + const indent = contentColumn(/^[ \t]*/.exec(line)![0]); + const continuesListItem = + listContentIndent !== null && + indent >= listContentIndent && + // A `#` line is a heading wherever it sits, and `firstForeignTail` + // already names it. Left to the checks below rather than absorbed. + !/^ {0,3}#{1,6}(?:[ \t]|$)/.test(line); // Fenced lines render as a code block inside the requirement, so they are // its own content however they are spelled - a `### Requirement:` in an // example is not a heading to any reader. Flagging them made a spec that // merely documents a command unretirable. - if (mask[index]) continue; + if (mask[index]) { + // A fence that starts left of the item's content column has ended it. + if (!continuesListItem) listContentIndent = null; + continue; + } + // A continuation of the list item above: indented to its content column + // with no blank line between. Whatever the item is, this line is part of + // it - accounted for when the item was, and already reported when it was + // not, so nothing is deleted unmentioned either way. + if (continuesListItem) continue; + // Any other line closes the item; a bullet opens the next one. The + // content column is the marker's own indent plus the marker itself, so a + // nested list and its own wrapped lines stay inside the item too. + const bullet = line.match(/^(\s*(?:[-*]|\d+[.)])\s+)\S/); + listContentIndent = bullet ? contentColumn(bullet[1]) : null; if ( index > 1 && /^ {0,3}(?:=+|-+)\s*$/.test(line) && diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index bbdfcf1bed..23435d5772 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -4421,6 +4421,112 @@ The system SHALL do the thing differently. expect(console.log).toHaveBeenCalledWith(expect.stringContaining('escrow keys')); }); + // #1780: a repository that wraps its prose at a column limit writes every + // long scenario bullet over two lines. The continuation line is part of the + // bullet, but it was counted as content the merge could not name - so a + // wrapped spec could not be retired at all, and the same count suppressed + // the hint that told an unmarked author the marker exists. + it('still retires a spec whose scenario bullets wrap onto a second line', async () => { + const changeName = 'retire-wrapped-bullet'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers, wrapped at the', + "repository's column limit like every other paragraph in this file.", + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the outstanding count becomes zero and the completions are recorded', + ' rather than the earned total being reduced', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + expect((await new Validator().validateSpecContent('legacy-layer', spec, 'strict')).valid).toBe( + true + ); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + }); + + it('names the marker for an unmarked change whose scenario bullets wrap', async () => { + // The hint was gated on there being nothing unaccounted for, so a wrapped + // spec got the bare `must have at least one requirement` abort and the + // author never learned the retirement path existed. + const changeName = 'retire-wrapped-bullet-unmarked'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL, { + declareRetirement: false, + }); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the outstanding count becomes zero and the completions are recorded', + ' rather than the earned total being reduced', + '', + ].join('\n') + ); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('add `retire_capabilities: true`') + ); + }); + + it('still refuses a note indented below a blank line after the scenarios', async () => { + // Indentation alone is not continuation: a blank line ends the list item, + // so what follows is the author's own note however it is indented. The + // wrapped-bullet allowance must not swallow it. + const changeName = 'retire-indented-note-after-blank'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + REQUIREMENT, + '', + ' IMPORTANT: escrow keys live in the "legacy" vault; rotate before deleting.', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('escrow keys')); + }); it('still retires a spec whose requirement uses lists and code examples', async () => { // The guard must not refuse ordinary spec prose: a numbered list, a fenced // example, and a statement opening with inline code are all a From e6bdb9aa51cfc89a7c5286a3c8fa7584e1529f67 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 08:45:00 -0500 Subject: [PATCH 2/8] fix(archive): keep an indented heading out of a bullet's continuation Continuation is for wrapped prose. A raw HTML heading indented under a scenario bullet was absorbed by it, so indenting a section one level would have smuggled it past the audit and deleted it with the file. ATX headings were already excluded; HTML ones now are too, matching how the pass above the requirements section reads them. Co-Authored-By: Claude Opus 5 --- src/core/specs-apply.ts | 10 ++++-- test/core/archive.test.ts | 73 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 260d3dc66c..e041a50753 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -737,9 +737,13 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { const continuesListItem = listContentIndent !== null && indent >= listContentIndent && - // A `#` line is a heading wherever it sits, and `firstForeignTail` - // already names it. Left to the checks below rather than absorbed. - !/^ {0,3}#{1,6}(?:[ \t]|$)/.test(line); + // A heading is a heading wherever it sits - an ATX `#` line, which + // `firstForeignTail` already names, or the raw HTML the `before` pass + // treats the same way. Left to the checks below rather than absorbed, + // so indenting a section under a bullet cannot smuggle it past the + // audit. + !/^ {0,3}#{1,6}(?:[ \t]|$)/.test(line) && + !/^\s* { + const changeName = 'retire-wrapped-nested-bullet'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** these happen in order:', + ' 1. the layer loads from the cache written by the previous run, or from disk', + ' when that cache is cold', + '\t2. the consumer proceeds', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + expect((await new Validator().validateSpecContent('legacy-layer', spec, 'strict')).valid).toBe( + true + ); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + }); + + it.each([ + { what: 'an ATX heading', line: ' ### Data Migration Notes' }, + { what: 'a raw HTML heading', line: '

Data Migration Notes

' }, + ])('still refuses $what indented directly under a scenario bullet', async ({ what, line }) => { + // Continuation is for wrapped prose. A heading is a heading wherever it + // sits, so indenting a section under a bullet must not smuggle it past + // the audit and delete it with the file. + const changeName = `retire-indented-heading-${what.split(' ')[1]}`; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + line, + ' Export the escrow table by hand first.', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Data Migration Notes') + ); + }); it('names the marker for an unmarked change whose scenario bullets wrap', async () => { // The hint was gated on there being nothing unaccounted for, so a wrapped // spec got the bare `must have at least one requirement` abort and the From 838e5c4aef0c3e8471c64e6a0e595ae7ed4dbe73 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 08:55:05 -0500 Subject: [PATCH 3/8] fix(archive): flag a setext heading indented under a bullet A setext underline turns the line above it into a heading, so indenting the pair one level under a scenario bullet let a whole section be absorbed as continuation and deleted with the file. Checked ahead of the continuation branch now, the same way the ATX and raw HTML forms already are. Found by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5 --- src/core/specs-apply.ts | 20 ++++++++++++-------- test/core/archive.test.ts | 1 + 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index e041a50753..614a713414 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -753,6 +753,18 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { if (!continuesListItem) listContentIndent = null; continue; } + // Checked ahead of the continuation branch: a setext underline turns the + // line above it into a heading, and indenting the pair under a bullet + // must not absorb them any more than an indented `#` line is absorbed. + if ( + index > 1 && + /^ {0,3}(?:=+|-+)\s*$/.test(line) && + lines[index - 1].trim() + ) { + leftovers.push(lines[index - 1].trim()); + listContentIndent = null; + continue; + } // A continuation of the list item above: indented to its content column // with no blank line between. Whatever the item is, this line is part of // it - accounted for when the item was, and already reported when it was @@ -763,14 +775,6 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { // nested list and its own wrapped lines stay inside the item too. const bullet = line.match(/^(\s*(?:[-*]|\d+[.)])\s+)\S/); listContentIndent = bullet ? contentColumn(bullet[1]) : null; - if ( - index > 1 && - /^ {0,3}(?:=+|-+)\s*$/.test(line) && - lines[index - 1].trim() - ) { - leftovers.push(lines[index - 1].trim()); - continue; - } if (/^ {0,3}####\s+Scenario:/i.test(line)) { seenScenario = true; inScenarioBullets = true; diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index a72cd72c6f..807a7a6e7d 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -4496,6 +4496,7 @@ The system SHALL do the thing differently. it.each([ { what: 'an ATX heading', line: ' ### Data Migration Notes' }, { what: 'a raw HTML heading', line: '

Data Migration Notes

' }, + { what: 'a setext heading', line: ' Data Migration Notes\n --------------------' }, ])('still refuses $what indented directly under a scenario bullet', async ({ what, line }) => { // Continuation is for wrapped prose. A heading is a heading wherever it // sits, so indenting a section under a bullet must not smuggle it past From dd7dbbbefd8fb5e1c49ec0c532566e67753b64bd Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 09:12:02 -0500 Subject: [PATCH 4/8] fix(archive): read an unindented wrapped bullet as one bullet too Not every wrap indents its continuation, and the indent-only rule left the reported bug fixed for one spelling and live for the other: a hand-wrapped scenario bullet still refused the retirement. Inside a scenario's unbroken bullet run a lazy continuation is now read as part of the bullet above it. This widens nothing - a sibling bullet written in that same position is already read as the scenario's own, and a lazy line is part of the bullet where a sibling is merely next to it. Past the blank line that ends the run the indent is still required, so a note bulleted below the scenarios and the line that wraps it stay the author's. Also covers CRLF specs, and asserts the refusal report names only the real leftover in a wrapped multi-requirement spec rather than burying it under continuations. Co-Authored-By: Claude Opus 5 --- src/core/specs-apply.ts | 10 +- test/core/archive.test.ts | 201 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 1 deletion(-) diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 614a713414..4d95724691 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -736,7 +736,15 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { const indent = contentColumn(/^[ \t]*/.exec(line)![0]); const continuesListItem = listContentIndent !== null && - indent >= listContentIndent && + // Indented to the item's content column, or - inside a scenario's + // unbroken bullet run - lazily continued without indenting, which is + // how a hand-wrapped bullet is usually written. Absorbing it widens + // nothing: a sibling bullet written in that same position is already + // read as the scenario's own, and a lazy line is part of the bullet + // above it where a sibling is merely next to it. Outside the run the + // indent is still required, so a note bulleted below the scenarios and + // its own wrapped lines stay the author's. + (indent >= listContentIndent || inScenarioBullets) && // A heading is a heading wherever it sits - an ATX `#` line, which // `firstForeignTail` already names, or the raw HTML the `before` pass // treats the same way. Left to the checks below rather than absorbed, diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 807a7a6e7d..7e5ff9b0f3 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -4459,6 +4459,74 @@ The system SHALL do the thing differently. await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); }); + it('still retires when a scenario bullet wraps without indenting the continuation', async () => { + // Not every wrap indents. A lazy continuation is part of the bullet above + // it the same way an indented one is, and inside a scenario's bullet run + // a sibling bullet written in that position is already read as the + // scenario's own - so reading this line as loose content refused specs + // for a spelling difference. + const changeName = 'retire-lazy-wrapped-bullet'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the outstanding count becomes zero and the completions are recorded', + 'rather than the earned total being reduced', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + expect((await new Validator().validateSpecContent('legacy-layer', spec, 'strict')).valid).toBe( + true + ); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + }); + + it('does not lazily absorb prose below a note bulleted after the scenarios', async () => { + // The lazy allowance is for a scenario's own bullet run. Past the blank + // line that ends it the author's note is the author's, and so is the line + // that wraps it - both must be named rather than deleted with the file. + const changeName = 'retire-lazy-note-after-scenarios'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + REQUIREMENT, + '', + '- IMPORTANT: escrow keys live in the "legacy" vault; rotate them before', + 'anyone deletes this capability.', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('escrow keys')); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('anyone deletes')); + }); it('still retires when a nested list item wraps, and when a tab does the indenting', async () => { const changeName = 'retire-wrapped-nested-bullet'; await createChange(changeName, 'legacy-layer', REMOVE_ALL); @@ -4533,6 +4601,139 @@ The system SHALL do the thing differently. expect.stringContaining('Data Migration Notes') ); }); + it.each([ + { what: 'an ATX heading', body: ['## Data Migration Notes', 'Export the escrow table by hand first.'] }, + { what: 'a setext heading', body: ['Data Migration Notes', '--------------------', 'Export the escrow table by hand first.'] }, + { what: 'a raw HTML heading', body: ['

Data Migration Notes

', 'Export the escrow table by hand first.'] }, + ])('still refuses $what opened with no blank line after the scenario bullets', async ({ what, body }) => { + // The lazy allowance must not reach past a heading. A section opened + // directly under the bullets is a section however tightly it is written, + // and deleting the file would take it. + const changeName = `retire-tight-heading-${what.split(' ')[1]}`; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + ...body, + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Data Migration Notes') + ); + }); + + it('reads a wrapped bullet the same way when the spec uses CRLF line endings', async () => { + const changeName = 'retire-wrapped-bullet-crlf'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the outstanding count becomes zero and the completions are recorded', + ' rather than the earned total being reduced', + '', + ].join('\r\n') + ); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + }); + it('names only the real leftover in a wrapped multi-requirement spec', async () => { + // The report is what an author acts on, so a wrapped spec must not bury + // the one line that matters under a list of its own continuations. + const changeName = 'retire-wrapped-multi'; + const removeBoth = [ + '# Legacy Layer - Changes', + '', + '## REMOVED Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + '**Reason**: The capability is retired.', + '**Migration**: None; consumers already moved off it.', + '', + '### Requirement: The system SHALL report legacy usage', + '**Reason**: The capability is retired.', + '**Migration**: None; consumers already moved off it.', + '', + ].join('\n'); + await createChange(changeName, 'legacy-layer', removeBoth); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers, wrapped at the', + "repository's column limit like every other paragraph in this file.", + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the outstanding count becomes zero and the completions are recorded', + ' rather than the earned total being reduced', + '', + '### Requirement: The system SHALL report legacy usage', + 'The system SHALL report legacy usage to the operator.', + '', + '#### Scenario: Usage is reported', + '- **WHEN** the nightly job runs', + '- **THEN** every consumer still importing the layer is listed in the report', + 'along with the last time it did so', + '', + '- IMPORTANT: escrow keys live in the "legacy" vault; rotate before deleting.', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + const refusal = (console.log as unknown as ReturnType).mock.calls + .map((call) => String(call[0])) + .find((line) => line.includes('cannot safely account for')); + expect(refusal).toContain('escrow keys'); + expect(refusal).not.toContain('column limit'); + expect(refusal).not.toContain('earned total'); + expect(refusal).not.toContain('last time it did so'); + }); it('names the marker for an unmarked change whose scenario bullets wrap', async () => { // The hint was gated on there being nothing unaccounted for, so a wrapped // spec got the bare `must have at least one requirement` abort and the From 26d34f27471debb80b948900bcf524325ff7e022 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 09:27:16 -0500 Subject: [PATCH 5/8] fix(archive): stop a lazy continuation at anything that opens a block CommonMark lets a blockquote, thematic break, table, list item or raw HTML interrupt a paragraph, so one written flush against a scenario bullet starts something new rather than continuing it. The lazy allowance absorbed all of them, which would have deleted an author's note with the file and named nothing. The bullet's paragraph is now tracked as its own state: opened by a bullet, closed by a blank line, a fence, a heading, or a line that opens a block - including one indented inside the item, whose own paragraph ends the bullet's. Lazy continuation applies only while it is open. Indented continuation is unaffected: a nested list or quote sitting inside the item is still the item's own content. Each of the six holes is pinned by a test proven to fail with the narrower rule removed. Found by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5 --- src/core/specs-apply.ts | 73 ++++++++++++++++------- test/core/archive.test.ts | 120 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 21 deletions(-) diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 4d95724691..49695e744f 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -629,6 +629,22 @@ function contentColumn(prefix: string): number { return column; } +/** + * A line that opens a block of its own: a blockquote, a thematic break, a list + * item, a table row, or raw HTML. CommonMark lets each of these interrupt a + * paragraph, so one written flush against a bullet starts something new rather + * than continuing it - and the audit has to name it rather than let it be + * deleted with the file. Headings interrupt too and are checked separately, + * since they are refused however they are indented. + */ +const INTERRUPTS_PARAGRAPH = + /^ {0,3}(?:>|(?:[-*_][ \t]*){3,}$|(?:[-*+]|\d{1,9}[.)])(?:[ \t]|$)|[<|])/; + +/** A heading in any form a spec can write one, ATX or raw HTML. */ +function isHeadingLine(line: string): boolean { + return /^ {0,3}#{1,6}(?:[ \t]|$)/.test(line) || /^\s*= listContentIndent || inScenarioBullets) && - // A heading is a heading wherever it sits - an ATX `#` line, which - // `firstForeignTail` already names, or the raw HTML the `before` pass - // treats the same way. Left to the checks below rather than absorbed, - // so indenting a section under a bullet cannot smuggle it past the - // audit. - !/^ {0,3}#{1,6}(?:[ \t]|$)/.test(line) && - !/^\s*= listContentIndent; + // Not indented at all, but continuing the bullet's own paragraph inside a + // scenario's unbroken bullet run - how a hand-wrapped bullet is usually + // written. Absorbing it widens nothing: a sibling bullet in that same + // position is already read as the scenario's own, and a lazy line is part + // of the bullet above it where a sibling is merely next to it. Outside + // the run the indent is required, so a note bulleted below the scenarios + // and its own wrapped lines stay the author's. + const lazilyContinuesBullet = + paragraphOpen && inScenarioBullets && !INTERRUPTS_PARAGRAPH.test(line); + // A heading is a heading wherever it sits, so neither form absorbs one: + // `firstForeignTail` names the ATX spelling and the `before` pass names + // the raw HTML, and indenting a section under a bullet must not smuggle + // it past the audit. + const continuesListItem = (insideItem || lazilyContinuesBullet) && !isHeadingLine(line); // Fenced lines render as a code block inside the requirement, so they are // its own content however they are spelled - a `### Requirement:` in an // example is not a heading to any reader. Flagging them made a spec that // merely documents a command unretirable. if (mask[index]) { - // A fence that starts left of the item's content column has ended it. - if (!continuesListItem) listContentIndent = null; + // A fence that starts left of the item's content column has ended it, + // and a fence ends the paragraph wherever it sits - so what follows is + // not a lazy continuation of anything. + if (!insideItem) listContentIndent = null; + paragraphOpen = false; continue; } // Checked ahead of the continuation branch: a setext underline turns the @@ -771,18 +794,26 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { ) { leftovers.push(lines[index - 1].trim()); listContentIndent = null; + paragraphOpen = false; continue; } // A continuation of the list item above: indented to its content column // with no blank line between. Whatever the item is, this line is part of // it - accounted for when the item was, and already reported when it was // not, so nothing is deleted unmentioned either way. - if (continuesListItem) continue; + if (continuesListItem) { + // An indented nested list or quote is still inside the item, but it + // ended the bullet's paragraph - so a later unindented line is not + // continuing that paragraph either. + paragraphOpen = !INTERRUPTS_PARAGRAPH.test(line); + continue; + } // Any other line closes the item; a bullet opens the next one. The // content column is the marker's own indent plus the marker itself, so a // nested list and its own wrapped lines stay inside the item too. const bullet = line.match(/^(\s*(?:[-*]|\d+[.)])\s+)\S/); listContentIndent = bullet ? contentColumn(bullet[1]) : null; + paragraphOpen = bullet !== null; if (/^ {0,3}####\s+Scenario:/i.test(line)) { seenScenario = true; inScenarioBullets = true; diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 7e5ff9b0f3..49f16d35f1 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -4734,6 +4734,126 @@ The system SHALL do the thing differently. expect(refusal).not.toContain('earned total'); expect(refusal).not.toContain('last time it did so'); }); + it.each([ + { what: 'a blockquote', body: ['> IMPORTANT: escrow keys live in the "legacy" vault.'], named: 'escrow keys' }, + { what: 'a thematic break', body: ['***', 'IMPORTANT: escrow keys live in the "legacy" vault.'], named: 'escrow keys' }, + { what: 'a table', body: ['| key | vault |', '| --- | ----- |', '| escrow | legacy |'], named: 'escrow' }, + { what: 'a nested list', body: ['- IMPORTANT: escrow keys live in the "legacy" vault.'], named: 'escrow keys' }, + ])('does not lazily absorb $what written flush against the scenario bullets', async ({ what, body, named }) => { + // CommonMark lets each of these interrupt a paragraph, so one written + // with no blank line after a bullet opens something new rather than + // continuing the bullet - and deleting the file would take it. + // + // The nested-list case is the one exception in kind: a sibling bullet in + // that position has always been read as the scenario's own, which is what + // makes the lazy allowance safe. It is here to pin that behavior, not to + // change it. + const changeName = `retire-lazy-block-${what.split(' ')[1]}`; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + ...body, + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + if (what === 'a nested list') { + // Pinned, not asserted as desirable: unchanged from before the lazy + // allowance existed. + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + return; + } + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining(named)); + }); + + it.each([ + { where: 'flush against the bullet', fence: ['```sh', 'openspec archive legacy', '```'] }, + { where: 'indented inside the bullet', fence: [' ```sh', ' openspec archive legacy', ' ```'] }, + ])('does not lazily absorb a note written under a fence $where', async ({ where, fence }) => { + // A fence ends the paragraph wherever it sits, so the line after it is + // not continuing the bullet however tightly it is written. + const changeName = `retire-lazy-after-fence-${where.split(' ')[0]}`; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + ...fence, + 'IMPORTANT: escrow keys live in the "legacy" vault.', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('escrow keys')); + }); + + it('does not lazily absorb a note written under an indented quote in the bullet', async () => { + // The quote is inside the item, so it is not named - but it closed the + // bullet's paragraph, and the unindented line below it is new content. + const changeName = 'retire-lazy-after-indented-quote'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + ' > and the operator is told which consumers are still importing it', + 'IMPORTANT: escrow keys live in the "legacy" vault.', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('escrow keys')); + }); it('names the marker for an unmarked change whose scenario bullets wrap', async () => { // The hint was gated on there being nothing unaccounted for, so a wrapped // spec got the bare `must have at least one requirement` abort and the From b1973b68498413a9762c1000bc6d24066653293a Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 09:56:23 -0500 Subject: [PATCH 6/8] fix(archive): classify a line as the list item sees it A marker as wide as `100. ` puts the item's content past the three columns a Markdown construct is allowed at the file's left margin, so `## Retention` written inside such an item read as five spaces of nothing and was absorbed as continuation - a regression against the behavior before continuation existed, which named it. Every syntax test in the audit now reads the line with the item's indent removed, so a heading, a setext underline or a block start is recognized wherever the item sits. Found by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5 --- src/core/specs-apply.ts | 34 +++++++++++++-- .../openspec/specs/auth/spec.md | 20 +++++++++ .../openspec/specs/payment/spec.md | 7 ++++ test/core/archive.test.ts | 41 +++++++++++++++++++ 4 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 test-spec-command-tmp/openspec/specs/auth/spec.md create mode 100644 test-spec-command-tmp/openspec/specs/payment/spec.md diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 49695e744f..d7bdd3dee3 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -640,6 +640,27 @@ function contentColumn(prefix: string): number { const INTERRUPTS_PARAGRAPH = /^ {0,3}(?:>|(?:[-*_][ \t]*){3,}$|(?:[-*+]|\d{1,9}[.)])(?:[ \t]|$)|[<|])/; +/** + * Drop up to `columns` visual columns of leading whitespace, so a line inside a + * list item is classified by what it is *within* that item. A `## Retention` + * indented under `100. Step` is a heading; measured against the file's left + * margin instead, it reads as five spaces of nothing and was absorbed as + * continuation. A tab straddling the boundary is consumed whole, which can only + * make a line look more like a construct - the direction that refuses. + */ +function dropIndent(line: string, columns: number): string { + let column = 0; + let index = 0; + while (index < line.length && column < columns) { + const char = line[index]; + if (char === ' ') column += 1; + else if (char === '\t') column += 4 - (column % 4); + else break; + index++; + } + return line.slice(index); +} + /** A heading in any form a spec can write one, ATX or raw HTML. */ function isHeadingLine(line: string): boolean { return /^ {0,3}#{1,6}(?:[ \t]|$)/.test(line) || /^\s*= listContentIndent; + // Every syntax test below reads the line as the item sees it. A wide + // marker (`100. `) pushes its content past the three columns Markdown + // constructs are allowed, so measuring from the file's left margin missed + // headings and block starts written inside such an item. + const withinItem = insideItem ? dropIndent(line, listContentIndent!) : line; // Not indented at all, but continuing the bullet's own paragraph inside a // scenario's unbroken bullet run - how a hand-wrapped bullet is usually // written. Absorbing it widens nothing: a sibling bullet in that same @@ -766,12 +792,12 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { // the run the indent is required, so a note bulleted below the scenarios // and its own wrapped lines stay the author's. const lazilyContinuesBullet = - paragraphOpen && inScenarioBullets && !INTERRUPTS_PARAGRAPH.test(line); + paragraphOpen && inScenarioBullets && !INTERRUPTS_PARAGRAPH.test(withinItem); // A heading is a heading wherever it sits, so neither form absorbs one: // `firstForeignTail` names the ATX spelling and the `before` pass names // the raw HTML, and indenting a section under a bullet must not smuggle // it past the audit. - const continuesListItem = (insideItem || lazilyContinuesBullet) && !isHeadingLine(line); + const continuesListItem = (insideItem || lazilyContinuesBullet) && !isHeadingLine(withinItem); // Fenced lines render as a code block inside the requirement, so they are // its own content however they are spelled - a `### Requirement:` in an // example is not a heading to any reader. Flagging them made a spec that @@ -789,7 +815,7 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { // must not absorb them any more than an indented `#` line is absorbed. if ( index > 1 && - /^ {0,3}(?:=+|-+)\s*$/.test(line) && + /^ {0,3}(?:=+|-+)\s*$/.test(withinItem) && lines[index - 1].trim() ) { leftovers.push(lines[index - 1].trim()); @@ -805,7 +831,7 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { // An indented nested list or quote is still inside the item, but it // ended the bullet's paragraph - so a later unindented line is not // continuing that paragraph either. - paragraphOpen = !INTERRUPTS_PARAGRAPH.test(line); + paragraphOpen = !INTERRUPTS_PARAGRAPH.test(withinItem); continue; } // Any other line closes the item; a bullet opens the next one. The diff --git a/test-spec-command-tmp/openspec/specs/auth/spec.md b/test-spec-command-tmp/openspec/specs/auth/spec.md new file mode 100644 index 0000000000..8391ee3434 --- /dev/null +++ b/test-spec-command-tmp/openspec/specs/auth/spec.md @@ -0,0 +1,20 @@ +## Purpose +This is a test specification for the authentication system. + +## Requirements + +### Requirement: User Authentication +The system SHALL provide secure user authentication + +#### Scenario: Successful login +- **GIVEN** a user with valid credentials +- **WHEN** they submit the login form +- **THEN** they are authenticated + +### Requirement: Password Reset +The system SHALL allow users to reset their password + +#### Scenario: Reset via email +- **GIVEN** a user with a registered email +- **WHEN** they request a password reset +- **THEN** they receive a reset link \ No newline at end of file diff --git a/test-spec-command-tmp/openspec/specs/payment/spec.md b/test-spec-command-tmp/openspec/specs/payment/spec.md new file mode 100644 index 0000000000..2097ec7493 --- /dev/null +++ b/test-spec-command-tmp/openspec/specs/payment/spec.md @@ -0,0 +1,7 @@ +## Purpose +This specification defines the payment processing system. + +## Requirements + +### Requirement: Process Payments +The system SHALL process credit card payments securely \ No newline at end of file diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 49f16d35f1..091d2ab533 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -4854,6 +4854,47 @@ The system SHALL do the thing differently. await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); expect(console.log).toHaveBeenCalledWith(expect.stringContaining('escrow keys')); }); + it.each([ + { what: 'an ATX heading', body: [' ## Retention'], named: 'Retention' }, + { what: 'a setext heading', body: [' Retention', ' ---------'], named: 'Retention' }, + { what: 'an unindented note', body: ['IMPORTANT: escrow keys live in the "legacy" vault.'], named: 'escrow keys' }, + ])('still refuses $what written under a wide ordered marker', async ({ what, body, named }) => { + // A marker as wide as `100. ` puts the item's content past the three + // columns a Markdown construct is allowed at the file's left margin, so + // reading these lines against that margin saw five spaces of nothing and + // absorbed them. They are classified as the item sees them - which is + // also what tells the audit that the nested item closed the outer + // bullet's paragraph, so the unindented note below it is not a wrap. + const changeName = `retire-wide-marker-${what.split(' ')[1]}`; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** these happen in order:', + ' 100. the layer loads', + ...body, + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining(named)); + }); it('names the marker for an unmarked change whose scenario bullets wrap', async () => { // The hint was gated on there being nothing unaccounted for, so a wrapped // spec got the bare `must have at least one requirement` abort and the From 23dfb2dc24f514917385babf3b0a447aaaedb1cc Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 10:23:04 -0500 Subject: [PATCH 7/8] chore: drop test scratch directory committed by mistake `test-spec-command-tmp/` is a fixture a test run leaves behind, swept up by `git add -A` in the previous commit. It is not part of the change. Co-Authored-By: Claude Opus 5 --- .../openspec/specs/auth/spec.md | 20 ------------------- .../openspec/specs/payment/spec.md | 7 ------- 2 files changed, 27 deletions(-) delete mode 100644 test-spec-command-tmp/openspec/specs/auth/spec.md delete mode 100644 test-spec-command-tmp/openspec/specs/payment/spec.md diff --git a/test-spec-command-tmp/openspec/specs/auth/spec.md b/test-spec-command-tmp/openspec/specs/auth/spec.md deleted file mode 100644 index 8391ee3434..0000000000 --- a/test-spec-command-tmp/openspec/specs/auth/spec.md +++ /dev/null @@ -1,20 +0,0 @@ -## Purpose -This is a test specification for the authentication system. - -## Requirements - -### Requirement: User Authentication -The system SHALL provide secure user authentication - -#### Scenario: Successful login -- **GIVEN** a user with valid credentials -- **WHEN** they submit the login form -- **THEN** they are authenticated - -### Requirement: Password Reset -The system SHALL allow users to reset their password - -#### Scenario: Reset via email -- **GIVEN** a user with a registered email -- **WHEN** they request a password reset -- **THEN** they receive a reset link \ No newline at end of file diff --git a/test-spec-command-tmp/openspec/specs/payment/spec.md b/test-spec-command-tmp/openspec/specs/payment/spec.md deleted file mode 100644 index 2097ec7493..0000000000 --- a/test-spec-command-tmp/openspec/specs/payment/spec.md +++ /dev/null @@ -1,7 +0,0 @@ -## Purpose -This specification defines the payment processing system. - -## Requirements - -### Requirement: Process Payments -The system SHALL process credit card payments securely \ No newline at end of file From 515cb10e47c29b9624cb74b2724670c9d903e546 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Mon, 7 Sep 2026 11:55:49 -0500 Subject: [PATCH 8/8] fix(archive): share one list-marker definition with the paragraph rule Folds in the marker coverage from the duplicate PR #1789, which fixes the same issue (#1780) with a shallower model. The audit named `-`, `*` and ordered items as list markers, while INTERRUPTS_PARAGRAPH, added in this same PR, already named `+` and capped an ordered marker at CommonMark's nine digits. The two disagreed, so a line one called a bullet and the other did not was read as both at once. Both now use one LIST_ITEM constant: - `+` is the behavior fix. A spec bulleted with `+` validates like any other, and every one of its scenario bullets was reported as unaccounted content, so that capability could not be retired at all. Regression added, verified to fail against the old marker set. - The nine-digit cap changes no verdict in this design, since a line the pattern rejects is weighed by the same rules either way. It is here for the consistency, and the comment says so rather than claiming a fix. The case is pinned so a later change cannot start deleting such a note. LIST_ITEM also no longer requires content after the marker, so an empty `- ` reads as the bullet it is instead of falling through to the leftovers, which is what the surrounding indent tracking already assumed. Co-Authored-By: Claude Opus 5 --- .changeset/wrapped-scenario-bullets-retire.md | 2 +- src/core/specs-apply.ts | 28 ++++++- test/core/archive.test.ts | 75 +++++++++++++++++++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/.changeset/wrapped-scenario-bullets-retire.md b/.changeset/wrapped-scenario-bullets-retire.md index 3344477e18..cf12469f48 100644 --- a/.changeset/wrapped-scenario-bullets-retire.md +++ b/.changeset/wrapped-scenario-bullets-retire.md @@ -2,4 +2,4 @@ "@fission-ai/openspec": patch --- -Fix `retire_capabilities` refusing any spec whose scenario bullets wrap onto a second line. The continuation line was counted as content the merge could not account for, which blocked the retirement and suppressed the hint that names the marker (#1780). +Fix `retire_capabilities` refusing any spec whose scenario bullets wrap onto a second line. The continuation line was counted as content the merge could not account for, which blocked the retirement and suppressed the hint that names the marker (#1780). A spec bulleted with `+` is covered too: naming only `-` and `*` as list markers reported every one of its scenario bullets as unaccounted content, so that capability could not be retired at all either. diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index d7bdd3dee3..b2bc156258 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -640,6 +640,30 @@ function contentColumn(prefix: string): number { const INTERRUPTS_PARAGRAPH = /^ {0,3}(?:>|(?:[-*_][ \t]*){3,}$|(?:[-*+]|\d{1,9}[.)])(?:[ \t]|$)|[<|])/; +/** + * A list item, spelled the way CommonMark spells one, with its marker and the + * space after it captured so a caller can measure the item's content column. + * + * Every marker, and only those. `+` is a list marker like `-` and `*`: a spec + * bulleted that way validates like any other, and naming only two of the three + * made every one of its scenario bullets unaccounted content, so such a + * capability could not be retired at all. + * + * The nine-digit cap is the other half of "only those": CommonMark stops an + * ordered marker at nine digits, so `1234567890.` opens a paragraph, not a + * list. It changes no verdict here, because a line this pattern rejects is + * weighed by the same rules either way; it is here so the audit and + * INTERRUPTS_PARAGRAPH cannot disagree about what a marker is. A line one of + * them calls a bullet and the other does not is read as both at once, and that + * disagreement is what a shared definition removes. + * + * Content after the marker is not required, so an empty `- ` still reads as + * the bullet it is rather than falling through to the leftovers. The captured + * group is the indent plus the marker plus its trailing space, which is the + * item's content column. + */ +const LIST_ITEM = /^(\s*(?:[-*+]|\d{1,9}[.)])\s+)/; + /** * Drop up to `columns` visual columns of leading whitespace, so a line inside a * list item is classified by what it is *within* that item. A `## Retention` @@ -837,7 +861,7 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { // Any other line closes the item; a bullet opens the next one. The // content column is the marker's own indent plus the marker itself, so a // nested list and its own wrapped lines stay inside the item too. - const bullet = line.match(/^(\s*(?:[-*]|\d+[.)])\s+)\S/); + const bullet = line.match(LIST_ITEM); listContentIndent = bullet ? contentColumn(bullet[1]) : null; paragraphOpen = bullet !== null; if (/^ {0,3}####\s+Scenario:/i.test(line)) { @@ -846,7 +870,7 @@ function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { bulletsSeen = false; continue; } - if (/^\s*(?:[-*]|\d+[.)])\s/.test(line)) { + if (bullet) { if (inScenarioBullets) { bulletsSeen = true; continue; diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 091d2ab533..1033fa1868 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -4459,6 +4459,81 @@ The system SHALL do the thing differently. await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); }); + it('still retires a spec whose scenarios are bulleted with +', async () => { + // `+` is a list marker like `-` and `*`. Naming only two of the three + // made every bullet in such a spec unaccounted content, so the + // capability could not be retired at all - and `openspec validate + // --specs` passes the file without a word, so nothing said why. + const changeName = 'retire-plus-bulleted'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '+ **WHEN** a consumer imports the layer', + '+ **THEN** the layer resolves, wrapped at the column limit like every other', + ' paragraph in this file', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + expect((await new Validator().validateSpecContent('legacy-layer', spec, 'strict')).valid).toBe( + true + ); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + }); + + it('refuses an authored note that opens with a number too long to be a marker', async () => { + // `1234567890.` is past CommonMark's nine-digit cap, so it opens a + // paragraph rather than a list. Either reading refuses this note, since a + // bullet below the scenarios is the author's own too; the case is pinned + // so the shared marker definition cannot start deleting it. + const changeName = 'retire-long-number-note'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const spec = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the layer resolves', + '', + '1234567890. Migration note: keep the escrow keys until the audit closes.', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), spec); + expect((await new Validator().validateSpecContent('legacy-layer', spec, 'strict')).valid).toBe( + true + ); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(spec); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('escrow keys')); + }); + it('still retires when a scenario bullet wraps without indenting the continuation', async () => { // Not every wrap indents. A lazy continuation is part of the bullet above // it the same way an indented one is, and inside a scenario's bullet run