From e95fcf14753a57c44b4a8be19445c0234bfee41f Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 8 Jun 2026 18:01:48 +0000 Subject: [PATCH 1/3] fix(deploy): auto-populate default target on non-interactive deploy A freshly-created project has an empty aws-targets.json by design (both interactive and non-interactive create write []; the target is populated at deploy time). The interactive deploy flow prompts for the target via the TUI, but the non-interactive path (deploy --yes/--json/--target) routes through handleDeploy, which looked up the 'default' target and failed with 'Target "default" not found' on any fresh project. Only 'agentcore dev' (runCliDeploy) auto-populated the target from the detected AWS context; plain non-interactive deploy did not. Extract that logic into a shared ensureDefaultDeploymentTarget helper and call it in handleDeploy before the target lookup, so all non-interactive deploys detect the account/region and write a single 'default' target (best-effort; if the account can't be detected the existing clear error still surfaces). runCliDeploy reuses the same helper. Addresses the deploy half of #699. Tests: unit (ensure-target.test.ts) + integ (deploy-autopopulate-target.test.ts); manually verified via npm run bundle + global install + create/deploy. --- .../deploy-autopopulate-target.test.ts | 64 +++++++++++++++ src/cli/commands/deploy/actions.ts | 8 +- src/cli/commands/deploy/progress.ts | 25 +----- .../deploy/__tests__/ensure-target.test.ts | 82 +++++++++++++++++++ src/cli/operations/deploy/ensure-target.ts | 41 ++++++++++ src/cli/operations/deploy/index.ts | 3 + 6 files changed, 201 insertions(+), 22 deletions(-) create mode 100644 integ-tests/deploy-autopopulate-target.test.ts create mode 100644 src/cli/operations/deploy/__tests__/ensure-target.test.ts create mode 100644 src/cli/operations/deploy/ensure-target.ts diff --git a/integ-tests/deploy-autopopulate-target.test.ts b/integ-tests/deploy-autopopulate-target.test.ts new file mode 100644 index 000000000..7ad06d3c3 --- /dev/null +++ b/integ-tests/deploy-autopopulate-target.test.ts @@ -0,0 +1,64 @@ +import { hasAwsCredentials, runCLI } from '../src/test-utils/index.js'; +import { randomUUID } from 'node:crypto'; +import { mkdir, readFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +/** + * Regression test for non-interactive deploy on a freshly-created project. + * + * `agentcore create` writes an empty aws-targets.json by design (the target is + * populated at deploy time). The interactive deploy prompts for it; the + * non-interactive deploy path (`--yes`/`--json`/`--target`) must auto-populate a + * default target from the detected AWS context instead of failing with + * `Target "default" not found`. + */ +describe('integration: non-interactive deploy auto-populates aws-targets', () => { + let testDir: string; + let projectDir: string; + + beforeAll(async () => { + testDir = join(tmpdir(), `agentcore-deploy-autopop-${randomUUID()}`); + await mkdir(testDir, { recursive: true }); + + const name = `AutoPop${Date.now().toString().slice(-6)}`; + const result = await runCLI(['create', '--name', name, '--no-agent', '--json'], testDir); + expect(result.exitCode, `create failed: ${result.stderr}`).toBe(0); + projectDir = join(testDir, name); + }); + + afterAll(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + it('create leaves aws-targets.json empty', async () => { + const targets = JSON.parse(await readFile(join(projectDir, 'agentcore', 'aws-targets.json'), 'utf-8')); + expect(targets).toEqual([]); + }); + + it('non-interactive deploy does not fail with "target not found"', async () => { + // --dry-run avoids real CDK deploy; the target lookup (and our auto-populate) + // still runs first. The key assertion: deploy must NOT bail with the + // pre-fix "Target default not found" error. + const result = await runCLI(['deploy', '--json', '--dry-run'], projectDir); + + const combined = `${result.stdout}\n${result.stderr}`; + expect(combined.toLowerCase()).not.toContain('not found in aws-targets.json'); + expect(combined.toLowerCase()).not.toContain('target "default" not found'); + }); + + it.skipIf(!hasAwsCredentials())( + 'auto-populates a default target from AWS context when credentials are available', + async () => { + await runCLI(['deploy', '--json', '--dry-run'], projectDir); + + const targets = JSON.parse(await readFile(join(projectDir, 'agentcore', 'aws-targets.json'), 'utf-8')); + expect(Array.isArray(targets)).toBe(true); + expect(targets.length).toBeGreaterThan(0); + expect(targets[0].name).toBe('default'); + expect(targets[0].account).toMatch(/^\d{12}$/); + expect(typeof targets[0].region).toBe('string'); + } + ); +}); diff --git a/src/cli/commands/deploy/actions.ts b/src/cli/commands/deploy/actions.ts index 65dbcddb1..9856b64b4 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -25,6 +25,7 @@ import { buildCdkProject, checkBootstrapNeeded, checkStackDeployability, + ensureDefaultDeploymentTarget, getAllCredentials, hasIdentityApiProviders, hasIdentityOAuthProviders, @@ -117,8 +118,13 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise t.name === options.target); if (!target) { diff --git a/src/cli/commands/deploy/progress.ts b/src/cli/commands/deploy/progress.ts index e222de52f..da7eb7b3c 100644 --- a/src/cli/commands/deploy/progress.ts +++ b/src/cli/commands/deploy/progress.ts @@ -1,7 +1,7 @@ import { ConfigIO } from '../../../lib'; -import { detectAwsContext } from '../../aws/aws-context'; import { ANSI } from '../../constants'; import { getErrorMessage } from '../../errors'; +import { ensureDefaultDeploymentTarget } from '../../operations/deploy'; import { canSkipDeploy } from '../../operations/deploy/change-detection'; import { handleDeploy } from './actions'; @@ -48,27 +48,10 @@ export async function runCliDeploy(): Promise { const { onProgress, cleanup } = createSpinnerProgress(); try { - // Auto-populate aws-targets.json if empty + // Auto-populate aws-targets.json if empty (best-effort). handleDeploy also + // does this, but we run it here first so canSkipDeploy sees a populated target. const configIO = new ConfigIO(); - try { - const targets = await configIO.readAWSDeploymentTargets(); - if (targets.length === 0) { - const ctx = await detectAwsContext(); - if (ctx.accountId) { - await configIO.writeAWSDeploymentTargets([{ name: 'default', account: ctx.accountId, region: ctx.region }]); - } - } - } catch { - // aws-targets.json doesn't exist — try to create it - try { - const ctx = await detectAwsContext(); - if (ctx.accountId) { - await configIO.writeAWSDeploymentTargets([{ name: 'default', account: ctx.accountId, region: ctx.region }]); - } - } catch { - // Can't detect — let handleDeploy fail with a clear error - } - } + await ensureDefaultDeploymentTarget(configIO); const noChanges = await canSkipDeploy(configIO); if (noChanges) { diff --git a/src/cli/operations/deploy/__tests__/ensure-target.test.ts b/src/cli/operations/deploy/__tests__/ensure-target.test.ts new file mode 100644 index 000000000..938c7077a --- /dev/null +++ b/src/cli/operations/deploy/__tests__/ensure-target.test.ts @@ -0,0 +1,82 @@ +import type { ConfigIO } from '../../../../lib'; +import { ensureDefaultDeploymentTarget } from '../ensure-target.js'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockDetectAwsContext } = vi.hoisted(() => ({ + mockDetectAwsContext: vi.fn(), +})); + +vi.mock('../../../aws', () => ({ + detectAwsContext: mockDetectAwsContext, +})); + +/** Build a fake ConfigIO with stubbed read/write target methods. */ +function makeConfigIO(opts: { read?: () => Promise }): { configIO: ConfigIO; writes: unknown[] } { + const writes: unknown[] = []; + const configIO = { + readAWSDeploymentTargets: opts.read ?? (() => Promise.resolve([])), + writeAWSDeploymentTargets: (data: unknown) => { + writes.push(data); + return Promise.resolve(); + }, + } as unknown as ConfigIO; + return { configIO, writes }; +} + +describe('ensureDefaultDeploymentTarget', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockDetectAwsContext.mockResolvedValue({ accountId: '123456789012', region: 'us-east-1', regionSource: 'env' }); + }); + + it('writes a default target when aws-targets.json is empty', async () => { + const { configIO, writes } = makeConfigIO({ read: () => Promise.resolve([]) }); + + const wrote = await ensureDefaultDeploymentTarget(configIO); + + expect(wrote).toBe(true); + expect(writes).toHaveLength(1); + expect(writes[0]).toEqual([{ name: 'default', account: '123456789012', region: 'us-east-1' }]); + }); + + it('does not overwrite when a target already exists', async () => { + const existing = [{ name: 'prod', account: '999888777666', region: 'us-west-2' }]; + const { configIO, writes } = makeConfigIO({ read: () => Promise.resolve(existing) }); + + const wrote = await ensureDefaultDeploymentTarget(configIO); + + expect(wrote).toBe(false); + expect(writes).toHaveLength(0); + expect(mockDetectAwsContext).not.toHaveBeenCalled(); + }); + + it('treats an unreadable/missing targets file as empty and populates it', async () => { + const { configIO, writes } = makeConfigIO({ + read: () => Promise.reject(new Error('ENOENT: aws-targets.json not found')), + }); + + const wrote = await ensureDefaultDeploymentTarget(configIO); + + expect(wrote).toBe(true); + expect(writes[0]).toEqual([{ name: 'default', account: '123456789012', region: 'us-east-1' }]); + }); + + it('does not write when the AWS account cannot be detected', async () => { + mockDetectAwsContext.mockResolvedValue({ accountId: null, region: 'us-east-1', regionSource: 'default' }); + const { configIO, writes } = makeConfigIO({ read: () => Promise.resolve([]) }); + + const wrote = await ensureDefaultDeploymentTarget(configIO); + + expect(wrote).toBe(false); + expect(writes).toHaveLength(0); + }); + + it('uses the detected region for the default target', async () => { + mockDetectAwsContext.mockResolvedValue({ accountId: '123456789012', region: 'eu-west-1', regionSource: 'config' }); + const { configIO, writes } = makeConfigIO({ read: () => Promise.resolve([]) }); + + await ensureDefaultDeploymentTarget(configIO); + + expect(writes[0]).toEqual([{ name: 'default', account: '123456789012', region: 'eu-west-1' }]); + }); +}); diff --git a/src/cli/operations/deploy/ensure-target.ts b/src/cli/operations/deploy/ensure-target.ts new file mode 100644 index 000000000..750e9d45a --- /dev/null +++ b/src/cli/operations/deploy/ensure-target.ts @@ -0,0 +1,41 @@ +import type { ConfigIO } from '../../../lib'; +import { detectAwsContext } from '../../aws'; + +/** + * Ensure `aws-targets.json` has at least one deployment target. + * + * Freshly-created projects (via `agentcore create`, interactive or not) write an + * empty `aws-targets.json` by design — the target is expected to be populated at + * deploy time. The interactive deploy flow prompts the user for it, but the + * non-interactive deploy path (`deploy --yes` / `--json` / `--target`) has no + * prompt, so it would otherwise fail with `Target "default" not found`. + * + * This mirrors the auto-populate behavior already used by `agentcore dev` + * (`runCliDeploy`): if no targets exist, detect the account/region from the + * environment and write a single `default` target. Best-effort — if the account + * can't be detected, the file is left as-is and the caller surfaces a clear + * "target not found" error. + * + * @returns true if a default target was written, false otherwise. + */ +export async function ensureDefaultDeploymentTarget(configIO: ConfigIO): Promise { + let targets; + try { + targets = await configIO.readAWSDeploymentTargets(); + } catch { + // aws-targets.json missing or unreadable — treat as empty and try to populate. + targets = []; + } + + if (targets.length > 0) { + return false; + } + + const ctx = await detectAwsContext(); + if (!ctx.accountId) { + return false; + } + + await configIO.writeAWSDeploymentTargets([{ name: 'default', account: ctx.accountId, region: ctx.region }]); + return true; +} diff --git a/src/cli/operations/deploy/index.ts b/src/cli/operations/deploy/index.ts index 93cdb6353..f71e9246f 100644 --- a/src/cli/operations/deploy/index.ts +++ b/src/cli/operations/deploy/index.ts @@ -60,6 +60,9 @@ export { type OnlineEvalEnableResult, } from './post-deploy-online-evals'; +// Auto-populate a default deployment target for non-interactive deploys +export { ensureDefaultDeploymentTarget } from './ensure-target'; + // Post-deploy config bundles export { setupConfigBundles, From 37d5a4b86d9486e6120e8ae039a078d7b551adb9 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 8 Jun 2026 18:28:24 +0000 Subject: [PATCH 2/3] refactor(deploy): use shared target helper in dev; drop integ test - useDevDeploy (dev TUI path) now uses ensureDefaultDeploymentTarget instead of its own duplicated inline auto-populate block, so all deploy entry points (handleDeploy, runCliDeploy, dev TUI) go through one helper. - Remove integ-tests/deploy-autopopulate-target.test.ts: per integ-tests/README.md integ tests run without AWS credentials, where the auto-populate is a no-op and the middle assertion would fail. The unit tests in ensure-target.test.ts already cover the empty/populated/no-account branches. --- .../deploy-autopopulate-target.test.ts | 64 ------------------- src/cli/tui/hooks/useDevDeploy.ts | 28 ++------ 2 files changed, 4 insertions(+), 88 deletions(-) delete mode 100644 integ-tests/deploy-autopopulate-target.test.ts diff --git a/integ-tests/deploy-autopopulate-target.test.ts b/integ-tests/deploy-autopopulate-target.test.ts deleted file mode 100644 index 7ad06d3c3..000000000 --- a/integ-tests/deploy-autopopulate-target.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { hasAwsCredentials, runCLI } from '../src/test-utils/index.js'; -import { randomUUID } from 'node:crypto'; -import { mkdir, readFile, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; - -/** - * Regression test for non-interactive deploy on a freshly-created project. - * - * `agentcore create` writes an empty aws-targets.json by design (the target is - * populated at deploy time). The interactive deploy prompts for it; the - * non-interactive deploy path (`--yes`/`--json`/`--target`) must auto-populate a - * default target from the detected AWS context instead of failing with - * `Target "default" not found`. - */ -describe('integration: non-interactive deploy auto-populates aws-targets', () => { - let testDir: string; - let projectDir: string; - - beforeAll(async () => { - testDir = join(tmpdir(), `agentcore-deploy-autopop-${randomUUID()}`); - await mkdir(testDir, { recursive: true }); - - const name = `AutoPop${Date.now().toString().slice(-6)}`; - const result = await runCLI(['create', '--name', name, '--no-agent', '--json'], testDir); - expect(result.exitCode, `create failed: ${result.stderr}`).toBe(0); - projectDir = join(testDir, name); - }); - - afterAll(async () => { - await rm(testDir, { recursive: true, force: true }); - }); - - it('create leaves aws-targets.json empty', async () => { - const targets = JSON.parse(await readFile(join(projectDir, 'agentcore', 'aws-targets.json'), 'utf-8')); - expect(targets).toEqual([]); - }); - - it('non-interactive deploy does not fail with "target not found"', async () => { - // --dry-run avoids real CDK deploy; the target lookup (and our auto-populate) - // still runs first. The key assertion: deploy must NOT bail with the - // pre-fix "Target default not found" error. - const result = await runCLI(['deploy', '--json', '--dry-run'], projectDir); - - const combined = `${result.stdout}\n${result.stderr}`; - expect(combined.toLowerCase()).not.toContain('not found in aws-targets.json'); - expect(combined.toLowerCase()).not.toContain('target "default" not found'); - }); - - it.skipIf(!hasAwsCredentials())( - 'auto-populates a default target from AWS context when credentials are available', - async () => { - await runCLI(['deploy', '--json', '--dry-run'], projectDir); - - const targets = JSON.parse(await readFile(join(projectDir, 'agentcore', 'aws-targets.json'), 'utf-8')); - expect(Array.isArray(targets)).toBe(true); - expect(targets.length).toBeGreaterThan(0); - expect(targets[0].name).toBe('default'); - expect(targets[0].account).toMatch(/^\d{12}$/); - expect(typeof targets[0].region).toBe('string'); - } - ); -}); diff --git a/src/cli/tui/hooks/useDevDeploy.ts b/src/cli/tui/hooks/useDevDeploy.ts index 893706e3f..a570676f4 100644 --- a/src/cli/tui/hooks/useDevDeploy.ts +++ b/src/cli/tui/hooks/useDevDeploy.ts @@ -1,8 +1,8 @@ import { ConfigIO } from '../../../lib'; -import { detectAwsContext } from '../../aws/aws-context'; import type { DeployMessage } from '../../cdk/toolkit-lib'; import { handleDeploy } from '../../commands/deploy/actions'; import { getErrorMessage } from '../../errors'; +import { ensureDefaultDeploymentTarget } from '../../operations/deploy'; import { canSkipDeploy } from '../../operations/deploy/change-detection'; import type { Step } from '../components/StepProgress'; import { useCallback, useEffect, useRef, useState } from 'react'; @@ -62,29 +62,9 @@ export function useDevDeploy({ skip, ready = true }: UseDevDeployOptions = {}): // If we can't read project spec, proceed with deploy as a safe default } - // Auto-populate aws-targets.json if empty - try { - const targets = await configIO.readAWSDeploymentTargets(); - if (targets.length === 0) { - const ctx = await detectAwsContext(); - if (ctx.accountId) { - await configIO.writeAWSDeploymentTargets([ - { name: 'default', account: ctx.accountId, region: ctx.region }, - ]); - } - } - } catch { - try { - const ctx = await detectAwsContext(); - if (ctx.accountId) { - await configIO.writeAWSDeploymentTargets([ - { name: 'default', account: ctx.accountId, region: ctx.region }, - ]); - } - } catch { - // Can't detect — let handleDeploy fail with a clear error - } - } + // Auto-populate aws-targets.json if empty (best-effort). handleDeploy also + // does this, but we run it here first so canSkipDeploy sees a populated target. + await ensureDefaultDeploymentTarget(configIO); const noChanges = await canSkipDeploy(configIO); if (noChanges) { From 0a64ed82cd81d22c8cb6308fada48aacc72c2fd4 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 8 Jun 2026 19:30:55 +0000 Subject: [PATCH 3/3] refactor(deploy): surface non-not-found read errors; address review nits Per review feedback on #1478: - ensureDefaultDeploymentTarget now only treats a genuinely missing file (ConfigNotFoundError) as empty; any other read failure (corrupt JSON, validation, permissions) is rethrown instead of silently overwriting a file that exists but couldn't be parsed. - Drop the redundant export comment in operations/deploy/index.ts. - Add a unit test covering the rethrow path. --- .../deploy/__tests__/ensure-target.test.ts | 15 ++++++++++--- src/cli/operations/deploy/ensure-target.ts | 22 ++++++++++++------- src/cli/operations/deploy/index.ts | 1 - 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/cli/operations/deploy/__tests__/ensure-target.test.ts b/src/cli/operations/deploy/__tests__/ensure-target.test.ts index 938c7077a..6910698e1 100644 --- a/src/cli/operations/deploy/__tests__/ensure-target.test.ts +++ b/src/cli/operations/deploy/__tests__/ensure-target.test.ts @@ -1,4 +1,4 @@ -import type { ConfigIO } from '../../../../lib'; +import { type ConfigIO, ConfigNotFoundError } from '../../../../lib'; import { ensureDefaultDeploymentTarget } from '../ensure-target.js'; import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -50,9 +50,9 @@ describe('ensureDefaultDeploymentTarget', () => { expect(mockDetectAwsContext).not.toHaveBeenCalled(); }); - it('treats an unreadable/missing targets file as empty and populates it', async () => { + it('treats a missing targets file (ConfigNotFoundError) as empty and populates it', async () => { const { configIO, writes } = makeConfigIO({ - read: () => Promise.reject(new Error('ENOENT: aws-targets.json not found')), + read: () => Promise.reject(new ConfigNotFoundError('aws-targets.json', 'AWS Targets')), }); const wrote = await ensureDefaultDeploymentTarget(configIO); @@ -61,6 +61,15 @@ describe('ensureDefaultDeploymentTarget', () => { expect(writes[0]).toEqual([{ name: 'default', account: '123456789012', region: 'us-east-1' }]); }); + it('surfaces a non-not-found read error instead of overwriting the file', async () => { + const { configIO, writes } = makeConfigIO({ + read: () => Promise.reject(new Error('Unexpected end of JSON input')), + }); + + await expect(ensureDefaultDeploymentTarget(configIO)).rejects.toThrow('Unexpected end of JSON input'); + expect(writes).toHaveLength(0); + }); + it('does not write when the AWS account cannot be detected', async () => { mockDetectAwsContext.mockResolvedValue({ accountId: null, region: 'us-east-1', regionSource: 'default' }); const { configIO, writes } = makeConfigIO({ read: () => Promise.resolve([]) }); diff --git a/src/cli/operations/deploy/ensure-target.ts b/src/cli/operations/deploy/ensure-target.ts index 750e9d45a..b07fe9c78 100644 --- a/src/cli/operations/deploy/ensure-target.ts +++ b/src/cli/operations/deploy/ensure-target.ts @@ -1,4 +1,4 @@ -import type { ConfigIO } from '../../../lib'; +import { type ConfigIO, ConfigNotFoundError } from '../../../lib'; import { detectAwsContext } from '../../aws'; /** @@ -10,11 +10,14 @@ import { detectAwsContext } from '../../aws'; * non-interactive deploy path (`deploy --yes` / `--json` / `--target`) has no * prompt, so it would otherwise fail with `Target "default" not found`. * - * This mirrors the auto-populate behavior already used by `agentcore dev` - * (`runCliDeploy`): if no targets exist, detect the account/region from the - * environment and write a single `default` target. Best-effort — if the account - * can't be detected, the file is left as-is and the caller surfaces a clear - * "target not found" error. + * This mirrors the auto-populate behavior already used by `agentcore dev`: if no + * targets exist, detect the account/region from the environment and write a + * single `default` target. Best-effort — if the account can't be detected, the + * file is left as-is and the caller surfaces a clear "target not found" error. + * + * A missing `aws-targets.json` is treated as empty. Any other read failure + * (corrupt JSON, validation error, permissions) is surfaced rather than silently + * overwriting a file that exists but couldn't be parsed. * * @returns true if a default target was written, false otherwise. */ @@ -22,8 +25,11 @@ export async function ensureDefaultDeploymentTarget(configIO: ConfigIO): Promise let targets; try { targets = await configIO.readAWSDeploymentTargets(); - } catch { - // aws-targets.json missing or unreadable — treat as empty and try to populate. + } catch (err) { + // Only treat a genuinely-missing file as empty; surface real read errors. + if (!(err instanceof ConfigNotFoundError)) { + throw err; + } targets = []; } diff --git a/src/cli/operations/deploy/index.ts b/src/cli/operations/deploy/index.ts index f71e9246f..db6d841eb 100644 --- a/src/cli/operations/deploy/index.ts +++ b/src/cli/operations/deploy/index.ts @@ -60,7 +60,6 @@ export { type OnlineEvalEnableResult, } from './post-deploy-online-evals'; -// Auto-populate a default deployment target for non-interactive deploys export { ensureDefaultDeploymentTarget } from './ensure-target'; // Post-deploy config bundles