diff --git a/CLAUDE.md b/CLAUDE.md index 05ebf84..5fe3ddf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,6 +81,25 @@ mixes audio and muxes/transcodes per preset. `userData/claude-env.json`, extra MCP servers via `userData/claude-mcp.json`; `sweepStaleSessions()` clears leftovers of hard-killed runs at startup. + THE CLI IS LOCATED IN NODE, NOT BY A SHELL: a bare name is walked along + the session's PATH (with PATHEXT on Windows — npm installs a `claude.cmd` + shim, the native installer a `claude.exe`, and CreateProcess alone would + find only the latter), then `~/.local/bin`; a missing CLI is reported by + name instead of as the pty's «File not found». On Windows the launcher is + spawned directly, without the bash watchdog: the pseudoconsole belongs to + the Electron process, so a hard death of Electron takes the console host + and every attached process with it (verified with taskkill /F on main: + claude.exe and its cmd.exe were gone within seconds), and node-pty's + ConPTY kill terminates the console's whole process list. A `.cmd` + launcher (npm's shim) is started through `%ComSpec% /d /s /c ""` + by `winLaunch`: `/d` skips the registry AutoRun, and `/s` plus the outer + quotes keep a launcher path with a space in one piece — a bare + `cmd /c "shim" args` (PR #7's route) stops at `C:\Users\Jane` because + cmd strips the first and last quote of its command. Verified with a + shim in a directory with a space. The caveat that remains: cmd's parser + rewrites `%VAR%`, `^` and unquoted `& | < >` — the built-in args avoid + those (verified: quotes and non-ASCII reach claude.exe intact) and a + user `args` override on Windows has to as well. Open and close are SERIALIZED through one promise chain and carry a generation: spawning is async (config read, `which`, the node-pty import) while a close is instant, so a close that overtakes an in-flight open would diff --git a/electron/claude.ts b/electron/claude.ts index 77b7d45..03aaadf 100644 --- a/electron/claude.ts +++ b/electron/claude.ts @@ -5,12 +5,13 @@ import { app, BrowserWindow, ipcMain } from 'electron' import { createServer, type Server } from 'http' import { randomBytes } from 'crypto' -import { execFile } from 'child_process' -import { promises as fs } from 'fs' -import { join } from 'path' -import { tmpdir } from 'os' +import { constants as fsConstants, promises as fs } from 'fs' +import { delimiter, extname, join } from 'path' +import { homedir, tmpdir } from 'os' import type { IPty } from 'node-pty' +const WIN = process.platform === 'win32' + // The session inherits this process's environment MINUS the markers of any // Claude session that launched the editor (see SESSION_MARKERS). Anything extra the // user's claude needs (proxies, custom PATH…) plus command/args overrides @@ -185,12 +186,102 @@ function startBridge( }) } -function which(cmd: string): Promise { - return new Promise((resolve) => { - execFile('/bin/sh', ['-c', `command -v ${cmd}`], (err, stdout) => { - resolve(err ? null : stdout.trim() || null) - }) - }) +function expandHome(p: string): string { + return p === '~' || p.startsWith('~/') || (WIN && p.startsWith('~\\')) + ? join(homedir(), p.slice(2)) + : p +} + +/** the value of an environment variable, by Windows' rules if need be (names are case-insensitive there) */ +function envVar(env: Record, name: string): string | undefined { + if (!WIN) return env[name] + const key = Object.keys(env).find((k) => k.toLowerCase() === name.toLowerCase()) + return key === undefined ? undefined : env[key] +} + +async function isFile(p: string): Promise { + try { + return (await fs.stat(p)).isFile() + } catch { + return false + } +} + +/** + * Where the CLI is. Resolved here rather than by asking a shell: there is no + * /bin/sh on Windows, and the shell never knew more than we do — a + * non-interactive sh reads no rc file, so its PATH was ours. + * + * A name with a separator is a path (`~` expanded). A bare name is walked + * along the session's PATH — the user may have extended it in claude-env.json + * precisely so that claude is found — with PATHEXT on Windows, which is how + * every installer's launcher turns up: npm's `claude.cmd` shim, the native + * installer's `claude.exe`, bun/pnpm/volta shims. (CreateProcess alone would + * try `.exe` and nothing else, and `where claude` lists npm's extensionless + * sh-script shim first — a file ConPTY cannot start.) Then ~/.local/bin, the + * native installer's directory on every platform, for an editor launched + * from a desktop entry whose PATH has not caught up with the install. + */ +async function resolveCommand( + cmd: string, + env: Record +): Promise { + const exts = WIN + ? (envVar(env, 'PATHEXT') ?? '.COM;.EXE;.BAT;.CMD').split(';').filter(Boolean) + : [] + const runnable = async (p: string): Promise => { + if (WIN) { + // a name with a known extension is taken as is; anything else gets one + const ownExt = extname(p).toUpperCase() + const tries = exts.some((e) => e.toUpperCase() === ownExt) ? [p] : exts.map((e) => p + e) + for (const t of tries) if (await isFile(t)) return t + return null + } + try { + await fs.access(p, fsConstants.X_OK) + return (await isFile(p)) ? p : null + } catch { + return null + } + } + const name = expandHome(cmd) + if (/[\\/]/.test(name)) return runnable(name) + const dirs = (envVar(env, 'PATH') ?? '').split(delimiter).filter(Boolean) + dirs.push(join(homedir(), '.local', 'bin')) + for (const dir of dirs) { + const hit = await runnable(join(dir, name)) + if (hit) return hit + } + return null +} + +/** One argument quoted by the MSVCRT rules every CreateProcess-started program parses by. */ +function quoteArg(arg: string): string { + if (arg !== '' && !/[\s"]/.test(arg)) return arg + return '"' + arg.replace(/(\\*)"/g, '$1$1\\"').replace(/(\\+)$/, '$1$1') + '"' +} + +/** + * How the launcher is started on Windows. An `.exe` takes the arguments as + * they are. A `.cmd`/`.bat` — npm's shim — needs cmd.exe. CreateProcess + * would supply one by itself, but the explicit route is worth having: + * `/d` keeps the registry's AutoRun (clink, conda hooks) from running and + * printing into the panel, and the quoting is ours to get right. `/s` plus + * an outer pair of quotes is what keeps a launcher path with a space in it + * (`C:\Users\Jane Doe\AppData\Roaming\npm\claude.cmd`) in one piece: cmd + * strips the first and the last quote of its command when the first + * character is one, so a bare `/c "shim" args` loses the quotes around the + * path and stops at `C:\Users\Jane`. Verified with a shim in a directory + * with a space, both routes, inner quotes and non-ASCII arriving intact. + */ +function winLaunch( + bin: string, + args: string[], + env: Record +): [file: string, args: string | string[]] { + if (!/\.(cmd|bat)$/i.test(bin)) return [bin, args] + const line = [bin, ...args].map(quoteArg).join(' ') + return [envVar(env, 'ComSpec') ?? 'cmd.exe', `/d /s /c "${line}"`] } /** @@ -236,7 +327,17 @@ const SESSION_MARKERS = [ function sessionEnv(extra?: Record): Record { const env: Record = { ...process.env } as Record for (const key of SESSION_MARKERS) delete env[key] - return { ...env, ...extra } + for (const [key, value] of Object.entries(extra ?? {})) { + // Windows variable names are case-insensitive: a `PATH` override next to + // the inherited `Path` would hand CreateProcess two spellings of one + // variable, and which of them the child sees is not defined + if (WIN) { + const clash = Object.keys(env).find((k) => k !== key && k.toLowerCase() === key.toLowerCase()) + if (clash) delete env[clash] + } + env[key] = value + } + return env } interface ClaudeConfig { @@ -262,8 +363,17 @@ async function spawnSession( ): Promise<{ ok: boolean; port?: number; error?: string }> { killSession() // a reopen without a close in between const cfg = await userConfig() + const env = sessionEnv(cfg.env) const cmdName = process.env.KADR_CLAUDE_CMD || cfg.command || 'claude' - const bin = (await which(cmdName)) ?? cmdName + const bin = await resolveCommand(cmdName, env) + if (!bin) { + return { + ok: false, + error: + `"${cmdName}" not found on PATH (nor in ~/.local/bin) — install the Claude Code CLI, ` + + `or set "command" in ${join(app.getPath('userData'), 'claude-env.json')}` + } + } let bridge: { server: Server; port: number; token: string } try { @@ -315,13 +425,30 @@ async function spawnSession( const wrapper = `(while kill -0 ${process.pid} 2>/dev/null; do sleep 3; done; ` + `kill -HUP -$$ 2>/dev/null; sleep 2; kill -9 -$$ 2>/dev/null) & exec "$0" "$@"` - const p = pty.spawn('/bin/bash', ['-c', wrapper, bin, ...args], { - name: 'xterm-256color', - cols: Math.max(20, cols), - rows: Math.max(5, rows), - cwd: dir, - env: sessionEnv(cfg.env) - }) + // On Windows the console itself is the watchdog. The pseudoconsole is a + // handle of THIS process: when it dies, however hard, the console host + // goes with it and every process attached to that console receives + // CTRL_CLOSE_EVENT and is terminated — claude and the MCP children it + // spawned alike. So the launcher is started directly, no wrapper. + // Mind the launcher: a `.cmd` (npm's shim) goes through cmd.exe (see + // winLaunch), whose parser rewrites `%VAR%`, `^` and unquoted `& | < >` + // on the way. The built-in args carry none of those; keep it so, and on + // Windows keep any `args` override in claude-env.json equally plain — + // or point `command` at the `.exe` the shim wraps. + const p = WIN + ? pty.spawn(...winLaunch(bin, args, env), { + cols: Math.max(20, cols), + rows: Math.max(5, rows), + cwd: dir, + env + }) + : pty.spawn('/bin/bash', ['-c', wrapper, bin, ...args], { + name: 'xterm-256color', + cols: Math.max(20, cols), + rows: Math.max(5, rows), + cwd: dir, + env + }) // publish BEFORE wiring the handlers: data emitted between spawn and the // assignment would otherwise be dropped by the identity guard below const mine: Session = { pty: p, server: bridge.server, port: bridge.port } @@ -351,20 +478,27 @@ function killSession() { if (!session) return const s = session session = null - // HUP the whole process group (claude + its MCP server children), then - // escalate: a busy tree that shrugs off SIGHUP must not outlive the panel - const pid = s.pty.pid - try { process.kill(-pid, 'SIGHUP') } catch { try { s.pty.kill() } catch { /* dead */ } } - setTimeout(() => { - try { process.kill(-pid, 'SIGKILL') } catch { /* already gone */ } - }, 1500) + if (WIN) { + // node-pty's ConPTY kill enumerates the console's process list and + // terminates every member (claude + its MCP server children), then + // closes the pseudoconsole; there are no process groups to signal + try { s.pty.kill() } catch { /* dead */ } + } else { + // HUP the whole process group (claude + its MCP server children), then + // escalate: a busy tree that shrugs off SIGHUP must not outlive the panel + const pid = s.pty.pid + try { process.kill(-pid, 'SIGHUP') } catch { try { s.pty.kill() } catch { /* dead */ } } + setTimeout(() => { + try { process.kill(-pid, 'SIGKILL') } catch { /* already gone */ } + }, 1500) + } s.server.close() } /** * Open and close run ONE AT A TIME, and every request takes a generation. * - * `spawnSession` is async (config read, `which`, the node-pty import) while a + * `spawnSession` is async (config read, `resolveCommand`, the node-pty import) while a * close is instant, so a close that overtook an in-flight open used to find * `session` still null, do nothing, and let the pending spawn install itself * afterwards — an orphaned claude nobody could reach or kill. React StrictMode