~ knotless Search/
↑↓ move↵ open⇧↵ all results

grain.social/grain

photography-focused photo sharing platform

grain.social
1 collaborator
0
2

grain-serve writes for the web and indexes it before answering

Merged
Merged chadtmiller.com asked to merge into feature/rust from grain.social/grain:web/writes

What changes

The web's writes. social.grain.repo.createRecord, putRecord, deleteRecord, applyWrites and uploadBlob (new lexicons under lexicons/social/grain/repo/, plus defs#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 the com.atproto.repo method of its name takes, less repo, which is the session's. Answers are {uri, cid}, {}, {results} (each social.grain.repo.applyWrites#…Result in its write's place) and {blob}.

Who may call them. Only the web, by its cookie, from a page of the site (viewer::writer's Origin check). A request carrying Authorization is refused 403: the apps and the CLI keep writing to their own PDS and calling social.grain.sync.indexRecords, unchanged.

What is written. The collection must be one the index keeps (else UnknownCollection); a record with no $type is given the collection's; the record must be what the index would take (Index::check, factored out of Index::write, so the same validation), else InvalidRecord, all before anything is sent. A delete's or update's rkey must make a record URI. applyWrites takes 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) (with session now 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 (401 AuthenticationRequired). Refused for want of scope (grain::oauth::out_of_scope): if the session was asked for less than grain::oauth::SCOPE, it ends (401: logging in again fixes it); if it was asked for all of it, 403 ScopeMissingError and the session stays (the PDS withheld it; logging in again would not help). DPoP nonce retries are the existing Xrpc::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), else InvalidRequest. Anything else is 502 UpstreamFailure. uploadBlob takes up to 50 MiB (write::MOST_BYTES) and requires Content-Type.

lexicons/README.md's mapping row now says the web uses social.grain.repo.* and the apps com.atproto.repo.* + indexRecords. hatk.generated.ts / .client.ts regenerated with npx 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's starRepo style) 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:

  • no repo input; swap fields pass through;
  • service tokens refused instead of served;
  • the PDS's URI is checked against the account's repo before indexing;
  • the $type default;
  • validation against the index's own lexicons (unknown collections refused, where hatk validated any lexicon it had);
  • PDS error names pass only from an allowlist, since Failure errors are static;
  • a scope refusal maps to "behind" (ends the session) or "withheld" (403 ScopeMissingError), using the scope the session was asked for. hatk compared granted against requested scope counts and named the second ScopeWithheld;
  • no label rules run on write (grain-serve has none yet).

How it was checked

crates/grain-serve/tests/write.rs runs against the reference PDS, logged in through the shared tests/web/mod.rs, which is tests/login.rs's site and browser moved out so that both use them:

  • create (no $type) → on the PDS and in the index with the PDS's CID; put with swapRecord → new CID indexed; a stale swap → 400 InvalidSwap from the PDS;
  • applyWrites create+update+delete, results typed and in place, index and PDS agree;
  • delete; uploadBlob of a PNG, and with no Content-Type;
  • every pre-write refusal, plus no Origin, a bearer token and no cookie;
  • an access token the PDS refuses is refreshed and the write succeeds;
  • against a stand-in PDS: a URI in another repo and an empty answer (502, nothing indexed), a 500, a 429 passed through, an applyWrites result count mismatch, an upload answer without blob, ScopeMissingError with 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.mjs now reads each answer with @atproto/lexicon's jsonToLex before validating it, as atproto reads JSON. Without that, a blob in JSON form (uploadBlob's answer) fails the blob type check.

Shortcomings

  • No domain procedures (e.g. social.grain.gallery.deleteGallery's cascade); the web stays on dev.hatk.* until cutover moves it.
  • uploadBlob buffers the body (50 MiB cap) rather than streaming it.
  • No Cache-Control/Vary work here; writes aren't cached.
  • The index write isn't in a transaction across an applyWrites batch. 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), the fresh refactor in login.rs, and the lexicons. Stacked on web/login (pull 3mz7oyx7oiu3c); its own diff is the last commit.

🤖 Generated with Claude Code

Approved

This 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 uploadBlob has its own DefaultBodyLimit::max(MOST_BYTES), so the 50 MiB cap works against axum's smaller default.
  • Partial applyWrites failure: if a later result fails the written check, 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.

Approved

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.

Approved

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.

Approved

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.

Approved

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.

Merged as 9aa342e
Part of a stack of 2
  1. grain-serve writes for the web and indexes it before answering Landed as 9aa342e
  2. main