From ffccd6c0144a582725db3c8e44e114d75adae0f7 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Wed, 22 Apr 2026 18:01:43 -0400 Subject: [PATCH 1/2] fix(deploy): honor aws-targets.json region for all SDK and CDK calls (#924) AWS SDK clients constructed by @aws-cdk/toolkit-lib internally (for CloudFormation, S3 asset upload, etc.) do not receive an explicit region option and fall back to the SDK's default region resolution chain (AWS_REGION -> AWS_DEFAULT_REGION -> shared config). When a user's aws-targets.json specified a non-default region but those env vars were unset, resources were created in the SDK default region instead of the configured target. Promote target.region to AWS_REGION and AWS_DEFAULT_REGION for the lifetime of deploy and teardown operations, restoring prior values in a finally block. This ensures downstream SDK clients (explicit and toolkit-lib internal) agree on the target region. Covers CLI non-interactive deploy (handleDeploy) and the interactive TUI deploy/teardown (useCdkPreflight, destroyTarget). Invoke/status/eval already pass target.region explicitly. --- src/cli/aws/__tests__/target-region.test.ts | 99 +++++++++++++++++++++ src/cli/aws/index.ts | 1 + src/cli/aws/target-region.ts | 55 ++++++++++++ src/cli/commands/deploy/actions.ts | 7 ++ src/cli/operations/deploy/teardown.ts | 19 ++-- src/cli/tui/hooks/useCdkPreflight.ts | 34 ++++++- 6 files changed, 205 insertions(+), 10 deletions(-) create mode 100644 src/cli/aws/__tests__/target-region.test.ts create mode 100644 src/cli/aws/target-region.ts diff --git a/src/cli/aws/__tests__/target-region.test.ts b/src/cli/aws/__tests__/target-region.test.ts new file mode 100644 index 000000000..21ec6c8af --- /dev/null +++ b/src/cli/aws/__tests__/target-region.test.ts @@ -0,0 +1,99 @@ +import { applyTargetRegionToEnv, withTargetRegion } from '../target-region.js'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +describe('target-region', () => { + let savedRegion: string | undefined; + let savedDefaultRegion: string | undefined; + + beforeEach(() => { + savedRegion = process.env.AWS_REGION; + savedDefaultRegion = process.env.AWS_DEFAULT_REGION; + delete process.env.AWS_REGION; + delete process.env.AWS_DEFAULT_REGION; + }); + + afterEach(() => { + if (savedRegion !== undefined) process.env.AWS_REGION = savedRegion; + else delete process.env.AWS_REGION; + if (savedDefaultRegion !== undefined) process.env.AWS_DEFAULT_REGION = savedDefaultRegion; + else delete process.env.AWS_DEFAULT_REGION; + }); + + describe('applyTargetRegionToEnv', () => { + it('sets AWS_REGION and AWS_DEFAULT_REGION to the provided region', () => { + applyTargetRegionToEnv('ap-southeast-2'); + expect(process.env.AWS_REGION).toBe('ap-southeast-2'); + expect(process.env.AWS_DEFAULT_REGION).toBe('ap-southeast-2'); + }); + + it('returns a restore function that clears env vars when they were previously unset', () => { + const restore = applyTargetRegionToEnv('eu-west-1'); + restore(); + expect(process.env.AWS_REGION).toBeUndefined(); + expect(process.env.AWS_DEFAULT_REGION).toBeUndefined(); + }); + + it('returns a restore function that restores previous env var values', () => { + process.env.AWS_REGION = 'us-east-1'; + process.env.AWS_DEFAULT_REGION = 'us-east-1'; + + const restore = applyTargetRegionToEnv('ap-south-1'); + expect(process.env.AWS_REGION).toBe('ap-south-1'); + expect(process.env.AWS_DEFAULT_REGION).toBe('ap-south-1'); + + restore(); + expect(process.env.AWS_REGION).toBe('us-east-1'); + expect(process.env.AWS_DEFAULT_REGION).toBe('us-east-1'); + }); + + it('restores each env var independently (only one was previously set)', () => { + process.env.AWS_REGION = 'us-west-2'; + // AWS_DEFAULT_REGION intentionally left unset + + const restore = applyTargetRegionToEnv('eu-central-1'); + expect(process.env.AWS_REGION).toBe('eu-central-1'); + expect(process.env.AWS_DEFAULT_REGION).toBe('eu-central-1'); + + restore(); + expect(process.env.AWS_REGION).toBe('us-west-2'); + expect(process.env.AWS_DEFAULT_REGION).toBeUndefined(); + }); + }); + + describe('withTargetRegion', () => { + it('applies region inside the callback and restores afterwards', async () => { + let seenRegion: string | undefined; + let seenDefaultRegion: string | undefined; + + await withTargetRegion('ap-northeast-1', () => { + seenRegion = process.env.AWS_REGION; + seenDefaultRegion = process.env.AWS_DEFAULT_REGION; + return Promise.resolve(); + }); + + expect(seenRegion).toBe('ap-northeast-1'); + expect(seenDefaultRegion).toBe('ap-northeast-1'); + expect(process.env.AWS_REGION).toBeUndefined(); + expect(process.env.AWS_DEFAULT_REGION).toBeUndefined(); + }); + + it('restores env vars even when the callback throws', async () => { + process.env.AWS_REGION = 'us-east-1'; + + await expect( + withTargetRegion('sa-east-1', () => { + expect(process.env.AWS_REGION).toBe('sa-east-1'); + return Promise.reject(new Error('boom')); + }) + ).rejects.toThrow('boom'); + + expect(process.env.AWS_REGION).toBe('us-east-1'); + expect(process.env.AWS_DEFAULT_REGION).toBeUndefined(); + }); + + it('returns the callback result', async () => { + const result = await withTargetRegion('eu-west-2', () => Promise.resolve(42)); + expect(result).toBe(42); + }); + }); +}); diff --git a/src/cli/aws/index.ts b/src/cli/aws/index.ts index 7ff2a5e28..a60c73da0 100644 --- a/src/cli/aws/index.ts +++ b/src/cli/aws/index.ts @@ -1,6 +1,7 @@ export { detectAwsContext, type AwsContext } from './aws-context'; export { detectAccount, getCredentialProvider } from './account'; export { detectRegion, type RegionDetectionResult } from './region'; +export { withTargetRegion } from './target-region'; export { invokeBedrockSync, invokeClaude, diff --git a/src/cli/aws/target-region.ts b/src/cli/aws/target-region.ts new file mode 100644 index 000000000..b5d9f837a --- /dev/null +++ b/src/cli/aws/target-region.ts @@ -0,0 +1,55 @@ +/** + * Make a deployment target's region authoritative for downstream AWS SDK calls. + * + * The AWS SDK (and CDK toolkit-lib's internal clients) resolve region from + * AWS_REGION / AWS_DEFAULT_REGION when constructed without an explicit `region` + * option. aws-targets.json is the user's source of truth for where resources + * should be created, so we promote the target's region onto the environment for + * the operation and restore any prior values afterwards. + * + * Without this override, a user with a non-default region in aws-targets.json + * but no AWS_DEFAULT_REGION set would see resources created in the SDK's default + * region — see https://github.com/aws/agentcore-cli/issues/924. + */ + +type RestoreEnv = () => void; + +/** + * Set AWS_REGION / AWS_DEFAULT_REGION to `region` and return a restore function. + * Callers that cannot wrap their work in a callback (e.g. CLI entrypoints that + * span many helpers) should use this, and invoke the returned function in a + * `finally` block. + */ +export function applyTargetRegionToEnv(region: string): RestoreEnv { + const prevRegion = process.env.AWS_REGION; + const prevDefaultRegion = process.env.AWS_DEFAULT_REGION; + + process.env.AWS_REGION = region; + process.env.AWS_DEFAULT_REGION = region; + + return () => { + if (prevRegion === undefined) { + delete process.env.AWS_REGION; + } else { + process.env.AWS_REGION = prevRegion; + } + if (prevDefaultRegion === undefined) { + delete process.env.AWS_DEFAULT_REGION; + } else { + process.env.AWS_DEFAULT_REGION = prevDefaultRegion; + } + }; +} + +/** + * Run `fn` with `region` applied to AWS_REGION / AWS_DEFAULT_REGION, restoring + * the prior values on return (including when `fn` throws). + */ +export async function withTargetRegion(region: string, fn: () => Promise): Promise { + const restore = applyTargetRegionToEnv(region); + try { + return await fn(); + } finally { + restore(); + } +} diff --git a/src/cli/commands/deploy/actions.ts b/src/cli/commands/deploy/actions.ts index 2ae8cf0e9..fda7d1de5 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -1,6 +1,7 @@ import { ConfigIO, SecureCredentials } from '../../../lib'; import type { AgentCoreMcpSpec, DeployedState } from '../../../schema'; import { validateAwsCredentials } from '../../aws/account'; +import { applyTargetRegionToEnv } from '../../aws/target-region'; import { createSwitchableIoHost } from '../../cdk/toolkit-lib'; import { buildDeployedState, @@ -48,6 +49,7 @@ const MEMORY_ONLY_NEXT_STEPS = ['agentcore add agent', 'agentcore status']; export async function handleDeploy(options: ValidatedDeployOptions): Promise { let toolkitWrapper = null; + let restoreEnv: (() => void) | null = null; const logger = new ExecLogger({ command: 'deploy' }); const { onProgress } = options; let currentStepName = ''; @@ -79,6 +81,10 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise { + await toolkit.initialize(); + await toolkit.destroy({ + stacks: { + strategy: StackSelectionStrategy.PATTERN_MUST_MATCH, + patterns: [target.stack.stackName], + }, + }); }); // Clean up deployed-state.json after successful destroy diff --git a/src/cli/tui/hooks/useCdkPreflight.ts b/src/cli/tui/hooks/useCdkPreflight.ts index d755d0235..e1aea4b43 100644 --- a/src/cli/tui/hooks/useCdkPreflight.ts +++ b/src/cli/tui/hooks/useCdkPreflight.ts @@ -1,6 +1,7 @@ import { ConfigIO, SecureCredentials } from '../../../lib'; import type { DeployedState } from '../../../schema'; import { AwsCredentialsError, validateAwsCredentials } from '../../aws/account'; +import { applyTargetRegionToEnv } from '../../aws/target-region'; import { type CdkToolkitWrapper, type SwitchableIoHost, createSwitchableIoHost } from '../../cdk/toolkit-lib'; import { getErrorMessage, isExpiredTokenError, isNoCredentialsError } from '../../errors'; import type { ExecLogger } from '../../logging'; @@ -137,6 +138,11 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { const isRunningRef = useRef(false); // Keep a ref to the wrapper so we can dispose it when starting a new run const wrapperRef = useRef(null); + // Restore function for AWS_REGION / AWS_DEFAULT_REGION overrides, applied + // after target resolution so downstream SDK / CDK toolkit-lib clients use the + // aws-targets.json region rather than whatever the SDK default chain resolves. + // See https://github.com/aws/agentcore-cli/issues/924. + const restoreRegionEnvRef = useRef<(() => void) | null>(null); const updateStep = (index: number, update: Partial) => { setSteps(prev => prev.map((s, i) => (i === index ? { ...s, ...update } : s))); @@ -158,10 +164,18 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { } }, []); + // Restore AWS_REGION / AWS_DEFAULT_REGION (no-op when nothing was applied) + const restoreRegionEnv = useCallback(() => { + restoreRegionEnvRef.current?.(); + restoreRegionEnvRef.current = null; + }, []); + const startPreflight = useCallback(async () => { if (isRunningRef.current) return; // Dispose any existing wrapper before starting a new run await disposeWrapper(); + // Restore any previously-applied region env override before re-running + restoreRegionEnv(); resetSteps(); setCdkToolkitWrapper(null); setStackNames([]); @@ -169,7 +183,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { setHasTokenExpiredError(false); // Reset token expired state when retrying setHasCredentialsError(false); // Reset credentials error state when retrying setPhase('running'); - }, [disposeWrapper]); + }, [disposeWrapper, restoreRegionEnv]); const clearTokenExpiredError = useCallback(() => { setHasTokenExpiredError(false); @@ -183,6 +197,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { useEffect(() => { const handleInterrupt = () => { void disposeWrapper(); + restoreRegionEnv(); }; process.on('SIGINT', handleInterrupt); @@ -193,8 +208,9 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { process.off('SIGTERM', handleInterrupt); // Dispose on unmount (user navigated away) void disposeWrapper(); + restoreRegionEnv(); }; - }, [disposeWrapper]); + }, [disposeWrapper, restoreRegionEnv]); const confirmTeardown = useCallback(() => { // Mark teardown as confirmed and restart the preflight flow @@ -206,7 +222,8 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { const cancelTeardown = useCallback(() => { setPhase('error'); isRunningRef.current = false; - }, []); + restoreRegionEnv(); + }, [restoreRegionEnv]); const confirmBootstrap = useCallback(() => { setPhase('bootstrapping'); @@ -274,6 +291,15 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { try { preflightContext = await validateProject(); setContext(preflightContext); + // Make aws-targets.json region authoritative for downstream SDK / CDK + // toolkit-lib clients that bypass explicit region options. Restored on + // unmount, teardown rejection, or subsequent preflight start. + // See https://github.com/aws/agentcore-cli/issues/924. + const firstTarget = preflightContext.awsTargets[0]; + if (firstTarget) { + restoreRegionEnv(); + restoreRegionEnvRef.current = applyTargetRegionToEnv(firstTarget.region); + } logger.endStep('success'); updateStep(STEP_VALIDATE, { status: 'success' }); } catch (err) { @@ -493,7 +519,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { return () => { process.off('unhandledRejection', handleUnhandledRejection); }; - }, [phase, logger, switchableIoHost, isInteractive, skipIdentityCheck, teardownConfirmed]); + }, [phase, logger, switchableIoHost, isInteractive, skipIdentityCheck, teardownConfirmed, restoreRegionEnv]); // Handle identity-setup phase (after user provides credentials) useEffect(() => { From f2bfd201098c91777e4fba67fe6288ee2f4cf252 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Wed, 22 Apr 2026 21:23:33 -0400 Subject: [PATCH 2/2] fix(deploy): restore region env on TUI error states; consolidate barrel exports Review feedback: 1. TUI preflight error branches called setPhase('error') without calling restoreRegionEnv(). Add a useEffect guarded on phase === 'error' so every error path restores the env override without threading the call into every branch. 2. Export applyTargetRegionToEnv from the aws barrel for consistency with withTargetRegion. Update CLI deploy, teardown, and TUI preflight hook to import from the barrel instead of the deep path. --- src/cli/aws/index.ts | 2 +- src/cli/commands/deploy/actions.ts | 2 +- src/cli/operations/deploy/teardown.ts | 2 +- src/cli/tui/hooks/useCdkPreflight.ts | 12 +++++++++++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/cli/aws/index.ts b/src/cli/aws/index.ts index a60c73da0..357896688 100644 --- a/src/cli/aws/index.ts +++ b/src/cli/aws/index.ts @@ -1,7 +1,7 @@ export { detectAwsContext, type AwsContext } from './aws-context'; export { detectAccount, getCredentialProvider } from './account'; export { detectRegion, type RegionDetectionResult } from './region'; -export { withTargetRegion } from './target-region'; +export { applyTargetRegionToEnv, withTargetRegion } from './target-region'; export { invokeBedrockSync, invokeClaude, diff --git a/src/cli/commands/deploy/actions.ts b/src/cli/commands/deploy/actions.ts index fda7d1de5..b3d7ad55b 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -1,7 +1,7 @@ import { ConfigIO, SecureCredentials } from '../../../lib'; import type { AgentCoreMcpSpec, DeployedState } from '../../../schema'; +import { applyTargetRegionToEnv } from '../../aws'; import { validateAwsCredentials } from '../../aws/account'; -import { applyTargetRegionToEnv } from '../../aws/target-region'; import { createSwitchableIoHost } from '../../cdk/toolkit-lib'; import { buildDeployedState, diff --git a/src/cli/operations/deploy/teardown.ts b/src/cli/operations/deploy/teardown.ts index 64ceaf087..28a8f326a 100644 --- a/src/cli/operations/deploy/teardown.ts +++ b/src/cli/operations/deploy/teardown.ts @@ -1,6 +1,6 @@ import { CONFIG_DIR, ConfigIO } from '../../../lib'; import type { AwsDeploymentTarget } from '../../../schema'; -import { withTargetRegion } from '../../aws/target-region'; +import { withTargetRegion } from '../../aws'; import { CdkToolkitWrapper, silentIoHost } from '../../cdk/toolkit-lib'; import { type DiscoveredStack, findStack } from '../../cloudformation/stack-discovery'; import { StackSelectionStrategy } from '@aws-cdk/toolkit-lib'; diff --git a/src/cli/tui/hooks/useCdkPreflight.ts b/src/cli/tui/hooks/useCdkPreflight.ts index e1aea4b43..062da752f 100644 --- a/src/cli/tui/hooks/useCdkPreflight.ts +++ b/src/cli/tui/hooks/useCdkPreflight.ts @@ -1,7 +1,7 @@ import { ConfigIO, SecureCredentials } from '../../../lib'; import type { DeployedState } from '../../../schema'; +import { applyTargetRegionToEnv } from '../../aws'; import { AwsCredentialsError, validateAwsCredentials } from '../../aws/account'; -import { applyTargetRegionToEnv } from '../../aws/target-region'; import { type CdkToolkitWrapper, type SwitchableIoHost, createSwitchableIoHost } from '../../cdk/toolkit-lib'; import { getErrorMessage, isExpiredTokenError, isNoCredentialsError } from '../../errors'; import type { ExecLogger } from '../../logging'; @@ -212,6 +212,16 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { }; }, [disposeWrapper, restoreRegionEnv]); + // Restore region env override when any preflight stage lands in 'error'. + // Individual error branches inside the stage effects only call setPhase('error') + // without cleanup, so this hook is the single place that guarantees restore + // happens on every error path without threading the call into every branch. + useEffect(() => { + if (phase === 'error') { + restoreRegionEnv(); + } + }, [phase, restoreRegionEnv]); + const confirmTeardown = useCallback(() => { // Mark teardown as confirmed and restart the preflight flow setTeardownConfirmed(true);