fix archive scenario drift for #1246

[zhangsan582]

Fix: Issue #1246

When archiving two consecutive changes that both MODIFIED the same requirement, the later archive with an older baseline will no longer silently overwrite and delete the scenario added by the previous change.

Previously, buildUpdatedSpec would replace the entire block based on the requirement name. As a result, the second change with an older baseline would delete newly added scenarios from the canonical spec.

This adds/changes/fixes archive-time drift protection: For MODIFIED blocks, it compares the #### Scenario: sections between the current spec and the incoming block. If the current spec contains existing scenarios that are missing from the incoming block, the archive process aborts and prompts the user to refresh the change spec, preventing data loss.

Changes

• Code: Added a MODIFIED scenario loss check to src/core/specs-apply.ts.
• Tests: Added a regression test case for Archiving two changes that MODIFY the same requirement silently drops scenarios (distinct from #1112) #1246 in test/core/archive.test.ts, covering the scenario where A archives first, and B's older baseline archive is aborted.
• OpenSpec: Created a local record under openspec/changes/fix-archive-modified-scenario-drift-1246/, which is excluded from the commit.

Verification

• pnpm exec vitest run test/core/archive.test.ts passed
• pnpm exec vitest run test/core/archive.test.ts test/core/parsers/requirement-blocks.test.ts passed
• pnpm exec vitest run test/commands/validate.test.ts passed
• pnpm run lint passed
• pnpm run build passed
• pnpm test was executed, but unrelated e2e timeout/EBUSY errors occurred under Windows: test/cli-e2e/basic.test.ts and one temp directory cleanup in test/commands/validate.test.ts. Relevant tests passed when re-run individually.

Commit

• Hash: cc1ac37880a7284b97408e2dc6b611d3d9d46843
• Message: fix archive scenario drift for #1246
• Scope: The commit only includes src/core/specs-apply.ts and test/core/archive.test.ts, excluding OpenSpec-related files.

---

Summary by CodeRabbit

Bug Fixes

• Prevented stale MODIFIED archive operations from silently dropping existing scenarios.

Documentation

• Added local OpenSpec proposal/design/tasks notes for the fix; intentionally excluded from commit.

Tests

• Added regression coverage for sequential archives modifying the same requirement.

Summary by CodeRabbit

• Bug Fixes

  • Prevented archive operations from silently removing existing scenarios when applying outdated modified requirement updates.
  • Archive now stops with a clear error when an incoming modified requirement is missing scenarios already present in the current spec.

• Tests

  • Added regression coverage for sequential archive operations to ensure existing scenarios are preserved and failed archives are safely aborted.

[coderabbitai]

📝 Walkthrough

Walkthrough

A scenario-drift guard is added to the MODIFIED delta application path in specs-apply.ts. It parses #### Scenario: headings from both the current canonical requirement and the incoming modified block and throws a pre-write error if any existing scenario would be silently dropped. Supporting documents (proposal, design, tasks) and a regression test for sequential archives are also added.

Changes

Scenario Drift Guard for MODIFIED Archives

Layer / File(s)	Summary
Proposal, design, and task tracking openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md, openspec/changes/fix-archive-modified-scenario-drift-1246/design.md, openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md	Three new documents define the abort condition, scenario-heading comparison strategy, scope limits, and the implementation checklist for Issue #1246.
ScenarioBlock interface and parsing helpers src/core/specs-apply.ts	Adds the ScenarioBlock interface plus parseScenarioBlocks (regex extraction of #### Scenario: sections) and findMissingCurrentScenarios (set-diff on scenario names), which the guard calls before any write.
MODIFIED guard wired into delta application loop src/core/specs-apply.ts	Replaces the has-check with get to capture currentBlock, then inserts the scenario-drift check that throws a MODIFIED failed error naming the dropped scenarios before any spec file is written.
Regression test for sequential stale MODIFIED archive test/core/archive.test.ts	New test archives two sequential changes modifying the same requirement; asserts the second archive logs the stale-block failure and "Aborted. No files were changed.", and confirms the stale directory is not moved to changes/archive.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Archiving two changes that MODIFY the same requirement silently drops scenarios (distinct from #1112) #1246: This PR directly implements the archive-time scenario-drift guard described in that issue, preventing silent scenario drops when archiving stale MODIFIED blocks.
• feat(specs-apply): error on unmatched tagged scenario ops in delta specs #847: Both touch tagged-scenario validation and missing-match error handling in src/core/specs-apply.ts.

Possibly related PRs

• Fission-AI/OpenSpec#450: Both PRs modify src/core/specs-apply.ts's delta application pipeline; #450 introduced the merge pipeline that this PR extends with the MODIFIED scenario-drift guard.

Suggested reviewers

• TabishB
• alfred-openspec

Poem

🐇 Hop, hop, hooray! No scenarios shall fall,
A guard at the gate checks them one and all.
If your MODIFIED block drops a heading unseen,
The archive says "Nope!" and keeps the spec clean.
Stale changes rebase — no silent data loss here,
The rabbit stands watch so your specs stay clear! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	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 is concise and accurately reflects the main change: fixing archive-time scenario drift for issue #1246.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md`:
- Line 7: The remaining checklist item in the task list is still unchecked even
though the PR includes proposal/design/task docs, so update the task status to
match the actual scope. Adjust the checklist entry in tasks.md by either marking
it complete if it should remain, or removing/closing it if it is no longer
applicable; use the existing checklist item text “Commit only source and test
changes” as the anchor.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 763f8388-630b-4be8-a213-143a5ab6c9fa

📥 Commits

Reviewing files that changed from the base of the PR and between 737518b and 178d2ab.

📒 Files selected for processing (5)

• openspec/changes/fix-archive-modified-scenario-drift-1246/design.md
• openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md
• openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md
• src/core/specs-apply.ts
• test/core/archive.test.ts

[alfred-openspec]

PR #1252 Review: fix archive scenario drift for #1246

Date: 2026-06-25 05:20 AEST
Author: @zhangsan582
PR: #1252

Summary

This is directionally the right shape for #1246: archive-time conflict detection is safer than silently whole-block replacing a stale MODIFIED requirement. The implementation adds a missing-current-scenario guard before nameToBlock.set(key, mod), so the reported A-then-B scenario data-loss case is blocked before the canonical spec is rewritten.

Verification

• Checked PR metadata and diff.
• Read src/core/specs-apply.ts and test/core/archive.test.ts changes.
• Ran targeted test on the PR branch:
  • pnpm exec vitest run test/core/archive.test.ts
  • Result: 25 passed.

Findings

Changes requested: committed OpenSpec change files should be removed

The PR includes:

• openspec/changes/fix-archive-modified-scenario-drift-1246/design.md
• openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md
• openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md

The PR body says these were intentionally excluded from the commit, but GitHub shows them as committed. For a narrow external bug fix, these local planning artifacts should not merge into the main repo unless maintainers explicitly want this change proposal recorded there.

Notes

• The current guard detects scenario-heading loss, not all possible stale-base drift. It can still overwrite body edits for scenarios with the same heading. That is acceptable as a scoped fix for #1246 if we treat this as a first guardrail before a future fingerprint/semantic merge design.
• The regression test exercises the actual archive command path and verifies B remains unarchived after the abort, which is the right coverage for this bug class.

Review decision

Changes requested. Remove the committed openspec/changes/fix-archive-modified-scenario-drift-1246/ files, then this looks mergeable as a scoped #1246 guard.

[zhangsan582]

thanks for catching that. I removed the mistakenly committed OpenSpec change files in the latest commit:

• openspec/changes/fix-archive-modified-scenario-drift-1246/design.md
• openspec/changes/fix-archive-modified-scenario-drift-1246/proposal.md
• openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md

The remaining diff is limited to the archive guard and regression test for #1246.

[alfred-openspec]

Thanks for cleaning that up. Re-reviewed the latest head c7e3800: the OpenSpec change docs are gone, the remaining diff is scoped to the archive guard and regression test, and pnpm exec vitest run test/core/archive.test.ts plus pnpm run lint both pass locally. Looks good to me as a scoped #1246 fix.