diff --git a/.changeset/retire-capability-on-removed-only-delta.md b/.changeset/retire-capability-on-removed-only-delta.md new file mode 100644 index 0000000000..0a3dd7a85a --- /dev/null +++ b/.changeset/retire-capability-on-removed-only-delta.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Retire a capability when a change removes its last requirement. A change that declares `retire_capabilities: true` in its `.openspec.yaml` (alongside the `schema:` that file requires) may now be archived even when its REMOVED entries take a capability's last requirement: `openspec archive` deletes that capability's main spec instead of aborting with "Spec must have at least one requirement". Without the marker nothing changes — the archive aborts exactly as before, except the message now names the marker as the way out. Retirement happens only when the emptied spec could not have been written at all, every one is named in the archive output, a pasteable `git checkout` is included when the spec lived in the caller's checkout, and `--no-validate` never retires. Archive now also rejects a main spec with duplicate canonical requirement names instead of letting delta reconciliation collapse one of the duplicate blocks. One thing to know before retiring: a capability's spec is the base another change's MODIFIED block is checked against, so an in-flight change that modifies the capability you just retired will keep validating clean and then refuse to archive ("target spec does not exist; only ADDED requirements are allowed for new specs") — close or rework that change alongside the retirement. diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 65e2004ae7..17cec31135 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -72,7 +72,7 @@ Change: `{ "id", "title", "deltaCount", "deltas": [...], "root" }`. Spec: `{ "id Success: `{ "change": { "id", "path", "metadataPath", "schema" }, "root" }`. Failure: `{ "change": null, "status": [d] }`, exit 1. ### 4.9 `archive --json` -Success: `{ "archive": { "change", "archivedAs": "YYYY-MM-DD-name", "path", "specsUpdated", "totals"?, "warnings"? }, "root" }`. Failure: `{ "archive": null, "root"?, "status": [d] }`, exit 1. `specsUpdated` is true only when at least one spec file was written; an already-synced change archives with all-zero totals and the skips listed in `warnings`. JSON mode is strictly non-interactive: every prompt point becomes an `archive_*` code. +Success: `{ "archive": { "change", "archivedAs": "YYYY-MM-DD-name", "path", "specsUpdated", "totals"?, "warnings"? }, "root" }`. Failure: `{ "archive": null, "root"?, "status": [d] }`, exit 1. `specsUpdated` is true only when at least one spec file was written or retired (a capability whose last requirement the change removed has its spec deleted, which requires `retire_capabilities: true` in the change's `.openspec.yaml`; every retirement is named in `warnings`, with a pasteable Git recovery command only when the spec lived in the caller's checkout); an already-synced change archives with all-zero totals and the skips listed in `warnings`. JSON mode is strictly non-interactive: every prompt point becomes an `archive_*` code. ### 4.10 `doctor --json` `{ "root": { "path", "source", "store_id"?, "healthy", "status": [] }, "store": { "id", "metadata": {present,valid,remote?}, "origin_url"?, "drift"?: {ahead,behind}, "status": [] } | null, "references": [...], "status": [] }`. `drift` (present only for a git-backed store checkout that has an upstream tracking ref) is ahead/behind counts against the last-fetched upstream, not the live remote. Health findings of any severity exit 0. Failure payload: `{ "root": null, "store": null, "references": [], "status": [d] }`, exit 1. @@ -119,7 +119,7 @@ setup/register: `{ "store": {id, root, metadata_path?}, "registry": {path, regis `relationship_registry_unreadable`, `root_pointer_ignored`, `root_pointer_invalid`, `pointer_declarations_inert`. ### Archive (JSON mode) -`archive_change_name_required`, `archive_change_not_found`, `archive_validation_failed`, `archive_confirmation_required`, `archive_tasks_incomplete`, `archive_spec_update_failed`, `archive_spec_validation_failed`, `archive_target_exists`, `archive_error`. +`archive_change_name_required`, `archive_change_not_found`, `archive_change_symlink`, `archive_validation_failed`, `archive_confirmation_required`, `archive_tasks_incomplete`, `archive_spec_update_failed`, `archive_spec_validation_failed`, `archive_target_exists`, `archive_error`. ### Context writes `context_file_exists`, `context_output_dir_missing`. diff --git a/docs/cli.md b/docs/cli.md index 881fc90133..271ca5c8c7 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -630,7 +630,7 @@ openspec archive [change-name] [options] |--------|-------------| | `-y, --yes` | Skip confirmation prompts. Required when nothing can answer them — an AI agent, a CI job, or any run with stdin closed | | `--skip-specs` | Skip spec updates for one archive run. A change that permanently has no spec deltas should declare `skip_specs: true` in its `.openspec.yaml` instead — it archives with no flag | -| `--no-validate` | Skip validation (requires confirmation) | +| `--no-validate` | Skip validation (requires confirmation). Also disables capability retirement — with no validator verdict, nothing is retired | **Examples:** @@ -652,8 +652,11 @@ openspec archive update-ci-config --skip-specs 1. Validates the change (unless `--no-validate`) 2. Prompts for confirmation (unless `--yes`) -3. Merges delta specs into `openspec/specs/` -4. Moves change folder to `openspec/changes/archive/YYYY-MM-DD-/` +3. Claims the archive destination before changing any main spec +4. Validates and merges the active delta specs into `openspec/specs/` — a capability whose last requirement the change removes is retired, and its spec file deleted, but only when the change's `.openspec.yaml` declares `retire_capabilities: true` next to its `schema:` +5. Moves the change folder to `openspec/changes/archive/YYYY-MM-DD-/` +6. If a spec mutation or final move fails before a complete archive is secured, restores the specs and leaves or returns the change at its active path +7. If a verified fallback copy completes but staged-source cleanup fails, retains the complete archive and committed spec state for recovery **Without a terminal:** an AI agent, a CI job, or any run with stdin closed cannot answer step 2, so archive stops before touching anything, exits 1, and names the diff --git a/docs/concepts.md b/docs/concepts.md index caca2bc140..10106c5b78 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -190,7 +190,7 @@ openspec/changes/add-dark-mode/ ├── proposal.md # Why and what ├── design.md # How (technical approach) ├── tasks.md # Implementation checklist -├── .openspec.yaml # Change metadata (optional): schema, created, skip_specs +├── .openspec.yaml # Change metadata (optional): schema, created, skip_specs, retire_capabilities └── specs/ # Delta specs └── ui/ └── spec.md # What's changing in ui/spec.md @@ -392,7 +392,7 @@ The system MUST expire sessions after 15 minutes of inactivity. |---------|---------|------------------------| | `## ADDED Requirements` | New behavior | Appended to main spec | | `## MODIFIED Requirements` | Changed behavior | Replaces existing requirement | -| `## REMOVED Requirements` | Deprecated behavior | Deleted from main spec | +| `## REMOVED Requirements` | Deprecated behavior | Deleted from main spec; removing the last requirement retires the capability and deletes its spec file, when the change declares `retire_capabilities: true` | | `## Purpose` | What a brand-new capability is for | Seeds the Purpose of the main spec being created; ignored when the spec already exists | ### Why Deltas Instead of Full Specs diff --git a/docs/faq.md b/docs/faq.md index 9afd9afcf7..770479aa3e 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -108,7 +108,7 @@ A spec that describes only what's changing, using `ADDED`, `MODIFIED`, and `REMO ### Where do archived changes go? -To `openspec/changes/archive/YYYY-MM-DD-/`, with all artifacts preserved. Nothing is deleted; the change just moves out of your active list. +To `openspec/changes/archive/YYYY-MM-DD-/`, with all change artifacts preserved. The change moves out of your active list. A change that explicitly declares `retire_capabilities: true` can also delete a main capability spec when it removes that capability's final requirement. ## Configuration and customization diff --git a/docs/writing-specs.md b/docs/writing-specs.md index c894c8f2cb..501c129cd1 100644 --- a/docs/writing-specs.md +++ b/docs/writing-specs.md @@ -56,7 +56,7 @@ A change describes its edits to the specs with three section types. Using the ri - **`## MODIFIED Requirements`** — behavior that already existed and is changing. Include the full new version; a short note on what changed helps a reviewer. - **`## REMOVED Requirements`** — behavior going away, with a line on why. -On archive, ADDED gets appended to the main spec, MODIFIED replaces the old version, and REMOVED is deleted. 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. +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//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. 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//spec.md` directly to change one. diff --git a/openspec/specs/cli-archive/spec.md b/openspec/specs/cli-archive/spec.md index 8cd8d9e268..6d13b79721 100644 --- a/openspec/specs/cli-archive/spec.md +++ b/openspec/specs/cli-archive/spec.md @@ -61,9 +61,12 @@ The archive operation SHALL follow a structured process to safely move changes t - **THEN** execute these steps: 1. Create archive/ directory if it doesn't exist 2. Generate target name as `YYYY-MM-DD-[change-name]` using current date, keeping the name as-is when it already starts with a `YYYY-MM-DD-` prefix - 3. Check if target directory already exists - 4. Update main specs from the change's future state specs (see Spec Update Process below) - 5. Move the entire change directory to the archive location + 3. Claim the target and verify that it does not already exist + 4. Prepare and validate spec updates from the active change's delta specs + 5. Apply the spec updates as a rollback-capable transaction + 6. Move the entire change directory to the archive location + 7. If a spec mutation or final move fails before a complete archive is secured, restore the spec transaction and leave or return the change at its active path + 8. If a verified fallback copy completes but staged-source cleanup fails, retain the complete archive and committed spec state for recovery instead of risking the only complete copy #### Scenario: Archive already exists @@ -78,7 +81,7 @@ The archive operation SHALL follow a structured process to safely move changes t ### Requirement: Spec Update Process -Before moving the change to archive, the command SHALL apply delta changes to main specs to reflect the deployed reality. +After claiming the archive destination, the command SHALL apply delta changes to main specs to reflect the deployed reality, then move the change to its archive destination. It SHALL restore the spec transaction when a mutation or final move fails before a complete archive is secured. Once a verified fallback archive is complete, a staged-source cleanup failure SHALL retain that archive and committed spec state for recovery. #### Scenario: Applying delta changes @@ -98,6 +101,12 @@ Before moving the change to archive, the command SHALL apply delta changes to ma - **THEN** abort with error message showing the conflict - **AND** suggest manual resolution +#### Scenario: Duplicate requirement already exists in the main spec + +- **WHEN** a main spec contains two canonical requirement headers with the same name +- **THEN** reject the structurally ambiguous main spec before applying any delta +- **AND** preserve the main spec and active change unchanged + #### Scenario: New main spec inherits the delta's Purpose - **WHEN** a delta creates a main spec that does not exist yet @@ -130,6 +139,70 @@ Before moving the change to archive, the command SHALL apply delta changes to ma - **THEN** leave the existing Purpose untouched - **AND** warn that the delta Purpose was ignored, naming the spec file to edit directly, but only when that spec has a Purpose of its own and it differs from the delta's +### 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: 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 + ### Requirement: Confirmation Behavior The spec update confirmation SHALL provide clear visibility into changes before they are applied. @@ -269,6 +342,6 @@ The archive command SHALL validate changes before applying them to ensure data i **Task checking**: Prevents accidental archiving of incomplete work **Date prefixing**: Maintains chronological order and prevents naming conflicts; a name that already carries a date prefix keeps it, so archived names never stack dates **No overwrite**: Preserves historical archives and prevents data loss -**Spec updates before archiving**: Specs in the main directory represent current reality; when a change is deployed and archived, its future state specs become the new reality and must replace the main specs +**Claim-first transaction**: The destination is claimed before main specs are mutated, spec changes are rollback-protected, and the active change is moved only after the spec transaction succeeds **Confirmation for spec updates**: Provides visibility into what will change, prevents accidental overwrites, and ensures users understand the impact before specs are modified -**--yes flag for automation**: Allows CI/CD pipelines to archive without interactive prompts while maintaining safety by default for manual use \ No newline at end of file +**--yes flag for automation**: Allows CI/CD pipelines to archive without interactive prompts while maintaining safety by default for manual use diff --git a/openspec/specs/opsx-archive-skill/spec.md b/openspec/specs/opsx-archive-skill/spec.md index 5ebf37a88d..2c76461e54 100644 --- a/openspec/specs/opsx-archive-skill/spec.md +++ b/openspec/specs/opsx-archive-skill/spec.md @@ -78,6 +78,7 @@ The skill SHALL prompt to sync delta specs before archiving if specs exist. - **AND** if user cancels, stop without archiving - **AND** if user confirms, execute `/opsx:sync` logic inline and wait for it to complete - **AND** verify every capability that has a delta spec, not only those the sync reports it touched: ADDED requirements present, MODIFIED requirements carrying the changes named in the delta, REMOVED requirements absent, RENAMED requirements present under the new name and absent under the old one +- **AND** treat a capability whose last requirement the sync removed as verified when its main spec was deleted rather than left empty, and a spec the sync deliberately kept and reported as verified too - **AND** stop without archiving if the sync fails or any capability does not verify - **AND** archive only after verification passes, or when the user explicitly chose to archive without syncing or to archive already-synced specs diff --git a/openspec/specs/specs-sync-skill/spec.md b/openspec/specs/specs-sync-skill/spec.md index 1b925049e2..a69263a634 100644 --- a/openspec/specs/specs-sync-skill/spec.md +++ b/openspec/specs/specs-sync-skill/spec.md @@ -48,6 +48,22 @@ The agent SHALL reconcile main specs with delta specs using the delta operation - **AND** the requirement exists in main spec - **THEN** remove the requirement from main spec +#### Scenario: REMOVED requirements retire the capability +- **WHEN** removing the requirements named in the delta leaves 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 +- **AND** the rest of the spec is well-formed and it was not already empty before this sync +- **AND** the change declares `retire_capabilities: true` in its metadata +- **AND** the `spec.md` resolves inside the real specs root +- **THEN** delete that capability's `spec.md`, and its directory once nothing else remains in it +- **AND** report the retirement and name the deleted `## Purpose` +- **AND** leave the file in place and say the marker is missing when it is not declared + +#### Scenario: Something is left in the spec +- **WHEN** any of those conditions fails - unaccounted content remains anywhere in the file, the spec is malformed, or nothing was removed this run +- **THEN** do not modify the main spec and stop the sync for that capability +- **AND** report the blocking condition and how the user can resolve it +- **AND** never write or leave an empty `## Requirements` section + #### Scenario: RENAMED requirements - **WHEN** delta contains `## RENAMED Requirements` with FROM:/TO: format - **AND** the FROM requirement exists in main spec diff --git a/skills/openspec-archive-change/SKILL.md b/skills/openspec-archive-change/SKILL.md index d028076057..fac60b5f37 100644 --- a/skills/openspec-archive-change/SKILL.md +++ b/skills/openspec-archive-change/SKILL.md @@ -119,7 +119,7 @@ Archive a completed change in the experimental workflow. Then re-run the comparison from the top of this step against every capability that has a delta spec in `artifactPaths.specs.existingOutputPaths` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: - ADDED requirements present - MODIFIED requirements carrying the scenario and description changes named in the delta, with their other scenarios intact - - REMOVED requirements gone + - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving `## Requirements` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match - RENAMED requirements present under the new name and absent under the old one If the sync failed, or any capability does not match, report what differs and stop — do not archive. Nothing has moved and `changeRoot` is intact, so the user can fix the mismatch or re-run the sync and start the archive again. diff --git a/skills/openspec-bulk-archive-change/SKILL.md b/skills/openspec-bulk-archive-change/SKILL.md index 8bd2ebdf6e..5d7289d812 100644 --- a/skills/openspec-bulk-archive-change/SKILL.md +++ b/skills/openspec-bulk-archive-change/SKILL.md @@ -190,7 +190,7 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig - Verify that main specs are updated: - ADDED requirements present - MODIFIED requirements carrying scenario and description changes named in the delta, with their other scenarios intact - - REMOVED requirements gone + - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving `## Requirements` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match - RENAMED requirements present under the new name and absent under the old one - Do not verify delta specs in `excludedDeltas`; they are intentionally left unsynced. - If sync failed or any capability does not match verification, report what differs and fail/skip moving that change's `changeRoot` — do not archive that change. `changeRoot` remains intact. diff --git a/skills/openspec-sync-specs/SKILL.md b/skills/openspec-sync-specs/SKILL.md index e907fc41c6..4fd5ffd7f4 100644 --- a/skills/openspec-sync-specs/SKILL.md +++ b/skills/openspec-sync-specs/SKILL.md @@ -107,6 +107,28 @@ This is an **agent-driven** operation - you will read delta specs and directly e **REMOVED Requirements:** - Remove the entire requirement block from main spec + - Retiring the capability. Delete the whole `spec.md` - and the directory once + nothing else is left in it - only when ALL of these hold: + 1. removing the requirements *this run* left no requirement blocks; + 2. the rest of the spec is well-formed (it still has a `## Purpose`); + 3. the main spec was not already empty before this sync - if you removed + nothing, change nothing; + 4. 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; + 5. the change's `.openspec.yaml` declares `retire_capabilities: true`; + 6. the `spec.md` resolves inside the real specs root (do not follow a + capability-directory symlink to delete an external file). + If removing the selected requirements would leave no requirement blocks and + any retirement condition is not satisfied, do not modify the main spec. Stop + the sync for that capability, report the blocking condition, and tell the user + how to resolve it. Never write or leave an empty `## Requirements` section. + When only the marker is missing, say that too - it is the one thing the user + can add to make the retirement go through. + - Deleting the file also deletes its `## Purpose`; any other section blocks + retirement. Name Purpose when you report the retirement. Include a pasteable + `git checkout` only when the spec lived in the caller's checkout; + otherwise give checkout-scoped recovery guidance. **RENAMED Requirements:** - Find the FROM requirement, rename to TO @@ -129,6 +151,8 @@ This is an **agent-driven** operation - you will read delta specs and directly e - What changes were made (requirements added/modified/removed/renamed) - Any new main spec left with a TBD Purpose placeholder, so it gets written now rather than lingering + - Any capability retired, naming the deleted `spec.md`, its Purpose, and + either a pasteable `git checkout` or checkout-scoped recovery guidance **Delta Spec Format Reference** diff --git a/src/core/archive.ts b/src/core/archive.ts index d8cdbf18d7..b5813fc558 100644 --- a/src/core/archive.ts +++ b/src/core/archive.ts @@ -1,8 +1,10 @@ -import { promises as fs } from 'fs'; +import { constants, createReadStream, promises as fs } from 'fs'; +import { createHash, randomUUID } from 'crypto'; import path from 'path'; import { formatLocalDate } from '../utils/date.js'; import { getTaskProgressForChange, formatTaskStatus } from '../utils/task-progress.js'; import { Validator } from './validation/validator.js'; +import { VALIDATION_MESSAGES } from './validation/constants.js'; import chalk from 'chalk'; import { emitStoreRootBanner, @@ -17,10 +19,12 @@ import { findSpecUpdates, buildUpdatedSpec, writeUpdatedSpec, + retireSpec, + finalizeRetiredSpec, type SpecUpdate, } from './specs-apply.js'; import { discoverSpecFiles, hasAnyFileUnder } from '../utils/spec-discovery.js'; -import { readSkipSpecsMarker } from '../utils/change-metadata.js'; +import { METADATA_FILENAME, readRetireCapabilitiesMarker, readSkipSpecsMarker } from '../utils/change-metadata.js'; import { isNonInteractivePromptError } from '../utils/interactive.js'; import { FileSystemUtils } from '../utils/file-system.js'; import { folderStyleNameProblem } from './id.js'; @@ -41,6 +45,90 @@ function isMissingPathError(error: unknown): boolean { */ const ARCHIVE_DATE_PREFIX_PATTERN = /^\d{4}-\d{2}-\d{2}-/; +/** + * True when the ONLY thing wrong with a rebuilt spec is that it has no + * requirements. That is the exact failure retiring a capability replaces + * (#1302); anything else means the spec is broken in a way the author still has + * to fix, so archive must abort exactly as it always did instead of retiring. + * + * Asking the validator - rather than counting requirement blocks a second time - + * is what makes "this spec could not have been written anyway" true by + * construction. The two counts genuinely disagree: `MarkdownParser` accepts any + * `###` heading under `## Requirements` as a requirement, while the delta block + * parser only indexes canonical `### Requirement:` headers and sweeps the rest + * into the preamble, which survives into the rebuilt spec. + */ +export async function isRetirableSpec(specName: string, rebuilt: string): Promise { + const report = await new Validator().validateSpecContent(specName, rebuilt); + if (report.valid) return false; + const errors = report.issues.filter((issue) => issue.level === 'ERROR'); + return ( + errors.length > 0 && + errors.every((issue) => issue.message === VALIDATION_MESSAGES.SPEC_NO_REQUIREMENTS) + ); +} + +/** + * What this run should do with a rebuilt spec: write it as usual, retire the + * capability because the delta removed its last requirement (#1302), or do + * nothing because there is no spec to write and none to retire. + */ +type SpecOutcome = 'write' | 'retire' | 'skip'; + +async function isRetirementCandidate( + update: SpecUpdate, + built: Pick< + Awaited>, + 'rebuilt' | 'noRequirementBlocks' | 'unaccountedContent' + >, + skipValidation: boolean +): Promise { + return ( + !skipValidation && + built.noRequirementBlocks && + built.unaccountedContent.length === 0 && + (await isRetirableSpec(update.id, built.rebuilt)) + ); +} + +async function decideSpecOutcome( + update: SpecUpdate, + built: Awaited>, + skipValidation: boolean, + retirementDeclared: boolean +): Promise { + // The author has to have asked. Without the marker this falls through to the + // ordinary write, which fails validation exactly as it always did - and the + // abort names the marker, so the dead end #1302 describes now comes with its + // own way out instead of just a rejected spec. + if (!retirementDeclared) return 'write'; + + // Retirement is decided by the validator, never by a second opinion about + // what counts as a requirement: the block parser sweeps some shapes the + // validator accepts into the preamble, so "no blocks left" alone would retire + // specs that validate fine. + // + // Residual `###` headings veto it outright. The validator can be talked out of + // seeing them - a stray `### Requirements` under Purpose captures its section + // lookup - but a reader cannot, and deleting the file would take them with it. + // + // Under --no-validate there is no verdict to lean on, so nothing is retired: + // the author opted out of the check that makes this safe, and the old + // behavior (write the spec) loses nothing. + // Nothing in the file may sit outside the parts the merge understands. Asked + // as "did anything land outside the parts I understand" rather than "does + // anything look like a requirement" - the second question is the one six + // review rounds each found a new way to answer wrongly. + const retirable = await isRetirementCandidate(update, built, skipValidation); + + if (!retirable) return 'write'; + // Nothing on disk to write or retire: the capability is already retired. + if (!update.exists) return 'skip'; + // A spec that was already requirement-less and lost nothing this run is still + // the author's to fix, so it takes the same abort it has always produced. + return built.counts.removed > 0 ? 'retire' : 'write'; +} + async function listActiveChangeNames(changesDir: string): Promise { try { const entries = await fs.readdir(changesDir, { withFileTypes: true }); @@ -125,9 +213,22 @@ class ArchiveBlockedError extends Error { * has to fill in. */ function quoteChangeName(name: string): string { - if (/^[A-Za-z0-9._-]+$/.test(name)) return name; - if (!/["\\$`\r\n%!]/.test(name)) return `"${name}"`; - return ''; + return quoteForShell(name) ?? ''; +} + +/** + * Quotes an argument for a line the reader is meant to paste, or returns + * undefined when no portable spelling exists. + * + * Double quotes are the one form bash, zsh, PowerShell and cmd.exe all read the + * same way. A value holding a character that stays special INSIDE double quotes + * in any of them has no portable spelling, so callers say something else rather + * than emit a command that expands to something the reader did not intend. + */ +function quoteForShell(value: string): string | undefined { + if (/^[A-Za-z0-9._\/-]+$/.test(value)) return value; + if (!/["\\$`\r\n%!]/.test(value)) return `"${value}"`; + return undefined; } /** @@ -222,47 +323,217 @@ async function copySymbolicLink(src: string, dest: string): Promise { await fs.symlink(destinationTarget, dest, isWindowsDirectoryLink ? 'junction' : undefined); } -async function copyDirRecursive(src: string, dest: string): Promise { - // Every destination is new: exclusive directory creation prevents a - // symlink introduced after the archive target check from redirecting the - // cross-device fallback outside the archive. - await fs.mkdir(dest); +async function copyDirContents(src: string, dest: string): Promise { + const sourceStat = await fs.lstat(src); + // Keep group/other access no broader than the source while ensuring this + // process can populate even a read-only source directory. + await fs.chmod(dest, (sourceStat.mode & 0o7777) | 0o700); const entries = await fs.readdir(src, { withFileTypes: true }); for (const entry of entries) { const srcPath = path.join(src, entry.name); const destPath = path.join(dest, entry.name); if (entry.isDirectory()) { - await copyDirRecursive(srcPath, destPath); + await fs.mkdir(destPath, { mode: 0o700 }); + await copyDirContents(srcPath, destPath); } else if (entry.isSymbolicLink()) { await copySymbolicLink(srcPath, destPath); } else if (entry.isFile()) { - await fs.copyFile(srcPath, destPath); + await fs.copyFile(srcPath, destPath, constants.COPYFILE_EXCL); } else { throw new Error(`Cannot archive unsupported filesystem entry: ${srcPath}`); } } + await fs.chmod(dest, sourceStat.mode & 0o7777); +} + +async function fingerprintDirectoryContents(root: string): Promise { + const hash = createHash('sha256'); + const updateHashField = (label: string, value: string | Buffer): void => { + const labelBuffer = Buffer.from(label); + const valueBuffer = typeof value === 'string' ? Buffer.from(value) : value; + const lengths = Buffer.allocUnsafe(16); + lengths.writeBigUInt64BE(BigInt(labelBuffer.length), 0); + lengths.writeBigUInt64BE(BigInt(valueBuffer.length), 8); + hash.update(lengths); + hash.update(labelBuffer); + hash.update(valueBuffer); + }; + const fingerprintFile = async (filePath: string): Promise => { + const fileHash = createHash('sha256'); + for await (const chunk of createReadStream(filePath)) { + fileHash.update(chunk); + } + return fileHash.digest(); + }; + + const visit = async (dir: string, relativeDir: string): Promise => { + const before = await fs.lstat(dir, { bigint: true }); + if (!before.isDirectory()) { + throw new Error(`Expected a directory while verifying ${dir}.`); + } + updateHashField('directory-mode', (before.mode & 0o7777n).toString()); + const entries = (await fs.readdir(dir, { withFileTypes: true })).sort((a, b) => + a.name < b.name ? -1 : a.name > b.name ? 1 : 0 + ); + + for (const entry of entries) { + const entryPath = path.join(dir, entry.name); + const relativePath = path.join(relativeDir, entry.name); + const stat = await fs.lstat(entryPath, { bigint: true }); + updateHashField('path', relativePath); + + if (stat.isDirectory()) { + updateHashField('type', 'directory'); + await visit(entryPath, relativePath); + } else if (stat.isSymbolicLink()) { + const target = await fs.readlink(entryPath); + const after = await fs.lstat(entryPath, { bigint: true }); + if (statIdentity(stat) !== statIdentity(after)) { + throw new Error(`Path changed while archive was reading ${entryPath}.`); + } + updateHashField('type', 'symlink'); + updateHashField('target', target); + } else if (stat.isFile()) { + const contentFingerprint = await fingerprintFile(entryPath); + const after = await fs.lstat(entryPath, { bigint: true }); + if (statIdentity(stat) !== statIdentity(after)) { + throw new Error(`Path changed while archive was reading ${entryPath}.`); + } + updateHashField('type', 'file'); + updateHashField('mode', (stat.mode & 0o7777n).toString()); + updateHashField('content-sha256', contentFingerprint); + } else { + updateHashField('type', 'other'); + updateHashField('mode', stat.mode.toString()); + updateHashField('size', stat.size.toString()); + } + } + + const after = await fs.lstat(dir, { bigint: true }); + if (statIdentity(before) !== statIdentity(after)) { + throw new Error(`Directory changed while archive was reading ${dir}.`); + } + }; + + await visit(root, ''); + return hash.digest('hex'); +} + +async function assertCopiedDirectoryUnchanged( + stagedSource: string, + destination: string, + expectedFingerprint: string +): Promise { + const sourceFingerprint = await fingerprintDirectoryContents(stagedSource); + const destinationFingerprint = await fingerprintDirectoryContents(destination); + if ( + sourceFingerprint !== expectedFingerprint || + destinationFingerprint !== expectedFingerprint + ) { + throw new Error( + `Change directory contents changed during the fallback copy from ${stagedSource} to ${destination}.` + ); + } } /** - * Move a directory from src to dest. On Windows, fs.rename() often fails with - * EPERM when the directory is non-empty or another process has it open (IDE, - * file watcher, antivirus). Fall back to copy-then-remove when rename fails - * with EPERM or EXDEV. + * Move a directory from src to dest. On Windows, fs.rename() can fail with + * EPERM, and cross-device moves fail with EXDEV. When the source can first be + * renamed to a private sibling, fall back to a verified copy-then-remove. A + * source that cannot be staged is left untouched rather than copied and deleted + * through a path another process may still be editing. */ -async function moveDirectory(src: string, dest: string): Promise { +class MoveDestinationRetainedError extends Error {} +class RetirementBackupsRetainedError extends Error {} + +async function moveDirectory( + src: string, + dest: string, + options: { + verifyCopiedDestination?: (stagedSource: string) => Promise; + } = {} +): Promise { try { await fs.rename(src, dest); } catch (err: any) { const code = err?.code; + // rename onto a non-empty directory: the destination was taken while the + // archive was running. Same condition the pre-flight check reports. + if (code === 'ENOTEMPTY' || code === 'EEXIST') { + throw new ArchiveBlockedError( + 'archive_target_exists', + `Archive '${path.basename(dest)}' already exists.` + ); + } if (code === 'EPERM' || code === 'EXDEV') { - const sourceStat = await fs.lstat(src); - if (sourceStat.isSymbolicLink()) { - await fs.mkdir(path.dirname(dest), { recursive: true }); - await copySymbolicLink(src, dest); - await fs.unlink(src); - } else { - await copyDirRecursive(src, dest); - await fs.rm(src, { recursive: true, force: true }); + const stagedSource = path.join(path.dirname(src), `.openspec-move-${randomUUID()}`); + try { + await fs.rename(src, stagedSource); + } catch (stageError) { + throw new Error( + `Could not safely stage ${src} before the fallback archive copy ` + + `(${stageError instanceof Error ? stageError.message : String(stageError)}). ` + + 'No fallback copy was attempted.' + ); + } + let destIsOurs = false; + let stagedFingerprint: string; + try { + stagedFingerprint = await fingerprintDirectoryContents(stagedSource); + await fs.mkdir(dest, { mode: 0o700 }); + destIsOurs = true; + await copyDirContents(stagedSource, dest); + await options.verifyCopiedDestination?.(stagedSource); + await assertCopiedDirectoryUnchanged(stagedSource, dest, stagedFingerprint); + } catch (copyError) { + if (destIsOurs) { + await fs.rm(dest, { recursive: true, force: true }).catch(() => undefined); + } + try { + await fs.rename(stagedSource, src); + } catch (restoreError) { + throw new Error( + `${copyError instanceof Error ? copyError.message : String(copyError)} ` + + `Could not restore the staged source at ${stagedSource} ` + + `(${restoreError instanceof Error ? restoreError.message : String(restoreError)}).` + ); + } + if ((copyError as NodeJS.ErrnoException).code === 'EEXIST') { + throw new ArchiveBlockedError( + 'archive_target_exists', + `Archive '${path.basename(dest)}' already exists.` + ); + } + throw copyError; + } + try { + await options.verifyCopiedDestination?.(stagedSource); + await assertCopiedDirectoryUnchanged(stagedSource, dest, stagedFingerprint); + } catch (verificationError) { + await fs.rm(dest, { recursive: true, force: true }).catch(() => undefined); + try { + await fs.rename(stagedSource, src); + } catch (restoreError) { + throw new Error( + `${verificationError instanceof Error ? verificationError.message : String(verificationError)} ` + + `Could not restore the staged source at ${stagedSource} ` + + `(${restoreError instanceof Error ? restoreError.message : String(restoreError)}).` + ); + } + throw verificationError; + } + try { + await fs.rm(stagedSource, { recursive: true, force: true }); + } catch (cleanupError) { + // Recursive removal may already have deleted part of the source. The + // destination is now the only complete copy, so never erase it while + // trying to make this failed move look atomic. + throw new MoveDestinationRetainedError( + `Copied ${src} to ${dest}, but could not remove the staged source at ` + + `${stagedSource} completely ` + + `(${cleanupError instanceof Error ? cleanupError.message : String(cleanupError)}). ` + + 'The complete destination was retained for recovery.' + ); } } else { throw err; @@ -270,6 +541,466 @@ async function moveDirectory(src: string, dest: string): Promise { } } +async function assertArchiveDestinationAvailable( + archivePath: string, + archiveName: string +): Promise { + try { + await fs.lstat(archivePath); + throw new ArchiveBlockedError( + 'archive_target_exists', + `Archive '${archiveName}' already exists.` + ); + } catch (error: any) { + if (error instanceof ArchiveBlockedError) throw error; + if (error.code !== 'ENOENT') throw error; + } +} + +function archiveClaimPath(archivePath: string, _archiveName: string): string { + return path.join(path.dirname(archivePath), '.openspec-archive.lock'); +} + +interface ArchiveClaim { + handle: Awaited>; + contents: string; +} + +async function releaseArchiveClaim( + claim: ArchiveClaim, + claimPath: string +): Promise { + const owned = await claim.handle.stat({ bigint: true }).catch(() => undefined); + await claim.handle.close().catch(() => undefined); + if (owned === undefined) return; + try { + const current = await fs.lstat(claimPath, { bigint: true }); + const contents = await fs.readFile(claimPath, 'utf8'); + const currentAfterRead = await fs.lstat(claimPath, { bigint: true }); + if ( + current.dev === owned.dev && + current.ino === owned.ino && + current.dev === currentAfterRead.dev && + current.ino === currentAfterRead.ino && + contents === claim.contents + ) { + await fs.unlink(claimPath); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } +} + +async function claimArchiveDestination( + archivePath: string, + archiveName: string +): Promise { + const claimPath = archiveClaimPath(archivePath, archiveName); + try { + const handle = await fs.open(claimPath, 'wx'); + const claim = { + handle, + contents: JSON.stringify({ pid: process.pid, nonce: randomUUID() }), + }; + try { + await handle.writeFile(claim.contents); + await handle.sync(); + return claim; + } catch (error) { + await releaseArchiveClaim(claim, claimPath).catch(() => undefined); + throw error; + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') { + throw new ArchiveBlockedError( + 'archive_target_exists', + `Archive '${archiveName}' is already being created. If no archive process is running, ` + + `remove the stale claim at ${claimPath} and rerun.` + ); + } + throw error; + } +} + +interface SpecSnapshot { + target: string; + existed: boolean; + outcome: 'write' | 'retire'; + expectedContent?: Buffer; + content?: Buffer; + contentExisted?: boolean; + mode?: number; + symlink?: string; + displacedPath?: string; + displacedFingerprint?: string; +} + +interface SpecMutation { + update: SpecUpdate; + outcome: 'write' | 'retire'; + rebuilt: string; +} + +function statIdentity(value: { + dev: bigint; + ino: bigint; + mode: bigint; + size: bigint; + mtimeNs: bigint; + ctimeNs: bigint; +}): string { + return `${value.dev}:${value.ino}:${value.mode}:${value.size}:${value.mtimeNs}:${value.ctimeNs}`; +} + +function movableStatIdentity(value: { + dev: bigint; + ino: bigint; + mode: bigint; + size: bigint; +}): string { + return `${value.dev}:${value.ino}:${value.mode}:${value.size}`; +} + +async function fingerprintPath(filePath: string): Promise { + try { + const stat = await fs.lstat(filePath, { bigint: true }); + const digest = async (): Promise => + createHash('sha256').update(await fs.readFile(filePath)).digest('hex'); + if (stat.isSymbolicLink()) { + const link = await fs.readlink(filePath); + try { + const referentBefore = await fs.stat(filePath, { bigint: true }); + const hash = await digest(); + const referentAfter = await fs.stat(filePath, { bigint: true }); + const entryAfter = await fs.lstat(filePath, { bigint: true }); + if ( + statIdentity(stat) !== statIdentity(entryAfter) || + statIdentity(referentBefore) !== statIdentity(referentAfter) || + link !== (await fs.readlink(filePath)) + ) { + throw new Error(`Path changed while archive was reading ${filePath}.`); + } + return `symlink:${statIdentity(stat)}:${link}:${statIdentity(referentAfter)}:${hash}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return `symlink:${statIdentity(stat)}:${link}:missing`; + } + throw error; + } + } + if (stat.isFile()) { + const hash = await digest(); + const after = await fs.lstat(filePath, { bigint: true }); + if (statIdentity(stat) !== statIdentity(after)) { + throw new Error(`Path changed while archive was reading ${filePath}.`); + } + return `file:${statIdentity(after)}:${hash}`; + } + return `other:${statIdentity(stat)}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return 'missing'; + throw error; + } +} + +async function fingerprintMovablePath(filePath: string): Promise { + try { + const entry = await fs.lstat(filePath, { bigint: true }); + const hash = createHash('sha256') + .update(await fs.readFile(filePath)) + .digest('hex'); + if (entry.isSymbolicLink()) { + const link = await fs.readlink(filePath); + const referent = await fs.stat(filePath, { bigint: true }); + const entryAfter = await fs.lstat(filePath, { bigint: true }); + const referentAfter = await fs.stat(filePath, { bigint: true }); + const linkAfter = await fs.readlink(filePath); + if ( + statIdentity(entry) !== statIdentity(entryAfter) || + statIdentity(referent) !== statIdentity(referentAfter) || + link !== linkAfter + ) { + throw new Error(`Path changed while archive was reading ${filePath}.`); + } + return ( + `symlink:${movableStatIdentity(entry)}:${link}:` + + `${movableStatIdentity(referentAfter)}:${hash}` + ); + } + const entryAfter = await fs.lstat(filePath, { bigint: true }); + if (statIdentity(entry) !== statIdentity(entryAfter)) { + throw new Error(`Path changed while archive was reading ${filePath}.`); + } + return `file:${movableStatIdentity(entry)}:${hash}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return 'missing'; + throw error; + } +} + +async function fingerprintPortableContent(filePath: string): Promise { + try { + const entry = await fs.lstat(filePath); + const hash = createHash('sha256') + .update(await fs.readFile(filePath)) + .digest('hex'); + return entry.isSymbolicLink() + ? `symlink:${await fs.readlink(filePath)}:${hash}` + : `file:${hash}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return 'missing'; + throw error; + } +} + +/** Fail closed if the metadata authorizing a retirement leaves its snapshot. */ +async function assertRetirementAuthorization( + changeDir: string, + expectedFingerprint: string, + options: { verifyMarker?: boolean } = {} +): Promise { + const metadataPath = path.join(changeDir, METADATA_FILENAME); + const before = await fingerprintPortableContent(metadataPath); + const markerStillDeclared = + options.verifyMarker === false || readRetireCapabilitiesMarker(changeDir).declared; + const after = await fingerprintPortableContent(metadataPath); + if ( + before !== expectedFingerprint || + after !== expectedFingerprint || + !markerStillDeclared + ) { + throw new Error( + `The ${METADATA_FILENAME} retirement authorization changed before archive could complete.` + ); + } +} + +async function fingerprintSpecInputs(update: SpecUpdate): Promise { + return `${await fingerprintPath(update.source)}\n${await fingerprintPath(update.target)}`; +} + +async function mutationTargetIdentity(mutation: SpecMutation): Promise { + try { + const stat = await fs.stat(mutation.update.target, { bigint: true }); + return `${stat.dev}:${stat.ino}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + const parent = path.dirname(mutation.update.target); + const realParent = await fs.realpath(parent).catch(() => path.resolve(parent)); + return `missing:${path.join(realParent, path.basename(mutation.update.target))}`; + } + throw error; + } +} + +async function assertDistinctMutationTargets(mutations: SpecMutation[]): Promise { + const owners = new Map(); + for (const mutation of mutations) { + const identity = await mutationTargetIdentity(mutation); + const existing = owners.get(identity); + if (existing !== undefined) { + throw new Error( + `Spec updates for '${existing}' and '${mutation.update.id}' resolve to the same target ` + + `${identity}. Replace the capability alias or combine the deltas before archiving.` + ); + } + owners.set(identity, mutation.update.id); + } +} + +async function captureSpecSnapshots(mutations: SpecMutation[]): Promise { + return Promise.all( + mutations.map(async ({ update, outcome, rebuilt }) => { + try { + const stat = await fs.lstat(update.target); + if (stat.isSymbolicLink()) { + let content: Buffer | undefined; + let contentExisted = false; + if (outcome === 'write') { + try { + content = await fs.readFile(update.target); + contentExisted = true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + } + return { + target: update.target, + existed: true, + outcome, + ...(outcome === 'write' ? { expectedContent: Buffer.from(rebuilt) } : {}), + content, + contentExisted, + symlink: await fs.readlink(update.target), + }; + } + return { + target: update.target, + existed: true, + outcome, + ...(outcome === 'write' ? { expectedContent: Buffer.from(rebuilt) } : {}), + ...(stat.isFile() ? { content: await fs.readFile(update.target) } : {}), + ...(stat.isFile() ? { mode: stat.mode } : {}), + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return { + target: update.target, + existed: false, + outcome, + ...(outcome === 'write' ? { expectedContent: Buffer.from(rebuilt) } : {}), + }; + } + throw error; + } + }) + ); +} + +async function restoreSpecSnapshots(snapshots: SpecSnapshot[]): Promise { + const errors: Error[] = []; + for (const snapshot of [...snapshots].reverse()) { + try { + if (snapshot.outcome === 'retire') { + if (snapshot.displacedPath !== undefined) { + try { + await fs.lstat(snapshot.target); + throw new Error( + `Archive rollback would overwrite a concurrent change at ${snapshot.target}. ` + + `The displaced spec was retained at ${snapshot.displacedPath}.` + ); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + await fs.rename(snapshot.displacedPath, snapshot.target); + snapshot.displacedPath = undefined; + continue; + } + try { + const current = await fs.lstat(snapshot.target); + const unchangedSymlink = + snapshot.symlink !== undefined && + current.isSymbolicLink() && + (await fs.readlink(snapshot.target)) === snapshot.symlink; + const unchangedFile = + snapshot.symlink === undefined && + snapshot.content !== undefined && + current.isFile() && + (await fs.readFile(snapshot.target)).equals(snapshot.content); + if (unchangedSymlink || unchangedFile) continue; + throw new Error( + `Archive rollback would overwrite a concurrent change at ${snapshot.target}.` + ); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + } else { + let current; + try { + current = await fs.lstat(snapshot.target); + } catch (error) { + if ( + (error as NodeJS.ErrnoException).code === 'ENOENT' && + !snapshot.existed + ) { + continue; + } + throw error; + } + if ( + (snapshot.symlink !== undefined && + (!current.isSymbolicLink() || + (await fs.readlink(snapshot.target)) !== snapshot.symlink)) || + (snapshot.symlink === undefined && + (!current.isFile() || + (snapshot.mode !== undefined && current.mode !== snapshot.mode))) + ) { + throw new Error( + `Archive rollback would overwrite a concurrent change at ${snapshot.target}.` + ); + } + const currentContent = await fs.readFile(snapshot.target); + const originalContent = + snapshot.symlink !== undefined && !snapshot.contentExisted + ? undefined + : snapshot.content; + if ( + originalContent !== undefined && + currentContent.equals(originalContent) + ) { + continue; + } + if ( + snapshot.expectedContent === undefined || + !currentContent.equals(snapshot.expectedContent) + ) { + throw new Error( + `Archive rollback would overwrite a concurrent change at ${snapshot.target}.` + ); + } + } + + if (!snapshot.existed) { + await fs.rm(snapshot.target, { force: true }); + continue; + } + if (snapshot.symlink !== undefined) { + if (snapshot.outcome === 'retire') { + await fs.mkdir(path.dirname(snapshot.target), { recursive: true }); + await fs.symlink(snapshot.symlink, snapshot.target); + } else if (snapshot.contentExisted) { + await fs.writeFile(snapshot.target, snapshot.content!); + } else { + const referent = path.resolve(path.dirname(snapshot.target), snapshot.symlink); + await fs.rm(referent, { force: true }); + } + continue; + } + if (snapshot.content !== undefined) { + await fs.mkdir(path.dirname(snapshot.target), { recursive: true }); + await fs.writeFile(snapshot.target, snapshot.content); + if (snapshot.mode !== undefined) await fs.chmod(snapshot.target, snapshot.mode); + } + } catch (error) { + errors.push(error instanceof Error ? error : new Error(String(error))); + } + } + if (errors.length > 0) { + throw new Error(errors.map(({ message }) => message).join(' ')); + } +} + +async function finalizeRetirementBackups( + snapshots: SpecSnapshot[], + mainSpecsDir: string +): Promise { + const errors: string[] = []; + for (const snapshot of snapshots) { + if (snapshot.outcome !== 'retire' || snapshot.displacedPath === undefined) continue; + const displacedPath = snapshot.displacedPath; + try { + if ( + snapshot.displacedFingerprint === undefined || + (await fingerprintMovablePath(displacedPath)) !== snapshot.displacedFingerprint + ) { + throw new Error('the displaced spec changed after retirement verification'); + } + await finalizeRetiredSpec(snapshot.target, displacedPath, mainSpecsDir); + snapshot.displacedPath = undefined; + } catch (error) { + errors.push( + `Could not remove the committed retirement backup at ${displacedPath} ` + + `(${error instanceof Error ? error.message : String(error)}).` + ); + } + } + if (errors.length > 0) { + throw new RetirementBackupsRetainedError( + `${errors.join(' ')} The change remains archived and each listed backup was retained for recovery.` + ); + } +} + export class ArchiveCommand { async execute(changeName?: string, options: ArchiveOptions = {}): Promise { const json = !!options.json; @@ -376,11 +1107,18 @@ export class ArchiveCommand { // Verify change exists try { - const stat = await fs.stat(changeDir); + const stat = await fs.lstat(changeDir); + if (stat.isSymbolicLink()) { + throw new ArchiveBlockedError( + 'archive_change_symlink', + `Change '${changeName}' is a symbolic link. Replace it with a real directory before archiving.` + ); + } if (!stat.isDirectory()) { throw new Error(`Change '${changeName}' not found.`); } - } catch { + } catch (error) { + if (error instanceof ArchiveBlockedError) throw error; const available = await listActiveChangeNames(changesDir); throw new ArchiveBlockedError( 'archive_change_not_found', @@ -583,11 +1321,41 @@ export class ArchiveCommand { } } - // Handle spec updates unless skipSpecs flag is set - let specsUpdated = false; - let totals: ArchiveResult['totals']; - const specWarnings: string[] = []; - if (options.skipSpecs) { + // Settle the archive destination BEFORE touching any spec. The name depends + // only on the change, and a collision is routine (archiving twice in a day, + // a restored change), so discovering it after the merge would leave specs + // rewritten - or a capability retired - for an archive that never happened. + // + // Names that already carry a date prefix keep it: re-prefixing would stutter + // the name, and when the archive runs on a later day the folder would sort + // under a day on which the change did not happen (#1309). + const archiveName = ARCHIVE_DATE_PREFIX_PATTERN.test(changeName) + ? changeName + : `${formatLocalDate()}-${changeName}`; + const archivePath = path.join(archiveDir, archiveName); + + // Read once, before any spec is touched: whether this change is allowed to + // retire a capability at all. An unhonorable marker counts as undeclared, + // exactly as skip_specs treats one, so metadata the rest of the CLI rejects + // can never authorise a deletion. + const retirementMarker = readRetireCapabilitiesMarker(changeDir); + const retirementDeclared = retirementMarker.declared; + const retirementAuthorizationFingerprint = retirementDeclared + ? await fingerprintPortableContent(path.join(changeDir, METADATA_FILENAME)) + : undefined; + + await assertArchiveDestinationAvailable(archivePath, archiveName); + await fs.mkdir(archiveDir, { recursive: true }); + const claimPath = archiveClaimPath(archivePath, archiveName); + let archiveClaim: ArchiveClaim | undefined; + + try { + // Handle spec updates unless skipSpecs flag is set + let specsUpdated = false; + let totals: ArchiveResult['totals']; + const specWarnings: string[] = []; + let changeArchived = false; + if (options.skipSpecs) { if (!json) { console.log('Skipping spec updates (--skip-specs flag provided).'); } @@ -612,12 +1380,47 @@ export class ArchiveCommand { update: SpecUpdate; rebuilt: string; counts: { added: number; modified: number; removed: number; renamed: number }; + outcome: SpecOutcome; + noRequirementBlocks: boolean; + unaccountedContent: string[]; + sourceFingerprint: string; + sourceContentFingerprint: string; + targetFingerprint: string; + targetMovableFingerprint: string; }> = []; let prepareError: unknown; try { for (const update of specUpdates) { + const sourceBeforeBuild = await fingerprintPath(update.source); + const targetBeforeBuild = await fingerprintPath(update.target); const built = await buildUpdatedSpec(update, changeName!, { silent: true }); - prepared.push({ update, rebuilt: built.rebuilt, counts: built.counts }); + const sourceAfterBuild = await fingerprintPath(update.source); + const targetAfterBuild = await fingerprintPath(update.target); + if ( + sourceBeforeBuild !== sourceAfterBuild || + targetBeforeBuild !== targetAfterBuild + ) { + throw new Error( + `Spec inputs for '${update.id}' changed while archive was preparing the preview.` + ); + } + prepared.push({ + update, + rebuilt: built.rebuilt, + counts: built.counts, + outcome: await decideSpecOutcome( + update, + built, + skipValidation, + retirementDeclared + ), + noRequirementBlocks: built.noRequirementBlocks, + unaccountedContent: built.unaccountedContent, + sourceFingerprint: sourceAfterBuild, + sourceContentFingerprint: await fingerprintPortableContent(update.source), + targetFingerprint: targetAfterBuild, + targetMovableFingerprint: await fingerprintMovablePath(update.target), + }); specWarnings.push(...built.warnings); } } catch (err: unknown) { @@ -658,6 +1461,66 @@ export class ArchiveCommand { } if (shouldUpdateSpecs) { + // The confirmation may stay open while another editor changes a main + // spec. Never apply the proposal built before the prompt to a newer + // baseline: in particular, a stale retirement decision must not + // delete a requirement added while the prompt was waiting. + if (prepareError === undefined) { + try { + const currentRetirementMarker = readRetireCapabilitiesMarker(changeDir); + if ( + currentRetirementMarker.declared !== retirementMarker.declared || + currentRetirementMarker.invalidReason !== retirementMarker.invalidReason + ) { + throw new Error( + `The ${METADATA_FILENAME} retirement authorization changed while archive was awaiting confirmation.` + ); + } + const currentUpdates = await findSpecUpdates(changeDir, mainSpecsDir); + const currentById = new Map(currentUpdates.map((update) => [update.id, update])); + if (currentUpdates.length !== prepared.length) { + throw new Error('The change specs changed while archive was awaiting confirmation.'); + } + for (const proposed of prepared) { + const current = currentById.get(proposed.update.id); + if (!current) { + throw new Error( + `The delta for '${proposed.update.id}' changed while archive was awaiting confirmation.` + ); + } + if ( + (await fingerprintPath(current.source)) !== proposed.sourceFingerprint || + (await fingerprintPath(current.target)) !== proposed.targetFingerprint + ) { + throw new Error( + `Spec inputs for '${proposed.update.id}' changed while archive was awaiting confirmation. ` + + 'No files were changed; review the new content and rerun.' + ); + } + const rebuilt = await buildUpdatedSpec(current, changeName!, { silent: true }); + const outcome = await decideSpecOutcome( + current, + rebuilt, + skipValidation, + retirementDeclared + ); + if ( + current.exists !== proposed.update.exists || + rebuilt.rebuilt !== proposed.rebuilt || + JSON.stringify(rebuilt.counts) !== JSON.stringify(proposed.counts) || + outcome !== proposed.outcome + ) { + throw new Error( + `Main spec '${proposed.update.id}' changed while archive was awaiting confirmation. ` + + 'No files were changed; review the new content and rerun.' + ); + } + } + } catch (error) { + prepareError = error; + } + } + if (prepareError !== undefined) { const message = prepareError instanceof Error ? prepareError.message : String(prepareError); @@ -674,18 +1537,60 @@ export class ArchiveCommand { return null; } - // Validate every rebuilt spec before writing any of them, so a late - // validation failure really does leave all targets unchanged. + // Validate every rebuilt spec before writing any of them, so a + // late validation failure really does leave all targets unchanged. if (!skipValidation) { for (const p of prepared) { + // A retirement was already put to the validator, and failed on + // nothing but "no requirements" - there is no spec left to write, + // so re-reporting that one error would just abort the fix (#1302). + if (p.outcome !== 'write') continue; const specName = p.update.id; const report = await new Validator().validateSpecContent(specName, p.rebuilt); if (!report.valid) { + // The dead end #1302 describes: the rebuilt spec is unwritable + // for exactly one reason, and retiring the capability is the + // fix - but only the author can authorise deleting the spec, so + // the abort names the marker instead of just rejecting. Says so + // only when the marker is the ONLY thing missing, so it never + // sends someone after a marker that would not have helped. + const retirementWouldFix = + !retirementDeclared && + p.update.exists && + p.counts.removed > 0 && + (await isRetirementCandidate(p.update, p, false)); + const retirementHint = retirementWouldFix + ? `This change removes the last requirement '${specName}' has. To retire the` + + ` capability and delete its spec, add \`retire_capabilities: true\` to the` + + ` change's ${METADATA_FILENAME} (alongside its \`schema:\`, which that file` + + ` requires), then rerun.` + + (retirementMarker.invalidReason + ? ` The marker present now cannot be honored (${retirementMarker.invalidReason}).` + : '') + : undefined; + // The marker was set and retirement was still refused. Saying + // nothing left the author who did exactly what the docs asked + // back in the original dead end with no signal that their + // marker had been read at all. + // The author asked for a retirement and got the bare + // validation abort. Name the lines that stood in the way. + const refusalReason = + retirementDeclared && + p.unaccountedContent.length > 0 && + (await isRetirableSpec(specName, p.rebuilt)) + ? `'${specName}' declares retire_capabilities, but the spec holds content the merge ` + + `cannot safely account for and deleting the file would take with it: ` + + `${p.unaccountedContent.slice(0, 3).map((line) => `"${line}"`).join(', ')}` + + `${p.unaccountedContent.length > 3 ? `, and ${p.unaccountedContent.length - 3} more line(s)` : ''}. ` + + 'Move it into `## Purpose` or a canonical requirement, or delete the spec by hand.' + : undefined; if (json) { throw new ArchiveBlockedError( 'archive_spec_validation_failed', `Rebuilt spec for '${specName}' failed validation. No files were changed.`, - `Run ${withStoreFlag(root, `openspec validate ${specName}`)} after fixing the change deltas.` + refusalReason ?? + retirementHint ?? + `Run ${withStoreFlag(root, `openspec validate ${specName}`)} after fixing the change deltas.` ); } console.log(chalk.red(`\nValidation errors in rebuilt spec for ${specName} (will not write changes):`)); @@ -693,6 +1598,8 @@ export class ArchiveCommand { if (issue.level === 'ERROR') console.log(chalk.red(` ✗ ${issue.message}`)); else if (issue.level === 'WARNING') console.log(chalk.yellow(` ⚠ ${issue.message}`)); } + if (retirementHint) console.log(chalk.yellow(` → ${retirementHint}`)); + if (refusalReason) console.log(chalk.yellow(` → ${refusalReason}`)); console.log('Aborted. No files were changed.'); process.exitCode = 1; return null; @@ -700,10 +1607,51 @@ export class ArchiveCommand { } } - // All validations passed; write files and display counts - const writeTotals = { added: 0, modified: 0, removed: 0, renamed: 0 }; - let wroteAny = false; + // A legitimate concurrent archive cannot pass the exclusive claim, + // while this catches an external process that created the final + // destination during a confirmation prompt. Check before the first + // spec mutation so a collision never strands a write or retirement. + await assertArchiveDestinationAvailable(archivePath, archiveName); + archiveClaim = await claimArchiveDestination(archivePath, archiveName); + await assertArchiveDestinationAvailable(archivePath, archiveName); + const mutations = prepared + .filter( + ({ outcome, counts }) => + outcome === 'retire' || + (outcome === 'write' && + counts.added + counts.modified + counts.removed + counts.renamed > 0) + ) + .map(({ update, outcome, rebuilt }) => ({ + update, + outcome: outcome as 'write' | 'retire', + rebuilt, + })); + const hasRetirements = mutations.some(({ outcome }) => outcome === 'retire'); + await assertDistinctMutationTargets(mutations); + for (const proposed of prepared) { + if ( + (await fingerprintPath(proposed.update.source)) !== proposed.sourceFingerprint || + (await fingerprintPath(proposed.update.target)) !== proposed.targetFingerprint + ) { + throw new Error( + `Spec inputs for '${proposed.update.id}' changed before archive could apply them. ` + + 'No files were changed; review the new content and rerun.' + ); + } + } + const specSnapshots = await captureSpecSnapshots(mutations); + const specSnapshotsByTarget = new Map( + specSnapshots.map((snapshot) => [snapshot.target, snapshot]) + ); + + const mutationAttempts = new Set(); + try { + // All validations passed; write files and display counts + const writeTotals = { added: 0, modified: 0, removed: 0, renamed: 0 }; + let wroteAny = false; for (const p of prepared) { + // Deletions are deferred to the loop below. + if (p.outcome !== 'write') continue; const { added, modified, removed, renamed } = p.counts; if (added + modified + removed + renamed === 0) { // Every operation was already synced: rewriting the file would @@ -712,6 +1660,17 @@ export class ArchiveCommand { } await writeUpdatedSpec(p.update, p.rebuilt, p.counts, { silent: json, + beforeMutate: async () => { + if ( + (await fingerprintSpecInputs(p.update)) !== + `${p.sourceFingerprint}\n${p.targetFingerprint}` + ) { + throw new Error( + `Spec inputs for '${p.update.id}' changed before archive could write them.` + ); + } + mutationAttempts.add(p.update.target); + }, // Cross-root paths must be absolute when a store is selected. ...(isStoreSelectedRoot(root) ? { displayPath: p.update.target } : {}), }); @@ -721,9 +1680,138 @@ export class ArchiveCommand { writeTotals.removed += removed; writeTotals.renamed += renamed; } - specsUpdated = wroteAny; - totals = writeTotals; - if (!json) { + + // Retirements run only after every write has succeeded. If any + // later mutation fails, the snapshots below restore every target. + for (const p of prepared) { + if (p.outcome !== 'retire') continue; + const { retired, resolvedPath, displacedPath } = await retireSpec( + p.update, + mainSpecsDir, + { + silent: json, + deferDelete: true, + beforeMutate: async () => { + if (retirementAuthorizationFingerprint === undefined) { + throw new Error( + `The ${METADATA_FILENAME} retirement authorization is unavailable.` + ); + } + await assertRetirementAuthorization( + changeDir, + retirementAuthorizationFingerprint + ); + if ( + (await fingerprintSpecInputs(p.update)) !== + `${p.sourceFingerprint}\n${p.targetFingerprint}` + ) { + throw new Error( + `Spec inputs for '${p.update.id}' changed before archive could retire them.` + ); + } + mutationAttempts.add(p.update.target); + }, + verifyDisplaced: async (displacedPath) => { + await assertRetirementAuthorization( + changeDir, + retirementAuthorizationFingerprint! + ); + if ( + (await fingerprintMovablePath(displacedPath)) !== + p.targetMovableFingerprint + ) { + throw new Error( + `Main spec '${p.update.id}' changed while archive was securing it for retirement.` + ); + } + }, + ...(isStoreSelectedRoot(root) ? { displayPath: p.update.target } : {}), + } + ); + if (!retired) continue; + const retirementSnapshot = specSnapshotsByTarget.get(p.update.target); + if (retirementSnapshot === undefined || displacedPath === undefined) { + throw new Error( + `Could not track the displaced main spec for '${p.update.id}' during retirement.` + ); + } + retirementSnapshot.displacedPath = displacedPath; + retirementSnapshot.displacedFingerprint = p.targetMovableFingerprint; + wroteAny = true; + // A rename applied on the way to the retirement still happened; + // folding every count in keeps the totals honest about the whole + // delta. + writeTotals.added += p.counts.added; + writeTotals.modified += p.counts.modified; + writeTotals.removed += p.counts.removed; + writeTotals.renamed += p.counts.renamed; + // Deleting a file is the one archive outcome a JSON consumer cannot + // infer from the totals, so it is recorded the way every other + // spec-merge divergence is. Purpose always goes with the file, so it + // is named too rather than left to the reader to work out, and the + // note carries the command that brings the file back. + const lost = ['Purpose']; + // Derived from the path that was unlinked, never rebuilt from the + // capability id: on a case-insensitive filesystem the id and the + // real directory can differ in case, and git is case-sensitive, so + // an id-derived path is one git rejects. + // `update.target` is built from the capability id, so on a + // case-insensitive filesystem it can differ in case from the file + // that was actually unlinked - and git is case-sensitive, so the + // printed command is one git rejects. A capability directory + // symlinked to a sibling has the same problem without leaving the + // tree. `retiredPath` carries the resolved path, so it wins + // whenever it disagrees, not only when it escapes. + const unlinkedPath = resolvedPath ?? p.update.target; + // Measured against the REAL root, so the platform's own + // `/var` -> `/private/var` link does not read as an escape. A path + // that genuinely sits outside stays absolute, which is what routes + // it to prose guidance instead of a command git would reject. + const realRoot = await fs.realpath(root.path).catch(() => root.path); + const relativeToRoot = path.relative(realRoot, unlinkedPath); + const insideRoot = + relativeToRoot !== '' && + !relativeToRoot.startsWith('..') && + !path.isAbsolute(relativeToRoot); + const deletedPath = + isStoreSelectedRoot(root) || !insideRoot + ? unlinkedPath + : relativeToRoot.split(path.sep).join('/'); + // A command is offered only when pasting it where archive was run + // would actually work. An absolute path here means the file did not + // live under that directory - a selected store, or a symlinked + // capability directory - and `git checkout HEAD -- ` is rejected + // from a different worktree however it is quoted, so that case gets + // guidance instead of a command that cannot run. A path with no + // portable shell spelling is handled the same way. + // + // Conditional on purpose, too: whether the file is in `HEAD` is not + // something archive knows - a spec an earlier archive CREATED and + // nobody has committed yet is not - and promising recovery is the one + // claim this feature must not get wrong. + const pasteablePath = path.isAbsolute(deletedPath) + ? undefined + : quoteForShell(`:(top)${deletedPath}`); + const recovery = pasteablePath + ? `If it was committed, restore it with: git checkout HEAD -- ${pasteablePath}` + : `It was deleted from ${deletedPath}; if it was committed, restore it from that checkout's history.`; + const retirementNote = + `${p.update.id} - capability retired; deleted the main spec (all requirements removed` + + `, declared by retire_capabilities) at ${deletedPath}` + + `. Its section(s) went with it: ${lost.join(', ')}. ` + + recovery; + specWarnings.push(retirementNote); + // The "Retiring ..." line already told a human the file is gone; the + // sections it took along, and how to get them back, are the parts + // they cannot see from the path. + if (!json) { + console.log(` ${recovery}`); + } + } + + specsUpdated = wroteAny; + totals = writeTotals; + if (!json) { console.log( `Totals: + ${writeTotals.added}, ~ ${writeTotals.modified}, - ${writeTotals.removed}, → ${writeTotals.renamed}` ); @@ -732,52 +1820,161 @@ export class ArchiveCommand { ? 'Specs updated successfully.' : 'Specs already in sync; no files changed.' ); + } + + for (const proposed of prepared) { + if ( + (await fingerprintPath(proposed.update.source)) !== + proposed.sourceFingerprint + ) { + throw new Error( + `The delta for '${proposed.update.id}' changed before the change could be archived.` + ); + } + } + if (hasRetirements) { + await assertRetirementAuthorization( + changeDir, + retirementAuthorizationFingerprint! + ); + } + const verifyArchivedDeltas = async ( + stagedSource?: string + ): Promise => { + if (hasRetirements) { + await assertRetirementAuthorization( + archivePath, + retirementAuthorizationFingerprint!, + // Archived changes are nested one level deeper than active + // changes, so the marker reader cannot resolve their schema. + // Exact content equality proves this is the authorization + // already validated at the active path. + { verifyMarker: false } + ); + if (stagedSource) { + await assertRetirementAuthorization( + stagedSource, + retirementAuthorizationFingerprint! + ); + } + } + for (const proposed of prepared) { + const archivedSource = path.join( + archivePath, + path.relative(changeDir, proposed.update.source) + ); + if ( + (await fingerprintPortableContent(archivedSource)) !== + proposed.sourceContentFingerprint + ) { + throw new Error( + `The archived delta for '${proposed.update.id}' changed during the final move.` + ); + } + if (stagedSource) { + const stagedDelta = path.join( + stagedSource, + path.relative(changeDir, proposed.update.source) + ); + if ( + (await fingerprintPortableContent(stagedDelta)) !== + proposed.sourceContentFingerprint + ) { + throw new Error( + `The active delta for '${proposed.update.id}' changed during the fallback copy.` + ); + } + } + } + }; + await moveDirectory(changeDir, archivePath, { + verifyCopiedDestination: verifyArchivedDeltas, + }); + changeArchived = true; + await verifyArchivedDeltas(); + await finalizeRetirementBackups(specSnapshots, mainSpecsDir); + } catch (error) { + if (error instanceof MoveDestinationRetainedError) { + changeArchived = true; + try { + await finalizeRetirementBackups(specSnapshots, mainSpecsDir); + } catch (cleanupError) { + throw new RetirementBackupsRetainedError( + `${error.message} ${ + cleanupError instanceof Error ? cleanupError.message : String(cleanupError) + }` + ); + } + throw error; + } + if (error instanceof RetirementBackupsRetainedError) throw error; + const rollbackErrors: Error[] = []; + try { + await restoreSpecSnapshots( + specSnapshots.filter(({ target }) => mutationAttempts.has(target)) + ); + } catch (rollbackError) { + rollbackErrors.push( + rollbackError instanceof Error + ? rollbackError + : new Error(String(rollbackError)) + ); + } + if (changeArchived) { + try { + await moveDirectory(archivePath, changeDir); + changeArchived = false; + } catch (rollbackError) { + rollbackErrors.push( + rollbackError instanceof Error + ? rollbackError + : new Error(String(rollbackError)) + ); + } + } + if (rollbackErrors.length > 0) { + const original = error instanceof Error ? error.message : String(error); + throw new Error( + `${original} Rollback also failed: ${rollbackErrors.map(({ message }) => message).join(' ')}` + ); + } + throw error; } } } } - // Create archive directory with date prefix. Names that already carry - // one keep it: re-prefixing would stutter the name, and when the archive - // runs on a later day the folder would sort under a day on which the - // change did not happen (#1309). - const archiveName = ARCHIVE_DATE_PREFIX_PATTERN.test(changeName) - ? changeName - : `${formatLocalDate()}-${changeName}`; - const archivePath = path.join(archiveDir, archiveName); + // The destination was checked before the merge, so anything claiming it now + // appeared while we were working. Report that as the collision it is: a raw + // ENOTEMPTY from rename would otherwise degrade to a bare `archive_error`. + if (!changeArchived) { + await assertArchiveDestinationAvailable(archivePath, archiveName); + archiveClaim = await claimArchiveDestination(archivePath, archiveName); + await assertArchiveDestinationAvailable(archivePath, archiveName); - // Check if archive already exists - let archiveExists = false; - try { - await fs.access(archivePath); - archiveExists = true; - } catch (error: any) { - if (error.code !== 'ENOENT') { - throw error; - } - } - if (archiveExists) { - throw new ArchiveBlockedError('archive_target_exists', `Archive '${archiveName}' already exists.`); - } + // Create archive directory if needed + await fs.mkdir(archiveDir, { recursive: true }); - // Create archive directory if needed - await fs.mkdir(archiveDir, { recursive: true }); + // Move change to archive (uses copy+remove on EPERM/EXDEV, e.g. Windows) + await moveDirectory(changeDir, archivePath); + changeArchived = true; + } - // Move change to archive (uses copy+remove on EPERM/EXDEV, e.g. Windows) - await moveDirectory(changeDir, archivePath); + if (!json) { + console.log(`Change '${changeName}' archived as '${archiveName}'.`); + } - if (!json) { - console.log(`Change '${changeName}' archived as '${archiveName}'.`); + return { + change: changeName, + archivedAs: archiveName, + path: archivePath, + specsUpdated, + ...(totals ? { totals } : {}), + ...(specWarnings.length > 0 ? { warnings: specWarnings } : {}), + }; + } finally { + if (archiveClaim) await releaseArchiveClaim(archiveClaim, claimPath).catch(() => undefined); } - - return { - change: changeName, - archivedAs: archiveName, - path: archivePath, - specsUpdated, - ...(totals ? { totals } : {}), - ...(specWarnings.length > 0 ? { warnings: specWarnings } : {}), - }; } private async selectChange( diff --git a/src/core/change-metadata/schema.ts b/src/core/change-metadata/schema.ts index 40c231d409..3644160052 100644 --- a/src/core/change-metadata/schema.ts +++ b/src/core/change-metadata/schema.ts @@ -39,6 +39,13 @@ export const ChangeMetadataSchema = z.object({ // complete - that path prefix, not the artifact id, is the contract custom // schemas inherit. skip_specs: z.boolean().optional(), + // Declares that this change may retire a capability: when its REMOVED entries + // take the last requirement a capability has, archive deletes that + // capability's main spec instead of aborting on a spec it could not write + // (#1302). Required because the deletion is not recoverable from the working + // tree - only from git - so it is the author's call, not an inference from the + // shape of a delta. + retire_capabilities: z.boolean().optional(), }); export type ChangeMetadata = z.infer; diff --git a/src/core/parsers/spec-structure.ts b/src/core/parsers/spec-structure.ts index 17a9be0bd9..3443836f74 100644 --- a/src/core/parsers/spec-structure.ts +++ b/src/core/parsers/spec-structure.ts @@ -6,7 +6,7 @@ const DELTA_HEADER = /^##\s+(ADDED|MODIFIED|REMOVED|RENAMED)\s+Requirements\s*$/ const REQUIREMENT_HEADER = /^###\s+Requirement:\s*(.+)\s*$/i; export interface MainSpecStructureIssue { - kind: 'delta-header' | 'requirement-outside-requirements'; + kind: 'delta-header' | 'requirement-outside-requirements' | 'duplicate-requirement'; line: number; header: string; message: string; @@ -17,6 +17,7 @@ export function findMainSpecStructureIssues(content: string): MainSpecStructureI const stripped = stripFencedCodeBlocksPreservingLines(normalized); const lines = stripped.split('\n'); const issues: MainSpecStructureIssue[] = []; + const requirementLines = new Map(); const requirementsHeaderIndex = lines.findIndex(line => REQUIREMENTS_SECTION_HEADER.test(line)); let requirementsEndIndex = lines.length; @@ -69,6 +70,22 @@ export function findMainSpecStructureIssues(content: string): MainSpecStructureI `Requirement header "${trimmed}" appears outside the main ## Requirements section. ` + 'Main specs only parse requirements inside that section, so this requirement is currently invisible to validate, list, and archive.', }); + continue; + } + + const requirementName = requirementMatch[1].trim(); + const previousLine = requirementLines.get(requirementName); + if (previousLine !== undefined) { + issues.push({ + kind: 'duplicate-requirement', + line: i + 1, + header: trimmed, + message: + `Requirement header "${trimmed}" duplicates the requirement declared on line ${previousLine}. ` + + 'Requirement names must be unique so spec updates cannot discard one block while updating another.', + }); + } else { + requirementLines.set(requirementName, i + 1); } } diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 956aca8b88..91b9043e7e 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -6,6 +6,7 @@ */ import { promises as fs } from 'fs'; +import { randomUUID } from 'crypto'; import path from 'path'; import chalk from 'chalk'; import { @@ -15,6 +16,7 @@ import { parseDeltaSpec, normalizeRequirementName, type RequirementBlock, + type RequirementsSectionParts, } from './parsers/requirement-blocks.js'; import { findMainSpecStructureIssues } from './parsers/spec-structure.js'; import { buildCodeFenceMask } from './parsers/code-fence.js'; @@ -140,6 +142,40 @@ export async function buildUpdatedSpec( rebuilt: string; counts: { added: number; modified: number; removed: number; renamed: number }; warnings: string[]; + /** + * Every canonical `### Requirement:` block the delta could act on is gone. + * This is only a *candidate* signal for retirement (#1302): the validator, not + * this count, decides whether `rebuilt` is actually unwritable - it recognises + * requirement shapes this parser sweeps into the preamble, so a spec can be + * blockless here and still validate. See `isRetirableSpec` in archive.ts. + */ + noRequirementBlocks: boolean; + /** + * Every non-blank line of the spec this merge cannot name. + * + * Retirement deletes the whole file, so the only safe question is whether the + * merge can account for all of it. `extractRequirementsSection` splits a spec + * into five slices, and auditing a subset is how this guard kept failing: for + * seven rounds it looked for requirement-SHAPED text and was beaten by a new + * disguise each time, and when it started asking where content landed it + * still read only the preamble and the tail - so content simply moved into a + * slice nobody checked, and authored prose sitting inside a removed block's + * raw was deleted while the report said only "Purpose" was lost. + * + * So this accounts for the whole file: the title, the `## Purpose` section, + * the `## Requirements` header, and, inside each requirement block, the parts + * that make up a requirement - its header, its statement, and its scenarios' + * bullets. Every other non-blank line is reported and refuses the retirement. + * + * Fails safe in every direction: a line this cannot classify counts as + * unaccounted, which refuses rather than deletes. + */ + unaccountedContent: string[]; + /** + * Authored `## ` sections other than Purpose and Requirements. Retirement + * deletes the whole file, so callers name these rather than discarding + * hand-written prose silently. + */ }> { // Collected so silent (JSON) callers can surface them; printed live for // human callers at the point they occur. @@ -523,6 +559,13 @@ export async function buildUpdatedSpec( renamed: renamedApplied, }, warnings, + noRequirementBlocks: keptOrder.length === 0, + // Read off the ORIGINAL requirements section, not the rebuilt one. Anything + // after the last `### Requirement:` header belongs to that block's raw and + // is discarded with it, so a rebuilt-body scan only ever sees headings above + // the first requirement - it would veto `### Notes` written before the + // requirements and miss the identical heading written after them. + unaccountedContent: contentTheMergeCannotName(parts), }; } @@ -554,6 +597,140 @@ function firstForeignTail(raw: string): { heading: string; raw: string } | undef return undefined; } +/** + * 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, + * and each requirement block's own header, statement and scenario bullets. + * + * Deliberately whole-file. Auditing a subset of the slices is what let authored + * prose inside a removed block, and content above the requirements section, be + * deleted unmentioned. + */ +function contentTheMergeCannotName(parts: RequirementsSectionParts): string[] { + const leftovers: string[] = []; + + // Above the requirements section: the title and the Purpose section are + // expected; anything else is authored content the deletion would take. + const beforeLines = parts.before.replace(/^\uFEFF/, '').replace(/\r\n?/g, '\n').split('\n'); + const beforeMask = buildCodeFenceMask(beforeLines); + let inPurpose = false; + let titleSeen = false; + let previousLine = ''; + for (let index = 0; index < beforeLines.length; index++) { + const line = beforeLines[index]; + if (!line.trim()) { + previousLine = ''; + continue; + } + if (!beforeMask[index]) { + const section = line.match(/^ {0,3}##\s+(.+?)\s*$/); + if (section) { + inPurpose = /^purpose$/i.test(section[1].trim()); + if (!inPurpose) leftovers.push(line.trim()); + previousLine = line; + continue; + } + // `##` is not the only way to open a section. A setext underline turns + // the line above it into a heading, and raw HTML says so outright - a + // reader sees a sibling of `## Purpose`, not more of its body. Treating + // everything up to the next ATX `##` as Purpose swallowed those whole and + // deleted them, reported as nothing but "Purpose". + const setext = inPurpose && previousLine.trim() && /^ {0,3}(=+|-+)\s*$/.test(line); + const htmlHeading = /^ {0,3} 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; + bulletsSeen = false; + continue; + } + if (/^\s*(?:[-*]|\d+[.)])\s/.test(line)) { + if (inScenarioBullets) { + bulletsSeen = true; + continue; + } + // A bullet outside a scenario. Before the first scenario it is part of + // the requirement statement; after one it is the author's own note. + if (!seenScenario) continue; + leftovers.push(line.trim()); + continue; + } + // Free prose above the first scenario is the requirement statement. + if (!seenScenario && !/^\s*[|<]/.test(line)) continue; + leftovers.push(line.trim()); + } + } + + return [...new Set(leftovers)]; +} + function normalizeBlockRaw(raw: string): string { return raw.replace(/\r\n?/g, '\n').trim(); } @@ -570,6 +747,213 @@ function countOccurrences(haystack: string, needle: string): number { return count; } +/** + * Retire a capability whose last requirement a delta removed: delete its main + * spec and prune any directories the deletion leaves empty. Returns false when + * there was nothing to delete. + * + * Gated by the caller on the change's `retire_capabilities` marker, so the one + * archive action that removes a file from `openspec/specs/` is always something + * the author asked for rather than something inferred from a delta's shape. The + * file is recoverable from git, which the report names; applying REMOVED already + * deletes requirement content from a main spec, so deleting the spec once + * nothing is left is the same operation carried to its end rather than a new + * kind of act. + * + * Only the generated `spec.md` is removed - a directory holding anything else (a + * nested capability, a hand-kept note) is left in place. + * + * The target must resolve inside the selected specs root. A capability-directory + * symlink must not turn a retirement marker into authorization to delete an + * unrelated external file. A symlinked `spec.md` itself is safe: unlink removes + * the link and leaves its target alone. + * + * Directory pruning IS bounded, by REAL paths rather than string prefixes: + * `path.resolve` collapses `..` but does not resolve symlinks, and `readdir` and + * `rmdir` both follow them, so a symlinked capability directory would otherwise + * let the walk delete directories outside the specs root entirely. + */ +export async function retireSpec( + update: SpecUpdate, + mainSpecsDir: string, + options: { + silent?: boolean; + displayPath?: string; + beforeMutate?: () => Promise; + verifyDisplaced?: (displacedPath: string) => Promise; + deferDelete?: boolean; + } = {} +): Promise<{ retired: boolean; resolvedPath?: string; displacedPath?: string }> { + if (options.deferDelete && options.verifyDisplaced === undefined) { + throw new Error('Deferred retirement requires displaced-file verification.'); + } + // Resolved before the unlink, while the link still exists, so the report can + // name the file that actually goes when a symlink points out of the tree. + // A symlinked `spec.md` is excluded: `realpath` would follow it, but `unlink` + // removes the link and leaves the target alone, so naming the target would + // claim a file was deleted that is still there. + let realSource: string | undefined; + try { + const link = await fs.lstat(update.target); + realSource = link.isSymbolicLink() ? undefined : await fs.realpath(update.target); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return { retired: false }; + throw new Error( + `Could not retire capability '${update.id}': could not verify ${update.target} ` + + `before deletion (${error instanceof Error ? error.message : String(error)}).` + ); + } + + if (realSource !== undefined) { + let inside: boolean; + try { + inside = await isInsideRealDir(realSource, mainSpecsDir); + } catch (error) { + throw new Error( + `Could not retire capability '${update.id}': could not verify that ${update.target} ` + + `is inside ${mainSpecsDir} (${error instanceof Error ? error.message : String(error)}).` + ); + } + if (!inside) { + throw new Error( + `Could not retire capability '${update.id}': ${update.target} resolves outside ` + + `${mainSpecsDir}. Remove the external file by hand, or replace the symlink and rerun.` + ); + } + } + + let displacedPath: string | undefined; + try { + await options.beforeMutate?.(); + if (options.verifyDisplaced) { + const displaced = `${update.target}.openspec-retire-${randomUUID()}`; + displacedPath = displaced; + await fs.rename(update.target, displaced); + try { + await options.verifyDisplaced(displaced); + try { + await fs.lstat(update.target); + throw new Error( + `A concurrent file appeared at ${update.target} while archive was retiring it.` + ); + } catch (targetError) { + if ((targetError as NodeJS.ErrnoException).code !== 'ENOENT') throw targetError; + } + if (!options.deferDelete) await fs.unlink(displaced); + } catch (error) { + try { + await fs.lstat(update.target); + throw new Error( + `${error instanceof Error ? error.message : String(error)} ` + + `A concurrent file now occupies ${update.target}; the displaced spec was retained at ${displaced}.` + ); + } catch (targetError) { + if ((targetError as NodeJS.ErrnoException).code !== 'ENOENT') throw targetError; + } + await fs.rename(displaced, update.target); + throw error; + } + } else { + await fs.unlink(update.target); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return { retired: false }; + // A bare errno here reads as an internal failure; say what was being + // attempted so the message is actionable on its own. + throw new Error( + `Could not retire capability '${update.id}': failed to delete ${update.target} ` + + `(${(error as Error).message}). Remove it by hand, then rerun the archive.` + ); + } + + if (!options.deferDelete) { + await pruneEmptyDirs(path.dirname(update.target), mainSpecsDir); + } + + const nominal = options.displayPath ?? `openspec/specs/${update.id}/spec.md`; + if (!options.silent) { + console.log(`Retiring ${nominal}: all requirements removed.`); + } + // `resolvedPath` is always the file that was actually unlinked - callers need + // it to report a path git will accept, since the nominal one is built from + // the capability id and can differ in case, or point through a symlink. + return { + retired: true, + ...(realSource ? { resolvedPath: realSource } : {}), + ...(options.deferDelete && displacedPath ? { displacedPath } : {}), + }; +} + +export async function finalizeRetiredSpec( + target: string, + displacedPath: string, + mainSpecsDir: string +): Promise { + await fs.unlink(displacedPath); + await pruneEmptyDirs(path.dirname(target), mainSpecsDir); +} + +/** Whether `realPath` (already canonical) sits under the real `dir`. */ +async function isInsideRealDir(realPath: string, dir: string): Promise { + const realDir = await fs.realpath(dir); + return realPath.startsWith(realDir + path.sep); +} + +/** + * Remove now-empty directories from `startDir` upward, never leaving the real + * `boundaryDir` and never removing that directory itself. + * + * The boundary is a parameter rather than the specs root directly so the walk's + * containment is stated at the call site, where the root it must not escape is + * the thing being reasoned about. + * + * The guard re-runs every iteration, so stepping to the LEXICAL parent is safe: + * a parent that is not the real one is simply re-resolved and rejected. Errors + * are swallowed and end the walk - ENOTEMPTY and ENOENT are correct outcomes (a + * file arriving mid-walk must win), and a permissions failure leaves an empty + * directory behind, which the next successful archive clears. + * + * Not race-free: an attacker who can swap an ancestor between the check and the + * `rmdir` could get an empty directory outside the root removed. Closing that + * needs fd-relative syscalls Node does not expose, and it requires local write + * access to `openspec/specs` during an archive. + */ +async function pruneEmptyDirs(startDir: string, boundaryDir: string): Promise { + let boundary: string; + try { + boundary = await fs.realpath(boundaryDir); + } catch { + return; + } + + let dir = startDir; + for (;;) { + let realDir: string; + try { + // lstat first: rmdir on a symlink fails anyway, but resolving one would + // walk us out of the tree, and the parent we then step to would be wrong. + const link = await fs.lstat(dir); + if (link.isSymbolicLink()) return; + realDir = await fs.realpath(dir); + } catch { + return; + } + + // Strictly inside the real boundary - the boundary itself is never pruned. + if (realDir === boundary || !realDir.startsWith(boundary + path.sep)) return; + + try { + const entries = await fs.readdir(dir); + if (entries.length > 0) return; + await fs.rmdir(dir); + } catch { + return; + } + + dir = path.dirname(dir); + } +} + /** * Write an updated spec to disk. */ @@ -577,15 +961,22 @@ export async function writeUpdatedSpec( update: SpecUpdate, rebuilt: string, counts: { added: number; modified: number; removed: number; renamed: number }, - options: { silent?: boolean; displayPath?: string } = {} + options: { + silent?: boolean; + displayPath?: string; + beforeMutate?: () => Promise; + } = {} ): Promise { assertTrustedSpecPath(update.targetRoot, update.target); // Create target directory if needed const targetDir = path.dirname(update.target); await fs.mkdir(targetDir, { recursive: true }); + await options.beforeMutate?.(); + // Preserve the established in-place write semantics: symlink referents, + // hard-linked specs, ACLs, extended attributes, and filesystems without hard + // links must continue to behave as they did before capability retirement. await fs.writeFile(update.target, rebuilt); - if (options.silent) return; const specName = update.id; diff --git a/src/core/templates/workflows/archive-change.ts b/src/core/templates/workflows/archive-change.ts index beae52e655..8c09666b4e 100644 --- a/src/core/templates/workflows/archive-change.ts +++ b/src/core/templates/workflows/archive-change.ts @@ -121,7 +121,7 @@ ${STORE_SELECTION_GUIDANCE} Then re-run the comparison from the top of this step against every capability that has a delta spec in \`artifactPaths.specs.existingOutputPaths\` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: - ADDED requirements present - MODIFIED requirements carrying the scenario and description changes named in the delta, with their other scenarios intact - - REMOVED requirements gone + - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving \`## Requirements\` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match - RENAMED requirements present under the new name and absent under the old one If the sync failed, or any capability does not match, report what differs and stop — do not archive. Nothing has moved and \`changeRoot\` is intact, so the user can fix the mismatch or re-run the sync and start the archive again. @@ -301,7 +301,7 @@ ${STORE_SELECTION_GUIDANCE} Then re-run the comparison from the top of this step against every capability that has a delta spec in \`artifactPaths.specs.existingOutputPaths\` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: - ADDED requirements present - MODIFIED requirements carrying the scenario and description changes named in the delta, with their other scenarios intact - - REMOVED requirements gone + - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving \`## Requirements\` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match - RENAMED requirements present under the new name and absent under the old one If the sync failed, or any capability does not match, report what differs and stop — do not archive. Nothing has moved and \`changeRoot\` is intact, so the user can fix the mismatch or re-run the sync and start the archive again. diff --git a/src/core/templates/workflows/bulk-archive-change.ts b/src/core/templates/workflows/bulk-archive-change.ts index 97211d39e1..5585fa77ba 100644 --- a/src/core/templates/workflows/bulk-archive-change.ts +++ b/src/core/templates/workflows/bulk-archive-change.ts @@ -192,7 +192,7 @@ ${STORE_SELECTION_GUIDANCE} - Verify that main specs are updated: - ADDED requirements present - MODIFIED requirements carrying scenario and description changes named in the delta, with their other scenarios intact - - REMOVED requirements gone + - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving \`## Requirements\` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match - RENAMED requirements present under the new name and absent under the old one - Do not verify delta specs in \`excludedDeltas\`; they are intentionally left unsynced. - If sync failed or any capability does not match verification, report what differs and fail/skip moving that change's \`changeRoot\` — do not archive that change. \`changeRoot\` remains intact. @@ -530,7 +530,7 @@ ${STORE_SELECTION_GUIDANCE} - Verify that main specs are updated: - ADDED requirements present - MODIFIED requirements carrying scenario and description changes named in the delta, with their other scenarios intact - - REMOVED requirements gone + - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving \`## Requirements\` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match - RENAMED requirements present under the new name and absent under the old one - Do not verify delta specs in \`excludedDeltas\`; they are intentionally left unsynced. - If sync failed or any capability does not match verification, report what differs and fail/skip moving that change's \`changeRoot\` — do not archive that change. \`changeRoot\` remains intact. diff --git a/src/core/templates/workflows/sync-specs.ts b/src/core/templates/workflows/sync-specs.ts index 164eefacde..ba742d06ce 100644 --- a/src/core/templates/workflows/sync-specs.ts +++ b/src/core/templates/workflows/sync-specs.ts @@ -109,6 +109,28 @@ ${STORE_SELECTION_GUIDANCE} **REMOVED Requirements:** - Remove the entire requirement block from main spec + - Retiring the capability. Delete the whole \`spec.md\` - and the directory once + nothing else is left in it - only when ALL of these hold: + 1. removing the requirements *this run* left no requirement blocks; + 2. the rest of the spec is well-formed (it still has a \`## Purpose\`); + 3. the main spec was not already empty before this sync - if you removed + nothing, change nothing; + 4. 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; + 5. the change's \`.openspec.yaml\` declares \`retire_capabilities: true\`; + 6. the \`spec.md\` resolves inside the real specs root (do not follow a + capability-directory symlink to delete an external file). + If removing the selected requirements would leave no requirement blocks and + any retirement condition is not satisfied, do not modify the main spec. Stop + the sync for that capability, report the blocking condition, and tell the user + how to resolve it. Never write or leave an empty \`## Requirements\` section. + When only the marker is missing, say that too - it is the one thing the user + can add to make the retirement go through. + - Deleting the file also deletes its \`## Purpose\`; any other section blocks + retirement. Name Purpose when you report the retirement. Include a pasteable + \`git checkout\` only when the spec lived in the caller's checkout; + otherwise give checkout-scoped recovery guidance. **RENAMED Requirements:** - Find the FROM requirement, rename to TO @@ -131,6 +153,8 @@ ${STORE_SELECTION_GUIDANCE} - What changes were made (requirements added/modified/removed/renamed) - Any new main spec left with a TBD Purpose placeholder, so it gets written now rather than lingering + - Any capability retired, naming the deleted \`spec.md\`, its Purpose, and + either a pasteable \`git checkout\` or checkout-scoped recovery guidance **Delta Spec Format Reference** @@ -338,6 +362,28 @@ ${STORE_SELECTION_GUIDANCE} **REMOVED Requirements:** - Remove the entire requirement block from main spec + - Retiring the capability. Delete the whole \`spec.md\` - and the directory once + nothing else is left in it - only when ALL of these hold: + 1. removing the requirements *this run* left no requirement blocks; + 2. the rest of the spec is well-formed (it still has a \`## Purpose\`); + 3. the main spec was not already empty before this sync - if you removed + nothing, change nothing; + 4. 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; + 5. the change's \`.openspec.yaml\` declares \`retire_capabilities: true\`; + 6. the \`spec.md\` resolves inside the real specs root (do not follow a + capability-directory symlink to delete an external file). + If removing the selected requirements would leave no requirement blocks and + any retirement condition is not satisfied, do not modify the main spec. Stop + the sync for that capability, report the blocking condition, and tell the user + how to resolve it. Never write or leave an empty \`## Requirements\` section. + When only the marker is missing, say that too - it is the one thing the user + can add to make the retirement go through. + - Deleting the file also deletes its \`## Purpose\`; any other section blocks + retirement. Name Purpose when you report the retirement. Include a pasteable + \`git checkout\` only when the spec lived in the caller's checkout; + otherwise give checkout-scoped recovery guidance. **RENAMED Requirements:** - Find the FROM requirement, rename to TO @@ -360,6 +406,8 @@ ${STORE_SELECTION_GUIDANCE} - What changes were made (requirements added/modified/removed/renamed) - Any new main spec left with a TBD Purpose placeholder, so it gets written now rather than lingering + - Any capability retired, naming the deleted \`spec.md\`, its Purpose, and + either a pasteable \`git checkout\` or checkout-scoped recovery guidance **Delta Spec Format Reference** diff --git a/src/utils/change-metadata.ts b/src/utils/change-metadata.ts index 57ed446c74..7ad17078dc 100644 --- a/src/utils/change-metadata.ts +++ b/src/utils/change-metadata.ts @@ -205,20 +205,23 @@ export function resolveSchemaForChange( return 'spec-driven'; } -export interface SkipSpecsMarker { +export interface MetadataMarker { /** * True when the metadata parses under ChangeMetadataSchema, sets - * skip_specs: true, and names a schema that loads. + * the requested boolean marker to true, and names a schema that loads. */ declared: boolean; /** - * Set when the marker cannot be honored: skip_specs appears in a file that + * Set when the marker cannot be honored: it appears in a file that * fails the metadata contract, or the metadata file exists but cannot be * read at all (so whether the marker is set cannot even be determined). */ invalidReason?: string; } +/** @deprecated Use MetadataMarker. */ +export type SkipSpecsMarker = MetadataMarker; + /** * Non-throwing read of the skip_specs marker. The marker only counts when the * metadata would load for status/instructions: the file parses under @@ -232,7 +235,33 @@ export interface SkipSpecsMarker { * Missing metadata means "not declared"; a marker that cannot be honored * yields invalidReason so callers can say why. */ -export function readSkipSpecsMarker(changeDir: string): SkipSpecsMarker { +export function readSkipSpecsMarker(changeDir: string): MetadataMarker { + return readBooleanMarker(changeDir, 'skip_specs'); +} + +/** + * Non-throwing read of the retire_capabilities marker, with exactly the + * semantics `readSkipSpecsMarker` documents above. + * + * Gates the one archive action that removes a file from `openspec/specs/`: when + * a change's REMOVED entries take a capability's last requirement, archive + * deletes the emptied main spec rather than aborting on a spec it cannot write + * (#1302). Declared rather than inferred because the delete is recoverable only + * from git, so it is the author's call. + */ +export function readRetireCapabilitiesMarker(changeDir: string): MetadataMarker { + return readBooleanMarker(changeDir, 'retire_capabilities'); +} + +/** + * Shared implementation for the boolean change-metadata markers, keyed by field + * name. One body rather than two, so a marker can never drift into honoring + * metadata the other rejects - the whole point of the contract described above. + */ +function readBooleanMarker( + changeDir: string, + key: 'skip_specs' | 'retire_capabilities' +): MetadataMarker { let raw: string; try { raw = fs.readFileSync(path.join(changeDir, METADATA_FILENAME), 'utf-8'); @@ -258,14 +287,15 @@ export function readSkipSpecsMarker(changeDir: string): SkipSpecsMarker { } catch { // Anchored so a comment like "# maybe add skip_specs later" does not // claim the marker was set. - return /^\s*(['"]?)skip_specs\1\s*:/m.test(raw) + const mentioned = new RegExp(`^\\s*(['"]?)${key}\\1\\s*:`, 'm').test(raw); + return mentioned ? { declared: false, invalidReason: 'the file is not valid YAML' } : { declared: false }; } const result = ChangeMetadataSchema.safeParse(parsed); if (result.success) { - if (result.data.skip_specs !== true) { + if (result.data[key] !== true) { return { declared: false }; } // Schema loading is checked only when the marker is set: a broken schema @@ -299,8 +329,8 @@ export function readSkipSpecsMarker(changeDir: string): SkipSpecsMarker { const markerMentioned = typeof parsed === 'object' && parsed !== null && - 'skip_specs' in parsed && - (parsed as Record).skip_specs !== false; + key in parsed && + (parsed as Record)[key] !== false; if (markerMentioned) { const first = result.error.issues[0]; const where = first.path.length > 0 ? `${first.path.join('.')}: ` : ''; diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 9cb30ed4b5..516b79cf4a 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -1,5 +1,6 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { ArchiveCommand } from '../../src/core/archive.js'; +import { describe, it, expect, beforeEach, afterEach, onTestFinished, vi } from 'vitest'; +import { ArchiveCommand, isRetirableSpec } from '../../src/core/archive.js'; +import { retireSpec } from '../../src/core/specs-apply.js'; import { Validator } from '../../src/core/validation/validator.js'; import { MarkdownParser } from '../../src/core/parsers/markdown-parser.js'; import { findMainSpecStructureIssues } from '../../src/core/parsers/spec-structure.js'; @@ -23,6 +24,16 @@ describe('ArchiveCommand', () => { const originalXdgDataHome = process.env.XDG_DATA_HOME; const originalTimeZone = process.env.TZ; + function archiveClaimPath(_archiveName: string): string { + return path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + '.openspec-archive.lock' + ); + } + beforeEach(async () => { // Create temp directory tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-archive-test-')); @@ -107,6 +118,230 @@ describe('ArchiveCommand', () => { await expect(fs.access(changeDir)).rejects.toThrow(); }); + it('retains the complete copied archive when fallback source cleanup partially fails', async () => { + const changeName = 'fallback-cleanup-failure'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Complete\n'); + await fs.writeFile(path.join(changeDir, 'notes.md'), 'keep this\n'); + + const realRename = fs.rename.bind(fs); + const realRm = fs.rm.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}changes${path.sep}${changeName}`) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('cross-device move'), { code: 'EXDEV' }); + } + return realRename(source, destination); + }); + vi.spyOn(fs, 'rm').mockImplementation(async (candidate, options) => { + if ( + String(candidate).includes(`${path.sep}changes${path.sep}.openspec-move-`) + ) { + throw Object.assign(new Error('source cleanup failed'), { code: 'EACCES' }); + } + return realRm(candidate, options); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true, skipSpecs: true }) + ).rejects.toThrow(/complete destination was retained for recovery/); + + const archived = path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ); + await expect(fs.readFile(path.join(archived, 'tasks.md'), 'utf-8')).resolves.toContain( + 'Complete' + ); + await expect(fs.readFile(path.join(archived, 'notes.md'), 'utf-8')).resolves.toBe( + 'keep this\n' + ); + }); + + it('does not discard an artifact changed during the fallback copy', async () => { + const changeName = 'fallback-artifact-race'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const tasksPath = path.join(changeDir, 'tasks.md'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(tasksPath, '- [x] Original task\n'); + + const realRename = fs.rename.bind(fs); + const realCopyFile = fs.copyFile.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}changes${path.sep}${changeName}`) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('cross-device move'), { code: 'EXDEV' }); + } + return realRename(source, destination); + }); + let edited = false; + vi.spyOn(fs, 'copyFile').mockImplementation(async (source, destination, mode) => { + await realCopyFile(source, destination, mode); + if ( + !edited && + String(source).includes(`${path.sep}.openspec-move-`) && + String(source).endsWith(`${path.sep}tasks.md`) + ) { + edited = true; + await fs.appendFile(source, '- [x] Concurrent task\n'); + } + }); + + await expect( + archiveCommand.execute(changeName, { yes: true, skipSpecs: true }) + ).rejects.toThrow(/changed during the fallback copy/); + + expect(edited).toBe(true); + await expect(fs.readFile(tasksPath, 'utf-8')).resolves.toContain('Concurrent task'); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + await expect( + fs.access( + path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ) + ) + ).rejects.toThrow(); + }); + + it.skipIf(process.platform === 'win32')( + 'does not discard an artifact permission change during the fallback copy', + async () => { + const changeName = 'fallback-mode-race'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const toolPath = path.join(changeDir, 'tool.sh'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(toolPath, '#!/bin/sh\n'); + await fs.chmod(toolPath, 0o644); + + const realRename = fs.rename.bind(fs); + const realCopyFile = fs.copyFile.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}changes${path.sep}${changeName}`) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('cross-device move'), { code: 'EXDEV' }); + } + return realRename(source, destination); + }); + let changed = false; + vi.spyOn(fs, 'copyFile').mockImplementation(async (source, destination, mode) => { + await realCopyFile(source, destination, mode); + if ( + !changed && + String(source).includes(`${path.sep}.openspec-move-`) && + String(source).endsWith(`${path.sep}tool.sh`) + ) { + changed = true; + await fs.chmod(source, 0o755); + } + }); + + await expect( + archiveCommand.execute(changeName, { yes: true, skipSpecs: true }) + ).rejects.toThrow(/changed during the fallback copy/); + + expect(changed).toBe(true); + expect((await fs.stat(toolPath)).mode & 0o777).toBe(0o755); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'preserves directory and file modes in an unchanged fallback copy', + async () => { + const changeName = 'fallback-preserves-modes'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const privateDir = path.join(changeDir, 'private'); + const toolPath = path.join(privateDir, 'tool.sh'); + await fs.mkdir(privateDir, { recursive: true }); + await fs.writeFile(toolPath, '#!/bin/sh\n'); + await fs.chmod(toolPath, 0o755); + await fs.chmod(privateDir, 0o700); + + const realRename = fs.rename.bind(fs); + const realCopyFile = fs.copyFile.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}changes${path.sep}${changeName}`) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('cross-device move'), { code: 'EXDEV' }); + } + return realRename(source, destination); + }); + let modeDuringCopy: number | undefined; + vi.spyOn(fs, 'copyFile').mockImplementation(async (source, destination, mode) => { + if (String(source).endsWith(`${path.sep}private${path.sep}tool.sh`)) { + modeDuringCopy = (await fs.stat(path.dirname(String(destination)))).mode & 0o777; + } + return realCopyFile(source, destination, mode); + }); + + await archiveCommand.execute(changeName, { yes: true, skipSpecs: true }); + + const archivedPrivate = path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}`, + 'private' + ); + expect(modeDuringCopy).toBe(0o700); + expect((await fs.stat(archivedPrivate)).mode & 0o777).toBe(0o700); + expect((await fs.stat(path.join(archivedPrivate, 'tool.sh'))).mode & 0o777).toBe( + 0o755 + ); + } + ); + + it.skipIf(process.platform === 'win32')( + 'uses a short staging name for a long change during fallback', + async () => { + const prefix = `${formatLocalDate()}-`; + const changeName = prefix + 'x'.repeat(220 - Buffer.byteLength(prefix)); + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Complete\n'); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('cross-device move'), { code: 'EXDEV' }); + } + return realRename(source, destination); + }); + + await archiveCommand.execute(changeName, { yes: true, skipSpecs: true }); + + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', 'archive', changeName)) + ).resolves.not.toThrow(); + } + ); it('preserves symlinks during the cross-device archive fallback', async () => { if (process.platform === 'win32') return; @@ -176,7 +411,7 @@ describe('ArchiveCommand', () => { ); }); - it('preserves a linked change during the cross-device archive fallback', async () => { + it('rejects a linked change before the cross-device archive fallback', async () => { if (process.platform === 'win32') return; const changeName = 'linked-change'; @@ -186,24 +421,19 @@ describe('ArchiveCommand', () => { await fs.writeFile(path.join(realChangeDir, 'tasks.md'), '- [x] Task 1\n'); await fs.symlink(realChangeDir, linkedChangeDir); - const rename = vi.spyOn(fs, 'rename').mockRejectedValueOnce( - Object.assign(new Error('cross-device move'), { code: 'EXDEV' }) - ); - try { - await archiveCommand.execute(changeName, { + await expect( + archiveCommand.execute(changeName, { yes: true, noValidate: true, skipSpecs: true, - }); - } finally { - rename.mockRestore(); - } + }) + ).rejects.toMatchObject({ + diagnostic: { code: 'archive_change_symlink' }, + }); const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive'); - const [archiveName] = await fs.readdir(archiveDir); - const archivedChange = path.join(archiveDir, archiveName); - expect((await fs.lstat(archivedChange)).isSymbolicLink()).toBe(true); - expect(await fs.readlink(archivedChange)).toBe(realChangeDir); + await expect(fs.readdir(archiveDir)).resolves.toHaveLength(0); + expect((await fs.lstat(linkedChangeDir)).isSymbolicLink()).toBe(true); await expect(fs.readFile(path.join(realChangeDir, 'tasks.md'), 'utf8')).resolves.toContain( 'Task 1' ); @@ -232,7 +462,9 @@ describe('ArchiveCommand', () => { noValidate: true, skipSpecs: true, }) - ).rejects.toMatchObject({ code: 'EEXIST' }); + ).rejects.toMatchObject({ + diagnostic: { code: 'archive_target_exists' }, + }); } finally { rename.mockRestore(); } @@ -1784,6 +2016,160 @@ New feature description. ).rejects.toThrow(`Archive '${date}-${changeName}' already exists.`); }); + it.skipIf(process.platform === 'win32')( + 'does not replace a dangling symlink at the archive destination', + async () => { + const changeName = 'dangling-archive-target'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(changeDir, { recursive: true }); + const archivePath = path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ); + await fs.symlink('missing-target', archivePath); + + await expect( + archiveCommand.execute(changeName, { yes: true, skipSpecs: true }) + ).rejects.toThrow(/already exists/); + + expect((await fs.lstat(archivePath)).isSymbolicLink()).toBe(true); + await expect(fs.readlink(archivePath)).resolves.toBe('missing-target'); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'archives a valid maximum-length date-prefixed change name', + async () => { + const prefix = `${formatLocalDate()}-`; + const changeName = prefix + 'x'.repeat(251 - Buffer.byteLength(prefix)); + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(changeDir, { recursive: true }); + + await archiveCommand.execute(changeName, { yes: true, skipSpecs: true }); + + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', 'archive', changeName)) + ).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'rejects an explicitly named symlinked active change', + async () => { + const changeName = 'symlinked-active-change'; + const realChange = path.join(tempDir, 'real-change'); + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(realChange, { recursive: true }); + await fs.symlink(realChange, changeDir, 'dir'); + + await expect( + archiveCommand.execute(changeName, { yes: true, skipSpecs: true }) + ).rejects.toThrow(/symbolic link/); + + expect((await fs.lstat(changeDir)).isSymbolicLink()).toBe(true); + await expect(fs.access(realChange)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'reports a symlinked active change as one JSON failure document', + async () => { + const changeName = 'symlinked-active-change-json'; + const realChange = path.join(tempDir, 'real-json-change'); + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(realChange, { recursive: true }); + await fs.symlink(realChange, changeDir, 'dir'); + + await archiveCommand.execute(changeName, { + json: true, + yes: true, + skipSpecs: true, + }); + + const calls = (console.log as unknown as ReturnType).mock.calls; + expect(calls).toHaveLength(1); + const payload = JSON.parse(String(calls[0][0])); + expect(payload.archive).toBeNull(); + expect(payload.status).toEqual([ + expect.objectContaining({ + severity: 'error', + code: 'archive_change_symlink', + }), + ]); + expect(process.exitCode).toBe(1); + expect((await fs.lstat(changeDir)).isSymbolicLink()).toBe(true); + } + ); + + it('gives safe recovery guidance for a stale archive claim', async () => { + const changeName = 'stale-archive-claim'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(changeDir, { recursive: true }); + const archiveName = `${formatLocalDate()}-${changeName}`; + const claimPath = archiveClaimPath(archiveName); + await fs.writeFile(claimPath, JSON.stringify({ pid: 2_147_483_647 })); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/remove the stale claim at .*\.openspec-archive\.lock/); + + await expect(fs.access(changeDir)).resolves.not.toThrow(); + await expect(fs.access(claimPath)).resolves.not.toThrow(); + }); + + it('keeps an archive claim owned by a running process', async () => { + const changeName = 'active-archive-claim'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(changeDir, { recursive: true }); + const archiveName = `${formatLocalDate()}-${changeName}`; + const claimPath = archiveClaimPath(archiveName); + await fs.writeFile(claimPath, JSON.stringify({ pid: process.pid })); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/already being created/); + + await expect(fs.access(changeDir)).resolves.not.toThrow(); + await expect(fs.access(claimPath)).resolves.not.toThrow(); + }); + + // Windows defers deletion of an open file until its original handle closes, + // so unlink-and-recreate cannot model a persistent replacement there. + it.skipIf(process.platform === 'win32')( + 'does not unlink a claim entry replaced by another process', + async () => { + const changeName = 'replaced-archive-claim'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(changeDir, { recursive: true }); + const archiveName = `${formatLocalDate()}-${changeName}`; + const claimPath = archiveClaimPath(archiveName); + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let replaced = false; + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + !replaced && + String(source).endsWith(`${path.sep}changes${path.sep}${changeName}`) && + String(destination).endsWith(`${path.sep}archive${path.sep}${archiveName}`) + ) { + replaced = true; + await fs.unlink(claimPath); + await fs.writeFile(claimPath, 'replacement claim\n'); + } + return realRename(source, destination); + }); + + await archiveCommand.execute(changeName, { yes: true, skipSpecs: true }); + + expect(replaced).toBe(true); + await expect(fs.readFile(claimPath, 'utf-8')).resolves.toBe('replacement claim\n'); + } + ); + it('should handle changes without tasks.md', async () => { const changeName = 'no-tasks-feature'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); @@ -2105,6 +2491,136 @@ The system SHALL survive. await expect(fs.access(changeDir)).rejects.toThrow(); }); + it('does not apply a stale retirement decision when discarded content changes at the prompt', async () => { + const { confirm } = await import('@inquirer/prompts'); + const mockConfirm = confirm as unknown as ReturnType; + const changeName = 'retirement-changed-at-prompt'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const deltaDir = path.join(changeDir, 'specs', 'legacy-layer'); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(mainSpecDir, 'spec.md'); + await fs.mkdir(deltaDir, { recursive: true }); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, '.openspec.yaml'), 'schema: spec-driven\nretire_capabilities: true\n'); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Done\n'); + await fs.writeFile( + path.join(deltaDir, 'spec.md'), + `## REMOVED Requirements + +### Requirement: Legacy behavior +**Reason**: It is retired. +**Migration**: None. +` + ); + await fs.writeFile( + target, + `# legacy-layer Specification + +## Purpose +This capability preserves legacy behavior for existing consumers. + +## Requirements + +### Requirement: Legacy behavior +The system SHALL preserve legacy behavior. + +#### Scenario: Legacy behavior applies +- **WHEN** legacy behavior is requested +- **THEN** it remains available +` + ); + + mockConfirm.mockReset(); + mockConfirm.mockImplementationOnce(async () => { + const current = await fs.readFile(target, 'utf-8'); + await fs.writeFile( + target, + current.replace( + '- **THEN** it remains available', + '- **THEN** this concurrent edit remains available' + ) + ); + return true; + }); + + await archiveCommand.execute(changeName); + + await expect(fs.readFile(target, 'utf-8')).resolves.toContain( + '- **THEN** this concurrent edit remains available' + ); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining("Spec inputs for 'legacy-layer' changed") + ); + }); + + it('does not use retirement authorization that changed at the prompt', async () => { + const { confirm } = await import('@inquirer/prompts'); + const mockConfirm = confirm as unknown as ReturnType; + const changeName = 'retirement-marker-changed-at-prompt'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const deltaDir = path.join(changeDir, 'specs', 'legacy-layer'); + await fs.mkdir(deltaDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Done\n'); + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nretire_capabilities: true\n' + ); + await fs.writeFile( + path.join(deltaDir, 'spec.md'), + `## REMOVED Requirements + +### Requirement: Legacy behavior +**Reason**: It is retired. +**Migration**: None. +` + ); + const target = path.join( + tempDir, + 'openspec', + 'specs', + 'legacy-layer', + 'spec.md' + ); + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile( + target, + `# legacy-layer Specification + +## Purpose +This capability preserves legacy behavior for existing consumers. + +## Requirements + +### Requirement: Legacy behavior +The system SHALL preserve legacy behavior. + +#### Scenario: Legacy behavior applies +- **WHEN** legacy behavior is requested +- **THEN** it remains available +` + ); + + mockConfirm.mockReset(); + mockConfirm.mockImplementationOnce(async () => { + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nretire_capabilities: false\n' + ); + return true; + }); + + await archiveCommand.execute(changeName); + + await expect(fs.access(target)).resolves.not.toThrow(); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('retirement authorization changed') + ); + }); + it('prints the loss warning before --yes writes the spec', async () => { const changeName = 'warn-before-yes-write'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); @@ -3151,6 +3667,2948 @@ The system SHALL do the thing differently. }); }); + // A delta whose REMOVED entries cover every requirement rebuilds the main + // spec empty, and an empty spec can never validate. Every such archive used + // to abort with "Spec must have at least one requirement", leaving no way to + // retire a capability (#1302). + describe('capability retirement (#1302)', () => { + const REQUIREMENT = [ + '### 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', + ].join('\n'); + + const PURPOSE = + 'Holds the behavior contract for the legacy layer that consumers still depend on today.'; + + function mainSpec(name: string, requirements = REQUIREMENT): string { + return `# ${name} Specification\n\n## Purpose\n${PURPOSE}\n\n## Requirements\n\n${requirements}\n`; + } + + const REMOVE_ALL = [ + '# 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.', + '', + ].join('\n'); + + /** The last thing printed, which in JSON mode is the one payload. */ + function lastJsonPayload(): string { + const calls = (console.log as unknown as ReturnType).mock.calls; + return String(calls[calls.length - 1][0]); + } + + /** + * A change that is allowed to retire a capability. Every retirement case + * below carries the marker, because without it archive aborts - which is the + * whole point of the marker, and has its own tests further down. + */ + async function createChange( + changeName: string, + capability: string, + deltaSpec: string, + options: { declareRetirement?: boolean } = {} + ): Promise { + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + await fs.mkdir(path.join(changeDir, 'specs', ...capability.split('/')), { + recursive: true, + }); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task 1\n'); + await fs.writeFile( + path.join(changeDir, 'specs', ...capability.split('/'), 'spec.md'), + deltaSpec + ); + if (options.declareRetirement !== false) { + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nretire_capabilities: true\n' + ); + } + return changeDir; + } + + // The marker is what makes the deletion the author's decision rather than + // an inference from the shape of a delta. Without it archive behaves exactly + // as it did before #1302 - it aborts on a spec it cannot write - except that + // the abort now names the way out. + describe('retire_capabilities marker', () => { + async function setUpUnmarked(changeName: string, metadata?: string): Promise { + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL, { + declareRetirement: false, + }); + if (metadata !== undefined) { + await fs.writeFile(path.join(changeDir, '.openspec.yaml'), metadata); + } + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpec('legacy-layer')); + return path.join(mainSpecDir, 'spec.md'); + } + + it('aborts without the marker, naming it, and deletes nothing', async () => { + const target = await setUpUnmarked('retire-unmarked'); + const original = await fs.readFile(target, 'utf-8'); + + await archiveCommand.execute('retire-unmarked', { yes: true }); + + // Pre-#1302 behavior, unchanged: the unwritable spec aborts the archive. + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining(VALIDATION_MESSAGES.SPEC_NO_REQUIREMENTS) + ); + // ...but the dead end now comes with its own way out. + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('add `retire_capabilities: true`') + ); + // Nothing touched: not the spec, not the change. + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', 'retire-unmarked')) + ).resolves.not.toThrow(); + }); + + it('refuses a marker it cannot honor, and says why', async () => { + // Mirrors skip_specs: a marker in metadata that fails the contract is + // not a marker. Silently ignoring it would be the worst outcome - the + // author believes they authorised the deletion. + const target = await setUpUnmarked( + 'retire-bad-marker', + 'schema: spec-driven\nretire_capabilities: yes-please\n' + ); + + await archiveCommand.execute('retire-bad-marker', { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('cannot be honored') + ); + await expect(fs.access(target)).resolves.not.toThrow(); + }); + + it('treats retire_capabilities: false as not declared', async () => { + const target = await setUpUnmarked( + 'retire-false-marker', + 'schema: spec-driven\nretire_capabilities: false\n' + ); + + await archiveCommand.execute('retire-false-marker', { yes: true }); + + expect(process.exitCode).toBe(1); + // An explicit false is the opposite of setting the marker, so it must + // not be reported as an unhonorable one. + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining('cannot be honored') + ); + await expect(fs.access(target)).resolves.not.toThrow(); + }); + + it('reports the missing marker as the fix in --json', async () => { + await setUpUnmarked('retire-unmarked-json'); + + await archiveCommand + .execute('retire-unmarked-json', { yes: true, json: true }) + .catch(() => undefined); + + const payload = JSON.parse(lastJsonPayload()); + expect(payload.archive).toBeNull(); + expect(JSON.stringify(payload.status)).toContain('retire_capabilities: true'); + }); + + it('does not name the marker when retirement would not have fixed it', async () => { + // A spec broken in some further way is not a retirement candidate, so + // pointing at the marker would send the author after the wrong fix. + const changeDir = await createChange('retire-also-broken-marker', 'legacy-layer', REMOVE_ALL, { + declareRetirement: false, + }); + expect(changeDir).toBeTruthy(); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + // No `## Purpose`: a second, independent validation error. + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + `# legacy-layer Specification\n\n## Requirements\n\n${REQUIREMENT}\n` + ); + + await archiveCommand.execute('retire-also-broken-marker', { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining('add `retire_capabilities: true`') + ); + }); + }); + + // A second `## Requirements` section is where every parser here stops short: + // `extractRequirementsSection` binds to the first one, so the validator's + // lookup, the block parser, the residual-heading veto and the lost-section + // report all ignore what follows. A spec shaped like this passed + // `validate --strict` and was then deleted with a live SHALL requirement in + // it, named nowhere in the report. + it('refuses to retire a spec that has a second Requirements section', async () => { + const changeName = 'retire-two-sections'; + await createChange(changeName, 'audit', [ + '# Audit - Changes', + '', + '## REMOVED Requirements', + '', + '### Requirement: Audit trail', + '**Reason**: Superseded.', + '**Migration**: None.', + '', + ].join('\n')); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'audit'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const original = [ + '# audit Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: Audit trail', + 'The system SHALL record an audit entry for every privileged action.', + '', + '#### Scenario: Entry recorded', + '- **WHEN** a privileged action runs', + '- **THEN** an entry is recorded', + '', + '## Requirements', + '', + '### Seven year retention', + 'The system SHALL retain audit entries for seven years.', + '', + '#### Scenario: Early purge refused', + '- **WHEN** a purge is attempted early', + '- **THEN** it is refused', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), original); + + // The spec as written is valid, which is what made the deletion silent. + const before = await new Validator().validateSpecContent('audit', original, 'strict'); + expect(before.valid).toBe(true); + + await archiveCommand.execute(changeName, { yes: true }); + + // Aborts instead, exactly as it did before retirement existed... + expect(process.exitCode).toBe(1); + // ...and the second section's requirement is still there. + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe( + original + ); + }); + + it('refuses to retire duplicate requirement names from the main spec', async () => { + const changeName = 'retire-duplicate-requirement'; + await createChange( + changeName, + 'audit', + '# Audit - Changes\n\n## REMOVED Requirements\n\n### Requirement: Same\n**Reason**: x.\n**Migration**: None.\n' + ); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'audit'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const original = [ + '# audit Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: Same', + 'The system SHALL keep the first behavior.', + '', + '#### Scenario: First', + '- **WHEN** the first path runs', + '- **THEN** the first behavior remains', + '', + '### Requirement: Same', + 'The system SHALL keep the independently authored second behavior.', + '', + '#### Scenario: Second', + '- **WHEN** the second path runs', + '- **THEN** the second behavior remains', + '', + ].join('\n'); + const target = path.join(mainSpecDir, 'spec.md'); + await fs.writeFile(target, original); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('duplicates the requirement declared') + ); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + }); + + it('refuses to retire an H1 section written after Purpose', async () => { + const changeName = 'retire-h1-after-purpose'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const original = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '# Architecture Notes', + 'Do not delete this independently authored section.', + '', + '## Requirements', + '', + REQUIREMENT, + '', + ].join('\n'); + const target = path.join(mainSpecDir, 'spec.md'); + await fs.writeFile(target, original); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('content the merge cannot safely account for') + ); + }); + + // `extractRequirementsSection` masks fences only, `findHeadings` masks HTML + // comments as well. That one-mask difference was a data-loss bug: a `##` + // inside a multi-line comment ends the section for the merge, so everything + // below it became a tail no comment-masking scan could see - and a + // `validate --strict`-clean spec was deleted with a live SHALL in it. + it('refuses to retire when a commented-out heading hid the section boundary', async () => { + const changeName = 'retire-comment-boundary'; + await createChange( + changeName, + 'audit', + '# Audit - Changes\n\n## REMOVED Requirements\n\n### Requirement: Audit trail\n**Reason**: x.\n**Migration**: None.\n' + ); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'audit'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const original = [ + '# audit Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '### Requirement: Audit trail', + 'The system SHALL record an audit entry.', + '', + '#### Scenario: Recorded', + '- **WHEN** a privileged action runs', + '- **THEN** an entry is recorded', + '', + '', + '', + '### Seven year retention', + 'The system SHALL retain audit entries for seven years.', + '', + '#### Scenario: Early purge refused', + '- **WHEN** a purge is attempted early', + '- **THEN** it is refused', + '', + ].join('\n'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), original); + + // Valid as written, which is what made the deletion silent. + expect((await new Validator().validateSpecContent('audit', original, '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( + original + ); + // And the author is told why their marker was refused. + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('content the merge cannot safely account for') + ); + }); + + // The guard audits the WHOLE file, not a couple of its slices. A block's + // raw carries everything the parser did not read as a new header - prose, + // tables, fences - and that content was deleted while the report said only + // "Purpose" was lost. Content above the requirements section had the same + // hole. + it.each([ + { + where: 'inside a removed block', + spec: [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + REQUIREMENT, + '', + 'MIGRATION RUNBOOK (authored by hand, not a heading):', + 'Step 1: rotate the customer keys before 2026-08-01.', + '', + '| host | owner |', + '| --- | --- |', + '| db-1 | payments |', + '', + ].join('\n'), + quoted: 'MIGRATION RUNBOOK', + }, + { + where: 'above the requirements section', + spec: [ + '# legacy-layer Specification', + '', + 'NOTE TO MAINTAINERS: the escrow keys live in the "legacy" vault.', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + REQUIREMENT, + '', + ].join('\n'), + quoted: 'NOTE TO MAINTAINERS', + }, + // Not a case: prose between `## Purpose` and `## Requirements` IS the + // Purpose body - the section runs to the next `##` - and the retirement + // warning already names Purpose as going with the file. + ])('refuses to retire with authored content $where', async ({ spec, quoted }) => { + const changeName = `retire-authored-${quoted.split(' ')[0].toLowerCase()}`; + 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'), 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); + // And the author is told which lines stood in the way. + expect(console.log).toHaveBeenCalledWith(expect.stringContaining(quoted)); + }); + + it('refuses to retire when a note is bulleted below the scenarios', async () => { + // Every bullet used to count as a scenario's own, so an operational note + // written under the last scenario was deleted with the file and named + // nowhere. A scenario's bullets run unbroken beneath its header; a blank + // line ends them. + const changeName = 'retire-bulleted-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, + '', + '- IMPORTANT: escrow keys live in the "legacy" vault; rotate before deleting.', + '', + ].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 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 + // requirement's own content. + // + // Known limitation, deliberate: a scenario whose bullets are split by a + // blank line reads the same as a note bulleted below the scenario, and no + // line-based rule separates them. Such a spec is REFUSED, never deleted - + // the abort names the lines and the author moves them or deletes the file + // by hand. + const changeName = 'retire-rich-requirement'; + 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', + '`openspec legacy` SHALL provide a legacy layer to existing consumers.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer runs `openspec legacy --check`', + '- **THEN** these happen in order:', + ' 1. the layer loads', + ' 2. the consumer proceeds', + '', + ].join('\n') + ); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + }); + + it.each([ + { what: 'a setext heading', body: ['Data Migration Notes', '--------------------', 'Export the table by hand first.'] }, + { what: 'a raw HTML heading', body: ['

Data Migration Notes

', 'Export the table by hand first.'] }, + ])('refuses to retire when $what opens a section inside Purpose', async ({ body }) => { + // `##` is not the only way to open a section. Treating everything up to + // the next ATX `##` as Purpose body swallowed these whole and deleted + // them, reported as nothing but "Purpose". + const changeName = `retire-purpose-span-${body.length}`; + 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, + '', + ...body, + '', + '## Requirements', + '', + REQUIREMENT, + '', + ].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('Data Migration Notes') + ); + }); + + it('refuses to retire a Setext section absorbed before a requirement scenario', async () => { + const changeName = 'retire-setext-inside-requirement'; + 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.', + '', + 'Migration Notes', + '---------------', + 'Keep this hand-written migration note.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + '', + ].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('Migration Notes') + ); + }); + + it('never retires under --no-validate, whatever else the spec holds', async () => { + // Isolates that conjunct: the spec is otherwise a clean retirement + // candidate, so only the flag can be stopping it. + const changeName = 'retire-novalidate-isolated'; + 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'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // Written, not deleted. + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).resolves.not.toThrow(); + }); + + it('names the marker only when retiring would really fix it', async () => { + // The same two-section spec, with no marker. The hint must stay quiet: + // adding the marker would not have made this spec writable. + const changeName = 'retire-two-sections-unmarked'; + await createChange( + changeName, + 'audit', + '# Audit - Changes\n\n## REMOVED Requirements\n\n### Requirement: Audit trail\n**Reason**: x.\n**Migration**: None.\n', + { declareRetirement: false } + ); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'audit'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + `# audit Specification\n\n## Purpose\n${PURPOSE}\n\n## Requirements\n\n### Requirement: Audit trail\nThe system SHALL audit.\n\n#### Scenario: S\n- **WHEN** w\n- **THEN** t\n\n## Requirements\n\n### Kept\nThe system SHALL keep this.\n\n#### Scenario: K\n- **WHEN** w\n- **THEN** t\n` + ); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining('add `retire_capabilities: true`') + ); + }); + + it.skipIf(process.platform === 'win32')( + 'gives guidance, not a broken command, when the spec lived outside the repo', + async () => { + // `git checkout HEAD -- ` is rejected from a different + // worktree however it is quoted, and an unquoted path with a space + // splits when pasted. A store-selected root and a symlinked capability + // directory both produce exactly that path, so those cases say where the + // file was instead of offering a command that cannot run. + const outside = path.join(tempDir, 'out side'); + await fs.mkdir(outside, { recursive: true }); + await fs.writeFile(path.join(outside, 'spec.md'), mainSpec('legacy-layer')); + await fs.mkdir(path.join(tempDir, 'openspec', 'specs'), { recursive: true }); + await fs.symlink(outside, path.join(tempDir, 'openspec', 'specs', 'legacy-layer'), 'dir'); + const changeName = 'retire-outside'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/resolves outside/); + await expect(fs.access(path.join(outside, 'spec.md'))).resolves.not.toThrow(); + } + ); + + it('does not promise git recovery outright, and names the real path', async () => { + // Archive cannot know whether the file is in HEAD - a spec an earlier + // archive created and nobody committed is not - so the recovery line is + // phrased as the condition it is rather than as a promise. + const changeName = 'retire-recovery-wording'; + 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'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true, json: true }); + + const notes = JSON.parse(lastJsonPayload()).archive.warnings.join('\n'); + expect(notes).toContain( + 'If it was committed, restore it with: git checkout HEAD -- ":(top)openspec/specs/legacy-layer/spec.md"' + ); + expect(notes).not.toContain('Recover with: git checkout'); + }); + + it('refuses a marker sitting in unparseable YAML', async () => { + // Fail-closed branch: metadata the rest of the CLI cannot read must never + // authorise a deletion, and the abort has to say why. + const changeName = 'retire-broken-yaml'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL, { + declareRetirement: false, + }); + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nretire_capabilities: true\n bad: [oops\n' + ); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('the file is not valid YAML') + ); + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).resolves.not.toThrow(); + }); + + it('reports an unlink failure instead of archiving over a spec it could not delete', async () => { + // If the unlink error were swallowed, archive would complete and leave a + // main spec that `openspec validate` rejects - the exact state #1302 is + // about, reached silently. + const capability = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(capability, { recursive: true }); + const target = path.join(capability, 'spec.md'); + await fs.writeFile(target, mainSpec('legacy-layer')); + const realUnlink = fs.unlink.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'unlink').mockImplementation( + async (candidate: Parameters[0]) => { + if (String(candidate) === target) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realUnlink(candidate); + } + ); + + await expect( + retireSpec( + { id: 'legacy-layer', source: 'x', target, exists: true }, + path.join(tempDir, 'openspec', 'specs'), + { silent: true } + ) + ).rejects.toThrow(/Could not retire capability 'legacy-layer'.*Remove it by hand/s); + + await expect(fs.access(target)).resolves.not.toThrow(); + }); + + it('fails closed when it cannot verify a retirement target', async () => { + const capability = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(capability, { recursive: true }); + const target = path.join(capability, 'spec.md'); + await fs.writeFile(target, mainSpec('legacy-layer')); + const realLstat = fs.lstat.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'lstat').mockImplementation(async (candidate, options) => { + if (String(candidate) === target) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realLstat(candidate, options); + }); + + await expect( + retireSpec( + { id: 'legacy-layer', source: 'x', target, exists: true }, + path.join(tempDir, 'openspec', 'specs'), + { silent: true } + ) + ).rejects.toThrow(/could not verify .* before deletion.*permission denied/s); + + await expect(fs.access(target)).resolves.not.toThrow(); + }); + + it('retires the capability when a delta removes its last requirement', async () => { + const changeName = 'retire-legacy-layer'; + 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'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true }); + + // The spec and the directory it was alone in are gone from the live tree... + await expect(fs.access(mainSpecDir)).rejects.toThrow(); + // ...but the specs root itself is never pruned. + await expect( + fs.access(path.join(tempDir, 'openspec', 'specs')) + ).resolves.not.toThrow(); + // The archive completed rather than aborting. + expect(process.exitCode).not.toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Retiring openspec/specs/legacy-layer/spec.md') + ); + // The one thing a reader needs that the path does not tell them: how to + // get the file back. + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + 'If it was committed, restore it with: git checkout HEAD -- ":(top)openspec/specs/legacy-layer/spec.md"' + ) + ); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Totals: + 0, ~ 0, - 1, → 0') + ); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Specs updated successfully.') + ); + await expect(fs.access(path.join(tempDir, 'openspec', 'changes', changeName))).rejects.toThrow(); + }); + + it('prunes empty parent directories in a nested layout but keeps siblings', async () => { + const changeName = 'retire-nested'; + await createChange(changeName, 'platform/legacy-layer', REMOVE_ALL); + const nestedDir = path.join(tempDir, 'openspec', 'specs', 'platform', 'legacy-layer'); + const siblingDir = path.join(tempDir, 'openspec', 'specs', 'platform', 'kept'); + await fs.mkdir(nestedDir, { recursive: true }); + await fs.mkdir(siblingDir, { recursive: true }); + await fs.writeFile(path.join(nestedDir, 'spec.md'), mainSpec('legacy-layer')); + await fs.writeFile(path.join(siblingDir, 'spec.md'), mainSpec('kept')); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(nestedDir)).rejects.toThrow(); + // The sibling keeps the shared parent alive. + await expect(fs.access(path.join(siblingDir, 'spec.md'))).resolves.not.toThrow(); + }); + + it('leaves a capability directory that still holds other files', async () => { + const changeName = 'retire-with-notes'; + 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'), mainSpec('legacy-layer')); + await fs.writeFile(path.join(mainSpecDir, 'NOTES.md'), 'Kept by hand.\n'); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + await expect(fs.readFile(path.join(mainSpecDir, 'NOTES.md'), 'utf-8')).resolves.toBe( + 'Kept by hand.\n' + ); + }); + + it('archives a REMOVED-only delta whose main spec was already deleted', async () => { + // The issue's second dead end: pre-deleting the spec made the delta look + // like a create, which landed on an empty spec and failed the same way. + const changeName = 'retire-already-gone'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).not.toBe(1); + // Nothing was recreated. + await expect( + fs.access(path.join(tempDir, 'openspec', 'specs', 'legacy-layer')) + ).rejects.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).rejects.toThrow(); + }); + + // The requirement-block count and the validator do NOT agree on what a + // requirement is: MarkdownParser accepts any `###` heading under + // `## Requirements`, while the delta block parser only indexes canonical + // `### Requirement:` headers and sweeps the rest into the preamble - which + // survives into the rebuilt spec. Retiring on the block count alone deleted + // specs that validate cleanly, so the validator is the only oracle. + it('does not retire a spec that still validates without any requirement blocks', async () => { + const changeName = 'retire-preamble-heading'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const preambleRequirement = [ + '### Notes on scope', + 'The system SHALL treat the notes below as normative for the legacy layer.', + '', + '#### Scenario: Notes apply', + '- **WHEN** a reader consults the notes', + '- **THEN** the notes apply', + ].join('\n'); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + mainSpec('legacy-layer', `${preambleRequirement}\n\n${REQUIREMENT}`) + ); + + await archiveCommand.execute(changeName, { yes: true }); + + const updated = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(updated).toContain('### Notes on scope'); + expect(process.exitCode).not.toBe(1); + // The rebuilt spec is still a valid spec, so it is written, not deleted. + const report = await new Validator().validateSpecContent('legacy-layer', updated); + expect(report.valid).toBe(true); + }); + + it('aborts, exactly as before, when the removal was already synced', async () => { + // Nothing was removed this run, so this is not a retirement: the spec is + // already requirement-less and stays the author's to fix. Deleting on a + // no-op delta would destroy a file the change never touched, and archiving + // anyway would leave a main spec that `validate` rejects. + const changeName = 'retire-noop'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const emptied = `# legacy-layer Specification\n\n## Purpose\n${PURPOSE}\n\n## Requirements\n`; + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), emptied); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(emptied); + // The change is still there to fix and retry. + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + }); + + it('aborts instead of retiring when the emptied spec is also broken another way', async () => { + // "No requirements" is the only error retirement replaces. A spec that is + // additionally malformed is the author's to fix, so archive must abort as + // it always did rather than delete the evidence. + const changeName = 'retire-also-broken'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + // No `## Purpose` section at all: the rebuilt spec fails on that too. + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + `# legacy-layer Specification\n\n## Requirements\n\n${REQUIREMENT}\n` + ); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).resolves.not.toThrow(); + }); + + it('still writes the spec when requirements remain after the removal', async () => { + const changeName = 'partial-removal'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const kept = [ + '### Requirement: The system SHALL provide a core layer', + 'The system SHALL provide a core layer to every consumer.', + '', + '#### Scenario: Core is available', + '- **WHEN** a consumer imports the core', + '- **THEN** the core layer is available', + ].join('\n'); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + mainSpec('legacy-layer', `${REQUIREMENT}\n\n${kept}`) + ); + + await archiveCommand.execute(changeName, { yes: true }); + + const updated = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(updated).toContain('core layer'); + expect(updated).not.toContain('legacy layer is available'); + }); + + it('keeps a nested capability alive under a retiring parent', async () => { + const changeName = 'retire-parent-of-nested'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const nestedDir = path.join(mainSpecDir, 'sub'); + await fs.mkdir(nestedDir, { recursive: true }); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpec('legacy-layer')); + await fs.writeFile(path.join(nestedDir, 'spec.md'), mainSpec('sub')); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow(); + await expect(fs.access(path.join(nestedDir, 'spec.md'))).resolves.not.toThrow(); + }); + + // path.resolve collapses `..` but does NOT resolve symlinks, and readdir and + // rmdir both follow them. A string-prefix bound therefore let the prune walk + // delete directories anywhere on disk through a symlinked capability path. + it.skipIf(process.platform === 'win32')( + 'never prunes directories outside the real specs root through a symlink', + async () => { + const changeName = 'retire-through-symlink'; + await createChange(changeName, 'platform/legacy-layer', REMOVE_ALL); + const outside = path.join(tempDir, 'outside', 'platform'); + const linkedCapability = path.join(outside, 'legacy-layer'); + await fs.mkdir(linkedCapability, { recursive: true }); + await fs.writeFile(path.join(linkedCapability, 'spec.md'), mainSpec('legacy-layer')); + await fs.symlink(outside, path.join(tempDir, 'openspec', 'specs', 'platform'), 'dir'); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/resolves outside/); + + await expect(fs.access(path.join(linkedCapability, 'spec.md'))).resolves.not.toThrow(); + await expect(fs.access(linkedCapability)).resolves.not.toThrow(); + await expect(fs.access(outside)).resolves.not.toThrow(); + } + ); + + it('does not delete anything until every spec write has succeeded', async () => { + // Retirement is the only irreversible step, and the write loop is not + // transactional, so a sibling that fails validation must leave the + // retiring spec on disk and the change unarchived. + const changeName = 'retire-with-failing-sibling'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const badDeltaDir = path.join(changeDir, 'specs', 'other-layer'); + await fs.mkdir(badDeltaDir, { recursive: true }); + await fs.writeFile( + path.join(badDeltaDir, 'spec.md'), + // A requirement with no scenario: rebuilds fine, fails spec validation. + '# Other Layer - Changes\n\n## ADDED Requirements\n\n### Requirement: The system SHALL do a new thing\nThe system SHALL do a new thing.\n' + ); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).resolves.not.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + }); + + it('applies a retirement and an ordinary update in the same archive', async () => { + const changeName = 'retire-and-add'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const addDeltaDir = path.join(changeDir, 'specs', 'core-layer'); + await fs.mkdir(addDeltaDir, { recursive: true }); + await fs.writeFile( + path.join(addDeltaDir, 'spec.md'), + [ + '# Core Layer - Changes', + '', + '## ADDED Requirements', + '', + '### Requirement: The system SHALL provide a core layer', + 'The system SHALL provide a core layer to every consumer.', + '', + '#### Scenario: Core is available', + '- **WHEN** a consumer imports the core', + '- **THEN** the core layer is available', + '', + ].join('\n') + ); + const legacyDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(legacyDir, { recursive: true }); + await fs.writeFile(path.join(legacyDir, 'spec.md'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(legacyDir)).rejects.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'specs', 'core-layer', 'spec.md')) + ).resolves.not.toThrow(); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Totals: + 1, ~ 0, - 1, → 0') + ); + }); + + it('counts a rename applied on the way to the removal', async () => { + const changeName = 'retire-after-rename'; + await createChange( + changeName, + 'legacy-layer', + [ + '# Legacy Layer - Changes', + '', + '## RENAMED Requirements', + '', + '- FROM: `### Requirement: The system SHALL serve old clients`', + '- TO: `### Requirement: The system SHALL provide a legacy layer`', + '', + '## REMOVED Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + '**Reason**: The capability is retired.', + '**Migration**: None.', + '', + ].join('\n') + ); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + mainSpec( + 'legacy-layer', + [ + '### Requirement: The system SHALL serve old clients', + 'The system SHALL serve old clients over the v1 endpoint.', + '', + '#### Scenario: Old client calls v1', + '- **WHEN** an old client calls v1', + '- **THEN** the response is served', + ].join('\n') + ) + ); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(mainSpecDir)).rejects.toThrow(); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Totals: + 0, ~ 0, - 1, → 1') + ); + }); + + + it('deletes nothing when the user declines the spec update', async () => { + const { confirm } = await import('@inquirer/prompts'); + vi.mocked(confirm).mockResolvedValue(false); + const changeName = 'retire-declined'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), original); + + await archiveCommand.execute(changeName, {}); + + await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(original); + }); + + it('reports nothing to retire when the spec vanished before the write', async () => { + // Guards the `if (retired)` branch: a racing deletion must not be counted + // as a retirement this run. + const update = { + id: 'legacy-layer', + source: path.join(tempDir, 'nope', 'spec.md'), + target: path.join(tempDir, 'openspec', 'specs', 'gone', 'spec.md'), + exists: false, + }; + + await expect( + retireSpec( + update, + path.join(tempDir, 'openspec', 'specs') + ) + ).resolves.toEqual({ retired: false }); + expect(console.log).not.toHaveBeenCalledWith(expect.stringContaining('Retiring')); + }); + + + // The archive destination is settled from the change name alone, so a + // collision is knowable before anything is touched. Discovering it after the + // merge deleted a spec for an archive that then never happened. + it('checks the archive destination before deleting anything', async () => { + const changeName = 'retire-colliding'; + 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'), mainSpec('legacy-layer')); + await fs.mkdir( + path.join(tempDir, 'openspec', 'changes', 'archive', `${formatLocalDate()}-${changeName}`), + { recursive: true } + ); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /already exists/ + ); + + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).resolves.not.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + }); + + it('keeps the retiring spec on disk when a later spec write fails', async () => { + // The validation pass runs before both loops, so only a failing WRITE + // proves deletions really are deferred to the end. + const changeName = 'retire-with-failing-write'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + // `zz-` keeps the retirement first in the prepared order, so an + // undeferred deletion would land before the failing write. + const otherDelta = path.join(changeDir, 'specs', 'zz-other-layer'); + await fs.mkdir(otherDelta, { recursive: true }); + await fs.writeFile( + path.join(otherDelta, 'spec.md'), + [ + '# Other - Changes', + '', + '## ADDED Requirements', + '', + '### Requirement: The system SHALL do a new thing', + 'The system SHALL do a new thing.', + '', + '#### Scenario: It happens', + '- **WHEN** invoked', + '- **THEN** it happens', + '', + ].join('\n') + ); + const legacyDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(legacyDir, { recursive: true }); + await fs.writeFile(path.join(legacyDir, 'spec.md'), mainSpec('legacy-layer')); + // Make the second spec's write throw, by putting a directory where its + // file belongs. Read-only permissions would be a no-op on Windows; this + // fails the write on every platform. + await fs.mkdir(path.join(tempDir, 'openspec', 'specs', 'zz-other-layer', 'spec.md'), { + recursive: true, + }); + + await archiveCommand.execute(changeName, { yes: true }).catch(() => undefined); + + await expect(fs.access(path.join(legacyDir, 'spec.md'))).resolves.not.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + }); + + it('prunes a whole chain of emptied parents, not just one level', async () => { + const changeName = 'retire-deep'; + await createChange(changeName, 'a/b/legacy-layer', REMOVE_ALL); + const deep = path.join(tempDir, 'openspec', 'specs', 'a', 'b', 'legacy-layer'); + await fs.mkdir(deep, { recursive: true }); + await fs.writeFile(path.join(deep, 'spec.md'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(tempDir, 'openspec', 'specs', 'a'))).rejects.toThrow(); + await expect(fs.access(path.join(tempDir, 'openspec', 'specs'))).resolves.not.toThrow(); + }); + + it('never prunes a sibling directory that merely shares the specs-root prefix', async () => { + const specsRoot = path.join(tempDir, 'openspec', 'specs'); + const sibling = path.join(tempDir, 'openspec', 'specs-extra', 'legacy-layer'); + await fs.mkdir(sibling, { recursive: true }); + await fs.writeFile(path.join(sibling, 'spec.md'), mainSpec('legacy-layer')); + + await expect( + retireSpec( + { id: 'legacy-layer', source: 'x', target: path.join(sibling, 'spec.md'), exists: true }, + specsRoot, + { silent: true } + ) + ).rejects.toThrow(/resolves outside/); + + await expect(fs.access(path.join(sibling, 'spec.md'))).resolves.not.toThrow(); + await expect(fs.access(sibling)).resolves.not.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'specs-extra')) + ).resolves.not.toThrow(); + }); + + it.skipIf(process.platform === 'win32')( + 'prunes even when the specs root is itself named through a symlink', + async () => { + const realRoot = path.join(tempDir, 'openspec', 'specs'); + const linkedRoot = path.join(tempDir, 'specs-link'); + await fs.symlink(realRoot, linkedRoot, 'dir'); + const capability = path.join(realRoot, 'legacy-layer'); + await fs.mkdir(capability, { recursive: true }); + await fs.writeFile(path.join(capability, 'spec.md'), mainSpec('legacy-layer')); + + await retireSpec( + { + id: 'legacy-layer', + source: 'x', + target: path.join(capability, 'spec.md'), + exists: true, + }, + linkedRoot, + { silent: true } + ); + + await expect(fs.access(capability)).rejects.toThrow(); + } + ); + + it('retires both capabilities when one archive empties two', async () => { + const changeName = 'retire-two'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const secondDelta = path.join(changeDir, 'specs', 'second-layer'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile( + path.join(secondDelta, 'spec.md'), + REMOVE_ALL.replace('Legacy Layer', 'Second Layer') + ); + for (const capability of ['legacy-layer', 'second-layer']) { + const dir = path.join(tempDir, 'openspec', 'specs', capability); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(path.join(dir, 'spec.md'), mainSpec(capability)); + } + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(path.join(tempDir, 'openspec', 'specs', 'legacy-layer'))).rejects.toThrow(); + await expect(fs.access(path.join(tempDir, 'openspec', 'specs', 'second-layer'))).rejects.toThrow(); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Totals: + 0, ~ 0, - 2, → 0') + ); + }); + + it.skipIf(process.platform === 'win32')( + 'rejects deltas whose capability paths resolve to the same spec', + async () => { + const changeName = 'aliased-spec-updates'; + const changeDir = await createChange( + changeName, + 'a', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const secondDelta = path.join(changeDir, 'specs', 'b'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile(path.join(secondDelta, 'spec.md'), REMOVE_ALL); + + const realCapability = path.join(tempDir, 'openspec', 'specs', 'a'); + const aliasCapability = path.join(tempDir, 'openspec', 'specs', 'b'); + const target = path.join(realCapability, 'spec.md'); + await fs.mkdir(realCapability, { recursive: true }); + const original = mainSpec('a'); + await fs.writeFile(target, original); + await fs.symlink(realCapability, aliasCapability, 'dir'); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/resolve to the same target/); + + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'rejects missing spec targets beneath aliased capability directories', + async () => { + const changeName = 'aliased-missing-spec-updates'; + const changeDir = await createChange( + changeName, + 'a', + `## ADDED Requirements + +### Requirement: Behavior A +The system SHALL provide behavior A. + +#### Scenario: Behavior A is available +- **WHEN** A is requested +- **THEN** A is available +` + ); + const secondDelta = path.join(changeDir, 'specs', 'b'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile( + path.join(secondDelta, 'spec.md'), + `## ADDED Requirements + +### Requirement: Behavior B +The system SHALL provide behavior B. + +#### Scenario: Behavior B is available +- **WHEN** B is requested +- **THEN** B is available +` + ); + const realCapability = path.join(tempDir, 'openspec', 'specs', 'a'); + const aliasCapability = path.join(tempDir, 'openspec', 'specs', 'b'); + await fs.mkdir(realCapability, { recursive: true }); + await fs.symlink(realCapability, aliasCapability, 'dir'); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/resolve to the same target/); + + await expect(fs.access(path.join(realCapability, 'spec.md'))).rejects.toThrow(); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it('preserves a concurrent edit made immediately before an ordinary write', async () => { + const changeName = 'write-race-before-mutate'; + const changeDir = await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + await fs.writeFile(target, mainSpec('legacy-layer')); + const concurrent = `${mainSpec('legacy-layer')}\nConcurrent edit.\n`; + + const realMkdir = fs.mkdir.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let edited = false; + vi.spyOn(fs, 'mkdir').mockImplementation(async (candidate, options) => { + const result = await realMkdir(candidate, options); + if ( + !edited && + String(candidate).endsWith( + `${path.sep}openspec${path.sep}specs${path.sep}legacy-layer` + ) + ) { + edited = true; + await fs.writeFile(target, concurrent); + } + return result; + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/changed before archive could write them/); + + expect(edited).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(concurrent); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('preserves a concurrent edit made immediately before retirement', async () => { + const changeName = 'retire-race-before-mutate'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const specsRoot = path.join(tempDir, 'openspec', 'specs'); + const targetDir = path.join(specsRoot, 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + const concurrent = `${mainSpec('legacy-layer')} +### Requirement: A concurrent requirement +The system SHALL preserve a concurrent requirement. + +#### Scenario: Concurrent requirement is available +- **WHEN** it is requested +- **THEN** it is available +`; + await fs.writeFile(target, mainSpec('legacy-layer')); + + const realRealpath = fs.realpath.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let edited = false; + vi.spyOn(fs, 'realpath').mockImplementation(async (candidate, options) => { + const result = await realRealpath(candidate, options as never); + if ( + !edited && + String(candidate).endsWith(`${path.sep}openspec${path.sep}specs`) + ) { + edited = true; + await fs.writeFile(target, concurrent); + } + return result; + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/changed before archive could retire them/); + + expect(edited).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(concurrent); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('preserves an edit that races the atomic retirement displacement', async () => { + const changeName = 'retire-race-at-displacement'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + await fs.writeFile(target, mainSpec('legacy-layer')); + const concurrent = `${mainSpec('legacy-layer')} +### Requirement: A concurrent requirement +The system SHALL preserve a concurrent requirement. + +#### Scenario: Concurrent requirement is available +- **WHEN** it is requested +- **THEN** it is available +`; + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let edited = false; + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + !edited && + String(source).endsWith( + `${path.sep}openspec${path.sep}specs${path.sep}legacy-layer${path.sep}spec.md` + ) && + String(destination).includes('.openspec-retire-') + ) { + edited = true; + await fs.writeFile(target, concurrent); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/changed while archive was securing it for retirement/); + + expect(edited).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(concurrent); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('does not retire when authorization is removed at the displacement boundary', async () => { + const changeName = 'retire-authorization-race-at-displacement'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const metadata = path.join(changeDir, '.openspec.yaml'); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(target, original); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let authorizationRemoved = false; + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + !authorizationRemoved && + String(source).endsWith( + `${path.sep}openspec${path.sep}specs${path.sep}legacy-layer${path.sep}spec.md` + ) && + String(destination).includes('.openspec-retire-') + ) { + authorizationRemoved = true; + await fs.writeFile( + metadata, + 'schema: spec-driven\nretire_capabilities: false\n' + ); + } + return realRename(source, destination); + }); + + let failure: unknown; + try { + await archiveCommand.execute(changeName, { yes: true }); + } catch (error) { + failure = error; + } + + expect(authorizationRemoved).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.readFile(metadata, 'utf-8')).resolves.toContain( + 'retire_capabilities: false' + ); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + expect(failure).toEqual( + expect.objectContaining({ + message: expect.stringMatching(/retirement authorization changed/), + }) + ); + }); + + it('rolls back retirement when authorization changes during the final move', async () => { + const changeName = 'retire-authorization-race-at-final-move'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const metadata = path.join(changeDir, '.openspec.yaml'); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(target, original); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let authorizationRemoved = false; + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + !authorizationRemoved && + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + authorizationRemoved = true; + await fs.writeFile( + metadata, + 'schema: spec-driven\nretire_capabilities: false\n' + ); + } + return realRename(source, destination); + }); + + let failure: unknown; + try { + await archiveCommand.execute(changeName, { yes: true }); + } catch (error) { + failure = error; + } + + expect(authorizationRemoved).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.readFile(metadata, 'utf-8')).resolves.toContain( + 'retire_capabilities: false' + ); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + expect(failure).toEqual( + expect.objectContaining({ + message: expect.stringMatching(/retirement authorization changed/), + }) + ); + }); + + it('restores a retired spec when the final archive move fails', async () => { + const changeName = 'retire-final-move-failure'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(target, original); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) + ) { + throw Object.assign(new Error('move denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/move denied/); + + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('restores an ordinary write when the final archive move fails', async () => { + const changeName = 'write-final-move-failure'; + const changeDir = await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(target, original); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) + ) { + throw Object.assign(new Error('move denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/move denied/); + + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it.skipIf(process.platform === 'win32')( + 'preserves the mode of an updated spec under a restrictive umask', + async () => { + const changeName = 'write-preserves-mode'; + await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + await fs.writeFile(target, mainSpec('legacy-layer')); + await fs.chmod(target, 0o664); + const previousUmask = process.umask(0o077); + onTestFinished(() => process.umask(previousUmask)); + + await archiveCommand.execute(changeName, { yes: true }); + + expect((await fs.stat(target)).mode & 0o777).toBe(0o664); + } + ); + + it.skipIf(process.platform === 'win32')( + 'preserves existing hard-link identity when updating a spec', + async () => { + const changeName = 'write-preserves-hard-link'; + await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + const linked = path.join(targetDir, 'linked-spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + await fs.writeFile(target, mainSpec('legacy-layer')); + await fs.link(target, linked); + const originalInode = (await fs.stat(target, { bigint: true })).ino; + + await archiveCommand.execute(changeName, { yes: true }); + + expect((await fs.stat(target, { bigint: true })).ino).toBe(originalInode); + expect((await fs.stat(linked, { bigint: true })).ino).toBe(originalInode); + await expect(fs.readFile(linked, 'utf-8')).resolves.toContain( + '### Requirement: A replacement behavior' + ); + } + ); + + it.skipIf(process.platform === 'win32')( + 'preserves a retired hard-link inode when the final archive move fails', + async () => { + const changeName = 'retire-hard-link-rollback'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + const linked = path.join(targetDir, 'linked-spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + await fs.writeFile(target, mainSpec('legacy-layer')); + await fs.link(target, linked); + const originalInode = (await fs.stat(target, { bigint: true })).ino; + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('final move denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /final move denied/ + ); + + expect((await fs.stat(target, { bigint: true })).ino).toBe(originalInode); + expect((await fs.stat(linked, { bigint: true })).ino).toBe(originalInode); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'retains a displaced backup changed through an open handle before commit cleanup', + async () => { + const changeName = 'retire-open-handle-race'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + await fs.writeFile(target, mainSpec('legacy-layer')); + const openTarget = await fs.open(target, 'r+'); + onTestFinished(() => openTarget.close().catch(() => undefined)); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let edited = false; + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + const result = await realRename(source, destination); + if ( + !edited && + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + edited = true; + await openTarget.truncate(0); + await openTarget.writeFile('concurrent content through open handle\n'); + await openTarget.sync(); + await openTarget.close(); + } + return result; + }); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /displaced spec changed.*backup was retained for recovery/s + ); + + expect(edited).toBe(true); + await expect(fs.access(target)).rejects.toThrow(); + await expect(fs.access(changeDir)).rejects.toThrow(); + const backup = (await fs.readdir(targetDir)).find((entry) => + entry.includes('.openspec-retire-') + ); + expect(backup).toBeDefined(); + await expect(fs.readFile(path.join(targetDir, backup!), 'utf-8')).resolves.toBe( + 'concurrent content through open handle\n' + ); + } + ); + + it('rolls back when a delta changes during the final archive move', async () => { + const changeName = 'delta-race-at-final-move'; + const changeDir = await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const delta = path.join(changeDir, 'specs', 'legacy-layer', 'spec.md'); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(target, original); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + let edited = false; + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + !edited && + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) + ) { + edited = true; + await fs.appendFile(delta, '\nConcurrent delta edit.\n'); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/archived delta.*changed during the final move/); + + expect(edited).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.readFile(delta, 'utf-8')).resolves.toContain('Concurrent delta edit.'); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('rolls back when a staged delta changes during the fallback copy', async () => { + const changeName = 'delta-race-during-fallback-copy'; + const changeDir = await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const delta = path.join(changeDir, 'specs', 'legacy-layer', 'spec.md'); + const targetDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(targetDir, 'spec.md'); + await fs.mkdir(targetDir, { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(target, original); + + const realRename = fs.rename.bind(fs); + const realCopyFile = fs.copyFile.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('cross-device move'), { code: 'EXDEV' }); + } + return realRename(source, destination); + }); + let edited = false; + vi.spyOn(fs, 'copyFile').mockImplementation(async (source, destination, mode) => { + await realCopyFile(source, destination, mode); + if ( + !edited && + String(source).includes(`${path.sep}.openspec-move-`) && + String(source).endsWith( + `${path.sep}specs${path.sep}legacy-layer${path.sep}spec.md` + ) + ) { + edited = true; + await fs.appendFile(source, '\nConcurrent staged delta edit.\n'); + } + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/active delta.*changed during the fallback copy/); + + expect(edited).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.readFile(delta, 'utf-8')).resolves.toContain( + 'Concurrent staged delta edit.' + ); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + await expect( + fs.access( + path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ) + ) + ).rejects.toThrow(); + }); + + it('archives through the staged fallback when the destination rename gets EPERM', async () => { + const changeName = 'eperm-fallback-succeeds'; + const changeDir = await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const target = path.join( + tempDir, + 'openspec', + 'specs', + 'legacy-layer', + 'spec.md' + ); + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, mainSpec('legacy-layer')); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source) === changeDir && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('directory is busy'), { code: 'EPERM' }); + } + return realRename(source, destination); + }); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.access(changeDir)).rejects.toThrow(); + await expect(fs.readFile(target, 'utf-8')).resolves.toContain( + '### Requirement: A replacement behavior' + ); + await expect( + fs.access( + path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}`, + 'specs', + 'legacy-layer', + 'spec.md' + ) + ) + ).resolves.not.toThrow(); + }); + + it('rolls back specs when EPERM also prevents staging the active change', async () => { + const changeName = 'eperm-staging-fails'; + const changeDir = await createChange( + changeName, + 'legacy-layer', + `## ADDED Requirements + +### Requirement: A replacement behavior +The system SHALL provide a replacement behavior. + +#### Scenario: Replacement is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const delta = path.join(changeDir, 'specs', 'legacy-layer', 'spec.md'); + const target = path.join( + tempDir, + 'openspec', + 'specs', + 'legacy-layer', + 'spec.md' + ); + await fs.mkdir(path.dirname(target), { recursive: true }); + const original = mainSpec('legacy-layer'); + await fs.writeFile(target, original); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) + ) { + throw Object.assign(new Error('directory is busy'), { code: 'EPERM' }); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/Could not safely stage/); + + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + await expect(fs.access(delta)).resolves.not.toThrow(); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + await expect( + fs.access( + path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ) + ) + ).rejects.toThrow(); + await expect( + fs.access(archiveClaimPath(`${formatLocalDate()}-${changeName}`)) + ).rejects.toThrow(); + expect( + (await fs.readdir(path.dirname(changeDir))).some((entry) => + entry.startsWith('.openspec-move-') + ) + ).toBe(false); + }); + + it('keeps applied specs when fallback retains a complete archive copy', async () => { + const changeName = 'retained-copy-keeps-specs'; + const changeDir = await createChange( + changeName, + 'updated-layer', + `## ADDED Requirements + +### Requirement: A new behavior +The system SHALL provide a new behavior. + +#### Scenario: New behavior is available +- **WHEN** it is requested +- **THEN** it is available +` + ); + const retiredDelta = path.join(changeDir, 'specs', 'legacy-layer'); + await fs.mkdir(retiredDelta, { recursive: true }); + await fs.writeFile(path.join(retiredDelta, 'spec.md'), REMOVE_ALL); + const updatedTarget = path.join( + tempDir, + 'openspec', + 'specs', + 'updated-layer', + 'spec.md' + ); + const retiredTarget = path.join( + tempDir, + 'openspec', + 'specs', + 'legacy-layer', + 'spec.md' + ); + await fs.mkdir(path.dirname(updatedTarget), { recursive: true }); + await fs.mkdir(path.dirname(retiredTarget), { recursive: true }); + await fs.writeFile(updatedTarget, mainSpec('updated-layer')); + await fs.writeFile(retiredTarget, mainSpec('legacy-layer')); + + const realRename = fs.rename.bind(fs); + const realRm = fs.rm.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith( + `${path.sep}openspec${path.sep}changes${path.sep}${changeName}` + ) && + String(destination).includes(`${path.sep}changes${path.sep}archive${path.sep}`) + ) { + throw Object.assign(new Error('cross-device move'), { code: 'EXDEV' }); + } + return realRename(source, destination); + }); + vi.spyOn(fs, 'rm').mockImplementation(async (candidate, options) => { + if ( + String(candidate).includes( + `${path.sep}openspec${path.sep}changes${path.sep}.openspec-move-` + ) + ) { + throw Object.assign(new Error('source cleanup failed'), { code: 'EACCES' }); + } + return realRm(candidate, options); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/complete destination was retained for recovery/); + + const archivePath = path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ); + await expect(fs.access(path.join(archivePath, 'specs'))).resolves.not.toThrow(); + await expect(fs.readFile(updatedTarget, 'utf-8')).resolves.toContain( + '### Requirement: A new behavior' + ); + await expect(fs.access(retiredTarget)).rejects.toThrow(); + }); + + it('rolls back earlier retirements when a later retirement fails', async () => { + const changeName = 'retire-two-rollback'; + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const secondDelta = path.join(changeDir, 'specs', 'second-layer'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile( + path.join(secondDelta, 'spec.md'), + REMOVE_ALL.replace('Legacy Layer', 'Second Layer') + ); + const targets = ['legacy-layer', 'second-layer'].map((capability) => + path.join(tempDir, 'openspec', 'specs', capability, 'spec.md') + ); + for (const [index, target] of targets.entries()) { + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, mainSpec(index === 0 ? 'legacy-layer' : 'second-layer')); + } + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}second-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/failed to delete/); + + for (const target of targets) { + await expect(fs.readFile(target, 'utf-8')).resolves.toContain('### Requirement:'); + } + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('keeps committed retirement state when one backup cleanup fails', async () => { + const changeName = 'retire-backup-cleanup-failure'; + const changeDir = await createChange(changeName, 'a-layer', REMOVE_ALL); + const secondDelta = path.join(changeDir, 'specs', 'z-layer'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile(path.join(secondDelta, 'spec.md'), REMOVE_ALL); + const targets = ['a-layer', 'z-layer'].map((capability) => + path.join(tempDir, 'openspec', 'specs', capability, 'spec.md') + ); + for (const [index, target] of targets.entries()) { + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, mainSpec(index === 0 ? 'a-layer' : 'z-layer')); + } + + const realUnlink = fs.unlink.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'unlink').mockImplementation(async (candidate) => { + if ( + String(candidate).includes( + `${path.sep}z-layer${path.sep}spec.md.openspec-retire-` + ) + ) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realUnlink(candidate); + }); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /change remains archived.*backup was retained for recovery/s + ); + + await expect(fs.access(changeDir)).rejects.toThrow(); + await expect( + fs.access( + path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ) + ) + ).resolves.not.toThrow(); + for (const target of targets) { + await expect(fs.access(target)).rejects.toThrow(); + } + await expect(fs.access(path.dirname(targets[0]))).rejects.toThrow(); + expect( + (await fs.readdir(path.dirname(targets[1]))).some((entry) => + entry.includes('.openspec-retire-') + ) + ).toBe(true); + }); + + it.skipIf(process.platform === 'win32')( + 'restores a retired symlink without overwriting its concurrently updated target', + async () => { + const changeName = 'retire-symlink-rollback'; + const changeDir = await createChange( + changeName, + 'a-layer', + REMOVE_ALL.replace('Legacy Layer', 'A Layer') + ); + const secondDelta = path.join(changeDir, 'specs', 'z-layer'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile( + path.join(secondDelta, 'spec.md'), + REMOVE_ALL.replace('Legacy Layer', 'Z Layer') + ); + + const shared = path.join(tempDir, 'shared-legacy.md'); + await fs.writeFile(shared, mainSpec('a-layer')); + const linkedSpec = path.join(tempDir, 'openspec', 'specs', 'a-layer', 'spec.md'); + await fs.mkdir(path.dirname(linkedSpec), { recursive: true }); + await fs.symlink(shared, linkedSpec); + + const secondSpec = path.join(tempDir, 'openspec', 'specs', 'z-layer', 'spec.md'); + await fs.mkdir(path.dirname(secondSpec), { recursive: true }); + await fs.writeFile(secondSpec, mainSpec('z-layer')); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}a-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + await realRename(source, destination); + await fs.writeFile(shared, 'concurrent update\n'); + return; + } + if ( + String(source).endsWith(`${path.sep}z-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /Path is outside the allowed directory/ + ); + + expect((await fs.lstat(linkedSpec)).isSymbolicLink()).toBe(true); + expect(await fs.readlink(linkedSpec)).toBe(shared); + await expect(fs.readFile(shared, 'utf-8')).resolves.toBe(mainSpec('a-layer')); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'preserves a concurrent replacement at a retired symlink path and restores the change', + async () => { + const changeName = 'retire-symlink-occupant'; + const changeDir = await createChange(changeName, 'a-layer', REMOVE_ALL); + const secondDelta = path.join(changeDir, 'specs', 'z-layer'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile(path.join(secondDelta, 'spec.md'), REMOVE_ALL); + + const shared = path.join(tempDir, 'shared-legacy.md'); + await fs.writeFile(shared, mainSpec('a-layer')); + const linkedSpec = path.join(tempDir, 'openspec', 'specs', 'a-layer', 'spec.md'); + await fs.mkdir(path.dirname(linkedSpec), { recursive: true }); + await fs.symlink(shared, linkedSpec); + const secondSpec = path.join(tempDir, 'openspec', 'specs', 'z-layer', 'spec.md'); + await fs.mkdir(path.dirname(secondSpec), { recursive: true }); + await fs.writeFile(secondSpec, mainSpec('z-layer')); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}a-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + await realRename(source, destination); + await fs.writeFile(linkedSpec, 'concurrent occupant\n'); + return; + } + if ( + String(source).endsWith(`${path.sep}z-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /Path is outside the allowed directory/ + ); + + expect((await fs.lstat(linkedSpec)).isSymbolicLink()).toBe(true); + expect(await fs.readlink(linkedSpec)).toBe(shared); + await expect(fs.readFile(shared, 'utf-8')).resolves.toBe(mainSpec('a-layer')); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'rolls back an ordinary write through a spec symlink when a later write fails', + async () => { + const changeName = 'write-symlink-rollback'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const modified = [ + '## MODIFIED Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide an updated legacy layer.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + '', + ].join('\n'); + const firstDeltaDir = path.join(changeDir, 'specs', 'a-layer'); + const secondDeltaDir = path.join(changeDir, 'specs', 'z-layer'); + await fs.mkdir(firstDeltaDir, { recursive: true }); + await fs.mkdir(secondDeltaDir, { recursive: true }); + await fs.writeFile(path.join(firstDeltaDir, 'spec.md'), modified); + await fs.writeFile(path.join(secondDeltaDir, 'spec.md'), REMOVE_ALL); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Done\n'); + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nretire_capabilities: true\n' + ); + + const shared = path.join(tempDir, 'shared-write.md'); + const original = mainSpec('a-layer'); + await fs.writeFile(shared, original); + const linkedSpec = path.join(tempDir, 'openspec', 'specs', 'a-layer', 'spec.md'); + await fs.mkdir(path.dirname(linkedSpec), { recursive: true }); + await fs.symlink(shared, linkedSpec); + const laterSpec = path.join(tempDir, 'openspec', 'specs', 'z-layer', 'spec.md'); + await fs.mkdir(path.dirname(laterSpec), { recursive: true }); + await fs.writeFile(laterSpec, mainSpec('z-layer')); + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}z-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /Path is outside the allowed directory/ + ); + + await expect(fs.readFile(shared, 'utf-8')).resolves.toBe(original); + expect((await fs.lstat(linkedSpec)).isSymbolicLink()).toBe(true); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it.skipIf(process.platform === 'win32')( + 'does not overwrite a concurrent chmod while rolling back an ordinary write', + async () => { + const changeName = 'write-mode-rollback-conflict'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const modified = [ + '## MODIFIED Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide an updated legacy layer.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + '', + ].join('\n'); + for (const [capability, delta] of [ + ['a-layer', modified], + ['z-layer', REMOVE_ALL], + ] as const) { + const deltaDir = path.join(changeDir, 'specs', capability); + await fs.mkdir(deltaDir, { recursive: true }); + await fs.writeFile(path.join(deltaDir, 'spec.md'), delta); + } + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Done\n'); + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nretire_capabilities: true\n' + ); + const writtenTarget = path.join( + tempDir, + 'openspec', + 'specs', + 'a-layer', + 'spec.md' + ); + const retiredTarget = path.join( + tempDir, + 'openspec', + 'specs', + 'z-layer', + 'spec.md' + ); + await fs.mkdir(path.dirname(writtenTarget), { recursive: true }); + await fs.mkdir(path.dirname(retiredTarget), { recursive: true }); + await fs.writeFile(writtenTarget, mainSpec('a-layer')); + await fs.writeFile(retiredTarget, mainSpec('z-layer')); + await fs.chmod(writtenTarget, 0o644); + + const realWriteFile = fs.writeFile.bind(fs); + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'writeFile').mockImplementation(async (candidate, data, options) => { + const result = await realWriteFile(candidate, data, options); + if (String(candidate).endsWith(`${path.sep}a-layer${path.sep}spec.md`)) { + await fs.chmod(candidate, 0o600); + } + return result; + }); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}z-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + throw Object.assign(new Error('later retirement failed'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow( + /rollback would overwrite a concurrent change/ + ); + + expect((await fs.stat(writtenTarget)).mode & 0o777).toBe(0o600); + await expect(fs.readFile(writtenTarget, 'utf-8')).resolves.toContain( + 'updated legacy layer' + ); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + } + ); + + it('preserves a concurrent replacement at a retired regular-file path', async () => { + const changeName = 'retire-regular-occupant'; + const changeDir = await createChange(changeName, 'a-layer', REMOVE_ALL); + const secondDelta = path.join(changeDir, 'specs', 'z-layer'); + await fs.mkdir(secondDelta, { recursive: true }); + await fs.writeFile(path.join(secondDelta, 'spec.md'), REMOVE_ALL); + const firstSpec = path.join(tempDir, 'openspec', 'specs', 'a-layer', 'spec.md'); + const secondSpec = path.join(tempDir, 'openspec', 'specs', 'z-layer', 'spec.md'); + for (const [target, capability] of [ + [firstSpec, 'a-layer'], + [secondSpec, 'z-layer'], + ] as const) { + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, mainSpec(capability)); + } + + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}a-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + await realRename(source, destination); + await fs.writeFile(firstSpec, 'concurrent regular occupant\n'); + return; + } + if ( + String(source).endsWith(`${path.sep}z-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/rollback would overwrite a concurrent change/); + + await expect(fs.readFile(firstSpec, 'utf-8')).resolves.toBe( + 'concurrent regular occupant\n' + ); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('continues restoring earlier writes after a later rollback conflict', async () => { + const changeName = 'rollback-continues-after-conflict'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const modified = [ + '## MODIFIED Requirements', + '', + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL provide an updated legacy layer.', + '', + '#### Scenario: Layer is available', + '- **WHEN** a consumer imports the layer', + '- **THEN** the legacy layer is available', + '', + ].join('\n'); + for (const [capability, delta] of [ + ['a-layer', modified], + ['b-layer', REMOVE_ALL], + ['z-layer', REMOVE_ALL], + ] as const) { + const deltaDir = path.join(changeDir, 'specs', capability); + await fs.mkdir(deltaDir, { recursive: true }); + await fs.writeFile(path.join(deltaDir, 'spec.md'), delta); + } + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nretire_capabilities: true\n' + ); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Done\n'); + + const targets = new Map(); + for (const capability of ['a-layer', 'b-layer', 'z-layer']) { + const target = path.join( + tempDir, + 'openspec', + 'specs', + capability, + 'spec.md' + ); + const original = mainSpec(capability); + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, original); + targets.set(target, original); + } + + const bTarget = [...targets.keys()].find((target) => + target.includes(`${path.sep}b-layer${path.sep}`) + )!; + const zTarget = [...targets.keys()].find((target) => + target.includes(`${path.sep}z-layer${path.sep}`) + )!; + const realRename = fs.rename.bind(fs); + onTestFinished(() => vi.restoreAllMocks()); + vi.spyOn(fs, 'rename').mockImplementation(async (source, destination) => { + if ( + String(source).endsWith(`${path.sep}b-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + await realRename(source, destination); + await fs.writeFile(bTarget, 'concurrent occupant\n'); + return; + } + if ( + String(source).endsWith(`${path.sep}z-layer${path.sep}spec.md`) && + String(destination).includes('.openspec-retire-') + ) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + return realRename(source, destination); + }); + + await expect( + archiveCommand.execute(changeName, { yes: true }) + ).rejects.toThrow(/rollback would overwrite a concurrent change/); + + const aTarget = [...targets.keys()].find((target) => + target.includes(`${path.sep}a-layer${path.sep}`) + )!; + await expect(fs.readFile(aTarget, 'utf-8')).resolves.toBe(targets.get(aTarget)); + await expect(fs.readFile(bTarget, 'utf-8')).resolves.toBe('concurrent occupant\n'); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + + it('does not retire under --no-validate, since nothing checked the result', async () => { + // The safety argument is the validator's verdict. With validation off + // there is none, so the pre-#1302 behavior stands: write the spec. + const changeName = 'retire-unvalidated'; + 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'), + `${mainSpec('legacy-layer')}\n## Notes\nHand-written notes worth keeping.\n` + ); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + const written = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(written).toContain('## Notes'); + expect(written).not.toContain('### Requirement:'); + }); + + it('refuses to retire while any ### heading remains under Requirements', async () => { + // A stray `### Requirements` under Purpose captures the validator's + // section lookup, so it reports "no requirements" for a spec that plainly + // still has one. A reader is not fooled, and neither is this guard. + const changeName = 'retire-residual-heading'; + 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', + '(a stray sub-heading a previous author left behind)', + '', + '## Requirements', + '', + '### Legacy note', + 'The system SHALL keep the legacy note until migration completes.', + '', + '#### Scenario: Note applies', + '- **WHEN** a reader consults the note', + '- **THEN** it applies', + '', + REQUIREMENT, + '', + ].join('\n') + ); + + await archiveCommand.execute(changeName, { yes: true }); + + const survived = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(survived).toContain('### Legacy note'); + }); + + it('does not claim a resolved path for an ordinary retirement', async () => { + // The temp root is itself reached through a symlink on macOS + // (/var -> /private/var), so comparing resolved-vs-canonical paths would + // decorate every retirement with a note that means nothing. + const changeName = 'retire-plain-path'; + 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'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true, json: true }); + + const payload = JSON.parse(lastJsonPayload()); + // The retirement warning carries no resolved-path suffix: the nominal + // path told the whole story. Asserted on the path, not on message prose. + const retirement = payload.archive.warnings.find((w: string) => + w.includes('capability retired') + ); + expect(retirement).toBeDefined(); + // Canonicalized for the same reason as the symlinked-spec.md test: the + // warning would print the resolved form, so comparing the raw tempDir + // would pass regardless of what the code did. + expect(retirement).not.toContain(await fs.realpath(tempDir)); + }); + + it.skipIf(process.platform === 'win32')( + 'refuses to retire through a capability symlink outside the specs tree', + async () => { + const changeName = 'retire-outside'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const outside = path.join(tempDir, 'outside', 'legacy-layer'); + await fs.mkdir(outside, { recursive: true }); + await fs.writeFile(path.join(outside, 'spec.md'), mainSpec('legacy-layer')); + await fs.symlink(outside, path.join(tempDir, 'openspec', 'specs', 'legacy-layer'), 'dir'); + + await archiveCommand.execute(changeName, { yes: true, json: true }); + + expect(process.exitCode).toBe(1); + expect(lastJsonPayload()).toContain('resolves outside'); + await expect(fs.access(path.join(outside, 'spec.md'))).resolves.not.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + } + ); + + // The veto must not depend on WHERE the heading sits. Anything after the + // last `### Requirement:` belongs to that block's raw and is discarded with + // it, so reading the rebuilt body only ever saw headings above the first + // requirement - and silently deleted the identical heading written below. + it.each(['before', 'after'])( + 'refuses to retire with a stray heading %s the requirement', + async (position) => { + const changeName = `retire-heading-${position}`; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const note = [ + '### Migration notes (hand-written, keep)', + 'Move consumers to v2 before deleting the shim.', + ].join('\n'); + const body = position === 'before' ? `${note}\n\n${REQUIREMENT}` : `${REQUIREMENT}\n\n${note}`; + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpec('legacy-layer', body)); + + await archiveCommand.execute(changeName, { yes: true }); + + const survived = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(survived).toContain('### Migration notes'); + expect(process.exitCode).toBe(1); + } + ); + + it('refuses to retire a reader-visible heading absorbed before a scenario', async () => { + const changeName = 'retire-indented-requirement'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const original = mainSpec( + 'legacy-layer', + [ + '### Requirement: The system SHALL provide a legacy layer', + 'The system SHALL preserve legacy behavior.', + '', + ' ### Requirement: Reader-visible', + 'The system SHALL keep this reader-visible requirement.', + '', + '#### Scenario: Legacy applies', + '- **WHEN** legacy behavior is requested', + '- **THEN** it remains available', + ].join('\n') + ); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + const target = path.join(mainSpecDir, 'spec.md'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile(target, original); + + await archiveCommand.execute(changeName, { yes: true }); + + await expect(fs.readFile(target, 'utf-8')).resolves.toBe(original); + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('### Requirement: Reader-visible') + ); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + }); + + it.skipIf(process.platform === 'win32')( + 'does not claim it deleted the target of a symlinked spec.md', + async () => { + // realpath follows the link; unlink removes the link and leaves the + // target alone. Naming the target would report a deletion that never + // happened. + const changeName = 'retire-symlinked-file'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const shared = path.join(tempDir, 'shared-legacy.md'); + await fs.writeFile(shared, mainSpec('legacy-layer')); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'legacy-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.symlink(shared, path.join(mainSpecDir, 'spec.md')); + + await archiveCommand.execute(changeName, { yes: true, json: true }); + + const payload = JSON.parse(lastJsonPayload()); + expect(payload.archive).toBeNull(); + expect(payload.status[0].message).toContain('Path is outside the allowed directory'); + expect((await fs.lstat(path.join(mainSpecDir, 'spec.md'))).isSymbolicLink()).toBe(true); + // The shared file really is still there. + await expect(fs.readFile(shared, 'utf-8')).resolves.toContain('### Requirement:'); + } + ); + + + it('reports a destination taken during the merge as a collision, not a raw errno', async () => { + // The pre-flight check cannot cover the whole merge, so the move itself + // has to name the same condition rather than leaking ENOTEMPTY. + const changeName = 'retire-raced'; + 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'), mainSpec('legacy-layer')); + const archived = path.join( + tempDir, + 'openspec', + 'changes', + 'archive', + `${formatLocalDate()}-${changeName}` + ); + // Claim the destination while the confirmation prompt is open. + const { confirm } = await import('@inquirer/prompts'); + onTestFinished(() => vi.mocked(confirm).mockReset()); + vi.mocked(confirm).mockImplementation(async () => { + await fs.mkdir(archived, { recursive: true }); + await fs.writeFile(path.join(archived, 'squatter.txt'), 'mine now\n'); + return true; + }); + + // Human mode: JSON mode never reaches the prompt, so the race cannot be + // staged there. The error carries the same diagnostic either way. + await expect(archiveCommand.execute(changeName, {})).rejects.toThrow(/already exists/); + await expect(fs.access(path.join(mainSpecDir, 'spec.md'))).resolves.not.toThrow(); + await expect( + fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) + ).resolves.not.toThrow(); + }); + + it('reports the retirement, and where it went, in the --json warnings', async () => { + const changeName = 'retire-json-warnings'; + 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'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true, json: true }); + + const payload = JSON.parse(lastJsonPayload()); + expect(payload.archive.warnings).toEqual( + expect.arrayContaining([ + expect.stringContaining( + 'legacy-layer - capability retired; deleted the main spec (all requirements removed' + + ', declared by retire_capabilities)' + ), + ]) + ); + // Purpose always goes with the file, so it is named alongside the rest, + // and a JSON consumer gets the recovery command too. + const notes = payload.archive.warnings.join('\n'); + expect(notes).toContain('Purpose'); + expect(notes).toContain('git checkout HEAD -- ":(top)openspec/specs/legacy-layer/spec.md"'); + }); + + it('claims no retirement for a spec that was already gone', async () => { + const changeName = 'retire-already-gone-json'; + await createChange(changeName, 'legacy-layer', REMOVE_ALL); + + await archiveCommand.execute(changeName, { yes: true, json: true }); + + const payload = JSON.parse(lastJsonPayload()); + expect(payload.archive.specsUpdated).toBe(false); + expect(payload.archive.totals).toEqual({ added: 0, modified: 0, removed: 0, renamed: 0 }); + expect(JSON.stringify(payload.archive.warnings ?? [])).not.toContain('capability retired'); + }); + + + describe('isRetirableSpec', () => { + const REQUIREMENTLESS = `# legacy-layer Specification\n\n## Purpose\n${PURPOSE}\n\n## Requirements\n`; + + it('is false for a spec that validates', async () => { + await expect( + isRetirableSpec('legacy-layer', mainSpec('legacy-layer')) + ).resolves.toBe(false); + }); + + it('is true when the only error is that it has no requirements', async () => { + await expect(isRetirableSpec('legacy-layer', REQUIREMENTLESS)).resolves.toBe(true); + }); + + it('is false for a different single error', async () => { + // No Purpose section: a real failure, but not the one retirement replaces. + await expect( + isRetirableSpec( + 'legacy-layer', + `# legacy-layer Specification\n\n## Requirements\n\n${REQUIREMENT}\n` + ) + ).resolves.toBe(false); + }); + + it('is false when another error accompanies the missing requirements', async () => { + // A requirement stranded under a trailing section: "no requirements" + // AND "header outside the main ## Requirements section". + const stranded = [ + '# legacy-layer Specification', + '', + '## Purpose', + PURPOSE, + '', + '## Requirements', + '', + '## Appendix', + '', + REQUIREMENT, + '', + ].join('\n'); + const report = await new Validator().validateSpecContent('legacy-layer', stranded); + const errors = report.issues.filter((issue) => issue.level === 'ERROR'); + // Guards the `every` rather than `some`: this shape carries the + // no-requirements error alongside at least one other. + expect(errors.length).toBeGreaterThan(1); + expect(errors.map((issue) => issue.message)).toContain( + VALIDATION_MESSAGES.SPEC_NO_REQUIREMENTS + ); + await expect(isRetirableSpec('legacy-layer', stranded)).resolves.toBe(false); + }); + }); + + it('reports the retirement in --json instead of printing progress lines', async () => { + const changeName = 'retire-json'; + 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'), mainSpec('legacy-layer')); + + await archiveCommand.execute(changeName, { yes: true, json: true }); + + await expect(fs.access(mainSpecDir)).rejects.toThrow(); + const calls = (console.log as unknown as ReturnType).mock.calls.map( + (call) => String(call[0]) + ); + // JSON mode prints exactly one payload and no human progress lines. + expect(calls.some((line) => line.includes('Retiring'))).toBe(false); + const payload = JSON.parse(calls[calls.length - 1]); + expect(payload.archive.specsUpdated).toBe(true); + expect(payload.archive.totals).toEqual({ added: 0, modified: 0, removed: 1, renamed: 0 }); + }); + }); + describe('non-interactive prompts (#1479)', () => { // An AI agent (or any script) runs the CLI with stdin closed, so every // prompt rejects with @inquirer's "User force closed the prompt with 0 diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index ac76fe60c7..a9aea3d822 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -42,20 +42,20 @@ const EXPECTED_FUNCTION_HASHES: Record = { getContinueChangeSkillTemplate: '676e7472977d2b6f4d922ce384db1f15020c195f94d6cd4ee71abcf0201e28a9', getApplyChangeSkillTemplate: '031cf8f8ffc2937fc4051651bd5e1fc6159bfd225605d8e4c3181054a4e52b38', getFfChangeSkillTemplate: '225a8eaf1b3769ac5d43e079297c5fa9cc20fc2e34fec9bb0d887c8c1fb0ea71', - getSyncSpecsSkillTemplate: '6824990431141eba855c9560cded184c53a44985e14ba354032fe5deedd270b4', + getSyncSpecsSkillTemplate: '904469b74b53021ca43f73f2b64a83080015707f49d7e6d913e2e1adb35ccb9f', getOnboardSkillTemplate: '856b5f451f45093f8906967da29b4e0479c7c271e401eab2ef58165800a67284', getOpsxExploreCommandTemplate: 'e9674ddace813e685b0e9fe37149140a3d33d48aa20b9ba2b0963a7c49c9aea7', getOpsxNewCommandTemplate: '652adc870f16bb260d54436356132b6ee051a9ed7cc0464603fb31f4db259762', getOpsxContinueCommandTemplate: 'bcf0ad1c55b71346147c5b4dbaed016c77c9718f960012d8efc9d3d2089d0e00', getOpsxApplyCommandTemplate: '18c82fc48e65084065171e44f811db8fdc96bd6cb0f61fe8f31324207f4861c7', getOpsxFfCommandTemplate: '678375642a21d255444f0ba717e659abb2cc2b7474981d52eae900a0793e3e4d', - getArchiveChangeSkillTemplate: '7c1bf2170ba57833f111c79002ea56be3cca499e2b13b2ea8141c182351b1a3b', - getBulkArchiveChangeSkillTemplate: 'de198c7b7c1472773b013b9af917de27773fd613083309f0e8e607c005c92d3d', - getOpsxSyncCommandTemplate: 'e30b1e1e7070da3521e3878065b400ced7b6260e532fd348df96df75d9d7f2e3', + getArchiveChangeSkillTemplate: 'ee27b4c15a2f13bbb0ab0ceb5f4b10fa5e19dd70128ca58dd3f482c1f2a8f97f', + getBulkArchiveChangeSkillTemplate: 'e67a6fae6553e01c9930bd08f11465a205637fec6c72726b0cfa1a735920bba4', + getOpsxSyncCommandTemplate: '40c7ff54221918dc6cae8a4d376a6e8897a6926721a337fb1f037f0000861d54', getVerifyChangeSkillTemplate: '917de96cc8341799107b0617979cdaf30e121c51676272f5caef143b090583f9', - getOpsxArchiveCommandTemplate: 'fa0d2f4c1ff9b499353399ba040caaf2ba070154dac8b94cb4ca8e2568b1717a', + getOpsxArchiveCommandTemplate: '729fcdc9be6af7abb65f4ed3400ce6e95eef256d3660cfaeac3ef07e89144671', getOpsxOnboardCommandTemplate: '3fda1bb6ce52cdb240d1ade84319ea44160aef79573052ce58b77eb662de98a1', - getOpsxBulkArchiveCommandTemplate: '93355fb7bc13e549e8646e4dc48db6f98ac5372545dff3cf3970c4f45f55c5f7', + getOpsxBulkArchiveCommandTemplate: '87a003ac49d0303a5b77dc935bcff1d830ca5434b129ba788d5d44253f814f87', getOpsxVerifyCommandTemplate: '29e3913c93566e689971d8c15c3348ba4169ebf6b1d403f5ac9974605c734baa', getOpsxProposeSkillTemplate: '06a8f7d272db8d3cb113dc05d606630d1e5aedd267c2722e971d1175e0d8bb40', getOpsxProposeCommandTemplate: 'ed3ad596d9bb238830b4fcbe566e3c1ba9d0db62f4a92cdb28c38262dc3f04df', @@ -70,9 +70,9 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-continue-change': '2e1a7d17ec021949d115c72227729609bf9980ad1f23445af117c09834711121', 'openspec-apply-change': '49fc5772404e3033085384ee214c44488c93880a596a9a05dcad42f9ce86cf83', 'openspec-ff-change': '4228d75e3571097164f2360e2ad3063a5b88d44750078c3601b23a89e74c1de6', - 'openspec-sync-specs': 'c7aff2b41cab0ba87257ea8a2b4892c34192f21f75e5924ab65490cfa924e66b', - 'openspec-archive-change': '84b9d3a5690b8d64e1845b3c7368a4ad43369ea8549a76ef78912690d434363b', - 'openspec-bulk-archive-change': '5ac320e2004e453c78541233f48e5f6e246cc674a44f1e427cecb7b2e9587f9b', + 'openspec-sync-specs': '668a2044cb1688969279357c04034ba6ef6c2d38123cc3f6ae9b5e7761baac04', + 'openspec-archive-change': '7fde55c06ae896b5f628b00e0dddceb9f86fdbf431e3f4dcd5fc0aacac16d808', + 'openspec-bulk-archive-change': '789d60d5874eab9714aef6a0b1109af0af2f9f23f57767b4641db89849dd0fce', 'openspec-verify-change': '1c3f73a36be691a18d3acb200d22e6874004d6d4a5d3e2e346ae95a7379e9da8', 'openspec-onboard': '6eb124af3a9f35efe601ff373406fad93447a1375e0bb4e27a35b0c3fd476851', 'openspec-propose': '6b49634d3672e7fef4750a8c7572a661fec0dafe6d52a0075b41a2c87a793871', @@ -717,4 +717,33 @@ describe('skill templates split parity', () => { ); } }); + + // A golden hash proves the generated file matches its source, never that the + // source is right - so a careless `regen:parity-hashes` over a dropped + // paragraph passes CI silently. The sync skill is the one place an agent + // learns that retiring a capability needs the marker; pin the fact, not the + // hash, so losing the guidance fails here instead of shipping. + it('tells the sync skill that retirement needs the retire_capabilities marker', () => { + const sync = getSkillTemplates().find( + ({ dirName }) => dirName === 'openspec-sync-specs' + ); + expect(sync, 'openspec-sync-specs template').toBeTruthy(); + const variants = [ + ['sync skill', sync!.template.instructions], + ['sync command', getOpsxSyncCommandTemplate().content], + ] as const; + for (const [variant, text] of variants) { + expect(text, variant).toContain('retire_capabilities: true'); + expect(text, variant).toContain('every other nonblank line in the whole file is accounted for'); + expect(text, variant).toContain('resolves inside the real specs root'); + expect(text, variant).toContain('checkout-scoped recovery guidance'); + expect(text, variant).toContain('do not modify the main spec'); + expect(text, variant).toMatch(/Stop\s+the sync for that capability/); + expect(text, variant).toContain( + 'Never write or leave an empty `## Requirements` section' + ); + expect(text, variant).not.toContain('any other sections'); + expect(text, variant).not.toContain('Loose prose left under `## Requirements` does NOT block'); + } + }); }); diff --git a/test/specs/source-specs-normalization.test.ts b/test/specs/source-specs-normalization.test.ts index 1169e8a26a..2611a85f9d 100644 --- a/test/specs/source-specs-normalization.test.ts +++ b/test/specs/source-specs-normalization.test.ts @@ -35,6 +35,39 @@ async function getSpecFiles(): Promise { } describe('source-of-truth specs normalization', () => { + it('reports duplicate canonical requirement names', () => { + const content = [ + '# Capability', + '', + '## Purpose', + 'A purpose.', + '', + '## Requirements', + '', + '### Requirement: Same name', + 'The first definition.', + '', + '#### Scenario: First', + '- **WHEN** something happens', + '- **THEN** the first result occurs', + '', + '### Requirement: Same name', + 'The second definition.', + '', + '#### Scenario: Second', + '- **WHEN** something else happens', + '- **THEN** the second result occurs', + '', + ].join('\n'); + + expect(findMainSpecStructureIssues(content)).toEqual([ + expect.objectContaining({ + kind: 'duplicate-requirement', + message: expect.stringContaining('Same name'), + }), + ]); + }); + it('enforces required sections and bans hidden requirements, placeholders, and delta headers', async () => { const files = await getSpecFiles(); expect(files.length).toBeGreaterThan(0);