Repository navigation
fix(auth): prevent setup module cycle on home - #3155
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Code-analysis diffPainscore total: 8348.63 → 8348.93 (+0.3) 🆕 New findings (8)
✅ Resolved (8)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No findings. The dependency-free screen ID list breaks the setup runtime cycle while preserving the fallback order, with parity guarded by a focused test.
Checked clean
- Confirmed the exact head and merge base, then traced the changed runtime import graph through Setup.consts, the setup Views barrel, and useSetupFlow.
- Verified SETUP_SCREEN_IDS matches setupSteps in membership and order and remains dependency-free at runtime through its type-only import.
- Checked fallback cursor, filtered-step, and no-back guard behavior around the changed initialization path; behavior is unchanged after steps populate.
- Reviewed exact-head CI: ci-success, unit, typecheck, eslint, format, native-export, and screen-tests passed.
- Confirmed the advisory ds-shots build failure also occurs at the supplied base SHA and is not introduced by this pull request.
- A local focused Jest rerun was unavailable because the detached worktree has no installed dependencies; the corresponding exact-head unit check passed in CI.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: 047b6e76a6c7 · Context: repo · Took 7m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No findings. The registry-derived screen order is injected through the provider, leaving the setup hook dependent only on context and types while preserving the pre-filter fallback behavior.
Checked clean
- Verified the detached HEAD, trusted author, dev base ref, supplied base SHA, and merge base exactly match the review request.
- Traced the setup registry, views, hook, and context imports and checked every SetupFlowProvider call site; the runtime dependency cycle is removed and the registry remains the single owner of screen order.
- Checked initial URL parsing, runtime step filtering, transition direction, no-back guards, and default-screen fallback against the setup-flow implementation and regression tests.
- Exact-head unit, typecheck, ESLint, format, screen-tests, and native-export checks succeeded. The advisory ds-shots build hit the same webpack WasmHash failure present on the supplied base SHA; Deploy Preview was still in progress when checked.
- Local focused Jest and ESLint commands were unavailable because their binaries are not installed in the detached worktree; the corresponding exact-head CI checks succeeded.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: f497fda77534 · Context: repo · Took 7m
This branch was successfully deployed
1 participant
Problem
Production setup recovery could fail while loading
/homewithReferenceError: Cannot access 'v' before initialization(SentryPEANUT-UI-T4Q, with related web reports inPEANUT-UI-T53). The failure could leave a logged-out user stuck in the home/setup recovery path.Root cause and history
The URL-stepper refactor introduced this runtime dependency cycle in commit
fcd12df/ PR #2949, and PR #3062 carried it intodev:The separate PR #3078 revert branch was closed without merging and is not an ancestor of
dev. The cycle therefore remained present continuously; this is an introduced regression, not a previously fixed bug that later returned.Fix
setupStepsas the single owner of setup screen order and derivesetupScreenIdsfrom it.SetupFlowProvider;useSetupFlownow depends only on the flow context and setup types, never the component-backed registry.useSetupFlowfrom importingSetup.constsor setup views, because the repository's generic cycle rule is intentionally disabled under the current resolver./home -> /setuppath. It captures uncaught page errors and console errors and rejects module-initialization failures.The resulting dependency direction is acyclic and enforced:
Validation
/home -> /setupinitialization regression passed; stale-session setup regression passed.ci-success, unit, typecheck, ESLint, format, native export, screen tests, and report.Advisory CI exception
ds-shotsremains red after one rerun because its Next.js build exits inside webpack after compilation, before any screenshot or regression assertion runs. The exactdevbase SHA (589d5f8) fails in the same job at the same webpack build point. This job is deliberately excluded fromci-success; the independent CI native-export build, the local optimized production build, and the Vercel preview all succeeded. No visual-diff result was produced.No backend, database, migration, localization, or user-visible UI change.
Sentry: