From 3e42c659c91e0d73d8080c2d23e83d63a4ba2b20 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Fri, 31 Jul 2026 20:49:12 +0300 Subject: [PATCH] clean up migration-context comments Comments should explain the code as it is, not narrate the change that produced it. Drops "the old X" / "now" / "no longer" framing and a stale version pin in favour of concrete names and the live constraints. Co-Authored-By: Claude Fable 5 --- .../session/__tests__/session-core-test.ts | 24 ++++++------ src/state/session/__tests__/session-test.ts | 2 +- src/state/session/bridge-agent.ts | 39 +++++++++---------- src/state/session/create-account.ts | 9 ++--- src/state/session/session-core.ts | 6 +-- 5 files changed, 36 insertions(+), 44 deletions(-) diff --git a/src/state/session/__tests__/session-core-test.ts b/src/state/session/__tests__/session-core-test.ts index 278847c03c..7551f95d2a 100644 --- a/src/state/session/__tests__/session-core-test.ts +++ b/src/state/session/__tests__/session-core-test.ts @@ -32,12 +32,12 @@ jest.mock('#/analytics', () => ({ })) /* - * `configureModerationForAccount` is now fully synchronous (the labeler cache - * is a local MMKV read), so it is no longer a prep await - but it still runs - * inside each factory with the freshly built bridge agent, before the awaited - * prep steps. The factory tests capture the agent with this mock, then inject a - * real refresh into the awaited AA prefetch so a token rotation happens during - * prep, before arm(). The default is a no-op so other tests are unaffected. + * `configureModerationForAccount` is synchronous (the labeler cache is a local + * MMKV read), so it is not a prep await - but it still runs inside each factory + * with the freshly built bridge agent, before the awaited prep steps. The + * factory tests capture the agent with this mock, then inject a real refresh + * into the awaited AA prefetch so a token rotation happens during prep, before + * arm(). The default is a no-op so other tests are unaffected. * (jest requires out-of-scope factory references to be `mock`-prefixed.) */ const mockConfigureModerationForAccount = @@ -553,10 +553,8 @@ function deriveRefreshedAccount( } /* - * `PasswordSession` fires onUpdated with - * the fresh session BEFORE committing it internally, so the live getter is - * still stale at hook time. Driven through the real library (not a hand-rolled - * fixture) so the ordering is authentic. + * `PasswordSession` fires onUpdated with the fresh session BEFORE committing it + * internally, so the live getter is still stale at hook time. */ describe('session-hook payload threading (pre-commit ordering)', () => { it('delivers the NEW tokens via the payload even though the live getter is still pre-commit stale', async () => { @@ -843,9 +841,9 @@ describe('refreshSession semantics', () => { * which is dead on the next cold start. * * We simulate the mid-prep rotation by capturing the bundle from the mocked - * (now synchronous) `configureModerationForAccount` and making the mocked - * `prefetchAgeAssuranceServerData` (a genuine prep await in each factory) run - * a real `session.refresh()`. The factory itself is re-required inside + * `configureModerationForAccount` and making the mocked + * `prefetchAgeAssuranceServerData` (a genuine prep await in each factory) run a + * real `session.refresh()`. The factory itself is re-required inside * `jest.isolateModulesAsync` AFTER overriding `globalThis.fetch`, because * the network leaf captures `globalThis.fetch` at module load - and that * captured fetch is what PasswordSession's auto-refresh routes through. diff --git a/src/state/session/__tests__/session-test.ts b/src/state/session/__tests__/session-test.ts index 5e067fdb0b..c084e3dc08 100644 --- a/src/state/session/__tests__/session-test.ts +++ b/src/state/session/__tests__/session-test.ts @@ -1121,7 +1121,7 @@ describe('session', () => { expect(state.currentBundleState.did).toBe('bob-did') /* - * An 'update' from the stale (background) Alice bundle is now dropped + * An 'update' from the stale (background) Alice bundle is dropped * ENTIRELY - identical state object, no token write. A refresh completing * after switching away must not resurrect fresh tokens into the * switched-away account entry. diff --git a/src/state/session/bridge-agent.ts b/src/state/session/bridge-agent.ts index a8bd24fb7c..5fd9117f8b 100644 --- a/src/state/session/bridge-agent.ts +++ b/src/state/session/bridge-agent.ts @@ -21,8 +21,8 @@ const UNSUPPORTED = 'Not supported on PasswordSessionManager; use the session factories in session-core' /** - * Convert live `PasswordSession` session data into the old `AtpSessionData` - * shape that `CredentialSession.session` consumers expect. + * Convert live `PasswordSession` session data into the `AtpSessionData` shape + * that `CredentialSession.session` consumers expect. * * The only real adaptation is `active`: `AtpSessionData` requires it, while the * lexicon payload leaves it optional (absent means active, per the lexicon @@ -59,8 +59,8 @@ function parseUrl(input: string): URL | undefined { /** * A `CredentialSession` whose auth core is a `PasswordSession`. * - * This is the compat shim that lets the new session layer sit under the old - * `AtpAgent`: every call site that reads `agent.session`, `agent.pdsUrl`, + * This is the compat shim that lets a `PasswordSession` sit under `AtpAgent`: + * every call site that reads `agent.session`, `agent.pdsUrl`, * `agent.dispatchUrl`, `agent.did` or calls `agent.resumeSession()` keeps * working, while the actual tokens, refresh serialization and PDS routing live * in the `PasswordSession` underneath. @@ -111,9 +111,10 @@ export class PasswordSessionManager extends CredentialSession { * Why `defineProperty` and not `get session()` in the class body: the * parent declares `session` and `pdsUrl` as *properties*, and TypeScript * rejects overriding a property with an accessor (TS2611). Installing them - * at runtime sidesteps that, and it is safe because the parent's emitted - * constructor never assigns either one - both are declaration-only in the - * 0.20.34 dist, so there is nothing to clobber and no ordering hazard. + * at runtime sidesteps that, and it is safe as long as + * `CredentialSession`'s emitted constructor does not assign either one + * (both are declaration-only), so there is nothing to clobber and no + * ordering hazard. * * For the same reason this class must NOT redeclare `session`/`pdsUrl` as * fields: under `useDefineForClassFields` semantics (target esnext) a field @@ -205,7 +206,7 @@ export class PasswordSessionManager extends CredentialSession { /* * `did`, `hasSession` and `dispatchUrl` are deliberately NOT overridden: the - * inherited getters read `this.session` / `this.pdsUrl`, which now resolve + * inherited getters read `this.session` / `this.pdsUrl`, which resolve * through the accessors above, so they are already live. */ @@ -214,21 +215,19 @@ export class PasswordSessionManager extends CredentialSession { init?: RequestInit, ): Promise { /* - * Absolutizing against `dispatchUrl` is what replaces the old - * `sessionManager.pdsUrl = ...` writes and `_updateApiEndpoint`: it routes - * to the stored PDS on the resume fast path and to the didDoc PDS from the - * first refresh onwards, since `PasswordSession` resolves an already - * absolute URL against its own base as a no-op. + * Absolutizing against `dispatchUrl` routes to the stored PDS on the resume + * fast path and to the didDoc PDS from the first refresh onwards, since + * `PasswordSession` resolves an already absolute URL against its own base + * as a no-op. */ const target = new URL(url, this.dispatchUrl) const inner = this.#disposed ? null : this.#inner /* * A caller that set its own `authorization` header bypasses the inner - * session entirely. This preserves old `CredentialSession` semantics (it - * skipped its own bearer in that case) and is mandatory here: - * `PasswordSession.fetchHandler` throws `TypeError` on a pre-set - * authorization header rather than deferring to it. + * session entirely. This is mandatory: `PasswordSession.fetchHandler` + * throws `TypeError` on a pre-set authorization header rather than + * deferring to it. */ if ( !inner || @@ -252,9 +251,9 @@ export class PasswordSessionManager extends CredentialSession { } const data = await inner.refresh() /* - * Re-shape the lex payload into the old XRPC envelope. `headers` is empty - * because the inner session does not surface response headers, and no - * caller in this app reads them off a refresh. + * Re-shape the lex payload into the `@atproto/api` XRPC response envelope. + * `headers` is empty because the inner session does not surface response + * headers, and no caller in this app reads them off a refresh. */ return { success: true, diff --git a/src/state/session/create-account.ts b/src/state/session/create-account.ts index 18ccbeef5e..985b2c40fd 100644 --- a/src/state/session/create-account.ts +++ b/src/state/session/create-account.ts @@ -123,8 +123,6 @@ export async function createSessionBundleAndCreateAccount( }) } - // Proxy-header ordering matches the old agent factories: after the void-fired - // post-signup writes are started, before the prep await. bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get()) await Promise.all([gates, aa]) @@ -138,10 +136,9 @@ export async function createSessionBundleAndCreateAccount( * Snapshot a just-created session as a `SessionAccount`. * * `com.atproto.server.createAccount` returns only tokens, handle, did and - * didDoc, so the lex session carries no email state or `active` flag until the - * first refresh. The old `CredentialSession.createAccount` synthesized those - * fields from the creation input; do the same here so the persisted account - * does not briefly claim the email is unknown. + * didDoc, so the session carries no email state or `active` flag until the + * first refresh. Synthesize those fields from the creation input so the + * persisted account does not briefly claim the email is unknown. */ function snapshotNewAccount( session: PasswordSession, diff --git a/src/state/session/session-core.ts b/src/state/session/session-core.ts index b77a3e08bc..4a6e5bfbfc 100644 --- a/src/state/session/session-core.ts +++ b/src/state/session/session-core.ts @@ -223,9 +223,8 @@ export async function createSessionBundleAndResume( const aa = prefetchAgeAssuranceServerData({agent: bundle.agent}) /* - * Proxy-header ordering matches the old agent factories: the header is - * applied after the age-assurance prefetch starts and before the prep await, - * so the PDS-targeting setup calls above run without it. + * The proxy header is applied after the PDS-targeting setup above, so those + * calls run without it. */ bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get()) @@ -285,7 +284,6 @@ export async function createSessionBundleAndLogin( configureModerationForAccount(bundle.agent, earlyAccount) const aa = prefetchAgeAssuranceServerData({agent: bundle.agent}) - // Proxy-header ordering matches the old agent factories. bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get()) await Promise.all([gates, aa])