Skip to content

fix(auth): set up new user defaults in betterAuth hook#3806

Merged
capJavert merged 4 commits intomainfrom
fix/betterauth-new-user-defaults
Apr 18, 2026
Merged

fix(auth): set up new user defaults in betterAuth hook#3806
capJavert merged 4 commits intomainfrom
fix/betterauth-new-user-defaults

Conversation

@capJavert
Copy link
Copy Markdown
Contributor

src/betterAuth.ts

  • resolveSignUpUserId + ensureNonDeletedUserId helpers — deleted-user collision handled once at the end of the before hook
  • user.create.after now sets: feed (id = user.id), notificationFlags (marketing muted when acceptedMarketing=false), UserAction CheckedCoresRole, DigestPost
    flags/referral/timezone/coresRole UPDATE
  • Inline calls to addClaimableItemsToUser + claimAnonOpportunities (each in its own try/catch)

src/entity/user/utils.ts

  • addClaimableItemsToUser signature narrowed to Pick<AddUserData, 'id' | 'email'>

Port the side effects from the old /newUser route that were missing from
the betterAuth user.create flow: default feed (id = user.id),
notificationFlags (respecting acceptedMarketing), UserAction
CheckedCoresRole, DigestPost stub, claimable Plus items, anonymous
opportunity claims, and deleted-user id collision handling in the
before hook.
@pulumi
Copy link
Copy Markdown

pulumi Bot commented Apr 17, 2026

🍹 The Update (preview) for dailydotdev/api/prod (at 350a1db) was successful.

✨ Neo Explanation

Routine deployment of commit `6098242c` across all API services: container images, version labels, and migration Jobs are updated fleet-wide with no infrastructure changes. ✅ Low Risk

This is a standard application deployment rolling out commit 350a1db (short: 6098242c) across all workloads to replace the previous commit b024649c. Every deployment and cron job is updated solely to point to the new container image and bump the version label/env var.

The two migration Jobs (DB and ClickHouse) follow the expected pattern for this stack: old Jobs tied to the previous commit hash are deleted and new ones are created for the new commit. The new migration Jobs will run against the primary database and ClickHouse before (or alongside) the rolling deployment updates — confirm the migration Jobs complete successfully before assuming the rollout is healthy.

🔵 Info — The GCP provider warning about missing ADC credentials is a diagnostic noise item in the preview environment and does not affect the Kubernetes resources being deployed.

Resource Changes

    Name                                                       Type                           Operation
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-validate-active-users-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-highlights-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-squad-posts-analytics-refresh-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-materialize-monthly-best-post-archives-cron     kubernetes:batch/v1:CronJob    update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-digests-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-update-highlighted-views-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-update-tag-materialized-views-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-updated-sync-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-expired-better-auth-sessions-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-update-current-streak-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-update-achievement-rarity-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob    update
+   vpc-native-api-db-migration-6098242c                       kubernetes:batch/v1:Job        create
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob    update
-   vpc-native-api-clickhouse-migration-b024649c               kubernetes:batch/v1:Job        delete
~   vpc-native-update-source-public-threshold-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment  update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment  update
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-update-tags-str-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-user-posts-analytics-refresh-cron               kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment  update
~   vpc-native-post-analytics-history-day-clickhouse-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-deployment                                      kubernetes:apps/v1:Deployment  update
... and 11 other changes

@capJavert
Copy link
Copy Markdown
Contributor Author

@claude review

@claude
Copy link
Copy Markdown

claude Bot commented Apr 18, 2026

Claude finished @capJavert's task in 1m 33s —— View job


Code Review

  • Gather context
  • Read changed files
  • Analyze code quality, correctness, and best practices
  • Post review feedback

Overall this is a clean refactor and good addition. The extraction of resolveSignUpUserId and ensureNonDeletedUserId improves readability, the feed fix (gen_random_uuid()$1) is a real bug fix, and the per-operation try/catch for addClaimableItemsToUser / claimAnonOpportunities is the right pattern. The test coverage is solid. A few things worth flagging:


src/betterAuth.ts

triggerTypedEvent can be skipped on errors in after hookinsertOrIgnoreAction and DigestPost creation have no individual try/catch, so any failure inside them bubbles up to the outer catch and skips triggerTypedEvent. Since ba-user-created drives downstream processing (notifications, etc.), a DigestPost insert failure silently drops that event for a real user. addClaimableItemsToUser and claimAnonOpportunities are correctly isolated, but the two items above should be too.

// Current: DigestPost failure skips triggerTypedEvent
await insertOrIgnoreAction(...);   // ← uncaught → outer catch skips everything below
await DigestPost.insert(...);      // ← uncaught

try { await addClaimableItemsToUser(...) } catch { ... }
try { for (...) await claimAnonOpportunities(...) } catch { ... }

await triggerTypedEvent(...);   // ← SKIPPED if either uncaught line throws

Fix this →


ensureNonDeletedUserId silently returns a still-colliding ID — after exhausting MAX_DELETED_USER_COLLISION_RETRIES, the function returns id which may still be in deleted_user (the last regenerated candidate). Practically impossible, but worth at least a logger.warn before returning. [src/betterAuth.ts:314–317]

  }
  return id;  // ← potentially still a deleted user ID; no log
};

claimAnonOpportunities sequential loop — the two identifiers (user.id, user.email) are independent; Promise.all would be marginally cleaner and avoids one blocking the other. [src/betterAuth.ts:616–624]


__tests__/routes/betterAuth.ts

Duplicate import from same moduleSource and DIGEST_SOURCE are imported from ../../src/entity/Source in two separate lines. They should be merged into one. [__tests__/routes/betterAuth.ts:7,12]

// Current
import { Source } from '../../src/entity/Source';
...
import { DIGEST_SOURCE } from '../../src/entity/Source';

// Should be
import { Source, DIGEST_SOURCE } from '../../src/entity/Source';

Non-null assertions (!) — multiple test assertions use feed!.id, digestPost!.sourceId, persisted!.notificationFlags, updatedClaimableItem!.claimedAt, etc. Per CLAUDE.md guidelines, these should be replaced with explicit checks (expect(feed).not.toBeNull(); if (!feed) return; or just rely on the preceding expect(...).not.toBeNull() and cast). Not blocking but inconsistent with the codebase standard.


src/entity/user/utils.ts

Narrowing body to Pick<AddUserData, 'id' | 'email'> is a clean, correct change. No issues.


Summary: The feed ID fix, deleted-user collision guard, and marketing notification defaults are all solid. The main concern is that triggerTypedEvent is not guarded against insertOrIgnoreAction / DigestPost insertion failures — if either throws, the ba-user-created event is silently dropped for a real user.

@capJavert capJavert enabled auto-merge (squash) April 18, 2026 07:42
@capJavert capJavert merged commit 3c0d51c into main Apr 18, 2026
10 checks passed
@capJavert capJavert deleted the fix/betterauth-new-user-defaults branch April 18, 2026 07:56
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.

2 participants