fix: write an audit row only when the retry happens

The audit was written before the work, in its own transaction, so a
retry that raised left a record of something that never happened. A
message retry that could not enqueue, and a scope retry whose revive
failed, both wrote one.

Each event goes in the transaction that causes it now. The same defect
was in the TypeScript port and is fixed there too, which is where it
surfaced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: cardmagic

lib/solid_objects/dead_letter_manager.rb

@@ -11,28 +11,18 @@ def all(authorization_context: nil)
     # @rbs (Integer, ?authorization_context: untyped) -> MessageReference
     def retry(dead_letter_id, authorization_context: nil)
       authorize!(:retry, authorization_context:, dead_letter_id:)
-      dead_letter = DeadLetter.find(dead_letter_id)
-      AdministrationAudit.record(
-        action: "dead_letter.retry",
-        kind: "message",
-        subject_id: dead_letter.id,
-        actor: AdministrationAudit.identity(authorization_context)
-      )
-      return MessageReference.from_message(Message.find(dead_letter.retried_message_id)) if dead_letter.retried_message_id
-
-      original_message = dead_letter.message
-      message_reference = Mailbox.new.enqueue(
-        reference: Reference.new(
-          actor_type: dead_letter.actor_type,
-          actor_id: dead_letter.actor_id
-        ),
-        operation: dead_letter.operation,
-        arguments: dead_letter.arguments,
-        delivery_mode: original_message.delivery_mode,
-        idempotency_key: "dead-letter:#{dead_letter.id}"
-      )
-      dead_letter.update!(retried_message_id: message_reference.id)
-      message_reference
+      actor = AdministrationAudit.identity(authorization_context)
+      SolidObjects.database_adapter.transaction do
+        dead_letter = DeadLetter.find(dead_letter_id)
+        reference = retried_reference(dead_letter)
+        AdministrationAudit.record(
+          action: "dead_letter.retry",
+          kind: "message",
+          subject_id: dead_letter.id,
+          actor:
+        )
+        reference
+      end
     end
 
     # @rbs () -> DeadLetterScope
@@ -57,6 +47,26 @@ def broadcasts
 
     private
 
+    # @rbs (DeadLetter) -> MessageReference
+    def retried_reference(dead_letter)
+      if dead_letter.retried_message_id
+        return MessageReference.from_message(Message.find(dead_letter.retried_message_id))
+      end
+
+      message_reference = Mailbox.new.enqueue(
+        reference: Reference.new(
+          actor_type: dead_letter.actor_type,
+          actor_id: dead_letter.actor_id
+        ),
+        operation: dead_letter.operation,
+        arguments: dead_letter.arguments,
+        delivery_mode: dead_letter.message.delivery_mode,
+        idempotency_key: "dead-letter:#{dead_letter.id}"
+      )
+      dead_letter.update!(retried_message_id: message_reference.id)
+      message_reference
+    end
+
     # @rbs (Symbol, authorization_context: untyped, ?dead_letter_id: Integer?) -> void
     def authorize!(action, authorization_context:, dead_letter_id: nil)
       authorized = SolidObjects.configuration.authorize_administration.call(

lib/solid_objects/dead_letter_scope.rb

@@ -36,17 +36,18 @@ def all(authorization_context: nil)
     # @rbs (String, ?authorization_context: untyped) -> untyped
     def retry(identifier_value, authorization_context: nil)
       authorize!(:retry, authorization_context:, resource_id: identifier_value)
-      row = model.find_by!(identifier => identifier_value)
-      AdministrationAudit.record(
-        action: "dead_letter.retry",
-        kind: kind,
-        subject_id: identifier_value,
-        actor: AdministrationAudit.identity(authorization_context)
-      )
-      return row unless row.status == DEAD
-
-      revive(row)
-      row
+      actor = AdministrationAudit.identity(authorization_context)
+      SolidObjects.database_adapter.transaction do
+        row = model.find_by!(identifier => identifier_value)
+        revive(row) if row.status == DEAD
+        AdministrationAudit.record(
+          action: "dead_letter.retry",
+          kind: kind,
+          subject_id: identifier_value,
+          actor:
+        )
+        row
+      end
     end
 
     # @rbs (?actor_type: String?, ?failed_after: untyped, ?limit: Integer?, ?authorization_context: untyped) -> RedriveTask

sig/generated/lib/solid_objects/dead_letter_manager.rbs

@@ -16,6 +16,9 @@ module SolidObjects
 
     private
 
+    # @rbs (DeadLetter) -> MessageReference
+    def retried_reference: (DeadLetter) -> MessageReference
+
     # @rbs (Symbol, authorization_context: untyped, ?dead_letter_id: Integer?) -> void
     def authorize!: (Symbol, authorization_context: untyped, ?dead_letter_id: Integer?) -> void
   end

test/integration/administration_audit_test.rb

@@ -100,6 +100,32 @@ def run
     assert_equal "user:42", SolidObjects::AdministrationEvent.sole.actor
   end
 
+  test "writes no audit row when the retry itself fails" do
+    effect = dead_effect
+
+    with_failing_method(SolidObjects::DeadLetterScope, :revive) do
+      assert_raises(RuntimeError) do
+        SolidObjects.dead_letters.effects.retry(effect.effect_id, authorization_context: "operator")
+      end
+    end
+
+    assert_equal 0, SolidObjects::AdministrationEvent.count
+  end
+
+  test "writes no audit row when a message retry fails to enqueue" do
+    PoisonActor.ref("one").async.run
+    run_actors
+    dead_letter = SolidObjects::DeadLetter.sole
+
+    with_failing_method(SolidObjects::Mailbox, :enqueue) do
+      assert_raises(RuntimeError) do
+        SolidObjects.dead_letters.retry(dead_letter.id, authorization_context: "operator")
+      end
+    end
+
+    assert_equal 0, SolidObjects::AdministrationEvent.count
+  end
+
   test "writes no audit row when the caller is refused" do
     effect = dead_effect
     SolidObjects.configuration.authorize_administration = ->(**) { false }
@@ -121,6 +147,14 @@ def run
 
   private
 
+  def with_failing_method(target, name)
+    original = target.instance_method(name)
+    target.define_method(name) { |*, **| raise "injected failure" }
+    yield
+  ensure
+    target.define_method(name, original)
+  end
+
   def dead_effect
     LedgerActor.ref("one").async.post
     run_actors