Claude: changes for issue #38 #62

claude-bot · 2026-07-25T14:51:54Z

claude-bot commented

2026-07-25 14:51:54 +00:00

Closes #38.

claude-bot added 1 commit

2026-07-25 14:51:54 +00:00

feat: EF Core-backed IAccountStore/IRepoStore for the PDS (closes #38 )

CI / build-and-test (pull_request) Successful in 39s

Details

dfa1a7b553

Persistent stores so hosting a PDS no longer means hand-rolling them. They
ship inside ATProtoNet.Pds under the ATProtoNet.Pds.EntityFrameworkCore
namespace rather than as a separate package, matching where the EF Core token
store landed in ATProtoNet.Server after the consolidation in #33.

- AddAtProtoPdsEfCoreStores<TContext>() registers both stores over an
  IDbContextFactory and replaces the in-memory defaults in either call order
- PdsDbContext, plus ConfigurePdsModel() for adding the tables to an existing
  application DbContext
- EfCoreAccountStore: case-insensitive handle/email lookups, matching
  InMemoryAccountStore's semantics
- EfCoreRepoStore: keyset pagination on rkey with the same exclusive-cursor
  contract as InMemoryRepoStore; content-addressed blobs stored once per CID
  with per-account reference rows, so identical uploads dedupe and one account
  deleting a blob cannot destroy another's copy
- PdsEfCoreStoreOptions.ClientSideAccountLookup for deployments whose handle /
  email columns are encrypted non-deterministically, where no SQL predicate can
  match; MaxClientSideLookupRows bounds the scan
- IAccountStore documents that lookups other than GetByDidAsync may load and
  filter in memory
- AddAtProtoPds() now uses TryAddSingleton for its in-memory defaults so an
  earlier store registration is respected
- Store suite runs against both the EF in-memory provider and SQLite, so the
  queries are proven to translate to real SQL

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude-bot commented

2026-07-25 14:53:51 +00:00

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/57); the reply will link the full session transcript. 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-62/run-57.md) — updating every ~5 minutes while the task runs.

Grandiras added 1 commit

2026-07-25 14:54:11 +00:00

Merge branch 'main' into claude/issue-38

CI / build-and-test (pull_request) Successful in 48s

Details

ce9ae36c98

claude-bot referenced this pull request

2026-07-25 14:55:20 +00:00

Pds: provide a persistent store package (ATProtoNet.Pds.EntityFrameworkCore) for IAccountStore/IRepoStore #38

claude-bot reviewed

2026-07-25 14:55:40 +00:00

claude-bot left a comment

Solid, well-documented addition of EF Core-backed PDS stores with good test coverage (including a real-SQL SQLite run, not just the EF in-memory provider). The keyset pagination, case-insensitive lookup handling, content-addressed blob dedup, and TryAddSingleton order-independence fix are all reasonable and match the documented contracts. Two non-blocking correctness concerns worth a look before/after merge: a check-then-insert race in blob content dedup, and a silently-ignored options update on repeated AddAtProtoPdsEfCoreStores registration. Neither is a merge-blocker for a first cut of this feature, but the blob race is worth tracking since concurrent uploads of identical content are exactly the case dedup is meant to handle well.

⏱ 1m 31s · 12 turns · tokens: 56.4k in / 7.2k out (+784.9k cache-read)
📜 Full session transcript

Solid, well-documented addition of EF Core-backed PDS stores with good test coverage (including a real-SQL SQLite run, not just the EF in-memory provider). The keyset pagination, case-insensitive lookup handling, content-addressed blob dedup, and `TryAddSingleton` order-independence fix are all reasonable and match the documented contracts. Two non-blocking correctness concerns worth a look before/after merge: a check-then-insert race in blob content dedup, and a silently-ignored options update on repeated `AddAtProtoPdsEfCoreStores` registration. Neither is a merge-blocker for a first cut of this feature, but the blob race is worth tracking since concurrent uploads of identical content are exactly the case dedup is meant to handle well. --- ⏱ 1m 31s · 12 turns · tokens: 56.4k in / 7.2k out (+784.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-62/run-57.md)

src/ATProtoNet.Pds/Stores/EntityFrameworkCore/EfCoreRepoStore.cs

					
				@ -0,0 +241,4 @@

				            context.Set<PdsBlobRefEntity>().Add(new PdsBlobRefEntity

				            {

				                Did = blob.Did,

				                Cid = blob.Cid,

PutBlobAsync checks contentExists and only inserts a PdsBlobEntity row if absent, but the check-then-insert isn't atomic. Two accounts uploading identical content concurrently (exactly the scenario blob dedup targets) can both observe contentExists == false and both Add the same CID; the second SaveChangesAsync then throws a PK-violation DbUpdateException instead of succeeding. Consider catching the unique-constraint violation and treating it as "already exists", or using a provider-agnostic upsert.

`PutBlobAsync` checks `contentExists` and only inserts a `PdsBlobEntity` row if absent, but the check-then-insert isn't atomic. Two accounts uploading identical content concurrently (exactly the scenario blob dedup targets) can both observe `contentExists == false` and both `Add` the same CID; the second `SaveChangesAsync` then throws a PK-violation `DbUpdateException` instead of succeeding. Consider catching the unique-constraint violation and treating it as "already exists", or using a provider-agnostic upsert.

src/ATProtoNet.Pds/Stores/EntityFrameworkCore/PdsEfCoreStoreExtensions.cs

					
				@ -0,0 +55,4 @@

				        services.RemoveAll<IAccountStore>();

				        services.RemoveAll<IRepoStore>();

options is registered with TryAddSingleton, but the store registrations (IAccountStore/IRepoStore) are re-added via AddSingleton after RemoveAll on every call. If AddAtProtoPdsEfCoreStores<TContext>() is invoked a second time with a different configure delegate, the new stores end up wired to the first call's PdsEfCoreStoreOptions instance — the second call's configuration is silently dropped rather than applied or erroring.

`options` is registered with `TryAddSingleton`, but the store registrations (`IAccountStore`/`IRepoStore`) are re-added via `AddSingleton` after `RemoveAll` on every call. If `AddAtProtoPdsEfCoreStores<TContext>()` is invoked a second time with a different `configure` delegate, the new stores end up wired to the *first* call's `PdsEfCoreStoreOptions` instance — the second call's configuration is silently dropped rather than applied or erroring.

Grandiras commented

2026-07-25 15:41:08 +00:00

@claude-bot Please fix the conflicts and work on the feedback.

claude-bot commented

2026-07-25 15:41:10 +00:00

🤖 On it — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/65); the reply will link the full session transcript. 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-62/run-65.md) — updating every ~5 minutes while the task runs.

claude-bot added 1 commit

2026-07-25 15:52:14 +00:00

Merge main into claude/issue-38; address review feedback

CI / build-and-test (pull_request) Successful in 39s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s

Details

dbaf40c3f2

Resolves the conflicts with the federation work from issue #40 (CHANGELOG,
docs/pds.md, PdsHostingExtensions usings) and carries the EF Core stores
forward onto the interfaces it introduced.

Review feedback:

- PutBlobAsync no longer fails when two accounts upload identical content
  concurrently. The check-then-insert is not atomic, so the loser used to get
  a primary-key DbUpdateException; it now retries once against the row the
  winner wrote. A PdsBlobRefs -> PdsBlobs foreign key (Restrict) closes the
  mirror-image window where orphan collection deletes content a reference has
  just appeared for, and collection tolerates losing that race.
- AddAtProtoPdsEfCoreStores no longer silently drops a second call's options.
  It replaces the options registration alongside the stores, and the stores
  close over the instance from the call that registered them.

Merge follow-through:

- EfCoreRepoStore implements ListAllRecordsAsync and ListBlobCidsAsync, so an
  EF-backed PDS can federate. Records are ordered by the ordinal
  collection/rkey MST key rather than the database collation.
- EfCoreRepoCommitStore persists the signed repository head; durable records
  with an in-memory head store would restart the revision sequence on every
  process start.
- PdsAccountEntity carries RotationKey.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude-bot commented

2026-07-25 15:52:38 +00:00

Merged main into claude/issue-38 and worked through both review comments — pushed as dbaf40c, so the PR is up to date and conflict-free.

Conflicts

Three, all from the federation work in #40 landing on main: CHANGELOG.md (both added entries at the same spot — kept both), docs/pds.md (the new Session signing key section and my Persistent Storage (EF Core) section collided — kept both, session key under Configuration where it belongs), and a using block in PdsHostingExtensions.cs.

Review feedback

Blob dedup race (EfCoreRepoStore.cs:244) — real, and fixed. PutBlobAsync now retries once against a fresh context when SaveChangesAsync throws DbUpdateException: the winner stored the same content-addressed bytes, so the retry finds the row and writes only its own reference. One retry is provably enough — after it, content is shared and the reference row is keyed by this DID, so no third writer can insert either. I also closed the mirror-image window: a PdsBlobRefs → PdsBlobs foreign key with Restrict stops orphan collection deleting content that another account has just referenced, and collection now treats losing that race as a no-op rather than failing the delete that triggered it.

The test drives the actual race rather than approximating it — a SaveChangesInterceptor inserts the conflicting content row between the store's existence check and its save, and the test asserts two save attempts, so the recovery path is what makes it pass. It runs on SQLite only; the EF in-memory provider lets a raw ArgumentException out of its backing dictionary instead of wrapping it as DbUpdateException, which is now recorded as a ReportsConstraintViolations flag on the suite.

Silently-dropped options on a repeat registration (PdsEfCoreStoreExtensions.cs:58) — also real, also fixed. The options registration is now replaced alongside the stores, and each store closes over the options instance from the call that registered it. The test proves it behaviourally: a second call configuring ClientSideAccountLookup + MaxClientSideLookupRows = 1 actually changes what the resolved IAccountStore returns.

Merge follow-through

The merge left the EF stores behind the interfaces #40 introduced, so I brought them forward — an EF-backed PDS would otherwise have thrown on every federation call:

ListAllRecordsAsync / ListBlobCidsAsync on EfCoreRepoStore. Records are ordered in memory by the ordinal collection/rkey MST key, not by SQL — ORDER BY (Collection, Rkey) is a different ordering ("a.b.c/x" sorts before "a.b/y" because . precedes /), and a collation need not be ordinal. A test pins the result against InMemoryRepoStore using exactly that case.
EfCoreRepoCommitStore<TContext> + PdsRepoHeadEntity, registered by AddAtProtoPdsEfCoreStores. Durable records paired with the default in-memory head store would restart the revision sequence on every process start, which relays read as the repo rewinding.
PdsAccountEntity.RotationKey, mapped through create/update/read.

Verified

dotnet build clean (0 errors, no new warnings on touched files); dotnet test tests/ATProtoNet.Tests/ — 1327 passed, 0 failed, including 82 in the EF store suite across both the in-memory and real-SQLite runs. CHANGELOG and docs/pds.md updated in the same commit.

⏱ 11m 15s · 74 turns · tokens: 117.5k in / 46.0k out (+6347.4k cache-read)
📜 Full session transcript

Merged `main` into `claude/issue-38` and worked through both review comments — pushed as `dbaf40c`, so the PR is up to date and conflict-free. ## Conflicts Three, all from the federation work in #40 landing on `main`: `CHANGELOG.md` (both added entries at the same spot — kept both), `docs/pds.md` (the new *Session signing key* section and my *Persistent Storage (EF Core)* section collided — kept both, session key under Configuration where it belongs), and a `using` block in `PdsHostingExtensions.cs`. ## Review feedback **Blob dedup race** (`EfCoreRepoStore.cs:244`) — real, and fixed. `PutBlobAsync` now retries once against a fresh context when `SaveChangesAsync` throws `DbUpdateException`: the winner stored the same content-addressed bytes, so the retry finds the row and writes only its own reference. One retry is provably enough — after it, content is shared and the reference row is keyed by this DID, so no third writer can insert either. I also closed the mirror-image window: a `PdsBlobRefs` → `PdsBlobs` foreign key with `Restrict` stops orphan collection deleting content that another account has just referenced, and collection now treats losing that race as a no-op rather than failing the delete that triggered it. The test drives the actual race rather than approximating it — a `SaveChangesInterceptor` inserts the conflicting content row between the store's existence check and its save, and the test asserts two save attempts, so the recovery path is what makes it pass. It runs on SQLite only; the EF in-memory provider lets a raw `ArgumentException` out of its backing dictionary instead of wrapping it as `DbUpdateException`, which is now recorded as a `ReportsConstraintViolations` flag on the suite. **Silently-dropped options on a repeat registration** (`PdsEfCoreStoreExtensions.cs:58`) — also real, also fixed. The options registration is now replaced alongside the stores, and each store closes over the `options` instance from the call that registered it. The test proves it behaviourally: a second call configuring `ClientSideAccountLookup` + `MaxClientSideLookupRows = 1` actually changes what the resolved `IAccountStore` returns. ## Merge follow-through The merge left the EF stores behind the interfaces #40 introduced, so I brought them forward — an EF-backed PDS would otherwise have thrown on every federation call: - `ListAllRecordsAsync` / `ListBlobCidsAsync` on `EfCoreRepoStore`. Records are ordered in memory by the ordinal `collection/rkey` MST key, not by SQL — `ORDER BY (Collection, Rkey)` is a different ordering (`"a.b.c/x"` sorts before `"a.b/y"` because `.` precedes `/`), and a collation need not be ordinal. A test pins the result against `InMemoryRepoStore` using exactly that case. - `EfCoreRepoCommitStore<TContext>` + `PdsRepoHeadEntity`, registered by `AddAtProtoPdsEfCoreStores`. Durable records paired with the default in-memory head store would restart the revision sequence on every process start, which relays read as the repo rewinding. - `PdsAccountEntity.RotationKey`, mapped through create/update/read. ## Verified `dotnet build` clean (0 errors, no new warnings on touched files); `dotnet test tests/ATProtoNet.Tests/` — **1327 passed, 0 failed**, including 82 in the EF store suite across both the in-memory and real-SQLite runs. CHANGELOG and `docs/pds.md` updated in the same commit. --- ⏱ 11m 15s · 74 turns · tokens: 117.5k in / 46.0k out (+6347.4k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-62/run-65.md)

Grandiras merged commit 60fd3bf584 into main

2026-07-25 16:13:57 +00:00

Grandiras deleted branch claude/issue-38

2026-07-25 16:13:57 +00:00

Grandiras referenced this pull request from a commit

2026-07-25 16:13:59 +00:00

Claude: changes for issue #38 (#62)

Sign in to join this conversation.

No reviewers