Skip to content

Commit 771edeb

Browse files
authored
fix(onboard): release stdin event-loop ref so the wizard exits after its final prompt (#2665)
## Summary `nemoclaw onboard` on Linux interactive TTY hangs after the post-onboard summary box — the wizard finishes, prints the dashboard URL, and then sits there with no shell prompt until the user reaches for Ctrl+C. The cause is `readline.createInterface({ input: process.stdin })` resuming stdin internally and leaving the underlying TTY handle ref'd to the event loop after `rl.close()`; once the wizard's last prompt resolves Node has no other refs but still cannot exit. CI and other non-TTY callers did not hit this because the previous cleanup pause+unref'd stdin only when `!process.stdin.isTTY`. This switches the shared `prompt()` cleanup to always pause+unref and adds a matching `process.stdin.ref()` at every prompt entry — the readline branch, the secret branch, `promptSecret()` itself, and the four raw-mode TUI selectors in `onboard.ts` — so a sticky `unref()` from a prior prompt cannot strand a follow-up read on a detached handle. The same TTY-guarded cleanup pattern in `policies.ts` and the missing pause/unref in `sandbox-config.ts:confirmYesNo` are aligned to the same shape. ## Related Issues Fixes #2518 ## Changes - `src/lib/credentials.ts`: drop the `if (!process.stdin.isTTY)` guard from `prompt()` cleanup so pause+unref runs on every code path; hoist `process.stdin.ref()` above the secret-vs-readline branch so both paths re-attach stdin before the next read; make `promptSecret()` self-contained — ref() at the top of the body and pause+unref in its own cleanup so a direct caller (or two sequential direct calls) is safe regardless of prior stdin state. - `src/lib/onboard.ts`: add `process.stdin.ref()` before the `setRawMode(true) + resume()` block in the messaging-channels selector and in the three identical arrow-key picker patterns (sandbox-prompt, model-prompt, policy-preset-prompt) so a sticky unref() from earlier `prompt()` cleanup does not leave the raw-mode read attached to a detached handle; add `unref()` to each cleanup so wizards that end on a TUI selector also exit naturally. - `src/lib/policies.ts`: drop the `if (!process.stdin.isTTY)` guards in both preset-selection callbacks (lines 446 and 770) and add ref() at the top of each prompt block. - `src/lib/sandbox-config.ts`: add the matching ref() + pause/unref pair to `confirmYesNo()`, which previously only ran `rl.close()` and would have leaked the same stdin ref had it ever been the wizard's last prompt. - `test/credentials.test.ts`: source-text assertions for the new contract — TTY guard is gone from `prompt()` cleanup, ref() precedes the silent/secret branch in `prompt()`, `promptSecret()` is self-contained, and the secret-path cleanup pauses+unrefs. - `test/policies.test.ts`: source-text assertions for the same contract on the two preset-selection prompts — TTY guard removed and a ref() precedes each `createInterface`. - `test/onboard.test.ts`: source-text assertion that all four raw-mode TUI sites (one `input.ref()` alias + three `process.stdin.ref()` calls) ref before `setRawMode(true)` and unref after `setRawMode(false)`, so any one of them stays safe to be the wizard's last prompt. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [x] AI-assisted — tool: Claude Code, Codex --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reliability of sequential interactive prompts (credentials, onboarding, policies, and configuration flows) so prompts no longer hang and the process exits cleanly after the final prompt. * **Tests** * Added and extended tests to validate interactive prompt lifecycle and ensure multiple sequential prompts complete without leaving lingering handles or blocking subsequent prompts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
1 parent ea7fbba commit 771edeb

7 files changed

Lines changed: 231 additions & 15 deletions

File tree

src/lib/credentials.ts

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,13 @@ export function promptSecret(question: string): Promise<string> {
390390
return new Promise((resolve, reject) => {
391391
const input = process.stdin;
392392
const output = process.stderr;
393+
// Re-attach stdin to the event loop. cleanup() below unrefs after the
394+
// read completes (so a wizard ending here exits naturally), and unref()
395+
// is sticky — without this the next direct promptSecret() call would
396+
// listen on a detached handle. Idempotent when prompt() already ref'd.
397+
if (typeof input.ref === "function") {
398+
input.ref();
399+
}
393400
let answer = "";
394401
let rawModeEnabled = false;
395402
let finished = false;
@@ -399,9 +406,15 @@ export function promptSecret(question: string): Promise<string> {
399406
if (rawModeEnabled && typeof input.setRawMode === "function") {
400407
input.setRawMode(false);
401408
}
409+
// pause+unref so a wizard ending on a secret prompt exits naturally.
410+
// The matching ref() in prompt() (and any direct caller) restores the
411+
// event-loop ref before the next read.
402412
if (typeof input.pause === "function") {
403413
input.pause();
404414
}
415+
if (typeof input.unref === "function") {
416+
input.unref();
417+
}
405418
}
406419

407420
function resolvePrompt(value: string) {
@@ -480,6 +493,14 @@ export function promptSecret(question: string): Promise<string> {
480493
*/
481494
export function prompt(question: string, opts: { secret?: boolean } = {}): Promise<string> {
482495
return new Promise((resolve, reject) => {
496+
// Re-attach stdin to the event loop before any prompt path. unref() in
497+
// cleanup (below, and in the secret path) is sticky — neither
498+
// readline.createInterface() nor the secret reader re-ref stdin on
499+
// their own, so a follow-up prompt of either kind would otherwise see
500+
// a detached handle and the process could exit before the user answers.
501+
if (typeof process.stdin.ref === "function") {
502+
process.stdin.ref();
503+
}
483504
const silent = opts.secret === true && process.stdin.isTTY && process.stderr.isTTY;
484505
if (silent) {
485506
promptSecret(question)
@@ -499,13 +520,15 @@ export function prompt(question: string, opts: { secret?: boolean } = {}): Promi
499520

500521
function cleanup() {
501522
rl.close();
502-
if (!process.stdin.isTTY) {
503-
if (typeof process.stdin.pause === "function") {
504-
process.stdin.pause();
505-
}
506-
if (typeof process.stdin.unref === "function") {
507-
process.stdin.unref();
508-
}
523+
// pause+unref so the process exits naturally after the last prompt
524+
// resolves. The matching ref() above keeps subsequent prompts working;
525+
// unref()-ing a TTY ReadStream only releases the event-loop reference,
526+
// cooked/raw mode and any subsequent reads remain unaffected.
527+
if (typeof process.stdin.pause === "function") {
528+
process.stdin.pause();
529+
}
530+
if (typeof process.stdin.unref === "function") {
531+
process.stdin.unref();
509532
}
510533
}
511534

src/lib/onboard.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5316,6 +5316,14 @@ async function setupMessagingChannels(): Promise<string[]> {
53165316
if (rawModeEnabled && typeof input.setRawMode === "function") {
53175317
input.setRawMode(false);
53185318
}
5319+
// Symmetric with the ref() at the entry; lets the wizard exit
5320+
// naturally if this is the last prompt.
5321+
if (typeof input.pause === "function") {
5322+
input.pause();
5323+
}
5324+
if (typeof input.unref === "function") {
5325+
input.unref();
5326+
}
53195327
}
53205328

53215329
function finish(): void {
@@ -5353,6 +5361,12 @@ async function setupMessagingChannels(): Promise<string[]> {
53535361
}
53545362
}
53555363

5364+
// Re-attach stdin to the event loop. A prior prompt cleanup may have
5365+
// unref'd it (sticky), and resume() alone would leave the raw-mode read
5366+
// detached from the loop.
5367+
if (typeof input.ref === "function") {
5368+
input.ref();
5369+
}
53565370
input.setEncoding("utf8");
53575371
if (typeof input.resume === "function") {
53585372
input.resume();
@@ -5766,6 +5780,10 @@ async function selectPolicyTier(): Promise<string> {
57665780
lineCount = lines.length;
57675781
};
57685782

5783+
// Re-attach stdin to the event loop. A prior prompt cleanup may have
5784+
// unref'd it (sticky), and resume() alone would leave the raw-mode read
5785+
// detached from the loop.
5786+
if (typeof process.stdin.ref === "function") process.stdin.ref();
57695787
process.stdin.setRawMode(true);
57705788
process.stdin.resume();
57715789
process.stdin.setEncoding("utf8");
@@ -5774,6 +5792,9 @@ async function selectPolicyTier(): Promise<string> {
57745792
const cleanup = () => {
57755793
process.stdin.setRawMode(false);
57765794
process.stdin.pause();
5795+
// Symmetric with the ref() at the entry; lets the wizard exit
5796+
// naturally if this is the last prompt.
5797+
if (typeof process.stdin.unref === "function") process.stdin.unref();
57775798
process.stdin.removeListener("data", onData);
57785799
process.removeListener("SIGTERM", onSigterm);
57795800
};
@@ -5950,6 +5971,10 @@ async function selectTierPresetsAndAccess(
59505971
lineCount = lines.length;
59515972
};
59525973

5974+
// Re-attach stdin to the event loop. A prior prompt cleanup may have
5975+
// unref'd it (sticky), and resume() alone would leave the raw-mode read
5976+
// detached from the loop.
5977+
if (typeof process.stdin.ref === "function") process.stdin.ref();
59535978
process.stdin.setRawMode(true);
59545979
process.stdin.resume();
59555980
process.stdin.setEncoding("utf8");
@@ -5958,6 +5983,9 @@ async function selectTierPresetsAndAccess(
59585983
const cleanup = () => {
59595984
process.stdin.setRawMode(false);
59605985
process.stdin.pause();
5986+
// Symmetric with the ref() at the entry; lets the wizard exit
5987+
// naturally if this is the last prompt.
5988+
if (typeof process.stdin.unref === "function") process.stdin.unref();
59615989
process.stdin.removeListener("data", onData);
59625990
process.removeListener("SIGTERM", onSigterm);
59635991
};
@@ -6093,6 +6121,10 @@ async function presetsCheckboxSelector(
60936121
lineCount = lines.length;
60946122
};
60956123

6124+
// Re-attach stdin to the event loop. A prior prompt cleanup may have
6125+
// unref'd it (sticky), and resume() alone would leave the raw-mode read
6126+
// detached from the loop.
6127+
if (typeof process.stdin.ref === "function") process.stdin.ref();
60966128
process.stdin.setRawMode(true);
60976129
process.stdin.resume();
60986130
process.stdin.setEncoding("utf8");
@@ -6101,6 +6133,9 @@ async function presetsCheckboxSelector(
61016133
const cleanup = () => {
61026134
process.stdin.setRawMode(false);
61036135
process.stdin.pause();
6136+
// Symmetric with the ref() at the entry; lets the wizard exit
6137+
// naturally if this is the last prompt.
6138+
if (typeof process.stdin.unref === "function") process.stdin.unref();
61046139
process.stdin.removeListener("data", onData);
61056140
process.removeListener("SIGTERM", onSigterm);
61066141
};

src/lib/policies.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -440,13 +440,16 @@ function selectForRemoval(
440440
});
441441
process.stderr.write("\n");
442442
const question = " Choose preset to remove: ";
443+
// Re-attach stdin to the event loop — unref() on exit is sticky and
444+
// would otherwise leave a follow-up prompt waiting on a detached handle.
445+
if (typeof process.stdin.ref === "function") process.stdin.ref();
443446
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
444447
rl.question(question, (answer: string) => {
445448
rl.close();
446-
if (!process.stdin.isTTY) {
447-
if (typeof process.stdin.pause === "function") process.stdin.pause();
448-
if (typeof process.stdin.unref === "function") process.stdin.unref();
449-
}
449+
// pause+unref so the process exits naturally after the last prompt.
450+
// The matching ref() above keeps subsequent prompts working.
451+
if (typeof process.stdin.pause === "function") process.stdin.pause();
452+
if (typeof process.stdin.unref === "function") process.stdin.unref();
450453
const trimmed = answer.trim();
451454
if (!trimmed) {
452455
resolve(null);
@@ -764,13 +767,16 @@ function selectFromList(
764767
const defaultIdx = items.findIndex((item) => !applied.includes(item.name));
765768
const defaultNum = defaultIdx >= 0 ? defaultIdx + 1 : null;
766769
const question = defaultNum ? ` Choose preset [${defaultNum}]: ` : " Choose preset: ";
770+
// Re-attach stdin to the event loop — unref() on exit is sticky and
771+
// would otherwise leave a follow-up prompt waiting on a detached handle.
772+
if (typeof process.stdin.ref === "function") process.stdin.ref();
767773
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
768774
rl.question(question, (answer: string) => {
769775
rl.close();
770-
if (!process.stdin.isTTY) {
771-
if (typeof process.stdin.pause === "function") process.stdin.pause();
772-
if (typeof process.stdin.unref === "function") process.stdin.unref();
773-
}
776+
// pause+unref so the process exits naturally after the last prompt.
777+
// The matching ref() above keeps subsequent prompts working.
778+
if (typeof process.stdin.pause === "function") process.stdin.pause();
779+
if (typeof process.stdin.unref === "function") process.stdin.unref();
774780
const trimmed = answer.trim();
775781
const effectiveInput = trimmed || (defaultNum ? String(defaultNum) : "");
776782
if (!effectiveInput) {

src/lib/sandbox-config.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -752,9 +752,16 @@ function readStdin(): Promise<string> {
752752
*/
753753
function confirmYesNo(prompt: string): Promise<boolean> {
754754
return new Promise((resolve) => {
755+
// Re-attach stdin to the event loop — unref() on exit is sticky and
756+
// would otherwise leave a follow-up prompt waiting on a detached handle.
757+
if (typeof process.stdin.ref === "function") process.stdin.ref();
755758
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
756759
rl.question(prompt, (answer: string) => {
757760
rl.close();
761+
// pause+unref so the process exits naturally after the last prompt.
762+
// The matching ref() above keeps subsequent prompts working.
763+
if (typeof process.stdin.pause === "function") process.stdin.pause();
764+
if (typeof process.stdin.unref === "function") process.stdin.unref();
758765
resolve(/^y(es)?$/i.test(answer.trim()));
759766
});
760767
});

test/credentials.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,4 +504,79 @@ describe("prompt machinery (unchanged)", () => {
504504
expect(source).toContain('output.write("*")');
505505
expect(source).toContain('output.write("\\b \\b")');
506506
});
507+
508+
it("releases stdin after a prompt resolves so the event loop drains on a TTY", () => {
509+
const source = fs.readFileSync(
510+
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
511+
"utf-8",
512+
);
513+
514+
// The previous TTY-only guard kept the event loop pinned on interactive
515+
// runs — the wizard would not exit after its last prompt.
516+
expect(source).not.toMatch(/cleanup\s*\(\s*\)\s*\{\s*rl\.close\(\);\s*if\s*\(\s*!process\.stdin\.isTTY\s*\)/);
517+
expect(source).toMatch(
518+
/function cleanup\(\)\s*\{\s*rl\.close\(\);[\s\S]*?process\.stdin\.pause\(\)[\s\S]*?process\.stdin\.unref\(\)/,
519+
);
520+
});
521+
522+
it("re-refs stdin before each prompt so a follow-up prompt is not stranded by a sticky unref()", () => {
523+
const source = fs.readFileSync(
524+
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
525+
"utf-8",
526+
);
527+
528+
// unref() is sticky — readline.createInterface() will not re-ref by
529+
// itself, so a sequential prompt after the first cleanup would see a
530+
// detached stdin handle and the process could exit before the user
531+
// can answer. The matching ref() at the top of `prompt()` undoes that.
532+
expect(source).toMatch(
533+
/process\.stdin\.ref\(\)[\s\S]*?readline\.createInterface\(\{\s*input:\s*process\.stdin/,
534+
);
535+
});
536+
537+
it("re-refs stdin even on the secret-prompt branch so a follow-up secret read is not stranded", () => {
538+
const source = fs.readFileSync(
539+
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
540+
"utf-8",
541+
);
542+
543+
// The ref() must come before the silent/secret branch so that a
544+
// sequence of `prompt()` -> `prompt({ secret: true })` after a normal
545+
// prompt's sticky unref() still has a ref'd handle for promptSecret().
546+
const refIdx = source.search(/process\.stdin\.ref\(\);/);
547+
const silentIdx = source.search(/const silent = opts\.secret === true/);
548+
expect(refIdx).toBeGreaterThan(0);
549+
expect(silentIdx).toBeGreaterThan(0);
550+
expect(refIdx).toBeLessThan(silentIdx);
551+
});
552+
553+
it("releases stdin in promptSecret() cleanup so a wizard ending on a secret prompt exits naturally", () => {
554+
const source = fs.readFileSync(
555+
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
556+
"utf-8",
557+
);
558+
559+
// The secret reader uses raw mode + a `data` listener instead of
560+
// readline. Its cleanup must still pause+unref or the wizard hangs the
561+
// same way the readline path did.
562+
expect(source).toMatch(
563+
/promptSecret[\s\S]*?function cleanup\(\)\s*\{[\s\S]*?input\.pause\(\)[\s\S]*?input\.unref\(\)/,
564+
);
565+
});
566+
567+
it("re-refs stdin at the top of promptSecret() so a direct caller is self-contained", () => {
568+
const source = fs.readFileSync(
569+
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
570+
"utf-8",
571+
);
572+
573+
// promptSecret() is exported and used directly elsewhere. Because its
574+
// own cleanup unref()s stdin, two sequential direct calls (or any call
575+
// after a prior unref) would strand the second read without an entry
576+
// ref(). Assert ref() is the first effectful call inside the body.
577+
expect(source).toMatch(
578+
/export function promptSecret[\s\S]*?const input = process\.stdin;[\s\S]{0,400}?input\.ref\(\);[\s\S]*?function cleanup/,
579+
);
580+
});
581+
507582
});

test/onboard.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2648,6 +2648,44 @@ const { setupInference } = require(${onboardPath});
26482648
assert.equal(typeof streamSandboxCreate, "function");
26492649
});
26502650

2651+
it("re-refs stdin before each raw-mode prompt and unrefs in cleanup so sticky unref() does not strand later prompts", () => {
2652+
const source = fs.readFileSync(
2653+
path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"),
2654+
"utf-8",
2655+
);
2656+
2657+
// The shared `prompt()` cleanup unref()s stdin so the wizard exits
2658+
// naturally after its last readline prompt. unref() is sticky, so the
2659+
// raw-mode TUI selectors (messaging channels + the three arrow-key
2660+
// pickers) must explicitly ref() stdin before resume()/setRawMode(true)
2661+
// or they would otherwise listen on a detached handle.
2662+
const refMatches = source.match(
2663+
/process\.stdin\.ref\(\);[\s\S]{0,180}?process\.stdin\.setRawMode\(true\)/g,
2664+
);
2665+
assert.ok(
2666+
refMatches !== null && refMatches.length >= 3,
2667+
`expected at least 3 ref()-then-setRawMode(true) sites, found ${
2668+
refMatches ? refMatches.length : 0
2669+
}`,
2670+
);
2671+
2672+
// The messaging-channels picker uses an `input.ref()` alias on the
2673+
// captured handle. Same contract, different binding.
2674+
assert.match(source, /input\.ref\(\)[\s\S]{0,200}?input\.setRawMode\(true\)/);
2675+
2676+
// Each raw-mode cleanup must release stdin too, so a wizard that ends
2677+
// on a TUI selector exits cleanly.
2678+
const unrefMatches = source.match(
2679+
/setRawMode\(false\);[\s\S]{0,400}?(?:process\.stdin|input)\.unref\(\)/g,
2680+
);
2681+
assert.ok(
2682+
unrefMatches !== null && unrefMatches.length >= 4,
2683+
`expected at least 4 setRawMode(false)-then-unref() sites, found ${
2684+
unrefMatches ? unrefMatches.length : 0
2685+
}`,
2686+
);
2687+
});
2688+
26512689
it("migrates a legacy credentials.json into env so setupInference can register the provider", () => {
26522690
const repoRoot = path.join(import.meta.dirname, "..");
26532691
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-resume-cred-"));

test/policies.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,4 +1498,36 @@ setImmediate(() => {
14981498
expect(loads[0]).toMatch(/real\.yaml$/);
14991499
});
15001500
});
1501+
1502+
describe("interactive prompt cleanup", () => {
1503+
it("releases stdin after preset prompts so the event loop drains on a TTY", () => {
1504+
const source = fs.readFileSync(
1505+
path.join(REPO_ROOT, "src", "lib", "policies.ts"),
1506+
"utf-8",
1507+
);
1508+
// A TTY-only guard around pause/unref pins the event loop on
1509+
// interactive runs and stops the wizard from exiting after its last
1510+
// prompt resolves.
1511+
expect(source).not.toMatch(/rl\.close\(\);\s*if\s*\(\s*!process\.stdin\.isTTY\s*\)/);
1512+
// Both prompt callbacks must release stdin after `rl.close()`.
1513+
const cleanupMatches = source.match(
1514+
/rl\.close\(\);[\s\S]*?process\.stdin\.pause\(\)[\s\S]*?process\.stdin\.unref\(\)/g,
1515+
);
1516+
expect(cleanupMatches?.length ?? 0).toBeGreaterThanOrEqual(2);
1517+
});
1518+
1519+
it("re-refs stdin before each preset prompt so a follow-up prompt is not stranded by a sticky unref()", () => {
1520+
const source = fs.readFileSync(
1521+
path.join(REPO_ROOT, "src", "lib", "policies.ts"),
1522+
"utf-8",
1523+
);
1524+
// unref() above is sticky — a subsequent createInterface will not
1525+
// re-ref by itself; an explicit ref() before each one keeps follow-up
1526+
// prompts able to wait for input.
1527+
const refMatches = source.match(
1528+
/process\.stdin\.ref\(\)[\s\S]*?readline\.createInterface\(\{\s*input:\s*process\.stdin/g,
1529+
);
1530+
expect(refMatches?.length ?? 0).toBeGreaterThanOrEqual(2);
1531+
});
1532+
});
15011533
});

0 commit comments

Comments
 (0)