Skip to content

fix(wizard): retry Kiro API key prompt on invalid format (do not merge yet) - #91

Open
royosherove wants to merge 2 commits into
mainfrom
fix/wizard-kiro-key-retry
Open

fix(wizard): retry Kiro API key prompt on invalid format (do not merge yet)#91
royosherove wants to merge 2 commits into
mainfrom
fix/wizard-kiro-key-retry

Conversation

@royosherove

Copy link
Copy Markdown
Member

Old behavior

User types ksk_... with a typo → wizard prints warn: 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.

Input Result
Valid ksk_... key Accept, break loop
Empty (Enter) Skip (existing convention)
skip typed (case-insensitive) Skip
Esc / Ctrl-C on the gum prompt Skip (gum returns non-zero → prompt_secret falls back to empty default → treated as skip)
Invalid format Warn with remaining attempt count, re-prompt
5 invalid attempts Warn and skip (avoid infinite loop)

Also updated the on-screen guidance: Press Enter (empty input) or type 'skip' to skip and authenticate via browser later.

Not touched

  • Telegram bot token prompt (line 3358) still fails hard on bad format. That's a separate step, harder to skip conceptually — worth its own follow-up if we want the same UX there.

Verification

  • bash -n install.sh: OK
  • 5 branches (empty, valid, invalid+skip-typed, invalid+retry+valid, invalid×5) all reach a clearly-defined end state.

Not merging per Aug 22 20:34 rule.

…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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread install.sh Outdated
_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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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
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.

1 participant