From c2beaf7d5569197f7267697f5d65ac3dd1c60d9f Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Thu, 4 Jun 2026 14:11:08 +0200 Subject: [PATCH] feat(setup): wizard prompt tweaks and quieter query-history filter output (#259) Setup wizard flow tweaks: - Add a reveal-tail password prompt (reveal-password-prompt.ts) that unmasks the last few characters of a typed/pasted secret, and wire it into the setup prompt adapter in place of clack's password(); adds the @clack/core dep. - Reorder wizard select options: surface "Paste a key" before the environment-variable option across embeddings/models/sources, promote Metabase/Notion in the source list, put Git URL before Local path, reorder the Notion crawl-mode choices, and relabel the sources "Done" action. Query-history filter picker output: - Collapse the per-template parse-failure lines into a single count in the setup output and route the full template-id list to --debug stderr. - Model parse failures as a structured parseFailedTemplateIds field instead of warning strings. - Add a privacy-safe query_history_filter_completed telemetry event (counts/enums only), mirrored into the Python daemon schema. --- packages/cli/package.json | 1 + packages/cli/src/commands/setup-commands.ts | 3 + .../query-history-filter-picker.ts | 9 +- packages/cli/src/reveal-password-prompt.ts | 93 +++++++++++++++++++ packages/cli/src/setup-databases.ts | 45 +++++++-- packages/cli/src/setup-embeddings.ts | 2 +- packages/cli/src/setup-models.ts | 2 +- packages/cli/src/setup-prompts.ts | 4 +- packages/cli/src/setup-sources.ts | 14 +-- packages/cli/src/setup.ts | 2 + packages/cli/src/telemetry/events.schema.json | 82 ++++++++++++++++ packages/cli/src/telemetry/events.ts | 16 ++++ .../query-history-filter-picker.test.ts | 48 ++++++++++ .../cli/test/reveal-password-prompt.test.ts | 40 ++++++++ packages/cli/test/setup-databases.test.ts | 51 ++++++++++ packages/cli/test/setup-prompts.test.ts | 13 ++- packages/cli/test/setup-sources.test.ts | 12 +-- packages/cli/test/telemetry/events.test.ts | 1 + pnpm-lock.yaml | 3 + .../ktx_daemon/telemetry/events.schema.json | 82 ++++++++++++++++ .../tests/test_telemetry_schema_sync.py | 1 + uv.lock | 4 +- 22 files changed, 494 insertions(+), 34 deletions(-) create mode 100644 packages/cli/src/reveal-password-prompt.ts create mode 100644 packages/cli/test/reveal-password-prompt.test.ts diff --git a/packages/cli/package.json b/packages/cli/package.json index 939a8b9c..ba769d58 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -47,6 +47,7 @@ "@ai-sdk/devtools": "0.0.18", "@ai-sdk/google-vertex": "^4.0.134", "@anthropic-ai/claude-agent-sdk": "0.3.146", + "@clack/core": "1.3.1", "@clack/prompts": "1.4.0", "@clickhouse/client": "^1.18.5", "@commander-js/extra-typings": "14.0.0", diff --git a/packages/cli/src/commands/setup-commands.ts b/packages/cli/src/commands/setup-commands.ts index 1619a80a..0302e9ed 100644 --- a/packages/cli/src/commands/setup-commands.ts +++ b/packages/cli/src/commands/setup-commands.ts @@ -406,6 +406,8 @@ export function registerSetupCommands(program: Command, context: KtxCliCommandCo } const resolvedAgentScope = options.local ? 'local' : options.global ? 'global' : 'project'; + const debugEnabled = + ((command.optsWithGlobals ? command.optsWithGlobals() : command.opts()) as { debug?: unknown }).debug === true; await runSetupArgs(context, { command: 'run', projectDir: resolveCommandProjectDir(command), @@ -415,6 +417,7 @@ export function registerSetupCommands(program: Command, context: KtxCliCommandCo agentScope: resolvedAgentScope, skipAgents: options.skipAgents === true, inputMode: options.input === false ? 'disabled' : 'auto', + ...(debugEnabled ? { debug: true } : {}), yes: options.yes === true, cliVersion: context.packageInfo.version, ...(options.llmBackend ? { llmBackend: options.llmBackend } : {}), diff --git a/packages/cli/src/context/ingest/adapters/historic-sql/query-history-filter-picker.ts b/packages/cli/src/context/ingest/adapters/historic-sql/query-history-filter-picker.ts index bb296513..3f77900d 100644 --- a/packages/cli/src/context/ingest/adapters/historic-sql/query-history-filter-picker.ts +++ b/packages/cli/src/context/ingest/adapters/historic-sql/query-history-filter-picker.ts @@ -23,6 +23,7 @@ export interface QueryHistoryFilterProposal { consideredRoleCount: number; skipped: { reason: 'no-llm' | 'no-daemon' | 'no-in-scope-history' | 'user-block-present' } | null; warnings: string[]; + parseFailedTemplateIds: string[]; } export interface ProposeQueryHistoryServiceAccountFiltersInput { @@ -74,7 +75,7 @@ const queryHistoryFilterAdjudicationSchema = z.object({ type QueryHistoryFilterAdjudication = z.infer; function emptyProposal(skipped: QueryHistoryFilterProposal['skipped'], warnings: string[] = []): QueryHistoryFilterProposal { - return { excludedRoles: [], consideredRoleCount: 0, skipped, warnings }; + return { excludedRoles: [], consideredRoleCount: 0, skipped, warnings, parseFailedTemplateIds: [] }; } function displayTableRef(ref: KtxTableRef): string { @@ -180,6 +181,7 @@ export async function proposeQueryHistoryServiceAccountFilters( const windowDays = 'windowDays' in config ? config.windowDays : 90; const windowStart = new Date(now.getTime() - windowDays * 24 * 60 * 60 * 1000); const warnings: string[] = []; + const parseFailedTemplateIds: string[] = []; const snapshot: AggregatedTemplate[] = []; try { @@ -212,7 +214,7 @@ export async function proposeQueryHistoryServiceAccountFilters( for (const template of snapshot) { const parsed = analysis.get(template.templateId); if (!parsed || parsed.error) { - warnings.push(`query_history_filter_picker_parse_failed:${template.templateId}`); + parseFailedTemplateIds.push(template.templateId); continue; } const tablesTouched = [...new Map(parsed.tablesTouched.map((ref) => [tableRefKey(ref), ref])).values()] @@ -236,6 +238,7 @@ export async function proposeQueryHistoryServiceAccountFilters( consideredRoleCount: records.length, skipped: { reason: 'no-in-scope-history' }, warnings, + parseFailedTemplateIds, }; } @@ -256,6 +259,7 @@ export async function proposeQueryHistoryServiceAccountFilters( ...warnings, `query_history_filter_picker_llm_failed:${error instanceof Error ? error.message : String(error)}`, ], + parseFailedTemplateIds, }; } @@ -274,5 +278,6 @@ export async function proposeQueryHistoryServiceAccountFilters( consideredRoleCount: records.length, skipped: input.userServiceAccountsPresent ? { reason: 'user-block-present' } : null, warnings, + parseFailedTemplateIds, }; } diff --git a/packages/cli/src/reveal-password-prompt.ts b/packages/cli/src/reveal-password-prompt.ts new file mode 100644 index 00000000..3fe3ed66 --- /dev/null +++ b/packages/cli/src/reveal-password-prompt.ts @@ -0,0 +1,93 @@ +import { styleText } from 'node:util'; +import { PasswordPrompt, type PasswordOptions } from '@clack/core'; +import { S_BAR, S_BAR_END, S_PASSWORD_MASK, settings, symbol } from '@clack/prompts'; + +// How many trailing characters of a pasted secret to leave visible so the user +// can confirm what landed (e.g. `••••••a1b2`). Kept small on purpose. +const REVEAL_TAIL_COUNT = 4; + +/** + * Mask every character of `userInput` except the last `tail`, but only reveal the + * tail once the secret is long enough that the hidden portion still dominates + * (`length > tail * 2`). Short secrets stay fully masked so we never expose most + * of a small value. The returned string keeps the same code-unit length as the + * input so clack's cursor slicing in `userInputWithCursor` stays aligned. + * + * @internal + */ +export function maskRevealingTail(userInput: string, maskChar: string, tail: number): string { + const revealLength = userInput.length > tail * 2 ? tail : 0; + const hiddenLength = userInput.length - revealLength; + return maskChar.repeat(hiddenLength) + userInput.slice(hiddenLength); +} + +class RevealTailPasswordPrompt extends PasswordPrompt { + readonly #maskChar: string; + readonly #tail: number; + + constructor(options: PasswordOptions & { tail: number }) { + super(options); + this.#maskChar = options.mask ?? S_PASSWORD_MASK; + this.#tail = options.tail; + } + + override get masked(): string { + return maskRevealingTail(this.userInput, this.#maskChar, this.#tail); + } +} + +// Reproduces the @clack/prompts password frame (pinned to the installed version) +// so this prompt is visually identical to every other setup prompt; the only +// behavioral change is the tail-revealing `masked` getter above. +function renderPasswordFrame(prompt: Omit, message: string): string { + const withGuide = settings.withGuide; + const title = `${withGuide ? `${styleText('gray', S_BAR)}\n` : ''}${symbol(prompt.state)} ${message}\n`; + const masked = prompt.masked; + switch (prompt.state) { + case 'error': { + const bar = withGuide ? `${styleText('yellow', S_BAR)} ` : ''; + const end = withGuide ? `${styleText('yellow', S_BAR_END)} ` : ''; + return `${title.trim()}\n${bar}${masked}\n${end}${styleText('yellow', prompt.error)}\n`; + } + case 'submit': { + const bar = withGuide ? `${styleText('gray', S_BAR)} ` : ''; + return `${title}${bar}${masked ? styleText('dim', masked) : ''}`; + } + case 'cancel': { + const bar = withGuide ? `${styleText('gray', S_BAR)} ` : ''; + const body = masked ? styleText(['strikethrough', 'dim'], masked) : ''; + return `${title}${bar}${body}${masked && withGuide ? `\n${styleText('gray', S_BAR)}` : ''}`; + } + default: { + const bar = withGuide ? `${styleText('cyan', S_BAR)} ` : ''; + const end = withGuide ? styleText('cyan', S_BAR_END) : ''; + return `${title}${bar}${prompt.userInputWithCursor}\n${end}\n`; + } + } +} + +export interface RevealPasswordOptions { + message: string; + mask?: string; + tail?: number; + validate?: PasswordOptions['validate']; + signal?: AbortSignal; +} + +/** + * Drop-in replacement for clack's `password()` that reveals the last few + * characters of the entered value while typing. Resolves to the raw value or the + * clack cancel symbol, matching `password()`'s contract. + */ +export function revealPassword(options: RevealPasswordOptions): Promise { + const prompt = new RevealTailPasswordPrompt({ + mask: options.mask ?? S_PASSWORD_MASK, + tail: options.tail ?? REVEAL_TAIL_COUNT, + validate: options.validate, + signal: options.signal, + render() { + return renderPasswordFrame(this, options.message); + }, + }); + return prompt.prompt() as Promise; +} diff --git a/packages/cli/src/setup-databases.ts b/packages/cli/src/setup-databases.ts index 3cb6c5d2..002ead30 100644 --- a/packages/cli/src/setup-databases.ts +++ b/packages/cli/src/setup-databases.ts @@ -73,6 +73,7 @@ export type KtxSetupDatabaseDriver = export interface KtxSetupDatabasesArgs { projectDir: string; inputMode: 'auto' | 'disabled'; + debug?: boolean; yes?: boolean; cliVersion?: string; runtimeInstallPolicy?: KtxManagedPythonInstallPolicy; @@ -1626,7 +1627,12 @@ function hasServiceAccountsBlock(connection: KtxProjectConnectionConfig | undefi return 'serviceAccounts' in filters; } -function printQueryHistoryFilterProposal(io: KtxCliIo, proposal: QueryHistoryFilterProposal): void { +function printQueryHistoryFilterProposal(io: KtxCliIo, proposal: QueryHistoryFilterProposal, debug = false): void { + if (debug && proposal.parseFailedTemplateIds.length > 0) { + io.stderr.write( + `[debug] query-history filter picker could not parse ${proposal.parseFailedTemplateIds.length} template(s): ${proposal.parseFailedTemplateIds.join(', ')}\n`, + ); + } if (proposal.excludedRoles.length === 0) { if (proposal.skipped?.reason === 'no-llm') { io.stdout.write('│ Query-history filter picker skipped: no LLM is configured.\n'); @@ -1635,6 +1641,12 @@ function printQueryHistoryFilterProposal(io: KtxCliIo, proposal: QueryHistoryFil } else if (proposal.skipped?.reason === 'no-in-scope-history') { io.stdout.write('│ Query-history filter picker found no in-scope service-account exclusions.\n'); } + if (proposal.parseFailedTemplateIds.length > 0) { + const count = proposal.parseFailedTemplateIds.length; + io.stdout.write( + `│ Skipped ${count} query template${count === 1 ? '' : 's'} ktx could not parse (run with --debug to list them).\n`, + ); + } for (const warning of proposal.warnings) { io.stdout.write(`│ ! ${warning}\n`); } @@ -1727,12 +1739,17 @@ async function maybeProposeQueryHistoryFilters(input: { deps: input.deps, }); if (!llmRuntime && !input.deps.queryHistoryFilterPicker) { - printQueryHistoryFilterProposal(input.io, { - excludedRoles: [], - consideredRoleCount: 0, - skipped: { reason: 'no-llm' }, - warnings: [], - }); + printQueryHistoryFilterProposal( + input.io, + { + excludedRoles: [], + consideredRoleCount: 0, + skipped: { reason: 'no-llm' }, + warnings: [], + parseFailedTemplateIds: [], + }, + input.args.debug === true, + ); return; } @@ -1773,7 +1790,19 @@ async function maybeProposeQueryHistoryFilters(input: { userServiceAccountsPresent, }); - printQueryHistoryFilterProposal(input.io, proposal); + printQueryHistoryFilterProposal(input.io, proposal, input.args.debug === true); + await emitTelemetryEvent({ + name: 'query_history_filter_completed', + projectDir: input.projectDir, + io: input.io, + fields: { + dialect, + consideredRoleCount: proposal.consideredRoleCount, + excludedRoleCount: proposal.excludedRoles.length, + parseFailedCount: proposal.parseFailedTemplateIds.length, + outcome: 'ok', + }, + }); if (proposal.skipped?.reason === 'user-block-present') { input.io.stdout.write('│ Existing query-history service-account filters left unchanged.\n'); return; diff --git a/packages/cli/src/setup-embeddings.ts b/packages/cli/src/setup-embeddings.ts index 8f49bcf1..5d02e3e4 100644 --- a/packages/cli/src/setup-embeddings.ts +++ b/packages/cli/src/setup-embeddings.ts @@ -222,8 +222,8 @@ async function chooseCredentialRef( const choice = await prompts.select({ message: `How should KTX find your ${embeddingBackendDisplayName(backend)} embedding API key?`, options: [ - { value: 'env', label: `Use ${defaultEnv} from the environment` }, { value: 'paste', label: 'Paste a key and save it as a local secret file' }, + { value: 'env', label: `Use ${defaultEnv} from the environment` }, { value: 'back', label: 'Back' }, ], }); diff --git a/packages/cli/src/setup-models.ts b/packages/cli/src/setup-models.ts index 8e8cf30b..e673cb99 100644 --- a/packages/cli/src/setup-models.ts +++ b/packages/cli/src/setup-models.ts @@ -470,8 +470,8 @@ async function chooseCredentialRef( const choice = await prompts.select({ message: `How should KTX find your Anthropic API key?\n\n${ANTHROPIC_CREDENTIAL_PROMPT_CONTEXT}`, options: [ - { value: 'env', label: 'Use ANTHROPIC_API_KEY from the environment' }, { value: 'paste', label: 'Paste a key and save it as a local secret file' }, + { value: 'env', label: 'Use ANTHROPIC_API_KEY from the environment' }, { value: 'back', label: 'Back' }, ], }); diff --git a/packages/cli/src/setup-prompts.ts b/packages/cli/src/setup-prompts.ts index 1609bd76..e508d8ff 100644 --- a/packages/cli/src/setup-prompts.ts +++ b/packages/cli/src/setup-prompts.ts @@ -9,12 +9,12 @@ import { log, multiselect, note, - password, select, text, } from '@clack/prompts'; import type { KtxCliIo } from './cli-runtime.js'; import { withMenuOptionsSpacing, withTextInputNavigation } from './prompt-navigation.js'; +import { revealPassword } from './reveal-password-prompt.js'; import { withSetupInterruptConfirmation } from './setup-interrupt.js'; export interface KtxSetupPromptOption { @@ -189,7 +189,7 @@ export function createKtxSetupPromptAdapter(options: KtxSetupPromptAdapterOption }, async password(promptOptions) { const value = await withSetupInterruptConfirmation(() => - password({ ...promptOptions, message: withTextInputNavigation(promptOptions.message) }), + revealPassword({ ...promptOptions, message: withTextInputNavigation(promptOptions.message) }), ); return isCancel(value) ? undefined : String(value); }, diff --git a/packages/cli/src/setup-sources.ts b/packages/cli/src/setup-sources.ts index 0a66c3a7..25552fbf 100644 --- a/packages/cli/src/setup-sources.ts +++ b/packages/cli/src/setup-sources.ts @@ -119,11 +119,11 @@ export interface KtxSetupSourcesDeps { const SOURCE_OPTIONS: Array<{ value: KtxSetupSourceType; label: string }> = [ { value: 'dbt', label: 'dbt' }, - { value: 'metricflow', label: 'MetricFlow' }, { value: 'metabase', label: 'Metabase' }, + { value: 'notion', label: 'Notion' }, + { value: 'metricflow', label: 'MetricFlow' }, { value: 'looker', label: 'Looker' }, { value: 'lookml', label: 'LookML' }, - { value: 'notion', label: 'Notion' }, ]; const SOURCE_LABELS = Object.fromEntries(SOURCE_OPTIONS.map((option) => [option.value, option.label])) as Record< @@ -269,8 +269,8 @@ async function chooseSourceCredentialRef(input: { message: `How should KTX find your ${input.label}?`, options: [ ...(input.existingRef ? [{ value: 'keep', label: 'Keep existing credential' }] : []), - { value: 'env', label: `Use ${input.envName} from the environment` }, { value: 'paste', label: 'Paste a key and save it as a local secret file' }, + { value: 'env', label: `Use ${input.envName} from the environment` }, { value: 'back', label: 'Back' }, ], }); @@ -307,8 +307,8 @@ async function chooseGitAuthCredentialRef(input: { message: `${label} repo requires authentication.`, options: [ ...(input.existingRef ? [{ value: 'keep', label: 'Keep existing credential' }] : []), - { value: 'env', label: 'Use GITHUB_TOKEN from the environment' }, { value: 'paste', label: 'Paste a token and save it as a local secret file' }, + { value: 'env', label: 'Use GITHUB_TOKEN from the environment' }, { value: 'skip', label: 'Skip — try without authentication' }, { value: 'back', label: 'Back' }, ], @@ -1063,8 +1063,8 @@ async function promptForInteractiveSource( const selectedLocation = await prompts.select({ message: `${source} source location`, options: [ - { value: 'path', label: 'Local path' }, { value: 'git', label: 'Git URL' }, + { value: 'path', label: 'Local path' }, { value: 'back', label: 'Back' }, ], }); @@ -1343,8 +1343,8 @@ async function promptForInteractiveSource( const crawlMode = await prompts.select({ message: 'Which Notion pages should KTX ingest?', options: [ - { value: 'selected_roots', label: 'Specific pages and their subpages (choose them in a picker)' }, { value: 'all_accessible', label: 'All pages the integration can access' }, + { value: 'selected_roots', label: 'Specific pages and their subpages (choose them in a picker)' }, { value: 'back', label: 'Back' }, ], }); @@ -2064,7 +2064,7 @@ export async function runKtxSetupSourcesStep( const addMore = await prompts.select({ message: `${readyConnectionIds.length} context source${readyConnectionIds.length > 1 ? 's' : ''} configured (${readyConnectionIds.join(', ')}). Add another?`, options: [ - { value: 'done', label: 'Done — continue to context build' }, + { value: 'done', label: 'Done adding context sources' }, { value: 'edit', label: 'Edit an existing context source' }, { value: 'add', label: 'Add another context source' }, ], diff --git a/packages/cli/src/setup.ts b/packages/cli/src/setup.ts index a991367e..fc45abb3 100644 --- a/packages/cli/src/setup.ts +++ b/packages/cli/src/setup.ts @@ -80,6 +80,7 @@ export type KtxSetupArgs = agentScope?: KtxAgentScope; skipAgents?: boolean; inputMode: 'auto' | 'disabled'; + debug?: boolean; yes: boolean; cliVersion: string; llmBackend?: KtxSetupLlmBackend; @@ -735,6 +736,7 @@ async function runKtxSetupInner(args: KtxSetupArgs, io: KtxCliIo, deps: KtxSetup { projectDir: projectResult.projectDir, inputMode: args.inputMode, + ...(args.debug !== undefined ? { debug: args.debug } : {}), yes: args.yes, cliVersion: args.cliVersion, runtimeInstallPolicy: setupRuntimeInstallPolicy(args), diff --git a/packages/cli/src/telemetry/events.schema.json b/packages/cli/src/telemetry/events.schema.json index a75f92f1..c6c3d6f8 100644 --- a/packages/cli/src/telemetry/events.schema.json +++ b/packages/cli/src/telemetry/events.schema.json @@ -206,6 +206,17 @@ "errorClass", "durationMs" ] + }, + { + "name": "query_history_filter_completed", + "description": "Emitted after the setup query-history service-account filter picker runs.", + "fields": [ + "dialect", + "consideredRoleCount", + "excludedRoleCount", + "parseFailedCount", + "outcome" + ] } ], "$defs": { @@ -1434,6 +1445,77 @@ "durationMs" ], "additionalProperties": false + }, + "query_history_filter_completed": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "cliVersion": { + "type": "string" + }, + "nodeVersion": { + "type": "string" + }, + "osPlatform": { + "type": "string" + }, + "osRelease": { + "type": "string" + }, + "arch": { + "type": "string" + }, + "runtime": { + "type": "string", + "enum": [ + "node", + "daemon-py" + ] + }, + "isCi": { + "type": "boolean" + }, + "dialect": { + "type": "string" + }, + "consideredRoleCount": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "excludedRoleCount": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "parseFailedCount": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "outcome": { + "type": "string", + "enum": [ + "ok", + "error" + ] + } + }, + "required": [ + "cliVersion", + "nodeVersion", + "osPlatform", + "osRelease", + "arch", + "runtime", + "isCi", + "dialect", + "consideredRoleCount", + "excludedRoleCount", + "parseFailedCount", + "outcome" + ], + "additionalProperties": false } } } diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts index c4fc2e6f..cf650492 100644 --- a/packages/cli/src/telemetry/events.ts +++ b/packages/cli/src/telemetry/events.ts @@ -206,6 +206,16 @@ const sqlGenCompletedSchema = telemetryCommonEnvelopeSchema }) .strict(); +const queryHistoryFilterCompletedSchema = telemetryCommonEnvelopeSchema + .extend({ + dialect: z.string(), + consideredRoleCount: z.number().int().nonnegative(), + excludedRoleCount: z.number().int().nonnegative(), + parseFailedCount: z.number().int().nonnegative(), + outcome: outcomeSchema, + }) + .strict(); + /** @internal */ export const telemetryEventSchemas = { install_first_run: installFirstRunSchema, @@ -225,6 +235,7 @@ export const telemetryEventSchemas = { daemon_stopped: daemonStoppedSchema, sl_plan_completed: slPlanCompletedSchema, sql_gen_completed: sqlGenCompletedSchema, + query_history_filter_completed: queryHistoryFilterCompletedSchema, } as const; /** @internal */ @@ -360,6 +371,11 @@ export const telemetryEventCatalog = [ description: 'Emitted after daemon SQL generation completes.', fields: ['outcome', 'dialect', 'errorClass', 'durationMs'], }, + { + name: 'query_history_filter_completed', + description: 'Emitted after the setup query-history service-account filter picker runs.', + fields: ['dialect', 'consideredRoleCount', 'excludedRoleCount', 'parseFailedCount', 'outcome'], + }, ] as const; export type TelemetryEventName = keyof typeof telemetryEventSchemas; diff --git a/packages/cli/test/context/ingest/adapters/historic-sql/query-history-filter-picker.test.ts b/packages/cli/test/context/ingest/adapters/historic-sql/query-history-filter-picker.test.ts index 4c295092..5c9e2e60 100644 --- a/packages/cli/test/context/ingest/adapters/historic-sql/query-history-filter-picker.test.ts +++ b/packages/cli/test/context/ingest/adapters/historic-sql/query-history-filter-picker.test.ts @@ -64,6 +64,27 @@ function sqlAnalysis(tablesById: Record>, + errorIds: string[], +): SqlAnalysisPort { + const errors = new Set(errorIds); + return { + analyzeForFingerprint: vi.fn(), + analyzeBatch: vi.fn(async (items: SqlAnalysisBatchItem[]): Promise> => + new Map( + items.map((item) => [ + item.id, + errors.has(item.id) + ? { tablesTouched: [], columnsByClause: {}, error: 'parse boom' } + : { tablesTouched: tablesById[item.id] ?? [], columnsByClause: {} }, + ]), + ), + ), + validateReadOnly: vi.fn(async () => ({ ok: true })), + }; +} + function llm(decisions: Array<{ role: string; exclude: boolean; reason: string }>): KtxLlmRuntimePort { const generateObject = vi.fn(async () => ({ roles: decisions })) as KtxLlmRuntimePort['generateObject']; return { @@ -198,6 +219,7 @@ describe('query-history filter picker', () => { consideredRoleCount: 0, skipped: { reason: 'no-llm' }, warnings: [], + parseFailedTemplateIds: [], }); }); @@ -227,6 +249,32 @@ describe('query-history filter picker', () => { expect(proposal.skipped).toEqual({ reason: 'no-in-scope-history' }); }); + it('records parse failures as template ids, not warnings', async () => { + const proposal = await proposeQueryHistoryServiceAccountFilters({ + connectionId: 'warehouse', + dialect: 'postgres', + queryClient: {}, + reader: reader( + aggregate({ + templateId: 'good', + canonicalSql: 'select * from analytics.orders', + topUsers: [{ user: 'analyst', executions: 30 }], + }), + aggregate({ + templateId: 'broken', + canonicalSql: 'select * from where', + topUsers: [{ user: 'analyst', executions: 5 }], + }), + ), + sqlAnalysis: sqlAnalysisWithErrors({ good: [{ catalog: null, db: 'analytics', name: 'orders' }] }, ['broken']), + llmRuntime: llm([]), + pullConfig: { dialect: 'postgres', enabledSchemas: ['analytics'], filters: { dropTrivialProbes: true } }, + }); + + expect(proposal.parseFailedTemplateIds).toEqual(['broken']); + expect(proposal.warnings).toEqual([]); + }); + it('keeps clean in-scope history when the model excludes nothing', async () => { const proposal = await proposeQueryHistoryServiceAccountFilters({ connectionId: 'warehouse', diff --git a/packages/cli/test/reveal-password-prompt.test.ts b/packages/cli/test/reveal-password-prompt.test.ts new file mode 100644 index 00000000..7bb8cc10 --- /dev/null +++ b/packages/cli/test/reveal-password-prompt.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import { maskRevealingTail } from '../src/reveal-password-prompt.js'; + +const MASK = '▪'; + +describe('maskRevealingTail', () => { + it('reveals the last `tail` characters of a long value', () => { + const value = 'example-token-value-abcd'; + const masked = maskRevealingTail(value, MASK, 4); + expect(masked).toBe(`${MASK.repeat(value.length - 4)}abcd`); + expect(masked.endsWith('abcd')).toBe(true); + }); + + it('keeps the same length as the input so cursor slicing stays aligned', () => { + for (const secret of ['', 'a', 'abcdefgh', 'abcdefghijklmnop']) { + expect(maskRevealingTail(secret, MASK, 4)).toHaveLength(secret.length); + } + }); + + it('fully masks secrets that are not longer than tail * 2', () => { + expect(maskRevealingTail('abcdefgh', MASK, 4)).toBe(MASK.repeat(8)); + expect(maskRevealingTail('abcd', MASK, 4)).toBe(MASK.repeat(4)); + expect(maskRevealingTail('ab', MASK, 4)).toBe(MASK.repeat(2)); + }); + + it('reveals the tail once the secret crosses the tail * 2 boundary', () => { + // length 9 > 8 → reveal last 4, hide the first 5 + expect(maskRevealingTail('abcdefghi', MASK, 4)).toBe(`${MASK.repeat(5)}fghi`); + }); + + it('fully masks an empty value', () => { + expect(maskRevealingTail('', MASK, 4)).toBe(''); + }); + + it('honors a custom tail count', () => { + // tail 2 reveals only when length > 4 + expect(maskRevealingTail('abcde', MASK, 2)).toBe(`${MASK.repeat(3)}de`); + expect(maskRevealingTail('abcd', MASK, 2)).toBe(MASK.repeat(4)); + }); +}); diff --git a/packages/cli/test/setup-databases.test.ts b/packages/cli/test/setup-databases.test.ts index 6adb0af0..957dfdb2 100644 --- a/packages/cli/test/setup-databases.test.ts +++ b/packages/cli/test/setup-databases.test.ts @@ -2654,6 +2654,7 @@ describe('setup databases step', () => { consideredRoleCount: 2, skipped: null, warnings: [], + parseFailedTemplateIds: [], })); const result = await runKtxSetupDatabasesStep( @@ -2706,6 +2707,54 @@ describe('setup databases step', () => { expect(io.stdout()).toContain('svc_loader'); }); + it('collapses query-history parse failures to a count and lists ids only with --debug', async () => { + const io = makeIo(); + const queryHistoryFilterPicker = vi.fn(async () => ({ + excludedRoles: [], + consideredRoleCount: 1, + skipped: { reason: 'no-in-scope-history' as const }, + warnings: [], + parseFailedTemplateIds: ['111', '222'], + })); + + const result = await runKtxSetupDatabasesStep( + { + projectDir: tempDir, + inputMode: 'disabled', + debug: true, + yes: true, + databaseDrivers: ['postgres'], + databaseConnectionId: 'warehouse', + databaseUrl: 'env:DATABASE_URL', + databaseSchemas: ['public'], + enableQueryHistory: true, + skipDatabases: false, + }, + io.io, + { + testConnection: vi.fn(async () => 0), + scanConnection: vi.fn(async () => 0), + historicSqlReadinessProbe: vi.fn(async () => { + const runner = fakeHistoricSqlRunner('postgres', 'pg_stat_statements'); + return { + ok: true as const, + dialect: 'postgres' as const, + runner, + result: { pgServerVersion: 'PostgreSQL 16.4', warnings: [], info: [] }, + }; + }), + queryHistoryFilterPicker, + createQueryHistoryLlmRuntime: vi.fn(() => null), + }, + ); + + expect(result.status).toBe('ready'); + expect(io.stdout()).toContain('Skipped 2 query templates ktx could not parse'); + expect(io.stdout()).not.toContain('111'); + expect(io.stdout()).not.toContain('222'); + expect(io.stderr()).toContain('could not parse 2 template(s): 111, 222'); + }); + it('lets interactive setup skip applying derived filters', async () => { const io = makeIo(); const prompts = makePromptAdapter({ @@ -2743,6 +2792,7 @@ describe('setup databases step', () => { consideredRoleCount: 2, skipped: null, warnings: [], + parseFailedTemplateIds: [], })), createQueryHistoryLlmRuntime: vi.fn(() => null), }, @@ -2811,6 +2861,7 @@ describe('setup databases step', () => { consideredRoleCount: 2, skipped: { reason: 'user-block-present' as const }, warnings: [], + parseFailedTemplateIds: [], })), createQueryHistoryLlmRuntime: vi.fn(() => null), }, diff --git a/packages/cli/test/setup-prompts.test.ts b/packages/cli/test/setup-prompts.test.ts index 46628b1c..8e83c558 100644 --- a/packages/cli/test/setup-prompts.test.ts +++ b/packages/cli/test/setup-prompts.test.ts @@ -17,7 +17,7 @@ const mocks = vi.hoisted(() => { autocomplete: vi.fn(), autocompleteMultiselect: vi.fn(), note: vi.fn(), - password: vi.fn(), + revealPassword: vi.fn(), select: vi.fn(), text: vi.fn(), withSetupInterruptConfirmation: vi.fn((prompt: () => Promise) => prompt()), @@ -34,11 +34,14 @@ vi.mock('@clack/prompts', () => ({ autocomplete: mocks.autocomplete, autocompleteMultiselect: mocks.autocompleteMultiselect, note: mocks.note, - password: mocks.password, select: mocks.select, text: mocks.text, })); +vi.mock('../src/reveal-password-prompt.js', () => ({ + revealPassword: mocks.revealPassword, +})); + vi.mock('../src/setup-interrupt.js', () => ({ withSetupInterruptConfirmation: mocks.withSetupInterruptConfirmation, })); @@ -54,7 +57,7 @@ describe('setup prompt adapter', () => { mocks.autocomplete.mockReset(); mocks.autocompleteMultiselect.mockReset(); mocks.note.mockReset(); - mocks.password.mockReset(); + mocks.revealPassword.mockReset(); mocks.select.mockReset(); mocks.text.mockReset(); mocks.withSetupInterruptConfirmation.mockClear(); @@ -96,7 +99,7 @@ describe('setup prompt adapter', () => { it('decorates text and password prompts with setup navigation copy', async () => { mocks.text.mockResolvedValueOnce('analytics-ktx'); - mocks.password.mockResolvedValueOnce('secret'); + mocks.revealPassword.mockResolvedValueOnce('secret'); const adapter = createKtxSetupPromptAdapter({ selectCancelValue: 'back' }); await expect(adapter.text({ message: 'Project folder path', placeholder: './analytics-ktx' })).resolves.toBe( @@ -108,7 +111,7 @@ describe('setup prompt adapter', () => { message: 'Project folder path\n│ Press Escape to go back.\n│', placeholder: './analytics-ktx', }); - expect(mocks.password).toHaveBeenCalledWith({ + expect(mocks.revealPassword).toHaveBeenCalledWith({ message: 'Anthropic API key\n│ Press Escape to go back.\n│', }); }); diff --git a/packages/cli/test/setup-sources.test.ts b/packages/cli/test/setup-sources.test.ts index e4f7af2d..ef18a1b6 100644 --- a/packages/cli/test/setup-sources.test.ts +++ b/packages/cli/test/setup-sources.test.ts @@ -447,8 +447,8 @@ describe('setup sources step', () => { expect(testPrompts.select).toHaveBeenCalledWith({ message: 'Which Notion pages should KTX ingest?', options: [ - { value: 'selected_roots', label: 'Specific pages and their subpages (choose them in a picker)' }, { value: 'all_accessible', label: 'All pages the integration can access' }, + { value: 'selected_roots', label: 'Specific pages and their subpages (choose them in a picker)' }, { value: 'back', label: 'Back' }, ], }); @@ -891,8 +891,8 @@ describe('setup sources step', () => { expect(testPrompts.select).toHaveBeenCalledWith({ message: 'This repo requires authentication.', options: [ - { value: 'env', label: 'Use GITHUB_TOKEN from the environment' }, { value: 'paste', label: 'Paste a token and save it as a local secret file' }, + { value: 'env', label: 'Use GITHUB_TOKEN from the environment' }, { value: 'skip', label: 'Skip — try without authentication' }, { value: 'back', label: 'Back' }, ], @@ -1407,8 +1407,8 @@ describe('setup sources step', () => { message: 'How should KTX find your Notion integration token?', options: [ { value: 'keep', label: 'Keep existing credential' }, - { value: 'env', label: 'Use NOTION_TOKEN from the environment' }, { value: 'paste', label: 'Paste a key and save it as a local secret file' }, + { value: 'env', label: 'Use NOTION_TOKEN from the environment' }, { value: 'back', label: 'Back' }, ], }); @@ -1476,8 +1476,8 @@ describe('setup sources step', () => { message: 'How should KTX find your Metabase API key?', options: [ { value: 'keep', label: 'Keep existing credential' }, - { value: 'env', label: 'Use METABASE_API_KEY from the environment' }, { value: 'paste', label: 'Paste a key and save it as a local secret file' }, + { value: 'env', label: 'Use METABASE_API_KEY from the environment' }, { value: 'back', label: 'Back' }, ], }); @@ -1582,8 +1582,8 @@ describe('setup sources step', () => { message: 'This MetricFlow repo requires authentication.', options: [ { value: 'keep', label: 'Keep existing credential' }, - { value: 'env', label: 'Use GITHUB_TOKEN from the environment' }, { value: 'paste', label: 'Paste a token and save it as a local secret file' }, + { value: 'env', label: 'Use GITHUB_TOKEN from the environment' }, { value: 'skip', label: 'Skip — try without authentication' }, { value: 'back', label: 'Back' }, ], @@ -1627,7 +1627,7 @@ describe('setup sources step', () => { expect(testPrompts.select).toHaveBeenCalledWith({ message: '1 context source configured (dbt-main). Add another?', options: [ - { value: 'done', label: 'Done — continue to context build' }, + { value: 'done', label: 'Done adding context sources' }, { value: 'edit', label: 'Edit an existing context source' }, { value: 'add', label: 'Add another context source' }, ], diff --git a/packages/cli/test/telemetry/events.test.ts b/packages/cli/test/telemetry/events.test.ts index 29108600..033c2def 100644 --- a/packages/cli/test/telemetry/events.test.ts +++ b/packages/cli/test/telemetry/events.test.ts @@ -37,6 +37,7 @@ describe('telemetry event schemas', () => { 'daemon_stopped', 'sl_plan_completed', 'sql_gen_completed', + 'query_history_filter_completed', ]); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a3eaad5f..871931c0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -131,6 +131,9 @@ importers: '@anthropic-ai/claude-agent-sdk': specifier: 0.3.146 version: 0.3.146(@anthropic-ai/sdk@0.97.1(zod@4.4.3))(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(zod@4.4.3) + '@clack/core': + specifier: 1.3.1 + version: 1.3.1 '@clack/prompts': specifier: 1.4.0 version: 1.4.0 diff --git a/python/ktx-daemon/src/ktx_daemon/telemetry/events.schema.json b/python/ktx-daemon/src/ktx_daemon/telemetry/events.schema.json index a75f92f1..c6c3d6f8 100644 --- a/python/ktx-daemon/src/ktx_daemon/telemetry/events.schema.json +++ b/python/ktx-daemon/src/ktx_daemon/telemetry/events.schema.json @@ -206,6 +206,17 @@ "errorClass", "durationMs" ] + }, + { + "name": "query_history_filter_completed", + "description": "Emitted after the setup query-history service-account filter picker runs.", + "fields": [ + "dialect", + "consideredRoleCount", + "excludedRoleCount", + "parseFailedCount", + "outcome" + ] } ], "$defs": { @@ -1434,6 +1445,77 @@ "durationMs" ], "additionalProperties": false + }, + "query_history_filter_completed": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "cliVersion": { + "type": "string" + }, + "nodeVersion": { + "type": "string" + }, + "osPlatform": { + "type": "string" + }, + "osRelease": { + "type": "string" + }, + "arch": { + "type": "string" + }, + "runtime": { + "type": "string", + "enum": [ + "node", + "daemon-py" + ] + }, + "isCi": { + "type": "boolean" + }, + "dialect": { + "type": "string" + }, + "consideredRoleCount": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "excludedRoleCount": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "parseFailedCount": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "outcome": { + "type": "string", + "enum": [ + "ok", + "error" + ] + } + }, + "required": [ + "cliVersion", + "nodeVersion", + "osPlatform", + "osRelease", + "arch", + "runtime", + "isCi", + "dialect", + "consideredRoleCount", + "excludedRoleCount", + "parseFailedCount", + "outcome" + ], + "additionalProperties": false } } } diff --git a/python/ktx-daemon/tests/test_telemetry_schema_sync.py b/python/ktx-daemon/tests/test_telemetry_schema_sync.py index 6f2ba634..0cc822f9 100644 --- a/python/ktx-daemon/tests/test_telemetry_schema_sync.py +++ b/python/ktx-daemon/tests/test_telemetry_schema_sync.py @@ -36,4 +36,5 @@ def test_python_schema_copy_matches_node_schema() -> None: "daemon_stopped", "sl_plan_completed", "sql_gen_completed", + "query_history_filter_completed", ] diff --git a/uv.lock b/uv.lock index 6d00951d..40553e46 100644 --- a/uv.lock +++ b/uv.lock @@ -466,7 +466,7 @@ wheels = [ [[package]] name = "ktx-daemon" -version = "0.8.0" +version = "0.9.0" source = { editable = "python/ktx-daemon" } dependencies = [ { name = "fastapi" }, @@ -523,7 +523,7 @@ dev = [ [[package]] name = "ktx-sl" -version = "0.8.0" +version = "0.9.0" source = { editable = "python/ktx-sl" } dependencies = [ { name = "pydantic" },