Skip to content

docs: write a single source of truth for the SQLite storage stack #142

@aspiers

Description

@aspiers

Background

ePDS has two SQLite databases owned by two different packages, accessed through two different libraries, with at least one important "don't open a second handle" rule that exists only in a commit message. A future contributor (human or AI) effectively has to reverse-engineer the storage architecture from grep + git log.

The trigger for this issue: in PR #141 a route-level test introduced a second better-sqlite3 connection to a SQLite file that already had one open via AuthServiceContext's constructor — exactly the dual-writer-on-WAL antipattern that commit 0f2b19d had previously identified and removed from pds-core. Reviewer-bot caught it. A short docs page would have prevented it.

What's currently undocumented

Topic Where it lives today Gap
Two SQLite files: account.sqlite (pds-core, owned by upstream @atproto/pds accountManager) and epds.sqlite (auth-service, owned by EpdsDb) Inferable from DB_LOCATION + PDS_DATA_DIRECTORY env vars in docs/configuration.md and docs/deployment.md Never spelled out as a contract: which file each package owns, what schemas live in each
Library per file: pds-core uses Kysely (via upstream's accountManager); auth-service uses raw better-sqlite3 via EpdsDb; better-auth has its own SQLite tables in the same file Code only; one passing mention in docs/design/testing-gaps.md Nothing in docs/architecture.md or docs/design/
Rule: do not open a second better-sqlite3 connection on account.sqlite — reuse the running PDS's Kysely handle (pds.ctx.accountManager.db.db) for cross-cutting access (e.g. test hooks) Commit 0f2b19d body + inline comment in packages/pds-core/src/lib/test-hooks.ts Decision is invisible to anyone who doesn't already know it exists
Corollary for tests: route-level tests that spin up an AuthServiceContext get one EpdsDb handle from the constructor — don't construct a second one against the same path Caught by reviewer-bot in PR #141, fixed in 47a62e9; not documented First-time contributors will keep tripping on this

Proposed scope

Add a new short page (suggested path docs/design/storage.md) covering:

  1. Inventory: the two SQLite files, who owns each, what tables live where (auth_flow, better-auth tables, OTP rate-limit + failure rows on the auth-service side; upstream PDS schema on the pds-core side).
  2. Library choice per file and why: pds-core inherits Kysely from upstream; auth-service uses raw better-sqlite3 via EpdsDb because it owns its own schema and migrations.
  3. The single-handle rule: WAL-mode SQLite gives opaque visibility issues across multiple writer connections to the same file. The rule is "one open handle per file per process". Cross-cutting code that needs to write to PDS's account.sqlite reuses upstream's Kysely instance; it does not open a second better-sqlite3 connection. Reference 0f2b19d as the worked example.
  4. Test patterns: route-level tests instantiate the production context (AuthServiceContext) against a tmp file and use ctx.db exclusively; pure unit tests can construct an EpdsDb directly (and own its lifecycle). Reference the leak-fix commit (47a62e9) as a worked example of getting it wrong.
  5. Cross-link from docs/architecture.md's package table (where shared is currently described as just "Database (SQLite)") and from docs/design/testing-gaps.md's discussion of "the full better-auth + better-sqlite3 stack".

Out of scope for this issue

  • Refactoring the storage layer itself.
  • Changing library choices (e.g. moving auth-service to Kysely too). This issue is purely documenting what's already true.

Acceptance

  • docs/design/storage.md exists and covers the five points above.
  • docs/architecture.md and docs/design/testing-gaps.md link to it where the topics already come up.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions