Skip to content

Commit efccad2

Browse files
authored
desktop + iOS: PR convergence rails, onboarding tour revamp, terminals overhaul (#192)
* desktop + iOS: PR convergence rails, onboarding tour revamp, terminals session overhaul * ship: iter 1 — address 24 bot comments, remove accidental untitled.pen
1 parent 70fd4e5 commit efccad2

86 files changed

Lines changed: 9130 additions & 2011 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: 11 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,16 @@ export function createIssueInventoryService(deps: { db: AdeDb }) {
740740
threadLatestCommentSource: source,
741741
};
742742

743-
if (thread.isResolved || thread.isOutdated) {
743+
// Only treat as a resolution ACK when the latest reply is from a real
744+
// human (or an unclassified author). Review bots — CodeRabbit (normalized
745+
// from `coderabbitai`), Copilot, Codex, Greptile, Seer, ADE, etc. — often
746+
// mention "fixed/resolved/done" inside long markdown bodies without
747+
// actually acknowledging a fix, so passing their comments through the
748+
// heuristic silently flips real, open threads to `"fixed"`.
749+
const latestReplyLooksResolved = (source === "human" || source === "unknown")
750+
&& looksLikeResolutionAck(body);
751+
752+
if (thread.isResolved || thread.isOutdated || latestReplyLooksResolved) {
744753
if (!existing) continue;
745754
upsertItem(prId, externalId, threadData, {
746755
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: 88 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,78 @@ 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+
// Only force-delete when the PR is merged. For closed (non-merged) PRs
2701+
// the branch may still hold unintegrated work, so use the safe `-d`
2702+
// variant which refuses to drop unmerged commits.
2703+
const deleteFlag = row.state === "merged" ? "-D" : "-d";
2704+
const deleted = await runGit(["branch", deleteFlag, branchName], { cwd: projectRoot, timeoutMs: 30_000 });
2705+
if (deleted.exitCode === 0) result.localDeleted = true;
2706+
else result.localError = deleted.stderr || deleted.stdout || `Failed to delete local branch ${branchName}`;
2707+
}
2708+
}
2709+
2710+
if (args.deleteRemoteBranch) {
2711+
const remote = args.remoteName?.trim() || "origin";
2712+
const remoteCheck = await runGit(["remote", "get-url", remote], { cwd: projectRoot, timeoutMs: 8_000 });
2713+
if (remoteCheck.exitCode !== 0) {
2714+
result.remoteError = `Remote '${remote}' is not configured for this repository`;
2715+
} else {
2716+
const remoteRefCheck = await runGit(["ls-remote", "--heads", remote, branchName], {
2717+
cwd: projectRoot,
2718+
timeoutMs: 12_000,
2719+
});
2720+
if (remoteRefCheck.exitCode === 0 && remoteRefCheck.stdout.trim().length > 0) {
2721+
const deleted = await runGit(["push", remote, "--delete", branchName], { cwd: projectRoot, timeoutMs: 45_000 });
2722+
if (deleted.exitCode === 0) result.remoteDeleted = true;
2723+
else result.remoteError = deleted.stderr || deleted.stdout || `Failed to delete remote branch ${branchName}`;
2724+
}
2725+
}
2726+
}
2727+
2728+
if (result.localError || result.remoteError) {
2729+
logger.warn("prs.branch_cleanup_partial_failure", result);
2730+
}
2731+
return result;
2732+
};
2733+
26562734
const land = async (args: LandPrArgs): Promise<LandResult> => {
26572735
const row = getRow(args.prId);
26582736
if (!row) throw new Error(`PR not found: ${args.prId}`);
@@ -5034,6 +5112,10 @@ export function createPrService({
50345112
return await linkToLane(args);
50355113
},
50365114

5115+
async cleanupBranch(args: CleanupPrBranchArgs): Promise<CleanupPrBranchResult> {
5116+
return await cleanupBranch(args);
5117+
},
5118+
50375119
getForLane(laneId: string): PrSummary | null {
50385120
const row = getRowForLane(laneId);
50395121
return row ? rowToSummary(row) : null;
@@ -5306,6 +5388,12 @@ export function createPrService({
53065388
return files;
53075389
},
53085390

5391+
async getCommits(prId: string): Promise<PrCommit[]> {
5392+
const commits = await getCommitsSnapshot(prId);
5393+
upsertSnapshotRow({ prId, commits });
5394+
return commits;
5395+
},
5396+
53095397
async getActionRuns(prId: string): Promise<PrActionRun[]> {
53105398
const row = requireRow(prId);
53115399
const repo = repoFromRow(row);

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

Lines changed: 39 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,41 @@ 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|won'?t|can'?t|cannot|hasn'?t|haven'?t)\s+(fix|address|resolve|handle|be\s+(fixed|addressed|resolved|done|handled))\b/i,
37+
/\b(will|to\s+be)\s+(fixed|addressed|resolved|done|handled)\s+(in|by)\b/i,
38+
/\bstill\s+(broken|failing|an\s+issue)\b/i,
39+
];
40+
41+
// ACK patterns are intentionally narrow: they must look like an actual
42+
// acknowledgement rather than any mention of the verbs "fixed/resolved/done".
43+
// `looksLikeResolutionAck` auto-flips issue inventory items to `"fixed"`, so
44+
// false positives silently drop live review threads from the convergence loop.
45+
const RESOLUTION_ACK_PATTERNS = [
46+
// Terse standalone acks: "fixed.", "done!", "resolved"
47+
/^\s*(fixed|addressed|resolved|done|handled)[.! ]*$/i,
48+
// First-person / subject acks: "I fixed", "we addressed", "this is fixed"
49+
/\b(i|we|i've|we've|this\s+(is|was)|that\s+(is|was)|it\s+(is|was))\s+(now\s+)?(fixed|addressed|resolved|done|handled)\b/i,
50+
// Commit/PR-reference acks: "Fixed in commit abc", "addressed in #123",
51+
// "resolved in the latest push". Scoped to concrete references to avoid
52+
// matching deferrals like "addressed in a follow-up PR".
53+
/^\s*(fixed|addressed|resolved|handled)\s+(in|by)\s+(the\s+latest|commit\b|#\d+|[0-9a-f]{7,40}\b|pr\s*#?\d+|my\s+(latest|most\s+recent))/i,
54+
/\bshould be (good|fixed|resolved)\b/i,
55+
/\bno longer (an issue|applies|reproduces)\b/i,
56+
/\bthanks[,! ]+(fixed|addressed|resolved)\b/i,
57+
/\bclear-to-merge\b/i,
58+
/\bci green\b/i,
59+
];
60+
61+
export function looksLikeResolutionAck(body: string | null | undefined): boolean {
62+
const text = (body ?? "").trim();
63+
if (!text) return false;
64+
if (RESOLUTION_NEGATION_PATTERNS.some((pattern) => pattern.test(text))) return false;
65+
return RESOLUTION_ACK_PATTERNS.some((pattern) => pattern.test(text));
66+
}
67+
2968
/**
3069
* Map ADE's permission mode to the agent chat permission mode.
3170
* 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)