Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/update-change-step-4-drafts.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@fission-ai/openspec': patch
---

Fix the update-change workflow contradicting its own confirmation gate. Step 4 said to apply the requested edit while step 5 said to write only after the user confirms, and "apply" reads as a write verb in that document - step 5 is itself titled "Confirm and apply". Step 4 now drafts the edit and step 5 remains the only write path, so `/opsx:update "the design now uses X"` always shows the revision before writing it. Both delivery surfaces carry the same wording and it is now pinned by tests.
2 changes: 1 addition & 1 deletion skills/openspec-update-change/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Revise a change's existing planning artifacts and keep them coherent. Never edit

4. **Read and reconcile**
- Read the artifact(s) the request touches and the change's other existing artifacts.
- Apply the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised.
- Draft the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised.
- Note everything that is now inconsistent, missing, or contradictory.
- Revise only files that already exist (`existingOutputPaths`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them and point the user to `/openspec-continue-change` to create them.
- If the change is already coherent, say so and make no edits.
Expand Down
4 changes: 2 additions & 2 deletions src/core/templates/workflows/update-change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ ${STORE_SELECTION_GUIDANCE}

4. **Read and reconcile**
- Read the artifact(s) the request touches and the change's other existing artifacts.
- Apply the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised.
- Draft the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised.
- Note everything that is now inconsistent, missing, or contradictory.
- Revise only files that already exist (\`existingOutputPaths\`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them and point the user to \`/opsx:continue\` to create them.
- If the change is already coherent, say so and make no edits.
Expand Down Expand Up @@ -149,7 +149,7 @@ ${STORE_SELECTION_GUIDANCE}

4. **Read and reconcile**
- Read the artifact(s) the request touches and the change's other existing artifacts.
- Apply the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised.
- Draft the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised.
- Note everything that is now inconsistent, missing, or contradictory.
- Revise only files that already exist (\`existingOutputPaths\`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them and point the user to \`/opsx:continue\` to create them.
- If the change is already coherent, say so and make no edits.
Expand Down
6 changes: 3 additions & 3 deletions test/core/templates/skill-templates-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ const EXPECTED_FUNCTION_HASHES: Record<string, string> = {
getOpsxProposeSkillTemplate: 'b7215583fefddae0127076465de9b3de9c230f2f1ea9ae6e4fb2a46fe510e8d6',
getOpsxProposeCommandTemplate: 'f016c66c2b6115b459751154c76a6270e444d6aee31973bb7cb8c0e6d505fb98',
getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133',
getUpdateChangeSkillTemplate: '7dc8abc6f64c58bf34d7581ed4ab095a3b7a53cb372349bee2d840db58622819',
getOpsxUpdateCommandTemplate: 'e2388521b22f92f74561df9a0c2f98e1fa4d265af93b5ba26f42fb47a6c5bfed',
getUpdateChangeSkillTemplate: 'af3c7dca6aeab8b3dfe54789f8d1570be564053d50749022091e1def59ffb9c3',
getOpsxUpdateCommandTemplate: '7808077fca5bc5985d463c3b3ef9235d4f1088c09417ab6258e5daad37701c89',
};

const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record<string, string> = {
Expand All @@ -77,7 +77,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record<string, string> = {
'openspec-verify-change': 'af9be013dcbe8c6d8f6d9ab10c893fbd03f4c62933c384d82f63894dd0ceb84f',
'openspec-onboard': 'f6f59476acaf5e4d65dbb180da4cef62432612f3cecf207d471a951295e2003a',
'openspec-propose': '679d0f868bed23cfb34a8ecc6b4ba4ff7b88dd7dbaef91563423e98f194f988f',
'openspec-update-change': '586547406aca94422dfeb3ffedce6c01049429b743f57ce829baa79ebc714d51',
'openspec-update-change': 'bd16c9ea9a8ade956f97a5a6390402c87c642a6d459bb186f991cc1d57554179',
};

// Intentionally excludes getFeedbackSkillTemplate: this list only models templates
Expand Down
30 changes: 30 additions & 0 deletions test/core/templates/update-change.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,34 @@ describe('update-change templates', () => {
expect(newRecommendation, label).toBeGreaterThan(newAvailabilityCheck);
}
});

// Regression for #1836: step 4 said "Apply the requested edit" while step 5
// said "Write only after the user confirms". "Apply" reads as a write verb in
// this document - step 5 is itself titled "Confirm and apply" - so the same
// request either wrote immediately or showed the revision first, depending on
// which passage the agent weighed. Step 5 is the only write path; step 4
// drafts.
it('keeps step 4 non-writing so step 5 owns the only write (#1836)', () => {
for (const [label, body] of bodies) {
const start = body.indexOf('4. **Read and reconcile**');
const end = body.indexOf('5. **Confirm and apply, one artifact at a time**');

expect(start, label).toBeGreaterThanOrEqual(0);
expect(end, label).toBeGreaterThan(start);

const step = body.slice(start, end);
expect(step, label).toContain('Draft the requested edit.');
expect(step, label).not.toContain('Apply the requested edit');
}
});

it('orders the draft before the confirmed write (#1836)', () => {
for (const [label, body] of bodies) {
const draft = body.indexOf('Draft the requested edit.');
const confirmedWrite = body.indexOf('Write only after the user confirms.');

expect(draft, label).toBeGreaterThanOrEqual(0);
expect(confirmedWrite, label).toBeGreaterThan(draft);
}
});
});