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..357896688 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 { applyTargetRegionToEnv, 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..b3d7ad55b 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -1,5 +1,6 @@ import { ConfigIO, SecureCredentials } from '../../../lib'; import type { AgentCoreMcpSpec, DeployedState } from '../../../schema'; +import { applyTargetRegionToEnv } from '../../aws'; import { validateAwsCredentials } from '../../aws/account'; import { createSwitchableIoHost } from '../../cdk/toolkit-lib'; import { @@ -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..062da752f 100644 --- a/src/cli/tui/hooks/useCdkPreflight.ts +++ b/src/cli/tui/hooks/useCdkPreflight.ts @@ -1,5 +1,6 @@ import { ConfigIO, SecureCredentials } from '../../../lib'; import type { DeployedState } from '../../../schema'; +import { applyTargetRegionToEnv } from '../../aws'; import { AwsCredentialsError, validateAwsCredentials } from '../../aws/account'; import { type CdkToolkitWrapper, type SwitchableIoHost, createSwitchableIoHost } from '../../cdk/toolkit-lib'; import { getErrorMessage, isExpiredTokenError, isNoCredentialsError } from '../../errors'; @@ -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,19 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { process.off('SIGTERM', handleInterrupt); // Dispose on unmount (user navigated away) void disposeWrapper(); + restoreRegionEnv(); }; - }, [disposeWrapper]); + }, [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 @@ -206,7 +232,8 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult { const cancelTeardown = useCallback(() => { setPhase('error'); isRunningRef.current = false; - }, []); + restoreRegionEnv(); + }, [restoreRegionEnv]); const confirmBootstrap = useCallback(() => { setPhase('bootstrapping'); @@ -274,6 +301,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 +529,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(() => {