From 4578b2d3a99c5b4dcc140627221c50d160ed277b Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Tue, 9 Jun 2026 23:38:01 +0200 Subject: [PATCH] fix(cli): guide setup away from foreign repos at the project dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ktx owns the git repo rooted at the project dir and refuses to adopt one it did not create (the Finding 3 isolation invariant). But setup steered users straight into that failure: the interactive menu offers "Current directory" first, and `--no-input --yes --project-dir ` created directly in place — both then threw a generic "Failed to initialize git repository:" wrapper from deep in GitService.initialize(). Extract the ownership rule into a shared `classifyKtxRepoOwnership(dir)` used by both GitService.initialize() (the invariant) and the setup wizard (pre-flight guidance), so the decision derives from one rule. Setup now detects a foreign repo before constructing GitService and: interactively re-prompts (the user picks the existing `ktx-project` subfolder), or non-interactively returns a clean missing-input with the actionable message. The typed foreign-repo error is also surfaced verbatim instead of being buried under the generic wrapper. Empty/non-repo current directories still work — only foreign repos are blocked. --- .../docs/getting-started/quickstart.mdx | 7 ++ packages/cli/src/context/core/git.service.ts | 83 ++++++++++++------- packages/cli/src/setup-project.ts | 28 +++++++ .../core/git.service.repo-isolation.test.ts | 48 ++++++++++- packages/cli/test/setup-project.test.ts | 49 +++++++++++ 5 files changed, 183 insertions(+), 32 deletions(-) diff --git a/docs-site/content/docs/getting-started/quickstart.mdx b/docs-site/content/docs/getting-started/quickstart.mdx index 372bdca0..ecd2fbe7 100644 --- a/docs-site/content/docs/getting-started/quickstart.mdx +++ b/docs-site/content/docs/getting-started/quickstart.mdx @@ -342,6 +342,13 @@ separate `ktx` binary on `PATH`. If the CLI path changes, rerun changes there. If the project directory is nested inside another repository, **ktx** still keeps its own repo and does not commit to the parent repo. +Because **ktx** owns that repository, it will not adopt one it did not create. If +you point setup at a directory that is already a git repository's root - such as +an existing application checkout - **ktx** stops and asks you to pick a dedicated +directory instead. In the setup wizard choose the **New subfolder** option (for +example `ktx-project`), or pass a fresh `--project-dir` when running setup +non-interactively. + | Path | Purpose | |------|---------| | `ktx.yaml` | Project configuration | diff --git a/packages/cli/src/context/core/git.service.ts b/packages/cli/src/context/core/git.service.ts index 82657182..f9942dac 100644 --- a/packages/cli/src/context/core/git.service.ts +++ b/packages/cli/src/context/core/git.service.ts @@ -29,7 +29,7 @@ export interface WorktreeEntry { const KTX_MANAGED_GIT_CONFIG_KEY = 'ktx.managed'; -type GitDirState = 'absent' | 'directory' | 'foreign'; +export type KtxRepoOwnership = 'unowned' | 'ktx-managed' | 'foreign'; class KtxForeignGitRepositoryError extends Error { constructor(configDir: string) { @@ -45,6 +45,46 @@ function isNodeErrnoException(error: unknown): error is NodeJS.ErrnoException { return error instanceof Error && 'code' in error; } +/** + * Classify whether ktx may own a git repository rooted exactly at `dir`. + * + * - `unowned`: no `/.git` exists → ktx can `git init` here. Covers a fresh + * standalone directory and a fresh directory nested inside a parent repo. + * - `ktx-managed`: `/.git` is a directory carrying ktx's ownership marker. + * - `foreign`: a repo ktx did not create — a `.git` directory without the marker, + * or a `.git` *file* (a linked worktree). ktx must never adopt or mutate it. + * + * Reads only `/.git` directly and never walks up the directory tree, so the + * classification of a project dir never depends on whether a parent repo exists. + * Shared by `GitService.initialize()` (the invariant) and the setup wizard (the + * pre-flight guidance) so both decide ownership from the same rule. + */ +export async function classifyKtxRepoOwnership(dir: string): Promise { + let dotGitIsDirectory: boolean; + try { + dotGitIsDirectory = (await fs.lstat(join(dir, '.git'))).isDirectory(); + } catch (error) { + if (isNodeErrnoException(error) && error.code === 'ENOENT') { + return 'unowned'; + } + throw error; + } + if (!dotGitIsDirectory) { + return 'foreign'; + } + try { + const marker = await createSimpleGit(dir).raw([ + 'config', + '--local', + '--get', + KTX_MANAGED_GIT_CONFIG_KEY, + ]); + return marker.trim() === 'true' ? 'ktx-managed' : 'foreign'; + } catch { + return 'foreign'; + } +} + export type SquashMergeResult = | { ok: true; squashSha: string; touchedPaths: string[] } | { ok: false; conflict: true; conflictPaths: string[] }; @@ -114,43 +154,19 @@ 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 { - const gitDirState = await this.gitDirState(); + const ownership = await classifyKtxRepoOwnership(this.configDir); - if (gitDirState === 'absent') { + if (ownership === 'foreign') { + throw new KtxForeignGitRepositoryError(this.configDir); + } + if (ownership === 'unowned') { await this.git.init(); 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); } + // ownership === 'ktx-managed' → ktx's own repo; proceed with the normal re-run path. // Keep any auto-maintenance triggered by writes in-process. Detached maintenance can // keep object-pack directories alive briefly after awaited git commands complete, @@ -171,6 +187,11 @@ export class GitService { this.logger.log('Wrote bootstrap commit to config repo'); } } catch (error) { + // The foreign-repo error is already typed and actionable; surface it verbatim so every + // command that loads the project shows the same clear guidance instead of a generic wrapper. + if (error instanceof KtxForeignGitRepositoryError) { + throw error; + } this.logger.error('Failed to initialize git repository', error); // Preserve the underlying git error: the generic message alone is undiagnosable in // telemetry and unactionable for the user. The exception reporter walks `cause` and diff --git a/packages/cli/src/setup-project.ts b/packages/cli/src/setup-project.ts index 08f935e6..3f36fb24 100644 --- a/packages/cli/src/setup-project.ts +++ b/packages/cli/src/setup-project.ts @@ -2,6 +2,7 @@ import { existsSync } from 'node:fs'; import { mkdir, readdir, readFile, stat, writeFile } from 'node:fs/promises'; import { homedir } from 'node:os'; import { join, resolve } from 'node:path'; +import { classifyKtxRepoOwnership } from './context/core/git.service.js'; import { initKtxProject, type KtxLocalProject, loadKtxProject } from './context/project/project.js'; import { markKtxSetupStateStepComplete, mergeKtxSetupGitignoreEntries } from './context/project/setup-config.js'; import { serializeKtxProjectConfig } from './context/project/config.js'; @@ -106,11 +107,32 @@ type ConfirmProjectDirResult = | { status: 'cancelled' } | { status: 'not-directory' }; +/** + * ktx owns the git repository at the project dir, so it refuses to create a + * project inside a repository it did not create (which it would otherwise have + * to adopt or fail on at first commit). Guides the user toward a dedicated + * directory instead of letting `GitService.initialize()` throw mid-setup. + */ +async function ensureProjectDirIsOwnable(selectedDir: string, io: KtxCliIo): Promise { + if ((await classifyKtxRepoOwnership(selectedDir)) === 'foreign') { + io.stderr.write( + `${selectedDir} is already a git repository that ktx did not create.\n` + + 'ktx keeps its context in a repository it owns. Choose a new subfolder or an empty directory instead.\n', + ); + return false; + } + return true; +} + async function confirmProjectDir( selectedDir: string, io: KtxCliIo, prompts: KtxSetupProjectPromptAdapter, ): Promise { + if (!(await ensureProjectDirIsOwnable(selectedDir, io))) { + return { status: 'choose-another' }; + } + const state = await existingFolderState(selectedDir); if (state === 'not-directory') { @@ -287,6 +309,9 @@ export async function runKtxSetupProjectStep( io.stderr.write('Missing setup choice: pass --yes to create a project in non-interactive setup.\n'); return { status: 'missing-input', projectDir }; } + if (!(await ensureProjectDirIsOwnable(projectDir, io))) { + return { status: 'missing-input', projectDir }; + } const project = await createProject(projectDir, deps); printProjectSummary(io, projectDir); return { @@ -332,6 +357,9 @@ export async function runKtxSetupProjectStep( } if (choice === 'current') { + if (!(await ensureProjectDirIsOwnable(projectDir, io))) { + continue; + } const project = await createProject(projectDir, deps); printProjectSummary(io, projectDir); return { 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 index c0e8b26a..0061c8ff 100644 --- a/packages/cli/test/context/core/git.service.repo-isolation.test.ts +++ b/packages/cli/test/context/core/git.service.repo-isolation.test.ts @@ -5,7 +5,7 @@ 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'; +import { classifyKtxRepoOwnership, GitService } from '../../../src/context/core/git.service.js'; function coreConfig(configDir: string): KtxCoreConfig { return { @@ -106,3 +106,49 @@ describe('GitService repository ownership', () => { expect(git(projectDir, ['config', '--local', '--get', 'ktx.managed'])).toBe('true'); }); }); + +describe('classifyKtxRepoOwnership', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'git-ownership-')); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + }); + + it('reports unowned when no .git exists at the directory', async () => { + const dir = join(tempDir, 'fresh'); + await mkdir(dir, { recursive: true }); + expect(await classifyKtxRepoOwnership(dir)).toBe('unowned'); + }); + + it('reports unowned for a fresh directory nested inside an enclosing repo', async () => { + const parentDir = join(tempDir, 'parent'); + const nestedDir = join(parentDir, 'nested'); + await mkdir(nestedDir, { recursive: true }); + git(parentDir, ['init']); + expect(await classifyKtxRepoOwnership(nestedDir)).toBe('unowned'); + }); + + it('reports ktx-managed for a repo ktx initialized', async () => { + const dir = join(tempDir, 'owned'); + await new GitService(coreConfig(dir)).onModuleInit(); + expect(await classifyKtxRepoOwnership(dir)).toBe('ktx-managed'); + }); + + it('reports foreign for a repo ktx did not create', async () => { + const dir = join(tempDir, 'foreign'); + await mkdir(dir, { recursive: true }); + git(dir, ['init']); + expect(await classifyKtxRepoOwnership(dir)).toBe('foreign'); + }); + + it('reports foreign for a .git file (linked worktree)', async () => { + const dir = join(tempDir, 'linked'); + await mkdir(dir, { recursive: true }); + await writeFile(join(dir, '.git'), 'gitdir: ../actual.git\n', 'utf-8'); + expect(await classifyKtxRepoOwnership(dir)).toBe('foreign'); + }); +}); diff --git a/packages/cli/test/setup-project.test.ts b/packages/cli/test/setup-project.test.ts index c77a2080..ba9a625b 100644 --- a/packages/cli/test/setup-project.test.ts +++ b/packages/cli/test/setup-project.test.ts @@ -1,3 +1,4 @@ +import { execFileSync } from 'node:child_process'; import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -44,6 +45,19 @@ function defaultSubfolderLabel(parentDir: string): string { return `New subfolder (${gray(childDir.slice(0, -childName.length))}${childName})`; } +function initForeignRepo(dir: string): void { + execFileSync('git', ['init'], { + cwd: dir, + env: { + ...process.env, + GIT_AUTHOR_NAME: 'Foreign User', + GIT_AUTHOR_EMAIL: 'foreign@example.com', + GIT_COMMITTER_NAME: 'Foreign User', + GIT_COMMITTER_EMAIL: 'foreign@example.com', + }, + }); +} + describe('setup project step', () => { let tempDir: string; @@ -295,6 +309,41 @@ describe('setup project step', () => { await expect(stat(join(projectDir, 'ktx.yaml'))).resolves.toBeDefined(); }); + it('refuses to create a project in a foreign git repo in non-interactive mode', async () => { + const projectDir = join(tempDir, 'app-repo'); + await mkdir(projectDir, { recursive: true }); + initForeignRepo(projectDir); + const testIo = makeIo(); + + await expect( + runKtxSetupProjectStep({ projectDir, mode: 'auto', inputMode: 'disabled', yes: true }, testIo.io), + ).resolves.toMatchObject({ status: 'missing-input', projectDir }); + + expect(testIo.stderr()).toContain('already a git repository that ktx did not create'); + await expect(stat(join(projectDir, 'ktx.yaml'))).rejects.toThrow(); + }); + + it('re-prompts away from a foreign current directory and creates the project in a subfolder', async () => { + const projectDir = join(tempDir, 'app-repo'); + await mkdir(projectDir, { recursive: true }); + initForeignRepo(projectDir); + const subfolderDir = join(projectDir, 'ktx-project'); + const prompts = makePromptAdapter({ choices: ['current', 'new-default', 'create'] }); + const testIo = makeIo({ stdoutIsTty: true }); + + const result = await runKtxSetupProjectStep( + { projectDir, mode: 'auto', inputMode: 'auto', yes: false }, + testIo.io, + { prompts }, + ); + + expect(result.status).toBe('ready'); + expect(result.projectDir).toBe(subfolderDir); + expect(testIo.stderr()).toContain('already a git repository that ktx did not create'); + await expect(stat(join(subfolderDir, 'ktx.yaml'))).resolves.toBeDefined(); + await expect(stat(join(projectDir, 'ktx.yaml'))).rejects.toThrow(); + }); + it('prompts to exit and returns cancelled in interactive auto mode', async () => { const projectDir = join(tempDir, 'warehouse'); const prompts = makePromptAdapter({ choice: 'exit' });