🕒 feat: Track terms acceptance timestamp - #10810
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds tracking for when users accept the terms of service by introducing a termsAcceptedAt timestamp field. The feature maintains consistency across the full stack while providing migration tooling for legacy users.
- Adds
termsAcceptedAtfield to user schema and type definitions - Updates API endpoints to track and return the acceptance timestamp
- Provides migration script to backfill timestamps for existing users who accepted terms
- Updates utility scripts (reset-terms, list-users) to handle the new field
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-schemas/src/types/user.ts | Adds termsAcceptedAt?: Date | null to IUser and UpdateUserRequest interfaces |
| packages/data-schemas/src/schema/user.ts | Adds termsAcceptedAt field to Mongoose schema with Date type and null default |
| packages/data-provider/src/types.ts | Updates TUserTermsResponse to include termsAcceptedAt field |
| packages/api/src/utils/env.ts | Adds termsAcceptedAt to allowed user fields list |
| config/reset-terms.js | Updates reset script to clear both termsAccepted and termsAcceptedAt |
| config/migrate-terms-timestamp.js | Adds migration script to backfill timestamps using createdAt as fallback |
| config/list-users.js | Updates user listing to display terms acceptance information |
| package.json | Adds migrate:terms-timestamp npm script |
| client/src/data-provider/mutations.ts | Updates query cache to include termsAcceptedAt when terms are accepted |
| api/server/controllers/UserController.js | Updates controllers to set termsAcceptedAt when terms are accepted and return it in status queries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
34b366c to
ef96ce2
Compare
d6d6125 to
5d797f5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'appleId', | ||
| 'emailVerified', | ||
| 'twoFactorEnabled', | ||
| 'termsAccepted', | ||
| 'termsAcceptedAt', | ||
| ] as const; |
There was a problem hiding this comment.
The comment says these are "non-sensitive string/boolean fields", but the newly added termsAcceptedAt is a Date. Please update the comment to reflect that date fields may be included (or adjust the allowed fields list/comment to match).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
5d797f5 to
7cb06f6
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cb06f6554
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rrors Preserve the original termsAcceptedAt on repeat accepts within a terms cycle so retried or duplicate requests no longer overwrite the first acceptance time. Exit the migration script with a non-zero status when any per-user update fails so partial failures are not reported as successful.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f08045e58e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… time Replace the read-then-write in acceptTermsController with a single atomic acceptTerms method that conditionally stamps termsAcceptedAt via an $ifNull aggregation update. This removes the TOCTOU window where two concurrent first-time accepts could overwrite the earlier acceptance timestamp, while still preserving an existing timestamp and backfilling legacy accepted users.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c98f865a57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Wrap the count, cursor scan, and per-user updates in runAsSystem so the tenant isolation plugin does not throw under TENANT_ISOLATION_STRICT or scope the cross-tenant migration to a non-existent tenant, matching the other maintenance migrations.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58e01eb7ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add the missing-timestamp predicate to the per-user updateOne filter so a user who accepts through the API between the cursor read and the write keeps their real acceptance time instead of being overwritten with createdAt. Track modified vs skipped so the summary reflects skips.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c7eaf89ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add termsAccepted: true to the per-user updateOne filter so a reset that clears acceptance between the cursor read and the write is not re-stamped with createdAt, which would otherwise poison the next acceptance cycle through the $ifNull preserve in acceptTerms.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Merged 113 upstream commits from v0.8.6 (9efe487) up to upstream/main HEAD ac759ef — v0.8.7 (tag 9e74cc0) plus 28 post-release commits. Gains Claude Sonnet 5 support (LibreChat-AI#14042), Memory as an agent capability (LibreChat-AI#13869), and HITL tool approval (LibreChat-AI#12938/LibreChat-AI#13942). Conflicts resolved (2): - client/src/components/Chat/Input/ToolsDropdown.tsx — kept upstream's new Memory dropdown block live; re-applied our commented-out Code Interpreter removal around only the `if (canRunCode && codeEnabled)` block. - package-lock.json — regenerated from upstream's lockfile + npm install + npm install --package-lock-only to re-add @emnapi/{core,runtime,wasi-threads}; verified clean with `npm ci --dry-run` (Node 24 / npm 11). Bookkeeping: bumped custom/overrides/upstream-version.txt; added a v0.8.7 sync section to custom/MODIFICATIONS.md documenting the resolution, the lockfile recipe, the pending migrate:terms-timestamp migration (LibreChat-AI#10810, run at next deploy), and two known non-blocking test failures (an upstream memoization timing test that flakes under our lazy Stripe-loading; upstream Anthropic-API integration tests that need network). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: add terms acceptance timestamp tracking and migration script * feat: update migration script to use countUsers method for user count * Update config/migrate-terms-timestamp.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: enhance terms acceptance response to include acceptance timestamp * fix: make terms acceptance idempotent and fail migration on partial errors Preserve the original termsAcceptedAt on repeat accepts within a terms cycle so retried or duplicate requests no longer overwrite the first acceptance time. Exit the migration script with a non-zero status when any per-user update fails so partial failures are not reported as successful. * style: fix import ordering in data-provider mutations * refactor: record terms acceptance atomically to preserve first-accept time Replace the read-then-write in acceptTermsController with a single atomic acceptTerms method that conditionally stamps termsAcceptedAt via an $ifNull aggregation update. This removes the TOCTOU window where two concurrent first-time accepts could overwrite the earlier acceptance timestamp, while still preserving an existing timestamp and backfilling legacy accepted users. * fix: run terms timestamp migration under system tenant context Wrap the count, cursor scan, and per-user updates in runAsSystem so the tenant isolation plugin does not throw under TENANT_ISOLATION_STRICT or scope the cross-tenant migration to a non-existent tenant, matching the other maintenance migrations. * fix: guard terms backfill against concurrent acceptances Add the missing-timestamp predicate to the per-user updateOne filter so a user who accepts through the API between the cursor read and the write keeps their real acceptance time instead of being overwritten with createdAt. Track modified vs skipped so the summary reflects skips. * fix: scope terms backfill to still-accepted users Add termsAccepted: true to the per-user updateOne filter so a reset that clears acceptance between the cursor read and the write is not re-stamped with createdAt, which would otherwise poison the next acceptance cycle through the $ifNull preserve in acceptTerms. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* feat: add terms acceptance timestamp tracking and migration script * feat: update migration script to use countUsers method for user count * Update config/migrate-terms-timestamp.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: enhance terms acceptance response to include acceptance timestamp * fix: make terms acceptance idempotent and fail migration on partial errors Preserve the original termsAcceptedAt on repeat accepts within a terms cycle so retried or duplicate requests no longer overwrite the first acceptance time. Exit the migration script with a non-zero status when any per-user update fails so partial failures are not reported as successful. * style: fix import ordering in data-provider mutations * refactor: record terms acceptance atomically to preserve first-accept time Replace the read-then-write in acceptTermsController with a single atomic acceptTerms method that conditionally stamps termsAcceptedAt via an $ifNull aggregation update. This removes the TOCTOU window where two concurrent first-time accepts could overwrite the earlier acceptance timestamp, while still preserving an existing timestamp and backfilling legacy accepted users. * fix: run terms timestamp migration under system tenant context Wrap the count, cursor scan, and per-user updates in runAsSystem so the tenant isolation plugin does not throw under TENANT_ISOLATION_STRICT or scope the cross-tenant migration to a non-existent tenant, matching the other maintenance migrations. * fix: guard terms backfill against concurrent acceptances Add the missing-timestamp predicate to the per-user updateOne filter so a user who accepts through the API between the cursor read and the write keeps their real acceptance time instead of being overwritten with createdAt. Track modified vs skipped so the summary reflects skips. * fix: scope terms backfill to still-accepted users Add termsAccepted: true to the per-user updateOne filter so a reset that clears acceptance between the cursor read and the write is not re-stamped with createdAt, which would otherwise poison the next acceptance cycle through the $ifNull preserve in acceptTerms. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* feat: add terms acceptance timestamp tracking and migration script * feat: update migration script to use countUsers method for user count * Update config/migrate-terms-timestamp.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: enhance terms acceptance response to include acceptance timestamp * fix: make terms acceptance idempotent and fail migration on partial errors Preserve the original termsAcceptedAt on repeat accepts within a terms cycle so retried or duplicate requests no longer overwrite the first acceptance time. Exit the migration script with a non-zero status when any per-user update fails so partial failures are not reported as successful. * style: fix import ordering in data-provider mutations * refactor: record terms acceptance atomically to preserve first-accept time Replace the read-then-write in acceptTermsController with a single atomic acceptTerms method that conditionally stamps termsAcceptedAt via an $ifNull aggregation update. This removes the TOCTOU window where two concurrent first-time accepts could overwrite the earlier acceptance timestamp, while still preserving an existing timestamp and backfilling legacy accepted users. * fix: run terms timestamp migration under system tenant context Wrap the count, cursor scan, and per-user updates in runAsSystem so the tenant isolation plugin does not throw under TENANT_ISOLATION_STRICT or scope the cross-tenant migration to a non-existent tenant, matching the other maintenance migrations. * fix: guard terms backfill against concurrent acceptances Add the missing-timestamp predicate to the per-user updateOne filter so a user who accepts through the API between the cursor read and the write keeps their real acceptance time instead of being overwritten with createdAt. Track modified vs skipped so the summary reflects skips. * fix: scope terms backfill to still-accepted users Add termsAccepted: true to the per-user updateOne filter so a reset that clears acceptance between the cursor read and the write is not re-stamped with createdAt, which would otherwise poison the next acceptance cycle through the $ifNull preserve in acceptTerms. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
This pull request introduces tracking for the timestamp when users accept the terms of service. It updates the user schema and related APIs to support a new
termsAcceptedAtfield, ensures this information is handled consistently across the backend and frontend, and provides migration and reset scripts to manage legacy and future user dataCloses #9350
Change Type
Testing
Tested on Firefox and Chrome
Test Configuration:
Checklist