fix(core): canonicalize rebuilt spec EOF - #1528
Conversation
📝 WalkthroughWalkthroughThe spec rebuild now collapses excessive blank lines, trims trailing whitespace, and adds exactly one terminal newline. End-to-end tests cover missing, LF, and CRLF inputs while preserving internal paragraph breaks and sections after ChangesSpec serialization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
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/specs-apply.ts`:
- Around line 551-552: Remove the global newline-collapsing replace from the
full rebuilt specification flow, keeping only trimEnd() plus a single trailing
newline. Add coverage verifying an internal \n\n\n sequence, including within
authored Markdown content, remains unchanged.
🪄 Autofix
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 Plus
Run ID: f43e2b65-6a61-4357-92d9-372f43d02aef
📒 Files selected for processing (2)
src/core/specs-apply.tstest/core/specs-apply.serialization.test.ts
alfred-openspec
left a comment
There was a problem hiding this comment.
Verified exact head 40fea72. A frozen install, build, lint, 249 focused archive/spec tests, and the full 3,789-test suite pass locally. The final trim runs only after reconstruction, preserves internal spacing and later sections, normalizes LF/CRLF inputs to one final LF, and closes #1527 cleanly. Approved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Merge-prep pass (maintainer): pushed one commit adding the missing changeset ( Verification on this head:
Re: the CodeRabbit comment to delete |
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-verified exact head 1b62d87. The follow-up adds only the missing patch changeset, and its description matches the tested one-final-LF behavior without overstating scope. The underlying code and regression coverage remain unchanged from the previously approved patch.
Summary
Fixes #1527
Testing
pnpm exec vitest run test/core/specs-apply.serialization.test.ts— 5 tests passedpnpm run lintpnpm run buildpnpm test— 127 test files passed, 1 skipped; 3,789 tests passed, 66 skippedgit diff --checkNotes
This changes only terminal whitespace after the complete spec has been rebuilt. Parsing, requirement ordering, and the existing internal Markdown normalization are unchanged.
AI-assisted with OpenAI Codex using a GPT-5 family model. I reviewed the patch and verification results before submission.
Summary by CodeRabbit
Bug Fixes
Tests