Skip to content

Commit ebc6ba7

Browse files
tris203tris203
committed
fix(server): clean squash-merged worktrees using exact PR heads
1 parent 6c8fed3 commit ebc6ba7

11 files changed

Lines changed: 131 additions & 26 deletions

File tree

‎apps/server/src/git/GitManager.test.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): {
526526
"--limit",
527527
String(input.limit),
528528
"--json",
529-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
529+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
530530
],
531531
}).pipe(
532532
Effect.map((result) => {
@@ -549,7 +549,7 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): {
549549
"--limit",
550550
String(input.limit ?? 1),
551551
"--json",
552-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,isCrossRepository,headRepository,headRepositoryOwner",
552+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,isCrossRepository,headRepository,headRepositoryOwner",
553553
],
554554
}).pipe(
555555
Effect.map((result) => JSON.parse(result.stdout) as unknown[]),
@@ -593,7 +593,7 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): {
593593
"view",
594594
input.reference,
595595
"--json",
596-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,isCrossRepository,headRepository,headRepositoryOwner",
596+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,isCrossRepository,headRepository,headRepositoryOwner",
597597
],
598598
}).pipe(
599599
Effect.map((result) => JSON.parse(result.stdout) as GitHubCli.GitHubPullRequestSummary),
@@ -1473,6 +1473,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
14731473
JSON.stringify([
14741474
{
14751475
number: 220,
1476+
headRefOid: "a".repeat(40),
14761477
title: "Shared cache PR",
14771478
url: "https://github.com/pingdotgg/codething-mvp/pull/220",
14781479
baseRefName: "main",
@@ -1484,6 +1485,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
14841485
encodeCliJson([
14851486
{
14861487
number: 221,
1488+
headRefOid: "b".repeat(40),
14871489
title: "New PR on the same branch",
14881490
url: "https://github.com/pingdotgg/codething-mvp/pull/221",
14891491
baseRefName: "main",
@@ -1504,6 +1506,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
15041506

15051507
expect(status.pr?.state).toBe("merged");
15061508
expect(pullRequest?.state).toBe("merged");
1509+
expect(pullRequest?.headSha).toBe("a".repeat(40));
15071510
expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(1);
15081511
const refreshed = yield* manager.branchPullRequest(
15091512
{ cwd: repoDir, branch: "feature/shared-pr-cache" },
@@ -1512,6 +1515,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
15121515
expect(refreshed).toMatchObject({
15131516
number: 221,
15141517
state: "open",
1518+
headSha: "b".repeat(40),
15151519
repositoryKey: "github.com/pingdotgg/codething-mvp",
15161520
});
15171521
expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(2);
@@ -2004,7 +2008,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
20042008
updatedAt: "2026-03-10T07:00:00.000Z",
20052009
});
20062010
expect(ghCalls).toContain(
2007-
"pr list --head statemachine --state all --limit 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
2011+
"pr list --head statemachine --state all --limit 100 --json number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
20082012
);
20092013
}),
20102014
20_000,
@@ -2070,7 +2074,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
20702074
updatedAt: "2026-03-10T07:00:00.000Z",
20712075
});
20722076
expect(ghCalls).toContain(
2073-
"pr list --head main --state all --limit 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
2077+
"pr list --head main --state all --limit 100 --json number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
20742078
);
20752079
}),
20762080
20_000,

‎apps/server/src/git/GitManager.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ export interface GitRemoteStatusOptions extends GitVcsDriver.GitRemoteStatusOpti
8787
}
8888

8989
export type GitBranchPullRequest = NonNullable<VcsStatusResult["pr"]> & {
90+
readonly headSha?: string;
9091
readonly repositoryKey: string | null;
9192
readonly updatedAt: string | null;
9293
readonly closedAt?: string | null;
@@ -188,6 +189,7 @@ interface OpenPrInfo {
188189
}
189190

190191
interface PullRequestInfo extends OpenPrInfo, PullRequestHeadRemoteInfo {
192+
headSha?: string | undefined;
191193
state: "open" | "closed" | "merged";
192194
isDraft?: boolean;
193195
closedAt?: string | null;
@@ -452,6 +454,7 @@ function toPullRequestInfo(summary: ChangeRequest): PullRequestInfo {
452454
url: summary.url,
453455
baseRefName: summary.baseRefName,
454456
headRefName: summary.headRefName,
457+
...(summary.headSha !== undefined ? { headSha: summary.headSha } : {}),
455458
state: summary.state ?? "open",
456459
...(summary.isDraft === true ? { isDraft: true } : {}),
457460
closedAt: summary.closedAt ?? null,
@@ -2286,6 +2289,7 @@ export const make = Effect.gen(function* () {
22862289
}
22872290
return {
22882291
...toStatusPr(latest),
2292+
...(latest.headSha !== undefined ? { headSha: latest.headSha } : {}),
22892293
closedAt: latest.closedAt ?? null,
22902294
mergedAt: latest.mergedAt ?? null,
22912295
// Hosting CLIs can select an upstream repository instead of origin.

‎apps/server/src/orchestration/ThreadSettlementReactor.test.ts‎

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
DEFAULT_SERVER_SETTINGS,
33
EventId,
4+
GitCommandError,
45
ProjectId,
56
ProviderInstanceId,
67
ProviderDriverKind,
@@ -1569,6 +1570,17 @@ describe("storage cleanup", () => {
15691570
"recent",
15701571
"merged",
15711572
"unmerged",
1573+
"squash-exact-head",
1574+
"squash-later-commit",
1575+
"squash-release-base",
1576+
"squash-stack-base",
1577+
"squash-missing-head",
1578+
"squash-lookup-failed",
1579+
"squash-different-branch",
1580+
"squash-stale-merged",
1581+
"squash-closed",
1582+
"squash-head-moved",
1583+
"unchanged-squash",
15721584
"unchanged",
15731585
"unchanged-two-worktrees",
15741586
"diverged",
@@ -1634,6 +1646,31 @@ describe("storage cleanup", () => {
16341646
const thread = makeThread("storage-thread", {
16351647
branch: "feature",
16361648
worktreePath,
1649+
pullRequests:
1650+
protection === "squash-stale-merged"
1651+
? [
1652+
{
1653+
host: "example.test",
1654+
repository: "owner/repository",
1655+
number: 42,
1656+
url: "https://example.test/owner/repository/pull/42",
1657+
source: "manual",
1658+
linkedAt: NOW,
1659+
stack: null,
1660+
snapshot: {
1661+
state: "merged",
1662+
title: "Previously merged",
1663+
headBranch: "feature",
1664+
baseBranch: "main",
1665+
isDraft: false,
1666+
updatedAt: NOW,
1667+
syncedAt: NOW,
1668+
mergedAt: NOW,
1669+
closedAt: null,
1670+
},
1671+
},
1672+
]
1673+
: [],
16371674
latestUserMessageAt:
16381675
protection === "recent" ? "2026-08-26T00:00:00.000Z" : "2026-08-01T00:00:00.000Z",
16391676
...(protection === "session"
@@ -1658,9 +1695,14 @@ describe("storage cleanup", () => {
16581695
const deleteRule = protection.startsWith("deleted");
16591696
let tombstoned = deleteRule && protection !== "deleted-event";
16601697
const removals: string[] = [];
1661-
const mergeRule = protection === "merged" || protection === "unmerged";
1698+
const squash = protection.startsWith("squash-") || protection === "unchanged-squash";
1699+
const mergeRule =
1700+
protection === "merged" ||
1701+
protection === "unmerged" ||
1702+
protection.startsWith("squash-");
16621703
const unchangedRule =
16631704
protection === "unchanged" ||
1705+
protection === "unchanged-squash" ||
16641706
protection === "unchanged-two-worktrees" ||
16651707
protection === "diverged" ||
16661708
protection === "head-moved";
@@ -1814,9 +1856,38 @@ describe("storage cleanup", () => {
18141856
invalidateStatus: () => Effect.void,
18151857
branchPullRequest: (_input, options) => {
18161858
assert.strictEqual(options?.refresh, true);
1817-
return Effect.succeed(
1818-
makeBranchPullRequest(protection === "unmerged" ? "open" : "merged"),
1819-
);
1859+
assert.notStrictEqual(protection, "unchanged-squash");
1860+
if (protection === "squash-lookup-failed") {
1861+
return Effect.fail(
1862+
new GitCommandError({
1863+
operation: "branchPullRequest",
1864+
command: "gh",
1865+
cwd: worktreePath,
1866+
detail: "Host unavailable",
1867+
}),
1868+
);
1869+
}
1870+
return Effect.succeed({
1871+
...makeBranchPullRequest(
1872+
protection === "unmerged" || protection === "squash-stale-merged"
1873+
? "open"
1874+
: protection === "squash-closed"
1875+
? "closed"
1876+
: "merged",
1877+
),
1878+
headRef: protection === "squash-different-branch" ? "other" : "feature",
1879+
baseRef:
1880+
protection === "squash-release-base"
1881+
? "release"
1882+
: protection === "squash-stack-base"
1883+
? "stack-parent"
1884+
: "main",
1885+
...(!squash || protection === "squash-missing-head"
1886+
? {}
1887+
: {
1888+
headSha: (protection === "squash-later-commit" ? "c" : "a").repeat(40),
1889+
}),
1890+
});
18201891
},
18211892
}),
18221893
Layer.mock(OrchestrationEngineService)({
@@ -1870,7 +1941,8 @@ describe("storage cleanup", () => {
18701941
headReads++;
18711942
return {
18721943
commitSha:
1873-
protection === "head-moved" && headReads > 1
1944+
(protection === "head-moved" || protection === "squash-head-moved") &&
1945+
headReads > 1
18741946
? "c".repeat(40)
18751947
: "a".repeat(40),
18761948
};
@@ -1894,7 +1966,9 @@ describe("storage cleanup", () => {
18941966
Effect.succeed({
18951967
exitCode: ChildProcessSpawner.ExitCode(
18961968
input.operation === "StorageCleanup.integratedBranch" &&
1897-
(protection === "diverged" || input.args.at(-1) !== "b".repeat(40))
1969+
(squash ||
1970+
protection === "diverged" ||
1971+
input.args.at(-1) !== "b".repeat(40))
18981972
? 1
18991973
: 0,
19001974
),
@@ -2007,6 +2081,7 @@ describe("storage cleanup", () => {
20072081
protection === "files-disabled" ||
20082082
protection === "files-extended" ||
20092083
protection === "merged" ||
2084+
protection === "squash-exact-head" ||
20102085
protection === "unchanged" ||
20112086
protection === "unchanged-two-worktrees";
20122087
assert.strictEqual(yield* fs.exists(worktreePath), !removed);

‎apps/server/src/sourceControl/GitHubCli.test.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ describe("GitHubCli.listPullRequestsByHead", () => {
232232
url: `https://github.com/acme/web/pull/${number}`,
233233
baseRefName: "main",
234234
headRefName,
235+
headRefOid: "a".repeat(40),
235236
state: "MERGED",
236237
mergedAt: "2026-01-01T00:00:00Z",
237238
updatedAt: "2026-01-02T00:00:00Z",
@@ -286,9 +287,11 @@ describe("GitHubCli.listPullRequestsByHead", () => {
286287
first?.map((pr) => [pr.number, pr.state, pr.headRepositoryNameWithOwner]),
287288
[[7, "merged", "acme/web"]],
288289
);
290+
assert.strictEqual(first?.[0]?.headSha, "a".repeat(40));
289291
assert.deepStrictEqual(second, []);
290292
assert.strictEqual(documents.length, 1);
291293
assert.include(documents[0]!.query, "rateLimit");
294+
assert.include(documents[0]!.query, "headRefOid");
292295
assert.deepStrictEqual(documents[0]!.variables, {
293296
owner: "acme",
294297
name: "web",
@@ -330,6 +333,7 @@ describe("GitHubCli.listPullRequestsByHead", () => {
330333
pullRequests.map((pr) => pr.number),
331334
[8],
332335
);
336+
assert.strictEqual(pullRequests[0]?.headSha, "a".repeat(40));
333337
assert.deepStrictEqual(commands.at(-1), [
334338
"gh",
335339
"pr",
@@ -341,7 +345,7 @@ describe("GitHubCli.listPullRequestsByHead", () => {
341345
"--limit",
342346
"100",
343347
"--json",
344-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
348+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
345349
]);
346350
const empty = yield* gh.listPullRequestsByHead({
347351
cwd: "/repo",
@@ -579,6 +583,7 @@ describe("GitHubCli.layer", () => {
579583
url: "https://github.com/pingdotgg/codething-mvp/pull/42",
580584
baseRefName: "main",
581585
headRefName: "feature/pr-threads",
586+
headRefOid: "a".repeat(40),
582587
state: "OPEN",
583588
isDraft: true,
584589
mergedAt: null,
@@ -607,6 +612,7 @@ describe("GitHubCli.layer", () => {
607612
url: "https://github.com/pingdotgg/codething-mvp/pull/42",
608613
baseRefName: "main",
609614
headRefName: "feature/pr-threads",
615+
headSha: "a".repeat(40),
610616
state: "open",
611617
closedAt: null,
612618
mergedAt: null,
@@ -624,7 +630,7 @@ describe("GitHubCli.layer", () => {
624630
"view",
625631
"#42",
626632
"--json",
627-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
633+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
628634
],
629635
cwd: "/repo",
630636
timeoutMs: 30_000,

‎apps/server/src/sourceControl/GitHubCli.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ export interface GitHubPullRequestSummary {
260260
readonly url: string;
261261
readonly baseRefName: string;
262262
readonly headRefName: string;
263+
readonly headSha?: string;
263264
readonly state?: "open" | "closed" | "merged";
264265
readonly isDraft?: boolean;
265266
readonly closedAt?: string | null;
@@ -417,10 +418,10 @@ function deriveRepositoryCloneUrlsFromCreateOutput(
417418
type PullRequestListState = "open" | "closed" | "merged" | "all";
418419

419420
const PULL_REQUEST_LIST_JSON_FIELDS =
420-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner";
421+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner";
421422
/** The `gh pr list --json` fields above, as GraphQL selects them. */
422423
const PULL_REQUEST_NODE_SELECTION =
423-
"number title url baseRefName headRefName state isDraft mergedAt closedAt updatedAt isCrossRepository headRepository { name nameWithOwner } headRepositoryOwner { login }";
424+
"number title url baseRefName headRefName headRefOid state isDraft mergedAt closedAt updatedAt isCrossRepository headRepository { name nameWithOwner } headRepositoryOwner { login }";
424425
const GRAPHQL_STATES: Record<PullRequestListState, ReadonlyArray<string>> = {
425426
open: ["OPEN"],
426427
closed: ["CLOSED"],
@@ -926,7 +927,7 @@ export const make = Effect.gen(function* () {
926927
"--limit",
927928
String(input.limit ?? 1),
928929
"--json",
929-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,isCrossRepository,headRepository,headRepositoryOwner",
930+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,isCrossRepository,headRepository,headRepositoryOwner",
930931
],
931932
}).pipe(
932933
Effect.map((result) => result.stdout.trim()),
@@ -960,7 +961,7 @@ export const make = Effect.gen(function* () {
960961
"view",
961962
input.reference,
962963
"--json",
963-
"number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
964+
"number,title,url,baseRefName,headRefName,headRefOid,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
964965
],
965966
}).pipe(
966967
Effect.map((result) => result.stdout.trim()),

‎apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ it.effect("lists change request history through the batched head lookup", () =>
152152
url: "https://enterprise.test/acme/web/pull/7",
153153
baseRefName: "main",
154154
headRefName: "feature/merged",
155+
headSha: "a".repeat(40),
155156
state: "merged",
156157
mergedAt: "2026-01-01T00:00:00Z",
157158
updatedAt: Option.some(DateTime.makeUnsafe("2026-01-02T00:00:00.000Z")),
@@ -181,6 +182,7 @@ it.effect("lists change request history through the batched head lookup", () =>
181182
});
182183
assert.strictEqual(changeRequests[0]?.provider, "github");
183184
assert.strictEqual(changeRequests[0]?.state, "merged");
185+
assert.strictEqual(changeRequests[0]?.headSha, "a".repeat(40));
184186
assert.strictEqual(changeRequests[0]?.mergedAt, "2026-01-01T00:00:00Z");
185187
assert.deepStrictEqual(
186188
changeRequests[0]?.updatedAt,

‎apps/server/src/sourceControl/GitHubSourceControlProvider.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ function toChangeRequest(summary: GitHubCli.GitHubPullRequestSummary): ChangeReq
2929
url: summary.url,
3030
baseRefName: summary.baseRefName,
3131
headRefName: summary.headRefName,
32+
...(summary.headSha !== undefined ? { headSha: summary.headSha } : {}),
3233
state: summary.state ?? "open",
3334
...(summary.isDraft === true ? { isDraft: true } : {}),
3435
closedAt: summary.closedAt ?? null,

0 commit comments

Comments
 (0)