chore(pg): witness the swallowed mirror write, delete the membership reader, and give the dead rows a script (TASK-167)

backend/__tests__/service/pgMessages.test.js

@@ -18,7 +18,6 @@ jest.mock('../../models/Pod', () => ({ findById: jest.fn() }));
 // Mock PG models
 jest.mock('../../models/pg/Pod', () => ({
   findById: jest.fn(),
-  isMember: jest.fn(),
   addMember: jest.fn(),
 }));
 
@@ -43,8 +42,9 @@ const PGMessage = require('../../models/pg/Message');
 const AgentMentionService = require('../../services/agentMentionService');
 
 // TASK-162: Mongo `members` decides access, so an arm that expects 200 names
-// the caller in the pod Mongo returns. `PGPod.isMember` is still mocked in these
-// arms on purpose — a live PG row that says "member" must not be enough.
+// the caller in the pod Mongo returns. There is no mirror reader left to keep in
+// check — TASK-167 deleted `PGPod.isMember` — so a live PG row cannot reach the
+// decision even in principle.
 const podListing = (...memberIds) => {
   MongoPod.findById.mockReturnValue({
     select: jest.fn().mockReturnValue({
@@ -71,7 +71,6 @@ afterEach(() => {
 describe('PostgreSQL Message Routes', () => {
   it('retrieves messages for a member of the pod, decided by Mongo membership', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
-    PGPod.isMember.mockResolvedValue(true); // a live PG row is not the reason
     podListing('user1');
     PGMessage.findByPodId.mockResolvedValue([{ id: 1, content: 'Hello' }]);
     const token = generateTestToken('user1');
@@ -82,17 +81,15 @@ describe('PostgreSQL Message Routes', () => {
       .expect(200);
 
     expect(MongoPod.findById).toHaveBeenCalledWith('pod1');
-    expect(PGPod.isMember).not.toHaveBeenCalled();
     expect(res.body[0].content).toBe('Hello');
   });
 
-  // TASK-162's witness at the route tier: a live PG row that says "member" for
-  // a caller Mongo no longer lists. `PGPod.isMember` returns true in this arm,
-  // so an arm that only leaves the pod cannot see the defect — the SURVIVOR row
-  // is what discriminates the read-time check from a mirror-on-leave fix.
+  // TASK-162's witness at the route tier: the SURVIVOR. The `pod_members` row is
+  // still present while Mongo membership is gone, so an arm that only leaves the
+  // pod cannot see the defect — the survivor is what discriminates the read-time
+  // check from a mirror-on-leave fix.
   it('refuses a post whose PG pod_members row survived a leave', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
-    PGPod.isMember.mockResolvedValue(true);
     podListing(); // Mongo membership is gone
     const token = generateTestToken('user1');
 
@@ -108,7 +105,6 @@ describe('PostgreSQL Message Routes', () => {
 
   it('returns 401 if user is not a member', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
-    PGPod.isMember.mockResolvedValue(false);
     podListing();
     const token = generateTestToken('user1');
 
@@ -122,7 +118,6 @@ describe('PostgreSQL Message Routes', () => {
 
   it('creates a message successfully', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
-    PGPod.isMember.mockResolvedValue(true);
     podListing('user1');
     PGMessage.create.mockResolvedValue({ id: 1 });
     PGMessage.findById.mockResolvedValue({ id: 1, content: 'Hi there' });
@@ -151,7 +146,6 @@ describe('PostgreSQL Message Routes', () => {
       userId: { _id: 'user1', username: 'sam' },
     };
     PGPod.findById.mockResolvedValue({ id: 'pod1', type: 'chat' });
-    PGPod.isMember.mockResolvedValue(true);
     podListing('user1');
     PGMessage.create.mockResolvedValue({ id: message.id });
     PGMessage.findById.mockResolvedValue(message);
@@ -176,7 +170,6 @@ describe('PostgreSQL Message Routes', () => {
       username: 'sam',
     };
     PGPod.findById.mockResolvedValue({ id: 'pod1', type: 'chat' });
-    PGPod.isMember.mockResolvedValue(true);
     podListing('user1');
     PGMessage.create.mockResolvedValue(persistedMessage);
     PGMessage.findById.mockResolvedValue(null);
@@ -209,7 +202,6 @@ describe('PostgreSQL Message Routes', () => {
       userId: { _id: 'user1', username: 'sam' },
     };
     PGPod.findById.mockResolvedValue({ id: 'pod1', type: 'agent-room' });
-    PGPod.isMember.mockResolvedValue(true);
     podListing('user1');
     PGMessage.create.mockResolvedValue({ id: message.id });
     PGMessage.findById.mockResolvedValue(message);
@@ -229,7 +221,6 @@ describe('PostgreSQL Message Routes', () => {
 
   it('rejects message creation for non-members', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
-    PGPod.isMember.mockResolvedValue(false);
     podListing();
     const token = generateTestToken('user1');
 

backend/__tests__/unit/controllers/pgMessageController.test.js

@@ -92,7 +92,9 @@ describe('pgMessageController', () => {
   // SURVIVOR: the ghost row still present, Mongo membership gone.
   it('refuses a post from a member whose PG row survived their departure', async () => {
     PGPod.findById.mockResolvedValue({ type: 'chat' }); // the ghost row is there
-    PGPod.isMember.mockResolvedValue(true); // and would still say yes
+    // No mirror reader to mock against any more: TASK-167 deleted
+    // `PGPod.isMember`, so the Mongo answer below is the only one this arm can
+    // turn on — which is the property, not a weakening of it.
     mongoPod([]); // Mongo is the truth, and this caller is not in it
     const req = {
       params: { podId: 'p1' },
@@ -106,12 +108,10 @@ describe('pgMessageController', () => {
 
     expect(res.status).toHaveBeenCalledWith(401);
     expect(PGMessage.create).not.toHaveBeenCalled();
-    expect(PGPod.isMember).not.toHaveBeenCalled();
   });
 
   it('refuses a read from the same stale row, so the ghost does not leak history', async () => {
     PGPod.findById.mockResolvedValue({ type: 'chat' });
-    PGPod.isMember.mockResolvedValue(true);
     mongoPod([]);
     const req = {
       params: { podId: 'p1' },
@@ -133,7 +133,6 @@ describe('pgMessageController', () => {
     // other. `createdBy` is not membership: it says who made the pod, not who is
     // in it.
     PGPod.findById.mockResolvedValue({ type: 'chat' });
-    PGPod.isMember.mockResolvedValue(true);
     mongoPod([]);
     MongoPod.findById.mockReturnValue({
       select: jest.fn().mockReturnValue({
@@ -199,4 +198,33 @@ describe('pgMessageController', () => {
     // nothing.
     expect(PGPod.addMember).toHaveBeenCalledWith('p1', 'u1');
   });
+
+  it('admits a listed member even when the mirror write is rejected', async () => {
+    PGPod.findById.mockResolvedValue({ type: 'chat' });
+    mongoPod(['u1']);
+    // The FK on pod_members.pod_id rejects ordinarily when the pod has no PG row
+    // yet, which is a state a legitimate member can be in. Warming the mirror is
+    // a cache write, so its failure must not deny the member — removing the
+    // inner try/catch in isPodMemberInMongo sends this rejection to the outer
+    // catch, which answers 401 to someone Mongo lists as a member.
+    PGPod.addMember.mockRejectedValue(
+      new Error('insert or update on table "pod_members" violates foreign key constraint "pod_members_pod_id_fkey"'),
+    );
+    PGMessage.findByPodId.mockResolvedValue([{ id: 'm1' }]);
+    const req = {
+      params: { podId: 'p1' },
+      query: {},
+      userId: 'u1',
+      user: { id: 'u1' },
+    };
+    const res = jsonRes();
+
+    await controller.getMessages(req, res);
+
+    // The attempt happened, and its failure did not become the answer.
+    expect(PGPod.addMember).toHaveBeenCalledWith('p1', 'u1');
+    expect(res.status).not.toHaveBeenCalledWith(401);
+    expect(res.status).not.toHaveBeenCalledWith(500);
+    expect(res.json).toHaveBeenCalledWith([{ id: 'm1' }]);
+  });
 });

backend/__tests__/unit/models/PgPod.test.js

@@ -23,7 +23,6 @@ jest.mock('../../../models/pg/Pod', () => ({
   delete: jest.fn(),
   addMember: jest.fn(),
   removeMember: jest.fn(),
-  isMember: jest.fn(),
 }));
 
 describe('PostgreSQL Pod Model Tests', () => {
@@ -236,13 +235,5 @@ describe('PostgreSQL Pod Model Tests', () => {
       expect(Pod.removeMember).toHaveBeenCalledWith('pod123', 'user456');
     });
 
-    it('should correctly check if a user is a member of a pod', async () => {
-      Pod.isMember.mockResolvedValue(true);
-
-      const result = await Pod.isMember('pod123', 'user456');
-
-      expect(result).toBe(true);
-      expect(Pod.isMember).toHaveBeenCalledWith('pod123', 'user456');
-    });
   });
 });

backend/__tests__/unit/models/PgPodModel.extra.test.js

@@ -13,11 +13,4 @@ describe('PG Pod model', () => {
     expect(Pod.addMember).toHaveBeenCalledWith('p1', 'u1');
     expect(result.id).toBe('p1');
   });
-
-  it('isMember checks membership', async () => {
-    pool.query.mockResolvedValue({ rows: [{ pod_id: 'p1' }] });
-    const res = await Pod.isMember('p1', 'u1');
-    expect(pool.query).toHaveBeenCalled();
-    expect(res).toBe(true);
-  });
 });

backend/__tests__/unit/models/PgPodModel.more.test.js

@@ -1,3 +1,6 @@
+const fs = require('fs');
+const path = require('path');
+
 jest.mock('../../../config/db-pg', () => ({ pool: { query: jest.fn() } }));
 const { pool } = require('../../../config/db-pg');
 const Pod = require('../../../models/pg/Pod');
@@ -25,11 +28,6 @@ describe('PG Pod model additional tests', () => {
     await expect(Pod.addMember('p1', 'u1')).rejects.toThrow('db');
   });
 
-  it('isMember throws when query fails', async () => {
-    pool.query.mockRejectedValue(new Error('oops'));
-    await expect(Pod.isMember('p1', 'u1')).rejects.toThrow('oops');
-  });
-
   it('update returns updated row', async () => {
     pool.query.mockResolvedValue({ rows: [{ id: 'p1', name: 'n' }] });
     const res = await Pod.update('p1', 'n', 'd');
@@ -73,4 +71,16 @@ describe('PG Pod model additional tests', () => {
     );
     expect(res).toEqual({ id: '1' });
   });
+
+  it('exposes no membership reader: the mirror decides nothing (TASK-167)', () => {
+    // The claim is absence, so the instrument is the source rather than a call:
+    // no execution can show that a method is gone, and an `isMember` on this
+    // model is exactly the shape the next author reaches for when they want a
+    // membership answer. The second assertion is the positive control — without
+    // it, a typo'd pattern matches nothing against a file where the code is
+    // sitting in plain sight.
+    const src = fs.readFileSync(path.join(__dirname, '../../../models/pg/Pod.ts'), 'utf8');
+    expect(src).not.toMatch(/static\s+async\s+isMember\b/);
+    expect(src).toMatch(/static\s+async\s+addMember\b/);
+  });
 });