refactor(runtime-host): consolidate host change broadcasts - #3248
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
💤 Files with no reviewable changes (4)
🚧 Files skipped from review as they are similar to previous changes (9)
Included review availability: Your plan provides up to 3 included reviews per hour; 0 remain after this review. 📝 WalkthroughSummaryThis PR consolidates four duplicated Runtime Host change services into one The PR extends the existing notification path. Coordinators publish through the shared feed, and connection sessions use one subscription handle. It does not create a parallel notification path. The solution is the smallest coherent design described by the diff. The subscription mask and feed-managed state are required for grant isolation, routing, independent revisions, and failure cleanup. The deleted service files and duplicated connection fields can remain removed. The migrated tests preserve coverage for routing, revisions, failed-send cleanup, and composition wiring. The added feed tests centralize behavior coverage. The summary reports typechecking, build, formatting/lint checks, and focused tests. The full suite reported one unrelated existing Complexity delta
Total maintenance complexity decreases. The added subscription and mask state is necessary for the consolidated, grant-filtered design. Review-relevant risks
The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughThe runtime host consolidates four change services into ChangesHost change feed consolidation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change consolidates host-change broadcasts while preserving grant filtering, frame formats, and revision semantics. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Replace four cloned connection registries with one grant-filtered host change feed while preserving frame and revision semantics. Generated-by: Codex
8cb3234 to
d6647a3
Compare
PR Summary by QodoConsolidate Runtime Host change broadcasts
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9159c2a4-9595-4e9b-a627-4e0ef22573af
📒 Files selected for processing (13)
packages/runtime-host/src/__tests__/host-change-feed.test.tspackages/runtime-host/src/__tests__/host-kernel.test.tspackages/runtime-host/src/__tests__/project-catalog-coordinator.test.tspackages/runtime-host/src/server/configuration-change-service.tspackages/runtime-host/src/server/connection-session.tspackages/runtime-host/src/server/execution-composition.tspackages/runtime-host/src/server/host-change-feed.tspackages/runtime-host/src/server/host-kernel.tspackages/runtime-host/src/server/project-catalog-change-service.tspackages/runtime-host/src/server/project-catalog-coordinator.tspackages/runtime-host/src/server/scheduled-task-change-service.tspackages/runtime-host/src/server/scheduled-task-coordinator.tspackages/runtime-host/src/server/session-catalog-change-service.ts
💤 Files with no reviewable changes (4)
- packages/runtime-host/src/server/scheduled-task-change-service.ts
- packages/runtime-host/src/server/project-catalog-change-service.ts
- packages/runtime-host/src/server/configuration-change-service.ts
- packages/runtime-host/src/server/session-catalog-change-service.ts
Included review availability: Your plan provides up to 3 included reviews per hour; 1 remains after this review.
Preserve the unified HostChangeFeed while incorporating current project directory authority changes. Generated-by: Codex
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Astro-Han
left a comment
There was a problem hiding this comment.
Approved on exact head 55d8d5fb9.
The consolidation preserves the four public frame kinds and independent revision sources while replacing duplicated services with one grant-filtered subscription. Connection grants map correctly to the four masks, teardown removes the unified handle, and failed-send cleanup remains identity-safe. I found no remaining P0–P2 correctness issue. The resolved CodeRabbit suggestion to count failed test sends directly is a nonblocking P3 coverage improvement and is not duplicated.
This is a Runtime Host refactor with no user-visible UI/UX change, so screenshots are not applicable. The PR body and both commits have complete Codex disclosure/trailers. Some required jobs are still pending; approval is code-level, and merge readiness still requires all checks green.
Reviewed with Codex as an AI-assisted code review. I verified the exact-head delta against current main, grants, revisions, subscription lifecycle, tests, CI, UI scope, and commit provenance; no external model output was used.
中文说明
当前 head 的代码审查通过:四类 frame 与独立 revision 保持不变,grant mask、统一 subscription 和 teardown/失败清理都正确,未发现剩余 P0–P2。该 PR 是 Runtime Host 重构,不涉及用户可见 UI/UX,因此无需截图;PR body 与两个 commit 的 Codex 说明完整。仍有 required jobs 在运行,所以这是代码层 APPROVE,只有全部 CI 变绿后才算 merge-ready。
|
Human merge confirmation: Astro-Han reviewed the final diff at |
Summary
Fixes #3217
Consolidate the four cloned Runtime Host change broadcast services into one grant-filtered
HostChangeFeed. Each connection now has one subscription handle and a mask derived from its operation grants. The existing four protocol frame kinds, payloads, and revision semantics remain unchanged.The old per-domain service files and duplicated connection lifecycles are removed. Focused tests cover grant isolation, independent revisions, send-failure cleanup, and migrated lifecycle fixtures.
Verification
npm --workspace @maka/runtime-host run typecheck -- --pretty falsenpm --workspace @maka/runtime-host run buildnpx biome check packages/runtime-host/src/server packages/runtime-host/src/__tests__/host-change-feed.test.ts packages/runtime-host/src/__tests__/host-kernel.test.ts packages/runtime-host/src/__tests__/project-catalog-coordinator.test.tsnode --test packages/runtime-host/dist/__tests__/host-change-feed.test.js packages/runtime-host/dist/__tests__/project-catalog-coordinator.test.jsThe focused checks pass. The full runtime-host suite was also run; one unrelated host-kernel shutdown scenario failed with an existing-style
write EPIPErace (answers an admitted bootstrap with draining after shutdown commits).Review focus
AI use
Tool(s) and scope: Codex performed the implementation, tests, code review, and PR preparation. A human contributor must review and own the submission.
Checklist
Does this PR entail a change in behavior?