Public: anyone can read this; the crew can change it.
The terminal's pull screen lists the pull's looks, each with its width, line and a link to open it #8465
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/7917-the-terminal-s-pull-screen-lists-the-pul → main 2 commits fork 3 threads · 3 unresolved
3 files
changed +199
−2
docs/features/pull-looks.md +1 −1
| @@ -11,7 +11,7 @@ One list, one rule (`pullLooks` in `src/looks/readable.ts`), three doors: | ||
| 11 | 11 | `{ items: [{ id, width, says, by, commit, url, at }] }`, newest first. `url` |
| 12 | 12 | is the same one the POST answers. |
| 13 | 13 | - **MCP** — the `pull` tool carries `looks`, the same items (no new tool). |
| 14 | - **Terminal** — `I` on a pull opens its looks. | |
| 14 | - **Terminal** — `I` on a pull opens its looks; the pull screen's conversation also lists them under a `Looks` heading (none, no heading), each with its width, who, the line it says and the absolute URL on its own line. It is the same `pullLooks` read as the API and MCP, so a reader sees only what they may fetch. | |
| 15 | 15 | |
| 16 | 16 | Who sees what is `mayFetchLook`, the rule the pictures themselves use: a |
| 17 | 17 | reader who may not read the repository gets 404; a writer sees every look |
src/tui/screens/pull.ts +53 −1
| @@ -46,6 +46,7 @@ import { revertPull } from "../../actions/revert.ts"; | ||
| 46 | 46 | import { takeBackWords } from "../../actions/revert_words.ts"; |
| 47 | 47 | import { openEnvUrl } from "../../actions/apps.ts"; |
| 48 | 48 | import { listRepoRuns, mayReadRunLog } from "../../actions/runs.ts"; |
| 49 | import { pullLooks, type PullLook } from "../../looks/readable.ts"; | |
| 49 | 50 | import { pullLooksScreen } from "./pulllooks.ts"; |
| 50 | 51 | import { previewEnv } from "../../apps/data.ts"; |
| 51 | 52 | import { findingsWords, pullPreviewFindings } from "../../core/preview_findings.ts"; |
| @@ -256,6 +257,8 @@ export function pullScreen( | ||
| 256 | 257 | /** Whether the repository is an app, and its preview of this pull if so. */ |
| 257 | 258 | let isApp = false; |
| 258 | 259 | let preview: EnvStatus | undefined; |
| 260 | /** The pictures taken of the preview, as the API would list them to this reader. */ | |
| 261 | let looks: PullLook[] = []; | |
| 259 | 262 | /** The deploy of the *current* head — what the merge box calls the checks. */ |
| 260 | 263 | let check: CheckSummary = NO_CHECK; |
| 261 | 264 | /** The pull's base branch, to name what the check was merged with. */ |
| @@ -321,6 +324,8 @@ export function pullScreen( | ||
| 321 | 324 | // the checks at the live head (a row), and what the crew has done here. |
| 322 | 325 | const record = repo === undefined ? undefined : getAppByRepo(f.db, repo.id); |
| 323 | 326 | isApp = record !== undefined; |
| 327 | looks = | |
| 328 | repo === undefined || row === undefined ? [] : pullLooks(f.app, repo, row, f.user); | |
| 324 | 329 | preview = |
| 325 | 330 | record === undefined |
| 326 | 331 | ? undefined |
| @@ -1113,7 +1118,10 @@ export function pullScreen( | ||
| 1113 | 1118 | } else { |
| 1114 | 1119 | if (width !== body!.w) { |
| 1115 | 1120 | width = body!.w; |
| 1116 | lines = threadLines(entries(), Math.max(20, body!.w - 1), c); | |
| 1121 | lines = [ | |
| 1122 | ...threadLines(entries(), Math.max(20, body!.w - 1), c), | |
| 1123 | ...looksLines(looks, lookOrigin(f.app.config), Math.max(20, body!.w - 1), c), | |
| 1124 | ]; | |
| 1117 | 1125 | } |
| 1118 | 1126 | drawLines(s, body!, c, scroll, lines); |
| 1119 | 1127 | hitScroll(ctx, body!, scroll, lines.length); |
| @@ -1797,3 +1805,47 @@ function drawMergeBox( | ||
| 1797 | 1805 | y++; |
| 1798 | 1806 | } |
| 1799 | 1807 | } |
| 1808 | ||
| 1809 | const lookOrigin = (config: { | |
| 1810 | url?: string | undefined; | |
| 1811 | host?: string | undefined; | |
| 1812 | port?: number | undefined; | |
| 1813 | }): string => | |
| 1814 | (config.url ?? `http://${config.host ?? "localhost"}:${config.port ?? 3000}`).replace( | |
| 1815 | /\/+$/, | |
| 1816 | "", | |
| 1817 | ); | |
| 1818 | ||
| 1819 | /** | |
| 1820 | * The Looks section under the conversation: one line each, absolute URL so a | |
| 1821 | * terminal can open it. Nothing at all when the pull has none. | |
| 1822 | */ | |
| 1823 | function looksLines( | |
| 1824 | items: readonly PullLook[], | |
| 1825 | origin: string, | |
| 1826 | width: number, | |
| 1827 | c: Tokens, | |
| 1828 | ): Span[][] { | |
| 1829 | if (items.length === 0) return []; | |
| 1830 | const clean = (t: string): string => t.replace(/[\p{Cc}\p{Cf}\u2028\u2029]/gu, " "); | |
| 1831 | const room = Math.max(10, width - 4); | |
| 1832 | return [ | |
| 1833 | [], | |
| 1834 | [{ text: ` Looks (${items.length})`, style: style(c.fg, c.bg, BOLD) }], | |
outdated
The absolute URL is appended to the same line as width, author and note; on a narrow terminal it is clipped. Give the URL its own line. |
||
| 1835 | ...items.flatMap((l): Span[][] => { | |
| 1836 | // The line may be cut to fit; the link never is — it takes its own | |
| 1837 | // indented line and wraps only when it is wider than the screen. | |
| 1838 | const says = truncate( | |
| 1839 | clean(`${l.width} · ${l.by ?? "forge"} · ${l.says ?? "taken by the forge"}`), | |
| 1840 | room, | |
| 1841 | ); | |
| 1842 | const url = `${origin}${l.url}`; | |
| 1843 | const parts: string[] = []; | |
| 1844 | for (let i = 0; i < url.length; i += room) parts.push(url.slice(i, i + room)); | |
| 1845 | return [ | |
| 1846 | [{ text: ` ${says}`, style: style(c.mutedFg, c.bg) }], | |
| 1847 | ...parts.map((part): Span[] => [{ text: ` ${part}`, style: style(c.fg, c.bg) }]), | |
| 1848 | ]; | |
| 1849 | }), | |
| 1850 | ]; | |
| 1851 | } |
test/tui-pull-looks-section.test.ts added +145
| @@ -0,0 +1,145 @@ | ||
| 1 | /** The terminal's pull screen lists the pull's looks with a full URL; none, no heading (#7917). */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { openPull } from "../src/actions/pulls.ts"; | |
| 7 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 8 | import { createApp, type App as Forge } from "../src/app.ts"; | |
| 9 | import { previewEnv } from "../src/apps/data.ts"; | |
| 10 | import { createApp as insertApp } from "../src/core/apps.ts"; | |
| 11 | import { pullLooks } from "../src/looks/readable.ts"; | |
| 12 | import { recordLook, type RecordLookInput } from "../src/core/looks.ts"; | |
| 13 | import type { Pull } from "../src/core/pulls.ts"; | |
| 14 | import type { Repo } from "../src/core/repos.ts"; | |
| 15 | import { createUser, type User } from "../src/core/users.ts"; | |
| 16 | import { createForgeApp } from "../src/tui/forge.ts"; | |
| 17 | import { pullScreen } from "../src/tui/screens/pull.ts"; | |
| 18 | import { local } from "../src/tui/screens/where.ts"; | |
| 19 | import { headlessTerminal } from "../src/tui/term/terminal.ts"; | |
| 20 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 21 | import { quietLogs } from "./helpers/globals.ts"; | |
| 22 | import { settleApp } from "./helpers/tui.ts"; | |
| 23 | ||
| 24 | quietLogs(); | |
| 25 | ||
| 26 | let dataDir: string; | |
| 27 | let forge: Forge; | |
| 28 | let alice: User; | |
| 29 | let bob: User; | |
| 30 | const made: Record<string, { repo: Repo; pull: Pull }> = {}; | |
| 31 | ||
| 32 | beforeAll(async () => { | |
| 33 | dataDir = await mkdtemp(join(tmpdir(), "forge-tui-looks-")); | |
| 34 | forge = createApp({ dataDir, port: 0, url: "https://forge.test" }); | |
| 35 | const a = await seedAdmin(forge, { username: "alice", email: "[email protected]", password: "a-long-passphrase" }); | |
| 36 | if (!a.ok) throw new Error("signup failed"); | |
| 37 | alice = a.value; | |
| 38 | const b = await createUser(forge.db, { | |
| 39 | origin: "seed", | |
| 40 | trusted: true, | |
| 41 | username: "bob", | |
| 42 | email: "[email protected]", | |
| 43 | password: "a-long-passphrase", | |
| 44 | }); | |
| 45 | if (!b.ok) throw new Error(b.error); | |
| 46 | bob = b.value; | |
| 47 | for (const name of ["pictured", "plain", "hush", "ownerly"]) { | |
| 48 | const r = await createRepo(forge, alice, { name, visibility: "public" }); | |
| 49 | if (!r.ok) throw new Error("repo creation failed"); | |
| 50 | await commitToBranch(forge, alice, r.value, { message: "seed", files: [{ path: "README.md", content: "# s\n" }] }); | |
| 51 | await commitToBranch(forge, alice, r.value, { | |
| 52 | branch: "feature/one", | |
| 53 | base: "main", | |
| 54 | message: "One", | |
| 55 | files: [{ path: "one.ts", content: "export const one = 1;\n" }], | |
| 56 | }); | |
| 57 | const o = await openPull(forge, alice, r.value, { | |
outdated
Cast hides a type mismatch; pass a typed record. Also add a non-writer/guest case showing the look is not listed. |
||
| 58 | title: "Pull one", | |
| 59 | body: "", | |
| 60 | baseBranch: "main", | |
| 61 | headBranch: "feature/one", | |
| 62 | }); | |
| 63 | if (!o.ok) throw new Error(String(o.error)); | |
| 64 | made[name] = { repo: r.value, pull: o.value }; | |
| 65 | } | |
| 66 | const take = (name: string, access: "public" | "private", extra: Partial<RecordLookInput>) => { | |
| 67 | const inserted = insertApp(forge.db, { repoId: made[name]!.repo.id, slug: name, access }); | |
| 68 | if (!inserted.ok) throw new Error(inserted.error); | |
| 69 | recordLook(forge.db, { | |
| 70 | appId: inserted.value.id, | |
| 71 | env: previewEnv(made[name]!.pull.number), | |
| 72 | commitOid: "a".repeat(40), | |
| 73 | kind: "phone", | |
| 74 | path: "looks/x.png", | |
| 75 | takenAs: "nobody", | |
| 76 | note: "the empty state reads well", | |
| 77 | attachedBy: "alice", | |
| 78 | ...extra, | |
| 79 | }); | |
| 80 | }; | |
| 81 | take("pictured", "public", {}); | |
| 82 | take("hush", "private", {}); | |
| 83 | take("ownerly", "public", { takenAs: "owner" }); | |
| 84 | insertApp(forge.db, { repoId: made["plain"]!.repo.id, slug: "plain", access: "public" }); | |
| 85 | }); | |
| 86 | ||
| 87 | afterAll(async () => { | |
| 88 | forge.close(); | |
| 89 | await rm(dataDir, { recursive: true, force: true }); | |
| 90 | }); | |
| 91 | ||
| 92 | async function text(name: string, user: User = alice, cols = 160): Promise<string> { | |
| 93 | const { repo, pull } = made[name]!; | |
| 94 | const { app, session } = createForgeApp( | |
| 95 | forge, | |
| 96 | { sshUsername: "alice", remoteAddress: "127.0.0.1", user }, | |
| 97 | { terminal: headlessTerminal(cols, 50), exitHooks: false }, | |
| 98 | ); | |
| 99 | app.start(); | |
| 100 | await settleApp(app); | |
| 101 | app.push(pullScreen(session, app.ctx, local(session, repo), pull.number)); | |
| 102 | await settleApp(app); | |
| 103 | const out = app.screen.toText(); | |
| 104 | app.stop(); | |
| 105 | return out; | |
| 106 | } | |
| 107 | ||
| 108 | const fullUrl = (name: string) => { | |
| 109 | const { repo, pull } = made[name]!; | |
| 110 | const [look] = pullLooks(forge, repo, pull, alice); | |
| 111 | return `https://forge.test${look!.url}`; | |
| 112 | }; | |
| 113 | ||
| 114 | test("a pull with a look shows its width, line and full absolute URL", async () => { | |
| 115 | const out = await text("pictured"); | |
| 116 | expect(out).toContain("Looks (1)"); | |
| 117 | expect(out).toContain("phone · alice · the empty state reads well"); | |
| 118 | expect(out).toContain(fullUrl("pictured")); | |
| 119 | }); | |
| 120 | ||
| 121 | test("on a narrow terminal the line is cut and the whole URL still shows", async () => { | |
| 122 | const url = fullUrl("pictured"); | |
| 123 | const out = await text("pictured", alice, 50); | |
| 124 | // The link wraps rather than clips: rejoined, every character is there. | |
| 125 | const rows = out.split("\n").map((r) => r.trim()); | |
| 126 | const at = rows.findIndex((r) => url.startsWith(r) && r.startsWith("https://")); | |
| 127 | expect(at).toBeGreaterThan(-1); | |
| 128 | let joined = ""; | |
| 129 | for (let i = at; joined.length < url.length; i++) joined += rows[i]!.replace(/\s*[│|].*$/, ""); | |
| 130 | expect(joined.startsWith(url)).toBe(true); | |
| 131 | }); | |
| 132 | ||
| 133 | test("a pull without looks has no Looks heading", async () => { | |
| 134 | expect(await text("plain")).not.toContain("Looks ("); | |
| 135 | }); | |
| 136 | ||
| 137 | test("a reader who may not fetch a look does not see it listed", async () => { | |
| 138 | // A private app's look, to someone it does not name. | |
| 139 | expect(await text("hush", bob)).not.toContain("Looks ("); | |
| 140 | // An owner-taken look on a public app: the writers', not a reader's. | |
| 141 | expect(await text("ownerly", bob)).not.toContain("Looks ("); | |
| 142 | // The writer sees both. | |
| 143 | expect(await text("hush")).toContain("Looks (1)"); | |
| 144 | expect(await text("ownerly")).toContain("Looks (1)"); | |
| 145 | }); |
new above
offline
Non-blocking: check for an existing absolute-URL helper before keeping this config fallback copy.