diff --git a/backend/__tests__/service/pgMessages.test.js b/backend/__tests__/service/pgMessages.test.js index 1f7a6d8b8..00228d5c8 100644 --- a/backend/__tests__/service/pgMessages.test.js +++ b/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'); diff --git a/backend/__tests__/unit/controllers/pgMessageController.test.js b/backend/__tests__/unit/controllers/pgMessageController.test.js index d83301536..2a5d9c112 100644 --- a/backend/__tests__/unit/controllers/pgMessageController.test.js +++ b/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' }]); + }); }); diff --git a/backend/__tests__/unit/models/PgPod.test.js b/backend/__tests__/unit/models/PgPod.test.js index dc2d4d1c1..7daa38e29 100644 --- a/backend/__tests__/unit/models/PgPod.test.js +++ b/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'); - }); }); }); diff --git a/backend/__tests__/unit/models/PgPodModel.extra.test.js b/backend/__tests__/unit/models/PgPodModel.extra.test.js index 3972be209..c52a71aad 100644 --- a/backend/__tests__/unit/models/PgPodModel.extra.test.js +++ b/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); - }); }); diff --git a/backend/__tests__/unit/models/PgPodModel.more.test.js b/backend/__tests__/unit/models/PgPodModel.more.test.js index c6277a1ef..7abcc273b 100644 --- a/backend/__tests__/unit/models/PgPodModel.more.test.js +++ b/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/); + }); }); diff --git a/backend/__tests__/unit/scripts/cleanupGhostPodMembers.test.js b/backend/__tests__/unit/scripts/cleanupGhostPodMembers.test.js new file mode 100644 index 000000000..5eb6b2442 --- /dev/null +++ b/backend/__tests__/unit/scripts/cleanupGhostPodMembers.test.js @@ -0,0 +1,193 @@ +// The cleanup script writes to production PG, so its arms are about what it +// refrains from doing: the class it refuses to delete, the run it refuses to +// perform, and the reconciliation it reports. Every arm sets its own fixtures +// through `fixtures()` — the pool mock dispatches on SQL rather than on call +// order, so inserting a query into the script cannot silently re-point an arm. +jest.mock('../../../config/db-pg', () => ({ pool: { query: jest.fn() } })); +jest.mock('../../../models/Pod', () => ({ findById: jest.fn() })); + +const { pool } = require('../../../config/db-pg'); +const MongoPod = require('../../../models/Pod'); +const { cleanupGhostPodMembers, exitCodeFor, main } = require('../../../scripts/cleanup-ghost-pod-members'); + +const ROW_SQL = /SELECT pod_id, user_id FROM pod_members/; +const COUNT_SQL = /count\(\*\)/; +const DELETE_SQL = /^DELETE FROM pod_members/; + +/** + * rows: `{ pod_id, user_id }` rows the mirror holds. + * docs: podId → the Mongo document `findById` resolves to (`null` = no such + * pod, `undefined` = treat as not found). + * errors: podId → the error `findById` throws. + * remaining: the row count the post-sweep `count(*)` reports. + */ +const fixtures = ({ rows, docs = {}, errors = {}, remaining = 0 } = {}) => { + const deleted = []; + pool.query.mockImplementation(async (sql, params) => { + if (ROW_SQL.test(sql)) return { rows }; + if (COUNT_SQL.test(sql)) return { rows: [{ n: remaining }] }; + if (DELETE_SQL.test(sql)) { + deleted.push(`${params[0]}:${params[1]}`); + return { rows: [] }; + } + throw new Error(`unexpected SQL: ${sql}`); + }); + MongoPod.findById.mockImplementation((podId) => { + if (errors[podId]) throw errors[podId]; + return { + select: () => ({ lean: async () => (docs[podId] === undefined ? null : docs[podId]) }), + }; + }); + return { deleted }; +}; + +const row = (podId, userId) => ({ pod_id: podId, user_id: userId }); +const member = (...ids) => ({ members: ids }); +const castError = () => { + const err = new Error('Cast to ObjectId failed for value "legacy-1"'); + err.name = 'CastError'; + return err; +}; + +describe('cleanup-ghost-pod-members', () => { + afterEach(() => { + jest.clearAllMocks(); + // Keep the status from leaking between arms. Measured, not assumed: jest + // assigns its own exit status at teardown, so a leaked 3 does NOT make a + // green run fail — but an arm reading the status must not be able to see a + // previous arm's value. + delete process.exitCode; + }); + + it('a dry run classifies every row, names one example per class, and deletes nothing', async () => { + fixtures({ + rows: [row('podKeep', 'userKeep'), row('podGhost', 'userGone'), row('podOrphan', 'userO')], + docs: { podKeep: member('userKeep'), podGhost: member('someoneElse'), podOrphan: null }, + }); + + const r = await cleanupGhostPodMembers({ dryRun: true }); + + expect(r).toEqual(expect.objectContaining({ + examined: 3, legitimate: 1, ghost: 1, orphan: 1, deleted: 0, refused: false, + })); + expect(r.examples.legitimate).toEqual({ podId: 'podKeep', userId: 'userKeep' }); + expect(r.examples.ghost).toEqual({ podId: 'podGhost', userId: 'userGone' }); + expect(r.examples.orphan).toEqual({ podId: 'podOrphan', userId: 'userO' }); + expect(pool.query.mock.calls.some(([sql]) => DELETE_SQL.test(sql))).toBe(false); + }); + + it('deletes the ghost and the orphan and never the listed member', async () => { + const { deleted } = fixtures({ + rows: [row('podKeep', 'userKeep'), row('podGhost', 'userGone'), row('podOrphan', 'userO')], + docs: { podKeep: member('userKeep'), podGhost: member('someoneElse'), podOrphan: null }, + remaining: 1, + }); + + const r = await cleanupGhostPodMembers({ dryRun: false }); + + expect(deleted.sort()).toEqual(['podGhost:userGone', 'podOrphan:userO']); + expect(deleted).not.toContain('podKeep:userKeep'); + expect(r.deleted).toBe(2); + expect(r.legitimate).toBe(1); + }); + + it('refuses the whole run when a pod cannot be read, and deletes nothing even where it could', async () => { + const { deleted } = fixtures({ + rows: [row('podUnreadable', 'userU'), row('podGhost', 'userGone')], + docs: { podGhost: member('someoneElse') }, + errors: { podUnreadable: new Error('connection terminated unexpectedly') }, + }); + + const r = await cleanupGhostPodMembers({ dryRun: false }); + + expect(r.refused).toBe(true); + expect(r.unreadable).toBe(1); + expect(r.unreadablePodIds).toEqual(['podUnreadable']); + // The classifiable ghost is not swept either: a partial delete over a store + // we could not fully observe is not a result an operator can check. + expect(deleted).toEqual([]); + expect(r.deleted).toBe(0); + }); + + it('a malformed pod id is an orphan, not a read failure', async () => { + const { deleted } = fixtures({ + rows: [row('legacy-1', 'userL')], + errors: { 'legacy-1': castError() }, + remaining: 0, + }); + + const r = await cleanupGhostPodMembers({ dryRun: false }); + + expect(r).toEqual(expect.objectContaining({ orphan: 1, unreadable: 0, refused: false, deleted: 1 })); + expect(deleted).toEqual(['legacy-1:userL']); + }); + + it('reconciles the two numbers against the observed post-state, and reports when they disagree', async () => { + const rows = [row('podGhost', 'userGone')]; + const docs = { podGhost: member('someoneElse') }; + + fixtures({ rows, docs, remaining: 0 }); + const reconciled = await cleanupGhostPodMembers({ dryRun: false }); + expect(reconciled).toEqual(expect.objectContaining({ examined: 1, deleted: 1, remaining: 0, reconciled: true })); + + // The same report, but the store did not lose the row: 1 - 1 !== 1. + fixtures({ rows, docs, remaining: 1 }); + const diverged = await cleanupGhostPodMembers({ dryRun: false }); + expect(diverged).toEqual(expect.objectContaining({ examined: 1, deleted: 1, remaining: 1, reconciled: false })); + }); + + it('is idempotent: the second run finds nothing to delete', async () => { + fixtures({ rows: [row('podKeep', 'userKeep')], docs: { podKeep: member('userKeep') }, remaining: 1 }); + + const r = await cleanupGhostPodMembers({ dryRun: false }); + + expect(r).toEqual(expect.objectContaining({ examined: 1, toDelete: [], deleted: 0, reconciled: true })); + }); + + // ── TASK-167 gate: what the PROCESS reports, not just what the report says ─ + // A scripted caller reads the exit status; nobody greps the log line. The + // suite above proves the report carries `reconciled: false`; these prove the + // process does not call that a success. + describe('exit status', () => { + const mongoose = require('mongoose'); + const APPLY = ['node', 'cleanup-ghost-pod-members.js', '--apply']; + + beforeEach(() => jest.spyOn(mongoose, 'connect').mockResolvedValue(mongoose)); + + it('a diverged reconciliation exits non-zero: the numbers disagree and rows are gone', async () => { + fixtures({ rows: [row('podGhost', 'userGone')], docs: { podGhost: member('someoneElse') }, remaining: 1 }); + + await main(APPLY); + + expect(process.exitCode).toBe(3); + }); + + it('a clean run exits 0, so 3 is not a blanket non-zero (control)', async () => { + fixtures({ rows: [row('podGhost', 'userGone')], docs: { podGhost: member('someoneElse') }, remaining: 0 }); + + await main(APPLY); + + expect(process.exitCode).toBe(0); + }); + + it('a refused run exits 2, distinct from the divergence code', async () => { + fixtures({ + rows: [row('podUnreadable', 'userU')], + docs: {}, + errors: { podUnreadable: new Error('connection terminated unexpectedly') }, + }); + + await main(APPLY); + + expect(process.exitCode).toBe(2); + }); + + it('maps the three outcomes, and a refusal wins if both are somehow true', () => { + expect(exitCodeFor({ refused: false, reconciled: true })).toBe(0); + expect(exitCodeFor({ refused: false, reconciled: null })).toBe(0); // dry run + expect(exitCodeFor({ refused: true, reconciled: null })).toBe(2); + expect(exitCodeFor({ refused: false, reconciled: false })).toBe(3); + expect(exitCodeFor({ refused: true, reconciled: false })).toBe(2); + }); + }); +}); diff --git a/backend/models/pg/Pod.ts b/backend/models/pg/Pod.ts index b2e9683f6..1ea5fc79b 100644 --- a/backend/models/pg/Pod.ts +++ b/backend/models/pg/Pod.ts @@ -122,19 +122,14 @@ class Pod { return result.rows[0]; } - static async isMember(podId: string, userId: string): Promise { - console.log('Checking membership with params:', { podId, userId, podIdType: typeof podId }); - const query = `SELECT * FROM pod_members WHERE pod_id = $1 AND user_id = $2`; - try { - const result = await (pool as PgPool).query(query, [podId, userId]); - return result.rows.length > 0; - } catch (error) { - const e = error as { message?: string }; - console.error('SQL Error in Pod.isMember:', e.message); - console.error('Query parameters:', { podId, userId }); - throw error; - } - } + // `isMember` used to live here — a read of the `pod_members` MIRROR. It is + // deleted rather than kept and relabelled because a method with that name on + // this model is exactly what the next author reaches for when they want a + // membership answer, and the answer it gives authorises nothing: since + // TASK-162 the decision is Mongo (`utils/isPodMember` read through the + // controller), and #1942 renamed its last caller. If a reader of the mirror is + // ever genuinely needed, name it for what it is (`readMirrorRow`) and say at + // the call site that it decides nothing. } export default Pod; diff --git a/backend/scripts/cleanup-ghost-pod-members.ts b/backend/scripts/cleanup-ghost-pod-members.ts new file mode 100644 index 000000000..588c127d1 --- /dev/null +++ b/backend/scripts/cleanup-ghost-pod-members.ts @@ -0,0 +1,262 @@ +#!/usr/bin/env node +/* + * Clean the two dead classes out of the PG `pod_members` mirror. + * + * Since TASK-162 the authorisation decision is Mongo (`utils/isPodMember` read + * through the controller), and PG `pod_members` is a cache the PG listing + * surfaces still read. Rows that no longer correspond to anything are inert — + * they decide nothing — but they mislead anyone reading the mirror, so the + * hygiene pass removes exactly two classes and names what it leaves: + * + * legitimate : the pod exists in Mongo AND lists the user → keep + * ghost : the pod exists in Mongo, the user is not listed → delete + * orphan : Mongo has no pod with that id → delete + * + * Three properties this script is built to have, because it writes to + * production data: + * + * 1. Two numbers, not one. It prints rows examined and rows deleted, and it + * re-reads the row count afterwards: `before - after === deleted`. A run + * that matched nothing and a DELETE with a broken WHERE both report a + * cheerful "0 ghosts" otherwise. + * 2. The survivor is the discriminating observation. The dry run names one + * row in each class, so a run can be checked by looking at what is still + * there — a script that deletes every row passes "0 ghosts remain". + * 3. The orphan predicate is separate from the ghost one, and a read failure + * fails closed. A transient Mongo failure must not read as "the pod does + * not exist", which would turn the orphan sweep into the whole table. + * A malformed pod id cannot name a Mongo pod at all, so that one case is + * classified as an orphan rather than as a failure to read. + * + * Idempotent: a second run finds nothing to delete. Dry run by default; pass + * `--apply` to write. Run it on the operator's word, never inline in a deploy. + * + * Usage: + * ts-node backend/scripts/cleanup-ghost-pod-members.ts # report + * ts-node backend/scripts/cleanup-ghost-pod-members.ts --apply # delete + */ + +import mongoose from 'mongoose'; +import MongoPod from '../models/Pod'; +// eslint-disable-next-line @typescript-eslint/no-require-imports, global-require +const { isListedPodMember } = require('../utils/isPodMember'); + +interface PgPool { + query: (sql: string, params?: unknown[]) => Promise<{ rows: Record[] }>; +} + +export interface MemberRow { + podId: string; + userId: string; +} + +export interface CleanupReport { + dryRun: boolean; + /** Rows read from `pod_members` before anything was written. */ + examined: number; + legitimate: number; + ghost: number; + orphan: number; + /** Rows whose pod could not be read for a reason that is not "not found". */ + unreadable: number; + unreadablePodIds: string[]; + toDelete: MemberRow[]; + deleted: number; + /** Row count after the sweep; `null` in a dry run. */ + remaining: number | null; + /** `examined - remaining === deleted`. `null` in a dry run. */ + reconciled: boolean | null; + /** True when the run refused to write because something could not be read. */ + refused: boolean; + examples: { legitimate: MemberRow | null; ghost: MemberRow | null; orphan: MemberRow | null }; +} + +const loadPool = (): PgPool => { + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { pool } = require('../config/db-pg') as { pool: PgPool }; + return pool; +}; + +const readMemberRows = async (pool: PgPool): Promise => { + const result = await pool.query('SELECT pod_id, user_id FROM pod_members'); + return result.rows.map((r) => ({ + podId: String(r.pod_id), + userId: String(r.user_id), + })); +}; + +const countMemberRows = async (pool: PgPool): Promise => { + const result = await pool.query('SELECT count(*)::int AS n FROM pod_members'); + return Number((result.rows[0] as { n?: number }).n ?? 0); +}; + +const deleteMemberRow = async (pool: PgPool, row: MemberRow): Promise => { + await pool.query('DELETE FROM pod_members WHERE pod_id = $1 AND user_id = $2', [ + row.podId, + row.userId, + ]); +}; + +export async function cleanupGhostPodMembers( + options: { dryRun?: boolean } = {}, +): Promise { + const dryRun = options.dryRun !== false; + const pool = loadPool(); + + const rows = await readMemberRows(pool); + + const report: CleanupReport = { + dryRun, + examined: rows.length, + legitimate: 0, + ghost: 0, + orphan: 0, + unreadable: 0, + unreadablePodIds: [], + toDelete: [], + deleted: 0, + remaining: null, + reconciled: null, + refused: false, + examples: { legitimate: null, ghost: null, orphan: null }, + }; + + const rowsByPod = new Map(); + for (const row of rows) { + const bucket = rowsByPod.get(row.podId); + if (bucket) bucket.push(row); + else rowsByPod.set(row.podId, [row]); + } + + for (const [podId, podRows] of rowsByPod) { + let doc: { members?: unknown[] } | null = null; + try { + doc = (await MongoPod.findById(podId).select('members').lean()) as { members?: unknown[] } | null; + } catch (err) { + // Fail closed. "Cannot read this pod" is not "this pod does not exist": + // the second reading authorises a delete, so a transient failure must + // never be allowed to take on that meaning. + if ((err as { name?: string })?.name === 'CastError') { + // A pod id Mongo cannot even parse cannot name a pod. Same class as + // an id Mongo does not have, and not a failure to read. + report.orphan += podRows.length; + report.toDelete.push(...podRows); + if (!report.examples.orphan) report.examples.orphan = podRows[0]; + continue; + } + report.unreadable += podRows.length; + report.unreadablePodIds.push(podId); + continue; + } + + if (doc === null) { + report.orphan += podRows.length; + report.toDelete.push(...podRows); + if (!report.examples.orphan) report.examples.orphan = podRows[0]; + continue; + } + + for (const row of podRows) { + if (isListedPodMember(doc, row.userId)) { + report.legitimate += 1; + if (!report.examples.legitimate) report.examples.legitimate = row; + } else { + report.ghost += 1; + report.toDelete.push(row); + if (!report.examples.ghost) report.examples.ghost = row; + } + } + } + + if (report.unreadable > 0) { + // Refuse the whole write rather than sweeping the pods that did read: a + // partial delete over a store we could not fully observe is not a result + // the operator can check against a pre-state. + report.refused = true; + return report; + } + + if (dryRun) return report; + + for (const row of report.toDelete) { + await deleteMemberRow(pool, row); + report.deleted += 1; + } + report.remaining = await countMemberRows(pool); + report.reconciled = report.examined - report.remaining === report.deleted; + return report; +} + +const describe = (label: string, row: MemberRow | null): string => ( + row ? `${label}: pod=${row.podId} user=${row.userId}` : `${label}: none in this run` +); + +/** + * The exit status for a finished run. + * + * Two conditions must not read as success to a scripted caller, and they are not + * interchangeable: `refused` (2) means nothing was written, so a re-run is safe; + * a failed reconciliation (3) can only be discovered *after* rows are gone, so it + * means inspect the store before touching it again. Distinct codes so the caller + * can tell those apart without parsing the log. Refusal wins if both are somehow + * true — it is the state in which the sweep did not happen at all. + * + * TASK-167 gate: the report already knew about a divergence; the process did not. + */ +export function exitCodeFor(report: CleanupReport): number { + if (report.refused) return 2; + if (report.reconciled === false) return 3; + return 0; +} + +export async function main(argv: string[] = process.argv): Promise { + const dryRun = !argv.includes('--apply'); + await mongoose.connect(process.env.MONGO_URI ?? ''); + + const r = await cleanupGhostPodMembers({ dryRun }); + + console.log(`[pod-members] ${dryRun ? 'DRY RUN (pass --apply to delete)' : 'APPLIED'}`); + console.log(`[pod-members] examined : ${r.examined}`); + console.log(`[pod-members] legitimate keep : ${r.legitimate}`); + console.log(`[pod-members] ghost delete : ${r.ghost}`); + console.log(`[pod-members] orphan delete : ${r.orphan}`); + const unreadableIds = r.unreadablePodIds.length ? ` (${r.unreadablePodIds.join(', ')})` : ''; + console.log(`[pod-members] unreadable : ${r.unreadable}${unreadableIds}`); + console.log(`[pod-members] deleted : ${r.deleted}`); + console.log(`[pod-members] remaining : ${r.remaining === null ? '(dry run)' : r.remaining}`); + const reconciled = r.reconciled === null + ? '(dry run)' + : `${r.reconciled} (${r.examined} - ${r.remaining} === ${r.deleted})`; + console.log(`[pod-members] reconciles : ${reconciled}`); + console.log(`[pod-members] ${describe('example ghost', r.examples.ghost)}`); + console.log(`[pod-members] ${describe('example orphan', r.examples.orphan)}`); + console.log(`[pod-members] ${describe('example legitimate', r.examples.legitimate)}`); + + if (r.refused) { + console.error( + `[pod-members] REFUSED: ${r.unreadable} row(s) could not be classified because their pod` + + ' could not be read. Nothing was deleted.', + ); + } + if (r.reconciled === false) { + console.error( + `[pod-members] RECONCILIATION FAILED: after deleting ${r.deleted} of ${r.examined}` + + ` examined row(s), the store holds ${r.remaining}. It did not lose exactly what this run` + + ' deleted. Inspect the store before re-running.', + ); + } + // Set once, from the one function that decides status, so a new condition + // cannot be added to the log without being added to the exit code. + process.exitCode = exitCodeFor(r); +} + +if (require.main === module) { + main() + .catch((err) => { + console.error('[pod-members] failed:', err); + process.exitCode = 1; + }) + .finally(() => { + mongoose.connection.close().catch(() => {}); + }); +}