mirror of
https://github.com/Kaelio/ktx.git
synced 2026-07-25 12:01:03 +02:00
fix(cli): align ktx status checks with the runtime resolvers
Route status config references through resolveKtxConfigReference so file: keys are read (not reported as ready by trusting the raw path), read git-repo connection fields through the shared readGitRepoConnectionFields, and stop reporting openai embeddings as ready from a bare OPENAI_API_KEY since resolveLocalKtxEmbeddingConfig requires a resolved openai.api_key.
This commit is contained in:
parent
834e60625c
commit
7692d92ef0
2 changed files with 133 additions and 18 deletions
|
|
@ -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<string, unknown>).repoUrl ??
|
||||
(conn as Record<string, unknown>).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<string, unknown>).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<string, unknown>).base_url ?? (conn as Record<string, unknown>).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<string, unknown>).repoUrl ?? (conn as Record<string, unknown>).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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue