From ac2e2f7a0ad4d8a02433f7509f2173216b17d5ac Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sun, 15 Feb 2026 19:49:16 +0000 Subject: [PATCH 1/2] fix: resolve git root for init paths and fix Windows test compat (GIT-108) Init commands used process.cwd() which fails when the shell cwd differs from the repo root (e.g. JetBrains spawning a new terminal). Now uses git rev-parse --show-toplevel with path.resolve() for cross-platform normalization. Also fixes integration test helpers to use .cmd wrappers and shell:true on Windows for child process spawning. Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 --- src/commands/hook.ts | 5 +++-- src/commands/init-hooks.ts | 27 ++++++++++++++++++++++----- src/commands/init.ts | 22 +++++++++++++++++++--- src/infrastructure/logging/factory.ts | 3 ++- tests/integration/hooks/helpers.ts | 9 ++++++++- tests/integration/mcp/helpers.ts | 11 +++++++++-- tests/unit/hooks/utils/config.test.ts | 6 +++--- 7 files changed, 66 insertions(+), 17 deletions(-) diff --git a/src/commands/hook.ts b/src/commands/hook.ts index 25d3e2f4..d259905f 100644 --- a/src/commands/hook.ts +++ b/src/commands/hook.ts @@ -10,7 +10,7 @@ * git-mem hook prompt-submit */ -import { join } from 'path'; +import { join, resolve } from 'path'; import { execFileSync } from 'child_process'; import { config as loadEnv } from 'dotenv'; import { createContainer } from '../infrastructure/di'; @@ -109,11 +109,12 @@ export function buildEvent(eventType: HookEventType, input: IHookInput): HookEve */ function findGitRoot(cwd: string): string { try { - return execFileSync('git', ['rev-parse', '--show-toplevel'], { + const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { cwd, encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], }).trim(); + return resolve(root); } catch { return cwd; } diff --git a/src/commands/init-hooks.ts b/src/commands/init-hooks.ts index 58a43074..7444cc54 100644 --- a/src/commands/init-hooks.ts +++ b/src/commands/init-hooks.ts @@ -10,12 +10,29 @@ */ import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync, rmSync } from 'fs'; -import { join } from 'path'; +import { execFileSync } from 'child_process'; +import { join, resolve } from 'path'; import { homedir } from 'os'; import { parse as parseYaml, stringify as stringifyYaml } from 'yaml'; import type { ILogger } from '../domain/interfaces/ILogger'; import { getConfigPath, getConfigDir } from '../hooks/utils/config'; +/** + * Resolve the git repository root directory. + * Falls back to process.cwd() if not inside a git repo. + */ +function resolveGitRoot(): string { + try { + const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + return resolve(root); + } catch { + return process.cwd(); + } +} + interface IInitHooksOptions { yes?: boolean; scope?: string; @@ -147,11 +164,11 @@ export function deepMergeGitMemConfig( // ── Config builders ────────────────────────────────────────────────── -export function getSettingsPath(scope: string): string { +export function getSettingsPath(scope: string, cwd?: string): string { if (scope === 'user') { return join(homedir(), '.claude', 'settings.json'); } - return join(process.cwd(), '.claude', 'settings.json'); + return join(cwd ?? process.cwd(), '.claude', 'settings.json'); } export function readExistingSettings(path: string): Record { @@ -232,8 +249,8 @@ export function buildGitMemConfig(): Record { export async function initHooksCommand(options: IInitHooksOptions, logger?: ILogger): Promise { const log = logger?.child({ command: 'init-hooks' }); const scope = options.scope ?? 'project'; - const settingsPath = getSettingsPath(scope); - const cwd = process.cwd(); + const cwd = resolveGitRoot(); + const settingsPath = getSettingsPath(scope, cwd); const configDir = getConfigDir(cwd); const configPath = getConfigPath(cwd); diff --git a/src/commands/init.ts b/src/commands/init.ts index 69f8d93d..3dc43957 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -7,7 +7,7 @@ import { existsSync, readFileSync, writeFileSync, appendFileSync, mkdirSync } from 'fs'; import { execFileSync } from 'child_process'; -import { join } from 'path'; +import { join, resolve } from 'path'; import prompts from 'prompts'; import { parse as parseYaml, stringify as stringifyYaml } from 'yaml'; import type { ILogger } from '../domain/interfaces/ILogger'; @@ -27,6 +27,22 @@ import { createContainer } from '../infrastructure/di'; import { createStderrProgressHandler } from './progress'; import { getConfigPath, getConfigDir } from '../hooks/utils/config'; +/** + * Resolve the git repository root directory. + * Falls back to process.cwd() if not inside a git repo. + */ +function resolveGitRoot(): string { + try { + const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + return resolve(root); + } catch { + return process.cwd(); + } +} + interface IInitCommandOptions { yes?: boolean; uninstallHooks?: boolean; @@ -161,7 +177,7 @@ export function ensureEnvPlaceholder(cwd: string): void { /** Run unified project setup: hooks, MCP config, .gitignore, and .env. */ export async function initCommand(options: IInitCommandOptions, logger?: ILogger): Promise { const log = logger?.child({ command: 'init' }); - const cwd = process.cwd(); + const cwd = resolveGitRoot(); log?.info('Command invoked', { yes: options.yes, uninstallHooks: options.uninstallHooks }); @@ -235,7 +251,7 @@ export async function initCommand(options: IInitCommandOptions, logger?: ILogger // ── Claude Code hooks ────────────────────────────────────────── if (claudeIntegration) { - const settingsPath = getSettingsPath('project'); + const settingsPath = getSettingsPath('project', cwd); const settingsDir = join(settingsPath, '..'); if (!existsSync(settingsDir)) { mkdirSync(settingsDir, { recursive: true }); diff --git a/src/infrastructure/logging/factory.ts b/src/infrastructure/logging/factory.ts index 41a56a2d..7f7d5e4b 100644 --- a/src/infrastructure/logging/factory.ts +++ b/src/infrastructure/logging/factory.ts @@ -12,10 +12,11 @@ function isValidLogLevel(value: string): value is LogLevel { function getGitRoot(): string | null { try { - return execFileSync('git', ['rev-parse', '--show-toplevel'], { + const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], }).trim(); + return path.resolve(root); } catch { return null; } diff --git a/tests/integration/hooks/helpers.ts b/tests/integration/hooks/helpers.ts index a6999099..364df799 100644 --- a/tests/integration/hooks/helpers.ts +++ b/tests/integration/hooks/helpers.ts @@ -16,7 +16,12 @@ const PROJECT_ROOT = resolve(__dirname, '../../..'); const CLI_PATH = resolve(PROJECT_ROOT, 'src/cli.ts'); // Use tsx binary from project node_modules — works even when cwd is a temp dir -const TSX_BIN = resolve(PROJECT_ROOT, 'node_modules/.bin/tsx'); +// On Windows, spawnSync() needs the .cmd wrapper; on Unix, use the shell script +const TSX_BIN = resolve( + PROJECT_ROOT, + 'node_modules/.bin', + process.platform === 'win32' ? 'tsx.cmd' : 'tsx', +); export interface IRunResult { stdout: string; @@ -30,6 +35,7 @@ export function runHook(eventName: string, input: Record): IRun input: JSON.stringify(input), encoding: 'utf8', timeout: 15_000, + shell: process.platform === 'win32', }); return { @@ -46,6 +52,7 @@ export function runCli(args: string[], opts?: { cwd?: string; input?: string }): cwd: opts?.cwd, encoding: 'utf8', timeout: 15_000, + shell: process.platform === 'win32', }); return { diff --git a/tests/integration/mcp/helpers.ts b/tests/integration/mcp/helpers.ts index 4b5e7c2a..e1246334 100644 --- a/tests/integration/mcp/helpers.ts +++ b/tests/integration/mcp/helpers.ts @@ -36,6 +36,7 @@ export function sendMcpRequest(request: object): Promise { return new Promise((resolve, reject) => { const proc = spawn(TSX_BIN, [SERVER_PATH], { stdio: ['pipe', 'pipe', 'pipe'], + shell: process.platform === 'win32', }); let stdout = ''; @@ -90,6 +91,7 @@ export function mcpSession(cwd: string, requests: object[]): Promise { it('should return path to .git-mem/.git-mem.yaml', () => { const testDir = '/some/dir'; const result = getConfigPath(testDir); - assert.equal(result, '/some/dir/.git-mem/.git-mem.yaml'); + assert.equal(result, join('/some/dir', '.git-mem', '.git-mem.yaml')); }); it('should use process.cwd() when no cwd provided', () => { const result = getConfigPath(); - assert.ok(result.endsWith('.git-mem/.git-mem.yaml')); + assert.ok(result.endsWith(join('.git-mem', '.git-mem.yaml'))); }); }); @@ -54,7 +54,7 @@ describe('config', () => { it('should return path to .git-mem directory', () => { const testDir = '/some/dir'; const result = getConfigDir(testDir); - assert.equal(result, '/some/dir/.git-mem'); + assert.equal(result, join('/some/dir', '.git-mem')); }); it('should use process.cwd() when no cwd provided', () => { From 8f2124c2f266127254bba7e2868fc8f2c2f68d70 Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sun, 15 Feb 2026 20:07:07 +0000 Subject: [PATCH 2/2] refactor: extract resolveGitRoot to shared utility and scope cleanup catch (GIT-108) Addresses review feedback: deduplicate resolveGitRoot/getGitRoot into src/infrastructure/git/resolveGitRoot.ts shared by init, init-hooks, hook, and logging factory. Scope MCP cleanup catch to only suppress EPERM on Windows, rethrowing other errors on macOS/Linux. Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 --- src/commands/hook.ts | 15 ++-------- src/commands/init-hooks.ts | 20 ++----------- src/commands/init.ts | 19 ++---------- src/infrastructure/git/resolveGitRoot.ts | 38 ++++++++++++++++++++++++ src/infrastructure/logging/factory.ts | 14 +-------- tests/integration/mcp/helpers.ts | 11 +++++-- 6 files changed, 54 insertions(+), 63 deletions(-) create mode 100644 src/infrastructure/git/resolveGitRoot.ts diff --git a/src/commands/hook.ts b/src/commands/hook.ts index d259905f..9f28ed2c 100644 --- a/src/commands/hook.ts +++ b/src/commands/hook.ts @@ -10,13 +10,13 @@ * git-mem hook prompt-submit */ -import { join, resolve } from 'path'; -import { execFileSync } from 'child_process'; +import { join } from 'path'; import { config as loadEnv } from 'dotenv'; import { createContainer } from '../infrastructure/di'; import { readStdin } from '../hooks/utils/stdin'; import { setupShutdown } from '../hooks/utils/shutdown'; import { loadHookConfig } from '../hooks/utils/config'; +import { resolveGitRoot } from '../infrastructure/git/resolveGitRoot'; import type { ILogger } from '../domain/interfaces/ILogger'; import type { IHooksConfig } from '../domain/interfaces/IHookConfig'; import type { HookEvent, HookEventType } from '../domain/events/HookEvents'; @@ -108,16 +108,7 @@ export function buildEvent(eventType: HookEventType, input: IHookInput): HookEve * Returns cwd if git command fails (graceful fallback). */ function findGitRoot(cwd: string): string { - try { - const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { - cwd, - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - }).trim(); - return resolve(root); - } catch { - return cwd; - } + return resolveGitRoot(cwd); } /** Stderr labels per event for user-facing messages. */ diff --git a/src/commands/init-hooks.ts b/src/commands/init-hooks.ts index 7444cc54..7a6c1e20 100644 --- a/src/commands/init-hooks.ts +++ b/src/commands/init-hooks.ts @@ -10,28 +10,12 @@ */ import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync, rmSync } from 'fs'; -import { execFileSync } from 'child_process'; -import { join, resolve } from 'path'; +import { join } from 'path'; import { homedir } from 'os'; import { parse as parseYaml, stringify as stringifyYaml } from 'yaml'; import type { ILogger } from '../domain/interfaces/ILogger'; import { getConfigPath, getConfigDir } from '../hooks/utils/config'; - -/** - * Resolve the git repository root directory. - * Falls back to process.cwd() if not inside a git repo. - */ -function resolveGitRoot(): string { - try { - const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - }).trim(); - return resolve(root); - } catch { - return process.cwd(); - } -} +import { resolveGitRoot } from '../infrastructure/git/resolveGitRoot'; interface IInitHooksOptions { yes?: boolean; diff --git a/src/commands/init.ts b/src/commands/init.ts index 3dc43957..9f082a46 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -7,7 +7,7 @@ import { existsSync, readFileSync, writeFileSync, appendFileSync, mkdirSync } from 'fs'; import { execFileSync } from 'child_process'; -import { join, resolve } from 'path'; +import { join } from 'path'; import prompts from 'prompts'; import { parse as parseYaml, stringify as stringifyYaml } from 'yaml'; import type { ILogger } from '../domain/interfaces/ILogger'; @@ -26,22 +26,7 @@ import { installCommitMsgHook, uninstallCommitMsgHook } from '../hooks/commit-ms import { createContainer } from '../infrastructure/di'; import { createStderrProgressHandler } from './progress'; import { getConfigPath, getConfigDir } from '../hooks/utils/config'; - -/** - * Resolve the git repository root directory. - * Falls back to process.cwd() if not inside a git repo. - */ -function resolveGitRoot(): string { - try { - const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - }).trim(); - return resolve(root); - } catch { - return process.cwd(); - } -} +import { resolveGitRoot } from '../infrastructure/git/resolveGitRoot'; interface IInitCommandOptions { yes?: boolean; diff --git a/src/infrastructure/git/resolveGitRoot.ts b/src/infrastructure/git/resolveGitRoot.ts new file mode 100644 index 00000000..8d8c8d7e --- /dev/null +++ b/src/infrastructure/git/resolveGitRoot.ts @@ -0,0 +1,38 @@ +/** + * Resolve the git repository root directory. + * + * Uses `git rev-parse --show-toplevel` and normalizes the path with + * path.resolve() for cross-platform compatibility (Windows returns + * forward-slash paths from git). + * + * @param cwd - Optional working directory to resolve from + * @returns Absolute path to the repo root, or fallback on failure + */ + +import { execFileSync } from 'child_process'; +import { resolve } from 'path'; + +/** + * Resolve git root, returning null on failure. + * Use this when the caller needs to handle the missing-repo case explicitly. + */ +export function getGitRoot(cwd?: string): string | null { + try { + const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { + cwd, + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + return resolve(root); + } catch { + return null; + } +} + +/** + * Resolve git root, falling back to process.cwd() on failure. + * Use this when a directory is always needed (e.g. init commands). + */ +export function resolveGitRoot(cwd?: string): string { + return getGitRoot(cwd) ?? process.cwd(); +} diff --git a/src/infrastructure/logging/factory.ts b/src/infrastructure/logging/factory.ts index 7f7d5e4b..150795ed 100644 --- a/src/infrastructure/logging/factory.ts +++ b/src/infrastructure/logging/factory.ts @@ -1,8 +1,8 @@ import * as path from 'node:path'; -import { execFileSync } from 'node:child_process'; import { ILogger, ILoggerOptions, LogLevel } from '../../domain/interfaces/ILogger'; import { Logger } from './Logger'; import { NullLogger } from './NullLogger'; +import { getGitRoot } from '../git/resolveGitRoot'; const VALID_LEVELS: readonly LogLevel[] = ['trace', 'debug', 'info', 'warn', 'error', 'fatal']; @@ -10,18 +10,6 @@ function isValidLogLevel(value: string): value is LogLevel { return VALID_LEVELS.includes(value as LogLevel); } -function getGitRoot(): string | null { - try { - const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - }).trim(); - return path.resolve(root); - } catch { - return null; - } -} - export function defaultLogDir(): string { const base = getGitRoot() ?? process.cwd(); return path.join(base, '.git-mem', 'logs'); diff --git a/tests/integration/mcp/helpers.ts b/tests/integration/mcp/helpers.ts index e1246334..2d3658ed 100644 --- a/tests/integration/mcp/helpers.ts +++ b/tests/integration/mcp/helpers.ts @@ -198,8 +198,13 @@ export function createTestRepo(prefix = 'git-mem-mcp-'): { dir: string; sha: str export function cleanupRepo(dir: string): void { try { rmSync(dir, { recursive: true, force: true, maxRetries: 3, retryDelay: 500 }); - } catch { - // On Windows, killed shell processes may still hold .git lock files briefly. - // Temp dirs will be cleaned up by the OS eventually. + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (process.platform === 'win32' && code === 'EPERM') { + // On Windows, killed shell processes may still hold .git lock files briefly. + // Temp dirs will be cleaned up by the OS eventually. + return; + } + throw err; } }