Public: anyone can read this; the crew can change it.
GET /api/v1/repos/:owner/:name/runs carries an ETag and answers a repeat with 304 until a run in the page starts, moves or finishes #9158
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9148-get-api-v1-repos-owner-name-runs-carries → main 3 commits fork 2 threads · 2 unresolved
6 files
changed +227
−19
src/actions/runs.ts +11
| @@ -24,6 +24,7 @@ import { | ||
| 24 | 24 | cancelRun as markCancelled, |
| 25 | 25 | getRun, |
| 26 | 26 | listRuns as queryRuns, |
| 27 | runsMarker, | |
| 27 | 28 | type ListRunsOptions, |
| 28 | 29 | type Run, |
| 29 | 30 | setRunErrorLine, |
| @@ -170,6 +171,16 @@ export function listRepoRuns( | ||
| 170 | 171 | ); |
| 171 | 172 | } |
| 172 | 173 | |
| 174 | /** | |
| 175 | * What the runs list shows this reader, before the list is built: whether | |
| 176 | * they see cost and turns. Call it only for someone who may read runs. The | |
| 177 | * owner and a stranger never share it. | |
| 178 | */ | |
| 179 | export function runsViewKey(app: App, actor: User | undefined, repo: Repo, options: ListRunsOptions): string { | |
| 180 | const facts = canSeeOwnerFacts(app.db, repo, actor); | |
| 181 | return `${facts ? "f" : "-"}\n${runsMarker(app.db, repo.id, options)}`; | |
| 182 | } | |
| 183 | ||
| 173 | 184 | /** A run as the runs API lists it: the agent's name, and the pull or issue it is about. */ |
| 174 | 185 | export type NamedRun = Run & { |
| 175 | 186 | /** The state to show, worked out from what the run did (`runs/state.ts`). */ |
src/contract/openapi/routes/getApiV1ReposOwnerNameRuns.ts +1 −1
| @@ -1,4 +1,4 @@ | ||
| 1 | 1 | import { REPO, type Route, limitTo } from "../shared.ts"; |
| 2 | 2 | import { LIST_LIMIT_MAX } from "../../../core/issues.ts"; |
| 3 | 3 | |
| 4 | export const route: Route = { method: "get", path: `${REPO}/runs`, summary: "List a repository's runs, newest first: { items, next_cursor }; kind deploy or agent, state queued, running, ok, failed or cancelled (another is a 422), env an environment name, limit 1 to 200 (default 50), cursor a run id; each row carries `shown`, the state worked out from what the run did; a stranger's runs carry no cost, turns, agent session or node", auth: false, query: { kind: "string", state: "string", env: "string", limit: limitTo(LIST_LIMIT_MAX), cursor: "string" } }; | |
| 4 | export const route: Route = { method: "get", path: `${REPO}/runs`, summary: "List a repository's runs, newest first: { items, next_cursor }; kind deploy or agent, state queued, running, ok, failed or cancelled (another is a 422), env an environment name, limit 1 to 200 (default 50), cursor a run id; each row carries `shown`, the state worked out from what the run did; a stranger's runs carry no cost, turns, agent session or node; the answer carries a weak ETag (cache-control private, no-cache; vary authorization, cookie) over the page and the viewer, and a request with a matching If-None-Match gets an empty 304 until a run in the page starts, moves, spends or finishes", auth: false, query: { kind: "string", state: "string", env: "string", limit: limitTo(LIST_LIMIT_MAX), cursor: "string" } }; |
src/core/runs.ts +35 −7
| @@ -697,11 +697,7 @@ export interface ListRunsOptions { | ||
| 697 | 697 | cursor?: string | undefined; |
| 698 | 698 | } |
| 699 | 699 | |
| 700 | export function listRuns( | |
| 701 | db: Database, | |
| 702 | repoId: string, | |
| 703 | options: ListRunsOptions = {}, | |
| 704 | ): Run[] { | |
| 700 | function runsWhere(repoId: string, options: ListRunsOptions): { where: string; params: (string | number)[]; limit: number } { | |
| 705 | 701 | const limit = Math.min(Math.max(options.limit ?? 50, 1), 200); |
| 706 | 702 | const params: (string | number)[] = [repoId]; |
| 707 | 703 | let where = "repo_id = ?1"; |
| @@ -725,14 +721,46 @@ export function listRuns( | ||
| 725 | 721 | params.push(options.cursor); |
| 726 | 722 | where += ` AND id < ?${params.length}`; |
| 727 | 723 | } |
| 724 | return { where, params, limit }; | |
| 725 | } | |
| 726 | ||
| 727 | export function listRuns( | |
| 728 | db: Database, | |
| 729 | repoId: string, | |
| 730 | options: ListRunsOptions = {}, | |
| 731 | ): Run[] { | |
| 732 | const { where, params, limit } = runsWhere(repoId, options); | |
| 728 | 733 | params.push(limit); |
| 729 | const rows = db | |
| 734 | return db | |
| 730 | 735 | .query<RunRow, (string | number)[]>( |
| 731 | 736 | `SELECT * FROM run WHERE ${where} ORDER BY id DESC LIMIT ?${params.length}`, |
| 732 | 737 | ) |
| 733 | 738 | .all(...params) |
| 734 | 739 | .map(toRun); |
| 735 | return rows; | |
| 740 | } | |
| 741 | ||
| 742 | /** | |
| 743 | * A cheap marker over the page `listRuns` would give: each row's id and every | |
| 744 | * field a client sees move (state, timestamps, turns, cost, summary, error, | |
| 745 | * attempt), plus the title of the pull or issue it is about. The same page | |
| 746 | * gives the same marker; a run that starts, moves, finishes or spends more | |
| 747 | * changes it. One indexed read, no rows built. | |
| 748 | */ | |
| 749 | export function runsMarker(db: Database, repoId: string, options: ListRunsOptions = {}): string { | |
| 750 | const { where, params, limit } = runsWhere(repoId, options); | |
| 751 | params.push(limit); | |
| 752 | const rows = db | |
| 753 | .query<{ m: string }, (string | number)[]>( | |
| 754 | `SELECT r.id || '|' || COALESCE(r.state, '') || '|' || COALESCE(r.started_at, '') || '|' || COALESCE(r.finished_at, '') | |
| 755 | || '|' || COALESCE(r.serving_at, '') || '|' || COALESCE(r.turns, '') || '|' || COALESCE(r.cost_usd, '') || '|' || COALESCE(r.attempt, '') | |
| 756 | || '|' || COALESCE(r.not_before, '') || '|' || COALESCE(r.summary, '') || '|' || COALESCE(r.error_line, '') || '|' || COALESCE(r.error_at, '') | |
| 757 | || '|' || COALESCE(r.check_detail, '') || '|' || COALESCE(i.title, '') AS m | |
| 758 | FROM (SELECT * FROM run WHERE ${where} ORDER BY id DESC LIMIT ?${params.length}) r | |
| 759 | LEFT JOIN issue i ON i.id = r.issue_id | |
| 760 | ORDER BY r.id DESC`, | |
| 761 | ) | |
| 762 | .all(...params); | |
| 763 | return `${rows.length}\n${rows.map((r) => r.m).join("\n")}`; | |
| 736 | 764 | } |
| 737 | 765 | |
| 738 | 766 | /** Queued runs of one kind, oldest first — what a worker takes from. */ |
src/http/api.ts +30 −9
| @@ -222,7 +222,8 @@ import { | ||
| 222 | 222 | removeFromCrew, |
| 223 | 223 | } from "../actions/crew.ts"; |
| 224 | 224 | import { assignIssue, editIssue, TAKEN_UP } from "../actions/issues.ts"; |
| 225 | import { steerRepoRun } from "../actions/runs.ts"; | |
| 225 | import { requireReader } from "../actions/access.ts"; | |
| 226 | import { mayReadRuns, runsViewKey, steerRepoRun } from "../actions/runs.ts"; | |
| 226 | 227 | import { openPlan, parsePlan, proposePlan } from "../actions/plan.ts"; |
| 227 | 228 | import { repoGraph } from "../core/dependencies.ts"; |
| 228 | 229 | import { mapHeldFor } from "../crew/held.ts"; |
| @@ -264,6 +265,7 @@ import { | ||
| 264 | 265 | latestRunsForIssues, |
| 265 | 266 | listActiveRuns, |
| 266 | 267 | RUN_STATES, |
| 268 | type ListRunsOptions, | |
| 267 | 269 | type Run, |
| 268 | 270 | } from "../core/runs.ts"; |
| 269 | 271 | import { checkStale } from "../checks/stale.ts"; |
| @@ -3734,22 +3736,41 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3734 | 3736 | if (wanted !== null && state === undefined) |
| 3735 | 3737 | return apiError("invalid_state", 422, `state is one of ${RUN_STATES.join(", ")}`); |
| 3736 | 3738 | const limit = Math.min(Math.max(Number(q.get("limit")) || 50, 1), 200); |
| 3739 | const options: ListRunsOptions = { | |
| 3740 | ...(kind === "deploy" || kind === "agent" ? { kind } : {}), | |
| 3741 | ...(env !== null && isEnvName(env) ? { env } : {}), | |
| 3742 | ...(state === undefined ? {} : { state }), | |
| 3743 | ...(q.get("cursor") ? { cursor: q.get("cursor")! } : {}), | |
| 3744 | limit, | |
| 3745 | }; | |
| 3746 | // The tag: repository, viewer, what shapes the page, and a marker over | |
| 3747 | // the page's rows (every field a row shows), so a repeat is a 304 before | |
| 3748 | // the list is built. | |
| 3749 | // Someone who may not read the repository gets the 404 of the list path, | |
| 3750 | // never a tag or a 304; someone who may not read its runs gets the list | |
| 3751 | // as it is (empty), untagged. | |
| 3752 | if (!requireReader(ctx.app, ctx.auth.user, repo).ok) return notFound(); | |
| 3753 | const viewer = ctx.auth.user === undefined ? "anonymous" : ctx.auth.user.id; | |
| 3754 | let headers: Record<string, string> = {}; | |
| 3755 | if (mayReadRuns(ctx.app, ctx.auth.user, repo)) { | |
outdated
Check |
||
| 3756 | const etag = timePhase(ctx.request, "tag", () => | |
| 3757 | weakEtag( | |
| 3758 | [repo.id, viewer, JSON.stringify(options), runsViewKey(ctx.app, ctx.auth.user, repo, options)].join("\n"), | |
| 3759 | ), | |
| 3760 | ); | |
| 3761 | headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" }; | |
| 3762 | if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers }); | |
| 3763 | } | |
| 3737 | 3764 | // Already as this reader may see them (`runForReader`): a stranger's |
| 3738 | 3765 | // runs carry no cost, turns, agent session or node. Each row also names |
| 3739 | 3766 | // its agent and its pull or issue (`nameRuns`), so a client never parses |
| 3740 | 3767 | // the branch. |
| 3741 | 3768 | const runs = timePhase(ctx.request, "runs", () => { |
| 3742 | const listed = listRepoRuns(ctx.app, ctx.auth.user, repo, { | |
| 3743 | ...(kind === "deploy" || kind === "agent" ? { kind } : {}), | |
| 3744 | ...(env !== null && isEnvName(env) ? { env } : {}), | |
| 3745 | ...(state === undefined ? {} : { state }), | |
| 3746 | ...(q.get("cursor") ? { cursor: q.get("cursor")! } : {}), | |
| 3747 | limit, | |
| 3748 | }); | |
| 3769 | const listed = listRepoRuns(ctx.app, ctx.auth.user, repo, options); | |
| 3749 | 3770 | return listed.ok ? nameRuns(ctx.app, listed.value) : undefined; |
| 3750 | 3771 | }); |
| 3751 | 3772 | if (runs === undefined) return notFound(); |
| 3752 | return timePhase(ctx.request, "render", () => json(paged(runs, limit, (r) => r.id))); | |
| 3773 | return timePhase(ctx.request, "render", () => json(paged(runs, limit, (r) => r.id), 200, headers)); | |
| 3753 | 3774 | }); |
| 3754 | 3775 | |
| 3755 | 3776 | router.add("GET", "/api/v1/repos/:owner/:name/runs/:id", (ctx) => { |
test/api-runs-timing.test.ts +2 −2
| @@ -56,10 +56,10 @@ async function phases(path: string): Promise<{ status: number; names: string[]; | ||
| 56 | 56 | return { status: res.status, names: names.slice(names.indexOf("app") + 1), body }; |
| 57 | 57 | } |
| 58 | 58 | |
| 59 | test("the runs list names session, runs, render in order", async () => { | |
| 59 | test("the runs list names session, tag, runs, render in order", async () => { | |
| 60 | 60 | const got = await phases("/api/v1/repos/alice/r/runs"); |
| 61 | 61 | expect(got.status).toBe(200); |
| 62 | expect(got.names).toEqual(["session", "runs", "render"]); | |
| 62 | expect(got.names).toEqual(["session", "tag", "runs", "render"]); | |
| 63 | 63 | expect(Array.isArray(JSON.parse(got.body).items)).toBe(true); |
| 64 | 64 | }); |
| 65 | 65 |
test/runs-etag.test.ts added +148
| @@ -0,0 +1,148 @@ | ||
| 1 | /** GET …/runs carries an ETag over the page; a repeat is a 304 until a run in it moves (#9148). */ | |
| 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 { createSession } from "../src/core/sessions.ts"; | |
| 9 | import { createUser } from "../src/core/users.ts"; | |
| 10 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 11 | import { quietLogs } from "./helpers/globals.ts"; | |
| 12 | ||
| 13 | quietLogs(); | |
| 14 | ||
| 15 | let dataDir: string; | |
| 16 | let app: App; | |
| 17 | let servers: Servers; | |
| 18 | let owner = ""; | |
| 19 | let stranger = ""; | |
| 20 | let repoId = ""; | |
| 21 | const cells = process.env["FORGE_CELLS"]; | |
| 22 | ||
| 23 | beforeAll(async () => { | |
| 24 | process.env["FORGE_CELLS"] = "off"; | |
| 25 | dataDir = await mkdtemp(join(tmpdir(), "forge-runs-etag-")); | |
| 26 | app = createApp({ dataDir, port: 0 }); | |
| 27 | servers = await serve(app); | |
| 28 | const made = await createUser(app.db, { | |
| 29 | origin: "seed", trusted: true, username: "alice", email: "[email protected]", | |
| 30 | password: "a-good-passphrase", adminIfFirst: true, | |
| 31 | }); | |
| 32 | if (!made.ok) throw new Error(String(made.error)); | |
| 33 | owner = `forge_session=${createSession(app.db, made.value.id).token}`; | |
| 34 | const other = await createUser(app.db, { | |
| 35 | origin: "seed", trusted: true, username: "bob", email: "[email protected]", password: "a-good-passphrase", | |
| 36 | }); | |
| 37 | if (!other.ok) throw new Error(String(other.error)); | |
| 38 | stranger = `forge_session=${createSession(app.db, other.value.id).token}`; | |
| 39 | const repo = await createRepo(app, made.value, { name: "r", visibility: "public" }); | |
| 40 | if (!repo.ok) throw new Error(String(repo.error)); | |
| 41 | repoId = repo.value.id; | |
| 42 | }, 120_000); | |
| 43 | ||
| 44 | afterAll(async () => { | |
| 45 | await servers.stop(); | |
| 46 | app.close(); | |
| 47 | if (cells === undefined) delete process.env["FORGE_CELLS"]; | |
| 48 | else process.env["FORGE_CELLS"] = cells; | |
| 49 | await rm(dataDir, { recursive: true, force: true }); | |
| 50 | }); | |
| 51 | ||
| 52 | const get = (c: string | undefined, tag?: string, query = "") => | |
| 53 | fetch(`${servers.url}/api/v1/repos/alice/r/runs${query}`, { | |
| 54 | headers: { ...(c ? { cookie: c } : {}), ...(tag === undefined ? {} : { "if-none-match": tag }) }, | |
| 55 | }); | |
| 56 | ||
| 57 | const add = (id: string, state: string) => | |
| 58 | app.db.query("INSERT INTO run (id, repo_id, kind, state) VALUES (?, ?, 'deploy', ?)").run(id, repoId, state); | |
| 59 | ||
| 60 | test("a repeat is a 304 with no body; headers say private, no-cache", async () => { | |
| 61 | add("run-001", "queued"); | |
| 62 | const first = await get(owner); | |
| 63 | expect(first.status).toBe(200); | |
| 64 | const tag = first.headers.get("etag")!; | |
| 65 | expect(tag).toMatch(/^W\//); | |
| 66 | expect(first.headers.get("cache-control")).toBe("private, no-cache"); | |
| 67 | expect(first.headers.get("vary")).toContain("authorization, cookie"); | |
| 68 | await first.text(); | |
| 69 | const again = await get(owner, tag); | |
| 70 | expect(again.status).toBe(304); | |
| 71 | expect(await again.text()).toBe(""); | |
| 72 | expect(again.headers.get("server-timing") ?? "").toContain("tag"); | |
| 73 | expect(again.headers.get("server-timing") ?? "").not.toContain("runs"); | |
| 74 | }); | |
| 75 | ||
| 76 | test("a new run, then queued → running → ok → each changes the tag", async () => { | |
| 77 | let tag = (await get(owner)).headers.get("etag")!; | |
| 78 | const step = async (sql: string) => { | |
| 79 | app.db.query(sql).run(); | |
| 80 | const r = await get(owner, tag); | |
| 81 | expect(r.status).toBe(200); | |
| 82 | const next = r.headers.get("etag")!; | |
| 83 | expect(next).not.toBe(tag); | |
| 84 | await r.text(); | |
| 85 | tag = next; | |
| 86 | }; | |
| 87 | add("run-002", "queued"); | |
| 88 | const added = await get(owner, tag); | |
| 89 | expect(added.status).toBe(200); | |
| 90 | tag = added.headers.get("etag")!; | |
| 91 | await added.text(); | |
| 92 | await step("UPDATE run SET state='running', started_at='2026-01-01T00:00:00Z' WHERE id='run-002'"); | |
| 93 | await step("UPDATE run SET turns=3, cost_usd=0.5 WHERE id='run-002'"); | |
| 94 | await step("UPDATE run SET state='ok', finished_at='2026-01-01T00:01:00Z' WHERE id='run-002'"); | |
| 95 | }); | |
| 96 | ||
| 97 | test("a filter keeps its tag when a run that does not match changes", async () => { | |
| 98 | const first = await get(owner, undefined, "?state=ok"); | |
| 99 | const tag = first.headers.get("etag")!; | |
| 100 | await first.text(); | |
| 101 | app.db.query("UPDATE run SET state='running' WHERE id='run-001'").run(); | |
| 102 | expect((await get(owner, tag, "?state=ok")).status).toBe(304); | |
| 103 | app.db.query("UPDATE run SET state='ok' WHERE id='run-001'").run(); | |
| 104 | expect((await get(owner, tag, "?state=ok")).status).toBe(200); | |
| 105 | }); | |
| 106 | ||
| 107 | test("owner and stranger tags differ; bad state is a 422 and unknown repo a 404, untagged", async () => { | |
| 108 | const a = await get(owner); | |
| 109 | const b = await get(stranger); | |
| 110 | expect(a.headers.get("etag")).not.toBe(b.headers.get("etag")); | |
| 111 | await a.text(); await b.text(); | |
| 112 | const bad = await get(owner, undefined, "?state=nope"); | |
| 113 | expect(bad.status).toBe(422); | |
| 114 | expect(bad.headers.get("etag")).toBeNull(); | |
| 115 | const missing = await fetch(`${servers.url}/api/v1/repos/alice/none/runs`, { headers: { cookie: owner } }); | |
| 116 | expect(missing.status).toBe(404); | |
| 117 | expect(missing.headers.get("etag")).toBeNull(); | |
| 118 | }); | |
| 119 | ||
| 120 | test("a filter's page is its cursor and limit: a newer run that does not match moves the page", async () => { | |
| 121 | add("run-900", "ok"); | |
| 122 | const first = await get(owner, undefined, "?state=ok&limit=1"); | |
| 123 | const tag = first.headers.get("etag")!; | |
| 124 | await first.text(); | |
| 125 | add("run-950", "failed"); | |
| 126 | expect((await get(owner, tag, "?state=ok&limit=1")).status).toBe(304); | |
| 127 | add("run-960", "ok"); | |
| 128 | expect((await get(owner, tag, "?state=ok&limit=1")).status).toBe(200); | |
| 129 | }); | |
| 130 | ||
| 131 | test("a reader of a private repository's runs only: a stranger gets 404 and no tag, with any If-None-Match", async () => { | |
| 132 | const made = app.db.query("SELECT owner_id AS o FROM repo WHERE id = ?").get(repoId) as { o: string }; | |
| 133 | expect(made).toBeTruthy(); | |
| 134 | app.db.query("UPDATE repo SET visibility = 'private' WHERE id = ?").run(repoId); | |
| 135 | try { | |
| 136 | const ownerTag = (await get(owner)).headers.get("etag")!; | |
| 137 | for (const tag of [ownerTag, 'W/"x"', "*"]) { | |
| 138 | const r = await get(stranger, tag); | |
| 139 | expect(r.status).toBe(404); | |
| 140 | expect(r.headers.get("etag")).toBeNull(); | |
| 141 | } | |
| 142 | const anon = await get(undefined, ownerTag); | |
| 143 | expect(anon.status).toBe(404); | |
| 144 | expect(anon.headers.get("etag")).toBeNull(); | |
| 145 | } finally { | |
| 146 | app.db.query("UPDATE repo SET visibility = 'public' WHERE id = ?").run(repoId); | |
| 147 | } | |
| 148 | }); |
new above
offline
One NULL column here makes the whole marker NULL and the row stops contributing to the tag. COALESCE every nullable column, or confirm each is NOT NULL.