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([