mirror of
https://github.com/Kaelio/ktx.git
synced 2026-06-25 08:48:08 +02:00
feat(setup): apply per-role LLM model presets, remove --llm-model (#268)
* feat(setup): write per-role llm model presets * feat(setup): remove llm model setup flag * chore(setup): update llm preset guidance * docs(setup): document llm model presets * chore(release): sync uv.lock to 0.9.0 * fix(cli): make sl query --execute work on secret-backed connections sl query --execute used a parallel SQL executor (createDefaultLocalQueryExecutor) that passed connection.url verbatim into pg, so file:/env: secret references failed with "SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a string". Collapse onto the connector-based executor already used by MCP and ingest (createKtxCliIngestQueryExecutor), which resolves secret references and supports every driver. Delete the now-dead local/postgres/sqlite query executors, their tests, and the orphaned hasLocalQueryExecutor driver flag. * docs(agents): require one implementation per capability Add a design-reasoning default and a matching self-check question telling agents to route callers through a single shared implementation of a capability rather than forking a parallel one, and to fix the shared layer rather than patch one branch. Encodes the lesson from a divergent SQL-execution-path bug, stated generally. CLAUDE.md is a symlink to AGENTS.md, so both agent-instruction files are covered.
This commit is contained in:
Andrey Avtomonov
GitHub
parent
2896f9fb91
commit
2c18a62de4
25 changed files with 404 additions and 1384 deletions
|
|
@ -68,7 +68,6 @@ const connectionFixtures: Record<KtxConnectionDriver, FixtureFactory> = {
|
|||
|
||||
const allowedScopeKeys = new Set(['dataset_ids', 'databases', 'schemas', 'schema_names']);
|
||||
const historicSqlReaderDrivers = new Set<KtxConnectionDriver>(['postgres', 'bigquery', 'snowflake']);
|
||||
const localExecutorDrivers = new Set<KtxConnectionDriver>(['postgres', 'sqlite']);
|
||||
|
||||
function assertExportedRegistryBoundaryTypes(input: {
|
||||
scopeConfigKey: KtxScopeConfigKey;
|
||||
|
|
@ -140,6 +139,5 @@ describe('driverRegistrations', () => {
|
|||
expect(allowedScopeKeys.has(registration.scopeConfigKey ?? '')).toBe(true);
|
||||
}
|
||||
expect(registration.hasHistoricSqlReader).toBe(historicSqlReaderDrivers.has(registration.driver));
|
||||
expect(registration.hasLocalQueryExecutor).toBe(localExecutorDrivers.has(registration.driver));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,59 +0,0 @@
|
|||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { createDefaultLocalQueryExecutor } from '../../../src/context/connections/local-query-executor.js';
|
||||
|
||||
describe('createDefaultLocalQueryExecutor', () => {
|
||||
it('dispatches postgres and sqlite drivers to their executors', async () => {
|
||||
const postgres = {
|
||||
execute: vi.fn(async () => ({
|
||||
headers: ['pg'],
|
||||
rows: [[1]],
|
||||
totalRows: 1,
|
||||
command: 'SELECT',
|
||||
rowCount: 1,
|
||||
})),
|
||||
};
|
||||
const sqlite = {
|
||||
execute: vi.fn(async () => ({
|
||||
headers: ['sqlite'],
|
||||
rows: [[2]],
|
||||
totalRows: 1,
|
||||
command: 'SELECT',
|
||||
rowCount: 1,
|
||||
})),
|
||||
};
|
||||
const executor = createDefaultLocalQueryExecutor({ postgres, sqlite });
|
||||
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'pg',
|
||||
connection: { driver: 'postgres' },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).resolves.toMatchObject({ headers: ['pg'] });
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'local',
|
||||
connection: { driver: 'sqlite' },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).resolves.toMatchObject({ headers: ['sqlite'] });
|
||||
|
||||
expect(postgres.execute).toHaveBeenCalledTimes(1);
|
||||
expect(sqlite.execute).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('rejects unsupported local execution drivers', async () => {
|
||||
const executor = createDefaultLocalQueryExecutor({
|
||||
postgres: { execute: vi.fn() },
|
||||
sqlite: { execute: vi.fn() },
|
||||
});
|
||||
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
connection: { driver: 'snowflake' },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).rejects.toThrow('No local query executor is configured for driver "snowflake".');
|
||||
});
|
||||
});
|
||||
|
|
@ -1,103 +0,0 @@
|
|||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { createPostgresQueryExecutor } from '../../../src/context/connections/postgres-query-executor.js';
|
||||
|
||||
function makeClient() {
|
||||
const calls: unknown[] = [];
|
||||
const client = {
|
||||
connect: vi.fn(async () => undefined),
|
||||
query: vi.fn(async (input: unknown) => {
|
||||
calls.push(input);
|
||||
if (input === 'BEGIN READ ONLY') {
|
||||
return { rows: [], fields: [], rowCount: null, command: 'BEGIN' };
|
||||
}
|
||||
if (input === 'COMMIT') {
|
||||
return { rows: [], fields: [], rowCount: null, command: 'COMMIT' };
|
||||
}
|
||||
return {
|
||||
rows: [
|
||||
['paid', 2],
|
||||
['open', 1],
|
||||
],
|
||||
fields: [{ name: 'status' }, { name: 'order_count' }],
|
||||
rowCount: 2,
|
||||
command: 'SELECT',
|
||||
};
|
||||
}),
|
||||
end: vi.fn(async () => undefined),
|
||||
};
|
||||
return { client, calls };
|
||||
}
|
||||
|
||||
describe('createPostgresQueryExecutor', () => {
|
||||
it('runs a read-only transaction in array row mode and closes the client', async () => {
|
||||
const { client, calls } = makeClient();
|
||||
const executor = createPostgresQueryExecutor({
|
||||
clientFactory: vi.fn(() => client),
|
||||
});
|
||||
|
||||
const result = await executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
connection: { driver: 'postgres', url: 'postgres://example/db' },
|
||||
sql: 'select status, count(*) as order_count from public.orders group by status',
|
||||
maxRows: 50,
|
||||
});
|
||||
|
||||
expect(client.connect).toHaveBeenCalledTimes(1);
|
||||
expect(calls[0]).toBe('BEGIN READ ONLY');
|
||||
expect(calls[1]).toEqual({
|
||||
text: 'select * from (select status, count(*) as order_count from public.orders group by status) as ktx_query_result limit 50',
|
||||
rowMode: 'array',
|
||||
});
|
||||
expect(calls[2]).toBe('COMMIT');
|
||||
expect(client.end).toHaveBeenCalledTimes(1);
|
||||
expect(result).toEqual({
|
||||
headers: ['status', 'order_count'],
|
||||
rows: [
|
||||
['paid', 2],
|
||||
['open', 1],
|
||||
],
|
||||
totalRows: 2,
|
||||
command: 'SELECT',
|
||||
rowCount: 2,
|
||||
});
|
||||
});
|
||||
|
||||
it('rolls back and closes the client when query execution fails', async () => {
|
||||
const client = {
|
||||
connect: vi.fn(async () => undefined),
|
||||
query: vi.fn(async (input: unknown) => {
|
||||
if (input === 'BEGIN READ ONLY' || input === 'ROLLBACK') {
|
||||
return { rows: [], fields: [], rowCount: null, command: String(input) };
|
||||
}
|
||||
throw new Error('syntax error');
|
||||
}),
|
||||
end: vi.fn(async () => undefined),
|
||||
};
|
||||
const executor = createPostgresQueryExecutor({
|
||||
clientFactory: vi.fn(() => client),
|
||||
});
|
||||
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
connection: { driver: 'postgres', url: 'postgres://example/db' },
|
||||
sql: 'select * from broken',
|
||||
maxRows: 10,
|
||||
}),
|
||||
).rejects.toThrow('syntax error');
|
||||
expect(client.query).toHaveBeenCalledWith('ROLLBACK');
|
||||
expect(client.end).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('requires a Postgres url', async () => {
|
||||
const executor = createPostgresQueryExecutor({ clientFactory: vi.fn() });
|
||||
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
connection: { driver: 'postgres' },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).rejects.toThrow('Local Postgres execution requires connections.warehouse.url');
|
||||
});
|
||||
});
|
||||
|
|
@ -1,139 +0,0 @@
|
|||
import { mkdtemp, rm } from 'node:fs/promises';
|
||||
import { writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import Database from 'better-sqlite3';
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSqliteQueryExecutor, sqliteDatabasePathFromConnection } from '../../../src/context/connections/sqlite-query-executor.js';
|
||||
|
||||
describe('createSqliteQueryExecutor', () => {
|
||||
let tempDir: string;
|
||||
let dbPath: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = await mkdtemp(join(tmpdir(), 'ktx-sqlite-query-'));
|
||||
dbPath = join(tempDir, 'warehouse.db');
|
||||
const db = new Database(dbPath);
|
||||
db.exec(`
|
||||
CREATE TABLE orders (
|
||||
id INTEGER PRIMARY KEY,
|
||||
status TEXT NOT NULL,
|
||||
amount INTEGER NOT NULL
|
||||
);
|
||||
INSERT INTO orders (status, amount) VALUES
|
||||
('paid', 20),
|
||||
('paid', 30),
|
||||
('open', 10);
|
||||
`);
|
||||
db.close();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('executes read-only SELECT SQL against a relative SQLite path', async () => {
|
||||
const executor = createSqliteQueryExecutor();
|
||||
|
||||
const result = await executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
projectDir: tempDir,
|
||||
connection: { driver: 'sqlite', path: 'warehouse.db' },
|
||||
sql: 'select status, count(*) as order_count from orders group by status order by status',
|
||||
maxRows: 10,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
headers: ['status', 'order_count'],
|
||||
rows: [
|
||||
['open', 1],
|
||||
['paid', 2],
|
||||
],
|
||||
totalRows: 2,
|
||||
command: 'SELECT',
|
||||
rowCount: 2,
|
||||
});
|
||||
});
|
||||
|
||||
it('supports file urls for SQLite database paths', async () => {
|
||||
expect(
|
||||
sqliteDatabasePathFromConnection({
|
||||
connectionId: 'warehouse',
|
||||
projectDir: tempDir,
|
||||
connection: { driver: 'sqlite', url: `file://${dbPath}` },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).toBe(dbPath);
|
||||
});
|
||||
|
||||
it('resolves file references for SQLite path fields', async () => {
|
||||
const pointerPath = join(tempDir, 'sqlite-path.txt');
|
||||
writeFileSync(pointerPath, dbPath, 'utf-8');
|
||||
|
||||
expect(
|
||||
sqliteDatabasePathFromConnection({
|
||||
connectionId: 'warehouse',
|
||||
projectDir: tempDir,
|
||||
connection: { driver: 'sqlite', path: `file:${pointerPath}` },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).toBe(dbPath);
|
||||
});
|
||||
|
||||
it('resolves env references for SQLite database urls', async () => {
|
||||
const originalDatabaseUrl = process.env.KTX_SQLITE_TEST_URL;
|
||||
process.env.KTX_SQLITE_TEST_URL = `sqlite:${dbPath}`;
|
||||
|
||||
try {
|
||||
expect(
|
||||
sqliteDatabasePathFromConnection({
|
||||
connectionId: 'warehouse',
|
||||
projectDir: tempDir,
|
||||
connection: { driver: 'sqlite', url: 'env:KTX_SQLITE_TEST_URL' },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).toBe(dbPath);
|
||||
} finally {
|
||||
if (originalDatabaseUrl === undefined) {
|
||||
delete process.env.KTX_SQLITE_TEST_URL;
|
||||
} else {
|
||||
process.env.KTX_SQLITE_TEST_URL = originalDatabaseUrl;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects mutating SQL before opening the database', async () => {
|
||||
const executor = createSqliteQueryExecutor();
|
||||
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
projectDir: tempDir,
|
||||
connection: { driver: 'sqlite', path: 'warehouse.db' },
|
||||
sql: 'delete from orders',
|
||||
}),
|
||||
).rejects.toThrow('Only read-only SELECT/WITH queries can be executed locally');
|
||||
});
|
||||
|
||||
it('requires a SQLite driver and a database path', async () => {
|
||||
const executor = createSqliteQueryExecutor();
|
||||
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
projectDir: tempDir,
|
||||
connection: { driver: 'postgres', path: 'warehouse.db' },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).rejects.toThrow('Local SQLite execution cannot run driver "postgres"');
|
||||
|
||||
await expect(
|
||||
executor.execute({
|
||||
connectionId: 'warehouse',
|
||||
projectDir: tempDir,
|
||||
connection: { driver: 'sqlite' },
|
||||
sql: 'select 1',
|
||||
}),
|
||||
).rejects.toThrow('Local SQLite execution requires connections.warehouse.path or connections.warehouse.url');
|
||||
});
|
||||
});
|
||||
|
|
@ -80,8 +80,8 @@ describe('createLocalBundleIngestRuntime', () => {
|
|||
'ktx ingest requires llm.provider.backend: anthropic, vertex, gateway, claude-code, or codex, or an injected agentRunner.',
|
||||
'Configure a local Claude Code/Codex session or API-backed LLM, then rerun ingest:',
|
||||
` ktx setup --project-dir ${project.projectDir} --llm-backend claude-code --no-input`,
|
||||
` ktx setup --project-dir ${project.projectDir} --llm-backend codex --llm-model gpt-5.5 --no-input`,
|
||||
` ktx setup --project-dir ${project.projectDir} --llm-backend anthropic --anthropic-api-key-env ANTHROPIC_API_KEY --llm-model claude-sonnet-4-6 --no-input`,
|
||||
` ktx setup --project-dir ${project.projectDir} --llm-backend codex --no-input`,
|
||||
` ktx setup --project-dir ${project.projectDir} --llm-backend anthropic --anthropic-api-key-env ANTHROPIC_API_KEY --no-input`,
|
||||
].join('\n'),
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue