Fix OpenSpec Archive Structure in Archive Skill - #1759
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe archive workflow now verifies main-spec structure after synchronization. The skill and command templates contain the same verification rules. Template parity tests update expected hashes and assert the new contract. ChangesArchive sync verification
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Archive synchronization now verifies and preserves the intended main-spec structure, including Purpose and Requirements formatting. The generated skill and template parity coverage were updated consistently, with no current merge-blocking risk identified. 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.
Requesting changes at 6974b14.
The source/generated parity and Purpose ownership issues from #1742 are fixed. Two remaining blockers:
-
The new delta-header check omits ## RENAMED Requirements. That header is part of the canonical sync delta format, so a leaked rename section could pass this verification even though the PR claims all delta-only headers are excluded. Cover ADDED, MODIFIED, REMOVED, and RENAMED, ideally with the exact canonical headings rather than partial prefixes.
-
The only test change updates golden hashes. Those prove that text changed and generated artifacts match, not that this behavior contract stays intact. Add semantic assertions for both archive surfaces in the existing gates-the-archive-on-a-completed-spec-sync test: existing Purpose stays untouched; a new Purpose is copied verbatim or gets the TBD fallback; the main-spec title, Requirements, Requirement, and Scenario structure is required; and every delta header, including RENAMED, is rejected.
I approved the hosted workflows, and CI is running. Re-request review once these two points land.
|
Thanks @alfred-openspec for review. I had addressed the review comments and changes were pushed. |
alfred-openspec
left a comment
There was a problem hiding this comment.
Approved at e9269c5. Both requested blockers are resolved: the archive contract now rejects all four canonical delta headers, including RENAMED, and the parity test asserts the structure and Purpose rules for both archive surfaces. The focused test and build pass locally. The branch currently conflicts with main, so please update it and make sure the newly approved CI and Security runs finish green.
alfred-openspec
left a comment
There was a problem hiding this comment.
Approved at 0ec18a1. The merge from main preserved the archive changes and refreshed the unrelated parity hashes. Both prior blockers remain resolved: all four canonical delta headers are rejected, and both archive surfaces are pinned to the Purpose and main-spec structure contract. A frozen install, build, all 26 focused parity tests, and the full hosted CI/Security matrix pass. The canonical docs-lab skill contract remains accurate because this tightens internal post-sync validation without changing the documented arguments, outputs, or response.
Reason for Change:
Previous archiving actions using AI resulted in inconsistent spec formatting, outdated delta headers and missing purpose summaries. This update standardizes the output format to match expected OpenSpec conventions, ensuring clean, predictable, and parseable specifications across all archived changes.
Overview
Addresses maintainer review feedback on #1742 by moving workflow customization out of generated artifacts and into source templates, preserving existing ## Purpose semantics, and aligning post-sync main spec validation with canonical OpenSpec contracts.
Key Changes
For existing main specs, ## Purpose is authoritative and remains completely untouched.
For new main specs, the delta ## Purpose is copied verbatim (issuing a warning only if below expected length), preventing accidental AI rewrites of authored prose.
Verification
Summary by CodeRabbit
Bug Fixes
Tests