fix(idempotency): store observe results in the business transaction

.well-known/agent-skills/csharpessentials-efcore/SKILL.md

@@ -327,7 +327,7 @@ builder.Services.AddEfCoreIdempotencyRecordStore<AppDbContext>();            //
 builder.Services.AddEfCoreIdempotencyRecordStore<AppDbContext>("payments");  // or: keyed store for profiles with StoreKey = "payments"
 ```
 
-Records are written with SQL inside the context's current transaction, so a rolled-back request leaves no record and its retry runs again; a concurrent duplicate waits for the first commit and replays its response. The primary key is `KeyHash`, a `binary(32)` SHA-256 of the storage key, so keys that differ in case or trailing spaces stay separate under any collation; the readable `Namespace`/`Scope`/`Key` columns get the provider's binary collation (`KeyCollation` overrides it, e.g. `utf8mb4_nopad_bin` on MariaDB), which only makes them case-sensitive: SQL Server ignores trailing spaces under every collation, `_BIN2` included, so only the `KeyHash` key separates them there (on PostgreSQL never turn the columns into `char(n)`, which pads). Needs `READ COMMITTED`. In `Observe` mode the executor and the HTTP middleware use `store.Detached` (own context and connection, not atomic with the business data). Expired rows count as absent; call `PurgeExpiredAsync` from a hosted service to delete them.
+Records are written with SQL inside the context's current transaction, so a rolled-back request leaves no record and its retry runs again; a concurrent duplicate waits for the first commit and replays its response. The primary key is `KeyHash`, a `binary(32)` SHA-256 of the storage key, so keys that differ in case or trailing spaces stay separate under any collation; the readable `Namespace`/`Scope`/`Key` columns get the provider's binary collation (`KeyCollation` overrides it, e.g. `utf8mb4_nopad_bin` on MariaDB), which only makes them case-sensitive: SQL Server ignores trailing spaces under every collation, `_BIN2` included, so only the `KeyHash` key separates them there (on PostgreSQL never turn the columns into `char(n)`, which pads). Needs `READ COMMITTED`. In `Observe` mode the executor and the HTTP middleware reserve and release through `store.Detached` (own context and connection) and write a result to store through the store itself, in the business transaction, so a failed commit leaves no record and is not replayed after the switch to `Enforce`; a failed result write rolls back to a savepoint and the business data commits without the record (on PostgreSQL always so under an isolation level stricter than `READ COMMITTED`; on SQL Server a transaction doomed by `XACT_ABORT ON` or a `SNAPSHOT` update conflict, error 3960, is lost with the business data, because a savepoint cannot save it). Expired rows count as absent; call `PurgeExpiredAsync` from a hosted service to delete them.
 
 ---
 

CSharpEssentials.AspNetCore/Idempotency/IdempotencyMiddleware.cs

@@ -24,8 +24,11 @@ namespace CSharpEssentials.AspNetCore;
 /// The options are read once per request from <see cref="IOptionsMonitor{TOptions}"/>, so a reload applies to the next
 /// request; the endpoint's mode (<see cref="IdempotencyEndpointOptions.Mode"/>, its profile's, else
 /// <see cref="IdempotencyOptions.Mode"/>) decides whether the key is ignored, observed or enforced. In Observe an
-/// <see cref="ITransactionalIdempotencyRecordStore"/> is used through its
-/// <see cref="ITransactionalIdempotencyRecordStore.Detached"/> view, so observing never joins the request's transaction.
+/// <see cref="ITransactionalIdempotencyRecordStore"/> reserves and releases through its
+/// <see cref="ITransactionalIdempotencyRecordStore.Detached"/> view, so the reservation never joins the request's
+/// transaction. A response to store releases that reservation and is then written through the store itself, in the
+/// transaction open around <c>UseIdempotency</c> if there is one: the record exists only when that transaction
+/// commits, so a retry after the switch to Enforce never replays a response whose work rolled back.
 /// </summary>
 internal sealed partial class IdempotencyMiddleware(
     RequestDelegate next,
@@ -126,10 +129,11 @@ await WriteProblemAsync(context, StatusCodes.Status400BadRequest, "Idempotency s
 
         IdempotencyRecordKey recordKey = new(options.Namespace, scope.Value, key);
         IIdempotencyRecordStore store;
+        IIdempotencyRecordStore results;
         string? fingerprint;
         try
         {
-            store = settings.StoreKey is { } storeKey
+            store = results = settings.StoreKey is { } storeKey
                 ? context.RequestServices.GetRequiredKeyedService<IIdempotencyRecordStore>(storeKey)
                 : defaultStore;
             if (observe && store is ITransactionalIdempotencyRecordStore transactional)
@@ -141,7 +145,7 @@ await WriteProblemAsync(context, StatusCodes.Status400BadRequest, "Idempotency s
             LogObserveResolutionFailed(logger, exception, context.Request.Path);
             if (context.Request.Body.CanSeek)
                 context.Request.Body.Position = 0;
-            store = defaultStore;
+            store = results = defaultStore;
             fingerprint = null;
         }
 
@@ -172,7 +176,7 @@ await WriteProblemAsync(context, StatusCodes.Status400BadRequest, "Idempotency s
         switch (reservation.Status)
         {
             case IdempotencyReservationStatus.Reserved:
-                await ExecuteAsync(context, options, settings, store, recordKey, reservation.Token!, fingerprint, target).ConfigureAwait(false);
+                await ExecuteAsync(context, options, settings, store, results, recordKey, reservation.Token!, fingerprint, target).ConfigureAwait(false);
                 break;
             case IdempotencyReservationStatus.Completed when observe:
                 LogObserveWouldReplay(logger, context.Request.Path, new IdempotencyKeyHash(recordKey));
@@ -274,6 +278,7 @@ private async Task ExecuteAsync(
         IdempotencyOptions options,
         IdempotencyEndpointSettings settings,
         IIdempotencyRecordStore store,
+        IIdempotencyRecordStore results,
         IdempotencyRecordKey recordKey,
         string token,
         string fingerprint,
@@ -304,7 +309,7 @@ private async Task ExecuteAsync(
         try
         {
             bool filterInstalled = context.GetEndpoint()?.Metadata.GetMetadata<IdempotencyFilterInstalledMetadata>() is not null;
-            if (!await SettleAsync(context, options, settings, store, recordKey, token, fingerprint, capture, execution.HandlerStarted, filterInstalled, target).ConfigureAwait(false))
+            if (!await SettleAsync(context, options, settings, store, results, recordKey, token, fingerprint, capture, execution.HandlerStarted, filterInstalled, target).ConfigureAwait(false))
                 LogStaleReservation(logger, context.Request.Path, new IdempotencyKeyHash(recordKey));
         }
         catch (Exception exception)
@@ -318,14 +323,17 @@ private async Task ExecuteAsync(
     /// ignore <see cref="HttpContext.RequestAborted"/>: a client that went away still gets its outcome on retry.
     /// An endpoint without the execution filter cannot report whether its handler started, so a 2xx from it is
     /// settled as a started handler's response and every other status releases the key. A release or a stored response
-    /// is counted in <see cref="IdempotencyTelemetry.DecisionsInstrumentName"/>.
+    /// is counted in <see cref="IdempotencyTelemetry.DecisionsInstrumentName"/>. A response is written to
+    /// <paramref name="results"/>; when that is not the <paramref name="store"/> that holds the reservation, the
+    /// reservation is released first, so the response is written as the record of a key that is gone.
     /// </summary>
     /// <returns><see langword="false"/> when the reservation was taken over after <c>InFlightTimeout</c>.</returns>
     private async ValueTask<bool> SettleAsync(
         HttpContext context,
         IdempotencyOptions options,
         IdempotencyEndpointSettings settings,
         IIdempotencyRecordStore store,
+        IIdempotencyRecordStore results,
         IdempotencyRecordKey recordKey,
         string token,
         string fingerprint,
@@ -370,7 +378,9 @@ private async ValueTask<bool> SettleAsync(
                 headers[name] = [.. values.OfType<string>()];
         }
 
-        bool stored = await store.CompleteAsync(
+        if (!ReferenceEquals(store, results))
+            await TryReleaseAsync(store, recordKey, token, context.Request.Path).ConfigureAwait(false);
+        bool stored = await results.CompleteAsync(
             recordKey,
             token,
             fingerprint,

CSharpEssentials.AspNetCore/Idempotency/IdempotencyOptions.cs

@@ -20,7 +20,9 @@ public sealed class IdempotencyOptions
     /// answers differently from <see cref="IdempotencyMode.Off"/>: no replay, no <c>Idempotency-Replayed</c>, no
     /// <c>400</c>/<c>409</c>/<c>422</c>; it logs and counts what <see cref="IdempotencyMode.Enforce"/> would have done.
     /// Observe measures duplicates, it does not prevent them; keep it short for operations that move money. A record
-    /// stored in Observe is replayed once the mode becomes Enforce. This switch is separate from
+    /// stored in Observe is replayed once the mode becomes Enforce; an <see cref="ITransactionalIdempotencyRecordStore"/>
+    /// stores it in the transaction open around <c>UseIdempotency</c>, so a response whose transaction rolled back is
+    /// never replayed. This switch is separate from
     /// <c>IdempotencyExecutorOptions.Mode</c>, which Mediator and message consumers read. Default:
     /// <see cref="IdempotencyMode.Enforce"/>.
     /// </summary>

CSharpEssentials.AspNetCore/Readme.MD

@@ -272,8 +272,8 @@ app.MapPost("/payments", Pay).WithIdempotency(o => o.Mode = IdempotencyMode.Enfo
 
 - The options are read through `IOptionsMonitor<IdempotencyOptions>` once per request, so a reload of a bound configuration section changes `Mode`, or any other option, for the next request without a restart. Endpoint settings are resolved again after a reload. The store is chosen in `AddIdempotency`; a reload does not change it. A reload that makes the options invalid throws `OptionsValidationException` on every request to an idempotent endpoint until it is fixed. `IdempotencyOptions` resolved from DI is the current value.
 - The mode comes from the endpoint, else its profile, else `IdempotencyOptions.Mode`. It is separate from `IdempotencyExecutorOptions.Mode`, which the Mediator behavior and message consumers read.
-- After a switch from `Observe` to `Enforce`, the responses stored in `Observe` are replayed and a key still in flight gets 409.
-- `Observe` measures duplicates; it does not prevent them. Keep it short for endpoints that move money. A record that `Observe` stored for a request whose business commit then failed is replayed after the switch to `Enforce`: the [Idempotency Readme](../CSharpEssentials.Idempotency/Readme.MD#rollout-observe--enforce) has the rollout steps and this residual risk.
+- After a switch from `Observe` to `Enforce`, the responses stored in `Observe` are replayed and a key still in flight gets 409. With a store that writes in the request's transaction (the EF Core store) and a transaction open around `UseIdempotency`, a response is stored in that transaction, so a response whose commit failed is never replayed and its retry runs again.
+- `Observe` measures duplicates; it does not prevent them. Keep it short for endpoints that move money. A store that cannot join the request's transaction (in-memory, distributed cache, Redis) stores the response before a transaction open around `UseIdempotency` commits, in `Observe` and `Enforce` alike, so a response whose commit then failed is replayed: the [Idempotency Readme](../CSharpEssentials.Idempotency/Readme.MD#rollout-observe--enforce) has the rollout steps and what remains in `Observe`.
 - In `Observe`, a scope resolver, keyed store or fingerprint that throws is logged as `Idempotency.Observe.ResolutionFailed`, and a store that throws while reserving as `Idempotency.Observe.ReserveFailed`; the request then runs once without idempotency. In `Enforce`, or when the client aborted, the exception propagates and the handler does not run.
 - Decisions are counted on `idempotency.decisions` (meter `CSharpEssentials.Idempotency`) with `source` `http` and the route template as `target`. Logs carry a key hash, never the key or the scope. See the [Idempotency Readme](../CSharpEssentials.Idempotency/Readme.MD#telemetry) for the tags and the log events.