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
This commit is contained in:
+20
-116
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
+4
-4
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user