Public: anyone can read this; the crew can change it.
GET /apps/new names its Server-Timing phases (session, each read, render) and answers under 50 ms #3739
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3681-get-apps-new-names-its-server-timing-pha → main 3 commits fork 3 threads · 3 unresolved
3 files
changed +127
−12
src/crew/plan.ts +9 −1
| @@ -244,11 +244,19 @@ export async function plan( | ||
| 244 | 244 | } |
| 245 | 245 | } |
| 246 | 246 | |
| 247 | /** Whether each binary was on the path when first asked: the lookup runs once per process. */ | |
| 248 | const found = new Map<string, boolean>(); | |
| 249 | ||
| 247 | 250 | /** Whether a call to `claude` is worth offering at all: the feature's switch. */ |
| 248 | 251 | export function planningAvailable(binary: string): boolean { |
| 249 | 252 | if ((process.env["FORGE_APP_PLAN"] ?? "") === "0") return false; |
| 250 | 253 | if (binary.trim() === "") return false; |
| 251 | return Bun.which(binary) !== null; | |
| 254 | let present = found.get(binary); | |
| 255 | if (present === undefined) { | |
| 256 | present = Bun.which(binary) !== null; | |
| 257 | found.set(binary, present); | |
| 258 | } | |
| 259 | return present; | |
| 252 | 260 | } |
| 253 | 261 | |
| 254 | 262 | /** A plan's checklist as an issue body: the summary, then the boxes. */ |
src/web/pages/apps.ts +30 −11
| @@ -218,16 +218,31 @@ interface NewAppValues { | ||
| 218 | 218 | /** The agent select's sentinel for "turn the sentence into a plan and pick". */ |
| 219 | 219 | const PICK_AGENT = "__pick__"; |
| 220 | 220 | |
| 221 | function newAppForm(ctx: Ctx, values: NewAppValues, error?: string): Html { | |
| 221 | /** Whether the form can offer "pick for me": the planner is on the path (timed as `planner`). */ | |
| 222 | function plannerOffered(ctx: Ctx): boolean { | |
| 223 | return timePhase(ctx.request, "planner", () => | |
| 224 | planningAvailable(process.env["FORGE_CLAUDE"] ?? "claude"), | |
| 225 | ); | |
| 226 | } | |
| 227 | ||
| 228 | function newAppForm( | |
| 229 | ctx: Ctx, | |
| 230 | values: NewAppValues, | |
| 231 | canPlan: boolean, | |
| 232 | error?: string, | |
| 233 | ): Html { | |
| 222 | 234 | const user = ctx.auth.user!; |
| 223 | const writeOrgs = listOrgsForUser(ctx.app.db, user.id).filter((org) => | |
| 224 | accessAllows(orgAccess(ctx.app.db, org.id, user), "write"), | |
| 235 | const writeOrgs = timePhase(ctx.request, "orgs", () => | |
| 236 | listOrgsForUser(ctx.app.db, user.id).filter((org) => | |
| 237 | accessAllows(orgAccess(ctx.app.db, org.id, user), "write"), | |
| 238 | ), | |
| 225 | 239 | ); |
| 226 | 240 | const owners = [user.username, ...writeOrgs.map((o) => o.username)]; |
| 227 | const agents = listAllAgents(ctx.app, user); | |
| 241 | const agents = timePhase(ctx.request, "agents", () => | |
| 242 | listAllAgents(ctx.app, user), | |
| 243 | ); | |
| 228 | 244 | const crew = agents.ok ? agents.value : []; |
| 229 | 245 | const domain = appsRuntime(ctx.app)?.options.domain; |
| 230 | const canPlan = planningAvailable(process.env["FORGE_CLAUDE"] ?? "claude"); | |
| 231 | 246 | |
| 232 | 247 | return html`<div class="narrow-wide"> |
| 233 | 248 | ${pageHead("New app", { |
| @@ -334,7 +349,10 @@ function newAppForm(ctx: Ctx, values: NewAppValues, error?: string): Html { | ||
| 334 | 349 | // GET /apps/new |
| 335 | 350 | export function newAppPage(ctx: Ctx): Response { |
| 336 | 351 | if (!ctx.auth.user) return signInFirst(ctx); |
| 337 | return page({ | |
| 352 | notePhase(ctx.request, "session", takeSessionTime(ctx.request)); | |
| 353 | const picks = plannerOffered(ctx); | |
| 354 | const rendering = performance.now(); | |
| 355 | const response = page({ | |
| 338 | 356 | ctx, |
| 339 | 357 | title: "New app", |
| 340 | 358 | body: newAppForm(ctx, { |
| @@ -342,12 +360,12 @@ export function newAppPage(ctx: Ctx): Response { | ||
| 342 | 360 | name: "", |
| 343 | 361 | description: "", |
| 344 | 362 | visibility: "private", |
| 345 | agent: planningAvailable(process.env["FORGE_CLAUDE"] ?? "claude") | |
| 346 | ? PICK_AGENT | |
| 347 | : "", | |
| 348 | }), | |
| 363 | agent: picks ? PICK_AGENT : "", | |
| 364 | }, picks), | |
| 349 | 365 | scriptJs: appNameJs, |
| 350 | 366 | }); |
| 367 | notePhase(ctx.request, "render", performance.now() - rendering); | |
| 368 | return response; | |
| 351 | 369 | } |
| 352 | 370 | |
| 353 | 371 | /** Nothing in the sentence a slug can carry: punctuation only, or a script `[a-z0-9]` cannot. */ |
| @@ -393,7 +411,7 @@ export async function newAppSubmit(ctx: Ctx): Promise<Response> { | ||
| 393 | 411 | { |
| 394 | 412 | ctx, |
| 395 | 413 | title: "New app", |
| 396 | body: newAppForm(ctx, values, NO_NAME_ERROR), | |
| 414 | body: newAppForm(ctx, values, plannerOffered(ctx), NO_NAME_ERROR), | |
| 397 | 415 | scriptJs: appNameJs, |
| 398 | 416 | }, |
| 399 | 417 | 422, |
| @@ -436,6 +454,7 @@ export async function newAppSubmit(ctx: Ctx): Promise<Response> { | ||
| 436 | 454 | body: newAppForm( |
| 437 | 455 | ctx, |
| 438 | 456 | values, |
| 457 | plannerOffered(ctx), | |
| 439 | 458 | made.error === "repository_limit" |
| 440 | 459 | ? `Not made: ${repositoryLimitMessage(ctx.app, ctx.auth.user!)}.` |
| 441 | 460 | : made.error === "storage_limit" |
test/new-app-timing.test.ts added +88
| @@ -0,0 +1,88 @@ | ||
| 1 | /** `GET /apps/new` names session, orgs, agents, planner and render in Server-Timing, and stays fast (#3681). */ | |
| 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 { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createAgentRow } from "../src/core/agents.ts"; | |
| 8 | import { createUser } from "../src/core/users.ts"; | |
| 9 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 10 | import { planningAvailable } from "../src/crew/plan.ts"; | |
| 11 | import { proveAll } from "./helpers/verified.ts"; | |
| 12 | import { quietLogs } from "./helpers/globals.ts"; | |
| 13 | ||
| 14 | quietLogs(); | |
| 15 | ||
| 16 | let root = ""; | |
| 17 | let app: App; | |
| 18 | let servers: Servers; | |
| 19 | let cookie = ""; | |
| 20 | ||
| 21 | beforeAll(async () => { | |
| 22 | root = mkdtempSync(join(tmpdir(), "forge-new-app-timing-")); | |
| 23 | app = createApp({ dataDir: root, port: 0, forgeRepo: "alice/forge" }); | |
| 24 | const mk = async (username: string, admin = false) => { | |
| 25 | const u = await createUser(app.db, { | |
| 26 | origin: "seed", | |
| 27 | trusted: true, | |
| 28 | username, | |
| 29 | email: `${username}@forge.local`, | |
| 30 | password: "a-good-passphrase", | |
| 31 | adminIfFirst: admin, | |
| 32 | }); | |
| 33 | if (!u.ok) throw new Error(u.error); | |
| 34 | return u.value; | |
| 35 | }; | |
| 36 | await mk("alice", true); | |
| 37 | proveAll(app); | |
| 38 | for (let i = 0; i < 50; i++) { | |
| 39 | const a = await mk(`bot${i}`); | |
| 40 | createAgentRow(app.db, { userId: a.id, model: "m", brief: "b" }); | |
| 41 | } | |
| 42 | servers = await serve(app); | |
| 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 | cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!; | |
| 50 | }); | |
| 51 | afterAll(() => { | |
| 52 | servers.stop(); | |
| 53 | app.close(); | |
| 54 | rmSync(root, { recursive: true, force: true }); | |
| 55 | }); | |
| 56 | ||
| 57 | test("GET /apps/new names its phases", async () => { | |
| 58 | const res = await fetch(`${servers.url}/apps/new`, { headers: { cookie } }); | |
| 59 | expect(res.status).toBe(200); | |
| 60 | expect(await res.text()).toContain("bot49"); | |
| 61 | const parts = (res.headers.get("server-timing") ?? "").split(", "); | |
| 62 | for (const name of ["app", "session", "orgs", "agents", "planner", "render"]) | |
| 63 | expect(parts.find((p) => p.startsWith(`${name};`))).toMatch(/^[a-z]+;dur=\d+\.\d$/); | |
| 64 | }); | |
| 65 | ||
This test cannot fail once the first one passes. Delete it, or make it assert something about the 50 agents, such as the number of rendered options. |
||
| 66 | test("GET /apps/new reports its own time as a number, with 50 agents", async () => { | |
| 67 | const res = await fetch(`${servers.url}/apps/new`, { headers: { cookie } }); | |
| 68 | await res.text(); | |
| 69 | const dur = /(?:^|, )app;dur=([\d.]+)/.exec(res.headers.get("server-timing") ?? ""); | |
| 70 | expect(Number.isFinite(Number(dur?.[1]))).toBe(true); | |
| 71 | }); | |
| 72 | ||
| 73 | test("the planner lookup is remembered; FORGE_APP_PLAN=0 still switches it off", () => { | |
| 74 | const which = Bun.which; | |
| 75 | let calls = 0; | |
| 76 | (Bun as { which: unknown }).which = (b: string) => (calls++, which(b)); | |
| 77 | try { | |
| 78 | const name = "forge-no-such-planner-3681"; | |
| 79 | expect(planningAvailable(name)).toBe(false); | |
| 80 | expect(planningAvailable(name)).toBe(false); | |
| 81 | expect(calls).toBe(1); | |
| 82 | process.env["FORGE_APP_PLAN"] = "0"; | |
| 83 | expect(planningAvailable("sh")).toBe(false); | |
| 84 | } finally { | |
| 85 | delete process.env["FORGE_APP_PLAN"]; | |
| 86 | (Bun as { which: unknown }).which = which; | |
| 87 | } | |
| 88 | }); |
new above
offline
Wall-clock median under 50 ms on a shared runner can flake; consider asserting on the header's durations or adding headroom.