Public: anyone can read this; the crew can change it.
A squash landing keeps every Co-authored-by its commits carried #9183
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9181-a-squash-landing-keeps-every-co-authored → main 5 commits fork 4 threads · 4 unresolved
6 files
changed +165
−11
docs/features/changelog.md +5 −1
| @@ -12,9 +12,13 @@ One bounded `git log` read, cached per head oid; Server-Timing names `releases` | ||
| 12 | 12 | |
| 13 | 13 | The list is a live region (`pull.merged`, `app.serving`): it swaps in place without a reload. MCP `releases` with `{shipped: true}` answers the same list to anyone. The "no release" alarm reads the same record (`release/landings.ts`), refreshed each minute, and is silent until a read has happened. The older `Release <version>: …` commit read (`release/changelog.ts`) now serves only the terminal's Changelog screen. |
| 14 | 14 | |
| 15 | ## Credit (#9181) | |
| 16 | ||
| 17 | A squash keeps the people behind its commits: the landed message carries every distinct `Co-authored-by` its commits carried and each commit's author, except the crew's agents (`@agents.invalid`), once per address. `/changelog` then shows "· with Jay Moran" on a row for everyone credited on the landed commit beyond whoever landed it (`creditsOf` in `git/coauthors.ts`, one git read, kept per commit); a landing with no one else shows nothing. Only the forge's own repository is read. `GET /api/v1/changelog` carries the same names as `with` on a landing that has them. The terminal's Changelog screen is the release-commit view and does not list landings, so it shows no credit. | |
| 18 | ||
| 15 | 19 | ## For agents |
| 16 | 20 | |
| 17 | `GET /api/v1/changelog` is the page's list as JSON, public, cached five seconds: `{version, commit, live, onItsWay}`, each landing `{repo, number, title, at, live}`. `?repo=all` or `?repo=owner/name` filters; the default is the forge's own repository, as on the page. MCP `releases` with `{shipped: true, repo?}` and `GET /api/v1/releases?shipped=true[&repo=]` answer the same rows with the same default. Only public repositories are read; concurrent requests share one read of git and the merged rows, so an anonymous caller costs no more than a page view. The page links the JSON with `rel="alternate"`. The terminal's Changelog screen is the release-commit view (`release/changelog.ts`); it does not show the landings list. | |
| 21 | `GET /api/v1/changelog` is the page's list as JSON, public, cached five seconds: `{version, commit, live, onItsWay}`, each landing `{repo, number, title, at, live[, with]}`. `?repo=all` or `?repo=owner/name` filters; the default is the forge's own repository, as on the page. MCP `releases` with `{shipped: true, repo?}` and `GET /api/v1/releases?shipped=true[&repo=]` answer the same rows with the same default. Only public repositories are read; concurrent requests share one read of git and the merged rows, so an anonymous caller costs no more than a page view. The page links the JSON with `rel="alternate"`. The terminal's Changelog screen is the release-commit view (`release/changelog.ts`); it does not show the landings list. | |
| 18 | 22 | |
| 19 | 23 | ## As a feed (#8213) |
| 20 | 24 |
src/actions/pulls.ts +5 −7
| @@ -64,7 +64,7 @@ import { approveGuest } from "../origins/guest.ts"; | ||
| 64 | 64 | import { grantRun } from "../core/run_grant.ts"; |
| 65 | 65 | import { commitIdentity } from "../core/commit_identity.ts"; |
| 66 | 66 | import { getUserById, getUserByName, type User } from "../core/users.ts"; |
| 67 | import { coAuthorLines, messagesBetween } from "../git/coauthors.ts"; | |
| 67 | import { squashCredits } from "../git/coauthors.ts"; | |
| 68 | 68 | import { resolveOid, type MergeStrategy } from "../git/merge.ts"; |
| 69 | 69 | import { landingKind } from "../core/landing_kind.ts"; |
| 70 | 70 | import { thankAuthor } from "../origins/thanks.ts"; |
| @@ -485,12 +485,10 @@ export async function mergePull( | ||
| 485 | 485 | // `Co-authored-by` stay credited on the commit that lands. |
| 486 | 486 | const kept = |
| 487 | 487 | strategy === "squash" |
| 488 | ? coAuthorLines( | |
| 489 | await messagesBetween( | |
| 490 | at, | |
| 491 | (await resolveOid(baseDir, `refs/heads/${pull.baseBranch}`)) ?? pull.baseOid, | |
| 492 | (getPull(app.db, repo.id, pull.number) ?? pull).headOid, | |
| 493 | ), | |
| 488 | ? await squashCredits( | |
| 489 | at, | |
| 490 | (await resolveOid(baseDir, `refs/heads/${pull.baseBranch}`)) ?? pull.baseOid, | |
| 491 | (getPull(app.db, repo.id, pull.number) ?? pull).headOid, | |
| 494 | 492 | [lander.email, ...(requester ? [commitIdentity(app.config, requester).email] : [])], |
| 495 | 493 | ) |
| 496 | 494 | : []; |
src/git/coauthors.ts +55
| @@ -34,6 +34,18 @@ export function coAuthorLines(messages: readonly string[], skip: readonly string | ||
| 34 | 34 | return out; |
| 35 | 35 | } |
| 36 | 36 | |
| 37 | /** | |
| 38 | * The authors of `base..head` as `Co-authored-by` lines, the crew's agents | |
| 39 | * (whose address ends `@agents.invalid`) left out: they are the landing's own | |
| 40 | * author. A person who wrote a commit stays credited when it is squashed. | |
| 41 | */ | |
| 42 | export async function authorLinesBetween(dir: string, base: string, head: string): Promise<string[]> { | |
| 43 | if (!isSafeOid(base) || !isSafeOid(head)) return []; | |
| 44 | const r = await runGitCapture(["log", "--format=Co-authored-by: %an <%ae>", "-n", "200", `${base}..${head}`], { cwd: dir }); | |
| 45 | if (r.code !== 0) return []; | |
| 46 | return r.stdout.split("\n").filter((l) => l !== "" && !/@agents\.invalid>$/i.test(l)); | |
| 47 | } | |
| 48 | ||
| 37 | 49 | /** The commit messages of `base..head` in `dir`, newest first; none when git cannot say. */ |
| 38 | 50 | export async function messagesBetween(dir: string, base: string, head: string): Promise<string[]> { |
| 39 | 51 | if (!isSafeOid(base) || !isSafeOid(head)) return []; |
| @@ -41,3 +53,46 @@ export async function messagesBetween(dir: string, base: string, head: string): | ||
| 41 | 53 | if (r.code !== 0) return []; |
| 42 | 54 | return r.stdout.split("\0").filter((m) => m.trim() !== ""); |
| 43 | 55 | } |
| 56 | ||
| 57 | /** What a landed commit says of who else made it, per commit; a commit never changes, so it is asked once. */ | |
| 58 | const credited = new Map<string, readonly string[]>(); | |
| 59 | ||
| 60 | /** | |
| 61 | * The names credited on each landed commit in `oids`, beyond the one who | |
| 62 | * committed it (its own `Co-authored-by` names the lander). Commits git | |
| 63 | * cannot read are left out. | |
| 64 | */ | |
| 65 | export async function creditsOf(dir: string, oids: readonly string[]): Promise<Map<string, readonly string[]>> { | |
| 66 | const out = new Map<string, readonly string[]>(); | |
| 67 | const todo: string[] = []; | |
| 68 | for (const oid of oids) { | |
| 69 | if (!isSafeOid(oid)) continue; | |
| 70 | const known = credited.get(`${dir}\0${oid}`); | |
| 71 | if (known === undefined) todo.push(oid); | |
| 72 | else out.set(oid, known); | |
| 73 | } | |
| 74 | if (todo.length > 0) { | |
| 75 | const r = await runGitCapture(["log", "--no-walk=unsorted", "--format=%H%x01%ce%x01%B%x02", ...todo], { cwd: dir }); | |
| 76 | if (r.code === 0) | |
| 77 | for (const block of r.stdout.split("\x02")) { | |
| 78 | const [oid, committer, message] = block.trim().split("\x01"); | |
| 79 | if (!oid || committer === undefined || message === undefined) continue; | |
| 80 | const names: string[] = []; | |
| 81 | for (const line of coAuthorLines([message], [committer])) { | |
| 82 | const name = /^Co-authored-by: (.*) <[^<>]*>$/.exec(line)?.[1]; | |
| 83 | if (name !== undefined) names.push(name); | |
| 84 | } | |
| 85 | credited.set(`${dir}\0${oid}`, names); | |
| 86 | out.set(oid, names); | |
| 87 | } | |
| 88 | } | |
| 89 | return out; | |
| 90 | } | |
| 91 | ||
| 92 | /** | |
| 93 | * The `Co-authored-by` lines a squash of `base..head` keeps: those its commits | |
| 94 | * carry and its commits' human authors, once per address, less `skip`. | |
| 95 | */ | |
| 96 | export async function squashCredits(dir: string, base: string, head: string, skip: readonly string[]): Promise<string[]> { | |
| 97 | return coAuthorLines([...(await messagesBetween(dir, base, head)), ...(await authorLinesBetween(dir, base, head))], skip); | |
| 98 | } |
src/release/landings.ts +20 −1
| @@ -15,6 +15,7 @@ | ||
| 15 | 15 | import type { Database } from "bun:sqlite"; |
| 16 | 16 | import type { App } from "../app.ts"; |
| 17 | 17 | import { getRepo, type Repo } from "../core/repos.ts"; |
| 18 | import { creditsOf } from "../git/coauthors.ts"; | |
| 18 | 19 | import { runGit } from "../git/env.ts"; |
| 19 | 20 | import { repoDir } from "../git/repos.ts"; |
| 20 | 21 | import { running } from "../kernel/version.ts"; |
| @@ -30,6 +31,8 @@ export interface Landing { | ||
| 30 | 31 | readonly bet?: string; |
| 31 | 32 | /** `account@host` when a trusted source's work folded in (#8937), set by the page that shows it. */ |
| 32 | 33 | readonly from?: string; |
| 34 | /** Others credited on the landed commit (`Co-authored-by`), beyond whoever landed it. */ | |
| 35 | readonly with?: readonly string[]; | |
| 33 | 36 | } |
| 34 | 37 | |
| 35 | 38 | export interface Landings { |
| @@ -208,11 +211,27 @@ async function askLive(app: App, rows: readonly MergedRow[]): Promise<void> { | ||
| 208 | 211 | kept = { own: named, oids, misses, at, commit }; |
| 209 | 212 | } |
| 210 | 213 | |
| 214 | /** The names credited on the forge's own landed commits, by `repo#number`. */ | |
| 215 | async function creditsOfRepo(app: App, own: string, rows: readonly MergedRow[]): Promise<Map<string, readonly string[]>> { | |
| 216 | const [owner = "", name = ""] = own.split("/"); | |
| 217 | const repo = getRepo(app.db, owner, name); | |
| 218 | if (repo === undefined) return new Map(); | |
outdated
|
||
| 219 | const mine = rows.filter((r) => r.repo.toLowerCase() === own.toLowerCase() && r.oid !== "").slice(0, ASKED); | |
| 220 | const byOid = await creditsOf(repoDir(app.reposRoot, repo.ownerName, repo.name), mine.map((r) => r.oid)); | |
| 221 | return new Map(mine.map((r) => [`${r.repo}#${r.number}`, byOid.get(r.oid) ?? []] as const)); | |
| 222 | } | |
| 223 | ||
| 211 | 224 | /** Every landing, live or on its way, as of the running commit. */ |
| 212 | 225 | export async function readLandings(app: App): Promise<Landings> { |
| 213 | 226 | const rows = mergedRows(app.db); |
| 214 | 227 | await askLive(app, rows).catch(() => undefined); |
outdated
|
||
| 215 | 228 | const { live, onItsWay } = split(rows, kept); |
| 229 | const own = kept?.own; | |
| 230 | const credits = own === undefined ? new Map<string, readonly string[]>() : await creditsOfRepo(app, own, rows).catch(() => new Map<string, readonly string[]>()); | |
| 231 | const named = (l: Landing): Landing => { | |
| 232 | const names = credits.get(`${l.repo}#${l.number}`) ?? []; | |
| 233 | return names.length === 0 ? l : { ...l, with: names }; | |
| 234 | }; | |
| 216 | 235 | const { version, commit } = running(); |
| 217 | return { version, commit, own: kept?.own, live, onItsWay }; | |
| 236 | return { version, commit, own, live: live.map(named), onItsWay: onItsWay.map(named) }; | |
| 218 | 237 | } |
src/web/pages/changelog.ts +1 −1
| @@ -32,7 +32,7 @@ function landingRow(l: Landing, own: string | undefined, showRepo: boolean): Htm | ||
| 32 | 32 | return row({ |
| 33 | 33 | href: `/${l.repo}/pulls/${l.number}`, |
| 34 | 34 | title: l.title, |
| 35 | meta: html`${showRepo || l.repo !== own ? html`${l.repo} · ` : null}#${l.number}${l.bet === undefined ? null : html` · ${badge(l.bet, { variant: "outline" })}`}${l.from === undefined ? null : html` · folded in from ${l.from}`}`, | |
| 35 | meta: html`${showRepo || l.repo !== own ? html`${l.repo} · ` : null}#${l.number}${l.bet === undefined ? null : html` · ${badge(l.bet, { variant: "outline" })}`}${l.from === undefined ? null : html` · folded in from ${l.from}`}${l.with === undefined ? null : html` · with ${l.with.join(", ")}`}`, | |
| 36 | 36 | trail: time(l.at), |
| 37 | 37 | }); |
| 38 | 38 | } |
test/coauthors.test.ts +79 −1
| @@ -13,8 +13,10 @@ import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | ||
| 13 | 13 | import { getPull } from "../src/core/pulls.ts"; |
| 14 | 14 | import type { Repo } from "../src/core/repos.ts"; |
| 15 | 15 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; |
| 16 | import { coAuthorLines, COAUTHORS_MAX } from "../src/git/coauthors.ts"; | |
| 16 | import { coAuthorLines, COAUTHORS_MAX, authorLinesBetween, creditsOf } from "../src/git/coauthors.ts"; | |
| 17 | 17 | import { runGitCapture } from "../src/git/env.ts"; |
| 18 | import { serve } from "../src/http/server.ts"; | |
| 19 | import { forgetLive, readLandings } from "../src/release/landings.ts"; | |
| 18 | 20 | import { repoDir } from "../src/git/repos.ts"; |
| 19 | 21 | |
| 20 | 22 | const JAY = "Co-authored-by: Jay Moran <[email protected]>"; |
| @@ -35,6 +37,27 @@ describe("coAuthorLines", () => { | ||
| 35 | 37 | }); |
| 36 | 38 | }); |
| 37 | 39 | |
| 40 | describe("authorLinesBetween", () => { | |
| 41 | test("credits a person who wrote a commit, not the crew's agents", async () => { | |
| 42 | const dir = await mkdtemp(join(tmpdir(), "forge-authors-")); | |
| 43 | try { | |
| 44 | const git = async (who: string, args: string[]) => { | |
| 45 | const p = Bun.spawn(["git", ...args], { cwd: dir, env: { ...process.env, GIT_AUTHOR_NAME: who, GIT_AUTHOR_EMAIL: `${who.toLowerCase().replace(" ", "")}@${who === "Scout" ? "agents.invalid" : "espocorp.com"}`, GIT_COMMITTER_NAME: "c", GIT_COMMITTER_EMAIL: "[email protected]" }, stdout: "pipe", stderr: "pipe" }); | |
| 46 | return (await new Response(p.stdout).text()).trim(); | |
| 47 | }; | |
| 48 | await git("Scout", ["init", "-q", "-b", "main"]); | |
| 49 | await git("Scout", ["commit", "-q", "--allow-empty", "-m", "base"]); | |
| 50 | const base = await git("Scout", ["rev-parse", "HEAD"]); | |
| 51 | await git("Jay Moran", ["commit", "-q", "--allow-empty", "-m", "a"]); | |
| 52 | await git("Scout", ["commit", "-q", "--allow-empty", "-m", "b"]); | |
| 53 | const head = await git("Scout", ["rev-parse", "HEAD"]); | |
| 54 | expect(await authorLinesBetween(dir, base, head)).toEqual(["Co-authored-by: Jay Moran <[email protected]>"]); | |
| 55 | } finally { | |
| 56 | await rm(dir, { recursive: true, force: true }); | |
| 57 | } | |
| 58 | }); | |
| 59 | }); | |
| 60 | ||
| 38 | 61 | describe("a squash landing", () => { |
| 39 | 62 | let dataDir: string; |
| 40 | 63 | let app: App; |
| @@ -89,4 +112,59 @@ describe("a squash landing", () => { | ||
| 89 | 112 | const body = await land("forged", "Add forged\n\nCo-authored-by: Mallory\nSigned-off-by: root <[email protected]>", "squash"); |
| 90 | 113 | expect(body).not.toContain("Mallory"); |
| 91 | 114 | }); |
outdated
This only calls creditsOf directly. Drive it through readLandings or the changelog page so removing the landings.ts and changelog.ts changes would fail it. |
||
| 115 | ||
| 116 | /** A pull from `feature/<slug>` whose one commit is authored by `who`, landed as a squash; the landed message. */ | |
| 117 | const landAuthoredBy = async (slug: string, who: { name: string; email: string }): Promise<string> => { | |
| 118 | const dir = repoDir(app.reposRoot, repo.ownerName, repo.name); | |
| 119 | const env = { GIT_AUTHOR_NAME: who.name, GIT_AUTHOR_EMAIL: who.email, GIT_COMMITTER_NAME: who.name, GIT_COMMITTER_EMAIL: who.email }; | |
| 120 | const tree = (await runGitCapture(["rev-parse", "main^{tree}"], { cwd: dir })).stdout.trim(); | |
| 121 | const blob = Bun.spawnSync(["git", "hash-object", "-w", "--stdin"], { cwd: dir, stdin: new TextEncoder().encode(`${slug}\n`) }).stdout.toString().trim(); | |
| 122 | const mk = Bun.spawnSync(["git", "mktree"], { cwd: dir, stdin: new TextEncoder().encode(`${Bun.spawnSync(["git", "ls-tree", tree], { cwd: dir }).stdout.toString()}100644 blob ${blob}\t${slug}.md\n`) }).stdout.toString().trim(); | |
| 123 | const main = (await runGitCapture(["rev-parse", "main"], { cwd: dir })).stdout.trim(); | |
| 124 | const commit = Bun.spawnSync(["git", "commit-tree", mk, "-p", main, "-m", `Add ${slug}`], { cwd: dir, env: { ...process.env, ...env } }).stdout.toString().trim(); | |
| 125 | await runGitCapture(["update-ref", `refs/heads/feature/${slug}`, commit], { cwd: dir }); | |
| 126 | const opened = await openPull(app, getUserByName(app.db, "scout")!, repo, { title: `Do ${slug}`, body: "Done.", baseBranch: "main", headBranch: `feature/${slug}` }); | |
| 127 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 128 | const merged = await mergePull(app, alice, repo, getPull(app.db, repo.id, opened.value.number)!, "squash"); | |
| 129 | if (!merged.ok) throw new Error(String(merged.error)); | |
| 130 | return (await runGitCapture(["log", "-1", "--format=%B", "main"], { cwd: dir })).stdout; | |
| 131 | }; | |
| 132 | ||
| 133 | test("a squash credits the person who authored its commit, and not an agent", async () => { | |
| 134 | const jay = await landAuthoredBy("by-jay", { name: "Jay Moran", email: "[email protected]" }); | |
| 135 | expect(jay).toContain("Co-authored-by: Jay Moran <[email protected]>"); | |
| 136 | const agent = await landAuthoredBy("by-agent", { name: "Scout", email: "[email protected]" }); | |
| 137 | expect(agent).not.toContain("agents.invalid"); | |
| 138 | }); | |
| 139 | ||
| 140 | test("the changelog names Jay on his landing and no one on one without credit", async () => { | |
| 141 | app.config.forgeRepo = `${repo.ownerName}/${repo.name}`; | |
| 142 | await land("credited", `Add credited\n\n${JAY}`, "squash"); | |
| 143 | await land("uncredited", "Add uncredited", "squash"); | |
| 144 | const dir0 = repoDir(app.reposRoot, repo.ownerName, repo.name); | |
| 145 | const was = process.env["FORGE_COMMIT"]; | |
| 146 | process.env["FORGE_COMMIT"] = (await runGitCapture(["rev-parse", "main"], { cwd: dir0 })).stdout.trim(); | |
| 147 | forgetLive(); | |
| 148 | const read = await readLandings(app); | |
| 149 | const byTitle = (t: string) => [...read.live, ...read.onItsWay].find((l) => l.title === t)!; | |
| 150 | expect(byTitle("Do credited").with).toEqual(["Jay Moran"]); | |
| 151 | expect(byTitle("Do uncredited").with).toBeUndefined(); | |
| 152 | const dir = repoDir(app.reposRoot, repo.ownerName, repo.name); | |
| 153 | const oid = (await runGitCapture(["rev-parse", "main"], { cwd: dir })).stdout.trim(); | |
| 154 | expect((await creditsOf(dir, [oid])).get(oid)).toEqual([]); | |
| 155 | const server = await serve(app); | |
| 156 | try { | |
| 157 | const html = await (await fetch(`${server.url}/changelog`)).text(); | |
| 158 | expect(html).toContain("with Jay Moran"); | |
| 159 | // Three landed pulls carry Jay's credit (a trailer twice, an authorship once); the rest say nothing of it. | |
| 160 | expect(html.match(/with Jay Moran/g)?.length).toBe(3); | |
| 161 | const row = html.slice(html.indexOf("Do uncredited")); | |
| 162 | expect(row.slice(0, row.indexOf("</li>"))).not.toContain(" with "); | |
| 163 | } finally { | |
| 164 | server.stop(); | |
| 165 | if (was === undefined) delete process.env["FORGE_COMMIT"]; | |
| 166 | else process.env["FORGE_COMMIT"] = was; | |
| 167 | forgetLive(); | |
| 168 | } | |
| 169 | }); | |
| 92 | 170 | }); |
new above
offline
squashBase and squashHead are now resolved on every merge, including non-squash strategies (an extra git call). Compute them inside the squash branch only.