diff --git a/test/e2e/README.md b/test/e2e/README.md index 815192f13c0..2e4ea5eb42a 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -798,6 +798,12 @@ supplies the PR number, recorded head SHA, recorded base SHA, and a specific review reason. GitHub supplies the triggering actor; the controller requires that account to have current `maintain` or `admin` permission. +Approval is valid only while coordination has the exact +`Maintainer approval required to run fork E2E` title for the live revision. +An early or stale approval fails closed. The diagnostic classifies the +coordination state as preparing, executing, terminal, or malformed. It gives a +fixed remediation and the expected title without echoing the observed check +output. The shared resolver revalidates the open PR, head repository, PR SHA and base SHA, deterministic plan, matching pending coordination state, and that the diff --git a/test/pr-e2e-gate-fork-approval.test.ts b/test/pr-e2e-gate-fork-approval.test.ts index 822ec196857..3658bd25bb4 100644 --- a/test/pr-e2e-gate-fork-approval.test.ts +++ b/test/pr-e2e-gate-fork-approval.test.ts @@ -793,7 +793,7 @@ describe("PR E2E controller fork credentialed E2E approval safety", () => { }, }); await expect(approvePrE2E(approvalCommand(workDirs[1]!))).rejects.toThrow( - /matching pending E2E authorization state/u, + /coordination is terminal/u, ); expect(requests.filter((request) => request.url.endsWith("/dispatches"))).toHaveLength(1); expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); @@ -802,6 +802,141 @@ describe("PR E2E controller fork credentialed E2E approval safety", () => { } }); + it.each([ + { + name: "approval is early", + check: { + status: "in_progress", + conclusion: null, + output: { title: "Waiting for PR CI" }, + }, + expected: /coordination is still preparing.*Wait for the coordination title/u, + }, + { + name: "coordination is queued", + check: { + status: "queued", + conclusion: null, + output: { title: "Maintainer approval required to run fork E2E" }, + }, + expected: /coordination is still preparing.*Wait for the coordination title/u, + }, + { + name: "runner-loss retry is preparing", + check: { + status: "in_progress", + conclusion: null, + output: { title: "Preparing one-time hosted-runner-loss retry" }, + }, + expected: /coordination is still preparing.*Wait for the coordination title/u, + }, + { + name: "E2E authorization is already published", + check: { + status: "in_progress", + conclusion: null, + output: { title: "E2E execution authorized by @maintainer" }, + }, + expected: /E2E is already executing.*do not launch another approval/u, + }, + { + name: "E2E is already running", + check: { + status: "in_progress", + conclusion: null, + output: { title: "Running 3 E2E checks" }, + }, + expected: /E2E is already executing.*do not launch another approval/u, + }, + { + name: "the gate is terminal", + check: { + status: "completed", + conclusion: "failure", + output: { title: "Maintainer approval required to run fork E2E" }, + }, + expected: /coordination is terminal.*do not reuse this approval/u, + }, + { + name: "the coordination title is malformed", + check: { + status: "in_progress", + conclusion: null, + output: { title: "unexpected remote title" }, + }, + expected: /coordination is malformed or unknown.*do not retry/u, + }, + { + name: "the coordination title is missing", + check: { + status: "in_progress", + conclusion: null, + output: {}, + }, + expected: /coordination is malformed or unknown.*do not retry/u, + }, + { + name: "the coordination title is null", + check: { + status: "in_progress", + conclusion: null, + output: { title: null }, + }, + expected: /coordination is malformed or unknown.*do not retry/u, + }, + { + name: "the coordination title is not a string", + check: { + status: "in_progress", + conclusion: null, + output: { title: 7319 }, + }, + expected: /coordination is malformed or unknown.*do not retry/u, + }, + ])("rejects fork authorization when $name", async ({ check, expected }) => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-title-")); + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.endsWith("/collaborators/maintainer/permission"), + () => githubResponse({ role_name: "maintain", user: { login: "maintainer" } }), + ), + githubFetchRoute( + ({ url }) => url.endsWith("/pulls/42"), + () => githubResponse(forkPullRequest()), + ), + githubFetchRoute( + ({ url }) => url.includes("/pulls/42/files?"), + () => githubResponse([{ filename: "test/e2e/risk-signal-reporter.ts" }]), + ), + existingPrGateCheckRunsRoute(check), + ], + requests, + ), + ); + + try { + const error = await approvePrE2E(approvalCommand(workDir)).then( + () => undefined, + (reason: unknown) => reason, + ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch(expected); + expect((error as Error).message).toContain("Maintainer approval required to run fork E2E"); + expect((error as Error).message).not.toContain("unexpected remote title"); + expect((error as Error).message).not.toContain("7319"); + expect((error as Error).message).not.toContain("null"); + expect(requests.some((request) => request.method === "PATCH")).toBe(false); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); + it("rejects fork authorization from a collaborator below maintainer role", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-role-")); vi.stubEnv("GITHUB_TOKEN", "token"); diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 6813093f21d..01534a5e4e7 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -73,6 +73,7 @@ const FORK_E2E_AUTHORIZATION_TITLE = "Maintainer approval required to run fork E const EVALUATING_PR_COMMIT_TITLE = "Evaluating PR commit"; const RUNNER_LOSS_RETRY_PREPARATION_TITLE = "Preparing one-time hosted-runner-loss retry"; const AUTHORIZED_EXECUTION_TITLE_PREFIX = "E2E execution authorized by @"; +const RUNNING_E2E_TITLE_PATTERN = /^Running [1-9][0-9]* E2E checks?$/u; const PRE_DISPATCH_CHECK_READ_TIMEOUT_MS = 5_000; const RECONCILED_CHILD_VALIDATION_TIMEOUT_MS = 10_000; const CHILD_AUTHORIZATION_PUBLISH_TIMEOUT_MS = 5_000; @@ -2115,6 +2116,43 @@ function authorizedExecutionTitle(maintainer: string): string { return `${AUTHORIZED_EXECUTION_TITLE_PREFIX}${maintainer}`; } +function maintainerApprovalStateError(check: CheckRun, expectedTitle: string): Error { + const title = check.output?.title; + const expected = `Wait for the coordination title "${expectedTitle}", then launch a fresh first-attempt approve-e2e run for the same exact revision.`; + + if ( + check.status === "completed" || + (check.conclusion !== undefined && check.conclusion !== null) + ) { + return new Error( + `PR gate is not ready for maintainer approval: coordination is terminal. Update the PR or rerun eligible CI to create a fresh pending authorization state; do not reuse this approval. Expected title: "${expectedTitle}".`, + ); + } + if ( + check.status === "queued" || + (check.status === "in_progress" && + (title === RESERVED_CHECK_TITLE || + title === EVALUATING_PR_COMMIT_TITLE || + title === RUNNER_LOSS_RETRY_PREPARATION_TITLE)) + ) { + return new Error( + `PR gate is not ready for maintainer approval: coordination is still preparing. ${expected}`, + ); + } + if ( + check.status === "in_progress" && + typeof title === "string" && + (title.startsWith(AUTHORIZED_EXECUTION_TITLE_PREFIX) || RUNNING_E2E_TITLE_PATTERN.test(title)) + ) { + return new Error( + `PR gate is not ready for maintainer approval: E2E is already executing. Follow the existing controller and child run; do not launch another approval. Expected title: "${expectedTitle}".`, + ); + } + return new Error( + `PR gate is not ready for maintainer approval: coordination is malformed or unknown. Inspect the coordination check and do not retry until the state is understood. Expected title: "${expectedTitle}".`, + ); +} + function assertCurrentPreDispatchCheck( history: readonly CheckRun[], options: { repository: string; controllerCheckId: number; expectedCheckTitle: string }, @@ -3250,7 +3288,7 @@ async function startAuthorizedPrGate(command: AuthorizedE2ECommand): Promise