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/api.ts b/src/github/api.ts index a64351d..7275482 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. } @@ -84,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/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..70ae836 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,35 +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); -} - -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 })); + .filter((i) => i.status !== undefined && QUEUE_STATUSES.includes(i.status)); } /** The question an item asks, as the bot wrote it. */ diff --git a/src/github/config.ts b/src/github/config.ts index 025321b..513a834 100644 --- a/src/github/config.ts +++ b/src/github/config.ts @@ -11,8 +11,17 @@ 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]; + +/** 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 = { @@ -33,6 +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. */ @@ -43,6 +60,10 @@ export const PAGE_SIZE = 100; /** Comments shown in the item view. */ 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/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/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..99b1a5a 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,21 @@ 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, + THEME_KEY, } 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 +34,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 +94,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 +106,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 +120,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 +206,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 +289,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 +377,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 +407,124 @@ 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); + }); +} + +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"; @@ -190,9 +535,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 +572,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 +660,18 @@ 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 { + 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. if (window.top !== window.self) { 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/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..f02d93a 100644 --- a/static/index.html +++ b/static/index.html @@ -17,7 +17,8 @@

Needs you

Starting… - + +
diff --git a/static/style.css b/static/style.css index 62bcbca..0773968 100644 --- a/static/style.css +++ b/static/style.css @@ -7,19 +7,31 @@ --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; } +/* 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; --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; } } +: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; } @@ -44,7 +56,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 +87,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/api.test.ts b/test/api.test.ts index 4e3eb01..69f4734 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/); @@ -57,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); 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..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"; @@ -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"], @@ -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/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/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,