From 6b2f7c3365e87aa357fe128b8cd58be51f2cb83b Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Tue, 9 Jun 2026 12:10:02 +0200 Subject: [PATCH] fix(cli): ensure git committer identity during ktx setup (#276) * fix(cli): ensure git committer identity during ktx setup ktx setup threw "Failed to initialize git repository" when the project directory was already a git repo with no commits and the machine had no configured git identity (e.g. a fresh Mac with no ~/.gitconfig). GitService only set the identity on the path where it created the repo itself, so the bootstrap commit had no resolvable committer. Carry ktx's identity via GIT_AUTHOR/GIT_COMMITTER env on the shared git client so every commit succeeds regardless of whether ktx created the repo, without mutating the user's repo config. Also preserve the underlying git error when rethrowing so the failure is diagnosable in telemetry and actionable for the user. * chore: sync uv.lock ktx-daemon and ktx-sl versions to 0.10.0 --- packages/cli/src/context/core/git-env.ts | 19 +++- packages/cli/src/context/core/git.service.ts | 22 ++-- .../core/git.service.init-identity.test.ts | 101 ++++++++++++++++++ uv.lock | 4 +- 4 files changed, 135 insertions(+), 11 deletions(-) create mode 100644 packages/cli/test/context/core/git.service.init-identity.test.ts diff --git a/packages/cli/src/context/core/git-env.ts b/packages/cli/src/context/core/git-env.ts index 9ad3f121..645a29cc 100644 --- a/packages/cli/src/context/core/git-env.ts +++ b/packages/cli/src/context/core/git-env.ts @@ -24,6 +24,21 @@ function sanitizedGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEn return sanitized; } -export function createSimpleGit(baseDir: string): SimpleGit { - return simpleGit({ baseDir, unsafe: { allowUnsafeAskPass: true } }).env(sanitizedGitEnv()); +/** + * Create a simple-git client scoped to `baseDir`. When an identity is provided, ktx's own + * commits carry it through the GIT_AUTHOR and GIT_COMMITTER environment variables instead of + * relying on repo-local or global git config. This keeps commits working when the project + * directory is an existing repo ktx did not create and the machine has no configured git + * identity (e.g. a fresh Mac with no ~/.gitconfig), without mutating the user's repo config. + * Explicit `--author` flags on individual commits still take precedence over GIT_AUTHOR_NAME. + */ +export function createSimpleGit(baseDir: string, identity?: { name: string; email: string }): SimpleGit { + const env = sanitizedGitEnv(); + if (identity?.name && identity.email) { + env.GIT_AUTHOR_NAME = identity.name; + env.GIT_AUTHOR_EMAIL = identity.email; + env.GIT_COMMITTER_NAME = identity.name; + env.GIT_COMMITTER_EMAIL = identity.email; + } + return simpleGit({ baseDir, unsafe: { allowUnsafeAskPass: true } }).env(env); } diff --git a/packages/cli/src/context/core/git.service.ts b/packages/cli/src/context/core/git.service.ts index 216183ff..216c5460 100644 --- a/packages/cli/src/context/core/git.service.ts +++ b/packages/cli/src/context/core/git.service.ts @@ -85,8 +85,12 @@ export class GitService { await fs.mkdir(this.configDir, { recursive: true }); this.logger.log(`Config directory ensured at: ${this.configDir}`); - // Initialize simple-git - this.git = createSimpleGit(this.configDir); + // Initialize simple-git. Carry ktx's identity in the environment so commits succeed even + // when this repo already exists and the machine has no configured git identity. + this.git = createSimpleGit(this.configDir, { + name: this.config.git.userName, + email: this.config.git.userEmail, + }); // Initialize git repository await this.initialize(); @@ -99,9 +103,6 @@ export class GitService { if (!isRepo) { await this.git.init(); - const gitConfig = this.config.git; - await this.git.addConfig('user.name', gitConfig.userName); - await this.git.addConfig('user.email', gitConfig.userEmail); this.logger.log('Initialized git repository'); } @@ -125,7 +126,11 @@ export class GitService { } } catch (error) { this.logger.error('Failed to initialize git repository', error); - throw new Error('Failed to initialize git repository'); + // Preserve the underlying git error: the generic message alone is undiagnosable in + // telemetry and unactionable for the user. The exception reporter walks `cause` and + // redacts secrets before send. + const detail = error instanceof Error ? error.message : String(error); + throw new Error(`Failed to initialize git repository: ${detail}`, { cause: error }); } } @@ -899,7 +904,10 @@ export class GitService { */ forWorktree(workdir: string): GitService { const scoped = new GitService(this.config, this.logger); - scoped.git = createSimpleGit(workdir); + scoped.git = createSimpleGit(workdir, { + name: this.config.git.userName, + email: this.config.git.userEmail, + }); scoped.configDir = workdir; return scoped; } diff --git a/packages/cli/test/context/core/git.service.init-identity.test.ts b/packages/cli/test/context/core/git.service.init-identity.test.ts new file mode 100644 index 00000000..8589d1ed --- /dev/null +++ b/packages/cli/test/context/core/git.service.init-identity.test.ts @@ -0,0 +1,101 @@ +import { execFileSync } from 'node:child_process'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import type { KtxCoreConfig } from '../../../src/context/core/config.js'; +import { GitService } from '../../../src/context/core/git.service.js'; + +// Regression for the production exception "Failed to initialize git repository" +// (PostHog issue 019ea9df-96d6-7882-98e2-6b892bf9c1ab, ktx 0.10.0, darwin). +// +// Repro: the project directory is ALREADY a git repo with no commits (the user ran +// `git init` first, or ktx is pointed at an empty repo), AND the machine has no configured +// git identity (a fresh Mac with no ~/.gitconfig). GitService only set the committer identity +// on the path where it created the repo itself, so the bootstrap commit failed with +// "Committer identity unknown" and was rethrown opaquely. +describe('GitService.initialize without a configured git identity', () => { + let repoDir: string; + let homeDir: string; + let savedEnv: Record; + + const IDENTITY_ENV_KEYS = [ + 'HOME', + 'USERPROFILE', + 'XDG_CONFIG_HOME', + 'GIT_CONFIG_NOSYSTEM', + 'GIT_AUTHOR_NAME', + 'GIT_AUTHOR_EMAIL', + 'GIT_COMMITTER_NAME', + 'GIT_COMMITTER_EMAIL', + 'EMAIL', + ]; + + const coreConfig = (configDir: string): KtxCoreConfig => ({ + storage: { configDir, homeDir: configDir }, + git: { + userName: 'Test User', + userEmail: 'test@example.com', + bootstrapMessage: 'Initialize test config repo', + bootstrapAuthor: 'test-system', + bootstrapAuthorEmail: 'system@example.com', + }, + }); + + beforeEach(async () => { + repoDir = await mkdtemp(join(tmpdir(), 'git-service-identity-')); + homeDir = await mkdtemp(join(tmpdir(), 'git-service-home-')); + + // Model a machine with no configured git identity, deterministically and independent of + // the host's ~/.gitconfig. `useConfigOnly` disables git's username@hostname email guess, + // so a missing identity is a hard failure rather than a hostname-dependent one. Note we + // cannot use GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM here: simple-git rejects those env vars. + await writeFile(join(homeDir, '.gitconfig'), '[user]\n\tuseConfigOnly = true\n', 'utf-8'); + + savedEnv = Object.fromEntries(IDENTITY_ENV_KEYS.map((key) => [key, process.env[key]])); + process.env.HOME = homeDir; + process.env.USERPROFILE = homeDir; + process.env.XDG_CONFIG_HOME = join(homeDir, 'xdg-empty'); + process.env.GIT_CONFIG_NOSYSTEM = '1'; + for (const key of ['GIT_AUTHOR_NAME', 'GIT_AUTHOR_EMAIL', 'GIT_COMMITTER_NAME', 'GIT_COMMITTER_EMAIL', 'EMAIL']) { + delete process.env[key]; + } + + // Pre-create an empty repo: checkIsRepo() will be true, but there is no HEAD yet. + execFileSync('git', ['init'], { cwd: repoDir, env: process.env, stdio: 'ignore' }); + }); + + afterEach(async () => { + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + await rm(repoDir, { recursive: true, force: true }); + await rm(homeDir, { recursive: true, force: true }); + }); + + it('bootstraps a commit in a pre-existing empty repo so HEAD resolves', async () => { + const service = new GitService(coreConfig(repoDir)); + + await expect(service.onModuleInit()).resolves.toBeUndefined(); + + const head = await service.revParseHead(); + expect(head).toMatch(/^[0-9a-f]{40}$/); + }); + + it("does not write its identity into the user's repo config", async () => { + const service = new GitService(coreConfig(repoDir)); + await service.onModuleInit(); + + // ktx must not hijack the identity the user would use for their own commits in this repo. + const localName = execFileSync('git', ['config', '--local', '--default', '', 'user.name'], { + cwd: repoDir, + env: process.env, + encoding: 'utf-8', + }).trim(); + expect(localName).toBe(''); + }); +}); diff --git a/uv.lock b/uv.lock index 40553e46..1f35cc3e 100644 --- a/uv.lock +++ b/uv.lock @@ -466,7 +466,7 @@ wheels = [ [[package]] name = "ktx-daemon" -version = "0.9.0" +version = "0.10.0" source = { editable = "python/ktx-daemon" } dependencies = [ { name = "fastapi" }, @@ -523,7 +523,7 @@ dev = [ [[package]] name = "ktx-sl" -version = "0.9.0" +version = "0.10.0" source = { editable = "python/ktx-sl" } dependencies = [ { name = "pydantic" },