STE-113: Fix shared-content deletion bug in attachment storage

src/BitThicket.Steward.Api/AttachmentEndpoints.fs

@@ -53,6 +53,7 @@ type AttachmentResponse = {
     kind: string
     contentType: string
     sizeBytes: int64
+    storageRef: string
     uploadedAt: DateTimeOffset
 }
 
@@ -71,6 +72,7 @@ module private AttachmentHelpers =
                 | AttachmentKind.Other label -> $"other:{label}"
             contentType = a.ContentType
             sizeBytes = a.SizeBytes
+            storageRef = a.StorageRef
             uploadedAt = a.UploadedAt
         }
 
@@ -274,6 +276,11 @@ module AttachmentEndpoints =
         }
 
     // DELETE /api/attachments/{id}
+    ///
+    /// Deletes the attachment row first, then checks whether any other row in
+    /// the tenant still references the same content-addressed storage_ref.
+    /// Only deletes the on-disk file when the ref count drops to zero.
+    /// This fixes the shared-content deletion bug described in STE-113.
     let deleteAttachmentHandler (id: Guid) : HttpHandler = fun ctx ->
         task {
             let attachmentRepo = ctx.RequestServices.GetRequiredService<IAttachmentRepository>()
@@ -291,8 +298,13 @@ module AttachmentEndpoints =
                     ctx.Response.StatusCode <- 404
                     do! Response.ofJson {| error = "Attachment not found" |} ctx
                 | Some attachment ->
-                    do! storage.DeleteAsync tc.TenantId attachment.StorageRef
+                    // 1. Delete the attachment row first.
                     do! attachmentRepo.DeleteAsync(id)
+                    // 2. Check whether any other row still references the same file.
+                    let! remaining = attachmentRepo.CountByStorageRefAsync(attachment.StorageRef)
+                    // 3. Only delete the on-disk blob when no refs remain.
+                    if remaining = 0 then
+                        do! storage.DeleteAsync tc.TenantId attachment.StorageRef
                     ctx.Response.StatusCode <- 204
                     do! Response.ofEmpty ctx
         }

src/BitThicket.Steward.Api/AttachmentRepository.fs

@@ -13,6 +13,9 @@ type IAttachmentRepository =
     abstract ListBySplitAsync : splitId:Guid -> Task<Attachment list>
     abstract CreateAsync : attachment:Attachment -> Task<Guid>
     abstract DeleteAsync : id:Guid -> Task<unit>
+    /// Count how many attachment rows in the current tenant still reference the given storage_ref.
+    /// Used by the delete handler to avoid removing shared content-addressed blobs. (STE-113)
+    abstract CountByStorageRefAsync : storageRef:string -> Task<int>
 
 module AttachmentRepository =
 
@@ -138,6 +141,17 @@ module AttachmentRepository =
             return ()
         }
 
+    let countByStorageRefAsync (factory: IDbConnectionFactory) (tenantContext: TenantContext) (storageRef: string) =
+        task {
+            use! conn = factory.OpenForTenantAsync(tenantContext)
+            use cmd = conn.CreateCommand()
+            cmd.CommandText <- "SELECT COUNT(*) FROM attachments WHERE storage_ref = $1 AND tenant_id = $2"
+            cmd.Parameters.AddWithValue("$1", storageRef) |> ignore
+            cmd.Parameters.AddWithValue("$2", tenantContext.TenantId) |> ignore
+            let! result = cmd.ExecuteScalarAsync()
+            return Convert.ToInt32(result)
+        }
+
     let create (factory: IDbConnectionFactory) (accessor: ITenantContextAccessor) : IAttachmentRepository =
         let requireCtx () =
             match accessor.Context with
@@ -150,4 +164,5 @@ module AttachmentRepository =
             member _.ListBySplitAsync(splitId) = listBySplitAsync factory (requireCtx()) splitId
             member _.CreateAsync(attachment) = createAsync factory attachment
             member _.DeleteAsync(id) = deleteAsync factory (requireCtx()) id
+            member _.CountByStorageRefAsync(storageRef) = countByStorageRefAsync factory (requireCtx()) storageRef
         }

src/BitThicket.Steward.Api/Program.fs

@@ -930,12 +930,10 @@
         post "/api/connections/{connectionId:guid}/reauth" (AuthHelpers.requireAuth (fun ctx ->
             let connectionId = ctx.Request.RouteValues.["connectionId"] :?> Guid
             ConnectionEndpoints.reauthConnectionHandler connectionId ctx))
-=======
         // Transfers and credit card payments
         post "/api/transfers" (AuthHelpers.requireAuth TransferEndpoints.createTransferHandler)
         post "/api/credit-card-payments" (AuthHelpers.requireAuth TransferEndpoints.createCreditCardPaymentHandler)
         get "/api/credit-card-payments" (AuthHelpers.requireAuth TransferEndpoints.listCreditCardPaymentsHandler)
->>>>>>> 47f0a62 (STE-25: credit card payments + transfers)
         get "/api/transactions/needs-review" (AuthHelpers.requireAuth needsReviewHandler)
         post "/api/transactions/resolve" (AuthHelpers.requireAuth resolveHandler)
         post "/internal/transactions/upsert" internalUpsertHandler
@@ -951,7 +949,7 @@
                 let responses =
                     categories
                     |> List.map (fun c ->
                         {| id = c.Id; name = c.Name; color = (None :> string option) |})
                 do! Response.ofJson {| categories = responses |} ctx
             }))
         get "/api/budgets" (AuthHelpers.requireAuth BudgetEndpoints.listBudgetsHandler)

src/BitThicket.Steward.Api/TransferEndpoints.fs

@@ -216,7 +216,7 @@
 
     // POST /api/transfers
     let createTransferHandler : HttpHandler = fun ctx ->
         task {
             let factory = ctx.RequestServices.GetRequiredService<IDbConnectionFactory>()
             let accountRepo = ctx.RequestServices.GetRequiredService<IAccountRepository>()
             let! doc = TransferJson.readBody ctx
@@ -279,6 +279,7 @@
                             MatchConfidence = None
                             SyncEventId = None
                             CreatedAt = now
+                            DeletedAt = None
                             UpdatedAt = now
                         }
 
@@ -301,6 +302,7 @@
                             MatchConfidence = None
                             SyncEventId = None
                             CreatedAt = now
+                            DeletedAt = None
                             UpdatedAt = now
                         }
 
@@ -325,7 +327,7 @@
 
     // POST /api/credit-card-payments
     let createCreditCardPaymentHandler : HttpHandler = fun ctx ->
         task {
             let factory = ctx.RequestServices.GetRequiredService<IDbConnectionFactory>()
             let accountRepo = ctx.RequestServices.GetRequiredService<IAccountRepository>()
             let! doc = TransferJson.readBody ctx
@@ -404,6 +406,7 @@
                                         MatchConfidence = None
                                         SyncEventId = None
                                         CreatedAt = now
+                                        DeletedAt = None
                                         UpdatedAt = now
                                     }
 
@@ -426,6 +429,7 @@
                                         MatchConfidence = None
                                         SyncEventId = None
                                         CreatedAt = now
+                                        DeletedAt = None
                                         UpdatedAt = now
                                     }
 

test/BitThicket.Steward.Api.Test/AttachmentEndpointsTests.fs

@@ -221,3 +221,65 @@ type AttachmentEndpointsTests() =
             do! AttachmentEndpoints.getAttachmentHandler attachmentId getCtx
             test <@ getCtx.Response.StatusCode = 404 @>
         }
+
+    /// STE-113: deleting one attachment must not remove the on-disk blob when
+    /// another attachment row references the same content-addressed storage_ref.
+    [<Fact>]
+    member _.``DELETE /api/attachments/{id} does not remove shared file when another attachment references same storage_ref``() =
+        task {
+            if not (canConnect ()) then return () else
+
+            let cs = connectionString ()
+            runMigrations cs
+            use dataSource = NpgsqlDataSource.Create(cs)
+            let factory = DbConnectionFactory(dataSource) :> IDbConnectionFactory
+
+            let! token = registerAndGetToken factory "attshare@example.com"
+            let jwtDoc =
+                let ctx = createHttpContextWithAuth factory token
+                setJsonBody ctx """{"email":"x","password":"x","displayName":"x","tenantDisplayName":"x"}"""
+                Auth.registerHandler ctx |> Async.AwaitTask |> Async.RunSynchronously
+                readResponseJson ctx
+            let tenantId = Guid.Parse(jwtDoc.RootElement.GetProperty("tenantId").GetString())
+            let userId = Guid.Parse(jwtDoc.RootElement.GetProperty("userId").GetString())
+
+            use seedConn = dataSource.OpenConnection()
+            let accountId = Guid.NewGuid()
+            let txnA = Guid.NewGuid()
+            let txnB = Guid.NewGuid()
+            seedAccount seedConn tenantId userId accountId "Checking" "USD"
+            seedTransaction seedConn tenantId accountId txnA -10000L DateTimeOffset.UtcNow "manual" "cleared" DateTimeOffset.UtcNow
+            seedTransaction seedConn tenantId accountId txnB -20000L DateTimeOffset.UtcNow "manual" "cleared" DateTimeOffset.UtcNow
+
+            // Upload the SAME file to two different transactions → shared storage_ref.
+            let sharedFileBytes = Encoding.UTF8.GetBytes("shared receipt content")
+
+            let uploadCtxA = createHttpContextWithAuth factory token
+            setMultipartBody uploadCtxA sharedFileBytes "receipt.pdf" "application/pdf" "receipt"
+            do! AttachmentEndpoints.createTransactionAttachmentHandler txnA uploadCtxA
+            test <@ uploadCtxA.Response.StatusCode = 201 @>
+            let docA = readResponseJson uploadCtxA
+            let attachA = Guid.Parse(docA.RootElement.GetProperty("id").GetString())
+
+            let uploadCtxB = createHttpContextWithAuth factory token
+            setMultipartBody uploadCtxB sharedFileBytes "receipt.pdf" "application/pdf" "receipt"
+            do! AttachmentEndpoints.createTransactionAttachmentHandler txnB uploadCtxB
+            test <@ uploadCtxB.Response.StatusCode = 201 @>
+            let docB = readResponseJson uploadCtxB
+            let attachB = Guid.Parse(docB.RootElement.GetProperty("id").GetString())
+
+            // They must reference the same storage_ref (content addressing).
+            test <@ docA.RootElement.GetProperty("storageRef").GetString() = docB.RootElement.GetProperty("storageRef").GetString() @>
+
+            // Delete attachment A.
+            let delCtx = createHttpContextWithAuth factory token
+            do! AttachmentEndpoints.deleteAttachmentHandler attachA delCtx
+            test <@ delCtx.Response.StatusCode = 204 @>
+
+            // Attachment B must still be downloadable (shared file survived).
+            let getCtx = createHttpContextWithAuth factory token
+            do! AttachmentEndpoints.getAttachmentHandler attachB getCtx
+            test <@ getCtx.Response.StatusCode = 200 @>
+            let responseBytes = (getCtx.Response.Body :?> MemoryStream).ToArray()
+            test <@ Encoding.UTF8.GetString(responseBytes) = "shared receipt content" @>
+        }

test/BitThicket.Steward.Api.Test/AttachmentRepositoryTests.fs

@@ -152,3 +152,45 @@ type AttachmentRepositoryTests() =
             let! fetchedOpt = repoB.GetAsync(attachmentId)
             test <@ fetchedOpt.IsNone @>
         }
+
+    [<Fact>]
+    member _.``CountByStorageRefAsync reflects only current tenant``() =
+        task {
+            if not (canConnect ()) then return () else
+
+            let cs = connectionString ()
+            runMigrations cs
+            use dataSource = NpgsqlDataSource.Create(cs)
+            let factory = DbConnectionFactory(dataSource) :> IDbConnectionFactory
+            let tenantId = Guid.NewGuid()
+            let userId = Guid.NewGuid()
+            let accountId = Guid.NewGuid()
+            let txnId = Guid.NewGuid()
+            let attachA = Guid.NewGuid()
+            let attachB = Guid.NewGuid()
+            let sharedRef = "shared-ref-abc123"
+
+            use seedConn = dataSource.OpenConnection()
+            seedAccount seedConn tenantId userId accountId "Checking" "USD"
+            seedTransaction seedConn tenantId accountId txnId -10000L DateTimeOffset.UtcNow "manual" "cleared" DateTimeOffset.UtcNow
+            seedAttachment seedConn tenantId txnId attachA sharedRef
+            seedAttachment seedConn tenantId txnId attachB sharedRef
+
+            let accessor =
+                { new ITenantContextAccessor with
+                    member _.Context = Some { TenantId = tenantId; UserId = userId } }
+            let repo = AttachmentRepository.create factory accessor
+
+            let! count = repo.CountByStorageRefAsync(sharedRef)
+            test <@ count = 2 @>
+
+            // Delete one ref.
+            do! repo.DeleteAsync(attachA)
+            let! countAfter = repo.CountByStorageRefAsync(sharedRef)
+            test <@ countAfter = 1 @>
+
+            // Delete the remaining ref.
+            do! repo.DeleteAsync(attachB)
+            let! countFinal = repo.CountByStorageRefAsync(sharedRef)
+            test <@ countFinal = 0 @>
+        }

test/BitThicket.Steward.Api.Test/ReconciliationEndpointsTests.fs

@@ -210,7 +210,7 @@ type ReconciliationEndpointsTests() =
             seedAccount seedConn tenantId userId accountId
 
             let txn1 = Guid.NewGuid()
-            seedTransaction seedConn tenantId userId accountId txn1 100L (DateTimeOffset(2026, 4, 10, 0, 0, 0, TimeSpan.Zero)) "cleared"
+            seedTransaction seedConn tenantId accountId txn1 100L (DateTimeOffset(2026, 4, 10, 0, 0, 0, TimeSpan.Zero)) "cleared"
 
             let createCtx = createHttpContextWithAuth factory token
             setJsonBody createCtx $"{{\"accountId\":\"{accountId}\",\"statementDate\":\"2026-04-15\",\"statementBalanceMinor\":100,\"currency\":\"USD\"}}"