From ad385be0a98c8a333e5b108879ab26b0b91074e5 Mon Sep 17 00:00:00 2001 From: C <983545369@qq.com> Date: Wed, 30 Sep 2026 17:02:10 +0800 Subject: [PATCH 1/2] fix(hooks): render CodeBuddy's Windows hooks in POSIX, not cmd.exe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeBuddy runs a hook's `command` string through Git Bash on Windows — it dropped PowerShell/CMD for hooks in 2.52.4 and forces Git Bash since 2.55.1 — so the cmd-syntax commands added in #637 misbehave there: - `2>nul` is a file redirect for bash, not the null device, so every hook invocation leaves a 0-byte file named `nul` in the hook's working directory. PostToolUse (matcher `*`) fires after every tool call, so it reappears continuously in the project root, and a reserved device name cannot be removed with `del` or Explorer. - `set "PATH=…;%PATH%"` never prepends PATH under bash. - `|| exit /b 0` makes bash exit 2 on a failing dispatch, which CodeBuddy reads as `allowed:false` and would block a UserPromptSubmit with. The project gate is affected the same way: `cd| findstr … >nul` run by Git Bash errors on `findstr`, exits non-zero and leaves another `nul`. Render the POSIX wrapper form for CodeBuddy, which leaves the cmd renderer unused: drop it, and resolve CodeBuddy's shell to Git Bash the way WorkBuddy's resolver already asks for its bundled MSYS sh. Null when Git Bash is absent, which skips injection with the existing warning instead of installing hooks that could never run. Legacy cmd gates stay recognised on read, so entries written by 0.26.0 are replaced on the next pull rather than duplicated. --- docs/windows-hooks.md | 14 +-- docs/windows-hooks.zh-CN.md | 11 +- src/__tests__/hooks-golden.test.ts | 18 +-- src/__tests__/hooks-reconcile-scope.test.ts | 129 ++++++-------------- src/__tests__/hooks-shell-check.test.ts | 72 ++++++++--- src/__tests__/hooks-windows-bash.test.ts | 3 +- src/builtin-hooks.ts | 123 +++---------------- src/bundled-runtime.ts | 82 ++++++++++--- src/hooks.ts | 43 ++++--- 9 files changed, 221 insertions(+), 274 deletions(-) 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..3a5ecaa38 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,7 @@ vi.mock('../utils/logger.js', () => ({ })); import { resolveAnchors, listWorktrees } from '../utils/git.js'; +import { resetBundledRuntimeCache } from '../bundled-runtime.js'; import { CLAUDE_HOOK_OTHER_HOST_SKIP, reconcileTeamHooksForConfig } from '../hooks.js'; import * as gitHook from '../git-hook.js'; import type { LocalConfig, TeamaiConfig } from '../types.js'; @@ -928,8 +928,8 @@ describe('reconcileTeamHooksForConfig — legacy projectRoot sweep', () => { // // 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. +// gate and payload alike — never runs. CodeBuddy's runner is Git Bash, not +// cmd.exe, so every tool gets the POSIX gate now. describe('project gate rendering per host shell', () => { const codebuddyOnly = { toolPaths: { codebuddy: { settings: '.codebuddy/settings.json' } }, @@ -942,6 +942,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 +980,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 +988,13 @@ 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); + // 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 +1017,4 @@ 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 { - await writeYaml(` -hooks: - - id: gate - description: gate probe - event: Stop - command: echo TEAMAI_GATE_PAYLOAD - tools: [codebuddy] -`); - await fse.ensureDir(path.join(sandboxHome, '.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]; - } - - /** 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 ?? '' }; - } - - it.skipIf(process.platform !== 'win32')( - 'fires only inside the project and never executes part of the path', - 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); - } - 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); - } - }, - ); -}); +}); \ 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..c6ff01295 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'; @@ -375,6 +375,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,27 +402,20 @@ 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. + * + * 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 quoted = shellQuote(root); return `if [ "$PWD" = ${quoted} ] || case "$PWD" in ${quoted}/*) true;; *) false;; esac; then (${command}); fi`; } @@ -438,6 +436,11 @@ 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; @@ -557,7 +560,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 +1360,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)); From 3bbe9d2341482cb5ef6ec846862618bc4043a81f Mon Sep 17 00:00:00 2001 From: C <983545369@qq.com> Date: Thu, 8 Oct 2026 09:27:59 +0800 Subject: [PATCH 2/2] fix(hooks): compare the project gate in the shell's path spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate tests `$PWD`, and an MSYS shell spells a drive-letter path `/c/proj`, never the `C:/proj` a Windows root resolves to. A gate rendered in the native spelling only therefore never matched, so every project-scoped team hook silently never ran. Render the root in both spellings: the native one, which is what earlier versions wrote and what the read side keeps recognising, so a re-render replaces those entries instead of stacking beside them, plus the MSYS one the runner's `$PWD` actually holds. Drive roots lose their trailing separator (`C:/` -> `/c`). A root without a drive letter (Linux, macOS) is unchanged and the gate still carries a single test there. Restore the execution coverage this PR had dropped. The cmd.exe gate used to be executed for real by `project gate — real cmd.exe execution (win32)`; replacing that with shape assertions is what let a gate that could never match pass. Run the rendered gate through the runner's shell — from inside and outside the project, over directory names carrying the characters that would split the command if the root were not quoted. Two environment notes on that test: it builds its directories outside `os.tmpdir()`, because MSYS mounts the Windows temp directory at `/tmp` and a root under it is reported as `/tmp/...`, where no native-path gate can match; and it spawns asynchronously, because a sandbox can fail the synchronous spawn with EBUSY, which would show up as an empty stdout — the signature of a gate that never matched. --- src/__tests__/hooks-reconcile-scope.test.ts | 129 +++++++++++++++++++- src/hooks.ts | 44 ++++++- 2 files changed, 165 insertions(+), 8 deletions(-) diff --git a/src/__tests__/hooks-reconcile-scope.test.ts b/src/__tests__/hooks-reconcile-scope.test.ts index 3a5ecaa38..e2ef17770 100644 --- a/src/__tests__/hooks-reconcile-scope.test.ts +++ b/src/__tests__/hooks-reconcile-scope.test.ts @@ -15,6 +15,8 @@ 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. CodeBuddy's runner is Git Bash, not -// cmd.exe, so every tool gets the POSIX gate now. -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; @@ -990,6 +997,16 @@ hooks: const [command] = await teamStopCommands('.codebuddy/settings.json'); 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. @@ -1017,4 +1034,104 @@ hooks: platformSpy.mockRestore(); } }); + + /** + * 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(home, '.codebuddy')); + await reconcileTeamHooksForConfig(codebuddyOnly, { ...localConfig(), projectRoot: root } as LocalConfig); + const [command] = await teamStopCommands('.codebuddy/settings.json'); + expect(command).toBeDefined(); + return command; + } + + /** + * 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(!executor)( + 'fires inside the project and stays an exit-0 no-op outside it, under a real shell', + async () => { + 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); + } + } + } 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/hooks.ts b/src/hooks.ts index c6ff01295..e0319f3f1 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -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. @@ -410,14 +431,26 @@ function cmdProjectGate(root: string): string { * 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): string { if (!projectRoot) return command; const root = canonicalProjectRoot(projectRoot); + 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). */ @@ -447,7 +480,14 @@ function skipWhenAnotherHostLoadsClaudeSettings(command: string, tool: string): 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