mirror of https://github.com/Fission-AI/OpenSpec.git synced 2026-09-14 20:16:53 +08:00

Files

T

Clay Good 18688c8b27 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

2026-08-19 20:19:34 +00:00

2.8 KiB

Raw Permalink Blame History

Never dead-end a capability retirement

Why

A change whose delta removes the last requirement a capability has rebuilds the main spec empty, and an empty spec can never validate. Retirement is what archive does instead, and because it deletes a file it has to be asked for: the change declares retire_capabilities: true. The abort names that marker when it is the single thing missing.

Retirement is also refused while the spec holds any non-blank line the merge cannot name — a ## Notes section, a comment under a requirement. Both are ordinary things to find in a hand-written spec. When the marker was missing and such a line was present, neither hint fired: the marker hint was suppressed because adding it would not have let the archive through, and the hint that names those lines only spoke to authors who had already declared the marker.

The archive then aborted on a bare "Spec must have at least one requirement" with no guidance at all — the dead end the marker exists to close, still reachable (#1696, worked around there with --skip-specs plus a hand-applied sync).

What Changes

When this run emptied the capability, the marker is absent, and the spec holds content the merge cannot account for, the abort names that content and says what archive would otherwise do with the spec.
It still does not name the marker in that case. The marker is named only when adding it would really let the archive through; a spec with a second ## Requirements section holding a live requirement must not be pointed toward a deletion. Once the content is resolved, the rerun names the marker.
A marker that is present but cannot be honored is reported alongside the blocking content. An author who wrote retire_capabilities: yes-please believes they authorised the deletion; making them clear the content first, only to then learn the marker was never read, is two aborts for one mistake.
The blocking lines are authored file content printed to a terminal, so they are rendered with control characters replaced and their length bounded — the same treatment a change directory name already gets. This also hardens the marker-declared refusal, which echoed them verbatim.
The marker's own reason gets the same treatment, at its source in readBooleanMarker, because every reason quotes something the author wrote — a schema name, a parser message carrying one, a filesystem error carrying a path. Fixing it there covers openspec validate, which prints the same reason.

No change to what archive writes, deletes, or refuses. Message paths only.

Impact

Affected specs: cli-archive (MODIFIED: Capability Retirement)
Affected code: src/core/archive.ts, src/utils/change-metadata.ts
Affected docs: docs/writing-specs.md (states the second refusal condition, which was true before this change but undocumented)

2.8 KiB Raw Permalink Blame History

Never dead-end a capability retirement

Why

What Changes

Impact

2.8 KiB

Raw Permalink Blame History