fix(wizard): retry Kiro API key prompt on invalid format (do not merge yet) - #91
fix(wizard): retry Kiro API key prompt on invalid format (do not merge yet)#91royosherove wants to merge 2 commits into
Conversation
…kipping Previous behavior: user typos ksk_... key, wizard warns 'API key doesn't match expected format', silently skips the whole step, and user has to authenticate manually after install. New behavior: wrap the prompt in a retry loop. - Valid ksk_... key -> accept, break - Empty input (Enter) -> skip (existing convention) - Literal 'skip' typed (case-insensitive) -> skip - Invalid format -> warn with remaining attempt count and re-prompt - After 5 attempts -> warn and skip (avoid infinite loop) Also: gum prompt cancellation (Esc / Ctrl-C) returns non-zero and prompt_secret falls back to the empty default -> treated as skip, so 'press Esc to move on' works out of the box without extra keyboard plumbing. UX text updated from 'Press Enter to skip' to 'Press Enter (empty input) or type skip to skip and authenticate via browser later.' Verified: - bash -n install.sh: OK - All existing paths (empty, valid, invalid+skip, invalid+retry+valid, invalid+retry-until-exhausted) mapped to distinct branches with clear warn messages.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f96d0f491
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _KIRO_INPUT="" | ||
| prompt_secret "Kiro API key" _KIRO_INPUT "" | ||
| # Skip: empty input, or user typed "skip" (case-insensitive) | ||
| if [[ -z "$_KIRO_INPUT" ]] || [[ "${_KIRO_INPUT,,}" == "skip" ]]; then |
There was a problem hiding this comment.
Use a Bash 3-compatible skip check
On macOS with the stock Bash 3.2, any nonempty response at this Kiro prompt reaches the Bash 4-only ${_KIRO_INPUT,,} expansion and terminates the installer with bad substitution; this includes valid API keys before they can be validated. Since the installer explicitly supports Darwin, use a portable case-insensitive comparison such as a case pattern or tr instead.
Useful? React with 👍 / 👎.
) Codex flagged: ${_KIRO_INPUT,,} is a Bash 4+ feature. On macOS's stock Bash 3.2 this expansion errors with 'bad substitution' — and because the installer runs under 'set -euo pipefail', ANY non-empty API key entry (including valid keys) would kill the whole installer before validation. Fix: swap the case-conversion for a portable tr pipeline, matching what the rest of the installer already uses for Darwin support. Verified: - bash -n install.sh: OK - Skip paths (Enter / 'skip' / 'SKIP' / 'Skip') still work - Valid keys no longer trip on the lowercasing
Old behavior
User types
ksk_...with a typo → wizard printswarn: API key doesn't match expected format→ silently skips the whole step → user has to authenticate via browser after install. No second chance.New behavior
Wrap the prompt in a retry loop.
ksk_...keyskiptyped (case-insensitive)Also updated the on-screen guidance:
Press Enter (empty input) or type 'skip' to skip and authenticate via browser later.Not touched
Verification
bash -n install.sh: OKNot merging per Aug 22 20:34 rule.