From 44f3181998bbee686253213afd2e05e33cee4d6c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= <mbombeck@gmail.com>
Date: Tue, 20 Oct 2026 15:36:13 +0200
Subject: [PATCH] fix(sharing): re-read a Guardian's grant under the record
 lock

Renouncing a Guardian grant read the grant before taking the managed
record's lock. When the Guardian's own account deletion held the lock
first and ended the grant, the count under the lock then refused with
"add another Guardian" for a grant that was no longer active. The grant
is now re-read under the lock, so the renunciation answers that it has
already ended (or is gone) instead.

The race test accepts the already-ended answer beside 200 and 404, since
which side wins depends only on timing.
---
 src/lib/sharing/grants.ts                     | 15 ++++++++++-
 .../managed-profile-lifecycle.test.ts         | 26 +++++++++++++++++--
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/lib/sharing/grants.ts b/src/lib/sharing/grants.ts
index 38d9f71e5..183e38a55 100644
--- a/src/lib/sharing/grants.ts
+++ b/src/lib/sharing/grants.ts
@@ -983,7 +983,20 @@ async function endGrant(
     (current.expiresAt === null || current.expiresAt > new Date());
 
   if (!actorOwnsGrant || !isActiveManagedGuardian) return end();
-  return reduceManagedProfileGuardian(db, current.grantorId, end);
+  // `current` was read before the record lock. A concurrent change that holds
+  // the lock first (the Guardian's account deletion ends this grant) commits
+  // in between, and counting under the lock would then refuse with "add
+  // another Guardian" for a grant that is no longer active, instead of saying
+  // it has already ended. So the grant is re-read under the lock; only a grant
+  // still live there is a Guardian whose removal the count has to allow.
+  return withManagedProfileLock(db, current.grantorId, async () => {
+    const live = await db.accountGrant.findFirst({
+      where: { id: grantId, revokedAt: null },
+      select: { id: true },
+    });
+    if (!live) return end();
+    return reduceManagedProfileGuardian(db, current.grantorId, end);
+  });
 }
 
 /**
diff --git a/tests/integration/managed-profile-lifecycle.test.ts b/tests/integration/managed-profile-lifecycle.test.ts
index dbb4bc92e..bd570ee3b 100644
--- a/tests/integration/managed-profile-lifecycle.test.ts
+++ b/tests/integration/managed-profile-lifecycle.test.ts
@@ -771,7 +771,18 @@ describe("managed profile lifecycle (real Postgres)", () => {
       ),
     ]);
 
-    expect([200, 404]).toContain(renounced.status);
+    // Either order is a correct serialization: the renunciation lands first
+    // (200), or the deletion ends the grant first and the renunciation finds
+    // it gone (404) or already ended (409 `sharing.renounce.already_ended`).
+    // Which one wins depends only on timing, and the write ceiling charged
+    // ahead of the renunciation shifts that timing.
+    expect([200, 404, 409]).toContain(renounced.status);
+    if (renounced.status === 409) {
+      const body = (await renounced.clone().json()) as {
+        meta?: { errorCode?: string };
+      };
+      expect(body.meta?.errorCode).toBe("sharing.renounce.already_ended");
+    }
     expect(deleted.status).toBe(200);
     await expect(
       Promise.all([
@@ -818,7 +829,18 @@ describe("managed profile lifecycle (real Postgres)", () => {
       ),
     ]);
 
-    expect([200, 404]).toContain(renounced.status);
+    // Either order is a correct serialization: the renunciation lands first
+    // (200), or the deletion ends the grant first and the renunciation finds
+    // it gone (404) or already ended (409 `sharing.renounce.already_ended`).
+    // Which one wins depends only on timing, and the write ceiling charged
+    // ahead of the renunciation shifts that timing.
+    expect([200, 404, 409]).toContain(renounced.status);
+    if (renounced.status === 409) {
+      const body = (await renounced.clone().json()) as {
+        meta?: { errorCode?: string };
+      };
+      expect(body.meta?.errorCode).toBe("sharing.renounce.already_ended");
+    }
     expect(deleted.status).toBe(200);
     await expect(
       Promise.all([