feat(registry): agent asset catalog on AgentCore — provisioning, port/adapter, API, CLI (#246)

[Kalindi-Dev]

Summary

Read-side catalog for the central agent asset registry (#246, ADR-018 #548) built on AWS Agent Registry (Bedrock AgentCore) — the chosen substrate. Nothing upstream imports the AWS SDK directly; the AgentCore control plane sits behind a RegistryClient port.

• Provisioning: AgentRegistryStack (NestedStack) creates the registry via a custom resource (async CreateRegistry, no L2 in preview); bootstrap IAM + resource-action-map + golden DEPLOYMENT_ROLES.md in sync. Nested to keep the root stack under CloudFormation's 500-resource limit.
• Ports & adapters: RegistryClient port (TS + Py), one AgentCoreRegistryClient adapter per language. Native descriptors — MCP server.json + _meta, AGENT_SKILLS markdown frontmatter, CUSTOM verbatim.
• Grammar: registry://kind/namespace/name@constraint with mandatory semver pin, mirrored byte-for-byte across ref.ts/ref.py and enforced by the contracts/registry-resolution/ parity corpus.
• API: publish/resolve/list/show on TaskApi, gated by Cognito groups (RegistryPublisher/RegistryApprover); bgagent registry CLI.
• Wire types: resolved-asset triple stamped on TaskRecord/Detail/Summary for audit.

Integration (orchestrator resolve-step, agent loaders, blueprint pins) lands in the follow-up PR that builds on this catalog.

This is the AgentCore implementation of #246. The DDB+S3 PRs (#632/#633/#634) are the earlier approach and have been moved to draft.

Test plan

• mise run build green (2583 CDK tests + agent quality)
• TS↔Py resolution parity corpus passes
• Deploy to dev; confirm registry reaches READY and publish/resolve/list/show work end-to-end

[Kalindi-Dev]

Self-review (principal-architect pass)

Ran a /review_pr-style self-review, then a second verification pass after fixing. Findings and resolutions:

Fixed in this branch (commit d3e2b056):

1. TS↔Py grammar parity gap (near-blocking). Python's ref/semver regexes anchored with $, which also matches just before a trailing newline — so registry://…@1.0.0\n parsed in Python but was rejected by the JS mirror (ref.ts, no m flag). A byte-for-byte parity break in exactly the class of bug this repo has been burned by. Fix: switched Python anchors to \Z (absolute end-of-string) in ref.py + resolver.py, and added a trailing-newline-rejected case to contracts/registry-resolution/cases.json so both parity runners guard it permanently. Verified empirically: both sides now reject the trailing-newline ref and still accept valid pins (Python corpus 20/20, TS parity 20/20).

2. Bootstrap version not bumped. The policy surface changed in this PR (Cognito group, Step Functions, CloudFormation nested-stack ARN) but BOOTSTRAP_VERSION stayed 1.2.0. Fix: bumped → 1.3.0 and regenerated the bootstrap template so operators know their deploy role is stale. (BOOTSTRAP_HASH correctly unchanged — the version string isn't part of the policy hash.)

Open nits (non-blocking, fast-follow): no-op try/catch in agentcore-client.ts (~L180); double parseConstraint call in registry-publish.ts; O(n) resolve/list (List + N×Get) — acceptable for MVP given the GA-throwaway construct.

Verified clean: bootstrap synth-coverage (91/91) + golden-baseline, CDK↔CLI types sync, IAM least-privilege + fail-closed auth (401/403/422), docs mirror idempotent. All CI checks green.

[scottschreckengaust]

Verdict: Approve (with minor nits)

A large, careful, well-scoped read-side registry catalog on AgentCore. I verified the four critical review foci empirically (tests run locally, not just read): bootstrap policy coverage is complete and the golden baseline + synth-coverage pass; CDK↔CLI shared types are in sync; the registry:// grammar and resolution semantics match byte-for-byte across TS/Py; auth is Cognito-group-gated and resolution fails closed; and IAM is least-privilege on both the custom-resource role and the handler roles. No blocking issues. The open items are the same nits the author already flagged in the self-review.

Vision alignment

Advances #246 / ADR-018 without eroding tenets. Fire-and-forget is untouched (this is a catalog + API/CLI surface; no live pairing). Blast radius is bounded: publish/approve are gated by RegistryPublisher/RegistryApprover groups, resolution is fail-closed (422 with a specific reason on any unresolved/invalid ref), and IAM is scoped to registry/* + registry/*/record/*. Outcomes stay reviewable — the resolved-asset triple is stamped on TaskRecord/Detail/Summary for audit. The L2-vs-custom-resource trade is documented inline (no AgentCore L2 in preview) with a GA-throwaway note and the RegistryClient seam confining the future swap.

Blocking issues

None.

Non-blocking suggestions / nits

1. No-op try/catch in cdk/src/handlers/shared/registry/agentcore-client.ts:180-183 — both catch branches re-throw (if (err instanceof ConflictException) throw err; throw err;), so the whole try/catch is dead. Drop it. (Author already noted this.)
2. Double parseConstraint call in cdk/src/handlers/registry-publish.ts:124 — !parseConstraint(...) || parseConstraint(...)!.op !== 'exact' parses twice; compute once into a local. Cosmetic. (Author noted.)
3. O(n) resolve/list — AgentCoreRegistryClient.listRecords does List + N×Get per call, and resolve/show/getRecord all funnel through it. Acceptable for the MVP given the GA-throwaway construct, but worth a // TODO(GA) marker so it isn't forgotten when the native construct lands.
4. show response is an inline anonymous type on both server (registry-show.ts:68) and client (api-client.ts::registryShow) rather than a named RegistryShowResponse. The two inline shapes match today, but a named type would put it under the types-sync guard like the other envelopes. Coherence nit.
5. SKILL.md runtime round-trip via a single-quoted frontmatter line (agentcore-client.ts::parseSkillRuntime ↔ agentcore_client.py::_SKILL_RUNTIME_RE): a runtime JSON payload containing a ' or newline could mis-parse. Shared limitation on both sides (not a parity break), but consider base64-encoding the x-abca-runtime value to make it robust.

Documentation

Good. docs/design/REGISTRY.md (new) documents the grammar, storage modes, and group gating; docs/design/DEPLOYMENT_ROLES.md golden baseline updated to match the new bootstrap statements; contracts/registry-resolution/README.md documents the parity corpus. I regenerated the Starlight mirror locally (node docs/scripts/sync-starlight.mjs) and git status docs/src was clean — no drift. bgagent registry is self-documenting via commander help.

Tests & CI

• CDK: ran registry-resolution-parity, registry-handlers, agentcore-client, registry-resolver, constructs/registry, bootstrap/synth-coverage, bootstrap/golden-baseline, bootstrap/version, bootstrap/policies — 111 passed. Bootstrap synth-coverage explicitly PASSES with the new CFN types (AWS::CloudFormation::Stack, AWS::Cognito::UserPoolGroup, AWS::StepFunctions::StateMachine, Custom::AgentCoreRegistry) all mapped in resource-action-map.ts and covered by the policy bundle. BOOTSTRAP_VERSION bumped 1.2.0→1.3.0, hash + snapshot regenerated.
• Agent: test_registry_resolution_corpus + test_registry_agentcore_client — 24 passed; full workflow/validator suite — 182 passed (the widened _REGISTRY_REF regex is backward-compatible with the legacy 2-segment corpus).
• Types-sync: scripts/check-types-sync.ts — OK, 68 CLI exports validated.
• Failure paths covered: 401 (no auth), 403 (missing group / auto_approve without approver), 422 (bad ref / no matching version), 404 (unknown asset), 409 (version collision). Tests exercise these, not just the happy path.
• GitHub CI: all checks green (build agentcore, CodeQL, secrets/deps scan).
• Test perf: the registry construct test does not re-enable bundling or synth-per-test; parity/resolver tests are pure-function.

Review agents run

The pr-review-toolkit sub-agents and /security-review are not separately dispatchable in this execution context, so I performed the equivalent analysis by hand and state so here:

• code-reviewer (by hand): routing correct per AGENTS.md (agent runtime in agent/, API/Lambdas + bootstrap in cdk/, CLI in cli/); L2 preferred where available, custom resource justified for the preview-only AgentCore control plane.
• silent-failure-hunter (by hand): error handling surfaces failures — resolve/publish fail closed with specific codes; the custom-resource onEvent/isComplete handle ConflictException/ResourceNotFoundException deliberately and re-throw the rest; FAILED registry status throws. One dead no-op try/catch (nit #1). drainRecords best-effort with isComplete retry is sound.
• type-design-analyzer (by hand): new wire types mirrored cdk↔cli and guarded by the sync check; domain types (RegistryRecord, ResolvedAsset, port) intentionally excluded from the CLI contract. ParseResult discriminated union is clean.
• comment-analyzer (by hand): comments accurate; the \Z-vs-$ parity rationale in ref.py is correct and load-bearing.
• pr-test-analyzer (by hand): happy + failure paths covered on both languages; parity corpus enforces cross-language agreement including the trailing-newline-rejected regression case.
• /security-review scope (by hand): IAM least-privilege verified (custom-resource role uses * only for account-level Create/List/WorkloadIdentity with documented rationale + nag suppression; handler roles scoped to registry/*/registry/*/record/*; publish=read+write, resolve/list/show=read-only); Cognito group gating fail-closed; no secrets; no Cedar engine-pin movement in this PR (cedarpy/cedar-wasm untouched — confirmed).

Human heuristics

• Proportionality — Pass. The port/adapter split is justified by the documented GA substrate swap; not an over-abstraction. File sizes are essential, not accreted.
• Coherence — Pass (minor). Same concepts use the same terms across TS/Py; parity corpus enforces real substance, not copy-paste. Only nit: the show response is an inline type on both sides rather than a named shared envelope (#4).
• Clarity — Pass. Names communicate intent; errors surface rather than hide; magic values (poll intervals, group names) are named constants.
• Appropriateness — Pass. Adapter decisions (name-in-name Option A, native-vs-CUSTOM, 3-call publish) are grounded in the live AgentCore spike findings, not self-written mocks; tests assert intended behavior (fail-closed resolution, immutability) not just current behavior.

Governance: #246 is approved P0 (backing-issue gate satisfied); feat/246-registry-catalog is the sanctioned branch name.

[isadeks]

Verdict: Request changes

I re-reviewed head d3e2b056 independently, including the areas already approved. The focused registry baseline is green (112/112 CDK assertions and 39/39 Python tests), but executable probes expose untested contract, security, and CloudFormation failures. The blocking cases and concrete reproductions are inline below; these are separate from the existing no-op catch, double-parse, and O(n) nits.

[Kalindi-Dev]

Thanks @scottschreckengaust and @isadeks — this round addresses every inline finding. Force-pushed after rebasing onto latest main; build (agentcore) is green. Summary of what changed and where:

@isadeks — P1 blocking

1. Submit every publish for approval; gate only APPROVED on autoApprove (agentcore-client.ts:188) — Moved SubmitRegistryRecordForApprovalCommand out of the autoApprove branch. A normal publish now lands in PENDING_APPROVAL; only the final UpdateRegistryRecordStatus(APPROVED) is gated on autoApprove. Added adapter tests asserting the create → submit sequence with autoApprove: false.

2. Enforce the per-kind runtime contract at publish (registry-publish.ts:130) — Replaced the typeof object check with validateRuntime: rejects arrays, requires booleans for custom/auto_approve, and enforces each discriminated payload (transport + url/command invariants for mcp_server, cedar_text, prompt_fragment). Added the full per-kind validation test matrix. Verified live on the dev stack: {"transport":"http"} with no url now returns VALIDATION_ERROR instead of 201.

3. [security] Redact credential-bearing runtime in the resolve response (registry-resolve.ts:62) — Added redactRuntimeForResponse: masks headers values (keys retained as discovery signal) and also stdio command/args (secrets are routinely passed as CLI args). The orchestrator path is unaffected — it reads the unredacted payload via the port, not this handler. Added leak tests for both http-headers and stdio.

4. Encode skill runtime robustly (apostrophe-safe) (agentcore-client.ts:106) — SKILL.md x-abca-runtime frontmatter is now base64-encoded JSON instead of single-quoted, so prompt_fragment: "Don't skip tests" round-trips through native AgentCore descriptor validation. Both TS parseSkillRuntime and Python _extract_runtime decode base64 (with a legacy single-quoted fallback for older records). Added the apostrophe regression test on both sides.

5. Make custom-resource create replay-safe (registry-provisioning/index.ts:71) — CreateRegistry now sends a clientToken derived from the CloudFormation RequestId (falls back to registry name), so an at-least-once retry is a substrate no-op instead of a duplicate registry. Added a test proving same-RequestId → same token, different RequestId → different token.

6. Handle custom-resource Update (registry-provisioning/index.ts:81) — The Update branch now issues UpdateRegistryCommand when registryName/description change (description clear via the optionalValue wrapper), instead of silently reporting success. Added bedrock-agentcore:UpdateRegistry to the registry IAM + bootstrap. Added tests for name-only change, description-clear, and no-op.

7. Persist the authenticated publisher identity (registry-publish.ts:69) — Threaded userId into PublishInput; the adapter now stamps publisher immutably (in _meta for native MCP, frontmatter for skills, the CUSTOM body for cedar) and registry-show round-trips it. Added a test asserting publisher survives for all three descriptor types.

8. Treat async create failure/timeout as publish failure (agentcore-client.ts:293) — Rewrote waitPastCreating: throws on any *_FAILED status (with statusReason), keeps polling through transient not-found/CREATING, and throws on poll-budget exhaustion. A CREATE_FAILED record no longer returns 201. Added failure + timeout tests.

9. [security] Scope handler IAM to the wired registry, not * (task-api.ts:1342, now registry-api.ts) — Handler roles are scoped to registry/{agentRegistryId} + registry/{agentRegistryId}/record/* using the known agentRegistryId. (In this round the registry API also moved into its own nested stack — registry-api.ts — to fit under the 500-resource CloudFormation cap; the scoped ARNs live there now.)

10. [P2] Reject out-of-range semver components (TS/Py parity) (ref.ts:89) — Both parsers now reject any MAJOR.MINOR.PATCH component above MAX_SAFE_INTEGER, so TS can't round 9007199254740993 and diverge from Python. Added the case to the grammar parity corpus.

@scottschreckengaust — nits (all addressed)

1. No-op try/catch (agentcore-client.ts:182) — Removed; the create call throws directly.
2. Double parseConstraint (registry-publish.ts:124) — Computed once into a local.
3. O(n) listRecords (agentcore-client.ts:279) — Added the // TODO(GA) marker so it isn't carried past the native-construct swap.
4. show inline anonymous type — Extracted a named RegistryShowResponse in both types.ts files, now under the types-sync guard.
5. SKILL.md single-quoted frontmatter fragility — Same base64 fix as @isadeks feat: add FargateAgentStack as alternative compute backend #4 above.

Additional hardening from a second internal review pass

• HIGH: _extract_runtime returned {} for a resolvable record with a missing/corrupt runtime — resolve now fails closed (REGISTRY_RESOLUTION_FAILED / REMOVED) rather than handing back an empty runtime, on both TS and Python.
• _meta clobber — nativeDescriptors now merges a caller-supplied discovery._meta instead of overwriting it (test added).
• Partial-publish orphan — a failure after CreateRegistryRecord now raises RegistryPublishIncompleteError (surfaced as 502 REGISTRY_PUBLISH_INCOMPLETE with the stranded recordId) and logs the id, instead of a bare 500.
• Test coverage: new registry-provisioning handler suite (was 0 tests); new resolution-cases.json semver-resolver TS↔Py parity corpus (grammar corpus only covered parsing before).

@isadeks — since these were the items behind your change request, could you re-review when you have a moment?

[scottschreckengaust]

Verdict: Request changes

Eight of isadeks' ten blocking findings are genuinely FIXED and I verified each one against the code at 4f8da982 (not against the commit messages), with regression tests locking most of them in. But the two P1-security findings — the ones the house rule says to weigh hardest — are only partially remediated, and while probing the fix for finding (4) I found a new blocking issue: the SKILL.md builder still raw-interpolates a caller-controlled string into frontmatter, which lets a publisher inject an x-abca-runtime line that wins on read and completely bypasses the new publish-time runtime validation this PR added. That is a validation-bypass on the exact contract finding (2) was raised to establish.

I ran the registry suites locally rather than reading them: 63 CDK registry assertions, 34 parity assertions, 109 bootstrap assertions, 42 agent pytest — all green; tsc --noEmit clean; check-types-sync.ts OK (69 CLI exports); Starlight mirror regenerated with zero drift. The engineering quality here is high and the fix commits are real work, not theater. The remaining items are narrow and mechanical.

Vision alignment

Strong on fire-and-forget default (purely additive read/write catalog; nothing in the orchestrator calls it yet) and on documented tenet trades — the L2-vs-custom-resource trade, the Option-A namespace encoding, and the new separate-RestApi trade are each written down with the reason, in registry-api.ts:20-33 and REGISTRY.md §7. Bounded blast radius improved materially this round: the handler roles moved from * to registry/{id} (finding 10), and the nested-stack split is a legitimate answer to the 500-resource cap rather than a workaround. Where it still falls short of the tenet is reviewable outcomes under a hostile publisher: the resolve response redaction is a denylist and the SKILL.md descriptor is injectable, so "the audit says the pin was honored" is not yet a claim the substrate enforces. New blocker 1 is squarely a bounded-blast-radius regression.

---

Disposition of each prior blocking finding

(1) SubmitRegistryRecordForApproval inside the autoApprove branch — FIXED.
agentcore-client.ts:232-244: the submit call is now unconditional and only UpdateRegistryRecordStatus(APPROVED) is gated on input.autoApprove. The comment at :228-231 states the reason correctly. Regression test present: agentcore-client.test.ts:246 — "publish without autoApprove still submits, landing in PENDING_APPROVAL (not DRAFT)".

(2) runtime validated only by typeof object — FIXED (with a narrow residue, see nit 1).
registry-publish.ts:149-151 adds isPlainObject (rejects arrays), and :160-196 validateRuntime enforces the discriminated contract per kind: transport ∈ {http,sse,stdio}, command required for stdio, url required otherwise, cedar_text non-empty for cedar, prompt_fragment non-empty for skill. I re-ran isadeks' exact probes: runtime: [], runtime: {}, {prompt_fragment:"x"} on an mcp_server, and a CUSTOM array all now return 400, not 201. Regression tests present: registry-handlers.test.ts:168-197, seven cases.

(3) P1-SECURITY credential-bearing runtime returned to every authenticated caller — PARTIALLY FIXED. See new blocking issue 2.
registry-resolve.ts:45-62 adds redactRuntimeForResponse, and I confirmed by probe that headers.Authorization now returns *** with the key retained. That closes isadeks' literal reproduction, and the rationale comment at :30-43 is accurate about the orchestrator path. But the function is a denylist over three field names (headers, command, args) applied to an open Record<string, unknown> that publish does not restrict to known keys — so any other secret-bearing field survives verbatim. Probed at head: runtime: {transport:'http', url:'https://h/mcp?token=SUPERSECRET', api_key:'AKIA-LEAKED', env:{TOKEN:'ghp_leak'}} publishes 201 and resolves 200 with url, api_key, and env.TOKEN all in cleartext to a caller in no registry groups. A token in a URL query string is the single most common MCP auth pattern, so this is not a hypothetical residue.

(4) Skill runtime not encoded as valid YAML — FIXED for the reported case; the fix opened a new hole. See new blocking issue 1.
agentcore-client.ts:117-128 base64-encodes the runtime JSON into x-abca-runtime, and :151-160 / agentcore_client.py:74-81 parse it (both accepting the legacy single-quoted form). I re-ran isadeks' exact case — prompt_fragment: "Don't skip tests" — and js-yaml now parses the generated frontmatter cleanly and the runtime round-trips. TS/Py parity holds. Regression test present: agentcore-client.test.ts:184. The description line on the same frontmatter block was not given the same treatment, which is where the new blocker lives.

(5) Custom-resource create not replay-safe — FIXED.
registry-provisioning/index.ts:80-82 derives a deterministic clientToken via SHA-256 over ${RequestId}:${registryName}, and :93-98 passes it to CreateRegistryCommand. RequestId was added to the modeled event at :49. Regression test present: index.test.ts:96 asserts the token is stable per RequestId and varies across RequestIds — exactly the duplicate-delivery test isadeks asked for.

(6) Update reports success while ignoring desired state — FIXED.
index.ts:104-127 now diffs against OldResourceProperties and issues a real UpdateRegistryCommand with name and/or the description: { optionalValue } wrapper. I confirmed against the installed SDK (models_2.d.ts:684 UpdateRegistryRequest) that both fields are in fact mutable and that UpdatedDescription is a wrapper as the comment at :121 claims. bedrock-agentcore:UpdateRegistry is granted at registry.ts:110, and bootstrap coverage rides on bedrock-agentcore:* in policies/compute-agentcore.ts:34. Regression tests present: index.test.ts:109, :123, :135 (including the no-op case).

(7) Authenticated publisher identity extracted then dropped — FIXED.
registry-publish.ts:76 sets publisher: userId on PublishInput; types.ts:166-168 documents it; the adapter writes it into MCP _meta[dev.abca.publisher] (agentcore-client.ts:498), the SKILL.md x-abca-publisher frontmatter key (:130), and the CUSTOM body (:478), and reads all three back (extractPayload, :446/:457/:469). registry-show.ts:59 now surfaces a real value. I probed the wiring: PublishInput.publisher === 'u1'. Regression test present: agentcore-client.test.ts:224 round-trips the publisher across all three storage shapes.

(8) P2 Number() rounds large semver components / TS↔Py parity — FIXED.
ref.ts:93-95 and resolver.ts:47-49 both reject non-Number.isSafeInteger components; ref.py:46,87 and resolver.py:24,44 use the identical 9007199254740991 bound. Regression tests present in the parity corpus, which is the right place: cases.json:100 component-beyond-max-safe-integer-rejected (isadeks' 9007199254740993.0.0) and resolution-cases.json:71 near-max-safe-integer-patch-comparison. Both corpora run dual-runner and pass on both sides.

(9) Async create failure/timeout treated as success — FIXED, and this is the best-executed fix in the set.
agentcore-client.ts:374-397: *_FAILED throws with the substrate statusReason; ResourceNotFoundException is treated as transient and keeps polling instead of returning; budget exhaustion throws. The new RegistryPublishIncompleteError (types.ts:146-155) carries the recordId so the stranded-record case is actionable, and registry-publish.ts:97-105 maps it to a 502 rather than a bare 500. Regression tests present: agentcore-client.test.ts:262 (CREATE_FAILED), :285 (post-create submit failure), :304 (poll budget exhausted). I saw all three fire in the test log.

(10) P1-SECURITY both resource ARNs use * — FIXED. See nit 3 for a residual.
The grants moved out of task-api.ts into the new nested stack: registry-api.ts:111-137 formats registry/{props.agentRegistryId} and registry/{props.agentRegistryId}/record/* and applies read-only to resolve/list/show, read+write to publish. task-api.ts retains no bedrock-agentcore registry grant (verified by grep). The nag suppression at :189 was rewritten to describe the narrowed scope truthfully. Regression test present: registry-api.test.ts:63, though it asserts only on the action list, not the resource ARNs — see nit 3.

---

New blocking issues

B1 — agentcore-client.ts:114-136: caller-controlled description is raw-interpolated into SKILL.md frontmatter; an injected x-abca-runtime line wins on read and bypasses all publish-time runtime validation. (P1-security)

buildSkillMd base64-encodes the runtime (the finding-4 fix) but builds the description line by raw String(...) interpolation with no escaping and no newline stripping. discovery is an unrestricted Record<string, unknown> that publish validates only as "is a plain object" (registry-publish.ts:138-140), so a RegistryPublisher can smuggle a newline plus a second frontmatter key. Because parseSkillRuntime (:152) uses a ^…$ multiline regex and String.match returns the first occurrence, the injected line — appearing above the real one — is the one that resolves.

Probed at head, discovery: { description: "benign\nx-abca-runtime: <base64 of {\"prompt_fragment\":\"INJECTED-EXFIL-PROMPT\"}>" } with a legitimate validated runtime: { prompt_fragment: 'THE VALIDATED BENIGN FRAGMENT' } produced:

---
name: acme-tdd
description: benign
x-abca-runtime: <injected>      <- attacker's, never validated
version: 1.0.0
x-abca-runtime: <legitimate>    <- the validated one, shadowed
---

and publish() returned runtime = {"prompt_fragment":"INJECTED-EXFIL-PROMPT"}. agent/src/registry/agentcore_client.py:74 uses re.search on the same pattern and has identical first-match behavior, so the agent-side loader in PR 2/3 will read the injected payload too. For skill this is direct prompt injection into a resolved asset the audit trail records as the validated pin; the same trick against a duplicated name/version line corrupts the descriptor identity.

This is a strictly worse failure mode than finding (2), because the bypassed validator is the one added to fix (2): the record's stored runtime is no longer the runtime the API validated.

Suggested fix: build the frontmatter with a real YAML serializer (js-yaml's dump — already a cdk dependency and already used in this repo — plus yaml.safe_dump on the Python read side), or at minimum (a) strip \r\n and truncate description before interpolation, (b) parse only the frontmatter block delimited by the first ---/--- pair rather than regexing the whole document, and (c) reject a descriptor carrying more than one x-abca-runtime key instead of silently taking the first. A regression test asserting that a newline-bearing description cannot change the round-tripped runtime would lock it in on both sides.

B2 — registry-resolve.ts:45-62: the secret redaction is a denylist over an open payload, so finding (3) is closed only for headers/command/args. (P1-security)

Detailed above under finding (3). The concrete probe at head: a caller in no registry groups gets 200 with url: "https://h/mcp?token=SUPERSECRET", api_key: "AKIA-LEAKED", and env: {TOKEN: "ghp_leak"} verbatim. Two independent gaps compose to produce it — publish accepts unknown keys on runtime (validateRuntime checks required fields but never rejects extras), and resolve redacts only three known names. Fail-closed says the response should be an allowlist: project the per-kind known-safe fields (transport, tool_prefix, version, header keys, and a host-only form of url) and drop everything else, rather than enumerating what to hide. Complementarily, having validateRuntime reject unknown keys per kind would make the payload closed at the gateway and is a small change to code you already touched. The mcp_server row in REGISTRY.md §2 advertising headers as part of the runtime payload should then note that the human-facing resolve response is redacted while the orchestrator path is not.

B3 — registry-provisioning/index.ts:65 and agentcore-client.ts:176: new BedrockAgentCoreControlClient({}) bypasses the attributed client factory, silently dropping solution UA attribution (#319).

Both are naked constructions. makeClient from cdk/src/handlers/shared/ua.ts:113 exists on this PR's base (03228246) and is used by ~20 other handlers (get-trace-url.ts:33, reconcile-concurrency.ts:24, …); these two are the only naked SDK clients left in cdk/src/handlers/. Per AGENTS.md this drops the md/uksb-wt64nei4u6#{component} segment from every registry control-plane call, which is exactly the omission #345 was merged to prevent — and it is invisible because nothing fails. Note the intent is clearly there: registry-api.ts:86 already sets ABCA_COMPONENT: 'registry-api', so the component label is wired and only the factory call is missing. Fix is one line each: makeClient(BedrockAgentCoreControlClient), and in the adapter opts.client ?? makeClient(BedrockAgentCoreControlClient) (the injection seam for tests is unaffected). Worth adding ABCA_COMPONENT to the provisioning Lambdas in registry.ts:70-76 too, since they currently fall through to the api default.

---

Non-blocking nits

1. registry-publish.ts:141 / :174 — custom and auto_approve are typed boolean? but never type-checked, so custom: "false" reaches the adapter as the truthy string "false" and silently flips storage to CUSTOM (probed: 201, PublishInput.custom === "false"). This is the last live fragment of isadeks' "require booleans for the flags". Not blocking because the security-relevant direction fails closed — I verified auto_approve: "false" returns 403, not an approval bypass. A typeof !== 'boolean' guard alongside the others in validate closes it.
2. registry-publish.ts:134-136 — the double parseConstraint call from the first review round is now fixed (single const constraint). Noting it as resolved.
3. registry-api.test.ts:63 — the test that guards the finding-10 fix asserts only the action list, not the resource ARNs, so a regression back to resources: ['*'] would keep this test green. Add a Resources: Match.arrayWith([Match.objectLike({ 'Fn::Join': Match.arrayWith([Match.arrayWith(['reg-abc123'])]) })]) style assertion, or simply assert the rendered policy JSON contains no bare "*" resource. This is the highest-value test to add given #10 was a security finding.
4. agentcore-client.ts:280-283 — the TODO(GA) marker for the O(n) List+N×Get was added as requested. Good.
5. agentcore-client.ts:151-160 / agentcore_client.py:78-81 — the legacy single-quoted-JSON fallback is dead on arrival: no records exist yet (this PR provisions the registry for the first time), so it is permanent complexity guarding an empty set, plus it is the branch that makes the parser lenient. Consider dropping it; if kept, agentcore-client.ts:157 is uncovered (visible in the coverage report).
6. registry.ts:105 — resources: ['*'] on the create policy remains, with a documented "AccessDenied when scoped, observed on deploy" rationale and a matching nag suppression. Accepted as-is: CreateRegistry genuinely has no resource to scope to. Flagging only so it stays visible at GA when the native construct lands.

Documentation status

Good. docs/design/REGISTRY.md §7 gained an accurate "Why a separate API" box explaining the 500-resource cap and the registry_api_url config cost, and I verified the CLI side actually implements that claim (stack-outputs.ts:90,100; configure.ts:38,71,78). §6 now matches the shipped always-submit behavior. I regenerated the Starlight mirror (node docs/scripts/sync-starlight.mjs) and git status docs/ came back empty — no drift, so CI's "Fail build on mutation" step will pass. One doc gap tied to B2: §2's mcp_server row and §7.2's resolve contract should state that the human-facing response is redacted.

Tests and CI

Run locally at 4f8da982, not read:

• CDK registry: registry-handlers + agentcore-client + registry-provisioning + registry-resolver — 63 passed.
• Parity: registry-resolution-parity + registry-resolution-ranking-parity — 34 passed.
• Constructs: registry + registry-api — green; registry-api.test.ts synths once per helper call and does not re-enable aws:cdk:bundling-stacks, so #366 is respected.
• Bootstrap: all six suites — 109 passed, including synth-coverage and golden-baseline.
• Agent: test_registry_agentcore_client + both corpora — 42 passed.
• tsc --noEmit on cdk/ — clean. scripts/check-types-sync.ts — OK, 69 CLI exports validated (RegistryShowResponse is now a named shared type, so it rides the guard; that was a first-round nit and it is resolved).
• GitHub checks: all 8 green.

Bootstrap synth-coverage: verified, not taken on trust. BOOTSTRAP_VERSION 1.4.0 → 1.5.0 (version.ts:37) with the bump history extended; BOOTSTRAP_VERSION / BOOTSTRAP_HASH / bootstrap-template.yaml / the version snapshot are all regenerated and committed; artifact-sync.test.ts passes, which is what would catch an uncommitted regeneration. The new CFN types are mapped in resource-action-map.ts (AWS::CloudFormation::Stack, AWS::Cognito::UserPoolGroup, AWS::StepFunctions::StateMachine, Custom::AgentCoreRegistry). On the ARN-pattern gap that usually bites here: infrastructure.ts:57 correctly widens to stack/backgroundagent-dev-* for the nested stacks — necessary because both new nested stacks synth as backgroundagent-dev-<Child><hash> — and application.ts adds the Step Functions statement scoped to stateMachine:backgroundagent-dev-* plus states.amazonaws.com in the iam:PassRole service allowlist. I checked the nested-stack move specifically, since it shifts logical IDs, and synth-coverage passes at head. DEPLOYMENT_ROLES.md golden baseline is in sync.

Test-coverage gaps worth noting: nothing exercises the B1 frontmatter-injection path or the B2 non-denylisted-field path — both would be single tests. Jest reports the registry slice below the global branch/function thresholds when run in isolation, which is expected for a subset run and not a finding.

Review agents run

The pr-review-toolkit sub-agents are not dispatchable from this execution context (no Agent tool; ToolSearch for it returns nothing), so I applied each rubric explicitly by hand and label the results rubric-applied, not agent-run:

• code-reviewer (rubric-applied) — Routing is correct per AGENTS.md. The nested-stack extraction is the right call and honestly documented. Found the #319 attribution break (B3), which is a repo-specific guideline violation rather than a logic bug.
• silent-failure-hunter (rubric-applied) — This is where the PR improved most: finding 9's fix converts three silent successes into loud typed failures, and RegistryPublishIncompleteError turns a swallowed stranded-record into an actionable 502. Remaining silent failures: B1 (injected runtime silently shadows the validated one — no error, wrong data), and nit 1 (a string "false" silently changes storage mode). drainRecords ConflictException swallowing is correct-by-design with the isComplete retry.
• type-design-analyzer (rubric-applied) — RegistryPublishIncompleteError and the ParseResult discriminated union are well designed. The weakness is that runtime and discovery are both open Record<string, unknown> on the wire while RuntimePayload is a closed discriminated union internally, and the as unknown as RuntimePayload cast at registry-publish.ts:75 is where that gap is laundered — which is the type-level root cause of both B1 and B2. Narrowing the wire types per kind would collapse both findings.
• comment-analyzer (rubric-applied) — Comments are unusually accurate and I spot-checked the load-bearing ones against behavior. :117-121 (base64 rationale), :366-372 (prior-behavior description), :89-92 (registry.ts AccessDenied rationale), and :121 (optionalValue clears the description — confirmed against the SDK types) all hold. One comment is now overstated: registry-resolve.ts:30-43 reads as though the secret-leak class is closed, but it describes a denylist over exactly two field families; it should say what is not redacted.
• pr-test-analyzer (rubric-applied) — Eight of ten prior findings have a targeted regression test, which is a strong ratio. Gaps: nothing locks the finding-10 resource ARNs (nit 3), and nothing covers B1/B2.
• security-review skill scope (rubric-applied) — IAM: handler roles now least-privilege on registry/{id} (+ record wildcard, justified); provisioning * accepted with a deploy-observed rationale; iam:PassRole service allowlist addition is scoped. Auth: Cognito group gating verified fail-closed by probe (403 on non-boolean auto_approve). Input gateway: two bypasses found (B1 descriptor injection, B2 open runtime keys). No Cedar engine-pin movement in this PR (cedar-wasm/cedarpy untouched — confirmed). No secrets committed. Attribution regression B3.

Human heuristics

• Proportionality — Pass. The port/adapter split is earned by the documented GA substrate swap, and the nested-stack split is a response to a hard CloudFormation limit, not speculative structure. The fix commits are proportionate to what was asked; none of them over-reach.
• Coherence — Concern. agentcore-client.ts:122 vs :114 is incoherent within a single function: the runtime is carefully encoded to be injection-proof while the description on the adjacent line is raw-interpolated. Same class of bug, same frontmatter block, two different standards — which is precisely how B1 survived a fix aimed at it. Likewise registry-publish.ts:160 validates the runtime's required fields rigorously but leaves the payload open, so registry-resolve.ts:45 has to compensate with a denylist; one closed contract would remove the need for both.
• Clarity — Pass. Naming, named constants over magic values, and typed errors that carry recovery information (recordId) are all good. The comment overstatement at registry-resolve.ts:30-43 is the one place clarity works against the reader.
• Appropriateness — Pass. Adapter decisions remain grounded in the live spike findings rather than self-written mocks, tests assert intended behavior (fail-closed resolution, immutability, PENDING_APPROVAL landing) rather than merely current behavior, and the new tests added this round test the fix, not the implementation.

---

Governance is clear (#246 approved + P0) and I am not raising the branch name. To be explicit about what I did and did not inherit: I did not adopt ayushtr-aws' approval, and I did not adopt isadeks' change request either — eight of their ten findings are genuinely fixed and I say so with evidence. The request for changes rests on B1 (validation bypass via descriptor injection, both languages), B2 (denylist redaction leaves cleartext secrets on an ungated endpoint), and B3 (attribution regression). B3 is two lines; B1 and B2 are contained within buildSkillMd/parseSkillRuntime and redactRuntimeForResponse plus a tightening of validateRuntime. Happy to re-review quickly on the next push.

[Kalindi-Dev]

Thanks @scottschreckengaust — round-2 findings addressed in c8e4790c (rebased on latest main; build (agentcore) green). B1 (SKILL.md frontmatter injection) and B2 (resolve denylist → allowlist + publish-time unknown-key rejection) are the two security items; B3 (makeClient UA) and both nits (boolean-flag guard, IAM ARN assertion) done. Details inline on each thread. Re-review welcome.

[scottschreckengaust]

Verdict: Request changes — all three of my prior blocking findings are genuinely FIXED and locked by tests; one new blocker: this PR silently blinded the bootstrap deploy-IAM guard by moving resources into nested stacks.

Credit where it is due: this was a real remediation pass, not a comment-answering pass. I re-probed every one of my 2026-08-11 findings against this head SHA (c8e4790c) and each fix removes the capability, not just the symptom. Details and my reproduction attempts are in the table below.

The one blocker is a regression this PR introduced in cdk/test/bootstrap/synth-coverage.test.ts coverage — not in the shipped policies (those are correct today). It is ~10 lines to fix.

---

Vision alignment

Advances the platform. The registry replaces "vendor an MCP server into the image + CDK deploy" with a versioned, auditable, pinned catalog — that is squarely reviewable outcomes and bounded blast radius (immutability per version, fail-closed resolution, resolved_assets audit triple). The fail-closed choices are the right ones and are now consistent across both languages:

• agentcore-client.ts:355-364 and agentcore_client.py:166-172 both refuse to resolve a record with an empty/unreadable runtime rather than handing back {} — that is the tenet-correct call (a task must never run with a substituted asset while the audit claims the pin was honored).
• waitPastCreating (agentcore-client.ts:404-423) now throws on *_FAILED and on poll-budget exhaustion instead of reporting 201 for a record the substrate rejected.
• Resolve/list/show being open to any authenticated caller is documented (docs/design/REGISTRY.md:154) and is now safe because the response is an allowlist projection (see B2 below). Good.

The nested-stack split is a legitimate response to a hard CloudFormation constraint, and the rationale is written down honestly in three places (registry-api.ts:19-33, registry.ts:200-208, REGISTRY.md:90) including the cost (a second invoke URL). No undocumented tenet trade.

---

Status of prior blocking findings

#	Finding (2026-08-11)	Status	Evidence at this head
B1	Frontmatter injection — caller-controlled discovery.description raw-interpolated above x-abca-runtime; first-match parsers on both sides let an injected key shadow the validated runtime	FIXED	Fix is structural, which is what I asked for. buildSkillMd (cdk/src/handlers/shared/registry/agentcore-client.ts:104-121) now emits via yaml.dump, and parseSkillFrontmatter (:127-139) yaml.loads the whole block instead of a per-line regex. Python mirrors it (agent/src/registry/agentcore_client.py:76-92, yaml.safe_load over _SKILL_FRONTMATTER_RE). I re-ran my original payload plus five escalations — plain \n, a \n---\nx-abca-runtime: …\n---\n close-and-reopen, leading ---, \r\n, body-block injection, and a description that is the key — and all six fail: yaml.dump block-scalars or double-quotes the value, so the injected text stays inside description and the round-tripped x-abca-runtime is always the validated one. Verified identically through PyYAML on the exact bytes js-yaml emits, so TS↔Py parity holds. Regression tests: cdk/test/handlers/shared/agentcore-client.test.ts:207-226 and agent/tests/test_registry_agentcore_client.py:107-129 (both assert the round-tripped runtime, not just the serialized string).
B2	Denylist redaction — open runtime + ~3 masked names meant url?token=…, api_key, env.TOKEN returned cleartext to a caller in no registry groups	FIXED	Closed at both boundaries, as requested. Read path redactRuntimeForResponse (cdk/src/handlers/registry-resolve.ts:44-80) is now a per-kind allowlist that constructs a fresh object — url is reduced to new URL(...).origin (I confirmed this strips both ?token= and user:tok@ userinfo), header values masked with keys retained, and api_key/env/command/args/anything unnamed are structurally unreachable because they are never copied. Publish path validateRuntime (registry-publish.ts:172-186) rejects unknown keys per kind, so the payload is also closed at the gateway. My exact 2026-08-11 probe payload now yields a 400 at publish and, for a pre-existing record, a response with none of the three secrets. Tests: registry-handlers.test.ts:259 (url-token + header masking), :286 (fail-closed drop of unknown/secret keys), :197 (publish rejects api_key). The comment at :30-43 was also rewritten to describe the allowlist truthfully rather than implying the whole leak class was closed — thank you.
B3	UA attribution (#319) — naked new BedrockAgentCoreControlClient({}) in the provisioning handler and the adapter	FIXED	Both sites route through the attributed factory: cdk/src/handlers/registry-provisioning/index.ts:68 (makeClient(BedrockAgentCoreControlClient)) and shared/registry/agentcore-client.ts:202 (opts.client ?? makeClient(...), so the test seam is intact). ABCA_COMPONENT: 'registry-provisioning' is set on the provisioning Lambdas (registry.ts:76-78) and 'registry-api' on the API Lambdas (registry-api.ts:86). I swept cdk/src/handlers/, agent/src/registry/, and cli/src/ for new XxxClient(/boto3.client(/boto3.resource( — zero naked clients remain in this diff's scope.

Also re-verified fixed (from the 07-27/07-28 rounds, since the branch was force-pushed): SubmitRegistryRecordForApproval now runs for every publish with only the APPROVED transition gated on autoApprove (agentcore-client.ts:255-269); publisher identity is threaded and stamped for all three descriptor types (registry-publish.ts:75, customBody :500, _meta :523, frontmatter :117); clientToken derived from the CFN RequestId (registry-provisioning/index.ts:81, deterministic-per-RequestId test at index.test.ts:96); the Update branch issues a real UpdateRegistryCommand (:99-116); non-boolean custom/auto_approve now 400 (registry-publish.ts:145-150, test :205); IAM scoped to registry/{id} + record/* with a resource-asserting guard test (registry-api.test.ts:81-101 — it now walks every statement and fails a bare "*", which is the robust form I asked for). Registry parity corpora run both languages (43 Python cases pass locally: uv run pytest tests/test_registry_* → 43 passed).

Nothing I previously flagged survives. The blocker below is new and was introduced by the nested-stack commit (607279e4).

---

Blocking issues

B4 — [NEW] The bootstrap deploy-IAM guard is now blind to everything in the two new nested stacks

cdk/test/bootstrap/synth-coverage.test.ts:36-72 builds the root AgentStack and asserts over Template.fromStack(root). Before this PR there were no NestedStacks under cdk/src/ (git grep -l NestedStack main -- cdk/src/ → empty). This PR adds two (AgentRegistryStack, RegistryApi), and a nested stack's resources do not appear in the parent template — the parent only gets an AWS::CloudFormation::Stack. I confirmed this directly:

ROOT template resource types:   ["AWS::CloudFormation::Stack"]
NESTED template resource types: ["AWS::IAM::Role","AWS::StepFunctions::StateMachine"]
Root sees StateMachine? false

So the guard that exists specifically to prevent #350's failure mode (deploy dies at CFN with AccessDenied on an unmapped resource type) now provides zero coverage for AWS::StepFunctions::StateMachine, AWS::Cognito::UserPoolGroup, Custom::AgentCoreRegistry, and the whole second AWS::ApiGateway::* surface.

To be explicit and fair: the shipped bundle is correct today. I hand-checked every CFN type in both nested stacks against RESOURCE_ACTION_MAP + CFN_TYPES_WITHOUT_EXEC_ROLE_IAM and the policy documents — StateMachine, UserPoolGroup, CloudFormation::Stack, Custom::AgentCoreRegistry, apigw types, Logs::LogGroup are all mapped and granted (resource-action-map.ts:63,70,110-116; application.ts:155-159,241-256; infrastructure.ts:56-58,117), BOOTSTRAP_VERSION is a correct minor bump to 1.5.0 with a truthful changelog comment (version.ts:28-37), artifacts are regenerated, and the DEPLOYMENT_ROLES.md golden baseline is updated. You did the work by hand — that is not the problem.

The problem is that the next person to add a resource to either nested stack gets a green synth-coverage run and a red deploy, which is precisely the class of bug ADR-002 built this test to make impossible. A test that passes vacuously is worse than no test, because it launders a missing check as a satisfied one.

Risk: silent erosion of least-privilege bootstrap coverage; a future deploy-time AccessDenied that CI cannot predict.

Suggested fix (in synth-coverage.test.ts, union the nested templates into the assertion):

const root = app.node.tryFindChild('backgroundagent-dev') as Stack;
const stacks: Stack[] = [root, ...root.node.findAll().filter((c): c is NestedStack => c instanceof NestedStack)];
const typesInTemplate = new Set(
  stacks.flatMap((s) => Object.values(Template.fromStack(s).toJSON().Resources as Record<string, { Type: string }>).map((r) => r.Type)),
);

Please also add a one-line assertion that at least one nested stack was discovered, so the traversal itself cannot silently regress to only-the-root:

expect(stacks.length).toBeGreaterThan(1);

I would expect this to newly surface any nested-stack type you have not mapped — if it comes back clean, that is the proof the bundle is complete rather than the current assumption of it.

---

Non-blocking suggestions / nits

1. No throttling on the registry RestApi — with an O(n) read path behind it (cdk/src/constructs/registry-api.ts:147-153). The sibling TaskApi sets throttlingRateLimit: 60 / throttlingBurstLimit: 100 (task-api.ts:322-324); this new API sets neither, so it inherits the 10,000 rps account default. Meanwhile listRecords is List + N×GetRegistryRecord and every read path funnels through it (agentcore-client.ts:305-330, correctly TODO(GA)-marked), and resolve/list/show are open to any authenticated caller. So one caller-visible request amplifies into 1 + N AgentCore control-plane calls against a preview service's quota. Today the blast radius is confined to this API (nothing consumes resolve yet — REGISTRY.md:172), which is why this is a nit rather than a blocker. It becomes load-bearing the moment PR 2 wires the orchestrator resolve step, because resolution is fail-closed and a quota-starved resolve turns into FAILED tasks. Please mirror the TaskApi limits here now — it is two lines in the existing deployOptions.
2. No WAF on the registry API, unlike the main one. AwsSolutions-APIG3 is suppressed at registry-api.ts:199 with an "internal/dev, Cognito-authenticated" rationale, while TaskApi does associate a Web ACL (task-api.ts:532). The rationale is stated rather than hidden, so this is fine for MVP — but it is a real posture divergence between two APIs on the same user pool and is worth a line in REGISTRY.md so it is a decision rather than an omission.
3. Duplicate AWS::ApiGateway::Account — an account-level singleton now managed by two stacks. Both RestApis create one (root: TaskApiAccount…; nested: ApiAccount…, verified by synth). Two CFN resources contend for the same account-wide CloudWatch-role setting; deploy order decides which role ARN wins, and it makes the nested stack's lifecycle able to perturb the main API's access logging. Consider cloudWatchRole: false on the registry RestApi and letting the root own the singleton.
4. Dead branch: runtime.type (registry-resolve.ts:65). The reader projects type, but ALLOWED_RUNTIME_KEYS.mcp_server (registry-publish.ts:173) does not include type — so publish rejects it and this line can never fire for a record published through this API. Either drop it or add type to the publish allowlist if the intent is to accept the server.json spelling. As-is it is a small contract-drift signal between the two allowlists that the "kept in sync" comment at :169-171 claims are aligned.
5. TS↔Py asymmetry on a corrupt descriptor (availability, not disclosure). In TS, extractPayload (agentcore-client.ts:467,488) and parseSkillRuntime (:181-183) JSON.parse eagerly inside getRecordById, which listRecords calls for every record — so one malformed inlineContent or non-base64 frontmatter value throws a SyntaxError that escapes to a 500 and takes down list/show/resolve for every asset in that kind/namespace. Python is deliberately better here: it only parses the winner and catches (ValueError, JSONDecodeError) into a scoped REMOVED resolution error (agentcore_client.py:157-165). This also means the nice fail-closed isNonEmptyRuntime check at :355 is unreachable for a corrupt record on the TS side — the throw happens first. Requires an out-of-band write to reach (the code comments acknowledge that path), so it is a nit, but the TS side should mirror Python's per-record try/catch so one poisoned record degrades to one unresolvable asset rather than a namespace-wide outage.
6. Stray whitespace-only diff. cdk/src/constructs/task-api.ts:235 adds a blank line inside TaskApiProps for no reason. Worth dropping so the only change to that file is the explanatory comment block.

---

Documentation

Good coverage, and it explains why rather than just what.

• docs/design/REGISTRY.md is new and substantive: the separate-API rationale with the 500-resource constraint and the registry_api_url cost (:88-90), the access-control model (:149-154), the grammar plus the explicit "two grammars in the tree" note reconciling it with the looser agent/src/workflow/validator.py check (:167), and the staged PR1/PR2/PR3 plan (:172-178). The note that resolve happens in the orchestrator rather than createTaskCore, with the trade-off stated (FAILED task vs. 422 at submit), is exactly the kind of decision record that should be written down.
• docs/design/DEPLOYMENT_ROLES.md golden baseline updated in the same PR (Cognito group actions, the StepFunctions statement, the nested-stack CFN ARN, states.amazonaws.com in PassRole) — this is the file that most often drifts, so thank you for keeping it in lockstep.
• Starlight mirror is in sync. I diffed docs/src/content/docs/architecture/Registry.md and Deployment-roles.md against their sources; the only deltas are the generator's link rewrites. Not hand-edited.
• contracts/registry-resolution/ ships a README plus two corpora consumed by four runners (two TS, two Python) — the right pattern, mirroring contracts/cedar-parity/.
• Issue tracking: #246 is approved + P0 and covers this stack. Governance is settled; I am not re-litigating it, and the pr/*-style branch name is a waived nit.

Minor gap: nits 2 and 3 above (WAF divergence, apigw Account singleton) deserve a sentence each in REGISTRY.md so a future reader knows they were considered.

---

Tests & CI

CI is fully green (8/8, incl. build (agentcore), CodeQL ×3, secrets/deps scan). All 21 review threads have author replies. Two other approvals on record (ayushtr-aws, theagenticguy) — noting for completeness that neither an approval nor green CI discharges a standing change request, which is why I re-probed everything above independently rather than relying on either.

Coverage is genuinely good, and the new tests are the right kind: they assert what the code should do (the round-tripped runtime must equal the validated payload; no bare "*" resource in any bedrock-agentcore: statement) rather than merely pinning current output. That is the distinction that matters for regression tests on security findings, and the B1/B2/#10 tests all clear it.

Locally: uv run pytest agent/tests/test_registry_agentcore_client.py test_registry_resolution_corpus.py test_registry_resolution_ranking_corpus.py → 43 passed. I could not run the CDK suites in my worktree (TS5103: Invalid value for '--ignoreDeprecations' — cdk/tsconfig.jest.json:5 asks for "6.0" but the hoisted typescript resolves to 5.9.3 in my tree; this is a local-environment artifact of the TS 6.x pin, not a defect in this PR, and CI runs them green). I read all the relevant CDK tests instead.

Bootstrap synth-coverage status: passing but incomplete — see B4. BOOTSTRAP_VERSION 1.4.0 → 1.5.0, artifacts regenerated, BOOTSTRAP_HASH updated, golden baseline updated, policies.test.ts extended with the StepFunctions sid and states service. The bundle content is right; the guard's reach is not.

Test-performance check (#366): all four new CDK test files synth once in beforeAll and none re-enable aws:cdk:bundling-stacks. Clean.

---

Review agents run

I must be explicit about a process limitation: I am running as a fan-out subagent of /review_prs, and agent nesting in this harness is one level deep — I cannot dispatch the pr-review-toolkit agents (code-reviewer, silent-failure-hunter, type-design-analyzer, comment-analyzer, pr-test-analyzer) or the security-review skill as nested agents from this context. Rather than silently skip the mandatory Stage-3 step, I applied each agent's rubric by hand and am listing the rubric per dimension so the gaps in my coverage are auditable:

• code-reviewer (guidelines/style/routing) — applied. Routing matches the AGENTS.md table (API/Lambdas in cdk/, read-only port in agent/, bgagent surface in cli/). #319 UA rule swept across all three trees (clean). CDK↔CLI types-sync verified by hand: all seven registry wire types present in both cdk/src/handlers/shared/types.ts and cli/src/types.ts with identical field sets. No hardcoded ARNs (formatArn throughout). No Cedar engine pin movement in this diff, so no parity-fixture obligation. Found: nits 4 and 6.
• silent-failure-hunter (error handling/fallbacks) — applied; this was the highest-yield dimension. Traced every catch. The previously-dangerous swallows are now correct: waitPastCreating re-throws non-ResourceNotFoundException and throws on budget exhaustion (:404-423); RegistryPublishIncompleteError surfaces the stranded recordId as a 502 rather than a bare 500 (registry-publish.ts:94-102) — good, an operator can act on that. Two remaining swallows are justified and commented (parseSkillFrontmatter's catch → {} at :136 is the fail-closed direction because resolve then rejects the empty runtime; ConflictException tolerance in drainRecords is retried by isComplete). Found: nit 5 (the TS eager-parse blast radius, and the resulting unreachability of the isNonEmptyRuntime fail-closed check for corrupt records).
• type-design-analyzer (new types) — applied. RegistryStatus as a closed union mirroring the substrate tokens, StorageMode, and the discriminated RuntimePayload are well-modelled, and keeping the port-internal domain types deliberately out of the CLI types-sync contract (documented at registry/types.ts:24-25) is the right boundary. One observation: RuntimePayload is a real union but runtime crosses the API boundary as Record<string, unknown> and is cast with as unknown as RuntimePayload (registry-publish.ts:73), so the union buys no compile-time safety at the gateway — the runtime validateRuntime allowlist is doing all the work. That is acceptable (validation is the actual gate) but the cast is the seam a future refactor will misread; a narrowing parse returning RuntimePayload | null would make the type earn its keep.
• comment-analyzer (comment accuracy) — applied, and specifically re-checked the comments attached to the fixes, since a stale comment is how a fixed finding gets silently reintroduced. registry-resolve.ts:30-43 now accurately describes an allowlist and correctly states the orchestrator bypasses this handler. agentcore-client.ts:104-110 and :127-131 correctly explain why structural YAML emit/parse defeats key injection (not merely that it does). registry.ts:87-96 explains why CreateRegistry needs resources: ['*'] and the non-obvious workload-identity dependency — that comment will save someone a CREATE_FAILED. version.ts:28-37 changelog matches the actual policy delta. The TODO(GA) at :305 and the GA-throwaway markers are honest. No inaccurate comments found.
• pr-test-analyzer (coverage gaps) — applied. Failure paths are covered, not just happy paths (401/403/400/409/422/502, CREATE_FAILED, poll timeout, duplicate-delivery idempotency, the two injection regressions, the two leak regressions, the IAM resource guard). Found: B4 (the coverage hole is itself a test-design defect) and the observation that no test asserts either nested stack's resources are reachable from the bootstrap guard.
• security-review (IAM/Cedar/secrets/input-gateway) — applied by hand; this diff touches IAM, a new input gateway, and secret-bearing payloads, so it was in scope. Handler IAM is scoped to registry/{id} + registry/{id}/record/* with a regression test that rejects a bare "*". Provisioning's resources: ['*'] is genuinely create-time-unavoidable and suppressed with a truthful reason. Auth fails closed everywhere (extractUserId → 401; userInGroup → 403; auto_approve re-checked against RegistryApprover after validation). userInGroup (gateway.ts:43-56) correctly normalizes the cognito:groups claim across both array and delimited-string shapes and returns false on absence — fail-closed. Secrets: the allowlist projection is the fix, and publish additionally refuses to store unknown keys with an error message that tells the publisher to reference a Secrets Manager ARN instead of inlining. Cedar: untouched, no engine pin movement. Found: nits 1-3.

No agent's scope was omitted as out-of-scope; the omission is the mechanical dispatch, and it is disclosed above.

---

Human heuristics

• Proportionality — pass. The port/adapter seam looks like ceremony until you read that it is deliberately GA-throwaway (registry.ts:26-28), at which point one interface per language is the cheapest way to make the swap self-contained. No speculative abstraction: exactly one implementation per side, and the "engine" temptation was avoided. agentcore-client.ts at 540 lines is the largest new file and its size is essential (substrate encode/decode for three descriptor shapes), not accreted. The nested-stack split is forced by a hard CFN limit rather than chosen.
• Coherence — concern (minor). Same concepts are named the same across the two languages (RUNTIME_META_KEY/_RUNTIME_META_KEY, x-abca-runtime, identical status tokens), and the parity corpora enforce it mechanically. But the new API diverges from its sibling on two operational conventions with no stated reason — no throttling and no WAF (nits 1-2, registry-api.ts:147 vs. task-api.ts:322) — and the two allowlists that a comment claims are "kept in sync" disagree on type (nit 4). These are exactly the drifts that copy-paste-adjacent construct work produces.
• Clarity — pass. Names communicate intent (redactRuntimeForResponse, waitPastCreating, isNonEmptyRuntime, RegistryPublishIncompleteError). Error handling surfaces failures rather than hiding them behind plausible defaults — the 502-with-recordId for a stranded partial record is a notably good call, and the publish validation error text tells the caller what to do instead. Magic values are named constants (RECORD_CREATE_POLL_MS, SKILL_NAME_MAX, CLIENT_TOKEN_LENGTH).
• Appropriateness — concern (B4). Integration behavior was verified against the real substrate, not self-written mocks — the async-create/_meta-survival/CUSTOM-round-trip spike findings are cited inline and drive real design choices (agentcore-client.ts:19-32), and the workload-identity IAM dependency at registry.ts:87-96 is clearly a scar from an actual failed deploy rather than a guess. That is the AI001 trap avoided properly. The AI005 trap is also avoided: the new tests assert what the code should do. The failure is maintainability of the guard rather than of the code — B4 leaves the next contributor with a test that passes for the wrong reason.

[Kalindi-Dev]

@scottschreckengaust thanks — confirming your three prior blockers (B1 frontmatter injection, B2 denylist→allowlist, B3 UA) are all resolved, and thank you for re-probing each against c8e4790c. On B4 (the nested-stack split blinding the synth-coverage.test.ts bootstrap-IAM guard): agreed, and I've filed it as #757 so the fix (extend the guard to walk each nested stack's own template) is tracked. As you noted, the shipped bundle is correct today — this is a test-coverage regression, not a live policy gap. The two round-3 nits (API throttle, dead type branch) are noted inline and folded into that follow-up.

[scottschreckengaust]

Conditional approval based on backlog issues for blockers and nits.

on PTO