Skip to content

Fix Bunny review CI state handling#1925

Merged
Promansis merged 5 commits into
Pasta-Devs:refactorfrom
Promansis:fix/bunny-review-ci
Jun 2, 2026
Merged

Fix Bunny review CI state handling#1925
Promansis merged 5 commits into
Pasta-Devs:refactorfrom
Promansis:fix/bunny-review-ci

Conversation

@Promansis

Copy link
Copy Markdown
Collaborator

Summary

Fixes Bunny Review CI state handling, PR status reporting, and draft PR coverage.

  • keeps incremental review pinned to the actual current PR head before building the review packet
  • ignores failed/unexamined Bunny comments when choosing the last reviewed SHA
  • falls back to full review after force-push when the previous reviewed SHA is no longer an ancestor
  • posts a Bunny Review commit status on the PR head instead of relying on a custom check-run
  • retries one schema-repair turn when the model response is not JSON
  • renders failed model reviews as unexamined rather than implying clean no-findings review
  • removes the implementation-handoff prose from repair prompts
  • changes the review note label from Commit to Head, and includes a subject on Base when resolvable
  • allows automatic Bunny dispatch on draft PRs targeting refactor

Why

The previous workflow could stamp the latest PR SHA while reviewing an older checkout, advance incremental state after failed model runs, and fail to show Bunny as a PR-head status/check. Draft PRs were also skipped by the auto-dispatch gate, which meant Bunny would not run until the PR became ready for review.

Validation

Local:

  • python3 -m py_compile .github/bunny-review/bunny_review.py
  • Python AST parse for .github/bunny-review/bunny_review.py
  • local render harness for failed-review state, nitpick validation, repair-prompt text, and metadata labels
  • node scripts/check-workflow-names.mjs
  • git diff --check

Live fork test PRs targeting Promansis:fix/bunny-review-ci:

Upstream note

The code and workflow are repository-agnostic, but automatic dispatch still depends on upstream Actions token policy. If upstream has read-only workflow tokens, the pull_request_target dispatcher can still be blocked from gh workflow run until upstream allows write tokens or uses an app/PAT-backed dispatch path.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d6d4d6cd-b25c-4e93-9dd5-ac127a4524dd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@github-actions github-actions Bot added bugfix Bug fix ci labels Jun 2, 2026
@Promansis Promansis marked this pull request as ready for review June 2, 2026 10:44
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

✅ Bunny Review Completed

Tip

Review posted. The specimen has left the observation table.

  • Commit: 711aa59 - Allow Bunny review on draft PRs

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

🐰 Bunny Review

Warning

Anomalies remain. 3 finding(s): ⚠️ 1 medium, ℹ️ 2 low
Examine the findings and warning rows before merge.

Note

Mode: full
Commit: 711aa59 - Allow Bunny review on draft PRs
Base: origin/refactor

🧭 Specimen Summary

  • The specimen attempts to harden Bunny Review’s control loop: malformed model JSON gets a repair pass, failed reviews stop advertising themselves as completed state, PR heads are rechecked before inspection, and custom check-runs are replaced with commit statuses.
  • The experiment is interesting because the workflow now contains two head-SHA authorities, and one of them quietly keeps the keys to GitHub’s status API.
  • A tiny Rust storage-test formatting change tags along as harmless connective tissue; the reactive chemistry lives in the CI automation.

🔎 Isolated Defects

Severity Location Finding
⚠️ MEDIUM .github/workflows/bunny-review.yml:115 Head resync leaves status pinned to the old SHA
ℹ️ LOW src-tauri/src/http_dispatch.rs:1147 Test helper masks deleted connection rows
ℹ️ LOW src-tauri/src/http_dispatch.rs:1420 Default preset guard now ignores legacy default shape
🤖 Repair prompt for isolated Bunny findings
Use this as an implementation handoff, not as reviewer prose. Keep the response concise, technical, and direct.

Task: verify and repair `.github/workflows/bunny-review.yml` around line 115.
Finding: Head resync leaves status pinned to the old SHA
Severity: medium
Suggested repair: After the resync, export the corrected `HEAD_SHA` back into `GITHUB_ENV`, or have every status publication use the same freshly resolved SHA as the review execution.
Validate the fix with the narrowest relevant check.
If the finding is stale, leave the code unchanged and record why.

Task: verify and repair `src-tauri/src/http_dispatch.rs` around line 1147.
Finding: Test helper masks deleted connection rows
Severity: low
Suggested repair: Require the connection row to exist in the helper, then read `defaultForAgents`; absence should fail the test rather than masquerade as a cleared flag.
Validate the fix with the narrowest relevant check.
If the finding is stale, leave the code unchanged and record why.

Task: verify and repair `src-tauri/src/http_dispatch.rs` around line 1420.
Finding: Default preset guard now ignores legacy default shape
Severity: low
Suggested repair: Restore a legacy-shape assertion or add a second remote update test where the preset is protected by `default: true` rather than only `isDefault: true`.
Validate the fix with the narrowest relevant check.
If the finding is stale, leave the code unchanged and record why.

✅ Control Checks

Status Check Detail
✅ PASS CI Status Expected CI controls completed without a reported failure.
⚠️ WARN Rust dispatch coverage The added tests target remote storage invariants, but one helper masks destructive row loss and one fixture no longer exercises the legacy default preset shape.

❓ Open Questions

  • One wonders whether draft-review dispatch was designed, or merely survived the edit that added converted_to_draft.

🧪 Observations

  • Inspected the bounded diff for .github/bunny-review/bunny_review.py, both Bunny workflows, and the focused Rust storage test file.
  • Checked Bunny path rules for workflow automation, privileged storage contracts, architecture boundaries, and concrete CI behavior.
  • Traced PR head flow from initial checkout through shell resync, Python ensure_local_head, render/post metadata, and final commit status publication.
  • Ran the skeptical lens over head/base invariants, status/comment drift, failed-review state markers, and dispatch eligibility.
  • The full patch overview was truncated, so the verdict is bounded to the supplied per-file patches and identifier context.
  • Established review base origin/refactor at merge base 09da5d6a07c340c958f56bc72fad7e75610a1e98 and head 711aa59b0ad20c56cb15be27000cc752bb92008a from the packet.

📝 Reviewer Notes

Warning

Withheld 1 model finding(s) because their diff locations failed validation.

🧰 CI Status

  • Rust Capability Layer: success
  • Browser Smoke and Performance: success
  • Frontend, Architecture, and Organization: success

@github-actions github-actions 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.

Bunny Review inline findings

BUNNY_REVIEW_SKILL_PATH: /tmp/bunny-review-tool/.github/bunny-review/SKILL.md
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
HEAD_SHA=$(gh pr view "$PR_NUM" --json headRefOid -q .headRefOid)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ MEDIUM: Head resync leaves status pinned to the old SHA

Location: .github/workflows/bunny-review.yml:115

A fascinating split-brain specimen: the workflow re-reads the live PR head and may checkout that newer commit before producing the review, but the exported PR_HEAD_SHA used for commit statuses remains the earlier value from the previous step. If the PR advances between checkout and review, Bunny can inspect and comment on one commit while marking another commit as reviewed. The real head is then left without the status the branch protection organism expects.

Tip

Suggested fix: After the resync, export the corrected HEAD_SHA back into GITHUB_ENV, or have every status publication use the same freshly resolved SHA as the review execution.

🤖 Repair prompt for agents
Use this as an implementation handoff, not as reviewer prose. Keep the response concise, technical, and direct.

Task: verify and repair `.github/workflows/bunny-review.yml` around line 115.
Finding: Head resync leaves status pinned to the old SHA
Severity: medium
Suggested repair: After the resync, export the corrected `HEAD_SHA` back into `GITHUB_ENV`, or have every status publication use the same freshly resolved SHA as the review execution.
Validate the fix with the narrowest relevant check.
If the finding is stale, leave the code unchanged and record why.

})
}

fn default_for_agents(state: &AppState, id: &str) -> bool {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ LOW: Test helper masks deleted connection rows

Location: src-tauri/src/http_dispatch.rs:1147

A charming little anesthetic: default_for_agents converts a missing connection row into false, exactly the value expected for the demoted default. If a future remote create or update regression deletes language-a instead of clearing defaultForAgents, these tests would applaud absence as success. The prior direct row assertions at least demanded that the specimen still exist.

Tip

Suggested fix: Require the connection row to exist in the helper, then read defaultForAgents; absence should fail the test rather than masquerade as a cleared flag.

🤖 Repair prompt for agents
Use this as an implementation handoff, not as reviewer prose. Keep the response concise, technical, and direct.

Task: verify and repair `src-tauri/src/http_dispatch.rs` around line 1147.
Finding: Test helper masks deleted connection rows
Severity: low
Suggested repair: Require the connection row to exist in the helper, then read `defaultForAgents`; absence should fail the test rather than masquerade as a cleared flag.
Validate the fix with the narrowest relevant check.
If the finding is stale, leave the code unchanged and record why.

json!({
"id": "default-chat-preset",
"name": "Default Chat",
"mode": "chat",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ LOW: Default preset guard now ignores legacy default shape

Location: src-tauri/src/http_dispatch.rs:1420

A curious narrowing of the experiment: the remote default-preset mutation test now seeds only isDefault: true, while the removed fixture also exercised the legacy default: true shape. Adjacent readers still treat isDefault ?? default as the contract, so a hostable storage_update path that forgets legacy defaults could mutate protected presets without this test noticing. The guard may be healthy; this specimen simply stopped proving the older blood type still matches.

Tip

Suggested fix: Restore a legacy-shape assertion or add a second remote update test where the preset is protected by default: true rather than only isDefault: true.

🤖 Repair prompt for agents
Use this as an implementation handoff, not as reviewer prose. Keep the response concise, technical, and direct.

Task: verify and repair `src-tauri/src/http_dispatch.rs` around line 1420.
Finding: Default preset guard now ignores legacy default shape
Severity: low
Suggested repair: Restore a legacy-shape assertion or add a second remote update test where the preset is protected by `default: true` rather than only `isDefault: true`.
Validate the fix with the narrowest relevant check.
If the finding is stale, leave the code unchanged and record why.

@Promansis Promansis merged commit 17a1efc into Pasta-Devs:refactor Jun 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant