Skip to content

Commit 68d5d88

Browse files
committed
Addressed copilot feedback
1 parent 4bd4308 commit 68d5d88

2 files changed

Lines changed: 54 additions & 4 deletions

File tree

src/utils/prompt-confirmation.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import * as readline from "node:readline";
66
* Accepts both "y" and "yes" as affirmative responses (case-insensitive).
77
*
88
* @param message - The confirmation message to display to the user
9-
* @param defaultValue - The value returned when the user presses Enter without typing (default: false)
9+
* @param defaultValue - The value returned when the user presses Enter without typing (default: false).
10+
* Note: SIGINT and close always resolve to false regardless of defaultValue, since cancellation should never confirm.
1011
* @returns Promise<boolean> - true if user confirms (y/yes), false otherwise
1112
*/
1213
export function promptForConfirmation(
@@ -44,11 +45,11 @@ export function promptForConfirmation(
4445
};
4546

4647
rl.on("SIGINT", () => {
47-
finish(defaultValue);
48+
finish(false);
4849
});
4950

5051
rl.on("close", () => {
51-
finish(defaultValue);
52+
finish(false);
5253
});
5354

5455
rl.question(promptMessage, (answer) => {

test/unit/utils/prompt-confirmation.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, it, expect, vi, beforeEach } from "vitest";
22

33
let mockQuestion: (query: string, callback: (answer: string) => void) => void;
4+
let mockOnHandlers: Record<string, (() => void)[]>;
45

56
vi.mock("node:readline", () => ({
67
createInterface: () => ({
@@ -9,7 +10,12 @@ vi.mock("node:readline", () => ({
910
question: (query: string, callback: (answer: string) => void) => {
1011
mockQuestion(query, callback);
1112
},
12-
on: vi.fn(),
13+
on: (event: string, handler: () => void) => {
14+
if (!mockOnHandlers[event]) {
15+
mockOnHandlers[event] = [];
16+
}
17+
mockOnHandlers[event].push(handler);
18+
},
1319
}),
1420
}));
1521

@@ -18,6 +24,7 @@ import { promptForConfirmation } from "../../../src/utils/prompt-confirmation.js
1824
describe("promptForConfirmation", () => {
1925
beforeEach(() => {
2026
vi.restoreAllMocks();
27+
mockOnHandlers = {};
2128
});
2229

2330
it.each(["y", "yes", "Y", "YES", " yes "])(
@@ -103,4 +110,46 @@ describe("promptForConfirmation", () => {
103110
expect(capturedQuery).toBe("Continue? [Y/n]");
104111
});
105112
});
113+
114+
describe("SIGINT and close handling", () => {
115+
it("returns false on SIGINT even when defaultValue is true", async () => {
116+
mockQuestion = () => {
117+
for (const handler of mockOnHandlers["SIGINT"] ?? []) {
118+
handler();
119+
}
120+
};
121+
const result = await promptForConfirmation("Did you mean X?", true);
122+
expect(result).toBe(false);
123+
});
124+
125+
it("returns false on close even when defaultValue is true", async () => {
126+
mockQuestion = () => {
127+
for (const handler of mockOnHandlers["close"] ?? []) {
128+
handler();
129+
}
130+
};
131+
const result = await promptForConfirmation("Did you mean X?", true);
132+
expect(result).toBe(false);
133+
});
134+
135+
it("returns false on SIGINT with default defaultValue", async () => {
136+
mockQuestion = () => {
137+
for (const handler of mockOnHandlers["SIGINT"] ?? []) {
138+
handler();
139+
}
140+
};
141+
const result = await promptForConfirmation("Delete this?");
142+
expect(result).toBe(false);
143+
});
144+
145+
it("returns false on close with default defaultValue", async () => {
146+
mockQuestion = () => {
147+
for (const handler of mockOnHandlers["close"] ?? []) {
148+
handler();
149+
}
150+
};
151+
const result = await promptForConfirmation("Delete this?");
152+
expect(result).toBe(false);
153+
});
154+
});
106155
});

0 commit comments

Comments
 (0)