mirror of
https://github.com/Kaelio/ktx.git
synced 2026-06-16 08:25:14 +02:00
fix(cli): survive ktx.yaml version skew and derive repo ownership from disk (#293)
* fix(cli): survive ktx.yaml version skew and derive repo ownership from disk Loading ktx.yaml is now tolerant of keys this ktx version does not recognize: they are stripped from the in-memory config (the file on disk is never rewritten) and reported by ktx status as non-blocking warnings, while invalid values on recognized fields still fail hard. Repo ownership is derived from observed state (a .git directory plus a root ktx.yaml) instead of a ktx.managed git-config marker, so projects created by any past or future ktx classify identically. initKtxProject now runs an explicit foreign-repo pre-check and writes ktx.yaml before initializing git, so an interrupted init leaves only recoverable residue instead of a bare .git misread as foreign. * style(cli): trim comment blocks to constraint-only notes * docs(agents): require constraint-only code comments
This commit is contained in:
Andrey Avtomonov
GitHub
parent
a278d2f7d0
commit
0689d709d2
14 changed files with 502 additions and 146 deletions
|
|
@ -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`: `<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.
|
||||
* - `unowned`: no repo here (including a missing or non-directory path) → ktx may `git init`.
|
||||
* - `ktx-managed`: `<dir>/.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 `<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.
|
||||
* Reads only `<dir>` itself; never walks up, so a parent repo cannot change the answer.
|
||||
*/
|
||||
export async function classifyKtxRepoOwnership(dir: string): Promise<KtxRepoOwnership> {
|
||||
let dotGitIsDirectory: boolean;
|
||||
|
|
@ -78,13 +71,9 @@ export async function classifyKtxRepoOwnership(dir: string): Promise<KtxRepoOwne
|
|||
return 'foreign';
|
||||
}
|
||||
try {
|
||||
const marker = await createSimpleGit(dir).raw([
|
||||
'config',
|
||||
'--local',
|
||||
'--get',
|
||||
KTX_MANAGED_GIT_CONFIG_KEY,
|
||||
]);
|
||||
return marker.trim() === 'true' ? 'ktx-managed' : 'foreign';
|
||||
// stat (not lstat): follow symlinks, matching what `loadKtxProject`'s
|
||||
// readFile accepts — a dir that loads as a ktx project classifies as one.
|
||||
return (await fs.stat(join(dir, 'ktx.yaml'))).isFile() ? 'ktx-managed' : 'foreign';
|
||||
} catch {
|
||||
return 'foreign';
|
||||
}
|
||||
|
|
@ -168,7 +157,6 @@ export class GitService {
|
|||
}
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -301,6 +301,11 @@ export interface KtxConfigIssue {
|
|||
path: string;
|
||||
message: string;
|
||||
fix?: string;
|
||||
/**
|
||||
* 'error' blocks the project (bad value on a recognized field); 'warning' is
|
||||
* a condition the loader recovers from on its own (an ignored unknown key).
|
||||
*/
|
||||
severity: 'error' | 'warning';
|
||||
}
|
||||
|
||||
export interface KtxConfigValidation {
|
||||
|
|
@ -325,24 +330,61 @@ function valueAtPath(root: unknown, path: ReadonlyArray<PropertyKey>): unknown {
|
|||
return cursor;
|
||||
}
|
||||
|
||||
function formatIssue(issue: z.core.$ZodIssue, input: unknown): KtxConfigIssue[] {
|
||||
const basePath = dottedPath(issue.path);
|
||||
interface UnknownKeyLocation {
|
||||
containerPath: ReadonlyArray<PropertyKey>;
|
||||
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<string, unknown>): Record<string, unknown> {
|
||||
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<string, unknown>)[key];
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
function parseTolerant(input: Record<string, unknown>): 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<string, unknown> {
|
||||
|
|
|
|||
|
|
@ -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<In
|
|||
throw new Error(`Project already contains ktx.yaml: ${configPath}`);
|
||||
}
|
||||
|
||||
const config = buildDefaultKtxProjectConfig();
|
||||
const runtime = await createRuntime(projectDir, config, authorName, authorEmail, logger);
|
||||
// Must run before ktx.yaml is written: once that file exists the directory
|
||||
// classifies as ktx-managed, so a foreign repo would be silently adopted.
|
||||
if ((await classifyKtxRepoOwnership(projectDir)) === 'foreign') {
|
||||
throw new KtxForeignGitRepositoryError(projectDir);
|
||||
}
|
||||
|
||||
await writeProjectFile(projectDir, 'ktx.yaml', serializeKtxProjectConfig(config));
|
||||
const config = buildDefaultKtxProjectConfig();
|
||||
|
||||
// ktx.yaml (the ownership signal) is written before git init, so an
|
||||
// interrupted init can never leave a bare `.git` without it — residue that
|
||||
// would classify as a foreign repo and be unrecoverable.
|
||||
await fs.mkdir(join(projectDir, '.ktx/cache'), { recursive: true });
|
||||
for (const file of TRACKED_SCAFFOLD_FILES) {
|
||||
await writeProjectFile(projectDir, file.path, file.content);
|
||||
}
|
||||
await writeProjectFile(projectDir, 'ktx.yaml', serializeKtxProjectConfig(config));
|
||||
|
||||
const runtime = await createRuntime(projectDir, config, authorName, authorEmail, logger);
|
||||
|
||||
const commit = await runtime.git.commitFiles(
|
||||
['ktx.yaml', ...TRACKED_SCAFFOLD_FILES.map((file) => file.path)],
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue