grain-serve writes for the web and indexes it before answering
MergedThis adds the web-only social.grain.repo.* write methods. They write to the PDS with the held OAuth session and index the result before answering. I read the head of write.rs and the lib.rs routing. I did not read the full stacked diff (about 3,500 lines, mostly the web/login base), the login::refreshed refactor, or the new tests.
- Validation: records are checked before anything is sent, and the PDS's URI is checked against the account's repo and collection before indexing.
- Retry and scope handling: one refresh and one retry on a token refusal, then the session ends. The scope split between 401 (session ended) and 403 (session kept) matches the description.
- Upload limit: the route for
uploadBlobhas its ownDefaultBodyLimit::max(MOST_BYTES), so the 50 MiB cap works against axum's smaller default. - Partial
applyWritesfailure: if a later result fails thewrittencheck, the PDS has already applied the batch and the earlier results are indexed. The answer is a 502 and Jetstream reconciles the rest later. This is acceptable as described, and I would only note it in the doc comment. - Checks: all four checks (
check,rust,harness,test) were still queued when I looked. The PR says these tests have never run, so the coverage gate and the answers check against the lexicons are unverified. Please read them once they finish.
I have no blocking findings.
The only change since my last review is the head commit, b5b7ffc. By its message it makes the stale-swap test use a record that actually changes, and adds tests for index failures after a write. That touches tests only, so my earlier reading of write.rs still holds and I have no new findings. I did not read the new test code itself.
All four checks (check, rust, harness, test) are still queued at this head. The new tests have not run, so please read the results, including the coverage gate, once they finish.
Carried forward: the change this pull makes at 411c4de15c4e2c2c9241f5dd8f731d8fff9e7d17 is the one reviewed at b5b7ffc0e481a0c6fd30a65f78d61837e47270dd, so that review stands. What is new comes from its base.
Since my last review the head moved by two commits of its own, plus a merge of feature/rust. By their messages, a636a21 holds PDS answers to their lexicons, and 06d3fa8 documents what applyWrites leaves indexed when a later result fails. The second is the doc note I suggested. I did not read the diff of the answer-validation change or the merge, so I have no findings on them.
At this head check passed. rust and harness are still running, and test is still queued. The new write.rs tests and the coverage gate have not finished, so please read them when they do. I have no blocking findings.
Since my last review the head moved by a merge of web/login. That merge brings in 8042129, which signs the session cookie with its own key (web-cookie), kept out of the JWKS. write.rs is unchanged, so my earlier reading of it still holds. I did not read the cookie-key change itself. It belongs to the base pull, and write.rs only depends on the cookie being checked.
All four checks (check, rust, harness, test) are queued at this head. None has run, so the write tests and the coverage gate are still unverified. Please read the results once they finish. I have no blocking findings.
- grain-serve writes for the web and indexes it before answering
- main
What changes
The web's writes.
social.grain.repo.createRecord,putRecord,deleteRecord,applyWritesanduploadBlob(new lexicons underlexicons/social/grain/repo/, plusdefs#written) write to the logged-in account's PDS with the OAuth session grain-serve holds for it (com.atproto.repo.*, DPoP-bound), and put the write in the index before they answer, so a read straight after sees it. Each takes what thecom.atproto.repomethod of its name takes, lessrepo, which is the session's. Answers are{uri, cid},{},{results}(eachsocial.grain.repo.applyWrites#…Resultin its write's place) and{blob}.Who may call them. Only the web, by its cookie, from a page of the site (
viewer::writer'sOrigincheck). A request carryingAuthorizationis refused 403: the apps and the CLI keep writing to their own PDS and callingsocial.grain.sync.indexRecords, unchanged.What is written. The collection must be one the index keeps (else
UnknownCollection); a record with no$typeis given the collection's; the record must be what the index would take (Index::check, factored out ofIndex::write, so the same validation), elseInvalidRecord, all before anything is sent. A delete's or update's rkey must make a record URI.applyWritestakes 1 to 200 writes and checks them all first.What is indexed. The record as sent, under the URI and CID the PDS answers, after
track(did). A PDS answer whose URI is not in the account's repo and collection, or that lacks a CID, is 502 and nothing is indexed (a hostile PDS cannot plant records under another DID). An index failure after a successful write is logged and the write still answers 200; Jetstream brings it later.Sessions at the PDS.
login::refreshed(app, did, refused_token)(withsessionnow one function,fresh) refreshes under the same per-account lock, unless whoever held the lock already replaced that token. A PDS refusing the access token (InvalidToken,ExpiredToken,invalid_token) gets one refresh and one retry; refused again, the session ends (401AuthenticationRequired). Refused for want of scope (grain::oauth::out_of_scope): if the session was asked for less thangrain::oauth::SCOPE, it ends (401: logging in again fixes it); if it was asked for all of it, 403ScopeMissingErrorand the session stays (the PDS withheld it; logging in again would not help). DPoP nonce retries are the existingXrpc::send's.PDS refusals. A 4xx other than 401 keeps its status; its error name passes when it is one of a fixed list (
InvalidSwap,InvalidRecord,BlobTooLarge,InvalidMimeType,RateLimitExceeded,PayloadTooLarge, account/repo status names), elseInvalidRequest. Anything else is 502UpstreamFailure.uploadBlobtakes up to 50 MiB (write::MOST_BYTES) and requiresContent-Type.lexicons/README.md's mapping row now says the web usessocial.grain.repo.*and the appscom.atproto.repo.*+indexRecords.hatk.generated.ts/.client.tsregenerated withnpx hatk generate types.Why
The web can't write to the PDS itself: the session, and the DPoP key, are server-side by design. hatk's
dev.hatk.*record methods were generic, and the web calls them for twelve collections from a dozen components (follows, favorites, comments, galleries and their items, stories, blocks, group submissions/items/declines, the Bluesky cross-post), so a generic proxy under grain's name is a rename at cutover. Domain procedures per action (knotless'sstarRepostyle) would be twenty-odd methods to design before cutover. They can come later, one at a time, where a write needs server-side logic.How it differs from hatk's
pds-proxy.ts:repoinput; swap fields pass through;$typedefault;Failureerrors are static;ScopeMissingError), using the scope the session was asked for. hatk compared granted against requested scope counts and named the secondScopeWithheld;How it was checked
crates/grain-serve/tests/write.rsruns against the reference PDS, logged in through the sharedtests/web/mod.rs, which istests/login.rs's site and browser moved out so that both use them:$type) → on the PDS and in the index with the PDS's CID; put withswapRecord→ new CID indexed; a stale swap → 400InvalidSwapfrom the PDS;applyWritescreate+update+delete, results typed and in place, index and PDS agree;uploadBlobof a PNG, and with noContent-Type;Origin, a bearer token and no cookie;applyWritesresult count mismatch, an upload answer withoutblob,ScopeMissingErrorwith a full session (403, kept) and with a session asked for less (401, ended), and a fresh token refused again (401, ended).Unit tests cover the refusal mapping, the URI checks and an index failing after a write. All four checks pass at 06d3fa8, the coverage gate included (99.20% of lines, 98.88% of functions).
harness/answers.mjsnow reads each answer with@atproto/lexicon'sjsonToLexbefore validating it, as atproto reads JSON. Without that, a blob in JSON form (uploadBlob's answer) fails theblobtype check.Shortcomings
social.grain.gallery.deleteGallery's cascade); the web stays ondev.hatk.*until cutover moves it.uploadBlobbuffers the body (50 MiB cap) rather than streaming it.Cache-Control/Varywork here; writes aren't cached.applyWritesbatch. Each put or delete is applied on its own, as Jetstream would apply them.Review
crates/grain-serve/src/write.rs(call's retry and scope handling,written's URI check,apply_writes), thefreshrefactor inlogin.rs, and the lexicons. Stacked onweb/login(pull 3mz7oyx7oiu3c); its own diff is the last commit.🤖 Generated with Claude Code