From db147cfc1ff595e5450d1c38fbf2ffa1dee16365 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 26 Sep 2026 08:01:07 -0400 Subject: [PATCH 1/6] api: Include GitHub's error details in failures A 422 only said "Unprocessable Entity"; the reason (e.g. "Can not approve your own pull request") is in the errors array, and it is what a refused review needs to show. Generated-by: AI --- src/github/api.ts | 12 ++++++++++-- test/api.test.ts | 9 +++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/github/api.ts b/src/github/api.ts index a64351d..cfb1b1c 100644 --- a/src/github/api.ts +++ b/src/github/api.ts @@ -45,11 +45,19 @@ export function nextLink(link: string | null): string | undefined { return undefined; } +/** A message, or an error entry's message (GitHub sends strings or objects). */ +function errorText(e: unknown): string { + if (typeof e === "string") return e; + const m = (e as { message?: unknown } | null)?.message; + return typeof m === "string" ? m : ""; +} + async function errorMessage(res: Response, what: string): Promise { let detail = ""; try { - const body = (await res.json()) as { message?: unknown }; - if (typeof body.message === "string") detail = `: ${body.message}`; + const body = (await res.json()) as { message?: unknown; errors?: unknown }; + const parts = [body.message, ...(Array.isArray(body.errors) ? body.errors : [])].map(errorText).filter(Boolean); + if (parts.length) detail = `: ${parts.join("; ")}`; } catch { // Not JSON; the status is enough. } diff --git a/test/api.test.ts b/test/api.test.ts index 4e3eb01..120c669 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -44,6 +44,15 @@ describe("GitHub.get", () => { }); }); + it("includes GitHub's error details", async () => { + const { fetchImpl } = scriptedFetch(() => ({ + status: 422, + body: { message: "Unprocessable Entity", errors: ["Can not approve your own pull request", { message: "second" }, { code: "x" }] }, + })); + const gh = new GitHub(token, fetchImpl); + await assert.rejects(gh.send("POST", "/x"), /HTTP 422: Unprocessable Entity; Can not approve your own pull request; second$/); + }); + it("refuses paths that would leave the API origin", async () => { const gh = new GitHub(token, scriptedFetch(() => ({})).fetchImpl); await assert.rejects(gh.get("https://evil.example/x"), /refusing to send the token outside/); From 50cec9770cf26abfd3678bfcc48f1710776b0001 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 26 Sep 2026 08:01:07 -0400 Subject: [PATCH 2/6] api: Track the core rate limit only Search answers carry their own x-ratelimit-* headers (30 a minute), so reading the forge's PRs made the meta line flip between the two budgets and could trigger the low-budget backoff for the wrong one. Generated-by: AI --- src/github/api.ts | 3 +++ test/api.test.ts | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/github/api.ts b/src/github/api.ts index cfb1b1c..7275482 100644 --- a/src/github/api.ts +++ b/src/github/api.ts @@ -92,6 +92,9 @@ export class GitHub { } #noteRate(res: Response): void { + // Search and GraphQL have budgets of their own; track the core one. + const resource = res.headers.get("x-ratelimit-resource"); + if (resource !== null && resource !== "core") return; const limit = Number(res.headers.get("x-ratelimit-limit")); const remaining = Number(res.headers.get("x-ratelimit-remaining")); const reset = Number(res.headers.get("x-ratelimit-reset")); diff --git a/test/api.test.ts b/test/api.test.ts index 120c669..69f4734 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -66,6 +66,16 @@ describe("GitHub.get", () => { assert.equal(calls.length, 1); }); + it("tracks the core rate limit only", async () => { + const rate = (resource: string, remaining: string) => ({ "x-ratelimit-resource": resource, "x-ratelimit-limit": resource === "search" ? "30" : "5000", "x-ratelimit-remaining": remaining }); + const { fetchImpl } = scriptedFetch((_m, url) => ({ body: {}, headers: url.includes("search") ? rate("search", "2") : rate("core", "4000") })); + const gh = new GitHub(token, fetchImpl); + await gh.get("/user"); + await gh.get("/search/issues?q=x"); + assert.deepEqual(gh.rate, { limit: 5000, remaining: 4000, reset: 0 }); + assert.equal(gh.rateLow(0.1), false); + }); + it("notes a classic token's scopes", async () => { const { fetchImpl } = scriptedFetch(() => ({ body: {}, headers: { "x-oauth-scopes": "gist, repo" } })); const gh = new GitHub(token, fetchImpl); From 017745614d378dbca7072dbc9ac26d69a06bf8ff Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 26 Sep 2026 08:01:07 -0400 Subject: [PATCH 3/6] board: Queue Draft items as well as Needs human The board's "Needs cgwalters" view is Needs human and Draft: Draft items are forge PRs and gists ready for his review, and the app is meant to be his one inbox. Also keep each item's created_at, to rank by how long it has been waiting. Generated-by: AI --- src/github/backend.ts | 6 +++--- src/github/board.ts | 10 +++++++--- src/github/config.ts | 6 +++++- test/backend.test.ts | 2 +- test/board.test.ts | 2 +- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/github/backend.ts b/src/github/backend.ts index 2b85df6..95b1ee5 100644 --- a/src/github/backend.ts +++ b/src/github/backend.ts @@ -13,7 +13,7 @@ import { type RawItem, queueItems, } from "./board.ts"; -import { BOARD_NUMBER, BOARD_OWNER, NEEDS_HUMAN, OPERATOR, PAGE_SIZE, RECENT_COMMENTS } from "./config.ts"; +import { BOARD_NUMBER, BOARD_OWNER, OPERATOR, PAGE_SIZE, QUEUE_STATUSES, RECENT_COMMENTS } from "./config.ts"; import { checkReceipt, RECEIPT_FILE, type RawReceiptGist, type ReceiptCheck } from "./receipt.ts"; const PROJECT = `/users/${BOARD_OWNER}/projectsV2/${BOARD_NUMBER}`; @@ -25,14 +25,14 @@ export interface Queue { changed: boolean; } -/** Read the items needing a human, conditionally: 304s cost nothing. */ +/** Read the items needing a human or ready for review, conditionally: 304s cost nothing. */ export async function loadQueue(gh: GitHub): Promise { const project = await gh.get<{ public?: boolean }>(PROJECT); const fields = await gh.getAll(`${PROJECT}/fields?per_page=${PAGE_SIZE}`); const ids = fieldIds(fields.data).join(","); // The server-side filter keeps the poll to one page; queueItems filters // again, in case the filter syntax ever stops matching. - const q = encodeURIComponent(`status:"${NEEDS_HUMAN}"`); + const q = encodeURIComponent(`status:${QUEUE_STATUSES.map((s) => `"${s}"`).join(",")}`); const items = await gh.getAll(`${PROJECT}/items?per_page=${PAGE_SIZE}&fields=${ids}&q=${q}`); return { items: queueItems(items.data), diff --git a/src/github/board.ts b/src/github/board.ts index 7622432..8049b1e 100644 --- a/src/github/board.ts +++ b/src/github/board.ts @@ -3,7 +3,7 @@ // synthetic payloads. import { AnswerError, type Option, parseOptions, questionId, withoutDraftSection } from "../answer.ts"; -import { FIELD, NEEDS_HUMAN } from "./config.ts"; +import { FIELD, QUEUE_STATUSES } from "./config.ts"; /** The subset of a project field the app uses. */ export interface RawField { @@ -42,6 +42,7 @@ export interface RawItem { content_type: string; content?: RawContent | null; fields?: RawFieldValue[]; + created_at?: string; updated_at?: string; archived_at?: string | null; } @@ -78,6 +79,8 @@ export interface Item { org?: string; branch: string[]; gist: string[]; + /** When the item was added to the board. */ + createdAt?: string; updatedAt?: string; } @@ -159,6 +162,7 @@ export function parseItem(raw: RawItem): Item { opt("status", fields.get(FIELD.status)); opt("priority", fields.get(FIELD.priority)); opt("org", fields.get(FIELD.org)); + opt("createdAt", raw.created_at); opt("updatedAt", c.updated_at ?? raw.updated_at); if (kind === "draft") { opt("draftId", c.node_id); @@ -171,12 +175,12 @@ export function parseItem(raw: RawItem): Item { return item; } -/** The queue: unarchived items needing a human. */ +/** The board's part of the queue: unarchived items needing a human, or Draft (ready for review). */ export function queueItems(raw: readonly RawItem[]): Item[] { return raw .filter((r) => !r.archived_at) .map(parseItem) - .filter((i) => i.status === NEEDS_HUMAN); + .filter((i) => i.status !== undefined && QUEUE_STATUSES.includes(i.status)); } export interface PriorityGroup { diff --git a/src/github/config.ts b/src/github/config.ts index 025321b..69a68da 100644 --- a/src/github/config.ts +++ b/src/github/config.ts @@ -11,8 +11,12 @@ export const BOARD_URL = `https://github.com/users/${BOARD_OWNER}/projects/${BOA /** The only login whose answers the bot acts on. */ export const OPERATOR = "cgwalters"; -/** The Status value that puts an item in the queue. */ +/** The Status of an item blocked on his decision or action. */ export const NEEDS_HUMAN = "Needs human"; +/** The Status of an item ready for his review (a forge PR or a gist). */ +export const DRAFT = "Draft"; +/** The Statuses that put an item in the queue (the board's "Needs cgwalters" view). */ +export const QUEUE_STATUSES: readonly string[] = [NEEDS_HUMAN, DRAFT]; /** Board fields the app reads, by name; their ids are looked up at runtime. */ export const FIELD = { diff --git a/test/backend.test.ts b/test/backend.test.ts index 3fcc47b..25183db 100644 --- a/test/backend.test.ts +++ b/test/backend.test.ts @@ -42,7 +42,7 @@ describe("loadQueue", () => { assert.equal(first.items.length, 4); const itemsUrl = new URL(calls[2]?.url ?? ""); assert.equal(itemsUrl.searchParams.get("fields"), "102,104,103,105,106,107"); - assert.equal(itemsUrl.searchParams.get("q"), 'status:"Needs human"'); + assert.equal(itemsUrl.searchParams.get("q"), 'status:"Needs human","Draft"'); assert.equal((await loadQueue(gh)).changed, false); boardPublic = false; diff --git a/test/board.test.ts b/test/board.test.ts index 2687490..84f5c21 100644 --- a/test/board.test.ts +++ b/test/board.test.ts @@ -29,7 +29,7 @@ describe("queueItems", () => { const items = queueItems(rawItems()); const byId = new Map(items.map((i) => [i.nodeId, i])); - it("keeps unarchived Needs human items only, in board order", () => { + it("keeps unarchived Needs human and Draft items only, in board order", () => { assert.deepEqual( items.map((i) => i.nodeId), ["PVTI_synthetic_upstream_pr", "PVTI_synthetic_draft", "PVTI_synthetic_home_issue", "PVTI_synthetic_redacted"], From cc7b9ab58f52664b99e096fb4c175a65023d0e01 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 26 Sep 2026 08:01:08 -0400 Subject: [PATCH 4/6] github: Add the forge PR model and its API calls Prep for the PR review pane: cgwalters wants the bot's forge PRs in the same inbox as its questions, reviewed without going to github.com. The review is what bot-pr promote keys on, so the verdict mirrors its rule: his latest approving, change-requesting or dismissing review, read against the current head, or conversation comment with a /promote line. bot-pr dates a /promote against the fork's push log, which the app can't read, so such a PR shows as promoted and stays in the queue until bot-pr acts. A submitted review always names the head he was shown, after re-reading the PR to make sure it didn't move, and the detail read flags a commit list that doesn't end at the head yet (GitHub lags a push), so the pane can refuse to approve a stale diff. His text may not carry a line bot-pr would act on; the only command the app writes is the /draft line of an approval he asked for. The forge search has no ETags and a small budget, so verdicts are re-read only for PRs whose updated_at moved, with heads from one conditional open-PR list per repository. Generated-by: AI --- src/github/config.ts | 7 + src/github/forge.ts | 349 +++++++++++++++++++++++++++++++++++++++++++ src/github/prs.ts | 297 ++++++++++++++++++++++++++++++++++++ test/forge.test.ts | 216 ++++++++++++++++++++++++++ test/prs.test.ts | 222 +++++++++++++++++++++++++++ 5 files changed, 1091 insertions(+) create mode 100644 src/github/forge.ts create mode 100644 src/github/prs.ts create mode 100644 test/forge.test.ts create mode 100644 test/prs.test.ts diff --git a/src/github/config.ts b/src/github/config.ts index 69a68da..765a4f7 100644 --- a/src/github/config.ts +++ b/src/github/config.ts @@ -18,6 +18,11 @@ export const DRAFT = "Draft"; /** The Statuses that put an item in the queue (the board's "Needs cgwalters" view). */ export const QUEUE_STATUSES: readonly string[] = [NEEDS_HUMAN, DRAFT]; +/** The organization holding the forks where the bot proposes draft PRs. */ +export const FORGE_ORG = "cgwalters-forge"; +/** The bot's login: the forge PRs listed are the ones it opened. */ +export const BOT_LOGIN = "cgwalters-bot"; + /** Board fields the app reads, by name; their ids are looked up at runtime. */ export const FIELD = { status: "Status", @@ -37,6 +42,8 @@ export const HOME_OWNERS: readonly string[] = ["cgwalters-forge", "cgwalters-bot /** Poll every this many ms while the tab is visible. */ export const POLL_INTERVAL_MS = 30_000; +/** Parallel requests when refreshing PR verdicts. */ +export const FETCH_CONCURRENCY = 6; /** Poll this many times slower when the rate budget runs low. */ export const POLL_BACKOFF_FACTOR = 4; /** Below this fraction of the hourly budget, back off. */ diff --git a/src/github/forge.ts b/src/github/forge.ts new file mode 100644 index 0000000..99690f0 --- /dev/null +++ b/src/github/forge.ts @@ -0,0 +1,349 @@ +// Forge PRs: the bot's draft PRs in cgwalters-forge that wait for +// cgwalters' review. Pure functions over REST JSON (search results, +// reviews, patches, checks) and the review the app submits, so tests +// feed them synthetic payloads. +// +// The review is what `bot-pr promote` keys on: the latest APPROVED, +// CHANGES_REQUESTED or DISMISSED review by cgwalters, or conversation +// comment of his with a `/promote` line, decides, and an approval counts +// only for the commit it names (commit_id), which must be the PR's +// current head. So the app always submits with commit_id set to the head +// it showed, after checking the head didn't move. A `/promote` comment +// names no commit; bot-pr dates it against the push log, which the app +// can't read, so it shows such a PR as promoted but leaves the decision +// (and the PR, in the queue) to bot-pr. + +import { AnswerError, isCommandLine } from "../answer.ts"; +import { type IssueRef, parseIssueUrl } from "./board.ts"; + +/** A line in an approving review asking promote for a draft upstream PR. */ +export const DRAFT_LINE = "/draft"; +/** Lines in his conversation comments that approve, as bot-pr reads them. */ +export const PROMOTE_LINES: readonly string[] = ["/promote", "/promote --human-text"]; + +/** `owner/repo#number`, the key the app uses for a PR everywhere. */ +export function refKey(r: IssueRef): string { + return `${r.owner}/${r.repo}#${r.number}`; +} + +export interface ForgePr { + ref: IssueRef; + url: string; + title: string; + body: string; + author: string; + createdAt: string; + updatedAt: string; + draft: boolean; +} + +/** The subset of a search/issues result the app uses. */ +export interface RawSearchIssue { + html_url?: string; + title?: string; + body?: string | null; + user?: { login?: string } | null; + created_at?: string; + updated_at?: string; + draft?: boolean; + pull_request?: unknown; +} + +/** A PR from a search result, or undefined if it isn't one. */ +export function parseSearchPr(raw: RawSearchIssue): ForgePr | undefined { + if (!raw.pull_request || !raw.html_url || !/\/pull\/\d+$/.test(raw.html_url)) return undefined; + const ref = parseIssueUrl(raw.html_url); + if (!ref) return undefined; + return { + ref, + url: raw.html_url, + title: raw.title?.trim() || "(no title)", + body: raw.body ?? "", + author: raw.user?.login ?? "ghost", + createdAt: raw.created_at ?? "", + updatedAt: raw.updated_at ?? "", + draft: raw.draft === true, + }; +} + +const META_START = ""; +const META_END = ""; + +/** What bot-pr records in a fork PR's bot-meta section. */ +export interface BotMeta { + /** The upstream repository, `owner/repo`. */ + upstream?: string; + /** The upstream base branch. */ + base?: string; + /** The Workstream board item, `PVTI_...`. */ + item?: string; +} + +function metaSection(body: string): string | undefined { + const start = body.indexOf(META_START); + if (start < 0) return undefined; + const end = body.indexOf(META_END, start); + return body.slice(start + META_START.length, end < 0 ? undefined : end); +} + +/** Parse the bot-meta section of a fork PR body; empty if there is none. */ +export function parseBotMeta(body: string): BotMeta { + const meta: BotMeta = {}; + const section = metaSection(body.replace(/\r\n?/g, "\n")); + if (section === undefined) return meta; + const up = /^- Upstream: `([A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+)`(?:, base `([^`\s]+)`)?/m.exec(section); + if (up?.[1]) meta.upstream = up[1]; + if (up?.[2]) meta.base = up[2]; + const item = /^- Board item: `(PVTI_[A-Za-z0-9_-]+)`/m.exec(section); + if (item?.[1]) meta.item = item[1]; + return meta; +} + +/** The body as it will read upstream: without the bot-meta section. */ +export function withoutBotMeta(body: string): string { + const start = body.indexOf(META_START); + if (start < 0) return body; + const end = body.indexOf(META_END, start); + return (body.slice(0, start) + (end < 0 ? "" : body.slice(end + META_END.length))).trimEnd(); +} + +export interface RawReview { + id?: number; + user?: { login?: string } | null; + state?: string; + commit_id?: string | null; + submitted_at?: string | null; + html_url?: string; + body?: string | null; +} + +export interface RawIssueComment { + user?: { login?: string } | null; + body?: string | null; + created_at?: string; + html_url?: string; +} + +/** Whether a comment has a line bot-pr reads as `/promote` (trimming spaces and tabs, as it does). */ +export function hasPromoteLine(body: string): boolean { + return body + .replace(/\r/g, "") + .split("\n") + .some((l) => PROMOTE_LINES.includes(l.replace(/^[ \t]+|[ \t]+$/g, ""))); +} + +export type VerdictState = + /** No approval or change request by him (or the last was dismissed). */ + | "none" + /** He approved the current head: promote can go ahead. */ + | "approved" + /** He approved an older head; the commits since are unreviewed. */ + | "approved-older" + /** He asked for changes on the current head: the bot owes a push. */ + | "changes-requested" + /** He asked for changes, and the bot pushed since. */ + | "changes-requested-older" + /** His latest word is a `/promote` comment; bot-pr decides which head it approves. */ + | "promoted"; + +export interface Verdict { + state: VerdictState; + at?: string; + url?: string; +} + +const DECIDING = ["APPROVED", "CHANGES_REQUESTED", "DISMISSED"]; + +interface Decision { + kind: "APPROVED" | "CHANGES_REQUESTED" | "DISMISSED" | "PROMOTE"; + at: string; + commit?: string | null | undefined; + url?: string | undefined; +} + +/** + * His latest deciding review or `/promote` comment, read against the + * current head, as bot-pr does. Ties keep the later entry in API order, + * and entries without a date are ignored. + */ +export function reviewVerdict( + reviews: readonly RawReview[], + head: string, + reviewer: string, + comments: readonly RawIssueComment[] = [], +): Verdict { + const decisions: Decision[] = [ + ...reviews + .filter((r) => r.user?.login === reviewer && DECIDING.includes(r.state ?? "") && r.submitted_at) + .map((r): Decision => ({ kind: r.state as Decision["kind"], at: r.submitted_at ?? "", commit: r.commit_id, url: r.html_url })), + ...comments + .filter((c) => c.user?.login === reviewer && c.created_at && hasPromoteLine(c.body ?? "")) + .map((c): Decision => ({ kind: "PROMOTE", at: c.created_at ?? "", url: c.html_url })), + ]; + // A stable sort, so equal times keep reviews' and then comments' order. + // ISO 8601 times compare as strings, as bot-pr's sort_by does. + const last = decisions.sort((a, b) => (a.at < b.at ? -1 : a.at > b.at ? 1 : 0)).at(-1); + if (!last || last.kind === "DISMISSED") return { state: "none" }; + const current = last.commit === head; + const state: VerdictState = + last.kind === "PROMOTE" + ? "promoted" + : last.kind === "APPROVED" + ? current + ? "approved" + : "approved-older" + : current + ? "changes-requested" + : "changes-requested-older"; + const v: Verdict = { state, at: last.at }; + if (last.url) v.url = last.url; + return v; +} + +/** Whether the PR is still in his queue: not approved (nor sent back) at its head. */ +export function waitsOnReviewer(v: Verdict): boolean { + return v.state !== "approved" && v.state !== "changes-requested"; +} + +export const VERDICT_LABEL: Record = { + none: "not reviewed", + approved: "approved", + "approved-older": "approved an older head", + "changes-requested": "changes requested", + "changes-requested-older": "updated since your change request", + promoted: "/promote sent; bot-pr decides", +}; + +export type DiffKind = "hunk" | "add" | "del" | "ctx" | "note"; + +export interface DiffLine { + kind: DiffKind; + text: string; + /** Line number in the old file (del, ctx). */ + old?: number; + /** Line number in the new file (add, ctx). */ + new?: number; +} + +const HUNK_RE = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/; + +/** + * Parse the unified diff GitHub gives per file (the `patch` of a PR file: + * hunks only, no file headers) into numbered lines. + */ +export function parsePatch(patch: string): DiffLine[] { + const out: DiffLine[] = []; + let oldNo = 0; + let newNo = 0; + const lines = patch.replace(/\r\n?/g, "\n").split("\n"); + if (lines.at(-1) === "") lines.pop(); + for (const line of lines) { + const hunk = HUNK_RE.exec(line); + if (hunk) { + oldNo = Number(hunk[1]); + newNo = Number(hunk[2]); + out.push({ kind: "hunk", text: line }); + continue; + } + const mark = line[0]; + const text = line.slice(1); + if (mark === "+") out.push({ kind: "add", text, new: newNo++ }); + else if (mark === "-") out.push({ kind: "del", text, old: oldNo++ }); + else if (mark === "\\") out.push({ kind: "note", text: line }); + else out.push({ kind: "ctx", text, old: oldNo++, new: newNo++ }); + } + return out; +} + +export type CiState = "success" | "failure" | "pending" | "none"; + +export interface CiCheck { + name: string; + state: CiState; + url?: string; + /** The raw conclusion or state, e.g. "timed_out". */ + detail: string; +} + +export interface RawCheckRun { + name?: string; + status?: string; + conclusion?: string | null; + html_url?: string | null; + details_url?: string | null; +} + +export interface RawStatus { + context?: string; + state?: string; + target_url?: string | null; +} + +const PASSING = ["success", "neutral", "skipped"]; + +function withUrl(check: CiCheck, url: string | null | undefined): CiCheck { + if (url) check.url = url; + return check; +} + +/** Check runs and commit statuses as one list, failures first. */ +export function ciChecks(runs: readonly RawCheckRun[], statuses: readonly RawStatus[]): CiCheck[] { + const out: CiCheck[] = []; + for (const r of runs) { + const done = r.status === "completed"; + const detail = done ? (r.conclusion ?? "unknown") : (r.status ?? "queued"); + const state: CiState = !done ? "pending" : PASSING.includes(detail) ? "success" : "failure"; + out.push(withUrl({ name: r.name ?? "(unnamed)", state, detail }, r.html_url ?? r.details_url)); + } + for (const s of statuses) { + const detail = s.state ?? "unknown"; + const state: CiState = detail === "success" ? "success" : detail === "pending" ? "pending" : "failure"; + out.push(withUrl({ name: s.context ?? "(unnamed)", state, detail }, s.target_url)); + } + const order: CiState[] = ["failure", "pending", "success", "none"]; + return out.sort((a, b) => order.indexOf(a.state) - order.indexOf(b.state) || a.name.localeCompare(b.name)); +} + +/** One state for all checks: any failure, else any pending, else success. */ +export function ciSummary(checks: readonly CiCheck[]): CiState { + if (checks.length === 0) return "none"; + if (checks.some((c) => c.state === "failure")) return "failure"; + if (checks.some((c) => c.state === "pending")) return "pending"; + return "success"; +} + +export type ReviewAction = "approve" | "request-changes" | "comment"; + +/** The body of POST /repos/{o}/{r}/pulls/{n}/reviews. */ +export interface ReviewRequest { + commit_id: string; + event: "APPROVE" | "REQUEST_CHANGES" | "COMMENT"; + body: string; +} + +const EVENT: Record = { + approve: "APPROVE", + "request-changes": "REQUEST_CHANGES", + comment: "COMMENT", +}; + +/** + * Compose the review to submit. His text may not contain a line that + * bot-pr would read as a command (a `/draft` in a change request would + * still count), so the only command the app writes is the `/draft` line + * of an approval he asked for with the checkbox. + */ +export function composeReview(action: ReviewAction, text: string, head: string, opts: { draft?: boolean } = {}): ReviewRequest { + if (!/^[0-9a-f]{40}$/.test(head)) throw new AnswerError(`not a commit id: ${JSON.stringify(head)}`); + const clean = text.replace(/\r\n?/g, "\n").trim(); + const bad = clean.split("\n").find(isCommandLine); + if (bad !== undefined) { + throw new AnswerError(`the line ${JSON.stringify(bad.trim())} would be read as a bot command; reword it (e.g. put it in backticks)`); + } + if (action !== "approve" && !clean) { + throw new AnswerError(action === "comment" ? "write a comment first" : "say what to change"); + } + if (opts.draft && action !== "approve") throw new AnswerError(`${DRAFT_LINE} goes only with an approval`); + const body = [clean, opts.draft ? DRAFT_LINE : ""].filter(Boolean).join("\n\n"); + return { commit_id: head, event: EVENT[action], body }; +} diff --git a/src/github/prs.ts b/src/github/prs.ts new file mode 100644 index 0000000..6faad98 --- /dev/null +++ b/src/github/prs.ts @@ -0,0 +1,297 @@ +// What the app reads and writes for forge PRs: the list waiting on him, +// their verdicts, one PR's details for the review pane, and the review +// itself. Views call these; tests drive them with a scripted fetch. + +import type { GitHub } from "./api.ts"; +import type { IssueRef } from "./board.ts"; +import { BOT_LOGIN, FETCH_CONCURRENCY, FORGE_ORG, OPERATOR, PAGE_SIZE } from "./config.ts"; +import { + type CiCheck, + ciChecks, + type ForgePr, + parseSearchPr, + type RawCheckRun, + type RawIssueComment, + type RawReview, + type RawSearchIssue, + type RawStatus, + refKey, + type ReviewRequest, + reviewVerdict, + type Verdict, +} from "./forge.ts"; + +/** The search for the bot's open draft PRs on the forge. */ +export const FORGE_QUERY = `is:pr is:open draft:true org:${FORGE_ORG} author:${BOT_LOGIN}`; +/** Search pages to read at most (100 each). */ +const MAX_SEARCH_PAGES = 5; + +/** The bot's open draft PRs on the forge, oldest first. Search has no ETags, so poll it sparingly. */ +export async function loadForgePrs(gh: GitHub): Promise { + const out: ForgePr[] = []; + const q = encodeURIComponent(FORGE_QUERY); + for (let page = 1; page <= MAX_SEARCH_PAGES; page++) { + const r = await gh.get<{ items?: RawSearchIssue[]; total_count?: number }>( + `/search/issues?q=${q}&sort=created&order=asc&per_page=${PAGE_SIZE}&page=${page}`, + ); + const items = r.data.items ?? []; + for (const raw of items) { + const pr = parseSearchPr(raw); + if (pr) out.push(pr); + } + if (items.length < PAGE_SIZE || out.length >= (r.data.total_count ?? 0)) break; + } + return out; +} + +/** Run `fn` over `items`, at most `limit` at a time. */ +export async function mapLimit(items: readonly T[], limit: number, fn: (item: T) => Promise): Promise { + const out: R[] = new Array(items.length); + let next = 0; + const worker = async () => { + while (next < items.length) { + const i = next++; + out[i] = await fn(items[i] as T); + } + }; + await Promise.all(Array.from({ length: Math.min(limit, items.length) }, worker)); + return out; +} + +interface RawPull { + number: number; + html_url: string; + title?: string; + body?: string | null; + draft?: boolean; + state?: string; + merged_at?: string | null; + user?: { login?: string } | null; + created_at?: string; + updated_at?: string; + head: { sha: string; ref?: string; repo?: { full_name?: string } | null }; + base: { ref?: string; repo?: { full_name?: string; private?: boolean; parent?: { full_name?: string } } }; + additions?: number; + deletions?: number; + changed_files?: number; + commits?: number; +} + +export interface VerdictEntry { + /** The PR's updated_at when this was read; a newer one means re-read. */ + updatedAt: string; + head: string; + verdict: Verdict; +} + +/** + * Read the verdicts of PRs whose updated_at moved since `known` (a push + * or a review both move it). Heads come from one open-PR list per + * repository, conditionally; reviews from each changed PR. + */ +export async function refreshVerdicts( + gh: GitHub, + prs: readonly ForgePr[], + known: ReadonlyMap, +): Promise> { + const stale = prs.filter((p) => known.get(refKey(p.ref))?.updatedAt !== p.updatedAt); + const out = new Map(); + for (const p of prs) { + const k = known.get(refKey(p.ref)); + if (k && k.updatedAt === p.updatedAt) out.set(refKey(p.ref), k); + } + if (stale.length === 0) return out; + const repos = [...new Set(stale.map((p) => `${p.ref.owner}/${p.ref.repo}`))]; + const heads = new Map(); + await mapLimit(repos, FETCH_CONCURRENCY, async (repo) => { + const r = await gh.getAll(`/repos/${repo}/pulls?state=open&per_page=${PAGE_SIZE}`); + for (const pull of r.data) heads.set(`${repo}#${pull.number}`, pull.head.sha); + }); + await mapLimit(stale, FETCH_CONCURRENCY, async (p) => { + const key = refKey(p.ref); + const head = heads.get(key); + // Not open any more (the search lags): nothing to decide. + if (!head) { + out.set(key, { updatedAt: p.updatedAt, head: "", verdict: { state: "none" } }); + return; + } + const { reviews, comments } = await readDecisions(gh, p.ref); + out.set(key, { updatedAt: p.updatedAt, head, verdict: reviewVerdict(reviews, head, OPERATOR, comments) }); + }); + return out; +} + +/** His reviews and conversation comments on a PR: what bot-pr decides by. */ +async function readDecisions(gh: GitHub, ref: IssueRef): Promise<{ reviews: RawReview[]; comments: RawIssueComment[] }> { + const repo = `/repos/${ref.owner}/${ref.repo}`; + const [reviews, comments] = await Promise.all([ + gh.getAll(`${repo}/pulls/${ref.number}/reviews?per_page=${PAGE_SIZE}`), + gh.getAll(`${repo}/issues/${ref.number}/comments?per_page=${PAGE_SIZE}`), + ]); + return { reviews: reviews.data, comments: comments.data }; +} + +export interface Commit { + sha: string; + url: string; + message: string; + author: string; + date?: string; +} + +export interface FileDiff { + filename: string; + previous?: string; + status: string; + additions: number; + deletions: number; + /** Absent when GitHub omits it (binary, or too large). */ + patch?: string; + url?: string; +} + +export interface PrDetail { + ref: IssueRef; + url: string; + /** The PR's updated_at when read; the search's moving past it means stale. */ + updatedAt: string; + title: string; + body: string; + author: string; + state: string; + draft: boolean; + head: string; + headRef?: string; + baseRef?: string; + /** The fork's parent repository, `owner/repo`. */ + parent?: string; + isPrivate?: boolean; + additions: number; + deletions: number; + changedFiles: number; + /** The PR's total; `commits` holds at most 250 (the API's limit). */ + commitCount: number; + commits: Commit[]; + files: FileDiff[]; + checks: CiCheck[]; + verdict: Verdict; + /** + * False when the commit list doesn't end at the head: right after a + * push GitHub can serve the old commits and diff with the new head, and + * approving then would approve code he didn't see. + */ + consistent: boolean; + /** Problems reading optional parts, shown but not fatal. */ + warnings: string[]; +} + +interface RawCommit { + sha: string; + html_url: string; + commit: { message?: string; author?: { name?: string; date?: string } | null }; + author?: { login?: string } | null; +} + +interface RawFile { + filename: string; + previous_filename?: string; + status?: string; + additions?: number; + deletions?: number; + patch?: string; + blob_url?: string; +} + +/** Pages of files to read at most (the API stops at 3000 files). */ +export const MAX_FILE_PAGES = 30; +/** Commits the API lists for a PR at most. */ +const MAX_LISTED_COMMITS = 250; + +function pullPath(ref: IssueRef): string { + return `/repos/${ref.owner}/${ref.repo}/pulls/${ref.number}`; +} + +/** Everything the review pane shows about one PR. */ +export async function loadPrDetail(gh: GitHub, ref: IssueRef): Promise { + const base = pullPath(ref); + const pull = (await gh.get(base)).data; + const head = pull.head.sha; + const repo = `/repos/${ref.owner}/${ref.repo}`; + const warnings: string[] = []; + const optional = async (what: string, p: Promise, fallback: T): Promise => { + try { + return await p; + } catch (e) { + warnings.push(`Couldn't read ${what}: ${e instanceof Error ? e.message : String(e)}`); + return fallback; + } + }; + const [commits, files, runs, status, decisions, repoInfo] = await Promise.all([ + gh.getAll(`${base}/commits?per_page=${PAGE_SIZE}`, 3).then((r) => r.data), + gh.getAll(`${base}/files?per_page=${PAGE_SIZE}`, MAX_FILE_PAGES).then((r) => r.data), + optional("check runs", gh.get<{ check_runs?: RawCheckRun[] }>(`${repo}/commits/${head}/check-runs?per_page=${PAGE_SIZE}`).then((r) => r.data.check_runs ?? []), []), + optional("commit statuses", gh.get<{ statuses?: RawStatus[] }>(`${repo}/commits/${head}/status`).then((r) => r.data.statuses ?? []), []), + readDecisions(gh, ref), + optional("the repository", gh.get<{ private?: boolean; parent?: { full_name?: string } }>(repo).then((r) => r.data), {}), + ]); + const commitCount = pull.commits ?? commits.length; + // Past the API's listing limit the last commit listed isn't the head, + // so there is nothing to check; below it, a stale list (shorter or not) + // ends elsewhere. + const consistent = commitCount > MAX_LISTED_COMMITS || commits.at(-1)?.sha === head; + if (!consistent) warnings.push("GitHub is still updating this PR after a push: the commits and diff may not be the head's yet. Reload (r) before approving."); + const detail: PrDetail = { + ref, + url: pull.html_url, + updatedAt: pull.updated_at ?? "", + title: pull.title ?? "(no title)", + body: pull.body ?? "", + author: pull.user?.login ?? "ghost", + state: pull.merged_at ? "merged" : (pull.state ?? "unknown"), + draft: pull.draft === true, + head, + additions: pull.additions ?? 0, + deletions: pull.deletions ?? 0, + changedFiles: pull.changed_files ?? files.length, + commitCount, + commits: commits.map((c) => { + const out: Commit = { sha: c.sha, url: c.html_url, message: c.commit.message ?? "", author: c.author?.login ?? c.commit.author?.name ?? "unknown" }; + if (c.commit.author?.date) out.date = c.commit.author.date; + return out; + }), + files: files.map((f) => { + const out: FileDiff = { filename: f.filename, status: f.status ?? "modified", additions: f.additions ?? 0, deletions: f.deletions ?? 0 }; + if (f.previous_filename) out.previous = f.previous_filename; + if (f.patch !== undefined) out.patch = f.patch; + if (f.blob_url) out.url = f.blob_url; + return out; + }), + checks: ciChecks(runs, status), + verdict: reviewVerdict(decisions.reviews, head, OPERATOR, decisions.comments), + consistent, + warnings, + }; + if (pull.head.ref) detail.headRef = pull.head.ref; + if (pull.base.ref) detail.baseRef = pull.base.ref; + if (repoInfo.parent?.full_name) detail.parent = repoInfo.parent.full_name; + if (typeof repoInfo.private === "boolean") detail.isPrivate = repoInfo.private; + return detail; +} + +/** + * Submit a review of the head he was shown. The PR is re-read first, + * unconditionally, and a moved head refuses: an approval must name the + * commit he read (bot-pr's promote only honours it for the head), and + * a change request on stale code confuses the bot. + */ +export async function submitReview(gh: GitHub, ref: IssueRef, review: ReviewRequest): Promise { + const fresh = await gh.send("GET", pullPath(ref)); + if (fresh.state !== "open") throw new Error(`the PR is ${fresh.merged_at ? "merged" : (fresh.state ?? "not open")}; nothing was sent`); + if (fresh.head.sha !== review.commit_id) { + throw new Error( + `the PR's head moved to ${fresh.head.sha.slice(0, 12)} since you opened it; nothing was sent. Reload (r) to review the new commits.`, + ); + } + const r = await gh.send<{ html_url?: string }>("POST", `${pullPath(ref)}/reviews`, review); + return r.html_url ?? fresh.html_url; +} diff --git a/test/forge.test.ts b/test/forge.test.ts new file mode 100644 index 0000000..9c16672 --- /dev/null +++ b/test/forge.test.ts @@ -0,0 +1,216 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { + ciChecks, + hasPromoteLine, + ciSummary, + composeReview, + parseBotMeta, + parsePatch, + parseSearchPr, + type RawIssueComment, + type RawReview, + reviewVerdict, + waitsOnReviewer, + withoutBotMeta, +} from "../src/github/forge.ts"; + +const HEAD = "a".repeat(40); +const OLD = "b".repeat(40); + +describe("parseSearchPr", () => { + it("parses a PR result", () => { + const pr = parseSearchPr({ + html_url: "https://github.com/cgwalters-forge/bootc/pull/30", + title: " tests: Cover it ", + body: null, + user: { login: "cgwalters-bot" }, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-02T00:00:00Z", + draft: true, + pull_request: {}, + }); + assert.deepEqual(pr, { + ref: { owner: "cgwalters-forge", repo: "bootc", number: 30 }, + url: "https://github.com/cgwalters-forge/bootc/pull/30", + title: "tests: Cover it", + body: "", + author: "cgwalters-bot", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + draft: true, + }); + }); + const rejects: [string, Parameters[0]][] = [ + ["an issue", { html_url: "https://github.com/o/r/issues/1" }], + ["no pull_request", { html_url: "https://github.com/o/r/pull/1" }], + ["another host", { html_url: "https://evil.example/o/r/pull/1", pull_request: {} }], + ]; + for (const [name, raw] of rejects) it(`skips ${name}`, () => assert.equal(parseSearchPr(raw), undefined)); +}); + +describe("parseBotMeta and withoutBotMeta", () => { + const body = [ + "The change.", + "", + "Generated-by: x", + "", + "", + "---", + "- Upstream: `bootc-dev/bootc`, base `main`", + "- Board item: `PVTI_lAHOAQ_SPs4Bj2Gizg8vse8`", + "", + ].join("\r\n"); + it("reads the upstream, base and item", () => { + assert.deepEqual(parseBotMeta(body), { upstream: "bootc-dev/bootc", base: "main", item: "PVTI_lAHOAQ_SPs4Bj2Gizg8vse8" }); + }); + it("ignores the same lines outside the section", () => { + assert.deepEqual(parseBotMeta("- Upstream: `evil/x`, base `main`\n- Board item: `PVTI_x`"), {}); + }); + it("strips the section", () => { + assert.equal(withoutBotMeta(body), "The change.\r\n\r\nGenerated-by: x"); + assert.equal(withoutBotMeta("no meta\n"), "no meta\n"); + }); +}); + +describe("reviewVerdict", () => { + const r = (login: string, state: string, commit: string, at: string): RawReview => ({ + user: { login }, + state, + commit_id: commit, + submitted_at: at, + html_url: `https://github.com/r/${at}`, + }); + const cases: [string, RawReview[], string, boolean][] = [ + ["no reviews", [], "none", true], + ["approved the head", [r("cgwalters", "APPROVED", HEAD, "2026-01-02")], "approved", false], + ["approved an older head", [r("cgwalters", "APPROVED", OLD, "2026-01-02")], "approved-older", true], + ["changes on the head", [r("cgwalters", "CHANGES_REQUESTED", HEAD, "2026-01-02")], "changes-requested", false], + ["changes, then a push", [r("cgwalters", "CHANGES_REQUESTED", OLD, "2026-01-02")], "changes-requested-older", true], + ["someone else's approval", [r("someone", "APPROVED", HEAD, "2026-01-02")], "none", true], + ["comments don't decide", [r("cgwalters", "APPROVED", HEAD, "2026-01-01"), r("cgwalters", "COMMENTED", HEAD, "2026-01-03")], "approved", false], + ["the latest decides", [r("cgwalters", "APPROVED", HEAD, "2026-01-03"), r("cgwalters", "CHANGES_REQUESTED", HEAD, "2026-01-02")], "approved", false], + ["dismissed", [r("cgwalters", "APPROVED", HEAD, "2026-01-01"), r("cgwalters", "DISMISSED", HEAD, "2026-01-02")], "none", true], + ]; + for (const [name, reviews, state, waits] of cases) { + it(name, () => { + const v = reviewVerdict(reviews, HEAD, "cgwalters"); + assert.equal(v.state, state); + assert.equal(waitsOnReviewer(v), waits); + }); + } + + it("ignores undated reviews and keeps API order on ties", () => { + const undated = { ...r("cgwalters", "APPROVED", HEAD, "x"), submitted_at: null }; + assert.equal(reviewVerdict([undated], HEAD, "cgwalters").state, "none"); + const tie = [r("cgwalters", "APPROVED", HEAD, "2026-01-02T00:00:00Z"), r("cgwalters", "CHANGES_REQUESTED", HEAD, "2026-01-02T00:00:00Z")]; + assert.equal(reviewVerdict(tie, HEAD, "cgwalters").state, "changes-requested"); + }); + + describe("with /promote comments, as bot-pr counts them", () => { + const c = (login: string, body: string, at: string): RawIssueComment => ({ user: { login }, body, created_at: at, html_url: `https://github.com/c/${at}` }); + const cases: [string, RawReview[], RawIssueComment[], string, boolean][] = [ + ["a /promote comment", [], [c("cgwalters", "looks good\n /promote\t", "2026-01-02T00:00:00Z")], "promoted", true], + ["/promote --human-text", [], [c("cgwalters", "/promote --human-text", "2026-01-02T00:00:00Z")], "promoted", true], + ["a later approval wins", [r("cgwalters", "APPROVED", HEAD, "2026-01-03T00:00:00Z")], [c("cgwalters", "/promote", "2026-01-02T00:00:00Z")], "approved", false], + ["a later /promote wins", [r("cgwalters", "APPROVED", HEAD, "2026-01-01T00:00:00Z")], [c("cgwalters", "/promote", "2026-01-02T00:00:00Z")], "promoted", true], + ["someone else's /promote", [], [c("someone", "/promote", "2026-01-02T00:00:00Z")], "none", true], + ["/promote in prose", [], [c("cgwalters", "I'll /promote it later", "2026-01-02T00:00:00Z")], "none", true], + ]; + for (const [name, reviews, comments, state, waits] of cases) { + it(name, () => { + const v = reviewVerdict(reviews, HEAD, "cgwalters", comments); + assert.equal(v.state, state); + assert.equal(waitsOnReviewer(v), waits); + }); + } + }); +}); + +describe("hasPromoteLine", () => { + const cases: [string, boolean][] = [ + ["/promote", true], + ["ok\r\n /promote \r\n", true], + ["/promote --human-text", true], + ["/promote now", false], + ["`/promote`", false], + ["\u00a0/promote", false], + ]; + for (const [body, want] of cases) it(JSON.stringify(body), () => assert.equal(hasPromoteLine(body), want)); +}); + +describe("parsePatch", () => { + it("numbers lines per side", () => { + const lines = parsePatch("@@ -10,3 +10,4 @@ fn x() {\n a\n-b\n+c\n+d\n e\n\\ No newline at end of file\n"); + assert.deepEqual( + lines.map((l) => [l.kind, l.old ?? null, l.new ?? null, l.text]), + [ + ["hunk", null, null, "@@ -10,3 +10,4 @@ fn x() {"], + ["ctx", 10, 10, "a"], + ["del", 11, null, "b"], + ["add", null, 11, "c"], + ["add", null, 12, "d"], + ["ctx", 12, 13, "e"], + ["note", null, null, "\\ No newline at end of file"], + ], + ); + }); + it("handles a new file and an empty patch", () => { + assert.deepEqual(parsePatch("@@ -0,0 +1 @@\n+only").map((l) => [l.kind, l.new ?? null]), [["hunk", null], ["add", 1]]); + assert.deepEqual(parsePatch(""), []); + }); +}); + +describe("ciChecks and ciSummary", () => { + it("merges runs and statuses, failures first", () => { + const checks = ciChecks( + [ + { name: "build", status: "completed", conclusion: "success", html_url: "https://x/1" }, + { name: "lint", status: "completed", conclusion: "skipped" }, + { name: "tests", status: "in_progress", conclusion: null }, + { name: "vm", status: "completed", conclusion: "timed_out" }, + ], + [{ context: "DCO", state: "error", target_url: null }], + ); + assert.deepEqual(checks.map((c) => [c.name, c.state, c.detail]), [ + ["DCO", "failure", "error"], + ["vm", "failure", "timed_out"], + ["tests", "pending", "in_progress"], + ["build", "success", "success"], + ["lint", "success", "skipped"], + ]); + assert.equal(checks.find((c) => c.name === "build")?.url, "https://x/1"); + assert.equal(ciSummary(checks), "failure"); + }); + const summaries: [string[], string][] = [[[], "none"], [["success"], "success"], [["success", "pending"], "pending"]]; + for (const [states, want] of summaries) { + it(`summary of ${states.join(",") || "nothing"}`, () => { + const runs = states.map((s) => ({ name: s, status: s === "pending" ? "queued" : "completed", conclusion: s })); + assert.equal(ciSummary(ciChecks(runs, [])), want); + }); + } +}); + +describe("composeReview", () => { + it("approves the given head, with or without text and /draft", () => { + assert.deepEqual(composeReview("approve", " ", HEAD), { commit_id: HEAD, event: "APPROVE", body: "" }); + assert.deepEqual(composeReview("approve", "LGTM\r\n", HEAD, { draft: true }), { commit_id: HEAD, event: "APPROVE", body: "LGTM\n\n/draft" }); + assert.deepEqual(composeReview("approve", "", HEAD, { draft: true }).body, "/draft"); + }); + it("requests changes and comments with text", () => { + assert.deepEqual(composeReview("request-changes", "Split this commit", HEAD), { commit_id: HEAD, event: "REQUEST_CHANGES", body: "Split this commit" }); + assert.equal(composeReview("comment", "a `/promote` in backticks is fine", HEAD).event, "COMMENT"); + }); + const refusals: [string, () => unknown, RegExp][] = [ + ["empty change request", () => composeReview("request-changes", " ", HEAD), /say what to change/], + ["empty comment", () => composeReview("comment", "", HEAD), /write a comment/], + ["a /promote line", () => composeReview("comment", "ok\n /promote", HEAD), /bot command/], + ["a /draft line in a change request", () => composeReview("request-changes", "fix\n/draft", HEAD), /bot command/], + ["a typed /ready in an approval", () => composeReview("approve", "/ready", HEAD), /bot command/], + ["/promote --human-text", () => composeReview("comment", "/promote --human-text", HEAD), /bot command/], + ["an /answer line", () => composeReview("comment", "/answer A", HEAD), /bot command/], + ["/draft without approving", () => composeReview("comment", "x", HEAD, { draft: true }), /only with an approval/], + ["a short sha", () => composeReview("approve", "", "abc123"), /not a commit id/], + ]; + for (const [name, f, re] of refusals) it(`refuses ${name}`, () => assert.throws(f, re)); +}); diff --git a/test/prs.test.ts b/test/prs.test.ts new file mode 100644 index 0000000..abf629c --- /dev/null +++ b/test/prs.test.ts @@ -0,0 +1,222 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { GitHub } from "../src/github/api.ts"; +import { composeReview, type ForgePr } from "../src/github/forge.ts"; +import { FORGE_QUERY, loadForgePrs, loadPrDetail, mapLimit, refreshVerdicts, submitReview, type VerdictEntry } from "../src/github/prs.ts"; +import { scriptedFetch } from "./helpers.ts"; + +const token = async () => "t"; +const API = "https://api.github.com"; +const HEAD = "c".repeat(40); +const MOVED = "d".repeat(40); +const ref = { owner: "cgwalters-forge", repo: "widget", number: 7 }; +const PULL = `${API}/repos/cgwalters-forge/widget/pulls/7`; + +function pull(over: Record = {}) { + return { + number: 7, + html_url: "https://github.com/cgwalters-forge/widget/pull/7", + title: "widget: Fix it", + body: "Body", + draft: true, + state: "open", + user: { login: "cgwalters-bot" }, + head: { sha: HEAD, ref: "bot/fix" }, + base: { ref: "main" }, + additions: 3, + deletions: 1, + changed_files: 1, + commits: 1, + ...over, + }; +} + +describe("loadForgePrs", () => { + it("searches the bot's open forge drafts, page by page", async () => { + const item = (n: number) => ({ html_url: `https://github.com/cgwalters-forge/r/pull/${n}`, pull_request: {}, title: `#${n}` }); + const { fetchImpl, calls } = scriptedFetch((_m, url) => { + const page = Number(new URL(url).searchParams.get("page")); + if (page === 1) return { body: { total_count: 101, items: Array.from({ length: 100 }, (_, i) => item(i + 1)) } }; + if (page === 2) return { body: { total_count: 101, items: [item(101), { html_url: "https://github.com/x/y/issues/1" }] } }; + return undefined; + }); + const prs = await loadForgePrs(new GitHub(token, fetchImpl)); + assert.equal(prs.length, 101); + assert.equal(calls.length, 2); + assert.equal(new URL(calls[0]?.url ?? "").searchParams.get("q"), FORGE_QUERY); + assert.match(FORGE_QUERY, /is:open draft:true org:cgwalters-forge author:cgwalters-bot/); + }); +}); + +describe("mapLimit", () => { + it("keeps order and bounds concurrency", async () => { + let running = 0; + let peak = 0; + const out = await mapLimit([5, 1, 4, 2, 3], 2, async (n) => { + running++; + peak = Math.max(peak, running); + await new Promise((r) => setTimeout(r, n)); + running--; + return n * 10; + }); + assert.deepEqual(out, [50, 10, 40, 20, 30]); + assert.equal(peak, 2); + }); +}); + +describe("refreshVerdicts", () => { + const forgePr = (n: number, updatedAt: string): ForgePr => ({ + ref: { owner: "cgwalters-forge", repo: "widget", number: n }, + url: `https://github.com/cgwalters-forge/widget/pull/${n}`, + title: "", + body: "", + author: "cgwalters-bot", + createdAt: "", + updatedAt, + draft: true, + }); + + it("re-reads only PRs that changed, against their current heads", async () => { + const { fetchImpl, calls } = scriptedFetch((_m, url) => { + if (url.startsWith(`${API}/repos/cgwalters-forge/widget/pulls?`)) { + return { body: [pull({ number: 1, head: { sha: HEAD } }), pull({ number: 2, head: { sha: MOVED } })] }; + } + if (url.startsWith(`${API}/repos/cgwalters-forge/widget/pulls/2/reviews`)) { + return { body: [{ user: { login: "cgwalters" }, state: "APPROVED", commit_id: HEAD, submitted_at: "2026-01-01T00:00:00Z" }] }; + } + if (url.startsWith(`${API}/repos/cgwalters-forge/widget/issues/2/comments`)) return { body: [] }; + return undefined; + }); + const known = new Map([ + ["cgwalters-forge/widget#1", { updatedAt: "u1", head: HEAD, verdict: { state: "approved" } }], + ["cgwalters-forge/widget#3", { updatedAt: "gone", head: HEAD, verdict: { state: "none" } }], + ]); + const out = await refreshVerdicts(new GitHub(token, fetchImpl), [forgePr(1, "u1"), forgePr(2, "u2")], known); + assert.deepEqual([...out.keys()].sort(), ["cgwalters-forge/widget#1", "cgwalters-forge/widget#2"]); + assert.equal(out.get("cgwalters-forge/widget#1")?.verdict.state, "approved"); + assert.equal(out.get("cgwalters-forge/widget#2")?.verdict.state, "approved-older"); + assert.equal(out.get("cgwalters-forge/widget#2")?.head, MOVED); + assert.equal(calls.filter((c) => c.url.includes("/reviews")).length, 1); + }); + + it("marks a PR the open list no longer has as undecided", async () => { + const { fetchImpl } = scriptedFetch((_m, url) => (url.includes("/pulls?") ? { body: [] } : undefined)); + const out = await refreshVerdicts(new GitHub(token, fetchImpl), [forgePr(5, "u")], new Map()); + assert.deepEqual(out.get("cgwalters-forge/widget#5")?.verdict, { state: "none" }); + }); + + it("counts a /promote comment", async () => { + const { fetchImpl } = scriptedFetch((_m, url) => { + if (url.includes("/pulls?")) return { body: [pull({ number: 6 })] }; + if (url.includes("/pulls/6/reviews")) return { body: [] }; + if (url.includes("/issues/6/comments")) return { body: [{ user: { login: "cgwalters" }, body: "/promote", created_at: "2026-01-01T00:00:00Z" }] }; + return undefined; + }); + const out = await refreshVerdicts(new GitHub(token, fetchImpl), [forgePr(6, "u")], new Map()); + assert.equal(out.get("cgwalters-forge/widget#6")?.verdict.state, "promoted"); + }); + + it("makes no requests when nothing changed", async () => { + const { fetchImpl, calls } = scriptedFetch(() => undefined); + const known = new Map([["cgwalters-forge/widget#1", { updatedAt: "u1", head: HEAD, verdict: { state: "none" } }]]); + const out = await refreshVerdicts(new GitHub(token, fetchImpl), [forgePr(1, "u1")], known); + assert.equal(out.size, 1); + assert.equal(calls.length, 0); + }); +}); + +describe("loadPrDetail", () => { + const route = (commitSha: string, commitsTotal = 1) => (method: string, url: string, headers: Record) => { + if (url === PULL) return method === "GET" && headers["If-None-Match"] ? { status: 304 } : { body: pull({ commits: commitsTotal }), headers: { etag: '"p"' } }; + if (url.startsWith(`${PULL}/commits`)) { + return { body: [{ sha: commitSha, html_url: "https://github.com/c", commit: { message: "widget: Fix it\n\nBecause.", author: { name: "Bot", date: "2026-01-01T00:00:00Z" } }, author: { login: "cgwalters-bot" } }] }; + } + if (url.startsWith(`${PULL}/files`)) return { body: [] }; + if (url.startsWith(`${PULL}/reviews`) || url.includes("/issues/7/comments")) return { body: [] }; + if (url.includes("/check-runs")) return { body: { check_runs: [] } }; + if (url.endsWith("/status")) return { body: { statuses: [] } }; + if (url === `${API}/repos/cgwalters-forge/widget`) return { body: {} }; + if (method === "POST") return { body: { html_url: "https://github.com/review/2" } }; + return undefined; + }; + + it("flags a commit list that doesn't end at the head", async () => { + const cases: [string, number, boolean][] = [[HEAD, 1, true], [MOVED, 1, false], [MOVED, 2, false], [MOVED, 300, true]]; + for (const [sha, total, want] of cases) { + const d = await loadPrDetail(new GitHub(token, scriptedFetch(route(sha, total)).fetchImpl), ref); + assert.equal(d.consistent, want, `${sha.slice(0, 4)} of ${total}`); + assert.equal(d.warnings.length, want ? 0 : 1); + } + }); + + it("re-reads the PR unconditionally before reviewing, even when cached", async () => { + const { fetchImpl, calls } = scriptedFetch(route(HEAD)); + const gh = new GitHub(token, fetchImpl); + await loadPrDetail(gh, ref); + await submitReview(gh, ref, composeReview("comment", "hi", HEAD)); + const gets = calls.filter((c) => c.url === PULL && c.method === "GET"); + assert.equal(gets.length, 2); + assert.equal(gets[1]?.headers["If-None-Match"], undefined); + }); + + it("gathers the PR, commits, files, checks and verdict; optional parts only warn", async () => { + const { fetchImpl } = scriptedFetch((_m, url) => { + if (url === PULL) return { body: pull() }; + if (url.startsWith(`${PULL}/commits`)) { + return { body: [{ sha: HEAD, html_url: "https://github.com/c", commit: { message: "widget: Fix it\n\nBecause.", author: { name: "Bot", date: "2026-01-01T00:00:00Z" } }, author: { login: "cgwalters-bot" } }] }; + } + if (url.includes("/issues/7/comments")) return { body: [] }; + if (url.startsWith(`${PULL}/files`)) return { body: [{ filename: "src/a.rs", status: "modified", additions: 3, deletions: 1, patch: "@@ -1 +1 @@\n-a\n+b" }] }; + if (url.startsWith(`${PULL}/reviews`)) return { body: [] }; + if (url.includes(`/commits/${HEAD}/check-runs`)) return { status: 403, body: { message: "Resource not accessible" } }; + if (url.endsWith(`/commits/${HEAD}/status`)) return { body: { statuses: [{ context: "DCO", state: "success" }] } }; + if (url === `${API}/repos/cgwalters-forge/widget`) return { body: { private: false, parent: { full_name: "up/widget" } } }; + return undefined; + }); + const d = await loadPrDetail(new GitHub(token, fetchImpl), ref); + assert.equal(d.head, HEAD); + assert.equal(d.parent, "up/widget"); + assert.equal(d.baseRef, "main"); + assert.equal(d.commits[0]?.message, "widget: Fix it\n\nBecause."); + assert.equal(d.files[0]?.patch, "@@ -1 +1 @@\n-a\n+b"); + assert.deepEqual(d.checks.map((c) => c.name), ["DCO"]); + assert.equal(d.verdict.state, "none"); + assert.equal(d.warnings.length, 1); + assert.match(d.warnings[0] ?? "", /check runs.*403/); + }); +}); + +describe("submitReview", () => { + const run = async (fresh: Record) => { + const { fetchImpl, calls } = scriptedFetch((method, url) => { + if (method === "GET" && url === PULL) return { body: pull(fresh) }; + if (method === "POST" && url === `${PULL}/reviews`) return { body: { html_url: "https://github.com/review/1" } }; + return undefined; + }); + const review = composeReview("approve", "", HEAD, { draft: true }); + const result = await submitReview(new GitHub(token, fetchImpl), ref, review).then( + (url) => ({ url }), + (e: Error) => ({ error: e.message }), + ); + return { result, posts: calls.filter((c) => c.method === "POST") }; + }; + + it("approves the head he saw", async () => { + const { result, posts } = await run({}); + assert.deepEqual(result, { url: "https://github.com/review/1" }); + assert.deepEqual(posts[0]?.body, { commit_id: HEAD, event: "APPROVE", body: "/draft" }); + }); + + const refusals: [string, Record, RegExp][] = [ + ["a moved head", { head: { sha: MOVED } }, /head moved to dddddddddddd.*nothing was sent/], + ["a closed PR", { state: "closed" }, /PR is closed/], + ["a merged PR", { state: "closed", merged_at: "2026-01-01T00:00:00Z" }, /PR is merged/], + ]; + for (const [name, fresh, re] of refusals) { + it(`sends nothing on ${name}`, async () => { + const { result, posts } = await run(fresh); + assert.match("error" in result ? result.error : "", re); + assert.equal(posts.length, 0); + }); + } +}); From b2a4ad43a7872995591637c284cd0ed9a07cac0d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 26 Sep 2026 08:01:08 -0400 Subject: [PATCH 5/6] github: Rank forge PRs, questions and chores in one queue, with a review pane cgwalters wants one "things you need me for" inbox that beats GitHub's own UI, so the queue now lists the bot's forge draft PRs waiting on his review next to the board's Needs human and Draft items, ranked P0 first and then oldest first. A PR takes its board item's priority, and that Draft item isn't listed a second time; a Draft item whose forge PRs are all gone (promoted or closed) is dropped as stale, once the forge has been read. A PR opens a review pane: the description without bot-pr's meta section, CI, each commit's full message, and a per-file diff that folds (large files start folded and are rendered only when opened). Approve, request changes and comment submit reviews of the head shown. Approve is disabled while GitHub's commit list doesn't end at the head yet, and its confirmation names files he never expanded. The form appears only for the bot's own PRs in cgwalters-forge and cgwalters-bot, so a crafted link can't make the page a one-click approval elsewhere. The queue and the pane share one view model because the queue's order and labels depend on the verdicts the pane changes. It is keyboard driven (j/k, o, u, r; a to approve after a confirmation, x to fold a file, ? for the list), and usable at phone width. The board shows first; forge PRs and their verdicts fill in as they arrive. Generated-by: AI --- README.md | 38 ++++- src/github/board.ts | 23 --- src/github/config.ts | 7 + src/github/keys.ts | 64 +++++++ src/github/main.ts | 399 ++++++++++++++++++++++++++++++++++++++++--- src/github/prview.ts | 320 ++++++++++++++++++++++++++++++++++ src/github/queue.ts | 141 +++++++++++++++ src/github/view.ts | 94 +++++++--- static/index.html | 2 +- static/style.css | 72 +++++++- test/board.test.ts | 16 +- test/keys.test.ts | 45 +++++ test/prview.test.ts | 172 +++++++++++++++++++ test/queue.test.ts | 123 +++++++++++++ test/view.test.ts | 44 ++++- 15 files changed, 1447 insertions(+), 113 deletions(-) create mode 100644 src/github/keys.ts create mode 100644 src/github/prview.ts create mode 100644 src/github/queue.ts create mode 100644 test/keys.test.ts create mode 100644 test/prview.test.ts create mode 100644 test/queue.test.ts diff --git a/README.md b/README.md index 3551dbf..381117d 100644 --- a/README.md +++ b/README.md @@ -20,14 +20,36 @@ claude.ai-hosted prototype, and why it is being replaced, is described in ## What v0 does -It lists the Workstream board's "Needs human" items by priority. An item -shows its Why, links, description, gist and latest comments, rendered -from markdown and sanitized. You answer with a tap on one of the options -the bot offered (parsed from Why), free text, or both. On an issue or PR -the answer is a comment by you starting with `/answer` (or `/answer B`); -on a draft item it is a receipt gist plus a marked section in the draft -body, both readable by anyone while the board is public. The queue is polled every 30 seconds with ETags while the tab -is visible. +One ranked queue of everything waiting on you: the bot's open draft PRs +in cgwalters-forge that you haven't approved or sent back at their +current head, and the Workstream board's "Needs human" items (questions, +and other actions) and Draft items (gists to read). P0 comes first +(the board's Priority; a PR takes its board item's), then the oldest. + +- **A forge PR** opens a review pane: the description (without bot-pr's + meta section), CI checks, every commit with its full message, and the + diff per file, foldable. **Approve** submits an approving review of the + head you were shown, which is what `bot-pr promote` acts on; if the + head moved meanwhile, nothing is sent. A checkbox adds the `/draft` + line that asks promote for a draft upstream PR. **Request changes** and + **Comment** submit reviews with your text. +- **A board item** shows its Why, links, description, gist and latest + comments, rendered from markdown and sanitized. You answer with a tap + on one of the options the bot offered (parsed from Why), free text, or + both. On an issue or PR the answer is a comment by you starting with + `/answer` (or `/answer B`); on a draft item it is a receipt gist plus a + marked section in the draft body, both readable by anyone while the + board is public. + +Keys: `j`/`k` move, `o` opens, `u` goes back, `r` reloads; in a PR, +`j`/`k` step through files, `x` folds one, `a` approves (after a +confirmation) and `c` jumps to the review text; `?` lists them. Your +text never goes out with a line the bot would read as a command +(`/promote`, `/draft`, `/ready`, `/answer`). + +The board is polled every 30 seconds with ETags while the tab is +visible, the forge's PR search every minute, and a PR's reviews only +when it changed. The app contains no data: everything is fetched in your browser with your token, from `api.github.com` only. diff --git a/src/github/board.ts b/src/github/board.ts index 8049b1e..70ae836 100644 --- a/src/github/board.ts +++ b/src/github/board.ts @@ -183,29 +183,6 @@ export function queueItems(raw: readonly RawItem[]): Item[] { .filter((i) => i.status !== undefined && QUEUE_STATUSES.includes(i.status)); } -export interface PriorityGroup { - priority: string; - items: Item[]; -} - -/** Group items by priority in PRIORITY_ORDER, keeping board order within one. */ -export function groupByPriority(items: readonly Item[]): PriorityGroup[] { - const groups = new Map(); - for (const item of items) { - const key = item.priority ?? NO_PRIORITY; - const list = groups.get(key) ?? []; - list.push(item); - groups.set(key, list); - } - const rank = (p: string) => { - const i = PRIORITY_ORDER.indexOf(p); - return i < 0 ? (p === NO_PRIORITY ? PRIORITY_ORDER.length + 1 : PRIORITY_ORDER.length) : i; - }; - return [...groups.entries()] - .sort(([a], [b]) => rank(a) - rank(b) || a.localeCompare(b)) - .map(([priority, list]) => ({ priority, items: list })); -} - /** The question an item asks, as the bot wrote it. */ export interface Question { options: Option[]; diff --git a/src/github/config.ts b/src/github/config.ts index 765a4f7..c3c34c9 100644 --- a/src/github/config.ts +++ b/src/github/config.ts @@ -42,8 +42,14 @@ export const HOME_OWNERS: readonly string[] = ["cgwalters-forge", "cgwalters-bot /** Poll every this many ms while the tab is visible. */ export const POLL_INTERVAL_MS = 30_000; +/** Poll the forge's PR search this often: search has its own, smaller budget. */ +export const FORGE_POLL_INTERVAL_MS = 60_000; +/** ... and at most this often when asked to refresh (r, or after a review). */ +export const FORGE_MIN_INTERVAL_MS = 10_000; /** Parallel requests when refreshing PR verdicts. */ export const FETCH_CONCURRENCY = 6; +/** A file's diff starts collapsed above this many lines. */ +export const DIFF_COLLAPSE_LINES = 300; /** Poll this many times slower when the rate budget runs low. */ export const POLL_BACKOFF_FACTOR = 4; /** Below this fraction of the hourly budget, back off. */ @@ -54,6 +60,7 @@ export const PAGE_SIZE = 100; /** Comments shown in the item view. */ export const RECENT_COMMENTS = 5; + /** Storage key for the pasted token (sessionStorage, or localStorage if remembered). */ export const TOKEN_KEY = "review.token"; diff --git a/src/github/keys.ts b/src/github/keys.ts new file mode 100644 index 0000000..fecc80c --- /dev/null +++ b/src/github/keys.ts @@ -0,0 +1,64 @@ +// Keyboard commands, as a pure mapping from a key press to a command so +// tests can check it; main.ts carries them out. + +export type Route = "queue" | "item" | "pr"; + +export type Command = + | "next" + | "prev" + | "open" + | "back" + | "refresh" + | "approve" + | "fold" + | "compose" + | "help"; + +export interface KeyPress { + key: string; + ctrlKey: boolean; + metaKey: boolean; + altKey: boolean; + /** The focused element's tag, lower case, and whether it is editable. */ + editing: boolean; +} + +const COMMON: Record = { r: "refresh", "?": "help" }; + +const BY_ROUTE: Record> = { + queue: { j: "next", k: "prev", ArrowDown: "next", ArrowUp: "prev", o: "open", Enter: "open" }, + item: { u: "back", Escape: "back", c: "compose" }, + pr: { j: "next", k: "prev", x: "fold", a: "approve", c: "compose", u: "back", Escape: "back" }, +}; + +/** + * The command for a key press, if any. Keys typed into a text field + * belong to it, except Escape, which leaves the field; modified keys + * belong to the browser. + */ +export function keyCommand(press: KeyPress, route: Route): Command | "blur" | undefined { + if (press.ctrlKey || press.metaKey || press.altKey) return undefined; + if (press.editing) return press.key === "Escape" ? "blur" : undefined; + return BY_ROUTE[route][press.key] ?? COMMON[press.key]; +} + +export const HELP: Record = { + queue: "j/k or ↓/↑ move · o or Enter open · r refresh · ? keys", + item: "u or Esc back to the queue · c write an answer · r refresh", + pr: "j/k next/previous file · x fold file · a approve · c write a review · u or Esc back · r reload", +}; + +export type RouteInfo = + | { route: "queue" } + | { route: "item"; id: string } + | { route: "pr"; ref: { owner: string; repo: string; number: number } }; + +/** The view a location hash asks for; anything unknown is the queue. */ +export function parseRoute(hash: string): RouteInfo { + const item = /^#item\/(PVTI_[A-Za-z0-9_-]+)$/.exec(hash); + if (item?.[1]) return { route: "item", id: item[1] }; + const pr = /^#pr\/([A-Za-z0-9-]+)\/([A-Za-z0-9._-]+)\/([1-9][0-9]{0,9})$/.exec(hash); + // "." and ".." are no repository's name, and would climb the API path. + if (pr?.[1] && pr[2] && pr[3] && !/^\.+$/.test(pr[2])) return { route: "pr", ref: { owner: pr[1], repo: pr[2], number: Number(pr[3]) } }; + return { route: "queue" }; +} diff --git a/src/github/main.ts b/src/github/main.ts index a28220b..be059c3 100644 --- a/src/github/main.ts +++ b/src/github/main.ts @@ -1,4 +1,5 @@ -// Entry point: sign-in, routing between queue and item, and polling. +// Entry point: sign-in, routing between the queue, board items and forge +// PRs, keyboard commands, and polling while the tab is visible. import { h } from "../dom.ts"; import { createRenderer } from "../markdown.ts"; @@ -8,13 +9,20 @@ import { answerTarget, type Item, questionOf } from "./board.ts"; import { type Context, loadContext, loadQueue, postAnswer, type ReceiptStatus, verifyReceipt, viewer } from "./backend.ts"; import { CLASSIC_SCOPES, + FORGE_MIN_INTERVAL_MS, + FORGE_POLL_INTERVAL_MS, HOME_OWNERS, OPERATOR, POLL_BACKOFF_FACTOR, POLL_INTERVAL_MS, RATE_LOW_FRACTION, } from "./config.ts"; -import { CONTEXT_CLASS, contextView, itemView, queueView } from "./view.ts"; +import { composeReview, type ForgePr, refKey, VERDICT_LABEL } from "./forge.ts"; +import { type Command, HELP, keyCommand, parseRoute, type Route, type RouteInfo } from "./keys.ts"; +import { loadForgePrs, loadPrDetail, type PrDetail, refreshVerdicts, submitReview, type VerdictEntry } from "./prs.ts"; +import { APPROVE_ACTION, FILE_CLASS, prView, REVIEW_FORM_CLASS } from "./prview.ts"; +import { buildEntries, type Entry } from "./queue.ts"; +import { answerState, CONTEXT_CLASS, contextView, itemView, queueView, ROW_CLASS, ROW_KEY_ATTR, type RowLabel, STATE_LABEL } from "./view.ts"; const render = createRenderer(window); @@ -25,18 +33,45 @@ interface State { items: Item[]; boardPublic: boolean; loaded: boolean; + /** The bot's open draft PRs on the forge, from the last search. */ + prs: ForgePr[]; + /** Their verdicts and heads, by refKey. */ + verdicts: Map; + lastForgePoll: number; + /** Search the forge on the next poll if the minimum interval allows. */ + forceForge: boolean; + /** Whether a forge search has succeeded, so missing forge PRs mean gone. */ + forgeKnown: boolean; + verdictsRunning: boolean; + polling: boolean; + /** The ranked queue. */ + entries: Entry[]; + /** The queue row the keyboard is on. */ + selected: string | undefined; /** Items answered from this tab, until the bot moves them on. */ sent: Set; + /** PRs reviewed from this tab, by refKey. */ + reviewed: Set; /** Context of opened items, by node id. */ context: Map; + /** Loaded PR details, by refKey. */ + details: Map; + /** The search's updated_at a PR was last reloaded for, by refKey. */ + reloadedFor: Map; /** Checked receipts of drafts that claim an answer, by node id. */ receipts: Map; /** The open item as it was rendered, to notice changes under it. */ shown?: { nodeId: string; key: string }; - /** A note about the open item, e.g. that it changed on the board. */ + /** The open PR as it was rendered: its key and updated_at. */ + shownPr?: { key: string; updatedAt: string }; + /** A note about the open item or PR, e.g. that it changed. */ itemNote?: string; + /** The file the keyboard is on in a PR. */ + fileIndex: number; + help: boolean; lastPoll?: Date; error?: string; + forgeError?: string; /** What the token lacks, e.g. classic scopes. */ tokenWarning?: string; timer?: ReturnType; @@ -58,8 +93,11 @@ function setNotice(text: string | undefined): void { n.hidden = !text; } +const route = (): RouteInfo => parseRoute(window.location.hash); + function routeItemId(): string | undefined { - return /^#item\/(PVTI_[A-Za-z0-9_-]+)$/.exec(window.location.hash)?.[1]; + const r = route(); + return r.route === "item" ? r.id : undefined; } /** What the open item view shows; a change means the view is stale. */ @@ -67,6 +105,10 @@ function itemKey(item: Item): string { return JSON.stringify([item.title, item.why, item.body, item.priority, item.status]); } +function message(e: unknown): string { + return e instanceof Error ? e.message : String(e); +} + function renderMeta(state: State): void { const parts = [state.login ? `signed in as ${state.login}` : ""]; if (state.lastPoll) parts.push(`checked ${state.lastPoll.toLocaleTimeString([], { hour: "2-digit", minute: "2-digit" })}`); @@ -77,28 +119,74 @@ function renderMeta(state: State): void { /** Meta line and notices only: never touches the view, or a half-typed answer. */ function renderChrome(state: State): void { renderMeta(state); - const warnings = [state.error, state.tokenWarning, routeItemId() ? state.itemNote : undefined]; + const r = route().route; + const warnings = [state.error, state.forgeError, state.tokenWarning, r !== "queue" ? state.itemNote : undefined]; if (state.login && state.login !== OPERATOR) { - warnings.push(`You are signed in as ${state.login}; the bot acts only on answers from ${OPERATOR}.`); + warnings.push(`You are signed in as ${state.login}; the bot acts only on answers and reviews from ${OPERATOR}.`); } if (state.gh.rateLow(RATE_LOW_FRACTION)) warnings.push("The API rate budget is low; polling less often."); + if (state.help) warnings.push(`Keys: ${HELP[r]}`); setNotice(warnings.filter(Boolean).join(" ") || undefined); } +function labelOf(state: State): (e: Entry) => RowLabel | undefined { + return (e) => { + if (e.kind === "pr") { + if (e.pr && state.reviewed.has(refKey(e.pr.ref))) return { text: "reviewed from here", cls: "answered" }; + const v = e.verdict; + if (!v) return { text: "checking reviews…", cls: "pending" }; + return v.state === "none" ? undefined : { text: VERDICT_LABEL[v.state], cls: `v-${v.state}` }; + } + const st = e.item ? answerState(e.item, state.sent, state.receipts) : undefined; + return st ? { text: STATE_LABEL[st], cls: st } : undefined; + }; +} + +function rows(): HTMLElement[] { + return [...byId("view").querySelectorAll(`.${ROW_CLASS}`)]; +} + +/** Mark the selected queue row, defaulting to the first. */ +function markSelected(state: State, scroll: boolean): void { + const all = rows(); + const sel = all.find((r) => r.getAttribute(ROW_KEY_ATTR) === state.selected) ?? all[0]; + for (const r of all) { + r.classList.toggle("sel", r === sel); + if (r === sel) r.setAttribute("aria-current", "true"); + else r.removeAttribute("aria-current"); + } + state.selected = sel?.getAttribute(ROW_KEY_ATTR) ?? undefined; + if (scroll && sel) { + sel.scrollIntoView({ block: "nearest" }); + sel.focus({ preventScroll: true }); + } +} + +function renderQueue(state: State): void { + showMain(queueView(state.entries, labelOf(state))); + markSelected(state, false); +} + /** Rebuild the whole view for the current route. */ function renderRoute(state: State): void { delete state.shown; + delete state.shownPr; delete state.itemNote; + state.fileIndex = -1; renderChrome(state); if (!state.loaded) { showMain(h("p", { class: "empty" }, "Loading…")); return; } - const id = routeItemId(); - const item = id ? state.items.find((i) => i.nodeId === id) : undefined; + const r = route(); + if (r.route === "pr") { + renderPr(state, r.ref); + return; + } + const item = r.route === "item" ? state.items.find((i) => i.nodeId === r.id) : undefined; if (!item) { - if (id) setNotice("That item no longer needs you (or isn't on the board)."); - showMain(queueView(state.items, state.sent, state.receipts)); + if (r.route === "item") setNotice("That item no longer needs you (or isn't on the board)."); + renderQueue(state); return; } const ctx = state.context.get(item.nodeId); @@ -117,6 +205,73 @@ function renderRoute(state: State): void { if (!ctx) void refreshContext(state, item); } +function prEntry(state: State, key: string): Entry | undefined { + return state.entries.find((e) => e.key === `pr:${key}`); +} + +function renderPr(state: State, ref: { owner: string; repo: string; number: number }): void { + const key = refKey(ref); + const detail = state.details.get(key); + if (!detail) { + showMain(h("main", { class: "item" }, h("a", { href: "#", class: "back" }, "← Queue (u)"), h("p", { class: "empty" }, `Loading ${key}…`))); + void loadPr(state, ref); + return; + } + // Opening a PR the forge says changed since we read it reads it again, + // once per search result, in case the two timestamps never agree. + const searched = state.prs.find((p) => refKey(p.ref) === key)?.updatedAt; + if (searched && detail.updatedAt && searched > detail.updatedAt && state.reloadedFor.get(key) !== searched) { + state.reloadedFor.set(key, searched); + state.details.delete(key); + renderPr(state, ref); + return; + } + state.shownPr = { key, updatedAt: detail.updatedAt }; + showMain( + prView(detail, prEntry(state, key), render, { + review: async (action, text, draft) => { + const url = await submitReview(state.gh, ref, composeReview(action, text, detail.head, { draft })); + state.reviewed.add(key); + state.forceForge = true; + // Our own review moved updated_at: note the new one so it isn't + // reported as a change, without re-rendering the form. + void loadPrDetail(state.gh, ref) + .then((d) => { + state.details.set(key, d); + if (state.shownPr?.key === key) state.shownPr.updatedAt = d.updatedAt; + }) + .catch(() => { + // The next open reloads it. + }); + return url; + }, + }, { reviewedHere: state.reviewed.has(key) }), + ); +} + +/** Load (or reload) a PR's details, then show them if it is still open. */ +async function loadPr(state: State, ref: { owner: string; repo: string; number: number }): Promise { + const key = refKey(ref); + try { + state.details.set(key, await loadPrDetail(state.gh, ref)); + } catch (e) { + const r = route(); + if (r.route === "pr" && refKey(r.ref) === key) { + showMain( + h( + "main", + { class: "item" }, + h("a", { href: "#", class: "back" }, "← Queue (u)"), + h("p", { class: "warn" }, `Couldn't load ${key}: ${message(e)}. Press r to retry.`), + ), + ); + } + return; + } + const r = route(); + if (r.route === "pr" && refKey(r.ref) === key) renderRoute(state); +} + /** (Re)load an item's context and update only that part of its view. */ async function refreshContext(state: State, item: Item): Promise { const ctx = await loadContext(state.gh, item); @@ -133,8 +288,86 @@ async function refreshReceipts(state: State): Promise { state.receipts = new Map(checks.flatMap(([id, r]) => (r ? [[id, r] as const] : []))); } +/** Re-search the forge when due; true if its list of PRs changed. Verdicts follow in the background. */ +async function pollForge(state: State): Promise { + const wait = state.forceForge ? FORGE_MIN_INTERVAL_MS : FORGE_POLL_INTERVAL_MS; + if (Date.now() - state.lastForgePoll < wait) return false; + state.forceForge = false; + try { + const prs = await loadForgePrs(state.gh); + state.lastForgePoll = Date.now(); + state.forgeKnown = true; + delete state.forgeError; + const sig = (p: readonly ForgePr[]) => JSON.stringify(p.map((x) => [refKey(x.ref), x.updatedAt])); + const changed = sig(prs) !== sig(state.prs); + state.prs = prs; + void pollVerdicts(state); + return changed; + } catch (e) { + state.forgeError = `Couldn't read the forge's PRs: ${message(e)}`; + return false; + } +} + +/** Re-read the verdicts of PRs that changed, and update the view if any moved. */ +async function pollVerdicts(state: State): Promise { + // One at a time; the next forge poll catches up on what this one missed. + if (state.verdictsRunning) return; + state.verdictsRunning = true; + try { + const verdicts = await refreshVerdicts(state.gh, state.prs, state.verdicts); + const sig = (v: ReadonlyMap) => JSON.stringify([...v].map(([k, e]) => [k, e.head, e.verdict.state]).sort()); + const changed = sig(verdicts) !== sig(state.verdicts); + state.verdicts = verdicts; + if (changed && state.loaded) update(state, false, false); + } catch (e) { + state.forgeError = `Couldn't read the forge PRs' reviews: ${message(e)}`; + renderChrome(state); + } finally { + state.verdictsRunning = false; + } +} + +function rebuildEntries(state: State): void { + const verdicts = new Map([...state.verdicts].map(([k, v]) => [k, v.verdict])); + state.entries = buildEntries(state.items, state.prs, verdicts, state.forgeKnown); +} + +/** + * Rebuild the queue after new data, and refresh the view without losing + * a half-typed answer or review: only the queue is re-rendered; an open + * item or PR just gets a note if it changed underneath. + */ +function update(state: State, boardChanged: boolean, first: boolean): void { + rebuildEntries(state); + const r = route(); + if (first || r.route === "queue") { + renderRoute(state); + } else if (r.route === "item") { + const item = state.items.find((i) => i.nodeId === r.id); + if (!item) state.itemNote = "This item no longer needs you; the bot may have acted on it."; + else if (state.shown && itemKey(item) !== state.shown.key) { + state.itemNote = "This item changed on the board since you opened it; go back and reopen it to see the new version."; + } + if (item && boardChanged) void refreshContext(state, item); + renderChrome(state); + } else { + const key = refKey(r.ref); + const now = state.prs.find((p) => refKey(p.ref) === key)?.updatedAt; + if (state.shownPr?.key === key && now && state.shownPr.updatedAt && now > state.shownPr.updatedAt) { + state.itemNote = "This PR changed since you opened it (new commits or reviews); press r to reload."; + } + renderChrome(state); + } +} + async function poll(state: State): Promise { + // One poll at a time; the running one schedules the next. + if (state.polling) return; + state.polling = true; clearTimeout(state.timer); + // The board and the forge load side by side; whichever answers first shows first. + const forge = pollForge(state); try { const q = await loadQueue(state.gh); state.lastPoll = new Date(); @@ -143,35 +376,28 @@ async function poll(state: State): Promise { if (q.changed || first) { state.items = q.items; state.boardPublic = q.boardPublic; - state.loaded = true; // Keep "sent" only for items still waiting on the bot. const ids = new Set(q.items.map((i) => i.nodeId)); for (const s of state.sent) if (!ids.has(s)) state.sent.delete(s); state.context.clear(); await refreshReceipts(state); - const open = routeItemId(); - if (first || !open || !state.shown) { - renderRoute(state); - } else { - // Keep the open item's form (and whatever is typed in it); say - // if the item changed, and reload its context. - const item = q.items.find((i) => i.nodeId === open); - if (!item) state.itemNote = "This item no longer needs you; the bot may have acted on it."; - else if (itemKey(item) !== state.shown.key) { - state.itemNote = "This item changed on the board since you opened it; go back and reopen it to see the new version."; - } - if (item) void refreshContext(state, item); - renderChrome(state); - } + state.loaded = true; + update(state, true, first); } else { renderChrome(state); } } catch (e) { - state.error = `Couldn't read the board: ${e instanceof Error ? e.message : String(e)}`; + state.error = `Couldn't read the board: ${message(e)}`; if (state.loaded) renderChrome(state); else renderRoute(state); } - schedule(state); + try { + if ((await forge) && state.loaded) update(state, false, false); + } finally { + // Whatever went wrong above, keep polling. + state.polling = false; + schedule(state); + } } function schedule(state: State): void { @@ -180,6 +406,94 @@ function schedule(state: State): void { state.timer = setTimeout(() => void poll(state), POLL_INTERVAL_MS * slow); } +function isEditing(el: Element | null): boolean { + if (!(el instanceof HTMLElement)) return false; + return el.isContentEditable || ["INPUT", "TEXTAREA", "SELECT"].includes(el.tagName); +} + +function moveFile(state: State, step: number): void { + const files = [...byId("view").querySelectorAll(`details.${FILE_CLASS}`)]; + if (files.length === 0) return; + state.fileIndex = Math.max(0, Math.min(files.length - 1, state.fileIndex + step)); + const f = files[state.fileIndex]; + for (const x of files) x.classList.toggle("sel", x === f); + f?.scrollIntoView({ block: "start" }); + f?.querySelector("summary")?.focus({ preventScroll: true }); +} + +function run(state: State, cmd: Command, where: Route): void { + const view = byId("view"); + switch (cmd) { + case "next": + case "prev": { + const step = cmd === "next" ? 1 : -1; + if (where === "pr") { + moveFile(state, step); + return; + } + const all = rows(); + const i = all.findIndex((r) => r.getAttribute(ROW_KEY_ATTR) === state.selected); + const next = all[Math.max(0, Math.min(all.length - 1, i + step))]; + state.selected = next?.getAttribute(ROW_KEY_ATTR) ?? undefined; + markSelected(state, true); + return; + } + case "open": { + const sel = rows().find((r) => r.classList.contains("sel")); + if (sel) window.location.hash = sel.getAttribute("href") ?? "#"; + return; + } + case "back": + window.location.hash = "#"; + return; + case "refresh": { + const r = route(); + if (r.route === "pr") { + void loadPr(state, r.ref); + return; + } + state.forceForge = true; + void poll(state); + return; + } + case "approve": + view.querySelector(`.${REVIEW_FORM_CLASS} button[data-action="${APPROVE_ACTION}"]`)?.click(); + return; + case "fold": { + const files = view.querySelectorAll(`details.${FILE_CLASS}`); + const f = files[Math.max(0, state.fileIndex)]; + if (f) f.open = !f.open; + return; + } + case "compose": + view.querySelector("form textarea")?.focus(); + return; + case "help": + state.help = !state.help; + renderChrome(state); + return; + } +} + +function installKeys(state: State): void { + document.addEventListener("keydown", (ev) => { + const where = route().route; + const cmd = keyCommand( + { key: ev.key, ctrlKey: ev.ctrlKey, metaKey: ev.metaKey, altKey: ev.altKey, editing: ev.isComposing || isEditing(document.activeElement) }, + where, + ); + if (!cmd) return; + // Enter on a focused link or button does its own thing. + if (ev.key === "Enter" && document.activeElement instanceof HTMLElement && document.activeElement.matches("a, button, summary")) return; + ev.preventDefault(); + if (cmd === "blur") { + (document.activeElement as HTMLElement | null)?.blur(); + return; + } + run(state, cmd, where); + }); +} + /** Which token problem to explain on the sign-in page. */ type SignInReason = "none" | "rejected"; @@ -190,9 +504,23 @@ async function start(source: TokenSource): Promise { items: [], loaded: false, boardPublic: true, + prs: [], + verdicts: new Map(), + lastForgePoll: 0, + forceForge: false, + forgeKnown: false, + verdictsRunning: false, + polling: false, + entries: [], sent: new Set(), + reviewed: new Set(), context: new Map(), + details: new Map(), + reloadedFor: new Map(), receipts: new Map(), + fileIndex: -1, + help: false, + selected: undefined, }; byId("meta").textContent = "Checking the token…"; try { @@ -213,7 +541,12 @@ async function start(source: TokenSource): Promise { byId("signout").onclick = () => { void source.signOut().finally(() => window.location.reload()); }; - window.addEventListener("hashchange", () => renderRoute(state)); + window.addEventListener("hashchange", () => { + renderRoute(state); + if (route().route === "queue") markSelected(state, true); + else window.scrollTo(0, 0); + }); + installKeys(state); document.addEventListener("visibilitychange", () => { if (!document.hidden) void poll(state); else clearTimeout(state.timer); @@ -296,7 +629,17 @@ function showSignIn(reason: SignInReason): void { showMain(signInView(reason)); } +/** Keep --header-h at the sticky header's height, for the sticky file names below it. */ +function trackHeaderHeight(): void { + const header = document.querySelector("header.top"); + if (!header) return; + const set = () => document.documentElement.style.setProperty("--header-h", `${header.offsetHeight}px`); + new ResizeObserver(set).observe(header); + set(); +} + async function main(): Promise { + trackHeaderHeight(); // A CSP meta tag can't forbid framing, so refuse to run in a frame: // otherwise another site could overlay the approve and send buttons. if (window.top !== window.self) { diff --git a/src/github/prview.ts b/src/github/prview.ts new file mode 100644 index 0000000..b29b779 --- /dev/null +++ b/src/github/prview.ts @@ -0,0 +1,320 @@ +// The review pane for one forge PR: description, commits with their full +// messages, CI, a per-file diff, and the review form. Commit messages and +// diffs are untrusted too, and go in as text nodes only. + +import { h, link } from "../dom.ts"; +import type { Renderer } from "../markdown.ts"; +import { BOARD_URL, BOT_LOGIN, DIFF_COLLAPSE_LINES, FORGE_ORG } from "./config.ts"; +import { + type CiState, + ciSummary, + DRAFT_LINE, + type DiffLine, + parseBotMeta, + parsePatch, + type ReviewAction, + VERDICT_LABEL, + withoutBotMeta, +} from "./forge.ts"; +import type { FileDiff, PrDetail } from "./prs.ts"; +import type { Entry } from "./queue.ts"; +import { pill, time } from "./view.ts"; + +/** Classes the keyboard handler looks for. */ +export const FILE_CLASS = "file"; +export const REVIEW_FORM_CLASS = "review"; +export const APPROVE_ACTION = "approve"; + +export interface PrViewHandlers { + /** Submit a review; resolves to its URL. */ + review(action: ReviewAction, text: string, draft: boolean): Promise; +} + +export interface PrViewOptions { + /** A review was already sent from this tab. */ + reviewedHere: boolean; +} + +/** Owners whose bot-authored PRs can be reviewed from here. */ +const REVIEWABLE_OWNERS: readonly string[] = [FORGE_ORG, BOT_LOGIN]; + +/** + * Whether the pane offers a review form: only for the bot's own PRs in + * its own space, so a crafted link can't turn this page into a one-click + * approval of someone else's PR. + */ +export function canReview(d: PrDetail): boolean { + return d.state === "open" && d.author === BOT_LOGIN && REVIEWABLE_OWNERS.includes(d.ref.owner); +} + +/** What he hasn't seen of a PR, for the approval's confirmation. */ +export interface Unseen { + /** Files whose diff was never expanded. */ + unopened: number; + /** Files GitHub gives no diff for. */ + noDiff: number; + /** Files or commits beyond what the API lists. */ + truncated: boolean; +} + +export function unseenNote(u: Unseen): string { + const parts = [ + u.unopened ? `${u.unopened} file${u.unopened > 1 ? "s" : ""} never expanded` : "", + u.noDiff ? `${u.noDiff} without a diff here` : "", + u.truncated ? "files or commits beyond what GitHub lists" : "", + ].filter(Boolean); + return parts.length ? ` Not seen here: ${parts.join(", ")}.` : ""; +} + +const CI_LABEL: Record = { + success: "CI passed", + failure: "CI failed", + pending: "CI running", + none: "no CI", +}; + +const short = (sha: string) => sha.slice(0, 10); + +function ciBadge(state: CiState): HTMLElement { + return h("span", { class: `ci ci-${state}` }, CI_LABEL[state]); +} + +function linkRow(d: PrDetail, entry: Entry | undefined): HTMLElement { + const meta = parseBotMeta(d.body); + const upstream = meta.upstream ?? d.parent; + const links = h("div", { class: "links" }); + links.append(link(d.url, `${d.ref.owner}/${d.ref.repo}#${d.ref.number}`)); + links.append(link(`${d.url}/files`, "files on GitHub")); + if (upstream) { + const base = meta.base ?? d.baseRef; + links.append(link(`https://github.com/${upstream}`, `upstream ${upstream}${base ? ` (${base})` : ""}`)); + if (base && d.headRef) { + links.append(link(`https://github.com/${upstream}/compare/${base}...${d.ref.owner}:${d.ref.repo}:${d.headRef}`, "compare with upstream")); + } + } + if (entry?.item?.url) links.append(link(entry.item.url, "tracking issue")); + links.append(link(BOARD_URL, "board")); + return links; +} + +function reviewForm(d: PrDetail, opts: PrViewOptions, handlers: PrViewHandlers, unseen: () => Unseen): HTMLElement { + const text = h("textarea", { + name: "text", + rows: "3", + placeholder: "Optional for an approval; required to request changes or comment", + "aria-label": "Review text", + }); + const draft = h("input", { type: "checkbox", id: "review-draft" }); + const status = h("p", { class: "status", role: "status" }); + const approve = h("button", { type: "button", class: "primary", "data-action": APPROVE_ACTION }, "Approve (a)"); + const changes = h("button", { type: "button", "data-action": "request-changes" }, "Request changes"); + const comment = h("button", { type: "button", "data-action": "comment" }, "Comment"); + const buttons = [approve, changes, comment]; + const forge = d.ref.owner === FORGE_ORG; + let sent = opts.reviewedHere; + const enable = () => { + for (const b of buttons) b.disabled = false; + // Approving needs the diff shown to be the head's. + approve.disabled = !d.consistent; + }; + enable(); + if (!d.consistent) status.textContent = "Approve is off until a reload (r) shows the head's commits."; + const form = h( + "form", + { class: REVIEW_FORM_CLASS }, + text, + forge + ? h("label", { class: "check", for: "review-draft" }, draft, ` Ask for a draft upstream PR (adds ${DRAFT_LINE}; unchecked doesn't undo an earlier ${DRAFT_LINE})`) + : null, + h("div", { class: "actions" }, ...buttons), + h( + "p", + { class: "target" }, + `Reviews head ${short(d.head)} as you.`, + forge ? " An approval is what bot-pr promote acts on: it opens the upstream PR from exactly this head, signing off where the project wants DCO." : "", + " If the head moves before you send, nothing is sent.", + ), + status, + ); + form.addEventListener("submit", (ev) => ev.preventDefault()); + const send = (action: ReviewAction) => { + const wantDraft = action === "approve" && draft.checked; + const what = action === "approve" ? `Approve${wantDraft ? ` (with ${DRAFT_LINE})` : ""}` : action === "comment" ? "Comment on" : "Request changes on"; + const again = sent ? " You already reviewed it from here." : ""; + const note = action === "approve" ? unseenNote(unseen()) : ""; + if (!window.confirm(`${what} ${d.ref.owner}/${d.ref.repo}#${d.ref.number} at ${short(d.head)}?${note}${again}`)) return; + for (const b of buttons) b.disabled = true; + status.textContent = "Sending…"; + handlers + .review(action, text.value, wantDraft) + .then((url) => { + sent = true; + text.value = ""; + status.textContent = "Sent: "; + status.append(link(url, url)); + enable(); + }) + .catch((e: unknown) => { + status.textContent = `Not sent: ${e instanceof Error ? e.message : String(e)}`; + enable(); + }); + }; + for (const b of buttons) b.addEventListener("click", () => send(b.dataset.action as ReviewAction)); + return form; +} + +function commitsSection(d: PrDetail): HTMLElement { + const s = h("section", { class: "commits" }, h("h3", {}, `Commits · ${d.commitCount}`)); + const open = d.commits.length <= 5; + for (const c of d.commits) { + const [subject = "", ...rest] = c.message.replace(/\r\n?/g, "\n").split("\n"); + const body = rest.join("\n").replace(/^\n+/, "").trimEnd(); + const det = h( + "details", + { class: "commit" }, + h("summary", {}, h("code", {}, short(c.sha)), " ", h("span", { class: "subject" }, subject), h("span", { class: "tag" }, ` ${c.author}${c.date ? ` · ${time(c.date)}` : ""}`)), + body ? h("pre", { class: "msg" }, body) : h("p", { class: "note" }, "(no body)"), + h("p", { class: "note" }, link(c.url, "commit on GitHub")), + ); + det.open = open; + s.append(det); + } + if (d.commits.length < d.commitCount) s.append(h("p", { class: "note" }, `Showing the first ${d.commits.length}; see the rest on GitHub.`)); + return s; +} + +function checksSection(d: PrDetail): HTMLElement { + const s = h("section", { class: "checks" }, h("h3", {}, "CI ", ciBadge(ciSummary(d.checks)))); + if (d.checks.length === 0) { + s.append(h("p", { class: "note" }, "No checks on this head. Forge forks run no CI: the description says how it was tested, and upstream CI runs once promoted.")); + return s; + } + const list = h("ul", { class: "checklist" }); + for (const c of d.checks) list.append(h("li", { class: `ci-${c.state}` }, h("span", { class: "dot" }), link(c.url, c.name), h("span", { class: "tag" }, ` ${c.detail}`))); + s.append(list); + return s; +} + +function diffTable(lines: readonly DiffLine[]): HTMLElement { + const table = h("table", { class: "diff" }); + const body = h("tbody"); + for (const l of lines) { + if (l.kind === "hunk" || l.kind === "note") { + body.append(h("tr", { class: l.kind }, h("td", { class: "ln", colspan: "2" }), h("td", { class: "code" }, l.text))); + continue; + } + const sign = l.kind === "add" ? "+" : l.kind === "del" ? "-" : " "; + body.append( + h( + "tr", + { class: l.kind }, + h("td", { class: "ln" }, l.old === undefined ? "" : String(l.old)), + h("td", { class: "ln" }, l.new === undefined ? "" : String(l.new)), + h("td", { class: "code" }, sign + l.text), + ), + ); + } + table.append(body); + return table; +} + +interface FileView { + el: HTMLDetailsElement; + /** Whether its diff was ever shown. */ + seen(): boolean; + hasDiff: boolean; +} + +function fileView(f: FileDiff): FileView { + const lines = f.patch === undefined ? undefined : parsePatch(f.patch); + const hasDiff = lines !== undefined && lines.length > 0; + const name = f.previous && f.previous !== f.filename ? `${f.previous} → ${f.filename}` : f.filename; + const det = h( + "details", + { class: FILE_CLASS }, + h( + "summary", + {}, + h("span", { class: `fstatus s-${f.status}` }, f.status), + h("span", { class: "fname" }, name), + h("span", { class: "counts" }, h("span", { class: "plus" }, `+${f.additions}`), " ", h("span", { class: "minus" }, `−${f.deletions}`)), + ), + ); + // Build the table on first open: a large PR stays quick to show. + let built = false; + const build = () => { + if (built) return; + built = true; + if (!lines || !hasDiff) { + det.append(h("p", { class: "note" }, "No diff to show (binary, too large, or a rename only). ", link(f.url, "View the file"))); + } else { + det.append(h("div", { class: "diffwrap" }, diffTable(lines))); + } + }; + det.addEventListener("toggle", () => { + if (det.open) build(); + }); + if (!lines || lines.length <= DIFF_COLLAPSE_LINES) { + det.open = true; + build(); + } + return { el: det, seen: () => built, hasDiff }; +} + +function filesSection(d: PrDetail, files: readonly FileView[]): HTMLElement { + const expand = h("button", { type: "button", class: "small" }, "Expand all"); + const collapse = h("button", { type: "button", class: "small" }, "Collapse all"); + const s = h( + "section", + { class: "files" }, + h( + "div", + { class: "files-h" }, + h("h3", {}, `Files · ${d.changedFiles} · `, h("span", { class: "plus" }, `+${d.additions}`), " ", h("span", { class: "minus" }, `−${d.deletions}`)), + h("span", { class: "tag" }, "j/k file, x fold"), + expand, + collapse, + ), + ); + s.append(...files.map((f) => f.el)); + expand.addEventListener("click", () => files.forEach((f) => (f.el.open = true))); + collapse.addEventListener("click", () => files.forEach((f) => (f.el.open = false))); + if (d.files.length < d.changedFiles) s.append(h("p", { class: "note" }, `Showing ${d.files.length} of ${d.changedFiles} files; see the rest on GitHub.`)); + return s; +} + +export function prView(d: PrDetail, entry: Entry | undefined, render: Renderer, handlers: PrViewHandlers, opts: PrViewOptions): HTMLElement { + const verdict = d.verdict.state === "none" ? undefined : VERDICT_LABEL[d.verdict.state]; + const files = d.files.map(fileView); + const unseen = (): Unseen => ({ + unopened: files.filter((f) => f.hasDiff && !f.seen()).length, + noDiff: files.filter((f) => !f.hasDiff).length, + truncated: d.files.length < d.changedFiles || d.commits.length < d.commitCount, + }); + let form: HTMLElement; + if (canReview(d)) form = reviewForm(d, opts, handlers, unseen); + else if (d.state !== "open") form = h("p", { class: "warn" }, `This PR is ${d.state}; there is nothing to review.`); + else form = h("p", { class: "note" }, `Reviews from here are only for ${BOT_LOGIN}'s PRs in ${REVIEWABLE_OWNERS.join(" and ")}; use GitHub for this one.`); + return h( + "main", + { class: "item pr" }, + h("a", { href: "#", class: "back" }, "← Queue (u)"), + h( + "div", + { class: "hdr" }, + pill(entry?.priority), + h("span", { class: "tag" }, [entry?.item?.org, d.draft ? "draft PR" : "PR", d.state === "open" ? "" : d.state, d.isPrivate ? "private" : ""].filter(Boolean).join(" · ")), + ciBadge(ciSummary(d.checks)), + verdict ? h("span", { class: `state v-${d.verdict.state}` }, d.verdict.url ? link(d.verdict.url, verdict) : verdict) : null, + ), + h("h2", {}, d.title), + linkRow(d, entry), + ...d.warnings.map((w) => h("p", { class: "warn" }, w)), + entry?.item?.why ? h("section", {}, h("h3", {}, "Why (board)"), h("div", { class: "md" }, render(entry.item.why))) : null, + form, + h("section", {}, h("h3", {}, "Description"), h("div", { class: "md" }, render(withoutBotMeta(d.body) || "(empty)"))), + checksSection(d), + commitsSection(d), + filesSection(d, files), + ); +} diff --git a/src/github/queue.ts b/src/github/queue.ts new file mode 100644 index 0000000..72f3faa --- /dev/null +++ b/src/github/queue.ts @@ -0,0 +1,141 @@ +// The one ranked queue: forge PRs waiting for his review, board questions +// and chores, P0 first, then oldest first. Pure, so tests can check the +// ranking and what gets merged or dropped. + +import { type Item, NO_PRIORITY, PRIORITY_ORDER, questionOf } from "./board.ts"; +import { DRAFT, FORGE_ORG, NEEDS_HUMAN } from "./config.ts"; +import { type ForgePr, parseBotMeta, refKey, type Verdict, waitsOnReviewer } from "./forge.ts"; + +export type EntryKind = "pr" | "question" | "chore"; + +export interface Entry { + /** `pr:owner/repo#n` or `item:PVTI_...`. */ + key: string; + kind: EntryKind; + priority?: string; + /** When it started waiting, as far as the API says (ISO 8601). */ + since?: string; + title: string; + /** Where it is, e.g. `owner/repo#12`, or "draft item". */ + where: string; + /** The app route that opens it. */ + href: string; + /** The board item: the entry itself, or the one tracking a PR. */ + item?: Item; + pr?: ForgePr; + verdict?: Verdict; +} + +/** Rank in PRIORITY_ORDER; anything else ranks after it, and none last. */ +export function priorityRank(p: string | undefined): number { + if (p === undefined) return PRIORITY_ORDER.length + 1; + const i = PRIORITY_ORDER.indexOf(p); + return i < 0 ? PRIORITY_ORDER.length : i; +} + +/** Priority first, then oldest first (no date last), then by key for stability. */ +export function rankEntries(entries: readonly Entry[]): Entry[] { + const time = (e: Entry) => { + const t = e.since ? Date.parse(e.since) : Number.NaN; + return Number.isNaN(t) ? Number.POSITIVE_INFINITY : t; + }; + return [...entries].sort( + (a, b) => priorityRank(a.priority) - priorityRank(b.priority) || time(a) - time(b) || a.key.localeCompare(b.key), + ); +} + +const FORGE_PR_RE = new RegExp(`^https://github\\.com/${FORGE_ORG}/[A-Za-z0-9._-]+/pull/\\d+$`); + +export function prHref(pr: ForgePr): string { + return `#pr/${pr.ref.owner}/${pr.ref.repo}/${pr.ref.number}`; +} + +export function itemHref(item: Item): string { + return `#item/${item.nodeId}`; +} + +function itemWhere(item: Item): string { + if (item.ref) return refKey(item.ref); + return item.kind === "draft" ? "draft item" : "item"; +} + +/** + * Merge the board and the forge into one ranked list. + * + * - A forge PR is listed while it waits on him (see waitsOnReviewer; an + * unknown verdict counts as waiting). Its priority is its board item's, + * found by the item id in its bot-meta section, else by Branch. + * - A Draft board item tracking a forge PR is that PR's entry, never a + * second one. One whose Branch holds only forge PRs, none of them open + * and waiting, is stale (promoted or closed) and dropped. + * - Other Draft items (a gist to read) are chores, and so are Needs human + * items that ask no question with options or an id. + * + * Until the forge has been read once (`forgeKnown`), nothing is stale: + * forge-only Draft items are listed as chores rather than dropped. + */ +export function buildEntries( + items: readonly Item[], + prs: readonly ForgePr[], + verdicts: ReadonlyMap, + forgeKnown = true, +): Entry[] { + const byNode = new Map(items.map((i) => [i.nodeId, i])); + const byBranch = new Map(); + for (const i of items) for (const u of i.branch) if (!byBranch.has(u)) byBranch.set(u, i); + const tracked = new Set(); + const out: Entry[] = []; + + for (const pr of prs) { + const metaItem = parseBotMeta(pr.body).item; + const item = (metaItem ? byNode.get(metaItem) : undefined) ?? byBranch.get(pr.url); + if (item?.status === DRAFT) tracked.add(item.nodeId); + const key = refKey(pr.ref); + const verdict = verdicts.get(key); + if (verdict && !waitsOnReviewer(verdict)) continue; + const e: Entry = { key: `pr:${key}`, kind: "pr", title: pr.title, where: key, href: prHref(pr), pr }; + if (item?.priority) e.priority = item.priority; + if (item) e.item = item; + if (pr.createdAt) e.since = pr.createdAt; + if (verdict) e.verdict = verdict; + out.push(e); + } + + for (const item of items) { + if (tracked.has(item.nodeId)) continue; + let kind: EntryKind; + if (item.status === DRAFT) { + const forgeOnly = item.branch.length > 0 && item.branch.every((u) => FORGE_PR_RE.test(u)); + if (forgeOnly && forgeKnown) continue; + kind = "chore"; + } else if (item.status === NEEDS_HUMAN) { + const q = questionOf(item); + kind = q.options.length > 0 || q.id !== undefined ? "question" : "chore"; + } else { + continue; + } + const e: Entry = { key: `item:${item.nodeId}`, kind, title: item.title, where: itemWhere(item), href: itemHref(item), item }; + if (item.priority) e.priority = item.priority; + const since = item.createdAt ?? item.updatedAt; + if (since) e.since = since; + out.push(e); + } + return rankEntries(out); +} + +export interface EntryGroup { + priority: string; + entries: Entry[]; +} + +/** Consecutive runs of one priority in a ranked list, for headings. */ +export function groupRanked(entries: readonly Entry[]): EntryGroup[] { + const out: EntryGroup[] = []; + for (const e of entries) { + const p = e.priority ?? NO_PRIORITY; + const last = out.at(-1); + if (last?.priority === p) last.entries.push(e); + else out.push({ priority: p, entries: [e] }); + } + return out; +} diff --git a/src/github/view.ts b/src/github/view.ts index 94ceb65..45af101 100644 --- a/src/github/view.ts +++ b/src/github/view.ts @@ -4,28 +4,25 @@ import { type Answer, getDraftSection, parseAnswer } from "../answer.ts"; import { h, link } from "../dom.ts"; import type { Renderer } from "../markdown.ts"; -import { type AnswerTarget, groupByPriority, type Item, type Question } from "./board.ts"; +import type { AnswerTarget, Item, Question } from "./board.ts"; import type { Context, ReceiptStatus } from "./backend.ts"; import { BOARD_URL, OPERATOR } from "./config.ts"; +import { type Entry, type EntryKind, groupRanked } from "./queue.ts"; -/** Characters of Why shown on a queue card. */ -const WHY_EXCERPT = 240; - -export function itemHash(item: Item): string { - return `#item/${item.nodeId}`; -} +/** Characters of Why shown on a queue row. */ +const WHY_EXCERPT = 160; function refLabel(item: Item): string { if (item.ref) return `${item.ref.owner}/${item.ref.repo}#${item.ref.number}`; return item.kind === "draft" ? "draft item" : "item"; } -function excerpt(text: string, max: number): string { +export function excerpt(text: string, max: number): string { const flat = text.replace(/\s+/g, " ").trim(); return flat.length > max ? `${flat.slice(0, max - 1)}…` : flat; } -function time(iso: string | undefined): string { +export function time(iso: string | undefined): string { if (!iso) return ""; const d = new Date(iso); return Number.isNaN(d.getTime()) @@ -33,7 +30,23 @@ function time(iso: string | undefined): string { : d.toLocaleString([], { month: "short", day: "numeric", hour: "2-digit", minute: "2-digit" }); } -function pill(priority: string | undefined): HTMLElement { +const AGE_UNITS: [number, string][] = [ + [7 * 24 * 3600_000, "w"], + [24 * 3600_000, "d"], + [3600_000, "h"], + [60_000, "m"], +]; + +/** A compact age, e.g. "3d", or "" for no or a bad date. */ +export function age(iso: string | undefined, now: number): string { + const t = iso ? Date.parse(iso) : Number.NaN; + if (Number.isNaN(t)) return ""; + const ms = Math.max(0, now - t); + for (const [unit, suffix] of AGE_UNITS) if (ms >= unit) return `${Math.floor(ms / unit)}${suffix}`; + return "now"; +} + +export function pill(priority: string | undefined): HTMLElement { const p = priority ?? "–"; return h("span", { class: `pill ${/^P[0-3]$/.test(p) ? p.toLowerCase() : "pn"}` }, p); } @@ -55,38 +68,63 @@ export function answerState( return receipts.get(item.nodeId)?.check.ok ? "answered" : "claimed"; } -const STATE_LABEL: Record, string> = { +export const STATE_LABEL: Record, string> = { answered: "answered", claimed: "body claims an answer (unverified)", }; +/** A short status shown on a queue row, e.g. "answered". */ +export interface RowLabel { + text: string; + cls: string; +} + +const KIND_LABEL: Record = { pr: "PR", question: "Q", chore: "act" }; +const KIND_TITLE: Record = { pr: "forge PR to review", question: "question for you", chore: "action for you" }; + +/** The class queue rows carry, and the attribute holding their key. */ +export const ROW_CLASS = "row"; +export const ROW_KEY_ATTR = "data-key"; + export function queueView( - items: readonly Item[], - sent: ReadonlySet, - receipts: ReadonlyMap, + entries: readonly Entry[], + labelOf: (e: Entry) => RowLabel | undefined, + now: number = Date.now(), ): HTMLElement { const root = h("main", { class: "queue" }); - if (items.length === 0) { + if (entries.length === 0) { root.append(h("p", { class: "empty" }, "Nothing needs you right now.")); return root; } - for (const group of groupByPriority(items)) { - const section = h("section", { class: "group" }, h("h2", { class: "group-h" }, `${group.priority} · ${group.items.length}`)); - for (const item of group.items) { - const state = answerState(item, sent, receipts); + const counts = { pr: 0, question: 0, chore: 0 }; + for (const e of entries) counts[e.kind]++; + root.append( + h("p", { class: "summary" }, `${counts.pr} PRs to review · ${counts.question} questions · ${counts.chore} other · j/k to move, o to open, ? for keys`), + ); + for (const group of groupRanked(entries)) { + const section = h("section", { class: "group" }, h("h2", { class: "group-h" }, `${group.priority} · ${group.entries.length}`)); + for (const e of group.entries) { + const label = labelOf(e); + const why = e.item?.why ? excerpt(e.item.why, WHY_EXCERPT) : ""; section.append( h( "a", - { class: `card${state ? ` ${state}` : ""}`, href: itemHash(item) }, + { class: `${ROW_CLASS} k-${e.kind}${label ? ` ${label.cls}` : ""}`, href: e.href, [ROW_KEY_ATTR]: e.key }, + h("span", { class: `kind k-${e.kind}`, title: KIND_TITLE[e.kind] }, KIND_LABEL[e.kind]), h( - "div", - { class: "hdr" }, - pill(item.priority), - h("span", { class: "tag" }, [item.org, refLabel(item)].filter(Boolean).join(" · ")), - state ? h("span", { class: `state ${state}` }, STATE_LABEL[state]) : null, + "span", + { class: "main" }, + h("span", { class: "title" }, e.title), + h( + "span", + { class: "sub" }, + pill(e.priority), + h("span", { class: "tag" }, [e.item?.org, e.where].filter(Boolean).join(" · ")), + label ? h("span", { class: `state ${label.cls}` }, label.text) : null, + ), + why ? h("span", { class: "why" }, why) : null, ), - h("h3", {}, item.title), - item.why ? h("p", { class: "why" }, excerpt(item.why, WHY_EXCERPT)) : null, + h("span", { class: "age", title: e.since ? `waiting since ${time(e.since)}` : "" }, age(e.since, now)), ), ); } @@ -282,7 +320,7 @@ export function itemView( return h( "main", { class: "item" }, - h("a", { href: "#", class: "back" }, "← Queue"), + h("a", { href: "#", class: "back" }, "← Queue (u)"), h( "div", { class: "hdr" }, diff --git a/static/index.html b/static/index.html index 5a04e22..586749a 100644 --- a/static/index.html +++ b/static/index.html @@ -17,7 +17,7 @@

Needs you

Starting… - +
diff --git a/static/style.css b/static/style.css index 62bcbca..411a21f 100644 --- a/static/style.css +++ b/static/style.css @@ -7,6 +7,7 @@ --p0: #b01c1c; --p0-soft: #fbe3e3; --p1: #b93b12; --p1-soft: #fbe9e2; --p2: #9a6512; --p2-soft: #f8efdd; --p3: #667385; --p3-soft: #eceff3; --ok: #2c7a4f; --ok-soft: #e2f2e8; + --add: #e6f4ea; --add-ink: #1d5b33; --del: #fbe9ea; --del-ink: #8a1f24; --hunk: #eef2fb; --sans: system-ui, -apple-system, "Segoe UI", sans-serif; --mono: ui-monospace, SFMono-Regular, Menlo, monospace; } @@ -18,6 +19,7 @@ --p0: #ff7070; --p0-soft: #3d1414; --p1: #ff8a63; --p1-soft: #3a1d14; --p2: #f0b85a; --p2-soft: #33270f; --p3: #9aa6b8; --p3-soft: #1f2631; --ok: #6fcf97; --ok-soft: #13291d; + --add: #12291b; --add-ink: #8fdcab; --del: #32151a; --del-ink: #ff9ea4; --hunk: #19223a; } } * { box-sizing: border-box; } @@ -44,7 +46,7 @@ header.top h1 a { color: inherit; text-decoration: none; } .tag { font: 12px var(--mono); color: var(--muted); } .state { margin-left: auto; font: 11px var(--mono); padding: 2px 7px; border-radius: 4px; color: var(--ok); background: var(--ok-soft); } .state.claimed { color: var(--p2); background: var(--p2-soft); } -.item { display: grid; gap: 12px; padding-top: 12px; } +.item { display: grid; grid-template-columns: minmax(0, 1fr); gap: 12px; padding-top: 12px; } .item section { background: var(--surface); border: 1px solid var(--line); border-radius: 8px; padding: 4px 14px; overflow-wrap: anywhere; } .item h3 { font-size: 13px; font-family: var(--mono); color: var(--muted); text-transform: uppercase; letter-spacing: .05em; } .links { display: flex; flex-wrap: wrap; gap: 4px 14px; font-size: 14px; } @@ -75,3 +77,71 @@ label.check input { margin-top: 4px; } details.help { font-size: 14px; color: var(--muted); } details.help summary { cursor: pointer; color: var(--accent); } .scopes code { font: .92em var(--mono); } + +/* The ranked queue */ +.summary { font: 12px var(--mono); color: var(--muted); margin: 12px 0 0; } +.row { display: grid; grid-template-columns: auto 1fr auto; gap: 10px; align-items: start; background: var(--surface); border: 1px solid var(--line); border-radius: 8px; padding: 9px 12px; margin-bottom: 6px; color: inherit; text-decoration: none; } +.row:hover { border-color: var(--accent); } +.row.sel { border-color: var(--accent); box-shadow: 0 0 0 1px var(--accent); } +.row.answered, .row.v-approved-older { opacity: .75; } +.row .main { display: grid; gap: 3px; min-width: 0; } +.row .title { font-weight: 600; overflow-wrap: anywhere; line-height: 1.35; } +.row .sub { display: flex; flex-wrap: wrap; gap: 4px 8px; align-items: center; } +.row .why { color: var(--muted); font-size: 13px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } +.row .age { font: 12px var(--mono); color: var(--muted); } +.kind { font: 11px var(--mono); font-weight: 600; padding: 2px 6px; border-radius: 4px; min-width: 3.2em; text-align: center; border: 1px solid var(--line); } +.kind.k-pr { color: var(--accent); background: var(--accent-soft); border-color: transparent; } +.kind.k-question { color: var(--p1); background: var(--p1-soft); border-color: transparent; } +.kind.k-chore { color: var(--muted); } +.row .state { margin-left: 0; } +.state.pending { color: var(--muted); background: var(--p3-soft); } +.state.v-approved-older, .state.v-changes-requested-older { color: var(--p2); background: var(--p2-soft); } +.state.v-changes-requested { color: var(--p1); background: var(--p1-soft); } +.state a { color: inherit; } +button.small, header.top button { font-size: 12px; padding: 4px 9px; } + +/* The PR review pane */ +.ci { font: 11px var(--mono); padding: 2px 7px; border-radius: 4px; background: var(--p3-soft); color: var(--p3); } +.ci-success .dot, .ci.ci-success { color: var(--ok); background: var(--ok-soft); } +.ci.ci-failure { color: var(--p0); background: var(--p0-soft); } +.ci.ci-pending { color: var(--p2); background: var(--p2-soft); } +.checklist { list-style: none; padding: 0; margin: 0 0 8px; display: grid; gap: 4px; font-size: 14px; } +.checklist .dot { display: inline-block; width: 8px; height: 8px; border-radius: 50%; margin-right: 8px; background: var(--p3); } +.checklist .ci-success .dot { background: var(--ok); } +.checklist .ci-failure .dot { background: var(--p0); } +.checklist .ci-pending .dot { background: var(--p2); } +form.review { display: grid; gap: 8px; position: sticky; bottom: 0; z-index: 4; background: var(--ground); padding: 8px 0; border-top: 1px solid var(--line); } +form.review .actions { flex-wrap: wrap; } +details.commit { border-top: 1px solid var(--line); padding: 6px 0; } +details.commit summary { cursor: pointer; overflow-wrap: anywhere; } +details.commit .subject { font-weight: 600; } +pre.msg { white-space: pre-wrap; overflow-wrap: anywhere; margin: 6px 0; } +.files { padding-bottom: 8px !important; } +.files-h { display: flex; flex-wrap: wrap; gap: 6px 10px; align-items: center; } +.files-h h3 { margin-right: auto; } +.plus { color: var(--ok); } .minus { color: var(--p0); } +details.file { border: 1px solid var(--line); border-radius: 6px; margin: 8px 0; background: var(--surface); } +details.file.sel { border-color: var(--accent); } +details.file > summary { cursor: pointer; display: flex; flex-wrap: wrap; gap: 4px 10px; align-items: baseline; padding: 6px 10px; font: 13px var(--mono); position: sticky; top: var(--header-h, 52px); background: var(--surface); border-radius: 6px; z-index: 2; } +details.file[open] > summary { border-bottom: 1px solid var(--line); border-radius: 6px 6px 0 0; } +.fname { overflow-wrap: anywhere; flex: 1; min-width: 12em; } +.fstatus { font-size: 11px; color: var(--muted); text-transform: uppercase; } +.counts { white-space: nowrap; } +.diffwrap { overflow-x: auto; } +table.diff { border-collapse: collapse; width: 100%; font: 12px/1.45 var(--mono); } +table.diff td { padding: 0 8px; vertical-align: top; } +table.diff td.ln { color: var(--muted); text-align: right; user-select: none; width: 1%; white-space: nowrap; } +table.diff td.code { white-space: pre; } +table.diff tr.add { background: var(--add); } table.diff tr.add td.code { color: var(--add-ink); } +table.diff tr.del { background: var(--del); } table.diff tr.del td.code { color: var(--del-ink); } +table.diff tr.hunk, table.diff tr.note { background: var(--hunk); color: var(--muted); } +@media (max-width: 600px) { + body { padding: 0 10px 48px; } + header.top .meta { flex-basis: 100%; order: 3; } + .row { grid-template-columns: auto 1fr; } + .row .age { grid-column: 2; } + table.diff td.ln { padding: 0 4px; } + details.file > summary { position: static; } + form.review { position: static; } + form.review .target { display: none; } +} diff --git a/test/board.test.ts b/test/board.test.ts index 84f5c21..3e6ccd3 100644 --- a/test/board.test.ts +++ b/test/board.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; import { setDraftSection } from "../src/answer.ts"; -import { answerTarget, fieldIds, groupByPriority, type Item, parseIssueUrl, questionOf, queueItems } from "../src/github/board.ts"; +import { answerTarget, fieldIds, type Item, parseIssueUrl, questionOf, queueItems } from "../src/github/board.ts"; import { HOME_OWNERS } from "../src/github/config.ts"; import { fields, rawItems } from "./helpers.ts"; @@ -67,20 +67,6 @@ describe("queueItems", () => { }); }); -describe("groupByPriority", () => { - it("orders P0 first and unprioritised last, keeping board order", () => { - const mk = (nodeId: string, priority?: string): Item => ({ - id: 0, nodeId, kind: "issue", title: nodeId, body: "", why: "", branch: [], gist: [], - ...(priority ? { priority } : {}), - }); - const groups = groupByPriority([mk("a", "P2"), mk("b"), mk("c", "P0"), mk("d", "P2"), mk("e", "P9")]); - assert.deepEqual( - groups.map((g) => [g.priority, g.items.map((i) => i.nodeId)]), - [["P0", ["c"]], ["P2", ["a", "d"]], ["P9", ["e"]], ["No priority", ["b"]]], - ); - }); -}); - describe("answerTarget", () => { const items = new Map(queueItems(rawItems()).map((i) => [i.nodeId, i])); const get = (id: string) => items.get(id) as Item; diff --git a/test/keys.test.ts b/test/keys.test.ts new file mode 100644 index 0000000..09cebbb --- /dev/null +++ b/test/keys.test.ts @@ -0,0 +1,45 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { keyCommand, parseRoute, type Route } from "../src/github/keys.ts"; + +describe("keyCommand", () => { + const press = (key: string, over: Partial[0]> = {}) => ({ + key, ctrlKey: false, metaKey: false, altKey: false, editing: false, ...over, + }); + const cases: [string, Route, Partial[0]>, string | undefined][] = [ + ["j", "queue", {}, "next"], + ["k", "queue", {}, "prev"], + ["o", "queue", {}, "open"], + ["Enter", "queue", {}, "open"], + ["a", "queue", {}, undefined], + ["a", "pr", {}, "approve"], + ["x", "pr", {}, "fold"], + ["u", "pr", {}, "back"], + ["Escape", "item", {}, "back"], + ["r", "item", {}, "refresh"], + ["?", "pr", {}, "help"], + ["a", "pr", { editing: true }, undefined], + ["Escape", "pr", { editing: true }, "blur"], + ["j", "queue", { ctrlKey: true }, undefined], + ["r", "queue", { metaKey: true }, undefined], + ]; + for (const [key, route, over, want] of cases) { + it(`${key} on ${route}${Object.keys(over).length ? ` ${JSON.stringify(over)}` : ""}`, () => assert.equal(keyCommand(press(key, over), route), want)); + } +}); + +describe("parseRoute", () => { + const cases: [string, ReturnType][] = [ + ["", { route: "queue" }], + ["#", { route: "queue" }], + ["#item/PVTI_abc-_1", { route: "item", id: "PVTI_abc-_1" }], + ["#pr/cgwalters-forge/bootc/30", { route: "pr", ref: { owner: "cgwalters-forge", repo: "bootc", number: 30 } }], + ["#pr/o/r.s_t/1", { route: "pr", ref: { owner: "o", repo: "r.s_t", number: 1 } }], + ["#pr/o/r/0", { route: "queue" }], + ["#pr/o/../1", { route: "queue" }], + ["#pr/o/./1", { route: "queue" }], + ["#pr/o/r/1/extra", { route: "queue" }], + ["#item/'; +const HEAD = "e".repeat(40); + +function detail(over: Partial = {}): PrDetail { + return { + ref: { owner: "cgwalters-forge", repo: "widget", number: 7 }, + url: "https://github.com/cgwalters-forge/widget/pull/7", + title: `widget: ${EVIL}`, + body: `Why ${EVIL}\n\n\n- Upstream: \`up/widget\`, base \`main\`\n- Board item: \`PVTI_x\`\n`, + author: "cgwalters-bot", + state: "open", + draft: true, + head: HEAD, + headRef: "bot/fix", + baseRef: "main", + additions: 2, + deletions: 1, + changedFiles: 2, + commitCount: 1, + commits: [{ sha: HEAD, url: "javascript:alert(3)", message: `subject ${EVIL}\n\nbody line ${EVIL}`, author: "cgwalters-bot" }], + files: [ + { filename: `src/${EVIL}.rs`, status: "modified", additions: 2, deletions: 1, patch: `@@ -1,2 +1,3 @@\n ctx\n-${EVIL}\n+new\n+more` }, + { filename: "big.bin", status: "added", additions: 0, deletions: 0 }, + ], + checks: [], + verdict: { state: "none" }, + consistent: true, + updatedAt: "2026-01-01T00:00:00Z", + warnings: [], + ...over, + }; +} + +function assertNoActiveContent(root: Element): void { + assert.equal(root.querySelectorAll("script, img, iframe, svg, style").length, 0); + for (const el of root.querySelectorAll("*")) { + for (const attr of el.getAttributeNames()) assert.ok(!/^on/i.test(attr), `${attr} on ${el.tagName}`); + const href = el.getAttribute("href"); + if (href !== null) assert.match(href, /^(https:|#)/, `href ${href}`); + } +} + +const noReview = { review: async () => "https://github.com/r" }; + +describe("prView", () => { + it("shows untrusted text as text, and drops the bot-meta section", () => { + const root = prView(detail(), undefined, render, noReview, { reviewedHere: false }); + assertNoActiveContent(root); + const text = root.textContent ?? ""; + assert.ok(text.includes(`widget: ${EVIL}`)); + assert.ok(text.includes(`body line ${EVIL}`)); + assert.ok(text.includes(`src/${EVIL}.rs`)); + assert.ok(!text.includes("Board item")); + const code = [...root.querySelectorAll("table.diff tr")].map((tr) => [tr.className, tr.querySelector(".code")?.textContent]); + assert.deepEqual(code, [ + ["hunk", "@@ -1,2 +1,3 @@"], + ["ctx", " ctx"], + ["del", `-${EVIL}`], + ["add", "+new"], + ["add", "+more"], + ]); + assert.match(text, /No diff to show/); + const links = [...root.querySelectorAll("a")].map((a) => a.getAttribute("href")); + assert.ok(links.includes("https://github.com/up/widget")); + assert.ok(links.includes("https://github.com/up/widget/compare/main...cgwalters-forge:widget:bot/fix")); + }); + + it("collapses a large file's diff and builds it when opened", () => { + const patch = `@@ -1,0 +1,400 @@\n${Array.from({ length: 400 }, (_, i) => `+line ${i}`).join("\n")}`; + const root = prView(detail({ files: [{ filename: "big.rs", status: "added", additions: 400, deletions: 0, patch }] }), undefined, render, noReview, { reviewedHere: false }); + const file = root.querySelector("details.file") as HTMLDetailsElement; + assert.equal(file.open, false); + assert.equal(file.querySelectorAll("tr").length, 0); + file.open = true; + file.dispatchEvent(new win.Event("toggle")); + assert.equal(file.querySelectorAll("tr").length, 401); + }); + + it("submits the button's action after confirming, with /draft only for approval", async () => { + const got: [ReviewAction, string, boolean][] = []; + const confirms: string[] = []; + win.confirm = (m?: string) => { + confirms.push(m ?? ""); + return confirms.length !== 2; + }; + const root = prView(detail(), undefined, render, { + review: async (action, text, draft) => { + got.push([action, text, draft]); + return "https://github.com/r/1"; + }, + }, { reviewedHere: false }); + win.document.body.replaceChildren(root); + const text = root.querySelector("form.review textarea") as HTMLTextAreaElement; + const draft = root.querySelector("#review-draft") as HTMLInputElement; + const click = (action: string) => (root.querySelector(`button[data-action="${action}"]`) as HTMLButtonElement).click(); + const tick = () => new Promise((r) => setTimeout(r, 0)); + + text.value = "LGTM"; + draft.checked = true; + click("approve"); + await tick(); + click("comment"); // refused at the confirm + await tick(); + text.value = "nit"; + click("request-changes"); + await tick(); + assert.deepEqual(got, [["approve", "LGTM", true], ["request-changes", "nit", false]]); + assert.match(confirms[0] ?? "", /^Approve \(with \/draft\) cgwalters-forge\/widget#7 at eeeeeeeeee\? Not seen here: 1 without a diff here\.$/); + assert.match(confirms[2] ?? "", /You already reviewed it from here/); + assert.match(root.querySelector("form.review .status")?.textContent ?? "", /^Sent: /); + }); + + it("shows a refusal and lets him retry", async () => { + win.confirm = () => true; + const root = prView(detail(), undefined, render, { review: async () => Promise.reject(new Error("the PR's head moved")) }, { reviewedHere: false }); + (root.querySelector('button[data-action="approve"]') as HTMLButtonElement).click(); + await new Promise((r) => setTimeout(r, 0)); + assert.match(root.querySelector("form.review .status")?.textContent ?? "", /Not sent: the PR's head moved/); + assert.equal((root.querySelector('button[data-action="approve"]') as HTMLButtonElement).disabled, false); + }); + + it("names unexpanded files and truncation in the approval's confirmation", () => { + const confirms: string[] = []; + win.confirm = (m?: string) => (confirms.push(m ?? ""), false); + const patch = `@@ -1,0 +1,400 @@\n${Array.from({ length: 400 }, (_, i) => `+l${i}`).join("\n")}`; + const d = detail({ changedFiles: 4, files: [{ filename: "big.rs", status: "added", additions: 400, deletions: 0, patch }, ...detail().files] }); + const root = prView(d, undefined, render, noReview, { reviewedHere: false }); + (root.querySelector('button[data-action="approve"]') as HTMLButtonElement).click(); + assert.match(confirms[0] ?? "", /Not seen here: 1 file never expanded, 1 without a diff here, files or commits beyond what GitHub lists\.$/); + }); + + it("won't approve a diff that may not be the head's", () => { + const root = prView(detail({ consistent: false }), undefined, render, noReview, { reviewedHere: false }); + assert.equal((root.querySelector('button[data-action="approve"]') as HTMLButtonElement).disabled, true); + assert.equal((root.querySelector('button[data-action="comment"]') as HTMLButtonElement).disabled, false); + assert.match(root.querySelector("form.review .status")?.textContent ?? "", /Approve is off until a reload/); + }); + + it("offers a review form only for the bot's PRs in its own space", () => { + const cases: [Partial, boolean][] = [ + [{}, true], + [{ ref: { owner: "cgwalters-bot", repo: "sandbox", number: 1 } }, true], + [{ ref: { owner: "bootc-dev", repo: "bootc", number: 1 } }, false], + [{ author: "someone" }, false], + ]; + for (const [over, want] of cases) { + const root = prView(detail(over), undefined, render, noReview, { reviewedHere: false }); + assert.equal(root.querySelector("form.review") !== null, want, JSON.stringify(over)); + assert.equal(canReview(detail(over)), want); + } + }); + + it("offers no review form on a closed PR", () => { + const root = prView(detail({ state: "merged" }), undefined, render, noReview, { reviewedHere: false }); + assert.equal(root.querySelector("form.review"), null); + assert.match(root.textContent ?? "", /This PR is merged/); + }); +}); diff --git a/test/queue.test.ts b/test/queue.test.ts new file mode 100644 index 0000000..640f286 --- /dev/null +++ b/test/queue.test.ts @@ -0,0 +1,123 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import type { Item } from "../src/github/board.ts"; +import type { ForgePr, Verdict } from "../src/github/forge.ts"; +import { buildEntries, type Entry, groupRanked, priorityRank, rankEntries } from "../src/github/queue.ts"; + +function item(nodeId: string, over: Partial = {}): Item { + return { id: 0, nodeId, kind: "draft", title: nodeId, body: "", why: "", branch: [], gist: [], status: "Needs human", ...over }; +} + +function pr(owner: string, repo: string, number: number, over: Partial = {}): ForgePr { + return { + ref: { owner, repo, number }, + url: `https://github.com/${owner}/${repo}/pull/${number}`, + title: `${repo} #${number}`, + body: "", + author: "cgwalters-bot", + createdAt: "2026-01-10T00:00:00Z", + updatedAt: "2026-01-11T00:00:00Z", + draft: true, + ...over, + }; +} + +const meta = (item: string) => + `Text\n\n\n---\n- Upstream: \`up/r\`, base \`main\`\n- Board item: \`${item}\`\n`; + +describe("priorityRank", () => { + const cases: [string | undefined, number][] = [["P0", 0], ["P2", 2], ["P3", 3], ["P9", 4], [undefined, 5]]; + for (const [p, want] of cases) it(String(p), () => assert.equal(priorityRank(p), want)); +}); + +describe("rankEntries", () => { + it("puts P0 first, then the oldest, undated last, then by key", () => { + const e = (key: string, priority?: string, since?: string): Entry => ({ + key, kind: "chore", title: key, where: "", href: "#", + ...(priority ? { priority } : {}), + ...(since ? { since } : {}), + }); + const ranked = rankEntries([ + e("new-p1", "P1", "2026-03-01T00:00:00Z"), + e("none", undefined, "2020-01-01T00:00:00Z"), + e("old-p1", "P1", "2026-01-01T00:00:00Z"), + e("undated-p1", "P1"), + e("bad-date-p1", "P1", "yesterday"), + e("p0", "P0", "2026-06-01T00:00:00Z"), + e("odd", "P7", "2026-01-01T00:00:00Z"), + ]); + assert.deepEqual(ranked.map((x) => x.key), ["p0", "old-p1", "new-p1", "bad-date-p1", "undated-p1", "odd", "none"]); + assert.deepEqual(groupRanked(ranked).map((g) => [g.priority, g.entries.length]), [["P0", 1], ["P1", 4], ["P7", 1], ["No priority", 1]]); + }); +}); + +describe("buildEntries", () => { + const verdicts = (pairs: [string, Verdict["state"]][]) => new Map(pairs.map(([k, state]) => [k, { state }])); + + it("merges board items and forge PRs into one ranked list", () => { + const items = [ + item("PVTI_q", { why: "Q#2: which? Options: A) this B) that", priority: "P1", createdAt: "2026-01-05T00:00:00Z" }), + item("PVTI_chore", { why: "Please rerun the job", priority: "P0", createdAt: "2026-02-01T00:00:00Z" }), + item("PVTI_trackmeta", { status: "Draft", priority: "P0", branch: [] }), + item("PVTI_trackbranch", { status: "Draft", priority: "P2", branch: ["https://github.com/cgwalters-forge/b/pull/2"] }), + item("PVTI_stale", { status: "Draft", priority: "P0", branch: ["https://github.com/cgwalters-forge/c/pull/9"] }), + item("PVTI_gist", { status: "Draft", priority: "P1", gist: ["https://gist.github.com/x/1"], createdAt: "2026-01-01T00:00:00Z" }), + item("PVTI_todo", { status: "Todo", priority: "P0" }), + ]; + const prs = [ + pr("cgwalters-forge", "a", 1, { body: meta("PVTI_trackmeta"), createdAt: "2026-01-20T00:00:00Z" }), + pr("cgwalters-forge", "b", 2), + pr("cgwalters-forge", "untracked", 3), + ]; + const entries = buildEntries(items, prs, verdicts([])); + assert.deepEqual( + entries.map((e) => [e.key, e.kind, e.priority ?? "-"]), + [ + ["pr:cgwalters-forge/a#1", "pr", "P0"], + ["item:PVTI_chore", "chore", "P0"], + ["item:PVTI_gist", "chore", "P1"], + ["item:PVTI_q", "question", "P1"], + ["pr:cgwalters-forge/b#2", "pr", "P2"], + ["pr:cgwalters-forge/untracked#3", "pr", "-"], + ], + ); + const first = entries[0]; + assert.equal(first?.href, "#pr/cgwalters-forge/a/1"); + assert.equal(first?.item?.nodeId, "PVTI_trackmeta"); + assert.equal(entries.find((e) => e.kind === "question")?.href, "#item/PVTI_q"); + }); + + it("keeps a PR while it waits on him, and drops it and its Draft item otherwise", () => { + const items = [item("PVTI_t", { status: "Draft", priority: "P0", branch: ["https://github.com/cgwalters-forge/a/pull/1"] })]; + const cases: [Verdict["state"], boolean][] = [ + ["none", true], + ["approved-older", true], + ["changes-requested-older", true], + ["approved", false], + ["changes-requested", false], + ["promoted", true], + ]; + for (const [state, listed] of cases) { + const entries = buildEntries(items, [pr("cgwalters-forge", "a", 1)], verdicts([["cgwalters-forge/a#1", state]])); + assert.deepEqual(entries.map((e) => e.key), listed ? ["pr:cgwalters-forge/a#1"] : [], state); + if (listed) assert.equal(entries[0]?.verdict?.state, state); + } + }); + + it("keeps a Needs human item about a forge PR as its own entry", () => { + const items = [item("PVTI_nh", { branch: ["https://github.com/cgwalters-forge/a/pull/1"], why: "Q#1: ok? Options: A) yes B) no" })]; + const keys = buildEntries(items, [pr("cgwalters-forge", "a", 1)], verdicts([])).map((e) => e.key); + assert.deepEqual(keys.sort(), ["item:PVTI_nh", "pr:cgwalters-forge/a#1"]); + }); + + it("drops no forge-only Draft item before the forge was read", () => { + const items = [item("PVTI_f", { status: "Draft", branch: ["https://github.com/cgwalters-forge/a/pull/1"] })]; + assert.deepEqual(buildEntries(items, [], verdicts([])).length, 0); + assert.deepEqual(buildEntries(items, [], verdicts([]), false).map((e) => [e.key, e.kind]), [["item:PVTI_f", "chore"]]); + }); + + it("keeps a Draft item whose Branch is not only forge PRs", () => { + const items = [item("PVTI_up", { status: "Draft", branch: ["https://github.com/up/r/compare/main...cgwalters-bot:bot/x"] })]; + assert.deepEqual(buildEntries(items, [], verdicts([])).map((e) => e.kind), ["chore"]); + }); +}); diff --git a/test/view.test.ts b/test/view.test.ts index 5d069b4..1e01fc0 100644 --- a/test/view.test.ts +++ b/test/view.test.ts @@ -7,7 +7,8 @@ import { answerTarget, type Item, questionOf, queueItems } from "../src/github/b import type { Context, ReceiptStatus } from "../src/github/backend.ts"; import { HOME_OWNERS } from "../src/github/config.ts"; import { createRenderer } from "../src/markdown.ts"; -import { describeTarget, itemView, queueView } from "../src/github/view.ts"; +import { buildEntries, type Entry } from "../src/github/queue.ts"; +import { age, answerState, describeTarget, itemView, queueView, STATE_LABEL } from "../src/github/view.ts"; import { installDom, rawItems } from "./helpers.ts"; const win = installDom(); @@ -40,15 +41,25 @@ function assertNoActiveContent(root: Element): void { } describe("queueView", () => { - it("groups by priority and shows untrusted text as text", () => { + const entriesOf = (items: Item[]) => buildEntries(items, [], new Map()); + const labels = (sent: Set, receipts: Map) => (e: Entry) => { + const st = e.item ? answerState(e.item, sent, receipts) : undefined; + return st ? { text: STATE_LABEL[st], cls: st } : undefined; + }; + + it("ranks by priority and shows untrusted text as text", () => { const items = [evilItem(), ...queueItems(rawItems()).slice(1)]; - const root = queueView(items, new Set(), new Map()); + const root = queueView(entriesOf(items), labels(new Set(), new Map()), Date.parse("2026-02-01T00:00:00Z")); assertNoActiveContent(root); assert.deepEqual( [...root.querySelectorAll(".group-h")].map((e) => e.textContent), ["P0 · 1", "P1 · 1", "P2 · 1", "No priority · 1"], ); assert.ok(root.textContent?.includes("")); + assert.deepEqual( + [...root.querySelectorAll(".row")].map((r) => r.getAttribute("href")), + ["#item/PVTI_synthetic_draft", "#item/PVTI_synthetic_upstream_pr", "#item/PVTI_synthetic_redacted", "#item/PVTI_synthetic_home_issue"], + ); }); it("trusts a draft's answer section only with a verified receipt", () => { @@ -56,21 +67,36 @@ describe("queueView", () => { const draft = items.find((i) => i.kind === "draft") as Item; draft.body += "\n\n/answer A\n\n"; const sent = new Set(["PVTI_synthetic_home_issue"]); - const labels = (receipts: Map) => - [...queueView(items, sent, receipts).querySelectorAll(".state")].map((e) => e.textContent); + const shown = (receipts: Map) => + [...queueView(entriesOf(items), labels(sent, receipts)).querySelectorAll(".state")].map((e) => e.textContent); - assert.deepEqual(labels(new Map()), ["body claims an answer (unverified)", "answered"]); + assert.deepEqual(shown(new Map()), ["body claims an answer (unverified)", "answered"]); const failed: ReceiptStatus = { url: "https://gist.github.com/abc", check: { ok: false, reason: "not his" } }; - assert.deepEqual(labels(new Map([[draft.nodeId, failed]])), ["body claims an answer (unverified)", "answered"]); + assert.deepEqual(shown(new Map([[draft.nodeId, failed]])), ["body claims an answer (unverified)", "answered"]); const good: ReceiptStatus = { url: "https://gist.github.com/abc", check: { ok: true, receipt: { choice: "A", text: "", item: draft.nodeId } } }; - assert.deepEqual(labels(new Map([[draft.nodeId, good]])), ["answered", "answered"]); + assert.deepEqual(shown(new Map([[draft.nodeId, good]])), ["answered", "answered"]); }); it("says when nothing needs you", () => { - assert.match(queueView([], new Set(), new Map()).textContent ?? "", /Nothing needs you/); + assert.match(queueView([], () => undefined).textContent ?? "", /Nothing needs you/); }); }); +describe("age", () => { + const now = Date.parse("2026-01-15T12:00:00Z"); + const cases: [string | undefined, string][] = [ + [undefined, ""], + ["not a date", ""], + ["2026-01-15T11:59:50Z", "now"], + ["2026-01-15T11:15:00Z", "45m"], + ["2026-01-15T02:00:00Z", "10h"], + ["2026-01-12T12:00:00Z", "3d"], + ["2025-12-01T12:00:00Z", "6w"], + ["2026-01-16T00:00:00Z", "now"], + ]; + for (const [iso, want] of cases) it(String(iso), () => assert.equal(age(iso, now), want)); +}); + describe("itemView", () => { const ctx: Context = { isPrivate: false, From 6be937d8684e906830de3819721044850a346454 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 26 Sep 2026 08:01:08 -0400 Subject: [PATCH 6/6] github: Add a light, dark or automatic theme switch The page follows the system's color scheme; the switch lets him pick one for this browser (kept in localStorage, a convenience that may be blocked). The dark palette now applies when chosen or when the system prefers it and light wasn't chosen. Generated-by: AI --- src/github/config.ts | 3 +++ src/github/main.ts | 32 ++++++++++++++++++++++++++++++++ static/index.html | 1 + static/style.css | 12 +++++++++++- 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/github/config.ts b/src/github/config.ts index c3c34c9..513a834 100644 --- a/src/github/config.ts +++ b/src/github/config.ts @@ -61,6 +61,9 @@ export const PAGE_SIZE = 100; export const RECENT_COMMENTS = 5; +/** localStorage key for the chosen theme (auto, light, dark). */ +export const THEME_KEY = "review.theme"; + /** Storage key for the pasted token (sessionStorage, or localStorage if remembered). */ export const TOKEN_KEY = "review.token"; diff --git a/src/github/main.ts b/src/github/main.ts index be059c3..99b1a5a 100644 --- a/src/github/main.ts +++ b/src/github/main.ts @@ -16,6 +16,7 @@ import { POLL_BACKOFF_FACTOR, POLL_INTERVAL_MS, RATE_LOW_FRACTION, + THEME_KEY, } from "./config.ts"; import { composeReview, type ForgePr, refKey, VERDICT_LABEL } from "./forge.ts"; import { type Command, HELP, keyCommand, parseRoute, type Route, type RouteInfo } from "./keys.ts"; @@ -494,6 +495,36 @@ function installKeys(state: State): void { }); } +type Theme = "auto" | "light" | "dark"; +const THEMES: readonly Theme[] = ["auto", "light", "dark"]; + +function applyTheme(theme: Theme): void { + if (theme === "auto") delete document.documentElement.dataset.theme; + else document.documentElement.dataset.theme = theme; + byId("theme").textContent = `Theme: ${theme}`; +} + +/** A per-browser convenience; storage may be blocked. */ +function installTheme(): void { + let theme: Theme = "auto"; + try { + const saved = window.localStorage.getItem(THEME_KEY); + if (saved && (THEMES as readonly string[]).includes(saved)) theme = saved as Theme; + } catch { + // Default theme. + } + applyTheme(theme); + byId("theme").onclick = () => { + theme = THEMES[(THEMES.indexOf(theme) + 1) % THEMES.length] ?? "auto"; + applyTheme(theme); + try { + window.localStorage.setItem(THEME_KEY, theme); + } catch { + // Not remembered. + } + }; +} + /** Which token problem to explain on the sign-in page. */ type SignInReason = "none" | "rejected"; @@ -639,6 +670,7 @@ function trackHeaderHeight(): void { } async function main(): Promise { + installTheme(); trackHeaderHeight(); // A CSP meta tag can't forbid framing, so refuse to run in a frame: // otherwise another site could overlay the approve and send buttons. diff --git a/static/index.html b/static/index.html index 586749a..f02d93a 100644 --- a/static/index.html +++ b/static/index.html @@ -17,6 +17,7 @@

Needs you

Starting… +
diff --git a/static/style.css b/static/style.css index 411a21f..0773968 100644 --- a/static/style.css +++ b/static/style.css @@ -11,8 +11,9 @@ --sans: system-ui, -apple-system, "Segoe UI", sans-serif; --mono: ui-monospace, SFMono-Regular, Menlo, monospace; } +/* Dark by preference unless light was picked, or when dark was picked. */ @media (prefers-color-scheme: dark) { - :root { + :root:not([data-theme="light"]) { color-scheme: dark; --ground: #0e131a; --surface: #161d27; --ink: #e4e9f0; --muted: #95a1b3; --line: #29333f; --accent: #86a6ff; --accent-soft: #1c2842; @@ -22,6 +23,15 @@ --add: #12291b; --add-ink: #8fdcab; --del: #32151a; --del-ink: #ff9ea4; --hunk: #19223a; } } +:root[data-theme="dark"] { + color-scheme: dark; + --ground: #0e131a; --surface: #161d27; --ink: #e4e9f0; --muted: #95a1b3; --line: #29333f; + --accent: #86a6ff; --accent-soft: #1c2842; + --p0: #ff7070; --p0-soft: #3d1414; --p1: #ff8a63; --p1-soft: #3a1d14; + --p2: #f0b85a; --p2-soft: #33270f; --p3: #9aa6b8; --p3-soft: #1f2631; + --ok: #6fcf97; --ok-soft: #13291d; + --add: #12291b; --add-ink: #8fdcab; --del: #32151a; --del-ink: #ff9ea4; --hunk: #19223a; +} * { box-sizing: border-box; } body { margin: 0; background: var(--ground); color: var(--ink); font: 15px/1.5 var(--sans); padding: 0 16px 48px; } .wrap { max-width: 900px; margin: 0 auto; }