diff --git a/.github/workflows/ci-closed-pr-caches.yml b/.github/workflows/ci-closed-pr-caches.yml index 38cf5803a14..468559ccd4d 100644 --- a/.github/workflows/ci-closed-pr-caches.yml +++ b/.github/workflows/ci-closed-pr-caches.yml @@ -1,4 +1,4 @@ -name: Clean closed PR caches +name: Clean closed PR work on: pull_request_target: @@ -6,14 +6,81 @@ on: permissions: actions: write + pull-requests: read jobs: clean: runs-on: ubuntu-latest timeout-minutes: 5 steps: + - name: Cancel checks for an unmerged closed PR + if: github.event.pull_request.merged == false + uses: actions/github-script@v8 + with: + script: | + const closed = context.payload.pull_request + const targets = [ + ['pr-checks', 'pr.yml'], + ['node-server', 'node-server-tests.yml'], + ['ssh-windows-hosts', 'ssh-windows-hosts.yml'], + ['ssh-hostile-hosts', 'ssh-hostile-hosts.yml'], + ['mobile', 'mobile.yml'], + ['computer-e2e', 'computer-e2e.yml'] + ] + const stillClosed = async () => { + const { data: current } = await github.rest.pulls.get({ + ...context.repo, pull_number: closed.number + }) + return current.state === 'closed' && current.merged_at === null && + current.closed_at === closed.closed_at + } + if (!Number.isSafeInteger(closed.number) || closed.number <= 0 || + !Number.isFinite(Date.parse(closed.closed_at))) { + throw new Error('Missing closed PR identity') + } + if (!await stillClosed()) return + for (const [prefix, workflow] of targets) { + const group = `${prefix}-${closed.number}` + let data + try { + const response = await github.request( + 'GET /repos/{owner}/{repo}/actions/concurrency_groups/{concurrency_group_name}', { + ...context.repo, concurrency_group_name: group, + headers: { 'X-GitHub-Api-Version': '2026-03-10' } + } + ) + data = response.data + } catch (error) { + if (error.status === 404) continue + throw error + } + if (data.group_name !== group || !Array.isArray(data.group_members)) { + throw new Error(`Unexpected concurrency group: ${group}`) + } + for (const member of data.group_members) { + // Only whole PR runs; release/manual jobs never share this identity. + if (member.job_id !== undefined || !Number.isSafeInteger(member.run_id)) continue + const { data: run } = await github.rest.actions.getWorkflowRun({ + ...context.repo, run_id: member.run_id + }) + if (run.event !== 'pull_request' || run.path !== `.github/workflows/${workflow}` || + run.status === 'completed' || + !Number.isFinite(Date.parse(run.created_at)) || + Date.parse(run.created_at) > Date.parse(closed.closed_at)) continue + if (!await stillClosed()) return + try { + await github.rest.actions.cancelWorkflowRun({ + ...context.repo, run_id: member.run_id + }) + core.info(`Requested cancellation of ${group}: ${member.run_id}`) + } catch (error) { + if (error.status !== 409) throw error + } + } + } # No checkout: this runs trusted default-branch code, including for fork PRs. - uses: actions/github-script@v8 + if: '!cancelled()' with: script: | const ref = `refs/pull/${context.payload.pull_request.number}/merge` diff --git a/config/scripts/ci-closed-pr-caches.test.mjs b/config/scripts/ci-closed-pr-caches.test.mjs index 44edb5e48df..dbab17e889a 100644 --- a/config/scripts/ci-closed-pr-caches.test.mjs +++ b/config/scripts/ci-closed-pr-caches.test.mjs @@ -4,7 +4,7 @@ import { expect, it, vi } from 'vitest' import { parse } from 'yaml' const workflow = parse(readFileSync('.github/workflows/ci-closed-pr-caches.yml', 'utf8')) -const script = workflow.jobs.clean.steps[0].with.script +const script = workflow.jobs.clean.steps[1].with.script const ref = 'refs/pull/123/merge' function run(caches, remove = vi.fn().mockResolvedValue(undefined)) { @@ -23,9 +23,11 @@ function run(caches, remove = vi.fn().mockResolvedValue(undefined)) { it('uses default-branch code without checking out a closed PR', () => { expect(workflow.on).toEqual({ pull_request_target: { types: ['closed'] } }) - expect(workflow.permissions).toEqual({ actions: 'write' }) - expect(workflow.jobs.clean.steps).toHaveLength(1) - expect(workflow.jobs.clean.steps[0].uses).toBe('actions/github-script@v8') + expect(workflow.permissions).toEqual({ actions: 'write', 'pull-requests': 'read' }) + expect(workflow.jobs.clean.steps).toHaveLength(2) + expect(workflow.jobs.clean.steps.every((step) => step.uses === 'actions/github-script@v8')).toBe( + true + ) }) it('lists and deletes only caches scoped to the closed merge ref', async () => { diff --git a/config/scripts/ci-closed-pr-cancellation.test.mjs b/config/scripts/ci-closed-pr-cancellation.test.mjs new file mode 100644 index 00000000000..13f933932ca --- /dev/null +++ b/config/scripts/ci-closed-pr-cancellation.test.mjs @@ -0,0 +1,152 @@ +import { readFileSync } from 'node:fs' +import { runInNewContext } from 'node:vm' +import { expect, it, vi } from 'vitest' +import { parse } from 'yaml' + +const workflow = parse(readFileSync('.github/workflows/ci-closed-pr-caches.yml', 'utf8')) +const step = workflow.jobs.clean.steps[0] +const closed = { number: 123, closed_at: '2026-10-06T00:00:00Z' } +const current = { ...closed, state: 'closed', merged_at: null } +const run = { + id: 42, + event: 'pull_request', + path: '.github/workflows/pr.yml', + status: 'in_progress', + created_at: '2026-10-05T23:59:00Z' +} + +function execute(options = {}) { + const request = vi.fn(async (_route, args) => { + if (options.requestError) { + throw options.requestError + } + if (args.concurrency_group_name !== 'pr-checks-123') { + throw { status: 404 } + } + return { + data: { + group_name: options.group ?? 'pr-checks-123', + group_members: options.members ?? [{ run_id: 42 }] + } + } + }) + const getPr = vi.fn(async () => ({ + data: options.current ?? current + })) + if (options.reopened) { + getPr.mockResolvedValueOnce({ data: current }) + } + const getRun = vi.fn(async () => ({ data: { ...run, ...options.run } })) + const cancel = vi.fn(async () => { + if (options.cancelError) { + throw options.cancelError + } + }) + const result = runInNewContext(`(async () => { ${step.with.script} })()`, { + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { pull_request: options.closed ?? closed } + }, + github: { + request, + rest: { + pulls: { get: getPr }, + actions: { getWorkflowRun: getRun, cancelWorkflowRun: cancel } + } + }, + core: { info: vi.fn() } + }) + return { result, request, getPr, getRun, cancel } +} + +it('uses trusted inline code and only enters cancellation on an unmerged close', () => { + expect(workflow.on).toEqual({ pull_request_target: { types: ['closed'] } }) + expect(step.if).toBe('github.event.pull_request.merged == false') + expect(step.uses).toBe('actions/github-script@v8') + expect( + workflow.jobs.clean.steps.some((entry) => entry.uses?.startsWith('actions/checkout')) + ).toBe(false) +}) + +it('looks up exact PR groups without branch or head-SHA inference', async () => { + const { result, request, cancel } = execute() + await result + expect(request.mock.calls.map(([, args]) => args.concurrency_group_name)).toEqual([ + 'pr-checks-123', + 'node-server-123', + 'ssh-windows-hosts-123', + 'ssh-hostile-hosts-123', + 'mobile-123', + 'computer-e2e-123' + ]) + expect( + request.mock.calls.every( + ([route, args]) => + route === 'GET /repos/{owner}/{repo}/actions/concurrency_groups/{concurrency_group_name}' && + args.headers['X-GitHub-Api-Version'] === '2026-03-10' + ) + ).toBe(true) + expect(cancel.mock.calls).toEqual([[{ owner: 'owner', repo: 'repo', run_id: 42 }]]) +}) + +it.each([ + { event: 'push' }, + { event: 'workflow_dispatch' }, + { path: '.github/workflows/release-cut.yml' }, + { status: 'completed' }, + { created_at: '2026-10-06T00:00:01Z' }, + { created_at: 'invalid' } +])('retains unrelated, completed and post-close runs: %j', async (otherRun) => { + const { result, cancel } = execute({ run: otherRun }) + await result + expect(cancel).not.toHaveBeenCalled() +}) + +it.each([ + { ...current, state: 'open' }, + { ...current, merged_at: closed.closed_at }, + { ...current, closed_at: '2026-10-06T00:01:00Z' } +])('retains checks if the closure is no longer current: %j', async (pr) => { + const { result, request, cancel } = execute({ current: pr }) + await result + expect(request).not.toHaveBeenCalled() + expect(cancel).not.toHaveBeenCalled() +}) + +it('rechecks closure before cancelling when a PR reopens during lookup', async () => { + const { result, getRun, cancel } = execute({ + current: { ...current, state: 'open' }, + reopened: true + }) + await result + expect(getRun).toHaveBeenCalledOnce() + expect(cancel).not.toHaveBeenCalled() +}) + +it('ignores job-level leases and invalid run identities', async () => { + const { result, getRun, cancel } = execute({ + members: [{ run_id: 42, job_id: 1 }, { run_id: '42' }] + }) + await result + expect(getRun).not.toHaveBeenCalled() + expect(cancel).not.toHaveBeenCalled() +}) + +it('refuses a mismatched concurrency group', async () => { + const { result, cancel } = execute({ group: 'pr-checks-124' }) + await expect(result).rejects.toThrow('Unexpected concurrency group') + expect(cancel).not.toHaveBeenCalled() +}) + +it('surfaces lookup and cancellation permission failures', async () => { + await expect(execute({ requestError: { status: 403 } }).result).rejects.toEqual({ status: 403 }) + await expect(execute({ cancelError: { status: 403 } }).result).rejects.toEqual({ status: 403 }) + await expect(execute({ cancelError: { status: 409 } }).result).resolves.toBeUndefined() +}) + +it('validates the event identity before looking up any work', async () => { + const { result, request, getPr } = execute({ closed: { ...closed, number: '123' } }) + await expect(result).rejects.toThrow('Missing closed PR identity') + expect(request).not.toHaveBeenCalled() + expect(getPr).not.toHaveBeenCalled() +})