docs(decisions): ADR-018 propose central agent asset registry (#246)

[Kalindi-Dev]

Status

** approved label on #246 per ADR-003.**

Summary

Proposes ADR-018, which fixes the contract for a central agent asset registry closing #246:

• URI grammar — registry://<kind>/<namespace>/<name>@<constraint> (already declared at agent/src/workflow/validator.py:50).
• MVP asset kinds — mcp_server, cedar_policy_module, skill. Others declared but not loaded.
• Semver — exact / ^ / ~ only. Rejects *, latest, >=.
• Immutability — (kind, namespace, name, version) immutable once published; republish returns 409.
• Lifecycle — draft → submitted → approved | rejected → deprecated → removed.
• Resolve at create-task boundary — same location workflow_ref resolves today; resolved_assets stamped on the task record.
• Fail-closed — unresolvable refs fail admission with a specific reason.
• Descriptor validation at publish — typed descriptor per kind (tool surface, egress domains, Cedar actions, min compute profile, permissions).
• Resolver interface as the seam — CDK/TS orchestrator and Python agent talk to a RegistryClient abstraction; substrate changes and any AgentCore Aug 6 2026 rename are confined to one implementation file per language.
• MVP E2E path is one asset kind: MCP server (per issue AC4).
• Governance workflow ships in MVP — publisher/approver roles, audit trail on every state transition, event-driven review integration.
• Workflows do not migrate to the registry in this ADR — ADR-014 workflows stay filesystem-backed.
• Clean split from docs(governance): refine ADR-012 to many-to-many ADR↔Persona↔Skill graph with bidirectional backlinks + parity linter #381 — registry stores skill runtime artifacts; docs(governance): refine ADR-012 to many-to-many ADR↔Persona↔Skill graph with bidirectional backlinks + parity linter #381's ADR↔Persona↔Skill documentation graph stays in docs/ + plugin markdown.

Substrate

The ADR does not pick the substrate. It ranks candidates by fit for ABCA and defers the pick to the design PR:

1. Preferred: AWS Agent Registry (Bedrock AgentCore).
2. Fallback: DynamoDB + S3.
3. Considered: mcp-gateway-registry (AWS OSS Blog June 2026), agentregistry.ai (Solo.io), Microsoft Entra Agent Registry + AGT.
4. Also considered, briefly: Google Cloud Agent Registry, Smithery, Glama, PulseMCP, ACI.dev, Composio, Toolhouse, Docker MCP Catalog, Jozu Hub, Stacklok/ToolHive — each with a one-line disqualifier.

Design PR decision framework names four concrete conditions under which the design PR flips from preferred to fallback (region GA, immutability guards, semver-on-top complexity, Aug 2026 rename cost).

Addressing #381 (per @scottschreckengaust's comment on #246)

Sub-decision 13 splits the two concerns cleanly:

• This registry stores skill runtime artifacts (prompt+tools bundles the agent loads at task start).
• docs(governance): refine ADR-012 to many-to-many ADR↔Persona↔Skill graph with bidirectional backlinks + parity linter #381 owns the ADR↔Persona↔Skill documentation graph in docs/ and plugin markdown with frontmatter edges + parity linter.

If a skill record's descriptor eventually cites an ADR, that's metadata on the record — not a graph edge #381 owns. Happy to iterate on the boundary if you see it differently.

Test plan

• ADR file present at docs/decisions/ADR-018-agent-asset-registry.md
• Starlight mirror regenerated at docs/src/content/docs/decisions/Adr-018-agent-asset-registry.md
• Sub-decisions referenced by number are internally consistent (1–13, no gaps)
• References section links resolve (spot-checked in commit)
• Maintainer review + approved label on feat(registry): central agent asset registry for capabilities, skills, plugins, and MCP servers #246
• After approval, undraft and merge as proposed status

Related

Progresses #246. See also #381 (documentation graph — split resolved in sub-decision 13), ADR-014 (workflow-driven tasks — resolver-interface precedent), and the ADR-003 contribution governance model.

[codecov-commenter]

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@32407d5). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #548   +/-   ##
=======================================
  Coverage        ?   90.06%           
=======================================
  Files           ?      241           
  Lines           ?    58836           
  Branches        ?     6318           
=======================================
  Hits            ?    52990           
  Misses          ?     5846           
  Partials        ?        0           

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

🚀 New features to boost your workflow:

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

[scottschreckengaust]

How will the lookup "Registry of registries" be maintained?

For example:

• python: PyPi
• node: npm.js
• container: Docker Hub / ecr / ghr / gcr
• mcp: many - one example is the MCP registry https://registry.modelcontextprotocol.io/ from https://github.com/modelcontextprotocol/registry

Is there an abstraction from the Cloud Computing Native Foundation (CCNF) that can be utilized versus rolling a bespoke system?

[scottschreckengaust]

First off — this is genuinely one of the most thorough, well-reasoned ADRs I've read in this repo, and it was a pleasure to review. 🙌 You did the hard things right: it's template-compliant, it's honest about its own risks (the (!) section is excellent), and it correctly reaches for the repo's twice-learned parity lesson instead of re-discovering it. The contracts/registry-resolution/ proposal mirroring contracts/cedar-parity/ and contracts/workflow-validation/ is exactly the instinct we want to see — I verified both of those directories exist and this slots right in beside them. The four substrate flip-conditions turn "preferred with a fallback" into something falsifiable rather than hand-wavy, and resolving at the create-task boundary keeps you consistent with ADR-014's shipped workflow_ref model. Really strong work, and the effort clearly shows.

Everything below is in service of making an already-good document airtight before it graduates from proposed to accepted — I'm leaving this as a comment, not a change request, since it's a draft awaiting the approved label on #246. Please read the tone as "help me help this land," not "stop." 💛

Two things I'd fix before acceptance (both quick)

1. The "grammar already committed" claim doesn't hold against the shipped regex (inline on the sub-decision). I ran the ADR's own example refs against _REGISTRY_REF at validator.py:50, and the committed regex has no @ in its character class (so @<constraint> can't be parsed) and only allows hyphens in the kind segment (so mcp_server / cedar_policy_module / prompt_fragment don't match — only mcp-server does). This is totally fixable and doesn't weaken the design — it just means the framing "closing the loop rather than opening it" slightly overstates what's shipped. A one-line reframe ("this ADR extends the committed shape") plus a note on where the regex change lands fully resolves it. Details inline.

2. Small off-by-one in the References section (inline). It credits "sub-decision 12" for the #381 split, but 12 is the workflows-don't-migrate decision — the #381 split is 13 (your PR body has it right!). Tiny, but the test-plan box "sub-decisions referenced by number are internally consistent (1–13, no gaps)" is checked, so worth tidying so that claim stays true. Appears in both the source file and the generated mirror, so the fix lands in docs/decisions/ and re-syncs.

A few design-level thoughts (take or leave — you're the author)

These aren't blockers, just things I'd love your read on:

• Altitude. The doc is part ADR, part design spec, part market survey — and it's great diligence — but the ~15-vendor enumeration and the pinned implementation details (error-code strings, Cognito group names) are the most perishable content in an artifact whose whole job is to be durable. Totally your call, but you might keep the invariants + preferred/fallback ranking + flip-conditions here, and let the vendor deep-dive live in the design PR or a dated appendix. The decision survives; the survey ages.
• Governance-in-MVP sizing (inline). Sub-decision 10 is a strong principle ("governance is what separates a registry from a directory" — love it), but a full lifecycle + audit + event integration + auto-approve, on top of a two-language semver resolver and descriptor validation, is a big MVP for a reference project whose AC4 goal is one E2E kind. Might be worth an explicit sentence acknowledging governance may need staging if the estimate tightens.
• AgentCore timing. The ADR is dated 2026-07-08 and the preferred substrate hard-migrates namespaces on 2026-08-06 — likely mid-build. You flag it as a risk and lean on the RegistryClient seam, which is right for code, but the seam doesn't confine data migration / IAM action renames as cleanly as the prose implies. Maybe promote "don't take a production dependency until post-cutover GA" from a risk bullet into the decision framework.
• One status, two names (inline). approved "(also called active)" then both get used interchangeably — in a byte-for-byte two-language contract that's a latent divergence, and it's exactly the parity hazard your own (−) bullet warns about. Pick one canonical token.

Nits

• Two reference URLs look off by host convention (didn't fetch): www.docs.microsoft.com/... (usually learn.microsoft.com) and docs.cloud.google.com/... (usually cloud.google.com/...). Worth a spot-check since the test plan mentions link verification.
• Reserving a capability kind for workflows while sub-decision 12 says workflows won't migrate reads slightly oddly — fine as "declared but not loaded," just a one-clause note would smooth it.

Thank you for the care you put into this — the bones are excellent and none of the above is hard to address. Happy to pair on the grammar reconciliation (#1) if useful, since that's the one place code and contract need to meet. Excited to see this one land. 🚀

[scottschreckengaust]

Small but load-bearing accuracy fix 🙂 — this says the URI regex "matches the shape already committed at validator.py:50," but I tested the ADR's own example refs against the actual committed regex:

committed: ^registry://[a-z][a-z0-9-]*/[a-z0-9][a-z0-9./-]*$

MATCH   registry://mcp-server/acme/pdf-tools
NO      registry://mcp_server/acme/pdf-tools@^1.4.1   ← this ADR's grammar
NO      registry://mcp_server/acme/pdf-tools          ← underscore kind
NO      registry://skill/acme/refactor@~2.0.0

Two gaps:

1. No @<constraint> support — there's no @ in the character class, so the semver constraints from sub-decision 2 have nowhere to live in the shipped grammar.
2. No underscores in the kind segment — it's [a-z][a-z0-9-]* (hyphen only), but every MVP kind here is snake_case (mcp_server, cedar_policy_module, prompt_fragment). Only a hyphenated mcp-server matches today.

None of this hurts the design — it just means _REGISTRY_REF has to be extended to carry this contract. Suggested tweak: reframe as "extends the committed shape (adding @constraint and reconciling kind naming)" and note whether that regex change rides in this ADR's companion or the design PR. Same reasoning applies to the "the code already parses and validates registry:// refs" line down in Consequences. Happy to pair on this bit! 💪

[scottschreckengaust]

Tiny off-by-one: this credits sub-decision 12 for the #381 split, but 12 is "Workflows do not migrate to the registry" — the clean #381 split is sub-decision 13 (and your PR description has it right ✅). Worth fixing because the test-plan checkbox "sub-decisions referenced by number are internally consistent (1–13, no gaps)" is ticked, so this keeps that honest. It's in the generated mirror too, so the fix lands in this source file and re-syncs via mise //docs:sync.

[scottschreckengaust]

Really like the principle here — "governance is what separates a registry from a directory" is the right north star, and putting it up front is a good instinct. My only nudge: this is a lot of MVP (full lifecycle + audit trail + event-driven review integration + auto-approve mode) layered on top of a two-language semver resolver, descriptor validation, and a parity corpus — for a reference project whose stated E2E goal (AC4) is a single asset kind. Not asking you to cut it — just maybe one explicit sentence that governance can be staged if the estimate tightens, so a future implementer has permission to sequence it rather than treating all of (a)–(d) as an atomic MVP gate. Your call entirely. 🙏

[scottschreckengaust]

One canonical-name nudge: approved "(also called active)" introduces two tokens for one state, and sub-decisions 10 and 11 then use both interchangeably (e.g. "in submitted state (or active…)"). Since the resolver contract has to agree byte-for-byte across the TypeScript orchestrator and the Python agent — the exact parity hazard your own (−) bullet flags so well — I'd pick one canonical status string and mention the other only as "(formerly/aka)" prose. Cheap insurance against a sneaky divergence bug. 🙂

[scottschreckengaust]

Verdict

Comment (not a change request). This is an exceptionally thorough, template-compliant ADR and the design is sound: contract-first, substrate-deferred with falsifiable flip-conditions, fail-closed resolution, and it correctly reaches for the repo's twice-learned parity lesson (contracts/registry-resolution/ mirroring the existing contracts/cedar-parity/ and contracts/workflow-validation/, both of which I verified exist). It should not merge yet only because it's a self-declared draft awaiting the approved label on #246 — governance-wise that gate is now satisfied per the review brief. Before it graduates proposed → accepted, two factual defects should be fixed (grammar-parity claim + a cross-reference off-by-one). I independently re-verified the prior maintainer review (@scottschreckengaust) against the current worktree at f3e8972; every claim it makes is still accurate — none are stale.

Vision alignment

Strong fit with VISION.md:

• Fire-and-forget default (tenet 1): assets resolve at the create-task boundary (sub-decision 5) — no live intervention introduced; the submitter path stays asynchronous.
• Bounded blast radius & cost (tenet 2): fail-closed on unresolvable refs (sub-decision 6), semver-only with */latest/>= rejected (sub-decision 2), immutability per version (sub-decision 3), governance lifecycle + coarse Cognito ACL (sub-decisions 10/11). Widening surface is documented, not silent.
• Reviewable outcomes (tenet 8): resolved_assets stamped on every task record for audit + reproducibility; state transitions carry rich audit metadata.
• Tenet trade-offs documented: the (−)/(!) sections are exactly the explicit rationale the tenets require of an architectural decision. No undocumented tenet trade.

Blocking issues

None that block this draft PR. The two items below are fix-before-accepted correctness defects in the design-of-record (I'd re-raise them as blocking if this PR tried to move status to accepted without addressing them):

1. docs/decisions/ADR-018-agent-asset-registry.md:40 / :134 / :162 — the "grammar already committed" claim is factually false against the shipped regex. _REGISTRY_REF at agent/src/workflow/validator.py:50 is ^registry://[a-z][a-z0-9-]*/[a-z0-9][a-z0-9./-]*$. I ran the ADR's own example refs against it:

   • registry://mcp_server/acme/foo@^1.4.1 → no match (no @ in any char class; underscore not allowed in the kind segment)
   • registry://cedar_policy_module/acme/foo → no match (underscore)
   • registry://prompt_fragment/acme/foo → no match (underscore)
   • only registry://mcp-server/acme/foo (hyphen, no constraint) matches.
     So the ADR's MVP kinds (mcp_server, cedar_policy_module) and the @<constraint> grammar (line 34) are not parseable by the committed regex. Lines 134/162 ("the code already parses and validates registry:// refs", "URI grammar already shipped") overstate what exists and would mislead #632+ implementers into thinking no parser change is needed. Fix: reframe as "this ADR extends the committed shape" and add a one-line note that the regex must widen (add @<constraint>, allow _ in the kind segment) in the implementation PR. Risk if unfixed: the design-of-record is inaccurate about its own foundation.

2. docs/decisions/ADR-018-agent-asset-registry.md:152 — off-by-one cross-reference. References credits "sub-decision 12" for the #381 split, but sub-decision 12 (line 62) is workflows-don't-migrate and the #381 split is sub-decision 13 (line 64; the PR body has it right). The test-plan box "sub-decisions referenced by number are internally consistent (1–13, no gaps)" is checked, so this contradicts a checked claim. One-character fix, lands in source + re-syncs to the mirror.

Non-blocking suggestions / nits

• Dual status token (approved "also called active"). Lines 46/58/60 use both interchangeably. In a byte-for-byte two-language contract this is the exact parity hazard the ADR's own (−) bullet warns about — pick one canonical token now while it's cheap.
• Reference host conventions. docs.cloud.google.com/... (line 109) is normally cloud.google.com/...; www.docs.microsoft.com/... (line 171) is normally learn.microsoft.com/.... Worth a spot-check since the test plan claims link verification. (Not fetched.)
• Altitude / perishability. The ~15-vendor survey and pinned implementation strings (error codes, Cognito group names) are the most perishable content in an artifact whose job is durability. Consider keeping invariants + preferred/fallback + flip-conditions here and moving the vendor deep-dive to a dated appendix or the design PR. Author's call.
• Governance-in-MVP sizing. Sub-decision 10 (full lifecycle + audit + event integration + auto-approve) on top of a two-language semver resolver + descriptor validation is a large MVP for a reference project whose AC4 is one E2E kind. A sentence acknowledging governance may need staging if the estimate tightens would be honest.
• AgentCore 2026-08-06 rename. The RegistryClient seam confines code but not data migration / IAM-action renames as cleanly as the prose implies. Consider promoting "no production dependency until post-cutover GA" from a risk bullet into the decision framework.
• Reserved capability kind vs sub-decision 12. Reserving capability (= workflow) while sub-decision 12 says workflows won't migrate reads slightly oddly; a one-clause "declared but not loaded" note smooths it.

Documentation

• ADR follows the docs/decisions/ template (Status/Date/Context/Decision/Sub-decisions/Consequences/References), matching ADR-016/ADR-017 shape. ADR-018 number is free (no collision). Linked ADR-014 and ADR-003 files both exist.
• Starlight mirror is correctly regenerated, not hand-edited. docs/src/content/docs/decisions/Adr-018-agent-asset-registry.md has the generated frontmatter and site-rewritten internal links; after stripping frontmatter and normalizing link targets, the mirror body is identical to the source. CI "Fail build on mutation" will pass. This satisfies the mise //docs:sync requirement.

Tests & CI

• CI is green (Codecov note is a config warning about a missing BASE report, not a failure).
• No unit tests apply — the PR is strictly two docs files; no executable behavior added.
• Bootstrap synth-coverage: N/A for this PR. The review brief's CDK bootstrap-policy check (DynamoDB table, S3 bucket, new Lambda handlers, BOOTSTRAP_VERSION bump, resource-action-map.ts, DEPLOYMENT_ROLES.md golden baseline) applies to the implementation PRs (#632/#633/#634), not to this ADR. I confirmed the diff touches zero CDK/handler/CLI/agent code (git diff --name-only = the ADR source + its mirror only). No constructs changed here, so there is nothing to gate. When #632 lands, that PR MUST carry the full bootstrap bundle update — I'll enforce it there.
• Shared-type sync: N/A. cdk/src/handlers/shared/types.ts is untouched, so no cli/src/types.ts sync is owed by this PR.

Review agents run

• Hand review by principal-architect judgment (primary): appropriate here because the diff is prose — the ADR itself is the reviewable artifact. I empirically validated the two load-bearing factual claims (regex parity via a Python harness against the actual _REGISTRY_REF; sub-decision numbering) rather than trusting prose.
• Omitted — code-reviewer, silent-failure-hunter, type-design-analyzer, pr-test-analyzer: no executable code, no error-handling paths, no new types, no tests in the diff; nothing in their scope to analyze.
• Omitted — /security-review: no IAM, Cedar policy, network, secret, DynamoDB, S3, API-handler, or input-validation code in this diff. Those resources are proposed in the ADR and will be security-reviewed when their code lands in #632+. The security-relevant design assertions (fail-closed resolution, coarse-ACL trust decision, publish-endpoint exposure) are called out in the ADR's own (!) risks and I reviewed them by hand under Vision alignment.
• comment-analyzer sensibility applied by hand: verified the ADR's cross-references and its claims about existing code/contracts dirs match reality — that's how I found blocking-issue #1 and #2.

Human heuristics

• Proportionality — concern (minor): governance-in-MVP + full vendor survey is heavy relative to AC4's one-E2E-kind goal (see nits). Not a blocker for an ADR; flagged for the design PR to right-size.
• Coherence — concern (minor): dual approved/active status token (:46/:58/:60) risks the same-concept-two-terms hazard the ADR itself warns about; and the grammar claim (#1) conflicts with the shipped code. Both fixable.
• Clarity — pass: names communicate intent; error/failure modes surface explicitly (fail-closed, specific reason codes); no defaults hiding failures.
• Appropriateness — pass: maintainable by this team; correctly slots beside existing parity-corpus mechanisms; integration claims mostly verified against real repo state (the one exception is blocking issue #1, where the claim was checked against the real regex and found overstated).

Stack / merge-order note

This ADR is the independent design-of-record (ADR-018) that the implementation PRs #632/#633/#634 build on. Recommended order: land this ADR first (after the two fixes and the approved label lands per the PR's own draft gate), so #632+ have a stable, accurate contract to cite. Critically, fix blocking issue #1 before #632 — otherwise #632's implementers may skip the required _REGISTRY_REF regex widening (add @<constraint>, allow _ in the kind segment) because the ADR told them the grammar was already shipped. The bootstrap-policy, shared-type-sync, and CDK synth-coverage gates all attach to #632, not here.

[scottschreckengaust]

Verified against the shipped regex and this claim is overstated. _REGISTRY_REF at agent/src/workflow/validator.py:50 is ^registry://[a-z][a-z0-9-]*/[a-z0-9][a-z0-9./-]*$. Running the ADR's own examples: registry://mcp_server/acme/foo@^1.4.1 does NOT match — there is no @ in any character class (so @<constraint> can't parse) and no _ in the kind segment (so mcp_server and cedar_policy_module fail; only mcp-server matches). So the committed regex does not yet match this ADR's grammar or MVP kinds. Suggest reframing to "this ADR extends the committed shape" and noting the regex must widen (@<constraint>, underscore in the kind segment) in the implementation PR. Same overstatement recurs at line 134 ("the code already parses and validates registry:// refs") and line 162 ("URI grammar already shipped").

[scottschreckengaust]

Off-by-one: the #381 split is sub-decision 13 (line 64), not 12 — sub-decision 12 (line 62) is "workflows do not migrate." Your PR body has it right. The test-plan box "sub-decisions referenced by number are internally consistent (1–13, no gaps)" is checked, so this needs the one-char fix to keep that claim true. Fix lands in source and re-syncs to the mirror.

[scottschreckengaust]

Nit: approved "(also called active)" then both tokens get used interchangeably here and in sub-decisions 10/11. In a byte-for-byte two-language contract that's the exact parity hazard your own (−) bullet warns about. Pick one canonical status token now while it's cheap.

[isadeks]

Verdict: Request changes

I re-reviewed the current ADR head (f3e8972) against the composed implementation in #632-#634. The existing grammar and cross-reference findings remain valid; the points below are additional design-of-record gaps.

Blocking findings

1. [P1] Choose one resolution owner (:48, :50, :54, :139). Sub-decision 5 says the TypeScript orchestrator resolves refs fully before dispatch, and sub-decision 6 says a running task never re-resolves. Sub-decision 8 and the consequences then require a Python RegistryClient plus (ref, catalog) -> verdict fixtures against every implementation. Those are different architectures. The implementation exposes the contradiction: TypeScript is the only catalog/semver resolver; Python receives an already-resolved bundle and only checks URI syntax. There is no Python RegistryClient and no resolve-*.json corpus. Make the ADR explicit that TypeScript owns resolution and Python validates/loads the wire bundle, or define when Python resolves without violating sub-decisions 5/6. Scope the golden corpus to the implementations that actually own each semantic.

2. [P1] Reconcile the required MVP governance with the tracked and shipped scope (:58-60, :137). This ADR says transitions, rationale-bearing audit history, transition events, and dev-only auto-approval are MVP requirements. Issue #478 describes lifecycle as a follow-up, while the complete #632-#634 stack exposes only publish/resolve/list/show. A normal RegistryPublisher record therefore remains submitted forever, and auto_approve=true is approver-gated but not environment-gated. Either make the transition workflow a merge gate for this stack, or explicitly defer it and remove the claims that governance ships in MVP.

3. [P1] Make the descriptor contract enforceable (:52, :140). The ADR requires tool surface, egress, Cedar actions, minimum compute, and permissions, with JSON Schema as the single source of truth. #632 implements ad hoc TypeScript checks only: several required fields are absent, arrays are not element-validated, and MCP server_config is not checked against its declared transport/egress. No JSON Schema is present or consumed by both languages. Require that contract in the implementation before accepting this ADR, or narrow the ADR to the descriptor guarantees that actually ship.

Verification

The composed CDK registry suites pass, so these are contract and missing-case failures rather than existing test failures.

[Kalindi-Dev]

Self-review (design-of-record pass)

Ran a self-review after addressing the earlier review feedback (@scottschreckengaust, @isadeks), then a verification pass.

Fixed (commit e998a430):

• Dead links failing //docs:link-check (blocking, CI-red). The ADR linked cdk/src/handlers/shared/registry/ref.ts and agent/src/registry/ref.py as relative paths, but those ship in the implementation PRs (feat(registry): agent asset catalog on AgentCore — provisioning, port/adapter, API, CLI (#246) #664/feat(registry): resolve + load registry assets into tasks (#246) #665), not on this branch or main. Fix: demoted both to inline code spans with a note that they land with feat(registry): agent asset catalog on AgentCore — provisioning, port/adapter, API, CLI (#246) #664/feat(registry): resolve + load registry assets into tasks (#246) #665. Mirror regenerated; every remaining relative link resolves; build (agentcore) now green.

@isadeks P1 items re-confirmed resolved against shipped code:

1. Single resolution owner — sub-decision 5 now states the TypeScript orchestrator owns catalog/semver resolution and the Python agent loads the already-resolved bundle (mirrored Python resolver exists for the parity contract only). Verified: resolved_assets stamped in orchestrator.ts.
2. Governance MVP surface reconciled — sub-decision 10 now separates substrate-provided lifecycle machinery from the thin ABCA surface actually shipped (publish + auto_approve + resolve/list/show); standalone transitions / env-gated auto-approve / event consumption named as future scope (feat(registry): asset lifecycle (publish, deprecate, retire, migrate) #478/RFC: Event-Driven Governance and Actions #230). Verified against the route wiring in task-api.ts.
3. Descriptor validation — sub-decision 7 narrowed to native descriptor types (MCP server.json, skill frontmatter, CUSTOM); the rich shared-JSON-Schema capability descriptor is future scope (feat(registry): capability descriptors for dynamic environment config #481).

@scottschreckengaust — thanks for the review; each of your findings addressed:

• Grammar claim overstated (:40/:134/:162) — you ran the ADR's own example refs against the committed _REGISTRY_REF and showed it had no @<constraint> and no _ in the kind segment, so mcp_server/@^1.4.1 didn't actually parse. Reframed from "matches the shape already committed" → "extends the committed shape," and pointed at the authoritative strict grammar in registry/ref.{ts,py}.
• Off-by-one cross-reference (:152) — you caught that it credited sub-decision 12 for the docs(governance): refine ADR-012 to many-to-many ADR↔Persona↔Skill graph with bidirectional backlinks + parity linter #381 split; it's 13 (12 is "workflows don't migrate"). Corrected in source + mirror.
• Dual status token (:46/:58/:60) — you flagged approved "(also called active)" used both interchangeably, a latent parity hazard in a two-language contract. Collapsed to the single canonical approved (substrate APPROVED).
• Reference URL hosts — you spotted docs.cloud.google.com → cloud.google.com and www.docs.microsoft.com → learn.microsoft.com. Fixed.
• Altitude / vendor-survey perishability (your take-or-leave design note) — you flagged the ~15-vendor survey + pinned strings as the most perishable content. Kept as-is by decision (retained as the "why AgentCore won / when to fall back" record), but reconciled the substrate-section intro so it no longer reads as undecided now that AgentCore is chosen.

Status moved proposed → accepted with a dated Changelog per the docs/decisions/README refinement rules.

Note: the substrate moved from DDB+S3 to AgentCore since the last review — the earlier findings were reviewed against #632–#634. @isadeks, when you have a moment, could you re-review / dismiss the stale changes-request? All three P1 items are addressed and verified against the AgentCore implementation.

[krokoko]

Independent review (post–Jul 27 reconcile)

Thanks for the careful reconcile pass (@Kalindi-Dev) — and to @scottschreckengaust / @isadeks for the earlier reviews. I re-read head e998a43 against those threads, #246, and the live #664/#665 track.

Bottom line: the contract is sound, and the Jul 27 edits honestly close Scott’s accuracy nits and isadeks’s three P1s (resolution owner, thin governance surface, native descriptors). I’d still hold merge for a short tightening pass + a human re-review dismissal.

Prior findings — disposition

Item	Status
Grammar “already committed” overclaim	Fixed — extends framing matches main _REGISTRY_REF
#381 → sub-decision 12 off-by-one	Fixed → 13
Dual approved / active	Fixed → canonical approved
Reference URL hosts	Fixed
isadeks P1s (resolve owner / governance MVP / descriptors)	Fixed in prose; aligns with #664/#665 design
Vendor-survey altitude	Open by author choice — fine as decision record
Registry-of-registries / federation (Scott Jul 8)	Still open

Suggested before merge

1. Status: accepted feels early relative to docs/decisions/README.md (accepted when the implementing PR merges). #664/#665 are still open. Prefer proposed here, then bump to accepted with a Changelog once the catalog/resolve PRs land — unless maintainers explicitly waive that for a post-hoc ADR.
2. CHANGES_REQUESTED is still live — the P1s look addressed; when you have a moment, @isadeks, could you re-review / dismiss if you agree? Outdated inline threads can be resolved at the same time.
3. Kind vocabulary map — shipped WORKFLOWS.md / workflow YAMLs still use short forms (registry://mcp/…, prompt/…, cedar/…); this ADR + #664 use mcp_server / prompt_fragment / cedar_policy_module with mandatory @constraint. A one-line alias/migration note (and whether short forms stay lenient-only) would save implementers a dig.
4. Qualify “shipped E2E” until #664/#665 are on main (the demoted ref.{ts,py} paths already show the race). Same for citing #632–#634 vs the AgentCore track.
5. Optional but useful: one Non-goal / sub-decision bullet answering Scott’s federation question (operator-curated catalog; external registries = discovery only until a future ADR), and promote the AgentCore 2026-08-06 cutover into the decision-framework gates given how close it is.

Merge order (suggestion)

Corrected ADR (as proposed) → #664 (catalog) → #665 (resolve/load) → ADR proposed → accepted Changelog pointing at the merged SHAs.

Happy to clarify any of the above — none of this is meant to slow a strong design-of-record, just to keep it accurate the day it hits main.

[scottschreckengaust]

Verdict

Approve (with two minor nits). At head e998a43 this ADR has flipped proposed → accepted and, in doing so, resolved every prior blocking finding from all three earlier reviews. I re-verified each claim against the current text and the shipped code rather than inheriting or dismissing the prior CHANGES_REQUESTED wholesale — the substrate pivot to AWS AgentCore (per #664/#665) genuinely moots the DDB+S3-based P1 findings. The document is now accurate about its own foundation, internally consistent (1–13, no gaps), and the Starlight mirror regenerates byte-identically. This is one of the most disciplined ADR revisions I've reviewed here.

Vision alignment

Strong fit with VISION.md:

• Fire-and-forget (tenet 1): assets resolve at the create-task boundary (sub-decision 5); no live intervention introduced. Sub-decision 5 now names the TypeScript orchestrator as the single resolution owner, keeping sub-decision 6 (no re-resolution) true.
• Bounded blast radius (tenet 2): fail-closed on unresolvable refs (sub-decision 6), semver-only rejecting */latest/>= (sub-decision 2), immutability per version (sub-decision 3), coarse Cognito ACL with documented Phase-3 tightening (sub-decision 11). Surface widening is documented, not silent.
• Reviewable outcomes (tenet 8): resolved_assets stamped per task record for audit + reproducibility.
• Tenet trades documented: the (−)/(!) sections give the explicit rationale the tenets require. No undocumented trade.

Blocking issues

None. All five prior blocking findings are resolved at e998a43:

1. Grammar-parity (prev. blocking, scottschreckengaust): RESOLVED. Sub-decision 1 (:41) now reads "extends the shape pre-declared" and states outright the committed _REGISTRY_REF "admitted a 2-segment registry://kind/name form but not the @<constraint> suffix or _ (snake_case)". Consequences (:143) and References (:171) reframed to match. I independently re-ran the ADR's own example refs against the shipped regex ^registry://[a-z][a-z0-9-]*/[a-z0-9][a-z0-9./-]*$ at agent/src/workflow/validator.py:50: mcp_server/cedar_policy_module/@^1.4.1 all fail, only mcp-server (hyphen, no constraint) matches — exactly as the revised ADR now describes. The overstatement is gone.
2. Off-by-one #381 cross-reference (prev. blocking): RESOLVED. References (:161) now credits sub-decision 13; Changelog (:191) records the fix.
3. [isadeks P1] Resolution-owner ambiguity vs sub-decisions 5/6: RESOLVED by the substrate pivot + rewrite. Sub-decision 5 (:49) now explicitly: TypeScript owns catalog/semver resolution; Python receives the already-resolved bundle and loads it; Python's mirrored RegistryClient exists only for the parity contract and direct lookups. The architecture-vs-architecture contradiction isadeks flagged against #632–#634 no longer holds against this text.
4. [isadeks P1] MVP-governance vs shipped scope: RESOLVED. Sub-decision 10 (:63–:67) now separates substrate-provided machinery from the thin ABCA surface actually shipped (publish/resolve/list/show), and names standalone promote/reject/deprecate, environment-gated auto_approve, and event consumption as future scope tracked in #478/#230. The "submitted forever" and "auto_approve not env-gated" gaps isadeks raised are now stated as known deferrals, not implied guarantees.
5. [isadeks P1] Descriptor-contract enforceability / JSON-Schema single source: RESOLVED. Sub-decision 7 (:55–:57) splits MVP (shipped) native/CUSTOM validation from the future shared-JSON-Schema capability descriptor, explicitly deferred to #481. The consequence bullet (:149) matches. The ADR no longer claims a single-source JSON Schema ships in MVP.

Non-blocking suggestions / nits

1. Accepted-status asserts implementation facts not visible on this branch. The ADR flips to accepted and asserts "shipped" / "proven E2E on a live stack" (:37, :55, :61, :65, :151, :185), but cdk/src/handlers/shared/registry/ref.ts, agent/src/registry/ref.py, and contracts/registry-resolution/ do not exist on this branch — they land in #664/#665. The ADR is honest about this (References :172 explicitly caveats those paths as not-yet-on-main), which is exactly the discipline the original grammar claim lacked, so this is not blocking. But an ADR whose "shipped" claims are unverifiable until sibling PRs merge is mildly incoherent as a design-of-record. Consider either (a) merging this after/with #664/#665, or (b) a one-line header note that accepted is contingent on those PRs landing. Not a blocker: the claims are issue-tracked and caveated.

2. Altitude / perishability (carried from prior review, author's call). The ~15-vendor survey and pinned strings (error codes, Cognito group names, the 2026-08-06 date) remain the most perishable content in a durability artifact. Retaining them as the "why AgentCore won" record is defensible; just flagging that this content will age fastest.

Documentation

• ADR follows the docs/decisions/ template; ADR-018 number is free; linked ADR-003/ADR-014 and WORKFLOWS.md all exist.
• Starlight mirror is correctly generated, not hand-edited. I ran node scripts/sync-starlight.mjs in docs/; the mirror regenerates with an empty git status (no drift) — CI "Fail build on mutation" will pass. Source and mirror bodies are identical after link-rewrite + whitespace normalization (35167 chars each). Frontmatter is the generated title: block only.
• Reference URLs normalized (cloud.google.com, learn.microsoft.com) per the Changelog — verified at :118/:181.
• Issue tracking: #246 carries approved + P0; child issues #478/#479/#480/#481/#230 map 1:1 to deferred sub-decisions.

Tests & CI

• No unit tests apply — strictly two docs files (git diff --name-only = ADR source + its mirror). No executable behavior.
• Bootstrap synth-coverage: N/A. Diff touches zero CDK/handler/CLI/agent code. The bootstrap-policy / BOOTSTRAP_VERSION / resource-action-map.ts / DEPLOYMENT_ROLES.md gates attach to the implementation PRs (#664/#665), not this ADR. When those land they MUST carry the full bootstrap bundle (new AgentCore resource types) + shared-type parity — enforce there.
• Shared-type sync: N/A. cdk/src/handlers/shared/types.ts untouched.
• Cedar engine pin: N/A. No cedarpy/cedar-wasm change.

Review agents run

• Hand review by principal-architect judgment (primary): appropriate — the diff is prose; the ADR is the reviewable artifact. I empirically validated the load-bearing claims: re-ran the shipped _REGISTRY_REF regex against the ADR's example refs; regenerated the Starlight mirror to prove zero drift; confirmed ref.{ts,py} + contracts/registry-resolution/ are absent (matching the ADR's own caveat); confirmed #246 governance labels; verified sub-decision numbering 1–13 with no gaps.
• comment-analyzer (by hand): verified cross-references (#381 → sub-decision 13), the grammar caveat, and "shipped" assertions against actual repo state — this is how I confirmed each prior finding is resolved and surfaced nit #1.
• Omitted — code-reviewer, silent-failure-hunter, type-design-analyzer, pr-test-analyzer: no executable code, error-handling paths, new types, or tests in the diff; nothing in scope.
• Omitted — /security-review: no IAM/Cedar/network/secret/API/input code in this diff. The security-relevant design assertions (fail-closed resolution :51, coarse-ACL trust decision :156, publish-endpoint exposure :156) are called out in the ADR's own (!) risks and reviewed by hand under Vision alignment; their code lands in #664/#665 and must be security-reviewed there.

Human heuristics

• Proportionality — pass (improved): the governance-in-MVP over-scoping the prior review flagged is resolved — sub-decision 10 now right-sizes the shipped surface (publish + auto_approve + read) and defers the rest.
• Coherence — pass with minor concern: same-concept-same-term restored (dual approved/active collapsed to canonical approved, :47/:190). Minor residual: accepted status asserting sibling-PR facts (nit #1).
• Clarity — pass: names communicate intent; failure modes surface explicitly (fail-closed, specific reason codes :51); MVP-vs-future boundaries are unambiguous.
• Appropriateness — pass: maintainable by this team; slots beside the existing contracts/cedar-parity/ and contracts/workflow-validation/ parity corpora (both verified present); integration claims now checked against real repo state, and the one place code and contract must meet (the regex) is accurately described rather than overstated.

Excellent revision — thank you for taking the prior feedback seriously and re-grounding the whole document against what actually shipped. 🚀

[scottschreckengaust]

Nit (non-blocking): status flips to accepted here, but the load-bearing "shipped" facts this ADR now asserts — the strict parity-tested grammar in registry/ref.{ts,py}, the contracts/registry-resolution/ corpus, and the live-stack E2E proof — are not present on this branch; they land in #664/#665. The References caveat at line 172 is honest about this, which is why it's not a blocker. Consider either merging this alongside #664/#665, or adding a one-line note that accepted is contingent on those PRs landing, so the design-of-record isn't asserting unverifiable implementation facts.

[scottschreckengaust]

Verified resolved. I re-ran the ADR's example refs against the shipped _REGISTRY_REF (^registry://[a-z][a-z0-9-]*/[a-z0-9][a-z0-9./-]*$, validator.py:50): mcp_server/...@^1.4.1, cedar_policy_module/..., and mcp_server/... all fail to match; only mcp-server/acme/foo matches. Your reframe ("extends the committed shape … admitted a 2-segment form but not the @<constraint> suffix or _") is now exactly accurate. This closes the prior blocking finding.

[isadeks]

Verdict: Request changes on the current implementation claims

The Jul 27 rewrite does resolve my three earlier prose findings: TypeScript is now the single task-boundary resolver, the ABCA governance surface is explicitly thin, and the rich shared descriptor schema is correctly deferred. I rechecked those rather than carrying the stale review forward.

I still cannot dismiss the change request at head e998a430, because the ADR is now accepted and repeatedly labels behavior as shipped/proven while the exact #664/#665 heads violate those invariants. The executable findings are documented in the current reviews on #664 and #665. Either land those fixes before this ADR is accepted, or keep the ADR proposed/remove the shipped assertions until the implementation matches.

[isadeks]

[P1] This task-visible warning event is not shipped. #665 logs DEPRECATED only through the Lambda logger, then stamps {kind,id,version} and drops warnings from both the task record and agent payload. A user inspecting the task cannot tell that a deprecated asset ran. Persist the warning or emit a durable TaskEvent before retaining this implementation claim.

[isadeks]

[P1] The composed runtime is not fail-closed after resolution. In #665, missing repo dirs, empty MCP runtime, and .mcp.json write errors all return 0; malformed skills become an empty fragment; the pipeline ignores those outcomes and runs after the pin was stamped. A probe also showed two legal MCP names (foo-bar/foo_bar) collapsing to one server key. That is a resolved asset being silently omitted/substituted, contrary to this invariant.

[isadeks]

[P1] The stated shipped validation is broader than the implementation. #664 accepted 201 for array discovery/runtime, empty or wrong-kind MCP runtime, and a string custom flag; CUSTOM receives no equivalent structural validation. Native skill runtime containing an apostrophe generates invalid YAML (Don't caused bad indentation of a mapping entry). Narrow this claim or enforce the runtime/CUSTOM/frontmatter contracts before calling the MVP shipped.

[isadeks]

[P1] Non-auto-approved publish does not reach PENDING_APPROVAL. In #664, SubmitRegistryRecordForApprovalCommand is inside the autoApprove branch. An adapter probe with autoApprove: false executed List/Create/Get/Get and returned DRAFT; there is no ABCA submit endpoint to move it forward. The fix is to submit every successful create and gate only the final approval transition.

[isadeks]

[P1] Reproducibility is not yet guaranteed by the shipped path. #665 cannot clear all Blueprint refs on update (no REMOVE), accepts cross-kind refs in typed fields, and can stamp two MCP assets while its non-injective key mapping writes only one. Re-running from the recorded triples therefore does not prove the same assets were attached. These need implementation guards before this consequence is factual.

[Kalindi-Dev]

Thanks for the thorough review, @scottschreckengaust, @krokoko, @isadeks!
I'm working through the comments individually.

The most common one, about the ADR being marked "Accepted," came from my misunderstanding: I assumed an accepted issue implied the ADR was accepted along with the PR. I'll flip that.