Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions apps/desktop/src/main/services/lanes/laneService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2939,6 +2939,35 @@ describe("laneService delete teardown + cancellation + streaming", () => {
expect(wtStep?.status).toBe("completed");
});

it("removes residual worktree files before deleting the lane row", async () => {
const events: any[] = [];
const fake = makeFakeServices();
const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events });
const childPath = path.join(repoRoot, "child");
fs.writeFileSync(path.join(childPath, "residual.log"), "left behind by git\n", "utf8");

vi.mocked(runGit).mockImplementation(async (args: string[]) => {
const laneBranchGitStub = defaultLaneBranchGitStub(args);
if (laneBranchGitStub) return laneBranchGitStub;
if (args[0] === "status") return { exitCode: 0, stdout: "", stderr: "" } as any;
if (args[0] === "show-ref") return { exitCode: 1, stdout: "", stderr: "" } as any;
if (args[0] === "worktree" && args[1] === "remove") {
fake.calls.push("git_worktree_remove");
return { exitCode: 0, stdout: "", stderr: "" } as any;
}
return { exitCode: 0, stdout: "", stderr: "" } as any;
});
vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any);

await service.delete({ laneId: "lane-child", deleteBranch: false });

expect(fs.existsSync(childPath)).toBe(false);
expect(db.get<{ id: string }>("select id from lanes where id = ?", ["lane-child"])).toBeNull();
expect(vi.mocked(runGitOrThrow).mock.calls.some(([args]) => args[0] === "worktree" && args[1] === "prune")).toBe(true);
const last = events[events.length - 1];
expect(last.progress.steps.find((s: any) => s.name === "git_worktree_remove")?.detail).toContain("removed residual files");
});

it("keeps recent delete progress queryable for remounted renderers", async () => {
const events: any[] = [];
const fake = makeFakeServices();
Expand Down
33 changes: 21 additions & 12 deletions apps/desktop/src/main/services/lanes/laneService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3750,29 +3750,38 @@ export function createLaneService({
const removeArgs = ["worktree", "remove"];
if (force) removeArgs.push("--force");
removeArgs.push(row.worktree_path);
const removeResidualDirectory = async (detail: string, failurePrefix?: string) => {
try {
await fs.promises.rm(row.worktree_path, { recursive: true, force: true });
} catch (rmError) {
throw new Error(
`${failurePrefix ? `${failurePrefix}; ` : ""}manual cleanup failed: ${
rmError instanceof Error ? rmError.message : String(rmError)
}`
);
}
await runGitOrThrow(["worktree", "prune"], { cwd: projectRoot, timeoutMs: 30_000 });
return { detail };
};
// 60s — large worktrees (e.g. with node_modules) can take longer than 15s
// to walk; a timeout here mid-remove leaves the worktree in a half-deleted
// state that blocks future deletes.
const removeRes = await runGit(removeArgs, { cwd: projectRoot, timeoutMs: 60_000 });
if (removeRes.exitCode === 0) {
if (fs.existsSync(row.worktree_path)) {
return removeResidualDirectory(`${row.worktree_path} (removed residual files)`);
}
return { detail: row.worktree_path };
}
// Recovery path: a previous failed delete (or this one's first attempt)
// can leave the worktree dir present without its `.git` pointer file, or
// the dir gone with stale metadata still registered. Either way: rm the
// dir if any, then prune git's metadata.
try {
await fs.promises.rm(row.worktree_path, { recursive: true, force: true });
} catch (rmError) {
const original = (removeRes.stderr || removeRes.stdout || "").trim();
throw new Error(
`git worktree remove failed (${original}); manual cleanup also failed: ${
rmError instanceof Error ? rmError.message : String(rmError)
}`
);
}
await runGitOrThrow(["worktree", "prune"], { cwd: projectRoot, timeoutMs: 30_000 });
return { detail: `${row.worktree_path} (recovered from stale state)` };
const original = (removeRes.stderr || removeRes.stdout || "").trim();
return removeResidualDirectory(
`${row.worktree_path} (recovered from stale state)`,
`git worktree remove failed (${original})`
);
});
}

Expand Down
140 changes: 140 additions & 0 deletions apps/desktop/src/main/services/prs/prService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,146 @@ describe("prService.getGithubSnapshot", () => {
]));
});

it("fetches a targeted same-repo lane branch PR when the repo snapshot window misses it", async () => {
const githubService = makeGithubService({
getStatus: vi.fn(async () => ({
tokenStored: true,
repo: REPO,
userLogin: "octocat",
})),
apiRequest: vi.fn(async (args: { path: string; query?: Record<string, unknown> }) => {
if (args.path !== `/repos/${REPO.owner}/${REPO.name}/pulls`) {
throw new Error(`Unexpected GitHub API path: ${args.path}`);
}
if (args.query?.head === `${REPO.owner}:feature/missed`) {
return {
data: [
makeGitHubPull({
number: 222,
title: "Missed branch PR",
state: "closed",
merged_at: null,
head: {
ref: "feature/missed",
user: { login: REPO.owner },
repo: { owner: { login: REPO.owner }, name: REPO.name },
},
}),
],
};
}
return {
data: [
makeGitHubPull({
number: 111,
title: "Recent unrelated PR",
head: {
ref: "feature/recent",
user: { login: REPO.owner },
repo: { owner: { login: REPO.owner }, name: REPO.name },
},
}),
],
};
}),
});
const lane = makeFakeLane({ branchRef: "refs/heads/feature/missed" });
const db = makeMockDb();
const { service } = buildService({ db, githubService, laneService: makeLaneService([lane]) });

const snapshot = await service.getGithubSnapshot({ force: true });

expect(githubService.apiRequest).toHaveBeenCalledWith(expect.objectContaining({
query: expect.objectContaining({ head: `${REPO.owner}:feature/missed` }),
}));
expect(snapshot.repoPullRequests).toEqual(expect.arrayContaining([
expect.objectContaining({
githubPrNumber: 222,
title: "Missed branch PR",
state: "closed",
headRepoOwner: REPO.owner,
headRepoName: REPO.name,
}),
]));
expect(db.run).toHaveBeenCalledWith(
expect.stringContaining("insert into pull_requests("),
expect.arrayContaining([LANE_ID, REPO.owner, REPO.name, 222, "Missed branch PR", "closed", "main", "feature/missed"]),
);
});

it("continues targeted lane branch PR lookups after one branch lookup fails", async () => {
const githubService = makeGithubService({
getStatus: vi.fn(async () => ({
tokenStored: true,
repo: REPO,
userLogin: "octocat",
})),
apiRequest: vi.fn(async (args: { path: string; query?: Record<string, unknown> }) => {
if (args.path !== `/repos/${REPO.owner}/${REPO.name}/pulls`) {
throw new Error(`Unexpected GitHub API path: ${args.path}`);
}
if (args.query?.head === `${REPO.owner}:feature/flaky`) {
throw new Error("temporary GitHub failure");
}
if (args.query?.head === `${REPO.owner}:feature/missed`) {
return {
data: [
makeGitHubPull({
number: 222,
title: "Missed branch PR",
state: "closed",
merged_at: null,
head: {
ref: "feature/missed",
user: { login: REPO.owner },
repo: { owner: { login: REPO.owner }, name: REPO.name },
},
}),
],
};
}
return {
data: [
makeGitHubPull({
number: 111,
title: "Recent unrelated PR",
head: {
ref: "feature/recent",
user: { login: REPO.owner },
repo: { owner: { login: REPO.owner }, name: REPO.name },
},
}),
],
};
}),
});
const db = makeMockDb();
const laneService = makeLaneService([
makeFakeLane({ id: "lane-flaky", branchRef: "refs/heads/feature/flaky" }),
makeFakeLane({ branchRef: "refs/heads/feature/missed" }),
]);
const { service } = buildService({ db, githubService, laneService });

const snapshot = await service.getGithubSnapshot({ force: true });

expect(githubService.apiRequest).toHaveBeenCalledWith(expect.objectContaining({
query: expect.objectContaining({ head: `${REPO.owner}:feature/flaky` }),
}));
expect(githubService.apiRequest).toHaveBeenCalledWith(expect.objectContaining({
query: expect.objectContaining({ head: `${REPO.owner}:feature/missed` }),
}));
expect(snapshot.repoPullRequests).toEqual(expect.arrayContaining([
expect.objectContaining({
githubPrNumber: 222,
title: "Missed branch PR",
}),
]));
expect(db.run).toHaveBeenCalledWith(
expect.stringContaining("insert into pull_requests("),
expect.arrayContaining([LANE_ID, REPO.owner, REPO.name, 222, "Missed branch PR", "closed", "main", "feature/missed"]),
);
});

it("updates an existing repo PR row during lane PR backfill instead of duplicating it", async () => {
const githubService = makeGithubService({
getStatus: vi.fn(async () => ({
Expand Down
86 changes: 81 additions & 5 deletions apps/desktop/src/main/services/prs/prService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1546,6 +1546,19 @@ export function createPrService({
}
};

const rawPullHeadBranch = (rawPr: any): string => normalizeBranchName(asString(rawPr?.head?.ref));
const rawPullHeadOwner = (rawPr: any): string => asString(rawPr?.head?.repo?.owner?.login)
|| asString(rawPr?.head?.user?.login)
|| asString(rawPr?.head?.repo?.owner);
const rawPullHeadRepoName = (rawPr: any): string => asString(rawPr?.head?.repo?.name);
const rawPullHasSameRepoHead = (rawPr: any, repo: GitHubRepoRef): boolean => {
const owner = rawPullHeadOwner(rawPr);
const name = rawPullHeadRepoName(rawPr);
if (!owner || owner.toLowerCase() !== repo.owner.toLowerCase()) return false;
if (name && name.toLowerCase() !== repo.name.toLowerCase()) return false;
return true;
};

const backfillLanePrRowsFromGithubPulls = (rawPulls: any[], repo: GitHubRepoRef, lanes: LaneSummary[]): number => {
const activeLaneByBranch = new Map<string, LaneSummary>();
for (const lane of lanes) {
Expand All @@ -1558,13 +1571,11 @@ export function createPrService({

const backfilledIds: string[] = [];
for (const rawPr of rawPulls) {
const headBranch = normalizeBranchName(asString(rawPr?.head?.ref));
const headBranch = rawPullHeadBranch(rawPr);
const lane = headBranch ? activeLaneByBranch.get(headBranch) ?? null : null;
if (!lane) continue;

const headOwner = asString(rawPr?.head?.repo?.owner?.login)
|| asString(rawPr?.head?.user?.login)
|| asString(rawPr?.head?.repo?.owner);
const headOwner = rawPullHeadOwner(rawPr);
if (!headOwner || headOwner.toLowerCase() !== repo.owner.toLowerCase()) continue;

const prNumber = asNumber(rawPr?.number);
Expand Down Expand Up @@ -1609,6 +1620,68 @@ export function createPrService({
return backfilledIds.length;
};

const fetchMissingSameRepoLanePulls = async (
rawPulls: any[],
repo: GitHubRepoRef,
lanes: LaneSummary[],
): Promise<any[]> => {
const branchHasSameRepoPr = new Set<string>();
const seenRepoPrNumbers = new Set<number>();
for (const rawPr of rawPulls) {
const prNumber = asNumber(rawPr?.number);
if (prNumber) seenRepoPrNumbers.add(prNumber);
const branch = rawPullHeadBranch(rawPr);
if (branch && rawPullHasSameRepoHead(rawPr, repo)) branchHasSameRepoPr.add(branch);
}

const candidateBranches: string[] = [];
const seenBranches = new Set<string>();
for (const lane of lanes) {
if (lane.archivedAt || lane.laneType === "primary") continue;
const branch = normalizeBranchName(branchNameFromRef(lane.branchRef));
if (!branch || seenBranches.has(branch) || branchHasSameRepoPr.has(branch)) continue;
seenBranches.add(branch);
candidateBranches.push(branch);
}

const MAX_TARGETED_LANE_PR_BRANCH_LOOKUPS = 12;
const targetedBranches = candidateBranches.slice(0, MAX_TARGETED_LANE_PR_BRANCH_LOOKUPS);
if (targetedBranches.length === 0) return rawPulls;

const extras: any[] = [];
for (const branch of targetedBranches) {
let branchPulls: any[];
try {
branchPulls = await fetchAllPages<any>({
path: `/repos/${repo.owner}/${repo.name}/pulls`,
query: {
state: "all",
head: `${repo.owner}:${branch}`,
sort: "updated",
direction: "desc",
},
maxPages: 1,
});
} catch (error) {
logger.warn("prs.github_snapshot_targeted_lane_pull_lookup_failed", {
owner: repo.owner,
repo: repo.name,
branch,
error: getErrorMessage(error),
});
continue;
}
for (const rawPr of branchPulls) {
const prNumber = asNumber(rawPr?.number);
if (!prNumber || seenRepoPrNumbers.has(prNumber)) continue;
Comment thread
arul28 marked this conversation as resolved.
seenRepoPrNumbers.add(prNumber);
extras.push(rawPr);
}
}

return extras.length ? [...rawPulls, ...extras] : rawPulls;
};

const pluralizeCommit = (count: number): string => count === 1 ? "commit" : "commits";

const parseLeftRightCounts = (stdout: string): { left: number; right: number } | null => {
Expand Down Expand Up @@ -5070,6 +5143,8 @@ export function createPrService({
isDraft: Boolean(rawPr?.draft),
baseBranch: asString(rawPr?.base?.ref) || null,
headBranch: asString(rawPr?.head?.ref) || null,
headRepoOwner: rawPullHeadOwner(rawPr) || null,
headRepoName: rawPullHeadRepoName(rawPr) || null,
author: asString(rawPr?.user?.login) || null,
createdAt: asString(rawPr?.created_at) || nowIso(),
updatedAt: asString(rawPr?.updated_at) || asString(rawPr?.created_at) || nowIso(),
Expand All @@ -5090,10 +5165,11 @@ export function createPrService({
};
};

const repoPullRequestsRaw = await fetchAllPages<any>({
let repoPullRequestsRaw = await fetchAllPages<any>({
path: `/repos/${repo.owner}/${repo.name}/pulls`,
query: { state: "all", sort: "updated", direction: "desc" },
});
repoPullRequestsRaw = await fetchMissingSameRepoLanePulls(repoPullRequestsRaw, repo, lanes);
if (backfillLanePrRowsFromGithubPulls(repoPullRequestsRaw, repo, lanes) > 0) {
pullRequestRows = listRows();
linkedPrByRepoKey = new Map(
Expand Down
Loading
Loading