authorgravatar for admin@snappeh.comTristan Knight <admin@snappeh.com> 2026-10-07 18:21:43+01:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-07 10:21:43-07:00
log93e864042cf066f4cdb3efe65dbefaaf0d168894
tree042911da4bfd55f52c15382382db8511094a2d5b
parentd9068d38099fe5cb4f3fed4cb5455fee878db491
signature Signed by PGP key B5690EEEBB952194

fix(server): merged-worktree cleanup removes worktrees after squash merges (#14847)

11 files changed, 119 insertions(+), 8 deletions(-)

apps/server/src/git/GitManager.test.ts+6
......@@ -1368,6 +1368,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
13681368 url: "https://github.com/pingdotgg/t3code/pull/216",
13691369 baseRefName: "main",
13701370 headRefName: "feature/saved-branch",
1371 headRefOid: "a".repeat(40),
13711372 state: "OPEN",
13721373 updatedAt: "2026-04-03T15:00:00Z",
13731374 },
......@@ -1387,6 +1388,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
13871388 url: "https://github.com/pingdotgg/t3code/pull/216",
13881389 baseRef: "main",
13891390 headRef: "feature/saved-branch",
1391 headSha: "a".repeat(40),
13901392 state: "open",
13911393 closedAt: null,
13921394 mergedAt: null,
......@@ -1664,6 +1666,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
16641666 url: "https://github.com/pingdotgg/codething-mvp/pull/220",
16651667 baseRefName: "main",
16661668 headRefName: "feature/shared-pr-cache",
1669 headRefOid: "a".repeat(40),
16671670 state: "MERGED",
16681671 updatedAt: "2026-04-07T15:00:00Z",
16691672 },
......@@ -1675,6 +1678,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
16751678 url: "https://github.com/pingdotgg/codething-mvp/pull/221",
16761679 baseRefName: "main",
16771680 headRefName: "feature/shared-pr-cache",
1681 headRefOid: "b".repeat(40),
16781682 state: "OPEN",
16791683 updatedAt: "2026-04-08T15:00:00Z",
16801684 },
......@@ -1691,6 +1695,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
16911695
16921696 expect(status.pr?.state).toBe("merged");
16931697 expect(pullRequest?.state).toBe("merged");
1698 expect(pullRequest?.headSha).toBe("a".repeat(40));
16941699 expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(1);
16951700 const refreshed = yield* manager.branchPullRequest(
16961701 { cwd: repoDir, branch: "feature/shared-pr-cache" },
......@@ -1699,6 +1704,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
16991704 expect(refreshed).toMatchObject({
17001705 number: 221,
17011706 state: "open",
1707 headSha: "b".repeat(40),
17021708 repositoryKey: "github.com/pingdotgg/codething-mvp",
17031709 });
17041710 expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(2);
apps/server/src/git/GitManager.ts+5
......@@ -95,6 +95,8 @@ export type GitBranchPullRequest = NonNullable<VcsStatusResult["pr"]> & {
9595 readonly updatedAt: string | null;
9696 readonly closedAt?: string | null;
9797 readonly mergedAt?: string | null;
98 /** The pull request's head commit, when the host read reports it. */
99 readonly headSha?: string | null;
98100};
99101
100102interface SourceControlTextGenerationSettings {
......@@ -201,6 +203,7 @@ interface PullRequestInfo extends OpenPrInfo, PullRequestHeadRemoteInfo {
201203 closedAt?: string | null;
202204 mergedAt?: string | null;
203205 updatedAt: Option.Option<DateTime.Utc>;
206 headSha?: string | undefined;
204207}
205208
206209const pullRequestUpdatedAtDescOrder: Order.Order<PullRequestInfo> = Order.mapInput(
......@@ -460,6 +463,7 @@ function toPullRequestInfo(summary: ChangeRequest): PullRequestInfo {
460463 url: summary.url,
461464 baseRefName: summary.baseRefName,
462465 headRefName: summary.headRefName,
466 ...(summary.headSha !== undefined ? { headSha: summary.headSha } : {}),
463467 state: summary.state ?? "open",
464468 ...(summary.isDraft === true ? { isDraft: true } : {}),
465469 closedAt: summary.closedAt ?? null,
......@@ -2321,6 +2325,7 @@ export const make = Effect.gen(function* () {
23212325 ...toStatusPr(latest),
23222326 closedAt: latest.closedAt ?? null,
23232327 mergedAt: latest.mergedAt ?? null,
2328 headSha: latest.headSha ?? null,
23242329 // Hosting CLIs can select an upstream repository instead of origin.
23252330 // The returned PR URL names the repository that actually owns it.
23262331 repositoryKey: pullRequestRepositoryKey(latest.url),
apps/server/src/sourceControl/GitHubCli.test.ts+8-1
......@@ -260,7 +260,12 @@ describe("GitHubCli.listPullRequestsByHead", () => {
260260 Effect.sync(() => {
261261 documents.push(input);
262262 return encodeJson({
263 data: { repository: { h0: { nodes: [node(7, "feature/a")] }, h1: { nodes: [] } } },
263 data: {
264 repository: {
265 h0: { nodes: [{ ...node(7, "feature/a"), headRefOid: "a".repeat(40) }] },
266 h1: { nodes: [] },
267 },
268 },
264269 });
265270 }),
266271 },
......@@ -288,8 +293,10 @@ describe("GitHubCli.listPullRequestsByHead", () => {
288293 first?.map((pr) => pr.number),
289294 [7],
290295 );
296 assert.strictEqual(first?.[0]?.headSha, "a".repeat(40));
291297 assert.deepStrictEqual(second, []);
292298 assert.strictEqual(documents.length, 1);
299 assert.include(documents[0]!.query, "headRefOid");
293300 assert.deepStrictEqual(documents[0]!.variables, {
294301 owner: "acme",
295302 name: "web",
apps/server/src/sourceControl/GitHubCli.ts+2-1
......@@ -183,6 +183,7 @@ export interface GitHubPullRequestSummary {
183183 readonly url: string;
184184 readonly baseRefName: string;
185185 readonly headRefName: string;
186 readonly headSha?: string;
186187 readonly state?: "open" | "closed" | "merged";
187188 readonly isDraft?: boolean;
188189 readonly closedAt?: string | null;
......@@ -337,7 +338,7 @@ type PullRequestListState = "open" | "closed" | "merged" | "all";
337338
338339/** The pull request fields every read selects, in GraphQL. */
339340const PULL_REQUEST_NODE_SELECTION =
340 "number title url baseRefName headRefName state isDraft mergedAt closedAt updatedAt isCrossRepository headRepository { name nameWithOwner } headRepositoryOwner { login }";
341 "number title url baseRefName headRefName headRefOid state isDraft mergedAt closedAt updatedAt isCrossRepository headRepository { name nameWithOwner } headRepositoryOwner { login }";
341342const GRAPHQL_STATES: Record<PullRequestListState, ReadonlyArray<string>> = {
342343 open: ["OPEN"],
343344 closed: ["CLOSED"],
apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts+2
......@@ -142,6 +142,7 @@ it.effect("lists change request history through the batched head lookup", () =>
142142 url: "https://enterprise.test/acme/web/pull/7",
143143 baseRefName: "main",
144144 headRefName: "feature/merged",
145 headSha: "a".repeat(40),
145146 state: "merged",
146147 mergedAt: "2026-01-01T00:00:00Z",
147148 updatedAt: Option.some(DateTime.makeUnsafe("2026-01-02T00:00:00.000Z")),
......@@ -171,6 +172,7 @@ it.effect("lists change request history through the batched head lookup", () =>
171172 });
172173 assert.strictEqual(changeRequests[0]?.provider, "github");
173174 assert.strictEqual(changeRequests[0]?.state, "merged");
175 assert.strictEqual(changeRequests[0]?.headSha, "a".repeat(40));
174176 assert.strictEqual(changeRequests[0]?.mergedAt, "2026-01-01T00:00:00Z");
175177 assert.deepStrictEqual(
176178 changeRequests[0]?.updatedAt,
apps/server/src/sourceControl/GitHubSourceControlProvider.ts+1
......@@ -48,6 +48,7 @@ function toChangeRequest(summary: GitHubCli.GitHubPullRequestSummary): ChangeReq
4848 url: summary.url,
4949 baseRefName: summary.baseRefName,
5050 headRefName: summary.headRefName,
51 ...(summary.headSha !== undefined ? { headSha: summary.headSha } : {}),
5152 state: summary.state ?? "open",
5253 ...(summary.isDraft === true ? { isDraft: true } : {}),
5354 closedAt: summary.closedAt ?? null,
apps/server/src/sourceControl/gitHubPullRequests.ts+5
......@@ -13,6 +13,8 @@ export interface NormalizedGitHubPullRequestRecord {
1313 readonly url: string;
1414 readonly baseRefName: string;
1515 readonly headRefName: string;
16 /** The head commit, when the read asked for `headRefOid`. */
17 readonly headSha?: string;
1618 readonly state: "open" | "closed" | "merged";
1719 readonly isDraft?: boolean;
1820 readonly closedAt?: string | null;
......@@ -29,6 +31,7 @@ const GitHubPullRequestSchema = Schema.Struct({
2931 url: TrimmedNonEmptyString,
3032 baseRefName: TrimmedNonEmptyString,
3133 headRefName: TrimmedNonEmptyString,
34 headRefOid: Schema.optional(Schema.NullOr(Schema.String)),
3235 state: Schema.optional(Schema.NullOr(Schema.String)),
3336 isDraft: Schema.optional(Schema.Boolean),
3437 closedAt: Schema.optional(Schema.NullOr(Schema.String)),
......@@ -90,6 +93,7 @@ function normalizeGitHubPullRequestRecord(
9093 (headRepositoryOwnerLogin && headRepositoryName
9194 ? `${headRepositoryOwnerLogin}/${headRepositoryName}`
9295 : null);
96 const headSha = trimOptionalString(raw.headRefOid);
9397
9498 return {
9599 number: raw.number,
......@@ -97,6 +101,7 @@ function normalizeGitHubPullRequestRecord(
97101 url: raw.url,
98102 baseRefName: raw.baseRefName,
99103 headRefName: raw.headRefName,
104 ...(headSha ? { headSha } : {}),
100105 state: normalizeGitHubPullRequestState(raw),
101106 ...(raw.isDraft === true ? { isDraft: true } : {}),
102107 closedAt: raw.closedAt ?? null,
apps/server/src/storageCleanup.test.ts+51-1
......@@ -7,7 +7,11 @@ import {
77 type OrchestrationV2ThreadShell,
88} from "@t3tools/contracts";
99import * as DateTime from "effect/DateTime";
10import { storageCleanupActivityAt, storageCleanupThreadIdle } from "./storageCleanup.ts";
10import {
11 storageCleanupActivityAt,
12 storageCleanupPullRequestMerged,
13 storageCleanupThreadIdle,
14} from "./storageCleanup.ts";
1115
1216const NOW_MS = Date.parse("2026-06-10T12:00:00.000Z");
1317const DAY_MS = 24 * 60 * 60 * 1_000;
......@@ -104,3 +108,49 @@ describe("V2 storage cleanup eligibility", () => {
104108 return { ...candidate(), status };
105109 }
106110});
111
112describe("merged pull request cleanup", () => {
113 const HEAD_SHA = "a".repeat(40);
114 const integrated = {
115 branch: "feature",
116 defaultBranch: "main",
117 headSha: HEAD_SHA,
118 integrated: true,
119 };
120 const squashed = { ...integrated, integrated: false };
121 const pullRequest = (
122 overrides: Partial<NonNullable<Parameters<typeof storageCleanupPullRequestMerged>[0]>> = {},
123 ) => ({
124 state: "merged" as const,
125 headRef: "feature",
126 baseRef: "main",
127 headSha: HEAD_SHA,
128 ...overrides,
129 });
130
131 it("removes a worktree whose head reached the default branch through a merged pull request", () => {
132 expect(storageCleanupPullRequestMerged(pullRequest({ headSha: null }), integrated)).toBe(true);
133 });
134
135 it("removes a squash-merged worktree when the pull request names its exact head", () => {
136 expect(storageCleanupPullRequestMerged(pullRequest(), squashed)).toBe(true);
137 });
138
139 it.each([
140 ["has a later commit than the merged head", { headSha: "c".repeat(40) }],
141 ["was merged into a release branch", { baseRef: "release" }],
142 ["was merged into its stack parent", { baseRef: "stack-parent" }],
143 ["was merged without a reported head commit", { headSha: null }],
144 ["belongs to a different branch", { headRef: "other" }],
145 ["is still open", { state: "open" }],
146 ["was closed without merging", { state: "closed" }],
147 ] as const)("keeps a squash worktree whose pull request %s", (_name, overrides) => {
148 expect(storageCleanupPullRequestMerged(pullRequest(overrides), squashed)).toBe(false);
149 });
150
151 it("keeps a worktree with no pull request, or one that is not merged", () => {
152 expect(storageCleanupPullRequestMerged(null, squashed)).toBe(false);
153 expect(storageCleanupPullRequestMerged(null, integrated)).toBe(false);
154 expect(storageCleanupPullRequestMerged(pullRequest({ state: "open" }), integrated)).toBe(false);
155 });
156});
apps/server/src/storageCleanup.ts+34-3
......@@ -106,6 +106,32 @@ export function storageCleanupActivityAt(thread: OrchestrationV2ThreadShell): nu
106106 );
107107}
108108
109/**
110 * Whether the host's pull request proves this worktree's head was merged. A
111 * squash or rebase merge leaves the head outside the default branch, so the
112 * merged pull request then has to name this exact commit.
113 */
114export function storageCleanupPullRequestMerged(
115 pullRequest: Pick<
116 GitManager.GitBranchPullRequest,
117 "state" | "headRef" | "baseRef" | "headSha"
118 > | null,
119 worktree: {
120 readonly branch: string;
121 readonly defaultBranch: string;
122 readonly headSha: string;
123 readonly integrated: boolean;
124 },
125): boolean {
126 return (
127 pullRequest?.state === "merged" &&
128 (worktree.integrated ||
129 (pullRequest.headRef === worktree.branch &&
130 pullRequest.baseRef === worktree.defaultBranch &&
131 pullRequest.headSha === worktree.headSha))
132 );
133}
134
109135export const make = Effect.gen(function* () {
110136 const config = yield* ServerConfig.ServerConfig;
111137 const settingsService = yield* Settings.ServerSettingsService;
......@@ -295,14 +321,19 @@ export const make = Effect.gen(function* () {
295321 args: ["merge-base", "--is-ancestor", head.commitSha, base.commitSha],
296322 allowNonZeroExit: true,
297323 });
298 if (ancestor.exitCode !== 0) return;
299 eligible = settings.worktreeUnchanged;
324 const integrated = ancestor.exitCode === 0;
325 eligible = integrated && settings.worktreeUnchanged;
300326 if (!eligible && settings.worktreeOnMerge && thread.branch !== null) {
301327 const pullRequest = yield* gitManager.branchPullRequest(
302328 { cwd: worktreePath, branch: thread.branch },
303329 { refresh: true },
304330 );
305 eligible = pullRequest?.state === "merged";
331 eligible = storageCleanupPullRequestMerged(pullRequest, {
332 branch: thread.branch,
333 defaultBranch: branch,
334 headSha: head.commitSha,
335 integrated,
336 });
306337 }
307338 }
308339 if (!eligible) return;
docs/user/project-settings.md+3-2
......@@ -145,8 +145,9 @@ Worktrees can be removed after a chosen number of inactive days, after merging,
145145have no commits beyond the default branch. Only T3-managed worktrees are eligible. Active
146146sessions, shared worktrees, uncommitted changes, and ignored files other than `node_modules`
147147prevent removal. Branches and thread history stay; starting another turn recreates the checkout.
148Merge cleanup requires the commits to be included in the remote default branch, so squash merges
149may need the inactivity rule instead.
148Merge cleanup requires a merged pull request whose commits are included in the remote default
149branch. A squash or rebase merge on GitHub also counts when the pull request targeted the default
150branch and the worktree is still at the pull request's last commit.
150151
151152Enable **Delete worktrees with deleted threads** to remove safe worktrees after their last
152153thread is deleted, including archived threads and worktrees left by earlier deletions. The
packages/contracts/src/sourceControl.ts+2
......@@ -29,6 +29,8 @@ export const ChangeRequest = Schema.Struct({
2929 url: Schema.String,
3030 baseRefName: TrimmedNonEmptyString,
3131 headRefName: TrimmedNonEmptyString,
32 /** The head commit, when the provider's read includes it. */
33 headSha: Schema.optional(TrimmedNonEmptyString),
3234 state: ChangeRequestState,
3335 /** Present when the provider can tell that an open change request is still a draft. */
3436 isDraft: Schema.optional(Schema.Boolean),