Skip to content

Upgrade mongoose version to 9.5.* #789

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Luquitasjeffrey wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Upgrade mongoose version to 9.5.* #789

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d60b29e
Upgrade mongoose version to 9.5.*
Luquitasjeffrey Apr 26, 2026
90d30b4
Code formatting
Luquitasjeffrey Apr 27, 2026
fe31341
Fix lint errors
Luquitasjeffrey Apr 27, 2026
403ba97
Fix CI: In mongoose 9 when creating a new ObjectId from a string it s…
Luquitasjeffrey Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 32 additions & 30 deletions bot/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import mongoose from 'mongoose';
const { ObjectId } = mongoose.Types;
import { validateFiatSentOrder, validateReleaseOrder } from './validations';

Check failure on line 3 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 3 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
Comment on lines +1 to 2
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix ESLint import/first failure breaking CI.

Pipeline reports: ESLint (import/first): Import in body of module; reorder to top. The const { ObjectId } = mongoose.Types; declaration on line 2 sits between two import statements; move it below all imports.

As per coding guidelines: "address warnings instead of disabling them".

🔧 Proposed fix 
 import mongoose from 'mongoose';
-const { ObjectId } = mongoose.Types;
 import { validateFiatSentOrder, validateReleaseOrder } from './validations';
 import {
   createHoldInvoice,
@@ -27,6 +26,8 @@
 import { HasTelegram, MainContext } from './start';
 import { CommunityContext } from './modules/community/communityContext';
 
+const { ObjectId } = mongoose.Types;
+
 enum UserOrderRole {
📝 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
import mongoose from 'mongoose';
const { ObjectId } = mongoose.Types;
import { validateFiatSentOrder, validateReleaseOrder } from './validations';
import mongoose from 'mongoose';
import { validateFiatSentOrder, validateReleaseOrder } from './validations';
🧰 Tools
🪛 ESLint

[error] 3-3: Import in body of module; reorder to top.

(import/first)

🪛 GitHub Actions: Auto Check Lint

[error] 3-3: ESLint (import/first): Import in body of module; reorder to top.

🪛 GitHub Check: Lint

[failure] 3-3:
Import in body of module; reorder to top

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

In `@bot/commands.ts` around lines 1 - 3, Move the in-module declaration so all
imports come first: place "const { ObjectId } = mongoose.Types;" after the
import statements (below the imports of mongoose and the named validators like
validateFiatSentOrder and validateReleaseOrder) so that ObjectId is still
derived from mongoose but no longer sits between import lines, resolving the
import/first ESLint error.

import {

Check failure on line 4 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 4 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
createHoldInvoice,
subscribeInvoice,
Expand All @@ -6,21 +8,21 @@
settleHoldInvoice,
getInvoice,
} from '../ln';
import { Order, User, Dispute } from '../models';

Check failure on line 11 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 11 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
import * as messages from './messages';

Check failure on line 12 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 12 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
import {

Check failure on line 13 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 13 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
getBtcFiatPrice,
deleteOrderFromChannel,
getUserI18nContext,
getFee,
removeLightningPrefix,
} from '../util';
import * as ordersActions from './ordersActions';

Check failure on line 20 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 20 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
import * as OrderEvents from './modules/events/orders';

Check failure on line 21 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 21 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

import { resolvLightningAddress } from '../lnurl/lnurl-pay';

Check failure on line 23 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 23 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
import { logger } from '../logger';

Check failure on line 24 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 24 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
import { IOrder } from '../models/order';

Check failure on line 25 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top

Check failure on line 25 in bot/commands.ts

View workflow job for this annotation

GitHub Actions / Lint 

Import in body of module; reorder to top
import { UserDocument } from '../models/user';
import { HasTelegram, MainContext } from './start';
import { CommunityContext } from './modules/community/communityContext';
Expand All @@ -41,7 +43,7 @@
const update = { [propName]: true };
return await Order.findOneAndUpdate(
{
_id: orderId,
_id: new ObjectId(orderId),
[propName]: { $ne: true },
status: { $in: ['ACTIVE', 'FIAT_SENT', 'DISPUTE'] },
},
Expand All @@ -62,7 +64,7 @@
// If there is not fiat amount the function don't do anything
if (order.fiat_amount === undefined) {
logger.debug(
`waitPayment: fiat_amount === undefined, User Id ${ctx.user.id} order Id: ${order.id}`,
`waitPayment: fiat_amount === undefined, User Id ${ctx.user._id.toString()} order Id: ${order._id.toString()}`,
);
return;
}
Expand All @@ -87,7 +89,7 @@
// We create a hold invoice
const description = i18nCtx.t('hold_invoice_memo', {
botName: ctx.botInfo.username,
orderId: order._id,
orderId: order._id.toString(),
fiatCode: order.fiat_code,
fiatAmount: order.fiat_amount,
});
Expand Down Expand Up @@ -137,7 +139,7 @@
if (!order) {
const orderId = (ctx.update as any).callback_query.message.text;
if (!orderId) return;
order = await Order.findOne({ _id: orderId });
order = await Order.findOne({ _id: new ObjectId(orderId) });
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inventory all `new ObjectId(...)` usages introduced in the PR for cross-file consistency review.
rg -nP '\bnew\s+ObjectId\s*\(' --type=ts -C1

Repository: lnp2pBot/bot

Length of output: 8827

🏁 Script executed:

fd -t f '\.ts$' -path '*/models/*' | head -20

Repository: lnp2pBot/bot

Length of output: 225

🏁 Script executed:

rg -l "interface IOrder|class Order|model.*Order" --type=ts -m 5

Repository: lnp2pBot/bot

Length of output: 92

🏁 Script executed:

# Find Order model definition
git ls-files | grep -i 'order' | grep -E 'models|schema' | head -10

Repository: lnp2pBot/bot

Length of output: 72

🏁 Script executed:

cat -n models/order.ts

Repository: lnp2pBot/bot

Length of output: 6007

🏁 Script executed:

rg -l "interface IUser|export.*User|model.*User" --type=ts models/ | head -5

Repository: lnp2pBot/bot

Length of output: 107

🏁 Script executed:

cat -n models/user.ts

Repository: lnp2pBot/bot

Length of output: 2230

🏁 Script executed:

# Test what happens with new ObjectId(null)
python3 << 'EOF'
# Since we can't run MongoDB/Mongoose here, let's at least verify the MongoDB ObjectId behavior
# by checking if we have access to mongodb package

import subprocess
import sys

# Try to see if mongodb is available
try:
    result = subprocess.run(['npm', 'list', 'mongodb'], cwd='.', capture_output=True, text=True)
    print("NPM mongodb search:")
    print(result.stdout)
    print(result.stderr)
except Exception as e:
    print(f"Error: {e}")
EOF

Repository: lnp2pBot/bot

Length of output: 126

🏁 Script executed:

# Check package.json to see mongodb/mongoose versions
cat -n package.json | head -50

Repository: lnp2pBot/bot

Length of output: 1711

🏁 Script executed:

# Search for any tests related to ObjectId or null handling
rg "ObjectId\(null\)" --type=ts

Repository: lnp2pBot/bot

Length of output: 38

🏁 Script executed:

# Look for test cases that might reveal expected behavior
rg -A5 "new ObjectId" tests/ --type=ts | head -60

Repository: lnp2pBot/bot

Length of output: 2630

🏁 Script executed:

# Check if there's any Mongoose usage documentation or comments
rg -B2 -A2 "auto.cast|auto-cast" --type=ts

Repository: lnp2pBot/bot

Length of output: 38

🏁 Script executed:

# Look for how buyer_id and seller_id are used after being retrieved
rg -B2 -A2 "buyer_id\!|seller_id\!" bot/commands.ts | head -80

Repository: lnp2pBot/bot

Length of output: 153

🏁 Script executed:

# Check if there's any validation before using these fields
rg "buyer_id|seller_id" bot/commands.ts -B1 -A1 | grep -E "if.*buyer_id|if.*seller_id" | head -20

Repository: lnp2pBot/bot

Length of output: 505

🏁 Script executed:

# Look at the actual code patterns around line 151 where the review starts
sed -n '140,160p' bot/commands.ts

Repository: lnp2pBot/bot

Length of output: 717

🏁 Script executed:

# Check if there are cases where buyer_id or seller_id could be null at the point of use
rg -B5 "new ObjectId\(order\.buyer_id" bot/commands.ts | head -40

Repository: lnp2pBot/bot

Length of output: 1552

🏁 Script executed:

# Look for how order.buyer_id is checked before being used
rg -B10 "new ObjectId\(order\.buyer_id" bot/commands.ts | grep -A10 "order =" | head -50

Repository: lnp2pBot/bot

Length of output: 1522

🏁 Script executed:

# Create a simple test to check ObjectId(null) behavior
python3 << 'EOF'
# Let's check what the mongodb/bson ObjectId library does with null
# First, let's look at package-lock.json or npm ls to see what's installed

import subprocess
import json

result = subprocess.run(['npm', 'ls', '--json'], capture_output=True, text=True)
try:
    data = json.loads(result.stdout)
    # Find mongodb or bson
    for key in ['mongodb', 'bson']:
        if key in data.get('dependencies', {}):
            print(f"{key}: {data['dependencies'][key]}")
except:
    print("Could not parse npm ls output")

# Also check package.json dependencies
with open('package.json') as f:
    pkg = json.load(f)
    deps = pkg.get('dependencies', {})
    if 'mongoose' in deps:
        print(f"mongoose: {deps['mongoose']}")
EOF

Repository: lnp2pBot/bot

Length of output: 73

🏁 Script executed:

# Search for how ObjectId is imported and used - check if it's the mongodb ObjectId
rg "import.*ObjectId|from.*ObjectId" --type=ts | head -10

Repository: lnp2pBot/bot

Length of output: 160

🏁 Script executed:

# Let's check what happens when we look at the actual usage patterns
# Find all instances where buyer_id or seller_id are used after being wrapped in ObjectId
rg -B3 -A3 'new ObjectId\(order\.(buyer_id|seller_id)' bot/commands.ts | head -100

Repository: lnp2pBot/bot

Length of output: 3316

🏁 Script executed:

# Check if there are any defensive checks before using these fields
sed -n '230,245p' bot/commands.ts

Repository: lnp2pBot/bot

Length of output: 617

🏁 Script executed:

# Look for similar patterns in other files to understand if this is intentional
rg -B2 'new ObjectId\(.*\.buyer_id' bot/validations.ts jobs/pending_payments.ts

Repository: lnp2pBot/bot

Length of output: 267

🏁 Script executed:

# Check if there's error handling that catches ObjectId construction errors
rg -B2 -A2 "try.*new ObjectId|catch.*Error.*ObjectId" --type=ts

Repository: lnp2pBot/bot

Length of output: 38

🏁 Script executed:

# Look for any comments or documentation about ObjectId usage in the codebase
rg -i "objectid.*null|null.*objectid" --type=ts -B1 -A1

Repository: lnp2pBot/bot

Length of output: 38

🏁 Script executed:

# Check what version of mongodb driver is being used (indirectly through mongoose)
cat -n package-lock.json | grep -A5 -B5 '"bson"' | head -30

Repository: lnp2pBot/bot

Length of output: 565

🏁 Script executed:

# Let's create a test to verify ObjectId behavior with null
python3 << 'EOF'
# We need to understand what ObjectId(null) actually does
# Let's check the mongoose/mongodb documentation behavior

# Since we're on Mongoose 9.5.0, let's look at what that version does
# by examining node_modules if available

import subprocess
import os

# Check if node_modules exists and has mongodb
result = subprocess.run(['find', 'node_modules', '-name', 'index.d.ts', '-path', '*bson*'], 
                       capture_output=True, text=True, timeout=5)
if result.stdout:
    print("Found BSON type definitions:")
    files = result.stdout.strip().split('\n')[:3]
    for f in files:
        print(f)
        
# Try to find ObjectId constructor definition
result = subprocess.run(['grep', '-r', 'class ObjectId', 'node_modules/bson/', '--include=*.js', '--include=*.d.ts'], 
                       capture_output=True, text=True, timeout=5)
if result.returncode == 0 and result.stdout:
    print("\nObjectId definition found:")
    print(result.stdout[:500])
EOF

Repository: lnp2pBot/bot

Length of output: 38

🏁 Script executed:

# Check what the actual behavior is - look for error handling around ObjectId construction
rg "new ObjectId" bot/commands.ts -B5 -A5 | grep -E "try|catch|throw" -A2 -B2 | head -40

Repository: lnp2pBot/bot

Length of output: 1672

🏁 Script executed:

# Look for any specific Mongoose 9 changes or notes in comments
rg -i "mongoose.*9|version.*9|auto.*cast" --type=ts --type=md

Repository: lnp2pBot/bot

Length of output: 38

🏁 Script executed:

# Verify if plain strings work by checking if there are any examples without ObjectId wrapping
rg "findOne.*{.*_id.*:" bot/validations.ts -A1 | grep -v "new ObjectId" | head -20

Repository: lnp2pBot/bot

Length of output: 167

🏁 Script executed:

# Search for findOne patterns more carefully
rg "findOne.*_id" bot/validations.ts -A1 | head -30

Repository: lnp2pBot/bot

Length of output: 589

🏁 Script executed:

# Check if there are any successful queries without ObjectId wrapping
rg "\.findOne\(" --type=ts -A1 | grep -v "new ObjectId" | head -40

Repository: lnp2pBot/bot

Length of output: 2305

🏁 Script executed:

# Look at the test file to see how mocked queries handle this
rg "findOne" tests/bot/validation.spec.ts -B2 -A2 | head -60

Repository: lnp2pBot/bot

Length of output: 1524

🏁 Script executed:

# This is crucial - check if there's a case where buyer_id or seller_id could be null at runtime
# Look for all places where an order is created/fetched, then used
rg -B5 "buyer_id.*null" models/order.ts

Repository: lnp2pBot/bot

Length of output: 204

🏁 Script executed:

# Check the ln/subscribe_invoice.ts file more carefully since it doesn't wrap in ObjectId
sed -n '1,50p' ln/subscribe_invoice.ts

Repository: lnp2pBot/bot

Length of output: 1536

🏁 Script executed:

# Look for what happens in subscribe_invoice with buyer_id/seller_id
sed -n '80,120p' ln/subscribe_invoice.ts

Repository: lnp2pBot/bot

Length of output: 1535

🏁 Script executed:

# Find the critical part where buyer_id and seller_id are used without ObjectId wrapping
rg "buyer_id|seller_id" ln/subscribe_invoice.ts -B3 -A3

Repository: lnp2pBot/bot

Length of output: 1109

🏁 Script executed:

# Check if ln/subscribe_invoice is using a different pattern - look for the actual queries
grep -n "User.findOne" ln/subscribe_invoice.ts -A1 -B1

Repository: lnp2pBot/bot

Length of output: 729

🏁 Script executed:

# Let me see the full context around those User.findOne calls
sed -n '140,160p' ln/subscribe_invoice.ts

Repository: lnp2pBot/bot

Length of output: 1144

🏁 Script executed:

# Check if there's any validation that ensures buyer_id/seller_id are not null before use
rg -B10 "User.findOne.*buyer_id" ln/subscribe_invoice.ts | grep -E "if.*buyer_id|if.*null|!=="

Repository: lnp2pBot/bot

Length of output: 94

🏁 Script executed:

# Also check the pattern in bot/validations.ts to see if they check before wrapping
rg -B5 "new ObjectId\(.*Id\)" bot/validations.ts | head -40

Repository: lnp2pBot/bot

Length of output: 578

Remove unnecessary new ObjectId() wrappings on string-typed ID fields — Mongoose 9 auto-casts them.

The codebase has two patterns: ln/subscribe_invoice.ts queries User.findOne({ _id: order.buyer_id }) directly (no ObjectId wrapper), while bot/commands.ts wraps the same fields with new ObjectId(order.buyer_id!). Both work identically in Mongoose 9, proving auto-casting.

Wrapping adds two problems:

  1. Silent miss on null values. order.buyer_id is typed string | null. When null reaches new ObjectId(null), it generates a random ObjectId instead of failing — converting a legitimate "not found" into a silent missed query. Direct strings let Mongoose's caster handle the null naturally.
  2. Non-null assertion masks the type. The ! silences TypeScript but doesn't guarantee runtime safety. If buyer_id is null at runtime, the assertion is false.

Remove the new ObjectId() wrappings from the listed lines. If buyer_id or seller_id can legitimately be null, add explicit guards before queries:

if (!order.buyer_id) return;
const buyer = await User.findOne({ _id: order.buyer_id });

Applies to: 142, 151, 180, 233, 236, 238, 305, 315, 324, 326, 418, 422, 512, 521, 523, 651, 656, 742, 745, 835 in bot/commands.ts and similar patterns in bot/validations.ts, jobs/pending_payments.ts, bot/messages.ts.

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

In `@bot/commands.ts` at line 142, Remove the unnecessary new ObjectId(...)
wrappings around string-typed ID fields (e.g. places calling Order.findOne({
_id: new ObjectId(orderId) }) and User.findOne({ _id: new
ObjectId(order.buyer_id!) })) because Mongoose 9 auto-casts strings; instead
pass the raw string IDs to findOne. Replace occurrences that wrap
order.buyer_id, order.seller_id, orderId, etc., with the plain values and add
explicit null/undefined guards where the field is typed string | null (for
example, if (!order.buyer_id) return; const buyer = await User.findOne({ _id:
order.buyer_id });). Update all listed call sites in bot/commands.ts (and
similar patterns in bot/validations.ts, jobs/pending_payments.ts,
bot/messages.ts) to remove new ObjectId and drop non-null assertions so runtime
nulls are handled explicitly.

if (!order) return;
}

Expand All @@ -146,7 +148,7 @@
return;
}

const buyer = await User.findOne({ _id: order.buyer_id });
const buyer = await User.findOne({ _id: new ObjectId(order.buyer_id!) });
if (buyer === null) throw new Error('buyer was not found');

if (order.fiat_amount === undefined) {
Expand Down Expand Up @@ -175,7 +177,7 @@
return;
}
await order.save();
const seller = await User.findOne({ _id: order.seller_id });
const seller = await User.findOne({ _id: new ObjectId(order.seller_id!) });
if (seller === null) throw new Error('seller was not found');

if (buyer.lightning_address) {
Expand Down Expand Up @@ -228,12 +230,12 @@
const callerId = ctx.from?.id;

if (!orderId) return;
const order = await Order.findOne({ _id: orderId });
const order = await Order.findOne({ _id: new ObjectId(orderId) });

if (order === null) return;
const buyer = await User.findOne({ _id: order.buyer_id });
const buyer = await User.findOne({ _id: new ObjectId(order.buyer_id!) });
if (buyer === null) throw new Error('buyer was not found');
const seller = await User.findOne({ _id: order.seller_id });
const seller = await User.findOne({ _id: new ObjectId(order.seller_id!) });
if (seller === null) throw new Error('seller was not found');

let targetUser = buyer;
Expand Down Expand Up @@ -300,7 +302,7 @@
const orderId =
!!ctx && (ctx.update as any).callback_query.message.text;
if (!orderId) return;
order = await Order.findOne({ _id: orderId });
order = await Order.findOne({ _id: new ObjectId(orderId) });
}
}
if (order === null) return;
Expand All @@ -310,7 +312,7 @@
await cancelHoldInvoice({ hash: order.hash });
}

const user = await User.findOne({ _id: order.buyer_id });
const user = await User.findOne({ _id: new ObjectId(order.buyer_id!) });

if (user == null) return;

Expand All @@ -319,9 +321,9 @@
if (order.status !== 'WAITING_BUYER_INVOICE')
return await messages.genericErrorMessage(ctx, user, i18nCtx);

const sellerUser = await User.findOne({ _id: order.seller_id });
const sellerUser = await User.findOne({ _id: new ObjectId(order.seller_id!) });
if (sellerUser === null) throw new Error('sellerUser was not found');
const buyerUser = await User.findOne({ _id: order.buyer_id });
const buyerUser = await User.findOne({ _id: new ObjectId(order.buyer_id!) });
if (buyerUser === null) throw new Error('buyerUser was not found');
const sellerTgId = sellerUser.tg_id;
// If order creator cancels it, it will not be republished
Expand Down Expand Up @@ -413,11 +415,11 @@
if (!order) {
const orderId = (ctx.update as any).callback_query?.message?.text;
if (!orderId) return;
order = await Order.findOne({ _id: orderId });
order = await Order.findOne({ _id: new ObjectId(orderId) });
if (order === null || order === undefined) return;
}

const user = await User.findOne({ _id: order.seller_id });
const user = await User.findOne({ _id: new ObjectId(order.seller_id!) });
if (!user) return;

if (order.status !== 'WAITING_PAYMENT') {
Expand Down Expand Up @@ -507,7 +509,7 @@
const orderId =
!!ctx && (ctx.update as any).callback_query.message.text;
if (!orderId) return;
order = await Order.findOne({ _id: orderId });
order = await Order.findOne({ _id: new ObjectId(orderId) });
}
}
if (order === null) return;
Expand All @@ -516,9 +518,9 @@
if (order.hash) {
await cancelHoldInvoice({ hash: order.hash });
}
const buyerUser = await User.findOne({ _id: order.buyer_id });
const buyerUser = await User.findOne({ _id: new ObjectId(order.buyer_id!) });
if (buyerUser === null) throw new Error('buyerUser was not found');
const sellerUser = await User.findOne({ _id: order.seller_id });
const sellerUser = await User.findOne({ _id: new ObjectId(order.seller_id!) });
if (sellerUser === null) throw new Error('sellerUser was not found');

const i18nCtxSeller = await getUserI18nContext(sellerUser);
Expand Down Expand Up @@ -646,12 +648,12 @@
) => {
try {
ctx.deleteMessage();
const order = await Order.findOne({ _id: orderId });
const order = await Order.findOne({ _id: new ObjectId(orderId) });
if (order === null) throw new Error('order was not found');
// only orders with status PAID_HOLD_INVOICE are released payments
if (order.status !== 'PAID_HOLD_INVOICE') return;

const buyer = await User.findOne({ _id: order.buyer_id });
const buyer = await User.findOne({ _id: new ObjectId(order.buyer_id!) });
if (buyer === null) return;
if (order.amount === 0) {
await messages.genericErrorMessage(bot, buyer, ctx.i18n);
Expand Down Expand Up @@ -680,7 +682,7 @@
if (!user) return;
}
if (user.banned) return await messages.bannedUserErrorMessage(ctx, user);
const order = await ordersActions.getOrder(ctx, user, orderId);
const order = await ordersActions.getOrder(ctx, user, new ObjectId(orderId) as any);
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Mongoose 9.5, does Model.findOne({ _id: '<24-hex-string>' }) still auto-cast the string to ObjectId, or is explicit new mongoose.Types.ObjectId(...) required?

💡 Result:

Yes, in Mongoose 9.5, Model.findOne({ _id: '<24-hex-string>' }) still auto-casts the string to ObjectId. Explicit new mongoose.Types.ObjectId(...) is not required.

Citations:

🏁 Script executed:

# First, check the bot/commands.ts file around lines 685 and 905
head -n 710 bot/commands.ts | tail -n +670 | cat -n

Repository: lnp2pBot/bot

Length of output: 1809

🏁 Script executed:

# Check around line 905 too
head -n 920 bot/commands.ts | tail -n +890 | cat -n

Repository: lnp2pBot/bot

Length of output: 1145

🏁 Script executed:

# Find the ordersActions.getOrder function definition
rg "getOrder\(" bot/ --type ts -A 10 | head -100

Repository: lnp2pBot/bot

Length of output: 1138

🏁 Script executed:

# Find the ordersActions module and getOrder definition
fd "ordersActions" bot/ --type ts | head -5

Repository: lnp2pBot/bot

Length of output: 226

🏁 Script executed:

# Search more broadly for getOrder definition
rg "const getOrder|export.*getOrder|getOrder\s*=" bot/ --type ts -B 2 -A 15

Repository: lnp2pBot/bot

Length of output: 3605

🏁 Script executed:

# Get the complete getOrder implementation
rg "const getOrder = async" bot/ordersActions.ts -A 30

Repository: lnp2pBot/bot

Length of output: 704

Remove new ObjectId() wrapping and type-casting that bypasses getOrder's validation.

getOrder expects an orderId: string parameter and validates it via ObjectId.isValid(orderId) before showing the user-facing notValidIdMessage on invalid input. Wrapping orderId in new ObjectId(...) as any causes the constructor to throw synchronously on invalid 24-hex strings, bypassing validation and user-facing error messaging. The outer catch (error) { logger.error(error); } swallows the exception silently—a UX regression. Mongoose 9.5 still auto-casts string filters to ObjectId for schema-typed _id fields, so pass the string directly.

🔧 Proposed fix 
-    const order = await ordersActions.getOrder(ctx, user, new ObjectId(orderId) as any);
+    const order = await ordersActions.getOrder(ctx, user, orderId);

(apply at both line 685 and line 905)

📝 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 order = await ordersActions.getOrder(ctx, user, new ObjectId(orderId) as any);
const order = await ordersActions.getOrder(ctx, user, orderId);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bot/commands.ts` at line 685, The call currently constructs and casts an
ObjectId around orderId which bypasses ordersActions.getOrder's own validation;
update both occurrences (the call at the order retrieval near the earlier block
and the second similar call) to pass the raw string orderId directly to
ordersActions.getOrder so its ObjectId.isValid(orderId) check and user-facing
notValidIdMessage remain effective (remove new ObjectId(orderId) as any and pass
orderId: string to getOrder).


if (!order) return;

Expand All @@ -689,7 +691,7 @@
if (order.hash) await cancelHoldInvoice({ hash: order.hash });

order.status = 'CANCELED';
order.canceled_by = user._id;
order.canceled_by = user._id.toString();
await order.save();
OrderEvents.orderUpdated(order);
// we sent a private message to the user
Expand Down Expand Up @@ -737,16 +739,16 @@

const initiatorUser = user;
if (initiatorUser._id.toString() === order.buyer_id?.toString()) {
counterPartyUser = await User.findOne({ _id: order.seller_id });
counterPartyUser = await User.findOne({ _id: new ObjectId(order.seller_id!) });
initiator = UserOrderRole.BUYER;
} else {
counterPartyUser = await User.findOne({ _id: order.buyer_id });
counterPartyUser = await User.findOne({ _id: new ObjectId(order.buyer_id!) });
initiator = UserOrderRole.SELLER;
}
if (counterPartyUser == null)
throw new Error('counterPartyUser was not found');

const updateOrder = await setCooperativeCancelFlag(order._id, initiator);
const updateOrder = await setCooperativeCancelFlag(new ObjectId(order._id) as any, initiator);
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 26, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Drop the as any cast — setCooperativeCancelFlag accepts a string and order._id already serializes cleanly.

setCooperativeCancelFlag(orderId: string, ...) is typed for a string, but the call passes new ObjectId(order._id) as any — order._id is already a mongoose.Types.ObjectId, so the wrapping is a no-op other than forcing an unsafe cast. Use order._id.toString() and let the function's _id: new ObjectId(orderId) (line 46) do the conversion, or better, also drop that wrapping and rely on Mongoose's auto-cast.

🔧 Proposed fix 
-    const updateOrder = await setCooperativeCancelFlag(new ObjectId(order._id) as any, initiator);
+    const updateOrder = await setCooperativeCancelFlag(order._id.toString(), initiator);
📝 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 updateOrder = await setCooperativeCancelFlag(new ObjectId(order._id) as any, initiator);
const updateOrder = await setCooperativeCancelFlag(order._id.toString(), initiator);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bot/commands.ts` at line 751, The call to setCooperativeCancelFlag currently
wraps order._id with new ObjectId(...) and casts to any; remove the unsafe cast
and pass a string instead (e.g., use order._id.toString() or pass order._id
directly if already a string) so the call becomes
setCooperativeCancelFlag(order._id.toString(), initiator); update callers to
stop using "as any" and, optionally, remove the redundant new ObjectId(orderId)
conversion inside setCooperativeCancelFlag if you want to rely on Mongoose
auto-casting.


// If the call returns null, the flag was already set (or order is missing),
// so we treat it as a duplicate request.
Expand All @@ -768,12 +770,12 @@
if (updateOrder.hash) await cancelHoldInvoice({ hash: updateOrder.hash });

updateOrder.status = 'CANCELED';
updateOrder.canceled_by = String(user._id);
updateOrder.canceled_by = user._id.toString();
await updateOrder.save();

let seller = initiatorUser;
let i18nCtxSeller = ctx.i18n;
if (order.seller_id == counterPartyUser._id) {
if (order.seller_id?.toString() == counterPartyUser._id.toString()) {
seller = counterPartyUser;
i18nCtxSeller = i18nCtxCP;
}
Expand Down Expand Up @@ -830,7 +832,7 @@
if (!order) return;

order.status = 'FIAT_SENT';
const seller = await User.findOne({ _id: order.seller_id });
const seller = await User.findOne({ _id: new ObjectId(order.seller_id!) });
if (seller === null) throw new Error('seller was not found');
await order.save();
// We sent messages to both parties
Expand Down Expand Up @@ -868,7 +870,7 @@
const order = await validateReleaseOrder(ctx, user, orderId);
if (!order) return;
// We look for a dispute for this order
const dispute = await Dispute.findOne({ order_id: order._id });
const dispute = await Dispute.findOne({ order_id: order._id.toString() });

if (dispute) {
dispute.status = 'RELEASED';
Expand Down Expand Up @@ -900,7 +902,7 @@
if (!user) return;
}
if (user.banned) return await messages.bannedUserErrorMessage(ctx, user);
const order = await ordersActions.getOrder(ctx, user, orderId);
const order = await ordersActions.getOrder(ctx, user, new ObjectId(orderId) as any);

if (!order) return;
if (!order.hash) return;
Expand Down
Loading
Loading