Correct the merge trigger: pull_request, and why not pull_request_target - #118
Merged
Conversation
The page said GitHub reads a `pull_request_target` workflow from the pull request's base branch. That is wrong, and it is the mistake that made the merge path inert: `pull_request_target` is read from the repository's DEFAULT branch, `stable`. A stub living on working-1.6 and dev-branch was never consulted -- it did not appear in fogproject's Actions workflow list at all, and four pull requests merged into working-1.6 without producing a run. fogproject's stub now uses `pull_request: types: [closed]`, which does resolve per-ref and so reaches the file where it lives. Adds a short section on the general point, because the safety rule already documented here is not the only question a trigger has to answer. Safety is "can the bot's own push raise this event". Reachability is "which ref is this trigger read from" -- most events, including pull_request_target, schedule and workflow_dispatch, are read only from the default branch, while push, create and pull_request resolve per-ref. Getting the first right and the second wrong produces a workflow that is correct and silent. It ends on the detection problem rather than the rule, since that is what actually cost the time here: a workflow that never runs looks exactly like one that ran and found nothing to do. The only way to tell is to confirm a run happened. Also records the trade that came with the change: `pull_request` withholds secrets from a fork PR, so the App token cannot be minted there. The stub skips merged fork PRs with a same-repo guard and the daily schedule picks them up, the same way it covers direct pushes -- so the schedule's coverage sentence now names fork PRs alongside them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PqufBbuckux8kitJeW3uAK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes a factual error introduced in #116 and documents the general point behind it.
The error
The page said GitHub reads a
pull_request_targetworkflow from the pull request's base branch. It doesn't — it reads it from the repository's default branch,stable.That's the mistake that made the merge path inert. A stub living on
working-1.6anddev-branchwas never consulted: it didn't appear in fogproject's Actions workflow list at all, and four pull requests merged intoworking-1.6without producing a run. fogproject's stub now usespull_request: types: [closed], which does resolve per-ref (FOGProject/fogproject#1171, #1172).New section
The safety rule already on this page isn't the only question a trigger has to answer:
pull_request_target,scheduleandworkflow_dispatch, are read only from the default branch;push,createandpull_requestresolve per-ref.Getting the first right and the second wrong produces a workflow that is correct and silent. The section ends on the detection problem rather than the rule, because that's what actually cost the time: a workflow that never runs looks exactly like one that ran and found nothing to do. The only way to tell them apart is to confirm a run happened.
Also recorded
The trade that came with the change:
pull_requestwithholds secrets from fork PRs, so the App token can't be minted there. The stub skips merged fork PRs with a same-repo guard and the daily schedule picks them up — so the schedule's coverage sentence now names fork PRs alongside direct pushes.translations/untouched, as before — machine-generated fromdocs/.🤖 Generated with Claude Code
https://claude.ai/code/session_01PqufBbuckux8kitJeW3uAK