-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix(archive): account for wrapped scenario bullets when retiring a capability #1789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
4026f4d
fix(archive): account for wrapped scenario bullets when retiring a ca…
clay-good 02ef646
test(archive): pin the lazy-continuation edge of the wrapped-bullet rule
clay-good 55b0cb6
fix(archive): read every list marker when accounting for a spec's con…
clay-good 6e09bc6
test(archive): pin the blank-line-split scenario, and sweep every rea…
clay-good a8be7ac
fix(archive): stop an ordered list marker where CommonMark stops it
clay-good File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "@fission-ai/openspec": patch | ||
| --- | ||
|
|
||
| ### Bug Fixes | ||
|
|
||
| - **`retire_capabilities` on specs that wrap their bullets** — Archive refused to retire a capability whenever a scenario bullet wrapped onto a second line: the wrapped remainder was counted as content the merge could not account for. Projects that hold their Markdown to a column limit could not retire any capability through the supported path. A line that continues the bullet above it is now accounted for with that bullet, while a heading, fence, block quote, thematic break, table row, or raw HTML written beneath a bullet is still weighed on its own and still blocks the deletion. | ||
| - **`retire_capabilities` on specs bulleted with `+`** — Scenario bullets written with the `+` list marker were read as unaccounted content, so a capability written that way could not be retired at all, even though such a spec validates cleanly. Every CommonMark list marker is now recognised. | ||
| - **A note mistaken for an ordered list item** — A line opening with a ten-or-more-digit number and a period was read as a list marker, which CommonMark does not allow, so the same authored note was refused when it began with a word and deleted without mention when it began with a long enough number. Ordered markers now stop at nine digits, where CommonMark stops them. |
2 changes: 2 additions & 0 deletions
2
openspec/changes/fix-wrapped-scenario-bullet-retirement/.openspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-09-04 |
69 changes: 69 additions & 0 deletions
69
openspec/changes/fix-wrapped-scenario-bullet-retirement/proposal.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| ## Why | ||
|
|
||
| `retire_capabilities: true` — the retirement path added for #1302 — is unusable on | ||
| any spec whose scenario bullets wrap onto a second line. | ||
|
|
||
| Before deleting a main spec, archive accounts for every non-blank line in the | ||
| file: if anything sits outside the parts the merge understands, the retirement is | ||
| refused rather than taking authored content with it. That audit reads the file a | ||
| line at a time, and a wrapped bullet's remainder is a line that is not a bullet, | ||
| not a heading and not a requirement statement — so it counts as content the merge | ||
| cannot name, and the retirement is refused. | ||
|
|
||
| Wrapping prose at a column limit is the norm in a repository that lints for it. | ||
| [#1780](https://github.com/Fission-AI/OpenSpec/issues/1780) reports a project held | ||
| to 100 columns, where effectively every scenario bullet longer than a short | ||
| sentence wraps, and therefore no capability in the project can be retired through | ||
| the supported path. The reporter hand-deleted the spec directory instead. | ||
|
|
||
| The same classification also decided whether the abort could name the marker at | ||
| all. That half is already fixed on `main` (#1696 / PR #1699): a spec with | ||
| unaccounted content now gets a hint that names the blocking lines instead of a | ||
| bare `Spec must have at least one requirement`. What is left is the | ||
| classification itself. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - A line inside a requirement block that continues the list item above it is | ||
| accounted for as part of that item, not as a line of its own. Nothing may have | ||
| closed the item first — no blank line, heading, fence, or new bullet — which is | ||
| CommonMark's rule for a paragraph running on inside a list item, and covers both | ||
| an indented remainder and a lazy (unindented) one. | ||
| - Lines that open a block of their own are excluded, so they are still weighed | ||
| individually: a heading, a fence, a block quote, a thematic break, a table row, | ||
| or raw HTML written directly beneath a bullet. | ||
| - Every CommonMark list marker counts as a list item. The audit named only `-`, | ||
| `*` and ordered items, so a spec bulleted with `+` — which OpenSpec's own | ||
| validator accepts without complaint — had *every* scenario bullet reported as | ||
| unaccounted content and could not be retired at all. Found while hardening the | ||
| wrapping fix; it is the same defect wearing a different marker. | ||
| - Only those markers count. CommonMark stops an ordered marker at nine digits, | ||
| so `1234567890. Migration note...` opens a paragraph, not a list — and reading | ||
| it as a marker meant the same authored note was refused when it began with a | ||
| word and silently deleted when it began with a long enough number. | ||
| - Nothing else changes. The audit still fails safe — a line it cannot classify | ||
| still refuses the retirement — and a note written past the blank line that ends | ||
| a scenario is still named and still blocks, wrapped or not. | ||
| - Not breaking: the only behavior that moves is which specs `retire_capabilities` | ||
| accepts, and it moves strictly toward the specs it was always meant to accept. | ||
| No spec that retired before stops retiring, and no content is deleted that was | ||
| not already deletable. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| - `cli-archive`: states that a line continuing a wrapped list item is accounted | ||
| for with that item, and that a line opening a block of its own is not. | ||
|
|
||
| ## Impact | ||
|
|
||
| - **Affected behavior**: `openspec archive` and `openspec specs apply` on a change | ||
| declaring `retire_capabilities: true`, for specs whose bullets wrap. The | ||
| refusal, and the hint that names the blocking lines, are unchanged for every | ||
| other shape. | ||
| - **Unaffected**: writing, validating and merging specs. `unaccountedContent` is | ||
| read only by the retirement decision and the messages that explain it, so no | ||
| spec content is parsed differently anywhere else. | ||
| - **Docs**: none required — no documented rule changes; a shape that the | ||
| documented rule always implied now behaves that way. |
72 changes: 72 additions & 0 deletions
72
openspec/changes/fix-wrapped-scenario-bullet-retirement/specs/cli-archive/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Capability Retirement | ||
|
|
||
| A delta whose REMOVED entries cover every requirement a capability has SHALL retire that capability instead of writing a main spec with no requirements, which can never pass validation. | ||
|
|
||
| #### Scenario: Deciding that a rebuilt spec cannot be written | ||
|
|
||
| - **WHEN** applying a delta leaves the rebuilt spec with no requirement blocks, and every other nonblank line in the whole file is accounted for as the title, Purpose, Requirements header, or a canonical requirement's statement, scenarios, or fenced examples | ||
| - **THEN** put that rebuilt spec to the spec validator | ||
| - **AND** treat it as retirable only when its sole validation error is that the spec has no requirements | ||
| - **AND** otherwise write or reject it exactly as any other rebuilt spec, so a spec the validator still accepts, one broken in some further way, and one still holding a `###` heading are all left alone | ||
|
|
||
| #### Scenario: Validation was skipped | ||
|
|
||
| - **WHEN** the archive runs with validation disabled | ||
| - **THEN** retire nothing, because no verdict was produced to justify a deletion | ||
| - **AND** write the rebuilt spec exactly as an archive without this behavior would | ||
|
|
||
| #### Scenario: Retirement is not declared | ||
|
|
||
| - **WHEN** a rebuilt spec is retirable but the change does not declare `retire_capabilities: true` in its metadata, or declares it in metadata that cannot be honored | ||
| - **THEN** write the spec as any other, so the archive aborts on it exactly as it did before this behavior existed | ||
| - **AND** name the marker as the fix in that abort, and say when a marker that is present cannot be honored | ||
| - **AND** say nothing about the marker when retiring would not have made the spec writable anyway | ||
|
|
||
| #### Scenario: Delta removes the capability's last requirement | ||
|
|
||
| - **WHEN** a retirable rebuilt spec belongs to a capability whose main spec exists | ||
| - **AND** at least one requirement was actually removed by this run | ||
| - **AND** the change declares `retire_capabilities: true` | ||
| - **THEN** delete the capability's `spec.md` instead of writing it | ||
| - **AND** refuse to delete when the target resolves outside the real specs root | ||
| - **AND** delete any in-root directory the deletion leaves empty, and never the specs root itself | ||
| - **AND** count every operation the delta applied in the archive totals | ||
| - **AND** record the retirement in the archive warnings, naming what the deleted file held and giving a pasteable Git recovery command only when the spec lived in the caller's checkout | ||
|
|
||
| #### Scenario: Retirement is deferred until every spec is written | ||
|
|
||
| - **WHEN** an archive both retires one capability and updates another | ||
| - **THEN** settle the archive destination before touching any spec, so a name collision cannot strand a retirement | ||
| - **AND** perform the deletion only after every spec write has succeeded | ||
| - **AND** report a destination claimed while the merge ran as the same collision, rather than as a raw filesystem error | ||
|
|
||
| #### Scenario: Capability directory holds other files | ||
|
|
||
| - **WHEN** retiring a capability whose directory still holds other files after `spec.md` is deleted | ||
| - **THEN** leave that directory in place | ||
|
|
||
| #### Scenario: Removal was already synced | ||
|
|
||
| - **WHEN** a retirable rebuilt spec removed nothing this run and its main spec exists | ||
| - **THEN** leave the file untouched | ||
| - **AND** abort the archive with the validation error, as for any other unwritable spec, unless validation was skipped | ||
|
|
||
| #### Scenario: Content the merge cannot account for | ||
|
|
||
| - **WHEN** the spec holds any non-blank line the merge cannot name - anywhere in the file, including above the requirements section and inside a requirement block, where content the parser did not read as a new header rides along | ||
| - **THEN** refuse the retirement, because deleting the file would take that content with it | ||
| - **AND** say which lines stood in the way when the change declared the marker, rather than aborting on the bare validation error | ||
|
|
||
| #### Scenario: A bullet that wraps onto further lines | ||
|
|
||
| - **WHEN** a line inside a requirement block continues the list item above it - no blank line, heading, fence or new bullet has closed that item, and the line does not open a block of its own | ||
| - **THEN** account for it as part of that item rather than as a line in its own right, so a scenario bullet wrapped to a column limit does not refuse the retirement | ||
| - **AND** still weigh a table row, block quote, raw HTML, thematic break, fence or heading written directly beneath a bullet on its own, because each opens a block a reader sees as separate content | ||
| - **AND** read every CommonMark list marker as a list item, and only those - `-`, `*`, `+` and ordered markers of at most nine digits - so a capability is neither unretirable for the marker its author chose nor deleted because a note happened to open with a long number | ||
|
|
||
| #### Scenario: Main spec is already gone | ||
|
|
||
| - **WHEN** a REMOVED-only delta targets a capability that has no main spec, and the change declares `retire_capabilities: true` | ||
| - **THEN** complete the archive without creating or retiring one |
75 changes: 75 additions & 0 deletions
75
openspec/changes/fix-wrapped-scenario-bullet-retirement/tasks.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| ## 1. Reproduce | ||
|
|
||
| - [x] 1.1 Reproduce #1780 against the current build: the reporter's spec retires | ||
| with its `THEN` bullet on one line and is refused with the same bullet | ||
| wrapped, naming the wrapped remainder as the blocking content | ||
| - [x] 1.2 Confirm the second half of the report — the hint being suppressed — is | ||
| already fixed on `main` by #1699, so this change is scoped to the | ||
| classification alone | ||
|
|
||
| ## 2. Fix the classification | ||
|
|
||
| - [x] 2.1 Track, inside each requirement block, whether the previous line was a | ||
| list item or the continuation of one, and reset it on every construct that | ||
| closes an item: a blank line, a fenced line, a setext underline, a scenario | ||
| header, and any line that opens a block of its own | ||
| - [x] 2.2 Account for a continuation line exactly as the item above it was | ||
| accounted for, so a wrapped scenario bullet is silent and a wrapped note | ||
| below the last scenario is still named by its own first line | ||
| - [x] 2.3 Add `opensOwnBlock` for the constructs that interrupt a paragraph in | ||
| CommonMark — ATX heading, fence, block quote, thematic break, table row, | ||
| raw HTML — so none of them is swallowed by the bullet above it | ||
|
|
||
| ## 3. Close the same hole for every list marker | ||
|
|
||
| - [x] 3.1 Confirm the gap first: a spec whose scenarios use `+` bullets passes | ||
| `openspec validate --specs` and is refused retirement with every one of its | ||
| bullets named as unaccounted content | ||
| - [x] 3.2 Read `+` as a list marker alongside `-`, `*` and ordered items, and | ||
| confirm a `+` note written past the blank line that ends a scenario is | ||
| still named and still blocks | ||
| - [x] 3.3 Cap an ordered marker at nine digits, where CommonMark caps it, after | ||
| review found the same note refused when it opened with a word and deleted | ||
| when it opened with a ten-digit number | ||
|
|
||
| ## 4. Tests | ||
|
|
||
| - [x] 4.1 Retire a capability whose scenario bullet wraps with indentation — the | ||
| reporter's exact shape | ||
| - [x] 4.2 Retire one whose bullet wraps without indentation (CommonMark lazy | ||
| continuation), which is what an editor that hard-wraps produces | ||
| - [x] 4.3 Retire one wrapped under each list marker — `-`, `*`, `+` and an | ||
| ordered item | ||
| - [x] 4.4 Retire one whose bullet wraps onto three lines, so the item is known to | ||
| stay open past the first continuation | ||
| - [x] 4.5 Retire one whose spec is saved with CRLF endings, since the reporter | ||
| ran on Windows and Windows is in the CI matrix | ||
| - [x] 4.6 Keep refusing when a wrapped note follows the last scenario, and assert | ||
| the scenario's own wrapped remainder is not named among the blocking lines | ||
| - [x] 4.7 Keep refusing a `+` note written past the blank line, and assert the | ||
| scenario's own `+` bullets are no longer named alongside it | ||
| - [x] 4.8 Keep refusing a table row, block quote, raw HTML, and heading written | ||
| directly beneath a bullet | ||
| - [x] 4.9 Keep refusing a scenario whose bullets are split by a blank line - the | ||
| shape this repository's own `cli-show` spec uses | ||
| - [x] 4.10 Name a note opening with a ten-digit ordered marker, and keep | ||
| retiring one whose scenario uses a nine-digit marker, so the cap is held | ||
| from both sides | ||
| - [x] 4.11 Pin the deliberate edge: a line written under a bullet with no blank | ||
| line above it counts as part of that bullet | ||
|
|
||
| ## 5. Verify | ||
|
|
||
| - [x] 5.1 Mutation-check every new guard against `main` and by reverting each | ||
| guard in turn: 11 of the 15 cases fail on `main`; neutering | ||
| `opensOwnBlock` kills the four block cases; dropping `+` from the marker | ||
| set kills the plus case; closing the item after the first continuation | ||
| kills the three-line wrap and the lazy-aside case; loosening the digit cap | ||
| kills the long-marker case and tightening it kills the nine-digit case | ||
| - [x] 5.2 Run the full suite and confirm no existing test changes behavior | ||
| - [x] 5.3 Run lint, typecheck and the build | ||
| - [x] 5.4 Run `openspec validate --specs --strict` on this repo | ||
| - [x] 5.5 Sweep all 36 of this repository's own specs through a simulated | ||
| retirement on both `main` and this branch: identical verdicts (30 | ||
| retirable, 6 blocked), so no real spec changes classification | ||
| - [x] 5.6 Add a `.changeset/` entry |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.