Skip to content

fix(unit-only): Thread the picker selection through preflight AND scope t... (#1267)#60

Draft
aidandaly24 wants to merge 1 commit into
mainfrom
fix/1267
Draft

fix(unit-only): Thread the picker selection through preflight AND scope t... (#1267)#60
aidandaly24 wants to merge 1 commit into
mainfrom
fix/1267

Conversation

@aidandaly24

Copy link
Copy Markdown
Owner

Refs aws#1267

Issues

  • All targets are shown even if only 1 was selected aws/agentcore-cli#1267 — In the TUI deploy flow, when aws-targets.json has more than one target the multi-select picker lets the user choose a subset, but the choice is dropped: the deploy screen header lists every target (cosmetic), and far worse, the deploy itself runs against ALL synthesized stacks (all targets), not the selected one(s). A user who picks a single target silently deploys real infrastructure to every configured account/region. Only affects multi-target projects; single-target (the default) is correct.

Root cause

The TUI target picker (useAwsTargetConfig.ts) records the user's choice in selectedTargetIndices (set by confirmTargetSelection at useAwsTargetConfig.ts:218-222 / selectAllTargets at :202-206), but nothing downstream consumes it. useDeployFlow -> useCdkPreflight calls validateProject() (preflight.ts:66), which unconditionally loads every target via configIO.resolveAWSDeploymentTargets() (preflight.ts:78) into context.awsTargets (preflight.ts:146). Two consequences: (1) Display - DeployScreen.tsx:279 builds targetDisplay from context.awsTargets.map(...), showing all targets. (2) Deploy scope - useDeployFlow.ts:784 calls cdkToolkitWrapper.deploy() with no stacks selector; wrapper.ts:256 forwards { stacks: undefined } to toolkit.deploy, and toolkit-lib's stacksOpt (node_modules/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.js:1592-1593) defaults undefined to ALL_STACKS. The vended CDK app synthesizes one stack per target (src/assets/cdk/bin/cdk.ts:124-190), so ALL_STACKS == deploy every target. CLI mode is immune because actions.ts:124-126 scopes deploy() with StackSelectionStrategy.PATTERN_MUST_MATCH + [toStackName(project,target)]. Post-deploy keying also assumes a single target via context.awsTargets[0] (persistDeployedState useDeployFlow.ts:267; teardown :805,812; transaction-search :844-845) and checkBootstrapNeeded at preflight.ts:307. Confirmed not fixed at v0.20.2 (HEAD e92c79a): grep for selectedTargets in the deploy screens/useCdkPreflight returns nothing.

The fix

Thread the picker selection through preflight AND scope the deploy. (1) Display + context (what open PR aws#1302 does): add selectedTargets?: AwsDeploymentTarget[] to PreflightOptions/DeployFlowOptions; DeployScreen derives it from awsConfig.selectedTargetIndices over availableTargets and passes it down; useCdkPreflight filters preflightContext.awsTargets to the selected names after validateProject(). This fixes the header (DeployScreen.tsx:279) and makes awsTargets[0] reads (persist/teardown/transaction-search; checkBootstrapNeeded preflight.ts:307) reflect the user's choice. (2) ADDITIONALLY (NOT in PR aws#1302, and the load-bearing part): scope the actual deploy by passing a stacks selector into the TUI cdkToolkitWrapper.deploy() at useDeployFlow.ts:784, built from the selected target names mapped through toStackName(projectName, targetName) with StackSelectionStrategy.PATTERN_MUST_MATCH, exactly as CLI mode does at actions.ts:124-126. Note: for a true multi-target deploy the post-deploy persist/teardown/transaction-search loop also needs to iterate awsTargets rather than reading [0]; PR aws#1302 only narrows the array so its [0] reads happen to be correct for the common single-selection case but would still mis-handle a genuine multi-select. Without step (2) the deploy keeps applying ALL_STACKS regardless of selection. PR aws#1302's claim that it fully fixes the silent wrong-target deploy is overstated; as-is it fixes only display + state keying.

Files touched: src/cli/tui/screens/deploy/DeployScreen.tsx (derive+pass selectedTargets; header line 279); src/cli/tui/screens/deploy/useDeployFlow.ts (accept selectedTargets; ADD stacks selector to cdkToolkitWrapper.deploy() at line 784; ideally loop awsTargets for persist/teardown/transaction-search at :267,805,812,844-845); src/cli/tui/hooks/useCdkPreflight.ts (filter preflightContext.awsTargets). Display+context portion covered by open PR aws#1302 (3 files, +48/-6).

Validation evidence

The fix was verified by reproducing the original symptom and re-running after the change:

Reproduced original symptom by stashing the fix: pre-fix useDeployFlow.ts:783 calls await cdkToolkitWrapper.deploy(); with NO args. Wrote a faithful unit test (src/cli/tui/screens/deploy/tests/useDeployFlow.targets.test.tsx) that mocks useCdkPreflight to return phase 'complete' + a spy-backed cdkToolkitWrapper + a 2-target context, renders useDeployFlow via ink-testing-library so the deploy effect fires, then inspects deploySpy.mock.calls[0][0]. BEFORE fix: all 5 tests FAIL with TypeError: Cannot read properties of undefined (reading 'stacks') — proving deploy() is called with zero args -> wrapper forwards { stacks: undefined } -> toolkit-lib defaults to ALL_STACKS -> every configured account/region deployed regardless of picker (exact symptom). AFTER fix (useDeployFlow.ts:810 now deploy({ stacks: deployStacks })): all 5 PASS — selecting target B yields { stacks: { strategy: PATTERN_MUST_MATCH, patterns: [toStackName('myproj','prod-west')] } } and never A's stack; selecting A scopes only to A (no cross-leak); selecting both targets covers both stack names; single-target project still deploys exactly one stack; empty selection falls back to undefined (unscoped assembly). Restored fix via git stash pop and re-confirmed green.

Test suite: green.


Staged on the fork as a draft for human review. Promote to aws/agentcore-cli after vetting.

The TUI multi-select target picker records the user's choice in
selectedTargetIndices, but nothing downstream consumed it: useDeployFlow
called cdkToolkitWrapper.deploy() with no stacks selector, so toolkit-lib
defaulted to ALL_STACKS and deployed every synthesized stack (one per
configured target). A user who picked a single target silently deployed
infrastructure to every configured account/region.

Thread selectedTargets from DeployScreen through useDeployFlow and scope
the deploy with StackSelectionStrategy.PATTERN_MUST_MATCH over
toStackName(project, target), mirroring CLI mode. Also fix the header to
show the picked target(s) rather than the full configured list.

Refs aws#1267
@github-actions github-actions Bot added the size/m PR size: M label Jun 25, 2026
@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 37.71% 13799 / 36586
🔵 Statements 36.97% 14670 / 39680
🔵 Functions 32.1% 2357 / 7341
🔵 Branches 31.51% 9122 / 28945
Generated in workflow #114 for commit c28ea67 by the Vitest Coverage Report Action

@github-actions github-actions Bot added agentcore-harness-reviewing AgentCore Harness review in progress and removed agentcore-harness-reviewing AgentCore Harness review in progress labels Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant