From a5ad09aad7f79a2115b5efce1f24eced215a0855 Mon Sep 17 00:00:00 2001 From: Smart Mekiliuwa Date: Mon, 20 Jul 2026 19:36:23 +0100 Subject: [PATCH 1/4] ci: author, review, and automerge regen prs via github apps --- .github/workflows/sdk_generation.yaml | 123 ++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 6 deletions(-) diff --git a/.github/workflows/sdk_generation.yaml b/.github/workflows/sdk_generation.yaml index 5015e2b..ac43229 100644 --- a/.github/workflows/sdk_generation.yaml +++ b/.github/workflows/sdk_generation.yaml @@ -39,12 +39,21 @@ jobs: generate: runs-on: ubuntu-latest steps: + - name: Mint bot app token + id: bot-token + # Pin to commit SHA for v1 (mutable tags can be retargeted). + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1 + with: + app-id: ${{ secrets.SDK_BOT_APP_ID }} + private-key: ${{ secrets.SDK_BOT_APP_KEY }} + - name: Checkout Code uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 with: - # Prefer a PAT: PRs opened with GITHUB_TOKEN do not trigger - # pull_request workflows, so verify CI would never run on them. - token: ${{ secrets.SDK_BOT_PAT || secrets.GITHUB_TOKEN }} + # App token, not GITHUB_TOKEN: PRs opened with GITHUB_TOKEN do not + # trigger pull_request workflows, so verify CI would never run on + # them. The convoy-sdk-bot app token triggers CI like a PAT. + token: ${{ steps.bot-token.outputs.token }} - name: Setup Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 @@ -95,11 +104,11 @@ jobs: - name: Push branch and open PR if: steps.diff.outputs.changed == 'true' env: - GH_TOKEN: ${{ secrets.SDK_BOT_PAT || secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ steps.bot-token.outputs.token }} BRANCH: ${{ steps.branch.outputs.name }} run: | - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git config user.name "convoy-sdk-bot[bot]" + git config user.email "307218117+convoy-sdk-bot[bot]@users.noreply.github.com" git checkout -B "$BRANCH" git add -A @@ -118,3 +127,105 @@ jobs: else echo "Updated existing PR #$existing" >> "$GITHUB_STEP_SUMMARY" fi + + - name: Mint reviewer app token + if: steps.diff.outputs.changed == 'true' + id: app-token + # Pin to commit SHA for v1 (mutable tags can be retargeted). + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1 + with: + app-id: ${{ secrets.SDK_REVIEWER_APP_ID }} + private-key: ${{ secrets.SDK_REVIEWER_APP_KEY }} + + - name: Approve and enable auto-merge + if: steps.diff.outputs.changed == 'true' + env: + APP_TOKEN: ${{ steps.app-token.outputs.token }} + # Merge via the bot app token, not GITHUB_TOKEN: merges performed + # by GITHUB_TOKEN do not trigger the downstream publish workflows. + MERGE_TOKEN: ${{ steps.bot-token.outputs.token }} + GH_REPO: ${{ github.repository }} + BRANCH: ${{ steps.branch.outputs.name }} + run: | + # Failure policy: fail open to human review, never to merge. Any + # gate miss below (wrong base, stale head, non-generated paths) + # leaves the PR unapproved for a human; auto-merge completes only + # after the required status checks pass. + set -euo pipefail + + pr_json=$(GH_TOKEN="$APP_TOKEN" gh pr list --head "$BRANCH" --state open \ + --json number,baseRefName,headRefOid --jq '.[0] // empty') + if [ -z "$pr_json" ]; then + echo "No open PR for $BRANCH; nothing to approve." + exit 0 + fi + pr=$(echo "$pr_json" | jq -r .number) + base=$(echo "$pr_json" | jq -r .baseRefName) + head_sha=$(echo "$pr_json" | jq -r .headRefOid) + + if [ "$base" != "main" ]; then + echo "Not approving PR #$pr: base is $base, not main." + exit 0 + fi + + # Only regen PRs authored by the bot app qualify for auto-review. + # REST is used because it returns the stable "convoy-sdk-bot[bot]" + # login for app-authored PRs. + author=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr" --jq '.user.login') + if [ "$author" != "convoy-sdk-bot[bot]" ]; then + echo "Not approving PR #$pr: author is $author, not convoy-sdk-bot[bot]." + GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body "SDK reviewer app: not auto-approving; PR author is not convoy-sdk-bot[bot]. Left for human review (policy: fail open to human review, never to merge)." + exit 0 + fi + + # Bind the approval to the commit this run pushed; a concurrent + # push to the regen branch means the diff is no longer this run's + # generated output. + pushed_sha=$(git rev-parse HEAD) + if [ "$head_sha" != "$pushed_sha" ]; then + echo "Not approving PR #$pr: head $head_sha is not the commit this run pushed." + exit 0 + fi + + # Allowlist mirrors scripts/generate.sh: the generator mirrors into + # src/convoy/ but never touches the hand-written utils/ or py.typed. + # Renames are checked on both sides so a file cannot be moved into + # the generated tree from outside it. + bad=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr/files" --paginate \ + --jq '.[] | .filename, (.previous_filename // empty)' \ + | while read -r f; do + case "$f" in + src/convoy/utils/*|src/convoy/py.typed) echo "$f" ;; + src/convoy/*) ;; + *) echo "$f" ;; + esac + done) + if [ -n "$bad" ]; then + echo "Not approving PR #$pr: diff touches non-generated paths:" + echo "$bad" + { + echo "SDK reviewer app: not auto-approving; the diff touches paths outside the generated allowlist:" + echo + echo '```' + echo "$bad" + echo '```' + echo + echo "Left for human review (policy: fail open to human review, never to merge)." + } | GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body-file - + # Best effort: drop any auto-merge left enabled by an earlier + # clean run of this branch. + GH_TOKEN="$MERGE_TOKEN" gh pr merge "$pr" --disable-auto || true + exit 0 + fi + + GH_TOKEN="$APP_TOKEN" gh api -X POST "repos/$GH_REPO/pulls/$pr/reviews" \ + -f event=APPROVE \ + -f commit_id="$head_sha" \ + -f body="SDK reviewer app: approving a generated-paths-only diff on the regen branch. Auto-merge completes only after required status checks pass." + + enabled=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json autoMergeRequest --jq '.autoMergeRequest != null') + if [ "$enabled" = "true" ]; then + echo "Auto-merge already enabled on PR #$pr." + else + GH_TOKEN="$MERGE_TOKEN" gh pr merge "$pr" --auto --squash + fi From 5b0910dc5fa5e9cf4c5c089833d5f0964b07cfb6 Mon Sep 17 00:00:00 2001 From: Smart Mekiliuwa Date: Mon, 20 Jul 2026 20:32:11 +0100 Subject: [PATCH 2/4] fix: harden sdk auto-review gate deny paths now clear stale auto-merge before commenting and fail the step if the disable errors, instead of best-effort || true. an empty changed-files listing no longer counts as an allowlist pass. renames are checked on both sides everywhere. approvals are pinned to the head commit. --- .github/workflows/sdk_generation.yaml | 54 ++++++++++++++++----------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/.github/workflows/sdk_generation.yaml b/.github/workflows/sdk_generation.yaml index ac43229..29ee372 100644 --- a/.github/workflows/sdk_generation.yaml +++ b/.github/workflows/sdk_generation.yaml @@ -149,10 +149,23 @@ jobs: run: | # Failure policy: fail open to human review, never to merge. Any # gate miss below (wrong base, stale head, non-generated paths) - # leaves the PR unapproved for a human; auto-merge completes only - # after the required status checks pass. + # leaves the PR unapproved for a human. Clearing a stale auto-merge + # is the one hard failure: if --disable-auto errors, this step goes + # red instead of leaving a rejected head silently mergeable. set -euo pipefail + deny() { + # Auto-merge is cleared before commenting: under set -e a comment + # failure must not skip the disable, and a disable failure must + # fail the step, not be swallowed. + enabled=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json autoMergeRequest --jq '.autoMergeRequest != null') + if [ "$enabled" = "true" ]; then + GH_TOKEN="$MERGE_TOKEN" gh pr merge "$pr" --disable-auto + fi + printf '%s\n' "$1" | GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body-file - || true + exit 0 + } + pr_json=$(GH_TOKEN="$APP_TOKEN" gh pr list --head "$BRANCH" --state open \ --json number,baseRefName,headRefOid --jq '.[0] // empty') if [ -z "$pr_json" ]; then @@ -165,7 +178,7 @@ jobs: if [ "$base" != "main" ]; then echo "Not approving PR #$pr: base is $base, not main." - exit 0 + deny "SDK reviewer app: not auto-approving; the PR base is \`$base\`, not \`main\`. Left for human review (policy: fail open to human review, never to merge)." fi # Only regen PRs authored by the bot app qualify for auto-review. @@ -174,8 +187,7 @@ jobs: author=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr" --jq '.user.login') if [ "$author" != "convoy-sdk-bot[bot]" ]; then echo "Not approving PR #$pr: author is $author, not convoy-sdk-bot[bot]." - GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body "SDK reviewer app: not auto-approving; PR author is not convoy-sdk-bot[bot]. Left for human review (policy: fail open to human review, never to merge)." - exit 0 + deny "SDK reviewer app: not auto-approving; PR author is not convoy-sdk-bot[bot]. Left for human review (policy: fail open to human review, never to merge)." fi # Bind the approval to the commit this run pushed; a concurrent @@ -184,15 +196,24 @@ jobs: pushed_sha=$(git rev-parse HEAD) if [ "$head_sha" != "$pushed_sha" ]; then echo "Not approving PR #$pr: head $head_sha is not the commit this run pushed." - exit 0 + deny "SDK reviewer app: not auto-approving; the PR head is not the commit this generation run pushed. Left for human review (policy: fail open to human review, never to merge)." + fi + + # This run committed a real diff, so an empty changed-files listing + # is an API anomaly, not a clean PR; it must not count as an + # allowlist pass. + files=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr/files" --paginate \ + --jq '.[] | .filename, (.previous_filename // empty)') + if [ -z "$files" ]; then + echo "Not approving PR #$pr: changed-files listing came back empty." + deny "SDK reviewer app: not auto-approving; the changed-files listing came back empty for a non-empty regen commit. Left for human review (policy: fail open to human review, never to merge)." fi # Allowlist mirrors scripts/generate.sh: the generator mirrors into # src/convoy/ but never touches the hand-written utils/ or py.typed. # Renames are checked on both sides so a file cannot be moved into # the generated tree from outside it. - bad=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr/files" --paginate \ - --jq '.[] | .filename, (.previous_filename // empty)' \ + bad=$(printf '%s\n' "$files" \ | while read -r f; do case "$f" in src/convoy/utils/*|src/convoy/py.typed) echo "$f" ;; @@ -203,19 +224,10 @@ jobs: if [ -n "$bad" ]; then echo "Not approving PR #$pr: diff touches non-generated paths:" echo "$bad" - { - echo "SDK reviewer app: not auto-approving; the diff touches paths outside the generated allowlist:" - echo - echo '```' - echo "$bad" - echo '```' - echo - echo "Left for human review (policy: fail open to human review, never to merge)." - } | GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body-file - - # Best effort: drop any auto-merge left enabled by an earlier - # clean run of this branch. - GH_TOKEN="$MERGE_TOKEN" gh pr merge "$pr" --disable-auto || true - exit 0 + deny "$(printf '%s\n' \ + "SDK reviewer app: not auto-approving; the diff touches paths outside the generated allowlist:" \ + "" '```' "$bad" '```' "" \ + "Left for human review (policy: fail open to human review, never to merge).")" fi GH_TOKEN="$APP_TOKEN" gh api -X POST "repos/$GH_REPO/pulls/$pr/reviews" \ From 1e795ffd84981d6bf5b5766b6cff4b621eccd412 Mon Sep 17 00:00:00 2001 From: Smart Mekiliuwa Date: Mon, 20 Jul 2026 20:43:50 +0100 Subject: [PATCH 3/4] fix: close validate-then-approve race in auto-review pin the approval to the head captured before the allowlist run, then re-read the head after submitting; if it moved, dismiss the approval and deny. dismiss_stale_reviews (now enabled) covers pushes after the review; this covers the window before it. --- .github/workflows/sdk_generation.yaml | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/sdk_generation.yaml b/.github/workflows/sdk_generation.yaml index 29ee372..c37304e 100644 --- a/.github/workflows/sdk_generation.yaml +++ b/.github/workflows/sdk_generation.yaml @@ -230,11 +230,30 @@ jobs: "Left for human review (policy: fail open to human review, never to merge).")" fi - GH_TOKEN="$APP_TOKEN" gh api -X POST "repos/$GH_REPO/pulls/$pr/reviews" \ + review_id=$(GH_TOKEN="$APP_TOKEN" gh api -X POST "repos/$GH_REPO/pulls/$pr/reviews" \ -f event=APPROVE \ -f commit_id="$head_sha" \ - -f body="SDK reviewer app: approving a generated-paths-only diff on the regen branch. Auto-merge completes only after required status checks pass." + -f body="SDK reviewer app: approving a generated-paths-only diff on the regen branch. Auto-merge completes only after required status checks pass." \ + --jq '.id') + # Close the validate-then-approve race: a push landing between the + # allowlist check and the approval is not covered by + # dismiss_stale_reviews (that only dismisses on pushes after the + # review), so re-read the head and dismiss our own approval if it + # moved off the validated commit. + now_sha=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json headRefOid --jq '.headRefOid') + if [ "$now_sha" != "$head_sha" ]; then + echo "Head moved from $head_sha to $now_sha during approval; dismissing review." + GH_TOKEN="$APP_TOKEN" gh api -X PUT "repos/$GH_REPO/pulls/$pr/reviews/$review_id/dismissals" \ + -f message="Head moved during approval; the approved commit is no longer the PR head." \ + -f event=DISMISS + deny "SDK reviewer app: approval dismissed; the PR head changed while the review was being submitted. Left for human review (policy: fail open to human review, never to merge)." + fi + + # Post-validation head drift is closed by branch protection, not + # here: the approval is pinned to the validated commit_id and + # dismiss_stale_reviews dismisses it on any later push, so + # auto-merge fail-closes back to human review. enabled=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json autoMergeRequest --jq '.autoMergeRequest != null') if [ "$enabled" = "true" ]; then echo "Auto-merge already enabled on PR #$pr." From be6915413c49e89ca4593c477114e4377db3fe3f Mon Sep 17 00:00:00 2001 From: Smart Mekiliuwa Date: Mon, 20 Jul 2026 20:50:23 +0100 Subject: [PATCH 4/4] fix: make review dismissal best-effort ahead of deny a dismiss failure under set -e must not skip deny, which is the hard gate that clears stale auto-merge. dismiss_stale_reviews already covers the dismissal in the normal case. --- .github/workflows/sdk_generation.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sdk_generation.yaml b/.github/workflows/sdk_generation.yaml index c37304e..2e22bfe 100644 --- a/.github/workflows/sdk_generation.yaml +++ b/.github/workflows/sdk_generation.yaml @@ -244,9 +244,13 @@ jobs: now_sha=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json headRefOid --jq '.headRefOid') if [ "$now_sha" != "$head_sha" ]; then echo "Head moved from $head_sha to $now_sha during approval; dismissing review." + # Best-effort: dismiss_stale_reviews normally dismissed this + # approval already when the head moved. The hard gate is deny + # below, which clears auto-merge or fails the step; a dismiss + # error must not skip it. GH_TOKEN="$APP_TOKEN" gh api -X PUT "repos/$GH_REPO/pulls/$pr/reviews/$review_id/dismissals" \ -f message="Head moved during approval; the approved commit is no longer the PR head." \ - -f event=DISMISS + -f event=DISMISS || true deny "SDK reviewer app: approval dismissed; the PR head changed while the review was being submitted. Left for human review (policy: fail open to human review, never to merge)." fi