From 385f598b0ed794924b580bad32d851aed8e19908 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Sun, 26 Jul 2026 22:40:38 -0700 Subject: [PATCH 1/4] ci(e2e): publish direct PR gate check Signed-off-by: Charan Jagwani --- .../nemoclaw-maintainer-day/MERGE-GATE.md | 54 +- .../scripts/check-gates.ts | 35 +- .github/workflows/e2e.yaml | 2 +- .github/workflows/pr-e2e-gate.yaml | 42 - test/e2e/README.md | 188 ++--- test/e2e/docs/README.md | 40 +- .../e2e-operations-workflow-boundary.test.ts | 2 +- test/helpers/vitest-watch-triggers.ts | 2 +- test/maintainer-skills-policy.test.ts | 2 +- test/pr-e2e-gate-dispatch-recovery.test.ts | 2 +- test/pr-e2e-gate-fork-approval.test.ts | 2 +- test/pr-e2e-gate-lifecycle.test.ts | 2 +- test/pr-e2e-gate-remediation.test.ts | 4 +- test/pr-e2e-gate-retry-history.test.ts | 4 +- ...gate-runner-loss-dispatch-recovery.test.ts | 2 +- test/pr-e2e-gate-runner-loss-retry.test.ts | 2 +- test/pr-e2e-gate-workflow.test.ts | 45 +- test/pr-e2e-gate.test.ts | 4 +- test/pr-e2e-required.test.ts | 765 ------------------ test/pr-risk-plan.test.ts | 1 - test/skills/check-gates-evidence.test.ts | 27 +- test/skills/check-gates-retry-history.test.ts | 15 +- test/skills/check-gates-rollout.test.ts | 34 + test/skills/check-gates-test-fixtures.ts | 20 +- test/vitest-watch-triggers.test.ts | 1 - tools/e2e/operations-workflow-boundary.mts | 2 +- tools/e2e/pr-e2e-gate.mts | 2 +- tools/e2e/pr-e2e-required.mts | 495 ------------ 28 files changed, 241 insertions(+), 1555 deletions(-) delete mode 100644 test/pr-e2e-required.test.ts create mode 100644 test/skills/check-gates-rollout.test.ts delete mode 100644 tools/e2e/pr-e2e-required.mts diff --git a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md index a9479170214..7fa58ec01b9 100644 --- a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md +++ b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md @@ -69,14 +69,10 @@ Required PR workflows must identify the PR number, PR SHA, and base SHA. Its run name records `gate true` and the base SHA. A metadata edit must not create skipped evidence. - Each `pull_request_target` E2E controller run for an open PR must use an immutable `gate true` run name. The run name must identify the PR number, PR SHA, and base SHA. -- Metadata-only edits must keep the `E2E / PR Gate` observer active. - They must not create a new coordination check. -- A closed event must use `E2E / PR Gate (not applicable)` for its skipped observer. - It must not publish the required check name. - GitHub usually associates the run with the PR. For a fork run with an empty association, require the Actions event, workflow path, fork repository, branch, and PR SHA to match the current PR. The PR and installer workflows must also name the exact PR, PR SHA, and base SHA. - The E2E controller must still enclose the trusted coordination check. + The E2E controller must still enclose the trusted required check. - If GitHub omits `headRepository.nameWithOwner`, derive it only from the returned repository name and repository-owner login. Fail closed when those fields are missing, malformed, or contradictory. - Treat an all-skipped `gate false` run from an older workflow version as non-evidence. @@ -85,9 +81,8 @@ Required PR workflows must identify the PR number, PR SHA, and base SHA. ### Controller status and PR status `E2E / PR Gate Controller` reports whether the controller published an outcome. -It can pass while `E2E / PR Gate Coordination` and the required `E2E / PR Gate` job fail. -Use the required `E2E / PR Gate` job as merge evidence. -Use the coordination check for the verdict and evidence links. +It can pass while the required `E2E / PR Gate` custom check fails. +Use the required check as merge evidence and for verdict and evidence links. `Superseded by PR update` and `PR closed — gate no longer applies` cancel checks for a prior SHA. Do not act on those checks. The PR SHA and base SHA still need a successful gate. @@ -95,7 +90,7 @@ The closed-PR outcome also covers a deleted fork repository with no head-reposit ### Retry an E2E gate -Rerun `CI / Pull Request` only when the failed coordination check has a supported retry reason for its gate version. +Rerun `CI / Pull Request` only when the failed required check has a supported retry reason for its gate version. The retry must apply to the PR SHA and base SHA. - `prerequisite-ci` — Rerun CI. Let the controller retry after CI passes. @@ -106,9 +101,9 @@ The retry must apply to the PR SHA and base SHA. Never resubmit the child workflow manually. The controller must recheck the old correlation before it creates a replacement check and fresh correlation. -The controller keeps each completed coordination check as audit history. +The controller keeps each completed required check as audit history. For a retry, it creates an `in_progress` check for the same PR SHA and base SHA. -The controller and observer select the check with the highest ID only when all older duplicates are completed failures with supported retry markers. +The controller selects the check with the highest ID only when all older duplicates are completed failures with supported retry markers. Fail closed for an unexpected app, identity mismatch, duplicate ID, unsupported terminal state, or multiple active checks. Do not retry these terminal failures on the same SHA: @@ -125,13 +120,13 @@ A validated `dispatch-not-observed` receipt on a trusted GitHub Actions check is If a late child, incomplete inventory, or contradiction appears while the old correlation is rechecked, stop and investigate rather than dispatching again. Push a change to create another SHA, and then run CI again. -A passing controller does not override a failing required job. +A passing controller does not override a failing required check. ### Evidence download failures If a selected child passes but `Download evidence` fails, the gate fails closed. This also applies when the step is cancelled or skipped. -The coordination check records `evidence-download`, and the controller fails. +The required check records `evidence-download`, and the controller fails. Inspect the download step before you rerun eligible PR CI. Do not use the successful child by itself as evidence. @@ -142,16 +137,14 @@ In this case, the controller can pass without a retry reason. Malformed or unsafe evidence is a terminal controller error. Schema mismatches, identity mismatches, and traversal-limit errors are also terminal. -The coordination check, required job, and controller must fail closed. +The required check and controller must fail closed. ### Approve credentialed E2E -Use the maintainer workflow when coordination reports either of these states: +Use the maintainer workflow when the required check reports +`Maintainer approval required to run fork E2E`. -- `Maintainer approval required to run E2E` -- `Maintainer approval required to run fork E2E` - -1. Follow the `E2E / PR Gate Controller run ` link in the coordination summary. +1. Follow the `E2E / PR Gate Controller run ` link in the required-check summary. 2. Verify the exact head repository, PR SHA, base SHA, selected jobs and targets, and risk-plan artifact. 3. Select **Run workflow** on `main`. 4. Select `approve-e2e`. @@ -159,15 +152,15 @@ Use the maintainer workflow when coordination reports either of these states: 6. Run the workflow. The first attempt requires the triggering actor to have current `maintain` or `admin` access. -The controller checks the PR number, head repository, PR SHA, base SHA, deterministic plan, matching pending coordination check, compatible `main`, and open PR state. -Immediately before dispatch, it confirms that the PR SHA, base SHA, head repository, and coordination identity still match. +The controller checks the PR number, head repository, PR SHA, base SHA, deterministic plan, matching pending required check, compatible `main`, and open PR state. +Immediately before dispatch, it confirms that the PR SHA, base SHA, head repository, and required-check identity still match. It fails closed if any value changed or does not match. For a fork, the trusted workflow definition comes from `main`; each PR-code checkout uses the reviewed fork repository and exact PR SHA. Before approval, no selected credential-bearing work runs. -If the run-specific authorization response is lost, the controller accepts only an exact persisted child binding and otherwise attempts to revoke coordination before requesting child cancellation. +If the run-specific authorization response is lost, the controller accepts only an exact persisted child binding and otherwise attempts to revoke the required check before requesting child cancellation. -Approval returns coordination to `Running E2E check(s)`. +Approval returns the required check to `Running E2E check(s)`. The gate passes only after the selected jobs and targets return verified passing evidence. Failed, missing, skipped, pending, or mismatched evidence keeps the gate from passing. Approval cannot record success by itself. @@ -183,14 +176,9 @@ The `e2e-control-plane` path group includes these areas: - Non-documentation files under `tools/e2e/` and `test/e2e/`. - Shell and Python support files in those directories. -An internal PR can run automatically when it changes only these files: - -- `.github/workflows/pr-e2e-gate.yaml` -- `tools/e2e/pr-e2e-gate.mts` -- `tools/e2e/pr-e2e-required.mts` - -Another control-plane change waits with `Maintainer approval required to run E2E`. -The gate must not run selected jobs or expose secrets before authorization. +Every internal revision with selected jobs or targets dispatches its deterministic plan after eligible PR CI passes. +This behavior includes all internal E2E control-plane changes. +Internal dispatch does not use `approve-e2e` or a maintainer role check. ### Authorize a typed target @@ -208,6 +196,10 @@ Use this order: 3. Verify that `E2E / PR Gate` is attached to that PR SHA and base SHA. 4. Use the gate checker to find PRs that still need a check. +During rollout, the checker accepts the former `E2E / PR Gate Coordination` +custom-check name only when the current name is absent, and prefers the current +name when both exist. + Do not enable the required context before the producer is ready. GitHub does not create a context for prior runs. If you enable the rule first, open PRs can wait for a status that does not exist. diff --git a/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts b/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts index b1a34e22067..3aeda111c2b 100644 --- a/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts +++ b/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts @@ -316,7 +316,7 @@ interface E2eCoordinationEvidence { valid: boolean | null; startedAt?: number; completedAt?: number; - trustedLegacyCheckId?: number; + trustedCustomCheckId?: number; } const E2E_RETRYABLE_FAILURE_MARKER_PREFIX = "", - title: "Selected E2E jobs failed", - }, - }), - identity.repository, - ), - ).toEqual({ - state: "complete", - result: { conclusion: "failure", title: "Selected E2E jobs failed" }, - }); - }); - - it("carries direct failed-job links into the terminal error message", () => { - const jobUrl = "https://github.com/NVIDIA/NemoClaw/actions/runs/23/job/77"; - const reflectedUrl = "https://github.com/NVIDIA/NemoClaw/actions/runs/23/job/88"; - const classified = classifyCoordinationCheck( - check(undefined, { - conclusion: "failure", - details_url: "https://github.com/NVIDIA/NemoClaw/runs/17", - output: { - summary: [ - "[Selected E2E run 23](https://github.com/NVIDIA/NemoClaw/actions/runs/23) concluded `failure`.", - "Jobs that did not pass:", - `- [hermes-e2e ${reflectedUrl}](${jobUrl}) — concluded \`failure\`.`, - `Reflected prose must not become a link: ${reflectedUrl}`, - "[untrusted](https://example.com/actions/runs/23/job/88)", - ].join("\n"), - title: "hermes-e2e failed", - }, - }), - identity.repository, - ); - - expect(classified).toEqual({ - state: "complete", - result: { - conclusion: "failure", - detailsUrl: "https://github.com/NVIDIA/NemoClaw/runs/17", - logUrls: [jobUrl], - title: "hermes-e2e failed", - }, - }); - const result = classified as { state: "complete"; result: RequiredGateResult }; - expect(formatRequiredGateOutcome(result.result)).toBe( - `conclusion=failure title=hermes-e2e failed logs=${jobUrl}`, - ); - }); - - it("links waiting messages to the trusted coordination job", () => { - expect( - classifyCoordinationCheck( - check(undefined, { - status: "in_progress", - conclusion: null, - details_url: "https://github.com/NVIDIA/NemoClaw/runs/17", - output: { title: "Running 3 E2E jobs" }, - }), - identity.repository, - ), - ).toEqual({ - state: "waiting", - description: "Running 3 E2E jobs", - detailsUrl: "https://github.com/NVIDIA/NemoClaw/runs/17", - logUrls: ["https://github.com/NVIDIA/NemoClaw/runs/17"], - }); - }); - - it("prefers the renamed coordination check without querying the legacy name", async () => { - const urls: string[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => { - const url = String(input); - urls.push(url); - return githubResponse(listing([check()])); - }); - - await expect(findCoordinationCheck(identity)).resolves.toMatchObject({ id: 17 }); - expect(urls).toHaveLength(1); - expect(urls[0]).toContain("E2E%20%2F%20PR%20Gate%20Coordination"); - }); - - it("selects the newest PR/base SHA check after marker-backed immutable history", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue( - githubResponse( - listing([ - check(undefined, { - status: "completed", - conclusion: "failure", - output: { - title: "Selected E2E did not pass", - summary: - "The child run was cancelled.\n\n", - }, - }), - check(undefined, { - id: 18, - status: "in_progress", - conclusion: null, - output: { title: "Maintainer approval required to run fork E2E" }, - }), - ]), - ), - ); - - await expect(findCoordinationCheck(identity)).resolves.toMatchObject({ id: 18 }); - }); - - it("waits only for a dispatch-not-observed failure with a validated receipt", () => { - const receipt = dispatchNotObservedReceiptMarker({ - correlationId: "123e4567-e89b-42d3-a456-426614174000", - workflowSha: "d".repeat(40), - sentAtMs: 1_785_050_400_000, - deadlineAtMs: 1_785_050_445_000, - result: "not-observed", - failureKind: "http", - status: 500, - requestId: "ABCD:1234", - }); - const valid = check(undefined, { - conclusion: "failure", - output: { - title: "Workflow dispatch was not observed", - summary: `No child was observed.\n\n${receipt}\n\n${retryableFailureMarker("dispatch-not-observed")}`, - }, - }); - const missingReceipt = check(undefined, { - conclusion: "failure", - output: { - title: "Workflow dispatch was not observed", - summary: `No child was observed.\n\n${retryableFailureMarker("dispatch-not-observed")}`, - }, - }); - - expect(classifyCoordinationCheck(valid, identity.repository)).toMatchObject({ - state: "waiting", - }); - expect(classifyCoordinationCheck(missingReceipt, identity.repository)).toMatchObject({ - state: "complete", - result: { conclusion: "failure" }, - }); - }); - - 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", - checks: [ - check(undefined, { - status: "completed", - conclusion: "failure", - output: { title: "Unknown controller failure", summary: "No retry marker." }, - }), - check(undefined, { id: 18, status: "in_progress", conclusion: null }), - ], - expectedError: "history contains a non-retryable older check", - }, - { - label: "multiple active current candidates", - checks: [ - check(undefined, { status: "in_progress", conclusion: null }), - check(undefined, { id: 18, status: "in_progress", conclusion: null }), - ], - expectedError: "Multiple active coordination checks exist for one PR/base SHA pair", - }, - ])("rejects PR/base SHA coordination history with $label", async ({ checks, expectedError }) => { - vi.spyOn(globalThis, "fetch").mockResolvedValue(githubResponse(listing(checks))); - - await expect(findCoordinationCheck(identity)).rejects.toThrow(expectedError); - }); - - it("uses the old required-name check during the native-job migration", async () => { - vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => { - const url = String(input); - return githubResponse( - url.includes("Coordination") ? listing([]) : listing([check("E2E / PR Gate")]), - ); - }); - - await expect(findCoordinationCheck(identity)).resolves.toMatchObject({ - name: "E2E / PR Gate", - }); - }); - - it("rejects a PR/base SHA identity claimed by another app", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue( - githubResponse(listing([check(undefined, { app: { id: 1 } })])), - ); - - await expect(findCoordinationCheck(identity)).rejects.toThrow("unexpected GitHub App"); - }); - - it("waits through pending authorization and running states before passing", async () => { - let coordinationQueries = 0; - let clock = 0; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter([ - githubFetchRoute( - ({ url }) => url.includes("/pulls/42"), - () => githubResponse(pullRequest()), - ), - githubFetchRoute( - ({ url }) => url.includes("Coordination"), - () => { - coordinationQueries += 1; - return githubResponse( - listing([ - coordinationQueries === 1 - ? check(undefined, { - status: "in_progress", - conclusion: null, - output: { title: "Maintainer approval required to run fork E2E" }, - }) - : coordinationQueries === 2 - ? check(undefined, { - status: "in_progress", - conclusion: null, - output: { title: "Running 3 E2E jobs" }, - }) - : check(), - ]), - ); - }, - ), - ]), - ); - - await expect( - waitForRequiredGate(identity, { - timeoutMs: 100, - pollIntervalMs: 10, - now: () => clock, - sleep: async (milliseconds) => { - clock += milliseconds; - }, - }), - ).resolves.toMatchObject({ conclusion: "success" }); - expect(coordinationQueries).toBe(3); - }); - - it("waits for a replacement after a retryable coordination failure", async () => { - let coordinationQueries = 0; - let clock = 0; - const retryableFailure = check(undefined, { - conclusion: "failure", - output: { - title: "Selected E2E did not pass", - summary: - "The child run was cancelled.\n\n", - }, - }); - const coordinationListings = [ - listing([retryableFailure]), - listing([ - retryableFailure, - check(undefined, { - id: 18, - status: "in_progress", - conclusion: null, - output: { title: "Running selected E2E jobs" }, - }), - ]), - listing([ - retryableFailure, - check(undefined, { - id: 18, - output: { title: "All selected E2E jobs passed" }, - }), - ]), - ]; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter([ - githubFetchRoute( - ({ url }) => url.includes("/pulls/42"), - () => githubResponse(pullRequest()), - ), - githubFetchRoute( - ({ url }) => url.includes("Coordination"), - () => { - coordinationQueries += 1; - return githubResponse(coordinationListings.shift()); - }, - ), - ]), - ); - - await expect( - waitForRequiredGate(identity, { - timeoutMs: 100, - pollIntervalMs: 10, - now: () => clock, - sleep: async (milliseconds) => { - clock += milliseconds; - }, - }), - ).resolves.toMatchObject({ conclusion: "success" }); - expect(coordinationQueries).toBe(3); - }); - - it("includes the trusted coordination link when polling times out", async () => { - let clock = 0; - vi.spyOn(console, "log").mockImplementation(() => undefined); - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter([ - githubFetchRoute( - ({ url }) => url.includes("/pulls/42"), - () => githubResponse(pullRequest()), - ), - githubFetchRoute( - ({ url }) => url.includes("Coordination"), - () => - githubResponse( - listing([ - check(undefined, { - status: "in_progress", - conclusion: null, - details_url: "https://github.com/NVIDIA/NemoClaw/runs/17", - output: { title: "Running E2E jobs" }, - }), - ]), - ), - ), - ]), - ); - - await expect( - waitForRequiredGate(identity, { - timeoutMs: 10, - pollIntervalMs: 10, - now: () => clock, - sleep: async (milliseconds) => { - clock += milliseconds; - }, - }), - ).rejects.toThrow("https://github.com/NVIDIA/NemoClaw/runs/17"); - }); - - it("fails closed when the PR head changes before a terminal verdict is accepted", async () => { - vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => { - const url = String(input); - return githubResponse( - url.includes("/pulls/42") - ? pullRequest({ head: { sha: "c".repeat(40) } }) - : listing([check()]), - ); - }); - - await expect( - waitForRequiredGate(identity, { timeoutMs: 100, pollIntervalMs: 10 }), - ).rejects.toThrow("not the expected open PR with the observed PR SHA and base SHA"); - }); - - describe("bounded GitHub read retries (#7207)", () => { - it("classifies only network, rate-limit, and server failures as retryable", () => { - expect(isRetryableGithubReadError(new TypeError("fetch failed"))).toBe(true); - expect( - isRetryableGithubReadError(new Error("GitHub API repos/x/pulls/1 failed: 429 limited")), - ).toBe(true); - expect( - isRetryableGithubReadError(new Error("GitHub API repos/x/pulls/1 failed: 503 unavailable")), - ).toBe(true); - expect( - isRetryableGithubReadError(new Error("GitHub API repos/x/pulls/1 failed: 404 missing")), - ).toBe(false); - expect( - isRetryableGithubReadError(new Error("GitHub API repos/x/pulls/1 failed: 401 denied")), - ).toBe(false); - expect( - isRetryableGithubReadError(new Error("GitHub API repos/x/pulls/1 failed: 403 denied")), - ).toBe(false); - expect(isRetryableGithubReadError(new Error("fetch failed"))).toBe(false); - }); - - it("retries a transient read with deterministic bounded delay", async () => { - const sleepCalls: number[] = []; - const read = vi - .fn() - .mockRejectedValueOnce(new TypeError("fetch failed")) - .mockResolvedValue("success"); - - await expect( - retryableGithubRead("coordination checks", read, null, { - baseDelayMs: 100, - random: () => 0, - sleep: async (milliseconds) => { - sleepCalls.push(milliseconds); - }, - }), - ).resolves.toBe("success"); - - expect(read).toHaveBeenCalledTimes(2); - expect(sleepCalls).toEqual([50]); - }); - - it("preserves the first retryable failure as the exhaustion cause", async () => { - let calls = 0; - const error = await retryableGithubRead( - "coordination checks", - async () => { - calls += 1; - throw new TypeError(`fetch failed attempt ${calls}`); - }, - null, - { baseDelayMs: 1, random: () => 0, sleep: async () => {} }, - ).catch((caught: unknown) => caught); - - expect(calls).toBe(3); - expect(error).toBeInstanceOf(Error); - expect((error as Error).message).toBe( - "E2E / PR Gate [coordination checks] attempt 3/3: network", - ); - expect((error as Error & { cause?: Error }).cause?.message).toBe("fetch failed attempt 1"); - }); - - it("does not retry or expose a non-retryable response body", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - let calls = 0; - - const error = await retryableGithubRead( - "coordination checks", - async () => { - calls += 1; - throw new Error("GitHub API repos/x/pulls/1 failed: 404 secret response"); - }, - null, - { sleep: async () => {} }, - ).catch((caught: unknown) => caught); - - expect(calls).toBe(1); - expect((error as Error).message).toBe( - "E2E / PR Gate [coordination checks] attempt 1/3: http", - ); - expect((error as Error).message).not.toContain("secret response"); - expect((error as Error & { cause?: Error }).cause?.message).toContain("secret response"); - expect(log).not.toHaveBeenCalled(); - }); - - it("uses a later terminal failure as the cause after a transient failure", async () => { - const read = vi - .fn() - .mockRejectedValueOnce(new TypeError("fetch failed")) - .mockRejectedValueOnce( - new Error("GitHub API repos/x/pulls/1 failed: 404 terminal response"), - ); - - const error = await retryableGithubRead("coordination checks", read, null, { - baseDelayMs: 1, - random: () => 0, - sleep: async () => {}, - }).catch((caught: unknown) => caught); - - expect(read).toHaveBeenCalledTimes(2); - expect((error as Error).message).toBe( - "E2E / PR Gate [coordination checks] attempt 2/3: http", - ); - expect((error as Error & { cause?: Error }).cause?.message).toBe( - "GitHub API repos/x/pulls/1 failed: 404 terminal response", - ); - }); - - it("logs a retry class without reflecting the GitHub response body", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - const read = vi - .fn() - .mockRejectedValueOnce( - new Error("GitHub API repos/x/check-runs failed: 503 credential-like-body"), - ) - .mockResolvedValue("success"); - - await retryableGithubRead("coordination checks", read, null, { - baseDelayMs: 1, - random: () => 0, - sleep: async () => {}, - }); - - expect(log).toHaveBeenCalledWith("E2E / PR Gate [coordination checks] attempt 1/3: http"); - expect(log.mock.calls.flat().join(" ")).not.toContain("credential-like-body"); - }); - - it("does not expose a retryable response body after exhaustion", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - - const error = await retryableGithubRead( - "coordination checks", - async () => { - throw new Error("GitHub API repos/x/check-runs failed: 503 credential-like-body"); - }, - null, - { baseDelayMs: 1, random: () => 0, sleep: async () => {} }, - ).catch((caught: unknown) => caught); - - expect((error as Error).message).toBe( - "E2E / PR Gate [coordination checks] attempt 3/3: http", - ); - expect((error as Error).message).not.toContain("credential-like-body"); - expect(log.mock.calls.flat().join(" ")).not.toContain("credential-like-body"); - expect((error as Error & { cause?: Error }).cause?.message).toContain("credential-like-body"); - }); - - it("fails closed when exact PR identity changes before a retry", async () => { - let calls = 0; - vi.spyOn(globalThis, "fetch").mockResolvedValue( - githubResponse(pullRequest({ head: { sha: "c".repeat(40) } })), - ); - - await expect( - retryableGithubRead( - "coordination checks", - async () => { - calls += 1; - throw new TypeError("fetch failed"); - }, - identity, - { baseDelayMs: 1, random: () => 0, sleep: async () => {} }, - ), - ).rejects.toThrow("not the expected open PR with the observed PR SHA and base SHA"); - - expect(calls).toBe(1); - }); - - it("proves exact PR identity through transient revalidation before retrying", async () => { - const sleepCalls: number[] = []; - const identityRead = vi - .spyOn(globalThis, "fetch") - .mockRejectedValueOnce(new TypeError("identity fetch failed")) - .mockResolvedValue(githubResponse(pullRequest())); - const coordinationRead = vi - .fn() - .mockRejectedValueOnce(new TypeError("coordination fetch failed")) - .mockResolvedValue("success"); - - await expect( - retryableGithubRead("coordination checks", coordinationRead, identity, { - baseDelayMs: 100, - random: () => 0, - sleep: async (milliseconds) => { - sleepCalls.push(milliseconds); - }, - }), - ).resolves.toBe("success"); - - expect(coordinationRead).toHaveBeenCalledTimes(2); - expect(identityRead).toHaveBeenCalledTimes(2); - expect(sleepCalls).toEqual([50, 50]); - }); - - it("does not retry the original read when identity cannot be proven", async () => { - let calls = 0; - let identityCalls = 0; - vi.spyOn(globalThis, "fetch").mockImplementation(async () => { - identityCalls += 1; - throw new TypeError(`identity fetch failed ${identityCalls}`); - }); - - await expect( - retryableGithubRead( - "coordination checks", - async () => { - calls += 1; - throw new TypeError("coordination fetch failed"); - }, - identity, - { baseDelayMs: 1, random: () => 0, sleep: async () => {} }, - ), - ).rejects.toThrow("E2E / PR Gate [exact PR identity] attempt 3/3: network"); - - expect(calls).toBe(1); - expect(identityCalls).toBe(3); - }); - - it("revalidates identity after the backoff and before the original read", async () => { - let calls = 0; - let identityChanged = false; - vi.spyOn(globalThis, "fetch").mockImplementation(async () => - githubResponse( - identityChanged ? pullRequest({ head: { sha: "c".repeat(40) } }) : pullRequest(), - ), - ); - - await expect( - retryableGithubRead( - "coordination checks", - async () => { - calls += 1; - throw new TypeError("coordination fetch failed"); - }, - identity, - { - baseDelayMs: 1, - random: () => 0, - sleep: async () => { - identityChanged = true; - }, - }, - ), - ).rejects.toThrow("not the expected open PR with the observed PR SHA and base SHA"); - - expect(calls).toBe(1); - }); - }); -}); diff --git a/test/pr-risk-plan.test.ts b/test/pr-risk-plan.test.ts index 717c90728b8..dac49a5fe04 100644 --- a/test/pr-risk-plan.test.ts +++ b/test/pr-risk-plan.test.ts @@ -345,7 +345,6 @@ describe("deterministic PR risk plan", () => { "tools/advisors/io.mts", "tools/advisors/risk-plan.mts", "tools/e2e/pr-e2e-gate.mts", - "tools/e2e/pr-e2e-required.mts", "tools/e2e/risk-signal.ts", "tools/e2e/private-file.mts", "tools/e2e/workflow-plan.mts", diff --git a/test/skills/check-gates-evidence.test.ts b/test/skills/check-gates-evidence.test.ts index c39bf355cd6..2e8e2d13f16 100644 --- a/test/skills/check-gates-evidence.test.ts +++ b/test/skills/check-gates-evidence.test.ts @@ -425,26 +425,18 @@ describe("maintainer merge-gate contributor compliance", () => { }); }); - it("accepts the legacy E2E coordination check name", () => { - const legacyCheck = coordinationCheck({ id: 8001, name: "E2E / PR Gate" }); + it("accepts the former exact-diff E2E coordination check name during rollout", () => { + const formerCheck = coordinationCheck({ + id: 8001, + name: "E2E / PR Gate Coordination", + }); const output = JSON.parse( runGate({ body: "Signed-off-by: Example User ", verified: true, - statusChecks: [ - ...successfulRequiredChecksWithoutE2e(), - { - __typename: "CheckRun", - name: "E2E / PR Gate", - workflowName: "Automation / Request NVSkills CI", - detailsUrl: "https://github.com/NVIDIA/NemoClaw/runs/8001", - startedAt: "2026-01-01T00:00:00Z", - status: "COMPLETED", - conclusion: "SUCCESS", - }, - ], + statusChecks: [...successfulRequiredChecksWithoutE2e(), e2eGateCheck([94, 1, "SUCCESS"])], coordinationCheckPages: [{ total_count: 0, check_runs: [] }], - legacyCoordinationCheckPages: [{ total_count: 1, check_runs: [legacyCheck] }], + formerCoordinationCheckPages: [{ total_count: 1, check_runs: [formerCheck] }], }).stdout, ); @@ -456,6 +448,11 @@ describe("maintainer merge-gate contributor compliance", () => { runGate({ body: "Signed-off-by: Example User ", verified: true, + statusChecks: successfulRequiredChecks().map((check) => + check.name === "E2E / PR Gate" + ? { ...check, detailsUrl: "https://github.com/NVIDIA/NemoClaw/runs/8002" } + : check, + ), coordinationCheckPages: [ { total_count: 2, diff --git a/test/skills/check-gates-retry-history.test.ts b/test/skills/check-gates-retry-history.test.ts index 3faa81e38f5..53ace525fd7 100644 --- a/test/skills/check-gates-retry-history.test.ts +++ b/test/skills/check-gates-retry-history.test.ts @@ -2,7 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it } from "vitest"; -import { coordinationCheck, runGate } from "./check-gates-test-fixtures.ts"; +import { + coordinationCheck, + runGate, + successfulRequiredChecks, +} from "./check-gates-test-fixtures.ts"; const SIGNED_BODY = "Signed-off-by: Example User "; @@ -18,10 +22,19 @@ function retryableFailure(id: number, reason: string, title = "Retryable E2E fai } function gateOutput(checkRuns: unknown[]) { + const currentCheckId = Math.max(...checkRuns.map((check) => (check as { id: number }).id)); return JSON.parse( runGate({ body: SIGNED_BODY, verified: true, + statusChecks: successfulRequiredChecks().map((check) => + check.name === "E2E / PR Gate" + ? { + ...check, + detailsUrl: `https://github.com/NVIDIA/NemoClaw/runs/${currentCheckId}`, + } + : check, + ), coordinationCheckPages: [{ total_count: checkRuns.length, check_runs: checkRuns }], }).stdout, ); diff --git a/test/skills/check-gates-rollout.test.ts b/test/skills/check-gates-rollout.test.ts new file mode 100644 index 00000000000..be4d4527a62 --- /dev/null +++ b/test/skills/check-gates-rollout.test.ts @@ -0,0 +1,34 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; +import { coordinationCheck, runGate } from "./check-gates-test-fixtures.ts"; + +describe("maintainer merge-gate check-name rollout", () => { + it("prefers the current exact-diff E2E check when both rollout names exist", () => { + const output = JSON.parse( + runGate({ + body: "Signed-off-by: Example User ", + verified: true, + coordinationCheckPages: [{ total_count: 1, check_runs: [coordinationCheck({ id: 8000 })] }], + formerCoordinationCheckPages: [ + { + total_count: 1, + check_runs: [ + coordinationCheck({ + id: 8001, + name: "E2E / PR Gate Coordination", + conclusion: "failure", + }), + ], + }, + ], + }).stdout, + ); + + expect(output).toMatchObject({ + allPass: true, + gates: { ci: { pass: true, trustedCustomCheckId: 8000 } }, + }); + }); +}); diff --git a/test/skills/check-gates-test-fixtures.ts b/test/skills/check-gates-test-fixtures.ts index adb4a4ea058..e7f9a3cc495 100644 --- a/test/skills/check-gates-test-fixtures.ts +++ b/test/skills/check-gates-test-fixtures.ts @@ -20,7 +20,7 @@ const CUSTOM_RUN_URL = "https://github.com/NVIDIA/NemoClaw/runs/123"; const INCOMPLETE_E2E = ["E2E / PR Gate: latest attempt evidence incomplete"]; const HEAD_SHA = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; const BASE_SHA = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; -const E2E_COORDINATION_NAME = "E2E / PR Gate Coordination"; +const E2E_COORDINATION_NAME = "E2E / PR Gate"; const E2E_COORDINATION_EXTERNAL_ID = `nemoclaw-pr-e2e:v2:42:${HEAD_SHA}:${BASE_SHA}`; const PR_WORKFLOW_JOB_NAMES = [ "changes", @@ -125,7 +125,7 @@ interface ComplianceFixture { actionRunAttempts?: Record; issueEventPages?: unknown[]; coordinationCheckPages?: unknown[]; - legacyCoordinationCheckPages?: unknown[]; + formerCoordinationCheckPages?: unknown[]; finalPr?: Record; finalPrAfterCurrentBase?: Record; } @@ -151,6 +151,16 @@ function successfulRequiredChecks() { } function requiredCheck(name: string, conclusion = "SUCCESS") { + if (name === "E2E / PR Gate") { + return { + __typename: "CheckRun", + name, + detailsUrl: "https://github.com/NVIDIA/NemoClaw/runs/8000", + startedAt: "2026-01-01T00:01:30Z", + status: "COMPLETED", + conclusion, + }; + } const { runId, jobId, workflowName } = REQUIRED_CHECK_RUNS[name]; return e2eGateCheck([runId, jobId, conclusion, undefined, undefined, workflowName, name]); } @@ -356,7 +366,7 @@ function runGate(fixture: ComplianceFixture) { check_runs: [coordinationCheck()], }, ]; - const legacyCoordinationCheckPages = fixture.legacyCoordinationCheckPages ?? [ + const formerCoordinationCheckPages = fixture.formerCoordinationCheckPages ?? [ { total_count: 0, check_runs: [] }, ]; const actionRunCases = Object.entries({ @@ -458,8 +468,8 @@ case "$*" in "api repos/NVIDIA/NemoClaw/issues/42/comments"*) printf '%s' '{"id":1,"body":"ordinary comment","user":{"login":"reviewer"},"updated_at":"2026-01-01T00:00:00Z"}' ;; "api repos/NVIDIA/NemoClaw/pulls/42/commits"*) printf '%s' ${shellSingleQuote(commitOutput)} ;; "api --paginate --slurp repos/NVIDIA/NemoClaw/issues/42/events?per_page=100") printf '%s' ${shellSingleQuote(JSON.stringify(issueEventPages))} ;; - "api --paginate --slurp repos/NVIDIA/NemoClaw/commits/${HEAD_SHA}/check-runs?check_name=E2E%20%2F%20PR%20Gate%20Coordination&filter=all&per_page=100") printf '%s' ${shellSingleQuote(JSON.stringify(coordinationCheckPages))} ;; - "api --paginate --slurp repos/NVIDIA/NemoClaw/commits/${HEAD_SHA}/check-runs?check_name=E2E%20%2F%20PR%20Gate&filter=all&per_page=100") printf '%s' ${shellSingleQuote(JSON.stringify(legacyCoordinationCheckPages))} ;; + "api --paginate --slurp repos/NVIDIA/NemoClaw/commits/${HEAD_SHA}/check-runs?check_name=E2E%20%2F%20PR%20Gate&filter=all&per_page=100") printf '%s' ${shellSingleQuote(JSON.stringify(coordinationCheckPages))} ;; + "api --paginate --slurp repos/NVIDIA/NemoClaw/commits/${HEAD_SHA}/check-runs?check_name=E2E%20%2F%20PR%20Gate%20Coordination&filter=all&per_page=100") printf '%s' ${shellSingleQuote(JSON.stringify(formerCoordinationCheckPages))} ;; ${actionRunCases} *) echo "unexpected gh args: $*" >&2; exit 9 ;; esac diff --git a/test/vitest-watch-triggers.test.ts b/test/vitest-watch-triggers.test.ts index 5d9234cbb2c..4bc7ac0b255 100644 --- a/test/vitest-watch-triggers.test.ts +++ b/test/vitest-watch-triggers.test.ts @@ -123,7 +123,6 @@ describe("Vitest opaque-input watch triggers", () => { ]); expect(triggeredBy(".github/workflows/pr-e2e-gate.yaml")).toEqual([ "test/pr-e2e-gate-workflow.test.ts", - "test/pr-e2e-required.test.ts", ]); expect(triggeredBy(".github/workflows/pr-review-advisor.yaml")).toEqual([ "test/pr-review-advisor-workflow-boundary.test.ts", diff --git a/tools/e2e/operations-workflow-boundary.mts b/tools/e2e/operations-workflow-boundary.mts index adb12602b80..1bfde0dedcf 100644 --- a/tools/e2e/operations-workflow-boundary.mts +++ b/tools/e2e/operations-workflow-boundary.mts @@ -260,7 +260,7 @@ function validateControllerAuthorization( '--header "Cache-Control: no-cache"', "Child run: ${expected_run_url}.", `[[ "$(jq -r '.output.summary // ""' <<< "$check_json")" == "$expected_summary" ]]`, - '.name == "E2E / PR Gate Coordination"', + '.name == "E2E / PR Gate"', ".app.id == 15368", '.app.slug == "github-actions"', ".external_id == $external_id", diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index caa48cc50df..377ad6d14a9 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -64,7 +64,7 @@ export { validateWorkflowDispatchDetails } from "./pr-e2e-dispatch-reconciliatio const E2E_WORKFLOW = "e2e.yaml"; const E2E_WORKFLOW_PATH = `.github/workflows/${E2E_WORKFLOW}`; const PR_GATE_WORKFLOW = "pr-e2e-gate.yaml"; -const CHECK_NAME = "E2E / PR Gate Coordination"; +const CHECK_NAME = "E2E / PR Gate"; const WORKFLOW_NAME = "E2E / PR Gate Controller"; const RESERVED_CHECK_TITLE = "Waiting for PR CI"; const RESERVED_CHECK_SUMMARY = diff --git a/tools/e2e/pr-e2e-required.mts b/tools/e2e/pr-e2e-required.mts deleted file mode 100644 index 44f502c42a8..00000000000 --- a/tools/e2e/pr-e2e-required.mts +++ /dev/null @@ -1,495 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import fs from "node:fs"; -import { pathToFileURL } from "node:url"; - -import { githubApi } from "../advisors/github.mts"; -import { parseArgs } from "../advisors/io.mts"; -import { retryableFailureReason } from "./pr-e2e-retry-receipt.mts"; - -const COORDINATION_CHECK_NAME = "E2E / PR Gate Coordination"; -const LEGACY_COORDINATION_CHECK_NAME = "E2E / PR Gate"; -const EXTERNAL_ID_PREFIX = "nemoclaw-pr-e2e:v2"; -const GITHUB_ACTIONS_APP_ID = 15368; -const USER_AGENT = "nemoclaw-pr-e2e-required"; -const SHA_PATTERN = /^[a-f0-9]{40}$/u; -const REPOSITORY_PATTERN = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/u; -const MAX_LOG_URLS = 20; -const MAX_GITHUB_READ_ATTEMPTS = 3; -const GITHUB_HTTP_ERROR_PATTERN = /^GitHub API [^\r\n]* failed: ([1-5]\d{2})\b/u; -const RETRYABLE_HTTP_PATTERN = /^GitHub API [^\r\n]* failed: (?:429|5\d{2})\b/u; - -type CheckConclusion = "success" | "failure" | "cancelled"; -type GithubReadOperation = "check runs" | "coordination checks" | "exact PR identity"; - -export type RetryableGithubReadOptions = { - baseDelayMs?: number; - maxAttempts?: number; - random?: () => number; - sleep?: (milliseconds: number) => Promise; -}; - -export type CoordinationCheckRun = { - id: number; - name: string; - head_sha: string; - external_id: string | null; - status: string; - conclusion: string | null; - details_url?: string | null; - output?: { summary?: string | null; title?: string | null }; - app?: { id?: number } | null; -}; - -type CheckRunsResponse = { - total_count: number; - check_runs: CoordinationCheckRun[]; -}; - -type PullRequest = { - number: number; - state: string; - head: { sha: string }; - base: { sha: string }; -}; - -export type RequiredGateIdentity = { - repository: string; - token: string; - prNumber: number; - headSha: string; - baseSha: string; -}; - -export type RequiredGateResult = { - conclusion: CheckConclusion; - title: string; - detailsUrl?: string; - logUrls?: string[]; -}; - -type WaitingGateResult = { - state: "waiting"; - description: string; - detailsUrl?: string; - logUrls?: string[]; -}; - -function isObjectRecord(value: unknown): value is Record { - return !!value && typeof value === "object" && !Array.isArray(value); -} - -function requiredArgument(value: string | undefined, name: string): string { - if (!value) throw new Error(`--${name} is required`); - return value; -} - -function parsePositiveInteger(value: string | undefined, name: string): number { - const input = requiredArgument(value, name); - if (!/^[1-9][0-9]*$/u.test(input)) throw new Error(`--${name} must be a positive integer`); - const parsed = Number(input); - if (!Number.isSafeInteger(parsed)) throw new Error(`--${name} exceeds the safe integer range`); - return parsed; -} - -function assertIdentity(identity: RequiredGateIdentity): void { - if (!REPOSITORY_PATTERN.test(identity.repository)) { - throw new Error("GITHUB_REPOSITORY must be an owner/repository name"); - } - if (!identity.token) throw new Error("GITHUB_TOKEN is required"); - if (!Number.isSafeInteger(identity.prNumber) || identity.prNumber < 1) { - throw new Error("PR number is invalid"); - } - if (!SHA_PATTERN.test(identity.headSha)) throw new Error("PR head SHA is invalid"); - if (!SHA_PATTERN.test(identity.baseSha)) throw new Error("PR base SHA is invalid"); -} - -export function coordinationExternalId(prNumber: number, headSha: string, baseSha: string): string { - return `${EXTERNAL_ID_PREFIX}:${prNumber}:${headSha}:${baseSha}`; -} - -function validateCheckRunsResponse(value: unknown): CheckRunsResponse { - if ( - !isObjectRecord(value) || - !Number.isSafeInteger(value.total_count) || - (value.total_count as number) < 0 || - !Array.isArray(value.check_runs) || - value.check_runs.length !== value.total_count - ) { - throw new Error("GitHub returned an invalid or incomplete coordination check listing"); - } - for (const check of value.check_runs) { - if ( - !isObjectRecord(check) || - !Number.isSafeInteger(check.id) || - (check.id as number) < 1 || - typeof check.name !== "string" || - typeof check.head_sha !== "string" || - (check.external_id !== null && typeof check.external_id !== "string") - ) { - throw new Error("GitHub returned an invalid coordination check"); - } - } - return value as CheckRunsResponse; -} - -function validatePullRequest(value: unknown, identity: RequiredGateIdentity): PullRequest { - if ( - !isObjectRecord(value) || - value.number !== identity.prNumber || - value.state !== "open" || - !isObjectRecord(value.head) || - value.head.sha !== identity.headSha || - !isObjectRecord(value.base) || - value.base.sha !== identity.baseSha - ) { - throw new Error("PR is not the expected open PR with the observed PR SHA and base SHA"); - } - return value as PullRequest; -} - -async function requireExactPullRequest(identity: RequiredGateIdentity): Promise { - validatePullRequest( - await githubApi( - `repos/${identity.repository}/pulls/${identity.prNumber}`, - identity.token, - { userAgent: USER_AGENT }, - ), - identity, - ); -} - -export function isRetryableGithubReadError(error: unknown): boolean { - return ( - error instanceof TypeError || - (error instanceof Error && RETRYABLE_HTTP_PATTERN.test(error.message)) - ); -} - -export async function retryableGithubRead( - operation: GithubReadOperation, - read: () => Promise, - identity: RequiredGateIdentity | null, - options: RetryableGithubReadOptions = {}, -): Promise { - const maxAttempts = options.maxAttempts ?? MAX_GITHUB_READ_ATTEMPTS; - const baseDelayMs = options.baseDelayMs ?? 1000; - if (!Number.isSafeInteger(maxAttempts) || maxAttempts < 1 || maxAttempts > 5) { - throw new Error("GitHub read retry attempts must be an integer from 1 through 5"); - } - if (!Number.isSafeInteger(baseDelayMs) || baseDelayMs < 1 || baseDelayMs > 4000) { - throw new Error("GitHub read retry delay must be an integer from 1 through 4000"); - } - const random = options.random ?? Math.random; - const sleep = - options.sleep ?? - ((milliseconds: number) => new Promise((resolve) => setTimeout(resolve, milliseconds))); - - let firstError: Error | undefined; - for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { - try { - return await read(); - } catch (error: unknown) { - const retryable = isRetryableGithubReadError(error); - if (!retryable || attempt === maxAttempts) { - const errorClass = - error instanceof TypeError - ? "network" - : error instanceof Error && GITHUB_HTTP_ERROR_PATTERN.test(error.message) - ? "http" - : null; - if (errorClass) { - throw new Error( - `E2E / PR Gate [${operation}] attempt ${attempt}/${maxAttempts}: ${errorClass}`, - { cause: retryable ? (firstError ?? error) : error }, - ); - } - throw error; - } - if (error instanceof Error) firstError ??= error; - const errorClass = error instanceof TypeError ? "network" : "http"; - console.log(`E2E / PR Gate [${operation}] attempt ${attempt}/${maxAttempts}: ${errorClass}`); - - const jitter = 0.5 + random() * 0.5; - const delay = Math.min(baseDelayMs * 2 ** (attempt - 1) * jitter, 4000); - await sleep(delay); - - if (identity) { - await retryableGithubRead( - "exact PR identity", - () => requireExactPullRequest(identity), - null, - options, - ); - } - } - } - throw new Error("GitHub read retry loop ended unexpectedly"); -} - -function currentCoordinationCheck( - checks: CoordinationCheckRun[], -): CoordinationCheckRun | undefined { - if (checks.length === 0) return undefined; - const ordered = [...checks].sort((left, right) => left.id - right.id); - if (new Set(ordered.map((check) => check.id)).size !== ordered.length) { - throw new Error("Duplicate coordination check IDs exist for one PR/base SHA pair"); - } - const active = ordered.filter((check) => check.status !== "completed"); - if (active.length > 1) - throw new Error("Multiple active coordination checks exist for one PR/base SHA pair"); - if (ordered.slice(0, -1).some((check) => retryableFailureReason(check) === undefined)) { - throw new Error( - "Coordination history contains a non-retryable older check for one PR/base SHA pair", - ); - } - const current = ordered.at(-1)!; - if (active[0] && active[0].id !== current.id) { - throw new Error("Coordination history for one PR/base SHA pair contains an older active check"); - } - return current; -} - -async function matchingChecks( - identity: RequiredGateIdentity, - name: string, -): Promise { - const response = validateCheckRunsResponse( - await retryableGithubRead( - "check runs", - () => - githubApi( - `repos/${identity.repository}/commits/${identity.headSha}/check-runs?check_name=${encodeURIComponent(name)}&filter=all&per_page=100`, - identity.token, - { userAgent: USER_AGENT }, - ), - identity, - ), - ); - const externalId = coordinationExternalId(identity.prNumber, identity.headSha, identity.baseSha); - const claimed = response.check_runs.filter( - (check) => - check.name === name && - check.head_sha === identity.headSha && - check.external_id === externalId, - ); - if (claimed.some((check) => check.app?.id !== GITHUB_ACTIONS_APP_ID)) { - throw new Error( - "The PR/base SHA coordination identity was claimed by an unexpected GitHub App", - ); - } - const current = currentCoordinationCheck( - claimed.filter((check) => check.app?.id === GITHUB_ACTIONS_APP_ID), - ); - return current ? [current] : []; -} - -export async function findCoordinationCheck( - identity: RequiredGateIdentity, -): Promise { - assertIdentity(identity); - const current = await matchingChecks(identity, COORDINATION_CHECK_NAME); - if (current.length > 1) - throw new Error("Multiple coordination checks exist for one PR/base SHA pair"); - if (current[0]) return current[0]; - - // Migration bridge for PRs whose base-branch controller still publishes the - // old name. Remove after this workflow is on main and open PRs resynchronize. - const legacy = await matchingChecks(identity, LEGACY_COORDINATION_CHECK_NAME); - if (legacy.length > 1) - throw new Error("Multiple legacy coordination checks exist for one PR/base SHA pair"); - return legacy[0]; -} - -function escapeRegExp(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"); -} - -function trustedCheckDetailsUrl(value: unknown, repository: string): string | undefined { - if (typeof value !== "string") return undefined; - const repo = escapeRegExp(repository); - const pattern = new RegExp( - `^https://github\\.com/${repo}/(?:actions/runs/[1-9][0-9]*(?:/attempts/[1-9][0-9]*)?(?:/job/[1-9][0-9]*)?|runs/[1-9][0-9]*)$`, - "u", - ); - return pattern.test(value) ? value : undefined; -} - -function trustedSummaryLogUrls(summary: unknown, repository: string): string[] { - if (typeof summary !== "string") return []; - const repo = escapeRegExp(repository); - const firstLine = summary.split(/\r?\n/u, 1)[0] ?? ""; - const runTarget = firstLine.match( - new RegExp( - `\\]\\((https://github\\.com/${repo}/actions/runs/([1-9][0-9]*)(?:/attempts/[1-9][0-9]*)?)\\)`, - "u", - ), - ); - if (!runTarget?.[1] || !runTarget[2]) return []; - const runUrl = `https://github.com/${repository}/actions/runs/${runTarget[2]}`; - const jobTargetPattern = new RegExp( - `^-[ \\t]+\\[.*\\]\\((${escapeRegExp(runUrl)}/job/[1-9][0-9]*)\\)[ \\t]+—[ \\t]+.*$`, - "gmu", - ); - const jobUrls = [...summary.matchAll(jobTargetPattern)].flatMap((match) => - match[1] ? [match[1]] : [], - ); - return jobUrls.length > 0 ? [...new Set(jobUrls)].slice(0, MAX_LOG_URLS) : [runTarget[1]]; -} - -function coordinationLogUrls( - check: CoordinationCheckRun, - repository: string, -): { detailsUrl?: string; logUrls?: string[] } { - const detailsUrl = trustedCheckDetailsUrl(check.details_url, repository); - const summaryUrls = trustedSummaryLogUrls(check.output?.summary, repository); - const logUrls = summaryUrls.length > 0 ? summaryUrls : detailsUrl ? [detailsUrl] : []; - return { - ...(detailsUrl ? { detailsUrl } : {}), - ...(logUrls.length > 0 ? { logUrls } : {}), - }; -} - -function logUrlSuffix(logUrls: readonly string[] | undefined): string { - return logUrls && logUrls.length > 0 ? ` logs=${logUrls.join(" ")}` : ""; -} - -export function formatRequiredGateOutcome(result: RequiredGateResult): string { - return `conclusion=${result.conclusion} title=${result.title}${logUrlSuffix(result.logUrls)}`; -} - -export function classifyCoordinationCheck( - check: CoordinationCheckRun | undefined, - repository: string, -): WaitingGateResult | { state: "complete"; result: RequiredGateResult } { - if (!check) return { state: "waiting", description: "waiting for trusted coordination" }; - if (!REPOSITORY_PATTERN.test(repository)) { - throw new Error("repository must be an owner/repository name"); - } - const title = check.output?.title?.trim() || "Trusted E2E coordination result"; - const links = coordinationLogUrls(check, repository); - if (check.status !== "completed") { - return { state: "waiting", description: title, ...links }; - } - if (check.conclusion === "failure" && retryableFailureReason(check) !== undefined) { - return { state: "waiting", description: title, ...links }; - } - if ( - !(["success", "failure", "cancelled"] as const).includes(check.conclusion as CheckConclusion) - ) { - throw new Error( - `Coordination check completed with unsupported conclusion ${check.conclusion}${logUrlSuffix(links.logUrls)}`, - ); - } - return { - state: "complete", - result: { - conclusion: check.conclusion as CheckConclusion, - title, - ...links, - }, - }; -} - -export async function waitForRequiredGate( - identity: RequiredGateIdentity, - options: { - timeoutMs: number; - pollIntervalMs?: number; - now?: () => number; - sleep?: (milliseconds: number) => Promise; - }, -): Promise { - assertIdentity(identity); - if (!Number.isSafeInteger(options.timeoutMs) || options.timeoutMs < 1) { - throw new Error("gate timeout is invalid"); - } - const pollIntervalMs = options.pollIntervalMs ?? 30_000; - if (!Number.isSafeInteger(pollIntervalMs) || pollIntervalMs < 1) { - throw new Error("gate poll interval is invalid"); - } - const now = options.now ?? Date.now; - const sleep = - options.sleep ?? - ((milliseconds: number) => new Promise((resolve) => setTimeout(resolve, milliseconds))); - const deadline = now() + options.timeoutMs; - let lastDescription = ""; - let lastLogUrls: string[] | undefined; - - await retryableGithubRead("exact PR identity", () => requireExactPullRequest(identity), null, { - sleep, - }); - while (now() < deadline) { - const classified = classifyCoordinationCheck( - await findCoordinationCheck(identity), - identity.repository, - ); - if (classified.state === "complete") { - await retryableGithubRead( - "exact PR identity", - () => requireExactPullRequest(identity), - null, - { sleep }, - ); - return classified.result; - } - const message = `${classified.description}${logUrlSuffix(classified.logUrls)}`; - if (message !== lastDescription) { - console.log(`E2E / PR Gate: ${message}`); - lastDescription = message; - } - if (classified.logUrls) lastLogUrls = classified.logUrls; - await sleep(Math.min(pollIntervalMs, Math.max(1, deadline - now()))); - } - throw new Error(`Timed out waiting for the trusted E2E verdict${logUrlSuffix(lastLogUrls)}`); -} - -function appendJobSummary(): void { - const summaryPath = process.env.GITHUB_STEP_SUMMARY; - if (!summaryPath) return; - const descriptor = fs.openSync( - summaryPath, - fs.constants.O_WRONLY | fs.constants.O_APPEND | (fs.constants.O_NOFOLLOW ?? 0), - ); - try { - if (!fs.fstatSync(descriptor).isFile()) { - throw new Error("GITHUB_STEP_SUMMARY must be a regular file"); - } - fs.writeFileSync( - descriptor, - "## E2E / PR Gate\n\nSee the job log for the trusted terminal verdict and links.\n", - "utf8", - ); - } finally { - fs.closeSync(descriptor); - } -} - -async function main(): Promise { - const args = parseArgs(process.argv.slice(2)); - const identity: RequiredGateIdentity = { - repository: process.env.GITHUB_REPOSITORY ?? "", - token: process.env.GITHUB_TOKEN ?? "", - prNumber: parsePositiveInteger(args.pr, "pr"), - headSha: requiredArgument(args.head, "head"), - baseSha: requiredArgument(args.base, "base"), - }; - const timeoutSeconds = parsePositiveInteger(args.timeoutSeconds, "timeout-seconds"); - 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)}`); - if (result.conclusion !== "success") { - throw new Error(`Trusted E2E verdict: ${formatRequiredGateOutcome(result)}`); - } -} - -if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - main().catch((error: unknown) => { - const message = error instanceof Error ? error.message : String(error); - const escaped = message.replace(/%/gu, "%25").replace(/\r/gu, "%0D").replace(/\n/gu, "%0A"); - console.error(`::error title=E2E / PR Gate failed::${escaped}`); - process.exit(1); - }); -} From af996b22510251cbdfe0711980f6f714d3ae36be Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Thu, 30 Jul 2026 14:51:55 -0700 Subject: [PATCH 2/4] fix(e2e): validate compatible main comparisons Signed-off-by: Charan Jagwani --- test/pr-e2e-gate.test.ts | 29 +++++++++++++++++++++++------ tools/e2e/pr-e2e-gate.mts | 7 +++++-- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index 2afd9e64ddf..f4673daf363 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -475,7 +475,7 @@ describe("PR E2E controller", () => { ).toThrow(/mismatched workflow dispatch URLs/u); }); - it("dispatches from a safe descendant of the triggering workflow commit", async () => { + it("dispatches from a safe descendant in GitHub comparison commit order", async () => { const requests: RecordedGitHubRequest[] = []; vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter( @@ -495,9 +495,10 @@ describe("PR E2E controller", () => { status: "ahead", ahead_by: 1, behind_by: 0, + total_commits: 1, base_commit: { sha: WORKFLOW_SHA }, merge_base_commit: { sha: WORKFLOW_SHA }, - head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + commits: [{ sha: ADVANCED_WORKFLOW_SHA }], files: [{ filename: "docs/quickstart.mdx" }], }), ), @@ -589,9 +590,10 @@ describe("PR E2E controller", () => { status: "ahead", ahead_by: 1, behind_by: 0, + total_commits: 1, base_commit: { sha: WORKFLOW_SHA }, merge_base_commit: { sha: WORKFLOW_SHA }, - head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + commits: [{ sha: ADVANCED_WORKFLOW_SHA }], files, }), ), @@ -628,7 +630,20 @@ describe("PR E2E controller", () => { behind_by: 1, base_commit: { sha: WORKFLOW_SHA }, merge_base_commit: { sha: WORKFLOW_SHA }, - head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + files: [{ filename: "docs/quickstart.mdx" }], + }, + error: /not a validated descendant/u, + }, + { + label: "an incomplete descendant commit list", + comparison: { + status: "ahead", + ahead_by: 2, + behind_by: 0, + total_commits: 2, + base_commit: { sha: WORKFLOW_SHA }, + merge_base_commit: { sha: WORKFLOW_SHA }, + commits: [{ sha: ADVANCED_WORKFLOW_SHA }], files: [{ filename: "docs/quickstart.mdx" }], }, error: /not a validated descendant/u, @@ -639,9 +654,10 @@ describe("PR E2E controller", () => { status: "ahead", ahead_by: 1, behind_by: 0, + total_commits: 1, base_commit: { sha: WORKFLOW_SHA }, merge_base_commit: { sha: WORKFLOW_SHA }, - head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + commits: [{ sha: ADVANCED_WORKFLOW_SHA }], files: Array.from({ length: 300 }, (_, index) => ({ filename: `docs/generated-${index}.mdx`, })), @@ -716,9 +732,10 @@ describe("PR E2E controller", () => { status: "ahead", ahead_by: 1, behind_by: 0, + total_commits: 1, base_commit: { sha: WORKFLOW_SHA }, merge_base_commit: { sha: WORKFLOW_SHA }, - head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + commits: [{ sha: ADVANCED_WORKFLOW_SHA }], files: [{ filename: "docs/quickstart.mdx" }], }), ), diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 377ad6d14a9..027fad932fe 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -1968,12 +1968,15 @@ function validateCompatibleMainComparison( !Number.isSafeInteger(value.ahead_by) || (value.ahead_by as number) < 1 || value.behind_by !== 0 || + value.total_commits !== value.ahead_by || !isObjectRecord(value.base_commit) || value.base_commit.sha !== workflowSha || !isObjectRecord(value.merge_base_commit) || value.merge_base_commit.sha !== workflowSha || - !isObjectRecord(value.head_commit) || - value.head_commit.sha !== mainSha || + !Array.isArray(value.commits) || + value.commits.length !== value.ahead_by || + !isObjectRecord(value.commits.at(-1)) || + value.commits.at(-1)?.sha !== mainSha || !Array.isArray(value.files) ) { throw new Error(`main is not a validated descendant of workflow commit ${workflowSha}`); From 217b10aad701f81144eb619a66ae2f6d78a2b1bb Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Thu, 30 Jul 2026 15:01:36 -0700 Subject: [PATCH 3/4] docs(e2e): specify compatible main comparison Signed-off-by: Charan Jagwani --- test/e2e/README.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/test/e2e/README.md b/test/e2e/README.md index 139ced8cab3..84d119d134c 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -607,14 +607,17 @@ check. Before dispatch, the controller verifies that the live PR still matches the CI run's PR SHA and base SHA. It uses its own workflow commit when that commit is -still `main`. If `main` advanced, the controller accepts the current commit -only when GitHub reports it as a descendant whose merge base is the workflow -commit, the comparison contains fewer than 300 fully enumerated files, neither -side of a rename enters the `e2e-control-plane` risk family, and a second read -confirms that `main` did not move again. Any divergence, incomplete comparison, -control-plane change, or second advance fails closed. The accepted `main` -commit is recorded as the workflow SHA and passed as `workflow_sha`. Before -matrix or secret-bearing jobs can run, `e2e.yaml` requires +still `main`. If `main` advanced, the GitHub comparison `status` must be +`ahead`, `ahead_by` must be positive, and `behind_by` must be `0`. +The `base_commit` and `merge_base_commit` SHAs must equal the workflow commit. +The `total_commits` value and ordered `commits` length must both equal +`ahead_by`, and the final commit SHA must equal the current `main` SHA. +The comparison must contain fewer than 300 fully enumerated files, and neither +side of a rename can enter the `e2e-control-plane` risk family. A second read +must confirm that `main` did not move again. Any divergence, incomplete +comparison, control-plane change, or second advance fails closed. The accepted +`main` commit is recorded as the workflow SHA and passed as `workflow_sha`. +Before matrix or secret-bearing jobs can run, `e2e.yaml` requires `github.workflow_sha` to match that accepted commit. Each selected job checks out `checkout_sha` from the live PR head repository. The same validation verifies that the PR remains open in `NVIDIA/NemoClaw`, the checkout repository From 848d79aba6afa47631c128eff2b294a34c0aec20 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Tue, 4 Aug 2026 01:29:59 -0700 Subject: [PATCH 4/4] test(ci): exercise ordered main comparison Signed-off-by: Charan Jagwani --- test/pr-e2e-gate.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index f6936176c09..548d5929a8c 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -488,12 +488,12 @@ describe("PR E2E controller", () => { () => githubResponse({ status: "ahead", - ahead_by: 1, + ahead_by: 2, behind_by: 0, - total_commits: 1, + total_commits: 2, base_commit: { sha: WORKFLOW_SHA }, merge_base_commit: { sha: WORKFLOW_SHA }, - commits: [{ sha: ADVANCED_WORKFLOW_SHA }], + commits: [{ sha: "f".repeat(40) }, { sha: ADVANCED_WORKFLOW_SHA }], files: [{ filename: "docs/quickstart.mdx" }], }), ),