Diffinery

codician/diffinerypublic

Open-source, agent-first GitHub and GitHub Actions alternative that runs on your own Cloudflare account: git hosting, pull requests, code review and drop-in GitHub Actions YAML on Workers, Durable Objects, R2 and Containers.

Merged pull requests keep their commits and files; compare takes main~1 #326

Mergedtest-botagenton behalf of cnbrkdmrcimerged into main from claude/merged-pr-diff
9 files changed+69−11base 04dacaa → head a5e35b1
Files (9)
modifiedapps/diffinery/docs/collaboration/pull-requests.md+2−0View file
@@ -193,6 +193,8 @@
193193 | **Checks** | Check runs and commit statuses reported on the head commit, and the workflow runs for this pull request |
194194 | **Files changed** | The diff from the merge base to the head (up to 300 files), where you can comment on lines |
195195
196+Once a pull request is merged, its commits and files are counted from where the base branch was when it merged, so they stay as they were (the base now contains the head). Pull requests imported from GitHub use GitHub's record of the base, or their merge commit's first parent.
197+
196198 The header says `wants to merge into main from fix-pricing-link` (or `merged into` once merged). Agent authors and reviewers are marked with an `agent` badge and `on behalf of` their owner.
197199
198200 Labels, assignees, milestones, comments, mentions, editing the title and description, and subscribing work exactly as on issues. See [Issues](/docs/collaboration/issues).
modifiedapps/diffinery/docs/repositories/browsing-code.md+1−1View file
@@ -270,7 +270,7 @@
270270
271271 Click **Create pull request**. If an open pull request for the same branches exists, the page links to it instead. If the head has no new commits, it says "There isn't anything to compare." See [Pull requests](/docs/collaboration/pull-requests).
272272
273-The API returns the same comparison: `GET /api/v3/repos/acme/website/compare/main...feature/login` (three dots, base first).
273+The API returns the same comparison: `GET /api/v3/repos/acme/website/compare/main...feature/login` (three dots, base first). Either side can be a branch, a tag, a commit SHA (full or abbreviated) or any of these with git's ancestry suffixes: `main~3` (three first parents back), `main^` (the first parent) or `HEAD^2` (a merge's second parent), for example `/compare/main~1...main`.
274274
275275 ## Command palette
276276
modifiedapps/diffinery/src/imports/mapping.ts+3−0View file
@@ -174,6 +174,8 @@
174174 mergedAt: string | null;
175175 mergedBy: string | null;
176176 mergeCommit: string | null;
177+ /** GitHub's `base.sha` of a merged pull request: the base branch's commit its commits and files are counted from. */
178+ baseSha: string | null;
177179 requestedReviewers: string[];
178180 };
179181 }
@@ -263,6 +265,7 @@
263265 mergedAt: p.merged_at ?? null,
264266 mergedBy: merged && p.merged_by ? people.login(p.merged_by) : null,
265267 mergeCommit: merged ? (p.merge_commit_sha ?? null) : null,
268+ baseSha: merged ? (p.base.sha ?? null) : null,
266269 requestedReviewers: p.state === "open" ? (p.requested_reviewers ?? []).map((r) => people.login(r)) : [],
267270 },
268271 };
modifiedapps/diffinery/src/repo/RepoDO.ts+17−0View file
@@ -571,6 +571,23 @@
571571 /** Resolve a branch, tag, full ref, HEAD or (abbreviated) SHA to a commit SHA. */
572572 async resolve(rev: string | undefined | null): Promise<string | null> {
573573 const r = rev && rev.length ? rev : "HEAD";
574+ // Ancestry suffixes, as git takes them: `main~2` (first parents back), `HEAD^2` (the second parent), `v1.0^`.
575+ // Ref names can't hold `~` or `^`, so whatever is before the first one names the commit.
576+ const at = r.search(/[~^]/);
577+ if (at > 0) {
578+ const ops = r.slice(at);
579+ if (!/^([~^]\d{0,4})+$/.test(ops)) return null;
580+ let oid = await this.resolve(r.slice(0, at));
581+ for (const [, op, n] of ops.matchAll(/([~^])(\d*)/g)) {
582+ const count = n === "" ? 1 : Number(n);
583+ for (let i = 0; oid && i < (op === "~" ? count : 1); i++) {
584+ if (op === "^" && count === 0) break;
585+ const parents: string[] = (await readCommit(this.store, oid).catch(() => null))?.parents ?? [];
586+ oid = (op === "~" ? parents[0] : parents[count - 1]) ?? null;
587+ }
588+ }
589+ return oid;
590+ }
574591 let oid: string | null = null;
575592 if (r === "HEAD") oid = this.getRef(this.headRef);
576593 else if (isOid(r)) oid = this.store.has(r) ? r : null;
modifiedapps/diffinery/src/repo/collab.ts+24−7View file
@@ -55,6 +55,7 @@
5555 merged_at: string | null;
5656 merged_by: string | null;
5757 merge_commit: string | null;
58+ merged_base?: string | null;
5859 requested_reviewers: string;
5960 intent: string | null;
6061 auto_merge: string | null;
@@ -707,12 +708,28 @@
707708 return out;
708709 }
709710
711+ /**
712+ * The commit a pull request's commits and files are counted from: its base branch, or for a merged
713+ * one, where the base branch was when it merged (afterwards the base contains the head, and nothing
714+ * would be left). Pull requests merged before that was kept use their merge commit's first parent.
715+ */
716+ private async diffBase(p: PullRow): Promise<string | null> {
717+ if (p.merged) {
718+ const store = this.repo.store;
719+ if (p.merged_base && (await readCommit(store, p.merged_base).catch(() => null))) return p.merged_base;
720+ if (p.merge_commit && p.merge_commit !== p.head_sha) {
721+ const parent = (await readCommit(store, p.merge_commit).catch(() => null))?.parents[0];
722+ if (parent) return parent;
723+ }
724+ }
725+ return this.repo.getRef(`refs/heads/${p.base_ref}`);
726+ }
727+
710728 async pullFiles(number: number) {
711729 const p = this.pullRow(number);
712- const baseSha = this.repo.getRef(`refs/heads/${p.base_ref}`) ?? p.head_sha;
713- const head = p.merged && p.merge_commit ? p.head_sha : p.head_sha;
714- const mb = await mergeBase(this.repo.store, baseSha, head);
715- return { mergeBase: mb, headSha: head, ...(await this.repo.diff(mb, head)) };
730+ const baseSha = (await this.diffBase(p)) ?? p.head_sha;
731+ const mb = await mergeBase(this.repo.store, baseSha, p.head_sha);
732+ return { mergeBase: mb, headSha: p.head_sha, ...(await this.repo.diff(mb, p.head_sha)) };
716733 }
717734
718735 /**
@@ -721,7 +738,7 @@
721738 */
722739 async pullStats(number: number): Promise<{ additions: number; deletions: number; changedFiles: number }> {
723740 const p = this.pullRow(number);
724- const baseSha = this.repo.getRef(`refs/heads/${p.base_ref}`) ?? p.head_sha;
741+ const baseSha = (await this.diffBase(p)) ?? p.head_sha;
725742 const key = `${number}:${baseSha}:${p.head_sha}`;
726743 const hit = this.statsCache.get(key);
727744 if (hit) return hit;
@@ -768,7 +785,7 @@
768785
769786 async pullCommits(number: number) {
770787 const p = this.pullRow(number);
771- const baseSha = this.repo.getRef(`refs/heads/${p.base_ref}`);
788+ const baseSha = await this.diffBase(p);
772789 const mb = baseSha ? await mergeBase(this.repo.store, baseSha, p.head_sha) : null;
773790 const stop = new Set<string>();
774791 if (mb) for (const c of await walkCommits(this.repo.store, [mb], { limit: 100000 })) stop.add(c.oid);
@@ -1089,7 +1106,7 @@
10891106 const r = this.row(number);
10901107 const baseRef = `refs/heads/${p.base_ref}`;
10911108 const t = now();
1092- this.sql.exec("UPDATE pulls SET merged = 1, merged_at = ?, merged_by = ?, merge_commit = ?, auto_merge = NULL WHERE number = ?", t, caller.login, newSha, number);
1109+ this.sql.exec("UPDATE pulls SET merged = 1, merged_at = ?, merged_by = ?, merge_commit = ?, merged_base = ?, auto_merge = NULL WHERE number = ?", t, caller.login, newSha, baseSha, number);
10931110 if (this.repo.queue.isQueued(number)) this.repo.queue.schedule(p.base_ref);
10941111 this.sql.exec("UPDATE issues SET state = 'closed', state_reason = 'merged', closed_at = ?, updated_at = ? WHERE number = ?", t, t, number);
10951112 this.repo.social.record(number, "merged", caller, { commit_id: newSha });
modifiedapps/diffinery/src/repo/imports.ts+4−3View file
@@ -113,11 +113,11 @@
113113 if (i.pull) {
114114 const p = i.pull;
115115 this.sql.exec(
116- `INSERT INTO pulls (number, head_ref, head_repo_id, head_sha, base_ref, draft, merged, merged_at, merged_by, merge_commit, requested_reviewers, imported)
117- VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
116+ `INSERT INTO pulls (number, head_ref, head_repo_id, head_sha, base_ref, draft, merged, merged_at, merged_by, merge_commit, merged_base, requested_reviewers, imported)
117+ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
118118 ON CONFLICT(number) DO UPDATE SET head_ref = excluded.head_ref, head_repo_id = excluded.head_repo_id, head_sha = excluded.head_sha, base_ref = excluded.base_ref, draft = excluded.draft,
119119 merged = excluded.merged, merged_at = excluded.merged_at, merged_by = COALESCE(excluded.merged_by, pulls.merged_by),
120- merge_commit = excluded.merge_commit, requested_reviewers = excluded.requested_reviewers, imported = excluded.imported
120+ merge_commit = excluded.merge_commit, merged_base = excluded.merged_base, requested_reviewers = excluded.requested_reviewers, imported = excluded.imported
121121 WHERE pulls.imported = excluded.imported`,
122122 i.number,
123123 p.headRef,
@@ -130,6 +130,7 @@
130130 p.mergedAt,
131131 p.mergedBy,
132132 p.mergeCommit,
133+ p.baseSha ?? null,
133134 JSON.stringify(p.requestedReviewers),
134135 importId,
135136 );
modifiedapps/diffinery/src/repo/schema.ts+3−0View file
@@ -232,6 +232,9 @@
232232 ["protections", "imported", "TEXT"],
233233 // Auto-merge while it is on (repo/auto-merge.ts AutoMergeState as JSON); null when off.
234234 ["pulls", "auto_merge", "TEXT"],
235+ // The base branch's commit when the pull request was merged (GitHub's base.sha for an imported one):
236+ // a merged pull request's commits and files are counted from it (repo/collab.ts diffBase).
237+ ["pulls", "merged_base", "TEXT"],
235238 ["releases", "imported", "TEXT"],
236239 // "task" for an agent task's sandbox (agentic/): no public sandbox call reaches it, and its git
237240 // credential also needs the person who started the task (task_person) to still be allowed.
modifiedapps/diffinery/test/e2e/e2e.test.mjs+12−0View file
@@ -511,6 +511,14 @@
511511 git(src, ["push", "-q", "origin", "main"]);
512512 const cmp = await ok("GET", `/repos/${owner}/sizes/compare/${first}...main`);
513513 assert.equal(cmp.files.length, sizes.length);
514+ // Relative refs, as git takes them.
515+ for (const spec of ["main~1...main", "main^...main", "main~...HEAD", `${first}...main~0`]) {
516+ const rel = await ok("GET", `/repos/${owner}/sizes/compare/${spec}`);
517+ assert.deepEqual([rel.base_commit.sha, rel.total_commits, rel.files.length], [first, 1, sizes.length], spec);
518+ }
519+ assert.equal((await ok("GET", `/repos/${owner}/sizes/commits/main~1`)).sha, first);
520+ assert.equal((await api("GET", `/repos/${owner}/sizes/compare/main~2...main`)).status, 404, "past the first commit");
521+ assert.equal((await api("GET", `/repos/${owner}/sizes/compare/main^2...main`)).status, 404, "no second parent");
514522 await ok("GET", `/repos/${owner}/sizes/commits/main`);
515523 const clone = join(work, "sizes-clone");
516524 git(work, ["clone", "-q", `${base}/${owner}/sizes.git`, clone]);
@@ -2825,6 +2833,10 @@
28252833 assert.equal(p2.merge_commit_sha, fx.mergeSha);
28262834 assert.equal(p2.head.ref, "feature");
28272835 assert.equal(p2.head.sha, fx.featureSha);
2836+ // Its commits and files are counted from where main was when it merged, not from main now (which contains it).
2837+ assert.deepEqual([p2.commits, p2.changed_files], [1, 1]);
2838+ assert.deepEqual((await ok("GET", `/repos/${owner}/imported-app/pulls/2/commits`)).map((c) => c.sha), [fx.featureSha]);
2839+ assert.deepEqual((await ok("GET", `/repos/${owner}/imported-app/pulls/2/files`)).map((f) => f.filename), ["feature.txt"]);
28282840 const reviews = await ok("GET", `/repos/${owner}/imported-app/pulls/2/reviews`);
28292841 assert.deepEqual(reviews.map((r) => [r.user.login, r.state, r.body]), [["github:bob", "APPROVED", "Looks good"]]);
28302842 const lineComments = await ok("GET", `/repos/${owner}/imported-app/pulls/2/comments`);
modifiedapps/diffinery/test/e2e/security.test.mjs+3−0View file
@@ -1744,6 +1744,9 @@
17441744 // An agent's pull request needs a person's approval before a person merges it.
17451745 await ok("POST", `${R}/pulls/${pr.number}/reviews`, { event: "APPROVE" });
17461746 await ok("PUT", `${R}/pulls/${pr.number}/merge`, { merge_method: "merge" });
1747+ // Merged with a merge commit, main now contains the head, and the pull request still lists its two commits.
1748+ assert.deepEqual((await ok("GET", `${R}/pulls/${pr.number}/commits`)).map((c) => c.commit.message.trim()), ["ci", "notes"]);
1749+ assert.deepEqual([...(await ok("GET", `${R}/pulls/${pr.number}/files`)).map((f) => f.filename)].sort(), [".github/workflows/ci.yml", "notes.txt"]);
17471750 assert.deepEqual(await secretsOfNextJob(), ["DEPLOY", "DIFFINERY_TOKEN", "GITHUB_TOKEN"]);
17481751 // A person's new branch from there has no agent commits of its own.
17491752 const tip = (await ok("GET", `${R}/branches/main`)).commit.sha;