trevato/forge

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

More
Forks1g fMapg aRuns11g rEventsFlows

A contrib pull's latest check reads as plain text at /<owner>/<repo>/pulls/<n>/check.txt #12146

masoncrew opened this

Check · Review · Land · Live — Its check is running. Next: reviewer, no review yet.

ConversationFiles
Ready to land

mason/forge:mason/12100-a-contrib-pull-s-latest-check-reads-as-p → main 4 commits fork

Review

Sign in to review this change.
9 files changed +239 −6
docs/features/contribute-token.md +2
@@ -19,3 +19,5 @@ or the MCP `start` tool with `action: "contribute"` (same arguments). The answer
1919 - **Claimed, not verified:** the agent name and origin are what the agent said. The forge makes a throwaway guest account per token (no password, no crew, no credits) so a pull has an author; the account stays after the token ends.
2020
2121 **The push opens the pull (#12013).** A push of a new `contrib/<agent>/<segment>/<topic>` branch opens a pull on the default branch (the tip commit's first line is the title, its whole message the body); a later push to the branch updates that pull. The push output prints, as `remote:` lines, the pull's URL and `checks (plain text)`, the URL of `GET /api/v1/repos/<owner>/<name>/pulls/<n>/checks?format=text` (`state:`, `head:`, `required:`). Refused with one line: a push to the default branch or another token's branch (the line names the branches the token may push), a token past its expiry (401), a push over 5 MiB (`CONTRIBUTE_PUSH_MAX_BYTES`; git says "pack exceeds maximum allowed size (5.00 MiB)"), and a new branch once the token has 5 pulls open has opened 20 in a day, the agent name has opened 25, or the repository has taken 30 in a day (the line carries the number). A push that creates more new branches than the token has room to open pulls is refused whole with both numbers. Opens are serialized per token (`inTurn`), shared by the API and the push, with the caps read inside the turn; a branch that loses a race to a cap is still pushed and the output says no pull was opened. Code: `src/actions/contribute_push.ts`; test: `test/contribute-push.test.ts`.
22
23 **The latest check as text (#12100).** `GET /<owner>/<repo>/pulls/<n>/check.txt` answers `text/plain`: `pending`, `green` or `red` on the first line, `head: <commit>` on the second, and on red the last 80 lines of the failing log; the pull's own token and anyone who can read the pull may ask, another pull's token gets 403 (`src/http/check_text.ts`, test `test/contribute-check-text.test.ts`).
src/core/contribute_token.ts +9 −4
@@ -177,17 +177,22 @@ export function allows(scope: ContributeScope, repo: { owner: string; name: stri
177177 if (ask.method === "POST" && /^\/[^/]+\/[^/]+\.git\/git-upload-pack$/.test(path)) return true;
178178 if (ask.method === "POST" && path === `${at}.git/git-receive-pack`) return true;
179179 if (ask.method === "POST" && path === `${api}/pulls`) return true;
180 // Its own pull's latest check as plain text (#12100).
181 if (read && ask.ownPull === true && path.startsWith(`${at}/pulls/`) && /^[1-9][0-9]{0,8}\/check\.txt$/.test(path.slice(`${at}/pulls/`.length)))
182 return true;
180183 if ((ask.method === "PATCH" || ask.method === "POST") && ask.ownPull === true && path.startsWith(`${api}/pulls/`)) return true;
181184 return false;
182185 }
183186
184187 /** The pull number a path names under `repo`'s pulls, or undefined. */
185188 export function pullNumberIn(path: string, repo: { owner: string; name: string }): number | undefined {
186 const head = `/api/v1/repos/${repo.owner}/${repo.name}/pulls/`.toLowerCase();
187189 const lower = path.toLowerCase();
188 if (!lower.startsWith(head)) return undefined;
189 const m = /^([1-9][0-9]{0,8})(?:\/|$)/.exec(lower.slice(head.length));
190 return m ? Number(m[1]) : undefined;
190 for (const head of [`/api/v1/repos/${repo.owner}/${repo.name}/pulls/`.toLowerCase(), `/${repo.owner}/${repo.name}/pulls/`.toLowerCase()]) {
191 if (!lower.startsWith(head)) continue;
192 const m = /^([1-9][0-9]{0,8})(?:\/|$)/.exec(lower.slice(head.length));
193 if (m) return Number(m[1]);
194 }
195 return undefined;
191196 }
192197
193198 // ---- the rows ----
src/core/reserved.ts +1
@@ -66,6 +66,7 @@ export const SYSTEM_NAMES: readonly string[] = [
6666 "big",
6767 "board",
6868 "boards",
69 "broker",
6970 "contribute",
7071 "csp-report",
7172 "deploy-to",
src/http/api.ts +1 −1
@@ -1101,7 +1101,7 @@ export function agentJson(agent: Agent) {
11011101 }
11021102
11031103 /** The live head of a pull — the branch tip, or the snapshot for a fork's. */
1104 async function pullHeadOid(ctx: Ctx, repo: Repo, pull: Pull): Promise<string> {
1104 export async function pullHeadOid(ctx: Ctx, repo: Repo, pull: Pull): Promise<string> {
11051105 const ref = pull.crossRepo
11061106 ? `refs/pull/${pull.number}/head`
11071107 : `refs/heads/${pull.headBranch}`;
src/http/check_text.ts added +62
@@ -0,0 +1,62 @@
1 /**
2 * A pull's latest check as plain text (#12100): `GET /:owner/:name/pulls/:n/check.txt`.
3 * The first line is the verdict (`pending`, `green` or `red`), the second the
4 * head commit, and on red the tail of the failing log. For an agent that has
5 * only curl. Who may read it is who may read the pull (`loadPull`); the log is
6 * withheld, as everywhere, from a reader who may not read runs' logs.
7 */
8
9 import { runLog } from "../actions/runs.ts";
10 import { checksForHead } from "../core/runs.ts";
11 import type { Run } from "../core/runs.ts";
12 import type { Pull } from "../core/pulls.ts";
13 import type { Repo } from "../core/repos.ts";
14 import { readRunLogTail } from "../runs/log.ts";
15 import { loadPull, pullHeadOid } from "./api.ts";
16 import type { Ctx } from "./context.ts";
17
18 /** The most log lines a red verdict carries. */
19 export const CHECK_TAIL_LINES = 80;
20
21 export type CheckVerdict = "pending" | "green" | "red";
22
23 /** What a run's state says about the commit: only `ok` is green; a stop or failure is red. */
24 export function verdictOf(latest: Pick<Run, "state"> | undefined): CheckVerdict {
25 if (latest === undefined || latest.state === "queued" || latest.state === "running") return "pending";
26 return latest.state === "ok" ? "green" : "red";
27 }
28
29 /** The text of a verdict: plain data in, plain text out. */
30 export function checkText(verdict: CheckVerdict, head: string, tail: readonly string[]): string {
31 const lines = [verdict, `head: ${head}`];
32 if (verdict === "red" && tail.length > 0) lines.push("", ...tail.slice(-CHECK_TAIL_LINES));
33 return `${lines.join("\n")}\n`;
34 }
35
36 /**
37 * The log's last lines. The log door shows it to the repository's owner and
38 * crew; the pull's own author may read the check of their own pull too, which
39 * is what an outside agent has no crew to do otherwise.
40 */
41 function tailOf(ctx: Ctx, repo: Repo, pull: Pull, run: Run): string[] {
42 const log = runLog(ctx.app, ctx.auth.user, repo, run.id, CHECK_TAIL_LINES);
43 if (!log.ok) return [];
44 if (!log.value.logHidden) return log.value.lines;
45 if (ctx.auth.user === undefined || ctx.auth.user.id !== pull.authorId) return [];
46 return readRunLogTail(ctx.app.config.dataDir, run.id, CHECK_TAIL_LINES).lines;
47 }
48
49 export async function pullCheckText(ctx: Ctx): Promise<Response> {
50 const found = loadPull(ctx);
51 if ("error" in found) return new Response("not found\n", { status: 404, headers: textHeaders });
52 const head = await pullHeadOid(ctx, found.repo, found.pull);
53 const latest = checksForHead(ctx.app.db, found.pull.id, head, 1)[0];
54 const verdict = verdictOf(latest);
55 const tail =
56 verdict === "red" && latest !== undefined
57 ? tailOf(ctx, found.repo, found.pull, latest)
58 : [];
59 return new Response(checkText(verdict, head, tail), { headers: textHeaders });
60 }
61
62 const textHeaders = { "content-type": "text/plain; charset=utf-8", "cache-control": "no-store" };
src/http/server.ts +2
@@ -509,6 +509,7 @@ import { movedResponse } from "./moved.ts";
509509 import { originGate } from "./origin_gate.ts";
510510 import { proposalGate } from "./proposal_gate.ts";
511511 import { contributeGate } from "./contribute_gate.ts";
512 import { pullCheckText } from "./check_text.ts";
512513 import { runGate } from "./run_gate.ts";
513514 import { viaGate } from "./via_gate.ts";
514515 import { checksRecheckRoutes } from "./checks_recheck.ts";
@@ -1364,6 +1365,7 @@ export function buildRouter(): Router<Handler> {
13641365 // The change's Files step: the diff, the review threads and the merge
13651366 // box, one tab under the change itself.
13661367 router.add("GET", "/:owner/:name/pulls/:number/files", pullFilesPage);
1368 router.add("GET", "/:owner/:name/pulls/:number/check.txt", pullCheckText);
13671369 router.add("GET", "/:owner/:name/pulls/:number/resolve", conflictsPage);
13681370 router.add("POST", "/:owner/:name/pulls/:number/resolve", conflictsFinish);
13691371 router.add("POST", "/:owner/:name/pulls/:number/resolve/propose", proposeSubmit);
test/contribute-check-text.test.ts added +147
@@ -0,0 +1,147 @@
1 /**
2 * A pull's latest check as plain text (#12100): `GET /<owner>/<repo>/pulls/<n>/check.txt`.
3 * A green and a red verdict (the red with its log tail, never past 80 lines),
4 * the pull's own contribute token reads it, another token's is refused, an
5 * unknown pull answers 404.
6 */
7 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
8 import { mkdtemp, rm } from "node:fs/promises";
9 import { tmpdir } from "node:os";
10 import { join } from "node:path";
11 import { mintContributeToken, setOutsideContributions } from "../src/actions/contribute_token.ts";
12 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
13 import { createApp, type App } from "../src/app.ts";
14 import { contribNamespace } from "../src/core/contribute_token.ts";
15 import { getPull } from "../src/core/pulls.ts";
16 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
17 import { createUser } from "../src/core/users.ts";
18 import { appendRunLog } from "../src/runs/log.ts";
19 import { checkText, verdictOf } from "../src/http/check_text.ts";
20 import { serve, type Servers } from "../src/http/server.ts";
21 import { gitClientEnv } from "./helpers/git-client.ts";
22 import { quietLogs } from "./helpers/globals.ts";
23
24 quietLogs();
25
26 describe("the text", () => {
27 test("the verdict leads, then the head; only red carries a tail, at most 80 lines", () => {
28 expect(verdictOf(undefined)).toBe("pending");
29 expect(verdictOf({ state: "running" })).toBe("pending");
30 expect(verdictOf({ state: "ok" })).toBe("green");
31 expect(verdictOf({ state: "failed" })).toBe("red");
32 expect(checkText("green", "abc", ["x"])).toBe("green\nhead: abc\n");
33 const tail = Array.from({ length: 200 }, (_, i) => `l${i}`);
34 const lines = checkText("red", "abc", tail).split("\n");
35 expect(lines.slice(0, 2)).toEqual(["red", "head: abc"]);
36 expect(lines.slice(3, -1)).toHaveLength(80);
37 expect(lines.at(-2)).toBe("l199");
38 });
39 });
40
41 describe("over HTTP", () => {
42 let dataDir: string, work: string;
43 let app: App, servers: Servers;
44 let seq = 0;
45
46 const git = async (args: string[], cwd = work) => {
47 const proc = Bun.spawn(["git", "-c", "credential.helper=", ...args], {
48 cwd,
49 env: gitClientEnv({ GIT_TERMINAL_PROMPT: "0" }),
50 stdout: "pipe",
51 stderr: "pipe",
52 });
53 const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]);
54 return { code: await proc.exited, out, err };
55 };
56
57 beforeAll(async () => {
58 dataDir = await mkdtemp(join(tmpdir(), "forge-check-text-"));
59 app = createApp({ dataDir, port: 0, host: "forge.test" });
60 servers = await serve(app);
61 const made = await createUser(app.db, {
62 origin: "seed",
63 trusted: true,
64 username: "alice",
65 email: "[email protected]",
66 password: "a-good-passphrase",
67 adminIfFirst: true,
68 });
69 if (!made.ok) throw new Error(made.error);
70 const r = await createRepo(app, made.value, { name: "lib", visibility: "public" });
71 if (!r.ok) throw new Error(String(r.error));
72 const seeded = await commitToBranch(app, made.value, r.value, {
73 branch: "main",
74 message: "Start",
75 files: [{ path: "README.md", content: "hi\n" }],
76 });
77 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
78 setOutsideContributions(app, made.value, r.value, true);
79 work = join(dataDir, "work");
80 expect((await git(["clone", `${servers.url}/alice/lib.git`, work], dataDir)).code).toBe(0);
81 await git(["config", "user.email", "[email protected]"]);
82 await git(["config", "user.name", "codex"]);
83 });
84
85 afterAll(async () => {
86 await servers.stop();
87 await rm(dataDir, { recursive: true, force: true });
88 });
89
90 /** A token and a pull it opened by pushing. */
91 const contributor = async () => {
92 const m = await mintContributeToken(app, { agent: "codex", origin: "test", repo: "alice/lib" }, `203.0.113.${++seq}`, "https://forge.test");
93 if (!m.ok) throw new Error(`mint: ${JSON.stringify(m.error)}`);
94 await Bun.write(join(work, `f${++seq}.md`), "x\n");
95 await git(["add", "."]);
96 await git(["commit", "-m", `Change ${seq}`]);
97 const user = (app.db.query("SELECT u.username FROM token t JOIN user u ON u.id = t.user_id WHERE t.id = ?1").get(m.value.id) as { username: string }).username;
98 const url = `${servers.url.replace("://", `://${user}:${m.value.token}@`)}/alice/lib.git`;
99 const pushed = await git(["push", url, `HEAD:contrib/${contribNamespace("codex", m.value.id)}/fix`]);
100 const number = Number(/opened pull #(\d+)/.exec(pushed.err)?.[1]);
101 expect(number).toBeGreaterThan(0);
102 const row = getPull(app.db, repoId(), number)!;
103 return { number, token: m.value.token, pull: { id: row.id, head_oid: row.headOid } };
104 };
105 const repoId = () => (app.db.query("SELECT id FROM repo WHERE name = 'lib'").get() as { id: string }).id;
106 const check = (tokenOrNone: string | undefined, n: number) =>
107 fetch(`${servers.url}/alice/lib/pulls/${n}/check.txt`, tokenOrNone ? { headers: { authorization: `Bearer ${tokenOrNone}` } } : {});
108 const ran = (c: { pull: { id: string; head_oid: string } }, state: "ok" | "failed", log: string[]) => {
109 const run = createRun(app.db, { repoId: repoId(), kind: "check", issueId: c.pull.id, commitOid: c.pull.head_oid });
110 for (const line of log) appendRunLog(app.config.dataDir, run.id, line);
111 claimRun(app.db, run.id);
112 finishRun(app.db, run.id, { state, summary: state });
113 };
114
115 test("green, then red with its tail; the floor is on", async () => {
116 const c = await contributor();
117 const none = await check(c.token, c.number);
118 expect(none.status).toBe(200);
119 expect(none.headers.get("content-type")).toBe("text/plain; charset=utf-8");
120 expect(none.headers.get("x-content-type-options")).toBe("nosniff");
121 expect((await none.text()).split("\n")[0]).toBe("pending");
122
123 ran(c, "ok", ["fine"]);
124 const green = (await (await check(c.token, c.number)).text()).split("\n");
125 expect(green[0]).toBe("green");
126 expect(green[1]).toBe(`head: ${c.pull.head_oid}`);
127
128 ran(c, "failed", Array.from({ length: 120 }, (_, i) => `step ${i} broke`));
129 const red = (await (await check(c.token, c.number)).text()).split("\n");
130 expect(red[0]).toBe("red");
131 expect(red[1]).toBe(`head: ${c.pull.head_oid}`);
132 const tail = red.slice(3).filter((l) => l !== "");
133 expect(tail.length).toBeLessThanOrEqual(80);
134 expect(tail.at(-1)).toBe("step 119 broke");
135 });
136
137 test("another pull's token is refused; an unknown pull is 404", async () => {
138 const mine = await contributor();
139 const other = await contributor();
140 ran(mine, "failed", ["secret-ish line"]);
141 const refused = await check(other.token, mine.number);
142 expect(refused.status).toBe(403);
143 expect(await refused.text()).not.toContain("secret-ish");
144 expect((await check(mine.token, 9999)).status).toBe(403);
145 expect((await check(undefined, 9999)).status).toBe(404);
146 });
147 });
test/pages-budget.test.ts +1 −1
@@ -44,7 +44,7 @@ const HEAVIER: Record<string, { statements?: number; git?: number }> = {
4444 };
4545
4646 /** Patterns with no fixture today: lower this as fixtures are added, never raise it silently. */
47 const MAX_UNFIXTURED = 147;
47 const MAX_UNFIXTURED = 148; // +1: /:owner/:name/pulls/:number/check.txt, the plain-text check (#12100), covered by test/contribute-check-text.test.ts
4848
4949 /** Streams and live fragments: no page. */
5050 const NOT_PAGES = new Set([

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs