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
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/12107-plain-git-is-a-first-class-way-to-contri → main 13 commits fork 1 thread · 1 unresolved
32 files
changed +840
−38
docs/design/attention-budget.json +4 −4
| @@ -235,13 +235,13 @@ | ||
| 235 | 235 | }, |
| 236 | 236 | "crew": { |
| 237 | 237 | "390": { |
| 238 | "words": 730, | |
| 239 | "blocks": 86, | |
| 238 | "words": 742, | |
| 239 | "blocks": 89, | |
| 240 | 240 | "colours": 4 |
| 241 | 241 | }, |
| 242 | 242 | "1280": { |
| 243 | "words": 730, | |
| 244 | "blocks": 86, | |
| 243 | "words": 742, | |
| 244 | "blocks": 89, | |
| 245 | 245 | "colours": 4 |
| 246 | 246 | } |
| 247 | 247 | } |
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"; | ||
| 77 | 77 | import { getUserById, getUserByName, type User } from "../core/users.ts"; |
| 78 | 78 | import { squashCredits } from "../git/coauthors.ts"; |
| 79 | 79 | 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"; | |
| 81 | 81 | import { thankAuthor } from "../origins/thanks.ts"; |
| 82 | 82 | import { repoDir } from "../git/repos.ts"; |
| 83 | 83 | import { isStrangerPull, strangerGuard } from "../crew/stranger.ts"; |
| @@ -514,7 +514,7 @@ export async function mergePull( | ||
| 514 | 514 | if (pull.originHeadRepo !== null || repo.origin !== null) |
| 515 | 515 | return err("mirrored"); |
| 516 | 516 | // 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); | |
| 518 | 518 | // A fork may have pushed since the pull opened; merge what is being |
| 519 | 519 | // proposed now, not the snapshot taken when it was. |
| 520 | 520 | await refreshPullHead(app, repo, pull); |
| @@ -530,8 +530,9 @@ export async function mergePull( | ||
| 530 | 530 | // asked is named in `Requested-by`, the person who landed it in |
| 531 | 531 | // `Co-authored-by`. |
| 532 | 532 | const writtenBy = getUserById(app.db, pull.authorId); |
| 533 | const linearFork = fromKeptFork(app.db, pull) && strategy === "squash"; | |
| 533 | 534 | const agentAuthor = |
| 534 | writtenBy?.kind === "agent" | |
| 535 | writtenBy?.kind === "agent" || (linearFork && writtenBy !== undefined) | |
| 535 | 536 | ? commitIdentity(app.config, writtenBy) |
| 536 | 537 | : undefined; |
| 537 | 538 | const asked = toldBy(app.db, pull); |
| @@ -580,11 +581,12 @@ export async function mergePull( | ||
| 580 | 581 | }), |
| 581 | 582 | }, |
| 582 | 583 | strategy, |
| 584 | ...(linearFork ? { fastForward: true } : {}), | |
| 583 | 585 | ...(expect === undefined ? {} : { expect }), |
| 584 | 586 | ...(stranger ? { headOid: judged.headOid } : {}), |
| 585 | 587 | message: `${ |
| 586 | 588 | strategy === "squash" |
| 587 | ? `${pull.title} (#${pull.number})` | |
| 589 | ? `${bareTitle(pull.title)} (#${pull.number})` | |
| 588 | 590 | : `Merge pull request #${pull.number} from ${pull.headBranch}\n\n${pull.title}` |
| 589 | 591 | }\n\n${trailers}`, |
| 590 | 592 | actorId: writer.value.id, |
src/actions/repos.ts +1
| @@ -376,6 +376,7 @@ const AUDITED = [ | ||
| 376 | 376 | "defaultBranch", |
| 377 | 377 | "requireReview", |
| 378 | 378 | "requireChecks", |
| 379 | "landing", | |
| 379 | 380 | "crewOutsiders", |
| 380 | 381 | "decisionsBy", |
| 381 | 382 | "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 | */ | |
| 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 @@ | ||
| 18 | 18 | * so it is read here as the push it amounts to. |
| 19 | 19 | */ |
| 20 | 20 | |
| 21 | import { carryApproval } from "./carry_approval.ts"; | |
| 21 | 22 | import type { Database } from "bun:sqlite"; |
| 22 | 23 | import type { EventBus, ForgeEvent } from "./events.ts"; |
| 23 | 24 | import { getPull, listOpenPullsFromHead, type Pull } from "./pulls.ts"; |
| @@ -471,6 +472,11 @@ export async function refreshPullHead( | ||
| 471 | 472 | : await fetchInto(baseDir, headDir, `refs/heads/${pull.headBranch}`); |
| 472 | 473 | if (tip === undefined) return undefined; |
| 473 | 474 | 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 | } | |
| 474 | 480 | // The row says where the head is too, so a reader of the pull — the API, |
| 475 | 481 | // a page — sees the same tip the ref holds. |
| 476 | 482 | forge.db |
src/core/landing_kind.ts +20 −3
| @@ -7,7 +7,7 @@ | ||
| 7 | 7 | */ |
| 8 | 8 | |
| 9 | 9 | import type { Database } from "bun:sqlite"; |
| 10 | import { getRepoById } from "./repos.ts"; | |
| 10 | import { getRepoById, type Landing } from "./repos.ts"; | |
| 11 | 11 | import { getUserById } from "./users.ts"; |
| 12 | 12 | import type { Pull } from "./pulls.ts"; |
| 13 | 13 | import type { MergeStrategy } from "../git/merge.ts"; |
| @@ -24,11 +24,28 @@ export function fromKeptFork( | ||
| 24 | 24 | return getUserById(db, head.ownerId)?.kind !== "agent"; |
| 25 | 25 | } |
| 26 | 26 | |
| 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 | */ | |
| 28 | 33 | export function landingKind( |
| 29 | 34 | db: Database, |
| 30 | 35 | pull: Pick<Pull, "crossRepo" | "headRepoId" | "authorId">, |
| 31 | 36 | asked: MergeStrategy, |
| 37 | landing: Landing = "merge", | |
| 32 | 38 | ): 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 | } | |
| 34 | 51 | } |
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"; | ||
| 19 | 19 | import { |
| 20 | 20 | aheadCount, |
| 21 | 21 | contentOnBase, |
| 22 | fastForward, | |
| 22 | 23 | fetchInto, |
| 24 | landsAsIs, | |
| 23 | 25 | merge, |
| 24 | 26 | mergeBase, |
| 25 | 27 | mergeable, |
| @@ -365,6 +367,24 @@ export function pullForBranch( | ||
| 365 | 367 | return row ? getPull(db, repoId, row.number) : undefined; |
| 366 | 368 | } |
| 367 | 369 | |
| 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 | ||
| 368 | 388 | /** |
| 369 | 389 | * `pullForBranch` for many branches of one repository: each branch's newest |
| 370 | 390 | * pull, in two queries per chunk. Branches with none are absent. |
| @@ -1132,6 +1152,11 @@ export async function mergePull( | ||
| 1132 | 1152 | pull: Pull, |
| 1133 | 1153 | input: { |
| 1134 | 1154 | 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; | |
| 1135 | 1160 | message: string; |
| 1136 | 1161 | actorId: string; |
| 1137 | 1162 | author: { name: string; email: string }; |
| @@ -1235,10 +1260,16 @@ export async function mergePull( | ||
| 1235 | 1260 | // A kept fork lands its combined branch, brought up to the base first: |
| 1236 | 1261 | // the author's head is its ancestor, so it lands too (#9012). |
| 1237 | 1262 | const combined = |
| 1238 | input.sealedFrom === undefined && fromKeptFork(db, pull) | |
| 1263 | input.sealedFrom === undefined && input.strategy === "merge" && fromKeptFork(db, pull) | |
| 1239 | 1264 | ? await combine(dir, { number: pull.number, baseOid: status.baseOid, headOid: status.headOid }) |
| 1240 | 1265 | : 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, { | |
| 1242 | 1273 | baseBranch: pull.baseBranch, |
| 1243 | 1274 | baseOid: status.baseOid, |
| 1244 | 1275 | headOid: combined?.oid ?? status.headOid, |
src/core/repos.ts +12 −2
| @@ -141,6 +141,8 @@ export interface Repo { | ||
| 141 | 141 | * merges. Read live inside `mergePull`, like `requireReview`. |
| 142 | 142 | */ |
| 143 | 143 | readonly requireChecks: boolean; |
| 144 | /** How a pull lands: `linear` keeps history a line, `merge` records a merge commit. */ | |
| 145 | readonly landing: Landing; | |
| 144 | 146 | /** |
| 145 | 147 | * How much the crew does here on its own (docs/PLANNING.md): `off`, every |
| 146 | 148 | * run starts because a person acted; `assisted`, the crew works what is |
| @@ -208,6 +210,7 @@ interface RepoRow { | ||
| 208 | 210 | protect_default: number; |
| 209 | 211 | require_review: number; |
| 210 | 212 | require_checks: number; |
| 213 | landing: string; | |
| 211 | 214 | autonomy: string; |
| 212 | 215 | autonomy_direction: string; |
| 213 | 216 | autonomy_ceiling_usd: number; |
| @@ -254,6 +257,7 @@ function toRepo(row: RepoRow): Repo { | ||
| 254 | 257 | protectDefault: row.protect_default === 1, |
| 255 | 258 | requireReview: row.require_review === 1, |
| 256 | 259 | requireChecks: row.require_checks === 1, |
| 260 | landing: row.landing === "linear" ? "linear" : "merge", | |
| 257 | 261 | autonomy: isAutonomy(row.autonomy) ? row.autonomy : "off", |
| 258 | 262 | autonomyDirection: row.autonomy_direction, |
| 259 | 263 | autonomyCeilingUsd: row.autonomy_ceiling_usd, |
| @@ -943,7 +947,7 @@ export function listReposToTake(db: Database, viewer: User | undefined, limit = | ||
| 943 | 947 | /** The columns `toRepo` shows; `issue_seq` is not one, since a sealed report moves it. */ |
| 944 | 948 | const SHOWN = [ |
| 945 | 949 | "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", | |
| 947 | 951 | "repo.autonomy", "repo.autonomy_direction", "repo.autonomy_ceiling_usd", "repo.autonomy_slots", |
| 948 | 952 | "repo.planner_slots", "repo.reviewer_slots", "repo.polish_share", "repo.autonomy_by", |
| 949 | 953 | "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]; | ||
| 1336 | 1340 | export const isDecisionsBy = (v: unknown): v is DecisionsBy => |
| 1337 | 1341 | typeof v === "string" && (DECISIONS_BY as readonly string[]).includes(v); |
| 1338 | 1342 | |
| 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 | ||
| 1339 | 1347 | export interface RepoSettings { |
| 1348 | landing?: Landing; | |
| 1340 | 1349 | visibility?: "public" | "private"; |
| 1341 | 1350 | description?: string; |
| 1342 | 1351 | protectDefault?: boolean; |
| @@ -1384,7 +1393,7 @@ export function updateRepoSettings( | ||
| 1384 | 1393 | default_branch = ?4, require_review = ?5, require_checks = ?7, |
| 1385 | 1394 | autonomy = ?8, autonomy_direction = ?9, autonomy_ceiling_usd = ?10, |
| 1386 | 1395 | 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 | |
| 1388 | 1397 | WHERE id = ?6`, |
| 1389 | 1398 | ).run( |
| 1390 | 1399 | settings.visibility ?? repo.visibility, |
| @@ -1405,6 +1414,7 @@ export function updateRepoSettings( | ||
| 1405 | 1414 | settings.plannerSlots ?? repo.plannerSlots, |
| 1406 | 1415 | settings.reviewerSlots ?? repo.reviewerSlots, |
| 1407 | 1416 | settings.polishShare ?? repo.polishShare, |
| 1417 | settings.landing ?? repo.landing, | |
| 1408 | 1418 | ); |
| 1409 | 1419 | forgetListings(db); |
| 1410 | 1420 | return getRepoById(db, repo.id)!; |
src/git/hook-protocol.ts +21
| @@ -22,6 +22,8 @@ export interface HookRequest { | ||
| 22 | 22 | /** The forge's address as the pusher reached it, for the URLs a push prints. */ |
| 23 | 23 | readonly pushOrigin?: string; |
| 24 | 24 | 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[]; | |
| 25 | 27 | /** |
| 26 | 28 | * Quarantine environment forwarded from the hook process. Any git command |
| 27 | 29 | * evaluating not-yet-accepted objects (ancestry checks in pre-receive) |
| @@ -73,3 +75,22 @@ export function parseHookStdin(text: string): RefUpdate[] { | ||
| 73 | 75 | } |
| 74 | 76 | |
| 75 | 77 | 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 { | ||
| 15 | 15 | FORWARDED_GIT_ENV, |
| 16 | 16 | HOOK_SECRET_HEADER, |
| 17 | 17 | parseHookStdin, |
| 18 | pushOptionsFrom, | |
| 18 | 19 | } from "./hook-protocol.ts"; |
| 19 | 20 | |
| 20 | 21 | const HOOK_NAMES = ["pre-receive", "post-receive"] as const; |
| @@ -138,6 +139,7 @@ export async function runHookClient(hook: HookName): Promise<never> { | ||
| 138 | 139 | : {}), |
| 139 | 140 | ...(process.env.FORGE_PUSH_ORIGIN !== undefined ? { pushOrigin: process.env.FORGE_PUSH_ORIGIN } : {}), |
| 140 | 141 | updates: parseHookStdin(stdin), |
| 142 | pushOptions: pushOptionsFrom(process.env), | |
| 141 | 143 | gitEnv, |
| 142 | 144 | }), |
| 143 | 145 | }); |
src/git/merge.ts +33
| @@ -265,6 +265,39 @@ export async function merge( | ||
| 265 | 265 | return ok({ mergeOid, previousOid: input.baseOid }); |
| 266 | 266 | } |
| 267 | 267 | |
| 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 | ||
| 268 | 301 | /** Resolve a ref or rev to its full object id. */ |
| 269 | 302 | export async function resolveOid( |
| 270 | 303 | dir: string, |
src/git/smart-http.ts +13 −2
| @@ -50,14 +50,23 @@ const UPLOAD_PACK_CONFIG = [ | ||
| 50 | 50 | "uploadpack.keepAlive=5", |
| 51 | 51 | ] as const; |
| 52 | 52 | |
| 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 | ||
| 53 | 60 | /** The argv that runs `service`: `git [config] <subcommand> …args`. */ |
| 54 | 61 | export function serviceCommand( |
| 55 | 62 | service: GitService, |
| 56 | 63 | args: readonly string[], |
| 64 | pushOptions = true, | |
| 57 | 65 | ): string[] { |
| 58 | 66 | // "git-upload-pack" the service is "upload-pack" the subcommand. |
| 59 | 67 | 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 : []; | |
| 61 | 70 | return ["git", ...config, subcommand, ...args]; |
| 62 | 71 | } |
| 63 | 72 | |
| @@ -77,6 +86,8 @@ export interface SmartHttpOptions { | ||
| 77 | 86 | * whole time. Upload-pack only; a push's pack may pause. @default 30_000 |
| 78 | 87 | */ |
| 79 | 88 | idleMs?: number; |
| 89 | /** Offer push options (`git push -o`); a mirror's push never takes them. @default true */ | |
| 90 | pushOptions?: boolean; | |
| 80 | 91 | /** Given back when the transfer ends, however it ends (a stranger's share). */ |
| 81 | 92 | release?: () => void; |
| 82 | 93 | } |
| @@ -188,7 +199,7 @@ async function spawnService( | ||
| 188 | 199 | try { |
| 189 | 200 | pipe = await pullPipe(() => proc.kill("SIGKILL")); |
| 190 | 201 | proc = Bun.spawn( |
| 191 | serviceCommand(service, ["--stateless-rpc", ...extraArgs, repoDir]), | |
| 202 | serviceCommand(service, ["--stateless-rpc", ...extraArgs, repoDir], options.pushOptions), | |
| 192 | 203 | { |
| 193 | 204 | env: gitEnv(env), |
| 194 | 205 | stdin: "pipe", |
src/git/smart-ssh.ts +3 −1
| @@ -114,6 +114,8 @@ export interface GitChannel { | ||
| 114 | 114 | } |
| 115 | 115 | |
| 116 | 116 | export interface GitServiceOptions { |
| 117 | /** Offer push options (`git push -o`); a mirror's push never takes them. @default true */ | |
| 118 | pushOptions?: boolean; | |
| 117 | 119 | /** Extra env for the spawned git process (hook identity, GIT_PROTOCOL). */ |
| 118 | 120 | env?: Record<string, string>; |
| 119 | 121 | /** |
| @@ -145,7 +147,7 @@ export function runGitService( | ||
| 145 | 147 | let proc: Bun.Subprocess<"pipe", number, "pipe">; |
| 146 | 148 | const pipe = pullPipeSync(() => proc.kill("SIGKILL")); |
| 147 | 149 | try { |
| 148 | proc = Bun.spawn(serviceCommand(service, [dir]), { | |
| 150 | proc = Bun.spawn(serviceCommand(service, [dir], options.pushOptions), { | |
| 149 | 151 | env: gitEnv(options.env), |
| 150 | 152 | stdin: "pipe", |
| 151 | 153 | stdout: pipe.writeFd, |
src/http/api.ts +4
| @@ -178,6 +178,7 @@ import { | ||
| 178 | 178 | type RepoCounts, |
| 179 | 179 | canWrite, |
| 180 | 180 | getRepoById, |
| 181 | isLanding, | |
| 181 | 182 | } from "../core/repos.ts"; |
| 182 | 183 | import { CODE_BUDGET_MS, greppable, searchKind, searchNote } from "../core/search.ts"; |
| 183 | 184 | import { strangerCodeSearch } from "./strangers.ts"; |
| @@ -1674,6 +1675,8 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 1674 | 1675 | // The caller can already read this repository, so saying no is no leak. |
| 1675 | 1676 | if (input.archived !== undefined && !canAdmin(ctx.app.db, repo, ctx.auth.user)) |
| 1676 | 1677 | 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"); | |
| 1677 | 1680 | if (input.archived !== undefined && typeof input.archived !== "boolean") |
| 1678 | 1681 | return apiError("bad_archived", 422, "archived is true or false"); |
| 1679 | 1682 | if (input.decisionsBy !== undefined && !isDecisionsBy(input.decisionsBy)) |
| @@ -1726,6 +1729,7 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 1726 | 1729 | ...(typeof input.requireChecks === "boolean" |
| 1727 | 1730 | ? { requireChecks: input.requireChecks } |
| 1728 | 1731 | : {}), |
| 1732 | ...(isLanding(input.landing) ? { landing: input.landing } : {}), | |
| 1729 | 1733 | ...(typeof input.description === "string" |
| 1730 | 1734 | ? { description: input.description } |
| 1731 | 1735 | : {}), |
src/http/server.ts +38 −11
| @@ -80,7 +80,7 @@ import { | ||
| 80 | 80 | getRepo, |
| 81 | 81 | getRepoById, |
| 82 | 82 | } from "../core/repos.ts"; |
| 83 | import { trackPullHead } from "../core/pulls.ts"; | |
| 83 | import { pullForBranch, trackPullHead } from "../core/pulls.ts"; | |
| 84 | 84 | import { recombine } from "../core/recombine.ts"; |
| 85 | 85 | import { createRepo, repositoryLimitMessage, resolveOwner } from "../actions/repos.ts"; |
| 86 | 86 | import { sessionOf } from "../core/sessions.ts"; |
| @@ -100,9 +100,12 @@ import { running } from "../kernel/version.ts"; | ||
| 100 | 100 | import { runGit } from "../git/env.ts"; |
| 101 | 101 | import { noteRefusedInternal } from "../kernel/internal_refused.ts"; |
| 102 | 102 | import { CONTRIBUTE_PUSH_MAX_BYTES } from "../core/contribute_token.ts"; |
| 103 | import { forkPushLines } from "../actions/fork_push.ts"; | |
| 103 | 104 | import { contribPushPulls, contribPushRefusal } from "../actions/contribute_push.ts"; |
| 104 | 105 | import { pushCap, storageChanged, storageRefusal, storageRefusedText, storageSentence } from "../core/storage.ts"; |
| 105 | 106 | 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"; | |
| 106 | 109 | import { repoDir } from "../git/repos.ts"; |
| 107 | 110 | import { |
| 108 | 111 | advertiseRefs, |
| @@ -119,6 +122,7 @@ import { | ||
| 119 | 122 | } from "./strangers.ts"; |
| 120 | 123 | import { timingSafeEqual } from "node:crypto"; |
| 121 | 124 | import { |
| 125 | asksForPull, | |
| 122 | 126 | forwardedGitEnv, |
| 123 | 127 | HOOK_SECRET_HEADER, |
| 124 | 128 | } from "../git/hook-protocol.ts"; |
| @@ -1220,7 +1224,10 @@ export function buildRouter(): Router<Handler> { | ||
| 1220 | 1224 | return new Response("smart http only", { status: 400 }); |
| 1221 | 1225 | return withGitAccess(ctx, service, (dir) => |
| 1222 | 1226 | 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 | }), | |
| 1224 | 1231 | ), |
| 1225 | 1232 | ); |
| 1226 | 1233 | }); |
| @@ -1241,9 +1248,8 @@ export function buildRouter(): Router<Handler> { | ||
| 1241 | 1248 | repoId, |
| 1242 | 1249 | actorId: ctx.auth.user?.id ?? "", |
| 1243 | 1250 | ...(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 } : {}), | |
| 1247 | 1253 | }), |
| 1248 | 1254 | ...cap, |
| 1249 | 1255 | }, |
| @@ -2270,12 +2276,16 @@ async function handleHook(app: App, request: Request): Promise<Response> { | ||
| 2270 | 2276 | for (const update of input.updates) { |
| 2271 | 2277 | if (!update.ref.startsWith("refs/heads/") || ZERO_OID.test(update.newOid)) |
| 2272 | 2278 | 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); | |
| 2279 | 2289 | } |
| 2280 | 2290 | |
| 2281 | 2291 | // 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> { | ||
| 2296 | 2306 | })).join("\n"); |
| 2297 | 2307 | } |
| 2298 | 2308 | |
| 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 | ||
| 2299 | 2326 | // post-receive: the push is durable — record it and wake listeners. |
| 2300 | 2327 | app.bus.append({ |
| 2301 | 2328 | kind: "push", |
src/tui/screens/about.ts +22
| @@ -141,6 +141,7 @@ export function aboutScreen( | ||
| 141 | 141 | ...movedFromRow(f.db, repo), |
| 142 | 142 | ...movingRow(f, repo), |
| 143 | 143 | ...outsidersRow(repo), |
| 144 | ...landingRow(repo), | |
| 144 | 145 | ...seatRows(f, repo), |
| 145 | 146 | ...maintainerRows(f, repo), |
| 146 | 147 | ...aboutFacts(f, src, facts, ref, repo), |
| @@ -228,6 +229,9 @@ export function aboutScreen( | ||
| 228 | 229 | ), |
| 229 | 230 | ] |
| 230 | 231 | : []), |
| 232 | ...(mayPerson() | |
| 233 | ? [binding(["l"], repo?.landing === "merge" ? "land as one commit" : "land as a merge commit")] | |
| 234 | : []), | |
| 231 | 235 | ...(mayTrail() |
| 232 | 236 | ? [ |
| 233 | 237 | binding( |
| @@ -457,6 +461,18 @@ export function aboutScreen( | ||
| 457 | 461 | }); |
| 458 | 462 | return true; |
| 459 | 463 | } |
| 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 | } | |
| 460 | 476 | if (e.key === "V" && repo !== undefined && mayPerson()) { |
| 461 | 477 | const at = repo; |
| 462 | 478 | const visibility = at.visibility === "public" ? "private" : "public"; |
| @@ -779,6 +795,12 @@ export function outsidersRow(repo: Repo | undefined): [string, string][] { | ||
| 779 | 795 | ]); |
| 780 | 796 | } |
| 781 | 797 | |
| 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 | ||
| 782 | 804 | /** Where a detach from the parent stands (#7452): the Danger zone card's line. */ |
| 783 | 805 | export function detachRows(repo: Repo | undefined): [string, string][] { |
| 784 | 806 | if (repo === undefined) return []; |
src/tui/ssh.ts +1 −1
| @@ -399,7 +399,7 @@ export async function serveGitCommand( | ||
| 399 | 399 | if (protocol !== undefined && /^[A-Za-z0-9=:._-]{1,200}$/.test(protocol)) |
| 400 | 400 | env.GIT_PROTOCOL = protocol; |
| 401 | 401 | |
| 402 | await runGitService(dir, parsed.service, session, { env }); | |
| 402 | await runGitService(dir, parsed.service, session, { env, pushOptions: repo.origin === null }); | |
| 403 | 403 | if (write) storageChanged(forge, repo.ownerId); |
| 404 | 404 | } |
| 405 | 405 |
src/web/pages/settings.ts +8 −1
| @@ -53,7 +53,7 @@ import { | ||
| 53 | 53 | APP_ENV_NOT_SET, |
| 54 | 54 | SCOPE_WORDS, |
| 55 | 55 | } 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"; | |
| 57 | 57 | import { TRANSFER_SAID, TRANSFER_STATUS, renameRepository, transferRepository } from "../../actions/transfer.ts"; |
| 58 | 58 | import { updateRepoSettings } from "../../actions/repos.ts"; |
| 59 | 59 | import { appsRuntime } from "../../apps/runtime.ts"; |
| @@ -284,6 +284,12 @@ export async function settingsPage( | ||
| 284 | 284 | 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.`)} |
| 285 | 285 | ${toggleLine("requireChecks", "Require a passing deploy", repo.requireChecks, |
| 286 | 286 | 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.")} | |
| 287 | 293 | ${toggleLine("crewOutsiders", "Outsiders may trigger the crew", repo.crewOutsiders, |
| 288 | 294 | "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.", |
| 289 | 295 | "crewOutsidersShown")} |
| @@ -446,6 +452,7 @@ export async function settingsUpdate(ctx: Ctx): Promise<Response> { | ||
| 446 | 452 | // Who directs the crew changes only from a form that shows the switch: |
| 447 | 453 | // a form without it (the studio's About card) never turns it off by |
| 448 | 454 | // leaving it out. |
| 455 | ...(form.get("landingShown") !== null && isLanding(form.get("landing")) ? { landing: form.get("landing") as Landing } : {}), | |
| 449 | 456 | ...(form.get("crewOutsidersShown") !== null |
| 450 | 457 | ? { crewOutsiders: form.get("crewOutsiders") !== null } |
| 451 | 458 | : {}), |
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)", () => { | ||
| 80 | 80 | await git(["add", "."], join(work, "a")); |
| 81 | 81 | await git(["commit", "-m", "seed"], join(work, "a")); |
| 82 | 82 | await git(["push", "origin", "main"], join(work, "a")); |
| 83 | app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run(); | |
| 83 | 84 | await call("/api/v1/repos/alice/app/forks", bob, {}); |
| 84 | 85 | }); |
| 85 | 86 |
test/following.test.ts +4 −1
| @@ -830,7 +830,10 @@ describe("a fork that follows its parent", () => { | ||
| 830 | 830 | // A clean pair of its own: the fixture's pair has diverged on purpose. |
| 831 | 831 | const made = await createRepo(app, alice, { name: "lamp", visibility: "public" }); |
| 832 | 832 | 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)!; | |
| 834 | 837 | await commit(alice, lamp, "README.md", "# lamp\n"); |
| 835 | 838 | const forked = await forkRepo(app, bob, lamp); |
| 836 | 839 | if (!forked.ok) throw new Error(String(forked.error)); |
test/kept-fork-landing.test.ts +5 −2
| @@ -20,7 +20,7 @@ const gitEnv = gitClientEnv({ | ||
| 20 | 20 | GIT_COMMITTER_EMAIL: "[email protected]", |
| 21 | 21 | }); |
| 22 | 22 | |
| 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)", () => { | |
| 24 | 24 | let dataDir: string; |
| 25 | 25 | let work: string; |
| 26 | 26 | let app: App; |
| @@ -73,6 +73,7 @@ describe("a kept fork lands as a true merge (#8935)", () => { | ||
| 73 | 73 | await git(["add", "."], join(work, "a")); |
| 74 | 74 | await git(["commit", "-m", "seed"], join(work, "a")); |
| 75 | 75 | await git(["push", "origin", "main"], join(work, "a")); |
| 76 | app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run(); | |
| 76 | 77 | await call("/api/v1/repos/alice/app/forks", bob, {}); |
| 77 | 78 | }); |
| 78 | 79 | |
| @@ -170,7 +171,9 @@ describe("a kept fork lands as a true merge (#8935)", () => { | ||
| 170 | 171 | app.db.query("UPDATE user SET is_agent = ?1 WHERE username = ?2").run(on, name); |
| 171 | 172 | |
| 172 | 173 | 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"); | |
| 174 | 177 | }); |
| 175 | 178 | test("an agent who authors the pull", () => { |
| 176 | 179 | 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)", () => { | ||
| 115 | 115 | await git(["add", "."], join(dir, "alice")); |
| 116 | 116 | await git(["commit", "-m", "seed"], join(dir, "alice")); |
| 117 | 117 | await git(["push", "origin", "main"], join(dir, "alice")); |
| 118 | a.app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run(); | |
| 118 | 119 | |
| 119 | 120 | // Forge B: bob's repository is a fork of A's, the same history. |
| 120 | 121 | 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)", () => { | ||
| 76 | 76 | await git(["add", "."], join(work, "a")); |
| 77 | 77 | await git(["commit", "-m", "seed"], join(work, "a")); |
| 78 | 78 | await git(["push", "origin", "main"], join(work, "a")); |
| 79 | app.db.query("UPDATE repo SET landing = 'merge' WHERE name = 'app'").run(); | |
| 79 | 80 | await call("/api/v1/repos/alice/app/forks", bob, {}); |
| 80 | 81 | }); |
| 81 | 82 |
test/settings-lines.test.ts +2 −2
| @@ -38,7 +38,7 @@ afterAll(async () => { | ||
| 38 | 38 | await rm(dir, { recursive: true, force: true }); |
| 39 | 39 | }); |
| 40 | 40 | |
| 41 | const KEYS = ["defaultBranch", "protect", "requireReview", "requireChecks", "crewOutsiders", "polishShare"]; | |
| 41 | const KEYS = ["defaultBranch", "protect", "requireReview", "requireChecks", "landing", "crewOutsiders", "polishShare"]; | |
| 42 | 42 | |
| 43 | 43 | test("every setting key is one row inside the form that posts to /settings, reason in a details", async () => { |
| 44 | 44 | const html = await ( |
| @@ -63,7 +63,7 @@ test("every setting key is one row inside the form that posts to /settings, reas | ||
| 63 | 63 | }); |
| 64 | 64 | |
| 65 | 65 | /** 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"]; | |
| 67 | 67 | |
| 68 | 68 | const page = async (): Promise<string> => |
| 69 | 69 | (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> { | ||
| 72 | 72 | ], |
| 73 | 73 | }); |
| 74 | 74 | 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); | |
| 76 | 76 | const trusted = addTrust(app, a.value, { host: FROM, account: "jmoran", until: "2099-12-02" }); |
| 77 | 77 | if (!trusted.ok) throw new Error(trusted.error); |
| 78 | 78 | 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)", () => { | ||
| 315 | 315 | ], |
| 316 | 316 | }); |
| 317 | 317 | 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); | |
| 319 | 319 | open = getRepoById(f.app.db, made.value.id)!; |
| 320 | 320 | const people: User[] = []; |
| 321 | 321 | 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 | }); |
new above
offline
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.