Fix re-login hang on auth page after logout

[FadhlanR]

Problem

After an in-app logout → re-login (logout is a router.transitionTo, not a page reload), several things break. They share one root cause: the host centralizes session teardown but not session re-initialization. ResetService.resetAll() fanned resetState() out to ~30 services on logout, but there was no symmetric "session started" broadcast — re-initialization lived in one-shot sites (service constructors, boot-only loadSDK(), the index route's start guard, user clicks). So anything torn down in resetState() whose re-establishment lived in a one-shot site stayed broken for the life of the singleton after re-login.

Three confirmed instances of this family:

1. Auth page hangs on re-login. #clientReadyDeferred (a boot-only barrier) was recreated unfulfilled by resetState(); the next login's createRealmSession() awaited it forever.
2. AI assistant panel spins forever. loadRoomsTask was armed only in the panel service constructor and the openPanel() click. Logout cancelled it and cleared currentRoomId; nothing re-armed it, so isReady never became true (perpetual loading indicator).
3. Billing push updates go silently dead. RealmServerService.resetState() wiped the eventSubscribers map, but BillingService subscribes to billing-notification only in its constructor — which never re-runs after an in-app logout.

Approach

Classify every piece of host state as app-scoped (page-load → page-unload: SDK bundle, event-binding tables, subscriber wiring) or session-scoped (login → logout: tokens, room caches, subscription data, current room). The enforceable contract:

Anything you tear down in resetState() must be re-established in sessionStarted() or derived lazily; anything that should survive logout doesn't belong in resetState() at all.

All three bugs are misclassifications under this model.

ResetService becomes SessionService — a single session authority:

• @tracked isAuthenticated, the single source of truth for "a session is established" (written only by MatrixService, read everywhere; MatrixService.postLoginCompleted folds into it).
• Symmetric notifySessionStarted() / notifySessionEnded() broadcasts. MatrixService.start() fires the former only on the unauthenticated → authenticated edge — so re-running start() on an already-established session (e.g. the /connect route, which runs it on every visit) doesn't re-fire participants; logout() fires the latter (replacing resetAll()).
• Replay-on-register: registering while a session is already established replays sessionStarted(), so participants need no constructor arming and the hook runs exactly once per participant per session regardless of construction timing.
• Per-participant error isolation on both edges (a throwing resetState()/sessionStarted() no longer skips the rest of the registry).

MatrixService is the orchestrator, not a participant, which also ends the double-resetState() it previously ran on logout. On logout it now tears down its own session state synchronously, before the awaited network logout, so an interleaved re-login or a late /sync response can't act on a half-reset session.

The fixes

• Bug 1 (already on this branch): resetState() leaves the once-fulfilled #clientReadyDeferred alone rather than recreating it unfulfilled.
• Bug 2: the panel re-arms via sessionStarted() → ensureRoomsLoaded(); the remount-dependent constructor arming is removed. openPanel() keeps performing loadRoomsTask unconditionally (an explicit open must reselect the persisted/latest room); ensureRoomsLoaded()'s "leave the current room alone" guard is only for the passive session-start hook.
• Bug 3: RealmServerService.resetState() no longer clears the app-scoped eventSubscribers; BillingService.sessionStarted() repopulates its session-scoped data on re-login.

Audit / hygiene

• matrix-sdk-loader is removed from the participant registry — the loaded SDK bundle is app-scoped and must survive logout (its resetState() clearing was a latent bug).
• message-service's subscriber-map wipe is kept (its callbacks are component/resource-scoped and die with the <Auth/> swap) but documented as a test-environment safety net, distinct from the app-scoped wiring in realm-server.
• The remaining ~20 resetState() implementations are confirmed either explicitly re-driven by start() or lazily derived, so they need no hook.

Testing

• New: session-test.ts (broadcasts reach all participants, error isolation both directions, isAuthenticated flips, replay semantics); ai-assistant-panel-relogin-test.ts (panel re-enters a room after logout → re-login instead of spinning); realm-server-subscriber-test.ts (subscribers survive resetState()). Bug 1's matrix-service-relogin-test.ts remains.
• pnpm lint on @cardstack/host is green (lint:js, lint:types/glint, lint:hbs).
• The browser suite (integration/acceptance, plus the matrix E2E re-login spec) runs on CI — worth watching the panel re-login integration test, which drives real room creation.

Out of scope

Reload-on-logout (window.location.replace('/') instead of an in-app transition, matching ember-simple-auth's default) is a separate product decision. The session registry here is needed regardless (tests, the failed-boot start() → logout() path, any future soft-logout).

🤖 Generated with Claude Code

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

  1 files  ±    0    1 suites  ±0   5m 21s ⏱️ - 2h 54m 11s
152 tests  - 3 412  152 ✅  - 3 397  0 💤  - 15  0 ❌ ±0 
152 runs   - 3 431  152 ✅  - 3 416  0 💤  - 15  0 ❌ ±0 

Results for commit c7f5e4a. ± Comparison against earlier commit f9f2ee2.

Realm Server Test Results

    1 files  ± 0      1 suites  ±0   11m 48s ⏱️ - 5m 19s
1 913 tests +11  1 913 ✅ +11  0 💤 ±0  0 ❌ ±0 
1 992 runs  +11  1 992 ✅ +11  0 💤 ±0  0 ❌ ±0 

Results for commit c7f5e4a. ± Comparison against earlier commit f9f2ee2.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f9f2ee2d92

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Coalesce the billing load started by session replay

When BillingService is first touched by the index route after matrixService.start(), session.register(this) replays sessionStarted() because session.isAuthenticated is already true. This line starts loadSubscriptionData() fire-and-forget; immediately afterward routes/index.gts calls initializeSubscriptionData() and, since subscriptionData is still null while the first request is in flight, issues a second _user request. The same duplicate fetch happens after re-login because resetState() clears the data before the session-start broadcast, so this should share an in-flight/initialized path or avoid the eager load when the route will initialize it.

Useful? React with 👍 / 👎.

[FadhlanR]

[Claude Code 🤖]

Traced and confirmed — efficiency, not correctness. We're keeping this as-is rather than coalescing. The eager load is the only path that repopulates the cleared subscription data on re-login (where the index route's model() may not re-run), and coalescing was tried and reverted earlier on this branch because routing the profile popover through the present-or-load path showed stale credits after a billing-notification push.

Documented the accepted duplicate at the sessionStarted() call site in c7f5e4a so the decision is visible here rather than only on loadSubscriptionData().

[habdelra]

[Claude Code 🤖]

Review lens: I focused on the session-lifecycle contract this PR introduces — whether every resetState() teardown has a matching re-establishment, whether the app-scoped vs. session-scoped reclassifications are correct, and whether the new broadcast edges (start-success, logout, replay-on-register) each fire exactly once without racing an in-flight /sync or a re-login. I checked out the head branch and traced each edge through the code.

Bottom line: no blocking issues. The refactor is sound and all three bug fixes are real and verified. Auth-state accounting is consistent (every isAuthenticated write routes through MatrixService, and #clientReadyDeferred correctly survives resetState()), no lingering reset-service references remain anywhere, and the ~20 trivial reset → session renames are behavior-preserving — those services define no sessionStarted(), so replay-on-register is a no-op for them, making the blast radius genuinely small. One minor efficiency nit and two test-coverage follow-ups, all non-blocking.

What lands right (mechanistically):

• Bug 1 — verified: #clientReadyDeferred is fulfilled exactly once in loadSDK(), and createRealmSession() awaits it. Leaving it untouched in resetState() (which recreates _client synchronously whenever the SDK is loaded) preserves the "fulfilled whenever a client exists" invariant; the old fresh-Deferred stranded every post-logout await forever.
• Bug 3 — verified: subscribeEvent() has exactly one caller (BillingService's constructor, an app-lifetime singleton), so preserving eventSubscribers across resetState() is correct. The integration test drives handleEvent() after a resetState() and asserts delivery — a real pin, not a shape assertion.
• Edge-guarded broadcast — capturing wasAuthenticated before setPostLoginCompleted(true) is the correct ordering; a /connect re-run of start() on an already-established session correctly no-ops the broadcast.
• Synchronous logout teardown — capturing client before resetState() keeps the network logout pointed at the old client while closing the window where an in-flight /sync or a re-login-during-await could act on a half-reset session.
• The inline comments throughout are unusually good: they explain the why at each seam, which is what makes a refactor this broad reviewable.

On the open Codex thread (billing double-fetch): confirmed real and traced — it's a minor efficiency issue, not correctness. Mechanism plus a narrow fix that avoids the previously-reverted stale-popover regression is in the inline comment on billing-service.ts.

Recommendations (all non-blocking):

1. Billing boot double-fetch — see inline on billing-service.ts sessionStarted().
2. Add a unit test for the snapshot-iteration double-invocation guard — see inline on session.ts notifySessionStarted(). The same test file could also pin the start-success edge guard (assert a second start() on an established session does not re-broadcast).
3. Consider whether the test-only participant-error surfacing (reraiseParticipantErrorsInTests → floated Promise.reject) can misattribute failures: it runs inside notifySessionEnded() in the afterEach teardown, so a rejection floated there may be observed by the next test's unhandledrejection hook rather than the one whose resetState() actually threw. The loud-failure intent is right; worth confirming the attribution is deterministic. CI is currently green.

Adjacent, out of scope: the ai-assistant-panel-relogin-test drives matrixService.resetState() before session.notifySessionEnded(), the reverse of logout()'s real order (notifySessionEnded() then resetState()). The end state is identical so the test is valid, but the accompanying comment describes the production order. Not asked of this PR.

---

Generated by Claude Code

[habdelra]

[Claude Code 🤖]

Confirmed: a duplicate /_user fetch fires at boot. Verified end-to-end, non-blocking, low severity. This is the Codex P2 lead, traced.

The mechanism. The index route's model() does await this.matrixService.start(); start() fires notifySessionStarted() on the auth edge, which reaches BillingService.sessionStarted() here and kicks off loadSubscriptionData() fire-and-forget (fetch #1). A few lines later the same model() calls this.billingService.initializeSubscriptionData(); subscriptionData is still null because fetch #1 hasn't resolved, so it falls through to a second loadSubscriptionData() (fetch #2). Two GETs to /_user, same server state, last-writer-wins on _subscriptionData — so this is efficiency, not correctness, and it's newly introduced here (before, only the route fetched).

Why the documented anti-coalescing rationale doesn't quite cover this. The comment on loadSubscriptionData() says coalescing was reverted because it made the profile popover show stale credits. But the popover mounts <WithSubscriptionData>, whose constructor calls loadSubscriptionData() directly, not initializeSubscriptionData(). So the boot double-fetch lives entirely on the sessionStarted() + route-initializeSubscriptionData() pair. Coalescing those two (in-flight tracking inside initializeSubscriptionData(), leaving loadSubscriptionData() always-fresh for the popover) would drop the duplicate without reintroducing the stale-popover regression.

Scope. Non-blocking follow-up. Don't just delete the eager load — it's genuinely needed on re-login, where the route model() may not re-run and sessionStarted() is the only thing that repopulates the cleared data. If one extra boot request isn't worth the machinery, leaving it is defensible; flagging so the choice is deliberate rather than incidental.

---

Generated by Claude Code

[FadhlanR]

[Claude Code 🤖]

Agreed on all points, and thanks for tracing the narrower initializeSubscriptionData()-only coalescing — you're right that it would avoid the reverted stale-popover regression (the popover calls loadSubscriptionData() directly). We're still choosing to leave it: one duplicate /_user at boot isn't worth reintroducing the in-flight machinery, and the eager load has to stay for the re-login repopulation path.

Made the choice explicit in c7f5e4a — added a comment at sessionStarted() noting the accepted duplicate and pointing at the revert rationale on loadSubscriptionData().

[habdelra]

[Claude Code 🤖]

Confirmation + a test-coverage gap on this exact guard. Iterating a snapshot ([...this.participants]) is the right call, and the inline comment explains the mechanism precisely: since isAuthenticated is set true at the top of this method, a participant whose sessionStarted() lazily first-injects another participant service triggers that service's constructor → register(), and register() replays sessionStarted() immediately (because the session is already established). Without the snapshot, the live-array loop would then also reach that late registrant — a double sessionStarted() in one broadcast.

The gap. session-test.ts covers replay-on-register, error isolation on both edges, and isAuthenticated flips — but nothing exercises this double-invocation guard. Reverting to for (let p of this.participants) would pass the entire existing suite while silently re-introducing the double invocation (extra re-arm work, no crash) — exactly the kind of regression that needs a pin.

Concrete test. Register participant A whose sessionStarted() calls session.register(B); after notifySessionStarted(), assert B.startedCount === 1 (not 2). The same shape covers the notifySessionEnded() snapshot on the teardown edge.

Class: follow-up (test coverage), non-blocking.

---

Generated by Claude Code

[FadhlanR]

[Claude Code 🤖]

Added in c7f5e4a. Two unit tests pin the guard on both edges:

• notifySessionStarted(): register A whose sessionStarted() calls session.register(B), then assert B.startedCount === 1 — the replay only, not a second invocation from the loop.
• notifySessionEnded(): symmetric shape asserting a participant registered mid-teardown is not reached by the same resetState() pass.

Reverting either broadcast to a live-array for…of now fails these.

[FadhlanR]

[Claude Code 🤖]

Thanks for the thorough pass. Addressed the follow-ups in c7f5e4a (all non-blocking):

• Participant-error attribution (rec Sprite which is removed from one part of AnimationContext subtree and added to another is classified as a keptSprite #3): switched from a floated Promise.reject to buffering the errors on SessionService and draining them in the setup.ts afterEach. A broken participant now fails the test that actually caused it deterministically, instead of possibly landing on the next test's unhandledrejection hook — and it still can't be swallowed by start()/logout()'s try/catch.
• Snapshot double-invocation guard (rec far-match animate Sprite C moving between animation contexts in Basic example #2): unit tests added on both edges (replied inline).
• Edge-guard coverage (rec far-match animate Sprite C moving between animation contexts in Basic example #2, second part): integration test asserting a second start() on an already-authenticated session does not re-broadcast sessionStarted().
• Billing double-fetch (rec Bring in a demo from ember-animated #1): documented as an accepted cost rather than coalesced (replied inline).

On the adjacent note — ai-assistant-panel-relogin-test driving resetState() before notifySessionEnded(), the reverse of logout()'s real order: left as-is since the end state is identical, but happy to reorder the test + its comment to mirror production if you'd prefer.