# Reversal Implementation Gap Analysis **Date**: October 13, 2025 **Repository**: mercury_device_middlelayer **Branch**: main **Analysis Scope**: Transaction Reversal & Temp Table Management --- ## Executive Summary The codebase has **foundational reversal infrastructure** in place but lacks **operational automation** for the 5 critical reversal scenarios outlined in the requirements document. Key gaps include missing automatic reversal triggers, incomplete status lifecycle management, and lack of systematic cleanup orchestration. **Risk Level**: πŸ”΄ HIGH - Current implementation may leave transactions stuck in temp table without proper reversal processing. --- ## Gap Analysis Table | # | Scenario | Requirement | Current Implementation | Gap Status | Priority | Risk | |---|----------|-------------|------------------------|------------|----------|------| | **1** | **Duplicate Request Detection** | New request with stale temp (>45s) triggers reversal, then processes new request | βœ… `list_timed_out_temp_transactions/1` exists
❌ No automatic reversal orchestration
❌ No duplicate detection logic | πŸ”΄ **MAJOR GAP** | P0 | HIGH | | **2** | **Response Timeout Auto-Reversal** | Bank timeout (>30s) triggers automatic reversal with retry (max 3 attempts) | βœ… Timeout detection helper exists
❌ No automatic reversal trigger
❌ No retry mechanism
❌ Retry count not tracked for reversals | πŸ”΄ **MAJOR GAP** | P0 | HIGH | | **3** | **Connection Loss During Transaction** | TCP loss triggers recovery attempt, then automatic reversal | βœ… Connection health monitoring exists
❌ No connectionβ†’reversal mapping
❌ No affected transaction detection | 🟑 **PARTIAL** | P1 | MEDIUM | | **4** | **System Startup Cleanup** | Startup finds orphaned temps, creates reversals (never delete without reversal) | βœ… `cleanup_old_temp_transactions/1` exists
πŸ”΄ **Deletes directly without reversal**
❌ No startup reversal orchestration | πŸ”΄ **MAJOR GAP** | P0 | CRITICAL | | **5** | **Database Transaction Rollback** | DB error during tempβ†’final triggers automatic reversal | βœ… `finalize_transaction/2` has transaction wrapper
❌ No reversal on rollback
❌ No errorβ†’reversal handler | πŸ”΄ **MAJOR GAP** | P1 | HIGH | | **6** | **Reversal State Management** | Temp table tracks: REVERSAL_PENDING, REVERSAL_SENT, REVERSAL_COMPLETED, REVERSAL_FAILED, etc. | βœ… Basic status enum exists
❌ Missing reversal lifecycle states
❌ No REVERSAL_* statuses defined | πŸ”΄ **MAJOR GAP** | P0 | MEDIUM | | **7** | **Atomic Reversal Operations** | All reversal ops in DB transactions: update reversal β†’ save β†’ cleanup temp β†’ log | βœ… Some functions use `Repo.transaction`
🟑 Not consistently applied
❌ No unified atomic reversal helper | 🟑 **PARTIAL** | P1 | MEDIUM | | **8** | **Retry Mechanism** | Failed reversals retry with exponential backoff (max 3, delay 60s) | ❌ No retry tracking in schema
❌ No retry orchestration
❌ No exponential backoff | πŸ”΄ **MAJOR GAP** | P1 | MEDIUM | | **9** | **Monitoring & Alerting** | Track: stuck temp count, reversal success/failure rate, stale temp growth | βœ… Logging exists
❌ No Telemetry events for reversals
❌ No metrics collection
❌ No alerting | 🟑 **PARTIAL** | P2 | LOW | | **10** | **Configuration Management** | Centralized config for thresholds, timeouts, retry params | βœ… Config files exist
❌ Reversal-specific configs missing
❌ No config validation | 🟑 **PARTIAL** | P1 | LOW | --- ## Detailed Gap Breakdown ### 1. Duplicate Request Detection & Stale Transaction Cleanup **Requirement**: New MTI 0200/0100 with existing temp >45s old β†’ trigger reversal on stale, process new request **Current State**: - βœ… **EXISTS**: `DaProductApp.Acquirer.list_timed_out_temp_transactions/1` can find candidates - βœ… **EXISTS**: `pos_temp_transaction` schema with `created_dateTime` and `status` fields - ❌ **MISSING**: No service/worker consuming the list and triggering reversals - ❌ **MISSING**: No duplicate detection when new request arrives - ❌ **MISSING**: No atomicity guarantee between reversal creation and new request processing **Code References**: - `lib/da_product_app/acquirer.ex:369-380` - `list_timed_out_temp_transactions/1` - No consumer found for this function **Gap Impact**: Duplicate requests may overwrite or conflict with existing pending transactions --- ### 2. Response Timeout Auto-Reversal **Requirement**: Bank doesn't respond in 30s β†’ automatic reversal, retry up to 3 times with 60s delay **Current State**: - βœ… **EXISTS**: Timeout detection via `list_timed_out_temp_transactions/1` - βœ… **EXISTS**: Reversal creation APIs (`create_reversal/1`, `process_reversal/2`) - ❌ **MISSING**: No scheduled worker to check for timed-out transactions - ❌ **MISSING**: No retry counter in `pos_temp_transaction` or `pos_reversal` for reversal attempts - ❌ **MISSING**: No exponential backoff implementation - ❌ **MISSING**: No max retry limit enforcement - ⚠️ **ISSUE**: `TransactionProcessor.send_to_upstream/2` currently returns mock responses (deprecated) **Code References**: - `lib/da_product_app/acquirer/ysp/transaction_processor.ex:347-384` - `send_to_upstream/2` (mocked/deprecated) - `lib/da_product_app/acquirer/schemas/pos_temp_transaction.ex:107` - `retry_count` exists but used for transaction attempts, not reversal retry **Gap Impact**: Timed-out transactions remain stuck without automatic recovery --- ### 3. Connection Loss During Transaction **Requirement**: TCP connection lost β†’ attempt recovery β†’ trigger reversal if recovery fails **Current State**: - βœ… **EXISTS**: Connection health monitoring (`upstream_health_monitor.ex`) - βœ… **EXISTS**: Connection management infrastructure - ❌ **MISSING**: No mapping from connection/channel to affected temp transactions - ❌ **MISSING**: No event handler to detect connection loss and trigger reversals - ❌ **MISSING**: No channel_id/connection_id tracking in temp transaction metadata **Code References**: - `lib/da_product_app/switch/upstream_health_monitor.ex` - health monitoring - `lib/da_product_app/switch/upstream_connection_manager.ex` - connection management - No integration with reversal system found **Gap Impact**: Connection failures leave transactions orphaned without reversal --- ### 4. System Startup Cleanup **Requirement**: On startup, find orphaned temps and process reversals (**never delete without reversal**) **Current State**: - βœ… **EXISTS**: `cleanup_old_temp_transactions/1` for cleanup - πŸ”΄ **VIOLATION**: Current implementation **deletes rows directly** without creating reversals - ❌ **MISSING**: Startup reversal orchestration job - ❌ **MISSING**: Grace period before deletion - ❌ **MISSING**: Startup cleanup age threshold configuration **Code References**: - `lib/da_product_app/acquirer.ex:395-401` - `cleanup_old_temp_transactions/1` (direct delete) ```elixir def cleanup_old_temp_transactions(hours_old \\ 24) do cutoff_time = DateTime.add(DateTime.utc_now(), -hours_old * 3600, :second) from(t in PosTempTransaction, where: t.created_dateTime < ^cutoff_time ) |> Repo.delete_all() # πŸ”΄ DIRECT DELETE - VIOLATES REQUIREMENT end ``` **Gap Impact**: πŸ”΄ **CRITICAL** - Violates core principle #1, potential financial reconciliation issues --- ### 5. Database Transaction Rollback **Requirement**: DB error during tempβ†’final movement triggers automatic reversal **Current State**: - βœ… **EXISTS**: `finalize_transaction/2` wrapped in `Repo.transaction/1` - ❌ **MISSING**: No reversal creation in rollback handler - ❌ **MISSING**: No errorβ†’reversal orchestration - ❌ **MISSING**: No idempotency checks for rollback scenarios **Code References**: - `lib/da_product_app/acquirer.ex:69-92` - `finalize_transaction/2` ```elixir def finalize_transaction(temp_transaction, additional_attrs \\ %{}) do Repo.transaction(fn -> case create_pos_transaction(pos_transaction_attrs) do {:ok, pos_transaction} -> case Repo.delete(temp_transaction) do {:ok, _} -> pos_transaction {:error, reason} -> Repo.rollback(reason) # ❌ No reversal created end {:error, reason} -> Repo.rollback(reason) # ❌ No reversal created end end) end ``` **Gap Impact**: DB failures may leave transactions in indeterminate state without reversal notification to bank --- ### 6. Reversal State Management **Requirement**: Temp table tracks comprehensive reversal lifecycle states **Required States**: - ACTIVE - RESPONSE_TIMEOUT - CONNECTION_LOST - REVERSAL_PENDING - REVERSAL_SENT - REVERSAL_COMPLETED - REVERSAL_FAILED - REVERSAL_ERROR - PENDING_MANUAL_REVIEW **Current State**: - βœ… **EXISTS**: Status enum in `pos_temp_transaction` - Current statuses: ["CREATED", "VALIDATED", "SENT_TO_UPSTREAM", "WAITING_RESPONSE", "RESPONSE_RECEIVED", "PROCESSING_RESPONSE", "COMPLETED", "ERROR", "TIMEOUT", "PENDING", "APPROVED", "DECLINED", "REVERSED"] - ❌ **MISSING**: All REVERSAL_* lifecycle states - ❌ **MISSING**: CONNECTION_LOST status - ❌ **MISSING**: PENDING_MANUAL_REVIEW status **Code References**: - `lib/da_product_app/acquirer/schemas/pos_temp_transaction.ex:115-118` - status validation - `lib/da_product_app/acquirer/schemas/pos_reversal.ex:87` - reversal status validation **Gap Impact**: Cannot track reversal progress, difficult to debug stuck transactions --- ### 7. Atomic Reversal Operations **Requirement**: All reversal operations atomic - update reversal β†’ save β†’ cleanup temp β†’ log **Current State**: - βœ… **EXISTS**: Some functions use `Repo.transaction/1` - 🟑 **PARTIAL**: Not consistently applied across reversal flows - ❌ **MISSING**: Unified helper function `complete_reversal_and_cleanup/3` - ❌ **MISSING**: Idempotency guarantees via DB constraints **Code References**: - `lib/da_product_app/acquirer.ex:135-159` - `process_reversal/2` uses transaction - `lib/da_product_app/acquirer/ysp/transaction_processor.ex:267-287` - `finalize_reversal_transaction/3` updates separately (not atomic) **Gap Impact**: Race conditions, partial state updates, difficult rollback scenarios --- ### 8. Retry Mechanism **Requirement**: Failed reversals retry with exponential backoff (max 3 attempts, 60s base delay) **Current State**: - βœ… **EXISTS**: `retry_count` field in `pos_temp_transaction` - ❌ **MISSING**: Reversal-specific retry tracking - ❌ **MISSING**: `last_reversal_attempt_at` timestamp field - ❌ **MISSING**: Retry orchestration worker - ❌ **MISSING**: Exponential backoff calculation - ❌ **MISSING**: Max retry limit enforcement **Code References**: - `lib/da_product_app/acquirer/schemas/pos_temp_transaction.ex:107` - `retry_count` exists but for transaction, not reversal **Gap Impact**: Failed reversals not automatically retried, manual intervention required --- ### 9. Monitoring & Alerting **Requirement**: Track metrics - stuck temp count, reversal rates, stale temp growth, alert on anomalies **Current State**: - βœ… **EXISTS**: Extensive Logger.* calls throughout codebase - βœ… **EXISTS**: Telemetry dependency in `mix.exs` - ❌ **MISSING**: Telemetry events for reversal lifecycle - ❌ **MISSING**: Metrics collection for stuck transactions - ❌ **MISSING**: Dashboards/alerting configuration - ❌ **MISSING**: Operational runbooks **Code References**: - Deps include: `telemetry`, `telemetry_metrics`, `telemetry_poller` - No reversal-specific telemetry events found **Gap Impact**: No visibility into reversal health, delayed incident response --- ### 10. Configuration Management **Requirement**: Centralized config for all reversal thresholds and timeouts **Required Configs**: ```elixir reversal.stale.transaction.threshold.seconds = 45 reversal.response.timeout.seconds = 30 reversal.retry.max.attempts = 3 reversal.retry.delay.seconds = 60 connection.recovery.timeout.seconds = 30 connection.health.check.interval.seconds = 10 startup.cleanup.age.threshold.minutes = 5 scheduled.cleanup.interval.minutes = 15 ``` **Current State**: - βœ… **EXISTS**: Config files in `config/*.exs` - ❌ **MISSING**: All reversal-specific configurations - ❌ **MISSING**: Config validation module - ❌ **MISSING**: Runtime config access helpers **Code References**: - `config/dev.exs`, `config/prod.exs`, `config/runtime.exs` exist - Hardcoded timeout values found in multiple places (e.g., `timeout_minutes \\ 15` in `list_timed_out_temp_transactions/1`) **Gap Impact**: Difficult to tune reversal behavior, inconsistent timeouts across environments --- ## Additional Issues Discovered ### Issue A: Inconsistent Field Access Patterns **Location**: `lib/da_product_app/acquirer/ysp/transaction_event_listener.ex:485` ```elixir defp build_reversal_field_90(%ISOMsg{} = message) do orig_mti = get_field_safe(message, :original_mti, "0200") # ⚠️ Atom key on ISO message # ... end ``` **Problem**: Using atom key `:original_mti` instead of numeric field ID; ISOMsg.get expects integers **Impact**: May cause runtime errors or incorrect DE90 construction **Priority**: P1 --- ### Issue B: Deprecated Upstream Send **Location**: `lib/da_product_app/acquirer/ysp/transaction_processor.ex:347` ```elixir defp send_to_upstream(%ISOMsg{} = message, context) do # DEPRECATED: Direct upstream routing replaced by event-driven architecture Logger.warning("⚠️ DEPRECATED: send_to_upstream called directly") create_fallback_response(message, :deprecated_direct_call) # ❌ Returns mock end ``` **Problem**: Reversal upstream sends use mocked responses, not real routing **Impact**: Reversals not actually sent to bank during event-driven migration **Priority**: P0 --- ### Issue C: Duplicate Implementation **Location**: Multiple modules have similar reversal logic - `TransactionProcessor.process_reversal/2` - `TransactionEventListener.process_reversal_business_logic/3` **Problem**: Code duplication risk during migration to event-driven architecture **Impact**: Maintenance burden, potential behavior divergence **Priority**: P2 --- ## Summary Statistics | Metric | Count | |--------|-------| | **Total Scenarios** | 5 | | **Major Gaps** | 7 | | **Partial Implementations** | 4 | | **Complete Implementations** | 0 | | **Critical Issues** | 1 (Startup cleanup deletes without reversal) | | **High Priority Gaps** | 4 | | **Additional Issues** | 3 | --- ## Risk Assessment ### πŸ”΄ CRITICAL RISKS 1. **Startup cleanup deletes temp transactions without reversals** - Violates core requirement, potential financial loss 2. **No automatic reversal orchestration** - Transactions can get permanently stuck ### 🟠 HIGH RISKS 1. **Timeout scenarios not handled** - Bank may charge merchant without system knowing 2. **DB rollback doesn't trigger reversal** - Data inconsistency with bank 3. **Mocked upstream send for reversals** - Reversals not actually reaching bank ### 🟑 MEDIUM RISKS 1. **Missing reversal lifecycle states** - Difficult to debug, operational blind spots 2. **No retry mechanism** - Manual intervention required for failed reversals 3. **Connection loss not mapped to reversals** - Network issues cause orphaned transactions --- ## Next Steps See `REVERSAL_IMPLEMENTATION_PLAN.md` for prioritized implementation approach. --- **Document Version**: 1.0 **Last Updated**: October 13, 2025 **Status**: πŸ”΄ PENDING APPROVAL