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

The API's run log read names its Server-Timing phases: session, log, render #3793

Merged opened this

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

Merged

scout/forge:scout/3782-the-api-s-run-log-read-names-its-server → main 3 commits fork 1 thread · 1 unresolved

  • Merged. scout/3782-the-api-s-run-log-read-names-its-server was merged into main as f2fa749.

Review

Sign in to review this change.
2 files changed +61 −2
src/http/api.ts +6 −2
@@ -5640,15 +5640,19 @@ export function apiRoutes(router: Router<Handler>): void {
56405640 // an `Accept: text/event-stream`). Lines already written come first,
56415641 // then each new one as it lands; `done` carries the finished run.
56425642 router.add("GET", "/api/v1/repos/:owner/:name/runs/:id/log", (ctx) => {
5643 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
56435644 const repo = loadRepo(ctx);
56445645 if (!repo) return notFound();
5645 const found = runLog(ctx.app, ctx.auth.user, repo, ctx.params.id ?? "");
5646 const found = timePhase(ctx.request, "log", () =>
5647 runLog(ctx.app, ctx.auth.user, repo, ctx.params.id ?? ""),
5648 );

Good: runLog still runs with the caller's identity inside the timed phase, so authorization and the logHidden 404 are unchanged.

56465649 if (!found.ok || found.value.logHidden) return notFound();
56475650 const follow =
56485651 ctx.url.searchParams.get("follow") === "1" ||
56495652 (ctx.request.headers.get("accept") ?? "").includes("text/event-stream");
56505653 if (!follow) {
5651 return new Response(`${found.value.lines.join("\n")}\n`, {
5654 const body = timePhase(ctx.request, "render", () => `${found.value.lines.join("\n")}\n`);
5655 return new Response(body, {
56525656 headers: {
56535657 "content-type": "text/plain; charset=utf-8",
56545658 "cache-control": "no-store",
test/api-run-log-timing.test.ts added +55
@@ -0,0 +1,55 @@
1 /** The API's run log read names its phases in Server-Timing (#3782). */
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 { createRepo } from "../src/actions/repos.ts";
8 import { createRun, finishRun } from "../src/core/runs.ts";
9 import { serve, type Servers } from "../src/http/server.ts";
10 import { seedAdmin } from "./helpers/admin.ts";
11
12 let dataDir = "";
13 let app: App, servers: Servers, bearer = "", runId = "";
14
15 beforeAll(async () => {
16 dataDir = await mkdtemp(join(tmpdir(), "forge-apilogt-"));
17 app = createApp({ dataDir, port: 0 });
18 servers = await serve(app);
19 const admin = await seedAdmin(app, { username: "alice", email: "[email protected]", password: "correct-horse-battery" });
20 if (!admin.ok) throw new Error(admin.error);
21 const made = await createRepo(app, admin.value, { name: "notes", visibility: "public", description: "" });
22 if (!made.ok) throw new Error(String(made.error));
23 const run = createRun(app.db, { repoId: made.value.id, kind: "check" });
24 runId = run.id;
25 finishRun(app.db, runId, { state: "ok", summary: "fine" });
26 const minted = await fetch(`${servers.url}/api/v1/tokens`, {
27 method: "POST",
28 headers: { "content-type": "application/json" },
29 body: JSON.stringify({ adminConfirm: true, username: "alice", password: "correct-horse-battery", name: "cli", scope: "admin" }),
30 });
31 bearer = ((await minted.json()) as { token: string }).token;
32 });
33
34 afterAll(async () => {
35 servers.stop();
36 app.close();
37 await rm(dataDir, { recursive: true, force: true });
38 });
39
40 const names = (res: Response): string[] =>
41 (res.headers.get("server-timing") ?? "").split(", ").map((p) => p.split(";")[0]!);
42
43 test("a finished run's plain log names session, log and render once each", async () => {
44 const res = await fetch(`${servers.url}/api/v1/repos/alice/notes/runs/${runId}/log`, { headers: { authorization: `Bearer ${bearer}` } });
45 expect(res.status).toBe(200);
46 expect(res.headers.get("content-type")).toContain("text/plain");
47 await res.text();
48 for (const n of ["session", "log", "render"]) expect(names(res).filter((x) => x === n).length).toBe(1);
49 });
50
51 test("a missing run answers 404 and still names session", async () => {
52 const res = await fetch(`${servers.url}/api/v1/repos/alice/notes/runs/nope/log`, { headers: { authorization: `Bearer ${bearer}` } });
53 expect(res.status).toBe(404);
54 expect(names(res)).toContain("session");
55 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs