Fixes repeated publish of review.md and description#13
Conversation
📝 WalkthroughWalkthroughSeparates summary publish-state from review pending-state, updates publish and status command flows, and changes CLI error output formatting with matching test coverage. ChangesOutput State and CLI Publishing Adjustments
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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
🧹 Nitpick comments (1)
CHANGELOG.md (1)
11-12: 📐 Maintainability & Code Quality | ⚡ Quick winImprove readability by varying the opening words.
Both entries start with "Fixed," which the static analysis tool flagged as repetitive. Consider restructuring to improve flow while maintaining clarity and specificity.
📝 Suggested restructuring (example)
- Fixed `revpack publish` leaving `review.md` populated after publishing, which could republish the same review note during later incremental reviews. - Fixed `revpack publish all` updating the PR/MR description summary again when `revpack status` already reported the summary as published. + Fixed `revpack publish` leaving `review.md` populated after publishing, which could republish the same review note during later incremental reviews. + Prevented `revpack publish all` from redundantly updating the PR/MR description summary when `revpack status` already reported it as published.Alternatively, combine under a shared prefix:
- Fixed `revpack publish` leaving `review.md` populated after publishing, which could republish the same review note during later incremental reviews. - Fixed `revpack publish all` updating the PR/MR description summary again when `revpack status` already reported the summary as published. + Fixed issues with redundant publishing: `revpack publish` no longer leaves `review.md` populated (preventing republishing during incremental reviews), and `revpack publish all` no longer re-updates the PR/MR description summary when already published.🤖 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 `@CHANGELOG.md` around lines 11 - 12, Change the two CHANGELOG entries so they don't both start with "Fixed" to improve readability; update the first bullet about "revpack publish" (mentioning review.md) to begin with a different verb like "Prevented" or "Avoided" and reword the second bullet about "revpack publish all" (mentioning PR/MR description summary and revpack status) to start with "Stop" or "Avoid" or combine both under a single prefix (e.g., "Fixes:") while keeping the existing specifics about review.md and the summary behavior intact.Source: Linters/SAST tools
🤖 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/cli/commands/publish.ts`:
- Around line 421-423: The code unconditionally calls clearDefaultReviewOutput
for the default review file which can erase user changes even when a publish
failed; update publishReviewCmd so that after computing isDefaultReviewFile
(from workspacePath(filePath) === workspacePath(DEFAULT_REVIEW_FILE)) you only
call await clearDefaultReviewOutput(isDefaultReviewFile) when result.created is
true (i.e., wrap the clearDefaultReviewOutput call in a guard that checks
result.created && isDefaultReviewFile) so the default review is only cleared on
successful creation.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 11-12: Change the two CHANGELOG entries so they don't both start
with "Fixed" to improve readability; update the first bullet about "revpack
publish" (mentioning review.md) to begin with a different verb like "Prevented"
or "Avoided" and reword the second bullet about "revpack publish all"
(mentioning PR/MR description summary and revpack status) to start with "Stop"
or "Avoid" or combine both under a single prefix (e.g., "Fixes:") while keeping
the existing specifics about review.md and the summary behavior intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3295dce6-6c7e-4b61-8874-19ec3193d646
📒 Files selected for processing (9)
CHANGELOG.mdsrc/cli/commands/publish.test.tssrc/cli/commands/publish.tssrc/cli/commands/status.tssrc/core/types.tssrc/orchestration/orchestrator.test.tssrc/providers/local/local-git-provider.test.tssrc/workspace/workspace-manager.test.tssrc/workspace/workspace-manager.ts
This allows us to dim some commands, if they are not primary
This reverts commit cba68d4.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/cli/commands/status.ts`:
- Around line 274-276: The fresh-state message in status handling is hardcoded
to PR, which makes non-PR targets show the wrong label even though targetKind is
already available. Update the success branch in the status command’s bundle
freshness formatting to use targetKind consistently in the “current” message,
matching how the stale branch already does, so the output stays target-agnostic.
- Around line 279-295: In compareCheckoutToTargetHead, the two isAncestor()
checks can misclassify a checkout as diverged when comparisonTargetHead is not
present locally, because both probes fall back to false; add a local
commit-existence check for comparisonTargetHead (and any required target/current
refs) before running ancestry comparisons, and treat missing target data as an
unknown/behind case instead of returning diverged so the downstream “Next”
guidance stays correct.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc69c383-48ae-4faa-8271-0882726452f6
📒 Files selected for processing (4)
CHANGELOG.mdsrc/cli/commands/status.test.tssrc/cli/commands/status.tssrc/workspace/git-helper.ts
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Summary by CodeRabbit
Bug Fixes
UI / UX
statusnow more accurately reflects pending review state and improves “Next” guidance based on local checkout relation (including when local commits are ahead).prepareoutput formatting and guidance prompts were updated for clarity.Fixed
review.mdonly after a review note is actually created.--replaceand custom-summary publishes.Tests