From f12437deb22c62adfe09c8a3d3962c15bdd4a73d Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Tue, 6 Oct 2026 06:15:10 -0400 Subject: [PATCH] ci(auto-format): run the format step through the steps script The format job's one multi-line run step moves into auto-format-steps.ts as the format subcommand, beside push: the same commands, GITHUB_OUTPUT lines, log line, and patch file. On a runner, one difference: a failed git rev-parse HEAD ends the step with git's status, where the shell wrote an empty head= and went on. Off a runner, two more: an unset RUNNER_TEMP ends the step before any patch is written, and a patch file removed under the step ends it rather than reading as changed=false. --- .github/scripts/auto-format-steps.ts | 35 +++++++++++--- .github/workflows/auto-format.yml | 17 +------ test/scripts/auto-format-steps.test.ts | 67 +++++++++++++++++++++++++- 3 files changed, 95 insertions(+), 24 deletions(-) diff --git a/.github/scripts/auto-format-steps.ts b/.github/scripts/auto-format-steps.ts index 9000a3f5..5a260654 100644 --- a/.github/scripts/auto-format-steps.ts +++ b/.github/scripts/auto-format-steps.ts @@ -1,16 +1,37 @@ /** - * The auto-format.yml push step: the formatting patch the format job cut on HEAD_SHA, committed and pushed to the - * PR branch under a lease on that commit. + * The two auto-format.yml steps: the format job's run of the formatter, cut into a patch for the push job, and + * that job's commit of the patch to the PR branch under a lease on the commit it was cut on. * - * push push job GH_TOKEN, HEAD_REF, HEAD_SHA, RUNNER_TEMP, GITHUB_REPOSITORY -> the commit on the PR branch + * format format job RUNNER_TEMP, GITHUB_OUTPUT -> format.patch, head=, changed= + * push push job GH_TOKEN, HEAD_REF, HEAD_SHA, RUNNER_TEMP, GITHUB_REPOSITORY -> the commit on the PR branch * * Node builtins only: the push job installs nothing, so no git hook or lifecycle script exists where the write * token is. */ +import { statSync } from "node:fs"; import { join } from "node:path"; import { configureBotIdentity, leasePush } from "./lib/pr-branch.js"; -import { capture, requireEnv, run } from "./lib/workflow-step.js"; +import { capture, requireEnv, run, setOutput } from "./lib/workflow-step.js"; + +function format(): void { + // The patch belongs to this commit alone; the push job refuses any other head. + setOutput("head", capture(["git", "rev-parse", "HEAD"])); + run(["bun", "run", "lint:fix"]); + // Anything the formatter left staged is not this workflow's fix: start from an empty index, then stage + // modifications and deletions of tracked files alone (a formatter adds nothing). + run(["git", "reset", "-q"]); + run(["git", "add", "-u"]); + const patch = join(requireEnv("RUNNER_TEMP"), "format.patch"); + run(["git", "diff", "--cached", "--binary"], { stdoutFile: patch }); + if (statSync(patch).size === 0) { + console.log("nothing to format"); + setOutput("changed", "false"); + return; + } + run(["git", "diff", "--cached", "--stat"]); + setOutput("changed", "true"); +} function push(): void { // bun has already read its bunfig.toml and .env from the trusted checkout it started in; only git runs in the @@ -42,11 +63,13 @@ function push(): void { if (import.meta.main) { const command = process.argv[2]; - if (command === "push") { + if (command === "format") { + format(); + } else if (command === "push") { push(); } else { console.error( - `auto-format-steps: unknown command ${JSON.stringify(command ?? null)}; expected push`, + `auto-format-steps: unknown command ${JSON.stringify(command ?? null)}; expected format | push`, ); process.exit(1); } diff --git a/.github/workflows/auto-format.yml b/.github/workflows/auto-format.yml index 00976148..d6f9734a 100644 --- a/.github/workflows/auto-format.yml +++ b/.github/workflows/auto-format.yml @@ -40,22 +40,7 @@ jobs: - uses: ./.github/actions/setup - name: Format (biome) and stage the patch id: format - run: | - # The patch belongs to this commit alone; the push job refuses any other head. - echo "head=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" - bun run lint:fix - # Anything the formatter left staged is not this workflow's fix: start from an empty index, then stage - # modifications and deletions of tracked files alone (a formatter adds nothing). - git reset -q - git add -u - git diff --cached --binary > "$RUNNER_TEMP/format.patch" - if [ -s "$RUNNER_TEMP/format.patch" ]; then - git diff --cached --stat - echo "changed=true" >> "$GITHUB_OUTPUT" - else - echo "nothing to format" - echo "changed=false" >> "$GITHUB_OUTPUT" - fi + run: bun .github/scripts/auto-format-steps.ts format - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: steps.format.outputs.changed == 'true' with: diff --git a/test/scripts/auto-format-steps.test.ts b/test/scripts/auto-format-steps.test.ts index 800ce0d4..d91908de 100644 --- a/test/scripts/auto-format-steps.test.ts +++ b/test/scripts/auto-format-steps.test.ts @@ -1,10 +1,12 @@ /** - * auto-format.yml's push step against local fixture repositories: the branches the workflow text cannot show, - * which checkout the patch is committed on, under whose identity, and what a moved branch does to the push. + * auto-format.yml's two steps against local fixture repositories: the branches the workflow text cannot show. + * For the format step, what the patch holds and when the step stops; for the push step, which checkout the patch + * is committed on, under whose identity, and what a moved branch does to the push. */ import { describe, expect, setDefaultTimeout, test } from "bun:test"; import { execFileSync } from "node:child_process"; +import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { clone, @@ -22,6 +24,67 @@ installReleasePipelineFixture(); const REMOTE_URL = "https://x-access-token:t0ken@github.com/o/r.git"; +/** The format job's checkout of a same-repo PR branch: its lint:fix runs whatever LINT_FIX says. */ +function formatFixture(): { pr: string; headSha: string; temp: string } { + const fx = seedFixture(); + const pr = clone(fx.root, fx.origin, "pr"); + write( + pr, + "package.json", + `${JSON.stringify( + { name: "fixture", version: "0.0.0", scripts: { "lint:fix": 'sh -c "$LINT_FIX"' } }, + null, + 2, + )}\n`, + ); + const headSha = commitAll(pr, "feat: a pull request"); + return { pr, headSha, temp: join(fx.root, "runner-temp") }; +} + +const format = (pr: string, temp: string, lintFix: string) => + runStep("auto-format-steps", pr, temp, { LINT_FIX: lintFix }, "format"); + +function patchPaths(temp: string): string[] { + return [...readFileSync(join(temp, "format.patch"), "utf8").matchAll(/^diff --git a\/(\S+)/gm)] + .map((match) => match[1] ?? "") + .sort(); +} + +describe("format", () => { + test("a formatter that changes nothing leaves an empty patch and changed=false, and an addition staged before it is not the fix", () => { + const { pr, headSha, temp } = formatFixture(); + write(pr, "staged.ts", "export const staged = 1;\n"); + git(pr, "add", "staged.ts"); + const result = format(pr, temp, "true"); + expect(result).toMatchObject({ + status: 0, + stdout: "nothing to format\n", + outputs: [`head=${headSha}`, "changed=false"], + }); + expect(readFileSync(join(temp, "format.patch"), "utf8")).toBe(""); + expect(git(pr, "diff", "--cached", "--name-only")).toBe(""); + }); + + test("the formatter's modifications and deletions of tracked files enter the patch; a file it adds does not", () => { + const { pr, headSha, temp } = formatFixture(); + const result = format( + pr, + temp, + "echo 'export const marker=1' > src/marker.ts && rm .github/dependabot.yml && echo new > added.ts", + ); + expect(result).toMatchObject({ status: 0, outputs: [`head=${headSha}`, "changed=true"] }); + expect(result.stdout).toContain("2 files changed, 1 insertion(+), 3 deletions(-)"); + expect(patchPaths(temp)).toEqual([".github/dependabot.yml", "src/marker.ts"]); + }); + + test("a failing formatter ends the step with its status after head= is written, with no patch cut", () => { + const { pr, headSha, temp } = formatFixture(); + const result = format(pr, temp, "exit 3"); + expect(result).toMatchObject({ status: 3, outputs: [`head=${headSha}`] }); + expect(existsSync(join(temp, "format.patch"))).toBe(false); + }); +}); + /** The push job's checkout of a same-repo PR branch, with the format job's patch downloaded beside it. */ function prFixture(): { fx: Fixture; pr: string; headSha: string; temp: string } { const fx = seedFixture();