fix(vpc): fix 3 bug-bash-found defects in create-harness VPC + backfill (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

@@ -169,7 +169,11 @@ async function handleCreateHarnessCLI(options: CreateOptions): Promise<void> {
         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,
@@ -216,7 +220,11 @@ async function handleCreateHarnessCLI(options: CreateOptions): Promise<void> {
           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,

src/cli/commands/deploy/actions.ts

@@ -411,10 +411,12 @@ 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);
     if (backfill.backfilled.length > 0) {
       logger.log(`Resolved networkConfig.vpcId for Container+VPC build(s): ${backfill.backfilled.join(', ')}`, 'info');
     }
@@ -477,8 +479,10 @@ 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. Revert any vpcId the backfill wrote to
+    // disk for synth — a preview must not dirty the working tree.
     if (options.plan) {
+      await backfill.restore();
       logger.finalize(true);
       await toolkitWrapper.dispose();
       toolkitWrapper = null;
@@ -490,12 +494,14 @@ 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. Revert any vpcId the backfill wrote to disk
+    // for synth — like plan mode, a diff preview must not dirty the working tree.
     if (options.diff) {
       startStep('Run CDK diff');
       await runDiff(toolkitWrapper, stackName, switchableIoHost);
       endStep('success');
 
+      await backfill.restore();
       logger.finalize(true);
       await toolkitWrapper.dispose();
       toolkitWrapper = null;

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 && {

src/cli/operations/deploy/__tests__/backfill-vpc-id.test.ts

@@ -1,6 +1,10 @@
 import type { ConfigIO } from '../../../../lib';
 import type { AgentCoreProjectSpec, HarnessSpec } from '../../../../schema';
 import { backfillContainerVpcIds } from '../backfill-vpc-id';
+import { randomUUID } from 'node:crypto';
+import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 const { mockResolveVpcId } = vi.hoisted(() => ({ mockResolveVpcId: vi.fn() }));
@@ -147,4 +151,61 @@ describe('backfillContainerVpcIds', () => {
     expect(harnessSpecs.h2.networkConfig?.vpcId).toBe('vpc-0123456789abcdef0');
     expect(result.backfilled).toEqual(['h2']);
   });
+
+  // BUG-2 regression: in preview mode (persist=false) the resolved vpcId is written to disk so the
+  // synth subprocess can read it, but restore() must revert the file so a --dry-run/--diff preview
+  // leaves the working tree untouched.
+  describe('dry-run (persist=false) restore', () => {
+    it('writes vpcId for synth then restore() reverts agentcore.json to its original bytes', async () => {
+      const dir = mkdtempSync(join(tmpdir(), `backfill-${randomUUID()}-`));
+      const agentPath = join(dir, 'agentcore.json');
+      const original = JSON.stringify({ name: 'proj', runtimes: [{ name: 'a' }] }, null, 2);
+      writeFileSync(agentPath, original, 'utf-8');
+
+      const runtime = containerVpcRuntime();
+      const spec = { name: 'proj', runtimes: [runtime], harnesses: [] } as unknown as AgentCoreProjectSpec;
+      // ConfigIO stub whose writeProjectSpec writes the real file at agentPath (what CDK synth reads).
+      const configIO = {
+        getAgentConfigPath: () => agentPath,
+        getHarnessConfigPath: (n: string) => join(dir, `${n}.json`),
+        writeProjectSpec: (s: AgentCoreProjectSpec) => {
+          writeFileSync(agentPath, JSON.stringify(s, null, 2), 'utf-8');
+          return Promise.resolve();
+        },
+        readHarnessSpec: () => Promise.reject(new Error('no harness')),
+      } as unknown as ConfigIO;
+
+      const result = await backfillContainerVpcIds(configIO, spec, 'us-east-1', false);
+
+      // While synth would run, the resolved vpcId is on disk...
+      expect(readFileSync(agentPath, 'utf-8')).toContain('vpc-0123456789abcdef0');
+      expect(result.backfilled).toEqual(['agent1']);
+
+      // ...but after restore() the file is byte-for-byte what it was before the preview.
+      await result.restore();
+      expect(readFileSync(agentPath, 'utf-8')).toBe(original);
+    });
+
+    it('persist=true leaves the value on disk (no restore) — restore() is a no-op', async () => {
+      const dir = mkdtempSync(join(tmpdir(), `backfill-${randomUUID()}-`));
+      const agentPath = join(dir, 'agentcore.json');
+      writeFileSync(agentPath, JSON.stringify({ name: 'proj', runtimes: [{ name: 'a' }] }, null, 2), 'utf-8');
+
+      const runtime = containerVpcRuntime();
+      const spec = { name: 'proj', runtimes: [runtime], harnesses: [] } as unknown as AgentCoreProjectSpec;
+      const configIO = {
+        getAgentConfigPath: () => agentPath,
+        getHarnessConfigPath: (n: string) => join(dir, `${n}.json`),
+        writeProjectSpec: (s: AgentCoreProjectSpec) => {
+          writeFileSync(agentPath, JSON.stringify(s, null, 2), 'utf-8');
+          return Promise.resolve();
+        },
+        readHarnessSpec: () => Promise.reject(new Error('no harness')),
+      } as unknown as ConfigIO;
+
+      const result = await backfillContainerVpcIds(configIO, spec, 'us-east-1', true);
+      await result.restore(); // no-op on a real deploy
+      expect(readFileSync(agentPath, 'utf-8')).toContain('vpc-0123456789abcdef0');
+    });
+  });
 });

src/cli/operations/deploy/backfill-vpc-id.ts

@@ -2,31 +2,55 @@ import type { ConfigIO } from '../../../lib';
 import type { AgentCoreProjectSpec } from '../../../schema';
 import { isContainerBuild } from '../../../schema/constants';
 import { resolveVpcIdFromSubnets } from '../../commands/shared/vpc-utils';
+import { readFile, writeFile } from 'fs/promises';
 
 export interface BackfillVpcIdResult {
-  /** Names of runtimes/harnesses whose vpcId was resolved and persisted this run. */
+  /** Names of runtimes/harnesses whose vpcId was resolved this run. */
   backfilled: string[];
+  /**
+   * Restore the config files this call rewrote to their original on-disk bytes. Populated only when
+   * `persist` is false (dry-run/plan): the value is still written to disk so the CDK synth subprocess
+   * can read it, then reverted so a preview leaves the working tree untouched. No-op on a real deploy.
+   */
+  restore: () => Promise<void>;
 }
 
 /**
- * Backfill `networkConfig.vpcId` for any Container+VPC runtime or harness that is missing it, then
- * persist the change to disk so CDK synth (a separate process that re-reads agentcore.json /
- * harness.json) sees the resolved value.
+ * Backfill `networkConfig.vpcId` for any Container+VPC runtime or harness that is missing it.
  *
  * vpcId became required for Container+VPC builds after this feature shipped, but agentcore.json
  * files written before then have only subnets + security groups. Rather than hard-fail those
  * configs on read, deploy resolves the VPC from the first subnet via ec2:DescribeSubnets — a subnet
- * uniquely determines its VPC — and writes it back once. Fresh creates already collect --vpc-id at
- * the CLI layer, so this is a no-op for them.
+ * uniquely determines its VPC. Fresh creates already collect --vpc-id at the CLI layer, so this is a
+ * no-op for them.
  *
- * @returns the names that were backfilled (empty if none needed it).
+ * CDK synth is a separate process that re-reads agentcore.json / harness.json from disk, so the
+ * resolved value must be written there. On a real deploy (`persist: true`) the write is permanent.
+ * On a preview (`persist: false` — dry-run/plan) the write is temporary: the returned `restore()`
+ * reverts the files to their original bytes after synth, honoring the "preview without deploying"
+ * contract (the working tree is left untouched).
+ *
+ * @returns the names that were backfilled and a `restore()` callback (a no-op when persist is true).
  */
 export async function backfillContainerVpcIds(
   configIO: ConfigIO,
   projectSpec: AgentCoreProjectSpec,
-  region: string
+  region: string,
+  persist = true
 ): Promise<BackfillVpcIdResult> {
   const backfilled: string[] = [];
+  // path -> original bytes, captured before the first rewrite of each file (preview mode only, so
+  // restore() can revert them). Never touched on a real deploy (persist=true).
+  const originals = new Map<string, string>();
+
+  // Snapshot a file's current bytes before we rewrite it — but only in preview mode. Computes the
+  // path lazily so a real deploy never calls the path getter (keeps the persist=true path minimal).
+  const snapshot = async (getPath: () => string): Promise<void> => {
+    if (persist) return;
+    const path = getPath();
+    if (originals.has(path)) return;
+    originals.set(path, await readFile(path, 'utf-8'));
+  };
 
   // Runtimes live inline in agentcore.json; mutate the spec and write it back once at the end if any
   // changed.
@@ -40,6 +64,7 @@ export async function backfillContainerVpcIds(
     }
   }
   if (runtimesChanged) {
+    await snapshot(() => configIO.getAgentConfigPath());
     await configIO.writeProjectSpec(projectSpec);
   }
 
@@ -49,10 +74,17 @@ export async function backfillContainerVpcIds(
     const nc = harness.networkConfig;
     if (isContainerBuild(harness) && harness.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) {
       nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region);
+      await snapshot(() => configIO.getHarnessConfigPath(entry.name));
       await configIO.writeHarnessSpec(entry.name, harness);
       backfilled.push(entry.name);
     }
   }
 
-  return { backfilled };
+  const restore = async (): Promise<void> => {
+    for (const [path, content] of originals) {
+      await writeFile(path, content, 'utf-8');
+    }
+  };
+
+  return { backfilled, restore };
 }

src/cli/tui/screens/generate/GenerateWizardUI.tsx

@@ -690,7 +690,7 @@ function ConfirmView({ config, credentialProjectName }: { config: GenerateConfig
             <Text>{config.securityGroups.join(', ')}</Text>
           </Text>
         )}
-        {config.networkMode === 'VPC' && config.vpcId && (
+        {config.networkMode === 'VPC' && config.buildType === 'Container' && config.vpcId && (
           <Text>
             <Text dimColor>VPC ID: </Text>
             <Text>{config.vpcId}</Text>

src/lib/schemas/io/config-io.ts

@@ -105,6 +105,20 @@ export class ConfigIO {
     return this.pathResolver.getBaseDir();
   }
 
+  /**
+   * Absolute path to the project config file (agentcore.json).
+   */
+  getAgentConfigPath(): string {
+    return this.pathResolver.getAgentConfigPath();
+  }
+
+  /**
+   * Absolute path to a harness's config file (app/<name>/harness.json).
+   */
+  getHarnessConfigPath(harnessName: string): string {
+    return this.pathResolver.getHarnessConfigPath(harnessName);
+  }
+
   /**
    * Read and validate the project configuration.
    */