Fix review findings: integration/tui-prompts (hy-3a7ri)

package.json

@@ -28,7 +28,7 @@
   },
   "scripts": {
     "lint": "node scripts/check-syntax.js",
-    "test": "find test -name '*.test.js' -print0 | xargs -0 node --test",
+    "test": "node scripts/run-tests.js",
     "typecheck": "tsc -p tsconfig.json --noEmit",
     "smoke": "node ./hypaware-core/smoke/index.js"
   },

scripts/run-tests.js

@@ -0,0 +1,57 @@
+#!/usr/bin/env node
+// @ts-check
+
+import { spawnSync } from 'node:child_process'
+import fs from 'node:fs'
+import path from 'node:path'
+import process from 'node:process'
+
+const ROOT = 'test'
+const IGNORED_DIRS = new Set(['.git', '.github', 'node_modules'])
+
+/** @type {string[]} */
+const files = []
+collectTestFiles(path.resolve(ROOT), files)
+files.sort()
+
+if (files.length === 0) {
+  process.stderr.write(`no test files found under ${ROOT}\n`)
+  process.exit(1)
+}
+
+const result = spawnSync(
+  process.execPath,
+  ['--test', ...files, ...process.argv.slice(2)],
+  { stdio: 'inherit' },
+)
+
+if (result.error) {
+  process.stderr.write(`failed to spawn node --test: ${result.error.message}\n`)
+  process.exit(1)
+}
+process.exit(result.status ?? 1)
+
+/**
+ * @param {string} dir
+ * @param {string[]} out
+ */
+function collectTestFiles(dir, out) {
+  let entries
+  try {
+    entries = fs.readdirSync(dir, { withFileTypes: true })
+  } catch (err) {
+    if (/** @type {NodeJS.ErrnoException} */ (err).code === 'ENOENT') return
+    throw err
+  }
+  for (const entry of entries) {
+    if (entry.isDirectory()) {
+      if (!IGNORED_DIRS.has(entry.name)) {
+        collectTestFiles(path.join(dir, entry.name), out)
+      }
+      continue
+    }
+    if (entry.isFile() && entry.name.endsWith('.test.js')) {
+      out.push(path.join(dir, entry.name))
+    }
+  }
+}

src/core/cli/tui-router.js

@@ -12,11 +12,17 @@ import process from 'node:process'
  * a deliberate escape hatch for CI shells that report `isTTY=true` but
  * are wrapped by something that mangles ANSI sequences.
  *
- * @param {{ stdin?: NodeJS.ReadableStream, stdout?: unknown }} opts
+ * The env lookup goes through `opts.env` when supplied so callers that
+ * inject env (the walkthrough threads `ctx.env`) get the same answer
+ * the rest of the command pipeline does. Falls back to `process.env`
+ * only when no env is provided.
+ *
+ * @param {{ stdin?: NodeJS.ReadableStream, stdout?: unknown, env?: NodeJS.ProcessEnv }} opts
  * @returns {boolean}
  */
 export function shouldUseTui(opts) {
-  if (process.env.HYP_NO_TUI === '1') return false
+  const env = opts.env ?? process.env
+  if (env.HYP_NO_TUI === '1') return false
   const inp = opts.stdin ?? process.stdin
   const out = opts.stdout
   return isTty(inp) && isTty(out)

src/core/cli/tui/index.js

@@ -24,6 +24,7 @@ export { PromptCancelledError }
  * @property {{ min?: number, max?: number }} [bounds]
  * @property {NodeJS.ReadableStream} [stdin]
  * @property {NodeJS.WritableStream} [stdout]
+ * @property {NodeJS.ProcessEnv} [env]
  */
 
 /**
@@ -70,6 +71,7 @@ export async function multiselect(spec) {
  * @property {string|number} [default]
  * @property {NodeJS.ReadableStream} [stdin]
  * @property {NodeJS.WritableStream} [stdout]
+ * @property {NodeJS.ProcessEnv} [env]
  */
 
 /**
@@ -112,6 +114,7 @@ export async function select(spec) {
  * @property {boolean} [mask]
  * @property {NodeJS.ReadableStream} [stdin]
  * @property {NodeJS.WritableStream} [stdout]
+ * @property {NodeJS.ProcessEnv} [env]
  */
 
 /**
@@ -146,6 +149,7 @@ export async function text(spec) {
  * @property {boolean} [default]
  * @property {NodeJS.ReadableStream} [stdin]
  * @property {NodeJS.WritableStream} [stdout]
+ * @property {NodeJS.ProcessEnv} [env]
  */
 
 /**
@@ -169,12 +173,13 @@ export async function confirm(spec) {
 }
 
 /**
- * @param {{ stdin?: NodeJS.ReadableStream, stdout?: NodeJS.WritableStream }} spec
- * @returns {{ stdin: NodeJS.ReadableStream, stdout: NodeJS.WritableStream }}
+ * @param {{ stdin?: NodeJS.ReadableStream, stdout?: NodeJS.WritableStream, env?: NodeJS.ProcessEnv }} spec
+ * @returns {{ stdin: NodeJS.ReadableStream, stdout: NodeJS.WritableStream, env?: NodeJS.ProcessEnv }}
  */
 function resolveIo(spec) {
   return {
     stdin:  spec.stdin  ?? process.stdin,
     stdout: spec.stdout ?? process.stdout,
+    ...(spec.env !== undefined ? { env: spec.env } : {}),
   }
 }

src/core/cli/walkthrough.js

@@ -8,9 +8,17 @@ import { Attr, getLogger, withSpan } from '../observability/index.js'
 import { defaultConfigPath } from '../config/schema.js'
 import { readObservabilityEnv } from '../observability/env.js'
 import { ensureDurableBinForNpx } from './global_install.js'
-import { multiselect, text } from './tui/index.js'
+import { multiselect, text, PromptCancelledError } from './tui/index.js'
 import { shouldUseTui } from './tui-router.js'
 
+/**
+ * Exit code returned when the user cancels the picker walkthrough
+ * (escape / ctrl+c at any TUI prompt). 130 matches the POSIX
+ * convention for SIGINT and keeps the dispatcher from reporting the
+ * cancel as an unhandled exception.
+ */
+export const WALKTHROUGH_CANCEL_EXIT_CODE = 130
+
 /** @typedef {import('../../../collectivus-plugin-kernel-types').AiGatewayCapability} AiGatewayCapability */
 /** @typedef {import('../../../collectivus-plugin-kernel-types').CapabilityRegistry} CapabilityRegistry */
 /** @typedef {import('../../../collectivus-plugin-kernel-types').HypAwareV2Config} HypAwareV2Config */
@@ -445,6 +453,7 @@ function tuiPromptFactory(opts) {
       ...(question.bounds ? { bounds: question.bounds } : {}),
       stdin: opts.stdin ?? process.stdin,
       stdout: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (opts.stdout)),
+      env: opts.env,
     })
     return /** @type {string[]} */ (result)
   }
@@ -470,6 +479,7 @@ function tuiRetentionPromptFactory(opts) {
       },
       stdin: opts.stdin ?? process.stdin,
       stdout: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (opts.stdout)),
+      env: opts.env,
     })
     const trimmed = v.trim()
     if (trimmed === '') return defaultDays
@@ -667,26 +677,33 @@ export async function runPickerWalkthrough(opts) {
 
     stdout.write('Welcome to HypAware — the local logs+telemetry collector.\n\n')
 
-    const sourceRaw = await ask({
-      pickType: 'sources',
-      title: 'What do you want to collect? (space to toggle, enter to confirm)',
-      options: PICKER_SOURCES.map((s) => ({ value: s.value, label: s.label, summary: s.summary })),
-    })
-    const sources = /** @type {PickerSource[]} */ (
-      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 })),
-    })
-    const exportChoice = /** @type {PickerExport} */ (
-      PICKER_EXPORTS.find((e) => exportRaw.includes(e.value))?.value ?? 'keep-local'
-    )
+    try {
+      const sourceRaw = await ask({
+        pickType: 'sources',
+        title: 'What do you want to collect? (space to toggle, enter to confirm)',
+        options: PICKER_SOURCES.map((s) => ({ value: s.value, label: s.label, summary: s.summary })),
+      })
+      const sources = /** @type {PickerSource[]} */ (
+        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 })),
+      })
+      const exportChoice = /** @type {PickerExport} */ (
+        PICKER_EXPORTS.find((e) => exportRaw.includes(e.value))?.value ?? 'keep-local'
+      )
 
-    const retentionDays = await retentionAsk('Cache retention (days)', DEFAULT_RETENTION_DAYS)
-    picks = { sources, exportChoice, retentionDays }
+      const retentionDays = await retentionAsk('Cache retention (days)', DEFAULT_RETENTION_DAYS)
+      picks = { sources, exportChoice, retentionDays }
+    } catch (err) {
+      if (err instanceof PromptCancelledError) {
+        return cancelledResult(opts)
+      }
+      throw err
+    }
   }
 
   for (const value of picks.sources) {
@@ -1148,6 +1165,40 @@ function clientSkillDir(client) {
   return '.codex/skills'
 }
 
+/**
+ * Build the canonical cancel result returned by {@link runPickerWalkthrough}
+ * when the user cancels via escape / ctrl+c. Writes a one-line cancel
+ * notice to stderr so the dispatcher does not eat it silently, and
+ * surfaces {@link WALKTHROUGH_CANCEL_EXIT_CODE} (130, matching SIGINT
+ * convention) as the exit code. The returned object satisfies the
+ * required shape of {@link PickerWalkthroughResult} but contains no
+ * config — callers that key off `exitCode` already short-circuit on
+ * non-zero values.
+ *
+ * @param {RunPickerWalkthroughOptions} opts
+ * @returns {PickerWalkthroughResult}
+ */
+function cancelledResult(opts) {
+  try {
+    opts.stderr.write('hyp init: cancelled\n')
+  } catch {
+    // best-effort: stderr might be closed during cleanup
+  }
+  return {
+    exitCode: WALKTHROUGH_CANCEL_EXIT_CODE,
+    configPath: '',
+    config: /** @type {HypAwareV2Config} */ ({
+      version: 2,
+      plugins: [],
+      query: { cache: { retention: { default_days: DEFAULT_RETENTION_DAYS } } },
+    }),
+    sourcesPicked: [],
+    exportPicked: 'keep-local',
+    clientsPicked: [],
+    retentionDays: DEFAULT_RETENTION_DAYS,
+  }
+}
+
 /**
  * @param {string} src
  * @param {string} dest

test/core/cli/tui/non-tty.test.js

@@ -99,3 +99,24 @@ test('HYP_NO_TUI=1 forces the same TTY error even for fake-TTY streams', async (
     else process.env.HYP_NO_TUI = prevFlag
   }
 })
+
+test('injected env.HYP_NO_TUI=1 forces the TTY error even when process.env is clean', async () => {
+  const io = makeFakeTty()
+  const prevFlag = process.env.HYP_NO_TUI
+  delete process.env.HYP_NO_TUI
+  try {
+    await assert.rejects(
+      multiselect({
+        title: 'pick',
+        options: [{ value: 'a', label: 'A' }],
+        stdin: io.stdin,
+        stdout: io.stdout,
+        env: { HYP_NO_TUI: '1' },
+      }),
+      (err) => err instanceof Error && ERROR_RE.test(err.message),
+    )
+  } finally {
+    if (prevFlag === undefined) delete process.env.HYP_NO_TUI
+    else process.env.HYP_NO_TUI = prevFlag
+  }
+})

test/core/walkthrough-tui-happy.test.js

@@ -7,7 +7,10 @@ import os from 'node:os'
 import path from 'node:path'
 import { PassThrough } from 'node:stream'
 
-import { runPickerWalkthrough } from '../../src/core/cli/walkthrough.js'
+import {
+  runPickerWalkthrough,
+  WALKTHROUGH_CANCEL_EXIT_CODE,
+} from '../../src/core/cli/walkthrough.js'
 
 /**
  * Build a PassThrough pair that satisfies the TUI runtime's `isTTY` and
@@ -109,40 +112,74 @@ test('runPickerWalkthrough drives the TUI multiselect end-to-end when stdin+stdo
   }
 })
 
-test('runPickerWalkthrough falls back to the legacy numbered prompt under HYP_NO_TUI=1', async () => {
-  const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'hypaware-walkthrough-tui-fallback-'))
-  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.
-  Object.defineProperty(input, 'isTTY', { value: true })
-  const stdout = answerDrivenOutput(input, ['3\n', '1\n', '\n'], true)
+test('runPickerWalkthrough returns a deterministic cancel exit code when the user cancels at the source prompt', async () => {
+  const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'hypaware-walkthrough-cancel-'))
+  const io = makeFakeTty()
   const stderr = makeBuf()
 
-  const prev = process.env.HYP_NO_TUI
-  process.env.HYP_NO_TUI = '1'
+  const prevNoColor = process.env.NO_COLOR
+  process.env.NO_COLOR = '1'
+  const prevNoTui = process.env.HYP_NO_TUI
+  delete process.env.HYP_NO_TUI
+
   try {
-    const result = await runPickerWalkthrough({
+    const promise = runPickerWalkthrough({
       capabilities: /** @type {any} */ ({}),
-      stdout,
+      stdout: io.stdout,
       stderr,
-      stdin: /** @type {any} */ (input),
+      stdin: io.stdin,
       env: {
         HOME: tmp,
         HYP_HOME: path.join(tmp, '.hyp'),
       },
     })
-    assert.equal(result.exitCode, 0)
-    assert.deepEqual(result.sourcesPicked, ['raw-anthropic'])
-    assert.equal(result.exportPicked, 'keep-local')
-    // The legacy prompt prints the numbered-list signature.
-    assert.match(stdout.text(), /select \(e\.g\. 1,3 or "all"\):/)
+
+    // ctrl+c at the first prompt cancels the walkthrough.
+    await settle()
+    await feed(io.stdin, ['\x03'])
+
+    const result = await promise
+    assert.equal(result.exitCode, WALKTHROUGH_CANCEL_EXIT_CODE)
+    assert.equal(result.exitCode, 130)
+    assert.match(stderr.text(), /hyp init: cancelled/)
   } finally {
-    if (prev === undefined) delete process.env.HYP_NO_TUI
-    else process.env.HYP_NO_TUI = prev
+    if (prevNoColor === undefined) delete process.env.NO_COLOR
+    else process.env.NO_COLOR = prevNoColor
+    if (prevNoTui === undefined) delete process.env.HYP_NO_TUI
+    else process.env.HYP_NO_TUI = prevNoTui
   }
 })
 
+test('runPickerWalkthrough falls back to the legacy numbered prompt under HYP_NO_TUI=1', async () => {
+  const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'hypaware-walkthrough-tui-fallback-'))
+  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.
+  Object.defineProperty(input, 'isTTY', { value: true })
+  const stdout = answerDrivenOutput(input, ['3\n', '1\n', '\n'], true)
+  const stderr = makeBuf()
+
+  // HYP_NO_TUI flows through opts.env — the same channel real callers
+  // use — so this test also exercises the env-threading contract.
+  const result = await runPickerWalkthrough({
+    capabilities: /** @type {any} */ ({}),
+    stdout,
+    stderr,
+    stdin: /** @type {any} */ (input),
+    env: {
+      HOME: tmp,
+      HYP_HOME: path.join(tmp, '.hyp'),
+      HYP_NO_TUI: '1',
+    },
+  })
+  assert.equal(result.exitCode, 0)
+  assert.deepEqual(result.sourcesPicked, ['raw-anthropic'])
+  assert.equal(result.exportPicked, 'keep-local')
+  // The legacy prompt prints the numbered-list signature.
+  assert.match(stdout.text(), /select \(e\.g\. 1,3 or "all"\):/)
+})
+
 /**
  * @param {PassThrough} input
  * @param {string[]} answers