Public: anyone can read this; the crew can change it.
An app host's /__forge/ door and /__forge/crew name their Server-Timing phases: session, each read, render #5861
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5857-an-app-host-s-forge-door-and-forge-crew → main 2 commits fork 3 threads · 3 unresolved
3 files
changed +129
−34
src/apps/crew.ts +24 −10
| @@ -9,6 +9,7 @@ | ||
| 9 | 9 | import type { Database } from "bun:sqlite"; |
| 10 | 10 | import type { AppRecord } from "../core/apps.ts"; |
| 11 | 11 | import { mayDirectCrew } from "../crew/direct.ts"; |
| 12 | import { timePhase } from "../core/route_timing.ts"; | |
| 12 | 13 | import { getIssue } from "../core/issues.ts"; |
| 13 | 14 | import { canRead, getRepoById } from "../core/repos.ts"; |
| 14 | 15 | import { workingAgentRuns } from "../core/runs.ts"; |
| @@ -23,22 +24,35 @@ export function crewNow( | ||
| 23 | 24 | app: AppRecord, |
| 24 | 25 | env: string, |
| 25 | 26 | viewerId: string | undefined, |
| 27 | /** When given, the reads are named on it: `app`, `pull`, `working`. */ | |
| 28 | request?: Request, | |
| 26 | 29 | ): CrewNow | undefined { |
| 30 | const phase = <T>(name: string, work: () => T): T => | |
| 31 | request === undefined ? work() : timePhase(request, name, work); | |
| 27 | 32 | if (viewerId === undefined) return undefined; |
| 28 | const viewer = getUserById(db, viewerId); | |
| 29 | const repo = getRepoById(db, app.repoId); | |
| 30 | if (viewer === undefined || !isActive(viewer) || repo === undefined) return undefined; | |
| 31 | if (!canRead(db, repo, viewer)) return undefined; | |
| 33 | const seen = phase("app", () => { | |
| 34 | const viewer = getUserById(db, viewerId); | |
| 35 | const repo = getRepoById(db, app.repoId); | |
| 36 | if (viewer === undefined || !isActive(viewer) || repo === undefined) return undefined; | |
| 37 | if (!canRead(db, repo, viewer)) return undefined; | |
| 38 | return { viewer, repo }; | |
| 39 | }); | |
| 40 | if (seen === undefined) return undefined; | |
| 41 | const { viewer, repo } = seen; | |
| 32 | 42 | const number = /^pull-(\d+)$/.exec(env)?.[1]; |
| 33 | let issueId: string | undefined; | |
| 34 | if (number !== undefined) { | |
| 43 | if (number === undefined && env !== "production") return undefined; | |
| 44 | const pulled = phase("pull", () => { | |
| 45 | if (number === undefined) return { issueId: undefined }; | |
| 35 | 46 | const pull = getIssue(db, repo.id, Number(number)); |
| 36 | 47 | if (pull === undefined || !mayView(db, repo, pull, viewer)) return undefined; |
| 37 | issueId = pull.id; | |
| 38 | } else if (env !== "production") return undefined; | |
| 48 | return { issueId: pull.id as string | undefined }; | |
| 49 | }); | |
| 50 | if (pulled === undefined) return undefined; | |
| 39 | 51 | // A run on a sealed issue is not the viewer's to know of. |
| 40 | const run = workingAgentRuns(db, repo.id, issueId).find((r) => | |
| 41 | mayViewId(db, repo, r.issueId, viewer), | |
| 52 | const run = phase("working", () => | |
| 53 | workingAgentRuns(db, repo.id, pulled.issueId).find((r) => | |
| 54 | mayViewId(db, repo, r.issueId, viewer), | |
| 55 | ), | |
| 42 | 56 | ); |
| 43 | 57 | if (run === undefined) return undefined; |
| 44 | 58 | const actor = run.actorId === null ? undefined : getUserById(db, run.actorId); |
src/apps/proxy.ts +54 −24
| @@ -39,6 +39,7 @@ import { crewNow, tellLink } from "./crew.ts"; | ||
| 39 | 39 | import { feedback } from "./feedback.ts"; |
| 40 | 40 | import { clientBucket } from "../http/peer.ts"; |
| 41 | 41 | import { doorPage } from "./door.ts"; |
| 42 | import { takePhases, timePhase } from "../core/route_timing.ts"; | |
| 42 | 43 | import { notFound } from "../http/respond.ts"; |
| 43 | 44 | import { appMeasure } from "./measure.ts"; |
| 44 | 45 | import { holdPortRequest, holdPortSocket } from "./handoff.ts"; |
| @@ -375,6 +376,17 @@ function sealedPull(runtime: AppsRuntime, target: AppTarget): Issue | undefined | ||
| 375 | 376 | return pull?.sealed === true ? pull : undefined; |
| 376 | 377 | } |
| 377 | 378 | |
| 379 | /** The response with the phases the request named, as `Server-Timing`. */ | |
| 380 | function withPhases(request: Request, res: Response): Response { | |
| 381 | const phases = takePhases(request); | |
| 382 | if (phases.length > 0) | |
| 383 | res.headers.set( | |
| 384 | "server-timing", | |
| 385 | phases.map((p) => `${p.name};dur=${p.ms.toFixed(1)}`).join(", "), | |
| 386 | ); | |
| 387 | return res; | |
| 388 | } | |
| 389 | ||
| 378 | 390 | /** |
| 379 | 391 | * What the pill says: the app, the environment and, for a preview, the pull |
| 380 | 392 | * and its title. The title goes only to a viewer who may read the pull. |
| @@ -578,17 +590,19 @@ export async function proxyApp( | ||
| 578 | 590 | // The pill's fourth door: who of the crew is working on this app now, |
| 579 | 591 | // for the person the app's own grant names, read once as the pill opens. |
| 580 | 592 | if (url.pathname === "/__forge/crew" && !target.nested) { |
| 581 | const cookie = cookieValue(request, grantCookieName(target)); | |
| 582 | const signed = | |
| 583 | cookie === undefined | |
| 593 | const signed = timePhase(request, "session", () => { | |
| 594 | const cookie = cookieValue(request, grantCookieName(target)); | |
| 595 | return cookie === undefined | |
| 584 | 596 | ? undefined |
| 585 | 597 | : readGrant(runtime.app.config.dataDir, cookie, target); |
| 598 | }); | |
| 586 | 599 | const crew = crewNow( |
| 587 | 600 | runtime.app.db, |
| 588 | 601 | runtime.options.forgeUrl, |
| 589 | 602 | target.app, |
| 590 | 603 | target.env, |
| 591 | 604 | signed?.who?.id, |
| 605 | request, | |
| 592 | 606 | ); |
| 593 | 607 | const tell = tellLink( |
| 594 | 608 | runtime.app.db, |
| @@ -596,12 +610,19 @@ export async function proxyApp( | ||
| 596 | 610 | target.app, |
| 597 | 611 | signed?.who?.id, |
| 598 | 612 | ); |
| 599 | return Response.json({ ...crew, ...(tell === undefined ? {} : { tell }) }, { | |
| 600 | headers: { | |
| 601 | "cache-control": "no-store", | |
| 602 | "x-content-type-options": "nosniff", | |
| 603 | }, | |
| 604 | }); | |
| 613 | const body = timePhase(request, "render", () => | |
| 614 | JSON.stringify({ ...crew, ...(tell === undefined ? {} : { tell }) }), | |
| 615 | ); | |
| 616 | return withPhases( | |
| 617 | request, | |
| 618 | new Response(body, { | |
| 619 | headers: { | |
| 620 | "content-type": "application/json;charset=utf-8", | |
| 621 | "cache-control": "no-store", | |
| 622 | "x-content-type-options": "nosniff", | |
| 623 | }, | |
| 624 | }), | |
| 625 | ); | |
| 605 | 626 | } |
| 606 | 627 | // The forge's small page on the app's own host: the way back when the pill |
| 607 | 628 | // is not shown. A nested host is its guest's, and has no such page. |
| @@ -612,25 +633,34 @@ export async function proxyApp( | ||
| 612 | 633 | status: 405, |
| 613 | 634 | headers: { allow: "GET, HEAD", "x-content-type-options": "nosniff" }, |
| 614 | 635 | }); |
| 615 | const cookie = cookieValue(request, grantCookieName(target)); | |
| 616 | const signed = | |
| 617 | cookie === undefined | |
| 636 | const signed = timePhase(request, "session", () => { | |
| 637 | const cookie = cookieValue(request, grantCookieName(target)); | |
| 638 | return cookie === undefined | |
| 618 | 639 | ? undefined |
| 619 | 640 | : readGrant(runtime.app.config.dataDir, cookie, target); |
| 620 | const tell = tellLink( | |
| 621 | runtime.app.db, | |
| 622 | runtime.options.forgeUrl, | |
| 623 | target.app, | |
| 624 | signed?.who?.id, | |
| 641 | }); | |
| 642 | const tell = timePhase(request, "app", () => | |
| 643 | tellLink( | |
| 644 | runtime.app.db, | |
| 645 | runtime.options.forgeUrl, | |
| 646 | target.app, | |
| 647 | signed?.who?.id, | |
| 648 | ), | |
| 625 | 649 | ); |
| 626 | return doorPage( | |
| 627 | { | |
| 628 | where: pillWhere(runtime, target, signed?.who), | |
| 629 | forgeUrl: runtime.options.forgeUrl, | |
| 630 | ...(tell === undefined ? { signin: "/__forge/signin" } : { tell }), | |
| 631 | }, | |
| 632 | request.method === "HEAD", | |
| 650 | const where = timePhase(request, "pull", () => | |
| 651 | pillWhere(runtime, target, signed?.who), | |
| 633 | 652 | ); |
| 653 | const page = timePhase(request, "render", () => | |
| 654 | doorPage( | |
| 655 | { | |
outdated
This names a read that did not happen, at 0 ms. Remove it; the door honestly reads session, app, pull, render. |
||
| 656 | where, | |
| 657 | forgeUrl: runtime.options.forgeUrl, | |
| 658 | ...(tell === undefined ? { signin: "/__forge/signin" } : { tell }), | |
| 659 | }, | |
| 660 | request.method === "HEAD", | |
| 661 | ), | |
| 662 | ); | |
| 663 | return withPhases(request, page); | |
| 634 | 664 | } |
| 635 | 665 | // The third door: a problem report, answered without waking or even |
| 636 | 666 | // reaching the app — the app being down is the usual reason to ask. The |
test/apps-door.test.ts +51
| @@ -165,3 +165,54 @@ test("HEAD answers the headers with no body; POST is a 405 that says what is all | ||
| 165 | 165 | expect(post.headers.get("allow")).toBe("GET, HEAD"); |
| 166 | 166 | expect(post.headers.get("x-content-type-options")).toBe("nosniff"); |
| 167 | 167 | }); |
| 168 | ||
| 169 | const phasesOf = async (host: string, path: string, cookie?: string) => { | |
| 170 | let header = ""; | |
| 171 | for (let i = 0; i < 2; i++) { | |
| 172 | const res = await fetch(`${servers.url}${path}`, { | |
| 173 | headers: { host, ...(cookie === undefined ? {} : { cookie }) }, | |
| 174 | }); | |
| 175 | expect(res.status).toBe(200); | |
| 176 | await res.text(); | |
| 177 | header = res.headers.get("server-timing") ?? ""; | |
| 178 | } | |
| 179 | return new Map( | |
| 180 | header.split(", ").map((p) => { | |
| 181 | const [name, dur] = p.split(";dur="); | |
| 182 | return [name!, Number(dur)] as const; | |
| 183 | }), | |
| 184 | ); | |
| 185 | }; | |
| 186 | ||
| 187 | test("the door and the crew line name the phases they read, signed in, on production and a preview", async () => { | |
| 188 | const who = { id: alice, name: "alice", role: "admin" } as const; | |
| 189 | const name = `forge_grant_${record.id.slice(-8)}`; | |
| 190 | const grantFor = (env: string) => | |
| 191 | `${name}=${signGrant(dataDir, { app: record, env } as AppTarget, 60_000, Date.now(), who)}`; | |
| 192 | const hosts: [string, string][] = [ | |
| 193 | [`notes.${DOMAIN}`, grantFor("production")], | |
| 194 | [`notes--${pull}.${DOMAIN}`, grantFor(`pull-${pull}`)], | |
| 195 | ]; | |
| 196 | const reads: Record<string, string[]> = { | |
| 197 | "/__forge/": ["session", "app", "pull", "render"], | |
| 198 | "/__forge/crew": ["session", "app", "pull", "working", "render"], | |
| 199 | }; | |
| 200 | for (const [host, cookie] of hosts) | |
| 201 | for (const [path, names] of Object.entries(reads)) { | |
| 202 | const phases = await phasesOf(host, path, cookie); | |
| 203 | expect([...phases.keys()].sort()).toEqual([...names].sort()); | |
| 204 | for (const ms of phases.values()) expect(Number.isFinite(ms)).toBe(true); | |
| 205 | } | |
| 206 | }); | |
outdated
A wall-clock bound of 20 ms is a flake waiting on a busy runner. Assert the phase is a finite number instead. |
||
| 207 | ||
| 208 | test("a guest names session and render, and the crew line stops before the reads it never makes", async () => { | |
| 209 | for (const host of [`notes.${DOMAIN}`, `notes--${pull}.${DOMAIN}`]) { | |
| 210 | // No cookie: the crew line has no viewer, so no app, pull or working read. | |
| 211 | const crew = await phasesOf(host, "/__forge/crew"); | |
| 212 | expect([...crew.keys()].sort()).toEqual(["render", "session"]); | |
| 213 | for (const ms of crew.values()) expect(Number.isFinite(ms)).toBe(true); | |
| 214 | // The door still looks the app and pull up for its words. | |
| 215 | const page = await phasesOf(host, "/__forge/"); | |
| 216 | expect([...page.keys()].sort()).toEqual(["app", "pull", "render", "session"]); | |
| 217 | } | |
| 218 | }); |
new above
offline
The early return for a signed-in viewer who cannot read the repo is untested; add a case asserting only session, app and render appear.