diff --git a/backend/__tests__/unit/services/installableEventHandlers.test.js b/backend/__tests__/unit/services/installableEventHandlers.test.js index e6afe3018..2d396057a 100644 --- a/backend/__tests__/unit/services/installableEventHandlers.test.js +++ b/backend/__tests__/unit/services/installableEventHandlers.test.js @@ -13,9 +13,14 @@ const { install, } = require('../../../services/installable/installableInstallationService'); const { + activeHandlersForPod, dispatch, eventHandlers, } = require('../../../services/installable/eventHandlers'); +const { + isGatedPodTarget, + isRoutedPodTarget, +} = require('../../../services/connectorRelayPolicy'); const telegramSend = require('../../../services/telegramService'); const { TELEGRAM_CONNECTOR, SLACK_CONNECTOR } = require('../../../scripts/seed-builtin-connectors'); const { @@ -439,4 +444,73 @@ describe('installable event dispatcher', () => { podMessageId: 'message-fail', })).resolves.toBeUndefined(); }); + // TASK-160 residue (2). The outbound selector is a Mongo `$match` reading + // `config.gates..enabled` plus membership; the bridges' authorisation + // rule is `connectorRelayPolicy.isRoutedPodTarget` reading the same two + // halves. Until now the two were only asserted apart, so a change to either + // could leave the other behind: the selector is what decides whether a + // connector is ever reached, and the predicate is what decides whether it may + // speak. This walks one fixture matrix through BOTH and asserts they agree per + // cell — plus the concrete expectation, so the two drifting together to "no" + // reddens as well. + describe('the outbound selector agrees with connectorRelayPolicy', () => { + // The matrix the row names: member / non-member × gate on / off / absent. + const CELLS = [ + { label: 'member, gate on', member: true, gate: true }, + { label: 'member, gate off', member: true, gate: false }, + { label: 'member, gate absent', member: true, gate: null }, + { label: 'non-member, gate on', member: false, gate: true }, + { label: 'non-member, gate off', member: false, gate: false }, + { label: 'non-member, gate absent', member: false, gate: null }, + ]; + + it.each(CELLS)('$label: the selector and the predicate reach the same verdict', async ({ + member, gate, + }) => { + const podId = freshId(); + const creatorId = freshId(); + const ownerId = freshId(); + // Installed while the owner IS a member, then removed for the non-member + // cells — the same shape the membership arm above uses, and the only order + // that leaves the row itself unchanged between cells. + await createPod(podId, creatorId, [ownerId]); + const installed = await install({ installableId: 'telegram', installedBy: ownerId, podId }); + // `install` seeds the installed pod's gate ON, so the absent cell has to + // unset the key rather than skip the write — an absent gate and a false + // one are different inputs, and only the absent one exercises `$exists`. + if (gate === null) { + await Integration.updateOne( + { _id: installed.integration._id }, + { $unset: { [`config.gates.${podId}`]: '' } }, + ); + } else { + await Integration.updateOne( + { _id: installed.integration._id }, + { $set: { [`config.gates.${podId}.enabled`]: gate } }, + ); + } + if (!member) await Pod.updateOne({ _id: podId }, { $pull: { members: ownerId } }); + + const row = await Integration.findById(installed.integration._id).lean(); + const pod = await Pod.findById(podId).lean(); + // These cells are only meaningful on the user-scoped branch: that is the + // one whose selection reads `config.gates..enabled` at all. + expect(String(row.scope)).toBe('user'); + + const selected = await activeHandlersForPod(podId); + const selectorSays = selected.some( + ({ integration }) => String(integration._id) === String(row._id), + ); + const predicateSays = isRoutedPodTarget({ + integration: row, pod, podId, userId: row.createdBy, + }); + + // Both halves, then their conjunction, then the verdict all three must + // reach — so agreement-by-both-saying-no cannot pass as agreement. + const expected = member && gate === true; + expect(isGatedPodTarget(row, podId)).toBe(gate === true); + expect(selectorSays).toBe(expected); + expect(predicateSays).toBe(expected); + }); + }); }); diff --git a/backend/services/slackBridgeService.ts b/backend/services/slackBridgeService.ts index 7ca6dac48..dbe107b09 100644 --- a/backend/services/slackBridgeService.ts +++ b/backend/services/slackBridgeService.ts @@ -203,10 +203,16 @@ export const relayAgentMessageToSlack = async (opts: { try { const integration = opts.integration ?? await findLiveIntegration(podId); if (!integration) return; + // TASK-160: the "why is this held" label reads the gate through the same + // predicate the relay itself uses, so a change to gate semantics cannot + // leave the reason a user is shown stale. The scope test stays because it + // asks a different question than the gate read does: the label means "this + // person's gate for that pod is off", not "this connector owns that pod", + // which is what the predicate's pod-scoped arm answers. const cardHoldReason = opts.card ? (integration.config?.adminPause ? 'paused' - : integration.scope === 'user' && integration.config?.gates?.[podId]?.enabled !== true + : integration.scope === 'user' && !isGatedPodTarget(integration, podId) ? 'gate_off' : null) : null; diff --git a/backend/services/telegramBridgeService.ts b/backend/services/telegramBridgeService.ts index 7e58f44fc..7c25f9de4 100644 --- a/backend/services/telegramBridgeService.ts +++ b/backend/services/telegramBridgeService.ts @@ -282,10 +282,16 @@ export const relayAgentMessageToTelegram = async (opts: { // authority for this send; the fallback preserves legacy direct rows. const integration = opts.integration ?? await findLiveIntegration(podId); if (!integration) return; + // TASK-160: the "why is this held" label reads the gate through the same + // predicate the relay itself uses, so a change to gate semantics cannot + // leave the reason a user is shown stale. The scope test stays because it + // asks a different question than the gate read does: the label means "this + // person's gate for that pod is off", not "this connector owns that pod", + // which is what the predicate's pod-scoped arm answers. const cardHoldReason = opts.card ? (integration.config?.adminPause ? 'paused' - : integration.scope === 'user' && integration.config?.gates?.[podId]?.enabled !== true + : integration.scope === 'user' && !isGatedPodTarget(integration, podId) ? 'gate_off' : null) : null;