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
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/7212-the-terminal-s-pull-screen-takes-back-a → main 4 commits fork
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 | ||
| 44 | 44 | |
| 45 | 45 | 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. |
| 46 | 46 | |
| 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 | ||
| 47 | 49 | 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. |
| 48 | 50 | |
| 49 | 51 | 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 { | ||
| 39 | 39 | resolveReviewThread, |
| 40 | 40 | submitReview, |
| 41 | 41 | } from "../../actions/pulls.ts"; |
| 42 | import { revertPull } from "../../actions/revert.ts"; | |
| 43 | import { takeBackWords } from "../../actions/revert_words.ts"; | |
| 42 | 44 | import { openEnvUrl } from "../../actions/apps.ts"; |
| 43 | 45 | import { listRepoRuns, mayReadRunLog } from "../../actions/runs.ts"; |
| 44 | 46 | import { previewEnv } from "../../apps/data.ts"; |
| @@ -680,6 +682,45 @@ export function pullScreen( | ||
| 680 | 682 | }); |
| 681 | 683 | }; |
| 682 | 684 | |
| 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 | ||
| 683 | 724 | /** |
| 684 | 725 | * Whether `A` is on offer: an open origin-fork pull whose current head |
| 685 | 726 | * nobody here has approved to run. The same condition as the page's card. |
| @@ -770,6 +811,7 @@ export function pullScreen( | ||
| 770 | 811 | const screen: Screen = { |
| 771 | 812 | title: `#${number}`, |
| 772 | 813 | get keys() { |
| 814 | const takeBackKey = mayTakeBack(); | |
| 773 | 815 | return [ |
| 774 | 816 | binding(["tab"], tabCount() === 3 ? "next tab" : "conversation/files"), |
| 775 | 817 | ...(tab === 1 |
| @@ -781,7 +823,8 @@ export function pullScreen( | ||
| 781 | 823 | ? [binding(["O"], "open the preview")] |
| 782 | 824 | : []), |
| 783 | 825 | ...(isApp ? [binding(["P"], "open production")] : []), |
| 784 | ...(isApp && preview !== undefined | |
| 826 | ...(takeBackKey ? [binding(["T"], "take it back")] : []), | |
| 827 | ...(isApp && preview !== undefined && !takeBackKey | |
| 785 | 828 | ? [binding(["T"], "the preview's terminal")] |
| 786 | 829 | : []), |
| 787 | 830 | ...(mayWrite() |
| @@ -1019,6 +1062,10 @@ export function pullScreen( | ||
| 1019 | 1062 | openProduction(); |
| 1020 | 1063 | return true; |
| 1021 | 1064 | } |
| 1065 | if (e.key === "T" && mayTakeBack()) { | |
| 1066 | takeBack(); | |
| 1067 | return true; | |
| 1068 | } | |
| 1022 | 1069 | if (e.key === "T" && isApp && preview !== undefined && repo) { |
| 1023 | 1070 | void openAppTerminal(f, ctx, repo, previewEnv(number)); |
| 1024 | 1071 | 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 | }); |
new above
offline