feat(messaging): add manifest types and registry - #4000
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
📝 WalkthroughWalkthroughAdds serializable TypeScript contracts for messaging channel manifests and sandbox plans, a ChannelManifest registry with availability filtering, barrel exports to expose the manifest surface, and Vitest suites validating JSON round-trips, secret placeholders, absence of function-valued fields, and import-dependency isolation. ChangesMessaging Manifest Type Contracts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
PR Review AdvisorRecommendation: blocked This is an automated advisory review. A human maintainer must make the final merge decision. Limitations: This advisory review used read-only repository inspection plus trusted deterministic context; it did not execute tests, package-manager commands, workflows, or PR-provided scripts.; CI/status checks are still pending, so required check success for the exact head SHA cannot be fully verified.; The current E2E Advisor check is in progress; an existing E2E Advisor comment recommends no required E2E jobs but does not include an explicit analyzed SHA.; The parent issue #3896 was referenced but its body/comments were not included, so parent acceptance mapping is limited. Full advisor summaryPR Review AdvisorBase: The isolated messaging manifest/registry implementation looks well-scoped and unit-tested, but merge is blocked by GitHub mergeStateStatus=BLOCKED and pending status contexts for the exact head SHA. Gate status
🔴 Blockers
🟡 Warnings
🔵 Suggestions
Acceptance coverage
Security review
Test / E2E status
✅ What looks good
Review completeness
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/messaging/manifest/registry.ts`:
- Line 48: Normalize manifest.id before checking membership against
supportedChannelIds in listAvailable; e.g., compute a normalized id (trim and
lowercase) for manifest.id and use that normalized value in the if check (and
ensure supportedChannelIds contains normalized ids) so mixed-case or whitespace
in manifest.id won't cause false exclusions; update the membership check that
references supportedChannelIds and manifest.id accordingly (symbols:
supportedChannelIds, manifest.id, listAvailable).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ea7fa8ac-df51-483f-9e23-c2c441b6118a
📒 Files selected for processing (5)
src/lib/messaging/index.tssrc/lib/messaging/manifest/index.tssrc/lib/messaging/manifest/registry.test.tssrc/lib/messaging/manifest/registry.tssrc/lib/messaging/manifest/types.test.ts
|
Replaced by #4003, which uses a signed commit on a new branch. |
|
closed due to unsigned commit |
Summary
Adds the phase-1 messaging manifest foundation under
src/lib/messaging: serializable manifest/plan type contracts plus a small in-memory manifest registry. This phase is intentionally isolated: no existing onboarding, channel lifecycle, rendering, policy, credential, or rebuild production workflow imports or consumes these contracts yet.Related Issue
Fixes #3991
Fixes #3992
Part of #3896
Changes
src/lib/messagingmodule surface.SandboxMessagingPlanshape for future compiler/applier work without wiring it into production code.ChannelManifestRegistrywithregister,get,list, andlistAvailable.Type of Change
Verification
npm test -- src/lib/messaging/manifestnpm run typecheck:clinpm run lintThe commit hook's full CLI coverage run hit an existing timeout in
test/cli.test.ts(doctor fails a present sandbox that is not Ready); that exact test passed in isolation afterward.npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
Tests
New Features
Chores