fix(update-change): draft the requested edit in step 4, write in step 5

[choi138]

Fixes #1836.

What was wrong

update-change stated its write gate twice and then contradicted it a step earlier, with nothing saying which governs.

Step 5, "Confirm and apply, one artifact at a time":

• Show each proposed revision and why. Write only after the user confirms.

Guardrails:

• Confirm every edit with the user before writing.

Step 4, "Read and reconcile":

• Apply the requested edit. Then check every other existing artifact against it…

"Apply" reads as a write verb in this document — step 5 is itself titled "Confirm and apply", and step 4's closing bullet ("If the change is already coherent, say so and make no edits") only parses if step 4 is the editing stage. So /opsx:update "the design now uses X" either wrote immediately or stopped and showed the revision first, depending on which passage the agent weighed. Step 5 is the workflow's only write path, so its confirmation guarantee was unenforceable whenever step 4 governed.

Same shape as #1828: a guarantee stated in one part of the body, contradicted by an instruction earlier in the same body.

The fix

Apply → Draft in step 4, on both delivery surfaces (update-change.ts:61 skill, :152 command) and the committed skill. Step 5 keeps the write. One word; the fix @clay-good named in the issue.

I looked at also rewording step 4's Revise only files that already exist for the same reason and left it alone: it constrains which files may ever be revised rather than telling the agent to revise now, and it is the scope rule step 5 applies. Widening the diff there would change guidance the issue did not report.

Verification

• New regression tests on each surface: step 4 contains Draft the requested edit. and not Apply the requested edit, and the draft precedes Write only after the user confirms.
• Negative check: reverting Draft → Apply in src/ fails both new tests (2 failed | 7 passed) and passes again with the fix. The contract was unpinned before this PR.
• pnpm lint clean. Full suite: 4563 passed, test/core/completion-tip.test.ts 2 failed — pre-existing, reproduced on clean upstream/main with my changes stashed. Unrelated to this change.
• Parity hashes regenerated via pnpm regen:parity-hashes; skills/ regenerated via pnpm generate:skills.

One correction to the issue

Also: test/core/templates/ has no update-change.test.ts, so nothing pins this contract on either delivery surface.

test/core/templates/update-change.test.ts does exist on main (121 lines). The second half of the claim holds though — it asserts Write only after the user confirms but never that step 4 is non-writing, so the contradiction passed its checks. The new cases are appended to that file rather than to a new one.

Guidance-only; no CLI behavior changes.

Summary by CodeRabbit

• Bug Fixes

  • Updated the change-update workflow so requested edits are drafted and reviewed before writing.
  • Ensured the confirmation step remains the only point where changes are applied.

• Tests

  • Added regression coverage to verify draft-before-write behavior and instruction ordering.
  • Updated template consistency checks for the revised workflow guidance.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5c30e660-dd8f-4a92-a693-dd6246a9013e

📥 Commits

Reviewing files that changed from the base of the PR and between 9d4e597 and e6b7e4c.

📒 Files selected for processing (5)

• .changeset/update-change-step-4-drafts.md
• skills/openspec-update-change/SKILL.md
• src/core/templates/workflows/update-change.ts
• test/core/templates/skill-templates-parity.test.ts
• test/core/templates/update-change.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The update-change workflow now drafts requested edits during step 4. Step 5 remains the write gate after user confirmation. Templates, skill instructions, parity hashes, regression tests, and the patch changeset reflect this contract.

Changes

Update-change workflow

Layer / File(s)	Summary
Draft-before-write workflow contract skills/openspec-update-change/SKILL.md, src/core/templates/workflows/update-change.ts	Step 4 now drafts the requested edit before checking other artifacts. Both delivery surfaces retain step 5 as the confirmed write path.
Regression coverage and release metadata test/core/templates/update-change.test.ts, test/core/templates/skill-templates-parity.test.ts, .changeset/update-change-step-4-drafts.md	Tests verify the draft wording and its order before confirmed writing. Parity hashes are updated, and a patch changeset documents the correction.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to e6b7e

Step 4 now drafts edits and step 5 remains the confirmed write path across the delivered workflow surfaces.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (2 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: step 4 drafts the requested edit and step 5 performs the write.
Linked Issues check	✅ Passed	The changes satisfy issue [#1836]. Both workflow surfaces and the committed skill now draft the edit in step 4, preserve step 5 as the write gate, and add regression tests for this contract.
Out of Scope Changes check	✅ Passed	The changeset declaration, parity hash updates, generated skill updates, and regression tests directly support the requested workflow clarification. No unrelated code changes are present.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[choi138]

Duplicate of #1840 — closing.

I checked #1836's state (open, unassigned) but not its cross-references, so I missed that you'd already opened #1840 eight hours earlier. Entirely my error.

#1840 is also the better patch. Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write closes a gap mine left open: Draft alone fixes the verb but doesn't say where the draft goes, and step 4 still has Revise only files that already exist two bullets later, which an agent can read as license to write during step 4. Amending step 5 to say "including the requested edit drafted in step 4" is the part that actually makes the two steps one story rather than two rules that happen not to contradict. Mine touched neither, and didn't touch docs/commands.md.

Nothing here to salvage. Sorry for the noise.