Skip to content

Conversation

@jon-freed
Copy link
Contributor

@jon-freed jon-freed commented Jan 17, 2026

What does this PR do?

Fixes a bug where CLI arguments fail to override --flags-dir values in certain scenarios. The bug is in the tpreparse hook where it is using ?? (nullish coalescing) instead of || (logical OR) when checking if a CLI flag was already present.

Bug Details

Introduced in: Commit c83f81a (2025-04-22, "chore: bump wireit, fix types")

Affected versions: SF CLI 2.100.0 through 2.120.x

The change added a necessary guard (flagOptions.char &&) to prevent checking for undefined short chars, but incorrectly used ?? instead of || to chain conditions:

// Before c83f81a (correct):
argv.includes(`-${flagOptions.char}`) ||
argv.includes(`--${flagName}`) ||

// After c83f81a (buggy):
(flagOptions.char && argv.includes(`-${flagOptions.char}`)) ??
argv.includes(`--${flagName}`) ??

Since ?? only evaluates the right side when left is null/undefined, and false is neither, the long-form check was skipped when the short-char check returned false.

Affected Scenarios

The bug affects scenarios where:

  • A flag has a short char defined (e.g., { char: 'l' } for --test-level)
  • The user invokes the long form on CLI (e.g., --test-level instead of -l)
  • The same flag exists in --flags-dir

Example:

# flags-dir/test-level contains: NoTestRun
# User wants to override with RunLocalTests:
sf project deploy start --flags-dir ./flags --test-level RunLocalTests

# Expected: RunLocalTests wins (CLI overrides flags-dir)
# With bug: Error "Flag --test-level can only be specified once"

5 specific scenarios are affected (out of 14 tested):

  1. String flag with char, CLI uses long form
  2. Boolean flag with char, CLI uses long form
  3. Boolean allowNo flag (no char), CLI uses --no-<flag> form
  4. Boolean allowNo with char, CLI uses --<flag> long form
  5. Boolean allowNo with char, CLI uses --no-<flag> long form

Root Cause

The ESLint rule @typescript-eslint/prefer-nullish-coalescing likely suggested the change from || to ??, but the rule's advice was incorrect in this context. The fix includes an eslint-disable comment to prevent this from recurring.


Acceptance Criteria

  • CLI arguments properly override --flags-dir values as documented in PR feat: add preparse hook for --flags-dir #1536
  • Multiple-value flags still combine CLI and file values (unchanged behavior)
  • Comprehensive tests cover all flag type / char config / CLI form combinations
  • All existing tests continue to pass

Testing Notes

Added systematic tests organized by:

  • [A] Flag type: String single, String multiple, Boolean, Boolean allowNo
  • [B] Short char config: No char vs has char
  • [C] CLI invocation: Short form vs long form

The tests verify the documented behavior from PR #1536:

"Flags/values provided by the user will override any flag/values found by --flags-dir -- unless the flag supports multiple, in which case they will all be combined."


Checklist


What issues does this PR fix or reference?

Fixes forcedotcom/cli#3486

Fixes regression introduced in c83f81a. Restores the behavior documented in PR #1536.

…e hook

The preparse hook was using ?? (nullish coalescing) instead of || (logical OR)
when checking if a CLI flag was already present. This caused a bug where CLI
arguments would not override flags-dir values in certain scenarios.

Bug introduced in: c83f81a (2025-04-22, SF CLI 2.100.0)
The change added a necessary guard (flagOptions.char &&) to prevent checking
for undefined short chars, but incorrectly used ?? instead of || to chain the
conditions. Since ?? only evaluates the right side when left is null/undefined,
and false is neither, the subsequent checks were skipped.

Affected scenarios (5 of 14 systematic test cases):
- String flag with char defined, CLI uses long form (--test-level vs -l)
- Boolean flag with char defined, CLI uses long form (--dry-run vs -d)
- Boolean allowNo flag, CLI uses --no-<flag> form to override <flag> file
- Boolean allowNo with char, CLI uses --<flag> to override no-<flag> file
- Boolean allowNo with char, CLI uses --no-<flag> to override <flag> file

Example: Flag defined as Flags.string({ char: 'l' }) with flags-dir containing
a 'test-level' file. Running `sf cmd --test-level NoTestRun` should override
the file value, but with ?? it was erroneously combined (conflict error).

This fix restores the documented behavior from PR salesforcecli#1536:
"Flags/values provided by the user will override any flag/values found by
--flags-dir -- unless the flag supports `multiple`, in which case they will
all be combined."

The eslint-disable is intentional because the lint rule incorrectly suggests ??
when || is actually required for proper boolean short-circuit evaluation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Command line arguments fail to override --flags-dir values when flag has short/char form but long form is used

1 participant