fix: Intermittent subscription update loss and socket lifecycle reliability

[PeterC89]

About the Contributor

This pull request is posted on behalf of the BBC.

Type of Contribution

This is a:

Bug fix / Code improvement

Current Behavior

• Only the first matching subscription callback was fired per path.
• Unsubscribe mutated the subscription array during iteration, which could skip removals.
• Recursive tree update could recurse into undefined child nodes when new children appeared.
• Socket close handling could leave lifecycle state/timers inconsistent.
• Socket layer had a fragile dependency path around shared connection status symbols.

New Behavior

• Dispatch now invokes all callbacks subscribed to a changed path.
• Unsubscribe now performs stable full-path removal.
• Child merge now inserts missing children and sets parent links before recursion.
• Socket close/handleClose now enforce consistent disconnected state and timer/resource cleanup.

Testing Instructions

Other Information

Status

• PR is ready to be reviewed.
• The functionality has been tested by the author.
• Relevant unit tests has been added / updated.
• Relevant documentation (code comments, system documentation) has been added / updated.

[coderabbitai]

Walkthrough

This PR moves ConnectionStatus into its own module, updates client and socket imports, changes client subscription cleanup and child-node reconciliation, and adjusts S101Socket keepalive and disconnect teardown with lifecycle tests.

Changes

Connection lifecycle and teardown handling

Layer / File(s)	Summary
Status module and imports src/Ember/Client/ConnectionStatus.ts, src/Ember/Client/index.ts, src/Ember/Socket/S101Client.ts, src/Ember/Socket/S101Socket.ts, src/__mocks__/S101Client.ts	ConnectionStatus is exported from a new module and imported directly by client, socket, and mock code.
Subscription and tree handling src/Ember/Client/index.ts, src/Ember/Client/__tests__/index.spec.ts	unsubscribe() removes matching subscriptions with filter(), _handleIncoming() notifies every matching callback, _updateTree() merges inserted children, and regression tests cover fanout, unsubscribe removal, and child insertion.
Socket keepalive teardown src/Ember/Socket/S101Socket.ts, src/Ember/Socket/__tests__/S101Socket.spec.ts	S101Socket clears the keepalive response-window timer during response, disconnect, close, and handleClose(), destroys the socket in handleClose(), and lifecycle tests cover close and teardown behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title concisely reflects the main fixes around subscription dispatch and socket lifecycle reliability.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description directly matches the changes to subscription handling, tree updates, connection status imports, and socket lifecycle cleanup.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/tidy-subscriptions

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 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/Ember/Client/index.ts`:
- Around line 709-728: The child insertion path in _updateTree() is pushing the
same parent-path change multiple times when update.children adds several missing
children. Update the TreeNode merge logic so the parent getPath(tree)
notification is queued only once per qualified update, while still attaching
each new child’s parent and inserting it into tree.children; use _updateTree(),
tree.children, and changes.push() to locate the fan-out branch.

In `@src/Ember/Socket/S101Socket.ts`:
- Around line 83-93: Finalize the socket teardown in S101Socket’s close handler
before notifying listeners: in the `this.socket.on('close', ...)` callback, move
the `status` update and keepalive timer cleanup ahead of
`this.emit('disconnected')` so any `disconnected` listeners observe
`ConnectionStatus.Disconnected` and no active timers. Keep the fix localized to
the close callback in `S101Socket`.

🪄 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: CHILL

Plan: Pro

Run ID: 53cf2ee9-ccb8-4e06-b94c-9005dcb63729

📥 Commits

Reviewing files that changed from the base of the PR and between 67d143f and c545932.

📒 Files selected for processing (7)

• src/Ember/Client/ConnectionStatus.ts
• src/Ember/Client/__tests__/index.spec.ts
• src/Ember/Client/index.ts
• src/Ember/Socket/S101Client.ts
• src/Ember/Socket/S101Socket.ts
• src/Ember/Socket/__tests__/S101Socket.spec.ts
• src/__mocks__/S101Client.ts