Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3074,8 +3074,10 @@ function parseCreatePrArgs(value: Record<string, unknown>): CreatePrFromLaneArgs
if (!laneId || !title) throw new Error("prs.createFromLane requires laneId and title.");
const strategy: CreatePrFromLaneArgs["strategy"] =
normalizePrCreationStrategy(asTrimmedString(value.strategy)) ?? undefined;
const sessionId = asTrimmedString(value.sessionId);
return {
laneId,
...(sessionId ? { sessionId } : {}),
title,
body,
draft: value.draft === true,
Expand All @@ -3089,9 +3091,11 @@ function parseCreatePrArgs(value: Record<string, unknown>): CreatePrFromLaneArgs
}

function parseLinkPrToLaneArgs(value: Record<string, unknown>): LinkPrToLaneArgs {
const sessionId = asTrimmedString(value.sessionId);
return {
laneId: requireString(value.laneId, "prs.linkToLane requires laneId."),
prUrlOrNumber: requireString(value.prUrlOrNumber, "prs.linkToLane requires prUrlOrNumber."),
...(sessionId ? { sessionId } : {}),
};
}

Expand Down
14 changes: 7 additions & 7 deletions apps/desktop/src/main/services/lanes/laneService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5749,7 +5749,7 @@ describe("laneService - branchSwitch", () => {
}
});

it("preserves PR rows whose head_branch matches the new branch and deletes stale ones", async () => {
it("preserves current PR rows and retains previous-branch PR history", async () => {
const repoRoot = makeTempRepoRoot("ade-bsw-switch-pr-detach-");
const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger());
try {
Expand Down Expand Up @@ -5797,22 +5797,22 @@ describe("laneService - branchSwitch", () => {
);
expect(keep?.lane_id).toBe("lane-a");

// Stale rows are detached, not deleted — the PR happened on this lane even
// though the lane now tracks a different branch.
// Branch switching changes the lane's current role, not the PR's lane
// ownership. The old row remains available as previous-branch history.
const stale = db.get<{ lane_id: string | null; detached_at: string | null; detached_lane_name: string | null }>(
"select lane_id, detached_at, detached_lane_name from pull_requests where id = ?",
["pr-stale"],
);
expect(stale?.detached_at).toBeTruthy();
expect(stale?.detached_at).toBeNull();
expect(stale?.lane_id).toBe("lane-a");
// A detached row must not be treated as live: the kept row is the only one
// still claiming the lane's current branch.
// Both rows still belong to the lane; the renderer derives active vs
// previous from the lane branch and each PR head branch.
expect(
db.get<{ count: number }>(
"select count(1) as count from pull_requests where lane_id = ? and detached_at is null",
["lane-a"],
)?.count,
).toBe(1);
).toBe(2);
} finally {
db.close();
fs.rmSync(repoRoot, { recursive: true, force: true });
Expand Down
58 changes: 4 additions & 54 deletions apps/desktop/src/main/services/lanes/laneService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { AsyncLocalStorage } from "node:async_hooks";
import { randomUUID } from "node:crypto";
import type { AdeDb } from "../state/kvDb";
import { getHeadSha, runGit, runGitOrThrow } from "../git/git";
import { detachPullRequestRowsByIds, detachPullRequestRowsForLane } from "../prs/pullRequestRowCleanup";
import { detachPullRequestRowsForLane } from "../prs/pullRequestRowCleanup";
import { isWithinDir, normalizeBranchName, resolvePathWithinRoot } from "../shared/utils";
import { fetchRemoteTrackingBranch } from "../shared/remoteTrackingBranch";
import { pathsEqual } from "../shared/pathCompare";
Expand Down Expand Up @@ -4910,11 +4910,9 @@ export function createLaneService({
};
}

// Wrap the profile upsert + lanes update + stale-PR cleanup in a single
// transaction so a partial failure can't leave the lane row referencing
// the new branch while the orphaned PR rows linger (or vice versa), or
// leave the post-checkout profile written without the matching lanes
// row update.
// Wrap the profile upsert + lanes update in a single transaction so a
// partial failure can't leave the lane row referencing the new branch
// while the profile still points at the old one.
db.run("begin");
try {
if (pendingProfileUpsert) {
Expand All @@ -4934,30 +4932,6 @@ export function createLaneService({
`,
[targetBranchRef, baseRef, parentLaneId, row.id, projectId],
);
// PR rows whose head_branch no longer matches the lane's current branch are
// stale references and must not bleed into PR lookups. Detach rather than
// delete: the PR still happened on this lane, and erasing it is what made
// merged PRs render as `unmapped`. detached_at takes them out of live lookups
// while keeping the history.
const stalePrRows = db.all<{ id: string }>(
`
select id from pull_requests
where lane_id = ?
and project_id = ?
and head_branch <> ?
`,
[row.id, projectId, targetBranchRef],
);
if (stalePrRows.length > 0) {
detachPullRequestRowsByIds(db, {
projectId,
laneId: row.id,
laneName: row.name ?? null,
laneColor: row.color ?? null,
detachedAt: new Date().toISOString(),
prIds: stalePrRows.map((r) => r.id),
});
}
db.run("commit");
} catch (err) {
try { db.run("rollback"); } catch { /* swallow rollback failures */ }
Expand Down Expand Up @@ -5128,30 +5102,6 @@ export function createLaneService({
projectId,
],
);
// Same rationale as switchBranch: PR rows whose head branch no longer
// matches the lane are stale references and must not bleed into PR
// lookups now that the lane tracks a different branch. Detached, not deleted,
// so the merged view keeps the record. `row` is the pre-update snapshot, so
// `row.name` is the name these PRs were actually built under.
const stalePrRows = db.all<{ id: string }>(
`
select id from pull_requests
where lane_id = ?
and project_id = ?
and head_branch <> ?
`,
[row.id, projectId, targetBranchRef],
);
if (stalePrRows.length > 0) {
detachPullRequestRowsByIds(db, {
projectId,
laneId: row.id,
laneName: row.name ?? null,
laneColor: row.color ?? null,
detachedAt: new Date().toISOString(),
prIds: stalePrRows.map((entry) => entry.id),
});
}
db.run("commit");
} catch (err) {
try { db.run("rollback"); } catch { /* swallow rollback failures */ }
Expand Down
36 changes: 36 additions & 0 deletions apps/desktop/src/main/services/prs/prAsync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,42 @@ describe("prMergeAutoSettlementService", () => {
);
});

it("settles only the chats explicitly linked to a merged PR", async () => {
const db = createMemoryDb();
const settleSessionsWithOutcome = vi.fn((ids: string[]) => ids);
const getSettlementBlockers = vi.fn(async () => []);
const service = createPrMergeAutoSettlementService({
db: db as any,
sessionService: {
list: vi.fn(() => [
{ id: "chat-owned", toolType: "codex-chat", archivedAt: null, settledAt: null },
{ id: "chat-other", toolType: "codex-chat", archivedAt: null, settledAt: null },
]),
settleSessionsWithOutcome,
} as any,
agentChatService: { getSettlementBlockers } as any,
emitEvent: vi.fn(),
});
const openPr = createSummary({ state: "open" });
await service.processSnapshot({ prs: [openPr], polledAt: "2026-03-24T12:00:00.000Z" });

const mergedPr = createSummary({
state: "merged",
mergedAt: "2026-03-24T12:01:00.000Z",
chatSessionIds: ["chat-owned"],
});
await service.processSnapshot({ prs: [mergedPr], polledAt: "2026-03-24T12:01:05.000Z" });

expect(getSettlementBlockers).toHaveBeenCalledTimes(1);
expect(getSettlementBlockers).toHaveBeenCalledWith("chat-owned", { includeCurrentTurn: true });
expect(settleSessionsWithOutcome).toHaveBeenCalledWith(
["chat-owned"],
"PR #101 merged",
"2026-03-24T12:01:05.000Z",
"pr_merge",
);
});

it("settles a PR that was already merged when first seen, but announces nothing", async () => {
// The machine-switch bug. Point the project tab at another machine and that
// machine reconciles, backfilling rows for PRs it had never stored. Every
Expand Down
47 changes: 47 additions & 0 deletions apps/desktop/src/main/services/prs/prChatCards.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
buildPrReviewCard,
emitPrCardsForChange,
selectPrCardSession,
selectPrCardSessions,
} from "./prChatCards";

function pr(overrides: Partial<PrSummary> = {}): PrSummary {
Expand Down Expand Up @@ -122,6 +123,18 @@ describe("PR chat cards", () => {
])?.sessionId).toBe("newer");
});

it("selects every explicitly linked Work chat for a PR", () => {
expect(selectPrCardSessions([
session("older", "2026-07-27T10:00:00.000Z"),
session("newer", "2026-07-27T12:00:00.000Z"),
session("personal", "2026-07-27T13:00:00.000Z", { surface: "personal" }),
session("archived", "2026-07-27T14:00:00.000Z", { archivedAt: "2026-07-27T14:01:00.000Z" }),
], ["older", "newer", "personal", "archived"]).map((entry) => entry.sessionId)).toEqual([
"newer",
"older",
]);
});

it("builds one live CI episode keyed by head and attempt", () => {
const card = buildPrCiCard({ pr: pr(), runs: [run()], checks: [check("external", { appSlug: "vercel" })] });
expect(card).toMatchObject({
Expand Down Expand Up @@ -451,6 +464,40 @@ describe("PR chat cards", () => {
expect(emitAdeCard.mock.calls.every(([call]) => call.sessionId === "newer")).toBe(true);
});

it("fans a PR transition out to every linked Work chat", async () => {
const emitAdeCard = vi.fn().mockResolvedValue(undefined);
const count = await emitPrCardsForChange({
change: {
pr: pr({
chatSessionIds: ["newer", "older"],
checksStatus: "failing",
}),
previousState: "open",
previousChecksStatus: "passing",
previousReviewStatus: "approved",
previousMergeConflicts: false,
previousBehindBaseBy: 0,
},
dataSource: {
getActionRuns: vi.fn().mockResolvedValue([run()]),
getChecks: vi.fn().mockResolvedValue([]),
getReviews: vi.fn().mockResolvedValue([]),
getReviewThreads: vi.fn().mockResolvedValue([]),
},
chat: {
listSessions: vi.fn().mockResolvedValue([
session("older", "2026-07-27T10:00:00.000Z"),
session("newer", "2026-07-27T12:00:00.000Z"),
session("other-lane", "2026-07-27T13:00:00.000Z", { laneId: "lane-2" }),
]),
emitAdeCard,
},
});

expect(count).toBe(1);
expect(emitAdeCard.mock.calls.map(([call]) => call.sessionId).sort()).toEqual(["newer", "older"]);
});

it("emits a merge-ready episode once when the prior state was not ready", async () => {
const emitAdeCard = vi.fn().mockResolvedValue(undefined);
const count = await emitPrCardsForChange({
Expand Down
42 changes: 32 additions & 10 deletions apps/desktop/src/main/services/prs/prChatCards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,31 @@ function formatContextList(contexts: readonly string[], limit = MISSING_REQUIRED
export function selectPrCardSession(
sessions: AgentChatSessionSummary[],
): AgentChatSessionSummary | null {
return sessions
return selectPrCardSessions(sessions)[0] ?? null;
}

/**
* Route a PR card to every live chat that explicitly worked on the PR. Older
* PR rows have no edge, so they retain the historic single most-recent-lane
* fallback instead of disappearing from chat altogether.
*/
export function selectPrCardSessions(
sessions: AgentChatSessionSummary[],
chatSessionIds?: readonly string[] | null,
): AgentChatSessionSummary[] {
const eligible = sessions
.filter((session) => (session.surface ?? "work") === "work" && session.archivedAt == null)
.sort((left, right) => (
timeMs(right.lastActivityAt) - timeMs(left.lastActivityAt)
|| right.sessionId.localeCompare(left.sessionId)
))[0] ?? null;
));
const linkedIds = new Set(
(chatSessionIds ?? []).map((sessionId) => String(sessionId ?? "").trim()).filter(Boolean),
);
if (linkedIds.size > 0) {
return eligible.filter((session) => linkedIds.has(session.sessionId));
}
return eligible.slice(0, 1);
}

/**
Expand Down Expand Up @@ -470,10 +489,11 @@ export async function emitPrCardsForChange(args: {
return 0;
}

const session = selectPrCardSession(
const sessions = selectPrCardSessions(
await chat.listSessions(pr.laneId, { includeArchived: false }),
pr.chatSessionIds,
);
if (!session) return 0;
if (sessions.length === 0) return 0;

const cards: AdeCardPayload[] = [];
if (checksChanged) {
Expand Down Expand Up @@ -512,12 +532,14 @@ export async function emitPrCardsForChange(args: {
cards.push(buildPrMergedCard(pr));
}

const results = await Promise.allSettled(cards.map((card) => (
chat.emitAdeCard({
sessionId: session.sessionId,
card,
})
)));
const results = await Promise.allSettled(
sessions.flatMap((session) => cards.map((card) => (
chat.emitAdeCard({
sessionId: session.sessionId,
card,
})
))),
);
const failures = results.filter((result) => result.status === "rejected");
if (failures.length > 0) {
throw new AggregateError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,17 @@ export function createPrMergeAutoSettlementService(args: {
);

for (const pr of candidates) {
const linkedChatSessionIds = new Set(
(pr.chatSessionIds ?? []).map((sessionId) => String(sessionId ?? "").trim()).filter(Boolean),
);
const rows = args.sessionService.list({
laneId: pr.laneId,
limit: 500,
}).filter((session) =>
!session.archivedAt
&& !session.settledAt
&& (isChatToolType(session.toolType) || isTrackedAgentCliToolType(session.toolType)),
);
).filter((session) => linkedChatSessionIds.size === 0 || linkedChatSessionIds.has(session.id));

const settledSessionIds: string[] = [];
for (const session of rows) {
Expand Down
Loading
Loading