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 <noreply@anthropic.com>
This commit is contained in:
Samuel Newman
2026-07-31 20:49:12 +03:00
parent 9f381485dc
commit 3e42c659c9
5 changed files with 36 additions and 44 deletions
@@ -32,12 +32,12 @@ jest.mock('#/analytics', () => ({
})) }))
/* /*
* `configureModerationForAccount` is now fully synchronous (the labeler cache * `configureModerationForAccount` is synchronous (the labeler cache is a local
* is a local MMKV read), so it is no longer a prep await - but it still runs * MMKV read), so it is not a prep await - but it still runs inside each factory
* inside each factory with the freshly built bridge agent, before the awaited * with the freshly built bridge agent, before the awaited prep steps. The
* prep steps. The factory tests capture the agent with this mock, then inject a * factory tests capture the agent with this mock, then inject a real refresh
* real refresh into the awaited AA prefetch so a token rotation happens during * into the awaited AA prefetch so a token rotation happens during prep, before
* prep, before arm(). The default is a no-op so other tests are unaffected. * arm(). The default is a no-op so other tests are unaffected.
* (jest requires out-of-scope factory references to be `mock`-prefixed.) * (jest requires out-of-scope factory references to be `mock`-prefixed.)
*/ */
const mockConfigureModerationForAccount = const mockConfigureModerationForAccount =
@@ -553,10 +553,8 @@ function deriveRefreshedAccount(
} }
/* /*
* `PasswordSession` fires onUpdated with * `PasswordSession` fires onUpdated with the fresh session BEFORE committing it
* the fresh session BEFORE committing it internally, so the live getter is * internally, so the live getter is still stale at hook time.
* still stale at hook time. Driven through the real library (not a hand-rolled
* fixture) so the ordering is authentic.
*/ */
describe('session-hook payload threading (pre-commit ordering)', () => { 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 () => { 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. * which is dead on the next cold start.
* *
* We simulate the mid-prep rotation by capturing the bundle from the mocked * We simulate the mid-prep rotation by capturing the bundle from the mocked
* (now synchronous) `configureModerationForAccount` and making the mocked * `configureModerationForAccount` and making the mocked
* `prefetchAgeAssuranceServerData` (a genuine prep await in each factory) run * `prefetchAgeAssuranceServerData` (a genuine prep await in each factory) run a
* a real `session.refresh()`. The factory itself is re-required inside * real `session.refresh()`. The factory itself is re-required inside
* `jest.isolateModulesAsync` AFTER overriding `globalThis.fetch`, because * `jest.isolateModulesAsync` AFTER overriding `globalThis.fetch`, because
* the network leaf captures `globalThis.fetch` at module load - and that * the network leaf captures `globalThis.fetch` at module load - and that
* captured fetch is what PasswordSession's auto-refresh routes through. * captured fetch is what PasswordSession's auto-refresh routes through.
+1 -1
View File
@@ -1121,7 +1121,7 @@ describe('session', () => {
expect(state.currentBundleState.did).toBe('bob-did') 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 * ENTIRELY - identical state object, no token write. A refresh completing
* after switching away must not resurrect fresh tokens into the * after switching away must not resurrect fresh tokens into the
* switched-away account entry. * switched-away account entry.
+19 -20
View File
@@ -21,8 +21,8 @@ const UNSUPPORTED =
'Not supported on PasswordSessionManager; use the session factories in session-core' 'Not supported on PasswordSessionManager; use the session factories in session-core'
/** /**
* Convert live `PasswordSession` session data into the old `AtpSessionData` * Convert live `PasswordSession` session data into the `AtpSessionData` shape
* shape that `CredentialSession.session` consumers expect. * that `CredentialSession.session` consumers expect.
* *
* The only real adaptation is `active`: `AtpSessionData` requires it, while the * The only real adaptation is `active`: `AtpSessionData` requires it, while the
* lexicon payload leaves it optional (absent means active, per the lexicon * 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`. * A `CredentialSession` whose auth core is a `PasswordSession`.
* *
* This is the compat shim that lets the new session layer sit under the old * This is the compat shim that lets a `PasswordSession` sit under `AtpAgent`:
* `AtpAgent`: every call site that reads `agent.session`, `agent.pdsUrl`, * every call site that reads `agent.session`, `agent.pdsUrl`,
* `agent.dispatchUrl`, `agent.did` or calls `agent.resumeSession()` keeps * `agent.dispatchUrl`, `agent.did` or calls `agent.resumeSession()` keeps
* working, while the actual tokens, refresh serialization and PDS routing live * working, while the actual tokens, refresh serialization and PDS routing live
* in the `PasswordSession` underneath. * in the `PasswordSession` underneath.
@@ -111,9 +111,10 @@ export class PasswordSessionManager extends CredentialSession {
* Why `defineProperty` and not `get session()` in the class body: the * Why `defineProperty` and not `get session()` in the class body: the
* parent declares `session` and `pdsUrl` as *properties*, and TypeScript * parent declares `session` and `pdsUrl` as *properties*, and TypeScript
* rejects overriding a property with an accessor (TS2611). Installing them * rejects overriding a property with an accessor (TS2611). Installing them
* at runtime sidesteps that, and it is safe because the parent's emitted * at runtime sidesteps that, and it is safe as long as
* constructor never assigns either one - both are declaration-only in the * `CredentialSession`'s emitted constructor does not assign either one
* 0.20.34 dist, so there is nothing to clobber and no ordering hazard. * (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 * For the same reason this class must NOT redeclare `session`/`pdsUrl` as
* fields: under `useDefineForClassFields` semantics (target esnext) a field * 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 * `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. * through the accessors above, so they are already live.
*/ */
@@ -214,21 +215,19 @@ export class PasswordSessionManager extends CredentialSession {
init?: RequestInit, init?: RequestInit,
): Promise<Response> { ): Promise<Response> {
/* /*
* Absolutizing against `dispatchUrl` is what replaces the old * Absolutizing against `dispatchUrl` routes to the stored PDS on the resume
* `sessionManager.pdsUrl = ...` writes and `_updateApiEndpoint`: it routes * fast path and to the didDoc PDS from the first refresh onwards, since
* to the stored PDS on the resume fast path and to the didDoc PDS from the * `PasswordSession` resolves an already absolute URL against its own base
* first refresh onwards, since `PasswordSession` resolves an already * as a no-op.
* absolute URL against its own base as a no-op.
*/ */
const target = new URL(url, this.dispatchUrl) const target = new URL(url, this.dispatchUrl)
const inner = this.#disposed ? null : this.#inner const inner = this.#disposed ? null : this.#inner
/* /*
* A caller that set its own `authorization` header bypasses the inner * A caller that set its own `authorization` header bypasses the inner
* session entirely. This preserves old `CredentialSession` semantics (it * session entirely. This is mandatory: `PasswordSession.fetchHandler`
* skipped its own bearer in that case) and is mandatory here: * throws `TypeError` on a pre-set authorization header rather than
* `PasswordSession.fetchHandler` throws `TypeError` on a pre-set * deferring to it.
* authorization header rather than deferring to it.
*/ */
if ( if (
!inner || !inner ||
@@ -252,9 +251,9 @@ export class PasswordSessionManager extends CredentialSession {
} }
const data = await inner.refresh() const data = await inner.refresh()
/* /*
* Re-shape the lex payload into the old XRPC envelope. `headers` is empty * Re-shape the lex payload into the `@atproto/api` XRPC response envelope.
* because the inner session does not surface response headers, and no * `headers` is empty because the inner session does not surface response
* caller in this app reads them off a refresh. * headers, and no caller in this app reads them off a refresh.
*/ */
return { return {
success: true, success: true,
+3 -6
View File
@@ -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()) bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get())
await Promise.all([gates, aa]) await Promise.all([gates, aa])
@@ -138,10 +136,9 @@ export async function createSessionBundleAndCreateAccount(
* Snapshot a just-created session as a `SessionAccount`. * Snapshot a just-created session as a `SessionAccount`.
* *
* `com.atproto.server.createAccount` returns only tokens, handle, did and * `com.atproto.server.createAccount` returns only tokens, handle, did and
* didDoc, so the lex session carries no email state or `active` flag until the * didDoc, so the session carries no email state or `active` flag until the
* first refresh. The old `CredentialSession.createAccount` synthesized those * first refresh. Synthesize those fields from the creation input so the
* fields from the creation input; do the same here so the persisted account * persisted account does not briefly claim the email is unknown.
* does not briefly claim the email is unknown.
*/ */
function snapshotNewAccount( function snapshotNewAccount(
session: PasswordSession, session: PasswordSession,
+2 -4
View File
@@ -223,9 +223,8 @@ export async function createSessionBundleAndResume(
const aa = prefetchAgeAssuranceServerData({agent: bundle.agent}) const aa = prefetchAgeAssuranceServerData({agent: bundle.agent})
/* /*
* Proxy-header ordering matches the old agent factories: the header is * The proxy header is applied after the PDS-targeting setup above, so those
* applied after the age-assurance prefetch starts and before the prep await, * calls run without it.
* so the PDS-targeting setup calls above run without it.
*/ */
bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get()) bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get())
@@ -285,7 +284,6 @@ export async function createSessionBundleAndLogin(
configureModerationForAccount(bundle.agent, earlyAccount) configureModerationForAccount(bundle.agent, earlyAccount)
const aa = prefetchAgeAssuranceServerData({agent: bundle.agent}) const aa = prefetchAgeAssuranceServerData({agent: bundle.agent})
// Proxy-header ordering matches the old agent factories.
bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get()) bundle.agent.configureProxy(BLUESKY_PROXY_HEADER.get())
await Promise.all([gates, aa]) await Promise.all([gates, aa])