From d634bdfed7306eae6a8e89e9a4779c0e2a5d4a74 Mon Sep 17 00:00:00 2001 From: JJ Fullmer Date: Mon, 17 Aug 2026 21:33:11 -0600 Subject: [PATCH] Trigger the merge sync from an event that actually reaches this file The stub added in #1160 never fired. Not once, across four PRs merged into working-1.6 -- it did not even appear in the repository's Actions workflow list. The cause is that `pull_request_target` is read from the repository's DEFAULT branch, `stable`, and not from the pull request's base branch. GitHub documents this plainly ("runs in the context of the default branch of the base repository"), and it is the general rule for most events -- `push` and `create` are the exceptions that resolve per-ref. A copy of the workflow living on working-1.6 and dev-branch is therefore never consulted, however correct its contents. `tests.yml` was the wrong precedent to reason from. It uses `pull_request`, which is evaluated from the head-into-base merge ref, which is why one copy per base branch works there and why it works without a copy on stable. So use `pull_request: types: [closed]`, which is read from the base branch and therefore reaches this file where it already lives. The alternative -- putting the stub on stable -- would work, but only stable's copy would ever execute, so editing this file on working-1.6 would silently do nothing and every future change to the sync behaviour would need a pull request into the release branch. That trade costs fork PRs. `pull_request` withholds secrets from a fork, and the reusable workflow needs FOG_WORKFLOWS_PRIVATE_KEY to mint its App token, so a merged fork PR would fail rather than sync. The added same-repo guard skips those instead, and the daily sweep picks them up -- the same gap, and the same backstop, that direct pushes already rely on. A gap the schedule already covers beats a red X on every external contribution. Everything else is unchanged: still `pull_request`-family rather than `push`, so the bot's own fixup push cannot re-fire it and the 2026-07-28 loop stays closed; still the same branch allowlist keeping rc-*/feature-* on the schedule; still no logic in this repo. The header now records why `pull_request_target` is wrong here, so the next reader does not switch it back on the reasonable-looking grounds that it is the variant which gets fork secrets. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PqufBbuckux8kitJeW3uAK --- .github/workflows/sync-generated-files.yml | 51 +++++++++++++++------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/.github/workflows/sync-generated-files.yml b/.github/workflows/sync-generated-files.yml index 9c8ca7f47d..b690ecd516 100644 --- a/.github/workflows/sync-generated-files.yml +++ b/.github/workflows/sync-generated-files.yml @@ -10,27 +10,45 @@ name: Sync generated files and version # FOG_VERSION goes stale until the daily sweep fires at 10:10 UTC. This closes # that window. # -# pull_request_target, NOT push -- and that distinction is the whole safety -# argument, not caution. The sweep pushes its fixup commit straight to the -# branch, and a direct push is not a PR merge, so it cannot re-fire this stub. -# The 2026-07-28 runaway that put ~30 commits on dev-branch in about 20 minutes -# was a push-triggered stub doing exactly that. The schedule in fog-workflows -# stays exactly as it is, and remains the backstop for direct pushes and for -# rc-*/feature-* branches. +# WHY `pull_request`, AND NOT `pull_request_target` # -# pull_request_target rather than pull_request because secrets are withheld from -# fork PRs, and the reusable workflow needs FOG_WORKFLOWS_PRIVATE_KEY to mint its -# App token. The usual pull_request_target hazard does not apply here: nothing -# checks out the PR head. The reusable workflow checks out FOGProject/fogproject -# at the base branch -- code a maintainer has already merged. +# `pull_request_target` looks like the right answer -- it is the variant that +# gets secrets on fork PRs -- and the first version of this file used it. It +# never fired once. GitHub reads a `pull_request_target` workflow from the +# repository's DEFAULT branch (`stable` here), not from the PR's base branch, so +# a copy living on working-1.6 and dev-branch is simply never consulted: the +# workflow did not even appear in the Actions list, and four PRs merged into +# working-1.6 without it running. +# +# `pull_request` is read from the base branch instead, so this file works where +# it actually lives. Do not "fix" it back to `pull_request_target` without also +# putting the file on `stable` -- and note that then only stable's copy would +# execute, which makes editing the version on this branch a no-op. +# +# WHY NOT `push` +# +# That distinction is the whole safety argument, not caution. The sweep pushes +# its fixup commit straight to the branch, and a direct push is not a PR merge, +# so it cannot re-fire this stub. The 2026-07-28 runaway that put ~30 commits on +# dev-branch in about 20 minutes was a push-triggered stub doing exactly that. +# The schedule in fog-workflows stays exactly as it is, and remains the backstop +# for direct pushes and for rc-*/feature-* branches. +# +# WHY THE SAME-REPO GUARD +# +# `pull_request` withholds secrets from fork PRs, and the reusable workflow needs +# FOG_WORKFLOWS_PRIVATE_KEY to mint its App token -- so on a fork PR it would +# fail rather than work. Skipping is right: a merged fork PR is picked up by the +# daily sweep, exactly as a direct push already is. Better a gap the schedule +# already covers than a red X on every external contribution. # # One file, identical on every branch that carries it, for the same reason -# tests.yml is: fixing it should not mean editing it on three branches. GitHub -# reads this from the PR's BASE branch, so each branch's copy only ever acts on -# merges into that branch, and the allowlist below is what scopes it. +# tests.yml is: fixing it should not mean editing it on three branches. Each +# branch's copy only ever acts on merges into that branch, and the allowlist +# below is what scopes it. on: - pull_request_target: + pull_request: types: [closed] concurrency: @@ -51,6 +69,7 @@ jobs: # against its own watched list, so constraining it is this file's job. if: >- github.event.pull_request.merged == true + && github.event.pull_request.head.repo.full_name == github.repository && contains(fromJson('["working-1.6", "dev-branch"]'), github.event.pull_request.base.ref) # Least privilege, stated rather than inherited. Everything the reusable