Skip to content

Commit 18688c8

Browse files
authored
fix(archive): never dead-end a capability retirement (#1699)
* fix(archive): never dead-end a capability retirement A change whose delta removes the last requirement a capability has rebuilds the main spec empty, which can never validate. Archive already knows retiring is the fix and names the `retire_capabilities: true` marker that authorises deleting the spec - but only when the marker is the single thing missing. If the spec also holds a line the merge cannot account for (a `## Notes` section, a comment under a requirement - both ordinary), that hint was suppressed, and the hint that names such lines only spoke to authors who had already set the marker. Neither fired, so the archive aborted on "Spec must have at least one requirement" with no guidance at all: the exact dead end the marker exists to close. Archive now names the blocking content in that case. It deliberately does not name the marker there - adding it would not have let this run through, and the marker is only ever named when it really is the one thing missing. Once the content is resolved, the rerun names the marker. Closes #1696 * fix(archive): harden the blocked-retirement abort Three follow-ups to the same message. The blocking lines are authored spec content printed verbatim to a terminal, so they now get the treatment `describeChangeName` already gives a change directory name: control characters replaced, since a raw CR could forge a line of its own and an ESC could redraw the screen. Each line is bounded too - one very long line would push the way out of the abort off the reader's screen - and the cut counts code points so it can never leave half a surrogate pair. Both the declared and undeclared branches share the helper, so the marker-declared abort that shipped with #1484 is hardened with it. The wording no longer claims retiring is "the way through". It is not, in the one case this fires on that has a live requirement hiding in a second `## Requirements` section: merging the sections fixes that spec without deleting anything. `openspec/specs/cli-archive/spec.md` records the behavior change - the blocking lines are named whether or not the marker was declared, and the marker is still named only when adding it would let the archive through. * refactor(archive): drop a helper the revised wording made single-use The marker sentence is said in one place again, so it goes back inline rather than through a function that now has one caller. Also corrects the comment above `emptiedByThisRun`: retiring is not the only fix in every case it covers, which is exactly why the message stopped saying so. * docs(openspec): record the change as a delta, not a direct spec edit Both conventions exist in this repo's history, but the two most recent behavior fixes (#1609, #1616) carry an `openspec/changes/` delta rather than editing the main spec in place, which is also the workflow this project asks of everyone else. The delta reproduces the whole Capability Retirement requirement, so archiving it drops no scenario. Verified by archiving into a scratch copy of `openspec/`: the merged main spec differs from today's by exactly the three added bullets. * fix(archive): report an unhonorable marker alongside the blocking content An author who set `retire_capabilities: yes-please` believes they have authorised the deletion. Clearing the blocking content first, only to then learn the marker was never read, is two aborts for one mistake. The abort still never invites the marker to be added while content blocks the retirement - it only reports the one already there. The spec delta records that distinction, which the old bullet ("say nothing about the marker") did not draw. * style(archive): use one sentence for an unhonorable marker in both aborts * fix(metadata): strip control characters from an unhonorable marker reason Every reason a boolean change-metadata marker gives quotes something the author wrote - a schema name, a parser message carrying one, a filesystem error carrying a path - and two commands print it straight to a terminal. A schema name carrying a raw ESC, with the marker set, put that ESC on screen through `openspec archive`; `openspec validate` prints the same reason. Fixed at the source in `readBooleanMarker` rather than at either call site, so no consumer has to remember. The reason still quotes the name recognisably; only control characters are replaced. Reported by CodeRabbit on #1699. Pre-existing on main, and this PR would have added a second place it reaches the terminal. * test(archive): fix a comment left behind by the reworded abort
1 parent c747ed1 commit 18688c8

10 files changed

Lines changed: 400 additions & 30 deletions

File tree

.changeset/tidy-moons-smell.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@fission-ai/openspec': patch
3+
---
4+
5+
archive: tell the author how to retire a capability when the emptied spec also holds content the merge cannot account for. That combination printed only "Spec must have at least one requirement" and no guidance at all; the abort now names the blocking lines and reports a `retire_capabilities` marker that is present but cannot be honored. Authored content quoted in those messages - the blocking lines, and the marker's own reason, which `openspec validate` prints too - is stripped of control characters and bounded in length before it reaches the terminal.

docs/writing-specs.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ A change describes its edits to the specs with three section types. Using the ri
5656
- **`## MODIFIED Requirements`** — behavior that already existed and is changing. Include the full new version; a short note on what changed helps a reviewer.
5757
- **`## REMOVED Requirements`** — behavior going away, with a line on why.
5858

59-
On archive, ADDED gets appended to the main spec, MODIFIED replaces the old version, and REMOVED is dropped from it. Remove the last requirement a capability has and you retire it: rather than leave a spec with nothing in it, archive deletes `openspec/specs/<capability>/spec.md`. Because that is the one archive step that removes a file, it has to be asked for — add `retire_capabilities: true` to the change's `.openspec.yaml`, alongside the `schema:` that file already needs. Without it the archive aborts and tells you so. For a spec in the caller's checkout, the archive output also names the `git checkout` that restores a committed file; selected stores receive checkout-scoped recovery guidance instead. If you mark a real change as ADDED, you end up with two competing requirements; if you describe new behavior as MODIFIED, there's nothing to replace. When in doubt, open the current spec and see whether the requirement is already there.
59+
On archive, ADDED gets appended to the main spec, MODIFIED replaces the old version, and REMOVED is dropped from it. Remove the last requirement a capability has and you retire it: rather than leave a spec with nothing in it, archive deletes `openspec/specs/<capability>/spec.md`. Because that is the one archive step that removes a file, it has to be asked for — add `retire_capabilities: true` to the change's `.openspec.yaml`, alongside the `schema:` that file already needs. Without it the archive aborts and tells you so. Retirement deletes the whole file, so it is also refused while the spec holds anything outside its title, `## Purpose`, and its requirement blocks — a `## Notes` section, a comment under a requirement. The abort names those lines; move them into `## Purpose` or a requirement, or delete the spec by hand. For a spec in the caller's checkout, the archive output also names the `git checkout` that restores a committed file; selected stores receive checkout-scoped recovery guidance instead. If you mark a real change as ADDED, you end up with two competing requirements; if you describe new behavior as MODIFIED, there's nothing to replace. When in doubt, open the current spec and see whether the requirement is already there.
6060

6161
One more section is worth knowing about. When your delta creates a capability that doesn't exist yet, open it with `## Purpose` — a sentence or two on what the capability is for. Archive uses it as the Purpose of the main spec it creates; skip it and you get a `TBD` placeholder to fill in by hand. An existing spec already has a Purpose, so a delta's is ignored there — edit `openspec/specs/<capability-path>/spec.md` directly to change one. Here, `<capability-path>` is the directory relative to `specs/`, such as `user-auth` in a flat project or `identity/user-auth` in a project organized by domain.
6262

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-08-19
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# Never dead-end a capability retirement
2+
3+
## Why
4+
5+
A change whose delta removes the last requirement a capability has rebuilds the
6+
main spec empty, and an empty spec can never validate. Retirement is what
7+
archive does instead, and because it deletes a file it has to be asked for: the
8+
change declares `retire_capabilities: true`. The abort names that marker when it
9+
is the single thing missing.
10+
11+
Retirement is also refused while the spec holds any non-blank line the merge
12+
cannot name — a `## Notes` section, a comment under a requirement. Both are
13+
ordinary things to find in a hand-written spec. When the marker was missing *and*
14+
such a line was present, neither hint fired: the marker hint was suppressed
15+
because adding it would not have let the archive through, and the hint that names
16+
those lines only spoke to authors who had already declared the marker.
17+
18+
The archive then aborted on a bare "Spec must have at least one requirement" with
19+
no guidance at all — the dead end the marker exists to close, still reachable
20+
(#1696, worked around there with `--skip-specs` plus a hand-applied sync).
21+
22+
## What Changes
23+
24+
- When this run emptied the capability, the marker is absent, and the spec holds
25+
content the merge cannot account for, the abort names that content and says
26+
what archive would otherwise do with the spec.
27+
- It still does not name the marker in that case. The marker is named only when
28+
adding it would really let the archive through; a spec with a second
29+
`## Requirements` section holding a live requirement must not be pointed toward
30+
a deletion. Once the content is resolved, the rerun names the marker.
31+
- A marker that is present but cannot be honored is reported alongside the
32+
blocking content. An author who wrote `retire_capabilities: yes-please`
33+
believes they authorised the deletion; making them clear the content first,
34+
only to then learn the marker was never read, is two aborts for one mistake.
35+
- The blocking lines are authored file content printed to a terminal, so they are
36+
rendered with control characters replaced and their length bounded — the same
37+
treatment a change directory name already gets. This also hardens the
38+
marker-declared refusal, which echoed them verbatim.
39+
- The marker's own reason gets the same treatment, at its source in
40+
`readBooleanMarker`, because every reason quotes something the author wrote —
41+
a schema name, a parser message carrying one, a filesystem error carrying a
42+
path. Fixing it there covers `openspec validate`, which prints the same reason.
43+
44+
No change to what archive writes, deletes, or refuses. Message paths only.
45+
46+
## Impact
47+
48+
- Affected specs: `cli-archive` (MODIFIED: Capability Retirement)
49+
- Affected code: `src/core/archive.ts`, `src/utils/change-metadata.ts`
50+
- Affected docs: `docs/writing-specs.md` (states the second refusal condition,
51+
which was true before this change but undocumented)
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
## MODIFIED Requirements
2+
3+
### Requirement: Capability Retirement
4+
5+
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.
6+
7+
#### Scenario: Deciding that a rebuilt spec cannot be written
8+
9+
- **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
10+
- **THEN** put that rebuilt spec to the spec validator
11+
- **AND** treat it as retirable only when its sole validation error is that the spec has no requirements
12+
- **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
13+
14+
#### Scenario: Validation was skipped
15+
16+
- **WHEN** the archive runs with validation disabled
17+
- **THEN** retire nothing, because no verdict was produced to justify a deletion
18+
- **AND** write the rebuilt spec exactly as an archive without this behavior would
19+
20+
#### Scenario: Retirement is not declared
21+
22+
- **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
23+
- **THEN** write the spec as any other, so the archive aborts on it exactly as it did before this behavior existed
24+
- **AND** name the marker as the fix in that abort, and say when a marker that is present cannot be honored, with control characters replaced in the reason because it repeats what the author wrote
25+
- **AND** say nothing about adding the marker when retiring would not have made the spec writable anyway, while still reporting a marker that is present but cannot be honored
26+
27+
#### Scenario: Delta removes the capability's last requirement
28+
29+
- **WHEN** a retirable rebuilt spec belongs to a capability whose main spec exists
30+
- **AND** at least one requirement was actually removed by this run
31+
- **AND** the change declares `retire_capabilities: true`
32+
- **THEN** delete the capability's `spec.md` instead of writing it
33+
- **AND** refuse to delete when the target resolves outside the real specs root
34+
- **AND** delete any in-root directory the deletion leaves empty, and never the specs root itself
35+
- **AND** count every operation the delta applied in the archive totals
36+
- **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
37+
38+
#### Scenario: Retirement is deferred until every spec is written
39+
40+
- **WHEN** an archive both retires one capability and updates another
41+
- **THEN** settle the archive destination before touching any spec, so a name collision cannot strand a retirement
42+
- **AND** perform the deletion only after every spec write has succeeded
43+
- **AND** report a destination claimed while the merge ran as the same collision, rather than as a raw filesystem error
44+
45+
#### Scenario: Capability directory holds other files
46+
47+
- **WHEN** retiring a capability whose directory still holds other files after `spec.md` is deleted
48+
- **THEN** leave that directory in place
49+
50+
#### Scenario: Removal was already synced
51+
52+
- **WHEN** a retirable rebuilt spec removed nothing this run and its main spec exists
53+
- **THEN** leave the file untouched
54+
- **AND** abort the archive with the validation error, as for any other unwritable spec, unless validation was skipped
55+
56+
#### Scenario: Content the merge cannot account for
57+
58+
- **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
59+
- **THEN** refuse the retirement, because deleting the file would take that content with it
60+
- **AND** say which lines stood in the way whether or not the change declared the marker, rather than aborting on the bare validation error
61+
- **AND** name the marker only when adding it would let the archive through, so an author whose spec still holds such content is pointed at that content first
62+
- **AND** render those lines with control characters replaced and their length bounded, because a spec that redraws the terminal or fills the screen would take the way out of the abort with it
63+
64+
#### Scenario: Main spec is already gone
65+
66+
- **WHEN** a REMOVED-only delta targets a capability that has no main spec, and the change declares `retire_capabilities: true`
67+
- **THEN** complete the archive without creating or retiring one
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Tasks
2+
3+
## 1. Name the blocking content when the marker is absent
4+
- [x] 1.1 Derive "this run emptied the capability" once, and hint on it in both the marker-missing and content-blocked cases
5+
- [x] 1.2 Keep the marker unnamed while content still blocks the retirement, while still reporting one that cannot be honored
6+
7+
## 2. Render the blocking lines safely
8+
- [x] 2.1 Replace control characters and bound each line, sharing one helper with the marker-declared refusal
9+
- [x] 2.2 Sanitize the marker's own reason at its source, so `validate` is covered too
10+
- [x] 2.3 Cover the human abort, the `--json` detail, and the rendering with tests
11+
12+
## 3. Record the behavior
13+
- [x] 3.1 Update the `cli-archive` spec delta and `docs/writing-specs.md`

0 commit comments

Comments
 (0)