From 108c8bdb84bf9ac80f5b4b8fd5d83e17049f1b50 Mon Sep 17 00:00:00 2001 From: Evgeniy Podivilov Date: Tue, 21 Jul 2026 21:46:26 +0100 Subject: [PATCH] fix(hooks): warn when the platform has no sh instead of failing silently Hook commands run via `sh -c`, which native Windows does not provide, so every configured hook failed with an opaque "Failed to execute command". The shell adapter now resolves `sh` on PATH and returns a dedicated SHELL_UNAVAILABLE error. Hook call sites report it once, name how many hooks were skipped, and stop retrying commands that cannot run. Document the hook execution model in the README, including native Windows (cmd.exe / PowerShell) as a known limitation. --- README.md | 18 ++++++ src/application/use-cases/run-hooks.test.ts | 45 ++++++++++++++ src/application/use-cases/run-hooks.ts | 20 +++++-- src/cli/commands/create.ts | 13 +++- src/cli/commands/remove.ts | 23 ++++++- src/domain/ports/shell-port.ts | 6 +- .../adapters/bun-shell-adapter.test.ts | 60 +++++++++++++++++++ .../adapters/bun-shell-adapter.ts | 29 ++++++++- 8 files changed, 204 insertions(+), 10 deletions(-) create mode 100644 src/infrastructure/adapters/bun-shell-adapter.test.ts diff --git a/README.md b/README.md index 81f0f8b..4e312a7 100644 --- a/README.md +++ b/README.md @@ -528,6 +528,24 @@ All hooks receive the following environment variables: | `REPO_ROOT` | Repository root path | all | | `BASE_BRANCH` | Base branch (create: `--base` value; update: parent branch) | `post-create`, `post-update`, `on-conflict` | +### Hook Execution Model + +Hooks are shell strings, not argument arrays. Each command is executed as `sh -c ""` with the worktree as the working directory, so pipes, redirects, `&&`, globs, and variable expansion all work as they do in a terminal: + +```jsonc +{ + "hooks": { + "post-create": ["pnpm install --frozen-lockfile && cp ../../.env.shared .env"] + } +} +``` + +This is deliberate. Hook commands come from your own repo config (`.worktreekit.jsonc`, `.worktreekit.local.jsonc`, or the global config), which is the same trust level as a `package.json` script or a git hook — worktree-kit does not escape, sandbox, or validate them. Treat a config file from an untrusted repo the way you would treat its build scripts. + +Each command inherits the parent environment plus the hook variables above. A non-zero exit code is reported as a warning and does not abort the command (except `on-conflict`, which is expected to resolve the rebase). Commands time out after 5 minutes. + +**Windows:** hooks require `sh` on `PATH`. Under Git Bash or WSL they work normally; under native `cmd.exe` / PowerShell there is no `sh`, so worktree-kit warns that hooks were skipped and continues with the rest of the command. Running hooks through `cmd.exe` or PowerShell is not supported — a config written for `sh` would not survive the translation, and the alternative shells are not interchangeable enough to pick one automatically. + ## Migration from `.worktreekitrc` If you have an existing `.worktreekitrc` config, run: diff --git a/src/application/use-cases/run-hooks.test.ts b/src/application/use-cases/run-hooks.test.ts index 23d2ea5..bac3a71 100644 --- a/src/application/use-cases/run-hooks.test.ts +++ b/src/application/use-cases/run-hooks.test.ts @@ -105,6 +105,51 @@ describe("runHooks", () => { expect(shell.calls[0]?.options.cwd).toBe("/worktrees/feature"); }); + test("warns once and skips remaining commands when no shell is available", async () => { + const shell = createFakeShell({ + defaultResult: Result.err({ code: "SHELL_UNAVAILABLE", message: "no POSIX shell on PATH" }), + }); + + const result = await runHooks( + { + commands: ["pnpm install", "cp .env.example .env", "echo done"], + context: defaultContext, + }, + { shell }, + ); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.notifications).toHaveLength(1); + expect(result.data.notifications[0]?.level).toBe("warn"); + expect(result.data.notifications[0]?.message).toBe("Skipped 3 hook(s): no POSIX shell on PATH"); + expect(result.data.failedCommands).toEqual(["pnpm install", "cp .env.example .env", "echo done"]); + } + // Stops after the first attempt — no shell means no command can run. + expect(shell.calls).toHaveLength(1); + }); + + test("reports only the unrun commands when the shell disappears mid-run", async () => { + const results = new Map(); + results.set("second", Result.err({ code: "SHELL_UNAVAILABLE", message: "no POSIX shell on PATH" })); + + const shell = createFakeShell({ results }); + const result = await runHooks( + { + commands: ["first", "second", "third"], + context: defaultContext, + }, + { shell }, + ); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.failedCommands).toEqual(["second", "third"]); + expect(result.data.notifications.map((n) => n.level)).toEqual(["info", "warn"]); + expect(result.data.notifications[1]?.message).toBe("Skipped 2 hook(s): no POSIX shell on PATH"); + } + }); + test("returns empty results for empty commands", async () => { const shell = createFakeShell(); const result = await runHooks( diff --git a/src/application/use-cases/run-hooks.ts b/src/application/use-cases/run-hooks.ts index 06a7dc5..51ac121 100644 --- a/src/application/use-cases/run-hooks.ts +++ b/src/application/use-cases/run-hooks.ts @@ -38,18 +38,28 @@ export async function runHooks(input: RunHooksInput, deps: RunHooksDeps): Promis ...(context.baseBranch && { BASE_BRANCH: context.baseBranch }), }; - for (const command of commands) { + for (const [index, command] of commands.entries()) { const result = await shell.execute(command, { cwd: context.worktreePath, env, }); - if (!result.success) { - failedCommands.push(command); - notifications.push(N.warn(`Hook failed: "${command}" - ${result.error.message}`)); - } else { + if (result.success) { notifications.push(N.info(`Hook completed: "${command}"`)); + continue; } + + // No shell means no hook can ever run — report once and stop instead of + // repeating the same failure for every remaining command. + if (result.error.code === "SHELL_UNAVAILABLE") { + const skipped = commands.slice(index); + failedCommands.push(...skipped); + notifications.push(N.warn(`Skipped ${skipped.length} hook(s): ${result.error.message}`)); + break; + } + + failedCommands.push(command); + notifications.push(N.warn(`Hook failed: "${command}" - ${result.error.message}`)); } return R.ok({ notifications, failedCommands }); diff --git a/src/cli/commands/create.ts b/src/cli/commands/create.ts index 5e84715..bb30289 100644 --- a/src/cli/commands/create.ts +++ b/src/cli/commands/create.ts @@ -186,6 +186,8 @@ export function createCommand(container: Container) { env.BASE_BRANCH = hookContext.baseBranch; } + let shellUnavailable: { message: string; skipped: number } | undefined; + for (const [i, command] of hookCommands.entries()) { const message = `Running hook ${i + 1}/${total}: ${command}...`; @@ -201,11 +203,20 @@ export function createCommand(container: Container) { }); if (!result.success) { + if (result.error.code === "SHELL_UNAVAILABLE") { + shellUnavailable = { message: result.error.message, skipped: total - i }; + break; + } ui.warn(`Hook failed: "${command}" - ${result.error.message}`); } } - hooksSpinner.stop(pc.green("Hooks completed")); + if (shellUnavailable) { + hooksSpinner.stop(pc.yellow("Hooks skipped")); + ui.warn(`Skipped ${shellUnavailable.skipped} hook(s): ${shellUnavailable.message}`); + } else { + hooksSpinner.stop(pc.green("Hooks completed")); + } } ui.success(`Created worktree for branch: ${branch} at ${createResult.data.worktree.path}`); diff --git a/src/cli/commands/remove.ts b/src/cli/commands/remove.ts index 7de8f4d..fadfe89 100644 --- a/src/cli/commands/remove.ts +++ b/src/cli/commands/remove.ts @@ -141,6 +141,8 @@ export function removeCommand(container: Container) { REPO_ROOT: repoRoot, }; + let shellUnavailable: { message: string; skipped: number } | undefined; + for (const [i, command] of preRemoveHooks.entries()) { const message = `Running pre-remove hook ${i + 1}/${total}: ${command}...`; if (i === 0) { @@ -155,10 +157,20 @@ export function removeCommand(container: Container) { }); if (!hookResult.success) { + if (hookResult.error.code === "SHELL_UNAVAILABLE") { + shellUnavailable = { message: hookResult.error.message, skipped: total - i }; + break; + } ui.warn(`Pre-remove hook failed: "${command}" - ${hookResult.error.message}`); } } - hooksSpinner.stop(pc.green("Pre-remove hooks completed")); + + if (shellUnavailable) { + hooksSpinner.stop(pc.yellow("Pre-remove hooks skipped")); + ui.warn(`Skipped ${shellUnavailable.skipped} pre-remove hook(s): ${shellUnavailable.message}`); + } else { + hooksSpinner.stop(pc.green("Pre-remove hooks completed")); + } } // Remove worktree @@ -231,6 +243,7 @@ export function removeCommand(container: Container) { const ms = ui.createMultiSpinner(keys); const warnings: string[] = []; const unmergedBranches: string[] = []; + let shellUnavailableMessage: string | undefined; await Promise.all( worktreesToRemove.map(async (wt) => { @@ -251,6 +264,10 @@ export function removeCommand(container: Container) { env, }); if (!hookResult.success) { + if (hookResult.error.code === "SHELL_UNAVAILABLE") { + shellUnavailableMessage = hookResult.error.message; + break; + } warnings.push(`Hook failed for "${displayLabel}": ${command}`); } } @@ -298,6 +315,10 @@ export function removeCommand(container: Container) { ms.stop(); + if (shellUnavailableMessage) { + warnings.push(`Pre-remove hooks skipped: ${shellUnavailableMessage}`); + } + for (const warning of warnings) { ui.warn(warning); } diff --git a/src/domain/ports/shell-port.ts b/src/domain/ports/shell-port.ts index ee08b22..7963808 100644 --- a/src/domain/ports/shell-port.ts +++ b/src/domain/ports/shell-port.ts @@ -1,7 +1,11 @@ import type { Result } from "../../shared/result.ts"; export interface ShellError { - readonly code: "EXECUTION_FAILED" | "TIMEOUT" | "UNKNOWN"; + /** + * `SHELL_UNAVAILABLE` — the platform provides no POSIX shell to run the command with. + * Commands are shell strings, so there is no fallback: callers should report and skip. + */ + readonly code: "EXECUTION_FAILED" | "TIMEOUT" | "SHELL_UNAVAILABLE" | "UNKNOWN"; readonly message: string; readonly exitCode?: number; readonly stderr?: string; diff --git a/src/infrastructure/adapters/bun-shell-adapter.test.ts b/src/infrastructure/adapters/bun-shell-adapter.test.ts new file mode 100644 index 0000000..3a53263 --- /dev/null +++ b/src/infrastructure/adapters/bun-shell-adapter.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test } from "bun:test"; +import { expectErr, expectOk } from "../../test-utils/assertions.ts"; +import { createNoopLogger } from "../../test-utils/noop-logger.ts"; +import { createTempDir } from "../../test-utils/temp-dir.ts"; +import { createBunShellAdapter } from "./bun-shell-adapter.ts"; + +describe("BunShellAdapter", () => { + test("runs the command through the resolved shell", async () => { + await using tmp = await createTempDir(); + const shell = createBunShellAdapter(createNoopLogger()); + + const result = expectOk(await shell.execute("echo hello", { cwd: tmp.path })); + + expect(result.stdout).toBe("hello"); + expect(result.exitCode).toBe(0); + }); + + test("passes env variables to the command", async () => { + await using tmp = await createTempDir(); + const shell = createBunShellAdapter(createNoopLogger()); + + const result = expectOk( + await shell.execute("echo $WORKTREE_BRANCH", { cwd: tmp.path, env: { WORKTREE_BRANCH: "feature" } }), + ); + + expect(result.stdout).toBe("feature"); + }); + + test("returns EXECUTION_FAILED for a non-zero exit code", async () => { + await using tmp = await createTempDir(); + const shell = createBunShellAdapter(createNoopLogger()); + + const error = expectErr(await shell.execute("exit 3", { cwd: tmp.path })); + + expect(error.code).toBe("EXECUTION_FAILED"); + expect(error.exitCode).toBe(3); + }); + + test("returns SHELL_UNAVAILABLE when sh is not on PATH", async () => { + const shell = createBunShellAdapter(createNoopLogger(), () => null); + + const error = expectErr(await shell.execute("echo hello", { cwd: process.cwd() })); + + expect(error.code).toBe("SHELL_UNAVAILABLE"); + expect(error.message).toContain("sh -c"); + }); + + test("resolves the shell once and reuses the lookup", async () => { + const lookups: string[] = []; + const shell = createBunShellAdapter(createNoopLogger(), (cmd) => { + lookups.push(cmd); + return null; + }); + + await shell.execute("echo one", { cwd: process.cwd() }); + await shell.execute("echo two", { cwd: process.cwd() }); + + expect(lookups).toEqual(["sh"]); + }); +}); diff --git a/src/infrastructure/adapters/bun-shell-adapter.ts b/src/infrastructure/adapters/bun-shell-adapter.ts index 9cd52cf..a70bc4c 100644 --- a/src/infrastructure/adapters/bun-shell-adapter.ts +++ b/src/infrastructure/adapters/bun-shell-adapter.ts @@ -4,7 +4,23 @@ import { Result } from "../../shared/result.ts"; const DEFAULT_TIMEOUT = 5 * 60 * 1000; // 5 minutes -export function createBunShellAdapter(logger: LoggerPort): ShellPort { +const SHELL_UNAVAILABLE_MESSAGE = + "no POSIX shell on PATH — commands are shell strings run via `sh -c`, which native Windows does not provide (run wt from Git Bash or WSL)"; + +/** Resolves an executable to its absolute path, or `null` when it is not on PATH. */ +export type WhichFn = (command: string) => string | null; + +export function createBunShellAdapter(logger: LoggerPort, which: WhichFn = (cmd) => Bun.which(cmd)): ShellPort { + let shellPath: string | null | undefined; + + function resolveShell(): string | null { + if (shellPath === undefined) { + shellPath = which("sh"); + logger.debug("shell", `sh -> ${shellPath ?? "not found"}`); + } + return shellPath; + } + return { async execute(command: string, options: ShellExecuteOptions): Promise> { const { cwd, env = {}, timeout = DEFAULT_TIMEOUT } = options; @@ -12,10 +28,19 @@ export function createBunShellAdapter(logger: LoggerPort): ShellPort { logger.debug("shell", command); logger.debug("shell", `cwd: ${cwd}`); + const sh = resolveShell(); + if (sh === null) { + logger.debug("shell", "-> SHELL_UNAVAILABLE"); + return Result.err({ + code: "SHELL_UNAVAILABLE", + message: SHELL_UNAVAILABLE_MESSAGE, + }); + } + const startTime = Date.now(); try { - const proc = Bun.spawn(["sh", "-c", command], { + const proc = Bun.spawn([sh, "-c", command], { cwd, env: { ...process.env, ...env }, stdout: "pipe",