From 9de9562cb78fe961ebc983cc09dcc7b1883a7f3a Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 18 Jun 2026 20:09:40 +0200 Subject: [PATCH] fix(git-hooks): avoid precommit dependency hydration --- scripts/pre-commit/run-node-tool.sh | 14 ++++- test/git-hooks-pre-commit.test.ts | 92 ++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/scripts/pre-commit/run-node-tool.sh b/scripts/pre-commit/run-node-tool.sh index 341630755173..168d74fec1f3 100755 --- a/scripts/pre-commit/run-node-tool.sh +++ b/scripts/pre-commit/run-node-tool.sh @@ -11,8 +11,20 @@ fi tool="$1" shift +local_tool="$ROOT_DIR/node_modules/.bin/$tool" +if [[ -x "$local_tool" ]]; then + exec "$local_tool" "$@" +fi + if [[ -f "$ROOT_DIR/pnpm-lock.yaml" ]] && command -v pnpm >/dev/null 2>&1; then - exec pnpm exec "$tool" "$@" + if [[ ! -e "$ROOT_DIR/node_modules" ]]; then + echo "Missing repo dependencies: cannot run $tool without node_modules." >&2 + echo "Run pnpm install in a normal checkout, or bypass the hook only after separate formatting proof." >&2 + exit 1 + fi + + echo "Missing local tool: $local_tool" >&2 + exit 1 fi if { [[ -f "$ROOT_DIR/bun.lockb" ]] || [[ -f "$ROOT_DIR/bun.lock" ]]; } && command -v bun >/dev/null 2>&1; then diff --git a/test/git-hooks-pre-commit.test.ts b/test/git-hooks-pre-commit.test.ts index d9655d704923..541a840fbc8a 100644 --- a/test/git-hooks-pre-commit.test.ts +++ b/test/git-hooks-pre-commit.test.ts @@ -1,6 +1,6 @@ // Git hook tests validate pre-commit hook behavior and scripts. import { execFileSync } from "node:child_process"; -import { mkdirSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, symlinkSync, writeFileSync } from "node:fs"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { cleanupTempDirs, makeTempRepoRoot } from "./helpers/temp-repo.js"; @@ -20,6 +20,35 @@ const run = (cwd: string, cmd: string, args: string[] = [], env?: NodeJS.Process }).trim(); }; +type FailedCommand = { + status: number; + stderr: string; + stdout: string; +}; + +const runFailure = ( + cwd: string, + cmd: string, + args: string[] = [], + env?: NodeJS.ProcessEnv, +): FailedCommand => { + try { + run(cwd, cmd, args, env); + } catch (error) { + if (error instanceof Error && "status" in error) { + const failure = error as Error & { status?: number; stderr?: string; stdout?: string }; + return { + status: failure.status ?? 1, + stderr: String(failure.stderr ?? ""), + stdout: String(failure.stdout ?? ""), + }; + } + throw error; + } + + throw new Error("expected command to fail"); +}; + function writeExecutable(dir: string, name: string, contents: string): void { writeFileSync(path.join(dir, name), contents, { encoding: "utf8", @@ -54,6 +83,14 @@ function installPreCommitFixture(dir: string): string { return fakeBinDir; } +function installRunNodeToolFixture(dir: string): void { + mkdirSync(path.join(dir, "scripts", "pre-commit"), { recursive: true }); + symlinkSync( + path.join(process.cwd(), "scripts", "pre-commit", "run-node-tool.sh"), + path.join(dir, "scripts", "pre-commit", "run-node-tool.sh"), + ); +} + function splitNonEmptyLines(output: string): string[] { const lines: string[] = []; for (const line of output.split("\n")) { @@ -163,3 +200,56 @@ describe("git-hooks/pre-commit (integration)", () => { expect(run(dir, "git", ["diff", "--cached", "--name-only"])).toBe("tracked.txt"); }); }); + +describe("scripts/pre-commit/run-node-tool.sh", () => { + it("runs the installed local tool without invoking pnpm", () => { + const dir = makeTempRepoRoot(tempDirs, "openclaw-run-node-tool-local-"); + installRunNodeToolFixture(dir); + writeFileSync(path.join(dir, "pnpm-lock.yaml"), "lockfileVersion: '9.0'\n", "utf8"); + + const fakeBinDir = path.join(dir, "bin"); + const toolBinDir = path.join(dir, "node_modules", ".bin"); + mkdirSync(fakeBinDir, { recursive: true }); + mkdirSync(toolBinDir, { recursive: true }); + writeExecutable( + fakeBinDir, + "pnpm", + "#!/usr/bin/env bash\necho 'pnpm should not run from run-node-tool' >&2\nexit 99\n", + ); + writeExecutable(toolBinDir, "oxfmt", "#!/usr/bin/env bash\nprintf 'local:%s\\n' \"$*\"\n"); + + expect( + run(dir, "bash", ["scripts/pre-commit/run-node-tool.sh", "oxfmt", "--write", "a.ts"], { + PATH: `${fakeBinDir}:${process.env.PATH ?? ""}`, + }), + ).toBe("local:--write a.ts"); + }); + + it("fails before pnpm can hydrate dependencies when node_modules is missing", () => { + const dir = makeTempRepoRoot(tempDirs, "openclaw-run-node-tool-missing-deps-"); + installRunNodeToolFixture(dir); + writeFileSync(path.join(dir, "pnpm-lock.yaml"), "lockfileVersion: '9.0'\n", "utf8"); + + const fakeBinDir = path.join(dir, "bin"); + const markerPath = path.join(dir, "pnpm-called"); + mkdirSync(fakeBinDir, { recursive: true }); + writeExecutable( + fakeBinDir, + "pnpm", + `#!/usr/bin/env bash\ntouch ${JSON.stringify(markerPath)}\nexit 99\n`, + ); + + const result = runFailure( + dir, + "bash", + ["scripts/pre-commit/run-node-tool.sh", "oxfmt", "--write", "a.ts"], + { PATH: `${fakeBinDir}:${process.env.PATH ?? ""}` }, + ); + + expect(result.status).toBe(1); + expect(result.stderr).toContain( + "Missing repo dependencies: cannot run oxfmt without node_modules.", + ); + expect(existsSync(markerPath)).toBe(false); + }); +});