fix(ci): clarify image triage checks and prevent bad filename cascading

.github/scripts/auto-triage.mjs

@@ -21,7 +21,7 @@ import {
 	upsertBotComment,
 	validateMetadataHeader,
 } from "./review-utils.mjs";
-import { buildSubmissionManifest } from "./submission-manifest.mjs";
+import { buildSubmissionManifest, getImageCountCheck, validateImageFiles } from "./submission-manifest.mjs";
 import {
 	DUPLICATE_LABEL,
 	LEGACY_DUPLICATE_LABEL,
@@ -355,10 +355,8 @@ async function validateSubmission({ pullRequest, pullFiles, workspace, reviewBas
 		playUrl = buildReviewUrl(reviewBaseUrl);
 	}
 
-	if (gameFile) {
-		const gameBase = path.basename(gameFile.filename, ".js");
-		screenshotUrl = validateImages(imageFiles, gameBase, owner, repo, pullRequest, addCheck);
-	}
+	const gameBase = gameFile ? path.basename(gameFile.filename, ".js") : null;
+	screenshotUrl = validateImages(imageFiles, gameBase, owner, repo, pullRequest, addCheck);
 
 	const ok = checks.every((check) => check.ok);
 	return {
@@ -585,6 +583,7 @@ function buildComment(result) {
 	const categoryMap = {
 		"Files stay in allowed folders": "file",
 		"Exactly one game file": "file",
+		"At most one image file": "file",
 		"Only new files added": "file",
 		"Filename uses safe characters": "file",
 		"Game file is directly inside games/": "file",
@@ -705,8 +704,9 @@ function formatPercent(value) {
 
 function validateSubmissionFiles(pullFiles, addCheck) {
 	const manifest = buildSubmissionManifest(pullFiles);
-	const { gameFiles: jsFiles, gameFilesLoose, imageFiles, disallowedFiles, uppercaseGames, changedNonAddedFiles } = manifest;
+	const { gameFiles: jsFiles, gameFilesLoose, allImageFiles, disallowedFiles, uppercaseGames, changedNonAddedFiles } = manifest;
 	const effectiveGameFiles = gameFilesLoose;
+	const effectiveImageFiles = allImageFiles;
 	if (uppercaseGames.length > 0) {
 		const badNames = uppercaseGames.map((file) => `\`${file.filename}\``).join(", ");
 		addCheck("Directory must be lowercase", false, `Your file must be in the lowercase \`games/\` directory. Found ${badNames}.`);
@@ -732,6 +732,9 @@ function validateSubmissionFiles(pullFiles, addCheck) {
 				: `Only one game file is allowed per submission. Found ${jsNames}.`
 	);
 
+	const imgCountCheck = getImageCountCheck(effectiveImageFiles);
+	addCheck(imgCountCheck.name, imgCountCheck.ok, imgCountCheck.detail);
+
 	const changedNames = changedNonAddedFiles.map((file) => `\`${file.filename}\``).join(", ");
 	addCheck(
 		"Only new or game files changed",
@@ -740,27 +743,19 @@ function validateSubmissionFiles(pullFiles, addCheck) {
 			? `Submissions should only add new game files or modify existing ones in \`games/\`. These files outside \`games/\` were modified: ${changedNames}.`
 			: "All submitted files are new or inside \`games/\`."
 	);
-	return { jsFiles: effectiveGameFiles, imageFiles };
+	return { jsFiles: effectiveGameFiles, imageFiles: effectiveImageFiles };
 }
 
 function validateImages(imageFiles, gameBase, owner, repo, pullRequest, addCheck) {
-	const badImagePaths = imageFiles.filter((file) => !file.filename.startsWith("games/img/"));
-	const badImgNames = badImagePaths.map((file) => `\`${file.filename}\``).join(", ");
-	let imgPathDetail = "No image provided; this is OK.";
-	if (badImagePaths.length) imgPathDetail = `Move images into \`games/img/\`: ${badImgNames}.`;
-	else if (imageFiles.length) imgPathDetail = "Image files are in `games/img/`.";
-	addCheck("Optional image path", badImagePaths.length === 0, imgPathDetail);
-
-	const mismatchedImages = imageFiles.filter((file) => path.basename(file.filename, path.extname(file.filename)) !== gameBase);
-	const mismatchNames = mismatchedImages.map((file) => `\`${file.filename}\``).join(", ");
-	let imgNameDetail = "No image provided; gallery thumbnail can be generated/default.";
-	if (mismatchedImages.length) imgNameDetail = `Image filename must match \`${gameBase}.js\`. Found ${mismatchNames}.`;
-	else if (imageFiles.length) imgNameDetail = "Image filename matches the game file.";
-	addCheck("Optional image name", mismatchedImages.length === 0, imgNameDetail);
-
-	if (imageFiles.length > 0) {
-		const image = imageFiles.find((file) => path.basename(file.filename, path.extname(file.filename)) === gameBase) ?? imageFiles[0];
-		return `https://raw.githubusercontent.com/${owner}/${repo}/${pullRequest.head.sha}/${image.filename}`;
+	const checks = validateImageFiles({ imageFiles, gameBase });
+	for (const check of checks) {
+		addCheck(check.name, check.ok, check.detail);
+	}
+
+	if (imageFiles.length > 0 && pullRequest?.head?.sha) {
+		const matchingImage = gameBase ? imageFiles.find((file) => file.filename === `games/img/${gameBase}.png`) : null;
+		const image = matchingImage ?? imageFiles[0];
+		return `https://raw.githubusercontent.com/${owner}/${repo}/${pullRequest.head.sha}/${image.filename.split("/").map(encodeURIComponent).join("/")}`;
 	}
 	return null;
 }
@@ -784,12 +779,16 @@ async function validateSingleGameFile(gameFile, workspace, addCheck, warnings) {
 		return { metadata, similarity };
 	}
 
+	const safeFilename = /^[a-zA-Z0-9_-]+\.js$/.test(filename);
+	const base = path.basename(filename, ".js");
+	const suggestedBase = base.trim().replace(/\s+/g, "_").replace(/[^a-zA-Z0-9_-]/g, "") || "game";
+	const suggestedFilename = `${suggestedBase}.js`;
 	addCheck(
 		"Filename uses safe characters",
-		/^[a-zA-Z0-9_-]+\.js$/.test(filename),
-		/^[a-zA-Z0-9_-]+\.js$/.test(filename)
+		safeFilename,
+		safeFilename
 			? "Filename is safe."
-			: `Rename \`${filename}\` to use only letters, numbers, \`-\`, and \`_\`.`
+			: `Rename \`${filename}\` to use only letters, numbers, \`-\`, and \`_\`${suggestedFilename !== filename ? ` (for example: \`${suggestedFilename}\`)` : ""}.`
 	);
 
 	addCheck(

.github/scripts/submission-manifest.mjs

@@ -1,6 +1,8 @@
 const GAME_FILE_RE = /^games\/[A-Za-z0-9_-]+\.js$/;
 const GAME_FILE_LOOSE_RE = /^games\/[^/]+\.js$/;
 const IMAGE_FILE_RE = /^games\/img\/[A-Za-z0-9_-]+\.png$/i;
+const IMAGE_FILE_LOOSE_RE = /^games\/img\/[^/]+$/;
+const ANY_IMAGE_RE = /\.(png|jpe?g|gif|webp)$/i;
 
 export function buildSubmissionManifest(pullFiles) {
 	const files = pullFiles.map((file) => ({
@@ -10,17 +12,69 @@ export function buildSubmissionManifest(pullFiles) {
 		isGame: GAME_FILE_RE.test(file.filename),
 		isGameLoose: GAME_FILE_LOOSE_RE.test(file.filename),
 		isImage: IMAGE_FILE_RE.test(file.filename),
-		isAllowed: GAME_FILE_LOOSE_RE.test(file.filename) || IMAGE_FILE_RE.test(file.filename),
+		isImageLoose: IMAGE_FILE_LOOSE_RE.test(file.filename),
+		isAnyImage: ANY_IMAGE_RE.test(file.filename),
+		isAllowed: GAME_FILE_LOOSE_RE.test(file.filename) || IMAGE_FILE_LOOSE_RE.test(file.filename),
 	}));
 
 	return {
 		files,
 		gameFiles: files.filter((file) => file.isGame),
 		gameFilesLoose: files.filter((file) => file.isGameLoose),
 		imageFiles: files.filter((file) => file.isImage),
+		imageFilesLoose: files.filter((file) => file.isImageLoose),
+		allImageFiles: files.filter((file) => file.isImageLoose || file.isAnyImage),
 		disallowedFiles: files.filter((file) => !file.isAllowed),
 		changedNonAddedFiles: files.filter((file) => file.status !== "added" && !file.filename.startsWith("games/")),
 		uppercaseGames: files.filter((file) => file.filename.toLowerCase().startsWith("games/") && !file.filename.startsWith("games/")),
 		renamedFiles: files.filter((file) => file.status === "renamed" || file.previousFilename),
 	};
 }
+
+export function getImageCountCheck(imageFiles) {
+	const ok = imageFiles.length <= 1;
+	const imgNames = imageFiles.map((file) => `\`${file.filename}\``).join(", ");
+	const detail = ok
+		? imageFiles.length === 1
+			? "Exactly one image file provided."
+			: "No image provided; this is OK."
+		: `Only one image is allowed per game (used as the gallery thumbnail). Found ${imageFiles.length} images: ${imgNames}. Please delete the extra image(s).`;
+	return { name: "At most one image file", ok, detail };
+}
+
+export function validateImageFiles({ imageFiles, gameBase }) {
+	const checks = [];
+	const badImagePaths = imageFiles.filter((file) => !file.filename.startsWith("games/img/"));
+	const badImgNames = badImagePaths.map((file) => `\`${file.filename}\``).join(", ");
+	let imgPathDetail = "No image provided; this is OK.";
+	if (badImagePaths.length) imgPathDetail = `Move images into \`games/img/\`: ${badImgNames}.`;
+	else if (imageFiles.length) imgPathDetail = "Image files are in `games/img/`.";
+	checks.push({ name: "Optional image path", ok: badImagePaths.length === 0, detail: imgPathDetail });
+
+	let imgNameOk = true;
+	let imgNameDetail = "No image provided; gallery thumbnail can be generated/default.";
+
+	if (imageFiles.length > 0) {
+		if (!gameBase) {
+			imgNameOk = false;
+			imgNameDetail = "Add exactly one game file in `games/` so the image filename can match it.";
+		} else if (!/^[a-zA-Z0-9_-]+$/.test(gameBase)) {
+			imgNameOk = false;
+			const suggestedBase = gameBase.trim().replace(/\s+/g, "_").replace(/[^a-zA-Z0-9_-]/g, "") || "game";
+			imgNameDetail = `Both your game file and image filename must match and use only letters, numbers, '-', and '_'. For example, rename to \`games/${suggestedBase}.js\` and \`games/img/${suggestedBase}.png\`.`;
+		} else {
+			const expectedImage = `games/img/${gameBase}.png`;
+			const mismatchedImages = imageFiles.filter((file) => file.filename !== expectedImage);
+			if (mismatchedImages.length > 0) {
+				imgNameOk = false;
+				const mismatchNames = mismatchedImages.map((file) => `\`${file.filename}\``).join(", ");
+				imgNameDetail = `Image must be named \`${expectedImage}\` to match \`games/${gameBase}.js\`. Found ${mismatchNames}.`;
+			} else {
+				imgNameDetail = "Image filename matches the game file.";
+			}
+		}
+	}
+
+	checks.push({ name: "Optional image name", ok: imgNameOk, detail: imgNameDetail });
+	return checks;
+}

.github/scripts/submission-manifest.test.mjs

@@ -0,0 +1,62 @@
+import { describe, expect, it } from "vitest";
+import {
+	buildSubmissionManifest,
+	getImageCountCheck,
+	validateImageFiles,
+} from "./submission-manifest.mjs";
+
+describe("buildSubmissionManifest", () => {
+	it("classifies valid files and separates loose/disallowed paths", () => {
+		const manifest = buildSubmissionManifest([
+			{ filename: "games/my_game.js", status: "added" },
+			{ filename: "games/img/my_game.png", status: "added" },
+			{ filename: "games/img/Retro space shooter.png", status: "added" },
+			{ filename: "screenshots/my_game.png", status: "added" },
+			{ filename: "games/sub/nested.js", status: "added" },
+		]);
+		expect(manifest.gameFiles).toHaveLength(1);
+		expect(manifest.imageFiles).toHaveLength(1);
+		expect(manifest.imageFilesLoose).toHaveLength(2);
+		expect(manifest.allImageFiles).toHaveLength(3);
+		expect(manifest.disallowedFiles.map((f) => f.filename)).toEqual([
+			"screenshots/my_game.png",
+			"games/sub/nested.js",
+		]);
+	});
+});
+
+describe("getImageCountCheck", () => {
+	it.each([
+		{ count: 0, files: [], ok: true },
+		{ count: 1, files: [{ filename: "games/img/a.png" }], ok: true },
+		{ count: 2, files: [{ filename: "games/img/a.png" }, { filename: "games/img/b.png" }], ok: false },
+	])("handles $count images (ok: $ok)", ({ files, ok }) => {
+		const check = getImageCountCheck(files);
+		expect(check.ok).toBe(ok);
+		if (!ok) {
+			expect(check.detail).toContain("Only one image is allowed per game");
+			expect(check.detail).toContain("`games/img/a.png`");
+		}
+	});
+});
+
+describe("validateImageFiles", () => {
+	it.each([
+		{ files: [], base: "my_game", pathOk: true, nameOk: true, match: /gallery thumbnail/ },
+		{ files: ["games/img/my_game.png"], base: "my_game", pathOk: true, nameOk: true, match: /matches the game/ },
+		{ files: ["games/my_game.png"], base: "my_game", pathOk: false, nameOk: false, match: /Image must be named `games\/img\/my_game\.png`/ },
+		{ files: ["screenshots/thumb.png"], base: "my_game", pathOk: false, nameOk: false, match: /Image must be named `games\/img\/my_game\.png`/ },
+		{ files: ["games/img/wrong.png"], base: "my_game", pathOk: true, nameOk: false, match: /Image must be named `games\/img\/my_game\.png`/ },
+		{ files: ["games/img/my_game.png"], base: null, pathOk: true, nameOk: false, match: /Add exactly one game file/ },
+		{ files: ["games/img/Retro space.png"], base: "Retro space", pathOk: true, nameOk: false, match: /rename to `games\/Retro_space\.js` and `games\/img\/Retro_space\.png`/ },
+		{ files: ["games/img/dr. mario.png"], base: "dr. mario", pathOk: true, nameOk: false, match: /games\/dr_mario\.js/ },
+		{ files: ["games/img/@#$.png"], base: "@#$", pathOk: true, nameOk: false, match: /games\/game\.js/ },
+	])("validates files for base '$base'", ({ files, base, pathOk, nameOk, match }) => {
+		const imageFiles = files.map((filename) => ({ filename }));
+		const checks = validateImageFiles({ imageFiles, gameBase: base });
+		expect(checks.find((c) => c.name === "Optional image path")?.ok).toBe(pathOk);
+		const nameCheck = checks.find((c) => c.name === "Optional image name");
+		expect(nameCheck?.ok).toBe(nameOk);
+		expect(nameCheck?.detail).toMatch(match);
+	});
+});