Skip to content

fix: ensure --yes flag assigns defaults for all config fields #1907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nikolasdehor wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: ensure --yes flag assigns defaults for all config fields #1907

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 deletions tools/cli/installers/lib/core/config-collector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,12 +735,13 @@ class ConfigCollector {
// Skip prompts mode: use all defaults without asking
if (this.skipPrompts) {
await prompts.log.info(`Using default configuration for ${moduleDisplayName}`);
// Use defaults for all questions
// Use defaults for all questions; use empty string for fields without defaults
for (const question of questions) {
const hasDefault = question.default !== undefined && question.default !== null && question.default !== '';
if (hasDefault && typeof question.default !== 'function') {
allAnswers[question.name] = question.default;
if (typeof question.default === 'function') {
continue;
Comment on lines +740 to +741
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Evaluate computed defaults here instead of dropping them.

Line 1151 already turns same-module defaults into question.default(answers). continue here skips those fields entirely, so --yes can still leave computed defaults unset.

Suggested fix 
         for (const question of questions) {
-          if (typeof question.default === 'function') {
-            continue;
-          }
-          const hasDefault = question.default !== undefined && question.default !== null && question.default !== '';
-          allAnswers[question.name] = hasDefault ? question.default : '';
+          const resolvedDefault =
+            typeof question.default === 'function'
+              ? question.default(allAnswers)
+              : question.default;
+          const hasDefault =
+            resolvedDefault !== undefined &&
+            resolvedDefault !== null &&
+            resolvedDefault !== '';
+          allAnswers[question.name] = hasDefault ? resolvedDefault : '';
         }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tools/cli/installers/lib/core/config-collector.js` around lines 740 - 741,
The code currently skips questions whose default is a function by executing
"continue" when it sees typeof question.default === 'function', which prevents
computed defaults from being applied in non-interactive (--yes) mode; instead,
call the default function with the current answers and assign its return value
into the answers map (the same way done earlier where question.default(answers)
is used), and only skip if the computed default is explicitly undefined; update
the branch around question.default to compute const value = await
question.default(answers) (handle Promise) and set answers[question.name] =
value when applicable so computed defaults are preserved in --yes mode.

}
const hasDefault = question.default !== undefined && question.default !== null && question.default !== '';
allAnswers[question.name] = hasDefault ? question.default : '';
Comment on lines +743 to +744
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't serialize '' for every prompt that lacks an explicit default.

This bypasses the prompt's validation rules and produces the wrong shape for non-input questions like confirm and checkbox. In headless mode, either use a type-aware fallback or fail fast when a required prompt has no explicit default.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tools/cli/installers/lib/core/config-collector.js` around lines 743 - 744,
The current headless fallback always serializes an empty string into
allAnswers[question.name] which breaks non-input prompts and bypasses
validation; update the headless branch that uses question.default to instead use
a type-aware fallback based on question.type (e.g., for 'confirm' set false, for
'checkbox' set [], for 'number' set null or a numeric sentinel, for 'list' use
null/empty array as appropriate) and preserve question.default when present;
additionally, when in headless mode and a prompt is required (question.required
or validation present) but has no explicit default, throw a clear error (fail
fast) rather than inserting a generic '' so validations still run correctly —
change the logic around question.default/allAnswers[question.name] to inspect
question.type and question.required and either assign the type-appropriate value
or raise an error.

⚠️ Potential issue | 🟠 Major

'' is not a real fix if downstream lookup still treats it as “missing”.

Lines 596-618 and 958-980 resolve placeholders with truthiness checks. A blank written here will be ignored by that code, so dependent {field} references can still leak into rendered config.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tools/cli/installers/lib/core/config-collector.js` around lines 743 - 744,
The code writes an empty string for unanswered questions which downstream
truthiness checks treat as "missing", allowing placeholder leaks; in the block
using allAnswers[question.name] and hasDefault, change the fallback from '' to
undefined (or omit the key) so that missing answers are represented as
undefined/null rather than an empty string; update the assignment in the
config-collector logic (the hasDefault check and the allAnswers[question.name]
assignment) to use question.default when present and otherwise leave the
property undefined or delete it instead of setting ''.

}
} else {
if (!this._silentConfig) await prompts.log.step(`Configuring ${moduleDisplayName}`);
Expand Down