fix(ci): preserve review states across empty and merge commits without game changes

.github/scripts/auto-triage.mjs

@@ -14,6 +14,7 @@ import {
 	githubRequest,
 	hasLabel,
 	inferSubmissionMetadata,
+	isReviewCommitOutdated,
 	parseTags,
 	readGitHubEvent,
 	removeLabel,
@@ -32,7 +33,7 @@ import {
 	findDuplicateGroup,
 	parseOlderNotice,
 } from "./duplicate-detection.mjs";
-import { autoReviewLabelChanges, reconcileReviewStatus } from "./review-state.mjs";
+import { autoReviewLabelChanges, getActiveReviewCommits, reconcileReviewStatus } from "./review-state.mjs";
 
 const token = process.env.GITHUB_TOKEN;
 if (!token) throw new Error("GITHUB_TOKEN is required");
@@ -523,11 +524,31 @@ function readFileSafe(filePath) {
 	}
 }
 
-async function getLatestReviewStatus({ owner, repo, token, prNumber, reviewers, authorLogin, headSha }) {
+async function getLatestReviewStatus({ owner, repo, token, prNumber, reviewers, authorLogin, headSha, pullFiles }) {
 	if (!reviewers || reviewers.size === 0) return "unknown";
 	try {
 		const reviews = await githubPaginated(token, `/repos/${owner}/${repo}/pulls/${prNumber}/reviews`);
-		return reconcileReviewStatus({ reviews, reviewers, authorLogin, headSha });
+		const pendingCommits = getActiveReviewCommits({ reviews, reviewers, authorLogin, headSha });
+		const outdatedCommitIds = new Set();
+		for (const commitId of pendingCommits) {
+			const isOutdated = await isReviewCommitOutdated({
+				owner,
+				repo,
+				token,
+				prNumber,
+				commitId,
+				headSha,
+				submissionFiles: pullFiles,
+			});
+			if (isOutdated) outdatedCommitIds.add(commitId);
+		}
+		return reconcileReviewStatus({
+			reviews,
+			reviewers,
+			authorLogin,
+			headSha,
+			isCommitOutdated: (cid) => !cid || (cid !== headSha && outdatedCommitIds.has(cid)),
+		});
 	} catch (e) {
 		console.warn("Unable to fetch reviews; skipping review reconciliation:", e.message);
 		return "unknown";
@@ -545,6 +566,7 @@ async function applyLabels(result) {
 		reviewers,
 		authorLogin: pullRequest.user?.login,
 		headSha: pullRequest.head?.sha,
+		pullFiles,
 	});
 	const changes = autoReviewLabelChanges({
 		labels: currentLabels,

.github/scripts/review-state.mjs

@@ -47,8 +47,8 @@ export function autoReviewLabelChanges({ labels, validationOk, eventAction, revi
 	}
 }
 
-export function reconcileReviewStatus({ reviews, reviewers, authorLogin, headSha }) {
-	if (!reviewers || reviewers.size === 0) return "unknown";
+export function getActiveReviews({ reviews, reviewers, authorLogin }) {
+	if (!reviewers || reviewers.size === 0) return [];
 	const lowerReviewers = new Set([...reviewers].map((u) => u.toLowerCase()));
 	const lowerAuthor = authorLogin?.toLowerCase();
 
@@ -63,11 +63,32 @@ export function reconcileReviewStatus({ reviews, reviewers, authorLogin, headSha
 		}
 	}
 
-	const active = [...latestByReviewer.values()];
-	if (active.some((r) => r.state === "CHANGES_REQUESTED" && (!headSha || r.commit_id === headSha))) {
+	return [...latestByReviewer.values()];
+}
+
+export function getActiveReviewCommits({ reviews, reviewers, authorLogin, headSha }) {
+	const active = getActiveReviews({ reviews, reviewers, authorLogin });
+	const commits = new Set();
+	for (const r of active) {
+		if ((r.state === "APPROVED" || r.state === "CHANGES_REQUESTED") && r.commit_id && r.commit_id !== headSha) {
+			commits.add(r.commit_id);
+		}
+	}
+	return [...commits];
+}
+
+export function reconcileReviewStatus({ reviews, reviewers, authorLogin, headSha, isCommitOutdated }) {
+	if (!reviewers || reviewers.size === 0) return "unknown";
+
+	// ponytail: missing commit ID or mismatch defaults to outdated; upgrade path is passing isCommitOutdated comparing game files
+	const isOutdated = (commitId) =>
+		!commitId || (isCommitOutdated ? isCommitOutdated(commitId) : Boolean(headSha) && commitId !== headSha);
+
+	const active = getActiveReviews({ reviews, reviewers, authorLogin });
+	if (active.some((r) => r.state === "CHANGES_REQUESTED" && !isOutdated(r.commit_id))) {
 		return "changes_requested";
 	}
-	if (active.some((r) => r.state === "APPROVED" && Boolean(headSha) && r.commit_id === headSha)) {
+	if (active.some((r) => r.state === "APPROVED" && Boolean(headSha) && !isOutdated(r.commit_id))) {
 		return "approved";
 	}
 	if (active.some((r) => r.state === "DISMISSED")) {

.github/scripts/review-state.test.mjs

@@ -1,5 +1,6 @@
-import { describe, expect, it } from "vitest";
-import { autoReviewLabelChanges, reconcileReviewStatus } from "./review-state.mjs";
+import { describe, expect, it, vi } from "vitest";
+import { autoReviewLabelChanges, getActiveReviewCommits, getActiveReviews, reconcileReviewStatus } from "./review-state.mjs";
+import { isReviewCommitOutdated } from "./review-utils.mjs";
 
 describe("autoReviewLabelChanges", () => {
 	it("sets Ready for Playtest on passing validation for new submissions", () => {
@@ -203,4 +204,159 @@ describe("reconcileReviewStatus", () => {
 	it("returns unknown when reviewers set is empty or undefined", () => {
 		expect(reconcileReviewStatus({ reviews: [], reviewers: null, authorLogin: "author1", headSha })).toBe("unknown");
 	});
+
+	it("preserves changes_requested across non-game commits when isCommitOutdated is false", () => {
+		const reviews = [
+			{ user: { login: "SSoggyTacoMan" }, state: "CHANGES_REQUESTED", commit_id: "mergeSha", submitted_at: "2026-10-04T09:00:00Z" },
+		];
+		const status = reconcileReviewStatus({
+			reviews,
+			reviewers,
+			authorLogin: "author1",
+			headSha,
+			isCommitOutdated: (cid) => cid !== "mergeSha",
+		});
+		expect(status).toBe("changes_requested");
+	});
+
+	it("preserves approved across non-game commits when isCommitOutdated is false", () => {
+		const reviews = [
+			{ user: { login: "LucasHT22" }, state: "APPROVED", commit_id: "mergeSha", submitted_at: "2026-10-04T09:00:00Z" },
+		];
+		const status = reconcileReviewStatus({
+			reviews,
+			reviewers,
+			authorLogin: "author1",
+			headSha,
+			isCommitOutdated: (cid) => cid !== "mergeSha",
+		});
+		expect(status).toBe("approved");
+	});
+
+	it("resets changes_requested to none when isCommitOutdated is true", () => {
+		const reviews = [
+			{ user: { login: "SSoggyTacoMan" }, state: "CHANGES_REQUESTED", commit_id: "olderSha", submitted_at: "2026-10-04T09:00:00Z" },
+		];
+		const status = reconcileReviewStatus({
+			reviews,
+			reviewers,
+			authorLogin: "author1",
+			headSha,
+			isCommitOutdated: () => true,
+		});
+		expect(status).toBe("none");
+	});
+
+	it("treats null or missing commit_id as outdated even with custom isCommitOutdated", () => {
+		const reviews = [
+			{ user: { login: "LucasHT22" }, state: "APPROVED", commit_id: null, submitted_at: "2026-10-04T09:00:00Z" },
+		];
+		const status = reconcileReviewStatus({
+			reviews,
+			reviewers,
+			authorLogin: "author1",
+			headSha,
+			isCommitOutdated: () => false,
+		});
+		expect(status).toBe("none");
+	});
+});
+
+describe("getActiveReviews and getActiveReviewCommits", () => {
+	it("filters out non-reviewers, author, and non-actionable review states", () => {
+		const reviews = [
+			{ user: { login: "random" }, state: "APPROVED", commit_id: "sha1", submitted_at: "2026-10-04T09:00:00Z" },
+			{ user: { login: "author1" }, state: "APPROVED", commit_id: "sha1", submitted_at: "2026-10-04T09:00:00Z" },
+			{ user: { login: "SSoggyTacoMan" }, state: "COMMENTED", commit_id: "sha1", submitted_at: "2026-10-04T09:00:00Z" },
+			{ user: { login: "SSoggyTacoMan" }, state: "CHANGES_REQUESTED", commit_id: "sha2", submitted_at: "2026-10-04T10:00:00Z" },
+			{ user: { login: "LucasHT22" }, state: "APPROVED", commit_id: "sha3", submitted_at: "2026-10-04T11:00:00Z" },
+		];
+		const active = getActiveReviews({ reviews, reviewers: new Set(["ssoggytacoman", "lucasht22"]), authorLogin: "author1" });
+		expect(active).toHaveLength(2);
+		expect(active.map((r) => r.user.login)).toEqual(["SSoggyTacoMan", "LucasHT22"]);
+
+		const commits = getActiveReviewCommits({
+			reviews,
+			reviewers: new Set(["ssoggytacoman", "lucasht22"]),
+			authorLogin: "author1",
+			headSha: "sha3",
+		});
+		expect(commits).toEqual(["sha2"]);
+	});
+});
+
+describe("isReviewCommitOutdated", () => {
+	it("returns false when commitId matches headSha or either is missing", async () => {
+		expect(await isReviewCommitOutdated({ commitId: "abc", headSha: "abc" })).toBe(false);
+		expect(await isReviewCommitOutdated({ commitId: null, headSha: "abc" })).toBe(false);
+		expect(await isReviewCommitOutdated({ commitId: "abc", headSha: null })).toBe(false);
+	});
+
+	it("returns false when file blob SHA at commitId matches headSha", async () => {
+		const originalFetch = globalThis.fetch;
+		globalThis.fetch = vi.fn().mockResolvedValue({
+			ok: true,
+			status: 200,
+			text: async () => JSON.stringify({ sha: "blobSha1" }),
+		});
+
+		try {
+			const outdated = await isReviewCommitOutdated({
+				owner: "hackclub",
+				repo: "sprig",
+				token: "token",
+				prNumber: 123,
+				commitId: "sha1",
+				headSha: "sha2",
+				submissionFiles: [{ filename: "games/my-game.js", sha: "blobSha1" }],
+			});
+			expect(outdated).toBe(false);
+		} finally {
+			globalThis.fetch = originalFetch;
+		}
+	});
+
+	it("returns true when file blob SHA at commitId differs from headSha", async () => {
+		const originalFetch = globalThis.fetch;
+		globalThis.fetch = vi.fn().mockResolvedValue({
+			ok: true,
+			status: 200,
+			text: async () => JSON.stringify({ sha: "differentBlobSha" }),
+		});
+
+		try {
+			const outdated = await isReviewCommitOutdated({
+				owner: "hackclub",
+				repo: "sprig",
+				token: "token",
+				prNumber: 123,
+				commitId: "sha1",
+				headSha: "sha2",
+				submissionFiles: [{ filename: "games/my-game.js", sha: "blobSha1" }],
+			});
+			expect(outdated).toBe(true);
+		} finally {
+			globalThis.fetch = originalFetch;
+		}
+	});
+
+	it("returns true as safe fallback when contents request throws", async () => {
+		const originalFetch = globalThis.fetch;
+		globalThis.fetch = vi.fn().mockRejectedValue(new Error("Network error"));
+
+		try {
+			const outdated = await isReviewCommitOutdated({
+				owner: "hackclub",
+				repo: "sprig",
+				token: "token",
+				prNumber: 123,
+				commitId: "sha1",
+				headSha: "sha2",
+				submissionFiles: [{ filename: "games/my-game.js", sha: "blobSha1" }],
+			});
+			expect(outdated).toBe(true);
+		} finally {
+			globalThis.fetch = originalFetch;
+		}
+	});
 });

.github/scripts/review-utils.mjs

@@ -276,6 +276,49 @@ export function daysBetween(start, end = new Date()) {
 	return Math.floor((endTime - startTime) / 86_400_000);
 }
 
+export async function isReviewCommitOutdated({ owner, repo, token, prNumber, commitId, headSha, submissionFiles }) {
+	if (!commitId || !headSha || commitId === headSha) return false;
+	try {
+		const files = submissionFiles ?? (await githubPaginated(token, `/repos/${owner}/${repo}/pulls/${prNumber}/files`));
+		const submissionFileObjects = files.filter((f) => {
+			const fn = typeof f === "string" ? f : f.filename;
+			return fn.toLowerCase().startsWith("games/");
+		});
+		if (submissionFileObjects.length === 0) return true;
+
+		for (const file of submissionFileObjects) {
+			const filename = typeof file === "string" ? file : file.filename;
+			const targetHeadSha =
+				typeof file === "object" && file?.sha
+					? file.sha
+					: await getFileBlobSha({ owner, repo, token, path: filename, ref: headSha });
+			if (!targetHeadSha) return true;
+
+			const commitBlobSha = await getFileBlobSha({ owner, repo, token, path: filename, ref: commitId });
+			if (!commitBlobSha || commitBlobSha !== targetHeadSha) {
+				return true;
+			}
+		}
+
+		return false;
+	} catch {
+		return true;
+	}
+}
+
+async function getFileBlobSha({ owner, repo, token, path: filePath, ref }) {
+	try {
+		const data = await githubRequest(
+			token,
+			"GET",
+			`/repos/${owner}/${repo}/contents/${filePath.split("/").map(encodeURIComponent).join("/")}?ref=${encodeURIComponent(ref)}`
+		);
+		return data?.sha ?? null;
+	} catch {
+		return null;
+	}
+}
+
 export function getMetadataValue(content, key) {
 	const match = content.match(new RegExp(String.raw`@${key}:[^\S\r\n]*([\s\S]*?)(?=\r?\n\s*\*?\s*@|\r?\n\s*\*\/|\*\/)`, "i"));
 	if (!match?.[1]) return "";