fix(grants): a close names only the grants its own write revoked (TASK-333)

backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js

@@ -89,7 +89,7 @@ test.each(['complete', 'patch', 'github', 'onboarding'])('%s close revokes the b
 });
 
 test('a failed revocation write leaves the close committed and the grant refused by its epoch', async () => {
-  const spy = jest.spyOn(RoomGrant, 'updateMany').mockRejectedValueOnce(new Error('revocation write failed'));
+  const spy = jest.spyOn(RoomGrant, 'findOneAndUpdate').mockRejectedValueOnce(new Error('revocation write failed'));
   expect((await request(app).post(`/tasks/${POD}/TASK-1/complete`).send({})).status).toBe(200);
   expect(spy).toHaveBeenCalledTimes(1);
   spy.mockRestore();

backend/__tests__/unit/services/roomGrantRecord.test.js

@@ -168,3 +168,74 @@ test('a pod outside RECORD_V1_PODS gets no lines, and its grants still revoke',
   expect(mockChains.size).toBe(0);
   expect((await mongoose.model('RoomGrant').findById(grant._id)).revocationReason).toBe('task.done');
 });
+
+// TASK-333: a mint racing a close. Only Date is faked, so both writers can be
+// given one timestamp; the driver's timers and hrtime stay real.
+const DATE_ONLY = {
+  doNotFake: ['hrtime', 'nextTick', 'performance', 'queueMicrotask', 'requestAnimationFrame', 'cancelAnimationFrame',
+    'requestIdleCallback', 'cancelIdleCallback', 'setImmediate', 'clearImmediate', 'setInterval', 'clearInterval',
+    'setTimeout', 'clearTimeout'],
+};
+const RoomGrant = () => mongoose.model('RoomGrant');
+const raceCreate = (between) => {
+  const realCreate = RoomGrant().create.bind(RoomGrant());
+  return jest.spyOn(RoomGrant(), 'create').mockImplementationOnce(async (doc) => {
+    const created = await realCreate(doc);
+    await between(created);
+    return created;
+  });
+};
+
+test('a grant a refused mint revoked in the same millisecond is not named by the close', async () => {
+  let live;
+  jest.useFakeTimers({ ...DATE_ONLY, now: Date.now() });
+  try {
+    live = await service.mintGrant(input());
+    const spy = raceCreate(() => Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $inc: { grantEpoch: 1 } }));
+    await expect(service.mintGrant(input())).rejects.toMatchObject({ code: 'task_closed' });
+    spy.mockRestore();
+    await close();
+  } finally {
+    jest.useRealTimers();
+  }
+  const revoked = (await chainLines()).filter((l) => l.kind === 'grant.revoked');
+  expect(revoked.map((l) => l.data.grantIds)).toEqual([[live.grantId]]);
+});
+
+test('a refused mint the close already revoked keeps the close\'s revocation and stays named', async () => {
+  let closedAt;
+  jest.useFakeTimers({ ...DATE_ONLY, now: Date.now() });
+  try {
+    const spy = raceCreate(async (created) => {
+      await close();
+      closedAt = (await RoomGrant().findById(created._id)).revokedAt;
+      jest.setSystemTime(Date.now() + 1000);
+    });
+    await expect(service.mintGrant(input())).rejects.toMatchObject({ code: 'task_closed' });
+    spy.mockRestore();
+  } finally {
+    jest.useRealTimers();
+  }
+  const [grant] = await RoomGrant().find({}).lean();
+  expect(closedAt).toBeInstanceOf(Date);
+  expect(grant.revokedAt).toEqual(closedAt);
+  expect(grant.revocationReason).toBe('task.done');
+  const revoked = (await chainLines()).filter((l) => l.kind === 'grant.revoked');
+  expect(revoked.map((l) => l.data.grantIds)).toEqual([[grant.grantId]]);
+});
+
+test('a close whose write fails for one grant still names the grants it did revoke', async () => {
+  const grants = [await service.mintGrant(input()), await service.mintGrant(input())];
+  const spy = jest.spyOn(RoomGrant(), 'findOneAndUpdate').mockRejectedValueOnce(new Error('revocation write failed'));
+  const logged = jest.spyOn(console, 'error').mockImplementation(() => {});
+  await close();
+  expect(spy).toHaveBeenCalledTimes(2);
+  spy.mockRestore();
+  expect(logged).toHaveBeenCalledTimes(1);
+  logged.mockRestore();
+  const rows = await RoomGrant().find({ grantId: { $in: grants.map((g) => g.grantId) } }).lean();
+  const revokedIds = rows.filter((row) => row.revokedAt).map((row) => row.grantId);
+  expect(revokedIds).toHaveLength(1);
+  const revoked = (await chainLines()).filter((l) => l.kind === 'grant.revoked');
+  expect(revoked.map((l) => l.data.grantIds)).toEqual([revokedIds]);
+});

backend/services/roomGrantService.ts

@@ -240,8 +240,10 @@ const assertGrantLineageActive = async (
   return lineage;
 };
 
+// Guarded on `revokedAt: null`: a close that already revoked this grant keeps
+// its own timestamp and reason, so the close's record line still names it.
 const markMintedGrantRejected = async (grantId: string, error: RoomGrantError): Promise<never> => {
-  await RoomGrant.updateOne({ grantId }, { $set: { revokedAt: new Date(),
+  await RoomGrant.updateOne({ grantId, revokedAt: null }, { $set: { revokedAt: new Date(),
     ...(error.code === 'task_closed' ? { revocationReason: 'task.done' } : {}),
   } });
   throw error;
@@ -346,31 +348,40 @@ const assertTaskContext = async (grant: IRoomGrant | Record<string, unknown>, op
 
 /**
  * Hook successful close transitions. An old close never revokes new grants issued after reopen.
- * updateMany reports a count, not ids, so the record line reads back the grants
- * carrying this write's own `revokedAt`. A grant an earlier close or another
- * writer revoked keeps its own timestamp and is not named again.
+ * Each grant is revoked by its own guarded write, and the record line names
+ * exactly the grants those writes matched. Reading back by `revokedAt` instead
+ * would also name a grant a refused mint revoked in the same millisecond with
+ * the same reason. A grant an earlier close or another writer revoked keeps its
+ * own timestamp and is not named again.
  */
 export const revokeTaskGrants = async ({ taskId, podId, closedEpoch }: Pick<RoomGrantTaskBinding, 'taskId' | 'podId'>
   & { closedEpoch: number }): Promise<number> => {
   const filter = {
     'binding.taskId': taskId, 'binding.podId': podId,
     taskEpoch: { $lt: closedEpoch }, revokedAt: null,
   };
+  const candidates = await RoomGrant.find(filter).select('grantId').lean();
   const revokedAt = new Date();
-  const result = await RoomGrant.updateMany(filter, { $set: { revokedAt, revocationReason: 'task.done' } });
-  if (result.modifiedCount > 0) {
-    const revoked = await RoomGrant.find({
-      'binding.taskId': taskId, 'binding.podId': podId, revokedAt, revocationReason: 'task.done',
-    }).select('grantId').lean();
-    const grantIds = Array.from(new Set(revoked.map((grant) => String(grant.grantId))));
-    // ADR-033 §4: the close's own task.done line is queued first, by the caller.
-    if (grantIds.length) {
-      emitRecord({ podId, taskId }, `grant.revoked/task.done/${closedEpoch}`, [
-        { kind: 'grant.revoked', actor: BOARD_ACTOR, data: { grantIds, reason: 'task.done' } },
-      ]);
-    }
-  }
-  return result.modifiedCount;
+  // allSettled, not all: a write that committed stays revoked even when a
+  // sibling write fails, and no later close can name it (its filter needs
+  // `revokedAt: null`), so the line is written for every match before the
+  // failure is rethrown to the caller's log.
+  const settled = await Promise.allSettled(candidates.map(({ grantId }) => RoomGrant.findOneAndUpdate(
+    { ...filter, grantId },
+    { $set: { revokedAt, revocationReason: 'task.done' } },
+    { projection: { grantId: 1 }, lean: true },
+  )));
+  const grantIds = Array.from(new Set(settled.flatMap((result) => (
+    result.status === 'fulfilled' && result.value ? [String(result.value.grantId)] : []))));
+  // ADR-033 §4: the close's own task.done line is queued first, by the caller.
+  if (grantIds.length) {
+    emitRecord({ podId, taskId }, `grant.revoked/task.done/${closedEpoch}`, [
+      { kind: 'grant.revoked', actor: BOARD_ACTOR, data: { grantIds, reason: 'task.done' } },
+    ]);
+  }
+  const failure = settled.find((result): result is PromiseRejectedResult => result.status === 'rejected');
+  if (failure) throw failure.reason;
+  return grantIds.length;
 };
 
 /**