diff --git a/.changeset/create-main-spec-for-new-capability.md b/.changeset/create-main-spec-for-new-capability.md new file mode 100644 index 0000000000..d578818462 --- /dev/null +++ b/.changeset/create-main-spec-for-new-capability.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Agent-driven archive and sync workflows now create a missing main spec from `ADDED` requirements instead of treating it as already synced. They block sync rather than inventing `MODIFIED` or `RENAMED` requirements or writing an empty spec for a `REMOVED`-only delta, while preserving the user's explicit choice to archive without syncing. A REMOVED-only delta with `retire_capabilities: true` remains already synced when its main spec is gone. Fixes #1222 and #1264. diff --git a/docs-lab/reference/glossary.md b/docs-lab/reference/glossary.md index acd4b503b7..10dd5aba56 100644 --- a/docs-lab/reference/glossary.md +++ b/docs-lab/reference/glossary.md @@ -19,7 +19,7 @@ OpenSpec reuses words that mean something else in git, CI, and agent tooling. Ea | **Fast-forward** | Create a change proposal with every planning artifact in one pass, ready to implement. Skill: `openspec-ff-change`. Not a git fast-forward. | [Skills](skills.md) | | **Legacy workflow** | The pre-OPSX `/openspec:*` commands. | [Migration](../help/legacy/migration.md) | | **Loop** | The cycle a change proposal moves through: explore, propose, review, apply, archive. | [Quickstart](../start/quickstart.md) | -| **Main specs** | The `openspec/specs/` tree: the current, agreed behavior of your system. Archiving merges deltas into it. | [Concepts](../guides/concepts.md) | +| **Main specs** | The `openspec/specs/` tree: the current, agreed behavior of your system. Archiving merges deltas into it. A capability with no spec yet gets one from its `ADDED` requirements. | [Concepts](../guides/concepts.md) | | **OpenSpec root** | The `openspec/` tree a command resolves to and operates on: your repo's, or a store's. | [Stores](../multi-repo/stores.md#where-artifacts-get-created-when-using-stores) | | **OPSX** | The current OpenSpec workflow system, and the command prefix it installs (`/opsx:`). | [Architecture](architecture/index.md) | | **Profile** | Which workflows init installs: `core` or `custom`. | [Profiles](../customize/profiles.md) | diff --git a/openspec/specs/opsx-archive-skill/spec.md b/openspec/specs/opsx-archive-skill/spec.md index 2c76461e54..26c1c66f60 100644 --- a/openspec/specs/opsx-archive-skill/spec.md +++ b/openspec/specs/opsx-archive-skill/spec.md @@ -82,6 +82,52 @@ The skill SHALL prompt to sync delta specs before archiving if specs exist. - **AND** stop without archiving if the sync fails or any capability does not verify - **AND** archive only after verification passes, or when the user explicitly chose to archive without syncing or to archive already-synced specs +#### Scenario: Applicable ADDED delta whose main spec does not exist yet + +- **WHEN** agent compares a delta spec against its main spec at `openspec/specs//spec.md` +- **AND** that main spec does not exist yet +- **AND** the delta has `## ADDED Requirements` +- **AND** the delta has no `## MODIFIED Requirements` or `## RENAMED Requirements` +- **THEN** count that capability as needing sync rather than as already synced +- **AND** name it in the summary as a main spec the sync will create +- **AND** never treat the missing main spec as nothing to apply +- **AND** if the delta also has `## REMOVED Requirements`, warn that they will be ignored because there is no main spec to remove them from +- **AND** create the main spec from only the delta's `## ADDED Requirements` + +#### Scenario: Unsupported delta operation whose main spec does not exist yet + +- **WHEN** a delta targets a capability whose main spec does not exist yet +- **AND** the delta has `## MODIFIED Requirements` or `## RENAMED Requirements` +- **THEN** report that only ADDED requirements can create a new main spec +- **AND** mark the capability as sync-blocked without writing a main spec + +#### Scenario: Explicitly retired capability whose main spec is missing + +- **WHEN** a delta contains only `## REMOVED Requirements` and its main spec is missing +- **AND** the change's `.openspec.yaml` declares `retire_capabilities: true` +- **THEN** count that capability as already synced and report that it is already retired +- **AND** warn that there is nothing left to remove and do not recreate the main spec +- **AND** apply the same rule when verifying a completed sync, so retiring a capability does not block archiving + +#### Scenario: Nothing to put in a missing main spec without a declared retirement + +- **WHEN** a delta targets a capability whose main spec does not exist yet +- **AND** the delta has no `## ADDED Requirements` +- **AND** it is not a REMOVED-only delta with `retire_capabilities: true` +- **THEN** report that no sync is possible +- **AND** if the delta has only `## REMOVED Requirements`, warn that there is no main spec to remove them from and leave the main-spec tree unchanged +- **AND** mark the capability as sync-blocked, since the verification pass would re-read the same missing spec + +#### Scenario: Sync-blocked capability during archive assessment + +- **WHEN** any capability is sync-blocked during the initial assessment +- **THEN** assess the remaining capabilities and summarize the blockers before prompting +- **AND** offer only "Archive without syncing" and "Cancel" +- **AND** archive without writing main specs only if the user explicitly chooses "Archive without syncing" +- **AND** stop without archiving if the user cancels +- **AND** do not start any sync while a capability is blocked, even if other capabilities could sync +- **AND** a failed sync or post-sync verification still stops without archiving; do not silently fall back to skipping sync + #### Scenario: No delta specs - **WHEN** agent checks for delta specs diff --git a/openspec/specs/specs-sync-skill/spec.md b/openspec/specs/specs-sync-skill/spec.md index 3d14288802..abb99ce16d 100644 --- a/openspec/specs/specs-sync-skill/spec.md +++ b/openspec/specs/specs-sync-skill/spec.md @@ -71,10 +71,27 @@ The agent SHALL reconcile main specs with delta specs using the delta operation #### Scenario: New capability spec - **WHEN** delta spec exists for a capability not in main specs +- **AND** it has ADDED requirements and no MODIFIED or RENAMED requirements - **THEN** create new main spec file at `openspec/specs//spec.md`, preserving the delta's path relative to `specs/` - **AND** copy the delta's `## Purpose` body into it when the delta has one, matching what `openspec archive` does - **AND** write a brief TBD placeholder Purpose only when the delta has none +#### Scenario: MODIFIED or RENAMED against a capability with no main spec +- **WHEN** delta contains `## MODIFIED Requirements` or `## RENAMED Requirements` +- **AND** the capability has no main spec yet +- **THEN** stop the sync for that capability and report that only ADDED requirements are allowed for a new spec, matching what `openspec archive` does +- **AND** never invent the missing requirement +- **AND** skip any `## REMOVED Requirements` with a warning, since there is nothing to remove + +#### Scenario: Nothing to put in a new spec +- **WHEN** a delta targets a capability with no main spec +- **AND** the delta has no `## ADDED Requirements` to seed it with +- **THEN** create no main spec and leave the specs directory untouched +- **AND** for a REMOVED-only delta with `retire_capabilities: true` in the change's `.openspec.yaml`, report the capability as already retired and continue without recreating it +- **AND** without that marker, report a REMOVED-only sync as blocked, matching `openspec archive`, which aborts with `Spec must have at least one requirement` +- **AND** report an empty delta as blocked because it has no operations to sync +- **AND** never write an empty `## Requirements` section + #### Scenario: Merged main spec keeps canonical structure - **WHEN** the agent writes a main spec during sync - **THEN** every requirement lives under a single `## Requirements` section diff --git a/skills/openspec-archive-change/SKILL.md b/skills/openspec-archive-change/SKILL.md index 5f34ed53a7..62ca3fbfac 100644 --- a/skills/openspec-archive-change/SKILL.md +++ b/skills/openspec-archive-change/SKILL.md @@ -94,17 +94,23 @@ Archive a completed change in the experimental workflow. **If delta specs exist:** - Compare each delta spec with its corresponding main spec at `/openspec/specs//spec.md` (use the store-aware `planningHome.root` from step 2, not a hardcoded repo path) + - A missing main spec is **not automatically** "already synced". For a new capability, the main spec is an *output* of the sync, not an input: + - If the delta has MODIFIED or RENAMED requirements, report that only ADDED requirements can create a new main spec and mark that capability as sync-blocked. Never invent a requirement that has no current version. + - Otherwise, if the delta has only REMOVED requirements and the change's `.openspec.yaml` declares `retire_capabilities: true`, the capability is already retired: count it as already synced, warn that there is nothing left to remove, and do not recreate the main spec. Apply this rule both now and when verifying a completed sync. + - Otherwise, if the delta has no ADDED requirements, report that no sync is possible and mark that capability as sync-blocked. For a REMOVED-only delta, warn that there is no main spec to remove from and leave the main-spec tree unchanged. `openspec archive` refuses the unmarked REMOVED-only case with `Spec must have at least one requirement`. + - Otherwise, count the capability as needing sync and name it in the summary (`: new main spec will be created`). If the delta also has REMOVED requirements, warn that they will be ignored because there is no main spec to remove from. The sync creates the main spec from only the delta's ADDED requirements, exactly as `openspec archive` does. - Determine what changes would be applied (adds, modifications, removals, renames) - - Show a combined summary before prompting + - Continue assessing the remaining capabilities even when one is sync-blocked. Show a combined summary before prompting. **Prompt options:** - - If changes needed: "Sync now (recommended)", "Archive without syncing" - - If already synced: "Archive now", "Sync anyway", "Cancel" + - If any capability is sync-blocked: explain why and offer only "Archive without syncing", "Cancel" + - Otherwise, if changes needed: "Sync now (recommended)", "Archive without syncing" + - Otherwise, if already synced: "Archive now", "Sync anyway", "Cancel" Route on the answer: - "Cancel" — stop, do not archive - "Archive without syncing" or "Archive now" — proceed to archive - - "Sync now" or "Sync anyway" — sync, then verify (below) + - "Sync now" or "Sync anyway" — sync, then verify (below). Do not start any sync while a capability is sync-blocked; explain the blocker and repeat the available choices. - Anything else — ask again rather than archiving Before a selected sync writes any main spec, run @@ -118,7 +124,7 @@ Archive a completed change in the experimental workflow. Then run the `openspec-sync-specs` workflow inline (agent-driven intelligent merge) for change '', passing the delta spec analysis and the fetched specs-rule snapshot from above, and wait for it to finish. The inline sync must reuse that snapshot without fetching `specs` instructions again. Do not delegate it to a background task — step 5 would move `changeRoot` out from under a sync that is still reading it, leaving the change archived and the main specs never updated. If your agent can only run it by delegation, delegate synchronously and wait for the result. - Then re-run the comparison from the top of this step against every capability that has a delta spec in `artifactPaths.specs.existingOutputPaths` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: + Then re-run the comparison from the top of this step, including the explicitly retired, missing-spec case, against every capability that has a delta spec in `artifactPaths.specs.existingOutputPaths` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: - ADDED requirements present - MODIFIED requirements carrying the scenario and description changes named in the delta, with their other scenarios intact - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving `## Requirements` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match diff --git a/skills/openspec-sync-specs/SKILL.md b/skills/openspec-sync-specs/SKILL.md index d12d56b857..1babf3b613 100644 --- a/skills/openspec-sync-specs/SKILL.md +++ b/skills/openspec-sync-specs/SKILL.md @@ -95,6 +95,13 @@ This is an **agent-driven** operation - you will read delta specs and directly e b. **Read the main spec** at `/openspec/specs//spec.md` (may not exist yet) + **If it does not exist yet** (a new capability), match what `openspec archive` does: + only ADDED requirements may be applied - step d creates the spec from them. + MODIFIED and RENAMED have no requirement to act on, so stop the sync for that + capability and report that its main spec does not exist and only ADDED is allowed + for a new spec; never invent the missing requirement. REMOVED has nothing to + remove - skip it and warn. + c. **Apply changes intelligently**: **ADDED Requirements:** @@ -142,6 +149,14 @@ This is an **agent-driven** operation - you will read delta specs and directly e (this is what `openspec archive` does; it warns and moves on) d. **Create new main spec** if capability doesn't exist yet: + - Only when the delta has ADDED requirements to put in it and no MODIFIED or + RENAMED requirements blocked this capability in step b. Otherwise create nothing + and leave the specs directory untouched. For a REMOVED-only delta, if the change's + `.openspec.yaml` declares `retire_capabilities: true`, report it as already retired + and continue without recreating the spec. Without that marker, report the sync as blocked: + `openspec archive` rejects it with `Spec must have at least one requirement`. + An empty delta has no operations to sync; report it as blocked too. + Never write an empty `## Requirements` section. - Create `/openspec/specs//spec.md` - Add Purpose section: copy the delta's `## Purpose` body verbatim when it has one (this is what `openspec archive` does); only write a brief TBD placeholder when it does not diff --git a/src/core/templates/workflows/archive-change.ts b/src/core/templates/workflows/archive-change.ts index 2dae74d436..7c7511fc27 100644 --- a/src/core/templates/workflows/archive-change.ts +++ b/src/core/templates/workflows/archive-change.ts @@ -96,17 +96,23 @@ ${STORE_SELECTION_GUIDANCE} **If delta specs exist:** - Compare each delta spec with its corresponding main spec at \`/openspec/specs//spec.md\` (use the store-aware \`planningHome.root\` from step 2, not a hardcoded repo path) + - A missing main spec is **not automatically** "already synced". For a new capability, the main spec is an *output* of the sync, not an input: + - If the delta has MODIFIED or RENAMED requirements, report that only ADDED requirements can create a new main spec and mark that capability as sync-blocked. Never invent a requirement that has no current version. + - Otherwise, if the delta has only REMOVED requirements and the change's \`.openspec.yaml\` declares \`retire_capabilities: true\`, the capability is already retired: count it as already synced, warn that there is nothing left to remove, and do not recreate the main spec. Apply this rule both now and when verifying a completed sync. + - Otherwise, if the delta has no ADDED requirements, report that no sync is possible and mark that capability as sync-blocked. For a REMOVED-only delta, warn that there is no main spec to remove from and leave the main-spec tree unchanged. \`openspec archive\` refuses the unmarked REMOVED-only case with \`Spec must have at least one requirement\`. + - Otherwise, count the capability as needing sync and name it in the summary (\`: new main spec will be created\`). If the delta also has REMOVED requirements, warn that they will be ignored because there is no main spec to remove from. The sync creates the main spec from only the delta's ADDED requirements, exactly as \`openspec archive\` does. - Determine what changes would be applied (adds, modifications, removals, renames) - - Show a combined summary before prompting + - Continue assessing the remaining capabilities even when one is sync-blocked. Show a combined summary before prompting. **Prompt options:** - - If changes needed: "Sync now (recommended)", "Archive without syncing" - - If already synced: "Archive now", "Sync anyway", "Cancel" + - If any capability is sync-blocked: explain why and offer only "Archive without syncing", "Cancel" + - Otherwise, if changes needed: "Sync now (recommended)", "Archive without syncing" + - Otherwise, if already synced: "Archive now", "Sync anyway", "Cancel" Route on the answer: - "Cancel" — stop, do not archive - "Archive without syncing" or "Archive now" — proceed to archive - - "Sync now" or "Sync anyway" — sync, then verify (below) + - "Sync now" or "Sync anyway" — sync, then verify (below). Do not start any sync while a capability is sync-blocked; explain the blocker and repeat the available choices. - Anything else — ask again rather than archiving Before a selected sync writes any main spec, run @@ -120,7 +126,7 @@ ${STORE_SELECTION_GUIDANCE} Then run the \`openspec-sync-specs\` workflow inline (agent-driven intelligent merge) for change '', passing the delta spec analysis and the fetched specs-rule snapshot from above, and wait for it to finish. The inline sync must reuse that snapshot without fetching \`specs\` instructions again. Do not delegate it to a background task — step 5 would move \`changeRoot\` out from under a sync that is still reading it, leaving the change archived and the main specs never updated. If your agent can only run it by delegation, delegate synchronously and wait for the result. - Then re-run the comparison from the top of this step against every capability that has a delta spec in \`artifactPaths.specs.existingOutputPaths\` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: + Then re-run the comparison from the top of this step, including the explicitly retired, missing-spec case, against every capability that has a delta spec in \`artifactPaths.specs.existingOutputPaths\` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: - ADDED requirements present - MODIFIED requirements carrying the scenario and description changes named in the delta, with their other scenarios intact - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving \`## Requirements\` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match @@ -278,17 +284,23 @@ ${STORE_SELECTION_GUIDANCE} **If delta specs exist:** - Compare each delta spec with its corresponding main spec at \`/openspec/specs//spec.md\` (use the store-aware \`planningHome.root\` from step 2, not a hardcoded repo path) + - A missing main spec is **not automatically** "already synced". For a new capability, the main spec is an *output* of the sync, not an input: + - If the delta has MODIFIED or RENAMED requirements, report that only ADDED requirements can create a new main spec and mark that capability as sync-blocked. Never invent a requirement that has no current version. + - Otherwise, if the delta has only REMOVED requirements and the change's \`.openspec.yaml\` declares \`retire_capabilities: true\`, the capability is already retired: count it as already synced, warn that there is nothing left to remove, and do not recreate the main spec. Apply this rule both now and when verifying a completed sync. + - Otherwise, if the delta has no ADDED requirements, report that no sync is possible and mark that capability as sync-blocked. For a REMOVED-only delta, warn that there is no main spec to remove from and leave the main-spec tree unchanged. \`openspec archive\` refuses the unmarked REMOVED-only case with \`Spec must have at least one requirement\`. + - Otherwise, count the capability as needing sync and name it in the summary (\`: new main spec will be created\`). If the delta also has REMOVED requirements, warn that they will be ignored because there is no main spec to remove from. The sync creates the main spec from only the delta's ADDED requirements, exactly as \`openspec archive\` does. - Determine what changes would be applied (adds, modifications, removals, renames) - - Show a combined summary before prompting + - Continue assessing the remaining capabilities even when one is sync-blocked. Show a combined summary before prompting. **Prompt options:** - - If changes needed: "Sync now (recommended)", "Archive without syncing" - - If already synced: "Archive now", "Sync anyway", "Cancel" + - If any capability is sync-blocked: explain why and offer only "Archive without syncing", "Cancel" + - Otherwise, if changes needed: "Sync now (recommended)", "Archive without syncing" + - Otherwise, if already synced: "Archive now", "Sync anyway", "Cancel" Route on the answer: - "Cancel" — stop, do not archive - "Archive without syncing" or "Archive now" — proceed to archive - - "Sync now" or "Sync anyway" — sync, then verify (below) + - "Sync now" or "Sync anyway" — sync, then verify (below). Do not start any sync while a capability is sync-blocked; explain the blocker and repeat the available choices. - Anything else — ask again rather than archiving Before a selected sync writes any main spec, run @@ -302,7 +314,7 @@ ${STORE_SELECTION_GUIDANCE} Then run the \`/opsx:sync\` workflow inline (agent-driven intelligent merge) for change '', passing the delta spec analysis and the fetched specs-rule snapshot from above, and wait for it to finish. The inline sync must reuse that snapshot without fetching \`specs\` instructions again. Do not delegate it to a background task — step 5 would move \`changeRoot\` out from under a sync that is still reading it, leaving the change archived and the main specs never updated. If your agent can only run it by delegation, delegate synchronously and wait for the result. - Then re-run the comparison from the top of this step against every capability that has a delta spec in \`artifactPaths.specs.existingOutputPaths\` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: + Then re-run the comparison from the top of this step, including the explicitly retired, missing-spec case, against every capability that has a delta spec in \`artifactPaths.specs.existingOutputPaths\` — not only the ones the sync reports it touched. A successful sync leaves nothing left to apply, so each capability must now read as already synced: - ADDED requirements present - MODIFIED requirements carrying the scenario and description changes named in the delta, with their other scenarios intact - REMOVED requirements gone — and where this sync retired a capability (removed its last requirement, leaving \`## Requirements\` empty), its main spec deleted rather than left empty; a spec the sync deliberately kept and reported is also a match diff --git a/src/core/templates/workflows/sync-specs.ts b/src/core/templates/workflows/sync-specs.ts index bedbaa7164..3f9e7aa362 100644 --- a/src/core/templates/workflows/sync-specs.ts +++ b/src/core/templates/workflows/sync-specs.ts @@ -97,6 +97,13 @@ ${STORE_SELECTION_GUIDANCE} b. **Read the main spec** at \`/openspec/specs//spec.md\` (may not exist yet) + **If it does not exist yet** (a new capability), match what \`openspec archive\` does: + only ADDED requirements may be applied - step d creates the spec from them. + MODIFIED and RENAMED have no requirement to act on, so stop the sync for that + capability and report that its main spec does not exist and only ADDED is allowed + for a new spec; never invent the missing requirement. REMOVED has nothing to + remove - skip it and warn. + c. **Apply changes intelligently**: **ADDED Requirements:** @@ -144,6 +151,14 @@ ${STORE_SELECTION_GUIDANCE} (this is what \`openspec archive\` does; it warns and moves on) d. **Create new main spec** if capability doesn't exist yet: + - Only when the delta has ADDED requirements to put in it and no MODIFIED or + RENAMED requirements blocked this capability in step b. Otherwise create nothing + and leave the specs directory untouched. For a REMOVED-only delta, if the change's + \`.openspec.yaml\` declares \`retire_capabilities: true\`, report it as already retired + and continue without recreating the spec. Without that marker, report the sync as blocked: + \`openspec archive\` rejects it with \`Spec must have at least one requirement\`. + An empty delta has no operations to sync; report it as blocked too. + Never write an empty \`## Requirements\` section. - Create \`/openspec/specs//spec.md\` - Add Purpose section: copy the delta's \`## Purpose\` body verbatim when it has one (this is what \`openspec archive\` does); only write a brief TBD placeholder when it does not @@ -359,6 +374,13 @@ ${STORE_SELECTION_GUIDANCE} b. **Read the main spec** at \`/openspec/specs//spec.md\` (may not exist yet) + **If it does not exist yet** (a new capability), match what \`openspec archive\` does: + only ADDED requirements may be applied - step d creates the spec from them. + MODIFIED and RENAMED have no requirement to act on, so stop the sync for that + capability and report that its main spec does not exist and only ADDED is allowed + for a new spec; never invent the missing requirement. REMOVED has nothing to + remove - skip it and warn. + c. **Apply changes intelligently**: **ADDED Requirements:** @@ -406,6 +428,14 @@ ${STORE_SELECTION_GUIDANCE} (this is what \`openspec archive\` does; it warns and moves on) d. **Create new main spec** if capability doesn't exist yet: + - Only when the delta has ADDED requirements to put in it and no MODIFIED or + RENAMED requirements blocked this capability in step b. Otherwise create nothing + and leave the specs directory untouched. For a REMOVED-only delta, if the change's + \`.openspec.yaml\` declares \`retire_capabilities: true\`, report it as already retired + and continue without recreating the spec. Without that marker, report the sync as blocked: + \`openspec archive\` rejects it with \`Spec must have at least one requirement\`. + An empty delta has no operations to sync; report it as blocked too. + Never write an empty \`## Requirements\` section. - Create \`/openspec/specs//spec.md\` - Add Purpose section: copy the delta's \`## Purpose\` body verbatim when it has one (this is what \`openspec archive\` does); only write a brief TBD placeholder when it does not diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 1033fa1868..fa7df0e868 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -2383,7 +2383,7 @@ The system will log all events. } }); - it('should proceed with archive when user declines spec updates', async () => { + it.each(['legacy', 'MODIFIED', 'RENAMED', 'REMOVED'])('archives when the user declines %s sync', async (operation) => { const { confirmPrompt: confirm } = await import('../../src/utils/interactive.js'); const mockConfirm = confirm as unknown as ReturnType; @@ -2392,8 +2392,21 @@ The system will log all events. const changeSpecDir = path.join(changeDir, 'specs', 'test-capability'); await fs.mkdir(changeSpecDir, { recursive: true }); - // Create valid spec in change - const specContent = `# Test Capability Spec + // These deltas cannot build a new main spec. Declining sync must still + // archive them without creating one, including the legacy no-operations case. + const specContent = operation === 'RENAMED' + ? '## RENAMED Requirements\n- FROM: `### Requirement: Old name`\n- TO: `### Requirement: New name`\n' + : operation !== 'legacy' + ? `## ${operation} Requirements + +### Requirement: Test capability +The system SHALL provide test capability. + +#### Scenario: Basic test +- **WHEN** an action occurs +- **THEN** the expected result happens +` + : `# Test Capability Spec ## Purpose This is a test capability specification. @@ -2434,6 +2447,10 @@ Then expected result happens`; const archives = await fs.readdir(archiveDir); expect(archives.length).toBe(1); expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`)); + expect(process.exitCode).not.toBe(1); + await expect( + fs.readFile(path.join(archiveDir, archives[0], 'specs', 'test-capability', 'spec.md'), 'utf-8') + ).resolves.toBe(specContent); }); it('warns about absorbed content before asking to apply the destructive spec update', async () => { @@ -5383,22 +5400,35 @@ The system SHALL do the thing differently. ); }); - it('archives a REMOVED-only delta whose main spec was already deleted', async () => { + it.each([true, false])('handles an already-deleted main spec with retirement declared: %s', async (declareRetirement) => { // The issue's second dead end: pre-deleting the spec made the delta look // like a create, which landed on an empty spec and failed the same way. const changeName = 'retire-already-gone'; - await createChange(changeName, 'legacy-layer', REMOVE_ALL); + const changeDir = await createChange(changeName, 'legacy-layer', REMOVE_ALL, { declareRetirement }); await archiveCommand.execute(changeName, { yes: true }); - expect(process.exitCode).not.toBe(1); // Nothing was recreated. await expect( fs.access(path.join(tempDir, 'openspec', 'specs', 'legacy-layer')) ).rejects.toThrow(); - await expect( - fs.access(path.join(tempDir, 'openspec', 'changes', changeName)) - ).rejects.toThrow(); + if (declareRetirement) { + expect(process.exitCode).not.toBe(1); + await expect(fs.access(changeDir)).rejects.toThrow(); + const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive'); + const [archivedName] = await fs.readdir(archiveDir); + await expect( + fs.readFile(path.join(archiveDir, archivedName, 'specs', 'legacy-layer', 'spec.md'), 'utf-8') + ).resolves.toBe(REMOVE_ALL); + } else { + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining(VALIDATION_MESSAGES.SPEC_NO_REQUIREMENTS) + ); + await expect(fs.readFile(path.join(changeDir, 'specs', 'legacy-layer', 'spec.md'), 'utf-8')) + .resolves.toBe(REMOVE_ALL); + expect(await fs.readdir(path.join(tempDir, 'openspec', 'changes', 'archive'))).toEqual([]); + } }); // The requirement-block count and the validator do NOT agree on what a diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 4d2e717cae..1b11e06c26 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -43,18 +43,18 @@ const EXPECTED_FUNCTION_HASHES: Record = { getContinueChangeSkillTemplate: '012136f6411a99c8fa228e2f9444cb64b0a89e0f56fdeac2fe03b2f5bee0c5d7', getApplyChangeSkillTemplate: 'd1e7d5ceb85193c0964057dbb88e9651526754bd33f84020e2440ff0621d5dbb', getFfChangeSkillTemplate: 'efa6a70c111b18b61a7720250b9622afa9a212fb64edf609cf80e2182a9bdf8c', - getSyncSpecsSkillTemplate: 'b099e2ff31859c9b10d928066e662524f9aad9ecf2be12fceacb732d718c4146', + getSyncSpecsSkillTemplate: '271825029d9fa83c19ceb07e6f1ca3c10a17cf0822899d7709180a7487b9ae29', getOnboardSkillTemplate: '3a836faae463d88c289a1c129cb7ee556a563b7e53e1a52a4711ff152a3b51f7', getOpsxExploreCommandTemplate: '8046003e97d885a86ed392d4fb522bb78544a02872b042e51347a5021cc10523', getOpsxNewCommandTemplate: 'f2d30e569798a4c92ba932859d6ba4e0ad10e18feccbade1cfee0957597b3463', getOpsxContinueCommandTemplate: 'e50e50266efa1b8e64ff9b6274ee8254f0a240d6adc1b862d126e2f1c9d3a559', getOpsxApplyCommandTemplate: 'e3579ac78f2e2c75fa3d3a7ac7dc3e49c395e96f7323398f0f041d94f8de9bb0', getOpsxFfCommandTemplate: '21132fc9c6d3b3ab2d2295d6bbd72d1e0052eb35ea1be0258c8b1ab3e200c4db', - getArchiveChangeSkillTemplate: '56bfada1a5f35a127791b70de9d428a75b5aedd1584d6c9803a1ecb1fd1b4a23', + getArchiveChangeSkillTemplate: '80576b4b51a5cdd5ab2e7fb4e8616619e8a100b2aff17eef5a7c004865e07f6f', getBulkArchiveChangeSkillTemplate: '93875998cade5322d95b43299fba794bc1da754e917dd63a770406386a6d295d', - getOpsxSyncCommandTemplate: '0d2427efb79986e8fff3f96bd075a739c80d45eb29159fae717e950030da8202', + getOpsxSyncCommandTemplate: 'b4b2bbcfa7e3709bc7cfdfefed7d4c5921878d03ba483e6c97bd33eb94cb583c', getVerifyChangeSkillTemplate: '223b7ffd99299a7d430e13092b9a0a3421b39f0d3217232f46c39d79b5f619ff', - getOpsxArchiveCommandTemplate: '9f973c819b11620985b03322945f0e0a92a02a2ef455b94e74482f5e6292ac5d', + getOpsxArchiveCommandTemplate: '320ab6015bc310b9d1974fd6dcb06cfae1b93bf7716b8a86a07beaab956db723', getOpsxOnboardCommandTemplate: 'ee99aa99252c602720fbb8c63fb3ac438a5bd4e952fd961ddf1ae956cbfc2c8f', getOpsxBulkArchiveCommandTemplate: '9fa8cdebe2f5667ebfc37bdc023396762c59d5b038c771dac2d8fd2c19e2627b', getOpsxVerifyCommandTemplate: '1efcf7eff0671f48e9d9420f50865c563dd3079ee60f8c380bb7a90dd0102696', @@ -71,8 +71,8 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-continue-change': 'bb6194a16c54891cdb253678e8f70ce53b2af86735243980f366ce551d37e42e', 'openspec-apply-change': '81ea96d9fa6ec8536cd23c1fe561ed28e1cc1cad0a8ceb700588e08974cc0e49', 'openspec-ff-change': '31355250514bce51b16ff37ee2b833bc9d475cd0dbd4b1f68fe2041694575623', - 'openspec-sync-specs': 'd933d8856584d6c1253de91e652e7aee9e85c77ad4d3531f6476f79d84e6e5e8', - 'openspec-archive-change': '7c65053d674ba4e1e20e2bf73ba7e5a7f94baef2eaa9b33cee48d4cadea51b7a', + 'openspec-sync-specs': '0690c2290e74b3f7ce8f19d3204fb2d5630eb06290d1c6a79370026099758c98', + 'openspec-archive-change': '7a0a33ded7b47f941b12ff9e1847476b06548b580be2336c53599fa90f35b222', 'openspec-bulk-archive-change': '2039b9ecf6e64339dffe0e16272507a386d9fe326f419ff758315aa736fdd96c', 'openspec-verify-change': 'af9be013dcbe8c6d8f6d9ab10c893fbd03f4c62933c384d82f63894dd0ceb84f', 'openspec-onboard': 'f6f59476acaf5e4d65dbb180da4cef62432612f3cecf207d471a951295e2003a', @@ -481,6 +481,142 @@ describe('skill templates split parity', () => { } }); + it('requires sync to create a missing main spec from ADDED requirements (#1222, #1264)', () => { + // `openspec archive` creates the main spec from the delta's ADDED requirements + // when it does not exist yet (`buildUpdatedSpec`, specs-apply.ts). The agent + // workflow only told the agent to "compare each delta spec with its + // corresponding main spec", so a capability with no main spec compared against + // nothing, read as "already synced", and the change archived with the spec + // never written. Assertions are scoped to the sync-assessment step so they + // cannot pass on unrelated text elsewhere in the body. + const archiveVariants: Array<[string, string]> = [ + ['archive skill', generateSkillContent(getArchiveChangeSkillTemplate(), 'PARITY-BASELINE')], + ['archive opsx command', getOpsxArchiveCommandTemplate().content], + ]; + + for (const [variant, content] of archiveVariants) { + const start = content.indexOf('**Assess delta spec sync state**'); + const end = content.indexOf('**Perform the archive**'); + expect(start, variant).toBeGreaterThan(-1); + expect(end, variant).toBeGreaterThan(start); + const assessStep = content.slice(start, end); + + expect(assessStep, variant).toContain( + 'A missing main spec is **not automatically** "already synced"' + ); + expect(assessStep, variant).toContain('is an *output* of the sync, not an input'); + expect(assessStep, variant).toContain('If the delta has MODIFIED or RENAMED'); + expect(assessStep, variant).toContain('only ADDED requirements can create'); + expect(assessStep, variant).toContain('Never invent a requirement'); + expect(assessStep, variant).toContain('Otherwise, if the delta has no ADDED requirements'); + expect(assessStep, variant).toContain('report that no sync is possible'); + expect(assessStep, variant).toContain('For a REMOVED-only delta'); + expect(assessStep, variant).toContain('leave the main-spec tree unchanged'); + expect(assessStep, variant).toContain('mark that capability as sync-blocked'); + expect(assessStep, variant).toContain('Spec must have at least one requirement'); + expect(assessStep, variant).toContain('Otherwise, count the capability as needing sync'); + expect(assessStep, variant).toContain('If the delta also has REMOVED requirements'); + expect(assessStep, variant).toContain('warn that they will be ignored'); + expect(assessStep, variant).toContain( + "creates the main spec from only the delta's ADDED requirements" + ); + } + + // The sync itself must not invent a requirement that has no base to modify: + // the CLI throws "only ADDED requirements are allowed for new specs". + const syncVariants: Array<[string, string]> = [ + ['sync skill', getSyncSpecsSkillTemplate().instructions], + ['sync command', getOpsxSyncCommandTemplate().content], + ]; + + for (const [variant, content] of syncVariants) { + const start = content.indexOf('b. **Read the main spec**'); + const end = content.indexOf('c. **Apply changes intelligently**'); + expect(start, variant).toBeGreaterThan(-1); + expect(end, variant).toBeGreaterThan(start); + const readStep = content.slice(start, end); + + expect(readStep, variant).toContain('**If it does not exist yet** (a new capability)'); + expect(readStep, variant).toContain('only ADDED requirements may be applied'); + expect(readStep, variant).toContain('MODIFIED and RENAMED have no requirement to act on'); + expect(readStep, variant).toContain('never invent the missing requirement'); + expect(readStep, variant).toContain('REMOVED has nothing to'); + + // ...and the creation step must not then write the empty spec the CLI refuses: + // an unmarked REMOVED-only delta against a capability with no main spec aborts with + // "Spec must have at least one requirement" and leaves the tree untouched. + const createStart = content.indexOf("d. **Create new main spec**"); + const createEnd = content.indexOf('**Validate updated main specs**'); + expect(createStart, variant).toBeGreaterThan(-1); + expect(createEnd, variant).toBeGreaterThan(createStart); + const createStep = content.slice(createStart, createEnd); + + expect(createStep, variant).toContain( + 'Only when the delta has ADDED requirements to put in it' + ); + expect(createStep, variant).toContain('RENAMED requirements blocked this capability in step b'); + expect(createStep, variant).toContain('create nothing'); + expect(createStep, variant).toContain('Spec must have at least one requirement'); + expect(createStep, variant).toContain('Never write an empty'); + } + }); + + it('preserves explicit archive-without-sync when a missing target blocks sync', () => { + for (const content of [ + getArchiveChangeSkillTemplate().instructions, + getOpsxArchiveCommandTemplate().content, + ]) { + const assessment = content.slice( + content.indexOf('**If delta specs exist:**'), + content.indexOf('Before a selected sync writes any main spec') + ); + expect(assessment).not.toContain('stop instead of prompting to sync'); + expect(assessment).toContain('mark that capability as sync-blocked'); + expect(assessment).toContain('Continue assessing the remaining capabilities'); + expect(assessment).toContain( + 'If any capability is sync-blocked: explain why and offer only "Archive without syncing", "Cancel"' + ); + expect(assessment).toContain('Do not start any sync while a capability is sync-blocked'); + expect(assessment).toContain('"Archive without syncing" or "Archive now" — proceed to archive'); + expect(assessment).toContain('"Cancel" — stop, do not archive'); + expect(content).toContain('If the sync failed, or any capability does not match'); + expect(content).toContain('stop — do not archive'); + } + }); + + it('recognizes explicitly retired missing specs without blocking archive verification', () => { + for (const content of [ + getArchiveChangeSkillTemplate().instructions, + getOpsxArchiveCommandTemplate().content, + ]) { + const assessment = content.slice( + content.indexOf('**If delta specs exist:**'), + content.indexOf('**Prompt options:**') + ); + const retirement = assessment.indexOf('Otherwise, if the delta has only REMOVED requirements'); + expect(retirement).toBeGreaterThan(-1); + expect(retirement).toBeLessThan(assessment.indexOf('Otherwise, if the delta has no ADDED requirements')); + expect(assessment).toContain('`retire_capabilities: true`'); + expect(assessment).toContain('count it as already synced'); + expect(assessment).toContain('do not recreate the main spec'); + expect(content).toContain('including the explicitly retired, missing-spec case'); + } + + for (const content of [ + getSyncSpecsSkillTemplate().instructions, + getOpsxSyncCommandTemplate().content, + ]) { + const createStep = content.slice( + content.indexOf('d. **Create new main spec**'), + content.indexOf('**Validate updated main specs**') + ); + expect(createStep).toContain('`retire_capabilities: true`'); + expect(createStep).toContain('report it as already retired'); + expect(createStep).toContain('Without that marker, report the sync as blocked'); + expect(createStep).toContain('create nothing'); + } + }); + it('gates bulk archive on inline synchronous spec sync and verification before moving change root', () => { const generatedSkill = generateSkillContent(getBulkArchiveChangeSkillTemplate(), 'PARITY-BASELINE'); const commandContent = getOpsxBulkArchiveCommandTemplate().content;