feat: add anonRefreshToken to CLI auth flow and enhance session management#1303
Merged
mantrakp04 merged 13 commits intodevfrom Apr 14, 2026
Merged
feat: add anonRefreshToken to CLI auth flow and enhance session management#1303mantrakp04 merged 13 commits intodevfrom
mantrakp04 merged 13 commits intodevfrom
Conversation
…ement - Updated the CliAuthAttempt model to include anonRefreshToken. - Added migration to support the new anonRefreshToken field. - Enhanced CLI authentication routes to handle anonymous user sessions, including validation and merging of anonymous user data into authenticated sessions. - Updated tests to cover new functionality for anonymous sessions and refresh token handling.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds nullable Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (stack)
participant SDK as StackClientApp
participant Auth as /auth/cli
participant DB as Database
participant Poll as /auth/cli/poll
participant Complete as /auth/cli/complete
CLI->>SDK: promptCliLogin({ anonRefreshToken?, promptLink })
SDK->>Auth: POST /auth/cli { anon_refresh_token? }
Auth->>DB: validate anon_refresh_token via $queryRaw
DB-->>Auth: anon token -> anonymous user (or not)
Auth->>DB: INSERT CliAuthAttempt (anonRefreshToken) RETURNING codes
Auth-->>SDK: polling_code, login_code, expires_at
SDK->>CLI: invoke promptLink(url, loginCode)
CLI->>Poll: poll /auth/cli/poll (polling_code)
Poll->>DB: SELECT CliAuthAttempt status
DB-->>Poll: waiting / success / used / expired
Poll-->>CLI: status
Browser->>Complete: POST /auth/cli/complete mode:"check" (login_code)
Complete->>DB: SELECT CliAuthAttempt
DB-->>Complete: anonRefreshToken present -> cli_session_state:"anonymous"
Complete-->>Browser: { cli_session_state:"anonymous" }
Browser->>Complete: POST mode:"claim-anon-session" (login_code)
Complete->>DB: derive tokens from anonRefreshToken
DB-->>Complete: access_token, refresh_token
Complete-->>Browser: tokens
Browser->>Complete: POST mode:"complete" (login_code, refresh_token)
Complete->>DB: UPDATE ... WHERE refreshToken IS NULL AND usedAt IS NULL AND expiresAt > NOW() RETURNING *
DB-->>Complete: success / failure
Complete-->>Browser: { success: true } or error
sequenceDiagram
participant CLI as Stack CLI
participant Config as Config Module
participant SDK as StackClientApp
participant Auth as /auth/cli
participant Complete as /auth/cli/complete
CLI->>Config: readConfigValue(STACK_CLI_ANON_REFRESH_TOKEN)
Config-->>CLI: anon token?
CLI->>SDK: promptCliLogin({ anonRefreshToken, promptLink })
SDK->>Auth: POST /auth/cli with anon_refresh_token
Auth-->>SDK: polling_code, login_code
SDK->>CLI: promptLink(url, loginCode) or log code
CLI->>SDK: poll until success
SDK->>Complete: receives refresh_token
SDK-->>CLI: return refresh_token
CLI->>Config: writeConfigValue(STACK_CLI_REFRESH_TOKEN)
CLI->>Config: removeConfigValue(STACK_CLI_ANON_REFRESH_TOKEN) (if provided)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the CLI authentication flow to support carrying an anonymous CLI session through browser-based authentication, including optionally claiming/merging anonymous user state into an authenticated session.
Changes:
- Add
anonRefreshTokensupport to CLI auth attempts (schema + migration) and to the JS SDKpromptCliLoginflow. - Enhance browser confirmation handling to support anonymous-session claiming and post-login auto-completion.
- Add extensive e2e coverage for CLI auth initiation/poll/complete flows (including anonymous session scenarios) and add demo utilities.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sdks/spec/src/apps/client-app.spec.md | Updates CLI login spec (new confirm URL, callback signature, anon refresh token support). |
| packages/template/src/lib/stack-app/apps/interfaces/client-app.ts | Extends promptCliLogin options to include anonRefreshToken + updated promptLink signature. |
| packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts | Sends anon_refresh_token during CLI auth initiation and surfaces login code to prompt callback/default output. |
| packages/template/src/components-page/cli-auth-confirm.tsx | Enhances confirm page to support anonymous session claiming + auto-complete after sign-in/sign-up. |
| packages/stack-cli/src/lib/config.ts | Adds STACK_CLI_ANON_REFRESH_TOKEN to CLI config key set. |
| packages/stack-cli/src/commands/login.ts | Passes anon refresh token into promptCliLogin and clears it after successful login. |
| examples/demo/src/app/cli-auth-demo/page.tsx | Adds a UI demo page to exercise multiple CLI/browser auth scenarios. |
| examples/demo/cli-sim.mjs | Adds a minimal CLI simulator script for local demo/testing. |
| apps/e2e/tests/js/restricted-users.test.ts | Minor formatting-only change (blank lines). |
| apps/e2e/tests/backend/endpoints/api/v1/auth/cli/route.test.ts | Adds tests for anon_refresh_token validation and client access type initiation. |
| apps/e2e/tests/backend/endpoints/api/v1/auth/cli/poll.test.ts | Adds client access type coverage for polling and full client-cycle test. |
| apps/e2e/tests/backend/endpoints/api/v1/auth/cli/complete.test.ts | Adds extensive coverage for check/claim/complete, anonymous claiming, merges, and edge cases. |
| apps/backend/src/lib/user-merge.tsx | Introduces transactional merge logic from anonymous user → authenticated user, then deletes anonymous user. |
| apps/backend/src/app/api/latest/auth/cli/route.tsx | Accepts/validates anon_refresh_token and persists it on the CLI auth attempt. |
| apps/backend/src/app/api/latest/auth/cli/complete/route.tsx | Adds mode (check/claim-anon-session/complete) and merge-on-complete behavior. |
| apps/backend/prisma/schema.prisma | Adds anonRefreshToken column to CliAuthAttempt. |
| apps/backend/prisma/migrations/20260331000000_add_anon_refresh_token_to_cli_auth/migration.sql | Migration to add anonRefreshToken column. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR extends the CLI auth flow with support for associating an anonymous session at login-initiation time, and adds a three-mode Confidence Score: 5/5Safe to merge; remaining findings are P2 style and documentation issues that do not affect correctness. The core backend logic (atomic SQL updates with optimistic-locking WHERE clauses, one-shot anonRefreshToken consumption, tenancy validation) is sound. The migration is a safe nullable-column addition. The two open findings are a P2 rule-style violation in the confirm page and a misleading flow description in the dev-harness page — neither affects runtime behavior. packages/template/src/components-page/cli-auth-confirm.tsx (style rule), examples/demo/src/app/cli-auth-demo/page.tsx (incorrect flow description for case 5) Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as Stack CLI
participant API as Backend API
participant Browser as Browser / Template
participant Poll as Poll Endpoint
CLI->>API: POST /auth/cli {anon_refresh_token?}
API-->>CLI: {polling_code, login_code, expires_at}
CLI->>Browser: Open URL /handler/cli-auth-confirm?login_code=...
alt Browser NOT signed in
Browser->>API: POST /auth/cli/complete mode=check
API-->>Browser: {cli_session_state: anonymous|none}
alt cli_session_state == anonymous
Browser->>API: POST /auth/cli/complete mode=claim-anon-session
API-->>Browser: {access_token, refresh_token} for anon user
Note over API: anonRefreshToken set to NULL atomically
Browser->>Browser: signInWithTokens(anonTokens)
Browser->>Browser: redirectToSignUp
Note over Browser: User signs up, real user created
else cli_session_state == none
Browser->>Browser: redirectToSignIn
Note over Browser: User signs in, real user session
end
end
Note over Browser: confirmed=true in URL, user now signed in
Browser->>API: POST /auth/cli/complete mode=complete, refresh_token
Note over API: refreshToken stored, anonRefreshToken cleared
API-->>Browser: {success: true}
loop Poll until success
CLI->>Poll: POST /auth/cli/poll {polling_code}
Poll-->>CLI: {status: waiting|success, refresh_token?}
end
CLI->>CLI: Save refresh_token, remove STACK_CLI_ANON_REFRESH_TOKEN
Prompt To Fix All With AIThis is a comment left during a code review.
Path: examples/demo/src/app/cli-auth-demo/page.tsx
Line: 117-122
Comment:
**Test case 5 flow description contradicts the PR's stated design**
The `flow` string says `"Confirm → complete directly → merge anon into real user → CLI gets token"`, but the PR description explicitly states that anonymous user merging was **removed as a security risk**. When `handleAuthorize` runs with a real browser user, it calls `completeCliAuthWithRefreshToken` directly — the CLI's anonymous user is silently orphaned, never merged. The description will mislead anyone running this case and expecting to verify a merge.
```suggestion
flow: 'Confirm \u2192 complete directly \u2192 CLI gets token (anon user orphaned, no merge)',
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/template/src/components-page/cli-auth-confirm.tsx
Line: 78-133
Comment:
**`handleAuthorize` uses bare try/catch instead of `runAsynchronouslyWithAlert`**
The `handleAuthorize` async click handler uses a manual try/catch block, while the sibling auto-complete `useEffect` already wraps its async work with `runAsynchronouslyWithAlert`. Per the project rule, async button click handlers should use `runAsynchronouslyWithAlert` instead of bare try/catch. Consider wrapping the body of `handleAuthorize` with `runAsynchronouslyWithAlert(async () => { ... })`, keeping the inner try/catch for the inline error-state update — matching the pattern already used in the `useEffect` above.
**Rule Used:** Use `runAsynchronouslyWithAlert` from `@stackframe... ([source](https://app.greptile.com/review/custom-context?memory=5e671275-7493-402a-93a8-969537ec4d63))
**Learnt From**
[stack-auth/stack-auth#943](https://github.com/stack-auth/stack-auth/pull/943)
How can I resolve this? If you propose a fix, please make it concise.Reviews (4): Last reviewed commit: "Merge branch 'dev' into anon-cli-auth" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
packages/stack-cli/src/commands/login.ts (1)
30-32: Consider exposing the login code to the user.The
promptLinkcallback receives bothurlandloginCode, but only the URL is displayed. Showing the login code could help users verify they're completing authentication for the correct session, improving security UX.🔧 Optionally display the login code
- promptLink: (url) => { - console.log(`\nPlease visit the following URL to authenticate:\n${url}`); + promptLink: (url, loginCode) => { + console.log(`\nPlease visit the following URL to authenticate:\n${url}`); + console.log(`Login code: ${loginCode}`); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/login.ts` around lines 30 - 32, The promptLink callback currently only prints the URL and ignores the provided loginCode; update the promptLink implementation (in login.ts) to include the loginCode alongside the URL in the console output so users can verify the session (e.g., print both url and loginCode in a clear message), and guard against missing loginCode by conditionally including it when present to avoid undefined being shown.packages/template/src/components-page/cli-auth-confirm.tsx (1)
10-11: Avoid erasing types in this auth handoff.
app as anyanderr as Errorremove the only contracts around the symbol internals and thrown errors, so a shape drift here becomes a runtime auth failure. GivegetStackAppInternals()a concrete return type and normalizeunknownbefore callingsetError.As per coding guidelines "Try to avoid the
anytype. Whenever you need to useany, leave a comment explaining why you're using it and how you can be certain that errors would still be flagged." and "Do NOT useas/any/type casts or anything else to bypass the type system unless you specifically asked the user about it."Also applies to: 68-69, 123-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/components-page/cli-auth-confirm.tsx` around lines 10 - 11, Replace the unchecked casts by defining a concrete interface for the internals (e.g., StackAppInternals) and a type guard isStackAppInternals(obj: unknown): obj is StackAppInternals, then change getStackAppInternals(app: unknown) to return StackAppInternals | undefined and use the guard to safely access (stackAppInternalsSymbol) instead of app as any; similarly, add a normalizeError(err: unknown): Error helper or an isError type guard and call it before setError (replace err as Error) so thrown values are validated/normalized; update the call sites referenced (the getStackAppInternals usage and the places that call setError) to use these guards/helpers to avoid any/ casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/auth/cli/route.tsx`:
- Around line 54-65: The error message is misleading when the DB returns null;
update the logic after globalPrismaClient.projectUser.findUnique so it first
checks if user is null and throws a clear StatusError (e.g., "User not found for
provided refresh token" or similar) and only then checks user.isAnonymous and
throws the existing "does not belong to an anonymous user" error; reference the
variables/globalPrismaClient.projectUser.findUnique, user, refreshTokenObj, and
the StatusError throw site to locate where to add the distinct null-check.
In `@examples/demo/src/app/cli-auth-demo/page.tsx`:
- Around line 237-243: doBrowserSignOut currently calls browserUser.signOut()
which triggers the app's after-sign-out redirect and navigates away before the
CLI flow setup; change the call in doBrowserSignOut to prevent that redirect by
using browserUser.signOut with options to either disable automatic redirect
(e.g., redirect: false) or explicitly set the post-sign-out URL to the current
demo path (e.g., callbackUrl or returnTo set to window.location.pathname or
'/cli-auth-demo'), and if you disable redirect, perform any required local state
cleanup and keep the user on the same pathname so the CLI setup can proceed.
- Around line 170-183: The hydration and signup flows drop anonymous sessions
because cliApp.getUser({ includeRestricted: true }) can return null for
anonymous users; update both the useEffect hydration (where loadCliState and
refreshCliAppSession are used) and doCliAnonSignUp to detect when refreshed
response contains partial/anon user info or anonRefreshToken even if getUser
returns null, and populate setCliState from the refreshed payload (e.g.,
refreshed.user / refreshed.partialUser or refreshed.anonRefreshToken + parsed
userId/isAnonymous) instead of relying solely on getUser; reuse the same
partial-user fallback logic in both places so anonymous sessions are persisted
and anonRefreshToken is passed through.
- Around line 342-344: The confirmUrl computation reads window.location.origin
during render which breaks SSR; change it so window is only accessed on the
client (e.g., compute confirmUrl inside a client-only hook/effect or guard with
typeof window !== 'undefined' before using window). Update the code that defines
confirmUrl (the variable that currently depends on loginCode) to fall back to
null during SSR and set the full URL on the client after mount (or use a
useMemo/useState + effect) so rendering/hydration no longer attempts to read
window at build time.
In `@packages/template/src/components-page/cli-auth-confirm.tsx`:
- Around line 100-122: The code treats non-OK responses from postCliAuthComplete
as a benign “not anonymous” path; change it to stop and handle errors instead:
after calling postCliAuthComplete in the "check" flow (variable checkResult) if
!checkResult.ok, abort the flow (e.g., surface an error / redirect to sign-in
with an error) rather than proceeding; likewise for the "claim-anon-session"
call (variable claimResult) if !claimResult.ok, abort immediately instead of
treating it as a fallthrough to sign-in. Locate postCliAuthComplete usage and
update the branches around checkResult and claimResult to explicitly handle
non-OK responses before attempting JSON parsing, calling
getStackAppInternals(...).signInWithTokens, or redirecting to
app.redirectToSignUp/app.redirectToSignIn.
In `@packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts`:
- Around line 3006-3008: The payload builder currently uses a truthiness check
for options.anonRefreshToken which omits empty-string values; update the
conditional in the JSON body construction (where expires_in_millis and
anon_refresh_token are set) to include anon_refresh_token when
options.anonRefreshToken is not null or undefined (explicit !== null && !==
undefined) so empty strings are preserved and sent to the backend; adjust the
spread expression that currently reads ...(options.anonRefreshToken ? {
anon_refresh_token: options.anonRefreshToken } : {}) to use this explicit
null/undefined check.
In `@sdks/spec/src/apps/client-app.spec.md`:
- Around line 841-853: The documented poll state is out of sync with the
implementation/tests: update the CLI flow docs (the section describing POST
/api/v1/auth/cli, the polling contract and any examples) to state that poll
responses return { status: "waiting" } instead of { status: "pending" }, and
ensure any references to options.promptLink or options.anonRefreshToken and the
returned refresh token example reflect this "waiting" status so the spec matches
the client implementation and tests.
---
Nitpick comments:
In `@packages/stack-cli/src/commands/login.ts`:
- Around line 30-32: The promptLink callback currently only prints the URL and
ignores the provided loginCode; update the promptLink implementation (in
login.ts) to include the loginCode alongside the URL in the console output so
users can verify the session (e.g., print both url and loginCode in a clear
message), and guard against missing loginCode by conditionally including it when
present to avoid undefined being shown.
In `@packages/template/src/components-page/cli-auth-confirm.tsx`:
- Around line 10-11: Replace the unchecked casts by defining a concrete
interface for the internals (e.g., StackAppInternals) and a type guard
isStackAppInternals(obj: unknown): obj is StackAppInternals, then change
getStackAppInternals(app: unknown) to return StackAppInternals | undefined and
use the guard to safely access (stackAppInternalsSymbol) instead of app as any;
similarly, add a normalizeError(err: unknown): Error helper or an isError type
guard and call it before setError (replace err as Error) so thrown values are
validated/normalized; update the call sites referenced (the getStackAppInternals
usage and the places that call setError) to use these guards/helpers to avoid
any/ casts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91e27ad8-a0ef-4bb4-8193-010c8d589930
📒 Files selected for processing (17)
apps/backend/prisma/migrations/20260331000000_add_anon_refresh_token_to_cli_auth/migration.sqlapps/backend/prisma/schema.prismaapps/backend/src/app/api/latest/auth/cli/complete/route.tsxapps/backend/src/app/api/latest/auth/cli/route.tsxapps/backend/src/lib/user-merge.tsxapps/e2e/tests/backend/endpoints/api/v1/auth/cli/complete.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/cli/poll.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/cli/route.test.tsapps/e2e/tests/js/restricted-users.test.tsexamples/demo/cli-sim.mjsexamples/demo/src/app/cli-auth-demo/page.tsxpackages/stack-cli/src/commands/login.tspackages/stack-cli/src/lib/config.tspackages/template/src/components-page/cli-auth-confirm.tsxpackages/template/src/lib/stack-app/apps/implementations/client-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/client-app.tssdks/spec/src/apps/client-app.spec.md
- Updated user validation logic to throw an error if the user is not found for the provided refresh token. - Enhanced error handling for anonymous user merges to skip the merge if the user has been upgraded concurrently. - Adjusted CLI auth demo page to correctly handle anonymous user state and refresh tokens. - Updated tests to reflect changes in user validation and session management.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
examples/demo/src/app/cli-auth-demo/page.tsx (1)
141-146:⚠️ Potential issue | 🟠 MajorThe browser reset helper still can't produce a clean signed-out setup.
useUser({ includeRestricted: true })hides anonymous sessions, sodoBrowserSignOut()is a no-op for anonymous browsers. When it does find a real user,browserUser.signOut({ redirectUrl: '/cli-auth-demo' })still performs a full navigation, which aborts the rest ofrunTestCase(). The sign-out cases therefore either keep stale anon state or reload before CLI setup continues.Also applies to: 243-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/demo/src/app/cli-auth-demo/page.tsx` around lines 141 - 146, The browser reset currently hides anonymous sessions and triggers a full page navigation on sign-out; change useUser({ includeRestricted: true }) to include anonymous sessions (remove the option or set includeRestricted: false) so doBrowserSignOut can operate on anon sessions, and update doBrowserSignOut/browserUser.signOut calls to perform a silent sign-out (e.g., call browserUser.signOut with options that disable redirect/navigation such as redirect: false or no redirectUrl, or use browserApp.auth.signOut() if available) so runTestCase can continue without the page reloading.
🧹 Nitpick comments (1)
apps/backend/src/lib/user-merge.tsx (1)
159-187: Consider batching permission operations for scalability.The current approach queries
authenticatedPermissionIdsand processes permissions sequentially per team. While acceptable for typical CLI auth scenarios with few team memberships, this could become slow if users have many teams with many permissions.For future-proofing, consider batching the delete and update operations. However, given the CLI auth context where large permission sets are unlikely, this is not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/user-merge.tsx` around lines 159 - 187, Refactor the per-permission sequential delete/update loop to batch operations: collect ids to delete and ids to reassign (from membership.teamMemberDirectPermissions) into arrays, then call tx.teamMemberDirectPermission.deleteMany({ where: { id: { in: deleteIds } } }) and tx.teamMemberDirectPermission.updateMany({ where: { id: { in: updateIds } }, data: { projectUserId: options.authenticatedUserId } }) (or multiple updateMany calls if scopes differ); keep the existing authenticatedPermissionIds lookup but replace the await-per-permission delete/update in the loop with population of deleteIds/updateIds and perform the bulk operations after the loop to improve scalability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/auth/cli/route.tsx`:
- Line 20: The code currently skips validation for empty anon_refresh_token due
to a truthiness check; change the conditional that reads something like if
(anon_refresh_token) to an explicit null/undefined check like if
(anon_refresh_token != null) so empty strings are passed into the validation
path and will produce an explicit "Invalid anon refresh token" error; update any
related logic (e.g., the validateAnonRefreshToken/anon_refresh_token handling in
route handler) to ensure empty string inputs are treated as invalid and return
the appropriate error response.
In `@examples/demo/src/app/cli-auth-demo/page.tsx`:
- Around line 44-49: The JWT decoding in parseAccessTokenUserSnapshot is using
atob on a base64url payload and inferring anonymity from iss; change it to first
convert the base64url payload to valid base64 (replace '-'->'+' and '_'->'/' and
add '=' padding) before decoding the JSON, and read the actual is_anonymous
boolean claim (e.g., payload.is_anonymous) to set isAnonymous; ensure you still
fall back to payload.userId or payload.sub for userId and handle malformed
tokens safely.
In `@packages/template/src/components-page/cli-auth-confirm.tsx`:
- Line 36: The useUser hook call currently omits anonymous sessions which causes
user to be null for anonymous browsers; update the app.useUser invocation in
cli-auth-confirm.tsx (the const user = app.useUser(...) line) to include or:
"anonymous" in addition to includeRestricted: true so it becomes useUser({
includeRestricted: true, or: "anonymous" }); this will allow the autocomplete
block (the !user gating around lines ~51-67) and handleAuthorize flow to detect
anonymous browser sessions correctly instead of falling through to
redirectToSignIn().
---
Duplicate comments:
In `@examples/demo/src/app/cli-auth-demo/page.tsx`:
- Around line 141-146: The browser reset currently hides anonymous sessions and
triggers a full page navigation on sign-out; change useUser({ includeRestricted:
true }) to include anonymous sessions (remove the option or set
includeRestricted: false) so doBrowserSignOut can operate on anon sessions, and
update doBrowserSignOut/browserUser.signOut calls to perform a silent sign-out
(e.g., call browserUser.signOut with options that disable redirect/navigation
such as redirect: false or no redirectUrl, or use browserApp.auth.signOut() if
available) so runTestCase can continue without the page reloading.
---
Nitpick comments:
In `@apps/backend/src/lib/user-merge.tsx`:
- Around line 159-187: Refactor the per-permission sequential delete/update loop
to batch operations: collect ids to delete and ids to reassign (from
membership.teamMemberDirectPermissions) into arrays, then call
tx.teamMemberDirectPermission.deleteMany({ where: { id: { in: deleteIds } } })
and tx.teamMemberDirectPermission.updateMany({ where: { id: { in: updateIds } },
data: { projectUserId: options.authenticatedUserId } }) (or multiple updateMany
calls if scopes differ); keep the existing authenticatedPermissionIds lookup but
replace the await-per-permission delete/update in the loop with population of
deleteIds/updateIds and perform the bulk operations after the loop to improve
scalability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4d9ca78-1220-4596-9bb5-79936b61b109
📒 Files selected for processing (7)
apps/backend/src/app/api/latest/auth/cli/route.tsxapps/backend/src/lib/user-merge.tsxexamples/demo/src/app/cli-auth-demo/page.tsxpackages/stack-cli/src/commands/login.tspackages/template/src/components-page/cli-auth-confirm.tsxpackages/template/src/lib/stack-app/apps/implementations/client-app-impl.tssdks/spec/src/apps/client-app.spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stack-cli/src/commands/login.ts
N2D4
requested changes
Apr 8, 2026
…nctionality - Replaced direct Prisma queries with raw SQL for fetching anonymous refresh tokens and CLI auth attempts. - Removed the user merge functionality to enhance security, ensuring anonymous users remain untouched during authentication. - Updated tests to reflect changes in the handling of anonymous user sessions and authentication flow. - Improved error handling for invalid login codes and refresh tokens.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/backend/src/app/api/latest/auth/cli/route.tsx (1)
43-43:⚠️ Potential issue | 🟡 MinorTreat empty
anon_refresh_tokenvalues as invalid, not absent.
if (anon_refresh_token)skips validation for"", so this endpoint silently creates a regular CLI auth attempt instead of returning the invalid anon-token error. Please add a tiny regression case foranon_refresh_token: ""when you flip this guard.Suggested fix
- if (anon_refresh_token) { + if (anon_refresh_token != null) {As per coding guidelines "Prefer explicit null/undefinedness checks over boolean checks (e.g.,
foo == nullinstead of!foo)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/auth/cli/route.tsx` at line 43, The guard using if (anon_refresh_token) incorrectly treats empty string as absent; change the conditional to an explicit null/undefined check (e.g., if (anon_refresh_token == null) or if (anon_refresh_token !== undefined && anon_refresh_token !== null) as appropriate) so empty string falls through to validation, then add an explicit validation that returns the invalid anon-token error when anon_refresh_token === "" (update the handler that reads anon_refresh_token in route.tsx); also add a regression test case sending anon_refresh_token: "" to assert the endpoint returns the invalid anon-token error.
🧹 Nitpick comments (2)
apps/e2e/tests/backend/endpoints/api/v1/auth/cli/complete.test.ts (2)
167-175: Encode the new dynamic URL parts.These requests interpolate
teamIdanduserIddirectly into paths/query strings. The values are UUID-like today, but the repo rule here is to go throughurlStringorencodeURIComponent()so tests exercise the same safe URL construction path as production code.As per coding guidelines "Use
urlString``/encodeURIComponent()` instead of normal string interpolation for URLs".Also applies to: 400-418, 438-447, 565-565
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/auth/cli/complete.test.ts` around lines 167 - 175, The test constructs URLs by interpolating dynamic values directly into strings (see calls to niceBackendFetch that build `/api/v1/users?team_id=${teamId}` and `/api/v1/users/${cliAnonymousUser.userId}` where variables like teamId and cliAnonymousUser.userId are used); update these calls to pass dynamic parts through the project's safe URL helper (use urlString or wrap with encodeURIComponent()) so the query/path parameters are encoded the same way as production code—modify the invocations that produce teamUsersResponse and anonymousUserResponse (and the other occurrences noted) to encode teamId and userId before embedding them in the URL passed to niceBackendFetch.
290-396: Prefer inline snapshots for these new 400-path cases.The new negative tests only pin
statusand a substring, so body-shape regressions can slip through unnoticed. Inline snapshots would keep these aligned with the rest of this file and lock the contract more tightly.As per coding guidelines "Prefer .toMatchInlineSnapshot over other selectors in tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/auth/cli/complete.test.ts` around lines 290 - 396, Replace the loose substring assertions with inline snapshots so the 400-path response bodies are pinned: for the tests that createResponse/claimResponse/completeResponse/checkResponse (the variables named createResponse, claimResponse, completeResponse, checkResponse in this test file) change the expect(...status).toBe(400) + expect(...body).toContain(...) checks to assert the full response body using expect(response.body).toMatchInlineSnapshot(...) (one snapshot per negative case) so the exact error payload/shape is locked; keep the status assertions and generate appropriate inline snapshot strings that reflect the current error responses for each of the four failing cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/auth/cli/complete/route.tsx`:
- Around line 50-52: The CLI auth expiry check uses new Date() while the claim
uses the DB NOW(), causing clock-drift failures; update the code paths (e.g., in
complete route and in getPendingCliAuthAttempt() / the atomic claim logic
referenced around lines 197-208) to use a single clock source by obtaining the
DB time (SELECT NOW() or the DB client's current timestamp) once and comparing
cliAuth.expiresAt against that DB time (or pass the DB time through
getPendingCliAuthAttempt()), then use that same DB-derived timestamp when
deciding to throw the StatusError in the completion handler so both check and
claim use the same clock.
- Around line 183-185: Replace the boolean falsy check on refresh_token with an
explicit null/undefined check: instead of throwing StatusError when
(!refresh_token), check (refresh_token == null) so empty string values are
treated as provided and will progress to the subsequent validation that throws
"Invalid refresh token"; update the conditional that currently throws new
StatusError(400, "refresh_token is required when mode is 'complete'") to use
refresh_token == null to locate the change near the refresh_token usage in
route.tsx.
In `@apps/backend/src/app/api/latest/auth/cli/route.tsx`:
- Around line 87-90: The SQL in route.tsx uses gen_random_uuid() in the INSERT
into "CliAuthAttempt" but pgcrypto is never enabled in migrations; either add a
migration that runs CREATE EXTENSION IF NOT EXISTS pgcrypto; early in the schema
migrations, or stop using raw gen_random_uuid() and instead rely on Prisma's
UUID default (e.g., using `@default`(uuid()) on the CliAuthAttempt.id field) and
remove the raw UUID call in the route; update route.tsx to match whichever
approach you choose and ensure migrations reflect the change.
---
Duplicate comments:
In `@apps/backend/src/app/api/latest/auth/cli/route.tsx`:
- Line 43: The guard using if (anon_refresh_token) incorrectly treats empty
string as absent; change the conditional to an explicit null/undefined check
(e.g., if (anon_refresh_token == null) or if (anon_refresh_token !== undefined
&& anon_refresh_token !== null) as appropriate) so empty string falls through to
validation, then add an explicit validation that returns the invalid anon-token
error when anon_refresh_token === "" (update the handler that reads
anon_refresh_token in route.tsx); also add a regression test case sending
anon_refresh_token: "" to assert the endpoint returns the invalid anon-token
error.
---
Nitpick comments:
In `@apps/e2e/tests/backend/endpoints/api/v1/auth/cli/complete.test.ts`:
- Around line 167-175: The test constructs URLs by interpolating dynamic values
directly into strings (see calls to niceBackendFetch that build
`/api/v1/users?team_id=${teamId}` and `/api/v1/users/${cliAnonymousUser.userId}`
where variables like teamId and cliAnonymousUser.userId are used); update these
calls to pass dynamic parts through the project's safe URL helper (use urlString
or wrap with encodeURIComponent()) so the query/path parameters are encoded the
same way as production code—modify the invocations that produce
teamUsersResponse and anonymousUserResponse (and the other occurrences noted) to
encode teamId and userId before embedding them in the URL passed to
niceBackendFetch.
- Around line 290-396: Replace the loose substring assertions with inline
snapshots so the 400-path response bodies are pinned: for the tests that
createResponse/claimResponse/completeResponse/checkResponse (the variables named
createResponse, claimResponse, completeResponse, checkResponse in this test
file) change the expect(...status).toBe(400) + expect(...body).toContain(...)
checks to assert the full response body using
expect(response.body).toMatchInlineSnapshot(...) (one snapshot per negative
case) so the exact error payload/shape is locked; keep the status assertions and
generate appropriate inline snapshot strings that reflect the current error
responses for each of the four failing cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 234bc383-2fc0-4489-a24f-afcf10c983cb
📒 Files selected for processing (3)
apps/backend/src/app/api/latest/auth/cli/complete/route.tsxapps/backend/src/app/api/latest/auth/cli/route.tsxapps/e2e/tests/backend/endpoints/api/v1/auth/cli/complete.test.ts
…ement and error handling - Integrated users CRUD handlers for better user validation during anonymous session handling. - Updated SQL queries to ensure proper fetching of refresh tokens and CLI auth attempts from the correct tenancy database. - Enhanced error handling for invalid refresh tokens and user not found scenarios. - Improved response schemas for CLI authentication completion to support various response types. - Updated demo page to utilize sessionStorage for refresh tokens, ensuring better security practices in the development environment.
Contributor
|
Tip: Greploops — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/backend/src/app/api/latest/auth/cli/route.tsx (2)
108-116: Turn the insert result into an explicit invariant.
rows[0]is assumed to exist. If this query ever returns zero rows, the response builder degrades into a genericundefinedcrash instead of a clear backend error. Guard it before dereferencing.As per coding guidelines, "Use
?? throwErr(...)instead of non-null assertions or silent fallback values; include explicit error messages."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/auth/cli/route.tsx` around lines 108 - 116, The code dereferences rows[0] (assigned to cliAuth) without checking it; change this to an explicit invariant using the project's throwErr helper (e.g., replace direct rows[0] use in the route handler that builds the response with cliAuth = rows[0] ?? throwErr("No CLI auth row returned for polling_code query") ), then use cliAuth.pollingCode / loginCode / expiresAt as before; this ensures a clear error is thrown if the query returns zero rows instead of an undefined crash.
67-81: Keepuserinside the type system.
let user;drops this value out of compile-time checking, souser.is_anonymousis no longer verified by TypeScript. Returning the value from a small helper/IIFE keeps the error mapping and the user shape typed.As per coding guidelines, "Code defensively and avoid `any` type; if used, include a comment explaining why the type system fails."♻️ Proposed refactor
- let user; - try { - user = await usersCrudHandlers.adminRead({ - tenancy, - user_id: refreshTokenObj.projectUserId, - allowedErrorTypes: [KnownErrors.UserNotFound], - }); - } catch (error) { - if (error instanceof KnownErrors.UserNotFound) { - throw new StatusError(400, "User not found for provided refresh token"); - } - throw error; - } + const user = await (async () => { + try { + return await usersCrudHandlers.adminRead({ + tenancy, + user_id: refreshTokenObj.projectUserId, + allowedErrorTypes: [KnownErrors.UserNotFound], + }); + } catch (error) { + if (error instanceof KnownErrors.UserNotFound) { + throw new StatusError(400, "User not found for provided refresh token"); + } + throw error; + } + })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/auth/cli/route.tsx` around lines 67 - 81, Replace the untyped "let user;" pattern with a typed helper/IIFE that returns the result of usersCrudHandlers.adminRead so the compiler knows the user shape; inside the helper call usersCrudHandlers.adminRead({ tenancy, user_id: refreshTokenObj.projectUserId, allowedErrorTypes: [KnownErrors.UserNotFound] }) and map KnownErrors.UserNotFound to throw new StatusError(400, "User not found for provided refresh token"), then return the typed user to the outer scope so subsequent checks like user.is_anonymous are statically type-checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/backend/src/app/api/latest/auth/cli/route.tsx`:
- Around line 108-116: The code dereferences rows[0] (assigned to cliAuth)
without checking it; change this to an explicit invariant using the project's
throwErr helper (e.g., replace direct rows[0] use in the route handler that
builds the response with cliAuth = rows[0] ?? throwErr("No CLI auth row returned
for polling_code query") ), then use cliAuth.pollingCode / loginCode / expiresAt
as before; this ensures a clear error is thrown if the query returns zero rows
instead of an undefined crash.
- Around line 67-81: Replace the untyped "let user;" pattern with a typed
helper/IIFE that returns the result of usersCrudHandlers.adminRead so the
compiler knows the user shape; inside the helper call
usersCrudHandlers.adminRead({ tenancy, user_id: refreshTokenObj.projectUserId,
allowedErrorTypes: [KnownErrors.UserNotFound] }) and map
KnownErrors.UserNotFound to throw new StatusError(400, "User not found for
provided refresh token"), then return the typed user to the outer scope so
subsequent checks like user.is_anonymous are statically type-checked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a0598829-87ba-494d-9c99-185ac92adc06
📒 Files selected for processing (6)
apps/backend/src/app/api/latest/auth/cli/complete/route.tsxapps/backend/src/app/api/latest/auth/cli/route.tsxexamples/demo/src/app/cli-auth-demo/page.tsxpackages/stack-cli/src/commands/login.tspackages/template/src/components-page/cli-auth-confirm.tsxpackages/template/src/lib/stack-app/apps/interfaces/client-app.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/stack-cli/src/commands/login.ts
- apps/backend/src/app/api/latest/auth/cli/complete/route.tsx
- examples/demo/src/app/cli-auth-demo/page.tsx
- packages/template/src/components-page/cli-auth-confirm.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)
3060-3062: Use explicit null/undefined check forpromptLinkcallback.Prefer
options.promptLink != nullover a truthiness check here for consistency and to avoid relying on implicit coercion.♻️ Suggested tweak
- if (options.promptLink) { + if (options.promptLink != null) { options.promptLink(url, loginCode); } else {As per coding guidelines: "Prefer explicit null/undefinedness checks over boolean checks (e.g.,
foo == nullinstead of!foo)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts` around lines 3060 - 3062, The current truthiness check for the callback uses "if (options.promptLink)" which can mis-handle falsy but present values; change it to an explicit null/undefined check using "options.promptLink != null" before invoking options.promptLink(url, loginCode). Locate the block building url from loginCode and calling options.promptLink and replace the conditional accordingly, preserving the same call to options.promptLink(url, loginCode).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts`:
- Around line 3060-3062: The current truthiness check for the callback uses "if
(options.promptLink)" which can mis-handle falsy but present values; change it
to an explicit null/undefined check using "options.promptLink != null" before
invoking options.promptLink(url, loginCode). Locate the block building url from
loginCode and calling options.promptLink and replace the conditional
accordingly, preserving the same call to options.promptLink(url, loginCode).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7e0001c-5d8d-44f5-8726-6d12e2305e23
📒 Files selected for processing (2)
apps/backend/prisma/schema.prismapackages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/backend/prisma/schema.prisma
N2D4
approved these changes
Apr 12, 2026
- Removed obsolete migrations for adding `anonRefreshToken` and creating an index on `signUpEmailNormalized`. - Introduced a new migration that combines the previous changes into a single migration file. - Updated import statements in `route.tsx` files to ensure proper usage of the `Prisma` client and `Tenancy` module.
- Changed the error handling logic to throw known errors instead of redirecting, improving clarity in error management.
…logic - Introduced a new function to check for specific known errors during OAuth callback processing. - Refactored the account linking and sign-in logic to improve clarity and reduce redundancy. - Ensured proper handling of existing OAuth accounts and streamlined the flow for linking accounts with users.
- Created a migration to add the "anonRefreshToken" column to the "CliAuthAttempt" table. - Added a new index for "signUpEmailNormalized" on the "ProjectUser" table to improve query performance. - Removed obsolete migration that previously handled these changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CliAuthAttemptwithanonRefreshTokenand a migration.POST /auth/cliaccepts optionalanon_refresh_token(must be an anonymous user's refresh token for the current project).POST /auth/cli/completesupportsmodecheck(anonymous vs none),claim-anon-session(issue tokens for the linked anonymous session), andcomplete(bind the browser session's refresh token to the attempt). Completing clearsanonRefreshTokenon the row. We do not merge anonymous account data into the signed-in user (that behavior was removed as a security risk; the anonymous user remains unchanged).STACK_CLI_ANON_REFRESH_TOKEN, SDK/spec updates, and e2e coverage.Summary by CodeRabbit
New Features
Tests
Documentation