Skip to content

[Auth] Add extraFields to sign in method#2359

Merged
nezaj merged 34 commits intomainfrom
extra-fields
Mar 20, 2026
Merged

[Auth] Add extraFields to sign in method#2359
nezaj merged 34 commits intomainfrom
extra-fields

Conversation

@nezaj
Copy link
Contributor

@nezaj nezaj commented Mar 12, 2026

When someone signs up with Instant, we create a bare user with just an email. If a dev wants to add custom fields, they have to run a separate transaction after login. There's also no way to distinguish a new signup from a returning user, and no way to restrict who can sign up.

Solution

extraFields + created

Every auth flow now accepts an optional extraFields map and returns a created boolean.

extraFields writes custom $users properties atomically during user creation. If the user already exists, the fields are ignored and validation is skipped. Returning created helps clients differentiate between sign-up and login.

db.auth.signInWithMagicCode({ email, code, extraFields })
db.auth.signInWithIdToken({ clientName, idToken, nonce, extraFields })
db.auth.createAuthorizationURL({ clientName, redirectURL, extraFields })
db.auth.exchangeOAuthCode({ code, codeVerifier, extraFields })
db.auth.checkMagicCode(email, code, { extraFields }) // admin SDK

For web redirects, extraFields is stored in the kv store under a unique ID that's appended to the redirect URL. Each createAuthorizationURL call gets its own ID so multiple sign-in links on the same page don't overwrite each other. Entries are cleaned up after login.

checkMagicCode (admin SDK)

The existing verifyMagicCode in the admin SDK returns a User directly. Adding created to it would risk colliding with a user-defined created field. So we added checkMagicCode which returns { user, created } and supports typed extraFields via the schema. verifyMagicCode is deprecated but unchanged for backwards compatibility.

$users create permission rules

Devs can now write create rules on $users to restrict who can sign up or validate extraFields. The rule runs during auth signup flows only (not via db.transact, which is blocked by a new validate-system-create-entity! check in permissioned transactions).

Default is true (anyone can sign up). auth and data are both set to the user being created. ref() is not available since no relationships exist yet.

// Restrict to a domain
{ "$users": { "allow": { "create": "data.email.endsWith('@mycompany.com')" } } }

// Validate field values
{ "$users": { "allow": { "create": "data.username == null || data.username.size() >= 3" } } }

// Disable all signups (waitlist mode)
{ "$users": { "allow": { "create": "false" } } }

In the magic code flow, the permission check runs before consuming the code so a failed check doesn't burn the one-time code.

Tests

Server tests (routes_test.clj)

extraFields tests:

  • Magic code (runtime + admin): new user gets fields + created: true, returning user ignores fields + created: false, backwards compat, unknown keys rejected, system fields rejected, returning user with invalid extraFields still signs in
  • Guest upgrade: extra-fields applied on initial upgrade, user keeps original ID
  • OAuth: upsert-oauth-link! direct call, new + existing user
  • Admin refresh_tokens: create applies extra-fields, existing user ignores

Create permission tests:

  • Rule validation: create rules can be saved, delete rules still blocked
  • Magic code: rule blocks signup, code not burned on failure, domain restriction, field validation, default allows, skips for existing users
  • Admin bypass: admin SDK bypasses create rule
  • OAuth: rule blocks/allows OAuth signup
  • Guest: rule blocks/allows guest signup
  • Transact blocked: $users creation via db.transact blocked even with create rule set to true

Rule model test (rule_test.clj):

  • Updated to verify create rules are now allowed for $users

E2e tests (separate PR #2394)

Moved to extra-fields-e2e-auth branch since they hit prod which doesn't have the backend changes yet.

Manual sandbox tests

  • /play/oauth-extra-fields -- magic code flow with extraFields on an ephemeral app
  • /play/oauth-extra-fields-google -- Google OAuth with extraFields
  • /play/signup-rules -- interactive testing of $users create permission rules with domain restriction and field validation

Docs + Rules

  • users.md -- "Setting properties at signup" section with examples for all auth patterns plus created-based scaffolding. New "Signup rules" section with examples for domain restriction, field validation, and waitlist mode.
  • backend.md -- checkMagicCode example with extraFields and created.
  • http-api.md -- extra-fields documented on refresh_tokens and verify_magic_code endpoints.
  • instant-rules.md -- Updated $users permissions to reflect create rules are now supported.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request introduces optional custom property support at signup via extraFields, allowing users to atomically write additional $users properties during authentication. It adds a new checkMagicCode API, implements signup permission rules, and updates authentication flows across magic code, OAuth, and admin refresh token methods to support this feature.

Changes

Cohort / File(s) Summary
Admin SDK Type Updates
client/packages/admin/src/index.ts
Made Auth class generic with schema parameter; added checkMagicCode method supporting extraFields parameter; deprecated verifyMagicCode.
Core SDK Auth Types
client/packages/core/src/authAPI.ts, client/packages/core/src/index.ts
Introduced CheckMagicCodeParams and CheckMagicCodeResponse types; added checkMagicCode function; extended ExchangeCodeForTokenParams and SignInWithIdTokenParams with extraFields support; updated return types for OAuth/token exchange methods.
Reactor Auth Flow
client/packages/core/src/Reactor.js
Updated signInWithMagicCode, createAuthorizationURL, exchangeCodeForToken, and signInWithIdToken methods to accept and forward extraFields; added local storage persistence for OAuth extraFields across redirect.
E2E Test Infrastructure
client/packages/core/__tests__/src/utils/e2e.ts, client/packages/core/package.json, client/packages/core/vitest.config.ts
Added port-based local dev configuration; exposed appId and adminToken from test infrastructure; added test:e2e npm script.
SDK Type Exports
client/packages/react/src/index.ts, client/packages/react-native/src/index.ts, client/packages/solidjs/src/index.ts, client/packages/svelte/src/lib/index.ts
Re-exported CheckMagicCodeParams and CheckMagicCodeResponse types from core package.
Backend Admin Routes
server/src/instant/admin/routes.clj
Updated /admin/refresh_tokens and /admin/verify_magic_code endpoints to accept and process extra-fields, validate via signup rules, and return created boolean flag.
User Model & Validation
server/src/instant/model/app_user.clj
Added validate-extra-fields!, assert-signup! public functions and private assert-create-permission!; extended create! to merge extra-fields into user attributes during transaction.
Magic Code Auth Flow
server/src/instant/runtime/magic_code_auth.clj
Updated verify! to accept extra-fields and admin? parameters; checks $users.create permission during signup; returns created flag alongside user and refresh token.
Runtime OAuth & Auth Routes
server/src/instant/runtime/routes.clj
Extended /runtime/auth/verify_magic_code, OAuth token/id-token callbacks, and guest signin to handle extra-fields, check create permissions, and return created boolean.
Permission Rules System
server/src/instant/model/rule.clj, server/src/instant/db/permissioned_transaction.clj
Added $users.create rule support (allowing create/update/view overrides, blocking delete); configured default-allow for $users.create; blocked $users creation via transact!.
Comprehensive Test Suite
server/test/instant/runtime/routes_test.clj, server/test/instant/model/rule_test.clj
Added 333+ lines of tests covering extraFields behavior, $users.create rule validation, signup rule enforcement across magic code/OAuth/guest/admin flows, and permission bypass logic.
Authentication Examples
client/sandbox/react-nextjs/pages/play/oauth-extra-fields.tsx, client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx, client/sandbox/react-nextjs/pages/play/signup-rules.tsx
Added three new example pages demonstrating magic code with extra fields, Google OAuth with extra fields, and custom signup rules with field validation.
Documentation Updates
client/www/pages/docs/auth.md, client/www/pages/docs/backend.md, client/www/pages/docs/http-api.md, client/www/pages/docs/users.md, client/www/lib/intern/instant-rules.md, client/packages/create-instant-app/template/rules/*
Updated API reference, admin SDK examples, rule defaults, and permission matrix; added "Setting custom properties at signup" section explaining extraFields feature and created flag usage across all auth methods.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SDK as SDK/Auth
    participant Backend as Backend Auth
    participant RuleEngine as Rule Engine
    participant UserDB as User DB

    Client->>SDK: signInWithMagicCode({ email, code, extraFields })
    SDK->>Backend: POST /runtime/auth/verify_magic_code<br/>(email, code, extra-fields)
    Backend->>Backend: Fetch existing user by email
    Backend->>RuleEngine: Check $users.create rule<br/>(if new user)
    alt User creates
        RuleEngine-->>Backend: Permission check result
        Backend->>Backend: validate-extra-fields!
        Backend->>RuleEngine: Evaluate create rule<br/>with data context
        RuleEngine-->>Backend: Permission granted/denied
    end
    alt Permission granted OR existing user
        Backend->>UserDB: Create or update user with extra-fields
        Backend->>UserDB: Consume magic code
        Backend-->>SDK: { user, created: true/false, refresh_token }
    else Permission denied
        Backend-->>SDK: HTTP 400 validation error
    end
    SDK-->>Client: { user, created }
Loading
sequenceDiagram
    participant Client
    participant SDK as SDK/Reactor
    participant Storage as Local Storage
    participant Backend as OAuth Backend
    participant Exchange as Token Exchange

    Client->>SDK: createAuthorizationURL<br/>({ clientName, redirectURL, extraFields })
    SDK->>Storage: Store extraFields under oauthExtraFieldsKey
    SDK-->>Client: OAuth authorization URL
    Client->>Backend: Initiate OAuth flow
    Client->>SDK: _oauthLoginInit (redirect callback)
    SDK->>Storage: Retrieve and delete extraFields
    SDK->>Exchange: exchangeCodeForToken<br/>({ code, extraFields })
    Exchange->>Backend: POST /runtime/auth/oauth/callback<br/>(with extra_fields)
    Backend-->>Exchange: { user, created, refresh_token }
    Exchange-->>SDK: { user, created }
    SDK-->>Client: User signed in with custom properties
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

  • stopachka
  • dwwoelfel
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title '[Auth] Add extraFields to sign in method' accurately summarizes the main change in the changeset by identifying the new extraFields parameter added to authentication sign-in methods.
Description check ✅ Passed The description clearly explains the problem being solved, the solution provided (extraFields + created), updated auth flows, testing approach, and documentation changes, all of which directly relate to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch extra-fields

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

View Vercel preview at instant-www-js-extra-fields-jsv.vercel.app.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/src/instant/runtime/routes.clj (1)

586-600: ⚠️ Potential issue | 🟠 Major

Validate extra_fields before consuming the OAuth code.

app-oauth-code-model/consume! runs before validate-extra-fields!. If the request carries an unknown/system field, the call fails after the one-time OAuth code is already spent, so the user cannot retry with corrected input.

Suggested fix
   (let [app-id (ex/get-some-param! req (param-paths :app_id) uuid-util/coerce)
         code (ex/get-some-param! req (param-paths :code) uuid-util/coerce)
         code-verifier (some #(get-in req %) (param-paths :code_verifier))
         extra-fields (get-in req [:body :extra_fields])
+        _ (app-user-model/validate-extra-fields! app-id extra-fields)
         oauth-code (app-oauth-code-model/consume! {:code code
                                                    :app-id app-id
                                                    :verifier code-verifier})
@@
-        _ (assert (= app-id app_id) (str "(= " app-id " " app_id ")"))
-        _ (app-user-model/validate-extra-fields! app-id extra-fields)
+        _ (assert (= app-id app_id) (str "(= " app-id " " app_id ")"))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/instant/runtime/routes.clj` around lines 586 - 600, Move
validation of request extra_fields to run before consuming the one-time OAuth
code: retrieve extra-fields from req (extra-fields), call
app-user-model/validate-extra-fields! with app-id and extra-fields first, then
call app-oauth-code-model/consume!; keep the existing origin header check and
the subsequent assertions the same but ensure
app-user-model/validate-extra-fields! runs before app-oauth-code-model/consume!
to avoid spending the code on invalid input.
🧹 Nitpick comments (3)
client/packages/create-instant-app/template/rules/cursor-rules.md (1)

286-308: Inconsistent transaction reference in code example.

The code example on line 302 uses tx.settings[id()], but the convention shown elsewhere in this file (line 243) uses a fully-qualified reference like db.tx.todos[id()]. For consistency and clarity, the example should show where tx comes from or use the full db.tx reference.

📝 Suggested fix for consistency
 // Scaffold data for new users
 if (created) {
   db.transact([
-    tx.settings[id()]
+    db.tx.settings[id()]
       .update({ theme: 'light', notifications: true })
       .link({ user: user.id }),
   ]);
 }

Otherwise, the documentation accurately reflects the new extraFields API and the created flag, and the code examples effectively demonstrate the key use cases.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/packages/create-instant-app/template/rules/cursor-rules.md` around
lines 286 - 308, The example uses an undefined local `tx` (tx.settings[id()])
causing inconsistency; update the snippet to reference the full transaction
helper `db.tx` (e.g., use db.tx.settings[id()]) or explicitly show how `tx` is
defined before use; ensure the example uses the same convention as other
examples with signInWithMagicCode/extraFields and the created flag so the
db.transact call references db.tx.settings[id()] (or introduces a prior const tx
= db.tx) for clarity.
client/packages/core/vitest.config.ts (1)

4-5: LGTM with a minor observation on edge case handling.

The nullish coalescing (?? 0) handles undefined, but if DEV_SLOT is set to a non-numeric string (e.g., DEV_SLOT=abc), localPort will be NaN. Given the context in e2e.ts, this is safe since NaN is falsy and tests would fall back to production URLs—but this silent fallback might mask configuration errors.

If explicit validation is preferred, consider:

🔧 Optional: Add validation for DEV_SLOT
-const devSlot = Number(process.env.DEV_SLOT ?? 0);
-const localPort = process.env.CI ? 0 : 8888 + devSlot * 1000;
+const devSlot = Number(process.env.DEV_SLOT ?? 0);
+if (Number.isNaN(devSlot)) {
+  throw new Error(`Invalid DEV_SLOT value: ${process.env.DEV_SLOT}`);
+}
+const localPort = process.env.CI ? 0 : 8888 + devSlot * 1000;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/packages/core/vitest.config.ts` around lines 4 - 5, The code uses
Number(process.env.DEV_SLOT ?? 0) which yields NaN for non-numeric DEV_SLOT
values; update the logic in vitest.config.ts around devSlot and localPort to
validate DEV_SLOT explicitly: read the raw string from process.env.DEV_SLOT,
attempt to parse it (e.g., parseInt/Number), check isFinite/Number.isNaN, and if
invalid fall back to 0 (or optionally throw) and compute localPort from the
validated devSlot so stray non-numeric environment values no longer produce NaN
silently.
client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts (1)

57-70: Cover the “ignore extraFields for existing users” contract.

This test proves created=false, but it would still pass if a later sign-in overwrote the existing $users row. Since that behavior is a core contract in this PR, re-sign in with different extraFields and assert the stored values stay unchanged.

💡 Suggested test expansion
 authTest(
   'returning user gets created=false',
   async ({ db, appId, adminToken }) => {
     const email = `returning-${Date.now()}@test.com`;

     // First sign in -- creates user
     const code1 = await generateMagicCode(appId, adminToken, email);
-    const res1 = await db.auth.signInWithMagicCode({ email, code: code1 });
+    const res1 = await db.auth.signInWithMagicCode({
+      email,
+      code: code1,
+      extraFields: { username: 'first_user', displayName: 'First User' },
+    });
     expect(res1.created).toBe(true);

     // Second sign in -- existing user
     const code2 = await generateMagicCode(appId, adminToken, email);
-    const res2 = await db.auth.signInWithMagicCode({ email, code: code2 });
+    const res2 = await db.auth.signInWithMagicCode({
+      email,
+      code: code2,
+      extraFields: { username: 'second_user', displayName: 'Second User' },
+    });
     expect(res2.created).toBe(false);
+
+    const { data } = await db.queryOnce({ $users: {} });
+    const user = data.$users.find((u: any) => u.email === email);
+    expect(user!.username).toBe('first_user');
+    expect(user!.displayName).toBe('First User');
   },
 );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts` around
lines 57 - 70, The test currently only asserts created=false but doesn't verify
that extraFields were not overwritten; update the test in
auth-extra-fields.e2e.test.ts to capture the stored user after the first sign-in
(use the returned user id or query the $users row) then perform the second
sign-in with different extraFields via generateMagicCode and
db.auth.signInWithMagicCode, and finally re-fetch the same $users row and assert
its extraFields still equal the original values (and that res2.created is false)
to ensure existing users are not updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/packages/core/src/Reactor.js`:
- Around line 2270-2276: The createAuthorizationURL method currently only writes
OAUTH_EXTRA_FIELDS_KEY to sessionStorage when extraFields is provided, leaving
stale data from prior attempts; update createAuthorizationURL so that when
sessionStorage is available but extraFields is falsy it explicitly removes
OAUTH_EXTRA_FIELDS_KEY (or sets it to null) from sessionStorage to prevent
_oauthLoginInit from reusing stale fields; refer to createAuthorizationURL,
OAUTH_EXTRA_FIELDS_KEY and _oauthLoginInit when making this change.

In `@client/packages/create-instant-app/template/rules/AGENTS.md`:
- Around line 296-299: The example uses an undefined variable `tx` causing a
ReferenceError; update the scaffold to call the transaction builder off `db`
(use `db.tx` instead of `tx`) when constructing the operations such as
`tx.settings[id()].update(...)` and `db.transact(...)` — ensure the snippet
consistently references `db.tx` (and `db.transact`) so symbols like
`db.transact`, `db.tx`, and `settings[id()]` are used correctly and no undefined
`tx` remains.

In `@client/packages/create-instant-app/template/rules/windsurf-rules.md`:
- Around line 286-308: The transaction example uses an undefined tx variable;
update the transaction to use the DB transaction namespace by changing the call
in the created branch to use db.tx.settings (i.e., replace tx.settings[id()]
with db.tx.settings[id()]) so the scaffold uses db.transact([...
db.tx.settings[id()].update(...).link(...) ...]) alongside the existing
db.auth.signInWithMagicCode, extraFields, created, and db.transact usage.

In `@client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx`:
- Around line 101-130: Move the call to db.auth.createAuthorizationURL out of
the component render and into the click handler for the "Google (Redirect)"
control: remove redirectLoginURL from render (and do not call
createAuthorizationURL during SSR), add an onClick handler for the anchor/button
that constructs the URL using db.auth.createAuthorizationURL({ clientName:
'google-web', redirectURL: window.location.href, extraFields: displayName ? {
displayName } : undefined }) only when running in the browser, then navigate
(e.g. window.location.href = url); this avoids accessing window during SSR and
prevents createAuthorizationURL's sessionStorage side effects from running on
render/Strict Mode.

In `@client/www/lib/intern/instant-rules.md`:
- Around line 296-301: The example uses an undefined bare symbol tx inside
db.transact; update the snippet to use the database object's transaction helper
(replace tx with db.tx) so it matches other examples (e.g., clientDb.tx) and is
import/usage-consistent; modify the call to db.transact([...
db.tx.settings[id()].update({ theme: 'light', notifications: true }).link({
user: user.id }) ]) accordingly so the example compiles and follows the same
pattern.

In `@client/www/pages/docs/backend.md`:
- Around line 403-408: The snippet incorrectly destructures { user, created }
from verifyMagicCode as if using the client SDK; in the Admin SDK
verifyMagicCode returns the user object (with user.created), so change the call
to capture the user directly (e.g., const user = await
db.auth.verifyMagicCode(...)) and then read token from user.refresh_token and
created from user.created; update any subsequent uses (token and created) to
reference the user object.

In `@client/www/pages/docs/users.md`:
- Around line 206-216: The docs example calls db.auth.createAuthorizationURL
during render which writes extraFields to sessionStorage; change the example to
call db.auth.createAuthorizationURL (or the wrapper createAuthorizationURL)
inside a user interaction handler (e.g., an onClick handler) immediately before
performing navigation so extraFields are saved at the correct time; locate the
usage of createAuthorizationURL in the docs example and move its invocation into
the click handler and then navigate to the returned URL, rather than computing
it during render.

---

Outside diff comments:
In `@server/src/instant/runtime/routes.clj`:
- Around line 586-600: Move validation of request extra_fields to run before
consuming the one-time OAuth code: retrieve extra-fields from req
(extra-fields), call app-user-model/validate-extra-fields! with app-id and
extra-fields first, then call app-oauth-code-model/consume!; keep the existing
origin header check and the subsequent assertions the same but ensure
app-user-model/validate-extra-fields! runs before app-oauth-code-model/consume!
to avoid spending the code on invalid input.

---

Nitpick comments:
In `@client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts`:
- Around line 57-70: The test currently only asserts created=false but doesn't
verify that extraFields were not overwritten; update the test in
auth-extra-fields.e2e.test.ts to capture the stored user after the first sign-in
(use the returned user id or query the $users row) then perform the second
sign-in with different extraFields via generateMagicCode and
db.auth.signInWithMagicCode, and finally re-fetch the same $users row and assert
its extraFields still equal the original values (and that res2.created is false)
to ensure existing users are not updated.

In `@client/packages/core/vitest.config.ts`:
- Around line 4-5: The code uses Number(process.env.DEV_SLOT ?? 0) which yields
NaN for non-numeric DEV_SLOT values; update the logic in vitest.config.ts around
devSlot and localPort to validate DEV_SLOT explicitly: read the raw string from
process.env.DEV_SLOT, attempt to parse it (e.g., parseInt/Number), check
isFinite/Number.isNaN, and if invalid fall back to 0 (or optionally throw) and
compute localPort from the validated devSlot so stray non-numeric environment
values no longer produce NaN silently.

In `@client/packages/create-instant-app/template/rules/cursor-rules.md`:
- Around line 286-308: The example uses an undefined local `tx`
(tx.settings[id()]) causing inconsistency; update the snippet to reference the
full transaction helper `db.tx` (e.g., use db.tx.settings[id()]) or explicitly
show how `tx` is defined before use; ensure the example uses the same convention
as other examples with signInWithMagicCode/extraFields and the created flag so
the db.transact call references db.tx.settings[id()] (or introduces a prior
const tx = db.tx) for clarity.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 18a896ff-f102-4210-94a3-918ee8920a53

📥 Commits

Reviewing files that changed from the base of the PR and between dda1eb0 and 115ad24.

📒 Files selected for processing (23)
  • client/packages/admin/src/index.ts
  • client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts
  • client/packages/core/__tests__/src/utils/e2e.ts
  • client/packages/core/package.json
  • client/packages/core/src/Reactor.js
  • client/packages/core/src/authAPI.ts
  • client/packages/core/src/index.ts
  • client/packages/core/vitest.config.ts
  • client/packages/create-instant-app/template/rules/AGENTS.md
  • client/packages/create-instant-app/template/rules/cursor-rules.md
  • client/packages/create-instant-app/template/rules/windsurf-rules.md
  • client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx
  • client/sandbox/react-nextjs/pages/play/oauth-extra-fields.tsx
  • client/www/lib/intern/instant-rules.md
  • client/www/pages/docs/auth.md
  • client/www/pages/docs/backend.md
  • client/www/pages/docs/http-api.md
  • client/www/pages/docs/users.md
  • server/src/instant/admin/routes.clj
  • server/src/instant/model/app_user.clj
  • server/src/instant/runtime/magic_code_auth.clj
  • server/src/instant/runtime/routes.clj
  • server/test/instant/runtime/routes_test.clj

Comment on lines 403 to 408
const { user, created } = await db.auth.verifyMagicCode(
req.body.email,
req.body.code,
{ extraFields: { nickname: req.body.nickname } },
);
const token = user.refresh_token;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the Admin SDK verifyMagicCode example shape.

This page documents @instantdb/admin, but the snippet destructures { user, created } as if it were the client SDK response. The later example already shows the actual Admin SDK shape via user.created.

Suggested fix
-const { user, created } = await db.auth.verifyMagicCode(
+const user = await db.auth.verifyMagicCode(
   req.body.email,
   req.body.code,
   { extraFields: { nickname: req.body.nickname } },
 );
+const { created } = user;
 const token = user.refresh_token;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { user, created } = await db.auth.verifyMagicCode(
req.body.email,
req.body.code,
{ extraFields: { nickname: req.body.nickname } },
);
const token = user.refresh_token;
const user = await db.auth.verifyMagicCode(
req.body.email,
req.body.code,
{ extraFields: { nickname: req.body.nickname } },
);
const { created } = user;
const token = user.refresh_token;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/www/pages/docs/backend.md` around lines 403 - 408, The snippet
incorrectly destructures { user, created } from verifyMagicCode as if using the
client SDK; in the Admin SDK verifyMagicCode returns the user object (with
user.created), so change the call to capture the user directly (e.g., const user
= await db.auth.verifyMagicCode(...)) and then read token from
user.refresh_token and created from user.created; update any subsequent uses
(token and created) to reference the user object.

let extraFields;
if (typeof sessionStorage !== 'undefined') {
try {
const stored = sessionStorage.getItem(OAUTH_EXTRA_FIELDS_KEY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen in Expo?

--

imo it may make sense to store in the storage key that we have here, so we are good against different platforms. We may also want to use some specific key to signify this particular session

},
);
return user;
return { ...res.user, created: res.created };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if someone has a field called created?

I think we may need to make a breaking change. Not sure if there's an alternative. cc @dwwoelfel for thoughts too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, yea what do we do here?

Copy link
Contributor

@stopachka stopachka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool!

if (extraFields && typeof sessionStorage !== 'undefined') {
sessionStorage.setItem(
OAUTH_EXTRA_FIELDS_KEY,
JSON.stringify(extraFields),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some way we could create a unique entry in the sessionStorage for each call to createAuthorizationUrl.

With the current implementation you can't remove the extra fields if you've set them and last call wins. An example of why you might want this: maybe you have two sign in urls, one sets source=homepage and one that sets source=pricing-page. With the current approach, you'd always get source=pricing-page if that was the last call to createAuthorizationUrl.

(when (and (contains? params :imageURL) imageURL)
[[:add-triple id (resolve-id :imageURL) imageURL]])))
[[:add-triple id (resolve-id :imageURL) imageURL]])
(map (fn [[k v]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be running auth checks on these fields?

They're user-provided, so this might be surprising to apps that expected that their custom fields couldn't be set by the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should now be auth checked!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
client/packages/core/src/Reactor.js (1)

2266-2273: ⚠️ Potential issue | 🟠 Major

Prevent stale extraFields reuse across OAuth attempts.

createAuthorizationURL only writes when extraFields exists, so a later flow without extraFields can reuse old persisted data. Also, one shared key makes concurrent auth links last-write-wins.

Suggested minimum fix
 createAuthorizationURL({ clientName, redirectURL, extraFields }) {
-  if (extraFields) {
-    this.kv.updateInPlace((prev) => {
-      prev[oauthExtraFieldsKey] = extraFields;
-    });
-  }
+  this.kv.updateInPlace((prev) => {
+    if (extraFields) {
+      prev[oauthExtraFieldsKey] = extraFields;
+    } else {
+      delete prev[oauthExtraFieldsKey];
+    }
+  });
   const { apiURI, appId } = this.config;
   return `${apiURI}/runtime/oauth/start?app_id=${appId}&client_name=${clientName}&redirect_uri=${redirectURL}`;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/packages/core/src/Reactor.js` around lines 2266 - 2273,
createAuthorizationURL currently only writes extraFields to kv when present,
causing stale data reuse and last-write-wins across concurrent flows; modify
createAuthorizationURL to (1) when extraFields is provided, generate a unique
per-attempt key (e.g., include a nonce/timestamp or clientName) instead of a
single shared oauthExtraFieldsKey, persist extraFields under that unique key via
kv.updateInPlace (or equivalent), and include that unique key as a query param
in the returned URL; and (2) when extraFields is not provided, ensure you do not
reuse an existing shared key—either omit the extraFields query param or
explicitly clear the stored value for that unique key so no stale data is
applied; reference createAuthorizationURL, kv.updateInPlace, and
oauthExtraFieldsKey to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@client/packages/core/src/Reactor.js`:
- Around line 2266-2273: createAuthorizationURL currently only writes
extraFields to kv when present, causing stale data reuse and last-write-wins
across concurrent flows; modify createAuthorizationURL to (1) when extraFields
is provided, generate a unique per-attempt key (e.g., include a nonce/timestamp
or clientName) instead of a single shared oauthExtraFieldsKey, persist
extraFields under that unique key via kv.updateInPlace (or equivalent), and
include that unique key as a query param in the returned URL; and (2) when
extraFields is not provided, ensure you do not reuse an existing shared
key—either omit the extraFields query param or explicitly clear the stored value
for that unique key so no stale data is applied; reference
createAuthorizationURL, kv.updateInPlace, and oauthExtraFieldsKey to locate and
update the logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 504a9d62-0422-43a4-ad0f-1991d3216819

📥 Commits

Reviewing files that changed from the base of the PR and between 115ad24 and 8325302.

📒 Files selected for processing (1)
  • client/packages/core/src/Reactor.js

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
server/src/instant/runtime/routes.clj (1)

583-600: ⚠️ Potential issue | 🟠 Major

Run request validation before consuming the OAuth code.

Line 587 invalidates the one-time code before the request-only checks below (Origin and extra_fields) run. With the new extra_fields support, a typo now forces the user back through the provider flow even though the grant itself was valid.

↕️ Suggested reordering
   (let [app-id (ex/get-some-param! req (param-paths :app_id) uuid-util/coerce)
         code (ex/get-some-param! req (param-paths :code) uuid-util/coerce)
         code-verifier (some #(get-in req %) (param-paths :code_verifier))
         extra-fields (get-in req [:body :extra_fields])
-        oauth-code (app-oauth-code-model/consume! {:code code
-                                                   :app-id app-id
-                                                   :verifier code-verifier})
         _ (when-let [origin (get-in req [:headers "origin"])]
             (let [authorized-origins (app-authorized-redirect-origin-model/get-all-for-app
                                       {:app-id app-id})]
               (when-not (app-authorized-redirect-origin-model/find-match
                          authorized-origins origin)
                 (ex/throw-validation-err! :origin origin [{:message "Unauthorized origin."}]))))
         _ (app-user-model/validate-extra-fields! app-id extra-fields)
+        oauth-code (app-oauth-code-model/consume! {:code code
+                                                   :app-id app-id
+                                                   :verifier code-verifier})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/instant/runtime/routes.clj` around lines 583 - 600, Reorder the
request handling so validation runs before the one-time OAuth code is consumed:
keep computing app-id, code, code-verifier, and extra-fields from req as-is,
then perform the Origin header check (using
app-authorized-redirect-origin-model/get-all-for-app and find-match) and call
app-user-model/validate-extra-fields! before calling
app-oauth-code-model/consume!; after consume! you can destructure {:keys [app_id
client_id user_info]} from oauth-code and assert (= app-id app_id) as before.
Ensure the call site of app-oauth-code-model/consume! (and the subsequent
equality assert) is moved below the two validation steps so a validation error
does not prematurely invalidate the one-time code.
server/src/instant/runtime/magic_code_auth.clj (1)

141-160: ⚠️ Potential issue | 🟠 Major

Don't merge the created flag into the flat user map.

validate-extra-fields! only rejects unknown/system attrs, so a schema-defined $users.created is still legal. The assoc on Line 160 overwrites that user field for both new and existing users, and verify-magic-code-post then strips it back out in Lines 110-111 of server/src/instant/runtime/routes.clj, so a legitimate attribute disappears from magic-code sign-in responses.

Return created alongside the user instead of merging it into the entity map, or reserve created during extra-field validation.

♻️ Duplicate comments (3)
client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx (1)

101-105: ⚠️ Potential issue | 🟠 Major

Move redirect URL creation out of render.

window.location.href in render can break SSR, and calling createAuthorizationURL() during render can repeatedly execute redirect-persistence side effects. Compute it in the click handler instead.

🛠️ Suggested fix
-  const redirectLoginURL = db.auth.createAuthorizationURL({
-    clientName: 'google-web',
-    redirectURL: window.location.href,
-    extraFields: displayName ? { displayName } : undefined,
-  });
-
   return (
@@
-          <a
-            href={redirectLoginURL}
-            className="inline-block rounded bg-blue-500 px-4 py-2 text-white"
+          <button
+            type="button"
+            className="inline-block rounded bg-blue-500 px-4 py-2 text-white"
+            onClick={() => {
+              const url = db.auth.createAuthorizationURL({
+                clientName: 'google-web',
+                redirectURL: window.location.href,
+                extraFields: displayName ? { displayName } : undefined,
+              });
+              window.location.assign(url);
+            }}
           >
             Google (Redirect)
-          </a>
+          </button>
#!/bin/bash
# Verify render-time usage of window/createAuthorizationURL on this page.
rg -n "createAuthorizationURL|window\\.location\\.href|href=\\{redirectLoginURL\\}" client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx

Also applies to: 125-130

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx` around
lines 101 - 105, The code currently computes redirectLoginURL during render
using window.location.href and db.auth.createAuthorizationURL (variable
redirectLoginURL), which breaks SSR and causes side effects; move the
createAuthorizationURL call into the sign-in click handler (the onClick that
triggers the login flow), compute const redirectURL = window.location.href
inside that handler (or derive it safely client-side) and then call
db.auth.createAuthorizationURL({ clientName: 'google-web', redirectURL,
extraFields: displayName ? { displayName } : undefined }) there so nothing runs
at render time and the extraFields (displayName) are passed from the current
component state.
client/www/pages/docs/users.md (1)

237-243: ⚠️ Potential issue | 🟡 Minor

Use db.tx in this docs snippet.

Line 240 references tx.settings, but tx is not defined anywhere in the example. As written, the copy-paste path throws before it creates the default record.

📝 Suggested fix
 if (created) {
   // Create default data for the new user
   db.transact([
-    tx.settings[id()]
+    db.tx.settings[id()]
       .update({ theme: 'light', notifications: true })
       .link({ user: user.id }),
   ]);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/www/pages/docs/users.md` around lines 237 - 243, The snippet uses an
undefined identifier `tx` inside the db.transact call
(tx.settings[id()].update(...).link(...)), causing a runtime error; replace the
`tx` usage with the correct transactional API `db.tx` (i.e., use
db.tx.settings[id()].update(...) .link(...)) so the default user settings are
created within the transaction invoked by db.transact/db.tx and the references
to created, db.transact and settings[id()].update(...).link(...) remain
consistent.
client/packages/core/src/Reactor.js (1)

2282-2290: ⚠️ Potential issue | 🟠 Major

Clear stale OAuth extraFields when a new flow doesn't provide any.

The current implementation only writes to storage when extraFields is provided. If a previous OAuth flow stored fields and the next attempt omits them, the old payload survives and _oauthLoginInit() will attach it to the later sign-in.

Suggested fix
   createAuthorizationURL({ clientName, redirectURL, extraFields }) {
-    if (extraFields) {
-      this.kv.updateInPlace((prev) => {
-        prev[oauthExtraFieldsKey] = extraFields;
-      });
-    }
+    this.kv.updateInPlace((prev) => {
+      if (extraFields) {
+        prev[oauthExtraFieldsKey] = extraFields;
+      } else {
+        delete prev[oauthExtraFieldsKey];
+      }
+    });
     const { apiURI, appId } = this.config;
     return `${apiURI}/runtime/oauth/start?app_id=${appId}&client_name=${clientName}&redirect_uri=${redirectURL}`;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/packages/core/src/Reactor.js` around lines 2282 - 2290, The
createAuthorizationURL function only updates storage when extraFields is
present, leaving stale oauthExtraFieldsKey values around; modify
createAuthorizationURL so that when extraFields is falsy it explicitly clears
the stored value (e.g., call this.kv.updateInPlace and delete or null-out
prev[oauthExtraFieldsKey]) so subsequent calls (and _oauthLoginInit) won't pick
up prior extraFields; keep the existing write behavior when extraFields is
provided.
🧹 Nitpick comments (2)
client/sandbox/react-nextjs/pages/play/oauth-extra-fields.tsx (1)

136-137: Consider removing temporary debug logs before merge.

These console.log calls are useful during development, but trimming them keeps sandbox output cleaner.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/sandbox/react-nextjs/pages/play/oauth-extra-fields.tsx` around lines
136 - 137, Remove the temporary console.log debug statements in the
signInWithMagicCode flow (the lines logging 'signInWithMagicCode result:' and
'created:') — either delete them or replace with a proper debug/logger call if
persistent telemetry is desired (e.g., use the app's logger at debug level).
Ensure you update the function handling sign-in so no leftover console.log
remains after verification.
client/packages/create-instant-app/template/rules/windsurf-rules.md (1)

294-298: Avoid using a client clock for createdAt in the signup example.

Line 297 makes createdAt look like a trusted signup timestamp, but this rules file is meant to be copied into generated apps and Date.now() here is entirely client-controlled. A neutral profile field is safer unless the timestamp is set server-side.

✏️ Suggested tweak
 const { user, created } = await db.auth.signInWithMagicCode({
   email,
   code,
-  extraFields: { nickname, createdAt: Date.now() },
+  extraFields: { nickname },
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/packages/create-instant-app/template/rules/windsurf-rules.md` around
lines 294 - 298, The example uses Date.now() for the signup `createdAt` in the
call to db.auth.signInWithMagicCode (inside extraFields), which relies on an
untrusted client clock; update the example to avoid setting a client-controlled
timestamp—remove or null out the `createdAt` field (or replace it with a neutral
profile field like `bio` or `signupSource`) in the `extraFields` passed to
db.auth.signInWithMagicCode so that the server can set the canonical creation
timestamp instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts`:
- Around line 20-31: The helper currently returns data.code without checking the
HTTP response, so failures are masked; update the helper that calls fetch (the
block using apiUrl/admin/magic_code with appId and adminToken) to check res.ok
and throw a descriptive error when not OK (include res.status and the response
body or JSON) before returning data.code, so callers get a clear failure instead
of undefined.

In `@client/www/pages/docs/http-api.md`:
- Around line 270-272: The prose incorrectly says "provider id or email" which
conflicts with the documented contract for POST /admin/refresh_tokens; update
the copy to refer to the endpoint's accepted inputs as "id or email" (or
explicitly "user id or email") so it matches the endpoint contract, keep the
rest of the sentence about passing `extra-fields` to set `$users` properties and
that the response includes `user.refresh_token` and `"created": true` when a new
user is created, and optionally add a short link reference to the existing POST
/admin/refresh_tokens section for clarity.

In `@server/src/instant/admin/routes.clj`:
- Around line 400-402: The extra-fields validation (the call to
app-user-model/validate-extra-fields! using extra-fields from req) and the
magic-code verification should be deferred from the shared pre-check path into
the branch that handles creating a new user; update routes.clj so you first
determine if the user already exists (the logic used in the
refresh-tokens/sign-in path) and only call app-user-model/validate-extra-fields!
and the magic-code validation in the create/new-user branch (and likewise move
the validations out of the refresh-token/pre-auth path), ensuring existing-user
sign-ins skip those validations.

In `@server/src/instant/model/app_user.clj`:
- Around line 15-33: validate-extra-fields! must reject the reserved key
"created" to avoid response collisions; inside the existing loop (in
validate-extra-fields! which calls attr-model/get-by-app-id and
attr-model/seek-by-fwd-ident-name) add a check after computing k-str and before
attr validation that if k-str equals "created" then call
ex/throw-validation-err! with :extra-fields, extra-fields and a message like
"Cannot set reserved field: created" so user-supplied $users.created is
rejected.

---

Outside diff comments:
In `@server/src/instant/runtime/routes.clj`:
- Around line 583-600: Reorder the request handling so validation runs before
the one-time OAuth code is consumed: keep computing app-id, code, code-verifier,
and extra-fields from req as-is, then perform the Origin header check (using
app-authorized-redirect-origin-model/get-all-for-app and find-match) and call
app-user-model/validate-extra-fields! before calling
app-oauth-code-model/consume!; after consume! you can destructure {:keys [app_id
client_id user_info]} from oauth-code and assert (= app-id app_id) as before.
Ensure the call site of app-oauth-code-model/consume! (and the subsequent
equality assert) is moved below the two validation steps so a validation error
does not prematurely invalidate the one-time code.

---

Duplicate comments:
In `@client/packages/core/src/Reactor.js`:
- Around line 2282-2290: The createAuthorizationURL function only updates
storage when extraFields is present, leaving stale oauthExtraFieldsKey values
around; modify createAuthorizationURL so that when extraFields is falsy it
explicitly clears the stored value (e.g., call this.kv.updateInPlace and delete
or null-out prev[oauthExtraFieldsKey]) so subsequent calls (and _oauthLoginInit)
won't pick up prior extraFields; keep the existing write behavior when
extraFields is provided.

In `@client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx`:
- Around line 101-105: The code currently computes redirectLoginURL during
render using window.location.href and db.auth.createAuthorizationURL (variable
redirectLoginURL), which breaks SSR and causes side effects; move the
createAuthorizationURL call into the sign-in click handler (the onClick that
triggers the login flow), compute const redirectURL = window.location.href
inside that handler (or derive it safely client-side) and then call
db.auth.createAuthorizationURL({ clientName: 'google-web', redirectURL,
extraFields: displayName ? { displayName } : undefined }) there so nothing runs
at render time and the extraFields (displayName) are passed from the current
component state.

In `@client/www/pages/docs/users.md`:
- Around line 237-243: The snippet uses an undefined identifier `tx` inside the
db.transact call (tx.settings[id()].update(...).link(...)), causing a runtime
error; replace the `tx` usage with the correct transactional API `db.tx` (i.e.,
use db.tx.settings[id()].update(...) .link(...)) so the default user settings
are created within the transaction invoked by db.transact/db.tx and the
references to created, db.transact and settings[id()].update(...).link(...)
remain consistent.

---

Nitpick comments:
In `@client/packages/create-instant-app/template/rules/windsurf-rules.md`:
- Around line 294-298: The example uses Date.now() for the signup `createdAt` in
the call to db.auth.signInWithMagicCode (inside extraFields), which relies on an
untrusted client clock; update the example to avoid setting a client-controlled
timestamp—remove or null out the `createdAt` field (or replace it with a neutral
profile field like `bio` or `signupSource`) in the `extraFields` passed to
db.auth.signInWithMagicCode so that the server can set the canonical creation
timestamp instead.

In `@client/sandbox/react-nextjs/pages/play/oauth-extra-fields.tsx`:
- Around line 136-137: Remove the temporary console.log debug statements in the
signInWithMagicCode flow (the lines logging 'signInWithMagicCode result:' and
'created:') — either delete them or replace with a proper debug/logger call if
persistent telemetry is desired (e.g., use the app's logger at debug level).
Ensure you update the function handling sign-in so no leftover console.log
remains after verification.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d88e4f2c-4540-4208-9bca-87640d5afda6

📥 Commits

Reviewing files that changed from the base of the PR and between 8325302 and 54e8476.

📒 Files selected for processing (23)
  • client/packages/admin/src/index.ts
  • client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts
  • client/packages/core/__tests__/src/utils/e2e.ts
  • client/packages/core/package.json
  • client/packages/core/src/Reactor.js
  • client/packages/core/src/authAPI.ts
  • client/packages/core/src/index.ts
  • client/packages/core/vitest.config.ts
  • client/packages/create-instant-app/template/rules/AGENTS.md
  • client/packages/create-instant-app/template/rules/cursor-rules.md
  • client/packages/create-instant-app/template/rules/windsurf-rules.md
  • client/sandbox/react-nextjs/pages/play/oauth-extra-fields-google.tsx
  • client/sandbox/react-nextjs/pages/play/oauth-extra-fields.tsx
  • client/www/lib/intern/instant-rules.md
  • client/www/pages/docs/auth.md
  • client/www/pages/docs/backend.md
  • client/www/pages/docs/http-api.md
  • client/www/pages/docs/users.md
  • server/src/instant/admin/routes.clj
  • server/src/instant/model/app_user.clj
  • server/src/instant/runtime/magic_code_auth.clj
  • server/src/instant/runtime/routes.clj
  • server/test/instant/runtime/routes_test.clj
🚧 Files skipped from review as they are similar to previous changes (6)
  • client/packages/admin/src/index.ts
  • client/packages/core/vitest.config.ts
  • client/www/pages/docs/backend.md
  • client/packages/core/src/index.ts
  • client/packages/create-instant-app/template/rules/AGENTS.md
  • client/packages/create-instant-app/template/rules/cursor-rules.md

Comment on lines +20 to +31
const res = await fetch(`${apiUrl}/admin/magic_code`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'app-id': appId,
authorization: `Bearer ${adminToken}`,
},
body: JSON.stringify({ email }),
});
const data = await res.json();
return data.code;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fail fast when magic-code generation returns non-2xx.

The helper returns data.code without checking res.ok, which can mask the real failure source.

🛠️ Suggested fix
   const res = await fetch(`${apiUrl}/admin/magic_code`, {
@@
   });
+  if (!res.ok) {
+    throw new Error(await res.text());
+  }
   const data = await res.json();
+  if (!data?.code) {
+    throw new Error('Missing magic code in admin/magic_code response');
+  }
   return data.code;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts` around
lines 20 - 31, The helper currently returns data.code without checking the HTTP
response, so failures are masked; update the helper that calls fetch (the block
using apiUrl/admin/magic_code with appId and adminToken) to check res.ok and
throw a descriptive error when not OK (include res.status and the response body
or JSON) before returning data.code, so callers get a clear failure instead of
undefined.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
server/test/instant/runtime/routes_test.clj (2)

647-672: Remove unused app binding.

The static analyzer flagged an unused app binding at line 649. The test only uses app-id.

🧹 Suggested fix
 (deftest users-create-rule-oauth-test
   (with-empty-app
-    (fn [{app-id :id :as app}]
+    (fn [{app-id :id}]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/instant/runtime/routes_test.clj` around lines 647 - 672, The test
binding includes an unused :as app in the with-empty-app callback; update the
argument destructuring in users-create-rule-oauth-test from (fn [{app-id :id :as
app}]) to only capture app-id (e.g. (fn [{app-id :id}]) ) so the unused app
symbol is removed, leaving the rest of the test (rule-model/put!,
route/upsert-oauth-link!, assertions) unchanged.

486-518: Remove unused app binding.

The static analyzer flagged an unused app binding at line 488. The test only uses app-id.

🧹 Suggested fix
 (deftest extra-fields-oauth-test
   (with-empty-app
-    (fn [{app-id :id :as app}]
+    (fn [{app-id :id}]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/instant/runtime/routes_test.clj` around lines 486 - 518, The
anonymous function in extra-fields-oauth-test currently destructures its
argument as ({app-id :id :as app}) but never uses the app binding; remove the
unused :as app to silence the analyzer by changing the parameter destructuring
to ({app-id :id}) in the with-empty-app callback (the anonymous fn used inside
extra-fields-oauth-test) so only app-id is bound.
server/src/instant/db/permissioned_transaction.clj (1)

49-57: Remove unused eid binding.

The eid parameter is destructured but never used in the function body. The static analyzer correctly flagged this.

🧹 Suggested fix
-(defn- validate-system-create-entity! [{:keys [admin? attrs]} {:keys [aid eid] :as tx-step}]
+(defn- validate-system-create-entity! [{:keys [admin? attrs]} {:keys [aid] :as tx-step}]
   (let [attr (attr-model/seek-by-id aid attrs)
         [etype label] (attr-model/fwd-ident-name attr)]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/instant/db/permissioned_transaction.clj` around lines 49 - 57, The
destructured eid in the function validate-system-create-entity! is unused;
remove it from the destructuring vector (change {:keys [aid eid] :as tx-step} to
{:keys [aid] :as tx-step}) or rename it to _eid to silence the analyzer,
updating only the parameter list in validate-system-create-entity! so behavior
remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/www/pages/docs/users.md`:
- Around line 237-244: The code example uses an unimported bare symbol tx;
replace usages of tx inside the transaction array with db.tx so the example is
consistent with other docs (e.g., change tx.settings[id()] to
db.tx.settings[id()]) and ensure the db.transact call remains the same so the
transaction uses the namespaced transaction builder provided by db.

---

Nitpick comments:
In `@server/src/instant/db/permissioned_transaction.clj`:
- Around line 49-57: The destructured eid in the function
validate-system-create-entity! is unused; remove it from the destructuring
vector (change {:keys [aid eid] :as tx-step} to {:keys [aid] :as tx-step}) or
rename it to _eid to silence the analyzer, updating only the parameter list in
validate-system-create-entity! so behavior remains unchanged.

In `@server/test/instant/runtime/routes_test.clj`:
- Around line 647-672: The test binding includes an unused :as app in the
with-empty-app callback; update the argument destructuring in
users-create-rule-oauth-test from (fn [{app-id :id :as app}]) to only capture
app-id (e.g. (fn [{app-id :id}]) ) so the unused app symbol is removed, leaving
the rest of the test (rule-model/put!, route/upsert-oauth-link!, assertions)
unchanged.
- Around line 486-518: The anonymous function in extra-fields-oauth-test
currently destructures its argument as ({app-id :id :as app}) but never uses the
app binding; remove the unused :as app to silence the analyzer by changing the
parameter destructuring to ({app-id :id}) in the with-empty-app callback (the
anonymous fn used inside extra-fields-oauth-test) so only app-id is bound.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7c39d75a-2754-4a51-8164-e9f96be02124

📥 Commits

Reviewing files that changed from the base of the PR and between 1787a3e and 9229700.

📒 Files selected for processing (13)
  • client/packages/create-instant-app/template/rules/AGENTS.md
  • client/packages/create-instant-app/template/rules/cursor-rules.md
  • client/packages/create-instant-app/template/rules/windsurf-rules.md
  • client/sandbox/react-nextjs/pages/play/signup-rules.tsx
  • client/www/lib/intern/instant-rules.md
  • client/www/pages/docs/users.md
  • server/src/instant/admin/routes.clj
  • server/src/instant/db/permissioned_transaction.clj
  • server/src/instant/model/app_user.clj
  • server/src/instant/model/rule.clj
  • server/src/instant/runtime/magic_code_auth.clj
  • server/src/instant/runtime/routes.clj
  • server/test/instant/runtime/routes_test.clj
✅ Files skipped from review due to trivial changes (1)
  • client/packages/create-instant-app/template/rules/cursor-rules.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • client/packages/create-instant-app/template/rules/windsurf-rules.md
  • server/src/instant/runtime/routes.clj
  • server/src/instant/admin/routes.clj
  • client/packages/create-instant-app/template/rules/AGENTS.md

@nezaj nezaj merged commit b1ac7ef into main Mar 20, 2026
33 checks passed
@nezaj nezaj deleted the extra-fields branch March 20, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants