refactor(onboarding): retire /onboarding/* into /setup/* via config redirects (chat#1889 item 1, part 1) - #1887
Conversation
…quence The funnel-entry deep-link (/onboarding/first-task) rendered a standalone FirstTaskStep mount — a surface distinct from the unified OnboardingSequence that direct-chat signups see. Redirect it to the home gate, which renders OnboardingSequence at the account's derived step, so both entry paths converge on one onboarding experience and the funnel CTA always lands at the correct step rather than a hard-coded one. Deferred (noted in PR): fully retire the interim standalone /onboarding/* mounts and repoint external deep-links. Refs chat#1885. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLegacy onboarding entrypoints are adjusted toward the canonical setup flow. The catalog report CTA now links to ChangesOnboarding route migration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb7472c12c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| */ | ||
| const FirstTaskOnboardingPage = () => <FirstTaskStep />; | ||
| const FirstTaskOnboardingPage = () => { | ||
| redirect("/"); |
There was a problem hiding this comment.
For users coming from the catalog report CTA, CatalogReportCta still links to /onboarding/first-task, but this redirect sends them to /, where the task step is only the generic OnboardingStepCard linking to /tasks rather than FirstTaskStep with the pre-run report and Monday scheduling confirmation. In the funnel case where the derived onboarding step is task, this removes the promised one-click weekly-report setup path and drops users into the generic app instead of running/scheduling the first report.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Replaces a standalone onboarding step with a redirect to the shared home gate, converging funnel entry onto existing logic. The change is bounded, clearly beneficial, and the redirect preserves backward compatibility and is trivially verifiable.
Re-trigger cubic
chat#1887 previously redirected /onboarding/first-task to `/`. That route's only internal referrer is the catalog report's primary CTA, which is where both the marketing valuation funnel and the valuation email land — so the redirect sent converting users to the home surface instead of the step, where they hit the placeholder onboarding cards. Repoint at the canonical `/setup/*` sequence instead (chat#1889): - /onboarding/first-task -> /setup/tasks - /onboarding/roster -> /setup/artists - CatalogReportCta href -> /setup/tasks The `/onboarding/*` mounts stay as redirects rather than being deleted so any pasted or indexed link still resolves; deletion is a follow-up once logs show no traffic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sweetmantech
changed the title
Re-scoped 2026-07-26The prior diff redirected What changed
The VerificationTDD, red → green on a new regression guard:
Tracked in chat#1889 (matrix row 1). |
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Consolidates two onboarding surfaces into one by replacing standalone mounts with redirects to canonical setup routes, and updates the CTA link. Bounded refactor with no new behavior; backward-compatible via redirects.
Re-trigger cubic
Preview verification — 2026-07-27Preview: https://chat-hhy9sxnkd-recoup.vercel.app Results
The blocker note on chat#1889 is satisfied for rows 1–5: neither retired route lands on home, and neither renders the placeholder step cards. Both resolve to the canonical
Row 6 — why the CTA could not be verified live
I also scanned all 36 chunks loaded on the catalog report route for The href change is covered by the new unit test in this PR ( Finding — the redirects are client-side, not HTTPBoth retired routes return HTTP 200, not 307: Next prerendered both pages, so the Pre-existing issues observed (not caused by this PR, already tracked) |
…te the page stubs The interim /onboarding/first-task and /onboarding/roster mounts were scaffolding from chat#1880 so each step was user-testable before the sequence container existed. Replace the redirect() page components with 308s in next.config.mjs and delete the files. Two reasons over redirect() pages: - Next prerenders a page whose body is just redirect(), so the old URL answers HTTP 200 with the redirect in its RSC payload and only moves once React hydrates. A crawler, link previewer or curl sees a 200 and an empty shell. Config redirects are real 308s at the edge, checked before the filesystem. - No page files left behind carrying a 'delete once logs show no traffic' marker. chat#1889 exists because interim surfaces accumulated and were never retired; this does not add two more. After this PR nothing internal references /onboarding/* — the only referrer was CatalogReportCta, repointed here at /setup/tasks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reworked: config redirects instead of
|
| # | Check | Before (page stubs) | After (config redirects) | Result |
|---|---|---|---|---|
| 1 | GET /onboarding/first-task |
HTTP 200, x-nextjs-prerender: 1 |
HTTP 308, location: /setup/tasks |
✅ fixed |
| 2 | GET /onboarding/roster |
HTTP 200, x-nextjs-prerender: 1 |
HTTP 308, location: /setup/artists |
✅ fixed |
| 3 | Query passthrough | dropped (destination was hardcoded) | ?utm_source=email&x=1 → location: /setup/tasks?utm_source=email&x=1 |
✅ improved |
| 4 | Redirect chain | n/a | 1 hop, final 200 at /setup/tasks and /setup/artists |
✅ |
| 5 | Browser navigation | landed correctly | landed /setup/tasks?utm_source=valuation_email and /setup/artists |
✅ |
| 6 | Response body | 43,295-byte HTML shell | Redirecting... |
✅ |
| 7 | Destinations render directly | 200 | 200 | ✅ |
Row 3 is a real gain, not just cleanup: the old redirect("/setup/tasks") discarded query strings, so any utm_* on a funnel or email link was lost at the hop. Config redirects pass them through.
Local gates
- Full suite: 68 files / 257 tests pass, including 3 new TDD units in
__tests__/nextConfigRedirects.test.ts(confirmed RED —expected 'undefined' to be 'function'— before the config existed). tsc --noEmit: 8 errors, all pre-existing inSpotifyDeepResearchResult.tsxandextractSendEmailResults.test.ts, none in changed files.pnpm buildcan't run locally (no.env, only.env.example), so the Vercel preview build is the gate — it's green.
Note on the earlier approval
This PR is carrying an approval that predates both the re-scope and this rework. Worth another look before merge.
sweetmantech
changed the title
There was a problem hiding this comment.
0 issues found across 7 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Removes standalone onboarding pages and replaces with 308 redirects to canonical /setup/* routes; updates link in CatalogReportCta. Bounded cleanup that preserves behavior and simplifies codebase.
Re-trigger cubic
Full verification on the current build — 2026-07-27Supersedes the screenshots in my first comment, which were captured on Preview: https://chat-9o5pms78a-recoup.vercel.app Protocol level
Negative probes — the directory is genuinely gone
Rows 7–9 confirm the two rules match exactly what they should and nothing more, and that no Browser — logged out
Browser — authenticatedSigned in on this deployment via Privy email OTP, then re-ran both entries.
Neither entry reaches Still not verified
Local gates
Pre-existing, not caused by this PRVisible in rows 11 and 13: |
Onboarded-account regression pass — 2026-07-27Follow-up to the cold-start verification above. Question being answered: does a fully onboarded account get pulled into Preview: https://chat-9o5pms78a-recoup.vercel.app ( Why it can't happenThis PR's whole diff is 8 files: Results
Row 3–4: the redirect is auth-blind, and that's the correct tradeoffA If we ever want onboarded users routed differently, that belongs in PR B's home convergence (#1890), which is auth-aware via Row 5: pre-existing, and worse than it looks with a real roster
export default function SetupPage() {
redirect("/setup/artists"); // unchanged on main
}
This is exactly the item #1890 carries — "make Correction to the screenshots in my previous commentThe two images showing a Privy login popup are the logged-out captures ( |
1 participant
What
Today direct-chat signups render the unified
OnboardingSequence(viaHomePage→useOnboardingGate), while marketing-funnel signups deep-link to the standalone/onboarding/first-taskmount — a second, divergent onboarding surface.This ships the minimal user-testable first slice of convergence: the funnel entry now routes through the same surface.
Change
app/onboarding/first-task/page.tsx— replace the standalone<FirstTaskStep />mount withredirect("/").The home gate renders
OnboardingSequenceat the account's derived step (getOnboardingStep— first unmet checkpoint), so the funnel CTA always lands at the right step instead of a hard-codedfirst-task, and both entry paths converge on one experience.Why a redirect (not re-mounting the sequence here)
OnboardingSequenceneeds the full gate wiring (checkpoints, session skip flags, resume) thatHomePagealready owns. Redirecting to/reuses that single source of truth (DRY) rather than duplicating the gate on this route.Deferred follow-up (out of scope here)
/onboarding/*mounts (/onboarding/first-taskonce nothing links to it, plus/onboarding/roster)./.Kept as a redirect for now so any live funnel link keeps working through the cutover.
Verification
pnpm exec tsc --noEmit— clean for the changed page.pnpm exec eslint app/onboarding/first-task/page.tsx— clean./onboarding/first-tasknow server-redirects to/, where an incomplete account resumesOnboardingSequenceat its derived step (existingHomePagebehavior, covered bycomponents/Home/__tests__/HomePage.onboarding.test.tsx).Targets
main. Part of chat#1885 (converge the two onboarding surfaces).🤖 Generated with Claude Code
Summary by cubic
Converges funnel and email entry on the canonical setup flow.
/onboarding/first-tasknow 308-redirects to/setup/tasksand/onboarding/rosterto/setup/artists; updates the catalog report CTA to/setup/tasks. Aligns withchat#1889and progresseschat#1885.next.config.mjsand delete the/onboarding/*page stubs.CatalogReportCtalinking to/setup/tasks.Written for commit df30211. Summary will update on new commits.
Summary by CodeRabbit