Sign in

flagon-io/g1t

public

Commit

Acceptance checks in sandboxes, line comments and review verdicts

Checks - an issue's acceptance checks now run. When a pull request for the issue is opened ready, marked ready, or its head moves, the runner starts a sandbox holding only that commit, runs each command and records whether it passed and what it printed - only that sandbox can report: each run has a one-time token that the agent being checked never sees - merging is refused until the checks pass, unless a workspace member chooses to ignore them - checks.completed and pull.updated events Review - comments can sit on a line of a pull request's change, shown under that line in the Changes tab, with a button on each line to add one - approve and request-changes verdicts, for people and agents; nobody can give one on their own pull request Also - sign out did nothing: the menu closed and removed the submit button before it could submit. The menu item now submits directly - the site's type-check uses a full build; the incremental one missed changes in the shared contracts

syntaqxcommitted Parent4e001d6Browse files
35 files+1806−1450/35 viewed
+3−0
918918 dependencies = [
919919 "g1t-contracts",
920920 "g1t-kit",
921+ "getrandom 0.2.17",
922+ "hex",
921923 "serde",
922924 "serde_json",
925+ "sha2 0.10.9",
923926 "worker",
924927 ]
925928
+9−4
2929 - Pull requests with a diff and a recorded agent session: in a
3030 copy-on-write fork, which is how agents work, or from a branch pushed to
3131 the repository. Several can be made for one issue.
32+- Acceptance checks: an issue's commands are run against each pull request
33+ in a clean sandbox, by g1t and not by the agent being checked, and gate
34+ the merge.
35+- Review: comments on lines of a change, and approve or request-changes
36+ verdicts, from people and from agents.
3237 - Merging: lands a pull request on `main`, closes its issue naming the pull
3338 request that resolved it, and closes the others for that issue as
3439 superseded. Refused when the pull request is behind, so no commit is lost.
3843 - An event bus: every state change is published, logged and delivered to
3944 subscribers.
4045
41−Not built yet: server-side merge commits, review comments on lines, running acceptance checks, git over SSH. See the build order in the plan.
46+Not built yet: server-side merge commits, required reviews, git over SSH. See the build order in the plan.
4247
4348 ## Try it
4449
6368 | `apps/api` | REST API and MCP server. Rust. |
6469 | `services/identity` | Accounts, workspaces, sessions, keys and tokens. Rust. |
6570 | `services/repos` | Repository registry, contents, forks, diffs, landing, git over HTTPS. Rust. |
66−| `services/work` | Issues, pull requests, comments and sessions. Rust. |
71+| `services/work` | Issues, pull requests, reviews, check runs and sessions. Rust. |
6772 | `services/events` | The event bus and its log. Rust. |
68−| `services/runner` | Starts the sandboxes g1t agents work in. |
69−| `crates/runner` | The program inside a sandbox: runs the agent and reports back. Rust. |
73+| `services/runner` | Starts sandboxes: for g1t agents, and for acceptance checks. |
74+| `crates/runner` | The program inside a sandbox: runs an agent, or a set of checks, and reports back. Rust. |
7075 | `crates/contracts` | Types and service interfaces for the Rust services. |
7176 | `crates/kit` | Plumbing shared by Rust services on Workers. |
7277 | `crates/sshd` | Git over SSH, bridged to Artifacts. Not deployed yet. |
+33−0
1313 use g1t_contracts::identity::{
1414 DeviceClaim, DeviceClaimArgs, DeviceStart, DeviceStartArgs, TokenArgs,
1515 };
16+use g1t_contracts::work::{CheckRun, ReportChecksArgs};
1617 use g1t_contracts::{Failure, FailureCode, Outcome, Viewer};
1718 use serde_json::{Value, json};
1819 use worker::{Context, Env, Method, Request, Response, Result, event};
104105 "pulls_url": format!("{repo}/pulls{{?state}}"),
105106 "pull_url": format!("{repo}/pulls/{{number}}"),
106107 "pull_changes_url": format!("{repo}/pulls/{{number}}/changes"),
108+ "pull_reviews_url": format!("{repo}/pulls/{{number}}/reviews"),
107109 "pull_session_url": format!("{repo}/pulls/{{number}}/session{{?after}}"),
108110 "device_code_url": format!("{API}/v1/device/code"),
109111 "device_token_url": format!("{API}/v1/device/token"),
158160 })
159161 }
160162
163+/// A sandbox reporting on its run of a pull request's acceptance checks.
164+/// The run's own token, in the body, is the credential: it was given to
165+/// that sandbox and to nothing else.
166+async fn report_checks(
167+ request: &mut Request,
168+ services: &Services,
169+ run_id: &str,
170+) -> Result<Response> {
171+ let body = json_body(request).await;
172+ let reported: Outcome<CheckRun> = g1t_kit::call(
173+ &services.work,
174+ "report_checks",
175+ &ReportChecksArgs {
176+ run_id: run_id.to_owned(),
177+ token: body["token"].as_str().unwrap_or_default().to_owned(),
178+ results: serde_json::from_value(body["results"].clone()).unwrap_or_default(),
179+ error: body["error"].as_str().map(str::to_owned),
180+ skip: false,
181+ },
182+ )
183+ .await?;
184+ match reported {
185+ Outcome::Ok(run) => Response::from_json(&json!({ "status": run.status })),
186+ Outcome::Fail(refused) => failure(&refused),
187+ }
188+}
189+
161190 async fn respond(mut request: Request, env: &Env) -> Result<Response> {
162191 let method = method_name(request.method());
163192 if method == "OPTIONS" {
184213 ("GET", "/openapi.json") => return Response::from_json(&openapi::document()),
185214 ("POST", "/v1/device/code") => return device_code(&mut request, &services).await,
186215 ("POST", "/v1/device/token") => return device_token(&mut request, &services).await,
216+ ("POST", path) if path.starts_with("/v1/checks/") => {
217+ let run_id = path.trim_start_matches("/v1/checks/").to_owned();
218+ return report_checks(&mut request, &services, &run_id).await;
219+ }
187220 _ => {}
188221 }
189222
+53−6
5050 ReopenIssue,
5151 ListLabels,
5252 AddComment,
53+ ReviewPullRequest,
5354 ListPullRequests,
5455 GetPullRequest,
5556 CreatePullRequest,
192193 }
193194
194195 impl Op {
195− pub const ALL: [Op; 23] = [
196+ pub const ALL: [Op; 24] = [
196197 Op::Whoami,
197198 Op::CreateWorkspace,
198199 Op::ListRepos,
206207 Op::ReopenIssue,
207208 Op::ListLabels,
208209 Op::AddComment,
210+ Op::ReviewPullRequest,
209211 Op::ListPullRequests,
210212 Op::GetPullRequest,
211213 Op::CreatePullRequest,
238240 Op::ReopenIssue => "reopen_issue",
239241 Op::ListLabels => "list_labels",
240242 Op::AddComment => "add_comment",
243+ Op::ReviewPullRequest => "review_pull_request",
241244 Op::ListPullRequests => "list_pull_requests",
242245 Op::GetPullRequest => "get_pull_request",
243246 Op::CreatePullRequest => "create_pull_request",
275278 }
276279 Op::ReopenIssue => "Reopen a closed issue.",
277280 Op::ListLabels => "The labels available on a repository's issues.",
278− Op::AddComment => "Comment on an issue or a pull request.",
281+ Op::AddComment => {
282+ "Comment on an issue or a pull request. On a pull request, give path and line to comment on one line of the change."
283+ }
284+ Op::ReviewPullRequest => {
285+ "Give a verdict on a pull request: approve it, or request changes and say what. Read get_pull_request_changes first. You cannot review a pull request you opened."
286+ }
279287 Op::ListPullRequests => {
280288 "Pull requests on a repository, newest first. State open covers drafts and those ready for review; closed covers merged and closed."
281289 }
282290 Op::GetPullRequest => {
283− "A pull request's status, head commit, comments and the issue it is for."
291+ "A pull request's status, head commit, comments and reviews, the issue it is for, and the latest run of that issue's acceptance checks with each command's output."
284292 }
285293 Op::CreatePullRequest => {
286294 "Start a change. Opens a draft pull request with its own fork of the repository and returns the fork's git remote. Clone it, commit your work there, push, record your session as you go, then call mark_pull_request_ready. Give the issue it is for whenever there is one. If the change is already on a branch pushed to the repository, give that branch instead: no fork is made and the pull request is ready for review at once."
297305 "What a pull request changes: the files it touches and their line-by-line diff against the commit it started from. Use it to review a pull request or to compare several made for the same issue."
298306 }
299307 Op::MergePullRequest => {
300− "Land a pull request on the repository's main branch. Only members of the repository's workspace can merge, and only once it is marked ready. Merging resolves the issue it was made for: the issue closes recording this pull request, and the other pull requests still in progress for that issue close as superseded. Fails if main has moved since the pull request was opened; pull main into its fork or branch and push, then merge again."
308+ "Land a pull request on the repository's main branch. Only members of the repository's workspace can merge, and only once it is marked ready and its acceptance checks have passed. Merging resolves the issue it was made for: the issue closes recording this pull request, and the other pull requests still in progress for that issue close as superseded. Fails if main has moved since the pull request was opened; pull main into its fork or branch and push, then merge again."
301309 }
302310 Op::ListEvents => {
303311 "The timeline of a repository: pushes, issues, pull requests, comments and session activity, newest first."
394402 &["repo", "number"],
395403 ),
396404 Op::AddComment => object(
397− numbered(json!({ "body": { "type": "string", "description": "Markdown." } })),
405+ numbered(json!({
406+ "body": { "type": "string", "description": "Markdown." },
407+ "path": {
408+ "type": "string",
409+ "description": "On a pull request: the file to comment on.",
410+ },
411+ "line": {
412+ "type": "integer",
413+ "description": "The line of that file, as numbered after the change.",
414+ },
415+ })),
398416 &["repo", "number", "body"],
399417 ),
418+ Op::ReviewPullRequest => object(
419+ numbered(json!({
420+ "verdict": { "type": "string", "enum": ["approve", "request_changes"] },
421+ "body": {
422+ "type": "string",
423+ "description": "Markdown. Required when requesting changes.",
424+ },
425+ })),
426+ &["repo", "number", "verdict"],
427+ ),
400428 Op::ListPullRequests => {
401429 object(json!({ "repo": repo_schema(), "state": states }), &["repo"])
402430 }
459487 "type": "boolean",
460488 "description": "Set when this pull request is only part of the work: the issue stays open and the other pull requests for it are left alone.",
461489 },
490+ "ignore_checks": {
491+ "type": "boolean",
492+ "description": "Merge although the acceptance checks have not passed.",
493+ },
462494 })),
463495 &["repo", "number"],
464496 ),
537569 number,
538570 summary: text(input, "summary"),
539571 keep_issue_open: input["keep_issue_open"].as_bool() == Some(true),
572+ ignore_checks: input["ignore_checks"].as_bool() == Some(true),
540573 };
541574 let Services {
542575 identity,
673706 .await
674707 }
675708 Op::ListLabels => pass(work, "list_labels", &view()).await,
676− Op::AddComment => {
709+ Op::AddComment | Op::ReviewPullRequest => {
710+ let verdict = match (self, input["verdict"].as_str()) {
711+ (Op::AddComment, _) => None,
712+ (_, Some("approve")) => Some(Verdict::Approve),
713+ (_, Some("request_changes")) => Some(Verdict::RequestChanges),
714+ _ => {
715+ return failed(
716+ FailureCode::Invalid,
717+ "verdict must be approve or request_changes.",
718+ );
719+ }
720+ };
677721 pass(
678722 work,
679723 "add_comment",
682726 repo,
683727 number,
684728 body: text(input, "body"),
729+ path: optional_text(input, "path"),
730+ line: integer(input, "line"),
731+ verdict,
685732 },
686733 )
687734 .await
+6−0
107107 &[],
108108 ),
109109 route(
110+ "POST",
111+ "/v1/repos/:owner/:name/pulls/:number/reviews",
112+ Op::ReviewPullRequest,
113+ &[],
114+ ),
115+ route(
110116 "GET",
111117 "/v1/repos/:owner/:name/pulls/:number/session",
112118 Op::ReadSession,
+39−4
2929 - a **title** and a **description** in Markdown. An agent given the issue
3030 works from this text;
3131 - **labels**, which say what kind of issue it is;
32−- **acceptance checks**: commands a pull request should make pass;
32+- **acceptance checks**: commands a pull request should make pass, which
33+ g1t [runs itself](#acceptance-checks);
3334 - **comments**;
3435 - a **state**: open or closed. A closed issue records why: `completed` or
3536 `not_planned`.
9798 that the issue should stay open. The pull request merges, and the issue and
9899 the other pull requests are left as they are.
99100
101+## Acceptance checks
102+
103+An issue can list **acceptance checks**: commands, such as `cargo test`,
104+that a pull request for it should make pass.
105+
106+When a pull request for that issue is ready for review, g1t runs the checks
107+itself. It starts a sandbox that holds nothing but the pull request's head
108+commit, runs each command there, and records whether it passed and what it
109+printed. Pushing to the pull request runs them again.
110+
111+- The sandbox is clean. No agent has worked in it, so a pass says something
112+ about the code and not about what was left lying around.
113+- Only that sandbox can report the result. An agent cannot mark its own work
114+ as passing.
115+- Each pull request for an issue is checked the same way, which makes
116+ several of them comparable at a glance.
117+
118+A pull request whose checks have not passed cannot be merged, unless a
119+member of the workspace chooses to merge anyway.
120+
121+Running checks is in preview. They run when the issue's author or the pull
122+request's author is an account that g1t's sandboxes are enabled for.
123+
124+## Review
125+
126+Anyone who can see a pull request can comment on it, on the whole of it or
127+on a single line of its change. Line comments are shown in the **Changes**
128+tab under the line they are about.
129+
130+A reviewer can also give a verdict: **approve**, or **request changes**.
131+The pull request shows where each reviewer stands. You cannot give a verdict
132+on a pull request you opened, and that holds for agents too: one agent can
133+review another's work, but not its own.
134+
100135 ## Merging
101136
102137 A member of the repository's workspace merges a pull request once it is
103−marked ready. Merging moves `main` to the pull request's head commit.
138+marked ready and its checks have passed. Merging moves `main` to the pull request's head commit.
104139
105140 A pull request can only merge if it contains everything already on `main`.
106141 If something else landed first, merging is refused and the pull request is
134169 - **Merging in g1t.** Merging moves `main` forward to the pull request's
135170 head. When `main` has moved, the pull request has to pull it in first; g1t
136171 does not create merge commits or rebase for you yet.
137−- **Review comments on lines.** Comments are on the pull request as a whole.
138−- **Checks.** Running an issue's acceptance checks automatically.
172+- **Required reviews.** Verdicts are recorded and shown, but do not yet
173+ block a merge.
139174 - **Assignees and milestones.**
140175 - **g1t agents for everyone.** g1t can put its own agents on an issue, each
141176 in a sandbox. This is in preview and limited to selected accounts; anyone
+12−4
4343 4. `record_session` as it goes, so people can see its reasoning.
4444 5. `mark_pull_request_ready` with a summary of what changed and why.
4545
46+When the pull request is ready, g1t runs the issue's acceptance checks
47+against it in a clean sandbox. `get_pull_request` returns each command's
48+result and output, so an agent whose checks failed can read why, push a fix,
49+and have them run again.
50+
4651 If merging reports that `main` has moved, pull `main` from the repository
4752 into the fork and push. The pull request can then be merged.
4853
6671 | `close_issue` | Close an issue as completed or not planned. |
6772 | `reopen_issue` | Reopen a closed issue. |
6873 | `list_labels` | The labels in use on a repository. |
69−| `add_comment` | Comment on an issue or a pull request. |
74+| `add_comment` | Comment on an issue or a pull request, or on one line of a pull request's change. |
75+| `review_pull_request` | Approve a pull request or request changes. |
7076 | `list_pull_requests` | Pull requests on a repository, open or closed. |
71−| `get_pull_request` | A pull request's status, head commit, comments and issue. |
77+| `get_pull_request` | A pull request's status, comments, reviews, issue, and the result of its acceptance checks. |
7278 | `create_pull_request` | Open a draft pull request with a fork, or one from a branch already pushed. |
7379 | `record_session` | Append prompts, messages and tool calls to the session. |
7480 | `read_session` | Read a pull request's recorded session. |
8288
8389 An agent can review as well as write. Given an issue with several pull
8490 requests, it can call `get_pull_request_changes` and `read_session` on each,
85−compare them, leave its findings with `add_comment`, and, if its account is
86−a member of the workspace, `merge_pull_request` the best one.
91+compare them, and read each one's check results from `get_pull_request`. It
92+can leave findings on specific lines with `add_comment`, give a verdict with
93+`review_pull_request`, and, if its account is a member of the workspace,
94+`merge_pull_request` the best one. It cannot review a pull request it opened.
8795
8896 ## Filing issues from another system
8997
+7−2
3232 Everything it reads, runs and decides is recorded in the pull request's
3333 **Session** as it happens. The **Changes** tab shows the resulting diff.
3434
35+Once the pull request is ready, the issue's
36+[acceptance checks](/concepts/overview/#acceptance-checks) run against it in
37+a separate, clean sandbox. The agent has no say in the result.
38+
3539 If an agent fails, or finishes without changing anything, its pull request
3640 is closed and its session says why.
3741
3842 ## Choosing between pull requests
3943
40−Open each pull request, read its description and its changes, and merge the
41−one you want. Merging lands it on `main` and closes the issue, which records
44+Each pull request on the issue's page shows whether its checks passed. Open
45+the ones that did, read their descriptions and changes, and merge the one
46+you want. Merging lands it on `main` and closes the issue, which records
4247 that pull request as the one that resolved it. The other pull requests for
4348 the issue close as superseded. See
4449 [merging](/concepts/overview/#merging) for what happens when `main` has moved.
+25−2
107107 | `PATCH` | `/v1/repos/{owner}/{name}/issues/{number}` | Change `title`, `body` or `labels`. |
108108 | `POST` | `/v1/repos/{owner}/{name}/issues/{number}/close` | Close. Body: `reason`, `completed` or `not_planned`. |
109109 | `POST` | `/v1/repos/{owner}/{name}/issues/{number}/reopen` | Reopen. |
110−| `POST` | `/v1/repos/{owner}/{name}/issues/{number}/comments` | Comment. Body: `body`. The number may be a pull request's. |
110+| `POST` | `/v1/repos/{owner}/{name}/issues/{number}/comments` | Comment. Body: `body`. The number may be a pull request's, and then `path` and `line` put the comment on a line of its change. |
111111 | `GET` | `/v1/repos/{owner}/{name}/labels` | The labels in use. |
112112
113113 ```sh
146146 | `GET` | `/v1/repos/{owner}/{name}/pulls/{number}/changes` | The files it changes, with diffs. |
147147 | `POST` | `/v1/repos/{owner}/{name}/pulls/{number}/ready` | Mark ready for review. Body: `summary`. |
148148 | `POST` | `/v1/repos/{owner}/{name}/pulls/{number}/close` | Close without merging. |
149−| `POST` | `/v1/repos/{owner}/{name}/pulls/{number}/merge` | Land it on `main`. Body: `keep_issue_open`. Workspace members only; `409` if it is a draft or `main` has moved. |
149+| `POST` | `/v1/repos/{owner}/{name}/pulls/{number}/reviews` | Give a verdict. Body: `verdict` (`approve` or `request_changes`), `body`. Not on your own pull request. |
150+| `POST` | `/v1/repos/{owner}/{name}/pulls/{number}/merge` | Land it on `main`. Body: `keep_issue_open`, `ignore_checks`. Workspace members only; `409` if it is a draft, its checks have not passed, or `main` has moved. |
150151
151152 Opening a pull request returns the git remote of its fork:
152153
176177 that are still a draft or open are closed with `supersededBy` set. Send
177178 `"keep_issue_open": true` to merge without any of that.
178179
180+### Checks
181+
182+A pull request carries `checkStatus`: `queued`, `running`, `passed`, `failed`,
183+`errored`, or `null` when no checks have run against its head. Fetching one
184+pull request also returns the latest run in full:
185+
186+```json
187+{
188+ "pull": { "number": 14, "status": "open", "checkStatus": "failed" },
189+ "checks": {
190+ "headCommit": "8f3c2e1…",
191+ "status": "failed",
192+ "results": [
193+ { "command": "cargo test", "passed": false, "exitCode": 101, "output": "…", "durationMs": 8420 }
194+ ]
195+ }
196+}
197+```
198+
199+Checks are started by g1t, not through the API. They run when a pull
200+request becomes ready for review and again when its head moves.
201+
179202 ## Sessions
180203
181204 | Method | Path | |
+122−0
1+import { ChevronRight, CircleCheck, CircleX, LoaderCircle, RotateCw, TriangleAlert } from "lucide-react";
2+import { Form } from "react-router";
3+
4+import type { CheckRun, CheckStatus } from "@g1t/contracts";
5+
6+const LABELS: Record<CheckStatus, string> = {
7+ queued: "Checks queued",
8+ running: "Checks running",
9+ passed: "Checks passed",
10+ failed: "Checks failed",
11+ errored: "Checks could not run",
12+};
13+
14+export function CheckIcon({ status, size = 15 }: { status: CheckStatus; size?: number }) {
15+ if (status === "passed") return <CircleCheck size={size} className="shrink-0 text-accent" />;
16+ if (status === "failed") return <CircleX size={size} className="shrink-0 text-danger" />;
17+ if (status === "errored") return <TriangleAlert size={size} className="shrink-0 text-muted" />;
18+ return <LoaderCircle size={size} className="shrink-0 animate-spin text-info" />;
19+}
20+
21+/** The state of a pull request's checks at a glance, for lists. */
22+export function CheckBadge({ status }: { status: CheckStatus | null }) {
23+ if (!status) return null;
24+ return (
25+ <span className="flex shrink-0 items-center gap-1 text-xs text-muted" title={LABELS[status]}>
26+ <CheckIcon status={status} size={13} />
27+ <span className="sr-only">{LABELS[status]}</span>
28+ </span>
29+ );
30+}
31+
32+function seconds(ms: number): string {
33+ return ms < 1000 ? `${ms}ms` : `${(ms / 1000).toFixed(ms < 10_000 ? 1 : 0)}s`;
34+}
35+
36+/** The latest run of the acceptance checks: each command and what it printed. */
37+export function ChecksPanel({
38+ run,
39+ commands,
40+ canRerun,
41+}: {
42+ run: CheckRun | null;
43+ /** The issue's checks, shown before any run exists. */
44+ commands: string[];
45+ canRerun: boolean;
46+}) {
47+ if (commands.length === 0 && !run) return null;
48+ const pending = run?.status === "queued" || run?.status === "running";
49+ return (
50+ <section className="rounded-xl border border-line bg-surface p-4">
51+ <div className="flex items-center gap-2">
52+ {run && <CheckIcon status={run.status} />}
53+ <h3 className="grow text-sm font-medium">
54+ {run ? LABELS[run.status] : "Acceptance checks"}
55+ </h3>
56+ {canRerun && !pending && (
57+ <Form method="post">
58+ <button
59+ type="submit"
60+ name="action"
61+ value="recheck"
62+ title="Run the checks again"
63+ className="rounded-md p-1 text-faint transition-colors hover:bg-raised hover:text-fg"
64+ >
65+ <RotateCw size={14} />
66+ <span className="sr-only">Run the checks again</span>
67+ </button>
68+ </Form>
69+ )}
70+ </div>
71+ {run ? (
72+ <p className="mt-1 text-xs text-muted">
73+ On <span className="font-mono">{run.headCommit.slice(0, 7)}</span>, in a clean
74+ sandbox.
75+ </p>
76+ ) : (
77+ <p className="mt-1 text-xs text-muted">
78+ They run in a clean sandbox once the pull request is ready for review.
79+ </p>
80+ )}
81+ {run?.error && <p className="mt-2 text-xs text-muted">{run.error}</p>}
82+ <ul className="mt-3 space-y-1.5">
83+ {run && run.results.length > 0
84+ ? run.results.map((result) => (
85+ <li key={result.command}>
86+ <details className="group rounded-md border border-line bg-bg">
87+ <summary className="flex cursor-pointer list-none items-center gap-2 px-2.5 py-1.5">
88+ <ChevronRight
89+ size={13}
90+ className="shrink-0 text-faint transition-transform group-open:rotate-90"
91+ />
92+ <span className="min-w-0 grow truncate font-mono text-xs">
93+ {result.command}
94+ </span>
95+ <span className="shrink-0 text-[0.6875rem] text-faint">
96+ {seconds(result.durationMs)}
97+ </span>
98+ {result.passed ? (
99+ <CircleCheck size={14} className="shrink-0 text-accent" />
100+ ) : (
101+ <CircleX size={14} className="shrink-0 text-danger" />
102+ )}
103+ </summary>
104+ <pre className="max-h-72 overflow-auto border-t border-line p-2.5 font-mono text-[0.6875rem] leading-relaxed whitespace-pre-wrap text-muted">
105+ {result.output || "No output."}
106+ {result.exitCode != null && result.exitCode !== 0 && `\n\nExit code ${result.exitCode}.`}
107+ </pre>
108+ </details>
109+ </li>
110+ ))
111+ : commands.map((command) => (
112+ <li
113+ key={command}
114+ className="truncate rounded-md border border-line bg-bg px-2.5 py-1.5 font-mono text-xs text-muted"
115+ >
116+ {command}
117+ </li>
118+ ))}
119+ </ul>
120+ </section>
121+ );
122+}
+159−32
1−import { FileDiff as FileIcon, FileMinus, FilePlus } from "lucide-react";
1+import { FileDiff as FileIcon, FileMinus, FilePlus, MessageSquarePlus } from "lucide-react";
2+import { useState } from "react";
3+import { Form } from "react-router";
24
3−import type { Comparison, DiffLine, FileDiff } from "@g1t/contracts";
5+import type { Comment, Comparison, DiffLine, FileDiff } from "@g1t/contracts";
46
5−import { EmptyState } from "./ui";
7+import { Markdown } from "./markdown";
8+import { Avatar, Button, EmptyState, Textarea, TimeAgo } from "./ui";
69
710 const ROW_STYLES: Record<DiffLine["kind"], string> = {
811 context: "",
1619 delete: "-",
1720 };
1821
22+/** Comments on lines of the change, and whether the viewer may add one. */
23+export type LineReview = { comments: Comment[]; canComment: boolean };
24+
1925 /** `+12 −3` with a five-block bar, like the summary on a pull request. */
2026 function Stat({
2127 additions,
4450 );
4551 }
4652
47−function File({ file }: { file: FileDiff }) {
53+/** One comment made on a line, shown under that line. */
54+function LineComment({ comment }: { comment: Comment }) {
55+ return (
56+ <div className="rounded-lg border border-line bg-bg font-sans">
57+ <p className="flex items-center gap-2 border-b border-line px-3 py-1.5 text-xs text-muted">
58+ <Avatar name={comment.author.username} size={16} />
59+ <span className="font-medium text-fg">{comment.author.username}</span>
60+ <TimeAgo at={comment.createdAt} />
61+ </p>
62+ <div className="px-3 py-2 text-sm">
63+ <Markdown source={comment.body} />
64+ </div>
65+ </div>
66+ );
67+}
68+
69+function File({ file, review }: { file: FileDiff; review?: LineReview }) {
4870 const Icon =
4971 file.status === "added"
5072 ? FilePlus
5173 : file.status === "deleted"
5274 ? FileMinus
5375 : FileIcon;
76+ const comments = review?.comments.filter((comment) => comment.path === file.path) ?? [];
77+ const shown = new Set(
78+ file.hunks.flatMap((hunk) => hunk.lines.map((line) => line.new)),
79+ );
80+ // Made against an earlier version of the change, on a line no longer in it.
81+ const outdated = comments.filter(
82+ (comment) => comment.line == null || !shown.has(comment.line),
83+ );
5484 return (
5585 <section
5686 id={`file-${file.path}`}
72102 </span>
73103 <Stat additions={file.additions} deletions={file.deletions} />
74104 </header>
105+ {outdated.length > 0 && (
106+ <div className="space-y-2 border-b border-line bg-surface/50 p-3">
107+ <p className="text-xs text-faint">On lines that have since changed</p>
108+ {outdated.map((comment) => (
109+ <LineComment key={comment.id} comment={comment} />
110+ ))}
111+ </div>
112+ )}
75113 {file.binary ? (
76114 <p className="px-4 py-6 text-sm text-muted">
77115 Binary or large file; its contents are not shown.
83121 <table className="w-full border-collapse font-mono text-xs leading-5">
84122 <tbody>
85123 {file.hunks.map((hunk, index) => (
86− <HunkRows key={index} lines={hunk.lines} first={index === 0} />
124+ <HunkRows
125+ key={index}
126+ path={file.path}
127+ lines={hunk.lines}
128+ first={index === 0}
129+ comments={comments}
130+ canComment={review?.canComment ?? false}
131+ />
87132 ))}
88133 </tbody>
89134 </table>
93138 );
94139 }
95140
96−function HunkRows({ lines, first }: { lines: DiffLine[]; first: boolean }) {
141+function HunkRows({
142+ path,
143+ lines,
144+ first,
145+ comments,
146+ canComment,
147+}: {
148+ path: string;
149+ lines: DiffLine[];
150+ first: boolean;
151+ comments: Comment[];
152+ canComment: boolean;
153+}) {
154+ // The line whose comment box is open, if any.
155+ const [writing, setWriting] = useState<number | null>(null);
97156 return (
98157 <>
99158 {!first && (
106165 </td>
107166 </tr>
108167 )}
109− {lines.map((line, index) => (
110− <tr key={index} className={ROW_STYLES[line.kind]}>
111− <td className="w-10 px-2 text-right text-faint select-none">
112− {line.old}
113− </td>
114− <td className="w-10 px-2 text-right text-faint select-none">
115− {line.new}
116− </td>
117− <td
118− className={`w-5 text-center select-none ${
119− line.kind === "add"
120− ? "text-accent"
121− : line.kind === "delete"
122− ? "text-danger"
123− : "text-faint"
124− }`}
125− >
126− {MARKERS[line.kind]}
127− </td>
128− <td className="pr-4 whitespace-pre">{line.text}</td>
129− </tr>
130− ))}
168+ {lines.map((line, index) => {
169+ const here = comments.filter(
170+ (comment) => line.new != null && comment.line === line.new,
171+ );
172+ const open = line.new != null && writing === line.new;
173+ return (
174+ <Rows key={index}>
175+ <tr className={`group ${ROW_STYLES[line.kind]}`}>
176+ <td className="w-10 px-2 text-right text-faint select-none">{line.old}</td>
177+ <td className="relative w-10 px-2 text-right text-faint select-none">
178+ {line.new}
179+ {/* Lines that exist after the change can be commented on. */}
180+ {canComment && line.new != null && (
181+ <button
182+ type="button"
183+ aria-label={`Comment on line ${line.new}`}
184+ onClick={() => setWriting(open ? null : line.new)}
185+ className="absolute top-0 -right-2.5 z-10 hidden size-5 items-center justify-center rounded bg-accent text-bg group-hover:flex focus-visible:flex"
186+ >
187+ <MessageSquarePlus size={12} />
188+ </button>
189+ )}
190+ </td>
191+ <td
192+ className={`w-5 text-center select-none ${
193+ line.kind === "add"
194+ ? "text-accent"
195+ : line.kind === "delete"
196+ ? "text-danger"
197+ : "text-faint"
198+ }`}
199+ >
200+ {MARKERS[line.kind]}
201+ </td>
202+ <td className="pr-4 whitespace-pre">{line.text}</td>
203+ </tr>
204+ {(here.length > 0 || open) && (
205+ <tr>
206+ <td colSpan={4} className="border-y border-line bg-surface p-3">
207+ <div className="max-w-2xl space-y-2">
208+ {here.map((comment) => (
209+ <LineComment key={comment.id} comment={comment} />
210+ ))}
211+ {open && (
212+ <Form
213+ method="post"
214+ className="space-y-2 font-sans"
215+ onSubmit={() => setWriting(null)}
216+ >
217+ <input type="hidden" name="action" value="comment" />
218+ <input type="hidden" name="path" value={path} />
219+ <input type="hidden" name="line" value={line.new ?? ""} />
220+ <Textarea
221+ name="body"
222+ rows={3}
223+ required
224+ autoFocus
225+ placeholder={`Comment on line ${line.new}`}
226+ />
227+ <div className="flex gap-2">
228+ <Button type="submit">Comment</Button>
229+ <Button variant="quiet" type="button" onClick={() => setWriting(null)}>
230+ Cancel
231+ </Button>
232+ </div>
233+ </Form>
234+ )}
235+ </div>
236+ </td>
237+ </tr>
238+ )}
239+ </Rows>
240+ );
241+ })}
131242 </>
132243 );
133244 }
134245
135−export function DiffView({ comparison }: { comparison: Comparison }) {
246+/** Groups a line's rows without adding an element to the table. */
247+function Rows({ children }: { children: React.ReactNode }) {
248+ return <>{children}</>;
249+}
250+
251+export function DiffView({
252+ comparison,
253+ review,
254+}: {
255+ comparison: Comparison;
256+ review?: LineReview;
257+}) {
136258 const { files, truncated } = comparison;
137259 if (files.length === 0) {
138260 return (
139261 <EmptyState title="No changes yet">
140− Nothing has been pushed to this pull request's fork, or it matches the
141− repository it came from.
262+ Nothing has been pushed to this pull request yet, or it matches the
263+ branch it would merge into.
142264 </EmptyState>
143265 );
144266 }
152274 {files.length === 1 ? "file" : "files"} changed
153275 </span>
154276 <Stat additions={additions} deletions={deletions} />
277+ {review?.canComment && (
278+ <span className="ml-auto text-xs text-faint">
279+ Hover a line and press the button beside its number to comment on it.
280+ </span>
281+ )}
155282 </div>
156283 {files.length > 1 && (
157284 <ul className="rounded-xl border border-line bg-surface p-2 text-sm">
171298 </ul>
172299 )}
173300 {files.map((file) => (
174− <File key={file.path} file={file} />
301+ <File key={file.path} file={file} review={review} />
175302 ))}
176303 {truncated && (
177304 <p className="text-center text-sm text-muted">
+83−17
155155 );
156156 }
157157
158−/** Comments in order, then the box to add one. */
158+const VERDICTS = {
159+ approve: { label: "approved these changes", style: "text-accent" },
160+ request_changes: { label: "requested changes", style: "text-danger" },
161+} as const;
162+
163+/**
164+ * Comments in order, then the box to add one. On a pull request, `review`
165+ * says where its changes are shown and whether the viewer may give a
166+ * verdict.
167+ */
159168 export function Comments({
160169 comments,
161170 canComment,
171+ review,
162172 }: {
163173 comments: Comment[];
164174 canComment: boolean;
175+ review?: { changesUrl: string; canJudge: boolean };
165176 }) {
166177 return (
167178 <div className="space-y-4">
168− {comments.map((comment) => (
169− <article key={comment.id} className="rounded-xl border border-line bg-surface">
170− <header className="flex items-center gap-2 border-b border-line px-4 py-2 text-sm text-muted">
171− <Avatar name={comment.author.username} size={18} />
172− <span className="font-medium text-fg">{comment.author.username}</span>
173− <span>
174− commented <TimeAgo at={comment.createdAt} />
175− </span>
176− </header>
177− <div className="px-4 py-3">
178− <Markdown source={comment.body} />
179− </div>
180− </article>
181− ))}
179+ {comments.map((comment) => {
180+ const verdict = comment.verdict && VERDICTS[comment.verdict];
181+ return (
182+ <article key={comment.id} className="rounded-xl border border-line bg-surface">
183+ <header className="flex flex-wrap items-center gap-2 border-b border-line px-4 py-2 text-sm text-muted">
184+ <Avatar name={comment.author.username} size={18} />
185+ <span className="font-medium text-fg">{comment.author.username}</span>
186+ {verdict ? (
187+ <span className={`flex items-center gap-1 font-medium ${verdict.style}`}>
188+ {comment.verdict === "approve" ? (
189+ <CircleCheck size={14} />
190+ ) : (
191+ <CircleSlash size={14} />
192+ )}
193+ {verdict.label}
194+ </span>
195+ ) : (
196+ <span>commented</span>
197+ )}
198+ <TimeAgo at={comment.createdAt} />
199+ {comment.path && review && (
200+ <Link
201+ to={`${review.changesUrl}#file-${comment.path}`}
202+ className="ml-auto truncate font-mono text-xs text-faint hover:text-fg"
203+ >
204+ {comment.path}
205+ {comment.line != null && `:${comment.line}`}
206+ </Link>
207+ )}
208+ </header>
209+ {comment.body && (
210+ <div className="px-4 py-3">
211+ <Markdown source={comment.body} />
212+ </div>
213+ )}
214+ </article>
215+ );
216+ })}
182217 {canComment ? (
183218 <Form method="post" className="space-y-2" key={comments.length}>
184219 <input type="hidden" name="action" value="comment" />
185− <Textarea name="body" rows={3} required placeholder="Leave a comment. Markdown works." />
186− <Button type="submit">Comment</Button>
220+ <Textarea
221+ name="body"
222+ rows={3}
223+ placeholder={
224+ review?.canJudge
225+ ? "Leave a comment, or a review. Markdown works."
226+ : "Leave a comment. Markdown works."
227+ }
228+ />
229+ <div className="flex flex-wrap gap-2">
230+ <Button type="submit">Comment</Button>
231+ {review?.canJudge && (
232+ <>
233+ <Button variant="quiet" type="submit" name="verdict" value="approve">
234+ <CircleCheck size={14} className="text-accent" />
235+ Approve
236+ </Button>
237+ <Button variant="quiet" type="submit" name="verdict" value="request_changes">
238+ <CircleSlash size={14} className="text-danger" />
239+ Request changes
240+ </Button>
241+ </>
242+ )}
243+ </div>
187244 </Form>
188245 ) : (
189246 <p className="text-sm text-muted">
196253 </div>
197254 );
198255 }
256+
257+/** Where each reviewer stands: their most recent verdict. */
258+export function verdicts(comments: Comment[]): { reviewer: string; verdict: NonNullable<Comment["verdict"]> }[] {
259+ const latest = new Map<string, NonNullable<Comment["verdict"]>>();
260+ for (const comment of comments) {
261+ if (comment.verdict) latest.set(comment.author.username, comment.verdict);
262+ }
263+ return [...latest].map(([reviewer, verdict]) => ({ reviewer, verdict }));
264+}
+9−6
1919 Scripts,
2020 ScrollRestoration,
2121 useRouteLoaderData,
22+ useSubmit,
2223 } from "react-router";
2324
2425 import type { User } from "@g1t/contracts";
7374 }
7475
7576 function Header({ user }: { user: User | null | undefined }) {
77+ const submit = useSubmit();
7678 return (
7779 <header className="sticky top-0 z-40 border-b border-line bg-surface/85 backdrop-blur">
7880 <div className="mx-auto flex h-14 max-w-6xl items-center gap-2 px-4">
162164 </Link>
163165 </DropdownMenuItem>
164166 <DropdownMenuSeparator />
165− <DropdownMenuItem asChild>
166− <button type="submit" form="sign-out" className="w-full">
167− <LogOut />
168− Sign out
169− </button>
167+ {/* Submitted from here: the menu closes on select, and a button
168+ that has left the page cannot submit a form. */}
169+ <DropdownMenuItem
170+ onSelect={() => submit(null, { method: "post", action: "/logout" })}
171+ >
172+ <LogOut />
173+ Sign out
170174 </DropdownMenuItem>
171175 </DropdownMenuContent>
172176 </DropdownMenu>
173− <Form method="post" action="/logout" id="sign-out" hidden />
174177 </>
175178 ) : (
176179 <>
+11−2
1717 Textarea,
1818 TimeAgo,
1919 } from "../../components/ui";
20+import { CheckBadge } from "../../components/checks";
2021 import { Comments, IssueState, Label, PullIcon } from "../../components/work";
2122 import { work } from "../../lib/services.server";
2223 import { assertSameOrigin, getViewer, requireUser } from "../../lib/session.server";
8384 throw redirect(`/${params.owner}/${params.repo}/pull/${result.value.number}`);
8485 }
8586 case "comment": {
86− const result = await work.addComment(user, path, number, String(form.get("body") ?? ""));
87+ const result = await work.addComment(user, path, number, {
88+ body: String(form.get("body") ?? ""),
89+ });
8790 return result.ok ? null : { error: result.error.message };
8891 }
8992 case "labels": {
141144 <span className="size-1.5 animate-pulse rounded-full bg-accent" />
142145 )}
143146 <span className="ml-auto flex items-center gap-3 text-xs text-faint">
147+ <CheckBadge status={pull.checkStatus} />
144148 <span className="flex items-center gap-1 font-mono">
145149 <GitCommitHorizontal size={13} />
146150 {pull.headCommit?.slice(0, 7) ?? "no commits"}
170174 // Follow agents at work without a manual reload.
171175 const revalidator = useRevalidator();
172176 const navigation = useNavigation();
173− const running = pulls.some((pull) => pull.status === "draft");
177+ const running = pulls.some(
178+ (pull) =>
179+ pull.status === "draft" ||
180+ pull.checkStatus === "queued" ||
181+ pull.checkStatus === "running",
182+ );
174183 useEffect(() => {
175184 if (!running) return;
176185 const timer = setInterval(() => {
+86−12
1+import { env } from "cloudflare:workers";
12 import {
23 Bot,
34 ChevronRight,
45 FileDiff,
56 GitBranch,
67 GitCommitHorizontal,
8+ CircleCheck,
9+ CircleSlash,
710 GitMerge,
811 MessageSquare,
912 MessagesSquare,
2831 Textarea,
2932 TimeAgo,
3033 } from "../../components/ui";
31−import { Comments, IssueIcon, PullState } from "../../components/work";
34+import { ChecksPanel } from "../../components/checks";
35+import { Comments, IssueIcon, PullState, verdicts } from "../../components/work";
3236 import { repos, work } from "../../lib/services.server";
3337 import { assertSameOrigin, getViewer, requireUser } from "../../lib/session.server";
3438
9397 const path = { namespace: params.owner, name: params.repo };
9498 const number = Number(params.number);
9599 const action = form.get("action");
100+ const verdict = form.get("verdict");
101+ const line = Number(form.get("line"));
96102 const result =
97103 action === "merge"
98− ? await work.mergePull(user, path, number, form.get("keepIssueOpen") === "on")
104+ ? await work.mergePull(user, path, number, {
105+ keepIssueOpen: form.get("keepIssueOpen") === "on",
106+ ignoreChecks: form.get("ignoreChecks") === "on",
107+ })
99108 : action === "close"
100109 ? await work.closePull(user, path, number)
101− : action === "comment"
102− ? await work.addComment(user, path, number, String(form.get("body") ?? ""))
103− : await work.readyPull(user, path, number, String(form.get("summary") ?? ""));
110+ : action === "recheck"
111+ ? await env.RUNNER.recheck(user, path, number)
112+ : action === "comment"
113+ ? await work.addComment(user, path, number, {
114+ body: String(form.get("body") ?? ""),
115+ path: String(form.get("path") ?? "") || undefined,
116+ line: line > 0 ? line : undefined,
117+ verdict:
118+ verdict === "approve" || verdict === "request_changes" ? verdict : undefined,
119+ })
120+ : await work.readyPull(user, path, number, String(form.get("summary") ?? ""));
104121 return result.ok ? null : { error: result.error.message, action };
105122 }
106123
209226 pull,
210227 issue,
211228 comments,
229+ checks,
212230 tab,
213231 session,
214232 comparison,
224242 : `https://g1t.sh/${params.owner}/${params.repo}.git`;
225243 const active = pull.status === "draft" || pull.status === "open";
226244
227− // Follow an agent at work without a manual reload.
245+ // Follow an agent at work, or checks in progress, without a manual reload.
228246 const revalidator = useRevalidator();
229247 const working = pull.status === "draft";
248+ const checking = checks?.status === "queued" || checks?.status === "running";
249+ const reviews = verdicts(comments);
250+ // What stands between this pull request and a merge, if anything.
251+ const unchecked = checks && checks.status !== "passed";
230252 useEffect(() => {
231− if (!working) return;
253+ if (!working && !checking) return;
232254 const timer = setInterval(() => {
233255 if (document.visibilityState === "visible") revalidator.revalidate();
234256 }, REFRESH_MS);
235257 return () => clearInterval(timer);
236− }, [working, revalidator]);
258+ }, [working, checking, revalidator]);
237259
238260 return (
239261 <div className="grid gap-8 lg:grid-cols-[1fr_19rem]">
267289 )}
268290 </div>
269291
292+ {reviews.length > 0 && (
293+ <p className="mt-3 flex flex-wrap items-center gap-x-4 gap-y-1 text-sm">
294+ {reviews.map(({ reviewer, verdict }) => (
295+ <span
296+ key={reviewer}
297+ className={`flex items-center gap-1.5 ${
298+ verdict === "approve" ? "text-accent" : "text-danger"
299+ }`}
300+ >
301+ {verdict === "approve" ? <CircleCheck size={15} /> : <CircleSlash size={15} />}
302+ {verdict === "approve" ? "Approved by" : "Changes requested by"}{" "}
303+ <span className="font-medium">{reviewer}</span>
304+ </span>
305+ ))}
306+ </p>
307+ )}
308+
270309 {issue && (
271310 <Link
272311 to={`${base}/issues/${issue.number}`}
330369 </nav>
331370 <div className="mt-5">
332371 {comparison ? (
333− <DiffView comparison={comparison} />
372+ <DiffView
373+ comparison={comparison}
374+ review={{
375+ comments: comments.filter((comment) => comment.path),
376+ canComment: Boolean(viewer),
377+ }}
378+ />
334379 ) : tab === "session" ? (
335380 session.length === 0 ? (
336381 <EmptyState title="Nothing recorded yet">
357402 : "No description."}
358403 </p>
359404 )}
360− <Comments comments={comments} canComment={Boolean(viewer)} />
361− {actionData?.action === "comment" && <ErrorText>{actionData.error}</ErrorText>}
405+ <Comments
406+ comments={comments}
407+ canComment={Boolean(viewer)}
408+ review={{
409+ changesUrl: here + "?tab=changes",
410+ // Nobody reviews their own pull request.
411+ canJudge: active && viewer != null && viewer.id !== pull.author.id,
412+ }}
413+ />
362414 </div>
363415 )}
416+ {actionData?.action === "comment" && (
417+ <div className="mt-2">
418+ <ErrorText>{actionData.error}</ErrorText>
419+ </div>
420+ )}
364421 </div>
365422 </div>
366423
367424 <aside className="space-y-6">
425+ <ChecksPanel
426+ run={checks}
427+ commands={issue?.checks ?? []}
428+ canRerun={canManage && pull.status === "open"}
429+ />
430+ {actionData?.action === "recheck" && <ErrorText>{actionData.error}</ErrorText>}
431+
368432 {canMerge && pull.status === "open" && (
369433 <section className="rounded-xl border border-accent/30 bg-accent/5 p-4">
370434 <h3 className="text-sm font-medium">Merge this pull request</h3>
382446 </span>
383447 </label>
384448 )}
449+ {unchecked && (
450+ <label className="flex items-start gap-2 text-xs text-muted">
451+ <input type="checkbox" name="ignoreChecks" className="mt-0.5 accent-accent" />
452+ <span>
453+ Merge although the checks{" "}
454+ {checking ? "have not finished" : "did not pass"}.
455+ </span>
456+ </label>
457+ )}
385458 <div className="*:w-full">
386459 <Button variant="accent" type="submit" name="action" value="merge">
387460 <GitMerge size={15} />
414487 </Button>
415488 </div>
416489 </Form>
417− {actionData && actionData.action !== "merge" && actionData.action !== "comment" && (
490+ {actionData &&
491+ !["merge", "comment", "recheck"].includes(String(actionData.action)) && (
418492 <ErrorText>{actionData.error}</ErrorText>
419493 )}
420494 </section>
+4−0
33
44 import type { Route } from "./+types/pulls";
55 import { ButtonLink, EmptyState, TimeAgo } from "../../components/ui";
6+import { CheckBadge } from "../../components/checks";
67 import { PullIcon, StateTabs } from "../../components/work";
78 import { work } from "../../lib/services.server";
89 import { getViewer, unwrap } from "../../lib/session.server";
7273 {pull.supersededBy != null && <> · superseded by #{pull.supersededBy}</>}
7374 </span>
7475 </span>
76+ <span className="mt-0.5">
77+ <CheckBadge status={pull.checkStatus} />
78+ </span>
7579 <span className="mt-0.5 flex shrink-0 items-center gap-1 font-mono text-xs text-muted">
7680 {pull.branch ? <GitBranch size={13} /> : <Bot size={13} />}
7781 {pull.branch ?? pull.agent}
+1−1
55 "scripts": {
66 "build": "react-router build",
77 "dev": "react-router dev",
8− "typecheck": "wrangler types --include-env=false && react-router typegen && tsc -b",
8+ "typecheck": "wrangler types --include-env=false && react-router typegen && tsc -b --force",
99 "deploy": "npm run build && wrangler deploy",
1010 "preview": "npm run build && vite preview",
1111 "cf-typegen": "wrangler types --include-env=false",
+13−4
112112 - **Mark it ready:** `POST {repo}/pulls/{number}/ready` with `summary`, which
113113 becomes the pull request's description.
114114 - **See what a pull request changes:** `GET {repo}/pulls/{number}/changes`.
115+- **Checks:** once a pull request is ready, g1t runs the issue's `checks`
116+ against it in a clean sandbox. `GET {repo}/pulls/{number}` returns
117+ `checks.results`, each with `passed` and `output`. If they failed, push a
118+ fix and they run again.
115119 - **Comment** on an issue or a pull request:
116− `POST {repo}/issues/{number}/comments` with `body`.
120+ `POST {repo}/issues/{number}/comments` with `body`. On a pull request, add
121+ `path` and `line` to comment on one line of the change.
122+- **Review** someone else's pull request:
123+ `POST {repo}/pulls/{number}/reviews` with `verdict` (`approve` or
124+ `request_changes`) and `body`.
117125 - **Merge** (members of the repository's workspace):
118126 `POST {repo}/pulls/{number}/merge`. This closes the issue it was for and
119127 closes the other pull requests for that issue as superseded; send
126134 `list_labels`, `add_comment`, `list_pull_requests`, `get_pull_request`,
127135 `create_pull_request`, `record_session`, `read_session`,
128136 `mark_pull_request_ready`, `close_pull_request`,
129−`get_pull_request_changes`, `merge_pull_request`, and `list_repos`,
137+`get_pull_request_changes`, `review_pull_request`, `merge_pull_request`, and `list_repos`,
130138 `get_repo`, `create_repo`, `list_events`, `create_workspace`, `whoami`.
131139 MCP tools take the repository as `repo`, written `owner/name`.
132140
147155 - OAuth 2.1 for applications: metadata at
148156 `https://api.g1t.sh/.well-known/oauth-authorization-server`; authorization
149157 code with PKCE (S256), public clients, dynamic registration.
150−- Not available yet: merge commits made on the server, running checks
151− automatically.
158+- A pull request whose checks have not passed is refused a merge with
159+ `409`; a workspace member can send `{"ignore_checks": true}`.
160+- Not available yet: merge commits made on the server.
152161
153162 ## More
154163
+16−2
6666 pub resolved_by: Option<u32>,
6767 }
6868
69−/// The payload of `pull.opened`, `pull.ready`, `pull.closed` and
70−/// `pull.merged`; each uses the fields that apply to it.
69+/// The payload of `pull.opened`, `pull.ready`, `pull.updated` (its head
70+/// moved), `pull.closed` and `pull.merged`; each uses the fields that apply to it.
7171 #[derive(Debug, Default, Serialize)]
7272 #[serde(rename_all = "camelCase")]
7373 pub struct PullEvent {
8787 pub superseded_by: Option<u32>,
8888 }
8989
90+/// `checks.completed`: a run of an issue's acceptance checks against a pull
91+/// request finished.
92+#[derive(Debug, Serialize)]
93+#[serde(rename_all = "camelCase")]
94+pub struct ChecksEvent {
95+ pub pull_id: String,
96+ pub repo_id: String,
97+ pub number: u32,
98+ /// `passed`, `failed` or `errored`.
99+ pub status: &'static str,
100+ /// The commit that was checked.
101+ pub commit: String,
102+}
103+
90104 /// `comment.created`. `number` is the issue or pull request commented on.
91105 #[derive(Debug, Serialize)]
92106 #[serde(rename_all = "camelCase")]
+147−1
148148 /// Set on a pull request closed because another one for the same issue
149149 /// was merged: that one's number.
150150 pub superseded_by: Option<u32>,
151+ /// Where the latest run of the issue's acceptance checks stands, if
152+ /// there has been one against the current head.
153+ pub check_status: Option<CheckStatus>,
151154 pub author: User,
152155 /// RFC 3339.
153156 pub created_at: String,
155158 pub updated_at: String,
156159 }
157160
161+#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
162+#[serde(rename_all = "lowercase")]
163+pub enum CheckStatus {
164+ /// Waiting for a sandbox.
165+ Queued,
166+ Running,
167+ Passed,
168+ Failed,
169+ /// The checks could not be run at all.
170+ Errored,
171+}
172+
173+impl CheckStatus {
174+ pub fn as_str(self) -> &'static str {
175+ match self {
176+ CheckStatus::Queued => "queued",
177+ CheckStatus::Running => "running",
178+ CheckStatus::Passed => "passed",
179+ CheckStatus::Failed => "failed",
180+ CheckStatus::Errored => "errored",
181+ }
182+ }
183+}
184+
185+/// How one acceptance check went.
186+#[derive(Clone, Debug, Serialize, Deserialize)]
187+#[serde(rename_all = "camelCase")]
188+pub struct CheckResult {
189+ pub command: String,
190+ pub passed: bool,
191+ /// Absent when the command was stopped for taking too long.
192+ #[serde(default)]
193+ pub exit_code: Option<i32>,
194+ /// What the command printed, standard output and error together. The
195+ /// end of it, when there was a lot.
196+ #[serde(default)]
197+ pub output: String,
198+ #[serde(default)]
199+ pub duration_ms: u64,
200+}
201+
202+/// One run of an issue's acceptance checks against a pull request's head,
203+/// in a sandbox that holds nothing but that commit.
204+#[derive(Clone, Debug, Serialize, Deserialize)]
205+#[serde(rename_all = "camelCase")]
206+pub struct CheckRun {
207+ pub id: String,
208+ /// The commit that was checked.
209+ pub head_commit: String,
210+ pub status: CheckStatus,
211+ pub results: Vec<CheckResult>,
212+ /// Why the checks could not be run, when `status` is `errored`.
213+ pub error: Option<String>,
214+ /// RFC 3339.
215+ pub created_at: String,
216+ /// RFC 3339.
217+ pub finished_at: Option<String>,
218+}
219+
220+/// A reviewer's decision on a pull request.
221+#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
222+#[serde(rename_all = "snake_case")]
223+pub enum Verdict {
224+ Approve,
225+ RequestChanges,
226+}
227+
228+impl Verdict {
229+ pub fn as_str(self) -> &'static str {
230+ match self {
231+ Verdict::Approve => "approve",
232+ Verdict::RequestChanges => "request_changes",
233+ }
234+ }
235+}
236+
237+/// A comment on an issue or a pull request. On a pull request it can sit
238+/// on one line of the change, and it can carry a reviewer's verdict.
158239 #[derive(Clone, Debug, Serialize, Deserialize)]
159240 #[serde(rename_all = "camelCase")]
160241 pub struct Comment {
162243 pub author: User,
163244 /// Markdown.
164245 pub body: String,
246+ /// The file commented on, for a comment on a line.
247+ pub path: Option<String>,
248+ /// The line of that file, as numbered after the change.
249+ pub line: Option<u32>,
250+ pub verdict: Option<Verdict>,
165251 /// RFC 3339.
166252 pub created_at: String,
167253 }
214300 /// The issue it is for, if any.
215301 pub issue: Option<Issue>,
216302 pub comments: Vec<Comment>,
303+ /// The latest run of the issue's acceptance checks.
304+ pub checks: Option<CheckRun>,
217305 }
218306
219307 /// `open_issue`. Returns `Outcome<Issue>`.
300388 pub reason: Option<IssueReason>,
301389 }
302390
303−/// `add_comment`, on an issue or a pull request. Returns `Outcome<Comment>`.
391+/// `add_comment`, on an issue or a pull request. On a pull request it may
392+/// name a line of the change, and may carry a verdict; nobody can give a
393+/// verdict on their own pull request. Returns `Outcome<Comment>`.
304394 #[derive(Debug, Serialize, Deserialize)]
305395 pub struct AddCommentArgs {
306396 pub actor: User,
307397 pub repo: RepoPath,
308398 pub number: u32,
399+ /// May be empty when approving.
400+ #[serde(default)]
309401 pub body: String,
402+ #[serde(default)]
403+ pub path: Option<String>,
404+ #[serde(default)]
405+ pub line: Option<u32>,
406+ #[serde(default)]
407+ pub verdict: Option<Verdict>,
310408 }
311409
312410 /// `open_pull`. Without `branch`, forks the repo and returns a draft pull
347445 /// for it untouched, because this one is only part of the work.
348446 #[serde(default)]
349447 pub keep_issue_open: bool,
448+ /// For `merge_pull`: merge although the acceptance checks have not
449+ /// passed.
450+ #[serde(default)]
451+ pub ignore_checks: bool,
452+}
453+
454+/// `start_checks`: begins a run of the acceptance checks for a pull request
455+/// that is ready for review. Called by the runner service, which starts the
456+/// sandbox. Returns `Outcome<CheckJob>`.
457+#[derive(Debug, Serialize, Deserialize)]
458+#[serde(rename_all = "camelCase")]
459+pub struct StartChecksArgs {
460+ pub pull_id: String,
461+}
462+
463+/// What a sandbox needs to carry out a check run.
464+#[derive(Debug, Serialize, Deserialize)]
465+#[serde(rename_all = "camelCase")]
466+pub struct CheckJob {
467+ pub run_id: String,
468+ /// Lets the sandbox, and nothing else, report this run's results.
469+ pub token: String,
470+ pub commands: Vec<String>,
471+ /// The repository holding the commit: the fork, or the repository itself.
472+ pub source: RepoPath,
473+ pub commit: String,
474+ /// Who opened the pull request, and so can read its source.
475+ pub author: User,
476+ /// Username of whoever wrote the checks: the issue's author.
477+ pub requested_by: String,
478+ pub repo: RepoPath,
479+ pub number: u32,
480+}
481+
482+/// `report_checks`: what a sandbox says about its run. With no results and
483+/// no error it has started. `skip` forgets the run, for one that will not
484+/// be carried out. Returns `Outcome<CheckRun>`.
485+#[derive(Debug, Serialize, Deserialize)]
486+#[serde(rename_all = "camelCase")]
487+pub struct ReportChecksArgs {
488+ pub run_id: String,
489+ pub token: String,
490+ #[serde(default)]
491+ pub results: Vec<CheckResult>,
492+ #[serde(default)]
493+ pub error: Option<String>,
494+ #[serde(default)]
495+ pub skip: bool,
350496 }
351497
352498 /// A pull request in progress, with where it lives.
+226−0
1+//! Runs an issue's acceptance checks against one commit and reports how
2+//! each went.
3+//!
4+//! The sandbox holds nothing but that commit: no agent has run here, so a
5+//! passing result says something about the code and not about what an
6+//! agent left lying around.
7+//!
8+//! Configuration comes from the environment:
9+//!
10+//! - `G1T_API`, `CHECK_RUN`, `CHECK_TOKEN`: where and how to report.
11+//! - `GIT_REMOTE`, `GIT_COMMIT`: what to check out.
12+//! - `G1T_USER`, `G1T_TOKEN`: to read the repository, if it is private.
13+//! - `CHECKS`: the commands, as a JSON array.
14+
15+use std::path::Path;
16+use std::process::{Command, Stdio};
17+use std::time::Instant;
18+
19+use anyhow::{Context, Result, bail};
20+use serde::Serialize;
21+
22+use crate::{WORKDIR, auth_option, env, git};
23+
24+/// The longest one command may run.
25+const COMMAND_TIMEOUT_SECONDS: u32 = 10 * 60;
26+/// How much of a command's output is kept: the end, where failures are.
27+const MAX_OUTPUT_CHARS: usize = 12_000;
28+/// What `timeout` exits with when it had to stop the command.
29+const TIMED_OUT: i32 = 124;
30+const KILLED: i32 = 137;
31+
32+#[derive(Debug, Serialize)]
33+#[serde(rename_all = "camelCase")]
34+struct CheckResult {
35+ command: String,
36+ passed: bool,
37+ exit_code: Option<i32>,
38+ output: String,
39+ duration_ms: u64,
40+}
41+
42+/// The last `limit` characters of `text`, saying so if any were dropped.
43+fn tail(text: &str, limit: usize) -> String {
44+ let length = text.chars().count();
45+ if length <= limit {
46+ return text.to_owned();
47+ }
48+ let kept: String = text.chars().skip(length - limit).collect();
49+ format!("… (earlier output not shown)\n{kept}")
50+}
51+
52+fn redact(text: &str, secrets: &[String]) -> String {
53+ secrets.iter().fold(text.to_owned(), |text, secret| {
54+ text.replace(secret, "[redacted]")
55+ })
56+}
57+
58+/// Runs one command in the checkout, without this process's credentials.
59+fn run_command(command: &str, workdir: &Path, secrets: &[String]) -> CheckResult {
60+ let started = Instant::now();
61+ let output = Command::new("timeout")
62+ .args([
63+ "--signal=KILL",
64+ &COMMAND_TIMEOUT_SECONDS.to_string(),
65+ "sh",
66+ "-c",
67+ // One stream, in the order it was written.
68+ &format!("( {command}\n) 2>&1"),
69+ ])
70+ .current_dir(workdir)
71+ .env_remove("G1T_TOKEN")
72+ .env_remove("CHECK_TOKEN")
73+ .stdin(Stdio::null())
74+ .output();
75+ let duration_ms = started.elapsed().as_millis() as u64;
76+ match output {
77+ Ok(output) => {
78+ let code = output.status.code();
79+ let timed_out = matches!(code, Some(TIMED_OUT | KILLED) | None);
80+ let mut text = String::from_utf8_lossy(&output.stdout).into_owned();
81+ if timed_out {
82+ text.push_str(&format!(
83+ "\nStopped after {} minutes.",
84+ COMMAND_TIMEOUT_SECONDS / 60
85+ ));
86+ }
87+ CheckResult {
88+ command: command.to_owned(),
89+ passed: output.status.success(),
90+ exit_code: code.filter(|_| !timed_out),
91+ output: redact(&tail(text.trim_end(), MAX_OUTPUT_CHARS), secrets),
92+ duration_ms,
93+ }
94+ }
95+ Err(error) => CheckResult {
96+ command: command.to_owned(),
97+ passed: false,
98+ exit_code: None,
99+ output: format!("Could not start the command: {error}"),
100+ duration_ms,
101+ },
102+ }
103+}
104+
105+struct Reporter {
106+ url: String,
107+ token: String,
108+}
109+
110+impl Reporter {
111+ fn send(&self, mut body: serde_json::Value) -> Result<()> {
112+ body["token"] = self.token.clone().into();
113+ ureq::post(&self.url)
114+ .send_json(body)
115+ .context("could not report the check run")?;
116+ Ok(())
117+ }
118+}
119+
120+fn check_out(secrets: &[String]) -> Result<()> {
121+ let remote = env("GIT_REMOTE")?;
122+ let commit = env("GIT_COMMIT")?;
123+ let auth = auth_option(&env("G1T_USER")?, &env("G1T_TOKEN")?);
124+ std::fs::create_dir_all("/work")?;
125+ let cloned = git(
126+ Path::new("/work"),
127+ &["-c", &auth, "clone", "--quiet", &remote, WORKDIR],
128+ )
129+ .and_then(|_| {
130+ git(
131+ Path::new(WORKDIR),
132+ &[
133+ "-c",
134+ "advice.detachedHead=false",
135+ "checkout",
136+ "--quiet",
137+ &commit,
138+ ],
139+ )
140+ });
141+ if let Err(error) = cloned {
142+ bail!("{}", redact(&format!("{error:#}"), secrets));
143+ }
144+ Ok(())
145+}
146+
147+pub fn main() -> i32 {
148+ let reporter = match (env("G1T_API"), env("CHECK_RUN"), env("CHECK_TOKEN")) {
149+ (Ok(api), Ok(run), Ok(token)) => Reporter {
150+ url: format!("{api}/v1/checks/{run}"),
151+ token,
152+ },
153+ _ => {
154+ eprintln!("g1t-runner: G1T_API, CHECK_RUN and CHECK_TOKEN must be set");
155+ return 2;
156+ }
157+ };
158+ let secrets: Vec<String> = ["G1T_TOKEN", "CHECK_TOKEN"]
159+ .iter()
160+ .filter_map(|name| std::env::var(name).ok())
161+ .filter(|secret| !secret.is_empty())
162+ .collect();
163+ let commands: Vec<String> = std::env::var("CHECKS")
164+ .ok()
165+ .and_then(|json| serde_json::from_str(&json).ok())
166+ .unwrap_or_default();
167+
168+ // Says the run has started.
169+ if let Err(error) = reporter.send(serde_json::json!({})) {
170+ eprintln!("g1t-runner: {error:#}");
171+ return 1;
172+ }
173+ let report = match check_out(&secrets) {
174+ Err(error) => serde_json::json!({
175+ "error": format!("The commit could not be checked out: {error:#}"),
176+ }),
177+ Ok(()) => {
178+ let results: Vec<CheckResult> = commands
179+ .iter()
180+ .map(|command| run_command(command, Path::new(WORKDIR), &secrets))
181+ .collect();
182+ serde_json::json!({ "results": results })
183+ }
184+ };
185+ match reporter.send(report) {
186+ Ok(()) => 0,
187+ Err(error) => {
188+ eprintln!("g1t-runner: {error:#}");
189+ 1
190+ }
191+ }
192+}
193+
194+#[cfg(test)]
195+mod tests {
196+ use super::*;
197+
198+ #[test]
199+ fn long_output_keeps_its_end() {
200+ let text = format!("{}END", "x".repeat(50));
201+ let kept = tail(&text, 10);
202+ assert!(kept.ends_with("xxxxxxxEND"));
203+ assert!(kept.starts_with("… (earlier output not shown)"));
204+ assert_eq!(tail("short", 10), "short");
205+ }
206+
207+ #[test]
208+ fn secrets_do_not_reach_a_report() {
209+ let secrets = vec!["g1t_secret".to_owned()];
210+ assert_eq!(redact("token=g1t_secret", &secrets), "token=[redacted]");
211+ }
212+
213+ #[cfg(unix)]
214+ #[test]
215+ fn a_command_passes_or_fails_by_its_exit_code() {
216+ let here = std::env::temp_dir();
217+ let passed = run_command("echo out; echo err >&2", &here, &[]);
218+ assert!(passed.passed);
219+ assert_eq!(passed.exit_code, Some(0));
220+ assert_eq!(passed.output, "out\nerr");
221+ let failed = run_command("echo nope; exit 3", &here, &[]);
222+ assert!(!failed.passed);
223+ assert_eq!(failed.exit_code, Some(3));
224+ assert_eq!(failed.output, "nope");
225+ }
226+}
+11−4
66 //! and marks the pull request ready for review. It talks to g1t only through the public API and git, exactly
77 //! as an agent on someone's own machine would.
88 //!
9+//! With `MODE=checks` it runs acceptance checks instead; see `checks`.
10+//!
911 //! Configuration comes from the environment:
1012 //!
1113 //! - `G1T_API`, `G1T_TOKEN`, `G1T_USER`: where and who to report as.
1416 //! - `COMMIT_MESSAGE`: used if the agent leaves changes uncommitted.
1517 //! - `ANTHROPIC_API_KEY`: read by the harness itself.
1618
19+mod checks;
1720 mod harness;
1821 mod report;
1922
2629
2730 use report::{Entry, Reporter};
2831
29−const WORKDIR: &str = "/work/repo";
32+pub(crate) const WORKDIR: &str = "/work/repo";
3033
31−fn env(name: &str) -> Result<String> {
34+pub(crate) fn env(name: &str) -> Result<String> {
3235 std::env::var(name).with_context(|| format!("{name} is not set"))
3336 }
3437
3538 /// Runs git and returns its trimmed output, failing on a non-zero exit.
36−fn git(dir: &Path, args: &[&str]) -> Result<String> {
39+pub(crate) fn git(dir: &Path, args: &[&str]) -> Result<String> {
3740 let output = Command::new("git")
3841 .current_dir(dir)
3942 .args(args)
5255 /// A git option that authenticates one command. The credential is passed
5356 /// per command and never written to the clone's config or its remote URL,
5457 /// where the agent would find it.
55−fn auth_option(user: &str, token: &str) -> String {
58+pub(crate) fn auth_option(user: &str, token: &str) -> String {
5659 let credentials = STANDARD.encode(format!("{user}:{token}"));
5760 format!("http.extraHeader=Authorization: Basic {credentials}")
5861 }
112115 }
113116
114117 fn main() {
118+ // The same image also runs acceptance checks, with no agent involved.
119+ if std::env::var("MODE").as_deref() == Ok("checks") {
120+ std::process::exit(checks::main());
121+ }
115122 let mut reporter = match Reporter::from_env() {
116123 Ok(reporter) => reporter,
117124 Err(error) => {
+4−4
646646 and device sign-in; an OAuth 2.1 server, so MCP clients sign in through the
647647 browser with no token to paste; workspaces with members; issues with labels, checks and
648648 comments; pull requests in forks or from branches, with diffs and sessions,
649−several per issue; merging with a behind check, which resolves the issue and supersedes
649+several per issue; acceptance checks run in clean sandboxes, gating the
650+merge; line comments and review verdicts; merging with a behind check, which resolves the issue and supersedes
650651 the rest; g1t agents in sandboxes with a choice of model; REST API, OpenAPI
651652 and MCP server; event bus. Every service and the API are in Rust.
652653
655656 3. Event storage per the design above: per-repo hot log, Iceberg on R2,
656657 hash-chained audit.
657658 4. CLI with Claude Code hooks to record sessions automatically.
658−5. Acceptance checks run in sandboxes; review comments on lines.
659+5. Reviewer agents assigned automatically; required reviews; risk tiers.
659660 6. Server-side merge and rebase; landing queue with speculative checks;
660661 resolve-on-move.
661−7. Compare view, proof bundles, reviewers, risk tiers; work registry,
662− handoff.
662+7. Compare view, proof bundles; work registry, handoff.
663663 8. Projects, mission control, steering; why-blame, digest, timeline.
664664 9. Context hub, portfolio; automations and integrations (Sentry first).
665665 10. SSH; bot protection; own keys, endpoints and runners.
+7−4
111111 reopenIssue: (actor, repo, number) => call("reopen_issue", { actor, repo, number }),
112112 listLabels: (repo, viewer) => call("list_labels", { repo, viewer }),
113113 counts: (repo, viewer) => call("counts", { repo, viewer }),
114− addComment: (actor, repo, number, body) =>
115− call("add_comment", { actor, repo, number, body }),
114+ addComment: (actor, repo, number, comment) =>
115+ call("add_comment", { actor, repo, number, ...comment }),
116+ startChecks: (pullId) => call("start_checks", { pullId }),
117+ reportChecks: (runId, token, report) =>
118+ call("report_checks", { runId, token, ...report }),
116119 openPull: (actor, repo, input) => call("open_pull", { actor, repo, ...input }),
117120 listPulls: (repo, viewer, state) => call("list_pulls", { repo, viewer, state }),
118121 getPull: (repo, number, viewer) => call("get_pull", { repo, number, viewer }),
119122 readyPull: (actor, repo, number, summary) =>
120123 call("ready_pull", { actor, repo, number, summary }),
121124 closePull: (actor, repo, number) => call("close_pull", { actor, repo, number }),
122− mergePull: (actor, repo, number, keepIssueOpen = false) =>
123− call("merge_pull", { actor, repo, number, keepIssueOpen }),
125+ mergePull: (actor, repo, number, options = {}) =>
126+ call("merge_pull", { actor, repo, number, ...options }),
124127 listActivePulls: (viewer) => call("list_active_pulls", { viewer }),
125128 appendSession: (actor, repo, number, entries) =>
126129 call("append_session", { actor, repo, number, entries }),
+10−0
2828 /** `issue` is the number of the issue the pull request is for. */
2929 "pull.opened": { pullId: string; repoId: string; number: number; issue?: number; agent: string };
3030 "pull.ready": { pullId: string; repoId: string; number: number; issue?: number };
31+ /** A push moved the head of a pull request that is ready for review. */
32+ "pull.updated": { pullId: string; repoId: string; number: number; issue?: number; commit: string };
3133 "pull.closed": { pullId: string; repoId: string; number: number; issue?: number };
3234 "pull.merged": { pullId: string; repoId: string; number: number; issue?: number; commit: string };
35+ /** A run of the acceptance checks finished. `commit` is what was checked. */
36+ "checks.completed": {
37+ pullId: string;
38+ repoId: string;
39+ number: number;
40+ status: "passed" | "failed" | "errored";
41+ commit: string;
42+ };
3343 /** `number` is the issue or pull request commented on. */
3444 "comment.created": { commentId: string; repoId: string; number: number };
3545 "session.appended": { pullId: string; repoId: string; number: number; count: number };
+6−1
2222 model?: string;
2323 };
2424
25−/** Hosted agents: sandboxes on g1t that work on an issue. */
25+/** Sandboxes on g1t: agents that work on an issue, and acceptance checks. */
2626 export interface RunnerApi {
2727 /** The models this viewer may run g1t agents on; empty if they may not. */
2828 models(viewer: Viewer): Promise<AgentModel[]>;
3232 * progress shows up in each pull request's session.
3333 */
3434 run(actor: User, repo: RepoPath, issue: number, input: RunHostedInput): Promise<Result<Pull[]>>;
35+ /**
36+ * Runs the acceptance checks of a pull request again. Whoever opened it,
37+ * or a member of the repository's workspace, may ask.
38+ */
39+ recheck(actor: User, repo: RepoPath, number: number): Promise<Result<boolean>>;
3540 }
+104−3
9292 * merged: that one's number.
9393 */
9494 supersededBy: number | null;
95+ /**
96+ * Where the latest run of the issue's acceptance checks stands, if there
97+ * has been one against the current head.
98+ */
99+ checkStatus: CheckStatus | null;
95100 author: User;
96101 /** RFC 3339. */
97102 createdAt: string;
99104 updatedAt: string;
100105 };
101106
107+/** `queued` waits for a sandbox; `errored` means the checks could not be run. */
108+export type CheckStatus = "queued" | "running" | "passed" | "failed" | "errored";
109+
110+/** How one acceptance check went. */
111+export type CheckResult = {
112+ command: string;
113+ passed: boolean;
114+ /** Null when the command was stopped for taking too long. */
115+ exitCode: number | null;
116+ /** What the command printed; the end of it, when there was a lot. */
117+ output: string;
118+ durationMs: number;
119+};
120+
121+/**
122+ * One run of an issue's acceptance checks against a pull request's head, in
123+ * a sandbox that holds nothing but that commit.
124+ */
125+export type CheckRun = {
126+ id: string;
127+ /** The commit that was checked. */
128+ headCommit: string;
129+ status: CheckStatus;
130+ results: CheckResult[];
131+ /** Why the checks could not be run, when `status` is `errored`. */
132+ error: string | null;
133+ /** RFC 3339. */
134+ createdAt: string;
135+ /** RFC 3339. */
136+ finishedAt: string | null;
137+};
138+
139+/** A reviewer's decision on a pull request. */
140+export type Verdict = "approve" | "request_changes";
141+
142+/**
143+ * A comment on an issue or a pull request. On a pull request it can sit on
144+ * one line of the change, and it can carry a reviewer's verdict.
145+ */
102146 export type Comment = {
103147 id: string;
104148 author: User;
105149 /** Markdown. */
106150 body: string;
151+ /** The file commented on, for a comment on a line. */
152+ path: string | null;
153+ /** The line of that file, as numbered after the change. */
154+ line: number | null;
155+ verdict: Verdict | null;
107156 /** RFC 3339. */
108157 createdAt: string;
109158 };
110159
160+export type NewComment = {
161+ /** May be empty when approving. */
162+ body: string;
163+ path?: string;
164+ line?: number;
165+ verdict?: Verdict;
166+};
167+
168+/** What a sandbox needs to carry out a check run. */
169+export type CheckJob = {
170+ runId: string;
171+ /** Lets the sandbox, and nothing else, report this run's results. */
172+ token: string;
173+ commands: string[];
174+ /** The repository holding the commit: the fork, or the repository itself. */
175+ source: RepoPath;
176+ commit: string;
177+ /** Who opened the pull request, and so can read its source. */
178+ author: User;
179+ /** Username of whoever wrote the checks: the issue's author. */
180+ requestedBy: string;
181+ repo: RepoPath;
182+ number: number;
183+};
184+
185+export type CheckReport = { results?: CheckResult[]; error?: string; skip?: boolean };
186+
111187 export type SessionEntryKind = "prompt" | "message" | "tool_call" | "tool_result" | "note";
112188
113189 /** One step of an agent's session: the "why" behind a pull request's commits. */
138214 /** The issue it is for, if any. */
139215 issue: Issue | null;
140216 comments: Comment[];
217+ /** The latest run of the issue's acceptance checks. */
218+ checks: CheckRun | null;
141219 };
142220
143221 export type OpenIssueInput = {
184262 /** How many issues and pull requests are open. */
185263 counts(repo: RepoPath, viewer: Viewer): Promise<Result<{ issues: number; pulls: number }>>;
186264
187− /** On an issue or a pull request. */
188− addComment(actor: User, repo: RepoPath, number: number, body: string): Promise<Result<Comment>>;
265+ /**
266+ * On an issue or a pull request. On a pull request it may name a line of
267+ * the change and carry a verdict; nobody can give a verdict on their own.
268+ */
269+ addComment(actor: User, repo: RepoPath, number: number, comment: NewComment): Promise<Result<Comment>>;
270+
271+ /**
272+ * Begins a run of the acceptance checks for a pull request that is ready
273+ * for review. For the runner service, which starts the sandbox.
274+ */
275+ startChecks(pullId: string): Promise<Result<CheckJob>>;
276+ /**
277+ * What a sandbox says about its run. With no results and no error it has
278+ * started; `skip` forgets a run that will not be carried out.
279+ */
280+ reportChecks(runId: string, token: string, report: CheckReport): Promise<Result<CheckRun>>;
189281
190282 /**
191283 * Opens a pull request: a draft with a fork to push to, or, given a
204296 * naming this pull request, and the others still in progress for it close
205297 * as superseded. Only members of the repository's workspace may merge.
206298 */
207− mergePull(actor: User, repo: RepoPath, number: number, keepIssueOpen?: boolean): Promise<Result<Pull>>;
299+ mergePull(
300+ actor: User,
301+ repo: RepoPath,
302+ number: number,
303+ options?: {
304+ keepIssueOpen?: boolean;
305+ /** Merge although the acceptance checks have not passed. */
306+ ignoreChecks?: boolean;
307+ },
308+ ): Promise<Result<Pull>>;
208309 /** Drafts and open pull requests the viewer started, most recently active first. */
209310 listActivePulls(viewer: Viewer): Promise<{ pull: Pull; issue: Issue | null }[]>;
210311
+2−1
2020 // One queue per subscribing service, so each consumes, retries
2121 // and scales on its own. Bindings named SUBSCRIBER_* receive
2222 // every event.
23− { "binding": "SUBSCRIBER_WORK", "queue": "g1t-events-work" }
23+ { "binding": "SUBSCRIBER_WORK", "queue": "g1t-events-work" },
24+ { "binding": "SUBSCRIBER_RUNNER", "queue": "g1t-events-runner" }
2425 ],
2526 "consumers": [{ "queue": "g1t-events", "max_batch_size": 100, "max_batch_timeout": 1 }]
2627 },
+112−10
33
44 import {
55 type AgentModel,
6+ type CheckJob,
7+ type G1tEvent,
68 type Issue,
79 type Pull,
810 type RepoPath,
5557 /** How g1t's own agent is labelled. What runs behind it is g1t's choice. */
5658 const AGENT = "g1t-agent";
5759
58−/** Which pull request a sandbox is working on, and as whom. */
59−type Run = { actor: User; repo: RepoPath; number: number };
60+/**
61+ * What a sandbox is doing: an agent working on a pull request as someone,
62+ * or a run of acceptance checks.
63+ */
64+type Run =
65+ | { kind: "agent"; actor: User; repo: RepoPath; number: number }
66+ | { kind: "checks"; runId: string; token: string };
6067 type RunRequest = Run & { envVars: Record<string, string> };
6168
69+/** Long enough to clone, install and test; then the token stops working. */
70+const CHECKS_TOKEN_TTL_SECONDS = 45 * 60;
71+
6272 /**
63− * One sandbox, for one pull request. The image's entrypoint is the g1t runner,
64− * which does the work and exits; this class only starts it and cleans up
65− * if it dies without reporting.
73+ * One sandbox, for one agent or one run of checks. The image's entrypoint
74+ * is the g1t runner, which does the work and exits; this class only starts
75+ * it and cleans up if it dies without reporting.
6676 */
6777 export class AttemptSandbox extends Container<RunnerEnv> {
6878 sleepAfter = "45m";
7585
7686 override async onStop({ exitCode }: StopParams): Promise<void> {
7787 if (exitCode === 0) return;
88+ const run = await this.ctx.storage.get<Run>("run");
89+ if (!run) return;
90+ const work = workClient(this.env.WORK);
91+ if (run.kind === "checks") {
92+ // Refused harmlessly if the run did report before it stopped.
93+ await work.reportChecks(run.runId, run.token, {
94+ error: "The sandbox stopped before the checks finished.",
95+ });
96+ return;
97+ }
7898 // The runner closes its own pull request when it fails. This covers a
7999 // sandbox that was killed before it could; closing twice is refused
80100 // harmlessly.
81− const run = await this.ctx.storage.get<Run>("run");
82− if (run) await workClient(this.env.WORK).closePull(run.actor, run.repo, run.number);
101+ await work.closePull(run.actor, run.repo, run.number);
83102 }
84103 }
85104
114133 return JSON.parse(this.env.AGENT_MODELS);
115134 }
116135
136+ /** Whether sandboxes may be started on this person's say-so. */
137+ private enabledFor(username: string): boolean {
138+ return this.env.HOSTED_AGENT_USERS.split(",")
139+ .map((name) => name.trim())
140+ .includes(username);
141+ }
142+
117143 private allowed(viewer: Viewer): boolean {
118144 if (!viewer || !this.env.ANTHROPIC_API_KEY) return false;
119− return this.env.HOSTED_AGENT_USERS.split(",")
120− .map((name) => name.trim())
121− .includes(viewer.username);
145+ return this.enabledFor(viewer.username);
146+ }
147+
148+ /** Events from the bus: a pull request was opened, became ready, or moved. */
149+ async queue(batch: MessageBatch<G1tEvent>): Promise<void> {
150+ for (const message of batch.messages) {
151+ const event = message.body;
152+ // A pull request opened from a branch is ready from the start; one
153+ // opened as a draft is refused below until it is marked ready.
154+ if (
155+ event.type === "pull.opened" ||
156+ event.type === "pull.ready" ||
157+ event.type === "pull.updated"
158+ ) {
159+ await this.startChecks(event.data.pullId);
160+ }
161+ message.ack();
162+ }
163+ }
164+
165+ /**
166+ * Runs a pull request's acceptance checks in a sandbox of its own. Does
167+ * nothing when there is nothing to run.
168+ */
169+ private async startChecks(pullId: string): Promise<boolean> {
170+ const work = workClient(this.env.WORK);
171+ const started = await work.startChecks(pullId);
172+ if (!started.ok) return false;
173+ const job: CheckJob = started.value;
174+ // Checks are commands one person wrote, run against code another
175+ // pushed, on g1t's machines. In the preview they run only when one of
176+ // the two is someone sandboxes are enabled for.
177+ if (!this.enabledFor(job.requestedBy) && !this.enabledFor(job.author.username)) {
178+ await work.reportChecks(job.runId, job.token, { skip: true });
179+ return false;
180+ }
181+ // To read the commit, which may be private, as the one who pushed it.
182+ const { token } = await identityClient(this.env.IDENTITY).createAccessToken(
183+ job.author,
184+ `Checks on ${job.repo.namespace}/${job.repo.name}#${job.number}`,
185+ CHECKS_TOKEN_TTL_SECONDS,
186+ );
187+ const sandbox = this.env.SANDBOX.get(this.env.SANDBOX.idFromName(job.runId));
188+ await sandbox.run({
189+ kind: "checks",
190+ runId: job.runId,
191+ token: job.token,
192+ envVars: {
193+ MODE: "checks",
194+ G1T_API: "https://api.g1t.sh",
195+ CHECK_RUN: job.runId,
196+ CHECK_TOKEN: job.token,
197+ G1T_USER: job.author.username,
198+ G1T_TOKEN: token,
199+ GIT_REMOTE: `https://g1t.sh/${job.source.namespace}/${job.source.name}.git`,
200+ GIT_COMMIT: job.commit,
201+ CHECKS: JSON.stringify(job.commands),
202+ },
203+ });
204+ return true;
205+ }
206+
207+ async recheck(actor: User, repo: RepoPath, number: number): Promise<Result<boolean>> {
208+ const found = await workClient(this.env.WORK).getPull(repo, number, actor);
209+ if (!found.ok) return found;
210+ const { pull } = found.value;
211+ const member = (actor.workspaces ?? []).some(
212+ (membership) => membership.slug === repo.namespace,
213+ );
214+ if (!member && pull.author.id !== actor.id) {
215+ return fail(
216+ "forbidden",
217+ "Only whoever opened a pull request, or a member of the workspace, can run its checks.",
218+ );
219+ }
220+ return (await this.startChecks(pull.id))
221+ ? ok(true)
222+ : fail("conflict", "There are no checks to run for this pull request right now.");
122223 }
123224
124225 async models(viewer: Viewer): Promise<AgentModel[]> {
177278 );
178279 const sandbox = this.env.SANDBOX.get(this.env.SANDBOX.idFromName(pull.id));
179280 await sandbox.run({
281+ kind: "agent",
180282 actor,
181283 repo,
182284 number: pull.number,
+4−0
2424 { "binding": "IDENTITY", "service": "g1t-identity" },
2525 { "binding": "WORK", "service": "g1t-work" }
2626 ],
27+ // Events it reacts to: a pull request ready for review, or its head moving.
28+ "queues": {
29+ "consumers": [{ "queue": "g1t-events-runner", "max_batch_size": 20, "max_batch_timeout": 1 }]
30+ },
2731 "vars": {
2832 "HOSTED_AGENT_USERS": "syntaqx",
2933 // What a person can choose when starting g1t agents. The first is the
+4−1
33 version = "0.1.0"
44 edition.workspace = true
55 license.workspace = true
6−description = "Intents, attempts and sessions."
6+description = "Issues, pull requests, reviews, checks and sessions."
77
88 [lib]
99 crate-type = ["cdylib"]
1414 serde.workspace = true
1515 serde_json.workspace = true
1616 worker.workspace = true
17+getrandom = { version = "0.2", features = ["js"] }
18+hex = "0.4"
19+sha2 = "0.10"
+26−0
5656 merged_at TEXT,
5757 -- The pull request merged instead of this one.
5858 superseded_by INTEGER,
59+ -- The latest run of the issue's acceptance checks, and where it stands.
60+ check_run_id TEXT,
61+ check_status TEXT,
5962 author_id TEXT NOT NULL,
6063 author_name TEXT NOT NULL,
6164 created_at TEXT NOT NULL,
7477 author_id TEXT NOT NULL,
7578 author_name TEXT NOT NULL,
7679 body TEXT NOT NULL,
80+ -- For a comment on one line of a pull request's change: the file, and
81+ -- the line as numbered after the change.
82+ path TEXT,
83+ line INTEGER,
84+ -- A reviewer's decision: approve or request_changes.
85+ verdict TEXT,
7786 created_at TEXT NOT NULL
7887 );
7988 CREATE INDEX comments_by_subject ON comments (repo_id, number, id);
8998 at TEXT NOT NULL,
9099 PRIMARY KEY (pull_id, seq)
91100 );
101+
102+-- Runs of an issue's acceptance checks against a pull request's head.
103+CREATE TABLE check_runs (
104+ id TEXT PRIMARY KEY,
105+ pull_id TEXT NOT NULL REFERENCES pulls (id),
106+ head_commit TEXT NOT NULL,
107+ -- queued, running, passed, failed or errored.
108+ status TEXT NOT NULL DEFAULT 'queued',
109+ -- JSON array of { command, passed, exitCode, output, durationMs }.
110+ results TEXT NOT NULL DEFAULT '[]',
111+ error TEXT,
112+ -- SHA-256 of the token the sandbox reports with.
113+ token_hash TEXT NOT NULL,
114+ created_at TEXT NOT NULL,
115+ finished_at TEXT
116+);
117+CREATE INDEX check_runs_by_pull ON check_runs (pull_id, id);
+323−0
1+//! Check runs: an issue's acceptance checks, run against a pull request's
2+//! head in a clean sandbox.
3+//!
4+//! This service keeps the record. The runner service starts the sandbox:
5+//! it asks for a job with `start_checks`, and the sandbox reports back
6+//! through the API with the job's one-time token. Nothing else can write a
7+//! result, including the agent whose work is being checked.
8+
9+use g1t_contracts::events::ChecksEvent;
10+use g1t_contracts::repos::{GetByIdArgs, HeadArgs, Repo, RepoPath};
11+use g1t_contracts::time::rfc3339;
12+use g1t_contracts::work::*;
13+use g1t_contracts::{FailureCode, Outcome, new_id};
14+use g1t_kit::now_ms;
15+use serde::Deserialize;
16+use sha2::{Digest, Sha256};
17+use worker::Result;
18+use worker::wasm_bindgen::JsValue;
19+
20+use crate::rows::PullRow;
21+use crate::{Work, optional};
22+
23+const MAX_OUTPUT_CHARS: usize = 16_000;
24+const MAX_RESULTS: usize = 20;
25+
26+#[derive(Deserialize)]
27+struct RunRow {
28+ id: String,
29+ pull_id: String,
30+ head_commit: String,
31+ status: CheckStatus,
32+ /// JSON array of results.
33+ results: String,
34+ error: Option<String>,
35+ token_hash: String,
36+ created_at: String,
37+ finished_at: Option<String>,
38+}
39+
40+impl From<RunRow> for CheckRun {
41+ fn from(row: RunRow) -> Self {
42+ CheckRun {
43+ id: row.id,
44+ head_commit: row.head_commit,
45+ status: row.status,
46+ results: serde_json::from_str(&row.results).unwrap_or_default(),
47+ error: row.error,
48+ created_at: row.created_at,
49+ finished_at: row.finished_at,
50+ }
51+ }
52+}
53+
54+fn hash(token: &str) -> String {
55+ hex::encode(Sha256::digest(token.as_bytes()))
56+}
57+
58+fn new_token() -> String {
59+ let mut bytes = [0u8; 32];
60+ getrandom::getrandom(&mut bytes).expect("no source of randomness");
61+ hex::encode(bytes)
62+}
63+
64+fn refused<T>(message: &str) -> Outcome<T> {
65+ Outcome::fail(FailureCode::Conflict, message)
66+}
67+
68+impl Work {
69+ /// The most recent check run of a pull request.
70+ pub(crate) async fn latest_checks(&self, pull_id: &str) -> Result<Option<CheckRun>> {
71+ Ok(self
72+ .db
73+ .prepare("SELECT * FROM check_runs WHERE pull_id = ? ORDER BY id DESC LIMIT 1")
74+ .bind(&[pull_id.into()])?
75+ .first::<RunRow>(None)
76+ .await?
77+ .map(CheckRun::from))
78+ }
79+
80+ /// Begins a check run for a pull request that is ready for review, and
81+ /// returns what a sandbox needs to carry it out. Any run still in
82+ /// progress for the pull request is abandoned.
83+ pub(crate) async fn start_checks(&self, a: StartChecksArgs) -> Result<Outcome<CheckJob>> {
84+ let pull = self
85+ .db
86+ .prepare("SELECT * FROM pulls WHERE id = ?")
87+ .bind(&[a.pull_id.as_str().into()])?
88+ .first::<PullRow>(None)
89+ .await?
90+ .map(Pull::from);
91+ let Some(pull) = pull else {
92+ return Ok(Outcome::fail(
93+ FailureCode::NotFound,
94+ "Pull request not found.",
95+ ));
96+ };
97+ if pull.status != PullStatus::Open {
98+ return Ok(refused(
99+ "Checks run once a pull request is ready for review.",
100+ ));
101+ }
102+ let issue = match pull.issue {
103+ Some(number) => self.issue(&pull.repo_id, number).await?,
104+ None => None,
105+ };
106+ let Some(issue) = issue.filter(|issue| !issue.checks.is_empty()) else {
107+ return Ok(refused(
108+ "This pull request's issue has no acceptance checks.",
109+ ));
110+ };
111+
112+ // The author can read both the repository and the pull request's source.
113+ let viewer = Some(pull.author.clone());
114+ let repo: Outcome<Repo> = g1t_kit::call(
115+ &self.repos,
116+ "get_by_id",
117+ &GetByIdArgs {
118+ id: pull.repo_id.clone(),
119+ viewer,
120+ },
121+ )
122+ .await?;
123+ let Outcome::Ok(repo) = repo else {
124+ return Ok(Outcome::fail(
125+ FailureCode::NotFound,
126+ "Pull request not found.",
127+ ));
128+ };
129+ // Asked of the store, since the recorded head can lag a push.
130+ let head: Option<String> = g1t_kit::call(
131+ &self.repos,
132+ "head",
133+ &HeadArgs {
134+ repo_id: pull.fork_repo_id.clone().unwrap_or_else(|| repo.id.clone()),
135+ branch: pull
136+ .branch
137+ .clone()
138+ .unwrap_or_else(|| repo.default_branch.clone()),
139+ },
140+ )
141+ .await?;
142+ let Some(commit) = head else {
143+ return Ok(refused("This pull request has no commits to check."));
144+ };
145+
146+ let now = now_ms();
147+ let timestamp = rfc3339(now);
148+ let run_id = new_id("chk", now);
149+ let token = new_token();
150+ self.db
151+ .batch(vec![
152+ self.db
153+ .prepare(
154+ "UPDATE check_runs
155+ SET status = 'errored', error = 'Replaced by a newer run.', finished_at = ?
156+ WHERE pull_id = ? AND finished_at IS NULL",
157+ )
158+ .bind(&[timestamp.as_str().into(), pull.id.as_str().into()])?,
159+ self.db
160+ .prepare(
161+ "INSERT INTO check_runs (id, pull_id, head_commit, token_hash, created_at)
162+ VALUES (?, ?, ?, ?, ?)",
163+ )
164+ .bind(&[
165+ run_id.as_str().into(),
166+ pull.id.as_str().into(),
167+ commit.as_str().into(),
168+ hash(&token).into(),
169+ timestamp.as_str().into(),
170+ ])?,
171+ self.db
172+ .prepare(
173+ "UPDATE pulls SET check_status = 'queued', check_run_id = ?, head_commit = ?
174+ WHERE id = ?",
175+ )
176+ .bind(&[
177+ run_id.as_str().into(),
178+ commit.as_str().into(),
179+ pull.id.as_str().into(),
180+ ])?,
181+ ])
182+ .await?;
183+
184+ Ok(Outcome::Ok(CheckJob {
185+ run_id,
186+ token,
187+ commands: issue.checks,
188+ source: pull.fork.clone().unwrap_or(RepoPath {
189+ namespace: repo.namespace.clone(),
190+ name: repo.name.clone(),
191+ }),
192+ commit,
193+ author: pull.author,
194+ requested_by: issue.author.username,
195+ repo: RepoPath {
196+ namespace: repo.namespace,
197+ name: repo.name,
198+ },
199+ number: pull.number,
200+ }))
201+ }
202+
203+ /// Records what a sandbox reports for its run: that it has started, its
204+ /// results, that it could not run, or that the run should be forgotten.
205+ /// The run's token is the only credential, and a finished run accepts
206+ /// nothing more.
207+ pub(crate) async fn report_checks(&self, a: ReportChecksArgs) -> Result<Outcome<CheckRun>> {
208+ let run = self
209+ .db
210+ .prepare("SELECT * FROM check_runs WHERE id = ?")
211+ .bind(&[a.run_id.as_str().into()])?
212+ .first::<RunRow>(None)
213+ .await?;
214+ let Some(run) = run.filter(|run| run.token_hash == hash(&a.token)) else {
215+ return Ok(Outcome::fail(FailureCode::NotFound, "Check run not found."));
216+ };
217+ if run.finished_at.is_some() {
218+ return Ok(refused("This check run has already finished."));
219+ }
220+ let latest = "id = ? AND check_run_id = ?";
221+ let pull_keys =
222+ || -> [JsValue; 2] { [run.pull_id.as_str().into(), run.id.as_str().into()] };
223+
224+ if a.skip {
225+ self.db
226+ .batch(vec![
227+ self.db
228+ .prepare("DELETE FROM check_runs WHERE id = ?")
229+ .bind(&[run.id.as_str().into()])?,
230+ self.db
231+ .prepare(format!(
232+ "UPDATE pulls SET check_status = NULL, check_run_id = NULL WHERE {latest}"
233+ ))
234+ .bind(&pull_keys())?,
235+ ])
236+ .await?;
237+ return Ok(Outcome::Ok(run.into()));
238+ }
239+
240+ let results: Vec<CheckResult> = a
241+ .results
242+ .into_iter()
243+ .take(MAX_RESULTS)
244+ .map(|mut result| {
245+ // Keep the end of long output: that is where failures are.
246+ let length = result.output.chars().count();
247+ if length > MAX_OUTPUT_CHARS {
248+ result.output = result
249+ .output
250+ .chars()
251+ .skip(length - MAX_OUTPUT_CHARS)
252+ .collect();
253+ }
254+ result
255+ })
256+ .collect();
257+ let status = if a.error.is_some() {
258+ CheckStatus::Errored
259+ } else if results.is_empty() {
260+ CheckStatus::Running
261+ } else if results.iter().all(|result| result.passed) {
262+ CheckStatus::Passed
263+ } else {
264+ CheckStatus::Failed
265+ };
266+ let finished = (status != CheckStatus::Running).then(|| rfc3339(now_ms()));
267+ self.db
268+ .batch(vec![
269+ self.db
270+ .prepare(
271+ "UPDATE check_runs SET status = ?, results = ?, error = ?, finished_at = ?
272+ WHERE id = ?",
273+ )
274+ .bind(&[
275+ status.as_str().into(),
276+ serde_json::to_string(&results)?.into(),
277+ optional(&a.error),
278+ optional(&finished),
279+ run.id.as_str().into(),
280+ ])?,
281+ self.db
282+ .prepare(format!("UPDATE pulls SET check_status = ? WHERE {latest}"))
283+ .bind(&[
284+ status.as_str().into(),
285+ run.pull_id.as_str().into(),
286+ run.id.as_str().into(),
287+ ])?,
288+ ])
289+ .await?;
290+
291+ if finished.is_some() {
292+ let pull = self
293+ .db
294+ .prepare("SELECT * FROM pulls WHERE id = ?")
295+ .bind(&[run.pull_id.as_str().into()])?
296+ .first::<PullRow>(None)
297+ .await?
298+ .map(Pull::from);
299+ if let Some(pull) = pull {
300+ self.publish_as(
301+ "checks.completed",
302+ &pull.repo_id,
303+ None,
304+ ChecksEvent {
305+ pull_id: pull.id.clone(),
306+ repo_id: pull.repo_id.clone(),
307+ number: pull.number,
308+ status: status.as_str(),
309+ commit: run.head_commit.clone(),
310+ },
311+ )
312+ .await?;
313+ }
314+ }
315+ Ok(Outcome::Ok(CheckRun {
316+ status,
317+ results,
318+ error: a.error,
319+ finished_at: finished,
320+ ..run.into()
321+ }))
322+ }
323+}
+109−17
44 //! `g1t_contracts::work` for the methods and their arguments. It also
55 //! consumes its queue of events from the bus.
66
7+mod checks;
78 mod rows;
89
910 use g1t_contracts::events::{
2021 Context, D1Database, Env, Fetcher, MessageBatch, MessageExt, Request, Response, Result, event,
2122 };
2223
23−use rows::{CommentRow, IssueRow, NumberRow, PullRow, SessionRow, ValueRow};
24+use rows::{CommentRow, IssueRow, MovedRow, NumberRow, PullRow, SessionRow, ValueRow};
2425
2526 const SOURCE: &str = "work";
2627 const MAX_ENTRY_BATCH: usize = 200;
9596 actor: &User,
9697 data: T,
9798 ) -> Result<()> {
99+ self.publish_as(kind, repo_id, Some(actor.id.clone()), data)
100+ .await
101+ }
102+
103+ /// Publishes an event caused by `actor`, or by g1t itself.
104+ async fn publish_as<T: Serialize>(
105+ &self,
106+ kind: &'static str,
107+ repo_id: &str,
108+ actor: Option<String>,
109+ data: T,
110+ ) -> Result<()> {
98111 let event = NewEvent {
99112 kind,
100113 source: SOURCE,
101114 repo_id: Some(repo_id.to_owned()),
102− actor: Some(actor.id.clone()),
115+ actor,
103116 data,
104117 };
105118 g1t_kit::call(
528541 return Ok(Outcome::fail(FailureCode::Forbidden, UNVERIFIED));
529542 }
530543 let body = a.body.trim();
531− if body.is_empty() {
544+ // An approval speaks for itself; anything else has to say something.
545+ if body.is_empty() && a.verdict != Some(Verdict::Approve) {
532546 return Ok(Outcome::fail(
533547 FailureCode::Invalid,
534548 "A comment cannot be empty.",
535549 ));
536550 }
551+ let path = a
552+ .path
553+ .as_deref()
554+ .map(str::trim)
555+ .filter(|path| !path.is_empty());
556+ let line = a.line.filter(|line| *line > 0 && path.is_some());
537557 if body.chars().count() > MAX_ENTRY_CHARS {
538558 return Ok(Outcome::fail(
539559 FailureCode::Invalid,
543563 let repo = check!(self.repo(&a.repo, &Some(a.actor.clone())).await?);
544564 // The number names an issue or a pull request, never both.
545565 let table = if self.issue(&repo.id, a.number).await?.is_some() {
566+ if path.is_some() || a.verdict.is_some() {
567+ return Ok(Outcome::fail(
568+ FailureCode::Invalid,
569+ "Only a pull request can be reviewed or commented on by line.",
570+ ));
571+ }
546572 "issues"
547− } else if self.pull(&repo.id, a.number).await?.is_some() {
573+ } else if let Some(pull) = self.pull(&repo.id, a.number).await? {
574+ if a.verdict.is_some() && pull.author.id == a.actor.id {
575+ return Ok(Outcome::fail(
576+ FailureCode::Forbidden,
577+ "You cannot approve or request changes on your own pull request.",
578+ ));
579+ }
548580 "pulls"
549581 } else {
550582 return Ok(Outcome::fail(
558590 id: new_id("cmt", now),
559591 author: a.actor.clone(),
560592 body: body.to_owned(),
593+ path: path.map(str::to_owned),
594+ line,
595+ verdict: a.verdict,
561596 created_at: rfc3339(now),
562597 };
563598 self.db
565600 self.db
566601 .prepare(
567602 "INSERT INTO comments
568− (id, repo_id, number, author_id, author_name, body, created_at)
569− VALUES (?, ?, ?, ?, ?, ?, ?)",
603+ (id, repo_id, number, author_id, author_name, body, path, line,
604+ verdict, created_at)
605+ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
570606 )
571607 .bind(&[
572608 comment.id.as_str().into(),
575611 a.actor.id.as_str().into(),
576612 a.actor.username.as_str().into(),
577613 body.into(),
614+ optional(&comment.path),
615+ optional_number(line),
616+ a.verdict
617+ .map_or(JsValue::NULL, |verdict| verdict.as_str().into()),
578618 comment.created_at.as_str().into(),
579619 ])?,
580620 self.db
783823 };
784824 Ok(Outcome::Ok(PullDetail {
785825 comments: self.comments(&repo.id, pull.number).await?,
826+ checks: self.latest_checks(&pull.id).await?,
786827 issue,
787828 pull,
788829 }))
887928 ));
888929 }
889930 }
931+ if !a.ignore_checks {
932+ let waiting = match pull.check_status {
933+ Some(CheckStatus::Queued | CheckStatus::Running) => {
934+ Some("The acceptance checks are still running.")
935+ }
936+ Some(CheckStatus::Failed) => Some("The acceptance checks did not pass."),
937+ Some(CheckStatus::Errored) => Some("The acceptance checks could not be run."),
938+ Some(CheckStatus::Passed) | None => None,
939+ };
940+ if let Some(reason) = waiting {
941+ return Ok(Outcome::fail(
942+ FailureCode::Conflict,
943+ format!("{reason} Wait or fix them, or merge anyway by ignoring the checks."),
944+ ));
945+ }
946+ }
890947 let issue = match pull.issue {
891948 Some(number) if !a.keep_issue_open => self
892949 .issue(&repo.id, number)
11171174 return Ok(());
11181175 };
11191176 let now = rfc3339(now_ms());
1120− let active = "status IN ('draft', 'open')";
1121− let mut statements = Vec::new();
1177+ // The head moved, so whatever the checks said no longer applies.
1178+ let moved = "UPDATE pulls
1179+ SET head_commit = ?, updated_at = ?, check_status = NULL, check_run_id = NULL";
1180+ let active = "status IN ('draft', 'open') AND head_commit IS NOT ?";
1181+ let returning = "RETURNING id, repo_id, number, issue_number, status";
1182+ let mut pulls: Vec<MovedRow> = Vec::new();
11221183 // A fork carries its pull request on its default branch.
11231184 if event.data["defaultBranch"].as_bool() == Some(true) {
1124− statements.push(
1185+ pulls.extend(
11251186 self.db
11261187 .prepare(format!(
1127− "UPDATE pulls SET head_commit = ?, updated_at = ?
1128− WHERE fork_repo_id = ? AND {active}"
1188+ "{moved} WHERE fork_repo_id = ? AND {active} {returning}"
11291189 ))
1130− .bind(&[after.into(), now.as_str().into(), repo_id.into()])?,
1190+ .bind(&[
1191+ after.into(),
1192+ now.as_str().into(),
1193+ repo_id.into(),
1194+ after.into(),
1195+ ])?
1196+ .all()
1197+ .await?
1198+ .results::<MovedRow>()?,
11311199 );
11321200 }
11331201 if let Some(branch) = git_ref.strip_prefix("refs/heads/") {
1134− statements.push(
1202+ pulls.extend(
11351203 self.db
11361204 .prepare(format!(
1137− "UPDATE pulls SET head_commit = ?, updated_at = ?
1138− WHERE repo_id = ? AND source_branch = ? AND {active}"
1205+ "{moved} WHERE repo_id = ? AND source_branch = ? AND {active} {returning}"
11391206 ))
11401207 .bind(&[
11411208 after.into(),
11421209 now.as_str().into(),
11431210 repo_id.into(),
11441211 branch.into(),
1145− ])?,
1212+ after.into(),
1213+ ])?
1214+ .all()
1215+ .await?
1216+ .results::<MovedRow>()?,
11461217 );
11471218 }
1148− self.db.batch(statements).await?;
1219+ // A draft is announced when it is marked ready instead.
1220+ for pull in pulls
1221+ .into_iter()
1222+ .filter(|pull| pull.status == PullStatus::Open)
1223+ {
1224+ self.publish_as(
1225+ "pull.updated",
1226+ &pull.repo_id,
1227+ event.actor.clone(),
1228+ PullEvent {
1229+ pull_id: pull.id,
1230+ repo_id: pull.repo_id.clone(),
1231+ number: pull.number,
1232+ issue: pull.issue_number,
1233+ commit: Some(after.to_owned()),
1234+ ..PullEvent::default()
1235+ },
1236+ )
1237+ .await?;
1238+ }
11491239 Ok(())
11501240 }
11511241 }
11761266 "list_labels" => reply(&work.list_labels(args(body)?).await?),
11771267 "counts" => reply(&work.counts(args(body)?).await?),
11781268 "add_comment" => reply(&work.add_comment(args(body)?).await?),
1269+ "start_checks" => reply(&work.start_checks(args(body)?).await?),
1270+ "report_checks" => reply(&work.report_checks(args(body)?).await?),
11791271 "open_pull" => reply(&work.open_pull(args(body)?).await?),
11801272 "list_pulls" => reply(&work.list_pulls(args(body)?).await?),
11811273 "get_pull" => reply(&work.get_pull(args(body)?).await?),
+20−1
33 use g1t_contracts::User;
44 use g1t_contracts::repos::RepoPath;
55 use g1t_contracts::work::{
6− Comment, Issue, IssueReason, Pull, PullStatus, Runtime, SessionEntry, SessionEntryKind, State,
6+ CheckStatus, Comment, Issue, IssueReason, Pull, PullStatus, Runtime, SessionEntry,
7+ SessionEntryKind, State, Verdict,
78 };
89 use serde::Deserialize;
910
8182 pub merged_by: Option<String>,
8283 pub merged_at: Option<String>,
8384 pub superseded_by: Option<u32>,
85+ pub check_status: Option<CheckStatus>,
8486 pub author_id: String,
8587 pub author_name: String,
8688 pub created_at: String,
110112 merged_by: row.merged_by,
111113 merged_at: row.merged_at,
112114 superseded_by: row.superseded_by,
115+ check_status: row.check_status,
113116 author: user(row.author_id, row.author_name),
114117 created_at: row.created_at,
115118 updated_at: row.updated_at,
123126 pub author_id: String,
124127 pub author_name: String,
125128 pub body: String,
129+ pub path: Option<String>,
130+ pub line: Option<u32>,
131+ pub verdict: Option<Verdict>,
126132 pub created_at: String,
127133 }
128134
132138 id: row.id,
133139 author: user(row.author_id, row.author_name),
134140 body: row.body,
141+ path: row.path,
142+ line: row.line,
143+ verdict: row.verdict,
135144 created_at: row.created_at,
136145 }
137146 }
160169 }
161170 }
162171
172+/// A pull request whose head a push moved.
173+#[derive(Deserialize)]
174+pub struct MovedRow {
175+ pub id: String,
176+ pub repo_id: String,
177+ pub number: u32,
178+ pub issue_number: Option<u32>,
179+ pub status: PullStatus,
180+}
181+
163182 /// A single number selected as `n`.
164183 #[derive(Deserialize)]
165184 pub struct NumberRow {