From 7e5551906ecf266817e8b67778f8834c1aa52782 Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Thu, 18 Jun 2026 13:12:13 +0200 Subject: [PATCH] fix(cli): honor KTX_SQL_ANALYSIS_URL/KTX_DAEMON_URL in ktx sql and MCP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ktx ingest resolved the SQL-analysis port via resolveKtxCliSqlAnalysis, which honors the external-daemon env overrides, but `ktx sql` and the MCP server built their port by calling createManagedDaemonSqlAnalysisPort directly. On a uv-less host with an external daemon and the override set, ingest worked while `ktx sql` and MCP sql_execution ignored the override and tried to install the managed Python runtime — contradicting the documented contract and runtime-requirements, which already skip the managed runtime when those vars are set. Collapse the three call sites onto one env-override-aware resolver: add externalDaemonSqlAnalysisBaseUrl + resolveSqlAnalysisPort in managed-python-http.ts and route sql.ts, mcp-server-factory.ts, and resolveKtxCliSqlAnalysis through it. createManagedDaemonSqlAnalysisPort becomes @internal (test-only export). --- packages/cli/src/local-adapters.ts | 15 ++- packages/cli/src/managed-python-http.ts | 26 ++++++ packages/cli/src/mcp-server-factory.ts | 4 +- packages/cli/src/sql.ts | 4 +- packages/cli/test/managed-python-http.test.ts | 93 +++++++++++++++++++ packages/cli/test/mcp-server-factory.test.ts | 2 +- 6 files changed, 130 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/local-adapters.ts b/packages/cli/src/local-adapters.ts index 671bce67..8d7cb57c 100644 --- a/packages/cli/src/local-adapters.ts +++ b/packages/cli/src/local-adapters.ts @@ -29,8 +29,9 @@ import { createHttpSqlAnalysisPort } from './context/sql-analysis/http-sql-analy import type { SqlAnalysisPort } from './context/sql-analysis/ports.js'; import { createManagedDaemonLookerTableIdentifierParser, - createManagedDaemonSqlAnalysisPort, + externalDaemonSqlAnalysisBaseUrl, managedDaemonDatabaseIntrospectionOptions, + resolveSqlAnalysisPort, type ManagedPythonDaemonHttpOptions, } from './managed-python-http.js'; import type { KtxOperationalLogger } from './io/logger.js'; @@ -78,16 +79,12 @@ export function resolveKtxCliSqlAnalysis(options: KtxCliLocalIngestAdaptersOptio if (options.sqlAnalysisUrl) { return createHttpSqlAnalysisPort({ baseUrl: options.sqlAnalysisUrl }); } - if (process.env.KTX_SQL_ANALYSIS_URL) { - return createHttpSqlAnalysisPort({ baseUrl: process.env.KTX_SQL_ANALYSIS_URL }); - } - if (process.env.KTX_DAEMON_URL) { - return createHttpSqlAnalysisPort({ baseUrl: process.env.KTX_DAEMON_URL }); - } if (options.managedDaemon) { - return createManagedDaemonSqlAnalysisPort(options.managedDaemon); + return resolveSqlAnalysisPort(options.managedDaemon); } - return createHttpSqlAnalysisPort({ baseUrl: 'http://127.0.0.1:8765' }); + return createHttpSqlAnalysisPort({ + baseUrl: externalDaemonSqlAnalysisBaseUrl(process.env) ?? 'http://127.0.0.1:8765', + }); } function createKtxCliLiveDatabaseIntrospection( diff --git a/packages/cli/src/managed-python-http.ts b/packages/cli/src/managed-python-http.ts index 392f375c..e0542280 100644 --- a/packages/cli/src/managed-python-http.ts +++ b/packages/cli/src/managed-python-http.ts @@ -178,6 +178,7 @@ export function createManagedDaemonLookerTableIdentifierParser( }); } +/** @internal */ export function createManagedDaemonSqlAnalysisPort(options: ManagedPythonDaemonHttpOptions): SqlAnalysisPort { return createHttpSqlAnalysisPort({ baseUrl: 'http://127.0.0.1:0', @@ -185,6 +186,31 @@ export function createManagedDaemonSqlAnalysisPort(options: ManagedPythonDaemonH }); } +export function externalDaemonSqlAnalysisBaseUrl( + env: NodeJS.ProcessEnv | Record, +): string | undefined { + const sqlAnalysisUrl = env.KTX_SQL_ANALYSIS_URL?.trim(); + if (sqlAnalysisUrl) { + return sqlAnalysisUrl; + } + const daemonUrl = env.KTX_DAEMON_URL?.trim(); + if (daemonUrl) { + return daemonUrl; + } + return undefined; +} + +export function resolveSqlAnalysisPort( + managedDaemon: ManagedPythonDaemonHttpOptions, + env: NodeJS.ProcessEnv | Record = process.env, +): SqlAnalysisPort { + const externalBaseUrl = externalDaemonSqlAnalysisBaseUrl(env); + if (externalBaseUrl) { + return createHttpSqlAnalysisPort({ baseUrl: externalBaseUrl }); + } + return createManagedDaemonSqlAnalysisPort(managedDaemon); +} + export function managedDaemonDatabaseIntrospectionOptions( options: ManagedPythonDaemonHttpOptions, ): Pick { diff --git a/packages/cli/src/mcp-server-factory.ts b/packages/cli/src/mcp-server-factory.ts index fbecccd8..ff1fc14f 100644 --- a/packages/cli/src/mcp-server-factory.ts +++ b/packages/cli/src/mcp-server-factory.ts @@ -9,7 +9,7 @@ import { resolveProjectEmbeddingProvider } from './embedding-resolution.js'; import { createKtxCliIngestQueryExecutor } from './ingest-query-executor.js'; import { createKtxCliScanConnector } from './local-scan-connectors.js'; import { createManagedPythonSemanticLayerComputePort } from './managed-python-command.js'; -import { createManagedDaemonSqlAnalysisPort } from './managed-python-http.js'; +import { resolveSqlAnalysisPort } from './managed-python-http.js'; function noopMcpIo(): KtxCliIo { return { @@ -31,7 +31,7 @@ export async function createKtxMcpServerFactory(input: { installPolicy: 'auto', io, }); - const sqlAnalysis = createManagedDaemonSqlAnalysisPort({ + const sqlAnalysis = resolveSqlAnalysisPort({ cliVersion: input.cliVersion, projectDir: input.projectDir, installPolicy: 'auto', diff --git a/packages/cli/src/sql.ts b/packages/cli/src/sql.ts index d3eb6a81..14a0da56 100644 --- a/packages/cli/src/sql.ts +++ b/packages/cli/src/sql.ts @@ -4,7 +4,7 @@ import type { SqlAnalysisDialect, SqlAnalysisPort } from './context/sql-analysis import type { KtxCliIo } from './cli-runtime.js'; import { type KtxOutputMode, resolveOutputMode } from './io/mode.js'; import { createKtxCliScanConnector } from './local-scan-connectors.js'; -import { createManagedDaemonSqlAnalysisPort } from './managed-python-http.js'; +import { resolveSqlAnalysisPort } from './managed-python-http.js'; import { profileMark } from './startup-profile.js'; import { isDemoConnection } from './telemetry/demo-detect.js'; import { emitTelemetryEvent, reportException } from './telemetry/index.js'; @@ -156,7 +156,7 @@ export async function runKtxSql(args: KtxSqlArgs, io: KtxCliIo = process, deps: const createSqlAnalysis = deps.createSqlAnalysis ?? (() => - createManagedDaemonSqlAnalysisPort({ + resolveSqlAnalysisPort({ cliVersion: args.cliVersion, projectDir: args.projectDir, installPolicy: 'auto', diff --git a/packages/cli/test/managed-python-http.test.ts b/packages/cli/test/managed-python-http.test.ts index ef06960f..84461efe 100644 --- a/packages/cli/test/managed-python-http.test.ts +++ b/packages/cli/test/managed-python-http.test.ts @@ -1,10 +1,14 @@ +import { createServer } from 'node:http'; +import type { AddressInfo } from 'node:net'; import { describe, expect, it, vi } from 'vitest'; import { createManagedDaemonHttpJsonRunner, createManagedDaemonLookerTableIdentifierParser, createManagedDaemonSqlAnalysisPort, createManagedPythonDaemonBaseUrlResolver, + externalDaemonSqlAnalysisBaseUrl, managedDaemonDatabaseIntrospectionOptions, + resolveSqlAnalysisPort, } from '../src/managed-python-http.js'; function io() { @@ -18,6 +22,25 @@ function io() { }; } +async function startLoopbackDaemon() { + const paths: string[] = []; + const server = createServer((req, res) => { + paths.push(req.url ?? ''); + req.on('data', () => {}); + req.on('end', () => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ ok: true })); + }); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as AddressInfo; + return { + baseUrl: `http://127.0.0.1:${port}`, + paths, + close: () => new Promise((resolve, reject) => server.close((error) => (error ? reject(error) : resolve()))), + }; +} + describe('createManagedPythonDaemonBaseUrlResolver', () => { it('ensures the core runtime, starts the daemon, reports the URL, and caches the result', async () => { const testIo = io(); @@ -203,3 +226,73 @@ describe('ktx daemon ingest ports', () => { expect(requestJson).toHaveBeenCalledWith('/database/introspect', { connection_id: 'warehouse' }); }); }); + +describe('externalDaemonSqlAnalysisBaseUrl', () => { + it('prefers KTX_SQL_ANALYSIS_URL over KTX_DAEMON_URL', () => { + expect( + externalDaemonSqlAnalysisBaseUrl({ + KTX_SQL_ANALYSIS_URL: 'http://analysis.example', + KTX_DAEMON_URL: 'http://daemon.example', + }), + ).toBe('http://analysis.example'); + }); + + it('falls back to KTX_DAEMON_URL when only it is set', () => { + expect(externalDaemonSqlAnalysisBaseUrl({ KTX_DAEMON_URL: 'http://daemon.example' })).toBe('http://daemon.example'); + }); + + it('trims surrounding whitespace and ignores whitespace-only overrides', () => { + expect(externalDaemonSqlAnalysisBaseUrl({ KTX_SQL_ANALYSIS_URL: ' http://analysis.example ' })).toBe( + 'http://analysis.example', + ); + expect(externalDaemonSqlAnalysisBaseUrl({ KTX_SQL_ANALYSIS_URL: ' ', KTX_DAEMON_URL: '' })).toBeUndefined(); + expect(externalDaemonSqlAnalysisBaseUrl({})).toBeUndefined(); + }); +}); + +describe('resolveSqlAnalysisPort', () => { + it('uses the managed daemon when no external override is set', async () => { + const daemon = await startLoopbackDaemon(); + const testIo = io(); + const ensureRuntime = vi.fn(async () => ({ layout: {} as never, manifest: {} as never })); + const startDaemon = vi.fn(async () => ({ + status: 'started' as const, + layout: {} as never, + state: { pid: 1234 } as never, + baseUrl: daemon.baseUrl, + })); + try { + const port = resolveSqlAnalysisPort( + { cliVersion: '0.2.0', projectDir: '/work/proj', installPolicy: 'auto', io: testIo.io, ensureRuntime, startDaemon }, + {}, + ); + await expect(port.validateReadOnly('select 1', 'postgres')).resolves.toEqual({ ok: true }); + expect(ensureRuntime).toHaveBeenCalledTimes(1); + expect(startDaemon).toHaveBeenCalledTimes(1); + expect(daemon.paths).toContain('/sql/validate-read-only'); + } finally { + await daemon.close(); + } + }); + + it('routes to the external daemon and skips the managed runtime when KTX_SQL_ANALYSIS_URL is set', async () => { + const daemon = await startLoopbackDaemon(); + const testIo = io(); + const ensureRuntime = vi.fn(async () => ({ layout: {} as never, manifest: {} as never })); + const startDaemon = vi.fn(async () => { + throw new Error('managed runtime must not start when an external daemon URL is set'); + }); + try { + const port = resolveSqlAnalysisPort( + { cliVersion: '0.2.0', projectDir: '/work/proj', installPolicy: 'auto', io: testIo.io, ensureRuntime, startDaemon }, + { KTX_SQL_ANALYSIS_URL: daemon.baseUrl }, + ); + await expect(port.validateReadOnly('select 1', 'postgres')).resolves.toEqual({ ok: true }); + expect(ensureRuntime).not.toHaveBeenCalled(); + expect(startDaemon).not.toHaveBeenCalled(); + expect(daemon.paths).toContain('/sql/validate-read-only'); + } finally { + await daemon.close(); + } + }); +}); diff --git a/packages/cli/test/mcp-server-factory.test.ts b/packages/cli/test/mcp-server-factory.test.ts index e43fcb49..7ca2d2b7 100644 --- a/packages/cli/test/mcp-server-factory.test.ts +++ b/packages/cli/test/mcp-server-factory.test.ts @@ -66,7 +66,7 @@ vi.mock('../src/managed-python-command.js', () => ({ })); vi.mock('../src/managed-python-http.js', () => ({ - createManagedDaemonSqlAnalysisPort: vi.fn(() => mocks.sqlAnalysis), + resolveSqlAnalysisPort: vi.fn(() => mocks.sqlAnalysis), })); const project = {