diff --git a/packages/cli/src/connection.ts b/packages/cli/src/connection.ts index d12dccb77..0762e8f2a 100644 --- a/packages/cli/src/connection.ts +++ b/packages/cli/src/connection.ts @@ -1,3 +1,5 @@ +import { stat } from 'node:fs/promises'; +import { resolve } from 'node:path'; import { DEFAULT_METABASE_CLIENT_CONFIG, DefaultMetabaseConnectionClientFactory } from './context/ingest/adapters/metabase/client.js'; import { DefaultLookerConnectionClientFactory } from './context/ingest/adapters/looker/factory.js'; import type { LookerClient } from './context/ingest/adapters/looker/client.js'; @@ -10,6 +12,7 @@ import { federatedConnectionListing } from './context/connections/federation.js' import { getDriverRegistration } from './context/connections/drivers.js'; import { parseNotionConnectionConfig, resolveNotionConnectionAuthToken } from './context/connections/notion-config.js'; import { resolveKtxConfigReference } from './context/core/config-reference.js'; +import { readGitRepoConnectionFields } from './context/project/git-connection-fields.js'; import { type KtxLocalProject, loadKtxProject } from './context/project/project.js'; import type { KtxScanConnector } from './context/scan/types.js'; import type { KtxCliIo } from './index.js'; @@ -183,49 +186,46 @@ async function testNotionConnection( return { bot: describeNotionBot(bot) }; } -interface GitConnectionFields { - repoUrl: string; - authToken: string | null; +async function checkLocalDbtSourceDir(projectDir: string, sourceDir: string): Promise { + const stats = await stat(resolve(projectDir, sourceDir)).catch(() => null); + if (!stats) { + throw new Error(`dbt source_dir "${sourceDir}" does not exist`); + } + if (!stats.isDirectory()) { + throw new Error(`dbt source_dir "${sourceDir}" is not a directory`); + } } -function extractGitConnectionFields( +async function testGitRepoConnection( project: KtxLocalProject, connectionId: string, driver: string, -): GitConnectionFields { + runTest: TestRepoConnection, +): Promise<{ location: string }> { const connection = project.config.connections[connectionId]; if (!connection) { throw new Error(`Connection "${connectionId}" is not configured in ktx.yaml`); } - const stringField = (value: unknown): string | null => - typeof value === 'string' && value.trim().length > 0 ? value.trim() : null; - const record = - driver === 'metricflow' && typeof connection.metricflow === 'object' && connection.metricflow !== null - ? (connection.metricflow as Record) - : (connection as Record); - const repoUrl = driver === 'dbt' ? stringField(record.repo_url) : stringField(record.repoUrl); - if (!repoUrl) { - const field = driver === 'dbt' ? 'repo_url' : 'repoUrl'; - throw new Error(`Connection "${connectionId}" (driver: ${driver}) is missing ${field}`); + const fields = readGitRepoConnectionFields(connection, driver); + + // dbt setup writes source_dir for a local project and ingest consumes it, so a + // local source check must stand in for the remote repo reachability check. + if (fields.sourceDir) { + await checkLocalDbtSourceDir(project.projectDir, fields.sourceDir); + return { location: fields.sourceDir }; } - const literalToken = stringField(record.auth_token); - const ref = stringField(record.auth_token_ref); - const resolvedRef = ref ? resolveKtxConfigReference(ref, process.env) : null; - return { repoUrl, authToken: literalToken ?? resolvedRef ?? null }; -} -async function testGitRepoConnection( - project: KtxLocalProject, - connectionId: string, - driver: string, - runTest: TestRepoConnection, -): Promise<{ repoUrl: string }> { - const { repoUrl, authToken } = extractGitConnectionFields(project, connectionId, driver); - const result = await runTest({ repoUrl, authToken }); + if (!fields.repoUrl) { + const field = driver === 'dbt' ? 'repo_url or source_dir' : 'repoUrl'; + throw new Error(`Connection "${connectionId}" (driver: ${driver}) is missing ${field}`); + } + const resolvedRef = fields.authTokenRef ? resolveKtxConfigReference(fields.authTokenRef, process.env) : null; + const authToken = fields.authTokenLiteral ?? resolvedRef ?? null; + const result = await runTest({ repoUrl: fields.repoUrl, authToken }); if (!result.ok) { throw new Error(`${driver} repository check failed: ${result.error}`); } - return { repoUrl }; + return { location: fields.repoUrl }; } interface DriverTestOutcome { @@ -278,7 +278,7 @@ async function testConnectionByDriver( driver, deps.testRepoConnection ?? testRepoConnection, ); - return { driver, detailKey: 'Repo', detailValue: result.repoUrl }; + return { driver, detailKey: 'Repo', detailValue: result.location }; } if (getDriverRegistration(driver)) { diff --git a/packages/cli/src/context/ingest/adapters/looker/local-looker.adapter.ts b/packages/cli/src/context/ingest/adapters/looker/local-looker.adapter.ts index ea8ba6581..b0dd5c53b 100644 --- a/packages/cli/src/context/ingest/adapters/looker/local-looker.adapter.ts +++ b/packages/cli/src/context/ingest/adapters/looker/local-looker.adapter.ts @@ -1,3 +1,4 @@ +import { resolveKtxConfigReference } from '../../../core/config-reference.js'; import type { KtxLocalProject } from '../../../../context/project/project.js'; import type { KtxProjectConnectionConfig } from '../../../../context/project/config.js'; import { @@ -8,13 +9,6 @@ function stringField(value: unknown): string | null { return typeof value === 'string' && value.trim().length > 0 ? value.trim() : null; } -function resolveEnvReference(ref: string, env: NodeJS.ProcessEnv): string | null { - if (!ref.startsWith('env:')) { - return null; - } - return stringField(env[ref.slice('env:'.length)]); -} - export function lookerCredentialsFromLocalConnection( connectionId: string, connection: KtxProjectConnectionConfig | undefined, @@ -27,7 +21,8 @@ export function lookerCredentialsFromLocalConnection( const clientId = stringField(connection.client_id); const clientSecret = stringField(connection.client_secret) ?? - (stringField(connection.client_secret_ref) ? resolveEnvReference(String(connection.client_secret_ref), env) : null); + resolveKtxConfigReference(stringField(connection.client_secret_ref) ?? undefined, env) ?? + null; if (!baseUrl) { throw new Error(`Connection "${connectionId}" is missing Looker base_url`); diff --git a/packages/cli/src/context/project/git-connection-fields.ts b/packages/cli/src/context/project/git-connection-fields.ts new file mode 100644 index 000000000..4cec6edca --- /dev/null +++ b/packages/cli/src/context/project/git-connection-fields.ts @@ -0,0 +1,47 @@ +import type { KtxProjectConnectionConfig } from './config.js'; + +export interface GitRepoConnectionFields { + repoUrl: string | null; + sourceDir: string | null; + branch: string | null; + path: string | null; + authTokenLiteral: string | null; + authTokenRef: string | null; +} + +function trimmedString(value: unknown): string | null { + return typeof value === 'string' && value.trim().length > 0 ? value.trim() : null; +} + +// metricflow nests its repo fields under `metricflow.*`; dbt/lookml keep them top-level. +function fieldRecord(connection: KtxProjectConnectionConfig, driver: string): Record { + if (driver === 'metricflow') { + const nested = (connection as Record).metricflow; + if (typeof nested === 'object' && nested !== null && !Array.isArray(nested)) { + return nested as Record; + } + return {}; + } + return connection as Record; +} + +/** + * Single source of truth for where the git-repo context-source drivers + * (dbt, metricflow, lookml) store their repository fields: dbt uses snake_case + * `repo_url`/`source_dir`, lookml uses camelCase `repoUrl`, metricflow nests + * camelCase fields under `metricflow.*`. + */ +export function readGitRepoConnectionFields( + connection: KtxProjectConnectionConfig, + driver: string, +): GitRepoConnectionFields { + const record = fieldRecord(connection, driver); + return { + repoUrl: trimmedString(driver === 'dbt' ? record.repo_url : record.repoUrl), + sourceDir: driver === 'dbt' ? trimmedString(record.source_dir) : null, + branch: trimmedString(record.branch), + path: trimmedString(record.path), + authTokenLiteral: trimmedString(record.auth_token), + authTokenRef: trimmedString(record.auth_token_ref), + }; +} diff --git a/packages/cli/src/local-adapters.ts b/packages/cli/src/local-adapters.ts index 671bce673..8d7cb57cf 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 392f375c5..e05422808 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 84a00253c..e66341f7f 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 { createLazyManagedPythonSemanticLayerComputePort } 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 8a5431eb9..8e76d536b 100644 --- a/packages/cli/src/sql.ts +++ b/packages/cli/src/sql.ts @@ -9,7 +9,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'; @@ -140,7 +140,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/src/status-project.ts b/packages/cli/src/status-project.ts index c3faa63de..7ed8c1c50 100644 --- a/packages/cli/src/status-project.ts +++ b/packages/cli/src/status-project.ts @@ -6,8 +6,10 @@ import { CODEX_ISOLATION_WARNING_FIX, } from './context/llm/codex-isolation.js'; import { runCodexAuthProbe } from './context/llm/codex-runtime.js'; +import { resolveKtxConfigReference } from './context/core/config-reference.js'; import type { KtxConfigIssue, KtxProjectConfig, KtxProjectConnectionConfig, KtxProjectEmbeddingConfig, KtxProjectLlmConfig } from './context/project/config.js'; import type { KtxLocalProject } from './context/project/project.js'; +import { readGitRepoConnectionFields } from './context/project/git-connection-fields.js'; import { ktxLocalStateDbPath } from './context/project/local-state-db.js'; import { isQueryHistoryEnabled, @@ -167,19 +169,21 @@ export interface ProjectStatus { }; } +// Delegates to the canonical resolver so status reports what the runtime resolves; +// it reads file: refs and throws on a missing/empty file, so the catch maps that +// to "missing" rather than letting status crash. function resolveRef(value: unknown, env: NodeJS.ProcessEnv): { resolved: string; via: 'literal' | 'env' | 'file' | 'missing' } { if (typeof value !== 'string') return { resolved: '', via: 'missing' }; const trimmed = value.trim(); if (trimmed.length === 0) return { resolved: '', via: 'missing' }; - if (trimmed.startsWith('env:')) { - const name = trimmed.slice(4).trim(); - const v = env[name]; - return v && v.trim().length > 0 ? { resolved: v, via: 'env' } : { resolved: '', via: 'missing' }; - } - if (trimmed.startsWith('file:')) { - return { resolved: trimmed.slice(5), via: 'file' }; + const via = trimmed.startsWith('env:') ? 'env' : trimmed.startsWith('file:') ? 'file' : 'literal'; + let resolved: string | undefined; + try { + resolved = resolveKtxConfigReference(trimmed, env); + } catch { + resolved = undefined; } - return { resolved: trimmed, via: 'literal' }; + return resolved && resolved.length > 0 ? { resolved, via } : { resolved: '', via: 'missing' }; } function failureDetail(error: unknown): string { @@ -350,7 +354,9 @@ function buildEmbeddingsStatus(config: KtxProjectEmbeddingConfig, env: NodeJS.Pr if (backend === 'openai') { const ref = config.openai?.api_key; const resolved = resolveRef(ref, env); - if (resolved.resolved.length > 0 || (env.OPENAI_API_KEY && env.OPENAI_API_KEY.trim().length > 0)) { + // resolveLocalKtxEmbeddingConfig requires a resolved openai.api_key and never + // falls back to a bare OPENAI_API_KEY env var, so status must not either. + if (resolved.resolved.length > 0) { return { backend, model, dimensions, status: 'ok', detail: 'key set' }; } const hint = envHint(ref); @@ -427,26 +433,25 @@ function buildConnectionStatus( case 'dbt': case 'dbt-core': case 'dbt-cloud': { - const repoUrl = - (conn as Record).repoUrl ?? - (conn as Record).repo_url; - if (typeof repoUrl === 'string' && repoUrl.length > 0) return ok(`repo: ${repoUrl}`); - return warn('repoUrl not set', 'Rerun `ktx setup`'); + const { repoUrl, sourceDir } = readGitRepoConnectionFields(conn, 'dbt'); + const source = repoUrl ?? sourceDir; + if (source) return ok(`repo: ${source}`); + return warn('repo_url not set', 'Rerun `ktx setup`'); } case 'metabase': { const url = (conn as Record).api_url; if (typeof url === 'string' && url.length > 0) return ok(`url: ${url}`); return warn('api_url not set', 'Rerun `ktx setup`'); } - case 'looker': - case 'lookml': { + case 'looker': { const url = (conn as Record).base_url ?? (conn as Record).url; if (typeof url === 'string' && url.length > 0) return ok(`url: ${url}`); return warn('base_url not set', 'Rerun `ktx setup`'); } + case 'lookml': case 'metricflow': { - const repoUrl = (conn as Record).repoUrl ?? (conn as Record).repo_url; - if (typeof repoUrl === 'string' && repoUrl.length > 0) return ok(`repo: ${repoUrl}`); + const { repoUrl } = readGitRepoConnectionFields(conn, driver); + if (repoUrl) return ok(`repo: ${repoUrl}`); return warn('repoUrl not set', 'Rerun `ktx setup`'); } default: diff --git a/packages/cli/test/connection.test.ts b/packages/cli/test/connection.test.ts index 22c8bbe9c..4069ba985 100644 --- a/packages/cli/test/connection.test.ts +++ b/packages/cli/test/connection.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import type { LookerClient } from '../src/context/ingest/adapters/looker/client.js'; @@ -470,6 +470,49 @@ describe('runKtxConnection', () => { expect(io.stderr()).toContain('dbt repository check failed: fatal: auth failed'); }); + it('tests a local dbt source_dir without a remote repo check', async () => { + const projectDir = join(tempDir, 'project'); + await initKtxProject({ projectDir }); + const sourceDir = join(tempDir, 'local-dbt'); + await mkdir(sourceDir, { recursive: true }); + await writeConnections(projectDir, { + 'dbt-local': { + driver: 'dbt', + source_dir: sourceDir, + }, + }); + const testRepoConnection = vi.fn(async () => ({ ok: true as const })); + const io = makeIo(); + + await expect( + runKtxConnection({ command: 'test', projectDir, connectionId: 'dbt-local' }, io.io, { testRepoConnection }), + ).resolves.toBe(0); + + expect(testRepoConnection).not.toHaveBeenCalled(); + expect(io.stdout()).toContain('Connection test passed: dbt-local'); + expect(io.stdout()).toContain(`Repo: ${sourceDir}`); + }); + + it('fails a dbt source_dir that does not exist', async () => { + const projectDir = join(tempDir, 'project'); + await initKtxProject({ projectDir }); + await writeConnections(projectDir, { + 'dbt-local': { + driver: 'dbt', + source_dir: join(tempDir, 'missing-dbt'), + }, + }); + const testRepoConnection = vi.fn(async () => ({ ok: true as const })); + const io = makeIo(); + + await expect( + runKtxConnection({ command: 'test', projectDir, connectionId: 'dbt-local' }, io.io, { testRepoConnection }), + ).resolves.toBe(1); + + expect(testRepoConnection).not.toHaveBeenCalled(); + expect(io.stderr()).toContain('does not exist'); + }); + it('tests a LookML connection via testRepoConnection with camelCase repoUrl', async () => { const projectDir = join(tempDir, 'project'); await initKtxProject({ projectDir }); diff --git a/packages/cli/test/context/ingest/adapters/looker/local-looker.adapter.test.ts b/packages/cli/test/context/ingest/adapters/looker/local-looker.adapter.test.ts new file mode 100644 index 000000000..3cbed0333 --- /dev/null +++ b/packages/cli/test/context/ingest/adapters/looker/local-looker.adapter.test.ts @@ -0,0 +1,72 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { lookerCredentialsFromLocalConnection } from '../../../../../src/context/ingest/adapters/looker/local-looker.adapter.js'; +import type { KtxProjectConnectionConfig } from '../../../../../src/context/project/config.js'; + +const connectionId = '11111111-1111-4111-8111-111111111111'; + +function lookerConnection(overrides: Partial): KtxProjectConnectionConfig { + return { + driver: 'looker', + base_url: 'https://looker.example.com', + client_id: 'client-123', + ...overrides, + } as KtxProjectConnectionConfig; +} + +describe('lookerCredentialsFromLocalConnection', () => { + let secretsDir: string; + + beforeEach(async () => { + secretsDir = await mkdtemp(join(tmpdir(), 'looker-secret-')); + }); + + afterEach(async () => { + await rm(secretsDir, { recursive: true, force: true }); + }); + + it('resolves client_secret_ref written as a local secret file (ktx setup default)', async () => { + const secretPath = join(secretsDir, 'looker-main-client-secret'); + await writeFile(secretPath, 'file-secret\n', 'utf-8'); + + const credentials = lookerCredentialsFromLocalConnection( + connectionId, + lookerConnection({ client_secret_ref: `file:${secretPath}` }), // pragma: allowlist secret + {}, + ); + + expect(credentials.client_secret).toBe('file-secret'); + }); + + it('resolves client_secret_ref from the environment', () => { + const credentials = lookerCredentialsFromLocalConnection( + connectionId, + lookerConnection({ client_secret_ref: 'env:LOOKER_CLIENT_SECRET' }), // pragma: allowlist secret + { LOOKER_CLIENT_SECRET: 'env-secret' }, // pragma: allowlist secret + ); + + expect(credentials.client_secret).toBe('env-secret'); + }); + + it('prefers a literal client_secret over the reference', () => { + const credentials = lookerCredentialsFromLocalConnection( + connectionId, + lookerConnection({ client_secret: 'literal-secret', client_secret_ref: 'env:UNSET' }), // pragma: allowlist secret + {}, + ); + + expect(credentials.client_secret).toBe('literal-secret'); + }); + + it('throws when neither client_secret nor a resolvable client_secret_ref is present', () => { + expect(() => + lookerCredentialsFromLocalConnection( + connectionId, + lookerConnection({ client_secret_ref: 'env:UNSET' }), // pragma: allowlist secret + {}, + ), + ).toThrow(/missing Looker client_secret/); + }); +}); diff --git a/packages/cli/test/doctor.test.ts b/packages/cli/test/doctor.test.ts index 9d9aa1a9f..e420674e2 100644 --- a/packages/cli/test/doctor.test.ts +++ b/packages/cli/test/doctor.test.ts @@ -457,6 +457,8 @@ describe('runKtxDoctor', () => { ' backend: openai', ' model: text-embedding-3-small', ' dimensions: 1536', + ' openai:', + ' api_key: env:OPENAI_API_KEY', // pragma: allowlist secret '', ].join('\n'), 'utf-8', @@ -592,6 +594,8 @@ describe('runKtxDoctor', () => { ' backend: openai', ' model: text-embedding-3-small', ' dimensions: 1536', + ' openai:', + ' api_key: env:OPENAI_API_KEY', // pragma: allowlist secret '', ].join('\n'), 'utf-8', diff --git a/packages/cli/test/managed-python-http.test.ts b/packages/cli/test/managed-python-http.test.ts index ef06960fa..84461efea 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 eb378bcff..16f4ddfad 100644 --- a/packages/cli/test/mcp-server-factory.test.ts +++ b/packages/cli/test/mcp-server-factory.test.ts @@ -67,7 +67,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 = { diff --git a/packages/cli/test/status-project.test.ts b/packages/cli/test/status-project.test.ts index 303138975..1a38f7f78 100644 --- a/packages/cli/test/status-project.test.ts +++ b/packages/cli/test/status-project.test.ts @@ -144,6 +144,80 @@ describe('buildProjectStatus embeddings', () => { expect(status.embeddings.status).toBe('warn'); expect(status.verdictReason).toMatch(/embedding credentials missing/); }); + + // resolveLocalKtxEmbeddingConfig never consults a bare OPENAI_API_KEY, so status + // must not show ready when only the env var (not config.openai.api_key) is set. + it('reports openai backend with only OPENAI_API_KEY env set as warn', async () => { + const project = projectWithConfig( + withEmbeddings(baseProjectConfig(), { + backend: 'openai', + model: 'text-embedding-3-small', + dimensions: 1536, + openai: {}, + }), + ); + + const status = await buildProjectStatus(project, { + env: { OPENAI_API_KEY: 'sk-test' }, // pragma: allowlist secret + claudeCodeAuthProbe: stubClaudeCodeAuthProbe, + }); + + expect(status.embeddings.status).toBe('warn'); + expect(status.verdictReason).toMatch(/embedding credentials missing/); + }); + + describe('openai file: api_key references', () => { + let secretDir: string; + + beforeEach(async () => { + secretDir = await mkdtemp(join(tmpdir(), 'ktx-status-secret-')); + }); + + afterEach(async () => { + await rm(secretDir, { recursive: true, force: true }); + }); + + // A file: ref must be read, not trusted as a path; a missing file is not ready. + it('reports a missing file: api_key as warn', async () => { + const project = projectWithConfig( + withEmbeddings(baseProjectConfig(), { + backend: 'openai', + model: 'text-embedding-3-small', + dimensions: 1536, + openai: { api_key: `file:${join(secretDir, 'absent')}` }, // pragma: allowlist secret + }), + ); + + const status = await buildProjectStatus(project, { + env: {}, + claudeCodeAuthProbe: stubClaudeCodeAuthProbe, + }); + + expect(status.embeddings.status).toBe('warn'); + expect(status.verdictReason).toMatch(/embedding credentials missing/); + }); + + it('reports a populated file: api_key as ok', async () => { + const secretPath = join(secretDir, 'openai-key'); + await writeFile(secretPath, 'sk-from-file\n', 'utf8'); // pragma: allowlist secret + const project = projectWithConfig( + withEmbeddings(baseProjectConfig(), { + backend: 'openai', + model: 'text-embedding-3-small', + dimensions: 1536, + openai: { api_key: `file:${secretPath}` }, // pragma: allowlist secret + }), + ); + + const status = await buildProjectStatus(project, { + env: {}, + claudeCodeAuthProbe: stubClaudeCodeAuthProbe, + }); + + expect(status.embeddings.status).toBe('ok'); + expect(status.embeddings.detail).toBe('key set'); + }); + }); }); function withPostgresQueryHistory(config: KtxProjectConfig): KtxProjectConfig { @@ -209,6 +283,42 @@ function withMysqlQueryHistory(config: KtxProjectConfig): KtxProjectConfig { }; } +function withGitRepoSources(config: KtxProjectConfig): KtxProjectConfig { + return { + ...config, + connections: { + ...config.connections, + metrics: { + driver: 'metricflow', + metricflow: { repoUrl: 'https://github.com/example/metricflow' }, + } as KtxProjectConfig['connections'][string], + looks: { + driver: 'lookml', + repoUrl: 'https://github.com/example/lookml', + } as KtxProjectConfig['connections'][string], + local_dbt: { + driver: 'dbt', + source_dir: '/repo/dbt', + } as KtxProjectConfig['connections'][string], + }, + }; +} + +describe('buildProjectStatus git-repo connections', () => { + it('reports schema-valid metricflow, lookml, and dbt connections as ok', async () => { + const project = projectWithConfig(withGitRepoSources(baseProjectConfig())); + + const status = await buildProjectStatus(project, { + claudeCodeAuthProbe: stubClaudeCodeAuthProbe, + }); + + const byName = new Map(status.connections.map((conn) => [conn.name, conn])); + expect(byName.get('metrics')).toMatchObject({ driver: 'metricflow', status: 'ok' }); + expect(byName.get('looks')).toMatchObject({ driver: 'lookml', status: 'ok' }); + expect(byName.get('local_dbt')).toMatchObject({ driver: 'dbt', status: 'ok' }); + }); +}); + function fakeStatusRunner( dialect: 'postgres' | 'snowflake' | 'bigquery', catalogName: string,