From a5d8f413b89fbf81a081f2cec683e0391a5a7c55 Mon Sep 17 00:00:00 2001 From: Sphia Sadek Date: Wed, 26 Aug 2026 13:22:55 -0400 Subject: [PATCH 1/5] feat(cdk): switch the Bedrock inference profile to the global geo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #747. Final step of the #740 stack; the plumbing landed in #746. Sets `bedrockGeoRegion` to `global` in cdk/cdk.json (the file had no context block before) and moves the four default strings that name a geography with it: the agent ANTHROPIC_MODEL fallback, TaskConfig.anthropic_model, the CLI's platform-default mirror, and the agent-side ANTHROPIC_DEFAULT_HAIKU_MODEL fallback. Adds the `global.` forms of Opus 5 and Haiku 4.5 to the workflow model allow-list. A `global.` profile routes to any supported commercial Region, which gives better throughput and resilience under burst — worth having for tasks that run for hours. The tradeoff is data residency: a deployer with a residency requirement sets `bedrockGeoRegion=us` (or eu/apac) instead, which is exactly why #746 made this a context key rather than a second hardcode. Verified against the synthesized template rather than assumed: all 10 inference-profile ARNs become `global.`, none remain `us.`, and the 20 foundation-model ARNs are unchanged because that half of the grant was already geo-agnostic. `ANTHROPIC_DEFAULT_HAIKU_MODEL` picks up the prefix on its own from the context key, as #746 intended. Rollback rehearsed: `-c bedrockGeoRegion=us` synths cleanly and reverts all 10 ARNs, so recovery is one context value. Two things the issue's scope table did not anticipate, both caught by existing guards rather than by reading: The agent-side haiku fallback in config.py is a SEPARATE value from the env var the stack injects. The issue said the haiku model needs no edit because #746 derives its prefix — true of the deployed env var, but a run with no env set reaches the Python literal, so leaving it would have made local and deployed runs use different geographies. The docs-parity contract test caught the inconsistency. The allow-list pairing invariant was written as bare-vs-`us.`, so it read `global.anthropic.…` as a bare id and demanded a nonsensical `us.global.anthropic.…` pairing. Generalized to the geo prefixes the list actually uses, keeping the real invariant (no prefixed entry without its bare form, no bare entry admitted in zero geographies) and confirmed still failing against an orphaned entry. Both prefixes stay in the allow-list deliberately. It has to accept whatever geography a deployment is configured for, and dropping the `us.` forms would reject a residency-constrained deployer's workflows at admission. Docs: the global-vs-geo tradeoff and the documented defaults move together, since the #742 drift test enforces them. Also corrects statements that asserted a `us.` prefix as a rule rather than as the then-current default. Starlight mirrors regenerated. Not yet done, and required before merge per the issue's acceptance criteria: deploy and an agentcore smoke test proving a task completes end to end on the global profile, plus a `platform doctor` access probe. --- agent/README.md | 10 +++--- agent/src/config.py | 11 ++++-- agent/src/models.py | 2 +- agent/tests/test_config.py | 8 +++-- cdk/cdk.json | 3 ++ cdk/src/handlers/shared/workflows.ts | 21 +++++++---- cdk/test/handlers/shared/workflows.test.ts | 35 +++++++++++++------ cli/src/repo-display.ts | 2 +- docs/design/REPO_ONBOARDING.md | 2 +- docs/guides/DEVELOPER_GUIDE.md | 8 ++--- docs/guides/USER_GUIDE.md | 2 +- .../docs/architecture/Repo-onboarding.md | 2 +- .../docs/customizing/Per-repo-overrides.md | 2 +- .../docs/developer-guide/Installation.md | 2 +- .../developer-guide/Model-configuration.md | 6 ++-- 15 files changed, 74 insertions(+), 42 deletions(-) diff --git a/agent/README.md b/agent/README.md index 8c3c33fd1..d05ef0329 100644 --- a/agent/README.md +++ b/agent/README.md @@ -119,11 +119,11 @@ The `run.sh` script overrides the container's default CMD to run `python /app/sr | `AWS_SECRET_ACCESS_KEY` | Conditional† | | Explicit keys, if you are not using CLI-based resolution | | `AWS_SESSION_TOKEN` | No | | For temporary credentials | | `AWS_PROFILE` | No | | Profile for `aws configure export-credentials` in `run.sh`, or default profile when using the `~/.aws` mount fallback | -| `ANTHROPIC_MODEL` | No | `us.anthropic.claude-opus-5` | Bedrock **inference profile** ID for `InvokeModel` (see [inference profiles](https://docs.aws.amazon.com/bedrock/latest/userguide/inference-profiles-use.html)). Must be the `us.`-prefixed profile ID, not a bare foundation-model ID — see [Model configuration](../docs/guides/DEVELOPER_GUIDE.md#model-configuration) | +| `ANTHROPIC_MODEL` | No | `global.anthropic.claude-opus-5` | Bedrock **inference profile** ID for `InvokeModel` (see [inference profiles](https://docs.aws.amazon.com/bedrock/latest/userguide/inference-profiles-use.html)). Must be a geo-prefixed profile ID matching the deployment's `bedrockGeoRegion`, not a bare foundation-model ID — see [Model configuration](../docs/guides/DEVELOPER_GUIDE.md#model-configuration) | | `MAX_TURNS` | No | `100` | Max agent turns before stopping | | `MAX_BUDGET_USD` | No | | **Local batch only** (shell env when running `entrypoint.py` directly). Range 0.01–100; agent stops when the budget is reached. For deployed AgentCore **server** mode and production tasks, set **`max_budget_usd`** on task creation (REST API, CLI `--max-budget`, or Blueprint default); the orchestrator sends it in the `/invocations` JSON body — server mode does not read `MAX_BUDGET_USD` from the environment. | | `DRY_RUN` | No | | Set to `1` to validate config and print the prompt without running the agent | -| `ANTHROPIC_DEFAULT_HAIKU_MODEL` | No | `us.anthropic.claude-haiku-4-5-20251001-v1:0` | Bedrock **inference profile** ID for the small/fast auxiliary model — the pre-flight safety check and WebFetch summarization (see below). Set by the CDK stack (`cdk/src/stacks/agent.ts` (the runtime environment block)); the `us.` prefix is required | +| `ANTHROPIC_DEFAULT_HAIKU_MODEL` | No | `global.anthropic.claude-haiku-4-5-20251001-v1:0` | Bedrock **inference profile** ID for the small/fast auxiliary model — the pre-flight safety check and WebFetch summarization (see below). Set by the CDK stack (`cdk/src/stacks/agent.ts` (the runtime environment block)), which derives the prefix from `bedrockGeoRegion` — a geo prefix is required | | `NUDGES_TABLE_NAME` | No | | **Phase 2.** DynamoDB table for mid-task user nudges (`` XML blocks injected between turns). If unset, the agent runs without nudge support — `nudge_reader.read_pending()` returns `[]` and logs a WARN once. Set automatically by the CDK stack on both AgentCore runtimes. | | `JIRA_APP_ACTOR_PROXY_URL` | No | | Resolved per-task from the Jira tenant secret. Forge v2 web-trigger URL used for app-authored Jira comments and transitions. | | `JIRA_APP_ACTOR_SHARED_SECRET` | No | | Resolved per-task from the Jira tenant secret. HMAC key for the Forge proxy; redacted from agent diagnostics. | @@ -133,7 +133,7 @@ The `run.sh` script overrides the container's default CMD to run `python /app/sr including non-Jira tasks, so a warm AgentCore process cannot expose one tenant's OAuth or Forge credential to the next task. -**Bedrock model access (main model):** Configuring `ANTHROPIC_MODEL` and IAM credentials is not enough. Your AWS account must be able to **invoke** that model in Amazon Bedrock: follow [Request access to models](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html) (Marketplace permissions on first use, Anthropic first-time use where required, valid payment method for Marketplace-backed models). Always use an inference profile ID such as `us.anthropic.claude-opus-5`: a bare foundation-model ID cannot be invoked with on-demand throughput and Bedrock rejects it with `ValidationException`. IAM must also grant the model — see [Model configuration](../docs/guides/DEVELOPER_GUIDE.md#model-configuration) for the full layering. If the CLI stops with a message that the model is not available on your Bedrock deployment, fix model access in the console or switch `ANTHROPIC_MODEL` to an entitled profile, then retry. +**Bedrock model access (main model):** Configuring `ANTHROPIC_MODEL` and IAM credentials is not enough. Your AWS account must be able to **invoke** that model in Amazon Bedrock: follow [Request access to models](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html) (Marketplace permissions on first use, Anthropic first-time use where required, valid payment method for Marketplace-backed models). Always use an inference profile ID such as `global.anthropic.claude-opus-5`: a bare foundation-model ID cannot be invoked with on-demand throughput and Bedrock rejects it with `ValidationException`. IAM must also grant the model — see [Model configuration](../docs/guides/DEVELOPER_GUIDE.md#model-configuration) for the full layering. If the CLI stops with a message that the model is not available on your Bedrock deployment, fix model access in the console or switch `ANTHROPIC_MODEL` to an entitled profile, then retry. **Pre-flight check model**: Claude Code runs a quick safety verification using a small Haiku model before executing each tool command. On Bedrock, the default Haiku model ID may not be enabled in your account, causing the check to time out with *"Pre-flight check is taking longer than expected"* warnings. The agent sets `ANTHROPIC_DEFAULT_HAIKU_MODEL` to a known-available Bedrock Haiku model ID to avoid this. If you see pre-flight timeout warnings, verify that this model is enabled in your Bedrock model access settings. @@ -145,8 +145,8 @@ tenant's OAuth or Forge credential to the next task. # Dry run — validate config, fetch issue, print assembled prompt, then exit DRY_RUN=1 ./agent/run.sh "owner/repo" 42 -# Run with a specific model (overrides the us.anthropic.claude-opus-5 default). -# Must be a `us.`-prefixed inference profile that IAM grants — see Model configuration. +# Run with a specific model (overrides the global.anthropic.claude-opus-5 default). +# Must be a geo-prefixed inference profile that IAM grants — see Model configuration. ANTHROPIC_MODEL="us.anthropic.claude-sonnet-4-6" ./agent/run.sh "owner/repo" 42 # Limit agent to 50 turns diff --git a/agent/src/config.py b/agent/src/config.py index 82f7e3253..61570d7ba 100644 --- a/agent/src/config.py +++ b/agent/src/config.py @@ -560,13 +560,18 @@ def build_config( resolved_github_token = github_token or resolve_github_token() resolved_aws_region = aws_region or os.environ.get("AWS_REGION", "") resolved_anthropic_model = anthropic_model or os.environ.get( - "ANTHROPIC_MODEL", "us.anthropic.claude-opus-5" + "ANTHROPIC_MODEL", "global.anthropic.claude-opus-5" ) # Small/fast auxiliary model (WebFetch summarization etc.). Falls back to the # deployed ANTHROPIC_DEFAULT_HAIKU_MODEL env, then the platform default. Must - # be an inference-profile id (us.*), not a bare model id (see runner). + # be a geo-prefixed inference-profile id, not a bare model id (see runner). + # + # This fallback is a SEPARATE value from the env var the stack injects, which + # derives its prefix from the bedrockGeoRegion context key. Only a run with no + # env set at all reaches this literal, so the two must be moved together or a + # local run silently uses a different geography than a deployed one. resolved_haiku_model = haiku_model or os.environ.get( - "ANTHROPIC_DEFAULT_HAIKU_MODEL", "us.anthropic.claude-haiku-4-5-20251001-v1:0" + "ANTHROPIC_DEFAULT_HAIKU_MODEL", "global.anthropic.claude-haiku-4-5-20251001-v1:0" ) # Resolve the workflow id (the create-task boundary already pinned it; local diff --git a/agent/src/models.py b/agent/src/models.py index 14a6c1f2a..22af5bda7 100644 --- a/agent/src/models.py +++ b/agent/src/models.py @@ -154,7 +154,7 @@ class TaskConfig(BaseModel): task_description: str = "" github_token: str = "" aws_region: str - anthropic_model: str = "us.anthropic.claude-opus-5" + anthropic_model: str = "global.anthropic.claude-opus-5" # The "small/fast" model Claude Code uses for auxiliary work (e.g. WebFetch # page summarization). Must be a cross-region INFERENCE-PROFILE id (``us.`` # prefix), not a bare foundation-model id — Claude 4.x cannot be invoked diff --git a/agent/tests/test_config.py b/agent/tests/test_config.py index 41e0aedcf..2207f71a3 100644 --- a/agent/tests/test_config.py +++ b/agent/tests/test_config.py @@ -52,8 +52,10 @@ def test_default_workflow_when_omitted(self): assert config.is_pr_workflow is False def test_haiku_model_defaults_to_inference_profile(self, monkeypatch): - # No override, no env → platform default, which must be a us.* inference - # profile (Claude 4.x can't be invoked on-demand by bare model id). + # No override, no env → platform default, which must be a GEO-PREFIXED + # inference profile (Claude 4.x can't be invoked on-demand by bare model + # id). The geography must match the deployment's bedrockGeoRegion, or a run + # with no env set calls a profile the IAM grant does not cover. monkeypatch.delenv("ANTHROPIC_DEFAULT_HAIKU_MODEL", raising=False) config = build_config( repo_url="owner/repo", @@ -61,7 +63,7 @@ def test_haiku_model_defaults_to_inference_profile(self, monkeypatch): github_token="ghp_test123", aws_region="us-east-1", ) - assert config.haiku_model == "us.anthropic.claude-haiku-4-5-20251001-v1:0" + assert config.haiku_model == "global.anthropic.claude-haiku-4-5-20251001-v1:0" def test_haiku_model_resolves_from_env(self, monkeypatch): # The deployed ANTHROPIC_DEFAULT_HAIKU_MODEL (set by agent.ts) flows diff --git a/cdk/cdk.json b/cdk/cdk.json index 6cf84b04b..90ca7eada 100644 --- a/cdk/cdk.json +++ b/cdk/cdk.json @@ -14,5 +14,8 @@ "yarn.lock", "node_modules" ] + }, + "context": { + "bedrockGeoRegion": "global" } } diff --git a/cdk/src/handlers/shared/workflows.ts b/cdk/src/handlers/shared/workflows.ts index dfc720b9d..16af1e46c 100644 --- a/cdk/src/handlers/shared/workflows.ts +++ b/cdk/src/handlers/shared/workflows.ts @@ -94,16 +94,25 @@ export const WORKFLOW_MODEL_ALLOWLIST: readonly string[] = [ // `bedrockModels` context) in the same change. 'anthropic.claude-opus-4-8', 'us.anthropic.claude-opus-4-8', - // Claude Opus 5 (#744) — kept in step with the IAM grant added to - // DEFAULT_BEDROCK_MODEL_IDS in the same change, per the note above. Only the - // bare and `us.`-prefixed forms: `global.anthropic.claude-opus-5` is a live - // profile but is deliberately withheld until the grant sites derive the - // `global.` ARN (#747) — admitting it here first would pass admission and then - // fail at turn 0 with AccessDenied, exactly the drift this comment warns about. + // Claude Opus 5 — kept in step with the IAM grant in + // DEFAULT_BEDROCK_MODEL_IDS, per the note above. + // + // The `global.` forms were withheld until the grant sites could derive a + // `global.` ARN, because admitting one earlier would have passed admission and + // then failed at turn 0 with AccessDenied — exactly the drift this comment + // warns about. They are admitted now that the geography is configurable and + // `bedrockGeoRegion` defaults to `global`, so the grant covers them. + // + // Both prefixes stay listed. The allow-list has to accept whatever geography a + // deployment is configured for, and a residency-constrained deployer sets + // `bedrockGeoRegion=us` — dropping the `us.` forms would reject their + // workflows at admission. 'anthropic.claude-opus-5', 'us.anthropic.claude-opus-5', + 'global.anthropic.claude-opus-5', 'anthropic.claude-haiku-4-5-20251001-v1:0', 'us.anthropic.claude-haiku-4-5-20251001-v1:0', + 'global.anthropic.claude-haiku-4-5-20251001-v1:0', ]; /** diff --git a/cdk/test/handlers/shared/workflows.test.ts b/cdk/test/handlers/shared/workflows.test.ts index bdc7cbbaa..0cc6b0f7a 100644 --- a/cdk/test/handlers/shared/workflows.test.ts +++ b/cdk/test/handlers/shared/workflows.test.ts @@ -305,19 +305,32 @@ describe('disallowedWorkflowModel (WORKFLOWS.md rule 13)', () => { expect(WORKFLOW_MODEL_ALLOWLIST).toContain('us.anthropic.claude-sonnet-4-6'); }); - test('every bare allow-listed id is paired with its us- inference-profile form', () => { - // An id admitted in only one of the two forms is a latent rejection: the - // workflow YAML may legitimately pin either, and admission compares the - // literal string. Pairing them is the invariant, so assert it for every - // entry rather than spot-checking one model. - const bare = WORKFLOW_MODEL_ALLOWLIST.filter(id => !id.startsWith('us.')); + test('every geo-prefixed allow-listed id is paired with its bare form', () => { + // An id admitted in only one form is a latent rejection: the workflow YAML may + // legitimately pin either, and admission compares the literal string. Pairing + // is the invariant, so assert it for every entry rather than spot-checking. + // + // Keyed on the geo prefixes the allow-list actually uses, not on `us.` alone. + // The list carries more than one geography now that `bedrockGeoRegion` is + // configurable, and a `us.`-only rule would read `global.anthropic.…` as a BARE + // id and demand a nonsensical `us.global.anthropic.…` pairing. + const GEO_PREFIXES = ['us.', 'global.']; + const geoOf = (id: string) => GEO_PREFIXES.find(p => id.startsWith(p)); + + const bare = WORKFLOW_MODEL_ALLOWLIST.filter(id => geoOf(id) === undefined); expect(bare.length).toBeGreaterThan(0); - const missing = bare.filter(id => !WORKFLOW_MODEL_ALLOWLIST.includes(`us.${id}`)); - expect(missing).toEqual([]); - // ...and no us- entry is orphaned (its bare form must be admitted too). + + // Every prefixed entry must have its bare form admitted too. const orphaned = WORKFLOW_MODEL_ALLOWLIST - .filter(id => id.startsWith('us.')) - .filter(id => !WORKFLOW_MODEL_ALLOWLIST.includes(id.slice('us.'.length))); + .filter(id => geoOf(id) !== undefined) + .filter(id => !WORKFLOW_MODEL_ALLOWLIST.includes(id.slice(geoOf(id)!.length))); expect(orphaned).toEqual([]); + + // And every bare entry must be admitted in at least one geography, or a + // workflow pinning the profile form it is actually deployed with is rejected. + const unprefixed = bare.filter( + id => !GEO_PREFIXES.some(p => WORKFLOW_MODEL_ALLOWLIST.includes(`${p}${id}`)), + ); + expect(unprefixed).toEqual([]); }); }); diff --git a/cli/src/repo-display.ts b/cli/src/repo-display.ts index fa7b04c9d..58810c554 100644 --- a/cli/src/repo-display.ts +++ b/cli/src/repo-display.ts @@ -45,7 +45,7 @@ export const PLATFORM_REPO_DEFAULTS = { * if it names a different model than the runtime invokes, doctor can report a * healthy stack while every task fails at turn 0 with AccessDenied. */ - model_id: 'us.anthropic.claude-opus-5', + model_id: 'global.anthropic.claude-opus-5', max_turns: 200, poll_interval_ms: 30_000, approval_gate_cap: 50, diff --git a/docs/design/REPO_ONBOARDING.md b/docs/design/REPO_ONBOARDING.md index a4b40dead..4ca233132 100644 --- a/docs/design/REPO_ONBOARDING.md +++ b/docs/design/REPO_ONBOARDING.md @@ -120,7 +120,7 @@ From lowest to highest priority: |---|---|---| | `compute_type` | `agentcore` | Platform constant | | `runtime_arn` | Stack-level env var | CDK stack props | -| `model_id` | `us.anthropic.claude-opus-5` | Python literal in `agent/src/config.py` (no CDK prop or env knob today) — see [Model configuration](../guides/DEVELOPER_GUIDE.md#model-configuration) | +| `model_id` | `global.anthropic.claude-opus-5` | Python literal in `agent/src/config.py` (no CDK prop or env knob today) — see [Model configuration](../guides/DEVELOPER_GUIDE.md#model-configuration) | | `max_turns` | 100 | Platform constant | | `max_budget_usd` | None (unlimited) | No platform default by design — a global ceiling would kill long tasks mid-change. Set a per-repo default with Blueprint `agent.maxBudgetUsd` (`0.01`–`100`, validated at synth) or per task with `--max-budget` / `max_budget_usd`. See [Per-repo overrides](../guides/USER_GUIDE.md#per-repo-overrides) for the complete list of surfaces a budget can come from | | `memory_token_budget` | 2000 | Platform constant | diff --git a/docs/guides/DEVELOPER_GUIDE.md b/docs/guides/DEVELOPER_GUIDE.md index b8f1bf316..0c88e35db 100644 --- a/docs/guides/DEVELOPER_GUIDE.md +++ b/docs/guides/DEVELOPER_GUIDE.md @@ -142,7 +142,7 @@ See the [Cedar policy guide](./CEDAR_POLICY_GUIDE.md) for the full authoring ref per-task payload model_id (layer 5) > blueprint agent.modelId (layer 4, arrives as stack env ANTHROPIC_MODEL) > stack env ANTHROPIC_MODEL (layer 3-adjacent / local shell) - > agent/src/config.py fallback (layer 2 — us.anthropic.claude-opus-5) + > agent/src/config.py fallback (layer 2 — global.anthropic.claude-opus-5) ``` Every one of those is gated by the **IAM invoke allowlist** (layer 1), which is itself gated by **account-level Bedrock model access**. Both gates are silent until invocation: a model that resolves fine through precedence still fails at turn 0 with `AccessDenied` if it is not in the grant list, and fails again if your account has not completed [Bedrock model access](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html) for it. @@ -160,7 +160,7 @@ throughput isn't supported. Retry your request with the ID or ARN of an inferenc profile that contains this model. ``` -So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `us.anthropic.claude-opus-5`. +So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `global.anthropic.claude-opus-5`. ### Bumping the default model @@ -179,7 +179,7 @@ Model choice is a **cost** decision, which is why it is adjustable per repo and | Model | Input tokens | Reported `cost_usd` | Implied input rate | |---|---|---|---| | `us.anthropic.claude-opus-4-8` | 32,145 | $0.160850 | **$5.00/MTok** | -| `us.anthropic.claude-opus-5` | 37,584 | $0.188020 | **$5.00/MTok** | +| `global.anthropic.claude-opus-5` | 37,584 | $0.188020 | **$5.00/MTok** | Token ratio 1.169; cost ratio 1.169 — identical. **The per-token rate is unchanged; the whole delta is token volume on an identical prompt.** Read it that way: "Opus 5 costs ~17% more per task" invites the wrong remedy (switch models), while "same rate, more tokens" points at the real levers — prompt size, prompt caching, and `max_turns`. @@ -336,7 +336,7 @@ The `--local-events` flag connects the agent container to DynamoDB Local on the | Variable | Default | Description | |---|---|---| -| `ANTHROPIC_MODEL` | `us.anthropic.claude-opus-5` | Bedrock inference-profile ID for the main coding model | +| `ANTHROPIC_MODEL` | `global.anthropic.claude-opus-5` | Bedrock inference-profile ID for the main coding model | | `MAX_TURNS` | `100` | Max agent turns before stopping | | `MAX_BUDGET_USD` | | Cost ceiling for local batch runs only (production uses the API field) | | `DRY_RUN` | | Set to `1` to validate and print prompt without running the agent | diff --git a/docs/guides/USER_GUIDE.md b/docs/guides/USER_GUIDE.md index 735daa09d..acf0e9d9c 100644 --- a/docs/guides/USER_GUIDE.md +++ b/docs/guides/USER_GUIDE.md @@ -221,7 +221,7 @@ Blueprints can configure per-repository settings that override platform defaults |---|---|---| | `compute_type` | Compute strategy (`agentcore` or `ecs`) | `agentcore` | | `runtime_arn` | AgentCore runtime ARN override | Platform default | -| `model_id` | Bedrock inference-profile ID (`us.`-prefixed) | `us.anthropic.claude-opus-5` | +| `model_id` | Bedrock inference-profile ID (geo-prefixed, matching the deployment's `bedrockGeoRegion`) | `global.anthropic.claude-opus-5` | | `max_turns` | Default turn limit for tasks | 100 | | `max_budget_usd` | Default cost budget in USD per task, `0.01`–`100` (Blueprint `agent.maxBudgetUsd`) | None (unlimited) | | `system_prompt_overrides` | Additional system prompt instructions | None | diff --git a/docs/src/content/docs/architecture/Repo-onboarding.md b/docs/src/content/docs/architecture/Repo-onboarding.md index 2b70c09de..c0c3cfcb8 100644 --- a/docs/src/content/docs/architecture/Repo-onboarding.md +++ b/docs/src/content/docs/architecture/Repo-onboarding.md @@ -124,7 +124,7 @@ From lowest to highest priority: |---|---|---| | `compute_type` | `agentcore` | Platform constant | | `runtime_arn` | Stack-level env var | CDK stack props | -| `model_id` | `us.anthropic.claude-opus-5` | Python literal in `agent/src/config.py` (no CDK prop or env knob today) — see [Model configuration](/sample-autonomous-cloud-coding-agents/developer-guide/model-configuration) | +| `model_id` | `global.anthropic.claude-opus-5` | Python literal in `agent/src/config.py` (no CDK prop or env knob today) — see [Model configuration](/sample-autonomous-cloud-coding-agents/developer-guide/model-configuration) | | `max_turns` | 100 | Platform constant | | `max_budget_usd` | None (unlimited) | No platform default by design — a global ceiling would kill long tasks mid-change. Set a per-repo default with Blueprint `agent.maxBudgetUsd` (`0.01`–`100`, validated at synth) or per task with `--max-budget` / `max_budget_usd`. See [Per-repo overrides](/sample-autonomous-cloud-coding-agents/customizing/per-repo-overrides) for the complete list of surfaces a budget can come from | | `memory_token_budget` | 2000 | Platform constant | diff --git a/docs/src/content/docs/customizing/Per-repo-overrides.md b/docs/src/content/docs/customizing/Per-repo-overrides.md index 2584b76c3..7fbd9c89e 100644 --- a/docs/src/content/docs/customizing/Per-repo-overrides.md +++ b/docs/src/content/docs/customizing/Per-repo-overrides.md @@ -8,7 +8,7 @@ Blueprints can configure per-repository settings that override platform defaults |---|---|---| | `compute_type` | Compute strategy (`agentcore` or `ecs`) | `agentcore` | | `runtime_arn` | AgentCore runtime ARN override | Platform default | -| `model_id` | Bedrock inference-profile ID (`us.`-prefixed) | `us.anthropic.claude-opus-5` | +| `model_id` | Bedrock inference-profile ID (geo-prefixed, matching the deployment's `bedrockGeoRegion`) | `global.anthropic.claude-opus-5` | | `max_turns` | Default turn limit for tasks | 100 | | `max_budget_usd` | Default cost budget in USD per task, `0.01`–`100` (Blueprint `agent.maxBudgetUsd`) | None (unlimited) | | `system_prompt_overrides` | Additional system prompt instructions | None | diff --git a/docs/src/content/docs/developer-guide/Installation.md b/docs/src/content/docs/developer-guide/Installation.md index 268210c5d..4ba9d85ea 100644 --- a/docs/src/content/docs/developer-guide/Installation.md +++ b/docs/src/content/docs/developer-guide/Installation.md @@ -133,7 +133,7 @@ The `--local-events` flag connects the agent container to DynamoDB Local on the | Variable | Default | Description | |---|---|---| -| `ANTHROPIC_MODEL` | `us.anthropic.claude-opus-5` | Bedrock inference-profile ID for the main coding model | +| `ANTHROPIC_MODEL` | `global.anthropic.claude-opus-5` | Bedrock inference-profile ID for the main coding model | | `MAX_TURNS` | `100` | Max agent turns before stopping | | `MAX_BUDGET_USD` | | Cost ceiling for local batch runs only (production uses the API field) | | `DRY_RUN` | | Set to `1` to validate and print prompt without running the agent | diff --git a/docs/src/content/docs/developer-guide/Model-configuration.md b/docs/src/content/docs/developer-guide/Model-configuration.md index a8a315aac..39ea033a2 100644 --- a/docs/src/content/docs/developer-guide/Model-configuration.md +++ b/docs/src/content/docs/developer-guide/Model-configuration.md @@ -28,7 +28,7 @@ title: Model configuration per-task payload model_id (layer 5) > blueprint agent.modelId (layer 4, arrives as stack env ANTHROPIC_MODEL) > stack env ANTHROPIC_MODEL (layer 3-adjacent / local shell) - > agent/src/config.py fallback (layer 2 — us.anthropic.claude-opus-5) + > agent/src/config.py fallback (layer 2 — global.anthropic.claude-opus-5) ``` Every one of those is gated by the **IAM invoke allowlist** (layer 1), which is itself gated by **account-level Bedrock model access**. Both gates are silent until invocation: a model that resolves fine through precedence still fails at turn 0 with `AccessDenied` if it is not in the grant list, and fails again if your account has not completed [Bedrock model access](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html) for it. @@ -46,7 +46,7 @@ throughput isn't supported. Retry your request with the ID or ARN of an inferenc profile that contains this model. ``` -So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `us.anthropic.claude-opus-5`. +So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `global.anthropic.claude-opus-5`. ### Bumping the default model @@ -65,7 +65,7 @@ Model choice is a **cost** decision, which is why it is adjustable per repo and | Model | Input tokens | Reported `cost_usd` | Implied input rate | |---|---|---|---| | `us.anthropic.claude-opus-4-8` | 32,145 | $0.160850 | **$5.00/MTok** | -| `us.anthropic.claude-opus-5` | 37,584 | $0.188020 | **$5.00/MTok** | +| `global.anthropic.claude-opus-5` | 37,584 | $0.188020 | **$5.00/MTok** | Token ratio 1.169; cost ratio 1.169 — identical. **The per-token rate is unchanged; the whole delta is token volume on an identical prompt.** Read it that way: "Opus 5 costs ~17% more per task" invites the wrong remedy (switch models), while "same rate, more tokens" points at the real levers — prompt size, prompt caching, and `max_turns`. From 8e3953572678e5f747eba9a6b67953730a282e8c Mon Sep 17 00:00:00 2001 From: Sphia Sadek Date: Thu, 27 Aug 2026 09:38:46 -0400 Subject: [PATCH 2/5] fix(cli): strip every geo prefix in the doctor Bedrock check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression this branch introduced. The check derives its bare foundation-model id from the platform default by stripping the inference-profile prefix, but the strip matched `us|eu|apac` only. Moving the default to a `global.` profile made it silently do nothing, so `GetFoundationModel` was handed a profile id it cannot resolve. Confirmed against the live API rather than reasoned about: anthropic.claude-opus-5 → 200 global.anthropic.claude-opus-5 → ResourceNotFoundException So the one Bedrock check `doctor` performs would have reported a false failure on every deploy of this branch — and the check exists precisely to catch model-access problems before a task fails at turn 0. Lists all seven geographies the CDK models, longest-first so `us-gov` is stripped as `us-gov` rather than leaving a stray `-gov.`. Mirrored rather than imported, matching how the CLI already mirrors PLATFORM_REPO_DEFAULTS — it is a separate package and does not depend on CDK. The guard asserts the queried id for EVERY geography, so a future default on any of them cannot reopen the hole, and it was confirmed to fail against the `us|eu|apac` version. Worth noting for the follow-up work: the check still only probes the FOUNDATION MODEL catalog, never the inference profile the deployment is configured for. A stack granted profiles its account cannot invoke still reports healthy. Fixing that is a behaviour change to what doctor checks, so it is left to its own issue rather than folded in here. --- cli/src/platform-doctor.ts | 24 +++++++++++++++++---- cli/test/platform-doctor.test.ts | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/cli/src/platform-doctor.ts b/cli/src/platform-doctor.ts index 6fae91ea0..c6d4773de 100644 --- a/cli/src/platform-doctor.ts +++ b/cli/src/platform-doctor.ts @@ -37,16 +37,32 @@ import { listRepoConfigs, RepoConfigRow } from './repo-lookup'; import { getStackOutput } from './stack-outputs'; import { makeClient } from './ua'; +/** + * Every cross-Region inference-profile geography, mirroring + * `CrossRegionInferenceProfileRegion` in `@aws-cdk/aws-bedrock-alpha` (the CLI is + * a separate package and does not depend on CDK — see `PLATFORM_REPO_DEFAULTS` + * for the same mirroring). + * + * Must list them ALL. An earlier version matched only `us|eu|apac`, so when the + * platform default moved to a `global.` profile the strip below silently did + * nothing and `GetFoundationModel` was handed a profile id it cannot resolve — + * turning the one Bedrock check `doctor` performs into a false failure. + */ +const BEDROCK_GEO_PREFIXES = ['global', 'us-gov', 'us', 'eu', 'apac', 'jp', 'au'] as const; + +/** Strips a leading `.` inference-profile prefix, if present. */ +const GEO_PREFIX_RE = new RegExp(`^(?:${BEDROCK_GEO_PREFIXES.join('|')})\\.`); + /** * Default foundation model checked when no onboarded repo specifies model_id. * * Derived from the platform default model so the two never drift on a model - * bump: `PLATFORM_REPO_DEFAULTS.model_id` is the cross-region inference profile - * (`us.anthropic.…`) used at invoke time, while `GetFoundationModel` requires - * the bare foundation-model id, so we strip the regional inference prefix. + * bump: `PLATFORM_REPO_DEFAULTS.model_id` is the cross-Region inference profile + * used at invoke time, while `GetFoundationModel` requires the bare + * foundation-model id, so the geo prefix is stripped. */ const DEFAULT_BEDROCK_MODEL_ID = - PLATFORM_REPO_DEFAULTS.model_id.replace(/^(us|eu|apac)\./, ''); + PLATFORM_REPO_DEFAULTS.model_id.replace(GEO_PREFIX_RE, ''); export type DoctorCheckStatus = 'pass' | 'fail' | 'warn'; diff --git a/cli/test/platform-doctor.test.ts b/cli/test/platform-doctor.test.ts index 36ed98b13..c554bc753 100644 --- a/cli/test/platform-doctor.test.ts +++ b/cli/test/platform-doctor.test.ts @@ -199,3 +199,39 @@ describe('doctor verdict for Linear workspace auth', () => { expect(healthMock.mock.calls[1][0]).toHaveProperty('verifyRefresh', verify); }); }); + +describe('doctor Bedrock catalog check', () => { + it('strips ANY geo prefix before calling GetFoundationModel', async () => { + // GetFoundationModel resolves BARE foundation-model ids only; handed a + // `.`-prefixed inference-profile id it returns ResourceNotFoundException + // (verified against the live API). So the one Bedrock check doctor performs + // would report a false failure. + // + // This regressed once: the strip matched `us|eu|apac` only, and when the + // platform default moved to a `global.` profile it silently stopped stripping. + // Asserting every geography the CDK models means a future default on any of + // them cannot reopen the hole. + const { PLATFORM_REPO_DEFAULTS } = await import('../src/repo-display'); + const GEOS = ['global', 'us-gov', 'us', 'eu', 'apac', 'jp', 'au']; + + for (const geo of GEOS) { + jest.clearAllMocks(); + jest.resetModules(); + jest.doMock('../src/repo-display', () => ({ + ...jest.requireActual('../src/repo-display'), + PLATFORM_REPO_DEFAULTS: { + ...PLATFORM_REPO_DEFAULTS, + model_id: `${geo}.anthropic.claude-opus-5`, + }, + })); + const { runPlatformDoctor: run } = await import('../src/platform-doctor'); + const checks = await run({ region: 'us-east-1', stackName: 'Abca' }); + const bedrock = checks.find((c) => c.id === 'bedrock_model'); + if (!bedrock) throw new Error('doctor no longer reports a Bedrock check'); + // The label carries the id that was queried, so it proves what was sent + // without reaching into the mocked client. + expect(bedrock.label).toContain('anthropic.claude-opus-5'); + expect(bedrock.label).not.toContain(`${geo}.anthropic`); + } + }); +}); From c629e1e1c2e39b2ecda60d6908897c76073d6487 Mon Sep 17 00:00:00 2001 From: Sphia Sadek Date: Thu, 27 Aug 2026 10:13:41 -0400 Subject: [PATCH 3/5] fix(cli): check the inference profile doctor will invoke, and reject an unusable --model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #804. Closes #805. Both issues are the same failure shape: a model or geography that cannot work is accepted silently, and the only symptom is every task dying at turn 0 with an AccessDenied that names nothing. #804 — doctor's Bedrock check called GetFoundationModel on the bare model id, which answers "is this model published in this Region". That is not what decides whether tasks run: the agent invokes a `.` cross-Region PROFILE, and the IAM grant is scoped to profile ARNs. A stack configured for a geography with no profile, or whose entitlements the account lacks, passed the check and then failed everything. Observed while verifying the geo switch: doctor reported anthropic.claude-sonnet-4-6 visible in us-east-1 while the deployment was configured for global.anthropic.claude-opus-5 — a different model, and a geography it never looked at. Adds a second check that resolves the actual profile via GetInferenceProfile. Both are kept because their remedies differ: a missing catalog entry means the model is unavailable here at all, a missing profile means the geography is wrong for this model or Region. The new check says "resolves" rather than "is invocable", because resolving a profile does not prove a task can call it — only InvokeModel would, and doctor does not spend a token to find out. The geography comes from a new BedrockGeoRegion stack output, mirroring the existing ComputeSubstrate output that the CLI already reads to refuse a mismatched --compute-type. On a stack that predates the output the check WARNS rather than defaulting to `us`: passing would report a verification that never happened. #805 — `repo onboard --model` wrote any string to the RepoTable unchecked. Now rejected at the boundary: a bare foundation-model id (Bedrock refuses those for on-demand invocation, so it is always wrong), and a geography the stack does not grant. Each error carries the fix — the profile form to use, or the redeploy that would grant the geography asked for. Deliberately NOT checking membership in the granted model set. The CLI cannot read `bedrockModels` today, and a guess presented as validation is worse than no check; the profile check above covers the reachable part. Noted in the code. Verified by mutation, not just by green tests: probing the bare id instead of the profile, defaulting a missing geography to `us`, dropping either --model rule, and dropping a geography from the prefix list each fail. One comment corrected in the process. I had written that the geo list must be longest-first so `us-gov` is not read as `us`. Reordering it did not fail any test, and it should not have: the match requires `` followed by a literal `.`, so `us.` cannot match `us-gov.…` at all. The comment now says ordering is for readability and the test asserts the behaviour rather than the ordering. --- cdk/src/stacks/agent.ts | 12 ++++ cdk/test/stacks/agent.test.ts | 8 +++ cli/src/commands/repo.ts | 11 +++- cli/src/model-id.ts | 94 ++++++++++++++++++++++++++++ cli/src/platform-doctor.ts | 68 ++++++++++++++++++++- cli/test/model-id.test.ts | 101 +++++++++++++++++++++++++++++++ cli/test/platform-doctor.test.ts | 73 +++++++++++++++++++++- 7 files changed, 363 insertions(+), 4 deletions(-) create mode 100644 cli/src/model-id.ts create mode 100644 cli/test/model-id.test.ts diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index 0e762b0bf..ea517d226 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -921,6 +921,18 @@ export class AgentStack extends Stack { // runtime. ``ecs`` implies the AgentCore runtime is ALSO available (the ECS // gate is additive), so an agentcore repo works on either substrate — and the // same holds for ``lambda-microvm`` (ADR-021). + // Surfaced so a client can check the profile the deployment will actually + // invoke, not just whether the model exists in the catalog. `platform doctor` + // reads it: without the geography, its Bedrock check can only ask "is this + // model published in this Region", which passes on a stack granted profiles + // the account cannot invoke — the failure then lands at turn 0 as AccessDenied. + new CfnOutput(this, 'BedrockGeoRegion', { + value: bedrockGeoRegion, + description: 'Cross-Region inference-profile geography this deploy grants ' + + '(the `bedrockGeoRegion` context key, default "us"). Model ids are invoked ' + + 'as `.`.', + }); + new CfnOutput(this, 'ComputeSubstrate', { value: ecsCluster ? 'ecs' : (lambdaMicrovm ? 'lambda-microvm' : 'agentcore'), description: 'Compute substrate provisioned by this deploy: "agentcore" (default), "ecs" ' diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 33ef368fb..c9ff8a623 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -88,6 +88,14 @@ describe('AgentStack', () => { template.hasOutput('ComputeSubstrate', { Value: 'agentcore' }); }); + test('outputs BedrockGeoRegion so a client can check the profile it will invoke', () => { + // `platform doctor` reads this. Without it, its Bedrock check can only ask "is + // this model published in this Region", which passes on a stack granted + // profiles the account cannot invoke — the failure then lands at turn 0 as + // AccessDenied. Defaults to `us`; the test app passes no context. + template.hasOutput('BedrockGeoRegion', { Value: 'us' }); + }); + test('outputs CedarWasmLayerArn', () => { template.hasOutput('CedarWasmLayerArn', {}); }); diff --git a/cli/src/commands/repo.ts b/cli/src/commands/repo.ts index 626c4cbdb..75ad77108 100644 --- a/cli/src/commands/repo.ts +++ b/cli/src/commands/repo.ts @@ -20,6 +20,7 @@ import { Command } from 'commander'; import { assertComputeSubstrateDeployed } from '../compute-substrate'; import { CliError } from '../errors'; +import { assertModelIdUsable } from '../model-id'; import { DEFAULT_STACK_NAME, redactSecretArn, resolveOperatorContext } from '../operator-context'; import { buildRepoShowLines, @@ -170,11 +171,14 @@ export function makeRepoCommand(): Command { } const { region, stackName } = resolveOperatorContext(opts); - const [tableName, platformRuntimeArn, platformGithubTokenSecretArn, computeSubstrate] = await Promise.all([ + const [ + tableName, platformRuntimeArn, platformGithubTokenSecretArn, computeSubstrate, deployedGeo, + ] = await Promise.all([ getStackOutput(region, stackName, 'RepoTableName'), getStackOutput(region, stackName, 'RuntimeArn'), getStackOutput(region, stackName, 'GitHubTokenSecretArn'), getStackOutput(region, stackName, 'ComputeSubstrate'), + getStackOutput(region, stackName, 'BedrockGeoRegion'), ]); if (!tableName) { throw new CliError( @@ -203,6 +207,11 @@ export function makeRepoCommand(): Command { // exact semantics (single-valued today, list-tolerant by construction). assertComputeSubstrateDeployed({ stackName, computeType: opts.computeType, computeSubstrate }); + // Same reasoning as the substrate gate above: reuse an output already + // fetched, and fail here rather than let a task die at turn 0 with an + // AccessDenied that names nothing. + assertModelIdUsable({ modelId: opts.model, deployedGeo, stackName }); + const config = await onboardRepo(region, tableName, repoId, { computeType: opts.computeType, runtimeArn: opts.runtimeArn, diff --git a/cli/src/model-id.ts b/cli/src/model-id.ts new file mode 100644 index 000000000..852cd84ee --- /dev/null +++ b/cli/src/model-id.ts @@ -0,0 +1,94 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { CliError } from './errors'; + +/** + * Every cross-Region inference-profile geography, mirroring + * `CrossRegionInferenceProfileRegion` in `@aws-cdk/aws-bedrock-alpha`. The CLI is a + * separate package and does not depend on CDK — the same mirroring `platform-doctor` + * and `PLATFORM_REPO_DEFAULTS` already do. + * + * Order does NOT matter here, and that is worth stating because it looks like it + * should: `geoPrefixOf` matches `` followed by a literal `.`, so `us.` cannot + * match `us-gov.…` — the dot fails against the hyphen. Listed longest-first only so + * the list reads unambiguously; a reader should not have to reason about the + * matcher to trust it. (`platform-doctor` builds a REGEX from a similar list, where + * alternation order does matter more visibly — same reasoning recorded there.) + */ +export const BEDROCK_GEO_PREFIXES = [ + 'global', 'us-gov', 'us', 'eu', 'apac', 'jp', 'au', +] as const; + +/** The `.` prefix on a model id, or undefined for a bare foundation-model id. */ +export function geoPrefixOf(modelId: string): string | undefined { + return BEDROCK_GEO_PREFIXES.find((geo) => modelId.startsWith(`${geo}.`)); +} + +/** + * Reject a `--model` value that cannot work, at the point the operator can still + * fix it. + * + * Both mistakes this catches produce the SAME opaque outcome today: the task + * dispatches, runs, and dies at turn 0 with `AccessDenied` — with nothing at + * onboard time and nothing in the failure naming the model as the cause. + * + * - A **bare** foundation-model id. Bedrock refuses bare ids for on-demand + * invocation of Claude 4.x and later ("ValidationException: … isn't supported. + * Retry your request with the ID or ARN of an inference profile"), so a bare id + * here is always wrong regardless of what is granted. + * - A geography that does not match the deployment's. The IAM grant is scoped to + * `.` profile ARNs resolved at synth, so a `us.` model on a stack + * deployed with `bedrockGeoRegion=global` is granted nothing. + * + * Deliberately does NOT check membership in the granted model set: the CLI has no + * way to read `bedrockModels` today, and a guess dressed as validation is worse + * than no check. `platform doctor` covers the profile-resolves question; see #805. + * + * `deployedGeo` is null on a stack that predates the `BedrockGeoRegion` output — the + * geography check is then skipped rather than assumed, but the bare-id check still + * applies because it holds for every geography. + */ +export function assertModelIdUsable(args: { + modelId: string | undefined; + deployedGeo: string | null | undefined; + stackName: string; +}): void { + const { modelId, deployedGeo, stackName } = args; + if (!modelId) return; + + const geo = geoPrefixOf(modelId); + if (!geo) { + throw new CliError( + `--model '${modelId}' looks like a bare foundation-model id, which Bedrock cannot ` + + 'invoke on demand — a task using it would fail at turn 0 with a ValidationException. ' + + `Use the inference-profile form: '${deployedGeo ?? 'us'}.${modelId}'.`, + ); + } + + if (!deployedGeo || geo === deployedGeo) return; + + throw new CliError( + `--model '${modelId}' is a '${geo}' inference profile, but stack '${stackName}' grants ` + + `'${deployedGeo}' profiles (BedrockGeoRegion). The IAM grant is scoped to ` + + `'${deployedGeo}.' ARNs, so this model is granted nothing and tasks would fail at turn 0 ` + + `with AccessDenied. Use '${deployedGeo}.${modelId.slice(geo.length + 1)}', or redeploy the ` + + `stack with -c bedrockGeoRegion=${geo}.`, + ); +} diff --git a/cli/src/platform-doctor.ts b/cli/src/platform-doctor.ts index c6d4773de..c3f52eb9c 100644 --- a/cli/src/platform-doctor.ts +++ b/cli/src/platform-doctor.ts @@ -17,7 +17,7 @@ * SOFTWARE. */ -import { BedrockClient, GetFoundationModelCommand } from '@aws-sdk/client-bedrock'; +import { BedrockClient, GetFoundationModelCommand, GetInferenceProfileCommand } from '@aws-sdk/client-bedrock'; import { CognitoIdentityProviderClient, DescribeUserPoolClientCommand, @@ -101,6 +101,7 @@ export async function runPlatformDoctor( repoTableName, linearRegistryTableName, jiraRegistryTableName, + bedrockGeoRegion, ] = await Promise.all([ getStackOutput(region, stackName, 'ApiUrl'), getStackOutput(region, stackName, 'UserPoolId'), @@ -109,6 +110,7 @@ export async function runPlatformDoctor( getStackOutput(region, stackName, 'RepoTableName'), getStackOutput(region, stackName, 'LinearWorkspaceRegistryTableName'), getStackOutput(region, stackName, 'JiraWorkspaceRegistryTableName'), + getStackOutput(region, stackName, 'BedrockGeoRegion'), ]); const checks: DoctorCheckResult[] = []; @@ -119,6 +121,7 @@ export async function runPlatformDoctor( const activeRepoResult = await loadActiveRepos(region, repoTableName); checks.push(checkActiveRepos(repoTableName, activeRepoResult)); checks.push(await checkBedrockModel(region, DEFAULT_BEDROCK_MODEL_ID)); + checks.push(await checkBedrockInferenceProfile(region, DEFAULT_BEDROCK_MODEL_ID, bedrockGeoRegion)); if (activeRepoResult.repos.some((repo) => repo.compute_type === 'lambda-microvm')) { checks.push(await checkLambdaMicrovmAvailability( region, @@ -410,6 +413,69 @@ async function checkBedrockModel(region: string, modelId: string): Promise.` PROFILE and the IAM grant is scoped to profile ARNs. A stack + * configured for a geography whose profile does not exist — or whose entitlements + * the account lacks — passes the catalog check and then fails every task at turn 0 + * with AccessDenied, which is exactly what doctor is supposed to pre-empt. + * + * Keeping the two separate also keeps the remedies distinct: a missing catalog + * entry means the model is unavailable here at all, while a missing profile means + * the geography is wrong for this model or Region. + * + * `geoRegion` is null when the stack predates the `BedrockGeoRegion` output. That + * is reported rather than defaulted: guessing `us` and passing would state a + * verification that never happened. + */ +async function checkBedrockInferenceProfile( + region: string, + bareModelId: string, + geoRegion: string | null, +): Promise { + const id = 'bedrock_inference_profile'; + if (!geoRegion) { + return { + id, + label: 'Bedrock inference profile', + status: 'warn', + detail: 'Stack does not export BedrockGeoRegion, so the inference profile the ' + + 'agent invokes cannot be determined. Redeploy to surface it; until then this ' + + 'check is skipped rather than assuming a geography.', + }; + } + + const profileId = `${geoRegion}.${bareModelId}`; + const label = `Bedrock inference profile (${profileId})`; + const bedrock = makeClient(BedrockClient, { region }); + try { + await bedrock.send(new GetInferenceProfileCommand({ inferenceProfileIdentifier: profileId })); + return { + id, + label, + status: 'pass', + // Deliberately not claiming invocability: resolving a profile proves it + // exists and is reachable, not that a task can call it. Only InvokeModel + // proves that, and doctor does not spend a token to find out. + detail: `Inference profile ${profileId} resolves in ${region}.`, + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + const status: DoctorCheckStatus = message.includes('AccessDenied') ? 'warn' : 'fail'; + return { + id, + label, + status, + detail: `${message} The deployment grants '${geoRegion}' profiles (bedrockGeoRegion). ` + + `Either ${bareModelId} has no profile in that geography, or this account lacks its ` + + 'entitlements — tasks would fail at turn 0 with AccessDenied.', + }; + } +} + /** * Linear workspaces whose OAuth authorization has died. This is the one failure * mode that is otherwise INVISIBLE: the webhook processor can't resolve a token, diff --git a/cli/test/model-id.test.ts b/cli/test/model-id.test.ts new file mode 100644 index 000000000..0dd660ad9 --- /dev/null +++ b/cli/test/model-id.test.ts @@ -0,0 +1,101 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { BEDROCK_GEO_PREFIXES, assertModelIdUsable, geoPrefixOf } from '../src/model-id'; + +const STACK = 'Abca'; +const check = (modelId: string | undefined, deployedGeo: string | null) => + () => assertModelIdUsable({ modelId, deployedGeo, stackName: STACK }); + +describe('assertModelIdUsable', () => { + it('accepts a profile matching the deployed geography', () => { + expect(check('global.anthropic.claude-opus-5', 'global')).not.toThrow(); + expect(check('us.anthropic.claude-opus-5', 'us')).not.toThrow(); + }); + + it('accepts no --model at all (the common case)', () => { + expect(check(undefined, 'global')).not.toThrow(); + }); + + it('rejects a bare foundation-model id, and names the profile form to use', () => { + // Bedrock refuses bare ids for on-demand invocation of Claude 4.x+, so this is + // wrong regardless of what is granted. The error has to carry the fix: the + // operator is one prefix away from a working value. + expect(check('anthropic.claude-opus-5', 'global')) + .toThrow(/bare foundation-model id/); + expect(check('anthropic.claude-opus-5', 'global')) + .toThrow(/global\.anthropic\.claude-opus-5/); + }); + + it('rejects a geography the stack does not grant, and names both sides', () => { + // The IAM grant is scoped to `.` profile ARNs resolved at synth, so a + // mismatched geography is granted nothing. Today that surfaces only as an + // AccessDenied at turn 0 with no mention of the model. + const t = check('us.anthropic.claude-opus-5', 'global'); + expect(t).toThrow(/'us' inference profile/); + expect(t).toThrow(/grants 'global' profiles/); + // Both remedies: fix the model, or redeploy the stack for that geography. + expect(t).toThrow(/global\.anthropic\.claude-opus-5/); + expect(t).toThrow(/bedrockGeoRegion=us/); + }); + + it('strips the right prefix length for a hyphenated geography', () => { + // `us-gov` must be recognized as `us-gov`, not as `us` leaving a stray `-gov.` + // in the suggested replacement. + expect(check('us-gov.anthropic.claude-opus-5', 'global')) + .toThrow(/global\.anthropic\.claude-opus-5/); + }); + + it('skips the geography check when the stack does not export one', () => { + // An older stack has no BedrockGeoRegion output. Guessing a geography and + // rejecting on it would block a legitimate onboard. + expect(check('us.anthropic.claude-opus-5', null)).not.toThrow(); + expect(check('global.anthropic.claude-opus-5', null)).not.toThrow(); + }); + + it('still rejects a bare id with no exported geography', () => { + // The bare-id rule holds for every geography, so it must not be skipped along + // with the geography comparison. + expect(check('anthropic.claude-opus-5', null)).toThrow(/bare foundation-model id/); + }); +}); + +describe('geoPrefixOf', () => { + it('recognizes every geography the CDK models', () => { + for (const geo of BEDROCK_GEO_PREFIXES) { + expect(geoPrefixOf(`${geo}.anthropic.claude-opus-5`)).toBe(geo); + } + }); + + it('matches us-gov as us-gov regardless of list order', () => { + // Independent of ordering: the match requires `` followed by a literal `.`, + // so `us.` cannot match `us-gov.…`. Asserted because the list is written + // longest-first and a future tidy-up must not be read as load-bearing. + expect(geoPrefixOf('us-gov.anthropic.claude-opus-5')).toBe('us-gov'); + expect(geoPrefixOf('us.anthropic.claude-opus-5')).toBe('us'); + }); + + it('returns undefined for a bare id, and for a name that merely starts with a geo word', () => { + expect(geoPrefixOf('anthropic.claude-opus-5')).toBeUndefined(); + // Keyed on the `.` separator, so a hypothetical vendor named `august` or + // `european` is not collateral damage. + expect(geoPrefixOf('august-labs.model-1')).toBeUndefined(); + expect(geoPrefixOf('european.model-1')).toBeUndefined(); + }); +}); diff --git a/cli/test/platform-doctor.test.ts b/cli/test/platform-doctor.test.ts index c554bc753..459e6ab55 100644 --- a/cli/test/platform-doctor.test.ts +++ b/cli/test/platform-doctor.test.ts @@ -40,9 +40,11 @@ jest.mock('../src/dynamo-clients', () => ({ documentClient: () => ({ send: (...args: unknown[]) => ddbSendMock(...args) }), })); +const bedrockSendMock = jest.fn(); jest.mock('@aws-sdk/client-bedrock', () => ({ - BedrockClient: jest.fn(() => ({ send: jest.fn().mockRejectedValue(new Error('not under test')) })), - GetFoundationModelCommand: jest.fn(), + BedrockClient: jest.fn(() => ({ send: (...args: unknown[]) => bedrockSendMock(...args) })), + GetFoundationModelCommand: jest.fn((input: unknown) => ({ _type: 'GetFoundationModel', input })), + GetInferenceProfileCommand: jest.fn((input: unknown) => ({ _type: 'GetInferenceProfile', input })), })); import { @@ -235,3 +237,70 @@ describe('doctor Bedrock catalog check', () => { } }); }); + +describe('doctor Bedrock inference-profile check', () => { + /** Drive the doctor with a given BedrockGeoRegion output and Bedrock behaviour. */ + async function profileCheck( + geo: string | null, + send: jest.Mock = jest.fn().mockResolvedValue({}), + ): Promise { + bedrockSendMock.mockImplementation((...args: unknown[]) => send(...args)); + stackOutputMock.mockImplementation(async (_r: string, _s: string, output: string) => { + if (output === 'BedrockGeoRegion') return geo; + if (output === 'LinearWorkspaceRegistryTableName') return REGISTRY; + return null; + }); + const checks = await runPlatformDoctor({ region: 'us-east-1', stackName: 'Abca' }); + const check = checks.find((c) => c.id === 'bedrock_inference_profile'); + if (!check) throw new Error('doctor no longer reports an inference-profile check'); + return check; + } + + it('probes the profile the deployment will invoke, not just the catalog', async () => { + // The catalog check answers "is this model published in this Region"; the agent + // invokes a `.` PROFILE and the IAM grant is scoped to profile ARNs. + // A stack whose geography has no profile passes the catalog check and then fails + // every task at turn 0 with AccessDenied — the thing doctor exists to pre-empt. + const send = jest.fn().mockResolvedValue({}); + const check = await profileCheck('global', send); + expect(check.status).toBe('pass'); + expect(check.label).toContain('global.anthropic.claude-opus-5'); + + // The queried identifier is the geo-prefixed profile, not the bare model id. + const queried = send.mock.calls + .map(([c]) => (c as { _type?: string; input?: { inferenceProfileIdentifier?: string } })) + .filter((c) => c._type === 'GetInferenceProfile') + .map((c) => c.input?.inferenceProfileIdentifier); + expect(queried).toContain('global.anthropic.claude-opus-5'); + }); + + it('uses the geography the stack reports, not a hardcoded one', async () => { + // The whole point of reading the output: a residency-constrained deployment runs + // `us` and must be checked against `us.`, not against the current default. + const send = jest.fn().mockResolvedValue({}); + const check = await profileCheck('us', send); + expect(check.label).toContain('us.anthropic.claude-opus-5'); + expect(check.label).not.toContain('global.'); + }); + + it('fails, with the geography named, when the profile does not resolve', async () => { + // Verified against the live API: an absent profile returns + // ResourceNotFoundException. The remedy has to name the configured geography, + // because "not found" alone does not tell an operator which knob is wrong. + const check = await profileCheck( + 'jp', + jest.fn().mockRejectedValue(new Error('ResourceNotFoundException: profile not found')), + ); + expect(check.status).toBe('fail'); + expect(check.detail).toContain('jp'); + expect(check.detail).toMatch(/bedrockGeoRegion/); + }); + + it('warns rather than passing when the stack does not export the geography', async () => { + // An older stack has no BedrockGeoRegion output. Defaulting to `us` and passing + // would report a verification that never happened. + const check = await profileCheck(null); + expect(check.status).toBe('warn'); + expect(check.detail).toMatch(/BedrockGeoRegion/); + }); +}); From a2550be52b84b2f9edf1370caa18c5acb5f81772 Mon Sep 17 00:00:00 2001 From: Sphia Sadek Date: Thu, 27 Aug 2026 12:02:07 -0400 Subject: [PATCH 4/5] fix(agent): inject both model env vars from the resolved geography MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all six review findings on the geo switch. The first was a real defect, not a documentation error. FINDING 1 — the one-line rollback was false. `synth -c bedrockGeoRegion=us` granted `us.` profiles while the agent still asked for `global.anthropic.claude-opus-5`, because the stack injected only the AUXILIARY model into the runtime env and never the main one. The main model came from a Python literal that a geography change does not touch, so every task with no per-repo override would fail at turn 0 with AccessDenied. My own PR body half-knew this — the prose said "roll back context and defaults together" while the summary claimed one line. Fixed by removing the divergence rather than re-syncing literals: both models are now injected from the resolved geography, via one shared helper, on BOTH substrates (the ECS task definitions set neither var either, so an ECS task had the same mismatch). Verified by synthesizing both geographies: grants, AgentCore env and ECS env now agree in each, so the rollback genuinely is one context value. Also fixes the fifth stale default the review found, TaskConfig.haiku_model, which I had missed entirely while counting four. FINDING 2 — the generalized allow-list invariant was too weak in both directions. It only proved entries were paired WITHIN the list, which is satisfiable while wrong: three granted models had no `global.` form (a workflow pinning one was rejected at admission), and the reviewer's invented pair passed all 22 tests despite being granted nothing. Replaced with parity against DEFAULT_BEDROCK_MODEL_IDS across the deployable geographies, in both directions, and confirmed the reviewer's exact mutation now fails. FINDING 3 — the `--model` guard let through a well-formed but ungranted model. My justification for that was wrong: I claimed the CLI cannot read `bedrockModels`, but `get-template` recovers the granted set from the profile ARNs using credentials the CLI already needs. Added a `BedrockModelIds` output — a documented contract rather than a regex over CloudFormation — and the guard now rejects an ungranted model, listing what the stack does grant. Also fixes the reverse-compatibility bug: with no geography exported, the bare-id error no longer prescribes `us.`, which a default-global stack does not grant. FINDING 4 — doctor's profile check is relabelled "visible" and its detail states that resolution happens under operator credentials and does not prove the workload role can invoke. The narrow wording was already accurate, but a PASS feeding "All checks passed" still read as readiness. FINDINGS 5-6 — carries #799's documentation so this branch is not self-contradictory, corrects the onboarding and troubleshooting skills that recommended `us.` overrides this branch would now reject (and still called Sonnet 4.6 the default), and fixes the stale "default us" wording in the new output description. One test rewritten rather than re-pinned: the agent's haiku-default test asserted `startswith("us.")` when its stated intent was "must be a profile, not a bare id". It now asserts a geo prefix, so it tests the property instead of a geography that is a deploy-time choice. --- agent/src/models.py | 9 ++- agent/tests/test_runner.py | 11 ++- cdk/src/constructs/bedrock-models.ts | 27 ++++++++ cdk/src/constructs/ecs-agent-cluster.ts | 23 ++++++- cdk/src/handlers/shared/workflows.ts | 3 + cdk/src/stacks/agent.ts | 47 +++++++++---- cdk/test/handlers/shared/workflows.test.ts | 67 ++++++++++++------- cli/src/commands/repo.ts | 9 ++- cli/src/model-id.ts | 27 +++++++- cli/src/platform-doctor.ts | 10 ++- cli/test/model-id.test.ts | 37 +++++++++- docs/abca-plugin/skills/onboard-repo/SKILL.md | 14 ++-- docs/abca-plugin/skills/troubleshoot/SKILL.md | 6 +- docs/guides/DEVELOPER_GUIDE.md | 27 +++++++- .../developer-guide/Model-configuration.md | 27 +++++++- 15 files changed, 276 insertions(+), 68 deletions(-) diff --git a/agent/src/models.py b/agent/src/models.py index 22af5bda7..8c170f97c 100644 --- a/agent/src/models.py +++ b/agent/src/models.py @@ -156,10 +156,15 @@ class TaskConfig(BaseModel): aws_region: str anthropic_model: str = "global.anthropic.claude-opus-5" # The "small/fast" model Claude Code uses for auxiliary work (e.g. WebFetch - # page summarization). Must be a cross-region INFERENCE-PROFILE id (``us.`` + # page summarization). Must be a cross-region INFERENCE-PROFILE id (geo # prefix), not a bare foundation-model id — Claude 4.x cannot be invoked # on-demand by bare id on Bedrock. Threaded to ANTHROPIC_DEFAULT_HAIKU_MODEL. - haiku_model: str = "us.anthropic.claude-haiku-4-5-20251001-v1:0" + # + # A deployed task never reaches this default: the stack injects both model env + # vars from its resolved bedrockGeoRegion. It applies to direct construction + # (tests, local runs), so it must name the same geography as the deployment or + # those paths silently exercise a different one. + haiku_model: str = "global.anthropic.claude-haiku-4-5-20251001-v1:0" dry_run: bool = False max_turns: int = 10 max_budget_usd: float | None = None diff --git a/agent/tests/test_runner.py b/agent/tests/test_runner.py index 9ebd049cd..66808f55c 100644 --- a/agent/tests/test_runner.py +++ b/agent/tests/test_runner.py @@ -419,9 +419,14 @@ def test_haiku_model_env_is_set_from_config(self, monkeypatch): ) def test_config_default_haiku_model_is_an_inference_profile(self): - # The platform default (no override) must be a us.* profile, never a bare - # foundation-model id — the whole point of the fix. - assert _config().haiku_model.startswith("us.") + # The platform default (no override) must be a GEO-PREFIXED profile, never a + # bare foundation-model id — Claude 4.x cannot be invoked on-demand by bare + # id. Asserted as "has a geo prefix" rather than "starts with us.": the + # geography is a deploy-time choice (bedrockGeoRegion), so pinning one here + # would fail the moment the platform default moves, which tells us nothing + # about the property that matters. + geos = ("global.", "us.", "us-gov.", "eu.", "apac.", "jp.", "au.") + assert _config().haiku_model.startswith(geos) class TestRegisterGatewayServer: diff --git a/cdk/src/constructs/bedrock-models.ts b/cdk/src/constructs/bedrock-models.ts index 42a5f1dc2..1d5faf01f 100644 --- a/cdk/src/constructs/bedrock-models.ts +++ b/cdk/src/constructs/bedrock-models.ts @@ -55,6 +55,33 @@ export const DEFAULT_BEDROCK_MODEL_IDS: readonly string[] = [ 'anthropic.claude-haiku-4-5-20251001-v1:0', ]; +/** + * The bare foundation-model ids the platform itself defaults to, as opposed to the + * full grant list. Both are injected into the runtime as geo-prefixed profile ids + * so a deploy cannot grant one geography and tell the agent to call another. + * + * Named separately from {@link DEFAULT_BEDROCK_MODEL_IDS} because that list is the + * IAM ALLOWANCE — several models a repo may pin — while these two are what a task + * uses when it pins nothing. A model can be granted without being a default. + */ +export const PLATFORM_DEFAULT_MODEL_ID = 'anthropic.claude-opus-5'; +export const PLATFORM_DEFAULT_AUX_MODEL_ID = 'anthropic.claude-haiku-4-5-20251001-v1:0'; + +/** + * The geo-prefixed inference-profile id for a bare model, in the geography this + * deploy grants. The single place that prefix is applied for runtime env vars. + * + * Exists because the main model was previously NOT injected at all: the stack set + * only the auxiliary var, so the main model came from a Python literal that a + * geography change did not touch. Deploying with a different geography then granted + * one set of profiles while the agent asked for another, and every task with no + * per-repo override failed at turn 0 with AccessDenied. Deriving both from here + * makes that divergence unrepresentable rather than merely documented. + */ +export function inferenceProfileId(geoRegion: string, bareModelId: string): string { + return `${geoRegion}.${bareModelId}`; +} + /** CDK context key whose value (a string array) overrides the model set. */ export const BEDROCK_MODELS_CONTEXT_KEY = 'bedrockModels'; diff --git a/cdk/src/constructs/ecs-agent-cluster.ts b/cdk/src/constructs/ecs-agent-cluster.ts index dae168aec..d4913309f 100644 --- a/cdk/src/constructs/ecs-agent-cluster.ts +++ b/cdk/src/constructs/ecs-agent-cluster.ts @@ -30,7 +30,13 @@ import { NagSuppressions } from 'cdk-nag'; import { Construct, type Node } from 'constructs'; import { AgentMemory } from './agent-memory'; import { AgentSessionRole } from './agent-session-role'; -import { resolveBedrockGeoRegion, resolveBedrockModelIds } from './bedrock-models'; +import { + PLATFORM_DEFAULT_AUX_MODEL_ID, + PLATFORM_DEFAULT_MODEL_ID, + inferenceProfileId, + resolveBedrockGeoRegion, + resolveBedrockModelIds, +} from './bedrock-models'; import { buildAppId } from './solution-ua-aspect'; import { ToolGateway } from './tool-gateway'; @@ -367,8 +373,22 @@ export class EcsAgentCluster extends Construct { // IDENTICAL; only the enclosing task def's cpu/mem differ. BUILD_VERIFY_TIMEOUT_S // is a build-tier concern (a read-only planner never runs the post-agent build // verify), so it's set per-def below, not here. + // Resolved once, above baseEnvironment: the env vars below and the IAM grants + // further down must name the SAME geography, so they read one value. + const bedrockGeoRegion = resolveBedrockGeoRegion(this.node); + const baseEnvironment: Record = { CLAUDE_CODE_USE_BEDROCK: '1', + // Both models as geo-prefixed inference-profile ids, from the same resolved + // geography as the IAM grants below. Previously neither was set here, so an + // ECS task fell through to the literals in agent/src/config.py — which a + // geography change does not touch — and a non-default `bedrockGeoRegion` + // granted one geography while the agent called another's profile. Parity with + // the AgentCore runtime env (stacks/agent.ts) is the point: the two substrates + // must not disagree about which model a task runs. + ANTHROPIC_MODEL: inferenceProfileId(bedrockGeoRegion, PLATFORM_DEFAULT_MODEL_ID), + ANTHROPIC_DEFAULT_HAIKU_MODEL: + inferenceProfileId(bedrockGeoRegion, PLATFORM_DEFAULT_AUX_MODEL_ID), TASK_TABLE_NAME: props.taskTable.tableName, TASK_EVENTS_TABLE_NAME: props.taskEventsTable.tableName, USER_CONCURRENCY_TABLE_NAME: props.userConcurrencyTable.tableName, @@ -590,7 +610,6 @@ export class EcsAgentCluster extends Construct { // values (constructs/bedrock-models.ts: `bedrockModels`, `bedrockGeoRegion`) // so the ECS and AgentCore backends can't drift. const stack = Stack.of(this); - const bedrockGeoRegion = resolveBedrockGeoRegion(this.node); const bedrockResources: string[] = []; for (const modelId of resolveBedrockModelIds(this.node)) { bedrockResources.push( diff --git a/cdk/src/handlers/shared/workflows.ts b/cdk/src/handlers/shared/workflows.ts index 16af1e46c..356656376 100644 --- a/cdk/src/handlers/shared/workflows.ts +++ b/cdk/src/handlers/shared/workflows.ts @@ -84,8 +84,10 @@ export interface WorkflowDescriptor { export const WORKFLOW_MODEL_ALLOWLIST: readonly string[] = [ 'anthropic.claude-sonnet-4-6', 'us.anthropic.claude-sonnet-4-6', + 'global.anthropic.claude-sonnet-4-6', 'anthropic.claude-opus-4-20250514-v1:0', 'us.anthropic.claude-opus-4-20250514-v1:0', + 'global.anthropic.claude-opus-4-20250514-v1:0', // Claude Opus 4.8. Admitting an id here does NOT grant permission to invoke // it — this list and the IAM grant in `bedrock-models.ts` are independent, and // a model must be on BOTH to be usable. A workflow pinning an allow-listed but @@ -94,6 +96,7 @@ export const WORKFLOW_MODEL_ALLOWLIST: readonly string[] = [ // `bedrockModels` context) in the same change. 'anthropic.claude-opus-4-8', 'us.anthropic.claude-opus-4-8', + 'global.anthropic.claude-opus-4-8', // Claude Opus 5 — kept in step with the IAM grant in // DEFAULT_BEDROCK_MODEL_IDS, per the note above. // diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index ea517d226..f813aef6d 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -36,7 +36,13 @@ import { AgentVpc } from '../constructs/agent-vpc'; import { ApiKeyTable } from '../constructs/api-key-table'; import { ApprovalMetricsPublisherConsumer } from '../constructs/approval-metrics-publisher-consumer'; import { AttachmentsBucket } from '../constructs/attachments-bucket'; -import { resolveBedrockGeoRegion, resolveBedrockModelIds } from '../constructs/bedrock-models'; +import { + PLATFORM_DEFAULT_AUX_MODEL_ID, + PLATFORM_DEFAULT_MODEL_ID, + inferenceProfileId, + resolveBedrockGeoRegion, + resolveBedrockModelIds, +} from '../constructs/bedrock-models'; import { Blueprint } from '../constructs/blueprint'; import { CedarWasmLayer } from '../constructs/cedar-wasm-layer'; import { ConcurrencyReconciler } from '../constructs/concurrency-reconciler'; @@ -478,15 +484,22 @@ export class AgentStack extends Stack { AWS_REGION: process.env.AWS_REGION ?? 'us-east-1', CLAUDE_CODE_USE_BEDROCK: '1', ANTHROPIC_LOG: 'debug', - // Cross-region inference-profile id (geo prefix, `us.` by default), NOT - // the bare foundation-model id: Claude 4.x can't be invoked on-demand by - // bare id (400 "on-demand throughput isn't supported"). The prefix is - // derived from `bedrockGeoRegion` rather than hardcoded so this auxiliary - // model routes through the same geography as the granted profiles (see - // bedrock-models.ts) — a second hardcode here would silently split the - // two on any non-`us` deploy. runner.py re-sets this at spawn time. + // Both models as geo-prefixed inference-profile ids, NOT bare foundation-model + // ids: Claude 4.x can't be invoked on-demand by bare id (400 "on-demand + // throughput isn't supported"). + // + // The MAIN model is set here deliberately, and it was previously missing. Only + // the auxiliary var was injected, so the main model fell through to a literal + // in agent/src/config.py that a geography change did not touch — deploying a + // different `bedrockGeoRegion` granted one geography's profiles while the agent + // asked for another's, and every task with no per-repo override failed at turn 0 + // with AccessDenied. Injecting both from the resolved geography makes the + // divergence impossible rather than something a checklist has to catch. + // + // runner.py re-sets these at spawn time; a per-repo `model_id` still overrides. + ANTHROPIC_MODEL: inferenceProfileId(bedrockGeoRegion, PLATFORM_DEFAULT_MODEL_ID), ANTHROPIC_DEFAULT_HAIKU_MODEL: - `${bedrockGeoRegion}.anthropic.claude-haiku-4-5-20251001-v1:0`, + inferenceProfileId(bedrockGeoRegion, PLATFORM_DEFAULT_AUX_MODEL_ID), TASK_TABLE_NAME: taskTable.table.tableName, TASK_EVENTS_TABLE_NAME: taskEventsTable.table.tableName, NUDGES_TABLE_NAME: taskNudgesTable.table.tableName, @@ -926,11 +939,21 @@ export class AgentStack extends Stack { // reads it: without the geography, its Bedrock check can only ask "is this // model published in this Region", which passes on a stack granted profiles // the account cannot invoke — the failure then lands at turn 0 as AccessDenied. + // The granted model set, so a client can reject an ungranted --model instead of + // letting the task reach turn 0 and fail with AccessDenied. Recoverable from the + // deployed template's profile ARNs, but an output makes it a documented contract + // rather than something a consumer has to regex out of CloudFormation. + new CfnOutput(this, 'BedrockModelIds', { + value: resolveBedrockModelIds(this.node).join(','), + description: 'Comma-separated BARE foundation-model ids this deploy grants. ' + + 'Invoked as `.`.', + }); + new CfnOutput(this, 'BedrockGeoRegion', { value: bedrockGeoRegion, - description: 'Cross-Region inference-profile geography this deploy grants ' - + '(the `bedrockGeoRegion` context key, default "us"). Model ids are invoked ' - + 'as `.`.', + description: 'Cross-Region inference-profile geography this deploy grants (the ' + + '`bedrockGeoRegion` context key; "global" in the shipped cdk.json, "us" if no ' + + 'context is supplied at all). Model ids are invoked as `.`.', }); new CfnOutput(this, 'ComputeSubstrate', { diff --git a/cdk/test/handlers/shared/workflows.test.ts b/cdk/test/handlers/shared/workflows.test.ts index 0cc6b0f7a..2ae0de2ce 100644 --- a/cdk/test/handlers/shared/workflows.test.ts +++ b/cdk/test/handlers/shared/workflows.test.ts @@ -20,6 +20,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as yaml from 'js-yaml'; +import { DEFAULT_BEDROCK_MODEL_IDS } from '../../../src/constructs/bedrock-models'; import { CODING_WORKFLOW_ID, DEFAULT_WORKFLOW_ID, @@ -305,32 +306,46 @@ describe('disallowedWorkflowModel (WORKFLOWS.md rule 13)', () => { expect(WORKFLOW_MODEL_ALLOWLIST).toContain('us.anthropic.claude-sonnet-4-6'); }); - test('every geo-prefixed allow-listed id is paired with its bare form', () => { - // An id admitted in only one form is a latent rejection: the workflow YAML may - // legitimately pin either, and admission compares the literal string. Pairing - // is the invariant, so assert it for every entry rather than spot-checking. + // Geographies a deployment may realistically select. Not the full CDK enum: the + // allow-list is hand-maintained, and demanding all seven would add entries nobody + // can deploy against today. `us` is the code default, `global` the cdk.json default, + // so both must be admitted for the same reason a residency deployer needs `us`. + const DEPLOYABLE_GEOS = ['us', 'global'] as const; + + test('the allow-list covers every GRANTED model in every geography it may be deployed with', () => { + // The real invariant: parity with the IAM grant list, not internal pairing. // - // Keyed on the geo prefixes the allow-list actually uses, not on `us.` alone. - // The list carries more than one geography now that `bedrockGeoRegion` is - // configurable, and a `us.`-only rule would read `global.anthropic.…` as a BARE - // id and demand a nonsensical `us.global.anthropic.…` pairing. - const GEO_PREFIXES = ['us.', 'global.']; - const geoOf = (id: string) => GEO_PREFIXES.find(p => id.startsWith(p)); - - const bare = WORKFLOW_MODEL_ALLOWLIST.filter(id => geoOf(id) === undefined); - expect(bare.length).toBeGreaterThan(0); - - // Every prefixed entry must have its bare form admitted too. - const orphaned = WORKFLOW_MODEL_ALLOWLIST - .filter(id => geoOf(id) !== undefined) - .filter(id => !WORKFLOW_MODEL_ALLOWLIST.includes(id.slice(geoOf(id)!.length))); - expect(orphaned).toEqual([]); - - // And every bare entry must be admitted in at least one geography, or a - // workflow pinning the profile form it is actually deployed with is rejected. - const unprefixed = bare.filter( - id => !GEO_PREFIXES.some(p => WORKFLOW_MODEL_ALLOWLIST.includes(`${p}${id}`)), - ); - expect(unprefixed).toEqual([]); + // The previous version only checked that each entry had a matching bare/prefixed + // partner WITHIN the allow-list. That is satisfiable while being wrong in both + // directions, and both were live: three granted models had no `global.` form (so + // a workflow pinning one was rejected at admission on a global deploy), and an + // invented model pair passed the whole suite despite being granted nothing. + // + // Admission compares the literal string a workflow pins, and a deployment may run + // any geography, so every granted model needs its bare form plus a form for each + // geography a deploy could select. + const missing: string[] = []; + for (const bare of DEFAULT_BEDROCK_MODEL_IDS) { + if (!WORKFLOW_MODEL_ALLOWLIST.includes(bare)) missing.push(bare); + for (const geo of DEPLOYABLE_GEOS) { + const id = `${geo}.${bare}`; + if (!WORKFLOW_MODEL_ALLOWLIST.includes(id)) missing.push(id); + } + } + expect(missing).toEqual([]); + }); + + test('the allow-list admits nothing that is not granted', () => { + // The other direction, which the old test could not see: an id admitted here but + // absent from the grant list passes admission and then fails at turn 0 with + // AccessDenied. The file's own comment warns about exactly this; now it is + // enforced instead of hoped for. + const grantedForms = new Set(); + for (const bare of DEFAULT_BEDROCK_MODEL_IDS) { + grantedForms.add(bare); + for (const geo of DEPLOYABLE_GEOS) grantedForms.add(`${geo}.${bare}`); + } + const ungranted = WORKFLOW_MODEL_ALLOWLIST.filter((id) => !grantedForms.has(id)); + expect(ungranted).toEqual([]); }); }); diff --git a/cli/src/commands/repo.ts b/cli/src/commands/repo.ts index 75ad77108..e920c0c11 100644 --- a/cli/src/commands/repo.ts +++ b/cli/src/commands/repo.ts @@ -173,12 +173,14 @@ export function makeRepoCommand(): Command { const { region, stackName } = resolveOperatorContext(opts); const [ tableName, platformRuntimeArn, platformGithubTokenSecretArn, computeSubstrate, deployedGeo, + grantedModelIds, ] = await Promise.all([ getStackOutput(region, stackName, 'RepoTableName'), getStackOutput(region, stackName, 'RuntimeArn'), getStackOutput(region, stackName, 'GitHubTokenSecretArn'), getStackOutput(region, stackName, 'ComputeSubstrate'), getStackOutput(region, stackName, 'BedrockGeoRegion'), + getStackOutput(region, stackName, 'BedrockModelIds'), ]); if (!tableName) { throw new CliError( @@ -210,7 +212,12 @@ export function makeRepoCommand(): Command { // Same reasoning as the substrate gate above: reuse an output already // fetched, and fail here rather than let a task die at turn 0 with an // AccessDenied that names nothing. - assertModelIdUsable({ modelId: opts.model, deployedGeo, stackName }); + assertModelIdUsable({ + modelId: opts.model, + deployedGeo, + stackName, + ...(grantedModelIds && { grantedBareIds: grantedModelIds.split(',').filter(Boolean) }), + }); const config = await onboardRepo(region, tableName, repoId, { computeType: opts.computeType, diff --git a/cli/src/model-id.ts b/cli/src/model-id.ts index 852cd84ee..d8bcf8e6c 100644 --- a/cli/src/model-id.ts +++ b/cli/src/model-id.ts @@ -69,16 +69,37 @@ export function assertModelIdUsable(args: { modelId: string | undefined; deployedGeo: string | null | undefined; stackName: string; + /** Bare ids the stack grants, from its `BedrockModelIds` output. */ + grantedBareIds?: readonly string[]; }): void { - const { modelId, deployedGeo, stackName } = args; + const { modelId, deployedGeo, stackName, grantedBareIds } = args; if (!modelId) return; const geo = geoPrefixOf(modelId); if (!geo) { + // Suggest a geography only when the stack told us one. Naming `us` on a stack + // deployed as `global` would hand the operator a value it does not grant — + // trading one turn-0 failure for another. + const suggestion = deployedGeo + ? ` Use the inference-profile form: '${deployedGeo}.${modelId}'.` + : " Use the inference-profile form, prefixed with the deployment's geography " + + "(e.g. 'global.' or 'us.')."; throw new CliError( `--model '${modelId}' looks like a bare foundation-model id, which Bedrock cannot ` - + 'invoke on demand — a task using it would fail at turn 0 with a ValidationException. ' - + `Use the inference-profile form: '${deployedGeo ?? 'us'}.${modelId}'.`, + + 'invoke on demand — a task using it would fail at turn 0 with a ValidationException.' + + suggestion, + ); + } + + // Membership in the granted set. Skipped when the stack does not export it, so an + // older stack is not blocked — but checked whenever the information exists, + // because this is the case that otherwise reaches turn 0 with no explanation. + const bare = modelId.slice(geo.length + 1); + if (grantedBareIds && grantedBareIds.length > 0 && !grantedBareIds.includes(bare)) { + throw new CliError( + `--model '${modelId}' is not granted by stack '${stackName}'. It grants: ` + + `${grantedBareIds.join(', ')}. A task using an ungranted model fails at turn 0 with ` + + 'AccessDenied. Add it with -c bedrockModels=\'[…]\' and redeploy, or pick a granted one.', ); } diff --git a/cli/src/platform-doctor.ts b/cli/src/platform-doctor.ts index c3f52eb9c..00f6dc56f 100644 --- a/cli/src/platform-doctor.ts +++ b/cli/src/platform-doctor.ts @@ -449,7 +449,11 @@ async function checkBedrockInferenceProfile( } const profileId = `${geoRegion}.${bareModelId}`; - const label = `Bedrock inference profile (${profileId})`; + // Labelled as VISIBILITY, not readiness. This resolves under the operator's + // credentials while tasks invoke under the workload role, so a PASS here can + // coexist with AccessDenied at turn 0 — and a PASS feeding "All checks passed" + // would otherwise read as "the workload can call this". + const label = `Bedrock inference profile visible (${profileId})`; const bedrock = makeClient(BedrockClient, { region }); try { await bedrock.send(new GetInferenceProfileCommand({ inferenceProfileIdentifier: profileId })); @@ -460,7 +464,9 @@ async function checkBedrockInferenceProfile( // Deliberately not claiming invocability: resolving a profile proves it // exists and is reachable, not that a task can call it. Only InvokeModel // proves that, and doctor does not spend a token to find out. - detail: `Inference profile ${profileId} resolves in ${region}.`, + detail: `Inference profile ${profileId} resolves in ${region} for these operator ` + + 'credentials. Does not prove the workload role can invoke it — that grant is ' + + 'checked at task time.', }; } catch (err) { const message = err instanceof Error ? err.message : String(err); diff --git a/cli/test/model-id.test.ts b/cli/test/model-id.test.ts index 0dd660ad9..412d138ea 100644 --- a/cli/test/model-id.test.ts +++ b/cli/test/model-id.test.ts @@ -20,8 +20,13 @@ import { BEDROCK_GEO_PREFIXES, assertModelIdUsable, geoPrefixOf } from '../src/model-id'; const STACK = 'Abca'; -const check = (modelId: string | undefined, deployedGeo: string | null) => - () => assertModelIdUsable({ modelId, deployedGeo, stackName: STACK }); +const check = ( + modelId: string | undefined, + deployedGeo: string | null, + grantedBareIds?: readonly string[], +) => () => assertModelIdUsable({ modelId, deployedGeo, stackName: STACK, grantedBareIds }); + +const GRANTED = ['anthropic.claude-opus-5', 'anthropic.claude-sonnet-4-6']; describe('assertModelIdUsable', () => { it('accepts a profile matching the deployed geography', () => { @@ -74,6 +79,34 @@ describe('assertModelIdUsable', () => { // with the geography comparison. expect(check('anthropic.claude-opus-5', null)).toThrow(/bare foundation-model id/); }); + + it('rejects a model the stack does not grant, and lists what it does', () => { + // The gap the first version of this guard left open: well-formed, right geography, + // granted nothing. It reached turn 0 as an AccessDenied naming no model. + const t = check('global.anthropic.example-ungranted-model', 'global', GRANTED); + expect(t).toThrow(/is not granted by stack/); + expect(t).toThrow(/anthropic\.claude-opus-5/); + expect(check('global.not-a-bedrock-model', 'global', GRANTED)).toThrow(/not granted/); + }); + + it('accepts a granted model', () => { + expect(check('global.anthropic.claude-opus-5', 'global', GRANTED)).not.toThrow(); + }); + + it('skips the grant check when the stack does not export the set', () => { + // An older stack has no BedrockModelIds output. Blocking every --model there + // would be worse than the gap this closes. + expect(check('global.anthropic.whatever', 'global')).not.toThrow(); + expect(check('global.anthropic.whatever', 'global', [])).not.toThrow(); + }); + + it('does not prescribe a geography it was not told', () => { + // Suggesting `us.` on a stack deployed as `global` trades one turn-0 failure for + // another. With no exported geography the message must offer examples, not a value. + const t = check('anthropic.claude-opus-5', null); + expect(t).toThrow(/prefixed with the deployment's geography/); + expect(t).not.toThrow(/Use the inference-profile form: 'us\./); + }); }); describe('geoPrefixOf', () => { diff --git a/docs/abca-plugin/skills/onboard-repo/SKILL.md b/docs/abca-plugin/skills/onboard-repo/SKILL.md index 7811155fb..f3a466f86 100644 --- a/docs/abca-plugin/skills/onboard-repo/SKILL.md +++ b/docs/abca-plugin/skills/onboard-repo/SKILL.md @@ -40,7 +40,9 @@ Use AskUserQuestion to collect (only the repository is required — the rest fal - **Repository** — GitHub `owner/repo`. Must match exactly what's passed to `bgagent submit --repo` later. - **Compute type** — `agentcore` (default) or `ecs`. -- **Model** — default is the platform model (Sonnet 4.6). If overriding, it must be a model **already granted to the runtime** (see "Model not yet wired into the runtime"), specified as a cross-Region **inference-profile ID** (e.g. `us.anthropic.claude-sonnet-4-6`), not a raw `anthropic.*` foundation-model ID. +- **Model** — default is the platform model (Opus 5). The geo prefix in the examples + below (`global.`) must match the deployment's `bedrockGeoRegion`; `bgagent repo + onboard` rejects a mismatch at the CLI rather than letting the task fail at turn 0. If overriding, it must be a model **already granted to the runtime** (see "Model not yet wired into the runtime"), specified as a cross-Region **inference-profile ID** (e.g. `global.anthropic.claude-opus-5`), not a raw `anthropic.*` foundation-model ID. - **Max turns** — default 100 (range 1–500). - **Per-repo GitHub token** — only if this repo needs a different token than the platform default (provide its Secrets Manager ARN). @@ -57,7 +59,7 @@ no `cdk deploy`.** ```bash bgagent repo onboard # common overrides: -# --model e.g. us.anthropic.claude-sonnet-4-6 (must be runtime-granted) +# --model e.g. global.anthropic.claude-opus-5 (must be runtime-granted) # --compute-type # --max-turns per-repo default turn limit # --token-secret-arn per-repo GitHub token (else platform default) @@ -76,7 +78,7 @@ That's it — the repo is onboarded. Submit a task with the `submit-task` skill. **Pick a model that is already wired into the runtime.** With no `--model`, the repo uses the platform default (Sonnet 4.6). If you pass `--model`, use a cross-Region -**inference profile ID** (e.g. `us.anthropic.claude-sonnet-4-6`), not a raw +**inference profile ID** (e.g. `global.anthropic.claude-opus-5`), not a raw `anthropic.*` foundation-model ID. Only models the stack has granted the runtime can be invoked — see "Model not yet wired into the runtime" before choosing a model the deployment doesn't already support. @@ -97,7 +99,7 @@ editing the stack and redeploying. repoTable: repoTable.table, // Optional overrides: // computeType: 'agentcore', - // modelId: 'us.anthropic.claude-sonnet-4-6', + // modelId: 'global.anthropic.claude-opus-5', // maxTurns: 100, // maxBudgetUsd: 50, // githubTokenSecretArn: 'arn:aws:secretsmanager:...', @@ -116,7 +118,7 @@ editing the stack and redeploying. A repo can only use a model the **runtime IAM role has `grantInvoke` for**. As of now the stack wires **Sonnet 4.6, Opus 4 (`claude-opus-4-20250514`), and Haiku 4.5** (see the `grantInvoke` block in `agent.ts`). Onboarding a repo pinned to any **other** model -(e.g. Opus 4.8 / `us.anthropic.claude-opus-4-8`) will fail at invoke with a 403 — the +(e.g. Opus 4.8 / `global.anthropic.claude-opus-4-8`) will fail at invoke with a 403 — the CLI onboard succeeds, but tasks can't run. Adding a new model **is** a platform source change, so it follows ADR-003 (issue → @@ -164,6 +166,6 @@ Task-level parameters override per-repo defaults; if neither specifies a value, - **`REPO_NOT_ONBOARDED` / 422** — the repo isn't registered. Run `bgagent repo onboard ` (Path A). Confirm the `owner/repo` matches exactly what you pass to `bgagent submit --repo`. - **Preflight failure after onboarding** — the GitHub PAT lacks access to the new repo. Ensure the token has Contents (read/write) + Pull requests (read/write) on it, or onboard with a repo-specific `--token-secret-arn`. -- **400 "Invocation with on-demand throughput isn't supported"** — `model_id` is a raw foundation-model ID; use the inference-profile ID (e.g. `us.anthropic.claude-sonnet-4-6`). +- **400 "Invocation with on-demand throughput isn't supported"** — `model_id` is a raw foundation-model ID; use the inference-profile ID (e.g. `global.anthropic.claude-opus-5`). - **403 "not authorized to perform bedrock:InvokeModelWithResponseStream"** — the repo's model isn't wired into the runtime. See "Model not yet wired into the runtime." - **Model not available / "not available on your Bedrock deployment"** — account-level Bedrock access isn't enabled for that model/Region (separate from IAM); complete [model access](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html), then use an enabled inference-profile ID. diff --git a/docs/abca-plugin/skills/troubleshoot/SKILL.md b/docs/abca-plugin/skills/troubleshoot/SKILL.md index 38d3e058a..8d4b4f23f 100644 --- a/docs/abca-plugin/skills/troubleshoot/SKILL.md +++ b/docs/abca-plugin/skills/troubleshoot/SKILL.md @@ -112,18 +112,18 @@ node cli/lib/bin/bgagent.js events --output json **403 "not authorized to perform bedrock:InvokeModelWithResponseStream":** - The repo's `model_id` is a model the runtime IAM role wasn't granted. The runtime only has `grantInvoke` for the models in the stack's configured set (Sonnet 4.6, Opus 4, Haiku 4.5 by default). -- **Quick fix:** point the repo at an already-granted model — `bgagent repo onboard --model us.anthropic.claude-sonnet-4-6` (no redeploy). +- **Quick fix:** point the repo at an already-granted model — `bgagent repo onboard --model global.anthropic.claude-opus-5` (no redeploy). - **To add a new model to the runtime:** grant it in the stack and redeploy. The model set is the shared list in `cdk/src/constructs/bedrock-models.ts` — add the model via the `bedrockModels` CDK context (`cdk.json`) so both the AgentCore and ECS backends grant it (#433). Adding a model also requires **account-level Bedrock access** for it (separate from IAM — see the next row). **Model not enabled / "not available on your Bedrock deployment" (often immediate failure, few turns, zero or near-zero tokens):** - **IAM is necessary but not sufficient.** The AgentCore role may already have `bedrock:InvokeModel*`, but the **account** must also satisfy [Amazon Bedrock model access](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html): Marketplace subscription flow on first serverless use (with `aws-marketplace:Subscribe` / `ViewSubscriptions` where needed), Anthropic **first-time use** details (`PutUseCaseForModelAccess` or the console model catalog), and a valid payment method for Marketplace-backed models. -- **Use an inference profile ID** in the Blueprint / DynamoDB `model_id` when Bedrock requires it for on-demand invocation (for example `us.anthropic.claude-sonnet-4-6` for US Sonnet 4.6). See [Use an inference profile in model invocation](https://docs.aws.amazon.com/bedrock/latest/userguide/inference-profiles-use.html). Raw `anthropic.*` IDs often hit "on-demand not supported" or wrong routing — see the **400** row below. +- **Use an inference profile ID** in the Blueprint / DynamoDB `model_id` when Bedrock requires it for on-demand invocation (for example `global.anthropic.claude-opus-5` for global Opus 5). See [Use an inference profile in model invocation](https://docs.aws.amazon.com/bedrock/latest/userguide/inference-profiles-use.html). Raw `anthropic.*` IDs often hit "on-demand not supported" or wrong routing — see the **400** row below. - **Cross-Region profiles** route across Regions in a geography; ensure IAM and any SCPs allow Bedrock in **all destination Regions** for that profile. See [Supported Regions and models for inference profiles](https://docs.aws.amazon.com/bedrock/latest/userguide/inference-profiles-support.html). - **Task status:** When the Claude CLI reports a terminal error via `ResultMessage.is_error`, the agent marks the task **FAILED** (not COMPLETED) and persists `error_message` in DynamoDB. **400 "Invocation with on-demand throughput isn't supported":** - The Blueprint `modelId` uses a raw foundation model ID (e.g. `anthropic.claude-opus-4-20250514-v1:0`) -- Fix: change to the inference profile ID (e.g. `us.anthropic.claude-opus-4-20250514-v1:0`), update DynamoDB via redeploy +- Fix: change to the inference profile ID (e.g. `global.anthropic.claude-opus-4-20250514-v1:0`), update DynamoDB via redeploy **503 "Too many connections" / task completes with 0 tokens after long duration:** - Bedrock is throttling model invocations. The agent retries for minutes then gives up. diff --git a/docs/guides/DEVELOPER_GUIDE.md b/docs/guides/DEVELOPER_GUIDE.md index 0c88e35db..19bddd2cd 100644 --- a/docs/guides/DEVELOPER_GUIDE.md +++ b/docs/guides/DEVELOPER_GUIDE.md @@ -122,7 +122,7 @@ See the [Cedar policy guide](./CEDAR_POLICY_GUIDE.md) for the full authoring ref | # | Layer | What it controls | Where | ID form | |---|---|---|---|---| -| 1 | **IAM invoke allowlist** | Which models the agent's roles may invoke at all. The outer gate — everything below fails without it. | `DEFAULT_BEDROCK_MODEL_IDS` (`cdk/src/constructs/bedrock-models.ts:34`); override with CDK context `bedrockModels` (key at `:48`, resolver at `:67`) | **Bare** (`anthropic.claude-…`) | +| 1 | **IAM invoke allowlist** | Which models the agent's roles may invoke at all, and in which geography. The outer gate — everything below fails without it. | `DEFAULT_BEDROCK_MODEL_IDS` (`cdk/src/constructs/bedrock-models.ts`); override with CDK context `bedrockModels`. Geography via context `bedrockGeoRegion` (default `us`) | **Bare** (`anthropic.claude-…`) | | 2 | **Platform default model** | The model used when nothing narrower is set. A **Python literal only** — there is no CDK prop or environment knob in front of it today. | `agent/src/config.py:563` (the `ANTHROPIC_MODEL` fallback) and `agent/src/models.py:157` (`TaskConfig.anthropic_model`) | Prefixed (`us.anthropic.…`) | | 3 | **Auxiliary / fast model** | The small model Claude Code uses for auxiliary work (WebFetch page summarization, the pre-flight safety check). | Stack env `ANTHROPIC_DEFAULT_HAIKU_MODEL` (`cdk/src/stacks/agent.ts` (the runtime environment block)); agent-side fallback at `agent/src/config.py:569` | Prefixed (`us.anthropic.…`) | | 4 | **Per-repo override** | One repository's model, with no agent redeploy. | Blueprint `agent.modelId` (`cdk/src/constructs/blueprint.ts`, `BlueprintProps.agent.modelId`) → RepoTable `model_id` (`cdk/src/handlers/shared/repo-config.ts:37`) → ECS injects `ANTHROPIC_MODEL` (`cdk/src/handlers/shared/strategies/ecs-strategy.ts:217`) | Prefixed (`us.anthropic.…`) | @@ -149,7 +149,7 @@ Every one of those is gated by the **IAM invoke allowlist** (layer 1), which is ### Bare vs. prefixed IDs — the one rule that bites -Layer 1 takes **bare foundation-model IDs**; every other layer takes the **prefixed inference-profile ID**. This asymmetry is deliberate: both grant sites derive the inference-profile ARN by *adding* the `us.` prefix themselves, so a prefixed entry in `bedrockModels` would produce an invalid `us.us.anthropic.…` ARN. The resolver rejects a `us.`/`eu.`/`apac.`-prefixed entry at `cdk/src/constructs/bedrock-models.ts:84` so the typo fails at synth rather than at runtime. +Layer 1 takes **bare foundation-model IDs**; every other layer takes the **prefixed inference-profile ID**. This asymmetry is deliberate: both grant sites derive the inference-profile ARN by *adding* the geo prefix themselves, so a prefixed entry in `bedrockModels` would produce an invalid `us.us.anthropic.…` ARN. The resolver rejects an entry carrying any modelled geo prefix so the typo fails at synth rather than at runtime. In the other direction, a **bare** ID cannot be invoked on demand at all. Verified: @@ -160,7 +160,28 @@ throughput isn't supported. Retry your request with the ID or ARN of an inferenc profile that contains this model. ``` -So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `global.anthropic.claude-opus-5`. +So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `global.anthropic.claude-opus-5` (or whatever geo you have configured — see below). + +### Choosing the inference-profile geography + +Which geography those prefixes name is itself configurable, via CDK context `bedrockGeoRegion`: + +```console +$ cdk deploy -c bedrockGeoRegion=global +``` + +or as a `context` entry in `cdk/cdk.json`. It defaults to `us`, and the accepted values are whatever `@aws-cdk/aws-bedrock-alpha` models — currently `global`, `us`, `us-gov`, `eu`, `apac`, `jp`, `au`. An unrecognized value **fails at synth** rather than at deploy: an invented geography produces a well-formed ARN for a profile that does not exist, so the grant would authorize nothing and the agent would fail at turn 0 with `AccessDenied` and nothing to explain why. + +One value drives everything that needs a prefix — both grant sites (the AgentCore runtime and the ECS task role) and the layer-3 `ANTHROPIC_DEFAULT_HAIKU_MODEL` env var. That is deliberate: a deployment can never grant one geography's profiles while telling the agent to call another's. + +**Which to choose.** A `global.` profile routes to any supported commercial Region, which gives better throughput and resilience under peak demand — worth having for tasks that run for hours and burst. A geo profile (`us.`, `eu.`, `apac.`, …) keeps inference within that geography, which is what you need under a **data-residency requirement**. Pick the geo profile in that case; the throughput benefit is not worth a compliance breach. + +Two things to check when changing it, both of which fail at runtime rather than synth: + +- The models you use must have an active profile in the target geography. Not every model is published to every geo. +- Your account's [Bedrock model access](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html) must cover the deployment Region's entitlements for that geography. + +Changing the geography requires a **redeploy**, because the IAM grants are scoped to explicit profile ARNs resolved at synth. Switching among already-granted models does not — see layer 4. ### Bumping the default model diff --git a/docs/src/content/docs/developer-guide/Model-configuration.md b/docs/src/content/docs/developer-guide/Model-configuration.md index 39ea033a2..c39db314b 100644 --- a/docs/src/content/docs/developer-guide/Model-configuration.md +++ b/docs/src/content/docs/developer-guide/Model-configuration.md @@ -8,7 +8,7 @@ title: Model configuration | # | Layer | What it controls | Where | ID form | |---|---|---|---|---| -| 1 | **IAM invoke allowlist** | Which models the agent's roles may invoke at all. The outer gate — everything below fails without it. | `DEFAULT_BEDROCK_MODEL_IDS` (`cdk/src/constructs/bedrock-models.ts:34`); override with CDK context `bedrockModels` (key at `:48`, resolver at `:67`) | **Bare** (`anthropic.claude-…`) | +| 1 | **IAM invoke allowlist** | Which models the agent's roles may invoke at all, and in which geography. The outer gate — everything below fails without it. | `DEFAULT_BEDROCK_MODEL_IDS` (`cdk/src/constructs/bedrock-models.ts`); override with CDK context `bedrockModels`. Geography via context `bedrockGeoRegion` (default `us`) | **Bare** (`anthropic.claude-…`) | | 2 | **Platform default model** | The model used when nothing narrower is set. A **Python literal only** — there is no CDK prop or environment knob in front of it today. | `agent/src/config.py:563` (the `ANTHROPIC_MODEL` fallback) and `agent/src/models.py:157` (`TaskConfig.anthropic_model`) | Prefixed (`us.anthropic.…`) | | 3 | **Auxiliary / fast model** | The small model Claude Code uses for auxiliary work (WebFetch page summarization, the pre-flight safety check). | Stack env `ANTHROPIC_DEFAULT_HAIKU_MODEL` (`cdk/src/stacks/agent.ts` (the runtime environment block)); agent-side fallback at `agent/src/config.py:569` | Prefixed (`us.anthropic.…`) | | 4 | **Per-repo override** | One repository's model, with no agent redeploy. | Blueprint `agent.modelId` (`cdk/src/constructs/blueprint.ts`, `BlueprintProps.agent.modelId`) → RepoTable `model_id` (`cdk/src/handlers/shared/repo-config.ts:37`) → ECS injects `ANTHROPIC_MODEL` (`cdk/src/handlers/shared/strategies/ecs-strategy.ts:217`) | Prefixed (`us.anthropic.…`) | @@ -35,7 +35,7 @@ Every one of those is gated by the **IAM invoke allowlist** (layer 1), which is ### Bare vs. prefixed IDs — the one rule that bites -Layer 1 takes **bare foundation-model IDs**; every other layer takes the **prefixed inference-profile ID**. This asymmetry is deliberate: both grant sites derive the inference-profile ARN by *adding* the `us.` prefix themselves, so a prefixed entry in `bedrockModels` would produce an invalid `us.us.anthropic.…` ARN. The resolver rejects a `us.`/`eu.`/`apac.`-prefixed entry at `cdk/src/constructs/bedrock-models.ts:84` so the typo fails at synth rather than at runtime. +Layer 1 takes **bare foundation-model IDs**; every other layer takes the **prefixed inference-profile ID**. This asymmetry is deliberate: both grant sites derive the inference-profile ARN by *adding* the geo prefix themselves, so a prefixed entry in `bedrockModels` would produce an invalid `us.us.anthropic.…` ARN. The resolver rejects an entry carrying any modelled geo prefix so the typo fails at synth rather than at runtime. In the other direction, a **bare** ID cannot be invoked on demand at all. Verified: @@ -46,7 +46,28 @@ throughput isn't supported. Retry your request with the ID or ARN of an inferenc profile that contains this model. ``` -So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `global.anthropic.claude-opus-5`. +So: `bedrockModels` context → `anthropic.claude-opus-5`. Everywhere else → `global.anthropic.claude-opus-5` (or whatever geo you have configured — see below). + +### Choosing the inference-profile geography + +Which geography those prefixes name is itself configurable, via CDK context `bedrockGeoRegion`: + +```console +$ cdk deploy -c bedrockGeoRegion=global +``` + +or as a `context` entry in `cdk/cdk.json`. It defaults to `us`, and the accepted values are whatever `@aws-cdk/aws-bedrock-alpha` models — currently `global`, `us`, `us-gov`, `eu`, `apac`, `jp`, `au`. An unrecognized value **fails at synth** rather than at deploy: an invented geography produces a well-formed ARN for a profile that does not exist, so the grant would authorize nothing and the agent would fail at turn 0 with `AccessDenied` and nothing to explain why. + +One value drives everything that needs a prefix — both grant sites (the AgentCore runtime and the ECS task role) and the layer-3 `ANTHROPIC_DEFAULT_HAIKU_MODEL` env var. That is deliberate: a deployment can never grant one geography's profiles while telling the agent to call another's. + +**Which to choose.** A `global.` profile routes to any supported commercial Region, which gives better throughput and resilience under peak demand — worth having for tasks that run for hours and burst. A geo profile (`us.`, `eu.`, `apac.`, …) keeps inference within that geography, which is what you need under a **data-residency requirement**. Pick the geo profile in that case; the throughput benefit is not worth a compliance breach. + +Two things to check when changing it, both of which fail at runtime rather than synth: + +- The models you use must have an active profile in the target geography. Not every model is published to every geo. +- Your account's [Bedrock model access](https://docs.aws.amazon.com/bedrock/latest/userguide/model-access.html) must cover the deployment Region's entitlements for that geography. + +Changing the geography requires a **redeploy**, because the IAM grants are scoped to explicit profile ARNs resolved at synth. Switching among already-granted models does not — see layer 4. ### Bumping the default model From b51712c6d7b6bfc861b8aa803808d5b0657d2199 Mon Sep 17 00:00:00 2001 From: Sphia Sadek Date: Thu, 27 Aug 2026 12:44:14 -0400 Subject: [PATCH 5/5] fix(cdk): clear a dropped model override, widen the drift guard, correct false comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review round. Two findings were already fixed by the previous commit (TaskConfig's geography, and the comment claiming bedrockGeoRegion kept both models aligned — it now does, because both are injected from it). The rest are addressed here. FINDING 1 — a Blueprint that drops `agent.modelId` left the old `model_id` live in DynamoDB, because the update only ever SET fields and never REMOVEd them. The repo kept overriding the platform default with nothing in the Blueprint source saying so, and after a geography change that surviving override named a profile the stack no longer granted: every task on that repo failed at turn 0 while the source looked clean. Extended the REMOVE clause the asset refs already used — same mechanism, same reason — and guarded both directions, since removing it unconditionally would break a legitimate override. FINDING 2 — the SDK smoke diagnostic was unreliable in four ways, each of which made it say something untrue. It defaulted to a model and geography the platform no longer uses (so it silently probed the wrong thing — now it requires ANTHROPIC_MODEL rather than guessing); it printed a hardcoded SDK version four minors stale (removed, since it already reads the real one); it printed FAIL and exited 0, so every caller that checked the status read a failure as success; and its PASS attributed the cause to "threading" when all it establishes is that the SDK/CLI/Bedrock path works. FINDING 3 — the documentation drift guard had blind spots that let the review mutate a documented default to `us-gov.anthropic.claude-stale-test` with every test still green. Its geography pattern omitted us-gov, jp and au; it read only config.py, so models.py could drift freely; and it never looked at the operator skills, which is where an operator actually copies a value from. All three closed, and each is now confirmed to fail against a deliberately introduced drift. FINDING 4 — the skills recommended `us.` overrides this branch would now reject, and still called Sonnet 4.6 the default. Corrected, with a note that the prefix must match the deployment's geography rather than being a fixed string. FINDING 5 — comments asserting things that are not true: that the two guarded mistakes produce the same error (a bare id raises ValidationException from Bedrock, a wrong geography raises AccessDenied from IAM — a real distinction when reading a failure), and that `us` is "the default" when the shipped cdk.json sets `global`. FINDING 6 — the geography list existed twice in the CLI, which is exactly how the strip bug happened; doctor now imports the one list. Also: agent README said Node 20 while the Dockerfile installs 24, the interactive-agents doc cited an SDK pin four minors stale, and the Blueprint header described its update path as PutItem when it is an UpdateItem. One existing test was rewritten rather than re-pinned. It asserted the update expression contained no "REMOVE" at all, to mean "asset refs are not dropped" — a blanket claim that coupled it to an unrelated field and failed for the wrong reason as soon as `model_id` was legitimately removed. It now checks the REMOVE clause per column. The first attempt at that split on the word and swept in ExpressionAttributeNames, which matched every column; scoped to the expression. --- agent/README.md | 2 +- agent/scripts/diagnostics/test_sdk_smoke.py | 31 ++++++++++++---- cdk/src/constructs/bedrock-models.ts | 4 +-- cdk/src/constructs/blueprint.ts | 32 ++++++++++++----- cdk/src/stacks/agent.ts | 2 +- cdk/test/constructs/blueprint.test.ts | 33 +++++++++++++++-- .../model-default-docs-parity.test.ts | 35 +++++++++++++++++-- cli/src/model-id.ts | 8 +++-- cli/src/platform-doctor.ts | 17 ++++----- docs/design/INTERACTIVE_AGENTS.md | 4 +-- docs/guides/DEVELOPER_GUIDE.md | 8 ++--- .../docs/architecture/Interactive-agents.md | 4 +-- .../developer-guide/Model-configuration.md | 8 ++--- 13 files changed, 140 insertions(+), 48 deletions(-) diff --git a/agent/README.md b/agent/README.md index d05ef0329..3044a0a99 100644 --- a/agent/README.md +++ b/agent/README.md @@ -384,7 +384,7 @@ docker images bgagent-local --format "{{.Size}}" ``` agent/ -├── Dockerfile Python 3.13 + Node.js 20 + Claude Code CLI + git + gh + mise (default platform linux/arm64) +├── Dockerfile Python 3.13 + Node.js 24 + Claude Code CLI + git + gh + mise (default platform linux/arm64) ├── .dockerignore ├── pyproject.toml App dependencies (claude-agent-sdk, FastAPI, boto3, OpenTelemetry distro, MCP, cedarpy, …) ├── uv.lock Locked deps for reproducible `uv sync` in the image diff --git a/agent/scripts/diagnostics/test_sdk_smoke.py b/agent/scripts/diagnostics/test_sdk_smoke.py index a0d2f7203..6a45ef8eb 100644 --- a/agent/scripts/diagnostics/test_sdk_smoke.py +++ b/agent/scripts/diagnostics/test_sdk_smoke.py @@ -26,12 +26,24 @@ async def smoke_test(): # Ensure required env vars os.environ.setdefault("CLAUDE_CODE_USE_BEDROCK", "1") region = os.environ.get("AWS_REGION", "") - model = os.environ.get("ANTHROPIC_MODEL", "us.anthropic.claude-sonnet-4-6") + # No hardcoded fallback: this file drifted to a model and geography the platform + # no longer defaults to, so the diagnostic silently tested something other than + # what runs. Require the caller to state it — a diagnostic that quietly probes the + # wrong model is worse than one that refuses. + model = os.environ.get("ANTHROPIC_MODEL", "") if not region: print("ERROR: AWS_REGION not set", file=sys.stderr) sys.exit(1) + if not model: + print( + "ERROR: ANTHROPIC_MODEL not set. Pass the geo-prefixed inference-profile " + "id the deployment uses, e.g. ANTHROPIC_MODEL=global.anthropic.claude-opus-5", + file=sys.stderr, + ) + sys.exit(1) + print(f"Region: {region}") print(f"Model: {model}") print(f"Python: {sys.version}") @@ -108,9 +120,12 @@ def on_stderr(line: str): print(f"Duration: {elapsed:.1f}s") print(f"Counts: {counts}") - if counts["assistant"] > 0 and counts["result"] > 0: - print("\nPASS — SDK yields messages. Issue is specific to the") - print(" server threading context, not SDK/CLI/Bedrock.") + ok = counts["assistant"] > 0 and counts["result"] > 0 + if ok: + print("\nPASS — SDK yields messages for this model in this Region.") + print(" Rules OUT the SDK/CLI/Bedrock path. It does not identify the") + print(" cause of any other failure — threading is one candidate, not a") + print(" conclusion this test can reach.") elif counts["system"] > 0 and counts["assistant"] == 0: print("\nFAIL — Got init but zero AssistantMessages.") print(" Same symptom as production. Issue is SDK/CLI level,") @@ -118,7 +133,6 @@ def on_stderr(line: str): print(" 1. CLI stderr output above for errors") print(" 2. Bedrock model availability / permissions") print(" 3. SDK ↔ CLI version compatibility") - print(" SDK: claude-agent-sdk==0.1.43") try: import importlib.metadata @@ -132,6 +146,11 @@ def on_stderr(line: str): if errors: print(f"\nErrors: {errors}") + # Exit code must match the verdict. It printed FAIL and exited 0, so any caller + # that checked the status — CI, a script, a person using `&&` — read a failure as + # success. A diagnostic that lies in its exit code is worse than no diagnostic. + return ok + if __name__ == "__main__": - asyncio.run(smoke_test()) + sys.exit(0 if asyncio.run(smoke_test()) else 1) diff --git a/cdk/src/constructs/bedrock-models.ts b/cdk/src/constructs/bedrock-models.ts index 1d5faf01f..8978bc2f4 100644 --- a/cdk/src/constructs/bedrock-models.ts +++ b/cdk/src/constructs/bedrock-models.ts @@ -48,7 +48,7 @@ export const DEFAULT_BEDROCK_MODEL_IDS: readonly string[] = [ // for on-demand invocation ("ValidationException: … isn't supported. Retry // your request with the ID or ARN of an inference profile"), and both grant // sites derive the geo-prefixed inference-profile ARN — the invocable one — - // from this entry plus `bedrockGeoRegion` (default `us`). Opus 4.8 above + // from this entry plus `bedrockGeoRegion` (`global` in the shipped cdk.json; `us` only if no context is supplied). Opus 4.8 above // stays granted: blueprints may pin it per-repo, so removing it would fail // those repos at turn 0. 'anthropic.claude-opus-5', @@ -165,7 +165,7 @@ export function resolveBedrockGeoRegion(node: Node): CrossRegionInferenceProfile * **Use the bare foundation-model ID (`anthropic.claude-…`), NOT a * geo-prefixed inference-profile ID.** Both grant sites derive the * inference-profile ARN by prefixing the geography from - * {@link resolveBedrockGeoRegion} (`bedrockGeoRegion`, default `us`), so passing + * {@link resolveBedrockGeoRegion} (`bedrockGeoRegion` — `global` in the shipped cdk.json, `us` absent any context), so passing * `us.anthropic.…` here would produce an invalid `us.us.anthropic.…` ARN. The * resolver rejects an entry carrying ANY modelled geo prefix * ({@link BEDROCK_GEO_REGIONS}) to catch that early — including `global.`, diff --git a/cdk/src/constructs/blueprint.ts b/cdk/src/constructs/blueprint.ts index acfa44c22..d526fdd75 100644 --- a/cdk/src/constructs/blueprint.ts +++ b/cdk/src/constructs/blueprint.ts @@ -216,7 +216,9 @@ export interface BlueprintProps { * CDK construct that registers a repository with the platform by writing * a RepoConfig record to the shared RepoTable via a custom resource. * - * Create/Update: PutItem with status='active' and all config fields. + * Create: PutItem with status='active' and all config fields. Update: UpdateItem, + * which SETs the fields a Blueprint declares and REMOVEs the per-repo overrides it + * no longer declares — a SET-only update would leave a dropped override live. * Delete: UpdateItem to set status='removed' and TTL for eventual cleanup. * * NOTE: Timestamps (onboarded_at, updated_at) are captured at CDK synth time, @@ -371,12 +373,12 @@ export class Blueprint extends Construct { parameters: { TableName: props.repoTable.tableName, Key: { repo: { S: props.repo } }, - UpdateExpression: `SET #status = :active, #updated = :now${this.buildUpdateFields(props)}${this.buildRemoveClause()}`, + UpdateExpression: `SET #status = :active, #updated = :now${this.buildUpdateFields(props)}${this.buildRemoveClause(props)}`, ExpressionAttributeNames: { '#status': 'status', '#updated': 'updated_at', ...this.buildExpressionNames(props), - ...this.buildRemoveNames(), + ...this.buildRemoveNames(props), }, ExpressionAttributeValues: { ':active': { S: 'active' }, @@ -491,14 +493,28 @@ export class Blueprint extends Construct { return empty; } - private buildRemoveClause(): string { - const empty = this.emptyAssetFields(); - return empty.length > 0 ? ` REMOVE ${empty.map(f => `#${f}`).join(', ')}` : ''; + /** Per-repo overrides to CLEAR when their prop is dropped, same reason as the + * asset refs above: SET-only means removing `agent.modelId` from a Blueprint + * leaves the old `model_id` live in DynamoDB, so the repo keeps overriding the + * platform default forever. That is worse than stale — after a geography change + * the surviving override names a profile the stack no longer grants, and every + * task on that repo fails at turn 0 with AccessDenied while the Blueprint + * source says nothing is overridden. */ + private clearedOverrideFields(props: BlueprintProps): string[] { + const cleared: string[] = []; + if (!props.agent?.modelId) cleared.push('model_id'); + return cleared; } - private buildRemoveNames(): Record { + private buildRemoveClause(props?: BlueprintProps): string { + const fields = [...this.emptyAssetFields(), ...(props ? this.clearedOverrideFields(props) : [])]; + return fields.length > 0 ? ` REMOVE ${fields.map(f => `#${f}`).join(', ')}` : ''; + } + + private buildRemoveNames(props?: BlueprintProps): Record { const names: Record = {}; - for (const f of this.emptyAssetFields()) names[`#${f}`] = f; + const fields = [...this.emptyAssetFields(), ...(props ? this.clearedOverrideFields(props) : [])]; + for (const f of fields) names[`#${f}`] = f; return names; } } diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index f813aef6d..ebfa81c48 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -653,7 +653,7 @@ export class AgentStack extends Stack { // Grant the runtime invoke on each configured foundation model + its // cross-Region inference profile in the configured geography - // (`bedrockGeoRegion`, default `us`). The model set is a single source of + // (`bedrockGeoRegion` — `global` in the shipped cdk.json, `us` absent any context). The model set is a single source of // truth (constructs/bedrock-models.ts), shared with the ECS task role and // overridable via the `bedrockModels` CDK context. Each invokable is also // collected so the same set is granted to the SessionRole below (for cost diff --git a/cdk/test/constructs/blueprint.test.ts b/cdk/test/constructs/blueprint.test.ts index 96e79bef2..a1edb4e27 100644 --- a/cdk/test/constructs/blueprint.test.ts +++ b/cdk/test/constructs/blueprint.test.ts @@ -424,8 +424,37 @@ describe('Blueprint construct', () => { expect(serialized).toContain('#mcp_servers'); expect(serialized).toContain('#cedar_policy_modules'); expect(serialized).toContain('#skills'); - // All three populated → nothing to REMOVE. - expect(serialized).not.toContain('REMOVE'); + // All three populated → none of the ASSET columns is removed. Asserted per + // column rather than as "no REMOVE anywhere": the clause is shared with the + // per-repo overrides, and this blueprint declares no `agent.modelId`, so it + // legitimately removes `model_id`. A blanket assertion coupled this test to + // an unrelated field and failed for the wrong reason. + // Only the REMOVE clause of the UpdateExpression — not everything after the + // word, which would sweep in ExpressionAttributeNames and match every column. + const removeClause = /REMOVE ([^"\\]*)/.exec(serialized)?.[1] ?? ''; + expect(removeClause).not.toContain('#mcp_servers'); + expect(removeClause).not.toContain('#cedar_policy_modules'); + expect(removeClause).not.toContain('#skills'); + }); + + test('onUpdate REMOVEs model_id when the Blueprint no longer declares one', () => { + // SET-only updates left a dropped `agent.modelId` live in DynamoDB, so the repo + // kept overriding the platform default with no trace of it in the Blueprint + // source. After a geography change that surviving override names a profile the + // stack no longer grants, and every task on the repo fails at turn 0 with + // AccessDenied while the source says nothing is overridden. + const { template } = createStack(); + const removeClause = /REMOVE ([^"\\]*)/.exec(getUpdateJoinParts(template).join(''))?.[1] ?? ''; + expect(removeClause).toContain('#model_id'); + }); + + test('onUpdate does NOT remove model_id when the Blueprint declares one', () => { + // The other direction: a declared override must survive its own redeploy. + const { template } = createStack({ agent: { modelId: 'global.anthropic.claude-opus-5' } }); + const serialized = getUpdateJoinParts(template).join(''); + const removeClause = /REMOVE ([^"\\]*)/.exec(serialized)?.[1] ?? ''; + expect(removeClause).not.toContain('#model_id'); + expect(serialized).toContain('#model_id = :model_id'); }); test('onUpdate REMOVEs asset columns that are now empty (detach on redeploy)', () => { diff --git a/cdk/test/contracts/model-default-docs-parity.test.ts b/cdk/test/contracts/model-default-docs-parity.test.ts index 77fa43dcf..3b870614e 100644 --- a/cdk/test/contracts/model-default-docs-parity.test.ts +++ b/cdk/test/contracts/model-default-docs-parity.test.ts @@ -98,6 +98,35 @@ describe('documented model defaults match the agent runtime defaults', () => { 'agent/README.md', ] as const; + it('models.py TaskConfig defaults match the config.py fallbacks', () => { + // TaskConfig is a SECOND set of literals, reached by direct construction (tests, + // local runs) rather than by a deployed task. It drifted to a different geography + // than config.py and nothing noticed, because this guard only ever read config.py. + const modelsPy = read('agent/src/models.py'); + const field = (name: string) => { + const m = new RegExp(`${name}: str = "([^"]+)"`).exec(modelsPy); + if (!m) throw new Error(`models.py no longer declares ${name}`); + return m[1]; + }; + expect(field('anthropic_model')).toBe(agentDefaultFor('ANTHROPIC_MODEL')); + expect(field('haiku_model')).toBe(agentDefaultFor('ANTHROPIC_DEFAULT_HAIKU_MODEL')); + }); + + it('operator skills do not advertise a different geography than the platform default', () => { + // The skills are what an operator copies from. They recommended `us.` overrides + // on a stack that grants `global.`, which the CLI now rejects — so a doc that is + // merely stale becomes a doc that hands out a value the tooling refuses. + const expectedGeo = /^([a-z-]+)\./.exec(agentDefaultFor('ANTHROPIC_MODEL'))?.[1]; + expect(expectedGeo).toBeDefined(); + for (const skill of ['onboard-repo', 'troubleshoot']) { + const text = read(`docs/abca-plugin/skills/${skill}/SKILL.md`); + const wrong = [...text.matchAll(/\b(global|us-gov|us|eu|apac|jp|au)\.anthropic\.[a-z0-9.:-]+/g)] + .filter((m) => m[1] !== expectedGeo) + .map((m) => m[0]); + expect(wrong).toEqual([]); + } + }); + it.each(DOCS_WITH_ENV_TABLES)('%s documents the real ANTHROPIC_MODEL default', (docPath) => { const expected = agentDefaultFor('ANTHROPIC_MODEL'); const documented = documentedDefaults(read(docPath), 'ANTHROPIC_MODEL'); @@ -123,7 +152,9 @@ describe('documented model defaults match the agent runtime defaults', () => { // (Bedrock returns ValidationException), so documenting one sends readers // down a dead end. Guards the specific bug fixed in agent/README.md. for (const envVar of ['ANTHROPIC_MODEL', 'ANTHROPIC_DEFAULT_HAIKU_MODEL']) { - expect(agentDefaultFor(envVar)).toMatch(/^(us|eu|apac|global)\./); + // All seven geographies the CDK models, not a subset. A stale default under an + // omitted geography (us-gov, jp, au) passed this guard silently. + expect(agentDefaultFor(envVar)).toMatch(/^(global|us-gov|us|eu|apac|jp|au)\./); } }); @@ -160,7 +191,7 @@ describe('documented model defaults match the agent runtime defaults', () => { // legitimately when the docs explain WHY a bare id is not invocable, and the // IAM grant list in bedrock-models.ts is bare-by-contract — both out of scope // here and already covered by bedrock-models.test.ts. - const MODEL_ID = /\b(?:us|eu|apac|global)\.anthropic\.claude-[a-z0-9-]+(?::[0-9]+)?/g; + const MODEL_ID = /\b(?:global|us-gov|us|eu|apac|jp|au)\.anthropic\.claude-[a-z0-9-]+(?::[0-9]+)?/g; // Hand-authored sources plus every generated mirror that actually quotes a // prefixed id (enumerated from the tree, not guessed — `using/Overview.md` // mirrors USER_GUIDE but carries no literal, so listing it would assert diff --git a/cli/src/model-id.ts b/cli/src/model-id.ts index d8bcf8e6c..f08cff8ea 100644 --- a/cli/src/model-id.ts +++ b/cli/src/model-id.ts @@ -45,9 +45,11 @@ export function geoPrefixOf(modelId: string): string | undefined { * Reject a `--model` value that cannot work, at the point the operator can still * fix it. * - * Both mistakes this catches produce the SAME opaque outcome today: the task - * dispatches, runs, and dies at turn 0 with `AccessDenied` — with nothing at - * onboard time and nothing in the failure naming the model as the cause. + * Each mistake this catches fails at turn 0 with nothing naming the model as the + * cause — but NOT with the same error, and the distinction matters when reading a + * failure: a bare id raises `ValidationException` from Bedrock (the id is not + * invocable at all), while a wrong geography or an ungranted model raises + * `AccessDenied` from IAM (the id is fine, the grant is not). * * - A **bare** foundation-model id. Bedrock refuses bare ids for on-demand * invocation of Claude 4.x and later ("ValidationException: … isn't supported. diff --git a/cli/src/platform-doctor.ts b/cli/src/platform-doctor.ts index 00f6dc56f..0fc7d3976 100644 --- a/cli/src/platform-doctor.ts +++ b/cli/src/platform-doctor.ts @@ -32,25 +32,20 @@ import { probeLambdaMicrovmAvailability, } from './lambda-microvm-availability'; import { checkLinearWorkspaceAuth, type LinearProbe, type LinearRefreshVerifier } from './linear-auth-health'; +import { BEDROCK_GEO_PREFIXES } from './model-id'; import { PLATFORM_REPO_DEFAULTS } from './repo-display'; import { listRepoConfigs, RepoConfigRow } from './repo-lookup'; import { getStackOutput } from './stack-outputs'; import { makeClient } from './ua'; /** - * Every cross-Region inference-profile geography, mirroring - * `CrossRegionInferenceProfileRegion` in `@aws-cdk/aws-bedrock-alpha` (the CLI is - * a separate package and does not depend on CDK — see `PLATFORM_REPO_DEFAULTS` - * for the same mirroring). + * Strips a leading `.` inference-profile prefix, if present. * - * Must list them ALL. An earlier version matched only `us|eu|apac`, so when the - * platform default moved to a `global.` profile the strip below silently did - * nothing and `GetFoundationModel` was handed a profile id it cannot resolve — - * turning the one Bedrock check `doctor` performs into a false failure. + * Built from the ONE geography list (`model-id.ts`) rather than a second copy. Two + * copies is how this broke: the list here matched only `us|eu|apac`, so when the + * platform default moved to `global.` the strip silently did nothing and + * `GetFoundationModel` was handed a profile id it cannot resolve. */ -const BEDROCK_GEO_PREFIXES = ['global', 'us-gov', 'us', 'eu', 'apac', 'jp', 'au'] as const; - -/** Strips a leading `.` inference-profile prefix, if present. */ const GEO_PREFIX_RE = new RegExp(`^(?:${BEDROCK_GEO_PREFIXES.join('|')})\\.`); /** diff --git a/docs/design/INTERACTIVE_AGENTS.md b/docs/design/INTERACTIVE_AGENTS.md index 7e611aa0d..c17955d97 100644 --- a/docs/design/INTERACTIVE_AGENTS.md +++ b/docs/design/INTERACTIVE_AGENTS.md @@ -766,12 +766,12 @@ Opt-in per task: 4 KB previews + full trajectory to S3 with TTL. ## Appendix A — Claude Agent SDK reference -Pinned version: `claude-agent-sdk==0.2.82` (Python; see `agent/pyproject.toml`). +Pinned version: `claude-agent-sdk==0.2.110` (Python; see `agent/pyproject.toml`). > **Caution — re-verify against 0.2.x.** The hook-surface details in this > appendix (the `HookEvent` enum members, `PostToolUseFailure`, the Stop-hook > return-value contract, etc.) were originally written against -> `claude-agent-sdk==0.1.53`. The pin has since advanced to `0.2.82`, and the +> `claude-agent-sdk==0.1.53`. The pin has since advanced to `0.2.110`, and the > SDK's hook API may have changed across that range. Before relying on any > specific enum member or hook signature below, verify it against the installed > 0.2.x SDK and the actual usage in `agent/src/hooks.py`. diff --git a/docs/guides/DEVELOPER_GUIDE.md b/docs/guides/DEVELOPER_GUIDE.md index 19bddd2cd..70bcb2b81 100644 --- a/docs/guides/DEVELOPER_GUIDE.md +++ b/docs/guides/DEVELOPER_GUIDE.md @@ -123,10 +123,10 @@ See the [Cedar policy guide](./CEDAR_POLICY_GUIDE.md) for the full authoring ref | # | Layer | What it controls | Where | ID form | |---|---|---|---|---| | 1 | **IAM invoke allowlist** | Which models the agent's roles may invoke at all, and in which geography. The outer gate — everything below fails without it. | `DEFAULT_BEDROCK_MODEL_IDS` (`cdk/src/constructs/bedrock-models.ts`); override with CDK context `bedrockModels`. Geography via context `bedrockGeoRegion` (default `us`) | **Bare** (`anthropic.claude-…`) | -| 2 | **Platform default model** | The model used when nothing narrower is set. A **Python literal only** — there is no CDK prop or environment knob in front of it today. | `agent/src/config.py:563` (the `ANTHROPIC_MODEL` fallback) and `agent/src/models.py:157` (`TaskConfig.anthropic_model`) | Prefixed (`us.anthropic.…`) | -| 3 | **Auxiliary / fast model** | The small model Claude Code uses for auxiliary work (WebFetch page summarization, the pre-flight safety check). | Stack env `ANTHROPIC_DEFAULT_HAIKU_MODEL` (`cdk/src/stacks/agent.ts` (the runtime environment block)); agent-side fallback at `agent/src/config.py:569` | Prefixed (`us.anthropic.…`) | -| 4 | **Per-repo override** | One repository's model, with no agent redeploy. | Blueprint `agent.modelId` (`cdk/src/constructs/blueprint.ts`, `BlueprintProps.agent.modelId`) → RepoTable `model_id` (`cdk/src/handlers/shared/repo-config.ts:37`) → ECS injects `ANTHROPIC_MODEL` (`cdk/src/handlers/shared/strategies/ecs-strategy.ts:217`) | Prefixed (`us.anthropic.…`) | -| 5 | **Per-task / local** | One task's model. Payload `model_id` is aliased to `anthropic_model` (`agent/src/pipeline.py`, `_PAYLOAD_KEY_ALIASES`); local batch runs read `ANTHROPIC_MODEL` from the shell via `agent/run.sh`. | Task payload `model_id`; shell `ANTHROPIC_MODEL` | Prefixed (`us.anthropic.…`) | +| 2 | **Platform default model** | The model used when nothing narrower is set. A **Python literal only** — there is no CDK prop or environment knob in front of it today. | `agent/src/config.py:563` (the `ANTHROPIC_MODEL` fallback) and `agent/src/models.py:157` (`TaskConfig.anthropic_model`) | Prefixed (`.anthropic.…`) | +| 3 | **Auxiliary / fast model** | The small model Claude Code uses for auxiliary work (WebFetch page summarization, the pre-flight safety check). | Stack env `ANTHROPIC_DEFAULT_HAIKU_MODEL` (`cdk/src/stacks/agent.ts` (the runtime environment block)); agent-side fallback at `agent/src/config.py:569` | Prefixed (`.anthropic.…`) | +| 4 | **Per-repo override** | One repository's model, with no agent redeploy. | Blueprint `agent.modelId` (`cdk/src/constructs/blueprint.ts`, `BlueprintProps.agent.modelId`) → RepoTable `model_id` (`cdk/src/handlers/shared/repo-config.ts:37`) → ECS injects `ANTHROPIC_MODEL` (`cdk/src/handlers/shared/strategies/ecs-strategy.ts:217`) | Prefixed (`.anthropic.…`) | +| 5 | **Per-task / local** | One task's model. Payload `model_id` is aliased to `anthropic_model` (`agent/src/pipeline.py`, `_PAYLOAD_KEY_ALIASES`); local batch runs read `ANTHROPIC_MODEL` from the shell via `agent/run.sh`. | Task payload `model_id`; shell `ANTHROPIC_MODEL` | Prefixed (`.anthropic.…`) | ### Environment variables diff --git a/docs/src/content/docs/architecture/Interactive-agents.md b/docs/src/content/docs/architecture/Interactive-agents.md index 616346cba..f968bc830 100644 --- a/docs/src/content/docs/architecture/Interactive-agents.md +++ b/docs/src/content/docs/architecture/Interactive-agents.md @@ -770,12 +770,12 @@ Opt-in per task: 4 KB previews + full trajectory to S3 with TTL. ## Appendix A — Claude Agent SDK reference -Pinned version: `claude-agent-sdk==0.2.82` (Python; see `agent/pyproject.toml`). +Pinned version: `claude-agent-sdk==0.2.110` (Python; see `agent/pyproject.toml`). > **Caution — re-verify against 0.2.x.** The hook-surface details in this > appendix (the `HookEvent` enum members, `PostToolUseFailure`, the Stop-hook > return-value contract, etc.) were originally written against -> `claude-agent-sdk==0.1.53`. The pin has since advanced to `0.2.82`, and the +> `claude-agent-sdk==0.1.53`. The pin has since advanced to `0.2.110`, and the > SDK's hook API may have changed across that range. Before relying on any > specific enum member or hook signature below, verify it against the installed > 0.2.x SDK and the actual usage in `agent/src/hooks.py`. diff --git a/docs/src/content/docs/developer-guide/Model-configuration.md b/docs/src/content/docs/developer-guide/Model-configuration.md index c39db314b..28d00ce4e 100644 --- a/docs/src/content/docs/developer-guide/Model-configuration.md +++ b/docs/src/content/docs/developer-guide/Model-configuration.md @@ -9,10 +9,10 @@ title: Model configuration | # | Layer | What it controls | Where | ID form | |---|---|---|---|---| | 1 | **IAM invoke allowlist** | Which models the agent's roles may invoke at all, and in which geography. The outer gate — everything below fails without it. | `DEFAULT_BEDROCK_MODEL_IDS` (`cdk/src/constructs/bedrock-models.ts`); override with CDK context `bedrockModels`. Geography via context `bedrockGeoRegion` (default `us`) | **Bare** (`anthropic.claude-…`) | -| 2 | **Platform default model** | The model used when nothing narrower is set. A **Python literal only** — there is no CDK prop or environment knob in front of it today. | `agent/src/config.py:563` (the `ANTHROPIC_MODEL` fallback) and `agent/src/models.py:157` (`TaskConfig.anthropic_model`) | Prefixed (`us.anthropic.…`) | -| 3 | **Auxiliary / fast model** | The small model Claude Code uses for auxiliary work (WebFetch page summarization, the pre-flight safety check). | Stack env `ANTHROPIC_DEFAULT_HAIKU_MODEL` (`cdk/src/stacks/agent.ts` (the runtime environment block)); agent-side fallback at `agent/src/config.py:569` | Prefixed (`us.anthropic.…`) | -| 4 | **Per-repo override** | One repository's model, with no agent redeploy. | Blueprint `agent.modelId` (`cdk/src/constructs/blueprint.ts`, `BlueprintProps.agent.modelId`) → RepoTable `model_id` (`cdk/src/handlers/shared/repo-config.ts:37`) → ECS injects `ANTHROPIC_MODEL` (`cdk/src/handlers/shared/strategies/ecs-strategy.ts:217`) | Prefixed (`us.anthropic.…`) | -| 5 | **Per-task / local** | One task's model. Payload `model_id` is aliased to `anthropic_model` (`agent/src/pipeline.py`, `_PAYLOAD_KEY_ALIASES`); local batch runs read `ANTHROPIC_MODEL` from the shell via `agent/run.sh`. | Task payload `model_id`; shell `ANTHROPIC_MODEL` | Prefixed (`us.anthropic.…`) | +| 2 | **Platform default model** | The model used when nothing narrower is set. A **Python literal only** — there is no CDK prop or environment knob in front of it today. | `agent/src/config.py:563` (the `ANTHROPIC_MODEL` fallback) and `agent/src/models.py:157` (`TaskConfig.anthropic_model`) | Prefixed (`.anthropic.…`) | +| 3 | **Auxiliary / fast model** | The small model Claude Code uses for auxiliary work (WebFetch page summarization, the pre-flight safety check). | Stack env `ANTHROPIC_DEFAULT_HAIKU_MODEL` (`cdk/src/stacks/agent.ts` (the runtime environment block)); agent-side fallback at `agent/src/config.py:569` | Prefixed (`.anthropic.…`) | +| 4 | **Per-repo override** | One repository's model, with no agent redeploy. | Blueprint `agent.modelId` (`cdk/src/constructs/blueprint.ts`, `BlueprintProps.agent.modelId`) → RepoTable `model_id` (`cdk/src/handlers/shared/repo-config.ts:37`) → ECS injects `ANTHROPIC_MODEL` (`cdk/src/handlers/shared/strategies/ecs-strategy.ts:217`) | Prefixed (`.anthropic.…`) | +| 5 | **Per-task / local** | One task's model. Payload `model_id` is aliased to `anthropic_model` (`agent/src/pipeline.py`, `_PAYLOAD_KEY_ALIASES`); local batch runs read `ANTHROPIC_MODEL` from the shell via `agent/run.sh`. | Task payload `model_id`; shell `ANTHROPIC_MODEL` | Prefixed (`.anthropic.…`) | ### Environment variables