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

A repository's toolbar page's Server-Timing names its phases: session, each read, render #3537

Merged opened this

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

Merged

scout/forge:scout/3529-a-repository-s-toolbar-page-s-server-tim → main 1 commit fork 2 threads · 2 unresolved

  • Merged. scout/3529-a-repository-s-toolbar-page-s-server-tim was merged into main as 6a8093f.

Review

Sign in to review this change.
2 files changed +81 −11
src/web/studio/toolbar.ts +25 −11
@@ -23,6 +23,7 @@ import { canWrite } from "../../core/repos.ts";
2323 import { listRuns } from "../../core/runs.ts";
2424 import { mayView, mayViewId } from "../../core/sealed.ts";
2525 import { mayDirectCrew } from "../../crew/direct.ts";
26 import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts";
2627 import type { Ctx } from "../../http/context.ts";
2728 import { notFoundPage, pageHead, loadRepo, repoBase } from "../chrome.ts";
2829 import { field, readForm } from "../form.ts";
@@ -60,36 +61,47 @@ const toolbarPath = (base: string, env: string, address: string): string =>
6061
6162 // GET /:owner/:name/toolbar
6263 export function toolbarPage(ctx: Ctx): Response {
63 const repo = loadRepo(ctx);
64 const request = ctx.request;
65 notePhase(request, "session", takeSessionTime(request));
66 const repo = timePhase(request, "repo", () => loadRepo(ctx));
6467 if (!repo) return notFoundPage(ctx);
6568 const where = parseEnv(ctx.url.searchParams.get("env") ?? "production");
66 if (where === undefined || getAppByRepo(ctx.app.db, repo.id) === undefined)
69 if (where === undefined || timePhase(request, "app-row", () => getAppByRepo(ctx.app.db, repo.id)) === undefined)
6770 return notFoundPage(ctx);

Long line. Hoist the app lookup into a const so it reads like the other timed reads.

6871 const db = ctx.app.db;
6972 const user = ctx.auth.user;
70 const pull = where.pull === undefined ? undefined : getIssue(db, repo.id, where.pull);
73 const pull =
74 where.pull === undefined
75 ? undefined
76 : timePhase(request, "pull", () => getIssue(db, repo.id, where.pull!));
7177 if (where.pull !== undefined && (!pull || pull.kind !== "pull" || !mayView(db, repo, pull, user)))

where.pull! is avoidable: const number = where.pull; before the ternary narrows it.

7278 return notFoundPage(ctx);
7379 const address = pageAddress(ctx.url.searchParams.get("page") ?? "");
7480 const base = repoBase(repo);
7581 const here = html`${repo.name}, ${pull ? html`preview of pull #${pull.number}` : "production"}`;
7682 const home = btn("Home", { href: "/", variant: "ghost" });
83 const rendering = performance.now();
7784 if (!user) {
78 return page({
85 const response = page({
7986 ctx,
8087 title: `${repo.name} · forge`,
8188 body: html`<div class="narrow-wide">${pageHead(here)}<div class="form-actions">${home}</div></div>`,
8289 });
90 notePhase(request, "render", performance.now() - rendering);
91 return response;
8392 }
8493 // Read once, now, as the page opens: the live agent run on this app, or
8594 // on this preview's pull.
8695 // A run on work the viewer may not see (a sealed report) is not theirs to read.
87 const live = listRuns(db, repo.id, {
88 kind: "agent",
89 ...(pull ? { issueId: pull.id } : {}),
90 state: "running",
91 limit: 200,
92 }).find((r) => mayViewId(db, repo, r.issueId, user));
96 const live = timePhase(request, "runs", () =>
97 listRuns(db, repo.id, {
98 kind: "agent",
99 ...(pull ? { issueId: pull.id } : {}),
100 state: "running",
101 limit: 200,
102 }).find((r) => mayViewId(db, repo, r.issueId, user)),
103 );
104 const renderStart = performance.now();
93105 const comment = pull ? "Comment on this pull" : "Report this page";
94106 const body = html`<div class="narrow-wide">
95107 ${pageHead(here)}
@@ -131,7 +143,9 @@ export function toolbarPage(ctx: Ctx): Response {
131143 ${home}
132144 </div>
133145 </div>`;
134 return page({ ctx, title: `${repo.name} · forge`, body });
146 const response = page({ ctx, title: `${repo.name} · forge`, body });
147 notePhase(request, "render", performance.now() - renderStart);
148 return response;
135149 }
136150
137151 // POST /:owner/:name/toolbar/comment
test/toolbar-page-timing.test.ts added +56
@@ -0,0 +1,56 @@
1 /** `/:owner/:name/toolbar` names session, repo, app-row, runs and render in Server-Timing (#3529). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtempSync, rmSync } from "node:fs";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { newApp } from "../src/actions/apps.ts";
7 import { createApp, type App } from "../src/app.ts";
8 import { createUser } from "../src/core/users.ts";
9 import { serve, type Servers } from "../src/http/server.ts";
10 import { proveAll } from "./helpers/verified.ts";
11 import { quietLogs } from "./helpers/globals.ts";
12
13 quietLogs();
14
15 let root = "";
16 let app: App;
17 let servers: Servers;
18
19 beforeAll(async () => {
20 root = mkdtempSync(join(tmpdir(), "forge-toolbar-timing-"));
21 app = createApp({ dataDir: root, port: 0, forgeRepo: "alice/forge" });
22 const u = await createUser(app.db, {
23 origin: "seed",
24 trusted: true,
25 username: "alice",
26 email: "[email protected]",
27 password: "a-good-passphrase",
28 adminIfFirst: true,
29 });
30 if (!u.ok) throw new Error(u.error);
31 proveAll(app);
32 const made = await newApp(app, { id: u.value.id, username: "alice" } as never, { name: "forge", description: "x", visibility: "public" });
33 if (!made.ok) throw new Error(made.error);
34 servers = await serve(app);
35 });
36 afterAll(() => {
37 servers.stop();
38 app.close();
39 rmSync(root, { recursive: true, force: true });
40 });
41
42 test("GET /:owner/:name/toolbar names its phases", async () => {
43 const signin = await fetch(`${servers.url}/signin`, {
44 method: "POST",
45 headers: { origin: servers.url },
46 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
47 redirect: "manual",
48 });
49 const cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
50 const res = await fetch(`${servers.url}/alice/forge/toolbar`, { headers: { cookie } });
51 expect(res.status).toBe(200);
52 await res.text();
53 const parts = (res.headers.get("server-timing") ?? "").split(", ");
54 for (const name of ["app", "session", "repo", "app-row", "runs", "render"])
55 expect(parts.find((p) => p.startsWith(`${name};`))).toMatch(/^[a-z-]+;dur=\d+\.\d$/);
56 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs