Skip to content

fix(convex-plugin): await async definePayload return value#336

Closed
agucova wants to merge 1 commit into
get-convex:mainfrom
agucova:fix/async-define-payload
Closed

fix(convex-plugin): await async definePayload return value#336
agucova wants to merge 1 commit into
get-convex:mainfrom
agucova:fix/async-define-payload

Conversation

@agucova

@agucova agucova commented Apr 15, 2026

Copy link
Copy Markdown

Fixes #335

Summary

The convex plugin's JWT definePayload wrapper in src/plugins/convex/index.ts spreads the user-supplied definePayload return value synchronously. When a consumer returns a Promise (as the public type signature explicitly permits), the spread yields nothing and custom JWT claims are silently dropped, and only the plugin's own sessionId and iat survive into the JWT.

This PR makes the wrapper async and awaits the inner call.

Why

The public type allows async:

definePayload?: (session: { user, session }) =>
  Promise<Record<string, any>> | Record<string, any> | undefined;

The upstream better-auth jwt plugin awaits definePayload once at the outer call site (better-auth@1.4.9, dist/plugins/jwt/sign.mjs:53), so it awaits the convex plugin's sync wrapper, not the inner user function. Result: ...Promise spreads no enumerable own properties, and every custom claim is dropped. A Convex runtime warning (unawaited operation: [query]) also appears when the user's async definePayload does DB work.

The change (4 characters)

-      definePayload: ({ user, session }) => ({
+      definePayload: async ({ user, session }) => ({
         ...(opts.jwt?.definePayload
-          ? opts.jwt.definePayload({ user, session })
+          ? await opts.jwt.definePayload({ user, session })
           : omit(user, ["id", "image"])),
         sessionId: session.id,
         iat: Math.floor(new Date().getTime() / 1000),
       }),

Synchronous definePayload remains correct: await on a non-Promise is a no-op, and the outer better-auth code already awaits the wrapper.

Test plan

  • npx tsc --noEmit passes
  • npm test — failure count unchanged vs main (111 pre-existing adapter test failures on both branches; no test regressions introduced). There are no existing tests that exercise definePayload; happy to add one if maintainers prefer.
  • Manually verified in a downstream app (@convex-dev/better-auth@0.10.10 patched with this change): async definePayload returning { roles, programs, email } now populates those claims in the JWT returned by /api/auth/token and /api/auth/get-session; without the patch the claims are absent.

Possibly closes

May also be the root cause of #291 (First generated JWT is missing custom payload fields), see issue description for details.

Summary by CodeRabbit

  • Refactor
    • Enhanced JWT payload configuration in Convex authentication plugin to support asynchronous functions while maintaining backward compatibility.

The convex plugin's JWT definePayload wrapper spreads the user-supplied
definePayload return value synchronously. When the user returns a Promise
(as the type signature allows), spreading it yields nothing useful and
all custom JWT claims are silently dropped. Only the plugin's own
sessionId and iat additions survive into the JWT.

Make the wrapper async and await the inner call. Sync definePayload
continues to work because awaiting a non-promise is a no-op.
@vercel

vercel Bot commented Apr 15, 2026

Copy link
Copy Markdown

@agucova is attempting to deploy a commit to the Convex Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Apr 15, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The Convex plugin's JWT definePayload wrapper function is modified to be async and properly await the result of the user-supplied definePayload function, if provided. This ensures custom JWT claims are not dropped when the function returns a Promise.

Changes

Cohort / File(s) Summary
Convex JWT async handling
src/plugins/convex/index.ts
Made definePayload wrapper function async and added await when invoking opts.jwt.definePayload() to properly handle both synchronous and asynchronous implementations.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A promise was made but never awaited,
Custom claims silently abated!
Now async flows swift and true,
The Convex plugin works like new ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: converting definePayload to async and awaiting its return value, which directly addresses the core issue.
Linked Issues check ✅ Passed The pull request fully implements the proposed fix from #335: making the wrapper async and awaiting opts.jwt.definePayload before spreading, while maintaining backward compatibility with synchronous functions.
Out of Scope Changes check ✅ Passed All changes are scoped to the fix: only the definePayload wrapper in src/plugins/convex/index.ts is modified to await the async call, with no unrelated alterations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

@erquhart

Copy link
Copy Markdown
Member

Thanks for this - we ended up fixing in #323, will be released in 0.12.0.

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.

Convex plugin silently drops custom JWT claims when definePayload is async

2 participants