Grandiras/ATProto.NET

Fork 0

Claude: changes for issue #36 #64

Merged

Grandiras merged 3 commits from claude/issue-36 into main

2026-07-25 17:22:43 +00:00

claude-bot commented

2026-07-25 14:56:57 +00:00

Collaborator

Closes #36.

claude-bot added 1 commit

2026-07-25 14:56:57 +00:00

feat: store-backed PDS invite codes with admin endpoints

CI / build-and-test (pull_request) Has been cancelled

Details

ba09312109

With OpenRegistration = false, CreateAccountAsync only checked that the
invite code was non-empty, so any string created an account. Codes are now
stored, validated, and consumed.

- IInviteCodeStore + PdsInviteCode/PdsInviteCodeUse models, with a
  claim/confirm/release protocol so concurrent sign-ups can't double-spend
  a code; a failed sign-up releases the reservation
- InMemoryInviteCodeStore as the default, plus an
  AddAtProtoPds<TAccountStore, TRepoStore, TInviteCodeStore>() overload
- PdsService.CreateInviteCodeAsync/CreateInviteCodesAsync/GetInviteCodesAsync/
  GetAccountInviteCodesAsync/DisableInviteCodesAsync
- Five mapped XRPC endpoints: com.atproto.server.createInviteCode(s),
  getAccountInviteCodes, com.atproto.admin.getInviteCodes/disableInviteCodes,
  guarded by PdsOptions.AdminPassword (HTTP Basic, fixed-time compare, fails
  closed when unconfigured)
- 60 tests covering redemption, concurrency, admin auth, and paging

closes #36

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

Grandiras added 1 commit

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

Merge branch 'main' into claude/issue-36

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

Details

a11fc3df72

claude-bot commented

2026-07-25 14:57:49 +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/59); 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-64/run-59.md) — updating every ~5 minutes while the task runs.

claude-bot reviewed

2026-07-25 14:59:09 +00:00

claude-bot left a comment

src/ATProtoNet.Pds/PdsHostingExtensions.cs:545 — The admin username is compared with string.Equals(..., StringComparison.Ordinal) rather than a constant-time comparison, while the password uses FixedTimeEquals. This is standard practice (usernames aren't normally treated as secret) and fine for the default "admin", but worth noting if a deployment ever treats a custom AdminUsername as sensitive.
src/ATProtoNet.Pds/PdsService.cs:328 — CreateInviteCodesAsync issues codes per-account sequentially with no transactional rollback: if code creation fails partway through the account list (e.g. a store error on the second account), earlier accounts keep their already-created codes while the caller sees an exception with no indication of the partial state. Likely acceptable for this store's semantics, but worth a doc note since it's not obvious from the signature.

⏱ 1m 5s · 1 turns · tokens: 45.6k in / 5.7k out (+29.1k cache-read)
📜 Full session transcript

Solid, well-scoped fix for issue #36 (closed registration was previously bypassable with any non-empty string). The three-step claim/confirm/release protocol correctly prevents double-spending under concurrency (verified by the `TryClaimAsync_ConcurrentClaims_NeverExceedsAvailableUses` and `CreateAccountAsync_ConcurrentRedemptions_DoNotDoubleSpendACode` tests), the failure-releases-the-claim path is tested, admin endpoints fail closed when `AdminPassword` is unset, and the password comparison uses `FixedTimeEquals`. CHANGELOG is thorough and correctly flags the binary-breaking constructor change and the behavioral change for closed-registration deployments. Test coverage across the store, service, and HTTP endpoint layers is extensive. No blocking correctness or security issues found; a couple of minor, non-blocking observations below. - `src/ATProtoNet.Pds/PdsHostingExtensions.cs:545` — The admin username is compared with `string.Equals(..., StringComparison.Ordinal)` rather than a constant-time comparison, while the password uses `FixedTimeEquals`. This is standard practice (usernames aren't normally treated as secret) and fine for the default `"admin"`, but worth noting if a deployment ever treats a custom `AdminUsername` as sensitive. - `src/ATProtoNet.Pds/PdsService.cs:328` — `CreateInviteCodesAsync` issues codes per-account sequentially with no transactional rollback: if code creation fails partway through the account list (e.g. a store error on the second account), earlier accounts keep their already-created codes while the caller sees an exception with no indication of the partial state. Likely acceptable for this store's semantics, but worth a doc note since it's not obvious from the signature. --- ⏱ 1m 5s · 1 turns · tokens: 45.6k in / 5.7k out (+29.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-64/run-59.md)

claude-bot referenced this pull request

2026-07-25 15:00:43 +00:00

Pds: invite codes are only presence-checked — no store, validation, single-use semantics, or admin endpoints #36

Grandiras commented

2026-07-25 17:09:26 +00:00

Owner

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

claude-bot commented

2026-07-25 17:09:28 +00:00

Author

Collaborator

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

claude-bot added 1 commit

2026-07-25 17:19:30 +00:00

merge: main into claude/issue-36

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

Details

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

Details

c134a875d4

Resolves the conflicts between the invite code work (Issue #36) and the
federation/EF Core work that landed on main (Issues #38, #40):

- PdsService: both constructors keep their federation parameters and gain a
  trailing optional `IInviteCodeStore? inviteCodes = null`. CreateAccountAsync
  claims the invite code up front, then mints the identity through
  PdsIdentityService inside the try, so a failed identity mint or account write
  releases the claim.
- PdsHostingExtensions: `AddAtProtoPds<TAccountStore, TRepoStore, TInviteCodeStore>()`
  is replaced by `AddAtProtoPdsInviteCodeStore<TInviteCodeStore>()` — main added
  a three-generic overload of the same arity for the head store, and the two
  could not coexist. The default invite store is now registered with TryAdd, so
  a host store registered in either order wins, matching the EF Core stores.

Also addresses the PR review feedback:

- The admin Basic credential compares both halves in fixed time and does not
  short-circuit, so response time reveals neither which half was wrong nor how
  much of it matched.
- CreateInviteCodesAsync documents that it is not atomic, validates useCount up
  front so a bad argument writes nothing, and points at the recovery path.

Tests: 1394 passing (Debug and Release). New coverage for the invite store
registration in either call order, the useCount-writes-nothing guarantee, and
redemption against a DID minted by a federating PDS.

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

claude-bot commented

2026-07-25 17:19:45 +00:00

Author

Collaborator

Pushed to claude/issue-36 (commit c134a87) — conflicts resolved and both review points addressed.

Conflicts

main had moved a long way underneath this branch (federation from #40, EF Core PDS stores from #38), so four files conflicted:

PdsService.cs — both constructors now keep main's federation parameters and gain a trailing optional IInviteCodeStore? inviteCodes = null. CreateAccountAsync claims the invite code up front, then mints the identity through PdsIdentityService inside the try block, so a failed identity mint or account write releases the claim just like a taken handle does.
PdsHostingExtensions.cs — a real collision, not just textual: main added AddAtProtoPds<TAccountStore, TRepoStore, TCommitStore>(), which has the same arity as this branch's AddAtProtoPds<TAccountStore, TRepoStore, TInviteCodeStore>(). Generic constraints don't disambiguate overloads, so the two could not coexist (CS0111). I replaced the invite-code overload with AddAtProtoPdsInviteCodeStore<TInviteCodeStore>(), matching the AddAtProtoPdsEfCoreStores<TContext>() pattern main established, and the in-memory default is now registered with TryAdd so a host store wins in either call order. CreatePdsService passes the resolved store through.
CHANGELOG.md / docs/pds.md — both sides' entries kept; the Invite Codes doc section moved to sit after the Configuration section rather than swallowing ### Session signing key as a subsection.

Review feedback

Admin username comparison (PdsHostingExtensions.cs:918) — both halves of the Basic credential now go through fixed-time comparison, joined with non-short-circuiting &, so response time reveals neither which half failed nor how much of it matched. Length is still not hidden — FixedTimeEquals returns early on unequal spans — and that's stated in the doc comment.
CreateInviteCodesAsync partial state (PdsService.cs:406) — documented as non-atomic in XML docs, docs/pds.md, and the CHANGELOG, with the recovery path spelled out (GetAccountInviteCodesAsync + DisableInviteCodesAsync, not a blind retry). useCount is now validated up front alongside codeCount, so a bad argument can no longer leave one account's batch written and the rest not — a store error is the only remaining partial-failure path.

Verified

dotnet build clean (0 errors), and dotnet test tests/ATProtoNet.Tests/ passes 1394/1394 in both Debug and Release (CI runs Release). Three new tests: the invite store registration winning in either call order and actually being the one PdsService writes to, useCount: 0 issuing nothing across a multi-account batch, and redemption recorded against a DID minted by a federating PDS — the path the merge newly created.

⏱ 10m 6s · 78 turns · tokens: 106.1k in / 33.9k out (+6246.0k cache-read)
📜 Full session transcript

Pushed to `claude/issue-36` (commit `c134a87`) — conflicts resolved and both review points addressed. ## Conflicts `main` had moved a long way underneath this branch (federation from #40, EF Core PDS stores from #38), so four files conflicted: - **`PdsService.cs`** — both constructors now keep main's federation parameters *and* gain a trailing optional `IInviteCodeStore? inviteCodes = null`. `CreateAccountAsync` claims the invite code up front, then mints the identity through `PdsIdentityService` **inside** the try block, so a failed identity mint or account write releases the claim just like a taken handle does. - **`PdsHostingExtensions.cs`** — a real collision, not just textual: main added `AddAtProtoPds<TAccountStore, TRepoStore, TCommitStore>()`, which has the same arity as this branch's `AddAtProtoPds<TAccountStore, TRepoStore, TInviteCodeStore>()`. Generic constraints don't disambiguate overloads, so the two could not coexist (CS0111). I replaced the invite-code overload with **`AddAtProtoPdsInviteCodeStore<TInviteCodeStore>()`**, matching the `AddAtProtoPdsEfCoreStores<TContext>()` pattern main established, and the in-memory default is now registered with `TryAdd` so a host store wins in either call order. `CreatePdsService` passes the resolved store through. - **`CHANGELOG.md` / `docs/pds.md`** — both sides' entries kept; the Invite Codes doc section moved to sit after the Configuration section rather than swallowing `### Session signing key` as a subsection. ## Review feedback - **Admin username comparison** (`PdsHostingExtensions.cs:918`) — both halves of the Basic credential now go through fixed-time comparison, joined with non-short-circuiting `&`, so response time reveals neither which half failed nor how much of it matched. Length is still not hidden — `FixedTimeEquals` returns early on unequal spans — and that's stated in the doc comment. - **`CreateInviteCodesAsync` partial state** (`PdsService.cs:406`) — documented as non-atomic in XML docs, `docs/pds.md`, and the CHANGELOG, with the recovery path spelled out (`GetAccountInviteCodesAsync` + `DisableInviteCodesAsync`, not a blind retry). `useCount` is now validated up front alongside `codeCount`, so a bad argument can no longer leave one account's batch written and the rest not — a store error is the only remaining partial-failure path. ## Verified `dotnet build` clean (0 errors), and `dotnet test tests/ATProtoNet.Tests/` passes **1394/1394 in both Debug and Release** (CI runs Release). Three new tests: the invite store registration winning in either call order and actually being the one `PdsService` writes to, `useCount: 0` issuing nothing across a multi-account batch, and redemption recorded against a DID minted by a federating PDS — the path the merge newly created. --- ⏱ 10m 6s · 78 turns · tokens: 106.1k in / 33.9k out (+6246.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-64/run-67.md)

Grandiras merged commit 83f68712a4 into main

2026-07-25 17:22:43 +00:00

Grandiras deleted branch claude/issue-36

2026-07-25 17:22:43 +00:00

Grandiras referenced this pull request from a commit

2026-07-25 17:22:45 +00:00

Claude: changes for issue #36 (#64)