From 307300ac975d4358a4e2dde1557e5af4a54b5e05 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 23 Jun 2026 16:56:11 +0800 Subject: [PATCH] fix(ci): report missing workflow pre-commit runtime --- scripts/check-workflows.mjs | 42 ++-- test/scripts/check-workflows.test.ts | 298 +++++++++++++++++---------- 2 files changed, 212 insertions(+), 128 deletions(-) diff --git a/scripts/check-workflows.mjs b/scripts/check-workflows.mjs index ce991d971563..35f9e6f82cf8 100644 --- a/scripts/check-workflows.mjs +++ b/scripts/check-workflows.mjs @@ -44,38 +44,44 @@ function runChecked(command, args) { return null; } +function exitWithFailure(failure) { + if (failure.message) { + console.error(failure.message); + } + process.exit(failure.status); +} + function runPreCommitFromTempVenv(hook, hookArgs) { if (!commandExists("python3", ["--version"])) { return false; } const venvDir = mkdtempSync(join(tmpdir(), "openclaw-check-workflows-pre-commit-")); const python = join(venvDir, process.platform === "win32" ? "Scripts/python.exe" : "bin/python"); - let failure; + let postVenvFailure; try { - failure = runChecked("python3", ["-m", "venv", venvDir]); - if (!failure) { - failure = runChecked(python, [ - "-m", - "pip", - "install", - "--disable-pip-version-check", - `pre-commit==${PRE_COMMIT_VERSION}`, - ]); + const venvFailure = runChecked("python3", ["-m", "venv", venvDir]); + if (venvFailure) { + return false; } - if (!failure) { - failure = runChecked(python, ["-m", "pre_commit", ...hookArgs]); + postVenvFailure = runChecked(python, [ + "-m", + "pip", + "install", + "--disable-pip-version-check", + `pre-commit==${PRE_COMMIT_VERSION}`, + ]); + if (postVenvFailure) { + return false; } - if (failure) { + postVenvFailure = runChecked(python, ["-m", "pre_commit", ...hookArgs]); + if (postVenvFailure) { return false; } return true; } finally { rmSync(venvDir, { force: true, recursive: true }); - if (failure) { - if (failure.message) { - console.error(failure.message); - } - process.exit(failure.status); + if (postVenvFailure) { + exitWithFailure(postVenvFailure); } } } diff --git a/test/scripts/check-workflows.test.ts b/test/scripts/check-workflows.test.ts index 0b5412fbb6cd..966af788bce4 100644 --- a/test/scripts/check-workflows.test.ts +++ b/test/scripts/check-workflows.test.ts @@ -1,11 +1,16 @@ // Check Workflows tests cover check workflows script behavior. import { spawnSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import os from "node:os"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; +import { cleanupTempDirs, makeTempDir } from "../helpers/temp-dir.js"; const scriptPath = path.resolve("scripts/check-workflows.mjs"); +const tempDirs: string[] = []; + +afterEach(() => { + cleanupTempDirs(tempDirs); +}); describe("check-workflows", () => { it("prints an actionable diagnostic when actionlint and go are unavailable", () => { @@ -23,117 +28,186 @@ describe("check-workflows", () => { }); it("uses the pinned go fallback and audits all workflows with zizmor", () => { - const tempDir = mkdtempSync(path.join(os.tmpdir(), "check-workflows-")); - try { - const binDir = path.join(tempDir, "bin"); - const markerPath = path.join(tempDir, "go-run.txt"); - const preCommitMarkerPath = path.join(tempDir, "pre-commit.txt"); - mkdirSync(binDir); - writeFileSync( - path.join(binDir, "go"), - [ - "#!/bin/sh", - 'if [ "$1" = "version" ]; then exit 0; fi', - 'if [ "$1" = "run" ]; then printf "%s\\n" "$*" > "$GO_FALLBACK_MARKER"; exit 0; fi', - "exit 1", - "", - ].join("\n"), - { mode: 0o755 }, - ); - writeFileSync( - path.join(binDir, "pre-commit"), - [ - "#!/bin/sh", - 'if [ "$1" = "--version" ]; then exit 0; fi', - 'printf "%s\\n" "$*" >> "$PRE_COMMIT_MARKER"', - "exit 0", - "", - ].join("\n"), - { mode: 0o755 }, - ); - for (const command of ["python3", "node"]) { - writeFileSync(path.join(binDir, command), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); - } - - const result = spawnSync(process.execPath, [scriptPath], { - encoding: "utf8", - env: { - ...process.env, - GO_FALLBACK_MARKER: markerPath, - PRE_COMMIT_MARKER: preCommitMarkerPath, - PATH: binDir, - }, - }); - - expect(result.status).toBe(0); - expect(readFileSync(markerPath, "utf8")).toContain( - "github.com/rhysd/actionlint/cmd/actionlint@v1.7.11", - ); - const preCommitArgs = readFileSync(preCommitMarkerPath, "utf8"); - expect(preCommitArgs).toContain("run --config .pre-commit-config.yaml zizmor --files"); - expect(preCommitArgs).toContain(".github/workflows/ci.yml"); - expect(preCommitArgs).toContain(".github/workflows/windows-testbox-probe.yml"); - } finally { - rmSync(tempDir, { force: true, recursive: true }); + const tempDir = makeTempDir(tempDirs, "check-workflows-"); + const binDir = path.join(tempDir, "bin"); + const markerPath = path.join(tempDir, "go-run.txt"); + const preCommitMarkerPath = path.join(tempDir, "pre-commit.txt"); + mkdirSync(binDir); + writeFileSync( + path.join(binDir, "go"), + [ + "#!/bin/sh", + 'if [ "$1" = "version" ]; then exit 0; fi', + 'if [ "$1" = "run" ]; then printf "%s\\n" "$*" > "$GO_FALLBACK_MARKER"; exit 0; fi', + "exit 1", + "", + ].join("\n"), + { mode: 0o755 }, + ); + writeFileSync( + path.join(binDir, "pre-commit"), + [ + "#!/bin/sh", + 'if [ "$1" = "--version" ]; then exit 0; fi', + 'printf "%s\\n" "$*" >> "$PRE_COMMIT_MARKER"', + "exit 0", + "", + ].join("\n"), + { mode: 0o755 }, + ); + for (const command of ["python3", "node"]) { + writeFileSync(path.join(binDir, command), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); } + + const result = spawnSync(process.execPath, [scriptPath], { + encoding: "utf8", + env: { + ...process.env, + GO_FALLBACK_MARKER: markerPath, + PRE_COMMIT_MARKER: preCommitMarkerPath, + PATH: binDir, + }, + }); + + expect(result.status).toBe(0); + expect(readFileSync(markerPath, "utf8")).toContain( + "github.com/rhysd/actionlint/cmd/actionlint@v1.7.11", + ); + const preCommitArgs = readFileSync(preCommitMarkerPath, "utf8"); + expect(preCommitArgs).toContain("run --config .pre-commit-config.yaml zizmor --files"); + expect(preCommitArgs).toContain(".github/workflows/ci.yml"); + expect(preCommitArgs).toContain(".github/workflows/windows-testbox-probe.yml"); }); it("bootstraps pinned pre-commit in a temporary Python venv when needed", () => { - const tempDir = mkdtempSync(path.join(os.tmpdir(), "check-workflows-")); - try { - const binDir = path.join(tempDir, "bin"); - const markerPath = path.join(tempDir, "python.txt"); - mkdirSync(binDir); - writeFileSync(path.join(binDir, "node"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); - writeFileSync( - path.join(binDir, "python3"), - [ - "#!/bin/sh", - 'if [ "$1" = "--version" ]; then exit 0; fi', - 'if [ "$1" = "-m" ] && [ "$2" = "pre_commit" ] && [ "$3" = "--version" ]; then exit 1; fi', - 'if [ "$1" = "-m" ] && [ "$2" = "pip" ]; then', - ' printf "%s\\n" "$*" >> "$PRE_COMMIT_BOOTSTRAP_MARKER"', - " exit 0", - "fi", - 'if [ "$1" = "-m" ] && [ "$2" = "pre_commit" ]; then', - ' printf "%s\\n" "$*" >> "$PRE_COMMIT_BOOTSTRAP_MARKER"', - " exit 0", - "fi", - 'if [ "$1" = "-m" ] && [ "$2" = "venv" ]; then', - ' /bin/mkdir -p "$3/bin"', - ' /bin/cp "$0" "$3/bin/python"', - ' /bin/chmod +x "$3/bin/python"', - " exit 0", - "fi", - "exit 0", - "", - ].join("\n"), - { mode: 0o755 }, - ); + const tempDir = makeTempDir(tempDirs, "check-workflows-"); + const binDir = path.join(tempDir, "bin"); + const markerPath = path.join(tempDir, "python.txt"); + mkdirSync(binDir); + writeFileSync(path.join(binDir, "node"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + writeFileSync( + path.join(binDir, "python3"), + [ + "#!/bin/sh", + 'if [ "$1" = "--version" ]; then exit 0; fi', + 'if [ "$1" = "-m" ] && [ "$2" = "pre_commit" ] && [ "$3" = "--version" ]; then exit 1; fi', + 'if [ "$1" = "-m" ] && [ "$2" = "pip" ]; then', + ' printf "%s\\n" "$*" >> "$PRE_COMMIT_BOOTSTRAP_MARKER"', + " exit 0", + "fi", + 'if [ "$1" = "-m" ] && [ "$2" = "pre_commit" ]; then', + ' printf "%s\\n" "$*" >> "$PRE_COMMIT_BOOTSTRAP_MARKER"', + " exit 0", + "fi", + 'if [ "$1" = "-m" ] && [ "$2" = "venv" ]; then', + ' /bin/mkdir -p "$3/bin"', + ' /bin/cp "$0" "$3/bin/python"', + ' /bin/chmod +x "$3/bin/python"', + " exit 0", + "fi", + "exit 0", + "", + ].join("\n"), + { mode: 0o755 }, + ); - const result = spawnSync(process.execPath, [scriptPath], { - encoding: "utf8", - env: { - ...process.env, - PATH: binDir, - PRE_COMMIT_BOOTSTRAP_MARKER: markerPath, - }, - }); + const result = spawnSync(process.execPath, [scriptPath], { + encoding: "utf8", + env: { + ...process.env, + PATH: binDir, + PRE_COMMIT_BOOTSTRAP_MARKER: markerPath, + }, + }); - expect(result.status).toBe(0); - const pythonArgs = readFileSync(markerPath, "utf8"); - expect(pythonArgs).toContain( - "-m pip install --disable-pip-version-check pre-commit==4.2.0", - ); - expect(pythonArgs).toContain( - "-m pre_commit run --config .pre-commit-config.yaml actionlint --files", - ); - expect(pythonArgs).toContain( - "-m pre_commit run --config .pre-commit-config.yaml zizmor --files", - ); - } finally { - rmSync(tempDir, { force: true, recursive: true }); - } + expect(result.status).toBe(0); + const pythonArgs = readFileSync(markerPath, "utf8"); + expect(pythonArgs).toContain("-m pip install --disable-pip-version-check pre-commit==4.2.0"); + expect(pythonArgs).toContain( + "-m pre_commit run --config .pre-commit-config.yaml actionlint --files", + ); + expect(pythonArgs).toContain( + "-m pre_commit run --config .pre-commit-config.yaml zizmor --files", + ); + }); + + it("prints the missing runtime diagnostic when Python venv support is unavailable", () => { + const tempDir = makeTempDir(tempDirs, "check-workflows-"); + const binDir = path.join(tempDir, "bin"); + mkdirSync(binDir); + writeFileSync(path.join(binDir, "node"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + writeFileSync( + path.join(binDir, "python3"), + [ + "#!/bin/sh", + 'if [ "$1" = "--version" ]; then exit 0; fi', + 'if [ "$1" = "-m" ] && [ "$2" = "pre_commit" ] && [ "$3" = "--version" ]; then exit 1; fi', + 'if [ "$1" = "-m" ] && [ "$2" = "venv" ]; then', + ' printf "%s\\n" "python venv unavailable" >&2', + " exit 1", + "fi", + "exit 1", + "", + ].join("\n"), + { mode: 0o755 }, + ); + + const result = spawnSync(process.execPath, [scriptPath], { + encoding: "utf8", + env: { + ...process.env, + PATH: binDir, + }, + }); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("python venv unavailable"); + expect(result.stderr).toContain("missing pre-commit runtime for actionlint"); + expect(result.stderr).toContain("Python venv support for pre-commit 4.2.0"); + }); + + it("cleans the temporary Python venv before exiting on hook failure", () => { + const tempDir = makeTempDir(tempDirs, "check-workflows-"); + const binDir = path.join(tempDir, "bin"); + const markerPath = path.join(tempDir, "venv-path.txt"); + mkdirSync(binDir); + writeFileSync(path.join(binDir, "node"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + writeFileSync( + path.join(binDir, "python3"), + [ + "#!/bin/sh", + 'if [ "$1" = "--version" ]; then exit 0; fi', + 'if [ "$1" = "-m" ] && [ "$2" = "pre_commit" ] && [ "$3" = "--version" ]; then exit 1; fi', + 'if [ "$1" = "-m" ] && [ "$2" = "venv" ]; then', + ' /bin/mkdir -p "$3/bin"', + ' /bin/cp "$0" "$3/bin/python"', + ' /bin/chmod +x "$3/bin/python"', + ' printf "%s\\n" "$3" > "$PRE_COMMIT_VENV_MARKER"', + " exit 0", + "fi", + 'if [ "$1" = "-m" ] && [ "$2" = "pip" ]; then exit 0; fi', + 'if [ "$1" = "-m" ] && [ "$2" = "pre_commit" ]; then', + ' printf "%s\\n" "hook failed" >&2', + " exit 13", + "fi", + "exit 1", + "", + ].join("\n"), + { mode: 0o755 }, + ); + + const result = spawnSync(process.execPath, [scriptPath], { + encoding: "utf8", + env: { + ...process.env, + PATH: binDir, + PRE_COMMIT_VENV_MARKER: markerPath, + }, + }); + + expect(result.status).toBe(13); + expect(result.stderr).toContain("hook failed"); + expect(existsSync(readFileSync(markerPath, "utf8").trim())).toBe(false); }); it("keeps Windows WSL2 probe output normalized through the shared wrapper", () => { @@ -151,7 +225,9 @@ describe("check-workflows", () => { expect(workflow).toContain("wsl2_restart_required=true"); expect(workflow).toContain("import_ubuntu_wsl2=skipped_restart_required"); expect(workflow).toContain("wsl_exec_skipped=restart_required"); - expect(workflow).toContain('"wsl2_restart_required=$($restartRequired.ToString().ToLowerInvariant())"'); + expect(workflow).toContain( + '"wsl2_restart_required=$($restartRequired.ToString().ToLowerInvariant())"', + ); expect(workflow).toContain( '$exec = Invoke-WslText -Arguments @("-d", $distro, "--exec", "bash", "-lc"', ); @@ -164,7 +240,9 @@ describe("check-workflows", () => { const workflow = readFileSync(".github/workflows/windows-testbox-probe.yml", "utf8"); expect(workflow).toContain("run_windows_ci:"); - expect(workflow).toContain('description: "Run the focused Windows-native CI test shard after probing"'); + expect(workflow).toContain( + 'description: "Run the focused Windows-native CI test shard after probing"', + ); expect(workflow).toContain("default: false"); expect(workflow).toContain("if: ${{ inputs.run_windows_ci }}"); expect(workflow).toContain("source .github/actions/setup-pnpm-store-cache/ensure-node.sh");