diff --git a/packages/cli/src/context/core/git.service.ts b/packages/cli/src/context/core/git.service.ts index 216c5460..82657182 100644 --- a/packages/cli/src/context/core/git.service.ts +++ b/packages/cli/src/context/core/git.service.ts @@ -27,6 +27,24 @@ export interface WorktreeEntry { head: string | null; } +const KTX_MANAGED_GIT_CONFIG_KEY = 'ktx.managed'; + +type GitDirState = 'absent' | 'directory' | 'foreign'; + +class KtxForeignGitRepositoryError extends Error { + constructor(configDir: string) { + super( + `${configDir} is already a git repository that ktx did not create. ` + + 'ktx maintains its context in a repository it owns; run ktx in a dedicated directory or move the existing repository aside.', + ); + this.name = 'KtxForeignGitRepositoryError'; + } +} + +function isNodeErrnoException(error: unknown): error is NodeJS.ErrnoException { + return error instanceof Error && 'code' in error; +} + export type SquashMergeResult = | { ok: true; squashSha: string; touchedPaths: string[] } | { ok: false; conflict: true; conflictPaths: string[] }; @@ -96,14 +114,42 @@ export class GitService { await this.initialize(); } + private async gitDirState(): Promise { + try { + const stat = await fs.lstat(join(this.configDir, '.git')); + return stat.isDirectory() ? 'directory' : 'foreign'; + } catch (error) { + if (isNodeErrnoException(error) && error.code === 'ENOENT') { + return 'absent'; + } + throw error; + } + } + + private async hasKtxManagedMarker(): Promise { + try { + const value = await this.git.raw(['config', '--local', '--get', KTX_MANAGED_GIT_CONFIG_KEY]); + return value.trim() === 'true'; + } catch { + return false; + } + } + private async initialize(): Promise { try { - // Check if already initialized - const isRepo = await this.git.checkIsRepo(); + const gitDirState = await this.gitDirState(); - if (!isRepo) { + if (gitDirState === 'absent') { await this.git.init(); - this.logger.log('Initialized git repository'); + await this.git.addConfig(KTX_MANAGED_GIT_CONFIG_KEY, 'true'); + this.logger.log('Initialized ktx-managed git repository'); + } else if (gitDirState === 'directory') { + const isManaged = await this.hasKtxManagedMarker(); + if (!isManaged) { + throw new KtxForeignGitRepositoryError(this.configDir); + } + } else { + throw new KtxForeignGitRepositoryError(this.configDir); } // Keep any auto-maintenance triggered by writes in-process. Detached maintenance can 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 index 8589d1ed..0b3347a8 100644 --- a/packages/cli/test/context/core/git.service.init-identity.test.ts +++ b/packages/cli/test/context/core/git.service.init-identity.test.ts @@ -6,14 +6,10 @@ 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. +// Regression for bootstrapping a marked ktx repo on a machine with no configured +// git identity. A foreign pre-existing repo is rejected by the ownership rule; +// this test covers the still-valid path where the repo is already ktx-managed +// but has no HEAD yet. describe('GitService.initialize without a configured git identity', () => { let repoDir: string; let homeDir: string; @@ -61,8 +57,12 @@ describe('GitService.initialize without a configured git identity', () => { 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' }); + execFileSync('git', ['config', '--local', 'ktx.managed', 'true'], { + cwd: repoDir, + env: process.env, + stdio: 'ignore', + }); }); afterEach(async () => { diff --git a/packages/cli/test/context/core/git.service.repo-isolation.test.ts b/packages/cli/test/context/core/git.service.repo-isolation.test.ts new file mode 100644 index 00000000..c0e8b26a --- /dev/null +++ b/packages/cli/test/context/core/git.service.repo-isolation.test.ts @@ -0,0 +1,108 @@ +import { execFileSync } from 'node:child_process'; +import { mkdir, mkdtemp, readFile, realpath, 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'; + +function coreConfig(configDir: string): KtxCoreConfig { + return { + storage: { configDir, homeDir: configDir }, + git: { + userName: 'Test User', + userEmail: 'test@example.com', + bootstrapMessage: 'Initialize test config repo', + bootstrapAuthor: 'test-system', + bootstrapAuthorEmail: 'system@example.com', + }, + }; +} + +function git(cwd: string, args: string[]): string { + return execFileSync('git', args, { + cwd, + encoding: 'utf-8', + env: { + ...process.env, + GIT_AUTHOR_NAME: 'Parent User', + GIT_AUTHOR_EMAIL: 'parent@example.com', + GIT_COMMITTER_NAME: 'Parent User', + GIT_COMMITTER_EMAIL: 'parent@example.com', + }, + }).trim(); +} + +describe('GitService repository ownership', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'git-service-isolation-')); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + }); + + it('creates and commits inside its own repo when nested in an enclosing repo', async () => { + const parentDir = join(tempDir, 'parent'); + const projectDir = join(parentDir, '.ktx-project'); + await mkdir(projectDir, { recursive: true }); + + git(parentDir, ['init']); + await writeFile(join(parentDir, 'README.md'), '# Parent\n', 'utf-8'); + git(parentDir, ['add', 'README.md']); + git(parentDir, ['commit', '-m', 'parent baseline']); + const parentHeadBefore = git(parentDir, ['rev-parse', 'HEAD']); + + const service = new GitService(coreConfig(projectDir)); + await service.onModuleInit(); + + expect(git(projectDir, ['config', '--local', '--get', 'ktx.managed'])).toBe('true'); + expect(git(parentDir, ['rev-parse', 'HEAD'])).toBe(parentHeadBefore); + expect(await realpath(git(projectDir, ['rev-parse', '--show-toplevel']))).toBe(await realpath(projectDir)); + + await writeFile(join(projectDir, 'wiki.md'), '# Wiki\n', 'utf-8'); + await service.commitFile('wiki.md', 'Add wiki page', 'Test User', 'test@example.com'); + + expect(git(parentDir, ['rev-parse', 'HEAD'])).toBe(parentHeadBefore); + expect(git(projectDir, ['log', '--oneline', '--max-count=1'])).toContain('Add wiki page'); + expect(git(parentDir, ['status', '--short'])).toContain('?? .ktx-project/'); + }); + + it('rejects a foreign repo rooted at the project dir', async () => { + const projectDir = join(tempDir, 'foreign'); + await mkdir(projectDir, { recursive: true }); + git(projectDir, ['init']); + const configBefore = await readFile(join(projectDir, '.git', 'config'), 'utf-8'); + + const service = new GitService(coreConfig(projectDir)); + + await expect(service.onModuleInit()).rejects.toThrow(/already a git repository that ktx did not create/); + expect(await readFile(join(projectDir, '.git', 'config'), 'utf-8')).toBe(configBefore); + }); + + it('rejects a gitfile at the project dir as foreign', async () => { + const projectDir = join(tempDir, 'linked-worktree'); + await mkdir(projectDir, { recursive: true }); + await writeFile(join(projectDir, '.git'), 'gitdir: ../actual.git\n', 'utf-8'); + + const service = new GitService(coreConfig(projectDir)); + + await expect(service.onModuleInit()).rejects.toThrow(/already a git repository that ktx did not create/); + }); + + it('accepts a marked ktx repo and does not create a second bootstrap commit', async () => { + const projectDir = join(tempDir, 'owned'); + const service = new GitService(coreConfig(projectDir)); + await service.onModuleInit(); + const before = await service.revParseHead(); + + const second = new GitService(coreConfig(projectDir)); + await second.onModuleInit(); + + expect(await second.revParseHead()).toBe(before); + expect(git(projectDir, ['config', '--local', '--get', 'ktx.managed'])).toBe('true'); + }); +}); diff --git a/packages/cli/test/context/core/git.service.test.ts b/packages/cli/test/context/core/git.service.test.ts index 8b19ed09..1318add8 100644 --- a/packages/cli/test/context/core/git.service.test.ts +++ b/packages/cli/test/context/core/git.service.test.ts @@ -1,3 +1,4 @@ +import { execFileSync } from 'node:child_process'; import { mkdtemp, readFile, realpath, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -44,6 +45,11 @@ describe('GitService', () => { // beforeEach already ran onModuleInit() against an empty temp dir. const head = await service.revParseHead(); expect(head).toMatch(/^[0-9a-f]{40}$/); + const marker = execFileSync('git', ['config', '--local', '--get', 'ktx.managed'], { + cwd: tempDir, + encoding: 'utf-8', + }).trim(); + expect(marker).toBe('true'); }); it('does not double-commit when re-initialized', async () => {