CI: skip queue suites already passed on the exact same merge (#1130)

.github/workflows/phpunit-feature.yml

@@ -112,7 +112,7 @@ jobs:
         run: node --test tests/Unit/PatchCohortMaintenanceTest.mjs
 
       - name: Run the required checks for merge queue groups
-        run: node --test tests/Unit/MergeGroupTriggersTest.mjs tests/Unit/MergeQueueSonarTest.mjs
+        run: node --test tests/Unit/MergeGroupTriggersTest.mjs tests/Unit/MergeQueueSonarTest.mjs tests/Unit/TestedMergeTest.mjs
 
       - name: Report one Shared Server fixtures result on every pull request
         run: node --test tests/Unit/ServerParityAggregateTest.mjs tests/Unit/ServerParityPartsTest.mjs
@@ -192,11 +192,54 @@ jobs:
       - name: Require all PHP replay runtime cells and exact artifact identity
         run: python3 -m unittest tests/Unit/Support/php_replay_published_test.py tests/Unit/Support/php_condition_wait_test.py
 
+  # A merge-queue group built straight onto the base a pull request run was
+  # tested on merges the same two parents into the same tree. When that pull
+  # request run passed, the queue does not repeat the feature suite (Server
+  # #1130). Repository contracts and the central action policy still run on
+  # every group, and any doubt runs the suite. See scripts/ci/tested-merge.mjs.
+  tested-merge:
+    name: Tested merge
+    runs-on: ubuntu-latest
+    timeout-minutes: 3
+    # Only the merge-group check reads workflow runs and artifacts.
+    permissions:
+      contents: read
+      actions: read
+    outputs:
+      marker: ${{ steps.record.outputs.marker }}
+      tested: ${{ steps.check.outputs.tested }}
+    steps:
+      - name: Check out the merge and its parents
+        uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803  # v6
+        with:
+          fetch-depth: 2
+          persist-credentials: false
+          sparse-checkout: |
+            scripts/ci/tested-merge.sh
+            scripts/ci/tested-merge.mjs
+          sparse-checkout-cone-mode: false
+
+      - name: Name the marker a passing pull request run records
+        id: record
+        if: ${{ github.event_name == 'pull_request' }}
+        env:
+          PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        run: bash scripts/ci/tested-merge.sh record phpunit-feature .github/workflows/phpunit-feature.yml >> "$GITHUB_OUTPUT"
+
+      - name: Find a passing pull request run of this exact merge
+        id: check
+        if: ${{ github.event_name == 'merge_group' }}
+        env:
+          MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }}
+          RUNS_TOKEN: ${{ github.token }}
+        run: bash scripts/ci/tested-merge.sh check phpunit-feature .github/workflows/phpunit-feature.yml >> "$GITHUB_OUTPUT"
+
   feature:
     name: PHPUnit feature suite
-    needs: action-policy
-    # Report policy failures while allowing cancelled revisions to stop.
-    if: ${{ always() && !cancelled() }}
+    needs: [action-policy, tested-merge]
+    # Report policy failures while allowing cancelled revisions to stop. A
+    # failed tested-merge check reports nothing, so the suite runs.
+    if: ${{ always() && !cancelled() && needs.tested-merge.outputs.tested != 'true' }}
     runs-on: ubuntu-latest
     timeout-minutes: 25
     steps:
@@ -490,22 +533,49 @@ jobs:
 
   qualification:
     name: Feature source qualification
-    needs: [preflight, feature]
+    needs: [preflight, tested-merge, feature]
     if: ${{ always() && !cancelled() }}
     runs-on: ubuntu-latest
     timeout-minutes: 2
 
     steps:
       - name: Require repository contracts and feature tests
         env:
+          EVENT_NAME: ${{ github.event_name }}
           PREFLIGHT_RESULT: ${{ needs.preflight.result }}
           FEATURE_RESULT: ${{ needs.feature.result }}
+          TESTED_MERGE: ${{ needs.tested-merge.outputs.tested }}
         run: |
           set -euo pipefail
           test "$PREFLIGHT_RESULT" = success
+          if [ "$EVENT_NAME" = merge_group ] && [ "$TESTED_MERGE" = true ] && [ "$FEATURE_RESULT" = skipped ]; then
+            echo "Server repository contracts succeeded; the feature tests already passed on this exact merge in its pull request run."
+            exit 0
+          fi
           test "$FEATURE_RESULT" = success
           echo "Server repository contracts and feature tests succeeded."
 
+      # Only after every check above passed. An upload failure only means the
+      # queue runs the suite again.
+      - name: Write the tested-merge marker
+        if: ${{ success() && github.event_name == 'pull_request' && needs.tested-merge.outputs.marker != '' }}
+        env:
+          MARKER: ${{ needs.tested-merge.outputs.marker }}
+        run: |
+          mkdir -p "$RUNNER_TEMP/tested-merge"
+          printf '%s\n' "$MARKER" > "$RUNNER_TEMP/tested-merge/marker.txt"
+
+      - name: Record that this exact merge passed
+        if: ${{ success() && github.event_name == 'pull_request' && needs.tested-merge.outputs.marker != '' }}
+        continue-on-error: true
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a  # v7
+        with:
+          name: ${{ needs.tested-merge.outputs.marker }}
+          path: ${{ runner.temp }}/tested-merge/marker.txt
+          if-no-files-found: error
+          retention-days: 7
+          overwrite: true
+
   prepared-local-source:
     name: Candidate cancellation and prepared-local source qualification
     needs: [action-policy, preflight]

.github/workflows/server-parity.yml

@@ -169,20 +169,40 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 5
     # Reading this workflow's own runs lets a push to main that the merge queue
-    # already qualified skip repeating that qualification.
+    # already qualified skip repeating that qualification. Reading runs and
+    # artifacts lets a merge group that is exactly a merge a pull request run
+    # already passed skip repeating it (Server #1130).
     permissions:
       contents: read
       actions: read
     outputs:
-      run: ${{ steps.classify.outputs.run }}
+      run: ${{ steps.tested.outputs.tested == 'true' && 'false' || steps.classify.outputs.run }}
       full: ${{ steps.classify.outputs.full }}
+      tested: ${{ steps.tested.outputs.tested }}
+      marker: ${{ steps.record.outputs.marker }}
     steps:
       - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6
         with:
           fetch-depth: 2
           persist-credentials: false
       - name: Reject unsafe Docker ownership before starting fixture jobs
         run: python3 scripts/ci/check-docker-workflow-isolation.py
+      # The result job records this name only after every fixture job passed.
+      - name: Name the marker a passing pull request run records
+        id: record
+        if: ${{ github.event_name == 'pull_request' }}
+        env:
+          PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        run: bash scripts/ci/tested-merge.sh record server-parity .github/workflows/server-parity.yml >> "$GITHUB_OUTPUT"
+      # Same parents and tree as a passing pull request run: nothing new to
+      # qualify. Any doubt reports tested=false and the classifier decides.
+      - name: Find a passing pull request run of this exact merge
+        id: tested
+        if: ${{ github.event_name == 'merge_group' }}
+        env:
+          MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }}
+          RUNS_TOKEN: ${{ github.token }}
+        run: bash scripts/ci/tested-merge.sh check server-parity .github/workflows/server-parity.yml >> "$GITHUB_OUTPUT"
       - name: Decide whether fixture inputs changed
         id: classify
         env:
@@ -3006,3 +3026,22 @@ jobs:
         env:
           NEEDS: ${{ toJSON(needs) }}
         run: bash scripts/conformance/server-parity/aggregate.sh
+      # Only after the result passed. An upload failure only means the queue
+      # runs the fixtures again.
+      - name: Write the tested-merge marker
+        if: ${{ success() && github.event_name == 'pull_request' && needs.changes.outputs.marker != '' }}
+        env:
+          MARKER: ${{ needs.changes.outputs.marker }}
+        run: |
+          mkdir -p "$RUNNER_TEMP/tested-merge"
+          printf '%s\n' "$MARKER" > "$RUNNER_TEMP/tested-merge/marker.txt"
+      - name: Record that this exact merge passed
+        if: ${{ success() && github.event_name == 'pull_request' && needs.changes.outputs.marker != '' }}
+        continue-on-error: true
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
+        with:
+          name: ${{ needs.changes.outputs.marker }}
+          path: ${{ runner.temp }}/tested-merge/marker.txt
+          if-no-files-found: error
+          retention-days: 7
+          overwrite: true

scripts/ci/tested-merge.mjs

@@ -0,0 +1,132 @@
+#!/usr/bin/env node
+// Lets a merge-queue group skip a suite that already passed on exactly the
+// same merge (Server #1130).
+//
+// A pull request run checks out GitHub's test merge of the pull request head
+// into its base. When a merge-queue group is built straight onto that same
+// base, the group commit merges the same two parents and has the same tree:
+// the files, workflows and base revisions the suite sees are identical, so
+// running it again proves nothing new.
+//
+// `record` runs in the pull request run. It names a marker artifact after the
+// workflow, the merge's first parent and its tree. The workflow uploads the
+// marker only after its suites passed.
+//
+// `check` runs in the merge group. It skips only when all of these hold:
+//   - the checkout is this merge group's head commit, a two-parent merge;
+//   - a marker exists for this workflow, this group's first parent and tree;
+//   - the run that uploaded it is a completed, successful `pull_request` run
+//     of this same workflow file, in this repository, from a branch in this
+//     repository (never a fork), on exactly the group's second parent.
+// A group stacked on an unmerged predecessor has a first parent no pull
+// request was tested on, so it runs everything. Any other doubt, error or
+// missing permission runs everything.
+import {execFileSync} from 'node:child_process';
+import {pathToFileURL} from 'node:url';
+
+const SHA = /^[0-9a-f]{40}$/;
+const KEY = /^[a-z0-9-]{1,40}$/;
+
+export function markerName(key, base, tree) {
+  if (!KEY.test(key ?? '')) throw new Error('Invalid workflow key');
+  if (!SHA.test(base ?? '') || !SHA.test(tree ?? '')) throw new Error('Invalid merge identity');
+  return `tested-merge-${key}-${base}-${tree}`;
+}
+
+// The checked-out merge commit: its two parents and its tree. The runner's
+// fixed Git runs without a shell or PATH lookup, from the current directory.
+export function localMerge(git = (...args) => execFileSync('/usr/bin/git', args, {encoding: 'utf8'})) {
+  const [commit, ...parents] = git('rev-list', '--parents', '-n', '1', 'HEAD').trim().split(' ');
+  const tree = git('rev-parse', 'HEAD^{tree}').trim();
+  if (!SHA.test(commit) || parents.length !== 2 || !parents.every(parent => SHA.test(parent)) || !SHA.test(tree)) {
+    throw new Error('HEAD is not a two-parent merge commit');
+  }
+  return {commit, base: parents[0], head: parents[1], tree};
+}
+
+// The marker this pull request run may record, or an error.
+export function recordName({key, prHeadSha, merge}) {
+  if (!SHA.test(prHeadSha ?? '') || merge.head !== prHeadSha) {
+    throw new Error('HEAD is not this pull request\'s test merge');
+  }
+  return markerName(key, merge.base, merge.tree);
+}
+
+// Whether a successful pull request run of `workflowPath` already tested this
+// merge group's exact merge. Returns {tested, reason}; every doubt is false.
+export async function testedRun({key, workflowPath, merge, mergeGroupHeadSha, repository, repositoryId, token,
+  apiUrl = 'https://api.github.com', fetchImpl = fetch}) {
+  const no = reason => ({tested: false, reason});
+  if (!SHA.test(mergeGroupHeadSha ?? '') || merge?.commit !== mergeGroupHeadSha) return no('HEAD is not this merge group');
+  if (!/^\.github\/workflows\/[a-z0-9-]+\.yml$/.test(workflowPath ?? '')) return no('invalid workflow path');
+  if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repository ?? '') || !/^[1-9]\d*$/.test(String(repositoryId ?? ''))) {
+    return no('invalid repository identity');
+  }
+  if (!token) return no('no token to read workflow runs');
+  if (!/^https:\/\/[A-Za-z0-9.-]+(?::\d+)?(?:\/[A-Za-z0-9._/-]*)?$/.test(apiUrl)) return no('invalid API URL');
+  let name;
+  try {
+    name = markerName(key, merge.base, merge.tree);
+  } catch (error) {
+    return no(error.message);
+  }
+  const get = async path => {
+    const response = await fetchImpl(`${apiUrl}/repos/${repository}${path}`, {
+      headers: {authorization: `Bearer ${token}`, accept: 'application/vnd.github+json', 'x-github-api-version': '2022-11-28'},
+      signal: AbortSignal.timeout(30_000),
+    });
+    if (!response.ok) throw new Error(`GitHub API answered ${response.status}`);
+    return response.json();
+  };
+  const id = Number(repositoryId);
+  try {
+    const listing = await get(`/actions/artifacts?${new URLSearchParams({name, per_page: '100'})}`);
+    const candidates = (Array.isArray(listing?.artifacts) ? listing.artifacts : []).filter(artifact => artifact?.name === name
+      && artifact?.expired === false && artifact?.workflow_run?.head_sha === merge.head
+      && Number.isSafeInteger(artifact?.workflow_run?.id) && artifact.workflow_run.id > 0);
+    for (const runId of new Set(candidates.map(artifact => artifact.workflow_run.id))) {
+      const run = await get(`/actions/runs/${runId}`);
+      if (run?.id === runId && run?.event === 'pull_request' && run?.status === 'completed' && run?.conclusion === 'success'
+          && run?.path === workflowPath && run?.head_sha === merge.head
+          && run?.repository?.id === id && run?.head_repository?.id === id) {
+        return {tested: true, runId, reason: `pull request run ${runId} already passed this exact merge (${name})`};
+      }
+    }
+    return no(`no successful same-repository pull request run recorded ${name}`);
+  } catch {
+    // Network errors may echo request details; the decision needs none.
+    return no('workflow runs could not be read');
+  }
+}
+
+if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
+  const [mode, key, workflowPath] = process.argv.slice(2);
+  const env = process.env;
+  if (mode === 'record') {
+    // Never fails the suite: without a marker the queue simply runs it again.
+    try {
+      const name = recordName({key, prHeadSha: env.PR_HEAD_SHA, merge: localMerge()});
+      console.error(`A passing run records ${name}`);
+      console.log(`marker=${name}`);
+    } catch (error) {
+      console.error(`No tested-merge marker: ${error.message}`);
+      console.log('marker=');
+    }
+  } else if (mode === 'check') {
+    let decision;
+    try {
+      decision = await testedRun({
+        key, workflowPath, merge: localMerge(), mergeGroupHeadSha: env.MERGE_GROUP_HEAD_SHA,
+        repository: env.GITHUB_REPOSITORY, repositoryId: env.GITHUB_REPOSITORY_ID, token: env.RUNS_TOKEN,
+        apiUrl: env.GITHUB_API_URL ?? 'https://api.github.com',
+      });
+    } catch (error) {
+      decision = {tested: false, reason: error.message};
+    }
+    console.error(decision.tested ? `Skipping: ${decision.reason}` : `Running the suite: ${decision.reason}`);
+    console.log(`tested=${decision.tested}`);
+  } else {
+    console.error('Usage: tested-merge.mjs record|check <key> <workflow path>');
+    process.exitCode = 2;
+  }
+}

scripts/ci/tested-merge.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+# Prints the tested-merge decision for $GITHUB_OUTPUT, from the repository
+# root. The rules live in tested-merge.mjs.
+set -euo pipefail
+exec node scripts/ci/tested-merge.mjs "$@"

scripts/conformance/server-parity/aggregate.mjs

@@ -3,7 +3,8 @@
 // context as JSON.
 //
 // Passes only when every fixture job passed, or when the classifier reported
-// that no fixture input changed and every fixture job was skipped for that
+// that no fixture input changed, or that a pull request run already passed
+// this exact merge (tested=true), and every fixture job was skipped for that
 // reason. A push to main the merge queue already qualified (full=false)
 // passes when exactly the cache-saving jobs passed and every other job was
 // skipped. A failed, cancelled or unexpectedly skipped job fails the result,
@@ -60,7 +61,10 @@ export function decide(needs) {
   }
   return {
     pass: true,
-    reason: run === 'true' ? `Every fixture job passed: ${summary}.` : `No fixture input changed; every fixture job was skipped: ${summary}.`,
+    reason: run === 'true' ? `Every fixture job passed: ${summary}.`
+      : changes.outputs?.tested === 'true'
+        ? `A pull request run already passed this exact merge; every fixture job was skipped: ${summary}.`
+        : `No fixture input changed; every fixture job was skipped: ${summary}.`,
   };
 }
 

tests/Unit/PhpunitFeatureWorkflowContractTest.php

@@ -211,10 +211,14 @@ public function test_final_gate_requires_preflight_and_feature_suite(): void
         foreach ([
             'qualification:',
             'name: Feature source qualification',
-            'needs: [preflight, feature]',
+            'needs: [preflight, tested-merge, feature]',
             'if: ${{ always() }}',
             'test "$PREFLIGHT_RESULT" = success',
             'test "$FEATURE_RESULT" = success',
+            // Only a merge group a pull request run already passed exactly
+            // (tests/Unit/TestedMergeTest.mjs) may skip the feature suite.
+            'if [ "$EVENT_NAME" = merge_group ] && [ "$TESTED_MERGE" = true ] && [ "$FEATURE_RESULT" = skipped ]; then',
+            "if: \${{ always() && !cancelled() && needs.tested-merge.outputs.tested != 'true' }}",
         ] as $needle) {
             $this->assertStringContainsString($needle, $this->workflow);
         }

tests/Unit/ServerParityAggregateTest.mjs

@@ -166,7 +166,10 @@ test('the fixture jobs run only through the classifier and the result waits for
   const ids = Object.keys(blocks);
   assert.equal(ids[0], 'changes');
   assert.match(blocks.changes, /^ {4}timeout-minutes: \d+$/m);
-  assert.match(blocks.changes, /run: \$\{\{ steps\.classify\.outputs\.run \}\}/);
+  // A merge group a pull request run already passed exactly skips the fixture
+  // jobs; TestedMergeTest.mjs covers that decision.
+  assert.ok(blocks.changes.split('\n').includes(
+    "      run: ${{ steps.tested.outputs.tested == 'true' && 'false' || steps.classify.outputs.run }}"));
   assert.match(blocks.changes, /run: bash scripts\/conformance\/server-parity\/changes\.sh >> "\$GITHUB_OUTPUT"$/m);
   assert.match(blocks.changes, /full: \$\{\{ steps\.classify\.outputs\.full \}\}/);
   // The root fixture jobs run only through the classifier. Only the cache
@@ -235,6 +238,22 @@ const fixture = (result, run) => ({
   'execution-and-codec': {result, outputs: {}},
 });
 
+test('the result names an exact tested merge as the reason every fixture job was skipped', () => {
+  const skipped = fixture('skipped', 'false');
+  skipped.changes.outputs.tested = 'true';
+  assert.match(decide(skipped).reason, /^A pull request run already passed this exact merge; every fixture job was skipped/);
+  assert.equal(decide(skipped).pass, true);
+  // Being tested never excuses a fixture job that ran and failed or one that
+  // was skipped while the classifier asked for it.
+  const ran = fixture('skipped', 'true');
+  ran.changes.outputs.tested = 'true';
+  assert.equal(decide(ran).pass, false);
+  const failed = fixture('skipped', 'false');
+  failed.changes.outputs.tested = 'true';
+  failed['mysql-execution'] = {result: 'failure', outputs: {}};
+  assert.equal(decide(failed).pass, false);
+});
+
 test('the result passes when every fixture job passed', () => {
   const outcome = runAggregate(fixture('success', 'true'));
   assert.equal(outcome.status, 0, outcome.stderr);