{"item":{"uri":"at://did:plc:bcgltzqazw5tb6k2g3ttenbj/dev.knotless.pull/3mza7ky6ut5on","kind":"pull","title":"Every query answer says it varies by who asks, as knotless's do","state":"closed","author":{"did":"did:plc:bcgltzqazw5tb6k2g3ttenbj","handle":"chadtmiller.com","avatar":"https://cdn.bsky.app/img/avatar/plain/did:plc:bcgltzqazw5tb6k2g3ttenbj/bafkreig5bcb7dl7aphw6eg4e7kjkvpdshfexdvk7jldg53ypre6ecdhesu@jpeg"},"createdAt":"2026-11-01T06:46:00.528Z","comments":3},"body":"## What changes\n\n**Every query answer says it varies by who asks.** grain-serve's queries (`GET /xrpc/*`) answer `Vary: authorization, cookie`, always. One that found someone asking, by a service token or the web's session cookie, is `Cache-Control: private, no-cache`; one that read what is the asker's alone, `private, no-store`: an account's preferences (`social.grain.actor.getPreferences`) and everything an admin reads (`listReports`, `getReport`, `getSubjectStatus`, `listLabelDefinitions`, `getReviewSubject`). An answer asked by no one, or by a token that does not hold, has no `Cache-Control`. Procedures and the routes off `/xrpc/` are as they were.\n\n`viewer::answers` is one layer over the whole router, so a query added later is covered without naming it. `viewer::viewer` keeps whom it found in a task-local the layer reads, and `viewer::unshared()` marks an answer for no cache, as knotless's `private::touched` does.\n\n## Why\n\nThe rewrite copies knotless wherever knotless already does the thing. knotless's queries answer with these headers (`docs/reference/api.md`, \"Who asks\"), and grain's need them for the same reason: the same URL answers differently for each asker, so a shared cache in front, or the browser's, must not hand one asker's answer to another.\n\nMirrors knotless's `crates/knotless-serve/src/viewer.rs` (`viewer`, `answered`, `private`, `vary`) and `crates/knotless-serve/src/private.rs` (`touched`, `unshared`). It differs from knotless in two grain-specific ways. grain finds the asker inside each handler (`viewer::viewer`) rather than in a layer over a `queries` router, so the layer learns of the asker from what the handler found. And it covers every `GET /xrpc/*` route, not one sub-router, so that the parallel pulls adding queries need not touch it. Where knotless marks private repositories unshared, grain marks preferences and admin reads; the private galleries pull should call `viewer::unshared()` where it reads a space.\n\n## How it was checked\n\nTests through `tests/common` (a new `get_answered` returns the headers too):\n\n- `read.rs`: `getProfile` with a good token is `private, no-cache`, and with none or a bad one has no `Cache-Control`. All three vary.\n- `login.rs`: `getUnreadCount` with the session cookie is `private, no-cache`.\n- `notifications.rs`: `getPreferences` is `private, no-store`.\n- `moderation.rs`: an admin's `listLabelDefinitions` is `private, no-store`.\n- `health.rs`: `_health` varies with no `Cache-Control`, and `/oauth/session` has neither.\n\nThe fmt, clippy, harness and coverage runs are left to CI.\n\n## Shortcomings\n\n- `/_status` does not exist here yet. When it lands (the image, health and rollout pull) it should answer `no-store` as knotless's does.\n- No CORS on the queries, as knotless has. grain-serve has no `tower-http`, and that change is separate.\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)","html":"

What changes

Every query answer says it varies by who asks. grain-serve's queries (GET /xrpc/*) answer Vary: authorization, cookie, always. One that found someone asking, by a service token or the web's session cookie, is Cache-Control: private, no-cache; one that read what is the asker's alone, private, no-store: an account's preferences (social.grain.actor.getPreferences) and everything an admin reads (listReports, getReport, getSubjectStatus, listLabelDefinitions, getReviewSubject). An answer asked by no one, or by a token that does not hold, has no Cache-Control. Procedures and the routes off /xrpc/ are as they were.

viewer::answers is one layer over the whole router, so a query added later is covered without naming it. viewer::viewer keeps whom it found in a task-local the layer reads, and viewer::unshared() marks an answer for no cache, as knotless's private::touched does.

Why

The rewrite copies knotless wherever knotless already does the thing. knotless's queries answer with these headers (docs/reference/api.md, \"Who asks\"), and grain's need them for the same reason: the same URL answers differently for each asker, so a shared cache in front, or the browser's, must not hand one asker's answer to another.

Mirrors knotless's crates/knotless-serve/src/viewer.rs (viewer, answered, private, vary) and crates/knotless-serve/src/private.rs (touched, unshared). It differs from knotless in two grain-specific ways. grain finds the asker inside each handler (viewer::viewer) rather than in a layer over a queries router, so the layer learns of the asker from what the handler found. And it covers every GET /xrpc/* route, not one sub-router, so that the parallel pulls adding queries need not touch it. Where knotless marks private repositories unshared, grain marks preferences and admin reads; the private galleries pull should call viewer::unshared() where it reads a space.

How it was checked

Tests through tests/common (a new get_answered returns the headers too):

read.rs: getProfile with a good token is private, no-cache, and with none or a bad one has no Cache-Control. All three vary.
login.rs: getUnreadCount with the session cookie is private, no-cache.
notifications.rs: getPreferences is private, no-store.
moderation.rs: an admin's listLabelDefinitions is private, no-store.
health.rs: _health varies with no Cache-Control, and /oauth/session has neither.

The fmt, clippy, harness and coverage runs are left to CI.

Shortcomings

/_status does not exist here yet. When it lands (the image, health and rollout pull) it should answer no-store as knotless's does.
No CORS on the queries, as knotless has. grain-serve has no tower-http, and that change is separate.

🤖 Generated with Claude Code

\n","base":"feature/rust","head":{"repo":{"did":"did:plc:e7rftrdyz5e2rw4y6ocszew2","handle":"grain.social","name":"grain"},"branch":"infra/headers"},"comments":[{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mza7m4awz62b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-01T06:46:38.345Z","body":"This adds one layer over the router. It marks every `GET /xrpc/*` answer `Vary: authorization, cookie`. It adds `private, no-cache` when the handler found an asker, and `private, no-store` when the handler called `viewer::unshared()`. It calls `unshared()` for preferences and for any successful admin check. I read the diff only and did not run the tests or look at CI.\n\nThe design holds up. The layer is outermost, and a handler runs inside the same future as `next.run`, so the task-local is visible to it. Procedures and non-`/xrpc/` routes pass through untouched. The tests cover the token, cookie, bad-token, no-store and non-query cases.\n\nOne point to keep in mind, not a blocker: the layer relies on a handler not spawning a separate task before it calls `viewer` or `unshared`. If it did, `try_with` would silently skip the marking. Today's handlers call them inline.","html":"

This adds one layer over the router. It marks every GET /xrpc/* answer Vary: authorization, cookie. It adds private, no-cache when the handler found an asker, and private, no-store when the handler called viewer::unshared(). It calls unshared() for preferences and for any successful admin check. I read the diff only and did not run the tests or look at CI.

The design holds up. The layer is outermost, and a handler runs inside the same future as next.run, so the task-local is visible to it. Procedures and non-/xrpc/ routes pass through untouched. The tests cover the token, cookie, bad-token, no-store and non-query cases.

One point to keep in mind, not a blocker: the layer relies on a handler not spawning a separate task before it calls viewer or unshared. If it did, try_with would silently skip the marking. Today's handlers call them inline.

\n","verdict":"approve","commit":"ff6be0dd623abf070c85b534687b447c2d32ada0"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mza7q5kejw2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-01T06:48:53.920Z","body":"Round 2 adds one commit on top of what I approved. It rewrites `cached` in `viewer.rs` to compute the `Cache-Control` value first and then set all headers in one pass. The test in `notifications.rs` now passes its bearer token as `&str`. The behaviour is unchanged: no asker and not unshared gives no `Cache-Control`, an asker gives `private, no-cache`, and unshared gives `private, no-store`. `Vary` is still always appended. Nothing from the earlier review needed fixing. I read the diff only and did not run the tests.","html":"

Round 2 adds one commit on top of what I approved. It rewrites cached in viewer.rs to compute the Cache-Control value first and then set all headers in one pass. The test in notifications.rs now passes its bearer token as &str. The behaviour is unchanged: no asker and not unshared gives no Cache-Control, an asker gives private, no-cache, and unshared gives private, no-store. Vary is still always appended. Nothing from the earlier review needed fixing. I read the diff only and did not run the tests.

\n","verdict":"approve","commit":"839bd503b1709dbfcf27ef565438d523c742c40b"},{"uri":"at://did:plc:bcgltzqazw5tb6k2g3ttenbj/dev.knotless.comment/3mzafjkostm2b","author":{"did":"did:plc:bcgltzqazw5tb6k2g3ttenbj","handle":"chadtmiller.com","avatar":"https://cdn.bsky.app/img/avatar/plain/did:plc:bcgltzqazw5tb6k2g3ttenbj/bafkreig5bcb7dl7aphw6eg4e7kjkvpdshfexdvk7jldg53ypre6ecdhesu@jpeg"},"createdAt":"2026-11-01T08:32:35.267Z","body":"Landed on feature/rust at fc5f686 through the batch pull at://did:plc:bcgltzqazw5tb6k2g3ttenbj/dev.knotless.pull/3mzadedttgi2b, checked there as a whole. This pull's head is in feature/rust's history.","html":"

Landed on feature/rust at fc5f686 through the batch pull Seven reviewed pulls land on feature/rust together, checked once, checked there as a whole. This pull's head is in feature/rust's history.

\n"}],"reviews":[{"reviewer":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"verdict":"approve","uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mza7q5kejw2b","commit":"839bd503b1709dbfcf27ef565438d523c742c40b","outdated":false,"maintainer":false}],"closes":[],"checks":[],"minutes":[{"runner":{"did":"did:plc:4bocpkn5zkwe4wuu4plrj5kt","handle":"runner.knotless.dev","bot":true},"minutes":2505,"freeing":[{"at":"2026-11-25T00:00:00Z","minutes":2},{"at":"2026-11-27T00:00:00Z","minutes":24},{"at":"2026-11-28T00:00:00Z","minutes":10},{"at":"2026-11-29T00:00:00Z","minutes":7},{"at":"2026-11-30T00:00:00Z","minutes":310},{"at":"2026-12-01T00:00:00Z","minutes":1355},{"at":"2026-12-02T00:00:00Z","minutes":509},{"at":"2026-12-06T00:00:00Z","minutes":43},{"at":"2026-12-07T00:00:00Z","minutes":185},{"at":"2026-12-08T00:00:00Z","minutes":16},{"at":"2026-12-09T00:00:00Z","minutes":38}],"updatedAt":"2026-11-10T01:31:15.298Z"},{"runner":{"did":"did:plc:ejh2v3z5ieqhqgz3ybk6r35x","handle":"ci.knotless.dev","bot":true},"minutes":28,"freeing":[{"at":"2026-11-29T00:00:00Z","minutes":2},{"at":"2026-11-30T00:00:00Z","minutes":7},{"at":"2026-12-01T00:00:00Z","minutes":1},{"at":"2026-12-06T00:00:00Z","minutes":6},{"at":"2026-12-07T00:00:00Z","minutes":8}],"updatedAt":"2026-11-08T00:05:20.251Z"}],"changes":{"repo":{"did":"did:plc:e7rftrdyz5e2rw4y6ocszew2","handle":"grain.social","name":"grain"},"commits":[],"files":[],"head":"839bd503b1709dbfcf27ef565438d523c742c40b"},"card":"/grain.social/grain/pull/did:plc:bcgltzqazw5tb6k2g3ttenbj/3mza7ky6ut5on/card.png?v=1aacd1ca05d21764"}

grain.social/grain Switch repository Loading…

Every query answer says it varies by who asks, as knotless's do

grain.social/grain

Switch repository

Loading…