Clarify archive sync for new specs#1271
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTwo archive workflow template functions gain explicit guidance for delta specs when the corresponding main spec is missing. Parity test hashes are updated, and a new test asserts the added instruction strings are present. ChangesArchive workflow missing main spec guidance
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the archive-change workflow templates so agent-driven archive instructions explicitly handle delta specs whose target main spec does not yet exist (aligning guidance with the CLI behavior described in #1264).
Changes:
- Extends the archive workflow template instructions with a missing-main-spec decision branch for
ADDED/MODIFIED/RENAMED/REMOVEDrequirement ops. - Updates template parity hashes and adds a regression test asserting both the skill and
/opsx:archivetemplates contain the new guidance.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/core/templates/skill-templates-parity.test.ts | Updates expected template hashes and adds regression assertions for missing-main-spec guidance (but currently has a backtick-matching bug in the new assertions). |
| src/core/templates/workflows/archive-change.ts | Adds explicit instructions for how to handle delta specs when the corresponding main spec is missing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(content).toContain('If a corresponding main spec does not exist yet'); | ||
| expect(content).toContain('ADDED` requirements as sync work that will create a new main spec'); | ||
| expect(content).toContain('MODIFIED` or `RENAMED` requirements as blocking errors'); | ||
| expect(content).toContain('Do not skip sync just because the target main spec is missing'); |
There was a problem hiding this comment.
he new assertions for the archive templates have mismatched backtick quoting: they look for "ADDED"/"MODIFIED" (missing the leading backtick), but the templates include markdown code spans like "ADDED" and "MODIFIED". As written, this test will fail even when the templates are correct.
There was a problem hiding this comment.
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 `@src/core/templates/workflows/archive-change.ts`:
- Around line 67-72: The missing-main-spec handling in archive-change.ts still
allows the generic "Archive without syncing" flow to continue even when
`MODIFIED` or `RENAMED` requirements are marked as blocking errors. Update the
logic around the missing-main-spec analysis in the workflow template so that the
archive/skip path is explicitly disabled when those blocking deltas are present,
and only allow proceed-with-archive behavior for cases like `ADDED` or `REMOVED`
that are allowed by the rules. Use the existing missing-main-spec decision block
and any sync/archive gating branch in archive-change.ts to enforce this stop
condition.
🪄 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: 5cd9e663-2d9b-42ff-9a6b-2a0938be16b0
📒 Files selected for processing (2)
src/core/templates/workflows/archive-change.tstest/core/templates/skill-templates-parity.test.ts
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>
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>
Summary
Closes #1264 by making archive workflow instructions explicitly handle delta specs whose target main spec does not exist yet.
Changes
/opsx:archivecommand templates.ADDEDrequirements still need sync and create a new main spec.MODIFIED/RENAMEDas blocking for a new spec, andREMOVEDas ignored with a warning.Validation
./node_modules/.bin/vitest run test/core/templates/skill-templates-parity.test.tsnode build.js./node_modules/.bin/eslint src/core/templates/workflows/archive-change.tsSummary by CodeRabbit
Bug Fixes
ADDEDrequirements now proceed as sync work to create a new main spec;MODIFIED/RENAMEDrequirements are treated as blocking errors.REMOVEDrequirements are ignored with a warning; sync is no longer skipped due to a missing target, and the workflow will stop when blocking errors are present (without offering “archive without syncing”).Tests