From 547f5047d37beb4f69d832487c6ca9d5bdeee0a4 Mon Sep 17 00:00:00 2001 From: jnjpng Date: Wed, 21 Jan 2026 22:30:20 -0800 Subject: [PATCH] feat: add cross-platform support for hooks executor (#623) --- src/hooks/executor.ts | 286 ++++++++++++++++++---------- src/tests/hooks/e2e.test.ts | 3 +- src/tests/hooks/executor.test.ts | 3 +- src/tests/hooks/integration.test.ts | 3 +- 4 files changed, 192 insertions(+), 103 deletions(-) diff --git a/src/hooks/executor.ts b/src/hooks/executor.ts index 155fc7f..f3b0079 100644 --- a/src/hooks/executor.ts +++ b/src/hooks/executor.ts @@ -1,7 +1,9 @@ // src/hooks/executor.ts // Executes hook commands with JSON input via stdin +// Cross-platform: uses platform-appropriate shell (PowerShell on Windows, sh/bash/zsh on Unix) -import { spawn } from "node:child_process"; +import { type ChildProcess, spawn } from "node:child_process"; +import { buildShellLaunchers } from "../tools/impl/shellLaunchers"; import { type HookCommand, type HookExecutionResult, @@ -13,8 +15,35 @@ import { /** Default timeout for hook execution (60 seconds) */ const DEFAULT_TIMEOUT_MS = 60000; +/** + * Try to spawn a hook command with a specific launcher + * Returns the child process or throws an error + */ +function trySpawnWithLauncher( + launcher: string[], + workingDirectory: string, + input: HookInput, +): ChildProcess { + const [executable, ...args] = launcher; + if (!executable) { + throw new Error("Empty launcher"); + } + + return spawn(executable, args, { + cwd: workingDirectory, + env: { + ...process.env, + // Add hook-specific environment variables + LETTA_HOOK_EVENT: input.event_type, + LETTA_WORKING_DIR: workingDirectory, + }, + stdio: ["pipe", "pipe", "pipe"], + }); +} + /** * Execute a single hook command with JSON input via stdin + * Uses cross-platform shell launchers with fallback support */ export async function executeHookCommand( hook: HookCommand, @@ -25,7 +54,73 @@ export async function executeHookCommand( const timeout = hook.timeout ?? DEFAULT_TIMEOUT_MS; const inputJson = JSON.stringify(input); - return new Promise((resolve) => { + // Get platform-appropriate shell launchers + const launchers = buildShellLaunchers(hook.command); + if (launchers.length === 0) { + return { + exitCode: HookExitCode.ERROR, + stdout: "", + stderr: "", + timedOut: false, + durationMs: Date.now() - startTime, + error: "No shell launchers available for this platform", + }; + } + + // Try each launcher until one works + let lastError: Error | null = null; + + for (const launcher of launchers) { + try { + const result = await executeWithLauncher( + launcher, + inputJson, + workingDirectory, + input, + timeout, + hook.command, + startTime, + ); + return result; + } catch (error) { + // If ENOENT (executable not found), try the next launcher + if ( + error instanceof Error && + "code" in error && + (error as NodeJS.ErrnoException).code === "ENOENT" + ) { + lastError = error; + continue; + } + // For other errors, fail immediately + throw error; + } + } + + // All launchers failed + return { + exitCode: HookExitCode.ERROR, + stdout: "", + stderr: "", + timedOut: false, + durationMs: Date.now() - startTime, + error: `Failed to execute hook: ${lastError?.message || "No suitable shell found"}`, + }; +} + +/** + * Execute a hook with a specific launcher + */ +function executeWithLauncher( + launcher: string[], + inputJson: string, + workingDirectory: string, + input: HookInput, + timeout: number, + command: string, + startTime: number, +): Promise { + return new Promise((resolve, reject) => { let stdout = ""; let stderr = ""; let timedOut = false; @@ -48,112 +143,103 @@ export async function executeHookCommand( } }; + // Log hook start + console.log(`\x1b[90m[hook] Running: ${command}\x1b[0m`); + + let child: ChildProcess; try { - // Log hook start - console.log(`\x1b[90m[hook] Running: ${hook.command}\x1b[0m`); - - // Spawn shell process to run the hook command - const child = spawn("sh", ["-c", hook.command], { - cwd: workingDirectory, - env: { - ...process.env, - // Add hook-specific environment variables - LETTA_HOOK_EVENT: input.event_type, - LETTA_WORKING_DIR: workingDirectory, - }, - stdio: ["pipe", "pipe", "pipe"], - }); - - // Set up timeout - const timeoutId = setTimeout(() => { - timedOut = true; - child.kill("SIGTERM"); - // Give process time to clean up, then force kill - setTimeout(() => { - if (!resolved) { - child.kill("SIGKILL"); - } - }, 1000); - }, timeout); - - // Write JSON input to stdin - if (child.stdin) { - // Handle stdin errors (e.g., EPIPE if process exits before reading) - child.stdin.on("error", () => { - // Silently ignore - process may have exited before reading stdin - }); - child.stdin.write(inputJson); - child.stdin.end(); - } - - // Collect stdout - if (child.stdout) { - child.stdout.on("data", (data: Buffer) => { - stdout += data.toString(); - }); - } - - // Collect stderr - if (child.stderr) { - child.stderr.on("data", (data: Buffer) => { - stderr += data.toString(); - }); - } - - // Handle process exit - child.on("close", (code) => { - clearTimeout(timeoutId); - const durationMs = Date.now() - startTime; - - // Map exit code to our enum - let exitCode: HookExitCode; - if (timedOut) { - exitCode = HookExitCode.ERROR; - } else if (code === null) { - exitCode = HookExitCode.ERROR; - } else if (code === 0) { - exitCode = HookExitCode.ALLOW; - } else if (code === 2) { - exitCode = HookExitCode.BLOCK; - } else { - exitCode = HookExitCode.ERROR; - } - - safeResolve({ - exitCode, - stdout: stdout.trim(), - stderr: stderr.trim(), - timedOut, - durationMs, - ...(timedOut && { error: `Hook timed out after ${timeout}ms` }), - }); - }); - - // Handle spawn error - child.on("error", (error) => { - clearTimeout(timeoutId); - const durationMs = Date.now() - startTime; - - safeResolve({ - exitCode: HookExitCode.ERROR, - stdout: stdout.trim(), - stderr: stderr.trim(), - timedOut: false, - durationMs, - error: `Failed to execute hook: ${error.message}`, - }); - }); + child = trySpawnWithLauncher(launcher, workingDirectory, input); } catch (error) { + reject(error); + return; + } + + // Set up timeout + const timeoutId = setTimeout(() => { + timedOut = true; + child.kill("SIGTERM"); + // Give process time to clean up, then force kill + setTimeout(() => { + if (!resolved) { + child.kill("SIGKILL"); + } + }, 1000); + }, timeout); + + // Write JSON input to stdin + if (child.stdin) { + // Handle stdin errors (e.g., EPIPE if process exits before reading) + child.stdin.on("error", () => { + // Silently ignore - process may have exited before reading stdin + }); + child.stdin.write(inputJson); + child.stdin.end(); + } + + // Collect stdout + if (child.stdout) { + child.stdout.on("data", (data: Buffer) => { + stdout += data.toString(); + }); + } + + // Collect stderr + if (child.stderr) { + child.stderr.on("data", (data: Buffer) => { + stderr += data.toString(); + }); + } + + // Handle process exit + child.on("close", (code: number | null) => { + clearTimeout(timeoutId); + const durationMs = Date.now() - startTime; + + // Map exit code to our enum + let exitCode: HookExitCode; + if (timedOut) { + exitCode = HookExitCode.ERROR; + } else if (code === null) { + exitCode = HookExitCode.ERROR; + } else if (code === 0) { + exitCode = HookExitCode.ALLOW; + } else if (code === 2) { + exitCode = HookExitCode.BLOCK; + } else { + exitCode = HookExitCode.ERROR; + } + + safeResolve({ + exitCode, + stdout: stdout.trim(), + stderr: stderr.trim(), + timedOut, + durationMs, + ...(timedOut && { error: `Hook timed out after ${timeout}ms` }), + }); + }); + + // Handle spawn error - reject to try next launcher + child.on("error", (error: NodeJS.ErrnoException) => { + clearTimeout(timeoutId); + + // For ENOENT, reject so we can try the next launcher + if (error.code === "ENOENT") { + reject(error); + return; + } + + // For other errors, resolve with error result const durationMs = Date.now() - startTime; safeResolve({ exitCode: HookExitCode.ERROR, - stdout: "", - stderr: "", + stdout: stdout.trim(), + stderr: stderr.trim(), timedOut: false, durationMs, - error: `Failed to spawn hook process: ${error instanceof Error ? error.message : String(error)}`, + error: `Failed to execute hook: ${error.message}`, }); - } + }); }); } diff --git a/src/tests/hooks/e2e.test.ts b/src/tests/hooks/e2e.test.ts index e2dc43b..1a21d8e 100644 --- a/src/tests/hooks/e2e.test.ts +++ b/src/tests/hooks/e2e.test.ts @@ -15,7 +15,8 @@ import { join } from "node:path"; const projectRoot = process.cwd(); -// Skip on Windows - hooks executor uses `sh -c` which doesn't exist on Windows +// Skip on Windows - test commands use bash syntax (>>, &&, cat, etc.) +// The executor itself is cross-platform, but these test commands are bash-specific const isWindows = process.platform === "win32"; interface TestEnv { diff --git a/src/tests/hooks/executor.test.ts b/src/tests/hooks/executor.test.ts index f875847..10ca6ca 100644 --- a/src/tests/hooks/executor.test.ts +++ b/src/tests/hooks/executor.test.ts @@ -16,7 +16,8 @@ import { type StopHookInput, } from "../../hooks/types"; -// Skip on Windows - hooks executor uses `sh -c` which doesn't exist on Windows +// Skip on Windows - test commands use bash syntax (&&, >&2, sleep, etc.) +// The executor itself is cross-platform, but these test commands are bash-specific const isWindows = process.platform === "win32"; describe.skipIf(isWindows)("Hooks Executor", () => { diff --git a/src/tests/hooks/integration.test.ts b/src/tests/hooks/integration.test.ts index 8290f33..9e9d80c 100644 --- a/src/tests/hooks/integration.test.ts +++ b/src/tests/hooks/integration.test.ts @@ -21,7 +21,8 @@ import { runUserPromptSubmitHooks, } from "../../hooks"; -// Skip on Windows - hooks executor uses `sh -c` which doesn't exist on Windows +// Skip on Windows - test commands use bash syntax (&&, >&2, etc.) +// The executor itself is cross-platform, but these test commands are bash-specific const isWindows = process.platform === "win32"; describe.skipIf(isWindows)("Hooks Integration Tests", () => {