feat: guest middleware - #26
Merged
Merged
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds guest-only route protection across Next/Nuxt/SvelteKit, a Next request-scoped AsyncLocalStorage context, client-side auth refresh after login, Nuxt shared schema/type relocations, CLI-generated model-registry type emission, and updates tests and documentation. ChangesGuest-Only Route Protection
Auth State Refresh After Login
Nuxt Migration to Shared Schemas and Type Paths
Model Registry Generation and CLI Updates
Next.js Request Context Infrastructure
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Browser
participant Server as Auth Server
participant Store as Session Store
User->>Client: Click Login
Client->>Server: POST /api/login (credentials)
Server->>Store: Validate & create session
Store-->>Server: Session token
Server-->>Client: 200 OK + Set-Cookie
Note over Client: client calls refreshUser()
Client->>Server: GET /api/auth/current-user
Server->>Store: Fetch session user
Store-->>Server: User data
Server-->>Client: User profile
Client->>Client: update auth state
Client->>Client: router.replace(redirectTo)
sequenceDiagram
actor Visitor
participant Browser
participant Middleware as Guest-Only Middleware
participant AuthProvider as Auth Provider
Visitor->>Browser: Navigate to /login
Browser->>Middleware: route request
Middleware->>AuthProvider: check auth
AuthProvider-->>Middleware: { authenticated: true/false }
alt authenticated
Middleware-->>Browser: 303 Redirect to /admin
else
Middleware-->>Browser: continue to /login
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/adapter-nuxt/src/module.ts (1)
381-387:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
model-registry.d.tsreferenced unconditionally may cause TypeScript errors on cold-start.
model-registry.d.tsis generated by the Holo CLI, not by this module. When developers runnuxt dev/nuxt preparedirectly on a fresh project (before the CLI has been executed), the file won't exist and TypeScript will fail to resolve the/// <reference path="..." />directive with "File not found."Since
mkdirandwriteFileare already imported, the simplest fix is to write an empty placeholder during module setup if the file is absent:🛡️ Proposed fix — create an empty placeholder if the file is missing
+ const modelRegistryDir = resolve(rootDir, '.holo-js/generated') const modelRegistryTypesPath = resolve(rootDir, '.holo-js/generated/model-registry.d.ts') + try { + await mkdir(modelRegistryDir, { recursive: true }) + await writeFile(modelRegistryTypesPath, '', { flag: 'ax' }) // create only if absent + } catch { + // file already exists — CLI will overwrite with real content + }🤖 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/adapter-nuxt/src/module.ts` around lines 381 - 387, The prepare:types hook currently unconditionally references modelRegistryTypesPath which can cause TypeScript "File not found" on cold-start; adjust the module setup so when opts._holoTypesRegistered is being set and before pushing the reference in nuxt.hook('prepare:types') check whether modelRegistryTypesPath exists and if not create its parent directory (use mkdir) and write an empty placeholder file (use writeFile) so the /// <reference path="..."> resolves even before the Holo CLI generates it; keep the same opts._holoTypesRegistered guard and only create the placeholder when the file is missing.
🧹 Nitpick comments (1)
packages/cli/src/project/registry.ts (1)
141-174: ⚡ Quick winUnused type imports in
renderGeneratedModelTypeswhenmodelsis empty.When
modelsis an empty array, the generated.d.tsfile still unconditionally emits:import type { EmptyScopeMap, GeneratedSchemaTable, ModelReference } from '@holo-js/db'but none of these types are referenced (no
aliasesare produced). This departs from the pattern used by sibling renderers —renderGeneratedQueueTypes,renderGeneratedBroadcastTypes, andrenderGeneratedEventTypesall guard their type imports with a conditional. TypeScript'sverbatimModuleSyntax/isolatedDeclarationsor a linting rule such as@typescript-eslint/no-unused-varsapplied to the generated output could surface errors.♻️ Suggested fix — guard type import on non-empty models
export function renderGeneratedModelTypes(models: readonly GeneratedModelRegistryEntry[]): string { const aliases = models.map((entry, index) => { return `type HoloModel${index} = ModelReference<GeneratedSchemaTable<${JSON.stringify(entry.tableName)}>, EmptyScopeMap>` }) const registryEntries = models.map((entry, index) => { return ` readonly ${JSON.stringify(entry.name)}: HoloModel${index}` }) const registeredModelsDeclaration = registryEntries.length > 0 ? [ ' interface RegisteredModels {', ...registryEntries, ' }', ] : [ ' // eslint-disable-next-line `@typescript-eslint/no-empty-object-type`', ' interface RegisteredModels {}', ] return [ '// Generated by holo prepare. Do not edit.', '', 'import \'./schema.generated\'', + ...(models.length > 0 + ? ['import type { EmptyScopeMap, GeneratedSchemaTable, ModelReference } from \'@holo-js/db\''] + : []), - 'import type { EmptyScopeMap, GeneratedSchemaTable, ModelReference } from \'@holo-js/db\'', '', - ...aliases, - '', + ...(aliases.length > 0 ? [...aliases, ''] : []), 'declare module \'@holo-js/db\' {', ...registeredModelsDeclaration, '}', '', 'export {}', '', ].join('\n') }🤖 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/cli/src/project/registry.ts` around lines 141 - 174, The generated .d.ts always includes the type import even when models is empty; update renderGeneratedModelTypes to only emit the line "import type { EmptyScopeMap, GeneratedSchemaTable, ModelReference } from '@holo-js/db'" when models.length > 0 (i.e., when aliases will be produced). Locate renderGeneratedModelTypes and wrap or conditionally add that import in the returned array/build so the import is omitted for empty models, matching the conditional pattern used in renderGeneratedQueueTypes/renderGeneratedBroadcastTypes/renderGeneratedEventTypes.
🤖 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.
Inline comments:
In `@apps/docs/docs/auth/current-auth-client.md`:
- Around line 56-59: The example should not let refreshUser() rejection block
navigation: wrap the auth.refreshUser() call in a try-catch (or handle its
promise rejection) so that regardless of refreshUser() failing you still call
router.replace(submission.data.redirectTo); locate the block that checks
submission?.ok and submission.data?.redirectTo and update it to attempt
auth.refreshUser() inside try { await auth.refreshUser() } catch (e) { /*
optional log */ } then call router.replace(submission.data.redirectTo)
outside/after the catch to ensure redirect always occurs.
In `@packages/adapter-next/src/server.ts`:
- Around line 45-47: The RegExp route check using route.test(normalizedPathname)
can produce flaky results for stateful regexes with /g or /y because lastIndex
is preserved; before calling route.test(normalizedPathname) inside the route
instanceof RegExp branch, reset route.lastIndex = 0 so each test starts from the
beginning (locate the route instanceof RegExp check around the RegExp.test call
and set lastIndex to 0 just prior to invoking test).
In `@packages/adapter-nuxt/src/runtime/server/protection.ts`:
- Around line 37-39: The RegExp matcher can be stateful because test() advances
lastIndex for /g or /y flags; before using the existing `route` RegExp to match
`normalizedPathname` (the branch where `if (route instanceof RegExp)`), reset
`route.lastIndex = 0` and then call `route.test(normalizedPathname)` so repeated
calls using the same RegExp instance do not yield intermittent false negatives;
update the branch handling the `route` RegExp to reset lastIndex prior to
testing.
In `@packages/adapter-sveltekit/src/server.ts`:
- Around line 92-104: The guestOnly handler may cause self-redirect loops when
options.redirectTo resolves to the same path (or when routes are broad); update
the guestOnly function to compute the resolved redirect URL (new
URL(options.redirectTo, event.url)) and compare its pathname (and optionally
search/hash) against event.url.pathname (and search/hash) and, if they match,
skip the redirect and return resolve(event) instead; keep using matchesRoutes,
auth({ guard: options.guard }) and Response.redirect as before, but only call
Response.redirect when the resolved redirect target is different from the
current request URL.
- Around line 53-55: The RegExp matcher can carry a persistent lastIndex for
global or sticky flags causing intermittent failures; before calling
route.test(normalizedPathname) reset the regex's lastIndex (e.g., set
route.lastIndex = 0) so tests are consistent; update the check in the
route-matching logic where the symbols route and normalizedPathname are used and
apply the same change to the equivalent matcher locations in adapter-next
(packages/adapter-next/src/server.ts) and adapter-nuxt
(packages/adapter-nuxt/src/runtime/server/protection.ts).
---
Outside diff comments:
In `@packages/adapter-nuxt/src/module.ts`:
- Around line 381-387: The prepare:types hook currently unconditionally
references modelRegistryTypesPath which can cause TypeScript "File not found" on
cold-start; adjust the module setup so when opts._holoTypesRegistered is being
set and before pushing the reference in nuxt.hook('prepare:types') check whether
modelRegistryTypesPath exists and if not create its parent directory (use mkdir)
and write an empty placeholder file (use writeFile) so the /// <reference
path="..."> resolves even before the Holo CLI generates it; keep the same
opts._holoTypesRegistered guard and only create the placeholder when the file is
missing.
---
Nitpick comments:
In `@packages/cli/src/project/registry.ts`:
- Around line 141-174: The generated .d.ts always includes the type import even
when models is empty; update renderGeneratedModelTypes to only emit the line
"import type { EmptyScopeMap, GeneratedSchemaTable, ModelReference } from
'@holo-js/db'" when models.length > 0 (i.e., when aliases will be produced).
Locate renderGeneratedModelTypes and wrap or conditionally add that import in
the returned array/build so the import is omitted for empty models, matching the
conditional pattern used in
renderGeneratedQueueTypes/renderGeneratedBroadcastTypes/renderGeneratedEventTypes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62eef95a-1145-406a-a2e9-83455e31154b
📒 Files selected for processing (59)
apps/blog-next/app/login/page.tsxapps/blog-next/proxy.tsapps/blog-nuxt/app/app.vueapps/blog-nuxt/app/middleware/guest-only.global.tsapps/blog-nuxt/app/pages/admin/categories/[id]/edit.vueapps/blog-nuxt/app/pages/admin/categories/index.vueapps/blog-nuxt/app/pages/admin/index.vueapps/blog-nuxt/app/pages/admin/posts/[id]/edit.vueapps/blog-nuxt/app/pages/admin/posts/index.vueapps/blog-nuxt/app/pages/admin/posts/new.vueapps/blog-nuxt/app/pages/admin/tags/[id]/edit.vueapps/blog-nuxt/app/pages/admin/tags/index.vueapps/blog-nuxt/app/pages/categories/[slug].vueapps/blog-nuxt/app/pages/forgot-password.vueapps/blog-nuxt/app/pages/index.vueapps/blog-nuxt/app/pages/login.vueapps/blog-nuxt/app/pages/posts/[slug].vueapps/blog-nuxt/app/pages/posts/index.vueapps/blog-nuxt/app/pages/register.vueapps/blog-nuxt/app/pages/reset-password.vueapps/blog-nuxt/app/pages/tags/[slug].vueapps/blog-nuxt/app/pages/verify-email.vueapps/blog-nuxt/package.jsonapps/blog-nuxt/server/api/forgot-password.post.tsapps/blog-nuxt/server/api/login.post.tsapps/blog-nuxt/server/api/register.post.tsapps/blog-nuxt/server/api/reset-password.post.tsapps/blog-nuxt/server/api/verify-email.post.tsapps/blog-nuxt/server/holo-models.d.tsapps/blog-nuxt/shared/schemas/auth.tsapps/blog-sveltekit/src/hooks.server.tsapps/docs/docs/auth/current-auth-client.mdapps/docs/docs/forms/framework-integration.mdapps/docs/docs/forms/server-validation.mdpackages/adapter-next/src/request-context.tspackages/adapter-next/src/runtime.tspackages/adapter-next/src/server.tspackages/adapter-next/tests/server.test.tspackages/adapter-nuxt/package.jsonpackages/adapter-nuxt/src/module.tspackages/adapter-nuxt/src/runtime/server/protection.d.tspackages/adapter-nuxt/src/runtime/server/protection.tspackages/adapter-nuxt/src/runtime/shims.d.tspackages/adapter-nuxt/tests/module.test.tspackages/adapter-nuxt/tests/package.test.tspackages/adapter-nuxt/tests/protection.test.tspackages/adapter-nuxt/tests/setup.test.tspackages/adapter-sveltekit/src/server.tspackages/adapter-sveltekit/tests/server.test.tspackages/auth/src/client-runtime.tspackages/auth/tests/package.test.tspackages/cli/src/dev.tspackages/cli/src/project/discovery.tspackages/cli/src/project/registry.tspackages/cli/src/project/scaffold/framework-renderers.tspackages/cli/src/project/scaffold/framework.tspackages/cli/src/project/shared.tspackages/cli/tests/cli.test.tstests/example-app-auth-flow.mjs
💤 Files with no reviewable changes (1)
- apps/blog-nuxt/server/holo-models.d.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/docs/docs/auth/current-auth-client.md (1)
107-110:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing error handling for
refreshUser()prevents navigation on failure.If
auth.refreshUser()rejects, the subsequentinvalidateAll()andgotocalls are never reached, leaving the user on the login page despite successful authentication. Apply the same error-handling pattern used in the Next.js example for consistency.📝 Proposed fix
if (submission?.ok === true && typeof submission.data?.redirectTo === 'string') { - await auth.refreshUser() + try { + await auth.refreshUser() + } catch (error) { + console.warn('Auth refresh failed after login.', error) + } + await invalidateAll() await goto(submission.data.redirectTo) }🤖 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 `@apps/docs/docs/auth/current-auth-client.md` around lines 107 - 110, The call to auth.refreshUser() can reject and block subsequent navigation; wrap the refresh in a try/catch (as done in the Next.js example) so errors from auth.refreshUser() are caught and handled (log or ignore), then always call invalidateAll() and goto(submission.data.redirectTo) after the catch; update the block around refreshUser/invalidateAll/goto to ensure refreshUser failures do not prevent invalidateAll() or goto() from running.
🤖 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.
Inline comments:
In `@apps/docs/docs/auth/current-auth-client.md`:
- Around line 84-86: The code in the condition where `submission?.ok === true`
and `submission.data?.redirectTo` is a string calls `refreshUser()` without
handling possible rejections, preventing `navigateTo` on failure. To fix this,
wrap the `refreshUser()` call in a try-catch block or use `.catch()` to handle
errors, ensuring that `navigateTo(submission.data.redirectTo)` is still invoked
even if `refreshUser()` fails, following the error-handling pattern demonstrated
in the Next.js example.
---
Duplicate comments:
In `@apps/docs/docs/auth/current-auth-client.md`:
- Around line 107-110: The call to auth.refreshUser() can reject and block
subsequent navigation; wrap the refresh in a try/catch (as done in the Next.js
example) so errors from auth.refreshUser() are caught and handled (log or
ignore), then always call invalidateAll() and goto(submission.data.redirectTo)
after the catch; update the block around refreshUser/invalidateAll/goto to
ensure refreshUser failures do not prevent invalidateAll() or goto() from
running.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd6a1fce-7291-4787-9611-0b9d3c64da41
📒 Files selected for processing (11)
apps/docs/docs/auth/current-auth-client.mdpackages/adapter-next/src/server.tspackages/adapter-next/tests/server.test.tspackages/adapter-nuxt/src/module.tspackages/adapter-nuxt/src/runtime/server/protection.tspackages/adapter-nuxt/tests/module.test.tspackages/adapter-nuxt/tests/protection.test.tspackages/adapter-sveltekit/src/server.tspackages/adapter-sveltekit/tests/server.test.tspackages/cli/src/project/registry.tspackages/cli/tests/cli.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/adapter-nuxt/tests/module.test.ts
- packages/adapter-nuxt/tests/protection.test.ts
- packages/adapter-nuxt/src/module.ts
- packages/adapter-sveltekit/tests/server.test.ts
- packages/adapter-sveltekit/src/server.ts
- packages/adapter-nuxt/src/runtime/server/protection.ts
- packages/adapter-next/src/server.ts
- packages/cli/tests/cli.test.ts
This was referenced May 24, 2026
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
1 participant
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.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes