Skip to content

fix(pre-download): reject missing option value for tier in pre-download.sh - #6340

Closed
vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/pre-download-missing-tier-arg
Closed

vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/pre-download-missing-tier-arg

Conversation

@vaibhavsrv

Copy link
Copy Markdown
Contributor

Why this matters

In ods/scripts/pre-download.sh, the CLI option parser handles --tier by directly reading $2 and performing shift 2 without checking available argument count. When invoked with --tier at the end of the argument string or without a parameter (e.g., ./pre-download.sh --tier), the script trips under set -u with $2: unbound variable or crashes during shift 2. This leaves users with an unhandled shell error rather than a descriptive usage diagnostic.

This change surgically adds argument count and non-empty validation (if [[ $# -lt 2 || -z "${2:-}" ]]) prior to reading the tier value and shifting. The utility emits Option --tier requires an argument and exits cleanly with status 1 while preserving standard tier selections (nano, edge, pro, cluster).

Validation

  • Baseline reproduction: Running bash ods/scripts/pre-download.sh --tier failed with line 381: $2: unbound variable.
  • Post-fix behavior: Invoking --tier with missing values emits [ERROR] Option --tier requires an argument and terminates with status 1.
  • Telemetry: Pre-download suites: 2 passed. New-test Ruff, ShellCheck, and diff checks pass; new regressions wired into Linux CI.

Overlap check

Recent PRs #6337 (environment variable diff in migrate-config.sh), #6325 (option argument validation in ods-backup.sh), and #5878 (download_model error trapping) address separate scripts or failure pathways. This PR isolates argument validation to ods/scripts/pre-download.sh without conflict.

Risk / AI disclosure

AI-assisted investigation, implementation and CLI regressions. This strengthens a CLI check, not runtime admission. Independent human review and platform/runtime qualification remain gates. No running configuration, deployment or upstream merge changed.

Follow-up integration evidence

Composed with #6337 at 5ad1dd7 without conflicts. Production and test diffs passed together; pre-download and installer checks remain intact.
Backlog composition was local-only (production/test diffs, excluding workflow/Makefile wiring); it is not an upstream merge or independent human approval. Declared live-review gates remain open.

@Lightheartdevs

Copy link
Copy Markdown
Collaborator

Thanks for this contribution. public-beta was promoted into main on 2026-09-24 and no longer receives changes, so we're closing pull requests that target it. This isn't a judgment on the change itself. If it's still needed, please rebase onto main and open a focused PR. See #7253 for details and the contribution policy.

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.

2 participants