From 733bd37c3775121047ad76d58f69d7bd968ff2a9 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Wed, 22 Jul 2026 10:33:25 +0300 Subject: [PATCH] feat: make AI review verdicts advisory Removes the separate review check and relies on review labels/comments instead Marks workflow/tooling failures with review:automation-failed Updates review guidance to reflect the advisory verdict model --- .github/workflows/pr-review.yml | 136 +++++--------------------------- .opencode/agent/pr-review.md | 2 +- CONTRIBUTING.md | 8 +- 3 files changed, 25 insertions(+), 121 deletions(-) diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index b04d54ae..33f0fde1 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -17,8 +17,6 @@ concurrency: jobs: review: - # Manual comment workflows run on the default branch, so their native job - # check cannot represent the reviewed PR HEAD. Publish that check explicitly. name: automation if: | github.event_name == 'pull_request_target' || @@ -26,7 +24,6 @@ jobs: (github.event_name == 'pull_request_review_comment' && github.event.comment.user.login != 'openchamber-bot[bot]' && (github.event.comment.body == '/oc-review' || startsWith(github.event.comment.body, '/oc-review ') || github.event.comment.body == '@openchamber-bot review' || startsWith(github.event.comment.body, '@openchamber-bot review '))) runs-on: ubuntu-latest permissions: - checks: write contents: read pull-requests: write issues: write @@ -62,44 +59,10 @@ jobs: echo "head_repo_owner=$(printf '%s' "$pr_json" | jq -r '.headRepositoryOwner.login')" } >> "$GITHUB_OUTPUT" - - name: Start review check - id: review-check - env: - GH_TOKEN: ${{ github.token }} - REVIEW_HEAD_SHA: ${{ steps.pr.outputs.head_sha }} - run: | - check_run="$(jq -n \ - --arg head_sha "$REVIEW_HEAD_SHA" \ - --arg details_url "${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" \ - '{ - name: "review", - head_sha: $head_sha, - status: "in_progress", - details_url: $details_url, - output: { - title: "OpenChamber review in progress", - summary: "Reviewing the current pull request HEAD." - } - }')" - - check_id="$(printf '%s' "$check_run" | gh api \ - --method POST \ - "repos/${GITHUB_REPOSITORY}/check-runs" \ - --input - \ - --jq '.id')" - echo "id=$check_id" >> "$GITHUB_OUTPUT" - - - name: Generate review app token - id: app-token - uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 - with: - app-id: ${{ secrets.OC_REVIEW_APP_ID }} - private-key: ${{ secrets.OC_REVIEW_APP_PRIVATE_KEY }} - - name: Clear review status for draft if: steps.pr.outputs.draft == 'true' env: - GH_TOKEN: ${{ steps.app-token.outputs.token }} + GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} run: | remove_args=() @@ -113,6 +76,14 @@ jobs: gh pr edit "$PR_NUMBER" "${remove_args[@]}" fi + - name: Generate review app token + id: app-token + if: steps.pr.outputs.draft == 'false' + uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 + with: + app-id: ${{ secrets.OC_REVIEW_APP_ID }} + private-key: ${{ secrets.OC_REVIEW_APP_PRIVATE_KEY }} + - name: Check review safety if: steps.pr.outputs.draft == 'false' id: safety @@ -226,9 +197,7 @@ jobs: $CHANGED_SENSITIVE_FILES \`\`\` - Automated review cannot clear changes to its own policy or trust boundary. A maintainer must review and explicitly override this failing check." - - exit 1 + Automated review cannot clear changes to its own policy or trust boundary. A maintainer must review it directly." - name: Debounce new commits if: steps.pr.outputs.draft == 'false' && steps.safety.outputs.safe == 'true' && github.event_name == 'pull_request_target' && github.event.action == 'synchronize' @@ -396,88 +365,23 @@ jobs: echo "- Status: \`$review_label\`" } >> "$GITHUB_STEP_SUMMARY" - if [ "$verdict" != "pass" ]; then - echo "Review verdict is $verdict; only pass satisfies this check." >&2 - exit 1 - fi - - - name: Publish review check - if: always() && steps.review-check.outputs.id != '' + - name: Mark automation failure + if: always() && steps.pr.outputs.draft == 'false' && steps.verdict.outcome != 'success' && steps.safety.outputs.safe != 'false' env: GH_TOKEN: ${{ github.token }} - CHECK_RUN_ID: ${{ steps.review-check.outputs.id }} PR_NUMBER: ${{ steps.pr.outputs.number }} REVIEW_HEAD_SHA: ${{ steps.pr.outputs.head_sha }} - DRAFT: ${{ steps.pr.outputs.draft }} - VERDICT_OUTCOME: ${{ steps.verdict.outcome }} run: | current_head="$(gh pr view "$PR_NUMBER" --json headRefOid --jq '.headRefOid')" - review_label="$(gh pr view "$PR_NUMBER" --json labels --jq '[.labels[].name | select(startswith("review:"))] | first // ""')" - - set_review_status() { - local target_label="$1" - local remove_args=() - - while IFS= read -r label; do - case "$label" in - review:*) remove_args+=(--remove-label "$label") ;; - esac - done < <(gh pr view "$PR_NUMBER" --json labels --jq '.labels[].name') - - if [ -n "$target_label" ]; then - gh pr edit "$PR_NUMBER" "${remove_args[@]}" --add-label "$target_label" - elif [ "${#remove_args[@]}" -gt 0 ]; then - gh pr edit "$PR_NUMBER" "${remove_args[@]}" - fi - - review_label="$target_label" - } - - if [ "$current_head" = "$REVIEW_HEAD_SHA" ]; then - if [ "$DRAFT" = "true" ]; then - set_review_status "" - elif [ "$VERDICT_OUTCOME" != "success" ]; then - case "$review_label" in - review:needs-evidence|review:blocked|review:human-required|review:automation-failed) ;; - *) set_review_status "review:automation-failed" ;; - esac - fi - fi - if [ "$current_head" != "$REVIEW_HEAD_SHA" ]; then - conclusion="cancelled" - title="Review superseded by a newer HEAD" - summary="The pull request HEAD moved before this review completed." - elif [ "$DRAFT" = "true" ]; then - conclusion="neutral" - title="Review skipped for draft pull request" - summary="Mark the pull request ready for review to start the readiness check." - elif [ "$VERDICT_OUTCOME" = "success" ] && [ "$review_label" = "review:ready" ]; then - conclusion="success" - title="OpenChamber review passed" - summary="The reviewed HEAD is ready for maintainer review." - else - conclusion="failure" - title="OpenChamber review did not pass" - summary="Current readiness state: ${review_label:-review:automation-failed}." + exit 0 fi - check_run="$(jq -n \ - --arg conclusion "$conclusion" \ - --arg title "$title" \ - --arg summary "$summary" \ - --arg details_url "${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" \ - '{ - status: "completed", - conclusion: $conclusion, - details_url: $details_url, - output: { - title: $title, - summary: $summary - } - }')" + remove_args=() + while IFS= read -r label; do + case "$label" in + review:*) remove_args+=(--remove-label "$label") ;; + esac + done < <(gh pr view "$PR_NUMBER" --json labels --jq '.labels[].name') - printf '%s' "$check_run" | gh api \ - --method PATCH \ - "repos/${GITHUB_REPOSITORY}/check-runs/${CHECK_RUN_ID}" \ - --input - >/dev/null + gh pr edit "$PR_NUMBER" "${remove_args[@]}" --add-label "review:automation-failed" diff --git a/.opencode/agent/pr-review.md b/.opencode/agent/pr-review.md index 72503836..5ceab70f 100644 --- a/.opencode/agent/pr-review.md +++ b/.opencode/agent/pr-review.md @@ -179,7 +179,7 @@ Choose exactly one review verdict: - `blocked`: at least one concrete correctness, security, mandatory-guidance, or contribution-contract blocker must be fixed. - `human-review-required`: the PR changes review policy/automation or another trust boundary that automation must not clear by itself, or safe automated review is otherwise impossible. -Verdict precedence is `human-review-required`, `blocked`, `needs-evidence`, then `pass`. CI status is intentionally outside this verdict: a review may return `pass` while a separate required check fails, and both gates must pass independently before merge. +Verdict precedence is `human-review-required`, `blocked`, `needs-evidence`, then `pass`. CI status is intentionally outside this verdict: a review may return `pass` while a separate required check fails. The AI verdict is advisory, is communicated through the `review:*` label and review comment, and must not fail the pull request check. ## Comment style diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f746f3e5..ca2b672e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -219,10 +219,10 @@ the previous readiness label before it starts, and only `review:ready` means the pull request is ready to enter the maintainer review queue. Draft pull requests have no readiness label. -The workflow publishes a separate `review` check on the exact reviewed HEAD; -only `review:ready` passes it. The `automation` job reports workflow execution -independently so a manual `/oc-review` result can update readiness without -being attached to the default-branch commit that triggered the command. +AI review verdicts are advisory and never fail the pull request check. Readiness +is communicated only through the `review:*` label and immutable review comment. +The `automation` job fails only when the workflow itself cannot complete or +verify a trustworthy result, in which case it applies `review:automation-failed`. Each completed review creates a new comment tied to its reviewed HEAD so the conversation remains chronological. Previous review comments are not rewritten.