fix(inventory): see a flock archived while feed usage waits on the item lock

docs/decisions/852-flock-contract.md

@@ -44,19 +44,21 @@ commands they carry, and the `FlockStatus` and `BirdMovementType` enums.
 `FlockRepository` and `BirdMovementRepository` are its listed implementations
 under #850.
 
-Each port method calls the repository method its caller used before, so the SQL,
-the tracking and the filters do not change. In particular:
+Each port method runs the query its caller ran before, so the SQL and the
+filters do not change. In particular:
 
 - The flock-scope read filter (#613) composes in every lookup, and
   `GetForFlockScopedWriteAsync` still reinstates `AccountId` itself (#388).
 - The resolved-actor requirement for flock-scoped writes (#787) stays in
   `FlockScopeGuard`, which the handlers still call first.
 - `RecordFeedUsageHandler` still locks the item, then reads eligibility, then
   reads lots, inside one transaction. The flock row stays unlocked.
-- The lookup reads stay tracked where they were tracked. `RecordFeedUsageHandler`
-  reads the same flock before and inside its transaction, and EF returns the
-  instance tracked by the first read. A no-tracking port would change what the
-  second read sees.
+- The lookup's two flock reads are untracked (#1022). `RecordFeedUsageHandler`
+  reads the same flock before and inside its transaction. While the first read
+  was tracked, EF handed the second read that same instance with its earlier
+  state, so a flock archived while the request waited on the item lock went
+  unseen. The tracked repository reads stay for the lifecycle handlers that
+  mutate the flock.
 
 ## Why three interfaces, not one
 

src/Cluckwork.Application/Features/Flocks/FlockLookup.cs

@@ -5,10 +5,10 @@ namespace Cluckwork.Application.Features.Flocks;
 public sealed class FlockLookup(IFlockRepository flocks) : IFlockLookup
 {
     public async Task<FlockDetails?> GetAsync(Guid id, CancellationToken ct) =>
-        await flocks.GetByIdAsync(id, ct) is { } flock ? ToDetails(flock) : null;
+        await flocks.GetReadOnlyAsync(id, ct) is { } flock ? ToDetails(flock) : null;
 
     public async Task<FlockDetails?> GetForFlockScopedWriteAsync(Guid id, Guid accountId, CancellationToken ct) =>
-        await flocks.GetByIdForFlockScopedWriteAsync(id, accountId, ct) is { } flock ? ToDetails(flock) : null;
+        await flocks.GetReadOnlyForFlockScopedWriteAsync(id, accountId, ct) is { } flock ? ToDetails(flock) : null;
 
     public Task<IReadOnlyDictionary<Guid, FlockReference>> GetDisplayNamesAsync(
         IReadOnlyCollection<Guid> ids, CancellationToken ct) =>

src/Cluckwork.Application/Features/Flocks/IFlockLookup.cs

@@ -10,7 +10,8 @@ public interface IFlockLookup
     Task<FlockDetails?> GetAsync(Guid id, CancellationToken ct);
 
     // For a write that has already passed IFlockScopeGuard; see
-    // IFlockRepository.GetByIdForFlockScopedWriteAsync. Takes no lock.
+    // IFlockRepository.GetByIdForFlockScopedWriteAsync. Takes no lock and tracks
+    // nothing, so a second call sees a change committed after the first (#1022).
     Task<FlockDetails?> GetForFlockScopedWriteAsync(Guid id, Guid accountId, CancellationToken ct);
 
     // See IFlockRepository.GetDisplayNamesAsync: a missing key is not "unnamed".

src/Cluckwork.Application/Features/Flocks/IFlockRepository.cs

@@ -53,4 +53,13 @@ Task<IReadOnlyDictionary<Guid, FlockReference>> GetDisplayNamesAsync(
     // state to an unassigned caller or crossing tenants.
     Task<Flock?> GetByIdForFlockScopedWriteAsync(
         Guid id, Guid accountId, CancellationToken ct = default);
+
+    // #1022 — untracked twins of GetByIdAsync and GetByIdForFlockScopedWriteAsync,
+    // for IFlockLookup. A tracked read hands back the instance the change tracker
+    // already holds, with the state it had when first loaded, so a second read in
+    // the same request would miss a lifecycle change committed in between.
+    Task<Flock?> GetReadOnlyAsync(Guid id, CancellationToken ct = default);
+
+    Task<Flock?> GetReadOnlyForFlockScopedWriteAsync(
+        Guid id, Guid accountId, CancellationToken ct = default);
 }

src/Cluckwork.Infrastructure/Repositories/FlockRepository.cs

@@ -20,6 +20,17 @@ public sealed class FlockRepository(AppDbContext db) : IFlockRepository
             .Where(f => f.AccountId == accountId)
             .FirstOrDefaultAsync(f => f.Id == id, ct);
 
+    public Task<Flock?> GetReadOnlyAsync(Guid id, CancellationToken ct = default) =>
+        db.Flocks.AsNoTracking().FirstOrDefaultAsync(f => f.Id == id, ct);
+
+    public Task<Flock?> GetReadOnlyForFlockScopedWriteAsync(
+        Guid id, Guid accountId, CancellationToken ct = default) =>
+        db.Flocks
+            .IgnoreQueryFilters()
+            .AsNoTracking()
+            .Where(f => f.AccountId == accountId)
+            .FirstOrDefaultAsync(f => f.Id == id, ct);
+
     // Read-only, paged. Archived flocks only surface in the management view.
     public async Task<IReadOnlyList<Flock>> ListAsync(
         int limit, int offset, bool includeArchived = false, CancellationToken ct = default) =>

tests/Cluckwork.Api.IntegrationTests/FeedUsageLockOrderTests.cs

@@ -0,0 +1,105 @@
+using System.Net;
+using Cluckwork.Api.IntegrationTests.Infrastructure;
+using Cluckwork.Infrastructure.Persistence;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Storage;
+
+namespace Cluckwork.Api.IntegrationTests;
+
+// RecordFeedUsage locks the item, then reads flock eligibility, then locks the
+// FIFO lots, in one transaction. A held row lock parks the usage request at one
+// step while an archive commits, so the outcome shows which side of that step
+// the eligibility read sits on.
+[Collection(IntegrationCollection.Name)]
+public sealed class FeedUsageLockOrderTests(CluckworkWebApplicationFactory factory)
+{
+    private sealed record Created(Guid Id);
+    private sealed record LotCreated(Guid LotId);
+
+    private static readonly DateOnly Today = DateOnly.FromDateTime(DateTime.UtcNow.Date);
+
+    // #1022: the eligibility read after the item lock must not reuse the flock
+    // instance loaded by the read before the transaction.
+    [Fact]
+    public async Task FlockArchivedWhileUsageWaitsForTheItemLock_IsRefused()
+    {
+        var (client, accountId, flockId, itemId, lotId) = await SetupAsync();
+
+        await using var holder = await HoldAsync(accountId,
+            db => db.Database.ExecuteSqlInterpolatedAsync(
+                $"""SELECT 1 FROM "InventoryItems" WHERE "Id" = {itemId} FOR UPDATE"""));
+        var usage = RecordUsageAsync(client, flockId, itemId);
+        Assert.True(await factory.WaitUntilDoneOrBlockedAsync(usage, holder.Pid),
+            "the usage request must park on the item lock");
+
+        await ArchiveAsync(accountId, flockId);
+        await holder.Transaction.CommitAsync();
+
+        var response = await usage;
+        Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode);
+        Assert.Equal("FeedUsage.FlockNotActive",
+            (await response.Content.ReadFromJsonAsync<ProblemDetails>())!.Title);
+        Assert.Equal((100m, 0), await LotAndUsagesAsync(accountId, lotId, flockId));
+    }
+
+    private async Task<(HttpClient Client, Guid AccountId, Guid FlockId, Guid ItemId, Guid LotId)> SetupAsync()
+    {
+        var email = $"u-{Guid.NewGuid():N}@test.local";
+        var accountId = await factory.SeedAccountWithUserAsync(email);
+        var flockId = await factory.SeedFlockAsync(accountId, Guid.NewGuid());
+        var client = factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(email));
+
+        var item = await client.PostWithKeyAsync("/api/v1/inventory/items", Guid.NewGuid().ToString(),
+            new { name = "Layer feed", category = "Feed", unit = "kg", defaultUnitCostMinorUnits = 2500 });
+        item.EnsureSuccessStatusCode();
+        var itemId = (await item.Content.ReadFromJsonAsync<Created>())!.Id;
+
+        var purchase = await client.PostWithKeyAsync(
+            $"/api/v1/inventory/items/{itemId}/purchases", Guid.NewGuid().ToString(),
+            new { receivedDate = Today.AddDays(-1), quantity = 100m, unitCostMinorUnits = 2500 });
+        purchase.EnsureSuccessStatusCode();
+        var lotId = (await purchase.Content.ReadFromJsonAsync<LotCreated>())!.LotId;
+        return (client, accountId, flockId, itemId, lotId);
+    }
+
+    private static Task<HttpResponseMessage> RecordUsageAsync(HttpClient client, Guid flockId, Guid itemId) =>
+        client.PostWithKeyAsync($"/api/v1/inventory/items/{itemId}/usage", Guid.NewGuid().ToString(),
+            new { flockId, date = Today, quantity = 5m });
+
+    // Built directly rather than from factory.Services: that context retries on
+    // failure, which forbids a hand-begun transaction (#269).
+    private async Task<Holder> HoldAsync(Guid accountId, Func<AppDbContext, Task> takeLock)
+    {
+        var tenant = new TenantContext();
+        tenant.Resolve(accountId);
+        var db = new AppDbContext(
+            new DbContextOptionsBuilder<AppDbContext>().UseNpgsql(factory.ConnectionString).Options,
+            tenant, new FlockScope());
+        var transaction = await db.Database.BeginTransactionAsync();
+        await takeLock(db);
+        return new Holder(db, transaction, await db.BackendPidAsync());
+    }
+
+    private Task ArchiveAsync(Guid accountId, Guid flockId) =>
+        factory.WithTenantScopeAsync(accountId, async db =>
+        {
+            var flock = await db.Flocks.SingleAsync(f => f.Id == flockId);
+            Assert.True(flock.Archive(Today).IsSuccess);
+            await db.SaveChangesAsync();
+        });
+
+    private Task<(decimal Available, int Usages)> LotAndUsagesAsync(Guid accountId, Guid lotId, Guid flockId) =>
+        factory.WithTenantScopeAsync(accountId, async db => (
+            (await db.InventoryLots.SingleAsync(l => l.Id == lotId)).QuantityAvailable,
+            await db.FeedUsages.CountAsync(u => u.FlockId == flockId)));
+
+    private sealed record Holder(AppDbContext Db, IDbContextTransaction Transaction, int Pid) : IAsyncDisposable
+    {
+        public async ValueTask DisposeAsync()
+        {
+            await Transaction.DisposeAsync();
+            await Db.DisposeAsync();
+        }
+    }
+}

tests/Cluckwork.Application.Tests/Flocks/FlockLookupTests.cs

@@ -78,14 +78,17 @@ public async Task ResolveByName_TwoMatchesAreAmbiguousWithBothCandidates()
 
     private sealed class StubFlocks(Flock? flock, IReadOnlyList<FlockReference>? byName = null) : IFlockRepository
     {
-        public Task<Flock?> GetByIdAsync(Guid id, CancellationToken ct = default) => Task.FromResult(flock);
+        public Task<Flock?> GetReadOnlyAsync(Guid id, CancellationToken ct = default) => Task.FromResult(flock);
 
-        public Task<Flock?> GetByIdForFlockScopedWriteAsync(Guid id, Guid accountId, CancellationToken ct = default) =>
+        public Task<Flock?> GetReadOnlyForFlockScopedWriteAsync(Guid id, Guid accountId, CancellationToken ct = default) =>
             Task.FromResult(flock);
 
         public Task<IReadOnlyList<FlockReference>> ListByNameAsync(string name, CancellationToken ct = default) =>
             Task.FromResult(byName ?? throw new NotSupportedException());
 
+        public Task<Flock?> GetByIdAsync(Guid id, CancellationToken ct = default) => throw new NotSupportedException();
+        public Task<Flock?> GetByIdForFlockScopedWriteAsync(Guid id, Guid accountId, CancellationToken ct = default) =>
+            throw new NotSupportedException();
         public Task<IReadOnlyList<Flock>> ListAsync(int limit, int offset, bool includeArchived = false, CancellationToken ct = default) =>
             throw new NotSupportedException();
         public Task<IReadOnlyList<Flock>> SearchAsync(string? search, FlockEligibility eligibility, int limit, int offset, CancellationToken ct = default) =>

tests/Cluckwork.Application.Tests/TenantBypass/Data/tenant-bypass-allowlist.json

@@ -104,6 +104,11 @@
     "file": "src/Cluckwork.Infrastructure/Repositories/FlockRepository.cs",
     "justification": "#388 post-authorization lifecycle lookup: after live FlockScopeGuard succeeds, bypasses the request-start flock snapshot so a newly assigned flock is lifecycle-checked; explicitly reinstates AccountId so foreign ids remain null. Read endpoints never call it."
   },
+  {
+    "symbol": "Cluckwork.Infrastructure.Repositories.FlockRepository.GetReadOnlyForFlockScopedWriteAsync(Guid id, Guid accountId, CancellationToken ct)",
+    "file": "src/Cluckwork.Infrastructure/Repositories/FlockRepository.cs",
+    "justification": "#1022 untracked twin of GetByIdForFlockScopedWriteAsync, same query plus AsNoTracking, for IFlockLookup's snapshot reads: after live FlockScopeGuard succeeds, bypasses the request-start flock snapshot and explicitly reinstates AccountId so foreign ids remain null. Read endpoints never call it."
+  },
   {
     "symbol": "Cluckwork.Infrastructure.Repositories.DailyEntryRepository.FindByNaturalKeyForFlockScopedWriteAsync(Guid accountId, Guid farmId, Guid houseId, Guid flockId, DateOnly date, CancellationToken ct)",
     "file": "src/Cluckwork.Infrastructure/Repositories/DailyEntryRepository.cs",