From 0dc77178cfd82a60fd77e39a998c84768d05e0d9 Mon Sep 17 00:00:00 2001 From: zhuangxialie Date: Thu, 24 Sep 2026 09:20:21 +0800 Subject: [PATCH] fix(polls): a poll never reached the phone createPoll commits a kind='poll' row and enqueues the same message.new broadcast every other sender enqueues, and then stops. dispatchMessagePush has four call sites -- the HTTP message route, cmdReply, startPrivateChat and startPulledGroup -- and neither poll producer (POST /api/polls, `cumora poll create`) is among them. It was the last human-visible message kind that never reached a phone. This is an asymmetry, not a missing feature. NotificationToasts skips only `kind === 'system'`, so a poll already toasts on desktop and in the Electron notification window, and Message.tsx renders the poll bubble on both shells -- so a phone-only member could see and vote on a poll nobody ever told them about. polls.ts says what the shadow body is for in its own comment: "notifications, search index, plain-text logs". A body built for a notification that was never sent. This is the gap my own #241 left. Its commit message claimed the set was complete -- "Every other INSERT INTO messages in the server is correct as it stands: membership.ts, calendar.ts and inproc-client.ts write kind='system'" -- and polls.ts writes kind='poll', so it fell outside that check. I looked at it at the time and set it aside as a product question, whether a poll is worth a phone buzz. That was the wrong call: the desktop already answers yes, and the code documents the intent. Recipients need no new filtering: computeMessageRecipients joins `users`, so agents are never notified, and the mute and "currently looking at the app" filters apply unchanged. A test pins the mute case. --- .../__integration__/agent-reply-push.test.ts | 63 +++++++++++++++++++ server/src/polls.ts | 24 +++++++ 2 files changed, 87 insertions(+) diff --git a/server/src/__integration__/agent-reply-push.test.ts b/server/src/__integration__/agent-reply-push.test.ts index 1c1d6a95..2f562e46 100644 --- a/server/src/__integration__/agent-reply-push.test.ts +++ b/server/src/__integration__/agent-reply-push.test.ts @@ -214,3 +214,66 @@ test('[integration] an agent-only conversation still pushes to nobody', async () const withRecipients = sent.filter((s) => s.recipientUserIds.length > 0) assert.deepEqual(withRecipients, [], 'pushed for a conversation with no humans in it') }) + +// ─── a poll is a message too ──────────────────────────────────────────────── +// +// `createPoll` commits a kind='poll' row and enqueues the same message.new +// broadcast every other sender enqueues, and then stopped: it was the one +// human-visible message kind that never reached dispatchMessagePush. +// +// Not a missing feature, an asymmetry. NotificationToasts skips only +// `kind === 'system'`, so a poll already toasts on desktop and in the Electron +// notification window, and Message.tsx renders the poll bubble on both shells — +// so a phone-only member could see and vote on a poll nobody told them about. +// polls.ts says what the shadow body is for in its own comment: "notifications, +// search index, plain-text logs". + +test('[integration] a poll reaches the phone', async () => { + const { companyId, agentId } = await seedCompanyWithAgent() + const humanId = 'u-poll-target' + await seedOfflineHuman(companyId, humanId) + await seedRoom(companyId, 'c-poll', [agentId, humanId]) + + const { createPoll } = await import('../polls.js') + const { messageId } = await createPoll({ + conversationId: 'c-poll', + companyId, + authorId: agentId, + question: 'Ship Friday?', + mode: 'single', + options: ['yes', 'no'], + }) + assert.ok(messageId) + await new Promise((r) => setTimeout(r, 150)) + + assert.equal(sent.length, 1, 'a poll produced no push — the phone is never told') + assert.deepEqual(sent[0].recipientUserIds, [humanId]) + assert.match(sent[0].body, /Ship Friday\?/, 'the notification body does not name the question') +}) + +test('[integration] a poll obeys the same recipient filters', async () => { + // The guard rail: adding a dispatch must not add a delivery path that ignores + // mute or the "currently looking at the app" rule. + const { companyId, agentId } = await seedCompanyWithAgent() + const muted = 'u-poll-muted' + await seedOfflineHuman(companyId, muted) + await seedRoom(companyId, 'c-poll-muted', [agentId, muted]) + await pool.query( + `INSERT INTO conversation_mutes (user_id, conversation_id, muted_until) VALUES ($1, 'c-poll-muted', NULL)`, + [muted], + ) + + const { createPoll } = await import('../polls.js') + await createPoll({ + conversationId: 'c-poll-muted', + companyId, + authorId: agentId, + question: 'Ship Friday?', + mode: 'single', + options: ['yes', 'no'], + }) + await new Promise((r) => setTimeout(r, 150)) + + const withRecipients = sent.filter((s) => s.recipientUserIds.length > 0) + assert.deepEqual(withRecipients, [], 'pushed a poll into a muted conversation') +}) diff --git a/server/src/polls.ts b/server/src/polls.ts index 6a62c749..dc62b29e 100644 --- a/server/src/polls.ts +++ b/server/src/polls.ts @@ -16,6 +16,7 @@ * tally shape stay consistent across actors. */ import { randomUUID } from 'node:crypto' +import { dispatchMessagePush } from './push.js' import { pool } from './db/pool.js' import { CH_MESSAGE_NEW, CH_POLLS, type PollUpdatedEvent } from './redis.js' import { @@ -175,6 +176,29 @@ export async function createPoll(input: CreatePollInput): Promise { client.release() } + // The row is durable now, so the phone can be told. Fire-and-forget for the + // same reason the other four dispatches are: a push must never hold up the + // write. + // + // A poll was the one human-visible message kind that never reached a phone. + // It is not a missing feature, it is an asymmetry: NotificationToasts skips + // only `kind === 'system'`, so a poll already toasts on desktop and in the + // Electron notification window, and Message.tsx renders the poll bubble on + // both shells — so a phone-only member could see and vote on a poll nobody + // ever told them about. The body built above says what it is for in its own + // comment: "notifications, search index, plain-text logs". + // + // Recipients need no new filtering: computeMessageRecipients joins `users`, + // so agents are never notified, and the mute / "currently looking at the app" + // filters apply unchanged. + void dispatchMessagePush({ + conversationId: input.conversationId, + authorId: input.authorId, + messageId, + body, + companyId: input.companyId, + }) + return { messageId, sequence, poll: payload } }