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

Plain git is a first-class way to contribute: a push to a fork offers the pul… #12115

Merged opened this in answer to

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

Merged

mason/forge:mason/12107-plain-git-is-a-first-class-way-to-contri → main 13 commits fork 1 thread · 1 unresolved

  • Merged. mason/12107-plain-git-is-a-first-class-way-to-contri was merged into main as 749c831.

Review

Sign in to review this change.
32 files changed +840 −38
docs/design/attention-budget.json +4 −4
@@ -235,13 +235,13 @@
235235 },
236236 "crew": {
237237 "390": {
238 "words": 730,
239 "blocks": 86,
238 "words": 742,
239 "blocks": 89,
240240 "colours": 4
241241 },
242242 "1280": {
243 "words": 730,
244 "blocks": 86,
243 "words": 742,
244 "blocks": 89,
245245 "colours": 4
246246 }
247247 }
docs/features/plain-git.md added +6
@@ -0,0 +1,6 @@
1 # Plain git contributes
2
3 - **Push to a fork.** A new branch pushed to a fork prints the address that opens its pull into the fork's upstream. `git push -o pull origin <branch>` opens it (title and body from the tip commit's message); a later push to that branch prints the open pull's address.
4 - **Landing.** Each repository has a setting, *How a pull lands* (`landing` in the API's repository settings): `merge` (default, a merge commit for a kept fork's pull) or `linear` (opt-in). Linear: a one-commit pull on an unmoved base whose author is a real address and whose subject has no pull number fast-forwards; any other lands as one commit on the base, authored by the contributor, titled `<title> (#<upstream number>)` with any `(#n)` from the contributor's own forge dropped. Existing repositories keep `merge`.
5 - **Approval.** A force-push that only rebases a pull onto a moved base (the same change, line for line) keeps its approvals; any other change makes them stale as before (`src/core/carry_approval.ts`).
6 - Not done: a non-member pushing to the upstream still hears "repository not found"; rebase as a third way to land.
src/actions/fork_push.ts added +75
@@ -0,0 +1,75 @@
1 /**
2 * What a push to a fork says about the pull (#12107). A new branch pushed to a
3 * fork prints the address that opens its pull, as other forges do; `git push -o
4 * pull` opens it from the tip commit's message, and a later push to a branch
5 * whose pull is open prints that pull's address. The lines are what git shows
6 * the pusher after the push.
7 */
8
9 import type { App } from "../app.ts";
10 import { getRepoById, forkParentFor, type Repo } from "../core/repos.ts";
11 import { openPullFromFork } from "../core/pulls.ts";
12 import type { User } from "../core/users.ts";
13 import { runGit } from "../git/env.ts";
14 import { ZERO_OID, type RefUpdate } from "../git/hook-protocol.ts";
15 import { repoDir } from "../git/repos.ts";
16 import { pullWords } from "./contribute_push.ts";
17 import { openPull } from "./pulls.ts";
18
19 const HEADS = "refs/heads/";
20
21 /**
22 * Lines a push to `fork` prints about the pulls of the branches it moved;
23 * none when `fork` is not a fork.
24 */
25 export async function forkPushLines(
26 app: App,
27 fork: Repo,
28 actor: User | undefined,
29 updates: readonly RefUpdate[],
30 origin: string,
31 asked: boolean,
32 ): Promise<string[]> {
33 const lines: string[] = [];
34 if (actor === undefined || fork.forkParentId === null) return lines;
35 const parent = forkParentFor(app.db, fork, actor);
36 if (parent === undefined || getRepoById(app.db, fork.id) === undefined) return lines;
37 for (const update of updates) {
38 if (!update.ref.startsWith(HEADS) || ZERO_OID.test(update.newOid)) continue;
39 const branch = update.ref.slice(HEADS.length);
40 // The fork's copy of the default branch is its own to move, not a proposal.
41 if (branch === fork.defaultBranch) continue;
42 const at = `${origin}/${parent.ownerName}/${parent.name}/pulls`;
43 const known = openPullFromFork(app.db, parent.id, fork.id, branch);
44 if (known !== undefined) {
45 lines.push(`pull #${known.number} follows this branch: ${at}/${known.number}`);
46 continue;
47 }
48 const retry = `${origin}/${fork.ownerName}/${fork.name}/pulls/new?head=${encodeURIComponent(branch)}`;
49 if (!asked) {
50 if (ZERO_OID.test(update.oldOid))
51 lines.push(
52 `To open a pull into ${parent.ownerName}/${parent.name}, visit ${retry}`,
53 `or push again with: git push -o pull`,
54 );
55 continue;
56 }
57 const log = await runGit(["log", "-1", "--format=%B", update.newOid], {
58 cwd: repoDir(app.reposRoot, fork.ownerName, fork.name),
59 });
60 const words = pullWords(log.ok ? log.value : "", branch);
61 const opened = await openPull(app, actor, parent, {
62 title: words.title,
63 body: words.body,
64 baseBranch: parent.defaultBranch,
65 headBranch: branch,
66 headRepo: fork,
67 });
68 if (opened.ok) lines.push(`opened pull #${opened.value.number}: ${at}/${opened.value.number}`);
69 else {
70 const why = typeof opened.error === "string" ? opened.error : "refused";
71 lines.push(`${branch} is pushed but no pull was opened (${why}): ${retry}`);
72 }
73 }
74 return lines;
75 }
src/actions/pulls.ts +6 −4
@@ -77,7 +77,7 @@ import { commitIdentity } from "../core/commit_identity.ts";
7777 import { getUserById, getUserByName, type User } from "../core/users.ts";
7878 import { squashCredits } from "../git/coauthors.ts";
7979 import { resolveOid, type MergeStrategy } from "../git/merge.ts";
80 import { landingKind } from "../core/landing_kind.ts";
80 import { bareTitle, fromKeptFork, landingKind } from "../core/landing_kind.ts";
8181 import { thankAuthor } from "../origins/thanks.ts";
8282 import { repoDir } from "../git/repos.ts";
8383 import { isStrangerPull, strangerGuard } from "../crew/stranger.ts";
@@ -514,7 +514,7 @@ export async function mergePull(
514514 if (pull.originHeadRepo !== null || repo.origin !== null)
515515 return err("mirrored");
516516 // A fork someone keeps lands as a true merge whatever was asked (#8935).
517 const strategy = landingKind(app.db, pull, requested);
517 const strategy = landingKind(app.db, pull, requested, repo.landing);
518518 // A fork may have pushed since the pull opened; merge what is being
519519 // proposed now, not the snapshot taken when it was.
520520 await refreshPullHead(app, repo, pull);
@@ -530,8 +530,9 @@ export async function mergePull(
530530 // asked is named in `Requested-by`, the person who landed it in
531531 // `Co-authored-by`.
532532 const writtenBy = getUserById(app.db, pull.authorId);
533 const linearFork = fromKeptFork(app.db, pull) && strategy === "squash";
533534 const agentAuthor =
534 writtenBy?.kind === "agent"
535 writtenBy?.kind === "agent" || (linearFork && writtenBy !== undefined)
535536 ? commitIdentity(app.config, writtenBy)
536537 : undefined;
537538 const asked = toldBy(app.db, pull);
@@ -580,11 +581,12 @@ export async function mergePull(
580581 }),
581582 },
582583 strategy,
584 ...(linearFork ? { fastForward: true } : {}),
583585 ...(expect === undefined ? {} : { expect }),
584586 ...(stranger ? { headOid: judged.headOid } : {}),
585587 message: `${
586588 strategy === "squash"
587 ? `${pull.title} (#${pull.number})`
589 ? `${bareTitle(pull.title)} (#${pull.number})`
588590 : `Merge pull request #${pull.number} from ${pull.headBranch}\n\n${pull.title}`
589591 }\n\n${trailers}`,
590592 actorId: writer.value.id,
src/actions/repos.ts +1
@@ -376,6 +376,7 @@ const AUDITED = [
376376 "defaultBranch",
377377 "requireReview",
378378 "requireChecks",
379 "landing",
379380 "crewOutsiders",
380381 "decisionsBy",
381382 "plansNeedNod",
src/core/carry_approval.ts added +88
@@ -0,0 +1,88 @@
1 /**
2 * A refresh that changes nothing but the base keeps its approval (#12107). A
3 * force-push that rebases a pull onto a moved base gives it a new head commit
4 * with the same change; the verdicts passed on the old head stand for the new
5 * one when the two heads change exactly the same lines against their own
6 * bases, at the same places, in files the base did not move. A head that
7 * changes anything else is a new head, and its verdicts are
8 * stale as they always were.
9 */
10
11 import type { Database } from "bun:sqlite";
12 import { runGitCapture } from "../git/env.ts";
13 import { isSafeOid } from "../git/guard.ts";
14 import { mergeBase } from "../git/merge.ts";
15
16 const MAX_DIFF = 2_000_000;
17
18 /** One head's change against its fork point: where it forked, what it touches, and the diff. */
19 interface Change {
20 readonly from: string;
21 readonly files: readonly string[];
22 readonly diff: string;
23 }
24
25 /**
26 * The diff keeps its hunk headers, so a hunk moved to another place, or the
27 * same lines at another offset, is another change. Only the index hashes,
28 * which a moved base rewrites, are left out.
29 */
outdated

Collapsing every @@ header to a bare @@ drops where the change is, so a moved hunk compares equal and keeps the approval. Keep the new-side start/length or use patch-id; add a test for a moved hunk.

30 async function changeOf(dir: string, baseOid: string, headOid: string): Promise<Change | undefined> {
31 const from = await mergeBase(dir, baseOid, headOid);
32 if (from === undefined) return undefined;
33 const run = (args: string[]) => runGitCapture(["diff", "--no-color", "--no-ext-diff", "--no-renames", ...args, from, headOid], { cwd: dir, timeoutMs: 30_000 });
34 const [full, names] = await Promise.all([run(["--binary"]), run(["--name-only", "-z"])]);
35 if (full.code !== 0 || names.code !== 0 || full.stdout.length > MAX_DIFF) return undefined;
36 const diff = full.stdout
37 .split("\n")
38 .filter((line) => !line.startsWith("index "))
39 .join("\n");
40 return { from, files: names.stdout.split("\0").filter((f) => f !== ""), diff };
41 }
42
43 /**
44 * Whether the base moved under the change: a file it touches differs between
45 * the two fork points. Line numbers may then legitimately shift, and the
46 * carried verdict could cover a different place, so nothing carries.
47 */
48 async function baseMovedUnder(dir: string, a: Change, b: Change): Promise<boolean> {
49 if (a.from === b.from) return false;
50 const out = await runGitCapture(["diff", "--no-renames", "--name-only", "-z", a.from, b.from], { cwd: dir, timeoutMs: 30_000 });
51 if (out.code !== 0) return true;
52 const moved = new Set(out.stdout.split("\0"));
53 return a.files.some((f) => moved.has(f));
54 }
55
56 /** Whether `newOid` makes the same change onto `baseOid` that `oldOid` did (and makes one). */
57 export async function sameChange(dir: string, baseOid: string, oldOid: string, newOid: string): Promise<boolean> {
58 if (!isSafeOid(baseOid) || !isSafeOid(oldOid) || !isSafeOid(newOid)) return false;
59 if (oldOid === newOid) return true;
60 const [before, after] = await Promise.all([changeOf(dir, baseOid, oldOid), changeOf(dir, baseOid, newOid)]);
61 if (before === undefined || after === undefined || before.diff === "") return false;
62 if (before.diff !== after.diff) return false;
63 return !(await baseMovedUnder(dir, before, after));
64 }
65
66 /**
67 * Re-stamp a pull's verdicts from the head they were passed on to the head
68 * that changes the same thing.
69 */
70 export function carryReviews(db: Database, pullId: string, from: string, to: string): number {
71 return db.query("UPDATE review SET commit_oid = ?3 WHERE issue_id = ?1 AND commit_oid = ?2").run(pullId, from, to).changes;
72 }
73
74 /**
75 * Carry a pull's verdicts to its new head when the change is the same. `dir`
76 * holds both heads and the base tip; true when they were carried.
77 */
78 export async function carryApproval(
79 db: Database,
80 dir: string,
81 pull: { readonly id: string; readonly headOid: string },
82 baseOid: string,
83 newOid: string,
84 ): Promise<boolean> {
85 if (pull.headOid === newOid || pull.headOid === "") return false;
86 if (!(await sameChange(dir, baseOid, pull.headOid, newOid))) return false;
87 return carryReviews(db, pull.id, pull.headOid, newOid) > 0;
88 }
src/core/following.ts +6
@@ -18,6 +18,7 @@
1818 * so it is read here as the push it amounts to.
1919 */
2020
21 import { carryApproval } from "./carry_approval.ts";
2122 import type { Database } from "bun:sqlite";
2223 import type { EventBus, ForgeEvent } from "./events.ts";
2324 import { getPull, listOpenPullsFromHead, type Pull } from "./pulls.ts";
@@ -471,6 +472,11 @@ export async function refreshPullHead(
471472 : await fetchInto(baseDir, headDir, `refs/heads/${pull.headBranch}`);
472473 if (tip === undefined) return undefined;
473474 await writeRef(snapshotDir, `refs/pull/${pull.number}/head`, tip);
475 // A rebase onto a moved base changes no line: its approval stands.
476 if (!apart) {
477 const baseNow = await resolveOid(baseDir, `refs/heads/${pull.baseBranch}`);
478 if (baseNow !== undefined) await carryApproval(forge.db, baseDir, pull, baseNow, tip);
479 }
474480 // The row says where the head is too, so a reader of the pull — the API,
475481 // a page — sees the same tip the ref holds.
476482 forge.db
src/core/landing_kind.ts +20 −3
@@ -7,7 +7,7 @@
77 */
88
99 import type { Database } from "bun:sqlite";
10 import { getRepoById } from "./repos.ts";
10 import { getRepoById, type Landing } from "./repos.ts";
1111 import { getUserById } from "./users.ts";
1212 import type { Pull } from "./pulls.ts";
1313 import type { MergeStrategy } from "../git/merge.ts";
@@ -24,11 +24,28 @@ export function fromKeptFork(
2424 return getUserById(db, head.ownerId)?.kind !== "agent";
2525 }
2626
27 /** The strategy a pull lands with: a kept fork's is always a merge, whatever was asked. */
27 /**
28 * The strategy a pull lands with. A kept fork's pull merges, as it always
29 * did, unless the repository opted into `landing: linear`: then it lands as
30 * one commit in a line (a squash, or the commit itself when it is one on an
31 * unmoved base) (#12107).
32 */
2833 export function landingKind(
2934 db: Database,
3035 pull: Pick<Pull, "crossRepo" | "headRepoId" | "authorId">,
3136 asked: MergeStrategy,
37 landing: Landing = "merge",
3238 ): MergeStrategy {
33 return fromKeptFork(db, pull) ? "merge" : asked;
39 if (!fromKeptFork(db, pull)) return asked;
40 return landing === "linear" ? "squash" : "merge";
41 }
42
43 /** A title without the pull numbers another forge gave it: `Fix x (#2)` is `Fix x`. */
44 export function bareTitle(title: string): string {
45 let t = title.trim();
46 for (;;) {
47 const next = t.replace(/\s*\(#\d+\)\s*$/, "");
48 if (next === t) return t || title.trim();
49 t = next;
50 }
3451 }
src/core/migration-pins/20261114030000-repo-landing.pin added +1
@@ -0,0 +1,1 @@
1 ab484df709b01b224a0de47af8f2dd78463fe8edf0dc2ce6678dd522fda6f0a0
src/core/migrations/20261114030000-repo-landing.ts added +7
@@ -0,0 +1,7 @@
1 // How a pull lands in this repository (#12107): `merge` (the default, what every repository did
2 // before) records a merge commit for a kept fork's pull; `linear` is opt-in and fast-forwards a
3 // one-commit pull whose base has not moved, otherwise lands one commit authored by its contributor.
4 // Additive.
5 export default `
6 ALTER TABLE repo ADD COLUMN landing TEXT NOT NULL DEFAULT 'merge';
7 `;
src/core/pulls.ts +33 −2
@@ -19,7 +19,9 @@ import { readTips } from "../git/tips.ts";
1919 import {
2020 aheadCount,
2121 contentOnBase,
22 fastForward,
2223 fetchInto,
24 landsAsIs,
2325 merge,
2426 mergeBase,
2527 mergeable,
@@ -365,6 +367,24 @@ export function pullForBranch(
365367 return row ? getPull(db, repoId, row.number) : undefined;
366368 }
367369
370 /** The open pull a fork's branch already proposes into `baseRepoId`, if any. */
371 export function openPullFromFork(
372 db: Database,
373 baseRepoId: string,
374 headRepoId: string,
375 branch: string,
376 ): Pull | undefined {
377 const row = db
378 .query<{ number: number }, [string, string, string]>(
379 `SELECT issue.number AS number FROM pull JOIN issue ON issue.id = pull.issue_id
380 WHERE issue.repo_id = ?1 AND pull.head_repo_id = ?2 AND pull.head_branch = ?3
381 AND pull.merged_at IS NULL AND issue.state = 'open'
382 ORDER BY issue.created_at DESC LIMIT 1`,
383 )
384 .get(baseRepoId, headRepoId, branch);
385 return row ? getPull(db, baseRepoId, row.number) : undefined;
386 }
387
368388 /**
369389 * `pullForBranch` for many branches of one repository: each branch's newest
370390 * pull, in two queries per chunk. Branches with none are absent.
@@ -1132,6 +1152,11 @@ export async function mergePull(
11321152 pull: Pull,
11331153 input: {
11341154 strategy: MergeStrategy;
1155 /**
1156 * A one-commit pull on an unmoved base may land as that commit
1157 * (`landsAsIs`), no new commit.
1158 */
1159 fastForward?: boolean;
11351160 message: string;
11361161 actorId: string;
11371162 author: { name: string; email: string };
@@ -1235,10 +1260,16 @@ export async function mergePull(
12351260 // A kept fork lands its combined branch, brought up to the base first:
12361261 // the author's head is its ancestor, so it lands too (#9012).
12371262 const combined =
1238 input.sealedFrom === undefined && fromKeptFork(db, pull)
1263 input.sealedFrom === undefined && input.strategy === "merge" && fromKeptFork(db, pull)
12391264 ? await combine(dir, { number: pull.number, baseOid: status.baseOid, headOid: status.headOid })
12401265 : undefined;
1241 const landed = await merge(dir, {
1266 const asIs =
1267 input.fastForward === true &&
1268 input.expect === undefined &&
1269 (await landsAsIs(dir, status.baseOid, status.headOid));
1270 const landed = asIs
1271 ? await fastForward(dir, { baseBranch: pull.baseBranch, baseOid: status.baseOid, headOid: status.headOid })
1272 : await merge(dir, {
12421273 baseBranch: pull.baseBranch,
12431274 baseOid: status.baseOid,
12441275 headOid: combined?.oid ?? status.headOid,
src/core/repos.ts +12 −2
@@ -141,6 +141,8 @@ export interface Repo {
141141 * merges. Read live inside `mergePull`, like `requireReview`.
142142 */
143143 readonly requireChecks: boolean;
144 /** How a pull lands: `linear` keeps history a line, `merge` records a merge commit. */
145 readonly landing: Landing;
144146 /**
145147 * How much the crew does here on its own (docs/PLANNING.md): `off`, every
146148 * run starts because a person acted; `assisted`, the crew works what is
@@ -208,6 +210,7 @@ interface RepoRow {
208210 protect_default: number;
209211 require_review: number;
210212 require_checks: number;
213 landing: string;
211214 autonomy: string;
212215 autonomy_direction: string;
213216 autonomy_ceiling_usd: number;
@@ -254,6 +257,7 @@ function toRepo(row: RepoRow): Repo {
254257 protectDefault: row.protect_default === 1,
255258 requireReview: row.require_review === 1,
256259 requireChecks: row.require_checks === 1,
260 landing: row.landing === "linear" ? "linear" : "merge",
257261 autonomy: isAutonomy(row.autonomy) ? row.autonomy : "off",
258262 autonomyDirection: row.autonomy_direction,
259263 autonomyCeilingUsd: row.autonomy_ceiling_usd,
@@ -943,7 +947,7 @@ export function listReposToTake(db: Database, viewer: User | undefined, limit =
943947 /** The columns `toRepo` shows; `issue_seq` is not one, since a sealed report moves it. */
944948 const SHOWN = [
945949 "repo.id", "repo.owner_id", "user.username", "repo.name", "repo.visibility", "repo.description",
946 "repo.default_branch", "repo.protect_default", "repo.require_review", "repo.require_checks",
950 "repo.default_branch", "repo.protect_default", "repo.require_review", "repo.require_checks", "repo.landing",
947951 "repo.autonomy", "repo.autonomy_direction", "repo.autonomy_ceiling_usd", "repo.autonomy_slots",
948952 "repo.planner_slots", "repo.reviewer_slots", "repo.polish_share", "repo.autonomy_by",
949953 "repo.crew_outsiders", "repo.decisions_by", "repo.plans_need_nod", "repo.direction_issue_id",
@@ -1336,7 +1340,12 @@ export type DecisionsBy = (typeof DECISIONS_BY)[number];
13361340 export const isDecisionsBy = (v: unknown): v is DecisionsBy =>
13371341 typeof v === "string" && (DECISIONS_BY as readonly string[]).includes(v);
13381342
1343 /** How a repository's pulls land (#12107). */
1344 export type Landing = "linear" | "merge";
1345 export const isLanding = (v: unknown): v is Landing => v === "linear" || v === "merge";
1346
13391347 export interface RepoSettings {
1348 landing?: Landing;
13401349 visibility?: "public" | "private";
13411350 description?: string;
13421351 protectDefault?: boolean;
@@ -1384,7 +1393,7 @@ export function updateRepoSettings(
13841393 default_branch = ?4, require_review = ?5, require_checks = ?7,
13851394 autonomy = ?8, autonomy_direction = ?9, autonomy_ceiling_usd = ?10,
13861395 autonomy_slots = ?11, autonomy_by = ?12, crew_outsiders = ?13, decisions_by = ?14,
1387 plans_need_nod = ?15, planner_slots = ?16, reviewer_slots = ?17, polish_share = ?18
1396 plans_need_nod = ?15, planner_slots = ?16, reviewer_slots = ?17, polish_share = ?18, landing = ?19
13881397 WHERE id = ?6`,
13891398 ).run(
13901399 settings.visibility ?? repo.visibility,
@@ -1405,6 +1414,7 @@ export function updateRepoSettings(
14051414 settings.plannerSlots ?? repo.plannerSlots,
14061415 settings.reviewerSlots ?? repo.reviewerSlots,
14071416 settings.polishShare ?? repo.polishShare,
1417 settings.landing ?? repo.landing,
14081418 );
14091419 forgetListings(db);
14101420 return getRepoById(db, repo.id)!;
src/git/hook-protocol.ts +21
@@ -22,6 +22,8 @@ export interface HookRequest {
2222 /** The forge's address as the pusher reached it, for the URLs a push prints. */
2323 readonly pushOrigin?: string;
2424 readonly updates: readonly RefUpdate[];
25 /** The `-o` options of the push (`git push -o pull`), as git hands them to a hook. */
26 readonly pushOptions?: readonly string[];
2527 /**
2628 * Quarantine environment forwarded from the hook process. Any git command
2729 * evaluating not-yet-accepted objects (ancestry checks in pre-receive)
@@ -73,3 +75,22 @@ export function parseHookStdin(text: string): RefUpdate[] {
7375 }
7476
7577 export const ZERO_OID = /^0+$/;
78
79 /**
80 * The push options git put in a hook's environment: `GIT_PUSH_OPTION_COUNT`
81 * and `GIT_PUSH_OPTION_<n>`, bounded.
82 */
83 export function pushOptionsFrom(env: Record<string, string | undefined>): string[] {
84 const count = Number(env["GIT_PUSH_OPTION_COUNT"] ?? "0");
85 const out: string[] = [];
86 if (!Number.isInteger(count) || count < 1) return out;
87 for (let i = 0; i < Math.min(count, 8); i++) {
88 const value = env[`GIT_PUSH_OPTION_${i}`];
89 if (typeof value === "string" && value.length <= 100) out.push(value);
90 }
91 return out;
92 }
93
94 /** Whether the push asked to open its pull: `-o pull`. */
95 export const asksForPull = (options: readonly string[] | undefined): boolean =>
96 (options ?? []).some((o) => o.trim() === "pull");
src/git/hooks.ts +2
@@ -15,6 +15,7 @@ import {
1515 FORWARDED_GIT_ENV,
1616 HOOK_SECRET_HEADER,
1717 parseHookStdin,
18 pushOptionsFrom,
1819 } from "./hook-protocol.ts";
1920
2021 const HOOK_NAMES = ["pre-receive", "post-receive"] as const;
@@ -138,6 +139,7 @@ export async function runHookClient(hook: HookName): Promise<never> {
138139 : {}),
139140 ...(process.env.FORGE_PUSH_ORIGIN !== undefined ? { pushOrigin: process.env.FORGE_PUSH_ORIGIN } : {}),
140141 updates: parseHookStdin(stdin),
142 pushOptions: pushOptionsFrom(process.env),
141143 gitEnv,
142144 }),
143145 });
src/git/merge.ts +33
@@ -265,6 +265,39 @@ export async function merge(
265265 return ok({ mergeOid, previousOid: input.baseOid });
266266 }
267267
268 /**
269 * Whether `headOid` is exactly one commit on top of `baseOid` that may land
270 * as it is: the same history, no new commit. Its author must not be an
271 * agent's placeholder address (`.invalid`) and its subject must carry no pull
272 * number, which a landing gives its own (#12107).
273 */
274 export async function landsAsIs(dir: string, baseOid: string, headOid: string): Promise<boolean> {
275 if (!isSafeOid(baseOid) || !isSafeOid(headOid)) return false;
276 const log = await runGit(["log", "-1", "--format=%P%n%ae%n%s", headOid], { cwd: dir });
277 if (!log.ok) return false;
278 const [parents = "", email = "", subject = ""] = log.value.split("\n");
279 return (
280 parents.trim() === baseOid &&
281 !/\.invalid$/i.test(email.trim()) &&
282 !/\(#\d+\)\s*$/.test(subject)
283 );
284 }
285
286 /** Move the base branch to `headOid` and nothing else, only if it is still at `baseOid`. */
287 export async function fastForward(
288 dir: string,
289 input: { baseBranch: string; baseOid: string; headOid: string },
290 ): Promise<Result<MergeResult, MergeError>> {
291 if (!isSafeRef(input.baseBranch) || !isSafeOid(input.baseOid) || !isSafeOid(input.headOid))
292 return err({ kind: "unsafe" });
293 const update = await runGit(
294 ["update-ref", `refs/heads/${input.baseBranch}`, input.headOid, input.baseOid],
295 { cwd: dir },
296 );
297 if (!update.ok) return err({ kind: "base_moved" });
298 return ok({ mergeOid: input.headOid, previousOid: input.baseOid });
299 }
300
268301 /** Resolve a ref or rev to its full object id. */
269302 export async function resolveOid(
270303 dir: string,
src/git/smart-http.ts +13 −2
@@ -50,14 +50,23 @@ const UPLOAD_PACK_CONFIG = [
5050 "uploadpack.keepAlive=5",
5151 ] as const;
5252
53 /**
54 * Receive-pack offers push options (`git push -o pull`): the hook reads them
55 * from the environment and a fork's push opens its pull. `-c`, as above, so
56 * the blanked global config stays blank.
57 */
58 const RECEIVE_PACK_CONFIG = ["-c", "receive.advertisePushOptions=true"] as const;
59
5360 /** The argv that runs `service`: `git [config] <subcommand> …args`. */
5461 export function serviceCommand(
5562 service: GitService,
5663 args: readonly string[],
64 pushOptions = true,
5765 ): string[] {
5866 // "git-upload-pack" the service is "upload-pack" the subcommand.
5967 const subcommand = service.slice(4);
60 const config = service === "git-upload-pack" ? UPLOAD_PACK_CONFIG : [];
68 const config =
69 service === "git-upload-pack" ? UPLOAD_PACK_CONFIG : pushOptions ? RECEIVE_PACK_CONFIG : [];
6170 return ["git", ...config, subcommand, ...args];
6271 }
6372
@@ -77,6 +86,8 @@ export interface SmartHttpOptions {
7786 * whole time. Upload-pack only; a push's pack may pause. @default 30_000
7887 */
7988 idleMs?: number;
89 /** Offer push options (`git push -o`); a mirror's push never takes them. @default true */
90 pushOptions?: boolean;
8091 /** Given back when the transfer ends, however it ends (a stranger's share). */
8192 release?: () => void;
8293 }
@@ -188,7 +199,7 @@ async function spawnService(
188199 try {
189200 pipe = await pullPipe(() => proc.kill("SIGKILL"));
190201 proc = Bun.spawn(
191 serviceCommand(service, ["--stateless-rpc", ...extraArgs, repoDir]),
202 serviceCommand(service, ["--stateless-rpc", ...extraArgs, repoDir], options.pushOptions),
192203 {
193204 env: gitEnv(env),
194205 stdin: "pipe",
src/git/smart-ssh.ts +3 −1
@@ -114,6 +114,8 @@ export interface GitChannel {
114114 }
115115
116116 export interface GitServiceOptions {
117 /** Offer push options (`git push -o`); a mirror's push never takes them. @default true */
118 pushOptions?: boolean;
117119 /** Extra env for the spawned git process (hook identity, GIT_PROTOCOL). */
118120 env?: Record<string, string>;
119121 /**
@@ -145,7 +147,7 @@ export function runGitService(
145147 let proc: Bun.Subprocess<"pipe", number, "pipe">;
146148 const pipe = pullPipeSync(() => proc.kill("SIGKILL"));
147149 try {
148 proc = Bun.spawn(serviceCommand(service, [dir]), {
150 proc = Bun.spawn(serviceCommand(service, [dir], options.pushOptions), {
149151 env: gitEnv(options.env),
150152 stdin: "pipe",
151153 stdout: pipe.writeFd,
src/http/api.ts +4
@@ -178,6 +178,7 @@ import {
178178 type RepoCounts,
179179 canWrite,
180180 getRepoById,
181 isLanding,
181182 } from "../core/repos.ts";
182183 import { CODE_BUDGET_MS, greppable, searchKind, searchNote } from "../core/search.ts";
183184 import { strangerCodeSearch } from "./strangers.ts";
@@ -1674,6 +1675,8 @@ export function apiRoutes(router: Router<Handler>): void {
16741675 // The caller can already read this repository, so saying no is no leak.
16751676 if (input.archived !== undefined && !canAdmin(ctx.app.db, repo, ctx.auth.user))
16761677 return apiError("forbidden", 403, "only an admin archives or wakes");
1678 if (input.landing !== undefined && !isLanding(input.landing))
1679 return apiError("bad_landing", 422, "landing is linear or merge");
16771680 if (input.archived !== undefined && typeof input.archived !== "boolean")
16781681 return apiError("bad_archived", 422, "archived is true or false");
16791682 if (input.decisionsBy !== undefined && !isDecisionsBy(input.decisionsBy))
@@ -1726,6 +1729,7 @@ export function apiRoutes(router: Router<Handler>): void {
17261729 ...(typeof input.requireChecks === "boolean"
17271730 ? { requireChecks: input.requireChecks }
17281731 : {}),
1732 ...(isLanding(input.landing) ? { landing: input.landing } : {}),
17291733 ...(typeof input.description === "string"
17301734 ? { description: input.description }
17311735 : {}),
src/http/server.ts +38 −11
@@ -80,7 +80,7 @@ import {
8080 getRepo,
8181 getRepoById,
8282 } from "../core/repos.ts";
83 import { trackPullHead } from "../core/pulls.ts";
83 import { pullForBranch, trackPullHead } from "../core/pulls.ts";
8484 import { recombine } from "../core/recombine.ts";
8585 import { createRepo, repositoryLimitMessage, resolveOwner } from "../actions/repos.ts";
8686 import { sessionOf } from "../core/sessions.ts";
@@ -100,9 +100,12 @@ import { running } from "../kernel/version.ts";
100100 import { runGit } from "../git/env.ts";
101101 import { noteRefusedInternal } from "../kernel/internal_refused.ts";
102102 import { CONTRIBUTE_PUSH_MAX_BYTES } from "../core/contribute_token.ts";
103 import { forkPushLines } from "../actions/fork_push.ts";
103104 import { contribPushPulls, contribPushRefusal } from "../actions/contribute_push.ts";
104105 import { pushCap, storageChanged, storageRefusal, storageRefusedText, storageSentence } from "../core/storage.ts";
105106 import { hookEnv, ZERO_OID, type HookRequest } from "../git/hooks.ts";
107 import { carryApproval } from "../core/carry_approval.ts";
108 import { resolveOid } from "../git/merge.ts";
106109 import { repoDir } from "../git/repos.ts";
107110 import {
108111 advertiseRefs,
@@ -119,6 +122,7 @@ import {
119122 } from "./strangers.ts";
120123 import { timingSafeEqual } from "node:crypto";
121124 import {
125 asksForPull,
122126 forwardedGitEnv,
123127 HOOK_SECRET_HEADER,
124128 } from "../git/hook-protocol.ts";
@@ -1220,7 +1224,10 @@ export function buildRouter(): Router<Handler> {
12201224 return new Response("smart http only", { status: 400 });
12211225 return withGitAccess(ctx, service, (dir) =>
12221226 asStranger(ctx, (options) =>
1223 advertiseRefs(dir, service, ctx.request, options),
1227 advertiseRefs(dir, service, ctx.request, {
1228 ...options,
1229 pushOptions: getRepo(ctx.app.db, ctx.params.owner!, ctx.params.name!)?.origin === null,
1230 }),
12241231 ),
12251232 );
12261233 });
@@ -1241,9 +1248,8 @@ export function buildRouter(): Router<Handler> {
12411248 repoId,
12421249 actorId: ctx.auth.user?.id ?? "",
12431250 ...(ctx.auth.proposal ? { proposalChild: ctx.auth.proposal.child } : {}),
1244 ...(ctx.auth.contribute
1245 ? { contribAgent: ctx.auth.contribute.namespace, pushOrigin: publicUrl(ctx.request, ctx.app.config.url).origin }
1246 : {}),
1251 pushOrigin: publicUrl(ctx.request, ctx.app.config.url).origin,
1252 ...(ctx.auth.contribute ? { contribAgent: ctx.auth.contribute.namespace } : {}),
12471253 }),
12481254 ...cap,
12491255 },
@@ -2270,12 +2276,16 @@ async function handleHook(app: App, request: Request): Promise<Response> {
22702276 for (const update of input.updates) {
22712277 if (!update.ref.startsWith("refs/heads/") || ZERO_OID.test(update.newOid))
22722278 continue;
2273 trackPullHead(
2274 app.db,
2275 repo.id,
2276 update.ref.slice("refs/heads/".length),
2277 update.newOid,
2278 );
2279 const branch = update.ref.slice("refs/heads/".length);
2280 // A force-push that only rebases keeps the pull's approval (#12107).
2281 const open = pullForBranch(app.db, repo.id, branch);
2282 if (open !== undefined && !open.crossRepo && open.mergedAt === null && open.state === "open") {
2283 const dir = repoDir(app.reposRoot, repo.ownerName, repo.name);
2284 const baseTip = await resolveOid(dir, `refs/heads/${open.baseBranch}`);
2285 if (baseTip !== undefined)
2286 await carryApproval(app.db, dir, open, baseTip, update.newOid).catch(() => false);
2287 }
2288 trackPullHead(app.db, repo.id, branch, update.newOid);
22792289 }
22802290
22812291 // An outside agent's push opens its pull (or finds it) and prints where it is.
@@ -2296,6 +2306,23 @@ async function handleHook(app: App, request: Request): Promise<Response> {
22962306 })).join("\n");
22972307 }
22982308
2309 // A push to a fork says where its pull is, or opens it when asked (`-o pull`).
2310 if (input.contribAgent === undefined && repo.forkParentId !== null) {
2311 const origin = (() => {
2312 try {
2313 return new URL(input.pushOrigin ?? "").origin;
2314 } catch {
2315 return app.config.url !== undefined ? new URL(app.config.url).origin : `http://localhost:${app.config.port}`;
2316 }
2317 })();
2318 const actor = input.actorId ? getUserById(app.db, input.actorId) : undefined;
2319 const lines = await forkPushLines(app, repo, actor, input.updates, origin, asksForPull(input.pushOptions)).catch((error) => {
2320 app.log.warn("could not offer the fork's pull", { repo: `${repo.ownerName}/${repo.name}`, error: String(error) });
2321 return [] as string[];
2322 });
2323 printed = lines.join("\n");
2324 }
2325
22992326 // post-receive: the push is durable — record it and wake listeners.
23002327 app.bus.append({
23012328 kind: "push",
src/tui/screens/about.ts +22
@@ -141,6 +141,7 @@ export function aboutScreen(
141141 ...movedFromRow(f.db, repo),
142142 ...movingRow(f, repo),
143143 ...outsidersRow(repo),
144 ...landingRow(repo),
144145 ...seatRows(f, repo),
145146 ...maintainerRows(f, repo),
146147 ...aboutFacts(f, src, facts, ref, repo),
@@ -228,6 +229,9 @@ export function aboutScreen(
228229 ),
229230 ]
230231 : []),
232 ...(mayPerson()
233 ? [binding(["l"], repo?.landing === "merge" ? "land as one commit" : "land as a merge commit")]
234 : []),
231235 ...(mayTrail()
232236 ? [
233237 binding(
@@ -457,6 +461,18 @@ export function aboutScreen(
457461 });
458462 return true;
459463 }
464 if (e.key === "l" && repo !== undefined && mayPerson()) {
465 const landing = repo.landing === "merge" ? "linear" : "merge";
466 void updateRepoSettings(f.app, f.user, repo, { landing }).then((done) => {
467 if (done.ok) {
468 repo = getRepoById(f.db, done.value.id) ?? done.value;
469 rows = factRows();
470 ctx.toast(landing === "merge" ? "pulls land as a merge commit" : "pulls land as one commit", "success");
471 } else ctx.toast("that could not be changed", "danger");
472 ctx.invalidate();
473 });
474 return true;
475 }
460476 if (e.key === "V" && repo !== undefined && mayPerson()) {
461477 const at = repo;
462478 const visibility = at.visibility === "public" ? "private" : "public";
@@ -779,6 +795,12 @@ export function outsidersRow(repo: Repo | undefined): [string, string][] {
779795 ]);
780796 }
781797
798 /** How a pull lands here, as the settings page's select says it; every reader sees it. */
799 export function landingRow(repo: Repo | undefined): [string, string][] {
800 if (repo === undefined) return [];
801 return [["lands", repo.landing === "merge" ? "a merge commit" : "one commit, in a line"]];
802 }
803
782804 /** Where a detach from the parent stands (#7452): the Danger zone card's line. */
783805 export function detachRows(repo: Repo | undefined): [string, string][] {
784806 if (repo === undefined) return [];
src/tui/ssh.ts +1 −1
@@ -399,7 +399,7 @@ export async function serveGitCommand(
399399 if (protocol !== undefined && /^[A-Za-z0-9=:._-]{1,200}$/.test(protocol))
400400 env.GIT_PROTOCOL = protocol;
401401
402 await runGitService(dir, parsed.service, session, { env });
402 await runGitService(dir, parsed.service, session, { env, pushOptions: repo.origin === null });
403403 if (write) storageChanged(forge, repo.ownerId);
404404 }
405405
src/web/pages/settings.ts +8 −1
@@ -53,7 +53,7 @@ import {
5353 APP_ENV_NOT_SET,
5454 SCOPE_WORDS,
5555 } from "../../actions/app_env_words.ts";
56 import { AUTONOMY_SAID, checkAutonomy, isDecisionsBy } from "../../core/repos.ts";
56 import { AUTONOMY_SAID, checkAutonomy, isDecisionsBy, isLanding, type Landing } from "../../core/repos.ts";
5757 import { TRANSFER_SAID, TRANSFER_STATUS, renameRepository, transferRepository } from "../../actions/transfer.ts";
5858 import { updateRepoSettings } from "../../actions/repos.ts";
5959 import { appsRuntime } from "../../apps/runtime.ts";
@@ -284,6 +284,12 @@ export async function settingsPage(
284284 html`Of the commit that is head <em>now</em>, so a push after an approval dismisses it. Refused in the merge itself, not just hidden on the button.`)}
285285 ${toggleLine("requireChecks", "Require a passing deploy", repo.requireChecks,
286286 html`The check is the pull request's preview coming up at the commit that is head <em>now</em>. Read live inside the merge, like the review.`)}
287 ${settingLine("landing", html`<input type="hidden" name="landingShown" value="1" />
288 <label for="f-landing">How a pull lands</label>
289 <select class="input" id="f-landing" name="landing" aria-describedby="why-landing">
290 <option value="merge" ${repo.landing === "merge" ? raw("selected") : null}>A merge commit</option>
291 <option value="linear" ${repo.landing === "linear" ? raw("selected") : null}>One commit, in a line</option>
292 </select>`, "For a pull from a fork.")}
287293 ${toggleLine("crewOutsiders", "Outsiders may trigger the crew", repo.crewOutsiders,
288294 "Anyone signed in who can read this repository starts and steers runs by commenting, on your budget. Off, only people who can write here direct the crew; anyone else's words stay on the thread. Turning it on or off is recorded in the activity.",
289295 "crewOutsidersShown")}
@@ -446,6 +452,7 @@ export async function settingsUpdate(ctx: Ctx): Promise<Response> {
446452 // Who directs the crew changes only from a form that shows the switch:
447453 // a form without it (the studio's About card) never turns it off by
448454 // leaving it out.
455 ...(form.get("landingShown") !== null && isLanding(form.get("landing")) ? { landing: form.get("landing") as Landing } : {}),
449456 ...(form.get("crewOutsidersShown") !== null
450457 ? { crewOutsiders: form.get("crewOutsiders") !== null }
451458 : {}),
test/carry-approval.test.ts added +78
@@ -0,0 +1,78 @@
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 { sameChange } from "../src/core/carry_approval.ts";
6 import { serviceCommand } from "../src/git/smart-http.ts";
7 import { gitClientEnv } from "./helpers/git-client.ts";
8
9 const env = gitClientEnv({
10 GIT_AUTHOR_NAME: "T",
11 GIT_AUTHOR_EMAIL: "[email protected]",
12 GIT_COMMITTER_NAME: "T",
13 GIT_COMMITTER_EMAIL: "[email protected]",
14 });
15
16 describe("a verdict carries only to the same change at the same place (#12107)", () => {
17 let dir: string;
18 const git = async (...args: string[]): Promise<string> => {
19 const p = Bun.spawn(["git", ...args], { cwd: dir, env, stdout: "pipe", stderr: "pipe" });
20 const [out, err, code] = await Promise.all([new Response(p.stdout).text(), new Response(p.stderr).text(), p.exited]);
21 if (code !== 0) throw new Error(`git ${args.join(" ")}: ${err}`);
22 return out.trim();
23 };
24 /** A branch off `from` that has `X` inserted after line `at` of the all-`a` file. */
25 async function insertAt(branch: string, from: string, at: number): Promise<string> {
26 await git("checkout", "-q", "-B", branch, from);
27 const lines = (await Bun.file(join(dir, "f.txt")).text()).trimEnd().split("\n");
28 lines.splice(at, 0, "X");
29 await Bun.write(join(dir, "f.txt"), `${lines.join("\n")}\n`);
30 await git("commit", "-qam", `insert at ${at}`);
31 return git("rev-parse", "HEAD");
32 }
33
34 beforeAll(async () => {
35 dir = await mkdtemp(join(tmpdir(), "forge-carry-"));
36 await git("init", "-q", "-b", "main");
37 await Bun.write(join(dir, "f.txt"), `h\n${"a\n".repeat(60)}`);
38 await Bun.write(join(dir, "g.txt"), "g\n");
39 await git("add", ".");
40 await git("commit", "-qm", "seed");
41 });
42 afterAll(() => rm(dir, { recursive: true, force: true }));
43
44 test("the same lines at another place in the same file are another change", async () => {
45 const base = await git("rev-parse", "main");
46 const here = await insertAt("here", base, 10);
47 const there = await insertAt("there", base, 40);
48 expect(await sameChange(dir, base, here, there)).toBe(false);
49 const again = await insertAt("again", base, 10);
50 expect(await sameChange(dir, base, here, again)).toBe(true);
51 });
52
53 test("a base that moved in an untouched file carries; one that moved in the touched file does not", async () => {
54 const base = await git("rev-parse", "main");
55 const old = await insertAt("old", base, 10);
56 await git("checkout", "-q", "-B", "main", base);
57 await Bun.write(join(dir, "g.txt"), "g2\n");
58 await git("commit", "-qam", "main moves, elsewhere");
59 const moved = await git("rev-parse", "main");
60 await git("checkout", "-q", "-B", "rebased", moved);
61 await git("cherry-pick", old);
62 const rebased = await git("rev-parse", "HEAD");
63 expect(await sameChange(dir, moved, old, rebased)).toBe(true);
64
65 await git("checkout", "-q", "-B", "main", moved);
66 await Bun.write(join(dir, "f.txt"), `h\ntop\n${"a\n".repeat(60)}`);
67 await git("commit", "-qam", "main moves, in the same file");
68 const shifted = await git("rev-parse", "main");
69 await git("checkout", "-q", "-B", "rebased2", shifted);
70 await git("cherry-pick", old);
71 expect(await sameChange(dir, shifted, old, await git("rev-parse", "HEAD"))).toBe(false);
72 });
73
74 test("a mirror's push advertises no push options; any other repository's does", () => {
75 expect(serviceCommand("git-receive-pack", ["d"], false).join(" ")).not.toContain("advertisePushOptions");
76 expect(serviceCommand("git-receive-pack", ["d"]).join(" ")).toContain("receive.advertisePushOptions=true");
77 });
78 });
test/combined-fork.test.ts +1
@@ -80,6 +80,7 @@ describe("a conflicting fork pull is combined on our side (#9012)", () => {
8080 await git(["add", "."], join(work, "a"));
8181 await git(["commit", "-m", "seed"], join(work, "a"));
8282 await git(["push", "origin", "main"], join(work, "a"));
83 app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run();
8384 await call("/api/v1/repos/alice/app/forks", bob, {});
8485 });
8586
test/following.test.ts +4 −1
@@ -830,7 +830,10 @@ describe("a fork that follows its parent", () => {
830830 // A clean pair of its own: the fixture's pair has diverged on purpose.
831831 const made = await createRepo(app, alice, { name: "lamp", visibility: "public" });
832832 if (!made.ok) throw new Error(String(made.error));
833 const lamp = made.value;
833 // A fork that follows its parent needs the parent to take its work as a
834 // merge; a one-commit landing would leave the two histories apart.
835 app.db.query("UPDATE repo SET landing = 'merge' WHERE id = ?").run(made.value.id);
836 const lamp = getRepoById(app.db, made.value.id)!;
834837 await commit(alice, lamp, "README.md", "# lamp\n");
835838 const forked = await forkRepo(app, bob, lamp);
836839 if (!forked.ok) throw new Error(String(forked.error));
test/kept-fork-landing.test.ts +5 −2
@@ -20,7 +20,7 @@ const gitEnv = gitClientEnv({
2020 GIT_COMMITTER_EMAIL: "[email protected]",
2121 });
2222
23 describe("a kept fork lands as a true merge (#8935)", () => {
23 describe("a kept fork lands as a true merge when the repository keeps `landing: merge` (#8935, #12107)", () => {
2424 let dataDir: string;
2525 let work: string;
2626 let app: App;
@@ -73,6 +73,7 @@ describe("a kept fork lands as a true merge (#8935)", () => {
7373 await git(["add", "."], join(work, "a"));
7474 await git(["commit", "-m", "seed"], join(work, "a"));
7575 await git(["push", "origin", "main"], join(work, "a"));
76 app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run();
7677 await call("/api/v1/repos/alice/app/forks", bob, {});
7778 });
7879
@@ -170,7 +171,9 @@ describe("a kept fork lands as a true merge (#8935)", () => {
170171 app.db.query("UPDATE user SET is_agent = ?1 WHERE username = ?2").run(on, name);
171172
172173 test("a person's fork is a merge whatever is asked", () => {
173 expect(landingKind(app.db, shape(), "squash")).toBe("merge");
174 expect(landingKind(app.db, shape(), "squash", "merge")).toBe("merge");
175 expect(landingKind(app.db, shape(), "merge", "linear")).toBe("squash");
176 expect(landingKind(app.db, shape(), "merge")).toBe("merge");
174177 });
175178 test("an agent who authors the pull", () => {
176179 asAgent("bob", 1);
test/linear-landing.test.ts added +254
@@ -0,0 +1,254 @@
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 { createApp, type App } from "../src/app.ts";
6 import { serve, type Servers } from "../src/http/server.ts";
7 import { gitClientEnv } from "./helpers/git-client.ts";
8 import { proveAll } from "./helpers/verified.ts";
9 import { promote } from "./helpers/members.ts";
10 import { tokenFor } from "./helpers/tokens.ts";
11 import { bareTitle } from "../src/core/landing_kind.ts";
12 import { asksForPull, pushOptionsFrom } from "../src/git/hook-protocol.ts";
13
14 const env = gitClientEnv({
15 GIT_AUTHOR_NAME: "Jay Moran",
16 GIT_AUTHOR_EMAIL: "[email protected]",
17 GIT_COMMITTER_NAME: "Jay Moran",
18 GIT_COMMITTER_EMAIL: "[email protected]",
19 });
20
21 describe("plain git contributes (#12107)", () => {
22 let dataDir: string;
23 let work: string;
24 let app: App;
25 let servers: Servers;
26 let base: string;
27 let alice = "";
28 let bob = "";
29
30 async function git(args: string[], cwd: string, extra: Record<string, string> = {}): Promise<{ out: string; err: string }> {
31 const p = Bun.spawn(["git", ...args], { cwd, env: { ...env, ...extra }, stdout: "pipe", stderr: "pipe" });
32 const [out, err, code] = await Promise.all([new Response(p.stdout).text(), new Response(p.stderr).text(), p.exited]);
33 if (code !== 0) throw new Error(`git ${args.join(" ")}: ${err}`);
34 return { out: out.trim(), err };
35 }
36 const call = (path: string, token: string, body?: unknown): Promise<Response> =>
37 fetch(`${base}${path}`, {
38 method: body === undefined ? "GET" : "POST",
39 headers: { authorization: `Bearer ${token}`, "content-type": "application/json" },
40 ...(body === undefined ? {} : { body: JSON.stringify(body) }),
41 });
42 const authed = (user: string, token: string) => base.replace("http://", `http://${user}:${token}@`);
43 async function signup(username: string): Promise<string> {
44 await fetch(`${base}/signup`, {
45 method: "POST",
46 body: new URLSearchParams({ username, email: `${username}@f.dev`, password: "linear-pass-1" }),
47 redirect: "manual",
48 });
49 return tokenFor(app, username, "admin", "k");
50 }
51 async function commit(dir: string, file: string, message: string, extra: Record<string, string> = {}): Promise<void> {
52 await Bun.write(join(dir, file), `${file}\n`);
53 await git(["add", "."], dir);
54 await git(["commit", "-m", message], dir, extra);
55 }
56
57 beforeAll(async () => {
58 dataDir = await mkdtemp(join(tmpdir(), "forge-lin-"));
59 work = await mkdtemp(join(tmpdir(), "forge-lin-work-"));
60 app = createApp({ dataDir, port: 0 });
61 servers = await serve(app);
62 base = servers.url;
63 alice = await signup("alice");
64 bob = await signup("bob");
65 proveAll(app);
66 promote(app.db);
67 await call("/api/v1/repos", alice, { name: "app", visibility: "public" });
68 await git(["clone", `${authed("alice", alice)}/alice/app.git`, "a"], work);
69 await commit(join(work, "a"), "one.txt", "seed");
70 await git(["push", "origin", "main"], join(work, "a"));
71 await call("/api/v1/repos/alice/app/forks", bob, {});
72 await git(["clone", `${authed("bob", bob)}/bob/app.git`, "b"], work);
73 });
74
75 afterAll(async () => {
76 servers.stop();
77 app.close();
78 await rm(dataDir, { recursive: true, force: true });
79 await rm(work, { recursive: true, force: true });
80 });
81
82 test("landing is merge until a repository's admin sets it; a bad value is refused; a change is audited", async () => {
83 const repoUrl = "/api/v1/repos/alice/app";
84 const patch = (token: string, body: unknown): Promise<Response> =>
85 fetch(`${base}${repoUrl}`, {
86 method: "PATCH",
87 headers: { authorization: `Bearer ${token}`, "content-type": "application/json" },
88 body: JSON.stringify(body),
89 });
90 const landing = async (): Promise<unknown> => ((await (await call(repoUrl, alice)).json()) as { landing?: unknown }).landing;
91 expect(await landing()).toBe("merge");
92 const bad = await patch(alice, { landing: "squash" });
93 expect(bad.status).toBe(422);
94 expect(await landing()).toBe("merge");
95 expect((await patch(bob, { landing: "linear" })).status).toBeGreaterThanOrEqual(400);
96 expect(await landing()).toBe("merge");
97 expect((await patch(alice, { landing: "linear" })).status).toBe(200);
98 expect(await landing()).toBe("linear");
99 const trail = app.db
100 .query<{ payload: string }, []>("SELECT payload FROM event WHERE kind = 'repo.settings_updated' ORDER BY rowid DESC LIMIT 1")
101 .get();
102 expect(JSON.parse(trail!.payload).changed.landing).toEqual({ from: "merge", to: "linear" });
103 });
104
105 test("pure helpers: push options and titles", () => {
106 expect(pushOptionsFrom({ GIT_PUSH_OPTION_COUNT: "2", GIT_PUSH_OPTION_0: "pull", GIT_PUSH_OPTION_1: "x" })).toEqual(["pull", "x"]);
107 expect(pushOptionsFrom({})).toEqual([]);
108 expect(asksForPull(["pull"])).toBe(true);
109 expect(asksForPull(["other"])).toBe(false);
110 expect(bareTitle("Fix the thing (#2) (#7)")).toBe("Fix the thing");
111 expect(bareTitle("(#3)")).toBe("(#3)");
112 });
113
114 test("a new branch pushed to a fork prints the address that opens the pull; -o pull opens it", async () => {
115 const b = join(work, "b");
116 await git(["checkout", "-b", "offer"], b);
117 await commit(b, "offer.txt", "Offer a thing");
118 const first = await git(["push", "origin", "offer"], b);
119 expect(first.err).toContain("/bob/app/pulls/new?head=offer");
120 expect(first.err).toContain("git push -o pull");
121
122 await git(["checkout", "-b", "opened"], b);
123 await commit(b, "opened.txt", "Open it for me\n\nWith a body.");
124 const second = await git(["push", "-o", "pull", "origin", "opened"], b);
125 expect(second.err).toMatch(/opened pull #\d+: .*\/alice\/app\/pulls\/\d+/);
126 const row = app.db
127 .query<{ title: string }, []>("SELECT title FROM issue WHERE kind = 'pull' ORDER BY number DESC LIMIT 1")
128 .get();
129 expect(row?.title).toBe("Open it for me");
130
131 // A later push to the same branch names the pull instead of offering another.
132 await commit(b, "opened2.txt", "more");
133 const third = await git(["push", "origin", "opened"], b);
134 expect(third.err).toMatch(/pull #\d+ follows this branch/);
135 await git(["checkout", "main"], b);
136 });
137
138 test("a fork's own default branch is not a proposal, even with -o pull", async () => {
139 const b = join(work, "b");
140 const pulls = () => app.db.query<{ n: number }, []>("SELECT count(*) AS n FROM issue WHERE kind = 'pull'").get()!.n;
141 const before = pulls();
142 await git(["checkout", "main"], b);
143 await commit(b, "own-main.txt", "Move my main");
144 const pushed = await git(["push", "-o", "pull", "origin", "main"], b);
145 expect(pushed.err).not.toContain("pull");
146 expect(pulls()).toBe(before);
147 });
148
149 test("-o pull from someone who cannot open a pull upstream says so and opens none", async () => {
150 const b = join(work, "b");
151 const pulls = () => app.db.query<{ n: number }, []>("SELECT count(*) AS n FROM issue WHERE kind = 'pull'").get()!.n;
152 await git(["checkout", "-B", "refused", "main"], b);
153 await commit(b, "refused.txt", "Try anyway");
154 const before = pulls();
155 // The upstream turns private: bob can no longer read it, so no pull opens into it.
156 const archive = (hidden: boolean) =>
157 fetch(`${base}/api/v1/repos/alice/app`, {
158 method: "PATCH",
159 headers: { authorization: `Bearer ${alice}`, "content-type": "application/json" },
160 body: JSON.stringify({ visibility: hidden ? "private" : "public" }),
161 });
162 expect((await archive(true)).status).toBe(200);
163 try {
164 const pushed = await git(["push", "-o", "pull", "origin", "refused"], b);
165 expect(pushed.err).not.toMatch(/opened pull #|pulls\/new/);
166 } finally {
167 expect((await archive(false)).status).toBe(200);
168 }
169 expect(pulls()).toBe(before);
170 });
171
172 test("a one-commit pull on an unmoved base fast-forwards", async () => {
173 const b = join(work, "b");
174 await git(["fetch", `${authed("alice", alice)}/alice/app.git`, "main"], b);
175 await git(["checkout", "-B", "ff", "FETCH_HEAD"], b);
176 await commit(b, "ff.txt", "Add ff");
177 const tip = (await git(["rev-parse", "HEAD"], b)).out;
178 await git(["push", "origin", "ff"], b);
179 const opened = await call("/api/v1/repos/alice/app/pulls", bob, { title: "Add ff", base: "main", head: "ff", headRepo: "bob/app" });
180 expect(opened.status).toBe(201);
181 const { number } = (await opened.json()) as { number: number };
182 const merged = await call(`/api/v1/repos/alice/app/pulls/${number}/merge`, alice, { strategy: "merge" });
183 expect(merged.status).toBe(200);
184 const a = join(work, "a");
185 await git(["pull", "--ff-only", "origin", "main"], a);
186 expect((await git(["rev-parse", "HEAD"], a)).out).toBe(tip);
187 });
188
189 test("a pull on a moved base lands as one commit by its contributor, numbered upstream only", async () => {
190 const b = join(work, "b");
191 const a = join(work, "a");
192 await git(["fetch", `${authed("alice", alice)}/alice/app.git`, "main"], b);
193 await git(["checkout", "-B", "multi", "FETCH_HEAD"], b);
194 const builder = {
195 GIT_AUTHOR_NAME: "builder",
196 GIT_AUTHOR_EMAIL: "[email protected]",
197 GIT_COMMITTER_NAME: "builder",
198 GIT_COMMITTER_EMAIL: "[email protected]",
199 };
200 await commit(b, "m1.txt", "First part (#2)", builder);
201 await commit(b, "m2.txt", "Second part", builder);
202 await git(["push", "origin", "multi"], b);
203 const opened = await call("/api/v1/repos/alice/app/pulls", bob, { title: "Multi part (#2)", base: "main", head: "multi", headRepo: "bob/app" });
204 const { number } = (await opened.json()) as { number: number };
205 await commit(a, "moved.txt", "alice moves on");
206 await git(["push", "origin", "main"], a);
207 const merged = await call(`/api/v1/repos/alice/app/pulls/${number}/merge`, alice, { strategy: "merge" });
208 expect(merged.status).toBe(200);
209 await git(["pull", "--ff-only", "origin", "main"], a);
210 expect((await git(["log", "-1", "--format=%P"], a)).out.split(" ")).toHaveLength(1);
211 expect((await git(["log", "-1", "--format=%s"], a)).out).toBe(`Multi part (#${number})`);
212 expect((await git(["log", "-1", "--format=%an"], a)).out).toBe("bob");
213 expect((await git(["log", "-1", "--format=%P"], a)).out).not.toContain(" ");
214 });
215
216 test("a refresh that changes nothing but the base keeps the approval", async () => {
217 const b = join(work, "b");
218 const a = join(work, "a");
219 await git(["fetch", `${authed("alice", alice)}/alice/app.git`, "main"], b);
220 await git(["checkout", "-B", "keep", "FETCH_HEAD"], b);
221 await commit(b, "keep.txt", "Keep me");
222 await git(["push", "origin", "keep"], b);
223 const opened = await call("/api/v1/repos/alice/app/pulls", bob, { title: "Keep me", base: "main", head: "keep", headRepo: "bob/app" });
224 const { number } = (await opened.json()) as { number: number };
225 const approved = await call(`/api/v1/repos/alice/app/pulls/${number}/review-states`, alice, { state: "approved", body: "ok" });
226 expect(approved.status).toBeLessThan(300);
227
228 await commit(a, "later.txt", "main moves");
229 await git(["push", "origin", "main"], a);
230 await git(["fetch", `${authed("alice", alice)}/alice/app.git`, "main"], b);
231 await git(["rebase", "FETCH_HEAD"], b);
232 const rebased = (await git(["rev-parse", "HEAD"], b)).out;
233 await git(["push", "--force", "origin", "keep"], b);
234 // Reading the pull refreshes its head, as every read does.
235 await call(`/api/v1/repos/alice/app/pulls/${number}`, alice);
236 const stamped = app.db
237 .query<{ commit_oid: string }, [number]>(
238 "SELECT commit_oid FROM review WHERE issue_id = (SELECT id FROM issue WHERE kind = 'pull' AND number = ?1)",
239 )
240 .all(number);
241 expect(stamped.map((r) => r.commit_oid)).toEqual([rebased]);
242
243 // A head that changes something else is a new head: the approval goes stale.
244 await commit(b, "extra.txt", "different change");
245 await git(["push", "origin", "keep"], b);
246 await call(`/api/v1/repos/alice/app/pulls/${number}`, alice);
247 const after = app.db
248 .query<{ commit_oid: string }, [number]>(
249 "SELECT commit_oid FROM review WHERE issue_id = (SELECT id FROM issue WHERE kind = 'pull' AND number = ?1)",
250 )
251 .all(number);
252 expect(after.map((r) => r.commit_oid)).toEqual([rebased]);
253 });
254 });
test/pull-slices-two-forges.test.ts +1
@@ -115,6 +115,7 @@ describe("a fork's pull lands in slices, across two forges (#9043)", () => {
115115 await git(["add", "."], join(dir, "alice"));
116116 await git(["commit", "-m", "seed"], join(dir, "alice"));
117117 await git(["push", "origin", "main"], join(dir, "alice"));
118 a.app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run();
118119
119120 // Forge B: bob's repository is a fork of A's, the same history.
120121 await call(b, "bob", "/api/v1/repos", { name: "app", visibility: "public" });
test/pull-slices.test.ts +1
@@ -76,6 +76,7 @@ describe("a fork's pull lands in slices (#9010)", () => {
7676 await git(["add", "."], join(work, "a"));
7777 await git(["commit", "-m", "seed"], join(work, "a"));
7878 await git(["push", "origin", "main"], join(work, "a"));
79 app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run();
7980 await call("/api/v1/repos/alice/app/forks", bob, {});
8081 });
8182
test/settings-lines.test.ts +2 −2
@@ -38,7 +38,7 @@ afterAll(async () => {
3838 await rm(dir, { recursive: true, force: true });
3939 });
4040
41 const KEYS = ["defaultBranch", "protect", "requireReview", "requireChecks", "crewOutsiders", "polishShare"];
41 const KEYS = ["defaultBranch", "protect", "requireReview", "requireChecks", "landing", "crewOutsiders", "polishShare"];
4242
4343 test("every setting key is one row inside the form that posts to /settings, reason in a details", async () => {
4444 const html = await (
@@ -63,7 +63,7 @@ test("every setting key is one row inside the form that posts to /settings, reas
6363 });
6464
6565 /** The names the form posts beside `card=gates` (matched by gatesForm): on main before #10999 they were exactly these. */
66 const POSTED = ["defaultBranch", "protect", "requireReview", "requireChecks", "crewOutsidersShown", "crewOutsiders", "polishShareShown", "polishShare"];
66 const POSTED = ["defaultBranch", "protect", "requireReview", "requireChecks", "landingShown", "landing", "crewOutsidersShown", "crewOutsiders", "polishShareShown", "polishShare"];
6767
6868 const page = async (): Promise<string> =>
6969 (await fetch(`${servers.url}/olive/proj/settings`, { headers: { cookie, accept: "text/html" } })).text();
test/trusted-fold.test.ts +2 −2
@@ -72,7 +72,7 @@ async function forge(host: string): Promise<Forge> {
7272 ],
7373 });
7474 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
75 app.db.query("UPDATE repo SET autonomy = 'autonomous', autonomy_by = ?2 WHERE id = ?1").run(made.value.id, a.value.id);
75 app.db.query("UPDATE repo SET autonomy = 'autonomous', autonomy_by = ?2, landing = 'merge' WHERE id = ?1").run(made.value.id, a.value.id);
7676 const trusted = addTrust(app, a.value, { host: FROM, account: "jmoran", until: "2099-12-02" });
7777 if (!trusted.ok) throw new Error(trusted.error);
7878 return { app, alice: a.value, repo: getRepoById(app.db, made.value.id)!, gated, dir };
@@ -315,7 +315,7 @@ describe("a trusted account working on this same forge (#9450)", () => {
315315 ],
316316 });
317317 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
318 f.app.db.query("UPDATE repo SET autonomy = 'autonomous', autonomy_by = ?2 WHERE id = ?1").run(made.value.id, f.alice.id);
318 f.app.db.query("UPDATE repo SET autonomy = 'autonomous', autonomy_by = ?2, landing = 'merge' WHERE id = ?1").run(made.value.id, f.alice.id);
319319 open = getRepoById(f.app.db, made.value.id)!;
320320 const people: User[] = [];
321321 for (const username of ["grokbot", "carol"]) {
test/tui-about-landing.test.ts added +88
@@ -0,0 +1,88 @@
1 /**
2 * How a pull lands at the terminal's door (#12107): on the About card a
3 * person who administers the repository presses `l` to switch between a merge
4 * commit and one commit in a line; a writer who does not administer it is not
5 * offered the key.
6 */
7 import { afterAll, beforeAll, 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 { createApp, type App } from "../src/app.ts";
12 import { addTeamMember, createOrg, createTeam } from "../src/core/orgs.ts";
13 import { createRepo, getRepoById, type Repo } from "../src/core/repos.ts";
14 import { createUser, type User } from "../src/core/users.ts";
15 import type { KeyEvent } from "../src/tui/term/input.ts";
16 import type { Ctx, Screen } from "../src/tui/app/app.ts";
17 import { aboutScreen } from "../src/tui/screens/about.ts";
18 import { Screen as Surface } from "../src/tui/term/screen.ts";
19 import { theme } from "../src/tui/ui/theme.ts";
20 import { localSource } from "../src/view/local.ts";
21 import type { Session } from "../src/tui/session.ts";
22 import { quietLogs } from "./helpers/globals.ts";
23
24 quietLogs();
25
26 let dataDir: string;
27 let app: App;
28 let founder: User;
29 let writer: User;
30 let repo: Repo;
31
32 const ok = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => {
33 if (!r.ok) throw new Error(JSON.stringify(r.error));
34 return r.value;
35 };
36
37 const press = (key: string): KeyEvent => ({ type: "key", key, ctrl: false, alt: false, shift: false });
38
39 function card(user: User) {
40 // `redrawn` resolves when the card asks for a redraw: the key's save is done.
41 let redraw!: () => void;
42 const redrawn = new Promise<void>((r) => (redraw = r));
43 const ctx = { theme: theme("dark"), invalidate: () => redraw(), toast() {}, push() {}, pop() {}, copy() {}, hit() {}, setCursor() {} } as unknown as Ctx;
44 const f = { app, db: app.db, user } as unknown as Session;
45 const src = localSource(app, getRepoById(app.db, repo.id)!, user);
46 const screen = aboutScreen(f, ctx, { render() {} } as unknown as Screen, src, undefined, "main");
47 const text = () => {
48 const s = new Surface(120, 40);
49 screen.render({ x: 0, y: 0, w: 120, h: 40 }, s);
50 return s.toText();
51 };
52 return { screen, text, redrawn, label: (k: string) => screen.keys.find((b) => b.keys.includes(k))?.help };
53 }
54
55 beforeAll(async () => {
56 dataDir = await mkdtemp(join(tmpdir(), "forge-tui-landing-"));
57 app = createApp({ dataDir, port: 0 });
58 const make = async (username: string, adminIfFirst = false) =>
59 ok(await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase", adminIfFirst }));
60 await make("founding", true);
61 founder = await make("founder");
62 writer = await make("writer");
63 const org = ok(createOrg(app.db, "acme", founder.id));
64 repo = ok(await createRepo(app.db, app.reposRoot, { ownerId: org.id, name: "site", visibility: "public" }));
65 addTeamMember(app.db, ok(createTeam(app.db, org.id, "Writers", "write")), writer.id);
66 }, 30_000);
67
68 afterAll(async () => {
69 app?.close();
70 await rm(dataDir, { recursive: true, force: true });
71 });
72
73 test("an admin presses l to change how a pull lands; a writer is not offered it", async () => {
74 const c = card(founder);
75 expect(c.text()).toContain("a merge commit");
76 expect(c.label("l")).toBe("land as one commit");
77 expect(c.screen.key!(press("l"))).toBe(true);
78 await c.redrawn;
79 expect(getRepoById(app.db, repo.id)!.landing).toBe("linear");
80 expect(c.text()).toContain("one commit, in a line");
81 expect(c.label("l")).toBe("land as a merge commit");
82
83 const w = card(writer);
84 expect(w.label("l")).toBeUndefined();
85 w.screen.key!(press("l"));
86 await Promise.resolve();
87 expect(getRepoById(app.db, repo.id)!.landing).toBe("linear");
88 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs