fix(ui): fire streaming capability announce after authenticate()#2005
Open
kgabryje wants to merge 2 commits into
Open
fix(ui): fire streaming capability announce after authenticate()#2005kgabryje wants to merge 2 commits into
kgabryje wants to merge 2 commits into
Conversation
The announce was wired to client.on('authenticated'), which never fires
in the real Feathers v5 client, so #1896's owner-fallback gate never
engaged and idle tabs still got the streaming firehose. Fire it
explicitly after each successful authenticate (initial/reconnect/refresh).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The seam test covered only the initial-auth announce. Add renderHook tests for the reconnect direct re-auth, reconnect refresh-fallback, and in-place token-refresh reauth paths so dropping any announce site goes red. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The client-side session-streams capability announce was wired only to
client.on('authenticated'), an event the real Feathers v5 client never emits (it emitslogin;useBoardPresenceRoomonly survives because it also wiresclient.io.on('connect'), masking the same dead listener). Because the announce never fired in prod, no connection was ever marked session-streams aware, so #1896's daemon owner-fallback gate never engaged and idle home/board tabs kept receiving the full streaming firehose.Fix: replace the listener with a plain
announceSessionStreamsCapability(client)and fire it explicitly after every successfulauthenticate()— initial auth, reconnect re-auth, reconnect refresh-fallback, and in-place token-refresh reauth. Firing once per (re)auth is correct (each is a connection to mark aware; the server side is idempotent). The announce stays post-auth because a pre-auth call would 401 and trigger refresh churn.Daemon gate unchanged — it is already correct and deployed. Proven in prod: manually firing the announce in a live idle tab dropped the streaming firehose from ~18 events/15s to 0.
Tests: the old test emitted a mock
authenticatedevent, so the announce fired in-test and went green while prod was broken (false green). Replaced with (1) a unit test ofannounceSessionStreamsCapabilityand (2) a realrenderHookseam test that drives the actual connect→authenticate flow with a controllableauthenticate()promise, asserting the announce fires after authenticate resolves and not before (guards the pre-auth 401 regression). Verified this seam test goes red when the post-auth announce is removed.🤖 Generated with Claude Code