release: v0.43.1 - #217
Conversation
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCoverage publishing now snapshots the existing artifact before Vitest execution and reconciles it using atomic claim-and-restore behavior. Tests cover concurrent replacements, stale cleanup, Vitest directory deletion, failed runs, and first-run publishing. The package version is updated to 0.43.1. ChangesCoverage publishing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant produceCoverage
participant Vitest
participant publishCoverage
participant CoverageArtifact
produceCoverage->>CoverageArtifact: snapshotArtifact(cwd)
produceCoverage->>Vitest: run coverage suite
Vitest->>CoverageArtifact: write or remove coverage report
produceCoverage->>publishCoverage: pass pre-run snapshot
publishCoverage->>CoverageArtifact: claim stable artifact atomically
publishCoverage->>CoverageArtifact: restore or clear by mtime identity
Possibly related PRs
🚥 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 |
…1214) Found by running v0.43.1 against frink's real suite. Two independent defects in `publishCoverage`, both in the fail-CLOSED guarantee the function exists to provide — a failing run left the PREVIOUS run's report in place for the gate to trust. **1. The claim file lived inside the run directory.** On a failed run vitest's `cleanAfterRun()` deletes the reports directory once it is empty — which is exactly what a failed run leaves. Renaming into a directory vitest had just removed threw ENOENT, the catch swallowed it, and the stale artifact survived. Every unit test created the run directory by hand, so the one arrangement that actually occurs in production was never exercised. The claim now sits beside the artifact in `coverage/`, which is guaranteed to exist and on the same filesystem. **2. The wall-clock comparison was unsound.** `Date.now()` is millisecond-truncated; file mtimes carry sub-millisecond precision. An artifact written in the SAME millisecond the run started reads as "newer than the run" and was put back as if a sibling had published it — a fail-open whose likelihood depends on machine speed. Replaced with an identity check: the artifact's mtime is snapshotted before the run, and the claimed file is deleted only if it still matches. Unchanged means it is the very file this run started with. ## Tests +6 tests, including the two arrangements that were missing: the run directory being gone at clear time, and an artifact that appeared from nothing mid-run. The end-to-end test now runs a real FAILING vitest and asserts the previous report is gone — that one reproduces the field failure and fails against the old code. Full suite green: 130 files, exit 0.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gate-engine/coverage/produce.mts (1)
141-153: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSplit the restore from ownership validation
The
claim firstpart works, but lines 141-151 still let the laterstatSync/renameSyncrace: afterstatSync(identified as changed), another run can re-publish a newer report tostable, andrenameSync(claimed, stable)will silently overwrite it. The shared catch also treats restore failures fromrenameSyncas "unreadable/unrestorable" and deletesclaimedwithrmSync; wraprenameSync(claimed, stable)separately so a failed restore does not destroy an artifact whose ownership has already been determined.🤖 Prompt for 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. In `@gate-engine/coverage/produce.mts` around lines 141 - 153, Update the restore logic in the surrounding claim-handling function: validate `claimed` ownership, then re-check that `stable` is still absent or unchanged immediately before restoring to prevent overwriting a newer report. Separate `renameSync(claimed, stable)` error handling from the `statSync` validation catch, and if restoration fails, preserve `claimed` rather than deleting it with `rmSync`.
🤖 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.
Outside diff comments:
In `@gate-engine/coverage/produce.mts`:
- Around line 141-153: Update the restore logic in the surrounding
claim-handling function: validate `claimed` ownership, then re-check that
`stable` is still absent or unchanged immediately before restoring to prevent
overwriting a newer report. Separate `renameSync(claimed, stable)` error
handling from the `statSync` validation catch, and if restoration fails,
preserve `claimed` rather than deleting it with `rmSync`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 716321fd-a2a6-41b0-aae3-7c604bdc822e
⛔ Files ignored due to path filters (8)
dist/gate-engine/coverage/produce.mjsis excluded by!**/dist/**dist/gate-engine/decisions/check-alignment.mjsis excluded by!**/dist/**dist/gate-engine/decisions/detect.mjsis excluded by!**/dist/**dist/gate-engine/decisions/git-io.mjsis excluded by!**/dist/**dist/gate-engine/judge/run-judge.mjsis excluded by!**/dist/**dist/gate-engine/review/completeness.mjsis excluded by!**/dist/**dist/gate-engine/review/run-review.mjsis excluded by!**/dist/**dist/package.jsonis excluded by!**/dist/**
📒 Files selected for processing (4)
docs/decisions/coverage-gate.mdgate-engine/coverage/__tests__/produce.test.mtsgate-engine/coverage/produce.mtspackage.json
1 participant
Patch bump 0.43.0 → 0.43.1 so consumers can pin the
devkit coverage-runrunner from #215.Tag
v0.43.1is already pushed (that is whatdevkit releasedoes) and points at the branch commitfdbc717. Note it will sit off main once this squash-merges — flagged rather than hidden, since every earlier tag sits on main. Content is identical either way; frink is being upgraded against this tag now.Full suite green: 2130 passed.
Summary by CodeRabbit
Bug Fixes
Documentation
Chores