Skip to content

fix(clusters): Back leaves the new-cluster page after a saved-draft or Stripe redirect - #1851

Open
dawsontoth wants to merge 2 commits into
claude/1311-guide-region-swapfrom
claude/1326-new-cluster-back-button
Open

dawsontoth wants to merge 2 commits into
claude/1311-guide-region-swapfrom
claude/1326-new-cluster-back-button

Conversation

@dawsontoth

@dawsontoth dawsontoth commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

A hedgehog showing that the back door now leads back to the path

⊙ Problem

Back on the new-cluster page could fail to leave it (#1326). The issue guessed "a double navigate without replacement", and that is what it is, though not in ClusterForm.tsx or clusters/routes.ts where triage looked. When a cluster draft is saved (Try Again, a billing redirect, the ?createCluster= deep link), the clusters list redirects to the form with <Navigate>, and that redirect pushed a history entry. The form clears the draft only once it has mounted and loaded its plans, regions and versions.

Reproduced on stage in a real browser before this rebase (Playwright, local dev server, mocked API), going from the org picker to an org with a saved draft:

  • Back from the form landed on the clusters list, a page the user never saw, instead of the picker (history held one entry too many).
  • Back pressed while the form was still loading (plans response delayed 4s) landed on the list, which still had the draft and pushed the form again. The page stayed on #/org-fixture/new-cluster, so Back did nothing.

The normal flows (New Cluster button, Try Again from a card, the billing step, an org with no clusters) already went back correctly and still do. #1780 leaves both redirects as they were (it adds the organization's custom-region names to the list but does not touch the redirect), so the bug is the same on top of it.

Stack: #1780 ← #1842 ← #1849 ← this PR ← #1870. Based on #1849's branch (claude/1311-guide-region-swap); retarget as the PRs below it merge.

❓ Your call: is the requirement right as filed? Yes as far as it goes: redirects should replace. I kept the Stripe return page fix (below) in this PR because it ends on the same form with the same mistake; it is one flag and can be split out if you prefer.

💡 Solution

The list's draft redirect now replaces its history entry, so Back from the form goes to wherever the user was before the list.

Also fixed, same shape: billing's Stripe return page strips the setup-intent secret from its own URL and then pushed the form (or the cluster edit, or the billing page). Back from the form therefore landed on that confirm page with no secret, which shows a spinner forever because it only processes when a secret is present. It now replaces itself. The secret stays out of the replaced entry: TanStack's hash history builds the href from window.location.search at navigation time (@tanstack/history createHashHistory), which the existing replaceState has already cleared.

🔧 Changes

Noticed and not changed here: the list's redirect sends every draft to new-cluster, including a draft saved from an edit (it carries clusterId), so such a draft reopens as a create form. ProcessSetupIntent already routes those to the edit page. Filed as #1852 and fixed in #1870, stacked on this PR.

The rebase onto #1780 was mechanical: both commits replayed without conflicts, and the branch no longer carries the merge commits it picked up from #1849. On 2026-10-13 it was rebased again with #1780 onto stage, and this PR's commits replayed unchanged. At 832b06f, npx tsc -b exit 0, and npx vitest run src/features/clusters src/features/organization/billing 221 passed.

✅ Verification

Route: live browser repro before and after (real TanStack hash router), plus unit tests pinning the navigation options. The Stripe return path cannot be driven locally (no Stripe key in the dev environment), so that half is verified at the mechanism level only.

Cross-model review: one round before the rebase, codex (graded, no findings) and gemini. Rejected on evidence: a crash from 'clusterId' in savedClusterState on a primitive (the code reads savedClusterState.clusterId, which is safe on a string, and that line is unchanged), and that mocked-router tests cannot show the history stack (the browser repro above uses the real router, and the createHref behaviour is cited from the installed source). The replay onto #1780 changed no line of this PR, so it had no new round.

gemini-code-assist feedback: both test-hygiene comments were taken in ec5865a, which resets the URL in ProcessSetupIntent.test.tsx's afterEach and drops an unused setupIntent fixture field.

Closes #1326

🤖 Generated by Anthropic Claude Code (Claude Opus 5.5); posted via @dawsontoth.

🤖 Generated with Claude Code

Related PRs: #1849 overlaps (this branch is stacked on it), #1842 overlaps (lower in the stack), #1780 overlaps (base of the stack; it edits ClustersList.tsx but not the redirect), #1870 overlaps (stacked on this branch; it changes where the same redirect goes)
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 @ ec5865a

Review-Attention: study ~9m (raised: degraded review, open major) @ ec5865a

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the navigation behavior when redirecting from a saved cluster draft or returning from Stripe billing setup to use history replacement (replace: true) instead of pushing to the history stack, preventing back-button redirect loops. Corresponding unit tests and design documentation have been updated. Feedback on the new test file suggests resetting the JSDOM window history in afterEach to prevent test pollution and removing an unused setupIntent property from the hoisted state.

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 79.48% 14769 / 18580
🔵 Statements 79.41% 15671 / 19733
🔵 Functions 74.83% 3937 / 5261
🔵 Branches 74.09% 11592 / 15645
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/clusters/ClustersList.tsx 96.87% 89.18% 93.75% 96.77% 144
src/features/organization/billing/confirm/ProcessSetupIntent.tsx 68.42% 68.42% 100% 68.42% 40, 51-52, 62-74
Generated in workflow #2353 for commit 832b06f by the Vitest Coverage Report Action

dawsontoth and others added 2 commits October 13, 2026 21:17
A saved cluster draft makes the clusters list redirect to the new-cluster
form, and that redirect pushed a history entry. Back from the form then
landed on the list, a page the user never saw, and while the form was still
loading (before it clears the draft) the list redirected straight back to the
form, so Back did nothing at all. Redirect in place instead.

Billing's Stripe return page had the same shape: it strips the setup-intent
secret from its own URL and then pushed the form, so Back from the form
landed on a confirm page with no secret, which spins forever. It now replaces
itself too.

Refs #1326

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rop an unused fixture

Refs #1326

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dawsontoth
dawsontoth force-pushed the claude/1326-new-cluster-back-button branch from ec5865a to 832b06f Compare October 14, 2026 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix back button issue on 'new cluster' page

1 participant