fix(socket): clean up room listeners after unsubscribe

[yoavf]

Summary

• Delete a room's listener bucket after the last local listener unsubscribes.
• Make each returned unsubscribe function idempotent, so repeated cleanup calls do not emit repeated socket leave events.
• Add focused RoomsSocket tests for shared room joins, rejoining after the last unsubscribe, and duplicate unsubscribe calls.

Why

A client can create room churn without having many users or many browser tabs. For example, a React effect can repeatedly clean up and recreate a realtime subscription as component state or query results change. Cleanup functions may also be called more than once by app code or lifecycle edge cases.

Before this change, the final unsubscribe emitted leave, but the SDK kept an empty listener array for that room. That had two bad effects:

• a later subscription to the same room saw the existing empty entry and skipped the required join, so realtime updates could be missed;
• reconnect handling still treated the empty room as active, so stale rooms could be joined again even with no local listeners.

Duplicate calls to the same unsubscribe callback could also produce extra leave emissions for a room that was already cleaned up.

Mitigation

The SDK now treats room membership as a real refcounted lifecycle:

• first local listener joins the room;
• additional local listeners share that room membership;
• removing the last listener emits one leave and deletes the room entry;
• a future listener for the same room creates a fresh entry and joins again;
• repeated calls to the same unsubscribe function are no-ops.

This keeps the low-level socket behavior consistent and avoids stale empty-room state. It does not add new public options, analytics, caps, or entity-specific subscription behavior.

Risk / side effects

• A room with zero local listeners is no longer remembered across reconnects. That is intentional: with no callbacks, there is no active subscription to preserve.
• A later subscription after the room was fully cleaned up will emit a fresh join. That restores the expected behavior for unsubscribe/resubscribe flows.
• This does not debounce rapid unsubscribe/resubscribe loops; it only makes each room's lifecycle correct and idempotent. Server-side leave handling should still be idempotent as a defense in depth.
• No API surface changes.

Validation

• npm run test:unit -- tests/unit/socket-utils.test.ts

[claude]

Claude finished @yoavf's task —— View job

---

PR Review: Entity Subscription Deduplication and Rate Limiting

I've thoroughly reviewed this pull request that implements subscription deduplication, caps, and churn prevention for the entity realtime subscriptions. Here's my comprehensive feedback:

🟢 Code Quality & Best Practices

Excellent Architecture:

• Clean separation of concerns with the EntitySubscriptionManager handling all subscription logic
• Well-structured state management using Maps for efficient lookups
• Good use of TypeScript interfaces for type safety
• Proper encapsulation of subscription logic away from the main entities module

Strong Points:

• Comprehensive configuration options with sensible defaults
• Clear error messages that guide developers on how to resolve issues
• Good use of timers with unref() to prevent keeping the process alive
• Proper cleanup logic for unsubscriptions

Minor Suggestions:

• Consider extracting magic numbers like 500 (batch size) and 10 KB (oversize threshold) into named constants

🟡 Potential Bugs or Issues

Edge Case in Grace Period Logic:
The grace period timer implementation looks correct, but there's a subtle race condition window:

// In scheduleRoomClose (line 324-336)
const closeTimer = setTimeout(() => {
  state.closeTimer = null;
  if (state.callbacks.size === 0 && activeSubscriptions.get(state.room) === state) {
    closeRoomSubscription(state);
  }
}, normalizedOptions.emptyRoomGraceMs);

If a new subscription comes in exactly when the timer fires but before closeRoomSubscription executes, it could lead to unexpected behavior. The current implementation handles this correctly by checking state.callbacks.size === 0 again, but it's worth noting.

Analytics Event Error Handling:
Good catch on wrapping the analytics call in try-catch (line 205-212). This prevents telemetry from breaking the application.

🟢 Performance Considerations

Excellent Optimizations:

• Single socket room connection shared across multiple subscribers (fan-out pattern)
• Efficient use of Maps for O(1) lookups
• Grace period prevents socket churn from rapid mount/unmount cycles
• Churn detection sliding window efficiently filters old events

Memory Management:

• Good cleanup of churn tracking data using sliding window (line 228-230)
• Proper cleanup of room subscriptions when empty
• No apparent memory leaks

🟢 Security Concerns

No significant security issues found:

• No sensitive data exposure in error messages or analytics
• Proper isolation between different entity subscriptions
• Safe JSON parsing with error handling

🟢 Test Coverage

Comprehensive Test Suite:

• Excellent coverage of core functionality
• Good edge case testing (grace periods, churn detection, caps)
• Proper use of fake timers for time-dependent tests
• Tests for error scenarios (invalid JSON, callback errors)

Test Quality Highlights:

• Tests verify both positive and negative cases
• Good isolation between tests with proper setup/teardown
• Clear test descriptions that document expected behavior

Additional Test Suggestions:

1. Consider adding a test for concurrent subscriptions during grace period expiry
2. Test behavior when maxActiveSubscriptions is reached and then a subscription is removed

📊 Additional Observations

Observability:
The analytics events provide excellent visibility into subscription health:

• active_subscription_cap warnings help identify scaling issues
• subscription_churn warnings highlight potential React effect issues
• Safe metadata ensures no PII leakage

Documentation:
The PR description clearly explains the why, what, and how. The code comments are helpful, especially the explanation of the oversize broadcast handling.

Socket Utils Enhancement:
The fix to socket-utils.ts properly cleans up room state, allowing rejoin after complete unsubscription - this is crucial for the grace period feature to work correctly.

✅ Verdict

This is a well-implemented solution that addresses real-world issues with React effect churn and socket room management. The code is clean, performant, and thoroughly tested. The addition of client-side caps and diagnostic analytics will help developers identify and fix subscription issues early.

Approved - Great work on this implementation! The subscription deduplication and grace period features will significantly improve the stability of realtime subscriptions in React applications.

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.28-pr.173.b39e4d0

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.28-pr.173.b39e4d0"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.28-pr.173.b39e4d0"
  }
}

• 📦 Preview Package: @base44-preview/sdk@0.8.28-pr.173.b39e4d0
• 🔗 View this commit on GitHub

---

_{Preview published to npm registry — try new features instantly!}

[claude]

Claude finished @yoavf's task —— View job

---

PR Review: Socket Room Unsubscribe Lifecycle Fix

I've reviewed this pull request that addresses important issues with socket room cleanup and unsubscription idempotency. Here's my comprehensive feedback:

✅ Code Quality and Best Practices

The implementation follows excellent coding practices:

• Clean and focused change: The fix is minimal and targeted, adding only 8 lines to solve the problem
• Proper use of closure: The unsubscribed flag is scoped correctly within each subscription's closure
• Consistent code style: Maintains the existing codebase conventions
• Good separation of concerns: The fix is contained within the subscription lifecycle logic

✅ Potential Bugs or Issues

No bugs identified. The implementation correctly addresses the issues described:

• The idempotent check (if (unsubscribed) return;) prevents multiple executions
• The room cleanup logic properly deletes the room entry when the last listener unsubscribes
• The order of operations is correct: set flag first, then perform cleanup

One minor observation: The use of optional chaining (roomsToListeners[room]?.filter) with a fallback to [] is defensive programming, though the room should always exist at this point since we're in the unsubscribe closure.

✅ Performance Considerations

The changes have positive performance implications:

• Prevents memory leaks: Deleting empty room entries (delete roomsToListeners[room]) ensures the object doesn't accumulate stale entries
• Reduces unnecessary operations: Idempotent unsubscribe prevents redundant array filtering and socket emissions
• Minimal overhead: Adding a boolean flag has negligible performance impact

✅ Security Concerns

No security issues identified. The changes:

• Don't expose any internal state
• Don't introduce new attack vectors
• Maintain proper encapsulation of the subscription lifecycle

✅ Test Coverage

Excellent test coverage! The new test file comprehensively covers all scenarios:

1. Shared room subscriptions: Verifies that multiple listeners share a single room join
2. Rejoin after cleanup: Confirms that rooms can be rejoined after full cleanup
3. Idempotency: Ensures duplicate unsubscribe calls don't cause issues

The tests are well-structured with:

• Clear test names that describe the behavior
• Proper mocking of socket.io dependencies
• Good use of assertion helpers to verify exact call sequences
• Appropriate test isolation with beforeEach cleanup

📊 Additional Observations

1. Root cause well addressed: The fix properly solves the described issues where empty listener arrays prevented rejoins and caused stale reconnect behavior

2. Backwards compatible: This change doesn't break any existing API contracts

3. Clear commit message: The commit "fix: make socket room unsubscribe lifecycle idempotent" follows conventional commit format

✅ Verdict

This is a well-crafted fix that addresses real issues in the socket room lifecycle management. The implementation is clean, the tests are comprehensive, and the change is minimal yet effective. The idempotent unsubscribe pattern will prevent the described React effect cleanup issues.

Approved - Great work on identifying and fixing this subtle but important issue!