feat(atproto): remaining com.atproto and site.standard gaps #161

Merged
Grandiras merged 1 commit from issue-129-atproto-gaps into main 2026-09-26 00:02:51 +00:00
Owner

Closes #129.

Fills the remaining com.atproto.* and site.standard.* gaps. Everything is additive: no breaking changes.

com.atproto.sync.getRecord and getBlocks

  • SyncClient.GetRecordAsync(did, collection, rkey) returns the proof CAR as an XrpcStreamResponse, over the existing binary download path (#117).
  • SyncClient.GetVerifiedRecordAsync(did, collection, rkey, signingKey) downloads the proof (bounded read, 16 MiB ceiling, checked against both a declared Content-Length and the bytes read) and verifies it. RecordProof.Verify(car, did, collection, rkey, signingKey) (new, ATProtoNet.Repo) verifies a CAR the caller already holds. Both return a VerifiedRecord: Uri, Commit (CID), Rev, and Cid / Value / Exists. Exists == false means the signed tree proves the record absent.
  • Verification builds on #111's CAR/MST code and #112's crypto:
    • every block CID is recomputed (CarReader.FromBytes(verifyBlockCids: true));
    • there is exactly one root, and it is the commit;
    • the commit must be version 3 and name the expected DID, and its signature must verify against the did:key (splicing out sig via FirehoseVerifier.ExtractSignedView);
    • the walk follows MST.get of @atproto/repo, and additionally checks that each node on the path holds valid, strictly increasing keys, inside the range its parent leaves for the subtree, all on one layer, with each child exactly one layer below its parent;
    • the record block must be present and DAG-CBOR, and it is decoded to AT Protocol JSON.
  • Failures throw the new RepoVerificationException : AtProtoException. A malformed did:key throws FormatException.
  • SyncClient.GetBlocksAsync(did, cids) returns a CAR stream, with cids repeated per the XRPC array convention.

com.atproto.repo.importRepo

  • RepoClient.ImportRepoAsync(Stream car) is a CAR upload (application/vnd.ipld.car) over the existing upload path.
  • XrpcClient gains a non-generic UploadAsync for procedures without output. It shares request construction with the generic one.
  • A seekable stream sends Content-Length, which the Lexicon asks for, and can be replayed on a DPoP-nonce or 429 retry. A non-seekable stream goes chunked, which the reference PDS accepts.

com.atproto.admin

  • AdminClient.SearchAccountsAsync(email?, limit?, cursor?) plus EnumerateSearchAccountsAsync. SearchAccountsResponse implements ICursorPage<AccountInfo>.
  • AdminClient.UpdateAccountSigningKeyAsync(did, signingKey), where signingKey is a Did because the Lexicon's format is did (a did:key). The request body is internal.
  • PdsAdminClient.SearchAccountsAsync / EnumerateSearchAccountsAsync go through the usual admin-session handling. I checked the sources:
    • Tranquil PDS serves searchAccounts; the reference PDS doesn't (the entryway does, and Ozone proxies it).
    • updateAccountSigningKey is entryway-only; neither PDS implements it.
    • So only search is surfaced on PdsAdminClient; the signing-key update stays reachable through PdsAdminClient.Admin.

com.atproto.temp

New AtProtoClient.Temp (TempClient, Lexicon/Com/AtProto/Temp/{TempClient,TempModels}.cs):

  • CheckHandleAvailabilityAsync(handle, email?, birthDate?): the result is an open union (HandleAvailable / HandleUnavailable with HandleSuggestions / UnknownHandleAvailabilityResult), plus a convenience IsAvailable.
  • CheckSignupQueueAsync()
  • DereferenceScopeAsync(scope) returns the full scope string.
  • RequestPhoneVerificationAsync(phoneNumber)
  • RevokeAccountCredentialsAsync(account), a moderator action that Ozone proxies to the PDS.

Not covered: fetchLabels (deprecated upstream) and addReservedHandle (entryway-internal admin).

site.standard

  • RecommendRecord (site.standard.graph.recommend: document, createdAt) and StandardSiteClient.CreateRecommendationAsync / GetRecommendationAsync (by repo + rkey, or by AtUri) / DeleteRecommendationAsync / ListRecommendationsAsync / EnumerateRecommendationsAsync, built like the subscription methods.
  • Two checklist items were already done:
    • #123 added the record fields (contributors, labels, links, createdAt);
    • #119 made the list methods typed (RecordPage<T> via RecordCollection<T>.ToPage).

Verification

  • Full solution build (--no-incremental): 0 warnings, 0 errors.
  • Unit tests: 3324 passed, 0 failed, 5 skipped. The skips are the existing environment-gated Aspire manifest tests.
  • Reference vectors: tests/ATProtoNet.Tests/Repo/TestData/record-proof.json holds a presence proof and an absence proof produced by @atproto/repo 0.10.14 / @atproto/crypto 0.5.5.
    • The repository is 60 records, signed with a fixed K-256 key; the path is 3 MST layers deep.
    • The reference verifyRecords / verifyProofs accept both proofs before the file is written.
    • The generator (gen-record-proof.mjs) and a README sit next to it; the output is deterministic.
  • RecordProofTests (23 tests):
    • positive reference proofs (record content, $bytes, CIDs, commit, rev) and the reference absence proof;
    • tampered proofs: another signing key, another DID, altered record bytes, a record swapped for another with its own valid CID, a missing tree node, a missing commit block, a flipped signature byte, a commit re-signed by another key, two roots, a non-CAR input, a key off the proven path, and an invalid did:key;
    • proofs of SDK-written repos (P-256, 200 records, plus the empty repo);
    • hand-built malformed trees (keys out of order, mixed layers, a child on its parent's layer, a subtree key outside its range), plus a well-formed control.
  • Endpoint tests:
    • wire shape (method, path, query, content type and length, body) and response parsing for each new method: SyncClientTests, ComAtProtoGapTests, PdsAdminClient*Tests, StandardSiteClientTests;
    • the bounded proof read, with both a declared-length and a chunked oversize response.
  • The upstream drift test (#123) passes. The one allow-list addition is DefOf[HandleSuggestion] = com.atproto.temp.checkHandleAvailability#suggestion.
  • Integration test (gated, compiles; not run locally): RepositoryTests.GetVerifiedRecord_ProvesACreatedRecordAndThenItsDeletion checks the proof against a live PDS, taking the key from getRecommendedDidCredentials.

Notes

  • The issue describes dereferenceScope as expanding include: scopes. Per the Lexicon and the PDS (ScopeReferenceGetter), it expands ref: scope references; the docs say so.
  • Rebased onto ca43404 (#158, session rework). The conflicts were CHANGELOG and docs/architecture.md only.
  • Edits outside this issue's area, all minimal:
    • AtProtoClient.cs: one using, one constructor line, one property;
    • Http/XrpcClient.cs: the non-generic UploadAsync, sharing CreateUpload;
    • MerkleSearchTree.MaxTreeDepth: private → internal.

🤖 Generated with Claude Code

Closes #129. Fills the remaining `com.atproto.*` and `site.standard.*` gaps. Everything is additive: no breaking changes. ## `com.atproto.sync.getRecord` and `getBlocks` - **`SyncClient.GetRecordAsync(did, collection, rkey)`** returns the proof CAR as an `XrpcStreamResponse`, over the existing binary download path (#117). - **`SyncClient.GetVerifiedRecordAsync(did, collection, rkey, signingKey)`** downloads the proof (bounded read, 16 MiB ceiling, checked against both a declared `Content-Length` and the bytes read) and verifies it. **`RecordProof.Verify(car, did, collection, rkey, signingKey)`** (new, `ATProtoNet.Repo`) verifies a CAR the caller already holds. Both return a **`VerifiedRecord`**: `Uri`, `Commit` (CID), `Rev`, and `Cid` / `Value` / `Exists`. `Exists == false` means the signed tree proves the record absent. - Verification builds on #111's CAR/MST code and #112's crypto: - every block CID is recomputed (`CarReader.FromBytes(verifyBlockCids: true)`); - there is exactly one root, and it is the commit; - the commit must be version 3 and name the expected DID, and its signature must verify against the `did:key` (splicing out `sig` via `FirehoseVerifier.ExtractSignedView`); - the walk follows `MST.get` of `@atproto/repo`, and additionally checks that each node on the path holds valid, strictly increasing keys, inside the range its parent leaves for the subtree, all on one layer, with each child exactly one layer below its parent; - the record block must be present and DAG-CBOR, and it is decoded to AT Protocol JSON. - Failures throw the new **`RepoVerificationException : AtProtoException`**. A malformed `did:key` throws `FormatException`. - **`SyncClient.GetBlocksAsync(did, cids)`** returns a CAR stream, with `cids` repeated per the XRPC array convention. ## `com.atproto.repo.importRepo` - **`RepoClient.ImportRepoAsync(Stream car)`** is a CAR upload (`application/vnd.ipld.car`) over the existing upload path. - `XrpcClient` gains a non-generic `UploadAsync` for procedures without output. It shares request construction with the generic one. - A seekable stream sends `Content-Length`, which the Lexicon asks for, and can be replayed on a DPoP-nonce or 429 retry. A non-seekable stream goes chunked, which the reference PDS accepts. ## `com.atproto.admin` - **`AdminClient.SearchAccountsAsync(email?, limit?, cursor?)`** plus `EnumerateSearchAccountsAsync`. `SearchAccountsResponse` implements `ICursorPage<AccountInfo>`. - **`AdminClient.UpdateAccountSigningKeyAsync(did, signingKey)`**, where `signingKey` is a `Did` because the Lexicon's format is `did` (a `did:key`). The request body is internal. - **`PdsAdminClient.SearchAccountsAsync` / `EnumerateSearchAccountsAsync`** go through the usual admin-session handling. I checked the sources: - Tranquil PDS serves `searchAccounts`; the reference PDS doesn't (the entryway does, and Ozone proxies it). - `updateAccountSigningKey` is entryway-only; neither PDS implements it. - So only search is surfaced on `PdsAdminClient`; the signing-key update stays reachable through `PdsAdminClient.Admin`. ## `com.atproto.temp` New **`AtProtoClient.Temp`** (`TempClient`, `Lexicon/Com/AtProto/Temp/{TempClient,TempModels}.cs`): - **`CheckHandleAvailabilityAsync(handle, email?, birthDate?)`**: the result is an open union (`HandleAvailable` / `HandleUnavailable` with `HandleSuggestion`s / `UnknownHandleAvailabilityResult`), plus a convenience `IsAvailable`. - **`CheckSignupQueueAsync()`** - **`DereferenceScopeAsync(scope)`** returns the full scope string. - **`RequestPhoneVerificationAsync(phoneNumber)`** - **`RevokeAccountCredentialsAsync(account)`**, a moderator action that Ozone proxies to the PDS. Not covered: `fetchLabels` (deprecated upstream) and `addReservedHandle` (entryway-internal admin). ## `site.standard` - **`RecommendRecord`** (`site.standard.graph.recommend`: `document`, `createdAt`) and `StandardSiteClient.CreateRecommendationAsync` / `GetRecommendationAsync` (by repo + rkey, or by `AtUri`) / `DeleteRecommendationAsync` / `ListRecommendationsAsync` / `EnumerateRecommendationsAsync`, built like the subscription methods. - Two checklist items were already done: - #123 added the record fields (`contributors`, `labels`, `links`, `createdAt`); - #119 made the list methods typed (`RecordPage<T>` via `RecordCollection<T>.ToPage`). ## Verification - Full solution build (`--no-incremental`): **0 warnings, 0 errors**. - Unit tests: **3324 passed, 0 failed, 5 skipped**. The skips are the existing environment-gated Aspire manifest tests. - **Reference vectors**: `tests/ATProtoNet.Tests/Repo/TestData/record-proof.json` holds a presence proof and an absence proof produced by `@atproto/repo` 0.10.14 / `@atproto/crypto` 0.5.5. - The repository is 60 records, signed with a fixed K-256 key; the path is 3 MST layers deep. - The reference `verifyRecords` / `verifyProofs` accept both proofs before the file is written. - The generator (`gen-record-proof.mjs`) and a README sit next to it; the output is deterministic. - `RecordProofTests` (23 tests): - positive reference proofs (record content, `$bytes`, CIDs, commit, rev) and the reference absence proof; - tampered proofs: another signing key, another DID, altered record bytes, a record swapped for another with its own valid CID, a missing tree node, a missing commit block, a flipped signature byte, a commit re-signed by another key, two roots, a non-CAR input, a key off the proven path, and an invalid `did:key`; - proofs of SDK-written repos (P-256, 200 records, plus the empty repo); - hand-built malformed trees (keys out of order, mixed layers, a child on its parent's layer, a subtree key outside its range), plus a well-formed control. - Endpoint tests: - wire shape (method, path, query, content type and length, body) and response parsing for each new method: `SyncClientTests`, `ComAtProtoGapTests`, `PdsAdminClient*Tests`, `StandardSiteClientTests`; - the bounded proof read, with both a declared-length and a chunked oversize response. - The upstream drift test (#123) passes. The one allow-list addition is `DefOf[HandleSuggestion] = com.atproto.temp.checkHandleAvailability#suggestion`. - Integration test (gated, compiles; not run locally): `RepositoryTests.GetVerifiedRecord_ProvesACreatedRecordAndThenItsDeletion` checks the proof against a live PDS, taking the key from `getRecommendedDidCredentials`. ## Notes - The issue describes `dereferenceScope` as expanding `include:` scopes. Per the Lexicon and the PDS (`ScopeReferenceGetter`), it expands `ref:` scope references; the docs say so. - Rebased onto `ca43404` (#158, session rework). The conflicts were CHANGELOG and `docs/architecture.md` only. - Edits outside this issue's area, all minimal: - `AtProtoClient.cs`: one `using`, one constructor line, one property; - `Http/XrpcClient.cs`: the non-generic `UploadAsync`, sharing `CreateUpload`; - `MerkleSearchTree.MaxTreeDepth`: `private` → `internal`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(atproto): remaining com.atproto and site.standard gaps (closes #129)
All checks were successful
CI / pds-integration (pull_request) Successful in 28s
CI / build-and-test (pull_request) Successful in 1m0s
6f99cecb21
Fills the com.atproto and site.standard methods the SDK still lacked.

- com.atproto.sync.getRecord: SyncClient.GetRecordAsync returns the proof
  CAR, and GetVerifiedRecordAsync / RecordProof.Verify check it against
  the signed commit (block CIDs, commit DID and signature, and the MST
  path, including layer and key-range checks) and return the record or
  proof of its absence. Pinned to proofs from @atproto/repo 0.10.14,
  plus tampered and malformed-tree cases.
- sync.getBlocks and repo.importRepo (a CAR upload, for migration),
  over the existing binary download and upload paths.
- admin.searchAccounts / updateAccountSigningKey on AdminClient;
  PdsAdminClient gains SearchAccountsAsync, which Tranquil serves.
- com.atproto.temp: checkHandleAvailability (open result union),
  checkSignupQueue, dereferenceScope, requestPhoneVerification and
  revokeAccountCredentials on a new AtProtoClient.Temp.
- site.standard.graph.recommend: RecommendRecord and the
  StandardSiteClient recommendation methods. The record fields the
  issue lists were already added by #123, and the typed list methods
  by #119.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Owner

@claude /review — please read RecordProof.Verify (the sync.getRecord proof verification) in full; it's the security-relevant part.

@claude /review — please read `RecordProof.Verify` (the `sync.getRecord` proof verification) in full; it's the security-relevant part.
Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-161/run-2026-09-25T20-54-41-695Z.md) — updating every ~5 minutes while the review runs.
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/602); 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-161/run-602.md) — updating every ~5 minutes while the task runs.
Collaborator

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T01:54:42.152Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T01:54:42.152Z) — nothing to re-trigger. <!-- opencode-run:paused --> <!-- opencode-session:ses_f25a6a7f8ffemCWJuymM83KdEo -->
Collaborator

🤖 Resolving the merge conflicts — the reply will link the full session transcript.

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

🤖 Resolving the merge conflicts — the reply will link the full session transcript. <!-- opencode-run:conflict --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-161/run-2026-09-25T21-03-09-194Z.md) — updating every ~5 minutes while the fix runs.
Collaborator

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T02:00:33.556Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T02:00:33.556Z) — nothing to re-trigger. <!-- opencode-run:paused --> <!-- opencode-session:ses_f259eeba4ffeVS55EyA67WDNad -->
Grandiras force-pushed issue-129-atproto-gaps from 6f99cecb21
All checks were successful
CI / pds-integration (pull_request) Successful in 28s
CI / build-and-test (pull_request) Successful in 1m0s
to 09ca3709c6
All checks were successful
CI / pds-integration (pull_request) Successful in 28s
CI / build-and-test (pull_request) Successful in 1m2s
2026-09-25 21:03:14 +00:00
Compare
Author
Owner

@claude /review — rebased onto main after #162 (CHANGELOG-only conflict). Please read RecordProof.Verify in full.

@claude /review — rebased onto main after #162 (CHANGELOG-only conflict). Please read `RecordProof.Verify` in full.
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/605); 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-161/run-605.md) — updating every ~5 minutes while the task runs.
Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-161/run-2026-09-25T21-25-16-680Z.md) — updating every ~5 minutes while the review runs.
Collaborator

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T02:25:17.152Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T02:25:17.152Z) — nothing to re-trigger. <!-- opencode-run:paused --> <!-- opencode-session:ses_f258aa4e8ffe9b6xljBZlohbnl -->
Grandiras force-pushed issue-129-atproto-gaps from 09ca3709c6
All checks were successful
CI / pds-integration (pull_request) Successful in 28s
CI / build-and-test (pull_request) Successful in 1m2s
to 3bc47125a7
All checks were successful
CI / pds-integration (pull_request) Successful in 29s
CI / build-and-test (pull_request) Successful in 1m9s
Sync Closures to GitHub / sync-closure (pull_request) Successful in 10s
2026-09-25 23:58:25 +00:00
Compare
Author
Owner

@claude /review — please read RecordProof.Verify (the sync.getRecord proof verification) in full; it's the security-relevant part.

@claude /review — please read `RecordProof.Verify` (the `sync.getRecord` proof verification) in full; it's the security-relevant part.
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/607); 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-161/run-607.md) — updating every ~5 minutes while the task runs.
Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-161/run-2026-09-25T23-59-26-963Z.md) — updating every ~5 minutes while the review runs.
Collaborator

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T04:57:05.468Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T04:57:05.468Z) — nothing to re-trigger. <!-- opencode-run:paused --> <!-- opencode-session:ses_f24fd8435ffe9G84YO3DjluWSo -->
claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a large, carefully executed PR and I could not find correctness or security issues after a close read.

Things I specifically checked:

  • MST proof walk (RecordProof.FindInTree/ReadKeys): the layer-consistency checks (expected = layer - 1, throwing when it goes below 0), the strictly-increasing/in-range key checks, and the empty-node/Left-link handling for padding nodes all line up with how @atproto/repo's MST is structured, and the extensive hand-built-tree tests (out-of-order keys, mixed layers, child-on-parent's-layer, subtree-key-outside-range) exercise exactly the invariants the code enforces. The well-formed control test guards against the checks being over-eager.
  • Commit/signature verification: single-root requirement, DID match, version check, and did:key signature verification via FirehoseVerifier.ExtractSignedView all look right, and are covered by tampering tests (re-signed by another key, flipped signature byte, two roots, etc.).
  • ReadBoundedAsync: the declared-Content-Length short-circuit and the exponential-growth-capped-at-maxBytes+1 buffer correctly catch both a lying declared length and an actually-oversized chunked body, matching the two dedicated tests.
  • Lexicon fidelity: I checked upstream com.atproto.temp.checkHandleAvailability (union variant names #resultAvailable/#resultUnavailable, suggestion object fields) and com.atproto.sync.getRecord (params did/collection/rkey) against the current lexicon JSON on GitHub — the C# models and wire calls match. I also confirmed the reference PDS's getRecord handler defers entirely to @atproto/repo's getRecords, which is what the PR's own proof generator uses to produce the absence proof — so VerifiedRecord.Exists == false is a real, reachable server behavior, not just a synthetic test construct.
  • XrpcClient.UploadAsync/CreateUpload refactor: purely extracts shared request construction; the generic overload's behavior is unchanged.
  • New admin/temp/site.standard surfaces: thin, consistent wrappers over existing QueryAsync/ProcedureAsync/RepoClient patterns, matching the rest of the codebase's conventions (pagination via ICursorPage<T>, RecordPage<T> typed listings).

Docs (CHANGELOG, api-reference, architecture, low-level-repo, managed-pds, standard-site) are all updated consistently with the code, and the CHANGELOG bullets are appropriately categorized under Added. Test coverage is thorough (wire-shape tests, reference vectors from the actual @atproto/repo implementation, tampering tests, hand-built malformed trees, bounded-read edge cases).

No blocking issues found.


⏱ 1m 55s · 6 turns · tokens: 89.5k in / 9.9k out (+435.7k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, carefully executed PR and I could not find correctness or security issues after a close read. Things I specifically checked: - **MST proof walk (`RecordProof.FindInTree`/`ReadKeys`)**: the layer-consistency checks (`expected = layer - 1`, throwing when it goes below 0), the strictly-increasing/in-range key checks, and the empty-node/Left-link handling for padding nodes all line up with how `@atproto/repo`'s MST is structured, and the extensive hand-built-tree tests (out-of-order keys, mixed layers, child-on-parent's-layer, subtree-key-outside-range) exercise exactly the invariants the code enforces. The well-formed control test guards against the checks being over-eager. - **Commit/signature verification**: single-root requirement, DID match, version check, and `did:key` signature verification via `FirehoseVerifier.ExtractSignedView` all look right, and are covered by tampering tests (re-signed by another key, flipped signature byte, two roots, etc.). - **`ReadBoundedAsync`**: the declared-Content-Length short-circuit and the exponential-growth-capped-at-`maxBytes+1` buffer correctly catch both a lying declared length and an actually-oversized chunked body, matching the two dedicated tests. - **Lexicon fidelity**: I checked upstream `com.atproto.temp.checkHandleAvailability` (union variant names `#resultAvailable`/`#resultUnavailable`, `suggestion` object fields) and `com.atproto.sync.getRecord` (params `did`/`collection`/`rkey`) against the current lexicon JSON on GitHub — the C# models and wire calls match. I also confirmed the reference PDS's `getRecord` handler defers entirely to `@atproto/repo`'s `getRecords`, which is what the PR's own proof generator uses to produce the absence proof — so `VerifiedRecord.Exists == false` is a real, reachable server behavior, not just a synthetic test construct. - **`XrpcClient.UploadAsync`/`CreateUpload` refactor**: purely extracts shared request construction; the generic overload's behavior is unchanged. - **New admin/temp/site.standard surfaces**: thin, consistent wrappers over existing `QueryAsync`/`ProcedureAsync`/`RepoClient` patterns, matching the rest of the codebase's conventions (pagination via `ICursorPage<T>`, `RecordPage<T>` typed listings). Docs (CHANGELOG, api-reference, architecture, low-level-repo, managed-pds, standard-site) are all updated consistently with the code, and the CHANGELOG bullets are appropriately categorized under `Added`. Test coverage is thorough (wire-shape tests, reference vectors from the actual `@atproto/repo` implementation, tampering tests, hand-built malformed trees, bounded-read edge cases). No blocking issues found. --- ⏱ 1m 55s · 6 turns · tokens: 89.5k in / 9.9k out (+435.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-161/run-607.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:3bc47125a74bf3afc9b616be1d5574df2bc3bc6e -->
Grandiras deleted branch issue-129-atproto-gaps 2026-09-26 00:02:51 +00:00
Sign in to join this conversation.
No description provided.