From c28ea6718bb354de102630d886e0a4f6f6691c19 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 25 Jun 2026 06:13:49 +0000 Subject: [PATCH] fix(unit-only): scope TUI deploy to picker-selected targets (#1267) 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/agentcore-cli#1267 --- src/cli/tui/screens/deploy/DeployScreen.tsx | 13 +- .../__tests__/useDeployFlow.targets.test.tsx | 217 ++++++++++++++++++ src/cli/tui/screens/deploy/useDeployFlow.ts | 34 ++- 3 files changed, 259 insertions(+), 5 deletions(-) create mode 100644 src/cli/tui/screens/deploy/__tests__/useDeployFlow.targets.test.tsx diff --git a/src/cli/tui/screens/deploy/DeployScreen.tsx b/src/cli/tui/screens/deploy/DeployScreen.tsx index 1c7a9c65e..e033bb9fa 100644 --- a/src/cli/tui/screens/deploy/DeployScreen.tsx +++ b/src/cli/tui/screens/deploy/DeployScreen.tsx @@ -59,6 +59,12 @@ export function DeployScreen({ }: DeployScreenProps) { const { stdout } = useStdout(); const awsConfig = useAwsTargetConfig(); + // Targets the user picked in the multi-select. Drives both the header and the deploy scope so a + // single-target selection no longer deploys to every configured account/region (issue #1267). + const selectedTargets = useMemo( + () => awsConfig.selectedTargetIndices.map(i => awsConfig.availableTargets[i]).filter(t => t !== undefined), + [awsConfig.selectedTargetIndices, awsConfig.availableTargets] + ); const [showInvoke, setShowInvoke] = useState(false); const [showResourceGraph, setShowResourceGraph] = useState(false); const [showDiff, setShowDiff] = useState(diffMode ?? false); @@ -98,7 +104,7 @@ export function DeployScreen({ useEnvLocalCredentials, useManualCredentials, skipCredentials, - } = useDeployFlow({ preSynthesized, isInteractive, diffMode }); + } = useDeployFlow({ preSynthesized, isInteractive, diffMode, selectedTargets }); const allSuccess = !hasError && isComplete; const skipPreflight = !!preSynthesized; @@ -276,7 +282,10 @@ export function DeployScreen({ ); } - const targetDisplay = context?.awsTargets.map(t => `${t.region}:${t.account}`).join(', '); + // Show the target(s) the user actually picked, not the full configured list. Fall back to the + // resolved context targets for the plan path (no picker) or when nothing was selected. + const displayTargets = selectedTargets.length > 0 ? selectedTargets : (context?.awsTargets ?? []); + const targetDisplay = context && displayTargets.map(t => `${t.region}:${t.account}`).join(', '); // Show deploy status box once CloudFormation has started (after asset publishing) const showDeployStatus = !diffMode && (hasStartedCfn || isComplete); diff --git a/src/cli/tui/screens/deploy/__tests__/useDeployFlow.targets.test.tsx b/src/cli/tui/screens/deploy/__tests__/useDeployFlow.targets.test.tsx new file mode 100644 index 000000000..3be14e49b --- /dev/null +++ b/src/cli/tui/screens/deploy/__tests__/useDeployFlow.targets.test.tsx @@ -0,0 +1,217 @@ +/** + * Regression test for issue #1267. + * + * In the TUI deploy flow a multi-target project synthesizes one stack per configured target. + * The deploy was calling `cdkToolkitWrapper.deploy()` with no stacks selector, which the wrapper + * forwards as `{ stacks: undefined }` — toolkit-lib defaults that to ALL_STACKS and provisions + * infrastructure to EVERY configured account/region, ignoring the picker selection. + * + * These tests render `useDeployFlow` with a mocked preflight + CDK wrapper and assert the deploy + * is scoped to ONLY the selected target's stack name via PATTERN_MUST_MATCH. + */ +import { toStackName } from '../../../../commands/import/import-utils'; +import { useDeployFlow } from '../useDeployFlow'; +import { StackSelectionStrategy } from '@aws-cdk/toolkit-lib'; +import { render } from 'ink-testing-library'; +import React from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// ---- Module mocks ----------------------------------------------------------- + +const deploySpy = vi.fn().mockResolvedValue(undefined); +const diffSpy = vi.fn().mockResolvedValue(undefined); +const disposeSpy = vi.fn().mockResolvedValue(undefined); + +const fakeWrapper = { + deploy: deploySpy, + diff: diffSpy, + dispose: disposeSpy, +}; + +// switchableIoHost: noop setters so the deploy effect can wire callbacks without crashing. +const fakeIoHost = { + setOnRawMessage: vi.fn(), + setOnMessage: vi.fn(), + setVerbose: vi.fn(), +}; + +// preflightState is mutated per-test before render so the same mock can vary phase/context. +let preflightState: any; + +vi.mock('../../../hooks', async () => { + const actual = await vi.importActual('../../../hooks'); + return { + ...actual, + useCdkPreflight: () => preflightState, + }; +}); + +// Telemetry wrapper: just run the deploy closure so deploy() actually fires. +vi.mock('../../../../telemetry/cli-command-run.js', () => ({ + withCommandRunTelemetry: async (_cmd: string, _attrs: unknown, fn: () => Promise) => fn(), +})); + +// persistDeployedState polls getStackOutputs; make it throw fast so the (caught) post-deploy path +// resolves immediately instead of polling for 15s. The deploy() call has already happened by then. +vi.mock('../../../../cloudformation', async () => { + const actual = await vi.importActual('../../../../cloudformation'); + return { + ...actual, + getStackOutputs: vi.fn().mockRejectedValue(new Error('test: skip persist')), + }; +}); + +// Managed-memory notice does a config read; short-circuit it. +vi.mock('../../../../operations/deploy', async () => { + const actual = await vi.importActual('../../../../operations/deploy'); + return { + ...actual, + hasManagedMemoryHarness: vi.fn().mockResolvedValue(false), + setupTransactionSearch: vi.fn().mockResolvedValue({ success: true }), + }; +}); + +// ---- Fixtures --------------------------------------------------------------- + +const TARGET_A = { name: 'prod-east', account: '111111111111', region: 'us-east-1' as const }; +const TARGET_B = { name: 'prod-west', account: '222222222222', region: 'us-west-2' as const }; +const PROJECT_NAME = 'myproj'; + +function makePreflight(opts: { awsTargets: any[]; projectName?: string }) { + const stackNames = opts.awsTargets.map(t => toStackName(opts.projectName ?? PROJECT_NAME, t.name)); + return { + phase: 'complete', + steps: [], + context: { + projectSpec: { name: opts.projectName ?? PROJECT_NAME, runtimes: [] }, + awsTargets: opts.awsTargets, + isTeardownDeploy: false, + isFirstDeploy: true, // skip the pre-deploy diff branch + }, + cdkToolkitWrapper: fakeWrapper, + stackNames, + switchableIoHost: fakeIoHost, + hasTokenExpiredError: false, + hasCredentialsError: false, + missingCredentials: [], + allCredentials: {}, + identityKmsKeyArn: undefined, + startPreflight: vi.fn().mockResolvedValue(undefined), + confirmTeardown: vi.fn(), + cancelTeardown: vi.fn(), + confirmBootstrap: vi.fn(), + skipBootstrap: vi.fn(), + clearTokenExpiredError: vi.fn(), + clearCredentialsError: vi.fn(), + useEnvLocalCredentials: vi.fn(), + useManualCredentials: vi.fn(), + skipCredentials: vi.fn(), + }; +} + +// Harness component: mounts the hook so its effects fire under ink-testing-library. +function Harness({ selectedTargets }: { selectedTargets?: any[] }) { + useDeployFlow({ isInteractive: true, selectedTargets }); + return null as unknown as React.ReactElement; +} + +async function flush() { + // Let queued microtasks + effect timers settle. + for (let i = 0; i < 10; i += 1) { + await new Promise(resolve => setTimeout(resolve, 5)); + } +} + +// ---- Tests ------------------------------------------------------------------ + +describe('useDeployFlow target scoping (issue #1267)', () => { + beforeEach(() => { + deploySpy.mockClear(); + diffSpy.mockClear(); + disposeSpy.mockClear(); + fakeIoHost.setOnRawMessage.mockClear(); + fakeIoHost.setOnMessage.mockClear(); + fakeIoHost.setVerbose.mockClear(); + }); + afterEach(() => { + vi.clearAllTimers(); + }); + + it('scopes deploy to ONLY the single selected target stack (not ALL_STACKS)', async () => { + preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] }); + + const { unmount } = render(); + await flush(); + unmount(); + + expect(deploySpy).toHaveBeenCalledTimes(1); + const arg = deploySpy.mock.calls[0][0]; + + // Must NOT be called with stacks undefined (the pre-fix behavior → ALL_STACKS). + expect(arg).toBeDefined(); + expect(arg.stacks).toBeDefined(); + expect(arg.stacks.strategy).toBe(StackSelectionStrategy.PATTERN_MUST_MATCH); + expect(arg.stacks.patterns).toEqual([toStackName(PROJECT_NAME, TARGET_B.name)]); + + // Regression: selecting B must never produce a pattern for A. + expect(arg.stacks.patterns).not.toContain(toStackName(PROJECT_NAME, TARGET_A.name)); + }); + + it('selecting target A produces only A’s stack (no cross-leak to B)', async () => { + preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] }); + + const { unmount } = render(); + await flush(); + unmount(); + + expect(deploySpy).toHaveBeenCalledTimes(1); + const arg = deploySpy.mock.calls[0][0]; + expect(arg.stacks.patterns).toEqual([toStackName(PROJECT_NAME, TARGET_A.name)]); + expect(arg.stacks.patterns).not.toContain(toStackName(PROJECT_NAME, TARGET_B.name)); + }); + + it('selecting ALL targets yields a selector covering every target stack', async () => { + preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] }); + + const { unmount } = render(); + await flush(); + unmount(); + + expect(deploySpy).toHaveBeenCalledTimes(1); + const arg = deploySpy.mock.calls[0][0]; + expect(arg.stacks.strategy).toBe(StackSelectionStrategy.PATTERN_MUST_MATCH); + expect(arg.stacks.patterns).toEqual([ + toStackName(PROJECT_NAME, TARGET_A.name), + toStackName(PROJECT_NAME, TARGET_B.name), + ]); + }); + + it('single-target project still deploys (unscoped → the lone synthesized stack)', async () => { + preflightState = makePreflight({ awsTargets: [TARGET_A] }); + + // Single-target picker selects the one target. + const { unmount } = render(); + await flush(); + unmount(); + + expect(deploySpy).toHaveBeenCalledTimes(1); + const arg = deploySpy.mock.calls[0][0]; + // Either a pattern selector for the single stack, or undefined (assembly's lone stack) — both + // deploy exactly one stack. Must never resolve to a broader set. + if (arg.stacks) { + expect(arg.stacks.patterns).toEqual([toStackName(PROJECT_NAME, TARGET_A.name)]); + } + }); + + it('no picker selection falls back to the unscoped assembly (undefined selector)', async () => { + preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] }); + + const { unmount } = render(); + await flush(); + unmount(); + + expect(deploySpy).toHaveBeenCalledTimes(1); + const arg = deploySpy.mock.calls[0][0]; + expect(arg.stacks).toBeUndefined(); + }); +}); diff --git a/src/cli/tui/screens/deploy/useDeployFlow.ts b/src/cli/tui/screens/deploy/useDeployFlow.ts index 9b2ebd719..6527b40b3 100644 --- a/src/cli/tui/screens/deploy/useDeployFlow.ts +++ b/src/cli/tui/screens/deploy/useDeployFlow.ts @@ -1,5 +1,7 @@ import { ConfigIO } from '../../../../lib'; +import type { AwsDeploymentTarget } from '../../../../schema'; import type { CdkToolkitWrapper, DeployMessage, SwitchableIoHost } from '../../../cdk/toolkit-lib'; +import { toStackName } from '../../../commands/import/import-utils'; import { buildDeployedState, getStackOutputs, @@ -43,6 +45,7 @@ import { parseStackDiff, } from '../../components'; import { type MissingCredential, type PreflightContext, useCdkPreflight } from '../../hooks'; +import { StackSelectionStrategy } from '@aws-cdk/toolkit-lib'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; type DeployPhase = @@ -75,6 +78,13 @@ interface DeployFlowOptions { isInteractive?: boolean; /** Run CDK diff instead of deploy */ diffMode?: boolean; + /** + * Targets the user chose in the multi-select picker. The vended CDK app synthesizes one stack + * per configured target, so without scoping the deploy runs against ALL_STACKS (every target). + * When set, the deploy is restricted to these targets' stacks. Empty/undefined falls back to the + * full assembly (single-target projects, and the pre-synthesized plan path). + */ + selectedTargets?: AwsDeploymentTarget[]; } interface DeployFlowState { @@ -130,7 +140,7 @@ interface DeployFlowState { } export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState { - const { preSynthesized, isInteractive = false, diffMode = false } = options; + const { preSynthesized, isInteractive = false, diffMode = false, selectedTargets } = options; const skipPreflight = !!preSynthesized; // Create logger once for the entire deploy flow @@ -147,6 +157,21 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState const identityKmsKeyArn = preSynthesized?.identityKmsKeyArn ?? preflight.identityKmsKeyArn; const allCredentials = preSynthesized?.allCredentials ?? preflight.allCredentials; + // Scope the deploy to the picker's selected targets. The vended CDK app synthesizes one stack + // per target, so an unscoped deploy resolves to ALL_STACKS and provisions every configured + // account/region — even the ones the user did not pick (see issue #1267). Mirrors CLI mode + // (commands/deploy/actions.ts), which patterns deploy() by toStackName(project, target). + // Skipped on the pre-synthesized plan path, which already targets a single stack. + const deployStacks = useMemo(() => { + if (skipPreflight) return undefined; + const projectName = context?.projectSpec.name; + if (!projectName || !selectedTargets || selectedTargets.length === 0) return undefined; + return { + strategy: StackSelectionStrategy.PATTERN_MUST_MATCH, + patterns: selectedTargets.map(t => toStackName(projectName, t.name)), + }; + }, [skipPreflight, context?.projectSpec.name, selectedTargets]); + const [preDeployDiffStep, setPreDeployDiffStep] = useState({ label: 'Computing diff changes...', status: 'pending', @@ -779,8 +804,10 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState try { // Run deploy - toolkit-lib handles CloudFormation orchestration - // Output goes to stdout via the switchable ioHost - await cdkToolkitWrapper.deploy(); + // Output goes to stdout via the switchable ioHost. + // deployStacks restricts the deploy to the picker's selected targets; undefined (single + // target or plan path) lets the assembly's lone stack deploy as before. + await cdkToolkitWrapper.deploy({ stacks: deployStacks }); // CDK deploy itself is done. Mark "Deploy to AWS" success and let post-deploy // phases (persist, hydrate KBs, auto-ingest, dataset sync, online evals, @@ -934,6 +961,7 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState context?.awsTargets, context?.projectSpec.runtimes, diffMode, + deployStacks, ]); // Start diff when preflight completes (diff mode only)