feat(registry): design + storage + publish/resolve API (#246)

[Kalindi-Dev]

Area

• cdk — infrastructure, handlers, constructs
• agent — Python runtime / Docker image
• cli — bgagent client
• docs — guides or design sources (docs/guides/, docs/design/)

Related

• Progresses feat(registry): central agent asset registry for capabilities, skills, plugins, and MCP servers #246 (central agent asset registry).
• Decision record: ADR-018 (PR docs(decisions): ADR-018 propose central agent asset registry (#246) #548) — this PR depends on it; the REGISTRY.md → ADR-018 link fails docs:link-check until docs(decisions): ADR-018 propose central agent asset registry (#246) #548 lands on main.
• Stacked series — merge in order: ADR docs(decisions): ADR-018 propose central agent asset registry (#246) #548 → PR 1 (this) → PR 2 (feat/246-registry-orchestrator-mcp) → PR 3 (feat/246-registry-cedar-skills).

Changes

First of three PRs. Purely additive infrastructure — nothing in ABCA reads from the registry yet.

• Design: docs/design/REGISTRY.md — asset-kind catalog, DDB/S3 schema, publish/resolve/list/show API contract, resolution semantics (semver, immutability, status), and the upstream-registry/federation stance (out of MVP scope).
• Substrate: DynamoDB + S3 (ADR-018 fallback option 2) — AgentCore Registry is in preview + hard-migrates namespaces 2026-08-06, so the four flip-conditions select the fallback at authoring time. The RegistryClient seam keeps the AgentCore swap open.
• Grammar: extend _REGISTRY_REF to registry://kind/namespace/name@constraint (snake_case kinds, mandatory semver pin) on both the Python validator and the TS resolver, with a contracts/registry-resolution/ parity corpus proving both agree byte-for-byte.
• Types: shared RegistryAssetKind/Status/Ref/Descriptor/Record, ResolvedAsset(Bundle/Summary) in cdk types.ts, mirrored in cli types.ts. Canonical status token is approved (no active).
• Resolver: registry-resolver.ts — parseRef / resolveRef / resolveAll, semver ranked in code (DDB sorts versions lexicographically), fail-closed with specific reasons.
• Storage: RegistryAssetsTable (pk/sk + kind-index GSI) and RegistryArtifactsBucket (versioned, TLS-only, SSE) constructs.
• API: publish/resolve/list/show handlers (immutability 409, descriptor validation, Cognito RegistryPublisher/RegistryApprover group gating) + /registry routes on TaskApi with least-privilege IAM.

Tests: full cdk + agent suites green. Only the known docs:link-check (ADR-018 cross-PR link) fails.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

[krokoko]

Qq: this seems like a lot of infra addition for the registry, isn't it worth it to wait for agentcore registry ?

[scottschreckengaust]

Verdict: Request changes

Strong, well-documented foundational layer for the #246 asset registry — the DDB/S3 substrate, immutability guard, fail-closed resolver, Cognito group gating, byte-for-byte grammar parity corpus, and IAM least-privilege are all high quality. One blocking correctness bug in the resolver's status-vs-version selection contradicts the documented spec (REGISTRY.md §5) and, because this is the stack base that PR 2's orchestrator resolve step builds on, it should be fixed here rather than propagated. The red CI is a known stacked-ordering artifact (not a defect in this code) — noted below so it is not mistaken for a real break.

Vision alignment

Fits the north star well. Purely-additive infra (nothing reads from the registry yet) keeps blast radius bounded; resolution is fail-closed (mandatory pins, no floating latest, REGISTRY_RESOLUTION_FAILED on any unresolved ref), which directly serves "bounded blast radius" and "reviewable/reproducible outcomes" (resolved versions become an audit triple). The tenet trade-off (DDB+S3 fallback vs AgentCore Registry) is documented in REGISTRY.md and deferred to ADR-018. The maintainer question on the thread ("worth waiting for AgentCore registry?") is answered by the four fallback conditions + the RegistryClient seam that keeps the AgentCore swap open — a reasonable, documented call.

Blocking issues

B1. Resolver picks the highest version then checks status, instead of resolving the highest resolvable version — contradicts REGISTRY.md §5 (cdk/src/handlers/shared/registry-resolver.ts:206-215).

REGISTRY.md §5 states submitted/draft/rejected are "not a candidate; if it's the only match → NO_MATCHING_VERSION" — i.e. these statuses should be excluded from ranking so a lower approved/deprecated version can still resolve. The code filters matching only by semver constraint + valid-version (status is NOT in the filter), sorts descending, takes matching[0], and only then evaluates status in statusWarnings. Consequence: publish 1.2.0 as submitted (the normal publisher path — it lands submitted), and a task pinned ^1.0.0 that previously resolved to 1.0.0 approved now fails with NO_MATCHING_VERSION — an unapproved newer version silently shadows a working approved pin. The code's own comment at line 254 ("no lower approved version was selected") describes a fallback the code never performs. Suggested fix: filter to resolvable statuses (approved + deprecated) before ranking, then pick the highest of those; keep the special case that a removed version which is the highest match overall still yields the distinct REMOVED reason (tombstone visibility) rather than being silently skipped. Add resolver tests for [1.0.0 approved, 1.2.0 submitted]@^1.0.0 → 1.0.0 and [1.0.0 approved, 1.2.0 removed]@^1.0.0 → REMOVED.

Non-blocking suggestions / nits

• N1 — Test gap: exact prerelease pin resolution. The grammar + corpus accept @1.4.1-rc.1 (contracts/registry-resolution/grammar-valid-prerelease.json) but there is no resolveRef test that a matching 1.4.1-rc.1 row actually resolves under semver.satisfies(..., { includePrerelease: false }). It should (exact comparator carries the prerelease at the same tuple), but the grammar permits the pin so the resolve path deserves a guard test. cdk/test/handlers/shared/registry-resolver.test.ts.
• N2 — Comment/spec drift on status_history (cdk/src/handlers/registry-publish.ts:134). RegistryStatusEvent doc says "rich actor+timestamp+rationale is a MUST", but the initial publish event omits rationale. rationale is optional in the type, so this is only a doc-vs-code wording mismatch — soften the type comment or note the initial event has no rationale.
• N3 — extractGroups string fallback (cdk/src/handlers/shared/gateway.ts). The bracket-strip + split-on-comma-or-whitespace fallback assumes API-GW stringification; it is exercised only for the authz gate (a group name containing a space would be mis-split, but Cognito group names disallow spaces). Fine to keep; a one-line test on the array path + the stringified path would lock the contract.

Documentation

• docs/design/REGISTRY.md is thorough (schema, API contract, resolution semantics, grammar, security/ACL, out-of-scope federation) and the Starlight mirror docs/src/content/docs/architecture/Registry.md is in sync (ran mise //docs:sync in the worktree — zero drift). New ErrorCodes and the registry:// grammar are documented.
• CI RED is expected here: the only dead link is docs/design/REGISTRY.md → ../decisions/ADR-018-agent-asset-registry.md (verified via docs:link-check). ADR-018 (#548) is still OPEN/unmerged and the file does not exist on this branch, so docs:link-check fails by design per the PR body's stated merge order (#548 → this PR). Not a defect in this diff. The Secrets/deps red is a pre-existing svgo CVE (GHSA-2p49-hgcm-8545) inherited via the docs/astro tree from the main-merge — not introduced by this PR.
• ROADMAP.md has no registry line item; #246 references a roadmap section — a one-line update would be nice but is non-blocking.

Tests & CI

• CDK: ran the registry suites in the worktree — 147 tests pass (handlers publish/resolve/list/show, both constructs, descriptor + resolver, agent stack). tsc --noEmit clean; cdk:eslint --fix clean (no mutations). Agent: grammar parity + workflow validator = 53 pass.
• Coverage is good on the happy + failure paths (401/403/400/409/422/404, immutability guard, fail-closed table-name, invalid-ref-never-queries). Gaps: the B1 status-fallback case (untested — masks the bug) and N1 prerelease resolution.
• Bootstrap synth-coverage: PASS / not-applicable. This is the key CDK check for new resource types. Verified AWS::DynamoDB::Table (resource-action-map.ts:66) and AWS::S3::Bucket (line 93) are already in cdk/src/bootstrap/resource-action-map.ts and the policy bundle (the stack already deploys many DDB tables + S3 buckets). No new CFN resource type is introduced (the four registry Lambdas are AWS::Lambda::Function, already covered), so no BOOTSTRAP_VERSION bump / artifact regen is required — correctly omitted. The registry table/bucket use the same L2 constructs + IAM grant patterns as existing resources, so the CFN-exec-role ARN patterns already cover them.
• IAM least-privilege on the four Lambdas is correct: publish = table RW + bucket grantPut; resolve = table read + bucket read; list/show = table read only. S3 reads are only via short-lived (300s) presigned GETs from the authn'd resolve handler; the S1 access-log nag suppression is justified for an admin-published catalog.

Review agents run

• /security-review — invoked; the skill mis-detected git state (local main, not the worktree), so I performed the security pass manually over the actual registry diff. Findings: no HIGH/MED. kind/namespace/name are regex-constrained (^[a-z][a-z0-9-]*$ / ^[a-z0-9][a-z0-9._-]*$) and cannot contain /, so no path traversal into the {kind}/{namespace}/{name}/{version}/artifact S3 key or the DDB pk; DDB queries use parameterized ExpressionAttributeValues (no NoSQL injection); base64 artifact decode is bounded by API-GW payload limits; publish gated on RegistryPublisher/RegistryApprover, auto_approve gated on approver; presigned URLs are short-lived and key-scoped.
• code-reviewer — ran (manual, guidelines/style): clean; L2 constructs, sane removal policies (DESTROY default with documented RETAIN-for-prod rationale), no hardcoded ARNs/accounts.
• silent-failure-hunter — ran: found B1 (fail-closed in the wrong direction — shadows an approved version); handlers otherwise fail closed with specific reasons and 500-wrap unexpected errors.
• type-design-analyzer — ran: shared types are clean and CDK↔CLI mirrored; check-types-sync.ts allowlist correctly updated for the server-only RegistryAssetRecord/RegistryStatusEvent.
• pr-test-analyzer — ran: strong coverage; gaps N1 + the B1 case.
• comment-analyzer — ran: N2 (status_history rationale) and the line-254 "no lower approved version" comment that describes behavior the code lacks (part of B1).
• Omitted: none in scope.

Human heuristics

• Proportionality — Pass. Complexity matches a foundational registry; the two-language RegistryClient seam + parity corpus is justified by the real drift hazard, not over-abstraction.
• Coherence — Pass. Same approved token byte-for-byte across TS resolver, Python loader, and DDB; pk/sk/artifactKey helpers are single-sourced; constructs mirror existing bucket/table patterns.
• Clarity — Concern. The line-254 comment (registry-resolver.ts) claims a lower-approved fallback the code does not implement (B1) — a name/behavior mismatch that would mislead the PR-2 author.
• Appropriateness — Pass. Maintainable by this team; resolver semantics verified against real node-semver behavior (lexicographic-DDB-sort caveat handled by ranking in code), not only self-written mocks.

Stack / merge-order note

This is the base of the #246 series (ADR #548 → this PR → PR 2 orchestrator/MCP → PR 3 Cedar/skills). Two consequences: (1) merge #548 first so docs:link-check goes green — do not merge this ahead of the ADR. (2) Fix B1 in this PR, not in PR 2 — PR 2's orchestrator resolve step consumes this exact resolveRef/resolveAll; shipping the wrong status-selection semantics into the foundation would silently break approved-pin resolution for every downstream consumer.

[scottschreckengaust]

Blocking (B1): this filter selects candidates by semver constraint + valid-version only — status is not considered until matching[0] is chosen below. REGISTRY.md §5 says submitted/draft/rejected are "not a candidate", meaning they should be excluded from ranking so a lower approved/deprecated version still resolves. As written, publishing a newer submitted version (the normal publisher landing state) shadows an existing approved pin and makes ^1.0.0 fail with NO_MATCHING_VERSION. Filter to resolvable statuses (approved+deprecated) before ranking; keep the case where a removed version that is the overall highest match still surfaces the distinct REMOVED reason.

[scottschreckengaust]

This comment says "no lower approved version was selected", but the code never attempts to select a lower version — matching is not status-filtered before matching[0], so no fallback exists. Comment describes intended behavior the implementation lacks (see B1).

[scottschreckengaust]

Nit (N2): RegistryStatusEvent's doc calls rationale a MUST, but the initial publish event omits it. rationale is optional in the type so this is fine at runtime — soften the type comment or note that the initial submitted/approved event carries no rationale.