From 23d1ad473daa82c9caa41d80e10f25582f0acca5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 28 Jul 2026 22:14:34 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8D=83=20fix:=20Amazon=20DocumentDB=20Com?= =?UTF-8?q?patibility=20for=20Pipeline-Form=20Updates=20(#14495)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🍃 fix: Amazon DocumentDB Compatibility for Pipeline-Form Updates - Rewrite acceptTerms without aggregation-pipeline update + $$NOW (null-guarded first-acceptance claim preserves the original timestamp under concurrent and repeat requests) - Rewrite decrementTagCounts clamp-at-zero decrement as ordered two-op bulkWrite (clamp before guarded $inc) - Rewrite extendFilesTTL TTL hold as projected read + per-doc guarded $set via tenantSafeBulkWrite, preserving only-widens/ceiling/cleared-stays-permanent semantics - Log background index-build failures via Model 'index' listeners (previously swallowed silently, e.g. partialFilterExpression rejection on DocumentDB <5.0) - Add misc/documentdb live-compatibility harness + assessment (AWS-cited) - Fix latent file.spec helper bug: createdAt backdating was silently stripped by mongoose immutability Closes #14488 * 🍃 fix: Harden DocumentDB-Safe Updates Against Cross-Call Races Addresses Codex review on #14495: - decrementTagCounts: normalize-null / $inc / clamp-negative op triple so interleaved decrements of the same tag converge on max(0, ...) exactly as the serialized pipeline did (clamp keys on count < 0, not count < amount) - acceptTerms: guard the repeat-acceptance fallback with a non-null timestamp (exact complement of the claim guard) and retry the claim when a config/reset-terms.js reset races between the two updates, so acceptance never resurrects a reset cycle without a fresh audit timestamp * 👷 ci: Suppress Ignored-File Warnings in Changed-File ESLint Run Changed files under config-ignored paths (packages/data-schemas/misc/**) emit "File ignored" warnings that fail --max-warnings=0. --- .github/workflows/eslint-ci.yml | 3 + .../misc/documentdb/compat.documentdb.spec.ts | 234 ++++++++++++++++++ .../misc/documentdb/documentdb-compat.md | 195 +++++++++++++++ .../documentdb/jest.documentdb.config.mjs | 20 ++ .../misc/documentdb/tsconfig.json | 13 + .../methods/conversationTag.methods.spec.ts | 87 ++++++- .../src/methods/conversationTag.ts | 40 ++- .../data-schemas/src/methods/file.spec.ts | 32 ++- packages/data-schemas/src/methods/file.ts | 60 +++-- .../src/methods/user.methods.spec.ts | 59 +++++ packages/data-schemas/src/methods/user.ts | 53 ++-- packages/data-schemas/src/models/index.ts | 18 +- 12 files changed, 757 insertions(+), 57 deletions(-) create mode 100644 packages/data-schemas/misc/documentdb/compat.documentdb.spec.ts create mode 100644 packages/data-schemas/misc/documentdb/documentdb-compat.md create mode 100644 packages/data-schemas/misc/documentdb/jest.documentdb.config.mjs create mode 100644 packages/data-schemas/misc/documentdb/tsconfig.json diff --git a/.github/workflows/eslint-ci.yml b/.github/workflows/eslint-ci.yml index 115ad522dd..20c0aaf876 100644 --- a/.github/workflows/eslint-ci.yml +++ b/.github/workflows/eslint-ci.yml @@ -61,8 +61,11 @@ jobs: fi # Run ESLint + # --no-warn-ignored: changed files under config-ignored paths + # (e.g. packages/data-schemas/misc/**) must not fail --max-warnings=0 npx eslint --no-error-on-unmatched-pattern \ --config eslint.config.mjs \ + --no-warn-ignored \ --max-warnings=0 \ -- "${CHANGED_FILES[@]}" diff --git a/packages/data-schemas/misc/documentdb/compat.documentdb.spec.ts b/packages/data-schemas/misc/documentdb/compat.documentdb.spec.ts new file mode 100644 index 0000000000..e31db558c7 --- /dev/null +++ b/packages/data-schemas/misc/documentdb/compat.documentdb.spec.ts @@ -0,0 +1,234 @@ +import mongoose from 'mongoose'; +import { randomUUID } from 'crypto'; +import type { ConnectOptions, Model } from 'mongoose'; +import type { IConversationTag } from '~/schema/conversationTag'; +import type * as t from '~/types'; +import { decrementTagCounts } from '~/methods/conversationTag'; +import { supportsTransactions } from '~/utils/transactions'; +import { createUserMethods } from '~/methods/user'; +import { createFileMethods } from '~/methods/file'; +import { createModels } from '~/models'; + +/** + * Amazon DocumentDB live-compatibility suite. + * + * Exercises the operations that have historically broken on DocumentDB + * (aggregation-pipeline updates — issue #14488) against a REAL cluster, plus + * informational capability probes whose results print as a matrix at the end. + * There is no faithful local emulator of Amazon DocumentDB (the open-source + * "DocumentDB Local" image is an unrelated PostgreSQL-based engine), so this + * suite only runs when DOCUMENTDB_URI is set and skips otherwise. + * + * Run (from packages/data-schemas, against a DEDICATED database): + * DOCUMENTDB_URI="mongodb://user:pass@127.0.0.1:27017/librechat_compat?tls=true&retryWrites=false" \ + * DOCUMENTDB_TLS_CA_FILE="global-bundle.pem" \ + * npx jest --config misc/documentdb/jest.documentdb.config.mjs + * + * Through an SSH tunnel, additionally set + * DOCUMENTDB_TLS_ALLOW_INVALID_HOSTNAMES=true + * because the tunnel endpoint will not match the cluster certificate. + * + * Set DOCUMENTDB_EXPECT_PARTIAL_INDEXES=true when targeting DocumentDB 5.0+ + * instance-based clusters to turn the partial-index probe into a hard assertion. + */ +const DOCUMENTDB_URI = process.env.DOCUMENTDB_URI ?? ''; +const describeLive = DOCUMENTDB_URI ? describe : describe.skip; + +const HOUR = 3_600_000; +const runId = randomUUID().slice(0, 8); +const capabilities: Record = {}; + +function getDb() { + const db = mongoose.connection.db; + if (!db) { + throw new Error('MongoDB database handle not available'); + } + return db; +} + +describeLive('Amazon DocumentDB live compatibility', () => { + let User: Model; + let ConversationTag: Model; + let userMethods: ReturnType; + let fileMethods: ReturnType; + + const testEmail = (label: string) => `${label}-${runId}@compat.test`; + + beforeAll(async () => { + const options: ConnectOptions = { autoIndex: false, autoCreate: false }; + if (process.env.DOCUMENTDB_TLS_CA_FILE) { + options.tlsCAFile = process.env.DOCUMENTDB_TLS_CA_FILE; + } + if (process.env.DOCUMENTDB_TLS_ALLOW_INVALID_HOSTNAMES === 'true') { + options.tlsAllowInvalidHostnames = true; + } + await mongoose.connect(DOCUMENTDB_URI, options); + + const models = createModels(mongoose); + Object.assign(mongoose.models, models); + User = mongoose.models.User; + ConversationTag = mongoose.models.ConversationTag; + userMethods = createUserMethods(mongoose); + fileMethods = createFileMethods(mongoose); + }); + + afterAll(async () => { + if (mongoose.connection.readyState === 1) { + await User.deleteMany({ email: { $regex: runId } }); + await ConversationTag.deleteMany({ tag: { $regex: runId } }); + await mongoose.models.File.deleteMany({ filename: { $regex: runId } }); + + const rows = Object.entries(capabilities).map( + ([capability, verdict]) => ` ${capability.padEnd(36)} ${verdict}`, + ); + console.log(`\nDocumentDB capability matrix (run ${runId}):\n${rows.join('\n')}\n`); + await mongoose.disconnect(); + } + }); + + describe('pipeline-form updates (bug class behind #14488)', () => { + it('records whether the engine accepts pipeline updates and $$NOW', async () => { + const probe = getDb().collection(`pipeline_probe_${runId}`); + await probe.insertOne({ probe: 1 }); + + capabilities['pipeline-form updateOne'] = await probe + .updateOne({ probe: 1 }, [{ $set: { probed: true } }]) + .then(() => 'supported') + .catch((error: Error) => `rejected (${error.message})`); + capabilities['$$NOW system variable'] = await probe + .updateOne({ probe: 1 }, [{ $set: { probedAt: '$$NOW' } }]) + .then(() => 'supported') + .catch((error: Error) => `rejected (${error.message})`); + + await probe.drop().catch(() => undefined); + expect(capabilities['pipeline-form updateOne']).toBeDefined(); + }); + + it('acceptTerms stamps once and preserves the first timestamp', async () => { + const user = await User.create({ + name: 'DocDB Terms', + email: testEmail('terms'), + provider: 'local', + }); + const userId = String(user._id); + + const first = await userMethods.acceptTerms(userId); + expect(first?.termsAccepted).toBe(true); + expect(first?.termsAcceptedAt).toBeInstanceOf(Date); + + const repeat = await userMethods.acceptTerms(userId); + expect((repeat?.termsAcceptedAt as Date).getTime()).toBe( + (first?.termsAcceptedAt as Date).getTime(), + ); + }); + + it('acceptTerms converges under concurrent requests', async () => { + const user = await User.create({ + name: 'DocDB Concurrent', + email: testEmail('concurrent'), + provider: 'local', + }); + const userId = String(user._id); + + const results = await Promise.all( + Array.from({ length: 5 }, () => userMethods.acceptTerms(userId)), + ); + + expect(results.every((result) => result?.termsAccepted === true)).toBe(true); + const stamped = new Set(results.map((result) => (result?.termsAcceptedAt as Date).getTime())); + expect(stamped.size).toBe(1); + }); + + it('decrementTagCounts clamps at zero', async () => { + const user = `docdb-user-${runId}`; + const tag = `tag-${runId}`; + await ConversationTag.create({ user, tag, position: 1, count: 1 }); + + await decrementTagCounts(mongoose, user, [tag, tag, tag]); + + const stored = await ConversationTag.findOne({ user, tag }).lean(); + expect(stored?.count).toBe(0); + }); + + it('extendFilesTTL widens toward the window and clamps to the ceiling', async () => { + const userId = new mongoose.Types.ObjectId(); + const fileId = randomUUID(); + await fileMethods.createFile({ + file_id: fileId, + user: userId, + filename: `${fileId}-${runId}.txt`, + filepath: `/uploads/${fileId}.txt`, + type: 'text/plain', + bytes: 1, + }); + await mongoose.models.File.updateOne( + { file_id: fileId }, + { $set: { expiresAt: new Date(Date.now() + 60_000) } }, + { timestamps: false }, + ); + + const hold = { renewMs: 24 * HOUR, maxLifetimeMs: 48 * HOUR }; + const widened = await fileMethods.extendFilesTTL([fileId], hold, { user: String(userId) }); + expect(widened).toBe(1); + + const stored = await mongoose.models.File.findOne({ file_id: fileId }).lean<{ + createdAt: Date; + expiresAt?: Date; + }>(); + expect(stored?.expiresAt).toBeDefined(); + expect(stored!.expiresAt!.getTime()).toBeGreaterThan(Date.now() + 23 * HOUR); + expect(stored!.expiresAt!.getTime()).toBeLessThanOrEqual( + stored!.createdAt.getTime() + hold.maxLifetimeMs, + ); + }); + }); + + describe('capability probes (informational)', () => { + it('probes multi-document transaction support', async () => { + const supported = await supportsTransactions(mongoose); + capabilities['multi-document transactions'] = supported + ? 'supported' + : 'unsupported (runtime fallback engages)'; + expect(typeof supported).toBe('boolean'); + }); + + it('probes partial unique index support (OAuth id uniqueness relies on it)', async () => { + const probe = getDb().collection(`partial_index_probe_${runId}`); + await probe.insertOne({ seeded: true }); + + const outcome = await probe + .createIndex( + { googleId: 1, tenantId: 1 }, + { unique: true, partialFilterExpression: { googleId: { $exists: true } } }, + ) + .then(() => 'supported') + .catch((error: Error) => `REJECTED (${error.message})`); + capabilities['partial unique indexes'] = outcome; + + await probe.drop().catch(() => undefined); + if (process.env.DOCUMENTDB_EXPECT_PARTIAL_INDEXES === 'true') { + expect(outcome).toBe('supported'); + } + }); + + it('verifies TTL index support (session/token expiry relies on it)', async () => { + const probe = getDb().collection(`ttl_probe_${runId}`); + await probe.insertOne({ createdAt: new Date() }); + + await expect( + probe.createIndex({ createdAt: 1 }, { expireAfterSeconds: 60 }), + ).resolves.toBeDefined(); + capabilities['TTL indexes'] = 'supported'; + + await probe.drop().catch(() => undefined); + }); + + it('flags a connection string missing retryWrites=false', () => { + const disabled = /retryWrites=false/i.test(DOCUMENTDB_URI); + capabilities['retryWrites=false in URI'] = disabled + ? 'present' + : 'MISSING — DocumentDB rejects retryable writes'; + expect(typeof disabled).toBe('boolean'); + }); + }); +}); diff --git a/packages/data-schemas/misc/documentdb/documentdb-compat.md b/packages/data-schemas/misc/documentdb/documentdb-compat.md new file mode 100644 index 0000000000..d3569d9687 --- /dev/null +++ b/packages/data-schemas/misc/documentdb/documentdb-compat.md @@ -0,0 +1,195 @@ +# Amazon DocumentDB Compatibility Assessment (issue #14488) + +Adjudicated against official AWS documentation on 2026-07-28. Engine columns +throughout: DocumentDB **3.6 / 4.0 / 5.0 / 8.0 instance-based** and **elastic +clusters**. AWS's supported-APIs page states that unsupported operators are +*omitted* from its tables, so several verdicts below are implicit-by-omission +and flagged as such. + +## Executive summary + +- The login blocker was real: three aggregation-pipeline updates (one using + `$$NOW`) existed in the codebase. DocumentDB documents no support for + pipeline-form updates on any engine version. All three are now rewritten + with plain update operators that work on every engine, including elastic. +- Partial-index creation fails on DocumentDB < 5.0 and elastic — and the + failure was **provably silent** (no log, no crash, uniqueness quietly + unenforced). Model creation now attaches an `index` listener so failed + builds log loudly. +- Transactions already degrade gracefully via a runtime probe. GridFS is + unreachable dead code. `retryWrites=false` is a deployment-docs item. +- Recommendation: **DocumentDB 5.0+ instance-based is a supportable target**; + 4.0 runs but with degraded uniqueness enforcement (now logged); **elastic + clusters should be documented as unsupported** (no unique indexes at all). + +## Proven incompatibilities — fixed in this PR + +### 1. `acceptTerms` pipeline update + `$$NOW` (P0 — blocked login) + +`packages/data-schemas/src/methods/user.ts:303` used a pipeline-form +`findByIdAndUpdate` with `$ifNull`/`$$NOW` (introduced by PR #10810, matching +the reporter's regression window). When Terms gating is on, every login hits +this and DocumentDB rejects it. + +AWS evidence: the [supported APIs page](https://docs.aws.amazon.com/documentdb/latest/developerguide/mongo-apis.html) +lists only classic update operators (no pipeline form anywhere); the `$set`/ +`$unset` *stage* operators are marked unsupported for 3.6/4.0/5.0; `$$NOW` is +absent from the System variables table entirely (`$$CURRENT` and `$$REMOVE` +are explicitly "No"). Implicit-by-omission, but consistent with the reported +`Failed to parse update: field must be of BSON type object` class of error +(AWS documents no exact error string). + +Fix: null-guarded first-acceptance claim (`termsAcceptedAt: null` matches both +the schema's explicit `null` default and missing legacy fields — a +`$exists: false` guard would never fire because of that default), with a +plain-`$set` fallback for repeat acceptance. First-acceptance timestamp +preservation, concurrency convergence, and the `IUser | null` contract are +covered by tests, including a raw-inserted legacy document without the field. + +### 2. `decrementTagCounts` pipeline update (P1 — silent tag-count drift) + +`packages/data-schemas/src/methods/conversationTag.ts:47` used a +`$max`/`$subtract`/`$ifNull` pipeline inside `bulkWrite`, wrapped in a +try/catch that only logs — on DocumentDB, conversation deletion succeeded +while tag counts silently drifted. + +Fix: two mutually exclusive plain ops per tag in one ordered `bulkWrite` — +clamp-to-zero (`count` below the decrement amount, or null/missing) first, +then a guarded `$inc`. Clamp-at-zero, missing-count tolerance, and +variable-amount semantics all preserved; now covered by a new test block +(previously untested). + +### 3. `extendFilesTTL` pipeline update (P1 — `/files/usage` TTL holds fail) + +`packages/data-schemas/src/methods/file.ts:607` — **not in the reporter's +list; found by sweeping the codebase** (`rg` for pipeline-shaped update args +and `$$NOW`; these three sites were the only hits). + +Fix: read the candidate files (one projected query), compute each file's +`min(now + renewMs, createdAt + maxLifetimeMs)` ceiling client-side, then +issue per-document guarded `$set`s (`expiresAt: { $exists: true, $lt: next }`) +through `tenantSafeBulkWrite`. Only-widens, per-file ceiling, and +cleared-TTL-stays-permanent semantics are preserved under concurrency by the +write guard. Cost: one extra read round trip on this path — unavoidable +without a schema change, because the ceiling is per-document. + +## Proven, made loud — partial indexes (P1 on < 5.0 / elastic) + +Four unique partial indexes exist: + +- `packages/data-schemas/src/schema/user.ts:192` and `:199` — OAuth ids + (`googleId`, `openidId`, …) with `partialFilterExpression: { $exists: true }` +- `packages/data-schemas/src/schema/file.ts:173` — `execute_code` files + (`$eq`-shaped filter) +- `packages/data-schemas/src/schema/group.ts:56` — group source ids + (`$exists: true`) + +AWS: [partial-index.html](https://docs.aws.amazon.com/documentdb/latest/developerguide/partial-index.html) +— "The partial index feature is supported in Amazon DocumentDB 5.0 +instance-based clusters"; the index-properties table marks Partial as +No/No/Yes/Yes/No across 3.6/4.0/5.0/8.0/elastic. The `$exists` and `$eq` +filter shapes used here are inside DocumentDB 5.0's supported operator list +(`$eq, $exists, $and, $gt/$gte/$lt/$lte`), so on 5.0+ these indexes build. + +On 3.6/4.0/elastic the builds fail — and empirically (probe: unique index over +pre-seeded duplicates, mongoose 8, autoIndex) the failure is **completely +silent**: no unhandled rejection, no log, the index simply doesn't exist and +duplicate inserts succeed. Mongoose only surfaces build errors through a +`Model.on('index')` listener, which nothing attached. `createModels` now +attaches one that logs every failed build (`packages/data-schemas/src/models/index.ts`). +Operational consequence on < 5.0 remains: OAuth-account uniqueness is not +DB-enforced — documented, loud, but not fixable in application code. + +## Proven compatible — no action needed + +- **Transactions**: supported on 4.0+ instance-based ("Amazon DocumentDB … + supports transactions in Amazon DocumentDB 4.0 and later" — + [transactions.html](https://docs.aws.amazon.com/documentdb/latest/developerguide/transactions.html)); + unsupported on 3.6 and elastic. LibreChat already probes at runtime + (`packages/data-schemas/src/utils/transactions.ts`, cached in + `api/server/services/PermissionService.js`) and falls back to + non-transactional writes — the same mode as standalone MongoDB without a + replica set. DocumentDB's restrictions (1-minute execution limit, no cursors + in transactions, no retryable commit/abort) don't intersect LibreChat's + usage. +- **GridFS**: `packages/api/src/cache/keyvMongo.ts` only constructs a + `GridFSBucket` when `useGridFS` is set — no caller ever sets it and the + class isn't exported (the singleton uses a plain `logs` collection). Dead + code. Moot regardless: AWS lists GridFS as supported on instance-based + clusters (elastic: no). +- **TTL indexes**: supported everywhere including elastic. AWS warns deletion + is best-effort ("Documents are not guaranteed to be deleted within any + specific period") — acceptable, since LibreChat treats TTL as cleanup, not + as a security boundary. +- **`$ifNull`**: supported on all versions (only its pipeline-update context + was the problem). + +## Deployment requirements (documentation, not code) + +- **`retryWrites=false` is mandatory in `MONGO_URI`.** AWS: "Amazon DocumentDB + does not currently support retryable writes"; the failure mode is + `{"ok":0,"errmsg":"Unrecognized field: 'txnNumber'","code":9}` + ([functional-differences.html](https://docs.aws.amazon.com/documentdb/latest/developerguide/functional-differences.html)). + `api/db/connect.js` passes `MONGO_URI` through verbatim, so this belongs in + the deployment docs (and the live harness flags a URI missing it). +- TLS with the AWS CA bundle; clusters are VPC-only (tunnel/bastion for + external access). + +## Document as unsupported — elastic clusters + +[Elastic cluster limitations](https://docs.aws.amazon.com/documentdb/latest/developerguide/docdb-using-elastic-clusters.html): +no **unique indexes** (any), no partial indexes, no ACID transactions, no +GridFS, no change streams, `$expr` unsupported, and the cursor-methods table +even lists `sort()`/`skip()`/`limit()` as "No". The `email + tenantId` unique +index alone disqualifies elastic clusters. Recommend stating this explicitly +in the docs. + +## Undetermined — honest gaps + +- **The reporter's engine version and cluster type** — still unknown; it + decides whether the partial-index caveat applies to them (5.0+: it doesn't). + Worth asking directly on the issue. +- **DocumentDB 8.0 pipeline-update acceptance** — 8.0 added `$set`/`$unset` + aggregation *stages*, but AWS never documents pipeline-form updates; the + harness probe answers this live. +- **`collMod` is only "Partial"** on every version — avoid + `Model.syncIndexes()` against DocumentDB (it may issue `collMod` beyond the + documented `expireAfterSeconds`). +- **Read-side aggregations** (3 files: `methods/prompt.ts`, + `methods/aclEntry.ts`, `methods/agentCategory.ts`) were not audited + stage-by-stage; no exotic stages (`$facet`, `$setWindowFields`, + `$unionWith`, `$graphLookup`) are used anywhere. +- **No faithful local emulator exists.** The `documentdb-local` Docker image + is the PostgreSQL-based Linux Foundation project — AWS's own OSS blog + confirms "a different engine than the one used in Amazon DocumentDB." Live + regression testing must run against a real cluster. + +## Regression strategy + +1. **In-repo (CI today)**: the behavioral tests added in + `user.methods.spec.ts`, `conversationTag.methods.spec.ts`, and + `file.spec.ts` lock the pipeline-free implementations' semantics + (first-acceptance preservation, clamp-at-zero, per-file ceiling). +2. **Live harness (this directory)**: `compat.documentdb.spec.ts` exercises + the exact operations behind #14488 against a real cluster and prints a + capability matrix (pipeline updates, `$$NOW`, transactions, partial unique + indexes, TTL, `retryWrites`). Gated on `DOCUMENTDB_URI`; verified green + against real MongoDB as a baseline. Suggested cadence: before releases and + whenever update-operator code in `data-schemas` changes; optionally a + scheduled GitHub Action on a runner with VPC access to a dev cluster. + +## Support matrix and recommendation + +| Capability (LibreChat dependency) | 3.6 | 4.0 | 5.0 | 8.0 | Elastic | +| --- | --- | --- | --- | --- | --- | +| Pipeline updates (**no longer used**) | ✗ | ✗ | ✗ | ? | ✗ | +| Plain update operators (all writes now) | ✓ | ✓ | ✓ | ✓ | ✓ | +| Unique indexes | ✓ | ✓ | ✓ | ✓ | ✗ | +| Partial unique indexes (OAuth ids) | ✗ | ✗ | ✓ | ✓ | ✗ | +| Transactions (runtime-probed) | ✗ | ✓ | ✓ | ✓ | ✗ | +| TTL indexes | ✓ | ✓ | ✓ | ✓ | ✓ | + +**Recommendation**: support **DocumentDB 5.0+ instance-based** with +`retryWrites=false` documented as required. 4.0 functions with +partial-unique-index loss (now logged loudly at startup) — "works, with a +documented caveat". Elastic clusters: unsupported, full stop. diff --git a/packages/data-schemas/misc/documentdb/jest.documentdb.config.mjs b/packages/data-schemas/misc/documentdb/jest.documentdb.config.mjs new file mode 100644 index 0000000000..3a149c0863 --- /dev/null +++ b/packages/data-schemas/misc/documentdb/jest.documentdb.config.mjs @@ -0,0 +1,20 @@ +/** + * Jest config for Amazon DocumentDB live-compatibility tests. + * These tests require network access to a real Amazon DocumentDB cluster + * (VPC-only — run from a bastion/tunnel) and are NOT run in CI by default. + * + * Usage: + * DOCUMENTDB_URI="mongodb://user:pass@127.0.0.1:27017/librechat_compat?tls=true&retryWrites=false" \ + * DOCUMENTDB_TLS_CA_FILE="global-bundle.pem" \ + * npx jest --config misc/documentdb/jest.documentdb.config.mjs + */ +export default { + rootDir: '../..', + testMatch: ['/misc/documentdb/**/*.documentdb.spec.ts'], + moduleNameMapper: { + '^@src/(.*)$': '/src/$1', + '^~/(.*)$': '/src/$1', + }, + restoreMocks: true, + testTimeout: 120000, +}; diff --git a/packages/data-schemas/misc/documentdb/tsconfig.json b/packages/data-schemas/misc/documentdb/tsconfig.json new file mode 100644 index 0000000000..a64965da40 --- /dev/null +++ b/packages/data-schemas/misc/documentdb/tsconfig.json @@ -0,0 +1,13 @@ +{ + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + "target": "ES2020", + "lib": ["ES2020"], + "paths": { + "~/*": ["../../src/*"] + } + }, + "include": ["./**/*.ts"], + "exclude": ["node_modules"] +} diff --git a/packages/data-schemas/src/methods/conversationTag.methods.spec.ts b/packages/data-schemas/src/methods/conversationTag.methods.spec.ts index 0b4c6268d6..cbc1990973 100644 --- a/packages/data-schemas/src/methods/conversationTag.methods.spec.ts +++ b/packages/data-schemas/src/methods/conversationTag.methods.spec.ts @@ -1,9 +1,9 @@ import mongoose from 'mongoose'; import { MongoMemoryServer } from 'mongodb-memory-server'; -import { createConversationTagMethods } from './conversationTag'; -import { createModels } from '~/models'; -import type { IConversationTag } from '~/schema/conversationTag'; import type { IConversation } from '..'; +import type { IConversationTag } from '~/schema/conversationTag'; +import { createConversationTagMethods, decrementTagCounts } from './conversationTag'; +import { createModels } from '~/models'; jest.mock('~/config/winston', () => ({ error: jest.fn(), @@ -137,3 +137,84 @@ describe('ConversationTag model - $pullAll operations', () => { }); }); }); + +describe('decrementTagCounts', () => { + const userId = new mongoose.Types.ObjectId().toString(); + + const readCount = async (tag: string, user: string = userId) => + (await ConversationTag.findOne({ user, tag }).lean())?.count; + + it('decrements once per tag occurrence', async () => { + await ConversationTag.create({ tag: 'work', user: userId, position: 1, count: 5 }); + + await decrementTagCounts(mongoose, userId, ['work', 'work']); + + expect(await readCount('work')).toBe(3); + }); + + it('decrements an exact count down to zero', async () => { + await ConversationTag.create({ tag: 'work', user: userId, position: 1, count: 2 }); + + await decrementTagCounts(mongoose, userId, ['work', 'work']); + + expect(await readCount('work')).toBe(0); + }); + + it('clamps at zero when the decrement exceeds the current count', async () => { + await ConversationTag.create({ tag: 'work', user: userId, position: 1, count: 1 }); + + await decrementTagCounts(mongoose, userId, ['work', 'work', 'work']); + + expect(await readCount('work')).toBe(0); + }); + + it('leaves a zero count at zero', async () => { + await ConversationTag.create({ tag: 'empty', user: userId, position: 1, count: 0 }); + + await decrementTagCounts(mongoose, userId, ['empty']); + + expect(await readCount('empty')).toBe(0); + }); + + it('clamps pre-existing negative drift to zero', async () => { + await ConversationTag.create({ tag: 'drift', user: userId, position: 1, count: -3 }); + + await decrementTagCounts(mongoose, userId, ['drift']); + + expect(await readCount('drift')).toBe(0); + }); + + it('treats a missing count as zero', async () => { + await ConversationTag.collection.insertOne({ tag: 'legacy', user: userId, position: 1 }); + + await decrementTagCounts(mongoose, userId, ['legacy']); + + expect(await readCount('legacy')).toBe(0); + }); + + it('converges to zero under concurrent decrements exceeding the count', async () => { + await ConversationTag.create({ tag: 'race', user: userId, position: 1, count: 3 }); + + await Promise.all([ + decrementTagCounts(mongoose, userId, ['race', 'race']), + decrementTagCounts(mongoose, userId, ['race', 'race']), + ]); + + expect(await readCount('race')).toBe(0); + }); + + it("leaves other users' tags untouched", async () => { + const otherUserId = new mongoose.Types.ObjectId().toString(); + await ConversationTag.create({ tag: 'work', user: userId, position: 1, count: 4 }); + await ConversationTag.create({ tag: 'work', user: otherUserId, position: 1, count: 4 }); + + await decrementTagCounts(mongoose, userId, ['work']); + + expect(await readCount('work')).toBe(3); + expect(await readCount('work', otherUserId)).toBe(4); + }); + + it('ignores empty and unknown tags without throwing', async () => { + await expect(decrementTagCounts(mongoose, userId, ['', 'nonexistent'])).resolves.not.toThrow(); + }); +}); diff --git a/packages/data-schemas/src/methods/conversationTag.ts b/packages/data-schemas/src/methods/conversationTag.ts index be65f6b446..cc24070b9a 100644 --- a/packages/data-schemas/src/methods/conversationTag.ts +++ b/packages/data-schemas/src/methods/conversationTag.ts @@ -17,6 +17,16 @@ interface IConversationTag { * single decrement, so callers must dedupe tags per conversation before flattening * to avoid double-decrementing a conversation's duplicate tag entries. Counts are * clamped at zero to tolerate any pre-existing drift. + * + * Each tag emits three ops in one ordered bulkWrite instead of a + * `$max`/`$subtract` aggregation-pipeline update (which Amazon DocumentDB + * rejects): normalize a null/missing count to zero, apply the `$inc`, then + * clamp a negative result back to zero. The clamp keys on `count < 0` rather + * than `count < amount` so it composes with concurrent decrements of the same + * tag: increments commute and every interleaved call ends with its own clamp, + * so the count still converges on `max(0, ...)` exactly as the serialized + * pipeline did. The only trade-off is a transiently negative count between an + * op pair, which readers already tolerate. */ export async function decrementTagCounts( mongoose: typeof import('mongoose'), @@ -41,18 +51,26 @@ export async function decrementTagCounts( try { const ConversationTag = mongoose.models.ConversationTag as Model; - const bulkOps = [...decrementByTag.entries()].map(([tag, amount]) => ({ - updateOne: { - filter: { user, tag }, - update: [ - { - $set: { - count: { $max: [0, { $subtract: [{ $ifNull: ['$count', 0] }, amount] }] }, - }, - }, - ], + const bulkOps = [...decrementByTag.entries()].flatMap(([tag, amount]) => [ + { + updateOne: { + filter: { user, tag, count: null }, + update: { $set: { count: 0 } }, + }, }, - })); + { + updateOne: { + filter: { user, tag }, + update: { $inc: { count: -amount } }, + }, + }, + { + updateOne: { + filter: { user, tag, count: { $lt: 0 } }, + update: { $set: { count: 0 } }, + }, + }, + ]); await tenantSafeBulkWrite(ConversationTag, bulkOps); } catch (error) { diff --git a/packages/data-schemas/src/methods/file.spec.ts b/packages/data-schemas/src/methods/file.spec.ts index 4aa424301b..54eece631b 100644 --- a/packages/data-schemas/src/methods/file.spec.ts +++ b/packages/data-schemas/src/methods/file.spec.ts @@ -1143,9 +1143,17 @@ describe('File Methods', () => { }); await mongoose.models.File.updateOne( { file_id: fileId }, - { $set: { expiresAt, ...(createdAt ? { createdAt } : {}) } }, + { $set: { expiresAt } }, { timestamps: false }, ); + /** Mongoose strips the immutable `createdAt` from updates, so backdating + * must go through the raw driver to actually land. */ + if (createdAt) { + await mongoose.models.File.collection.updateOne( + { file_id: fileId }, + { $set: { createdAt } }, + ); + } return fileId; }; @@ -1214,6 +1222,28 @@ describe('File Methods', () => { expect(file.expiresAt!.getTime()).toBeGreaterThan(Date.now() + 50 * 60_000); }); + it("applies each file's own ceiling within a single batch", async () => { + const userId = new mongoose.Types.ObjectId(); + const soon = new Date(Date.now() + 60_000); + const freshId = await seedTempFile(userId, soon); + const agedId = await seedTempFile(userId, soon, new Date(Date.now() - 47 * HOUR)); + const releasedId = await seedTempFile(userId, soon); + await fileMethods.updateFileUsage({ file_id: releasedId, user: String(userId) }); + + const count = await fileMethods.extendFilesTTL([freshId, agedId, releasedId], HOLD, { + user: String(userId), + }); + + expect(count).toBe(2); + const fresh = await readFile(freshId); + expect(fresh.expiresAt!.getTime()).toBeGreaterThan(Date.now() + 23 * HOUR); + const aged = await readFile(agedId); + expect(aged.expiresAt!.getTime()).toBe(aged.createdAt.getTime() + HOLD.maxLifetimeMs); + expect(aged.expiresAt!.getTime()).toBeLessThan(Date.now() + 2 * HOUR); + const released = await readFile(releasedId); + expect(released.expiresAt).toBeUndefined(); + }); + it('does not resurrect a TTL on an already-released file', async () => { const userId = new mongoose.Types.ObjectId(); const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000)); diff --git a/packages/data-schemas/src/methods/file.ts b/packages/data-schemas/src/methods/file.ts index ac428bdce0..cb544862c1 100644 --- a/packages/data-schemas/src/methods/file.ts +++ b/packages/data-schemas/src/methods/file.ts @@ -557,20 +557,23 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { * * A renewable hold, not a release: unlike `updateFileUsage` this never * unsets `expiresAt`, so a file that is held but never actually sent is - * still reaped once the hold lapses. Four properties hold by - * construction, which is what makes the write safe to drive from a - * client-supplied id list: - * - `$min` against `createdAt + maxLifetimeMs` caps every renewal against - * an immutable anchor, so repeated calls converge on a fixed ceiling + * still reaped once the hold lapses. Candidates are read first, then each + * doc gets a guarded write — no aggregation-pipeline update, which Amazon + * DocumentDB rejects. Four properties hold by construction, which is what + * makes the write safe to drive from a client-supplied id list: + * - capping the renewal at `createdAt + maxLifetimeMs` anchors it to an + * immutable ceiling, so repeated calls converge on a fixed deadline * instead of walking a file's lifetime forward a window at a time; * - renewing from `now` up to that ceiling lets a queue that is still * draining keep its attachments alive across successive runs, while an * abandoned queue lapses a single `renewMs` after its last touch rather * than surviving to the ceiling; - * - `$max` against the current value means a hold only ever widens; - * - `expiresAt: { $exists: true }` means a file whose TTL was already - * cleared by a real send stays permanent. Re-adding `expiresAt` there - * would schedule a live file for deletion. + * - the `expiresAt: { $lt: next }` write guard means a hold only ever + * widens, even against renewals landing between the read and the write; + * - `expiresAt: { $exists: true }` in the read filter and the write guard + * means a file whose TTL was already cleared by a real send stays + * permanent. Re-adding `expiresAt` there would schedule a live file for + * deletion. * * `createdAt` is required rather than defaulted: without the anchor there * is no ceiling to enforce, so such a file is skipped instead of held. @@ -603,23 +606,34 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { }, { userId: owner.user, tenantId: owner.tenantId }, ); - const renewUntil = new Date(Date.now() + renewMs); - const result = await File.updateMany( - filter, - [ + const renewUntil = Date.now() + renewMs; + const candidates = await File.find(filter) + .select({ _id: 1, expiresAt: 1, createdAt: 1 }) + .lean[]>(); + const holdOps = candidates.flatMap((file) => { + if (!file.createdAt || !file.expiresAt) { + return []; + } + const next = new Date(Math.min(renewUntil, file.createdAt.getTime() + maxLifetimeMs)); + if (file.expiresAt.getTime() >= next.getTime()) { + return []; + } + return [ { - $set: { - expiresAt: { - $max: ['$expiresAt', { $min: [renewUntil, { $add: ['$createdAt', maxLifetimeMs] }] }], - }, + updateOne: { + filter: { _id: file._id, expiresAt: { $exists: true, $lt: next } }, + update: { $set: { expiresAt: next } }, }, }, - ], - /** `timestamps: false`: a hold is TTL bookkeeping, not a content write. - * Bumping `updatedAt` would also make every re-touch count as a - * modification, hiding whether the deadline actually moved. */ - { timestamps: false }, - ); + ]; + }); + if (holdOps.length === 0) { + return 0; + } + /** `timestamps: false`: a hold is TTL bookkeeping, not a content write. + * Bumping `updatedAt` would also make every re-touch count as a + * modification, hiding whether the deadline actually moved. */ + const result = await tenantSafeBulkWrite(File, holdOps, { timestamps: false }); return result.modifiedCount ?? 0; } diff --git a/packages/data-schemas/src/methods/user.methods.spec.ts b/packages/data-schemas/src/methods/user.methods.spec.ts index 9f15d2f9a4..f320c49e4a 100644 --- a/packages/data-schemas/src/methods/user.methods.spec.ts +++ b/packages/data-schemas/src/methods/user.methods.spec.ts @@ -498,6 +498,65 @@ describe('User Methods - Database Tests', () => { expect(result).toBeNull(); }); + test('stamps a fresh termsAcceptedAt when accepting after a terms reset', async () => { + const user = await User.create({ + name: 'Reset User', + email: 'reset-terms@example.com', + provider: 'local', + }); + const userId = user._id?.toString() ?? ''; + const first = await methods.acceptTerms(userId); + + await User.updateOne( + { _id: userId }, + { $set: { termsAccepted: false, termsAcceptedAt: null } }, + ); + const reaccepted = await methods.acceptTerms(userId); + + expect(reaccepted?.termsAccepted).toBe(true); + expect(reaccepted?.termsAcceptedAt).toBeInstanceOf(Date); + expect((reaccepted?.termsAcceptedAt as Date).getTime()).toBeGreaterThanOrEqual( + (first?.termsAcceptedAt as Date).getTime(), + ); + }); + + test('stamps termsAcceptedAt for a legacy document missing the field entirely', async () => { + const legacyId = new mongoose.Types.ObjectId(); + await User.collection.insertOne({ + _id: legacyId, + name: 'Pre-Terms User', + email: 'pre-terms@example.com', + provider: 'local', + }); + + const updated = await methods.acceptTerms(legacyId.toString()); + + expect(updated?.termsAccepted).toBe(true); + expect(updated?.termsAcceptedAt).toBeInstanceOf(Date); + }); + + test('converges on a single termsAcceptedAt under concurrent acceptance', async () => { + const user = await User.create({ + name: 'Concurrent User', + email: 'concurrent-terms@example.com', + provider: 'local', + }); + const userId = user._id?.toString() ?? ''; + + const results = await Promise.all( + Array.from({ length: 5 }, () => methods.acceptTerms(userId)), + ); + + expect(results.every((result) => result?.termsAccepted === true)).toBe(true); + const stampedTimes = new Set( + results.map((result) => (result?.termsAcceptedAt as Date).getTime()), + ); + expect(stampedTimes.size).toBe(1); + + const repeat = await methods.acceptTerms(userId); + expect((repeat?.termsAcceptedAt as Date).getTime()).toBe([...stampedTimes][0]); + }); + test('should invalidate cached auth user documents on acceptance', async () => { enableAuthUserDocCache(); const user = await User.create({ diff --git a/packages/data-schemas/src/methods/user.ts b/packages/data-schemas/src/methods/user.ts index d044bf40d4..35b5be6cae 100644 --- a/packages/data-schemas/src/methods/user.ts +++ b/packages/data-schemas/src/methods/user.ts @@ -296,28 +296,45 @@ export function createUserMethods( /** * Atomically records terms acceptance for a user. - * Sets termsAccepted and, only when no timestamp is already stored, stamps - * termsAcceptedAt with the server time so the first acceptance within a terms - * cycle is preserved across concurrent or repeated requests. + * A null-guarded claim stamps termsAcceptedAt only when no timestamp is + * already stored (explicit null from the schema default, a missing legacy + * field, or a terms reset), so the first acceptance within a terms cycle is + * preserved across concurrent or repeated requests. The repeat-acceptance + * fallback is guarded by the exact complement (a non-null timestamp) so it + * can never resurrect termsAccepted into a cycle that config/reset-terms.js + * started between the two updates; when both guards miss because a reset + * raced in, the claim retries and records a fresh stamped acceptance. Plain + * updates are used instead of an aggregation pipeline with $$NOW, which + * Amazon DocumentDB rejects. */ async function acceptTerms(userId: string): Promise { const User = mongoose.models.User; - const updated = await User.findByIdAndUpdate( - userId, - [ - { - $set: { - termsAccepted: true, - termsAcceptedAt: { $ifNull: ['$termsAcceptedAt', '$$NOW'] }, - }, - }, - ], - { new: true, runValidators: true }, - ).lean(); - if (updated) { - await invalidateAuthUserDocCache(userId); + const maxAttempts = 3; + for (let attempt = 0; attempt < maxAttempts; attempt++) { + const firstAcceptance = await User.findOneAndUpdate( + { _id: userId, termsAcceptedAt: null }, + { $set: { termsAccepted: true, termsAcceptedAt: new Date() } }, + { new: true, runValidators: true }, + ).lean(); + if (firstAcceptance) { + await invalidateAuthUserDocCache(userId); + return firstAcceptance; + } + const reacceptance = await User.findOneAndUpdate( + { _id: userId, termsAcceptedAt: { $ne: null } }, + { $set: { termsAccepted: true } }, + { new: true, runValidators: true }, + ).lean(); + if (reacceptance) { + await invalidateAuthUserDocCache(userId); + return reacceptance; + } + const exists = await User.exists({ _id: userId }); + if (!exists) { + return null; + } } - return updated; + return null; } /** diff --git a/packages/data-schemas/src/models/index.ts b/packages/data-schemas/src/models/index.ts index 440106f2e0..ed1e56d9a5 100644 --- a/packages/data-schemas/src/models/index.ts +++ b/packages/data-schemas/src/models/index.ts @@ -35,6 +35,7 @@ import { createUserModel } from './user'; import { createRoleModel } from './role'; import { createFileModel } from './file'; import { createKeyModel } from './key'; +import logger from '~/config/winston'; /** * Creates all database models for all collections @@ -78,7 +79,7 @@ export function createModels(mongoose: typeof import('mongoose')): { Group: ReturnType; Config: ReturnType; } { - return { + const models = { User: createUserModel(mongoose), Token: createTokenModel(mongoose), Session: createSessionModel(mongoose), @@ -117,4 +118,19 @@ export function createModels(mongoose: typeof import('mongoose')): { Group: createGroupModel(mongoose), Config: createConfigModel(mongoose), }; + /** + * Background index builds fail silently unless an 'index' listener is + * attached (e.g. Amazon DocumentDB <5.0 rejecting partialFilterExpression), + * leaving unique constraints unenforced with no trace in the logs. + */ + for (const model of Object.values(models)) { + if (model.listenerCount('index') === 0) { + model.on('index', (error?: Error) => { + if (error) { + logger.error(`Index build failed for "${model.modelName}": ${error.message}`); + } + }); + } + } + return models; }