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
9 changes: 9 additions & 0 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2150,6 +2150,15 @@ export async function fetchLivePullRequestState(env: Env, repoFullName: string,
return result?.data.state ?? undefined;
}

/** The PR's LIVE head commit SHA via REST `GET /pulls/{n}`. The stored `pr.headSha` lags GitHub when a commit
* lands between a webhook and its processing; the gate-override command (#16 / audit) re-fetches the live head
* so the neutral check-run targets the commit a maintainer is actually looking at, not a phantom old SHA.
* Best-effort: returns undefined on any error so the caller fails open to the stored head. */
export async function fetchLivePullRequestHeadSha(env: Env, repoFullName: string, prNumber: number, token: string | undefined): Promise<string | undefined> {
const result = await githubJsonWithHeaders<{ head?: { sha?: string | null } | null }>(env, repoFullName, `/pulls/${prNumber}`, token).catch(() => undefined);
return result?.data.head?.sha ?? undefined;
}

/** Resolve the OPEN PRs associated with a commit SHA via the REST `GET /repos/{owner}/{repo}/commits/{sha}/pulls`
* endpoint. This is the only PR↔commit resolution that works for FORK (cross-repo) PRs, whose CI-completion
* webhooks (`check_suite`/`check_run`) carry an EMPTY `pull_requests[]`. Returns the de-duplicated open PR numbers.
Expand Down
24 changes: 22 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
fetchAndStorePullRequestFilesForReview,
fetchLinkedIssueFacts,
fetchLiveCiAggregate,
fetchLivePullRequestHeadSha,
fetchLivePullRequestMergeState,
fetchLivePullRequestReviewDecision,
fetchLivePullRequestState,
Expand Down Expand Up @@ -3106,6 +3107,19 @@ async function recordGithubProductUsage(
}).catch(() => undefined);
}

/**
* Resolve the head SHA a `gate-override` should neutralize (#16 / audit). The stored `pr.headSha` lags GitHub
* when a commit lands between the override comment and its processing, so re-fetch the LIVE head and override
* THAT commit (the neutral check-run is per-commit by design). FAIL-OPEN: an unreadable live fetch returns the
* cached head, so a transient GitHub hiccup never strands the override — it just targets the stored SHA as before.
* Mirrors the rebase path's live re-fetch (prReadyForReview) and the dup-winner live reconcile.
*/
export async function resolveOverrideHeadSha(env: Env, installationId: number, repoFullName: string, pr: PullRequestRecord): Promise<string | null | undefined> {
const token = (await createInstallationToken(env, installationId).catch(() => undefined)) ?? env.GITHUB_PUBLIC_TOKEN;
const liveHeadSha = await fetchLivePullRequestHeadSha(env, repoFullName, pr.number, token);
return liveHeadSha ?? pr.headSha;
}

/**
* Handle `@gittensory gate-override <reason>` on a PR thread. SECURITY-SENSITIVE: this finalizes the Gate
* check to neutral for the current commit, so authorization MUST come from real repo permission
Expand Down Expand Up @@ -3171,7 +3185,13 @@ async function maybeProcessGateOverrideCommand(env: Env, deliveryId: string, pay
return true;
}

const { advisory } = await buildAuthorizedPrActionAdvisory(env, repoFullName, pr, settings);
// #16 (audit): the cached pr.headSha can be stale if a commit landed between the comment and this processing.
// The override is a per-commit neutral check-run, so posting it on the cached SHA is a silent no-op on the LIVE
// head (whose Gate check stays blocking). Re-fetch the live head and override THAT commit (fail-open to the
// cached head), then thread it through the advisory so the check-run + audit target the right SHA.
const headForOverride = await resolveOverrideHeadSha(env, installationId, repoFullName, pr);
const prAtLiveHead = headForOverride === pr.headSha ? pr : { ...pr, headSha: headForOverride };
const { advisory } = await buildAuthorizedPrActionAdvisory(env, repoFullName, prAtLiveHead, settings);
const safeReason = sanitizePublicComment((command.reason ?? "").trim() || "No reason provided.");
await createOrUpdateOverriddenGateCheckRun(env, installationId, repoFullName, advisory, { actor, reason: safeReason });
await recordAuditEvent(env, {
Expand All @@ -3180,7 +3200,7 @@ async function maybeProcessGateOverrideCommand(env: Env, deliveryId: string, pay
targetKey: `${repoFullName}#${pr.number}`,
outcome: "completed",
detail: safeReason,
metadata: { deliveryId, repoFullName, headSha: advisory.headSha ?? null },
metadata: { deliveryId, repoFullName, headSha: advisory.headSha ?? null, cachedHeadSha: pr.headSha ?? null },
});
const confirmation = sanitizePublicComment(
[
Expand Down
133 changes: 133 additions & 0 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6214,6 +6214,139 @@ describe("queue processors", () => {
expect(overrideAdvisory ?? null).toBeNull();
});

it("overrides the LIVE head, not the stale cached SHA, when a commit landed after the command (#16)", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123);
await upsertRepositorySettings(env, {
repoFullName: "JSONbored/gittensory",
commentMode: "off",
publicSurface: "off",
autoLabelEnabled: false,
checkRunMode: "off",
gateCheckMode: "enabled",
linkedIssueGateMode: "off",
});
// The stored row still carries the OLD head; a new commit ("live-sha") landed between the comment and now.
await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
number: 90,
title: "Override me",
state: "open",
user: { login: "contributor" },
author_association: "CONTRIBUTOR",
head: { sha: "stale-sha" },
labels: [],
body: "Validation: npm test",
});
const seen = { staleCheckGets: 0, liveCheckGets: 0 };
const patchBodies: Array<{ conclusion?: string }> = [];
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/collaborators/maintainer/permission")) return Response.json({ permission: "admin" });
// The LIVE head re-fetch — the row says stale-sha but GitHub's head is now live-sha.
if (url.includes("/pulls/90") && method === "GET") return Response.json({ number: 90, state: "open", head: { sha: "live-sha" } });
if (url.includes("/commits/stale-sha/check-runs") && method === "GET") {
seen.staleCheckGets += 1;
return Response.json({ total_count: 0, check_runs: [] });
}
if (url.includes("/commits/live-sha/check-runs") && method === "GET") {
seen.liveCheckGets += 1;
return Response.json({ total_count: 1, check_runs: [{ id: 556, name: "Gittensory Gate" }] });
}
if (url.includes("/check-runs/556") && method === "PATCH") {
patchBodies.push(JSON.parse(String(init?.body ?? "{}")) as { conclusion?: string });
return Response.json({ id: 556 });
}
if (url.includes("/issues/90/comments") && method === "GET") return Response.json([]);
if (url.includes("/issues/90/comments") && method === "POST") return Response.json({ id: 9101 });
return new Response("not found", { status: 404 });
});

await processJob(env, {
type: "github-webhook",
deliveryId: "gate-override-live-head",
eventName: "issue_comment",
payload: {
action: "created",
installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
issue: { number: 90, title: "Override me", state: "open", user: { login: "contributor" }, pull_request: {} },
comment: { id: 803, body: "@gittensory gate-override flaky", author_association: "NONE", user: { login: "maintainer", type: "User" } },
sender: { login: "maintainer", type: "User" },
},
});

// The neutral PATCH targeted the LIVE head's Gate run (id 556), and the stale SHA was never touched.
expect(seen.liveCheckGets).toBe(1);
expect(seen.staleCheckGets).toBe(0);
expect(patchBodies[0]?.conclusion).toBe("neutral");
const audit = await env.DB.prepare("select metadata_json from audit_events where event_type = ?")
.bind("github_app.gate_overridden")
.first<{ metadata_json: string }>();
const metadata = JSON.parse(audit?.metadata_json ?? "{}") as { headSha?: string; cachedHeadSha?: string };
expect(metadata.headSha).toBe("live-sha");
expect(metadata.cachedHeadSha).toBe("stale-sha");
});

it("records null head SHAs in the override audit when the PR head is unresolved (#16 fail-safe)", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123);
await upsertRepositorySettings(env, {
repoFullName: "JSONbored/gittensory",
commentMode: "off",
publicSurface: "off",
autoLabelEnabled: false,
checkRunMode: "off",
gateCheckMode: "enabled",
linkedIssueGateMode: "off",
});
// A cached row with no head SHA (never detail-synced); the live fetch also yields no head.
await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
number: 90,
title: "Override me",
state: "open",
user: { login: "contributor" },
author_association: "CONTRIBUTOR",
head: {},
labels: [],
body: "Validation: npm test",
});
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/collaborators/maintainer/permission")) return Response.json({ permission: "admin" });
if (url.includes("/pulls/90") && method === "GET") return Response.json({ number: 90, state: "open", head: {} });
if (url.includes("/issues/90/comments") && method === "GET") return Response.json([]);
if (url.includes("/issues/90/comments") && method === "POST") return Response.json({ id: 9102 });
return new Response("not found", { status: 404 });
});

await processJob(env, {
type: "github-webhook",
deliveryId: "gate-override-null-head",
eventName: "issue_comment",
payload: {
action: "created",
installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
issue: { number: 90, title: "Override me", state: "open", user: { login: "contributor" }, pull_request: {} },
// No reason after the command — exercises the "No reason provided." fallback too.
comment: { id: 804, body: "@gittensory gate-override", author_association: "NONE", user: { login: "maintainer", type: "User" } },
sender: { login: "maintainer", type: "User" },
},
});

const audit = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ?")
.bind("github_app.gate_overridden")
.first<{ detail: string; metadata_json: string }>();
expect(audit?.detail).toBe("No reason provided.");
const metadata = JSON.parse(audit?.metadata_json ?? "{}") as { headSha?: string | null; cachedHeadSha?: string | null };
expect(metadata.headSha).toBeNull();
expect(metadata.cachedHeadSha).toBeNull();
});

it("ignores gate-override commands on edited comments", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123);
Expand Down
62 changes: 62 additions & 0 deletions test/unit/resolve-override-head-sha.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { resolveOverrideHeadSha } from "../../src/queue/processors";
import { createInstallationToken } from "../../src/github/app";
import type { PullRequestRecord } from "../../src/types";
import { createTestEnv } from "../helpers/d1";

// resolveOverrideHeadSha re-fetches the live head via createInstallationToken + REST /pulls/{n}; mock the token
// mint (the test env holds no App key) and stub fetch per case.
vi.mock("../../src/github/app", async (importOriginal) => ({
...(await importOriginal<typeof import("../../src/github/app")>()),
createInstallationToken: vi.fn(),
}));
const mockedToken = vi.mocked(createInstallationToken);

function makePr(headSha: string | null): PullRequestRecord {
return { repoFullName: "owner/repo", number: 90, title: "Override me", state: "open", labels: [], linkedIssues: [], headSha };
}

function stubLiveHead(sha: string | null): void {
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
if (String(input).includes("/pulls/90")) return new Response(JSON.stringify(sha === null ? {} : { head: { sha } }), { status: 200 });
return new Response("not found", { status: 404 });
});
}

describe("resolveOverrideHeadSha (#16 / gate-override stale head)", () => {
beforeEach(() => {
mockedToken.mockReset();
});
afterEach(() => {
vi.unstubAllGlobals();
});

it("returns the LIVE head when the token mint succeeds (overrides the current commit, not the cached one)", async () => {
const env = createTestEnv();
mockedToken.mockResolvedValue("inst-tok");
stubLiveHead("live-sha");
expect(await resolveOverrideHeadSha(env, 123, "owner/repo", makePr("stale-sha"))).toBe("live-sha");
expect(mockedToken).toHaveBeenCalledWith(env, 123);
});

it("falls back to the public token when the mint throws, and still resolves the live head", async () => {
const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-tok" });
mockedToken.mockRejectedValue(new Error("no app key"));
stubLiveHead("live-sha");
expect(await resolveOverrideHeadSha(env, 123, "owner/repo", makePr("stale-sha"))).toBe("live-sha");
});

it("fails OPEN to the cached head when the live fetch is unreadable", async () => {
const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-tok" });
mockedToken.mockResolvedValue("inst-tok");
vi.stubGlobal("fetch", async () => new Response("nope", { status: 500 }));
expect(await resolveOverrideHeadSha(env, 123, "owner/repo", makePr("stale-sha"))).toBe("stale-sha");
});

it("returns the cached head when the live payload omits head.sha (fail-open)", async () => {
const env = createTestEnv();
mockedToken.mockResolvedValue("inst-tok");
stubLiveHead(null);
expect(await resolveOverrideHeadSha(env, 123, "owner/repo", makePr("stale-sha"))).toBe("stale-sha");
});
});
Loading