test(cards): pin a second card click during a pending navigation

[dawsontoth]

⊙ Problem

#1301 reported repeated ids in the URL after clicking a sign-in link again while the first navigation was still loading. I reproduced it on stage, and the cause is the relative card links that #1798 makes absolute.

A card click calls router.navigate with the card's own options. TanStack resolves a relative target against the pending location: router-core buildLocation resolves against dest.from ?? currentMatch().fullPath of _pendingLocation || latestLocation. The card is still on screen until the next route finishes loading, so a second click appends the target to the first one. A second click on an organization card lands on /org-a/org-a, and a second Direct Sign In from a cluster card lands on /org-a/clu-b/sign-in/clu-b/sign-in.

❓ Your call: this PR is test-only, stacked on #1798, because #1798 (for #1711) already makes exactly these targets absolute. I first wrote the same production fix independently, and the pre-push review caught the overlap, so I dropped mine rather than open a duplicate. What this adds is the click-during-a-pending-navigation regression that #1301 describes, which #1798's location-independence tests do not exercise. If you would rather land one PR, fold this test file into #1798 and close this one. Either way #1798 is what fixes #1301.

💡 Solution

One new test file, cardLinksDuringPendingNavigation.test.tsx. It renders the real OrgCard and ClusterCard under a real router whose next route never finishes loading and clicks twice: a second organization card click, the organization menu's hrefs from the pending location, and a second Direct Sign In. It does not debounce or disable a card while its navigation is pending: with absolute targets a second click goes to the same URL, so it does no harm.

✅ Verification

End-to-end route: the real cards under a real TanStack router with a never-settling route load, which is the report's slow-network double click, minus a browser.

• On stage (dcaeb6c93, the same OrgCard/ClusterCard as this stack's base): all three fail, with Received: "/org-a/org-a" and Received: "/org-a/clu-b/sign-in/clu-b/sign-in". On fix: org and cluster card menu links no longer resolve one segment too deep #1798's head (6ba1b9cc9) plus this commit: all three pass.
• npx vitest run: 399 files, 3695 passed, 11 skipped, exit 0 (the pre-commit hook's run). npx tsc -b, npx oxlint and npx dprint check all exit 0.
• Cross-model: one full round by codex and gemini on the test, plus one round on my dropped production version. That round is the one that found fix: org and cluster card menu links no longer resolve one segment too deep #1798. Codex had no findings. Gemini's two were rejected: it called the exact menu-href list brittle, but the exact list is the point, since a new menu item must also be shown absolute; and it flagged the three test comments, which state the pending precondition each assertion depends on. The Harper domain adjudicator failed on an expired Claude OAuth session, and the Cursor lenses failed on the 1Password SSH agent.

Stack: #1798 (#1711, base stage) → this PR. Closes #1301.

🤖 Generated with Claude Code

Related PRs: #1798 overlaps (stack base; its absolute card targets are the fix this test pins), 1 others independent
Complexity: easy

_{Review-Coverage: authored=claude; ran=codex,gemini; blocked=cursor-composer(no-receipt),domain(auth); declined=cursor-grok,cursor-kimi,cursor-muse; rounds=1; full=1 @ dbd09d8}

_{Review-Attention: skim ~2m (raised: degraded review) @ dbd09d8}

[gemini-code-assist]

Code Review

This pull request introduces a new test suite, cardLinksDuringPendingNavigation.test.tsx, to verify navigation behavior when organization or cluster cards are clicked while a previous navigation is still loading. The review feedback highlights several improvement opportunities: stabilizing the mock references for useLocalStorage and useTerminateClusterMutation to prevent unnecessary re-renders, instantiating QueryClient outside of the JSX render call to maintain a stable reference, and removing redundant act wrappers around fireEvent in the test helper functions.

[gemini-code-assist]

The mock for useLocalStorage returns a new array containing a new mock function reference (vi.fn()) on every invocation. Since the setter function returned by useLocalStorage is used in a dependency array, this unstable reference will cause unnecessary recreations. To ensure a stable reference, define a stable mock function outside the factory. Additionally, to guard against unstable references, include a test assertion that re-renders the component without changing anything and verifies that the effect does not re-run.

const mockSetSavedClusterState = vi.fn();
vi.mock('@/hooks/useLocalStorage', () => ({ useLocalStorage: () => [null, mockSetSavedClusterState] }));

References

1. When mocking a hook whose return value is used in a useEffect dependency array, ensure the mock returns a stable object reference (e.g., by instantiating the object once in the mock factory and using getters for dynamic properties). To guard against unstable references, include a test assertion that re-renders the component without changing anything and verifies that the effect does not re-run.

[gemini-code-assist]

The mock for useTerminateClusterMutation returns a new object with a new mutate function reference on every call. Since terminateCluster is used in a dependency array, this unstable reference will cause unnecessary recreations. We should use a stable mock reference to prevent this. Additionally, to guard against unstable references, include a test assertion that re-renders the component without changing anything and verifies that the effect does not re-run.

Suggested change

	vi.mock(
	'@/features/clusters/mutations/terminateCluster',
	() => ({ useTerminateClusterMutation: () => ({ mutate: vi.fn(), isPending: false }) }),
	);
	const mockTerminate = vi.fn();
	vi.mock(
	'@/features/clusters/mutations/terminateCluster',
	() => ({ useTerminateClusterMutation: () => ({ mutate: mockTerminate, isPending: false }) }),
	);

References

1. When mocking a hook whose return value is used in a useEffect dependency array, ensure the mock returns a stable object reference (e.g., by instantiating the object once in the mock factory and using getters for dynamic properties). To guard against unstable references, include a test assertion that re-renders the component without changing anything and verifies that the effect does not re-run.

[gemini-code-assist]

Instantiating new QueryClient() inline inside the render call of renderRouter creates a new QueryClient instance on every render of the wrapper. It is a best practice to instantiate the QueryClient outside of the JSX to ensure a stable reference and avoid wiping out the cache if the wrapper component re-renders.

Suggested change

	render(
	<QueryClientProvider client={new QueryClient()}>
	<RouterProvider router={router as never} />
	</QueryClientProvider>,
	);
	const queryClient = new QueryClient();
	render(
	<QueryClientProvider client={queryClient}>
	<RouterProvider router={router as never} />
	</QueryClientProvider>,
	);

[gemini-code-assist]

React Testing Library's fireEvent already wraps event dispatches in act internally. Wrapping them in act manually here is redundant and adds unnecessary boilerplate. We can simplify these helper functions by removing the redundant act wrappers.

async function openOptions(name: string) {
	const trigger = await screen.findByRole('button', { name });
	fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false });
}

async function click(element: HTMLElement) {
	fireEvent.click(element);
}