Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions src/lib/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,13 @@ export function promptSecret(question: string): Promise<string> {
return new Promise((resolve, reject) => {
const input = process.stdin;
const output = process.stderr;
// Re-attach stdin to the event loop. cleanup() below unrefs after the
// read completes (so a wizard ending here exits naturally), and unref()
// is sticky — without this the next direct promptSecret() call would
// listen on a detached handle. Idempotent when prompt() already ref'd.
if (typeof input.ref === "function") {
input.ref();
}
let answer = "";
let rawModeEnabled = false;
let finished = false;
Expand All @@ -399,9 +406,15 @@ export function promptSecret(question: string): Promise<string> {
if (rawModeEnabled && typeof input.setRawMode === "function") {
input.setRawMode(false);
}
// pause+unref so a wizard ending on a secret prompt exits naturally.
// The matching ref() in prompt() (and any direct caller) restores the
// event-loop ref before the next read.
if (typeof input.pause === "function") {
input.pause();
}
if (typeof input.unref === "function") {
input.unref();
}
}

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

function cleanup() {
rl.close();
if (!process.stdin.isTTY) {
if (typeof process.stdin.pause === "function") {
process.stdin.pause();
}
if (typeof process.stdin.unref === "function") {
process.stdin.unref();
}
// pause+unref so the process exits naturally after the last prompt
// resolves. The matching ref() above keeps subsequent prompts working;
// unref()-ing a TTY ReadStream only releases the event-loop reference,
// cooked/raw mode and any subsequent reads remain unaffected.
if (typeof process.stdin.pause === "function") {
process.stdin.pause();
}
if (typeof process.stdin.unref === "function") {
process.stdin.unref();
}
}

Expand Down
35 changes: 35 additions & 0 deletions src/lib/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5316,6 +5316,14 @@ async function setupMessagingChannels(): Promise<string[]> {
if (rawModeEnabled && typeof input.setRawMode === "function") {
input.setRawMode(false);
}
// Symmetric with the ref() at the entry; lets the wizard exit
// naturally if this is the last prompt.
if (typeof input.pause === "function") {
input.pause();
}
if (typeof input.unref === "function") {
input.unref();
}
}

function finish(): void {
Expand Down Expand Up @@ -5353,6 +5361,12 @@ async function setupMessagingChannels(): Promise<string[]> {
}
}

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

// Re-attach stdin to the event loop. A prior prompt cleanup may have
// unref'd it (sticky), and resume() alone would leave the raw-mode read
// detached from the loop.
if (typeof process.stdin.ref === "function") process.stdin.ref();
process.stdin.setRawMode(true);
process.stdin.resume();
process.stdin.setEncoding("utf8");
Expand All @@ -5774,6 +5792,9 @@ async function selectPolicyTier(): Promise<string> {
const cleanup = () => {
process.stdin.setRawMode(false);
process.stdin.pause();
// Symmetric with the ref() at the entry; lets the wizard exit
// naturally if this is the last prompt.
if (typeof process.stdin.unref === "function") process.stdin.unref();
process.stdin.removeListener("data", onData);
process.removeListener("SIGTERM", onSigterm);
};
Expand Down Expand Up @@ -5950,6 +5971,10 @@ async function selectTierPresetsAndAccess(
lineCount = lines.length;
};

// Re-attach stdin to the event loop. A prior prompt cleanup may have
// unref'd it (sticky), and resume() alone would leave the raw-mode read
// detached from the loop.
if (typeof process.stdin.ref === "function") process.stdin.ref();
process.stdin.setRawMode(true);
process.stdin.resume();
process.stdin.setEncoding("utf8");
Expand All @@ -5958,6 +5983,9 @@ async function selectTierPresetsAndAccess(
const cleanup = () => {
process.stdin.setRawMode(false);
process.stdin.pause();
// Symmetric with the ref() at the entry; lets the wizard exit
// naturally if this is the last prompt.
if (typeof process.stdin.unref === "function") process.stdin.unref();
process.stdin.removeListener("data", onData);
process.removeListener("SIGTERM", onSigterm);
};
Expand Down Expand Up @@ -6093,6 +6121,10 @@ async function presetsCheckboxSelector(
lineCount = lines.length;
};

// Re-attach stdin to the event loop. A prior prompt cleanup may have
// unref'd it (sticky), and resume() alone would leave the raw-mode read
// detached from the loop.
if (typeof process.stdin.ref === "function") process.stdin.ref();
process.stdin.setRawMode(true);
process.stdin.resume();
process.stdin.setEncoding("utf8");
Expand All @@ -6101,6 +6133,9 @@ async function presetsCheckboxSelector(
const cleanup = () => {
process.stdin.setRawMode(false);
process.stdin.pause();
// Symmetric with the ref() at the entry; lets the wizard exit
// naturally if this is the last prompt.
if (typeof process.stdin.unref === "function") process.stdin.unref();
process.stdin.removeListener("data", onData);
process.removeListener("SIGTERM", onSigterm);
};
Expand Down
22 changes: 14 additions & 8 deletions src/lib/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,13 +440,16 @@ function selectForRemoval(
});
process.stderr.write("\n");
const question = " Choose preset to remove: ";
// Re-attach stdin to the event loop — unref() on exit is sticky and
// would otherwise leave a follow-up prompt waiting on a detached handle.
if (typeof process.stdin.ref === "function") process.stdin.ref();
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
rl.question(question, (answer: string) => {
rl.close();
if (!process.stdin.isTTY) {
if (typeof process.stdin.pause === "function") process.stdin.pause();
if (typeof process.stdin.unref === "function") process.stdin.unref();
}
// pause+unref so the process exits naturally after the last prompt.
// The matching ref() above keeps subsequent prompts working.
if (typeof process.stdin.pause === "function") process.stdin.pause();
if (typeof process.stdin.unref === "function") process.stdin.unref();
const trimmed = answer.trim();
if (!trimmed) {
resolve(null);
Expand Down Expand Up @@ -764,13 +767,16 @@ function selectFromList(
const defaultIdx = items.findIndex((item) => !applied.includes(item.name));
const defaultNum = defaultIdx >= 0 ? defaultIdx + 1 : null;
const question = defaultNum ? ` Choose preset [${defaultNum}]: ` : " Choose preset: ";
// Re-attach stdin to the event loop — unref() on exit is sticky and
// would otherwise leave a follow-up prompt waiting on a detached handle.
if (typeof process.stdin.ref === "function") process.stdin.ref();
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
rl.question(question, (answer: string) => {
rl.close();
if (!process.stdin.isTTY) {
if (typeof process.stdin.pause === "function") process.stdin.pause();
if (typeof process.stdin.unref === "function") process.stdin.unref();
}
// pause+unref so the process exits naturally after the last prompt.
// The matching ref() above keeps subsequent prompts working.
if (typeof process.stdin.pause === "function") process.stdin.pause();
if (typeof process.stdin.unref === "function") process.stdin.unref();
const trimmed = answer.trim();
const effectiveInput = trimmed || (defaultNum ? String(defaultNum) : "");
if (!effectiveInput) {
Expand Down
7 changes: 7 additions & 0 deletions src/lib/sandbox-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -752,9 +752,16 @@ function readStdin(): Promise<string> {
*/
function confirmYesNo(prompt: string): Promise<boolean> {
return new Promise((resolve) => {
// Re-attach stdin to the event loop — unref() on exit is sticky and
// would otherwise leave a follow-up prompt waiting on a detached handle.
if (typeof process.stdin.ref === "function") process.stdin.ref();
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
rl.question(prompt, (answer: string) => {
rl.close();
// pause+unref so the process exits naturally after the last prompt.
// The matching ref() above keeps subsequent prompts working.
if (typeof process.stdin.pause === "function") process.stdin.pause();
if (typeof process.stdin.unref === "function") process.stdin.unref();
resolve(/^y(es)?$/i.test(answer.trim()));
});
});
Expand Down
75 changes: 75 additions & 0 deletions test/credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,4 +504,79 @@ describe("prompt machinery (unchanged)", () => {
expect(source).toContain('output.write("*")');
expect(source).toContain('output.write("\\b \\b")');
});

it("releases stdin after a prompt resolves so the event loop drains on a TTY", () => {
const source = fs.readFileSync(
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
"utf-8",
);

// The previous TTY-only guard kept the event loop pinned on interactive
// runs — the wizard would not exit after its last prompt.
expect(source).not.toMatch(/cleanup\s*\(\s*\)\s*\{\s*rl\.close\(\);\s*if\s*\(\s*!process\.stdin\.isTTY\s*\)/);
expect(source).toMatch(
/function cleanup\(\)\s*\{\s*rl\.close\(\);[\s\S]*?process\.stdin\.pause\(\)[\s\S]*?process\.stdin\.unref\(\)/,
);
});

it("re-refs stdin before each prompt so a follow-up prompt is not stranded by a sticky unref()", () => {
const source = fs.readFileSync(
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
"utf-8",
);

// unref() is sticky — readline.createInterface() will not re-ref by
// itself, so a sequential prompt after the first cleanup would see a
// detached stdin handle and the process could exit before the user
// can answer. The matching ref() at the top of `prompt()` undoes that.
expect(source).toMatch(
/process\.stdin\.ref\(\)[\s\S]*?readline\.createInterface\(\{\s*input:\s*process\.stdin/,
);
});

it("re-refs stdin even on the secret-prompt branch so a follow-up secret read is not stranded", () => {
const source = fs.readFileSync(
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
"utf-8",
);

// The ref() must come before the silent/secret branch so that a
// sequence of `prompt()` -> `prompt({ secret: true })` after a normal
// prompt's sticky unref() still has a ref'd handle for promptSecret().
const refIdx = source.search(/process\.stdin\.ref\(\);/);
const silentIdx = source.search(/const silent = opts\.secret === true/);
expect(refIdx).toBeGreaterThan(0);
expect(silentIdx).toBeGreaterThan(0);
expect(refIdx).toBeLessThan(silentIdx);
});

it("releases stdin in promptSecret() cleanup so a wizard ending on a secret prompt exits naturally", () => {
const source = fs.readFileSync(
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
"utf-8",
);

// The secret reader uses raw mode + a `data` listener instead of
// readline. Its cleanup must still pause+unref or the wizard hangs the
// same way the readline path did.
expect(source).toMatch(
/promptSecret[\s\S]*?function cleanup\(\)\s*\{[\s\S]*?input\.pause\(\)[\s\S]*?input\.unref\(\)/,
);
});

it("re-refs stdin at the top of promptSecret() so a direct caller is self-contained", () => {
const source = fs.readFileSync(
path.join(import.meta.dirname, "..", "src", "lib", "credentials.ts"),
"utf-8",
);

// promptSecret() is exported and used directly elsewhere. Because its
// own cleanup unref()s stdin, two sequential direct calls (or any call
// after a prior unref) would strand the second read without an entry
// ref(). Assert ref() is the first effectful call inside the body.
expect(source).toMatch(
/export function promptSecret[\s\S]*?const input = process\.stdin;[\s\S]{0,400}?input\.ref\(\);[\s\S]*?function cleanup/,
);
});

});
38 changes: 38 additions & 0 deletions test/onboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2648,6 +2648,44 @@ const { setupInference } = require(${onboardPath});
assert.equal(typeof streamSandboxCreate, "function");
});

it("re-refs stdin before each raw-mode prompt and unrefs in cleanup so sticky unref() does not strand later prompts", () => {
const source = fs.readFileSync(
path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"),
"utf-8",
);

// The shared `prompt()` cleanup unref()s stdin so the wizard exits
// naturally after its last readline prompt. unref() is sticky, so the
// raw-mode TUI selectors (messaging channels + the three arrow-key
// pickers) must explicitly ref() stdin before resume()/setRawMode(true)
// or they would otherwise listen on a detached handle.
const refMatches = source.match(
/process\.stdin\.ref\(\);[\s\S]{0,180}?process\.stdin\.setRawMode\(true\)/g,
);
assert.ok(
refMatches !== null && refMatches.length >= 3,
`expected at least 3 ref()-then-setRawMode(true) sites, found ${
refMatches ? refMatches.length : 0
}`,
);

// The messaging-channels picker uses an `input.ref()` alias on the
// captured handle. Same contract, different binding.
assert.match(source, /input\.ref\(\)[\s\S]{0,200}?input\.setRawMode\(true\)/);

// Each raw-mode cleanup must release stdin too, so a wizard that ends
// on a TUI selector exits cleanly.
const unrefMatches = source.match(
/setRawMode\(false\);[\s\S]{0,400}?(?:process\.stdin|input)\.unref\(\)/g,
);
assert.ok(
unrefMatches !== null && unrefMatches.length >= 4,
`expected at least 4 setRawMode(false)-then-unref() sites, found ${
unrefMatches ? unrefMatches.length : 0
}`,
);
});

it("migrates a legacy credentials.json into env so setupInference can register the provider", () => {
const repoRoot = path.join(import.meta.dirname, "..");
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-resume-cred-"));
Expand Down
32 changes: 32 additions & 0 deletions test/policies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1498,4 +1498,36 @@ setImmediate(() => {
expect(loads[0]).toMatch(/real\.yaml$/);
});
});

describe("interactive prompt cleanup", () => {
it("releases stdin after preset prompts so the event loop drains on a TTY", () => {
const source = fs.readFileSync(
path.join(REPO_ROOT, "src", "lib", "policies.ts"),
"utf-8",
);
// A TTY-only guard around pause/unref pins the event loop on
// interactive runs and stops the wizard from exiting after its last
// prompt resolves.
expect(source).not.toMatch(/rl\.close\(\);\s*if\s*\(\s*!process\.stdin\.isTTY\s*\)/);
// Both prompt callbacks must release stdin after `rl.close()`.
const cleanupMatches = source.match(
/rl\.close\(\);[\s\S]*?process\.stdin\.pause\(\)[\s\S]*?process\.stdin\.unref\(\)/g,
);
expect(cleanupMatches?.length ?? 0).toBeGreaterThanOrEqual(2);
});

it("re-refs stdin before each preset prompt so a follow-up prompt is not stranded by a sticky unref()", () => {
const source = fs.readFileSync(
path.join(REPO_ROOT, "src", "lib", "policies.ts"),
"utf-8",
);
// unref() above is sticky — a subsequent createInterface will not
// re-ref by itself; an explicit ref() before each one keeps follow-up
// prompts able to wait for input.
const refMatches = source.match(
/process\.stdin\.ref\(\)[\s\S]*?readline\.createInterface\(\{\s*input:\s*process\.stdin/g,
);
expect(refMatches?.length ?? 0).toBeGreaterThanOrEqual(2);
});
});
});
Loading