From b9482ef227ab305b9907d6967b83842961f45be8 Mon Sep 17 00:00:00 2001
From: LauAubert <aubertlautaroa@gmail.com>
Date: Fri, 2 Oct 2026 02:03:58 -0300
Subject: [PATCH] fix: [TESIS-999021] keep a discard made while a retry of the
 event is running

The DLQ worker loads the failed event, replays it and then writes the result
without reading it again. If an operator discarded the event through the API
while the replay ran, a failed attempt wrote over the discard: the event went
back to pending with a next_retry_at and the sweep kept retrying something
the operator had taken out of the queue.

The result is now written under a row lock. A failed attempt leaves the
event alone if it is no longer processing, so the operator's decision wins.
A successful attempt is still recorded: the sale or shipment already went
through, and succeeded is as terminal as discarded while telling the truth.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 app/poros/webhooks/retry_failed_event.rb      | 36 +++++++++++++------
 .../poros/webhooks/retry_failed_event_spec.rb | 29 +++++++++++++++
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/app/poros/webhooks/retry_failed_event.rb b/app/poros/webhooks/retry_failed_event.rb
index d77421c..1532952 100644
--- a/app/poros/webhooks/retry_failed_event.rb
+++ b/app/poros/webhooks/retry_failed_event.rb
@@ -21,22 +21,38 @@ def call
 
     private
 
+    # Un éxito se registra aunque el evento se haya descartado mientras corría:
+    # la venta o el envío ya se procesaron, y `succeeded` es tan terminal como
+    # `discarded` —no vuelve a la cola—, pero además dice la verdad.
     def mark_succeeded
-      @event.update!(status: :succeeded, attempts: @event.attempts + 1,
-                     next_retry_at: nil, last_error: nil, claimed_at: nil)
+      @event.with_lock do
+        @event.update!(status: :succeeded, attempts: @event.attempts + 1,
+                       next_retry_at: nil, last_error: nil, claimed_at: nil)
+      end
       @event
     end
 
+    # El intento falló, pero el evento pudo cambiar de manos mientras corría: un
+    # operador lo descartó (o lo reencoló) desde la API. Esa decisión gana. Antes
+    # se escribía encima sin releer, el evento volvía a `pending` con un
+    # `next_retry_at` y el barrido lo seguía reintentando aunque lo hubieran
+    # descartado (hallazgo de la auditoría de TESIS-89).
+    #
+    # `with_lock` relee la fila con FOR UPDATE: la decisión del operador y este
+    # resultado no se pueden cruzar a mitad de camino.
     def mark_failed(error)
-      @event.attempts += 1
-      exhausted = @event.attempts_exhausted?
+      @event.with_lock do
+        next unless @event.processing?
 
-      @event.update!(
-        status: exhausted ? :dead : :pending,
-        next_retry_at: exhausted ? nil : FailedEvent.next_retry_at(@event.attempts),
-        claimed_at: nil,
-        **failure_details(error)
-      )
+        @event.attempts += 1
+        exhausted = @event.attempts_exhausted?
+        @event.update!(
+          status: exhausted ? :dead : :pending,
+          next_retry_at: exhausted ? nil : FailedEvent.next_retry_at(@event.attempts),
+          claimed_at: nil,
+          **failure_details(error)
+        )
+      end
       @event
     end
   end
diff --git a/spec/poros/webhooks/retry_failed_event_spec.rb b/spec/poros/webhooks/retry_failed_event_spec.rb
index 91b5c96..7c6acad 100644
--- a/spec/poros/webhooks/retry_failed_event_spec.rb
+++ b/spec/poros/webhooks/retry_failed_event_spec.rb
@@ -54,6 +54,35 @@ def retry_event = described_class.new(failed_event: event).call
     end
   end
 
+  # Hallazgo de auditoría (TESIS-89): el worker cargó el evento antes de que un
+  # operador lo descartara, y al terminar escribía su resultado encima. El
+  # evento volvía a `pending` y el barrido lo seguía reintentando.
+  context 'when an operator discards the event while the retry runs' do
+    def discard_meanwhile
+      worker = described_class.new(failed_event: event)
+      Webhooks::DiscardFailedEvent.new(failed_event: FailedEvent.find(event.id)).call
+      worker
+    end
+
+    it 'keeps it discarded when the attempt fails', :aggregate_failures do
+      stub_request(:post, url).to_return(status: 503, body: 'unavailable')
+
+      discard_meanwhile.call
+
+      expect(event.reload).to have_attributes(status: 'discarded', next_retry_at: nil)
+    end
+
+    # Ya se procesó: `succeeded` es tan terminal como `discarded` y dice la verdad.
+    it 'records the success when the attempt went through' do
+      stub_request(:post, url).to_return(status: 200, headers: { 'Content-Type' => 'application/json' },
+                                         body: { estado: 'Entregado' }.to_json)
+
+      discard_meanwhile.call
+
+      expect(event.reload.status).to eq('succeeded')
+    end
+  end
+
   context 'when the replay fails and attempts remain' do
     before { stub_request(:post, url).to_return(status: 503, body: 'unavailable') }