Public: anyone can read this; the crew can change it.
A preview's pill says when its pull has merged or closed, and when the preview runs an older commit than the pull's newest push #9116
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9111-a-preview-s-pill-says-when-its-pull-has → main 2 commits fork 1 thread · 1 unresolved
3 files
changed +234
−1
src/apps/proxy.ts +14 −1
| @@ -18,6 +18,7 @@ | ||
| 18 | 18 | import { compressBytes, MIN_BYTES, negotiate } from "../http/compress.ts"; |
| 19 | 19 | import type { Database } from "bun:sqlite"; |
| 20 | 20 | import { getIssue, type Issue } from "../core/issues.ts"; |
| 21 | import { getPull } from "../core/pulls.ts"; | |
| 21 | 22 | import { mayView } from "../core/sealed.ts"; |
| 22 | 23 | import { holdOpen } from "../http/caller.ts"; |
| 23 | 24 | import { appHostSuffix } from "./host.ts"; |
| @@ -466,7 +467,19 @@ export function pillWhere( | ||
| 466 | 467 | where.pull = Number(number); |
| 467 | 468 | const pull = getIssue(db, target.app.repoId, where.pull); |
| 468 | 469 | if (pull === undefined) return where; |
| 469 | if (reads && mayView(db, repo, pull, user)) where.title = pull.title; | |
| 470 | if (!reads || !mayView(db, repo, pull, user)) return where; | |
| 471 | where.title = pull.title; | |
| 472 | // How current the preview is: the row's head against the commit it runs. A | |
| 473 | // signed-in reader only, so a page an anonymous viewer may share never carries it. | |
| 474 | if (user === undefined) return where; | |
| 475 | const row = getPull(db, target.app.repoId, where.pull); | |
| 476 | if (row === undefined) return where; | |
| 477 | where.state = row.mergedAt !== null ? "merged" : row.state === "closed" ? "closed" : "open"; | |
| 478 | // Only an open pull has a newer push to be behind. | |
| 479 | if (where.state !== "open") return where; | |
| 480 | const status = runtime.envStatus?.(target.app, target.env); | |
| 481 | if (status?.state === "deploying") where.deploying = true; | |
| 482 | where.behind = status?.state !== "deploying" && status?.commitOid !== undefined && status.commitOid !== row.headOid; | |
| 470 | 483 | return where; |
| 471 | 484 | } |
| 472 | 485 |
src/apps/toolbar.ts +15
| @@ -88,6 +88,12 @@ export const toolbarJs = `(function () { | ||
| 88 | 88 | panel.appendChild(pl); |
| 89 | 89 | } else if (d.pull) line("Preview of pull #" + d.pull); |
| 90 | 90 | if (d.title) line(d.title); |
| 91 | // Whether this is the pull as it stands now; an open, current preview says nothing. | |
| 92 | var freshness = d.state === "merged" ? "Merged: this preview shows the pull as it landed" | |
| 93 | : d.state === "closed" ? "Closed without landing" | |
| 94 | : d.deploying ? "A newer push is deploying" | |
| 95 | : d.behind ? "Shows an older push than the pull's newest" : ""; | |
| 96 | if (freshness) line(freshness); | |
| 91 | 97 | var link = document.createElement("a"); |
| 92 | 98 | link.textContent = "Home"; |
| 93 | 99 | link.rel = "noopener"; |
| @@ -565,6 +571,12 @@ export type PillWhere = { | ||
| 565 | 571 | env: string; |
| 566 | 572 | pull?: number; |
| 567 | 573 | title?: string; |
| 574 | /** Whether the pull is open, merged or closed; for a reader of the pull only. */ | |
| 575 | state?: "open" | "merged" | "closed"; | |
| 576 | /** The preview runs a commit that is not the pull's head (and none is deploying). */ | |
| 577 | behind?: boolean; | |
| 578 | /** A newer commit is being deployed now. */ | |
| 579 | deploying?: boolean; | |
| 568 | 580 | }; |
| 569 | 581 | |
| 570 | 582 | /** `text` safe inside a double-quoted HTML attribute. */ |
| @@ -612,6 +624,9 @@ export function toolbarTag( | ||
| 612 | 624 | ["env", where.env], |
| 613 | 625 | ["pull", where.pull === undefined ? undefined : String(where.pull)], |
| 614 | 626 | ["title", where.title], |
| 627 | ["state", where.state], | |
| 628 | ["behind", where.behind === true ? "1" : undefined], | |
| 629 | ["deploying", where.deploying === true ? "1" : undefined], | |
| 615 | 630 | ] |
| 616 | 631 | .filter((pair): pair is [string, string] => pair[1] !== undefined) |
| 617 | 632 | .map(([k, v]) => ` data-${k}="${escapeAttr(v)}"`) |
test/apps-pill-freshness.test.ts added +205
| @@ -0,0 +1,205 @@ | ||
| 1 | /** | |
| 2 | * The pill says whether its preview is the pull as it stands (#9111): the | |
| 3 | * pull's state and `behind`, for a reader of the pull only. | |
| 4 | */ | |
| 5 | import { afterAll, beforeAll, describe, expect, spyOn, test } from "bun:test"; | |
| 6 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 7 | import { tmpdir } from "node:os"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { createApp, type App } from "../src/app.ts"; | |
| 10 | import { newApp } from "../src/actions/apps.ts"; | |
| 11 | import { pillWhere, proxyApp, signGrant, type AppSocket, type AppTarget } from "../src/apps/proxy.ts"; | |
| 12 | import { toolbarJs, toolbarTag } from "../src/apps/toolbar.ts"; | |
| 13 | import { AppsRuntime, type EnvStatus } from "../src/apps/runtime.ts"; | |
| 14 | import { getAppByRepo } from "../src/core/apps.ts"; | |
| 15 | import { createIssue } from "../src/core/issues.ts"; | |
| 16 | import { createUser } from "../src/core/users.ts"; | |
| 17 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 18 | import { quietLogs } from "./helpers/globals.ts"; | |
| 19 | ||
| 20 | quietLogs(); | |
| 21 | ||
| 22 | describe("the pill's freshness", () => { | |
| 23 | let dataDir: string; | |
| 24 | let app: App; | |
| 25 | let aliceId: string; | |
| 26 | let target: AppTarget; | |
| 27 | let repoId: string; | |
| 28 | const live = { state: "running", commitOid: "a".repeat(40) } as { state: string; commitOid: string }; | |
| 29 | const numbers: number[] = []; | |
| 30 | const runtime = () => | |
| 31 | ({ app, envStatus: () => live }) as unknown as AppsRuntime; | |
| 32 | ||
| 33 | beforeAll(async () => { | |
| 34 | dataDir = await mkdtemp(join(tmpdir(), "forge-pill-fresh-")); | |
| 35 | app = createApp({ dataDir, port: 0, host: "forge.test" }); | |
| 36 | const a = await createUser(app.db, { | |
| 37 | origin: "seed", trusted: true, username: "alice", | |
| 38 | email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true, | |
| 39 | }); | |
| 40 | if (!a.ok) throw new Error(a.error); | |
| 41 | aliceId = a.value.id; | |
| 42 | const made = await newApp(app, a.value, { name: "site", description: "x", visibility: "public" }); | |
| 43 | if (!made.ok) throw new Error(String(made.error)); | |
| 44 | repoId = made.value.repo.id; | |
| 45 | target = { app: getAppByRepo(app.db, repoId)!, env: "production" } as AppTarget; | |
| 46 | for (let i = 0; i < 3; i++) { | |
| 47 | const p = createIssue(app.db, { repoId, authorId: aliceId, title: `p${i}`, kind: "pull" }); | |
| 48 | if (!p.ok) throw new Error(p.error); | |
| 49 | numbers.push(p.value.number); | |
| 50 | app.db.run("INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid) VALUES (?1, 'main', 'topic', ?2, ?2)", [p.value.id, "a".repeat(40)]); | |
| 51 | } | |
| 52 | }, 120_000); | |
| 53 | ||
| 54 | afterAll(async () => { | |
| 55 | app.close(); | |
| 56 | await rm(dataDir, { recursive: true, force: true }); | |
| 57 | }); | |
| 58 | ||
| 59 | const at = (n: number): AppTarget => ({ ...target, env: `pull-${n}` }) as AppTarget; | |
| 60 | const alice = () => ({ id: aliceId }) as never; | |
| 61 | ||
| 62 | test("an open pull whose preview runs its head says nothing extra", () => { | |
| 63 | live.state = "running"; live.commitOid = "a".repeat(40); | |
| 64 | const w = pillWhere(runtime(), at(numbers[0]!), alice()); | |
| 65 | expect(w.state).toBe("open"); | |
| 66 | expect(w.behind).toBe(false); | |
| 67 | expect(w.deploying).toBeUndefined(); | |
| 68 | }); | |
| 69 | ||
| 70 | test("a merged pull's preview says merged; a closed one, closed", () => { | |
| 71 | const id = (n: number) => app.db.query<{ id: string }, [string, number]>("SELECT id FROM issue WHERE repo_id=?1 AND number=?2").get(repoId, n)!.id; | |
| 72 | app.db.run("UPDATE pull SET merged_at='2026-01-01T00:00:00Z' WHERE issue_id=?1", [id(numbers[1]!)]); | |
| 73 | app.db.run("UPDATE issue SET state='closed' WHERE id=?1", [id(numbers[1]!)]); | |
| 74 | expect(pillWhere(runtime(), at(numbers[1]!), alice()).state).toBe("merged"); | |
| 75 | app.db.run("UPDATE issue SET state='closed' WHERE id=?1", [id(numbers[2]!)]); | |
| 76 | expect(pillWhere(runtime(), at(numbers[2]!), alice()).state).toBe("closed"); | |
| 77 | }); | |
| 78 | ||
| 79 | test("a live commit that is not the head is behind; a deploy in flight is deploying instead", () => { | |
| 80 | live.state = "running"; live.commitOid = "b".repeat(40); | |
| 81 | expect(pillWhere(runtime(), at(numbers[0]!), alice()).behind).toBe(true); | |
| 82 | live.state = "deploying"; | |
| 83 | const w = pillWhere(runtime(), at(numbers[0]!), alice()); | |
| 84 | expect(w.behind).toBe(false); | |
| 85 | expect(w.deploying).toBe(true); | |
| 86 | }); | |
| 87 | ||
| 88 | test("an anonymous viewer gets neither field", () => { | |
| 89 | live.state = "running"; live.commitOid = "b".repeat(40); | |
| 90 | const w = pillWhere(runtime(), at(numbers[0]!), undefined); | |
| 91 | expect(w.state).toBeUndefined(); | |
| 92 | expect(w.behind).toBeUndefined(); | |
| 93 | expect(w.deploying).toBeUndefined(); | |
| 94 | }); | |
| 95 | ||
| 96 | test("the tag carries the fields and the script holds the words", () => { | |
| 97 | const tag = toolbarTag("http://f.test", { app: "a", env: "pull-3", pull: 3, state: "merged", behind: true }); | |
| 98 | expect(tag).toContain('data-state="merged"'); | |
| 99 | expect(tag).toContain('data-behind="1"'); | |
| 100 | for (const words of [ | |
| 101 | "Merged: this preview shows the pull as it landed", | |
| 102 | "Closed without landing", | |
| 103 | "A newer push is deploying", | |
| 104 | "Shows an older push than the pull's newest", | |
| 105 | ]) expect(toolbarJs).toContain(words); | |
| 106 | }); | |
| 107 | }); | |
| 108 | ||
| 109 | describe("the pill's freshness, through the proxy", () => { | |
| 110 | const HEAD = "a".repeat(40); | |
| 111 | const NO_SOCKETS = { upgrade: () => false } as unknown as Bun.Server<AppSocket>; | |
| 112 | let dataDir: string; | |
| 113 | let app: App; | |
| 114 | let servers: Servers; | |
| 115 | let runtime: AppsRuntime; | |
| 116 | let backend: ReturnType<typeof Bun.serve>; | |
| 117 | let status: ReturnType<typeof spyOn>; | |
| 118 | let live = HEAD; | |
| 119 | let target: AppTarget; | |
| 120 | let alice: { id: string; name: string; role: "admin" }; | |
| 121 | const pulls: Record<"open" | "merged", number> = { open: 0, merged: 0 }; | |
| 122 | ||
| 123 | beforeAll(async () => { | |
| 124 | backend = Bun.serve({ | |
| 125 | port: 0, | |
| 126 | fetch: () => new Response("<!doctype html><html><head></head><body><p>hi</p></body></html>", { headers: { "content-type": "text/html; charset=utf-8" } }), | |
| 127 | }); | |
| 128 | dataDir = await mkdtemp(join(tmpdir(), "forge-pill-fresh-proxy-")); | |
| 129 | app = createApp({ dataDir, port: 0, host: "forge.test" }); | |
| 130 | servers = await serve(app); | |
| 131 | runtime = new AppsRuntime(app, { domain: "apps.test", scheme: "http", publicPort: servers.port, forgeUrl: servers.url, concurrency: 1 }); | |
| 132 | await runtime.start(); | |
| 133 | status = spyOn(AppsRuntime.prototype, "envStatus").mockImplementation( | |
| 134 | () => ({ env: "x", state: "running", port: backend.port, commitOid: live }) as unknown as EnvStatus, | |
| 135 | ); | |
| 136 | const a = await createUser(app.db, { | |
| 137 | origin: "seed", trusted: true, username: "alice", | |
| 138 | email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true, | |
| 139 | }); | |
| 140 | if (!a.ok) throw new Error(a.error); | |
| 141 | alice = { id: a.value.id, name: "alice", role: "admin" }; | |
| 142 | const made = await newApp(app, a.value, { name: "site", description: "x", visibility: "public" }); | |
| 143 | if (!made.ok) throw new Error(String(made.error)); | |
| 144 | const repoId = made.value.repo.id; | |
| 145 | target = { app: getAppByRepo(app.db, repoId)!, env: "production" } as AppTarget; | |
| 146 | for (const kind of ["open", "merged"] as const) { | |
| 147 | const p = createIssue(app.db, { repoId, authorId: alice.id, title: kind, kind: "pull" }); | |
| 148 | if (!p.ok) throw new Error(p.error); | |
| 149 | pulls[kind] = p.value.number; | |
| 150 | app.db.run("INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid) VALUES (?1, 'main', 'topic', ?2, ?2)", [p.value.id, HEAD]); | |
| 151 | if (kind === "merged") { | |
| 152 | app.db.run("UPDATE pull SET merged_at='2026-01-01T00:00:00Z' WHERE issue_id=?1", [p.value.id]); | |
| 153 | app.db.run("UPDATE issue SET state='closed' WHERE id=?1", [p.value.id]); | |
| 154 | } | |
| 155 | } | |
| 156 | }, 120_000); | |
| 157 | ||
| 158 | afterAll(async () => { | |
| 159 | status.mockRestore(); | |
| 160 | await runtime.stop(); | |
| 161 | servers.stop(); | |
| 162 | backend.stop(true); | |
| 163 | app.close(); | |
| 164 | await rm(dataDir, { recursive: true, force: true }); | |
| 165 | }); | |
| 166 | ||
| 167 | /** The pill's injected script tag, as one served preview page carries it. */ | |
| 168 | async function tagFor(n: number, signedIn: boolean): Promise<string> { | |
| 169 | const at = { ...target, env: `pull-${n}` } as AppTarget; | |
| 170 | const headers: Record<string, string> = { host: "site.apps.test", accept: "text/html" }; | |
| 171 | if (signedIn) | |
| 172 | headers.cookie = `forge_grant_${at.app.id.slice(-8)}=${signGrant(dataDir, at, 60_000, Date.now(), alice)}`; | |
| 173 | const res = await proxyApp(runtime, new Request(`${servers.url}/`, { headers }), at, NO_SOCKETS); | |
| 174 | const html = await res!.text(); | |
| 175 | const tag = html.match(/<script[^>]*data-app=[^>]*>/)?.[0]; | |
| 176 | if (tag === undefined) throw new Error(`no pill tag in: ${html}`); | |
| 177 | return tag; | |
| 178 | } | |
| 179 | ||
| 180 | test("a signed-in reader's open preview at its head says open, not behind", async () => { | |
| 181 | live = HEAD; | |
| 182 | const tag = await tagFor(pulls.open, true); | |
| 183 | expect(tag).toContain('data-state="open"'); | |
| 184 | expect(tag).not.toContain("data-behind"); | |
| 185 | expect(tag).not.toContain("data-deploying"); | |
| 186 | }); | |
| 187 | ||
| 188 | test("a merged pull's preview says merged", async () => { | |
| 189 | expect(await tagFor(pulls.merged, true)).toContain('data-state="merged"'); | |
| 190 | }); | |
| 191 | ||
| 192 | test("a live commit that is not the head says behind", async () => { | |
| 193 | live = "b".repeat(40); | |
| 194 | expect(await tagFor(pulls.open, true)).toContain('data-behind="1"'); | |
| 195 | live = HEAD; | |
| 196 | }); | |
| 197 | ||
| 198 | test("an anonymous viewer's page carries none of the three", async () => { | |
| 199 | live = "b".repeat(40); | |
| 200 | const tag = await tagFor(pulls.open, false); | |
| 201 | expect(tag).toContain("data-pull="); | |
| 202 | for (const a of ["data-state", "data-behind", "data-deploying"]) expect(tag).not.toContain(a); | |
| 203 | live = HEAD; | |
| 204 | }); | |
| 205 | }); |
new above
offline
Add the through-the-proxy cases here (box 4): assert the served HTML tag, signed-in vs anonymous, not only pillWhere return values.