diff --git a/.changeset/sync-lifecycle-status.md b/.changeset/sync-lifecycle-status.md new file mode 100644 index 0000000000..513f06bf15 --- /dev/null +++ b/.changeset/sync-lifecycle-status.md @@ -0,0 +1,13 @@ +--- +"@fission-ai/openspec": minor +--- + +Add `openspec sync`, which folds a change's delta specs into the main specs without archiving it, and an optional `status: proposed | shipped` field in a change's `.openspec.yaml`. + +`openspec sync --check` gates on one property: a change that claims to be shipped has its deltas in `specs/`. A proposed change passes for free, so the check is green as its resting state and red only on a real mistake — unlike a check for "is everything archived?", which is red for the whole life of every open pull request. It reads only files on disk, so a pre-commit hook, a pre-push hook and CI run the same command and agree. + +`openspec list --status ` filters changes by that field. + +Everything here is opt-in and inert by default. The `status` field is absent unless a project writes it, nothing generates it, and `archive` is unchanged. + +Designed by [@ixxie](https://github.com/ixxie) in [#1683](https://github.com/Fission-AI/OpenSpec/issues/1683) — the diagnosis that `archive` welds a state transition to a text merge, `shipped ⇒ folded` as a predicate over the working tree, and the standalone `sync` that makes it checkable. This ships a smaller, additive subset of that proposal. diff --git a/docs-lab/reference/cli.md b/docs-lab/reference/cli.md index e9e4eb14fd..229efdc5a8 100644 --- a/docs-lab/reference/cli.md +++ b/docs-lab/reference/cli.md @@ -22,6 +22,7 @@ | [`openspec show`](#openspec-show) | Print a change or spec, as markdown or JSON. | | [`openspec view`](#openspec-view) | One-screen dashboard of specs and changes. | | [`openspec validate`](#openspec-validate) | Check changes and specs for structural issues. | +| [`openspec sync`](#openspec-sync) | Fold a change's delta specs into the main specs, without archiving it. | | [`openspec archive`](#openspec-archive) | Move a completed change to the archive and update the main specs. | **Workflows and schemas** @@ -410,6 +411,7 @@ Rows come from `openspec/changes/` and `openspec/specs/` under the resolved root | `--specs` | List specs instead of changes. | | `--changes` | List changes. This is the default. | | `--sort ` | `recent` (last modified first) or `name`. Default: `recent`. Specs always sort by name. | +| `--status ` | Only list changes in this lifecycle state: `proposed` or `shipped`. A change with no `status` in its `.openspec.yaml` counts as `proposed`. | | `--json` | Print JSON instead of the table. | | `--store ` | Use a registered store as the OpenSpec root instead of the current project. | @@ -867,6 +869,100 @@ exit $validationExit These custom views keep the full report's keys but omit clean items. They are neither complete full-v1 reports nor the versioned `--report findings` shape. +## openspec sync + +Folds a change's delta specs into the main specs, without archiving the change. + +```bash +openspec sync add-rate-limit # fold one change now; nothing moves +openspec sync add-rate-limit --ship # mark it shipped and fold it, in one set of changes +openspec sync # fold every change declaring status: shipped +openspec sync --check # exit 1 if a shipped change has unfolded deltas +``` + +`archive` folds and moves in one step, so the fold can only happen at the moment the +change is finished. `sync` separates them: the specs can be brought up to date while +the change is still open, and CI can check that they are. + +**Arguments** + +| Argument | What it is | +|---|---| +| `change-name` | The change to sync. Omitted, every change declaring `status: shipped` | + +**Options** + +| Flag | Effect | +|---|---| +| `--check` | Report shipped changes with unfolded deltas and exit 1. Writes nothing. | +| `--ship` | Fold the named change, then set `status: shipped` on it. If the fold fails, the field is not set. | +| `-y, --yes` | Sync even when the change has incomplete tasks. | +| `--no-validate` | Skip validation. | +| `--json` | Print a structured result instead of text. | +| `--store ` | Use a registered store as the OpenSpec root. | + +**The lifecycle field** + +A change may declare where it sits, in its `.openspec.yaml`: + +```yaml +schema: spec-driven +status: shipped +``` + +Optional and absent by default. No `status` means `proposed`, which is what a change +under `changes/` has always meant. Nothing writes the field on its own. + +**The gate** + +`openspec sync --check` asserts that a change claiming to be shipped has its deltas in +`specs/`. A proposed change passes for free, so green is the resting state: + +``` +✓ 1 shipped change(s) are folded into the main specs. +``` + +and red names both the gap and the fix: + +``` +Sync check failed: + + add-rate-limit + api: +1 not applied + +Run openspec sync to fold them, then commit the result. +``` + +It reads only files on disk — no VCS history, no timing — so a pre-commit hook, a +pre-push hook and CI run the same command and agree. + +**Output** + +``` +Applying changes to openspec/specs/api/spec.md: + + 1 added +Totals: + 1, ~ 0, - 0, → 0 +Specs updated successfully. +``` + +Running it again reports `Specs already in sync; no files changed.` — and so does +`openspec archive` afterwards, because re-applying a folded delta is a no-op. + +**What it will not do** + +Sync never deletes a spec. When a change's `REMOVED` entries take a capability's last +requirement, retiring it deletes the file, which stays with `openspec archive` behind +the `retire_capabilities` marker. Sync reports the case and names archive instead. + +Sync also never examines archived changes: their deltas are history, superseded by +whatever came after. + +**Exit codes** + +- `0`: the specs were folded, or `--check` found nothing wrong. +- `1`: `--check` found an unfolded shipped change, validation failed, tasks were + incomplete, or the change was not found. + ## openspec archive Moves a completed change to the archive and updates the main specs. diff --git a/docs/cli.md b/docs/cli.md index e74bc60f2d..7725dad59a 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -13,7 +13,7 @@ The OpenSpec CLI (`openspec`) provides terminal commands for project setup, vali | **Personal worksets** | `workset create`, `workset list`, `workset open`, `workset remove` | Keep and open personal, local working views in your tool | | **Browsing** | `list`, `view`, `show` | Explore changes and specs | | **Validation** | `validate` | Check changes and specs for issues | -| **Lifecycle** | `archive` | Finalize completed changes | +| **Lifecycle** | `sync`, `archive` | Fold delta specs into the main specs, and finalize completed changes | | **Workflow** | `new change`, `status`, `instructions`, `templates`, `schemas` | Artifact-driven workflow support | | **Schemas** | `schema init`, `schema fork`, `schema validate`, `schema which` | Create and manage custom workflows | | **Config** | `config` | View and modify settings | @@ -443,6 +443,7 @@ openspec list [options] | `--specs` | List specs instead of changes | | `--changes` | List changes (default) | | `--sort ` | Sort by `recent` (default) or `name` | +| `--status ` | Only list changes in this lifecycle state: `proposed` or `shipped`. A change whose `.openspec.yaml` has no `status` counts as `proposed` | | `--json` | Output as JSON | **Examples:** @@ -626,6 +627,107 @@ Validating add-dark-mode... ## Lifecycle Commands +### `openspec sync` + +Fold a change's delta specs into the main specs, without archiving the change. + +``` +openspec sync [change-name] [options] +``` + +`archive` does two things at once: it folds a change's deltas into `openspec/specs/` +and it moves the change folder. `sync` does only the first, so the specs can be +brought up to date while the change is still open for review — and so CI can check +that they are. + +**Arguments:** + +| Argument | Required | Description | +|----------|----------|-------------| +| `change-name` | No | Change to sync. Omitted, `sync` acts on every change that declares `status: shipped` | + +**Options:** + +| Option | Description | +|--------|-------------| +| `--check` | Report shipped changes whose deltas are not in the main specs and exit 1. Writes nothing | +| `--ship` | Fold the named change, then set `status: shipped` on it — both land in one set of file changes for you to commit. If the fold fails, the field is not set | +| `-y, --yes` | Sync even when the change still has incomplete tasks | +| `--no-validate` | Skip validation (not recommended) | +| `--json` | Structured output for hooks and CI | +| `--store ` | Use a registered store as the OpenSpec root | + +**The lifecycle field.** A change's `.openspec.yaml` may declare where it sits: + +```yaml +schema: spec-driven +status: shipped # or: proposed +``` + +The field is optional and absent by default. A change with no `status` is +`proposed`, which is what every change under `changes/` has always meant, so a +project that never opts in is unaffected. Nothing writes the field on its own — +not `openspec new change`, not `archive`. + +If the fold fails — validation, incomplete tasks, a retirement, a write error — +the field is not set. `--ship` writes `status: shipped` only after the specs are +correct, so a failed run never leaves a change claiming to be shipped with its +deltas absent. + +**The CI gate.** `openspec sync --check` asserts one property: *a change that +claims to be shipped has its deltas in `specs/`*. A proposed change passes for +free, so the check is green as its resting state and red only on a real mistake — +unlike "is everything archived?", which is red for the entire life of every open +PR. It reads only files on disk, so a pre-commit hook, a pre-push hook and CI run +the same command and reach the same verdict. + +```bash +# CI, pre-commit, pre-push — same command +openspec sync --check +``` + +**Examples:** + +```bash +# Fold one change's deltas now; the change stays where it is +openspec sync add-rate-limit + +# Mark it shipped and fold it, so both land in one commit when you make it +openspec sync add-rate-limit --ship + +# Fold every change that declares status: shipped +openspec sync + +# Gate: exits 1 if any shipped change has unfolded deltas +openspec sync --check + +# Which changes have claimed to be shipped but aren't archived yet +openspec list --status shipped +``` + +**What it does:** + +1. Validates the change's delta specs (unless `--no-validate`) +2. Refuses a change with incomplete tasks, unless `--yes` — folding a change + nothing implements yet writes requirements into `specs/` that aren't true +3. Validates every rebuilt spec before writing any of them, so a late failure + leaves the whole tree unchanged +4. Writes the updated main specs. Nothing moves; nothing is deleted + +**What it deliberately does not do:** + +- **It never deletes a spec.** When a change's `REMOVED` entries take a + capability's last requirement, retiring that capability deletes its `spec.md`. + That stays with `openspec archive`, behind the `retire_capabilities` marker. + `sync` reports the case and points you there. +- **It never checks archived changes.** Archived deltas are history, and later + changes supersede them. `--check` looks only at active changes that declare + `status: shipped` — a set that drains itself as those changes archive. + +**Syncing early does not change archiving.** Re-applying a delta that is already +in the main specs is a no-op, so `openspec archive` afterwards reports +`Specs already in sync` and moves the folder exactly as it always did. + ### `openspec archive` Archive a completed change and merge delta specs into main specs. diff --git a/docs/commands.md b/docs/commands.md index 7546dae82d..84ad92fe25 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -444,6 +444,8 @@ AI: Verifying add-dark-mode... **Optional command.** Merge delta specs from a change into main specs. Archive will prompt to sync if needed, so you typically don't need to run this manually. +> Not the same as the CLI's `openspec sync`. This one is the agent doing the merge in your session. `openspec sync` is a deterministic terminal command that does the same fold without a model, and carries the `--check` gate for CI — see [CLI](cli.md#openspec-sync). + **Syntax:** ``` /opsx:sync [change-name] diff --git a/docs/glossary.md b/docs/glossary.md index 345125f38a..790b550fb5 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -38,7 +38,9 @@ Terms are grouped by topic, then alphabetized within each group. **Archive.** The act of finishing a change. Its delta specs merge into the main specs, and the change folder moves to `openspec/changes/archive/YYYY-MM-DD-/`. After archiving, your specs describe the new reality. See [Concepts](concepts.md#archive). -**Sync.** Merging a change's delta specs into the main specs *without* archiving the change. Usually automatic (archive offers to do it), but available on its own as `/opsx:sync` for long-running changes. See [Commands](commands.md#opsxsync). +**Sync.** Merging a change's delta specs into the main specs *without* archiving the change. Usually automatic (archive offers to do it). Available on its own two ways: `/opsx:sync`, where the agent does the merge ([Commands](commands.md#opsxsync)), and `openspec sync`, the deterministic CLI command ([CLI](cli.md#openspec-sync)). + +**Shipped / proposed.** A change may declare its lifecycle state as `status: proposed | shipped` in its `.openspec.yaml`. The field is optional and absent by default; no `status` means `proposed`. `openspec sync --check` gates on it — a change that claims to be shipped must have its deltas in the main specs — which makes the specs enforceable in CI without a check that is red for the whole life of every PR. See [OpenSpec on a Team](team-workflow.md#enforcing-it-in-ci). ## Workflow and commands diff --git a/docs/team-workflow.md b/docs/team-workflow.md index 76c83817b8..6b240fa18a 100644 --- a/docs/team-workflow.md +++ b/docs/team-workflow.md @@ -55,6 +55,38 @@ Archiving folds a change's deltas into your main `openspec/specs/` and moves the Pick one and be consistent. Either way, `/opsx:archive` checks that tasks are complete and offers to sync first, so nothing merges half-finished by accident. +## Enforcing it in CI + +The obvious CI check — "nothing is left unarchived" — doesn't work, because it's red for the whole life of every PR. An open change sits in `changes/`, unarchived, precisely because it isn't finished. A gate that is red as its resting state is one everyone learns to ignore. + +`openspec sync --check` is the check that works. It asks a different question: **does anything that claims to be shipped still have deltas missing from `specs/`?** A change that hasn't made that claim passes for free, so green is the resting state and red means a real mistake. + +```yaml +# .github/workflows/specs.yml +- run: npx openspec sync --check +``` + +The claim is one line in the change's `.openspec.yaml`: + +```yaml +schema: spec-driven +status: shipped +``` + +The everyday shape of it: + +1. Open the PR. The change is `proposed` (the default — nothing to write). The gate is green. +2. When the work is done and reviewed, mark it shipped and fold its deltas in one step: + ```bash + openspec sync add-rate-limit --ship + ``` + That sets `status: shipped` and writes the deltas into `specs/` in one command, so both land in the same set of file changes for you to commit together. OpenSpec never runs git itself — commit the result as usual. +3. Merge. Archive whenever you like afterwards — re-applying a delta that's already folded is a no-op, so `openspec archive` behaves exactly as it always did. + +The check is a pure function of the files on disk, so the same command works as a pre-commit hook, a pre-push hook, and the CI gate, and all three agree. + +`openspec list --status shipped` shows which changes have made the claim but aren't archived yet. + ## Two people, parallel changes Because changes are separate folders, they don't collide: diff --git a/openspec/changes/add-standalone-spec-sync/.openspec.yaml b/openspec/changes/add-standalone-spec-sync/.openspec.yaml new file mode 100644 index 0000000000..2e24cfa4fa --- /dev/null +++ b/openspec/changes/add-standalone-spec-sync/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-07 diff --git a/openspec/changes/add-standalone-spec-sync/proposal.md b/openspec/changes/add-standalone-spec-sync/proposal.md new file mode 100644 index 0000000000..4b1d22f723 --- /dev/null +++ b/openspec/changes/add-standalone-spec-sync/proposal.md @@ -0,0 +1,74 @@ +# Let a change's specs be folded before it is archived + +## Why + +`archive` does two separable jobs in one command. It folds a change's deltas into +`openspec/specs/`, and it declares the change finished by moving its directory. +Welding them means the fold can only happen at the moment the move happens, which +on a team that reviews before merging is after the pull request closes. + +So a team that wants CI to assert "the specs describe what shipped" has nothing to +assert during review. The only property expressible today is "nothing is left +unarchived", and that is violated by design for the entire life of every open PR: +the change sits in `changes/`, unarchived, precisely because it is not finished. +A gate that is red as its resting state is one everyone learns to ignore, and it +masks the real failures underneath (#1683). + +The fix is to make the check conditional on the change's own claim — not "is +everything archived?" but "does anything claiming to be shipped still have deltas +missing from the specs?" A proposed change passes for free, so green is the +resting state and red means a real mistake. + +## What Changes + +- **`openspec sync [change]`** folds delta specs into the main specs without + archiving. The merge engine already supports this: re-applying a folded delta + is a no-op it names the "early-sync pattern", so `archive` afterwards behaves + exactly as it always did. +- **`openspec sync --check`** asserts `shipped ⇒ folded` over the working tree and + exits 1 with the offending changes named. A pure function of files on disk, so + a pre-commit hook, a pre-push hook and CI run one command and agree. +- **`status: proposed | shipped`** becomes an optional field in a change's + `.openspec.yaml`. Absent means `proposed`, which is what a change under + `changes/` has always meant. Nothing writes it: not `new change`, not `archive`. +- **`openspec sync --ship`** sets the field and folds in one working-tree + diff, so no intermediate commit claims a change is shipped while the specs say + otherwise. +- **`openspec list --status `** filters by the field, and renders a + lifecycle column only when some change in the root declares one. + +Two deliberate limits, both to keep this additive rather than a second lifecycle: + +- **Sync never deletes a spec.** Retiring a capability is the one irreversible + operation in the system; it stays with `archive`, behind the + `retire_capabilities` marker and its rollback-safe deletion. Sync reports the + case and names archive. +- **Sync never examines archived changes.** Their deltas are history and later + changes supersede them; re-applying a months-old delta over everything that + came after is a merge conflict, not a drift check. The checked set is the + active changes declaring `shipped`, which drains itself as they archive. + +"Folded" is decided by running the merge builder and seeing that it applied zero +operations — the same predicate `archive` uses to decide it has nothing to write. +Not a byte-comparison of the rebuilt output: the rebuild normalizes blank lines, +so a hand-formatted main spec would compare unequal while being perfectly in +sync. Sharing archive's own predicate is also what stops the checker and the doer +from drifting apart (#1112). + +## Impact + +- Affected specs: `cli-sync` (ADDED), `cli-list` (MODIFIED: filtering) +- Affected code: `src/core/sync.ts` (new), `src/core/list.ts`, + `src/utils/change-metadata.ts`, `src/core/change-metadata/schema.ts`, + `src/cli/index.ts`, `src/core/completions/command-registry.ts`, + `src/core/archive.ts` (two helpers exported, no behavior change) +- Affected docs: `docs/cli.md`, `docs/team-workflow.md`, + `docs-lab/reference/cli.md` + +Credit: the design is Matan Bendix Shenhav's, from #1683 and his implementation +#1684. His: the diagnosis, `shipped ⇒ folded` as a tree predicate (V), the +checker-versus-doer argument (IV), the standalone idempotent `sync` (III), status +as data (I and II), and shipping in one working-tree diff (VI). This change takes +a smaller, additive subset — no mode, no layout change, no migration — and +decides folded-ness by archive's zero-operations predicate rather than his +byte-identical regeneration. diff --git a/openspec/changes/add-standalone-spec-sync/specs/cli-list/spec.md b/openspec/changes/add-standalone-spec-sync/specs/cli-list/spec.md new file mode 100644 index 0000000000..e80d7cc220 --- /dev/null +++ b/openspec/changes/add-standalone-spec-sync/specs/cli-list/spec.md @@ -0,0 +1,29 @@ +## ADDED Requirements + +### Requirement: Lifecycle Status Filtering +The command SHALL be able to filter changes by their declared lifecycle state, and +SHALL surface that state without changing the output of a project that has never +declared one. + +#### Scenario: Filtering by state +- **WHEN** `openspec list --status shipped` is executed +- **THEN** only changes declaring `status: shipped` SHALL be listed +- **AND** `--status proposed` SHALL list every change that declares `proposed` or + declares no status at all + +#### Scenario: An unknown state is rejected +- **WHEN** `--status` is given a value other than `proposed` or `shipped` +- **THEN** the command SHALL exit 1 naming the accepted values +- **AND** SHALL NOT list every change as though the filter matched nothing + +#### Scenario: No lifecycle output without a declaration +- **WHEN** no change in the root declares a `status` +- **THEN** the human listing SHALL render no lifecycle column +- **AND** the JSON output SHALL carry no lifecycle key + +#### Scenario: The lifecycle appears once any change declares one +- **WHEN** at least one change declares a `status` +- **THEN** the human listing SHALL render a lifecycle column, showing `proposed` + for changes that declare nothing +- **AND** the JSON output SHALL carry a `lifecycle` key for the declaring changes + only, leaving the existing `status` key meaning task progress diff --git a/openspec/changes/add-standalone-spec-sync/specs/cli-sync/spec.md b/openspec/changes/add-standalone-spec-sync/specs/cli-sync/spec.md new file mode 100644 index 0000000000..5854223a1d --- /dev/null +++ b/openspec/changes/add-standalone-spec-sync/specs/cli-sync/spec.md @@ -0,0 +1,142 @@ +# Sync Command Specification + +## Purpose + +The `openspec sync` command SHALL fold a change's delta specs into the main specs +without archiving the change, and SHALL provide a check that a change claiming to +be shipped has its deltas present in the main specs. + +## ADDED Requirements + +### Requirement: Lifecycle Status Field +A change SHALL be able to declare its lifecycle state as data in its +`.openspec.yaml`, using an optional `status` field whose value is `proposed` or +`shipped`. A change that does not declare one SHALL be treated as `proposed`. + +#### Scenario: Undeclared status reads as proposed +- **WHEN** a change's `.openspec.yaml` has no `status` field, or the change has no + metadata file at all +- **THEN** every reader SHALL treat the change as `proposed` +- **AND** no command SHALL write the field on the change's behalf + +#### Scenario: A status that cannot be determined is not rounded to proposed +- **WHEN** a change's metadata mentions `status` but cannot be honored, because the + file does not parse, carries an unknown value, or names a schema that does not + resolve +- **THEN** the state SHALL be reported as undetermined with its reason +- **AND** `openspec sync --check` SHALL fail rather than pass the change + +#### Scenario: Broken metadata that never mentions status is left alone +- **WHEN** a change's metadata cannot be honored and does not mention `status` +- **THEN** the change SHALL read as `proposed` +- **AND** `openspec sync --check` SHALL NOT report it + +### Requirement: Folding Delta Specs +The command SHALL apply a change's delta specs to the main specs, leaving the +change directory where it is. + +#### Scenario: Folding a named change +- **WHEN** `openspec sync ` is executed +- **THEN** each delta under the change's `specs/` SHALL be applied to its main spec +- **AND** the change directory SHALL NOT be moved +- **AND** the change's declared status SHALL NOT affect whether it is folded + +#### Scenario: Folding every shipped change +- **WHEN** `openspec sync` is executed with no change name +- **THEN** every active change declaring `status: shipped` SHALL be folded +- **AND** a change declaring no status SHALL NOT be folded + +#### Scenario: Folding is idempotent +- **WHEN** `openspec sync` is run against a change whose deltas are already in the + main specs +- **THEN** no file SHALL be written +- **AND** the command SHALL report that the specs are already in sync + +#### Scenario: Archiving after a sync is unaffected +- **WHEN** a change is folded by `openspec sync` and later archived +- **THEN** `openspec archive` SHALL apply zero operations and write no spec file +- **AND** the change SHALL be moved to the archive as it always was + +### Requirement: Shipped Changes Are Folded +The command SHALL provide a check that asserts one property over the working tree: +every change claiming to be shipped has its deltas present in the main specs. + +#### Scenario: A proposed change passes for free +- **WHEN** `openspec sync --check` is executed and no active change declares + `status: shipped` +- **THEN** the command SHALL exit 0 +- **AND** SHALL write no file + +#### Scenario: A shipped change with unfolded deltas fails the check +- **WHEN** `openspec sync --check` is executed and an active change declaring + `status: shipped` has a delta that is not in its main spec +- **THEN** the command SHALL exit 1 +- **AND** SHALL name the change and each capability whose delta is unapplied +- **AND** SHALL name the command that folds them +- **AND** SHALL write no file + +#### Scenario: Folded-ness is decided by the merge builder +- **WHEN** deciding whether a change's deltas are present in the main specs +- **THEN** the decision SHALL be that re-applying the delta produces zero applied + operations, which is the same predicate the archive command uses to decide it + has nothing to write +- **AND** SHALL NOT be a byte comparison against a rebuilt spec + +#### Scenario: Archived changes are never examined +- **WHEN** `openspec sync --check` is executed +- **THEN** only active changes SHALL be examined +- **AND** a change that has been archived SHALL NOT be checked + +### Requirement: Sync Never Deletes A Spec +The command SHALL NOT delete a main spec under any circumstance. Retiring a +capability remains the archive command's operation. + +#### Scenario: A retirement is handed to archive +- **WHEN** a change's REMOVED entries would take a capability's last requirement +- **THEN** `openspec sync` SHALL refuse to fold that change +- **AND** SHALL name `openspec archive` as the command that performs a retirement +- **AND** the main spec file SHALL remain on disk + +#### Scenario: The check reports a retirement without offering sync as the fix +- **WHEN** `openspec sync --check` finds a shipped change that would retire a + capability +- **THEN** the command SHALL exit 1 naming the retirement +- **AND** SHALL NOT tell the user to run `openspec sync` + +### Requirement: Guards Before Writing +The command SHALL run the same guards the archive command runs before it writes a +main spec. + +#### Scenario: Delta specs are validated +- **WHEN** a change's delta specs fail validation and `--no-validate` was not passed +- **THEN** the command SHALL refuse the change and write no file + +#### Scenario: Incomplete tasks block the fold +- **WHEN** a change has incomplete tasks and `--yes` was not passed +- **THEN** the command SHALL refuse the change and write no file +- **AND** SHALL name the rerun that proceeds anyway + +#### Scenario: Every rebuilt spec is validated before any is written +- **WHEN** any rebuilt spec would fail validation +- **THEN** no spec file SHALL be written at all + +#### Scenario: A fold that does not settle is named +- **WHEN** two shipped changes claim the same requirement in ways that cannot both + hold, so re-evaluating after the write still reports unfolded deltas +- **THEN** the command SHALL name the changes involved +- **AND** SHALL NOT retry the fold + +### Requirement: Shipping In One Diff +The command SHALL offer to set a change's status and fold it in a single run, so +that no intermediate commit claims a change is shipped while its deltas are absent +from the main specs. + +#### Scenario: Marking a change shipped and folding it +- **WHEN** `openspec sync --ship` is executed +- **THEN** the change's `.openspec.yaml` SHALL be set to `status: shipped` +- **AND** its deltas SHALL be folded in the same run +- **AND** the metadata file's comments and key order SHALL be preserved + +#### Scenario: Ship is refused where it cannot apply +- **WHEN** `--ship` is passed with `--check`, or with no change name +- **THEN** the command SHALL refuse and say which flag combination is valid diff --git a/openspec/changes/add-standalone-spec-sync/tasks.md b/openspec/changes/add-standalone-spec-sync/tasks.md new file mode 100644 index 0000000000..56ee1301ef --- /dev/null +++ b/openspec/changes/add-standalone-spec-sync/tasks.md @@ -0,0 +1,25 @@ +## 1. Lifecycle field + +- [x] 1.1 Add optional `status: proposed | shipped` to `ChangeMetadataSchema` +- [x] 1.2 Add `readChangeStatus`, failing closed on metadata it cannot honor +- [x] 1.3 Add `writeChangeStatus`, preserving comments and key order + +## 2. Sync command + +- [x] 2.1 Add `src/core/sync.ts` with the fold and the `--check` predicate +- [x] 2.2 Run archive's guards before writing: validation, task completion, + rebuilt-spec validation +- [x] 2.3 Refuse retirements and name `openspec archive` instead +- [x] 2.4 Re-evaluate after writing so a non-convergent pair is named, not looped on +- [x] 2.5 Register the CLI command and its completion entry + +## 3. List filter + +- [x] 3.1 Add `--status `, counting an undeclared change as `proposed` +- [x] 3.2 Render the lifecycle column and the JSON `lifecycle` key only when declared + +## 4. Docs and verification + +- [x] 4.1 Document `openspec sync` and `list --status` +- [x] 4.2 Add the CI section to the team workflow guide +- [x] 4.3 Tests covering the gate, the guards, and the archive interaction diff --git a/skills/openspec-apply-change/SKILL.md b/skills/openspec-apply-change/SKILL.md index 098f63fecb..21bd424e2c 100644 --- a/skills/openspec-apply-change/SKILL.md +++ b/skills/openspec-apply-change/SKILL.md @@ -11,7 +11,7 @@ metadata: Implement tasks from an OpenSpec change. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. **Input**: Optionally specify a change name (e.g., `/openspec-apply-change add-auth`). If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. diff --git a/skills/openspec-archive-change/SKILL.md b/skills/openspec-archive-change/SKILL.md index 5f34ed53a7..0fe97ea943 100644 --- a/skills/openspec-archive-change/SKILL.md +++ b/skills/openspec-archive-change/SKILL.md @@ -11,7 +11,7 @@ metadata: Archive a completed change in the experimental workflow. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. `` is the spec directory relative to `specs/` (for example, `user-auth` or `identity/user-auth`). Preserve the full path from each delta spec when resolving its main spec. diff --git a/skills/openspec-bulk-archive-change/SKILL.md b/skills/openspec-bulk-archive-change/SKILL.md index 252e1dd155..f210e1214d 100644 --- a/skills/openspec-bulk-archive-change/SKILL.md +++ b/skills/openspec-bulk-archive-change/SKILL.md @@ -13,7 +13,7 @@ Archive multiple completed changes in a single operation. This skill allows you to batch-archive changes, handling spec conflicts intelligently by checking the codebase to determine what's actually implemented. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. `` is the spec directory relative to `specs/` (for example, `user-auth` or `identity/user-auth`). Preserve the full path from each delta spec when resolving its main spec. diff --git a/skills/openspec-continue-change/SKILL.md b/skills/openspec-continue-change/SKILL.md index 5991b06891..30e2cf5f05 100644 --- a/skills/openspec-continue-change/SKILL.md +++ b/skills/openspec-continue-change/SKILL.md @@ -11,7 +11,7 @@ metadata: Continue working on a change by creating the next artifact. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. **Input**: Optionally specify a change name. If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. diff --git a/skills/openspec-explore/SKILL.md b/skills/openspec-explore/SKILL.md index b597796fb1..fbe6ba0494 100644 --- a/skills/openspec-explore/SKILL.md +++ b/skills/openspec-explore/SKILL.md @@ -15,7 +15,7 @@ Enter explore mode. Think deeply. Visualize freely. Follow the conversation wher **This is a stance, not a workflow.** There are no fixed steps, no required sequence, no mandatory outputs. You're a thinking partner helping the user explore. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. --- diff --git a/skills/openspec-ff-change/SKILL.md b/skills/openspec-ff-change/SKILL.md index 72a95620bb..986e7d2905 100644 --- a/skills/openspec-ff-change/SKILL.md +++ b/skills/openspec-ff-change/SKILL.md @@ -11,7 +11,7 @@ metadata: Fast-forward through artifact creation - generate everything needed to start implementation in one go. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. **Input**: The user's request should include a change name (kebab-case) OR a description of what they want to build. diff --git a/skills/openspec-new-change/SKILL.md b/skills/openspec-new-change/SKILL.md index 9aea11d391..35f9b7b5e9 100644 --- a/skills/openspec-new-change/SKILL.md +++ b/skills/openspec-new-change/SKILL.md @@ -11,7 +11,7 @@ metadata: Start a new change using the experimental artifact-driven approach. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. **Input**: The user's request should include a change name (kebab-case) OR a description of what they want to build. diff --git a/skills/openspec-onboard/SKILL.md b/skills/openspec-onboard/SKILL.md index fb3f13bec7..aefee7f0c0 100644 --- a/skills/openspec-onboard/SKILL.md +++ b/skills/openspec-onboard/SKILL.md @@ -11,7 +11,7 @@ metadata: Guide the user through their first complete OpenSpec workflow cycle. This is a teaching experience—you'll do real work in their codebase while explaining each step. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. --- diff --git a/skills/openspec-propose/SKILL.md b/skills/openspec-propose/SKILL.md index 2d5709dfe1..fd7757f007 100644 --- a/skills/openspec-propose/SKILL.md +++ b/skills/openspec-propose/SKILL.md @@ -25,7 +25,7 @@ When the user is ready to implement, they must start the apply workflow explicit --- -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. **Input**: The user's request should include a change name (kebab-case) OR a description of what they want to build. diff --git a/skills/openspec-sync-specs/SKILL.md b/skills/openspec-sync-specs/SKILL.md index d12d56b857..ac6b8de945 100644 --- a/skills/openspec-sync-specs/SKILL.md +++ b/skills/openspec-sync-specs/SKILL.md @@ -13,7 +13,7 @@ Sync delta specs from a change to main specs. This is an **agent-driven** operation - you will read delta specs and directly edit main specs to apply the changes. This allows intelligent merging (e.g., adding a scenario without copying the entire requirement). -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. `` is the spec directory relative to `specs/` (for example, `user-auth` or `identity/user-auth`). Preserve the full path from each delta spec when resolving its main spec. diff --git a/skills/openspec-update-change/SKILL.md b/skills/openspec-update-change/SKILL.md index 24c9f88367..6bdaec2ada 100644 --- a/skills/openspec-update-change/SKILL.md +++ b/skills/openspec-update-change/SKILL.md @@ -11,7 +11,7 @@ metadata: Revise a change's existing planning artifacts and keep them coherent. Never edit code. -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. **Input**: Optionally specify a change name. If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. diff --git a/skills/openspec-verify-change/SKILL.md b/skills/openspec-verify-change/SKILL.md index 2165a6a910..e32eacbd31 100644 --- a/skills/openspec-verify-change/SKILL.md +++ b/skills/openspec-verify-change/SKILL.md @@ -11,7 +11,7 @@ metadata: Verify that an implementation matches the change artifacts (specs, tasks, design). -**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. +**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run `openspec store list --json` to discover registered store ids, then pass `--store ` on the commands that read or write specs and changes (`new change`, `status`, `instructions`, `list`, `show`, `validate`, `sync`, `archive`, `doctor`, `context`, `schemas`, `view`). Once selected, treat `--store ` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run `openspec status --change "" --json --store ""`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local `openspec/` root. **Input**: Optionally specify a change name. If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. diff --git a/src/cli/index.ts b/src/cli/index.ts index 75324cfa38..9c5175c57f 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -19,6 +19,7 @@ import { } from '../core/version-check.js'; import { ListCommand } from '../core/list.js'; import { ArchiveCommand, type ArchiveOptions } from '../core/archive.js'; +import { SyncCommand, type SyncOptions } from '../core/sync.js'; import { ViewCommand } from '../core/view.js'; import { resolveRootForCommand, toRootOutput } from '../core/root-selection.js'; import { registerSpecCommand } from '../commands/spec.js'; @@ -356,10 +357,11 @@ program .option('--specs', 'List specs instead of changes') .option('--changes', 'List changes explicitly (default)') .option('--sort ', 'Sort order: "recent" (default) or "name"', 'recent') + .option('--status ', 'Only list changes in this lifecycle state: proposed|shipped') .option('--json', 'Output as JSON (for programmatic use)') .option('--store ', STORE_OPTION_DESCRIPTION) .addOption(hiddenStorePathOption()) - .action(async (options?: { specs?: boolean; changes?: boolean; sort?: string; json?: boolean; store?: string; storePath?: string }) => { + .action(async (options?: { specs?: boolean; changes?: boolean; sort?: string; status?: string; json?: boolean; store?: string; storePath?: string }) => { try { const root = await resolveRootForCommand(options ?? {}, { json: options?.json, @@ -374,9 +376,23 @@ program const listCommand = new ListCommand(); const mode: 'changes' | 'specs' = options?.specs ? 'specs' : 'changes'; const sort = options?.sort === 'name' ? 'name' : 'recent'; + // Rejected rather than ignored: a typo would otherwise silently list + // everything, which reads as "no change has that state". + if (options?.status !== undefined && options.status !== 'proposed' && options.status !== 'shipped') { + throw new Error( + `Unknown --status '${options.status}'. Use 'proposed' or 'shipped'.` + ); + } + // A lifecycle state belongs to a change, not a spec, so the flag has + // nothing to filter in specs mode. Silently ignoring it would print the + // full spec list as though the filter had matched everything. + if (options?.status !== undefined && mode === 'specs') { + throw new Error('--status filters changes and cannot be combined with --specs.'); + } await listCommand.execute(root.path, mode, { sort, json: options?.json, + ...(options?.status ? { status: options.status as 'proposed' | 'shipped' } : {}), ...(options?.json ? { root: toRootOutput(root) } : {}), }); } catch (error) { @@ -495,6 +511,26 @@ program } }); +program + .command('sync [change-name]') + .description('Fold a change\'s spec deltas into the main specs without archiving it') + .option('--check', 'Report shipped changes whose deltas are not in the main specs; write nothing') + .option('--ship', 'Fold the named change, then mark it `status: shipped`') + .option('-y, --yes', 'Sync even when the change still has incomplete tasks') + .option('--no-validate', 'Skip validation (not recommended)') + .option('--json', 'Output as JSON (for hooks and CI)') + .option('--store ', STORE_OPTION_DESCRIPTION) + .addOption(hiddenStorePathOption()) + .action(async (changeName?: string, options?: SyncOptions) => { + try { + const syncCommand = new SyncCommand(); + await syncCommand.execute(changeName, options); + } catch (error) { + failWithError(error, { enabled: options?.json, fallbackCode: 'sync_error' }); + process.exit(1); + } + }); + registerSpecCommand(program); registerConfigCommand(program); registerSchemaCommand(program); diff --git a/src/core/archive.ts b/src/core/archive.ts index 888a6135a6..e526d31630 100644 --- a/src/core/archive.ts +++ b/src/core/archive.ts @@ -158,7 +158,12 @@ async function decideSpecOutcome( return built.counts.removed > 0 ? 'retire' : 'write'; } -async function listActiveChangeNames(changesDir: string): Promise { +/** + * Every change directory directly under `changes/`, excluding the archive. + * Exported so `openspec sync` enumerates the same set archive does - the two + * commands must never disagree about which changes are active. + */ +export async function listActiveChangeNames(changesDir: string): Promise { try { const entries = await fs.readdir(changesDir, { withFileTypes: true }); return entries @@ -816,35 +821,57 @@ async function fingerprintSpecInputs(update: SpecUpdate): Promise { return `${await fingerprintPath(update.source)}\n${await fingerprintPath(update.target)}`; } -async function mutationTargetIdentity(mutation: SpecMutation): Promise { +async function specTargetIdentity(target: string): Promise { try { - const stat = await fs.stat(mutation.update.target, { bigint: true }); + const stat = await fs.stat(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 parent = path.dirname(target); const realParent = await fs.realpath(parent).catch(() => path.resolve(parent)); - return `missing:${path.join(realParent, path.basename(mutation.update.target))}`; + return `missing:${path.join(realParent, path.basename(target))}`; } throw error; } } -async function assertDistinctMutationTargets(mutations: SpecMutation[]): Promise { +/** + * Refuse a run in which two capability ids resolve to the SAME file. + * + * `resolveTrustedSpecPath` deliberately permits a capability directory to be a + * symlink (monorepos point one at another), so two ids aliasing one spec is a + * shape the trust model allows rather than an exotic accident. Writing both in + * sequence is last-writer-wins: one capability's fold is silently destroyed and + * the other's requirements are filed under the wrong name. + * + * Shared with `openspec sync`, which writes the same targets - the two commands + * must not differ on which trees they are willing to write. + */ +export async function assertDistinctSpecTargets( + entries: Array<{ id: string; target: string }>, + action: string +): Promise { const owners = new Map(); - for (const mutation of mutations) { - const identity = await mutationTargetIdentity(mutation); + for (const entry of entries) { + const identity = await specTargetIdentity(entry.target); 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.` + `Spec updates for '${existing}' and '${entry.id}' resolve to the same target ` + + `${identity}. Replace the capability alias or combine the deltas before ${action}.` ); } - owners.set(identity, mutation.update.id); + owners.set(identity, entry.id); } } +async function assertDistinctMutationTargets(mutations: SpecMutation[]): Promise { + await assertDistinctSpecTargets( + mutations.map(({ update }) => ({ id: update.id, target: update.target })), + 'archiving' + ); +} + async function captureSpecSnapshots(mutations: SpecMutation[]): Promise { return Promise.all( mutations.map(async ({ update, outcome, rebuilt }) => { @@ -1050,6 +1077,66 @@ async function finalizeRetirementBackups( } } +/** + * Whether a change carries spec deltas that must be validated before its specs + * are folded into `openspec/specs/`. + * + * A `spec.md` at the `specs/` root is never merged, so archiving a change that + * has one drops its content whether or not it carries delta headers (#1385). + * Its existence alone forces validation, which reports it and blocks the run. A + * directory named `spec.md` is a normal capability folder, so only a regular + * file counts. + * + * A change that declares `skip_specs` must not carry any file under `specs/` - + * validate reports that as a conflict, so this has to run the same check + * instead of skipping validation because the files happen to have no delta + * headers. A marker that cannot be honored (skip_specs mentioned but the + * metadata fails the shared shape, or names a schema that does not resolve) + * also forces validation, so every caller and validate always agree about the + * marker. Unreadable specs/ fails closed into validation too. + * + * An UNMARKED zero-delta change returns false - a gap that predates the marker, + * kept here so `openspec sync` inherits archive's exact answer rather than a + * stricter one of its own. + * + * Exported so `archive`, `sync`, and anything else that folds deltas ask one + * question rather than three that drift. + */ +export async function changeHasDeltaSpecsToValidate(changeDir: string): Promise { + const changeSpecsDir = path.join(changeDir, 'specs'); + const rootSpecStat = await fs.stat(path.join(changeSpecsDir, 'spec.md')).catch(() => null); + let hasDeltaSpecs = rootSpecStat?.isFile() === true; + + if (!hasDeltaSpecs) { + const marker = readSkipSpecsMarker(changeDir); + if (marker.invalidReason) { + hasDeltaSpecs = true; + } else if (marker.declared) { + let specsDirHasFiles = true; + try { + specsDirHasFiles = await hasAnyFileUnder(changeSpecsDir); + } catch { + // fall through with true: let validation surface the conflict + } + hasDeltaSpecs = specsDirHasFiles; + } + } + + for (const { specFile } of hasDeltaSpecs ? [] : await discoverSpecFiles(changeSpecsDir)) { + try { + const content = await fs.readFile(specFile, 'utf-8'); + // Case-insensitive to match the delta parser, so a lowercase header + // routes through the same delta validation that validate runs. + if (/^##\s+(ADDED|MODIFIED|REMOVED|RENAMED)\s+Requirements/im.test(content)) { + hasDeltaSpecs = true; + break; + } + } catch {} + } + + return hasDeltaSpecs; +} + export class ArchiveCommand { async execute(changeName?: string, options: ArchiveOptions = {}): Promise { const json = !!options.json; @@ -1217,50 +1304,7 @@ export class ArchiveCommand { } // Validate delta-formatted spec files under the change directory if present - const changeSpecsDir = path.join(changeDir, 'specs'); - // A spec.md at the specs/ root is never merged, so archiving a change - // that has one drops its content whether or not it carries delta headers - // (#1385). Its existence alone must run validation, which reports it and - // blocks the archive. A directory named spec.md is a normal capability - // folder, so only a regular file counts. - const rootSpecStat = await fs.stat(path.join(changeSpecsDir, 'spec.md')).catch(() => null); - let hasDeltaSpecs = rootSpecStat?.isFile() === true; - // A change that declares skip_specs must not carry any file under - // specs/ — validate reports that as a conflict, so archive has to run - // the same check instead of skipping validation because the files - // happen to have no delta headers. A marker that cannot be honored - // (skip_specs mentioned but the metadata fails the shared shape, or - // names a schema that does not resolve) also - // forces validation, so archive and validate always agree about the - // marker. Unreadable specs/ fails closed into validation too. (An - // UNMARKED zero-delta change still archives with only non-blocking - // proposal warnings — a gap that predates the marker and is left - // unchanged here.) - if (!hasDeltaSpecs) { - const marker = readSkipSpecsMarker(changeDir); - if (marker.invalidReason) { - hasDeltaSpecs = true; - } else if (marker.declared) { - let specsDirHasFiles = true; - try { - specsDirHasFiles = await hasAnyFileUnder(changeSpecsDir); - } catch { - // fall through with true: let validation surface the conflict - } - hasDeltaSpecs = specsDirHasFiles; - } - } - for (const { specFile } of hasDeltaSpecs ? [] : await discoverSpecFiles(changeSpecsDir)) { - try { - const content = await fs.readFile(specFile, 'utf-8'); - // Case-insensitive to match the delta parser, so a lowercase header - // routes through the same delta validation that validate runs. - if (/^##\s+(ADDED|MODIFIED|REMOVED|RENAMED)\s+Requirements/im.test(content)) { - hasDeltaSpecs = true; - break; - } - } catch {} - } + const hasDeltaSpecs = await changeHasDeltaSpecsToValidate(changeDir); if (hasDeltaSpecs) { // No mainSpecsDir here on purpose: the scenario-loss check standalone // validate runs (#1477) is the same one buildUpdatedSpec enforces a few diff --git a/src/core/change-metadata/schema.ts b/src/core/change-metadata/schema.ts index 3644160052..7d706b0ec1 100644 --- a/src/core/change-metadata/schema.ts +++ b/src/core/change-metadata/schema.ts @@ -46,6 +46,18 @@ export const ChangeMetadataSchema = z.object({ // 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(), + // Where the change sits in its own lifecycle, as data rather than as a + // directory position. Optional and absent by default: a change with no + // `status` is `proposed`, which is what every change in `changes/` has always + // meant. Declaring `shipped` says "these deltas belong in `specs/` now", and + // is what `openspec sync --check` gates on - so a proposed change passes the + // gate for free and red means a real mistake, instead of a check that is red + // for the whole life of an open PR (#1683). + // + // Nothing writes this field on its own: `openspec new change` does not emit + // it, and `archive` neither reads nor stamps it. A project that never opts in + // never sees it. + status: z.enum(['proposed', 'shipped']).optional(), }); export type ChangeMetadata = z.infer; diff --git a/src/core/completions/command-registry.ts b/src/core/completions/command-registry.ts index 95e550a7ff..5e23ec2234 100644 --- a/src/core/completions/command-registry.ts +++ b/src/core/completions/command-registry.ts @@ -73,6 +73,12 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ takesValue: true, values: ['recent', 'name'], }, + { + name: 'status', + description: 'Only list changes in this lifecycle state', + takesValue: true, + values: ['proposed', 'shipped'], + }, COMMON_FLAGS.json, COMMON_FLAGS.store, ], @@ -191,6 +197,34 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ COMMON_FLAGS.store, ], }, + { + name: 'sync', + description: "Fold a change's spec deltas into the main specs without archiving it", + acceptsPositional: true, + positionalType: 'change-id', + positionals: [{ name: 'change-name', type: 'change-id', optional: true }], + flags: [ + { + name: 'check', + description: 'Report shipped changes whose deltas are not in the main specs; write nothing', + }, + { + name: 'ship', + description: 'Fold the named change, then mark it `status: shipped`', + }, + { + name: 'yes', + short: 'y', + description: 'Sync even when the change still has incomplete tasks', + }, + { + name: 'no-validate', + description: 'Skip validation (not recommended)', + }, + COMMON_FLAGS.json, + COMMON_FLAGS.store, + ], + }, { name: 'status', description: 'Display artifact completion status for a change', diff --git a/src/core/list.ts b/src/core/list.ts index f6b6faf2f8..9e60b4692d 100644 --- a/src/core/list.ts +++ b/src/core/list.ts @@ -5,18 +5,27 @@ import { readFileSync, type Dirent } from 'fs'; import { MarkdownParser } from './parsers/markdown-parser.js'; import type { RootOutput } from './root-selection.js'; import { discoverSpecFiles } from '../utils/spec-discovery.js'; +import { readChangeStatus, type ChangeStatus } from '../utils/change-metadata.js'; interface ChangeInfo { name: string; completedTasks: number; totalTasks: number; lastModified: Date; + /** + * Only set when the change's `.openspec.yaml` declares `status` itself. Left + * undefined otherwise so a project that never opts in sees no new column and + * no new JSON key. + */ + status?: ChangeStatus; } interface ListOptions { sort?: 'recent' | 'name'; json?: boolean; root?: RootOutput; + /** Filter to changes in this lifecycle state. Undeclared counts as `proposed`. */ + status?: ChangeStatus; } function isMissingPathError(error: unknown): boolean { @@ -96,7 +105,7 @@ function formatRelativeTime(date: Date): string { export class ListCommand { async execute(targetPath: string = '.', mode: 'changes' | 'specs' = 'changes', options: ListOptions = {}): Promise { - const { sort = 'recent', json = false, root } = options; + const { sort = 'recent', json = false, root, status: statusFilter } = options; if (mode === 'changes') { const changesDir = path.join(targetPath, 'openspec', 'changes'); @@ -120,17 +129,40 @@ export class ListCommand { const changes: ChangeInfo[] = []; for (const changeDir of changeDirs) { - const progress = await getTaskProgressForChange(changesDir, changeDir, targetPath); const changePath = path.join(changesDir, changeDir); + // Undeclared reads as `proposed`, which is what a change under + // `changes/` has always meant. + // + // A change whose metadata cannot be honored matches NEITHER filter. A + // filter is a claim of membership, and membership cannot be + // established here - listing it under both `--status proposed` and + // `--status shipped` states something false in one of the two. It stays + // visible in the unfiltered listing, and `openspec sync --check` is + // where the broken file gets named. + const marker = readChangeStatus(changePath); + if (statusFilter && (marker.invalidReason || marker.status !== statusFilter)) { + continue; + } + const progress = await getTaskProgressForChange(changesDir, changeDir, targetPath); const lastModified = await getLastModified(changePath); changes.push({ name: changeDir, completedTasks: progress.completed, totalTasks: progress.total, - lastModified + lastModified, + ...(marker.declared ? { status: marker.status } : {}) }); } + if (changes.length === 0) { + if (json) { + console.log(JSON.stringify({ changes: [], ...(root ? { root } : {}) }, null, 2)); + } else { + console.log(`No changes with status '${statusFilter}' found.`); + } + return; + } + // Sort by preference (default: recent first) if (sort === 'recent') { changes.sort((a, b) => b.lastModified.getTime() - a.lastModified.getTime()); @@ -145,7 +177,11 @@ export class ListCommand { completedTasks: c.completedTasks, totalTasks: c.totalTasks, lastModified: c.lastModified.toISOString(), - status: c.totalTasks === 0 ? 'no-tasks' : c.completedTasks === c.totalTasks ? 'complete' : 'in-progress' + // `status` here has always meant task progress. The lifecycle state is + // a different axis and gets its own key, emitted only when the change + // declares one, so existing consumers see byte-identical output. + status: c.totalTasks === 0 ? 'no-tasks' : c.completedTasks === c.totalTasks ? 'complete' : 'in-progress', + ...(c.status ? { lifecycle: c.status } : {}) })); console.log(JSON.stringify({ changes: jsonOutput, ...(root ? { root } : {}) }, null, 2)); return; @@ -155,11 +191,17 @@ export class ListCommand { console.log('Changes:'); const padding = ' '; const nameWidth = Math.max(...changes.map(c => c.name.length)); + const anyLifecycleDeclared = changes.some(c => c.status !== undefined); for (const change of changes) { const paddedName = change.name.padEnd(nameWidth); const status = formatTaskStatus({ total: change.totalTasks, completed: change.completedTasks }); const timeAgo = formatRelativeTime(change.lastModified); - console.log(`${padding}${paddedName} ${status.padEnd(12)} ${timeAgo}`); + // Only rendered when some change in this root declares a lifecycle + // state, so the default listing is unchanged for everyone else. + const lifecycle = anyLifecycleDeclared + ? ` ${(change.status ?? 'proposed').padEnd(8)}` + : ''; + console.log(`${padding}${paddedName}${lifecycle} ${status.padEnd(12)} ${timeAgo}`); } return; } diff --git a/src/core/sync.ts b/src/core/sync.ts new file mode 100644 index 0000000000..4e1f49fa07 --- /dev/null +++ b/src/core/sync.ts @@ -0,0 +1,976 @@ +/** + * Standalone spec sync. + * + * `archive` does two separable jobs in one command: it folds a change's deltas + * into `openspec/specs/`, and it declares the change finished by moving its + * directory. Welding the two means the fold can only happen at the moment the + * move happens - which, on a team that reviews before merging, is after the + * pull request closes. So a CI check for "the specs match what shipped" has + * nothing it can assert during review: every open PR has an unarchived change + * by definition, and a check phrased as "is everything archived?" is red as its + * resting state, from a change's first commit to its last (#1683). + * + * This module unwelds them, additively: + * + * - `openspec sync ` folds one change's deltas now, leaving the change + * where it is. `archive` still moves it later, and re-applying an already + * folded delta is a no-op the merge builder has supported all along (the + * "early-sync pattern" `specs-apply` names in ADDED, MODIFIED, REMOVED and + * RENAMED alike), so nothing about the archive step changes. + * - `openspec sync --check` asserts `shipped => folded` over the working tree. + * A change that has not declared itself shipped passes for free, so green is + * the resting state and red means a real mistake. The predicate is a pure + * function of files on disk - no VCS history, no timing - so a pre-commit + * hook, a pre-push hook and CI can all run the same command and get the same + * verdict. + * + * Two things this deliberately does NOT do, both for the same reason - they + * would turn an additive command into a second lifecycle: + * + * 1. **It never deletes a spec.** When a change's REMOVED entries take a + * capability's last requirement, `archive` retires the capability and + * deletes its main spec, gated on the author's `retire_capabilities` marker + * and wrapped in a displace-verify-delete dance that can roll back. Sync + * reports that case and points at `archive` instead of reimplementing the + * one irreversible operation in the system. + * 2. **It never checks archived changes.** Once a change is archived its deltas + * are history, and later changes supersede them; re-applying a five-month-old + * delta on top of everything that came after it is not a drift check, it is + * a merge conflict waiting to be written back over current text. The checked + * set is exactly the active changes that declare `status: shipped`, which is + * bounded and drains itself as those changes archive. + * + * Credit: this design is Matan Bendix Shenhav's, from his proposal #1683 and his + * implementation #1684, which he closed himself. No code from it is reused here. + * His, not ours: the diagnosis above; `shipped => folded` as a tree predicate + * evaluable at every tier (his decision V); the argument that a checker which + * reimplements the doer eventually disagrees with it (IV); the standalone + * idempotent `sync` (III); status as data rather than directory position (I and + * II); and setting the field and folding in one working-tree diff (VI, his + * `ship`). + * + * One deliberate divergence. His IV decides folded-ness by byte-identical + * regeneration; this module uses archive's zero-operations predicate instead, + * because the rebuild normalizes blank lines - a hand-formatted main spec would + * compare unequal while being perfectly in sync, and the gate would be red for a + * change nobody made. Same goal as IV, reached by sharing the doer's own + * predicate rather than comparing its output. + */ + +import { promises as fs } from 'fs'; +import path from 'path'; +import chalk from 'chalk'; +import { Validator } from './validation/validator.js'; +import { getTaskProgressForChange, formatTaskStatus } from '../utils/task-progress.js'; +import { + emitStoreRootBanner, + isRootSelectionError, + resolveOpenSpecRoot, + toRootOutput, + withStoreFlag, + isStoreSelectedRoot, + type ResolvedOpenSpecRoot, +} from './root-selection.js'; +import { + findSpecUpdates, + buildUpdatedSpec, + writeUpdatedSpec, + type SpecUpdate, +} from './specs-apply.js'; +import { + assertDistinctSpecTargets, + changeHasDeltaSpecsToValidate, + isRetirableSpec, + listActiveChangeNames, +} from './archive.js'; +import { + readChangeStatus, + writeChangeStatus, + METADATA_FILENAME, + type ChangeStatus, +} from '../utils/change-metadata.js'; +import { FileSystemUtils } from '../utils/file-system.js'; +import { folderStyleNameProblem } from './id.js'; + +// ----------------------------------------------------------------------------- +// Types +// ----------------------------------------------------------------------------- + +export interface SyncOptions { + /** Report what is unfolded and exit non-zero, without writing anything. */ + check?: boolean; + /** + * Fold the change, then set `status: shipped` on it - one working-tree diff. + * The stamp is last on purpose: a failed write or a non-convergent fold must + * not leave the field claiming shipped with the deltas absent. + */ + ship?: boolean; + /** Proceed past incomplete tasks without asking. */ + yes?: boolean; + /** Commander sets this to false for `--no-validate`. */ + validate?: boolean; + json?: boolean; + store?: string; + storePath?: string; +} + +export interface SyncSpecReport { + /** Capability id relative to the specs root, e.g. `billing/invoices`. */ + capability: string; + /** True when re-applying this delta would still change the main spec. */ + pending: boolean; + counts: { added: number; modified: number; removed: number; renamed: number }; +} + +export interface SyncChangeReport { + change: string; + status: ChangeStatus; + /** True when every delta this change carries is already in the main specs. */ + folded: boolean; + specs: SyncSpecReport[]; + warnings: string[]; + /** + * Reasons this change cannot be folded by `sync` at all: a delta the merge + * refuses, a retirement only `archive` can perform, or metadata whose + * `status` could not be determined. + */ + blockers: string[]; +} + +export interface SyncResult { + /** True for `--check`: nothing was written. */ + checked: boolean; + /** True when every examined change is folded and no change is blocked. */ + clean: boolean; + changes: SyncChangeReport[]; + totals: { added: number; modified: number; removed: number; renamed: number }; +} + +/** + * Carries the same `diagnostic` envelope RootSelectionError and StoreError do, + * so the CLI's shared failure plumbing prints the `Fix:` line and JSON callers + * get one status object - without this class joining their hierarchy. + */ +export class SyncBlockedError extends Error { + readonly diagnostic: { + severity: 'error'; + code: string; + message: string; + fix?: string; + }; + + constructor(code: string, message: string, fix?: string) { + super(message); + this.name = 'SyncBlockedError'; + this.diagnostic = { + severity: 'error', + code, + message, + ...(fix ? { fix } : {}), + }; + } +} + +// ----------------------------------------------------------------------------- +// Evaluation +// ----------------------------------------------------------------------------- + +interface PreparedSpec { + update: SpecUpdate; + rebuilt: string; + counts: SyncSpecReport['counts']; +} + +interface Evaluation { + report: SyncChangeReport; + /** Only the specs that still need writing. Empty when the change is folded. */ + writes: PreparedSpec[]; +} + +function sumCounts(counts: SyncSpecReport['counts']): number { + return counts.added + counts.modified + counts.removed + counts.renamed; +} + +/** + * Decide whether a change's deltas are already in the main specs, by running the + * merge builder and looking at how much it had to do. + * + * "Folded" is `buildUpdatedSpec` applying zero operations - the *same* predicate + * `archive` uses to decide it has nothing to write ("Every operation was already + * synced: rewriting the file would only churn normalization differences into + * it"). Deliberately not a byte-comparison against the rebuilt output: the + * rebuild normalizes blank lines, so a main spec that a human formatted by hand + * would compare unequal while being perfectly in sync, and the gate would be red + * for a change nobody made. Sharing archive's own predicate is also what keeps + * checker and doer from drifting apart, which is the failure #1112 describes. + */ +async function evaluateChange( + changeName: string, + changeDir: string, + mainSpecsDir: string, + validate = true +): Promise { + const status = readChangeStatus(changeDir); + const report: SyncChangeReport = { + change: changeName, + status: status.status, + folded: true, + specs: [], + warnings: [], + blockers: [], + }; + + if (status.invalidReason) { + // Undetermined is not "proposed". A change whose metadata broke would + // otherwise pass a gate whose whole job is noticing that kind of rot. + report.folded = false; + report.blockers.push( + `Could not read the change's lifecycle status from ${METADATA_FILENAME}: ${status.invalidReason}` + ); + return { report, writes: [] }; + } + + // Run BEFORE the fold, and on the `--check` path too. + // + // The gate promises `shipped => folded`, and a delta the merge would refuse is + // not folded and never will be. Leaving this to the write path made `--check` + // certify as clean a change whose only delta sat at `specs/spec.md`, which + // `discoverSpecFiles` does not walk (#1385): zero updates found, nothing + // pending, green - while `openspec sync` and `openspec archive` both refused + // the same tree. A gate that is green on a silently dropped requirement is + // worse than no gate. + // + // Whether a change HAS deltas to validate is archive's own question, asked + // through its own function, so a zero-delta change is treated identically by + // both commands. + if (validate) { + let hasDeltas: boolean; + try { + hasDeltas = await changeHasDeltaSpecsToValidate(changeDir); + } catch (error) { + report.folded = false; + report.blockers.push( + `Could not read this change's delta specs: ${ + error instanceof Error ? error.message : String(error) + }` + ); + return { report, writes: [] }; + } + if (hasDeltas) { + // No mainSpecsDir, matching archive: the scenario-loss check standalone + // validate runs (#1477) is the same one buildUpdatedSpec enforces below, + // and reporting it here would relabel that failure. + const deltaReport = await new Validator().validateChangeDeltaSpecs(changeDir); + if (!deltaReport.valid) { + report.folded = false; + for (const issue of deltaReport.issues) { + if (issue.level === 'ERROR') report.blockers.push(issue.message); + } + // A report that is invalid with no ERROR issue would otherwise pass + // silently while claiming to have blocked. + if (report.blockers.length === 0) { + report.blockers.push(`Delta specs for '${changeName}' failed validation.`); + } + return { report, writes: [] }; + } + } + } + + let updates: SpecUpdate[]; + try { + updates = await findSpecUpdates(changeDir, mainSpecsDir); + } catch (error) { + report.folded = false; + report.blockers.push( + `Could not read this change's delta specs: ${ + error instanceof Error ? error.message : String(error) + }` + ); + return { report, writes: [] }; + } + + const writes: PreparedSpec[] = []; + for (const update of updates) { + let built: Awaited>; + try { + built = await buildUpdatedSpec(update, changeName, { silent: true }); + } catch (error) { + report.folded = false; + report.blockers.push( + `${update.id}: ${error instanceof Error ? error.message : String(error)}` + ); + continue; + } + + report.warnings.push(...built.warnings); + + // The one case sync hands back to archive. When a change's REMOVED entries + // take a capability's last requirement, the spec that would be written has + // no requirements and cannot be validated - archive's answer is to delete + // the file, which needs the author's `retire_capabilities` marker and a + // rollback-safe deletion. Reimplementing that here would put the system's + // only irreversible operation behind a second, less careful door. + if ( + update.exists && + built.counts.removed > 0 && + built.noRequirementBlocks && + (await isRetirableSpec(update.id, built.rebuilt)) + ) { + report.folded = false; + report.blockers.push( + `${update.id}: this change removes the capability's last requirement. ` + + `Retiring a capability deletes its spec, which only openspec archive does. ` + + `Archive the change instead of syncing it.` + ); + continue; + } + + const pending = sumCounts(built.counts) > 0; + report.specs.push({ capability: update.id, pending, counts: built.counts }); + if (pending) { + report.folded = false; + writes.push({ update, rebuilt: built.rebuilt, counts: built.counts }); + } + } + + if (report.blockers.length > 0) report.folded = false; + return { report, writes }; +} + +// ----------------------------------------------------------------------------- +// Command +// ----------------------------------------------------------------------------- + +export class SyncCommand { + async execute(changeName?: string, options: SyncOptions = {}): Promise { + const json = !!options.json; + + let root: ResolvedOpenSpecRoot; + try { + root = await resolveOpenSpecRoot({ + ...(options.store !== undefined ? { store: options.store } : {}), + ...(options.storePath !== undefined ? { storePath: options.storePath } : {}), + }); + } catch (error) { + if (json && isRootSelectionError(error)) { + this.printJsonFailure(undefined, { + code: error.diagnostic.code, + message: error.diagnostic.message, + ...(error.diagnostic.fix ? { fix: error.diagnostic.fix } : {}), + }); + return; + } + throw error; + } + + if (json) { + try { + const result = await this.run(changeName, options, root, true); + if (!result) return; + console.log( + JSON.stringify({ sync: result, root: toRootOutput(root) }, null, 2) + ); + if (!result.clean) process.exitCode = 1; + } catch (error) { + this.printJsonFailure(root, toDiagnostic(error)); + } + return; + } + + emitStoreRootBanner(root); + await this.run(changeName, options, root, false); + } + + private printJsonFailure( + root: ResolvedOpenSpecRoot | undefined, + diagnostic: { code: string; message: string; fix?: string } + ): void { + console.log( + JSON.stringify( + { + sync: null, + ...(root ? { root: toRootOutput(root) } : {}), + status: [{ severity: 'error', ...diagnostic }], + }, + null, + 2 + ) + ); + process.exitCode = 1; + } + + private async run( + changeName: string | undefined, + options: SyncOptions, + root: ResolvedOpenSpecRoot, + json: boolean + ): Promise { + const changesDir = root.changesDir; + const mainSpecsDir = root.specsDir; + + for (const [allowedDirectory, managedDir] of [ + [root.path, changesDir], + [root.path, mainSpecsDir], + ] as const) { + try { + FileSystemUtils.assertPathWithin(allowedDirectory, managedDir); + } catch { + throw new SyncBlockedError( + 'sync_path_outside_root', + `Refusing to sync through a path outside the OpenSpec root: ${managedDir}` + ); + } + } + + const check = !!options.check; + if (options.ship && check) { + throw new SyncBlockedError( + 'sync_ship_with_check', + '--ship writes to the change and --check writes nothing; pass one or the other.' + ); + } + if (options.ship && !changeName) { + throw new SyncBlockedError( + 'sync_ship_needs_change', + '--ship needs the change to mark shipped.', + withStoreFlag(root, 'openspec sync --ship') + ); + } + + // Archive refuses to skip validation without an explicit answer, because + // skipping it can write a spec that would never have validated. Sync is + // unattended by design, so there is no prompt to give - `--yes` is the + // answer, exactly as archive's own JSON path requires. + if (options.validate === false && !options.yes && !options.check) { + throw new SyncBlockedError( + 'sync_confirmation_required', + 'Skipping validation can fold a spec that would never have validated, so it needs confirmation.', + withStoreFlag(root, `openspec sync ${changeName ?? ''} --no-validate --yes`) + ); + } + + const targets = changeName + ? [await this.resolveNamedChange(changeName, changesDir, root)] + : await this.shippedChanges(changesDir); + + // Checked before the fold, not after it. `writeChangeStatus` refuses a + // change with no `.openspec.yaml`, and discovering that only once the specs + // are written leaves a fold that is never stamped - and a rerun that fails + // in exactly the same place, so the ordering's usual self-correction does + // not apply. + if (options.ship) { + const metaPath = path.join(changesDir, targets[0], METADATA_FILENAME); + try { + await fs.access(metaPath); + } catch { + throw new SyncBlockedError( + 'sync_ship_no_metadata', + `Change '${targets[0]}' has no ${METADATA_FILENAME}, so there is no file to record ` + + `\`status: shipped\` in. No specs were folded.`, + `Create the change with openspec new change, or add ${METADATA_FILENAME} by hand, then rerun.` + ); + } + } + + if (targets.length === 0) { + const result: SyncResult = { + checked: check, + clean: true, + changes: [], + totals: { added: 0, modified: 0, removed: 0, renamed: 0 }, + }; + if (!json) { + console.log( + check + ? 'No change declares `status: shipped`; nothing to check.' + : 'No change declares `status: shipped`; nothing to sync. ' + + 'Name a change to sync it directly, or add `status: shipped` to its ' + + `${METADATA_FILENAME}.` + ); + } + return result; + } + + const evaluations: Evaluation[] = []; + for (const name of targets) { + evaluations.push( + await evaluateChange( + name, + path.join(changesDir, name), + mainSpecsDir, + options.validate !== false + ) + ); + } + + return check + ? this.reportCheck(evaluations, root, json) + : this.applyFolds(evaluations, changesDir, mainSpecsDir, root, options, json); + } + + /** A named change has to exist, exactly as archive requires. */ + private async resolveNamedChange( + changeName: string, + changesDir: string, + root: ResolvedOpenSpecRoot + ): Promise { + const problem = folderStyleNameProblem(changeName, 'Change name'); + if (problem) throw new SyncBlockedError('sync_change_name_invalid', problem); + + const changeDir = path.join(changesDir, changeName); + try { + const stat = await fs.lstat(changeDir); + if (stat.isSymbolicLink()) { + throw new SyncBlockedError( + 'sync_change_symlink', + `Change '${changeName}' is a symbolic link. Replace it with a real directory before syncing.` + ); + } + if (!stat.isDirectory()) throw new Error('not a directory'); + } catch (error) { + if (error instanceof SyncBlockedError) throw error; + const available = await listActiveChangeNames(changesDir); + throw new SyncBlockedError( + 'sync_change_not_found', + available.length > 0 + ? `Change '${changeName}' not found. Available changes: ${available.join(', ')}` + : `Change '${changeName}' not found. No active changes exist in this root.`, + withStoreFlag(root, 'openspec list') + ); + } + return changeName; + } + + /** + * Every active change that declares `status: shipped`, plus every change + * whose status could not be read at all. + * + * The second half is what keeps the gate honest. Skipping an unreadable + * `.openspec.yaml` here would be the fail-open direction: a change that + * declared itself shipped and then had its metadata broken would silently + * stop being checked. `evaluateChange` turns it into a named blocker. + */ + private async shippedChanges(changesDir: string): Promise { + const names = await listActiveChangeNames(changesDir); + return names.filter((name) => { + const status = readChangeStatus(path.join(changesDir, name)); + return status.invalidReason !== undefined || status.status === 'shipped'; + }); + } + + private reportCheck( + evaluations: Evaluation[], + root: ResolvedOpenSpecRoot, + json: boolean + ): SyncResult { + const changes = evaluations.map((evaluation) => evaluation.report); + const clean = changes.every((change) => change.folded); + const result: SyncResult = { + checked: true, + clean, + changes, + totals: { added: 0, modified: 0, removed: 0, renamed: 0 }, + }; + + if (json) { + if (!clean) process.exitCode = 1; + return result; + } + + if (clean) { + console.log( + `✓ ${changes.length} shipped change(s) are folded into the main specs.` + ); + return result; + } + + // Titled for both shapes of failure it reports: a shipped change whose + // deltas are not in the main specs, and a change whose lifecycle status + // could not be determined at all. + console.log(chalk.red('Sync check failed:\n')); + for (const change of changes) { + if (change.folded) continue; + console.log(` ${change.change}`); + for (const spec of change.specs) { + if (!spec.pending) continue; + const { added, modified, removed, renamed } = spec.counts; + const parts = [ + added ? `+${added}` : '', + modified ? `~${modified}` : '', + removed ? `-${removed}` : '', + renamed ? `→${renamed}` : '', + ].filter(Boolean); + console.log(` ${spec.capability}: ${parts.join(' ')} not applied`); + } + for (const blocker of change.blockers) { + console.log(chalk.yellow(` ${blocker}`)); + } + } + const fixable = changes.filter( + (change) => !change.folded && change.blockers.length === 0 + ); + if (fixable.length > 0) { + console.log( + `\nRun ${withStoreFlag(root, 'openspec sync')} to fold them, then commit the result.` + ); + } + process.exitCode = 1; + return result; + } + + private async applyFolds( + evaluations: Evaluation[], + changesDir: string, + mainSpecsDir: string, + root: ResolvedOpenSpecRoot, + options: SyncOptions, + json: boolean + ): Promise { + const skipValidation = options.validate === false; + + const blocked = evaluations.filter( + (evaluation) => evaluation.report.blockers.length > 0 + ); + if (blocked.length > 0) { + const first = blocked[0]; + throw new SyncBlockedError( + 'sync_change_blocked', + `Cannot sync '${first.report.change}': ${first.report.blockers[0]}`, + blocked.length > 1 + ? `${blocked.length} changes are blocked; run openspec sync --check for the full list.` + : undefined + ); + } + + // Delta validation already ran inside `evaluateChange`, on the check path + // too, so a blocked change never reaches here. Task completion is the one + // guard that is about the change rather than about its deltas, and it has + // no bearing on whether the tree satisfies the gate - so it gates the write + // and deliberately does not make `--check` red. + for (const { report } of evaluations) { + await this.assertTasksComplete(report.change, changesDir, options, root, json); + } + + // Fold ONE CHANGE AT A TIME, rebuilding each against the specs as they are + // on disk at that moment. + // + // Evaluating every change up front and then writing them all would rebuild + // each one from the same pre-write baseline, so two shipped changes adding + // different requirements to the same capability would each produce a spec + // containing only their own - and the second write would erase the first, + // silently, while the console reported both as applied. That is not a + // conflict between the changes; they compose fine. It is the batch reading + // a stale baseline. `archive` never had the bug because it takes one change + // per invocation, and folding sequentially is how sync inherits that. + // + // Every target written across the whole run is captured first, so a failure + // on the third change still puts the first two back rather than handing + // back a tree nobody asked for. + const totals = { added: 0, modified: 0, removed: 0, renamed: 0 }; + const snapshots: TargetSnapshot[] = []; + // What this run last wrote to each target, so the rollback can tell its own + // output apart from a concurrent edit it must not clobber. + const wrote = new Map(); + let wroteAny = false; + + try { + for (const evaluation of evaluations) { + const changeName = evaluation.report.change; + // Re-evaluated against the current tree rather than reusing the plan + // built before the previous change was folded. + const current = await evaluateChange( + changeName, + path.join(changesDir, changeName), + mainSpecsDir, + !skipValidation + ); + if (current.report.blockers.length > 0) { + throw new SyncBlockedError( + 'sync_change_blocked', + `Cannot sync '${changeName}': ${current.report.blockers[0]}` + ); + } + evaluation.report.specs = current.report.specs; + evaluation.report.warnings = current.report.warnings; + if (current.writes.length === 0) continue; + + // Two capability ids can resolve to the SAME file - a symlinked + // capability directory is explicitly allowed by the trust model, and a + // case-variant id aliases on a case-insensitive filesystem. Writing + // both in sequence is last-writer-wins, which loses one fold and files + // the other's requirements under the wrong name. Archive refuses this + // outright; sync uses archive's own check so the two agree on which + // trees they will write. + await assertDistinctSpecTargets( + current.writes.map(({ update }) => ({ id: update.id, target: update.target })), + 'syncing' + ); + + // Validated before any of THIS change's specs is written, so a late + // failure inside one change leaves that change wholly unapplied. + if (!skipValidation) { + const validator = new Validator(); + for (const write of current.writes) { + const specReport = await validator.validateSpecContent( + write.update.id, + write.rebuilt + ); + if (specReport.valid) continue; + const details = specReport.issues + .filter((issue) => issue.level === 'ERROR') + .map((issue) => issue.message) + .join('; '); + throw new SyncBlockedError( + 'sync_spec_validation_failed', + `The spec '${write.update.id}' would be rebuilt into an invalid state by ` + + `change '${changeName}': ${details}.`, + `Run ${withStoreFlag(root, `openspec validate ${write.update.id}`)} after fixing the change deltas.` + ); + } + } + + if (!json) { + for (const warning of current.report.warnings) { + console.log(chalk.yellow(`⚠️ Warning: ${warning}`)); + } + } + + for (const write of current.writes) { + if (!wrote.has(write.update.target)) { + snapshots.push(await captureTarget(write.update.target)); + } + await writeUpdatedSpec(write.update, write.rebuilt, write.counts, { + silent: json, + ...(isStoreSelectedRoot(root) ? { displayPath: write.update.target } : {}), + }); + wrote.set(write.update.target, write.rebuilt); + wroteAny = true; + totals.added += write.counts.added; + totals.modified += write.counts.modified; + totals.removed += write.counts.removed; + totals.renamed += write.counts.renamed; + } + } + } catch (error) { + const restoreFailure = await restoreTargets(snapshots, wrote); + if (error instanceof SyncBlockedError) { + throw new SyncBlockedError( + error.diagnostic.code, + `${error.message}${ + restoreFailure ? ` ${restoreFailure}` : ' No spec was left partly folded.' + }`, + restoreFailure ? 'Restore the named files from git, then rerun.' : error.diagnostic.fix + ); + } + throw new SyncBlockedError( + 'sync_write_failed', + `Could not write the main specs: ${ + error instanceof Error ? error.message : String(error) + }.${restoreFailure ? ` ${restoreFailure}` : ' No spec was left partly folded.'}`, + restoreFailure ? 'Restore the named files from git, then rerun.' : undefined + ); + } + + // Re-evaluate rather than assume. Sequential folding removes the stale + // baseline, but two shipped changes can still genuinely disagree - one + // adding a requirement the other removes - and such a pair never settles. + // The merge builder catches the destructive shapes on its own (a MODIFIED + // that would drop a scenario, an ADDED whose content differs), so what + // reaches here is the non-convergent rest, and naming it beats looping. + const unsettled: string[] = []; + for (const { report } of evaluations) { + const after = await evaluateChange( + report.change, + path.join(changesDir, report.change), + mainSpecsDir, + !skipValidation + ); + if (!after.report.folded) unsettled.push(report.change); + } + if (unsettled.length > 0) { + const restoreFailure = await restoreTargets(snapshots, wrote); + throw new SyncBlockedError( + 'sync_did_not_converge', + `These changes still report unfolded deltas after a fold: ` + + `${unsettled.join(', ')}. Two shipped changes are claiming the same ` + + `requirement in ways that cannot both hold.${ + restoreFailure ? ` ${restoreFailure}` : ' The main specs were left unchanged.' + }`, + 'Reconcile the conflicting deltas, then rerun.' + ); + } + + // Stamped last, once the specs on disk are known to be correct. Writing the + // field any earlier means every later failure - a write that cannot + // complete, a fold that does not settle - has to remember to take the + // metadata back with it, and the one that forgets leaves a change claiming + // `shipped` with its deltas absent, which is the state this flag exists to + // prevent. Ordering removes the failure rather than compensating for it. + // + // The reverse order is harmless and self-correcting: a fold that lands + // without the stamp is a proposed change whose deltas happen to already be + // in the specs, which the gate ignores, and rerunning `--ship` folds + // nothing and stamps the field. + if (options.ship) { + const shipped = evaluations[0].report; + writeChangeStatus(path.join(changesDir, shipped.change), 'shipped'); + shipped.status = 'shipped'; + if (!json) console.log(`Marked '${shipped.change}' as shipped.`); + } + + const changes = evaluations.map((evaluation) => ({ + ...evaluation.report, + folded: true, + })); + if (!json) { + if (wroteAny) { + console.log( + `Totals: + ${totals.added}, ~ ${totals.modified}, - ${totals.removed}, → ${totals.renamed}` + ); + console.log('Specs updated successfully.'); + } else { + console.log('Specs already in sync; no files changed.'); + } + } + + return { + checked: false, + clean: true, + changes, + totals, + }; + } + + /** + * Folding a change whose tasks are unfinished writes requirements into + * `specs/` that nothing implements yet - the exact drift the gate exists to + * prevent, arriving through the gate's own command. Blocks rather than warns, + * because sync is designed to run unattended in a hook. + */ + private async assertTasksComplete( + changeName: string, + changesDir: string, + options: SyncOptions, + root: ResolvedOpenSpecRoot, + json: boolean + ): Promise { + const progress = await getTaskProgressForChange( + changesDir, + changeName, + path.resolve(changesDir, '..', '..') + ); + const incomplete = Math.max(progress.total - progress.completed, 0); + if (incomplete === 0) return; + + if (options.yes) { + if (!json) { + console.log( + `Warning: ${incomplete} incomplete task(s) in '${changeName}'. Continuing due to --yes flag.` + ); + } + return; + } + + if (!json) console.log(`Task status: ${formatTaskStatus(progress)}`); + throw new SyncBlockedError( + 'sync_tasks_incomplete', + `${incomplete} incomplete task(s) in '${changeName}'. Syncing now would write ` + + `requirements into the main specs that nothing implements yet.`, + `Complete the tasks, or rerun with ${withStoreFlag(root, `openspec sync ${changeName} --yes`)}.` + ); + } +} + +interface TargetSnapshot { + target: string; + /** The bytes that were there, or undefined when the file did not exist. */ + content?: Buffer; +} + +/** Read the current bytes of a target so a failed write can be undone. */ +async function captureTarget(target: string): Promise { + try { + return { target, content: await fs.readFile(target) }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return { target }; + throw error; + } +} + +/** + * Put every target that this run actually changed back the way it was, in + * reverse order. + * + * Two things it will not do, both mirroring `archive`'s rollback: + * + * - **It does not touch a target whose bytes already match the snapshot.** The + * write that failed is usually the one that never landed, and "restoring" an + * unchanged file only to fail on a read-only one produced a false "may hold + * partly folded content" alarm about a file nothing had written. + * - **It does not overwrite content this run did not produce.** A target whose + * bytes match neither the snapshot nor what was written was changed by + * something else while the fold was running; clobbering it would destroy an + * edit to save a rollback. It is reported instead. + * + * Returns a sentence naming what could not be put back, or undefined when the + * tree is back to its original state. Never throws: it runs inside a failure + * path, and losing the original error to a rollback error would hide the cause. + */ +async function restoreTargets( + snapshots: TargetSnapshot[], + wrote: Map +): Promise { + const failed: string[] = []; + const foreign: string[] = []; + for (const snapshot of [...snapshots].reverse()) { + try { + const current = await fs.readFile(snapshot.target).catch((error) => { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + throw error; + }); + + if (snapshot.content === undefined) { + // The file did not exist before this run. + if (current === undefined) continue; + if (current.toString() !== wrote.get(snapshot.target)) { + foreign.push(snapshot.target); + continue; + } + await fs.unlink(snapshot.target); + continue; + } + + if (current !== undefined && current.equals(snapshot.content)) continue; + if (current !== undefined && current.toString() !== wrote.get(snapshot.target)) { + foreign.push(snapshot.target); + continue; + } + // Written in place, exactly as writeUpdatedSpec does, so a symlinked or + // hard-linked spec keeps the semantics it had before the fold. + await fs.writeFile(snapshot.target, snapshot.content); + } catch { + failed.push(snapshot.target); + } + } + + const problems = [ + failed.length > 0 + ? `These specs could not be restored and may hold partly folded content: ${failed.join(', ')}.` + : '', + foreign.length > 0 + ? `These specs changed underneath this run and were left as they are: ${foreign.join(', ')}.` + : '', + ].filter(Boolean); + return problems.length > 0 ? problems.join(' ') : undefined; +} + +function toDiagnostic(error: unknown): { code: string; message: string; fix?: string } { + if (error instanceof SyncBlockedError) { + const { severity: _severity, ...rest } = error.diagnostic; + return rest; + } + return { + code: 'sync_error', + message: error instanceof Error ? error.message : String(error), + }; +} diff --git a/src/core/templates/workflows/store-selection.ts b/src/core/templates/workflows/store-selection.ts index dfa132c3e5..771269e6ef 100644 --- a/src/core/templates/workflows/store-selection.ts +++ b/src/core/templates/workflows/store-selection.ts @@ -4,4 +4,4 @@ * Interpolated into every workflow's instructions so generated skills * consistently teach how to target a registered store with `--store `. */ -export const STORE_SELECTION_GUIDANCE = `**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run \`openspec store list --json\` to discover registered store ids, then pass \`--store \` on the commands that read or write specs and changes (\`new change\`, \`status\`, \`instructions\`, \`list\`, \`show\`, \`validate\`, \`archive\`, \`doctor\`, \`context\`, \`schemas\`, \`view\`). Once selected, treat \`--store \` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run \`openspec status --change "" --json --store ""\`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local \`openspec/\` root.`; +export const STORE_SELECTION_GUIDANCE = `**Store selection:** If the user names a store (a store is a standalone OpenSpec repo registered on this machine) or the work lives in one, run \`openspec store list --json\` to discover registered store ids, then pass \`--store \` on the commands that read or write specs and changes (\`new change\`, \`status\`, \`instructions\`, \`list\`, \`show\`, \`validate\`, \`sync\`, \`archive\`, \`doctor\`, \`context\`, \`schemas\`, \`view\`). Once selected, treat \`--store \` as sticky for the rest of the workflow. Every unscoped example of those commands below is shorthand: before running it, append the flag. For example, run \`openspec status --change "" --json --store ""\`, not the unscoped form shown below. Other commands do not take the flag. Hints printed by commands already carry the flag; keep it on follow-ups. Without a store, commands act on the nearest local \`openspec/\` root.`; diff --git a/src/utils/change-metadata.ts b/src/utils/change-metadata.ts index 1f31a44596..42f236fd15 100644 --- a/src/utils/change-metadata.ts +++ b/src/utils/change-metadata.ts @@ -343,3 +343,190 @@ function readBooleanMarker( } return { declared: false }; } + +/** + * Where a change sits in its own lifecycle. + * + * `proposed` is the resting state and needs no declaration: it is what every + * change under `changes/` has always meant, so a project that never opts in + * reads as proposed everywhere. + */ +export type ChangeStatus = 'proposed' | 'shipped'; + +export interface ChangeStatusMarker { + /** The change's state. `proposed` unless the metadata explicitly says otherwise. */ + status: ChangeStatus; + /** True only when `.openspec.yaml` sets `status` itself. */ + declared: boolean; + /** + * Set when the state could not be determined: the metadata file exists but + * cannot be read, does not parse, or fails the contract the rest of the CLI + * enforces. Callers must never round this to `proposed` - that is the + * direction that lets a shipped change slip past `openspec sync --check`. + */ + invalidReason?: string; +} + +/** + * Non-throwing read of the `status` field, with the same metadata contract the + * boolean markers above enforce: the file has to parse under + * ChangeMetadataSchema and name a schema that both passes `listSchemas` + * membership and actually resolves. + * + * The one difference is what an unreadable file means. A boolean marker that + * cannot be honored falls back to "not declared", which is the safe direction + * for `skip_specs` and `retire_capabilities` - both authorize an action, so + * withholding them does less. `status` gates a *check*, so the safe direction + * is the opposite: undetermined must stay undetermined and be reported, or a + * change whose metadata broke would quietly pass the gate that exists to + * notice exactly that kind of rot. + */ +export function readChangeStatus(changeDir: string): ChangeStatusMarker { + let raw: string; + try { + raw = fs.readFileSync(path.join(changeDir, METADATA_FILENAME), 'utf-8'); + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code === 'ENOENT') { + // No metadata at all is the ordinary case for changes authored before + // the file existed, and for every change that never opts in. + return { status: 'proposed', declared: false }; + } + const message = err instanceof Error ? err.message : String(err); + return undetermined(`the metadata file cannot be read (${message})`); + } + + let parsed: unknown; + try { + parsed = yaml.parse(raw); + } catch { + // Anchored so a comment like "# set status: shipped once merged" does not + // turn an unrelated YAML problem into an undetermined state. + const mentioned = /^\s*(['"]?)status\1\s*:/m.test(raw); + return mentioned + ? undetermined('the file is not valid YAML') + : { status: 'proposed', declared: false }; + } + + const result = ChangeMetadataSchema.safeParse(parsed); + if (result.success) { + if (result.data.status === undefined) { + return { status: 'proposed', declared: false }; + } + // Checked only when the field is declared, exactly as the boolean markers + // do: a broken schema on an ordinary change is `openspec status`'s problem + // to report, not this reader's. + try { + const projectRoot = path.resolve(changeDir, '../../..'); + if (!listSchemas(projectRoot).includes(result.data.schema)) { + return undetermined(`schema: unknown schema '${result.data.schema}'`); + } + resolveSchema(result.data.schema, projectRoot); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return undetermined(message); + } + return { status: result.data.status, declared: true }; + } + + // Key presence, not value: `status: shiped` must surface as undetermined + // rather than silently reading as proposed. Metadata that is broken for some + // unrelated reason, on a change that never mentions `status`, is simply not + // declared - the same restraint the boolean markers show. + const mentioned = + typeof parsed === 'object' && parsed !== null && 'status' in parsed; + if (mentioned) { + const first = result.error.issues[0]; + const where = first.path.length > 0 ? `${first.path.join('.')}: ` : ''; + return undetermined(`${where}${first.message}`); + } + return { status: 'proposed', declared: false }; +} + +/** + * A state that could not be determined, with its reason made safe to print. + * Same treatment as `unhonorable` above: every reason quotes something the + * author wrote, and callers print it straight to a terminal. + */ +function undetermined(reason: string): ChangeStatusMarker { + return { + status: 'proposed', + declared: false, + invalidReason: reason.replace(/[\u0000-\u001f\u007f]/g, '?'), + }; +} + +/** + * Set `status` in a change's `.openspec.yaml`, preserving every other field and + * the file's own formatting. + * + * Edits the parsed document rather than rewriting it from the validated object, + * so comments and key order survive - `writeChangeMetadata` would flatten both, + * and this file is hand-authored. + */ +export function writeChangeStatus(changeDir: string, status: ChangeStatus): void { + const metaPath = path.join(changeDir, METADATA_FILENAME); + let raw: string; + try { + raw = fs.readFileSync(metaPath, 'utf-8'); + } catch (err) { + const ioError = err instanceof Error ? err : new Error(String(err)); + throw new ChangeMetadataError( + (err as NodeJS.ErrnoException)?.code === 'ENOENT' + ? `No ${METADATA_FILENAME} in this change, so there is nothing to set status on. ` + + `Create the change with openspec new change, or add the file by hand.` + : `Failed to read metadata: ${ioError.message}`, + metaPath, + ioError + ); + } + + let doc: ReturnType; + try { + doc = yaml.parseDocument(raw); + if (doc.errors.length > 0) throw new Error(doc.errors[0].message); + } catch (err) { + const parseError = err instanceof Error ? err : new Error(String(err)); + throw new ChangeMetadataError( + `Invalid YAML in metadata file: ${parseError.message}`, + metaPath, + parseError + ); + } + + doc.set('status', status); + + // The edited document still has to satisfy the contract every reader + // enforces, or this would be a way to write metadata the CLI then rejects. + const check = ChangeMetadataSchema.safeParse(doc.toJS()); + if (!check.success) { + throw new ChangeMetadataError( + `Invalid metadata: ${check.error.message}`, + metaPath + ); + } + + // Written through a sibling temp file and renamed into place. A direct + // write that fails partway (ENOSPC, a full disk, a killed process) truncates + // the file, and this one carries the change's `schema:` - losing it breaks + // every command that reads the change, not just the field being set. The + // rename is atomic on the same filesystem, so the file is either the old + // content or the new one. + const tempPath = `${metaPath}.openspec-status-${process.pid}-${Date.now()}`; + try { + fs.writeFileSync(tempPath, doc.toString(), 'utf-8'); + fs.renameSync(tempPath, metaPath); + } catch (err) { + try { + fs.unlinkSync(tempPath); + } catch { + // Nothing to clean up, or it cannot be removed; the original file is + // intact either way, which is the property that matters here. + } + const ioError = err instanceof Error ? err : new Error(String(err)); + throw new ChangeMetadataError( + `Failed to write metadata: ${ioError.message}`, + metaPath, + ioError + ); + } +} diff --git a/test/commands/sync-cli.test.ts b/test/commands/sync-cli.test.ts new file mode 100644 index 0000000000..b4d76d8f91 --- /dev/null +++ b/test/commands/sync-cli.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { runCLI } from '../helpers/run-cli.js'; +import { cleanupTempPath } from '../helpers/temp-cleanup.js'; + +/** + * Flag handling that lives in the CLI layer rather than in the command class, + * so it can only be exercised through the real argument parser. + */ +describe('openspec sync / list --status (CLI surface)', () => { + let tempDir: string; + let env: NodeJS.ProcessEnv; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-sync-cli-')); + await fs.mkdir(path.join(tempDir, 'openspec', 'changes'), { recursive: true }); + await fs.mkdir(path.join(tempDir, 'openspec', 'specs'), { recursive: true }); + await fs.writeFile(path.join(tempDir, 'openspec', 'project.md'), '# Demo\n'); + // Only the override: runCLI merges process.env itself, and forwarding a + // host XDG_CONFIG_HOME would count as an explicit one, sending the CLI to + // the developer's real config directory instead of runCLI's isolated one. + env = { XDG_DATA_HOME: path.join(tempDir, 'xdg-data') }; + }); + + afterEach(async () => { + await cleanupTempPath(tempDir); + }); + + it('rejects --status combined with --specs', async () => { + const result = await runCLI(['list', '--specs', '--status', 'shipped'], { + cwd: tempDir, + env, + }); + + // Silently ignoring it would print the full spec list as though the filter + // had matched everything. + expect(result.exitCode).toBe(1); + expect(`${result.stdout}${result.stderr}`).toContain('cannot be combined with --specs'); + }); + + it('rejects an unknown --status value', async () => { + const result = await runCLI(['list', '--status', 'bogus'], { + cwd: tempDir, + env, + }); + + expect(result.exitCode).toBe(1); + expect(`${result.stdout}${result.stderr}`).toContain("Unknown --status 'bogus'"); + }); + + it('exits 0 from sync --check when nothing declares a status', async () => { + const result = await runCLI(['sync', '--check'], { cwd: tempDir, env }); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('nothing to check'); + }); + + it('registers sync in --help', async () => { + const result = await runCLI(['--help'], { cwd: tempDir, env }); + + expect(result.stdout).toContain('sync'); + }); +}); diff --git a/test/core/completions/command-registry.test.ts b/test/core/completions/command-registry.test.ts index c3ada9fb79..6e48e66b5e 100644 --- a/test/core/completions/command-registry.test.ts +++ b/test/core/completions/command-registry.test.ts @@ -174,6 +174,7 @@ describe('command completion registry', () => { 'schemas', 'show', 'status', + 'sync', 'validate', 'view', ]); diff --git a/test/core/list.test.ts b/test/core/list.test.ts index 5b23a5d712..a994a73f49 100644 --- a/test/core/list.test.ts +++ b/test/core/list.test.ts @@ -187,4 +187,106 @@ Regular text that should be ignored expect(logOutput.some(line => line.includes('no-tasks') && line.includes('No tasks'))).toBe(true); }); }); + + describe('lifecycle status', () => { + async function change(name: string, metadata?: string): Promise { + const dir = path.join(tempDir, 'openspec', 'changes', name); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(path.join(dir, 'tasks.md'), '- [x] 1.1 Done\n'); + if (metadata !== undefined) { + await fs.writeFile(path.join(dir, '.openspec.yaml'), metadata); + } + } + + it('renders no lifecycle column when no change declares one', async () => { + await change('a'); + await change('b', 'schema: spec-driven\n'); + + await new ListCommand().execute(tempDir, 'changes'); + + // A project that never opts in must see byte-identical output. + expect(logOutput.join('\n')).not.toContain('proposed'); + expect(logOutput.join('\n')).not.toContain('shipped'); + }); + + it('renders the column once any change declares one', async () => { + await change('a'); + await change('b', 'schema: spec-driven\nstatus: shipped\n'); + + await new ListCommand().execute(tempDir, 'changes'); + + const text = logOutput.join('\n'); + expect(text).toContain('shipped'); + // An undeclared change reads as proposed rather than blank. + expect(text).toContain('proposed'); + }); + + it('filters to shipped changes', async () => { + await change('a'); + await change('b', 'schema: spec-driven\nstatus: shipped\n'); + + await new ListCommand().execute(tempDir, 'changes', { status: 'shipped' }); + + const text = logOutput.join('\n'); + expect(text).toContain('b'); + expect(text).not.toMatch(/^\s+a\s/m); + }); + + it('counts an undeclared change as proposed when filtering', async () => { + await change('a'); + await change('b', 'schema: spec-driven\nstatus: shipped\n'); + + await new ListCommand().execute(tempDir, 'changes', { status: 'proposed' }); + + const text = logOutput.join('\n'); + expect(text).toContain('a'); + expect(text).not.toContain('shipped'); + }); + + it('excludes a change whose status cannot be determined from either filter', async () => { + await change('a', 'schema: spec-driven\nstatus: shiped\n'); + await change('b', 'schema: spec-driven\nstatus: shipped\n'); + + await new ListCommand().execute(tempDir, 'changes', { status: 'shipped' }); + const shippedOnly = logOutput.join('\n'); + logOutput = []; + await new ListCommand().execute(tempDir, 'changes', { status: 'proposed' }); + const proposedOnly = logOutput.join('\n'); + + // A filter is a claim of membership; an undetermined change belongs to + // neither list rather than to both. + expect(shippedOnly).toContain('b'); + expect(shippedOnly).not.toMatch(/^\s+a\s/m); + expect(proposedOnly).toBe("No changes with status 'proposed' found."); + }); + + it('still lists an undetermined change when no filter is given', async () => { + await change('a', 'schema: spec-driven\nstatus: shiped\n'); + + await new ListCommand().execute(tempDir, 'changes'); + + expect(logOutput.join('\n')).toContain('a'); + }); + + it('says so when a filter matches nothing', async () => { + await change('a'); + + await new ListCommand().execute(tempDir, 'changes', { status: 'shipped' }); + + expect(logOutput).toEqual(["No changes with status 'shipped' found."]); + }); + + it('emits the lifecycle key in JSON only when declared', async () => { + await change('a'); + await change('b', 'schema: spec-driven\nstatus: shipped\n'); + + await new ListCommand().execute(tempDir, 'changes', { json: true, sort: 'name' }); + + const payload = JSON.parse(logOutput.join('\n')); + expect(payload.changes[0]).not.toHaveProperty('lifecycle'); + expect(payload.changes[1].lifecycle).toBe('shipped'); + // The pre-existing `status` key still means task progress. + expect(payload.changes[1].status).toBe('complete'); + }); + }); }); diff --git a/test/core/sync.test.ts b/test/core/sync.test.ts new file mode 100644 index 0000000000..22ca9df72b --- /dev/null +++ b/test/core/sync.test.ts @@ -0,0 +1,975 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { SyncCommand } from '../../src/core/sync.js'; +import { ArchiveCommand } from '../../src/core/archive.js'; +import { readChangeStatus, writeChangeStatus } from '../../src/utils/change-metadata.js'; +import { + writeStoreMetadataState, + writeStoreRegistryState, +} from '../../src/core/store/foundation.js'; + +vi.mock('@inquirer/prompts', () => ({ + select: vi.fn(), + confirm: vi.fn(), +})); + +const MAIN_SPEC = `# api Specification + +## Purpose +The API surface exposed to clients, and the rules requests are admitted under. + +## Requirements + +### Requirement: Rate limiting +The API SHALL reject requests above the configured rate. + +#### Scenario: Over the limit +- **WHEN** a client exceeds the rate +- **THEN** the API responds 429 +`; + +const ADDED_DELTA = `## ADDED Requirements + +### Requirement: Request tracing +The API SHALL attach a trace id to every response. + +#### Scenario: Traced response +- **WHEN** a request is served +- **THEN** the response carries a trace id +`; + +describe('SyncCommand', () => { + let tempDir: string; + let sync: SyncCommand; + const originalConsoleLog = console.log; + const originalExitCode = process.exitCode; + const originalXdgDataHome = process.env.XDG_DATA_HOME; + const originalCwd = process.cwd(); + let logged: string[]; + + const changesDir = (): string => path.join(tempDir, 'openspec', 'changes'); + const specsDir = (): string => path.join(tempDir, 'openspec', 'specs'); + const output = (): string => logged.join('\n'); + + /** A complete, valid change with one ADDED delta against `api`. */ + async function makeChange( + name: string, + options: { + status?: string; + delta?: string; + tasks?: string; + metadata?: string; + /** Capability id relative to `specs/`, e.g. `platform/session-layout`. */ + capability?: string; + } = {} + ): Promise { + const dir = path.join(changesDir(), name); + const capability = options.capability ?? 'api'; + await fs.mkdir(path.join(dir, 'specs', ...capability.split('/')), { + recursive: true, + }); + await fs.writeFile( + path.join(dir, '.openspec.yaml'), + options.metadata ?? + `schema: spec-driven\n${options.status ? `status: ${options.status}\n` : ''}` + ); + await fs.writeFile( + path.join(dir, 'proposal.md'), + '## Why\nThe API needs request tracing, and today nothing correlates calls.\n\n' + + '## What Changes\n- Add request tracing to the API surface.\n' + ); + await fs.writeFile( + path.join(dir, 'tasks.md'), + options.tasks ?? '## 1. Work\n- [x] 1.1 Done\n' + ); + await fs.writeFile( + path.join(dir, 'specs', ...capability.split('/'), 'spec.md'), + options.delta ?? ADDED_DELTA + ); + return dir; + } + + async function mainSpec(): Promise { + return fs.readFile(path.join(specsDir(), 'api', 'spec.md'), 'utf-8'); + } + + beforeEach(async () => { + // realpath'd: a Windows runner can hand back an 8.3 short path while the + // CLI canonicalizes to the long form, and macOS /var resolves to + // /private/var - both make a root read as outside itself. + tempDir = await fs.realpath( + await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-sync-test-')) + ); + process.chdir(tempDir); + // Keep root resolution off any real store registry on the host. + process.env.XDG_DATA_HOME = path.join(tempDir, 'xdg-data'); + + await fs.mkdir(path.join(specsDir(), 'api'), { recursive: true }); + await fs.mkdir(path.join(changesDir(), 'archive'), { recursive: true }); + await fs.writeFile(path.join(tempDir, 'openspec', 'project.md'), '# Demo\n'); + await fs.writeFile(path.join(specsDir(), 'api', 'spec.md'), MAIN_SPEC); + + logged = []; + console.log = vi.fn((...args: unknown[]) => { + logged.push(args.map(String).join(' ')); + }); + process.exitCode = undefined; + sync = new SyncCommand(); + }); + + afterEach(async () => { + // Before the rm: Windows locks the process working directory, so removing + // a tree we are standing inside fails and leaks it, leaving the next + // describe running from a deleted path. + process.chdir(originalCwd); + console.log = originalConsoleLog; + process.exitCode = originalExitCode; + if (originalXdgDataHome === undefined) delete process.env.XDG_DATA_HOME; + else process.env.XDG_DATA_HOME = originalXdgDataHome; + vi.clearAllMocks(); + try { + await fs.rm(tempDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors. + } + }); + + describe('--check is green at rest', () => { + it('passes a proposed change without examining its deltas', async () => { + await makeChange('add-tracing'); + + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBeUndefined(); + // The whole point of #1683: an open change is the resting state, not a + // failure, so the gate must not go red for one. + expect(output()).toContain('nothing to check'); + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('passes when no change declares a status at all', async () => { + await makeChange('a'); + await makeChange('b'); + + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBeUndefined(); + }); + }); + + describe('--check fails only on a real mistake', () => { + it('reports a shipped change whose deltas are not in the main specs', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBe(1); + expect(output()).toContain('add-tracing'); + expect(output()).toContain('api'); + expect(output()).toContain('openspec sync'); + // A check writes nothing, ever. + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('passes the same change once it is folded', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + + await sync.execute(undefined, { yes: true }); + process.exitCode = undefined; + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBeUndefined(); + expect(await mainSpec()).toContain('Request tracing'); + }); + }); + + describe('folding', () => { + it('applies a shipped change and leaves it in changes/', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + + await sync.execute(undefined, { yes: true }); + + expect(await mainSpec()).toContain('### Requirement: Request tracing'); + // Unlike archive, nothing moves: the change is still open for review. + await expect( + fs.stat(path.join(changesDir(), 'add-tracing')) + ).resolves.toBeTruthy(); + }); + + it('is idempotent: a second run writes nothing', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + + await sync.execute(undefined, { yes: true }); + const afterFirst = await mainSpec(); + logged = []; + await sync.execute(undefined, { yes: true }); + + expect(await mainSpec()).toBe(afterFirst); + expect(output()).toContain('already in sync'); + }); + + it('folds a named change regardless of its status', async () => { + // `openspec sync ` is the deterministic counterpart of the + // agent-driven `/opsx:sync` workflow, and predates any lifecycle field. + await makeChange('add-tracing'); + + await sync.execute('add-tracing', {}); + + expect(await mainSpec()).toContain('Request tracing'); + }); + + it('leaves archive able to run afterwards, changing nothing further', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + await sync.execute(undefined, { yes: true }); + const afterSync = await mainSpec(); + + await new ArchiveCommand().execute('add-tracing', { yes: true }); + + // The early-sync pattern: re-applying a folded delta is a no-op, so the + // archive step is unchanged by having synced first. + expect(await mainSpec()).toBe(afterSync); + await expect( + fs.stat(path.join(changesDir(), 'archive')) + ).resolves.toBeTruthy(); + }); + + it('folds a nested capability into the same nested path', async () => { + const nested = path.join(specsDir(), 'platform', 'session-layout'); + await fs.mkdir(nested, { recursive: true }); + await fs.writeFile( + path.join(nested, 'spec.md'), + '# session-layout Specification\n\n## Purpose\n' + + 'How sessions are laid out across the platform surface.\n\n' + + '## Requirements\n\n### Requirement: Session store\n' + + 'The platform SHALL persist sessions.\n\n' + + '#### Scenario: Persisted\n- **WHEN** a session is created\n- **THEN** it is persisted\n' + ); + await makeChange('evict-sessions', { + status: 'shipped', + capability: 'platform/session-layout', + delta: + '## ADDED Requirements\n\n### Requirement: Session eviction\n' + + 'The platform SHALL evict idle sessions.\n\n#### Scenario: Idle session\n' + + '- **WHEN** a session idles out\n- **THEN** it is evicted\n', + }); + + await sync.execute(undefined, { yes: true }); + + expect(await fs.readFile(path.join(nested, 'spec.md'), 'utf-8')).toContain( + 'Session eviction' + ); + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('folds a MODIFIED delta and reports folded afterwards', async () => { + await makeChange('retry-after', { + status: 'shipped', + delta: + '## MODIFIED Requirements\n\n### Requirement: Rate limiting\n' + + 'The API SHALL reject requests above the configured rate, with a Retry-After header.\n\n' + + '#### Scenario: Over the limit\n- **WHEN** a client exceeds the rate\n' + + '- **THEN** the API responds 429 with Retry-After\n', + }); + + await sync.execute(undefined, { yes: true }); + logged = []; + process.exitCode = undefined; + await sync.execute(undefined, { check: true }); + + expect(await mainSpec()).toContain('Retry-After'); + expect(process.exitCode).toBeUndefined(); + }); + + it('folds a RENAMED delta and counts it as a rename', async () => { + await makeChange('rename-limits', { + status: 'shipped', + delta: + '## RENAMED Requirements\n\n- FROM: `### Requirement: Rate limiting`\n' + + '- TO: `### Requirement: Request throttling`\n', + }); + + await sync.execute(undefined, { check: true, json: true }); + // The only path that increments `renamed`. + expect(JSON.parse(output()).sync.changes[0].specs[0].counts).toEqual({ + added: 0, + modified: 0, + removed: 0, + renamed: 1, + }); + + logged = []; + process.exitCode = undefined; + await sync.execute(undefined, { yes: true }); + + const folded = await mainSpec(); + expect(folded).toContain('### Requirement: Request throttling'); + expect(folded).not.toContain('### Requirement: Rate limiting'); + }); + + it('folds every shipped change and leaves proposed ones alone', async () => { + await makeChange('a-tracing', { status: 'shipped' }); + await makeChange('b-billing', { + status: 'shipped', + capability: 'billing', + delta: + '## ADDED Requirements\n\n### Requirement: Invoice totals\n' + + 'The system SHALL total invoices in the account currency.\n\n' + + '#### Scenario: Totalling\n- **WHEN** an invoice is issued\n' + + '- **THEN** its total is in the account currency\n', + }); + await makeChange('c-proposed'); + + await sync.execute(undefined, { yes: true }); + + expect(await mainSpec()).toContain('Request tracing'); + expect( + await fs.readFile(path.join(specsDir(), 'billing', 'spec.md'), 'utf-8') + ).toContain('Invoice totals'); + expect(output()).toContain('Totals: + 2'); + }); + + it('creates the specs tree when the project has none yet', async () => { + await fs.rm(specsDir(), { recursive: true, force: true }); + await makeChange('add-tracing', { status: 'shipped' }); + + await sync.execute(undefined, { yes: true }); + + expect(await mainSpec()).toContain('### Requirement: Request tracing'); + }); + + it('stops checking a change once it is archived', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + await sync.execute(undefined, { yes: true }); + await new ArchiveCommand().execute('add-tracing', { yes: true }); + + logged = []; + process.exitCode = undefined; + await sync.execute(undefined, { check: true }); + + // Archived deltas are history: re-applying them on top of everything + // that came later is a merge conflict, not a drift check. + expect(process.exitCode).toBeUndefined(); + expect(output()).toContain('nothing to check'); + }); + }); + + describe('folding several changes in one run', () => { + /** A second change adding a different requirement to the SAME capability. */ + async function secondChange(name: string): Promise { + const dir = path.join(changesDir(), name); + await fs.mkdir(path.join(dir, 'specs', 'api'), { recursive: true }); + await fs.writeFile( + path.join(dir, '.openspec.yaml'), + 'schema: spec-driven\nstatus: shipped\n' + ); + await fs.writeFile( + path.join(dir, 'proposal.md'), + '## Why\nThe API needs audit logging, and today nothing records calls.\n\n' + + '## What Changes\n- Add audit logging to the API surface.\n' + ); + await fs.writeFile(path.join(dir, 'tasks.md'), '## 1. Work\n- [x] 1.1 Done\n'); + await fs.writeFile( + path.join(dir, 'specs', 'api', 'spec.md'), + '## ADDED Requirements\n\n### Requirement: Audit logging\n' + + 'The API SHALL record every call in the audit log.\n\n' + + '#### Scenario: Logged call\n- **WHEN** a request is served\n' + + '- **THEN** the audit log gains an entry\n' + ); + } + + it('keeps both folds when two shipped changes touch one capability', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + await secondChange('add-audit'); + + await sync.execute(undefined, { yes: true }); + + // Evaluating both against the same pre-write baseline and then writing + // them in sequence makes the second write erase the first: each rebuilt + // body is a whole file derived from the original spec. The changes do not + // conflict, so losing one is pure data loss. + const spec = await mainSpec(); + expect(spec).toContain('Request tracing'); + expect(spec).toContain('Audit logging'); + expect(spec).toContain('Rate limiting'); + }); + + it('is green afterwards for every change it folded', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + await secondChange('add-audit'); + await sync.execute(undefined, { yes: true }); + + logged = []; + process.exitCode = undefined; + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBeUndefined(); + }); + + // fs.symlink needs Developer Mode or elevation on a Windows runner. + it.skipIf(process.platform === 'win32')( + 'refuses when two capability ids resolve to the same file', + async () => { + // A capability directory may deliberately be a symlink, so two ids + // aliasing one spec is a shape the trust model allows. Writing both in + // sequence is last-writer-wins: one fold is destroyed and the other's + // requirements are filed under the wrong capability. + await makeChange('add-tracing', { status: 'shipped' }); + await fs.mkdir(path.join(changesDir(), 'add-tracing', 'specs', 'apiv2'), { + recursive: true, + }); + await fs.writeFile( + path.join(changesDir(), 'add-tracing', 'specs', 'apiv2', 'spec.md'), + '## ADDED Requirements\n\n### Requirement: Audit logging\n' + + 'The API SHALL record every call in the audit log.\n\n' + + '#### Scenario: Logged call\n- **WHEN** a request is served\n' + + '- **THEN** the audit log gains an entry\n' + ); + await fs.symlink('api', path.join(specsDir(), 'apiv2'), 'dir'); + + await expect(sync.execute('add-tracing', { yes: true })).rejects.toThrow( + /resolve to the same target/ + ); + expect(await mainSpec()).toBe(MAIN_SPEC); + } + ); + }); + + describe('the check path sees what the writer would refuse', () => { + it('fails a shipped change whose delta specs do not validate', async () => { + await makeChange('add-tracing', { + status: 'shipped', + delta: + '## ADDED Requirements\n\n### Requirement: Request tracing\n' + + 'The API SHALL attach a trace id.\n', + }); + + await sync.execute(undefined, { check: true }); + + // The gate promises `shipped => folded`. A delta the merge would refuse + // is not folded and never will be, so certifying it clean is a false + // green on the one surface teams wire into CI. + expect(process.exitCode).toBe(1); + expect(output()).toContain('at least one scenario'); + }); + + it('fails a shipped change whose only delta sits at the specs root', async () => { + // `discoverSpecFiles` does not walk `specs/spec.md`, so the change looks + // like it has nothing to fold while its requirement is silently dropped + // (#1385). Archive and the sync writer both refuse this tree. + const dir = await makeChange('add-tracing', { status: 'shipped' }); + await fs.rm(path.join(dir, 'specs', 'api'), { recursive: true }); + await fs.writeFile(path.join(dir, 'specs', 'spec.md'), ADDED_DELTA); + + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBe(1); + expect(output()).toContain('specs/spec.md'); + }); + + it('passes a shipped change that declares it has no deltas', async () => { + // Archive treats a zero-delta change as fine; sync must give the same + // answer rather than a stricter one of its own. + const dir = await makeChange('add-tracing', { + metadata: 'schema: spec-driven\nstatus: shipped\nskip_specs: true\n', + }); + await fs.rm(path.join(dir, 'specs'), { recursive: true }); + + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBeUndefined(); + }); + }); + + describe('guards', () => { + it('refuses a change with incomplete tasks', async () => { + await makeChange('add-tracing', { + status: 'shipped', + tasks: '## 1. Work\n- [ ] 1.1 Not done\n', + }); + + await expect(sync.execute('add-tracing', {})).rejects.toThrow( + /incomplete task/i + ); + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('proceeds past incomplete tasks with --yes', async () => { + await makeChange('add-tracing', { + status: 'shipped', + tasks: '## 1. Work\n- [ ] 1.1 Not done\n', + }); + + await sync.execute('add-tracing', { yes: true }); + + expect(await mainSpec()).toContain('Request tracing'); + }); + + it('refuses a delta that fails validation, writing nothing', async () => { + await makeChange('add-tracing', { + status: 'shipped', + // An ADDED requirement with no scenario is what validate rejects. + delta: + '## ADDED Requirements\n\n### Requirement: Request tracing\n' + + 'The API SHALL attach a trace id.\n', + }); + + await expect(sync.execute('add-tracing', { yes: true })).rejects.toThrow( + /must include at least one scenario/ + ); + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('never deletes a spec: a retirement is handed to archive', async () => { + await makeChange('retire-limits', { + status: 'shipped', + delta: + '## REMOVED Requirements\n\n### Requirement: Rate limiting\n' + + '**Reason**: Moved to the gateway.\n**Migration**: Configure the gateway.\n', + }); + + await expect(sync.execute('retire-limits', { yes: true })).rejects.toThrow( + /openspec archive/ + ); + // The one irreversible operation in the system stays behind archive's + // authorization marker and its rollback-safe deletion. + await expect( + fs.stat(path.join(specsDir(), 'api', 'spec.md')) + ).resolves.toBeTruthy(); + }); + + it('reports a retirement in --check without offering sync as the fix', async () => { + await makeChange('retire-limits', { + status: 'shipped', + delta: + '## REMOVED Requirements\n\n### Requirement: Rate limiting\n' + + '**Reason**: Moved to the gateway.\n**Migration**: Configure the gateway.\n', + }); + + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBe(1); + expect(output()).toContain('openspec archive'); + expect(output()).not.toContain('Run openspec sync to fold them'); + }); + }); + + describe('--no-validate', () => { + it('needs --yes, the way archive needs an answer', async () => { + await makeChange('add-tracing', { + status: 'shipped', + delta: + '## ADDED Requirements\n\n### Requirement: Request tracing\n' + + 'The API SHALL attach a trace id.\n', + }); + + await expect( + sync.execute('add-tracing', { validate: false }) + ).rejects.toThrow(/needs confirmation/); + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('folds a delta validation would refuse, once confirmed', async () => { + await makeChange('add-tracing', { + status: 'shipped', + // The same scenario-less ADDED the validation guard rejects. + delta: + '## ADDED Requirements\n\n### Requirement: Request tracing\n' + + 'The API SHALL attach a trace id.\n', + }); + + await sync.execute('add-tracing', { validate: false, yes: true }); + + expect(await mainSpec()).toContain('### Requirement: Request tracing'); + }); + }); + + describe('a fold that does not settle', () => { + it('refuses to report success when two shipped changes cannot both hold', async () => { + const conflicting = (discriminator: string): string => + '## MODIFIED Requirements\n\n### Requirement: Rate limiting\n' + + `The API SHALL reject requests above the configured rate, per ${discriminator}.\n\n` + + '#### Scenario: Over the limit\n- **WHEN** a client exceeds the rate\n' + + `- **THEN** the API responds 429 with a per-${discriminator} message\n`; + await makeChange('a-widen', { status: 'shipped', delta: conflicting('API key') }); + await makeChange('b-narrow', { status: 'shipped', delta: conflicting('IP address') }); + + // Reporting success would have `--check`, run immediately after, go red + // for a fold that just claimed to have succeeded. + await expect(sync.execute(undefined, { yes: true })).rejects.toThrow( + /still report unfolded deltas/ + ); + }); + }); + + describe('undetermined status fails closed', () => { + it('reports a change whose status value is not a known state', async () => { + await makeChange('add-tracing', { + metadata: 'schema: spec-driven\nstatus: shiped\n', + }); + + await sync.execute(undefined, { check: true }); + + // Rounding this to `proposed` is the fail-open direction: a change that + // declared itself shipped and then had its metadata broken would + // silently stop being checked. + expect(process.exitCode).toBe(1); + expect(output()).toContain('add-tracing'); + expect(output()).toContain('lifecycle status'); + }); + + it('reports a change whose metadata is not valid YAML but names status', async () => { + await makeChange('add-tracing', { + metadata: 'schema: spec-driven\nstatus: [unclosed\n', + }); + + await sync.execute(undefined, { check: true }); + + expect(process.exitCode).toBe(1); + expect(output()).toContain('not valid YAML'); + }); + + it('ignores broken metadata that never mentions status', async () => { + await makeChange('add-tracing', { metadata: 'schema: [unclosed\n' }); + + await sync.execute(undefined, { check: true }); + + // Not this gate's problem to report; `openspec status` and `validate` + // already fail on it, and claiming it here would be noise. + expect(process.exitCode).toBeUndefined(); + }); + }); + + describe('--ship', () => { + it('sets the field and folds in one run', async () => { + const dir = await makeChange('add-tracing'); + + await sync.execute('add-tracing', { ship: true }); + + expect(readChangeStatus(dir).status).toBe('shipped'); + expect(await mainSpec()).toContain('Request tracing'); + }); + + it('does not stamp the change when a guard refuses it', async () => { + const dir = await makeChange('add-tracing', { + tasks: '## 1. Work\n- [ ] 1.1 Not done\n', + }); + + await expect( + sync.execute('add-tracing', { ship: true }) + ).rejects.toThrow(/incomplete task/i); + + // Stamping before the guards would leave the tree in the exact state + // --ship exists to prevent: shipped, with its deltas absent. + expect(readChangeStatus(dir).status).toBe('proposed'); + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('does not stamp the change when its deltas fail validation', async () => { + const dir = await makeChange('add-tracing', { + delta: + '## ADDED Requirements\n\n### Requirement: Request tracing\n' + + 'The API SHALL attach a trace id.\n', + }); + + await expect( + sync.execute('add-tracing', { ship: true }) + ).rejects.toThrow(/must include at least one scenario/); + + expect(readChangeStatus(dir).status).toBe('proposed'); + }); + + it('does not stamp the change when the spec write fails', async () => { + const dir = await makeChange('add-tracing'); + const real = fs.writeFile; + const spy = vi + .spyOn(fs, 'writeFile') + .mockImplementation(async (...args: Parameters) => { + if (String(args[0]).endsWith(path.join('specs', 'api', 'spec.md'))) { + throw new Error('ENOSPC: no space left on device'); + } + return real(...args); + }); + + await expect(sync.execute('add-tracing', { ship: true })).rejects.toThrow( + /Could not write the main specs/ + ); + spy.mockRestore(); + + // The field is stamped only once the specs on disk are correct, so a + // failed write cannot leave a change claiming shipped with its deltas + // absent. + expect(readChangeStatus(dir).status).toBe('proposed'); + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('refuses before folding when there is no metadata file to stamp', async () => { + const dir = await makeChange('add-tracing'); + await fs.rm(path.join(dir, '.openspec.yaml')); + + await expect(sync.execute('add-tracing', { ship: true })).rejects.toThrow( + /no \.openspec\.yaml/ + ); + + // Folding first and discovering the missing file afterwards leaves a + // fold that is never stamped, and a rerun that fails in the same place. + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + + it('is refused alongside --check', async () => { + await makeChange('add-tracing'); + + await expect( + sync.execute('add-tracing', { ship: true, check: true }) + ).rejects.toThrow(/one or the other/); + }); + + it('is refused without a change name', async () => { + await makeChange('add-tracing'); + + await expect(sync.execute(undefined, { ship: true })).rejects.toThrow( + /needs the change/ + ); + }); + }); + + describe('JSON output', () => { + it('reports a clean check and exits 0', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + await sync.execute(undefined, { yes: true }); + logged = []; + process.exitCode = undefined; + + await sync.execute(undefined, { check: true, json: true }); + + const payload = JSON.parse(output()); + expect(payload.sync.clean).toBe(true); + expect(payload.sync.checked).toBe(true); + expect(payload.sync.changes[0].change).toBe('add-tracing'); + expect(process.exitCode).toBeUndefined(); + }); + + it('reports a dirty check and exits 1', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + + await sync.execute(undefined, { check: true, json: true }); + + const payload = JSON.parse(output()); + expect(payload.sync.clean).toBe(false); + expect(payload.sync.changes[0].specs[0].counts.added).toBe(1); + expect(process.exitCode).toBe(1); + }); + + it('names a blocked change rather than showing it as having no specs', async () => { + await makeChange('retire-limits', { + status: 'shipped', + delta: + '## REMOVED Requirements\n\n### Requirement: Rate limiting\n' + + '**Reason**: Moved to the gateway.\n**Migration**: Configure the gateway.\n', + }); + + await sync.execute(undefined, { check: true, json: true }); + + // Structurally unlike a dirty change: no spec entries at all, so a CI + // consumer reading `specs` alone would read this as clean. + const change = JSON.parse(output()).sync.changes[0]; + expect(change.folded).toBe(false); + expect(change.specs).toEqual([]); + expect(change.blockers[0]).toContain('openspec archive'); + expect(process.exitCode).toBe(1); + }); + + it('emits one status document for a blocked run', async () => { + await makeChange('add-tracing', { + status: 'shipped', + tasks: '## 1. Work\n- [ ] 1.1 Not done\n', + }); + + await sync.execute('add-tracing', { json: true }); + + const payload = JSON.parse(output()); + expect(payload.sync).toBeNull(); + expect(payload.status[0].code).toBe('sync_tasks_incomplete'); + expect(process.exitCode).toBe(1); + }); + }); + + describe('write failures leave no partly folded tree', () => { + it('restores every spec it had already written', async () => { + await makeChange('add-tracing', { status: 'shipped' }); + // A second capability, so the run writes more than one file and a + // failure on the later one can strand the earlier one. + await fs.mkdir(path.join(changesDir(), 'add-tracing', 'specs', 'billing'), { + recursive: true, + }); + await fs.writeFile( + path.join(changesDir(), 'add-tracing', 'specs', 'billing', 'spec.md'), + '## ADDED Requirements\n\n### Requirement: Invoice totals\n' + + 'The system SHALL total invoices in the account currency.\n\n' + + '#### Scenario: Totalling\n- **WHEN** an invoice is issued\n' + + '- **THEN** its total is in the account currency\n' + ); + + const before = await mainSpec(); + const real = fs.writeFile; + let writes = 0; + const spy = vi + .spyOn(fs, 'writeFile') + .mockImplementation(async (...args: Parameters) => { + // Let the first spec through, fail the second, then let the + // rollback's own writes succeed. + if (++writes === 2) throw new Error('ENOSPC: no space left on device'); + return real(...args); + }); + + await expect(sync.execute(undefined, { yes: true })).rejects.toThrow( + /Could not write the main specs/ + ); + spy.mockRestore(); + + expect(await mainSpec()).toBe(before); + // The spec this run would have created must not be left behind either. + await expect( + fs.stat(path.join(specsDir(), 'billing', 'spec.md')) + ).rejects.toThrow(); + }); + }); + + describe('stores', () => { + it("folds the selected store's specs and leaves the working directory alone", async () => { + const storeRoot = path.join(tempDir, 'stores', 'team-context'); + const storeSpec = path.join(storeRoot, 'openspec', 'specs', 'api', 'spec.md'); + const changeDir = path.join(storeRoot, 'openspec', 'changes', 'add-tracing'); + await fs.mkdir(path.join(storeRoot, 'openspec', 'specs', 'api'), { recursive: true }); + await fs.mkdir(path.join(storeRoot, 'openspec', 'changes', 'archive'), { + recursive: true, + }); + await fs.mkdir(path.join(changeDir, 'specs', 'api'), { recursive: true }); + await fs.writeFile( + path.join(storeRoot, 'openspec', 'config.yaml'), + 'schema: spec-driven\n' + ); + await fs.writeFile(storeSpec, MAIN_SPEC); + await fs.writeFile( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nstatus: shipped\n' + ); + await fs.writeFile( + path.join(changeDir, 'proposal.md'), + '## Why\nThe API needs request tracing, and today nothing correlates calls.\n\n' + + '## What Changes\n- Add request tracing to the API surface.\n' + ); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '## 1. Work\n- [x] 1.1 Done\n'); + await fs.writeFile(path.join(changeDir, 'specs', 'api', 'spec.md'), ADDED_DELTA); + await writeStoreMetadataState(storeRoot, { version: 1, id: 'team-context' }); + await writeStoreRegistryState({ + version: 1, + stores: { 'team-context': { backend: { type: 'git', local_path: storeRoot } } }, + }); + + await sync.execute(undefined, { yes: true, store: 'team-context' }); + + expect(await fs.readFile(storeSpec, 'utf-8')).toContain('Request tracing'); + // The working directory's own project has no shipped change; nothing + // there may be touched by a store-scoped run. + expect(await mainSpec()).toBe(MAIN_SPEC); + }); + }); + + describe('errors', () => { + it('names the available changes when the change does not exist', async () => { + await makeChange('add-tracing'); + + await expect(sync.execute('nope', {})).rejects.toThrow(/add-tracing/); + }); + }); +}); + +describe('readChangeStatus / writeChangeStatus', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-status-test-')); + await fs.mkdir(path.join(tempDir, 'openspec', 'changes', 'c'), { + recursive: true, + }); + }); + + afterEach(async () => { + try { + await fs.rm(tempDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors. + } + }); + + const changeDir = (): string => path.join(tempDir, 'openspec', 'changes', 'c'); + + it('reads an absent metadata file as proposed and undeclared', () => { + const marker = readChangeStatus(changeDir()); + expect(marker).toEqual({ status: 'proposed', declared: false }); + }); + + it('reads an absent status field as proposed and undeclared', async () => { + await fs.writeFile( + path.join(changeDir(), '.openspec.yaml'), + 'schema: spec-driven\n' + ); + expect(readChangeStatus(changeDir())).toEqual({ + status: 'proposed', + declared: false, + }); + }); + + it('preserves comments and key order when setting status', async () => { + const original = + '# hand-authored\nschema: spec-driven\ncreated: 2026-09-07\n'; + await fs.writeFile(path.join(changeDir(), '.openspec.yaml'), original); + + writeChangeStatus(changeDir(), 'shipped'); + + const written = await fs.readFile( + path.join(changeDir(), '.openspec.yaml'), + 'utf-8' + ); + expect(written).toContain('# hand-authored'); + expect(written.indexOf('schema:')).toBeLessThan(written.indexOf('created:')); + expect(readChangeStatus(changeDir()).status).toBe('shipped'); + }); + + it('leaves the state undetermined when the declared schema does not resolve', async () => { + await fs.writeFile( + path.join(changeDir(), '.openspec.yaml'), + 'schema: no-such-schema\nstatus: shipped\n' + ); + + // Distinct from a bad status value: the field parses, the schema does not + // resolve, and rounding that to `proposed` is the fail-open direction. + const marker = readChangeStatus(changeDir()); + + expect(marker.invalidReason).toContain('no-such-schema'); + expect(marker.declared).toBe(false); + }); + + it('replaces a status that is already set, in place', async () => { + await fs.writeFile( + path.join(changeDir(), '.openspec.yaml'), + '# hand-authored\nschema: spec-driven\nstatus: proposed\ncreated: 2026-09-07\n' + ); + + writeChangeStatus(changeDir(), 'shipped'); + + // Replaced, not appended: a duplicate `status` key would make the file + // parse differently in yaml and in a hand-reading author's head. + expect(await fs.readFile(path.join(changeDir(), '.openspec.yaml'), 'utf-8')).toBe( + '# hand-authored\nschema: spec-driven\nstatus: shipped\ncreated: 2026-09-07\n' + ); + }); + + it('refuses to stamp a change with no metadata file', () => { + expect(() => writeChangeStatus(changeDir(), 'shipped')).toThrow( + /nothing to set status on/ + ); + }); +}); diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 3f309bdaa3..9bb02d5376 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -38,46 +38,46 @@ import { import { STORE_SELECTION_GUIDANCE } from '../../../src/core/templates/workflows/store-selection.js'; const EXPECTED_FUNCTION_HASHES: Record = { - getExploreSkillTemplate: '6315fcc5c2eb848963bc8bca4c23e657412a99608e610daee59fb4e58cd21fd4', - getNewChangeSkillTemplate: 'eabd1e895c5881dcb17dcbaa3fb26098dd59e8eacb318e400820b4dc811ef781', - getContinueChangeSkillTemplate: '012136f6411a99c8fa228e2f9444cb64b0a89e0f56fdeac2fe03b2f5bee0c5d7', - getApplyChangeSkillTemplate: 'd1e7d5ceb85193c0964057dbb88e9651526754bd33f84020e2440ff0621d5dbb', - getFfChangeSkillTemplate: 'efa6a70c111b18b61a7720250b9622afa9a212fb64edf609cf80e2182a9bdf8c', - getSyncSpecsSkillTemplate: 'b099e2ff31859c9b10d928066e662524f9aad9ecf2be12fceacb732d718c4146', - getOnboardSkillTemplate: '3a836faae463d88c289a1c129cb7ee556a563b7e53e1a52a4711ff152a3b51f7', - getOpsxExploreCommandTemplate: 'b4706a5b8fd280f7929eea610ecc9d41676b2d2dd6653d259cbbc2bfe01813d9', - getOpsxNewCommandTemplate: 'f2d30e569798a4c92ba932859d6ba4e0ad10e18feccbade1cfee0957597b3463', - getOpsxContinueCommandTemplate: 'e50e50266efa1b8e64ff9b6274ee8254f0a240d6adc1b862d126e2f1c9d3a559', - getOpsxApplyCommandTemplate: 'e3579ac78f2e2c75fa3d3a7ac7dc3e49c395e96f7323398f0f041d94f8de9bb0', - getOpsxFfCommandTemplate: '21132fc9c6d3b3ab2d2295d6bbd72d1e0052eb35ea1be0258c8b1ab3e200c4db', - getArchiveChangeSkillTemplate: '56bfada1a5f35a127791b70de9d428a75b5aedd1584d6c9803a1ecb1fd1b4a23', - getBulkArchiveChangeSkillTemplate: '93875998cade5322d95b43299fba794bc1da754e917dd63a770406386a6d295d', - getOpsxSyncCommandTemplate: '0d2427efb79986e8fff3f96bd075a739c80d45eb29159fae717e950030da8202', - getVerifyChangeSkillTemplate: '223b7ffd99299a7d430e13092b9a0a3421b39f0d3217232f46c39d79b5f619ff', - getOpsxArchiveCommandTemplate: '9f973c819b11620985b03322945f0e0a92a02a2ef455b94e74482f5e6292ac5d', - getOpsxOnboardCommandTemplate: 'ee99aa99252c602720fbb8c63fb3ac438a5bd4e952fd961ddf1ae956cbfc2c8f', - getOpsxBulkArchiveCommandTemplate: '9fa8cdebe2f5667ebfc37bdc023396762c59d5b038c771dac2d8fd2c19e2627b', - getOpsxVerifyCommandTemplate: '1efcf7eff0671f48e9d9420f50865c563dd3079ee60f8c380bb7a90dd0102696', - getOpsxProposeSkillTemplate: '9c0fbf0137151bd03ec30c45180f83daec96e8976ceaf517c63147f84b803446', - getOpsxProposeCommandTemplate: 'b3c145f541dcc13d9859eae8f7bedbe4553371477ed2c5ac07a4a80f82c46f52', + getExploreSkillTemplate: '5866ff47a73ae83523c9d6846fa0524c060cc0b10408b4e0d7f5ed7a1d1b3055', + getNewChangeSkillTemplate: 'e32224f45cfadf3d8ca74e0f0eb27ffb826f9508c0a30ff597846c7f49016aed', + getContinueChangeSkillTemplate: '20ddf6b8131bedcf8cb64925062f37eedd649836f680e5de49705f09f4961824', + getApplyChangeSkillTemplate: '12ed95c079bbbe2f9f114854b7cac59b5ff93236292ba680c883ac91a3e8a8d4', + getFfChangeSkillTemplate: 'ce6c292cce0e26aee23f31fd1a61fa1869d5f9ad11f114f2d8fbc2071308bfa7', + getSyncSpecsSkillTemplate: 'c135a7a1f636a154d74caa54327fab04cfabfb3549e2d82b31776f443010e52f', + getOnboardSkillTemplate: 'f91615491da1bc1b08505eee28b199acbf7041fb6e6b841adeed50ac8138b2d7', + getOpsxExploreCommandTemplate: 'f052e24ca5efa83bbecd26090d1536878888f9752883046bae57f74e771fcbc3', + getOpsxNewCommandTemplate: '243cef36241e576095ebb41108b5624c624af97a1f877b26e1a95fd7082db8a7', + getOpsxContinueCommandTemplate: 'b6d7b6336659b35e69e657afa58cfa1601aedb8543b0380302d1a2467149ca5c', + getOpsxApplyCommandTemplate: '061b9e26ee81414a547f7920dd91b16a052b8e59e4aa767cf9103047386e7e74', + getOpsxFfCommandTemplate: 'a7e65e4eb0941bd4f2231347fd7080784d55c04cee21fa1691b477dfabb40e9e', + getArchiveChangeSkillTemplate: '49d670ca69a4dc48f5db8a18005abf08f43000666e70963e21f2c46cca46e543', + getBulkArchiveChangeSkillTemplate: '310ba62a8c3769925ebe7b12f46c27a1a93446ddcfa3903ec8085aaf2da24b31', + getOpsxSyncCommandTemplate: 'bf2e18a079e6a60a341d2eac5574396a5796cfe9f6cc503fb5635d42f91323dd', + getVerifyChangeSkillTemplate: 'ededd72f473fdde4e0466c10d41f81a3b7f3bd4f3d3da8d351b4fe7d0a2b56b6', + getOpsxArchiveCommandTemplate: 'ad3a85d957429374cb61b7e77760d51e1cb41fd86a1efb53d33c67c30c265aec', + getOpsxOnboardCommandTemplate: 'eadf07b8ae5215923cc8e398d0cfae6f5e319e337e37df74df287dda8e664e46', + getOpsxBulkArchiveCommandTemplate: '02a1176fa3183704c0975ece62ff690af9c8338a45d71134fbd0e92daf0bdf77', + getOpsxVerifyCommandTemplate: '555fe9158d31fde1806484711713633b392fcfc27e6ba69722385a7f5a2c2cca', + getOpsxProposeSkillTemplate: '9ba481f5710a67cf83861ce9a9fbb0e0be1bc1e73a2fe5901813e464b67bf079', + getOpsxProposeCommandTemplate: '3cc80f65739184446a04842a4fdb3c0209eb5ce2825201909a0e599012a1a9cc', getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133', - getUpdateChangeSkillTemplate: '7dc8abc6f64c58bf34d7581ed4ab095a3b7a53cb372349bee2d840db58622819', - getOpsxUpdateCommandTemplate: 'e2388521b22f92f74561df9a0c2f98e1fa4d265af93b5ba26f42fb47a6c5bfed', + getUpdateChangeSkillTemplate: '45c1e97de46c52cafef9d5e12a5ed96fe4264d8c978404b8e3c3ee519067c18b', + getOpsxUpdateCommandTemplate: '3a52e1495abbb6b9f50efb6651b95fd53fd1c4fd329deb828e2962cc26e24843', }; const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { - 'openspec-explore': 'dd84af68d3c93b40659dcdd8d383423b25b443cacdc4b514cd70614ae10c5cac', - 'openspec-new-change': 'ec4529beef978e34634a6f7286fab55d68fad8fb374dceb45691d52caab33fbb', - 'openspec-continue-change': 'bb6194a16c54891cdb253678e8f70ce53b2af86735243980f366ce551d37e42e', - 'openspec-apply-change': '81ea96d9fa6ec8536cd23c1fe561ed28e1cc1cad0a8ceb700588e08974cc0e49', - 'openspec-ff-change': '31355250514bce51b16ff37ee2b833bc9d475cd0dbd4b1f68fe2041694575623', - 'openspec-sync-specs': 'd933d8856584d6c1253de91e652e7aee9e85c77ad4d3531f6476f79d84e6e5e8', - 'openspec-archive-change': '7c65053d674ba4e1e20e2bf73ba7e5a7f94baef2eaa9b33cee48d4cadea51b7a', - 'openspec-bulk-archive-change': '2039b9ecf6e64339dffe0e16272507a386d9fe326f419ff758315aa736fdd96c', - 'openspec-verify-change': 'af9be013dcbe8c6d8f6d9ab10c893fbd03f4c62933c384d82f63894dd0ceb84f', - 'openspec-onboard': 'f6f59476acaf5e4d65dbb180da4cef62432612f3cecf207d471a951295e2003a', - 'openspec-propose': 'e358b45102a88082cf20f5c4441cba02533724ad6eef8ed15ba174e3496cb6ed', - 'openspec-update-change': '586547406aca94422dfeb3ffedce6c01049429b743f57ce829baa79ebc714d51', + 'openspec-explore': '2fa2c1d9d1e7b535a133b67b45cef7ca11d6cc769ace264b2711c36027151a9d', + 'openspec-new-change': '8e22baadbbf38ef5394c1ca598a1b49f1eba4e63f2dc6604c445f0dffa1b46d3', + 'openspec-continue-change': 'bf32daae9ac4e904be15dd40865f68a9c3370172ff4279ec32fde46e9aaca845', + 'openspec-apply-change': 'c8fc28e38f519156c61dc17db44dafdc4abecfdb814d83c801744e64a2197482', + 'openspec-ff-change': 'd95ffb998e62398c33193151f4f5e8a984b7c47f903776dd9628bd343c16ca3a', + 'openspec-sync-specs': '1b8fca8224cc6e2617dc225e54b5f35dddd67f0d773c79f7e84eeb6a4b82c304', + 'openspec-archive-change': '2f7628e22644ade9e5e59b0173044a6259a2ca63445e7ea1bb9e4384c407753a', + 'openspec-bulk-archive-change': 'f22045c417d243e6b2cf5c4bf3b6007b4e3bf27bf28de95a2c79cf97b9354924', + 'openspec-verify-change': 'd06e831bfe5d78979fdccebde4e615ced13f7c5c485b3bd6ab1ce90465b968f7', + 'openspec-onboard': 'c21e21a894e7a6dd03849846c04ce932bcd6ee835acc6712fa675cc938b3d104', + 'openspec-propose': 'c2696908254ee21b6f4500a05cb27ff9912a9d5d675e6799d3ee67fe69164285', + 'openspec-update-change': 'fc1a4b93e7310b0b30249c1b22e7bf0eb84dd5c523f0831d236be0067a0f8410', }; // Intentionally excludes getFeedbackSkillTemplate: this list only models templates diff --git a/test/utils/change-status-atomic-write.test.ts b/test/utils/change-status-atomic-write.test.ts new file mode 100644 index 0000000000..a7589c1cde --- /dev/null +++ b/test/utils/change-status-atomic-write.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { promises as fsp } from 'fs'; +import path from 'path'; +import os from 'os'; + +/** + * `writeChangeStatus` writes through a sibling temp file and renames it into + * place. A direct write that fails partway truncates `.openspec.yaml`, and that + * file carries the change's `schema:` — losing it breaks every command that + * reads the change, not only the field being set. + * + * Injected through a module mock rather than filesystem permissions: chmod does + * not constrain root and does not exist on Windows, so a permissions-based test + * would not run on two of the three CI legs. + */ +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + default: actual, + writeFileSync: (...args: Parameters) => { + if (String(args[0]).includes('.openspec-status-')) { + throw new Error('ENOSPC: no space left on device'); + } + return actual.writeFileSync(...args); + }, + }; +}); + +const { writeChangeStatus } = await import('../../src/utils/change-metadata.js'); + +describe('writeChangeStatus durability', () => { + let tempDir: string; + const changeDir = (): string => path.join(tempDir, 'openspec', 'changes', 'c'); + const metaPath = (): string => path.join(changeDir(), '.openspec.yaml'); + + beforeEach(async () => { + tempDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'openspec-atomic-')); + await fsp.mkdir(changeDir(), { recursive: true }); + }); + + afterEach(async () => { + await fsp.rm(tempDir, { recursive: true, force: true }).catch(() => {}); + }); + + it('leaves the original file intact when the write fails', async () => { + const original = '# hand-authored\nschema: spec-driven\ncreated: 2026-09-07\n'; + await fsp.writeFile(metaPath(), original); + + expect(() => writeChangeStatus(changeDir(), 'shipped')).toThrow(/ENOSPC/); + + expect(await fsp.readFile(metaPath(), 'utf-8')).toBe(original); + }); + + it('leaves no temp file behind when the write fails', async () => { + await fsp.writeFile(metaPath(), 'schema: spec-driven\n'); + + expect(() => writeChangeStatus(changeDir(), 'shipped')).toThrow(); + + const strays = (await fsp.readdir(changeDir())).filter((name) => + name.includes('.openspec-status-') + ); + expect(strays).toEqual([]); + }); +});