diff --git a/AGENTS.md b/AGENTS.md index ffe324bb..6f6dec86 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -263,6 +263,26 @@ and route ingest, setup, memory, indexing, and docs through it. Do not add an `auto_commit`-style switch unless the user explicitly asks for staged-only runs and accepts the extra runtime path. +## Code Comments + +Code must be self-explanatory. A comment exists only to state a constraint the +code cannot show; everything else belongs in the PR description or nowhere. + +- **MUST**: Keep each comment to 1-3 lines stating only what the code cannot + show: a cross-file invariant ("error-severity issues never reach here — the + doctor exits on them first"), a required ordering ("ktx.yaml is written + before git init, so a crash cannot leave a bare `.git`"), or a library quirk + ("zod reports unknown record keys as `invalid_key`"). +- **MUST**: State each invariant once, at the public entry point. Do not repeat + the same guarantee across a helper, its wrapper, and the call site. +- **MUST NOT**: Write prose comment blocks — design rationale, alternatives + considered, change narration ("is now written before…"), caller enumerations + ("shared by X, Y, and Z"), or restatements of what the code already shows. + That is the author addressing the reviewer, and it rots once merged. +- **MAY**: Open a regression test with a 1-3 line comment stating the scenario + it guards when the test name cannot carry it. Omit design history and + references to removed designs. + ## TypeScript Standards - Use Node 22+ and pnpm workspace commands. diff --git a/docs-site/content/docs/cli-reference/ktx-status.mdx b/docs-site/content/docs/cli-reference/ktx-status.mdx index 66e4964c..7bf498be 100644 --- a/docs-site/content/docs/cli-reference/ktx-status.mdx +++ b/docs-site/content/docs/cli-reference/ktx-status.mdx @@ -94,6 +94,6 @@ stats, and are always shown (they do not require external communication). |-------|-------|----------| | No **ktx** project found | Current directory has no `ktx.yaml` and `KTX_PROJECT_DIR` is unset | `ktx status` runs setup checks; run from a **ktx** project or set `KTX_PROJECT_DIR` for project checks | | Project config check fails | The project directory is missing or has an invalid `ktx.yaml` | Run `ktx setup` to resume setup | -| Schema validation fails | `ktx.yaml` does not match the current config schema | Run `ktx status --validate --json` for structured issue details, then edit `ktx.yaml` or rerun `ktx setup` | +| Schema validation fails | A field **ktx** recognizes has an invalid value. Unrecognized keys are reported as non-blocking warnings (exit `0`), not failures | Run `ktx status --validate --json` for structured issue details, then edit `ktx.yaml` or rerun `ktx setup` | | Semantic search check warns | Embeddings are not configured or the provider probe failed | Run `ktx setup` or inspect the check's `fix` field in JSON output | | Query history check warns | A database has query history enabled but the warehouse prerequisites are missing | Fix the warehouse extension, grants, or history access, then rerun `ktx status` | diff --git a/docs-site/content/docs/configuration/ktx-yaml.mdx b/docs-site/content/docs/configuration/ktx-yaml.mdx index 9e985eba..1c28b50a 100644 --- a/docs-site/content/docs/configuration/ktx-yaml.mdx +++ b/docs-site/content/docs/configuration/ktx-yaml.mdx @@ -666,11 +666,21 @@ agent: ## Validating your config -**ktx** validates `ktx.yaml` strictly: unknown keys at the top level or inside -strict blocks cause setup and CLI commands to fail with a precise path -(`scan.relationships.acceptThreshhold: Unrecognized key`). Warehouse -connections accept extra driver-specific fields, so passthrough values like -`historicSql` and `context.queryHistory` are allowed. +**ktx** validates `ktx.yaml` when it loads, and treats two kinds of problems +differently: + +- **An invalid value on a field ktx recognizes** (for example + `llm.provider.backend: nope`) is a hard error. Setup and CLI commands stop and + report the exact path so you can fix it. +- **An unrecognized key** — one left over from a different **ktx** version, or a + typo such as `scan.relationships.acceptThreshhold` — is tolerated, not fatal. + **ktx** ignores the key and keeps running, so a misspelled field quietly falls + back to its default instead of taking effect. `ktx status` lists each ignored + key as a warning (and exits `0`) so you can remove or correct it when + convenient. + +Warehouse connections accept extra driver-specific fields, so passthrough values +like `historicSql` and `context.queryHistory` are allowed. To re-validate without running anything else: diff --git a/packages/cli/src/context/core/git.service.ts b/packages/cli/src/context/core/git.service.ts index 3095ad00..febd9277 100644 --- a/packages/cli/src/context/core/git.service.ts +++ b/packages/cli/src/context/core/git.service.ts @@ -27,11 +27,9 @@ 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 { +export class KtxForeignGitRepositoryError extends Error { constructor(configDir: string) { super( `${configDir} is already a git repository that ktx did not create. ` + @@ -46,21 +44,16 @@ function isNodeErrnoException(error: unknown): error is NodeJS.ErrnoException { } /** - * Classify whether ktx may own a git repository rooted exactly at `dir`. + * Classify whether ktx may own a git repository rooted exactly at `dir`. A root + * `ktx.yaml` is the ownership signal; the working tree decides, not git history, + * because older ktx versions left `ktx.yaml` uncommitted (it holds secret refs). * - * - `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. + * - `unowned`: no repo here (including a missing or non-directory path) → ktx may `git init`. + * - `ktx-managed`: `/.git` is a directory and `ktx.yaml` sits at the root. + * - `foreign`: any other repo — no root `ktx.yaml`, 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. + * Reads only `` itself; never walks up, so a parent repo cannot change the answer. */ export async function classifyKtxRepoOwnership(dir: string): Promise { let dotGitIsDirectory: boolean; @@ -78,13 +71,9 @@ export async function classifyKtxRepoOwnership(dir: string): Promise): unknown { return cursor; } -function formatIssue(issue: z.core.$ZodIssue, input: unknown): KtxConfigIssue[] { - const basePath = dottedPath(issue.path); +interface UnknownKeyLocation { + containerPath: ReadonlyArray; + key: string; +} +/** + * Zod reports unknown keys in two shapes: strict objects emit + * `unrecognized_keys` (path → container, `keys` → offenders), enum-keyed + * records (`llm.models`) emit one `invalid_key` per offender (path ends with + * the key). Normalize both so the warning report and the strip always agree. + */ +function unknownKeyLocations(issue: z.core.$ZodIssue): UnknownKeyLocation[] { if (issue.code === 'unrecognized_keys') { - const keys = (issue as { keys?: readonly string[] }).keys ?? []; - return keys.map((key) => { - const fullPath = basePath.length > 0 ? `${basePath}.${key}` : key; - return { path: fullPath, message: `Unsupported ${fullPath}: unknown field` }; + return issue.keys.map((key) => ({ containerPath: issue.path, key })); + } + if (issue.code === 'invalid_key' && issue.path.length > 0) { + return [ + { + containerPath: issue.path.slice(0, -1), + key: String(issue.path[issue.path.length - 1]), + }, + ]; + } + return []; +} + +function formatIssue(issue: z.core.$ZodIssue, input: unknown): KtxConfigIssue[] { + const unknownKeys = unknownKeyLocations(issue); + if (unknownKeys.length > 0) { + return unknownKeys.map(({ containerPath, key }) => { + const base = dottedPath(containerPath); + const fullPath = base.length > 0 ? `${base}.${key}` : key; + return { + path: fullPath, + message: `Unsupported ${fullPath}: unknown field (ignored)`, + fix: 'Unknown to this ktx version; it is ignored. Delete it from ktx.yaml when convenient.', + severity: 'warning', + }; }); } + const basePath = dottedPath(issue.path); const lastSegment = issue.path[issue.path.length - 1]; if (lastSegment === 'backend' && (issue.code === 'invalid_value' || issue.code === 'invalid_type')) { const value = valueAtPath(input, issue.path); - return [{ path: basePath, message: `Unsupported ${basePath}: ${String(value)}` }]; + return [{ path: basePath, message: `Unsupported ${basePath}: ${String(value)}`, severity: 'error' }]; } - return [{ path: basePath, message: basePath.length > 0 ? `${basePath}: ${issue.message}` : issue.message }]; + return [ + { + path: basePath, + message: basePath.length > 0 ? `${basePath}: ${issue.message}` : issue.message, + severity: 'error', + }, + ]; } function collectIssues(error: z.ZodError, input: unknown): KtxConfigIssue[] { @@ -359,16 +401,45 @@ export function buildDefaultKtxProjectConfig(): KtxProjectConfig { return ktxProjectConfigSchema.parse({}); } +function stripUnrecognizedKeys(input: Record): Record { + const result = ktxProjectConfigSchema.safeParse(input); + if (result.success) { + return input; + } + const unknownKeys = result.error.issues.flatMap(unknownKeyLocations); + if (unknownKeys.length === 0) { + return input; + } + const value = structuredClone(input); + for (const { containerPath, key } of unknownKeys) { + const container = valueAtPath(value, containerPath); + if (container === null || typeof container !== 'object') continue; + delete (container as Record)[key]; + } + return value; +} + +function parseTolerant(input: Record): KtxProjectConfig { + const value = stripUnrecognizedKeys(input); + const result = ktxProjectConfigSchema.safeParse(value); + if (!result.success) { + throw new Error(formatZodError(result.error, value)); + } + return result.data; +} + +/** + * Parse and validate a ktx.yaml document. Keys this ktx version does not + * recognize are stripped from the returned config — never from the file, which + * a load must not rewrite — so a config written by a different ktx version + * still loads. Malformed values on recognized fields still throw. + */ export function parseKtxProjectConfig(raw: string): KtxProjectConfig { const parsed = YAML.parse(raw) as unknown; if (!isRecord(parsed)) { throw new Error('ktx.yaml must contain a YAML object'); } - const result = ktxProjectConfigSchema.safeParse(parsed); - if (!result.success) { - throw new Error(formatZodError(result.error, parsed)); - } - return result.data; + return parseTolerant(parsed); } export function validateKtxProjectConfig(raw: string): KtxConfigValidation { @@ -377,16 +448,18 @@ export function validateKtxProjectConfig(raw: string): KtxConfigValidation { parsed = YAML.parse(raw); } catch (error) { const message = error instanceof Error ? error.message : String(error); - return { ok: false, issues: [{ path: '', message: `ktx.yaml parse error: ${message}` }] }; + return { ok: false, issues: [{ path: '', message: `ktx.yaml parse error: ${message}`, severity: 'error' }] }; } if (!isRecord(parsed)) { - return { ok: false, issues: [{ path: '', message: 'ktx.yaml must contain a YAML object' }] }; + return { ok: false, issues: [{ path: '', message: 'ktx.yaml must contain a YAML object', severity: 'error' }] }; } const result = ktxProjectConfigSchema.safeParse(parsed); if (result.success) { return { ok: true, issues: [] }; } - return { ok: false, issues: collectIssues(result.error, parsed) }; + const issues = collectIssues(result.error, parsed); + const ok = !issues.some((issue) => issue.severity === 'error'); + return { ok, issues }; } export function generateKtxProjectConfigJsonSchema(): Record { diff --git a/packages/cli/src/context/project/project.ts b/packages/cli/src/context/project/project.ts index 6ca27d10..156b200c 100644 --- a/packages/cli/src/context/project/project.ts +++ b/packages/cli/src/context/project/project.ts @@ -1,6 +1,6 @@ import { promises as fs } from 'node:fs'; import { basename, dirname, join, resolve } from 'node:path'; -import { GitService } from '../../context/core/git.service.js'; +import { classifyKtxRepoOwnership, GitService, KtxForeignGitRepositoryError } from '../../context/core/git.service.js'; import { type KtxCoreConfig, type KtxLogger, noopLogger } from '../../context/core/config.js'; import type { KtxProjectConfig } from './config.js'; import { buildDefaultKtxProjectConfig, parseKtxProjectConfig, serializeKtxProjectConfig } from './config.js'; @@ -112,14 +112,24 @@ export async function initKtxProject(options: InitKtxProjectOptions): Promise file.path)], diff --git a/packages/cli/src/doctor.ts b/packages/cli/src/doctor.ts index 8b32b527..9ef6dcb3 100644 --- a/packages/cli/src/doctor.ts +++ b/packages/cli/src/doctor.ts @@ -481,12 +481,18 @@ export function renderInvalidConfigMessage( const status = (s: DoctorStatus, text: string) => styleStatus(useColor, s, text); const abbreviated = abbreviateHome(projectDir) ?? projectDir; + const errorCount = issues.filter((issue) => issue.severity === 'error').length; + const warningCount = issues.length - errorCount; + const lines: string[] = []; lines.push(`${bold('ktx status')} ${dim('·')} ${abbreviated}`); lines.push(''); - lines.push(` ${status('fail', '✗')} ${bold('Config')} ktx.yaml has ${issues.length} schema issue${issues.length === 1 ? '' : 's'}`); + lines.push( + ` ${status('fail', '✗')} ${bold('Config')} ktx.yaml has ${errorCount} schema issue${errorCount === 1 ? '' : 's'}${warningCount > 0 ? ` · ${warningCount} ignored field${warningCount === 1 ? '' : 's'}` : ''}`, + ); for (const issue of issues) { - lines.push(` ${status('fail', '✗')} ${issue.message}`); + const glyph = issue.severity === 'error' ? status('fail', '✗') : status('warn', '⚠'); + lines.push(` ${glyph} ${issue.message}`); if (issue.fix) { lines.push(` ${dim(`→ ${issue.fix}`)}`); } @@ -502,6 +508,7 @@ export function renderValidConfigMessage( projectDir: string, outputMode: KtxDoctorOutputMode, io: KtxDoctorIo, + warnings: KtxConfigIssue[] = [], ): void { if (outputMode === 'json') { io.stdout.write( @@ -509,6 +516,7 @@ export function renderValidConfigMessage( { ok: true, projectDir, + ...(warnings.length > 0 ? { warnings } : {}), }, null, 2, @@ -526,7 +534,19 @@ export function renderValidConfigMessage( const lines: string[] = []; lines.push(`${bold('ktx status')} ${dim('·')} ${abbreviated}`); lines.push(''); - lines.push(` ${status('pass', '✓')} ${bold('Config')} ${dim('ktx.yaml schema valid')}`); + if (warnings.length > 0) { + lines.push( + ` ${status('warn', '⚠')} ${bold('Config')} ktx.yaml schema valid · ${warnings.length} ignored field${warnings.length === 1 ? '' : 's'}`, + ); + for (const warning of warnings) { + lines.push(` ${status('warn', '⚠')} ${warning.message}`); + if (warning.fix) { + lines.push(` ${dim(`→ ${warning.fix}`)}`); + } + } + } else { + lines.push(` ${status('pass', '✓')} ${bold('Config')} ${dim('ktx.yaml schema valid')}`); + } lines.push(''); io.stdout.write(lines.join('\n')); @@ -589,14 +609,14 @@ export async function runKtxDoctor( renderMissingProjectMessage(args.projectDir, args.outputMode, io); return 1; } - const { validateKtxProjectConfig } = await import('./context/project/config.js');; + const { validateKtxProjectConfig } = await import('./context/project/config.js'); const rawConfig = await readFile(configPath, 'utf-8'); const validation = validateKtxProjectConfig(rawConfig); if (!validation.ok) { renderInvalidConfigMessage(args.projectDir, validation.issues, args.outputMode, io); return 1; } - renderValidConfigMessage(args.projectDir, args.outputMode, io); + renderValidConfigMessage(args.projectDir, args.outputMode, io, validation.issues); return 0; } @@ -607,7 +627,7 @@ export async function runKtxDoctor( return 1; } const { loadKtxProject } = await import('./context/project/project.js'); - const { validateKtxProjectConfig } = await import('./context/project/config.js');; + const { validateKtxProjectConfig } = await import('./context/project/config.js'); const { buildProjectStatus, renderProjectStatus } = await import('./status-project.js'); const rawConfig = await readFile(configPath, 'utf-8'); const validation = validateKtxProjectConfig(rawConfig); diff --git a/packages/cli/src/status-project.ts b/packages/cli/src/status-project.ts index 3a9dc1ee..c3faa63d 100644 --- a/packages/cli/src/status-project.ts +++ b/packages/cli/src/status-project.ts @@ -721,9 +721,10 @@ function buildConfigStatus(issues: KtxConfigIssue[] | undefined): ConfigStatus { if (list.length === 0) { return { status: 'ok', detail: 'ktx.yaml schema valid', issues: [] }; } + // Error-severity issues never reach here — the doctor exits on them first. return { status: 'warn', - detail: `${list.length} issue${list.length === 1 ? '' : 's'} in ktx.yaml`, + detail: `ktx.yaml schema valid · ${list.length} ignored field${list.length === 1 ? '' : 's'}`, issues: list, }; } 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 0b3347a8..b84b9aba 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,10 +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 bootstrapping a marked ktx repo on a machine with no configured +// Regression for bootstrapping a ktx-owned 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. +// this test covers the still-valid path where the repo is already ktx's own +// (root ktx.yaml present) but has no HEAD yet. describe('GitService.initialize without a configured git identity', () => { let repoDir: string; let homeDir: string; @@ -58,11 +58,7 @@ describe('GitService.initialize without a configured git identity', () => { } execFileSync('git', ['init'], { cwd: repoDir, env: process.env, stdio: 'ignore' }); - execFileSync('git', ['config', '--local', 'ktx.managed', 'true'], { - cwd: repoDir, - env: process.env, - stdio: 'ignore', - }); + await writeFile(join(repoDir, 'ktx.yaml'), 'connections: {}\n', 'utf-8'); }); 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 index f5493ad3..569750c8 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 @@ -56,10 +56,11 @@ describe('GitService repository ownership', () => { git(parentDir, ['commit', '-m', 'parent baseline']); const parentHeadBefore = git(parentDir, ['rev-parse', 'HEAD']); + await writeFile(join(projectDir, 'ktx.yaml'), 'connections: {}\n', 'utf-8'); const service = new GitService(coreConfig(projectDir)); await service.onModuleInit(); - expect(git(projectDir, ['config', '--local', '--get', 'ktx.managed'])).toBe('true'); + expect(await classifyKtxRepoOwnership(projectDir)).toBe('ktx-managed'); expect(git(parentDir, ['rev-parse', 'HEAD'])).toBe(parentHeadBefore); expect(await realpath(git(projectDir, ['rev-parse', '--show-toplevel']))).toBe(await realpath(projectDir)); @@ -83,18 +84,22 @@ describe('GitService repository ownership', () => { expect(await readFile(join(projectDir, '.git', 'config'), 'utf-8')).toBe(configBefore); }); - it('rejects a gitfile at the project dir as foreign', async () => { + it('rejects a gitfile at the project dir as foreign even when a ktx.yaml sits beside it', async () => { + // A linked worktree is never ktx's own repo, whatever files live in it. const projectDir = join(tempDir, 'linked-worktree'); await mkdir(projectDir, { recursive: true }); await writeFile(join(projectDir, '.git'), 'gitdir: ../actual.git\n', 'utf-8'); + await writeFile(join(projectDir, 'ktx.yaml'), 'connections: {}\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 () => { + it('re-initializes an existing ktx project repo without a second bootstrap commit', async () => { const projectDir = join(tempDir, 'owned'); + await mkdir(projectDir, { recursive: true }); + await writeFile(join(projectDir, 'ktx.yaml'), 'connections: {}\n', 'utf-8'); const service = new GitService(coreConfig(projectDir)); await service.onModuleInit(); const before = await service.revParseHead(); @@ -103,7 +108,37 @@ describe('GitService repository ownership', () => { await second.onModuleInit(); expect(await second.revParseHead()).toBe(before); - expect(git(projectDir, ['config', '--local', '--get', 'ktx.managed'])).toBe('true'); + }); + + it('accepts a project created by an older ktx: repo history plus an untracked root ktx.yaml', async () => { + // Older projects have ktx commit history and an uncommitted root ktx.yaml + // (it holds secret refs); the on-disk file is still the ownership signal. + const projectDir = join(tempDir, 'legacy'); + await mkdir(join(projectDir, '.ktx'), { recursive: true }); + git(projectDir, ['init']); + await writeFile(join(projectDir, '.ktx', '.gitignore'), 'secrets/\n', 'utf-8'); + git(projectDir, ['add', '.ktx/.gitignore']); + git(projectDir, ['commit', '-m', 'Initialize KTX project: legacy']); + await writeFile(join(projectDir, 'ktx.yaml'), 'connections: {}\n', 'utf-8'); + const headBefore = git(projectDir, ['rev-parse', 'HEAD']); + + const service = new GitService(coreConfig(projectDir)); + await expect(service.onModuleInit()).resolves.toBeUndefined(); + + expect(await service.revParseHead()).toBe(headBefore); + expect(git(projectDir, ['status', '--short'])).toContain('?? ktx.yaml'); + }); + + it('still rejects a user repo with history but no root ktx.yaml', async () => { + const projectDir = join(tempDir, 'app-repo'); + await mkdir(projectDir, { recursive: true }); + git(projectDir, ['init']); + await writeFile(join(projectDir, 'README.md'), '# App\n', 'utf-8'); + git(projectDir, ['add', 'README.md']); + git(projectDir, ['commit', '-m', 'app baseline']); + + const service = new GitService(coreConfig(projectDir)); + await expect(service.onModuleInit()).rejects.toThrow(/already a git repository that ktx did not create/); }); }); @@ -132,9 +167,11 @@ describe('classifyKtxRepoOwnership', () => { expect(await classifyKtxRepoOwnership(nestedDir)).toBe('unowned'); }); - it('reports ktx-managed for a repo ktx initialized', async () => { + it('reports ktx-managed for a repo with a root ktx.yaml (even untracked)', async () => { const dir = join(tempDir, 'owned'); - await new GitService(coreConfig(dir)).onModuleInit(); + await mkdir(dir, { recursive: true }); + git(dir, ['init']); + await writeFile(join(dir, 'ktx.yaml'), 'connections: {}\n', 'utf-8'); expect(await classifyKtxRepoOwnership(dir)).toBe('ktx-managed'); }); @@ -145,6 +182,16 @@ describe('classifyKtxRepoOwnership', () => { expect(await classifyKtxRepoOwnership(dir)).toBe('foreign'); }); + it('reports foreign for a non-ktx repo that has commits but no ktx.yaml', async () => { + const dir = join(tempDir, 'app'); + await mkdir(dir, { recursive: true }); + git(dir, ['init']); + await writeFile(join(dir, 'README.md'), '# App\n', 'utf-8'); + git(dir, ['add', 'README.md']); + git(dir, ['commit', '-m', 'baseline']); + 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 }); diff --git a/packages/cli/test/context/core/git.service.test.ts b/packages/cli/test/context/core/git.service.test.ts index db60924a..e4bc2d5c 100644 --- a/packages/cli/test/context/core/git.service.test.ts +++ b/packages/cli/test/context/core/git.service.test.ts @@ -1,4 +1,3 @@ -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'; @@ -27,8 +26,12 @@ describe('GitService', () => { }, }; + // Mirror production: initKtxProject writes ktx.yaml before the git repo is + // initialized (the root ktx.yaml is the ownership signal) and commits it. + await writeFile(join(tempDir, 'ktx.yaml'), 'connections: {}\n', 'utf-8'); service = new GitService(coreConfig); await service.onModuleInit(); + await service.commitFile('ktx.yaml', 'Initialize KTX project', 'Test', 'test@example.com'); }); afterEach(async () => { @@ -61,14 +64,9 @@ describe('GitService', () => { describe('cold-start bootstrap commit', () => { it('writes an empty commit on init so HEAD always resolves', async () => { - // beforeEach already ran onModuleInit() against an empty temp dir. + // beforeEach already ran onModuleInit() against a fresh 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/project/config.test.ts b/packages/cli/test/context/project/config.test.ts index 3fa2cb8e..47cbee2a 100644 --- a/packages/cli/test/context/project/config.test.ts +++ b/packages/cli/test/context/project/config.test.ts @@ -91,25 +91,61 @@ connections: }); }); - it('rejects removed auto-commit config keys', () => { - expect(() => - parseKtxProjectConfig(` + it('tolerates unrecognized keys left over from older ktx versions', () => { + // A project written by an older ktx still carries fields that newer ktx + // removed (storage.git.auto_commit, the top-level memory block). Loading + // must not brick every command — the keys are dropped, not rejected. + const config = parseKtxProjectConfig(` storage: git: ${removedAutoCommitKey}: false -`), - ).toThrow(new RegExp(`storage\\.git\\.${removedAutoCommitKey}`)); - - expect(() => - parseKtxProjectConfig(` memory: ${removedAutoCommitKey}: false -`), - ).toThrow(/memory/); +`); + expect(config.storage.git).toEqual({ author: 'ktx ' }); + expect(config).not.toHaveProperty('memory'); + }); - expect(validateKtxProjectConfig(`storage:\n git:\n ${removedAutoCommitKey}: false\n`)).toMatchObject({ + it('reports dropped keys as warnings, not blocking errors', () => { + const validation = validateKtxProjectConfig( + `storage:\n git:\n ${removedAutoCommitKey}: false\nmemory:\n ${removedAutoCommitKey}: false\n`, + ); + expect(validation.ok).toBe(true); + expect(validation.issues).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: `storage.git.${removedAutoCommitKey}`, severity: 'warning' }), + expect.objectContaining({ path: 'memory', severity: 'warning' }), + ]), + ); + }); + + it('tolerates llm.models roles this ktx version does not define', () => { + // Enum-keyed record entries surface as zod `invalid_key`, not + // `unrecognized_keys` — a distinct path from unknown object fields. + const config = parseKtxProjectConfig(` +llm: + models: + default: claude-sonnet-4-6 + summarizer_from_the_future: some-model +`); + expect(config.llm.models).toEqual({ default: 'claude-sonnet-4-6' }); + + const validation = validateKtxProjectConfig( + 'llm:\n models:\n default: claude-sonnet-4-6\n summarizer_from_the_future: some-model\n', + ); + expect(validation.ok).toBe(true); + expect(validation.issues).toEqual([ + expect.objectContaining({ path: 'llm.models.summarizer_from_the_future', severity: 'warning' }), + ]); + }); + + it('still rejects malformed values on recognized fields', () => { + // Tolerance is only for unknown keys. A bad value on a known field is a + // real misconfiguration and must still fail loudly. + expect(() => parseKtxProjectConfig('storage:\n state: mariadb\n')).toThrow(/storage\.state/); + expect(validateKtxProjectConfig('storage:\n state: mariadb\n')).toMatchObject({ ok: false, - issues: [expect.objectContaining({ path: `storage.git.${removedAutoCommitKey}` })], + issues: [expect.objectContaining({ path: 'storage.state', severity: 'error' })], }); }); @@ -471,41 +507,34 @@ scan: expect(() => parseKtxProjectConfig(yaml)).toThrow(/scan\.relationships\.validationBudget/); }); - it('rejects unsupported local LLM and embedding fields', () => { + it('tolerates unsupported nested fields and surfaces them as warnings', () => { + // Unknown nested keys (whether obsolete or a typo) are dropped rather than + // bricking the command; ktx status surfaces them via validate warnings. expect(() => parseKtxProjectConfig(` ingest: llm: backend: anthropic `), - ).toThrow('Unsupported ingest.llm: unknown field'); + ).not.toThrow(); - expect(() => - parseKtxProjectConfig(` + const validation = validateKtxProjectConfig(` +ingest: + llm: + backend: anthropic scan: enrichment: backend: gateway -`), - ).toThrow('Unsupported scan.enrichment.backend: unknown field'); - - expect(() => - parseKtxProjectConfig(` -scan: - enrichment: - mode: llm - llm: - backend: gateway -`), - ).toThrow('Unsupported scan.enrichment.llm: unknown field'); - - expect(() => - parseKtxProjectConfig(` -ingest: - embeddings: - provider: gateway - max_batch_size: 32 -`), - ).toThrow('Unsupported ingest.embeddings.provider'); +ingest_embeddings_typo: + provider: gateway +`); + expect(validation.ok).toBe(true); + expect(validation.issues).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: 'ingest.llm', severity: 'warning' }), + expect.objectContaining({ path: 'scan.enrichment.backend', severity: 'warning' }), + ]), + ); }); it('rejects gateway embedding configs', () => { @@ -552,13 +581,19 @@ scan: }); }); - it('rejects unknown top-level fields under strict mode', () => { + it('tolerates an unknown top-level field but warns about it', () => { + // A typo like `storrage` no longer bricks every command; it is dropped and + // reported as a warning so the user can notice the setting did not apply. expect(() => parseKtxProjectConfig(` storrage: state: sqlite `), - ).toThrow(/Unsupported storrage/); + ).not.toThrow(); + + const validation = validateKtxProjectConfig('storrage:\n state: sqlite\n'); + expect(validation.ok).toBe(true); + expect(validation.issues).toEqual([expect.objectContaining({ path: 'storrage', severity: 'warning' })]); }); }); @@ -598,7 +633,7 @@ scan: const result = validateKtxProjectConfig('- nope\n'); expect(result).toEqual({ ok: false, - issues: [{ path: '', message: 'ktx.yaml must contain a YAML object' }], + issues: [{ path: '', message: 'ktx.yaml must contain a YAML object', severity: 'error' }], }); }); }); diff --git a/packages/cli/test/context/project/project.test.ts b/packages/cli/test/context/project/project.test.ts index 253a4cf1..b4428a19 100644 --- a/packages/cli/test/context/project/project.test.ts +++ b/packages/cli/test/context/project/project.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process'; -import { mkdir, mkdtemp, readFile, realpath, rm, stat } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, realpath, rm, stat, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -61,6 +61,31 @@ describe('ktx local project runtime', () => { }); }); + it('loads a ktx.yaml carrying fields removed in a newer ktx without mutating it on disk', async () => { + const projectDir = join(tempDir, 'warehouse'); + await initKtxProject({ projectDir }); + + // Simulate a project written by a different ktx: inject unknown fields into + // the existing storage.git block and as a top-level memory block. + const configPath = join(projectDir, 'ktx.yaml'); + const original = await readFile(configPath, 'utf-8'); + const withStaleKeys = `${original.replace( + 'author: ktx ', + 'auto_commit: true\n author: ktx ', + )}memory:\n auto_commit: true\n`; + await writeFile(configPath, withStaleKeys, 'utf-8'); + + const loaded = await loadKtxProject({ projectDir }); + + // Loading tolerates the unknown fields instead of throwing: they are stripped + // from the in-memory config so every command still runs. + expect(loaded.config).not.toHaveProperty('memory'); + expect(loaded.config.storage.git).toEqual({ author: 'ktx ' }); + + // The file on disk stays exactly as the user wrote it. + await expect(readFile(configPath, 'utf-8')).resolves.toBe(withStaleKeys); + }); + it('initializes a dedicated git repo at the project dir even when nested inside an enclosing repo', async () => { // A ktx project dir living below an existing git working tree (e.g. an analytics // subfolder of an app repo). ktx must own its own repo rooted at the project dir, @@ -95,4 +120,40 @@ describe('ktx local project runtime', () => { configPath: join(projectDir, 'ktx.yaml'), }); }); + + it('refuses to initialize inside a foreign git repo and writes nothing into it', async () => { + // A user's own repo: has history, no root ktx.yaml. The guard must reject + // before writing ktx.yaml — that file would make the repo classify as ktx's. + const projectDir = join(tempDir, 'app-repo'); + await mkdir(projectDir, { recursive: true }); + execFileSync('git', ['init', '-q'], { cwd: projectDir }); + await writeFile(join(projectDir, 'README.md'), '# App\n', 'utf-8'); + execFileSync('git', ['add', 'README.md'], { cwd: projectDir }); + execFileSync( + 'git', + ['-c', 'user.name=App', '-c', 'user.email=app@example.com', 'commit', '-q', '-m', 'baseline'], + { cwd: projectDir }, + ); + + await expect(initKtxProject({ projectDir })).rejects.toThrow( + /already a git repository that ktx did not create/, + ); + + await expect(stat(join(projectDir, 'ktx.yaml'))).rejects.toMatchObject({ code: 'ENOENT' }); + const tracked = execFileSync('git', ['ls-files'], { cwd: projectDir, encoding: 'utf-8' }); + expect(tracked).not.toContain('ktx.yaml'); + }); + + it('recovers an init interrupted after ktx.yaml was written but before git finished', async () => { + // ktx.yaml is written before git init, so the only crash residue is a valid + // ktx.yaml with no `.git` — the next load must re-init, not reject as foreign. + const projectDir = join(tempDir, 'half-init'); + await initKtxProject({ projectDir }); + await rm(join(projectDir, '.git'), { recursive: true, force: true }); + + const loaded = await loadKtxProject({ projectDir }); + + await expect(stat(join(projectDir, '.git'))).resolves.toBeDefined(); + expect(await loaded.git.revParseHead()).toMatch(/^[0-9a-f]{40}$/); + }); }); diff --git a/packages/cli/test/doctor.test.ts b/packages/cli/test/doctor.test.ts index fb61b8d5..9d9aa1a9 100644 --- a/packages/cli/test/doctor.test.ts +++ b/packages/cli/test/doctor.test.ts @@ -9,6 +9,8 @@ import { type DoctorCheck, } from '../src/doctor.js'; +const removedAutoCommitKey = ['auto', 'commit'].join('_'); + function makeIo() { let stdout = ''; let stderr = ''; @@ -353,17 +355,10 @@ describe('runKtxDoctor', () => { expect(parsed.projectDir).toBe(tempDir); }); - it('prints schema issues and exits 1 when ktx.yaml fails Zod validation', async () => { + it('prints schema issues and exits 1 when ktx.yaml has an invalid value', async () => { await writeFile( join(tempDir, 'ktx.yaml'), - [ - 'storrage:', - ' state: sqlite', - 'ingest:', - ' llm:', - ' backend: anthropic', - '', - ].join('\n'), + ['storage:', ' state: mariadb', ''].join('\n'), 'utf-8', ); const testIo = makeIo(); @@ -379,15 +374,14 @@ describe('runKtxDoctor', () => { const out = testIo.stdout(); expect(out).toContain('ktx status'); expect(out).toContain('Config'); - expect(out).toContain('Unsupported storrage: unknown field'); - expect(out).toContain('Unsupported ingest.llm: unknown field'); + expect(out).toContain('storage.state'); expect(out).toContain('ktx.yaml'); }); - it('emits structured JSON when ktx.yaml fails Zod validation', async () => { + it('emits structured JSON when ktx.yaml has an invalid value', async () => { await writeFile( join(tempDir, 'ktx.yaml'), - ['storrage: {}', ''].join('\n'), + ['storage:', ' state: mariadb', ''].join('\n'), 'utf-8', ); const testIo = makeIo(); @@ -407,7 +401,7 @@ describe('runKtxDoctor', () => { }; expect(parsed.error).toBe('invalid_config'); expect(parsed.projectDir).toBe(tempDir); - expect(parsed.issues.some((issue) => issue.path === 'storrage')).toBe(true); + expect(parsed.issues.some((issue) => issue.path === 'storage.state')).toBe(true); }); it('shows a Config row labelled "ktx.yaml schema valid" on the happy path', async () => { @@ -490,6 +484,49 @@ describe('runKtxDoctor', () => { delete process.env.OPENAI_API_KEY; }); + it('exits 0 and shows a Config warn row when ktx.yaml carries stale unknown fields', async () => { + // The default `ktx status` path (command: 'project') must keep working on a + // ktx.yaml written by a different ktx version: unknown fields surface as an + // ignored-fields warning on the Config row, never as a failure. + process.env.ANTHROPIC_API_KEY = 'test-key'; // pragma: allowlist secret + await writeFile( + join(tempDir, 'ktx.yaml'), + [ + 'connections:', + ' warehouse:', + ' driver: sqlite', + ' path: ./warehouse.db', + 'llm:', + ' provider:', + ' backend: anthropic', + ' models:', + ' default: claude-sonnet-4-5', + 'storage:', + ' git:', + ` ${removedAutoCommitKey}: false`, + 'memory: {}', + '', + ].join('\n'), + 'utf-8', + ); + const testIo = makeIo(); + + await expect( + runKtxDoctor( + { command: 'project', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, + testIo.io, + {}, + ), + ).resolves.toBe(0); + + const out = testIo.stdout(); + expect(out).toContain('Config'); + expect(out).toContain('ktx.yaml schema valid · 2 ignored fields'); + expect(out).toContain(`⚠ Unsupported storage.git.${removedAutoCommitKey}: unknown field (ignored)`); + expect(out).toContain('⚠ Unsupported memory: unknown field (ignored)'); + delete process.env.ANTHROPIC_API_KEY; + }); + it('reports Claude Code auth failures and ignored prompt-caching fields in project doctor output', async () => { await writeFile( join(tempDir, 'ktx.yaml'), @@ -795,17 +832,10 @@ describe('runKtxDoctor', () => { expect(JSON.parse(testIo.stdout())).toEqual({ ok: true, projectDir: tempDir }); }); - it('prints schema issues and exits 1 when ktx.yaml fails Zod validation', async () => { + it('prints schema issues and exits 1 when ktx.yaml has an invalid value', async () => { await writeFile( join(tempDir, 'ktx.yaml'), - [ - 'storrage:', - ' state: sqlite', - 'ingest:', - ' llm:', - ' backend: anthropic', - '', - ].join('\n'), + ['storage:', ' state: mariadb', ''].join('\n'), 'utf-8', ); const testIo = makeIo(); @@ -819,14 +849,13 @@ describe('runKtxDoctor', () => { ).resolves.toBe(1); const out = testIo.stdout(); - expect(out).toContain('Unsupported storrage: unknown field'); - expect(out).toContain('Unsupported ingest.llm: unknown field'); + expect(out).toContain('storage.state'); }); it('emits structured JSON issues when validation fails', async () => { await writeFile( join(tempDir, 'ktx.yaml'), - ['storrage: {}', ''].join('\n'), + ['storage:', ' state: mariadb', ''].join('\n'), 'utf-8', ); const testIo = makeIo(); @@ -841,7 +870,75 @@ describe('runKtxDoctor', () => { const parsed = JSON.parse(testIo.stdout()) as { error: string; issues: Array<{ path: string }> }; expect(parsed.error).toBe('invalid_config'); - expect(parsed.issues.some((issue) => issue.path === 'storrage')).toBe(true); + expect(parsed.issues.some((issue) => issue.path === 'storage.state')).toBe(true); + }); + + it('tolerates unknown fields, reporting them as warnings and exiting 0', async () => { + await writeFile( + join(tempDir, 'ktx.yaml'), + ['storrage:', ' state: sqlite', 'ingest:', ' llm:', ' backend: anthropic', ''].join('\n'), + 'utf-8', + ); + const testIo = makeIo(); + + await expect( + runKtxDoctor( + { command: 'validate', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, + testIo.io, + {}, + ), + ).resolves.toBe(0); + + const out = testIo.stdout(); + expect(out).toContain('storrage'); + expect(out).toContain('ingest.llm'); + expect(out).toContain('ignored'); + }); + + it('emits structured JSON warnings for unknown fields and exits 0', async () => { + await writeFile( + join(tempDir, 'ktx.yaml'), + ['storrage: {}', ''].join('\n'), + 'utf-8', + ); + const testIo = makeIo(); + + await expect( + runKtxDoctor( + { command: 'validate', projectDir: tempDir, outputMode: 'json', inputMode: 'disabled' }, + testIo.io, + {}, + ), + ).resolves.toBe(0); + + const parsed = JSON.parse(testIo.stdout()) as { ok: boolean; warnings: Array<{ path: string; severity: string }> }; + expect(parsed.ok).toBe(true); + expect(parsed.warnings.some((warning) => warning.path === 'storrage' && warning.severity === 'warning')).toBe(true); + }); + + it('renders unknown fields as ignored even when a real error blocks', async () => { + // Mixed file: a bad value on a recognized field (blocks) plus a stale + // unknown key (ignored). Only the error counts as a schema issue; the + // warning keeps the ⚠ ignored-field treatment instead of a misleading ✗. + await writeFile( + join(tempDir, 'ktx.yaml'), + ['storage:', ' state: mariadb', 'memory: {}', ''].join('\n'), + 'utf-8', + ); + const testIo = makeIo(); + + await expect( + runKtxDoctor( + { command: 'validate', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, + testIo.io, + {}, + ), + ).resolves.toBe(1); + + const out = testIo.stdout(); + expect(out).toContain('ktx.yaml has 1 schema issue · 1 ignored field'); + expect(out).toContain('✗ storage.state'); + expect(out).toContain('⚠ Unsupported memory: unknown field (ignored)'); }); it('prints the missing-project message and exits 1 when ktx.yaml is absent', async () => {