From 8a50601582f990b1f367cf80b559211939dbc753 Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Mon, 15 Jun 2026 14:38:44 +0200 Subject: [PATCH] fix(cli): make connection-not-configured errors actionable and expected (#301) The MCP sql_execution/sl_query tools and the `ktx sql` CLI threw a plain Error naming no valid connection ids when an agent passed an unconfigured connectionId (or omitted it with multiple connections). The message reached the agent verbatim but gave it nothing to correct with, so it re-guessed for days, and each correct caller-driven rejection filed in PostHog Error Tracking as a ktx fault (issue 019eb10c, 8 occurrences on one install). Add a shared resolver (resolveConfiguredConnection / resolveRequiredConnectionId) that throws KtxExpectedError listing the configured connections, and route the three SQL-execution call sites through it. Expected-error classification keeps these out of Error Tracking while the actionable message lets agents self-correct. --- .../context/connections/resolve-connection.ts | 50 ++++++++++++ .../src/context/mcp/local-project-ports.ts | 6 +- packages/cli/src/context/sl/local-query.ts | 10 +-- packages/cli/src/sql.ts | 6 +- .../connections/resolve-connection.test.ts | 80 +++++++++++++++++++ .../context/mcp/local-project-ports.test.ts | 39 ++++++++- .../cli/test/context/sl/local-query.test.ts | 14 +++- packages/cli/test/sql.test.ts | 4 +- 8 files changed, 189 insertions(+), 20 deletions(-) create mode 100644 packages/cli/src/context/connections/resolve-connection.ts create mode 100644 packages/cli/test/context/connections/resolve-connection.test.ts diff --git a/packages/cli/src/context/connections/resolve-connection.ts b/packages/cli/src/context/connections/resolve-connection.ts new file mode 100644 index 00000000..1dee09ca --- /dev/null +++ b/packages/cli/src/context/connections/resolve-connection.ts @@ -0,0 +1,50 @@ +import { KtxExpectedError } from '../../errors.js'; +import type { KtxProjectConfig, KtxProjectConnectionConfig } from '../project/config.js'; + +function configuredConnectionIds(config: KtxProjectConfig): string[] { + return Object.keys(config.connections).sort(); +} + +function availableConnectionsHint(config: KtxProjectConfig): string { + const ids = configuredConnectionIds(config); + return ids.length === 0 + ? 'No connections are configured in ktx.yaml.' + : `Configured connections: ${ids.join(', ')}.`; +} + +/** + * Look up a connection by id, throwing an expected (caller-driven) error that + * names the configured connections so an agent or CLI user can self-correct. + */ +export function resolveConfiguredConnection( + config: KtxProjectConfig, + connectionId: string, +): KtxProjectConnectionConfig { + const connection = config.connections[connectionId]; + if (!connection) { + throw new KtxExpectedError( + `Connection "${connectionId}" is not configured in ktx.yaml. ${availableConnectionsHint(config)}`, + ); + } + return connection; +} + +/** + * Resolve the connection id to run against: validate a requested id against the + * configured connections, or default to the sole connection when none is given. + * Throws an expected error that lists the configured connections otherwise. + */ +export function resolveRequiredConnectionId( + config: KtxProjectConfig, + requested: string | undefined, +): string { + if (requested !== undefined) { + resolveConfiguredConnection(config, requested); + return requested; + } + const ids = configuredConnectionIds(config); + if (ids.length === 1) { + return ids[0]; + } + throw new KtxExpectedError(`connectionId is required. ${availableConnectionsHint(config)}`); +} diff --git a/packages/cli/src/context/mcp/local-project-ports.ts b/packages/cli/src/context/mcp/local-project-ports.ts index 4bada831..d3106593 100644 --- a/packages/cli/src/context/mcp/local-project-ports.ts +++ b/packages/cli/src/context/mcp/local-project-ports.ts @@ -1,4 +1,5 @@ import type { KtxSqlQueryExecutorPort } from '../../context/connections/query-executor.js'; +import { resolveConfiguredConnection } from '../../context/connections/resolve-connection.js'; import { KtxQueryError, isNativeProgrammingFault } from '../../errors.js'; import { localConnectionInfoFromConfig } from '../../context/connections/local-warehouse-descriptor.js'; import type { KtxEmbeddingPort } from '../../context/core/embedding.js'; @@ -39,10 +40,7 @@ async function executeValidatedReadOnlySql( ): Promise { await onProgress?.({ progress: 0, message: 'Validating SQL' }); const connectionId = assertSafeConnectionId(input.connectionId); - const connection = project.config.connections[connectionId]; - if (!connection) { - throw new Error(`Connection "${connectionId}" is not configured in ktx.yaml`); - } + const connection = resolveConfiguredConnection(project.config, connectionId); if (!options.sqlAnalysis) { throw new Error('sql_execution requires parser-backed SQL validation.'); } diff --git a/packages/cli/src/context/sl/local-query.ts b/packages/cli/src/context/sl/local-query.ts index f6de3aab..1622bd2d 100644 --- a/packages/cli/src/context/sl/local-query.ts +++ b/packages/cli/src/context/sl/local-query.ts @@ -2,6 +2,7 @@ import type { KtxSqlQueryExecutorPort } from '../../context/connections/query-ex import type { KtxSemanticLayerComputePort } from '../../context/daemon/semantic-layer-compute.js'; import type { KtxMcpProgressCallback } from '../mcp/types.js'; import type { KtxLocalProject } from '../../context/project/project.js'; +import { resolveRequiredConnectionId } from '../connections/resolve-connection.js'; import { sqlAnalysisDialectForDriver } from '../sql-analysis/dialect.js'; import { loadLocalSlSourceRecords } from './local-sl.js'; import { toResolvedWire } from './semantic-layer.service.js'; @@ -27,14 +28,7 @@ export interface CompileLocalSlQueryResult extends SemanticLayerQueryExecutionRe } function resolveLocalConnectionId(project: KtxLocalProject, requested: string | undefined): string { - if (requested) { - return assertSafeConnectionId(requested); - } - const ids = Object.keys(project.config.connections).sort(); - if (ids.length === 1) { - return assertSafeConnectionId(ids[0]); - } - throw new Error('connectionId is required when the local project has zero or multiple connections.'); + return assertSafeConnectionId(resolveRequiredConnectionId(project.config, requested)); } async function loadComputableSources( diff --git a/packages/cli/src/sql.ts b/packages/cli/src/sql.ts index d3eb6a81..cbfcbc47 100644 --- a/packages/cli/src/sql.ts +++ b/packages/cli/src/sql.ts @@ -1,3 +1,4 @@ +import { resolveConfiguredConnection } from './context/connections/resolve-connection.js'; import { loadKtxProject, type KtxLocalProject } from './context/project/project.js'; import type { KtxQueryResult, KtxScanConnector } from './context/scan/types.js'; import type { SqlAnalysisDialect, SqlAnalysisPort } from './context/sql-analysis/ports.js'; @@ -146,10 +147,7 @@ export async function runKtxSql(args: KtxSqlArgs, io: KtxCliIo = process, deps: let project: KtxLocalProject | undefined; try { project = await (deps.loadProject ?? loadKtxProject)({ projectDir: args.projectDir }); - const connection = project.config.connections[args.connectionId]; - if (!connection) { - throw new Error(`Connection "${args.connectionId}" is not configured in ktx.yaml`); - } + const connection = resolveConfiguredConnection(project.config, args.connectionId); driver = String(connection.driver ?? 'unknown').toLowerCase(); demoConnection = isDemoConnection(args.connectionId, connection); diff --git a/packages/cli/test/context/connections/resolve-connection.test.ts b/packages/cli/test/context/connections/resolve-connection.test.ts new file mode 100644 index 00000000..50de45d9 --- /dev/null +++ b/packages/cli/test/context/connections/resolve-connection.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from 'vitest'; +import { + buildDefaultKtxProjectConfig, + type KtxProjectConfig, +} from '../../../src/context/project/config.js'; +import { + resolveConfiguredConnection, + resolveRequiredConnectionId, +} from '../../../src/context/connections/resolve-connection.js'; +import { KtxExpectedError } from '../../../src/errors.js'; + +function configWith(ids: string[]): KtxProjectConfig { + const config = buildDefaultKtxProjectConfig(); + for (const id of ids) { + config.connections[id] = { driver: 'postgres' }; + } + return config; +} + +describe('resolveConfiguredConnection', () => { + it('returns the connection config when the id is configured', () => { + const config = configWith(['warehouse']); + expect(resolveConfiguredConnection(config, 'warehouse')).toEqual({ driver: 'postgres' }); + }); + + it('throws an expected error that lists the configured connections', () => { + const config = configWith(['analytics', 'warehouse']); + let error: unknown; + try { + resolveConfiguredConnection(config, 'ARK'); + } catch (caught) { + error = caught; + } + expect(error).toBeInstanceOf(KtxExpectedError); + expect((error as Error).message).toBe( + 'Connection "ARK" is not configured in ktx.yaml. Configured connections: analytics, warehouse.', + ); + }); + + it('reports when no connections are configured at all', () => { + const config = configWith([]); + expect(() => resolveConfiguredConnection(config, 'warehouse')).toThrow( + 'Connection "warehouse" is not configured in ktx.yaml. No connections are configured in ktx.yaml.', + ); + }); +}); + +describe('resolveRequiredConnectionId', () => { + it('returns the requested id when it is configured', () => { + const config = configWith(['warehouse']); + expect(resolveRequiredConnectionId(config, 'warehouse')).toBe('warehouse'); + }); + + it('throws an expected error listing connections when the requested id is unknown', () => { + const config = configWith(['analytics', 'warehouse']); + expect(() => resolveRequiredConnectionId(config, 'DIG_SMART_REP')).toThrow(KtxExpectedError); + expect(() => resolveRequiredConnectionId(config, 'DIG_SMART_REP')).toThrow( + 'Connection "DIG_SMART_REP" is not configured in ktx.yaml. Configured connections: analytics, warehouse.', + ); + }); + + it('defaults to the only connection when the id is omitted', () => { + const config = configWith(['warehouse']); + expect(resolveRequiredConnectionId(config, undefined)).toBe('warehouse'); + }); + + it('throws an expected error listing connections when the id is omitted and several exist', () => { + const config = configWith(['analytics', 'warehouse']); + let error: unknown; + try { + resolveRequiredConnectionId(config, undefined); + } catch (caught) { + error = caught; + } + expect(error).toBeInstanceOf(KtxExpectedError); + expect((error as Error).message).toBe( + 'connectionId is required. Configured connections: analytics, warehouse.', + ); + }); +}); diff --git a/packages/cli/test/context/mcp/local-project-ports.test.ts b/packages/cli/test/context/mcp/local-project-ports.test.ts index b2699d53..702cff64 100644 --- a/packages/cli/test/context/mcp/local-project-ports.test.ts +++ b/packages/cli/test/context/mcp/local-project-ports.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { initKtxProject } from '../../../src/context/project/project.js'; -import { KtxQueryError } from '../../../src/errors.js'; +import { KtxExpectedError, KtxQueryError } from '../../../src/errors.js'; import { createKtxConnectorCapabilities, type KtxQueryResult, type KtxScanConnector, type KtxSchemaSnapshot } from '../../../src/context/scan/types.js'; import { SemanticLayerService } from '../../../src/context/sl/semantic-layer.service.js'; import type { SemanticLayerSource } from '../../../src/context/sl/types.js'; @@ -245,6 +245,43 @@ describe('createLocalProjectMcpContextPorts', () => { expect(connector.cleanup).toHaveBeenCalled(); }); + it('rejects sql_execution against an unconfigured connection with an actionable expected error', async () => { + const project = await initKtxProject({ projectDir: tempDir }); + project.config.connections.warehouse = { + driver: 'postgres', + url: 'env:DATABASE_URL', + }; + const connector = testConnector(testSnapshot(), { + headers: ['id'], + headerTypes: ['integer'], + rows: [[1]], + totalRows: 1, + rowCount: 1, + }); + const createConnector = vi.fn(async () => connector); + const sqlAnalysis = { + analyzeForFingerprint: vi.fn(), + analyzeBatch: vi.fn(), + validateReadOnly: vi.fn(async () => ({ ok: true, error: null })), + }; + const ports = createLocalProjectMcpContextPorts(project, { + sqlAnalysis, + localScan: { createConnector }, + embeddingService: null, + }); + + const execution = ports.sqlExecution?.execute({ + connectionId: 'DIG_SMART_REP', + sql: 'select 1', + maxRows: 5, + }); + await expect(execution).rejects.toBeInstanceOf(KtxExpectedError); + await expect(execution).rejects.toThrow( + 'Connection "DIG_SMART_REP" is not configured in ktx.yaml. Configured connections: warehouse.', + ); + expect(createConnector).not.toHaveBeenCalled(); + }); + it('emits sql_execution progress stages from local MCP ports', async () => { const project = await initKtxProject({ projectDir: tempDir }); project.config.connections.warehouse = { diff --git a/packages/cli/test/context/sl/local-query.test.ts b/packages/cli/test/context/sl/local-query.test.ts index 4137f596..d1db5503 100644 --- a/packages/cli/test/context/sl/local-query.test.ts +++ b/packages/cli/test/context/sl/local-query.test.ts @@ -324,7 +324,7 @@ grain: [] ).rejects.toThrow('Local semantic-layer execution requires a query executor.'); }); - it('requires connectionId when multiple connections are configured', async () => { + it('requires connectionId, listing the configured connections, when several exist', async () => { project.config.connections.analytics = { driver: 'bigquery' }; await expect( @@ -332,6 +332,16 @@ grain: [] query: { measures: ['orders.order_count'], dimensions: [] }, compute, }), - ).rejects.toThrow('connectionId is required when the local project has zero or multiple connections.'); + ).rejects.toThrow('connectionId is required. Configured connections: analytics, warehouse.'); + }); + + it('rejects a connectionId that is not configured, listing the configured connections', async () => { + await expect( + compileLocalSlQuery(project, { + connectionId: 'DIG_SMART_REP', + query: { measures: ['orders.order_count'], dimensions: [] }, + compute, + }), + ).rejects.toThrow('Connection "DIG_SMART_REP" is not configured in ktx.yaml. Configured connections: warehouse.'); }); }); diff --git a/packages/cli/test/sql.test.ts b/packages/cli/test/sql.test.ts index 5e297429..067f76a2 100644 --- a/packages/cli/test/sql.test.ts +++ b/packages/cli/test/sql.test.ts @@ -306,7 +306,9 @@ describe('runKtxSql', () => { ), ).resolves.toBe(1); - expect(io.stderr()).toContain('Connection "warehouse" is not configured in ktx.yaml'); + expect(io.stderr()).toContain( + 'Connection "warehouse" is not configured in ktx.yaml. No connections are configured in ktx.yaml.', + ); }); it('rejects connectors without read-only SQL support and still cleans up', async () => {