Skip to content

Update docs agent self-approval guard #1333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SahilAujla merged 2 commits into from
May 26, 2026
Merged

Update docs agent self-approval guard #1333

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ca0038
Update docs agent self-approval guard
SahilAujla May 26, 2026
baa47c5
Remove ready-for-review guard trigger
SahilAujla May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 68 additions & 42 deletions .github/workflows/no-originator-self-approval.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ name: Docs agent review guard

# When the docs-agent (alchemy-bot) opens a PR on behalf of someone who
# requested the change via Slack, it includes a "Requested-by: @<github_username>"
# trailer in the commit message. This workflow watches for approvals on those
# PRs and dismisses any approval submitted by that user, so a docs-agent PR
# always requires a non-originator review before merging.
# trailer in the commit message. This workflow watches for approval submissions
# and PR updates on those PRs, dismisses any approval submitted by that user,
# and requires at least one non-originator approval in the PR review history.
#
# Note on shared bot identity: alchemy-bot is also used by other automation
# (Daikon spec updates, weekly changelog, etc.). To avoid this workflow
Expand All @@ -16,8 +16,8 @@ name: Docs agent review guard
#
# Branch protection on main needs both "Require 1 approval" and this workflow
# as a required check. The review requirement supplies an approval, and this
# check ensures that approval did not come from the person who originated the
# docs-agent request.
# check ensures that the PR has been approved at least once by someone who did
# not originate the docs-agent request.
#
# Why commit message and not PR body: the PR body is editable by anyone with
# write access (including the originator), who could remove their own @mention
Expand All @@ -33,6 +33,8 @@ name: Docs agent review guard
# the check can be required for every PR.

on:
pull_request:
types: [opened, reopened, synchronize]
pull_request_review:
types: [submitted]

Expand All @@ -52,20 +54,24 @@ jobs:
sparse-checkout: |
.github/workflows/docs-agent-pubkey.asc
sparse-checkout-cone-mode: false
- name: Compare approver to originator and dismiss if same
- name: Evaluate originator approval guard
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
APPROVER: ${{ github.event.review.user.login }}
REVIEW_STATE: ${{ github.event.review.state }}
REVIEW_ID: ${{ github.event.review.id }}
APPROVER: ${{ github.event.review.user.login || '' }}
REVIEW_STATE: ${{ github.event.review.state || '' }}
REVIEW_ID: ${{ github.event.review.id || '' }}
PR_NUMBER: ${{ github.event.pull_request.number }}
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
REVIEW_COMMIT_SHA: ${{ github.event.review.commit_id }}
TARGET_COMMIT_SHA: ${{ github.event.review.commit_id || github.event.pull_request.head.sha }}
run: |
set -euo pipefail

REVIEW_STATE_LOWER="$(printf '%s' "$REVIEW_STATE" | tr '[:upper:]' '[:lower:]')"
IS_APPROVAL_REVIEW=false
if [ "$GITHUB_EVENT_NAME" = "pull_request_review" ] && [ "$REVIEW_STATE_LOWER" = "approved" ]; then
IS_APPROVAL_REVIEW=true
fi

if [ "$PR_AUTHOR" != "alchemy-bot" ]; then
echo "PR author is $PR_AUTHOR, not alchemy-bot. No docs-agent enforcement needed."
Expand Down Expand Up @@ -97,8 +103,12 @@ jobs:
# all fail-closed paths AND the originator-match dismissal so a
# transient API hiccup doesn't leave the approval intact when our
# logic says it should be dismissed.
dismiss_with_retry() {
dismiss_current_review_with_retry() {
local message="$1"
if [ -z "${REVIEW_ID:-}" ]; then
echo "No submitted review to dismiss for $GITHUB_EVENT_NAME."
return 0
fi
for attempt in 1 2 3; do
if gh api -X PUT "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews/$REVIEW_ID/dismissals" \
-f message="$message" \
Expand Down Expand Up @@ -131,29 +141,35 @@ jobs:
# exactly what this rule is enforcing), fail closed.
if [ ! -f "$PUBKEY_FILE" ]; then
echo "ERROR: pubkey file $PUBKEY_FILE missing from checkout" >&2
dismiss_with_retry ":warning: Originator self-approval check could not initialize (pinned pubkey file missing from repo). Approval dismissed by default per fail-closed policy." || true
if [ "$IS_APPROVAL_REVIEW" = true ]; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not initialize (pinned pubkey file missing from repo). Approval dismissed by default per fail-closed policy." || true
fi
exit 1
fi

# Import. If import itself fails (corrupted file etc.), fail closed.
if ! gpg --batch --quiet --import "$PUBKEY_FILE" 2>/tmp/gpg-import-err; then
echo "ERROR: gpg --import failed:" >&2
cat /tmp/gpg-import-err >&2 || true
dismiss_with_retry ":warning: Originator self-approval check could not initialize (pubkey import failed). Approval dismissed by default per fail-closed policy." || true
if [ "$IS_APPROVAL_REVIEW" = true ]; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not initialize (pubkey import failed). Approval dismissed by default per fail-closed policy." || true
fi
exit 1
fi

# Post-import verify the pinned key actually loaded. Silent import
# without keys would otherwise make every commit look untrusted.
if ! gpg --list-keys "$EXPECTED_FPR" >/dev/null 2>&1; then
echo "ERROR: pinned pubkey $EXPECTED_FPR not present in keyring after import" >&2
dismiss_with_retry ":warning: Originator self-approval check could not initialize (pinned pubkey missing from imported keyring). Approval dismissed by default per fail-closed policy." || true
if [ "$IS_APPROVAL_REVIEW" = true ]; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not initialize (pinned pubkey missing from imported keyring). Approval dismissed by default per fail-closed policy." || true
fi
exit 1
fi

# Collect ALL Requested-by trailers from commits in this PR that
# are BOTH:
# 1. In the exact commit range approved by this review, AND
# 1. In the exact commit range being evaluated, AND
# 2. Cryptographically signed by the pinned agent GPG key
# (verified locally in the workflow, not just trusting
# GitHub's flag)
Expand Down Expand Up @@ -188,32 +204,41 @@ jobs:
# Fetch and inspect commits locally instead of using the REST pull
# request commits endpoint. That endpoint is capped at 250 commits,
# which is not acceptable for an enforcement decision. Local git
# history also lets us evaluate the exact commit SHA that the
# approval was submitted against, avoiding live-PR drift.
if [ -z "${BASE_SHA:-}" ] || [ -z "${REVIEW_COMMIT_SHA:-}" ]; then
echo "ERROR: missing base or reviewed commit SHA from event payload" >&2
dismiss_with_retry ":warning: Originator self-approval check could not run (missing review commit metadata). Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
# history also lets us evaluate the exact commit SHA under review:
# the reviewed commit for approval events, or the current PR head
# for PR update events.
if [ -z "${BASE_SHA:-}" ] || [ -z "${TARGET_COMMIT_SHA:-}" ]; then
echo "ERROR: missing base or target commit SHA from event payload" >&2
if [ "$IS_APPROVAL_REVIEW" = true ]; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not run (missing review commit metadata). Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
fi
exit 1
fi

if ! git fetch --no-tags --filter=blob:none origin "$REVIEW_COMMIT_SHA" 2>/tmp/git-fetch-err; then
echo "ERROR: failed to fetch reviewed commit $REVIEW_COMMIT_SHA:" >&2
if ! git fetch --no-tags --filter=blob:none origin "$TARGET_COMMIT_SHA" 2>/tmp/git-fetch-err; then
echo "ERROR: failed to fetch target commit $TARGET_COMMIT_SHA:" >&2
cat /tmp/git-fetch-err >&2 || true
dismiss_with_retry ":warning: Originator self-approval check could not fetch the reviewed commit. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
if [ "$IS_APPROVAL_REVIEW" = true ]; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not fetch the reviewed commit. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
fi
exit 1
fi

if ! git cat-file -e "$REVIEW_COMMIT_SHA^{commit}" 2>/dev/null; then
echo "ERROR: reviewed commit $REVIEW_COMMIT_SHA is not available after fetch" >&2
dismiss_with_retry ":warning: Originator self-approval check could not inspect the reviewed commit. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
if ! git cat-file -e "$TARGET_COMMIT_SHA^{commit}" 2>/dev/null; then
echo "ERROR: target commit $TARGET_COMMIT_SHA is not available after fetch" >&2
if [ "$IS_APPROVAL_REVIEW" = true ]; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not inspect the reviewed commit. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
fi
exit 1
fi

COMMITS_FILE="$(mktemp)"
if ! git rev-list --reverse "$BASE_SHA..$REVIEW_COMMIT_SHA" > "$COMMITS_FILE" 2>/tmp/rev-list-err; then
echo "ERROR: failed to enumerate PR commits from $BASE_SHA to $REVIEW_COMMIT_SHA:" >&2
if ! git rev-list --reverse "$BASE_SHA..$TARGET_COMMIT_SHA" > "$COMMITS_FILE" 2>/tmp/rev-list-err; then
echo "ERROR: failed to enumerate PR commits from $BASE_SHA to $TARGET_COMMIT_SHA:" >&2
cat /tmp/rev-list-err >&2 || true
dismiss_with_retry ":warning: Originator self-approval check could not enumerate PR commits. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
if [ "$IS_APPROVAL_REVIEW" = true ]; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not enumerate PR commits. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true
fi
exit 1
fi

Expand Down Expand Up @@ -284,39 +309,40 @@ jobs:

if ! REVIEWS_JSON="$(fetch_reviews)"; then
echo "ERROR: could not fetch PR reviews after 3 retries." >&2
if [ "$REVIEW_STATE_LOWER" = "approved" ] && printf '%s\n' "$ALL_REQUESTED_BY" | grep -qFx "$APPROVER_LOWER"; then
dismiss_with_retry ":warning: Originator self-approval check could not fetch current PR reviews. Approval dismissed by default because you are listed as the docs request originator." || true
if [ "$IS_APPROVAL_REVIEW" = true ] && printf '%s\n' "$ALL_REQUESTED_BY" | grep -qFx "$APPROVER_LOWER"; then
dismiss_current_review_with_retry ":warning: Originator self-approval check could not fetch current PR reviews. Approval dismissed by default because you are listed as the docs request originator." || true
fi
exit 1
fi

ACTIVE_APPROVERS="$(
HISTORICAL_APPROVERS="$(
printf '%s' "$REVIEWS_JSON" \
| jq -r 'map(select((.state | ascii_upcase) != "COMMENTED")) | sort_by(.submitted_at // "") | reduce .[] as $review ({}; .[$review.user.login | ascii_downcase] = ($review.state | ascii_upcase)) | to_entries[] | select(.value == "APPROVED") | .key'
| jq -r 'map(select((.state | ascii_upcase) == "APPROVED")) | .[] | .user.login | ascii_downcase' \
| sort -u
)"

NON_REQUESTER_APPROVAL_COUNT=0
while IFS= read -r active_approver; do
[ -n "$active_approver" ] || continue
if ! printf '%s\n' "$ALL_REQUESTED_BY" | grep -qFx "$active_approver"; then
while IFS= read -r historical_approver; do
[ -n "$historical_approver" ] || continue
if ! printf '%s\n' "$ALL_REQUESTED_BY" | grep -qFx "$historical_approver"; then
NON_REQUESTER_APPROVAL_COUNT=$((NON_REQUESTER_APPROVAL_COUNT + 1))
fi
done <<< "$ACTIVE_APPROVERS"
done <<< "$HISTORICAL_APPROVERS"

echo "Approver=$APPROVER_LOWER, AllRequestedBy=$(printf '%s' "$ALL_REQUESTED_BY" | tr '\n' ',' | sed 's/,$//'), NonRequesterApprovals=$NON_REQUESTER_APPROVAL_COUNT"
echo "Approver=$APPROVER_LOWER, AllRequestedBy=$(printf '%s' "$ALL_REQUESTED_BY" | tr '\n' ',' | sed 's/,$//'), NonRequesterApprovalHistory=$NON_REQUESTER_APPROVAL_COUNT"

if [ "$REVIEW_STATE_LOWER" = "approved" ] && printf '%s\n' "$ALL_REQUESTED_BY" | grep -qFx "$APPROVER_LOWER"; then
if [ "$IS_APPROVAL_REVIEW" = true ] && printf '%s\n' "$ALL_REQUESTED_BY" | grep -qFx "$APPROVER_LOWER"; then
echo "Approver matches a Requested-by trailer. Dismissing approval $REVIEW_ID."
if ! dismiss_with_retry "@$APPROVER you are listed as the originator of this docs request (via the Requested-by trailer on a docs-agent commit). Per the docs-agent self-review policy, the originator can't approve their own request. Please ask another team member to review."; then
if ! dismiss_current_review_with_retry "@$APPROVER you are listed as the originator of this docs request (via the Requested-by trailer on a docs-agent commit). Per the docs-agent self-review policy, the originator can't approve their own request. Please ask another team member to review."; then
echo "ERROR: dismissal of originator approval failed after 3 retries, exiting 1 to surface the failure" >&2
exit 1
fi
fi

if [ "$NON_REQUESTER_APPROVAL_COUNT" -gt 0 ]; then
echo "$NON_REQUESTER_APPROVAL_COUNT non-requester approval(s) active. Check passes."
echo "$NON_REQUESTER_APPROVAL_COUNT non-requester approval(s) found in review history. Check passes."
exit 0
fi

echo "No active non-requester approvals. Failing required check."
echo "No non-requester approvals found in review history. Failing required check."
exit 1
Loading