From f6e80962c0869e5b7f74f4716e4ef1ffa4e783e6 Mon Sep 17 00:00:00 2001 From: Serhii Zhabskyi Date: Thu, 9 Jul 2026 22:54:17 +0200 Subject: [PATCH] fix(cli): stop interactive prompts hanging behind the status spinner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit install, uninstall, and refresh held a clack spinner across the whole run; its redraw timer overwrote the interactive prompts underneath (skill-pack select, broken-link, invalid-resource confirm, uninstall drift, and the refresh consent prompt), so the command waited on invisible stdin — refresh silently timed out after 5 minutes and skipped the pack. Skip the spinner whenever a run may prompt (real TTY, no --force/--dry-run) via a shared mayPrompt() gate, mirroring init. Also harden the prompt primitives: - confirm() now delegates to the shared readLine helper, so a closed or erroring stdin declines gracefully instead of hanging on Ctrl-D or crashing. - readLine() resolves to '' (decline) on a stream error instead of throwing. - install --force --dry-run now previews the same resource set a real --force install writes (invalid resources were wrongly dropped from the preview). Tests: refresh spinner-gating, confirm() EOF/error via readLine, readLine stream-error, and pool-resolution force+dry-run precedence. --- .agentsmesh/lessons/lessons.json | 56 +++++++++++++++ .../fix-interactive-install-spinner-hang.md | 7 ++ src/cli/command-handlers.ts | 44 +++++++++--- src/install/core/pool-resolution.ts | 43 ++++-------- src/install/core/prompts.ts | 17 +++-- src/install/prompts/prompt-io.ts | 10 +++ .../command-handlers-refresh-spinner.test.ts | 70 +++++++++++++++++++ ...nd-handlers-uninstall-installs-mcp.test.ts | 40 +++++++++++ .../command-handlers-watch-install.test.ts | 60 ++++++++++++++++ .../install/pool-resolution-branches.test.ts | 14 ++++ tests/unit/install/prompts-confirm.test.ts | 47 +++++++++++++ tests/unit/install/prompts/prompt-io.test.ts | 16 +++++ 12 files changed, 378 insertions(+), 46 deletions(-) create mode 100644 .changeset/fix-interactive-install-spinner-hang.md create mode 100644 tests/unit/cli/command-handlers-refresh-spinner.test.ts create mode 100644 tests/unit/install/prompts-confirm.test.ts diff --git a/.agentsmesh/lessons/lessons.json b/.agentsmesh/lessons/lessons.json index a8e5d689..45859d92 100644 --- a/.agentsmesh/lessons/lessons.json +++ b/.agentsmesh/lessons/lessons.json @@ -478,6 +478,54 @@ "t-kw-3bb206ca" ] }, + "cli-presentation-a-clack-spinner-and-an": { + "createdAt": "2026-07-09", + "evidence": [ + "src/cli/command-handlers.ts", + "src/install/prompts/bulk-prompt.ts", + "src/install/prompts/modified-files-prompt.ts" + ], + "rule": "A clack spinner and an interactive readline prompt cannot share a TTY: the spinner's redraw timer overwrites the prompt line, so the question is invisible and the command hangs on stdin. In src/cli/command-handlers.ts, any handler whose runX may prompt (install → broken-link/bulk prompts; uninstall → modified-files drift prompt) MUST skip the spinner when it may prompt — gate with mayPrompt(nf) (stdin AND stdout isTTY, and neither --force nor --dry-run), mirroring the init handler. The earlier \"install/uninstall are spinner-safe because runX outputs only via the renderer after the run\" claim is WRONG: it ignored the interactive prompt flow (writeBanner + deps.ask/readLine) which writes DURING the run.", + "status": "active", + "topics": [ + "cli-presentation" + ], + "triggers": [ + "t-glob-8173701c" + ] + }, + "cli-presentation-all-interactive-install-uninstall-refres": { + "createdAt": "2026-07-09", + "evidence": [ + "src/install/core/prompts.ts", + "src/install/prompts/prompt-io.ts" + ], + "rule": "All interactive install/uninstall/refresh prompts MUST go through prompt-io.readLine (or delegate to it), never a bespoke readline.createInterface. A raw readline without a 'close' handler HANGS on EOF/Ctrl-D (the question callback never fires), and without an 'error' handler CRASHES the process on a stream error (broken pipe, SSH drop). readLine collapses both to '' (decline) — the uniform EOF contract. This is why confirm() (src/install/core/prompts.ts) was refactored to delegate to readLine. When adding a new prompt, reuse readLine and treat '' as the safe-decline default.", + "status": "active", + "topics": [ + "cli-presentation" + ], + "triggers": [ + "t-glob-7ba39ec1", + "t-glob-dd0538cf" + ] + }, + "cli-presentation-when-auditing-spinner-vs-prompt": { + "createdAt": "2026-07-09", + "evidence": [ + "src/cli/command-handlers.ts", + "src/install/refresh/run-refresh.ts", + "src/install/refresh/refresh-prompt.ts" + ], + "rule": "When auditing spinner-vs-prompt collisions, check EVERY command handler in src/cli/command-handlers.ts that holds a ui.spinner(), not just the one reported. refresh is a THIRD collision site (besides install/uninstall): runRefresh → runConsentPrompt (run-refresh.ts) → readLine shows a [y/N/per-pack] consent prompt for drifted packs, and it was ungated. generate/import/convert are genuinely prompt-free (verified) so their spinners are safe. Gate refresh with mayPrompt(nf) too. Rule: a handler is spinner-safe ONLY if its runX has zero reachable readLine/confirm/clack prompt; trace the full call graph before trusting an ungated spinner.", + "status": "active", + "topics": [ + "cli-presentation" + ], + "triggers": [ + "t-glob-8173701c" + ] + }, "coverage-analysis-agentsmesh-coverage-thresholds-in-vitest": { "createdAt": "2026-06-28", "evidence": [], @@ -8238,6 +8286,10 @@ "kind": "file_glob", "pattern": "src/lessons/keyword-match.ts" }, + "t-glob-7ba39ec1": { + "kind": "file_glob", + "pattern": "src/install/prompts/prompt-io.ts" + }, "t-glob-7d39ded4": { "kind": "file_glob", "pattern": "src/config/core/conversions.ts" @@ -8570,6 +8622,10 @@ "kind": "file_glob", "pattern": "tests/fixtures/lessons/recurrence/**" }, + "t-glob-dd0538cf": { + "kind": "file_glob", + "pattern": "src/install/core/prompts.ts" + }, "t-glob-dd0e31f0": { "kind": "file_glob", "pattern": "src/targets/copilot/**" diff --git a/.changeset/fix-interactive-install-spinner-hang.md b/.changeset/fix-interactive-install-spinner-hang.md new file mode 100644 index 00000000..18cf7b33 --- /dev/null +++ b/.changeset/fix-interactive-install-spinner-hang.md @@ -0,0 +1,7 @@ +--- +'agentsmesh': patch +--- + +Fix interactive prompts hanging behind the status spinner. `install`, `uninstall`, and `refresh` held the spinner across the whole run, and its redraw timer overwrote the interactive prompts underneath (skill-pack select, broken-link, invalid-resource confirm, uninstall drift, and the refresh consent prompt) — leaving the command waiting on invisible input (refresh silently timed out after 5 minutes and skipped the pack). The spinner now yields the terminal to the prompt flow whenever a run may prompt (real TTY, no `--force`/`--dry-run`), mirroring `init`. + +Also hardened the prompt primitives: `confirm()` now reuses the shared `readLine` helper so a closed or erroring stdin declines gracefully instead of hanging (Ctrl-D) or crashing; `readLine` resolves to empty (decline) on a stream `error` instead of throwing. And `install --force --dry-run` now previews the same resource set a real `--force` install would write (invalid resources were incorrectly dropped from the preview). diff --git a/src/cli/command-handlers.ts b/src/cli/command-handlers.ts index cd0f2e45..0e1c9454 100644 --- a/src/cli/command-handlers.ts +++ b/src/cli/command-handlers.ts @@ -52,6 +52,23 @@ function narrowFlags(flags: CliFlags): Record { return out; } +/** + * True when an install/uninstall/refresh run may show an interactive prompt: a + * real TTY on both streams and neither --force nor --dry-run (both bypass every + * prompt). Such runs must NOT hold a spinner — its redraw timer overwrites the + * prompt line, hiding the question and hanging the command on stdin. `--json` + * maps to --force upstream for install/uninstall, and refresh reads json→force + * internally, so json runs never reach a prompt either way. + */ +function mayPrompt(nf: Record): boolean { + return ( + process.stdin.isTTY === true && + process.stdout.isTTY === true && + nf.force !== true && + nf['dry-run'] !== true + ); +} + export const cmdHandlers: Record = { generate: async (flags, _args) => { void _args; @@ -161,10 +178,15 @@ export const cmdHandlers: Record = { const nf = narrowFlags(flags); if (nf.json === true) nf.force = true; ui.intro('agentsmesh install'); - const sp = ui.spinner(); - sp.start('Installing…'); + // An animated spinner and an interactive readline prompt cannot share a TTY: + // the spinner's redraw timer overwrites the prompt line, so the confirmation + // question is invisible and the install hangs waiting for stdin. When the run + // may prompt (real TTY, no --force/--dry-run), skip the spinner and let the + // prompt flow own the terminal. + const sp = mayPrompt(nf) ? null : ui.spinner(); + sp?.start('Installing…'); const result = await runInstall(nf, args, process.cwd()); - sp.stop('Install complete'); + sp?.stop('Install complete'); handleResult('install', result, nf, () => renderInstall(result)); ui.outro('Done'); }, @@ -172,20 +194,24 @@ export const cmdHandlers: Record = { const nf = narrowFlags(flags); if (nf.json === true) nf.force = true; ui.intro('agentsmesh uninstall'); - const sp = ui.spinner(); - sp.start('Removing…'); + // See install: the spinner must yield the TTY to the interactive prompt flow. + const sp = mayPrompt(nf) ? null : ui.spinner(); + sp?.start('Removing…'); const result = await runUninstall(nf, args, process.cwd()); - sp.stop('Uninstall complete'); + sp?.stop('Uninstall complete'); handleResult('uninstall', result, nf, () => renderUninstall(result)); ui.outro('Done'); }, refresh: async (flags, args) => { const nf = narrowFlags(flags); ui.intro('agentsmesh refresh'); - const sp = ui.spinner(); - sp.start('Refreshing…'); + // See install: refresh reaches an interactive consent prompt (packs with + // local edits, no --force/--dry-run) via runConsentPrompt → readLine. The + // spinner must yield the TTY to that prompt or it hangs/times-out unseen. + const sp = mayPrompt(nf) ? null : ui.spinner(); + sp?.start('Refreshing…'); const result = await runRefresh(nf, args, process.cwd()); - sp.stop('Refresh complete'); + sp?.stop('Refresh complete'); handleResult('refresh', result, nf, () => renderRefresh(result)); ui.outro('Done'); }, diff --git a/src/install/core/pool-resolution.ts b/src/install/core/pool-resolution.ts index ba778716..77360925 100644 --- a/src/install/core/pool-resolution.ts +++ b/src/install/core/pool-resolution.ts @@ -39,20 +39,19 @@ export async function resolveSkillPool( tty: boolean, ): Promise { const skillCandidates = narrowed.skills.map((s) => validateSkill(s)); - let pool = skillCandidates.filter((c) => c.ok).map((c) => c.skill); + const pool = skillCandidates.filter((c) => c.ok).map((c) => c.skill); const invalid = skillCandidates.filter((c) => !c.ok); - if (!force && !dryRun && tty) { + // `force` includes invalid resources; a dry-run must preview the SAME set a + // real run would install, so force wins over dryRun here. dryRun only gates + // writes downstream (run-install-execute early-returns), never selection. + if (force) return skillCandidates.map((c) => c.skill); + if (!dryRun && tty) { for (const inv of invalid) { const ok = await confirm( `Include invalid skill "${inv.skill.name}" anyway? (${inv.reason}). You can fix it later.`, ); if (ok) pool.push(inv.skill); } - } else if (force) { - pool = skillCandidates.map((c) => c.skill); - } - if (dryRun) { - pool = skillCandidates.filter((c) => c.ok).map((c) => c.skill); } return pool; } @@ -64,20 +63,16 @@ export async function resolveRulePool( tty: boolean, ): Promise { const candidates = narrowed.rules.map((r) => validateRule(r)); - let pool = candidates.filter((c) => c.ok).map((c) => c.rule); + const pool = candidates.filter((c) => c.ok).map((c) => c.rule); const invalid = candidates.filter((c) => !c.ok); - if (!force && !dryRun && tty) { + if (force) return candidates.map((c) => c.rule); + if (!dryRun && tty) { for (const inv of invalid) { const ok = await confirm( `Include invalid rule "${ruleSlug(inv.rule)}" anyway? (${inv.reason}). You can fix it later.`, ); if (ok) pool.push(inv.rule); } - } else if (force) { - pool = candidates.map((c) => c.rule); - } - if (dryRun) { - pool = candidates.filter((c) => c.ok).map((c) => c.rule); } return pool; } @@ -89,20 +84,16 @@ export async function resolveCommandPool( tty: boolean, ): Promise { const candidates = narrowed.commands.map((c) => validateCommand(c)); - let pool = candidates.filter((c) => c.ok).map((c) => c.command); + const pool = candidates.filter((c) => c.ok).map((c) => c.command); const invalid = candidates.filter((c) => !c.ok); - if (!force && !dryRun && tty) { + if (force) return candidates.map((c) => c.command); + if (!dryRun && tty) { for (const inv of invalid) { const ok = await confirm( `Include invalid command "${inv.command.name}" anyway? (${inv.reason}). You can fix it later.`, ); if (ok) pool.push(inv.command); } - } else if (force) { - pool = candidates.map((c) => c.command); - } - if (dryRun) { - pool = candidates.filter((c) => c.ok).map((c) => c.command); } return pool; } @@ -114,20 +105,16 @@ export async function resolveAgentPool( tty: boolean, ): Promise { const candidates = narrowed.agents.map((a) => validateAgent(a)); - let pool = candidates.filter((c) => c.ok).map((c) => c.agent); + const pool = candidates.filter((c) => c.ok).map((c) => c.agent); const invalid = candidates.filter((c) => !c.ok); - if (!force && !dryRun && tty) { + if (force) return candidates.map((c) => c.agent); + if (!dryRun && tty) { for (const inv of invalid) { const ok = await confirm( `Include invalid agent "${inv.agent.name}" anyway? (${inv.reason}). You can fix it later.`, ); if (ok) pool.push(inv.agent); } - } else if (force) { - pool = candidates.map((c) => c.agent); - } - if (dryRun) { - pool = candidates.filter((c) => c.ok).map((c) => c.agent); } return pool; } diff --git a/src/install/core/prompts.ts b/src/install/core/prompts.ts index 958bad32..754867d7 100644 --- a/src/install/core/prompts.ts +++ b/src/install/core/prompts.ts @@ -1,17 +1,16 @@ /** * Interactive install prompts (TTY only). + * + * Delegates to the shared `readLine` primitive so it inherits its EOF/error + * safety: a closed or erroring stdin resolves to '' → decline, instead of + * hanging on an unresolved promise (the old inline readline had no 'close'/ + * 'error' handler and hung on Ctrl-D / crashed on stream errors). */ -import * as readline from 'node:readline'; +import { readLine } from '../prompts/prompt-io.js'; export async function confirm(message: string): Promise { if (!process.stdin.isTTY) return false; - return new Promise((resolve) => { - const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); - rl.question(`${message} (y/n) `, (answer) => { - rl.close(); - const a = answer.trim().toLowerCase(); - resolve(a === 'y' || a === 'yes'); - }); - }); + const answer = (await readLine(`${message} (y/n) `)).trim().toLowerCase(); + return answer === 'y' || answer === 'yes'; } diff --git a/src/install/prompts/prompt-io.ts b/src/install/prompts/prompt-io.ts index 7bddcabc..d6c969ec 100644 --- a/src/install/prompts/prompt-io.ts +++ b/src/install/prompts/prompt-io.ts @@ -31,6 +31,16 @@ export function readLine(prompt: string, options?: PromptIOOptions): Promise { if (!answered) resolve(''); }); + // A stream 'error' (broken pipe, terminal disconnect, SSH drop) would + // otherwise throw an unhandled event and crash the process. Collapse it to + // the same EOF contract: resolve '' so callers uniformly treat it as "no + // input" (decline) instead of hanging or crashing. + rl.on('error', () => { + if (!answered) { + answered = true; + resolve(''); + } + }); rl.question(prompt, (answer) => { answered = true; rl.close(); diff --git a/tests/unit/cli/command-handlers-refresh-spinner.test.ts b/tests/unit/cli/command-handlers-refresh-spinner.test.ts new file mode 100644 index 00000000..6e4ed8f8 --- /dev/null +++ b/tests/unit/cli/command-handlers-refresh-spinner.test.ts @@ -0,0 +1,70 @@ +/** + * Refresh handler must skip the spinner when the run may prompt, so the + * refresh drift consent prompt (run-refresh.ts → runConsentPrompt → readLine) + * is not clobbered by the 'Refreshing…' spinner's redraw timer. Mirrors the + * install/uninstall spinner-gating tests. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { cmdHandlers } from '../../../src/cli/command-handlers.js'; +import { runRefresh } from '../../../src/cli/commands/refresh.js'; +import { handleResult } from '../../../src/cli/json-handler.js'; +import { ui } from '../../../src/cli/ui/ui.js'; + +vi.mock('../../../src/cli/commands/refresh.js', () => ({ runRefresh: vi.fn() })); +vi.mock('../../../src/cli/renderers/refresh.js', () => ({ renderRefresh: vi.fn() })); +vi.mock('../../../src/cli/json-handler.js', () => ({ handleResult: vi.fn() })); + +describe('cmdHandlers.refresh — spinner gating', () => { + const refreshResult = { + exitCode: 0, + data: { scope: 'project' as const, refreshed: [], unchanged: [], skipped: [], failed: [] }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(runRefresh).mockResolvedValue(refreshResult); + vi.mocked(handleResult).mockImplementation((_command, _result, flags, render) => { + if (flags.json !== true) render(); + }); + }); + + function withTty(fn: () => Promise): Promise { + const inDesc = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + const outDesc = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + return fn().finally(() => { + if (inDesc) Object.defineProperty(process.stdin, 'isTTY', inDesc); + if (outDesc) Object.defineProperty(process.stdout, 'isTTY', outDesc); + }); + } + + it('skips the spinner on an interactive TTY so the consent prompt is not clobbered', async () => { + const start = vi.fn(); + const spinnerSpy = vi + .spyOn(ui, 'spinner') + .mockReturnValue({ start, stop: vi.fn(), message: vi.fn() }); + try { + await withTty(() => cmdHandlers.refresh({}, [])); + expect(ui.spinner).not.toHaveBeenCalled(); + expect(start).not.toHaveBeenCalled(); + expect(runRefresh).toHaveBeenCalledWith({}, [], process.cwd()); + } finally { + spinnerSpy.mockRestore(); + } + }); + + it('runs the spinner when non-interactive (--force bypasses the consent prompt)', async () => { + const start = vi.fn(); + const spinnerSpy = vi + .spyOn(ui, 'spinner') + .mockReturnValue({ start, stop: vi.fn(), message: vi.fn() }); + try { + await withTty(() => cmdHandlers.refresh({ force: true }, [])); + expect(start).toHaveBeenCalledWith('Refreshing…'); + } finally { + spinnerSpy.mockRestore(); + } + }); +}); diff --git a/tests/unit/cli/command-handlers-uninstall-installs-mcp.test.ts b/tests/unit/cli/command-handlers-uninstall-installs-mcp.test.ts index 4e21aa7b..4d33fe2d 100644 --- a/tests/unit/cli/command-handlers-uninstall-installs-mcp.test.ts +++ b/tests/unit/cli/command-handlers-uninstall-installs-mcp.test.ts @@ -15,6 +15,7 @@ import { renderInstall } from '../../../src/cli/renderers/install.js'; import { renderUninstall } from '../../../src/cli/renderers/uninstall.js'; import { renderInstalls } from '../../../src/cli/renderers/installs.js'; import { handleResult } from '../../../src/cli/json-handler.js'; +import { ui } from '../../../src/cli/ui/ui.js'; vi.mock('../../../src/cli/commands/install.js', () => ({ runInstall: vi.fn() })); vi.mock('../../../src/cli/commands/uninstall.js', () => ({ runUninstall: vi.fn() })); @@ -91,6 +92,45 @@ describe('cmdHandlers — install/uninstall/installs/mcp', () => { expect(renderUninstall).toHaveBeenCalled(); }); + it('skips the uninstall spinner on an interactive TTY so the drift prompt is not clobbered', async () => { + const start = vi.fn(); + const spinnerSpy = vi + .spyOn(ui, 'spinner') + .mockReturnValue({ start, stop: vi.fn(), message: vi.fn() }); + const inDesc = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + const outDesc = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + try { + await cmdHandlers.uninstall({}, ['pack-name']); + expect(ui.spinner).not.toHaveBeenCalled(); + expect(start).not.toHaveBeenCalled(); + } finally { + if (inDesc) Object.defineProperty(process.stdin, 'isTTY', inDesc); + if (outDesc) Object.defineProperty(process.stdout, 'isTTY', outDesc); + spinnerSpy.mockRestore(); + } + }); + + it('runs the uninstall spinner when non-interactive (--force bypasses the prompt)', async () => { + const start = vi.fn(); + const spinnerSpy = vi + .spyOn(ui, 'spinner') + .mockReturnValue({ start, stop: vi.fn(), message: vi.fn() }); + const inDesc = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + const outDesc = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + try { + await cmdHandlers.uninstall({ force: true }, ['pack-name']); + expect(start).toHaveBeenCalledWith('Removing…'); + } finally { + if (inDesc) Object.defineProperty(process.stdin, 'isTTY', inDesc); + if (outDesc) Object.defineProperty(process.stdout, 'isTTY', outDesc); + spinnerSpy.mockRestore(); + } + }); + it('installs handler routes through handleResult and renders the list', async () => { await cmdHandlers.installs({}, ['list']); expect(runInstalls).toHaveBeenCalledWith({}, ['list'], process.cwd()); diff --git a/tests/unit/cli/command-handlers-watch-install.test.ts b/tests/unit/cli/command-handlers-watch-install.test.ts index 5af3651f..d7d321a2 100644 --- a/tests/unit/cli/command-handlers-watch-install.test.ts +++ b/tests/unit/cli/command-handlers-watch-install.test.ts @@ -9,6 +9,7 @@ import { renderPlugin } from '../../../src/cli/renderers/plugin.js'; import { renderTarget } from '../../../src/cli/renderers/target.js'; import { handleResult } from '../../../src/cli/json-handler.js'; import { emitJson } from '../../../src/cli/json-output.js'; +import { ui } from '../../../src/cli/ui/ui.js'; vi.mock('../../../src/cli/commands/watch.js', () => ({ runWatch: vi.fn() })); vi.mock('../../../src/cli/commands/install.js', () => ({ runInstall: vi.fn() })); @@ -100,6 +101,65 @@ describe('cmdHandlers watch/install/plugin/target', () => { expect(renderInstall).toHaveBeenCalledWith(installResult); }); + it('skips the install spinner on an interactive TTY so prompts are not clobbered', async () => { + const start = vi.fn(); + const spinnerSpy = vi + .spyOn(ui, 'spinner') + .mockReturnValue({ start, stop: vi.fn(), message: vi.fn() }); + const inDesc = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + const outDesc = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + try { + await cmdHandlers.install({}, ['pack']); + expect(ui.spinner).not.toHaveBeenCalled(); + expect(start).not.toHaveBeenCalled(); + expect(runInstall).toHaveBeenCalledWith({}, ['pack'], process.cwd()); + } finally { + if (inDesc) Object.defineProperty(process.stdin, 'isTTY', inDesc); + if (outDesc) Object.defineProperty(process.stdout, 'isTTY', outDesc); + spinnerSpy.mockRestore(); + } + }); + + it('runs the install spinner when non-interactive (--force bypasses prompts)', async () => { + const start = vi.fn(); + const spinnerSpy = vi + .spyOn(ui, 'spinner') + .mockReturnValue({ start, stop: vi.fn(), message: vi.fn() }); + const inDesc = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + const outDesc = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + try { + await cmdHandlers.install({ force: true }, ['pack']); + expect(start).toHaveBeenCalledWith('Installing…'); + } finally { + if (inDesc) Object.defineProperty(process.stdin, 'isTTY', inDesc); + if (outDesc) Object.defineProperty(process.stdout, 'isTTY', outDesc); + spinnerSpy.mockRestore(); + } + }); + + it('runs the install spinner for --dry-run on a TTY (dry-run bypasses prompts)', async () => { + const start = vi.fn(); + const spinnerSpy = vi + .spyOn(ui, 'spinner') + .mockReturnValue({ start, stop: vi.fn(), message: vi.fn() }); + const inDesc = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + const outDesc = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + try { + await cmdHandlers.install({ 'dry-run': true }, ['pack']); + expect(start).toHaveBeenCalledWith('Installing…'); + } finally { + if (inDesc) Object.defineProperty(process.stdin, 'isTTY', inDesc); + if (outDesc) Object.defineProperty(process.stdout, 'isTTY', outDesc); + spinnerSpy.mockRestore(); + } + }); + it('delegates plugin and target commands through structured results', async () => { await cmdHandlers.plugin({}, ['list']); await cmdHandlers.target({}, ['scaffold', 'acme']); diff --git a/tests/unit/install/pool-resolution-branches.test.ts b/tests/unit/install/pool-resolution-branches.test.ts index 767e05d7..bfb7af7e 100644 --- a/tests/unit/install/pool-resolution-branches.test.ts +++ b/tests/unit/install/pool-resolution-branches.test.ts @@ -125,6 +125,20 @@ describe('resolveSkillPool — branches', () => { expect(mockConfirm).not.toHaveBeenCalled(); }); + it('force overrides dry-run: --force --dry-run preview includes invalid candidates', async () => { + mockValidateSkill + .mockReturnValueOnce({ ok: true, skill: valid }) + .mockReturnValueOnce({ ok: false, skill: broken, reason: 'bad' }); + const pool = await resolveSkillPool( + makeCanonical({ skills: [valid, broken] }), + true, + true, + false, + ); + expect(pool.map((s) => s.name).sort()).toEqual(['broken', 'valid']); + expect(mockConfirm).not.toHaveBeenCalled(); + }); + it('prompts in interactive mode and includes invalid skill on yes', async () => { mockValidateSkill .mockReturnValueOnce({ ok: true, skill: valid }) diff --git a/tests/unit/install/prompts-confirm.test.ts b/tests/unit/install/prompts-confirm.test.ts new file mode 100644 index 00000000..da9776ab --- /dev/null +++ b/tests/unit/install/prompts-confirm.test.ts @@ -0,0 +1,47 @@ +/** + * confirm() must delegate to the shared readLine primitive so it inherits the + * safe EOF/error handling (EOF → decline, no hang, no crash). It declines + * immediately on a non-TTY without touching stdin. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const mockReadLine = vi.hoisted(() => vi.fn()); +vi.mock('../../../src/install/prompts/prompt-io.js', () => ({ readLine: mockReadLine })); + +import { confirm } from '../../../src/install/core/prompts.js'; + +const origIsTty = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + +function setTty(value: boolean): void { + Object.defineProperty(process.stdin, 'isTTY', { value, configurable: true }); +} + +describe('confirm', () => { + afterEach(() => { + vi.clearAllMocks(); + if (origIsTty) Object.defineProperty(process.stdin, 'isTTY', origIsTty); + }); + + it('declines without prompting on a non-TTY stdin', async () => { + setTty(false); + expect(await confirm('overwrite?')).toBe(false); + expect(mockReadLine).not.toHaveBeenCalled(); + }); + + it('returns true for y / yes (case-insensitive)', async () => { + setTty(true); + mockReadLine.mockResolvedValueOnce('y'); + expect(await confirm('overwrite?')).toBe(true); + mockReadLine.mockResolvedValueOnce(' YES '); + expect(await confirm('overwrite?')).toBe(true); + }); + + it("declines for anything else, including EOF ('') — never hangs", async () => { + setTty(true); + mockReadLine.mockResolvedValueOnce('n'); + expect(await confirm('overwrite?')).toBe(false); + mockReadLine.mockResolvedValueOnce(''); + expect(await confirm('overwrite?')).toBe(false); + }); +}); diff --git a/tests/unit/install/prompts/prompt-io.test.ts b/tests/unit/install/prompts/prompt-io.test.ts index 1f503ffd..21613210 100644 --- a/tests/unit/install/prompts/prompt-io.test.ts +++ b/tests/unit/install/prompts/prompt-io.test.ts @@ -81,4 +81,20 @@ describe('readLine', () => { const answer = await readLine('input', { input, output }); expect(answer).toBe(' spaced '); }); + + it("resolves to '' (does not crash or hang) when the input stream errors mid-prompt", async () => { + const input = new PassThrough(); + const { stream: output } = makeCapture(); + + const racePromise = Promise.race([ + readLine('continue?', { input, output }), + new Promise<'__timeout__'>((resolve) => { + setTimeout(() => resolve('__timeout__'), 1000); + }), + ]); + input.emit('error', new Error('EPIPE: broken pipe')); + + const result = await racePromise; + expect(result).toBe(''); + }); });