fix(persistence): make AmbientDbTransactionRegistry actually receive transaction events

src/BuildingBlocks/Persistence/AmbientDbTransactionRegistry.cs

@@ -11,6 +11,21 @@ namespace FSH.Framework.Persistence;
 /// attached to every Hero DbContext and records transactions as they start and end, which is what
 /// lets the outbox write enlist in the business transaction instead of committing separately.
 /// </summary>
+/// <remarks>
+/// <para>
+/// <b>Every method here must match <see cref="IDbTransactionInterceptor"/> exactly, sync *and*
+/// async.</b> The interface supplies default no-op implementations for all of its members, so a
+/// near-miss signature still compiles — it just silently never gets called, leaving this registry
+/// permanently empty and the outbox never enlisted.
+/// </para>
+/// <para>
+/// That failure mode is invisible on PostgreSQL: Npgsql associates a command with whatever
+/// transaction is open on its connection, so the outbox row joins the business transaction anyway.
+/// SQL Server does not — <c>SqlCommand.Transaction</c> must be set explicitly or execution throws
+/// "BeginExecuteReader requires the command to have a transaction…". Guarded by
+/// <c>AmbientDbTransactionRegistryTests</c>.
+/// </para>
+/// </remarks>
 public sealed class AmbientDbTransactionRegistry : IDbTransactionInterceptor
 {
     private readonly Dictionary<DbConnection, DbTransaction> _open = [];
@@ -22,21 +37,80 @@ public sealed class AmbientDbTransactionRegistry : IDbTransactionInterceptor
     public DbTransaction? Find(DbConnection connection)
         => connection is not null && _open.TryGetValue(connection, out var transaction) ? transaction : null;
 
-    public void TransactionStarted(DbConnection connection, TransactionEndEventData eventData)
-        => Track(connection, eventData?.Transaction);
+    public DbTransaction TransactionStarted(
+        DbConnection connection,
+        TransactionEndEventData eventData,
+        DbTransaction result)
+    {
+        Track(connection, result);
+        return result;
+    }
 
-    public void TransactionUsed(DbConnection connection, TransactionEventData eventData)
-        => Track(connection, eventData?.Transaction);
+    public ValueTask<DbTransaction> TransactionStartedAsync(
+        DbConnection connection,
+        TransactionEndEventData eventData,
+        DbTransaction result,
+        CancellationToken cancellationToken = default)
+    {
+        Track(connection, result);
+        return ValueTask.FromResult(result);
+    }
+
+    public DbTransaction TransactionUsed(
+        DbConnection connection,
+        TransactionEventData eventData,
+        DbTransaction result)
+    {
+        Track(connection, result);
+        return result;
+    }
+
+    public ValueTask<DbTransaction> TransactionUsedAsync(
+        DbConnection connection,
+        TransactionEventData eventData,
+        DbTransaction result,
+        CancellationToken cancellationToken = default)
+    {
+        Track(connection, result);
+        return ValueTask.FromResult(result);
+    }
 
     public void TransactionCommitted(DbTransaction transaction, TransactionEndEventData eventData)
         => Forget(transaction);
 
+    public Task TransactionCommittedAsync(
+        DbTransaction transaction,
+        TransactionEndEventData eventData,
+        CancellationToken cancellationToken = default)
+    {
+        Forget(transaction);
+        return Task.CompletedTask;
+    }
+
     public void TransactionRolledBack(DbTransaction transaction, TransactionEndEventData eventData)
         => Forget(transaction);
 
+    public Task TransactionRolledBackAsync(
+        DbTransaction transaction,
+        TransactionEndEventData eventData,
+        CancellationToken cancellationToken = default)
+    {
+        Forget(transaction);
+        return Task.CompletedTask;
+    }
+
     public void TransactionFailed(DbTransaction transaction, TransactionErrorEventData eventData)
         => Forget(transaction);
 
+    public Task TransactionFailedAsync(
+        DbTransaction transaction,
+        TransactionErrorEventData eventData,
+        CancellationToken cancellationToken = default)
+    {
+        Forget(transaction);
+        return Task.CompletedTask;
+    }
+
     private void Track(DbConnection connection, DbTransaction? transaction)
     {
         if (connection is not null && transaction is not null)
@@ -50,6 +124,17 @@ private void Forget(DbTransaction? transaction)
         if (transaction?.Connection is not null)
         {
             _open.Remove(transaction.Connection);
+            return;
+        }
+
+        // A disposed transaction reports a null Connection, so fall back to identity: leaving a
+        // completed transaction in the map would make the next write try to enlist in it.
+        if (transaction is not null)
+        {
+            foreach (var entry in _open.Where(e => ReferenceEquals(e.Value, transaction)).ToList())
+            {
+                _open.Remove(entry.Key);
+            }
         }
     }
 }

src/Tests/Framework.Tests/Persistence/AmbientDbTransactionRegistryTests.cs

@@ -0,0 +1,56 @@
+using FSH.Framework.Persistence;
+using Microsoft.EntityFrameworkCore.Diagnostics;
+using Shouldly;
+
+namespace Framework.Tests.Persistence;
+
+/// <summary>
+/// Guards the silent-no-op failure mode of <see cref="AmbientDbTransactionRegistry"/>.
+/// </summary>
+/// <remarks>
+/// <see cref="IDbTransactionInterceptor"/> gives every member a default no-op implementation, so a
+/// registry method whose signature does not match the interface still compiles and is simply never
+/// invoked. The registry then stays empty, the outbox never enlists in the business transaction,
+/// and the transactional-outbox guarantee is silently lost — which PostgreSQL masks, because Npgsql
+/// associates commands with the connection's open transaction regardless.
+/// </remarks>
+public class AmbientDbTransactionRegistryTests
+{
+    private static readonly string[] MustBeImplemented =
+    [
+        "TransactionStarted",
+        "TransactionStartedAsync",
+        "TransactionUsed",
+        "TransactionUsedAsync",
+        "TransactionCommitted",
+        "TransactionCommittedAsync",
+        "TransactionRolledBack",
+        "TransactionRolledBackAsync",
+        "TransactionFailed",
+        "TransactionFailedAsync"
+    ];
+
+    [Fact]
+    public void Registry_Should_Actually_Implement_Every_Interceptor_Hook_It_Relies_On()
+    {
+        var map = typeof(AmbientDbTransactionRegistry).GetInterfaceMap(typeof(IDbTransactionInterceptor));
+
+        var notWiredUp = new List<string>();
+        for (int i = 0; i < map.InterfaceMethods.Length; i++)
+        {
+            string name = map.InterfaceMethods[i].Name;
+            if (!MustBeImplemented.Contains(name)) continue;
+
+            // When a signature does not match, the interface's own default implementation is the
+            // target — meaning the registry's method is dead code.
+            if (map.TargetMethods[i].DeclaringType != typeof(AmbientDbTransactionRegistry))
+            {
+                notWiredUp.Add(name);
+            }
+        }
+
+        notWiredUp.ShouldBeEmpty(
+            "these IDbTransactionInterceptor hooks fall through to the interface default, so the "
+            + "registry never records transactions started via them: " + string.Join(", ", notWiredUp));
+    }
+}