mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-02 05:24:34 +08:00
fix(archive): require successful spec sync before archiving (#1759)
* Updated Archive Skill * Updated Testcase * Updated Skill and Updated Unit Test * Updated Archive and Bulk Archive Skill * Updated Bulk Archive Skill * fix(archive): keep existing main spec titles during sync verification The post-sync structure check required every main spec to start with a '# <capability> Specification' title. Neither sync-specs nor the validator enforces that, and valid specs (including this repo's opsx-archive-skill) use other titles, so an agent could stop the archive or rewrite a user's title. Scope the title and heading rules to what the sync writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(specs): match opsx-archive-skill spec to the stricter sync check The archive skill now treats a sync that reports a blocked retirement as a failed sync, so the capability spec no longer calls that kept spec verified. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
AJITHKUMAR
co-authored by
Claude Opus 5.5
Clay Good
Claude Opus 5.5
Clay Good
parent
d4e1c77eba
commit
0ff63dbae6
@@ -88,7 +88,8 @@ The skill SHALL prompt to sync delta specs before archiving if specs exist.
|
||||
- **AND** if user cancels, stop without archiving
|
||||
- **AND** if user confirms, execute `/opsx:sync` logic inline and wait for it to complete
|
||||
- **AND** verify every capability that has a delta spec, not only those the sync reports it touched: ADDED requirements present, MODIFIED requirements carrying the changes named in the delta, REMOVED requirements absent, RENAMED requirements present under the new name and absent under the old one
|
||||
- **AND** treat a capability whose last requirement the sync removed as verified when its main spec was deleted rather than left empty, and a spec the sync deliberately kept and reported as verified too
|
||||
- **AND** treat a capability whose last requirement the sync removed as verified when its main spec was deleted rather than left empty
|
||||
- **AND** treat any stop or blocking condition the sync reports as a failed sync, including a main spec it left unmodified because a retirement was blocked
|
||||
- **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
|
||||
|
||||
|
||||
@@ -146,10 +146,21 @@ In both branches, never create the root as a side effect: do not run `openspec i
|
||||
|
||||
Then run the `openspec-sync-specs` workflow inline (agent-driven intelligent merge) for change '<name>', 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.
|
||||
|
||||
If the sync reports any stop or blocking condition, treat the sync as failed.
|
||||
Stop the archive immediately. Do not perform the post-sync content comparison and do not move its `changeRoot`.
|
||||
Nothing has moved, so the user can fix the blocking condition or re-run the sync.
|
||||
|
||||
After the sync writes each main spec, verify its structure against the canonical sync contract:
|
||||
- A new main spec starts with a `# <capability> Specification` title. An existing main spec keeps its title exactly as it is.
|
||||
- Preserve existing `## Purpose` sections completely untouched for established main specs.
|
||||
- For a new main spec, copy the delta `## Purpose` verbatim. Warn only if the purpose text is shorter than standard validation expects. Do not regenerate or rewrite existing authored purpose. If no usable `## Purpose` is provided, use the existing TBD Purpose behavior and warning.
|
||||
- Verify that no delta-style section headers (`## ADDED Requirements`, `## MODIFIED Requirements`, `## REMOVED Requirements`, `## RENAMED Requirements`) remain in the main spec, adhering strictly to the sync workflow formatting rules.
|
||||
- Requirement blocks the sync wrote or changed use `### Requirement:` headings, and their scenarios use `#### Scenario:` headings, under the spec's `## Requirements` section. Leave content the delta does not mention exactly as it is.
|
||||
|
||||
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
|
||||
- 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.
|
||||
- RENAMED requirements present under the new name and absent under the old one
|
||||
|
||||
If the sync failed, or any capability does not match, report what differs and stop — do not archive. Nothing has moved and `changeRoot` is intact, so the user can fix the mismatch or re-run the sync and start the archive again.
|
||||
|
||||
@@ -206,6 +206,8 @@ In both branches, never create the root as a side effect: do not run `openspec i
|
||||
|
||||
a. **Sync included delta specs**:
|
||||
- Run the `openspec-sync-specs` workflow inline (agent-driven intelligent merge) only for changes with entries in `includedDeltas`, passing only the included delta paths and explicitly instructing it to ignore that change's `excludedDeltas`. Wait for it to finish.
|
||||
- If the sync reports any stop or blocking condition, treat the sync as failed. Stop processing that change immediately. Before continuing to the next change, record this change's outcome as Failed in the batch results, including the sync blocking/error condition.
|
||||
- Do not perform the post-sync content comparison and do not move its `changeRoot`; leave the change intact.
|
||||
- For conflicts, apply in resolved order.
|
||||
- Pass that change's fetched specs-rule snapshot into inline sync; inline
|
||||
sync must reuse it without fetching instructions again
|
||||
@@ -220,7 +222,7 @@ In both branches, never create the root as a side effect: do not run `openspec i
|
||||
- Verify that main specs are updated:
|
||||
- ADDED requirements present
|
||||
- MODIFIED requirements carrying scenario and description changes named in the delta, with their other scenarios intact
|
||||
- REMOVED requirements gone — 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
|
||||
- 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.
|
||||
- RENAMED requirements present under the new name and absent under the old one
|
||||
- Do not verify delta specs in `excludedDeltas`; they are intentionally left unsynced.
|
||||
- If sync failed or any capability does not match verification, report what differs and fail/skip moving that change's `changeRoot` — do not archive that change. `changeRoot` remains intact.
|
||||
|
||||
@@ -160,10 +160,21 @@ ${PROJECT_ROOT_GUARD}
|
||||
|
||||
Then run the \`openspec-sync-specs\` workflow inline (agent-driven intelligent merge) for change '<name>', 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.
|
||||
|
||||
If the sync reports any stop or blocking condition, treat the sync as failed.
|
||||
Stop the archive immediately. Do not perform the post-sync content comparison and do not move its \`changeRoot\`.
|
||||
Nothing has moved, so the user can fix the blocking condition or re-run the sync.
|
||||
|
||||
After the sync writes each main spec, verify its structure against the canonical sync contract:
|
||||
- A new main spec starts with a \`# <capability> Specification\` title. An existing main spec keeps its title exactly as it is.
|
||||
- Preserve existing \`## Purpose\` sections completely untouched for established main specs.
|
||||
- For a new main spec, copy the delta \`## Purpose\` verbatim. Warn only if the purpose text is shorter than standard validation expects. Do not regenerate or rewrite existing authored purpose. If no usable \`## Purpose\` is provided, use the existing TBD Purpose behavior and warning.
|
||||
- Verify that no delta-style section headers (\`## ADDED Requirements\`, \`## MODIFIED Requirements\`, \`## REMOVED Requirements\`, \`## RENAMED Requirements\`) remain in the main spec, adhering strictly to the sync workflow formatting rules.
|
||||
- Requirement blocks the sync wrote or changed use \`### Requirement:\` headings, and their scenarios use \`#### Scenario:\` headings, under the spec's \`## Requirements\` section. Leave content the delta does not mention exactly as it is.
|
||||
|
||||
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
|
||||
- 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.
|
||||
- RENAMED requirements present under the new name and absent under the old one
|
||||
|
||||
If the sync failed, or any capability does not match, report what differs and stop — do not archive. Nothing has moved and \`changeRoot\` is intact, so the user can fix the mismatch or re-run the sync and start the archive again.
|
||||
@@ -361,10 +372,21 @@ ${PROJECT_ROOT_GUARD}
|
||||
|
||||
Then ${SYNC_INLINE_HANDOFF} for change '<name>', 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.
|
||||
|
||||
If the sync reports any stop or blocking condition, treat the sync as failed.
|
||||
Stop the archive immediately. Do not perform the post-sync content comparison and do not move its \`changeRoot\`.
|
||||
Nothing has moved, so the user can fix the blocking condition or re-run the sync.
|
||||
|
||||
After the sync writes each main spec, verify its structure against the canonical sync contract:
|
||||
- A new main spec starts with a \`# <capability> Specification\` title. An existing main spec keeps its title exactly as it is.
|
||||
- Preserve existing \`## Purpose\` sections completely untouched for established main specs.
|
||||
- For a new main spec, copy the delta \`## Purpose\` verbatim. Warn only if the purpose text is shorter than standard validation expects. Do not regenerate or rewrite existing authored purpose. If no usable \`## Purpose\` is provided, use the existing TBD Purpose behavior and warning.
|
||||
- Verify that no delta-style section headers (\`## ADDED Requirements\`, \`## MODIFIED Requirements\`, \`## REMOVED Requirements\`, \`## RENAMED Requirements\`) remain in the main spec, adhering strictly to the sync workflow formatting rules.
|
||||
- Requirement blocks the sync wrote or changed use \`### Requirement:\` headings, and their scenarios use \`#### Scenario:\` headings, under the spec's \`## Requirements\` section. Leave content the delta does not mention exactly as it is.
|
||||
|
||||
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
|
||||
- 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.
|
||||
- RENAMED requirements present under the new name and absent under the old one
|
||||
|
||||
If the sync failed, or any capability does not match, report what differs and stop — do not archive. Nothing has moved and \`changeRoot\` is intact, so the user can fix the mismatch or re-run the sync and start the archive again.
|
||||
|
||||
@@ -220,6 +220,8 @@ ${PROJECT_ROOT_GUARD}
|
||||
|
||||
a. **Sync included delta specs**:
|
||||
- ${optionalWorkflow('sync', 'Run the `openspec-sync-specs` workflow inline (agent-driven intelligent merge)', 'Perform the delta-to-main-spec merge inline yourself (agent-driven intelligent merge)')} only for changes with entries in \`includedDeltas\`, passing only the included delta paths and explicitly instructing it to ignore that change's \`excludedDeltas\`. Wait for it to finish.
|
||||
- If the sync reports any stop or blocking condition, treat the sync as failed. Stop processing that change immediately. Before continuing to the next change, record this change's outcome as Failed in the batch results, including the sync blocking/error condition.
|
||||
- Do not perform the post-sync content comparison and do not move its \`changeRoot\`; leave the change intact.
|
||||
- For conflicts, apply in resolved order.
|
||||
- Pass that change's fetched specs-rule snapshot into inline sync; inline
|
||||
sync must reuse it without fetching instructions again
|
||||
@@ -234,7 +236,7 @@ ${PROJECT_ROOT_GUARD}
|
||||
- Verify that main specs are updated:
|
||||
- ADDED requirements present
|
||||
- MODIFIED requirements carrying scenario and description changes named in the delta, with their other scenarios intact
|
||||
- REMOVED requirements gone — 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
|
||||
- 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.
|
||||
- RENAMED requirements present under the new name and absent under the old one
|
||||
- Do not verify delta specs in \`excludedDeltas\`; they are intentionally left unsynced.
|
||||
- If sync failed or any capability does not match verification, report what differs and fail/skip moving that change's \`changeRoot\` — do not archive that change. \`changeRoot\` remains intact.
|
||||
@@ -586,6 +588,8 @@ ${PROJECT_ROOT_GUARD}
|
||||
|
||||
a. **Sync included delta specs**:
|
||||
- ${SYNC_INLINE_HANDOFF} only for changes with entries in \`includedDeltas\`, passing only the included delta paths and explicitly instructing it to ignore that change's \`excludedDeltas\`. Wait for it to finish.
|
||||
- If the sync reports any stop or blocking condition, treat the sync as failed. Stop processing that change immediately. Before continuing to the next change, record this change's outcome as Failed in the batch results, including the sync blocking/error condition.
|
||||
- Do not perform the post-sync content comparison and do not move its \`changeRoot\`; leave the change intact.
|
||||
- For conflicts, apply in resolved order.
|
||||
- Pass that change's fetched specs-rule snapshot into inline sync; inline
|
||||
sync must reuse it without fetching instructions again
|
||||
@@ -600,7 +604,7 @@ ${PROJECT_ROOT_GUARD}
|
||||
- Verify that main specs are updated:
|
||||
- ADDED requirements present
|
||||
- MODIFIED requirements carrying scenario and description changes named in the delta, with their other scenarios intact
|
||||
- REMOVED requirements gone — 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
|
||||
- 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.
|
||||
- RENAMED requirements present under the new name and absent under the old one
|
||||
- Do not verify delta specs in \`excludedDeltas\`; they are intentionally left unsynced.
|
||||
- If sync failed or any capability does not match verification, report what differs and fail/skip moving that change's \`changeRoot\` — do not archive that change. \`changeRoot\` remains intact.
|
||||
|
||||
@@ -88,13 +88,13 @@ const EXPECTED_FUNCTION_HASHES: Record<string, string> = {
|
||||
getOpsxContinueCommandTemplate: '241c50f97d5d681412d456d6b982743c3a5babeb77017fc8099c418bcf0d92df',
|
||||
getOpsxApplyCommandTemplate: 'd70cecce3b7d1dd4dbd5fd1fc2bccb538f5e61f5b43d520e4beca896e3f9e6b3',
|
||||
getOpsxFfCommandTemplate: '743a7304c7efc84aa87f556154c034e1e0e561c276c51870a30ada58f33eb9af',
|
||||
getArchiveChangeSkillTemplate: '71715f9d5899498942af03e182e6d1ac2c95952dde967950c2a9161084a53a8b',
|
||||
getBulkArchiveChangeSkillTemplate: '2a6ec08fea0f942158b4abe9c8d1af9622038e0c4dc684e7c73dad2fb8379a54',
|
||||
getArchiveChangeSkillTemplate: '286a8e56580f35f179fe050efbddcae87ef4d6ac68c24b1fce91c6651d5295be',
|
||||
getBulkArchiveChangeSkillTemplate: '44dbd3c7a347e5f8339b2141393f2ac36017527cce251483fe70f2059c1e286e',
|
||||
getOpsxSyncCommandTemplate: '60550b7bb9829421656d6324a9e4c951bc912f48f88882d1a07ce7f78397a5e7',
|
||||
getVerifyChangeSkillTemplate: 'eecb063792075191b613978dec45f9f2fee247d2ff3003f2ebf17d632e54352e',
|
||||
getOpsxArchiveCommandTemplate: '3d2a330b46043fbb9f220831aa42ebbb62f411e9597b2bb491ad1ac1fa2d0873',
|
||||
getOpsxArchiveCommandTemplate: '5d153490bf1ca24207f49856826720f793c8884a6ec15059104107b0e34ee345',
|
||||
getOpsxOnboardCommandTemplate: '0cf66e164c0e14c916c6d1ebb5d80ded07d7fb8e55d4eb34eba43e8ca9c28558',
|
||||
getOpsxBulkArchiveCommandTemplate: '4e2e39c4d634074f4a1ed67f076d5c4d0ead8b998f4d75218c33cdc6173719be',
|
||||
getOpsxBulkArchiveCommandTemplate: 'cb1d55d6ce53686bfe94be5e081c7a4d06a8e4d10b63019132df5bb3db7144cb',
|
||||
getOpsxVerifyCommandTemplate: 'f47bc0c30cfa8e93b5e42026e9417636c5f15bd8505fb9138872e34af8906abb',
|
||||
getOpsxProposeSkillTemplate: '1aa2f2eb9c8cbc4dcab9d777bf8832b92ca04f9ef91d0494f1224a566aefdfe8',
|
||||
getOpsxProposeCommandTemplate: '3b7090ce5e79e879ab9b5bdaf4ff2b52e3c02211f71188838772d36ac337f96c',
|
||||
@@ -110,8 +110,8 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record<string, string> = {
|
||||
'openspec-apply-change': 'f3e92c229fab8d77df9f0a77dcb117cf46279b53a208d53aed89bfe0bab2ac09',
|
||||
'openspec-ff-change': 'a7ab656d46f04d45dff0c8888df4a126a2e62288b7336f7445bce4d1715055f5',
|
||||
'openspec-sync-specs': '3909936a236a21a9a6d5bf495f90b396b3b68fc9220d7b2c1894668653beb2e4',
|
||||
'openspec-archive-change': 'd01d9eeb06223ee89708b7963e82c5ebc11719c5b2dc62d4abb268ee016fcb7b',
|
||||
'openspec-bulk-archive-change': '10f050ad5ef77084dc55a202427988b23903ee122f00985238a4eb9354a5dc3c',
|
||||
'openspec-archive-change': '5f0d131a885dcdcd9ba2172ea9a42bc6748125e24b8c4eecb7c86f1a4aea83af',
|
||||
'openspec-bulk-archive-change': 'd2a258055ab2f0d8086c4348d37212ebc95a5adef2d5f524db959fb93490d5c8',
|
||||
'openspec-verify-change': '62c2d471a1ebc4be38df0d06393eb94d3d8b803719b6349b8a1d8e9231448275',
|
||||
'openspec-onboard': '6993eff867d97d485e080078f9dfb80e968e242f3b17a924eeb077715fd548fa',
|
||||
'openspec-propose': '66e3395adf9f2d93a09e8ef1d20e4efb010e5e8d4811f2d42a9316e4d1ca5a8b',
|
||||
@@ -570,6 +570,10 @@ describe('skill templates split parity', () => {
|
||||
expect(content, variant).toContain('Do not delegate it to a background task');
|
||||
expect(content, variant).toContain('Never archive while a spec sync is still in flight');
|
||||
|
||||
expect(content, variant).toContain('If the sync reports any stop or blocking condition, treat the sync as failed');
|
||||
expect(content, variant).toContain('Do not perform the post-sync content comparison');
|
||||
expect(content, variant).toContain('do not move its `changeRoot`');
|
||||
|
||||
// Verification must follow delta semantics.
|
||||
expect(content, variant).toContain('MODIFIED requirements carrying the scenario and description changes');
|
||||
expect(content, variant).toContain('REMOVED requirements gone');
|
||||
@@ -580,6 +584,29 @@ describe('skill templates split parity', () => {
|
||||
|
||||
// Main spec paths are store-root aware
|
||||
expect(content, variant).toContain('<planningHome.root>/openspec/specs/<capability-path>/spec.md');
|
||||
|
||||
// Semantic main-spec structure contract.
|
||||
expect(content, variant).toContain('A new main spec starts with a `# <capability> Specification` title. An existing main spec keeps its title exactly as it is.');
|
||||
expect(content, variant).not.toContain('MUST start with a `# <capability> Specification` title');
|
||||
expect(content, variant).toContain('Preserve existing `## Purpose` sections completely untouched for established main specs.');
|
||||
expect(content, variant).toContain('For a new main spec, copy the delta `## Purpose` verbatim.');
|
||||
expect(content, variant).toContain('If no usable `## Purpose` is provided, use the existing TBD Purpose behavior and warning.');
|
||||
expect(content, variant).toContain('Requirement blocks the sync wrote or changed use `### Requirement:` headings');
|
||||
expect(content, variant).toContain('Leave content the delta does not mention exactly as it is.');
|
||||
|
||||
// Every canonical delta header must be rejected.
|
||||
const deltaHeaders = [
|
||||
'## ADDED Requirements',
|
||||
'## MODIFIED Requirements',
|
||||
'## REMOVED Requirements',
|
||||
'## RENAMED Requirements',
|
||||
];
|
||||
|
||||
expect(content, variant).toContain('Verify that no delta-style section headers (`## ADDED Requirements`, `## MODIFIED Requirements`, `## REMOVED Requirements`, `## RENAMED Requirements`) remain in the main spec');
|
||||
|
||||
for (const header of deltaHeaders) {
|
||||
expect(content, variant).toContain(header);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
@@ -742,6 +769,10 @@ describe('skill templates split parity', () => {
|
||||
expect(content, variant).toContain('REMOVED requirements gone');
|
||||
expect(content, variant).toContain('RENAMED requirements present under the new name and absent under the old one');
|
||||
|
||||
expect(content, variant).toContain('If the sync reports any stop or blocking condition, treat the sync as failed');
|
||||
expect(content, variant).toContain('Do not perform the post-sync content comparison');
|
||||
expect(content, variant).toContain('do not move its `changeRoot`');
|
||||
|
||||
// Main spec paths are store-root aware
|
||||
expect(content, variant).toContain('<planningHome.root>/openspec/specs/<capability-path>/spec.md');
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user