From 9052c5a78331b29569befeda1e85eb679b49a05b Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sat, 20 Jun 2026 19:49:45 -0300 Subject: [PATCH] fix(cli): non-interactive-safe prompts + singular `context` alias (#4439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): non-interactive-safe prompts + singular `context` alias Two CLI-polish follow-ups (the third — an async-warn refactor for #2807 — is moot: #4373 replaced the Gemini remote-URL drop+warn with fileData pass-through, so no async warn remains). 1) createPrompt (io.mjs) — the shared interactive helper used rl.question with no EOF guard, so a non-interactive stdin (pipe, CI, `< /dev/null`) left the await pending and Node warned about an 'unsettled top-level await' while the command hung. ask/askSecret now resolve on the readline `close` event (fired on EOF) with the default / empty string. A genuinely piped line still arrives via the question callback first, so `echo value | omniroute …` keeps working — only the no-input EOF case falls back. This fixes every command that prompts, centrally (mirrors the contexts `confirm()` fix from #4397). 2) `contexts` gains a singular `context` alias — the connect output and older docs said `omniroute context current`; the alias keeps that muscle-memory working. Tests: cli-io.test.ts (ask default/empty + askSecret resolve on EOF, no hang) and a cli-contexts.test.ts case asserting the `context` alias is registered. 11/11. * test(cli): mock .alias() in the fake program for the contexts subcommand test The existing 'registers a current subcommand' test uses a minimal fake commander program; registerContexts now calls .alias("context"), so the fake needs an alias() stub (returns this) or the chain throws. Add it. (Self-introduced by the alias in this branch; my 4 new tests already pass in CI.) --- bin/cli/commands/contexts.mjs | 1 + bin/cli/io.mjs | 31 +++++++++++++++++++++---- tests/unit/cli-contexts.test.ts | 37 ++++++++++++++++++++++++++++++ tests/unit/cli-io.test.ts | 33 ++++++++++++++++++++++++++ tests/unit/cli-remote-mode.test.ts | 3 +++ 5 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 tests/unit/cli-io.test.ts diff --git a/bin/cli/commands/contexts.mjs b/bin/cli/commands/contexts.mjs index 2057f051b7..e40b9ac2ee 100644 --- a/bin/cli/commands/contexts.mjs +++ b/bin/cli/commands/contexts.mjs @@ -34,6 +34,7 @@ function maskKey(k) { export function registerContexts(program) { const ctx = program .command("contexts") + .alias("context") // singular alias — docs/connect output historically said `context current` .description(t("config.contexts.description") || "Manage server contexts/profiles"); ctx diff --git a/bin/cli/io.mjs b/bin/cli/io.mjs index 97b419dc77..b72f7c4310 100644 --- a/bin/cli/io.mjs +++ b/bin/cli/io.mjs @@ -6,20 +6,43 @@ export function createPrompt() { output: process.stdout, }); + // Non-interactive stdin (pipe, CI, EOF via `< /dev/null`) cannot answer an + // interactive prompt. Without a guard, `rl.question` never fires its callback — + // the await stays pending and Node warns about an "unsettled top-level await" at + // exit. Resolving on the readline `close` event (which fires on stdin EOF) + // returns the default/empty instead of hanging. A genuinely piped line still + // arrives via the question callback first, so `echo value | omniroute …` keeps + // working — only the no-input EOF case falls back. function ask(question, defaultValue = "") { const suffix = defaultValue ? ` (${defaultValue})` : ""; return new Promise((resolve) => { + let settled = false; + const done = (v) => { + if (!settled) { + settled = true; + resolve(v); + } + }; + rl.once("close", () => done(defaultValue)); rl.question(`${question}${suffix}: `, (answer) => { const trimmed = answer.trim(); - resolve(trimmed || defaultValue); + done(trimmed || defaultValue); }); }); } function askSecret(question) { return new Promise((resolve) => { - let prompted = false; + let settled = false; const saved = rl._writeToOutput.bind(rl); + const done = (v) => { + if (!settled) { + settled = true; + rl._writeToOutput = saved; + resolve(v); + } + }; + let prompted = false; rl._writeToOutput = function (str) { if (!prompted) { rl.output.write(str); @@ -29,9 +52,9 @@ export function createPrompt() { // Suppress character echo; allow only newlines through if (str === "\r\n" || str === "\n" || str === "\r") rl.output.write("\n"); }; + rl.once("close", () => done("")); // non-interactive EOF → empty secret, no hang rl.question(`${question}: `, (answer) => { - rl._writeToOutput = saved; - resolve(answer.trim()); + done(answer.trim()); }); }); } diff --git a/tests/unit/cli-contexts.test.ts b/tests/unit/cli-contexts.test.ts index c596e394c1..21644a15aa 100644 --- a/tests/unit/cli-contexts.test.ts +++ b/tests/unit/cli-contexts.test.ts @@ -92,3 +92,40 @@ test("confirm() declines cleanly on non-interactive stdin (no hung await)", asyn else delete (process.stdin as { isTTY?: boolean }).isTTY; } }); + +test("registerContexts registers the singular `context` alias", async () => { + // The connect output and older docs say `omniroute context current` (singular); + // the command is `contexts`. An alias keeps the singular muscle-memory working. + const { registerContexts } = await import("../../bin/cli/commands/contexts.mjs"); + let aliasName: string | undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fakeCtx: any = { + command() { + return this; + }, + alias(a: string) { + aliasName = a; + return this; + }, + description() { + return this; + }, + requiredOption() { + return this; + }, + option() { + return this; + }, + action() { + return this; + }, + }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fakeProgram: any = { + command() { + return fakeCtx; + }, + }; + registerContexts(fakeProgram); + assert.equal(aliasName, "context"); +}); diff --git a/tests/unit/cli-io.test.ts b/tests/unit/cli-io.test.ts new file mode 100644 index 0000000000..7f7c5daba0 --- /dev/null +++ b/tests/unit/cli-io.test.ts @@ -0,0 +1,33 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +// Regression: the shared interactive prompt helper (createPrompt) used rl.question +// without an EOF guard, so a non-interactive stdin (pipe, CI, `< /dev/null`) left +// the promise pending — Node then warned about an "unsettled top-level await" at +// exit and the command hung. ask/askSecret now resolve on the readline `close` +// event (fired on EOF) with the default / empty string instead of hanging. close() +// here simulates that EOF/non-interactive close. + +test("createPrompt.ask resolves the default on EOF (non-interactive, no hang)", async () => { + const { createPrompt } = await import("../../bin/cli/io.mjs"); + const p = createPrompt(); + const pending = p.ask("Name", "fallback"); + p.close(); + assert.equal(await pending, "fallback"); +}); + +test("createPrompt.ask resolves empty when there is no default on EOF", async () => { + const { createPrompt } = await import("../../bin/cli/io.mjs"); + const p = createPrompt(); + const pending = p.ask("Name"); + p.close(); + assert.equal(await pending, ""); +}); + +test("createPrompt.askSecret resolves empty on EOF (no hang)", async () => { + const { createPrompt } = await import("../../bin/cli/io.mjs"); + const p = createPrompt(); + const pending = p.askSecret("Secret"); + p.close(); + assert.equal(await pending, ""); +}); diff --git a/tests/unit/cli-remote-mode.test.ts b/tests/unit/cli-remote-mode.test.ts index 6d35ca862b..61546d8c94 100644 --- a/tests/unit/cli-remote-mode.test.ts +++ b/tests/unit/cli-remote-mode.test.ts @@ -220,6 +220,9 @@ test("commands/contexts.mjs registers a `current` subcommand", async () => { sub.push(name.split(" ")[0]); return this; }, + alias() { + return this; + }, description() { return this; },