diff --git a/package.json b/package.json index 1d958e0..c16cae9 100644 --- a/package.json +++ b/package.json @@ -38,14 +38,14 @@ "hyparquet": "1.26.0", "hyparquet-compressors": "1.1.1", "icebird": "0.8.5", - "squirreling": "0.12.22" + "squirreling": "0.12.23" }, "optionalDependencies": { - "hyparquet-writer": "0.15.4" + "hyparquet-writer": "0.15.6" }, "overrides": { - "hyparquet-writer": "0.15.4", - "squirreling": "0.12.22" + "hyparquet-writer": "0.15.6", + "squirreling": "0.12.23" }, "devDependencies": { "@types/node": "25.9.1", diff --git a/src/core/cli/tui/index.js b/src/core/cli/tui/index.js index 288e710..1fd7743 100644 --- a/src/core/cli/tui/index.js +++ b/src/core/cli/tui/index.js @@ -25,6 +25,7 @@ export { PromptCancelledError } * @property {NodeJS.ReadableStream} [stdin] * @property {NodeJS.WritableStream} [stdout] * @property {NodeJS.ProcessEnv} [env] + * @property {boolean} [clearOnResolve] */ /** @@ -72,6 +73,7 @@ export async function multiselect(spec) { * @property {NodeJS.ReadableStream} [stdin] * @property {NodeJS.WritableStream} [stdout] * @property {NodeJS.ProcessEnv} [env] + * @property {boolean} [clearOnResolve] */ /** @@ -115,6 +117,7 @@ export async function select(spec) { * @property {NodeJS.ReadableStream} [stdin] * @property {NodeJS.WritableStream} [stdout] * @property {NodeJS.ProcessEnv} [env] + * @property {boolean} [clearOnResolve] */ /** @@ -150,6 +153,7 @@ export async function text(spec) { * @property {NodeJS.ReadableStream} [stdin] * @property {NodeJS.WritableStream} [stdout] * @property {NodeJS.ProcessEnv} [env] + * @property {boolean} [clearOnResolve] */ /** @@ -173,13 +177,14 @@ export async function confirm(spec) { } /** - * @param {{ stdin?: NodeJS.ReadableStream, stdout?: NodeJS.WritableStream, env?: NodeJS.ProcessEnv }} spec - * @returns {{ stdin: NodeJS.ReadableStream, stdout: NodeJS.WritableStream, env?: NodeJS.ProcessEnv }} + * @param {{ stdin?: NodeJS.ReadableStream, stdout?: NodeJS.WritableStream, env?: NodeJS.ProcessEnv, clearOnResolve?: boolean }} spec + * @returns {{ stdin: NodeJS.ReadableStream, stdout: NodeJS.WritableStream, env?: NodeJS.ProcessEnv, clearOnResolve?: boolean }} */ function resolveIo(spec) { return { stdin: spec.stdin ?? process.stdin, stdout: spec.stdout ?? process.stdout, ...(spec.env !== undefined ? { env: spec.env } : {}), + ...(spec.clearOnResolve ? { clearOnResolve: true } : {}), } } diff --git a/src/core/cli/tui/runtime.js b/src/core/cli/tui/runtime.js index e691b4d..c4da069 100644 --- a/src/core/cli/tui/runtime.js +++ b/src/core/cli/tui/runtime.js @@ -20,6 +20,9 @@ let activeRun = false * @property {NodeJS.ReadableStream} stdin * @property {NodeJS.WritableStream} stdout * @property {NodeJS.ProcessEnv} [env] + * @property {boolean} [clearOnResolve] Erase the prompt's frame from the + * terminal when it settles (resolve or cancel) so the next prompt + * redraws in its place instead of stacking below it. */ /** @@ -40,6 +43,7 @@ export async function run(initialState, io) { activeRun = true const color = env.NO_COLOR ? false : true + const clearOnResolve = io.clearOnResolve === true /** @type {NodeJS.ReadStream} */ const stdin = /** @type {any} */ (io.stdin) const stdout = io.stdout @@ -75,6 +79,13 @@ export async function run(initialState, io) { stdin.pause() } } catch {} + if (clearOnResolve && previousLineCount > 0) { + // Move the cursor back to the top of the rendered frame and clear + // everything below it, leaving the screen as it was before the + // prompt drew. The next prompt then redraws in the same position. + try { stdout.write(`\x1b[${previousLineCount}A\r${CLEAR_TO_END}`) } catch {} + previousLineCount = 0 + } try { stdout.write(CURSOR_SHOW) } catch {} } @@ -85,7 +96,7 @@ export async function run(initialState, io) { } const frame = render(state, { color }) buf += frame - previousLineCount = countTrailingLines(frame) + previousLineCount = countPhysicalRows(frame, terminalColumns(stdout)) stdout.write(buf) } @@ -160,17 +171,60 @@ function normalizeKey(str, key) { } /** - * Count the number of newline characters in `s`. The runtime uses this - * to know how far to move the cursor up before clearing the previous - * frame. Frames always end with `\n`, so the value equals the number of - * rows the frame occupied below the start point. + * Resolve the terminal width in columns, defaulting to 80 when the + * stream does not expose a usable `.columns` (non-TTY mocks, pipes). + * + * @param {NodeJS.WritableStream} stdout + * @returns {number} + */ +function terminalColumns(stdout) { + const cols = /** @type {any} */ (stdout).columns + return typeof cols === 'number' && cols > 0 ? cols : 80 +} + +// Match ANSI SGR (color/style) sequences so they are excluded from the +// visible-width measurement. The renderer only emits `\x1b[...m` codes. +const ANSI_SGR = /\x1b\[[0-9;]*m/g + +/** + * Visible (printable) width of a single logical line, ignoring ANSI + * style codes. Measured in code units, which matches column count for + * the Latin/punctuation text the prompts render. + * + * @param {string} line + * @returns {number} + */ +function visibleWidth(line) { + return line.replace(ANSI_SGR, '').length +} + +/** + * Count the number of *physical* terminal rows a frame occupies. The + * runtime uses this to know how far to move the cursor up before + * clearing the previous frame. A naive newline count is wrong whenever + * a logical line is wider than the terminal: the terminal soft-wraps it + * onto multiple rows, so the cursor descended further than the number of + * `\n` written. Undercounting here leaves stale rows on screen on every + * redraw — the classic "the question keeps duplicating when I move the + * cursor" symptom. + * + * Frames always end with a trailing `\n`; the empty segment after it + * contributes no row. * - * @param {string} s + * @param {string} frame + * @param {number} columns + * @returns {number} */ -function countTrailingLines(s) { - let n = 0 - for (let i = 0; i < s.length; i++) if (s.charCodeAt(i) === 10) n++ - return n +export function countPhysicalRows(frame, columns) { + const width = columns > 0 ? columns : 80 + const lines = frame.split('\n') + if (lines.length > 0 && lines[lines.length - 1] === '') lines.pop() + let rows = 0 + for (const line of lines) { + const len = visibleWidth(line) + rows += len === 0 ? 1 : Math.ceil(len / width) + } + return rows } /** diff --git a/src/core/cli/walkthrough.js b/src/core/cli/walkthrough.js index 7a6fd46..c1b6f26 100644 --- a/src/core/cli/walkthrough.js +++ b/src/core/cli/walkthrough.js @@ -11,7 +11,7 @@ import { discoverBundledPlugins } from '../runtime/bundled.js' import { buildPluginCatalog } from '../plugin_catalog.js' import { ensureDurableBinForNpx } from './global_install.js' import { detectClientSources } from './detect.js' -import { multiselect, text, confirm } from './tui/index.js' +import { multiselect, select, text } from './tui/index.js' import { isPromptCancelledError } from './tui/runtime.js' import { shouldUseTui } from './tui-router.js' @@ -426,6 +426,7 @@ function tuiPromptFactory(opts) { ...(o.checked ? { checked: true } : {}), })), ...(question.bounds ? { bounds: question.bounds } : {}), + clearOnResolve: true, stdin: opts.stdin ?? process.stdin, stdout: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (opts.stdout)), env: opts.env, @@ -452,6 +453,7 @@ function tuiRetentionPromptFactory(opts) { const n = Number.parseInt(s.trim(), 10) return Number.isInteger(n) && n >= 0 ? null : 'enter a non-negative integer' }, + clearOnResolve: true, stdin: opts.stdin ?? process.stdin, stdout: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (opts.stdout)), env: opts.env, @@ -488,9 +490,9 @@ function defaultRetentionPromptFactory(opts) { /** * Build the interactive backfill-consent prompt. Routes to the TUI - * yes/no confirm on a real TTY, else a legacy readline yes/no. Both - * default to yes so a bare enter opts in — the bead's "default backfill - * to enabled, but let the user choose no". + * arrow-navigable yes/no select on a real TTY, else a legacy readline + * yes/no. Both default to yes so a bare enter opts in — the bead's + * "default backfill to enabled, but let the user choose no". * * @param {Pick} opts * @returns {AsyncBackfillConsentPrompt} @@ -501,18 +503,28 @@ function defaultBackfillConsentPromptFactory(opts) { } /** + * Render the backfill consent as a `select` so it matches the look and + * feel of the source picker (arrow keys + pointer) rather than a plain + * y/n confirm. Cursor defaults to "Yes" so a bare enter opts in. + * * @param {Pick} opts * @returns {AsyncBackfillConsentPrompt} */ function tuiBackfillConsentPromptFactory(opts) { return async function ({ providers, retentionDays }) { - return confirm({ + const choice = await select({ title: backfillConsentTitle(providers, retentionDays), - default: true, + options: [ + { value: 'yes', label: 'Yes — import it now', summary: 'Reads local transcripts into the query cache.' }, + { value: 'no', label: 'No — skip for now', summary: 'You can import later with hyp backfill.' }, + ], + default: 'yes', + clearOnResolve: true, stdin: opts.stdin ?? process.stdin, stdout: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (opts.stdout)), env: opts.env, }) + return choice === 'yes' } } @@ -690,22 +702,13 @@ export async function runPickerWalkthrough(opts) { sourceRaw.filter((v) => PICKER_SOURCES.some((s) => s.value === v)) ) - const exportRaw = await ask({ - pickType: 'sinks', - title: 'Where should HypAware export captured data?', - options: PICKER_EXPORTS.map((e) => ({ - value: e.value, - label: e.label, - summary: e.summary, - // Default export: pre-check local Parquet so the interactive - // picker matches the documented `--yes` default (which also - // defaults to local-parquet). A plain default, not autodetect. - ...(e.value === 'local-parquet' ? { checked: true } : {}), - })), - }) - const exportChoice = /** @type {PickerExport} */ ( - PICKER_EXPORTS.find((e) => exportRaw.includes(e.value))?.value ?? 'keep-local' - ) + // Export destination is not asked interactively. A local query + // cache is always kept; on top of it we default to scheduled local + // Parquet exports so `npx hypaware` produces durable files out of + // the box. Other destinations (keep-local only, configure-later, + // S3, …) remain available via `hyp init --export ` and by + // editing the written config later. + const exportChoice = /** @type {PickerExport} */ ('local-parquet') const retentionDays = await retentionAsk('Cache retention (days)', DEFAULT_RETENTION_DAYS) picks = { sources, exportChoice, retentionDays } @@ -1100,12 +1103,16 @@ async function runPickerFinale(args) { status: 'ok', }, async (span) => { + let printedAny = false for (const skill of skills.list()) { for (const targetClient of skill.clients) { if (!clientsPicked.includes(targetClient)) continue const skillDir = skillDirMap.get(targetClient) if (!skillDir) continue const dest = path.join(homeDir, skillDir, skill.name) + // Separate the skills block from the preceding attach output. + if (!printedAny) stdout.write('\n') + printedAny = true if (dryRun) { stdout.write(`(dry-run) Would install skill '${skill.name}' → ${dest}\n`) } else { @@ -1116,6 +1123,8 @@ async function runPickerFinale(args) { summary.skillsInstalled.push({ name: skill.name, client: targetClient, dest, dryRun }) } } + // Trailing blank line so the next step (backfill prompt) stands apart. + if (printedAny) stdout.write('\n') if (span && typeof span.setAttribute === 'function') { span.setAttribute('installed_count', summary.skillsInstalled.length) } @@ -1244,6 +1253,13 @@ async function runFinaleBackfill(args) { // matching the attach/restart resilience above. for (const provider of providers) { try { + // Importing local history reads and writes potentially + // thousands of rows with no other output. Without this line + // the resolved consent frame is the last thing on screen, so a + // multi-second import looks like the prompt is stuck. Announce + // the work before it starts so the wizard visibly moves on. + const startTag = dryRun ? '(dry-run) ' : '' + stdout.write(`${startTag}backfill ${provider}: importing local history…\n`) const entry = await backfill.run({ provider, dryRun, retentionDays, until }) summary.backfill.push(entry) const tag = entry.dryRun ? '(dry-run) ' : '' diff --git a/test/core/cli/tui/runtime.test.js b/test/core/cli/tui/runtime.test.js index 3515b57..9076374 100644 --- a/test/core/cli/tui/runtime.test.js +++ b/test/core/cli/tui/runtime.test.js @@ -11,10 +11,23 @@ import { confirm, PromptCancelledError, } from '../../../../src/core/cli/tui/index.js' -import { isPromptCancelledError } from '../../../../src/core/cli/tui/runtime.js' +import { isPromptCancelledError, countPhysicalRows } from '../../../../src/core/cli/tui/runtime.js' const ENV = { NO_COLOR: '1' } +/** + * Parse the cursor-up row count (`\x1b[A`) the runtime emits at the + * start of a redraw frame. Returns 0 when the chunk has no cursor-up + * (the very first frame). + * + * @param {string} chunk + * @returns {number} + */ +function cursorUpCount(chunk) { + const m = /\x1b\[(\d+)A/.exec(chunk) + return m ? Number.parseInt(m[1], 10) : 0 +} + /** * Build a pair of PassThrough streams that look enough like a TTY for * the runtime to accept them. `stdin.setRawMode` is stubbed so the @@ -278,6 +291,85 @@ test('runtime: render failures during keypress reject and clean up', async () => assert.equal(/** @type {any} */ (io.stdin).isRaw, false) }) +test('countPhysicalRows: counts each logical line once when nothing wraps', () => { + // 'a\nb\nc\n' → three rows, trailing newline contributes none. + assert.equal(countPhysicalRows('a\nb\nc\n', 80), 3) +}) + +test('countPhysicalRows: empty lines still occupy one row', () => { + assert.equal(countPhysicalRows('a\n\nb\n', 80), 3) +}) + +test('countPhysicalRows: a line wider than the terminal counts its wrapped rows', () => { + // 25 visible chars at width 10 → ceil(25/10) = 3 physical rows. + const line = 'x'.repeat(25) + assert.equal(countPhysicalRows(line + '\n', 10), 3) +}) + +test('countPhysicalRows: a line exactly the terminal width stays one row', () => { + assert.equal(countPhysicalRows('x'.repeat(10) + '\n', 10), 1) + assert.equal(countPhysicalRows('x'.repeat(11) + '\n', 10), 2) +}) + +test('countPhysicalRows: ANSI style codes do not inflate the width', () => { + // 10 visible chars wrapped in color codes still fit one row at width 10. + const styled = '\x1b[36m' + 'x'.repeat(10) + '\x1b[0m' + assert.equal(countPhysicalRows(styled + '\n', 10), 1) +}) + +test('countPhysicalRows: defaults to 80 columns for non-TTY widths', () => { + assert.equal(countPhysicalRows('x'.repeat(81) + '\n', 0), 2) +}) + +test('runtime: redraw moves up by physical (wrapped) rows on a narrow terminal', async () => { + // Regression for the "moving the cursor duplicates the question" + // bug: long option summaries wrap to multiple physical rows, so the + // redraw must move the cursor up by the wrapped row count, not by the + // number of '\n' written. + const stdin = new PassThrough() + const stdout = new PassThrough() + Object.defineProperty(stdin, 'isTTY', { value: true }) + Object.defineProperty(stdout, 'isTTY', { value: true }) + Object.defineProperty(stdout, 'columns', { value: 24 }) + Object.defineProperty(stdin, 'isRaw', { value: false, writable: true }) + // @ts-expect-error — PassThrough has no setRawMode; the runtime probes for it. + stdin.setRawMode = (enabled) => { /** @type {any} */ (stdin).isRaw = enabled } + /** @type {string[]} */ + const chunks = [] + stdout.on('data', (chunk) => chunks.push(String(chunk))) + + const promise = multiselect({ + title: 'What do you want to collect?', + options: [ + { value: 'a', label: 'capture A', summary: 'A long summary that is certainly wider than twenty-four columns.' }, + { value: 'b', label: 'capture B', summary: 'Another summary long enough to wrap across several rows.' }, + ], + stdin, + stdout, + }) + // 'z' is ignored by the reducer but still triggers a redraw, then enter resolves. + await feed(stdin, ['z', '\r']) + await promise + + // The first frame is the first chunk carrying the title; the redraw is + // the next chunk that begins with a cursor-up move. (The CURSOR_HIDE + // escape is written as its own chunk before the first frame.) + const firstFrame = chunks.find((c) => c.includes('What do you want')) + const redrawFrame = chunks.find((c) => /\x1b\[\d+A/.test(c)) + assert.ok(firstFrame, 'first frame not captured') + assert.ok(redrawFrame, 'redraw frame not captured') + // The redraw's cursor-up count must equal the wrapped row count of the + // first frame — otherwise stale rows are left on screen (duplication). + assert.equal(cursorUpCount(redrawFrame), countPhysicalRows(firstFrame, 24)) + // And that count must exceed the naive newline count, proving wrapping + // actually occurred in this fixture. + const newlineCount = (firstFrame.match(/\n/g) ?? []).length + assert.ok( + countPhysicalRows(firstFrame, 24) > newlineCount, + 'fixture should wrap; otherwise the regression is not exercised', + ) +}) + test('runtime: overlapping prompts are rejected', async () => { const first = makeTty() const second = makeTty() diff --git a/test/core/walkthrough-detect.test.js b/test/core/walkthrough-detect.test.js index 5c0d3dd..09e774f 100644 --- a/test/core/walkthrough-detect.test.js +++ b/test/core/walkthrough-detect.test.js @@ -50,12 +50,9 @@ test('picker pre-checks detected sources, labels them, and defaults export to lo assert.notEqual(codex?.checked, true) assert.doesNotMatch(codex?.label ?? '', /detected/) - const sinks = seen.find((q) => q.pickType === 'sinks') - assert.ok(sinks, 'export question asked') - const parquet = sinks.options.find((o) => o.value === 'local-parquet') - const keepLocal = sinks.options.find((o) => o.value === 'keep-local') - assert.equal(parquet?.checked, true) - assert.notEqual(keepLocal?.checked, true) + // The export question is no longer asked — local-parquet is the + // unconditional default. + assert.equal(seen.find((q) => q.pickType === 'sinks'), undefined, 'export question must not be asked') // The confirmed picks reflect the preselected state. assert.deepEqual(result.sourcesPicked, ['claude']) diff --git a/test/core/walkthrough-prompt.test.js b/test/core/walkthrough-prompt.test.js index 67142e0..322d218 100644 --- a/test/core/walkthrough-prompt.test.js +++ b/test/core/walkthrough-prompt.test.js @@ -9,10 +9,12 @@ import { PassThrough } from 'node:stream' import { runPickerWalkthrough } from '../../src/core/cli/walkthrough.js' -test('picker prompt prints context under source and export options', async () => { +test('picker prompt prints context under source options and defaults export to local-parquet', async () => { const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'hypaware-walkthrough-prompt-')) const input = new PassThrough() - const stdout = answerDrivenOutput(input, ['3\n', '1\n', '\n']) + // Only the source question and the retention prompt are asked; the + // export question was removed in favour of the local-parquet default. + const stdout = answerDrivenOutput(input, ['3\n', '\n']) const stderr = makeBuf() const result = await runPickerWalkthrough({ @@ -28,11 +30,13 @@ test('picker prompt prints context under source and export options', async () => assert.equal(result.exitCode, 0) assert.deepEqual(result.sourcesPicked, ['raw-anthropic']) - assert.equal(result.exportPicked, 'keep-local') + assert.equal(result.exportPicked, 'local-parquet') const text = stdout.text() assert.match(text, /3\) capture raw Anthropic API traffic\n Advanced API proxy mode/) - assert.match(text, /1\) keep local query cache only\n Stores recent rows locally/) + // The export question is no longer rendered. + assert.doesNotMatch(text, /keep local query cache only/) + assert.doesNotMatch(text, /Where should HypAware export/) assert.equal(stderr.text(), '') }) diff --git a/test/core/walkthrough-tui-happy.test.js b/test/core/walkthrough-tui-happy.test.js index 0032fe6..95ee2f2 100644 --- a/test/core/walkthrough-tui-happy.test.js +++ b/test/core/walkthrough-tui-happy.test.js @@ -86,11 +86,7 @@ test('runPickerWalkthrough drives the TUI multiselect end-to-end when stdin+stdo await settle() await feed(io.stdin, ['\x1b[B', '\x1b[B', ' ', '\r']) - // Exports prompt — local-parquet is pre-checked by default, so a bare - // enter accepts it (matches the documented `--yes` default). - await settle() - await feed(io.stdin, ['\r']) - + // No export prompt: the picker always defaults to local-parquet now. // Retention prompt — empty buffer + enter accepts the 30-day default. await settle() await feed(io.stdin, ['\r']) @@ -177,9 +173,10 @@ test('runPickerWalkthrough falls back to the legacy numbered prompt under HYP_NO const input = new PassThrough() // Mark BOTH ends as TTYs so the only signal that flips the router is // the HYP_NO_TUI escape. This proves the env override wins over the - // TTY probe. + // TTY probe. Answers: source '3' (raw-anthropic), then retention + // default — the export question is no longer asked. Object.defineProperty(input, 'isTTY', { value: true }) - const stdout = answerDrivenOutput(input, ['3\n', '1\n', '\n'], true) + const stdout = answerDrivenOutput(input, ['3\n', '\n'], true) const stderr = makeBuf() // HYP_NO_TUI flows through opts.env — the same channel real callers @@ -197,7 +194,7 @@ test('runPickerWalkthrough falls back to the legacy numbered prompt under HYP_NO }) assert.equal(result.exitCode, 0) assert.deepEqual(result.sourcesPicked, ['raw-anthropic']) - assert.equal(result.exportPicked, 'keep-local') + assert.equal(result.exportPicked, 'local-parquet') // The legacy prompt prints the numbered-list signature. assert.match(stdout.text(), /select \(e\.g\. 1,3 or "all"\):/) })