Public: anyone can read this; the crew can change it.

GET /api/v1/repos/:owner/:name/following carries an ETag and answers a repeat with 304 until the fork, its parent or the chain moves #9189

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

`GET /api/v1/repos/:owner/:name/following` now carries a weak ETag and answers a repeat with 304. The new test file and the existing `test/following.test.ts` pass (33 tests), `tsc --noEmit` is clean, and `scripts/gate.ts` printed nothing. With the change to `src/http/api/following.ts` stashed, 4 of the 6 new tests fail.

shipped production is serving it deploy 7a91a55

opened this

GET /api/v1/repos/:owner/:name/following now carries a weak ETag and answers a repeat with 304. The new test file and the existing test/following.test.ts pass (33 tests), tsc --noEmit is clean, and scripts/gate.ts printed nothing. With the change to src/http/api/following.ts stashed, 4 of the 6 new tests fail.

  • Headers: a 200 carries weakEtag over the JSON body, cache-control: private, no-cache and vary: authorization, cookie. A request holding the tag gets a 304 with the same headers and no body, and the security headers still arrive.
  • Both branches tagged: the plain fork branch and the chain-only branch (a forge's own code repository with no parent) go through one small tagged() helper in following.ts.
  • Errors: not_a_fork and a missing repo are 404s with no ETag.
  • Tag moves: because the tag is over the rendered body, a push to the fork, a push to the parent's default branch, and severing the chain each change it. The tests check all three.
  • Hidden parent: I only checked that a stranger gets a 404 with no ETag once the parent turns private. I did not compare tags between someone who can see the parent and someone who can't.
  • Left alone: src/http/api.ts and /following/proposals are untouched.

To try it, request the route twice with If-None-Match set to the first response's etag; the second should be a 304.

🤖 Generated with Claude Code

Closes #9180

commented

GET /api/v1/repos/:owner/:name/following now carries a weak ETag and answers a repeat with 304. The new test file and the existing test/following.test.ts pass (33 tests), tsc --noEmit is clean, and scripts/gate.ts printed nothing. With the change to src/http/api/following.ts stashed, 4 of the 6 new tests fail.

  • Headers: a 200 carries weakEtag over the JSON body, cache-control: private, no-cache and vary: authorization, cookie. A request holding the tag gets a 304 with the same headers and no body, and the security headers still arrive.
  • Both branches tagged: the plain fork branch and the chain-only branch (a forge's own code repository with no parent) go through one small tagged() helper in following.ts.
  • Errors: not_a_fork and a missing repo are 404s with no ETag.
  • Tag moves: because the tag is over the rendered body, a push to the fork, a push to the parent's default branch, and severing the chain each change it. The tests check all three.
  • Hidden parent: I only checked that a stranger gets a 404 with no ETag once the parent turns private. I did not compare tags between someone who can see the parent and someone who can't.
  • Left alone: src/http/api.ts and /following/proposals are untouched.

To try it, request the route twice with If-None-Match set to the first response's etag; the second should be a 304.

🤖 Generated with Claude Code

commented

GET /api/v1/repos/:owner/:name/following now carries a weak ETag and answers a repeat with 304. The check failed because no commit had a "Try:" line. I added an empty commit with one and pushed it, but I haven't seen the forge's check re-run. The gate gave no complaint when I ran it locally; I didn't re-run the tests for this push, since only a commit message changed.

  • Headers: a 200 carries weakEtag over the JSON body, cache-control: private, no-cache and vary: authorization, cookie. A request holding the tag gets a 304 with the same headers and no body, and the security headers still arrive.
  • Both branches tagged: the plain fork branch and the chain-only branch (a forge's own code repository with no parent) share one small tagged() helper in src/http/api/following.ts.
  • Errors: not_a_fork and a missing repo are 404s with no ETag.
  • Tag moves: a push to the fork, a push to the parent's default branch, and severing the chain each change the tag. test/following-etag.test.ts covers all three; with the change to following.ts stashed, 4 of its 6 tests fail. That file and test/following.test.ts passed (33 tests), and tsc --noEmit is clean.
  • Hidden parent: I only checked that a stranger gets a 404 with no ETag once the parent turns private. I did not compare tags between someone who can see the parent and someone who can't.
  • Left alone: src/http/api.ts and /following/proposals are untouched.

To try it on the preview, request /api/v1/repos/bob/up/following with a bearer token, then repeat with If-None-Match set to the returned etag; the second request should be a 304.

🤖 Generated with Claude Code

Changes requested — before the latest push

The route change is small and right: one tagged() helper over the rendered body, both branches covered, errors untouched. One miss against the list: box 3's hidden-parent clause is not tested, and the PR description admits it. Fix that first.

What to change:

  1. Replace the last-but-one test. It compares nothing: carol gets a 404 with no ETag, which says nothing about tags. Make a viewer who can see the parent (bob, the fork's owner) and one who cannot get 200s. For example, use a private parent that carol was granted access to on the fork but not on the parent. Then assert their etags differ, or that the hidden viewer's body omits the parent and its etag differs from the seeing viewer's. If relationOf gives the same body to both, say so in the test and drop the claim. As written, the test title promises something it does not check.
  2. The test's seen tag is only used as an If-None-Match on a request that 404s. Remove that dead assertion path.
  3. Replace the inline await import("../src/core/repos.ts") with a static import of getRepo.

Not required, but worth knowing: no doc or TUI change is needed for a header-only API change.

commented

I've reworked the hidden-parent test, and test/following-etag.test.ts passes (6 tests, tsc --noEmit clean) and is pushed. The gate printed nothing. I did not re-run the existing test/following.test.ts suite, which was green on the earlier push.

  • Test setup: the parent.json chain now points at bob/up, the fork itself, instead of a separate alice/forge repo. That gives a hidden-parent viewer a 200. They get the chain-only body rather than a 404.
  • Hidden-parent test: it makes the parent private and compares three viewers, all on 200s.

- Bob (the fork's owner) gets the full body with parent: "alice/up". - Carol, who can read the fork but not the parent, gets a body with no parent, and her etag differs from Bob's. - Alice, who still sees the parent, gets an etag different from Carol's. Presenting Carol's tag to Alice gets a 200, not a 304.

  • Dead assertion path: the old seen tag sent as If-None-Match on a 404 is gone.
  • Static import: getRepo is now imported at the top of the test file instead of dynamically inside a test.
  • Test order: the sever test now runs after the hidden-parent test, because severing changes the body for every viewer of that repo.

To try it on the preview, request /api/v1/repos/bob/up/following with a bearer token for the fork's owner and for a stranger once the parent is private. The etags should differ.

🤖 Generated with Claude Code

Approved

Approved. The earlier ask (compare tags between a viewer who can see the parent and one who cannot) is met: the reworked test compares bob, carol and alice on 200s, and carol's tag buys alice no 304. The helper is small and shared by both branches, errors carry no tag, and the tests fail without the change. The earlier head is not in my clone, so I read the whole change rather than the increment.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs