Fix asset-only PR handling by fetching parent file content (#428)

When a PR only modifies asset files (e.g. assets/eip-1234/image.png),
fetch the parent markdown from the default branch so the authors rule
can extract authors and allow self-approval.

- Fix split index bug (split("/")[2] → split("/")[1])
- Use raw mediaType to simplify content fetching
- Handle 404 gracefully (fall through), re-throw unexpected errors
- Add tests for EIP/ERC asset changes, parent-in-PR skip, and non-assets

Author: dionysuzx

jest.config.js

@@ -4,4 +4,7 @@ export default {
     transform: {
         "^.+\\.(ts|tsx)$": "ts-jest",
     },
+    moduleNameMapper: {
+        "^(\\.{1,2}/.*)\\.js$": "$1",
+    },
 };

src/rules/__tests__/assets.test.ts

@@ -0,0 +1,145 @@
+import { Octokit } from "../../types";
+import checkAssets from "../assets";
+
+jest.mock("@actions/github", () => ({
+    __esModule: true,
+    default: {
+        context: {
+            repo: {
+                owner: "ethereum",
+                repo: "EIPs",
+            },
+        },
+    },
+}));
+
+jest.mock("../../process", () => ({
+    __esModule: true,
+    default: jest.fn(
+        (
+            _o: unknown,
+            _c: unknown,
+            files: { filename: string; contents?: string }[],
+        ) =>
+            files.flatMap((f) => {
+                const authors = f.contents
+                    ?.match(/author:\s*(.+)/)?.[1]
+                    ?.match(/@(\w+)/g)
+                    ?.map((s) => s.slice(1));
+                if (!authors?.length) return [];
+                return [
+                    {
+                        name: "authors",
+                        reviewers: authors,
+                        min: 1,
+                        pr_approval: true,
+                        annotation: { file: f.filename },
+                    },
+                ];
+            }),
+    ),
+}));
+
+const frontmatter =
+    "---\nstatus: Draft\ncategory: ERC\nauthor: Alice (@alice), Bob (@bob)\n---\nHello!";
+function makeFakeOctokit() {
+    return {
+        rest: {
+            repos: {
+                getContent: jest.fn().mockResolvedValue({
+                    data: frontmatter,
+                }),
+            },
+        },
+    } as unknown as Octokit;
+}
+
+describe("checkAssets", () => {
+    test("Should require author approval for asset-only EIP changes", async () => {
+        const fakeOctokit = makeFakeOctokit();
+        const result = await checkAssets(
+            fakeOctokit,
+            { erc: ["editor1", "editor2", "editor3"] },
+            [
+                {
+                    filename: "assets/eip-1234/image.png",
+                    status: "modified",
+                },
+            ],
+        );
+        expect(result).toMatchObject([
+            {
+                name: "authors",
+                reviewers: ["alice", "bob"],
+                min: 1,
+                pr_approval: true,
+                annotation: {
+                    file: "EIPS/eip-1234.md",
+                },
+            },
+        ]);
+    });
+
+    test("Should require author approval for asset-only ERC changes", async () => {
+        const fakeOctokit = makeFakeOctokit();
+        const result = await checkAssets(
+            fakeOctokit,
+            { erc: ["editor1", "editor2", "editor3"] },
+            [
+                {
+                    filename: "assets/erc-5678/diagram.svg",
+                    status: "modified",
+                },
+            ],
+        );
+        expect(result).toMatchObject([
+            {
+                name: "authors",
+                reviewers: ["alice", "bob"],
+                min: 1,
+                pr_approval: true,
+                annotation: {
+                    file: "ERCS/erc-5678.md",
+                },
+            },
+        ]);
+    });
+
+    test("Should skip when parent EIP file is also in the PR", async () => {
+        const fakeOctokit = makeFakeOctokit();
+        const result = await checkAssets(
+            fakeOctokit,
+            { erc: ["editor1", "editor2", "editor3"] },
+            [
+                {
+                    filename: "assets/eip-1234/image.png",
+                    status: "modified",
+                },
+                {
+                    filename: "EIPS/eip-1234.md",
+                    status: "modified",
+                    contents: frontmatter,
+                    previous_contents: frontmatter,
+                },
+            ],
+        );
+        expect(result).toEqual([]);
+    });
+
+    test("Should return empty for non-asset files", async () => {
+        const fakeOctokit = makeFakeOctokit();
+        const result = await checkAssets(
+            fakeOctokit,
+            { erc: ["editor1", "editor2", "editor3"] },
+            [
+                {
+                    filename: "EIPS/eip-1234.md",
+                    status: "modified",
+                    contents: frontmatter,
+                    previous_contents: frontmatter,
+                },
+            ],
+        );
+        expect(result).toEqual([]);
+    });
+});

src/rules/assets.ts

@@ -1,5 +1,6 @@
 import processFiles from "../process.js";
 import { Config, File, Octokit, Rule } from "../types.js";
+import github from "@actions/github";
 
 export default async function (
     octokit: Octokit,
@@ -14,15 +15,15 @@ export default async function (
                 file.filename.startsWith("assets/eip-") &&
                 !files.some(
                     (f) =>
-                        f.filename == `EIPS/${file.filename.split("/")[2]}.md`,
+                        f.filename == `EIPS/${file.filename.split("/")[1]}.md`,
                 )
             ) {
                 filename = `EIPS/${file.filename.split("/")[1]}.md`;
             } else if (
                 file.filename.startsWith("assets/erc-") &&
                 !files.some(
                     (f) =>
-                        f.filename == `ERCS/${file.filename.split("/")[2]}.md`,
+                        f.filename == `ERCS/${file.filename.split("/")[1]}.md`,
                 )
             ) {
                 filename = `ERCS/${file.filename.split("/")[1]}.md`;
@@ -33,10 +34,26 @@ export default async function (
             if (files.some((file) => file.filename == filename)) {
                 return []; // Already covered by the relevant rules, so avoid potential conflicts by short circuiting
             }
+            let contents: string | undefined;
+            try {
+                contents = (
+                    await octokit.rest.repos.getContent({
+                        ...github.context.repo,
+                        path: filename,
+                        mediaType: { format: "raw" },
+                    })
+                ).data as unknown as string;
+            } catch (e) {
+                if (!(e instanceof Object && "status" in e && e.status === 404))
+                    throw e;
+            }
+
             return processFiles(octokit, config, [
                 {
                     filename,
                     status: "modified",
+                    contents,
+                    previous_contents: contents,
                 },
             ]);
         }),