diff --git a/docs/windows-hooks.md b/docs/windows-hooks.md index 8a9a1e8e7..740a9f5d4 100644 --- a/docs/windows-hooks.md +++ b/docs/windows-hooks.md @@ -16,8 +16,8 @@ because shell detection (`fs.existsSync('/bin/sh')`) is always false on Windows. Current `teamai` handles Windows itself, so the user-side workaround below is only needed on an older version: hook commands launch through an absolute Git Bash path, and each GUI tool resolves its own hook shell — WorkBuddy through its -bundled PortableGit `sh.exe`, and **CodeBuddy through cmd.exe** (`%ComSpec%`), -which every Windows install provides. Neither tool is skipped. +bundled PortableGit `sh.exe`, and **CodeBuddy through the Git Bash it requires** +on Windows. Neither tool is skipped. The durable user-side fix combines two mechanisms so hooks fire no matter what `teamai` writes: @@ -80,11 +80,11 @@ export function hasShell(): boolean { all** on Windows, even when everything else worked. That skip is gone: gating now asks each tool for its own hook shell first -(`hasShellFor()` → `bundledShellFor()`). `workbuddy` resolves through the -PortableGit `sh.exe` it ships; `codebuddy` resolves through cmd.exe, because -CodeBuddy's Windows hook runner is `%ComSpec%` — it executes a hook's `command` -via `child_process.spawn(command, [], { shell: true })` — and every Windows -install provides cmd.exe. Only a tool with no resolvable shell is skipped. +(`hasShellFor()` → `bundledShellFor()`). Both resolve to a **POSIX** shell — +`workbuddy` to the PortableGit `sh.exe` it ships, `codebuddy` to the Git Bash it +requires on Windows and runs every hook's `command` through — so the rendered +hook commands are POSIX for both. Only a tool with no resolvable shell is +skipped. --- diff --git a/docs/windows-hooks.zh-CN.md b/docs/windows-hooks.zh-CN.md index e35f1aaee..f6bbfd931 100644 --- a/docs/windows-hooks.zh-CN.md +++ b/docs/windows-hooks.zh-CN.md @@ -14,8 +14,8 @@ 当前版本的 `teamai` 已自行处理 Windows，因此下文的用户侧绕行方案仅在旧版本上需要： 钩子命令通过 Git Bash 的绝对路径启动，且每个 GUI 工具都会解析各自的钩子 shell—— -WorkBuddy 使用其自带的 PortableGit `sh.exe`，**CodeBuddy 使用 cmd.exe** -（`%ComSpec%`），任何 Windows 安装都提供 cmd.exe。两者都不再被跳过。 +WorkBuddy 使用其自带的 PortableGit `sh.exe`，**CodeBuddy 使用它在 Windows 上必需 +的 Git Bash**。两者都不再被跳过。 持久化的用户侧修复结合两种机制，无论 `teamai` 写入什么都能让钩子触发： @@ -75,10 +75,9 @@ export function hasShell(): boolean { 该跳过已不再存在：门控会先向每个工具询问其自身的钩子 shell （`hasShellFor()` → `bundledShellFor()`）。`workbuddy` 通过其自带的 PortableGit -`sh.exe` 解析；`codebuddy` 通过 cmd.exe 解析——CodeBuddy 在 Windows 上的钩子运行器 -是 `%ComSpec%`（它通过 `child_process.spawn(command, [], { shell: true })` 执行钩子 -的 `command`），而任何 Windows 安装都提供 cmd.exe。只有无法解析出 shell 的工具才会 -被跳过。 +`sh.exe` 解析；`codebuddy` 解析到它在 Windows 上必需、并用来执行每条钩子 `command` +的 Git Bash。两者都是 **POSIX** shell，因此渲染出的钩子命令一律是 POSIX。只有无法 +解析出 shell 的工具才会被跳过。 --- diff --git a/src/__tests__/hooks-golden.test.ts b/src/__tests__/hooks-golden.test.ts index 80cbaf650..650cd6371 100644 --- a/src/__tests__/hooks-golden.test.ts +++ b/src/__tests__/hooks-golden.test.ts @@ -18,8 +18,9 @@ import { injectHooks } from '../hooks.js'; // the injector names Git Bash by absolute path instead — machine-specific, and // the Linux-captured fixtures cannot match it. Only the dispatch-command // renderers (claude, claude-internal, cursor) skip for that reason; workbuddy -// stays machine-independent and keeps coverage. codebuddy also skips there, but -// for its own reason — see PLATFORM_SPECIFIC_TOOLS below. +// and codebuddy both stay machine-independent and keep coverage, because both +// render the wrapper form (`PATH="$HOME/.teamai/bin:$PATH" teamai …`), which +// never embeds a host path. const fixturesDir = path.resolve(__dirname, 'fixtures', 'hooks'); // true = the command carries the dispatch shell prefix (`getDispatchCommand`). @@ -31,16 +32,6 @@ const cases: Array<[string, string, boolean]> = [ ['workbuddy', 'settings.json', false], ]; -/** - * Tools whose rendered command is platform-specific for a reason other than the - * dispatch shell, so they have no cross-platform baseline: codebuddy is - * rendered in cmd.exe syntax on Windows (its hook runner there is cmd.exe, not - * a POSIX shell — see bundled-runtime.ts), exactly like ZCode, which is absent - * from the fixture set for the same reason. Its Windows shape is pinned by - * hooks-shell-check.test.ts instead, so the anchor stays platform-independent. - */ -const PLATFORM_SPECIFIC_TOOLS = new Set(['codebuddy']); - describe('hooks golden — built-in output is byte-identical to the captured baseline', () => { let tmp: string; beforeEach(async () => { @@ -51,8 +42,7 @@ describe('hooks golden — built-in output is byte-identical to the captured bas }); for (const [tool, file, usesDispatchCommand] of cases) { - const skip = process.platform === 'win32' - && (usesDispatchCommand || PLATFORM_SPECIFIC_TOOLS.has(tool)); + const skip = process.platform === 'win32' && usesDispatchCommand; it.skipIf(skip)(`${tool} output matches golden fixture`, async () => { const p = path.join(tmp, tool, file); await injectHooks(p, tool); diff --git a/src/__tests__/hooks-reconcile-scope.test.ts b/src/__tests__/hooks-reconcile-scope.test.ts index 6dbedeb6c..e2ef17770 100644 --- a/src/__tests__/hooks-reconcile-scope.test.ts +++ b/src/__tests__/hooks-reconcile-scope.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import path from 'node:path'; import os from 'node:os'; -import { spawnSync } from 'node:child_process'; import fse from 'fs-extra'; vi.mock('../utils/git.js', async (importOriginal) => ({ @@ -15,6 +14,9 @@ vi.mock('../utils/logger.js', () => ({ })); import { resolveAnchors, listWorktrees } from '../utils/git.js'; +import { resetBundledRuntimeCache } from '../bundled-runtime.js'; +import { findOnPath } from '../utils/lookpath.js'; +import { spawn } from 'node:child_process'; import { CLAUDE_HOOK_OTHER_HOST_SKIP, reconcileTeamHooksForConfig } from '../hooks.js'; import * as gitHook from '../git-hook.js'; import type { LocalConfig, TeamaiConfig } from '../types.js'; @@ -924,13 +926,18 @@ describe('reconcileTeamHooksForConfig — legacy projectRoot sweep', () => { }); }); -// ── Project gate rendering per host shell ──────────────────── +// ── Project gate: what it renders, and whether it actually fires ─ // -// A tool whose Windows hook runner is cmd.exe cannot execute a POSIX -// `if [ "$PWD" ... ]` gate: cmd aborts on that syntax, so the whole team hook — -// gate and payload alike — never runs. Pin the cmd rendering for those tools and -// the POSIX rendering for everything else. -describe('project gate rendering per host shell', () => { +// Every hook runner is a POSIX shell — CodeBuddy's Git Bash, WorkBuddy's +// bundled PortableGit, `bash -lc` for the rest — so the gate is always the +// POSIX form. A cmd.exe gate would not run there at all: `findstr` errors out +// and the `>nul` redirect leaves a file named `nul` in the project. +// +// On Windows that shell names its cwd `/c/proj`, never the `C:\proj` the root +// resolves to, so the gate carries both spellings. The cases below check the +// shape; the last one runs a rendered gate through a real shell, which is the +// only way a gate that can never match is caught. +describe('project gate — rendering and real-shell execution', () => { const codebuddyOnly = { toolPaths: { codebuddy: { settings: '.codebuddy/settings.json' } }, } as unknown as TeamaiConfig; @@ -942,6 +949,34 @@ describe('project gate rendering per host shell', () => { .map((e: { hooks: Array<{ command: string }> }) => e.hooks[0].command); } + // findGitBashWindows() reads these off the real environment, so a Git + // installed on a non-standard drive (or found only via the HKLM registry key) + // would decide whether these cases run at all. Clear them and stage the one + // candidate under the mocked home instead, so the gate assertions below are + // the same on any developer box and on CI. WorkBuddy resolves its own MSYS + // sh under ~/.workbuddy, which needs no environment at all. + const winEnvKeys = ['ProgramFiles', 'ProgramFiles(x86)', 'LOCALAPPDATA']; + let savedEnv: Record; + + beforeEach(async () => { + savedEnv = {}; + for (const key of winEnvKeys) { + savedEnv[key] = process.env[key]; + delete process.env[key]; + } + resetBundledRuntimeCache(); + await fse.ensureFile(path.join(home, 'AppData', 'Local', 'Programs', 'Git', 'bin', 'bash.exe')); + await fse.ensureFile(path.join(home, '.workbuddy', 'binaries', 'PortableGit', 'versions', '9.9.9', 'bin', 'sh.exe')); + }); + + afterEach(async () => { + for (const key of winEnvKeys) { + if (savedEnv[key] === undefined) delete process.env[key]; + else process.env[key] = savedEnv[key]; + } + resetBundledRuntimeCache(); + }); + const telemetryYaml = (tool: string): string => ` hooks: - id: telemetry @@ -952,7 +987,7 @@ hooks: tools: [${tool}] `; - it('renders a cmd.exe gate for a tool whose Windows hook runner is cmd.exe', async () => { + it('renders the POSIX gate for codebuddy, which runs hooks through Git Bash', async () => { const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32'); try { await writeYaml(telemetryYaml('codebuddy')); @@ -960,17 +995,23 @@ hooks: await reconcileTeamHooksForConfig(codebuddyOnly, localConfig()); const [command] = await teamStopCommands('.codebuddy/settings.json'); - // `cd` prints the cwd into the pipe — never `%CD%` interpolated into a - // parsed command — and the root is caret-escaped inside `^"…^"` quotes. - expect(command.startsWith('cd| findstr /i /b /l /c:^"')).toBe(true); - expect(command).toContain(' >nul || cd| findstr /i /e /l /c:^"'); - // Outside the project the gate must exit 0 (a non-zero status would make - // CodeBuddy treat UserPromptSubmit as allowed:false and block the prompt), - // while the payload's own status is passed through inside it. - expect(command.endsWith('^" >nul & if not errorlevel 1 (python3 .docs/script/inject-telemetry.py) else exit /b 0')).toBe(true); - expect(command).not.toContain('echo %CD%'); - expect(command).not.toContain('&& (python3'); - expect(command).not.toContain('$PWD'); + expect(command.startsWith('if [ "$PWD" = ')).toBe(true); + expect(command.endsWith('); fi')).toBe(true); + // The runner names its cwd the MSYS way: on Windows that is `/c/...`, + // never the `C:\...` this root resolves to, so the gate must carry both + // spellings or it silently never fires. A root without a drive letter is + // already the shell's spelling and the gate carries a single test, which + // is what the suite's Linux and macOS jobs exercise. + if (/^[A-Za-z]:[\\/]/.test(project)) { + expect(command, 'a Windows root is tested in both spellings').toMatch(/\] \|\| \[ "\$PWD" = '\/[a-z]\//); + } else { + expect(command, 'a POSIX root needs one spelling').not.toContain(' || [ "$PWD" = '); + } + // 0.26.0 rendered a cmd.exe gate here. Git Bash cannot run it: `findstr` + // errors out, the gate never matches, and the `>nul` redirect leaves a + // file literally named `nul` in the project. + expect(command).not.toContain('findstr'); + expect(command).not.toContain('exit /b 0'); } finally { platformSpy.mockRestore(); } @@ -993,77 +1034,104 @@ hooks: platformSpy.mockRestore(); } }); -}); -// ── The rendered cmd gate, executed by a real cmd.exe ──────── -// -// The assertions above pin only the shape of the gate. These run it in real -// directories whose names carry the characters cmd.exe re-parses — `&`, which -// otherwise executes the rest of the directory name, plus `^`, `%` and a space -// — because a gate that merely looks right can still run part of a path as a -// command or silently stop matching. Windows-only: cmd.exe is the point. -describe('project gate — real cmd.exe execution (win32)', () => { - const codebuddyOnly = { - toolPaths: { codebuddy: { settings: '.codebuddy/settings.json' } }, - } as unknown as TeamaiConfig; - - /** Render the gate for `root`, then run it from `cwd` through cmd.exe. */ - async function renderGate(root: string, sandboxHome: string): Promise { + /** + * A real POSIX shell to run the gate with. Deliberately not the placeholder + * `bash.exe` staged above: that file only has to make + * `resolveCodebuddyShell()` report a shell and cannot execute anything. This + * is the MSYS bash the machine actually has on PATH, found through the real + * environment the staging did not touch. + */ + const executor = process.platform === 'win32' ? findOnPath('bash') : '/bin/sh'; + + /** + * Where these cases build their directories — deliberately outside + * `os.tmpdir()`. MSYS mounts the Windows temp directory at `/tmp`, so a root + * under it is reported as `/tmp/...` and no gate rendered from its native + * path can ever match it. `node_modules` sits on the workspace drive, which + * is what a real project looks like, and is never committed. + */ + const execBase = path.join(process.cwd(), 'node_modules'); + + /** Render the gate for `root` and read it back off the tool's settings file. */ + async function renderGate(root: string): Promise { + // A fresh file per call: entries for another root are kept by design, and + // this reads back exactly the gate just rendered. + await fse.remove(path.join(home, '.codebuddy', 'settings.json')); await writeYaml(` hooks: - id: gate description: gate probe event: Stop + matcher: "*" command: echo TEAMAI_GATE_PAYLOAD tools: [codebuddy] `); - await fse.ensureDir(path.join(sandboxHome, '.codebuddy')); + await fse.ensureDir(path.join(home, '.codebuddy')); await reconcileTeamHooksForConfig(codebuddyOnly, { ...localConfig(), projectRoot: root } as LocalConfig); - const settings = await fse.readJson(path.join(sandboxHome, '.codebuddy', 'settings.json')); - const commands = (settings.hooks.Stop ?? []) - .filter((e: { description?: string }) => e.description?.startsWith('[teamai:hook:')) - .map((e: { hooks: Array<{ command: string }> }) => e.hooks[0].command); - expect(commands).toHaveLength(1); - return commands[0]; + const [command] = await teamStopCommands('.codebuddy/settings.json'); + expect(command).toBeDefined(); + return command; } - /** Run a rendered hook command the way CodeBuddy's hook runner does. */ - function runCommand(command: string, cwd: string): { status: number | null; stdout: string } { - const result = spawnSync(command, { cwd, shell: true, encoding: 'utf8' }); - return { status: result.status, stdout: result.stdout ?? '' }; + /** + * Run a rendered hook command the way the runner does: through its shell. + * Async rather than `spawnSync` because a sandbox can fail the synchronous + * spawn with EBUSY, which would show up here as an empty stdout — the exact + * signature of a gate that never matched. + */ + function runCommand(command: string, cwd: string): Promise<{ status: number | null; stdout: string }> { + return new Promise((resolve) => { + const child = spawn(executor!, ['-c', command], { cwd }); + let stdout = ''; + child.stdout.on('data', (chunk: Buffer) => { stdout += chunk; }); + child.on('close', (status) => resolve({ status, stdout })); + child.on('error', () => resolve({ status: null, stdout })); + }); } - it.skipIf(process.platform !== 'win32')( - 'fires only inside the project and never executes part of the path', + it.skipIf(!executor)( + 'fires inside the project and stays an exit-0 no-op outside it, under a real shell', async () => { - for (const name of ['plain', 'sp&x', 'a^b', 'a%b', 'a%TEMP%b', 'sp ace', 'x&echo CANARY&y']) { - const root = path.join(project, name); - const sub = path.join(root, 'sub'); - const sibling = path.join(project, `${name}-sibling`); - await fse.ensureDir(sub); - await fse.ensureDir(sibling); - // A fresh HOME per project keeps the shared settings file free of the - // previous iteration's project-scoped entries. - const sandboxHome = path.join(project, 'home', name); - vi.stubEnv('HOME', sandboxHome); - const command = await renderGate(root, sandboxHome); - - for (const cwd of [root, sub]) { - const { status, stdout } = runCommand(command, cwd); - expect(stdout, `${name} inside ${cwd}`).toContain('TEAMAI_GATE_PAYLOAD'); - expect(status, `${name} inside ${cwd}`).toBe(0); + const execRoot = await fse.mkdtemp(path.join(execBase, '.teamai-gate-')); + try { + // `sp&x` and `x&echo CANARY&y` carry the characters that would split the + // command if the gate did not quote the root; `sp ace` covers the space. + for (const name of ['plain', 'sp&x', 'sp ace', 'x&echo CANARY&y']) { + const root = path.join(execRoot, name); + const sub = path.join(root, 'sub'); + const sibling = path.join(execRoot, `${name}-sibling`); + await fse.ensureDir(sub); + await fse.ensureDir(sibling); + const command = await renderGate(root); + + // Inside: the gate matches and the payload runs. A gate rendered in + // the native spelling only would silently never do this. + for (const cwd of [root, sub]) { + const { status, stdout } = await runCommand(command, cwd); + expect(stdout, `${name} inside ${cwd}`).toContain('TEAMAI_GATE_PAYLOAD'); + expect(status, `${name} inside ${cwd}`).toBe(0); + // A `&` in the directory name must never split the gate into + // commands — the payload's own output is the canary for that. + expect(stdout, `${name} injection canary`).not.toMatch(/^\s*CANARY\s*$/m); + } + // Outside: nothing runs, and the gate still exits 0 — CodeBuddy reads + // a non-zero hook status as `allowed:false` and would block every + // prompt typed outside the project. + for (const cwd of [execRoot, sibling]) { + const { status, stdout } = await runCommand(command, cwd); + expect(stdout, `${name} outside ${cwd}`).not.toContain('TEAMAI_GATE_PAYLOAD'); + expect(status, `${name} outside ${cwd}`).toBe(0); + } } - for (const cwd of [project, sibling]) { - const { status, stdout } = runCommand(command, cwd); - expect(stdout, `${name} outside ${cwd}`).not.toContain('TEAMAI_GATE_PAYLOAD'); - // A mismatch must stay an exit-0 no-op: CodeBuddy reads a non-zero - // hook status as allowed:false and would block every prompt typed - // outside the project. - expect(status, `${name} outside ${cwd}`).toBe(0); - } - // `&` in the directory name must never split the gate into commands. - expect(runCommand(command, root).stdout, `${name} injection canary`).not.toMatch(/^\s*CANARY\s*$/m); + } finally { + // Some sandboxes refuse the bulk delete; a leftover empty directory + // under node_modules is harmless. + await fse.remove(execRoot).catch(() => {}); } }, + // A Windows runner starts a fresh MSYS bash per call, which is far slower + // than the cmd.exe the gate used to be tested with. + 60_000, ); -}); +}); \ No newline at end of file diff --git a/src/__tests__/hooks-shell-check.test.ts b/src/__tests__/hooks-shell-check.test.ts index f669c120f..d679cbd43 100644 --- a/src/__tests__/hooks-shell-check.test.ts +++ b/src/__tests__/hooks-shell-check.test.ts @@ -25,6 +25,20 @@ vi.mock('node:fs', async () => { }; }); +// CodeBuddy's shell resolver falls back to the HKLM GitForWindows key, so on a +// developer machine that has Git for Windows installed the "Git Bash absent" +// case is unreachable. Fail the registry probe here and the case behaves the +// same on Windows as it does on ubuntu CI. +vi.mock('node:child_process', async () => { + const actual = await vi.importActual('node:child_process'); + return { + ...actual, + execFileSync: () => { + throw new Error('Git for Windows registry is not available in tests'); + }, + }; +}); + import { hasShell, _resetShellCache } from '../builtin-hooks.js'; import { injectHooksToAllTools } from '../hooks.js'; import { log } from '../utils/logger.js'; @@ -156,25 +170,43 @@ describe('injectHooksToAllTools — workbuddy bundled PortableGit sh (win32)', ( }); }); -describe('injectHooksToAllTools — codebuddy runs hooks through cmd.exe (win32)', () => { +describe('injectHooksToAllTools — codebuddy runs hooks through Git Bash (win32)', () => { let tmp: string; let platformSpy: ReturnType; + // findGitBashWindows() reads these off the real environment; clear them so the + // only candidate is the one under the mocked home, and the case is the same on + // a developer's Windows box as on ubuntu CI. + const winEnvKeys = ['ProgramFiles', 'ProgramFiles(x86)', 'LOCALAPPDATA']; + let savedEnv: Record; + + const fakeGitBash = (home: string): string => + path.join(home, 'AppData', 'Local', 'Programs', 'Git', 'bin', 'bash.exe'); beforeEach(async () => { + tmp = await fse.mkdtemp(path.join(os.tmpdir(), 'cb-sh-')); + homeState.home = tmp; + savedEnv = {}; + for (const key of winEnvKeys) { + savedEnv[key] = process.env[key]; + delete process.env[key]; + } platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32'); _resetShellCache(); - tmp = await fse.mkdtemp(path.join(os.tmpdir(), 'cb-cmd-')); - homeState.home = tmp; vi.mocked(log.warn).mockClear(); }); afterEach(async () => { platformSpy.mockRestore(); + for (const key of winEnvKeys) { + if (savedEnv[key] === undefined) delete process.env[key]; + else process.env[key] = savedEnv[key]; + } await fse.remove(tmp); }); - it('injects codebuddy hooks without /bin/sh, because its hook runner is %ComSpec%', async () => { + it('injects codebuddy hooks in POSIX syntax, without /bin/sh', async () => { shellExists = false; + await fse.ensureFile(fakeGitBash(tmp)); await fse.ensureDir(path.join(tmp, '.codebuddy')); await injectHooksToAllTools({ codebuddy: { settings: '.codebuddy/settings.json' } }, tmp); @@ -182,18 +214,18 @@ describe('injectHooksToAllTools — codebuddy runs hooks through cmd.exe (win32) expect(vi.mocked(log.warn)).not.toHaveBeenCalled(); const settings = await fse.readJson(path.join(tmp, '.codebuddy', 'settings.json')); const command: string = settings.hooks.SessionStart[0].hooks[0].command; - // Must point at the SAME bin dir the wrapper writer resolved through - // getUserHome() (HOME wins over USERPROFILE) — a %USERPROFILE% literal could - // name a different directory and the hook would silently miss the shim. - expect(command).toContain(`set "PATH=${path.join(tmp, '.teamai', 'bin')};%PATH%"`); - expect(command).not.toContain('%USERPROFILE%'); - expect(command).toContain('2>nul || exit /b 0'); - // The POSIX form never runs under cmd.exe (no VAR=value prefix, no /dev/null). - expect(command).not.toContain('/dev/null'); + expect(command).toBe( + 'PATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch session-start --tool codebuddy 2>/dev/null || true', + ); + // The cmd.exe form 0.26.0 wrote made Git Bash create a file literally named + // `nul` in the hook's cwd on every invocation. + expect(command).not.toContain('2>nul'); + expect(command).not.toContain('set "PATH='); }); - it('renders the per-matcher variant in cmd syntax too', async () => { + it('renders the per-matcher variant in POSIX syntax too', async () => { shellExists = false; + await fse.ensureFile(fakeGitBash(tmp)); await fse.ensureDir(path.join(tmp, '.codebuddy')); await injectHooksToAllTools({ codebuddy: { settings: '.codebuddy/settings.json' } }, tmp); @@ -201,6 +233,18 @@ describe('injectHooksToAllTools — codebuddy runs hooks through cmd.exe (win32) const settings = await fse.readJson(path.join(tmp, '.codebuddy', 'settings.json')); const todoWrite = settings.hooks.PostToolUse.find((g: { matcher: string }) => g.matcher === 'TodoWrite'); expect(todoWrite.hooks[0].command).toContain('hook-dispatch post-tool-use --tool codebuddy --matcher TodoWrite'); - expect(todoWrite.hooks[0].command).toContain('2>nul || exit /b 0'); + expect(todoWrite.hooks[0].command).toContain('2>/dev/null || true'); + }); + + it('skips codebuddy hook injection and warns when Git Bash is absent', async () => { + shellExists = false; + await fse.ensureDir(path.join(tmp, '.codebuddy')); + + await injectHooksToAllTools({ codebuddy: { settings: '.codebuddy/settings.json' } }, tmp); + + expect(vi.mocked(log.warn)).toHaveBeenCalledWith( + expect.stringContaining('Skipping hook injection for codebuddy'), + ); + expect(await fse.pathExists(path.join(tmp, '.codebuddy', 'settings.json'))).toBe(false); }); }); diff --git a/src/__tests__/hooks-windows-bash.test.ts b/src/__tests__/hooks-windows-bash.test.ts index 9adf58115..345578316 100644 --- a/src/__tests__/hooks-windows-bash.test.ts +++ b/src/__tests__/hooks-windows-bash.test.ts @@ -18,7 +18,8 @@ vi.mock('node:child_process', async (importOriginal) => { return { ...actual, default: actual, execFileSync: boom }; }); -import { getDispatchCommand, findGitBashWindows, _resetShellCache } from '../builtin-hooks.js'; +import { getDispatchCommand, _resetShellCache } from '../builtin-hooks.js'; +import { findGitBashWindows } from '../bundled-runtime.js'; // Windows resolves a bare `bash` to System32's WSL launcher before any PATH // entry, and the WSL side has a different $HOME and no npm-global teamai — the diff --git a/src/builtin-hooks.ts b/src/builtin-hooks.ts index 897874618..13b745d15 100644 --- a/src/builtin-hooks.ts +++ b/src/builtin-hooks.ts @@ -1,13 +1,12 @@ import fs from 'node:fs'; import path from 'node:path'; -import { execFileSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; import { TEAMAI_HOOK_DESCRIPTION_PREFIX } from './types.js'; import { CODEX_TOOL_IDS, isCodexTool } from './utils/tool-names.js'; import type { HookDef } from './types.js'; import { getUserHome } from './utils/home.js'; import { log } from './utils/logger.js'; -import { bundledShellFor, resetBundledRuntimeCache, resolveCodebuddyNode, resolveWorkbuddyNode } from './bundled-runtime.js'; +import { bundledShellFor, findGitBashWindows, resetBundledRuntimeCache, resolveCodebuddyNode, resolveWorkbuddyNode } from './bundled-runtime.js'; // ─── Built-in (A) operational hooks as data ───────────────── // @@ -25,13 +24,11 @@ import { bundledShellFor, resetBundledRuntimeCache, resolveCodebuddyNode, resolv // WorkBuddy and CodeBuddy use bundled Node runtimes and their hook // subprocesses may lack the user's PATH, so `teamai` is not found. // We write a thin wrapper at `~/.teamai/bin/teamai` — plus a `teamai.cmd` -// next to it on Windows, because cmd.exe cannot execute the extensionless sh -// script — that invokes the real entry script with the best available Node, -// then prepend `~/.teamai/bin` to PATH in hook commands for WorkBuddy and -// CodeBuddy. The POSIX PATH is expressed as `$HOME/.teamai/bin` (shell -// literal) so that the golden fixture output is stable across machines; the -// cmd.exe variant embeds the same bin dir resolved through getUserHome(), the -// resolver the wrapper writer uses, so write and lookup cannot diverge. +// next to it on Windows, so a cmd.exe host can still launch the CLI — that +// invokes the real entry script with the best available Node, then prepend +// `~/.teamai/bin` to PATH in hook commands for WorkBuddy and CodeBuddy. +// The PATH is expressed as `$HOME/.teamai/bin` (a shell literal) so that the +// golden fixture output stays stable across machines. // Other tools keep the plain `bash -lc "teamai ..."` form. export const TEAMAI_BIN_DIR = '.teamai/bin'; @@ -46,29 +43,14 @@ const WRAPPER_NAME = 'teamai'; */ export const SHELL_DEPENDENT_TOOLS = new Set(['workbuddy', 'codebuddy']); -/** - * Shell-dependent tools whose Windows hook runner is cmd.exe rather than a - * POSIX shell, so their rendered command must be cmd syntax. WorkBuddy is NOT - * here: its Windows hook runner is the MSYS shell from its bundled - * PortableGit, which executes the POSIX wrapper form. - */ -const WINDOWS_CMD_TOOLS = new Set(['codebuddy']); - -/** - * True when the tool's hook runner on this platform is cmd.exe, so every - * command rendered for it — built-in dispatch and team-hook project gate alike - * — must be cmd syntax. - */ -export function toolUsesCmdShell(tool: string): boolean { - return process.platform === 'win32' && WINDOWS_CMD_TOOLS.has(tool); -} - /** * Check whether /bin/sh exists. Remote containers (e.g. CloudStudio AI * inference nodes) may lack it, causing a POSIX hook runner's * `spawn('/bin/sh', ['-c', command])` to fail with ENOENT on every hook - * invocation. Tools whose runner is cmd.exe on Windows are exempt — they are - * covered by their bundledShellFor entry instead. Exported so the injection + * invocation. Shell-dependent tools are exempt — each is covered by its own + * bundledShellFor entry instead, because the POSIX shell it runs hooks through + * is not reachable from this process's namespace (WorkBuddy's bundled MSYS sh, + * CodeBuddy's Git Bash, both on Windows). Exported so the injection * entry points can skip hook installation and warn the user. */ let _hasShellCache: boolean | undefined; @@ -199,57 +181,6 @@ export function skipToolsWithoutShell(tools: string[]): Set { return skipped; } -/** - * Read the machine-wide InstallPath the Git for Windows installer records - * in HKLM. Exported so tests stub it at the module boundary instead of - * shelling out to a real reg.exe. Returns null on any failure — an - * unreadable registry just means "no extra candidate". - */ -export function queryGitInstallPath(): string | null { - try { - const out = execFileSync( - 'reg.exe', - ['query', 'HKLM\\SOFTWARE\\GitForWindows', '/v', 'InstallPath'], - { timeout: 5000, windowsHide: true, encoding: 'utf8' }, - ); - const match = out.match(/InstallPath\s+REG_SZ\s+(.+)/); - return match?.[1]?.trim() || null; - } catch { - return null; - } -} - -/** - * Locate Git Bash on Windows. CreateProcess resolves a bare `bash` to - * System32's WSL launcher before any PATH entry (the rationale already - * documented for ZCode in hooks.ts), and the ZCode cmd fallback is not - * available to rendered shell-string commands, so on Windows the - * interpreter has to be an absolute path. Standard install locations - * first; the HKLM `GitForWindows` key covers custom InstallPath. - * Returns the exe path, or null when Git is not found. - */ -export function findGitBashWindows( - env: NodeJS.ProcessEnv = process.env, - home: string = getUserHome(), - readInstallPath: () => string | null = queryGitInstallPath, -): string | null { - const candidates: string[] = []; - if (env.ProgramFiles) candidates.push(path.join(env.ProgramFiles, 'Git', 'bin', 'bash.exe')); - if (env['ProgramFiles(x86)']) candidates.push(path.join(env['ProgramFiles(x86)'], 'Git', 'bin', 'bash.exe')); - if (env.LOCALAPPDATA) candidates.push(path.join(env.LOCALAPPDATA, 'Programs', 'Git', 'bin', 'bash.exe')); - candidates.push(path.join(home, 'AppData', 'Local', 'Programs', 'Git', 'bin', 'bash.exe')); - for (const candidate of candidates) { - if (fs.existsSync(candidate)) return candidate; - } - - const installPath = readInstallPath(); - if (installPath) { - const candidate = path.join(installPath, 'bin', 'bash.exe'); - if (fs.existsSync(candidate)) return candidate; - } - return null; -} - let _winBashLauncherCache: string | undefined; /** @@ -304,32 +235,6 @@ function getWrapperDispatchCommand(event: string, tool: string, matcher?: string return `PATH="$HOME/${TEAMAI_BIN_DIR}:$PATH" teamai hook-dispatch ${event} --tool ${tool}${matcherArg} 2>/dev/null || true`; } -/** - * cmd.exe counterpart of getWrapperDispatchCommand, for tools whose Windows - * hook runner is cmd.exe rather than a POSIX shell. cmd.exe has no - * `VAR=value command` prefix, no /dev/null and no `|| true`, so the POSIX form - * above can never run there — it fails on its first token, whose `PATH=...` - * assignment cmd reads as a command name. Emit the cmd equivalent: prepend the - * wrapper dir to PATH (cmd resolves `teamai` to `teamai.cmd` through PATHEXT, - * falling through to the npm shim further down PATH) and force exit 0 on - * failure, mirroring the POSIX `|| true` — CodeBuddy reads a non-zero hook - * status as `allowed:false`, which would BLOCK a UserPromptSubmit instead of - * failing open. - * - * The PATH value is the bin dir resolved through getUserHome() — the SAME - * resolver ensureTeamaiWrapper() writes `teamai.cmd` through — embedded as a - * concrete path. A `%USERPROFILE%` literal here would disagree with the writer - * whenever HOME wins (Git Bash, a custom environment) or USERPROFILE is absent: - * the shim would land in one directory while the hook searched another, and the - * hook would silently fail to find the CLI. The POSIX form keeps its `$HOME` - * literal because getUserHome() prefers HOME and the shell expands it. - */ -function getCmdWrapperDispatchCommand(event: string, tool: string, matcher?: string): string { - const matcherArg = matcher && matcher !== '*' ? ` --matcher ${matcher}` : ''; - const binDir = path.join(getUserHome(), TEAMAI_BIN_DIR); - return `set "PATH=${binDir};%PATH%" && teamai hook-dispatch ${event} --tool ${tool}${matcherArg} 2>nul || exit /b 0`; -} - /** Canonical, ordered description of each built-in hook. Order is load-bearing * for byte-compat (it fixes array order within each event). */ interface BuiltinHookSpec { @@ -409,8 +314,10 @@ const SUBAGENT_STOP_TOOLS = new Set([ * same HookDef into each tool's on-disk shape. * * GUI tools (WorkBuddy, CodeBuddy) use the wrapper dispatch command so their - * hook subprocesses can find `teamai` even without the user's full PATH. On - * Windows the tools in WINDOWS_CMD_TOOLS get the cmd.exe syntax variant. + * hook subprocesses can find `teamai` even without the user's full PATH. Both + * run hook commands through a POSIX shell on every platform — WorkBuddy + * through the MSYS sh in its bundled PortableGit, CodeBuddy through Git Bash, + * which it requires on Windows — so the POSIX wrapper form always applies. */ const WRAPPER_TOOLS = SHELL_DEPENDENT_TOOLS; @@ -422,7 +329,7 @@ export function builtinHookDefs(tool: string): HookDef[] { const buildCommand = tool === 'zcode' ? getRawDispatchCommand : WRAPPER_TOOLS.has(tool) - ? (toolUsesCmdShell(tool) ? getCmdWrapperDispatchCommand : getWrapperDispatchCommand) + ? getWrapperDispatchCommand : getDispatchCommand; const specs = [ ...BUILTIN_HOOK_SPECS, diff --git a/src/bundled-runtime.ts b/src/bundled-runtime.ts index 584bfe9c1..c7fe8050b 100644 --- a/src/bundled-runtime.ts +++ b/src/bundled-runtime.ts @@ -1,11 +1,12 @@ // Bundled-runtime resolution: where GUI tools (WorkBuddy, CodeBuddy) ship // their own Node and shell, and which of them provide a shell their hook -// commands can execute with — either a POSIX shell they bundle themselves -// (WorkBuddy's PortableGit) or one the OS guarantees (CodeBuddy's cmd.exe on -// Windows). All layout knowledge for these runtimes lives here so hook -// injection can stay tool-agnostic. +// commands can execute with. Both provide a POSIX shell on Windows — WorkBuddy +// the MSYS sh in its bundled PortableGit, CodeBuddy the Git Bash it requires — +// so every hook command teamai renders is POSIX. All layout knowledge for +// these runtimes lives here so hook injection can stay tool-agnostic. import fs from 'node:fs'; import path from 'node:path'; +import { execFileSync } from 'node:child_process'; import { getUserHome } from './utils/home.js'; import { isOnPath, pathDirs } from './utils/lookpath.js'; import { log } from './utils/logger.js'; @@ -126,21 +127,72 @@ function resolveWorkbuddyShell(): string | null { } /** - * The shell CodeBuddy runs hook commands with on Windows. + * Read the machine-wide InstallPath the Git for Windows installer records + * in HKLM. Exported so tests stub it at the module boundary instead of + * shelling out to a real reg.exe. Returns null on any failure — an + * unreadable registry just means "no extra candidate". + */ +export function queryGitInstallPath(): string | null { + try { + const out = execFileSync( + 'reg.exe', + ['query', 'HKLM\\SOFTWARE\\GitForWindows', '/v', 'InstallPath'], + { timeout: 5000, windowsHide: true, encoding: 'utf8' }, + ); + const match = out.match(/InstallPath\s+REG_SZ\s+(.+)/); + return match?.[1]?.trim() || null; + } catch { + return null; + } +} + +/** + * Locate Git Bash on Windows. CreateProcess resolves a bare `bash` to + * System32's WSL launcher before any PATH entry (the rationale already + * documented for ZCode in hooks.ts), and the ZCode cmd fallback is not + * available to rendered shell-string commands, so on Windows the + * interpreter has to be an absolute path. Standard install locations + * first; the HKLM `GitForWindows` key covers custom InstallPath. + * Returns the exe path, or null when Git is not found. + */ +export function findGitBashWindows( + env: NodeJS.ProcessEnv = process.env, + home: string = getUserHome(), + readInstallPath: () => string | null = queryGitInstallPath, +): string | null { + const candidates: string[] = []; + if (env.ProgramFiles) candidates.push(path.join(env.ProgramFiles, 'Git', 'bin', 'bash.exe')); + if (env['ProgramFiles(x86)']) candidates.push(path.join(env['ProgramFiles(x86)'], 'Git', 'bin', 'bash.exe')); + if (env.LOCALAPPDATA) candidates.push(path.join(env.LOCALAPPDATA, 'Programs', 'Git', 'bin', 'bash.exe')); + candidates.push(path.join(home, 'AppData', 'Local', 'Programs', 'Git', 'bin', 'bash.exe')); + for (const candidate of candidates) { + if (fs.existsSync(candidate)) return candidate; + } + + const installPath = readInstallPath(); + if (installPath) { + const candidate = path.join(installPath, 'bin', 'bash.exe'); + if (fs.existsSync(candidate)) return candidate; + } + return null; +} + +/** + * The shell CodeBuddy runs hook commands with on Windows: Git Bash. * - * CodeBuddy's hook runner executes a hook's `command` string through - * `child_process.spawn(command, [], { shell: true })` (genie's - * HookExecutorImpl), which on Windows goes through %ComSpec% — cmd.exe, a shell - * the OS always provides — and NOT /bin/sh. Windows builds of the CodeBuddy IDE - * ship no POSIX shell at all (no sh.exe/bash.exe anywhere in the install tree), - * so gating the tool on /bin/sh is a false negative there. POSIX builds keep - * the conservative /bin/sh check. Memoized like its WorkBuddy sibling. + * CodeBuddy requires Git for Windows there — without it the CLI refuses to run + * hook commands and tells the user to install it — and it runs each hook's + * `command` string through that Git Bash, not cmd.exe. The rendered command + * must therefore be POSIX. This resolver answers the same question WorkBuddy's + * does: "does the tool provide a shell its hook commands can execute with?" + * Null when Git Bash is absent, which correctly skips injection (the hooks + * could never run) rather than gating the tool on /bin/sh — a false negative on + * Windows, because the CLI's own Node build has no /bin/sh. Memoized like its + * WorkBuddy sibling. */ function resolveCodebuddyShell(): string | null { if (_cbShellCache === undefined) { - _cbShellCache = process.platform === 'win32' - ? (process.env.ComSpec?.trim() || 'cmd.exe') - : null; + _cbShellCache = process.platform === 'win32' ? findGitBashWindows() : null; } return _cbShellCache; } diff --git a/src/hooks.ts b/src/hooks.ts index aa4dd69b0..e0319f3f1 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -30,7 +30,7 @@ import { managedMcpWorkspaceId, resolveToolRootDir, } from './types.js'; -import { builtinHookDefs, applyBuiltinOverride, getRawDispatchCommand, skipToolsWithoutShell, toolUsesCmdShell } from './builtin-hooks.js'; +import { builtinHookDefs, applyBuiltinOverride, getRawDispatchCommand, skipToolsWithoutShell } from './builtin-hooks.js'; import type { BuiltinHookOverride } from './builtin-hooks.js'; import { resolveTeamHooks } from './resources/hooks.js'; import { getUserHome } from './utils/home.js'; @@ -354,6 +354,27 @@ function canonicalProjectRoot(projectRoot: string): string { try { return realpathSync.native(projectRoot); } catch { return path.resolve(projectRoot); } } +/** + * The same project root as the shell that runs the hook names its cwd. + * + * On Windows every hook runner is an MSYS shell — CodeBuddy's required Git + * Bash, WorkBuddy's bundled PortableGit — and MSYS exposes a drive-letter path + * as `/c/proj`, never as the `C:\proj` a Windows path resolves to. A gate that + * compares `$PWD` against the Windows spelling therefore never matches, and the + * project's team hooks silently never run: `canonicalProjectRoot()` returns the + * native form, so the comparison has to be made in the form the shell uses. + * + * Drive roots lose their trailing separator (`C:\` → `/c`), which is how MSYS + * spells them. A path that already has no drive letter (Linux, macOS) is + * returned unchanged, and so is a UNC path (`\\server\share`), whose MSYS form + * this does not attempt. + */ +function msysProjectRoot(root: string): string { + const drive = /^([A-Za-z]):[\\/]*(.*)$/.exec(root); + if (!drive) return root; + return `/${drive[1].toLowerCase()}/${drive[2].replace(/\\/g, '/')}`.replace(/\/+$/, '') || '/'; +} + /** * Embed a Windows path in a cmd.exe command line so the child receives it * byte-for-byte. @@ -375,6 +396,11 @@ function cmdLiteral(value: string): string { * cmd.exe equivalent of the POSIX project gate, as a prefix that resolves to * true only inside `root`. * + * Read-only: this gate is only ever matched against (isGatedForProject), never + * rendered. 0.26.0 wrote it for CodeBuddy on Windows; those entries are still + * on disk, and recognising them is what lets a re-render replace the gate + * instead of stacking a second one. + * * The cwd is read with a bare `cd`, whose output goes straight into the pipe: * unlike `echo %CD%`, the directory name is never part of a parsed command, so * `&`, `%` and `^` in it cannot be re-interpreted. `cd` prints no trailing @@ -397,29 +423,34 @@ function cmdProjectGate(root: string): string { /** * Keep a project-scope team hook from firing in every project on the machine. - * The gate is rendered in the syntax of the shell that will actually run it: - * cmd.exe for tools whose Windows hook runner is cmd.exe — a POSIX - * `if [ "$PWD" ... ]` there is a syntax error that kills the whole command, - * gate and payload alike, before it ever runs — and POSIX sh for every other - * tool. * - * Exit-status contract, identical for both renderings: outside the project the - * gate is a no-op that exits 0, and inside it the command's own status is - * passed through. A gate mismatch that returned non-zero would make CodeBuddy - * read the hook as `allowed:false` and BLOCK every UserPromptSubmit outside the - * project, so the cmd form must not inherit `findstr`'s failure status. That is - * also why the cmd form is not `${gate} || exit /b 0 && (…)`: the `||` would - * swallow a genuine payload failure along with the mismatch, losing the - * pass-through the POSIX `if …; then …; fi` gives for free. + * Every tool runs hook commands through a POSIX shell on every platform + * (WorkBuddy's bundled MSYS sh, CodeBuddy's required Git Bash, `bash -lc` for + * the rest), so the gate is always the POSIX form. Outside the project it is a + * no-op that exits 0 and inside it the payload's own status is passed through: + * a gate mismatch that returned non-zero would make CodeBuddy read the hook as + * `allowed:false` and BLOCK every UserPromptSubmit outside the project. + * + * A Windows root is tested in both spellings. The shell names its cwd the MSYS + * way (`/c/proj`); the native form is kept because it is what previous versions + * wrote, so a re-render recognises and replaces those entries instead of + * stacking a second gate beside them. On a non-Windows root the two forms are + * identical and the gate carries one test. + * + * Legacy cmd.exe gates are still recognised on read by isGatedForProject(), so + * a re-render replaces 0.26.0's cmd gate instead of stacking a second one. */ -function gateTeamHookCommand(command: string, projectRoot: string | undefined, tool: string): string { +function gateTeamHookCommand(command: string, projectRoot: string | undefined): string { if (!projectRoot) return command; const root = canonicalProjectRoot(projectRoot); - if (toolUsesCmdShell(tool)) { - return `${cmdProjectGate(root)} & if not errorlevel 1 (${command}) else exit /b 0`; - } + const msys = msysProjectRoot(root); const quoted = shellQuote(root); - return `if [ "$PWD" = ${quoted} ] || case "$PWD" in ${quoted}/*) true;; *) false;; esac; then (${command}); fi`; + if (msys === root) { + return `if [ "$PWD" = ${quoted} ] || case "$PWD" in ${quoted}/*) true;; *) false;; esac; then (${command}); fi`; + } + const shellQuoted = shellQuote(msys); + return `if [ "$PWD" = ${quoted} ] || [ "$PWD" = ${shellQuoted} ]` + + ` || case "$PWD" in ${quoted}/*|${shellQuoted}/*) true;; *) false;; esac; then (${command}); fi`; } /** Recognise a project gate written by either renderer (entries outlive a platform switch). */ @@ -438,13 +469,25 @@ function isProjectGatedCommand(command: string): boolean { * and that host's own teamai hooks are installed. The prefix checks the hook * file; it does not skip a claude-only setup. Built-in hooks take the same * exit inside hook-dispatch instead of here. + * + * `tool` here keys the cursor/copilot wrapper, which is orthogonal to the + * project gate: the gate lost its per-tool form once every render became POSIX + * (CodeBuddy's Windows hooks run through Git Bash), but this wrapper still + * needs to know which tool it is rendering for. */ function skipWhenAnotherHostLoadsClaudeSettings(command: string, tool: string): string { if (tool !== 'claude') return command; return `${CLAUDE_HOOK_OTHER_HOST_SKIP}${command}`; } -const POSIX_GATE_ROOT_RE = /^if \[ "\$PWD" = ('(?:[^']|'"'"')*') \] \|\| case "\$PWD" in /; +/** + * Matches a POSIX gate and captures its *first* quoted root, which is always + * the native spelling: on Windows the MSYS spelling follows as an optional + * second test, on a non-Windows root the two are the same and only one is + * written. Reading the native form back keeps the root comparable with the + * Windows paths the callers hold. + */ +const POSIX_GATE_ROOT_RE = /^if \[ "\$PWD" = ('(?:[^']|'"'"')*') \](?: \|\| \[ "\$PWD" = '(?:[^']|'"'"')*' \])? \|\| case "\$PWD" in /; /** * The project root a POSIX gate was rendered for, or null for an ungated or @@ -557,7 +600,7 @@ function scopedTeamDefs(teamDefs: HookDef[], projectRoot: string | undefined, to command: skipWhenAnotherHostLoadsClaudeSettings(def.command, tool), })); if (!projectRoot) return prepared; - return prepared.map((def) => ({ ...def, command: gateTeamHookCommand(def.command, projectRoot, tool) })); + return prepared.map((def) => ({ ...def, command: gateTeamHookCommand(def.command, projectRoot) })); } function manifestRecordsForTool(teamDefs: HookDef[], tool: string, removeAll: boolean, projectRoot?: string): ManagedHookRecord[] { @@ -1357,7 +1400,7 @@ export async function reconcileHooks( const priorRecords = opts.teamHookProjectRoot ? allPriorRecords.filter((r) => isGatedForProject(r.command, opts.teamHookProjectRoot!)) : allPriorRecords; - const scopedDefs = scopedTeamDefs(teamDefs, opts.teamHookProjectRoot, tool); +const scopedDefs = scopedTeamDefs(teamDefs, opts.teamHookProjectRoot, tool); const desiredTeamCommands = new Set(scopedDefs.filter((d) => !d.tools || d.tools.includes(tool)).map((d) => d.command)); const priorTeamCommands = new Set(priorRecords.map((r) => r.command));