Skip to content

fix(tui): validate empty input in SecretInput before triggering cancel#1668

Open
notgitika wants to merge 1 commit into
mainfrom
fix/secret-input-empty-validation-v2
Open

fix(tui): validate empty input in SecretInput before triggering cancel#1668
notgitika wants to merge 1 commit into
mainfrom
fix/secret-input-empty-validation-v2

Conversation

@notgitika

@notgitika notgitika commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

This PR fixes the "add credential" TUI flow going back to the previous step when pressing Enter with no API key input

Fixes #1618

Previously, pressing Enter with an empty value in SecretInput would
call onCancel (which navigates back) even when customValidation
rejected empty values. Now validation runs first, showing the error
message instead of silently going back.

Fixes #1618
@notgitika notgitika requested a review from a team June 29, 2026 23:17
@github-actions github-actions Bot added the size/s PR size: S label Jun 29, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 29, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 29, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.21.1.tgz

How to install

gh release download pr-1668-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.21.1.tgz

@agentcore-cli-automation agentcore-cli-automation 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.

Looks good to merge.

Verified the fix:

  • Reordering validateValue so customValidation runs before the !value early-return is necessary for the new empty-submit path to surface validation errors, and it doesn't change render behavior — showCheckmark/showInvalidMark are still gated by hasInput, and the error block is still gated by showError/showInvalidMark.
  • Existing behavior is preserved: empty Enter with no customValidation still falls through to onSkip/onCancel; schema-only validators (which return undefined for empty) are unaffected.
  • The new test exercises the real component via ink-testing-library (no excessive mocking) and asserts on both the side effects (callbacks not invoked) and rendered output.
  • No new feature surface, so no telemetry instrumentation needed.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 38.03% 14092 / 37054
🔵 Statements 37.31% 15007 / 40213
🔵 Functions 32.66% 2423 / 7417
🔵 Branches 31.91% 9377 / 29381
Generated in workflow #3923 for commit 368134f by the Vitest Coverage Report Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add credential TUI flow simply goes back to the previous step if there is no input at the API key step

3 participants