From 9640d24a5f409f4d94dea37d02f54f0eab096d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20V=C3=A1clav?= <128968016+sv789@users.noreply.github.com> Date: Mon, 20 Jul 2026 16:48:10 +0200 Subject: [PATCH] fix: make prepare script cross-platform so npm install works on Windows The prepare script used POSIX sh syntax, which fails under npm's default cmd.exe script shell on Windows and aborts npm install in git checkouts. Port the hook installer to a Node script. Fixes #778 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 ++ package.json | 2 +- scripts/install-hooks.mjs | 18 +++++++++++++ scripts/install-hooks.sh | 19 -------------- scripts/package-smoke.mjs | 15 +++++++++++ test/install-hooks.test.ts | 53 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 89 insertions(+), 20 deletions(-) create mode 100644 scripts/install-hooks.mjs delete mode 100755 scripts/install-hooks.sh create mode 100644 test/install-hooks.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b33fe9a..5dfb467 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ ### Fixed +- `npm install` / `prepare` no longer fails on Windows in a git checkout (#778). The hook installer is now cross-platform `scripts/install-hooks.mjs` instead of a POSIX shell script; `prepare` still runs `node scripts/build.mjs` so git installs keep producing `dist/` (#824). + - `qmd mcp` (stdio) now shuts down gracefully when stdin reaches EOF instead of orphaning to PID 1 when the parent MCP client dies (#751): the server closes its transport, gives in-flight request handlers a bounded window to diff --git a/package.json b/package.json index 580bd51..cc54e2b 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "CHANGELOG.md" ], "scripts": { - "prepare": "([ -d .git ] && ./scripts/install-hooks.sh || true) && node scripts/build.mjs", + "prepare": "node scripts/install-hooks.mjs && node scripts/build.mjs", "build": "node scripts/build.mjs", "test": "node scripts/test-all.mjs", "test:types": "node ./node_modules/typescript/bin/tsc -p tsconfig.build.json --noEmit", diff --git a/scripts/install-hooks.mjs b/scripts/install-hooks.mjs new file mode 100644 index 0000000..7902ee4 --- /dev/null +++ b/scripts/install-hooks.mjs @@ -0,0 +1,18 @@ +#!/usr/bin/env node +// Self-installing git hooks for qmd +// Called from the package.json "prepare" script after install +import { chmodSync, copyFileSync, existsSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const root = join(fileURLToPath(new URL("..", import.meta.url))); +const hooksDir = join(root, ".git", "hooks"); + +if (!existsSync(hooksDir)) { + console.log("Not a git repository, skipping hook install"); + process.exit(0); +} + +copyFileSync(join(root, "scripts", "pre-push"), join(hooksDir, "pre-push")); +chmodSync(join(hooksDir, "pre-push"), 0o755); +console.log("Installed git hooks: pre-push"); diff --git a/scripts/install-hooks.sh b/scripts/install-hooks.sh deleted file mode 100755 index a5a7ca4..0000000 --- a/scripts/install-hooks.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Self-installing git hooks for qmd -# Called from package.json "prepare" script after bun install - -REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" -HOOKS_DIR="$REPO_ROOT/.git/hooks" - -if [[ ! -d "$HOOKS_DIR" ]]; then - echo "Not a git repository, skipping hook install" - exit 0 -fi - -# Install pre-push hook -cp "$REPO_ROOT/scripts/pre-push" "$HOOKS_DIR/pre-push" -chmod +x "$HOOKS_DIR/pre-push" - -echo "Installed git hooks: pre-push" diff --git a/scripts/package-smoke.mjs b/scripts/package-smoke.mjs index f9622e7..40dd719 100644 --- a/scripts/package-smoke.mjs +++ b/scripts/package-smoke.mjs @@ -35,6 +35,21 @@ function assertPath(path, label = path) { return full; } +// prepare must be pure Node (no POSIX test/[) so Windows cmd.exe can run it, +// and must still invoke the build so git installs produce dist/. +{ + const prepare = pkg.scripts?.prepare ?? ""; + if (/\[\s*-d/.test(prepare) || prepare.includes("install-hooks.sh")) { + console.error("Package smoke failed: prepare still uses a POSIX shell hook installer"); + process.exit(1); + } + if (!prepare.includes("install-hooks.mjs") || !prepare.includes("scripts/build.mjs")) { + console.error("Package smoke failed: prepare must run install-hooks.mjs and build.mjs"); + process.exit(1); + } + console.log("==> prepare script is Windows-safe and builds dist"); +} + run("build compiled package", process.execPath, ["scripts/build.mjs"]); run("AST grammar runtime packages", process.execPath, ["scripts/check-package-grammars.mjs"]); diff --git a/test/install-hooks.test.ts b/test/install-hooks.test.ts new file mode 100644 index 0000000..4ba43a4 --- /dev/null +++ b/test/install-hooks.test.ts @@ -0,0 +1,53 @@ +import { describe, test, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, existsSync, statSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +const repoRoot = fileURLToPath(new URL("..", import.meta.url)); +const installer = join(repoRoot, "scripts", "install-hooks.mjs"); + +describe("scripts/install-hooks.mjs", () => { + let tempRoot: string; + beforeEach(() => { tempRoot = mkdtempSync(join(tmpdir(), "qmd-install-hooks-")); }); + afterEach(() => { rmSync(tempRoot, { recursive: true, force: true }); }); + + test("skips cleanly when .git/hooks is absent", () => { + const fakeRoot = join(tempRoot, "pkg"); + mkdirSync(join(fakeRoot, "scripts"), { recursive: true }); + writeFileSync(join(fakeRoot, "scripts", "pre-push"), "#!/bin/sh\necho hook\n"); + writeFileSync(join(fakeRoot, "scripts", "install-hooks.mjs"), readFileSync(installer, "utf8")); + const result = spawnSync(process.execPath, [join(fakeRoot, "scripts", "install-hooks.mjs")], { cwd: fakeRoot, encoding: "utf8" }); + expect(result.status).toBe(0); + expect(result.stdout).toMatch(/Not a git repository, skipping hook install/); + expect(existsSync(join(fakeRoot, ".git", "hooks", "pre-push"))).toBe(false); + }); + + test("copies pre-push into .git/hooks and makes it executable", () => { + const fakeRoot = join(tempRoot, "repo"); + mkdirSync(join(fakeRoot, "scripts"), { recursive: true }); + mkdirSync(join(fakeRoot, ".git", "hooks"), { recursive: true }); + writeFileSync(join(fakeRoot, "scripts", "pre-push"), "#!/bin/sh\necho pre-push-ok\n"); + writeFileSync(join(fakeRoot, "scripts", "install-hooks.mjs"), readFileSync(installer, "utf8")); + const result = spawnSync(process.execPath, [join(fakeRoot, "scripts", "install-hooks.mjs")], { cwd: fakeRoot, encoding: "utf8" }); + expect(result.status).toBe(0); + expect(result.stdout).toMatch(/Installed git hooks: pre-push/); + const dest = join(fakeRoot, ".git", "hooks", "pre-push"); + expect(readFileSync(dest, "utf8")).toContain("pre-push-ok"); + if (process.platform !== "win32") { + expect(statSync(dest).mode & 0o111).toBeTruthy(); + } + }); +}); + +describe("package.json prepare script", () => { + test("is Windows-safe Node and still builds dist", () => { + const pkg = JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")); + const prepare: string = pkg.scripts.prepare; + expect(prepare.includes("[ -d")).toBe(false); + expect(prepare.includes("install-hooks.sh")).toBe(false); + expect(prepare.includes("node scripts/install-hooks.mjs")).toBe(true); + expect(prepare.includes("node scripts/build.mjs")).toBe(true); + }); +});