Skip to content

Commit 37064e0

Browse files
arul28claude
andauthored
Rebase tab overhaul: rename + URL routing migration (#180)
* ship: checkpoint before automate/finalize * ship: iter 1 — address greptile #3133271501 and copilot #4307244389 - Memoize readInitialRouteState in PrsProvider so the four useState initializers share one window.location read (greptile P2). - Mirror PRsPage's resolveRouteRebaseSelection inside readInitialRouteState so the initial selectedRebaseItemId has the same shape as the post-effect value. - QueueTab.openRebaseTab now navigates to ?tab=workflows&workflow=rebase to keep URL in sync with state. - Normalize the "Open Rebase/Merge tab" label to sentence case across CreatePrModal, IntegrationTab; keep LaneGitActionsPane tooltip title case to match its sibling tooltips. - Restore prRebaseResolver.sendMessage displayText — it is used by agentChatService for the transcript text bubble. Verified locally: desktop tsc clean; 46 affected tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ship: iter 2 — fix 3 capy-ai bugs, unblock test-desktop(5) - Wrap 5 CreatePrModal test render sites in MemoryRouter so the useNavigate() call added in the overhaul no longer trips React Router's invariant. This is the direct cause of the test-desktop(5) CI failure and the bug in capy-ai #3133285481. - In parsePrsRouteState, let hash workflow= win over search workflow= so BrowserRouter mock-mode URLs like ?tab=workflows&workflow=queue#/prs?...&workflow=rebase resolve to the hash's workflow. Adds a reproduction test (capy-ai #3133285495). - In useWorkSessions' URL-filter effect, hold off caching appliedUrlFilterKeyRef when laneId is specified but lanes haven't loaded yet, so a later lanes-populated render can still apply the lane filter instead of early-returning (capy-ai #3133331539). Verified locally: desktop tsc clean; 47 affected tests pass across CreatePrModal, prsRouteState, useWorkSessions, PrsContext. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ship: iteration 3 - address hash route precedence * ship: iteration 5 - address partial URL filter replay * ship: iteration 6 - reset URL filter cache on navigation * ship: iteration 7 - handle delayed PR comments * ship: iteration 8 - align finalize poll cadence * ship: iteration 10 - stabilize terminal fallback test * ship: reset iter 1 - preserve valid search route * ship: reset iter 3 - sync legacy pr routes --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 34f6319 commit 37064e0

23 files changed

Lines changed: 752 additions & 97 deletions

.claude/commands/finalize.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ It guarantees three outcomes:
1212
2. Docs are current
1313
3. Local CI checks pass
1414

15+
It does **not** guarantee that remote PR review is complete after a push. GitHub's
16+
first visible check list can look quiet before delayed checks, bot reviews, and
17+
inline comments arrive. After pushing a finalized branch, hand off to
18+
`/shipLane` or an equivalent PR poll loop. Use the ship-lane cadence: poll
19+
immediately after a push, wait 270s if CI has not registered, wait 720s while CI
20+
is running, and wait 1800s only when CI is done and the PR is just waiting on
21+
review.
22+
1523
**Usage:** `/finalize`
1624

1725
## Execution Mode: Autonomous
@@ -412,6 +420,34 @@ Kill selectively only if the parent is clearly gone (PPID == 1 on macOS/Linux).
412420

413421
Report killed PIDs in the Phase 4 summary under "Cleanup" so the user can see what happened.
414422

423+
### 3k. Remote PR poll handoff
424+
425+
If this finalize run is followed by a push or PR update, do not treat the first
426+
`gh pr checks` result as authoritative proof that remote review is done. Some
427+
checks and bot review systems appear late or post comments after the initial CI
428+
surface looks complete. In particular:
429+
430+
- `gh pr checks` can omit delayed or still-registering provider checks.
431+
- Bot reviewers can post inline comments after CI jobs have already gone green.
432+
- The absence of new comments immediately after a push is not evidence that no
433+
more comments are coming.
434+
435+
Handoff rule:
436+
437+
```bash
438+
# After the branch is pushed, continue with /shipLane or equivalent:
439+
# - poll PR checks, status rollup, review comments, issue comments, and reviews
440+
# - poll immediately after a push so early CI registration/failures are visible
441+
# - if CI has not started yet, wait 270s
442+
# - if any check is QUEUED/IN_PROGRESS/PENDING, wait 720s
443+
# - if CI is done and the PR is only waiting on review, wait 1800s
444+
# - poll again before declaring the PR clean or ready for human merge
445+
```
446+
447+
If `/finalize` is running as a sub-step inside `/shipLane`, return a summary that
448+
explicitly says remote checks/comments still require the ship-lane poll loop.
449+
Do not report "PR clean" from `/finalize` alone.
450+
415451
---
416452

417453
## Phase 4: Summary
@@ -447,6 +483,11 @@ Report killed PIDs in the Phase 4 summary under "Cleanup" so the user can see wh
447483
### Cleanup:
448484
- Orphan processes killed: N (PIDs: [list] or "none")
449485
486+
### Remote PR Handoff:
487+
- Post-push polling required: YES
488+
- Poll loop: `/shipLane` branch-specific cadence
489+
- Reason: delayed checks and bot comments may arrive after first visible green state
490+
450491
### Status: Ready to push / Issues found
451492
```
452493

@@ -466,3 +507,4 @@ Before marking complete:
466507
- [ ] All apps build successfully
467508
- [ ] Doc validation passed
468509
- [ ] Orphan worker processes cleaned up (vitest/tsup/tsc) — scoped to apps/ paths only
510+
- [ ] Remote PR review is not declared clean by finalize alone; after push, `/shipLane` or an equivalent poll loop must use the branch-specific cadence and re-check comments/reviews

apps/desktop/src/main/services/lanes/autoRebaseService.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,17 @@ function blockedMessage(
112112
laneId: string | null,
113113
reason: "conflict" | "manual" | "lookup" | "failed" | "unavailable" | null,
114114
): string {
115-
if (!laneId) return "Pending: auto-rebase stopped at an earlier lane. Open the Rebase tab to continue.";
115+
if (!laneId) return "Pending: auto-rebase stopped at an earlier lane. Open the Rebase/Merge tab to continue.";
116116
if (reason === "manual") {
117-
return `Pending: ancestor lane '${laneId}' has a fixed PR base. Rebase that lane manually from the Rebase tab before descendants can continue.`;
117+
return `Pending: ancestor lane '${laneId}' has a fixed PR base. Rebase that lane manually from the Rebase/Merge tab before descendants can continue.`;
118118
}
119119
if (reason === "lookup" || reason === "unavailable") {
120-
return `Pending: ancestor lane '${laneId}' needs review before descendants can continue. Open the Rebase tab to inspect it.`;
120+
return `Pending: ancestor lane '${laneId}' needs review before descendants can continue. Open the Rebase/Merge tab to inspect it.`;
121121
}
122122
if (reason === "failed") {
123-
return `Pending: ancestor lane '${laneId}' failed automatic rebase. Open the Rebase tab to retry.`;
123+
return `Pending: ancestor lane '${laneId}' failed automatic rebase. Open the Rebase/Merge tab to retry.`;
124124
}
125-
return `Pending: ancestor lane '${laneId}' has unresolved rebase conflicts. Open the Rebase tab to continue.`;
125+
return `Pending: ancestor lane '${laneId}' has unresolved rebase conflicts. Open the Rebase/Merge tab to continue.`;
126126
}
127127

128128
function resolveAffectedChainLaneId(
@@ -459,7 +459,7 @@ export function createAutoRebaseService(args: {
459459
parentHeadSha: null,
460460
state: "rebasePending",
461461
conflictCount: 0,
462-
message: "Pending: parent lane is unavailable. Open the Rebase tab to review the lane."
462+
message: "Pending: parent lane is unavailable. Open the Rebase/Merge tab to review the lane."
463463
});
464464
blocked = true;
465465
blockedLaneId = lane.id;
@@ -522,7 +522,7 @@ export function createAutoRebaseService(args: {
522522
parentHeadSha,
523523
state: "rebaseConflict",
524524
conflictCount: Math.max(1, need.conflictingFiles.length),
525-
message: `Auto-rebase blocked: ${Math.max(1, need.conflictingFiles.length)} conflict(s) expected. Open the Rebase tab to resolve and publish.`
525+
message: `Auto-rebase blocked: ${Math.max(1, need.conflictingFiles.length)} conflict(s) expected. Open the Rebase/Merge tab to resolve and publish.`
526526
});
527527
continue;
528528
}
@@ -541,7 +541,7 @@ export function createAutoRebaseService(args: {
541541
parentHeadSha,
542542
state: "rebasePending",
543543
conflictCount: 0,
544-
message: "PR carries an immutable base — drift detected. Rebase manually from the Rebase tab when ready."
544+
message: "PR carries an immutable base — drift detected. Rebase manually from the Rebase/Merge tab when ready."
545545
});
546546
continue;
547547
}
@@ -572,8 +572,8 @@ export function createAutoRebaseService(args: {
572572
state: conflictHint ? "rebaseConflict" : "rebaseFailed",
573573
conflictCount: conflictHint ? 1 : 0,
574574
message: conflictHint
575-
? "Auto-rebase stopped due to conflicts. Open the Rebase tab to resolve, then publish."
576-
: `Auto-rebase failed: ${rebaseRun.run.error}. Open the Rebase tab to retry.`
575+
? "Auto-rebase stopped due to conflicts. Open the Rebase/Merge tab to resolve, then publish."
576+
: `Auto-rebase failed: ${rebaseRun.run.error}. Open the Rebase/Merge tab to retry.`
577577
});
578578
continue;
579579
}
@@ -625,8 +625,8 @@ export function createAutoRebaseService(args: {
625625
state: "rebaseFailed",
626626
conflictCount: 0,
627627
message: rollbackError
628-
? `Auto-push failed: ${pushError}. Automatic rollback also failed: ${rollbackError}. Open the Rebase tab to retry.`
629-
: `Auto-push failed: ${pushError}. The lane was restored to its pre-rebase state. Open the Rebase tab to retry.`
628+
? `Auto-push failed: ${pushError}. Automatic rollback also failed: ${rollbackError}. Open the Rebase/Merge tab to retry.`
629+
: `Auto-push failed: ${pushError}. The lane was restored to its pre-rebase state. Open the Rebase/Merge tab to retry.`
630630
});
631631
}
632632
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ describe("launchRebaseResolutionChat", () => {
141141
expect(sendMessage).toHaveBeenCalledWith(
142142
expect.objectContaining({
143143
sessionId: "session-rebase-1",
144-
displayText: "Rebase feature/rebase-target onto main",
145144
reasoningEffort: "high",
145+
displayText: "Rebase feature/rebase-target onto main",
146146
}),
147147
);
148148
expect(result).toEqual({

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ export async function launchRebaseResolutionChat(
158158
await deps.agentChatService.sendMessage({
159159
sessionId: session.id,
160160
text: prompt,
161+
// Show the short human-readable title in the transcript instead of the
162+
// full composed prompt (which is long and technical). Mirrors
163+
// prIssueResolver's pattern; agentChatService falls back to `text` when
164+
// displayText is absent, which would surface the raw prompt to the user.
161165
displayText: title,
162166
...(reasoningEffort ? { reasoningEffort } : {}),
163167
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,7 @@ export function createPrService({
871871
parentHeadSha: null,
872872
state: "rebaseFailed",
873873
conflictCount: 0,
874-
message: `Auto-rebase failed after '${args.landedLaneName}' merged because ADE could not find a new parent lane. Open the Rebase tab to recover this lane.`,
874+
message: `Auto-rebase failed after '${args.landedLaneName}' merged because ADE could not find a new parent lane. Open the Rebase/Merge tab to recover this lane.`,
875875
}, child.id);
876876
}
877877
return {
@@ -979,8 +979,8 @@ export function createPrService({
979979
state: "rebaseFailed",
980980
conflictCount: 0,
981981
message: rollbackError
982-
? `Auto-rebase failed after '${args.landedLaneName}' merged: ${childError}. Automatic rollback also failed: ${rollbackError}. Open the Rebase tab to recover this lane.`
983-
: `Auto-rebase failed after '${args.landedLaneName}' merged: ${childError}. The lane was restored to its pre-rebase state. Open the Rebase tab to recover this lane.`,
982+
? `Auto-rebase failed after '${args.landedLaneName}' merged: ${childError}. Automatic rollback also failed: ${rollbackError}. Open the Rebase/Merge tab to recover this lane.`
983+
: `Auto-rebase failed after '${args.landedLaneName}' merged: ${childError}. The lane was restored to its pre-rebase state. Open the Rebase/Merge tab to recover this lane.`,
984984
}, child.id);
985985
failedLaneIds.push(child.id);
986986
}

apps/desktop/src/renderer/components/lanes/LaneGitActionsPane.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ describe("LaneGitActionsPane rescue action", () => {
328328
expect(syncButton.getAttribute("title")).toMatch(/before rebasing and pushing/i);
329329
});
330330

331-
it("treats auto-rebase conflicts as failures and links to the Rebase tab", async () => {
331+
it("treats auto-rebase conflicts as failures and links to the Rebase/Merge tab", async () => {
332332
const user = userEvent.setup();
333333
const resolveRebaseConflict = vi.fn();
334334
mockAutoRebaseStatuses = [
@@ -345,7 +345,7 @@ describe("LaneGitActionsPane rescue action", () => {
345345

346346
renderPane({ onResolveRebaseConflict: resolveRebaseConflict });
347347

348-
const rebaseTabButton = await screen.findByRole("button", { name: /open rebase tab/i });
348+
const rebaseTabButton = await screen.findByRole("button", { name: /open rebase\/merge tab/i });
349349
screen.getByText("AUTO-REBASE FAILED");
350350
screen.getByText(/auto-rebase failed\. files need follow-up before this lane can be pushed\./i);
351351

apps/desktop/src/renderer/components/lanes/LaneGitActionsPane.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,7 @@ export function LaneGitActionsPane({
15461546
onResolveRebaseConflict(laneId, rebaseConflictParentLaneId);
15471547
return;
15481548
}
1549-
const search = new URLSearchParams({ tab: "rebase", laneId });
1549+
const search = new URLSearchParams({ tab: "workflows", workflow: "rebase", laneId });
15501550
if (rebaseConflictParentLaneId) search.set("parentLaneId", rebaseConflictParentLaneId);
15511551
navigate(`/prs?${search.toString()}`);
15521552
};
@@ -1571,8 +1571,8 @@ export function LaneGitActionsPane({
15711571
{autoRebaseStatus.state !== "autoRebased" ? (
15721572
isAutoRebaseFailure ? (
15731573
<SmartTooltip content={{
1574-
label: "Open Rebase Tab",
1575-
description: "View detailed rebase conflict information and resolve issues.",
1574+
label: "Open Rebase/Merge Tab",
1575+
description: "View detailed rebase information and resolve issues.",
15761576
effect: "Navigate to the rebase details view",
15771577
}}>
15781578
<button
@@ -1581,7 +1581,7 @@ export function LaneGitActionsPane({
15811581
disabled={!laneId || busyAction != null}
15821582
onClick={openRebaseTab}
15831583
>
1584-
OPEN REBASE TAB
1584+
OPEN REBASE/MERGE TAB
15851585
</button>
15861586
</SmartTooltip>
15871587
) : (

apps/desktop/src/renderer/components/lanes/LaneRebaseBanner.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,13 @@ export function LaneRebaseBanner({
5050
</div>
5151
</div>
5252
<div className="flex shrink-0 flex-wrap items-center gap-1.5">
53-
<SmartTooltip content={{ label: "View in Rebase tab", description: "Open the Rebase tab for this lane." }}>
53+
<SmartTooltip content={{ label: "View in Rebase/Merge tab", description: "Open the Rebase/Merge tab for this lane." }}>
5454
<button
5555
type="button"
5656
style={primaryButton({ height: 24, padding: "0 8px", fontSize: 10 })}
5757
onClick={() => onViewRebaseDetails(s.laneId)}
5858
>
59-
View in Rebase tab
59+
View in Rebase/Merge tab
6060
</button>
6161
</SmartTooltip>
6262
<SmartTooltip content={{ label: "Dismiss", description: "Remove this rebase suggestion permanently until new parent commits arrive." }}>
@@ -120,13 +120,13 @@ export function LaneRebaseBanner({
120120
</div>
121121
</div>
122122
<div className="shrink-0 flex items-center gap-1.5">
123-
<SmartTooltip content={{ label: "View in Rebase tab", description: "Open the Rebase tab for this lane." }}>
123+
<SmartTooltip content={{ label: "View in Rebase/Merge tab", description: "Open the Rebase/Merge tab for this lane." }}>
124124
<button
125125
type="button"
126126
style={primaryButton({ height: 24, padding: "0 8px", fontSize: 10 })}
127127
onClick={() => onViewRebaseDetails(status.laneId)}
128128
>
129-
View in Rebase tab
129+
View in Rebase/Merge tab
130130
</button>
131131
</SmartTooltip>
132132
<SmartTooltip content={{ label: "Dismiss", description: "Hide this alert until the parent or base changes again." }}>

apps/desktop/src/renderer/components/lanes/LanesPage.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,7 @@ export function LanesPage() {
11011101
const failedLane = start.run.failedLaneId ? lanesById.get(start.run.failedLaneId)?.name ?? start.run.failedLaneId : null;
11021102
const detail = start.run.error ?? "Rebase failed.";
11031103
setRebaseSuggestionError(`Rebase needs attention${failedLane ? ` for ${failedLane}` : ""}. ${detail}`);
1104-
navigate("/prs?tab=rebase");
1104+
navigate("/prs?tab=workflows&workflow=rebase");
11051105
return;
11061106
}
11071107

@@ -1121,7 +1121,7 @@ export function LanesPage() {
11211121
} catch (err) {
11221122
const message = err instanceof Error ? err.message : String(err);
11231123
setRebaseSuggestionError(message);
1124-
navigate("/prs?tab=rebase");
1124+
navigate("/prs?tab=workflows&workflow=rebase");
11251125
}
11261126
}, [lanesById, navigate, refreshLanes, requestPushSelection, requestRebaseScope]);
11271127

apps/desktop/src/renderer/components/prs/CreatePrModal.test.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@ import React from "react";
44
import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
55
import userEvent from "@testing-library/user-event";
66
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
7+
import { MemoryRouter } from "react-router-dom";
78
import type { LaneSummary } from "../../../shared/types";
89

10+
function renderWithRouter(ui: React.ReactElement) {
11+
return render(<MemoryRouter>{ui}</MemoryRouter>);
12+
}
13+
914
function makeLane(overrides: Partial<LaneSummary> = {}): LaneSummary {
1015
return {
1116
id: "lane-1",
@@ -117,7 +122,7 @@ describe("CreatePrModal queue workflow", () => {
117122

118123
it("adds selected lanes to the queue order and removes them from the queue builder", async () => {
119124
const user = userEvent.setup();
120-
render(<CreatePrModal open onOpenChange={vi.fn()} />);
125+
renderWithRouter(<CreatePrModal open onOpenChange={vi.fn()} />);
121126

122127
await user.click(screen.getAllByRole("button", { name: /queue workflow/i })[0]!);
123128
await user.click(screen.getByRole("checkbox", { name: /01 queue lane/i }));
@@ -136,7 +141,7 @@ describe("CreatePrModal queue workflow", () => {
136141

137142
it("uses the dragged queue order when creating queue PRs", async () => {
138143
const user = userEvent.setup();
139-
render(<CreatePrModal open onOpenChange={vi.fn()} />);
144+
renderWithRouter(<CreatePrModal open onOpenChange={vi.fn()} />);
140145

141146
await user.click(screen.getAllByRole("button", { name: /queue workflow/i })[0]!);
142147
await user.click(screen.getByRole("checkbox", { name: /01 queue lane/i }));
@@ -165,7 +170,7 @@ describe("CreatePrModal queue workflow", () => {
165170

166171
it("lets single-PR creation target a different branch than Primary's current branch", async () => {
167172
const user = userEvent.setup();
168-
render(<CreatePrModal open onOpenChange={vi.fn()} />);
173+
renderWithRouter(<CreatePrModal open onOpenChange={vi.fn()} />);
169174

170175
// Select source lane
171176
const comboboxes = screen.getAllByRole("combobox");
@@ -191,7 +196,7 @@ describe("CreatePrModal queue workflow", () => {
191196

192197
it("warns when the PR target branch differs from the lane base branch", async () => {
193198
const user = userEvent.setup();
194-
render(<CreatePrModal open onOpenChange={vi.fn()} />);
199+
renderWithRouter(<CreatePrModal open onOpenChange={vi.fn()} />);
195200

196201
// Select source lane
197202
const comboboxes = screen.getAllByRole("combobox");
@@ -210,7 +215,7 @@ describe("CreatePrModal queue workflow", () => {
210215

211216
it("lets queue creation target a different branch than Primary's current branch", async () => {
212217
const user = userEvent.setup();
213-
render(<CreatePrModal open onOpenChange={vi.fn()} />);
218+
renderWithRouter(<CreatePrModal open onOpenChange={vi.fn()} />);
214219

215220
await user.click(screen.getAllByRole("button", { name: /queue workflow/i })[0]!);
216221
await user.click(screen.getByRole("checkbox", { name: /01 queue lane/i }));

0 commit comments

Comments
 (0)