fix(review): request changes for failed pre-merge checks #38

Merged

jwilger merged 1 commit from fix/issue-7-premerge-request-changes into main

2026-05-02 12:41:51 -07:00

jwilger commented

2026-05-02 12:38:50 -07:00

Owner

Summary

Promote the Forgejo review event to Request changes when any built-in or custom pre-merge check fails.
Add pipeline coverage for a source-only diff with no inline findings that fails Tests touched and posts REQUEST_CHANGES.
Update user-facing docs and changelog to describe the new review-event behavior while clarifying it remains advisory to branch protection.

RGR evidence

RED:

cargo test -p ar-review pipeline::tests::review_pull_request_request_changes_when_pre_merge_check_fails -- --nocapture
pre-merge failure should still post review: Forgejo(Api { status: 404, body: "" })

GREEN / verification:

cargo test -p ar-review pipeline::tests::review_pull_request_request_changes_when_pre_merge_check_fails -- --nocapture
cargo test -p ar-review pipeline::tests:: -- --nocapture
cargo fmt --all -- --check
git diff --check -- CHANGELOG.md docs/USER-GUIDE.md crates/ar-review/src/pre_merge.rs crates/ar-review/src/pipeline.rs

Closes #7

## Summary - Promote the Forgejo review event to Request changes when any built-in or custom pre-merge check fails. - Add pipeline coverage for a source-only diff with no inline findings that fails Tests touched and posts REQUEST_CHANGES. - Update user-facing docs and changelog to describe the new review-event behavior while clarifying it remains advisory to branch protection. ## RGR evidence RED: ``` cargo test -p ar-review pipeline::tests::review_pull_request_request_changes_when_pre_merge_check_fails -- --nocapture pre-merge failure should still post review: Forgejo(Api { status: 404, body: "" }) ``` GREEN / verification: - cargo test -p ar-review pipeline::tests::review_pull_request_request_changes_when_pre_merge_check_fails -- --nocapture - cargo test -p ar-review pipeline::tests:: -- --nocapture - cargo fmt --all -- --check - git diff --check -- CHANGELOG.md docs/USER-GUIDE.md crates/ar-review/src/pre_merge.rs crates/ar-review/src/pipeline.rs Closes #7

jwilger added 1 commit

2026-05-02 12:38:50 -07:00

fix(review): request changes for failed pre-merge checks

auto_review auto_review: no findings

CI / Nix flake check (pull_request) Successful in 1m44s

Details

03a8fbb8b0

Make the Forgejo review event reflect failed built-in or custom pre-merge checks, so missing tests, changelog entries, TODO cleanup, or repo-authored checks are visible as a request to fix before merging.

RED: cargo test -p ar-review pipeline::tests::review_pull_request_request_changes_when_pre_merge_check_fails -- --nocapture failed because the POST matcher expected REQUEST_CHANGES but the request used COMMENT.

auto-review reviewed

2026-05-02 12:39:01 -07:00

auto-review left a comment

Walkthrough

Pipeline Logic:
- The review_pull_request function now sets the review event to REQUEST_CHANGES if any pre-merge check fails, ensuring that the review event aligns with the pre-merge check results.
- A new helper function pre_merge_has_failure is introduced to determine if any pre-merge checks have failed.
Tests:
- New tests are added to verify that the review event is set to REQUEST_CHANGES when pre-merge checks fail.
- Existing tests are updated to reflect the new behavior of the review event.
Documentation:
- The CHANGELOG.md and USER-GUIDE.md are updated to describe the new behavior of the review event when pre-merge checks fail.
- The documentation clarifies that while the review event requests changes, it remains advisory to branch protection settings.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — source changed but no test file appears in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

This PR updates the review process to request changes when any pre-merge check fails, aligning the review event with the intended 'don't merge yet' signal. The changes include updates to the pipeline logic, tests, and documentation to reflect this new behavior. The implementation appears correct and safe to merge. ## Walkthrough - **Pipeline Logic**: - The `review_pull_request` function now sets the review event to `REQUEST_CHANGES` if any pre-merge check fails, ensuring that the review event aligns with the pre-merge check results. - A new helper function `pre_merge_has_failure` is introduced to determine if any pre-merge checks have failed. - **Tests**: - New tests are added to verify that the review event is set to `REQUEST_CHANGES` when pre-merge checks fail. - Existing tests are updated to reflect the new behavior of the review event. - **Documentation**: - The `CHANGELOG.md` and `USER-GUIDE.md` are updated to describe the new behavior of the review event when pre-merge checks fail. - The documentation clarifies that while the review event requests changes, it remains advisory to branch protection settings. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [ ] Tests touched — source changed but no test file appears in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers

jwilger merged commit 3ee2831e2e into main

2026-05-02 12:41:51 -07:00

jwilger deleted branch fix/issue-7-premerge-request-changes

2026-05-02 12:41:51 -07:00

jwilger referenced this pull request from a commit

2026-05-02 12:41:52 -07:00

fix(review): request changes for failed pre-merge checks (#38)