From 2877b85adca96b12e94d08fc334952ea81697393 Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Wed, 10 Jun 2026 14:12:25 +0200 Subject: [PATCH] fix(cli): isolate ktx-owned project repositories (#283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): isolate ktx project git repos * fix(cli): remove inert auto commit config * test(cli): drop stale auto commit fixtures * docs: document isolated ktx project repos * test(cli): keep stale config grep clean * 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 ` 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. * fix(cli): keep classifyKtxRepoOwnership total for non-directory paths The setup ownership guard runs before the existing not-a-directory check, so pointing a custom/--project-dir path at a file made classifyKtxRepoOwnership lstat `/.git`, hit ENOTDIR, and throw — crashing the setup step instead of returning the friendly "path exists and is not a directory" result. A path that is a file (or missing) holds no git repo for ktx to avoid, so treat ENOTDIR like ENOENT and return 'unowned'. The downstream existingFolderState check still rejects a non-directory with its friendly message, and the classifier no longer throws raw errno for any caller. --- .../content/docs/configuration/ktx-yaml.mdx | 24 +-- .../docs/getting-started/quickstart.mdx | 10 ++ .../content/docs/guides/reviewing-context.mdx | 7 +- examples/local-warehouse/ktx.yaml | 3 - .../orbit-relationship-verification/ktx.yaml | 1 - packages/cli/src/context/core/git.service.ts | 89 ++++++++-- packages/cli/src/context/project/config.ts | 12 +- packages/cli/src/demo-assets.ts | 1 - packages/cli/src/setup-project.ts | 28 +++ packages/cli/src/status-project.ts | 9 +- .../core/git.service.init-identity.test.ts | 18 +- .../core/git.service.repo-isolation.test.ts | 160 ++++++++++++++++++ .../cli/test/context/core/git.service.test.ts | 6 + .../local-ingest-acceptance.test.ts | 1 - .../ingest/local-bundle-ingest.test.ts | 4 - .../cli/test/context/project/config.test.ts | 30 +++- packages/cli/test/demo-assets.test.ts | 3 + packages/cli/test/setup-project.test.ts | 67 ++++++++ packages/cli/test/status-project.test.ts | 14 ++ .../test/telemetry/project-snapshot.test.ts | 3 +- 20 files changed, 412 insertions(+), 78 deletions(-) create mode 100644 packages/cli/test/context/core/git.service.repo-isolation.test.ts diff --git a/docs-site/content/docs/configuration/ktx-yaml.mdx b/docs-site/content/docs/configuration/ktx-yaml.mdx index db74ffa7..d5dd18c8 100644 --- a/docs-site/content/docs/configuration/ktx-yaml.mdx +++ b/docs-site/content/docs/configuration/ktx-yaml.mdx @@ -344,15 +344,14 @@ setup: ## `storage` -`storage` controls where **ktx** keeps its own state and search index, and how -state changes are committed. Defaults work for a single-user local project. +`storage` controls where **ktx** keeps its own state and search index. Defaults +work for a single-user local project. ```yaml storage: state: sqlite # sqlite | postgres search: sqlite-fts5 # sqlite-fts5 | postgres-hybrid git: - auto_commit: true author: "ktx " ``` @@ -360,8 +359,7 @@ storage: |-------|------|---------|---------| | `state` | `sqlite` \| `postgres` | `sqlite` | Backend for ktx state. `sqlite` uses `.ktx/db.sqlite`; `postgres` expects a configured Postgres connection. | | `search` | `sqlite-fts5` \| `postgres-hybrid` | `sqlite-fts5` | Backend for search indexes. `postgres-hybrid` combines lexical and vector search in Postgres. | -| `git.auto_commit` | `boolean` | `true` | When `true`, ktx auto-commits changes to the git-backed state store. | -| `git.author` | `string` | `ktx ` | Git author identity for auto-commits. Standard `Name ` form. | +| `git.author` | `string` | `ktx ` | Git author identity for commits. Standard `Name ` form. | ## `llm` @@ -608,19 +606,6 @@ agent: | `run_research.max_iterations` | `int ≥ 0` | `20` | Maximum tool-call iterations per research run. | | `run_research.default_toolset` | `string[]` | `[sl_query, wiki_search, sl_read_source]` | Tool identifiers exposed to the research agent. | -## `memory` - -`memory` controls the agent memory subsystem. - -```yaml -memory: - auto_commit: true -``` - -| Field | Type | Default | Purpose | -|-------|------|---------|---------| -| `auto_commit` | `boolean` | `true` | When `true`, ktx auto-commits memory updates to the git-backed store. | - ## A full example Combining the blocks above: @@ -645,7 +630,6 @@ storage: state: sqlite search: sqlite-fts5 git: - auto_commit: true author: "ktx " llm: provider: @@ -678,8 +662,6 @@ scan: agent: run_research: enabled: true -memory: - auto_commit: true ``` ## Validating your config diff --git a/docs-site/content/docs/getting-started/quickstart.mdx b/docs-site/content/docs/getting-started/quickstart.mdx index 50059cb6..ecd2fbe7 100644 --- a/docs-site/content/docs/getting-started/quickstart.mdx +++ b/docs-site/content/docs/getting-started/quickstart.mdx @@ -338,6 +338,16 @@ separate `ktx` binary on `PATH`. If the CLI path changes, rerun ## What setup writes **ktx** writes plain files so people and agents can review changes in git. +**ktx** initializes a git repository at the project directory and writes context +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 | |------|---------| diff --git a/docs-site/content/docs/guides/reviewing-context.mdx b/docs-site/content/docs/guides/reviewing-context.mdx index 63d4fceb..26f191b6 100644 --- a/docs-site/content/docs/guides/reviewing-context.mdx +++ b/docs-site/content/docs/guides/reviewing-context.mdx @@ -61,11 +61,14 @@ committing the file. ## A typical review session -The loop above describes the shape. In practice, one review session looks like -this: +The loop above describes the shape. Run these commands from the **ktx** project +directory. **ktx** keeps that directory as its own git repository, even when the +directory lives inside another repository, so reviewing context changes never +requires committing to a parent application repo. ```bash # 1. Run ingest on a branch +cd /path/to/ktx-project git checkout -b ingest/2026-05-21 ktx ingest --all diff --git a/examples/local-warehouse/ktx.yaml b/examples/local-warehouse/ktx.yaml index fb8ae027..3568da9d 100644 --- a/examples/local-warehouse/ktx.yaml +++ b/examples/local-warehouse/ktx.yaml @@ -5,7 +5,6 @@ storage: state: sqlite search: sqlite-fts5 git: - auto_commit: true author: "ktx " ingest: adapters: @@ -18,5 +17,3 @@ agent: - sl_query - wiki_search - sl_read_source -memory: - auto_commit: true diff --git a/examples/orbit-relationship-verification/ktx.yaml b/examples/orbit-relationship-verification/ktx.yaml index b1d30961..4bb605e2 100644 --- a/examples/orbit-relationship-verification/ktx.yaml +++ b/examples/orbit-relationship-verification/ktx.yaml @@ -6,7 +6,6 @@ storage: state: sqlite search: sqlite-fts5 git: - auto_commit: true author: "ktx " ingest: adapters: [] diff --git a/packages/cli/src/context/core/git.service.ts b/packages/cli/src/context/core/git.service.ts index 9a826e4b..3095ad00 100644 --- a/packages/cli/src/context/core/git.service.ts +++ b/packages/cli/src/context/core/git.service.ts @@ -1,6 +1,6 @@ import { promises as fs } from 'node:fs'; import { dirname, join } from 'node:path'; -import { CheckRepoActions, type SimpleGit } from 'simple-git'; +import type { SimpleGit } from 'simple-git'; import { noopLogger, resolveConfigDir, type KtxCoreConfig, type KtxLogger } from './config.js'; import { createSimpleGit } from './git-env.js'; @@ -27,6 +27,69 @@ export interface WorktreeEntry { head: string | null; } +const KTX_MANAGED_GIT_CONFIG_KEY = 'ktx.managed'; + +export type KtxRepoOwnership = 'unowned' | 'ktx-managed' | '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; +} + +/** + * Classify whether ktx may own a git repository rooted exactly at `dir`. + * + * - `unowned`: there is no git repository for ktx to avoid here → ktx may + * `git init`. Covers a fresh standalone directory, a fresh directory nested + * inside a parent repo, a path that does not exist yet, and a path that is not + * a directory at all (whether the path is a usable project directory is a + * separate concern for the caller to validate). + * - `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) { + // ENOENT: `/.git` is absent. ENOTDIR: `` itself is a file, so it + // can hold no repo. Either way there is nothing for ktx to avoid here. + if (isNodeErrnoException(error) && (error.code === 'ENOENT' || error.code === 'ENOTDIR')) { + 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[] }; @@ -98,18 +161,17 @@ export class GitService { private async initialize(): Promise { try { - // Adopt an existing repo ONLY when this directory is itself that repo's root. - // When it sits below an enclosing repo, a plain checkIsRepo() is true and ktx - // would silently piggyback on the enclosing tree — but every ktx relative path - // (file-store writes, session worktrees, squash-merges, reindex scans) assumes - // this directory IS the working-tree root. So treat "inside an enclosing repo" - // the same as "no repo" and initialize a dedicated repo rooted here. - const isRepoRoot = await this.git.checkIsRepo(CheckRepoActions.IS_REPO_ROOT); + const ownership = await classifyKtxRepoOwnership(this.configDir); - if (!isRepoRoot) { - await this.git.init(); - this.logger.log('Initialized git repository'); + 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'); + } + // 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, @@ -130,6 +192,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/context/project/config.ts b/packages/cli/src/context/project/config.ts index fd7f482c..2c0b075d 100644 --- a/packages/cli/src/context/project/config.ts +++ b/packages/cli/src/context/project/config.ts @@ -230,14 +230,13 @@ const setupSchema = z const storageGitSchema = z .strictObject({ - auto_commit: z.boolean().default(true).describe('When true, KTX automatically commits state changes to the local Git-backed store.'), author: z .string() .min(1) .default('ktx ') - .describe('Git author identity used for auto-commits, in standard "Name " form.'), + .describe('Git author identity used for commits, in standard "Name " form.'), }) - .describe('Git-backed storage commit policy.'); + .describe('Git-backed storage author policy.'); const storageSchema = z .strictObject({ @@ -276,12 +275,6 @@ const agentSchema = z }) .describe('Agent feature configuration.'); -const memorySchema = z - .strictObject({ - auto_commit: z.boolean().default(true).describe('When true, KTX automatically commits memory updates to the Git-backed store.'), - }) - .describe('Memory subsystem configuration.'); - const ktxProjectConfigSchema = z .strictObject({ setup: setupSchema.optional().describe('Setup-wizard state. Written by `ktx setup`; may be omitted.'), @@ -293,7 +286,6 @@ const ktxProjectConfigSchema = z llm: llmSchema.prefault({}).describe('LLM provider, per-role model overrides, and prompt-caching tunables.'), ingest: ingestSchema.prefault({}).describe('Ingest pipeline configuration.'), agent: agentSchema.prefault({}).describe('Agent feature configuration.'), - memory: memorySchema.prefault({}).describe('Memory subsystem configuration.'), scan: scanSchema.prefault({}).describe('Schema-scan configuration: enrichment and relationship discovery.'), }) .describe('Configuration schema for KTX project files (ktx.yaml).'); diff --git a/packages/cli/src/demo-assets.ts b/packages/cli/src/demo-assets.ts index 5c84b356..ce746649 100644 --- a/packages/cli/src/demo-assets.ts +++ b/packages/cli/src/demo-assets.ts @@ -66,7 +66,6 @@ function demoConfig(databasePath: string): string { ' state: sqlite', ' search: sqlite-fts5', ' git:', - ' auto_commit: true', ' author: ktx ', 'llm:', ' provider:', 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/src/status-project.ts b/packages/cli/src/status-project.ts index ff7b98f4..569ec79f 100644 --- a/packages/cli/src/status-project.ts +++ b/packages/cli/src/status-project.ts @@ -78,7 +78,6 @@ interface PipelineStatus { interface StorageStatus { state: string; search: string; - gitAutoCommit: boolean; gitAuthor: string; } @@ -160,7 +159,6 @@ export interface ProjectStatus { nextActions: string[]; promptCaching?: { enabled: boolean; systemTtl?: string; toolsTtl?: string; historyTtl?: string }; workUnits?: { stepBudget: number; maxConcurrency: number; failureMode: string }; - memoryAutoCommit: boolean; relationshipsDetail?: { acceptThreshold: number; reviewThreshold: number; @@ -579,7 +577,6 @@ function buildStorageStatus(config: KtxProjectConfig): StorageStatus { return { state: config.storage.state, search: config.storage.search, - gitAutoCommit: config.storage.git.auto_commit, gitAuthor: config.storage.git.author, }; } @@ -986,7 +983,6 @@ export async function buildProjectStatus(project: KtxLocalProject, options: Buil maxConcurrency: config.ingest.workUnits.maxConcurrency, failureMode: config.ingest.workUnits.failureMode, }, - memoryAutoCommit: config.memory.auto_commit, relationshipsDetail: { acceptThreshold: config.scan.relationships.acceptThreshold, reviewThreshold: config.scan.relationships.reviewThreshold, @@ -1272,10 +1268,7 @@ export function renderProjectStatus(status: ProjectStatus, options: RenderProjec lines.push( ` ${bold('Agent')} ${dim(`max_iterations=${status.pipeline.agentMaxIterations}, tools=${status.pipeline.agentTools.join(', ') || '(none)'}`)}`, ); - lines.push(` ${bold('Memory')} ${dim(`auto_commit=${status.memoryAutoCommit}`)}`); - lines.push( - ` ${bold('Git')} ${dim(`auto_commit=${status.storage.gitAutoCommit}, author=${status.storage.gitAuthor}`)}`, - ); + lines.push(` ${bold('Git')} ${dim(`author=${status.storage.gitAuthor}`)}`); lines.push(''); } 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..f5493ad3 --- /dev/null +++ b/packages/cli/test/context/core/git.service.repo-isolation.test.ts @@ -0,0 +1,160 @@ +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 { classifyKtxRepoOwnership, 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'); + }); +}); + +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'); + }); + + it('reports unowned when the path is itself a file', async () => { + const filePath = join(tempDir, 'notes.txt'); + await writeFile(filePath, 'a file, not a folder\n', 'utf-8'); + expect(await classifyKtxRepoOwnership(filePath)).toBe('unowned'); + }); +}); diff --git a/packages/cli/test/context/core/git.service.test.ts b/packages/cli/test/context/core/git.service.test.ts index c07a5313..db60924a 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 { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; @@ -63,6 +64,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 () => { diff --git a/packages/cli/test/context/ingest/adapters/historic-sql/local-ingest-acceptance.test.ts b/packages/cli/test/context/ingest/adapters/historic-sql/local-ingest-acceptance.test.ts index e818f1c9..f3d70c0f 100644 --- a/packages/cli/test/context/ingest/adapters/historic-sql/local-ingest-acceptance.test.ts +++ b/packages/cli/test/context/ingest/adapters/historic-sql/local-ingest-acceptance.test.ts @@ -165,7 +165,6 @@ async function writeHistoricSqlProject(project: KtxLocalProject): Promise', '', ].join('\n'), diff --git a/packages/cli/test/context/ingest/local-bundle-ingest.test.ts b/packages/cli/test/context/ingest/local-bundle-ingest.test.ts index 6140dd10..75f6e387 100644 --- a/packages/cli/test/context/ingest/local-bundle-ingest.test.ts +++ b/packages/cli/test/context/ingest/local-bundle-ingest.test.ts @@ -521,7 +521,6 @@ describe('canonical local ingest', () => { ' state: sqlite', ' search: sqlite-fts5', ' git:', - ' auto_commit: false', ' author: KTX Test ', '', ].join('\n'), @@ -683,7 +682,6 @@ describe('canonical local ingest', () => { ' state: sqlite', ' search: sqlite-fts5', ' git:', - ' auto_commit: false', ' author: KTX Test ', '', ].join('\n'), @@ -767,7 +765,6 @@ describe('canonical local ingest', () => { ' state: sqlite', ' search: sqlite-fts5', ' git:', - ' auto_commit: false', ' author: KTX Test ', '', ].join('\n'), @@ -811,7 +808,6 @@ describe('canonical local ingest', () => { ' state: sqlite', ' search: sqlite-fts5', ' git:', - ' auto_commit: false', ' author: KTX Test ', '', ].join('\n'), diff --git a/packages/cli/test/context/project/config.test.ts b/packages/cli/test/context/project/config.test.ts index e5911a25..ef8c6681 100644 --- a/packages/cli/test/context/project/config.test.ts +++ b/packages/cli/test/context/project/config.test.ts @@ -7,6 +7,8 @@ import { validateKtxProjectConfig, } from '../../../src/context/project/config.js'; +const removedAutoCommitKey = ['auto', 'commit'].join('_'); + describe('KTX project config', () => { it.each(['status', 'replay', 'run', 'watch'])('accepts former ingest subcommand name "%s" as a connection id', (connectionId) => { expect( @@ -29,7 +31,6 @@ connections: state: 'sqlite', search: 'sqlite-fts5', git: { - auto_commit: true, author: 'ktx ', }, }, @@ -70,9 +71,6 @@ connections: default_toolset: ['sl_query', 'wiki_search', 'sl_read_source'], }, }, - memory: { - auto_commit: true, - }, scan: { enrichment: { mode: 'none', @@ -93,6 +91,28 @@ connections: }); }); + it('rejects removed auto-commit config keys', () => { + expect(() => + parseKtxProjectConfig(` +storage: + git: + ${removedAutoCommitKey}: false +`), + ).toThrow(new RegExp(`storage\\.git\\.${removedAutoCommitKey}`)); + + expect(() => + parseKtxProjectConfig(` +memory: + ${removedAutoCommitKey}: false +`), + ).toThrow(/memory/); + + expect(validateKtxProjectConfig(`storage:\n git:\n ${removedAutoCommitKey}: false\n`)).toMatchObject({ + ok: false, + issues: [expect.objectContaining({ path: `storage.git.${removedAutoCommitKey}` })], + }); + }); + it('round-trips through YAML with stable defaults', () => { const serialized = serializeKtxProjectConfig(buildDefaultKtxProjectConfig()); const parsed = parseKtxProjectConfig(serialized); @@ -595,7 +615,7 @@ describe('generateKtxProjectConfigJsonSchema', () => { it('exposes every top-level ktx.yaml section under properties', () => { const properties = schema.properties as Record; - expect(Object.keys(properties).sort()).toEqual(['agent', 'connections', 'ingest', 'llm', 'memory', 'scan', 'setup', 'storage'].sort()); + expect(Object.keys(properties).sort()).toEqual(['agent', 'connections', 'ingest', 'llm', 'scan', 'setup', 'storage'].sort()); }); it('does not require any top-level fields', () => { diff --git a/packages/cli/test/demo-assets.test.ts b/packages/cli/test/demo-assets.test.ts index 80573d3d..bf7cd9ba 100644 --- a/packages/cli/test/demo-assets.test.ts +++ b/packages/cli/test/demo-assets.test.ts @@ -13,6 +13,7 @@ import { } from '../src/demo-assets.js'; const packagedDemoSource = 'packaged-orbit-demo'; +const removedAutoCommitKey = ['auto', 'commit'].join('_'); function packagedDemoAssetPath(relativePath: string): string { return fileURLToPath(new URL(`../assets/demo/orbit/${relativePath}`, import.meta.url)); @@ -133,6 +134,8 @@ describe('demo assets', () => { const config = await readFile(join(projectDir, 'ktx.yaml'), 'utf-8'); expect(config).toContain('backend: anthropic'); expect(config).toContain('api_key: env:ANTHROPIC_API_KEY'); + expect(config).not.toContain(removedAutoCommitKey); + expect(config).not.toContain('memory:'); expect(config).not.toContain('sk-ant-'); }); diff --git a/packages/cli/test/setup-project.test.ts b/packages/cli/test/setup-project.test.ts index c77a2080..1f0eae4d 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,59 @@ 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('rejects a custom path that points at an existing file without crashing', async () => { + const startDir = join(tempDir, 'start'); + await mkdir(startDir, { recursive: true }); + await writeFile(join(startDir, 'notes.txt'), 'a file, not a folder\n', 'utf-8'); + const prompts = makePromptAdapter({ choices: ['new-custom'], textValue: 'notes.txt' }); + const testIo = makeIo({ stdoutIsTty: true }); + + await expect( + runKtxSetupProjectStep( + { projectDir: startDir, mode: 'auto', inputMode: 'auto', yes: false }, + testIo.io, + { prompts }, + ), + ).resolves.toMatchObject({ status: 'missing-input', projectDir: startDir }); + + expect(testIo.stderr()).toContain('exists and is not a directory'); + }); + it('prompts to exit and returns cancelled in interactive auto mode', async () => { const projectDir = join(tempDir, 'warehouse'); const prompts = makePromptAdapter({ choice: 'exit' }); diff --git a/packages/cli/test/status-project.test.ts b/packages/cli/test/status-project.test.ts index cd63cf19..30313897 100644 --- a/packages/cli/test/status-project.test.ts +++ b/packages/cli/test/status-project.test.ts @@ -60,6 +60,7 @@ function baseProjectConfig(): KtxProjectConfig { } const stubClaudeCodeAuthProbe = async () => ({ ok: true as const }); +const removedAutoCommitKey = ['auto', 'commit'].join('_'); describe('buildProjectStatus embeddings', () => { it('reports sentence-transformers with explicit base_url as ok', async () => { @@ -402,6 +403,19 @@ describe('buildProjectStatus --fast', () => { }); }); +describe('renderProjectStatus config cleanup', () => { + it('does not render removed auto-commit status fields in verbose output', async () => { + const project = projectWithConfig(baseProjectConfig()); + const status = await buildProjectStatus(project, { + claudeCodeAuthProbe: stubClaudeCodeAuthProbe, + }); + const rendered = renderProjectStatus(status, { verbose: true, useColor: false }); + + expect(rendered).not.toContain(removedAutoCommitKey); + expect(rendered).toContain('author=ktx '); + }); +}); + describe('buildProjectStatus codex', () => { it('reports authenticated local Codex session', async () => { const project = projectWithConfig(withCodexLlm(buildDefaultKtxProjectConfig())); diff --git a/packages/cli/test/telemetry/project-snapshot.test.ts b/packages/cli/test/telemetry/project-snapshot.test.ts index ce58f40e..82dd88f1 100644 --- a/packages/cli/test/telemetry/project-snapshot.test.ts +++ b/packages/cli/test/telemetry/project-snapshot.test.ts @@ -66,10 +66,9 @@ describe('buildProjectStackSnapshotFields', () => { storage: { state: 'sqlite', search: 'sqlite-fts5', - git: { auto_commit: true, author: 'ktx ' }, + git: { author: 'ktx ' }, }, agent: { run_research: { enabled: false, max_iterations: 20, default_toolset: [] } }, - memory: { auto_commit: true }, }, });