Skip to content

Commit 15de4ae

Browse files
arul28claude
arul28
and
claude
committed
Fix PR review comments: fallback refs, validation, a11y, toast UX [skip ci]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 784a509 commit 15de4ae

6 files changed

Lines changed: 121 additions & 24 deletions

File tree

‎apps/desktop/src/main/services/conflicts/conflictService.test.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,6 +1112,85 @@ describe("conflictService conflict context integrity", () => {
11121112
}
11131113
});
11141114

1115+
it("falls back to local branchRef when origin/ remote ref is unavailable for primary parent", async () => {
1116+
const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-primary-fallback-"));
1117+
const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger());
1118+
const projectId = randomUUID();
1119+
const now = "2026-03-24T12:00:00.000Z";
1120+
1121+
try {
1122+
db.run(
1123+
"insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)",
1124+
[projectId, repoRoot, "demo", "main", now, now]
1125+
);
1126+
1127+
fs.writeFileSync(path.join(repoRoot, "file.txt"), "base\n", "utf8");
1128+
git(repoRoot, ["init", "-b", "main"]);
1129+
git(repoRoot, ["config", "user.email", "ade@test.local"]);
1130+
git(repoRoot, ["config", "user.name", "ADE Test"]);
1131+
git(repoRoot, ["add", "."]);
1132+
git(repoRoot, ["commit", "-m", "base"]);
1133+
1134+
// Primary parent with local branch but no origin remote
1135+
git(repoRoot, ["checkout", "-b", "feature/primary-local"]);
1136+
fs.writeFileSync(path.join(repoRoot, "parent.txt"), "parent\n", "utf8");
1137+
git(repoRoot, ["add", "parent.txt"]);
1138+
git(repoRoot, ["commit", "-m", "parent advance"]);
1139+
1140+
git(repoRoot, ["checkout", "-b", "feature/child-of-primary"]);
1141+
git(repoRoot, ["checkout", "feature/primary-local"]);
1142+
fs.writeFileSync(path.join(repoRoot, "parent2.txt"), "more parent\n", "utf8");
1143+
git(repoRoot, ["add", "parent2.txt"]);
1144+
git(repoRoot, ["commit", "-m", "parent advance again"]);
1145+
git(repoRoot, ["checkout", "feature/child-of-primary"]);
1146+
1147+
const parentLane = {
1148+
...createLaneSummary(repoRoot, {
1149+
id: "lane-primary",
1150+
name: "Primary",
1151+
branchRef: "feature/primary-local",
1152+
baseRef: "main",
1153+
parentLaneId: null,
1154+
}),
1155+
laneType: "primary" as const,
1156+
};
1157+
const childLane = createLaneSummary(repoRoot, {
1158+
id: "lane-child-primary",
1159+
name: "Child",
1160+
branchRef: "feature/child-of-primary",
1161+
baseRef: "main",
1162+
parentLaneId: "lane-primary",
1163+
});
1164+
1165+
const service = createConflictService({
1166+
db,
1167+
logger: createLogger(),
1168+
projectId,
1169+
projectRoot: repoRoot,
1170+
laneService: {
1171+
list: async () => [parentLane, childLane],
1172+
getLaneBaseAndBranch: () => ({ worktreePath: repoRoot, baseRef: "main", branchRef: "feature/child-of-primary" }),
1173+
} as any,
1174+
projectConfigService: {
1175+
get: () => ({ effective: { providerMode: "guest" }, local: {} }),
1176+
} as any,
1177+
});
1178+
1179+
// origin/feature/primary-local doesn't exist, but the fallback to local
1180+
// feature/primary-local should still detect the child is behind.
1181+
const needs = await service.scanRebaseNeeds();
1182+
expect(needs).toHaveLength(1);
1183+
expect(needs[0]).toMatchObject({
1184+
laneId: "lane-child-primary",
1185+
baseBranch: "feature/primary-local",
1186+
});
1187+
expect(needs[0]!.behindBy).toBeGreaterThan(0);
1188+
} finally {
1189+
db.close();
1190+
fs.rmSync(repoRoot, { recursive: true, force: true });
1191+
}
1192+
});
1193+
11151194
it("falls back to lane.baseRef when parent lane has no branchRef", async () => {
11161195
const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-no-parent-branch-"));
11171196
const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger());

‎apps/desktop/src/main/services/conflicts/conflictService.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ function resolveLaneRebaseTarget(args: {
276276
queueOverride: QueueRebaseOverride | null;
277277
}): {
278278
comparisonRef: string;
279+
fallbackRef?: string;
279280
displayBaseBranch: string;
280281
} {
281282
if (args.queueOverride) {
@@ -290,10 +291,12 @@ function resolveLaneRebaseTarget(args: {
290291
if (parentBranchRef) {
291292
// For primary lanes, prefer the remote tracking ref (origin/<branch>) to stay
292293
// consistent with laneService.resolveParentRebaseTarget which rebases against
293-
// the remote tracking ref rather than the local HEAD.
294+
// the remote tracking ref rather than the local HEAD. Fall back to the local
295+
// branch ref when the remote ref is unavailable (e.g. before first fetch).
294296
const comparisonRef = parent?.laneType === "primary" ? `origin/${parentBranchRef}` : parentBranchRef;
295297
return {
296298
comparisonRef,
299+
fallbackRef: parentBranchRef,
297300
displayBaseBranch: parentBranchRef,
298301
};
299302
}
@@ -4221,12 +4224,15 @@ export function createConflictService({
42214224
projectRoot,
42224225
laneId: lane.id,
42234226
});
4224-
const { comparisonRef, displayBaseBranch } = resolveLaneRebaseTarget({
4227+
const { comparisonRef, fallbackRef, displayBaseBranch } = resolveLaneRebaseTarget({
42254228
lane,
42264229
lanesById,
42274230
queueOverride,
42284231
});
4229-
const baseHead = await readHeadSha(projectRoot, comparisonRef);
4232+
const baseHead = await readHeadSha(projectRoot, comparisonRef)
4233+
.catch(() => fallbackRef ? readHeadSha(projectRoot, fallbackRef) : Promise.reject())
4234+
.catch(() => "");
4235+
if (!baseHead) continue;
42304236
const laneHead = await readHeadSha(lane.worktreePath, "HEAD");
42314237

42324238
// Count how many commits the lane is behind base
@@ -4294,12 +4300,15 @@ export function createConflictService({
42944300
projectRoot,
42954301
laneId: lane.id,
42964302
});
4297-
const { comparisonRef, displayBaseBranch } = resolveLaneRebaseTarget({
4303+
const { comparisonRef, fallbackRef, displayBaseBranch } = resolveLaneRebaseTarget({
42984304
lane,
42994305
lanesById,
43004306
queueOverride,
43014307
});
4302-
const baseHead = await readHeadSha(projectRoot, comparisonRef);
4308+
const baseHead = await readHeadSha(projectRoot, comparisonRef)
4309+
.catch(() => fallbackRef ? readHeadSha(projectRoot, fallbackRef) : Promise.reject())
4310+
.catch(() => "");
4311+
if (!baseHead) return null;
43034312
const laneHead = await readHeadSha(lane.worktreePath, "HEAD");
43044313

43054314
const behindRes = await runGit(

‎apps/desktop/src/renderer/components/app/AppShell.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,7 @@ export function AppShell({ children }: { children: React.ReactNode }) {
710710
type="button"
711711
className="shrink-0 rounded p-1 text-muted-fg transition-colors hover:bg-fg/[0.05] hover:text-fg"
712712
onClick={() => dismissPrToast(toast.id)}
713+
aria-label="Dismiss notification"
713714
title="Dismiss"
714715
>
715716
×
@@ -722,7 +723,7 @@ export function AppShell({ children }: { children: React.ReactNode }) {
722723
key={`${toast.id}-meta-${index}`}
723724
className="inline-flex max-w-full items-center gap-1 rounded-full border border-border/50 bg-black/10 px-2 py-1 text-[10px] text-muted-fg"
724725
>
725-
{index === 0 ? <GitPullRequest size={10} /> : index === 1 ? <GitBranch size={10} /> : <GithubLogo size={10} />}
726+
{item.includes("/") ? <GitBranch size={10} /> : item.includes("#") || (toast.event.repoOwner && item.includes(toast.event.repoOwner)) ? <GithubLogo size={10} /> : <GitPullRequest size={10} />}
726727
<span className="truncate">{item}</span>
727728
</span>
728729
))}
@@ -750,8 +751,10 @@ export function AppShell({ children }: { children: React.ReactNode }) {
750751
tone === "danger" ? "bg-red-300" : tone === "warning" ? "bg-amber-300" : tone === "success" ? "bg-emerald-300" : "bg-[#A78BFA]",
751752
)}
752753
onClick={() => {
753-
void window.ade.prs.openInGitHub(toast.event.prId).catch(() => { });
754-
dismissPrToast(toast.id);
754+
void window.ade.prs.openInGitHub(toast.event.prId).then(
755+
() => dismissPrToast(toast.id),
756+
() => { /* keep toast visible on failure */ },
757+
);
755758
}}
756759
>
757760
<ArrowSquareOut size={12} />

‎apps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsx‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -197,19 +197,25 @@ export function CreateLaneDialog({
197197
) : (
198198
<label className="mt-4 block">
199199
<span className={LABEL_CLASS_NAME}>Base branch on primary</span>
200-
<select
201-
value={createBaseBranch}
202-
onChange={(event) => setCreateBaseBranch(event.target.value)}
203-
className={SELECT_CLASS_NAME}
204-
disabled={busy}
205-
>
206-
{localBranches.map((branch) => (
207-
<option key={branch.name} value={branch.name}>
208-
{branch.name}
209-
{branch.isCurrent ? " (current)" : ""}
210-
</option>
211-
))}
212-
</select>
200+
{localBranches.length > 0 ? (
201+
<select
202+
value={createBaseBranch}
203+
onChange={(event) => setCreateBaseBranch(event.target.value)}
204+
className={SELECT_CLASS_NAME}
205+
disabled={busy}
206+
>
207+
{localBranches.map((branch) => (
208+
<option key={branch.name} value={branch.name}>
209+
{branch.name}
210+
{branch.isCurrent ? " (current)" : ""}
211+
</option>
212+
))}
213+
</select>
214+
) : (
215+
<div className="mt-1 rounded-lg border border-dashed border-white/[0.08] bg-black/10 px-3 py-2 text-sm text-muted-fg">
216+
No local branches found.
217+
</div>
218+
)}
213219
<div className="mt-2 text-xs text-muted-fg/80">
214220
Lane will be created from primary/{createBaseBranch || "..."}.
215221
</div>
@@ -241,7 +247,7 @@ export function CreateLaneDialog({
241247
</Button>
242248
<Button
243249
variant="primary"
244-
disabled={busy || !createLaneName.trim() || (createAsChild && !createParentLaneId)}
250+
disabled={busy || !createLaneName.trim() || (createAsChild ? !createParentLaneId : !createBaseBranch)}
245251
onClick={onSubmit}
246252
>
247253
{buttonLabel(busy, createAsChild, createParentLaneId, createBaseBranch)}

‎apps/desktop/src/renderer/components/lanes/LaneDialogShell.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export function LaneDialogShell({
2222
children: ReactNode;
2323
}) {
2424
return (
25-
<Dialog.Root open={open} onOpenChange={onOpenChange}>
25+
<Dialog.Root open={open} onOpenChange={(next) => { if (!busy || next) onOpenChange(next); }}>
2626
<Dialog.Portal>
2727
<Dialog.Overlay className="fixed inset-0 z-50 bg-black/40 backdrop-blur-sm" />
2828
<Dialog.Content

‎apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1474,7 +1474,7 @@ function OverviewTab(props: OverviewTabProps) {
14741474
color={checksRowVisuals.color}
14751475
icon={getChecksRowIcon(checksSummary)}
14761476
title={checksRowVisuals.title}
1477-
titleAccessory={checksRunning ? <PrCiRunningIndicator showLabel label="running" /> : undefined}
1477+
titleAccessory={checksRunning && checksSummary.total > 0 ? <PrCiRunningIndicator showLabel label="running" /> : undefined}
14781478
description={checksRowVisuals.description}
14791479
expandable={checks.length > 0}
14801480
expanded={checksExpanded}

0 commit comments

Comments
 (0)