trending/OpenSpec
1
0
Fork 0
mirror of https://github.com/Fission-AI/OpenSpec.git synced 2026-10-02 05:24:34 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(sync): fold each change against the live tree, and gate the check

Browse Source 
Hardening round. Four defects, all reproduced before being fixed.

**Two shipped changes touching one capability lost a fold.** `applyFolds`
evaluated every change against the pre-write baseline and then wrote them
all, so each rebuilt body was a whole file derived from the original spec
and the second write erased the first — silently, while the console
reported both as applied. The changes did not conflict; the batch read a
stale baseline. Archive never had this because it takes one change per
invocation. Sync now folds one change at a time, re-deriving each against
the specs as they are at that moment.

**Two capability ids resolving to one file overwrote each other.** A
capability directory may deliberately be a symlink, so this is a shape the
trust model allows rather than an accident. Archive refuses it outright;
sync wrote both and lost one. Archive's check is now shared by both, so
they cannot disagree about which trees they will write.

**`sync --check` was green for a change whose delta the writer refuses.**
The check only asked "is what was discovered folded?", and
`discoverSpecFiles` does not walk `specs/spec.md`, so a change whose only
delta sat there certified as clean while archive and the sync writer both
refused the same tree (#1385). Delta validation now runs inside the
evaluation, so it runs on the check path too — and it asks archive's own
question about whether a change has deltas at all, so a zero-delta change
gets the same answer from both commands.

**`--ship` could fold and then fail forever.** A change with no
`.openspec.yaml` had its specs written and its stamp refused, and the
rerun failed in the same place, so the ordering's usual self-correction
did not apply. Checked up front now.

Also: `--no-validate` requires `--yes`, matching archive's refusal to skip
validation without an explicit answer; the rollback no longer "restores"
targets it never wrote (a false data-loss alarm) and refuses to clobber a
file something else changed mid-run; and `test/core/sync.test.ts` restores
the process working directory before removing its temp tree, which Windows
locks.

Nineteen tests added, each mutation-verified.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Clay Good
2026-09-07 10:14:52 -05:00
co-authored by Claude Opus 5
parent eda3dd798c
commit 679d3f7bbb
11 changed files with 815 additions and 200 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/sync-lifecycle-status.md
+2
View File
@@ -9,3 +9,5 @@ Add `openspec sync`, which folds a change's delta specs into the main specs with
`openspec list --status <state>` 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.
docs-lab/reference/cli.md
+1 -1
View File
@@ -895,7 +895,7 @@ the change is still open, and CI can check that they are.
| Flag | Effect |
|---|---|
| `--check` | Report shipped changes with unfolded deltas and exit 1. Writes nothing. |
| `--ship` | Set `status: shipped` on the named change, then fold it. |
| `--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. |
docs/cli.md
+6 -1
View File
@@ -651,7 +651,7 @@ that they are.
| Option | Description |
|--------|-------------|
| `--check` | Report shipped changes whose deltas are not in the main specs and exit 1. Writes nothing |
| `--ship` | Set `status: shipped` on the named change, then fold it — both land in one set of file changes for you to commit |
| `--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 |
@@ -669,6 +669,11 @@ The field is optional and absent by default. A change with no `status` is
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 —
docs/commands.md
+2
View File
@@ -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]
docs/glossary.md
+3 -1
View File
@@ -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-<name>/`. 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
openspec/changes/add-standalone-spec-sync/proposal.md
+7 -2
View File
@@ -65,5 +65,10 @@ from drifting apart (#1112).
- Affected docs: `docs/cli.md`, `docs/team-workflow.md`,
`docs-lab/reference/cli.md`
Credit: the diagnosis and the `shipped ⇒ folded` framing are from Matan Bendix
Shenhav's proposal in #1683.
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.
src/cli/index.ts
+1 -1
View File
@@ -515,7 +515,7 @@ 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', 'Mark the named change `status: shipped` before folding it')
.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)')
src/core/archive.ts
+93 -54
View File
@@ -821,35 +821,57 @@ async function fingerprintSpecInputs(update: SpecUpdate): Promise<string> {
return `${await fingerprintPath(update.source)}\n${await fingerprintPath(update.target)}`;
}
async function mutationTargetIdentity(mutation: SpecMutation): Promise<string> {
async function specTargetIdentity(target: string): Promise<string> {
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<void> {
/**
* 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<void> {
const owners = new Map<string, string>();
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<void> {
await assertDistinctSpecTargets(
mutations.map(({ update }) => ({ id: update.id, target: update.target })),
'archiving'
);
}
async function captureSpecSnapshots(mutations: SpecMutation[]): Promise<SpecSnapshot[]> {
return Promise.all(
mutations.map(async ({ update, outcome, rebuilt }) => {
@@ -1055,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<boolean> {
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<void> {
const json = !!options.json;
@@ -1222,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
src/core/completions/command-registry.ts
+1 -1
View File
@@ -210,7 +210,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [
},
{
name: 'ship',
description: 'Mark the named change `status: shipped` before folding it',
description: 'Fold the named change, then mark it `status: shipped`',
},
{
name: 'yes',
src/core/sync.ts
+294 -134
View File
@@ -40,9 +40,21 @@
* set is exactly the active changes that declare `status: shipped`, which is
* bounded and drains itself as those changes archive.
*
* Credit: the diagnosis, the `shipped => folded` framing, and the argument that
* a checker which reimplements the doer eventually disagrees with it are all
* from Matan Bendix Shenhav's proposal in #1683.
* 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';
@@ -65,7 +77,12 @@ import {
writeUpdatedSpec,
type SpecUpdate,
} from './specs-apply.js';
import { isRetirableSpec, listActiveChangeNames } from './archive.js';
import {
assertDistinctSpecTargets,
changeHasDeltaSpecsToValidate,
isRetirableSpec,
listActiveChangeNames,
} from './archive.js';
import {
readChangeStatus,
writeChangeStatus,
@@ -82,7 +99,11 @@ import { folderStyleNameProblem } from './id.js';
export interface SyncOptions {
/** Report what is unfolded and exit non-zero, without writing anything. */
check?: boolean;
/** Set `status: shipped` on the change before folding, in one working-tree diff. */
/**
* 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;
@@ -186,7 +207,8 @@ function sumCounts(counts: SyncSpecReport['counts']): number {
async function evaluateChange(
changeName: string,
changeDir: string,
mainSpecsDir: string
mainSpecsDir: string,
validate = true
): Promise<Evaluation> {
const status = readChangeStatus(changeDir);
const report: SyncChangeReport = {
@@ -208,6 +230,52 @@ async function evaluateChange(
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);
@@ -369,10 +437,41 @@ export class SyncCommand {
);
}
// 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 ?? '<change-name>'} --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,
@@ -395,13 +494,18 @@ export class SyncCommand {
const evaluations: Evaluation[] = [];
for (const name of targets) {
evaluations.push(
await evaluateChange(name, path.join(changesDir, name), mainSpecsDir)
await evaluateChange(
name,
path.join(changesDir, name),
mainSpecsDir,
options.validate !== false
)
);
}
return check
? this.reportCheck(evaluations, root, json)
: this.applyFolds(evaluations, changesDir, root, options, json);
: this.applyFolds(evaluations, changesDir, mainSpecsDir, root, options, json);
}
/** A named change has to exist, exactly as archive requires. */
@@ -517,6 +621,7 @@ export class SyncCommand {
private async applyFolds(
evaluations: Evaluation[],
changesDir: string,
mainSpecsDir: string,
root: ResolvedOpenSpecRoot,
options: SyncOptions,
json: boolean
@@ -537,74 +642,126 @@ export class SyncCommand {
);
}
// Same guards archive runs before it writes a spec, in the same order.
// 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) {
const changeDir = path.join(changesDir, report.change);
if (!skipValidation) {
await this.assertDeltaSpecsValid(report.change, changeDir, root, json);
}
await this.assertTasksComplete(report.change, changesDir, options, root, json);
}
if (!json) {
for (const { report } of evaluations) {
for (const warning of report.warnings) {
console.log(chalk.yellow(`⚠️ Warning: ${warning}`));
}
}
}
// Every rebuilt spec is validated before any of them is written, so a late
// failure leaves every target unchanged rather than half the tree folded.
if (!skipValidation) {
const validator = new Validator();
for (const { report, writes } of evaluations) {
for (const write of 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 '${report.change}': ${details}. No files were changed.`,
`Run ${withStoreFlag(root, `openspec validate ${write.update.id}`)} after fixing the change deltas.`
);
}
}
}
const pending = evaluations.flatMap(({ writes }) => writes);
// Sync applies one change across several capabilities, so a failure part
// way through the loop would leave some main specs folded and others not.
// Re-running would finish the job - the fold is idempotent - but a tree
// nobody asked for is not a state to hand back, so the previous bytes are
// restored instead. Simpler than archive's equivalent because sync only
// ever writes: there is no retirement to undo and no directory move to
// unwind.
const snapshots = await captureTargets(pending.map((write) => write.update.target));
// 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<string, string>();
let wroteAny = false;
try {
for (const write of pending) {
await writeUpdatedSpec(write.update, write.rebuilt, write.counts, {
silent: json,
...(isStoreSelectedRoot(root) ? { displayPath: write.update.target } : {}),
});
wroteAny = true;
totals.added += write.counts.added;
totals.modified += write.counts.modified;
totals.removed += write.counts.removed;
totals.renamed += write.counts.renamed;
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);
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: ${
@@ -614,29 +771,31 @@ export class SyncCommand {
);
}
// Re-evaluate rather than assume. Folding is idempotent for a single
// change, but two shipped changes can disagree about the same requirement -
// one adding what the other removes - and a fold that does not settle would
// otherwise report success while `--check` immediately after went red. The
// merge builder catches the destructive shapes of that disagreement 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
// is better than looping on it.
// 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),
root.specsDir
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',
`Specs were written, but these changes still report unfolded deltas: ` +
`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.`,
`requirement in ways that cannot both hold.${
restoreFailure ? ` ${restoreFailure}` : ' The main specs were left unchanged.'
}`,
'Reconcile the conflicting deltas, then rerun.'
);
}
@@ -682,35 +841,6 @@ export class SyncCommand {
};
}
/**
* Archive's delta validation, restricted to what sync needs. Sync only ever
* runs on a change that has deltas to fold, so the `skip_specs` reconciliation
* archive performs (a change declaring it has no deltas, but carrying files)
* has nothing to decide here - `findSpecUpdates` already found the files.
*/
private async assertDeltaSpecsValid(
changeName: string,
changeDir: string,
root: ResolvedOpenSpecRoot,
json: boolean
): Promise<void> {
const report = await new Validator().validateChangeDeltaSpecs(changeDir);
if (report.valid) return;
if (!json) {
console.log(chalk.red(`\nValidation errors in change delta specs:`));
for (const issue of report.issues) {
if (issue.level === 'ERROR') console.log(chalk.red(` ✗ ${issue.message}`));
else if (issue.level === 'WARNING') console.log(chalk.yellow(` ⚠ ${issue.message}`));
}
}
throw new SyncBlockedError(
'sync_validation_failed',
`Validation failed for change '${changeName}'. No files were changed.`,
`Run ${withStoreFlag(root, `openspec validate ${changeName}`)} for details, fix the errors, or rerun with --no-validate.`
);
}
/**
* Folding a change whose tasks are unfinished writes requirements into
* `specs/` that nothing implements yet - the exact drift the gate exists to
@@ -757,51 +887,81 @@ interface TargetSnapshot {
content?: Buffer;
}
/** Read the current bytes of each target so a failed write can be undone. */
async function captureTargets(targets: string[]): Promise<TargetSnapshot[]> {
return Promise.all(
targets.map(async (target) => {
try {
return { target, content: await fs.readFile(target) };
} catch (error) {
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return { target };
throw error;
}
})
);
/** Read the current bytes of a target so a failed write can be undone. */
async function captureTarget(target: string): Promise<TargetSnapshot> {
try {
return { target, content: await fs.readFile(target) };
} catch (error) {
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return { target };
throw error;
}
}
/**
* Put every captured target back the way it was, in reverse order.
* Put every target that this run actually changed back the way it was, in
* reverse order.
*
* Returns a sentence naming what could not be restored, or undefined when the
* 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[]
snapshots: TargetSnapshot[],
wrote: Map<string, string>
): Promise<string | undefined> {
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, so the rollback is removing
// whatever was created. A missing file is already the desired state.
await fs.unlink(snapshot.target).catch((error) => {
if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error;
});
} else {
// 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);
// 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);
}
}
return failed.length > 0
? `These specs could not be restored and may hold partly folded content: ${failed.join(', ')}.`
: undefined;
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 } {
test/core/sync.test.ts
+405 -5
View File
@@ -5,6 +5,10 @@ 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(),
@@ -42,6 +46,7 @@ describe('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');
@@ -56,10 +61,15 @@ describe('SyncCommand', () => {
delta?: string;
tasks?: string;
metadata?: string;
/** Capability id relative to `specs/`, e.g. `platform/session-layout`. */
capability?: string;
} = {}
): Promise<string> {
const dir = path.join(changesDir(), name);
await fs.mkdir(path.join(dir, 'specs', 'api'), { recursive: true });
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 ??
@@ -75,7 +85,7 @@ describe('SyncCommand', () => {
options.tasks ?? '## 1. Work\n- [x] 1.1 Done\n'
);
await fs.writeFile(
path.join(dir, 'specs', 'api', 'spec.md'),
path.join(dir, 'specs', ...capability.split('/'), 'spec.md'),
options.delta ?? ADDED_DELTA
);
return dir;
@@ -86,7 +96,12 @@ describe('SyncCommand', () => {
}
beforeEach(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-sync-test-'));
// 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');
@@ -105,6 +120,10 @@ describe('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;
@@ -216,6 +235,110 @@ describe('SyncCommand', () => {
).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 });
@@ -232,6 +355,133 @@ describe('SyncCommand', () => {
});
});
describe('folding several changes in one run', () => {
/** A second change adding a different requirement to the SAME capability. */
async function secondChange(name: string): Promise<void> {
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', {
@@ -266,7 +516,7 @@ describe('SyncCommand', () => {
});
await expect(sync.execute('add-tracing', { yes: true })).rejects.toThrow(
/Validation failed/
/must include at least one scenario/
);
expect(await mainSpec()).toBe(MAIN_SPEC);
});
@@ -305,6 +555,54 @@ describe('SyncCommand', () => {
});
});
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', {
@@ -377,7 +675,7 @@ describe('SyncCommand', () => {
await expect(
sync.execute('add-tracing', { ship: true })
).rejects.toThrow(/Validation failed/);
).rejects.toThrow(/must include at least one scenario/);
expect(readChangeStatus(dir).status).toBe('proposed');
});
@@ -406,6 +704,19 @@ describe('SyncCommand', () => {
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');
@@ -450,6 +761,25 @@ describe('SyncCommand', () => {
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',
@@ -506,6 +836,47 @@ describe('SyncCommand', () => {
});
});
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');
@@ -567,6 +938,35 @@ describe('readChangeStatus / writeChangeStatus', () => {
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/