fix(vpc): correct create-harness VPC validation, dry-run persistence, and TUI review display (follow-up to #1671)

src/cli/commands/create/__tests__/create.test.ts

@@ -47,7 +47,7 @@
 
       const json = JSON.parse(result.stdout);
       expect(json.success).toBe(true);
       expect(json.projectPath).toMatch(new RegExp(`/${projectName}$`));
       expect(await exists(join(json.projectPath, 'agentcore'))).toBeTruthy();
     });
   });
@@ -192,7 +192,7 @@
 
       const json = JSON.parse(result.stdout);
       expect(json.success).toBe(true);
       expect(json.projectPath).toMatch(new RegExp(`/${projectName}$`));
       expect(json.agentName).toBe(agentName);
       expect(await exists(join(json.projectPath, 'app', agentName))).toBeTruthy();
 
@@ -230,7 +230,7 @@
 
       const json = JSON.parse(result.stdout);
       expect(json.success).toBe(true);
       expect(json.projectPath).toMatch(new RegExp(`/${projectName}$`));
       expect(json.agentName).toBe(agentName);
       expect(await exists(join(json.projectPath, 'app', agentName))).toBeTruthy();
 
@@ -275,7 +275,7 @@
 
       expect(result.exitCode).toBe(0);
       const json = JSON.parse(result.stdout);
       expect(json.projectPath).toMatch(new RegExp(`/${projectName}$`));
       expect(json.wouldCreate).toContain(`${json.projectPath}/app/${agentName}/`);
       expect(await exists(join(testDir, projectName)), 'Should not create directory').toBe(false);
     });
@@ -333,4 +333,97 @@
       expect(result.stderr).not.toContain('--name is required');
     });
   });
+
+  // Regression coverage for the create-harness VPC dispatch: the unit tests on
+  // validateCreateHarnessOptions call the validator directly with a full options object, so they
+  // could not catch command.tsx failing to forward container/subnets/securityGroups/vpcId into it.
+  // These drive the real CLI dispatch (via runCLI --dry-run, no AWS) so the wiring is exercised.
+  describe('harness VPC dispatch (--dry-run)', () => {
+    const SUBNET = 'subnet-05169b775866f2440';
+    const SG = 'sg-0390682a9d9f7dd8d';
+    const VPC = 'vpc-07086549ccf106a5d';
+
+    it('accepts a Container+VPC dockerfile harness when all VPC flags incl --vpc-id are supplied', async () => {
+      const name = `HVpcOk${Date.now().toString().slice(-6)}`;
+      const result = await runCLI(
+        [
+          'create',
+          '--name',
+          name,
+          '--container',
+          './Dockerfile',
+          '--network-mode',
+          'VPC',
+          '--subnets',
+          SUBNET,
+          '--security-groups',
+          SG,
+          '--vpc-id',
+          VPC,
+          '--model-provider',
+          'bedrock',
+          '--dry-run',
+        ],
+        testDir
+      );
+      // Must NOT fail with the bogus "--subnets is required" (the regression); dry-run reaches synth-info.
+      expect(result.exitCode, `stderr: ${result.stderr}, stdout: ${result.stdout}`).toBe(0);
+      expect(result.stderr).not.toContain('--subnets is required');
+    });
+
+    it('rejects a Container+VPC harness missing --vpc-id with the friendly error', async () => {
+      const name = `HVpcNoId${Date.now().toString().slice(-6)}`;
+      const result = await runCLI(
+        [
+          'create',
+          '--name',
+          name,
+          '--container',
+          './Dockerfile',
+          '--network-mode',
+          'VPC',
+          '--subnets',
+          SUBNET,
+          '--security-groups',
+          SG,
+          '--model-provider',
+          'bedrock',
+          '--dry-run',
+        ],
+        testDir
+      );
+      expect(result.exitCode).toBe(1);
+      const out = `${result.stderr}${result.stdout}`;
+      expect(out).toContain('--vpc-id is required');
+      // It must NOT be the misleading "--subnets is required" message.
+      expect(out).not.toContain('--subnets is required');
+    });
+
+    it('rejects a malformed --vpc-id on the harness path', async () => {
+      const name = `HVpcBad${Date.now().toString().slice(-6)}`;
+      const result = await runCLI(
+        [
+          'create',
+          '--name',
+          name,
+          '--container',
+          './Dockerfile',
+          '--network-mode',
+          'VPC',
+          '--subnets',
+          SUBNET,
+          '--security-groups',
+          SG,
+          '--vpc-id',
+          'vpc-XYZ',
+          '--model-provider',
+          'bedrock',
+          '--dry-run',
+        ],
+        testDir
+      );
+      expect(result.exitCode).toBe(1);
+      expect(`${result.stderr}${result.stdout}`).toContain('Invalid VPC ID format');
+    });
+  });
 });

src/cli/commands/create/command.tsx

@@ -160,23 +160,29 @@ async function handleCreateHarnessCLI(options: CreateOptions): Promise<void> {
   const name = options.name ?? options.projectName;
   const projectName = options.projectName ?? name;
 
+  // Single source for the harness validation input, shared by the dry-run and telemetry-wrapped
+  // paths below. Keeping ONE object prevents the two call sites from drifting — forwarding a field
+  // on one path but not the other is exactly what caused the create-harness VPC validation blocker.
+  const harnessValidationInput = {
+    name,
+    projectName,
+    modelProvider: options.modelProvider,
+    modelId: options.modelId,
+    apiKeyArn: options.apiKeyArn,
+    container: options.container,
+    networkMode: options.networkMode,
+    subnets: options.subnets,
+    securityGroups: options.securityGroups,
+    vpcId: options.vpcId,
+    efsAccessPointArn: options.efsAccessPointArn,
+    efsMountPath: options.efsMountPath,
+    s3AccessPointArn: options.s3AccessPointArn,
+    s3MountPath: options.s3MountPath,
+  };
+
   // Handle dry-run mode (no telemetry for dry-run)
   if (options.dryRun) {
-    const validation = validateCreateHarnessOptions(
-      {
-        name,
-        projectName,
-        modelProvider: options.modelProvider,
-        modelId: options.modelId,
-        apiKeyArn: options.apiKeyArn,
-        networkMode: options.networkMode,
-        efsAccessPointArn: options.efsAccessPointArn,
-        efsMountPath: options.efsMountPath,
-        s3AccessPointArn: options.s3AccessPointArn,
-        s3MountPath: options.s3MountPath,
-      },
-      cwd
-    );
+    const validation = validateCreateHarnessOptions(harnessValidationInput, cwd);
     if (!validation.valid) {
       if (options.json) {
         console.log(JSON.stringify({ success: false, error: validation.error }));
@@ -209,21 +215,7 @@ async function handleCreateHarnessCLI(options: CreateOptions): Promise<void> {
       network_mode: standardize(NetworkModeEnum, options.networkMode ?? 'public'),
     },
     async () => {
-      const validation = validateCreateHarnessOptions(
-        {
-          name,
-          projectName,
-          modelProvider: options.modelProvider,
-          modelId: options.modelId,
-          apiKeyArn: options.apiKeyArn,
-          networkMode: options.networkMode,
-          efsAccessPointArn: options.efsAccessPointArn,
-          efsMountPath: options.efsMountPath,
-          s3AccessPointArn: options.s3AccessPointArn,
-          s3MountPath: options.s3MountPath,
-        },
-        cwd
-      );
+      const validation = validateCreateHarnessOptions(harnessValidationInput, cwd);
       if (!validation.valid) {
         return { success: false as const, error: new ValidationError(validation.error!) };
       }

src/cli/commands/deploy/__tests__/deploy.test.ts

@@ -2,7 +2,7 @@ import { runCLI } from '../../../../test-utils/index.js';
 import { runDeploy, runDiff, selectTargetStack } from '../actions.js';
 import { StackSelectionStrategy } from '@aws-cdk/toolkit-lib';
 import { randomUUID } from 'node:crypto';
-import { mkdir, rm, writeFile } from 'node:fs/promises';
+import { mkdir, readFile, rm, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { afterAll, beforeAll, describe, expect, it } from 'vitest';
@@ -59,6 +59,88 @@ describe('deploy without agents', () => {
   });
 });
 
+// Regression for the preview-mode vpcId backfill: `deploy --dry-run` on a pre-existing Container+VPC
+// config missing vpcId resolves + writes it to disk for synth, but must revert on EVERY exit path —
+// including when synth/bootstrap fails (no real creds here) — so a preview never dirties the tree.
+describe('deploy --dry-run preview does not dirty a vpcId-less Container+VPC config', () => {
+  let testDir: string;
+  let projectDir: string;
+  let agentConfigPath: string;
+
+  beforeAll(async () => {
+    testDir = join(tmpdir(), `agentcore-deploy-preview-${randomUUID()}`);
+    await mkdir(testDir, { recursive: true });
+
+    const projectName = 'PreviewVpcProj';
+    const dockerfile = join(testDir, 'Dockerfile');
+    await writeFile(dockerfile, 'FROM public.ecr.aws/lambda/python:3.12\n');
+
+    // Container + VPC agent, created WITH a vpcId...
+    const create = await runCLI(
+      [
+        'create',
+        '--project-name',
+        projectName,
+        '--name',
+        'pvagent',
+        '--build',
+        'Container',
+        '--network-mode',
+        'VPC',
+        '--subnets',
+        'subnet-0123456789abcdef0',
+        '--security-groups',
+        'sg-0123456789abcdef0',
+        '--vpc-id',
+        'vpc-0123456789abcdef0',
+        '--language',
+        'Python',
+        '--framework',
+        'Strands',
+        '--model-provider',
+        'Bedrock',
+        '--memory',
+        'none',
+      ],
+      testDir
+    );
+    if (create.exitCode !== 0) {
+      throw new Error(`Failed to create project: ${create.stdout} ${create.stderr}`);
+    }
+    projectDir = join(testDir, projectName);
+    agentConfigPath = join(projectDir, 'agentcore', 'agentcore.json');
+
+    await writeFile(
+      join(projectDir, 'agentcore', 'aws-targets.json'),
+      JSON.stringify([{ name: 'default', account: '123456789012', region: 'us-east-1' }])
+    );
+
+    // ...then strip the vpcId to simulate a config written before the field existed.
+    const spec = JSON.parse(await readFile(agentConfigPath, 'utf-8'));
+    for (const rt of spec.runtimes ?? []) {
+      if (rt.networkConfig) delete rt.networkConfig.vpcId;
+    }
+    await writeFile(agentConfigPath, JSON.stringify(spec, null, 2));
+  });
+
+  afterAll(async () => {
+    await rm(testDir, { recursive: true, force: true });
+  });
+
+  it('leaves agentcore.json unchanged after a --dry-run (even when the preview fails without creds)', async () => {
+    const before = await readFile(agentConfigPath, 'utf-8');
+    expect(before).not.toContain('vpcId');
+
+    // No real AWS creds/bootstrap in CI, so the dry-run will fail somewhere after the backfill (at
+    // DescribeSubnets or synth). Either way the finally-block restore must revert the file.
+    await runCLI(['deploy', '--dry-run', '--json'], projectDir);
+
+    const after = await readFile(agentConfigPath, 'utf-8');
+    expect(after, 'preview must not persist the backfilled vpcId').toBe(before);
+    expect(after).not.toContain('vpcId');
+  });
+});
+
 describe('selectTargetStack', () => {
   // Multi-target projects synth one stack per target in aws-targets.json. The deploy flow
   // must persist/describe the stack for the *deployed* target — not blindly stackNames[0].

src/cli/commands/deploy/actions.ts

@@ -160,6 +160,10 @@ export async function runDeploy(toolkitWrapper: CdkToolkitWrapper, stackName: st
 export async function handleDeploy(options: ValidatedDeployOptions): Promise<DeployResult> {
   let toolkitWrapper = null;
   let restoreEnv: (() => void) | null = null;
+  // In preview modes (--dry-run / --diff) the vpcId backfill writes to agentcore.json/harness.json so
+  // the synth subprocess can read it; this reverts those writes on EVERY exit path (success, early
+  // return, or throw) so a preview never leaves the working tree dirty. No-op on a real deploy.
+  let previewRestore: (() => Promise<void>) | null = null;
   const logger = new ExecLogger({ command: 'deploy' });
   const { onProgress } = options;
   let currentStepName = '';
@@ -411,10 +415,15 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
     }
 
     // Backfill vpcId for pre-existing Container+VPC configs written before vpcId was required. This
-    // resolves the VPC from the subnets (ec2:DescribeSubnets) and persists it to disk so the CDK
-    // synth process — which re-reads agentcore.json / harness.json — has the value it needs. Fresh
-    // creates already carry a vpcId, so this is a no-op for them.
-    const backfill = await backfillContainerVpcIds(configIO, context.projectSpec, target.region);
+    // resolves the VPC from the subnets (ec2:DescribeSubnets) and writes it to disk so the CDK synth
+    // process — which re-reads agentcore.json / harness.json — has the value it needs. Fresh creates
+    // already carry a vpcId, so this is a no-op for them. In preview modes (--dry-run / --diff) the
+    // write is reverted after synth via backfill.restore() so a preview leaves the working tree clean.
+    const isPreview = !!options.plan || !!options.diff;
+    const backfill = await backfillContainerVpcIds(configIO, context.projectSpec, target.region, !isPreview);
+    // In preview mode, revert the on-disk backfill in `finally` so every exit path (including a synth
+    // throw or an early return) leaves the working tree clean. restore() is a no-op on a real deploy.
+    if (isPreview) previewRestore = backfill.restore;
     if (backfill.backfilled.length > 0) {
       logger.log(`Resolved networkConfig.vpcId for Container+VPC build(s): ${backfill.backfilled.join(', ')}`, 'info');
     }
@@ -477,7 +486,8 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
     }
     endStep('success');
 
-    // Plan mode: stop after synth and checks, don't deploy
+    // Plan mode: stop after synth and checks, don't deploy. The backfilled vpcId written to disk for
+    // synth is reverted in the `finally` (covers this and every other preview exit path).
     if (options.plan) {
       logger.finalize(true);
       await toolkitWrapper.dispose();
@@ -490,7 +500,8 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
       };
     }
 
-    // Diff mode: run cdk diff and exit without deploying
+    // Diff mode: run cdk diff and exit without deploying. Like plan mode, the backfilled vpcId is
+    // reverted in the `finally`, so even a throw in runDiff leaves the working tree clean.
     if (options.diff) {
       startStep('Run CDK diff');
       await runDiff(toolkitWrapper, stackName, switchableIoHost);
@@ -978,8 +989,23 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
     logger.finalize(false);
     return { success: false, error: toError(err), logPath: logger.getRelativeLogPath() };
   } finally {
+    // Each cleanup step must run regardless of whether an earlier one fails — a throw from
+    // dispose() (common after a synth/bootstrap failure on a creds-less preview) must not skip the
+    // vpcId-backfill revert or the env restore. Isolate each with try/catch.
     if (toolkitWrapper) {
-      await toolkitWrapper.dispose();
+      try {
+        await toolkitWrapper.dispose();
+      } catch (disposeErr) {
+        logger.log(`CDK toolkit dispose failed: ${getErrorMessage(disposeErr)}`, 'warn');
+      }
+    }
+    // Revert the preview-mode vpcId backfill on every exit path (success, early return, or throw).
+    if (previewRestore) {
+      try {
+        await previewRestore();
+      } catch (restoreErr) {
+        logger.log(`Failed to revert preview vpcId backfill: ${getErrorMessage(restoreErr)}`, 'warn');
+      }
     }
     restoreEnv?.();
   }

src/cli/operations/agent/generate/schema-mapper.ts

@@ -144,7 +144,9 @@ export function mapGenerateConfigToAgent(config: GenerateConfig): AgentEnvSpec {
         networkConfig: {
           subnets: config.subnets,
           securityGroups: config.securityGroups,
-          ...(config.vpcId && { vpcId: config.vpcId }),
+          // Only a Container build carries a vpcId; guard so a stale value left over from a
+          // Container→CodeZip switch in the wizard doesn't leak into a CodeZip networkConfig.
+          ...(config.buildType === 'Container' && config.vpcId && { vpcId: config.vpcId }),
         },
       }),
     ...(headerAllowlist.length > 0 && {