diff --git a/packages/cli/src/status-project.ts b/packages/cli/src/status-project.ts index c3faa63d..7ed8c1c5 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' }; + const via = trimmed.startsWith('env:') ? 'env' : trimmed.startsWith('file:') ? 'file' : 'literal'; + let resolved: string | undefined; + try { + resolved = resolveKtxConfigReference(trimmed, env); + } catch { + resolved = undefined; } - if (trimmed.startsWith('file:')) { - return { resolved: trimmed.slice(5), via: 'file' }; - } - 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/status-project.test.ts b/packages/cli/test/status-project.test.ts index 30313897..1a38f7f7 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,