fix archive scenario drift for #1246 by zhangsan582 · Pull Request #1252 · Fission-AI/OpenSpec

zhangsan582 · 2026-06-24T15:54:16Z

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 · 2026-06-24T15:56:29Z

📝 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

_{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 · 2026-06-26T14:50:34Z

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.

Propose a deterministic CLI backstop so a spec-driven change cannot archive while silently dropping the specs it declared. Root cause (confirmed): `openspec validate` runs validateChangeDeltaSpecs unconditionally and rejects a change with no deltas, but `openspec archive` gates the same check behind `if (hasDeltaSpecs)`, so a change with no specs/ skips validation and archives clean — the proposal's no-delta error is downgraded to a non-blocking warning. Fix: - Archive runs delta-spec validation unless --skip-specs (parity with validate); a no-delta change is blocked with CHANGE_NO_DELTAS. - instructions apply exits non-zero when blocked. - spec-driven apply.requires includes specs. - Archive skill prompt aligned with the new blocking behavior. Closes Fission-AI#1212, Fission-AI#1260, Fission-AI#1222, Fission-AI#1264, Fission-AI#799. Supersedes PRs Fission-AI#1250, Fission-AI#1271, Fission-AI#1241, Fission-AI#1233. References (out of scope) Fission-AI#1246, Fission-AI#1112, Fission-AI#1252. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Propose one deterministic completeness check, enforced in the validator and shared by `validate`, CI, and `archive`, so a spec-driven change cannot archive while silently dropping the specs it declared. Two silent failure modes, both closed: - Total drop: archive gated delta-spec validation behind `if (hasDeltaSpecs)`, so a change with no specs/ skipped the CHANGE_NO_DELTAS check that `validate` already enforces. Fix: validate unless --skip-specs (validate/archive parity). - Partial drop: nothing checked the proposal's declared capabilities against delivered delta specs. Fix: deterministic validateChangeCapabilityCoverage — every capability under `## Capabilities` must have specs/<id>/spec.md with a delta section. Parsing contract validated against all 93 in-repo proposals (found 7 real proposal/spec inconsistencies; zero false extractions). Plus earlier guardrails (incorporates Fission-AI#1250): apply exits non-zero when blocked; spec-driven apply.requires includes specs. Closes Fission-AI#1212, Fission-AI#1260, Fission-AI#1222, Fission-AI#1264, Fission-AI#799. Supersedes PRs Fission-AI#1250, Fission-AI#1271, Fission-AI#1241, Fission-AI#1233. References (out of scope) Fission-AI#1246, Fission-AI#1112, Fission-AI#1252. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its real root: one architectural fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — expressed at four layers. Fix, ordered by leverage: - Deterministic CLI gate: archive validates like `validate` unless --skip-specs, schema-aware (only when the schema graph has a `specs` artifact, per Fission-AI#997). Blocks total drop (CHANGE_NO_DELTAS), partial drop (new schema-aware capability-coverage rule), and format/mode-G (present non-delta spec → "No delta sections found"). - Loop fix: spec-driven apply.requires [specs, tasks] + propose/ff walk the full schema build order (incorporates Fission-AI#1250; Fission-AI#1250's reviewer explicitly deferred this archive guard). - Keystone: the archive skill calls `openspec archive` CLI instead of agent-judged sync + raw mv, so the CLI guarantees are reachable by agents (Fission-AI#656, Fission-AI#863). - Recovery audit for already-archived drift; delta-vs-full-spec clarity. Closes Fission-AI#1212 Fission-AI#1260 Fission-AI#1222 Fission-AI#1264 Fission-AI#799 Fission-AI#656 Fission-AI#863 Fission-AI#164. Supersedes PRs Fission-AI#1250 Fission-AI#1271 Fission-AI#1241 Fission-AI#1233. Addresses-in-part Fission-AI#997 Fission-AI#194 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 Fission-AI#913 Fission-AI#1120. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its root: one fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — expressed at four layers, fixed at the most deterministic point for each. - CLI gate (guarantee): archive validates like `validate` unless --skip-specs, schema-aware (only when the schema graph produces delta specs, per Fission-AI#997). Blocks total (CHANGE_NO_DELTAS), partial (new capability-coverage rule), and format/mode-G ("No delta sections"). - Loop fix: spec-driven apply.requires [specs, tasks] + propose/ff create every artifact transitively required by applyRequires (incorporates Fission-AI#1250; its reviewer deferred this archive guard). - Keystone: archive skill calls `openspec archive` instead of agent-judged sync + raw mv (Fission-AI#656, Fission-AI#863); also removes the sync-specs skill dependency (Fission-AI#913). - Recovery audit; delta-vs-full-spec clarity. Verified against current main (rebuilt CLI): bug reproduces; archive --json + ArchiveBlockedError exist; Fission-AI#1250 not merged; Fission-AI#194/Fission-AI#1268 closed. Closes Fission-AI#1212 Fission-AI#1260 Fission-AI#1222 Fission-AI#1264 Fission-AI#799 Fission-AI#656 Fission-AI#863 Fission-AI#913. Supersedes PRs Fission-AI#1250 Fission-AI#1271 Fission-AI#1241 Fission-AI#1233. Addresses Fission-AI#997 Fission-AI#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its root: one fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — fixed at the most deterministic point for each of four layers. - CLI gate (guarantee): ONE shared schema-aware predicate (schemaProducesDeltaSpecs) gates the delta requirement in BOTH `validate` and `archive`, so they agree by construction and proposal-only schemas pass both while spec-driven-with-no-specs fails both (Fission-AI#997). Blocks total (CHANGE_NO_DELTAS), partial (capability coverage), and format/mode-G ("No delta sections"). `--yes` never bypasses the gate. - Loop fix: spec-driven apply.requires [specs, tasks] + propose/ff create every artifact transitively required by applyRequires (incorporates Fission-AI#1250). - Keystone: archive skill calls `openspec archive` instead of agent-judged sync + raw mv (Fission-AI#656, Fission-AI#863); removes the sync-specs skill dependency (Fission-AI#913). - Recovery audit; delta-vs-full-spec clarity. Addresses review feedback (alfred-openspec: validate needs the same schema-aware gate as archive; coderabbit: document --yes does not bypass, fenced-block tags, clarify partial detection, fail-open/--yes tests). Closes Fission-AI#1212 Fission-AI#1260 Fission-AI#1222 Fission-AI#1264 Fission-AI#799 Fission-AI#656 Fission-AI#863 Fission-AI#913. Supersedes PRs Fission-AI#1250 Fission-AI#1271 Fission-AI#1241 Fission-AI#1233. Addresses Fission-AI#997 Fission-AI#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its root: one fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — fixed at the most deterministic point for each of four layers. CLI gate (guarantee): one shared schema-aware predicate (schemaProducesDeltaSpecs, memoized per run) gates the delta requirement in BOTH validate and archive; blocks total/partial/format drops; proposal-only schemas pass both (Fission-AI#997). Loop: spec-driven apply.requires [specs, tasks] (apply gate is non-transitive — design stays optional). Keystone: ALL archive templates (skill + command + bulk) call `openspec archive` instead of agent-judged sync + raw mv (Fission-AI#656, Fission-AI#863); never --skip-specs/--no-validate. Recovery: specced archived-drift audit (forward-only). Clarity: delta-vs-full spec + stricter Capabilities-section contract. Incorporates three review rounds (alfred-openspec, CodeRabbit) and an internal adversarial pass: validate schema-aware parity, --yes doesn't bypass, all archive surfaces reworked, non-transitive apply gate, schema-resolution memoization, capability deletion/rename + bold-label/non-kebab handling, proposal-only fixture for Fission-AI#997 tests. Closes Fission-AI#1212 Fission-AI#1260 Fission-AI#1222 Fission-AI#1264 Fission-AI#799 Fission-AI#656 Fission-AI#863 Fission-AI#913. Supersedes PRs Fission-AI#1250 Fission-AI#1271 Fission-AI#1241 Fission-AI#1233. Addresses Fission-AI#997 Fission-AI#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 (complementary; land first) Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Deterministic, CLI-enforced prevention of silent spec drop in the spec-driven workflow, addressing one root fault (correctness decisions in agent prompts) across four layers: schema-aware delta gate shared by validate+archive (Fission-AI#997), the non-transitive apply-gate loop fix (incorporates Fission-AI#1250), the keystone (all archive templates call `openspec archive` — Fission-AI#656/Fission-AI#863), and a specced archived-drift audit. Hardened across three review rounds (alfred-openspec, CodeRabbit), an internal adversarial pass, and an open-PR coordination scan: - reconciles Fission-AI#977 (allow-specless) via the schema-aware gate + --skip-specs, deliberately not allowing silent specless archives under spec-driven; - coordinates with Fission-AI#902 (propose/ff spec discovery), the unify-template-generation-pipeline manifest, add-change-stacking-awareness (provides/touches markers + overlap warnings), and add-artifact- regeneration-support (complementary staleness); - rebases onto approved Fission-AI#1186/Fission-AI#1151/Fission-AI#1153/Fission-AI#1252; Fission-AI#1252 is a prerequisite. Closes Fission-AI#1212 Fission-AI#1260 Fission-AI#1222 Fission-AI#1264 Fission-AI#799 Fission-AI#656 Fission-AI#863 Fission-AI#913. Supersedes PRs Fission-AI#1250 Fission-AI#1271 Fission-AI#1241 Fission-AI#1233. Addresses Fission-AI#997 Fission-AI#977 Fission-AI#902 Fission-AI#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252(prereq) Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

zhangsan582 added 2 commits June 24, 2026 23:48

fix archive scenario drift for Fission-AI#1246

cc1ac37

fix archive scenario drift for Fission-AI#1246

178d2ab

zhangsan582 requested a review from TabishB as a code owner June 24, 2026 15:54

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread openspec/changes/fix-archive-modified-scenario-drift-1246/tasks.md Outdated

alfred-openspec requested changes Jun 24, 2026

View reviewed changes

remove local openspec change docs

c7e3800

alfred-openspec approved these changes Jun 27, 2026

View reviewed changes

clay-good mentioned this pull request Jun 29, 2026

fix(cli): prevent silent spec drop in the spec-driven workflow #1277

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix archive scenario drift for #1246#1252

fix archive scenario drift for #1246#1252
zhangsan582 wants to merge 3 commits into
Fission-AI:mainfrom
zhangsan582:feature_0623_2

zhangsan582 commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

alfred-openspec left a comment

Uh oh!

zhangsan582 commented Jun 26, 2026

Uh oh!

alfred-openspec left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zhangsan582 commented Jun 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix: Issue #1246

Changes

Verification

Commit

Summary by CodeRabbit

Bug Fixes

Documentation

Tests

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alfred-openspec left a comment

Choose a reason for hiding this comment

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

Summary

Verification

Findings

Changes requested: committed OpenSpec change files should be removed

Notes

Review decision

Uh oh!

zhangsan582 commented Jun 26, 2026

Uh oh!

alfred-openspec left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zhangsan582 commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading