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.
This commit is contained in:
Andrey Avtomonov 2026-06-04 14:11:08 +02:00 committed by GitHub
parent 8eb1cd3e79
commit c2beaf7d55
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 494 additions and 34 deletions

View file

@ -64,6 +64,27 @@ function sqlAnalysis(tablesById: Record<string, Array<{ catalog: string | null;
};
}
function sqlAnalysisWithErrors(
tablesById: Record<string, Array<{ catalog: string | null; db: string | null; name: string }>>,
errorIds: string[],
): SqlAnalysisPort {
const errors = new Set(errorIds);
return {
analyzeForFingerprint: vi.fn(),
analyzeBatch: vi.fn(async (items: SqlAnalysisBatchItem[]): Promise<Map<string, SqlAnalysisBatchResult>> =>
new Map<string, SqlAnalysisBatchResult>(
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',

View file

@ -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));
});
});

View file

@ -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),
},

View file

@ -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<unknown>) => 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│',
});
});

View file

@ -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' },
],

View file

@ -37,6 +37,7 @@ describe('telemetry event schemas', () => {
'daemon_stopped',
'sl_plan_completed',
'sql_gen_completed',
'query_history_filter_completed',
]);
});