Skip to content

Commit 3bcf872

Browse files
committed
desktop + iOS: PR convergence rails, onboarding tour revamp, terminals session overhaul
1 parent 70fd4e5 commit 3bcf872

87 files changed

Lines changed: 26797 additions & 2006 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/desktop/src/main/services/ipc/registerIpc.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ import type {
151151
LandResult,
152152
LandStackEnhancedArgs,
153153
LandQueueNextArgs,
154+
CleanupPrBranchArgs,
155+
CleanupPrBranchResult,
154156
PrCheck,
157+
PrCommit,
155158
PrComment,
156159
PrReviewThread,
157160
PrHealth,
@@ -5878,6 +5881,7 @@ export function registerIpc({
58785881

58795882
ipcMain.handle(IPC.prsGetDetail, (_e, args: { prId: string }) => getCtx().prService.getDetail(args.prId));
58805883
ipcMain.handle(IPC.prsGetFiles, (_e, args: { prId: string }) => getCtx().prService.getFiles(args.prId));
5884+
ipcMain.handle(IPC.prsGetCommits, (_e, args: { prId: string }): Promise<PrCommit[]> => getCtx().prService.getCommits(args.prId));
58815885
ipcMain.handle(IPC.prsGetActionRuns, (_e, args: { prId: string }) => getCtx().prService.getActionRuns(args.prId));
58825886
ipcMain.handle(IPC.prsGetActivity, (_e, args: { prId: string }) => getCtx().prService.getActivity(args.prId));
58835887
ipcMain.handle(IPC.prsAddComment, (_e, args) => getCtx().prService.addComment(args));
@@ -5930,6 +5934,8 @@ export function registerIpc({
59305934
);
59315935
},
59325936
);
5937+
ipcMain.handle(IPC.prsCleanupBranch, (_e, args: CleanupPrBranchArgs): Promise<CleanupPrBranchResult> =>
5938+
getCtx().prService.cleanupBranch(args));
59335939

59345940
// Issue Inventory (PR convergence loop)
59355941
ipcMain.handle(IPC.prsIssueInventorySync, async (_e, args: { prId: string }): Promise<IssueInventorySnapshot> => {

apps/desktop/src/main/services/prs/issueInventoryService.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,108 @@ describe("issueInventoryService", () => {
288288
expect(insertCalls.length).toBe(0);
289289
});
290290

291+
it("treats unresolved threads with a latest resolution acknowledgement as fixed", () => {
292+
const db = makeMockDb();
293+
db.get.mockReturnValue(makeFakeRow({
294+
external_id: "thread:thread-ack",
295+
state: "new",
296+
thread_comment_count: 1,
297+
thread_latest_comment_id: "comment-1",
298+
}));
299+
db.all.mockReturnValue([makeFakeRow({
300+
external_id: "thread:thread-ack",
301+
state: "new",
302+
thread_comment_count: 1,
303+
thread_latest_comment_id: "comment-1",
304+
})]);
305+
306+
const service = createIssueInventoryService({ db });
307+
service.syncFromPrData(
308+
PR_ID,
309+
[],
310+
[makeReviewThread({
311+
id: "thread-ack",
312+
comments: [
313+
{
314+
id: "comment-1",
315+
author: "reviewer",
316+
authorAvatarUrl: null,
317+
body: "This needs a null check.",
318+
url: null,
319+
createdAt: "2026-03-23T12:00:00.000Z",
320+
updatedAt: "2026-03-23T12:00:00.000Z",
321+
},
322+
{
323+
id: "comment-2",
324+
author: "arul28",
325+
authorAvatarUrl: null,
326+
body: "Fixed in the latest commit.",
327+
url: null,
328+
createdAt: "2026-03-23T12:10:00.000Z",
329+
updatedAt: "2026-03-23T12:10:00.000Z",
330+
},
331+
],
332+
})],
333+
[],
334+
);
335+
336+
const updateCalls = db.run.mock.calls.filter(
337+
(call: unknown[]) => typeof call[0] === "string" && (call[0] as string).includes("update pr_issue_inventory"),
338+
);
339+
expect(updateCalls.some((call: unknown[]) => (call[1] as unknown[])[8] === "fixed")).toBe(true);
340+
});
341+
342+
it("does not treat negative resolution wording as fixed", () => {
343+
const db = makeMockDb();
344+
db.get.mockReturnValue(makeFakeRow({
345+
external_id: "thread:thread-not-fixed",
346+
state: "new",
347+
thread_comment_count: 1,
348+
thread_latest_comment_id: "comment-1",
349+
}));
350+
db.all.mockReturnValue([makeFakeRow({
351+
external_id: "thread:thread-not-fixed",
352+
state: "new",
353+
thread_comment_count: 1,
354+
thread_latest_comment_id: "comment-1",
355+
})]);
356+
357+
const service = createIssueInventoryService({ db });
358+
service.syncFromPrData(
359+
PR_ID,
360+
[],
361+
[makeReviewThread({
362+
id: "thread-not-fixed",
363+
comments: [
364+
{
365+
id: "comment-1",
366+
author: "reviewer",
367+
authorAvatarUrl: null,
368+
body: "Please tighten this logic.",
369+
url: null,
370+
createdAt: "2026-03-23T12:00:00.000Z",
371+
updatedAt: "2026-03-23T12:00:00.000Z",
372+
},
373+
{
374+
id: "comment-2",
375+
author: "reviewer",
376+
authorAvatarUrl: null,
377+
body: "This is not fixed yet.",
378+
url: null,
379+
createdAt: "2026-03-23T12:10:00.000Z",
380+
updatedAt: "2026-03-23T12:10:00.000Z",
381+
},
382+
],
383+
})],
384+
[],
385+
);
386+
387+
const updateCalls = db.run.mock.calls.filter(
388+
(call: unknown[]) => typeof call[0] === "string" && (call[0] as string).includes("update pr_issue_inventory"),
389+
);
390+
expect(updateCalls.some((call: unknown[]) => (call[1] as unknown[])[8] === "fixed")).toBe(false);
391+
});
392+
291393
it("skips outdated review threads", () => {
292394
const db = makeMockDb();
293395
db.get.mockReturnValue(null);

apps/desktop/src/main/services/prs/issueInventoryService.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import type {
1414
PrReviewThread,
1515
} from "../../../shared/types";
1616
import { DEFAULT_CONVERGENCE_RUNTIME_STATE, DEFAULT_PIPELINE_SETTINGS } from "../../../shared/types";
17-
import { isNoisyIssueComment } from "./resolverUtils";
17+
import { isNoisyIssueComment, looksLikeResolutionAck } from "./resolverUtils";
1818
import { nowIso } from "../shared/utils";
1919

2020
// ---------------------------------------------------------------------------
@@ -740,7 +740,12 @@ export function createIssueInventoryService(deps: { db: AdeDb }) {
740740
threadLatestCommentSource: source,
741741
};
742742

743-
if (thread.isResolved || thread.isOutdated) {
743+
const latestReplyLooksResolved = source !== "coderabbit"
744+
&& source !== "copilot"
745+
&& source !== "codex"
746+
&& looksLikeResolutionAck(body);
747+
748+
if (thread.isResolved || thread.isOutdated || latestReplyLooksResolved) {
744749
if (!existing) continue;
745750
upsertItem(prId, externalId, threadData, {
746751
state: "fixed",

apps/desktop/src/main/services/prs/prIssueResolver.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ describe("buildPrIssueResolutionPrompt", () => {
169169
expect(prompt).toContain("Watch carefully for regressions caused by your fixes.");
170170
expect(prompt).toContain("update the test");
171171
expect(prompt).toContain("rerun the complete failing test files or suites locally");
172+
expect(prompt).toContain("one bounded Path to Merge resolution round");
173+
expect(prompt).toContain("ADE will poll GitHub");
172174
expect(prompt).toContain("Commit the changes and push the PR branch before you stop.");
173175
expect(prompt).toContain("If you cannot safely commit or push the necessary changes");
174176
expect(prompt).toContain("prRefreshIssueInventory");

apps/desktop/src/main/services/prs/prIssueResolver.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,9 @@ export function buildPrIssueResolutionPrompt(args: IssueResolutionPromptArgs): s
514514
"",
515515
"Requirements",
516516
"- Fix all valid issues in the selected scope, not just the first one.",
517+
"- Treat this chat as one bounded Path to Merge resolution round. Make one coherent set of fixes for the current inventory, then hand control back to ADE.",
517518
"- If you make local code or git changes that should affect the PR, do not finish with local-only state. Commit the changes and push the PR branch before you stop.",
519+
"- After you push, do not wait indefinitely for CI or advisory review bots. ADE will poll GitHub, observe custom post-push comments, and launch the next round if new actionable work appears.",
518520
"- If you only resolve stale review threads or other PR metadata with ADE tools and no local git changes are needed, say that clearly in your final note.",
519521
"- If you cannot safely commit or push the necessary changes, stop with a concrete blocker instead of exiting as if the round succeeded.",
520522
);
@@ -559,7 +561,7 @@ export function buildPrIssueResolutionPrompt(args: IssueResolutionPromptArgs): s
559561
"- Before you push, rerun the complete failing test files or suites locally, not just the specific failing test names. Test runners and sharded CI can hide additional failures behind the first error in a file.",
560562
"- Treat newly added or heavily modified test files as likely regression hotspots, even if CI only surfaced a different failure first.",
561563
"- Watch carefully for regressions caused by your fixes. If a change breaks an existing test because the expected behavior legitimately changed, update the test. Do not change tests just to mask a bug.",
562-
"- Continue iterating until the selected issue set is cleared and CI is green, or stop only with a concrete blocker and explain it clearly.",
564+
"- Stop with a concise final note that lists what changed, what you validated, whether you pushed, and any concrete blocker ADE should surface to the operator.",
563565
);
564566
if (isIncremental) {
565567
promptSections.push(

apps/desktop/src/main/services/prs/prService.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import type {
1313
CommitIntegrationArgs,
1414
CleanupIntegrationWorkflowArgs,
1515
CleanupIntegrationWorkflowResult,
16+
CleanupPrBranchArgs,
17+
CleanupPrBranchResult,
1618
DeleteIntegrationProposalArgs,
1719
DeleteIntegrationProposalResult,
1820
DismissIntegrationCleanupArgs,
@@ -2615,6 +2617,10 @@ export function createPrService({
26152617
const createdAt = nowIso();
26162618
const headBranch = asString(pr?.head?.ref) || branchNameFromRef(lane.branchRef);
26172619
const baseBranch = asString(pr?.base?.ref) || branchNameFromRef(lane.baseRef);
2620+
const laneBranch = branchNameFromRef(lane.branchRef);
2621+
if (normalizeBranchName(laneBranch) !== normalizeBranchName(headBranch)) {
2622+
throw new Error(`Cannot link PR #${locator.number} to lane "${lane.name}" because the PR head branch is "${headBranch}" but the lane branch is "${laneBranch}".`);
2623+
}
26182624

26192625
// Backfill creation_strategy for imported PRs that don't have one stored yet.
26202626
// The wizard defaults to "pr_target" when users create via the UI; mirror
@@ -2653,6 +2659,74 @@ export function createPrService({
26532659
return await refreshOne(prId);
26542660
};
26552661

2662+
const cleanupBranch = async (args: CleanupPrBranchArgs): Promise<CleanupPrBranchResult> => {
2663+
const row = getRow(args.prId);
2664+
if (!row) throw new Error(`PR not found: ${args.prId}`);
2665+
if (row.state !== "merged" && row.state !== "closed") {
2666+
throw new Error("Branch cleanup is only available after a PR is merged or closed.");
2667+
}
2668+
2669+
const branchName = branchNameFromRef(row.head_branch);
2670+
if (!branchName || branchName === "HEAD") {
2671+
throw new Error("PR head branch is missing.");
2672+
}
2673+
2674+
const lanes = await laneService.list({ includeArchived: true, includeStatus: false });
2675+
const primaryBranches = new Set(
2676+
lanes
2677+
.filter((lane) => lane.laneType === "primary")
2678+
.map((lane) => normalizeBranchName(branchNameFromRef(lane.branchRef)))
2679+
.filter(Boolean),
2680+
);
2681+
if (primaryBranches.has(normalizeBranchName(branchName))) {
2682+
throw new Error(`Refusing to clean up protected branch "${branchName}".`);
2683+
}
2684+
2685+
const result: CleanupPrBranchResult = {
2686+
prId: args.prId,
2687+
branchName,
2688+
localDeleted: false,
2689+
remoteDeleted: false,
2690+
localError: null,
2691+
remoteError: null,
2692+
};
2693+
2694+
if (args.deleteLocalBranch !== false) {
2695+
const refCheck = await runGit(["show-ref", "--verify", "--quiet", `refs/heads/${branchName}`], {
2696+
cwd: projectRoot,
2697+
timeoutMs: 8_000,
2698+
});
2699+
if (refCheck.exitCode === 0) {
2700+
const deleted = await runGit(["branch", "-D", branchName], { cwd: projectRoot, timeoutMs: 30_000 });
2701+
if (deleted.exitCode === 0) result.localDeleted = true;
2702+
else result.localError = deleted.stderr || deleted.stdout || `Failed to delete local branch ${branchName}`;
2703+
}
2704+
}
2705+
2706+
if (args.deleteRemoteBranch) {
2707+
const remote = args.remoteName?.trim() || "origin";
2708+
const remoteCheck = await runGit(["remote", "get-url", remote], { cwd: projectRoot, timeoutMs: 8_000 });
2709+
if (remoteCheck.exitCode !== 0) {
2710+
result.remoteError = `Remote '${remote}' is not configured for this repository`;
2711+
} else {
2712+
const remoteRefCheck = await runGit(["ls-remote", "--heads", remote, branchName], {
2713+
cwd: projectRoot,
2714+
timeoutMs: 12_000,
2715+
});
2716+
if (remoteRefCheck.exitCode === 0 && remoteRefCheck.stdout.trim().length > 0) {
2717+
const deleted = await runGit(["push", remote, "--delete", branchName], { cwd: projectRoot, timeoutMs: 45_000 });
2718+
if (deleted.exitCode === 0) result.remoteDeleted = true;
2719+
else result.remoteError = deleted.stderr || deleted.stdout || `Failed to delete remote branch ${branchName}`;
2720+
}
2721+
}
2722+
}
2723+
2724+
if (result.localError || result.remoteError) {
2725+
logger.warn("prs.branch_cleanup_partial_failure", result);
2726+
}
2727+
return result;
2728+
};
2729+
26562730
const land = async (args: LandPrArgs): Promise<LandResult> => {
26572731
const row = getRow(args.prId);
26582732
if (!row) throw new Error(`PR not found: ${args.prId}`);
@@ -5034,6 +5108,10 @@ export function createPrService({
50345108
return await linkToLane(args);
50355109
},
50365110

5111+
async cleanupBranch(args: CleanupPrBranchArgs): Promise<CleanupPrBranchResult> {
5112+
return await cleanupBranch(args);
5113+
},
5114+
50375115
getForLane(laneId: string): PrSummary | null {
50385116
const row = getRowForLane(laneId);
50395117
return row ? rowToSummary(row) : null;
@@ -5306,6 +5384,12 @@ export function createPrService({
53065384
return files;
53075385
},
53085386

5387+
async getCommits(prId: string): Promise<PrCommit[]> {
5388+
const commits = await getCommitsSnapshot(prId);
5389+
upsertSnapshotRow({ prId, commits });
5390+
return commits;
5391+
},
5392+
53095393
async getActionRuns(prId: string): Promise<PrActionRun[]> {
53105394
const row = requireRow(prId);
53115395
const repo = repoFromRow(row);

apps/desktop/src/main/services/prs/resolverUtils.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ const NOISY_BODY_PATTERNS = [
1616
/<!-- internal state/i,
1717
/walkthrough/i,
1818
/@codex review/i,
19+
/^@(copilot|coderabbitai|greptile|codex)\s+review\b/i,
20+
/\bship-lane handoff\b/i,
21+
/\bclear-to-merge\b/i,
22+
/\bci green\b/i,
1923
];
2024

2125
export function isNoisyIssueComment(comment: PrComment): boolean {
@@ -26,6 +30,28 @@ export function isNoisyIssueComment(comment: PrComment): boolean {
2630
return NOISY_BODY_PATTERNS.some((pattern) => pattern.test(body));
2731
}
2832

33+
const RESOLUTION_NEGATION_PATTERNS = [
34+
/\bnot\s+(fixed|addressed|resolved|done|handled)\b/i,
35+
/\b(still|isn'?t|is not|not yet)\s+(an issue|fixed|addressed|resolved|done|handled|working)\b/i,
36+
/\b(no|doesn'?t|does not)\s+(fix|address|resolve|handle)\b/i,
37+
];
38+
39+
const RESOLUTION_ACK_PATTERNS = [
40+
/\b(fixed|addressed|resolved|done|handled)\b/i,
41+
/\bshould be (good|fixed|resolved)\b/i,
42+
/\bno longer (an issue|applies|reproduces)\b/i,
43+
/\bthanks[,! ]+(fixed|addressed|resolved)\b/i,
44+
/\bclear-to-merge\b/i,
45+
/\bci green\b/i,
46+
];
47+
48+
export function looksLikeResolutionAck(body: string | null | undefined): boolean {
49+
const text = (body ?? "").trim();
50+
if (!text) return false;
51+
if (RESOLUTION_NEGATION_PATTERNS.some((pattern) => pattern.test(text))) return false;
52+
return RESOLUTION_ACK_PATTERNS.some((pattern) => pattern.test(text));
53+
}
54+
2955
/**
3056
* Map ADE's permission mode to the agent chat permission mode.
3157
* Shared by both the issue resolver and rebase resolver.

apps/desktop/src/preload/global.d.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,10 @@ import type {
306306
PrActionRun,
307307
PrActivityEvent,
308308
PrCheck,
309+
PrCommit,
309310
PrComment,
311+
CleanupPrBranchArgs,
312+
CleanupPrBranchResult,
310313
PrConflictAnalysis,
311314
PrDetail,
312315
PrEventPayload,
@@ -1473,6 +1476,7 @@ declare global {
14731476
onEvent: (cb: (ev: PrEventPayload) => void) => () => void;
14741477
getDetail: (prId: string) => Promise<PrDetail>;
14751478
getFiles: (prId: string) => Promise<PrFile[]>;
1479+
getCommits: (prId: string) => Promise<PrCommit[]>;
14761480
getActionRuns: (prId: string) => Promise<PrActionRun[]>;
14771481
getActivity: (prId: string) => Promise<PrActivityEvent[]>;
14781482
addComment: (args: AddPrCommentArgs) => Promise<PrComment>;
@@ -1542,6 +1546,9 @@ declare global {
15421546
launchIssueResolutionFromThread: (
15431547
args: LaunchPrIssueResolutionFromThreadArgs,
15441548
) => Promise<LaunchPrIssueResolutionFromThreadResult>;
1549+
cleanupBranch: (
1550+
args: CleanupPrBranchArgs,
1551+
) => Promise<CleanupPrBranchResult>;
15451552
};
15461553
rebase: {
15471554
scanNeeds: () => Promise<RebaseNeed[]>;

0 commit comments

Comments
 (0)