refactor(web): extract WebSocket subscription into reusable composable#475
refactor(web): extract WebSocket subscription into reusable composable#475
Conversation
Replace duplicated connect/subscribe/handler/cleanup boilerplate across 6 page views with a single useWebSocketSubscription composable that manages the full WebSocket lifecycle on mount/unmount. Closes #351
Update CLAUDE.md Package Structure to include useWebSocketSubscription in composables listing. Add 4 additional test cases: connect-throw skips subscribe/handler wiring, subscribe-throw is caught, empty bindings array edge case, reconnectExhausted reactivity. Pre-reviewed by 4 agents, 5 findings addressed
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🧰 Additional context used📓 Path-based instructions (5){src/synthorg/**/*.py,tests/**/*.py,web/src/**/*.{ts,tsx,vue},cli/**/*.go,**/*.md,**/*.yml,**/*.yaml,**/*.json,pyproject.toml}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/api/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/stores/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/**/*.vue📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/__tests__/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📚 Learning: 2026-03-15T22:42:34.899ZApplied to files:
📚 Learning: 2026-03-15T11:48:14.867ZApplied to files:
📚 Learning: 2026-03-15T22:42:34.899ZApplied to files:
📚 Learning: 2026-03-15T22:42:34.899ZApplied to files:
🧬 Code graph analysis (2)web/src/stores/websocket.ts (1)
web/src/__tests__/composables/useWebSocketSubscription.test.ts (3)
🪛 GitHub Check: Dashboard Lintweb/src/__tests__/composables/useWebSocketSubscription.test.ts[warning] 304-304: [warning] 302-302: 🔇 Additional comments (9)
📝 WalkthroughSummary by CodeRabbit
WalkthroughExtracts duplicated WebSocket lifecycle logic into a new Changes
Sequence Diagram(s)sequenceDiagram
participant Component as "Page Component"
participant Composable as "useWebSocketSubscription"
participant WSStore as "WebSocket Store"
participant Server as "WebSocket Server"
Component->>Composable: mount + provide bindings (channel, handler, filters)
Composable->>WSStore: connect() (if token & not connected)
WSStore->>Server: open connection
Composable->>WSStore: subscribe(dedupedChannels, filters)
WSStore->>Server: subscribe channels with filters
Server-->>WSStore: events (per channel)
WSStore-->>Composable: dispatch channel event
Composable-->>Component: invoke handler(s) for binding(s)
Component->>Composable: unmount
Composable->>WSStore: unsubscribe(channels) && offChannelEvent(handler)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new reusable Vue composable, Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is an excellent refactoring that significantly reduces boilerplate code across multiple components by introducing the useWebSocketSubscription composable. The new composable is well-designed and its lifecycle management is correctly handled with onMounted and onUnmounted. The accompanying unit tests are comprehensive and cover a wide range of scenarios, including error handling and edge cases, which is great to see. I've found one potential logic issue in the new composable related to handling subscriptions when no auth token is present, for which I've left a specific comment and suggestion.
| if (authStore.token && !wsStore.connected) { | ||
| wsStore.connect(authStore.token) | ||
| } | ||
| wsStore.subscribe(uniqueChannels, options.filters) | ||
| for (const binding of options.bindings) { | ||
| wsStore.onChannelEvent(binding.channel, binding.handler) | ||
| } |
There was a problem hiding this comment.
The current implementation attempts to subscribe to WebSocket channels and wire event handlers even if no authentication token is present. This leads to subscriptions being queued in the websocket store that will never be sent, as a connection is never established. The entire setup logic within onMounted should be conditional on the presence of an auth token to prevent this. This change will likely require updates to some unit tests that currently expect subscribe to be called without a token.
if (authStore.token) {
if (!wsStore.connected) {
wsStore.connect(authStore.token)
}
wsStore.subscribe(uniqueChannels, options.filters)
for (const binding of options.bindings) {
wsStore.onChannelEvent(binding.channel, binding.handler)
}
}There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/composables/useWebSocketSubscription.ts`:
- Around line 57-62: The onUnmounted cleanup should use defensive try/catch so
one failing call doesn't abort remaining cleanup: wrap the
wsStore.unsubscribe(uniqueChannels) call in a try/catch and log the error, then
iterate options.bindings and for each binding call
wsStore.offChannelEvent(binding.channel, binding.handler) inside its own
try/catch (or catch per-iteration) so a thrown error for one binding doesn't
prevent unregistering the rest; reference onUnmounted, wsStore.unsubscribe,
wsStore.offChannelEvent, options.bindings, and uniqueChannels when locating
where to add these guards.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: eaf32335-89dc-4e2d-9173-bca247dc71b2
📒 Files selected for processing (9)
CLAUDE.mdweb/src/__tests__/composables/useWebSocketSubscription.test.tsweb/src/composables/useWebSocketSubscription.tsweb/src/views/AgentProfilesPage.vueweb/src/views/ApprovalQueuePage.vueweb/src/views/BudgetPanelPage.vueweb/src/views/DashboardPage.vueweb/src/views/MessageFeedPage.vueweb/src/views/TaskBoardPage.vue
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
{src/synthorg/**/*.py,tests/**/*.py,web/src/**/*.{ts,tsx,vue},cli/**/*.go,**/*.md,**/*.yml,**/*.yaml,**/*.json,pyproject.toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples; use generic names:
example-provider,example-large-001,example-medium-001,example-small-001, or size aliases
Files:
CLAUDE.mdweb/src/__tests__/composables/useWebSocketSubscription.test.tsweb/src/views/TaskBoardPage.vueweb/src/views/DashboardPage.vueweb/src/composables/useWebSocketSubscription.tsweb/src/views/BudgetPanelPage.vueweb/src/views/MessageFeedPage.vueweb/src/views/AgentProfilesPage.vueweb/src/views/ApprovalQueuePage.vue
web/src/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for Vue component unit tests with fast-check for property-based testing
Files:
web/src/__tests__/composables/useWebSocketSubscription.test.ts
web/src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Use PrimeVue components + Tailwind CSS for styling in Vue components
Files:
web/src/views/TaskBoardPage.vueweb/src/views/DashboardPage.vueweb/src/views/BudgetPanelPage.vueweb/src/views/MessageFeedPage.vueweb/src/views/AgentProfilesPage.vueweb/src/views/ApprovalQueuePage.vue
🧠 Learnings (9)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
CLAUDE.mdweb/src/views/TaskBoardPage.vueweb/src/views/BudgetPanelPage.vueweb/src/views/AgentProfilesPage.vueweb/src/views/ApprovalQueuePage.vue
📚 Learning: 2026-03-15T22:42:34.899Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T22:42:34.899Z
Learning: Applies to web/src/components/**/*.vue : Vue components should be organized by feature in `web/src/components/` (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T22:42:34.899Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T22:42:34.899Z
Learning: Applies to web/src/stores/**/*.{ts,tsx} : Use Pinia stores in `web/src/stores/` for state management (auth, agents, tasks, budget, messages, meetings, approvals, websocket, analytics, company, providers)
Applied to files:
CLAUDE.mdweb/src/views/TaskBoardPage.vueweb/src/views/DashboardPage.vue
📚 Learning: 2026-03-15T22:42:34.899Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T22:42:34.899Z
Learning: Applies to web/src/router/**/*.{ts,tsx} : Use Vue Router with auth guards in `web/src/router/` for routing configuration
Applied to files:
CLAUDE.mdweb/src/views/AgentProfilesPage.vue
📚 Learning: 2026-03-15T22:42:34.899Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T22:42:34.899Z
Learning: Applies to web/src/api/**/*.{ts,tsx} : Use Axios client in `web/src/api/` for REST API communication with TypeScript types mirroring backend Pydantic models
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
CLAUDE.mdweb/src/views/DashboardPage.vueweb/src/views/MessageFeedPage.vue
📚 Learning: 2026-03-15T22:42:34.899Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T22:42:34.899Z
Learning: Applies to web/src/**/*.vue : Use PrimeVue components + Tailwind CSS for styling in Vue components
Applied to files:
CLAUDE.mdweb/src/views/TaskBoardPage.vueweb/src/views/MessageFeedPage.vueweb/src/views/ApprovalQueuePage.vue
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T22:42:34.899Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T22:42:34.899Z
Learning: Applies to web/src/__tests__/**/*.{ts,tsx} : Use Vitest for Vue component unit tests with fast-check for property-based testing
Applied to files:
web/src/__tests__/composables/useWebSocketSubscription.test.ts
🧬 Code graph analysis (1)
web/src/__tests__/composables/useWebSocketSubscription.test.ts (3)
web/src/stores/websocket.ts (1)
useWebSocketStore(19-239)web/src/api/types.ts (1)
WsEventHandler(679-679)web/src/composables/useWebSocketSubscription.ts (1)
useWebSocketSubscription(35-68)
🔇 Additional comments (10)
web/src/__tests__/composables/useWebSocketSubscription.test.ts (1)
1-266: Well-structured and comprehensive test suite.The tests thoroughly cover the composable's functionality: connection guards, channel deduplication, filter forwarding, handler wiring, cleanup on unmount, error handling (both connect and subscribe failures), empty bindings edge case, and reactive ref synchronization. The mock setup for Vue lifecycle hooks is a clean approach for testing composables outside component context.
One minor observation: the coding guidelines mention fast-check for property-based testing, but deterministic unit tests are appropriate here since the composable behavior is straightforward to verify with specific inputs.
web/src/composables/useWebSocketSubscription.ts (1)
35-68: Clean composable implementation.The composable properly encapsulates WebSocket lifecycle management: conditional connect, channel deduplication via
Set, handler wiring, and cleanup. The use ofreadonlymodifiers on interfaces and computed refs for reactive state exposure are good TypeScript practices.web/src/views/TaskBoardPage.vue (1)
34-47: Clean refactor using the new composable.The WebSocket subscription is now declaratively configured, and
onMountedfocuses solely on initial data fetching. This separation of concerns improves maintainability and aligns with the PR's goal of eliminating duplicated boilerplate.web/src/views/ApprovalQueuePage.vue (1)
35-45: Consistent adoption of the composable pattern.The refactor follows the same clean pattern as other views, binding the
approvalschannel toapprovalStore.handleWsEventwhile keeping data fetching inonMounted.web/src/views/BudgetPanelPage.vue (1)
16-26: Consistent refactor pattern applied.Budget channel subscription is cleanly delegated to the composable while data fetching remains in
onMounted.web/src/views/MessageFeedPage.vue (1)
16-26: Refactor maintains existing channel-switching behavior.The composable handles WebSocket lifecycle while the
watchonactiveChannelcorrectly remains for fetching messages when the user switches channels.web/src/views/AgentProfilesPage.vue (1)
18-28: Consistent composable adoption.The agents channel subscription follows the established pattern, cleanly separating WebSocket lifecycle from data fetching.
web/src/views/DashboardPage.vue (2)
29-35: Good use of multi-channel bindings.The Dashboard demonstrates the composable's ability to handle multiple channel subscriptions in a single call. Destructuring
connectedto pass toSystemStatusis a clean way to expose connection state to the UI.
37-66: Resilient data loading with partial failure handling.Using
Promise.allSettledwith failure detection and toast notification provides good UX when some data sources are unavailable. This pattern gracefully degrades rather than failing the entire dashboard load.CLAUDE.md (1)
137-137: Documentation update is accurate and consistent with the refactor.The composables index correctly includes
useWebSocketSubscription, matching the PR’s stated structural change.
…d CodeRabbit Composable improvements: - Guard subscribe/handler wiring behind auth token check (Gemini) - Separate try/catch per operation so partial failures don't cascade - Add defensive try/catch to onUnmounted cleanup (CodeRabbit) - Add setupError ref to return type for user-facing error state - Update JSDoc: document error-swallowing, channel dedup, conditions - Use WsSubscriptionFilters (Readonly) for filters type Store type tightening: - Narrow onChannelEvent/offChannelEvent from string to WsChannel | '*' - Import and use WsSubscriptionFilters for subscribe parameter Type system: - Extract WsSubscriptionFilters type alias in api/types.ts - Update WsSubscribeMessage to use the shared type Page fixes: - Fix empty catch block in MessageFeedPage watch (was silently swallowing) - Log Promise.allSettled rejection reasons in DashboardPage - Re-add sanitizeForLog import to DashboardPage Test improvements: - Explicitly re-establish both onMounted and onUnmounted mocks after clearAllMocks - Strengthen error assertions with actual messages (replace expect.any(String)) - Add test: unmount cleanup after failed connect - Add test: handler wiring continues when subscribe throws - Combine reactive-ref tests with it.each - Add auth token to tests that exercise subscribe/handler wiring - Add setupError assertions to error path tests
🤖 I have created a release *beep* *boop* --- ## [0.2.8](v0.2.7...v0.2.8) (2026-03-16) ### Features * add RRF rank fusion to memory ranking ([#478](#478)) ([42242b5](42242b5)) * collaboration scoring enhancements — LLM sampling and human override ([#477](#477)) ([b3f3330](b3f3330)) ### Bug Fixes * add .gitattributes to enforce LF line endings for Go files ([#483](#483)) ([1b8c7b6](1b8c7b6)) * **cli:** Windows uninstall, update UX, health check, sigstore ([#476](#476)) ([470ca72](470ca72)) ### Refactoring * **web:** extract WebSocket subscription into reusable composable ([#475](#475)) ([96e6c46](96e6c46)), closes [#351](#351) ### Maintenance * bump hypothesis from 6.151.5 to 6.151.9 in the minor-and-patch group ([#482](#482)) ([a7297d5](a7297d5)) * bump nginxinc/nginx-unprivileged from `aec540f` to `ccbac1a` in /docker/web ([#479](#479)) ([176e052](176e052)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
useWebSocketSubscriptioncomposable{ bindings: ChannelBinding[], filters?: Record<string, string> }and manages the full lifecycle viaonMounted/onUnmounted{ connected, reconnectExhausted }as reactiveComputedRef<boolean>refsTest plan
vue-tsc --noEmitpasses (no type errors)Review coverage
Pre-reviewed by 4 agents (docs-consistency, frontend-reviewer, issue-resolution-verifier, test-quality-reviewer). 5 findings addressed: CLAUDE.md docs update + 4 additional test cases.
Closes #351