Public: anyone can read this; the crew can change it.
The terminal pull screen shows which named team still owes a review #2897
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/2885-the-terminal-pull-screen-shows-which-nam → main 3 commits fork 3 threads · 3 unresolved
3 files
changed +191
−2
README.md +1 −1
Diff not shown — a line 2,529 characters wide (limit 2,000). +1 −1
View filesrc/tui/screens/pull.ts +37 −1
| @@ -47,6 +47,7 @@ import { | ||
| 47 | 47 | type EnvStatus, envsFor } from "../../apps/runtime.ts"; |
| 48 | 48 | import { proposedByText } from "../../core/proposal_token.ts"; |
| 49 | 49 | import { getAppByRepo } from "../../core/apps.ts"; |
| 50 | import { teamVerdicts, type TeamVerdict } from "../../core/issue_teams.ts"; | |
| 50 | 51 | import { getIssue, type Issue } from "../../core/issues.ts"; |
| 51 | 52 | import { isProposed, pullMayRun } from "../../core/pull_trust.ts"; |
| 52 | 53 | import { runningProgress, stepsLine } from "../../checks/progress.ts"; |
| @@ -193,6 +194,13 @@ const VERDICT_WORDS: Record<ReviewFact["state"], string> = { | ||
| 193 | 194 | commented: "commented", |
| 194 | 195 | }; |
| 195 | 196 | |
| 197 | /** `Design: approved by quill` — one row per team the issue names, or `awaited`. */ | |
| 198 | export function teamLine(v: TeamVerdict): string { | |
| 199 | if (v.state === undefined) return `${v.name}: awaited`; | |
| 200 | const words = VERDICT_WORDS[v.state] ?? v.state; | |
| 201 | return `${v.name}: ${words} by ${v.by ?? ""}${v.stale ? " (on an older head)" : ""}`; | |
The stale suffix and the changes-requested wording are untested; the test only covers approved and awaited. Add cases for both. |
||
| 202 | } | |
| 203 | ||
| 196 | 204 | /** How often a pull's screen re-reads while its check is queued or running. */ |
| 197 | 205 | const CHECK_TICK_MS = 5000; |
| 198 | 206 | |
| @@ -208,6 +216,8 @@ export function pullScreen( | ||
| 208 | 216 | let diff: Diff = { files: [], additions: 0, deletions: 0 }; |
| 209 | 217 | let reviews: AnchoredReview[] = []; |
| 210 | 218 | let threads: Thread[] = []; |
| 219 | /** Where each team the issue names stands; empty when it names none. */ | |
| 220 | let teams: TeamVerdict[] = []; | |
| 211 | 221 | /** Settled threads the reader has opened back up, for this visit only. */ |
| 212 | 222 | const expanded = new Set<string>(); |
| 213 | 223 | let lines: Span[][] = []; |
| @@ -298,6 +308,13 @@ export function pullScreen( | ||
| 298 | 308 | check, |
| 299 | 309 | mayReadRunLog(f.app, f.user, settings), |
| 300 | 310 | ); |
| 311 | teams = | |
| 312 | row === undefined | |
| 313 | ? [] | |
| 314 | : teamVerdicts(f.db, { | |
| 315 | ...row, | |
| 316 | headOid: row.headOid || (facts?.headOid ?? ""), | |
| 317 | }); | |
| 301 | 318 | checkOld = |
| 302 | 319 | repo !== undefined && |
| 303 | 320 | row !== undefined && |
| @@ -804,7 +821,8 @@ export function pullScreen( | ||
| 804 | 821 | const boxH = |
| 805 | 822 | 3 + |
| 806 | 823 | (standingText(standing(), requireReview) === "" ? 0 : 1) + |
| 807 | (checksLine === "" ? 0 : 1); | |
| 824 | (checksLine === "" ? 0 : 1) + | |
| 825 | teams.length; | |
| 808 | 826 | const [strip, head, previewRow, boxRow, tabRow, body, comp] = split( |
| 809 | 827 | area, |
| 810 | 828 | "v", |
| @@ -861,6 +879,7 @@ export function pullScreen( | ||
| 861 | 879 | checksLine, |
| 862 | 880 | standing: standing(), |
| 863 | 881 | required: requireReview, |
| 882 | teams, | |
| 864 | 883 | }); |
| 865 | 884 | tabs(s, tabRow!, c, { |
| 866 | 885 | items: [ |
| @@ -1378,6 +1397,7 @@ function drawMergeBox( | ||
| 1378 | 1397 | checksLine: string; |
| 1379 | 1398 | standing: Standing; |
| 1380 | 1399 | required: boolean; |
| 1400 | teams: readonly TeamVerdict[]; | |
| 1381 | 1401 | }, |
| 1382 | 1402 | ): void { |
| 1383 | 1403 | const state = mergeState(issue, pull, mirrored, o.gated, o.checksGated); |
| @@ -1477,5 +1497,21 @@ function drawMergeBox( | ||
| 1477 | 1497 | ), |
| 1478 | 1498 | inner.x + inner.w, |
| 1479 | 1499 | ); |
| 1500 | y++; | |
| 1501 | } | |
| 1502 | // The teams the issue names, one row each: who still owes a review. | |
| 1503 | for (const v of o.teams) { | |
| 1504 | if (y >= inner.y + inner.h) break; | |
| 1505 | s.text( | |
| 1506 | inner.x, | |
| 1507 | y, | |
| 1508 | truncate(teamLine(v), inner.w), | |
| 1509 | style( | |
| 1510 | v.state === "changes_requested" ? c.danger : c.mutedFg, | |
| 1511 | c.card, | |
| 1512 | ), | |
| 1513 | inner.x + inner.w, | |
| 1514 | ); | |
| 1515 | y++; | |
| 1480 | 1516 | } |
| 1481 | 1517 | } |
test/pull-tui-teams.test.ts added +153
| @@ -0,0 +1,153 @@ | ||
| 1 | /** | |
| 2 | * The terminal's pull screen names the team that still owes a review (#2897), | |
outdated
Wrong issue number: this pull is #2897. |
||
| 3 | * as the web pull page's team card does. | |
| 4 | */ | |
| 5 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 6 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 7 | import { tmpdir } from "node:os"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; | |
| 10 | import { nameTeams } from "../src/actions/issue_teams.ts"; | |
| 11 | import { openIssue } from "../src/actions/issues.ts"; | |
| 12 | import { openPull } from "../src/actions/pulls.ts"; | |
| 13 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 14 | import { createApp, type App } from "../src/app.ts"; | |
| 15 | import { setCrewRole } from "../src/core/agents.ts"; | |
| 16 | import { getPull } from "../src/core/pulls.ts"; | |
| 17 | import { submitReview } from "../src/core/reviews.ts"; | |
| 18 | import { saveTeam } from "../src/core/teams.ts"; | |
| 19 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; | |
| 20 | import { createForgeApp } from "../src/tui/forge.ts"; | |
| 21 | import { pullScreen } from "../src/tui/screens/pull.ts"; | |
| 22 | import { local } from "../src/tui/screens/where.ts"; | |
| 23 | import { headlessTerminal } from "../src/tui/term/terminal.ts"; | |
| 24 | import { quietLogs } from "./helpers/globals.ts"; | |
| 25 | import { settleApp } from "./helpers/tui.ts"; | |
| 26 | ||
| 27 | quietLogs(); | |
| 28 | ||
| 29 | let dataDir: string; | |
| 30 | let app: App; | |
| 31 | ||
| 32 | beforeAll(async () => { | |
| 33 | dataDir = await mkdtemp(join(tmpdir(), "forge-tui-teams-")); | |
| 34 | app = createApp({ dataDir, port: 0, host: "forge.test" }); | |
| 35 | }); | |
| 36 | ||
| 37 | afterAll(async () => { | |
| 38 | app.close(); | |
| 39 | await rm(dataDir, { recursive: true, force: true }); | |
| 40 | }); | |
| 41 | ||
| 42 | test("a two-team issue shows one team approved and one awaited; no team shows nothing", async () => { | |
| 43 | const made = await createUser(app.db, { | |
| 44 | origin: "seed", | |
| 45 | trusted: true, | |
| 46 | username: "alice", | |
| 47 | email: "[email protected]", | |
| 48 | password: "a-good-passphrase", | |
| 49 | adminIfFirst: true, | |
| 50 | }); | |
| 51 | if (!made.ok) throw new Error(String(made.error)); | |
| 52 | const alice: User = made.value; | |
| 53 | const r = await createRepo(app, alice, { name: "lib", visibility: "public" }); | |
| 54 | if (!r.ok) throw new Error(String(r.error)); | |
| 55 | const repo = r.value; | |
| 56 | const id = (name: string) => getUserByName(app.db, name)!.id; | |
| 57 | for (const name of ["quill", "pike"]) { | |
| 58 | const agent = await createAgent(app, alice, { username: name }); | |
| 59 | if (!agent.ok) throw new Error(String(agent.error)); | |
| 60 | addToCrew(app, alice, repo, name); | |
| 61 | setCrewRole(app.db, repo.id, id(name), "reviewer"); | |
| 62 | } | |
| 63 | saveTeam(app.db, "design", { charter: "", dial: 0, members: [id("quill")] }); | |
| 64 | saveTeam(app.db, "data-science", { charter: "", dial: 0, members: [id("pike")] }); | |
| 65 | const seed = await commitToBranch(app, alice, repo, { | |
| 66 | branch: "main", | |
| 67 | message: "main", | |
| 68 | files: [{ path: "code.txt", content: "main" }], | |
| 69 | }); | |
| 70 | if (!seed.ok) throw new Error(JSON.stringify(seed.error)); | |
| 71 | ||
| 72 | let n = 0; | |
| 73 | const pullNaming = async (teams: string[]) => { | |
| 74 | const issue = openIssue(app, alice, repo, { title: "Chart" }); | |
| 75 | if (!issue.ok) throw new Error(issue.error); | |
| 76 | const set = nameTeams(app, alice, repo, issue.value, teams); | |
| 77 | if (!set.ok) throw new Error(set.error); | |
| 78 | const branch = `b${++n}`; | |
| 79 | const c = await commitToBranch(app, alice, repo, { | |
| 80 | branch, | |
| 81 | base: "main", | |
| 82 | message: branch, | |
| 83 | files: [{ path: `${branch}.md`, content: "x\n" }], | |
| 84 | }); | |
| 85 | if (!c.ok) throw new Error(JSON.stringify(c.error)); | |
| 86 | const o = await openPull(app, alice, repo, { | |
| 87 | title: `Pull ${branch}`, | |
| 88 | body: `Closes #${issue.value.number}`, | |
| 89 | baseBranch: "main", | |
| 90 | headBranch: branch, | |
| 91 | }); | |
| 92 | if (!o.ok) throw new Error(String(o.error)); | |
| 93 | return getPull(app.db, repo.id, o.value.number)!; | |
| 94 | }; | |
| 95 | ||
| 96 | const { app: tui, session } = createForgeApp( | |
| 97 | app, | |
| 98 | { | |
| 99 | sshUsername: "alice", | |
| 100 | remoteAddress: "127.0.0.1", | |
| 101 | fingerprint: "SHA256:alice", | |
| 102 | user: alice, | |
| 103 | }, | |
| 104 | { terminal: headlessTerminal(160, 44), exitHooks: false }, | |
| 105 | ); | |
| 106 | tui.start(); | |
| 107 | const show = async (number: number): Promise<string> => { | |
| 108 | tui.ctx.push(pullScreen(session, tui.ctx, local(session, repo), number)); | |
| 109 | await settleApp(tui); | |
| 110 | return tui.screen.toText(); | |
| 111 | }; | |
| 112 | try { | |
| 113 | const two = await pullNaming(["design", "data-science"]); | |
| 114 | submitReview(app.db, { | |
| 115 | issueId: two.id, | |
| 116 | authorId: id("quill"), | |
| 117 | state: "approved", | |
| 118 | commitOid: two.headOid, | |
| 119 | }); | |
| 120 | const text = await show(two.number); | |
| 121 | expect(text).toContain("Design: approved by quill"); | |
| 122 | expect(text).toContain("Data science: awaited"); | |
| 123 | ||
| 124 | submitReview(app.db, { | |
| 125 | issueId: two.id, | |
| 126 | authorId: id("pike"), | |
| 127 | state: "changes_requested", | |
| 128 | commitOid: two.headOid, | |
| 129 | }); | |
| 130 | expect(await show(two.number)).toContain( | |
| 131 | "Data science: changes requested by pike", | |
| 132 | ); | |
| 133 | ||
| 134 | // A verdict passed on a head the pull has since left is said to be old. | |
| 135 | const moved = await pullNaming(["design"]); | |
| 136 | submitReview(app.db, { | |
| 137 | issueId: moved.id, | |
| 138 | authorId: id("quill"), | |
| 139 | state: "approved", | |
| 140 | commitOid: "0".repeat(40), | |
| 141 | }); | |
| 142 | expect(await show(moved.number)).toContain( | |
| 143 | "Design: approved by quill (on an older head)", | |
| 144 | ); | |
| 145 | ||
| 146 | const none = await pullNaming([]); | |
| 147 | const bare = await show(none.number); | |
| 148 | expect(bare).not.toContain("awaited"); | |
| 149 | expect(bare).not.toContain("approved by"); | |
| 150 | } finally { | |
| 151 | tui.stop(); | |
| 152 | } | |
| 153 | }, 60_000); |
new above
offline
Comment example does not match the output; the row is
<Team name>: approved by <who>, with noReviewers:prefix.