# EIPConversation Engine — Detailed Analysis
**Date:** 2026-03-01
**Scope:** `FrameworkBLL.EIPConversation` + all callers, DAL, endpoints
**Status:** Work-in-progress codebase review

---

## Architecture Overview

EIPConversation is a multi-platform chatbot/conversational engine supporting WhatsApp,
Teams, Telegram, Slack, SMS, Email, and a Postman/internal TEXT channel. It drives
dynamic, configuration-based conversation flows over a webhook API.

```
HTTP POST /api/messagehub/webhook
    └── ReceiveMessageHubConversation (FastEndpoint)
         └── NormalizeInboundPayload() → MessageHubInboundDTO {Message, From, Platform}
              └── IEIPConversationBLL.HandleIncomingMessageAsync()
                   └── EIPConversationBLL
                        ├── GetOrCreateConversationSessionAsync()   ← cache → DAL
                        ├── DAL.AddConversationMessageAsync()       ← persist user message
                        ├── EIPConversationEngine.ProcessConversationStepAsync()
                        │    ├── ProcessTriggerStep()               ← keyword matching
                        │    ├── ProcessMessageStep()               ← static message
                        │    ├── ProcessInputStep()                 ← collect + validate
                        │    ├── ProcessChoiceStep()                ← menu selection
                        │    ├── ProcessActionStepAsync()           ← OTP / API / Conditional
                        │    ├── ProcessLoopStep()                  ← iteration
                        │    ├── ProcessFileUploadStep()            ← file
                        │    └── ProcessEndStep()                   ← terminate
                        ├── DAL.AddConversationMessageAsync()       ← persist bot reply
                        ├── ConversationPlatformType.SendMessageToPlatformAsync()
                        └── DAL.UpdateConversationLastActivityAsync()

GET /api/messagehub/webhook
    └── RequestMessageHubConversation  ← WhatsApp webhook verification handshake
```

### Key Files

| File | Role |
|------|------|
| `FrameworkBLL/EIPConversation/IEIPConversationBLL.cs` | Interface |
| `FrameworkBLL/EIPConversation/EIPConversationBLL.cs` | Orchestrator |
| `FrameworkBLL/EIPConversation/EIPConversationEngine.cs` | Flow processor |
| `FrameworkBLL/EIPConversation/ConversationPlatformType.cs` | Platform dispatcher |
| `FrameworkDAL/CustomCode/EIPConversation/IMessageHubConversationDAL.cs` | DAL interface |
| `FrameworkDAL/CustomCode/EIPConversation/MessageHubConversationDAL.cs` | DAL implementation |
| `FrameworkDAL/Query/EIPConversation/EIPConversationQB.cs` | SQL query builder |
| `FrameworkSL/Endpoints/EIPConversation/ReceiveMessageHubConversation.cs` | Webhook endpoint |
| `FrameworkSL/Endpoints/EIPConversation/RequestMessageHubConversation.cs` | WA verification endpoint |

---

## Domain Models

```csharp
MessageHubConversationDTO       // Session: one per user/channel session
  ├── SessionId, ClientId, PlatformTypeId, StatusId
  ├── CurrentStepId             // Tracks position in flow
  ├── Context: Dictionary<string,object>  // Runtime key-value state
  ├── Steps: List<ConversationFlowStepDTO>  // Flow definition
  └── Triggers: List<ConversationTriggerDTO>  // Entry keywords → StartStepId

ConversationFlowStepDTO         // One step in the conversation flow
  ├── StepId, Type (MESSAGE/INPUT/CHOICE/ACTION/LOOP/FILE_UPLOAD/END)
  ├── Message, Response         // Text shown to user (supports {placeholders})
  ├── Choices: Dict<string,int>  // Label → NextStepId
  ├── Validation (Required, Regex, MinLength, MaxLength)
  ├── ActionType (SEND_OTP, CALL_API, CONDITIONAL)
  ├── ApiConfig (Method, Url, Headers, BodyTemplate)
  ├── Conditions: List<ConditionDTO>  // Branching: IN/NOT IN/==/!=/>/<
  ├── NextStepId, DefaultNextStepId
  └── MaxIterations, LoopBackStepId, ExitStepId

MessageHubInboundDTO            // Normalized webhook payload (3 fields)
  ├── Message, From, Platform

Database tables:
  TEIPCONVERSATION              // One row per session
  TEIPCONVERSATIONDETAIL        // All messages (direction 0=user, 1=bot)
  TCONVERSATIONOTP              // OTP audit records
  TMESSAGEHUBSUPPORTREQUEST     // Support tickets
  MCONTACT                      // Contact/party master
  MPARTYPRODUCT                 // Party product list
```

---

## Critical Bugs

### BUG-1: `_sessionCache` is `static` — shared across ALL requests, ALL tenants, NEVER evicted
**File:** `EIPConversationBLL.cs` line 29

```csharp
private static readonly ConcurrentDictionary<string, MessageHubConversationDTO> _sessionCache = new();
```

`EIPConversationBLL` is registered as `Scoped` in DI, but `_sessionCache` is `static`.
This means:
- **Memory leak:** sessions accumulate indefinitely — no TTL, no eviction, no max size.
- **Cross-instance isolation broken:** horizontal scale-out means different pods have
  different caches; a session can stick to one pod and be "not found" on another.
- **All tenants share one cache:** session key uses `ClientId` so isolation is partial,
  but the Dict grows unbounded across all clients.
**Recommendation:** Replace with `IMemoryCache` (short TTL) or `IDistributedCache` (Redis)
with a configurable sliding expiry.

---

### BUG-2: Reply is sent to the wrong recipient — `loginContext.UserName` used as destination
**File:** `ConversationPlatformType.cs` lines 73–77

```csharp
var dto = new MessageHubDTO
{
    Recipient = loginContext.UserName ?? string.Empty,  // ← WRONG
    MessageText = message
};
```

`loginContext` is the **system operator** (server-side context), not the end user.
For WhatsApp/Telegram/SMS, `Recipient` must be the end user's phone number or chat ID
(i.e., `inbound.From`). This field is never passed into `SendMessageToPlatformAsync`.
**Impact:** Every outbound reply is routed to the wrong person (the system user), not the
actual user who sent the message. **All platform replies are broken.**

`HandleIncomingMessageAsync` call site (`EIPConversationBLL.cs` line 123):
```csharp
await _conversationPlatform.SendMessageToPlatformAsync(replyMessage, loginContext, platformName, ct);
// ← sessionToken (the actual destination) is never passed
```

---

### BUG-3: OTP verification is not implemented — `_otpStore` is declared but never used
**File:** `EIPConversationEngine.cs` lines 20–21

```csharp
private readonly ConcurrentDictionary<string, OtpEntry> _otpStore = new();
private const int MaxOtpAttempts = 5;
```

`HandleSendOtpAsync` (line 582) generates and sends OTP but **never adds to `_otpStore`**.
There is no `OTP_VERIFY` handler in the step-type switch (lines 63–73). The interface doc
says `OTP_VERIFY` is supported — it isn't.
- OTP expiry (stored as context value) is never validated.
- Max attempts (`MaxOtpAttempts = 5`) is never enforced.
- OTP is stored in plain text in `conversation.Context["OTP"]`.

---

### BUG-4: WhatsApp parsing is completely stubbed — platform non-functional
**File:** `ReceiveMessageHubConversation.cs` lines 197–201

```csharp
public class WhatsAppUpdate
{
    public string? GetMessageText() => string.Empty; // implement actual mapping
    public string? GetSender() => string.Empty;      // implement actual mapping
}
```

Both methods return empty string. Every WhatsApp webhook call results in:
- `inbound.Message = ""` → no step processing
- `inbound.From = "unknown"` → sessions keyed to "unknown" (all WhatsApp users share one session)

**WhatsApp integration is completely non-functional.**

---

### BUG-5: Session CurrentStepId is not resumed from DB — all sessions restart from triggers after app restart
**File:** `EIPConversationEngine.cs` lines 50–51

The engine checks `conversation.Context.ContainsKey("CurrentStepId")` to decide whether
to run trigger matching or resume a step. But `Context` is **not persisted** to the DB.
When a session is loaded from DB (`GetConversationSessionAsync`), only `conversation.CurrentStepId`
(the DB column) is populated; `Context["CurrentStepId"]` is not set.

Result: Every session loaded from DB (after a cache miss or restart) will re-run trigger
matching on the next message, regardless of where the flow was paused. In-progress
multi-step conversations are lost on restart.

---

### BUG-6: Duplicate session insert race condition (TOCTOU)
**File:** `MessageHubConversationDAL.cs` lines 72–83

```csharp
var existingConversationId = await _queryExecutor.ExecuteScalarAsync<long>(... SELECT TOP 1 ...);
if (existingConversationId > 0) return existingConversationId;
// Then INSERT...
```

Two concurrent requests for the same session token (rapid double-tap on WhatsApp) can
both pass the SELECT check simultaneously and both insert, creating duplicate session rows.
Needs a `UNIQUE` constraint on `(SESSIONTOKEN, CLIENTID)` + handle violation, or
`MERGE`/`INSERT WHERE NOT EXISTS`.

---

### BUG-7: Loop counter key mismatch — two different keys for the same counter
**File:** `EIPConversationEngine.cs`

`ProcessLoopStep` (line 315): `var counterKey = $"LoopCounter_{step.StepId}";`
`MoveToNextStep` loop handling (line 467): `var iterationKey = $"Loop_{nextStep.StepId}_Count";`

Both independently implement loop counting with **different key names** for the same step.
One path always reads 0 (key not found) and the other reads the correct count.
Loops configured through `MoveToNextStep` will never terminate if `ProcessLoopStep` is
the only updater — and vice versa.

---

### BUG-8: `GET_CONVERSATION` column alias mismatch — `MessageHubConversationId` always 0
**File:** `EIPConversationQB.cs` line 12 vs `MessageHubConversationDTO`

```sql
CONVERSATIONID AS ConversationId   -- alias is 'ConversationId'
```

But the DTO property is `MessageHubConversationId`. Dapper maps by alias, so
`MessageHubConversationId` is never populated from the DB read.
Every loaded session has `MessageHubConversationId = 0`. This breaks the foreign key
passed to `AddConversationMessageAsync(conversationId: 0)` — all messages are written
with `CONVERSATIONID = 0`.

---

### BUG-9: `INSERT_PARTYPRODUCT` includes `@OUID` but `SavePartyProduct` never provides it
**File:** `EIPConversationQB.cs` line 279 vs `MessageHubConversationDAL.cs` line 519–530

```sql
VALUES (@OUID, @PartyId, ...)   -- @OUID in SQL
```

```csharp
// parameters dict does not include "OUID"
{ "PartyId", dto.PartyId },
{ "PartyBranchId", dto.PartyBranchId },
...
```

**Runtime exception:** Dapper will throw "must declare scalar variable @OUID" (SQL Server)
or equivalent. `SavePartyProduct` will always fail.

---

## Serious Design Issues

### ISSUE-10: `TENANTID = -1` hardcoded throughout DAL — multi-tenancy broken
**File:** `MessageHubConversationDAL.cs` lines 41, 93, 157, 216, 251, 290, 338, 400, 477, 522

Every DAL call passes `TenantId = -1`. EIPConversation is functionally single-tenant
regardless of which tenant is actually using it. All records belong to tenant `-1`.

---

### ISSUE-11: `INSERT_CONTACT` drops FirstName, LastName, Salutation, Dob, PhoneNo, SourceType
**File:** `EIPConversationQB.cs` lines 219–248

`SaveContact` parses and populates all these fields in the DTO, but `INSERT_CONTACT` only
inserts `NAME, MOBILENO, EMAIL, STATUS`. The remaining contact data is silently lost even
though the name-splitting logic spent effort computing it.

---

### ISSUE-12: `SendMessageToPlatformAsync` has no `sessionToken` / `From` parameter
**File:** `ConversationPlatformType.cs` lines 47–51

```csharp
public async Task<bool> SendMessageToPlatformAsync(
    string message,
    LoginDTO loginContext,
    string platformName,
    CancellationToken ct)
```

The method signature has no way to accept the end-user's address. Platform adapters
(WhatsApp, Telegram, SMS) need the user's phone number / chat ID to route the reply.
**This is an architectural gap** — the interface needs a `string recipientAddress` parameter.

---

### ISSUE-13: No webhook signature verification — unauthenticated public endpoint
**File:** `ReceiveMessageHubConversation.cs` line 38

```csharp
AllowAnonymous();
```

The webhook has no HMAC/signature validation. Anyone can POST arbitrary payloads to
`/api/messagehub/webhook` and inject conversation messages, spoof sessions, or trigger
OTP sends. WhatsApp, Teams, and Telegram all provide cryptographic signatures — they must
be validated before processing.

---

### ISSUE-14: Platform detection by JSON string-contains is fragile
**File:** `ReceiveMessageHubConversation.cs` lines 114–158

```csharp
if (rawJson.Contains("\"update_id\""))           // Telegram
else if (rawJson.Contains("\"type\"") && rawJson.Contains("\"conversation\""))  // Teams
else if (rawJson.Contains("\"entry\"") && rawJson.Contains("\"changes\""))      // WhatsApp
```

Any payload with overlapping fields is misidentified. Platform should be declared
explicitly (via a URL route segment like `/webhook/telegram`, or an `X-Platform` header).

---

### ISSUE-15: Dual state tracking for `CurrentStepId` — can go out of sync
**File:** `EIPConversationEngine.cs` lines 412–413

```csharp
conversation.Context["CurrentStepId"] = nextStepId;   // in-memory context key
conversation.CurrentStepId = nextStepId;               // DTO property
```

Two sources of truth for the same value. `GetCurrentStep()` reads from `Context` (line 375),
`EIPConversationBLL` reads from `conversation.CurrentStepId` (line 69 log). If they diverge
(e.g., only one is updated in a code path), the engine and the logs show different steps.

---

### ISSUE-16: `ProcessEndStep` doesn't close the session in DB
**File:** `EIPConversationEngine.cs` lines 356–361

```csharp
private string ProcessEndStep(ConversationFlowStepDTO step, MessageHubConversationDTO conversation)
{
    conversation.Context?.Clear();
    return step?.Message ?? conversation.DefaultFallbackMessage ?? "Conversation ended.";
}
```

Context is cleared in memory but `CloseConversationSessionAsync` is never called.
The DB record remains `STATUS = 1` (active) after the END step, so the session is
never cleaned up.

---

### ISSUE-17: `HandleConditionalStep` uses simple `==` — inconsistent with rich condition engine
**File:** `EIPConversationEngine.cs` lines 607–615

```csharp
return value == step.ConditionValue
    ? MoveToNextStep(conversation, step.TrueStepId ?? 0)
    : MoveToNextStep(conversation, step.FalseStepId ?? 0);
```

This is a completely separate evaluation path from the rich condition engine in
`MoveToNextStep` (lines 437–450) which supports `IN`, `NOT IN`, `==`, `!=`, `CONTAINS`,
`>`, `<`, `>=`, `<=`. A CONDITIONAL ACTION step only gets `==` semantics.
These should share the same condition evaluation logic.

---

### ISSUE-18: `WhatsApp.SendWhatsappConversation` doesn't accept `CancellationToken`
**File:** `ConversationPlatformType.cs` lines 88–89

```csharp
case "WHATSAPP":
    await _whatsappPlatform.SendWhatsappConversation(dto, loginContext); // no ct
```

All other platforms accept `CancellationToken ct`. WhatsApp cannot be cancelled.
This is also inconsistent with its own `SendOtpToPlatformAsync` path (line 188) which has
the same problem.

---

## Medium Issues

### ISSUE-19: `INSERT_SUPPORT_REQUEST` has no `TENANTID` column
**File:** `EIPConversationQB.cs` lines 179–202

The `TMESSAGEHUBSUPPORTREQUEST` insert omits `TENANTID`. If the column is `NOT NULL`
in the schema this will fail at runtime. If nullable, all support records are unscoped.

---

### ISSUE-20: OTP stored in plain text in Context dictionary
**File:** `EIPConversationEngine.cs` lines 587–588

```csharp
conversation.Context["OTP"] = otp;
conversation.Context["OTP_EXPIRY"] = DateTime.UtcNow.AddMinutes(5);
```

OTP is readable from the context dictionary by any code that has access to the
conversation object. Context is also logged via `GetContextValue` debug calls.
OTP should be stored hashed (at minimum) and never in plain text in logs.

---

### ISSUE-21: Inline SQL in `CreateConversationSessionAsync` — inconsistent with QB pattern
**File:** `MessageHubConversationDAL.cs` lines 73–77

```csharp
@"SELECT TOP 1 CONVERSATIONID FROM TEIPCONVERSATION WHERE SESSIONTOKEN = @SessionId AND CLIENTID = @ClientId"
```

Should be in `EIPConversationQB` for consistency, testability, and easy review.

---

### ISSUE-22: `NormalizeInboundPayload` is private method on the endpoint class — untestable
**File:** `ReceiveMessageHubConversation.cs` lines 109–161

This is complex payload normalization logic that should be a separate injectable service or
static helper so it can be unit-tested independently of the HTTP endpoint.

---

### ISSUE-23: `SOURCETYPE = 5` is hardcoded in `INSERT_CONVERSATION`
**File:** `EIPConversationQB.cs` line 59

```sql
5,  -- hardcoded SOURCETYPE
```

Not configurable per platform or tenant. Should be driven by `PlatformTypeId` or a
constant/enum.

---

### ISSUE-24: `INSERT_CONVERSATION` does not include `TENANTID` in column list
**File:** `EIPConversationQB.cs` lines 34–65

`TENANTID` is in the VALUES clause as `@TenantId` but not listed in the column list.
SQL Server will accept this only if `TENANTID` is not in the table (and TENANTID = -1 is
passed as a parameter but mapped to a column without explicit listing). This should be
explicit for clarity and correctness.

---

## Issue Priority Summary

| # | Issue | Severity | Impact |
|---|-------|----------|--------|
| BUG-1 | Static session cache — memory leak + no eviction | **Critical** | Server OOM over time |
| BUG-2 | Reply sent to wrong recipient | **Critical** | All platform replies broken |
| BUG-3 | OTP verification not implemented | **Critical** | OTP security feature is a stub |
| BUG-4 | WhatsApp parsing returns empty — platform non-functional | **Critical** | WhatsApp dead |
| BUG-5 | CurrentStepId not resumed from DB after restart | **Critical** | All in-progress flows reset |
| BUG-6 | Duplicate session TOCTOU race condition | **Critical** | Duplicate session rows |
| BUG-7 | Loop counter key mismatch — loops never terminate | **Critical** | Infinite loop risk |
| BUG-8 | ConversationId alias mismatch — always 0 in DTO | **Critical** | All messages keyed to ID 0 |
| BUG-9 | `@OUID` in INSERT_PARTYPRODUCT not provided | **Critical** | Runtime exception on SavePartyProduct |
| ISSUE-10 | TenantId = -1 hardcoded — multi-tenancy broken | High | All records in single tenant |
| ISSUE-11 | INSERT_CONTACT drops FirstName/LastName/Salutation | High | Contact data silently lost |
| ISSUE-12 | `SendMessageToPlatformAsync` has no recipient parameter | High | Architectural gap |
| ISSUE-13 | No webhook signature validation | High | Security — open to injection |
| ISSUE-14 | Platform detection by JSON string-contains | High | Misidentification risk |
| ISSUE-15 | Dual tracking of CurrentStepId | Medium | State divergence |
| ISSUE-16 | END step doesn't close session in DB | Medium | Sessions never cleaned up |
| ISSUE-17 | HandleConditionalStep uses plain `==` only | Medium | Rich conditions unavailable |
| ISSUE-18 | WhatsApp missing CancellationToken | Medium | Cannot cancel WhatsApp calls |
| ISSUE-19 | INSERT_SUPPORT_REQUEST missing TENANTID | Medium | Potential schema violation |
| ISSUE-20 | OTP in plain text in Context | Medium | Security — OTP visible in logs |
| ISSUE-21 | Inline SQL outside QB | Low | Consistency / maintainability |
| ISSUE-22 | Payload normalizer untestable (private on endpoint) | Low | No unit test coverage |
| ISSUE-23 | SOURCETYPE hardcoded to 5 | Low | Not configurable |
| ISSUE-24 | TENANTID not listed in INSERT column list | Low | SQL clarity |

---

## Strengths (What Is Well Designed)

- **Rich flow engine:** MESSAGE, INPUT, CHOICE, ACTION, LOOP, FILE_UPLOAD, END step types
  provide a genuinely flexible conversational framework
- **Placeholder replacement:** `{ContextKey}` in message text, API URL, headers, body —
  config-driven dynamic content
- **Condition evaluation in MoveToNextStep:** supports IN, NOT IN, ==, !=, CONTAINS, >, <,
  >=, <= with numeric and string comparison
- **Recursive depth guard** (`MaxDepth = 20`) prevents infinite auto-advance recursion
- **Structured logging throughout:** ILogger used consistently in BLL, Engine, and DAL
  with session context in every log entry
- **Platform abstraction:** `ConversationPlatformType` cleanly separates routing from
  platform-specific send logic
- **File/platform-level DI registration** properly typed with nested interfaces
  (`IMessageHubPlatform.IWhatsAppPlatform` etc.)
- **Session key sanitization:** alphanumeric-only token prevents cache poisoning via
  special characters in session tokens
- **CancellationToken propagated** through most of the stack — async-friendly
- **Webhook verification endpoint** for WhatsApp (`GET /webhook`) is correctly separated
- **Contact normalization:** mobile number stripping + country code prepend + name splitting
  are useful real-world helpers

---

## Recommended Fixes (Action List)

1. **BUG-2 + ISSUE-12:** Add `string recipientAddress` to `SendMessageToPlatformAsync` and
   pass `sessionToken` (the `inbound.From` value) from `HandleIncomingMessageAsync`.

2. **BUG-3:** Implement OTP verification step handler — use `_otpStore` or the DB
   `TCONVERSATIONOTP` table; validate expiry and max attempts.

3. **BUG-4:** Implement `WhatsAppUpdate.GetMessageText()` and `GetSender()` with actual
   Meta Webhook JSON field traversal.

4. **BUG-5:** Persist `CurrentStepId` and serialized `Context` to the DB session row so
   sessions can be resumed after restart.

5. **BUG-6:** Add `UNIQUE` constraint on `TEIPCONVERSATION (SESSIONTOKEN, CLIENTID)` and
   use `MERGE` or `INSERT WHERE NOT EXISTS`.

6. **BUG-7:** Unify loop counter key to one pattern (remove `ProcessLoopStep` or merge
   its logic into `MoveToNextStep`).

7. **BUG-8:** Fix `GET_CONVERSATION` alias: `CONVERSATIONID AS MessageHubConversationId`.

8. **BUG-9:** Add `{ "OUID", dto.OuId }` (or appropriate value) to `SavePartyProduct`
   parameter dictionary.

9. **BUG-1:** Replace static `ConcurrentDictionary` with `IMemoryCache` with sliding
   expiry (e.g., 30 minutes), or use `IDistributedCache` for scale-out support.

10. **ISSUE-13:** Validate `X-Hub-Signature-256` header (WhatsApp) or equivalent
    per-platform signature on the webhook endpoint.

11. **ISSUE-14:** Use `X-Platform` request header or dedicated route segments
    (`/webhook/telegram`, `/webhook/whatsapp`) instead of JSON field sniffing.

12. **ISSUE-11:** Add FirstName, LastName, Salutation, Dob, PhoneNo, SourceType to
    `INSERT_CONTACT` SQL.

13. **ISSUE-10:** Pass actual `TenantId` from `loginContext` instead of hardcoded `-1`.

14. **ISSUE-16:** Call `CloseConversationSessionAsync` inside `ProcessEndStep` or in BLL
    after the engine returns the END step result.

15. **ISSUE-17:** Refactor `HandleConditionalStep` to use the same condition evaluator
    as `MoveToNextStep`.