From a164f42b8aa19cd6fc7ee75c7ab1e7aadf517a32 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 22 Oct 2026 01:39:19 -0700 Subject: [PATCH] fix(ci): exclude inherited job results from runner demand (#31919) --- config/scripts/ci-job-executions.mjs | 24 ++++++++++++++ config/scripts/ci-runner-demand.mjs | 2 +- config/scripts/ci-runner-metrics.mjs | 7 ++-- config/scripts/ci-runner-metrics.test.mjs | 40 +++++++++++++++++++++++ 4 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 config/scripts/ci-job-executions.mjs diff --git a/config/scripts/ci-job-executions.mjs b/config/scripts/ci-job-executions.mjs new file mode 100644 index 00000000000..e2575312e00 --- /dev/null +++ b/config/scripts/ci-job-executions.mjs @@ -0,0 +1,24 @@ +const executionKey = (job) => + job.name && + job.runner_name && + job.started_at && + job.completed_at && + Number.isSafeInteger(job.run_attempt) && + job.run_attempt > 0 + ? JSON.stringify([job.name, job.runner_name, job.started_at, job.completed_at]) + : null + +export function jobExecutions(jobs) { + const firstAttempt = new Map() + // Failed-only retries copy completed jobs with new IDs and the original execution timestamps. + for (const job of jobs) { + const key = executionKey(job) + if (key !== null) { + firstAttempt.set(key, Math.min(firstAttempt.get(key) ?? Infinity, job.run_attempt)) + } + } + return jobs.filter((job) => { + const key = executionKey(job) + return key === null || firstAttempt.get(key) === job.run_attempt + }) +} diff --git a/config/scripts/ci-runner-demand.mjs b/config/scripts/ci-runner-demand.mjs index 6f1f39ad0b7..0a60d1f40f6 100644 --- a/config/scripts/ci-runner-demand.mjs +++ b/config/scripts/ci-runner-demand.mjs @@ -78,7 +78,7 @@ export function demandMarkdown(report) { `| ${row.workflow} | ${row.runs} | ${(row.runnerMinutes / 60).toFixed(1)} | ${(row.cancelledRunnerMinutes / 60).toFixed(1)} | ${(row.repeatedSuccessfulRunnerMinutes / 60).toFixed(1)} | ${row.runnerMinutesPerCompletedPrRun?.toFixed(1) ?? '—'} |` ), '', - 'Repeated green-job hours count assigned, completed jobs with a successful job of the same name in an earlier attempt of that run. This is a review bucket, not guaranteed avoidable work; it overlaps total and cancelled-run hours.', + 'Completed records with the same job name, runner and execution timestamps across attempts are counted once: GitHub carries successful results forward without running them again. Repeated green-job hours count fresh executions after a successful job of the same name in an earlier attempt. This review bucket is not guaranteed avoidable work; it overlaps total and cancelled-run hours.', '', '| Runner labels | Hours | Queue/provisioning p95 minutes |', '| --- | ---: | ---: |', diff --git a/config/scripts/ci-runner-metrics.mjs b/config/scripts/ci-runner-metrics.mjs index 2e7a580eb0b..917711c6d7d 100644 --- a/config/scripts/ci-runner-metrics.mjs +++ b/config/scripts/ci-runner-metrics.mjs @@ -1,3 +1,5 @@ +import { jobExecutions } from './ci-job-executions.mjs' + const minutesBetween = (start, end) => { const value = (Date.parse(end) - Date.parse(start)) / 60_000 return Number.isFinite(value) ? Math.max(0, value) : 0 @@ -42,8 +44,9 @@ export function runnerDemand(runs, samples) { for (const { run, jobs, weight } of samples) { const row = workflows.get(workflowKey(run)) row.sampledRuns++ + const executions = jobExecutions(jobs) const firstSuccess = new Map() - for (const job of jobs) { + for (const job of executions) { if (job.name && Number.isInteger(job.run_attempt) && job.conclusion === 'success') { firstSuccess.set( job.name, @@ -51,7 +54,7 @@ export function runnerDemand(runs, samples) { ) } } - for (const job of jobs) { + for (const job of executions) { if (!job.runner_name || !job.started_at) { continue } diff --git a/config/scripts/ci-runner-metrics.test.mjs b/config/scripts/ci-runner-metrics.test.mjs index 3f9a766f483..6c86d090129 100644 --- a/config/scripts/ci-runner-metrics.test.mjs +++ b/config/scripts/ci-runner-metrics.test.mjs @@ -79,6 +79,10 @@ it('paginates jobs and bounds run discovery to 24 complete hours', async () => { it('counts only later attempts of previously successful jobs, including failed repeats', () => { const observation = (name, run_attempt, conclusion, extra = {}) => ({ ...job, + started_at: new Date(Date.parse(job.started_at) + (run_attempt ?? 0) * 600_000).toISOString(), + completed_at: new Date( + Date.parse(job.completed_at) + (run_attempt ?? 0) * 600_000 + ).toISOString(), name, run_attempt, conclusion, @@ -115,6 +119,42 @@ it('counts only later attempts of previously successful jobs, including failed r expect(report.incompleteJobs).toBe(1) }) +it('counts carried-forward results once while retaining fresh executions on the same runner', () => { + const first = { ...job, id: 1, name: 'unit', run_attempt: 1, conclusion: 'success' } + const carried = { ...first, id: 2, run_attempt: 2 } + const fresh = { + ...first, + id: 3, + run_attempt: 3, + started_at: '2026-09-27T00:23:00Z', + completed_at: '2026-09-27T00:28:00Z' + } + const failed = { ...first, id: 5, name: 'build', conclusion: 'failure' } + const report = runnerDemand( + [run], + [ + { + run, + weight: 2, + jobs: [ + { ...fresh, id: 4, run_attempt: 4 }, + carried, + first, + fresh, + failed, + { ...failed, id: 6, run_attempt: 2 } + ] + } + ] + ) + expect(report.workflows[0]).toMatchObject({ + runnerMinutes: 30, + repeatedSuccessfulJobs: 2, + repeatedSuccessfulRunnerMinutes: 10 + }) + expect(report.pools[0].runnerMinutes).toBe(30) +}) + it('refuses to publish a silently truncated inventory', async () => { await expect( collectRunnerDemand({}, async () => ({ total_count: 1001, workflow_runs: [] }))