mirror of
https://github.com/Kaelio/ktx.git
synced 2026-06-13 08:15:14 +02:00
fix(cli): guide setup away from foreign repos at the project dir
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 <repo-root>` 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.
This commit is contained in:
parent
133b879ed3
commit
4578b2d3a9
5 changed files with 183 additions and 32 deletions
|
|
@ -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 |
|
||||
|
|
|
|||
|
|
@ -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 `<dir>/.git` exists → ktx can `git init` here. Covers a fresh
|
||||
* standalone directory and a fresh directory nested inside a parent repo.
|
||||
* - `ktx-managed`: `<dir>/.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 `<dir>/.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<KtxRepoOwnership> {
|
||||
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<GitDirState> {
|
||||
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<boolean> {
|
||||
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<void> {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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<boolean> {
|
||||
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<ConfirmProjectDirResult> {
|
||||
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 {
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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' });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue