diff --git a/.github/workflows/keep-pr-current.yml b/.github/workflows/keep-pr-current.yml new file mode 100644 index 0000000000..e86a9ce3b6 --- /dev/null +++ b/.github/workflows/keep-pr-current.yml @@ -0,0 +1,121 @@ +name: Keep Pull Request Current + +# Replaces the Mergify `update` rule that used to merge `develop` into open +# pull requests (see `.github/mergify.yml` and #3574). +# +# Why this is a workflow and not a Mergify rule: Mergify reports a `failure` +# check whenever it cannot perform the update, with no option to change that. +# The schema accepts only `bot_account` for the action +# (https://docs.mergify.com/mergify-configuration-schema.json, `$defs.UpdateActionModel`, +# `additionalProperties: false`), so the red check could only be avoided by +# never attempting an update. That outcome is not gated — the attempt itself is +# what produced the failures. This workflow performs the same merge through +# GitHub's `update-branch` API and reports a conflict as a comment, so the +# check is green whether or not the merge was possible. +# +# The up-to-date requirement is unchanged: `develop-branch-ruleset` still sets +# `strict_required_status_checks_policy`, so a branch that is behind `develop` +# and cannot be updated is still blocked from merging. Nothing here relaxes the +# ruleset, and this workflow is deliberately not a required check. +# +# `synchronize` is deliberately absent from the `pull_request` types. Updating a +# branch is itself a push to it, so including `synchronize` would make the +# update re-trigger this workflow and loop. Branches fall behind because +# `develop` moves, which is why `push` is the trigger that matters. +# +# GitHub cannot run `pull_request` workflows at all on a pull request that +# conflicts (`refs/pull//merge` is 404 because the merge ref cannot be +# built), so a conflicting pull request is only ever reached from a push to +# `develop`. The `pull_request` trigger exists to cover the opened/reopened and +# ready-for-review cases. +on: + push: + branches: + - develop + pull_request: + types: + - opened + - reopened + - ready_for_review + workflow_dispatch: + +permissions: + contents: write + pull-requests: write + +# One run per repository state. Queued behind any run already in flight so a +# burst of pushes to `develop` cannot have two runs merging the same branch. +concurrency: + group: keep-pr-current + cancel-in-progress: false + +jobs: + keep-current: + name: Keep pull request current + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + # Sparse checkout: the decision logic in scripts/ is all this needs, and + # no pull request code is ever checked out. + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + fetch-depth: 1 + sparse-checkout: | + scripts/automation + + - name: Set up Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version-file: '.nvmrc' + + - name: Merge develop into open pull requests + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + // Relative specifier, matching the other workflows here + // (ai-feedback-validation.yml, orchestrate-phase-progression.yml). + const { processPullRequest } = require('./scripts/automation/keep-pr-current.cjs'); + + const { owner, repo } = context.repo; + + // A pull_request event carries its own number; a push to develop has + // to enumerate what is open against it. + let numbers = []; + if (context.eventName === 'pull_request') { + numbers = [context.payload.pull_request.number]; + } else { + const open = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + base: 'develop', + per_page: 100, + }); + numbers = open.map((item) => item.number); + } + + core.info(`Considering ${numbers.length} pull request(s) against develop.`); + + // Each pull request is handled independently by processPullRequest, + // which owns its own log lines and its own comment. Nothing is + // carried between iterations. + const summary = []; + for (const number of numbers) { + const result = await processPullRequest({ github, owner, repo, number, logger: core }); + summary.push(`#${number}: ${result.outcome}`); + } + + core.info(`Processed ${numbers.length} pull request(s).`); + await core.summary + .addHeading('Keep pull request current') + .addTable([ + [ + { data: 'Pull request', header: true }, + { data: 'Outcome', header: true }, + ], + ...summary.map((line) => [line.split(':')[0], line.split(':').slice(1).join(':').trim()]), + ]) + .write(); diff --git a/docs/BRANCHING_STRATEGY.md b/docs/BRANCHING_STRATEGY.md index a19c9c7df1..625d1d6532 100644 --- a/docs/BRANCHING_STRATEGY.md +++ b/docs/BRANCHING_STRATEGY.md @@ -1176,3 +1176,5 @@ This section provides comprehensive details for each of the 24 allowed branch ty > For questions or improvements, open an issue or PR in the `.github` repo. *Have questions? Ping us on GitHub! 🐙 Made with 💚 by LightSpeedWP* + + diff --git a/scripts/automation/keep-pr-current.cjs b/scripts/automation/keep-pr-current.cjs new file mode 100644 index 0000000000..ee4b415c68 --- /dev/null +++ b/scripts/automation/keep-pr-current.cjs @@ -0,0 +1,328 @@ +/** + * Decide whether to merge `develop` into a pull request branch, and how to + * report the outcome. + * + * This replaces the Mergify `update` rule that used to own this job + * (`.github/mergify.yml`, removed in the pull request that added this file). + * The behaviour is deliberately identical — merge the base in, skip drafts and + * forks, act on any staleness — but the *reporting* is different, and that is + * the whole point of the change. + * + * Mergify's `update` action has no option that affects how a failure is + * reported. Its schema accepts only `bot_account` + * (https://docs.mergify.com/mergify-configuration-schema.json, `$defs.UpdateActionModel`, + * `additionalProperties: false`), and when Mergify cannot perform the update it + * reports the rule's check run with conclusion `failure`. That happens both + * when the update genuinely conflicts and when Mergify declines for its own + * reasons, so a red `Base branch update has failed` is left behind on a pull + * request that nobody can act on. See #3574. + * + * Everything here is a pure function of its arguments, or takes an injected + * Octokit-like client, so the behaviour is testable without a network round trip. + */ + +/** + * Marker used to find and update this workflow's own comment, so repeated runs + * on the same pull request do not stack up duplicates. Deliberately distinct + * from the AI Feedback Validation marker and from the PR template routing + * comment's `## PR Template Routing` heading. + */ +const CONFLICT_COMMENT_MARKER = ''; + +/** + * Should the base branch be merged into this pull request's head branch? + * + * The exclusions mirror what the Mergify rule used to express, and Mergify's + * own documented preconditions (https://docs.mergify.com/workflow/actions/update: + * "A pull request is updated only when it is open, has no conflict, is not in a + * merge queue, and is behind its base branch"). + * + * `mergeableState` is advisory only and never blocks the attempt. GitHub + * reports `unknown` for a short window after the base branch moves, and + * treating that window as "conflicting" would strand every open pull request + * on every push to `develop`. The update API is the real test: it either + * merges, or it reports a conflict, and the conflict path is reported as + * information rather than as a failure. + * + * @param {object} pullRequest - A `pulls.get` response. + * @returns {{attempt: boolean, reason: string}} + */ +function shouldAttemptUpdate(pullRequest) { + if (!pullRequest || pullRequest.state !== 'open') { + return { attempt: false, reason: 'not-open' }; + } + + if (pullRequest.draft) { + return { attempt: false, reason: 'draft' }; + } + + const headOwner = pullRequest.head?.repo?.owner?.login; + const baseOwner = pullRequest.base?.repo?.owner?.login; + if (headOwner && baseOwner && headOwner !== baseOwner) { + return { attempt: false, reason: 'from-fork' }; + } + + if (pullRequest.head?.repo?.archived) { + return { attempt: false, reason: 'archived-head' }; + } + + if (pullRequest.base?.ref && pullRequest.base.ref !== 'develop') { + return { attempt: false, reason: 'not-develop' }; + } + + return { attempt: true, reason: 'behind' }; +} + +/** + * Classify the result of `PUT /repos/{owner}/{repo}/pulls/{pull_number}/update-branch`. + * + * The status alone is not enough, because GitHub answers **422 for three + * unrelated situations**. All four responses below were observed against this + * repository on 2026-09-30 against a deliberately wrong `expected_head_sha`: + * + * | HTTP | message | when | + * | ---- | --------------------------------------------------- | ------------------------------------- | + * | 202 | `Updating pull request branch.` | the merge was started | + * | 422 | `There are no new commits on the base branch.` | already current | + * | 422 | `merge conflict between base and head` | head conflicts with base | + * | 422 | `head ref does not exist` | head branch deleted after the merge | + * | 404 | `Not Found` | the pull request itself is gone | + * + * The third is the same text Mergify reported on #3580 and #3662, seconds + * after those pull requests merged and their branches were auto-deleted. Reading + * it as a conflict would post a false conflict comment on a merged pull + * request, so the message is what decides. + * + * GitHub documents only 202, 403 and 422 for this endpoint + * (https://docs.github.com/en/rest/pulls/pulls#update-a-pull-request-branch), + * where 422 is described as "Validation failed, or the endpoint has been + * spammed" — so a secondary rate limit can also arrive as 422 and is treated as + * retryable rather than as a conflict. + * + * An unrecognised response is `error`. It is surfaced as a warning and in the + * job summary, never as a passing no-op, and never as a conflict comment on a + * pull request that may have nothing wrong with it. + * + * @param {{status?: number, message?: string}} outcome + * @returns {'updated'|'current'|'conflict'|'gone'|'denied'|'retry'|'error'} + */ +function classifyUpdateResult({ status, message = '' } = {}) { + const text = String(message).toLowerCase(); + + // 202 Accepted — the base branch was merged into the head branch. + if (status === 202) { + return 'updated'; + } + + // Message checks come before status checks, because the 422 cases are + // distinguished only by their message. + if (text.includes('no new commits on the base branch')) { + return 'current'; + } + if (text.includes('merge conflict between base and head')) { + return 'conflict'; + } + if (text.includes('head ref does not exist')) { + return 'gone'; + } + + // Rate limiting and endpoint throttling: documented as 403/422, and the + // message is the only reliable signal. Not observed on this repository. + if ( + status === 429 || + /rate limit|abuse detection|try again later|endpoint has been spammed/.test(text) + ) { + return 'retry'; + } + + // 403 Forbidden. GitHub documents the status but not the message; the most + // common cause is a token that cannot write this head branch, for example a + // fork whose author has not enabled maintainer edits. Not observed here. + if (status === 403) { + return 'denied'; + } + + // 404 Not Found. Observed for a pull request that no longer exists. + if (status === 404) { + return 'gone'; + } + + // 5xx and anything unrecognised, including a 422 whose message is not one of + // the three above. + return 'error'; +} + +/** + * Build the comment posted when a pull request cannot be updated automatically. + * + * @param {object} params + * @param {number} params.pullNumber + * @param {string} params.mergeableState - `mergeable_state` from the pull request. + * @param {string[]} [params.log] - Log lines for this pull request only. + * @returns {string} Markdown comment body. + */ +function buildConflictComment({ pullNumber, mergeableState = 'unknown', log = [] }) { + const conflictUrl = `https://github.com/lightspeedwp/.github/pull/${pullNumber}/conflicts`; + + return [ + CONFLICT_COMMENT_MARKER, + `## Pull request #${pullNumber} could not be updated automatically`, + '', + `\`develop\` cannot be merged into this branch automatically because the two conflict`, + `(\`mergeable_state: ${mergeableState}\`). Merging cannot resolve a conflict, so this needs`, + 'a human. This is a property of the branch, not a failure of any check — no check is', + 'reporting red because of it, and the up-to-date requirement on `develop` is unchanged.', + '', + '**To resolve it:**', + '', + '1. Merge `develop` into this branch locally (`git merge origin/develop`) and push the', + ' result, or rebase the branch onto `develop`.', + '2. Open the **Resolve conflicts** view to jump straight to the conflicting files:', + ` ${conflictUrl}`, + '', + 'Common conflict sources in this repository are the per-pull-request files that every', + 'pull request rewrites (`FEEDBACK_RESPONSE.md`) and the shared `[Unreleased]` list in', + '`CHANGELOG.md`. `CHANGELOG.md` is union-merged via `.gitattributes`, so it merges cleanly;', + 'a conflict there means a change larger than an added entry.', + ...(log.length + ? ['', '
Log', '', '```', ...log, '```', '
'] + : []), + '', + ].join('\n'); +} + +/** + * The subset of {@link buildConflictComment} that changes as the branch + * changes, used to decide whether an existing comment still needs updating. + * + * @param {string} body + * @returns {string} + */ +function normaliseCommentBody(body) { + return String(body || '') + .split('\n') + .filter((line) => !/^\s*https:\/\/github\.com\//.test(line)) + .join('\n') + .trim(); +} + +/** + * Run the update for one pull request and report the outcome. + * + * The per-pull-request log is built here rather than by the caller, so state + * cannot carry from one pull request to the next: the caller iterates over many + * pull requests in a single run, and a log array shared across iterations put + * every earlier pull request's status into a later one's comment. + * + * @param {object} params + * @param {object} params.github - Octokit-like client (actions/github-script `github`). + * @param {string} params.owner + * @param {string} params.repo + * @param {number} params.number - Pull request number. + * @param {{info: Function, warning: Function}} [params.logger] - actions/core-like. + * @returns {Promise<{outcome: string, status?: number, message?: string, commented?: boolean}>} + */ +async function processPullRequest({ github, owner, repo, number, logger = console }) { + let pullRequest; + try { + ({ data: pullRequest } = await github.rest.pulls.get({ owner, repo, pull_number: number })); + } catch (error) { + // Merged or deleted between listing and reading. Nothing to do, and not a + // failure of this workflow. + logger.info(`#${number}: skipped, could not read (${error.status}).`); + return { outcome: 'unreadable', status: error.status, message: error.message || '' }; + } + + const decision = shouldAttemptUpdate(pullRequest); + if (!decision.attempt) { + logger.info(`#${number}: skipped (${decision.reason}).`); + return { outcome: 'skipped', message: decision.reason }; + } + + let status; + let message = ''; + try { + // expected_head_sha is documented as the guard against merging into a head + // that moved + // (https://docs.github.com/en/rest/pulls/pulls#update-a-pull-request-branch: + // "If the expected SHA does not match the pull request's HEAD, you will + // receive a 422 Unprocessable Entity status"). It was **not** observed to + // be enforced on 2026-09-30 — a deliberately wrong value still returned 202 + // and updated the branch — so it is defence in depth, not the thing that + // makes this call safe. The staleness check also runs first, which is why a + // wrong value on an already-current pull request still returns the + // "no new commits" 422. + const response = await github.rest.pulls.updateBranch({ + owner, + repo, + pull_number: number, + expected_head_sha: pullRequest.head.sha, + }); + status = response.status; + } catch (error) { + status = error.status; + // The API's own message is what distinguishes the 422 cases, so prefer it + // over Octokit's wrapper text. + message = error.response?.data?.message || error.message || ''; + } + + const outcome = classifyUpdateResult({ status, message }); + logger.info(`#${number}: ${outcome} (HTTP ${status}${message ? `, ${message}` : ''}).`); + + if (outcome === 'conflict') { + // Scoped to this pull request. Deliberately not accumulated across calls. + const log = [`#${number} HTTP ${status}: ${message}`]; + const body = buildConflictComment({ + pullNumber: number, + mergeableState: pullRequest.mergeable_state || 'unknown', + log, + }); + + // Upsert rather than append, so a pull request that stays conflicting + // across several pushes to develop carries one comment instead of one per + // run. Paginated, so a pull request with more than a page of earlier + // comments still finds its own. + const comments = await github.paginate(github.rest.issues.listComments, { + owner, + repo, + issue_number: number, + per_page: 100, + }); + const existing = comments.find( + (comment) => + comment.user?.type === 'Bot' && String(comment.body || '').includes(CONFLICT_COMMENT_MARKER) + ); + + if (existing && normaliseCommentBody(existing.body) === normaliseCommentBody(body)) { + logger.info(`#${number}: conflict comment already up to date.`); + return { outcome, status, message, commented: false }; + } + if (existing) { + await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number: number, body }); + } + return { outcome, status, message, commented: true }; + } + + // Surfaced rather than logged quietly. The job still exits 0, because a + // repository or permission problem is not a reason to fail every pull + // request's check, but it must not look like a clean success either. + if (outcome === 'error' || outcome === 'denied' || outcome === 'retry') { + logger.warning( + `#${number}: ${outcome} (HTTP ${status}${message ? `, ${message}` : ''}) — ` + + 'no conflict comment was posted and this pull request was not updated.' + ); + } + + return { outcome, status, message }; +} + +module.exports = { + CONFLICT_COMMENT_MARKER, + buildConflictComment, + classifyUpdateResult, + normaliseCommentBody, + processPullRequest, + shouldAttemptUpdate, +};