Skip to content

Commit 8b199d6

Browse files
arul28claude
andcommitted
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>
1 parent 7b14693 commit 8b199d6

4 files changed

Lines changed: 48 additions & 7 deletions

File tree

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 }));

apps/desktop/src/renderer/components/prs/prsRouteState.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,4 +140,19 @@ describe("resolvePrsActiveTab", () => {
140140
expect(resolved.isWorkflowRoute).toBe(false);
141141
expect(resolved.activeTab).toBe("normal");
142142
});
143+
144+
it("prefers the hash workflow over a stale outer search workflow (BrowserRouter mock mode)", () => {
145+
// Outer URL like `/?tab=workflows&workflow=queue#/prs?tab=workflows&workflow=rebase`
146+
// In BrowserRouter mock mode the outer search is stale; the inner hash is the
147+
// current in-app location and must win.
148+
const parsed = parsePrsRouteState({
149+
search: "?tab=workflows&workflow=queue",
150+
hash: "#/prs?tab=workflows&workflow=rebase",
151+
});
152+
expect(parsed.workflowTab).toBe("rebase");
153+
const resolved = resolvePrsActiveTab(parsed);
154+
expect(resolved.isWorkflowRoute).toBe(true);
155+
expect(resolved.effectiveWorkflow).toBe("rebase");
156+
expect(resolved.activeTab).toBe("rebase");
157+
});
143158
});

apps/desktop/src/renderer/components/prs/prsRouteState.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,19 @@ export function parsePrsRouteState(args: { search?: string | null; hash?: string
4848
const pick = (key: string): string | null =>
4949
parseOptionalId(searchParams.get(key)) ?? parseOptionalId(hashParams.get(key));
5050

51+
// Workflow precedence: when both search and hash carry a workflow value, the
52+
// hash is authoritative. In BrowserRouter mock mode (e.g. when the outer
53+
// location is `?tab=workflows&workflow=queue#/prs?...&workflow=rebase`), the
54+
// inner hash is the current in-app location; the outer search may be stale
55+
// and must not mask it. Still fall back to search when hash has no workflow.
56+
const hashWorkflowRaw = hashParams.get("workflow");
57+
const searchWorkflowRaw = searchParams.get("workflow");
58+
const hashWorkflow = parseWorkflowTab(hashWorkflowRaw);
59+
const workflowTab = hashWorkflow ?? parseWorkflowTab(searchWorkflowRaw);
60+
5161
return {
5262
tab: parseTab(searchParams.get("tab") ?? hashParams.get("tab")),
53-
workflowTab: parseWorkflowTab(searchParams.get("workflow") ?? hashParams.get("workflow")),
63+
workflowTab,
5464
laneId: pick("laneId"),
5565
prId: pick("prId"),
5666
queueGroupId: pick("queueGroupId"),

apps/desktop/src/renderer/components/terminals/useWorkSessions.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,18 @@ export function useWorkSessions() {
623623
const laneExists = laneParam && lanes.some((lane) => lane.id === laneParam);
624624
const status = mapUrlStatusFilter(statusParam);
625625
if (!laneExists && !status) return;
626-
appliedUrlFilterKeyRef.current = urlKey;
626+
// When the URL specifies a laneId but lanes haven't populated yet (e.g. on
627+
// project open/switch the store resets lanes to [] then repopulates async),
628+
// we can't tell whether the lane is missing-for-good or just-not-yet-loaded.
629+
// In that case, apply any status hint but don't cache the URL signature —
630+
// come back once lanes populate so the lane portion can apply too. We only
631+
// mark the key applied when the lane portion was definitively applied, or
632+
// when lanes are loaded (non-empty) so "not found" is an authoritative
633+
// negative signal.
634+
const laneDeterminable = !laneParam || laneExists || lanes.length > 0;
635+
if (laneDeterminable) {
636+
appliedUrlFilterKeyRef.current = urlKey;
637+
}
627638
setProjectViewState((prev) => ({
628639
...prev,
629640
laneFilter: laneExists ? laneParam : prev.laneFilter,

0 commit comments

Comments
 (0)