Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR migrates CLI authorization confirmation tracking from URL query parameters to sessionStorage. Internal helper functions are added to store, check, and clear a confirmation flag keyed by login code. The component's confirmation state computation and all auth completion flows are updated to use the new sessionStorage-based mechanism instead of URL-based tracking. The test is updated to assert against sessionStorage instead of URL parameters. ChangesCLI Auth Confirmation Storage Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR replaces the URL query-parameter approach (
Confidence Score: 4/5Safe to merge with a minor fix: the The production logic is well-structured — switching to a
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CliAuthConfirmation
participant sessionStorage
participant StackApp
User->>CliAuthConfirmation: click Authorize
CliAuthConfirmation->>StackApp: postCliAuthComplete (mode: check)
alt anonymous CLI session
StackApp-->>CliAuthConfirmation: cli_session_state = anonymous
CliAuthConfirmation->>StackApp: postCliAuthComplete (mode: claim-anon-session)
StackApp-->>CliAuthConfirmation: access_token + refresh_token
CliAuthConfirmation->>StackApp: signInWithTokens(...)
CliAuthConfirmation->>sessionStorage: setItem(stack-cli-auth-confirmed, loginCode)
CliAuthConfirmation->>StackApp: redirectToSignUp()
Note over CliAuthConfirmation: Page reloads / navigates
CliAuthConfirmation->>sessionStorage: getItem(stack-cli-auth-confirmed) == loginCode?
sessionStorage-->>CliAuthConfirmation: true → confirmed = true
CliAuthConfirmation->>StackApp: completeWithCurrentUser()
CliAuthConfirmation->>sessionStorage: removeItem(stack-cli-auth-confirmed)
CliAuthConfirmation-->>User: status = success
else user already signed in
StackApp-->>CliAuthConfirmation: user present
CliAuthConfirmation->>StackApp: completeWithCurrentUser()
CliAuthConfirmation-->>User: status = success
else no user, not anonymous
CliAuthConfirmation->>sessionStorage: setItem(stack-cli-auth-confirmed, loginCode)
CliAuthConfirmation->>StackApp: redirectToSignIn()
end
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/template/src/components-page/cli-auth-confirm.test.tsx (1)
93-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear
sessionStorageinafterEachto prevent test cross-contamination.The migration to sessionStorage means the
"stack-cli-auth-confirmed"key now persists between tests in the same jsdom environment. The "claims anonymous CLI sessions…" test at line 174 writes"login-code", but theafterEachonly resetswindow.history, mocks, and the act environment — not storage.If a future test (or a reorder) runs after that case with
login_code=login-code, the initialconfirmedstate inuseCliAuthConfirmationwill betrue, the auto-completeuseEffectwill fire on render, and assertions likeexpect(sendRequest).toHaveBeenCalledOnce()in the first two tests would silently break.🧪 Suggested cleanup
afterEach(() => { act(() => { root?.unmount(); }); container?.remove(); root = null; container = null; vi.restoreAllMocks(); window.history.replaceState({}, "", "/"); + sessionStorage.clear(); Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", previousActEnvironment); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/template/src/components-page/cli-auth-confirm.test.tsx` around lines 93 - 103, The tests leave the sessionStorage key "stack-cli-auth-confirmed" set between cases which causes useCliAuthConfirmation's initial confirmed state and subsequent effects to leak across tests; update the afterEach block (the teardown where afterEach unmounts root, removes container, restores mocks and resets history/act environment) to also clear sessionStorage (or explicitly removeItem("stack-cli-auth-confirmed")) so each test starts with a clean storage state and the auto-complete useEffect in useCliAuthConfirmation cannot fire from prior tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/template/src/components-page/cli-auth-confirm.test.tsx`:
- Around line 93-103: The tests leave the sessionStorage key
"stack-cli-auth-confirmed" set between cases which causes
useCliAuthConfirmation's initial confirmed state and subsequent effects to leak
across tests; update the afterEach block (the teardown where afterEach unmounts
root, removes container, restores mocks and resets history/act environment) to
also clear sessionStorage (or explicitly removeItem("stack-cli-auth-confirmed"))
so each test starts with a clean storage state and the auto-complete useEffect
in useCliAuthConfirmation cannot fire from prior tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac827a5d-fba0-433a-9734-35ed8a2123c0
📒 Files selected for processing (2)
packages/template/src/components-page/cli-auth-confirm.test.tsxpackages/template/src/components-page/cli-auth-confirm.tsx
2 participants
Summary by CodeRabbit