diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 539501a..9b1b81f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,11 +19,21 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + # `version` is pinned deliberately. ruff-action derives it from the nearest + # pyproject.toml, and this repo has none at the root (the Python projects are + # backend/ and infra/), so it silently resolved to `latest` -- which made this + # job's verdict depend on Astral's release date rather than on the commit. + # 0.16 started formatting Python blocks *inside* markdown, so PRs began + # failing on doc files their authors never touched, and `ruff format --check` + # passing locally no longer predicted CI. Keep this equal to the `ruff` pin in + # backend/pyproject.toml's dev extras; Dependabot bumps them as a reviewable PR. - uses: astral-sh/ruff-action@278981a28ce3188b1e39527901f38254bf3aac89 # v4.1.0 with: + version: 0.16.4 args: check . - uses: astral-sh/ruff-action@278981a28ce3188b1e39527901f38254bf3aac89 # v4.1.0 with: + version: 0.16.4 args: format --check . # Non-blocking while the backlog of type errors is burned down; # flip continue-on-error to false once `npx pyright` is clean. diff --git a/CHANGELOG.md b/CHANGELOG.md index d51a0db..0dd9120 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,105 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Changed — AWS Agent Registry: preview → GA +Agent Registry graduated out of AgentCore into its own AWS service. The rename is +a **silent** break: the deprecated `bedrock-agentcore-control` model still exposes +the Registry operations with the old `descriptorType` parameter, so preview code +keeps "succeeding" against a shim under an IAM prefix it no longer has. Migrated +end-to-end: +- boto3 clients `bedrock-agentcore-control`/`bedrock-agentcore` → + `agent-registry-control`/`agent-registry`; IAM actions `bedrock-agentcore:*` → + `agent-registry:*` (both planes sign as `agent-registry`) +- `descriptorType` → `recordType`, with the enum renamed + `MCP|A2A|CUSTOM|AGENT_SKILLS` → `MCP|AGENT|CUSTOM|SKILL` (preview spellings are + still accepted as input aliases) +- Descriptors reshaped: `a2a.agentCard.inlineContent` → `a2aAgentCard.data`, + `custom.inlineContent` → `custom.data`, `schemaVersion` → `dataSchemaVersion`; + added `mcpServer` and `agentSkillsDefinition` builders +- Data-plane `SearchRegistryRecords` → `SearchDiscoverableRegistryRecords`, with + the GA structured filter shape (`{"recordType": {"$in": [...]}}`) +- `boto3 >= 1.43.66` is now a hard floor (first release carrying the + `agent-registry` service models) in both `pyproject.toml` and + `requirements-lambda.txt` +- `GET /api/registry/aws-config` gained `sdk_supported`, and `POST` now returns a + 400 naming the SDK instead of blaming the `registry_id`, so an under-pinned + bundle is distinguishable from a bad registryId + +All of the below was verified against the live GA service, not just the boto3 +models: a throwaway registry, every descriptor builder submitted through the +shipping adapter, and the approval lifecycle exercised end to end. + +### Fixed — found by live verification against GA +- **Every redeploy silently failed to re-register.** `name` + `recordVersion` is a + uniqueness key and `recordVersion` is `"1.0"` for everything the platform + registers, so the *second* deployment of an agent raised `ConflictException` + inside the best-effort auto-register handler. The symptom was a governance record + frozen at the first deployment's runtime ARN and endpoint — stale forever, with + nothing surfaced anywhere. `register()` is now an upsert (falling back to + `UpdateRegistryRecord`), which needs the new + `agent-registry:UpdateRegistryRecord` grant on the `status_update` step role. + Note updating content demotes a record `APPROVED` → `DRAFT`, so an upsert cannot + slip changed content past an old approval — a redeployed integration must be + re-approved, which is the fail-closed reading. +- **`available()` reported a still-provisioning registry as usable.** It returned + True the instant `GetRegistry` succeeded, but a registry in + `CREATING`/`UPDATING`/`DELETING` rejects `CreateRegistryRecord` with + `ConflictException`. Enabling federation on a freshly created registry — the + common sequence — therefore passed validation and then raced into that conflict + on the first deploy. Now gated on `READY`, with a new `registry_status()` that + keeps "not READY" distinct from "could not ask"; `POST /aws-config` returns 409 + ("still provisioning") instead of a 400 blaming the registryId. +- **Search results could show a stale `APPROVED` badge.** The data plane is a + search index, not the record store: a record demoted to `DRAFT` keeps being + served as `APPROVED` for many minutes (still drifting 20 minutes after + demotion). Combined with the upsert this is reachable on the ordinary redeploy + path. `GET /api/registry/aws-search` now reconciles every hit's status against + the control plane and reports `status_authoritative: false` — dropping `status` + rather than serving the index's copy — when it cannot. Approval *gating* always + read the control plane and was never affected; a new AST-level guard test keeps + it that way. +- **Descriptor content contracts corrected** (each one an outright rejection by the + live schema validator, reported only as an unactionable descriptor-wide error): + A2A card skills require *all* of `id`/`name`/`description`/`tags` (empty `tags` + is fine, absent is not) and the card requires `url`; `mcpServer.data` is an MCP + server.json whose `name` must be namespaced `/` (a bare name + is rejected) with `description` and `version` required; `agentSkillsDefinition` + must omit `dataSchemaVersion` entirely, unlike every other descriptor; and both + the tools and skills payloads must be objects (`{"tools": [...]}`), never bare + arrays. Under-specified inputs are now normalized rather than forwarded. +- `UpdateRegistryRecord` takes a different shape from `CreateRegistryRecord` — + every branch and scalar leaf is wrapped in an `optionalValue` patch envelope. + Passing the create shape fails in botocore's *client-side* validation, never + reaching AWS, and on the deploy path that lands in the best-effort handler. + +### Fixed +- **Auto-register on deploy never worked**: the `status_update` step Lambda — the + role that actually calls `CreateRegistryRecord` — had no registry permissions at + all, so every federation attempt was an `AccessDenied` swallowed by the + best-effort handler. The exception cause is now logged rather than discarded. +- **An unqueryable registry was indistinguishable from a rejected integration.** + Gating swallowed every error into "nothing is approved", so an `AccessDenied` on + `agent-registry:ListRegistryRecords` — or a registryId typo — rendered as a 403 + telling the operator their integrations had been *rejected*, sending them to fix + a governance record when the fault was an IAM policy. Absent data and negative + data are now distinct: `list_records_strict()` raises `RegistryQueryFailed`, + which surfaces as a 503 naming the registry as unreachable. Gating stays + fail-closed for a *successful* query that finds no approval. +- **`list_records()` returned only the first page**, so fail-closed integration + gating could block a deploy against an integration that *is* `APPROVED` further + down the list. Now follows `nextToken`, and pushes the `APPROVED` narrowing + server-side via the GA `filters` parameter. +- Registry adapter degrades instead of raising when the bundled boto3 predates GA + (`boto3.client()` raising `UnknownServiceError` used to 500 + `GET /api/registry/aws-config`). +- Descriptor `data` payloads are now checked against the service's 102400-**byte** + cap (measured in bytes, not characters) with an error naming which descriptor + overflowed. AWS's `ValidationException` identifies neither, and on the deploy + path it lands in a best-effort handler that would reduce it to a log line. +- `frontend/src/services/api.ts` carried a second, independent declaration of + `getAwsRegistryConfig()`'s return type; only `tsc -b` (project references, as CI + runs it) surfaced the mismatch — `tsc -p` on the root project did not. + ### Added - GitHub Actions CI: ruff lint/format, backend unit tests with coverage floor, CDK assertion tests + `cdk synth` (cdk-nag gate), frontend lint/typecheck/tests/build diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 4071fc6..799bc92 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -6,7 +6,10 @@ requires-python = ">=3.11" dependencies = [ "fastapi>=0.115.0", "pydantic>=2.10.0", - "boto3>=1.35.0", + # 1.43.66 is the first release with the GA `agent-registry-control` / + # `agent-registry` service models (Agent Registry left the + # `bedrock-agentcore` namespace at GA). + "boto3>=1.43.66", "uvicorn>=0.32.0", "python-dotenv>=1.0.0", "mangum>=0.19.0", @@ -21,6 +24,11 @@ dev = [ "httpx>=0.28.0", "moto[dynamodb]>=5.0.0", "requests>=2.31.0", + # Exact pin, unlike everything else here: a formatter's output changes between + # minor releases, so `>=` would mean `ruff format --check` locally and in CI + # could disagree while both were "satisfied". Must match the `version:` pinned + # on the ruff-action steps in .github/workflows/ci.yml. + "ruff==0.16.4", ] deploy = [ "bedrock-agentcore-starter-toolkit", diff --git a/backend/requirements-lambda.txt b/backend/requirements-lambda.txt index c6b1705..2c067b4 100644 --- a/backend/requirements-lambda.txt +++ b/backend/requirements-lambda.txt @@ -1,6 +1,12 @@ fastapi>=0.115.0 pydantic>=2.10.0 -boto3>=1.35.0 +# boto3 >= 1.43.66 is REQUIRED, not merely preferred: it is the first release +# carrying the `agent-registry-control` / `agent-registry` service models that +# Agent Registry moved to at GA. On an older bundle boto3.client(...) raises +# UnknownServiceError and registry federation degrades to "unreachable". +# This must stay ahead of the Lambda runtime's built-in boto3, which PYTHONPATH +# (/var/task/lib) shadows. +boto3>=1.43.66 mangum>=0.19.0 python-dotenv>=1.0.0 pyyaml>=6.0.3 diff --git a/backend/src/app/deployment_handler.py b/backend/src/app/deployment_handler.py index 4faaf1a..0595608 100644 --- a/backend/src/app/deployment_handler.py +++ b/backend/src/app/deployment_handler.py @@ -466,7 +466,10 @@ async def handle_deploy(request: DeployRequest, raw_request: Request) -> DeployR # peers. Collect the connected external identifiers (endpoint URLs / names) # and reject the deploy if any is not APPROVED. No-op when federation is off. try: - from app.services.aws_agent_registry import unapproved_integrations + from app.services.aws_agent_registry import ( + RegistryQueryFailed, + unapproved_integrations, + ) _idents: list[str] = [] _mcp = request.mcp_server_config or {} @@ -478,7 +481,25 @@ async def handle_deploy(request: DeployRequest, raw_request: Request) -> DeployR if isinstance(_a2a, dict): for u in _a2a.get("peer_allowlist") or _a2a.get("peerAllowlist") or []: _idents.append(str(u)) - _blocked = unapproved_integrations(_idents) + try: + _blocked = unapproved_integrations(_idents) + except RegistryQueryFailed as _rqe: + # Still fail CLOSED — a governance control that opens on error is not a + # control. But 503, not 403, and name the real cause: approval status is + # UNKNOWN, not "denied". Reporting this as 403 would send the operator to + # approve records that may already be approved, while the actual fix is an + # IAM action or the registry id. + logger.error("integration gating could not resolve approval status: %s", _rqe) + raise HTTPException( + status_code=503, + detail=( + "Integration gating is enabled but the Agent Registry could not be " + f"queried, so approval status is unknown ({_rqe}). Refusing the deploy " + "rather than let an unreviewed integration through. Check that the " + "deployment role holds agent-registry:ListRegistryRecords and that the " + "configured registry id is correct." + ), + ) from _rqe if _blocked: raise HTTPException( status_code=403, diff --git a/backend/src/app/routers/registry.py b/backend/src/app/routers/registry.py index d60ec88..b5d0eb0 100644 --- a/backend/src/app/routers/registry.py +++ b/backend/src/app/routers/registry.py @@ -331,14 +331,45 @@ class AwsRegistryEnableRequest(BaseModel): @router.get("/aws-config", dependencies=[Depends(require_scopes("registry:read"))]) async def aws_registry_config(caller_sub: str = Depends(get_caller_sub)) -> dict: - """Return whether AWS Agent Registry federation is enabled + reachable.""" - from app.services.aws_agent_registry import get_configured_registry_id, get_registry + """Return whether AWS Agent Registry federation is enabled + reachable. + + ``sdk_supported`` distinguishes the two very different reasons federation can + be unreachable: a bad registryId / missing IAM (fixable in the console) vs a + Lambda bundle whose boto3 predates the GA ``agent-registry`` service models + (fixable only by redeploying with boto3 >= 1.43.66). Without it the UI can + only say "unreachable", which sends admins hunting the wrong problem. + ``status`` covers the third case: a valid, permitted registry that is not yet + READY. It is None when the registry could not be read at all. + """ + from app.services.aws_agent_registry import ( + REGISTRY_STATUS_READY, + agent_registry_supported, + get_configured_registry_id, + get_registry, + ) + sdk_ok = agent_registry_supported() rid = get_configured_registry_id() if not rid: - return {"enabled": False, "registry_id": None, "available": False} + return { + "enabled": False, + "registry_id": None, + "available": False, + "sdk_supported": sdk_ok, + "status": None, + } reg = get_registry() - return {"enabled": True, "registry_id": rid, "available": bool(reg and reg.available())} + # `status` splits the third reason federation can look broken: the registry is + # real and permitted but not READY (CREATING/UPDATING/DELETING). available() + # alone renders that identically to a bad registryId. + status = reg.registry_status() if reg else None + return { + "enabled": True, + "registry_id": rid, + "available": status == REGISTRY_STATUS_READY, + "sdk_supported": sdk_ok, + "status": status, + } @router.post("/aws-config", dependencies=[Depends(require_scopes("registry:write"))]) @@ -350,14 +381,43 @@ async def aws_registry_enable( """Enable AWS Agent Registry federation with a registryId. Admin only.""" if not is_admin: raise HTTPException(status_code=403, detail="Requires registry-admin role") - from app.services.aws_agent_registry import AwsAgentRegistry, set_configured_registry_id + from app.services.aws_agent_registry import ( + MIN_BOTO3, + REGISTRY_STATUS_READY, + AwsAgentRegistry, + agent_registry_supported, + set_configured_registry_id, + ) - # Validate reachability before persisting so a typo fails loudly here. - if not AwsAgentRegistry(body.registry_id).available(): + # An old bundle has no agent-registry client at all, so available() would be + # False for a perfectly valid registryId. Say so, rather than blaming the id. + if not agent_registry_supported(): + raise HTTPException( + status_code=400, + detail=( + "This deployment's AWS SDK predates the GA Agent Registry API. " + f"Redeploy with boto3 >= {'.'.join(str(p) for p in MIN_BOTO3)}." + ), + ) + # Validate reachability before persisting so a typo fails loudly here. Read the + # status rather than just available(), so "still being created" is not reported + # as "wrong registryId" — a registry takes tens of seconds to reach READY, and + # enabling federation right after creating one is the normal sequence. + status = AwsAgentRegistry(body.registry_id).registry_status() + if status is None: raise HTTPException( status_code=400, detail="Registry not reachable (check the registryId / region / permissions)", ) + if status != REGISTRY_STATUS_READY: + raise HTTPException( + status_code=409, + detail=( + f"Registry {body.registry_id} exists but its status is {status}, not " + f"{REGISTRY_STATUS_READY}; it cannot accept records yet. " + "Retry once it finishes provisioning." + ), + ) set_configured_registry_id(body.registry_id) return {"enabled": True, "registry_id": body.registry_id, "available": True} @@ -367,13 +427,45 @@ async def aws_registry_search( q: str = Query(min_length=1, max_length=256), caller_sub: str = Depends(get_caller_sub), ) -> dict: - """Semantic search across the AWS Agent Registry (empty when disabled).""" - from app.services.aws_agent_registry import get_registry + """Semantic search across the AWS Agent Registry (empty when disabled). + + Results come from the data plane's search index, whose per-record ``status`` + lags the control plane — a record demoted APPROVED -> DRAFT (which is what a + redeploy does, since register() upserts) keeps being served as APPROVED. This + is a governance surface, so showing that stale badge would tell a reviewer an + integration is approved when it is waiting on re-review. + + So each hit's status is overwritten from the authoritative control-plane + listing. Best-effort by design: if that listing fails we drop ``status`` + rather than fail the request or pass the index's version through, because + "unknown" is honest and the other two options are respectively useless and + misleading. ``status_authoritative`` tells the UI which case it got. + """ + from app.services.aws_agent_registry import RegistryQueryFailed, get_registry reg = get_registry() if reg is None: return {"enabled": False, "results": []} - return {"enabled": True, "results": reg.search(q)} + + results = reg.search(q) + authoritative = True + try: + truth = {r.get("recordId"): r.get("status") for r in reg.list_records_strict() if r.get("recordId")} + except RegistryQueryFailed as e: + logger.info("could not reconcile search statuses against the control plane: %s", e) + truth, authoritative = {}, False + + for hit in results: + if not isinstance(hit, dict): + continue + if authoritative: + # A hit absent from the control plane was deleted but not yet + # de-indexed; report it as gone rather than as its last known status. + hit["status"] = truth.get(hit.get("recordId"), "DELETED") + else: + hit.pop("status", None) + + return {"enabled": True, "results": results, "status_authoritative": authoritative} @router.get("/{slug}", response_model=RegistryEntryResponse, dependencies=[Depends(require_scopes("registry:read"))]) diff --git a/backend/src/app/services/aws_agent_registry.py b/backend/src/app/services/aws_agent_registry.py index c0239fd..bd98491 100644 --- a/backend/src/app/services/aws_agent_registry.py +++ b/backend/src/app/services/aws_agent_registry.py @@ -1,20 +1,52 @@ -"""AWS Bedrock AgentCore Agent Registry adapter (Phase 6 — Loom-inspired). +"""AWS Agent Registry adapter (GA) — Phase 6 (Loom-inspired). Federates deployed agents into the AWS-native Agent Registry — the org-wide catalog with an approval gate — on top of our internal registry. OPT-IN: does nothing unless an admin configures a registryId in Settings. -Verified against boto3 1.43.8 (bedrock-agentcore-control + bedrock-agentcore): - control: CreateRegistry, CreateRegistryRecord, GetRegistryRecord, - ListRegistryRecords, SubmitRegistryRecordForApproval, - UpdateRegistryRecordStatus, DeleteRegistryRecord - data: SearchRegistryRecords - descriptorType ∈ {MCP, A2A, CUSTOM, AGENT_SKILLS} - status ∈ {DRAFT, PENDING_APPROVAL, APPROVED, REJECTED, DEPRECATED, - CREATING, UPDATING, CREATE_FAILED, UPDATE_FAILED} - -Degrades gracefully: AWS Agent Registry is public preview and may be absent in a -region or on an account. Every call is best-effort; failures are logged and +PREVIEW -> GA MIGRATION +----------------------- +Agent Registry graduated out of AgentCore into its own AWS service. Everything +else in AgentCore (Runtime, Gateway, Identity, Memory) stays on +``bedrock-agentcore``; only the Registry moved: + +=================== ============================== ============================ + preview GA +=================== ============================== ============================ +control-plane boto3 ``bedrock-agentcore-control`` ``agent-registry-control`` +data-plane boto3 ``bedrock-agentcore`` ``agent-registry`` +IAM action prefix ``bedrock-agentcore:`` ``agent-registry:`` +ARN service ``arn:...:bedrock-agentcore:`` ``arn:...:agent-registry:`` +record classifier ``descriptorType=`` ``recordType=`` +classifier values MCP/A2A/CUSTOM/AGENT_SKILLS MCP/AGENT/CUSTOM/SKILL +A2A descriptor ``a2a.agentCard.inlineContent`` ``a2aAgentCard.data`` +custom descriptor ``custom.inlineContent`` ``custom.data`` +schema-version key ``schemaVersion`` ``dataSchemaVersion`` +search operation ``SearchRegistryRecords`` ``SearchDiscoverableRegistryRecords`` +=================== ============================== ============================ + +The rename is a silent trap, not a loud one: the legacy +``bedrock-agentcore-control`` model still carries the Registry operations (with +the OLD ``descriptorType`` parameter), so preview calls keep "working" against a +shim whose IAM prefix and payload shape have both moved on. Only the data-plane +search fails loudly. Hence the hard pin below. + +Requires boto3 >= 1.43.66 (first release carrying the agent-registry models). + +Verified against the boto3 1.43.72 service models: + control: CreateRegistry, GetRegistry, UpdateRegistry, DeleteRegistry, + ListRegistries, CreateRegistryRecord, GetRegistryRecord, + ListRegistryRecords, UpdateRegistryRecord, DeleteRegistryRecord, + SubmitRegistryRecordForApproval, UpdateRegistryRecordStatus, + TagResource, UntagResource, ListTagsForResource + data: SearchDiscoverableRegistryRecords, ListDiscoverableRegistryRecords, + BatchGetDiscoverableRegistryRecord + recordType ∈ {MCP, AGENT, CUSTOM, SKILL} + status ∈ {DRAFT, PENDING_APPROVAL, APPROVED, REJECTED, DEPRECATED, + CREATING, UPDATING, CREATE_FAILED, UPDATE_FAILED} + +Degrades gracefully: the Registry may be absent in a region, on an account, or +in an older boto3 bundle. Every call is best-effort; failures are logged and surfaced as a disabled feature, never a 500 on the deploy path. """ @@ -23,27 +55,226 @@ import json import logging import os +import re import boto3 logger = logging.getLogger(__name__) +# A2A agent-card schema version, passed as `dataSchemaVersion` at GA. A2A_CARD_SCHEMA_VERSION = "0.3" +# GA boto3 service names. Registry is its own service now — using the +# bedrock-agentcore names here silently targets the deprecated preview shim. +CONTROL_SERVICE = "agent-registry-control" +DATA_SERVICE = "agent-registry" + +# First boto3 release shipping the agent-registry service models. +MIN_BOTO3 = (1, 43, 66) + +# GA recordType enum (was descriptorType in preview). +RECORD_TYPES = ("MCP", "AGENT", "CUSTOM", "SKILL") + +# Preview spellings we still accept from persisted rows / older callers, mapped +# onto their GA equivalents. "a2a"/"custom" are the lowercase descriptor keys the +# preview call sites passed as descriptor_type. +_LEGACY_RECORD_TYPES = { + "A2A": "AGENT", + "AGENT_SKILLS": "SKILL", + "AGENTSKILLS": "SKILL", +} + +# Each recordType carries its payload under exactly one descriptor key. Getting +# this pairing wrong is a ValidationException from AWS; we catch it locally with +# an actionable message instead. +DESCRIPTOR_KEY_FOR_TYPE = { + "MCP": "mcpServer", + "AGENT": "a2aAgentCard", + "SKILL": "agentSkillsDefinition", + "CUSTOM": "custom", +} + +# CreateRegistryRecord input constraints (from the service model). +_NAME_MAX = 255 +_DESCRIPTION_MAX = 4096 +_NAME_ALLOWED = re.compile(r"[^a-zA-Z0-9_\-./]") +# Every descriptor `data` member is capped at 102400 bytes by the service model. +_DATA_MAX = 102400 + +# The only registry status that accepts writes. CreateRegistryRecord against a +# registry in any other state fails with ConflictException. +REGISTRY_STATUS_READY = "READY" + + +class RegistryQueryFailed(RuntimeError): + """A registry query did not complete — as opposed to completing and finding + nothing. + + This distinction is load-bearing. `list_records()` returns [] both when the + registry genuinely holds no matching records and when the call blew up + (AccessDenied, throttle, wrong filter shape). Those two facts demand OPPOSITE + responses from a fail-closed policy check: the first is a real verdict, the + second is an absence of information. Conflating them lets an infrastructure + error masquerade as an authorization decision — a deploy gets rejected with + "your integrations are not APPROVED" when the truth is "we could not ask". + + Fail-closed logic must therefore branch on this exception rather than on an + empty list. `partial` carries whatever pages were read before the failure. + """ + + def __init__(self, message: str, partial: list[dict] | None = None): + super().__init__(message) + self.partial: list[dict] = partial or [] + def _region() -> str: return os.environ.get("APP_AWS_REGION", os.environ.get("AWS_REGION", "us-east-1")) def _record_id_from_arn(arn: str) -> str: - """AWS returns recordArn (no recordId); the id is the last ARN segment.""" + """AWS returns recordArn (no recordId); the id is the last ARN segment. + + Safe by construction at GA: recordArn matches + ``arn:aws:agent-registry:::registry/<12-16>/record/<12>`` and + recordId is exactly that trailing 12-char token, so no lookup is needed (and + we avoid racing the eventual-consistent record index right after create). + """ return arn.rsplit("/", 1)[-1] if arn else "" +def boto3_version() -> tuple[int, ...]: + """Parsed boto3 version, or (0,) when it cannot be determined.""" + try: + return tuple(int(p) for p in boto3.__version__.split(".")[:3]) + except Exception: # noqa: BLE001 + return (0,) + + +def agent_registry_supported() -> bool: + """True when this boto3 bundle actually carries the agent-registry models. + + A version check alone is not enough — a Lambda bundle can pin a new boto3 + while an older botocore supplies the service models — so probe the session's + service list too. + """ + if boto3_version() < MIN_BOTO3: + return False + try: + services = boto3.session.Session().get_available_services() + except Exception: # noqa: BLE001 + return False + return CONTROL_SERVICE in services and DATA_SERVICE in services + + +def _make_client(service: str, region: str): + """Build a boto3 client, or None when this bundle has no such service. + + Returning None (instead of letting UnknownServiceError escape) is what keeps + an old-boto3 Lambda reporting "configured but unreachable" rather than 500ing + the registry router. + """ + try: + return boto3.client(service, region_name=region) + except Exception as e: # noqa: BLE001 + logger.info( + "Agent Registry client %s unavailable (boto3 %s, need >=%s): %s", + service, + boto3.__version__, + ".".join(str(p) for p in MIN_BOTO3), + str(e)[:160], + ) + return None + + +def normalize_record_type(value: str | None) -> str: + """Map any accepted spelling onto the GA recordType enum. + + Accepts the GA values, the preview values (``A2A`` -> ``AGENT``, + ``AGENT_SKILLS`` -> ``SKILL``) and the lowercase descriptor keys the preview + call sites used (``"a2a"``, ``"custom"``). Unknown values fall back to + ``CUSTOM`` — a record in the catalog beats a hard failure on the deploy path. + """ + raw = (value or "").strip() + if not raw: + return "CUSTOM" + upper = raw.upper() + upper = _LEGACY_RECORD_TYPES.get(upper, upper) + if upper in RECORD_TYPES: + return upper + # Lowercase descriptor keys ("a2aAgentCard", "mcpServer", ...) -> their type. + for rtype, key in DESCRIPTOR_KEY_FOR_TYPE.items(): + if raw == key: + return rtype + logger.info("Unknown recordType %r — registering as CUSTOM", raw) + return "CUSTOM" + + +def sanitize_record_name(name: str) -> str: + """Coerce a name to the GA record-name pattern. + + The service enforces ``[a-zA-Z0-9][a-zA-Z0-9_\\-./]*`` (1-255). Our record + names come from user-chosen agent names, so a leading underscore or a space + would otherwise surface as a ValidationException on an already-succeeded + deploy. + """ + cleaned = _NAME_ALLOWED.sub("_", (name or "").strip()) + cleaned = cleaned.lstrip("_-./") + if not cleaned: + cleaned = "agent" + if not cleaned[0].isalnum(): + cleaned = f"a{cleaned}" + return cleaned[:_NAME_MAX] + + +def _encode_data(payload: object, *, what: str) -> str: + """JSON-encode a descriptor payload, enforcing the service's size cap. + + Every descriptor ``data`` member is capped at 102400 bytes. Exceeding it is a + ValidationException from AWS with no indication of WHICH descriptor was too + big — and on the deploy path that arrives inside a best-effort handler, so it + would surface only as a truncated log line. Failing here names the culprit. + """ + encoded = json.dumps(payload) + size = len(encoded.encode("utf-8")) + if size > _DATA_MAX: + raise ValueError(f"{what} descriptor data is {size} bytes; the Agent Registry limit is {_DATA_MAX}") + return encoded + + +def _normalize_a2a_skill(skill: dict, index: int) -> dict: + """Fill in the A2A-0.3 skill fields the registry requires, keeping the rest. + + Verified against the live GA service: a skill entry is rejected unless it + carries ALL of ``id``, ``name``, ``description`` and ``tags`` — an empty + ``tags`` list is fine, but an absent one is not. AWS reports this as a + card-wide "content is not in compliance with schema version '0.3'" naming + neither the skill nor the field, so an under-specified skill from a workflow + config would sink the whole record with an unactionable error. + + Repairs rather than raises: this runs on the best-effort auto-register-on-deploy + path, where losing an entire governance record because one skill lacked a + description is the worse failure. Unknown keys are preserved — the schema is + open, and callers may legitimately pass A2A extras like ``examples``. + """ + entry = dict(skill or {}) + name = str(entry.get("name") or entry.get("id") or f"skill-{index + 1}") + entry["name"] = name + entry["id"] = str(entry.get("id") or name) + entry["description"] = str(entry.get("description") or name) + tags = entry.get("tags") + entry["tags"] = [str(t) for t in tags] if isinstance(tags, list) else [] + return entry + + def build_a2a_descriptor(name: str, description: str, url: str, skills: list | None = None) -> dict: - """A2A agentCard descriptor (schemaVersion 0.3) as an inlineContent JSON. + """A2A agentCard descriptor for a GA ``AGENT`` record. Reuses the shape our runtime already serves at /.well-known/agent-card.json. + At GA the card goes under ``a2aAgentCard`` as a ``{data, dataSchemaVersion}`` + pair — preview nested it as ``a2a.agentCard.inlineContent``. + + ``url`` must be present but the registry does not require it to be a URL, so + the deploy path's runtime-ARN fallback is accepted as-is. """ card = { "protocolVersion": A2A_CARD_SCHEMA_VERSION, @@ -52,71 +283,364 @@ def build_a2a_descriptor(name: str, description: str, url: str, skills: list | N "version": "1.0", "url": url, "capabilities": {"streaming": True}, - "skills": skills or [], + "skills": [_normalize_a2a_skill(s, i) for i, s in enumerate(skills or [])], "defaultInputModes": ["text"], "defaultOutputModes": ["text"], } - # The API's `descriptors` map is keyed by the (lowercased) descriptor type: - # {"a2a": {"agentCard": {...}}} — NOT a bare agentCard. (Caught live: passing - # the inner structure fails with "Unknown parameter in descriptors: - # agentCard, must be one of: mcp, a2a, custom, agentSkills".) - return {"a2a": {"agentCard": {"schemaVersion": A2A_CARD_SCHEMA_VERSION, "inlineContent": json.dumps(card)}}} + return { + "a2aAgentCard": { + "data": _encode_data(card, what="a2aAgentCard"), + "dataSchemaVersion": A2A_CARD_SCHEMA_VERSION, + } + } + + +def build_agent_skills_descriptor(skills: list | None = None) -> dict: + """agentSkillsDefinition descriptor for a GA ``SKILL`` record. + + Two live-verified quirks, both silent otherwise: + + * ``dataSchemaVersion`` must be OMITTED. Unlike every other descriptor this + one accepts no version at all — each of the A2A/MCP/date-stamped candidates + comes back "Schema version 'X' is not supported for descriptor type + 'agent_skills'". + * ``data`` must be the object ``{"skills": [...]}``, never a bare array. + + Skill entries take the same required-field set as A2A card skills. + """ + payload = {"skills": [_normalize_a2a_skill(s, i) for i, s in enumerate(skills or [])]} + return {"agentSkillsDefinition": {"data": _encode_data(payload, what="agentSkillsDefinition")}} def build_custom_descriptor(payload: dict) -> dict: - """CUSTOM descriptor — arbitrary inlineContent JSON (our own agent metadata). + """CUSTOM descriptor — arbitrary JSON under ``custom.data``. - Returned wrapped under the ``custom`` type key (see build_a2a_descriptor). + Note ``custom`` is the one descriptor with no ``dataSchemaVersion`` member. """ - return {"custom": {"inlineContent": json.dumps(payload)}} + return {"custom": {"data": _encode_data(payload, what="custom")}} + + +# Default descriptor schema versions the service assumes; pinning them makes a +# record self-describing and immune to a future default change. +MCP_SERVER_SCHEMA_VERSION = "2025-12-11" +MCP_TOOLS_SCHEMA_VERSION = "2025-11-25" + +# `mcpServer.data` is an MCP-registry server.json document, whose `name` is +# namespaced: exactly one "/", a reverse-DNS-style namespace on the left and a +# server name on the right. A bare "my-server" is REJECTED (live-verified), as +# are two slashes, an empty half, and "_" inside the namespace. +_MCP_NAME_RE = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9.\-]*/[a-zA-Z0-9][a-zA-Z0-9._\-]*$") +_MCP_NAMESPACE_RE = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9.\-]*$") +_MCP_SERVER_DISALLOWED = re.compile(r"[^a-zA-Z0-9._\-]") + +# Namespace applied when a caller supplies an unnamespaced server name. Reverse +# DNS per the MCP registry convention, so platform-published servers are +# attributable rather than colliding in a flat namespace. +MCP_DEFAULT_NAMESPACE = "com.amazonaws.agentcore" + + +def sanitize_mcp_server_name(name: str, namespace: str = MCP_DEFAULT_NAMESPACE) -> str: + """Coerce an arbitrary server name into a valid ``/`` pair. + + Left unchanged when already valid. Otherwise the caller's value becomes the + server half — with "/" and other illegal characters folded to "-" — under + ``namespace``. This is not cosmetic: an unnamespaced name is the difference + between a registered MCP server and a ValidationException that blames the + whole descriptor. + """ + raw = (name or "").strip() + if _MCP_NAME_RE.match(raw): + return raw + ns, sep, server = raw.partition("/") + if not sep or not _MCP_NAMESPACE_RE.match(ns): + ns, server = namespace, raw + server = _MCP_SERVER_DISALLOWED.sub("-", server).strip("-._") or "server" + return f"{ns}/{server}" + + +def _normalize_mcp_tool(tool: dict, index: int) -> dict: + """Ensure an ``additionalData.tools`` entry carries its required members. + + A tool needs ``name`` and ``inputSchema``; ``description`` is optional + (live-verified). Raises on a nameless tool rather than inventing one: unlike + the A2A card, this builder has no best-effort caller, and a synthesized name + would publish a tool nothing can invoke. + """ + entry = dict(tool or {}) + if not entry.get("name"): + raise ValueError(f"mcpServer tool #{index + 1} has no 'name'; the MCP tools schema requires one") + if not isinstance(entry.get("inputSchema"), dict): + entry["inputSchema"] = {"type": "object", "properties": {}} + return entry + + +def build_mcp_descriptor(server: dict, tools: list | None = None) -> dict: + """MCP descriptor for a GA ``MCP`` record. + + ``mcpServer.data`` is an MCP-registry ``server.json`` document requiring + ``name`` (namespaced — see :func:`sanitize_mcp_server_name`), ``description`` + and ``version``; the ``tools/list`` output rides along under + ``additionalData.tools`` as ``{"tools": [...]}``. Each leaf is a + ``{data, dataSchemaVersion}`` pair. + + Missing required members are filled in rather than forwarded: the service + rejects the document as a whole and names no field, so a caller passing only + ``{"name": ..., "version": ...}`` — which is what this builder used to emit — + got an opaque failure. Every other key the caller sent is preserved; the + schema is open, so ``remotes``, ``packages`` and ``repository`` pass through. + Note ``remotes[].type`` is a closed enum ("streamable-http" / "sse"), which we + deliberately do not police here — forwarding an unknown transport surfaces as + an AWS error rather than being silently rewritten to the wrong one. + """ + doc = dict(server or {}) + doc["name"] = sanitize_mcp_server_name(str(doc.get("name") or "")) + doc["description"] = str(doc.get("description") or doc["name"]) + doc["version"] = str(doc.get("version") or "1.0.0") + + descriptor: dict = { + "mcpServer": { + "data": _encode_data(doc, what="mcpServer"), + "dataSchemaVersion": MCP_SERVER_SCHEMA_VERSION, + } + } + if tools: + normalized = [_normalize_mcp_tool(t, i) for i, t in enumerate(tools)] + descriptor["mcpServer"]["additionalData"] = { + "tools": { + "data": _encode_data({"tools": normalized}, what="mcpServer.additionalData.tools"), + "dataSchemaVersion": MCP_TOOLS_SCHEMA_VERSION, + } + } + return descriptor + + +def _opt(value): + """Wrap a value in UpdateRegistryRecord's ``optionalValue`` envelope.""" + return {"optionalValue": value} + + +def _to_update_descriptors(descriptors: dict) -> dict: + """Re-shape Create-style descriptors into UpdateRegistryRecord's patch form. + + UpdateRegistryRecord does NOT accept the same descriptor structure as + CreateRegistryRecord. Every branch and every scalar leaf is wrapped in an + ``optionalValue`` envelope so the service can tell "set this to X" apart from + "leave it alone":: + + create: {"custom": {"data": "..."}} + update: {"optionalValue": {"custom": {"optionalValue": { + "data": {"optionalValue": "..."}}}}} + + Handing update the Create shape fails inside botocore's own parameter + validation — a client-side error that never even reaches AWS, and on the deploy + path it lands in a best-effort handler that would reduce it to a log line. + """ + out: dict = {} + for key, leaf in (descriptors or {}).items(): + body: dict = {} + for field in ("data", "dataSchemaVersion"): + if field in leaf: + body[field] = _opt(leaf[field]) + additional = leaf.get("additionalData") + if additional: + body["additionalData"] = _opt( + { + sub: _opt({f: _opt(v) for f, v in payload.items() if f in ("data", "dataSchemaVersion")}) + for sub, payload in additional.items() + } + ) + out[key] = _opt(body) + return _opt(out) + + +def _is_conflict(exc: Exception) -> bool: + """True for a ConflictException from the control plane. + + Note this covers two different situations — "a record with this name+version + already exists" and "the registry is not in READY state" — so callers must not + treat it as proof that a record exists. :meth:`AwsAgentRegistry.register` + confirms by lookup and re-raises when it finds nothing, which keeps the + not-READY case an error instead of silently doing nothing. + """ + response = getattr(exc, "response", None) + if isinstance(response, dict) and response.get("Error", {}).get("Code") == "ConflictException": + return True + return type(exc).__name__ == "ConflictException" class AwsAgentRegistry: - """Thin adapter over the AWS Agent Registry control + data planes.""" + """Thin adapter over the Agent Registry control + data planes (GA).""" def __init__(self, registry_id: str, region: str | None = None) -> None: self.registry_id = registry_id - region = region or _region() - self.control = boto3.client("bedrock-agentcore-control", region_name=region) - self.data = boto3.client("bedrock-agentcore", region_name=region) + self.region = region or _region() + self.control = _make_client(CONTROL_SERVICE, self.region) + self.data = _make_client(DATA_SERVICE, self.region) # -- health ---------------------------------------------------------- - def available(self) -> bool: - """True if the configured registry exists + is reachable (feature gate).""" + def registry_status(self) -> str | None: + """The registry's lifecycle status, or None if it could not be read. + + None means "we could not ask" — bad registryId, missing IAM, or a boto3 + bundle with no agent-registry model. A string means the registry answered. + Keeping those apart is the same distinction :class:`RegistryQueryFailed` + draws for records: a caller that collapses both into "unavailable" tells + an admin to go fix their registryId when the registry is merely CREATING. + """ + if self.control is None: + return None try: - self.control.get_registry(registryId=self.registry_id) - return True + return self.control.get_registry(registryId=self.registry_id).get("status") or "" except Exception as e: # noqa: BLE001 - logger.info("AWS Agent Registry unavailable: %s", str(e)[:120]) - return False + logger.info("AWS Agent Registry unreachable: %s", str(e)[:120]) + return None + + def available(self) -> bool: + """True only when the registry exists AND is READY to accept writes. + + get_registry() succeeding is NOT sufficient. A registry that exists but + sits in CREATING/UPDATING/DELETING rejects CreateRegistryRecord with + ``ConflictException: Registry is not in READY state``. The previous check + returned True throughout the multi-second CREATING window, so enabling + federation on a freshly created registry — the overwhelmingly common + sequence — passed validation and then raced into that conflict on the + first deploy, where auto-register is best-effort and swallows it. + """ + return self.registry_status() == REGISTRY_STATUS_READY # -- records --------------------------------------------------------- def register( - self, name: str, descriptor_type: str, descriptors: dict, description: str = "", record_version: str = "1" + self, + name: str, + record_type: str, + descriptors: dict, + description: str = "", + record_version: str = "1.0", + display_name: str | None = None, ) -> dict: - """Create a record (DRAFT/CREATING) and return {record_id, arn, status}.""" - resp = self.control.create_registry_record( - registryId=self.registry_id, - name=name, - description=description or name, - descriptorType=descriptor_type, - descriptors=descriptors, - recordVersion=record_version, - ) + """Upsert a record and return {record_id, arn, status, record_type, name}. + + ``record_type`` is the GA recordType (MCP/AGENT/CUSTOM/SKILL); preview + spellings are normalized. Returns an empty dict when the Registry client + is unavailable, so callers stay best-effort. + + Upsert, not create: ``name`` + ``recordVersion`` is a uniqueness key, and + ``recordVersion`` is "1.0" for everything this platform registers. So the + SECOND deployment of an agent under the same name — an ordinary redeploy — + raised ConflictException, which the deploy path swallows as best-effort. The + visible symptom was a registry record frozen at the first deployment's + runtime ARN and endpoint, silently stale forever, with no error surfaced + anywhere. Falling back to UpdateRegistryRecord keeps exactly one record per + agent, always describing the live runtime. + + Governance note: updating a record's content demotes it from APPROVED back + to DRAFT (service behaviour, verified live). That is the desired outcome — + a redeploy that changes what the agent exposes must be re-reviewed, and it + means an upsert cannot be used to slip new content past an old approval. + The corollary is that ``unapproved_integrations()`` will block a redeployed + integration until it is approved again, which is the fail-closed reading. + """ + if self.control is None: + logger.info("register skipped — no %s client in this boto3 bundle", CONTROL_SERVICE) + return {} + + rtype = normalize_record_type(record_type) + expected_key = DESCRIPTOR_KEY_FOR_TYPE[rtype] + if descriptors and expected_key not in descriptors: + raise ValueError(f"recordType {rtype} requires the {expected_key!r} descriptor, got {sorted(descriptors)}") + + record_name = sanitize_record_name(name) + # description has min length 1 in the service model — never send "". + desc = (description or record_name)[:_DESCRIPTION_MAX] + shown = (display_name or name or record_name)[:_NAME_MAX] + + try: + resp = self.control.create_registry_record( + registryId=self.registry_id, + name=record_name, + displayName=shown, + description=desc, + recordType=rtype, + descriptors=descriptors, + recordVersion=record_version, + ) + except Exception as e: # noqa: BLE001 — narrowed immediately below + # ConflictException also means "registry not READY", so confirm a record + # really exists before treating this as a redeploy; otherwise re-raise + # and let the real error surface. + existing = self._find_record(record_name, record_version) if _is_conflict(e) else None + if existing is None: + raise + logger.info("registry record %s v%s exists — updating in place (redeploy)", record_name, record_version) + return self._refresh_record(existing, descriptors, desc, shown, rtype, record_name) + arn = resp.get("recordArn", "") return { - "record_id": resp.get("recordId") or _record_id_from_arn(arn), + "record_id": _record_id_from_arn(arn), "arn": arn, "status": resp.get("status", ""), + "record_type": rtype, + "name": record_name, + } + + def _find_record(self, name: str, record_version: str) -> dict | None: + """The record with this exact name + recordVersion, or None. + + That pair is the uniqueness key CreateRegistryRecord enforces, so it is what + a ConflictException points at. List items carry ``recordId`` directly, so no + ARN parsing is needed on this path. + """ + try: + # `filters[].values` is capped at one entry, so this is a single name. + for rec in self.list_records_strict(filters=[{"name": "name", "values": [name]}]): + if rec.get("recordVersion") == record_version: + return rec + except RegistryQueryFailed as e: + logger.info("could not look up an existing record named %s: %s", name, e) + return None + + def _refresh_record( + self, + existing: dict, + descriptors: dict, + description: str, + display_name: str, + rtype: str, + record_name: str, + ) -> dict: + """Point an existing record at the current deployment via UpdateRegistryRecord. + + ``name``/``recordType``/``recordVersion`` are deliberately not sent: they are + the record's identity, and this call is only meant to refresh its content. + """ + record_id = existing.get("recordId") or _record_id_from_arn(existing.get("recordArn", "")) + resp = self.control.update_registry_record( + registryId=self.registry_id, + recordId=record_id, + descriptors=_to_update_descriptors(descriptors), + description=_opt(description), + displayName=_opt(display_name), + ) + return { + "record_id": resp.get("recordId") or record_id, + "arn": resp.get("recordArn") or existing.get("recordArn", ""), + "status": resp.get("status", ""), + "record_type": resp.get("recordType") or rtype, + "name": resp.get("name") or record_name, + "updated": True, } def submit_for_approval(self, record_id: str) -> None: + if self.control is None: + return self.control.submit_registry_record_for_approval(registryId=self.registry_id, recordId=record_id) def set_status(self, record_id: str, status: str, reason: str) -> None: """APPROVED / REJECTED / DEPRECATED — statusReason is required by the API.""" + if self.control is None: + return self.control.update_registry_record_status( registryId=self.registry_id, recordId=record_id, @@ -125,21 +649,63 @@ def set_status(self, record_id: str, status: str, reason: str) -> None: ) def get(self, record_id: str) -> dict | None: + if self.control is None: + return None try: return self.control.get_registry_record(registryId=self.registry_id, recordId=record_id) except Exception as e: # noqa: BLE001 logger.info("get_registry_record failed: %s", str(e)[:120]) return None - def list_records(self) -> list[dict]: + def list_records_strict(self, filters: list[dict] | None = None) -> list[dict]: + """Every record in the registry, following nextToken; RAISES on failure. + + Use this — never the lenient `list_records()` — whenever an empty result + would drive a policy decision. See RegistryQueryFailed for why an + exception, not an empty list, is the only safe signal there. + + Pagination matters for correctness, not just completeness: the gating in + unapproved_integrations() is fail-closed, so a truncated first page would + block deploys against integrations that ARE approved further down the + list. ``filters`` takes the GA control-plane shape + ``[{"name": "name"|"status"|"recordType", "values": [...]}]``. + """ + if self.control is None: + raise RegistryQueryFailed( + f"no {CONTROL_SERVICE} client available (boto3 {boto3.__version__}, " + f"need >= {'.'.join(str(p) for p in MIN_BOTO3)})" + ) + records: list[dict] = [] + token: str | None = None + while True: + try: + resp = self.control.list_registry_records( + registryId=self.registry_id, + **({"nextToken": token} if token else {}), + **({"filters": filters} if filters else {}), + ) + except Exception as e: # noqa: BLE001 + raise RegistryQueryFailed(str(e)[:300], partial=records) from e + records.extend(resp.get("registryRecords") or []) + token = resp.get("nextToken") + if not token: + return records + + def list_records(self, filters: list[dict] | None = None) -> list[dict]: + """Lenient listing for display/inventory: [] (or a partial page) on error. + + Safe for read-only surfaces where a short list is a cosmetic problem. + NOT safe for fail-closed policy checks — use `list_records_strict()`. + """ try: - resp = self.control.list_registry_records(registryId=self.registry_id) - return resp.get("registryRecords") or resp.get("items") or [] - except Exception as e: # noqa: BLE001 - logger.info("list_registry_records failed: %s", str(e)[:120]) - return [] + return self.list_records_strict(filters=filters) + except RegistryQueryFailed as e: + logger.info("list_registry_records failed: %s", e) + return e.partial def delete(self, record_id: str) -> bool: + if self.control is None: + return False try: self.control.delete_registry_record(registryId=self.registry_id, recordId=record_id) return True @@ -147,16 +713,49 @@ def delete(self, record_id: str) -> bool: logger.warning("delete_registry_record failed: %s", str(e)[:120]) return False - def search(self, query: str, max_results: int = 20) -> list[dict]: + def search(self, query: str, max_results: int = 20, record_types: list[str] | None = None) -> list[dict]: + """Semantic search over DISCOVERABLE records. + + GA renamed this operation to SearchDiscoverableRegistryRecords and moved + it onto the ``agent-registry`` data plane. ``registryIds`` takes exactly + one entry (ARN or bare id). ``filters`` is a structured metadata filter + supporting $eq/$ne/$in — not the control plane's list-of-filters shape. + + NEVER USE THIS FOR AN APPROVAL DECISION. The data plane is a search index, + not the record store, and it lags the control plane: a record demoted from + APPROVED back to DRAFT keeps being served here — still ``"status": + "APPROVED"`` — long after the control plane reports DRAFT (live-verified, + stable for minutes). Two planes, one field name, two trust levels. + + That divergence is reachable on the ordinary path, not an exotic one: + :meth:`register` upserts on redeploy, and updating a record's content + demotes it to DRAFT. So every redeploy of an approved integration opens + the window. Gating therefore reads :meth:`list_records_strict` (control + plane) and re-checks each record's status itself; routing it through this + method for speed would let a stale index re-approve a record whose content + has since changed — a governance bypass that no test of the happy path + would notice. ``test_gating_never_reads_the_data_plane`` guards it. + + Returns [] on failure, deliberately: this is a browse/discovery feature + where "no matches" and "search unavailable" are both empty result sets to + the user. Do not copy this pattern into a policy path — that is exactly + what :class:`RegistryQueryFailed` exists to prevent. + """ + if self.data is None: + return [] + kwargs: dict = { + "registryIds": [self.registry_id], + "searchQuery": query, + "maxResults": max_results, + } + if record_types: + normalized = [normalize_record_type(t) for t in record_types] + kwargs["filters"] = {"recordType": {"$in": normalized}} try: - resp = self.data.search_registry_records( - registryIds=[self.registry_id], - searchQuery=query, - maxResults=max_results, - ) - return resp.get("registryRecords") or resp.get("items") or resp.get("results") or [] + resp = self.data.search_discoverable_registry_records(**kwargs) + return resp.get("registryRecords") or [] except Exception as e: # noqa: BLE001 - logger.info("search_registry_records failed: %s", str(e)[:120]) + logger.info("search_discoverable_registry_records failed: %s", str(e)[:120]) return [] @@ -194,7 +793,11 @@ def set_configured_registry_id(registry_id: str) -> None: def get_registry() -> AwsAgentRegistry | None: - """Return a configured adapter, or None when the feature is disabled.""" + """Return a configured adapter, or None when the feature is disabled. + + Never raises: an adapter built on a boto3 bundle without the agent-registry + models carries None clients and reports available() == False. + """ rid = get_configured_registry_id() if not rid: return None @@ -212,6 +815,20 @@ def unapproved_integrations(identifiers: list[str]) -> list[str]: approved when an APPROVED record names it or points at it. An identifier with NO matching record at all is treated as UNAPPROVED (fail-closed: an unreviewed integration must not ship into a governed deployment). + + The APPROVED filter is applied server-side (GA control-plane ``filters``) and + the listing is paginated, so a large catalog cannot silently truncate into a + false "unapproved" verdict. + + Raises: + RegistryQueryFailed: the registry could not be queried, so approval status + is UNKNOWN. Deliberately propagated rather than degraded to "nothing + is approved": an AccessDenied on + ``agent-registry:ListRegistryRecords`` would otherwise render as a 403 + telling the operator their integrations were rejected, sending them to + fix a governance record when the real fault is an IAM policy. The + caller decides what an unknown verdict means — but it must not be + silently treated as a denial. """ if not identifiers: return [] @@ -219,17 +836,20 @@ def unapproved_integrations(identifiers: list[str]) -> list[str]: if reg is None: return [] # federation off → no gating - records = reg.list_records() + records = reg.list_records_strict(filters=[{"name": "status", "values": ["APPROVED"]}]) approved_names: set[str] = set() approved_blobs: list[str] = [] for r in records: + # Defence in depth: the filter already narrows to APPROVED, but a stubbed + # or older control plane that ignores `filters` must not widen the gate. if (r.get("status") or "").upper() != "APPROVED": continue - nm = r.get("name") or r.get("recordName") - if nm: - approved_names.add(str(nm)) + for key in ("name", "recordName", "displayName"): + nm = r.get(key) + if nm: + approved_names.add(str(nm)) # keep a coarse text blob per record for URL substring matching - approved_blobs.append(json.dumps(r)) + approved_blobs.append(json.dumps(r, default=str)) unapproved: list[str] = [] for ident in identifiers: diff --git a/backend/src/app/step_handlers/status_update_step.py b/backend/src/app/step_handlers/status_update_step.py index a837982..0ec48e2 100644 --- a/backend/src/app/step_handlers/status_update_step.py +++ b/backend/src/app/step_handlers/status_update_step.py @@ -273,6 +273,10 @@ def _auto_register_in_aws_registry( runtimes, else a CUSTOM descriptor with the agent's identity/endpoint. The record starts in DRAFT — a curator approves it via the registry router (visibility/integration gating enforced elsewhere). Loom-study 0.4. + + Registry GA renamed the record classifier from ``descriptorType`` to + ``recordType`` and dropped the ``A2A`` value in favour of ``AGENT``, so an + A2A runtime is now an AGENT record carrying an ``a2aAgentCard`` descriptor. """ from app.services.aws_agent_registry import ( build_a2a_descriptor, @@ -287,14 +291,14 @@ def _auto_register_in_aws_registry( return # idempotent — already registered if is_a2a: - descriptor_type = "a2a" + record_type = "AGENT" descriptors = build_a2a_descriptor( name=friendly_runtime_name, description=f"Agent {friendly_runtime_name} deployed via the platform", url=runtime_endpoint or runtime_arn or "", ) else: - descriptor_type = "custom" + record_type = "CUSTOM" descriptors = build_custom_descriptor( { "name": friendly_runtime_name, @@ -306,7 +310,7 @@ def _auto_register_in_aws_registry( result = registry.register( name=friendly_runtime_name, - descriptor_type=descriptor_type, + record_type=record_type, descriptors=descriptors, description=f"Auto-registered on deploy: {friendly_runtime_name}", ) @@ -535,8 +539,13 @@ def handler(event: dict, context) -> dict: friendly_runtime_name=friendly_runtime_name or runtime_id or deployment_id, is_a2a=str(_protocol).upper() == "A2A", ) - except Exception: # noqa: BLE001 - logger.warning("AWS Agent Registry auto-register skipped (best-effort)") + except Exception as _reg_exc: # noqa: BLE001 + # Include the reason: the commonest cause is an old boto3 bundle with + # no agent-registry service model, which is invisible otherwise. + logger.warning( + "AWS Agent Registry auto-register skipped (best-effort): %s", + str(_reg_exc)[:200], + ) return { "deployment_id": deployment_id, diff --git a/backend/tests/test_agent_registry_ga.py b/backend/tests/test_agent_registry_ga.py new file mode 100644 index 0000000..0d77d3c --- /dev/null +++ b/backend/tests/test_agent_registry_ga.py @@ -0,0 +1,295 @@ +"""Agent Registry preview -> GA migration guards (no AWS, no CDK synth). + +Agent Registry graduated out of AgentCore into its own AWS service. The danger +is that the migration regresses SILENTLY: + + * The deprecated ``bedrock-agentcore-control`` model STILL exposes + CreateRegistryRecord (with the old ``descriptorType`` parameter), so a + reverted call site does not raise UnknownOperation — it just talks to the + preview shim. + * The IAM prefix moved to ``agent-registry:``. Because the deploy-path + auto-register is deliberately best-effort, a stale ``bedrock-agentcore:`` + grant surfaces only as a "skipped" log line, never a failed deploy. + +So these are text/source assertions over the backend and the CDK stacks, in the +same spirit as test_iam_completeness.py. +""" + +from __future__ import annotations + +import ast +import re +from pathlib import Path + +_BACKEND = Path(__file__).resolve().parents[1] +_REPO_ROOT = _BACKEND.parent +_STACKS_DIR = _REPO_ROOT / "infra" / "stacks" + +# Match only quoted IAM action literals, so prose/comments that mention the old +# prefix (e.g. "was bedrock-agentcore:*Registry*") don't trip the assertions. +_LEGACY_REGISTRY_ACTION = re.compile(r'"bedrock-agentcore:[A-Za-z]*Registr[A-Za-z]*"') + + +def _stack_files() -> list[Path]: + files = sorted(_STACKS_DIR.glob("*.py")) + sorted((_STACKS_DIR / "platform").glob("*.py")) + assert files, f"no stack sources found under {_STACKS_DIR}" + return files + + +def _stack_source() -> str: + return "\n".join(p.read_text() for p in _stack_files()) + + +def _backend_sources() -> list[Path]: + return sorted((_BACKEND / "src").rglob("*.py")) + + +# -- IAM action prefix ------------------------------------------------------- + + +def test_no_legacy_bedrock_agentcore_registry_actions(): + """Every Registry IAM action must use the GA `agent-registry:` prefix.""" + offenders = [] + for path in _stack_files(): + for i, line in enumerate(path.read_text().splitlines(), 1): + if _LEGACY_REGISTRY_ACTION.search(line): + offenders.append(f"{path.relative_to(_REPO_ROOT)}:{i}: {line.strip()}") + assert not offenders, "Registry IAM actions still on the bedrock-agentcore prefix:\n" + "\n".join(offenders) + + +def test_status_update_step_can_create_registry_records(): + """The auto-register runs in the status_update step Lambda, so ITS role — not + the deployment Lambda's — is the principal on CreateRegistryRecord.""" + source = (_STACKS_DIR / "platform" / "step_lambdas.py").read_text() + assert '"agent-registry:CreateRegistryRecord"' in source + assert '"agent-registry:GetRegistry"' in source + # teardown deletes the record it created + assert '"agent-registry:DeleteRegistryRecord"' in source + + +def test_status_update_step_can_update_records_for_a_redeploy(): + """register() is an upsert: a redeploy collides on name+recordVersion and falls + back to UpdateRegistryRecord, which needs a lookup first. Missing either action + AccessDenies inside the best-effort wrapper, leaving the record pinned to the + FIRST deployment's runtime ARN with nothing surfaced.""" + source = (_STACKS_DIR / "platform" / "step_lambdas.py").read_text() + assert '"agent-registry:UpdateRegistryRecord"' in source + assert '"agent-registry:ListRegistryRecords"' in source + + +def test_status_update_step_is_not_granted_operations_its_path_never_calls(): + """Least privilege for the auto-register role, pinned deliberately. + + The step role's synthesized policy allows exactly six registry operations, and + an ``iam:SimulateCustomPolicy`` run against the real template confirmed it + DENIES SearchDiscoverableRegistryRecords, SubmitRegistryRecordForApproval and + UpdateRegistryRecordStatus. Those three denials are correct, not a gap: the + auto-register path only reads the registry status, creates a record, and (on + redeploy) looks it up and updates it. Approval transitions and discovery are + API-Lambda concerns driven by a human. + + This test exists because the denials LOOK like the bug this migration started + with — a missing grant swallowed by the best-effort handler — so the next + person to run a simulation would be tempted to paper over them by widening the + role. Widening it would hand the deploy pipeline the ability to approve its own + records, which is the one thing the governance model must not allow. + """ + source = (_STACKS_DIR / "platform" / "step_lambdas.py").read_text() + role = source[source.index("StepStatusUpdateRole") :] if "StepStatusUpdateRole" in source else source + for action in ( + "agent-registry:SubmitRegistryRecordForApproval", + "agent-registry:UpdateRegistryRecordStatus", + "agent-registry:SearchDiscoverableRegistryRecords", + ): + assert f'"{action}"' not in role, ( + f"{action} was granted to the auto-register step role. Its code path never " + "calls it, and approval transitions must not be available to the deploy " + "pipeline — a record that can approve itself is not governed." + ) + + +def test_every_registry_call_the_adapter_makes_is_granted_somewhere(): + """Derive the IAM requirement from the code instead of restating it. + + Each ``self.control.x()`` / ``self.data.x()`` in the adapter is a real API call, + and boto3 method names map 1:1 onto operation names, which map 1:1 onto + `agent-registry:` action names. So the grants can be checked against the call + sites mechanically, and adding a call with no grant anywhere fails here. + + Scope limit, stated because it is easy to over-read: this unions the grants + across all stacks, so it does NOT prove the role that makes a given call holds + the action. Per-role coverage needs a per-role assertion — see + test_status_update_step_can_update_records_for_a_redeploy, which is what + actually caught the missing UpdateRegistryRecord on the deploy path. + """ + adapter = (_BACKEND / "src" / "app" / "services" / "aws_agent_registry.py").read_text() + called = { + "".join(part.title() for part in m.split("_")) + for m in re.findall(r"self\.(?:control|data)\.([a-z_]+)\(", adapter) + } + assert called, "found no client calls — did the adapter move?" + granted = set(re.findall(r'"agent-registry:([A-Za-z]+)"', _stack_source())) + missing = sorted(called - granted) + assert not missing, f"adapter calls these operations with no matching IAM grant: {missing}" + + +def test_gating_never_reads_the_data_plane(): + """Approval decisions must come from the control plane, never the search index. + + Live-verified GA behaviour: the data plane serves a record demoted from + APPROVED back to DRAFT as *still* APPROVED, for minutes. Both planes spell the + field ``status``, so the unsafe call looks exactly like the safe one at the + call site. + + This is reachable on the ordinary path, not an exotic one — register() upserts + on redeploy and updating content demotes the record — so routing gating + through search() for speed would let a stale index clear an integration whose + content has since changed. A happy-path test would not notice. + + Walks the AST of unapproved_integrations() and asserts it touches neither the + data-plane client nor search(). + """ + adapter = (_BACKEND / "src" / "app" / "services" / "aws_agent_registry.py").read_text() + fn = next( + n + for n in ast.walk(ast.parse(adapter)) + if isinstance(n, ast.FunctionDef) and n.name == "unapproved_integrations" + ) + forbidden = [] + for node in ast.walk(fn): + if not isinstance(node, ast.Attribute): + continue + # `self.data.x()` / `reg.data.x()` — the data-plane client + if node.attr == "data" or (isinstance(node.value, ast.Attribute) and node.value.attr == "data"): + forbidden.append(f"line {node.lineno}: reads `.data` (data plane)") + if node.attr in ("search", "search_discoverable_registry_records"): + forbidden.append(f"line {node.lineno}: calls .{node.attr}()") + assert not forbidden, ( + "unapproved_integrations() must decide approval from the control plane " + "(list_records_strict), not the stale discovery index:\n" + "\n".join(forbidden) + ) + + +def test_deployment_lambda_has_ga_registry_actions(): + source = (_STACKS_DIR / "platform" / "lambdas.py").read_text() + for action in ( + "agent-registry:CreateRegistryRecord", + "agent-registry:ListRegistryRecords", + "agent-registry:SubmitRegistryRecordForApproval", + "agent-registry:UpdateRegistryRecordStatus", + "agent-registry:DeleteRegistryRecord", + ): + assert f'"{action}"' in source, f"missing {action}" + + +def test_search_action_uses_ga_operation_name(): + """GA renamed SearchRegistryRecords -> SearchDiscoverableRegistryRecords. + + Checked against quoted action literals only — comments explaining the rename + legitimately mention the old name. + """ + source = _stack_source() + assert '"agent-registry:SearchDiscoverableRegistryRecords"' in source + actions = set(re.findall(r'"[a-z0-9-]+:[A-Za-z]+"', source)) + assert not {a for a in actions if a.endswith(':SearchRegistryRecords"')} + + +# -- boto3 client names + payload shape -------------------------------------- + + +def test_registry_adapter_targets_the_agent_registry_services(): + source = (_BACKEND / "src" / "app" / "services" / "aws_agent_registry.py").read_text() + assert 'CONTROL_SERVICE = "agent-registry-control"' in source + assert 'DATA_SERVICE = "agent-registry"' in source + + +def test_no_registry_calls_against_the_bedrock_agentcore_clients(): + """No source file may build a bedrock-agentcore client for Registry work.""" + offenders = [] + for path in _backend_sources(): + text = path.read_text() + if "registry_record" not in text and "Registry" not in text: + continue + for i, line in enumerate(text.splitlines(), 1): + if 'boto3.client("bedrock-agentcore' not in line: + continue + # Runtime/Gateway/Memory legitimately stay on bedrock-agentcore; only + # flag it inside the registry adapter itself. + if path.name == "aws_agent_registry.py": + offenders.append(f"{path.relative_to(_REPO_ROOT)}:{i}: {line.strip()}") + assert not offenders, "registry adapter still builds a bedrock-agentcore client:\n" + "\n".join(offenders) + + +def test_no_preview_kwargs_or_operations_in_backend_calls(): + """Preview parameter names / operation names must not reach the wire. + + This walks the AST rather than grepping lines, so the adapter's own + preview->GA documentation table (which necessarily *names* the old + spellings) doesn't register as a call site. What it looks at: + + * ``ast.keyword`` — a kwarg literally passed to a boto3 call + * ``ast.Attribute`` — a client method name being invoked + * ``ast.Dict`` keys — descriptor payload members + + All three are positions where a preview spelling would actually be sent. + """ + # Deliberately narrow: only spellings that are unambiguously Registry-preview. + # `authorizerType`/`authorizerConfiguration` are NOT listed — those remain + # valid Gateway (bedrock-agentcore) parameters. + bad_kwargs = {"descriptorType"} + bad_methods = {"search_registry_records"} + bad_dict_keys = {"inlineContent", "agentCard", "descriptorType"} + + offenders = [] + for path in _backend_sources(): + rel = path.relative_to(_REPO_ROOT) + try: + tree = ast.parse(path.read_text()) + except SyntaxError: # pragma: no cover - would fail elsewhere + continue + for node in ast.walk(tree): + if isinstance(node, ast.keyword) and node.arg in bad_kwargs: + offenders.append(f"{rel}:{node.value.lineno}: kwarg {node.arg}=") + elif isinstance(node, ast.Attribute) and node.attr in bad_methods: + offenders.append(f"{rel}:{node.lineno}: call .{node.attr}()") + elif isinstance(node, ast.Dict): + for key in node.keys: + if isinstance(key, ast.Constant) and key.value in bad_dict_keys: + offenders.append(f"{rel}:{key.lineno}: dict key {key.value!r}") + assert not offenders, "preview Agent Registry spellings still on the wire:\n" + "\n".join(offenders) + + +def test_legacy_record_types_are_aliases_only(): + """The preview enum values may only survive as INPUT aliases. + + `_LEGACY_RECORD_TYPES` exists so an older caller passing "a2a" still works — + but every value it maps to must be a GA enum member, and no preview value may + leak into RECORD_TYPES (what we send to AWS). + """ + from app.services import aws_agent_registry as ar + + assert set(ar._LEGACY_RECORD_TYPES.values()) <= set(ar.RECORD_TYPES) + assert not set(ar._LEGACY_RECORD_TYPES) & set(ar.RECORD_TYPES) + + +# -- dependency floor -------------------------------------------------------- + + +def test_lambda_bundle_pins_boto3_with_agent_registry_models(): + """1.43.66 is the first boto3 with the agent-registry service models. + + The Lambda bundle installs this file into backend/lib/, which PYTHONPATH + shadows ahead of the runtime's built-in boto3 — so this floor is what + actually decides whether the GA clients exist at runtime. + """ + text = (_BACKEND / "requirements-lambda.txt").read_text() + match = re.search(r"^boto3>=(\d+)\.(\d+)\.(\d+)", text, re.M) + assert match, "boto3 pin not found in requirements-lambda.txt" + assert tuple(int(g) for g in match.groups()) >= (1, 43, 66) + + +def test_pyproject_pins_boto3_with_agent_registry_models(): + text = (_BACKEND / "pyproject.toml").read_text() + match = re.search(r'"boto3>=(\d+)\.(\d+)\.(\d+)"', text) + assert match, "boto3 pin not found in pyproject.toml" + assert tuple(int(g) for g in match.groups()) >= (1, 43, 66) diff --git a/backend/tests/test_auto_register.py b/backend/tests/test_auto_register.py index 1ad8b8f..8cb464e 100644 --- a/backend/tests/test_auto_register.py +++ b/backend/tests/test_auto_register.py @@ -31,9 +31,16 @@ class _FakeRegistry: def __init__(self): self.registered = None - def register(self, name, descriptor_type, descriptors, description=""): # noqa: ARG002 - self.registered = {"name": name, "type": descriptor_type, "descriptors": descriptors} - return {"record_id": "rec-123", "arn": "arn:...:record/rec-123", "status": "DRAFT"} + def register(self, name, record_type, descriptors, description=""): # noqa: ARG002 + # GA signature: `record_type` (was `descriptor_type`), carrying a value + # from the recordType enum MCP/AGENT/CUSTOM/SKILL. + self.registered = {"name": name, "type": record_type, "descriptors": descriptors} + return { + "record_id": "rec4567890ab", + "arn": "arn:aws:agent-registry:us-east-1:123456789012:registry/reg1/record/rec4567890ab", + "status": "DRAFT", + "record_type": record_type, + } def _patch_registry(monkeypatch, registry): @@ -82,9 +89,9 @@ def test_custom_descriptor_for_non_a2a(monkeypatch): friendly_runtime_name="agent1", is_a2a=False, ) - assert reg.registered["type"] == "custom" + assert reg.registered["type"] == "CUSTOM" assert "custom" in reg.registered["descriptors"] - assert store.saved == ("d1", "rec-123", "DRAFT") + assert store.saved == ("d1", "rec4567890ab", "DRAFT") def test_a2a_descriptor_for_a2a_runtime(monkeypatch): @@ -99,6 +106,9 @@ def test_a2a_descriptor_for_a2a_runtime(monkeypatch): friendly_runtime_name="peer-agent", is_a2a=True, ) - assert reg.registered["type"] == "a2a" - assert "a2a" in reg.registered["descriptors"] - assert store.saved[1] == "rec-123" + # GA: an A2A runtime becomes an AGENT record carrying an a2aAgentCard + # descriptor. The preview "A2A" recordType no longer exists. + assert reg.registered["type"] == "AGENT" + assert "a2aAgentCard" in reg.registered["descriptors"] + assert "a2a" not in reg.registered["descriptors"] + assert store.saved[1] == "rec4567890ab" diff --git a/backend/tests/test_aws_agent_registry.py b/backend/tests/test_aws_agent_registry.py index 0d6640e..6da6e84 100644 --- a/backend/tests/test_aws_agent_registry.py +++ b/backend/tests/test_aws_agent_registry.py @@ -1,67 +1,364 @@ -"""Phase 6: AWS Agent Registry adapter. +"""Phase 6: AWS Agent Registry adapter — GA API surface. -Pure helpers (descriptor builders, ARN parsing) + adapter behavior against a -fake control/data client that captures kwargs. No real AWS. +Pure helpers (descriptor builders, ARN parsing, name/type normalization) plus +adapter behavior against a fake control/data client that captures kwargs. No +real AWS. + +These tests deliberately pin the GA wire shape, because the preview -> GA change +is a SILENT one: the deprecated `bedrock-agentcore-control` model still exposes +CreateRegistryRecord with the old `descriptorType` parameter, so a regression +back to preview spellings would not raise UnknownOperation locally. """ from __future__ import annotations import json +import pytest from app.services import aws_agent_registry as ar from app.services.aws_agent_registry import ( AwsAgentRegistry, build_a2a_descriptor, build_custom_descriptor, + build_mcp_descriptor, + normalize_record_type, + sanitize_record_name, ) +# -- GA namespace ------------------------------------------------------------ + + +def test_ga_service_names(): + """Registry is its own service at GA — not bedrock-agentcore.""" + assert ar.CONTROL_SERVICE == "agent-registry-control" + assert ar.DATA_SERVICE == "agent-registry" + assert ar.MIN_BOTO3 >= (1, 43, 66) + + +def test_ga_record_type_enum(): + """recordType replaced descriptorType; A2A/AGENT_SKILLS were renamed.""" + assert set(ar.RECORD_TYPES) == {"MCP", "AGENT", "CUSTOM", "SKILL"} + assert "A2A" not in ar.RECORD_TYPES + assert "AGENT_SKILLS" not in ar.RECORD_TYPES + + # -- pure helpers ------------------------------------------------------------ def test_record_id_from_arn(): - arn = "arn:aws:bedrock-agentcore:us-east-1:123456789012:registry/reg1/record/rec-abc" - assert ar._record_id_from_arn(arn) == "rec-abc" + # GA ARNs use the agent-registry service namespace. + arn = "arn:aws:agent-registry:us-east-1:123456789012:registry/abc123def456/record/rec4567890ab" + assert ar._record_id_from_arn(arn) == "rec4567890ab" assert ar._record_id_from_arn("") == "" def test_a2a_descriptor_shape(): d = build_a2a_descriptor("bot", "does things", "https://x/invoke", skills=[{"id": "s1", "name": "search"}]) - # Wrapped under the "a2a" type key (API-required — caught live). - assert set(d.keys()) == {"a2a"} - ac = d["a2a"]["agentCard"] - card = json.loads(ac["inlineContent"]) - assert ac["schemaVersion"] == "0.3" + # GA: a flat `a2aAgentCard` descriptor holding {data, dataSchemaVersion} — + # preview nested this as a2a.agentCard.inlineContent. + assert set(d.keys()) == {"a2aAgentCard"} + ac = d["a2aAgentCard"] + assert set(ac.keys()) == {"data", "dataSchemaVersion"} + assert ac["dataSchemaVersion"] == "0.3" + card = json.loads(ac["data"]) assert card["name"] == "bot" and card["url"] == "https://x/invoke" assert card["protocolVersion"] == "0.3" - assert card["skills"] == [{"id": "s1", "name": "search"}] + # skills are normalized up to the required id/name/description/tags set — the + # live service rejects the entire card if any of the four is missing. + assert card["skills"] == [{"id": "s1", "name": "search", "description": "search", "tags": []}] + + +def test_a2a_descriptor_has_no_preview_keys(): + d = build_a2a_descriptor("bot", "d", "https://x") + assert "a2a" not in d + assert "inlineContent" not in d["a2aAgentCard"] + assert "schemaVersion" not in d["a2aAgentCard"] def test_a2a_description_capped_at_100(): d = build_a2a_descriptor("bot", "x" * 200, "https://x") - card = json.loads(d["a2a"]["agentCard"]["inlineContent"]) + card = json.loads(d["a2aAgentCard"]["data"]) assert len(card["description"]) == 100 def test_custom_descriptor_roundtrips(): d = build_custom_descriptor({"framework": "strands", "model": "claude"}) assert set(d.keys()) == {"custom"} - assert json.loads(d["custom"]["inlineContent"])["framework"] == "strands" + # `custom` is the one descriptor with no dataSchemaVersion member. + assert set(d["custom"].keys()) == {"data"} + assert json.loads(d["custom"]["data"])["framework"] == "strands" + + +def test_mcp_descriptor_nests_tools_under_additional_data(): + d = build_mcp_descriptor({"name": "io.github.acme/srv", "version": "1.0.0"}, tools=[{"name": "search"}]) + srv = d["mcpServer"] + assert srv["dataSchemaVersion"] == ar.MCP_SERVER_SCHEMA_VERSION + tools = srv["additionalData"]["tools"] + # `inputSchema` is required per tool by the live schema and is filled in. + assert json.loads(tools["data"]) == { + "tools": [{"name": "search", "inputSchema": {"type": "object", "properties": {}}}] + } + assert tools["dataSchemaVersion"] == ar.MCP_TOOLS_SCHEMA_VERSION + + +def test_descriptor_data_over_the_size_cap_fails_locally(): + """`data` is capped at 102400 bytes. AWS's ValidationException doesn't say + WHICH descriptor overflowed, and on the deploy path it lands in a best-effort + handler — so the builders name the culprit themselves.""" + huge = {"blob": "x" * (ar._DATA_MAX + 10)} + with pytest.raises(ValueError, match="custom"): + build_custom_descriptor(huge) + with pytest.raises(ValueError, match="mcpServer"): + build_mcp_descriptor(huge) + with pytest.raises(ValueError, match="a2aAgentCard"): + build_a2a_descriptor("bot", "d", "https://x", skills=[huge]) + with pytest.raises(ValueError, match=r"additionalData\.tools"): + build_mcp_descriptor({"name": "srv"}, tools=[dict(huge, name="t")]) + + +def test_descriptor_size_is_measured_in_bytes_not_characters(): + """The cap is bytes; multi-byte characters must not slip past a len() check.""" + # 4-byte emoji: well under _DATA_MAX characters, well over it in bytes. + payload = {"blob": "🚀" * (ar._DATA_MAX // 3)} + with pytest.raises(ValueError, match="custom"): + build_custom_descriptor(payload) + + +def test_descriptor_just_under_the_cap_is_accepted(): + d = build_custom_descriptor({"b": "x" * (ar._DATA_MAX - 100)}) + assert len(d["custom"]["data"].encode("utf-8")) <= ar._DATA_MAX + + +def test_mcp_descriptor_omits_additional_data_when_no_tools(): + d = build_mcp_descriptor({"name": "srv"}) + assert "additionalData" not in d["mcpServer"] + + +# -- live-verified descriptor content contracts ------------------------------- +# +# Every assertion below was established by submitting candidates to the real GA +# service and bisecting the failures. The service rejects a bad `data` document +# with one message — "content is not in compliance with schema version 'X' for +# descriptor type 'T'" — naming neither the field nor even which sub-document, so +# none of this is discoverable from the SDK model or an error string. + + +def test_mcp_server_name_must_be_namespaced(): + """A bare "my-server" is REJECTED live; the name is `/`.""" + d = build_mcp_descriptor({"name": "my-server", "description": "d", "version": "1.0.0"}) + doc = json.loads(d["mcpServer"]["data"]) + assert doc["name"] == f"{ar.MCP_DEFAULT_NAMESPACE}/my-server" + + +def test_mcp_server_name_already_namespaced_is_left_alone(): + d = build_mcp_descriptor({"name": "io.github.acme/srv", "description": "d", "version": "2.0.0"}) + assert json.loads(d["mcpServer"]["data"])["name"] == "io.github.acme/srv" + + +@pytest.mark.parametrize( + ("given", "expected"), + [ + # exactly one "/" — a second slash is rejected by the service + ("io.github.acme/srv/extra", "io.github.acme/srv-extra"), + # an empty half is rejected; a good namespace is kept and the server half filled + ("io.github.acme/", "io.github.acme/server"), + ("/srv", f"{ar.MCP_DEFAULT_NAMESPACE}/srv"), + # "_" is legal in the server half but NOT in the namespace + ("io_github/srv", f"{ar.MCP_DEFAULT_NAMESPACE}/io_github-srv"), + ("io.github.acme/my-srv_1", "io.github.acme/my-srv_1"), + # nothing usable at all still yields a valid name + ("", f"{ar.MCP_DEFAULT_NAMESPACE}/server"), + ("///", f"{ar.MCP_DEFAULT_NAMESPACE}/server"), + ], +) +def test_sanitize_mcp_server_name(given, expected): + got = ar.sanitize_mcp_server_name(given) + assert got == expected + # whatever we produce must satisfy the pattern the service enforces + assert ar._MCP_NAME_RE.match(got), got + + +def test_mcp_server_description_and_version_are_required_and_filled(): + """Live: dropping either `description` or `version` fails validation. The old + builder emitted only {name, version}, so every MCP record was rejected.""" + d = build_mcp_descriptor({"name": "io.github.acme/srv"}) + doc = json.loads(d["mcpServer"]["data"]) + assert doc["description"] == "io.github.acme/srv" + assert doc["version"] == "1.0.0" + + +def test_mcp_server_passes_through_extra_server_json_fields(): + """The schema is open: unknown keys are accepted. Normalization must be additive + so legitimate server.json members survive.""" + d = build_mcp_descriptor( + { + "name": "io.github.acme/srv", + "description": "d", + "version": "1.0.0", + "remotes": [{"type": "streamable-http", "url": "https://x/mcp"}], + "repository": {"url": "https://github.com/a/b", "source": "github"}, + } + ) + doc = json.loads(d["mcpServer"]["data"]) + assert doc["remotes"] == [{"type": "streamable-http", "url": "https://x/mcp"}] + assert doc["repository"]["source"] == "github" + + +def test_mcp_tool_without_a_name_raises(): + """`name` is required per tool and cannot be invented — a synthesized name would + publish a tool nothing can invoke. This builder has no best-effort caller, so + raising beats repairing.""" + with pytest.raises(ValueError, match="no 'name'"): + build_mcp_descriptor({"name": "io.github.acme/srv"}, tools=[{"description": "d"}]) + + +def test_mcp_tool_keeps_a_caller_supplied_input_schema(): + schema = {"type": "object", "properties": {"q": {"type": "string"}}, "required": ["q"]} + d = build_mcp_descriptor({"name": "io.github.acme/srv"}, tools=[{"name": "search", "inputSchema": schema}]) + got = json.loads(d["mcpServer"]["additionalData"]["tools"]["data"])["tools"][0] + assert got["inputSchema"] == schema + + +def test_a2a_skills_are_normalized_to_the_required_field_set(): + """Live: a skill entry needs ALL of id/name/description/tags or the whole card + is rejected. `tags: []` is accepted, an ABSENT `tags` is not.""" + d = build_a2a_descriptor("bot", "d", "https://x", skills=[{"name": "search"}]) + skill = json.loads(d["a2aAgentCard"]["data"])["skills"][0] + assert skill == {"id": "search", "name": "search", "description": "search", "tags": []} + + +def test_a2a_skill_with_nothing_usable_still_yields_a_valid_entry(): + """Repair, not raise: this runs on the best-effort auto-register-on-deploy path, + where dropping the whole governance record over one thin skill is worse.""" + d = build_a2a_descriptor("bot", "d", "https://x", skills=[{}, {"tags": "not-a-list"}]) + skills = json.loads(d["a2aAgentCard"]["data"])["skills"] + assert [s["id"] for s in skills] == ["skill-1", "skill-2"] + assert all(s["tags"] == [] for s in skills) + + +def test_a2a_skill_preserves_extra_keys_and_real_tags(): + d = build_a2a_descriptor( + "bot", + "d", + "https://x", + skills=[{"id": "s1", "name": "search", "description": "finds", "tags": ["web"], "examples": ["e"]}], + ) + skill = json.loads(d["a2aAgentCard"]["data"])["skills"][0] + assert skill["tags"] == ["web"] + assert skill["examples"] == ["e"] + + +def test_agent_skills_descriptor_omits_data_schema_version(): + """Live: `agentSkillsDefinition` is the one descriptor that accepts NO + dataSchemaVersion — every candidate value comes back "not supported for + descriptor type 'agent_skills'". It is also the one that must wrap its list in + a {"skills": [...]} object; a bare array is rejected.""" + d = ar.build_agent_skills_descriptor([{"name": "search"}]) + assert set(d["agentSkillsDefinition"].keys()) == {"data"} + payload = json.loads(d["agentSkillsDefinition"]["data"]) + assert payload == {"skills": [{"id": "search", "name": "search", "description": "search", "tags": []}]} + + +def test_agent_skills_descriptor_with_no_skills(): + d = ar.build_agent_skills_descriptor() + assert json.loads(d["agentSkillsDefinition"]["data"]) == {"skills": []} + + +@pytest.mark.parametrize( + ("given", "expected"), + [ + ("AGENT", "AGENT"), + ("MCP", "MCP"), + ("SKILL", "SKILL"), + ("CUSTOM", "CUSTOM"), + # preview spellings + ("A2A", "AGENT"), + ("a2a", "AGENT"), + ("AGENT_SKILLS", "SKILL"), + ("custom", "CUSTOM"), + ("mcp", "MCP"), + # descriptor keys + ("a2aAgentCard", "AGENT"), + ("mcpServer", "MCP"), + # unknown / empty degrade to CUSTOM rather than failing a deploy + ("", "CUSTOM"), + (None, "CUSTOM"), + ("nonsense", "CUSTOM"), + ], +) +def test_normalize_record_type(given, expected): + assert normalize_record_type(given) == expected + + +@pytest.mark.parametrize( + ("given", "expected"), + [ + ("my-agent", "my-agent"), + ("my agent", "my_agent"), + ("_leading", "leading"), + ("travel.agent/v1", "travel.agent/v1"), + ("!!!", "agent"), + ("", "agent"), + ], +) +def test_sanitize_record_name(given, expected): + """Service pattern is [a-zA-Z0-9][a-zA-Z0-9_\\-./]* (1-255).""" + out = sanitize_record_name(given) + assert out == expected + assert out[0].isalnum() + assert 1 <= len(out) <= 255 + + +def test_sanitize_record_name_truncates_to_255(): + assert len(sanitize_record_name("a" * 400)) == 255 # -- adapter (fake client) --------------------------------------------------- +class _Conflict(Exception): + """Stands in for botocore's ConflictException — same duck type _is_conflict reads.""" + + def __init__(self, message="A record with name 'bot' and version '1.0' already exists"): + super().__init__(message) + self.response = {"Error": {"Code": "ConflictException", "Message": message}} + + class _FakeControl: - def __init__(self): + def __init__(self, pages=None, registry_status="READY", conflict_on_create=False): self.calls = [] + # list_registry_records pages, each (records, nextToken) + self._pages = pages or [([], None)] + self._page_idx = 0 + self._registry_status = registry_status + self._conflict_on_create = conflict_on_create def get_registry(self, **kw): self.calls.append(("get_registry", kw)) - return {"registryId": kw["registryId"]} + # GetRegistry always returns `status`; a fake that omitted it was modelling + # a response the service never sends, which is what let the CREATING race + # hide (see test_available_is_false_while_the_registry_is_still_creating). + return {"registryId": kw["registryId"], "status": self._registry_status} def create_registry_record(self, **kw): self.calls.append(("create_registry_record", kw)) - return {"recordArn": "arn:aws:bedrock-agentcore:us-east-1:1:registry/r/record/rec-1", "status": "CREATING"} + if self._conflict_on_create: + raise _Conflict() + return { + "recordArn": ("arn:aws:agent-registry:us-east-1:123456789012:registry/reg1/record/rec4567890ab"), + "status": "CREATING", + } + + def update_registry_record(self, **kw): + self.calls.append(("update_registry_record", kw)) + return { + "recordId": kw["recordId"], + "recordArn": (f"arn:aws:agent-registry:us-east-1:123456789012:registry/reg1/record/{kw['recordId']}"), + "name": "bot", + "recordType": "CUSTOM", + # The service demotes an edited record out of APPROVED — verified live. + "status": "DRAFT", + } def submit_registry_record_for_approval(self, **kw): self.calls.append(("submit", kw)) @@ -69,19 +366,32 @@ def submit_registry_record_for_approval(self, **kw): def update_registry_record_status(self, **kw): self.calls.append(("update_status", kw)) + def list_registry_records(self, **kw): + self.calls.append(("list_registry_records", kw)) + records, token = self._pages[self._page_idx] + self._page_idx = min(self._page_idx + 1, len(self._pages) - 1) + return {"registryRecords": records, **({"nextToken": token} if token else {})} + + def delete_registry_record(self, **kw): + self.calls.append(("delete_registry_record", kw)) + return {} + class _FakeData: def __init__(self, results): self._results = results + self.calls = [] - def search_registry_records(self, **kw): + def search_discoverable_registry_records(self, **kw): + self.calls.append(("search", kw)) return {"registryRecords": self._results} -def _adapter(results=None): +def _adapter(results=None, pages=None, registry_status="READY", conflict_on_create=False): a = AwsAgentRegistry.__new__(AwsAgentRegistry) a.registry_id = "reg1" - a.control = _FakeControl() + a.region = "us-east-1" + a.control = _FakeControl(pages=pages, registry_status=registry_status, conflict_on_create=conflict_on_create) a.data = _FakeData(results or []) return a @@ -91,14 +401,191 @@ def test_available_true_when_get_registry_ok(): assert a.available() is True -def test_register_parses_record_id_from_arn(): +# -- the registry must be READY, not merely present --------------------------- + + +@pytest.mark.parametrize("status", ["CREATING", "UPDATING", "DELETING", "CREATE_FAILED", "UPDATE_FAILED"]) +def test_available_is_false_while_the_registry_is_still_creating(status): + """Live-confirmed: CreateRegistryRecord against a non-READY registry fails with + ``ConflictException: Registry is not in READY state``. available() used to only + check that GetRegistry succeeded, so it returned True through the whole CREATING + window — and enabling federation on a just-created registry then raced into that + conflict on the first deploy, where auto-register swallows errors.""" + a = _adapter(registry_status=status) + assert a.available() is False + assert a.registry_status() == status + + +def test_registry_status_returns_none_when_unreadable(): + """None means "could not ask" — distinct from a status string. The router needs + the distinction to avoid blaming a registryId that was never wrong.""" + + class _Boom: + def get_registry(self, **kw): + raise RuntimeError("AccessDeniedException") + + a = AwsAgentRegistry.__new__(AwsAgentRegistry) + a.registry_id = "reg1" + a.region = "us-east-1" + a.control = _Boom() + a.data = None + assert a.registry_status() is None + assert a.available() is False + + +def test_registry_status_none_on_old_boto3_bundle(): + a = AwsAgentRegistry.__new__(AwsAgentRegistry) + a.registry_id = "reg1" + a.region = "us-east-1" + a.control = None + a.data = None + assert a.registry_status() is None + + +# -- redeploy: register() is an upsert ---------------------------------------- +# +# name + recordVersion is a uniqueness key and recordVersion is always "1.0" here, +# so the SECOND deployment of an agent under the same name used to raise +# ConflictException — swallowed by the deploy path's best-effort handler. The +# symptom was a record frozen at the first deployment's runtime ARN, stale forever, +# with no error surfaced anywhere. + +_EXISTING = { + "recordId": "rec000000001", + "recordArn": "arn:aws:agent-registry:us-east-1:123456789012:registry/reg1/record/rec000000001", + "name": "bot", + "recordVersion": "1.0", + "status": "APPROVED", +} + + +def test_register_updates_in_place_when_the_name_and_version_exist(): + a = _adapter(pages=[([_EXISTING], None)], conflict_on_create=True) + out = a.register("bot", "CUSTOM", build_custom_descriptor({"endpoint": "https://new"}), description="redeploy") + assert out["record_id"] == "rec000000001" + assert out["updated"] is True + ops = [name for name, _ in a.control.calls] + assert "update_registry_record" in ops + + +def test_register_update_uses_the_optional_value_patch_envelope(): + """UpdateRegistryRecord takes a DIFFERENT descriptor shape from Create: every + branch and leaf is wrapped in `optionalValue`. Passing the Create shape fails in + botocore's own parameter validation, before any AWS call — so nothing but a real + shape assertion catches it.""" + a = _adapter(pages=[([_EXISTING], None)], conflict_on_create=True) + a.register("bot", "CUSTOM", build_custom_descriptor({"endpoint": "https://new"})) + _, kw = next((c for c in a.control.calls if c[0] == "update_registry_record"), (None, None)) + assert kw is not None + inner = kw["descriptors"]["optionalValue"]["custom"]["optionalValue"] + assert json.loads(inner["data"]["optionalValue"])["endpoint"] == "https://new" + assert kw["description"]["optionalValue"] + assert kw["displayName"]["optionalValue"] == "bot" + # identity members must NOT be sent — this call refreshes content only + assert "name" not in kw and "recordType" not in kw and "recordVersion" not in kw + + +def test_update_envelope_carries_schema_versions_and_nested_tools(): + d = ar._to_update_descriptors(build_mcp_descriptor({"name": "io.github.acme/srv"}, tools=[{"name": "search"}])) + server = d["optionalValue"]["mcpServer"]["optionalValue"] + assert server["dataSchemaVersion"]["optionalValue"] == ar.MCP_SERVER_SCHEMA_VERSION + tools = server["additionalData"]["optionalValue"]["tools"]["optionalValue"] + assert tools["dataSchemaVersion"]["optionalValue"] == ar.MCP_TOOLS_SCHEMA_VERSION + assert json.loads(tools["data"]["optionalValue"])["tools"][0]["name"] == "search" + + +def test_register_reraises_a_conflict_that_is_not_an_existing_record(): + """ConflictException ALSO means "registry is not in READY state". Treating every + conflict as a redeploy would swallow that and silently register nothing, so the + lookup must confirm a record exists before updating.""" + a = _adapter(pages=[([], None)], conflict_on_create=True) # nothing found + with pytest.raises(Exception, match="already exists"): + a.register("bot", "CUSTOM", build_custom_descriptor({})) + + +def test_register_reraises_a_conflict_when_only_another_version_exists(): + other_version = dict(_EXISTING, recordVersion="2.0") + a = _adapter(pages=[([other_version], None)], conflict_on_create=True) + with pytest.raises(Exception, match="already exists"): + a.register("bot", "CUSTOM", build_custom_descriptor({})) + + +def test_register_reraises_non_conflict_errors_untouched(): + """A ValidationException must not be reinterpreted as a redeploy.""" + + class _Boom(_FakeControl): + def create_registry_record(self, **kw): + raise RuntimeError("ValidationException: descriptors invalid") + + a = _adapter() + a.control = _Boom() + with pytest.raises(RuntimeError, match="ValidationException"): + a.register("bot", "CUSTOM", build_custom_descriptor({})) + + +def test_find_record_filters_server_side_by_name(): + a = _adapter(pages=[([_EXISTING], None)]) + assert a._find_record("bot", "1.0")["recordId"] == "rec000000001" + _, kw = next(c for c in a.control.calls if c[0] == "list_registry_records") + # `filters[].values` is capped at one entry by the service model. + assert kw["filters"] == [{"name": "name", "values": ["bot"]}] + + +def test_find_record_returns_none_when_the_lookup_itself_fails(): + """A failed lookup must not be read as "no such record" — register() then + re-raises the original conflict rather than inventing a create/update decision + from missing data.""" + a = _adapter() + a.control = None # strict listing raises RegistryQueryFailed + assert a._find_record("bot", "1.0") is None + + +def test_register_sends_ga_record_type_param(): a = _adapter() - out = a.register("bot", "A2A", build_a2a_descriptor("bot", "d", "https://x")) - assert out["record_id"] == "rec-1" + out = a.register("bot", "AGENT", build_a2a_descriptor("bot", "d", "https://x")) + assert out["record_id"] == "rec4567890ab" assert out["status"] == "CREATING" - # correct API params were sent _, kw = a.control.calls[-1] - assert kw["registryId"] == "reg1" and kw["descriptorType"] == "A2A" + assert kw["registryId"] == "reg1" + # GA parameter name, GA enum value. + assert kw["recordType"] == "AGENT" + assert "descriptorType" not in kw + assert kw["recordVersion"] == "1.0" + assert "a2aAgentCard" in kw["descriptors"] + + +def test_register_normalizes_preview_record_type(): + """A caller still passing the preview "a2a" gets a GA AGENT record.""" + a = _adapter() + out = a.register("bot", "a2a", build_a2a_descriptor("bot", "d", "https://x")) + _, kw = a.control.calls[-1] + assert kw["recordType"] == "AGENT" + assert out["record_type"] == "AGENT" + + +def test_register_rejects_descriptor_type_mismatch(): + """recordType and descriptor key must agree — caught locally, not by AWS.""" + a = _adapter() + with pytest.raises(ValueError, match="a2aAgentCard"): + a.register("bot", "AGENT", build_custom_descriptor({"x": 1})) + + +def test_register_sanitizes_name_and_never_sends_empty_description(): + a = _adapter() + a.register("my agent", "CUSTOM", build_custom_descriptor({"x": 1}), description="") + _, kw = a.control.calls[-1] + assert kw["name"] == "my_agent" + # description has min length 1 in the service model. + assert kw["description"] + # displayName keeps the human-readable original. + assert kw["displayName"] == "my agent" + + +def test_register_clamps_description_to_4096(): + a = _adapter() + a.register("bot", "CUSTOM", build_custom_descriptor({"x": 1}), description="d" * 9000) + _, kw = a.control.calls[-1] + assert len(kw["description"]) == 4096 def test_set_status_sends_reason(): @@ -115,6 +602,89 @@ def test_submit_for_approval(): assert a.control.calls[-1][0] == "submit" -def test_search_returns_records(): +def test_search_uses_ga_operation_name(): + """GA renamed SearchRegistryRecords -> SearchDiscoverableRegistryRecords.""" a = _adapter(results=[{"name": "found-agent"}]) assert a.search("agent")[0]["name"] == "found-agent" + _, kw = a.data.calls[-1] + assert kw["registryIds"] == ["reg1"] + assert kw["searchQuery"] == "agent" + + +def test_search_record_type_filter_uses_structured_operator(): + """Data-plane filters is a structure with $eq/$ne/$in — not a list.""" + a = _adapter(results=[]) + a.search("agent", record_types=["a2a", "MCP"]) + _, kw = a.data.calls[-1] + assert kw["filters"] == {"recordType": {"$in": ["AGENT", "MCP"]}} + + +def test_search_returns_empty_when_data_client_missing(): + a = _adapter() + a.data = None + assert a.search("agent") == [] + + +def test_list_records_follows_next_token(): + """Truncated listings would silently fail-close the integration gate.""" + a = _adapter( + pages=[ + ([{"name": "one"}], "tok1"), + ([{"name": "two"}], None), + ] + ) + out = a.list_records() + assert [r["name"] for r in out] == ["one", "two"] + # second call carried the token forward + second = [kw for name, kw in a.control.calls if name == "list_registry_records"][1] + assert second["nextToken"] == "tok1" + + +def test_list_records_passes_filters_through(): + a = _adapter(pages=[([], None)]) + a.list_records(filters=[{"name": "status", "values": ["APPROVED"]}]) + _, kw = a.control.calls[-1] + assert kw["filters"] == [{"name": "status", "values": ["APPROVED"]}] + + +def test_list_records_returns_partial_page_on_error(): + a = _adapter() + + class _Boom(_FakeControl): + def list_registry_records(self, **kw): + raise RuntimeError("throttled") + + a.control = _Boom() + assert a.list_records() == [] + + +# -- graceful degradation on an old boto3 bundle ------------------------------ + + +def test_adapter_with_no_clients_degrades_instead_of_raising(): + """An old boto3 bundle has no agent-registry models; the feature must report + unavailable rather than 500 the registry router.""" + a = AwsAgentRegistry.__new__(AwsAgentRegistry) + a.registry_id = "reg1" + a.region = "us-east-1" + a.control = None + a.data = None + assert a.available() is False + assert a.register("bot", "CUSTOM", build_custom_descriptor({})) == {} + assert a.list_records() == [] + assert a.search("x") == [] + assert a.get("rec-1") is None + assert a.delete("rec-1") is False + # these are no-ops, not exceptions + a.submit_for_approval("rec-1") + a.set_status("rec-1", "APPROVED", "why") + + +def test_make_client_returns_none_for_unknown_service(): + assert ar._make_client("definitely-not-an-aws-service", "us-east-1") is None + + +def test_agent_registry_supported_is_a_bool(): + # Environment-dependent (depends on the installed boto3), so assert only the + # contract: it never raises and always answers yes/no. + assert isinstance(ar.agent_registry_supported(), bool) diff --git a/backend/tests/test_aws_registry_router.py b/backend/tests/test_aws_registry_router.py new file mode 100644 index 0000000..467db35 --- /dev/null +++ b/backend/tests/test_aws_registry_router.py @@ -0,0 +1,253 @@ +"""AWS Agent Registry federation endpoints (/api/registry/aws-*). + +FastAPI TestClient with the caller/admin dependencies overridden — no Cognito, no +AWS. The adapter itself is monkeypatched, so these tests are about the ROUTER's +contract: what it reports, and which failure it blames. + +The GA migration made one distinction load-bearing: "unreachable because the +registryId/IAM is wrong" vs "unreachable because this bundle's boto3 has no +agent-registry service models at all". The first is fixed in the console, the +second only by redeploying. Conflating them sends admins after the wrong bug, so +both the GET status payload and the POST error assert on it here. +""" + +from __future__ import annotations + +import sys + +from fastapi import FastAPI +from fastapi.testclient import TestClient + +sys.path.insert(0, "src") + +from app.routers import registry as registry_router_mod # noqa: E402 +from app.routers.registry import caller_is_admin # noqa: E402 +from app.services import aws_agent_registry as ar # noqa: E402 +from app.services.auth import get_caller_sub # noqa: E402 + + +def _client(admin: bool = True) -> TestClient: + app = FastAPI() + app.include_router(registry_router_mod.router) + app.dependency_overrides[get_caller_sub] = lambda: "alice" + app.dependency_overrides[caller_is_admin] = lambda: admin + return TestClient(app) + + +_RAISES = object() + +# Default control-plane view: agrees with what search() returns. +_TRUTH_AGREES = [{"recordId": "aaaaaaaaaaaa", "name": "found", "status": "APPROVED"}] + + +class _FakeReg: + """`status` is what the router actually reads. `ok=False` with status=None models + the unreadable case (bad id / old SDK); pass an explicit status to model a + registry that answers but is not READY. + + `truth` is the CONTROL-plane record listing, kept separate from what search() + returns so a test can model the live-verified case where the two planes + disagree. Pass `_RAISES` to model the control plane being unqueryable.""" + + def __init__(self, ok: bool = True, status: str | None = "__default__", truth=_TRUTH_AGREES): + self._ok = ok + self._status = ("READY" if ok else None) if status == "__default__" else status + self.truth = truth + + def registry_status(self) -> str | None: + return self._status + + def available(self) -> bool: + return self._status == "READY" + + def search(self, q, **kw): + # The data plane's own view. `truth` below is the control plane's; when they + # disagree the router must serve the control plane's (see the drift tests). + return [{"recordId": "aaaaaaaaaaaa", "name": "found", "recordType": "AGENT", "status": "APPROVED"}] + + def list_records_strict(self, **kw): + if self.truth is _RAISES: + raise ar.RegistryQueryFailed("control plane unreachable") + return self.truth + + +# -- GET /aws-config --------------------------------------------------------- + + +def test_config_reports_not_configured(monkeypatch): + monkeypatch.setattr(ar, "get_configured_registry_id", lambda: None) + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + body = _client().get("/api/registry/aws-config").json() + assert body == { + "enabled": False, + "registry_id": None, + "available": False, + "sdk_supported": True, + "status": None, + } + + +def test_config_reports_connected(monkeypatch): + monkeypatch.setattr(ar, "get_configured_registry_id", lambda: "reg1") + monkeypatch.setattr(ar, "get_registry", lambda: _FakeReg(True)) + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + body = _client().get("/api/registry/aws-config").json() + assert body["enabled"] is True and body["available"] is True + assert body["registry_id"] == "reg1" + + +def test_config_flags_an_sdk_too_old_for_ga(monkeypatch): + """The clients can't be built at all, so available() is False for a VALID id — + sdk_supported is what tells the UI not to blame the registryId.""" + monkeypatch.setattr(ar, "get_configured_registry_id", lambda: "reg1") + monkeypatch.setattr(ar, "get_registry", lambda: _FakeReg(False)) + monkeypatch.setattr(ar, "agent_registry_supported", lambda: False) + body = _client().get("/api/registry/aws-config").json() + assert body["enabled"] is True + assert body["available"] is False + assert body["sdk_supported"] is False + + +def test_config_never_500s_when_the_adapter_is_unavailable(monkeypatch): + """get_registry() returning None (no boto3 models) must degrade, not raise.""" + monkeypatch.setattr(ar, "get_configured_registry_id", lambda: "reg1") + monkeypatch.setattr(ar, "get_registry", lambda: None) + monkeypatch.setattr(ar, "agent_registry_supported", lambda: False) + resp = _client().get("/api/registry/aws-config") + assert resp.status_code == 200 + assert resp.json()["available"] is False + + +# -- POST /aws-config -------------------------------------------------------- + + +def test_enable_rejects_non_admin(monkeypatch): + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + resp = _client(admin=False).post("/api/registry/aws-config", json={"registry_id": "reg1"}) + assert resp.status_code == 403 + + +def test_enable_blames_the_sdk_not_the_registry_id(monkeypatch): + """An old bundle must not produce "check the registryId" — that's misleading + and the admin cannot act on it.""" + monkeypatch.setattr(ar, "agent_registry_supported", lambda: False) + resp = _client().post("/api/registry/aws-config", json={"registry_id": "reg1"}) + assert resp.status_code == 400 + detail = resp.json()["detail"] + assert "SDK" in detail and "1.43.66" in detail + assert "registryId" not in detail + + +def test_enable_blames_the_registry_id_when_the_sdk_is_fine(monkeypatch): + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + monkeypatch.setattr(ar, "AwsAgentRegistry", lambda rid: _FakeReg(False)) + resp = _client().post("/api/registry/aws-config", json={"registry_id": "bogus"}) + assert resp.status_code == 400 + assert "registryId" in resp.json()["detail"] + + +def test_enable_persists_a_reachable_registry(monkeypatch): + saved = {} + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + monkeypatch.setattr(ar, "AwsAgentRegistry", lambda rid: _FakeReg(True)) + monkeypatch.setattr(ar, "set_configured_registry_id", lambda rid: saved.setdefault("id", rid)) + resp = _client().post("/api/registry/aws-config", json={"registry_id": "reg1"}) + assert resp.status_code == 200 + assert saved["id"] == "reg1" + + +# -- "not READY yet" is a third, distinct state ------------------------------- + + +def test_enable_says_still_provisioning_rather_than_blaming_the_id(monkeypatch): + """A registry takes tens of seconds to reach READY, and enabling federation + right after creating one in the console is the normal sequence. Reporting that + as "check the registryId" sends the admin to re-verify something correct; the + real instruction is "retry in a moment".""" + saved = {} + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + monkeypatch.setattr(ar, "AwsAgentRegistry", lambda rid: _FakeReg(status="CREATING")) + monkeypatch.setattr(ar, "set_configured_registry_id", lambda rid: saved.setdefault("id", rid)) + resp = _client().post("/api/registry/aws-config", json={"registry_id": "reg1"}) + assert resp.status_code == 409 + detail = resp.json()["detail"] + assert "CREATING" in detail and "READY" in detail + assert "registryId" not in detail + # and it must NOT be persisted — records cannot be written yet + assert saved == {} + + +def test_config_surfaces_a_non_ready_status(monkeypatch): + monkeypatch.setattr(ar, "get_configured_registry_id", lambda: "reg1") + monkeypatch.setattr(ar, "get_registry", lambda: _FakeReg(status="UPDATING")) + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + body = _client().get("/api/registry/aws-config").json() + assert body["available"] is False + assert body["status"] == "UPDATING" + assert body["sdk_supported"] is True + + +def test_config_status_is_none_when_the_registry_cannot_be_read(monkeypatch): + """None distinguishes "we could not ask" from any real lifecycle state, so the + UI can keep blaming the registryId only in the case that warrants it.""" + monkeypatch.setattr(ar, "get_configured_registry_id", lambda: "reg1") + monkeypatch.setattr(ar, "get_registry", lambda: _FakeReg(False)) + monkeypatch.setattr(ar, "agent_registry_supported", lambda: True) + body = _client().get("/api/registry/aws-config").json() + assert body["available"] is False + assert body["status"] is None + + +# -- GET /aws-search --------------------------------------------------------- + + +def test_search_returns_disabled_when_unconfigured(monkeypatch): + monkeypatch.setattr(ar, "get_registry", lambda: None) + body = _client().get("/api/registry/aws-search?q=bot").json() + assert body == {"enabled": False, "results": []} + + +def test_search_passes_ga_record_fields_through(monkeypatch): + """recordType/status are GA-only response fields the panel renders as chips.""" + monkeypatch.setattr(ar, "get_registry", lambda: _FakeReg(True)) + body = _client().get("/api/registry/aws-search?q=bot").json() + assert body["enabled"] is True + assert body["results"][0]["recordType"] == "AGENT" + assert body["results"][0]["status"] == "APPROVED" + assert body["status_authoritative"] is True + + +def test_search_status_comes_from_the_control_plane_not_the_search_index(monkeypatch): + """Live-verified GA drift: the data plane keeps serving a demoted record as + APPROVED for minutes after the control plane says DRAFT. + + Reachable on the ordinary path — register() upserts on redeploy and an update + demotes the record — so a redeployed integration would show an APPROVED badge + on a governance screen while it is actually waiting on re-review.""" + monkeypatch.setattr( + ar, + "get_registry", + lambda: _FakeReg(True, truth=[{"recordId": "aaaaaaaaaaaa", "name": "found", "status": "DRAFT"}]), + ) + body = _client().get("/api/registry/aws-search?q=bot").json() + assert body["results"][0]["status"] == "DRAFT", "served the stale index status" + assert body["status_authoritative"] is True + + +def test_search_reports_a_deleted_record_as_deleted_not_as_its_last_status(monkeypatch): + """A hit the control plane no longer has was deleted but not yet de-indexed.""" + monkeypatch.setattr(ar, "get_registry", lambda: _FakeReg(True, truth=[])) + body = _client().get("/api/registry/aws-search?q=bot").json() + assert body["results"][0]["status"] == "DELETED" + + +def test_search_drops_status_when_it_cannot_be_reconciled(monkeypatch): + """If the control plane can't be read we have no trustworthy status. Dropping the + field is honest; passing the index's version through would be misleading and + failing the whole request would break browse over a cosmetic concern.""" + monkeypatch.setattr(ar, "get_registry", lambda: _FakeReg(True, truth=_RAISES)) + body = _client().get("/api/registry/aws-search?q=bot").json() + assert body["enabled"] is True + assert body["status_authoritative"] is False + assert "status" not in body["results"][0] + assert body["results"][0]["name"] == "found", "results must still be served" diff --git a/backend/tests/test_gating_unknown_status.py b/backend/tests/test_gating_unknown_status.py new file mode 100644 index 0000000..edbaaf2 --- /dev/null +++ b/backend/tests/test_gating_unknown_status.py @@ -0,0 +1,141 @@ +"""A failed registry query must surface as 503 (unknown), never 403 (denied). + +The distinction is operational, not cosmetic. Integration gating is fail-closed, +and `list_records()` used to return [] on AccessDenied — so a single wrong +`agent-registry:` IAM action name would block every deploy that wired an external +MCP/A2A integration, while telling the operator their integrations were "not +APPROVED". They would go audit approval records; the actual fault was an IAM +policy. These tests pin the honest failure mode. +""" + +from __future__ import annotations + +import sys + +import pytest +from fastapi import HTTPException + +sys.path.insert(0, "src") + +from app import deployment_handler as dh # noqa: E402 +from app.services import aws_agent_registry as ar # noqa: E402 + + +class _Req: + """Minimal stand-in for DeployRequest's gating-relevant surface.""" + + def __init__(self, mcp=None, a2a=None): + self.mcp_server_config = mcp or {} + self.a2a_config = a2a or {} + self.config = None + + +def _gate(monkeypatch, raiser): + """Exercise only the gating block, with the registry lookup replaced.""" + monkeypatch.setattr(ar, "unapproved_integrations", raiser) + + +def test_registry_query_failure_is_503_not_403(monkeypatch): + def _boom(_idents): + raise ar.RegistryQueryFailed("AccessDenied: agent-registry:ListRegistryRecords") + + _gate(monkeypatch, _boom) + + with pytest.raises(HTTPException) as ei: + _run_gating(_Req(mcp={"endpoint": "https://mcp.example/mcp"})) + + assert ei.value.status_code == 503, "unknown approval status must not read as denied" + detail = str(ei.value.detail) + assert "unknown" in detail.lower() + assert "ListRegistryRecords" in detail, "the error must name the permission to fix" + assert "not APPROVED" not in detail, "must not blame the customer's integrations" + + +def test_genuine_denial_is_still_403(monkeypatch): + _gate(monkeypatch, lambda idents: list(idents)) + + with pytest.raises(HTTPException) as ei: + _run_gating(_Req(mcp={"endpoint": "https://mcp.example/mcp"})) + + assert ei.value.status_code == 403 + assert "not APPROVED" in str(ei.value.detail) + + +def test_approved_integration_passes_gating(monkeypatch): + _gate(monkeypatch, lambda _idents: []) + _run_gating(_Req(mcp={"endpoint": "https://mcp.example/mcp"})) # no raise + + +def test_federation_off_is_a_noop(monkeypatch): + """No identifiers collected → gating never consults the registry at all.""" + + def _must_not_be_called(_idents): + raise AssertionError("gating queried the registry with no integrations") + + _gate(monkeypatch, _must_not_be_called) + _run_gating(_Req()) # no mcp/a2a config → nothing to gate + + +def _run_gating(request): + """Replicates handle_deploy's gating block against the live module symbols. + + handle_deploy() itself needs DynamoDB, Step Functions and auth; this exercises + the branch under test using the same imports and control flow, so the 503/403 + mapping is verified rather than assumed. + """ + from app.services.aws_agent_registry import ( + RegistryQueryFailed, + unapproved_integrations, + ) + + idents: list[str] = [] + mcp = request.mcp_server_config or {} + if isinstance(mcp, dict): + for k in ("endpoint", "url", "name", "server_url", "serverUrl"): + if mcp.get(k): + idents.append(str(mcp[k])) + a2a = request.a2a_config or {} + if isinstance(a2a, dict): + for u in a2a.get("peer_allowlist") or a2a.get("peerAllowlist") or []: + idents.append(str(u)) + + if not idents: + return + + try: + blocked = unapproved_integrations(idents) + except RegistryQueryFailed as rqe: + raise HTTPException( + status_code=503, + detail=( + "Integration gating is enabled but the Agent Registry could not be " + f"queried, so approval status is unknown ({rqe}). Refusing the deploy " + "rather than let an unreviewed integration through. Check that the " + "deployment role holds agent-registry:ListRegistryRecords and that the " + "configured registry id is correct." + ), + ) from rqe + if blocked: + raise HTTPException( + status_code=403, + detail=( + "These integrations are not APPROVED in the Agent Registry and " + f"cannot be used in a deployment: {blocked}" + ), + ) + + +def test_handler_source_matches_this_replica(): + """Guard against the replica above drifting from handle_deploy(). + + A copy of production control flow is only worth testing if it stays a copy. + """ + import inspect + + src = inspect.getsource(dh.handle_deploy) + assert "except RegistryQueryFailed" in src + assert "status_code=503" in src + assert "agent-registry:ListRegistryRecords" in src + assert "approval status is unknown" in src + # the 403 denial branch must still exist alongside it + assert "not APPROVED in the Agent Registry" in src diff --git a/backend/tests/test_integration_gating.py b/backend/tests/test_integration_gating.py index 556cddd..626d762 100644 --- a/backend/tests/test_integration_gating.py +++ b/backend/tests/test_integration_gating.py @@ -14,13 +14,33 @@ class _FakeRegistry: + """Stands in for AwsAgentRegistry, recording the filters it was asked for. + + Exposes list_records_strict() because that is what gating must call: a + fail-closed check may not accept an empty list as a verdict. The fake ignores + `filters` when returning records so the caller's own APPROVED re-check stays + under test (a control plane that ignored `filters` must not widen the gate). + """ + def __init__(self, records): self._records = records + self.filters_seen: list | None = None - def list_records(self): + def list_records_strict(self, filters=None): + self.filters_seen = filters return self._records +class _BrokenRegistry: + """A registry that cannot answer — AccessDenied, throttle, bad filter shape.""" + + def __init__(self, message="AccessDenied: agent-registry:ListRegistryRecords"): + self._message = message + + def list_records_strict(self, filters=None): + raise reg.RegistryQueryFailed(self._message) + + def _patch(monkeypatch, registry): monkeypatch.setattr(reg, "get_registry", lambda: registry) @@ -91,3 +111,107 @@ def test_mixed(monkeypatch): ) blocked = reg.unapproved_integrations(["ok-mcp", "pending-mcp", "ghost-mcp"]) assert set(blocked) == {"pending-mcp", "ghost-mcp"} + + +# -- GA control-plane filter shape ------------------------------------------- + + +def test_gating_asks_the_control_plane_to_filter_approved(monkeypatch): + """The APPROVED narrowing is pushed server-side using the GA filters shape. + + GA's ListRegistryRecords takes filters=[{"name": ..., "values": [...]}] where + name ∈ {name, status, recordType}. Sending the old flat kwargs (or nothing) + means paging the whole catalog client-side. + """ + fake = _FakeRegistry([{"name": "ok-mcp", "status": "APPROVED"}]) + _patch(monkeypatch, fake) + reg.unapproved_integrations(["ok-mcp"]) + assert fake.filters_seen == [{"name": "status", "values": ["APPROVED"]}] + + +def test_approved_by_display_name_passes(monkeypatch): + """GA records carry displayName alongside name; either may match.""" + _patch( + monkeypatch, + _FakeRegistry([{"name": "notion_mcp", "displayName": "notion-mcp", "status": "APPROVED"}]), + ) + assert reg.unapproved_integrations(["notion-mcp"]) == [] + + +# -- absent data is not negative data ---------------------------------------- + + +def test_registry_failure_raises_instead_of_reporting_everything_unapproved(monkeypatch): + """The defect this guards: a wrong IAM action name made list_records() return + [], which made every integration look UNAPPROVED, which produced a 403 telling + the operator their integrations were rejected. The real cause was AccessDenied. + Gating must surface "unknown", never silently convert it into "denied".""" + _patch(monkeypatch, _BrokenRegistry()) + try: + reg.unapproved_integrations(["https://mcp.notion.com/mcp"]) + except reg.RegistryQueryFailed as e: + assert "ListRegistryRecords" in str(e) + else: + raise AssertionError("a failed registry query must not yield a silent verdict") + + +def test_genuinely_empty_registry_still_fails_closed(monkeypatch): + """The counterpart: a registry that answers and holds nothing IS a verdict. + Fail-closed must survive the fix — this is not a licence to fail open.""" + _patch(monkeypatch, _FakeRegistry([])) + assert reg.unapproved_integrations(["https://mcp.notion.com/mcp"]) == ["https://mcp.notion.com/mcp"] + + +def test_lenient_list_records_still_degrades_for_display_surfaces(): + """Read-only inventory views keep the forgiving behavior: a short list there is + cosmetic, not a policy decision.""" + a = reg.AwsAgentRegistry.__new__(reg.AwsAgentRegistry) + a.registry_id = "reg1" + a.region = "us-east-1" + a.control = None + a.data = None + assert a.list_records() == [] + + +def test_strict_list_records_raises_when_client_is_missing(): + """An old boto3 bundle has no agent-registry models. For display that's []; + for gating it must be an exception, or federation would silently stop gating.""" + a = reg.AwsAgentRegistry.__new__(reg.AwsAgentRegistry) + a.registry_id = "reg1" + a.region = "us-east-1" + a.control = None + a.data = None + try: + a.list_records_strict() + except reg.RegistryQueryFailed as e: + assert "boto3" in str(e) + else: + raise AssertionError("missing client must raise, not return []") + + +def test_strict_list_records_carries_partial_pages_on_failure(): + """Pages already read are preserved on the exception for diagnostics, without + being mistaken for a complete answer.""" + + class _HalfBroken: + def __init__(self): + self.n = 0 + + def list_registry_records(self, **kw): + self.n += 1 + if self.n == 1: + return {"registryRecords": [{"name": "one"}], "nextToken": "t1"} + raise RuntimeError("Throttling") + + a = reg.AwsAgentRegistry.__new__(reg.AwsAgentRegistry) + a.registry_id = "reg1" + a.region = "us-east-1" + a.control = _HalfBroken() + a.data = None + try: + a.list_records_strict() + except reg.RegistryQueryFailed as e: + assert [r["name"] for r in e.partial] == ["one"] + assert "Throttling" in str(e) + else: + raise AssertionError("a mid-pagination failure must raise") diff --git a/docs/API_REFERENCE.md b/docs/API_REFERENCE.md index d3a5efb..0353833 100644 --- a/docs/API_REFERENCE.md +++ b/docs/API_REFERENCE.md @@ -85,6 +85,24 @@ Every API endpoint the platform exposes, plus deploy-time configuration variable | `POST` | `/api/registry/{slug}/approve` | **Admin only** — approve a pending entry (403 otherwise) | | `POST` | `/api/registry/{slug}/reject` | **Admin only** — reject with optional reason (403 otherwise) | +#### AWS Agent Registry federation (opt-in) + +Federates deployed agents into the **AWS Agent Registry** — a GA AWS service in +its own right (it is no longer part of `bedrock-agentcore`). Requires the backend +to run **boto3 >= 1.43.66**, the first release carrying the `agent-registry` +service models, and the `agent-registry:*` IAM actions. + +| Method | Endpoint | Description | +|--------|----------|-------------| +| `GET` | `/api/registry/aws-config` | Federation status: `{enabled, registry_id, available, sdk_supported}` | +| `POST` | `/api/registry/aws-config` | **Admin only** — enable federation with a `registry_id` (reachability validated before persisting) | +| `GET` | `/api/registry/aws-search?q=` | Discovery search across the registry (`SearchDiscoverableRegistryRecords`) | + +`sdk_supported: false` means this deployment's boto3 predates the GA API, so no +`agent-registry` client can be built — a redeploy, not a configuration change. +`POST` returns `400` naming the SDK in that case rather than blaming the +`registry_id`. + ### Prompt Library | Method | Endpoint | Description | diff --git a/docs/DEPLOYMENT_INTERNALS.md b/docs/DEPLOYMENT_INTERNALS.md index c8a9782..fe34bb8 100644 --- a/docs/DEPLOYMENT_INTERNALS.md +++ b/docs/DEPLOYMENT_INTERNALS.md @@ -342,16 +342,14 @@ Each tool has an MCP-compliant schema registered in `GATEWAY_TOOL_SCHEMAS`: ```python GATEWAY_TOOL_SCHEMAS = { - 'duckduckgo_search': { - 'name': 'duckduckgo_search', - 'description': 'Search the web using DuckDuckGo...', - 'inputSchema': { - 'type': 'object', - 'properties': { - 'query': {'type': 'string', 'description': 'The search query'} - }, - 'required': ['query'] - } + "duckduckgo_search": { + "name": "duckduckgo_search", + "description": "Search the web using DuckDuckGo...", + "inputSchema": { + "type": "object", + "properties": {"query": {"type": "string", "description": "The search query"}}, + "required": ["query"], + }, }, # ... wikipedia_search, weather_api, web_page_fetcher } diff --git a/docs/ENTERPRISE_CAPABILITIES.md b/docs/ENTERPRISE_CAPABILITIES.md index 4b57584..64bd81a 100644 --- a/docs/ENTERPRISE_CAPABILITIES.md +++ b/docs/ENTERPRISE_CAPABILITIES.md @@ -44,7 +44,7 @@ An enterprise governance layer modeled on [awslabs/loom](https://github.com/awsl - **VPC-egress runtimes & named profiles** -- `/api/settings/vpc-profiles` define reusable `{subnet_ids, security_group_ids}` bundles; a deploy referencing one by name (`vpcProfile`) threads `networkMode=VPC` into the runtime so it runs in customer private subnets. Unknown profile → 400 at deploy. Optional PrivateLink ingress IaC (NLB + VPCEndpointService + SG) ships as a downloadable add-on ([`privatelink-ingress.yaml`](privatelink-ingress.yaml)). - **Import existing runtime by ARN** -- `POST /api/runtime/import` adopts an externally-built AgentCore Runtime as a caller-owned deployment (no codegen/deploy) so pre-existing runtimes join the platform's version/slot/cost/observability surfaces. - **Integration gating** -- When AWS Agent Registry federation is enabled, a deploy referencing an MCP/A2A integration is rejected unless each referenced integration is `APPROVED` in the registry (no-op when federation is off). -- **AWS Agent Registry federation** -- Opt-in federation of deployed agents into the org-wide AWS-native Agent Registry (`CreateRegistryRecord` → submit → approve → search), auto-registered on deploy and removed on teardown. +- **AWS Agent Registry federation** -- Opt-in federation of deployed agents into the org-wide AWS-native Agent Registry (`CreateRegistryRecord` → `SubmitRegistryRecordForApproval` → approve → `SearchDiscoverableRegistryRecords`), auto-registered on deploy and removed on teardown. Targets the **GA** API: Agent Registry is its own AWS service (`agent-registry-control` / `agent-registry` clients, `agent-registry:*` IAM actions, `recordType` ∈ `MCP|AGENT|CUSTOM|SKILL`), so the backend requires **boto3 >= 1.43.66**. `GET /api/registry/aws-config` reports `sdk_supported: false` on an older bundle rather than silently degrading. - **Cost budgets + scheduled FinOps reconciliation** -- `/api/cost/budgets` set per owner/agent/tag monthly limits (warn + hard thresholds) evaluated against the same `gen_ai.usage` pipeline as the cost dashboard. A daily EventBridge sweep (`cost_reconcile_step`) walks every budget, sums month-to-date actual spend, and emits a `BudgetBreach` CloudWatch metric for any warn/over — so an idle-but-overspending agent trips an alarm even when nobody opens the dashboard. - **Live model catalog** -- `GET /api/models` discovers text models live from Bedrock (`list_inference_profiles` + `list_foundation_models`, filtered to TEXT/ACTIVE/ON_DEMAND) merged with a curated friendly-label overlay, replacing the hardcoded picker; falls back to a static list if Bedrock is unreachable. - **Rich admin analytics** -- `GET /api/admin/audit` (admin scope) rolls up audited writes into `by_action`/`by_actor` plus `distinct_actors`, `distinct_sessions`, and a chart-ready `by_day` time-series rendered as summary tiles + a dependency-free activity chart. diff --git a/docs/MCP_CATALOG.md b/docs/MCP_CATALOG.md index 12f626a..647bea2 100644 --- a/docs/MCP_CATALOG.md +++ b/docs/MCP_CATALOG.md @@ -98,17 +98,23 @@ Airtable, Zoom, Canva**. from app.services.gateway_deployer import deploy_external_mcp_target from app.services.mcp_catalog import get_mcp_server -entry = get_mcp_server("aws-knowledge") # Tier 1, no creds +entry = get_mcp_server("aws-knowledge") # Tier 1, no creds deploy_external_mcp_target(agentcore_ctrl, gateway_id=gid, catalog_entry=entry) -entry = get_mcp_server("exa") # Tier 2, static key -deploy_external_mcp_target(agentcore_ctrl, gateway_id=gid, catalog_entry=entry, - secret_arn="") - -entry = get_mcp_server("databricks") # Tier 3, machine OAuth -deploy_external_mcp_target(agentcore_ctrl, gateway_id=gid, catalog_entry=entry, - endpoint="https://myws.cloud.databricks.com/api/2.0/mcp/sql", - oauth_provider_arn="", oauth_scopes=["sql"]) +entry = get_mcp_server("exa") # Tier 2, static key +deploy_external_mcp_target( + agentcore_ctrl, gateway_id=gid, catalog_entry=entry, secret_arn="" +) + +entry = get_mcp_server("databricks") # Tier 3, machine OAuth +deploy_external_mcp_target( + agentcore_ctrl, + gateway_id=gid, + catalog_entry=entry, + endpoint="https://myws.cloud.databricks.com/api/2.0/mcp/sql", + oauth_provider_arn="", + oauth_scopes=["sql"], +) ``` ## Live verification diff --git a/frontend/src/components/modals/AwsRegistryPanel.tsx b/frontend/src/components/modals/AwsRegistryPanel.tsx index b52c0e9..f70a8c3 100644 --- a/frontend/src/components/modals/AwsRegistryPanel.tsx +++ b/frontend/src/components/modals/AwsRegistryPanel.tsx @@ -4,7 +4,18 @@ * Opt-in: an admin enters an AWS registryId to federate deployed agents into * the org-wide AWS-native Agent Registry (with the AWS approval workflow). Also * offers semantic search across the registry. Degrades to a disabled state when - * the feature is unconfigured or the (public-preview) API is unavailable. + * the feature is unconfigured or the API is unavailable. + * + * Agent Registry is GA and is its own AWS service (no longer part of + * bedrock-agentcore). `sdk_supported: false` from /aws-config means the backend + * bundle's boto3 predates the GA models — a different fix from a bad registryId, + * so the two states are labelled differently below. + * + * Three distinct reasons federation can be unavailable, three distinct fixes, so + * three distinct labels: the SDK is too old (redeploy), the registry could not be + * read at all (registryId / IAM / region), or the registry is real but not yet + * READY (wait). Collapsing the last into "unreachable" sends an admin to re-check + * a registryId that was never wrong. */ import { useCallback, useEffect, useState } from 'react'; @@ -13,6 +24,11 @@ import { getApiClient, getErrorMessage } from '../../services/api'; export function AwsRegistryPanel() { const [enabled, setEnabled] = useState(false); const [available, setAvailable] = useState(false); + const [sdkSupported, setSdkSupported] = useState(true); + const [status, setStatus] = useState(null); + // False when the backend could not reconcile hit statuses against the control + // plane and therefore omitted them — distinguishes "no badge" from "no status". + const [statusAuthoritative, setStatusAuthoritative] = useState(true); const [registryId, setRegistryId] = useState(''); const [input, setInput] = useState(''); const [query, setQuery] = useState(''); @@ -25,6 +41,11 @@ export function AwsRegistryPanel() { const cfg = await getApiClient().getAwsRegistryConfig(); setEnabled(cfg.enabled); setAvailable(cfg.available); + // Older backends don't return this field; assume supported so we don't + // show a spurious "SDK too old" warning against them. + setSdkSupported(cfg.sdk_supported !== false); + // null/absent = the registry could not be read; a string = it answered. + setStatus(cfg.status ?? null); setRegistryId(cfg.registry_id ?? ''); } catch { /* feature optional — leave disabled */ @@ -50,6 +71,7 @@ export function AwsRegistryPanel() { try { const r = await getApiClient().searchAwsRegistry(query.trim()); setResults(r.results ?? []); + setStatusAuthoritative(r.status_authoritative !== false); } catch (e) { setError(getErrorMessage(e)); } finally { @@ -63,24 +85,41 @@ export function AwsRegistryPanel() { AWS Agent Registry - {enabled && available ? 'Connected' : enabled ? 'Configured (unreachable)' : 'Not configured'} + {enabled && available ? 'Connected' + : !sdkSupported ? 'SDK out of date' + : enabled && status ? `Configured (${status})` + : enabled ? 'Configured (unreachable)' : 'Not configured'} + {!sdkSupported && ( +
+ This deployment's AWS SDK predates the GA Agent Registry API. Redeploy the + backend with boto3 ≥ 1.43.66 to enable federation. +
+ )} + + {sdkSupported && enabled && !available && status && ( +
+ The registry exists but its status is {status}, so it cannot accept records + yet. Nothing to fix — it becomes available once AWS finishes provisioning. +
+ )} + {error &&
{error}
} {!enabled ? (
setInput(e.target.value)} />
+ {results.length > 0 && !statusAuthoritative && ( +
+ Approval status omitted: it could not be confirmed against the registry's + control plane, and the search index's copy goes stale after a redeploy. +
+ )} {results.length > 0 && (
{results.map((r, i) => ( -
- {String(r.name ?? r.recordArn ?? JSON.stringify(r))} +
+ + {String(r.displayName ?? r.name ?? r.recordArn ?? JSON.stringify(r))} + + {/* recordType/status are GA response fields; preview returned neither. + `status` here is the backend's control-plane-reconciled value, not + the search index's — DELETED means de-indexing hasn't caught up. */} + {r.recordType != null && ( + + {String(r.recordType)} + + )} + {r.status != null && ( + + {String(r.status)} + + )}
))}
diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 652be74..6c86c16 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -431,7 +431,12 @@ export class ApiClient { } // AWS Registry (Phase 6) - async getAwsRegistryConfig(): Promise<{ enabled: boolean; registry_id: string | null; available: boolean }> { + /** `sdk_supported` is optional: an older backend omits it. False means that + * backend's boto3 lacks the GA `agent-registry` service models (a redeploy, + * not a config fix). `status` is the registry's lifecycle state, null if it + * could not be read — a non-READY registry is valid but not yet writable. + * Mirrors the declaration in services/api/registry.ts; keep both in step. */ + async getAwsRegistryConfig(): Promise<{ enabled: boolean; registry_id: string | null; available: boolean; sdk_supported?: boolean; status?: string | null }> { return apiRequest(`/api/registry/aws-config`, {}, this.baseUrl); } @@ -439,7 +444,10 @@ export class ApiClient { return apiRequest(`/api/registry/aws-config`, { method: 'POST', body: JSON.stringify({ registry_id: registryId }) }, this.baseUrl); } - async searchAwsRegistry(q: string): Promise<{ enabled: boolean; results: Array> }> { + // `status_authoritative: false` means each hit's `status` was dropped because the + // control plane couldn't be reached — the data plane's own status is stale after a + // redeploy, so it is not served as a fallback. Mirrored in ./api/registry.ts. + async searchAwsRegistry(q: string): Promise<{ enabled: boolean; results: Array>; status_authoritative?: boolean }> { return apiRequest(`/api/registry/aws-search?q=${encodeURIComponent(q)}`, {}, this.baseUrl); } diff --git a/frontend/src/services/api/registry.ts b/frontend/src/services/api/registry.ts index 2341c38..135c0f2 100644 --- a/frontend/src/services/api/registry.ts +++ b/frontend/src/services/api/registry.ts @@ -141,8 +141,28 @@ export async function rejectRegistry( // AWS Agent Registry Federation (Phase 6) // ============================================================================ -/** Phase 6 (Loom) — AWS Agent Registry federation config/status. */ -export async function getAwsRegistryConfig(): Promise<{ enabled: boolean; registry_id: string | null; available: boolean }> { +/** + * Phase 6 (Loom) — AWS Agent Registry federation config/status. + * + * `sdk_supported` is optional because an older backend won't return it: false + * means the backend's boto3 lacks the GA `agent-registry` service models, which + * is a redeploy, not a config fix. + * + * `status` is the registry's own lifecycle state (READY / CREATING / UPDATING / + * DELETING / *_FAILED), or null when it could not be read. A registry that is + * not READY is unavailable but perfectly valid — it just needs another moment — + * so the UI must not report it as a bad registryId. + * + * NOTE: `ApiClient.getAwsRegistryConfig()` in ../api.ts declares this same shape + * independently. Keep the two in step; only `tsc -b` catches a divergence. + */ +export async function getAwsRegistryConfig(): Promise<{ + enabled: boolean; + registry_id: string | null; + available: boolean; + sdk_supported?: boolean; + status?: string | null; +}> { return apiRequest(`/api/registry/aws-config`); } @@ -154,7 +174,14 @@ export async function enableAwsRegistry(registryId: string): Promise<{ enabled: }); } -/** Phase 6 — semantic search across the AWS Agent Registry. */ -export async function searchAwsRegistry(q: string): Promise<{ enabled: boolean; results: Array> }> { +/** Phase 6 — semantic search across the AWS Agent Registry. + * + * Each hit's `status` is reconciled against the control plane, because the search + * index keeps serving a redeployed record as APPROVED after it has been demoted to + * DRAFT. `status_authoritative: false` means that reconciliation failed and `status` + * was omitted rather than served stale. Declared independently in ../api.ts — only + * `tsc -b` catches the two drifting apart. + */ +export async function searchAwsRegistry(q: string): Promise<{ enabled: boolean; results: Array>; status_authoritative?: boolean }> { return apiRequest(`/api/registry/aws-search?q=${encodeURIComponent(q)}`); } diff --git a/infra/stacks/platform/lambdas.py b/infra/stacks/platform/lambdas.py index 4182777..8eb92fa 100644 --- a/infra/stacks/platform/lambdas.py +++ b/infra/stacks/platform/lambdas.py @@ -523,20 +523,9 @@ def build_deployment_lambda( "bedrock-agentcore:GetPolicyEngine", "bedrock-agentcore:DeletePolicyEngine", "bedrock-agentcore:ListPolicyEngines", - # Phase 6 (Loom) — AWS Agent Registry federation (opt-in). - # The registry router publishes/approves/searches records in - # the org-wide AWS-native catalog. Public preview; feature is - # off unless an admin configures a registryId. - "bedrock-agentcore:CreateRegistry", - "bedrock-agentcore:GetRegistry", - "bedrock-agentcore:CreateRegistryRecord", - "bedrock-agentcore:GetRegistryRecord", - "bedrock-agentcore:ListRegistryRecords", - "bedrock-agentcore:SubmitRegistryRecordForApproval", - "bedrock-agentcore:UpdateRegistryRecordStatus", - "bedrock-agentcore:UpdateRegistryRecord", - "bedrock-agentcore:DeleteRegistryRecord", - "bedrock-agentcore:SearchRegistryRecords", + # NOTE: Agent Registry actions are NOT here — at GA the Registry + # became its own AWS service with an `agent-registry:` action + # prefix. See the dedicated statement below. "bedrock-agentcore:CreatePolicy", "bedrock-agentcore:DeletePolicy", # UpdatePolicy: the lazy promoter recovers a CREATE_FAILED @@ -592,6 +581,51 @@ def build_deployment_lambda( resources=["*"], ) ) + # Phase 6 (Loom) — AWS Agent Registry federation (opt-in). + # SEPARATE STATEMENT, DIFFERENT ACTION PREFIX. Agent Registry graduated out + # of AgentCore into its own AWS service at GA: the boto3 clients are + # `agent-registry-control` / `agent-registry`, and BOTH planes authorize + # under a single `agent-registry:` prefix (the control-plane model's + # signingName is `agent-registry`, not `agent-registry-control`). Leaving + # these as `bedrock-agentcore:*Registry*` silently AccessDenies every + # federation call — and because the deploy-path auto-register is + # best-effort, the denial shows up only as a skipped log line. + # The data-plane search was also RENAMED: SearchRegistryRecords -> + # SearchDiscoverableRegistryRecords. + role.add_to_policy( + iam.PolicyStatement( + actions=[ + # control plane (agent-registry-control) + "agent-registry:CreateRegistry", + "agent-registry:GetRegistry", + "agent-registry:UpdateRegistry", + "agent-registry:DeleteRegistry", + "agent-registry:ListRegistries", + "agent-registry:CreateRegistryRecord", + "agent-registry:GetRegistryRecord", + "agent-registry:ListRegistryRecords", + "agent-registry:UpdateRegistryRecord", + "agent-registry:UpdateRegistryRecordStatus", + "agent-registry:SubmitRegistryRecordForApproval", + "agent-registry:DeleteRegistryRecord", + # data plane (agent-registry) — discovery/search over APPROVED + # records. Note the GA operation names. + "agent-registry:SearchDiscoverableRegistryRecords", + "agent-registry:ListDiscoverableRegistryRecords", + "agent-registry:BatchGetDiscoverableRegistryRecord", + # tagging: registries/records created by the platform are tagged + # for cost attribution and teardown discovery. + "agent-registry:TagResource", + "agent-registry:UntagResource", + "agent-registry:ListTagsForResource", + ], + # The registryId is supplied by an admin at runtime (opt-in feature), + # so the registry/record ARNs are unknowable at synth time. Scoped by + # the exact action list instead of the resource, matching the + # AgentCore statement above. + resources=["*"], + ) + ) # Phase 1 Gap 1C — CloudWatch Logs Insights query for evaluator scores. # M-2: also delete the eval-results log groups on destroy_runtime. role.add_to_policy( diff --git a/infra/stacks/platform/step_lambdas.py b/infra/stacks/platform/step_lambdas.py index b57cace..e10daee 100644 --- a/infra/stacks/platform/step_lambdas.py +++ b/infra/stacks/platform/step_lambdas.py @@ -743,6 +743,44 @@ def _create_step_role( ) ) + # Phase 6 (Loom) — AWS Agent Registry federation (opt-in). The status_update + # step auto-registers a just-deployed agent as a DRAFT record + # (_auto_register_in_aws_registry), so THIS role — not the deployment + # Lambda's — is the principal on CreateRegistryRecord. + # + # This grant was missing entirely, which the best-effort wrapper around the + # auto-register hid: every deploy logged "auto-register skipped" and the + # federation feature silently never produced a record. + # + # Action prefix is `agent-registry:`, NOT `bedrock-agentcore:` — at GA the + # Registry became its own AWS service (boto3 `agent-registry-control` / + # `agent-registry`), and both planes authorize under the control model's + # signingName, which is `agent-registry`. Hence a separate statement rather + # than an entry in the agentcore_steps map above. + if step_name == "status_update": + role.add_to_policy( + iam.PolicyStatement( + actions=[ + "agent-registry:GetRegistry", + "agent-registry:CreateRegistryRecord", + "agent-registry:GetRegistryRecord", + "agent-registry:ListRegistryRecords", + # Redeploying an agent hits the name+recordVersion uniqueness + # key, so register() falls back to updating the existing record + # in place (see AwsAgentRegistry.register). Without this action + # that fallback AccessDenies and the best-effort wrapper hides + # it, leaving the record pinned to the FIRST deployment's + # runtime ARN — stale, and silently so. + "agent-registry:UpdateRegistryRecord", + "agent-registry:DeleteRegistryRecord", + "agent-registry:TagResource", + ], + # The registryId is configured by an admin at runtime (opt-in), + # so registry/record ARNs are unknowable at synth time. + resources=["*"], + ) + ) + # Bug 196 — auto-cleanup on failure. When a deployment fails, the # status_update step iterates created_resources and deletes them to # prevent orphans (KB, Cognito pools, gateways, IAM roles, Lambdas,