From 7c1ce0bfc4d26016fbfff558d21da15a333c15ec Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 22 Jul 2026 16:10:09 -0700 Subject: [PATCH 01/20] fix(ci): recognize terminalized runner loss Signed-off-by: Apurv Kumaria (cherry picked from commit d137bfef9e0cb551bb5c1d2ec00a866330d1360c) --- test/pr-e2e-gate-lifecycle.test.ts | 79 ++++++++++++++++++++++++++++++ tools/e2e/pr-e2e-gate.mts | 32 +++++++++--- 2 files changed, 105 insertions(+), 6 deletions(-) diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index 906c37387d9..f0728ecbc6d 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -680,6 +680,85 @@ describe("PR E2E controller lifecycle", () => { expectedSummary: "confirmed hosted-runner loss on attempt 1", expectedRetryReason: "child-cancelled", }, + { + label: "a GitHub-hosted runner shutdown terminalizes the live step and skips cleanup", + status: "completed", + conclusion: "failure", + jobs: [ + { + id: 77, + name: "Hermes security-posture", + status: "completed", + conclusion: "failure", + runner_id: 1_021_276_374, + runner_name: "GitHub Actions 1021276374", + labels: ["ubuntu-latest"], + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { + name: "Run security posture live Vitest test", + status: "completed", + conclusion: "cancelled", + }, + { + name: "Upload security posture artifacts", + status: "completed", + conclusion: "skipped", + }, + { + name: "Clean up Docker auth", + status: "completed", + conclusion: "skipped", + }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }, + ], + evidenceOutcome: "success" as const, + assertFinalization: expectHandledFinalization, + assertCompletionLink: expectSelectedRunLink, + expectCancellation: false, + expectedTitle: "Hermes security-posture failed", + expectedSummary: "confirmed hosted-runner loss on attempt 1", + expectedRetryReason: "child-cancelled", + }, + { + label: "a cancelled step followed by successful cleanup is not runner loss", + status: "completed", + conclusion: "failure", + jobs: [ + { + id: 77, + name: "Hermes security-posture", + status: "completed", + conclusion: "failure", + runner_id: 1_021_276_374, + runner_name: "GitHub Actions 1021276374", + labels: ["ubuntu-latest"], + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { + name: "Run security posture live Vitest test", + status: "completed", + conclusion: "cancelled", + }, + { + name: "Upload security posture artifacts", + status: "completed", + conclusion: "success", + }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }, + ], + evidenceOutcome: "success" as const, + assertFinalization: expectHandledFinalization, + assertCompletionLink: expectSelectedRunLink, + expectCancellation: false, + expectedTitle: "Hermes security-posture failed", + expectedSummary: "concluded `failure`", + expectedRetryReason: undefined, + }, { label: "runner-loss metadata coexists with an ordinary failed child", status: "completed", diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 4ed8726c548..ed09d075498 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -1774,11 +1774,12 @@ function ciFailureReport(options: { const GITHUB_HOSTED_RUNNER_NAME_PATTERN = /^GitHub Actions [1-9][0-9]*$/u; /** - * GitHub records a lost hosted runner as a completed failed job whose active - * step never received a terminal conclusion. This is stronger than a - * cancellation: user and concurrency cancellations finish the active step and - * run cleanup, while the release-run failures tracked by #7146 retained one - * `in_progress` step after the job itself became terminal. + * GitHub records a lost hosted runner as a completed failed job with no + * ordinary failed step. Older Jobs API responses left the interrupted step + * `in_progress`; current responses can terminalize it as `cancelled`, skip the + * remaining cleanup, and append the synthetic successful `Complete job` step. + * A user or concurrency cancellation concludes the job itself as `cancelled`, + * while an ordinary assertion records a failed step, so neither shape matches. */ function hasTrustedHostedRunnerLossMarker(job: WorkflowJob): boolean { if ( @@ -1796,12 +1797,31 @@ function hasTrustedHostedRunnerLossMarker(job: WorkflowJob): boolean { const strandedSteps = job.steps.filter( (step) => step.status === "in_progress" && step.conclusion === null, ); - return ( + const legacyStrandedStep = strandedSteps.length === 1 && job.steps.every( (step) => step.conclusion === "success" || (step.conclusion === null && ["in_progress", "pending"].includes(step.status ?? "")), + ); + if (legacyStrandedStep) return true; + + const cancelledStepIndexes = job.steps.flatMap((step, index) => + step.status === "completed" && step.conclusion === "cancelled" ? [index] : [], + ); + if (cancelledStepIndexes.length !== 1) return false; + const cancelledIndex = cancelledStepIndexes[0]!; + const beforeCancellation = job.steps.slice(0, cancelledIndex); + const afterCancellation = job.steps.slice(cancelledIndex + 1); + return ( + beforeCancellation.every((step) => ["success", "skipped"].includes(step.conclusion ?? "")) && + afterCancellation.some((step) => step.conclusion === "skipped") && + afterCancellation.every( + (step) => + step.conclusion === "skipped" || + (step.name === "Complete job" && + step.status === "completed" && + step.conclusion === "success"), ) ); } From 73986222befa33db67df36fc409cea6c1f92cd53 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 22 Jul 2026 16:35:52 -0700 Subject: [PATCH 02/20] fix(ci): tighten hosted runner-loss evidence Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-lifecycle.test.ts | 10 + ...pr-e2e-gate-runner-loss-classifier.test.ts | 243 ++++++++++++++++++ tools/e2e/pr-e2e-gate.mts | 68 +++-- 3 files changed, 302 insertions(+), 19 deletions(-) create mode 100644 test/pr-e2e-gate-runner-loss-classifier.test.ts diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index f0728ecbc6d..3eef3eb2edf 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -656,6 +656,8 @@ describe("PR E2E controller lifecycle", () => { conclusion: "failure", runner_id: 1_020_705_058, runner_name: "GitHub Actions 1020705058", + runner_group_id: 0, + runner_group_name: "GitHub Actions", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -692,6 +694,8 @@ describe("PR E2E controller lifecycle", () => { conclusion: "failure", runner_id: 1_021_276_374, runner_name: "GitHub Actions 1021276374", + runner_group_id: 0, + runner_group_name: "GitHub Actions", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -734,6 +738,8 @@ describe("PR E2E controller lifecycle", () => { conclusion: "failure", runner_id: 1_021_276_374, runner_name: "GitHub Actions 1021276374", + runner_group_id: 0, + runner_group_name: "GitHub Actions", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -771,6 +777,8 @@ describe("PR E2E controller lifecycle", () => { conclusion: "failure", runner_id: 1_020_705_058, runner_name: "GitHub Actions 1020705058", + runner_group_id: 0, + runner_group_name: "GitHub Actions", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -788,6 +796,8 @@ describe("PR E2E controller lifecycle", () => { conclusion: "failure", runner_id: 1_020_705_059, runner_name: "GitHub Actions 1020705059", + runner_group_id: 0, + runner_group_name: "GitHub Actions", labels: ["ubuntu-latest"], steps: [ { diff --git a/test/pr-e2e-gate-runner-loss-classifier.test.ts b/test/pr-e2e-gate-runner-loss-classifier.test.ts new file mode 100644 index 00000000000..c2172f16d68 --- /dev/null +++ b/test/pr-e2e-gate-runner-loss-classifier.test.ts @@ -0,0 +1,243 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; +import { verifiedRunnerLossEvidence } from "../tools/e2e/pr-e2e-gate.mts"; +import { detectRunnerLoss } from "../tools/e2e/runner-pressure-core.mts"; + +function hostedRunnerLossJob(overrides: Record = {}) { + return { + id: 89_074_697_099, + name: "Hermes security-posture", + status: "completed", + conclusion: "failure", + runnerId: 1_021_277_393, + runnerName: "GitHub Actions 1021277393", + runnerGroupId: 0, + runnerGroupName: "GitHub Actions", + labels: ["ubuntu-latest"], + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { + name: "Run security posture live Vitest test", + status: "completed", + conclusion: "cancelled", + }, + { name: "Upload security posture artifacts", status: "completed", conclusion: "skipped" }, + { name: "Clean up Docker auth", status: "completed", conclusion: "skipped" }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + ...overrides, + }; +} + +function legacyHostedRunnerLossJob(id: number, runnerId: number) { + return hostedRunnerLossJob({ + id, + runnerId, + runnerName: `GitHub Actions ${runnerId}`, + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { name: "Run live test", status: "in_progress", conclusion: null }, + { name: "Upload artifacts", status: "pending", conclusion: null }, + ], + }); +} + +function confirmsRunnerLoss( + options: { + workflowConclusion?: string; + jobs?: ReturnType[]; + complete?: boolean; + } = {}, +): boolean { + const evidence = verifiedRunnerLossEvidence({ + workflowConclusion: options.workflowConclusion ?? "failure", + jobs: options.jobs ?? [hostedRunnerLossJob()], + jobDetailsAvailable: true, + jobDetailsComplete: options.complete ?? true, + }); + return evidence === null ? false : detectRunnerLoss(evidence); +} + +describe("PR E2E hosted-runner-loss classifier", () => { + it("accepts the terminalized GitHub-hosted shutdown shape from run 29965049603", () => { + expect(confirmsRunnerLoss()).toBe(true); + }); + + it("accepts two strict standard-hosted legacy markers from run 29964500642", () => { + expect( + confirmsRunnerLoss({ + jobs: [ + legacyHostedRunnerLossJob(89_073_235_001, 1_021_276_370), + legacyHostedRunnerLossJob(89_073_235_002, 1_021_276_371), + ], + }), + ).toBe(true); + }); + + it.each([ + { + label: "the workflow is cancelled", + options: { workflowConclusion: "cancelled" }, + }, + { + label: "the workflow times out", + options: { workflowConclusion: "timed_out" }, + }, + { + label: "another selected job is cancelled", + options: { + jobs: [ + hostedRunnerLossJob(), + hostedRunnerLossJob({ id: 2, name: "other", conclusion: "cancelled", steps: [] }), + ], + }, + }, + { + label: "another selected job times out", + options: { + jobs: [ + hostedRunnerLossJob(), + hostedRunnerLossJob({ id: 2, name: "other", conclusion: "timed_out", steps: [] }), + ], + }, + }, + { + label: "the synthetic completion is not last", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: [ + ...hostedRunnerLossJob().steps, + { name: "Post cleanup", status: "completed", conclusion: "skipped" }, + ], + }), + ], + }, + }, + { + label: "two workload steps are cancelled", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { name: "First workload", status: "completed", conclusion: "cancelled" }, + { name: "Second workload", status: "completed", conclusion: "cancelled" }, + { name: "Cleanup", status: "completed", conclusion: "skipped" }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }), + ], + }, + }, + { + label: "a prior step failed", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: [ + { name: "Set up job", status: "completed", conclusion: "failure" }, + ...hostedRunnerLossJob().steps.slice(1), + ], + }), + ], + }, + }, + { + label: "no cleanup step is skipped", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { name: "Workload", status: "completed", conclusion: "cancelled" }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }), + ], + }, + }, + { + label: "cleanup succeeds", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: hostedRunnerLossJob().steps.map((step) => + step.name === "Clean up Docker auth" ? { ...step, conclusion: "success" } : step, + ), + }), + ], + }, + }, + { + label: "skipped cleanup remains pending", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: hostedRunnerLossJob().steps.map((step) => + step.name === "Clean up Docker auth" ? { ...step, status: "pending" } : step, + ), + }), + ], + }, + }, + { + label: "the runner is self-hosted", + options: { jobs: [hostedRunnerLossJob({ labels: ["self-hosted", "linux"] })] }, + }, + { + label: "a standard-looking runner belongs to a custom group", + options: { + jobs: [hostedRunnerLossJob({ runnerGroupId: 7, runnerGroupName: "larger-runner-pool" })], + }, + }, + { + label: "a custom-label runner omits self-hosted", + options: { + jobs: [ + hostedRunnerLossJob({ + runnerName: "ubuntu-latest-4-cores-1234", + runnerGroupId: 7, + runnerGroupName: "larger-runner-pool", + labels: ["ubuntu-latest-4-cores"], + }), + ], + }, + }, + { + label: "the jobs listing is incomplete", + options: { complete: false }, + }, + { + label: "a legacy successful step is still pending", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: [ + { name: "Set up job", status: "pending", conclusion: "success" }, + { name: "Workload", status: "in_progress", conclusion: null }, + ], + }), + ], + }, + }, + { + label: "a legacy run completes a later synthetic step", + options: { + jobs: [ + hostedRunnerLossJob({ + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { name: "Workload", status: "in_progress", conclusion: null }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }), + ], + }, + }, + ])("rejects runner loss when $label", ({ options }) => { + expect(confirmsRunnerLoss(options)).toBe(false); + }); +}); diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index ed09d075498..9fd0a52d4a7 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -245,6 +245,8 @@ type WorkflowJob = { conclusion: string | null; runnerId?: number | null; runnerName?: string | null; + runnerGroupId?: number | null; + runnerGroupName?: string | null; labels?: string[]; steps: Array<{ name: string; status?: string; conclusion: string | null }>; }; @@ -1562,6 +1564,12 @@ function validateWorkflowJob(value: unknown): WorkflowJob { (value.runner_name !== undefined && value.runner_name !== null && typeof value.runner_name !== "string") || + (value.runner_group_id !== undefined && + value.runner_group_id !== null && + (!Number.isSafeInteger(value.runner_group_id) || (value.runner_group_id as number) < 0)) || + (value.runner_group_name !== undefined && + value.runner_group_name !== null && + typeof value.runner_group_name !== "string") || (value.labels !== undefined && (!Array.isArray(value.labels) || value.labels.some((label) => typeof label !== "string"))) || (value.steps !== undefined && !Array.isArray(value.steps)) @@ -1591,6 +1599,10 @@ function validateWorkflowJob(value: unknown): WorkflowJob { conclusion: value.conclusion, ...(value.runner_id === undefined ? {} : { runnerId: value.runner_id as number | null }), ...(value.runner_name === undefined ? {} : { runnerName: value.runner_name }), + ...(value.runner_group_id === undefined + ? {} + : { runnerGroupId: value.runner_group_id as number | null }), + ...(value.runner_group_name === undefined ? {} : { runnerGroupName: value.runner_group_name }), ...(value.labels === undefined ? {} : { labels: value.labels as string[] }), steps, }; @@ -1789,7 +1801,10 @@ function hasTrustedHostedRunnerLossMarker(job: WorkflowJob): boolean { (job.runnerId ?? 0) < 1 || typeof job.runnerName !== "string" || !GITHUB_HOSTED_RUNNER_NAME_PATTERN.test(job.runnerName) || + job.runnerGroupId !== 0 || + job.runnerGroupName !== "GitHub Actions" || !Array.isArray(job.labels) || + !job.labels.includes("ubuntu-latest") || job.labels.includes("self-hosted") ) { return false; @@ -1797,13 +1812,20 @@ function hasTrustedHostedRunnerLossMarker(job: WorkflowJob): boolean { const strandedSteps = job.steps.filter( (step) => step.status === "in_progress" && step.conclusion === null, ); + const strandedIndex = job.steps.findIndex( + (step) => step.status === "in_progress" && step.conclusion === null, + ); const legacyStrandedStep = strandedSteps.length === 1 && - job.steps.every( - (step) => - step.conclusion === "success" || - (step.conclusion === null && ["in_progress", "pending"].includes(step.status ?? "")), - ); + job.steps + .slice(0, strandedIndex) + .every( + (step) => + step.status === "completed" && ["success", "skipped"].includes(step.conclusion ?? ""), + ) && + job.steps + .slice(strandedIndex + 1) + .every((step) => step.status === "pending" && step.conclusion === null); if (legacyStrandedStep) return true; const cancelledStepIndexes = job.steps.flatMap((step, index) => @@ -1811,22 +1833,30 @@ function hasTrustedHostedRunnerLossMarker(job: WorkflowJob): boolean { ); if (cancelledStepIndexes.length !== 1) return false; const cancelledIndex = cancelledStepIndexes[0]!; + if (job.steps[cancelledIndex]?.name === "Complete job") return false; const beforeCancellation = job.steps.slice(0, cancelledIndex); const afterCancellation = job.steps.slice(cancelledIndex + 1); + const syntheticCompletion = afterCancellation.at(-1); + const skippedCleanup = afterCancellation.slice(0, -1); return ( - beforeCancellation.every((step) => ["success", "skipped"].includes(step.conclusion ?? "")) && - afterCancellation.some((step) => step.conclusion === "skipped") && - afterCancellation.every( + beforeCancellation.every( + (step) => + step.status === "completed" && ["success", "skipped"].includes(step.conclusion ?? ""), + ) && + skippedCleanup.length > 0 && + skippedCleanup.every( (step) => - step.conclusion === "skipped" || - (step.name === "Complete job" && - step.status === "completed" && - step.conclusion === "success"), - ) + step.name !== "Complete job" && + step.status === "completed" && + step.conclusion === "skipped", + ) && + syntheticCompletion?.name === "Complete job" && + syntheticCompletion.status === "completed" && + syntheticCompletion.conclusion === "success" ); } -function verifiedRunnerLossEvidence(options: { +export function verifiedRunnerLossEvidence(options: { workflowConclusion: string | null; jobs: readonly WorkflowJob[]; jobDetailsAvailable: boolean; @@ -1836,17 +1866,17 @@ function verifiedRunnerLossEvidence(options: { !options.jobDetailsAvailable || !options.jobDetailsComplete || options.jobs.length === 0 || - !["failure", "cancelled"].includes(options.workflowConclusion ?? "") + options.workflowConclusion !== "failure" ) { return null; } const runnerLostMarkerCount = options.jobs.filter(hasTrustedHostedRunnerLossMarker).length; - const ordinaryFailureEvidencePresent = options.jobs.some( - (job) => job.conclusion === "failure" && !hasTrustedHostedRunnerLossMarker(job), + const otherNonPassingEvidencePresent = options.jobs.some( + (job) => !hasTrustedHostedRunnerLossMarker(job), ); return { - terminalClassificationPresent: ordinaryFailureEvidencePresent, - jobConclusion: options.workflowConclusion as "failure" | "cancelled", + terminalClassificationPresent: otherNonPassingEvidencePresent, + jobConclusion: "failure", runnerLostMarkerCount, }; } From 7b1c34a5c145e1f11af0fc83d23642843c703abf Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 22 Jul 2026 16:51:35 -0700 Subject: [PATCH 03/20] refactor(ci): isolate retry controller paths Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-command.test.ts | 61 ++++++++++++++++++++++++++++++++ tools/e2e/pr-e2e-gate.mts | 31 +++++++++++++--- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/test/pr-e2e-gate-command.test.ts b/test/pr-e2e-gate-command.test.ts index 86cbcdcdb77..18f27aec5a1 100644 --- a/test/pr-e2e-gate-command.test.ts +++ b/test/pr-e2e-gate-command.test.ts @@ -156,6 +156,67 @@ describe("PR E2E controller commands", () => { }); }); + it("binds retry finalization and download to separate state and evidence paths", () => { + withPrivateWorkDir((workDir) => { + expect( + parseControllerCommand([ + "--mode", + "finish", + "--work-dir", + workDir, + "--slot", + "runner-loss-retry", + "--check-id", + "18", + "--run-id", + "24", + "--state-hash", + "b".repeat(64), + "--evidence-outcome", + "success", + ]), + ).toMatchObject({ + mode: "finish", + statePath: path.join(workDir, "controller-state-runner-loss-retry.json"), + evidencePath: path.join(workDir, "evidence-runner-loss-retry"), + }); + + expect( + parseControllerCommand([ + "--mode", + "download", + "--run-id", + "24", + "--work-dir", + workDir, + "--slot", + "runner-loss-retry", + ]), + ).toMatchObject({ + mode: "download", + statePath: path.join(workDir, "controller-state-runner-loss-retry.json"), + evidencePath: path.join(workDir, "evidence-runner-loss-retry"), + }); + }); + }); + + it("rejects unknown controller path slots", () => { + withPrivateWorkDir((workDir) => { + expect(() => + parseControllerCommand([ + "--mode", + "download", + "--run-id", + "24", + "--work-dir", + workDir, + "--slot", + "unexpected", + ]), + ).toThrow("--slot must be initial or runner-loss-retry"); + }); + }); + it("parses a fork credentialed E2E skip resolution", () => { expect( parseControllerCommand([ diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 9fd0a52d4a7..0629c9fe966 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -117,6 +117,8 @@ type ControllerPaths = { evidencePath: string; }; +type ControllerPathSlot = "initial" | "runner-loss-retry"; + type EvidenceStepOutcome = "success" | "failure" | "cancelled" | "skipped"; type ManualForkSkipCommandBase = { @@ -429,7 +431,16 @@ function tokenAndRepository(): { token: string; repository: string } { return { token, repository }; } -export function privateControllerPaths(workDir: string): ControllerPaths { +function parseControllerPathSlot(value: string | undefined): ControllerPathSlot { + if (value === undefined || value === "initial") return "initial"; + if (value === "runner-loss-retry") return value; + throw new Error("--slot must be initial or runner-loss-retry"); +} + +export function privateControllerPaths( + workDir: string, + slot: ControllerPathSlot = "initial", +): ControllerPaths { const resolved = path.resolve(workDir); const stat = fs.lstatSync(resolved); const currentUid = typeof process.getuid === "function" ? process.getuid() : null; @@ -442,10 +453,14 @@ export function privateControllerPaths(workDir: string): ControllerPaths { ) { throw new Error("--work-dir must be an owned private absolute directory"); } + const retry = slot === "runner-loss-retry"; return { planPath: path.join(resolved, "risk-plan.json"), - statePath: path.join(resolved, "controller-state.json"), - evidencePath: path.join(resolved, "evidence"), + statePath: path.join( + resolved, + retry ? "controller-state-runner-loss-retry.json" : "controller-state.json", + ), + evidencePath: path.join(resolved, retry ? "evidence-runner-loss-retry" : "evidence"), }; } @@ -481,7 +496,10 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { if (args.mode === "finish") { return { mode: "finish", - ...privateControllerPaths(requiredArgument(args.workDir, "work-dir")), + ...privateControllerPaths( + requiredArgument(args.workDir, "work-dir"), + parseControllerPathSlot(args.slot), + ), checkRunId: parsePositiveId(requiredArgument(args.checkId, "check-id"), "--check-id"), childRunId: parsePositiveId(requiredArgument(args.runId, "run-id"), "--run-id"), stateHash: parseHash(args.stateHash, "state-hash"), @@ -522,7 +540,10 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { return { mode: "download", childRunId: parsePositiveId(requiredArgument(args.runId, "run-id"), "--run-id"), - ...privateControllerPaths(requiredArgument(args.workDir, "work-dir")), + ...privateControllerPaths( + requiredArgument(args.workDir, "work-dir"), + parseControllerPathSlot(args.slot), + ), }; } if (args.mode === "start-control-plane") { From caaa5740afbdb44923d3e17e92bc19479b8356b6 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 22 Jul 2026 17:04:02 -0700 Subject: [PATCH 04/20] fix(ci): dispatch one hosted runner retry Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-command.test.ts | 50 ++ test/pr-e2e-gate-fork-skip.test.ts | 3 +- test/pr-e2e-gate-internal-approval.test.ts | 1 + test/pr-e2e-gate-lifecycle.test.ts | 1 + test/pr-e2e-gate-runner-loss-retry.test.ts | 521 +++++++++++++++++++++ test/pr-e2e-gate.test.ts | 1 + tools/e2e/pr-e2e-gate.mts | 303 +++++++++++- 7 files changed, 874 insertions(+), 6 deletions(-) create mode 100644 test/pr-e2e-gate-runner-loss-retry.test.ts diff --git a/test/pr-e2e-gate-command.test.ts b/test/pr-e2e-gate-command.test.ts index 18f27aec5a1..8cbae3bc951 100644 --- a/test/pr-e2e-gate-command.test.ts +++ b/test/pr-e2e-gate-command.test.ts @@ -200,6 +200,56 @@ describe("PR E2E controller commands", () => { }); }); + it("parses a runner-loss retry with its original and isolated state paths", () => { + withPrivateWorkDir((workDir) => { + expect( + parseControllerCommand([ + "--mode", + "retry-runner-loss", + "--work-dir", + workDir, + "--check-id", + "17", + "--run-id", + "23", + "--state-hash", + "a".repeat(64), + "--workflow-run-attempt", + "1", + ]), + ).toEqual({ + mode: "retry-runner-loss", + checkRunId: 17, + childRunId: 23, + workflowRunAttempt: 1, + stateHash: "a".repeat(64), + statePath: path.join(workDir, "controller-state.json"), + retryStatePath: path.join(workDir, "controller-state-runner-loss-retry.json"), + }); + }); + }); + + it("rejects runner-loss retries from controller reruns", () => { + withPrivateWorkDir((workDir) => { + expect(() => + parseControllerCommand([ + "--mode", + "retry-runner-loss", + "--work-dir", + workDir, + "--check-id", + "17", + "--run-id", + "23", + "--state-hash", + "a".repeat(64), + "--workflow-run-attempt", + "2", + ]), + ).toThrow("--workflow-run-attempt must be exactly 1"); + }); + }); + it("rejects unknown controller path slots", () => { withPrivateWorkDir((workDir) => { expect(() => diff --git a/test/pr-e2e-gate-fork-skip.test.ts b/test/pr-e2e-gate-fork-skip.test.ts index def30d55a08..b028ae581cd 100644 --- a/test/pr-e2e-gate-fork-skip.test.ts +++ b/test/pr-e2e-gate-fork-skip.test.ts @@ -29,12 +29,10 @@ const CI_RUN_ID = 99; const CI_RUN_ATTEMPT = 3; const GATE_RUN_ID = 77; const APPROVAL_RUN_ID = 123; - afterEach(() => { vi.restoreAllMocks(); vi.unstubAllEnvs(); }); - function githubResponse(value?: unknown, status = 200): Response { return { ok: status >= 200 && status < 300, @@ -915,6 +913,7 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { name: `E2E PR #42 (${correlationId})`, path: ".github/workflows/e2e.yaml", workflow_id: 7, + run_attempt: 1, event: "workflow_dispatch", head_sha: WORKFLOW_SHA, status: "queued", diff --git a/test/pr-e2e-gate-internal-approval.test.ts b/test/pr-e2e-gate-internal-approval.test.ts index 150a27cfb8e..66e364453cd 100644 --- a/test/pr-e2e-gate-internal-approval.test.ts +++ b/test/pr-e2e-gate-internal-approval.test.ts @@ -200,6 +200,7 @@ describe("PR E2E protected internal approval", () => { name: `E2E PR #42 (${correlationId})`, path: ".github/workflows/e2e.yaml", workflow_id: 7, + run_attempt: 1, event: "workflow_dispatch", head_sha: WORKFLOW_SHA, status: "queued", diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index 3eef3eb2edf..623f2c5c759 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -297,6 +297,7 @@ function workflowRun(gate: PrGateState, overrides: Record = {}) name: "E2E", path: ".github/workflows/e2e.yaml", workflow_id: 304268429, + run_attempt: 1, event: "workflow_dispatch", head_sha: gate.workflowSha, status: "completed", diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts new file mode 100644 index 00000000000..7eb99ab9c42 --- /dev/null +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -0,0 +1,521 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + type PrGateState, + prGateExternalId, + retryRunnerLossPrGate, +} from "../tools/e2e/pr-e2e-gate.mts"; +import { + createGitHubFetchRouter, + githubFetchRoute, + type RecordedGitHubRequest, +} from "./support/github-fetch-router.ts"; + +const HEAD_SHA = "a".repeat(40); +const BASE_SHA = "b".repeat(40); +const WORKFLOW_SHA = "d".repeat(40); +const ORIGINAL_CORRELATION_ID = "12345678-1234-4123-8123-123456789abc"; +const ORIGINAL_RUN_URL = "https://github.com/NVIDIA/NemoClaw/actions/runs/23"; +const RETRY_MARKER = ""; + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); +}); + +function githubResponse(value?: unknown, status = 200): Response { + return { + ok: status >= 200 && status < 300, + status, + json: async () => value, + text: async () => (value === undefined ? "" : JSON.stringify(value)), + } as Response; +} + +function sha256(value: string): string { + return createHash("sha256").update(value).digest("hex"); +} + +function state(): PrGateState { + return { + version: 3, + commitSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + planHash: "c".repeat(64), + correlationId: ORIGINAL_CORRELATION_ID, + prNumber: 42, + expectedJobs: ["onboard-repair", "onboard-resume"], + expectedTargets: [], + expectedShards: { + "onboard-repair": ["default"], + "onboard-resume": ["default"], + }, + }; +} + +function checkRun(id: number, overrides: Record = {}) { + return { + id, + name: "E2E / PR Gate Coordination", + head_sha: HEAD_SHA, + external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), + status: "completed", + conclusion: "failure", + details_url: ORIGINAL_RUN_URL, + output: { + title: "Hermes security-posture failed", + summary: `GitHub-hosted runner disappeared.\n\n${RETRY_MARKER}`, + }, + app: { id: 15368 }, + ...overrides, + }; +} + +function workflowRun(overrides: Record = {}) { + return { + id: 23, + name: "E2E", + path: ".github/workflows/e2e.yaml", + workflow_id: 304_268_429, + event: "workflow_dispatch", + head_sha: WORKFLOW_SHA, + run_attempt: 1, + status: "completed", + conclusion: "failure", + display_title: `E2E PR #42 (${ORIGINAL_CORRELATION_ID})`, + html_url: ORIGINAL_RUN_URL, + ...overrides, + }; +} + +function hostedRunnerLossJob() { + return { + id: 89_074_697_099, + name: "Hermes security-posture", + status: "completed", + conclusion: "failure", + runner_id: 1_021_277_393, + runner_name: "GitHub Actions 1021277393", + runner_group_id: 0, + runner_group_name: "GitHub Actions", + labels: ["ubuntu-latest"], + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { + name: "Run security posture live Vitest test", + status: "completed", + conclusion: "cancelled", + }, + { name: "Upload security posture artifacts", status: "completed", conclusion: "skipped" }, + { name: "Clean up Docker auth", status: "completed", conclusion: "skipped" }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }; +} + +function pullRequest() { + return { + number: 42, + state: "open", + changed_files: 1, + head: { + ref: "feature/pr-e2e-gate", + sha: HEAD_SHA, + repo: { full_name: "NVIDIA/NemoClaw" }, + }, + base: { sha: BASE_SHA, repo: { full_name: "NVIDIA/NemoClaw" } }, + }; +} + +function mutationResponse(request: RecordedGitHubRequest, id = 18): Response { + return githubResponse( + checkRun(id, { + status: "in_progress", + conclusion: null, + details_url: null, + ...(request.body as Record | undefined), + }), + ); +} + +function setup(): { + workDir: string; + outputPath: string; + statePath: string; + retryStatePath: string; + serializedState: string; + command: Parameters[0]; +} { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-runner-loss-retry-")); + const outputPath = path.join(workDir, "github-output"); + const statePath = path.join(workDir, "controller-state.json"); + const retryStatePath = path.join(workDir, "controller-state-runner-loss-retry.json"); + const serializedState = `${JSON.stringify(state(), null, 2)}\n`; + fs.writeFileSync(outputPath, "", { mode: 0o600 }); + fs.writeFileSync(statePath, serializedState, { mode: 0o600 }); + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); + return { + workDir, + outputPath, + statePath, + retryStatePath, + serializedState, + command: { + mode: "retry-runner-loss", + checkRunId: 17, + childRunId: 23, + workflowRunAttempt: 1, + stateHash: sha256(serializedState), + statePath, + retryStatePath, + }, + }; +} + +function retryRoutes( + requests: RecordedGitHubRequest[], + options: { + histories?: unknown[][]; + jobs?: unknown[]; + jobPages?: Array<{ total_count: number; jobs: unknown[] }>; + createRetryStateDirectory?: string; + } = {}, +) { + let historyRead = 0; + const defaultHistories = [ + [checkRun(17)], + [checkRun(17), checkRun(18, { status: "in_progress", conclusion: null, details_url: null })], + [checkRun(17), checkRun(18, { status: "in_progress", conclusion: null, details_url: null })], + ]; + return createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", + () => githubResponse(workflowRun()), + ), + githubFetchRoute( + ({ url, method }) => url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => { + const histories = options.histories ?? defaultHistories; + const checks = histories[Math.min(historyRead, histories.length - 1)] ?? []; + historyRead += 1; + return githubResponse({ total_count: checks.length, check_runs: checks }); + }, + ), + githubFetchRoute( + ({ url, method }) => url.includes("/actions/runs/23/attempts/1/jobs?") && method === "GET", + (request) => { + if (options.jobPages) { + const page = Number(new URL(request.url).searchParams.get("page")); + return githubResponse(options.jobPages[page - 1]); + } + const jobs = options.jobs ?? [hostedRunnerLossJob()]; + return githubResponse({ total_count: jobs.length, jobs }); + }, + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(pullRequest()), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs") && method === "POST", + (request) => mutationResponse(request), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/18") && method === "PATCH", + (request) => mutationResponse(request), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/git/ref/heads/main") && method === "GET", + () => + githubResponse({ ref: "refs/heads/main", object: { type: "commit", sha: WORKFLOW_SHA } }), + ), + githubFetchRoute( + ({ url, method }) => + url.endsWith("/actions/workflows/e2e.yaml/dispatches") && method === "POST", + () => { + if (options.createRetryStateDirectory) { + fs.mkdirSync(options.createRetryStateDirectory); + } + return githubResponse({ + workflow_run_id: 24, + run_url: "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/24", + html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/24", + }); + }, + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/24/cancel") && method === "POST", + () => githubResponse(undefined, 202), + ), + ], + requests, + ); +} + +describe("PR E2E one-time hosted-runner-loss retry", () => { + it("rejects a direct retry call from a controller workflow rerun", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation(retryRoutes(requests)); + + try { + await expect( + retryRunnerLossPrGate({ ...context.command, workflowRunAttempt: 2 }), + ).rejects.toThrow(/first controller workflow run attempt/u); + expect(requests).toHaveLength(0); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it("dispatches the same plan once with fresh state and an independently bound check", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation(retryRoutes(requests)); + + try { + await expect(retryRunnerLossPrGate(context.command)).resolves.toBeUndefined(); + const dispatch = requests.find((request) => request.url.endsWith("/dispatches")); + expect(dispatch?.body).toMatchObject({ + ref: "main", + inputs: { + jobs: "onboard-repair,onboard-resume", + targets: "", + pr_number: "42", + checkout_sha: HEAD_SHA, + base_sha: BASE_SHA, + workflow_sha: WORKFLOW_SHA, + plan_hash: "c".repeat(64), + }, + }); + const correlationId = (dispatch?.body as { inputs?: { correlation_id?: string } }).inputs + ?.correlation_id; + expect(correlationId).toMatch(/^[a-f0-9-]{36}$/u); + expect(correlationId).not.toBe(ORIGINAL_CORRELATION_ID); + + const retryState = JSON.parse(fs.readFileSync(context.retryStatePath, "utf8")); + expect(retryState).toEqual({ ...state(), correlationId }); + expect(fs.readFileSync(context.statePath, "utf8")).toBe(context.serializedState); + expect(fs.readFileSync(context.outputPath, "utf8")).toMatch( + /^check_id=18\nrun_id=24\nstate_hash=[a-f0-9]{64}\ndispatched=true\n$/u, + ); + expect( + requests.filter( + (request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH", + ), + ).toHaveLength(0); + expect( + requests.filter((request) => request.url.includes(`/commits/${HEAD_SHA}/check-runs?`)), + ).toHaveLength(3); + expect( + requests.some((request) => request.url.includes("/actions/runs/23/attempts/1/jobs?")), + ).toBe(true); + expect(requests.some((request) => request.url.includes("/actions/runs/23/jobs?"))).toBe( + false, + ); + expect( + new Set( + requests + .filter((request) => request.url.endsWith("/check-runs/18")) + .map((request) => (request.body as { output?: { title?: string } }).output?.title), + ), + ).toEqual(new Set(["Preparing one-time hosted-runner-loss retry", "Running 2 E2E checks"])); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it("rejects child reruns and mixed non-passing jobs before creating a retry check", async () => { + for (const scenario of ["child-rerun", "mixed-jobs"] as const) { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const routes = retryRoutes(requests, { + jobs: + scenario === "mixed-jobs" + ? [ + hostedRunnerLossJob(), + { id: 2, name: "other", status: "completed", conclusion: "cancelled", steps: [] }, + ] + : undefined, + }); + if (scenario === "child-rerun") { + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", + () => githubResponse(workflowRun({ run_attempt: 2 })), + ), + ], + requests, + ), + ); + } else { + vi.spyOn(globalThis, "fetch").mockImplementation(routes); + } + + try { + await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow( + scenario === "child-rerun" ? /run_attempt/u : /not authorized/u, + ); + expect(requests.some((request) => request.method === "POST")).toBe(false); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + } + } + }); + + it("does not consume a second retry for the same PR/base SHA pair", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + retryRoutes(requests, { + histories: [ + [ + checkRun(16, { + details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/22", + }), + checkRun(17), + ], + ], + }), + ); + + try { + await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow(/already consumed/u); + expect(requests.some((request) => request.method === "POST")).toBe(false); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it("rejects overlapping workflow-job pages before creating a retry check", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const firstPage = Array.from({ length: 100 }, (_, index) => ({ + ...hostedRunnerLossJob(), + id: index + 1, + })); + vi.spyOn(globalThis, "fetch").mockImplementation( + retryRoutes(requests, { + jobPages: [ + { total_count: 101, jobs: firstPage }, + { total_count: 101, jobs: [{ ...hostedRunnerLossJob(), id: 100 }] }, + ], + }), + ); + + try { + await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow( + /duplicate workflow job IDs/u, + ); + expect(requests.some((request) => request.method === "POST")).toBe(false); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it.each([ + { + name: "marker", + source: checkRun(17, { + output: { + title: "PR prerequisite CI did not pass", + summary: "Prerequisite CI failed.\n\n", + }, + }), + }, + { + name: "run URL", + source: checkRun(17, { + details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/999", + }), + }, + ])("fails closed when the source $name changes immediately before dispatch", async ({ + source, + }) => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const retryCheck = checkRun(18, { + status: "in_progress", + conclusion: null, + details_url: null, + }); + vi.spyOn(globalThis, "fetch").mockImplementation( + retryRoutes(requests, { + histories: [[checkRun(17)], [checkRun(17), retryCheck], [source, retryCheck]], + }), + ); + + try { + await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow( + /lost the current PR gate check/u, + ); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + expect( + requests.filter( + (request) => + request.url.endsWith("/check-runs/18") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ), + ).toHaveLength(1); + expect( + requests.filter( + (request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH", + ), + ).toHaveLength(0); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it("cancels a dispatched retry whose isolated state cannot be written", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + retryRoutes(requests, { createRetryStateDirectory: context.retryStatePath }), + ); + + try { + await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow( + /retry child cancellation requested/u, + ); + expect( + requests.filter( + (request) => request.url.endsWith("/actions/runs/24/cancel") && request.method === "POST", + ), + ).toHaveLength(1); + const retryCompletion = requests.find( + (request) => + request.url.endsWith("/check-runs/18") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ); + expect(retryCompletion?.body).toMatchObject({ + status: "completed", + conclusion: "failure", + output: { title: "Runner-loss retry could not start" }, + }); + expect( + requests.filter( + (request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH", + ), + ).toHaveLength(0); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); +}); diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index b2dc1a61a26..e380b63138d 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -233,6 +233,7 @@ function workflowRun(gate: PrGateState, overrides: Record = {}) name: "E2E", path: ".github/workflows/e2e.yaml", workflow_id: 304268429, + run_attempt: 1, event: "workflow_dispatch", head_sha: gate.workflowSha, status: "completed", diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 0629c9fe966..eb7bdcdb496 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -207,6 +207,15 @@ export type ControllerCommand = } | { mode: "wait"; childRunId: number } | ({ mode: "download"; childRunId: number } & ControllerPaths) + | { + mode: "retry-runner-loss"; + checkRunId: number; + childRunId: number; + workflowRunAttempt: number; + stateHash: string; + statePath: string; + retryStatePath: string; + } | ControlPlaneDispatchCommand | ApprovedControlPlaneDispatchCommand | ManualForkSkipCommand @@ -233,6 +242,7 @@ type WorkflowRun = { workflow_id: number; event: string; head_sha: string; + run_attempt: number; status: string; conclusion: string | null; display_title: string; @@ -546,6 +556,25 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { ), }; } + if (args.mode === "retry-runner-loss") { + const workDir = requiredArgument(args.workDir, "work-dir"); + const workflowRunAttempt = parsePositiveId( + requiredArgument(args.workflowRunAttempt, "workflow-run-attempt"), + "--workflow-run-attempt", + ); + if (workflowRunAttempt !== 1) { + throw new Error("--workflow-run-attempt must be exactly 1"); + } + return { + mode: "retry-runner-loss", + checkRunId: parsePositiveId(requiredArgument(args.checkId, "check-id"), "--check-id"), + childRunId: parsePositiveId(requiredArgument(args.runId, "run-id"), "--run-id"), + workflowRunAttempt, + stateHash: parseHash(args.stateHash, "state-hash"), + statePath: privateControllerPaths(workDir).statePath, + retryStatePath: privateControllerPaths(workDir, "runner-loss-retry").statePath, + }; + } if (args.mode === "start-control-plane") { const maintainer = requiredArgument(args.maintainer, "maintainer"); if (!MAINTAINER_PATTERN.test(maintainer)) throw new Error("--maintainer is invalid"); @@ -639,7 +668,7 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { }; } throw new Error( - "--mode must be seed, start, start-control-plane, start-approved-control-plane, finish, abandon, cancel, wait, download, record-fork-e2e-skip, or record-approved-fork-e2e-skip", + "--mode must be seed, start, start-control-plane, start-approved-control-plane, finish, abandon, cancel, wait, download, retry-runner-loss, record-fork-e2e-skip, or record-approved-fork-e2e-skip", ); } @@ -714,6 +743,15 @@ export function validatePrGateState(value: unknown): PrGateState { return value as PrGateState; } +function readBoundPrGateState(statePath: string, stateHash: string): PrGateState { + if (!HASH_PATTERN.test(stateHash)) throw new Error("controller state hash is invalid"); + const serializedState = readPrivateRegularFile(statePath, { maxBytes: MAX_PLAN_BYTES })!; + if (sha256(serializedState) !== stateHash) { + throw new Error("controller state changed after E2E dispatch"); + } + return validatePrGateState(JSON.parse(serializedState)); +} + export function validateRiskPlan(value: unknown, allowedJobs: ReadonlySet): RiskPlan { if (!isObjectRecord(value)) throw new Error("risk plan must be an object"); if (value.version !== RISK_PLAN_VERSION) throw new Error("unsupported risk-plan version"); @@ -1648,15 +1686,23 @@ async function listNonPassingWorkflowJobs( repository: string, token: string, runId: number, - runAttempt?: number, + runAttempt: number, ): Promise<{ jobs: WorkflowJob[]; complete: boolean }> { + if ( + !Number.isSafeInteger(runId) || + runId < 1 || + !Number.isSafeInteger(runAttempt) || + runAttempt < 1 + ) { + throw new Error("workflow run and attempt IDs must be positive safe integers"); + } const jobs: WorkflowJob[] = []; + const jobIds = new Set(); let totalCount: number | undefined; for (let page = 1; page <= MAX_WORKFLOW_JOB_PAGES; page += 1) { - const runPath = runAttempt ? `runs/${runId}/attempts/${runAttempt}` : `runs/${runId}`; const response = validateWorkflowJobsPage( await githubApi( - `repos/${repository}/actions/${runPath}/jobs?per_page=100&page=${page}`, + `repos/${repository}/actions/runs/${runId}/attempts/${runAttempt}/jobs?per_page=100&page=${page}`, token, { userAgent: USER_AGENT }, ), @@ -1665,6 +1711,12 @@ async function listNonPassingWorkflowJobs( if (response.totalCount !== totalCount || jobs.length + response.jobs.length > totalCount) { throw new Error("GitHub returned an invalid workflow job count"); } + for (const job of response.jobs) { + if (jobIds.has(job.id)) { + throw new Error("GitHub returned duplicate workflow job IDs across the job listing"); + } + jobIds.add(job.id); + } jobs.push(...response.jobs); if (jobs.length === totalCount) { return { @@ -2221,6 +2273,7 @@ export function assertCorrelatedWorkflowRun( requireEqual("id", identity.childRunId, child.id); requireEqual("path", E2E_WORKFLOW_PATH, child.path); requireEqual("event", "workflow_dispatch", child.event); + requireEqual("run_attempt", 1, child.run_attempt); requireEqual("html_url", childRunUrl, child.html_url); requireEqual( "display_title", @@ -2538,6 +2591,244 @@ async function dispatchSelectedPrGate(options: { } } +async function dispatchRunnerLossRetry(options: { + repository: string; + token: string; + state: PrGateState; + checkRunId: number; + retryStatePath: string; +}): Promise { + const correlationId = randomUUID(); + if (!CORRELATION_PATTERN.test(correlationId)) { + throw new Error("generated correlation ID is invalid"); + } + const dispatch = await dispatchPrGate({ + repository: options.repository, + token: options.token, + jobs: options.state.expectedJobs, + targets: options.state.expectedTargets, + prNumber: options.state.prNumber, + commitSha: options.state.commitSha, + baseSha: options.state.baseSha, + workflowSha: options.state.workflowSha, + planHash: options.state.planHash, + correlationId, + }); + const childRunId = dispatch.runId; + try { + appendOutput("run_id", String(childRunId)); + const retryState: PrGateState = { + ...options.state, + workflowSha: dispatch.workflowSha, + correlationId, + }; + const serializedState = `${JSON.stringify(retryState, null, 2)}\n`; + writePrivateRegularFile(options.retryStatePath, serializedState); + await updateRunningCheck( + { + repository: options.repository, + checkRunId: options.checkRunId, + prNumber: retryState.prNumber, + headSha: retryState.commitSha, + baseSha: retryState.baseSha, + }, + options.token, + { + childRunId, + jobs: retryState.expectedJobs, + targets: retryState.expectedTargets, + planHash: retryState.planHash, + }, + ); + appendOutput("state_hash", sha256(serializedState)); + appendOutput("dispatched", "true"); + console.log( + `Runner-loss retry dispatched: pr=${retryState.prNumber} run=${childRunId} plan=${retryState.planHash} jobs=${retryState.expectedJobs.join(",")} targets=${retryState.expectedTargets.join(",")} url=https://github.com/${options.repository}/actions/runs/${childRunId}`, + ); + } catch (error) { + try { + await cancelChildRun(options.repository, options.token, childRunId); + } catch (cancelError) { + throw new DispatchedChildRunError( + `${controllerErrorMessage(error)}; retry child cancellation failed: ${controllerErrorMessage(cancelError)}`, + childRunId, + ); + } + throw new DispatchedChildRunError( + `${controllerErrorMessage(error)}; retry child cancellation requested`, + childRunId, + ); + } +} + +export async function retryRunnerLossPrGate( + command: Extract, +): Promise { + if (command.workflowRunAttempt !== 1) { + throw new Error("runner-loss retry must use the first controller workflow run attempt"); + } + const { token, repository } = tokenAndRepository(); + const originalRunUrl = `https://github.com/${repository}/actions/runs/${command.childRunId}`; + let retryCheckRunId: number | undefined; + try { + const state = readBoundPrGateState(command.statePath, command.stateHash); + const child = await githubApi( + `repos/${repository}/actions/runs/${command.childRunId}`, + token, + { userAgent: USER_AGENT }, + ); + assertCorrelatedWorkflowRun(child, { + childRunId: command.childRunId, + correlationId: state.correlationId, + prNumber: state.prNumber, + repository, + workflowSha: state.workflowSha, + }); + if ( + child.status !== "completed" || + !["failure", "cancelled"].includes(child.conclusion ?? "") + ) { + throw new Error("runner-loss retry requires a terminal failed or cancelled child run"); + } + + const history = await matchingPrGateHistory({ + repository, + token, + headSha: state.commitSha, + baseSha: state.baseSha, + prNumber: state.prNumber, + }); + const current = history.at(-1); + if (current?.id !== command.checkRunId) { + throw new Error("runner-loss retry source is not the current PR gate check"); + } + if ( + retryableFailureReason(current) !== "child-cancelled" || + current.details_url !== originalRunUrl + ) { + throw new Error("PR gate check does not authorize this runner-loss retry"); + } + if (priorRunnerLossRunUrls(repository, history, command.checkRunId).length !== 0) { + throw new Error("runner-loss retry was already consumed for this PR/base SHA pair"); + } + + const jobDetails = await listNonPassingWorkflowJobs(repository, token, command.childRunId, 1); + const runnerLossEvidence = verifiedRunnerLossEvidence({ + workflowConclusion: child.conclusion, + jobs: jobDetails.jobs, + jobDetailsAvailable: true, + jobDetailsComplete: jobDetails.complete, + }); + const retryDecision = runnerLossEvidence + ? decideRetry({ + runnerLoss: detectRunnerLoss(runnerLossEvidence), + classification: null, + attempt: 1, + }) + : { retry: false, reason: "runner-loss evidence is incomplete" }; + if (!retryDecision.retry) { + throw new Error(`runner-loss retry is not authorized: ${retryDecision.reason}`); + } + + const pull = await requireLiveExactDiff({ + repository, + token, + prNumber: state.prNumber, + headSha: state.commitSha, + baseSha: state.baseSha, + }); + if (pull.head.repo?.full_name !== repository) { + throw new Error("runner-loss retry requires an internal pull request"); + } + + const historySize = history.length; + const retryCheck = await createPrGateCheck({ + repository, + token, + headSha: state.commitSha, + baseSha: state.baseSha, + prNumber: state.prNumber, + }); + retryCheckRunId = retryCheck.id; + appendOutput("check_id", String(retryCheckRunId)); + const retryHistory = await matchingPrGateHistory({ + repository, + token, + headSha: state.commitSha, + baseSha: state.baseSha, + prNumber: state.prNumber, + }); + if (retryHistory.length !== historySize + 1 || retryHistory.at(-1)?.id !== retryCheckRunId) { + throw new Error("runner-loss retry did not acquire the current PR gate check"); + } + await markCheckInProgress( + { + repository, + checkRunId: retryCheckRunId, + prNumber: state.prNumber, + headSha: state.commitSha, + baseSha: state.baseSha, + }, + token, + "Preparing one-time hosted-runner-loss retry", + `Revalidating the exact PR/base SHA and risk plan after [attempt 1](${originalRunUrl}) lost its GitHub-hosted runner.`, + ); + + const currentPull = await requireLiveExactDiff({ + repository, + token, + prNumber: state.prNumber, + headSha: state.commitSha, + baseSha: state.baseSha, + }); + assertPullUnchanged(pull, currentPull); + const dispatchHistory = await matchingPrGateHistory({ + repository, + token, + headSha: state.commitSha, + baseSha: state.baseSha, + prNumber: state.prNumber, + }); + const dispatchSource = dispatchHistory.find((check) => check.id === command.checkRunId); + if ( + dispatchHistory.length !== historySize + 1 || + dispatchHistory.at(-1)?.id !== retryCheckRunId || + !dispatchSource || + retryableFailureReason(dispatchSource) !== "child-cancelled" || + dispatchSource.details_url !== originalRunUrl || + priorRunnerLossRunUrls(repository, dispatchHistory, retryCheckRunId).length !== 1 + ) { + throw new Error("runner-loss retry lost the current PR gate check before dispatch"); + } + await dispatchRunnerLossRetry({ + repository, + token, + state, + checkRunId: retryCheckRunId, + retryStatePath: command.retryStatePath, + }); + } catch (error) { + if (retryCheckRunId !== undefined) { + const retryRunId = error instanceof DispatchedChildRunError ? error.childRunId : undefined; + const closed = await completeFailureAfterControllerError( + { repository, checkRunId: retryCheckRunId }, + token, + "Runner-loss retry could not start", + { + error, + detailsUrl: retryRunId + ? `https://github.com/${repository}/actions/runs/${retryRunId}` + : originalRunUrl, + recovery: + "The original runner-loss evidence remains linked. This exact PR/base SHA pair will not receive another automatic retry.", + }, + ); + if (closed) appendOutput("finalized", "true"); + } + throw error; + } +} + export async function startPrGate( command: Extract, ): Promise { @@ -3693,6 +3984,10 @@ async function main(): Promise { await startApprovedControlPlanePrGate(command); return; } + if (command.mode === "retry-runner-loss") { + await retryRunnerLossPrGate(command); + return; + } if (command.mode === "finish") { await finishPrGate({ statePath: command.statePath, From 1b9ae49784a3a899193c346c6087f24ccc40704d Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 22 Jul 2026 17:29:20 -0700 Subject: [PATCH 05/20] fix(ci): terminalize runner retry lifecycle Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-lifecycle.test.ts | 3 +- test/pr-e2e-gate-runner-loss-retry.test.ts | 155 +++++++++++++++++++++ tools/e2e/pr-e2e-gate.mts | 40 +++--- 3 files changed, 181 insertions(+), 17 deletions(-) diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index 623f2c5c759..4629d711a09 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -956,7 +956,8 @@ describe("PR E2E controller lifecycle", () => { () => githubResponse(undefined, 202), ), githubFetchRoute( - ({ url, method }) => url.includes("/actions/runs/23/jobs?") && method === "GET", + ({ url, method }) => + url.includes("/actions/runs/23/attempts/1/jobs?") && method === "GET", () => jobs === null ? githubResponse({ message: "temporary failure" }, 503) diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index 7eb99ab9c42..06130f761af 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -8,6 +8,7 @@ import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { + finishPrGate, type PrGateState, prGateExternalId, retryRunnerLossPrGate, @@ -518,4 +519,158 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { fs.rmSync(context.workDir, { recursive: true, force: true }); } }); + + it.each([ + { label: "loses another hosted runner", conclusion: "failure", evidenceOutcome: "skipped" }, + { label: "cannot download evidence", conclusion: "success", evidenceOutcome: "failure" }, + ] as const)("terminalizes attempt 2 when it $label", async ({ conclusion, evidenceOutcome }) => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const retryCorrelationId = "87654321-4321-4123-8123-cba987654321"; + const retryState = { ...state(), correlationId: retryCorrelationId }; + const retryStateContents = `${JSON.stringify(retryState, null, 2)}\n`; + fs.writeFileSync(context.retryStatePath, retryStateContents, { mode: 0o600 }); + const currentCheck = checkRun(18, { + status: "in_progress", + conclusion: null, + details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/24", + output: { title: "Running 2 E2E checks", summary: "Attempt 2 is running." }, + }); + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/24") && method === "GET", + () => + githubResponse( + workflowRun({ + id: 24, + conclusion, + display_title: `E2E PR #42 (${retryCorrelationId})`, + html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/24", + }), + ), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => githubResponse({ total_count: 2, check_runs: [checkRun(17), currentCheck] }), + ), + githubFetchRoute( + ({ url, method }) => + url.includes("/actions/runs/24/attempts/1/jobs?") && method === "GET", + () => githubResponse({ total_count: 1, jobs: [hostedRunnerLossJob()] }), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(pullRequest()), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/18") && method === "PATCH", + (request) => mutationResponse(request), + ), + ], + requests, + ), + ); + + try { + const finalization = finishPrGate({ + statePath: context.retryStatePath, + stateHash: sha256(retryStateContents), + evidencePath: context.workDir, + checkRunId: 18, + childRunId: 24, + evidenceOutcome, + }); + if (conclusion === "success") { + await expect(finalization).rejects.toThrow(/Evidence download did not complete/u); + } else { + await expect(finalization).resolves.toBeUndefined(); + } + const completion = requests.find( + (request) => + request.url.endsWith("/check-runs/18") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ); + const summary = (completion?.body as { output?: { summary?: string } }).output?.summary; + expect(completion?.body).toMatchObject({ status: "completed", conclusion: "failure" }); + expect(summary).toContain(`[attempt 1](${ORIGINAL_RUN_URL})`); + expect(summary).toContain("[attempt 2](https://github.com/NVIDIA/NemoClaw/actions/runs/24)"); + expect(summary).not.toContain("nemoclaw-pr-e2e-retry:v1:"); + expect(fs.readFileSync(context.outputPath, "utf8")).not.toContain( + "runner_loss_retry_authorized=true", + ); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it("fails closed when retry authorization cannot be written to controller output", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + fs.rmSync(context.outputPath); + fs.mkdirSync(context.outputPath); + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", + () => githubResponse(workflowRun()), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => + githubResponse({ + total_count: 1, + check_runs: [checkRun(17, { status: "in_progress", conclusion: null })], + }), + ), + githubFetchRoute( + ({ url, method }) => + url.includes("/actions/runs/23/attempts/1/jobs?") && method === "GET", + () => githubResponse({ total_count: 1, jobs: [hostedRunnerLossJob()] }), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(pullRequest()), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", + (request) => mutationResponse(request, 17), + ), + ], + requests, + ), + ); + + try { + await expect( + finishPrGate({ + statePath: context.statePath, + stateHash: sha256(context.serializedState), + evidencePath: context.workDir, + checkRunId: 17, + childRunId: 23, + evidenceOutcome: "skipped", + }), + ).rejects.toThrow(); + const completions = requests.filter( + (request) => + request.url.endsWith("/check-runs/17") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ); + expect(completions).toHaveLength(1); + expect(completions[0]?.body).toMatchObject({ + conclusion: "failure", + output: { title: "Evidence could not be verified" }, + }); + expect(JSON.stringify(completions[0]?.body)).not.toContain("nemoclaw-pr-e2e-retry:v1:"); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); }); diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index eb7bdcdb496..92ad7eba3c0 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -943,6 +943,7 @@ function appendOutput(name: string, value: string): void { fork_skip_mode: (candidate) => candidate === "record-fork-e2e-skip", fork_skip_pr_number: (candidate) => /^[1-9][0-9]*$/u.test(candidate), finalized: (candidate) => /^(?:true|false)$/u.test(candidate), + runner_loss_retry_authorized: (candidate) => candidate === "true", run_id: (candidate) => /^[1-9][0-9]*$/u.test(candidate), state_hash: (candidate) => HASH_PATTERN.test(candidate), }; @@ -1091,18 +1092,27 @@ function priorRunnerLossRunUrls( }); } -function withRunnerLossLineage( - verdict: PrGateVerdict, +function runnerLossLineageSummary( priorRunUrls: readonly string[], currentRunUrl: string, -): PrGateVerdict { - if (priorRunUrls.length === 0) return verdict; +): string | undefined { + if (priorRunUrls.length === 0) return undefined; const links = [...priorRunUrls, currentRunUrl].map( (url, index) => `[attempt ${index + 1}](${url})`, ); + return `Runner-loss retry lineage: ${links.join(" → ")}.`; +} + +function withRunnerLossLineage( + verdict: PrGateVerdict, + priorRunUrls: readonly string[], + currentRunUrl: string, +): PrGateVerdict { + const lineage = runnerLossLineageSummary(priorRunUrls, currentRunUrl); + if (!lineage) return verdict; return { ...verdict, - summary: `${verdict.summary}\nRunner-loss retry lineage: ${links.join(" → ")}.`, + summary: `${verdict.summary}\n${lineage}`, }; } @@ -3358,14 +3368,7 @@ export async function finishPrGate(options: { let finalized = false; let controllerFailureRetryReason: RetryableFailureReason | undefined; try { - if (!HASH_PATTERN.test(options.stateHash)) throw new Error("controller state hash is invalid"); - const serializedState = readPrivateRegularFile(options.statePath, { - maxBytes: MAX_PLAN_BYTES, - })!; - if (sha256(serializedState) !== options.stateHash) { - throw new Error("controller state changed after E2E dispatch"); - } - const state = validatePrGateState(JSON.parse(serializedState)); + const state = readBoundPrGateState(options.statePath, options.stateHash); const child = await githubApi( `repos/${repository}/actions/runs/${options.childRunId}`, token, @@ -3431,7 +3434,8 @@ export async function finishPrGate(options: { let verdict: PrGateVerdict; if (workflowConclusion === "success") { if (options.evidenceOutcome !== "success") { - controllerFailureRetryReason = "evidence-download"; + controllerFailureRetryReason = + priorRunnerLossUrls.length === 0 ? "evidence-download" : undefined; const error = new Error( `Evidence download did not complete (outcome: ${options.evidenceOutcome}) after selected E2E run ${options.childRunId} succeeded. The controller could not verify its artifacts; inspect the Download evidence step and rerun the gate.`, ); @@ -3442,6 +3446,7 @@ export async function finishPrGate(options: { { error, detailsUrl: childRunUrl, + recovery: runnerLossLineageSummary(priorRunnerLossUrls, childRunUrl), retryableFailureReason: controllerFailureRetryReason, }, ); @@ -3473,7 +3478,7 @@ export async function finishPrGate(options: { let jobDetailsAvailable = true; let jobDetailsComplete = false; try { - const details = await listNonPassingWorkflowJobs(repository, token, options.childRunId); + const details = await listNonPassingWorkflowJobs(repository, token, options.childRunId, 1); jobs = details.jobs; jobDetailsComplete = details.complete; } catch (error) { @@ -3498,9 +3503,12 @@ export async function finishPrGate(options: { } verdict = withRunnerLossLineage(verdict, priorRunnerLossUrls, childRunUrl); if (await finalizeObsoleteExactDiff()) return; + if (priorRunnerLossUrls.length === 0 && verdict.retryableFailureReason === "child-cancelled") { + appendOutput("runner_loss_retry_authorized", "true"); + } await completeCheck(context, token, verdict, childRunUrl); - appendOutput("finalized", "true"); finalized = true; + appendOutput("finalized", "true"); console.log( `Run completed: run=${options.childRunId} conclusion=${verdict.conclusion} title=${verdict.title} url=${childRunUrl}`, ); From 7537ea286757c4605be74cf79a0612106c15a569 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 22 Jul 2026 17:34:55 -0700 Subject: [PATCH 06/20] fix(ci): preserve retry check ownership Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-fork-skip.test.ts | 9 ++-- test/pr-e2e-gate-lifecycle.test.ts | 38 +++++++++++++- test/pr-e2e-gate-runner-loss-retry.test.ts | 58 ++++++++++++++++++++++ test/pr-e2e-gate.test.ts | 5 ++ tools/e2e/pr-e2e-gate.mts | 45 ++++++++++++++--- 5 files changed, 144 insertions(+), 11 deletions(-) diff --git a/test/pr-e2e-gate-fork-skip.test.ts b/test/pr-e2e-gate-fork-skip.test.ts index b028ae581cd..035be950c08 100644 --- a/test/pr-e2e-gate-fork-skip.test.ts +++ b/test/pr-e2e-gate-fork-skip.test.ts @@ -41,14 +41,12 @@ function githubResponse(value?: unknown, status = 200): Response { text: async () => (value === undefined ? "" : JSON.stringify(value)), } as Response; } - function emptyPrGateCheckRunsRoute() { return githubFetchRoute( ({ url, method }) => url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", () => githubResponse({ total_count: 0, check_runs: [] }), ); } - function exactPrGateCheck(overrides: Record = {}) { return { id: 17, @@ -57,18 +55,21 @@ function exactPrGateCheck(overrides: Record = {}) { external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), status: "in_progress", conclusion: null, + output: { + title: "Waiting for PR CI", + summary: + "This PR SHA and base SHA are reserved for deterministic E2E planning after CI completes.", + }, app: { id: 15368 }, ...overrides, }; } - function existingPrGateCheckRunsRoute(overrides: Record = {}) { return githubFetchRoute( ({ url, method }) => url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", () => githubResponse({ total_count: 1, check_runs: [exactPrGateCheck(overrides)] }), ); } - function prGateMutationResponse(request: RecordedGitHubRequest, id = 17): Response { const body = (request.body ?? {}) as Record; return githubResponse(exactPrGateCheck({ id, ...body })); diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index 4629d711a09..9464de99dec 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -1341,6 +1341,10 @@ describe("PR E2E controller lifecycle", () => { ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", () => githubResponse(undefined, 202), ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "GET", + () => githubResponse(exactPrGateCheck({ status: "in_progress" })), + ), githubFetchRoute( ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", (request) => prGateMutationResponse(request), @@ -1353,10 +1357,11 @@ describe("PR E2E controller lifecycle", () => { try { await abandonPrGate(17, 23); expect(requests.map((request) => request.url)).toEqual([ + "https://api.github.com/repos/NVIDIA/NemoClaw/check-runs/17", "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/23/cancel", "https://api.github.com/repos/NVIDIA/NemoClaw/check-runs/17", ]); - expect(requests[1]?.body).toMatchObject({ + expect(requests[2]?.body).toMatchObject({ status: "completed", conclusion: "failure", output: { @@ -1370,6 +1375,37 @@ describe("PR E2E controller lifecycle", () => { } }); + it("does not cancel a child after an abandoned check is already completed", async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-abandon-done-")); + const outputPath = path.join(directory, "github-output"); + fs.writeFileSync(outputPath, "", { mode: 0o600 }); + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "GET", + () => githubResponse(exactPrGateCheck({ status: "completed", conclusion: "failure" })), + ), + ], + requests, + ), + ); + + try { + await expect(abandonPrGate(17, 23)).resolves.toBeUndefined(); + expect(requests).toHaveLength(1); + expect(requests[0]?.method).toBe("GET"); + expect(requests.some((request) => request.url.endsWith("/cancel"))).toBe(false); + expect(fs.readFileSync(outputPath, "utf8")).toBe("finalized=true\n"); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } + }); + it("bounds recursive signal discovery and rejects symlinks", () => { const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-evidence-")); try { diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index 06130f761af..af6da711c25 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -11,7 +11,9 @@ import { finishPrGate, type PrGateState, prGateExternalId, + privateControllerPaths, retryRunnerLossPrGate, + startPrGate, } from "../tools/e2e/pr-e2e-gate.mts"; import { createGitHubFetchRouter, @@ -520,6 +522,62 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { } }); + it("does not let a fresh CI event claim an active runner-loss retry check", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => + githubResponse({ + total_count: 1, + check_runs: [ + checkRun(18, { + status: "in_progress", + conclusion: null, + output: { title: "Running 2 E2E checks", summary: "Attempt 2 is running." }, + }), + ], + }), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(pullRequest()), + ), + ], + requests, + ), + ); + + try { + await expect( + startPrGate({ + mode: "start", + headSha: HEAD_SHA, + headRepository: "NVIDIA/NemoClaw", + headBranch: "feature/pr-e2e-gate", + workflowSha: WORKFLOW_SHA, + ciConclusion: "success", + ciDisplayTitle: `CI PR #42 head ${HEAD_SHA} base ${BASE_SHA} gate true`, + ciRunId: 99, + ciRunAttempt: 1, + gateRunId: 77, + prNumber: 42, + ...privateControllerPaths(context.workDir), + }), + ).rejects.toThrow(/not retryable/u); + expect(requests).toHaveLength(2); + expect(requests.some((request) => request.method === "POST")).toBe(false); + expect(requests.some((request) => request.method === "PATCH")).toBe(false); + expect(fs.readFileSync(context.outputPath, "utf8")).toBe(""); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + it.each([ { label: "loses another hosted runner", conclusion: "failure", evidenceOutcome: "skipped" }, { label: "cannot download evidence", conclusion: "success", evidenceOutcome: "failure" }, diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index e380b63138d..8b7d2db13fa 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -104,6 +104,11 @@ function exactPrGateCheck(overrides: Record = {}) { external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), status: "in_progress", conclusion: null, + output: { + title: "Waiting for PR CI", + summary: + "This PR SHA and base SHA are reserved for deterministic E2E planning after CI completes.", + }, app: { id: 15368 }, ...overrides, }; diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 92ad7eba3c0..329043f82e8 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -42,6 +42,9 @@ const FORK_SKIP_APPROVAL_ENVIRONMENT = "approve-credentialed-e2e-skip-for-fork-p const INTERNAL_E2E_APPROVAL_ENVIRONMENT = "approve-credentialed-e2e-for-internal-pr"; const CHECK_NAME = "E2E / PR Gate Coordination"; const WORKFLOW_NAME = "E2E / PR Gate Controller"; +const RESERVED_CHECK_TITLE = "Waiting for PR CI"; +const RESERVED_CHECK_SUMMARY = + "This PR SHA and base SHA are reserved for deterministic E2E planning after CI completes."; const CONTROL_PLANE_AUTHORIZATION_TITLE = "E2E reviewer authorization required to run E2E"; const RETRYABLE_FAILURE_MARKER_PREFIX = ""; @@ -1219,9 +1222,8 @@ async function createPrGateCheck(options: { prNumber: number; }): Promise { const externalId = prGateExternalId(options.prNumber, options.headSha, options.baseSha); - const title = "Waiting for PR CI"; - const summary = - "This PR SHA and base SHA are reserved for deterministic E2E planning after CI completes."; + const title = RESERVED_CHECK_TITLE; + const summary = RESERVED_CHECK_SUMMARY; const check = await githubApi(`repos/${options.repository}/check-runs`, options.token, { method: "POST", body: { @@ -1335,7 +1337,14 @@ async function markCheckInProgress( function assertCheckCanStart(check: CheckRun | undefined, ciConclusion: string): void { if (!check) return; - if (check.status === "in_progress" && check.conclusion === null) return; + if ( + check.status === "in_progress" && + check.conclusion === null && + check.output?.title === RESERVED_CHECK_TITLE && + check.output.summary === RESERVED_CHECK_SUMMARY + ) { + return; + } const reason = retryableFailureReason(check); if (ciConclusion === "success" && reason) return; const title = normalizedCiMetadata(check.output?.title ?? "untitled", "untitled"); @@ -2869,7 +2878,6 @@ export async function startPrGate( } const existingCheckRunId = existingChecks[0]?.status === "in_progress" ? existingChecks[0].id : undefined; - if (existingCheckRunId) appendOutput("check_id", String(existingCheckRunId)); let pull: PullRequest; try { pull = await requireLiveExactDiff({ @@ -2881,7 +2889,12 @@ export async function startPrGate( }); } catch (error) { if (!(error instanceof ObsoleteExactDiffError)) throw error; - if (existingCheckRunId) { + if ( + existingCheckRunId && + existingChecks[0]?.output?.title === RESERVED_CHECK_TITLE && + existingChecks[0].output.summary === RESERVED_CHECK_SUMMARY + ) { + appendOutput("check_id", String(existingCheckRunId)); await completeCheck({ repository, checkRunId: existingCheckRunId }, token, error.verdict); } appendOutput("dispatched", "false"); @@ -2898,6 +2911,7 @@ export async function startPrGate( throw new Error("PR repository or branch does not match the triggering CI run"); } assertCheckCanStart(existingChecks[0], command.ciConclusion); + if (existingCheckRunId) appendOutput("check_id", String(existingCheckRunId)); const checkRunId = await ensurePrGateCheck({ repository, token, @@ -3532,6 +3546,25 @@ export async function finishPrGate(options: { export async function abandonPrGate(checkRunId: number, childRunId?: number): Promise { const { token, repository } = tokenAndRepository(); + const existingCheck = await githubApi( + `repos/${repository}/check-runs/${checkRunId}`, + token, + { userAgent: USER_AGENT }, + ); + if ( + !isObjectRecord(existingCheck) || + existingCheck.id !== checkRunId || + existingCheck.name !== CHECK_NAME || + !isObjectRecord(existingCheck.app) || + existingCheck.app.id !== GITHUB_ACTIONS_APP_ID || + typeof existingCheck.status !== "string" + ) { + throw new Error("GitHub returned a mismatched PR gate check during abandonment"); + } + if (existingCheck.status === "completed") { + appendOutput("finalized", "true"); + return; + } let cancellationError: unknown; if (childRunId) { try { From a6d5546a8648c20842797d6d5a508f8d9525db3d Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 00:15:14 -0700 Subject: [PATCH 07/20] fix(ci): bind hosted runner-loss retry evidence Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-lifecycle.test.ts | 23 +- ...pr-e2e-gate-runner-loss-classifier.test.ts | 84 ++++- test/pr-e2e-gate-runner-loss-retry.test.ts | 250 +++++++++++-- tools/e2e/pr-e2e-gate.mts | 332 +++++++++++++++--- 4 files changed, 586 insertions(+), 103 deletions(-) diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index 9464de99dec..9e2717951c4 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -646,7 +646,7 @@ describe("PR E2E controller lifecycle", () => { expectedRetryReason: undefined, }, { - label: "a GitHub-hosted runner disappears with its live step still active", + label: "a lost-runner step shape lacks trusted annotation identity", status: "completed", conclusion: "failure", jobs: [ @@ -658,7 +658,7 @@ describe("PR E2E controller lifecycle", () => { runner_id: 1_020_705_058, runner_name: "GitHub Actions 1020705058", runner_group_id: 0, - runner_group_name: "GitHub Actions", + runner_group_name: "Unverified runner group", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -680,11 +680,11 @@ describe("PR E2E controller lifecycle", () => { assertCompletionLink: expectSelectedRunLink, expectCancellation: false, expectedTitle: "rebuild-hermes failed", - expectedSummary: "confirmed hosted-runner loss on attempt 1", - expectedRetryReason: "child-cancelled", + expectedSummary: "concluded `failure`", + expectedRetryReason: undefined, }, { - label: "a GitHub-hosted runner shutdown terminalizes the live step and skips cleanup", + label: "a terminalized shutdown shape lacks trusted annotation identity", status: "completed", conclusion: "failure", jobs: [ @@ -696,7 +696,7 @@ describe("PR E2E controller lifecycle", () => { runner_id: 1_021_276_374, runner_name: "GitHub Actions 1021276374", runner_group_id: 0, - runner_group_name: "GitHub Actions", + runner_group_name: "Unverified runner group", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -724,8 +724,8 @@ describe("PR E2E controller lifecycle", () => { assertCompletionLink: expectSelectedRunLink, expectCancellation: false, expectedTitle: "Hermes security-posture failed", - expectedSummary: "confirmed hosted-runner loss on attempt 1", - expectedRetryReason: "child-cancelled", + expectedSummary: "concluded `failure`", + expectedRetryReason: undefined, }, { label: "a cancelled step followed by successful cleanup is not runner loss", @@ -740,7 +740,7 @@ describe("PR E2E controller lifecycle", () => { runner_id: 1_021_276_374, runner_name: "GitHub Actions 1021276374", runner_group_id: 0, - runner_group_name: "GitHub Actions", + runner_group_name: "Unverified runner group", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -779,7 +779,7 @@ describe("PR E2E controller lifecycle", () => { runner_id: 1_020_705_058, runner_name: "GitHub Actions 1020705058", runner_group_id: 0, - runner_group_name: "GitHub Actions", + runner_group_name: "Unverified runner group", labels: ["ubuntu-latest"], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, @@ -814,8 +814,7 @@ describe("PR E2E controller lifecycle", () => { assertCompletionLink: expectSelectedRunLink, expectCancellation: false, expectedTitle: "Selected E2E did not pass", - expectedSummary: - "an unclassified failure is never retried; only a confirmed hosted-runner loss is", + expectedSummary: "failed step: `Run security posture live test`", expectedRetryReason: undefined, }, { diff --git a/test/pr-e2e-gate-runner-loss-classifier.test.ts b/test/pr-e2e-gate-runner-loss-classifier.test.ts index c2172f16d68..9e63cfac337 100644 --- a/test/pr-e2e-gate-runner-loss-classifier.test.ts +++ b/test/pr-e2e-gate-runner-loss-classifier.test.ts @@ -5,10 +5,30 @@ import { describe, expect, it } from "vitest"; import { verifiedRunnerLossEvidence } from "../tools/e2e/pr-e2e-gate.mts"; import { detectRunnerLoss } from "../tools/e2e/runner-pressure-core.mts"; +const WORKFLOW_SHA = "d".repeat(40); +const RUNNER_LOSS_MESSAGE = + "The hosted runner lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error."; + +function runnerLossAnnotation(message = RUNNER_LOSS_MESSAGE) { + return { + path: ".github", + blobHref: `https://github.com/NVIDIA/NemoClaw/blob/${WORKFLOW_SHA}/.github`, + startLine: 1, + startColumn: null, + endLine: 1, + endColumn: null, + annotationLevel: "failure", + title: "", + message, + rawDetails: "", + }; +} + function hostedRunnerLossJob(overrides: Record = {}) { return { id: 89_074_697_099, name: "Hermes security-posture", + headSha: WORKFLOW_SHA, status: "completed", conclusion: "failure", runnerId: 1_021_277_393, @@ -16,6 +36,7 @@ function hostedRunnerLossJob(overrides: Record = {}) { runnerGroupId: 0, runnerGroupName: "GitHub Actions", labels: ["ubuntu-latest"], + annotations: [runnerLossAnnotation()], steps: [ { name: "Set up job", status: "completed", conclusion: "success" }, { @@ -52,6 +73,8 @@ function confirmsRunnerLoss( } = {}, ): boolean { const evidence = verifiedRunnerLossEvidence({ + repository: "NVIDIA/NemoClaw", + workflowSha: WORKFLOW_SHA, workflowConclusion: options.workflowConclusion ?? "failure", jobs: options.jobs ?? [hostedRunnerLossJob()], jobDetailsAvailable: true, @@ -61,7 +84,7 @@ function confirmsRunnerLoss( } describe("PR E2E hosted-runner-loss classifier", () => { - it("accepts the terminalized GitHub-hosted shutdown shape from run 29965049603", () => { + it("accepts a terminalized hosted shutdown only with canonical lost-communication evidence", () => { expect(confirmsRunnerLoss()).toBe(true); }); @@ -76,7 +99,66 @@ describe("PR E2E hosted-runner-loss classifier", () => { ).toBe(true); }); + it("allows an unrelated notice beside the sole canonical failure annotation", () => { + expect( + confirmsRunnerLoss({ + jobs: [ + hostedRunnerLossJob({ + annotations: [ + runnerLossAnnotation(), + { + ...runnerLossAnnotation("Docker credentials were withheld."), + startLine: 53, + endLine: 53, + annotationLevel: "notice", + }, + ], + }), + ], + }), + ).toBe(true); + }); + it.each([ + { + label: "the only failure annotation is the generic cancellation from run 29965049603", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [runnerLossAnnotation("The operation was canceled.")], + }), + ], + }, + }, + { + label: "the failure annotation reports a job timeout", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [ + runnerLossAnnotation( + "The job running on runner GitHub Actions 123 has exceeded the maximum execution time of 75 minutes.", + ), + ], + }), + ], + }, + }, + { + label: "the canonical annotation is bound to a different workflow SHA", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [ + { + ...runnerLossAnnotation(), + blobHref: `https://github.com/NVIDIA/NemoClaw/blob/${"e".repeat(40)}/.github`, + }, + ], + }), + ], + }, + }, { label: "the workflow is cancelled", options: { workflowConclusion: "cancelled" }, diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index af6da711c25..3b0e8870189 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -99,9 +99,17 @@ function workflowRun(overrides: Record = {}) { }; } -function hostedRunnerLossJob() { +function hostedRunnerLossJob(runId = 23) { + const id = 89_074_697_099; return { - id: 89_074_697_099, + id, + run_id: runId, + run_attempt: 1, + head_sha: WORKFLOW_SHA, + run_url: `https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/${runId}`, + url: `https://api.github.com/repos/NVIDIA/NemoClaw/actions/jobs/${id}`, + html_url: `https://github.com/NVIDIA/NemoClaw/actions/runs/${runId}/job/${id}`, + check_run_url: `https://api.github.com/repos/NVIDIA/NemoClaw/check-runs/${id}`, name: "Hermes security-posture", status: "completed", conclusion: "failure", @@ -124,6 +132,38 @@ function hostedRunnerLossJob() { }; } +function runnerLossAnnotation() { + return { + path: ".github", + blob_href: `https://github.com/NVIDIA/NemoClaw/blob/${WORKFLOW_SHA}/.github`, + start_line: 1, + start_column: null, + end_line: 1, + end_column: null, + annotation_level: "failure", + title: "", + message: + "The hosted runner lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error.", + raw_details: "", + }; +} + +function workflowJobCheckRun(job: ReturnType) { + const annotationsUrl = `${job.check_run_url}/annotations`; + return { + id: job.id, + name: job.name, + head_sha: job.head_sha, + url: job.check_run_url, + html_url: job.html_url, + details_url: job.html_url, + status: "completed", + conclusion: "failure", + app: { id: 15368 }, + output: { annotations_count: 1, annotations_url: annotationsUrl }, + }; +} + function pullRequest() { return { number: 42, @@ -191,10 +231,14 @@ function retryRoutes( histories?: unknown[][]; jobs?: unknown[]; jobPages?: Array<{ total_count: number; jobs: unknown[] }>; + workflow?: unknown; + jobCheck?: unknown; + annotationPages?: unknown[][]; createRetryStateDirectory?: string; } = {}, ) { let historyRead = 0; + let annotationRead = 0; const defaultHistories = [ [checkRun(17)], [checkRun(17), checkRun(18, { status: "in_progress", conclusion: null, details_url: null })], @@ -204,7 +248,7 @@ function retryRoutes( [ githubFetchRoute( ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", - () => githubResponse(workflowRun()), + () => githubResponse(options.workflow ?? workflowRun()), ), githubFetchRoute( ({ url, method }) => url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", @@ -226,6 +270,20 @@ function retryRoutes( return githubResponse({ total_count: jobs.length, jobs }); }, ), + githubFetchRoute( + ({ url, method }) => + url.endsWith(`/check-runs/${hostedRunnerLossJob().id}`) && method === "GET", + () => githubResponse(options.jobCheck ?? workflowJobCheckRun(hostedRunnerLossJob())), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/check-runs/${hostedRunnerLossJob().id}/annotations?`) && method === "GET", + () => { + const annotations = options.annotationPages?.[annotationRead] ?? [runnerLossAnnotation()]; + annotationRead += 1; + return githubResponse(annotations); + }, + ), githubFetchRoute( ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", () => githubResponse(pullRequest()), @@ -339,40 +397,36 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { } }); - it("rejects child reruns and mixed non-passing jobs before creating a retry check", async () => { + it("closes the reserved retry check for child reruns and mixed non-passing jobs", async () => { for (const scenario of ["child-rerun", "mixed-jobs"] as const) { const context = setup(); const requests: RecordedGitHubRequest[] = []; - const routes = retryRoutes(requests, { - jobs: - scenario === "mixed-jobs" - ? [ - hostedRunnerLossJob(), - { id: 2, name: "other", status: "completed", conclusion: "cancelled", steps: [] }, - ] - : undefined, - }); - if (scenario === "child-rerun") { - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", - () => githubResponse(workflowRun({ run_attempt: 2 })), - ), - ], - requests, - ), - ); - } else { - vi.spyOn(globalThis, "fetch").mockImplementation(routes); - } + vi.spyOn(globalThis, "fetch").mockImplementation( + retryRoutes(requests, { + workflow: scenario === "child-rerun" ? workflowRun({ run_attempt: 2 }) : undefined, + jobs: + scenario === "mixed-jobs" + ? [ + hostedRunnerLossJob(), + { id: 2, name: "other", status: "completed", conclusion: "cancelled", steps: [] }, + ] + : undefined, + }), + ); try { await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow( scenario === "child-rerun" ? /run_attempt/u : /not authorized/u, ); - expect(requests.some((request) => request.method === "POST")).toBe(false); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + expect( + requests.some( + (request) => + request.url.endsWith("/check-runs/18") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ), + ).toBe(true); } finally { fs.rmSync(context.workDir, { recursive: true, force: true }); vi.restoreAllMocks(); @@ -404,7 +458,7 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { } }); - it("rejects overlapping workflow-job pages before creating a retry check", async () => { + it("closes the reserved retry check when workflow-job pages overlap", async () => { const context = setup(); const requests: RecordedGitHubRequest[] = []; const firstPage = Array.from({ length: 100 }, (_, index) => ({ @@ -424,7 +478,117 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow( /duplicate workflow job IDs/u, ); - expect(requests.some((request) => request.method === "POST")).toBe(false); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + expect( + requests.some( + (request) => + request.url.endsWith("/check-runs/18") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ), + ).toBe(true); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it.each([ + { + label: "job run ID mismatch", + options: () => ({ jobs: [{ ...hostedRunnerLossJob(), run_id: 24 }] }), + error: /job identity/u, + }, + { + label: "job run-attempt mismatch", + options: () => ({ jobs: [{ ...hostedRunnerLossJob(), run_attempt: 2 }] }), + error: /job identity/u, + }, + { + label: "job check URL mismatch", + options: () => ({ + jobs: [{ ...hostedRunnerLossJob(), check_run_url: "https://api.github.com/check-runs/1" }], + }), + error: /job identity/u, + }, + { + label: "check-run app mismatch", + options: () => ({ + jobCheck: { ...workflowJobCheckRun(hostedRunnerLossJob()), app: { id: 7 } }, + }), + error: /check run does not match/u, + }, + { + label: "check-run head mismatch", + options: () => ({ + jobCheck: { + ...workflowJobCheckRun(hostedRunnerLossJob()), + head_sha: "e".repeat(40), + }, + }), + error: /check run does not match/u, + }, + { + label: "incomplete annotation page", + options: () => { + const check = workflowJobCheckRun(hostedRunnerLossJob()); + return { + jobCheck: { ...check, output: { ...check.output, annotations_count: 2 } }, + annotationPages: [[runnerLossAnnotation()]], + }; + }, + error: /annotation listing is incomplete/u, + }, + { + label: "overlapping annotation pages", + options: () => { + const check = workflowJobCheckRun(hostedRunnerLossJob()); + const notices = Array.from({ length: 99 }, (_, index) => ({ + ...runnerLossAnnotation(), + start_line: index + 2, + end_line: index + 2, + annotation_level: "notice", + message: `notice ${index + 1}`, + })); + const firstPage = [runnerLossAnnotation(), ...notices]; + return { + jobCheck: { ...check, output: { ...check.output, annotations_count: 101 } }, + annotationPages: [firstPage, [notices[0]!]], + }; + }, + error: /duplicate workflow job annotations/u, + }, + { + label: "a second generic-cancellation failure annotation", + options: () => { + const check = workflowJobCheckRun(hostedRunnerLossJob()); + return { + jobCheck: { ...check, output: { ...check.output, annotations_count: 2 } }, + annotationPages: [ + [ + runnerLossAnnotation(), + { ...runnerLossAnnotation(), message: "The operation was canceled." }, + ], + ], + }; + }, + error: /not authorized/u, + }, + ])("fails closed after reservation for $label", async ({ options, error }) => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation(retryRoutes(requests, options())); + + try { + await expect(retryRunnerLossPrGate(context.command)).rejects.toThrow(error); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + expect( + requests.some( + (request) => + request.url.endsWith("/check-runs/18") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ), + ).toBe(true); } finally { fs.rmSync(context.workDir, { recursive: true, force: true }); } @@ -617,7 +781,18 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { githubFetchRoute( ({ url, method }) => url.includes("/actions/runs/24/attempts/1/jobs?") && method === "GET", - () => githubResponse({ total_count: 1, jobs: [hostedRunnerLossJob()] }), + () => githubResponse({ total_count: 1, jobs: [hostedRunnerLossJob(24)] }), + ), + githubFetchRoute( + ({ url, method }) => + url.endsWith(`/check-runs/${hostedRunnerLossJob().id}`) && method === "GET", + () => githubResponse(workflowJobCheckRun(hostedRunnerLossJob(24))), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/check-runs/${hostedRunnerLossJob().id}/annotations?`) && + method === "GET", + () => githubResponse([runnerLossAnnotation()]), ), githubFetchRoute( ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", @@ -691,6 +866,17 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { url.includes("/actions/runs/23/attempts/1/jobs?") && method === "GET", () => githubResponse({ total_count: 1, jobs: [hostedRunnerLossJob()] }), ), + githubFetchRoute( + ({ url, method }) => + url.endsWith(`/check-runs/${hostedRunnerLossJob().id}`) && method === "GET", + () => githubResponse(workflowJobCheckRun(hostedRunnerLossJob())), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/check-runs/${hostedRunnerLossJob().id}/annotations?`) && + method === "GET", + () => githubResponse([runnerLossAnnotation()]), + ), githubFetchRoute( ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", () => githubResponse(pullRequest()), diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 329043f82e8..96af78b1e5f 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -79,6 +79,9 @@ const MAX_PR_FILES = 3000; const MAX_COMPATIBILITY_FILES = 300; const MAX_ACTIVE_RUN_PAGES_PER_STATUS = 10; const MAX_WORKFLOW_JOB_PAGES = 10; +const MAX_JOB_ANNOTATION_PAGES = 10; +const HOSTED_RUNNER_LOST_COMMUNICATION_MESSAGE = + "The hosted runner lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error."; const MAX_REPORTED_WORKFLOW_JOBS = 10; const MAX_WAIVER_REASON_CHARS = 500; const MAX_APPROVAL_REVIEWS = 20; @@ -253,9 +256,28 @@ type WorkflowRun = { }; type WorkflowRunsResponse = { workflow_runs: WorkflowRun[] }; +type WorkflowJobAnnotation = { + path: string; + blobHref: string; + startLine: number; + startColumn: number | null; + endLine: number; + endColumn: number | null; + annotationLevel: string; + title: string; + message: string; + rawDetails: string; +}; type WorkflowJob = { id: number; name: string; + runId?: number; + runAttempt?: number; + headSha?: string; + runUrl?: string; + apiUrl?: string; + htmlUrl?: string; + checkRunUrl?: string; status?: string; conclusion: string | null; runnerId?: number | null; @@ -263,6 +285,7 @@ type WorkflowJob = { runnerGroupId?: number | null; runnerGroupName?: string | null; labels?: string[]; + annotations?: WorkflowJobAnnotation[]; steps: Array<{ name: string; status?: string; conclusion: string | null }>; }; type WorkflowJobsPage = { totalCount: number; jobs: WorkflowJob[] }; @@ -1634,6 +1657,16 @@ function validateWorkflowJob(value: unknown): WorkflowJob { (value.id as number) < 1 || typeof value.name !== "string" || value.name.length === 0 || + (value.run_id !== undefined && + (!Number.isSafeInteger(value.run_id) || (value.run_id as number) < 1)) || + (value.run_attempt !== undefined && + (!Number.isSafeInteger(value.run_attempt) || (value.run_attempt as number) < 1)) || + (value.head_sha !== undefined && + (typeof value.head_sha !== "string" || !SHA_PATTERN.test(value.head_sha))) || + (value.run_url !== undefined && typeof value.run_url !== "string") || + (value.url !== undefined && typeof value.url !== "string") || + (value.html_url !== undefined && typeof value.html_url !== "string") || + (value.check_run_url !== undefined && typeof value.check_run_url !== "string") || (value.status !== undefined && typeof value.status !== "string") || (value.conclusion !== null && typeof value.conclusion !== "string") || (value.runner_id !== undefined && @@ -1673,6 +1706,13 @@ function validateWorkflowJob(value: unknown): WorkflowJob { return { id: value.id as number, name: value.name, + ...(value.run_id === undefined ? {} : { runId: value.run_id as number }), + ...(value.run_attempt === undefined ? {} : { runAttempt: value.run_attempt as number }), + ...(value.head_sha === undefined ? {} : { headSha: value.head_sha }), + ...(value.run_url === undefined ? {} : { runUrl: value.run_url }), + ...(value.url === undefined ? {} : { apiUrl: value.url }), + ...(value.html_url === undefined ? {} : { htmlUrl: value.html_url }), + ...(value.check_run_url === undefined ? {} : { checkRunUrl: value.check_run_url }), ...(value.status === undefined ? {} : { status: value.status }), conclusion: value.conclusion, ...(value.runner_id === undefined ? {} : { runnerId: value.runner_id as number | null }), @@ -1686,6 +1726,120 @@ function validateWorkflowJob(value: unknown): WorkflowJob { }; } +function validateWorkflowJobAnnotation(value: unknown): WorkflowJobAnnotation { + if ( + !isObjectRecord(value) || + typeof value.path !== "string" || + value.path.length === 0 || + typeof value.blob_href !== "string" || + !Number.isSafeInteger(value.start_line) || + (value.start_line as number) < 1 || + (value.start_column !== null && + (!Number.isSafeInteger(value.start_column) || (value.start_column as number) < 1)) || + !Number.isSafeInteger(value.end_line) || + (value.end_line as number) < (value.start_line as number) || + (value.end_column !== null && + (!Number.isSafeInteger(value.end_column) || (value.end_column as number) < 1)) || + typeof value.annotation_level !== "string" || + typeof value.title !== "string" || + typeof value.message !== "string" || + typeof value.raw_details !== "string" + ) { + throw new Error("GitHub returned an invalid workflow job annotation"); + } + return { + path: value.path, + blobHref: value.blob_href, + startLine: value.start_line as number, + startColumn: value.start_column as number | null, + endLine: value.end_line as number, + endColumn: value.end_column as number | null, + annotationLevel: value.annotation_level, + title: value.title, + message: value.message, + rawDetails: value.raw_details, + }; +} + +async function listWorkflowJobAnnotations( + repository: string, + token: string, + job: WorkflowJob, + runId: number, + runAttempt: number, +): Promise { + const apiRepository = `https://api.github.com/repos/${repository}`; + const webRepository = `https://github.com/${repository}`; + const expectedRunUrl = `${apiRepository}/actions/runs/${runId}`; + const expectedJobUrl = `${apiRepository}/actions/jobs/${job.id}`; + const expectedCheckRunUrl = `${apiRepository}/check-runs/${job.id}`; + const expectedHtmlUrl = `${webRepository}/actions/runs/${runId}/job/${job.id}`; + if ( + !job.headSha || + job.runId !== runId || + job.runAttempt !== runAttempt || + job.runUrl !== expectedRunUrl || + job.apiUrl !== expectedJobUrl || + job.htmlUrl !== expectedHtmlUrl || + job.checkRunUrl !== expectedCheckRunUrl + ) { + throw new Error("workflow job identity does not match its exact run attempt"); + } + const check = await githubApi(`repos/${repository}/check-runs/${job.id}`, token, { + userAgent: USER_AGENT, + }); + const expectedAnnotationsUrl = `${expectedCheckRunUrl}/annotations`; + if ( + !isObjectRecord(check) || + check.id !== job.id || + check.name !== job.name || + check.head_sha !== job.headSha || + check.url !== expectedCheckRunUrl || + check.html_url !== expectedHtmlUrl || + check.details_url !== expectedHtmlUrl || + check.status !== "completed" || + check.conclusion !== "failure" || + !isObjectRecord(check.app) || + check.app.id !== GITHUB_ACTIONS_APP_ID || + !isObjectRecord(check.output) || + !Number.isSafeInteger(check.output.annotations_count) || + (check.output.annotations_count as number) < 0 || + check.output.annotations_url !== expectedAnnotationsUrl + ) { + throw new Error("workflow job check run does not match the exact failed job"); + } + const expectedCount = check.output.annotations_count as number; + const annotations: WorkflowJobAnnotation[] = []; + const fingerprints = new Set(); + for (let page = 1; page <= MAX_JOB_ANNOTATION_PAGES; page += 1) { + const value = await githubApi( + `repos/${repository}/check-runs/${job.id}/annotations?per_page=100&page=${page}`, + token, + { userAgent: USER_AGENT }, + ); + if (!Array.isArray(value) || value.length > 100) { + throw new Error("GitHub returned an invalid workflow job annotation listing"); + } + const pageAnnotations = value.map(validateWorkflowJobAnnotation); + for (const annotation of pageAnnotations) { + const fingerprint = JSON.stringify(annotation); + if (fingerprints.has(fingerprint)) { + throw new Error("GitHub returned duplicate workflow job annotations"); + } + fingerprints.add(fingerprint); + annotations.push(annotation); + } + if (annotations.length > expectedCount) { + throw new Error("workflow job annotation listing exceeds the trusted annotation count"); + } + if (annotations.length === expectedCount) return annotations; + if (value.length < 100) { + throw new Error("workflow job annotation listing is incomplete"); + } + } + throw new Error("workflow job annotation listing exceeded its page limit"); +} + function validateWorkflowJobsPage(value: unknown): WorkflowJobsPage { if ( !isObjectRecord(value) || @@ -1706,6 +1860,7 @@ async function listNonPassingWorkflowJobs( token: string, runId: number, runAttempt: number, + options: { includeAnnotations?: boolean } = {}, ): Promise<{ jobs: WorkflowJob[]; complete: boolean }> { if ( !Number.isSafeInteger(runId) || @@ -1738,10 +1893,22 @@ async function listNonPassingWorkflowJobs( } jobs.push(...response.jobs); if (jobs.length === totalCount) { + const nonPassingJobs = jobs.filter( + (job) => !["success", "skipped", "neutral"].includes(job.conclusion ?? ""), + ); + if (options.includeAnnotations) { + for (const job of nonPassingJobs.filter(hasTrustedHostedRunnerLossStepShape)) { + job.annotations = await listWorkflowJobAnnotations( + repository, + token, + job, + runId, + runAttempt, + ); + } + } return { - jobs: jobs.filter( - (job) => !["success", "skipped", "neutral"].includes(job.conclusion ?? ""), - ), + jobs: nonPassingJobs, complete: true, }; } @@ -1883,9 +2050,37 @@ const GITHUB_HOSTED_RUNNER_NAME_PATTERN = /^GitHub Actions [1-9][0-9]*$/u; * `in_progress`; current responses can terminalize it as `cancelled`, skip the * remaining cleanup, and append the synthetic successful `Complete job` step. * A user or concurrency cancellation concludes the job itself as `cancelled`, - * while an ordinary assertion records a failed step, so neither shape matches. + * while an ordinary assertion records a failed step. The step shape is still + * ambiguous with a job-level timeout, so a canonical GitHub runner-loss + * annotation bound to this workflow SHA is also required. */ -function hasTrustedHostedRunnerLossMarker(job: WorkflowJob): boolean { +function hasTrustedHostedRunnerLossAnnotation( + job: WorkflowJob, + repository: string, + workflowSha: string, +): boolean { + if (job.headSha !== workflowSha || !Array.isArray(job.annotations)) return false; + const blobPrefix = `https://github.com/${repository}/blob/${workflowSha}/`; + if ( + job.annotations.some((annotation) => annotation.blobHref !== `${blobPrefix}${annotation.path}`) + ) { + return false; + } + const failures = job.annotations.filter((annotation) => annotation.annotationLevel === "failure"); + return ( + failures.length === 1 && + failures[0]?.path === ".github" && + failures[0].startLine === 1 && + failures[0].startColumn === null && + failures[0].endLine === 1 && + failures[0].endColumn === null && + failures[0].title === "" && + failures[0].rawDetails === "" && + failures[0].message === HOSTED_RUNNER_LOST_COMMUNICATION_MESSAGE + ); +} + +function hasTrustedHostedRunnerLossStepShape(job: WorkflowJob): boolean { if ( job.status !== "completed" || job.conclusion !== "failure" || @@ -1948,7 +2143,20 @@ function hasTrustedHostedRunnerLossMarker(job: WorkflowJob): boolean { ); } +function hasTrustedHostedRunnerLossMarker( + job: WorkflowJob, + repository: string, + workflowSha: string, +): boolean { + return ( + hasTrustedHostedRunnerLossStepShape(job) && + hasTrustedHostedRunnerLossAnnotation(job, repository, workflowSha) + ); +} + export function verifiedRunnerLossEvidence(options: { + repository: string; + workflowSha: string; workflowConclusion: string | null; jobs: readonly WorkflowJob[]; jobDetailsAvailable: boolean; @@ -1962,10 +2170,10 @@ export function verifiedRunnerLossEvidence(options: { ) { return null; } - const runnerLostMarkerCount = options.jobs.filter(hasTrustedHostedRunnerLossMarker).length; - const otherNonPassingEvidencePresent = options.jobs.some( - (job) => !hasTrustedHostedRunnerLossMarker(job), - ); + const hasTrustedMarker = (job: WorkflowJob): boolean => + hasTrustedHostedRunnerLossMarker(job, options.repository, options.workflowSha); + const runnerLostMarkerCount = options.jobs.filter(hasTrustedMarker).length; + const otherNonPassingEvidencePresent = options.jobs.some((job) => !hasTrustedMarker(job)); return { terminalClassificationPresent: otherNonPassingEvidencePresent, jobConclusion: "failure", @@ -2691,25 +2899,6 @@ export async function retryRunnerLossPrGate( let retryCheckRunId: number | undefined; try { const state = readBoundPrGateState(command.statePath, command.stateHash); - const child = await githubApi( - `repos/${repository}/actions/runs/${command.childRunId}`, - token, - { userAgent: USER_AGENT }, - ); - assertCorrelatedWorkflowRun(child, { - childRunId: command.childRunId, - correlationId: state.correlationId, - prNumber: state.prNumber, - repository, - workflowSha: state.workflowSha, - }); - if ( - child.status !== "completed" || - !["failure", "cancelled"].includes(child.conclusion ?? "") - ) { - throw new Error("runner-loss retry requires a terminal failed or cancelled child run"); - } - const history = await matchingPrGateHistory({ repository, token, @@ -2731,35 +2920,6 @@ export async function retryRunnerLossPrGate( throw new Error("runner-loss retry was already consumed for this PR/base SHA pair"); } - const jobDetails = await listNonPassingWorkflowJobs(repository, token, command.childRunId, 1); - const runnerLossEvidence = verifiedRunnerLossEvidence({ - workflowConclusion: child.conclusion, - jobs: jobDetails.jobs, - jobDetailsAvailable: true, - jobDetailsComplete: jobDetails.complete, - }); - const retryDecision = runnerLossEvidence - ? decideRetry({ - runnerLoss: detectRunnerLoss(runnerLossEvidence), - classification: null, - attempt: 1, - }) - : { retry: false, reason: "runner-loss evidence is incomplete" }; - if (!retryDecision.retry) { - throw new Error(`runner-loss retry is not authorized: ${retryDecision.reason}`); - } - - const pull = await requireLiveExactDiff({ - repository, - token, - prNumber: state.prNumber, - headSha: state.commitSha, - baseSha: state.baseSha, - }); - if (pull.head.repo?.full_name !== repository) { - throw new Error("runner-loss retry requires an internal pull request"); - } - const historySize = history.length; const retryCheck = await createPrGateCheck({ repository, @@ -2793,6 +2953,58 @@ export async function retryRunnerLossPrGate( `Revalidating the exact PR/base SHA and risk plan after [attempt 1](${originalRunUrl}) lost its GitHub-hosted runner.`, ); + const child = await githubApi( + `repos/${repository}/actions/runs/${command.childRunId}`, + token, + { userAgent: USER_AGENT }, + ); + assertCorrelatedWorkflowRun(child, { + childRunId: command.childRunId, + correlationId: state.correlationId, + prNumber: state.prNumber, + repository, + workflowSha: state.workflowSha, + }); + if ( + child.status !== "completed" || + !["failure", "cancelled"].includes(child.conclusion ?? "") + ) { + throw new Error("runner-loss retry requires a terminal failed or cancelled child run"); + } + + const jobDetails = await listNonPassingWorkflowJobs(repository, token, command.childRunId, 1, { + includeAnnotations: true, + }); + const runnerLossEvidence = verifiedRunnerLossEvidence({ + repository, + workflowSha: state.workflowSha, + workflowConclusion: child.conclusion, + jobs: jobDetails.jobs, + jobDetailsAvailable: true, + jobDetailsComplete: jobDetails.complete, + }); + const retryDecision = runnerLossEvidence + ? decideRetry({ + runnerLoss: detectRunnerLoss(runnerLossEvidence), + classification: null, + attempt: 1, + }) + : { retry: false, reason: "runner-loss evidence is incomplete" }; + if (!retryDecision.retry) { + throw new Error(`runner-loss retry is not authorized: ${retryDecision.reason}`); + } + + const pull = await requireLiveExactDiff({ + repository, + token, + prNumber: state.prNumber, + headSha: state.commitSha, + baseSha: state.baseSha, + }); + if (pull.head.repo?.full_name !== repository) { + throw new Error("runner-loss retry requires an internal pull request"); + } + const currentPull = await requireLiveExactDiff({ repository, token, @@ -3492,7 +3704,9 @@ export async function finishPrGate(options: { let jobDetailsAvailable = true; let jobDetailsComplete = false; try { - const details = await listNonPassingWorkflowJobs(repository, token, options.childRunId, 1); + const details = await listNonPassingWorkflowJobs(repository, token, options.childRunId, 1, { + includeAnnotations: true, + }); jobs = details.jobs; jobDetailsComplete = details.complete; } catch (error) { @@ -3508,6 +3722,8 @@ export async function finishPrGate(options: { jobDetailsComplete, runnerLossAttempt, runnerLossEvidence: verifiedRunnerLossEvidence({ + repository, + workflowSha: state.workflowSha, workflowConclusion, jobs, jobDetailsAvailable, From 62fd99b4d4dd51f2708ee4bbce436f329d4aff48 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 00:16:58 -0700 Subject: [PATCH 08/20] fix(ci): terminalize interrupted runner retry Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-command.test.ts | 20 ++++ test/pr-e2e-gate-runner-loss-retry.test.ts | 114 ++++++++++++++++++ test/pr-e2e-required.test.ts | 45 ++++++++ tools/e2e/pr-e2e-gate.mts | 127 ++++++++++++++++++++- 4 files changed, 305 insertions(+), 1 deletion(-) diff --git a/test/pr-e2e-gate-command.test.ts b/test/pr-e2e-gate-command.test.ts index 8cbae3bc951..bed5752509b 100644 --- a/test/pr-e2e-gate-command.test.ts +++ b/test/pr-e2e-gate-command.test.ts @@ -250,6 +250,26 @@ describe("PR E2E controller commands", () => { }); }); + it("parses the narrowly scoped interrupted-retry cleanup", () => { + expect( + parseControllerCommand([ + "--mode", + "abandon-runner-loss-retry", + "--check-id", + "17", + "--run-id", + "23", + "--workflow-run-attempt", + "1", + ]), + ).toEqual({ + mode: "abandon-runner-loss-retry", + checkRunId: 17, + childRunId: 23, + workflowRunAttempt: 1, + }); + }); + it("rejects unknown controller path slots", () => { withPrivateWorkDir((workDir) => { expect(() => diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index 3b0e8870189..f0ad74ed906 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -8,6 +8,7 @@ import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { + abandonRunnerLossRetrySource, finishPrGate, type PrGateState, prGateExternalId, @@ -340,6 +341,21 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { } }); + it("rejects direct retry cleanup from a controller workflow rerun", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation(retryRoutes(requests)); + + try { + await expect(abandonRunnerLossRetrySource(17, 23, 2)).rejects.toThrow( + /first controller workflow run attempt/u, + ); + expect(requests).toHaveLength(0); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + it("dispatches the same plan once with fresh state and an independently bound check", async () => { const context = setup(); const requests: RecordedGitHubRequest[] = []; @@ -742,6 +758,104 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { } }); + it.each([ + { label: "before reservation", replacement: false }, + { label: "after a create response is lost", replacement: true }, + ])("terminalizes retry setup $label with older retryable history", async ({ replacement }) => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const older = checkRun(16, { + details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/22", + output: { + title: "PR prerequisite CI did not pass", + summary: "Prerequisite CI failed.\n\n", + }, + }); + const source = checkRun(17); + const reserved = checkRun(18, { + status: "in_progress", + conclusion: null, + details_url: null, + output: { + title: "Waiting for PR CI", + summary: + "This PR SHA and base SHA are reserved for deterministic E2E planning after CI completes.", + }, + }); + const history = replacement ? [older, source, reserved] : [older, source]; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "GET", + () => githubResponse(source), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => githubResponse({ total_count: history.length, check_runs: history }), + ), + githubFetchRoute( + ({ url, method }) => + url.endsWith(`/check-runs/${replacement ? 18 : 17}`) && method === "PATCH", + (request) => mutationResponse(request, replacement ? 18 : 17), + ), + ], + requests, + ), + ); + + try { + await expect(abandonRunnerLossRetrySource(17, 23, 1)).resolves.toBeUndefined(); + const completion = requests.find((request) => request.method === "PATCH"); + expect(completion?.url).toMatch(new RegExp(`/check-runs/${replacement ? 18 : 17}$`, "u")); + expect(completion?.body).toMatchObject({ + status: "completed", + conclusion: "failure", + output: { title: "Runner-loss retry could not start" }, + }); + expect(JSON.stringify(completion?.body)).not.toContain("nemoclaw-pr-e2e-retry:v1:"); + expect(fs.readFileSync(context.outputPath, "utf8")).toBe("finalized=true\n"); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + + it("rejects an ambiguous retry replacement without mutating either check", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const source = checkRun(17); + const ambiguous = checkRun(18, { + status: "in_progress", + conclusion: null, + details_url: null, + output: { title: "Unexpected owner", summary: "Not a canonical reservation." }, + }); + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "GET", + () => githubResponse(source), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => githubResponse({ total_count: 2, check_runs: [source, ambiguous] }), + ), + ], + requests, + ), + ); + + try { + await expect(abandonRunnerLossRetrySource(17, 23, 1)).rejects.toThrow(/ambiguous/u); + expect(requests.some((request) => request.method === "PATCH")).toBe(false); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + it.each([ { label: "loses another hosted runner", conclusion: "failure", evidenceOutcome: "skipped" }, { label: "cannot download evidence", conclusion: "success", evidenceOutcome: "failure" }, diff --git a/test/pr-e2e-required.test.ts b/test/pr-e2e-required.test.ts index 76f82183976..c36b9f0de71 100644 --- a/test/pr-e2e-required.test.ts +++ b/test/pr-e2e-required.test.ts @@ -217,6 +217,51 @@ describe("native PR E2E required job", () => { await expect(findCoordinationCheck(identity)).resolves.toMatchObject({ id: 18 }); }); + it.each([ + { label: "the source marker is removed before reservation", replacement: false }, + { label: "the reserved replacement is closed after create response loss", replacement: true }, + ])("observes a terminal retry-controller failure when $label", async ({ replacement }) => { + const older = check(undefined, { + id: 16, + conclusion: "failure", + output: { + title: "PR prerequisite CI did not pass", + summary: "Prerequisite CI failed.\n\n", + }, + }); + const source = check(undefined, { + id: 17, + conclusion: "failure", + output: { + title: replacement ? "Selected E2E did not pass" : "Runner-loss retry could not start", + summary: replacement + ? "Runner disappeared.\n\n" + : "Runner disappeared. The automatic retry controller could not start.", + }, + }); + const replacementCheck = check(undefined, { + id: 18, + conclusion: "failure", + output: { + title: "Runner-loss retry could not start", + summary: "The reserved replacement was terminalized without a retry marker.", + }, + }); + const checks = replacement ? [older, source, replacementCheck] : [older, source]; + vi.spyOn(globalThis, "fetch").mockResolvedValue(githubResponse(listing(checks))); + + const current = await findCoordinationCheck(identity); + expect(classifyCoordinationCheck(current, identity.repository)).toEqual({ + state: "complete", + result: { + conclusion: "failure", + title: "Runner-loss retry could not start", + detailsUrl: "https://github.com/NVIDIA/NemoClaw/actions/runs/99", + logUrls: ["https://github.com/NVIDIA/NemoClaw/actions/runs/99"], + }, + }); + }); + it.each([ { label: "an older unmarked terminal check", diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 96af78b1e5f..301052bd99a 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -61,6 +61,8 @@ const NEVER_RETRY_FAILURE_TITLES = new Set([ ]); const CHECK_EXTERNAL_ID_PREFIX = "nemoclaw-pr-e2e:v2"; const LEGACY_CHECK_EXTERNAL_ID_PREFIX = "nemoclaw-pr-e2e:v1"; +const CHECK_EXTERNAL_ID_PATTERN = + /^nemoclaw-pr-e2e:v2:([1-9][0-9]*):([0-9a-f]{40}):([0-9a-f]{40})$/u; const GITHUB_ACTIONS_APP_ID = 15368; const USER_AGENT = "nemoclaw-pr-e2e-gate"; const SHA_PATTERN = /^[a-f0-9]{40}$/u; @@ -205,6 +207,12 @@ export type ControllerCommand = evidenceOutcome: EvidenceStepOutcome; } & ControllerPaths) | { mode: "abandon"; checkRunId: number; childRunId?: number } + | { + mode: "abandon-runner-loss-retry"; + checkRunId: number; + childRunId: number; + workflowRunAttempt: number; + } | { mode: "cancel"; prNumber: number; @@ -549,6 +557,21 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { childRunId: args.runId ? parsePositiveId(args.runId, "--run-id") : undefined, }; } + if (args.mode === "abandon-runner-loss-retry") { + const workflowRunAttempt = parsePositiveId( + requiredArgument(args.workflowRunAttempt, "workflow-run-attempt"), + "--workflow-run-attempt", + ); + if (workflowRunAttempt !== 1) { + throw new Error("--workflow-run-attempt must be exactly 1"); + } + return { + mode: "abandon-runner-loss-retry", + checkRunId: parsePositiveId(requiredArgument(args.checkId, "check-id"), "--check-id"), + childRunId: parsePositiveId(requiredArgument(args.runId, "run-id"), "--run-id"), + workflowRunAttempt, + }; + } if (args.mode === "cancel") { if ((args.head === undefined) !== (args.supersededHead === undefined)) { throw new Error("--head and --superseded-head must be provided together"); @@ -694,7 +717,7 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { }; } throw new Error( - "--mode must be seed, start, start-control-plane, start-approved-control-plane, finish, abandon, cancel, wait, download, retry-runner-loss, record-fork-e2e-skip, or record-approved-fork-e2e-skip", + "--mode must be seed, start, start-control-plane, start-approved-control-plane, finish, abandon, abandon-runner-loss-retry, cancel, wait, download, retry-runner-loss, record-fork-e2e-skip, or record-approved-fork-e2e-skip", ); } @@ -3801,6 +3824,100 @@ export async function abandonPrGate(checkRunId: number, childRunId?: number): Pr if (cancellationError) throw cancellationError; } +export async function abandonRunnerLossRetrySource( + checkRunId: number, + childRunId: number, + workflowRunAttempt: number, +): Promise { + if (workflowRunAttempt !== 1) { + throw new Error("runner-loss retry cleanup must use the first controller workflow run attempt"); + } + if (!Number.isSafeInteger(checkRunId) || checkRunId < 1) { + throw new Error("runner-loss retry source check ID is invalid"); + } + if (!Number.isSafeInteger(childRunId) || childRunId < 1) { + throw new Error("runner-loss retry source run ID is invalid"); + } + const { token, repository } = tokenAndRepository(); + const childRunUrl = `https://github.com/${repository}/actions/runs/${childRunId}`; + const value = await githubApi(`repos/${repository}/check-runs/${checkRunId}`, token, { + userAgent: USER_AGENT, + }); + if (!isObjectRecord(value)) { + throw new Error("GitHub returned an invalid runner-loss retry source check"); + } + const source = value as CheckRun; + const externalIdMatch = + typeof source.external_id === "string" + ? CHECK_EXTERNAL_ID_PATTERN.exec(source.external_id) + : null; + if ( + source.id !== checkRunId || + source.name !== CHECK_NAME || + source.app?.id !== GITHUB_ACTIONS_APP_ID || + source.status !== "completed" || + source.conclusion !== "failure" || + source.details_url !== childRunUrl || + retryableFailureReason(source) !== "child-cancelled" || + !externalIdMatch + ) { + throw new Error("completed check does not match the exact runner-loss retry source"); + } + + const [, prNumberText, headSha, baseSha] = externalIdMatch; + const history = await matchingPrGateHistory({ + repository, + token, + prNumber: parsePositiveId(prNumberText!, "runner-loss retry source PR number"), + headSha: headSha!, + baseSha: baseSha!, + }); + const sourceIndex = history.findIndex((check) => check.id === checkRunId); + const current = history.at(-1); + if (sourceIndex < 0) { + throw new Error("runner-loss retry source is absent from its exact check history"); + } + if (current?.id !== checkRunId) { + const sourceImmediatelyPrecedesCurrent = sourceIndex === history.length - 2; + const canonicalReservedReplacement = + current?.status === "in_progress" && + current.conclusion === null && + (current.details_url === null || current.details_url === undefined) && + current.output?.title === RESERVED_CHECK_TITLE && + current.output.summary === RESERVED_CHECK_SUMMARY; + if (!sourceImmediatelyPrecedesCurrent || !canonicalReservedReplacement) { + throw new Error("runner-loss retry source has an ambiguous replacement check history"); + } + await completeCheck( + { repository, checkRunId: current.id }, + token, + { + conclusion: "failure", + title: "Runner-loss retry could not start", + summary: `The one-time automatic retry controller stopped after reserving this replacement check. The original runner-loss evidence remains linked at [attempt 1](${childRunUrl}); inspect the controller job before retrying the gate.`, + }, + childRunUrl, + ); + appendOutput("finalized", "true"); + return; + } + + const markerBoundary = `\n\n${retryableFailureMarker("child-cancelled")}`; + const sourceSummary = source.output!.summary!; + const evidenceSummary = sourceSummary.slice(0, -markerBoundary.length); + await completeCheck( + { repository, checkRunId }, + token, + { + conclusion: "failure", + title: "Runner-loss retry could not start", + summary: `${evidenceSummary}\n\nThe one-time automatic retry controller stopped before it reserved a replacement check. The original runner-loss run remains linked; inspect the controller job before retrying the gate.`, + }, + childRunUrl, + ); + appendOutput("finalized", "true"); +} + function validateApprovalWorkflowRun( value: unknown, options: { @@ -4260,6 +4377,14 @@ async function main(): Promise { await abandonPrGate(command.checkRunId, command.childRunId); return; } + if (command.mode === "abandon-runner-loss-retry") { + await abandonRunnerLossRetrySource( + command.checkRunId, + command.childRunId, + command.workflowRunAttempt, + ); + return; + } if (command.mode === "wait") { await waitForChildRun(command.childRunId); return; From 57fe2903140f6a7ad870df553ce85f18ddef69b7 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 00:18:03 -0700 Subject: [PATCH 09/20] ci(e2e): wire one hosted runner retry Signed-off-by: Apurv Kumaria --- .github/workflows/pr-e2e-gate.yaml | 180 ++++++++++++++++++++++++++--- test/e2e/README.md | 54 ++++++--- test/pr-e2e-gate-workflow.test.ts | 86 ++++++++++++-- tools/e2e/pr-e2e-gate.mts | 2 +- tools/e2e/pr-e2e-required.mts | 2 +- 5 files changed, 287 insertions(+), 37 deletions(-) diff --git a/.github/workflows/pr-e2e-gate.yaml b/.github/workflows/pr-e2e-gate.yaml index 262afabac19..bd825068fb0 100644 --- a/.github/workflows/pr-e2e-gate.yaml +++ b/.github/workflows/pr-e2e-gate.yaml @@ -53,7 +53,7 @@ permissions: {} jobs: initialize: - if: ${{ github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.action != 'closed' && (github.event.action != 'edited' || github.event.changes.base != null) }} + if: ${{ github.run_attempt == 1 && github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.action != 'closed' && (github.event.action != 'edited' || github.event.changes.base != null) }} runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -61,7 +61,7 @@ jobs: contents: read pull-requests: read concurrency: - group: pr-e2e-gate-${{ github.event.pull_request.head.repo.full_name }}-${{ github.event.pull_request.head.ref }} + group: pr-e2e-gate-${{ github.repository }}-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }}-${{ github.event.pull_request.base.sha }} cancel-in-progress: false steps: - name: Checkout controller @@ -95,7 +95,7 @@ jobs: name: E2E / PR Gate if: ${{ github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.action != 'closed' && (github.event.action != 'edited' || github.event.changes.base != null) }} runs-on: ubuntu-latest - timeout-minutes: 170 + timeout-minutes: 360 permissions: checks: read contents: read @@ -126,10 +126,10 @@ jobs: --pr "$PR_NUMBER" --head "$HEAD_SHA" --base "$BASE_SHA" - --timeout-seconds 9900 + --timeout-seconds 21480 cancel-superseded: - if: ${{ github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.pull_request.head.repo.full_name == github.repository && (github.event.action != 'edited' || github.event.changes.base != null) }} + if: ${{ github.run_attempt == 1 && github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.pull_request.head.repo.full_name == github.repository && (github.event.action != 'edited' || github.event.changes.base != null) }} runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -166,9 +166,9 @@ jobs: --superseded-head "$SUPERSEDED_HEAD_SHA" coordinate: - if: ${{ github.repository == 'NVIDIA/NemoClaw' && ((github.event_name == 'workflow_run' && github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.path == '.github/workflows/pr.yaml' && endsWith(github.event.workflow_run.display_title, ' gate true')) || (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main' && inputs.operation == 'run-control-plane' && github.run_attempt == 1)) }} + if: ${{ github.run_attempt == 1 && github.repository == 'NVIDIA/NemoClaw' && ((github.event_name == 'workflow_run' && github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.path == '.github/workflows/pr.yaml' && endsWith(github.event.workflow_run.display_title, ' gate true')) || (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main' && inputs.operation == 'run-control-plane')) }} runs-on: ubuntu-latest - timeout-minutes: 180 + timeout-minutes: 330 outputs: control_plane_approval_mode: ${{ steps.start.outputs.control_plane_approval_mode }} control_plane_approval_pr_number: ${{ steps.start.outputs.control_plane_approval_pr_number }} @@ -184,7 +184,7 @@ jobs: contents: read pull-requests: read concurrency: - group: pr-e2e-gate-${{ github.repository }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || inputs.expected_head_sha }} + group: pr-e2e-gate-${{ github.repository }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number || inputs.pr_number }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || inputs.expected_head_sha }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].base.sha || inputs.expected_base_sha }} # Let the previous coordinator observe E2E cancellation and close its check. cancel-in-progress: false steps: @@ -310,6 +310,81 @@ jobs: --run-id "${{ steps.start.outputs.run_id }}" --evidence-outcome "${{ steps.evidence.outcome }}" + - id: retry + name: Retry after hosted runner loss + if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && github.run_attempt == 1 }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode retry-runner-loss + --work-dir "${{ steps.workspace.outputs.work_dir }}" + --state-hash "${{ steps.start.outputs.state_hash }}" + --check-id "${{ steps.start.outputs.check_id }}" + --run-id "${{ steps.start.outputs.run_id }}" + --workflow-run-attempt "${{ github.run_attempt }}" + + - id: retry_wait + name: Wait for retry E2E run + if: ${{ steps.retry.outputs.dispatched == 'true' }} + continue-on-error: true + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode wait + --run-id "${{ steps.retry.outputs.run_id }}" + + - id: retry_evidence + name: Download retry evidence + if: ${{ always() && steps.retry.outputs.dispatched == 'true' }} + continue-on-error: true + env: + GH_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode download + --slot runner-loss-retry + --work-dir "${{ steps.workspace.outputs.work_dir }}" + --run-id "${{ steps.retry.outputs.run_id }}" + + - id: retry_finish + name: Verify retry evidence + if: ${{ always() && steps.retry.outputs.dispatched == 'true' }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode finish + --slot runner-loss-retry + --work-dir "${{ steps.workspace.outputs.work_dir }}" + --state-hash "${{ steps.retry.outputs.state_hash }}" + --check-id "${{ steps.retry.outputs.check_id }}" + --run-id "${{ steps.retry.outputs.run_id }}" + --evidence-outcome "${{ steps.retry_evidence.outcome }}" + + - name: Close incomplete retry check + if: ${{ always() && steps.retry.outputs.check_id != '' && steps.retry.outputs.finalized != 'true' && steps.retry_finish.outputs.finalized != 'true' }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode abandon + --check-id "${{ steps.retry.outputs.check_id }}" + --run-id "${{ steps.retry.outputs.run_id }}" + + - name: Terminalize interrupted retry setup + if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && steps.retry.outcome == 'failure' && steps.retry.outputs.check_id == '' }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode abandon-runner-loss-retry + --check-id "${{ steps.start.outputs.check_id }}" + --run-id "${{ steps.start.outputs.run_id }}" + --workflow-run-attempt "${{ github.run_attempt }}" + - name: Close incomplete check if: ${{ always() && steps.start.outputs.check_id != '' && steps.start.outputs.finalized != 'true' && steps.finish.outputs.finalized != 'true' }} env: @@ -329,7 +404,7 @@ jobs: needs: coordinate if: ${{ needs.coordinate.result == 'success' && needs.coordinate.outputs.control_plane_approval_mode != '' && github.run_attempt == 1 }} runs-on: ubuntu-latest - timeout-minutes: 180 + timeout-minutes: 330 environment: name: approve-credentialed-e2e-for-internal-pr deployment: false @@ -339,8 +414,8 @@ jobs: contents: read pull-requests: read concurrency: - group: pr-e2e-gate-approve-internal-${{ needs.coordinate.outputs.control_plane_approval_pr_number }} - cancel-in-progress: true + group: pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.control_plane_approval_pr_number }}-${{ needs.coordinate.outputs.control_plane_approval_head_sha }}-${{ needs.coordinate.outputs.control_plane_approval_base_sha }} + cancel-in-progress: false steps: - name: Checkout controller uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -438,6 +513,81 @@ jobs: --run-id "${{ steps.start.outputs.run_id }}" --evidence-outcome "${{ steps.evidence.outcome }}" + - id: retry + name: Retry approved E2E after hosted runner loss + if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && github.run_attempt == 1 }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode retry-runner-loss + --work-dir "${{ steps.workspace.outputs.work_dir }}" + --state-hash "${{ steps.start.outputs.state_hash }}" + --check-id "${{ steps.start.outputs.check_id }}" + --run-id "${{ steps.start.outputs.run_id }}" + --workflow-run-attempt "${{ github.run_attempt }}" + + - id: retry_wait + name: Wait for approved retry E2E run + if: ${{ steps.retry.outputs.dispatched == 'true' }} + continue-on-error: true + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode wait + --run-id "${{ steps.retry.outputs.run_id }}" + + - id: retry_evidence + name: Download approved retry evidence + if: ${{ always() && steps.retry.outputs.dispatched == 'true' }} + continue-on-error: true + env: + GH_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode download + --slot runner-loss-retry + --work-dir "${{ steps.workspace.outputs.work_dir }}" + --run-id "${{ steps.retry.outputs.run_id }}" + + - id: retry_finish + name: Verify approved retry evidence + if: ${{ always() && steps.retry.outputs.dispatched == 'true' }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode finish + --slot runner-loss-retry + --work-dir "${{ steps.workspace.outputs.work_dir }}" + --state-hash "${{ steps.retry.outputs.state_hash }}" + --check-id "${{ steps.retry.outputs.check_id }}" + --run-id "${{ steps.retry.outputs.run_id }}" + --evidence-outcome "${{ steps.retry_evidence.outcome }}" + + - name: Close incomplete approved retry check + if: ${{ always() && steps.retry.outputs.check_id != '' && steps.retry.outputs.finalized != 'true' && steps.retry_finish.outputs.finalized != 'true' }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode abandon + --check-id "${{ steps.retry.outputs.check_id }}" + --run-id "${{ steps.retry.outputs.run_id }}" + + - name: Terminalize interrupted approved retry setup + if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && steps.retry.outcome == 'failure' && steps.retry.outputs.check_id == '' }} + env: + GITHUB_TOKEN: ${{ github.token }} + run: >- + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts + --mode abandon-runner-loss-retry + --check-id "${{ steps.start.outputs.check_id }}" + --run-id "${{ steps.start.outputs.run_id }}" + --workflow-run-attempt "${{ github.run_attempt }}" + - name: Close incomplete approved check if: ${{ always() && steps.start.outputs.check_id != '' && steps.start.outputs.finalized != 'true' && steps.finish.outputs.finalized != 'true' }} env: @@ -467,8 +617,8 @@ jobs: contents: read pull-requests: read concurrency: - group: pr-e2e-gate-approve-fork-skip-${{ needs.coordinate.outputs.fork_skip_pr_number }} - cancel-in-progress: true + group: pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.fork_skip_pr_number }}-${{ needs.coordinate.outputs.fork_skip_head_sha }}-${{ needs.coordinate.outputs.fork_skip_base_sha }} + cancel-in-progress: false steps: - name: Checkout controller uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -505,7 +655,7 @@ jobs: record-fork-e2e-skip: name: Record credentialed E2E skip for fork PR - if: ${{ github.event_name == 'workflow_dispatch' && github.repository == 'NVIDIA/NemoClaw' && github.ref == 'refs/heads/main' && inputs.operation == 'approve-fork-e2e-skip' }} + if: ${{ github.run_attempt == 1 && github.event_name == 'workflow_dispatch' && github.repository == 'NVIDIA/NemoClaw' && github.ref == 'refs/heads/main' && inputs.operation == 'approve-fork-e2e-skip' }} runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -513,7 +663,7 @@ jobs: contents: read pull-requests: read concurrency: - group: pr-e2e-gate-record-fork-skip-${{ inputs.pr_number }} + group: pr-e2e-gate-${{ github.repository }}-${{ inputs.pr_number }}-${{ inputs.expected_head_sha }}-${{ inputs.expected_base_sha }} cancel-in-progress: false steps: - name: Checkout controller diff --git a/test/e2e/README.md b/test/e2e/README.md index 97210e223a1..9d7662790ad 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -158,11 +158,12 @@ failed or cancelled and the native job is non-passing. Only a successful native `E2E / PR Gate` for the current head and base satisfies the required check. An eligible prerequisite-CI failure records the versioned retry reason `prerequisite-ci`. A selected child records `child-cancelled` only when a -trusted hosted-runner-loss marker is present and no terminal classification -was produced; cancellation alone is not retryable. Assertion failures and -other selected-E2E outcomes do not receive a retry reason. An unexpected -controller error still fails the controller workflow and fails coordination -closed, which prevents the native job from passing. +trusted GitHub-hosted runner-loss annotation is bound to the exact failed job +and workflow commit and no other terminal classification was produced. +Cancellation alone is not retryable. Assertion failures and other selected-E2E +outcomes do not receive a retry reason. An unexpected controller error still +fails the controller workflow and fails coordination closed, which prevents +the native job from passing. On open, synchronization, reopen, transition out of draft, or base retarget, `.github/workflows/pr-e2e-gate.yaml` reserves `E2E / PR Gate Coordination` for @@ -411,14 +412,41 @@ fails. A failed coordination result links the selected E2E run and up to 10 non-passing jobs, including up to three failed step names per job. If GitHub truncates the job listing or the controller cannot load it, the coordination check directs the maintainer to the complete run. -The coordinator has a 180-minute job budget and gives the selected E2E run 105 -minutes to finish. When that limit expires, finalization cancels the child and -records the non-passing result in the coordination check. The native observer -has a 170-minute job budget and waits up to 165 minutes for a trusted terminal -verdict. Evidence download has its own 10-minute limit. If the selected child -succeeds but the `Download evidence` step fails, is cancelled, or is skipped, -the controller cannot authenticate the child's artifacts. It fails -coordination closed as +The coordinator has a 330-minute job budget and gives each selected E2E run 140 +minutes to finish. A first-attempt controller may dispatch one replacement run +when the first child fails because a standard GitHub-hosted `ubuntu-latest` +runner lost communication. The Jobs API response must identify the exact run, +attempt, workflow commit, job check, standard hosted runner group, and runner +name. Its exact check-run annotation must contain one canonical runner-loss +failure bound to `.github` at that workflow commit. Generic cancellation, +timeout, unknown runner identity, self-hosted or custom runner groups, an +ordinary failed step, another non-passing job, incomplete pagination, or +mismatched annotation identity all fail closed without a retry. + +Before the one-time retry dispatch, the controller revalidates the unchanged +internal PR head and base, original child, current coordination-check lineage, +trusted runner-loss evidence, and deterministic plan. It reserves a distinct +replacement coordination check before dispatch so the native observer can +follow the retry without mutating completed attempt-one history. Attempt two +uses separate private state and evidence paths. Its result is terminal: it can +never authorize another automatic retry. If the retry controller stops before +or after reservation, its always-run cleanup removes the retry authorization +from the source or closes the reserved replacement so no retryable or active +check is left behind. + +Each evidence download has its own 10-minute limit and 30-second process-kill +grace. Two 140-minute waits plus both download windows consume 301 minutes, +leaving 29 minutes of the coordinator budget for validation, dispatch, and +finalization. The native required observer waits up to 358 minutes inside a +360-minute job. That is 13 minutes longer than the 15-minute prerequisite-CI +budget plus the 330-minute controller budget, so it can observe the retry's +terminal result without racing the controller timeout. When a child wait +expires, finalization cancels that child and records the non-passing result in +the coordination check. + +If the selected child succeeds but the `Download evidence` step fails, is +cancelled, or is skipped, the controller cannot authenticate the child's +artifacts. It fails coordination closed as `Evidence could not be verified` and leaves `E2E / PR Gate Controller` red so maintainers inspect that infrastructure failure. This download-only outcome records `evidence-download`, so a later successful eligible PR CI run can create diff --git a/test/pr-e2e-gate-workflow.test.ts b/test/pr-e2e-gate-workflow.test.ts index d0f0b3a487a..ffff84f6a56 100644 --- a/test/pr-e2e-gate-workflow.test.ts +++ b/test/pr-e2e-gate-workflow.test.ts @@ -307,6 +307,14 @@ describe("PR E2E gate workflow", () => { const approveInternal = workflow.jobs["approve-internal-e2e"]; const approveForkSkip = workflow.jobs["approve-fork-e2e-skip"]; const recordForkSkip = workflow.jobs["record-fork-e2e-skip"]; + const longestSelectedE2eMinutes = 130; + const controllerWaitMinutes = 140; + const evidenceAndKillGraceMinutes = 10.5; + const twoAttemptMinimum = 2 * (controllerWaitMinutes + evidenceAndKillGraceMinutes); + const controllerSetupReserveMinutes = 25; + const maxPrerequisiteCiMinutes = 15; + const observerApiSlackMinutes = 13; + const observerPollMinutes = 21_480 / 60; expect(workflow.name).toBe("E2E / PR Gate Controller"); expect(workflow["run-name"]).toContain("E2E Gate PR #{0} head {1} base {2} gate {3}"); @@ -381,6 +389,7 @@ describe("PR E2E gate workflow", () => { expect(metadataOnlyGate.status, metadataOnlyGate.stderr).toBe(0); expect(metadataOnlyGate.stdout).toContain("Metadata-only PR edit"); expect(initialize.if).toContain("github.event_name == 'pull_request_target'"); + expect(initialize.if).toContain("github.run_attempt == 1"); expect(initialize.if).toContain("github.event.action != 'closed'"); expect(initialize.if).toContain("github.event.action != 'edited'"); expect(initialize.if).toContain("github.event.changes.base != null"); @@ -390,8 +399,9 @@ describe("PR E2E gate workflow", () => { "pull-requests": "read", }); expect(initialize.concurrency?.group).toBe( - "pr-e2e-gate-${{ github.event.pull_request.head.repo.full_name }}-${{ github.event.pull_request.head.ref }}", + "pr-e2e-gate-${{ github.repository }}-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }}-${{ github.event.pull_request.base.sha }}", ); + expect(initialize.concurrency?.["cancel-in-progress"]).toBe(false); expect(required.name).toBe("E2E / PR Gate"); expect(required.if).toContain("github.event_name == 'pull_request_target'"); expect(required.if).toContain("github.event.action != 'closed'"); @@ -406,7 +416,7 @@ describe("PR E2E gate workflow", () => { group: "pr-e2e-required-${{ github.event.pull_request.number }}", "cancel-in-progress": true, }); - expect(required["timeout-minutes"]).toBe(170); + expect(required["timeout-minutes"]).toBe(360); expect(required.secrets).toBeUndefined(); expect(step(required, "Checkout observer").with).toEqual({ ref: "${{ github.workflow_sha }}", @@ -422,7 +432,9 @@ describe("PR E2E gate workflow", () => { expect(observer.run).toContain("tools/e2e/pr-e2e-required.mts"); expect(observer.run).toContain('--head "$HEAD_SHA"'); expect(observer.run).toContain('--base "$BASE_SHA"'); + expect(observer.run).toContain("--timeout-seconds 21480"); expect(cancel.if).toContain("github.event_name == 'pull_request_target'"); + expect(cancel.if).toContain("github.run_attempt == 1"); expect(cancel.if).toContain( "github.event.pull_request.head.repo.full_name == github.repository", ); @@ -453,8 +465,10 @@ describe("PR E2E gate workflow", () => { "pull-requests": "read", }); expect(coordinate.concurrency?.group).toBe( - "pr-e2e-gate-${{ github.repository }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || inputs.expected_head_sha }}", + "pr-e2e-gate-${{ github.repository }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number || inputs.pr_number }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || inputs.expected_head_sha }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].base.sha || inputs.expected_base_sha }}", ); + expect(coordinate.concurrency?.["cancel-in-progress"]).toBe(false); + expect(coordinate["timeout-minutes"]).toBe(330); expect(coordinate.outputs).toEqual({ control_plane_approval_mode: "${{ steps.start.outputs.control_plane_approval_mode }}", control_plane_approval_pr_number: @@ -483,9 +497,21 @@ describe("PR E2E gate workflow", () => { }); expect(approveInternal.concurrency).toEqual({ group: - "pr-e2e-gate-approve-internal-${{ needs.coordinate.outputs.control_plane_approval_pr_number }}", - "cancel-in-progress": true, + "pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.control_plane_approval_pr_number }}-${{ needs.coordinate.outputs.control_plane_approval_head_sha }}-${{ needs.coordinate.outputs.control_plane_approval_base_sha }}", + "cancel-in-progress": false, }); + expect(approveInternal["timeout-minutes"]).toBe(330); + expect(controllerWaitMinutes).toBeGreaterThan(longestSelectedE2eMinutes); + expect(coordinate["timeout-minutes"]).toBeGreaterThanOrEqual( + twoAttemptMinimum + controllerSetupReserveMinutes, + ); + expect(approveInternal["timeout-minutes"]).toBeGreaterThanOrEqual( + twoAttemptMinimum + controllerSetupReserveMinutes, + ); + expect(observerPollMinutes).toBeGreaterThanOrEqual( + coordinate["timeout-minutes"]! + maxPrerequisiteCiMinutes + observerApiSlackMinutes, + ); + expect(required["timeout-minutes"]).toBeGreaterThan(observerPollMinutes); expect(approveInternal.secrets).toBeUndefined(); expect(approveForkSkip.name).toBe("Approve credentialed E2E skip for fork PR"); expect(approveForkSkip.needs).toBe("coordinate"); @@ -503,11 +529,13 @@ describe("PR E2E gate workflow", () => { "pull-requests": "read", }); expect(approveForkSkip.concurrency).toEqual({ - group: "pr-e2e-gate-approve-fork-skip-${{ needs.coordinate.outputs.fork_skip_pr_number }}", - "cancel-in-progress": true, + group: + "pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.fork_skip_pr_number }}-${{ needs.coordinate.outputs.fork_skip_head_sha }}-${{ needs.coordinate.outputs.fork_skip_base_sha }}", + "cancel-in-progress": false, }); expect(approveForkSkip.secrets).toBeUndefined(); expect(recordForkSkip.if).toContain("github.event_name == 'workflow_dispatch'"); + expect(recordForkSkip.if).toContain("github.run_attempt == 1"); expect(recordForkSkip.if).toContain("github.ref == 'refs/heads/main'"); expect(recordForkSkip.name).toBe("Record credentialed E2E skip for fork PR"); expect(recordForkSkip.if).toContain("inputs.operation == 'approve-fork-e2e-skip'"); @@ -516,6 +544,11 @@ describe("PR E2E gate workflow", () => { contents: "read", "pull-requests": "read", }); + expect(recordForkSkip.concurrency).toEqual({ + group: + "pr-e2e-gate-${{ github.repository }}-${{ inputs.pr_number }}-${{ inputs.expected_head_sha }}-${{ inputs.expected_base_sha }}", + "cancel-in-progress": false, + }); expect(collectStrings(initialize).some((value) => value.includes("--mode seed"))).toBe(true); expect( collectStrings(recordForkSkip).some((value) => value.includes("--mode record-fork-e2e-skip")), @@ -563,6 +596,22 @@ describe("PR E2E gate workflow", () => { expect(evidence.run).toContain('--run-id "${{ steps.start.outputs.run_id }}"'); const finish = step(coordinate, "Verify evidence"); expect(finish.run).toContain('--evidence-outcome "${{ steps.evidence.outcome }}"'); + const retry = step(coordinate, "Retry after hosted runner loss"); + expect(retry.if).toContain("steps.finish.outputs.runner_loss_retry_authorized == 'true'"); + expect(retry.if).toContain("github.run_attempt == 1"); + expect(retry.run).toContain("--mode retry-runner-loss"); + expect(retry.run).toContain('--workflow-run-attempt "${{ github.run_attempt }}"'); + const retryEvidence = step(coordinate, "Download retry evidence"); + expect(retryEvidence.if).toContain("always()"); + expect(retryEvidence.run).toContain("--slot runner-loss-retry"); + const retryFinish = step(coordinate, "Verify retry evidence"); + expect(retryFinish.if).toContain("always()"); + expect(retryFinish.run).toContain('--state-hash "${{ steps.retry.outputs.state_hash }}"'); + expect(retryFinish.run).toContain('--evidence-outcome "${{ steps.retry_evidence.outcome }}"'); + const interruptedRetry = step(coordinate, "Terminalize interrupted retry setup"); + expect(interruptedRetry.if).toContain("steps.retry.outcome == 'failure'"); + expect(interruptedRetry.if).toContain("steps.retry.outputs.check_id == ''"); + expect(interruptedRetry.run).toContain("--mode abandon-runner-loss-retry"); const approval = step(approveForkSkip, "Record approved credentialed E2E skip"); expect(approval.env).toEqual({ APPROVAL_RUN_ATTEMPT: "${{ github.run_attempt }}", @@ -762,6 +811,12 @@ describe("PR E2E gate workflow", () => { "Wait for E2E run", "Download evidence", "Verify evidence", + "Retry after hosted runner loss", + "Wait for retry E2E run", + "Download retry evidence", + "Verify retry evidence", + "Close incomplete retry check", + "Terminalize interrupted retry setup", "Close incomplete check", "Remove private workspace", ]); @@ -787,11 +842,28 @@ describe("PR E2E gate workflow", () => { "Wait for approved E2E run", "Download approved evidence", "Verify approved evidence", + "Retry approved E2E after hosted runner loss", + "Wait for approved retry E2E run", + "Download approved retry evidence", + "Verify approved retry evidence", + "Close incomplete approved retry check", + "Terminalize interrupted approved retry setup", "Close incomplete approved check", "Remove private workspace", ]); expect(step(approveInternal, "Download approved evidence").if).toContain("always()"); expect(step(approveInternal, "Verify approved evidence").if).toContain("always()"); + expect(step(approveInternal, "Retry approved E2E after hosted runner loss").if).toContain( + "github.run_attempt == 1", + ); + expect(step(approveInternal, "Download approved retry evidence").run).toContain( + "--slot runner-loss-retry", + ); + expect(step(approveInternal, "Verify approved retry evidence").if).toContain("always()"); + expect(step(approveInternal, "Close incomplete approved retry check").if).toContain("always()"); + expect(step(approveInternal, "Terminalize interrupted approved retry setup").run).toContain( + "--mode abandon-runner-loss-retry", + ); expect(step(approveInternal, "Close incomplete approved check").if).toContain("always()"); expect(step(approveInternal, "Remove private workspace").if).toContain("always()"); }); diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 301052bd99a..8f811a5d2ce 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -111,7 +111,7 @@ const TERMINAL_WORKFLOW_RUN_CONCLUSIONS = [ ] as const; const TERMINAL_WORKFLOW_RUN_CONCLUSION_SET = new Set(TERMINAL_WORKFLOW_RUN_CONCLUSIONS); const WAIT_POLL_INTERVAL_MS = 10_000; -const WAIT_TIMEOUT_MS = 105 * 60_000; +const WAIT_TIMEOUT_MS = 140 * 60_000; const EVIDENCE_DOWNLOAD_TIMEOUT_MS = 10 * 60_000; const EVIDENCE_DOWNLOAD_KILL_GRACE_MS = 30_000; const EVIDENCE_LIMITS = { diff --git a/tools/e2e/pr-e2e-required.mts b/tools/e2e/pr-e2e-required.mts index 5174a8af7cd..b58befcb9f6 100644 --- a/tools/e2e/pr-e2e-required.mts +++ b/tools/e2e/pr-e2e-required.mts @@ -421,7 +421,7 @@ async function main(): Promise { baseSha: requiredArgument(args.base, "base"), }; const timeoutSeconds = parsePositiveInteger(args.timeoutSeconds, "timeout-seconds"); - if (timeoutSeconds > 10_200) throw new Error("--timeout-seconds must not exceed 10200"); + if (timeoutSeconds > 21_480) throw new Error("--timeout-seconds must not exceed 21480"); const result = await waitForRequiredGate(identity, { timeoutMs: timeoutSeconds * 1000 }); appendJobSummary(); console.log(`E2E / PR Gate completed: ${formatRequiredGateOutcome(result)}`); From a419984477d72ce17a3b769606424dcefe965af9 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 00:38:22 -0700 Subject: [PATCH 10/20] fix(ci): preserve queued gate mutations Signed-off-by: Apurv Kumaria --- .github/workflows/pr-e2e-gate.yaml | 5 ++++ test/e2e/README.md | 48 ++++++++++++++++-------------- test/pr-e2e-gate-workflow.test.ts | 7 ++++- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/.github/workflows/pr-e2e-gate.yaml b/.github/workflows/pr-e2e-gate.yaml index bd825068fb0..951e93cd5eb 100644 --- a/.github/workflows/pr-e2e-gate.yaml +++ b/.github/workflows/pr-e2e-gate.yaml @@ -62,6 +62,7 @@ jobs: pull-requests: read concurrency: group: pr-e2e-gate-${{ github.repository }}-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }}-${{ github.event.pull_request.base.sha }} + queue: max cancel-in-progress: false steps: - name: Checkout controller @@ -185,6 +186,7 @@ jobs: pull-requests: read concurrency: group: pr-e2e-gate-${{ github.repository }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number || inputs.pr_number }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || inputs.expected_head_sha }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].base.sha || inputs.expected_base_sha }} + queue: max # Let the previous coordinator observe E2E cancellation and close its check. cancel-in-progress: false steps: @@ -415,6 +417,7 @@ jobs: pull-requests: read concurrency: group: pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.control_plane_approval_pr_number }}-${{ needs.coordinate.outputs.control_plane_approval_head_sha }}-${{ needs.coordinate.outputs.control_plane_approval_base_sha }} + queue: max cancel-in-progress: false steps: - name: Checkout controller @@ -618,6 +621,7 @@ jobs: pull-requests: read concurrency: group: pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.fork_skip_pr_number }}-${{ needs.coordinate.outputs.fork_skip_head_sha }}-${{ needs.coordinate.outputs.fork_skip_base_sha }} + queue: max cancel-in-progress: false steps: - name: Checkout controller @@ -664,6 +668,7 @@ jobs: pull-requests: read concurrency: group: pr-e2e-gate-${{ github.repository }}-${{ inputs.pr_number }}-${{ inputs.expected_head_sha }}-${{ inputs.expected_base_sha }} + queue: max cancel-in-progress: false steps: - name: Checkout controller diff --git a/test/e2e/README.md b/test/e2e/README.md index 9d7662790ad..284e25d1b13 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -186,15 +186,17 @@ the deterministic risk plan. Runtime families and changes to workflow-wired live tests select canonical selectors from the trusted `e2e.yaml` inventory independently of advisor output. Ordinary internal changes execute those focused selections. -Gate initialization and CI coordination share one non-cancelling concurrency -group for the head repository and branch. Before the controller creates or -updates coordination for the current revision, it reads the live PR and -requires the event's PR SHA and base SHA, including when PR CI failed. The -native observer performs the same live PR/base SHA check before waiting and -again before accepting a terminal verdict. This keeps a stale seed, completed -CI run, or observer from being applied to a newer PR/base SHA pair. A completed CI -event for an older revision is handled without creating or updating the current -revision's coordination check. +Gate initialization, CI coordination, protected approval, and manual fork-skip +recording share one non-cancelling FIFO concurrency group for the exact +repository, PR number, PR SHA, and base SHA. `queue: max` keeps pending jobs for +that exact identity instead of replacing them, up to GitHub's 100-job bound. +Before the controller creates or updates coordination for the current revision, +it reads the live PR and requires the event's PR SHA and base SHA, including +when PR CI failed. The native observer performs the same live PR/base SHA check +before waiting and again before accepting a terminal verdict. This keeps a +stale seed, completed CI run, or observer from being applied to a newer PR/base +SHA pair. A completed CI event for an older revision is handled without +creating or updating the current revision's coordination check. If the older revision still has an in-progress coordination check, the controller completes it as cancelled with `Superseded by PR update` or `PR closed — gate no longer applies` and identifies the obsolete head and base. @@ -256,17 +258,18 @@ false`, the job does not create a deployment record. After reviewing the exact head SHA, base SHA, and risk plan as described below, an environment reviewer opens the linked run, chooses **Review deployments**, selects that environment, and approves it. GitHub records the reviewer and optional comment. The -protected approval job uses per-PR concurrency with in-progress cancellation. -When a newer revision reaches authorization, its job cancels any waiting -approval job for the prior revision and becomes available for review without -waiting on the obsolete request. The controller reads that approval history and -requires one approved review naming only the exact environment in the first -attempt of the trusted `workflow_run` -controller. It then revalidates the internal repository origin, open PR, PR SHA -and base SHA, risk plan, matching pending coordination state, compatible -trusted controller commit, and final live revision. It updates coordination to -`Running E2E check(s)` and dispatches the selected jobs and targets in -one workflow run. +protected approval job uses the exact-revision FIFO concurrency group without +in-progress cancellation. A newer revision uses a distinct group and can +become available for review without waiting on the obsolete request. The +synchronization controller cancels active child runs and closes coordination +checks for the old revision. If an old approval job later starts, exact live +PR SHA and base SHA validation rejects it. The controller reads the approval +history and requires one approved review naming only the exact environment in +the first attempt of the trusted `workflow_run` controller. It then revalidates +the internal repository origin, open PR, PR SHA and base SHA, risk plan, +matching pending coordination state, compatible trusted controller commit, and +final live revision. It updates coordination to `Running E2E check(s)` +and dispatches the selected jobs and targets in one workflow run. The manual maintainer path remains available as a fallback. A repository maintainer or administrator chooses **Run workflow** on `main`, selects @@ -357,8 +360,9 @@ Configure the environment, update the PR to create a new head, and trigger fresh upstream PR CI to create a new gate run, or use the corresponding manual maintainer fallback. GitHub approval history is not bound to a run attempt, so the controller rejects reruns of an -approval run. Per-PR approval concurrency cancels an older waiting job when a -newer revision reaches the gate. +approval run. Approval concurrency is bound to the exact PR SHA and base SHA. +A newer revision creates a separate approval request, while an obsolete request +cannot authorize it. For the fork button path, the controller requires a first-attempt, in-progress run of this exact workflow on `main`, at the trusted workflow SHA and with the diff --git a/test/pr-e2e-gate-workflow.test.ts b/test/pr-e2e-gate-workflow.test.ts index ffff84f6a56..4db079a7391 100644 --- a/test/pr-e2e-gate-workflow.test.ts +++ b/test/pr-e2e-gate-workflow.test.ts @@ -21,7 +21,7 @@ const BASE_SHA = "b".repeat(40); const WORKFLOW_SHA = "d".repeat(40); type CoordinatorJob = WorkflowJob & { - concurrency?: { group: string; "cancel-in-progress": boolean }; + concurrency?: { group: string; queue?: "max"; "cancel-in-progress": boolean }; }; type TriggeredWorkflow = Omit & { @@ -401,6 +401,7 @@ describe("PR E2E gate workflow", () => { expect(initialize.concurrency?.group).toBe( "pr-e2e-gate-${{ github.repository }}-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }}-${{ github.event.pull_request.base.sha }}", ); + expect(initialize.concurrency?.queue).toBe("max"); expect(initialize.concurrency?.["cancel-in-progress"]).toBe(false); expect(required.name).toBe("E2E / PR Gate"); expect(required.if).toContain("github.event_name == 'pull_request_target'"); @@ -467,6 +468,7 @@ describe("PR E2E gate workflow", () => { expect(coordinate.concurrency?.group).toBe( "pr-e2e-gate-${{ github.repository }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number || inputs.pr_number }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || inputs.expected_head_sha }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].base.sha || inputs.expected_base_sha }}", ); + expect(coordinate.concurrency?.queue).toBe("max"); expect(coordinate.concurrency?.["cancel-in-progress"]).toBe(false); expect(coordinate["timeout-minutes"]).toBe(330); expect(coordinate.outputs).toEqual({ @@ -498,6 +500,7 @@ describe("PR E2E gate workflow", () => { expect(approveInternal.concurrency).toEqual({ group: "pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.control_plane_approval_pr_number }}-${{ needs.coordinate.outputs.control_plane_approval_head_sha }}-${{ needs.coordinate.outputs.control_plane_approval_base_sha }}", + queue: "max", "cancel-in-progress": false, }); expect(approveInternal["timeout-minutes"]).toBe(330); @@ -531,6 +534,7 @@ describe("PR E2E gate workflow", () => { expect(approveForkSkip.concurrency).toEqual({ group: "pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.fork_skip_pr_number }}-${{ needs.coordinate.outputs.fork_skip_head_sha }}-${{ needs.coordinate.outputs.fork_skip_base_sha }}", + queue: "max", "cancel-in-progress": false, }); expect(approveForkSkip.secrets).toBeUndefined(); @@ -547,6 +551,7 @@ describe("PR E2E gate workflow", () => { expect(recordForkSkip.concurrency).toEqual({ group: "pr-e2e-gate-${{ github.repository }}-${{ inputs.pr_number }}-${{ inputs.expected_head_sha }}-${{ inputs.expected_base_sha }}", + queue: "max", "cancel-in-progress": false, }); expect(collectStrings(initialize).some((value) => value.includes("--mode seed"))).toBe(true); From 08c6174416bfda61e55f3edf1a30266c0f4b4fa1 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 01:19:35 -0700 Subject: [PATCH 11/20] fix(ci): isolate gate checks across retargets Signed-off-by: Apurv Kumaria --- test/e2e/README.md | 9 ++++++--- test/pr-e2e-gate.test.ts | 22 ++++++++++------------ tools/e2e/pr-e2e-gate.mts | 11 ++--------- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/test/e2e/README.md b/test/e2e/README.md index 284e25d1b13..1bb49cfe49d 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -170,9 +170,12 @@ On open, synchronization, reopen, transition out of draft, or base retarget, the PR SHA and base SHA, including fork SHAs. The read-only native observer starts for every configured non-closed PR event; metadata-only edits mirror the existing PR/base SHA coordination result instead of publishing a -skipped success. A base retarget fails any still-active earlier coordination -result in that head's lineage, preserves completed audit history, and then -reserves the new PR/base SHA identity. The +skipped success. A base retarget reserves a distinct PR/base SHA identity. +Controllers never mutate coordination owned by another base because a newer +base can appear after an older controller's live validation. The exact-identity +observer ignores checks from other bases, and an older controller that resumes +fails its own coordination closed at final live validation. Completed results +remain audit history. The `CI / Pull Request` run name binds its PR number, head SHA, base SHA, and gate eligibility so the trusted controller can authenticate the completed run even when a fork `workflow_run` payload omits pull-request metadata. The controller diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index 8b7d2db13fa..a6a2716d8f5 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -735,10 +735,10 @@ describe("PR E2E controller", () => { expect(requests[1]?.url).toContain(`/commits/${HEAD_SHA}/check-runs?`); }); - it("closes a stale retarget check before reusing the original base check", async () => { + it("does not mutate a newer-base gate that appears after live validation", async () => { vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const otherBaseSha = "c".repeat(40); + const newerBaseSha = "c".repeat(40); const requests: RecordedGitHubRequest[] = []; vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter( @@ -749,19 +749,18 @@ describe("PR E2E controller", () => { url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", () => githubResponse({ - total_count: 2, + total_count: 1, check_runs: [ - exactPrGateCheck(), exactPrGateCheck({ id: 18, - external_id: prGateExternalId(42, HEAD_SHA, otherBaseSha), + external_id: prGateExternalId(42, HEAD_SHA, newerBaseSha), }), ], }), ), githubFetchRoute( - ({ url, method }) => url.endsWith("/check-runs/18") && method === "PATCH", - (request) => prGateMutationResponse(request, 18), + ({ url, method }) => url.endsWith("/check-runs") && method === "POST", + (request) => prGateMutationResponse(request), ), ], requests, @@ -771,14 +770,13 @@ describe("PR E2E controller", () => { await expect(seedPrGate(42, HEAD_SHA, BASE_SHA)).resolves.toBe(17); expect(requests).toHaveLength(3); expect(requests[2]).toMatchObject({ - method: "PATCH", + method: "POST", body: { - status: "completed", - conclusion: "failure", - output: { title: "PR base changed" }, + external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), + status: "in_progress", }, }); - expect(requests.some((request) => request.url.endsWith("/check-runs"))).toBe(false); + expect(requests.some((request) => request.method === "PATCH")).toBe(false); }); it("rejects a seeded identity claimed by another GitHub App", async () => { diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 8f811a5d2ce..3f108fec8ed 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -1313,15 +1313,8 @@ async function ensurePrGateCheck(options: { const externalId = prGateExternalId(options.prNumber, options.headSha, options.baseSha); const existing = lineage.filter((check) => check.external_id === externalId); const current = currentExactDiffCheck(existing); - for (const stale of lineage.filter((check) => check.external_id !== externalId)) { - if (stale.status === "completed") continue; - await completeCheck({ repository: options.repository, checkRunId: stale.id }, options.token, { - conclusion: "failure", - title: "PR base changed", - summary: - "This check was computed for an earlier PR base and cannot authorize the current diff.", - }); - } + // A base retarget can create another exact identity after this caller's live + // PR validation. Never mutate checks owned by a different base from here. if ( current && !(options.replaceRetryableCompleted && retryableFailureReason(current) !== undefined) From 357e4b83784337ee9bdd7c31b3c3f18367eb0d9a Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 01:36:31 -0700 Subject: [PATCH 12/20] test(ci): keep retry fixtures linear Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-runner-loss-retry.test.ts | 25 +++++++++++----------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index f0ad74ed906..deb9edfa795 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -263,12 +263,13 @@ function retryRoutes( githubFetchRoute( ({ url, method }) => url.includes("/actions/runs/23/attempts/1/jobs?") && method === "GET", (request) => { - if (options.jobPages) { - const page = Number(new URL(request.url).searchParams.get("page")); - return githubResponse(options.jobPages[page - 1]); - } + const page = Number(new URL(request.url).searchParams.get("page")); const jobs = options.jobs ?? [hostedRunnerLossJob()]; - return githubResponse({ total_count: jobs.length, jobs }); + const response = + options.jobPages === undefined + ? { total_count: jobs.length, jobs } + : options.jobPages[page - 1]; + return githubResponse(response); }, ), githubFetchRoute( @@ -306,9 +307,7 @@ function retryRoutes( ({ url, method }) => url.endsWith("/actions/workflows/e2e.yaml/dispatches") && method === "POST", () => { - if (options.createRetryStateDirectory) { - fs.mkdirSync(options.createRetryStateDirectory); - } + options.createRetryStateDirectory && fs.mkdirSync(options.createRetryStateDirectory); return githubResponse({ workflow_run_id: 24, run_url: "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/24", @@ -930,11 +929,11 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { childRunId: 24, evidenceOutcome, }); - if (conclusion === "success") { - await expect(finalization).rejects.toThrow(/Evidence download did not complete/u); - } else { - await expect(finalization).resolves.toBeUndefined(); - } + const expectedFinalization = + conclusion === "success" + ? expect(finalization).rejects.toThrow(/Evidence download did not complete/u) + : expect(finalization).resolves.toBeUndefined(); + await expectedFinalization; const completion = requests.find( (request) => request.url.endsWith("/check-runs/18") && From 6232500ba938327eed5fcfa4bd3ffed71ed692f4 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 02:15:15 -0700 Subject: [PATCH 13/20] fix(ci): authenticate hosted runner shutdown logs Signed-off-by: Apurv Kumaria --- ...pr-e2e-gate-runner-loss-classifier.test.ts | 328 ++++++++++- tools/e2e/pr-e2e-gate.mts | 538 +++++++++++++++++- 2 files changed, 847 insertions(+), 19 deletions(-) diff --git a/test/pr-e2e-gate-runner-loss-classifier.test.ts b/test/pr-e2e-gate-runner-loss-classifier.test.ts index 9e63cfac337..c5275e183ec 100644 --- a/test/pr-e2e-gate-runner-loss-classifier.test.ts +++ b/test/pr-e2e-gate-runner-loss-classifier.test.ts @@ -8,6 +8,13 @@ import { detectRunnerLoss } from "../tools/e2e/runner-pressure-core.mts"; const WORKFLOW_SHA = "d".repeat(40); const RUNNER_LOSS_MESSAGE = "The hosted runner lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error."; +const RUNNER_SHUTDOWN_MESSAGE = + "The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled."; +const JOB_STARTED_AT = "2026-07-23T07:26:56Z"; +const CANCELLED_STEP_STARTED_AT = "2026-07-23T07:27:43Z"; +const CANCELLED_STEP_COMPLETED_AT = "2026-07-23T07:32:49Z"; +const COMPLETE_JOB_AT = "2026-07-23T07:32:50Z"; +const JOB_COMPLETED_AT = "2026-07-23T07:32:54Z"; function runnerLossAnnotation(message = RUNNER_LOSS_MESSAGE) { return { @@ -24,6 +31,39 @@ function runnerLossAnnotation(message = RUNNER_LOSS_MESSAGE) { }; } +function genericCancellationAnnotation() { + return { + ...runnerLossAnnotation("The operation was canceled."), + startLine: 34, + endLine: 34, + }; +} + +function orphanProcessLogLine(index: number, pid = index + 1, processName = `node-${pid}`) { + return `2026-07-23T07:32:50.${String(1_000_000 + index).padStart( + 7, + "0", + )}Z Terminate orphan process: pid (${pid}) (${processName})`; +} + +function runnerShutdownLogTail(orphanProcessLines: string[] = []) { + const lines = [ + `2026-07-23T07:32:47.0261924Z ##[error]${RUNNER_SHUTDOWN_MESSAGE}`, + "2026-07-23T07:32:49.9360750Z ##[error]The operation was canceled.", + "2026-07-23T07:32:50.0577487Z Cleaning up orphan processes", + ...orphanProcessLines, + ]; + return `${lines.join("\n")}\n`; +} + +function runnerShutdownLogEvidence(tail = runnerShutdownLogTail()) { + return { + etag: '"runner-loss-log-etag"', + totalBytes: new TextEncoder().encode(tail).byteLength, + tail, + }; +} + function hostedRunnerLossJob(overrides: Record = {}) { return { id: 89_074_697_099, @@ -37,16 +77,44 @@ function hostedRunnerLossJob(overrides: Record = {}) { runnerGroupName: "GitHub Actions", labels: ["ubuntu-latest"], annotations: [runnerLossAnnotation()], + startedAt: JOB_STARTED_AT, + completedAt: JOB_COMPLETED_AT, steps: [ - { name: "Set up job", status: "completed", conclusion: "success" }, + { + name: "Set up job", + status: "completed", + conclusion: "success", + startedAt: "2026-07-23T07:26:57Z", + completedAt: "2026-07-23T07:27:03Z", + }, { name: "Run security posture live Vitest test", status: "completed", conclusion: "cancelled", + startedAt: CANCELLED_STEP_STARTED_AT, + completedAt: CANCELLED_STEP_COMPLETED_AT, + }, + { + name: "Upload security posture artifacts", + status: "completed", + conclusion: "skipped", + startedAt: CANCELLED_STEP_COMPLETED_AT, + completedAt: CANCELLED_STEP_COMPLETED_AT, + }, + { + name: "Clean up Docker auth", + status: "completed", + conclusion: "skipped", + startedAt: CANCELLED_STEP_COMPLETED_AT, + completedAt: CANCELLED_STEP_COMPLETED_AT, + }, + { + name: "Complete job", + status: "completed", + conclusion: "success", + startedAt: COMPLETE_JOB_AT, + completedAt: COMPLETE_JOB_AT, }, - { name: "Upload security posture artifacts", status: "completed", conclusion: "skipped" }, - { name: "Clean up Docker auth", status: "completed", conclusion: "skipped" }, - { name: "Complete job", status: "completed", conclusion: "success" }, ], ...overrides, }; @@ -88,6 +156,87 @@ describe("PR E2E hosted-runner-loss classifier", () => { expect(confirmsRunnerLoss()).toBe(true); }); + it("accepts the exact authenticated terminal shutdown block from run 29988226653", () => { + expect( + confirmsRunnerLoss({ + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence(), + }), + ], + }), + ).toBe(true); + }); + + it("accepts a bounded orphan-process suffix after the terminal shutdown block", () => { + expect( + confirmsRunnerLoss({ + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + runnerShutdownLogTail([orphanProcessLogLine(0, 4_321, "node")]), + ), + }), + ], + }), + ).toBe(true); + }); + + it("ignores carriage-return progress output before the exact terminal shutdown block", () => { + expect( + confirmsRunnerLoss({ + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + `2026-07-23T07:31:00.0000000Z build progress 50%\r\n${runnerShutdownLogTail()}`, + ), + }), + ], + }), + ).toBe(true); + }); + + it("allows a trusted notice beside the sole shutdown failure annotation", () => { + expect( + confirmsRunnerLoss({ + jobs: [ + hostedRunnerLossJob({ + annotations: [ + genericCancellationAnnotation(), + { + ...runnerLossAnnotation("Docker credentials were withheld."), + startLine: 53, + endLine: 53, + annotationLevel: "notice", + }, + ], + logEvidence: runnerShutdownLogEvidence(), + }), + ], + }), + ).toBe(true); + }); + + it("accepts at most 64 unique bounded orphan-process records", () => { + expect( + confirmsRunnerLoss({ + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + runnerShutdownLogTail( + Array.from({ length: 64 }, (_, index) => orphanProcessLogLine(index)), + ), + ), + }), + ], + }), + ).toBe(true); + }); + it("accepts two strict standard-hosted legacy markers from run 29964500642", () => { expect( confirmsRunnerLoss({ @@ -130,6 +279,177 @@ describe("PR E2E hosted-runner-loss classifier", () => { ], }, }, + { + label: "the shutdown log has no generic failure annotation", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [], + logEvidence: runnerShutdownLogEvidence(), + }), + ], + }, + }, + { + label: "the shutdown log is followed by later output", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + `${runnerShutdownLogTail()}2026-07-23T07:32:51.0000000Z later output\n`, + ), + }), + ], + }, + }, + { + label: "the shutdown log omits its final line feed", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence(runnerShutdownLogTail().slice(0, -1)), + }), + ], + }, + }, + { + label: "the shutdown log uses CRLF records", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + runnerShutdownLogTail().replaceAll("\n", "\r\n"), + ), + }), + ], + }, + }, + { + label: "the shutdown log ends with an extra blank line", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence(`${runnerShutdownLogTail()}\n`), + }), + ], + }, + }, + { + label: "the shutdown log has a second explicit cancellation failure", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [ + genericCancellationAnnotation(), + runnerLossAnnotation( + "Canceling since a higher priority waiting request for CI / Pull Request exists", + ), + ], + logEvidence: runnerShutdownLogEvidence(), + }), + ], + }, + }, + { + label: "the shutdown log records the terminal messages out of order", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + [ + "2026-07-23T07:32:47.0261924Z ##[error]The operation was canceled.", + `2026-07-23T07:32:49.9360750Z ##[error]${RUNNER_SHUTDOWN_MESSAGE}`, + "2026-07-23T07:32:50.0577487Z Cleaning up orphan processes", + "", + ].join("\n"), + ), + }), + ], + }, + }, + { + label: "the shutdown log timestamps move backward", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + [ + `2026-07-23T07:32:47.0261924Z ##[error]${RUNNER_SHUTDOWN_MESSAGE}`, + "2026-07-23T07:32:49.9360750Z ##[error]The operation was canceled.", + "2026-07-23T07:32:48.0577487Z Cleaning up orphan processes", + "", + ].join("\n"), + ), + }), + ], + }, + }, + { + label: "the shutdown log repeats an orphan-process PID", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + runnerShutdownLogTail([ + orphanProcessLogLine(0, 4_321, "node"), + orphanProcessLogLine(1, 4_321, "node"), + ]), + ), + }), + ], + }, + }, + { + label: "the shutdown log records an invalid orphan process", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + runnerShutdownLogTail([orphanProcessLogLine(0, 4_321, "node/worker")]), + ), + }), + ], + }, + }, + { + label: "the shutdown log contains more than 64 orphan-process records", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence( + runnerShutdownLogTail( + Array.from({ length: 65 }, (_, index) => orphanProcessLogLine(index)), + ), + ), + }), + ], + }, + }, + { + label: "the operation cancellation timestamp does not match the cancelled step", + options: { + jobs: [ + hostedRunnerLossJob({ + annotations: [genericCancellationAnnotation()], + logEvidence: runnerShutdownLogEvidence(), + steps: hostedRunnerLossJob().steps.map((step) => + step.conclusion === "cancelled" + ? { ...step, completedAt: "2026-07-23T07:32:48Z" } + : step, + ), + }), + ], + }, + }, { label: "the failure annotation reports a job timeout", options: { diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 3f108fec8ed..5846e7dd39e 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -84,6 +84,20 @@ const MAX_WORKFLOW_JOB_PAGES = 10; const MAX_JOB_ANNOTATION_PAGES = 10; const HOSTED_RUNNER_LOST_COMMUNICATION_MESSAGE = "The hosted runner lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error."; +const HOSTED_RUNNER_SHUTDOWN_MESSAGE = + "The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled."; +const HOSTED_RUNNER_OPERATION_CANCELLED_MESSAGE = "The operation was canceled."; +const HOSTED_RUNNER_ORPHAN_CLEANUP_MESSAGE = "Cleaning up orphan processes"; +const MAX_RUNNER_LOSS_JOB_INSPECTIONS = 20; +const MAX_RUNNER_LOSS_JOB_LOG_TAIL_BYTES = 64 * 1024; +const MAX_RUNNER_LOSS_ORPHAN_PROCESSES = 64; +const RUNNER_LOSS_JOB_LOG_TIMEOUT_MS = 30_000; +const JOB_LOG_DOWNLOAD_HOST_PATTERN = /^productionresultssa[0-9]+\.blob\.core\.windows\.net$/u; +const JOB_LOG_TIMESTAMPED_LINE_PATTERN = + /^([0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{7}Z) (.*)$/u; +const JOB_LOG_ORPHAN_PROCESS_PATTERN = + /^Terminate orphan process: pid \(([1-9][0-9]*)\) \(([A-Za-z0-9._+ -]{1,128})\)$/u; +const GITHUB_TIMESTAMP_PATTERN = /^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$/u; const MAX_REPORTED_WORKFLOW_JOBS = 10; const MAX_WAIVER_REASON_CHARS = 500; const MAX_APPROVAL_REVIEWS = 20; @@ -276,6 +290,17 @@ type WorkflowJobAnnotation = { message: string; rawDetails: string; }; +type WorkflowJobLogEvidence = { + etag: string; + totalBytes: number; + tail: string; +}; +type HostedRunnerShutdownLogMarker = { + shutdownTimestamp: string; + operationTimestamp: string; + cleanupTimestamp: string; + lastTimestamp: string; +}; type WorkflowJob = { id: number; name: string; @@ -294,7 +319,16 @@ type WorkflowJob = { runnerGroupName?: string | null; labels?: string[]; annotations?: WorkflowJobAnnotation[]; - steps: Array<{ name: string; status?: string; conclusion: string | null }>; + logEvidence?: WorkflowJobLogEvidence; + startedAt?: string | null; + completedAt?: string | null; + steps: Array<{ + name: string; + status?: string; + conclusion: string | null; + startedAt?: string | null; + completedAt?: string | null; + }>; }; type WorkflowJobsPage = { totalCount: number; jobs: WorkflowJob[] }; type CheckRun = { @@ -1666,6 +1700,14 @@ export async function resolvePullRequest(options: { return detail; } +function isOptionalGitHubTimestamp(value: unknown): boolean { + return ( + value === undefined || + value === null || + (typeof value === "string" && GITHUB_TIMESTAMP_PATTERN.test(value)) + ); +} + function validateWorkflowJob(value: unknown): WorkflowJob { if ( !isObjectRecord(value) || @@ -1685,6 +1727,8 @@ function validateWorkflowJob(value: unknown): WorkflowJob { (value.check_run_url !== undefined && typeof value.check_run_url !== "string") || (value.status !== undefined && typeof value.status !== "string") || (value.conclusion !== null && typeof value.conclusion !== "string") || + !isOptionalGitHubTimestamp(value.started_at) || + !isOptionalGitHubTimestamp(value.completed_at) || (value.runner_id !== undefined && value.runner_id !== null && (!Number.isSafeInteger(value.runner_id) || (value.runner_id as number) < 1)) || @@ -1709,7 +1753,9 @@ function validateWorkflowJob(value: unknown): WorkflowJob { typeof step.name !== "string" || step.name.length === 0 || (step.status !== undefined && typeof step.status !== "string") || - (step.conclusion !== null && typeof step.conclusion !== "string") + (step.conclusion !== null && typeof step.conclusion !== "string") || + !isOptionalGitHubTimestamp(step.started_at) || + !isOptionalGitHubTimestamp(step.completed_at) ) { throw new Error("GitHub returned an invalid workflow job step"); } @@ -1717,6 +1763,10 @@ function validateWorkflowJob(value: unknown): WorkflowJob { name: step.name, ...(step.status === undefined ? {} : { status: step.status }), conclusion: step.conclusion, + ...(step.started_at === undefined ? {} : { startedAt: step.started_at as string | null }), + ...(step.completed_at === undefined + ? {} + : { completedAt: step.completed_at as string | null }), }; }); return { @@ -1738,6 +1788,10 @@ function validateWorkflowJob(value: unknown): WorkflowJob { : { runnerGroupId: value.runner_group_id as number | null }), ...(value.runner_group_name === undefined ? {} : { runnerGroupName: value.runner_group_name }), ...(value.labels === undefined ? {} : { labels: value.labels as string[] }), + ...(value.started_at === undefined ? {} : { startedAt: value.started_at as string | null }), + ...(value.completed_at === undefined + ? {} + : { completedAt: value.completed_at as string | null }), steps, }; } @@ -1856,6 +1910,197 @@ async function listWorkflowJobAnnotations( throw new Error("workflow job annotation listing exceeded its page limit"); } +function parseJobLogContentLength(value: string | null, label: string): number { + if (!value || !/^(?:0|[1-9][0-9]*)$/u.test(value)) { + throw new Error(`${label} did not provide a valid content length`); + } + const length = Number(value); + if (!Number.isSafeInteger(length) || length < 0) { + throw new Error(`${label} content length is outside the safe integer range`); + } + return length; +} + +function validateJobLogEtag(value: string | null): string { + if (!value || value.length > 130 || !/^"[^"\r\n]{1,128}"$/u.test(value)) { + throw new Error("job log download did not provide a strong bounded ETag"); + } + return value; +} + +function validateJobLogDownloadUrl(value: string | null): URL { + let url: URL; + try { + url = new URL(value ?? ""); + } catch { + throw new Error("job log API returned an invalid signed download URL"); + } + if ( + url.protocol !== "https:" || + url.username !== "" || + url.password !== "" || + url.port !== "" || + !JOB_LOG_DOWNLOAD_HOST_PATTERN.test(url.hostname) || + !url.pathname.startsWith("/actions-results/") || + url.search.length < 2 || + url.hash !== "" + ) { + throw new Error("job log API returned an untrusted signed download URL"); + } + return url; +} + +function assertPlainUnencodedJobLog(response: Response, label: string): void { + const contentType = response.headers.get("content-type")?.split(";", 1)[0]?.trim(); + if (contentType !== "text/plain" || response.headers.get("content-encoding") !== null) { + throw new Error(`${label} did not return unencoded plain text`); + } +} + +async function cancelJobLogResponseBody(response: Response): Promise { + await response.body?.cancel().catch(() => undefined); +} + +async function readExactJobLogRange( + response: Response, + expectedBytes: number, + discardPartialFirstLine: boolean, +): Promise { + if (!response.body) throw new Error("job log range response did not include a body"); + const reader = response.body.getReader(); + const chunks: Uint8Array[] = []; + let receivedBytes = 0; + try { + for (;;) { + const chunk = await reader.read(); + if (chunk.done) break; + receivedBytes += chunk.value.byteLength; + if (receivedBytes > expectedBytes || receivedBytes > MAX_RUNNER_LOSS_JOB_LOG_TAIL_BYTES) { + throw new Error("job log range response exceeded its authenticated byte bound"); + } + chunks.push(chunk.value); + } + } catch (error) { + await reader.cancel().catch(() => undefined); + throw error; + } finally { + reader.releaseLock(); + } + if (receivedBytes !== expectedBytes) { + throw new Error("job log range response was incomplete"); + } + const bytes = new Uint8Array(receivedBytes); + let offset = 0; + for (const chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.byteLength; + } + const firstLineFeed = discardPartialFirstLine ? bytes.indexOf(0x0a) : -1; + if (discardPartialFirstLine && firstLineFeed < 0) { + throw new Error("job log range did not contain a complete record"); + } + const completeRecords = firstLineFeed < 0 ? bytes : bytes.subarray(firstLineFeed + 1); + return new TextDecoder("utf-8", { fatal: true }).decode(completeRecords); +} + +async function downloadWorkflowJobLogTail( + repository: string, + token: string, + jobId: number, +): Promise { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), RUNNER_LOSS_JOB_LOG_TIMEOUT_MS); + const apiUrl = `https://api.github.com/repos/${repository}/actions/jobs/${jobId}/logs`; + try { + const redirect = await fetch(apiUrl, { + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "User-Agent": USER_AGENT, + "X-GitHub-Api-Version": "2022-11-28", + }, + redirect: "manual", + signal: controller.signal, + }); + if (redirect.status !== 302) { + await cancelJobLogResponseBody(redirect); + throw new Error(`job log API returned unexpected status ${redirect.status}`); + } + const location = redirect.headers.get("location"); + await cancelJobLogResponseBody(redirect); + const downloadUrl = validateJobLogDownloadUrl(location); + + const downloadHeaders = { + Accept: "text/plain", + "Accept-Encoding": "identity", + "User-Agent": USER_AGENT, + }; + const metadata = await fetch(downloadUrl, { + method: "HEAD", + headers: downloadHeaders, + redirect: "error", + signal: controller.signal, + }); + if (metadata.status !== 200) { + await cancelJobLogResponseBody(metadata); + throw new Error(`job log metadata returned unexpected status ${metadata.status}`); + } + let totalBytes: number; + let etag: string; + try { + assertPlainUnencodedJobLog(metadata, "job log metadata"); + totalBytes = parseJobLogContentLength( + metadata.headers.get("content-length"), + "job log metadata", + ); + if (totalBytes < 1) throw new Error("job log is empty"); + etag = validateJobLogEtag(metadata.headers.get("etag")); + } catch (error) { + await cancelJobLogResponseBody(metadata); + throw error; + } + await cancelJobLogResponseBody(metadata); + + const rangeStart = Math.max(0, totalBytes - MAX_RUNNER_LOSS_JOB_LOG_TAIL_BYTES); + const rangeEnd = totalBytes - 1; + const expectedBytes = rangeEnd - rangeStart + 1; + const range = await fetch(downloadUrl, { + headers: { + ...downloadHeaders, + "If-Match": etag, + Range: `bytes=${rangeStart}-${rangeEnd}`, + }, + redirect: "error", + signal: controller.signal, + }); + if (range.status !== 206) { + await cancelJobLogResponseBody(range); + throw new Error(`job log range returned unexpected status ${range.status}`); + } + try { + assertPlainUnencodedJobLog(range, "job log range"); + if ( + range.headers.get("etag") !== etag || + range.headers.get("content-range") !== `bytes ${rangeStart}-${rangeEnd}/${totalBytes}` || + parseJobLogContentLength(range.headers.get("content-length"), "job log range") !== + expectedBytes + ) { + throw new Error("job log range did not match its authenticated metadata"); + } + } catch (error) { + await cancelJobLogResponseBody(range); + throw error; + } + return { + etag, + totalBytes, + tail: await readExactJobLogRange(range, expectedBytes, rangeStart > 0), + }; + } finally { + clearTimeout(timeout); + } +} + function validateWorkflowJobsPage(value: unknown): WorkflowJobsPage { if ( !isObjectRecord(value) || @@ -1913,7 +2158,11 @@ async function listNonPassingWorkflowJobs( (job) => !["success", "skipped", "neutral"].includes(job.conclusion ?? ""), ); if (options.includeAnnotations) { - for (const job of nonPassingJobs.filter(hasTrustedHostedRunnerLossStepShape)) { + const runnerLossCandidates = nonPassingJobs.filter(hasTrustedHostedRunnerLossStepShape); + if (runnerLossCandidates.length > MAX_RUNNER_LOSS_JOB_INSPECTIONS) { + throw new Error("workflow run exceeded the hosted-runner-loss inspection limit"); + } + for (const job of runnerLossCandidates) { job.annotations = await listWorkflowJobAnnotations( repository, token, @@ -1921,6 +2170,19 @@ async function listNonPassingWorkflowJobs( runId, runAttempt, ); + const workflowSha = job.headSha ?? ""; + if ( + !hasTrustedHostedRunnerLossAnnotation(job, repository, workflowSha) && + hasCompatibleHostedRunnerShutdownAnnotations(job, repository, workflowSha) + ) { + try { + job.logEvidence = await downloadWorkflowJobLogTail(repository, token, job.id); + } catch { + console.warn( + `Could not authenticate hosted-runner shutdown log for job ${job.id}; automatic retry remains disabled`, + ); + } + } } } return { @@ -2060,29 +2322,39 @@ function ciFailureReport(options: { const GITHUB_HOSTED_RUNNER_NAME_PATTERN = /^GitHub Actions [1-9][0-9]*$/u; +function trustedWorkflowJobAnnotations( + job: WorkflowJob, + repository: string, + workflowSha: string, +): WorkflowJobAnnotation[] | null { + if (job.headSha !== workflowSha || !Array.isArray(job.annotations)) return null; + const blobPrefix = `https://github.com/${repository}/blob/${workflowSha}/`; + if ( + job.annotations.some((annotation) => annotation.blobHref !== `${blobPrefix}${annotation.path}`) + ) { + return null; + } + return job.annotations; +} + /** * GitHub records a lost hosted runner as a completed failed job with no * ordinary failed step. Older Jobs API responses left the interrupted step * `in_progress`; current responses can terminalize it as `cancelled`, skip the * remaining cleanup, and append the synthetic successful `Complete job` step. * A user or concurrency cancellation concludes the job itself as `cancelled`, - * while an ordinary assertion records a failed step. The step shape is still - * ambiguous with a job-level timeout, so a canonical GitHub runner-loss - * annotation bound to this workflow SHA is also required. + * while an ordinary assertion records a failed step. The step shape must be + * paired with either a canonical GitHub runner-loss annotation or an + * authenticated exact terminal shutdown log. */ function hasTrustedHostedRunnerLossAnnotation( job: WorkflowJob, repository: string, workflowSha: string, ): boolean { - if (job.headSha !== workflowSha || !Array.isArray(job.annotations)) return false; - const blobPrefix = `https://github.com/${repository}/blob/${workflowSha}/`; - if ( - job.annotations.some((annotation) => annotation.blobHref !== `${blobPrefix}${annotation.path}`) - ) { - return false; - } - const failures = job.annotations.filter((annotation) => annotation.annotationLevel === "failure"); + const annotations = trustedWorkflowJobAnnotations(job, repository, workflowSha); + if (!annotations) return false; + const failures = annotations.filter((annotation) => annotation.annotationLevel === "failure"); return ( failures.length === 1 && failures[0]?.path === ".github" && @@ -2096,6 +2368,140 @@ function hasTrustedHostedRunnerLossAnnotation( ); } +function hasCompatibleHostedRunnerShutdownAnnotations( + job: WorkflowJob, + repository: string, + workflowSha: string, +): boolean { + const annotations = trustedWorkflowJobAnnotations(job, repository, workflowSha); + if (!annotations) return false; + const failures = annotations.filter((annotation) => annotation.annotationLevel === "failure"); + const failure = failures[0]; + return ( + failures.length === 1 && + failure?.path === ".github" && + failure.startLine === failure.endLine && + failure.startColumn === null && + failure.endColumn === null && + failure.title === "" && + failure.rawDetails === "" && + failure.message === HOSTED_RUNNER_OPERATION_CANCELLED_MESSAGE + ); +} + +function jobLogTimestampSecond(timestamp: string): string | null { + const second = `${timestamp.slice(0, 19)}Z`; + const milliseconds = Date.parse(second); + return Number.isFinite(milliseconds) && + new Date(milliseconds).toISOString().slice(0, 19) === timestamp.slice(0, 19) + ? second + : null; +} + +function parseHostedRunnerShutdownLogTail(logTail: string): HostedRunnerShutdownLogMarker | null { + if (!logTail.endsWith("\n") || logTail.endsWith("\n\n")) return null; + const lines = logTail.slice(0, -1).split("\n"); + const shutdownMessage = `##[error]${HOSTED_RUNNER_SHUTDOWN_MESSAGE}`; + const shutdownIndex = lines + .map((line) => JOB_LOG_TIMESTAMPED_LINE_PATTERN.exec(line)?.[2] ?? "") + .lastIndexOf(shutdownMessage); + if (shutdownIndex < 0) return null; + const terminalLines = lines.slice(shutdownIndex); + if (terminalLines.length < 3 || terminalLines.length > 3 + MAX_RUNNER_LOSS_ORPHAN_PROCESSES) { + return null; + } + if (terminalLines.some((line) => line.includes("\r"))) return null; + const parsed = terminalLines.map((line) => JOB_LOG_TIMESTAMPED_LINE_PATTERN.exec(line)); + if (parsed.some((line) => line === null)) return null; + const timestamps = parsed.map((line) => line?.[1] ?? ""); + const timestampSeconds = timestamps.map(jobLogTimestampSecond); + const messages = parsed.map((line) => line?.[2] ?? ""); + if ( + timestampSeconds.some((timestamp) => timestamp === null) || + messages[0] !== shutdownMessage || + messages[1] !== `##[error]${HOSTED_RUNNER_OPERATION_CANCELLED_MESSAGE}` || + messages[2] !== HOSTED_RUNNER_ORPHAN_CLEANUP_MESSAGE || + timestamps[0]! >= timestamps[1]! || + timestamps.slice(1).some((timestamp, index) => timestamp < timestamps[index]!) + ) { + return null; + } + const orphanProcesses = messages + .slice(3) + .map((message) => JOB_LOG_ORPHAN_PROCESS_PATTERN.exec(message)); + const orphanProcessIds = orphanProcesses.map((process) => process?.[1] ?? ""); + if ( + orphanProcesses.some((process) => process === null) || + new Set(orphanProcessIds).size !== orphanProcessIds.length + ) { + return null; + } + return { + shutdownTimestamp: timestamps[0]!, + operationTimestamp: timestamps[1]!, + cleanupTimestamp: timestamps[2]!, + lastTimestamp: timestamps.at(-1)!, + }; +} + +function isBoundedWorkflowJobLogEvidence(evidence: WorkflowJobLogEvidence): boolean { + const tailBytes = Buffer.byteLength(evidence.tail, "utf8"); + return ( + /^"[^"\r\n]{1,128}"$/u.test(evidence.etag) && + Number.isSafeInteger(evidence.totalBytes) && + evidence.totalBytes > 0 && + tailBytes > 0 && + tailBytes <= evidence.totalBytes && + tailBytes <= MAX_RUNNER_LOSS_JOB_LOG_TAIL_BYTES + ); +} + +function hasTrustedHostedRunnerShutdownLog( + job: WorkflowJob, + repository: string, + workflowSha: string, +): boolean { + const evidence = job.logEvidence; + if ( + !evidence || + !isBoundedWorkflowJobLogEvidence(evidence) || + !hasCompatibleHostedRunnerShutdownAnnotations(job, repository, workflowSha) + ) { + return false; + } + const marker = parseHostedRunnerShutdownLogTail(evidence.tail); + const cancelledSteps = job.steps.filter( + (step) => step.status === "completed" && step.conclusion === "cancelled", + ); + const cancelledStep = cancelledSteps[0]; + if ( + !marker || + cancelledSteps.length !== 1 || + !job.startedAt || + !job.completedAt || + !cancelledStep?.startedAt || + !cancelledStep.completedAt + ) { + return false; + } + const shutdownSecond = jobLogTimestampSecond(marker.shutdownTimestamp); + const operationSecond = jobLogTimestampSecond(marker.operationTimestamp); + const cleanupSecond = jobLogTimestampSecond(marker.cleanupTimestamp); + const lastSecond = jobLogTimestampSecond(marker.lastTimestamp); + return ( + shutdownSecond !== null && + operationSecond !== null && + cleanupSecond !== null && + lastSecond !== null && + job.startedAt <= cancelledStep.startedAt && + cancelledStep.startedAt <= shutdownSecond && + operationSecond === cancelledStep.completedAt && + cancelledStep.completedAt <= cleanupSecond && + cleanupSecond <= lastSecond && + lastSecond <= job.completedAt + ); +} + function hasTrustedHostedRunnerLossStepShape(job: WorkflowJob): boolean { if ( job.status !== "completed" || @@ -2166,7 +2572,8 @@ function hasTrustedHostedRunnerLossMarker( ): boolean { return ( hasTrustedHostedRunnerLossStepShape(job) && - hasTrustedHostedRunnerLossAnnotation(job, repository, workflowSha) + (hasTrustedHostedRunnerLossAnnotation(job, repository, workflowSha) || + hasTrustedHostedRunnerShutdownLog(job, repository, workflowSha)) ); } @@ -2536,6 +2943,55 @@ export function assertCorrelatedWorkflowRun( } } +async function requireUnchangedCompletedWorkflowRun( + repository: string, + token: string, + child: WorkflowRun, + identity: WorkflowRunIdentity, +): Promise { + const confirmed = await githubApi( + `repos/${repository}/actions/runs/${identity.childRunId}`, + token, + { userAgent: USER_AGENT }, + ); + assertCorrelatedWorkflowRun(confirmed, identity); + if ( + confirmed.status !== "completed" || + confirmed.status !== child.status || + confirmed.conclusion !== child.conclusion || + confirmed.workflow_id !== child.workflow_id + ) { + throw new Error("E2E run changed while its hosted-runner-loss evidence was authenticated"); + } +} + +function workflowJobEvidenceFingerprint(details: { + jobs: readonly WorkflowJob[]; + complete: boolean; +}): string { + const jobs = [...details.jobs] + .sort((left, right) => left.id - right.id) + .map((job) => { + const { annotations, logEvidence, ...metadata } = job; + return { + ...metadata, + ...(annotations === undefined + ? {} + : { annotations: annotations.map((annotation) => JSON.stringify(annotation)).sort() }), + ...(logEvidence === undefined + ? {} + : { + logEvidence: { + etag: logEvidence.etag, + totalBytes: logEvidence.totalBytes, + tailHash: sha256(logEvidence.tail), + }, + }), + }; + }); + return sha256(JSON.stringify({ complete: details.complete, jobs })); +} + export async function dispatchPrGate(options: { repository: string; token: string; @@ -2991,6 +3447,14 @@ export async function retryRunnerLossPrGate( const jobDetails = await listNonPassingWorkflowJobs(repository, token, command.childRunId, 1, { includeAnnotations: true, }); + await requireUnchangedCompletedWorkflowRun(repository, token, child, { + childRunId: command.childRunId, + correlationId: state.correlationId, + prNumber: state.prNumber, + repository, + workflowSha: state.workflowSha, + }); + const jobEvidenceFingerprint = workflowJobEvidenceFingerprint(jobDetails); const runnerLossEvidence = verifiedRunnerLossEvidence({ repository, workflowSha: state.workflowSha, @@ -3021,6 +3485,43 @@ export async function retryRunnerLossPrGate( throw new Error("runner-loss retry requires an internal pull request"); } + const confirmedJobDetails = await listNonPassingWorkflowJobs( + repository, + token, + command.childRunId, + 1, + { includeAnnotations: true }, + ); + await requireUnchangedCompletedWorkflowRun(repository, token, child, { + childRunId: command.childRunId, + correlationId: state.correlationId, + prNumber: state.prNumber, + repository, + workflowSha: state.workflowSha, + }); + if (workflowJobEvidenceFingerprint(confirmedJobDetails) !== jobEvidenceFingerprint) { + throw new Error("hosted-runner-loss evidence changed before retry dispatch"); + } + const confirmedRunnerLossEvidence = verifiedRunnerLossEvidence({ + repository, + workflowSha: state.workflowSha, + workflowConclusion: child.conclusion, + jobs: confirmedJobDetails.jobs, + jobDetailsAvailable: true, + jobDetailsComplete: confirmedJobDetails.complete, + }); + const confirmedRetryDecision = confirmedRunnerLossEvidence + ? decideRetry({ + runnerLoss: detectRunnerLoss(confirmedRunnerLossEvidence), + classification: null, + attempt: 1, + }) + : { retry: false, reason: "runner-loss evidence is incomplete" }; + if (!confirmedRetryDecision.retry) { + throw new Error( + `runner-loss retry lost authorization before dispatch: ${confirmedRetryDecision.reason}`, + ); + } const currentPull = await requireLiveExactDiff({ repository, token, @@ -3723,6 +4224,13 @@ export async function finishPrGate(options: { const details = await listNonPassingWorkflowJobs(repository, token, options.childRunId, 1, { includeAnnotations: true, }); + await requireUnchangedCompletedWorkflowRun(repository, token, child, { + childRunId: options.childRunId, + correlationId: state.correlationId, + prNumber: state.prNumber, + repository, + workflowSha: state.workflowSha, + }); jobs = details.jobs; jobDetailsComplete = details.complete; } catch (error) { From 97e22517048dc5d5179e150e0b45c4f06ac6fae5 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 02:16:21 -0700 Subject: [PATCH 14/20] test(ci): exercise runner shutdown retry transport Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-runner-loss-retry.test.ts | 273 +++++++++++++++++++++ 1 file changed, 273 insertions(+) diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index deb9edfa795..42960b20a67 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -28,6 +28,12 @@ const WORKFLOW_SHA = "d".repeat(40); const ORIGINAL_CORRELATION_ID = "12345678-1234-4123-8123-123456789abc"; const ORIGINAL_RUN_URL = "https://github.com/NVIDIA/NemoClaw/actions/runs/23"; const RETRY_MARKER = ""; +const JOB_LOG_DOWNLOAD_URL = + "https://productionresultssa0.blob.core.windows.net/actions-results/job-89074697099.txt?sp=r&sig=signed"; +const JOB_LOG_ETAG = '"hosted-runner-log"'; +const JOB_LOG_RANGE_BYTES = 64 * 1024; +const RUNNER_SHUTDOWN_MESSAGE = + "The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled."; afterEach(() => { vi.restoreAllMocks(); @@ -114,6 +120,8 @@ function hostedRunnerLossJob(runId = 23) { name: "Hermes security-posture", status: "completed", conclusion: "failure", + started_at: "2026-07-23T07:26:56Z", + completed_at: "2026-07-23T07:32:54Z", runner_id: 1_021_277_393, runner_name: "GitHub Actions 1021277393", runner_group_id: 0, @@ -125,6 +133,8 @@ function hostedRunnerLossJob(runId = 23) { name: "Run security posture live Vitest test", status: "completed", conclusion: "cancelled", + started_at: "2026-07-23T07:27:43Z", + completed_at: "2026-07-23T07:32:49Z", }, { name: "Upload security posture artifacts", status: "completed", conclusion: "skipped" }, { name: "Clean up Docker auth", status: "completed", conclusion: "skipped" }, @@ -149,6 +159,88 @@ function runnerLossAnnotation() { }; } +function genericCancellationAnnotation() { + return { + ...runnerLossAnnotation(), + start_line: 34, + end_line: 34, + message: "The operation was canceled.", + }; +} + +function runnerShutdownJobLog() { + return [ + "x".repeat(JOB_LOG_RANGE_BYTES), + `2026-07-23T07:32:47.0261924Z ##[error]${RUNNER_SHUTDOWN_MESSAGE}`, + "2026-07-23T07:32:49.9360750Z ##[error]The operation was canceled.", + "2026-07-23T07:32:50.0577487Z Cleaning up orphan processes", + "", + ].join("\n"); +} + +type JobLogFixture = { + body?: string; + downloadUrl?: string; + metadataHeaders?: Record; + rangeHeaders?: Record; + redirectStatus?: number; +}; + +function jobLogRoutes(options: JobLogFixture = {}) { + const body = options.body ?? runnerShutdownJobLog(); + const bytes = new TextEncoder().encode(body); + const rangeStart = Math.max(0, bytes.byteLength - JOB_LOG_RANGE_BYTES); + const rangeEnd = bytes.byteLength - 1; + const range = bytes.slice(rangeStart); + const downloadUrl = options.downloadUrl ?? JOB_LOG_DOWNLOAD_URL; + return [ + githubFetchRoute( + ({ url, method }) => + url.endsWith(`/actions/jobs/${hostedRunnerLossJob().id}/logs`) && method === "GET", + () => + new Response(null, { + status: options.redirectStatus ?? 302, + headers: { location: downloadUrl }, + }), + ), + githubFetchRoute( + ({ url, method }) => url === JOB_LOG_DOWNLOAD_URL && method === "HEAD", + () => + new Response(null, { + status: 200, + headers: { + "content-length": String(bytes.byteLength), + "content-type": "text/plain", + etag: JOB_LOG_ETAG, + ...options.metadataHeaders, + }, + }), + ), + githubFetchRoute( + ({ url, method }) => url === JOB_LOG_DOWNLOAD_URL && method === "GET", + () => + new Response(range, { + status: 206, + headers: { + "content-length": String(range.byteLength), + "content-range": `bytes ${rangeStart}-${rangeEnd}/${bytes.byteLength}`, + "content-type": "text/plain", + etag: JOB_LOG_ETAG, + ...options.rangeHeaders, + }, + }), + ), + ]; +} + +function authenticatedShutdownOptions(jobLog: JobLogFixture = {}) { + const annotations = [genericCancellationAnnotation()]; + return { + annotationPages: [annotations, annotations], + jobLog, + }; +} + function workflowJobCheckRun(job: ReturnType) { const annotationsUrl = `${job.check_run_url}/annotations`; return { @@ -235,6 +327,7 @@ function retryRoutes( workflow?: unknown; jobCheck?: unknown; annotationPages?: unknown[][]; + jobLog?: JobLogFixture; createRetryStateDirectory?: string; } = {}, ) { @@ -286,6 +379,7 @@ function retryRoutes( return githubResponse(annotations); }, ), + ...jobLogRoutes(options.jobLog), githubFetchRoute( ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", () => githubResponse(pullRequest()), @@ -412,6 +506,61 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { } }); + it("dispatches once for an authenticated hosted-runner shutdown log", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockImplementation(retryRoutes(requests, authenticatedShutdownOptions())); + + try { + await expect(retryRunnerLossPrGate(context.command)).resolves.toBeUndefined(); + expect(requests.filter((request) => request.url.endsWith("/dispatches"))).toHaveLength(1); + expect(requests.filter((request) => request.url.endsWith("/actions/runs/23"))).toHaveLength( + 3, + ); + + const calls = fetchSpy.mock.calls.map(([input, init]) => ({ + url: input instanceof Request ? input.url : String(input), + init, + })); + const apiLogCalls = calls.filter((call) => + call.url.endsWith(`/actions/jobs/${hostedRunnerLossJob().id}/logs`), + ); + const metadataCalls = calls.filter( + (call) => call.url === JOB_LOG_DOWNLOAD_URL && call.init?.method === "HEAD", + ); + const rangeCalls = calls.filter( + (call) => call.url === JOB_LOG_DOWNLOAD_URL && call.init?.method === undefined, + ); + expect(apiLogCalls).toHaveLength(2); + expect(metadataCalls).toHaveLength(2); + expect(rangeCalls).toHaveLength(2); + expect(apiLogCalls[0]?.init).toMatchObject({ + redirect: "manual", + headers: { Authorization: "Bearer token" }, + }); + expect(metadataCalls[0]?.init).toMatchObject({ + method: "HEAD", + redirect: "error", + headers: { "Accept-Encoding": "identity" }, + }); + const totalBytes = new TextEncoder().encode(runnerShutdownJobLog()).byteLength; + expect(rangeCalls[0]?.init).toMatchObject({ + redirect: "error", + headers: { + "If-Match": JOB_LOG_ETAG, + Range: `bytes=${totalBytes - JOB_LOG_RANGE_BYTES}-${totalBytes - 1}`, + }, + }); + for (const call of [...metadataCalls, ...rangeCalls]) { + expect(call.init?.headers).not.toHaveProperty("Authorization"); + } + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + it("closes the reserved retry check for child reruns and mixed non-passing jobs", async () => { for (const scenario of ["child-rerun", "mixed-jobs"] as const) { const context = setup(); @@ -588,6 +737,53 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { }, error: /not authorized/u, }, + { + label: "an untrusted job-log redirect", + options: () => + authenticatedShutdownOptions({ + downloadUrl: "https://example.com/actions-results/job.txt?sig=untrusted", + }), + error: /not authorized/u, + }, + { + label: "a weak job-log metadata ETag", + options: () => + authenticatedShutdownOptions({ metadataHeaders: { etag: 'W/"hosted-runner-log"' } }), + error: /not authorized/u, + }, + { + label: "a mismatched authenticated job-log range", + options: () => + authenticatedShutdownOptions({ + rangeHeaders: { "content-range": "bytes 0-1/2" }, + }), + error: /not authorized/u, + }, + { + label: "a completed-step marker after the shutdown tail", + options: () => + authenticatedShutdownOptions({ + body: `${runnerShutdownJobLog()}2026-07-23T07:32:51.0000000Z ##[end-action id=__self.__run;outcome=cancelled;conclusion=cancelled;duration_ms=1]\n`, + }), + error: /not authorized/u, + }, + { + label: "shutdown evidence that changes before dispatch", + options: () => ({ + ...authenticatedShutdownOptions(), + annotationPages: [ + [genericCancellationAnnotation()], + [ + { + ...genericCancellationAnnotation(), + start_line: 35, + end_line: 35, + }, + ], + ], + }), + error: /evidence changed/u, + }, ])("fails closed after reservation for $label", async ({ options, error }) => { const context = setup(); const requests: RecordedGitHubRequest[] = []; @@ -855,6 +1051,83 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { } }); + it("writes the retry authorization marker for an authenticated shutdown log", async () => { + const context = setup(); + const requests: RecordedGitHubRequest[] = []; + const currentCheck = checkRun(17, { + status: "in_progress", + conclusion: null, + output: { title: "Running 2 E2E checks", summary: "Attempt 1 is running." }, + }); + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", + () => githubResponse(workflowRun()), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => githubResponse({ total_count: 1, check_runs: [currentCheck] }), + ), + githubFetchRoute( + ({ url, method }) => + url.includes("/actions/runs/23/attempts/1/jobs?") && method === "GET", + () => githubResponse({ total_count: 1, jobs: [hostedRunnerLossJob()] }), + ), + githubFetchRoute( + ({ url, method }) => + url.endsWith(`/check-runs/${hostedRunnerLossJob().id}`) && method === "GET", + () => githubResponse(workflowJobCheckRun(hostedRunnerLossJob())), + ), + githubFetchRoute( + ({ url, method }) => + url.includes(`/check-runs/${hostedRunnerLossJob().id}/annotations?`) && + method === "GET", + () => githubResponse([genericCancellationAnnotation()]), + ), + ...jobLogRoutes(), + githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(pullRequest()), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", + (request) => mutationResponse(request, 17), + ), + ], + requests, + ), + ); + + try { + await expect( + finishPrGate({ + statePath: context.statePath, + stateHash: sha256(context.serializedState), + evidencePath: context.workDir, + checkRunId: 17, + childRunId: 23, + evidenceOutcome: "skipped", + }), + ).resolves.toBeUndefined(); + expect(fs.readFileSync(context.outputPath, "utf8")).toBe( + "runner_loss_retry_authorized=true\nfinalized=true\n", + ); + const completion = requests.find( + (request) => + request.url.endsWith("/check-runs/17") && + request.method === "PATCH" && + (request.body as { status?: string }).status === "completed", + ); + expect(completion?.body).toMatchObject({ conclusion: "failure" }); + expect(JSON.stringify(completion?.body)).toContain(RETRY_MARKER); + } finally { + fs.rmSync(context.workDir, { recursive: true, force: true }); + } + }); + it.each([ { label: "loses another hosted runner", conclusion: "failure", evidenceOutcome: "skipped" }, { label: "cannot download evidence", conclusion: "success", evidenceOutcome: "failure" }, From 2871cdb5404a2c22a7c85cd2c22bcf1101242e74 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 02:20:52 -0700 Subject: [PATCH 15/20] docs(ci): document authenticated runner shutdowns Signed-off-by: Apurv Kumaria --- test/e2e/README.md | 59 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/test/e2e/README.md b/test/e2e/README.md index 1bb49cfe49d..346bf697f27 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -424,22 +424,55 @@ minutes to finish. A first-attempt controller may dispatch one replacement run when the first child fails because a standard GitHub-hosted `ubuntu-latest` runner lost communication. The Jobs API response must identify the exact run, attempt, workflow commit, job check, standard hosted runner group, and runner -name. Its exact check-run annotation must contain one canonical runner-loss -failure bound to `.github` at that workflow commit. Generic cancellation, -timeout, unknown runner identity, self-hosted or custom runner groups, an -ordinary failed step, another non-passing job, incomplete pagination, or -mismatched annotation identity all fail closed without a retry. +name. The controller accepts one canonical runner-loss failure annotation bound +to `.github` at that workflow commit. + +When GitHub emits a generic cancellation instead, the controller requires +exactly one failure annotation whose message is `The operation was canceled.` +The annotation must use `.github`, equal start and end lines, null columns, and +empty title and detail fields. Every annotation must use a blob URL bound to the +same workflow commit. This permits a trusted non-failure notice beside the sole +failure annotation. + +The generic-cancellation fallback also authenticates the job log. The +controller requests the GitHub job-log endpoint and accepts only its signed +HTTPS redirect to GitHub Actions result storage. It does not forward the +repository token to that signed URL. A metadata request must return plain, +unencoded text with a strong bounded ETag and an exact content length. The +controller then reads at most the final 64 KiB with `If-Match` and requires an +exact partial-content range, length, and matching ETag. + +The authenticated tail must end with exactly one line feed after the +timestamped shutdown error, operation-cancelled error, and orphan-cleanup +record, in that order. Up to 64 unique orphan-process termination records may +follow the cleanup record. Each record must contain a positive process ID and a +bounded process name. The record timestamps must not move backward. The +job must start no later than the cancelled step. The shutdown must occur at or +after that step starts, and the cancellation second must equal the step's +completion time. Cleanup must not precede cancellation. Cleanup and +orphan-process records must finish no later than the job completion time. + +A generic cancellation without this log contract, timeout, unknown runner +identity, self-hosted or custom runner group, ordinary failed step, another +non-passing job, incomplete pagination, or mismatched annotation identity +fails closed without a retry. Before the one-time retry dispatch, the controller revalidates the unchanged internal PR head and base, original child, current coordination-check lineage, -trusted runner-loss evidence, and deterministic plan. It reserves a distinct -replacement coordination check before dispatch so the native observer can -follow the retry without mutating completed attempt-one history. Attempt two -uses separate private state and evidence paths. Its result is terminal: it can -never authorize another automatic retry. If the retry controller stops before -or after reservation, its always-run cleanup removes the retry authorization -from the source or closes the reserved replacement so no retryable or active -check is left behind. +trusted runner-loss evidence, and deterministic plan. It reads the complete +job, annotation, and optional log evidence twice. It confirms the unchanged +completed child after each read and requires identical evidence fingerprints, +including pagination state, log ETag, log length, and log-tail hash. After the +second classification, it validates the live PR head and base and the current +coordination-check lineage again. + +The controller reserves a distinct replacement coordination check before +dispatch so the native observer can follow the retry without mutating completed +attempt-one history. Attempt two uses separate private state and evidence paths. +Its result is terminal and cannot authorize another automatic retry. If the +retry controller stops before or after reservation, its always-run cleanup +removes the retry authorization from the source or closes the reserved +replacement. This prevents a retryable or active check from remaining. Each evidence download has its own 10-minute limit and 30-second process-kill grace. Two 140-minute waits plus both download windows consume 301 minutes, From f14bf640fbebfc83abcc683b01dee51c501f6ce6 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 02:32:38 -0700 Subject: [PATCH 16/20] fix(ci): terminalize cancelled retry setup Signed-off-by: Apurv Kumaria --- .github/workflows/pr-e2e-gate.yaml | 4 ++-- test/pr-e2e-gate-workflow.test.ts | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pr-e2e-gate.yaml b/.github/workflows/pr-e2e-gate.yaml index 951e93cd5eb..dd1c944418b 100644 --- a/.github/workflows/pr-e2e-gate.yaml +++ b/.github/workflows/pr-e2e-gate.yaml @@ -377,7 +377,7 @@ jobs: --run-id "${{ steps.retry.outputs.run_id }}" - name: Terminalize interrupted retry setup - if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && steps.retry.outcome == 'failure' && steps.retry.outputs.check_id == '' }} + if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && steps.retry.outcome != 'success' && steps.retry.outputs.check_id == '' }} env: GITHUB_TOKEN: ${{ github.token }} run: >- @@ -581,7 +581,7 @@ jobs: --run-id "${{ steps.retry.outputs.run_id }}" - name: Terminalize interrupted approved retry setup - if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && steps.retry.outcome == 'failure' && steps.retry.outputs.check_id == '' }} + if: ${{ always() && steps.finish.outputs.runner_loss_retry_authorized == 'true' && steps.retry.outcome != 'success' && steps.retry.outputs.check_id == '' }} env: GITHUB_TOKEN: ${{ github.token }} run: >- diff --git a/test/pr-e2e-gate-workflow.test.ts b/test/pr-e2e-gate-workflow.test.ts index 4db079a7391..0d4cd0790fe 100644 --- a/test/pr-e2e-gate-workflow.test.ts +++ b/test/pr-e2e-gate-workflow.test.ts @@ -614,7 +614,8 @@ describe("PR E2E gate workflow", () => { expect(retryFinish.run).toContain('--state-hash "${{ steps.retry.outputs.state_hash }}"'); expect(retryFinish.run).toContain('--evidence-outcome "${{ steps.retry_evidence.outcome }}"'); const interruptedRetry = step(coordinate, "Terminalize interrupted retry setup"); - expect(interruptedRetry.if).toContain("steps.retry.outcome == 'failure'"); + expect(interruptedRetry.if).toContain("steps.retry.outcome != 'success'"); + expect(interruptedRetry.if).not.toContain("steps.retry.outcome == 'failure'"); expect(interruptedRetry.if).toContain("steps.retry.outputs.check_id == ''"); expect(interruptedRetry.run).toContain("--mode abandon-runner-loss-retry"); const approval = step(approveForkSkip, "Record approved credentialed E2E skip"); @@ -866,9 +867,14 @@ describe("PR E2E gate workflow", () => { ); expect(step(approveInternal, "Verify approved retry evidence").if).toContain("always()"); expect(step(approveInternal, "Close incomplete approved retry check").if).toContain("always()"); - expect(step(approveInternal, "Terminalize interrupted approved retry setup").run).toContain( - "--mode abandon-runner-loss-retry", + const approvedInterruptedRetry = step( + approveInternal, + "Terminalize interrupted approved retry setup", ); + expect(approvedInterruptedRetry.if).toContain("steps.retry.outcome != 'success'"); + expect(approvedInterruptedRetry.if).not.toContain("steps.retry.outcome == 'failure'"); + expect(approvedInterruptedRetry.if).toContain("steps.retry.outputs.check_id == ''"); + expect(approvedInterruptedRetry.run).toContain("--mode abandon-runner-loss-retry"); expect(step(approveInternal, "Close incomplete approved check").if).toContain("always()"); expect(step(approveInternal, "Remove private workspace").if).toContain("always()"); }); From ef76d6c7048f3e584a4d20714392e093d2e4cb20 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 02:33:19 -0700 Subject: [PATCH 17/20] fix(ci): bound runner loss annotations Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate-runner-loss-retry.test.ts | 37 ++++++++++++++++++---- tools/e2e/pr-e2e-gate.mts | 28 +++++++++++++--- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index 42960b20a67..91f3b9ea368 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -703,23 +703,46 @@ describe("PR E2E one-time hosted-runner-loss retry", () => { error: /annotation listing is incomplete/u, }, { - label: "overlapping annotation pages", + label: "an annotation count above the runner-loss evidence limit", options: () => { const check = workflowJobCheckRun(hostedRunnerLossJob()); - const notices = Array.from({ length: 99 }, (_, index) => ({ + return { + jobCheck: { ...check, output: { ...check.output, annotations_count: 21 } }, + }; + }, + error: /annotation count exceeds/u, + }, + { + label: "an oversized annotation field", + options: () => ({ + annotationPages: [ + [ + { + ...runnerLossAnnotation(), + message: "x".repeat(16 * 1024 + 1), + }, + ], + ], + }), + error: /invalid workflow job annotation/u, + }, + { + label: "oversized aggregate annotation evidence", + options: () => { + const check = workflowJobCheckRun(hostedRunnerLossJob()); + const notices = Array.from({ length: 19 }, (_, index) => ({ ...runnerLossAnnotation(), start_line: index + 2, end_line: index + 2, annotation_level: "notice", - message: `notice ${index + 1}`, + message: `notice-${index}-${"x".repeat(4 * 1024)}`, })); - const firstPage = [runnerLossAnnotation(), ...notices]; return { - jobCheck: { ...check, output: { ...check.output, annotations_count: 101 } }, - annotationPages: [firstPage, [notices[0]!]], + jobCheck: { ...check, output: { ...check.output, annotations_count: 20 } }, + annotationPages: [[runnerLossAnnotation(), ...notices]], }; }, - error: /duplicate workflow job annotations/u, + error: /annotation evidence exceeds/u, }, { label: "a second generic-cancellation failure annotation", diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 5846e7dd39e..3a0e6fe6e38 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -81,7 +81,11 @@ const MAX_PR_FILES = 3000; const MAX_COMPATIBILITY_FILES = 300; const MAX_ACTIVE_RUN_PAGES_PER_STATUS = 10; const MAX_WORKFLOW_JOB_PAGES = 10; -const MAX_JOB_ANNOTATION_PAGES = 10; +const MAX_JOB_ANNOTATION_PAGES = 1; +const MAX_RUNNER_LOSS_JOB_ANNOTATIONS = 20; +const MAX_JOB_ANNOTATION_IDENTITY_BYTES = 8 * 1024; +const MAX_JOB_ANNOTATION_TEXT_BYTES = 16 * 1024; +const MAX_RUNNER_LOSS_JOB_ANNOTATION_BYTES = 64 * 1024; const HOSTED_RUNNER_LOST_COMMUNICATION_MESSAGE = "The hosted runner lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error."; const HOSTED_RUNNER_SHUTDOWN_MESSAGE = @@ -1801,7 +1805,9 @@ function validateWorkflowJobAnnotation(value: unknown): WorkflowJobAnnotation { !isObjectRecord(value) || typeof value.path !== "string" || value.path.length === 0 || + Buffer.byteLength(value.path, "utf8") > MAX_JOB_ANNOTATION_IDENTITY_BYTES || typeof value.blob_href !== "string" || + Buffer.byteLength(value.blob_href, "utf8") > MAX_JOB_ANNOTATION_IDENTITY_BYTES || !Number.isSafeInteger(value.start_line) || (value.start_line as number) < 1 || (value.start_column !== null && @@ -1811,9 +1817,13 @@ function validateWorkflowJobAnnotation(value: unknown): WorkflowJobAnnotation { (value.end_column !== null && (!Number.isSafeInteger(value.end_column) || (value.end_column as number) < 1)) || typeof value.annotation_level !== "string" || + Buffer.byteLength(value.annotation_level, "utf8") > MAX_JOB_ANNOTATION_IDENTITY_BYTES || typeof value.title !== "string" || + Buffer.byteLength(value.title, "utf8") > MAX_JOB_ANNOTATION_TEXT_BYTES || typeof value.message !== "string" || - typeof value.raw_details !== "string" + Buffer.byteLength(value.message, "utf8") > MAX_JOB_ANNOTATION_TEXT_BYTES || + typeof value.raw_details !== "string" || + Buffer.byteLength(value.raw_details, "utf8") > MAX_JOB_ANNOTATION_TEXT_BYTES ) { throw new Error("GitHub returned an invalid workflow job annotation"); } @@ -1879,15 +1889,19 @@ async function listWorkflowJobAnnotations( throw new Error("workflow job check run does not match the exact failed job"); } const expectedCount = check.output.annotations_count as number; + if (expectedCount > MAX_RUNNER_LOSS_JOB_ANNOTATIONS) { + throw new Error("workflow job annotation count exceeds the hosted-runner-loss limit"); + } const annotations: WorkflowJobAnnotation[] = []; const fingerprints = new Set(); + let annotationBytes = 0; for (let page = 1; page <= MAX_JOB_ANNOTATION_PAGES; page += 1) { const value = await githubApi( - `repos/${repository}/check-runs/${job.id}/annotations?per_page=100&page=${page}`, + `repos/${repository}/check-runs/${job.id}/annotations?per_page=${MAX_RUNNER_LOSS_JOB_ANNOTATIONS}&page=${page}`, token, { userAgent: USER_AGENT }, ); - if (!Array.isArray(value) || value.length > 100) { + if (!Array.isArray(value) || value.length > MAX_RUNNER_LOSS_JOB_ANNOTATIONS) { throw new Error("GitHub returned an invalid workflow job annotation listing"); } const pageAnnotations = value.map(validateWorkflowJobAnnotation); @@ -1897,13 +1911,17 @@ async function listWorkflowJobAnnotations( throw new Error("GitHub returned duplicate workflow job annotations"); } fingerprints.add(fingerprint); + annotationBytes += Buffer.byteLength(fingerprint, "utf8"); + if (annotationBytes > MAX_RUNNER_LOSS_JOB_ANNOTATION_BYTES) { + throw new Error("workflow job annotation evidence exceeds its byte limit"); + } annotations.push(annotation); } if (annotations.length > expectedCount) { throw new Error("workflow job annotation listing exceeds the trusted annotation count"); } if (annotations.length === expectedCount) return annotations; - if (value.length < 100) { + if (value.length < MAX_RUNNER_LOSS_JOB_ANNOTATIONS) { throw new Error("workflow job annotation listing is incomplete"); } } From 697d78e5fbcbeac6f6679ff733bafbf51151013a Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 02:34:20 -0700 Subject: [PATCH 18/20] docs(ci): document runner loss evidence bounds Signed-off-by: Apurv Kumaria --- test/e2e/README.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/e2e/README.md b/test/e2e/README.md index 346bf697f27..db2270219dc 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -431,8 +431,10 @@ When GitHub emits a generic cancellation instead, the controller requires exactly one failure annotation whose message is `The operation was canceled.` The annotation must use `.github`, equal start and end lines, null columns, and empty title and detail fields. Every annotation must use a blob URL bound to the -same workflow commit. This permits a trusted non-failure notice beside the sole -failure annotation. +same workflow commit. The controller accepts at most 20 annotations, bounds +each text field, and limits the normalized annotation evidence to 64 KiB. This +permits trusted bounded non-failure notices beside the sole failure annotation +without allowing annotation output to exhaust the coordinator. The generic-cancellation fallback also authenticates the job log. The controller requests the GitHub job-log endpoint and accepts only its signed @@ -470,9 +472,10 @@ The controller reserves a distinct replacement coordination check before dispatch so the native observer can follow the retry without mutating completed attempt-one history. Attempt two uses separate private state and evidence paths. Its result is terminal and cannot authorize another automatic retry. If the -retry controller stops before or after reservation, its always-run cleanup -removes the retry authorization from the source or closes the reserved -replacement. This prevents a retryable or active check from remaining. +retry setup fails, is cancelled, or is skipped before reservation, its +always-run cleanup removes the retry authorization from the source. If it stops +after reservation, cleanup closes the reserved replacement. This prevents a +retryable or active check from remaining. Each evidence download has its own 10-minute limit and 30-second process-kill grace. Two 140-minute waits plus both download windows consume 301 minutes, From ac430dcf735d9948c0632f0f4ca0b9d1bf9b1613 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 02:45:13 -0700 Subject: [PATCH 19/20] test(ci): scope newer-base immutability assertion Signed-off-by: Apurv Kumaria --- test/pr-e2e-gate.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index a6a2716d8f5..5d89dbc3246 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -776,7 +776,12 @@ describe("PR E2E controller", () => { status: "in_progress", }, }); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); + expect( + requests.some( + (request) => + request.url.endsWith("/check-runs/18") && !["GET", "HEAD"].includes(request.method), + ), + ).toBe(false); }); it("rejects a seeded identity claimed by another GitHub App", async () => { From d1f82472863175ed1fcde56d9a3445c135a078f6 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 23 Jul 2026 03:15:38 -0700 Subject: [PATCH 20/20] docs(ci): define runner fallback lifecycle Signed-off-by: Apurv Kumaria --- test/e2e/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/README.md b/test/e2e/README.md index db2270219dc..24344d693ac 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -436,6 +436,14 @@ each text field, and limits the normalized annotation evidence to 64 KiB. This permits trusted bounded non-failure notices beside the sole failure annotation without allowing annotation output to exhaust the coordinator. +GitHub Actions creates this `.github` failure annotation after the hosted runner +shuts down; NemoClaw workflow code cannot replace its generic message with the +canonical lost-communication annotation. The classifier test `accepts the +exact authenticated terminal shutdown block from run 29988226653` preserves +the observed fallback contract. Remove the fallback and that test together +only after GitHub's documented Jobs or Checks API contract provides an +authenticated structured runner-loss reason for this exact shutdown path. + The generic-cancellation fallback also authenticates the job log. The controller requests the GitHub job-log endpoint and accepts only its signed HTTPS redirect to GitHub Actions result storage. It does not forward the