From d1d29781756a570234063c6432939e4e145864e2 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sun, 13 Sep 2026 02:11:38 +0800 Subject: [PATCH 1/2] fix(ci): revert skipped PR edit checks Run the required test job for every eligible CI event. A newer metadata-only skipped run can hide an earlier successful check and leave the merge gate waiting for test. Keep edited events for base retargets and isolate metadata runs so they cannot cancel existing validation. Title and body edits now run real CI. Generated-by: Codex --- .asf.yaml | 9 +++++---- .github/workflows/ci.yml | 10 +++------- scripts/ci-workflow-policy.test.mjs | 18 +++++------------- 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/.asf.yaml b/.asf.yaml index fa87ccae0a..47618615e0 100644 --- a/.asf.yaml +++ b/.asf.yaml @@ -63,10 +63,11 @@ github: strict: false # test is the single required-check authority in # .github/workflows/ci.yml. It runs for every new or updated pull request - # targeting main, including a pull request retargeted there. Title- and - # body-only edits create a differently named skipped job, so they neither - # cancel nor satisfy this context. The job runs install-free contract - # checks before installing the toolchain only for the validation its own + # targeting main, including a pull request retargeted there. Title and + # body edits also run real validation in an isolated concurrency group; + # a newer skipped run can hide an earlier successful required check. + # The job runs install-free contract checks before installing the + # toolchain only for the validation its own # planning step selects, so a documentation-only change still reports # without paying for a build. Renaming the required job, adding a paths # filter that stops ci.yml from running, or splitting this authority back diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ad70db3bd..697e8a43af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,9 +26,9 @@ on: workflow_dispatch: concurrency: - # `edited` also covers title and body changes. Isolate those no-op runs so - # they cannot cancel the check for the current pull request revision. - group: ci-${{ github.workflow }}-${{ github.ref }}${{ github.event.action == 'edited' && github.event.changes.base.ref.from == '' && format('-ignored-{0}', github.run_id) || '' }} + # Title and body edits also run real validation: a newer skipped run can + # hide a successful required check. Keep edits from cancelling existing tests. + group: ci-${{ github.workflow }}-${{ github.ref }}${{ github.event.action == 'edited' && github.event.changes.base.ref.from == '' && format('-metadata-{0}', github.run_id) || '' }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} permissions: @@ -45,10 +45,6 @@ jobs: # Renaming it would leave that check unreported on every open pull request # until the rename merged, and nothing could merge while it was unreported. test: - # A base-ref edit keeps the protected `CI / test` name. Other edits create - # only a differently named skipped check, so they cannot satisfy it. - name: ${{ github.event_name == 'pull_request' && github.event.action == 'edited' && github.event.changes.base.ref.from == '' && 'ignored-edit' || 'test' }} - if: ${{ github.event_name != 'pull_request' || github.event.action != 'edited' || github.event.changes.base.ref.from != '' }} # Pinned, not `ubuntu-latest`. The two resolve to the same image, but only # the alias makes this required context wait at the tail, and the steps # below already assume this image. `ci-workflow-policy.test.mjs` holds the diff --git a/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index 6e4dae8147..d8399f3b8b 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -56,19 +56,19 @@ test('GitHub output matches the selections consumed by CI', () => { assert.deepEqual(outputKeys, consumedKeys); }); -test('one job remains the only required-check authority', () => { +test('one unconditional job carries the required context on every CI run', () => { const workflow = readWorkflow('ci.yml'); // `.asf.yaml` requires `test`. A paths filter would stop the workflow and // leave that check pending forever, and a second job would create another - // authority. Metadata-only edits may skip this job under a different name; - // the retarget contract below proves that exception cannot impersonate it. + // authority. Every triggered run must actually validate the required context. assert.doesNotMatch(triggerBlock('ci.yml'), /\bpaths(-ignore)?:/u); const jobsBlock = workflow.slice(workflow.indexOf('\njobs:')); const jobs = [...jobsBlock.matchAll(/^ {2}([a-z0-9_-]+):$/gmu)].map((match) => match[1]); assert.deepEqual(jobs, ['test']); assert.doesNotMatch(jobsBlock, /^ {4}needs:/mu); + assert.doesNotMatch(jobsBlock, /^ {4}(?:if|name):/mu); }); test('comparison precedes planning and every later gate uses plan outputs', () => { @@ -116,23 +116,15 @@ test('every core diff gate consumes the shared comparison without resolving anot assert.match(workflow, /HEAD_SHA: \$\{\{ steps\.comparison\.outputs\.head \}\}/u); }); -test('core CI runs on base retargets without letting metadata edits replace the required check', () => { +test('core CI includes retargets and isolates metadata runs from existing tests', () => { const workflow = readWorkflow('ci.yml'); assert.match(workflow, /types: \[opened, synchronize, reopened, edited\]/u); assert.match( workflow, - /group: ci-\$\{\{ github\.workflow \}\}-\$\{\{ github\.ref \}\}\$\{\{ github\.event\.action == 'edited' && github\.event\.changes\.base\.ref\.from == '' && format\('-ignored-\{0\}', github\.run_id\) \|\| '' \}\}/u, + /group: ci-\$\{\{ github\.workflow \}\}-\$\{\{ github\.ref \}\}\$\{\{ github\.event\.action == 'edited' && github\.event\.changes\.base\.ref\.from == '' && format\('-metadata-\{0\}', github\.run_id\) \|\| '' \}\}/u, ); assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u); - assert.match( - workflow, - /name: \$\{\{ github\.event_name == 'pull_request' && github\.event\.action == 'edited' && github\.event\.changes\.base\.ref\.from == '' && 'ignored-edit' \|\| 'test' \}\}/u, - ); - assert.match( - workflow, - /if: \$\{\{ github\.event_name != 'pull_request' \|\| github\.event\.action != 'edited' \|\| github\.event\.changes\.base\.ref\.from != '' \}\}/u, - ); }); test('core CI uses the Windows inventory package-script authority', () => { From 785f196fa07f16370eb86b8f2083960f80610a07 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sun, 13 Sep 2026 02:13:44 +0800 Subject: [PATCH 2/2] revert(ci): stop triggering validation on PR edits Complete the revert of #4186, including the edited trigger and metadata concurrency routing. Restore the default opened, synchronize, and reopened pull_request activities. Retargeting without a head update no longer starts CI automatically; a new commit or reopening the PR is needed to validate that merge base. Generated-by: Codex --- .asf.yaml | 19 ++++++++----------- .github/workflows/ci.yml | 5 +---- scripts/ci-workflow-policy.test.mjs | 19 ++++--------------- 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/.asf.yaml b/.asf.yaml index 47618615e0..bd40ce071f 100644 --- a/.asf.yaml +++ b/.asf.yaml @@ -61,17 +61,14 @@ github: required_approving_review_count: 1 required_status_checks: strict: false - # test is the single required-check authority in - # .github/workflows/ci.yml. It runs for every new or updated pull request - # targeting main, including a pull request retargeted there. Title and - # body edits also run real validation in an isolated concurrency group; - # a newer skipped run can hide an earlier successful required check. - # The job runs install-free contract checks before installing the - # toolchain only for the validation its own - # planning step selects, so a documentation-only change still reports - # without paying for a build. Renaming the required job, adding a paths - # filter that stops ci.yml from running, or splitting this authority back - # across jobs can leave the check unreported and freeze pull requests. + # test is the single unconditional job in .github/workflows/ci.yml. It + # runs the install-free contract checks on every change and installs the + # toolchain only for the validation its own planning step selects, so a + # documentation-only change still reports without paying for a build. + # Renaming the job there, adding a paths filter that stops ci.yml from + # running, or splitting the work back across jobs so this context comes + # from an aggregator that can be skipped, freezes every pull request: + # the check never reports and no committer can override it. # A required context must report on every pull request, so a lane # behind a paths filter cannot be listed here: the filter would keep # the workflow from starting and the check would stay pending forever. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 697e8a43af..83437e3117 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,15 +20,12 @@ name: CI on: pull_request: branches: [main] - types: [opened, synchronize, reopened, edited] push: branches: [main] workflow_dispatch: concurrency: - # Title and body edits also run real validation: a newer skipped run can - # hide a successful required check. Keep edits from cancelling existing tests. - group: ci-${{ github.workflow }}-${{ github.ref }}${{ github.event.action == 'edited' && github.event.changes.base.ref.from == '' && format('-metadata-{0}', github.run_id) || '' }} + group: ci-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} permissions: diff --git a/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index d8399f3b8b..6b1826c058 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -56,19 +56,19 @@ test('GitHub output matches the selections consumed by CI', () => { assert.deepEqual(outputKeys, consumedKeys); }); -test('one unconditional job carries the required context on every CI run', () => { +test('one unconditional job carries the required context on every pull request', () => { const workflow = readWorkflow('ci.yml'); // `.asf.yaml` requires `test`. A paths filter would stop the workflow and - // leave that check pending forever, and a second job would create another - // authority. Every triggered run must actually validate the required context. + // leave that check pending forever, and a second job would make the same + // pull request queue for a scarce runner twice to reach one verdict. assert.doesNotMatch(triggerBlock('ci.yml'), /\bpaths(-ignore)?:/u); const jobsBlock = workflow.slice(workflow.indexOf('\njobs:')); const jobs = [...jobsBlock.matchAll(/^ {2}([a-z0-9_-]+):$/gmu)].map((match) => match[1]); assert.deepEqual(jobs, ['test']); assert.doesNotMatch(jobsBlock, /^ {4}needs:/mu); - assert.doesNotMatch(jobsBlock, /^ {4}(?:if|name):/mu); + assert.doesNotMatch(jobsBlock, /^ {4}if:/mu); }); test('comparison precedes planning and every later gate uses plan outputs', () => { @@ -116,17 +116,6 @@ test('every core diff gate consumes the shared comparison without resolving anot assert.match(workflow, /HEAD_SHA: \$\{\{ steps\.comparison\.outputs\.head \}\}/u); }); -test('core CI includes retargets and isolates metadata runs from existing tests', () => { - const workflow = readWorkflow('ci.yml'); - - assert.match(workflow, /types: \[opened, synchronize, reopened, edited\]/u); - assert.match( - workflow, - /group: ci-\$\{\{ github\.workflow \}\}-\$\{\{ github\.ref \}\}\$\{\{ github\.event\.action == 'edited' && github\.event\.changes\.base\.ref\.from == '' && format\('-metadata-\{0\}', github\.run_id\) \|\| '' \}\}/u, - ); - assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u); -}); - test('core CI uses the Windows inventory package-script authority', () => { const workflow = readWorkflow('ci.yml');