diff --git a/.changeset/wrapped-scenario-bullets-retire.md b/.changeset/wrapped-scenario-bullets-retire.md new file mode 100644 index 0000000000..cf12469f48 --- /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). 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 821a7b2ca6..b2bc156258 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -618,6 +618,78 @@ 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; +} + +/** + * 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 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` + * 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 + // 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(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(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 // 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, + // 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 + // 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) && + /^ {0,3}(?:=+|-+)\s*$/.test(withinItem) && lines[index - 1].trim() ) { 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) { + // 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(withinItem); 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(LIST_ITEM); + listContentIndent = bullet ? contentColumn(bullet[1]) : null; + paragraphOpen = bullet !== null; if (/^ {0,3}####\s+Scenario:/i.test(line)) { seenScenario = true; inScenarioBullets = true; 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 bbdfcf1bed..1033fa1868 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -4421,6 +4421,623 @@ 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('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 + // 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); + 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

' }, + { 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 + // 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.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.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.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 + // 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