The space server shipped with only an in-memory implementation of each
of its three stores. Two of those defaults are more than an
inconvenience: the replay store is what makes a delegation token, a
client attestation, and a DPoP proof single-use, so being per-process
means two replicas accept the same delegation token twice; and a
simplespace member list is never published to the network, so a restart
that loses one loses the space's access control with nothing to rebuild
it from.

- RedisSpaceReplayStore: consuming a token is one SET NX with the
  token's own exp as the TTL, so the check and the write are atomic
  across instances and nothing sweeps.
- EfCoreSpaceReplayStore<T>: the same guarantee for a deployment with no
  Redis, from the table's primary key being (iss, jti, exp). A failed
  save is confirmed against the table before being reported as a replay,
  so a storage fault is not a silently refused token.
- EfCoreSimpleSpaceStore<T> and EfCoreSpaceAuthorityStore<T>: the member
  lists, the writer set, and the notification registrations, keeping the
  in-memory stores' DID-ordered pagination and their refusal to walk a
  repo's revision backwards.
- AddAtProtoSpaces() now says at startup which stores it ended up with,
  since nothing else reported it and the symptom is an accepted replay
  rather than an error. Suppress with WarnOnInMemoryStores.

Verified against SQLite and PostgreSQL (16-way concurrent presentation
of one token yields exactly one success on both) and against a live
Redis through a new [RequiresRedisFact] gate.

closes #102

claude-bot referenced this pull request

2026-08-21 02:10:00 +00:00

Spaces: durable and multi-instance implementations of the space server stores #102

claude-bot commented

2026-08-21 02:10:02 +00:00

Author

Collaborator

🤖 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/358); 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-104/run-358.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-21 02:13:08 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This PR adds durable/multi-instance implementations for the three space-server stores (Redis and EF Core replay stores, EF Core authority store, EF Core simplespace store), plus a startup warning hosted service and matching docs/changelog updates. The implementation is careful and internally consistent:

The replay stores correctly make token consumption atomic: Redis via SET NX EX, EF Core via the (Issuer, TokenId, ExpiresAt) primary key with a post-failure existence check to distinguish a genuine storage fault from a real replay.
EfCoreSpaceAuthorityStore's upsert-via-retry pattern (MutateAsync) correctly handles races between concurrent EnsureSpaceAsync/subscriber/writer upserts — verified the retry converges to the update path rather than double-inserting or looping.
The ISpaceReplayStore registration extensions correctly use services.Replace(...) (not TryAdd) because AddAtProtoSpaces() eagerly registers the in-memory default; AddAtProtoEfCoreSpaceAuthority/AddAtProtoEfCoreSimpleSpace correctly rely on plain TryAddSingleton inside the existing AddSpaceAuthority<TStore>/AddSimpleSpace<TStore> methods, since those have no eager default to fight with — checked this against SpaceServerExtensions.cs and it lines up, and SpaceStoreRegistrationTests exercises both call orders for the replay store.
InMemorySpaceStoreWarning only warns about a store type that's actually registered (GetService<T>() is InMemoryX), so services that don't register an authority/simplespace store (e.g. a bare repo host) won't get spurious warnings.
Verified StackExchange.Redis 2.13.17 and SQLitePCLRaw.bundle_e_sqlite3 3.0.5 (pinned to dodge GHSA-2m69-gcr7-jv3q, CVE-2025-6965, on the EF Core Sqlite provider's transitive 2.1.11 dependency) are both real, published package versions — the csproj comments check out.
Test coverage is solid: EF Core stores are tested against real SQLite (not the in-memory provider, which wouldn't model the PK-violation-as-replay-check behavior), the Redis store's command shape is unit-tested via NSubstitute plus a gated live-server integration test, and the startup-warning behavior is covered directly.

Minor, non-blocking observations: the Redis key format (issuer|tokenId|exp) isn't escaped, so a | inside an issuer or token ID could theoretically collide with a different tuple — low risk given DIDs/JWT jti values don't contain it. Nothing here rises to a blocking finding.

⏱ 2m 52s · 9 turns · tokens: 87.4k in / 14.8k out (+828.8k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This PR adds durable/multi-instance implementations for the three space-server stores (Redis and EF Core replay stores, EF Core authority store, EF Core simplespace store), plus a startup warning hosted service and matching docs/changelog updates. The implementation is careful and internally consistent: - The replay stores correctly make token consumption atomic: Redis via `SET NX EX`, EF Core via the `(Issuer, TokenId, ExpiresAt)` primary key with a post-failure existence check to distinguish a genuine storage fault from a real replay. - `EfCoreSpaceAuthorityStore`'s upsert-via-retry pattern (`MutateAsync`) correctly handles races between concurrent `EnsureSpaceAsync`/subscriber/writer upserts — verified the retry converges to the update path rather than double-inserting or looping. - The `ISpaceReplayStore` registration extensions correctly use `services.Replace(...)` (not `TryAdd`) because `AddAtProtoSpaces()` eagerly registers the in-memory default; `AddAtProtoEfCoreSpaceAuthority`/`AddAtProtoEfCoreSimpleSpace` correctly rely on plain `TryAddSingleton` inside the existing `AddSpaceAuthority<TStore>`/`AddSimpleSpace<TStore>` methods, since those have no eager default to fight with — checked this against `SpaceServerExtensions.cs` and it lines up, and `SpaceStoreRegistrationTests` exercises both call orders for the replay store. - `InMemorySpaceStoreWarning` only warns about a store type that's actually registered (`GetService<T>() is InMemoryX`), so services that don't register an authority/simplespace store (e.g. a bare repo host) won't get spurious warnings. - Verified `StackExchange.Redis 2.13.17` and `SQLitePCLRaw.bundle_e_sqlite3 3.0.5` (pinned to dodge GHSA-2m69-gcr7-jv3q, CVE-2025-6965, on the EF Core Sqlite provider's transitive 2.1.11 dependency) are both real, published package versions — the csproj comments check out. - Test coverage is solid: EF Core stores are tested against real SQLite (not the in-memory provider, which wouldn't model the PK-violation-as-replay-check behavior), the Redis store's command shape is unit-tested via NSubstitute plus a gated live-server integration test, and the startup-warning behavior is covered directly. Minor, non-blocking observations: the Redis key format (`issuer|tokenId|exp`) isn't escaped, so a `|` inside an issuer or token ID could theoretically collide with a different tuple — low risk given DIDs/JWT `jti` values don't contain it. Nothing here rises to a blocking finding. --- ⏱ 2m 52s · 9 turns · tokens: 87.4k in / 14.8k out (+828.8k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-104/run-358.md)