Decomposing core/src/pds.js

Status: complete. Fourteen clusters moved and three helper modules extracted, 1,337 lines left in pds.js from 10,205. What stays in the class is the dispatch pipeline, authentication, and the commit and blob machinery. The static route table is empty; every route comes from a handler module.

Read CLAUDE.md first for the conventions this follows. This file is the record: what moved, what each move needed, and the mistakes that cost time.

A second phase followed. The feature routes that handlers/account.js held for git, drive, sites, spaces, OCI and npm moved into those packages, each mounted through the extension seam described in architecture.md, along with the per-feature constructor options that wired them. The table below is the snapshot at the end of the first phase; account.js has since lost those routes and the adapters their feature construction.

Where it stands

module	lines	statements	branches	functions
handlers/account.js	2,998	88.4	74.6	96.5
handlers/oauth.js	1,682	83.7	73.8	100
handlers/xrpc-server.js	1,036	98.0	94.3	95.0
handlers/xrpc-repo.js	810	89.2	80.0	100
handlers/xrpc-identity.js	679	95.3	82.9	100
handlers/account-backup.js	535	97.5	90.3	100
handlers/migration.js	356	96.6	83.9	84.6
handlers/xrpc-sync.js	312	95.7	79.4	100
handlers/server-meta.js	252	100	100	100
handlers/xrpc-blob.js	189	100	100	100
handlers/xrpc-firehose.js	155	100	86.1	100
handlers/xrpc-preferences.js	81	100	100	100
session.js	331	96.4	84.2	100
proxy.js	282	98.8	96.9	100
http.js	103	92.6	88.9	100
passkey.js	85	100	80.0	100

Repo-wide statements are 78.1%, from 59.87% at the start. The unit suite is 1,318 tests, up from 941.

proxy.js, session.js, http.js and passkey.js are not under handlers/ because they contribute no routes. Name a module for what it knows when that is true, and put it in core/src.

What each cluster needs before it moves

Two numbers decide the work, and they are not correlated. Coverage under the unit suite against coverage with a PLATFORM=node e2e run says where the untested code is. The count of instance members a cluster reaches says how much of a seam it has. xrpc-blob was 0% covered and reached six members. The account surface reaches forty-seven.

Measure both before starting. The commands are in the commits; the shape is:

awk 'NR>=LO && NR<HI' packages/core/src/pds.js \
  | grep -oE "this\.[a-zA-Z_][a-zA-Z0-9_]*" | sort -u

A member count that is too high is a signal to extract a helper module first, not to widen the context. xrpc-server reached 24 members and six module-level names shared with the rest of the file. Three extractions — session.js for the lifetimes and the registry, http.js for CORS, cookies and isSameOriginPost, decodeJwtClaims into auth.js — brought it to a 17-member context with no edge in either direction, and the move itself was then mechanical.

The last rounds

The account surface, 2,696 lines, 47 members

Account Pages and its JSON API sub-section moved as one project into handlers/account.js. Most of the methods in the surface are shared machinery rather than route handlers — requireAccountSession alone has fifteen callers — so splitting the sub-sections would have traded a few hundred lines for fifty lines of indirection. accountApi, the spine of 29 routes, moved once.

Of the 47 members, 27 were the surface's own methods. The email-state helpers (sessionSeesEmail, readEmailState, emailSessionFields) went into email.js first: the PLC endpoints and the xrpc-server session context need them on the other side of the seam, and as module functions taking actorStorage neither side carries them in a context. That left a 20-member context, all ports, config values, and closures over the class.

The factory exposes six functions inward: readAccountSession, renewAccountSession, readAccountUiState and accountAppResponse for dispatch and the OAuth consent flow, accountApi for the backup routes, and listConnectedApps for its unit test, which builds the factory directly.

The identity and migration residue, 580 lines, 17 members

The ten route handlers left after the account surface had the worst payoff-per-line in the file: a fifth of that surface's lines reaching nearly the same seam width, and sitting directly on the signing-key cache and the commit and blob traversals. They also span three route families, so they went three ways. The PLC operation flow and getRecommendedDidCredentials joined xrpc-identity.js, whose renameHandle was their only sibling, with derivePublicKey going to crypto.js. getServiceAuth joined xrpc-server.js. The migration sequence — checkAccountStatus, importRepo, listMissingBlobs, activateAccount, deactivateAccount — became handlers/migration.js, taking the account-active helpers with it; the sync, repo and account contexts read isDeactivated, checkAccountActive and setAccountActive from its return. The traversals themselves (collectRepoBlocks, collectReferencedBlobs, relinkBlobRecords) stay on the class, which uses them for orphan cleanup, and reach the module as closures. With the residue gone the static route table held nothing and is deleted; this.routes is built from the module fragments alone.

Things that cost time here

• Check inbound references from elsewhere in the same file. Not just other packages. handleApiOverview called this.backupsAvailable() from 1,500 lines away and broke three tests.
• A test DID must satisfy DID_REGEX. did:plc: takes exactly 24 characters from [a-z2-7]. A short one is not rejected as a bad DID; isValidDid returns false and the failure surfaces as an unrelated assertion.
• A fake has to match arity. listRecords(collection, cursor, limit) given a two-parameter fake receives the cursor as its limit, which reads as an empty collection rather than a wrong call.
• A default parameter fires on undefined. A test helper that models "no password configured" by passing password: undefined gets the default back and tests the opposite branch. Give the helper a named flag instead.
• A blanket this.member → member rename collides with locals. The sign-in handler holds the submitted form value in a local named password; renaming this.password there produced password !== password, a check that passes every value. Biome's noSelfCompare and tsc's use-before-declaration errors found all four collisions; run both before trusting the transform.
• A file missing from the coverage table is not necessarily complete. Read coverage/coverage-final.json when it matters. xrpc-preferences.js was absent and was at 13.3%.
• Glob threshold keys aggregate. A well-covered file in the same key hides a bare one. One entry per module.
• Do not type a context member off the class. PersonalDataServer['buildFullRepoCar'] broke the moment that method moved. Structural types instead.
• A test that calls a handler as a method needs a new entry point. pds.handleRefreshSession(request) stops resolving once the handler is a closure in a module. pds.fetch(request) drives the same route through the table, which is what test/onboarding.test.js already does.
• The e2e suite binds port 2471. A just dev stack in any checkout blocks it.

Two fixes came out of this, and how

Neither was found by reading the code. Both came from writing a test for a specific property of a handler that had just moved.

buildFullRepoCar threw Missing block from inside a try whose catch discarded it, so only a missing root reached the caller and com.atproto.sync.getRepo answered 200 with part of a repository. Fixed by walking the structure it knows rather than probing field names on every block.

putRecord and applyWrites did not check the repo scope that createRecord and deleteRecord did, so a narrowed OAuth grant reached any collection through a different endpoint. Fixed to match packages/pds in bluesky-social/atproto.

That second fix then exposed a third thing: app-password sessions were being refused every write, because com.atproto.appPass names no granular permission and allowsRepo found nothing to allow. The reference gates its assertions on auth.credentials.type === 'oauth'; skipsGranularChecks() is the equivalent here.

Open questions

• allowsSpace and allowsSpaceManage still refuse app-password sessions. Permissioned data has no counterpart in the reference, so there was nothing to match and the change left it strict. If an app-password client should reach spaces, that is one line.
• skipsGranularChecks infers the credential type from the scope string. The reference carries credentials.type explicitly. Ours holds only because com.atproto.appPass is set at session creation and never appears in an OAuth grant. Carrying the type on auth would make it structural instead of implied.
• SCOPE_APP_PASS_PRIVILEGED is issued and never read. Nothing distinguishes a privileged app password from an ordinary one. The reference uses that distinction to gate chat access.
• No e2e test drives an app-password client through a write. The fix for it is covered by unit tests against a fake context, which is a weaker guarantee than the rest of this work. This is the first thing to verify.