Claude: changes for issue #105 #106

Merged
Grandiras merged 1 commit from claude/issue-105 into main 2026-08-21 02:23:48 +00:00
Collaborator

Closes #105.

Closes #105.
fix: bridge simplespace to the space authority store
All checks were successful
CI / pds-integration (pull_request) Successful in 26s
Sync Closures to GitHub / sync-closure (pull_request) Successful in 6s
CI / build-and-test (pull_request) Successful in 57s
cdc352c1a8
A space created through com.atproto.simplespace.createSpace was never
known to the authority store, so listRepos, registerNotify, and
notifyWrite all refused it with SpaceNotFound: the two stores hold
separate state and nothing but a test ever called DeclareSpace. The
writer set of a space could therefore never be populated, and the writer
set is the sync boundary. deleteSpace had the mirror gap — the authority
store kept answering Granted, so listRepos returned the writer set of a
deleted space instead of SpaceDeleted.

AddSpaceAuthority<T>() now wraps the store in SimpleSpaceAuthorityStore
whenever an ISimpleSpaceStore is registered, in either order. Existence
and deletion are read from the space-management store rather than copied
into the authority store, so there is no second write to keep in step;
the writer set and the notification registrations stay the authority's
own. A space the simplespace store has never heard of falls through to
the inner store, so a bespoke space type keeps declaring its spaces
directly.

closes #105
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/362); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-106/run-362.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

Solid fix for issue #105. SimpleSpaceAuthorityStore correctly bridges the two stores by reading existence/deletion from ISimpleSpaceStore (via GetSpaceAsync) while keeping the writer set and notification registrations in the wrapped inner store — this exactly mirrors the Granted/SpaceDeleted/SpaceNotFound semantics already implemented by InMemorySpaceAuthorityStore.GetSpaceStateAsync and EfCoreSpaceAuthorityStore<T>.GetSpaceStateAsync, so no behavioral drift between the wrapped and unwrapped cases.

The AddSpaceAuthority<TStore> registration now resolves the wrap lazily inside the ISpaceAuthorityStore factory (checking sp.GetService<ISimpleSpaceStore>() at resolution time), which correctly handles either registration order — verified against the two new SpaceStoreRegistrationTests cases. TryAddSingleton semantics mean a caller-supplied ISpaceAuthorityStore registered before this call is left untouched, matching the documented contract.

InMemorySpaceStoreWarning unwraps the bridge (bridge.Inner) before checking for InMemorySpaceAuthorityStore, so the restart-data-loss warning still fires correctly when simplespace is combined with the in-memory authority store.

All write-path endpoints (listRepos, registerNotify, notifyWrite) call GetSpaceStateAsync and check for Granted before invoking any mutating method, so the bridge's straight pass-through RecordWriteAsync/RegisterNotifyAsync (with no existence check of their own) introduces no new race or bypass — this matches the guard pattern the other two store implementations already rely on.

Test coverage is thorough: bridge unit tests cover found/deleted/fall-through/not-found paths and write pass-through, and the new integration tests exercise the full HTTP flow (create via createSpace, notify, list, delete-then-list expecting SpaceDeleted, and unknown-space SpaceNotFound). Docs and XML remarks were updated consistently across all four touched files. CHANGELOG entry is accurate and detailed.

No correctness or security issues found.


⏱ 0m 59s · 8 turns · tokens: 38.1k in / 5.2k out (+437.3k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Solid fix for issue #105. `SimpleSpaceAuthorityStore` correctly bridges the two stores by reading existence/deletion from `ISimpleSpaceStore` (via `GetSpaceAsync`) while keeping the writer set and notification registrations in the wrapped inner store — this exactly mirrors the `Granted`/`SpaceDeleted`/`SpaceNotFound` semantics already implemented by `InMemorySpaceAuthorityStore.GetSpaceStateAsync` and `EfCoreSpaceAuthorityStore<T>.GetSpaceStateAsync`, so no behavioral drift between the wrapped and unwrapped cases. The `AddSpaceAuthority<TStore>` registration now resolves the wrap lazily inside the `ISpaceAuthorityStore` factory (checking `sp.GetService<ISimpleSpaceStore>()` at resolution time), which correctly handles either registration order — verified against the two new `SpaceStoreRegistrationTests` cases. `TryAddSingleton` semantics mean a caller-supplied `ISpaceAuthorityStore` registered before this call is left untouched, matching the documented contract. `InMemorySpaceStoreWarning` unwraps the bridge (`bridge.Inner`) before checking for `InMemorySpaceAuthorityStore`, so the restart-data-loss warning still fires correctly when simplespace is combined with the in-memory authority store. All write-path endpoints (`listRepos`, `registerNotify`, `notifyWrite`) call `GetSpaceStateAsync` and check for `Granted` before invoking any mutating method, so the bridge's straight pass-through `RecordWriteAsync`/`RegisterNotifyAsync` (with no existence check of their own) introduces no new race or bypass — this matches the guard pattern the other two store implementations already rely on. Test coverage is thorough: bridge unit tests cover found/deleted/fall-through/not-found paths and write pass-through, and the new integration tests exercise the full HTTP flow (create via `createSpace`, notify, list, delete-then-list expecting `SpaceDeleted`, and unknown-space `SpaceNotFound`). Docs and XML remarks were updated consistently across all four touched files. CHANGELOG entry is accurate and detailed. No correctness or security issues found. --- ⏱ 0m 59s · 8 turns · tokens: 38.1k in / 5.2k out (+437.3k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-106/run-362.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:cdc352c1a8ff8cd44cda1bfc51af790d13e638c4 -->
Grandiras deleted branch claude/issue-105 2026-08-21 02:23:48 +00:00
Sign in to join this conversation.
No description provided.