Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions test/e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
137 changes: 136 additions & 1 deletion test/pr-e2e-gate-fork-approval.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand Down
40 changes: 39 additions & 1 deletion tools/e2e/pr-e2e-gate.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -3250,7 +3288,7 @@ async function startAuthorizedPrGate(command: AuthorizedE2ECommand): Promise<voi
const check = matchingChecks[0]!;
const pendingAuthorization = check.status === "in_progress" && check.conclusion === null;
if (!pendingAuthorization || check.output?.title !== pendingTitle) {
throw new Error("PR gate must have the matching pending E2E authorization state");
throw maintainerApprovalStateError(check, pendingTitle);
}
checkRunId = check.id;
appendOutput("check_id", String(checkRunId));
Expand Down
Loading