fix(cdk): pin VPC to AgentCore-supported availability zones

[AshrafBen10]

Summary

Adds an availabilityZones prop to AgentVpc and wires a CDK context key (agentcore:availabilityZones) so affected accounts can pin the VPC subnets to AZ names that map to AgentCore-supported physical zone IDs.

Problem

AgentCore only supports a subset of physical AZs per region (for us-east-1: use1-az1, use1-az2, use1-az4). AZ names are aliased per-account, so the default maxAzs: 2 selection can land in an unsupported zone, causing the AWS::BedrockAgentCore::Runtime resource to fail with NotStabilized and rolling back the entire stack.

Changes

• cdk/src/constructs/agent-vpc.ts — Added optional availabilityZones prop that takes precedence over maxAzs when provided.
• cdk/src/stacks/agent.ts — Reads agentcore:availabilityZones from CDK context and passes to AgentVpc.
• cdk/test/constructs/agent-vpc.test.ts — Added tests for the new prop (explicit AZs override maxAzs, 3-zone case).

Usage for affected accounts

# Discover your AZ mapping
aws ec2 describe-availability-zones --region us-east-1 \
  --query 'AvailabilityZones[].[ZoneName,ZoneId]' --output text

# Deploy with pinned AZs (choose names mapping to use1-az1, use1-az2, use1-az4)
cdk deploy -c agentcore:availabilityZones='["us-east-1b","us-east-1c"]'

Or in cdk.context.json:

{
  "agentcore:availabilityZones": ["us-east-1b", "us-east-1c"]
}

Testing

• All 15 AgentVpc tests pass (100% coverage)
• TypeScript compiles cleanly

Closes #353

[isadeks]

Review: fix(cdk): pin VPC to AgentCore-supported availability zones (#353)

The fix is correct and does what it claims — verified empirically against aws-cdk-lib@2.257.0:

• ✅ Works for the env-agnostic production stack. Passing explicit availabilityZones bakes the literal AZ names (us-east-1b, us-east-1c) straight into the CloudFormation subnets, whereas the original maxAzs path emits Fn::Select/Fn::GetAZs tokens that CloudFormation resolves nondeterministically at deploy time — exactly the cdk: deploy fails when VPC lands in an AgentCore-unsupported availability zone #353 failure mode. Pinning genuinely makes the deploy deterministic.
• ✅ The spread approach is valid. VpcProps.availabilityZones is a real prop, mutually exclusive with maxAzs (CDK throws if both are set), and ...(cond ? {availabilityZones} : {maxAzs}) correctly passes exactly one.
• ✅ Consistent with repo conventions — the tryGetContext(...) as T pattern matches how blueprintRepo, stackName, compute_type, and github:* are already read.

(I went in expecting the known CDK gotcha — a subset-of-stack-AZs validation throw on an env-agnostic stack — but reproduced that it does not throw in the production configuration. No correctness bug in the happy path.)

Two low-severity findings, neither blocking:

🔧 1. Unvalidated context cast (agent.ts:216)

const agentCoreAzs = this.node.tryGetContext('agentcore:availabilityZones') as string[] | undefined;

The as string[] cast trusts the operator to pass valid JSON. The intuitive shorthand -c agentcore:availabilityZones=us-east-1b (a bare string, not the JSON-array form the docs show) sails through the cast, gets spread into availabilityZones, and makes CDK throw:

this.availabilityZones.forEach is not a function

…a synth error that names neither the context key nor the expected shape. Since the operator hitting this is already mid-firefight over AZs (the whole point of the feature), a one-line guard is worth it:

if (agentCoreAzs !== undefined && !Array.isArray(agentCoreAzs)) {
  throw new Error("Context 'agentcore:availabilityZones' must be a JSON array of AZ names, e.g. -c agentcore:availabilityZones='[\"us-east-1b\",\"us-east-1c\"]'");
}

🔧 2. Test coverage gap (agent-vpc.test.ts:152)

The new tests deploy into a concrete-env stack (env: { region: 'us-east-1' }), but AgentStack is env-agnostic in production (main.ts:26, account/region from CDK_DEFAULT_*). The two branches happen to produce identical output today (I verified both bake in literal AZ names), so this isn't a current bug — but the tests don't exercise the path production actually synthesizes in, so a future CDK upgrade that changes env-agnostic AZ handling could regress production while these stay green. Worth adding an env-agnostic case that asserts the literal AZ names land in the subnets.

---

Both are nits — the PR is fundamentally sound and safe to merge.

Reviewed at xhigh effort. The fix mechanism (literal AZs vs Fn::GetAZs tokens), VpcProps.availabilityZones/maxAzs mutual exclusion, the env-agnostic synth behavior, and the bare-string crash all verified by reproduction against aws-cdk-lib@2.257.0.

[krokoko]

Thanks for tackling this — the root cause analysis is spot on, and the approach (pinning AgentVpc via CDK context so the template gets literal AZ names instead of Fn::GetAZs) is a valid escape hatch. The CDK wiring (availabilityZones vs maxAzs) also looks correct.

I'm requesting changes because, as shipped, this doesn't yet meet the bar for how we should handle AgentCore's AZ constraint in this sample. AWS documents the requirement in terms of physical zone IDs (e.g. use1-az1 / use1-az2 / use1-az4 in us-east-1), and AZ names are aliased per account — see:

• Configure AgentCore Runtime for VPC — Supported Availability Zones
• Availability Zone IDs

Issue #353 itself preferred auto-selecting supported zone IDs so a fresh deploy succeeds without account-specific guesswork. A manual name pin alone leaves the default path broken for affected accounts.

What to fix to make this right

1. Prefer auto-pin from AgentCore-supported zone IDs (default path)
   At synth time (we already need ec2:DescribeAvailabilityZones), resolve the account's name↔ID mapping, intersect with AgentCore's supported zone-ID set for the region, and pass those AZ names into ec2.Vpc({ availabilityZones }). Keep agentcore:availabilityZones only as an optional override. That matches AWS's model and closes the bug without per-account tribal knowledge.

2. Validate the context override
   Mirror the pattern in resolveBedrockModelIds (bedrock-models.ts): reject non-arrays / bare strings with a clear synth error naming the key and expected JSON shape. Prefer requiring ≥2 zones (AgentCore HA guidance).

3. Document the operator path
   Even if the escape hatch remains, add a short note to the deploy / quick-start / troubleshooting docs (discover mapping → pick supported IDs → set context). An override buried only in construct comments won't help someone mid-rollback.

4. Harden tests
   Assert subnet AvailabilityZone properties are the pinned names (not only subnet count), and add an env-agnostic case since production AgentStack synthesizes that way.

5. Avoid hardcoding only us-east-1 IDs in comments as if universal
   Supported sets differ by region and can change; prefer linking the AgentCore AZ table (and/or a small per-region constant map used by the auto-select logic).

Happy to re-review once the default path selects supported zone IDs (with the context override as a safety valve), validation, docs, and stronger tests are in place. Appreciate the careful write-up and the unblock for #353.

[AshrafBen10]

@krokoko thanks for the detailed review — pushed 392b55e reworking this from a manual pin into auto-select-by-default. Mapping to your points:

1. Auto-pin from supported zone IDs (default path). New cdk/src/constructs/agentcore-azs.ts resolves AZs at synth: the validated override wins, otherwise — when synth has a concrete account+region — it calls DescribeAvailabilityZones, intersects the account's name→zone-ID mapping with the region's supported set, and passes the resulting AZ names into ec2.Vpc({ availabilityZones }). main.ts is now async and threads the result through AgentStackProps.availabilityZones → AgentVpc.

One nuance worth flagging: this repo's pipeline pre-synthesizes env-agnostic in build.yml (no bound account) and deploy.yml deploys that artifact, so auto-pin takes effect for a local/dev cdk deploy (concrete account), while the override remains the pin for the CI-built artifact. That lines up with your item #4 note that the production stack synthesizes env-agnostic. Happy to also wire a creds-bound synth into the pipeline if you'd prefer auto-pin to cover that path too.

2. Validate the override. resolveAgentCoreAzOverride mirrors resolveBedrockModelIds — rejects non-arrays, empty/non-string entries, and fewer than two zones (HA), with a synth error naming the key and expected JSON shape.

3. Docs. Added a "Known deployment issues" subsection in DEPLOYMENT_GUIDE.md (symptom → root cause → auto-pin default → discover-mapping → pick supported IDs → set context), plus a QUICK_START.mdx note and a troubleshooting row for the NotStabilized / "unsupported availability zones" symptom.

4. Harden tests. agent-vpc.test.ts now asserts subnet AvailabilityZone equals the pinned names (not just subnet count) and adds an env-agnostic Fn::GetAZs fallback case. agentcore-azs.test.ts covers the per-region map, override validation, the pure selection function, and every resolver branch (override wins, env-agnostic, unknown region, auto-pin, <2 supported, lookup failure).

5. Per-region map, not us-east-1-only. AGENTCORE_SUPPORTED_AZ_IDS is a per-region constant map sourced from the AgentCore Supported Availability Zones table (9 regions), and the construct/module comments link that table rather than presenting us-east-1 IDs as universal.

Implementation note: added @aws-sdk/client-ec2 (loaded via a lazy dynamic import so it's only pulled in when auto-pin actually runs). Ready for another look.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.75309% with 17 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@22705e8). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
cdk/src/constructs/agentcore-azs.ts	93.38%	17 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #358   +/-   ##
=======================================
  Coverage        ?   89.33%           
=======================================
  Files           ?      225           
  Lines           ?    54676           
  Branches        ?     5212           
=======================================
  Hits            ?    48847           
  Misses          ?     5829           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[isadeks]

Re-review of a7f04df — the auto-pin rework

The rework from a manual pin into synth-time auto-selection is a real improvement and maps cleanly onto krokoko's five CHANGES_REQUESTED points. I checked it out and ran it — agentcore-azs.test.ts + agent-vpc.test.ts 33/33, tsc --noEmit clean, and I verified the behaviors below by synth/unit probes rather than reading. It also resolves both nits from my earlier review (context validation + env-agnostic test). Nice work, and the loud-fail resolveAgentCoreAzOverride mirroring resolveBedrockModelIds is the right shape.

One finding is worth fixing before merge because it breaks the exact operator recovery path this feature exists for; the rest are minor.

1. The documented -c 'agentcore:availabilityZones=[...]' escape hatch throws (should-fix — it's the mid-rollback recovery path)

resolveAgentCoreAzOverride rejects anything that isn't Array.isArray(...). But CDK delivers a -c key=value context value as a raw string, not parsed JSON — so the CLI form throws:

$ cdk deploy -c 'agentcore:availabilityZones=["us-east-1b","us-east-1c"]'
Error: Context 'agentcore:availabilityZones' must be a JSON array of availability-zone names …;
       got "[\"us-east-1b\",\"us-east-1c\"]".

Reproduced both ways: the cdk.context.json form (a real array) works; the -c CLI form throws. This matters because the PR description's "Usage" block leads with the -c form, and an operator hitting the AgentCore AZ rollback — the whole reason this feature exists — is the one most likely to reach for -c mid-firefight. It's the same crash I flagged in my June review (then .forEach is not a function); the new validator turns it into a clearer message but still doesn't accept the input the docs advertise.

Fix: when the context value is a string, JSON.parse it before the array check (fall back to treating a bare string as a single malformed entry so the error still fires for true typos). Then the -c and cdk.context.json forms behave identically. (resolveBedrockModelIds has the same structural limitation, but its docs don't advertise a -c form, so this PR is the one that creates the mismatch — worth fixing here and arguably worth a follow-up for the shared pattern.)

2. Auto-pin silently widens the VPC from 2 AZs to all supported zones (often 3) (document / confirm intent)

selectSupportedAzNames returns every matching zone, so on a typical us-east-1 account auto-pin yields 3 AZs where maxAzs: 2 previously gave 2 → 6 subnets instead of 4. I confirmed NAT gateways stay at 1 (the construct default), so there's no NAT/EIP cost blowup — just more subnets (free) and a wider ENI spread. Benign, but it's a silent topology change for accounts that deploy fine today, and it's not called out. Either cap the auto-pin at 2 (matching the old default and the MIN_AGENTCORE_AZS HA floor) or note in the docs that auto-pin uses all supported zones.

3. The CI-deployed stack still isn't auto-fixed — only local cdk deploy is (doc clarity)

Verified the deploy path: build.yml synthesizes cdk.out with persist-credentials: false and no aws-region, so the artifact is env-agnostic → resolveAgentCoreAzs returns undefined → the CI-deployed stack falls back to Fn::GetAZs/maxAzs, unpinned. deploy.yml then deploys that artifact (--app cdk/cdk.out). The DEPLOYMENT_GUIDE does say env-agnostic synth needs the override, so this is documented, not a bug — but the framing ("Default behavior (auto-pin)… No action needed") reads as if the common case is covered, when the CI/CD-deployed stack (the one most teams run) still needs the manual override. Worth making that contrast sharper so a team relying on deploy.yml doesn't assume they're protected.

Minor

• Override isn't cross-checked against AGENTCORE_SUPPORTED_AZ_IDS — a typo'd but well-formed override (["us-east-1x","us-east-1y"]) is passed straight to ec2.Vpc, deferring the failure to deploy time. Defensible (the override is a deliberate escape hatch, and names→IDs are account-specific so a name-level check can't validate zone-ID support), but a soft synth warning when an override's resolved IDs aren't in the supported set would catch fat-fingers earlier. Optional.
• defaultDescribeAzs (lines 153-169) is the one uncovered block — fine, since it's the injected live-call boundary and tests substitute it.

Verification

Ran at a7f04df: 33/33 tests, tsc clean. Reproduced: -c string form throws vs cdk.context.json array works (#1); auto-pin returns 3 zones / 6 subnets / 1 NAT (#2); env-agnostic synth returns undefined and build.yml synths credential-less (#3); well-formed-but-unsupported override passes unchecked (minor). Selection preserves the account's zone order (deterministic given a stable DescribeAZ response).

Recommendation: fix #1 before merge (small change, unblocks the documented recovery path); #2/#3 are doc/clarity items; the rest optional. Approach and structure are sound.

[AshrafBen10]

Thanks for the thorough re-review and for actually running it — pushed 2891801 on top of the latest main merge.

#1 (should-fix) — -c string override now works. You're right, this broke the exact recovery path the feature exists for. resolveAgentCoreAzOverride now JSON.parses a string context value before the array check, so -c 'agentcore:availabilityZones=["us-east-1b","us-east-1c"]' and the cdk.context.json array form behave identically. A non-JSON string (a real typo) is left as-is and still fails the array check with the same clear, key-named error, so typos keep erroring. Added tests for the JSON-string success path and a JSON-string-that-isn't-an-array (throws). Left resolveBedrockModelIds alone since its docs don't advertise -c — happy to file a follow-up for the shared pattern.

#2 — auto-pin no longer widens topology. Capped auto-pin to the first MIN_AGENTCORE_AZS (2) supported zones, matching AgentVpc's default maxAzs, so an account that deploys fine today stays at 2 AZs / 4 subnets instead of silently going to 3 / 6. Updated the auto-pin test to assert the cap (3 supported → 2 pinned).

#3 — sharpened the CI/CD doc contrast. Reworked the deployment-guide section: the "auto-pin" heading now reads "local deploy only," and there's a dedicated "The CI/CD-deployed stack is NOT auto-pinned" paragraph spelling out that build.yml synthesizes credential-less/env-agnostic and deploy.yml ships that artifact, so pipeline-deploying teams must set the override. Also added the -c example now that it parses.

Minors:

• Override-vs-AGENTCORE_SUPPORTED_AZ_IDS cross-check: I left this out deliberately. The override is the escape hatch precisely for env-agnostic/pipeline deploys where there's no bound account, so validating the resolved zone IDs would require a live DescribeAvailabilityZones and break the offline/CI use case. As you noted, a name-level check can't validate zone-ID support anyway. Deploy-time remains the failure point for a well-formed-but-wrong override.
• defaultDescribeAzs coverage: agreed, that's the injected live-call boundary; tests substitute it.

Verification on 2891801: agentcore-azs + agent-vpc + agent stack tests green (35 AZ-module tests incl. the new -c cases), tsc clean, eslint clean, AI004 masking scan clean, docs mirror synced. Ready for another look.

[isadeks]

Re-review of 2891801 — all findings addressed, approving

Checked out and ran it. All three findings from the last review are fixed and verified empirically (not just from the reply):

#1 — -c JSON-string override now works. resolveAgentCoreAzOverride JSON.parses a string context value before the array check. Verified at both layers:

• Unit: -c string form → ["us-east-1b","us-east-1c"]; array form unchanged; and the error paths all still fire — a bare-string typo, a JSON-but-non-array ('"us-east-1b"'), and a single-element array below the HA floor all throw the clear key-named error. So the recovery path works and typos still error.
• Integration: the exact resolveAgentCoreAzs call main.ts makes, fed the raw string CDK delivers for -c, now returns the array instead of throwing at synth entry (the crash from my last review is gone).

#2 — auto-pin no longer widens topology. names.slice(0, MIN_AGENTCORE_AZS) caps it. Verified: 3 supported zones → pins exactly 2 (["us-east-1a","us-east-1b"]), matching AgentVpc's default maxAzs. An account that deploys fine today stays at 2 AZs / 4 subnets.

#3 — CI/CD contrast sharpened. The doc now heads the default path "auto-pin — local deploy only" and adds a dedicated "The CI/CD-deployed stack is NOT auto-pinned" paragraph spelling out that build.yml synths credential-less and deploy.yml ships that artifact, so pipeline teams must set the override. Also documents the "first two" cap and adds the -c example. Reads clearly now.

Minors — both correctly dispositioned:

• Override-vs-supported-ID cross-check left out: agreed and well-reasoned — the override exists precisely for env-agnostic/pipeline deploys where there's no bound account to resolve zone IDs, so a live cross-check would break the offline use case, and a name-level check can't validate zone-ID support anyway. Deploy-time remains the failure point for a well-formed-but-wrong override, which is acceptable for an escape hatch.
• defaultDescribeAzs coverage: fine, it's the injected live-call boundary.

Verification on 2891801: 35 AZ-module + agent-vpc tests green, tsc --noEmit clean. The rework from a manual pin into auto-select-with-override is a solid outcome for #353 and matches AWS's zone-ID model.

LGTM — approving. (Optional follow-up, non-blocking: resolveBedrockModelIds shares the same -c-string-vs-array structural limitation; worth the same JSON.parse treatment if a -c form ever gets documented for it.)