feat: onboarding plugin #35
Conversation
|
|
@jslno is attempting to deploy a commit to the ping-maxwell's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughIntroduces a new onboarding plugin package with server, client, types, utilities, adapter, and presets. Adds build scripts, configuration, tests, and documentation. Implements endpoints for onboarding steps, access checks, optional skip, session hooks, and storage via internal DB or secondary storage. Updates docs navigation and plugin catalog. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as Client (onboardingClient)
participant S as Auth Server (onboarding)
participant A as Adapter/Storage
rect rgb(245,248,255)
note over C,S: Complete onboarding step (POST)
U->>C: Submit step data
C->>C: Coerce method to POST for /onboarding/(step|skip-step)
C->>S: POST /onboarding/step/:path {body}
S->>S: verifyOnboarding(ctx)
S->>A: getShouldOnboard(userId)
A-->>S: shouldOnboard
S->>S: validate input, once/required checks
S->>A: updateOnboardingState(userId, {completedSteps,...})
A-->>S: ok
S-->>C: { completedSteps, data }
C-->>U: Show result
end
sequenceDiagram
autonumber
actor U as User
participant C as Client (onboardingClient)
participant S as Auth Server (get-session hook)
participant A as Adapter/Storage
rect rgb(245,255,245)
note over C,S: Session retrieval and redirect hint
U->>C: getSession()
C->>S: GET /get-session
S->>A: getShouldOnboard(userId)
A-->>S: shouldOnboard
alt shouldOnboard
S-->>C: session + {onboardingRedirect:true}
C->>C: onSuccess -> options.onOnboardingRedirect()
else not onboarding
S-->>C: session
end
end
sequenceDiagram
autonumber
participant S as Auth Server (sign-up)
participant A as Adapter/Storage
note over S: Auto-enable on sign-up (optional)
S->>S: create user/session
alt autoEnableOnSignUp true
S->>A: updateOnboardingState(userId, {shouldOnboard:true})
A-->>S: ok
else disabled
S-->>S: no-op
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (20)
packages/plugins/onboarding/.npmignore (1)
1-4: Remove redundant .npmignore in onboarding pluginnpm will honor the
filesfield inpackages/plugins/onboarding/package.json(which is set to"./dist/**/*") and ignore any.npmignorerules. Keeping.npmignorehere has no effect on what gets published and may confuse contributors.• Delete
packages/plugins/onboarding/.npmignoreto clean up unused configuration.
• (Optional) runnpm pack --dry-runin theonboardingpackage to confirm onlydist/files are included.packages/plugins/onboarding/.gitignore (1)
1-5: Tighten ignore list for typical TS/Bun artifacts.Current entries are fine; adding a few common patterns reduces accidental churn in diffs.
.DS_Store .env .env.local node_modules dist +.turbo +*.log +*.tsbuildinfo +coverage/ +# If this package maintains its own lock (monorepo dependent) +# bun.lockbpackages/plugins/onboarding/README.md (1)
1-11: Add minimal install and quick-start sections.The README is clear but a bit sparse. A tiny “Install” + “Quick start” helps consumers wire this up without leaving the repo.
# @better-auth-kit/onboarding Easily add user onboarding to your authentication flow. ## Documentation Learn more about this plugin in the [better-auth-kit documentation](https://better-auth-kit.com/docs/plugins/onboarding). +## Installation + +```bash +pnpm add @better-auth-kit/onboarding +# or +npm i @better-auth-kit/onboarding +``` + +## Quick start + +See the docs for server/client setup, route wiring, and middleware integration. + ## License [MIT](LICENSE)packages/plugins/onboarding/build.ts (1)
1-6: Handle build errors and keep config strongly typed.If
buildreturns a Promise, the current call may drop rejections. Wrap with a resolved Promise to support both sync/async implementations, and usesatisfiesfor stronger config typing.-import { build, type Config } from "@better-auth-kit/internal-build"; - -export const config: Config = { - enableDts: true, -}; -build(config); +import { build, type Config } from "@better-auth-kit/internal-build"; + +export const config = { + enableDts: true, +} as const satisfies Config; + +void Promise.resolve(build(config)).catch((err) => { + console.error("[onboarding] build failed:", err); + // Ensure non-zero exit so CI fails loudly. + process.exit(1); +});packages/plugins/onboarding/build-dev.ts (1)
1-4: Add safety around dev build errors.Same rationale as build.ts—guard against unhandled rejections so local dev and CI report issues clearly.
import { buildDev } from "@better-auth-kit/internal-build"; import { config } from "./build"; -buildDev(config); +void Promise.resolve(buildDev(config)).catch((err) => { + console.error("[onboarding] dev build failed:", err); + process.exit(1); +});packages/plugins/onboarding/LICENSE (1)
1-7: Nit: Add standard "MIT License" header, normalize quotes, and add trailing newline
- Consider adding the conventional "MIT License" title and using straight ASCII quotes for portability and to minimize noisy diffs.
- Minor: add a trailing newline at EOF.
Apply:
+MIT License + -Copyright 2025 - present, ping-maxwell +Copyright (c) 2025-present, ping-maxwell - -Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the “Software”), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. -THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +packages/plugins/onboarding/src/error-codes.ts (1)
1-4: Normalize punctuation in ONBOARDING_ERROR_CODESThe
ALREADY_ONBOARDEDmessage is missing a trailing period, which makes it inconsistent withSTEP_ALREADY_COMPLETED.. This change is a low-risk nit:packages/plugins/onboarding/src/error-codes.ts export const ONBOARDING_ERROR_CODES = { - ALREADY_ONBOARDED: "Already onboarded", + ALREADY_ONBOARDED: "Already onboarded.", STEP_ALREADY_COMPLETED: "Step already completed.", } as const;Locations referencing
ONBOARDING_ERROR_CODES(to keep in mind if you rename or split this constant):
- packages/plugins/onboarding/src/error-codes.ts
- packages/plugins/onboarding/src/verify-onboarding.ts
- packages/plugins/onboarding/src/index.ts
- packages/plugins/onboarding/tests/onboarding.test.ts
Optional refactor for future consideration: separate stable machine codes from human‐readable messages to enable i18n and more robust client handling. This would involve:
+// machine codes +export const ONBOARDING_ERROR_CODE = { + ALREADY_ONBOARDED: "ALREADY_ONBOARDED", + STEP_ALREADY_COMPLETED: "STEP_ALREADY_COMPLETED", +} as const; + +export type OnboardingErrorCode = keyof typeof ONBOARDING_ERROR_CODE; + +// user-facing messages +export const ONBOARDING_ERROR_MESSAGES: Record<OnboardingErrorCode, string> = { + ALREADY_ONBOARDED: "Already onboarded.", + STEP_ALREADY_COMPLETED: "Step already completed.", +} as const; + +// maintain backward compatibility +export const ONBOARDING_ERROR_CODES = ONBOARDING_ERROR_MESSAGES;This split is purely optional—if adopted, remember to update all references listed above.
packages/plugins/onboarding/src/schema.ts (1)
10-14: Consider default values and readonly semantics for completedSteps
- Add default: [] to simplify downstream logic and avoid nullish checks.
- If steps should not be mutated directly via input, input: false is correctly set; also consider modeling as readonly for clarity (type-level).
completedSteps: { type: "string[]", required: false, input: false, + default: [], },packages/plugins/onboarding/tsconfig.json (1)
26-27: Broaden include globs to capture nested source files"./src/" will miss nested directories. Use "./src/**/".
- "include": ["./src/*"], + "include": ["./src/**/*"],packages/plugins/onboarding/src/types.ts (1)
27-34: Use ZodType over ZodSchema and prefer unknown over any for Schema
- zod’s primary generic is ZodType<TOutput, Def, TInput>; ZodSchema is looser and may not exist in all versions. Using ZodType is more idiomatic and compatible.
- Default Schema to unknown to avoid leaking any.
-import type { ZodSchema } from "zod"; +import type { ZodType } from "zod"; @@ -export type OnboardingStep< - Schema extends Record<string, any> | undefined | null = any, +export type OnboardingStep< + Schema extends Record<string, unknown> | undefined | null = unknown, Result = unknown, > = { - input?: ZodSchema<Schema>; + input?: ZodType<Schema>; handler: ActionEndpointContext<Schema, Result>; once?: boolean; };packages/plugins/onboarding/src/routes/should-onboard.ts (1)
11-15: Optional: Consider forward-compatible response shape.Returning a naked boolean limits extensibility (e.g., adding metadata, ETag, next-step hints). If you anticipate future additions, consider
{ shouldOnboard: true }. This would be a breaking API change for clients today, so only if you plan a version bump.- async (ctx) => { - await verifyOnboarding(ctx); - return true; - }, + async (ctx) => { + await verifyOnboarding(ctx); + return { shouldOnboard: true }; + },packages/plugins/onboarding/src/verify-onboarding.ts (1)
18-21: Avoid leaking sensitive identifiers in derived keys.
key: \${session.user.id}!${session.session.id}`` concatenates raw user and session IDs. Even if not currently returned to clients, it risks accidental exposure through logs/metrics. Prefer a stable hash/HMAC to derive a non-PII cache key.-import { APIError, getSessionFromCtx } from "better-auth/api"; +import { APIError, getSessionFromCtx } from "better-auth/api"; +import { createHash } from "node:crypto"; export async function verifyOnboarding(ctx: GenericEndpointContext) { const session = await getSessionFromCtx(ctx); @@ return { session, - key: `${session.user.id}!${session.session.id}`, + // Non-PII stable key derived from user+session ids + key: createHash("sha256") + .update(`${session.user.id}:${session.session.id}`) + .digest("hex"), }; }If an HMAC with a server secret is preferred for tamper resistance, I can provide that variant.
packages/plugins/onboarding/tests/auth.ts (1)
10-11: Use an in-memory SQLite DB for deterministic, isolated tests.A file-backed
test.dbcan persist across runs and introduce flakiness. Using":memory:"improves isolation and speed.-const db = database("test.db"); +const db = database(":memory:");If other tests rely on file persistence, consider parameterizing the DB path:
getAuth(options, dbPath = ":memory:").packages/plugins/onboarding/tests/onboarding.test.ts (1)
54-63: Nit: Strengthen redirect assertions for clarity.Optionally assert call count to ensure no duplicate redirects:
toHaveBeenCalledTimes(1). Not required, but it tightens expectations.- expect(mockOnboardingRedirect).toHaveBeenCalled(); + expect(mockOnboardingRedirect).toHaveBeenCalledTimes(1);packages/plugins/onboarding/src/client.ts (1)
28-31: Debounce onboarding redirect callback to avoid repeated triggers.If a client polls
/get-sessionwhileshouldOnboardis true,onSuccesscan trigger repeatedly. Debounce to fire once per plugin lifecycle.Apply this diff:
}) => { - return { + let didRedirect = false; + return { @@ - async onSuccess(context) { - if (context.data?.onboardingRedirect) { - if (options?.onOnboardingRedirect) { - await options.onOnboardingRedirect(); - } - } - }, + async onSuccess(context) { + if (context.data?.onboardingRedirect && !didRedirect) { + didRedirect = true; + await options?.onOnboardingRedirect?.(); + } + },Also applies to: 44-50
packages/plugins/onboarding/src/index.ts (2)
73-81: Potential race on concurrent step submissions; consider atomic persistence for completedSteps.Two rapid submissions (multi-tab or flaky network retries) can read the same
completedSteps, add different items, and last writer wins. If your adapter supports atomic set-merge/array-union ops, prefer them; otherwise fetch-and-compare with a where-clause that guards on the preimage.Example approach (pseudocode; adapt to your adapter):
await adapter.update({ model: "user", where: [{ field: "id", value: session.user.id }], // whereExtra: [{ field: "completedSteps", value: currentCompletedStepsSnapshot }], update: { // completedSteps: arrayUnion(id), // shouldOnboard: isCompletionStep ? false : undefined, }, });Also applies to: 81-91, 92-96
48-64: DRY: factor repeated completedSteps reads into a helper.
findOne(... select: ["completedSteps"])is duplicated in both handlers. Extract to a small util to reduce chances of divergent fixes later.+async function getCompletedSteps(adapter: typeof ctx.context.adapter, userId: string) { + const row = await adapter.findOne<{ completedSteps?: string[] }>({ + model: "user", + where: [{ field: "id", value: userId }], + select: ["completedSteps"], + }); + return row?.completedSteps ?? []; +}Also applies to: 112-132
packages/plugins/onboarding/src/utils.ts (2)
10-13: Redundant conditional in transformPath mapping.Both branches perform identical logic. Simplify for readability.
- return index === 0 - ? segment.charAt(0).toUpperCase() + segment.slice(1) - : segment.charAt(0).toUpperCase() + segment.slice(1); + return segment.charAt(0).toUpperCase() + segment.slice(1);
34-39: Optional: Handle underscores in transformClientPath if step keys may contain them.If step IDs can contain
_, consider normalizing them to dashes too (/_/g→-). If not applicable, ignore.- .replace(/[\/]+/g, "-") + .replace(/[\/_]+/g, "-")packages/plugins/onboarding/src/internal-types.ts (1)
6-11: Allow steps without an input schema.
InferStepInputyieldsneverwheninputis absent, tightening types more than necessary and making “no-input” steps awkward. Preferundefinedto represent no body schema.-type InferStepInput<K extends OnboardingStep> = K extends { input?: infer I } - ? I extends ZodSchema<any> - ? I - : never - : never; +type InferStepInput<K extends OnboardingStep> = K extends { input?: infer I } + ? I extends ZodSchema<any> + ? I + : undefined + : undefined;If
createAuthEndpointrequiresbodywhen present but allows omission, this change will better model optional input. Otherwise, keep as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackages/plugins/onboarding/tests/test.dbis excluded by!**/*.db
📒 Files selected for processing (19)
packages/plugins/onboarding/.gitignore(1 hunks)packages/plugins/onboarding/.npmignore(1 hunks)packages/plugins/onboarding/LICENSE(1 hunks)packages/plugins/onboarding/README.md(1 hunks)packages/plugins/onboarding/build-dev.ts(1 hunks)packages/plugins/onboarding/build.ts(1 hunks)packages/plugins/onboarding/package.json(1 hunks)packages/plugins/onboarding/src/client.ts(1 hunks)packages/plugins/onboarding/src/error-codes.ts(1 hunks)packages/plugins/onboarding/src/index.ts(1 hunks)packages/plugins/onboarding/src/internal-types.ts(1 hunks)packages/plugins/onboarding/src/routes/should-onboard.ts(1 hunks)packages/plugins/onboarding/src/schema.ts(1 hunks)packages/plugins/onboarding/src/types.ts(1 hunks)packages/plugins/onboarding/src/utils.ts(1 hunks)packages/plugins/onboarding/src/verify-onboarding.ts(1 hunks)packages/plugins/onboarding/tests/auth.ts(1 hunks)packages/plugins/onboarding/tests/onboarding.test.ts(1 hunks)packages/plugins/onboarding/tsconfig.json(1 hunks)
🔇 Additional comments (13)
packages/plugins/onboarding/src/schema.ts (2)
3-17: Please validate array type usage in Onboarding schemaI couldn’t find any precedent for using the
"string[]"literal in our other plugin schemas—all existing fields declaretype: "string"(and would rely on anisArray: trueflag or similar if arrays are intended). To avoid a silent mismatch, please:
- Check the
AuthPluginSchemadefinition to confirm that it permits"string[]"as atypevalue.- If it does not, change
completedStepsto explicitly use an array flag, for example:completedSteps: { - type: "string[]", + type: "string", + isArray: true, required: false, input: false, },- If
"string[]"is indeed supported byAuthPluginSchema, feel free to keep it as-is.
6-9: I've queued up a script to inspect the onboarding schema and compare it with other plugins for anyinputdirectives. Once we confirm the existing patterns (or absence thereof), we can determine if theshouldOnboardfield must explicitly setinput: false.packages/plugins/onboarding/tsconfig.json (1)
9-15: Disable allowJs for this TS-first packageThe
allowJsoption is intended for projects that mix JavaScript and TypeScript; since TypeScript 3.7, it can be used alongsidedeclarationto emit.d.tsfrom JavaScript sources. However, this onboarding plugin is TypeScript-only, so we can safely turn offallowJsand retaindeclaration: trueto generate declarations from your.tsfiles.• File packages/plugins/onboarding/tsconfig.json, adjust lines 9–15:
"jsx": "react-jsx", - "allowJs": true, + "allowJs": false, // Bundler mode "moduleResolution": "bundler", "verbatimModuleSyntax": true, "declaration": true,packages/plugins/onboarding/src/types.ts (1)
5-10: LGTM: ActionEndpointContext shape is clear and practicalThe body retyping via Omit<...,"body"> & { body: Schema } is a clean way to strongly type request payloads while keeping the rest of the context intact.
packages/plugins/onboarding/src/routes/should-onboard.ts (2)
5-16: LGTM: Authenticated boolean endpoint is minimal, consistent, and properly guarded.The endpoint correctly requires a session, defers authorization to
verifyOnboarding, and returns a boolean as exercised by tests. No blocking issues here.
9-10: KeepsessionMiddlewarehere for consistency and proper context enrichment.Although
verifyOnboarding(ctx)callsgetSessionFromCtx(ctx)to retrieve the session, that helper only reads the existing session (or decodes the token as a fallback) and does not populatectx.context.sessionfor downstream use. All other onboarding routes applysessionMiddlewarefirst and then callverifyOnboarding, ensuring:
ctx.context.sessionis set once by the middlewaregetSessionFromCtxcan read fromctx.contextand skip redundant decoding- Any future logic or plugins that rely on
ctx.context.session(beyond just verification) will continue to workRemoving
sessionMiddlewarehere would still allowverifyOnboardingto succeed, but it breaks the established pattern across the plugin and risks missing context in later handlers. Given that consistency and avoiding hidden side-effects are paramount, no change is needed.packages/plugins/onboarding/src/verify-onboarding.ts (2)
8-11: Good guard: 401 on missing session.Clear and consistent use of
APIError("UNAUTHORIZED"). Matches tests that assert 401 on missing session.
12-16: Good policy: 403 when already onboarded with a specific code.Returning
FORBIDDENwithALREADY_ONBOARDEDis precise and aligned with the test expectations. Nicely done.packages/plugins/onboarding/package.json (1)
1-51: Action Required: Manually verify compiled module format and side-effect settingsThe
dist/directory isn’t checked in, so we can’t inspect whether the built output is ESM or CJS. Before publishing, please:
- Build the plugin (e.g.
cd packages/plugins/onboarding && bun build.ts)- Inspect
dist/index.jsanddist/client.jsforexportvs.module.exports- If output is ESM, add
"type": "module"topackage.json- If there are truly no module-level side effects, add
"sideEffects": falsefor better tree-shakingSuggested optional updates to packages/plugins/onboarding/package.json:
{ "name": "@better-auth-kit/onboarding", "version": "0.1.0", + // If your dist is ESM, uncomment the next line. + // "type": "module", "description": "Easily add user onboarding to your authentication flow.", @@ "exports": { ".": { "types": "./dist/index.d.ts", "default": "./dist/index.js" }, "./client": { "types": "./dist/client.d.ts", "default": "./dist/client.js" } }, + // Add if there are no module-level side effects. + // "sideEffects": false, "publishConfig": { "access": "public" } }packages/plugins/onboarding/src/client.ts (2)
32-37: Confirm atom listener path contract.
matcher: (path) => path.startsWith("/onboarding/")assumes the listener receives a path relative tobaseURL. If the framework delivers full paths (e.g., including/api/auth), this won't fire. Verify and adjust to include the base path if needed.Would you like me to update this to normalize against
baseURLsimilarly to the fetch hook matcher?
31-31: TypeScript version check passed—instantiation expressions supportedThe project’s TypeScript dependency is v5.7.x (≥ 4.7), which fully supports instantiation expressions like
onboarding<…>inReturnType<typeof onboarding<…>>. No changes are needed.packages/plugins/onboarding/src/index.ts (1)
150-166: after hook response behavior—verify that overriding body is intended.The middleware returns
ctx.json({ onboardingRedirect: true })on/get-session. Ensure BetterAuth merges this into the original response (rather than replacing session payload) so consumers ofget-sessionaren’t broken.If it replaces the response, consider attaching the flag in a response extension field while preserving the original body.
packages/plugins/onboarding/src/internal-types.ts (1)
27-52: Key generation and path transforms look solid.
EndpointPairprecisely captures the two endpoints per step, tying the path templates toTransformClientPathand key names toTransformPath. This gives strong inference for consumers.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/plugins/onboarding/tests/onboarding.test.ts (2)
9-27: Fix: avoid async describe; move async setup into beforeAll.Vitest does not await asynchronous describe callbacks reliably. Initialize the test instance inside a synchronous suite with a beforeAll.
-describe("(success)", async () => { - const auth = getAuth(); - const { resetDatabase, client, signUpWithTestUser, testUser, db } = - await getTestInstance(auth, { - clientOptions: { - plugins: [ - onboardingClient({ - onOnboardingRedirect: mockOnboardingRedirect, - }), - ], - }, - }); - - let headers: Headers; - beforeAll(async () => { - await resetDatabase(); - const result = await signUpWithTestUser(); - headers = result.headers; - }); +describe("(success)", () => { + let headers: Headers; + let resetDatabase: any; + let client: any; + let signUpWithTestUser: any; + let testUser: any; + let db: any; + + beforeAll(async () => { + const auth = getAuth(); + ({ resetDatabase, client, signUpWithTestUser, testUser, db } = + await getTestInstance(auth, { + clientOptions: { + plugins: [ + onboardingClient({ + onOnboardingRedirect: mockOnboardingRedirect, + }), + ], + }, + })); + await resetDatabase(); + const result = await signUpWithTestUser(); + headers = result.headers; + });
138-154: Fix: same async describe issue in “auto enable on sign-up” suite.Make the suite synchronous; initialize with beforeAll.
-describe("(auto enable on sign-up)", async () => { - const { resetDatabase, signUpWithTestUser } = await getTestInstance( - getAuth(), - { - clientOptions: { - plugins: [ - onboardingClient({ - onOnboardingRedirect: mockOnboardingRedirect, - }), - ], - }, - }, - ); +describe("(auto enable on sign-up)", () => { + let resetDatabase: any; + let signUpWithTestUser: any; + beforeAll(async () => { + ({ resetDatabase, signUpWithTestUser } = await getTestInstance(getAuth(), { + clientOptions: { + plugins: [ + onboardingClient({ + onOnboardingRedirect: mockOnboardingRedirect, + }), + ], + }, + })); + }); beforeEach(async () => { await resetDatabase(); });
🧹 Nitpick comments (8)
packages/plugins/onboarding/tests/onboarding.test.ts (4)
162-177: Optional: avoid heavy getTestInstance calls inside the test body.Constructing a second instance within it can slow tests and complicate cleanup. Prefer a nested describe with its own beforeAll/beforeEach.
- it("should not trigger redirect during sign-up when autoEnableOnSignUp is false", async () => { - mockOnboardingRedirect.mockClear(); - const { resetDatabase, signUpWithTestUser } = await getTestInstance( - getAuth({ - autoEnableOnSignUp: false, - }), - { - clientOptions: { - plugins: [ - onboardingClient({ - onOnboardingRedirect: mockOnboardingRedirect, - }), - ], - }, - }, - ); - await signUpWithTestUser(); - expect(mockOnboardingRedirect).not.toHaveBeenCalled(); - }); + describe("when autoEnableOnSignUp is false", () => { + let resetDatabase2: any; + let signUpWithTestUser2: any; + beforeAll(async () => { + ({ resetDatabase: resetDatabase2, signUpWithTestUser: signUpWithTestUser2 } = + await getTestInstance( + getAuth({ autoEnableOnSignUp: false }), + { + clientOptions: { + plugins: [ + onboardingClient({ + onOnboardingRedirect: mockOnboardingRedirect, + }), + ], + }, + }, + )); + }); + beforeEach(async () => { + await resetDatabase2(); + mockOnboardingRedirect.mockClear(); + }); + it("should not trigger redirect during sign-up", async () => { + await signUpWithTestUser2(); + expect(mockOnboardingRedirect).not.toHaveBeenCalled(); + }); + });
7-8: Auto-reset the onboarding redirect mock in a hook.Simplifies individual tests and avoids order dependency. You can remove explicit mockClear calls inside tests.
const mockOnboardingRedirect = vi.fn(); describe("Onboarding", () => { + beforeEach(() => { + mockOnboardingRedirect.mockReset(); + });
66-76: Consistent input: pass the same payload when step expects input.Else this can become brittle if the step’s input schema becomes required.
it("should complete onboarding step successfully and return true", async () => { - const res = await (client.onboarding as any).step.newPassword({ - foo: "bar", + const res = await (client.onboarding as any).step.newPassword({ + foo: "bar", fetchOptions: { headers, }, });Follow-up: If newPassword’s input is optional, ignore. If required, also update line 80 to pass the same payload.
67-69: Reduce any usage: surface typed onboarding endpoints on the client.Casting to any hides mistakes. Consider exposing typed step endpoints from getTestInstance or narrowing here via a helper type so client.onboarding.step.newPassword is typed.
If helpful, I can wire a minimal helper type for the tests that augments the generated client with the onboarding step signatures inferred from getAuth().
Also applies to: 78-81, 95-96, 114-116, 232-248
packages/plugins/onboarding/src/utils.ts (2)
3-15: Nit: trim unused parameter in transformPath’s map callback.index is unused; drop it to reduce noise.
- .map((segment, index) => { + .map((segment) => { if (segment.length === 0) return ""; // handle leading separators return segment.charAt(0).toUpperCase() + segment.slice(1); })
42-60: Type/runtime parity tests recommended.Given the subtlety of the transforms, add tests asserting transformClientPath and TransformClientPath agree for cases like "NewPassword", "APIKey", "/onboarding/step/newPassword", and "-Foo/Bar".
I can add a small vitest suite and a .d.ts type assertion file to lock this down.
packages/plugins/onboarding/src/internal-types.ts (2)
24-26: Minor: unused generic parameter.K isn’t referenced in CanAccessOnboardingStepReturnType; simplify to boolean without the generic.
-export type CanAccessOnboardingStepReturnType<K extends OnboardingStep = any> = - boolean; +export type CanAccessOnboardingStepReturnType = boolean;
6-11: Optional: model body absence with never instead of undefined.If a step has no input schema, representing body as undefined may be less precise than never (i.e., “no body accepted”). Only apply if createAuthEndpoint omits or rejects body in that case.
-type InferStepInput<K extends OnboardingStep> = K extends { input?: infer I } - ? I extends ZodSchema<any> - ? I - : undefined - : undefined; +type InferStepInput<K extends OnboardingStep> = K extends { input?: infer I } + ? I extends ZodSchema<any> + ? I + : never + : never;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
packages/plugins/onboarding/src/client.ts(1 hunks)packages/plugins/onboarding/src/error-codes.ts(1 hunks)packages/plugins/onboarding/src/index.ts(1 hunks)packages/plugins/onboarding/src/internal-types.ts(1 hunks)packages/plugins/onboarding/src/schema.ts(1 hunks)packages/plugins/onboarding/src/types.ts(1 hunks)packages/plugins/onboarding/src/utils.ts(1 hunks)packages/plugins/onboarding/tests/onboarding.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/plugins/onboarding/src/error-codes.ts
- packages/plugins/onboarding/src/types.ts
- packages/plugins/onboarding/src/client.ts
- packages/plugins/onboarding/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/plugins/onboarding/tests/onboarding.test.ts (4)
packages/plugins/onboarding/tests/auth.ts (1)
getAuth(17-40)packages/libraries/tests/src/index.ts (1)
getTestInstance(25-205)packages/plugins/onboarding/src/client.ts (1)
onboardingClient(15-70)packages/plugins/onboarding/src/error-codes.ts (1)
ONBOARDING_ERROR_CODES(1-6)
packages/plugins/onboarding/src/internal-types.ts (2)
packages/plugins/onboarding/src/types.ts (1)
OnboardingStep(33-57)packages/plugins/onboarding/src/utils.ts (2)
TransformClientPath(60-60)TransformPath(21-26)
🔇 Additional comments (1)
packages/plugins/onboarding/src/schema.ts (1)
3-17: LGTM: minimal, well-scoped schema.Shape matches the plugin’s needs; completedSteps is correctly marked as input: false.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/plugins/onboarding/src/types.ts (1)
11-13: Tighten generics and avoid defaulting Steps to any; restrict completionStep to string keys.
- Using
anyfor theStepsdefault loses all safety. Default to a typed record instead.completionStepis ultimately a string identifier in routes; intersect with string to prevent symbol/number keys.export type OnboardingOptions< - Steps extends Record<string, OnboardingStep> = any, + Steps extends Record<string, OnboardingStep<any, any>> = + Record<string, OnboardingStep<any, any>>, > = { @@ - completionStep: keyof Steps; + completionStep: Extract<keyof Steps, string>;Also applies to: 21-21
🧹 Nitpick comments (3)
packages/plugins/onboarding/src/types.ts (3)
1-3: Prefer ZodType over ZodSchema (clearer and future-proof with zod v3).Replace the alias import with ZodType; pair this with the
inputtype change below.-import type { ZodSchema } from "zod"; +import type { ZodType } from "zod";
5-9: Export ActionEndpointContext for consumers.Library users likely want to type their handlers directly; exporting this improves DX without widening surface area.
-type ActionEndpointContext<Schema = unknown, Result = unknown> = ( +export type ActionEndpointContext<Schema = unknown, Result = unknown> = ( ctx: Omit<GenericEndpointContext, "body"> & { body: Schema; }, ) => Result | Promise<Result>;
35-59: Loosen Schema bound and switch to ZodType for input.
- The current
Schema extends Record<string, any> | undefined | nullexcludes valid non-object inputs and complicates defaults. Usingunknownis safer and more general.- Pair the change with
ZodType<Schema>and rely on each step’s schema to shape/validate as needed.-export type OnboardingStep< - Schema extends Record<string, any> | undefined | null = any, - Result = unknown, -> = { +export type OnboardingStep<Schema = unknown, Result = unknown> = { @@ - input?: ZodSchema<Schema>; + input?: ZodType<Schema>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/plugins/onboarding/src/index.ts(1 hunks)packages/plugins/onboarding/src/types.ts(1 hunks)packages/plugins/onboarding/tests/onboarding.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/plugins/onboarding/src/index.ts
- packages/plugins/onboarding/tests/onboarding.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/plugins/onboarding/src/types.ts (1)
packages/plugins/onboarding/src/schema.ts (1)
schema(3-17)
🔇 Additional comments (2)
packages/plugins/onboarding/src/types.ts (2)
1-60: Overall: strong, cohesive type surface.The API reads well, docs are clear, and the default semantics for auto-enable are consistent. With the optional tweaks above, you’ll tighten type-safety and improve consumer ergonomics.
23-29: ✅ VerifiedautoEnableOnSignUpdefault and public type exportThe runtime default for
autoEnableOnSignUpis set totrueinpackages/plugins/onboarding/src/index.ts(line 27), matching the@default trueannotation intypes.ts. The types are publicly re-exported viaexport * from "./types";inindex.ts(line 252). No further changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/plugins/onboarding/src/types.ts (1)
15-17: Tighten generics for OnboardingOptions to avoid any-leakage; doc default now correctPrevious feedback asked to constrain
Stepsto anOnboardingStepmap and fix the default doc. The JSDoc is now correct (@default true), but the generic still leaksany. Constrain it to a safe default.-export type OnboardingOptions< - Steps extends Record<string, OnboardingStep> = any, -> = { +export type OnboardingOptions< + Steps extends Record<string, OnboardingStep<any, any>> = + Record<string, OnboardingStep<unknown, unknown>>, +> = {
🧹 Nitpick comments (10)
packages/plugins/onboarding/src/presets/index.ts (1)
1-2: Barrel looks good; consider explicit re-exports to avoid accidental API surface expansion.Using export * can unintentionally expose new symbols added later. Prefer named re-exports for stronger API control and slightly better tree-shaking.
-export * from "./setup-new-password"; -export * from "./setup-2fa"; +export { setupNewPasswordStep, type SetupNewPasswordStepOptions } from "./setup-new-password"; +export { setup2FAStep, type Setup2FAOptions } from "./setup-2fa";packages/plugins/onboarding/tests/presets/setup-2fa.test.ts (2)
98-98: Fix suite name to match file and subject.The suite title says "setup-new-password preset" in a setup-2fa test file.
-describe("setup-new-password preset", async () => { +describe("setup-2fa preset", async () => {
9-97: Large commented-out test block — remove or restore as active tests.Dead commented code adds noise and can drift. Either delete it or re-enable with correct expectations.
I can help refactor these into focused tests (e.g., behavior when the two-factor plugin is absent).
packages/plugins/onboarding/tests/presets/setup-new-password.test.ts (2)
58-67: Strengthen success-case by verifying sign-in with the new password.This confirms the adapter actually persisted the change, not just returned success.
Diffs to (1) expose signInWithUser and (2) assert sign-in succeeds:
-const { resetDatabase, client, signUpWithTestUser, db, testUser } = +const { resetDatabase, client, signUpWithTestUser, db, testUser, signInWithUser } = await getTestInstance(auth, {it("should successfully update password when passwords match", async () => { const res = await (client.onboarding as any).step.newPassword({ newPassword: "newpassword123", confirmPassword: "newpassword123", fetchOptions: { headers }, }); if (res.error) throw res.error; expect(res.data.data.success).toBe(true); expect(res.data.completedSteps).toEqual(["newPassword"]); + // Verify the password actually works for sign-in + const { res: signInRes } = await signInWithUser(testUser.email, "newpassword123"); + expect(signInRes.session).toBeDefined(); });
83-97: Assert the exact error message/code for once-only enforcement.Consider asserting the canonical message to catch regressions in error wording.
If you adopt this, add the import:
import { ONBOARDING_ERROR_CODES } from "../../src/error-codes";And adjust:
- expect(res.error?.message).toBeDefined(); + expect(res.error?.message).toBe(ONBOARDING_ERROR_CODES.STEP_ALREADY_COMPLETED);packages/plugins/onboarding/src/presets/setup-2fa.ts (1)
15-17: Optional: enforce a minimum password length in the input schema.nonempty() permits 1-char passwords. If your ecosystem assumes a baseline (e.g., 8), set it here for consistency with the password preset.
- password: z.string().nonempty(), + password: z.string().min(8, "Password must be at least 8 characters"),packages/plugins/onboarding/tests/auth.ts (2)
16-21: Preserve step/completionStep type-safety by parameterizing getAuth with StepsTyping
optionsasPartial<OnboardingOptions>erases the relationship betweenstepsandcompletionStep. ParameterizinggetAuthkeeps compile‑time checks thatcompletionStepbelongs to the providedsteps.-export const getAuth = ( - options?: Partial<OnboardingOptions>, - authOptions?: { - plugins?: BetterAuthPlugin[]; - }, -) => { +export const getAuth = <Steps extends Record<string, OnboardingStep<any, any>>>( + options?: Partial<OnboardingOptions<Steps>>, + authOptions?: { + plugins?: BetterAuthPlugin[]; + }, +) => {Additionally, add this import at the top of the file:
import type { OnboardingStep } from "../src";
10-14: Tighten the test input schema to reject stray keysYou already constrain the payload shape; calling
.strict()on the object prevents unintentional extra keys from slipping into the handler during tests.-const onboardingSchema = z - .object({ - foo: z.string().optional(), - }) - .nullish(); +const onboardingSchema = z + .object({ + foo: z.string().optional(), + }) + .strict() + .nullish();packages/plugins/onboarding/src/types.ts (2)
39-42: Prefer unknown over any and set a safer default for SchemaDefaulting
Schematoanyweakens type safety for handlers. Useunknownin the record and set the default to the same constrained union.-export type OnboardingStep< - Schema extends Record<string, any> | undefined | null = any, - Result = unknown, -> = { +export type OnboardingStep< + Schema extends Record<string, unknown> | undefined | null = + Record<string, unknown> | undefined | null, + Result = unknown, +> = {
64-75: Polish JSDoc phrasing for clarityMinor copy edits to improve readability of developer‑facing docs.
- /** - * If true headers will be required to be passed in the context - */ + /** + * If true, request headers are required on the context. + */ requireHeaders?: boolean; - /** - * If true request object will be required - */ + /** + * If true, a Request object must be present on the context. + */ requireRequest?: boolean; - /** - * Clone the request object from the router - */ + /** + * If true, clone the Request object from the router before passing it to handlers. + */ cloneRequest?: boolean;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
packages/plugins/onboarding/build.ts(1 hunks)packages/plugins/onboarding/package.json(1 hunks)packages/plugins/onboarding/src/index.ts(1 hunks)packages/plugins/onboarding/src/presets/index.ts(1 hunks)packages/plugins/onboarding/src/presets/setup-2fa.ts(1 hunks)packages/plugins/onboarding/src/presets/setup-new-password.ts(1 hunks)packages/plugins/onboarding/src/types.ts(1 hunks)packages/plugins/onboarding/src/utils.ts(1 hunks)packages/plugins/onboarding/tests/auth.ts(1 hunks)packages/plugins/onboarding/tests/onboarding.test.ts(1 hunks)packages/plugins/onboarding/tests/presets/setup-2fa.test.ts(1 hunks)packages/plugins/onboarding/tests/presets/setup-new-password.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/plugins/onboarding/build.ts
- packages/plugins/onboarding/package.json
- packages/plugins/onboarding/src/index.ts
- packages/plugins/onboarding/src/utils.ts
- packages/plugins/onboarding/tests/onboarding.test.ts
🧰 Additional context used
🧬 Code graph analysis (6)
packages/plugins/onboarding/tests/presets/setup-new-password.test.ts (5)
packages/plugins/onboarding/tests/auth.ts (1)
getAuth(16-45)packages/plugins/onboarding/src/presets/setup-new-password.ts (1)
setupNewPasswordStep(23-60)packages/libraries/tests/src/index.ts (1)
getTestInstance(25-205)packages/plugins/onboarding/src/client.ts (1)
onboardingClient(15-70)packages/plugins/onboarding/src/routes/should-onboard.ts (1)
shouldOnboard(5-16)
packages/plugins/onboarding/src/presets/setup-new-password.ts (1)
packages/plugins/onboarding/src/index.ts (1)
createOnboardingStep(242-253)
packages/plugins/onboarding/src/presets/setup-2fa.ts (1)
packages/plugins/onboarding/src/index.ts (1)
createOnboardingStep(242-253)
packages/plugins/onboarding/tests/presets/setup-2fa.test.ts (5)
packages/plugins/onboarding/tests/auth.ts (1)
getAuth(16-45)packages/plugins/onboarding/src/presets/setup-2fa.ts (1)
setup2FAStep(12-39)packages/libraries/tests/src/index.ts (1)
getTestInstance(25-205)packages/plugins/onboarding/src/client.ts (1)
onboardingClient(15-70)packages/plugins/onboarding/src/error-codes.ts (1)
ONBOARDING_ERROR_CODES(1-6)
packages/plugins/onboarding/src/types.ts (1)
packages/plugins/onboarding/src/schema.ts (1)
schema(3-17)
packages/plugins/onboarding/tests/auth.ts (2)
packages/plugins/onboarding/src/types.ts (1)
OnboardingOptions(15-37)packages/plugins/onboarding/src/index.ts (2)
onboarding(23-240)createOnboardingStep(242-253)
🔇 Additional comments (5)
packages/plugins/onboarding/src/presets/setup-new-password.ts (1)
45-51: All onboarding step routes enforce sessionMiddleware – non-null assertion is safeVerification shows every onboarding endpoint is guarded by sessionMiddleware via createAuthEndpoint:
- In packages/plugins/onboarding/src/internal-types.ts (use: [sessionMiddleware] at lines 34 and 44)
- In packages/plugins/onboarding/src/index.ts (use: [sessionMiddleware] at lines 46 and 123)
Since every step route—including setup-new-password—is protected, ctx.context.session! cannot be undefined here. No changes required.
packages/plugins/onboarding/tests/presets/setup-2fa.test.ts (1)
164-178: Good coverage of the once-only constraint.Verifies idempotency and correct error code/message on re-run.
packages/plugins/onboarding/tests/presets/setup-new-password.test.ts (1)
48-56: Validation behavior is well-covered.Mismatch rejection and 400 status expectation look correct.
packages/plugins/onboarding/tests/auth.ts (2)
28-41: Confirm plugin ordering is intentional (onboarding first)Onboarding registers “after” hooks for sign‑up and session endpoints. If any test injects plugins that also hook these endpoints, order may affect behavior. Confirm that appending
authOptions?.pluginsafter onboarding is what you want for the tests; otherwise consider allowing callers to choose prepend/append.
22-27: LGTM: Simple, fast test harnessUsing an in‑memory SQLite database and enabling email/password keeps tests self‑contained and fast. Nice.
Also applies to: 44-45
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
packages/plugins/onboarding/src/client.ts (1)
60-74: URL parsing and strict path matching look good; this addresses the prior SSR/relative URL concernThe
toPathhelper and explicit checks for/onboarding/step/and/onboarding/skip-step/with trailing slashes resolve the earlier over-match and SSR issues.packages/plugins/onboarding/src/types.ts (1)
15-18: Tighten generics: preferunknownoveranyfor type safetyUsing
anyweakens static checking for step inputs/results.unknownpreserves safety while remaining ergonomic.export type OnboardingOptions< - Steps extends Record<string, OnboardingStep<any, any, any>>, + Steps extends Record<string, OnboardingStep<unknown, unknown, any>>, CompletionStep extends keyof Steps, > = {packages/plugins/onboarding/src/internal-types.ts (1)
1-1: Type-only import bug:typeofrequires value-side symbols — switch to module type queries to stay type-only and avoid bundlingUsing
import type { createAuthEndpoint, sessionMiddleware }and thentypeof createAuthEndpoint/typeof sessionMiddlewarewill error (“cannot be used as a value because it was imported using ‘import type’”). Use a module type query alias and index into it so no runtime import is emitted.Apply this diff:
-import type { createAuthEndpoint, sessionMiddleware } from "better-auth/api"; +// Keep this file type-only: derive types via a module type query +type BA = typeof import("better-auth/api"); @@ - onboardingStep: ReturnType< - typeof createAuthEndpoint< + onboardingStep: ReturnType< + BA["createAuthEndpoint"]< `/onboarding/step/${TransformClientPath<Path>}`, { method: "POST"; - body: ZodType<InferStepInput<K>>; - use: [typeof sessionMiddleware]; + body: ZodType<InferStepInput<K>>; + use: [BA["sessionMiddleware"]]; }, OnboardingStepReturnType<K> > >; canAccessOnboardingStep: ReturnType< - typeof createAuthEndpoint< + BA["createAuthEndpoint"]< `/onboarding/can-access-step/${TransformClientPath<Path>}`, { method: "GET"; - use: [typeof sessionMiddleware]; + use: [BA["sessionMiddleware"]]; metadata: { SERVER_ONLY: true; }; }, CanAccessOnboardingStepReturnType > >; @@ -type SkipOnboardingStepEndpoint<Path extends string> = ReturnType< - typeof createAuthEndpoint< +type SkipOnboardingStepEndpoint<Path extends string> = ReturnType< + BA["createAuthEndpoint"]< `/onboarding/skip-step/${TransformClientPath<Path>}`, { method: "POST"; - use: [typeof sessionMiddleware]; + use: [BA["sessionMiddleware"]]; }, SkipOnboardingStepReturnType > >;This keeps the file purely type-level and eliminates the compile error without introducing a runtime dependency.
Also applies to: 30-39, 41-53, 88-97
🧹 Nitpick comments (17)
apps/docs/content/docs/plugins/onboarding.mdx (3)
260-264: Correct the profile update example: don’t assignlastNametoname.Unless the user model expects a full-name
namefield, assigningname: lastNameis misleading. Prefer a full name or a dedicatedlastNamefield.Apply one of these diffs (pick based on your user schema):
Option A (full name):
- await ctx.context.internalAdapter.updateUser(ctx.context.session!.user.id, { - firstName, - name: lastName, - }); + await ctx.context.internalAdapter.updateUser(ctx.context.session!.user.id, { + firstName, + name: `${firstName} ${lastName}`, + });Option B (separate field):
- await ctx.context.internalAdapter.updateUser(ctx.context.session!.user.id, { - firstName, - name: lastName, - }); + await ctx.context.internalAdapter.updateUser(ctx.context.session!.user.id, { + firstName, + lastName, + });If the user model only defines a single
namefield, Option A is preferable. Confirm which schema your templates and adapters expect.
190-193: Optional: Add a client variant for step access checks.A small client example improves parity with the server snippet and helps frontend integrators.
Suggested addition (below the server example):
const { data: canAccess } = await authClient.onboarding.canAccess.preferences();
392-399: Clarify secondary storage prerequisites.Mention that secondary storage must be provided on
ctx.context.secondaryStorage(get/set) for the plugin to use it; otherwise it falls back to DB.Suggested wording:
- “Enable this only if you have configured a compatible secondary storage provider on the server context (implements async get/set). When not available, the plugin uses the database.”
packages/plugins/onboarding/src/verify-onboarding.ts (4)
22-25: Use nullish coalescing for brevity and clarity.The ternary can be simplified without changing behavior.
Apply this diff:
- const adapter = context.adapter - ? context.adapter - : getOnboardingAdapter(context.options, ctx); + const adapter = context.adapter ?? getOnboardingAdapter(context.options, ctx);
7-13: Add an explicit return type forverifyOnboardingfor API stability.Helps callers and prevents accidental return-shape drift during refactors.
Apply this diff:
-export async function verifyOnboarding( +export async function verifyOnboarding( ctx: GenericEndpointContext, context: { adapter?: ReturnType<typeof getOnboardingAdapter>; options: OnboardingOptions<any, any>; }, -) { +): Promise<VerifyOnboardingResult> {And add this type alias near the top of the file (or in a
types.tsif you prefer):export type VerifyOnboardingResult = { session: NonNullable<Awaited<ReturnType<typeof getSessionFromCtx>>>; key: string; };
27-31: Consider exposing an error code alongside the message.If your error handling middleware keys off machine-readable codes, include one here (e.g.,
code: "ALREADY_ONBOARDED"). If not used in this project, ignore.Example:
- throw new APIError("FORBIDDEN", { - message: ONBOARDING_ERROR_CODES.ALREADY_ONBOARDED, - }); + throw new APIError("FORBIDDEN", { + message: ONBOARDING_ERROR_CODES.ALREADY_ONBOARDED, + code: "ALREADY_ONBOARDED", + });
20-26: Optional: Ensure boolean semantics.If
session.user.shouldOnboardcan beundefined | null, normalize to a boolean to avoid surprises.-let shouldOnboard = session.user.shouldOnboard; +let shouldOnboard = Boolean(session.user.shouldOnboard);packages/plugins/onboarding/src/adapter.ts (6)
1-1: Unify type import path forGenericEndpointContext.Elsewhere (
verify-onboarding.ts) the type comes frombetter-auth/types. Aligning import paths avoids confusion.Apply this diff:
-import type { GenericEndpointContext } from "better-auth"; +import type { GenericEndpointContext } from "better-auth/types";
12-17: Defensive normalization ofcompletedSteps.Secondary storage and DB reads may yield unexpected shapes. Normalize to an array before constructing the Set.
Apply this diff:
- if (options.secondaryStorage && ctx.context.secondaryStorage) { - completedSteps = - JSON.parse( - (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? - "{}", - ).completedSteps ?? []; - } else { - completedSteps = JSON.parse( - ( - await ctx.context.adapter.findOne<{ - completedSteps?: string; - }>({ - model: "user", - where: [ - { - field: "id", - value: userId, - }, - ], - select: ["completedSteps"], - }) - )?.completedSteps ?? "[]", - ); - } + if (options.secondaryStorage && ctx.context.secondaryStorage) { + const raw = JSON.parse( + (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? + "{}", + ).completedSteps; + completedSteps = Array.isArray(raw) ? raw : []; + } else { + const raw = ( + await ctx.context.adapter.findOne<{ completedSteps?: string }>({ + model: "user", + where: [{ field: "id", value: userId }], + select: ["completedSteps"], + }) + )?.completedSteps; + const parsed = raw ? JSON.parse(raw) : []; + completedSteps = Array.isArray(parsed) ? parsed : []; + }Also applies to: 18-36
45-59: Avoid repeating the storage key and prevent persistingnullvalues.Minor cleanup: build the key once and strip
nullfields so state stays boolean/array orundefined.Apply this diff:
- if (options.secondaryStorage && ctx.context.secondaryStorage) { - const currentState = JSON.parse( - (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? - "{}", - ); + if (options.secondaryStorage && ctx.context.secondaryStorage) { + const key = `onboarding:${userId}`; + const currentState = JSON.parse((await ctx.context.secondaryStorage.get(key)) ?? "{}"); const baseState = { shouldOnboard: false, completedSteps: [], }; - await ctx.context.secondaryStorage.set(`onboarding:${userId}`, { - ...baseState, - ...currentState, - ...data, - }); + const sanitized = { + ...baseState, + ...currentState, + ...data, + }; + if (sanitized.shouldOnboard === null) delete (sanitized as any).shouldOnboard; + if (sanitized.completedSteps === null) delete (sanitized as any).completedSteps; + await ctx.context.secondaryStorage.set(key, sanitized); } else {
69-76: NormalizeshouldOnboardto boolean on read.Ensures callers always receive a boolean even if storage contains an unexpected value.
- return ( - JSON.parse( - (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? - "{}", - ).shouldOnboard ?? false - ); + const raw = JSON.parse( + (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? "{}", + ).shouldOnboard; + return Boolean(raw);
78-91: DB path: normalize boolean read as well.Consistency across backends.
- return ( - ( - await ctx.context.adapter.findOne<{ shouldOnboard?: boolean }>({ - model: "user", - where: [ - { - field: "id", - value: userId, - }, - ], - select: ["shouldOnboard"], - }) - )?.shouldOnboard ?? false - ); + const row = await ctx.context.adapter.findOne<{ shouldOnboard?: boolean }>({ + model: "user", + where: [{ field: "id", value: userId }], + select: ["shouldOnboard"], + }); + return Boolean(row?.shouldOnboard);
9-37: Optional: Extract a tiny helper to DRY JSON parsing.Not required, but a small
safeParse<T>(json, fallback)helper would cut repetition.If you’d like, I can draft this refactor in a follow-up.
packages/plugins/onboarding/src/client.ts (2)
19-24: Relax the generic constraint to support both plugin types and raw step mapsYour
InferSteps<T>already supports two shapes (plugin type or raw steps), but the generic parameter is currently constrained to only the plugin shape with$Infer. If you intendonboardingClientto accept either, update the constraint accordingly; otherwise, removeInferStepsto reduce indirection.Example change (if you want to accept both):
-export const onboardingClient = < - Steps extends { - $Infer: { - OnboardingSteps: Record<string, OnboardingStep<any, any, any>>; - OnboardingCompletionStep: string; - }; - }, ->(options?: { +export const onboardingClient = < + Steps extends + | { $Infer: { OnboardingSteps: Record<string, OnboardingStep<any, any, any>>; OnboardingCompletionStep: unknown } } + | Record<string, OnboardingStep<any, any, any>> +>( + options?: { ...If you intend to only accept server plugin types, drop
InferStepsentirely and referenceSteps["$Infer"]["OnboardingSteps"]directly throughout for simplicity.
52-58: Protect fetch layer from redirect callback failuresIf
onOnboardingRedirectthrows, it will poison the fetch pipeline. Consider catching and logging (or ignoring) to avoid breaking the original request flow.async onSuccess(context) { if (context.data?.onboardingRedirect) { - if (options?.onOnboardingRedirect) { - await options.onOnboardingRedirect(); - } + if (options?.onOnboardingRedirect) { + try { + await options.onOnboardingRedirect(); + } catch { + // swallow to avoid impacting the original fetch lifecycle + } + } } },packages/plugins/onboarding/src/types.ts (1)
6-6: Align Zod type usage with the rest of the codebase (ZodTypevsZodSchema)Elsewhere you use
ZodType(e.g., in internal-types). Consider usingZodTypehere for consistency to avoid confusion; both resolve to the same underlying type.-import type { ZodSchema } from "zod"; +import type { ZodType } from "zod"; @@ - input?: ZodSchema<Schema>; + input?: ZodType<Schema>;Also applies to: 55-55
packages/plugins/onboarding/src/internal-types.ts (1)
3-3: Remove unused zod symbols
zandZodSchemaare not used in this file.-import type { z, ZodSchema, ZodType } from "zod"; +import type { ZodType } from "zod";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/docs/content/docs/offer.mdx(1 hunks)apps/docs/content/docs/plugins/onboarding.mdx(1 hunks)apps/docs/src/components/sidebar-content.tsx(2 hunks)packages/plugins/onboarding/src/adapter.ts(1 hunks)packages/plugins/onboarding/src/client.ts(1 hunks)packages/plugins/onboarding/src/index.ts(1 hunks)packages/plugins/onboarding/src/internal-types.ts(1 hunks)packages/plugins/onboarding/src/presets/setup-2fa.ts(1 hunks)packages/plugins/onboarding/src/types.ts(1 hunks)packages/plugins/onboarding/src/verify-onboarding.ts(1 hunks)packages/plugins/onboarding/tests/auth.ts(1 hunks)packages/plugins/onboarding/tests/onboarding.test.ts(1 hunks)packages/plugins/onboarding/tests/presets/setup-2fa.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/plugins/onboarding/src/presets/setup-2fa.ts
- packages/plugins/onboarding/tests/auth.ts
- packages/plugins/onboarding/tests/presets/setup-2fa.test.ts
- packages/plugins/onboarding/tests/onboarding.test.ts
- packages/plugins/onboarding/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (5)
packages/plugins/onboarding/src/adapter.ts (1)
packages/plugins/onboarding/src/types.ts (1)
OnboardingOptions(15-44)
packages/plugins/onboarding/src/client.ts (3)
packages/plugins/onboarding/src/types.ts (1)
OnboardingStep(46-84)packages/plugins/onboarding/src/index.ts (1)
onboarding(25-274)packages/plugins/onboarding/src/utils.ts (1)
toPath(63-70)
packages/plugins/onboarding/src/verify-onboarding.ts (3)
packages/plugins/onboarding/src/adapter.ts (1)
getOnboardingAdapter(4-94)packages/plugins/onboarding/src/types.ts (1)
OnboardingOptions(15-44)packages/plugins/onboarding/src/error-codes.ts (1)
ONBOARDING_ERROR_CODES(1-6)
packages/plugins/onboarding/src/internal-types.ts (2)
packages/plugins/onboarding/src/types.ts (1)
OnboardingStep(46-84)packages/plugins/onboarding/src/utils.ts (2)
TransformClientPath(61-61)TransformPath(21-26)
packages/plugins/onboarding/src/types.ts (1)
packages/plugins/onboarding/src/schema.ts (1)
schema(3-17)
🪛 LanguageTool
apps/docs/content/docs/plugins/onboarding.mdx
[grammar] ~14-~14: There might be a mistake here.
Context: ...nboarding flows** with custom validation - Automatic completion tracking per user...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...Automatic completion tracking** per user - Required step enforcement before marki...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...ent** before marking onboarding complete - One-time step protection to prevent du...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...ction** to prevent duplicate completions - Built-in presets for common onboarding...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...resets** for common onboarding scenarios - Client-side integration with automatic...
(QB_NEW_EN)
[grammar] ~239-~239: There might be a mistake here.
Context: ...- Zod schema for request body validation - handler: `(ctx: GenericEndpointCont...
(QB_NEW_EN)
[grammar] ~240-~240: There might be a mistake here.
Context: ...e- Function that processes the step - **once**:boolean- Iftrue`, step can ...
(QB_NEW_EN)
[grammar] ~241-~241: There might be a mistake here.
Context: ...only be completed once (default: true) - required: boolean - If true, step ...
(QB_NEW_EN)
[grammar] ~242-~242: There might be a mistake here.
Context: ...t be completed before onboarding is done - requireHeader: boolean - If true, ...
(QB_NEW_EN)
[grammar] ~243-~243: There might be a mistake here.
Context: ... true, headers are required in context - requireRequest: boolean - If true,...
(QB_NEW_EN)
[grammar] ~244-~244: There might be a mistake here.
Context: ... - If true, request object is required - cloneRequest: boolean - Clone the re...
(QB_NEW_EN)
[grammar] ~311-~311: There might be a mistake here.
Context: ...onboarding" includeDefaultUser schema={{ user: { fields: { shouldOnboar...
(QB_NEW_EN)
[grammar] ~324-~324: There might be a mistake here.
Context: ... input: false, }, } } }} /> Fields: - shouldOnboard: Wheth...
(QB_NEW_EN)
[grammar] ~329-~329: There might be a mistake here.
Context: ...er the user needs to complete onboarding - completedSteps: JSON string array of completed step ID...
(QB_NEW_EN)
🔇 Additional comments (3)
apps/docs/src/components/sidebar-content.tsx (2)
25-26: LGTM: Icon import added correctly.DoorOpen is a valid lucide-react icon and matches the usage below.
77-81: LGTM: Sidebar entry for Onboarding.Route, title, and icon are consistent with existing entries. Placement after “App Invite” looks coherent.
apps/docs/content/docs/offer.mdx (1)
11-15: LGTM: Plugins list updated to include Onboarding and App Invite.Copy is concise and consistent with other plugin descriptions. “Waitlist” remains correctly marked as Coming Soon.
| import type { GenericEndpointContext } from "better-auth"; | ||
| import type { OnboardingOptions } from "./types"; | ||
|
|
||
| export const getOnboardingAdapter = ( | ||
| options: OnboardingOptions<any, any>, | ||
| ctx: GenericEndpointContext, | ||
| ) => { | ||
| return { | ||
| getCompletedSteps: async (userId: string) => { | ||
| let completedSteps: string[]; | ||
| if (options.secondaryStorage && ctx.context.secondaryStorage) { | ||
| completedSteps = | ||
| JSON.parse( | ||
| (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? | ||
| "{}", | ||
| ).completedSteps ?? []; | ||
| } else { | ||
| completedSteps = JSON.parse( | ||
| ( | ||
| await ctx.context.adapter.findOne<{ | ||
| completedSteps?: string; | ||
| }>({ | ||
| model: "user", | ||
| where: [ | ||
| { | ||
| field: "id", | ||
| value: userId, | ||
| }, | ||
| ], | ||
| select: ["completedSteps"], | ||
| }) | ||
| )?.completedSteps ?? "[]", | ||
| ); | ||
| } | ||
|
|
||
| return new Set<string>(completedSteps); | ||
| }, | ||
| updateOnboardingState: async ( | ||
| userId: string, | ||
| data: Partial<{ | ||
| shouldOnboard: boolean | null; | ||
| completedSteps: string[] | null; | ||
| }>, | ||
| ) => { | ||
| if (options.secondaryStorage && ctx.context.secondaryStorage) { | ||
| const currentState = JSON.parse( | ||
| (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? | ||
| "{}", | ||
| ); | ||
| const baseState = { | ||
| shouldOnboard: false, | ||
| completedSteps: [], | ||
| }; | ||
| await ctx.context.secondaryStorage.set(`onboarding:${userId}`, { | ||
| ...baseState, | ||
| ...currentState, | ||
| ...data, | ||
| }); | ||
| } else { | ||
| await ctx.context.internalAdapter.updateUser(userId, { | ||
| ...data, | ||
| completedSteps: Array.isArray(data.completedSteps) | ||
| ? JSON.stringify(data.completedSteps) | ||
| : data.completedSteps, | ||
| }); | ||
| } | ||
| }, | ||
| getShouldOnboard: async (userId: string) => { | ||
| if (options.secondaryStorage && ctx.context.secondaryStorage) { | ||
| return ( | ||
| JSON.parse( | ||
| (await ctx.context.secondaryStorage.get(`onboarding:${userId}`)) ?? | ||
| "{}", | ||
| ).shouldOnboard ?? false | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| ( | ||
| await ctx.context.adapter.findOne<{ shouldOnboard?: boolean }>({ | ||
| model: "user", | ||
| where: [ | ||
| { | ||
| field: "id", | ||
| value: userId, | ||
| }, | ||
| ], | ||
| select: ["shouldOnboard"], | ||
| }) | ||
| )?.shouldOnboard ?? false | ||
| ); | ||
| }, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Do we have tests for both DB and secondary storage modes?
To avoid regressions, ensure tests cover getCompletedSteps, updateOnboardingState, and getShouldOnboard in both modes.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for tests exercising secondaryStorage
rg -n -C2 --type=ts 'secondaryStorage\s*:\s*true' packages/plugins/onboarding/tests || true
# Sanity: verify adapter methods are referenced in tests
rg -n -C2 --type=ts 'getCompletedSteps|updateOnboardingState|getShouldOnboard' packages/plugins/onboarding/tests || true
# Verify verify-onboarding flow is tested
rg -n -C2 --type=ts 'verifyOnboarding|shouldOnboard' packages/plugins/onboarding/tests || trueLength of output: 5336
Add tests for secondaryStorage mode
Our grep verification shows no tests in packages/plugins/onboarding/tests referencing secondaryStorage: true. As a result, only the DB-backed branches of the adapter are covered. To prevent regressions, please add test cases exercising the secondaryStorage path for all three methods:
getCompletedStepswhenoptions.secondaryStorageandctx.context.secondaryStorageare both truthyupdateOnboardingStatein secondaryStorage mode, asserting that the stored payload merges base state, current state, and inputdatagetShouldOnboardin secondaryStorage mode, verifying the default (false) and overriddenshouldOnboardvalues
Target your new tests under packages/plugins/onboarding/tests, mirroring existing patterns for the DB-backed scenarios but configuring the client (or context) with secondaryStorage: true.
🤖 Prompt for AI Agents
In packages/plugins/onboarding/src/adapter.ts lines 1-94, the adapter contains
branches for secondaryStorage that are not covered by tests; add new tests under
packages/plugins/onboarding/tests that mirror the existing DB-backed test
patterns but configure the test context with options.secondaryStorage = true and
ctx.context.secondaryStorage implemented (mocked get/set). Create three tests:
one for getCompletedSteps reading and parsing completedSteps from
secondaryStorage, one for updateOnboardingState that sets secondaryStorage value
and asserts the stored payload merges baseState, currentState, and provided data
(including JSON-ifying completedSteps if needed), and one for getShouldOnboard
that verifies the default false and an overridden true value from
secondaryStorage; use the same test utilities/mocks used by existing tests and
assert expected values after invoking each adapter method.
1 participant
Summary by CodeRabbit
Possible future improvements