Skip to content
Open
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
93 changes: 93 additions & 0 deletions src/gateway/interactive-readline.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import {
createInteractiveReadline,
disposeInteractiveReadline,
__resetInteractiveReadlineForTests,
__getInteractiveReadlineInvocationCountForTests,
} from "./interactive-readline.js";

describe("interactive-readline factory", () => {
beforeEach(() => {
__resetInteractiveReadlineForTests();
});

afterEach(() => {
__resetInteractiveReadlineForTests();
delete process.env.LOG_LEVEL;
});

it("returns a fresh, independent interface per call", () => {
const rl1 = createInteractiveReadline();
const rl2 = createInteractiveReadline();

expect(rl1).not.toBe(rl2);
expect(__getInteractiveReadlineInvocationCountForTests()).toBe(2);
});

it("uses terminal:false (cooked mode) to avoid Windows raw-mode issues", () => {
// With terminal:false, readline does NOT attach a keypress listener
// to stdin. We can't directly read the option from the public API,
// but we can assert the side effect: no keypress listener growth.
const before = process.stdin.listenerCount("keypress");

const rl = createInteractiveReadline();

expect(process.stdin.listenerCount("keypress")).toBe(before);

disposeInteractiveReadline(rl);
});

it("disposeInteractiveReadline brings keypress listener count back down", () => {
const before = process.stdin.listenerCount("keypress");

const rl = createInteractiveReadline();
disposeInteractiveReadline(rl);

expect(process.stdin.listenerCount("keypress")).toBeLessThanOrEqual(before);
});

it("does not remove keypress listeners that existed before the prompt", () => {
const existingListener = () => undefined;
const leakedPromptListener = () => undefined;
process.stdin.on("keypress", existingListener);

try {
const rl = createInteractiveReadline();
process.stdin.on("keypress", leakedPromptListener);
disposeInteractiveReadline(rl);

expect(process.stdin.listeners("keypress")).toContain(existingListener);
expect(process.stdin.listeners("keypress")).not.toContain(
leakedPromptListener,
);
} finally {
process.stdin.removeListener("keypress", existingListener);
process.stdin.removeListener("keypress", leakedPromptListener);
}
});

it("disposeInteractiveReadline is safe to call on an already-closed interface", () => {
const rl = createInteractiveReadline();
rl.close();
expect(() => disposeInteractiveReadline(rl)).not.toThrow();
});

it("does not accumulate keypress listeners across create/dispose cycles", () => {
const before = process.stdin.listenerCount("keypress");

for (let i = 0; i < 5; i += 1) {
const rl = createInteractiveReadline();
disposeInteractiveReadline(rl);
}

expect(process.stdin.listenerCount("keypress")).toBeLessThanOrEqual(before);
});

it("does not throw when debug logging is enabled", () => {
process.env.LOG_LEVEL = "debug";
expect(() => {
const rl = createInteractiveReadline();
disposeInteractiveReadline(rl);
}).not.toThrow();
});
});
104 changes: 104 additions & 0 deletions src/gateway/interactive-readline.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import readline from "node:readline/promises";
import { stdin as input, stdout as output } from "node:process";
import { appendStderrDevLog } from "../runtime/stderr-dev-log.js";

/**
* Create a readline interface for one-shot prompts (tool approval, user
* clarification). Use `disposeInteractiveReadline` in the prompt's
* `finally` block to tear it down.
*
* Uses `terminal: false` deliberately. The previous `terminal: true`
* configuration put stdin into raw mode and attached a `keypress` listener,
* both of which are unreliable on Windows (ConHost / Windows Terminal):
*
* 1. `rl.close()` does not reliably remove the keypress listener, causing
* listener accumulation across prompts. At the Nth prompt, each leaked
* listener echoed the same keystroke, so pressing `y` once rendered as
* N `y` characters.
* 2. Windows console focus / IME / mode-switch events leak through raw
* mode as spurious empty line events, so `rl.question()` resolved with
* `""` on its own — the "auto-advance" symptom where each prompt
* disappeared without user input.
*
* `terminal: false` keeps stdin in cooked mode: the OS handles line editing
* and delivers complete lines only when the user presses Enter. No raw
* mode, no keypress listener, no spurious events.
*
* Trade-off: no in-line editing (backspace, arrow keys, history). For y/n
* approval prompts that's acceptable; users type one char + Enter.
*/
let promptInvocationCount = 0;
const keypressListenersByReadline = new WeakMap<
readline.Interface,
Function[]
>();

function isDebugLogEnabled(): boolean {
const level = process.env.LOG_LEVEL;
return level === "debug" || level === "trace";
}

function logApprovalReadlineDebug(message: string): void {
if (!isDebugLogEnabled()) return;
void appendStderrDevLog(`[debug] event=approval-readline ${message}\n`);
}

export function createInteractiveReadline(): readline.Interface {
promptInvocationCount += 1;
const keypressListenersBeforeCreate = input.listeners("keypress");
logApprovalReadlineDebug(
`create invocation=${promptInvocationCount} terminal=false`,
);
const rl = readline.createInterface({
input,
output,
terminal: false,
});
keypressListenersByReadline.set(rl, keypressListenersBeforeCreate);
return rl;
}

export function disposeInteractiveReadline(rl: readline.Interface): void {
const keypressListenersBeforeCreate =
keypressListenersByReadline.get(rl) ?? [];
logApprovalReadlineDebug(
`dispose invocation=${promptInvocationCount} keypressListeners=${input.listenerCount("keypress")}`,
);

try {
rl.close();
} catch {
// Ignore: may already be closed.
}

// A console host can still leave a keypress listener behind. Remove only
// listeners added after this interface was created, never ones owned by
// the long-lived REPL readline interface.
const retainedListenerCounts = new Map<Function, number>();
for (const listener of keypressListenersBeforeCreate) {
retainedListenerCounts.set(
listener,
(retainedListenerCounts.get(listener) ?? 0) + 1,
);
}
for (const listener of input.listeners("keypress")) {
const retainedCount = retainedListenerCounts.get(listener) ?? 0;
if (retainedCount > 0) {
retainedListenerCounts.set(listener, retainedCount - 1);
continue;
}
input.removeListener("keypress", listener);
}

keypressListenersByReadline.delete(rl);
}

/** Test-only: reset invocation counter between unit tests. */
export function __resetInteractiveReadlineForTests(): void {
promptInvocationCount = 0;
}

/** Test-only: observe invocation count. */
export function __getInteractiveReadlineInvocationCountForTests(): number {
return promptInvocationCount;
}
24 changes: 12 additions & 12 deletions src/gateway/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ import {
type StepCliInteractiveUi,
type StepCliInteractiveUiFactory,
} from "./interactive-ui.js";
import {
createInteractiveReadline,
disposeInteractiveReadline,
} from "./interactive-readline.js";
import { createFilesystemAgentRunArtifactStore } from "./artifacts/run-artifact-store.js";
import { FilesystemConversationTranscriptStore } from "./memory/filesystem-conversation-transcript-store.js";
import { FilesystemFreshAttemptProgressStore } from "./memory/filesystem-fresh-attempt-progress-store.js";
Expand Down Expand Up @@ -4606,11 +4610,10 @@ async function promptForToolApproval(
: "deny";
}

const rl = readline.createInterface({
input,
output,
terminal: true,
});
// Per-call readline in cooked mode (terminal:false). See
// `interactive-readline.ts` for why `terminal:true` is unreliable on
// Windows (listener leak + spurious empty line events).
const rl = createInteractiveReadline();

const preview =
request.rawArgs.length > 240
Expand Down Expand Up @@ -4692,7 +4695,7 @@ async function promptForToolApproval(
output.write(`Unrecognized decision: ${answer}. Type '?' for help.\n`);
}
} finally {
rl.close();
disposeInteractiveReadline(rl);
}
}

Expand All @@ -4708,11 +4711,8 @@ async function promptForUserClarification(
};
}

const rl = readline.createInterface({
input,
output,
terminal: true,
});
// Per-call readline in cooked mode. See `interactive-readline.ts`.
const rl = createInteractiveReadline();

const options = Array.isArray(request.options) ? request.options : [];
const helpLines = buildClarificationHelpLines(request);
Expand Down Expand Up @@ -4779,7 +4779,7 @@ async function promptForUserClarification(
output.write(`${parsed.message}\n`);
}
} finally {
rl.close();
disposeInteractiveReadline(rl);
}
}

Expand Down
1 change: 1 addition & 0 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export default defineConfig({
"src/bootstrap/config/defaults.ts",
"src/commands/option-parsers.ts",
"src/gateway/storage/layout.ts",
"src/gateway/interactive-readline.ts",
"src/gateway/verifier.ts",
"src/runtime/runtime-config.ts",
"src/runtime/runtime-utils.ts",
Expand Down
Loading