From 2e3a85fb1493cd060d9a9c7b73fb4b127197520f Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 6 Sep 2026 05:26:31 -0700 Subject: [PATCH 1/4] fix(attention): audit the replied stamps the broad resolver wrote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before the narrow resolver landed, any post by a recipient in a pod closed every open mention they held there. Rows stamped `resolvedBy: 'replied'` by that path can assert a reply nobody wrote, and they are the population the per-kind inbox counts will be computed over. `auditRepliedMentionAttention` re-reads each stamped row's source with the same evidence `sweepResolvedMentionAttention` uses, and reopens only the rows the source contradicts. Dry run by default, `--apply` to write, and `--resolved-before=` to bound the scan to stamps written before the narrow resolver deployed. Three outcomes, not two. A Mongo-fallback source persists no reply or thread edges, so absence of evidence there is not evidence the recipient stayed silent — reopening those would re-raise attention a user has already dealt with. They are counted as `undecidable` and left alone. Co-Authored-By: Claude Opus 5 Co-authored-by: wisp Agent-Model: claude/claude-haiku-5-5/default --- .../services/attentionItemService.test.js | 73 +++++++++++++++++ .../services/attentionItemService.test.js.rej | 81 +++++++++++++++++++ backend/package.json | 1 + .../scripts/audit-replied-attention-stamps.ts | 56 +++++++++++++ backend/services/attentionItemService.ts | 57 ++++++++++++- 5 files changed, 266 insertions(+), 2 deletions(-) create mode 100644 backend/__tests__/unit/services/attentionItemService.test.js.rej create mode 100644 backend/scripts/audit-replied-attention-stamps.ts diff --git a/backend/__tests__/unit/services/attentionItemService.test.js b/backend/__tests__/unit/services/attentionItemService.test.js index 64758dadd8..3b83bed148 100644 --- a/backend/__tests__/unit/services/attentionItemService.test.js +++ b/backend/__tests__/unit/services/attentionItemService.test.js @@ -693,6 +693,79 @@ describe('attentionItemService', () => { expect(sourcePattern.test('task.1:update-1')).toBe(true); expect(sourcePattern.test('taskx1:update-1')).toBe(false); }); + + it('reopens a replied stamp the source contradicts, and keeps the one it corroborates', async () => { + const rows = [ + { + _id: 'attention-false', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', + source: { type: 'message', id: '42' }, messageId: '42', threadRootId: '40', + sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), + }, + { + _id: 'attention-true', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', + source: { type: 'message', id: '43' }, messageId: '43', threadRootId: '43', + sourceCreatedAt: new Date('2026-09-05T09:30:00.000Z'), + }, + ]; + mockFind.mockReturnValue({ sort: () => ({ lean: async () => rows }) }); + mockPgMessageHasReplyByUserAfter.mockImplementation(async (podId, userId, after, target) => target.messageId === '43'); + + const result = await AttentionItemService.auditRepliedMentionAttention({ apply: true }); + + expect(result).toEqual({ scanned: 2, kept: 1, contradicted: 1, reopened: 1, undecidable: 0, unavailable: 0 }); + expect(mockUpdateOne).toHaveBeenCalledTimes(1); + expect(mockUpdateOne).toHaveBeenCalledWith( + { _id: 'attention-false', kind: 'mention', status: 'resolved', resolvedBy: 'replied' }, + { $set: { status: 'open' }, $unset: { resolvedAt: '', resolvedBy: '' } }, + ); + }); + + it('changes nothing without --apply, and still reports what it would reopen', async () => { + mockFind.mockReturnValue({ sort: () => ({ lean: async () => [{ + _id: 'attention-false', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', + source: { type: 'message', id: '42' }, messageId: '42', + sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), + }] }) }); + mockPgMessageHasReplyByUserAfter.mockResolvedValue(false); + + const result = await AttentionItemService.auditRepliedMentionAttention(); + + expect(result).toMatchObject({ scanned: 1, contradicted: 1, reopened: 0 }); + expect(mockUpdateOne).not.toHaveBeenCalled(); + }); + + it('counts a Mongo-sourced stamp as undecidable rather than reopening it', async () => { + mockFind.mockReturnValue({ sort: () => ({ lean: async () => [{ + _id: 'attention-mongo', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', + source: { type: 'message', id: '507f191e810c19729de860ea' }, + sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), + }] }) }); + + const result = await AttentionItemService.auditRepliedMentionAttention({ apply: true }); + + expect(result).toMatchObject({ scanned: 1, kept: 0, contradicted: 0, reopened: 0, undecidable: 1 }); + expect(mockUpdateOne).not.toHaveBeenCalled(); + expect(mockPgMessageHasReplyByUserAfter).not.toHaveBeenCalled(); + }); + + it('reads only replied stamps, and honours the resolvedBefore bound', async () => { + mockFind.mockReturnValue({ sort: () => ({ lean: async () => [] }) }); + const before = new Date('2026-09-06T00:00:00.000Z'); + + await AttentionItemService.auditRepliedMentionAttention({ resolvedBefore: before }); + + expect(mockFind).toHaveBeenCalledWith({ + kind: 'mention', status: 'resolved', resolvedBy: 'replied', resolvedAt: { $lt: before }, + }); + }); + + it('omits the resolvedAt bound entirely when none is given', async () => { + mockFind.mockReturnValue({ sort: () => ({ lean: async () => [] }) }); + + await AttentionItemService.auditRepliedMentionAttention(); + + expect(mockFind).toHaveBeenCalledWith({ kind: 'mention', status: 'resolved', resolvedBy: 'replied' }); + }); }); // Stickiness Phase 1b: an agent's answer to the diff --git a/backend/__tests__/unit/services/attentionItemService.test.js.rej b/backend/__tests__/unit/services/attentionItemService.test.js.rej new file mode 100644 index 0000000000..a1801d5c86 --- /dev/null +++ b/backend/__tests__/unit/services/attentionItemService.test.js.rej @@ -0,0 +1,81 @@ +diff a/backend/__tests__/unit/services/attentionItemService.test.js b/backend/__tests__/unit/services/attentionItemService.test.js (rejected hunks) +@@ -215,6 +215,79 @@ describe('attentionItemService', () => { + expect(mockMongoMessageExists).not.toHaveBeenCalled(); + }); + ++ it('reopens a replied stamp the source contradicts, and keeps the one it corroborates', async () => { ++ const rows = [ ++ { ++ _id: 'attention-false', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', ++ source: { type: 'message', id: '42' }, messageId: '42', threadRootId: '40', ++ sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), ++ }, ++ { ++ _id: 'attention-true', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', ++ source: { type: 'message', id: '43' }, messageId: '43', threadRootId: '43', ++ sourceCreatedAt: new Date('2026-09-05T09:30:00.000Z'), ++ }, ++ ]; ++ mockFind.mockReturnValue({ sort: () => ({ lean: async () => rows }) }); ++ mockPgMessageHasReplyByUserAfter.mockImplementation(async (podId, userId, after, target) => target.messageId === '43'); ++ ++ const result = await AttentionItemService.auditRepliedMentionAttention({ apply: true }); ++ ++ expect(result).toEqual({ scanned: 2, kept: 1, contradicted: 1, reopened: 1, undecidable: 0, unavailable: 0 }); ++ expect(mockUpdateOne).toHaveBeenCalledTimes(1); ++ expect(mockUpdateOne).toHaveBeenCalledWith( ++ { _id: 'attention-false', kind: 'mention', status: 'resolved', resolvedBy: 'replied' }, ++ { $set: { status: 'open' }, $unset: { resolvedAt: '', resolvedBy: '' } }, ++ ); ++ }); ++ ++ it('changes nothing without --apply, and still reports what it would reopen', async () => { ++ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [{ ++ _id: 'attention-false', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', ++ source: { type: 'message', id: '42' }, messageId: '42', ++ sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), ++ }] }) }); ++ mockPgMessageHasReplyByUserAfter.mockResolvedValue(false); ++ ++ const result = await AttentionItemService.auditRepliedMentionAttention(); ++ ++ expect(result).toMatchObject({ scanned: 1, contradicted: 1, reopened: 0 }); ++ expect(mockUpdateOne).not.toHaveBeenCalled(); ++ }); ++ ++ it('counts a Mongo-sourced stamp as undecidable rather than reopening it', async () => { ++ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [{ ++ _id: 'attention-mongo', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', ++ source: { type: 'message', id: '507f191e810c19729de860ea' }, ++ sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), ++ }] }) }); ++ ++ const result = await AttentionItemService.auditRepliedMentionAttention({ apply: true }); ++ ++ expect(result).toMatchObject({ scanned: 1, kept: 0, contradicted: 0, reopened: 0, undecidable: 1 }); ++ expect(mockUpdateOne).not.toHaveBeenCalled(); ++ expect(mockPgMessageHasReplyByUserAfter).not.toHaveBeenCalled(); ++ }); ++ ++ it('reads only replied stamps, and honours the resolvedBefore bound', async () => { ++ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [] }) }); ++ const before = new Date('2026-09-06T00:00:00.000Z'); ++ ++ await AttentionItemService.auditRepliedMentionAttention({ resolvedBefore: before }); ++ ++ expect(mockFind).toHaveBeenCalledWith({ ++ kind: 'mention', status: 'resolved', resolvedBy: 'replied', resolvedAt: { $lt: before }, ++ }); ++ }); ++ ++ it('omits the resolvedAt bound entirely when none is given', async () => { ++ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [] }) }); ++ ++ await AttentionItemService.auditRepliedMentionAttention(); ++ ++ expect(mockFind).toHaveBeenCalledWith({ kind: 'mention', status: 'resolved', resolvedBy: 'replied' }); ++ }); ++ + it('materializes a blocked board row once for each current human recipient', async () => { + mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); + mockUserFind.mockReturnValue(chain([ diff --git a/backend/package.json b/backend/package.json index 3fe65529af..0cd12861d3 100644 --- a/backend/package.json +++ b/backend/package.json @@ -29,6 +29,7 @@ "migrate:task-priority": "ts-node scripts/migrate-task-priority.ts", "migrate:task-source-ref-identity": "ts-node scripts/migrate-task-source-ref-identity.ts", "sweep:resolved-mention-attention": "ts-node scripts/sweep-resolved-mention-attention.ts", + "audit:replied-attention-stamps": "ts-node scripts/audit-replied-attention-stamps.ts", "sweep:done-task-handoffs": "ts-node scripts/sweep-done-task-handoffs.ts", "sweep:leftover-runtime-tokens": "ts-node scripts/sweep-leftover-runtime-tokens.ts", "sweep:stale-digest": "ts-node scripts/sweep-stale-digest.ts", diff --git a/backend/scripts/audit-replied-attention-stamps.ts b/backend/scripts/audit-replied-attention-stamps.ts new file mode 100644 index 0000000000..41e6486126 --- /dev/null +++ b/backend/scripts/audit-replied-attention-stamps.ts @@ -0,0 +1,56 @@ +/** + * Audit mention AttentionItems stamped `resolvedBy: 'replied'`. The broad + * pre-#1573 resolver closed every open mention a recipient held in a pod as + * soon as they posted anything there, so some of those stamps assert a reply + * nobody wrote. This re-reads each row's source and reopens only the rows the + * source contradicts; rows whose source cannot answer the question are + * counted and left alone. + * + * Dry run by default: + * npm run audit:replied-attention-stamps + * Bound the population to stamps written before the narrow resolver deployed: + * npm run audit:replied-attention-stamps -- --resolved-before=2026-09-06T00:00:00Z + * Apply the source-backed reopens: + * npm run audit:replied-attention-stamps -- --apply + */ +/* eslint-disable no-console */ +const mongoose = require('mongoose'); +const { auditRepliedMentionAttention } = require('../services/attentionItemService'); + +type PgPool = { end: () => Promise }; + +const APPLY = process.argv.includes('--apply'); +const BEFORE_ARG = process.argv.find((arg) => arg.startsWith('--resolved-before=')); + +export const main = async (): Promise => { + if (!process.env.MONGO_URI) throw new Error('MONGO_URI is required'); + let resolvedBefore: Date | undefined; + if (BEFORE_ARG) { + const parsed = new Date(BEFORE_ARG.split('=').slice(1).join('=')); + // An unparsable bound would silently widen the audit to every stamp ever + // written, which is the opposite of what the flag was passed to do. + if (Number.isNaN(parsed.getTime())) throw new Error(`--resolved-before is not a date: ${BEFORE_ARG}`); + resolvedBefore = parsed; + } + // The audit reads legacy PostgreSQL mention sources as well as Mongo ones. + // Close that pool like the sweep does so the one-shot process exits after + // printing its measured result. + // eslint-disable-next-line global-require + const { pool } = require('../config/db-pg') as { pool: PgPool | null }; + await mongoose.connect(process.env.MONGO_URI); + try { + const result = await auditRepliedMentionAttention({ apply: APPLY, resolvedBefore }); + console.log(JSON.stringify({ ...result, apply: APPLY, resolvedBefore: resolvedBefore?.toISOString() || null })); + if (!APPLY) console.log('DRY RUN — no AttentionItems changed. Re-run with --apply after review.'); + } finally { + await mongoose.disconnect(); + if (pool) await pool.end(); + } +}; + +if (require.main === module) { + main().catch((error) => { + console.error('replied-stamp audit failed:', error); + process.exit(1); + }); +} diff --git a/backend/services/attentionItemService.ts b/backend/services/attentionItemService.ts index 6885fca03c..eab0e1caa8 100644 --- a/backend/services/attentionItemService.ts +++ b/backend/services/attentionItemService.ts @@ -401,6 +401,59 @@ export const sweepMissedAgentMentions = async ( return { scanned: rows.length, eligible: ids.length, missed: Number(result.modifiedCount || 0) }; }; +/** + * Audit rows already stamped `resolvedBy: 'replied'`. Before the narrow + * resolver landed, any post by the recipient in the pod closed every open + * mention they held there, so a row can assert a reply that was never + * written. This re-reads each row's source with the same evidence the sweep + * uses and reopens only the ones the source contradicts. + * + * Three outcomes, not two. `undecidable` exists because Mongo-fallback + * sources persist no reply or thread edges: absence of evidence there is not + * evidence the recipient stayed silent, and reopening those would re-raise + * attention a user has already dealt with. They are counted and left alone. + */ +export const auditRepliedMentionAttention = async ( + { apply = false, resolvedBefore }: { apply?: boolean; resolvedBefore?: Date } = {}, +) => { + const before = validDate(resolvedBefore); + const rows = await AttentionItem.find({ + kind: 'mention', + status: 'resolved', + resolvedBy: 'replied', + ...(before ? { resolvedAt: { $lt: before } } : {}), + }).sort({ createdAt: 1 }).lean(); + let kept = 0; + let contradicted = 0; + let reopened = 0; + let undecidable = 0; + let unavailable = 0; + for (const row of rows) { + try { + const sourceId = String(row?.source?.id || ''); + // Only a PostgreSQL source carries the reply and thread edges this + // audit needs; see recipientRepliedAfterMention. + if (!/^\d+$/.test(sourceId)) { undecidable += 1; continue; } + const sourceCreatedAt = await sourceTimeForMention(row); + if (!sourceCreatedAt) { undecidable += 1; continue; } + if (await recipientRepliedAfterMention(row, sourceCreatedAt)) { kept += 1; continue; } + contradicted += 1; + if (!apply) continue; + // Repeat the stamp in the filter so a row an operator acknowledged + // between the read and the write is not reopened underneath them. + const result = await AttentionItem.updateOne( + { _id: row._id, kind: 'mention', status: 'resolved', resolvedBy: 'replied' }, + { $set: { status: 'open' }, $unset: { resolvedAt: '', resolvedBy: '' } }, + ); + reopened += Number(result.modifiedCount || 0); + } catch (error) { + unavailable += 1; + console.warn('[attention] replied-stamp audit skipped an unreadable source:', (error as Error).message); + } + } + return { scanned: rows.length, kept, contradicted, reopened, undecidable, unavailable }; +}; + export const recordApproval = async (approval: any, options: RecordOptions = {}): Promise => { try { const podId = approval?.podId; @@ -962,6 +1015,6 @@ export const hasEverHadAttention = async (recipientUserId: unknown): Promise Date: Sun, 6 Sep 2026 15:30:29 -0700 Subject: [PATCH 2/4] test(attention): pin the audit's --resolved-before bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flag's whole purpose is to NARROW the scan to the cutover window. An unparsable value fell through as `undefined`, the bound was spread away, and the audit scanned every replied stamp ever written — under `--apply`, an unbounded reopen from a typo. The comment named the hazard; nothing tested it, and deleting the throw left the suite green (lily-shen on #1577). Argv parsing moves into an exported `parseArgs` so the throw is reachable without a database. Deleting it now gives 3 red. One test written here was deleted rather than kept: no valid date contains an `=`, so `split('=')[1]` versus `.slice(1).join('=')` changes nothing a test can observe, and the case passed against both implementations. A test that cannot fail is a claim about nothing; the note in its place says so. Co-Authored-By: Claude Opus 5 Co-authored-by: wisp Agent-Model: claude/claude-haiku-5-5/default --- .../audit-replied-attention-stamps.test.js | 36 +++++++++++++++++++ .../scripts/audit-replied-attention-stamps.ts | 28 +++++++++------ 2 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js diff --git a/backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js b/backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js new file mode 100644 index 0000000000..112341516d --- /dev/null +++ b/backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js @@ -0,0 +1,36 @@ +// @ts-nocheck + +// Only the argv parsing is under test here; the audit itself is covered in +// __tests__/unit/services/attentionItemService.test.js. Stub the modules the +// script pulls in at import time so this needs no database. +jest.mock('mongoose', () => ({ connect: jest.fn(), disconnect: jest.fn() })); +jest.mock('../../../services/attentionItemService', () => ({ auditRepliedMentionAttention: jest.fn() })); + +const { parseArgs } = require('../../../scripts/audit-replied-attention-stamps'); + +describe('audit-replied-attention-stamps argv', () => { + it('reads --apply and an ISO bound', () => { + const { apply, resolvedBefore } = parseArgs(['node', 'script', '--apply', '--resolved-before=2026-09-06T12:42:00Z']); + expect(apply).toBe(true); + expect(resolvedBefore.toISOString()).toBe('2026-09-06T12:42:00.000Z'); + }); + + it('defaults to a dry run over every replied stamp when no flags are given', () => { + expect(parseArgs(['node', 'script'])).toEqual({ apply: false }); + }); + + // The whole point of the flag is to NARROW the scan to the cutover window. + // Falling through as `undefined` on a bad value widens it to every replied + // stamp ever written — under --apply, an unbounded reopen from a typo. + it.each(['--resolved-before=garbage', '--resolved-before=', '--resolved-before=2026-13-45'])( + 'throws rather than silently widening the scan: %s', + (flag) => { + expect(() => parseArgs(['node', 'script', flag])).toThrow(/not a date/); + }, + ); + + // A test for `split('=').slice(1).join('=')` was written here and deleted: + // no valid date contains an `=`, so truncating at the first one changes + // nothing a test can observe. It passed against both implementations, which + // makes it a claim about nothing. The join stays as defensive code. +}); diff --git a/backend/scripts/audit-replied-attention-stamps.ts b/backend/scripts/audit-replied-attention-stamps.ts index 41e6486126..6f3e4a1af8 100644 --- a/backend/scripts/audit-replied-attention-stamps.ts +++ b/backend/scripts/audit-replied-attention-stamps.ts @@ -19,19 +19,27 @@ const { auditRepliedMentionAttention } = require('../services/attentionItemServi type PgPool = { end: () => Promise }; -const APPLY = process.argv.includes('--apply'); -const BEFORE_ARG = process.argv.find((arg) => arg.startsWith('--resolved-before=')); +/** + * Read the flags out of argv. Exported so the throw below is testable without + * a database: an unparsable bound must NOT fall through as `undefined`. It + * would widen the audit from the cutover window to every replied stamp ever + * written, and under `--apply` that is an unbounded reopen from a typo. + */ +export const parseArgs = (argv: string[]): { apply: boolean; resolvedBefore?: Date } => { + const apply = argv.includes('--apply'); + const flag = argv.find((arg) => arg.startsWith('--resolved-before=')); + if (!flag) return { apply }; + const raw = flag.split('=').slice(1).join('='); + const parsed = new Date(raw); + if (!raw.trim() || Number.isNaN(parsed.getTime())) { + throw new Error(`--resolved-before is not a date: ${flag}`); + } + return { apply, resolvedBefore: parsed }; +}; export const main = async (): Promise => { if (!process.env.MONGO_URI) throw new Error('MONGO_URI is required'); - let resolvedBefore: Date | undefined; - if (BEFORE_ARG) { - const parsed = new Date(BEFORE_ARG.split('=').slice(1).join('=')); - // An unparsable bound would silently widen the audit to every stamp ever - // written, which is the opposite of what the flag was passed to do. - if (Number.isNaN(parsed.getTime())) throw new Error(`--resolved-before is not a date: ${BEFORE_ARG}`); - resolvedBefore = parsed; - } + const { apply: APPLY, resolvedBefore } = parseArgs(process.argv); // The audit reads legacy PostgreSQL mention sources as well as Mongo ones. // Close that pool like the sweep does so the one-shot process exits after // printing its measured result. From 239082a1571beb47e5b0d7922c1701adf9c5bd1f Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 30 Oct 2026 02:51:43 -0700 Subject: [PATCH 3/4] chore(attention): drop a stray patch reject from the refresh A failed git apply left a .rej file in the test directory. It was never meant to be committed. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: wisp Agent-Model: claude/claude-haiku-5-5/default --- .../services/attentionItemService.test.js.rej | 81 ------------------- 1 file changed, 81 deletions(-) delete mode 100644 backend/__tests__/unit/services/attentionItemService.test.js.rej diff --git a/backend/__tests__/unit/services/attentionItemService.test.js.rej b/backend/__tests__/unit/services/attentionItemService.test.js.rej deleted file mode 100644 index a1801d5c86..0000000000 --- a/backend/__tests__/unit/services/attentionItemService.test.js.rej +++ /dev/null @@ -1,81 +0,0 @@ -diff a/backend/__tests__/unit/services/attentionItemService.test.js b/backend/__tests__/unit/services/attentionItemService.test.js (rejected hunks) -@@ -215,6 +215,79 @@ describe('attentionItemService', () => { - expect(mockMongoMessageExists).not.toHaveBeenCalled(); - }); - -+ it('reopens a replied stamp the source contradicts, and keeps the one it corroborates', async () => { -+ const rows = [ -+ { -+ _id: 'attention-false', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', -+ source: { type: 'message', id: '42' }, messageId: '42', threadRootId: '40', -+ sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), -+ }, -+ { -+ _id: 'attention-true', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', -+ source: { type: 'message', id: '43' }, messageId: '43', threadRootId: '43', -+ sourceCreatedAt: new Date('2026-09-05T09:30:00.000Z'), -+ }, -+ ]; -+ mockFind.mockReturnValue({ sort: () => ({ lean: async () => rows }) }); -+ mockPgMessageHasReplyByUserAfter.mockImplementation(async (podId, userId, after, target) => target.messageId === '43'); -+ -+ const result = await AttentionItemService.auditRepliedMentionAttention({ apply: true }); -+ -+ expect(result).toEqual({ scanned: 2, kept: 1, contradicted: 1, reopened: 1, undecidable: 0, unavailable: 0 }); -+ expect(mockUpdateOne).toHaveBeenCalledTimes(1); -+ expect(mockUpdateOne).toHaveBeenCalledWith( -+ { _id: 'attention-false', kind: 'mention', status: 'resolved', resolvedBy: 'replied' }, -+ { $set: { status: 'open' }, $unset: { resolvedAt: '', resolvedBy: '' } }, -+ ); -+ }); -+ -+ it('changes nothing without --apply, and still reports what it would reopen', async () => { -+ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [{ -+ _id: 'attention-false', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', -+ source: { type: 'message', id: '42' }, messageId: '42', -+ sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), -+ }] }) }); -+ mockPgMessageHasReplyByUserAfter.mockResolvedValue(false); -+ -+ const result = await AttentionItemService.auditRepliedMentionAttention(); -+ -+ expect(result).toMatchObject({ scanned: 1, contradicted: 1, reopened: 0 }); -+ expect(mockUpdateOne).not.toHaveBeenCalled(); -+ }); -+ -+ it('counts a Mongo-sourced stamp as undecidable rather than reopening it', async () => { -+ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [{ -+ _id: 'attention-mongo', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', -+ source: { type: 'message', id: '507f191e810c19729de860ea' }, -+ sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), -+ }] }) }); -+ -+ const result = await AttentionItemService.auditRepliedMentionAttention({ apply: true }); -+ -+ expect(result).toMatchObject({ scanned: 1, kept: 0, contradicted: 0, reopened: 0, undecidable: 1 }); -+ expect(mockUpdateOne).not.toHaveBeenCalled(); -+ expect(mockPgMessageHasReplyByUserAfter).not.toHaveBeenCalled(); -+ }); -+ -+ it('reads only replied stamps, and honours the resolvedBefore bound', async () => { -+ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [] }) }); -+ const before = new Date('2026-09-06T00:00:00.000Z'); -+ -+ await AttentionItemService.auditRepliedMentionAttention({ resolvedBefore: before }); -+ -+ expect(mockFind).toHaveBeenCalledWith({ -+ kind: 'mention', status: 'resolved', resolvedBy: 'replied', resolvedAt: { $lt: before }, -+ }); -+ }); -+ -+ it('omits the resolvedAt bound entirely when none is given', async () => { -+ mockFind.mockReturnValue({ sort: () => ({ lean: async () => [] }) }); -+ -+ await AttentionItemService.auditRepliedMentionAttention(); -+ -+ expect(mockFind).toHaveBeenCalledWith({ kind: 'mention', status: 'resolved', resolvedBy: 'replied' }); -+ }); -+ - it('materializes a blocked board row once for each current human recipient', async () => { - mockPodFindById.mockReturnValue(chain({ _id: 'pod-1', name: 'Ship room', createdBy: 'owner', members: [{ userId: 'sam' }] })); - mockUserFind.mockReturnValue(chain([ From 2c227112ff7118487700032042b8edc8e1a10ee6 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 30 Oct 2026 03:23:37 -0700 Subject: [PATCH 4/4] fix(attention): fail closed on the audit's argv, and test a source-less row parseArgs read --resolved-before only in its = form, so the space form and a bound swallowed by --apply parsed as an unbounded scan under --apply. It now reads both forms, throws on a missing or unparsable bound, and throws on any unknown flag. The source-less case is now a test: a stamp with no source id is undecidable and never reopened. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: wisp Agent-Model: claude/claude-haiku-5-5/default --- .../audit-replied-attention-stamps.test.js | 25 ++++++++++-- .../services/attentionItemService.test.js | 13 ++++++ .../scripts/audit-replied-attention-stamps.ts | 40 ++++++++++++++----- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js b/backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js index 112341516d..8d186da172 100644 --- a/backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js +++ b/backend/__tests__/unit/scripts/audit-replied-attention-stamps.test.js @@ -29,8 +29,25 @@ describe('audit-replied-attention-stamps argv', () => { }, ); - // A test for `split('=').slice(1).join('=')` was written here and deleted: - // no valid date contains an `=`, so truncating at the first one changes - // nothing a test can observe. It passed against both implementations, which - // makes it a claim about nothing. The join stays as defensive code. + // The space form is the one a shell user types. It used to parse as an + // unbounded run under --apply: the bound was never read, so the scan widened. + it('reads the space form and keeps --apply after it', () => { + const { apply, resolvedBefore } = parseArgs(['node', 'script', '--resolved-before', '2026-09-06', '--apply']); + expect(apply).toBe(true); + expect(resolvedBefore.toISOString()).toBe('2026-09-06T00:00:00.000Z'); + }); + + it.each([ + ['--resolved-before', '--apply'], + ['--resolved-before'], + ])('throws when the bound has no value: %s', (...argv) => { + expect(() => parseArgs(['node', 'script', ...argv])).toThrow(/not a date/); + }); + + it.each([['--aplly'], ['--apply=yes'], ['--resolved_before=2026-09-06']])( + 'throws on an unknown or misspelled flag: %s', + (flag) => { + expect(() => parseArgs(['node', 'script', flag])).toThrow(/Unknown argument/); + }, + ); }); diff --git a/backend/__tests__/unit/services/attentionItemService.test.js b/backend/__tests__/unit/services/attentionItemService.test.js index 3b83bed148..28cd9547bc 100644 --- a/backend/__tests__/unit/services/attentionItemService.test.js +++ b/backend/__tests__/unit/services/attentionItemService.test.js @@ -748,6 +748,19 @@ describe('attentionItemService', () => { expect(mockPgMessageHasReplyByUserAfter).not.toHaveBeenCalled(); }); + it('counts a source-less stamp as undecidable and never reopens it', async () => { + mockFind.mockReturnValue({ sort: () => ({ lean: async () => [{ + _id: 'attention-sourceless', recipientUserId: 'sam', podId: 'pod-1', kind: 'mention', + sourceCreatedAt: new Date('2026-09-05T09:00:00.000Z'), + }] }) }); + + const result = await AttentionItemService.auditRepliedMentionAttention({ apply: true }); + + expect(result).toMatchObject({ scanned: 1, kept: 0, contradicted: 0, reopened: 0, undecidable: 1 }); + expect(mockUpdateOne).not.toHaveBeenCalled(); + expect(mockPgMessageHasReplyByUserAfter).not.toHaveBeenCalled(); + }); + it('reads only replied stamps, and honours the resolvedBefore bound', async () => { mockFind.mockReturnValue({ sort: () => ({ lean: async () => [] }) }); const before = new Date('2026-09-06T00:00:00.000Z'); diff --git a/backend/scripts/audit-replied-attention-stamps.ts b/backend/scripts/audit-replied-attention-stamps.ts index 6f3e4a1af8..4a23d32ef4 100644 --- a/backend/scripts/audit-replied-attention-stamps.ts +++ b/backend/scripts/audit-replied-attention-stamps.ts @@ -20,19 +20,39 @@ const { auditRepliedMentionAttention } = require('../services/attentionItemServi type PgPool = { end: () => Promise }; /** - * Read the flags out of argv. Exported so the throw below is testable without - * a database: an unparsable bound must NOT fall through as `undefined`. It - * would widen the audit from the cutover window to every replied stamp ever - * written, and under `--apply` that is an unbounded reopen from a typo. + * Read the flags out of argv. Exported so the throws below are testable without + * a database. Three things must fail loudly, never fall through: a bound that + * is missing or unparsable (it would widen the audit from the cutover window + * to every replied stamp ever written), a bound whose value is swallowed by a + * neighbouring flag, and an unknown flag such as a misspelled `--apply`. Under + * `--apply` each of those is an unbounded or unintended reopen. */ export const parseArgs = (argv: string[]): { apply: boolean; resolvedBefore?: Date } => { - const apply = argv.includes('--apply'); - const flag = argv.find((arg) => arg.startsWith('--resolved-before=')); - if (!flag) return { apply }; - const raw = flag.split('=').slice(1).join('='); + const args = argv.slice(2); + const BOUND = '--resolved-before'; + let apply = false; + let sawBound = false; + let raw = ''; + for (let i = 0; i < args.length; i += 1) { + const arg = args[i]; + if (arg === '--apply') { + apply = true; + } else if (arg === BOUND || arg.startsWith(`${BOUND}=`)) { + sawBound = true; + if (arg === BOUND) { + raw = args[i + 1] ?? ''; + i += 1; + } else { + raw = arg.slice(BOUND.length + 1); + } + } else { + throw new Error(`Unknown argument: ${arg}`); + } + } + if (!sawBound) return { apply }; const parsed = new Date(raw); - if (!raw.trim() || Number.isNaN(parsed.getTime())) { - throw new Error(`--resolved-before is not a date: ${flag}`); + if (!raw.trim() || raw.startsWith('--') || Number.isNaN(parsed.getTime())) { + throw new Error(`--resolved-before is not a date: ${raw || '(missing)'}`); } return { apply, resolvedBefore: parsed }; };