From 30d2f8a21eb0c24bb72b4e9e238d4a98077d9fce Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:02:50 -0700 Subject: [PATCH] fix(chat): a failure after the message insert no longer writes a second copy (TASK-313) PGMessage.create ran the pod touch and the attention bookkeeping inside the same try as the INSERT. Any error there rethrew to the controller, which falls back to Mongo, so a message already committed in Postgres was written again. The touch and the bookkeeping are now best-effort after the insert and logged; a missing reply target still throws before the insert, so the fallback behaviour there is unchanged. The reconciler that moves fallback rows back into Postgres is not in this change. It needs a schema column and a flag on fallback documents, and the reply guard in messageController stays until then. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: swift Agent-Model: claude/claude-haiku-5-5/default --- .../models/PgMessageCreateAfterInsert.test.js | 70 +++++++++++++++++++ backend/models/pg/Message.ts | 50 ++++++++----- 2 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 backend/__tests__/unit/models/PgMessageCreateAfterInsert.test.js diff --git a/backend/__tests__/unit/models/PgMessageCreateAfterInsert.test.js b/backend/__tests__/unit/models/PgMessageCreateAfterInsert.test.js new file mode 100644 index 0000000000..db14cdf991 --- /dev/null +++ b/backend/__tests__/unit/models/PgMessageCreateAfterInsert.test.js @@ -0,0 +1,70 @@ +// TASK-313: once the INSERT has committed, a failure in the pod touch or the +// attention bookkeeping must not reject create(). The controller's Mongo +// fallback would otherwise write a second copy of a message that is already +// in Postgres. +jest.mock('../../../config/db-pg', () => ({ + pool: { + query: jest.fn(), + }, +})); + +const mockRecordMentionedUsers = jest.fn(); +const mockResolveMentionAttentionForReply = jest.fn(); +jest.mock('../../../services/attentionItemService', () => ({ + recordMentionedUsers: (...args) => mockRecordMentionedUsers(...args), + resolveMentionAttentionForReply: (...args) => mockResolveMentionAttentionForReply(...args), + resolve: jest.fn(), + resolveMany: jest.fn(), +})); + +const { pool } = require('../../../config/db-pg'); +const Message = require('../../../models/pg/Message'); + +const insertedRow = { + id: '7', + pod_id: 'p', + user_id: 'u', + content: 'hello', + message_type: 'text', + created_at: new Date('2026-10-23T00:00:00Z'), + thread_root_id: null, + reply_to_message_id: null, +}; + +describe('PG Message create after the insert has committed', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockRecordMentionedUsers.mockResolvedValue(undefined); + mockResolveMentionAttentionForReply.mockResolvedValue(0); + }); + + it('resolves with the inserted row when the pod touch fails', async () => { + pool.query + .mockResolvedValueOnce({ rows: [insertedRow] }) + .mockRejectedValueOnce(new Error('pod update failed')); + + const row = await Message.create('p', 'u', 'hello', 'text', null, null, null); + + expect(row.id).toBe('7'); + expect(mockRecordMentionedUsers).toHaveBeenCalledTimes(1); + }); + + it('resolves with the inserted row when attention bookkeeping fails', async () => { + pool.query + .mockResolvedValueOnce({ rows: [insertedRow] }) + .mockResolvedValueOnce({ rows: [] }); + mockRecordMentionedUsers.mockRejectedValueOnce(new Error('attention write failed')); + + const row = await Message.create('p', 'u', 'hello', 'text', null, null, null); + + expect(row.id).toBe('7'); + }); + + it('still throws before the insert when the reply target is not in the pod', async () => { + pool.query.mockResolvedValueOnce({ rows: [] }); + + await expect(Message.create('p', 'u', 'hello', 'text', '99', null, null)) + .rejects.toThrow(/not in this pod|reply_target_unavailable|ThreadRoot/i); + expect(pool.query).toHaveBeenCalledTimes(1); + }); +}); diff --git a/backend/models/pg/Message.ts b/backend/models/pg/Message.ts index 645e8ed0ff..2c5210250d 100644 --- a/backend/models/pg/Message.ts +++ b/backend/models/pg/Message.ts @@ -176,26 +176,38 @@ class Message { const { ThreadRootError } = require('../../services/threadRootResolver'); throw new ThreadRootError('reply_target_unavailable', 'the message being replied to is not in this pod'); } - await (pool as PgPool).query( - 'UPDATE pods SET updated_at = CURRENT_TIMESTAMP WHERE id = $1', - [podId], - ); const row = result.rows[0] as unknown as MessageRow; - // eslint-disable-next-line global-require - const { - recordMentionedUsers, - resolveMentionAttentionForReply, - } = require('../../services/attentionItemService'); - await recordMentionedUsers({ - ...row, podId, userId, content, threadRootId: resolvedThreadRootId, answeringMessageId, - }); - await resolveMentionAttentionForReply({ - podId, - recipientUserId: userId, - repliedAt: row.created_at, - threadRootId: row.thread_root_id, - replyToMessageId: row.reply_to_message_id, - }); + // The INSERT has committed. Nothing below may reject back to the caller: + // the controller's Mongo fallback would then write a second copy of a + // message that is already in Postgres (TASK-313). Each step is best + // effort and logged, the same way the post-write read is. + try { + await (pool as PgPool).query( + 'UPDATE pods SET updated_at = CURRENT_TIMESTAMP WHERE id = $1', + [podId], + ); + } catch (touchErr) { + console.warn('[Message.create] pod touch failed after insert:', (touchErr as Error).message); + } + try { + // eslint-disable-next-line global-require + const { + recordMentionedUsers, + resolveMentionAttentionForReply, + } = require('../../services/attentionItemService'); + await recordMentionedUsers({ + ...row, podId, userId, content, threadRootId: resolvedThreadRootId, answeringMessageId, + }); + await resolveMentionAttentionForReply({ + podId, + recipientUserId: userId, + repliedAt: row.created_at, + threadRootId: row.thread_root_id, + replyToMessageId: row.reply_to_message_id, + }); + } catch (attentionErr) { + console.warn('[Message.create] attention bookkeeping failed after insert:', (attentionErr as Error).message); + } return row; } catch (error) { const e = error as { message?: string };