CODE_OF_CONDUCT.md (1)

62-62: Include mailto hyperlink for effortless experience, Sir. The email is now neatly enclosed in angle brackets. For smooth user interaction, consider converting it into a clickable markdown link:

- Instances of abusive, harassing, or otherwise unacceptable behavior may be reported to the community leaders responsible for enforcement at <opensource@wgtechlabs.com>.
+ Instances of abusive, harassing, or otherwise unacceptable behavior may be reported to the community leaders responsible for enforcement at [opensource@wgtechlabs.com](mailto:opensource@wgtechlabs.com).

src/database/verify.js (1)

1-53: Sir, there appears to be redundancy in your verification systems.

This script serves a similar purpose to src/sdk/bots-brain/verify.js but uses direct pg access instead of the DatabaseConnection abstraction. Consider consolidating these verification approaches for consistency.

Would you prefer to standardize on the DatabaseConnection class approach, or shall I help you differentiate their specific purposes more clearly?

src/handlers/webhookMessage.js (1)

153-168: Sir, the Markdown sanitization may be overly aggressive for legitimate formatting.

Your current implementation escapes all square brackets and backticks, which could interfere with intentional code blocks or links in agent responses.

Perhaps a more nuanced approach would preserve functionality while maintaining security:

 sanitizeMessageText(text) {
   if (!text) return '';
   
   // Basic cleanup
   let cleaned = text.trim();
   
-  // Escape common Markdown characters that might break formatting
-  // But preserve basic formatting like *bold* and _italic_
-  cleaned = cleaned
-    .replace(/\\/g, '\\\\')  // Escape backslashes
-    .replace(/`/g, '\\`')    // Escape backticks
-    .replace(/\[/g, '\\[')   // Escape square brackets
-    .replace(/\]/g, '\\]');  // Escape square brackets
+  // Escape only problematic patterns while preserving legitimate formatting
+  cleaned = cleaned
+    .replace(/\\/g, '\\\\')  // Escape backslashes
+    // Only escape backticks that aren't part of code blocks
+    .replace(/(?<!`)`(?!`)/g, '\\`')
+    // Only escape brackets that look like broken links
+    .replace(/\[(?![^\]]*\]\([^\)]*\))/g, '\\[')
+    .replace(/\](?!\([^\)]*\))/g, '\\]');

   return cleaned;
 }

src/sdk/bots-brain/storage-cache-schema.sql (1)

30-40: Automate the janitor, not the manager.

The cleanup_expired_cache() function does its duty, yet nothing schedules it.
Consider attaching a simple pg_cron entry or a PostgreSQL ALTER EVENT TRIGGER to ensure automatic execution, preventing table bloat without relying on the application tier.

src/sdk/unthread-webhook/EventValidator.js (3)

81-84: Static context ambiguity — replace this with class name.

Using this.validate inside a static method works, but some linters (and the Biome hint) flag it as confusing.
A direct call clarifies intent:

-    if (!this.validate(event)) {
+    if (!EventValidator.validate(event)) {
       return null;
     }

🧰 Tools

🪛 Biome (1.9.4)

[error] 83-83: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

---

41-44: Graceful early-exit logging should use the global logger.

console.log is sprinkled throughout, yet the project now ships with a rich logger utility.
Switching keeps log styling consistent and permits log-level filtering:

-      console.log(`📋 Event not targeted for telegram platform: ${event.targetPlatform}`);
+      logger.info('Event ignored – wrong target platform', { target: event.targetPlatform });

(remember to import logger).

---

139-146: Missing null-guard for event.targetPlatform.

getEventSummary interpolates event.targetPlatform without a fallback.
A defensive default avoids “undefined” strings:

-    const conversationId = event.data?.conversationId || 'unknown';
+    const conversationId  = event.data?.conversationId || 'unknown';
     const textPreview     = event.data?.text?.substring(0, 50) || 'no text';
+    const target          = event.targetPlatform || 'unknown';
-
-    return `${event.type} from ${event.sourcePlatform} to ${event.targetPlatform} (${conversationId}): "${textPreview}${event.data?.text?.length > 50 ? '...' : ''}"`;
+    return `${event.type} from ${event.sourcePlatform} to ${target} (${conversationId}): "${textPreview}${event.data?.text?.length > 50 ? '...' : ''}"`;

src/utils/logger.js (1)

44-59: Legacy helper now orphaned.

getFormattedTimestamp() survived the refactor but is no longer consumed anywhere.
Deprecate or remove to keep the surface minimal and future-proof.

src/sdk/unthread-webhook/WebhookConsumer.js (1)

9-13: Sir, I detect a minor architectural inconsistency.

The constructor documentation mentions batch processing capabilities with a default batch size of 10, yet the implementation utilizes BLPOP which processes events individually. This discrepancy could confuse future maintainers.

Consider updating the documentation to reflect the current single-event processing pattern:

-    this.batchSize = config.batchSize || 10; // Process 10 events at a time
+    this.batchSize = config.batchSize || 10; // Reserved for future batch processing

src/events/message.js (2)

20-41: Potential memory creep in patternHandlers registry

Every registerTextPattern pushes into the global array but deregistration relies on callers invoking the returned function—a pattern easy to forget. Over time the bot could accumulate stale handlers (especially during hot-reload in dev). Consider:

1. Adding an upper-bound guard or duplicate-pattern detection.
2. Logging when a handler is registered/deregistered for observability.

I can draft an auto-cleanup strategy if desired.

---

381-382: Minor wording tweak for user-facing clarity

"Sorry, this bot does not have feature to assist you via Telegram DM" feels a tad robotic. Perhaps:

await ctx.reply("Sorry, I can’t assist via direct messages. Please talk to me in the group chat.");

Gentle polish goes a long way with users.

src/services/unthread.js (3)

32-34: customerCache declared but never read

The cache is exported yet unused inside this module—missing the very optimisation its comment promises. Before creating a customer, perform:

if (customerCache.has(groupChatName)) return customerCache.get(groupChatName);

and remember to customerCache.set(groupChatName, customer); after a successful call.

---

205-214: Unused local ticketData variable

ticketData is assembled but never referenced afterwards, leaving dead code:

-        const ticketData = {
-            ticketId,
-            friendlyId,
-            customerId,
-            chatId,
-            userId,
-            createdAt: Date.now()
-        };

Removing it avoids lint warnings and cognitive overhead.

---

41-61: Duplicate customer creation risk

createCustomer happily fires a POST every time and relies on external deduplication. Prefer:

1. Look-up in BotsStore or customerCache first.
2. Fall back to API creation only when unknown.

This prevents orphan “shadow” customers and keeps the Unthread dashboard tidy.

src/sdk/bots-brain/BotsStore.js (3)

44-50: Naming inconsistency detected in the static method.

Sir, I've noticed that the static method getTicketByTelegramMessageId delegates to getTicketByMessageId. For consistency across your interface, might I suggest aligning these names?

-  static async getTicketByTelegramMessageId(messageId) {
-    return BotsStore.getInstance().getTicketByMessageId(messageId);
+  static async getTicketByMessageId(messageId) {
+    return BotsStore.getInstance().getTicketByMessageId(messageId);

---

242-257: Consider implementing a limit for chat ticket lists.

Sir, the addToChatTickets method could potentially allow unbounded growth of the ticket array. For chats with extensive ticket history, this might impact performance. May I suggest implementing a sliding window approach?

 async addToChatTickets(chatId, messageId, conversationId) {
   const key = `chat:tickets:${chatId}`;
+  const MAX_TICKETS_PER_CHAT = 100; // Configurable limit
   const existingTickets = await this.storage.get(key) || [];
   
   // Check if ticket already exists
   const ticketExists = existingTickets.some(t => t.conversationId === conversationId);
   if (!ticketExists) {
     existingTickets.push({
       messageId,
       conversationId,
       addedAt: new Date().toISOString()
     });
     
+    // Keep only the most recent tickets
+    if (existingTickets.length > MAX_TICKETS_PER_CHAT) {
+      existingTickets.sort((a, b) => new Date(b.addedAt) - new Date(a.addedAt));
+      existingTickets.splice(MAX_TICKETS_PER_CHAT);
+    }
+    
     await this.storage.set(key, existingTickets);
   }
 }

---

305-313: Consider adding more detailed statistics.

Sir, while the current statistics are useful, you might benefit from including operational metrics such as storage operation counts or cache hit rates.

 async getStats() {
   const storageStats = this.storage.getStats();
   
   return {
     ...storageStats,
     sdk: 'bots-brain',
-    version: '1.0.0'
+    version: '1.0.0',
+    uptime: Date.now() - this.startTime, // Add startTime in constructor
+    operations: {
+      tickets: this.ticketOperations || 0,
+      customers: this.customerOperations || 0,
+      userStates: this.userStateOperations || 0
+    }
   };
 }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/sdk/bots-brain/BotsStore.js (1)

79-96: Input validation still missing – risk of corrupt data

The constructor comment from a previous review remains un-addressed: none of the required ticketData fields are verified before storage. Malformed objects will silently propagate through every cache layer.

This was already pointed out in an earlier review; please act on it.

src/commands/index.js (1)

236-240: Username needs sanitisation to generate a valid RFC-5322 email

This logic was highlighted before and is still present unchanged.
Usernames may contain spaces, emojis or special characters that break the pseudo-email format.

-const username = ctx.from.username || 'user';
-userState.ticket.email = `${username}_${userId}@telegram.user`;
+const rawUsername = ctx.from.username || 'user';
+const safeUsername = rawUsername.replace(/[^a-zA-Z0-9._-]/g, '_');
+userState.ticket.email = `${safeUsername}_${userId}@telegram.user`;

src/sdk/bots-brain/BotsStore.js (2)

48-50: Inconsistent naming may confuse future maintainers

getTicketByTelegramMessageId() delegates to getTicketByMessageId(), yet no instance-level method with the “Telegram” suffix exists.
Either rename the static helper or add an alias for symmetry.

---

350-366: Same concurrency concern applies to deletion

deleteTicket relies on a non-atomic sequence: fetch ticket → delete multiple keys. If another process edits those keys between the fetch and delete phases, dangling references may remain. Consider a transactional or batched delete.

src/commands/index.js (1)

146-149: Minor performance hint

BotsStore.getUserState is invoked on every incoming update, including non-text messages. If the memory cache misses, a database round-trip is executed unnecessarily.
Short-circuit earlier by checking ctx.message?.text || ctx.callbackQuery before hitting storage.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/sdk/bots-brain/UnifiedStorage.js (4)

33-35: ⚠️ Potential issue

Interval handle is never retained

Sir, the startMemoryCleanup() call fires an orphaned setInterval; without capturing the handle you cannot clear it later, leaving a dangling timer after disconnect().

-    // Start memory cleanup interval
-    this.startMemoryCleanup();
+    // Start memory cleanup interval
+    this._cleanupHandle = this.startMemoryCleanup();

---

77-88: 🛠️ Refactor suggestion

Timer leak on shutdown

disconnect() gracefully ends Redis & PG, yet the memory-cleanup interval continues to tick, preventing Node from exiting. Please clear the interval, using the handle captured above.

   async disconnect() {
     if (this.redisClient) {
       await this.redisClient.quit();
     }
@@
     }
-    this.connected = false;
+    if (this._cleanupHandle) {
+      clearInterval(this._cleanupHandle);
+    }
+    this.connected = false;
     LogEngine.info('UnifiedStorage disconnected');
   }

---

208-213: ⚠️ Potential issue

Double-parse hazard on JSONB value remains

pg already returns a JS object for jsonb. Re-parsing throws Unexpected token o…. The earlier remark still applies.

-      return result.rows.length > 0 ? JSON.parse(result.rows[0].value) : null;
+      return result.rows.length > 0 ? result.rows[0].value : null;

---

221-227: 🛠️ Refactor suggestion

Manual stringify forfeits JSONB advantages

Passing a string to a jsonb column sacrifices indexing; let Postgres accept raw JSON and cast in SQL.

-        INSERT INTO storage_cache (key, value, expires_at) 
-        VALUES ($1, $2, $3)
+        INSERT INTO storage_cache (key, value, expires_at) 
+        VALUES ($1, $2::jsonb, $3)
@@
-      `, [key, JSON.stringify(value), expiresAt]);
+      `, [key, value, expiresAt]);

src/commands/index.js (1)

239-245: Sanitize username before composing fallback e-mail
The raw username may contain characters that violate RFC 5322 and will render the address invalid. A past review already raised this; please reuse that sanitation snippet.

src/index.js (1)

238-272: Duplicate graceful-shutdown logic – extract to single helper
The SIGINT and SIGTERM handlers still copy-paste identical blocks. A helper such as gracefulShutdown(signal) keeps maintenance cost low and was suggested earlier.

src/sdk/unthread-webhook/WebhookConsumer.js (1)

104-110: Polling with setInterval risks overlaps; adopt self-scheduling loop
Previous review flagged the race condition—pollEvents can run longer than the interval, spawning concurrent executions. The issue remains unaddressed.

src/events/message.js (2)

69-91: Sir, this async pattern issue persists from previous analysis.

The pattern handlers require proper awaiting to prevent unhandled promise rejections and ensure proper execution flow, as previously identified.

---

274-274: The async diagnostic issue remains unresolved, sir.

The hasTicketInfo field continues to evaluate !!Promise rather than the actual boolean result, as previously identified in our analysis.

src/sdk/bots-brain/UnifiedStorage.js (2)

17-23: Mixed time-unit semantics invite mis-configuration

memoryTTL is stored in milliseconds while redisConfig.ttl expects seconds. The silent unit split is a classic foot-gun. Either document aggressively or normalise units.

---

242-253: Return interval handle for external control

startMemoryCleanup() currently returns nothing; letting the constructor capture a handle requires returning it.

-    setInterval(() => {
+    return setInterval(() => {
@@
-    }, 60000); // Clean up every minute
+    }, 60000); // Clean up every minute

src/sdk/unthread-webhook/EventValidator.js (1)

7-157: Static-only class can be a simple module

A class with nothing but static members adds ceremony without benefit. Exporting plain functions will shrink bundle size and please the linter complaining in static analysis.

-export class EventValidator {
+/* eslint-disable complexity/noStaticOnlyClass */
+export class EventValidator {
   // …existing code…
 }
+/* eslint-enable complexity/noStaticOnlyClass */
+
+// Or, refactor to:
+// export const EventValidator = { validate, validateAndSanitize, isForPlatform, isEventType, getEventSummary };

🧰 Tools

🪛 Biome (1.9.4)

[error] 7-157: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)

---

[error] 92-92: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

src/commands/index.js (1)

255-260: Possible chat title shadowing in getOrCreateCustomer callback
The lambda parameter chatTitle shadows the outer groupChatName but is never used; you’re discarding the value supplied by BotsStore.
Consider:

-                (chatTitle) => unthreadService.createCustomer(chatTitle)
+                title => unthreadService.createCustomer(title)

Even better, inline unthreadService.createCustomer if its signature already matches.

src/index.js (1)

55-67: Tighten null-checking with optional chaining
Static analysis is correct: ctx.message?.text is terser and avoids the double check.

-    if (ctx.message && ctx.message.text) {
+    if (ctx.message?.text) {

No behaviour change, just readability.

🧰 Tools

🪛 Biome (1.9.4)

[error] 55-55: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/sdk/unthread-webhook/WebhookConsumer.js (1)

13-15: batchSize config is unused – remove or implement batching
The constructor exposes batchSize, but pollEvents pops only one event each cycle. Either honour the setting (e.g., loop blPop up to batchSize times) or drop the option to avoid confusion.

src/services/unthread.js (1)

217-225: Minor data redundancy detected, sir.

The ticketId field appears twice in the BotsStore payload. Consider streamlining the data structure for cleaner storage operations.

await BotsStore.storeTicket({
    messageId: messageId,
    conversationId: ticketId,
    friendlyId: friendlyId,
    chatId: chatId,
    userId: userId,
-   ticketId: ticketId,
    createdAt: Date.now()
});

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/commands/index.js (2)

239-245: Sir, the email generation pattern still requires enhancement.

The current email format {username_id@telegram.user} may not comply with RFC 5322 email standards if the username contains special characters or is undefined, as previously noted.

---

267-272: Sir, the collected email still never reaches Unthread.

The email stored in userState.ticket.email remains absent from the createTicket payload. Unless the service back-fills this field elsewhere, this valuable information will be lost in the digital ether.

src/index.js (1)

238-272: Sir, the redundant shutdown handlers persist.

Both SIGINT and SIGTERM handlers contain identical shutdown logic. Extract this into a shared gracefulShutdown function to eliminate code duplication and improve maintainability.

src/events/message.js (2)

276-276: Sir, the hasTicketInfo diagnostic remains perpetually optimistic.

The expression !!unthreadService.getTicketFromReply(...) evaluates to !!Promise, which is always true, rendering the diagnostic meaningless. The function requires awaiting for accurate logging.

---

69-91: Sir, the pattern handlers require asynchronous coordination.

If registered handlers return Promises, invoking them without await will create unhandled promise rejections and race conditions. Mark processPatterns as async and await each handler invocation.

src/services/unthread.js (1)

385-399: Sir, the unimplemented function continues to masquerade as functional.

The getTicketsForChat function returns an empty array without indicating it's unimplemented, potentially misleading callers. Consider throwing an explicit "NotImplementedError" to communicate the current limitation clearly.

src/sdk/bots-brain/BotsStore.js (2)

102-120: Input validation is still missing for storeTicket.

The earlier suggestion to guard against falsy/invalid ticketData hasn’t been applied. An undefined conversationId or messageId will poison every key you write.

---

407-421: Potential write-clobber under concurrent calls.

addToChatTickets still performs a read-modify-write without locks/atomic helpers, so two parallel tickets for the same chat may silently overwrite each other. Please adopt an atomic list op (e.g., Redis LPUSH/SADD or INSERT … ON CONFLICT).

src/index.js (1)

55-67: Sir, I detect an opportunity for optional chaining enhancement.

The static analysis correctly identifies a simplification opportunity:

-    if (ctx.message && ctx.message.text) {
+    if (ctx.message?.text) {

🧰 Tools

🪛 Biome (1.9.4)

[error] 55-55: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/sdk/bots-brain/BotsStore.js (2)

243-244: Prefer LogEngine over bare console.log for consistency.

Two stray console.log calls bypass structured logging and environment-level log control. Replace with LogEngine.info to keep the log stream uniform.

-      console.log(`✅ Customer stored: ${customerName || chatTitle} (${unthreadCustomerId})`);
+      LogEngine.info(`Customer stored: ${customerName || chatTitle} (${unthreadCustomerId})`);
@@
-      console.log(`✅ User stored: ${unthreadName} (${telegramUserId})`);
+      LogEngine.info(`User stored: ${unthreadName} (${telegramUserId})`);

Also applies to: 284-285

---

109-110: Unused destructured property telegramUserId.

You extract telegramUserId but never reference it. Either store it with the ticket or drop the destructuring to avoid dead code.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/database/schema.sql (1)

12-12: Sir, the friendly_id uniqueness concern remains unaddressed.

The previous review correctly identified that the friendly_id should have a UNIQUE constraint to prevent duplicate human-readable ticket numbers, which could cause lookup confusion and routing failures.

src/index.js (1)

201-235: Sir, the graceful shutdown duplication concern persists.

The SIGINT and SIGTERM handlers contain identical shutdown sequences, as previously identified. This redundancy should be addressed for cleaner maintainability.

src/commands/index.js (2)

239-245: Sir, I must reiterate the email generation enhancement requirement.

The current email format may not comply with RFC standards if the username contains special characters, as previously noted.

         if (messageText.toLowerCase() === 'skip') {
-            // Generate email in format {username_id@telegram.user}
             const username = ctx.from.username || 'user';
-            userState.ticket.email = `${username}_${telegramUserId}@telegram.user`;
+            // Generate email with sanitized username to ensure RFC compliance
+            const sanitizedUsername = username.replace(/[^a-zA-Z0-9._-]/g, '_');
+            userState.ticket.email = `${sanitizedUsername}_${telegramUserId}@telegram.user`;

This ensures RFC 5322 compliance for all generated email addresses.

---

267-272: Sir, the collected email address is still not being transmitted to Unthread.

As noted in previous reviews, the email stored in userState.ticket.email never reaches the createTicket API call, causing the information to be lost.

             const ticketResponse = await unthreadService.createTicket({
                 groupChatName,
                 customerId,
                 summary,
-                onBehalfOf: userData
+                onBehalfOf: userData,
+                email: userState.ticket.email
             });

This ensures the user's email preference is properly included in the ticket creation.

src/events/message.js (1)

284-284: Sir, there's a logging inconsistency that requires immediate attention.

The function uses logger instead of LogEngine, which conflicts with the established pattern throughout the codebase and may cause runtime errors.

-        logger.info('Processing private message', {
+        LogEngine.info('Processing private message', {

This maintains consistency with the rest of your sophisticated logging architecture.

src/services/unthread.js (2)

148-148: Sir, the logging inconsistency persists and requires immediate attention.

The undefined logger variable continues to appear throughout the implementation, which will trigger runtime errors. All logging operations must utilize the properly imported LogEngine.

Also applies to: 399-399, 441-441, 453-453, 476-476, 502-502, 514-514

---

371-385: Sir, the unimplemented function continues to mislead callers.

The current implementation may deceive consumers into believing no tickets exist rather than communicating that the feature remains unimplemented.

src/sdk/bots-brain/BotsStore.js (3)

102-152: Sir, the input validation concern remains unaddressed.

The storeTicket method continues to operate without validating the integrity of incoming ticketData, potentially allowing corrupt or incomplete records to populate the storage system.

---

407-422: Sir, the concurrency vulnerability persists in the chat ticket management.

The read-modify-write sequence remains susceptible to lost updates when multiple tickets are created simultaneously for the same chat. An atomic operation or transaction would provide the necessary protection.

---

543-549: ⚠️ Potential issue

Sir, duplicate static method declarations will trigger a SyntaxError upon module initialization.

ECMAScript strictly prohibits duplicate class member names, and Node.js will refuse to load this module entirely.

Apply this correction to eliminate the duplicate declarations:

-  static async getCustomerByChatId(chatId) {
-    return BotsStore.getInstance().getCustomerByChatId(chatId);
-  }
-
-  static async storeCustomer(customerData) {
-    return BotsStore.getInstance().storeCustomer(customerData);
-  }

🧰 Tools

🪛 Biome (1.9.4)

[error] 543-547: Duplicate class member name "getCustomerByChatId"

(lint/suspicious/noDuplicateClassMembers)

src/database/schema.sql (1)

73-79: Excellent storage architecture, sir, though I detect a potential efficiency opportunity.

The three-layer storage cache design is quite sophisticated. However, consider adding a composite index on frequently queried combinations.

+-- Composite index for common cache lookups with expiration filtering
+CREATE INDEX IF NOT EXISTS idx_storage_cache_key_expires ON storage_cache(key, expires_at);

src/sdk/unthread-webhook/EventValidator.js (1)

7-21: Sir, while the validation logic is sound, I concur with the static analysis regarding class structure.

The current implementation as a static-only class could be simplified. Consider converting to simple exported functions for better tree-shaking and reduced complexity.

-export class EventValidator {
-  /**
-   * Validate message_created event structure
-   * @param {Object} event - The webhook event to validate
-   * @returns {boolean} - True if valid
-   */
-  static validate(event) {
-    return event && 
-           event.type === 'message_created' &&
-           event.sourcePlatform === 'dashboard' &&
-           event.data &&
-           event.data.conversationId &&
-           event.data.content;
-  }
-}
+/**
+ * Validate message_created event structure
+ * @param {Object} event - The webhook event to validate
+ * @returns {boolean} - True if valid
+ */
+export function validateEvent(event) {
+  return event && 
+         event.type === 'message_created' &&
+         event.sourcePlatform === 'dashboard' &&
+         event.data &&
+         event.data.conversationId &&
+         event.data.content;
+}

🧰 Tools

🪛 Biome (1.9.4)

[error] 7-21: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)

src/sdk/unthread-webhook/WebhookConsumer-simple.js (2)

57-57: Sir, I suggest implementing the optional chaining recommendation.

The static analysis correctly identifies an opportunity for safer property access.

-      if (this.redisClient && this.redisClient.isOpen) {
+      if (this.redisClient?.isOpen) {

🧰 Tools

🪛 Biome (1.9.4)

[error] 57-57: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

216-217: Another optional chaining opportunity detected, sir.

Similar to the previous case, this can be simplified for cleaner code.

-      isConnected: this.redisClient && this.redisClient.isOpen,
+      isConnected: this.redisClient?.isOpen,

🧰 Tools

🪛 Biome (1.9.4)

[error] 216-217: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/sdk/unthread-webhook/WebhookConsumer.js (2)

57-57: Sir, allow me to suggest a more elegant approach to this connection check.

Your current implementation works perfectly, though we could embrace modern JavaScript's optional chaining for a cleaner expression.

-      if (this.redisClient && this.redisClient.isOpen) {
+      if (this.redisClient?.isOpen) {

The functionality remains identical, but the code reads more effortlessly.

🧰 Tools

🪛 Biome (1.9.4)

[error] 57-57: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

189-194: Sir, the optional chaining pattern would enhance readability here as well.

Similar to the earlier suggestion, we can streamline this connection status check.

-      isConnected: this.redisClient && this.redisClient.isOpen,
+      isConnected: this.redisClient?.isOpen,

🧰 Tools

🪛 Biome (1.9.4)

[error] 191-192: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

README.md (1)

41-41: Sir, a minor enhancement to elevate the documentation's polish.

The static analysis tools suggest adding language specifications to these fenced code blocks for improved syntax highlighting:

-```
+```mermaid
 ┌─────────────────┐    ┌─────────────────┐

-```
+```text
 User: /support

-```
+```text
 [INFO] Database initialized successfully

This provides better rendering in documentation viewers.

Also applies to: 190-190, 325-325

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

41-41: Fenced code blocks should have a language specified
null

(MD040, fenced-code-language)

src/sdk/bots-brain/BotsStore.js (1)

243-243: Sir, logging inconsistencies compromise observability standards.

Direct console operations bypass the centralized LogEngine infrastructure, potentially fragmenting log aggregation and monitoring capabilities.

Standardize the logging approach:

-      console.log(`✅ Customer stored: ${customerName || chatTitle} (${unthreadCustomerId})`);
+      LogEngine.info('Customer stored successfully', {
+        customerName: customerName || chatTitle,
+        unthreadCustomerId,
+        chatId
+      });

Apply similar patterns to lines 284 and 287.

Also applies to: 284-284, 287-287

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/sdk/unthread-webhook/WebhookConsumer.js (2)

48-64: Consider using optional chaining for client status check
Sir, you can simplify the existence check with optional chaining for readability:

-    if (this.redisClient && this.redisClient.isOpen) {
+    if (this.redisClient?.isOpen) {

🧰 Tools

🪛 Biome (1.9.4)

[error] 57-57: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

188-195: Normalize isConnected to return boolean
Sir, using optional chaining and a boolean cast ensures a consistent return type:

-      isConnected: this.redisClient && this.redisClient.isOpen,
+      isConnected: !!this.redisClient?.isOpen,

🧰 Tools

🪛 Biome (1.9.4)

[error] 191-192: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/sdk/unthread-webhook/WebhookConsumer.js (1)

133-133: Sir, the Redis blPop signature requires immediate attention.

The implementation still uses the incompatible Node Redis v4 syntax. This will cause runtime failures when attempting to poll for events.

src/sdk/bots-brain/BotsStore.js (3)

543-549: Sir, we have a critical syntax violation that requires immediate resolution.

The duplicate static method definitions will cause a SyntaxError at runtime, preventing the module from loading entirely. Node.js strictly forbids duplicate class member names.

🧰 Tools

🪛 Biome (1.9.4)

[error] 543-547: Duplicate class member name "getCustomerByChatId"

(lint/suspicious/noDuplicateClassMembers)

---

407-422: Sir, the concurrent ticket addition pattern presents a potential data integrity concern.

The read-modify-write operation in addToChatTickets lacks atomic guarantees, which could result in lost ticket references when multiple operations occur simultaneously.

---

102-152: Sir, while the ticket storage architecture is quite sophisticated, input validation would enhance robustness.

The multi-key indexing approach is excellent for various lookup patterns, but the absence of input validation could lead to storage inconsistencies with malformed data.

src/sdk/unthread-webhook/WebhookConsumer.js (2)

57-57: Sir, may I suggest implementing optional chaining for enhanced resilience?

The current condition checks could be optimized using optional chaining to improve code clarity and prevent potential issues.

-      if (this.redisClient && this.redisClient.isOpen) {
+      if (this.redisClient?.isOpen) {

🧰 Tools

🪛 Biome (1.9.4)

[error] 57-57: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

191-191: Sir, another opportunity for optional chaining optimization.

This condition can be streamlined using optional chaining for better code maintainability.

-      isConnected: this.redisClient && this.redisClient.isOpen,
+      isConnected: this.redisClient?.isOpen,

🧰 Tools

🪛 Biome (1.9.4)

[error] 191-191: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/sdk/unthread-webhook/WebhookConsumer.js (4)

60-63: Nitpick: adopt optional chaining
Sir, you may streamline null checks by using optional chaining:

- if (this.redisClient && this.redisClient.isOpen) {
+ if (this.redisClient?.isOpen) {
...
- isConnected: this.redisClient && this.redisClient.isOpen,
+ isConnected: this.redisClient?.isOpen,

This aligns with modern JS idioms and satisfies the linter hint.

Also applies to: 193-194

🧰 Tools

🪛 Biome (1.9.4)

[error] 60-60: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

110-123: Optional: refine polling cadence
Sir, the current design waits pollInterval after pollForEvents completes, so effective delay = processing time + interval. For a fixed-rate schedule you might:

• Call scheduleNextPoll() before await this.pollForEvents()
• Compute the next timeout based on the original timestamp
• Or revert to setInterval guarded by an isRunning check

Choose based on your desired semantics.

---

182-184: Nitpick: log full error object for diagnostics
Sir, please log the entire error rather than just error.message to preserve stack traces:

- LogEngine.error('Error processing event:', error.message);
+ LogEngine.error('Error processing event:', error);

---

14-198: Suggestion: add tests for critical workflows
Sir, the WebhookConsumer covers connection, polling, validation, and dispatch. To ensure resilience under Redis failures, malformed payloads, and missing handlers, please introduce tests for:

• Successful connect/disconnect
• Polling with/without events
• Invalid JSON and unregistered types
• Graceful shutdown during an active poll

May I draft a Jest/Mocha scaffold for these scenarios?

🧰 Tools

🪛 Biome (1.9.4)

[error] 60-60: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

[error] 194-194: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/services/unthread.js (1)

371-385: Sir, this unimplemented function requires explicit indication as previously advised.

The current implementation continues to mislead callers by returning an empty array instead of indicating the feature is unimplemented.

As noted in previous reviews, this should throw an explicit error:

 export async function getTicketsForChat(chatId) {
-    try {
-        // Note: This would require a new method in BotsStore to search by chatId
-        // For now, we'll return an empty array and implement this if needed
-        LogEngine.debug('getTicketsForChat called but not yet implemented with BotsStore', { chatId });
-        return [];
-    } catch (error) {
-        LogEngine.error('Error getting tickets for chat', {
-            error: error.message,
-            stack: error.stack,
-            chatId
-        });
-        return [];
-    }
+    throw new Error('getTicketsForChat is not yet implemented - requires BotsStore enhancement');
 }

src/services/unthread.js (2)

23-77: Impressive string parsing algorithm, sir, though I detect optimization opportunities.

The extraction logic is comprehensive but could benefit from more robust pattern matching and edge case handling.

Consider these enhancements:

 function extractCustomerCompanyName(groupChatTitle) {
+    // Input validation
+    if (typeof groupChatTitle !== 'string') {
+        return 'Unknown Company';
+    }
+    
     if (!groupChatTitle) {
         return 'Unknown Company';
     }

Also consider using named capture groups in regex patterns for better maintainability:

-    const separatorPatterns = [
-        /\s+x\s+/,     // matches " x "
-        /\s*<>\s*/,    // matches "<>" with optional spaces
-        /\s*×\s*/,     // matches "×" with optional spaces
-        /\s+and\s+/,   // matches " and "
-        /\s*&\s*/      // matches "&" with optional spaces
-    ];
+    const separatorPattern = /\s*(?:x|<>|×|and|&)\s*/i;

---

115-116: Sir, I observe an unused caching mechanism.

The customerCache Map is declared and exported but never utilized in the actual functions, which suggests incomplete optimization implementation.

Either implement the cache in getOrCreateCustomer:

 export async function getOrCreateCustomer(groupChatName, chatId) {
     try {
+        // Check cache first
+        if (customerCache.has(chatId)) {
+            return customerCache.get(chatId);
+        }
+        
         // First, check if we already have this customer in our database by chat ID
         const existingCustomer = await BotsStore.getCustomerByChatId(chatId);
         if (existingCustomer) {
+            const customerData = {
+                id: existingCustomer.unthreadCustomerId,
+                name: existingCustomer.customerName
+            };
+            customerCache.set(chatId, customerData);
             LogEngine.info('Using existing customer from database', {

Or remove it if caching is not needed.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/handlers/webhookMessage.js:76

• Remove the 'this.' prefix from LogEngine calls since LogEngine is imported and used as a module. Ensure consistent usage throughout the file (also check similar occurrences on lines 103 and 109).

this.        LogEngine.info('Agent message delivered to Telegram', {

src/sdk/unthread-webhook/WebhookConsumer.js (5)

5-10: Add unit tests for WebhookConsumer
The consumer’s core methods (connect, start, pollForEvents, processEvent, stop) currently lack test coverage. Please introduce unit tests that mock Redis clients and verify event lifecycle behaviors.

Shall I prepare a Jest/Mocha scaffold with Redis stubs for you?

---

30-49: Enhance Redis error handling
Might I suggest binding 'error' events on both clients to LogEngine? This prevents unhandled promise rejections and surfaces connection issues proactively.

Example diff:

+      this.redisClient.on('error', err => LogEngine.error('Redis client error:', err));
+      this.blockingRedisClient.on('error', err => LogEngine.error('Blocking Redis client error:', err));

---

55-71: Use optional chaining in disconnect
The checks if (this.redisClient && this.redisClient.isOpen) and its blocking counterpart can be streamlined.

Example diff:

-      if (this.redisClient && this.redisClient.isOpen) {
-        await this.redisClient.disconnect();
-      }
+      if (this.redisClient?.isOpen) {
+        await this.redisClient.disconnect();
+      }
-      if (this.blockingRedisClient && this.blockingRedisClient.isOpen) {
-        await this.blockingRedisClient.disconnect();
-      }
+      if (this.blockingRedisClient?.isOpen) {
+        await this.blockingRedisClient.disconnect();
+      }

🧰 Tools

🪛 Biome (1.9.4)

[error] 65-65: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

[error] 69-69: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

161-195: Improve error logging in processEvent
When JSON parsing or handler execution fails, consider logging the full error object instead of .message to capture stacks.

Example diff:

-    } catch (error) {
-      LogEngine.error('Error processing event:', error.message);
-    }
+    } catch (error) {
+      LogEngine.error('Error processing event:', error);
+    }

---

201-209: Streamline status checks with optional chaining
The status flags can be concise: isConnected: this.redisClient?.isOpen and isBlockingClientConnected: this.blockingRedisClient?.isOpen.

Example diff:

-      isConnected: this.redisClient && this.redisClient.isOpen,
-      isBlockingClientConnected: this.blockingRedisClient && this.blockingRedisClient.isOpen,
+      isConnected: this.redisClient?.isOpen,
+      isBlockingClientConnected: this.blockingRedisClient?.isOpen,

🧰 Tools

🪛 Biome (1.9.4)

[error] 204-204: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

[error] 205-205: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro