feat(registry): Cedar policy modules + skills support (#246)

[Kalindi-Dev]

Area

• cdk — infrastructure, handlers, constructs
• agent — Python runtime / Docker image
• 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) — completes all six acceptance criteria.
• 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 (feat(registry): orchestrator + agent + MCP server E2E (#246) #633) → PR 3 (this).
• Built on PR 2 (feat(registry): orchestrator + agent + MCP server E2E (#246) #633). Until feat(registry): orchestrator + agent + MCP server E2E (#246) #633 merges, this PR's diff-against-main also shows PR 1+2's commits — review commit b3cf5bf for PR-3-only changes.

Changes

Completes the MVP — the two remaining asset kinds now load end-to-end.

• Resolver: ResolvedAsset gains a substrate-agnostic content field, populated inline from the descriptor (cedar_text / prompt_fragment). The seam confines a future S3/AgentCore-backed fetch to the resolver — orchestrator/agent never depend on where the bytes live.
• Cedar policy modules: resolved text is merged orchestrator-side into the existing cedar_policies payload, so registry Cedar is byte-identical to inline blueprint policies and flows through the same PolicyEngine path (parity holds by construction).
• Skills: resolved prompt_fragment is appended to the system prompt (prompt_builder._registry_skill_addendum); tool_hints render as advisory prose (a skill cannot invoke tools — no transitive MCP dependency).
• Blueprint: assets.cedarPolicyModules + assets.skills props → RepoConfig columns with synth-time ref validation; orchestrator now collects all three ref kinds. Mixed inline-cedar + registry-cedar coexistence tested.
• Publish: cedar_text / prompt_fragment required inline (64 KiB cap); inline content satisfies the artifact requirement. Fixes the publish handler to skip the S3 upload when no artifact_b64 is present (inline kinds) instead of Buffer.from(undefined).
• Docs: REGISTRY.md §2 updated (all kinds implemented, content seam, cedar-parity note).

Validated live: a task resolved and stamped all three kinds (mcp_server, cedar_policy_module, skill) on the 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]

Verdict: Approve (with minor doc nits) — stacked, merge AFTER #548 → #632 → #633

PR 3 of 3 for the #246 central agent asset registry (ADR-018). It completes the MVP: cedar_policy_module and skill now load end-to-end alongside PR 2's mcp_server. The incremental diff (18 files, +533/-70) is small, cohesive, well-tested, and — importantly for the security-critical Cedar path — fail-closed by construction. I verified this against the current worktree (all agent + CDK tests green, tsc/ruff/ty clean, docs mirror in sync). No blocking issues in this layer.

Deep review focused on the incremental diff (7ade5df on top of #633/#632). #632 and #633 are under separate review and are not re-judged here.

Vision alignment

Advances the registry vision (decouple asset authoring from CDK deploys) while respecting the bounded-blast-radius tenet: registry-sourced Cedar reaches the agent through the same cedar_policies payload field as inline blueprint policies (orchestrator.ts merge → runner.py:288 extra_policies= → PolicyEngine), and extra_policies are force-wrapped as @tier("soft") and reject any @tier/@rule_id self-annotation (agent/src/policy.py:891-899). Net effect: a registry cedar_policy_module can only tighten policy (add soft-deny/HITL gates) — it cannot inject a hard permit, override a built-in rule, or widen authority. Parity holds by construction, exactly as the ADR-018 seam claims. Skills are prompt-text only (tool_hints are advisory prose, no transitive MCP dependency), so no tool-surface expansion. The 64 KiB publish cap + the engine's POLICIES_MAX_BYTES cap bound the payload.

Blocking issues

None in the incremental (#634) layer.

Non-blocking suggestions / nits

1. Stale doc comment — cdk/src/handlers/shared/registry-descriptor.ts:47 still says KINDS_REQUIRING_ARTIFACT = "kinds whose artifact bytes are required at publish"; after this PR, inline cedar_text/prompt_fragment satisfy the requirement, so the constant now really means "kinds that MAY carry loadable bytes / get a presigned URL" (its live use is registry-resolve.ts:78). Tighten the wording.
2. Stale doc comment — hasInlineDescriptorContent header at registry-descriptor.ts:137-138 says "Today only mcp_server supports this", but the function now handles all three publishable kinds. Update.
3. Publish-time Cedar semantics not validated — validateInlineContent checks presence/type/size of cedar_text but not that it parses as Cedar. Malformed text fails fail-loud at task start (soft-deny policy validation failed → FAILED task), which is acceptable fail-closed behavior and matches inline-blueprint behavior — but publish-time parse-validation would move the failure left. Consider as a follow-up.

Documentation

• docs/design/REGISTRY.md §2 updated (all three kinds implemented, content seam, cedar-parity note). Clear and accurate.
• Starlight mirror docs/src/content/docs/architecture/Registry.md regenerated and in sync — I ran node scripts/sync-starlight.mjs; it produced no drift (clean git status). Not hand-edited. CI's "Fail build on mutation" will pass on the mirror.
• Shared-type sync verified: ResolvedAsset.content?: string present and byte-identical in cdk/src/handlers/shared/types.ts:180 and cli/src/types.ts:105.

Tests & CI

• Tests (verified in worktree): agent test_registry_loader.py + test_registry_skill_prompt.py = 18 passed; CDK blueprint / registry-publish / orchestrator-registry / registry-descriptor = 87 passed. blueprint.ts 100% coverage. Coverage includes failure paths: size-cap rejection, missing content, mixed inline+registry cedar coexistence, the no-S3-upload regression (Buffer.from(undefined) fix), and three-kind stamping. Good failure-path discipline (not just happy path).
• Bootstrap synth-coverage: NOT APPLICABLE to this layer. The incremental diff touches no bootstrap files and introduces no new CFN resource types — the blueprint change adds only DynamoDB item columns (cedar_policy_modules/skills), not new resources. The new table/bucket/handlers live in #632, where the bootstrap check applies. Confirmed 0 bootstrap references in the incremental diff.
• CI is RED — both failures are INHERITED / environmental, NOT introduced by this Cedar/skills layer:
  1. build (agentcore) → //docs:link-check exit 123, 1 dead link: ../decisions/ADR-018-agent-asset-registry.md (Status 400). Root cause: ADR-018 (PR #548) is not yet on main. The link was introduced by 773764f (#632, PR 1) — 0 occurrences in #634's incremental diff — and the PR description explicitly calls this out as red-by-design until #548 merges. Resolves automatically once the base merges.
  2. Secrets, deps, and workflow scan → security:deps osv-scanner flags svgo 4.0.1 (GHSA-2p49-hgcm-8545, fix 4.0.2), a docs/Starlight transitive dep in yarn.lock. It is present on main (line 8060) and unchanged across the whole stack — a post-lockfile CVE, orthogonal to this PR. (Heads-up: merge-queue re-scan can eject the PR on this; a separate yarn.lock bump on main clears it.)
• Cedar engine pins UNCHANGED — cedarpy==4.8.4 / @cedar-policy/cedar-wasm 4.8.2 are untouched (the 4 grep hits are comment prose). No pin move ⇒ no parity-fixture refresh required. Good.

Review agents run

• code-reviewer (style/guidelines): ran — ruff, ty, tsc (cdk + cli), and full test suites clean; blueprint parallel-structure follows the existing mcp_servers pattern verbatim.
• silent-failure-hunter: ran — validation is fail-loud, resolver fail-closed, Cedar validation raises at task start; prompt_builder._registry_skill_addendum skips malformed skills (tested) rather than defaulting silently. No swallowed errors.
• type-design-analyzer: ran — ResolvedAsset.content optional & synced; RepoConfig/BlueprintConfig additions consistent with existing shape.
• pr-test-analyzer: ran — new behavior and failure paths covered; assertions test intended contract (e.g. no-S3-upload, byte-identical cedar merge).
• comment-analyzer: ran — surfaced nits #1 and #2 (stale comments).
• /security-review: ran (REQUIRED for Cedar) — see Vision alignment; conclusion: fail-closed, registry Cedar can only add soft-deny, cannot widen authority.
• Omitted: none in scope.

Human heuristics

• Proportionality — Pass. Change is minimal for the outcome; the content seam is a one-field abstraction with a documented future-swap rationale, not speculative over-engineering.
• Coherence — Pass. New columns/props mirror the existing mcpServers pattern exactly (blueprint.ts); same term (cedar_policy_modules, skills) used consistently across CDK↔CLI↔docs↔agent.
• Clarity — Pass with nits. Names communicate intent; two stale comments (registry-descriptor.ts:47, :137-138) lag the behavior.
• Appropriateness — Pass. Integration verified against the real PolicyEngine path (extra_policies wrapping), not only mocks; the parity claim is substantiated by the actual soft-deny wrapping in policy.py.

Stack / merge-order note

This is the top of a 4-deep stack. Merge order is strict: ADR #548 → PR 1 (#632) → PR 2 (#633) → PR 3 (this, #634). Do NOT merge #634 before #548/#632/#633 — the docs:link-check red clears only when ADR-018 lands on main. This layer itself is sound and ready; approval is contingent on the base stack merging first.

[isadeks]

Verdict: Request changes

I independently re-reviewed current head 20616df, including the assumptions inherited from the approved lower layer. The Cedar/skill additions are not bounded end-to-end.

Blocking findings

1. [P1] Registry Cedar bypasses the advertised aggregate policy cap (cdk/src/handlers/shared/orchestrator.ts:478-490). These modules are merged into cedar_policies and reach PolicyEngine(extra_policies=...), but policy.py:905-912 computes the 64 KiB cap from blueprint_hard_policies and blueprint_soft_policies only. It excludes extra_policies. A direct probe initialized the engine with 2,000 registry-path policies (72,000 bytes) and produced 2,006 soft rules despite POLICIES_MAX_BYTES == 65,536. Count all extra_policies in the aggregate before wrapping/parsing, and add an aggregate multi-module test.

2. [P1] Per-item limits do not bound the resolved bundle or prompt (registry-resolver.ts:148-158, orchestrator.ts:484-490, agent/src/prompt_builder.py:130-145). Blueprint ref counts are unbounded. Each valid 64 KiB skill/Cedar body remains inside descriptor and is copied to content; Cedar is copied again into cedar_policies. Skills are concatenated into the system prompt with no aggregate byte/token budget. Multiple individually valid assets can therefore exhaust durable-invocation payload, Cedar parser, or model-context budgets. Add synth-time and runtime aggregate count/byte limits, avoid duplicating inline text in the wire shape, and fail before dispatch with a specific error.

3. [P1] The new Blueprint fields cannot be cleared (cdk/src/constructs/blueprint.ts:381-430). cedar_policy_modules and skills are only included in UpdateExpression when non-empty. Removing all pins leaves the prior DynamoDB values in place, so tasks continue loading policies or prompt instructions that the deployed Blueprint no longer declares. Emit REMOVE clauses or explicit empty lists and test non-empty -> empty updates.

4. [P1] Field placement does not constrain the loaded asset kind (cdk/src/constructs/blueprint.ts:262-270). The combined grammar-only validator allows an mcp_server ref under skills, a skill under cedarPolicyModules, etc. The orchestrator ignores the source field and routes by the parsed kind, so a configuration review can approve one surface while runtime loads another. Validate each list with its expected kind before combining refs.

Additional correctness gap

5. [P2] Inline-only assets receive dead artifact URLs (cdk/src/handlers/registry-publish.ts:104-120). This PR intentionally uploads no object for inline server_config, cedar_text, or prompt_fragment, but registry-resolve.ts:75-84 still presigns a deterministic S3 key solely because the kind is in KINDS_REQUIRING_ARTIFACT. The API returns a plausible 300-second URL whose GET is NoSuchKey. Carry artifact_ref through resolution and only sign it when present.

Verification

All targeted suites pass. The 72,000-byte Cedar probe reproduced the cap bypass; current tests cover only per-item size checks and small bundles.