Skip to content

Fixes repeated publish of review.md and description#13

Merged
stefanvictora merged 12 commits into
mainfrom
cli-fixes
Jun 26, 2026
Merged

Fixes repeated publish of review.md and description#13
stefanvictora merged 12 commits into
mainfrom
cli-fixes

Conversation

@stefanvictora

@stefanvictora stefanvictora commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Bug Fixes

    • Publishing review now clears the default review file after success and prevents republishing the same review note in later runs.
    • Publishing description/summary avoids re-publishing identical content while still allowing explicit replace or custom-summary updates.
    • CLI debug output no longer duplicates the user-facing error message when showing stack traces.
  • UI / UX

    • status now more accurately reflects pending review state and improves “Next” guidance based on local checkout relation (including when local commits are ahead).
    • prepare output formatting and guidance prompts were updated for clarity.

Fixed

  • Fixes repeated publishing of default review notes by clearing review.md only after a review note is actually created.
  • Avoids re-publishing the default summary when it is already recorded as published, while preserving explicit --replace and custom-summary publishes.
  • Improves status and prepare guidance for local checkout/head mismatch cases, including provider-specific target wording and unknown local commit comparisons.
  • Removes repeated debug error text before stack frames.

Tests

  • Adds coverage for review-note cleanup, summary publish skipping/replacement, checkout guidance, and debug error output.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Separates summary publish-state from review pending-state, updates publish and status command flows, and changes CLI error output formatting with matching test coverage.

Changes

Output State and CLI Publishing Adjustments

Layer / File(s) Summary
Output state contracts and pending semantics
src/core/types.ts, src/workspace/workspace-manager.ts, src/workspace/workspace-manager.test.ts, src/orchestration/orchestrator.test.ts, src/providers/local/local-git-provider.test.ts
BundleOutputs.summary now uses a hash-tracked entry type; workspace state tracks summary publish hashes separately from review pending content; tests update to the new summary/review behavior.
Publish command behavior and validation
src/cli/commands/publish.ts, src/cli/commands/publish.test.ts
Publishing now clears the default review output after successful review publishing and skips default summary publishing when the recorded hash matches; tests cover review clearing, summary hash checks, and custom-file paths.
Status and prepare output updates
src/cli/commands/status.ts, src/cli/commands/status.test.ts, src/cli/commands/prepare.ts, src/orchestration/orchestrator.ts
Status now reads review pending state and uses checkout relation output for next-step guidance; prepare output text is reformatted; one orchestrator error message newline changes.
Debug error output formatting and tests
src/cli/helpers.ts, src/cli/helpers.test.ts, CHANGELOG.md
handleError prints stack traces differently in DEBUG mode, tests cover DEBUG and non-DEBUG behavior, and the changelog records the related fixes.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 I hop through hashes and notes so bright,
Summary stays put, review takes flight.
Stack traces whisper when DEBUG is near,
And status lines prance with cleaner cheer.
Hoppy changes all around!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the PR’s main change: preventing repeated publication of review notes and the default description.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
CHANGELOG.md (1)

11-12: 📐 Maintainability & Code Quality | ⚡ Quick win

Improve 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

📥 Commits

Reviewing files that changed from the base of the PR and between cba9ed3 and 098a51d.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • src/cli/commands/publish.test.ts
  • src/cli/commands/publish.ts
  • src/cli/commands/status.ts
  • src/core/types.ts
  • src/orchestration/orchestrator.test.ts
  • src/providers/local/local-git-provider.test.ts
  • src/workspace/workspace-manager.test.ts
  • src/workspace/workspace-manager.ts

Comment thread src/cli/commands/publish.ts
Comment thread src/cli/commands/publish.ts Outdated
@stefanvictora

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0ef1fa3 and 82f5f14.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/cli/commands/status.test.ts
  • src/cli/commands/status.ts
  • src/workspace/git-helper.ts
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md

Comment thread src/cli/commands/status.ts Outdated
Comment thread src/cli/commands/status.ts Outdated
Comment thread src/cli/commands/status.ts Outdated
@stefanvictora stefanvictora merged commit 716a363 into main Jun 26, 2026
2 checks passed
@stefanvictora stefanvictora deleted the cli-fixes branch June 26, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant