Skip to content

Commit a746a9d

Browse files
isadeksisadeks
authored
ci: add automated PR review gate workflow (#42)
When `build`/`integ` complete on a PR, aggregate all check-runs + commit statuses and either (a) comment which checks are failing (no review compute spent), (b) auto-update an out-of-date branch so CI re-runs (PAT so build re-fires), (c) comment asking to resolve conflicts, or (d) trigger the ABCA coding/pr-review-v1 agent via the Task API webhook once green. Comments-only, edit-in-place, per-SHA idempotent — does not touch Mergify or the integ-smoke gate. Runs in the trusted base-repo context via workflow_run so secrets/PAT are available for fork PRs; no PR code is checked out or executed. Adds review-gate.yml to .github/zizmor.yml dangerous-triggers and secrets-outside-env ignore lists (intentional workflow_run + repo-level secrets; the gate has no environment/approval boundary).
1 parent 3e2d9c7 commit a746a9d

2 files changed

Lines changed: 367 additions & 0 deletions

File tree

‎.github/workflows/review-gate.yml‎

Lines changed: 358 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,358 @@
1+
name: review-gate
2+
3+
# Automated PR review gate. When `build` (or `integ`) completes on a PR head
4+
# SHA, this workflow aggregates ALL check-runs + commit statuses to decide if CI
5+
# is green, then inspects mergeability and either (a) tells the author what to
6+
# fix, (b) updates an out-of-date branch so CI re-runs, or (c) triggers the ABCA
7+
# `coding/pr-review-v1` agent via the Task API webhook so a structured review is
8+
# waiting before a human looks.
9+
#
10+
# Trigger model mirrors integ.yml / deploy.yml: `workflow_run` runs in the
11+
# TRUSTED base-repo context, so secrets/vars/PAT are available even for fork PRs
12+
# (a fork `pull_request` job gets none). Unlike those workflows we also react to
13+
# a FAILED build so we can post the "tests failing" comment, and we listen to
14+
# `integ` too so the gate re-pulses once the slow `integ-smoke` status resolves.
15+
#
16+
# This gate posts COMMENTS ONLY — no check-run, no commit status, no formal
17+
# review — so it never interferes with Mergify (status-success=build,
18+
# #approved-reviews-by>=1, dismiss-stale-approvals-on-push) or the required
19+
# integ-smoke gate. It is a strictly advisory orchestration layer.
20+
#
21+
# OPERATOR SETUP (see plan): repo vars ABCA_TASK_API_URL, ABCA_WEBHOOK_ID; repo
22+
# secrets ABCA_WEBHOOK_SECRET, AUTOMATION_GITHUB_TOKEN (exists). Register the
23+
# webhook with `bgagent webhook create`; its Secrets Manager secret
24+
# (bgagent/webhook/<id>) must equal ABCA_WEBHOOK_SECRET.
25+
on:
26+
# zizmor: ignore[dangerous-triggers] — intentional; workflow_run is required so
27+
# this runs in the trusted base-repo context (secrets/PAT available for fork
28+
# PRs). Mitigations: no PR code is checked out or executed (pure gh api/curl),
29+
# least-privilege permissions, all untrusted event fields passed via env only.
30+
workflow_run:
31+
workflows: [build, integ]
32+
types: [completed]
33+
workflow_dispatch:
34+
inputs:
35+
pr_number:
36+
description: "PR number to evaluate (manual re-pulse)"
37+
required: true
38+
type: string
39+
40+
# One gate run per PR; a newer pulse supersedes an in-flight one. Fork PRs carry
41+
# an empty workflow_run.pull_requests[], so fall back to the head SHA.
42+
concurrency:
43+
group: >-
44+
review-gate-${{
45+
github.event.workflow_run.pull_requests[0].number
46+
|| github.event.workflow_run.head_sha
47+
|| inputs.pr_number
48+
|| github.ref
49+
}}
50+
cancel-in-progress: true
51+
52+
permissions: {}
53+
54+
jobs:
55+
gate:
56+
name: review-gate
57+
# React to a completed build/integ (any conclusion — the check aggregation
58+
# decides the verdict) or a manual dispatch.
59+
if: >-
60+
github.event_name == 'workflow_dispatch' ||
61+
github.event_name == 'workflow_run'
62+
runs-on: ubuntu-latest
63+
timeout-minutes: 20
64+
permissions:
65+
pull-requests: write # read PR, update-branch fallback
66+
issues: write # upsert PR (issue) comments
67+
contents: read # read commits/*
68+
checks: read # read check-runs
69+
statuses: read # read commit statuses (integ-smoke etc.)
70+
steps:
71+
# -------------------------------------------------------------------
72+
# Resolve PR context. All untrusted event fields flow through env (never
73+
# inlined into run:) to satisfy zizmor template-injection.
74+
# -------------------------------------------------------------------
75+
- name: Resolve PR context
76+
id: resolve
77+
env:
78+
GH_TOKEN: ${{ github.token }}
79+
REPO: ${{ github.repository }}
80+
EVENT_NAME: ${{ github.event_name }}
81+
PR_NUMBER_FROM_EVENT: ${{ github.event.workflow_run.pull_requests[0].number }}
82+
PR_NUMBER_FROM_INPUT: ${{ inputs.pr_number }}
83+
WF_HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
84+
WF_HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }}
85+
run: |
86+
set -euo pipefail
87+
88+
resolve_pr_number() {
89+
if [[ "$EVENT_NAME" == "workflow_dispatch" ]]; then
90+
echo "$PR_NUMBER_FROM_INPUT"; return
91+
fi
92+
if [[ -n "$PR_NUMBER_FROM_EVENT" ]]; then
93+
echo "$PR_NUMBER_FROM_EVENT"; return
94+
fi
95+
# Fork PRs: workflow_run.pull_requests[] is empty — resolve via API.
96+
gh api "repos/$REPO/commits/$WF_HEAD_SHA/pulls" --jq '.[0].number // empty' 2>/dev/null || true
97+
}
98+
99+
PR_NUMBER="$(resolve_pr_number)"
100+
if [[ -z "$PR_NUMBER" ]]; then
101+
echo "::notice::No PR resolved for this event — nothing to gate; skipping."
102+
echo "applicable=false" >> "$GITHUB_OUTPUT"
103+
exit 0
104+
fi
105+
106+
{
107+
echo "applicable=true"
108+
echo "pr_number=$PR_NUMBER"
109+
echo "head_repo=$WF_HEAD_REPO"
110+
} >> "$GITHUB_OUTPUT"
111+
echo "Gating PR #$PR_NUMBER (trigger=$EVENT_NAME)"
112+
113+
# -------------------------------------------------------------------
114+
# Evaluate the gate. Reads + comments use GITHUB_TOKEN; update-branch
115+
# uses the PAT (in a subshell) so the push re-fires `build`.
116+
# -------------------------------------------------------------------
117+
- name: Evaluate gate
118+
if: steps.resolve.outputs.applicable == 'true'
119+
env:
120+
GH_TOKEN: ${{ github.token }}
121+
AUTOMATION_TOKEN: ${{ secrets.AUTOMATION_GITHUB_TOKEN }}
122+
REPO: ${{ github.repository }}
123+
SERVER_URL: ${{ github.server_url }}
124+
RUN_ID: ${{ github.run_id }}
125+
PR_NUMBER: ${{ steps.resolve.outputs.pr_number }}
126+
HEAD_REPO_FROM_EVENT: ${{ steps.resolve.outputs.head_repo }}
127+
ABCA_TASK_API_URL: ${{ vars.ABCA_TASK_API_URL }}
128+
ABCA_WEBHOOK_ID: ${{ vars.ABCA_WEBHOOK_ID }}
129+
ABCA_WEBHOOK_SECRET: ${{ secrets.ABCA_WEBHOOK_SECRET }}
130+
run: |
131+
set -euo pipefail
132+
133+
MARKER='<!-- abca-review-gate -->'
134+
SELF_CHECK='review-gate' # exclude our own check-run (no self-deadlock)
135+
RUN_URL="$SERVER_URL/$REPO/actions/runs/$RUN_ID"
136+
137+
# Join args with newlines to build a markdown comment body. Keeping the
138+
# markdown inside function args (not a YAML-level multi-line string)
139+
# avoids column-0 lines that would break this block scalar.
140+
render() { printf '%s\n' "$@"; }
141+
142+
# ---- Resolve the PR's CURRENT state; operate only on live head. -----
143+
# The workflow_run head_sha can be stale (PR advanced since build ran).
144+
# Read the PR's current head + state once and key everything off it, so
145+
# checks + mergeability + review target stay mutually consistent.
146+
PR_JSON="$(gh api "repos/$REPO/pulls/$PR_NUMBER")"
147+
PR_STATE="$(echo "$PR_JSON" | jq -r '.state')"
148+
HEAD_SHA="$(echo "$PR_JSON" | jq -r '.head.sha')"
149+
HEAD_REPO="$(echo "$PR_JSON" | jq -r '.head.repo.full_name // empty')"
150+
IS_DRAFT="$(echo "$PR_JSON" | jq -r '.draft')"
151+
[[ -z "$HEAD_REPO" ]] && HEAD_REPO="$HEAD_REPO_FROM_EVENT"
152+
SHORT_SHA="${HEAD_SHA:0:7}"
153+
154+
if [[ "$PR_STATE" != "open" ]]; then
155+
echo "PR #$PR_NUMBER is $PR_STATE — nothing to gate."
156+
exit 0
157+
fi
158+
if [[ "$IS_DRAFT" == "true" ]]; then
159+
echo "PR #$PR_NUMBER is a draft — skipping."
160+
exit 0
161+
fi
162+
echo "PR #$PR_NUMBER open @ $HEAD_SHA (head_repo=$HEAD_REPO)"
163+
164+
# ---- Upsert a single marker comment (edit-in-place, never spam). ----
165+
upsert_comment() {
166+
local body="$1" existing_id
167+
existing_id="$(gh api "repos/$REPO/issues/$PR_NUMBER/comments" --paginate \
168+
--jq "[.[] | select(.body | contains(\"$MARKER\")) | .id] | first // empty")"
169+
if [[ -n "$existing_id" ]]; then
170+
gh api -X PATCH "repos/$REPO/issues/comments/$existing_id" -f body="$body" >/dev/null
171+
else
172+
gh api -X POST "repos/$REPO/issues/$PR_NUMBER/comments" -f body="$body" >/dev/null
173+
fi
174+
}
175+
176+
# ---- Per-SHA review dedup: already triggered a review for this SHA? --
177+
already_reviewed_this_sha() {
178+
gh api "repos/$REPO/issues/$PR_NUMBER/comments" --paginate \
179+
--jq "[.[] | select(.body | contains(\"abca-review-gate:reviewed-sha=$HEAD_SHA\")) | .id] | first // empty"
180+
}
181+
182+
# ---- Aggregate check-runs (latest only) + commit statuses. ----------
183+
# Failing: run conclusion in {failure,timed_out,cancelled,action_required,
184+
# startup_failure} or status state in {failure,error}. neutral/skipped/
185+
# stale/success => pass. Excludes our own check by name.
186+
evaluate_checks() {
187+
local runs_json status_json
188+
runs_json="$(gh api "repos/$REPO/commits/$HEAD_SHA/check-runs?filter=latest" --paginate \
189+
--jq '.check_runs[] | {name, status, conclusion}' | jq -s '.')"
190+
status_json="$(gh api "repos/$REPO/commits/$HEAD_SHA/status")"
191+
192+
local failed_runs pending_runs failed_status pending_status
193+
failed_runs="$(echo "$runs_json" | jq -r --arg self "$SELF_CHECK" '
194+
.[] | select(.name != $self)
195+
| select(.conclusion == "failure" or .conclusion == "timed_out"
196+
or .conclusion == "cancelled" or .conclusion == "action_required"
197+
or .conclusion == "startup_failure") | .name')"
198+
pending_runs="$(echo "$runs_json" | jq -r --arg self "$SELF_CHECK" '
199+
.[] | select(.name != $self) | select(.status != "completed") | .name')"
200+
failed_status="$(echo "$status_json" | jq -r '
201+
.statuses[] | select(.state == "failure" or .state == "error") | .context')"
202+
pending_status="$(echo "$status_json" | jq -r '
203+
.statuses[] | select(.state == "pending") | .context')"
204+
205+
FAILING="$(printf '%s\n%s\n' "$failed_runs" "$failed_status" | sed '/^$/d' | sort -u)"
206+
PENDING="$(printf '%s\n%s\n' "$pending_runs" "$pending_status" | sed '/^$/d' | sort -u)"
207+
}
208+
209+
# ---- 1. Check aggregation, with a short bounded poll for pending. ----
210+
FAILING=""; PENDING=""
211+
for attempt in $(seq 1 8); do # ~8 * 15s ~= 2 min cap
212+
evaluate_checks
213+
[[ -n "$FAILING" ]] && break
214+
[[ -z "$PENDING" ]] && break
215+
echo "Checks still pending (attempt $attempt): $(echo "$PENDING" | tr '\n' ' ')"
216+
sleep 15
217+
done
218+
219+
if [[ -n "$FAILING" ]]; then
220+
# Backticks below are literal markdown (inline code around each check
221+
# name), not command substitution — single quotes are intentional.
222+
# shellcheck disable=SC2016
223+
LIST="$(echo "$FAILING" | sed 's/^/- `/; s/$/`/')"
224+
BODY="$(render \
225+
"$MARKER" \
226+
"### ❌ CI is failing" \
227+
"" \
228+
"The following checks are not passing on \`$SHORT_SHA\`. Please address them before ABCA review:" \
229+
"" \
230+
"$LIST" \
231+
"" \
232+
"_This is an automated gate; it re-checks on every CI run. [View the latest run]($RUN_URL)._")"
233+
upsert_comment "$BODY"
234+
echo "Failing checks present — commented and stopping."
235+
exit 0
236+
fi
237+
238+
if [[ -n "$PENDING" ]]; then
239+
echo "Checks still pending after poll — exiting quietly; a later pulse re-evaluates."
240+
exit 0
241+
fi
242+
echo "All checks green for $HEAD_SHA."
243+
244+
# ---- 2. Mergeability (poll until mergeable != null). ----------------
245+
# GitHub computes .mergeable asynchronously; a fresh GET can return null.
246+
MERGEABLE="null"; MSTATE="unknown"
247+
for attempt in $(seq 1 10); do # ~10 * 6s ~= 1 min cap
248+
read -r MERGEABLE MSTATE < <(
249+
gh api "repos/$REPO/pulls/$PR_NUMBER" --jq '"\(.mergeable) \(.mergeable_state)"'
250+
)
251+
[[ "$MERGEABLE" != "null" ]] && break
252+
sleep 6
253+
done
254+
echo "mergeable=$MERGEABLE mergeable_state=$MSTATE"
255+
256+
# ---- 2a. Merge conflict. -------------------------------------------
257+
if [[ "$MSTATE" == "dirty" || "$MERGEABLE" == "false" ]]; then
258+
BODY="$(render \
259+
"$MARKER" \
260+
"### ⚠️ Merge conflict" \
261+
"" \
262+
"This PR has conflicts with the base branch. Please resolve them and push — the gate re-runs automatically once CI passes on the updated branch, then requests an ABCA review.")"
263+
upsert_comment "$BODY"
264+
echo "Merge conflict — commented and stopping."
265+
exit 0
266+
fi
267+
268+
# ---- 2b. Behind base — update branch (PAT so build re-fires). ------
269+
if [[ "$MSTATE" == "behind" ]]; then
270+
if [[ "$HEAD_REPO" != "$REPO" ]]; then
271+
# Cross-fork update-branch needs "maintainer can modify" + write to
272+
# the fork; the PAT usually can't push there. Ask the author.
273+
BODY="$(render \
274+
"$MARKER" \
275+
"### 🔄 Branch is out of date" \
276+
"" \
277+
"This fork PR is behind the base branch. Please update your branch (merge or rebase the base) and push so CI re-runs.")"
278+
upsert_comment "$BODY"
279+
echo "Behind + fork PR — cannot update-branch with PAT; commented."
280+
exit 0
281+
fi
282+
if [[ -z "${AUTOMATION_TOKEN:-}" ]]; then
283+
BODY="$(render \
284+
"$MARKER" \
285+
"### 🔄 Branch is out of date" \
286+
"" \
287+
"This branch is behind the base. Please update it (merge or rebase the base) and push so CI re-runs.")"
288+
upsert_comment "$BODY"
289+
echo "::warning::AUTOMATION_GITHUB_TOKEN unset — cannot auto-update; asked author."
290+
exit 0
291+
fi
292+
# Same-repo branch: merge base into head. PAT push re-triggers build.
293+
if GH_TOKEN="$AUTOMATION_TOKEN" gh api -X PUT \
294+
"repos/$REPO/pulls/$PR_NUMBER/update-branch" \
295+
-f expected_head_sha="$HEAD_SHA" >/dev/null 2>&1; then
296+
BODY="$(render \
297+
"$MARKER" \
298+
"### 🔄 Updated branch, re-running CI" \
299+
"" \
300+
"Merged the base branch in to bring this branch up to date. CI will re-run; the gate re-evaluates and requests an ABCA review once it's green.")"
301+
upsert_comment "$BODY"
302+
echo "update-branch succeeded — commented and stopping."
303+
else
304+
BODY="$(render \
305+
"$MARKER" \
306+
"### 🔄 Branch is out of date" \
307+
"" \
308+
"Automatic branch update did not succeed. Please update your branch (merge or rebase the base) and push so CI re-runs.")"
309+
upsert_comment "$BODY"
310+
echo "::warning::update-branch failed — asked author to update manually."
311+
fi
312+
exit 0
313+
fi
314+
315+
# ---- 2c. Green + not-behind + not-dirty → trigger ABCA review. -----
316+
# NOTE: we intentionally do NOT require mergeable_state == 'clean'. With
317+
# branch protection requiring an approval, a green conflict-free PR
318+
# reports 'blocked' (awaiting review), never 'clean' — the whole point
319+
# is to review BEFORE a human approves. clean/blocked/unstable all pass.
320+
if [[ -n "$(already_reviewed_this_sha)" ]]; then
321+
echo "ABCA review already triggered for $HEAD_SHA — nothing to do."
322+
exit 0
323+
fi
324+
325+
if [[ -z "${ABCA_TASK_API_URL:-}" || -z "${ABCA_WEBHOOK_ID:-}" || -z "${ABCA_WEBHOOK_SECRET:-}" ]]; then
326+
echo "::error::ABCA webhook not configured (need vars ABCA_TASK_API_URL, ABCA_WEBHOOK_ID and secret ABCA_WEBHOOK_SECRET)."
327+
exit 1
328+
fi
329+
330+
# Body SIGNED must be byte-identical to body POSTED. jq -c emits no
331+
# trailing newline; printf '%s' + curl --data-raw send it verbatim.
332+
POST_BODY="$(jq -nc --arg repo "$REPO" --argjson pr "$PR_NUMBER" \
333+
'{workflow_ref:"coding/pr-review-v1", repo:$repo, pr_number:$pr}')"
334+
# openssl output is "(stdin)= <hex>" or "SHA2-256(stdin)= <hex>"; take last.
335+
SIG="$(printf '%s' "$POST_BODY" | openssl dgst -sha256 -hmac "$ABCA_WEBHOOK_SECRET" | awk '{print $NF}')"
336+
337+
HTTP_CODE="$(curl -sS -o /tmp/abca-resp.txt -w '%{http_code}' \
338+
-X POST "${ABCA_TASK_API_URL%/}/webhooks/tasks" \
339+
-H 'Content-Type: application/json' \
340+
-H "X-Webhook-Id: $ABCA_WEBHOOK_ID" \
341+
-H "X-Webhook-Signature: sha256=$SIG" \
342+
-H "Idempotency-Key: review-$REPO-$PR_NUMBER-$HEAD_SHA" \
343+
--data-raw "$POST_BODY")"
344+
345+
if [[ "$HTTP_CODE" =~ ^2 ]]; then
346+
BODY="$(render \
347+
"$MARKER" \
348+
"<!-- abca-review-gate:reviewed-sha=$HEAD_SHA -->" \
349+
"### 🤖 ABCA review requested" \
350+
"" \
351+
"CI is green and the branch is up to date, so an automated ABCA review of \`$SHORT_SHA\` has been requested. Findings will be posted here shortly.")"
352+
upsert_comment "$BODY"
353+
echo "ABCA review triggered for $HEAD_SHA (HTTP $HTTP_CODE)."
354+
else
355+
echo "::error::Task API webhook returned HTTP $HTTP_CODE"
356+
cat /tmp/abca-resp.txt || true
357+
exit 1
358+
fi

‎.github/zizmor.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,19 @@ rules:
88
# Approve-only job: never checks out or runs PR code, only calls the
99
# review API; gated on the 'auto-approve' label.
1010
- auto-approve.yml
11+
# Comments-only PR gate: never checks out or runs PR code (pure gh
12+
# api/curl); workflow_run is required for the trusted base-repo context
13+
# so secrets/PAT are available for fork PRs. Untrusted event fields flow
14+
# through env only.
15+
- review-gate.yml
1116
# These secrets are intentionally repo-level, not environment-scoped:
1217
# CODECOV_TOKEN is an upload-only token; AUTOMATION_GITHUB_TOKEN is the PAT
1318
# the upgrade-main PR job needs so its PRs trigger the build workflow.
1419
secrets-outside-env:
1520
ignore:
1621
- build.yml
1722
- upgrade-main.yml
23+
# ABCA_WEBHOOK_SECRET + AUTOMATION_GITHUB_TOKEN are intentionally repo-level:
24+
# the gate has no environment/approval boundary (it only reads state, posts
25+
# comments, and fires the review webhook).
26+
- review-gate.yml

0 commit comments

Comments
 (0)