[codex] Fix confirm prompt default regression

[Oranquelui]

Summary

• avoid duplicating (default: ...) in confirm prompts when the message already includes a default suffix
• normalize empty confirm answers to a boolean instead of returning raw string defaults
• add regression coverage for both prompt behaviors

Root Cause

The previous fix switched confirm rendering from message to displayMessage, but some callers already embed (default: Yes) in the prompt text. That caused duplicated default text in the rendered confirm line. The confirm handler also returned the raw default value on empty input, which could leak string defaults instead of a boolean.

Validation

• COREPACK_ENABLE_AUTO_PIN=0 pnpm exec biome check src/lib/prompts.js src/test/test.js
• COREPACK_ENABLE_AUTO_PIN=0 pnpm test

Summary by CodeRabbit

• Bug Fixes

  • Avoid duplicate "(default: ...)" annotations in confirmation prompts.
  • Compute confirmation default using broader true-like checks (e.g., "true", "yes", "1") and display as Yes/No; empty submit now resolves to the boolean default.
  • Update displayed (Y/n)/(y/N) hint and redraw selected Yes/No on interactive terminals for clearer feedback.

• Tests

  • Added tests validating confirm prompt rendering, default annotation behavior, and boolean normalization.

Confidence Score: 4/5

Safe to merge; the logic fix and boolean normalization are correct, with only a minor cosmetic double-space regression remaining.

The core bugs (duplicate default label, non-boolean confirm answer) are correctly fixed and covered by new regression tests. Prior review concerns about normalization asymmetry are now addressed by including "yes" and "1" alongside "true". The single remaining issue is a visual double-space between the default annotation and the confirm hint — a cosmetic P2 introduced by switching from message to displayMessage without trimming.

src/lib/prompts.js line 47 (double-space before confirmHint)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[processQuestion called] --> B{type === 'confirm'?}
    B -- Yes --> C[Compute confirmDefault\nbool from defaultVal]
    C --> D[Compute hasDefaultSuffix\nregex on message]
    D --> E[Build displayMessage\nappend default suffix only\nif not already present]
    E --> F[Print prompt with\ndisplayMessage + confirmHint]
    F --> G{User input}
    G -- empty --> H[answer = confirmDefault boolean]
    G -- y/yes --> I[answer = true]
    G -- n/no --> J[answer = false]
    H & I & J --> K{isTTY?}
    K -- Yes --> L[renderConfirmSelection\nredraw line with Yes/No]
    K -- No --> M[skip redraw]
    L & M --> N[return answer as boolean]
    B -- No --> O[Other types: list / input]

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/lib/prompts.js
Line: 47

Comment:
**Double space before confirm hint**

`displayMessage` is constructed with a trailing space (see line 30: the template literal ends with `" "`). When that value is passed through `chalk.bold(displayMessage)`, the trailing space is preserved inside the bold section. The ` ` separator that follows it then produces a double space between the default annotation and the hint, e.g.:

```
? Do you want to use TypeScript? (default: Yes)  (Y/n)
```

The original code avoided this by using the raw `message` (no trailing space) in the confirm branch. The simplest fix here is to trim `displayMessage` inline, matching what `renderConfirmSelection` already does on line 35:

```suggestion
        `${chalk.green("?")} ${chalk.bold(displayMessage.trimEnd())} ${chalk.dim(confirmHint)} `,
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (3): Last reviewed commit: "Update src/lib/prompts.js" | Re-trigger Greptile}

[gaureshpai]

@coderabbitai review
@greptile review
/gemini review

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/prompts.js`:
- Around line 32-42: renderConfirmSelection assumes the prompt occupies one
terminal row and only moves the cursor up one line and clears a single line,
causing artifacts when the prompt wrapped to multiple rows; update
renderConfirmSelection (and any use of displayMessage) to compute the number of
wrapped rows using process.stdout.columns and the length of the renderedMessage
(including default/choice text) and then move the cursor and clear that many
lines, or alternatively seek to the start of the original prompt and call
readline.clearScreenDown before redrawing; ensure you handle non-TTY checks
unchanged and use readline.cursorTo/readline.moveCursor in a loop or call
readline.clearScreenDown from the original prompt position to reliably remove
all wrapped lines.

🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 041dddde-6dd3-4072-b62e-a9b84c1f81a1

📥 Commits

Reviewing files that changed from the base of the PR and between 63a895d and 4c12500.

📒 Files selected for processing (2)

• src/lib/prompts.js
• src/test/test.js

✅ Files skipped from review due to trivial changes (1)

• src/test/test.js

📜 Review details

🔇 Additional comments (1)

src/lib/prompts.js (1)

24-31: LGTM — default-suffix anchoring and boolean normalization look correct.

End-anchored regex correctly avoids matching mid-sentence (default: …), and confirmDefault now covers the "true" string case symmetrically with the already-tested "false" case. Reusing confirmDefault for both the hint and empty-input resolution keeps the three code paths consistent.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Redraw assumes prompt occupies exactly one terminal row.

renderConfirmSelection unconditionally moves the cursor up one row and clears a single line. When the rendered prompt wraps (long message + (default: …) + (Y/n) on a narrow terminal, or any caller with a multi-line message), only the final wrapped row is cleared, leaving a stale partial prompt above the redrawn line. Consider measuring wrapped rows from process.stdout.columns and clearing each, or using readline.clearScreenDown from the start of the original prompt.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/prompts.js` around lines 32 - 42, renderConfirmSelection assumes the
prompt occupies one terminal row and only moves the cursor up one line and
clears a single line, causing artifacts when the prompt wrapped to multiple
rows; update renderConfirmSelection (and any use of displayMessage) to compute
the number of wrapped rows using process.stdout.columns and the length of the
renderedMessage (including default/choice text) and then move the cursor and
clear that many lines, or alternatively seek to the start of the original prompt
and call readline.clearScreenDown before redrawing; ensure you handle non-TTY
checks unchanged and use readline.cursorTo/readline.moveCursor in a loop or call
readline.clearScreenDown from the original prompt position to reliably remove
all wrapped lines.

[greptile-apps]

Cursor-up by 1 assumes single-line prompt

readline.moveCursor(process.stdout, 0, -1) moves up exactly one terminal row. If displayMessage is long enough to wrap onto a second (or more) row, the redraw will land on the wrong line, leaving a ghost of the original prompt on screen.

This is a cosmetic edge-case for long messages, but worth noting. A more robust approach is to pre-calculate the number of wrapped rows:

const cols = process.stdout.columns || 80;
const rawLine = `? ${renderedMessage} ${confirmHint}`;
const wrappedRows = Math.max(1, Math.ceil(rawLine.length / cols));
readline.moveCursor(process.stdout, 0, -wrappedRows);

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/lib/prompts.js
Line: 36

Comment:
**Cursor-up by 1 assumes single-line prompt**

`readline.moveCursor(process.stdout, 0, -1)` moves up exactly one terminal row. If `displayMessage` is long enough to wrap onto a second (or more) row, the redraw will land on the wrong line, leaving a ghost of the original prompt on screen.

This is a cosmetic edge-case for long messages, but worth noting. A more robust approach is to pre-calculate the number of wrapped rows:
```js
const cols = process.stdout.columns || 80;
const rawLine = `? ${renderedMessage} ${confirmHint}`;
const wrappedRows = Math.max(1, Math.ceil(rawLine.length / cols));
readline.moveCursor(process.stdout, 0, -wrappedRows);
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Injecting JSON into an eval-style code string

${JSON.stringify(question)} is interpolated directly into the -e script string. JSON.stringify escapes all dangerous characters correctly, so this is safe for the current usage. However, the pattern is fragile: if question ever gains a field whose serialised value contains backticks or template-literal syntax in a future Node.js version, this could break or be exploited in a test-time attack.

A safer and more maintainable alternative is to pass the question as a serialised environment variable and parse it inside the script:

const env = { ...process.env, PROMPT_QUESTION: JSON.stringify(question) };
// inside -e script:
// const result = await prompts.prompt([JSON.parse(process.env.PROMPT_QUESTION)]);

For test code this is low-risk, but worth documenting or hardening.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/test/test.js
Line: 23-30

Comment:
**Injecting JSON into an eval-style code string**

`${JSON.stringify(question)}` is interpolated directly into the `-e` script string. `JSON.stringify` escapes all dangerous characters correctly, so this is safe for the current usage. However, the pattern is fragile: if `question` ever gains a field whose serialised value contains backticks or template-literal syntax in a future Node.js version, this could break or be exploited in a test-time attack.

A safer and more maintainable alternative is to pass the question as a serialised environment variable and parse it inside the script:
```js
const env = { ...process.env, PROMPT_QUESTION: JSON.stringify(question) };
// inside -e script:
// const result = await prompts.prompt([JSON.parse(process.env.PROMPT_QUESTION)]);
```
For test code this is low-risk, but worth documenting or hardening.

How can I resolve this? If you propose a fix, please make it concise.

[gaureshpai]

Check for the review comments above. Thank you

The behavior right now has redundant prompt display for example: if I go with default option for Typescript, that line in itself should change to display Yes in place of the options

Current behavior has the line display again with the proper status after the line

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

❌ Failed to clone repository into sandbox. Please try again.