fix(admin): preserve pending self-deletion during hard delete

backend/__tests__/service/accountDeletion.test.js

@@ -63,9 +63,12 @@ describe('Self-serve account deletion', () => {
     recovery.get('/profile', probeLimit, auth, (_req, res) => res.sendStatus(204));
     app.use('/api/auth', recovery);
     app.get('/runtime-probe', probeLimit, agentRuntimeAuth, (_req, res) => res.sendStatus(204));
+    // Reuse one listener across the counted loops and nested requests instead
+    // of allocating a fresh ephemeral port for every supertest request.
+    app = app.listen(0);
   });
   afterEach(async () => { await clearMongoDb(); await clearPgDb(); await mockPg.query('DELETE FROM users'); });
-  afterAll(async () => { await closeMongoDb(); await closePgDb(); });
+  afterAll(async () => { await new Promise((resolve) => app.close(resolve)); await closeMongoDb(); await closePgDb(); });
 
   it('deletes own bytes and legacy uploads, revokes credentials, preserves agents and other users', async () => {
     const deviceToken = 'cm_delete-device';
@@ -443,6 +446,85 @@ describe('Self-serve account deletion', () => {
     expect(await Lock.countDocuments()).toBe(0);
   });
 
+  it('keeps a billing-failed self-deletion recoverable when admin hard delete races its claim', async () => {
+    const admin = await makeUser({ role: 'admin' });
+    const user = await makeUser({ billing: { subscriptionId: 'sub-admin-race', subscriptionStatus: 'active' } });
+    const token = issueSessionToken(user);
+    const file = await addFile(user, 'admin-race-legacy.png', true);
+    const credential = await AgentCredential.create({ tokenHash: 'admin-race-key', kind: 'runtime', ownerUserId: user._id });
+    let adminResult;
+    mockStripe.subscriptions.cancel.mockImplementationOnce(async () => {
+      adminResult = await request(app).delete(`/api/admin/users/${user.id}`)
+        .set('Authorization', `Bearer ${issueSessionToken(admin)}`);
+      throw new Error('Stripe unavailable before receipt');
+    });
+    await deletion(app, token).expect(503);
+    expect({ status: adminResult.status, userRetained: !!(await User.findById(user.id)),
+      files: await File.countDocuments({ uploadedBy: user.id }), receipts: await AccountDeletion.countDocuments() })
+      .toEqual({ status: 409, userRetained: true, files: 1, receipts: 0 });
+    expect(adminResult.body.code).toBe('ACCOUNT_CHANGED');
+    const retained = await User.findById(user.id);
+    expect(retained).not.toBeNull();
+    expect(retained.accountDeletionRequestedAt).toBeUndefined();
+    expect(await AccountDeletion.countDocuments()).toBe(0);
+    expect(await File.findById(file.id)).not.toBeNull();
+    expect((await AgentCredential.findById(credential.id)).status).toBe('active');
+    expect(await require('../../models/AdminUserMutationLock').countDocuments()).toBe(0);
+    await deletion(app, token).expect(204);
+    expect(await User.findById(user.id)).toBeNull();
+    expect(await File.findById(file.id)).toBeNull();
+    expect((await AgentCredential.findById(credential.id)).status).toBe('revoked');
+  });
+
+  it('refuses hard delete when the deletion marker appears after the admin target read', async () => {
+    const admin = await makeUser({ role: 'admin' }); const user = await makeUser();
+    const Integration = require('../../models/Integration');
+    const find = Integration.find.bind(Integration);
+    let nestedStatus; let claimed = false;
+    jest.spyOn(Integration, 'find').mockImplementation((filter) => {
+      const query = find(filter);
+      if (!claimed && String(filter.createdBy) === user.id) {
+        claimed = true;
+        return { select: (fields) => ({ lean: async () => {
+          jest.spyOn(mockPg, 'query').mockRejectedValueOnce(new Error('PG unavailable'));
+          nestedStatus = (await deletion(app, issueSessionToken(user))).status;
+          return query.select(fields).lean();
+        } }) };
+      }
+      return query;
+    });
+    const result = await request(app).delete(`/api/admin/users/${user.id}`)
+      .set('Authorization', `Bearer ${issueSessionToken(admin)}`);
+    expect(nestedStatus).toBe(503);
+    expect(result.status).toBe(409);
+    expect(result.body.code).toBe('ACCOUNT_CHANGED');
+    expect((await User.findById(user.id)).accountDeletionRequestedAt).toBeTruthy();
+    expect(await AccountDeletion.exists({ _id: user.id })).toBeTruthy();
+    expect(await require('../../models/AdminUserMutationLock').countDocuments()).toBe(0);
+  });
+
+  it('refuses a self-deletion claim when admin hard delete wins first', async () => {
+    const admin = await makeUser({ role: 'admin' });
+    const user = await makeUser({ billing: { subscriptionId: 'sub-admin-first', subscriptionStatus: 'active' } });
+    const update = User.updateOne.bind(User);
+    let adminResult; let deleted = false;
+    jest.spyOn(User, 'updateOne').mockImplementation((filter, mutation, ...rest) => {
+      if (!deleted && mutation.$set?.accountDeletionRequestedAt) return (async () => {
+        deleted = true;
+        adminResult = await request(app).delete(`/api/admin/users/${user.id}`)
+          .set('Authorization', `Bearer ${issueSessionToken(admin)}`);
+        return update(filter, mutation, ...rest);
+      })();
+      return update(filter, mutation, ...rest);
+    });
+    mockStripe.subscriptions.cancel.mockClear();
+    await deletion(app, issueSessionToken(user)).expect(409);
+    expect(adminResult.status).toBe(200);
+    expect(await User.findById(user.id)).toBeNull();
+    expect(mockStripe.subscriptions.cancel).not.toHaveBeenCalled();
+    expect(await AccountDeletion.countDocuments()).toBe(0);
+  });
+
   it('refuses an admin role change after deletion has marked the account', async () => {
     const admin = await makeUser({ role: 'admin' });
     const user = await makeUser({ accountDeletionRequestedAt: new Date() });

backend/__tests__/service/adminPauseLeavesHostedRow.test.js

@@ -37,7 +37,7 @@ jest.mock('../../middleware/auth', () => (req, _res, next) => {
 });
 jest.mock('../../middleware/adminAuth', () => (_req, _res, next) => next());
 
-jest.mock('../../models/User', () => ({ findById: jest.fn(), countDocuments: jest.fn() }));
+jest.mock('../../models/User', () => ({ findById: jest.fn(), findOneAndDelete: jest.fn(), countDocuments: jest.fn() }));
 jest.mock('../../services/emailService', () => ({ sendEmail: jest.fn() }));
 
 const Integration = require('../../models/Integration');
@@ -201,7 +201,6 @@ describe('the admin user delete refuses while the user owns a hosted row (wren 7
     _id: new mongoose.Types.ObjectId(),
     isBot: false,
     role: 'user',
-    deleteOne: jest.fn().mockResolvedValue({}),
   });
   const hostedFor = (userId, overrides = {}) => ({
     type: 'hosted-mcp',
@@ -215,6 +214,7 @@ describe('the admin user delete refuses while the user owns a hosted row (wren 7
   it("refuses with a code and each row's id and entry, and does not delete the person", async () => {
     const target = person();
     User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
     const live = await Integration.create(hostedFor(target._id));
     // The case the predicate exists for (vera 75850): the provider step failed,
     // so the row sits at step 2 of the removal — `disconnected`, its material
@@ -239,18 +239,19 @@ describe('the admin user delete refuses while the user owns a hosted row (wren 7
     // removal route takes: an admin who gets only ids has to look each one up.
     expect(res.body.connections.map((row) => row.entryId).sort()).toEqual(['linear', 'notion']);
     // The control: the refusal is what stopped it, not a failed lookup.
-    expect(target.deleteOne).not.toHaveBeenCalled();
+    expect(User.findOneAndDelete).not.toHaveBeenCalled();
     expect(await Integration.countDocuments({ createdBy: target._id })).toBe(2);
   });
 
   it('accepts the delete for a person who owns no hosted row', async () => {
     const target = person();
     User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
 
     const res = await request(app).delete(`/admin/users/${target._id}`);
 
     expect(res.status).toBe(200);
-    expect(target.deleteOne).toHaveBeenCalledTimes(1);
+    expect(User.findOneAndDelete).toHaveBeenCalledTimes(1);
   });
 
   it('refuses for a row that is mid-exchange, which carries no ref yet', async () => {
@@ -265,6 +266,7 @@ describe('the admin user delete refuses while the user owns a hosted row (wren 7
     // it is there, so the connect lands into an ownerless row.
     const target = person();
     User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
     const midExchange = await Integration.create(hostedFor(target._id, {
       status: 'pending',
       config: { entryId: 'linear', intake: 'oauth' },
@@ -275,7 +277,7 @@ describe('the admin user delete refuses while the user owns a hosted row (wren 7
     expect(res.status).toBe(409);
     expect(res.body.code).toBe('hosted_mcp_connection_owned');
     expect(res.body.connections).toEqual([{ id: String(midExchange._id), entryId: 'linear' }]);
-    expect(target.deleteOne).not.toHaveBeenCalled();
+    expect(User.findOneAndDelete).not.toHaveBeenCalled();
   });
 
   it('does not refuse for a row that is not a hosted one', async () => {
@@ -285,6 +287,7 @@ describe('the admin user delete refuses while the user owns a hosted row (wren 7
     // Integration turns this arm red, which is what keeps it honest.
     const target = person();
     User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
     await Integration.create({
       type: 'github-app',
       scope: 'user',
@@ -296,6 +299,6 @@ describe('the admin user delete refuses while the user owns a hosted row (wren 7
     const res = await request(app).delete(`/admin/users/${target._id}`);
 
     expect(res.status).toBe(200);
-    expect(target.deleteOne).toHaveBeenCalledTimes(1);
+    expect(User.findOneAndDelete).toHaveBeenCalledTimes(1);
   });
 });

backend/__tests__/service/adminRoleConcurrency.test.js

@@ -101,7 +101,7 @@ describe('Administrator role concurrency', () => {
     const Lock = require('../../models/AdminUserMutationLock');
     if (kind === 'ban') await User.updateOne({ _id: bob._id }, { $set: { role: 'user' } });
     const write = (kind === 'role' ? jest.spyOn(User, 'findOneAndUpdate')
-      : jest.spyOn(User.prototype, kind === 'delete' ? 'deleteOne' : 'save'))
+      : kind === 'delete' ? jest.spyOn(User, 'findOneAndDelete') : jest.spyOn(User.prototype, 'save'))
       .mockRejectedValueOnce(new Error('write outcome unknown'));
     try {
       const pending = kind === 'delete' ? request(app).delete(`/api/admin/users/${bob._id}`)

backend/__tests__/unit/routes/admin.users.test.js

@@ -16,6 +16,7 @@ jest.mock('../../../models/User', () => ({
   find: jest.fn(),
   findById: jest.fn(),
   findOneAndUpdate: jest.fn(),
+  findOneAndDelete: jest.fn(),
   countDocuments: jest.fn(),
 }));
 
@@ -121,13 +122,15 @@ describe('admin users routes', () => {
       _id: '000000000000000000000002',
       role: 'user',
       isBot: false,
-      deleteOne: jest.fn().mockResolvedValue(true),
     };
     User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
 
     await handler(req, res);
 
-    expect(target.deleteOne).toHaveBeenCalledTimes(1);
+    expect(User.findOneAndDelete).toHaveBeenCalledWith({
+      _id: target._id, accountDeletionRequestedAt: { $exists: false },
+    });
     expect(res.json).toHaveBeenCalledWith({ message: 'User deleted successfully' });
   });
 
@@ -148,6 +151,7 @@ describe('admin users routes', () => {
       save: jest.fn().mockResolvedValue(true),
     };
     User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
     User.findOneAndUpdate.mockResolvedValue({ ...target, role: 'admin' });
 
     await handler(req, res);
@@ -368,6 +372,7 @@ describe('admin users routes', () => {
       const save = jest.fn().mockResolvedValue(undefined);
       const target = { _id: '000000000000000000000002', username: 'mallory', role: 'user', isBot: false, save };
       User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
       const req = { params: { userId: '000000000000000000000002' }, body: { banned: true, reason: 'spam' }, user: { id: 'admin1' } };
       const res = createRes();
 
@@ -387,6 +392,7 @@ describe('admin users routes', () => {
       const save = jest.fn().mockResolvedValue(undefined);
       const target = { _id: '000000000000000000000002', username: 'mallory', role: 'user', isBot: false, banned: true, banReason: 'spam', save };
       User.findById.mockResolvedValue(target);
+    User.findOneAndDelete.mockResolvedValue(target);
       const req = { params: { userId: '000000000000000000000002' }, body: { banned: false }, user: { id: 'admin1' } };
       const res = createRes();
 

backend/routes/admin/users.ts

@@ -386,8 +386,19 @@ router.delete('/:userId', adminWriteLimiter, auth, adminAuth, async (req: any, r
         });
       }
 
+      // Self-deletion claims do not take the admin lock. Coordinate on the
+      // target document itself so a claim appearing after the reads above
+      // cannot lose its recoverable owner during billing preflight or cleanup.
       beginWrite();
-      await target.deleteOne();
+      const deleted = await User.findOneAndDelete({
+        _id: target._id, accountDeletionRequestedAt: { $exists: false },
+      });
+      if (!deleted) {
+        return mutationResponse(409, {
+          error: 'Account changed or self-deletion is pending. Refresh before trying again.',
+          code: 'ACCOUNT_CHANGED',
+        });
+      }
       return mutationResponse(200, { message: 'User deleted successfully' });
     });
     return res.status(result.status).json(result.body);

docs/api/account-deletion.md

@@ -172,9 +172,22 @@ See [the lock runbook](../runbooks/admin-user-mutation-lock.md) for uncertain-wr
 recovery and the mixed-version rollout constraint. Synthetic purge DM cleanup
 remains a separate TASK-005 follow-up.
 
-The pre-existing administrative hard-delete endpoint remains separate from this
-lifecycle. Its `target.deleteOne()` does not condition on the self-deletion marker,
-so the global admin lock alone does not coordinate it with an in-flight
-self-deletion claim. TASK-006 tracks a controlled reproduction and atomic
-coordination/recoverable cleanup; this PR does not fix that accepted nonblocking
-finding. It is distinct from TASK-005's synthetic-purge DM cleanup.
+Administrative hard delete remains a separate lifecycle: it does not become the
+self-serve cleanup flow. TASK-006 makes its final `findOneAndDelete` conditional
+on `accountDeletionRequestedAt` being absent. The admin lock still serializes
+administrator writers, rechecks the actor and counts administrators; the target
+predicate additionally coordinates with self-deletion, which does not take that
+lock. If a claim wins before the hard delete, the admin returns 409 `ACCOUNT_CHANGED`
+and retains the owner through billing preflight or retryable cleanup. This also
+covers a marker written after the admin read its target. A stale marker still
+refuses until the self-deletion recovery path resolves it.
+
+If admin hard delete wins first, the self-deletion claim matches no user and
+returns 409 before Stripe or receipt creation. The pre-existing administrative
+hard-delete behavior still removes only the human row and does not clean uploads
+or credentials; this change does not claim to fix that broader lifecycle. A
+billing-preflight failure after a refused admin delete leaves the human and their
+uploads/credentials intact, releases only its own claim and allows a fresh
+reauthenticated retry. Hosted-connection refusal and unknown-write lock retention
+remain. The real-handler service cases exercise these orders and retry recovery.
+This is distinct from TASK-005's synthetic-purge DM cleanup.