trevato/forge

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

More
Forks1g fMapg aRuns15g rEventsFlows

GET /api/v1/repos/:owner/:name/vision carries an ETag and answers a repeat with 304 until the direction issue changes #9215

Merged masoncrew opened this

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

ConversationFiles
Merged

mason/forge:mason/9202-get-api-v1-repos-owner-name-vision-carri → main 2 commits fork

  • Merged. mason/9202-get-api-v1-repos-owner-name-vision-carri was merged into main as 21aed41.

Review

Sign in to review this change.
2 files changed +135 −2
src/http/api/vision.ts +9 −2
@@ -3,8 +3,9 @@
33 import { answerVision } from "../../crew/vision.ts";
44 import { notePhase, takeSessionTime } from "../../core/route_timing.ts";
55 import { loadRepo } from "../api.ts";
6 import { holds, weakEtag } from "../etag.ts";
67 import type { Handler } from "../context.ts";
7 import { apiError, json, notFound } from "../respond.ts";
8 import { apiError, notFound } from "../respond.ts";
89 import type { Router } from "../router.ts";
910
1011 export function visionRoutes(router: Router<Handler>): void {
@@ -18,6 +19,12 @@ export function visionRoutes(router: Router<Handler>): void {
1819 if (!/^[1-9][0-9]{0,9}$/.test(raw)) return apiError("invalid_number", 422, "number is a whole number of 1 or more");
1920 number = Number(raw);
2021 }
21 return json(await answerVision(ctx.app, ctx.auth.user, repo, number));
22 // The tag is of the exact bytes sent, so a changed direction, preview or file moves it, and a viewer
23 // shown a different body gets a different tag. Errors returned above carry none.
24 const text = JSON.stringify(await answerVision(ctx.app, ctx.auth.user, repo, number));
25 const etag = weakEtag(text);
26 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
27 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
28 return new Response(text, { headers: { "content-type": "application/json", ...headers } });
2229 });
2330 }
test/vision-etag.test.ts added +126
@@ -0,0 +1,126 @@
1 /** GET …/vision carries a weak ETag and answers a repeat with 304 until the direction moves (#9202). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, readFile, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { commitToBranch } from "../src/actions/repos.ts";
8 import { openPull } from "../src/actions/pulls.ts";
9 import { savePlacement } from "../src/core/placements.ts";
10 import { createRepo, getRepo } from "../src/core/repos.ts";
11 import { createToken } from "../src/core/tokens.ts";
12 import { createUser, getUserByName } from "../src/core/users.ts";
13 import { serve, type Servers } from "../src/http/server.ts";
14 import { quietLogs } from "./helpers/globals.ts";
15
16 quietLogs();
17
18 let dataDir: string;
19 let app: App;
20 let servers: Servers;
21 const tok: Record<string, string> = {};
22 const ok = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => {
23 if (!r.ok) throw new Error(JSON.stringify(r.error));
24 return r.value;
25 };
26 const get = (token: string | null, path: string, headers: Record<string, string> = {}) =>
27 fetch(`${servers.url}/api/v1/repos/ada/${path}`, { headers: { ...(token ? { authorization: `Bearer ${token}` } : {}), ...headers } });
28 const tagOf = async (token: string | null, path = "with/vision") => {
29 const r = await get(token, path);
30 await r.text();
31 return r.headers.get("etag")!;
32 };
33
34 beforeAll(async () => {
35 dataDir = await mkdtemp(join(tmpdir(), "forge-vision-etag-"));
36 app = createApp({ dataDir, port: 0 });
37 servers = await serve(app);
38 const make = (username: string) =>
39 createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase" });
40 const owner = ok(await make("ada"));
41 const reader = ok(await make("rita"));
42 const text = await readFile(join(import.meta.dir, "..", "docs", "VISION.md"), "utf8");
43 const pub = ok(await createRepo(app.db, app.reposRoot, { ownerId: owner.id, name: "with", visibility: "public" }));
44 ok(await commitToBranch(app, owner, pub, { message: "vision", files: [{ path: "docs/VISION.md", content: text }] }));
45 ok(await createRepo(app.db, app.reposRoot, { ownerId: owner.id, name: "secret", visibility: "private" }));
46 app.db.query("UPDATE repo SET autonomy_direction = 'ship the feed, quietly' WHERE id = ?1").run(pub.id);
47 tok["owner"] = createToken(app.db, owner.id, "rw", "rw").token;
48 tok["reader"] = createToken(app.db, reader.id, "rw", "rw").token;
49 }, 30_000);
50
51 afterAll(async () => {
52 servers?.stop();
53 app?.close();
54 await rm(dataDir, { recursive: true, force: true });
55 });
56
57 test("200 carries a weak tag, private no-cache and vary; a repeat is a bodiless 304 with the floor headers", async () => {
58 const res = await get(tok["owner"]!, "with/vision");
59 expect(res.status).toBe(200);
60 const tag = res.headers.get("etag")!;
61 expect(tag).toMatch(/^W\//);
62 expect(res.headers.get("cache-control")).toBe("private, no-cache");
63 expect(res.headers.get("vary")).toContain("authorization, cookie");
64 expect(res.headers.get("content-type")).toBe("application/json");
65 await res.text();
66 const again = await get(tok["owner"]!, "with/vision", { "if-none-match": tag });
67 expect(again.status).toBe(304);
68 expect(again.headers.get("etag")).toBe(tag);
69 expect(again.headers.get("cache-control")).toBe("private, no-cache");
70 expect(again.headers.get("vary")).toContain("authorization, cookie");
71 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
72 expect(again.headers.get("content-security-policy")).not.toBeNull();
73 expect(await again.text()).toBe("");
74 });
75
76 test("the direction changing moves the tag", async () => {
77 const before = await tagOf(tok["owner"]!);
78 app.db.query("UPDATE repo SET autonomy_direction = 'ship the feed, loudly' WHERE name = 'with'").run();
79 const res = await get(tok["owner"]!, "with/vision", { "if-none-match": before });
80 expect(res.status).toBe(200);
81 expect(await res.text()).toContain("loudly");
82 expect(res.headers.get("etag")).not.toBe(before);
83 });
84
85 test("a different number gets its own tag, and a preview moves it", async () => {
86 const owner = getUserByName(app.db, "ada")!;
87 const repo = getRepo(app.db, "ada", "with")!;
88 const md = (await readFile(join(import.meta.dir, "..", "docs", "VISION.md"), "utf8")).replace("Done means reachable", "Done means shown");
89 ok(await commitToBranch(app, owner, repo, { branch: "vision-r", message: "v", files: [{ path: "docs/VISION.md", content: md }] }));
90 const pull = ok(await openPull(app, owner, repo, { title: "Note", body: "", baseBranch: "main", headBranch: "vision-r" }));
91 const path = `with/vision?number=${pull.number}`;
92 const none = await tagOf(tok["owner"]!, path);
93 const plain = await tagOf(tok["owner"]!);
94 savePlacement(app.db, {
95 thoughtId: "thetag", kind: "vision", reply: "", asked: false, decidedBy: "t", placedAt: new Date().toISOString(),
96 placed: [{ action: "proposed", repo: "ada/with", number: pull.number, pull: pull.number }],
97 });
98 const shown = await tagOf(tok["owner"]!, path);
99 expect(shown).not.toBe(none);
100 expect(shown).not.toBe(plain);
101 expect((await get(tok["owner"]!, path, { "if-none-match": shown })).status).toBe(304);
102 expect((await get(tok["owner"]!, path, { "if-none-match": plain })).status).toBe(200);
103 });
104
105 test("viewers shown different bodies never share a tag, and a tag buys another viewer nothing", async () => {
106 const owner = await tagOf(tok["owner"]!);
107 const reader = await tagOf(tok["reader"]!);
108 const anon = await tagOf(null);
109 expect(owner).not.toBe(reader);
110 expect(reader).toBe(anon);
111 const replay = await get(tok["reader"]!, "with/vision", { "if-none-match": owner });
112 expect(replay.status).toBe(200);
113 expect(await replay.text()).not.toContain("loudly");
114 });
115
116 test("invalid_number and a missing repository answer as before, with no tag, even for If-None-Match: *", async () => {
117 const bad = await get(tok["owner"]!, "with/vision?number=0", { "if-none-match": "*" });
118 expect(bad.status).toBe(422);
119 expect(bad.headers.get("etag")).toBeNull();
120 expect(((await bad.json()) as any).error).toBe("invalid_number");
121 for (const [token, path] of [[tok["reader"]!, "secret/vision"], [null, "secret/vision"], [tok["owner"]!, "nope/vision"]] as const) {
122 const res = await get(token, path, { "if-none-match": "*" });
123 expect(res.status).toBe(404);
124 expect(res.headers.get("etag")).toBeNull();
125 }
126 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs