feat: add China (aws-cn) region support with feature gates - #2426
Conversation
agentcore-devx-automation
Bot
added
agentcore-harness-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
![agentcore-devx-automation[bot]](https://avatars.githubusercontent.com/u/2232217?s=60&v=4){kind=small}
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice, self-contained aws-cn support. The two-layer defense for telemetry (default flipped in accessor.tsx, plus an unconditional runtime guard in client.tsx that catches an explicit telemetry.enabled: true in a China context) is a good choice, and the gating for model-provider templates fires before any scaffolding or dependency check — the tests confirm that checkedTools stays empty and the runtime directory isn't created. Coverage in partition.test.ts, manager.test.ts, and client.test.tsx uses real temp directories and env-var toggling rather than mocks — well within the project's testing guidance.
A few things I looked at that are fine as-is but worth calling out for future reference:
agentcore createintentionally scaffolds before targets exist, so a user in acn-*shell runningagentcore create --template agent-python-strandsstill gets a Bedrock-wired scaffold; the mitigation ("add runtime is where the gate fires") is documented in the README and in the manager comment.- The LiteLLM CN gate only checks that a
modelIdwas supplied, not that it routes to a CN-reachable provider — also called out in the error message and README. isChinaContext()walks up fromprocess.cwd()and short-circuits on env vars, so telemetry stays off even when invoked from outside a project as long asAWS_REGION/AWS_DEFAULT_REGIONiscn-*.
No changes required from me.
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2426 +/- ##
============================================
+ Coverage 97.24% 97.29% +0.04%
============================================
Files 609 611 +2
Lines 42923 43154 +231
============================================
+ Hits 41742 41987 +245
+ Misses 1181 1167 -14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
shawnxli
changed the title
Add cn-north-1 and cn-northwest-1 to AgentCoreRegionSchema and its region test. Without this, aws-cn deployment targets fail aws-targets.json validation, blocking every command that resolves a target in China regions.
In aws-cn, Amazon Bedrock is not launched and Anthropic/OpenAI/Gemini are not reachable, and no telemetry collector exists there. Gate the affected features with clear regional behavior instead of letting them fail downstream: - Runtime templates: FsProjectManager.addResource rejects framework templates wired to Bedrock/Anthropic/OpenAI/Gemini with RegionUnsupportedFeatureError before scaffolding when any deployment target is in a China region (covers the flag handler and the TUI wizard). Provider-free scaffolds (agent-python-minimal, mcp-python-fastmcp) stay available as the bring-your-own-implementation path. - LiteLLM stays available in China as the routable-provider escape hatch, but requires an explicit model id there: its default model id routes to Bedrock. New --model-id flag on 'add runtime' threads into the template render context (generic optional override; per-provider defaults are unchanged commercially). - Bedrock Agent import (--type import): the add-runtime handler fails fast before any Bedrock call. - Telemetry: unconditionally disabled in a China context — ambient AWS region env vars OR any cn-* deployment target in aws-targets.json — regardless of config or endpoint overrides; the local audit-file sink is unaffected. The global-config default also flips to disabled under a China ambient region. - Add isChinaRegion()/isChinaContext() partition helpers and RegionUnsupportedFeatureError; document the China behavior in README. Known gap (as on main): project create scaffolds before targets exist, so the guards fire at add runtime where targets are known.
The for(;;) loop only exited via return, leaving its closing brace unexecutable and flagged by coverage. Same behavior, now a bounded while walk followed by a straight-line targets read.
- Telemetry now resolves the region through the same chain the CLI uses (--region flag from argv, env vars, shared AWS config profile) before the China gate, instead of env vars only; resolveRegion moves from the withRegion middleware into src/core/region.ts and is shared. - A first run in a China environment persists telemetry disabled, so a later run in a commercial region cannot start exporting without the first-run notice ever having been shown; an explicit pre-existing telemetry.enabled is never clobbered. - create accepts --model-id and applies the China provider gate when the command's resolved region is a China region, so the common create-then-deploy workflow fails before scaffolding; the default (harness) project is rejected there too. - Scaffolds persist the wired modelProvider on the runtime entry in agentcore.json (optional field, ignored by the CDK app and older CLIs), and deploy to a China target hard-fails when any runtime carries a Bedrock/Anthropic/OpenAI/Gemini provider, with remediation; unclassifiable runtimes (no field) get an informational note; harness projects are rejected. The ModelProvider enum moves to projectSchemas/runtime.ts (re-exported) so the schema layer owns it. - --model-provider help text now lists LiteLLM; command.md regenerated. - isChinaRegion resolves the partition via @aws-sdk/util-endpoints partition data instead of a cn- name-prefix guess.
c140551 to
a8eb63c
Compare
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
Pre-existing on the refactor base (reproduces on a clean checkout): GHSA-qw65-cvwx-89v3 and GHSA-58mr-gqgx-xq4g against transitive fast-uri. Lockfile-only bump within existing ranges; bun audit is now clean.
|
fc8f4f0 also carries a fix found during live China E2E: China scaffolds no longer include the template's default AgentCore Memory resource — the |
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
| build: scaffoldRuntimeInput.build, | ||
| // Persist the provider the template wired into the code (framework | ||
| // scaffolds only) so the China deploy gate can classify this runtime later. | ||
| ...(scaffoldRuntimeInput.framework !== "none" && { |
There was a problem hiding this comment.
Small gap: imported Bedrock Agents use framework: "none", so this skips modelProvider even though importScaffoldRuntimeInput sets it to Bedrock. If a CN target is added later, deploy treats that runtime as unclassified and proceeds. Could we persist the provider for imports too and cover import → CN deploy?
There was a problem hiding this comment.
Fixed in 8884652 — buildRuntimeSpec now persists modelProvider: "Bedrock" when the scaffold is a Bedrock Agent import (keyed on importBedrockAgent, since imports share framework: "none" with genuinely provider-free scaffolds like minimal/MCP, which stay unclassified). The existing deploy hard-fail then covers import → CN target. The import test asserts the persisted provider.
| // the first-run notice is not shown there (telemetry is disabled), so | ||
| // without persisting, a later run in a commercial region would flip | ||
| // telemetry back on without the notice ever having been displayed. | ||
| if (inChinaRegionEnv() && configFileData.telemetry?.enabled === undefined) { |
There was a problem hiding this comment.
One small follow-up: this only looks at region env vars. On a first run where CN comes from --region or the active profile, the telemetry client suppresses export, but this accessor still persists only the installation ID and reports telemetry enabled. Could we use the resolved region here too?
There was a problem hiding this comment.
Fixed in 8884652 — the accessor now resolves China through the exact chain the telemetry client uses (--region from argv → env vars → shared config profile → project targets, via isChinaContext + resolveRegion), for both the first-run persist and the CN default. Tests added for a first run with China only from --region and only from the active profile.
…esolve region for telemetry persist Third review round: - addResource on a project with a China target rejects any resource family whose CloudFormation type is not registered there (allowlist: runtime, runtime-endpoint, credential, gateway, gateway-target); deploy to a China target fails fast when the spec contains any non-empty collection outside the supported set, instead of CloudFormation's opaque 'Unrecognized resource types' error. New families default to blocked until confirmed. - Bedrock Agent imports persist modelProvider: Bedrock (their translated code calls Bedrock despite framework 'none'), so a later China deploy hard-fails instead of passing as unclassified. Provider-free scaffolds (minimal, MCP) stay unclassified. - The global-config accessor resolves China through the same chain as the telemetry client (--region, env vars, shared config profile, project targets), so a first run with China only from --region or the active profile persists telemetry off.
…-refactor # Conflicts: # command.md
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
| // older CLI) cannot be classified and only get an informational note. | ||
| if (isChinaRegion(target.region)) { | ||
| const blocked = project.spec.runtimes.filter( | ||
| (runtime) => runtime.modelProvider !== undefined && runtime.modelProvider !== "LiteLLM", |
There was a problem hiding this comment.
Could we persist/check the model ID here too? A LiteLLM runtime scaffolded before the China target can still use the Bedrock default, but this check lets it through based only on modelProvider.
There was a problem hiding this comment.
Fixed in a1fb82e — LiteLLM scaffolds now persist the rendered model id alongside modelProvider in agentcore.json, and this gate hard-fails a LiteLLM runtime whose persisted id uses LiteLLM's bedrock/ route (the default when scaffolded without --model-id), naming the runtime and id in the error. A LiteLLM runtime without a persisted id (older CLI / hand-edited spec) joins the existing unclassified informational note. Also added the same bedrock/ rejection at create/add in a China context, so the commercial-scaffold-then-CN-deploy hole is closed at both ends. Verifying that a non-bedrock/ model id is actually reachable from China stays the user's responsibility, as documented in the README.
A LiteLLM runtime scaffolded without --model-id renders LiteLLM's default model id, whose 'bedrock/' prefix routes to Amazon Bedrock — unreachable from China. The deploy gate previously passed any LiteLLM runtime. - Scaffolds persist the rendered model id for LiteLLM runtimes alongside modelProvider in agentcore.json. - Deploy to a China target hard-fails a LiteLLM runtime whose persisted model id starts with 'bedrock/'; one without a persisted id joins the informational unclassified note. - create/add in a China context reject an explicit 'bedrock/'-prefixed --model-id before scaffolding. Reachability of other model ids from China remains the user's responsibility, as documented.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
notgitika
left a comment
notgitika
left a comment
There was a problem hiding this comment.
Looks good. The LiteLLM model ID is now persisted and rechecked at create/add and deploy time, which closes the China-target gap.
4 participants
feat: China (aws-cn) region support — region enum + feature gates
Base branch:
refactor· 2 commitsAmazon Bedrock AgentCore is live in
cn-north-1(BJS) andcn-northwest-1(ZHY), but the CLIrejects both regions at config-validation time and several features cannot work in that
partition. This PR adds the regions and gates the unavailable features with explicit regional
errors instead of downstream failures.
Commit 1 — add China regions to the supported region enum
cn-north-1,cn-northwest-1inAgentCoreRegionSchema(per the enum's source-of-truthcomment, matching the AgentCore regions documentation), following the
AGENTS.md§Multi-Partition checklist.
aws-targets.jsonvalidation anddetectRegion()silentlyfalls back to
us-east-1for users whose environment region is CN.Commit 2 — gate features unavailable in aws-cn
In China regions, Amazon Bedrock is not launched and Anthropic/OpenAI/Gemini are not reachable;
no telemetry collector exists there either.
Runtime templates —
FsProjectManager.addResourcerejects framework template scaffoldswired to Bedrock/Anthropic/OpenAI/Gemini with a
RegionUnsupportedFeatureErrorwhen anydeployment target is
cn-*(one chokepoint covers the flag handler and the TUI wizard).Provider-free scaffolds (
agent-python-minimal,mcp-python-fastmcp) remain available as thebring-your-own-implementation path.
LiteLLM + new
--model-idflag — LiteLLM can route to providers reachable from China, so--model-provider litellmstays allowed there, but requires an explicit--model-id: thetemplate default routes to Bedrock.
--model-idis implemented as a generic optional overridethreaded into the template render context; every provider's commercial default model id is
unchanged (rendered output is byte-identical without the flag).
Bedrock Agent import —
add runtime --type importfails fast with a regional messagebefore any Bedrock call.
Telemetry — unconditionally disabled in a China context, regardless of config or endpoint
overrides, to comply with restrictions on sending telemetry out of the region. "China context"
= ambient
AWS_REGION/AWS_DEFAULT_REGIONiscn-*OR the enclosing project declares acn-*deployment target inaws-targets.json(best-effort read; telemetry never affects CLIbehavior). The local audit-file sink is unaffected. The global-config default also flips to
disabled under a China ambient region.
Docs — README gains a "China (aws-cn) regions" section;
command.mdregenerated.Known gaps / follow-ups (out of scope here)
agentcore createscaffolds before any deployment target exists, so the template guards fireat
add runtime/TUI where targets are known (same behavior as an ambient-region check wouldnot reliably fix; documented in the README section).
path is the workaround.
refactor(not introduced here, flagging for awareness): hardcodedarn:aws:partition strings in the harness/ab-test/evaluation IAM role generators
(
src/core/executionRole.tsx,src/core/abTestExecutionRole.tsx,src/core/eval.tsx) willproduce non-matching ARNs in aws-cn;
mainhad a partition helper layer that did not carryover. Happy to file a separate issue with the inventory.
@aws/agentcore-cdkpackage pinned by the vended CDK app needs the same two-region enumaddition on its release line (deploys gate at
cdk synthon the constructs' own enum copy —same mechanism as the eu-south-1/2 launch).
Testing
bun test: 3594/3594 across 243 files (includes new tests: CN gate matrix — blockedproviders, minimal/fastmcp allowed, LiteLLM with/without
--model-id;isChinaContextenv +targets-file detection; telemetry suppression in a CN env with telemetry enabled while the
audit sink still writes; commercial regression unchanged).
tsc --noEmit, oxlint, prettier: clean.