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

The terminal's pull screen takes back a shipped pull with one key and a confirm, as the page does #7222

Merged opened this

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

Merged

scout/forge:scout/7212-the-terminal-s-pull-screen-takes-back-a → main 4 commits fork

  • Merged. scout/7212-the-terminal-s-pull-screen-takes-back-a was merged into main as 4b1f19f.

Review

Sign in to review this change.
3 files changed +251 −1
README.md +2
@@ -44,6 +44,8 @@ A hold can be set "until green" (a checkbox on the hold form, `until: "green"` o
4444
4545 A hold an agent placed does not stand forever: the tick releases one still standing after 24 h, in one transaction, with a comment on the pull, "Hold expired after 24 h: <why>. Back to @builder.", which shows in the pull's timeline like any comment, and the pull goes back to its builder (`expireCrewHolds` in `src/actions/holds.ts`). A person's hold is never expired. The window is fixed; a pull held again and expiring again leaves one such comment per round in its timeline.
4646
47 In the terminal, `T` on a shipped pull's screen takes it back, as the pull page's button does: a one-line confirm names the pull, Yes opens the pull that undoes it (`revertPull`, `src/actions/revert.ts`) and shows it, No changes nothing. The key appears only to a person who can write a local repository (never an agent, a reader, a guest or a mirror), and refusals read as the page's do (`takeBackWords`). `T` also opens an app preview's terminal on an open pull; a shipped pull has no live preview, so the two never meet.
48
4749 A hand-over card on `/asks` (an agent's pull the reviewer stopped on after three rounds) has "Give back to the crew": one press assigns the pull, and the open issue it closes if a person holds it, to the agent that opened it, writes "Given back to the crew by @who", and the card leaves; the next tick starts its run. It is for a writer of the repository, and a pull a person opened, or by an agent gone from the crew, gets no button. In the terminal, `b` on a handed-over row of the On you pane does the same (`src/actions/give_back.ts`). The repository's planner can do it by comment: one that starts "Planner, one more round" on a handed-over crew pull emits the assignment to its builder, which starts the run (the pull is already the builder's, so nothing changes hands). It is the planner's alone; a reader's, the reviewer's or any other agent's does nothing. It is an agent's word, not a director's, so it does not end the hand-over: the card stays on `/asks` and the pull still counts as handed over during the round.
4850
4951 Home opens with "Since you were here · 2 days": the pulls that landed in public repositories since you last loaded `/` (1 hour to 7 days back, a day on a first visit); the terminal's home shows the same lines, and MCP `feed` answers them as `digest`. See `docs/features/since-you-were-here.md`.
src/tui/screens/pull.ts +48 −1
@@ -39,6 +39,8 @@ import {
3939 resolveReviewThread,
4040 submitReview,
4141 } from "../../actions/pulls.ts";
42 import { revertPull } from "../../actions/revert.ts";
43 import { takeBackWords } from "../../actions/revert_words.ts";
4244 import { openEnvUrl } from "../../actions/apps.ts";
4345 import { listRepoRuns, mayReadRunLog } from "../../actions/runs.ts";
4446 import { previewEnv } from "../../apps/data.ts";
@@ -680,6 +682,45 @@ export function pullScreen(
680682 });
681683 };
682684
685 /**
686 * Whether `T` takes this back: a shipped pull on a local repository, for a
687 * person who may write. The same gate `revertPull` holds; it decides only
688 * whether the key is shown.
689 */
690 const mayTakeBack = (): boolean => {
691 if (repo === undefined || repo.origin !== null || !mayWrite()) return false;
692 if (f.user === undefined || f.user.kind === "agent") return false;
693 return getPull(f.db, repo.id, number)?.mergedAt != null;
694 };
695
696 const takeBack = (): void => {
697 if (repo === undefined) return;
698 const target = repo;
699 const row = getPull(f.db, target.id, number);
700 if (row === undefined) return;
701 ctx.push(
702 confirmScreen(ctx, screen, {
703 title: `Take back #${number}`,
704 body: [`Open a pull that undoes "${row.title}" (#${number})?`],
705 actions: [{ label: "Cancel", variant: "ghost" }, { label: "Take it back" }],
706 onChoose: (choice) => {
707 if (choice !== 1) return;
708 runAsync(ctx, async () => {
709 const made = await revertPull(f.app, f.user, target, row);
710 if (!made.ok) {
711 ctx.toast(
712 `That could not be taken back: ${takeBackWords(made.error)}.`,
713 "danger",
714 );
715 return;
716 }
717 ctx.replace(pullScreen(f, ctx, local(f, target), made.value.number));
718 });
719 },
720 }),
721 );
722 };
723
683724 /**
684725 * Whether `A` is on offer: an open origin-fork pull whose current head
685726 * nobody here has approved to run. The same condition as the page's card.
@@ -770,6 +811,7 @@ export function pullScreen(
770811 const screen: Screen = {
771812 title: `#${number}`,
772813 get keys() {
814 const takeBackKey = mayTakeBack();
773815 return [
774816 binding(["tab"], tabCount() === 3 ? "next tab" : "conversation/files"),
775817 ...(tab === 1
@@ -781,7 +823,8 @@ export function pullScreen(
781823 ? [binding(["O"], "open the preview")]
782824 : []),
783825 ...(isApp ? [binding(["P"], "open production")] : []),
784 ...(isApp && preview !== undefined
826 ...(takeBackKey ? [binding(["T"], "take it back")] : []),
827 ...(isApp && preview !== undefined && !takeBackKey
785828 ? [binding(["T"], "the preview's terminal")]
786829 : []),
787830 ...(mayWrite()
@@ -1019,6 +1062,10 @@ export function pullScreen(
10191062 openProduction();
10201063 return true;
10211064 }
1065 if (e.key === "T" && mayTakeBack()) {
1066 takeBack();
1067 return true;
1068 }
10221069 if (e.key === "T" && isApp && preview !== undefined && repo) {
10231070 void openAppTerminal(f, ctx, repo, previewEnv(number));
10241071 return true;
test/tui-take-back.test.ts added +201
@@ -0,0 +1,201 @@
1 /**
2 * The terminal's pull screen takes back a shipped pull with `T` and a
3 * confirm, as the page does: absent on an open pull and to an agent, No
4 * changes nothing, Yes opens the revert pull, and a second take-back is
5 * refused in the page's words.
6 */
7 import { afterEach, beforeEach, expect, test } from "bun:test";
8 import { mkdtemp, rm } from "node:fs/promises";
9 import { tmpdir } from "node:os";
10 import { join } from "node:path";
11 import { addToCrew, createAgent } from "../src/actions/crew.ts";
12 import { mergePull, 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 { getPull, revertOf, type Pull } from "../src/core/pulls.ts";
16 import type { Repo } from "../src/core/repos.ts";
17 import { createUser, getUserByName, type User } from "../src/core/users.ts";
18 import { createForgeApp } from "../src/tui/forge.ts";
19 import { pullScreen } from "../src/tui/screens/pull.ts";
20 import { local } from "../src/tui/screens/where.ts";
21 import { headlessTerminal } from "../src/tui/term/terminal.ts";
22 import { quietLogs } from "./helpers/globals.ts";
23 import { settleApp } from "./helpers/tui.ts";
24
25 quietLogs();
26
27 let dataDir: string;
28 let app: App;
29 let alice: User;
30 let repo: Repo;
31 let n = 0;
32 const running: { stop(): void }[] = [];
33
34 beforeEach(async () => {
35 dataDir = await mkdtemp(join(tmpdir(), "forge-tui-take-back-"));
36 app = createApp({ dataDir, port: 0, host: "forge.test" });
37 const made = await createUser(app.db, {
38 origin: "seed",
39 trusted: true,
40 username: "alice",
41 email: "[email protected]",
42 password: "a-good-passphrase",
43 adminIfFirst: true,
44 });
45 if (!made.ok) throw new Error(String(made.error));
46 alice = made.value;
47 const r = await createRepo(app, alice, { name: "lib", visibility: "public" });
48 if (!r.ok) throw new Error(String(r.error));
49 repo = r.value;
50 await commitToBranch(app, alice, repo, {
51 branch: "main",
52 message: "base",
53 files: [{ path: "code.txt", content: "alpha\n" }],
54 });
55 });
56
57 afterEach(async () => {
58 // Stop the screens before the database closes: a reload in flight would
59 // otherwise land on a closed handle.
60 for (const t of running.splice(0)) t.stop();
61 app.close();
62 await rm(dataDir, { recursive: true, force: true });
63 });
64
65 const pullOf = async (merge: boolean): Promise<Pull> => {
66 const branch = `b${++n}`;
67 await commitToBranch(app, alice, repo, {
68 branch,
69 base: "main",
70 message: branch,
71 files: [{ path: "code.txt", content: `alpha\n${branch}\n` }],
72 });
73 const opened = await openPull(app, alice, repo, {
74 title: `Change ${branch}`,
75 baseBranch: "main",
76 headBranch: branch,
77 });
78 if (!opened.ok) throw new Error(String(opened.error));
79 if (merge) {
80 const m = await mergePull(app, alice, repo, opened.value, "squash");
81 if (!m.ok) throw new Error(JSON.stringify(m.error));
82 }
83 return getPull(app.db, repo.id, opened.value.number)!;
84 };
85
86 async function open(
87 number: number,
88 user: User | null = alice,
89 shown: Repo = repo,
90 ) {
91 const { app: tui, session } = createForgeApp(
92 app,
93 {
94 sshUsername: user?.username ?? "guest",
95 remoteAddress: "127.0.0.1",
96 fingerprint: "SHA256:x",
97 user: user ?? undefined,
98 },
99 { terminal: headlessTerminal(160, 44), exitHooks: false },
100 );
101 tui.start();
102 running.push(tui);
103 tui.ctx.push(pullScreen(session, tui.ctx, local(session, shown), number));
104 await settleApp(tui);
105 const press = async (...keys: string[]): Promise<void> => {
106 for (const key of keys)
107 tui.dispatch({ type: "key", key, ctrl: false, alt: false, shift: false });
108 await settleApp(tui);
109 };
110 return { press, text: () => tui.screen.toText() };
111 }
112
113 test("the key is absent on an open pull", async () => {
114 const pull = await pullOf(false);
115 const t = await open(pull.number);
116 expect(t.text()).not.toContain("take it back");
117 await t.press("T");
118 expect(t.text()).not.toContain("Take back");
119 expect(revertOf(app.db, repo.id, pull.id)).toBeUndefined();
120 });
121
122 test("the key is absent to an agent", async () => {
123 const pull = await pullOf(true);
124 const made = await createAgent(app, alice, { username: "quill" });
125 if (!made.ok) throw new Error(String(made.error));
126 addToCrew(app, alice, repo, "quill");
127 const t = await open(pull.number, getUserByName(app.db, "quill")!);
128 expect(t.text()).not.toContain("take it back");
129 await t.press("T");
130 expect(t.text()).not.toContain("Take back");
131 });
132
133 test("T asks, Yes opens the new pull's screen", async () => {
134 const pull = await pullOf(true);
135 const t = await open(pull.number);
136 expect(t.text()).toContain("take it back");
137 await t.press("T");
138 expect(t.text()).toContain(`Take back #${pull.number}`);
139 expect(t.text()).toContain(pull.title);
140 await t.press("right", "enter");
141 const made = revertOf(app.db, repo.id, pull.id);
142 expect(made).toBeDefined();
143 expect(t.text()).toContain(`Revert "${pull.title}"`);
144 });
145
146 test("No makes nothing", async () => {
147 const pull = await pullOf(true);
148 const t = await open(pull.number);
149 await t.press("T", "enter");
150 expect(t.text()).not.toContain("Take back #");
151 expect(revertOf(app.db, repo.id, pull.id)).toBeUndefined();
152 });
153
154 test("a second take-back is refused as already taken back", async () => {
155 const pull = await pullOf(true);
156 const first = await open(pull.number);
157 await first.press("T", "right", "enter");
158 expect(revertOf(app.db, repo.id, pull.id)).toBeDefined();
159 const again = await open(pull.number);
160 await again.press("T", "right", "enter");
161 expect(again.text()).toContain("it has already been taken back");
162 });
163
164 test("a guest and a read-only user are shown no key and press it for nothing", async () => {
165 const pull = await pullOf(true);
166 const made = await createUser(app.db, {
167 origin: "seed",
168 trusted: true,
169 username: "bob",
170 email: "[email protected]",
171 password: "a-good-passphrase",
172 });
173 if (!made.ok) throw new Error(String(made.error));
174 for (const who of [null, made.value]) {
175 const t = await open(pull.number, who);
176 expect(t.text()).not.toContain("take it back");
177 await t.press("T");
178 expect(t.text()).not.toContain("Take back");
179 expect(revertOf(app.db, repo.id, pull.id)).toBeUndefined();
180 }
181 });
182
183 test("a mirrored repository shows no key", async () => {
184 const pull = await pullOf(true);
185 const mirror: Repo = {
186 ...repo,
187 origin: {
188 originId: "o1",
189 kind: "github",
190 fullName: "up/lib",
191 htmlUrl: "https://example.test/up/lib",
192 lastFetchAt: null,
193 fetchError: "",
194 },
195 };
196 const t = await open(pull.number, alice, mirror);
197 expect(t.text()).not.toContain("take it back");
198 await t.press("T");
199 expect(t.text()).not.toContain("Take back");
200 expect(revertOf(app.db, repo.id, pull.id)).toBeUndefined();
201 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs