diff --git a/.github/scripts/enforce-pr-target.test.cjs b/.github/scripts/enforce-pr-target.test.cjs index bf5497d7ff..a2e835f9eb 100644 --- a/.github/scripts/enforce-pr-target.test.cjs +++ b/.github/scripts/enforce-pr-target.test.cjs @@ -197,6 +197,9 @@ describe("enforce-pr-target workflow", () => { ); assert.ok(qualityCall, "must call collectPrQualityFailures"); assert.match(qualityCall[1], /stackedBase/); + assert.match(qualityCall[1], /changedFilePaths/); + assert.match(qualityCall[1], /filesTruncated/); + assert.match(workflow, /isChangedFileListTruncated/); }); it("strips stale WRONG BRANCH prefix on failure when base is corrected", () => { diff --git a/.github/scripts/pr-quality-messages.cjs b/.github/scripts/pr-quality-messages.cjs index aec05331b8..6d9b2fff02 100644 --- a/.github/scripts/pr-quality-messages.cjs +++ b/.github/scripts/pr-quality-messages.cjs @@ -229,7 +229,7 @@ function buildFailureSections(failures, { pr, allowedBases, defaultBase }) { sections.push( "⚠️ **UI screenshot required**", "", - `This pull request mentions ${inlineCode("gui")} in its title or description, so it is treated as a GUI change.`, + `This pull request changes files under ${inlineCode("gui/")}, or GitHub returned an incomplete changed-file list for a large diff, so it is treated as a GUI change.`, "", `@${pr.user.login} Please add a screenshot of the UI change to the description — drag and drop the image into the description editor, or paste a markdown image such as ${inlineCode("![Screenshot](https://example.com/after.png)")}. The check re-runs automatically once the description is edited.` ); diff --git a/.github/scripts/pr-quality.cjs b/.github/scripts/pr-quality.cjs index eaceab06ad..cc841e3809 100644 --- a/.github/scripts/pr-quality.cjs +++ b/.github/scripts/pr-quality.cjs @@ -168,16 +168,46 @@ function assessPrDescription(body) { return { ok: false, reason: "thin" }; } +/** + * True when any changed path is the gui directory or inside it (slash-guarded). + * Mirrors `guiPathsChanged` in `scripts/doctor-gui-if-changed.ts`. + */ +function guiPathsChanged(files) { + return files.some( + (file) => file === "gui" || file.startsWith("gui/") + ); +} + +/** + * True when the changed-file list from `pulls.listFiles` cannot be trusted to + * be complete for screenshot gating. Missing or non-integer counts, a head + * mismatch between the count snapshot and the paginated list, or a count above + * the returned list length all fail closed. + */ +function isChangedFileListTruncated(changedFilesCount, listedLength, headMatches = true) { + if (!headMatches) return true; + if (!Number.isInteger(changedFilesCount) || changedFilesCount < 0) return true; + return changedFilesCount > listedLength; +} + /** * True when the PR title or description names the GUI surface as a whole word. * The description is template-stripped first so the template's own screenshot - * instruction cannot arm the gate on its own. + * instruction cannot arm the gate on its own. Negated phrases such as "no gui + * changes" are not treated as cues (see `segmentHasAffirmativeGuiCue`). */ +function segmentHasAffirmativeGuiCue(text) { + if (typeof text !== "string" || !text.trim()) return false; + const segments = text.split(/(?<=[.!?\n])/); + return segments.some((segment) => { + if (!GUI_CUE_RE.test(segment)) return false; + const withoutNegated = segment.replace(GUI_OVERRIDE_RE, ""); + return GUI_CUE_RE.test(withoutNegated); + }); +} + function hasGuiCue(title, body) { - return ( - (typeof title === "string" && GUI_CUE_RE.test(title)) || - (typeof body === "string" && GUI_CUE_RE.test(body)) - ); + return segmentHasAffirmativeGuiCue(title) || segmentHasAffirmativeGuiCue(body); } /** @@ -447,7 +477,14 @@ function collectPrQualityFailures({ /** True when baseRef is another open PR's head (stacked child). */ stackedBase = false, /** Issue comments; a maintainer comment waives the GUI-screenshot gate. */ - guiOverrideComments = [] + guiOverrideComments = [], + /** Changed file paths from `pulls.listFiles` (repo-relative). */ + changedFilePaths = [], + /** + * True when `pulls.listFiles` returned fewer paths than `pulls.get` + * `changed_files` (GitHub caps the file list at 3,000 entries). + */ + filesTruncated = false }) { const failures = []; const wrongBase = !allowedBases.includes(baseRef) && !stackedBase; @@ -475,14 +512,12 @@ function collectPrQualityFailures({ failures.push({ code: "bad_description", reason: desc.reason }); } - // GUI-cued PRs must prove the UI change visually. The template's own - // screenshot instruction is boilerplate, so it cannot trigger this gate. A - // maintainer comment saying the change does not touch the GUI waives it. + // PRs that change gui/ must prove the UI change visually. Text cues in the + // title or description are not enough — "no gui changes" in the body must + // not arm the gate when the diff is backend-only. A maintainer comment saying + // the change does not touch the GUI still waives a gui/ diff false positive. if ( - hasGuiCue( - title, - typeof body === "string" ? stripPrTemplateBoilerplate(body) : "", - ) && + (guiPathsChanged(changedFilePaths) || filesTruncated) && !hasScreenshotEvidence(body) && !hasGuiOverride({ comments: guiOverrideComments }) ) { @@ -500,6 +535,8 @@ module.exports = { isWrongAncestry, authorHasPushPermission, assessPrDescription, + guiPathsChanged, + isChangedFileListTruncated, hasGuiCue, hasGuiOverride, hasScreenshotEvidence, diff --git a/.github/scripts/pr-quality.test.cjs b/.github/scripts/pr-quality.test.cjs index e013fab88a..7d61925758 100644 --- a/.github/scripts/pr-quality.test.cjs +++ b/.github/scripts/pr-quality.test.cjs @@ -9,6 +9,8 @@ const { authorHasPushPermission, assessPrDescription, hasGuiCue, + guiPathsChanged, + isChangedFileListTruncated, hasGuiOverride, hasScreenshotEvidence, buildReviewReadinessSection, @@ -137,6 +139,16 @@ describe("hasGuiCue", () => { ); }); + it("does not match negated gui phrases", () => { + assert.equal(hasGuiCue("", "no gui changes in this PR"), false); + assert.equal(hasGuiCue("", "Without gui changes"), false); + assert.equal(hasGuiCue("No GUI changes", ""), false); + assert.equal( + hasGuiCue("", "This does not change the API. Please add a gui screenshot."), + true, + ); + }); + it("does not match gui inside other words", () => { assert.equal(hasGuiCue("Add contributor guidance", ""), false); assert.equal(hasGuiCue("", "Fix the guild invitation bug"), false); @@ -149,6 +161,28 @@ describe("hasGuiCue", () => { }); }); +describe("guiPathsChanged", () => { + it("matches gui/ paths with a slash guard", () => { + assert.equal(guiPathsChanged(["gui/src/App.tsx"]), true); + assert.equal(guiPathsChanged(["gui"]), true); + assert.equal(guiPathsChanged(["scripts/foo.ts", "gui/package.json"]), true); + assert.equal(guiPathsChanged(["scripts/foo.ts"]), false); + assert.equal(guiPathsChanged(["guitools/x.ts"]), false); + assert.equal(guiPathsChanged([]), false); + }); +}); + +describe("isChangedFileListTruncated", () => { + it("treats head drift, invalid counts, and oversized lists as truncated", () => { + assert.equal(isChangedFileListTruncated(10, 10, false), true); + assert.equal(isChangedFileListTruncated(undefined, 10, true), true); + assert.equal(isChangedFileListTruncated(10.5, 10, true), true); + assert.equal(isChangedFileListTruncated(11, 10, true), true); + assert.equal(isChangedFileListTruncated(10, 10, true), false); + assert.equal(isChangedFileListTruncated(5, 10, true), false); + }); +}); + describe("hasGuiOverride", () => { const owner = { author_association: "OWNER", body: "Not touching gui here." }; const collaborator = { author_association: "COLLABORATOR", body: "no gui changes needed" }; @@ -792,7 +826,21 @@ describe("collectPrQualityFailures", () => { assert.ok(failures.some((f) => f.code === "wrong_base")); }); - it("flags a gui title without a screenshot", () => { + it("flags gui/ file changes without a screenshot", () => { + const failures = collectPrQualityFailures({ + baseRef: "dev", + allowedBases: allowed, + title: "Fix dashboard spacing", + body: richBody, + behindMain: 0, + behindBase: 0, + authorPermission: "read", + changedFilePaths: ["gui/src/App.tsx"], + }); + assert.ok(failures.some((f) => f.code === "missing_ui_screenshot")); + }); + + it("does not flag a gui title when no gui/ files changed", () => { const failures = collectPrQualityFailures({ baseRef: "dev", allowedBases: allowed, @@ -801,11 +849,65 @@ describe("collectPrQualityFailures", () => { behindMain: 0, behindBase: 0, authorPermission: "read", + changedFilePaths: ["scripts/foo.ts"], + }); + assert.ok(!failures.some((f) => f.code === "missing_ui_screenshot")); + }); + + it("flags truncated file lists even when gui/ is not in the partial list", () => { + const truncatedPaths = Array.from({ length: 3000 }, (_, index) => `scripts/file-${index}.ts`); + const failures = collectPrQualityFailures({ + baseRef: "dev", + allowedBases: allowed, + title: "Large refactor", + body: richBody, + behindMain: 0, + behindBase: 0, + authorPermission: "read", + changedFilePaths: truncatedPaths, + filesTruncated: true, }); assert.ok(failures.some((f) => f.code === "missing_ui_screenshot")); }); - it("flags a gui mention in the body without a screenshot", () => { + it("flags truncated file lists when gui/ appears in the partial list", () => { + const truncatedPaths = Array.from({ length: 2999 }, (_, index) => `scripts/file-${index}.ts`); + truncatedPaths.push("gui/src/App.tsx"); + const failures = collectPrQualityFailures({ + baseRef: "dev", + allowedBases: allowed, + title: "Large refactor with gui tweak", + body: richBody, + behindMain: 0, + behindBase: 0, + authorPermission: "read", + changedFilePaths: truncatedPaths, + filesTruncated: true, + }); + assert.ok(failures.some((f) => f.code === "missing_ui_screenshot")); + }); + + it("does not flag no gui changes text without gui/ file changes", () => { + const failures = collectPrQualityFailures({ + baseRef: "dev", + allowedBases: allowed, + title: "Fix proxy routing", + body: [ + "## Summary", + "No gui changes in this PR; proxy routing only.", + "", + "## Test plan", + "- Ran bun test tests/ci-workflows.test.ts", + ].join("\n"), + behindMain: 0, + behindBase: 0, + authorPermission: "read", + changedFilePaths: ["scripts/foo.ts"], + }); + assert.ok(!failures.some((f) => f.code === "missing_ui_screenshot")); + }); + + it("flags a gui mention in the body without a screenshot when gui/ changed", () => { const failures = collectPrQualityFailures({ baseRef: "dev", allowedBases: allowed, @@ -820,6 +922,7 @@ describe("collectPrQualityFailures", () => { behindMain: 0, behindBase: 0, authorPermission: "read", + changedFilePaths: ["gui/src/styles.css"], }); assert.ok(failures.some((f) => f.code === "missing_ui_screenshot")); }); @@ -839,6 +942,7 @@ describe("collectPrQualityFailures", () => { behindMain: 0, behindBase: 0, authorPermission: "read", + changedFilePaths: ["gui/src/App.tsx"], guiOverrideComments: [ { author_association: "OWNER", body: "no gui changes here" }, ], @@ -861,6 +965,7 @@ describe("collectPrQualityFailures", () => { behindMain: 0, behindBase: 0, authorPermission: "read", + changedFilePaths: ["gui/src/App.tsx"], guiOverrideComments: [ { author_association: "CONTRIBUTOR", body: "no gui changes here" }, ], @@ -885,6 +990,7 @@ describe("collectPrQualityFailures", () => { behindMain: 0, behindBase: 0, authorPermission: "read", + changedFilePaths: ["gui/src/App.tsx"], }); assert.ok(!failures.some((f) => f.code === "missing_ui_screenshot")); }); @@ -908,11 +1014,12 @@ describe("collectPrQualityFailures", () => { behindMain: 0, behindBase: 0, authorPermission: "read", + changedFilePaths: ["gui/src/App.tsx"], }); assert.ok(!failures.some((f) => f.code === "missing_ui_screenshot")); }); - it("still flags a gui title when image syntax is only inside a code fence", () => { + it("still flags gui/ changes when image syntax is only inside a code fence", () => { const failures = collectPrQualityFailures({ baseRef: "dev", allowedBases: allowed, @@ -931,6 +1038,7 @@ describe("collectPrQualityFailures", () => { behindMain: 0, behindBase: 0, authorPermission: "read", + changedFilePaths: ["gui/src/App.tsx"], }); assert.ok(failures.some((f) => f.code === "missing_ui_screenshot")); }); diff --git a/.github/workflows/enforce-pr-target.yml b/.github/workflows/enforce-pr-target.yml index a6e93d8993..d7eb5d3453 100644 --- a/.github/workflows/enforce-pr-target.yml +++ b/.github/workflows/enforce-pr-target.yml @@ -139,6 +139,7 @@ jobs: collectPrQualityFailures, authorHasPushPermission, hasGuiOverride, + isChangedFileListTruncated, extractReviewReadiness, appendReviewReadinessSection, stripReviewReadinessSection, @@ -534,6 +535,51 @@ jobs: } } + const changedFiles = []; + const changedFilePaths = []; + let filesTruncated = true; + for (let attempt = 0; attempt < 2; attempt += 1) { + const { data: fileSnapshot } = await github.rest.pulls.get({ + owner, + repo, + pull_number + }); + const headShaForFiles = fileSnapshot.head?.sha ?? ""; + const listedFiles = await github.paginate( + github.rest.pulls.listFiles, + { owner, repo, pull_number, per_page: 100 } + ); + const { data: fileVerify } = await github.rest.pulls.get({ + owner, + repo, + pull_number + }); + const headMatches = fileVerify.head?.sha === headShaForFiles; + if (!headMatches && attempt === 0) { + core.info( + "PR head moved while listing changed files; retrying once." + ); + continue; + } + if (!headMatches) { + core.warning( + "PR head moved during changed-file snapshot; treating file list as truncated." + ); + } + changedFiles.length = 0; + changedFiles.push(...listedFiles); + changedFilePaths.length = 0; + changedFilePaths.push( + ...listedFiles.map(file => file.filename).filter(Boolean) + ); + filesTruncated = isChangedFileListTruncated( + fileSnapshot.changed_files, + listedFiles.length, + headMatches + ); + break; + } + let failures = collectPrQualityFailures({ baseRef: pr.base.ref, allowedBases: ALLOWED_BASES, @@ -548,17 +594,15 @@ jobs: stackedBase, // A maintainer issue comment ("not touching gui") waives the // GUI-screenshot gate; the comments are already fetched above. - guiOverrideComments: comments + guiOverrideComments: comments, + changedFilePaths, + filesTruncated }); // Hygiene is a separate workflow that owns the blocked label and // the Hygiene comment section, but Ready / review-ready must not // clear while those checks fail. Re-assess here from the same // trusted scripts so the gate cannot race ahead of hygiene. - const changedFiles = await github.paginate( - github.rest.pulls.listFiles, - { owner, repo, pull_number, per_page: 100 } - ); const labelNames = (pr.labels ?? []).map(label => label.name); failures = [ ...failures, diff --git a/docs-site/src/content/docs/contributing/pr-quality.md b/docs-site/src/content/docs/contributing/pr-quality.md index e29619e5a2..b8b5c39ce5 100644 --- a/docs-site/src/content/docs/contributing/pr-quality.md +++ b/docs-site/src/content/docs/contributing/pr-quality.md @@ -37,10 +37,13 @@ tells you exactly what to change: - **PR quality (`enforce-target`).** Pull requests must target `dev` and carry a real description: a **Summary** of what changed and why, plus a **Test - plan** (or equivalent substance). When the title or description mentions - `gui`, the description must include a screenshot of the UI change; the check - keeps the PR a draft and comments until the screenshot is present. A - maintainer can waive a false-positive GUI cue by adding the + plan** (or equivalent substance). When the diff changes files under `gui/`, or + when GitHub returns an incomplete changed-file list for a large diff, the + description must include a screenshot of the UI change; the check keeps + the PR a draft and comments until the screenshot is present. Incomplete file + lists are treated conservatively as a GUI change. A maintainer can waive the + screenshot requirement for a `gui/` change, for a false-positive GUI-path + classification, or for an incomplete-file-list false positive, by adding the `gui-screenshot-waived` label; adding or removing that label immediately re-evaluates the gate. Legacy maintainer comments such as "no gui changes" are still recognised on the next PR event for compatibility, but comments diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index e83017ed27..5652814903 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -788,7 +788,9 @@ describe("GitHub Actions hardening", () => { "repos.getCollaboratorPermissionLevel", "repos.compareCommitsWithBasehead", "repos.compareCommitsWithBasehead", + "pulls.get", "pulls.listFiles", + "pulls.get", ...tail, ]; } @@ -801,7 +803,9 @@ describe("GitHub Actions hardening", () => { "issues.listComments", "repos.getCollaboratorPermissionLevel", "pulls.list", + "pulls.get", "pulls.listFiles", + "pulls.get", ...tail, ]; } @@ -818,8 +822,10 @@ describe("GitHub Actions hardening", () => { "repos.compareCommitsWithBasehead", // The harness walks every paginate call across the same page count, so // listFiles appears once per comment page even when the file list is empty. + "pulls.get", "pulls.listFiles", "pulls.listFiles", + "pulls.get", ...tail, ]; } @@ -1090,8 +1096,11 @@ describe("GitHub Actions hardening", () => { expect(script).toContain("collectPrQualityFailures"); expect(script).toContain("collectDeterministicHygieneFailures"); expect(script).toContain("github.rest.pulls.listFiles"); - // The GUI screenshot gate reads the title as well as the body. - expect(script).toContain("title: pr.title"); + // The GUI screenshot gate reads changed file paths under gui/. + expect(script).toContain("changedFilePaths"); + expect(script).toContain("filesTruncated"); + expect(script).toContain("isChangedFileListTruncated"); + expect(script).toContain("PR head moved while listing changed files"); expect(script).toContain("github.rest.repos.getCollaboratorPermissionLevel"); expect(script).toContain("github.rest.repos.compareCommitsWithBasehead"); // The allow-list is the gate's whole policy, so it is pinned by value and @@ -1301,6 +1310,11 @@ describe("GitHub Actions hardening", () => { "- [@Wibias](https://github.com/Wibias) was added as a maintainer.", ].join("\n"); + const GUI_CHANGED_FILES = [ + { filename: "gui/src/App.tsx" }, + { filename: "tests/smoke.test.ts" }, + ]; + /** A PR body whose readiness checklist has exactly `checked` boxes ticked. */ function readinessChecklistBody(checked: number, base = CONTRIBUTOR_BODY): string { const boxes = CHECKLIST_ITEMS.map((item, index) => @@ -2458,6 +2472,7 @@ describe("GitHub Actions hardening", () => { title: "GUI: fix provider list spacing", body: readinessChecklistBody(4), }, + files: GUI_CHANGED_FILES, }); expect(methodsOf(result)).toEqual(readsAllowedBase([ @@ -2617,6 +2632,7 @@ describe("GitHub Actions hardening", () => { }, authorPermission: "read", maintainersFile: MAINTAINERS_FIXTURE, + files: GUI_CHANGED_FILES, }); expect( result.warnings.some( @@ -2842,9 +2858,10 @@ describe("GitHub Actions hardening", () => { expect(injected.body).toContain(CHECKLIST_START); }); - test("gui in the title without a screenshot fails and drafts", async () => { + test("gui/ changes without a screenshot fail and draft", async () => { const result = await run({ - pr: { base: { ref: "dev" }, title: "GUI: fix provider list spacing" }, + pr: { base: { ref: "dev" }, title: "Fix dashboard spacing" }, + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:"))).toBe(true); @@ -2855,7 +2872,56 @@ describe("GitHub Actions hardening", () => { expect(callsTo(result, "graphql")).toHaveLength(1); }); - test("gui in the body without a screenshot fails", async () => { + test("gui in the title alone does not demand a screenshot", async () => { + const result = await run({ + pr: { base: { ref: "dev" }, title: "GUI: fix provider list spacing" }, + authorPermission: "write", + files: [{ filename: "scripts/foo.ts" }], + }); + + expect(methodsOf(result)).toEqual(readsAllowedBase()); + expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(false); + }); + + test("no gui changes text without gui/ file changes does not demand a screenshot", async () => { + const result = await run({ + pr: { + base: { ref: "dev" }, + title: "Fix proxy routing", + body: [ + "## Summary", + "", + "This change adjusts proxy routing only; there are no gui changes in this PR.", + "The handler in scripts/foo.ts keeps the same public surface while fixing retry semantics.", + "", + "## Test plan", + "", + "- Ran \`bun test tests/ci-workflows.test.ts\`", + ].join("\n"), + }, + authorPermission: "write", + files: [{ filename: "scripts/foo.ts" }], + }); + + expect(methodsOf(result)).toEqual(readsAllowedBase()); + expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(false); + }); + + test("malformed changed_files metadata fails closed on the screenshot gate", async () => { + const result = await run({ + pr: { + base: { ref: "dev" }, + changed_files: 2.5, + }, + files: [{ filename: "scripts/foo.ts" }], + authorPermission: "write", + }); + + expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(true); + expect(lastEnforcerCommentBody(result)).toContain("UI screenshot required"); + }); + + test("gui in the body without a screenshot fails when gui/ changed", async () => { const result = await run({ pr: { base: { ref: "dev" }, @@ -2868,6 +2934,7 @@ describe("GitHub Actions hardening", () => { "- Ran bun test tests/ci-workflows.test.ts", ].join("\n"), }, + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:"))).toBe(true); @@ -2890,6 +2957,7 @@ describe("GitHub Actions hardening", () => { comments: [ { id: 1, user: { login: "lidge-jun" }, author_association: "OWNER", body: "Not touching gui here." }, ], + files: GUI_CHANGED_FILES, }); // The screenshot failure is gone: no setFailed for it, and the comment @@ -2915,6 +2983,7 @@ describe("GitHub Actions hardening", () => { comments: [ { id: 1, user: { login: "wibias" }, author_association: "COLLABORATOR", body: "no gui changes needed" }, ], + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(false); @@ -2947,6 +3016,7 @@ describe("GitHub Actions hardening", () => { actor: { login: "lidge-jun" }, label: { name: "gui-screenshot-waived" }, }], + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(false); @@ -2975,6 +3045,7 @@ describe("GitHub Actions hardening", () => { actor: { login: "lidge-jun" }, label: { name: "gui-screenshot-waived" }, }], + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(false); @@ -3008,6 +3079,7 @@ describe("GitHub Actions hardening", () => { actor: { login: "unauthorized-contributor" }, label: { name: "gui-screenshot-waived" }, }], + files: GUI_CHANGED_FILES, }); // The screenshot failure must remain because the label was applied by @@ -3041,6 +3113,8 @@ describe("GitHub Actions hardening", () => { expect(callsTo(result, "pulls.get")).toEqual([ { owner: "lidge-jun", repo: "opencodex", pull_number: 4242 }, + { owner: "lidge-jun", repo: "opencodex", pull_number: 4242 }, + { owner: "lidge-jun", repo: "opencodex", pull_number: 4242 }, ]); expect(callsTo(result, "repos.listPullRequestsAssociatedWithCommit")).toEqual([]); expect(methodsOf(result)).toContain("issues.listComments"); @@ -3199,6 +3273,7 @@ describe("GitHub Actions hardening", () => { comments: [ { id: 1, user: { login: "contributor" }, author_association: "CONTRIBUTOR", body: "Not touching gui here." }, ], + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(true); @@ -3222,6 +3297,7 @@ describe("GitHub Actions hardening", () => { comments: [ { id: 1, user: { login: "lidge-jun" }, author_association: "OWNER", body: "This is gui related, please add a screenshot." }, ], + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(true); @@ -3244,6 +3320,7 @@ describe("GitHub Actions hardening", () => { ].join("\n"), }, authorPermission: "write", + files: GUI_CHANGED_FILES, }); expect(methodsOf(result)).toEqual(readsAllowedBase()); @@ -3268,6 +3345,7 @@ describe("GitHub Actions hardening", () => { ].join("\n"), }, authorPermission: "write", + files: GUI_CHANGED_FILES, }); expect(methodsOf(result)).toEqual(readsAllowedBase()); @@ -3291,6 +3369,7 @@ describe("GitHub Actions hardening", () => { "- Ran bun test tests/ci-workflows.test.ts", ].join("\n"), }, + files: GUI_CHANGED_FILES, }); expect(result.warnings.some((w) => w.startsWith("setFailed:"))).toBe(true); diff --git a/tests/helpers/enforce-pr-target-harness.ts b/tests/helpers/enforce-pr-target-harness.ts index 79a788f1fd..ceac6073b6 100644 --- a/tests/helpers/enforce-pr-target-harness.ts +++ b/tests/helpers/enforce-pr-target-harness.ts @@ -53,6 +53,8 @@ export type PullRequestState = { draft?: boolean; base?: { ref: string }; user?: { login: string }; + /** `pulls.get` changed_files; omit to default to listed file count in harness. */ + changed_files?: number; }; export type Comment = { @@ -653,6 +655,13 @@ export async function runEnforcePrTarget( const filePages: unknown[][] = options.filePages ?? (options.files && options.files.length > 0 ? [options.files] : [[]]); + const listedFileCount = filePages.flat().length; + const prInput = options.pr as Record; + if (Object.prototype.hasOwnProperty.call(prInput, "changed_files")) { + (pr as Record).changed_files = prInput.changed_files; + } else { + (pr as { changed_files: number }).changed_files = listedFileCount; + } const checkRunPages = (options.checkRunPages ?? [options.checkRuns ?? DEFAULT_GREEN_CHECKS]) .map(page => page.map(check => ({ ...check,