feat(registry): orchestrator + agent + MCP server E2E (#246)

[Kalindi-Dev]

Area

• cdk — infrastructure, handlers, constructs
• agent — Python runtime / Docker image
• cli — bgagent client

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).
• Stacked series — merge in order: ADR docs(decisions): ADR-018 propose central agent asset registry (#246) #548 → PR 1 (feat(registry): design + storage + publish/resolve API (#246) #632) → PR 2 (this) → PR 3 (feat/246-registry-cedar-skills).
• Built on PR 1 (feat(registry): design + storage + publish/resolve API (#246) #632). Until feat(registry): design + storage + publish/resolve API (#246) #632 merges, this PR's diff-against-main also shows PR 1's commits — review the commit 59fe23d for PR-2-only changes.

Changes

Wires the registry (PR 1) end-to-end for one asset kind: a blueprint pins an MCP server, the orchestrator resolves it at the create-task boundary, stamps the resolved {kind,id,version} on the TaskRecord, and the agent merges the resolved server config into .mcp.json.

• Blueprint: assets.mcpServers prop → RepoConfig.mcp_servers column, with a synth-time RegistryRefValidation (rejects floating/malformed refs). Grammar extracted to a dependency-free registry-ref.ts so the construct layer validates without pulling aws-sdk into the synth graph.
• Orchestrator: resolveRegistryAssetsForTask resolves the blueprint's pins before hydration (fail-fast/fail-closed), stamps resolved_assets, threads the bundle into the agent payload. resolved_assets added to TaskRecord/TaskDetail/TaskSummary (+ CLI mirror).
• Agent: registry_loader.py merges the resolved MCP server_config into .mcp.json alongside the channel MCP; threaded server.py → pipeline.py → TaskConfig.
• CLI: bgagent registry publish/resolve/list/show.

Deploy-hardening fixes found validating against a live stack: create RegistryPublisher/RegistryApprover Cognito groups (bootstrap policy + action-map + DEPLOYMENT_ROLES.md updated); CLI --asset-version flag (--version collided with commander's global); memorySize: 256 on the registry Lambdas (128 MB OOM'd on init); mcp_server publish accepts inline server_config in lieu of an artifact; orchestrator Lambda gets REGISTRY_ASSETS_TABLE_NAME env + read grant.

Validated live: resolved_assets correctly stamped on a real TaskRecord.

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.

[scottschreckengaust]

Principal Architect Review — PR #633 (registry orchestrator + agent + MCP E2E, #246, PR 2 of 3)

Review re-pinned to head df3c0f6 (a merge of main into the branch, post-49ba04a); the merge did not touch the reviewed incremental files or my inline anchors.

STACK / MERGE-ORDER (read first)

This PR is stacked on #632 — commit 49ba04a sits directly on #632's 773764f. GitHub reports base=main, so the full diff-vs-merge-base re-includes all of #632. I reviewed the incremental layer (orchestrator wiring + agent registry_loader.py + MCP E2E + blueprint assets.mcpServers + CLI registry commands + Cognito groups) and only sanity-checked integration against the #632 base.
Merge order is mandatory: ADR #548 → #632 (PR 1) → this (#632-dependent) → PR 3. Do NOT merge this before #632; its diff/tests assume #632's constructs (registryAssetsTable, resolver, publish/resolve handlers) are present.

1. Verdict — APPROVE (with nits)

The incremental layer is well-structured, fail-closed, and matches the #246 MVP acceptance criteria (blueprint pins one asset kind → orchestrator resolves at create-task boundary → stamps resolved_assets on the TaskRecord → agent merges resolved MCP server_config into .mcp.json). Grammar parity, IAM, and bootstrap coverage are handled. The one CI failure is inherited and by-design (see §6). Nothing blocking in this layer.

2. Vision alignment

• Bounded blast radius / fail-closed — resolveRegistryAssetsForTask resolves BEFORE hydration; a RegistryResolutionError propagates (no partial resolution), emits a registry_resolution_failed audit event, and fails the task the same way a guardrail block does (ADR-018 sub-decision 6, VISION fail-closed tenet). resolveAll refuses reserved kinds that have no loader rather than silently dropping them. Good.
• Reviewable outcomes — the {kind,id,version} audit triples on the TaskRecord + registry_assets_resolved/…failed events keep 'what asset versions ran' inspectable (issue AC: reproducibility).
• Policy-gated escalation — publish/auto-approve is gated on RegistryPublisher/RegistryApprover Cognito groups; the fire-and-forget submit path is unchanged (registry resolution is a no-op returning the empty bundle when a blueprint pins nothing).
• Tenet trade-offs — the DDB+S3 substrate choice (vs AgentCore Registry) is documented in ADR-018/REGISTRY.md (#632). No undocumented tenet trade in this layer.

3. Blocking issues

None in the incremental layer.

4. Non-blocking suggestions / nits

1. BOOTSTRAP_VERSION not bumped (advisory). cdk/src/bootstrap/version.ts stays at 1.2.0 while this PR adds 4 cognito-idp:*Group actions to the application policy and a new AWS::Cognito::UserPoolGroup resource-action-map entry. The review bar (ADR-002) asks for a minor bump when adding permissions (→ 1.3.0). The generated artifacts (application.json, bootstrap-template.yaml, BOOTSTRAP_HASH) WERE correctly regenerated and the hash/golden-baseline tests pass — so this is a semver-hygiene nit, not a functional gap (the deploy role will work). Recommend bumping so operators can tell the bundle changed. (Applies to the stack: #632 also added DDB/S3-backed constructs, though those reuse resource types already in main's action map.)
2. agent/src/registry_loader.py:52 _mcp_server_key docstring drift (comment-analyzer). The docstring says it 'Prefer[s] the descriptor's tool_prefix stripped of the mcp__ scaffolding … falling back to namespace-name', but the implementation only ever computes namespace-name — tool_prefix is never read. Either wire the tool_prefix preference or drop that sentence so the comment matches the code.
3. Non-paginated resolver Query (base #632 — note only, not a #633 blocker). cdk/src/handlers/shared/registry-resolver.ts resolveRef issues a single QueryCommand over all versions of a pk with no LastEvaluatedKey loop. DynamoDB caps a Query page at 1 MB; an asset that accumulates enough version rows to exceed a page could resolve to a lower version or NO_MATCHING_VERSION silently. Realistically far off (needs thousands of versions), and the file is base-layer — raise on #632, not here.
4. MCP key collision surface (minor). Registry keys derive from namespace-name; the channel MCP uses fixed keys (linear-server, etc.), so a collision is unlikely today, but a published acme-linear-server could shadow a channel entry in .mcp.json. A brief note or a registry- prefix would make the namespaces provably disjoint.

5. Documentation

• docs/design/DEPLOYMENT_ROLES.md (golden baseline) updated with the 4 cognito group actions; the Starlight mirror docs/src/content/docs/architecture/Deployment-roles.md is regenerated and in sync (ran node docs/scripts/sync-starlight.mjs → no drift; git status clean under docs/src/content/docs/).
• REGISTRY.md + Registry.md mirror are #632 files. No new docs required for this incremental layer beyond the deployment-roles delta, which is present.

6. Tests & CI (incl. bootstrap synth-coverage)

• CI red is INHERITED & by-design. The failing build (agentcore) job fails at //docs:link-check: design/REGISTRY.md → ../decisions/ADR-018-agent-asset-registry.md returns Status 400 because ADR-018 lives in unmerged PR #548 (docs/decisions/ADR-018* does not exist on this branch). REGISTRY.md is a #632 base-layer file, unchanged in the incremental diff — this is NOT introduced by #633. It self-resolves once #548 merges to main (the PR description acknowledges this). Not a #633 blocker; it IS a merge-ordering gate.
• Incremental layer builds & tests clean (verified locally in the worktree): tsc --noEmit (cdk) clean; CDK registry suites 252 tests pass (blueprint, task-api incl. the two new Cognito-group assertions, orchestrator-registry, registry-descriptor/-resolver/-publish/-list-show, registry-assets-table, registry-artifacts-bucket, agent stack); bootstrap suite 91 tests pass (synth-coverage, golden-baseline, artifact-sync hash, version); CLI registry.test.ts 7 pass; agent test_registry_loader.py + grammar-corpus + workflow-validator 64 pass; scripts/check-types-sync.ts OK (CLI ↔ CDK type mirror validated; the 6 new registry envelopes are allowlisted).
• Bootstrap synth-coverage: PASS. New AWS::Cognito::UserPoolGroup → cognito-idp:CreateGroup mapped; Create/Delete/Get/Update group actions added to application.ts + regenerated application.json + bootstrap-template.yaml; BOOTSTRAP_HASH regenerated (artifact-sync test green). Only gap is the version-string bump (§4.1).
• Test quality: covers happy path + failure paths — fail-closed propagation + failure-event emission, deprecated-resolution warning, empty-bundle no-op (no DDB write), inline-server_config-without-artifact accept vs cedar-module-still-requires-artifact reject, malformed/absent .mcp.json tolerance, missing-server_config skip, unknown-kind + malformed-id CLI rejection, and synth-time rejection of floating/malformed refs. Grammar parity is corpus-enforced across Python and TS.

7. Review agents run

• Manual principal-architect deep review — full incremental diff (orchestrator, registry_loader, registry-ref/-descriptor, blueprint, task-api, gateway, CLI, types).
• Security review — performed directly against the incremental code (the packaged /security-review skill misfired on a stale git context — it captured main/#596 instead of this worktree — so I did the pass by hand): traced the .mcp.json merge (no path traversal — fixed write target, JSON-key not a path, operator-published approved content), Cognito authz (extractGroups fail-closed → 403; auto_approve requires Approver; JWT claims server-verified), IAM least-privilege (group actions on the deploy role only; orchestrator gets read-only grantReadData), and fail-closed resolution. No HIGH/MEDIUM findings.
• code-reviewer / silent-failure-hunter / type-design-analyzer / comment-analyzer / pr-test-analyzer — folded into the manual pass (findings: nit §4.2 comment drift from comment-analyzer; type-design confirmed sound — payload carries full ResolvedAssetBundle, TaskRecord stamps lightweight ResolvedAssetSummary[], CLI mirror in sync; silent-failure — the resolved_assets stamp write failure is logged loudly and deliberately non-fatal, resolution failures propagate; test-analyzer — coverage is thorough incl. failure paths). The plugin agents' own subprocess dispatch inherited the same stale-git context as the security skill, so I could not get clean isolated plugin output; I state that here per the mandatory-agents requirement rather than parrot a misfired run.

8. Human heuristics

• Proportionality — PASS. Extracting the grammar into a dependency-free registry-ref.ts (so the construct layer validates at synth without dragging aws-sdk into the synth graph) is proportional and well-justified; the re-export from registry-resolver.ts keeps existing importers working. Cedar/skill loaders are honest logged-only stubs deferred to PR 3, not speculative machinery.
• Coherence — PASS. Same concept, same term: resolved_assets bundle vs summary shapes are consistent CDK↔CLI↔agent; the .mcp.json read-merge-write mirrors channel_mcp.py intentionally (documented). Registry work lands in the right packages per the AGENTS.md routing table.
• Clarity — mostly PASS. Names communicate intent; error handling surfaces failures (fail-closed + audit events) rather than hiding them. One drift at _mcp_server_key docstring (§4.2).
• Appropriateness — PASS. Integration verified against real DDB semver semantics (in-code semver ranking because DDB sorts version SKs lexicographically — a real, correctly-handled hazard); tests assert what the code should do (fail-closed, immutability, grammar rejection), not just what it does. Validated live per the PR description (resolved_assets stamped on a real TaskRecord).

[scottschreckengaust]

Nit (comment-analyzer): this docstring says the key 'Prefer[s] the descriptor's tool_prefix stripped of the mcp__ scaffolding … falling back to namespace-name', but the implementation below only ever computes namespace-name — tool_prefix is never read. Either wire the tool_prefix preference or drop that sentence so the comment matches the code. Non-blocking.

[scottschreckengaust]

Bootstrap coverage is otherwise correct here (new AWS::Cognito::UserPoolGroup → cognito-idp:CreateGroup, group actions added to the application policy, artifacts + BOOTSTRAP_HASH regenerated, golden baseline updated — 91 bootstrap tests green). One advisory gap: BOOTSTRAP_VERSION in cdk/src/bootstrap/version.ts stays at 1.2.0 despite added permissions; ADR-002 asks for a minor bump (→1.3.0). Not test-enforced, so non-blocking, but recommended so operators can see the bundle changed.