feat(atproto): remaining com.atproto and site.standard gaps #161
No reviewers
Labels
No labels
breaking-change
bug
documentation
duplicate
enhancement
good first issue
help wanted
performance
question
wontfix
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Grandiras/ATProto.NET!161
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "issue-129-atproto-gaps"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #129.
Fills the remaining
com.atproto.*andsite.standard.*gaps. Everything is additive: no breaking changes.com.atproto.sync.getRecordandgetBlocksSyncClient.GetRecordAsync(did, collection, rkey)returns the proof CAR as anXrpcStreamResponse, 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 declaredContent-Lengthand 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 aVerifiedRecord:Uri,Commit(CID),Rev, andCid/Value/Exists.Exists == falsemeans the signed tree proves the record absent.CarReader.FromBytes(verifyBlockCids: true));did:key(splicing outsigviaFirehoseVerifier.ExtractSignedView);MST.getof@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;RepoVerificationException : AtProtoException. A malformeddid:keythrowsFormatException.SyncClient.GetBlocksAsync(did, cids)returns a CAR stream, withcidsrepeated per the XRPC array convention.com.atproto.repo.importRepoRepoClient.ImportRepoAsync(Stream car)is a CAR upload (application/vnd.ipld.car) over the existing upload path.XrpcClientgains a non-genericUploadAsyncfor procedures without output. It shares request construction with the generic one.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.adminAdminClient.SearchAccountsAsync(email?, limit?, cursor?)plusEnumerateSearchAccountsAsync.SearchAccountsResponseimplementsICursorPage<AccountInfo>.AdminClient.UpdateAccountSigningKeyAsync(did, signingKey), wheresigningKeyis aDidbecause the Lexicon's format isdid(adid:key). The request body is internal.PdsAdminClient.SearchAccountsAsync/EnumerateSearchAccountsAsyncgo through the usual admin-session handling. I checked the sources:searchAccounts; the reference PDS doesn't (the entryway does, and Ozone proxies it).updateAccountSigningKeyis entryway-only; neither PDS implements it.PdsAdminClient; the signing-key update stays reachable throughPdsAdminClient.Admin.com.atproto.tempNew
AtProtoClient.Temp(TempClient,Lexicon/Com/AtProto/Temp/{TempClient,TempModels}.cs):CheckHandleAvailabilityAsync(handle, email?, birthDate?): the result is an open union (HandleAvailable/HandleUnavailablewithHandleSuggestions /UnknownHandleAvailabilityResult), plus a convenienceIsAvailable.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) andaddReservedHandle(entryway-internal admin).site.standardRecommendRecord(site.standard.graph.recommend:document,createdAt) andStandardSiteClient.CreateRecommendationAsync/GetRecommendationAsync(by repo + rkey, or byAtUri) /DeleteRecommendationAsync/ListRecommendationsAsync/EnumerateRecommendationsAsync, built like the subscription methods.contributors,labels,links,createdAt);RecordPage<T>viaRecordCollection<T>.ToPage).Verification
--no-incremental): 0 warnings, 0 errors.tests/ATProtoNet.Tests/Repo/TestData/record-proof.jsonholds a presence proof and an absence proof produced by@atproto/repo0.10.14 /@atproto/crypto0.5.5.verifyRecords/verifyProofsaccept both proofs before the file is written.gen-record-proof.mjs) and a README sit next to it; the output is deterministic.RecordProofTests(23 tests):$bytes, CIDs, commit, rev) and the reference absence proof;did:key;SyncClientTests,ComAtProtoGapTests,PdsAdminClient*Tests,StandardSiteClientTests;DefOf[HandleSuggestion] = com.atproto.temp.checkHandleAvailability#suggestion.RepositoryTests.GetVerifiedRecord_ProvesACreatedRecordAndThenItsDeletionchecks the proof against a live PDS, taking the key fromgetRecommendedDidCredentials.Notes
dereferenceScopeas expandinginclude:scopes. Per the Lexicon and the PDS (ScopeReferenceGetter), it expandsref:scope references; the docs say so.ca43404(#158, session rework). The conflicts were CHANGELOG anddocs/architecture.mdonly.AtProtoClient.cs: oneusing, one constructor line, one property;Http/XrpcClient.cs: the non-genericUploadAsync, sharingCreateUpload;MerkleSearchTree.MaxTreeDepth:private→internal.🤖 Generated with Claude Code
@claude /review — please read
RecordProof.Verify(thesync.getRecordproof verification) in full; it's the security-relevant part.🤖 Reviewing this PR — the review will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the review runs.
🤖 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.
⏳ 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.
🤖 Resolving the merge conflicts — the reply will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the fix runs.
⏳ 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.
6f99cecb2109ca3709c6@claude /review — rebased onto main after #162 (CHANGELOG-only conflict). Please read
RecordProof.Verifyin full.🤖 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 — the review will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the review runs.
⏳ 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.
09ca3709c63bc47125a7@claude /review — please read
RecordProof.Verify(thesync.getRecordproof verification) in full; it's the security-relevant part.🤖 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 — the review will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the review runs.
⏳ 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.
✅ 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:
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.did:keysignature verification viaFirehoseVerifier.ExtractSignedViewall 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+1buffer correctly catch both a lying declared length and an actually-oversized chunked body, matching the two dedicated tests.com.atproto.temp.checkHandleAvailability(union variant names#resultAvailable/#resultUnavailable,suggestionobject fields) andcom.atproto.sync.getRecord(paramsdid/collection/rkey) against the current lexicon JSON on GitHub — the C# models and wire calls match. I also confirmed the reference PDS'sgetRecordhandler defers entirely to@atproto/repo'sgetRecords, which is what the PR's own proof generator uses to produce the absence proof — soVerifiedRecord.Exists == falseis a real, reachable server behavior, not just a synthetic test construct.XrpcClient.UploadAsync/CreateUploadrefactor: purely extracts shared request construction; the generic overload's behavior is unchanged.QueryAsync/ProcedureAsync/RepoClientpatterns, matching the rest of the codebase's conventions (pagination viaICursorPage<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/repoimplementation, 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