From 4720ef3c912b046d0b6bdad3d344c2a991bcce93 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Fri, 31 Jul 2026 14:58:48 +0300 Subject: [PATCH] cull comments --- src/state/session/index.tsx | 232 ++++-------------------------- src/state/session/logging.ts | 5 +- src/state/session/moderation.ts | 59 +------- src/state/session/reducer.ts | 49 +------ src/state/session/session-core.ts | 211 ++++----------------------- src/state/session/types.ts | 18 +-- src/state/session/util.ts | 15 +- 7 files changed, 71 insertions(+), 518 deletions(-) diff --git a/src/state/session/index.tsx b/src/state/session/index.tsx index fbbe79c628..13c24dd56b 100644 --- a/src/state/session/index.tsx +++ b/src/state/session/index.tsx @@ -130,21 +130,11 @@ export function Provider({children}: React.PropsWithChildren<{}>) { const state = useSyncExternalStore(store.subscribe, store.getState) const onboardingDispatch = useOnboardingDispatch() - /* - * Failed-token loop guard for the expiry rescue below. Maps a did to the set - * of refreshJwts that have already produced an 'expired'. Before rescuing - * from a candidate we require its refreshJwt not be in this set, and every - * expiry records its dying token here; a successful 'update' clears the set. - * See the rescue docblock in onSessionChange for why this is a set (not a - * single-shot flag) and why it stays bounded. - */ + // Refresh-token generations that have already failed during expiry rescue. const failedExpiryTokensRef = useRef>>(new Map()) /* - * Self-reference shim. The rescue path rebuilds a bundle and must wire it to - * this same onSessionChange (so the rescued bundle's own future events flow - * back here). Referencing onSessionChange inside its own useCallback body - * would be an unsatisfiable exhaustive-deps cycle, so we thread it through a - * ref kept current right after the callback is defined. + * Rescued bundles need this callback for their own events. A ref avoids a + * self-reference in the callback's dependency list. */ const onSessionChangeRef = useRef< | (( @@ -163,26 +153,13 @@ export function Provider({children}: React.PropsWithChildren<{}>) { sessionEvent: AtpSessionEvent, sessionData?: SessionData, ) => { - /* - * A successful refresh means this did's world is healthy again, so reset - * its failed-token guard set (a later expiry starts a fresh rescue - * budget). - */ if (sessionEvent === 'update' && sessionData) { failedExpiryTokensRef.current.get(accountDid)?.clear() } /* - * Build the refreshed account from the payload the hook delivers, NOT the - * live session getter. `PasswordSession` fires onUpdated/onDeleted BEFORE - * it commits its internal `#sessionData` (see password-session.js), so at - * hook time `bundle.session.session` still holds the OLD tokens (and, on - * the expiry path, `destroyed` is still false). Reading the live getter - * here would (a) persist stale tokens on 'update' -> eventual forced - * logout once the real refresh token expires, and (b) keep the user - * signed in on 'expired'. On 'update' the payload carries the new session; - * on 'expired'/'create-failed' we force it undefined so the reducer clears - * tokens and logs out (it treats undefined as "session gone"). + * PasswordSession invokes its hooks before updating its live getter. Use + * the delivered payload so a refresh persists the newly rotated tokens. */ const refreshedAccount = sessionEvent === 'update' && sessionData @@ -190,40 +167,10 @@ export function Provider({children}: React.PropsWithChildren<{}>) { : undefined /* - * Expiry rescue (compare-and-clear + resume-from-newer). THE BUG: a stale - * tab (frozen by Chrome, a failed localStorage write, or a native app - * killed before its async persist landed) can wake holding a >2h-old - * refresh token. Its refresh gets ExpiredToken, and the naive 'expired' - * handling clears the tokens in storage and logs out EVERY tab - even - * though another tab already rotated to a healthy generation (PDS refresh - * tokens keep a 2h grace window after rotation). - * - * Fix: before letting an expiry become a logout, check whether a NEWER - * generation for this did is known, and if so rebuild the current bundle - * from it instead of dropping the session. The compare lives here at the - * dispatch site (not in the reducer) because the reducer is pure and has - * no access to persisted storage. - * - * Two freshness sources, tried in order: - * - persisted.readLatest('session'): on web this re-reads localStorage - * directly, covering the frozen-tab case where queued cross-tab - * broadcasts have not been processed yet (so both the reducer state and - * persisted's in-memory cache are stale). On native it equals `get`. - * - the reducer's accounts: on native this IS the truth; on web it is - * kept fresh by 'synced-accounts' broadcasts. - * On native the two always agree, so the rescue effectively never fires - * (the dying bundle is the only generation) and expiry falls straight - * through to logout. On web, readLatest is what sees the healthy tokens. - * - * Termination: a rescued bundle that expires AGAIN now matches persisted - * (this tab wrote nothing newer), so no newer candidate exists and it - * falls through to a real logout. The failed-token set is the belt-and- - * suspenders bound - each rescue consumes a strictly newer generation, so - * the set grows by at most one per expiry and is hard-capped - * (MAX_EXPIRY_RESCUE_GENERATIONS). A set rather than a single-shot flag is - * required: with a flag, a second expiry would fall through to logout and - * clobber a healthy THIRD generation another tab just wrote, recreating - * the exact bug. + * A stale tab may expire a token after another tab has already rotated it. + * Prefer a newer persisted or reducer generation over logging every tab + * out. Failed generations are recorded and bounded to guarantee that a + * repeatedly expiring session eventually falls through to logout. */ if (sessionEvent === 'expired') { const current = store.getState() @@ -231,21 +178,12 @@ export function Provider({children}: React.PropsWithChildren<{}>) { | SessionBundle | PublicSessionBundle const dyingRefreshJwt = sessionData?.refreshJwt - /* - * The rescue only applies when the expiring bundle IS the current one. - * Otherwise fall through: the reducer's identity guard drops a stale - * bundle's expiry anyway. - */ + // Stale bundle events are handled by the reducer's identity guard. if ( currentAgent === bundle && current.currentAgentState.did === accountDid && dyingRefreshJwt ) { - /* - * Record the dying token FIRST (at the start of handling), so a - * rescued-then-failed generation is remembered and never rescued back - * into. - */ let failedSet = failedExpiryTokensRef.current.get(accountDid) if (!failedSet) { failedSet = new Set() @@ -253,10 +191,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) { } failedSet.add(dyingRefreshJwt) - /* - * Prefer the persisted re-read over the reducer state: storage is the - * cross-tab source of truth on web (on native they are identical). - */ const persistedCandidate = persisted .readLatest('session') .accounts.find(a => a.did === accountDid) @@ -270,12 +204,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) { }) if (candidate) { - /* - * Rebuild a bundle from the newer tokens synchronously, modeled on - * the same-did rebuild in the persisted.onUpdate handler below. No - * expiry is dispatched and no emitSessionDropped fires - the session - * is not dropped, it is healed. - */ let newBundle!: SessionBundle const hooks = makeSessionHooks( onSessionChangeRef.current!, @@ -289,11 +217,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) { newBundle = buildBundle(newSession) registerBundleKillSwitch(newBundle, hooks.kill) configureModerationForAccount(newBundle, candidate) - /* - * Re-snapshot through the freshly built session (fallback covers - * the destroyed case, which cannot happen for a just-built, - * never-armed session). - */ const newAccount = newBundle.session.destroyed ? candidate : (sessionDataToSessionAccount( @@ -311,32 +234,14 @@ export function Provider({children}: React.PropsWithChildren<{}>) { } } - /* - * Fall-through-to-logout path (no rescue was taken). emitSessionDropped - * fires here - never on the rescue path, where the session survives. - * 'create-failed' never fires in production but is kept for parity. - * - * Gate on the expiring bundle still being current: the reducer drops - * events from non-current bundles, and disposal of a replaced bundle - * happens in a deferred useEffect. A stale-but-still-armed bundle expiring - * in that window must not show a spurious "session expired" toast while - * the current session is healthy - only emit when a CURRENT bundle truly - * expires with no rescue. - */ + // Only the current bundle may report that its session was dropped. if ( (sessionEvent === 'expired' || sessionEvent === 'create-failed') && store.getState().currentAgentState.agent === bundle ) { emitSessionDropped() } - /* - * The bundle is the reducer's identity token: it stores the whole bundle - * as `currentAgentState.agent` and compares `action.agent` by identity to - * decide whether an event belongs to the active account. A same-bundle - * event acts on the active account; a stale (background) bundle does not - * match, so its events are ignored (background accounts must not be able - * to log the current user out or resurrect tokens). - */ + // Bundle identity prevents stale sessions from changing the active account. store.dispatch({ type: 'received-agent-event', agent: bundle, @@ -482,24 +387,14 @@ export function Provider({children}: React.PropsWithChildren<{}>) { ) if (signal.aborted) { - /* - * A newer task superseded this resume. The bundle is fully built and - * armed, so its session can still consume refresh tokens - dispose it - * before bailing (fixes a leak where an aborted resume left an armed, - * undisposed bundle behind). - */ + // The factory returns an armed bundle, so a superseded resume must dispose it. disposeBundle(bundle) return } /* - * Completion bail: re-read state and drop out if this account's entry is - * gone, or its tokens were cleared by a cross-tab logout that raced this - * resume (the residual hole where the leader logged in X then out while - * this follower's current did was still undefined, so the onUpdate cancel - * in 2a did not fire). The check is on the ACCOUNTS entry, not on - * "persisted current did": a persisted-current-did check would break - * normal user-initiated account switching, where the target account is - * deliberately not current yet. + * A cross-tab logout may clear or remove the account while resume is in + * flight. Check the account entry rather than the current did so ordinary + * account switching remains valid. */ const latest = store.getState() const latestEntry = latest.accounts.find(a => a.did === account.did) @@ -526,14 +421,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) { >(async () => { const bundle = state.currentAgentState.agent as unknown as SessionBundle const signal = cancelPendingTask() - /* - * Fetch through the merged Bluesky client and dispatch the patch. getSession - * must hit the user's PDS, not the appview proxy, so this raw call passes - * `{service: null}` to strip the instance's appview `atproto-proxy` header. - * We do NOT mutate the session object (PasswordSession's data is immutable to - * us); the reducer patches only the `accounts` entry, and the email-state - * hook reads from the account rather than the session. - */ + /* getSession targets the PDS; only the persisted account fields are patched. */ const data = await bundle.bskyClient.call( com.atproto.server.getSession, {}, @@ -557,13 +445,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) { | SessionBundle | PublicSessionBundle if (!bundle.session) return undefined // logged out: nothing to refresh - /* - * refresh() fires the session's onUpdated hook on success, which the armed - * hooks map to an 'update' event; the reducer snapshots the refreshed - * account, so no explicit dispatch is needed here. The returned snapshot - * lets callers read post-refresh fields without waiting on the (async) - * reducer update. - */ + // The hook updates state; the return value exposes fresh fields immediately. await bundle.session.refresh() return sessionDataToSessionAccount( bundle.session.session, @@ -601,15 +483,8 @@ export function Provider({children}: React.PropsWithChildren<{}>) { a => a.did === synced.currentAccount?.did, ) /* - * The leader tab's current account, but only if it still has tokens - - * a tokenless entry means the leader logged out. When the leader logged - * out (`syncedDid === undefined`) while THIS tab thinks it is logged in, - * cancel any pending task so a resume racing this logout does not win and - * dispatch a switch over the top of the synced logout. We do NOT cancel - * unconditionally on every no-current broadcast: a logged-out tab may be - * mid-login, and another logged-out tab removing a stored account must not - * abort that unrelated in-flight login. resumeSession already cancels at - * its start, so the different-did case is covered elsewhere. + * Cancel pending work when another tab logs out the account this tab + * considers current. Do not cancel unrelated work between logged-out tabs. */ const syncedDid = syncedAccount?.refreshJwt ? syncedAccount.did @@ -622,35 +497,17 @@ export function Provider({children}: React.PropsWithChildren<{}>) { } if (syncedAccount && syncedAccount.refreshJwt) { if (syncedAccount.did !== state.currentAgentState.did) { - /* - * Web handling: if leader tab has switched to a diff account that is - * stale, it will refresh the session before triggering the update to - * follower tabs. Follower tabs will therefore receive the fresh - * session. See APP-1960, or ask Eric. - */ + // The leader refreshes before broadcasting, so followers receive fresh tokens. void resumeSession(syncedAccount) } else { /* - * Same account, new tokens synced from the leader tab. PasswordSession - * is immutable (no in-place session patch), so rebuild a fresh bundle - * from the synced tokens WITHOUT a network call (the leader already - * refreshed) and swap it in via `replaced-current-bundle`. The - * bundle-identity effect disposes the previous session once it swaps, - * which strengthens the single-refresher guarantee (the stale-token - * session can no longer refresh). + * PasswordSession cannot be patched in place. Rebuild from the tokens + * the leader already refreshed, then dispose the previous bundle. */ const prevBundle = state.currentAgentState.agent as unknown as | SessionBundle | PublicSessionBundle - /* - * Any change to ANY saved account fires persisted.onUpdate, and the - * 'synced-accounts' dispatch above already keeps the accounts list - * fresh. So if the CURRENT account's tokens are unchanged, a change to - * a non-current account landed here: bail out before rebuilding, since - * rebuild+swap would kill the live bundle (client-identity churn, - * in-flight request kills) for no reason. Fall through to rebuild only - * when we have no usable live session. - */ + // Avoid replacing the live bundle for an unrelated account update. const live = prevBundle.session && !prevBundle.session.destroyed ? prevBundle.session.session @@ -674,31 +531,11 @@ export function Provider({children}: React.PropsWithChildren<{}>) { ) newBundle = buildBundle(newSession) registerBundleKillSwitch(newBundle, hooks.kill) - /* - * Reapply this account's subscribed labelers to the freshly built - * merged Bluesky client: buildBundle starts with an empty per-instance - * labeler set, and this rebuild path never runs - * configureModerationForAccount on its own. It is fully synchronous - * (the labeler cache is a local MMKV read), so the whole prep + arm + - * dispatch sequence below runs in one tick from the onUpdate - * broadcast - the new bundle enters the reducer with its labelers - * already applied, with no window where the new session is armed but - * the reducer still holds the old bundle. - */ + // Apply cached labelers before the new session is armed and installed. configureModerationForAccount(newBundle, syncedAccount) /* - * Defensive race guard. With the whole path synchronous, nothing can - * have dispatched between the 'synced-accounts' dispatch above and - * here, so these conditions are trivially satisfied today. They are - * kept as a cheap invariant check against a future edit reintroducing - * an await into this path: a competing rebuild, an account switch, a - * logout, or a newer token generation would each show up as a - * bundle-identity or token mismatch, and the stale completion must - * drop out (self-disposing the never-installed bundle) rather than - * clobber the newer bundle or resurrect an authenticated bundle into a - * logged-out/other-account slot (the reducer's - * 'replaced-current-bundle' keeps the current did and does no identity - * check on the outgoing agent). + * If this path becomes asynchronous, do not let a stale rebuild + * replace a newer bundle or token generation. */ const current = store.getState() const latestAccount = current.accounts.find( @@ -709,11 +546,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) { latestAccount?.accessJwt !== syncedAccount.accessJwt || latestAccount?.refreshJwt !== syncedAccount.refreshJwt ) { - /* - * This bundle was never armed and never installed, so dispose it - * here (the install path's normal disposal in the bundle-identity - * effect will never run for it). - */ + // This bundle was never installed, so the normal disposal effect cannot run. disposeBundle(newBundle) return } @@ -726,11 +559,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) { : undefined, nextSession: newBundle.session.session, }) - /* - * Re-read syncedAccount's data through the freshly built session (the - * fallbacks cover the destroyed case, which cannot happen here since - * the session was just built synchronously and never armed). - */ const newAccount = newBundle.session.destroyed ? syncedAccount : (sessionDataToSessionAccount( @@ -793,7 +621,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) { const currentBundleRef = useRef(bundle) useEffect(() => { if (currentBundleRef.current !== bundle) { - // Read the previous value and immediately advance the pointer. const prevBundle = currentBundleRef.current currentBundleRef.current = bundle addSessionDebugLog({ @@ -801,8 +628,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) { prevAgent: prevBundle, nextAgent: bundle, }) - // We never reuse bundles so let's fully neutralize the previous one. - // This ensures its session won't try to consume any refresh tokens. + // Replaced bundles must never consume another refresh token. disposeBundle(prevBundle) } }, [bundle]) diff --git a/src/state/session/logging.ts b/src/state/session/logging.ts index 93f0bace31..3be52525dc 100644 --- a/src/state/session/logging.ts +++ b/src/state/session/logging.ts @@ -69,9 +69,6 @@ export function wrapSessionReducerForLogging(reducer: Reducer): Reducer { } } -/** - * Stubs, previously used to log session errors to Statsig. We may revive this - * using Sentry or Bitdrift in the future. - */ +/** Reserved session logging hooks; currently no-ops. */ export function addSessionErrorLog(_did: string, _event: AtpSessionEvent) {} export function addSessionDebugLog(_log: Log) {} diff --git a/src/state/session/moderation.ts b/src/state/session/moderation.ts index b089bbc2a4..6107c5b4be 100644 --- a/src/state/session/moderation.ts +++ b/src/state/session/moderation.ts @@ -8,17 +8,9 @@ import {configureAdditionalModerationAuthorities} from './additional-moderation- import {type SessionBundle} from './session-core' import {type SessionAccount} from './types' -/* - * The Bluesky moderation labeler (`api.moderation.did`) flows ONLY through the - * global `Client.appLabelers` config: lex-client merges the static appLabelers - * with each client's per-instance `labelers` into the `atproto-accept-labelers` - * header on every request, and appLabelers entries carry the `;redact` suffix - * (redaction authority) while plain per-instance labelers don't. - */ - /** * Set the global app labelers on the lex `Client` static so every client emits - * the same global (`;redact`-suffixed) `atproto-accept-labelers` header. + * the same `;redact` moderation authorities. */ function configureGlobalAppLabelers(dids: string[]) { Client.configure({appLabelers: dids as `did:${string}:${string}`[]}) @@ -46,19 +38,8 @@ export function readLabelers(did: string): string[] | undefined { } /** - * Apply an account's subscribed labeler DIDs to a live client. The lex `Client` - * rebuilds the header per request, so this takes effect on the next request - * without a client rebuild. - * - * These labelers ride the `atproto-accept-labelers` header, which lex-client - * 0.3.0 emits only on raw/query calls: record helpers default `labelers = null` - * per call, stripping the header. That is fine here - labelers only matter for - * the read/query calls moderation cares about. - * - * We filter out the Bluesky moderation labeler: it is already asserted globally - * via `Client.appLabelers` (with `;redact`), and a user "subscribing" to it - * must not add a second, plain (non-redact) header entry alongside the redacted - * one. + * Apply account subscriptions without duplicating the globally redacted + * Bluesky moderation authority. */ export function applyLabelersToClient( client: Client, @@ -69,47 +50,26 @@ export function applyLabelersToClient( } export function configureModerationForGuest() { - // This global mutation is *only* OK because this code is only relevant for testing. - // Don't add any other global behavior here! switchToBskyAppLabeler() configureAdditionalModerationAuthorities() } -/** - * Configure moderation labelers for a signed-in account. Fully synchronous: - * the labeler cache is a local MMKV read, so the bundle leaves here with its - * per-account labelers already applied, in the same tick. - * - * Takes the whole {@link SessionBundle} so it can apply per-account labelers to - * the single merged Bluesky client (`bundle.bskyClient`, backing - * `useLexClient()`). - */ +/** Configure global authorities and cached account subscriptions. */ export function configureModerationForAccount( bundle: SessionBundle, account: SessionAccount, ) { - // This global mutation is *only* OK because this code is only relevant for testing. - // Don't add any other global behavior here! switchToBskyAppLabeler() if (IS_TEST_USER(account.handle)) { - /* - * Fire-and-forget: this resolves a handle over the network and only runs - * in the test environment. Requests made before it lands use the standard - * Bluesky app labeler; that race is acceptable for tests. - */ + // Test accounts may briefly use the production authority while this resolves. void trySwitchToTestAppLabeler(bundle) } - // The code below is actually relevant to production (and isn't global). const labelerDids = readLabelers(account.did) if (labelerDids) { applyLabelersToClient(bundle.bskyClient, labelerDids) } else { - /* - * No cached labelers yet (first session on this device), so the initial - * requests go out without them. We could block on the preferences query - * here to fix that, but choose not to. - */ + // The preferences query populates the cache after the initial requests. } configureAdditionalModerationAuthorities() @@ -119,12 +79,7 @@ function switchToBskyAppLabeler() { configureGlobalAppLabelers([api.moderation.did]) } -/** - * In the test environment, swap the global app labeler for the test-env - * moderation authority, resolving its handle via the bundle's merged Bluesky - * client. This is a raw call, so it inherits the appview proxy - which is - * correct, as `resolveHandle` is served by the appview. - */ +/** Resolve and install the test environment's moderation authority. */ async function trySwitchToTestAppLabeler(bundle: SessionBundle) { const did = ( await bundle.bskyClient diff --git a/src/state/session/reducer.ts b/src/state/session/reducer.ts index 7ac99c7d9b..cc9fc453f5 100644 --- a/src/state/session/reducer.ts +++ b/src/state/session/reducer.ts @@ -5,12 +5,7 @@ import {type AtpSessionEvent, createPublicSessionBundle} from './session-core' import {type SessionAccount} from './types' import {createTemporaryClientsAndResume} from './util' -/* - * A hack so the reducer can't read anything from the session bundle. The - * provider stores the full `SessionBundle` here, but the reducer's static type - * only sees `service` (a URL, used for logging/snapshots) - structurally the - * bundle has more, the reducer sees less. - */ +// Keep session internals outside the reducer's static view of a bundle. type OpaqueSessionBundle = { readonly service: URL } @@ -23,7 +18,7 @@ type AgentState = { export type State = { readonly accounts: SessionAccount[] readonly currentAgentState: AgentState - needsPersist: boolean // Mutated in an effect. + needsPersist: boolean // Cleared after persistence is scheduled. } export type Action = @@ -40,19 +35,7 @@ export type Action = newAccount: SessionAccount } | { - /* - * Swap the current bundle in place, keeping the current did and replacing - * the matching account entry, without persisting (avoid write cycles). - * `PasswordSession` cannot be patched in place, so the provider rebuilds a - * fresh bundle from a set of tokens and swaps it in. Two producers: - * - * - Same-did cross-tab sync: the leader tab refreshed and broadcast the - * new tokens; this tab rebuilds from them (no network). - * - Expiry rescue: the current bundle's refresh token expired, but a - * newer generation for the same did is known (from reducer state or a - * fresh persisted re-read), so the provider rebuilds from that newer - * generation instead of logging out (see onSessionChange in index.tsx). - */ + // Replace an immutable session from synced or rescued tokens without rebroadcasting. type: 'replaced-current-bundle' newAgent: OpaqueSessionBundle newAccount: SessionAccount @@ -99,21 +82,8 @@ let reducer = (state: State, action: Action): State => { const {agent, accountDid, refreshedAccount, sessionEvent} = action if (agent !== state.currentAgentState.agent) { /* - * Any event from a bundle that is not the current one is dropped - * entirely, in BOTH directions: - * - * - A clear (expiry/network-error, refreshedAccount === undefined) from - * a stale background bundle must not log the current user out. If the - * problem is transient, it works on the next resume. - * - An update (refreshedAccount present) from a stale bundle must not - * resurrect tokens: a refresh that completes after this bundle was - * logged out / switched away from would otherwise write fresh tokens - * back into a soft-logged-out (or switched-away) account entry. - * - * Trade-off: a background bundle's in-flight refresh that lands inside - * the disposal window now has its (already server-side-rotated) tokens - * discarded. The stored generation stays valid within the PDS 2h grace - * window, so this is strictly better than the resurrection bug. + * Stale bundles must neither log out the current account nor restore + * tokens after logout or an account switch. */ return state } @@ -126,7 +96,6 @@ let reducer = (state: State, action: Action): State => { !existingAccount || JSON.stringify(existingAccount) === JSON.stringify(refreshedAccount) ) { - // Fast path without a state update. return state } return { @@ -183,7 +152,6 @@ let reducer = (state: State, action: Action): State => { case 'removed-account': { const {accountDid} = action - // side effect const account = state.accounts.find(a => a.did === accountDid) if (account) { createTemporaryClientsAndResume([account]) @@ -211,7 +179,6 @@ let reducer = (state: State, action: Action): State => { case 'logged-out-current-account': { const {currentAgentState} = state const accountDid = currentAgentState.did - // side effect const account = state.accounts.find(a => a.did === accountDid) if (account && accountDid) { createTemporaryClientsAndResume([account]) @@ -276,11 +243,7 @@ let reducer = (state: State, action: Action): State => { case 'partial-refresh-session': { const {accountDid, patch} = action - /* - * Patch only the account entry: `PasswordSession` has no public session - * setter, and consumers that read these fields (useAccountEmailState) - * read from `currentAccount` rather than the session. - */ + // PasswordSession has no setter; consumers read these fields from the account. return { ...state, accounts: state.accounts.map(a => { diff --git a/src/state/session/session-core.ts b/src/state/session/session-core.ts index 89c4385a31..1417ce1dc6 100644 --- a/src/state/session/session-core.ts +++ b/src/state/session/session-core.ts @@ -65,10 +65,6 @@ export type AtpSessionEvent = /** * Whether an access token was issued for a queued (waitlisted) signup rather * than a full session. - * - * Canonical implementation - util.ts re-exports it. It lives here (rather than - * util.ts) so this module stays dependency-light: util.ts transitively pulls in - * a large chunk of the app. */ export function isSignupQueued(accessJwt: string | undefined) { if (accessJwt) { @@ -245,10 +241,6 @@ export function sessionAccountToSessionData( } } -/** - * The service (entryway) URL for a session, or the public appview URL when - * logged out / destroyed. Backs the {@link SessionBundle.service} getter. - */ function deriveServiceUrl(session: PasswordSession | null): URL { return new URL( session && !session.destroyed @@ -257,48 +249,23 @@ function deriveServiceUrl(session: PasswordSession | null): URL { ) } -/** - * The full set of read-through views over ONE `PasswordSession`. The - * `session` is the sole auth core (single refresher); the clients never refresh - * independently. - */ +/** Clients backed by one `PasswordSession`, the bundle's sole auth core. */ export type SessionBundle = { - /** The single auth core. Never exposed to the reducer. */ session: PasswordSession - /** - * The single authed Bluesky client (merged account + appview). Proxied to - * the Bluesky appview and carrying this account's labelers; record helpers - * on it auto-target the user's PDS. See {@link buildBskyClient}. - */ + /** Authed appview client whose record helpers target the account's PDS. */ bskyClient: Client - /** Chat client (proxied to `did:web:api.bsky.chat#bsky_chat`). */ chatClient: Client - /** - * The service (entryway) URL. Exposed so the reducer can read `.service` for - * its opaque snapshot/logging view (`OpaqueSessionBundle = {readonly service: - * URL}`) without reaching into the (never-exposed) session. - */ readonly service: URL } /** - * Kill-switches for live bundles, keyed by bundle identity. - * * `PasswordSession` exposes no local (logout-free) destroy, so disposal is - * implemented via a closure flag inside the session's injected `fetch` (see - * {@link makeSessionHooks}). The `kill()` that trips that flag is produced next - * to the hooks - before the bundle exists - so we stash it here once the bundle - * is built and look it up in {@link disposeBundle}. A `WeakMap` keeps this off - * the {@link SessionBundle} type (the reducer's opaque view must not see it) and - * lets the entry be GC'd with the bundle. + * implemented by disabling its injected fetch and hooks. Keep that lifecycle + * state private and tied to bundle identity. */ const bundleKillSwitches = new WeakMap void>() -/** - * Associate a bundle with the `kill()` from its {@link makeSessionHooks}, so - * {@link disposeBundle} can neutralize the underlying session. Call once, right - * after the bundle is built, at every session-construction site. - */ +/** Register the lifecycle closure used by {@link disposeBundle}. */ export function registerBundleKillSwitch( bundle: SessionBundle, kill: () => void, @@ -306,20 +273,11 @@ export function registerBundleKillSwitch( bundleKillSwitches.set(bundle, kill) } -/** - * Assemble a {@link SessionBundle} from a live session: the merged Bluesky - * client and the chat client, both read-through views over the one session. - */ export function buildBundle(session: PasswordSession): SessionBundle { return { session, - /* - * Starts with an empty per-account labeler set; configureModerationForAccount - * applies this account's labelers afterwards. - */ bskyClient: buildBskyClient(session, []), chatClient: buildChatClient(session), - /* A getter keeps `.service` live with the session's state (destroyed -> public). */ get service() { return deriveServiceUrl(session) }, @@ -327,21 +285,8 @@ export function buildBundle(session: PasswordSession): SessionBundle { } /** - * The session-change callback the provider passes into the hooks. - * - * The whole {@link SessionBundle} is handed through so the provider can snapshot - * the live session and use the bundle itself as the reducer's identity token. - * - * `sessionData` is the payload the library hands the hook. It is present on - * BOTH the `'update'` path (the fresh, rotated session) and the `'expired'` - * path (the DYING session's data, which `refresh()` passes to `onDeleted` - * BEFORE it nulls its internal `#sessionData`). It matters because - * `PasswordSession` fires `onUpdated`/`onDeleted` BEFORE committing - * `#sessionData` (see `refresh()`/`logout()` in password-session.js), so the - * live getter (`bundle.session.session`) still returns the OLD tokens at hook - * time. On `'update'` the provider builds the refreshed account from this - * argument; on `'expired'` it reads the dying refreshJwt from it to drive the - * compare-and-rescue at the dispatch site. + * PasswordSession delivers `sessionData` before updating its live getter. The + * provider uses that payload for rotated tokens and expiry rescue. */ type OnSessionChange = ( bundle: SessionBundle, @@ -351,22 +296,9 @@ type OnSessionChange = ( ) => void /** - * Build the `PasswordSession` hooks with an arm latch. - * - * `PasswordSession` fires `onUpdated` once during login/resume/createAccount - * before the factory returns. We must NOT dispatch that initial event, so hooks - * stay inert until `arm()` is called after the prepare tail resolves. - * - * `getBundle` is deferred because the bundle does not exist yet when the hooks - * are constructed (session first, then bundle built over it). - * - * The `fetch` option is wrapped in a kill-switch: `kill()` sets a closure flag - * so every subsequent request through this session - direct fetches AND the - * internal auto-refresh, which `PasswordSession` routes through the same - * captured `options.fetch` - throws instead of hitting the network. `kill()` - * also disarms the hooks so a disposed session can never dispatch into the - * reducer. This is the disposal mechanism {@link disposeBundle} relies on - * (`PasswordSession` exposes no local destroy). + * Hooks stay inert during initial session preparation. `kill()` disarms them + * and disables the injected fetch so a disposed session cannot refresh or + * dispatch. The bundle getters are deferred because hooks are created first. */ export function makeSessionHooks( onSessionChange: OnSessionChange, @@ -414,50 +346,21 @@ export function makeSessionHooks( }) } -/** - * The public (logged-out) bundle. Its `bskyClient` is the public appview client - * (reads work logged out); the chat client is the throwing unauthenticated - * client. - */ +/** Clients exposed while logged out. */ export type PublicSessionBundle = { session: null - /** - * The public appview client (reads work logged out). Logged-out WRITE - * protection is NOT a property of this client - it lives in the - * `usePdsClient` hook's fallback (which returns the throwing unauthenticated - * client when there is no session; see index.tsx). - */ bskyClient: Client - /** - * The throwing unauthenticated client (NOT the public client): chat is - * meaningless logged out, and `useChatClient()` must fail loudly rather than - * silently target the public appview. See {@link getUnauthenticatedClient}. - */ chatClient: Client - /** The public appview URL. See {@link SessionBundle.service}. */ readonly service: URL } -/** - * Build the logged-out bundle used before/without a session. Configures guest - * moderation as a side effect. - */ +/** Build the logged-out bundle and configure guest moderation. */ export function createPublicSessionBundle(): PublicSessionBundle { configureModerationForGuest() // Side effect but only relevant for tests const publicClient = getPublicLexClient() return { session: null, - /* - * The public client reads public data without auth. Logged-out write - * protection is enforced by the usePdsClient hook (which falls back to the - * throwing unauthenticated client when there is no session), NOT here - see - * index.tsx. - */ bskyClient: publicClient, - /* - * The chat client throws on use when logged out, so an unauthenticated chat - * call fails loudly instead of silently targeting the public appview. - */ chatClient: getUnauthenticatedClient(), service: new URL(PUBLIC_BSKY_SERVICE), } @@ -483,10 +386,7 @@ export async function createSessionBundleAndResume( let session: PasswordSession const sessionData = sessionAccountToSessionData(storedAccount) if (isSessionExpired(storedAccount)) { - /* - * Network resume (1 retry). resume() always refreshes; the initial - * onUpdated it fires is swallowed by the arm latch. - */ + // The arm latch swallows resume's initial onUpdated event. session = await networkRetry(1, () => PasswordSession.resume(sessionData, hooks), ) @@ -497,11 +397,7 @@ export async function createSessionBundleAndResume( bundle = buildBundle(session) registerBundleKillSwitch(bundle, hooks.kill) - /* - * Early snapshot: only used to configure moderation below (its handle/did are - * refresh-stable). The RETURNED account is re-snapshotted after the prep - * awaits (see below). - */ + // The returned account is captured again after asynchronous preparation. const earlyAccount = sessionDataToSessionAccount(session.session, session.session.service) ?? storedAccount @@ -509,13 +405,7 @@ export async function createSessionBundleAndResume( configureModerationForAccount(bundle, earlyAccount) const aa = prefetchAgeAssuranceServerData({client: bundle.bskyClient}) await Promise.all([gates, aa]) - /* - * Re-snapshot AFTER prep, right before arm(). A 401 during a prep request - * (e.g. the AA prefetch) triggers PasswordSession's internal auto-refresh, - * which rotates both tokens; its onUpdated is dropped by the still-disarmed - * latch. Snapshotting here (not before prep) persists the fresh refreshJwt - * rather than a stale one that is dead on the next cold start. - */ + // Preparation may auto-refresh the session while hooks are still disarmed. const account = sessionDataToSessionAccount(session.session, session.session.service) ?? storedAccount @@ -559,7 +449,7 @@ export async function createSessionBundleAndLogin( bundle = buildBundle(session) registerBundleKillSwitch(bundle, hooks.kill) - // Early snapshot: needed now to seed `accountDid` (the getDid closure). + // Seed the hook's did before it is armed. const earlyAccount = sessionDataToSessionAccountOrThrow(session) accountDid = earlyAccount.did @@ -567,13 +457,7 @@ export async function createSessionBundleAndLogin( configureModerationForAccount(bundle, earlyAccount) const aa = prefetchAgeAssuranceServerData({client: bundle.bskyClient}) await Promise.all([gates, aa]) - /* - * Re-snapshot AFTER prep, right before arm(): a 401 during a prep request - * triggers PasswordSession's internal auto-refresh, which rotates both tokens - * and fires an onUpdated the disarmed latch drops, so this persists the fresh - * refreshJwt. If the session was destroyed mid-prep, OrThrow throws (login - * effectively failed). - */ + // Preparation may auto-refresh the session while hooks are still disarmed. const account = sessionDataToSessionAccountOrThrow(session) hooks.arm() return {account, bundle} @@ -630,10 +514,7 @@ export async function createSessionBundleAndCreateAccount( bundle = buildBundle(session) registerBundleKillSwitch(bundle, hooks.kill) - /* - * Early snapshot: needed now to seed `accountDid` and for the DID/handle used - * across the local and deferred server writes below (all refresh-stable). - */ + // Seed the hook and the deferred writes with refresh-stable account fields. const earlyAccount = sessionDataToSessionAccountOrThrow(session) accountDid = earlyAccount.did @@ -762,21 +643,12 @@ export async function createSessionBundleAndCreateAccount( } await Promise.all([gates, aa]) - /* - * Re-snapshot AFTER prep, right before arm(): a 401 during a prep request - * triggers PasswordSession's internal auto-refresh, which rotates both tokens - * and fires an onUpdated the disarmed latch drops, so this persists the fresh - * refreshJwt. If the session was destroyed mid-prep, OrThrow throws. - */ + // Preparation may auto-refresh the session while hooks are still disarmed. const account = sessionDataToSessionAccountOrThrow(session) hooks.arm() return {account, bundle} } -/** - * Snapshot a live session as a `SessionAccount`, throwing if there is no active - * session. - */ function sessionDataToSessionAccountOrThrow( session: PasswordSession, ): SessionAccount { @@ -791,21 +663,9 @@ function sessionDataToSessionAccountOrThrow( } /** - * Neutralize a bundle's session so it can never refresh again. - * - * Called when switching away from / disposing an account. `PasswordSession` - * exposes no synchronous, hook-free way to mark itself destroyed without a - * network logout (and `logout()`/`delete()` would revoke on the server, which - * we do NOT want for a local switch - revocation is handled separately via the - * push-token unregister temporary sessions). So we trip the kill-switch - * installed in the session's injected `fetch` (see {@link makeSessionHooks} / - * {@link registerBundleKillSwitch}): every subsequent request through this - * session - direct fetch AND the internal auto-refresh, which shares the same - * captured `options.fetch` - throws before touching the network. A tripped - * refresh routes into the `onUpdateFailure` path (session preserved locally, - * refresh token NOT consumed server-side). `kill()` also disarms the hooks so - * the stale bundle can no longer dispatch into the reducer. The guarantee: this - * session's tokens are no longer reachable by any live network path. + * Disable a replaced bundle without revoking its server session. PasswordSession + * has no local destroy operation, so the registered lifecycle closure disables + * its fetch and hooks instead. */ export function disposeBundle(bundle: SessionBundle | PublicSessionBundle) { const session = bundle.session @@ -815,33 +675,10 @@ export function disposeBundle(bundle: SessionBundle | PublicSessionBundle) { bundleKillSwitches.get(bundle)?.() } -/** - * Hard bound on how many distinct refresh-token generations the expiry rescue - * will burn through for a single did before giving up and logging out. Each - * rescue consumes a strictly newer generation (a token that differs from every - * one already recorded as failed), so this set can only grow one entry per - * expiry and this cap guarantees termination even under a pathological storm - * of expiries against ever-newer tokens. - */ +/** Maximum failed token generations considered during one expiry rescue. */ export const MAX_EXPIRY_RESCUE_GENERATIONS = 5 -/** - * Pure decision for the cross-tab expiry rescue (side-effecting rebuild stays - * in the provider). Given the dying session's refreshJwt and the "latest known" - * candidate accounts for that did (in preference order), pick the first - * candidate that carries a usable, strictly-newer generation: - * - * - has a non-empty `refreshJwt`, - * - whose `refreshJwt` DIFFERS from the dying one (a same-token candidate is - * just as dead), and - * - whose `refreshJwt` is NOT already recorded as failed (loop guard). - * - * Returns `undefined` (fall through to logout) when nothing qualifies or the - * failed-generation set has hit {@link MAX_EXPIRY_RESCUE_GENERATIONS}. - * - * `candidates` are tried in order, so the caller passes its most-authoritative - * source first (on web, the fresh persisted re-read before the reducer state). - */ +/** Pick the first unfailed token generation newer than the one that expired. */ export function pickExpiryRescueCandidate({ dyingRefreshJwt, candidates, diff --git a/src/state/session/types.ts b/src/state/session/types.ts index 455f29e381..3f57c5532a 100644 --- a/src/state/session/types.ts +++ b/src/state/session/types.ts @@ -43,22 +43,8 @@ export type SessionApiContext = { isSwitchingAccounts?: boolean, ) => Promise removeAccount: (account: SessionAccount) => void - /** - * Fetches `com.atproto.server.getSession` through the active account's PDS - * client and patches the reducer's stored account entry with the returned - * `emailConfirmed`/`emailAuthFactor` fields. Unlike `refreshSession`, this - * does not rotate tokens, touch the `PasswordSession`, or fire session-change - * hooks - it only refreshes those email-state fields on the current account. - */ + /** Refresh email state without rotating tokens. */ partialRefreshSession: () => Promise - /** - * Force a full session refresh (re-runs `com.atproto.server.refreshSession` - * plus `getSession`) and return the refreshed account snapshot, or `undefined` - * when logged out. - * - * The session's success hook propagates the updated account into state; the - * returned snapshot lets callers read post-refresh fields synchronously - * without waiting on the (async) reducer update. Rejections propagate. - */ + /** Refresh tokens and return the new account snapshot immediately. */ refreshSession: () => Promise } diff --git a/src/state/session/util.ts b/src/state/session/util.ts index 325a33f756..32b51ea7fc 100644 --- a/src/state/session/util.ts +++ b/src/state/session/util.ts @@ -7,11 +7,6 @@ import * as persisted from '#/state/persisted' import {networkAwareFetch, sessionAccountToSessionData} from './session-core' import {type SessionAccount} from './types' -/* - * Canonical implementation lives in session-core.ts so that module stays - * dependency-light (this file transitively pulls in a large chunk of the app). - * Re-exported here for existing consumers. - */ export {isSignupQueued} from './session-core' export function readLastActiveAccount() { @@ -28,14 +23,8 @@ export function isSessionExpired(account: SessionAccount) { } /** - * Creates and resumes a throwaway session for every stored account. Intended to - * send push token revocations just before logout. - * - * Each returned {@link TemporaryPushClient} wraps a temporary `PasswordSession` - * resumed over the network for a valid access token. These sessions are - * deliberately hook-free (no `onUpdated`/`onDeleted`): they must NEVER persist - * or race the active session. They are used once for the unregister call and - * discarded (reclaimed by GC), so we never call `logout()` on them. + * Resume hook-free, single-use sessions for push-token revocation. They must + * never persist or race the active session. */ export async function createTemporaryClientsAndResume( accounts: SessionAccount[],