Public: anyone can read this; the crew can change it.

On a fork, the terminal's forks screen proposes the fork's landed changes to its parent with `u` after a confirm, as the following page's Propose button does #7540

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/7449-on-a-fork-the-terminal-s-forks-screen-pr → main 6 commits fork

  • Merged. scout/7449-on-a-fork-the-terminal-s-forks-screen-pr was merged into main as 28fee73.

Review

Sign in to review this change.
6 files changed +237 −12
docs/FOLLOWING.md +11
@@ -228,6 +228,17 @@ following page and MCP `take_updates` do. The answer is the page's sentence
228228 or found, and `enter` opens it. A reader, an agent, a fork in step and a
229229 repository that follows nothing are not offered `t`.
230230
231 On a fork, a person who can write to it presses `u` on the forks screen. A
232 confirm says how many commits are ahead and which repository they would go to;
233 confirming calls `proposeUpstream`, as the following page's Propose button
234 does, and one pull opens on the parent. When a proposal is already open the
235 screen shows `#N is open on owner/name`, and `u` (or `enter` on that row)
236 opens that pull instead of making another. Readers and agents are not offered
237 `u`. Refusals are said in the following page's words (`actions/follow_words.ts`):
238 `This repository follows nothing.`, `Nothing here is ahead of owner/name.`,
239 `You need write access to propose from here.` and `owner/name did not take the
240 proposal: …`.
241
231242 Forks with nothing ahead (or whose standing git cannot read) follow under
232243 `N forks with nothing new`. At most 60 forks are ranked per view; past that
233244 the page says `Ranking the first 60 of N forks`. A fork's standing is cached
src/actions/follow_words.ts +15
@@ -8,3 +8,18 @@ export const FOLLOWS_NOTHING = "This repository follows nothing.";
88
99 /** A reader asking to propose upstream without write access here. */
1010 export const PROPOSE_NEEDS_WRITE = "You need write access to propose from here.";
11
12 /**
13 * Why a proposal upstream was not made, in words: the following page and the
14 * terminal's forks screen both say it from here. Any refusal that is only a
15 * word is an access refusal; one with a reason is the parent's own.
16 */
17 export function proposeRefusalWords(
18 e: "not_a_fork" | "nothing_to_propose" | string | { readonly reason: string },
19 parentName: string,
20 ): string {
21 if (e === "not_a_fork") return FOLLOWS_NOTHING;
22 if (e === "nothing_to_propose") return `Nothing here is ahead of ${parentName}.`;
23 if (typeof e === "string") return PROPOSE_NEEDS_WRITE;
24 return `${parentName} did not take the proposal: ${e.reason}.`;
25 }
src/tui/screens/forks.ts +66 −2
@@ -19,6 +19,9 @@ import {
1919 type ForkRelation,
2020 } from "../../actions/forks.ts";
2121 import { FOLLOW_MODE_REFUSALS, convergeWords, followModeOf, setFollowMode, takeOrTell, takeWords } from "../../actions/follow.ts";
22 import { FOLLOWS_NOTHING, proposeRefusalWords } from "../../actions/follow_words.ts";
23 import { openProposal, proposeUpstream } from "../../actions/upstream.ts";
24 import type { Pull } from "../../core/pulls.ts";
2225 import { convergeState, type Converge } from "../../crew/converge.ts";
2326 import type { FollowMode } from "../../spawn/chain.ts";
2427 import { canAdmin, canRead, canWrite, forkTree, listForks, type ForkNode, type Repo } from "../../core/repos.ts";
@@ -33,6 +36,7 @@ import { issueScreen } from "./issue.ts";
3336 import { local } from "./where.ts";
3437 import { repoScreen } from "./workspace.ts";
3538 import { ListState, confirmScreen, drawList, plural, runAsync, type ListRow } from "./common.ts";
39 import { pullScreen } from "./pull.ts";
3640
3741 /** One line of the screen; `issue` is a converge issue on the fork, which Enter opens. */
3842 export interface ForkRow {
@@ -139,14 +143,56 @@ export function forksScreen(f: Session, ctx: Ctx, repo: Repo): Screen {
139143 let following: ForkRow[] = [];
140144 let relation: ForkRelation | undefined;
141145 let offer: string | undefined;
146 let open: Pull | undefined;
142147 let mode: FollowMode | undefined;
143148 const state = new ListState();
144 const rows = () => [...following, ...(ranked === undefined ? [] : forkRows(ranked, tree))];
149 /** Only a person who writes to a fork may propose its work upstream. */
150 const mayPropose = () => f.user !== undefined && f.user.kind !== "agent" && canWrite(f.db, repo, f.user);
151 const parentName = () =>
152 relation === undefined ? "" : `${relation.parent.ownerName}/${relation.parent.name}`;
153 const openIt = (pull: Pull) => {
154 if (relation !== undefined) ctx.push(pullScreen(f, ctx, local(f, relation.parent), pull.number));
155 };
156 const propose = () => {
157 if (relation === undefined) return ctx.toast(FOLLOWS_NOTHING, "danger");
158 if (open !== undefined) return openIt(open);
159 const name = parentName();
160 if (relation.ahead === 0) return ctx.toast(proposeRefusalWords("nothing_to_propose", name), "danger");
161 ctx.push(
162 confirmScreen(ctx, self, {
163 title: "Propose upstream",
164 body: [`${plural(relation.ahead, "commit")} ahead would go to ${name} as one pull.`],
165 actions: [{ label: "Cancel", variant: "ghost" }, { label: "Propose" }],
166 onChoose: (i) => {
167 if (i !== 1) return;
168 runAsync(ctx, async () => {
169 const made = await proposeUpstream(f.app, f.user, repo);
170 if (!made.ok) return ctx.toast(proposeRefusalWords(made.error, name), "danger");
171 const { pull, opened, changes } = made.value;
172 open = pull;
173 ctx.toast(
174 opened
175 ? `Proposed to ${name}: ${changes === 1 ? "one change" : changes === 0 ? "the commits ahead" : `${changes} changes`}, as #${pull.number}.`
176 : `Already proposed to ${name} as #${pull.number}.`,
177 "success",
178 );
179 ctx.invalidate();
180 });
181 },
182 }),
183 );
184 };
185 const rows = (): ForkRow[] => [
186 ...(open === undefined || relation === undefined ? [] : [openRow(open, relation.parent)]),
187 ...following,
188 ...(ranked === undefined ? [] : forkRows(ranked, tree)),
189 ];
145190 const read = async (): Promise<void> => {
146191 const forks = listForks(f.db, repo.id).filter((r) => canRead(f.db, r, f.user));
147192 ranked = await rankedForksOf(f.app, repo, forks);
148193 tree = forkTree(f.db, repo.id, f.user);
149194 relation = await relationOf(f.app, f.user, repo);
195 open = relation === undefined ? undefined : openProposal(f.app, repo, relation.parent);
150196 offer = takeOffer(relation, canWrite(f.db, repo, f.user), f.user?.kind === "user");
151197 const converge =
152198 relation !== undefined && relation.state === "diverged" && !relation.clean
@@ -193,6 +239,9 @@ export function forksScreen(f: Session, ctx: Ctx, repo: Repo): Screen {
193239 binding(["j", "k"], "move"),
194240 binding(["enter"], "open"),
195241 ...(offer === undefined ? [] : [binding(["t"], "take updates")]),
242 ...(relation !== undefined && mayPropose()
243 ? [binding(["u"], open === undefined ? "propose upstream" : "open proposal")]
244 : []),
196245 ...(canSetMode() ? [binding(["m"], "follow mode")] : []),
197246 binding(["esc"], "close"),
198247 ];
@@ -236,10 +285,15 @@ export function forksScreen(f: Session, ctx: Ctx, repo: Repo): Screen {
236285 if (state.key(e, list.length)) return true;
237286 if (e.key === "enter") {
238287 const row = list[state.index];
239 if (row?.issue !== undefined) ctx.push(issueScreen(f, ctx, local(f, repo), row.issue));
288 if (open !== undefined && state.index === 0) openIt(open);
289 else if (row?.issue !== undefined) ctx.push(issueScreen(f, ctx, local(f, repo), row.issue));
240290 else if (row !== undefined) ctx.push(repoScreen(f, ctx, local(f, row.repo)));
241291 return true;
242292 }
293 if (e.key === "u" && relation !== undefined && mayPropose()) {
294 propose();
295 return true;
296 }
243297 if (e.key === "t" && offer !== undefined) {
244298 take();
245299 return true;
@@ -263,3 +317,13 @@ export function forksScreen(f: Session, ctx: Ctx, repo: Repo): Screen {
263317 };
264318 return self;
265319 }
320
321 /** The line that says a proposal is already open, so `u` opens it instead of making another. */
322 function openRow(pull: Pull, parent: Repo): ForkRow {
323 return {
324 repo: parent,
325 name: `#${pull.number} is open on ${parent.ownerName}/${parent.name}`,
326 words: "enter or u opens it",
327 heading: "",
328 };
329 }
src/web/pages/following.ts +2 −9
@@ -41,7 +41,7 @@ import { modeForm } from "./chain.ts";
4141 import type { FollowMode } from "../../spawn/chain.ts";
4242 import { loadRepo, notFoundPage, repoBase, signInFirst } from "../chrome.ts";
4343 import { convergeWords, followModeOf, takeOrTell, takeWords } from "../../actions/follow.ts";
44 import { FOLLOWS_NOTHING, PROPOSE_NEEDS_WRITE } from "../../actions/follow_words.ts";
44 import { FOLLOWS_NOTHING, proposeRefusalWords } from "../../actions/follow_words.ts";
4545 import { openProposal, proposeUpstream } from "../../actions/upstream.ts";
4646 import { field, readForm } from "../form.ts";
4747 import { html, when, type Html } from "../html.ts";
@@ -417,14 +417,7 @@ export async function followingSubmit(ctx: Ctx): Promise<Response> {
417417 const e = proposed.error;
418418 return flashOrJson(ctx, base, {
419419 tone: "danger",
420 message:
421 e === "not_a_fork"
422 ? FOLLOWS_NOTHING
423 : e === "nothing_to_propose"
424 ? `Nothing here is ahead of ${parentName}.`
425 : typeof e === "string"
426 ? PROPOSE_NEEDS_WRITE
427 : `${parentName} did not take the proposal: ${e.reason}.`,
420 message: proposeRefusalWords(e, parentName),
428421 });
429422 }
430423 const { pull, opened, changes } = proposed.value;
test/follow-words.test.ts +3 −1
@@ -21,7 +21,9 @@ describe("following's words", () => {
2121 test(`${file} reads ${name} rather than typing it`, () => {
2222 const source = readFileSync(join(import.meta.dir, "..", file), "utf8");
2323 expect(source).not.toContain(sentence);
24 expect(source).toContain(name);
24 // The propose refusal reaches the pages through `proposeRefusalWords`.
25 const reads = name === "PROPOSE_NEEDS_WRITE" && file.endsWith("following.ts") ? "proposeRefusalWords" : name;
26 expect(source).toContain(reads);
2527 });
2628 }
2729 }
test/tui-forks-propose.test.ts added +140
@@ -0,0 +1,140 @@
1 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
2 import { mkdtemp, rm } from "node:fs/promises";
3 import { tmpdir } from "node:os";
4 import { join } from "node:path";
5 import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts";
6 import { getRepo, type Repo } from "../src/core/repos.ts";
7 import { listPulls } from "../src/core/pulls.ts";
8 import { createUser, type User } from "../src/core/users.ts";
9 import { createApp, type App } from "../src/app.ts";
10 import { createForgeApp } from "../src/tui/forge.ts";
11 import { forksScreen } from "../src/tui/screens/forks.ts";
12 import { headlessTerminal } from "../src/tui/term/terminal.ts";
13 import { settleApp } from "./helpers/tui.ts";
14 import { promote } from "./helpers/members.ts";
15
16 const must = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => {
17 if (!r.ok) throw new Error(JSON.stringify(r.error));
18 return r.value;
19 };
20
21 describe("the terminal's forks screen proposes a fork's work upstream with u", () => {
22 let dataDir: string;
23 let app: App;
24 let parent: Repo;
25 let fork: Repo;
26 const users: Record<string, User> = {};
27
28 const open = async (who: string, as: User = users[who]!, name = "app") => {
29 const terminal = headlessTerminal(120, 30);
30 const { app: tui, session } = createForgeApp(
31 app,
32 { sshUsername: who, remoteAddress: "127.0.0.1", fingerprint: "SHA256:test", user: as },
33 { terminal, exitHooks: false },
34 );
35 tui.start();
36 tui.ctx.push(forksScreen(session, tui.ctx, getRepo(app.db, "bob", name)!));
37 await settleApp(tui);
38 const press = async (keys: string) => {
39 terminal.feed(keys);
40 await settleApp(tui);
41 await Bun.sleep(100);
42 await settleApp(tui);
43 return tui.screen.toText();
44 };
45 return { press, text: () => tui.screen.toText(), stop: () => tui.stop?.() };
46 };
47 const pulls = () => listPulls(app.db, parent.id, { sealed: true, state: "open", limit: 50 });
48
49 beforeAll(async () => {
50 dataDir = await mkdtemp(join(tmpdir(), "forge-tui-forks-propose-"));
51 app = createApp({ dataDir, port: 0 });
52 for (const name of ["alice", "bob", "carol"]) {
53 users[name] = must(
54 await createUser(app.db, {
55 origin: "seed",
56 trusted: true,
57 username: name,
58 email: `${name}@f.dev`,
59 password: "a-good-passphrase",
60 adminIfFirst: name === "alice",
61 }),
62 );
63 promote(app.db, users[name]!);
64 }
65 parent = must(await createRepo(app, users["alice"]!, { name: "app", visibility: "public" }));
66 must(await commitToBranch(app, users["alice"]!, parent, { message: "base", files: [{ path: "a.txt", content: "a\n" }] }));
67 fork = must(await forkRepo(app, users["bob"]!, parent));
68 }, 60_000);
69
70 afterAll(async () => {
71 app.close();
72 await rm(dataDir, { recursive: true, force: true });
73 });
74
75 test("nothing ahead: u says so and opens no pull", async () => {
76 const t = await open("bob");
77 expect(await t.press("u")).toContain("Nothing here is ahead of alice/app.");
78 expect(pulls()).toHaveLength(0);
79 t.stop();
80 });
81
82 test("a writer confirms and one pull opens on the parent; a second u opens that pull", async () => {
83 must(await commitToBranch(app, users["bob"]!, fork, { message: "bob work", files: [{ path: "b.txt", content: "b\n" }] }));
84 const t = await open("bob");
85 const asked = await t.press("u");
86 expect(asked).toContain("1 commit ahead would go to alice/app");
87 expect(pulls()).toHaveLength(0);
88 // cancel leaves it alone
89 await t.press("\x1b");
90 expect(pulls()).toHaveLength(0);
91 await t.press("u");
92 const done = await t.press("\x1b[C\r");
93 expect(pulls()).toHaveLength(1);
94 const n = pulls()[0]!.number;
95 expect(done).toContain(`#${n} is open on alice/app`);
96 // u again opens the pull rather than making another
97 const second = await t.press("u");
98 expect(pulls()).toHaveLength(1);
99 expect(second).toContain(`#${n} 1 commit from bob/app`);
100 t.stop();
101 const entered = await open("bob");
102 expect(await entered.press("\r")).toContain(`#${n} 1 commit from bob/app`);
103 entered.stop();
104 const again = await open("bob");
105 expect(again.text()).toContain(`#${n} is open on alice/app`);
106 await again.press("u");
107 expect(pulls()).toHaveLength(1);
108 again.stop();
109 });
110
111 test("an agent is not offered u, nor a reader", async () => {
112 const t = await open("bob", { ...users["bob"]!, kind: "agent" });
113 expect(await t.press("u")).not.toContain("would go to");
114 t.stop();
115 });
116
117 test("a writer who loses write before confirming is told they need it", async () => {
118 const lib = must(await createRepo(app, users["alice"]!, { name: "lib", visibility: "public" }));
119 must(await commitToBranch(app, users["alice"]!, lib, { message: "base", files: [{ path: "a.txt", content: "a\n" }] }));
120 const mine = must(await forkRepo(app, users["bob"]!, lib));
121 must(await commitToBranch(app, users["bob"]!, mine, { message: "bob lib", files: [{ path: "b.txt", content: "b\n" }] }));
122 // carol writes to bob's fork only as an admin, and stops being one after the confirm shows
123 const carol = { ...users["carol"]!, isAdmin: true };
124 const t = await open("carol", carol, "lib");
125 expect(await t.press("u")).toContain("would go to alice/lib");
126 carol.isAdmin = false;
127 const said = await t.press("\x1b[C\r");
128 expect(said).toContain("You need write access to propose from here.");
129 expect(listPulls(app.db, lib.id, { sealed: true, state: "open", limit: 50 })).toHaveLength(0);
130 t.stop();
131 });
132
133 test("a reader is not offered u", async () => {
134 const t = await open("carol");
135 const text = await t.press("u");
136 expect(text).not.toContain("would go to");
137 expect(pulls()).toHaveLength(1);
138 t.stop();
139 });
140 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs