Skip to content

fix(cli): reject invalid scale numeric options#730

Merged
ralyodio merged 2 commits into
profullstack:masterfrom
lazyGPT07:fix/scale-reject-invalid-numbers
Jun 14, 2026
Merged

fix(cli): reject invalid scale numeric options#730
ralyodio merged 2 commits into
profullstack:masterfrom
lazyGPT07:fix/scale-reject-invalid-numbers

Conversation

@lazyGPT07

Copy link
Copy Markdown
Contributor

Summary

  • reject malformed, fractional, and non-finite scale count options before command actions run
  • require safe integers for instance counts, auto-scale settings, DNS TTL, and rollout percentage
  • require a positive finite number for --max-hourly-price
  • add focused parser regressions

Closes #729

Validation

  • vitest run packages/cli/src/commands/scale.test.ts (34 passed)
  • git diff --check
  • CLI workspace typecheck reaches existing unresolved optional workspace package errors in build-actions.ts, openapi.ts, and ship.ts; no errors point to the changed files

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds four parser functions (parsePositiveInteger, parseNonNegativeInteger, parsePositiveNumber, parsePercentage) that throw InvalidArgumentError before command actions run, and wires them to every numeric option across scale up, scale down, scale auto, scale dns, and scale rollout.

  • Parser correctness: all four parsers guard against empty/whitespace strings, fractional values, non-safe integers, non-finite values, and (for parsePercentage) values outside 1–100; the empty-string edge case is explicitly covered.
  • Test coverage: focused it.each suites cover valid accepts and the key reject cases for each parser; parsePercentage delegates to parsePositiveInteger so NaN rejection is covered transitively.
  • Residual action-handler guards: --target-cpu still uses parsePositiveInteger (no upper-bound enforcement at parse time), so the > 100 branch in the action handler remains live — but the < 1 side of that compound condition is now dead code.

Confidence Score: 5/5

Safe to merge; the changes add parser-level validation that narrows accepted inputs without altering any happy-path logic.

All changed code is additive input validation. Each parser is straightforward, correctly ordered (empty-check before coercion, safe-integer before range), and backed by focused regression tests. No data paths, state mutations, or existing behaviors are altered.

No files require special attention.

Important Files Changed

Filename Overview
packages/cli/src/commands/scale.ts Adds four exported parser functions and wires them to all numeric CLI options; parsers correctly guard empty/whitespace, floats, non-safe integers, non-finite values, and out-of-range percentages.
packages/cli/src/commands/scale.test.ts Imports and tests all four new parsers with focused it.each suites; covers valid accepts and key reject cases including empty string, fractions, zero, negatives, Infinity, NaN, and out-of-range percentages.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CLI argument string] --> B{Empty or whitespace?}
    B -- yes --> ERR[InvalidArgumentError thrown]
    B -- no --> C[Number coercion]
    C --> D{Which parser?}
    D -- parsePositiveInteger --> E{isSafeInteger AND gte 1?}
    E -- no --> ERR
    E -- yes --> OUT[Value returned to Commander]
    D -- parseNonNegativeInteger --> F{isSafeInteger AND gte 0?}
    F -- no --> ERR
    F -- yes --> OUT
    D -- parsePositiveNumber --> G{isFinite AND gt 0?}
    G -- no --> ERR
    G -- yes --> OUT
    D -- parsePercentage --> H[calls parsePositiveInteger]
    H --> I{parsed gt 100?}
    I -- yes --> ERR
    I -- no --> OUT
    OUT --> J[Action handler receives clean value]
Loading

Reviews (3): Last reviewed commit: "fix(cli): validate rollout percent and e..." | Re-trigger Greptile

Comment thread packages/cli/src/commands/scale.ts Outdated
Comment thread packages/cli/src/commands/scale.ts
Comment thread packages/cli/src/commands/scale.test.ts Outdated
@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@lazyGPT07 lazyGPT07 force-pushed the fix/scale-reject-invalid-numbers branch from 51db023 to 8583181 Compare June 14, 2026 03:05
@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

11 similar comments
@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@ralyodio ralyodio merged commit b794cf5 into profullstack:master Jun 14, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scale numeric options accept NaN, Infinity, and fractional counts

2 participants