From bbed2a38fdb3319d4eff42678524505ce807261c Mon Sep 17 00:00:00 2001 From: Clay Good Date: Mon, 20 Jul 2026 14:56:42 -0500 Subject: [PATCH] fix(templates): stop the bulk archive when the user picks Cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bulk archive confirmation offered a "Cancel" option but never told the agent what to do with it. Step 8 then archived every selected change, so an agent following the skill literally moved the changes even after the user cancelled. Route each answer by intent rather than by literal label: the option labels are written by the agent and carry an `N` placeholder, so matching them verbatim would send every legitimate answer down the "ask again" path. Cancel now stops without archiving and skips the remaining steps, the ready-only option is bound to the status table that decides what "ready" means (re-deriving conflict resolutions when a Ready* partner is skipped), and a guardrail repeats that a cancelled batch archives nothing. Regression tests cover both archive paths, so the single-change routing can no longer be silently reverted either. Instruction text only — no CLI behavior changes. Closes #1381 Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/bulk-archive-honors-cancel.md | 7 +++ skills/openspec-bulk-archive-change/SKILL.md | 8 +++ .../workflows/bulk-archive-change.ts | 16 ++++++ .../templates/skill-templates-parity.test.ts | 51 +++++++++++++++++-- 4 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 .changeset/bulk-archive-honors-cancel.md diff --git a/.changeset/bulk-archive-honors-cancel.md b/.changeset/bulk-archive-honors-cancel.md new file mode 100644 index 0000000000..0756501fdf --- /dev/null +++ b/.changeset/bulk-archive-honors-cancel.md @@ -0,0 +1,7 @@ +--- +"@fission-ai/openspec": patch +--- + +### Bug Fixes + +- **Bulk archive now stops when you pick "Cancel"** — the generated `openspec-bulk-archive-change` skill (and the matching `opsx:bulk-archive` command) offered a "Cancel" option at the confirmation prompt but never told the agent what to do with it, so the next step archived every selected change anyway. The prompt now routes each answer by intent: "Cancel" stops without archiving anything, the archive options proceed (the ready-only option archives just the changes the status table marks `Ready` or `Ready*`), and any other answer re-asks instead of archiving. The single-change archive skill already routes Cancel this way; this brings the bulk variant in line. diff --git a/skills/openspec-bulk-archive-change/SKILL.md b/skills/openspec-bulk-archive-change/SKILL.md index 8ef031b931..87dd2205a0 100644 --- a/skills/openspec-bulk-archive-change/SKILL.md +++ b/skills/openspec-bulk-archive-change/SKILL.md @@ -118,6 +118,13 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig If there are incomplete changes, make clear they'll be archived with warnings. + Route on the answer by intent, not by exact label — you wrote these labels, + so match what the user picked rather than the wording above: + - "Cancel" — stop, do not archive. Report that nothing was archived and skip the remaining steps. + - The archive-everything option — proceed with every selected change + - The ready-only option — proceed with only the changes the step 6 table marks `Ready` or `Ready*`, and record the rest as Skipped in step 8c. If a `Ready*` change's conflict partner is skipped, re-derive that conflict's resolution using only the changes being archived. + - Anything else — ask again rather than archiving + 8. **Execute archive for each confirmed change** Process changes in the determined order (respecting conflict resolution): @@ -245,6 +252,7 @@ No active changes found. Create a new change to get started. - Skip spec sync only when implementation is missing (warn user) - Show clear per-change status before confirming - Use single confirmation for entire batch +- Never archive after the user cancels the confirmation — a cancelled batch archives nothing - Track and report all outcomes (success/skip/fail) - Preserve .openspec.yaml when moving to archive - Archive directory target uses current date: YYYY-MM-DD-; a name that already starts with a `YYYY-MM-DD-` prefix is used as-is (never stack a second date) diff --git a/src/core/templates/workflows/bulk-archive-change.ts b/src/core/templates/workflows/bulk-archive-change.ts index 3acc0b2add..3cca28b022 100644 --- a/src/core/templates/workflows/bulk-archive-change.ts +++ b/src/core/templates/workflows/bulk-archive-change.ts @@ -120,6 +120,13 @@ ${STORE_SELECTION_GUIDANCE} If there are incomplete changes, make clear they'll be archived with warnings. + Route on the answer by intent, not by exact label — you wrote these labels, + so match what the user picked rather than the wording above: + - "Cancel" — stop, do not archive. Report that nothing was archived and skip the remaining steps. + - The archive-everything option — proceed with every selected change + - The ready-only option — proceed with only the changes the step 6 table marks \`Ready\` or \`Ready*\`, and record the rest as Skipped in step 8c. If a \`Ready*\` change's conflict partner is skipped, re-derive that conflict's resolution using only the changes being archived. + - Anything else — ask again rather than archiving + 8. **Execute archive for each confirmed change** Process changes in the determined order (respecting conflict resolution): @@ -247,6 +254,7 @@ No active changes found. Create a new change to get started. - Skip spec sync only when implementation is missing (warn user) - Show clear per-change status before confirming - Use single confirmation for entire batch +- Never archive after the user cancels the confirmation — a cancelled batch archives nothing - Track and report all outcomes (success/skip/fail) - Preserve .openspec.yaml when moving to archive - Archive directory target uses current date: YYYY-MM-DD-; a name that already starts with a \`YYYY-MM-DD-\` prefix is used as-is (never stack a second date) @@ -372,6 +380,13 @@ ${STORE_SELECTION_GUIDANCE} If there are incomplete changes, make clear they'll be archived with warnings. + Route on the answer by intent, not by exact label — you wrote these labels, + so match what the user picked rather than the wording above: + - "Cancel" — stop, do not archive. Report that nothing was archived and skip the remaining steps. + - The archive-everything option — proceed with every selected change + - The ready-only option — proceed with only the changes the step 6 table marks \`Ready\` or \`Ready*\`, and record the rest as Skipped in step 8c. If a \`Ready*\` change's conflict partner is skipped, re-derive that conflict's resolution using only the changes being archived. + - Anything else — ask again rather than archiving + 8. **Execute archive for each confirmed change** Process changes in the determined order (respecting conflict resolution): @@ -499,6 +514,7 @@ No active changes found. Create a new change to get started. - Skip spec sync only when implementation is missing (warn user) - Show clear per-change status before confirming - Use single confirmation for entire batch +- Never archive after the user cancels the confirmation — a cancelled batch archives nothing - Track and report all outcomes (success/skip/fail) - Preserve .openspec.yaml when moving to archive - Archive directory target uses current date: YYYY-MM-DD-; a name that already starts with a \`YYYY-MM-DD-\` prefix is used as-is (never stack a second date) diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index ec3f5ca6aa..76547e9c6a 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -50,12 +50,12 @@ const EXPECTED_FUNCTION_HASHES: Record = { getOpsxApplyCommandTemplate: 'daeb507206707169de73c828e199648dde5732cbc17791ef2a027adffd028574', getOpsxFfCommandTemplate: 'b859b1955cda6012877ae7f9ec6980e468f2e949a3838dfcdebc17209d133749', getArchiveChangeSkillTemplate: 'b04eccde2c57af4bc484fa7279fa873ad1d46474eb024467d68e784d8b985c18', - getBulkArchiveChangeSkillTemplate: 'f31d17602c274a3fc24d688fb368156618cd31e07762a267d2c506c63b4b4760', + getBulkArchiveChangeSkillTemplate: '2b74b1f73380ff32e35f580734780d843c6161a2748c39edb07f1e00453771b4', getOpsxSyncCommandTemplate: '98b20e00da5c588ff83ed6e6f0e959dfc540349090fb3f5792ea030d099b8169', getVerifyChangeSkillTemplate: 'cab4db01b5d2b1243d63d90c53747d8b39e488c60f76eba3fe8b994467f69267', getOpsxArchiveCommandTemplate: '8c113e2a8bca36fecd0e2152ae262fbfbef508e81378838e15d31308fb069b57', getOpsxOnboardCommandTemplate: '9430a0fb6530791ab720e068f4b172bc3dfc4e96a1ae29102bee0b92c2afe7b5', - getOpsxBulkArchiveCommandTemplate: '22dde4864ec494eee774a46fe5c0c6a68f4ca9ff67272c3177a5d4f5c2be07b7', + getOpsxBulkArchiveCommandTemplate: 'da7be1a7318f15b915f5aae8eb638797a8a24a31e5fc7fc0a2bad01bba137686', getOpsxVerifyCommandTemplate: 'f01c0c0cef53be0956de52363d955d4ace131b1b2d77adf902f35fead9a1486d', getOpsxProposeSkillTemplate: '59197064a46c53264b62925a1c725af4ebe7caf9f0eaed4101990b7c13a40db1', getOpsxProposeCommandTemplate: '04f808a36e850b9cdbc4f943ef324a9fd2b1b0cc59b92f127ab6cc452d66cc4e', @@ -72,7 +72,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-ff-change': '0c82830cd9bc98f86eb56b63ddaabe2bf5d35fe25b6c40a7059311aee2c8acac', 'openspec-sync-specs': 'b3f694ab81956d05126b089fe82dea78dec21788978bb9651485f996aee96740', 'openspec-archive-change': 'b24d326662ef58809de4464960440713748b9a281323357facdca24af52014e7', - 'openspec-bulk-archive-change': '98c682899a6fd4c83e71b790b27d6d4ccf832e51c0e754119537992a469c75ec', + 'openspec-bulk-archive-change': '49d410bda408c0411decd584be9c2355335e3b3db760fc6a0adcd82c172a280f', 'openspec-verify-change': '57693d22940f06080c6cf8d590ac2f48240d4a5e9ce7074dacd0f8d3c9945afa', 'openspec-onboard': '76225d10352454a304e56566997811d16f91de1b37653816f2bc5d8ec976febc', 'openspec-propose': '024db4bce28d9a4d7b25fa92525da6fc701a64ac07dfdcf777d286c95b5281b5', @@ -267,4 +267,49 @@ describe('skill templates split parity', () => { expect(text, id).not.toMatch(/\/YYYY-MM-DD-/); } }); + + // Covers both archive paths, not just the bulk one the fix targeted: the + // single-change routing has been correct since #1357 (current wording from + // #1394) but was never pinned, so a stale branch could silently reopen the + // bug #1381 actually reported. + it('honors Cancel at every archive confirmation (#1381)', () => { + const variants: Array<[string, string]> = [ + ['bulk skill', generateSkillContent(getBulkArchiveChangeSkillTemplate(), 'PARITY-BASELINE')], + ['bulk opsx command', getOpsxBulkArchiveCommandTemplate().content], + ['single skill', generateSkillContent(getArchiveChangeSkillTemplate(), 'PARITY-BASELINE')], + ['single opsx command', getOpsxArchiveCommandTemplate().content], + ]; + + for (const [variant, content] of variants) { + // Offering "Cancel" without routing it let an agent fall straight through + // to the archive step and move the changes anyway. + expect(content, variant).toContain('"Cancel" — stop, do not archive'); + + // An unrecognized answer must re-prompt; archiving is never the default. + expect(content, variant).toContain('Anything else — ask again rather than archiving'); + } + }); + + // The bulk confirmation labels are written by the agent and carry an `N` + // placeholder, so routing must match intent — matching the literal labels + // would send every legitimate answer down the "ask again" path forever. + it('routes the bulk archive confirmation by intent, not by literal label (#1381)', () => { + const variants: Array<[string, string]> = [ + ['bulk skill', generateSkillContent(getBulkArchiveChangeSkillTemplate(), 'PARITY-BASELINE')], + ['bulk opsx command', getOpsxBulkArchiveCommandTemplate().content], + ]; + + for (const [variant, content] of variants) { + expect(content, variant).toContain('Route on the answer by intent, not by exact label'); + + // The ready-only route has to name where "ready" is decided, or the agent + // cannot tell which subset to archive. + expect(content, variant).toContain('the changes the step 6 table marks'); + + // A cancelled batch must archive nothing, reinforced where agents skim. + expect(content, variant).toContain( + 'Never archive after the user cancels the confirmation' + ); + } + }); });