From 64389a2ce26a9ecd5fb745343f6247582def0eb8 Mon Sep 17 00:00:00 2001 From: ethernet Date: Wed, 15 Jul 2026 17:19:19 -0400 Subject: [PATCH] fix(ci): js-autofix pushes via PR instead of direct push to main (#65186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(js): never format package-lock.json prettier and eslint should never touch package-lock.json. main has a repo rule requiring team approval when lockfiles change, so an autofix PR touching it would hang waiting for review. - Add .prettierignore at repo root - Add '**/package-lock.json' to eslint shared config ignores * fix(ci): js-autofix pushes via PR instead of direct push to main Main now has repository rules requiring pull requests + required status checks ("All required checks pass"), so the workflow's direct push to main is rejected with GH013 every time eslint --fix produces changes. Switch apply-patch to push to a dedicated bot/js-autofix branch, create or update a PR, and enable auto-merge (squash). The PR auto-merges once CI passes. If CI fails or main moves, the PR is auto-closed and the branch deleted — the next run re-applies on the current state. The two-job security split is preserved: - generate-patch stays unprivileged (contents: read only) — it runs npm on an ephemeral runner with zero push permissions. - apply-patch (contents: write + pull-requests: write) still never runs npm, never installs anything, never executes repo code — it applies the trusted patch artifact and delivers it via PR. --- .github/workflows/js-autofix.yml | 120 ++++++++++++++++++++++++------- .prettierignore | 4 ++ eslint.config.shared.mjs | 2 +- 3 files changed, 101 insertions(+), 25 deletions(-) create mode 100644 .prettierignore diff --git a/.github/workflows/js-autofix.yml b/.github/workflows/js-autofix.yml index a3326ff324d0..f362f3c27dbb 100644 --- a/.github/workflows/js-autofix.yml +++ b/.github/workflows/js-autofix.yml @@ -1,14 +1,15 @@ name: auto-fix lint issues & formatting # On push to main (or manual trigger), run `npm run fix` on each workspace -# package and commit any changes. - +# package and apply any changes via a PR. +# # Fixable lint issues (import sorting, unused imports, curly braces, etc.) are # auto-corrected on merge so PRs aren't blocked by them. The PR-time eslint # check in typecheck.yml fails only when un-fixable errors remain. # # Pushes made by GITHUB_TOKEN don't trigger further workflow runs, so there's -# no infinite loop. +# no infinite loop — the push to bot/js-autofix and the resulting squash merge +# to main are both made by GITHUB_TOKEN. # # ── Security model: two-job split ─────────────────────────────────────────── # @@ -21,11 +22,14 @@ name: auto-fix lint issues & formatting # Worst case: malicious code runs here on an ephemeral runner with zero # push permissions. # -# 2. apply-patch (privileged, contents: write) -# Checks out, downloads the patch artifact, applies it, -# then pushes with --force-with-lease. This job never runs npm, never -# installs anything, never executes any repo code. The only input it -# trusts is the patch artifact. +# 2. apply-patch (privileged, contents: write + pull-requests: write) +# Checks out, downloads the patch artifact, applies it, pushes to the +# bot/js-autofix branch, creates/updates a PR, and enables auto-merge. +# This job never runs npm, never installs anything, never executes any +# repo code. The only input it trusts is the patch artifact. +# The PR auto-merges (squash) once CI passes. If CI fails or main moves, +# the PR is auto-closed and the branch deleted — the next run re-applies +# on the current state. on: push: @@ -106,9 +110,10 @@ jobs: name: Apply patch needs: generate-patch runs-on: ubuntu-latest - timeout-minutes: 3 + timeout-minutes: 15 permissions: - contents: write # needed to push + contents: write # needed to push to bot/js-autofix + pull-requests: write # needed for PR creation + auto-merge steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -122,10 +127,9 @@ jobs: # in the run step below. path: ${{ runner.temp }} - - name: Apply patch and push + - name: Apply patch and push to bot branch env: - START_SHA: ${{ github.sha }} - BRANCH: ${{ github.ref_name }} + BOT_BRANCH: bot/js-autofix run: | set -euo pipefail @@ -147,15 +151,83 @@ jobs: git add -A git commit -m "fmt(js): \`npm run fix\` on merge" - # --force-with-lease=: makes the precondition check atomic - # on the server side: the push only succeeds if origin/ still - # points at START_SHA when the server processes it. If the branch moved - # since checkout (another push landed), the server rejects it and - # the next run will re-apply on the current state. - if git push --force-with-lease="$BRANCH":"$START_SHA" origin HEAD:"$BRANCH"; then - echo "Pushed fixes." - else - echo "::error::$BRANCH moved since checkout ($START_SHA). Push rejected." - echo "Next run will re-apply on the current state." - exit 1 + # Push to the dedicated bot branch. Force-push is safe here: + # bot/js-autofix is a bot-only branch that gets rewritten each run. + # If the branch was deleted after a previous PR merge, this + # recreates it. + git push --force origin HEAD:"$BOT_BRANCH" + + - name: Create/update PR and enable auto-merge + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BOT_BRANCH: bot/js-autofix + run: | + set -euo pipefail + + # Create PR if one doesn't exist. If it already exists, the + # force-push above already updated it with the latest fixes. + PR_NUM=$(gh pr list --head "$BOT_BRANCH" --state open --json number --jq '.[0].number' 2>/dev/null || true) + if [ -z "$PR_NUM" ]; then + PR_NUM=$(gh pr create \ + --head "$BOT_BRANCH" --base main \ + --title 'fmt(js): `npm run fix` auto-fix' \ + --body 'Auto-generated by the `auto-fix lint issues & formatting` workflow. Auto-merges (squash) once CI passes. If CI fails or `main` moves, the PR is auto-closed and the branch deleted — the next run re-applies on the current state.' \ + --json number --jq '.number') fi + + # Enable auto-merge (squash). If already enabled, this is a no-op. + gh pr merge "$PR_NUM" --auto --squash || true + + - name: Wait for merge, auto-close on failure or stale + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + START_SHA: ${{ github.sha }} + run: | + set -euo pipefail + + PR_NUM=$(gh pr list --head bot/js-autofix --state open --json number --jq '.[0].number' 2>/dev/null || true) + if [ -z "$PR_NUM" ]; then + echo "No open PR. Nothing to wait for." + exit 0 + fi + + echo "Waiting for PR #$PR_NUM to merge..." + + # Poll every 15s for up to ~10 minutes. Auto-merge will handle the + # PR even if this job times out — the polling is for cleanup only + # (auto-close on CI failure, conflicts, or main moving). + for i in $(seq 1 40); do + sleep 15 + + STATE=$(gh pr view "$PR_NUM" --json state --jq '.state') + if [ "$STATE" = "MERGED" ] || [ "$STATE" = "CLOSED" ]; then + echo "PR #$PR_NUM is $STATE." + exit 0 + fi + + # If main moved, close the PR — the next workflow run re-applies + # on the current state. + CURRENT_SHA=$(gh api "repos/${{ github.repository }}/branches/main" --jq '.commit.sha') + if [ "$CURRENT_SHA" != "$START_SHA" ]; then + echo "Main moved ($START_SHA → $CURRENT_SHA). Closing stale PR." + gh pr close "$PR_NUM" --delete-branch + exit 0 + fi + + # If CI checks failed, close + delete the branch. + if gh pr checks "$PR_NUM" 2>/dev/null | grep -qi "fail"; then + echo "CI failed on PR #$PR_NUM. Closing + deleting branch." + gh pr close "$PR_NUM" --delete-branch + exit 0 + fi + + # If PR is conflicted, close + delete the branch. + MERGEABLE=$(gh pr view "$PR_NUM" --json mergeable --jq '.mergeable') + if [ "$MERGEABLE" = "CONFLICTING" ]; then + echo "PR #$PR_NUM is conflicted. Closing + deleting branch." + gh pr close "$PR_NUM" --delete-branch + exit 0 + fi + done + + echo "Timeout reached. Auto-merge will handle PR #$PR_NUM if CI passes." diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 000000000000..ca429279c2f0 --- /dev/null +++ b/.prettierignore @@ -0,0 +1,4 @@ +# Lockfiles must never be reformatted — main has a repo rule requiring +# team approval when lockfiles change, so an autofix PR touching one +# would hang waiting for review. +package-lock.json diff --git a/eslint.config.shared.mjs b/eslint.config.shared.mjs index c6723ece8fed..d20f00b511a1 100644 --- a/eslint.config.shared.mjs +++ b/eslint.config.shared.mjs @@ -22,7 +22,7 @@ import globals from 'globals' export default [ { - ignores: ['**/node_modules/**', '**/dist/**', 'src/**/*.js'] + ignores: ['**/node_modules/**', '**/dist/**', 'src/**/*.js', '**/package-lock.json'] }, js.configs.recommended, {