Skip to content

feat: dispatch AuthLogin and AuthLogout, add Event.listen, AuthChannelSubscription, Str.unwrap - #203

Merged
anilcancakir merged 5 commits into
masterfrom
feature/auth-lifecycle-events
Sep 25, 2026
Merged

anilcancakir merged 5 commits into
masterfrom
feature/auth-lifecycle-events

Conversation

@anilcancakir

Copy link
Copy Markdown
Member

feat: dispatch AuthLogin and AuthLogout, add Event.listen, AuthChannelSubscription, Str.unwrap and a public key collapse

AuthLogin and AuthLogout were defined and documented but never dispatched.
BaseGuard.startSession now dispatches AuthLogin last, guarded by the session
epoch and by the user still being the one it set, so a sign-in that a logout
overtook during its own cache write stays silent (and the late cache write is
undone). logout() captures the user at entry and dispatches AuthLogout after
the state bump and before rethrowing a vault failure, a guest included.
AuthRestored is unchanged: API-confirmed restores only. The fake guard
dispatches the same events in the same order.

Also adds Event.listen(factory) (Laravel Event::listen), an auth-scoped
private channel reconciler for broadcasting, Str.unwrap with Laravel
semantics, and CollapsesIndexedErrorKeys.collapse for controllers that cannot
take the mixin. Docs, skill references and CHANGELOG updated; AuthFailed is
documented as defined but not dispatched.

No version bump.

Part of the "framework-level support" round (plan 2 of 3): code that uptizm carried because a plugin lacked a seam moves into the owning package. Companion PRs: fluttersdk/magic, fluttersdk/magic_starter, fluttersdk/magic_notifications, fluttersdk/magic_deeplink, fluttersdk/wind, fluttersdk/magic_sentry, and the uptizm adoption.

This PR compiles and tests against its siblings' current default branches; nothing here depends on another PR in the round being merged first. No version bump and no publish.

…lSubscription, Str.unwrap and a public key collapse

AuthLogin and AuthLogout were defined and documented but never dispatched.
BaseGuard.startSession now dispatches AuthLogin last, guarded by the session
epoch and by the user still being the one it set, so a sign-in that a logout
overtook during its own cache write stays silent (and the late cache write is
undone). logout() captures the user at entry and dispatches AuthLogout after
the state bump and before rethrowing a vault failure, a guest included.
AuthRestored is unchanged: API-confirmed restores only. The fake guard
dispatches the same events in the same order.

Also adds Event.listen<T>(factory) (Laravel Event::listen), an auth-scoped
private channel reconciler for broadcasting, Str.unwrap with Laravel
semantics, and CollapsesIndexedErrorKeys.collapse for controllers that cannot
take the mixin. Docs, skill references and CHANGELOG updated; AuthFailed is
documented as defined but not dispatched.

No version bump.
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.23664% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../broadcasting/drivers/reverb_broadcast_driver.dart 98.41% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@kodizm

kodizm Bot commented Sep 25, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The auth event dispatch, Event.listen, Str.unwrap and CollapsesIndexedErrorKeys.collapse look correct; one reconnect-path issue in AuthChannelSubscription is worth fixing before merge.

Major

lib/src/broadcasting/auth_channel_subscription.dart:163: If the channel name changes while the Reverb driver is recovering from a drop, Echo.connect() runs beside the driver's own reconnect, and that opens the second socket the comment at step 2 warns about (correctness). It happens because the reconcile loop decides whether to connect using !Echo.connection.isConnected. After _onDone or _onError, isConnected is false and _scheduleReconnect has already armed a Timer that calls connect() (reverb_broadcast_driver.dart:728-740). A public connect() does not cancel that timer, so a team switch or logout-then-login while offline ends up with two sockets. The same happens after a timed-out boot connect, which also schedules a reconnect. Gating on the connectionState value, or on whether a reconnect is pending, would avoid it. Nothing in the tests covers a name change during reconnecting.

Minor

lib/src/broadcasting/auth_channel_subscription.dart:196: When channelName() returns null, the teardown calls Echo.disconnect() on the default connection. That also drops any public or presence channel the app subscribed to elsewhere through Echo. The class doc says it "only ever" manages its one channel, so either document that teardown disconnects the whole connection, or leave the connection alone and only leave the channel.

Tests

The new test/auth/auth_events_dispatch_test.dart covers login/logout dispatch, including the epoch race and the identity race. test/broadcasting/auth_channel_subscription_test.dart covers the reconciler, and test/facades/event_listen_test.dart, str_test.dart and validates_requests_form_request_test.dart cover the smaller additions. I found no test for a name change while the connection is reconnecting.

CI

  • Lint & Test: success
  • codecov/patch: success (100% of diff hit)
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

I checked the rules against CLAUDE.md: the CHANGELOG, doc/ and skills are updated in the same change, and the SKILL.md version is bumped (0.1.47 to 0.1.48). I read the doc and skill markdown only through the manifest, not line by line.

@kodizm

kodizm Bot commented Sep 25, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The auth event dispatch, Event.listen, Str.unwrap and collapse look correct. The new AuthChannelSubscription has one real problem: its teardown disconnects the whole shared connection.

Major

lib/src/broadcasting/auth_channel_subscription.dart:196 (correctness): When channelName() returns null, _teardown calls Echo.disconnect(). That closes the one shared default connection, and the Reverb driver's disconnect() disposes every channel on it. Any other subscription the app holds is killed, including a second AuthChannelSubscription (for example a user channel beside a team channel). That second instance still has its _subscribedName set, so branch 2 of _reconcile turns every later sync() into a no-op and it never resubscribes. Leaving only its own channel, or at least documenting the disconnect in doc/digging-deeper/broadcasting.md, would avoid this silent loss.

Minor

lib/src/broadcasting/auth_channel_subscription.dart:165 (correctness): The !Echo.connection.isConnected guard is also true while the Reverb driver is reconnecting with its _reconnectTimer pending. So a name change during a network drop, such as a team switch or re-login, still calls Echo.connect() beside the driver's own reconnect. That is the double-connect the comment at step 2 says this class avoids. Checking for reconnecting state, not only isConnected, would close the gap.

Tests

New tests cover login/logout dispatch ordering and the epoch/identity races (test/auth/auth_events_dispatch_test.dart), the channel reconciler against the fake broadcaster, Event.listen, Str.unwrap and collapse. The fake broadcaster cannot exercise the shared-connection or mid-reconnect cases above.

CI

  • Lint & Test: success
  • codecov/patch: success (100% of diff hit)
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

I skimmed the docs, skill references and CHANGELOG edits for consistency with the code but did not audit them line by line.

…ubscription, document the teardown disconnect
@anilcancakir

Copy link
Copy Markdown
Member Author

@kodizm review. Major fixed: the subscription tracks the driver's connection state and does not call connect() while a reconnect or connect is in flight (new test, red first). Minor: documented that a null channel name disconnects the default connection, deliberately.

@kodizm

kodizm Bot commented Sep 25, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The reconnect fix closes the double-socket case, but treating connecting as "a reconnect is pending" means a first connect that fails can leave the subscription stuck offline for good.

Since the last review: commit 1414b3f adds _lastConnectionState tracking and gates Echo.connect() on it. It also documents that a null channel name disconnects the whole connection. That settles the earlier Minor, since the disconnect is deliberate.

Major

lib/src/broadcasting/auth_channel_subscription.dart:194 (correctness): ReverbBroadcastDriver.connect() emits connecting first. If await _channel!.ready then throws (for example, the server cannot be reached at boot), no reconnect is scheduled and no further state is emitted, so _lastConnectionState stays connecting. On the next sync(), driverReconnectPending is true, so connect is skipped. Echo.private(name) then subscribes on a dead driver and sets _subscribedName. After that, branch 2 makes every later sync() a no-op, and the app never connects until the process restarts. This breaks the step 3 promise that "a failed Echo.connect() leaves a clean state the next sync retries". The same thing happens with reconnect: false: _onDone emits reconnecting, but _scheduleReconnect returns early, so nothing is actually pending. One fix is to reset _lastConnectionState when Echo.connect() throws, or to drop connecting from the gate and only trust reconnecting when reconnect is enabled.

Tests

The new test covers a name change while the connection is reconnecting. The spy's connect() never emits connecting and never fails after emitting it, so nothing covers the case above.

CI

  • Lint & Test: success
  • codecov/patch: success (100% of diff hit)
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

I checked the new commit against reverb_broadcast_driver.dart (connect, _onDone/_onError, _scheduleReconnect). The rest of the PR is as in my earlier reviews and I did not re-read it.

@anilcancakir

Copy link
Copy Markdown
Member Author

@kodizm review. Major fixed: only a reconnecting state skips connect, a failed own connect resets the tracked state so the next sync retries, and the Reverb driver now reports disconnected (not reconnecting) when reconnect is disabled. Both cases have red-first tests.

@kodizm

kodizm Bot commented Sep 25, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The boot-failure strand is fixed and so is the reconnect: false state. But removing connecting from the gate brings back the double-socket case for any outage that lasts past the driver's first retry.

Since the last review: commit b704709 narrows the gate to reconnecting only. It also resets _lastConnectionState when the subscription's own connect() fails, and makes _onDone/_onError report disconnected when reconnect is off. Those fix what I raised last time, and each has a new test.

Major

lib/src/broadcasting/auth_channel_subscription.dart:201 (correctness): The driver only emits reconnecting once, at the drop. When its reconnect Timer fires, the driver's own connect() emits connecting (reverb_broadcast_driver.dart:211). If that attempt fails, the catch at :777-784 arms another timer and emits nothing, so for the rest of the outage _lastConnectionState is connecting while a reconnect is really pending. A team switch or re-login at that point passes the gate and calls Echo.connect() next to the armed timer, which opens the second socket that step 2 warns about. The same happens during the driver's own in-flight retry, which can last up to connection_timeout.

A related case: when the subscription's own connect times out, the driver arms a reconnect (:270) before throwing, and the new catch then sets the state to null. The next sync then connects beside that timer too. One fix: have _scheduleReconnect emit reconnecting whenever it arms a timer, and in the catch only reset the state when it is still connecting.

Tests

The new tests cover a failed own connect followed by a retry, and disconnected on drop with reconnect: false. Nothing covers a name change after a failed driver-side retry or after a connect timeout, because the spy never arms a reconnect timer.

CI

  • Lint & Test: success
  • codecov/patch: success (100% of diff hit)
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

In this pass I read commit b704709 plus the driver's connect() and _scheduleReconnect. The rest of the PR is unchanged from my earlier reviews and I did not re-read it.

@anilcancakir

Copy link
Copy Markdown
Member Author

@kodizm review. Root cause fixed instead of inferring driver state: ReverbBroadcastDriver.connect() is now single-flight and supersedes an armed retry, so a second call (or one during a retry, failed retry or timeout backoff) cannot open a second socket. AuthChannelSubscription goes back to a plain isConnected gate. Red-first driver tests cover concurrent connects, connect after a drop, connect during a failed-retry backoff, and connect while connected.

@kodizm

kodizm Bot commented Sep 25, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The fix in the driver is correct. With single-flight connect() the driver can only hold one socket, so the double-socket issue from my last review is closed. I have no new findings, but CI has not finished.

Since the last review: commit 22f449e makes ReverbBroadcastDriver.connect() idempotent. It does this with _inFlight, the new _singleFlight and _releaseSocket, and a disconnect() epoch. It also takes the connectionState inference out of AuthChannelSubscription, so the gate there is a plain isConnected check again.

Why I think the socket count is now bounded, for each path I checked:

  • The one place a socket is created: only _openSocket creates one, and it runs only inside _singleFlight. It releases any earlier socket first, and cancels the stream subscription before closing the sink, so the released socket's close cannot reach _onDone and arm a retry of its own.
  • Connect while a retry is armed: the timer is cancelled and the call runs _reconnect now. That resubscribes every known channel and fires onReconnect, the same as a retry the timer ran. If it fails, it re-arms before it rethrows, so the reconnect loop keeps going.
  • Timeout inside _reconnect: the retry is armed twice, once in onTimeout and once in the catch. That is harmless because _scheduleReconnect cancels the earlier timer. The only side effect is that _attempt goes up twice, which already happened before this change.
  • disconnect() during an attempt: it completes the pending _connectionCompleter, so an attempt that is waiting returns quietly. An attempt suspended at _releaseSocket or channel.ready stops at the epoch check.

Tests

The new ReverbBroadcastDriver: idempotent connect group covers:

  • two concurrent connects
  • connect while already connected
  • connect after a drop
  • connect during the backoff after a failed retry
  • connect during the retry armed by a connection timeout
  • a failed connect that replaces an armed retry, then re-arms it

The subscription tests were changed to match the plain gate.

CI

  • Lint & Test: still running (checked twice). This change is not confirmed green yet.
  • Internal Links & Anchors: success
  • External Links: skipped
  • Auto-merge low-risk Dependabot PRs: skipped

In this pass I read commit 22f449e: the driver's connect/_openSocket/disconnect/_onDone/_onError/_scheduleReconnect/_reconnect, the subscription change and the CHANGELOG entry. The rest of the PR is as in my earlier reviews and I did not re-read it.

@anilcancakir
anilcancakir merged commit 73fa444 into master Sep 25, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant