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

grain.social/grain

photography-focused photo sharing platform

grain.social
1 collaborator
0
2

Galleries and accounts are deleted, and a deleted account only once its host agrees

Closed
Closed chadtmiller.com asked to merge into feature/rust from grain.social/grain:gaps/deletes

What changes

Deleting a gallery. social.grain.gallery.deleteGallery (crates/grain-serve/src/gallery.rs) deletes one of the logged-in account's galleries with what hangs off it: its items, the photos they place that no other of the account's galleries does, those photos' EXIF, and the account's own favorite of it and comments on it. They go children first, by com.atproto.repo.applyWrites at the account's PDS with the web session (at most 200 a call, as the PDS takes), and from the index as each call is answered. What others wrote about it stays, theirs. It is the web's alone, by its cookie from a page of the site, as social.grain.repo.* are; a service token is refused (403), as the apps delete at their own PDS and call indexRecords. A gallery the account does not have is NotFound.

Deleting an account's grain data. social.grain.server.deleteAccount (crates/grain-serve/src/accounts.rs) now does what knotless's dev.knotless.deleteAccountData does (knotless-serve/src/web/account_data.rs, knotless/src/account.rs), its lexicon in the same shape: input {did, limit} (the account named again; 1 to 1000, 400 unless said), output {deleted, done}, errors AuthenticationRequired, Forbidden, RecordsRemain.

  • By the web's cookie: the records in every social.grain.* collection the PDS's describeRepo lists go, by listRecords and applyWrites, up to limit a call, what hangs off a gallery first and the profile last. Once none is left, the browser is logged out of the account (the session revoked at the PDS, the cookies set to the next account it holds, as /oauth/logout does) and the account forgotten.
  • By a service token: nothing is deleted; the account is forgotten once its PDS lists no social.grain.* collection, else RecordsRemain.
  • Forgetting ends its web session (login::end_session, what /oauth/logout does, taken out of it: revoked at its authorization server, then dropped, with the space credentials it was reading with, spaces.log_out, as logging out does), then Index::purge, and the account no longer tracked (Index::untrack, its repo row and handle gone until it writes again).

What a purge takes. Index::purge, which a deletion at the host runs too, now also takes what grain keeps for the account (index::KEPT_FOR): its web_session, preference, notification_seen, notification_pref, push_token, its own mutes, story_views and space_invites. It keeps what an admin holds of it (takedown, label, report, classification) and others' mutes of it, which are theirs, as knotless keeps takedowns, labels and reports.

An account deleted at its host. Jetstream saying an account was deleted no longer purges it outright. As knotless-serve's accounts::deleted does, the host the account's identity names now is asked first (com.atproto.sync.getRepoStatus): one it answers for as active moved there, and stays; one whose identity does not resolve is taken at the event's word (accounts::gone, accounts::deletions, fed by jetstream::follow given a Ctx to ask with, as Settings::follow gives it). Given none, as most of the stream's tests are, and in jetstream::apply, the event is taken at its word as before.

The import. _mutes and _story_views are carried by grain-import already, row for row into mute and story_view, with _preferences, _push_tokens and _space_invites, and tested there (crates/grain-import/src/tests.rs); nothing here changes the old layout's reading.

The web. routes/settings/account calls deleteAccount with {did} until it is done, showing how many records have gone, and no longer calls /oauth/logout itself, as the last call has logged the browser out; then it goes home with the session read again.

This head holds feature/rust at 92c202b (the batch, the infra stack and spaces); deleteGallery's rkey keeps the lexicon lint's maxLength beside format: record-key, and harness/lexicons.mjs passes here.

One of the pulls closing what the clients call and grain-serve did not serve; the table of them all is in gaps/graph's body (Accounts mute and unmute, and list whom they block and mute).

Why

The web and both apps call both, and grain-serve answered 501. A purge that left sessions, push tokens and preferences behind kept a deleted account's devices registered and its session usable; and a deleted event from an account's old host, once it had moved, deleted everything grain held of an account that is alive elsewhere.

Differences from the old server

  • deleteAccount takes {did, limit} and answers {deleted, done}, where the old one took nothing and answered {} after deleting everything in one call. The web names its DID and calls again until done.
  • It deletes every social.grain.* collection the PDS lists, the group records (social.grain.group.*) among them, where the old server deleted a fixed list of ten that left those. As before, a Bluesky cross-post and other apps' records stay.
  • The old server deleted each record one call at a time and carried on past a failure; here a failure answers (502 for the PDS's), and the next call carries on from what is left.
  • Forgetting also takes notification state, mutes, story views and space invitations, and the account's repo row; the old server cleared mutes, preferences, push tokens and OAuth sessions only, and left its records in the index for the firehose to remove.
  • deleteGallery deletes only the account's own favorite and comments on the gallery. The old one asked its own repo to delete every favorite and comment on it by their record keys, whoever made them, which at best failed and at worst deleted an unrelated record of the account's that shared a key.
  • It keeps a photo another of the account's galleries also places; the old one deleted every photo the gallery placed.
  • A missing gallery is 400 NotFound, where the old server threw a 500.

How it was checked

crates/grain-serve/tests/deletes.rs, against the reference PDS: a gallery deleted with its items, its own photo and EXIF, the account's favorite and comment, and a shared photo, another gallery, and another account's comment kept, at the PDS and in the index; deleting it again, another's gallery, a bad key, a service token, no origin and no cookie refused. The web deleting an account two records at a time and then the rest, its cookies cleared, its PDS listing no grain collection, the index forgetting it and its mutes; another DID and a limit of 0 refused; the cookie naming no one after. An app refused with RecordsRemain while a record is left, then forgotten once it deletes it. accounts::gone for an account its host has and for a DID that resolves to nothing, and deletions keeping the first and purging the second. Unit tests: what a purge takes and leaves, untrack, and apply_with sending a deletion to be asked after. The plan check covers what a gallery's deletion reads. The web's page is not tried in a browser. The rest is the runner's.

Shortcomings

  • The apps, once they log in at their own PDS, delete their records there and then ask with a service token; until they move, they call the old method.
  • A host that cannot be reached when asked about a deletion counts as not answering for the account, so it is purged, as knotless does.
  • space_invite rows the account made are found without an index; a purge is rare.
  • space_support stays: it is kept per PDS (whether that host serves spaces), not per account.
  • An account deleted at its host loses its rows, web_session among them, but space credentials already in memory are not dropped then; they lapse on their own (spaces::REPROVE).

🤖 Generated with Claude Code

Approved an earlier version

This pull adds deleteGallery, makes deleteAccount delete in steps and forget the account, makes a purge also take the account's session, preference and notification state, and has a Jetstream deletion ask the account's current host first.

I found nothing that must change. Two limits on what I read:

  • The full diff was too large for me to read. I read gallery.rs and accounts.rs at the head commit, and I did not look at the Index::purge/untrack changes, the jetstream::follow_checked wiring or the lexicons.
  • All four checks (check, rust, test, harness) were still queued, so none has passed yet. The coverage gate and the lexicon-answer check run there.

What I checked in the code I read:

  • deleteGallery:
    • The query returns EXIF, then items, then the exclusive photos, then the account's favorite and comments, then the gallery. A failure part-way leaves a gallery with fewer photos, not photos with no gallery.
    • Retrying after a partial failure is safe, because the index drops each record as its call is answered.
    • A service token is refused, and a missing gallery gives NotFound.
  • deleteAccount:
    • The DID in the input must match the caller's.
    • The limit range is checked.
    • The service-token path deletes nothing and forgets the account only once the PDS lists no social.grain.* collection.
    • The web path logs the browser out before forgetting the account.

One small thing: limit is checked at the top of each page, so a call can delete up to PAGE - 1 more than limit records. That is harmless, since limit is a soft cap.

The web's deleteAccount caller must be changed in the web pull, as the body says.

Approved an earlier version

Carried forward: the change this pull makes at 270ad01daff8bdae7dbd2411382f88a76620eda3 is the one reviewed at a7a8ce7b46b1d8bf81d64eb4ce3c4dfc038ce414, so that review stands. What is new comes from its base.

The overshoot past limit: fixed at da30f44, each listRecords asks for no more than the call has left, and the two-step test now deletes one of a collection's two items with limit: 1. The parts the review did not read (Index::purge/untrack, follow_checked, the lexicons) are in the same head.

Approved an earlier version

The overshoot past limit that my earlier review noted is fixed at da30f44. delete_some now asks listRecords for (budget - step.deleted).min(PAGE) records, and the loop returns before listing once deleted >= budget. The subtraction can't underflow, and a call that stops exactly at limit reports done: false, so the next call finishes it. The two-step test now has two items in one collection and deletes one with limit: 1, then expects deleted: 5 on the rest, which covers the fix.

Nothing else changed in this round, so the earlier review stands. The check, rust, test and harness workflows were all still queued when I looked, so none has passed yet. The deleteAccount caller in the web pull still has to change, as the body says.

Approved an earlier version

This round only changes crates/grain-serve/tests/deletes.rs, and the change is sound. The photo fixture now includes alt, which the published lexicon takes. The service-token test now builds its own App with a known did:web:grain.test audience and its own index, so the token is for a DID the PDS signs for.

The production code is unchanged from the version I approved earlier, and my earlier approvals still stand. The check, rust, test and harness workflows were all still queued when I looked, so none has passed yet. The web caller of deleteAccount still has to change in the web pull, as the body says.

Approved an earlier version

This round only changes web/src/routes/settings/account/+page.svelte. It is the web caller change that earlier rounds said was still needed. handleDelete now calls deleteAccount with {did} in a loop until done, and the button shows a running count. It drops the client-side logout(), since the last call now logs the browser out, and it navigates with invalidateAll so the page drops the old session. limit is optional and defaults to 400, so omitting it is fine. A PDS failure surfaces as an error, which stops the loop and re-enables the button. A retry carries on from what is left.

The server code is unchanged, so my earlier approvals stand. The check, rust, test and harness workflows were all still queued when I looked, so none has passed yet.

Approved an earlier version

This round only merges batch/one into the branch, as the body says. The only commit of its own is the merge commit. My earlier approvals still stand. I did not re-read the merge result, so I can't say the merge is clean beyond what the body claims, and I did not look at the checks this round.

Landed on feature/rust at 7d031dc through the batch pull The web proxy, deletes and the cross-post test land together, checked once, checked and reviewed there as a whole. That pull also carries the fix to accounts::gone: an account is purged only on RepoNotFound, status deleted, or a 404 or 410 from PLC. An undoable inactive status is kept, and a lookup that fails is retried, then kept.