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

The toolbar's panel says which commit the page is serving and when it went live, linked to the commit #11486

opened this

Check · Review · Land · Live — Pushed since the last review. Next: reviewer, pushed since the last review.

Ready to land

scout/forge:scout/11480-the-toolbar-s-panel-says-which-commit-th → main 2 commits fork 2 threads · 2 unresolved

Review

Sign in to review this change.
4 files changed +114 −2
docs/features/forge-toolbar.md +7 −1
@@ -9,6 +9,12 @@ preview), so you always know where you are and can get back to the forge.
99 byte. A page that already carries the tag is not given a second.
1010 - **What it says:** the app, the environment and, on a preview, a link
1111 "Preview of pull #N" to the pull's page on the forge; Home goes to the forge.
12 - **Which commit:** one more line, `Serving a1b2c3d, live 4 min ago`, the short
13 id linked to the commit on the forge. The id and the instant come from the
14 forge's own record of the running environment (the same source as the note's
15 build), never the client's; they travel as escaped `data-commit` and
16 `data-live` attributes and are set as text. Only a reader of the repository
17 gets them, and with no running record the line is left out.
1218 - **What a note carries:** a note sent from the pill also carries where it came
1319 from, folded at the foot of the issue under "Where this came from": the
1420 build the app is serving (from the forge's own record of the running
@@ -29,4 +35,4 @@ preview), so you always know where you are and can get back to the forge.
2935 pill out; the app still works.
3036 - **Off:** `"toolbar": false` in `forge.json`.
3137
32 Tests: `test/toolbar-injection.test.ts`, `test/apps-toolbar.test.ts`, `test/note-context.test.ts`, `test/apps-feedback.test.ts`.
38 Tests: `test/toolbar-injection.test.ts`, `test/apps-toolbar.test.ts`, `test/toolbar-serving.test.ts`, `test/note-context.test.ts`, `test/apps-feedback.test.ts`.
src/apps/proxy.ts +5
@@ -557,6 +557,11 @@ export function pillWhere(
557557 // An app's access is not its repository's: the name goes only to a reader.
558558 const reads = canRead(db, repo, user);
559559 if (reads) where.repo = `${repo.ownerName}/${repo.name}`;
560 // What the forge's own record says is running here, for a reader of the repository only.
561 const live = reads ? runtime.envStatus?.(target.app, target.env) : undefined;
562 const liveAt = live?.servingAt ?? live?.since;
563 if (live?.state === "running" && live.commitOid !== "" && liveAt !== undefined)
564 where.serving = { oid: live.commitOid, at: liveAt };
560565 const number = /^pull-(\d+)$/.exec(target.env)?.[1];
561566 if (number === undefined) return where;
562567 where.pull = Number(number);
src/apps/toolbar.ts +27 −1
@@ -87,7 +87,8 @@ const toolbarSource = `(function () {
8787 }
8888 if (d.app) line(d.app, true);
8989 if (d.env === "production") line("production");
90 var LS = "color:#8ab4ff;text-decoration:none;display:flex;align-items:center;min-height:44px";
90 var LC = "#8ab4ff";
91 var LS = "color:" + LC + ";text-decoration:none;display:flex;align-items:center;min-height:44px";
9192 var pullHref = "";
9293 // The pull this preview shows, one tap away: built from the script's own origin, never from page text.
9394 if (d.env !== "production" && /^\\d{1,9}$/.test(d.pull || "") && /^[\\w.-]+\\/[\\w.-]+$/.test(d.repo || "")) {
@@ -112,6 +113,27 @@ const toolbarSource = `(function () {
112113 : d.deploying ? W.deploying
113114 : d.behind ? W.behind : "";
114115 if (freshness) line(freshness);
116 // Which commit this page is serving and since when: the forge's record, set as text.
117 var ago = function (iso) {
118 var t = Date.parse(iso || "");
119 if (!(t > 0)) return "";
120 var m = Math.max(0, Math.floor((Date.now() - t) / 60000));
121 return m < 1 ? "just now" : m < 60 ? m + " min ago" : m < 1440 ? Math.floor(m / 60) + " h ago" : Math.floor(m / 1440) + " d ago";
122 };
123 var liveAgo = "";
124 if (/^[0-9a-f]{7,64}$/.test(d.commit || "") && (liveAgo = ago(d.live))) {
125 var sv = document.createElement("div");
126 sv.style.cssText = "overflow-wrap:anywhere;opacity:.85";
outdated

ago(d.live) is computed twice (here and in the ', live ' part). Compute once into a variable and reuse it.

127 var part = function (t) { var sp = document.createElement("span"); sp.textContent = t; sv.appendChild(sp); };
128 part("Serving ");
129 var short = d.commit.slice(0, 7), cl = null;
130 if (/^[\\w.-]+\\/[\\w.-]+$/.test(d.repo || "") && fo) {
131 try { cl = document.createElement("a"); cl.href = new URL("/" + d.repo + "/commit/" + d.commit, fo).href; cl.rel = "noopener"; cl.style.color = LC; cl.textContent = short; } catch (e) { cl = null; }
132 }
133 if (cl) sv.appendChild(cl); else part(short);
134 part(", live " + liveAgo);
135 panel.appendChild(sv);
136 }
115137 var link = document.createElement("a");
116138 link.textContent = "Home";
117139 link.rel = "noopener";
@@ -638,6 +660,8 @@ export type PillWhere = {
638660 deploying?: boolean;
639661 /** The newest check of the pull's head, a key of `CHECK_WORDS`. */
640662 check?: PillCheck;
663 /** The commit the environment is serving and the instant it went live, from the forge's own record. */
664 serving?: { oid: string; at: string };
641665 };
642666
643667 /** `text` safe inside a double-quoted HTML attribute. */
@@ -687,6 +711,8 @@ export function toolbarTag(
687711 ["title", where.title],
688712 ["state", where.state],
689713 ["check", where.check],
714 ["commit", where.serving?.oid],
715 ["live", where.serving?.at],
690716 ["behind", where.behind === true ? "1" : undefined],
691717 ["deploying", where.deploying === true ? "1" : undefined],
692718 ]
test/toolbar-serving.test.ts added +75
@@ -0,0 +1,75 @@
1 /** The panel says which commit the page is serving and since when, from the forge's record (#11480). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, 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 { newApp } from "../src/actions/apps.ts";
8 import { createUser } from "../src/core/users.ts";
9 import { getAppByRepo } from "../src/core/apps.ts";
10 import { pillWhere, type AppTarget } from "../src/apps/proxy.ts";
11 import type { AppsRuntime } from "../src/apps/runtime.ts";
12 import { toolbarTag } from "../src/apps/toolbar.ts";
13 import { boot } from "./helpers/toolbar-boot.ts";
14
15 const OID = "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678";
16 const base = { app: "notes", env: "production", repo: "alice/notes", forge: "https://forge.test" };
17 const ago = (ms: number) => new Date(Date.now() - ms).toISOString();
18 const line = (set: Record<string, string>) => {
19 const b = boot(null, 400, 800, set);
20 const els: any[] = [b.panel];
21 const walk = (e: any) => e.children.forEach((c: any) => { els.push(c); walk(c); });
22 walk(b.panel);
23 return { texts: b.texts(), anchors: els.filter((e) => e.tag === "a") };
24 };
25
26 test("with a record the panel says the short id, linked to the commit, and how long ago", () => {
27 const { texts, anchors } = line({ ...base, commit: OID, live: ago(4 * 60000 + 5000) });
28 expect(texts).toContain("Serving ");
29 expect(texts).toContain("a1b2c3d");
30 expect(texts).toContain(", live 4 min ago");
31 expect(anchors.find((a) => a.textContent === "a1b2c3d")?.href).toBe(`https://forge.test/alice/notes/commit/${OID}`);
32 });
33
34 test("without a record the line is left out", () => {
35 expect(line(base).texts.some((t) => t.startsWith("Serving"))).toBe(false);
36 expect(line({ ...base, commit: OID }).texts.some((t) => t.startsWith("Serving"))).toBe(false);
37 expect(line({ ...base, commit: "<b>x</b>", live: ago(1000) }).texts.some((t) => t.startsWith("Serving"))).toBe(false);
38 });
39
40 test("the tag carries the record as escaped attributes, and an app name with markup stays text", () => {
41 const evil = 'a"><img src=x onerror=1>';
42 const tag = toolbarTag("https://forge.test", { app: evil, env: "production", serving: { oid: OID, at: "2026-01-01T00:00:00.000Z" } });
43 expect(tag).toContain(` data-commit="${OID}"`);
44 expect(tag).toContain(` data-live="2026-01-01T00:00:00.000Z"`);
outdated

Imports in the middle of the file; hoist them to the top with the others.

45 expect(tag).not.toContain("<img");
46 expect(toolbarTag("https://forge.test", { app: "a", env: "production" })).not.toContain("data-commit");
47 expect(line({ ...base, app: evil, commit: OID, live: ago(1000) }).texts).toContain(evil);
48 });
49
50 test("pillWhere takes the commit and time from the runtime's record, for a reader only", () => {
51 const status = { state: "running", commitOid: OID, servingAt: "2026-01-01T00:00:00.000Z" };
52 const w = (st: unknown, who?: unknown) =>
53 pillWhere({ app: fake.app, envStatus: () => st } as unknown as AppsRuntime, fake.target, who as never);
54 expect(w(status).serving).toEqual({ oid: OID, at: "2026-01-01T00:00:00.000Z" });
55 expect(w(undefined).serving).toBeUndefined();
56 expect(w({ ...status, state: "deploying" }).serving).toBeUndefined();
57 });
58
59
60 const fake = {} as { app: App; target: AppTarget };
61 let dir = "";
62 beforeAll(async () => {
63 dir = await mkdtemp(join(tmpdir(), "forge-toolbar-serving-"));
64 const app = createApp({ dataDir: dir, port: 0, host: "forge.test" });
65 const u = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true });
66 if (!u.ok) throw new Error(u.error);
67 const made = await newApp(app, u.value, { name: "open", description: "x", visibility: "public" });
68 if (!made.ok) throw new Error(String(made.error));
69 fake.app = app;
70 fake.target = { app: getAppByRepo(app.db, made.value.repo.id)!, env: "production" } as AppTarget;
71 }, 120_000);
72 afterAll(async () => {
73 fake.app.close();
74 await rm(dir, { recursive: true, force: true });
75 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs