Skip to content

feat: improve create TUI model setup - #2164

Merged
notgitika merged 1 commit into
refactorfrom
feat/project-create-tui-model-selection
Sep 2, 2026
Merged

feat: improve create TUI model setup#2164
notgitika merged 1 commit into
refactorfrom
feat/project-create-tui-model-selection

Conversation

@notgitika

@notgitika notgitika commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Improves the project create TUI model selection and spacing.

  • Added provider selection for Bedrock, OpenAI, Gemini, and LiteLLM.
  • Kept provider-specific model settings together on one page.
  • Reused the existing API key ARN input for supported providers.
  • Moved recommended runtime and memory options to the top.
  • Fixed radio alignment, review-page spacing, progress-spinner spacing, and small-terminal layout.
  • Added tests for model selection, navigation, validation, and layout.

Tested the TUI manually

@github-actions github-actions Bot added the size/l PR size: L label Sep 1, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Sep 1, 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 Sep 1, 2026
@codecov-commenter

codecov-commenter commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.63296% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.22%. Comparing base (8dc1051) to head (e2a6f99).
⚠️ Report is 13 commits behind head on refactor.

Files with missing lines Patch % Lines
src/handlers/project/create/screen.tsx 93.20% 17 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           refactor    #2164      +/-   ##
============================================
- Coverage     97.25%   97.22%   -0.04%     
============================================
  Files           508      508              
  Lines         33902    34115     +213     
============================================
+ Hits          32972    33168     +196     
- Misses          930      947      +17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@notgitika notgitika changed the title feat(project): improve create TUI model setup feat: improve create TUI model setup Sep 1, 2026
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Sep 1, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AgentCore Harness Review

Verdict: Looks good

Nice work. I traced the credential-name plumbing end-to-end and everything hangs together:

  • The wizard's per-provider configs map means switching providers no longer clobbers what the user typed for another provider, and the API key is password-masked in FormTextInput + stripped from the review summary (verified by the new tests).
  • credentialFor() correctly returns undefined for bedrock and for an empty-key LiteLLM, and throws only for openai/gemini with a missing key — a state the model step's onNext won't allow the user to reach, so summaryOf's render-time call is safe.
  • The credentialName → placeholder apiKeyArn: "credential:<name>" swap in src/assets/cdk/bin/cdk.ts cleanly avoids widening the pinned CDK schema, and AgentCoreStack re-parses with the resolved (deployed or deterministic same-account) ARN before it reaches AgentCoreApplication, so AgentCoreHarnessRole's apiKeyArn.split('/').pop() derives the right credential name for the IAM policy.
  • Schema change in projectSchemas/harness.ts makes apiKeyArn and credentialName mutually exclusive and keeps apiKeyArn-or-credentialName required for openai/gemini — with tests covering both branches. export.ts correctly prefers the explicit credentialName over the ARN-parsed fallback and reuses an already-declared project credential.
  • Tests use real temp dirs and TestCoreClient — no fs / manager mocking creeping in.

No changes requested from me.

@agentcore-devx-automation agentcore-devx-automation Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Sep 1, 2026
@notgitika
notgitika force-pushed the feat/project-create-tui-model-selection branch from 9480ccb to e2a6f99 Compare September 1, 2026 23:20
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Sep 1, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 1, 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 Sep 1, 2026
@notgitika
notgitika marked this pull request as ready for review September 1, 2026 23:26
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Sep 1, 2026
@notgitika
notgitika merged commit 588eb90 into refactor Sep 2, 2026
26 of 29 checks passed
@notgitika
notgitika deleted the feat/project-create-tui-model-selection branch September 2, 2026 03:03
aidandaly24 added a commit to aidandaly24/agentcore-cli that referenced this pull request Sep 2, 2026
aws#2163 made ProjectEvent a discriminated union and updated the create wizard
along with two of the three mocks in this file. The third belongs to a test
aws#2164 added 112 seconds earlier, so aws#2163's branch could not see it: both PRs
were green alone and only the merged pair fails typecheck and the spinner test.

Same one-line change aws#2163 already made twice here.
This was referenced Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/l PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants