From 273af399943e485e5e629ac2a066316b528d7255 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Thu, 27 Aug 2026 22:26:38 +0200 Subject: [PATCH] fix(whatsapp): company question falls back to numbered text and ignores archived companies (#1992) * fix(whatsapp): company question survives a Meta-rejected interactive send (#1589) The linked multi-company sender in #1589 never heard back because Meta rejected the reply-button payload synchronously (HTTP 400, #131009 "Duplicate button title"): the sender belongs to two companies with the same name, one of them archived. askCompanyQuestion rolled the question back and returned not_asked, the row stayed parked as staged_awaiting_company, and the channel went silent. - Exclude archived companies wherever the channel resolves memberships (isMember, resolveCompanyTarget, loadCompanyOptions, applyCompanyChoice, the M3 greeting count), same inner-join filter as the middleware. - uniqueTitles: interactive button/row titles are made unique (position suffix) so two live same-named companies, or names that truncate to the same prefix, no longer trip #131009. - Numbered-text fallback: when the interactive send is rejected at send time, ask the same M6 question as plain numbered text; roll back only when that fails too. A typed digit is recorded as via='numbered'. - Drain: when the sender now resolves as 'single', rows parked behind the dead question are re-opened and kicked instead of expiring at Meta. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FkUfWtuFCUkNtRAgMQCse2 * fix(whatsapp): single-company drain also clears the dead company question (#1589) Re-opening the parked rows left the conversation in state awaiting_company with company_options and the company pending_question intact, so the sender stayed behind a zombie question for up to 48h: every typed word became a company_retry re-offering the archived company, 'byt' was swallowed, and finalizeBurst could not ask about the drained receipts until the TTL sweep. - After the drain, when a company question is open in any of its shapes (awaiting_company state, kept company_options, company pending_question), clear it through the guarded updateConversation: state -> idle, options and the company pending_question deleted, other question types untouched. - Tests: the clear in its awaiting_company and post-TTL (idle + options) shapes, and its no-op for a representation question. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FkUfWtuFCUkNtRAgMQCse2 --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- DECISIONS.md | 2 + .../__tests__/company-question.test.ts | 152 ++++++++++++++- .../__tests__/graph-api.test.ts | 65 +++++++ .../__tests__/hardening.test.ts | 8 + .../__tests__/process-inbound.test.ts | 180 ++++++++++++++++++ extensions/general/whatsapp-inbox/index.ts | 6 +- .../whatsapp-inbox/lib/company-question.ts | 59 ++++-- .../general/whatsapp-inbox/lib/graph-api.ts | 40 +++- .../whatsapp-inbox/lib/process-inbound.ts | 78 +++++++- 9 files changed, 566 insertions(+), 24 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index ebb03f22..5bbed844 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1298,6 +1298,8 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-27] New `unlinked_documents` category on the Accounted://attention resource, backed by lib/documents/unlinked-documents.ts. The whole design is the mime ALLOW-LIST, and the naive predicate is a trap: "current version, no journal_entry_id, referenced by none of the eight linking tables" returns 15 806 rows on prod, of which 11 309 are application/json and every single one is named psd2-response__pN.json, the archived PSD2 bank-API responses the integration stores as evidence of each fetch. Those are unlinked BY DESIGN; surfacing them would hand an agent 11 309 items of work it must not action, which is worse than showing nothing. Measured 2026-08-27: application/json was 11 309 of 11 309 psd2, and pdf/png/jpeg/heic were 0 of 4 495, so the split is clean. Chose an allow-list of underlag-shaped mime types over excluding known-bad filenames, so a future machine-payload format (XML, CSV, an audit bundle) stays out by default instead of leaking until someone notices. Real remaining surface: 4 497 documents across 210 companies, median 3 per company, 481 in the preceding week, and NOT agent-specific (2 374 upload_source=api vs 1 623 file_upload from the web UI). Two-pass fetch mirroring fetchPurchasesWithoutUnderlag: indexed column filter, then eight reference lookups that run only when candidates exist, so the common case costs one query. Scan cap is 300 and is set by URL LENGTH, not table size: each candidate id is echoed through eight .in(column, ids) lookups at ~38 bytes per UUID, and a cap in the thousands would exceed the gateway limit, fail the lookups, and the "claims nothing" fallback would turn every candidate into a false positive. A failing lookup is deliberately treated as "claims nothing" (can only ADD a row) rather than dropping the category, so one misbehaving table cannot hide real work. UnlinkedDocument is a type alias not an interface: the resource assigns it into samples: Record[] and an interface has no implicit index signature; vitest does not typecheck so this only fails in npm run build. [2026-08-27] NOT fixed, and recorded so the next person does not act on an inflated number: the agent-facing readers (resources/attention.ts, resources/recent-activity.ts) still test booked-ness with a raw journal_entry_id null check instead of the canonical isTransactionBooked, which misses the bulk-book (transaction_voucher_links) and multi-allocation (invoice_payments / supplier_invoice_payments) cases. Real scale measured on prod 2026-08-27: 4 transactions, in 1 company, out of 567 column-filtered unbooked, all 4 via transaction_voucher_links and 0 via either payments table. Worth fixing as hygiene, but it is a 4-row problem and doing it properly in attention.ts needs the same two-pass treatment plus a decision about count semantics for a tenant with thousands of unbooked rows, so it does not belong bolted onto this change. [2026-08-27] Klarmarkera (markPeriodClosedExternally) gets an undo, reopenExternallyClosedPeriod, allowed only while the closed state still comes from klarmarkera (closed_externally set, no closing entry): that close was a person's control decision without a bokslutsverifikat, so reversing it strands nothing, whereas a closePeriod close keeps its closing entry and stays irreversible here. The reopen clears the lock too, because the reason to reopen is to change the period's contents (Forsslund Systems 2026-08-27: five imported years klarmarkerade, then the prior-year SIE turned out wrong; replace refused the closed year, unlock refused the closed state, no way back). Audit_log row plus period.unlocked event; the MCP staged-op surface (lock/unlock) does not get a reopen op yet, follow-up. +[2026-08-27] WhatsApp company question (#1589): the root cause was Meta rejecting the reply-button payload synchronously with HTTP 400 #131009 "Duplicate button title" because the sender belonged to two same-named companies, one archived, not a client that refuses interactive messages; fix = archived-membership filter on every channel membership lookup (mirrors lib/supabase/middleware.ts), unique interactive titles (position suffix), a synchronous numbered-text fallback under the same M6 template id, and a drain of rows parked behind the now-dead question only when the sender resolves as 'single'. The async delivery-status fallback leg was deliberately not built (every observed failure was a synchronous 400), and the drain is not extended to default/pin resolution (those choices are still changeable, so an open question there is not dead). +[2026-08-27] WhatsApp single-company drain (#1589) also clears the dead company question on the conversation (awaiting_company -> idle, company_options and the company pending_question deleted), guarded via updateConversation and checked against fresh state: with the question left in place every typed word became a company_retry re-offering the archived company, 'byt' was swallowed, and finalizeBurst could not ask about the drained receipts until the 48h TTL. Cleared whenever the sender resolves as 'single' and a company question exists, not only when rows were re-opened (the TTL sweep keeps company_options past the rows); other pending_question types stay untouched. [2026-08-27] WhatsApp unknown-sender quota RPC (check_and_increment_whatsapp_sender_quota) now fails OPEN to the throttled greeting path when the RPC errors or throws (#1599): the greeting throttle (1/h text, 10-min media burst, 3/day, itself fail-closed on read error) and the single-use link-code claim already bound outbound volume, whereas fail-closed silenced the first-touch linking moment on any transient DB hiccup. M2 (bad code) keeps its own small fail-closed throttle in that mode (badCodeThrottled: 1 per 10 min, 3/day per phone hash) since the quota no longer bounds it (review finding on #1991: withholding M2 left a bad code inside the M1 hour completely silent); when that throttle declines, the sender falls through to the throttled M1, which also says how to fetch a fresh code. Over-quota (ok:false) stays silent by design. [2026-08-27] Issue #1947: categorize fails closed (typed 409 TX_CATEGORIZE_JOURNAL_ENTRY_FAILED, nothing written) instead of adding a fourth worklist state for categorised-but-unbooked rows: the verifikat is the booking, and a new state would touch the load-bearing is_business IS NULL predicate, lockPeriod guard and badges. The MCP/bulk door (categorizeMatchedTransaction) was fail-closed only for thrown engine errors, not the engine's null return (closed year or missing period), so the same null guard now refuses there too, before any transactions write, with errorCode PERIOD_LOCKED or NO_OPEN_PERIOD_FOR_DATE via checkPeriodLock. Existing stranded prod rows left for a separate founder-approved repair. [2026-08-27] reverseEntry resets is_business, category and reconciliation_method together with journal_entry_id when it unlinks bank transactions (#1950): the worklist predicate is is_business IS NULL (lib/worklist/types.ts), so clearing only the link hid stornoed rows from Att bokföra and the nav badge while the reverse_warning dialog promised the opposite. The predicate was not switched to journal_entry_id IS NULL because bulk-booked and multi-allocated rows keep it NULL while booked (lib/transactions/is-booked.ts). Fixed in the engine, not per reverse route, so dashboard, v1 and MCP stornos all agree. diff --git a/extensions/general/whatsapp-inbox/__tests__/company-question.test.ts b/extensions/general/whatsapp-inbox/__tests__/company-question.test.ts index 0e1dc20e..b3b447be 100644 --- a/extensions/general/whatsapp-inbox/__tests__/company-question.test.ts +++ b/extensions/general/whatsapp-inbox/__tests__/company-question.test.ts @@ -19,6 +19,7 @@ import { sendReplyButtons, sendList, truncateTitle, + uniqueTitles, } from '@/extensions/general/whatsapp-inbox/lib/graph-api' import { askCompanyQuestion, @@ -93,7 +94,7 @@ describe('askCompanyQuestion', () => { }) it('uses reply buttons for <=3 companies, ids = company ids', async () => { - const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + const { supabase, enqueue, findCalls, calls } = createQueuedMockSupabase() enqueue({ data: memberships(3) }) enqueue({ data: companies(3) }) enqueue({ data: [{ id: 'conv-1' }] }) // guarded transition won @@ -112,6 +113,18 @@ describe('askCompanyQuestion', () => { expect(args.buttons).toHaveLength(3) expect(args.buttons[0]).toEqual({ id: 'company-1', title: 'Bolag A AB' }) expect(args.body).toContain('Vilket företag') + // The interactive send succeeded: no numbered fallback, no duplicate question. + expect(sendTextMock).not.toHaveBeenCalled() + // Archived companies are never offered (#1589). + expect( + calls.some( + (c) => + c.table === 'companies' && + c.method === 'is' && + c.args[0] === 'archived_at' && + c.args[1] === null, + ), + ).toBe(true) // State transition stored the options for digit replies too. const patch = findCalls('whatsapp_conversations', 'update')[0][0] as { state: string @@ -122,6 +135,101 @@ describe('askCompanyQuestion', () => { expect(patch.context.pending_question.type).toBe('company') }) + it('falls back to the numbered text question when the reply-button send is rejected', async () => { + sendButtonsMock.mockResolvedValueOnce({ + ok: false, + wamid: null, + errorDetail: + 'Send failed (HTTP 400): {"error":{"message":"(#131009) Parameter value is not valid","error_data":{"details":"Duplicate button title"}}}', + }) + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: memberships(3) }) + enqueue({ data: companies(3) }) + enqueue({ data: [{ id: 'conv-1' }] }) + + const asked = await askCompanyQuestion(supabase as unknown as SupabaseClient, { + conversation: makeConversation(), + link: makeLink(), + to: '46701234567', + replyBase, + stagedCount: 1, + }) + + expect(asked).toBe('asked') + expect(sendButtonsMock).toHaveBeenCalledTimes(1) + expect(sendTextMock).toHaveBeenCalledTimes(1) + const fallback = sendTextMock.mock.calls[0][1] + expect(fallback.template).toBe(TEMPLATE.m6CompanyQuestion) + expect(fallback.to).toBe('46701234567') + expect(fallback.body).toContain('1. Bolag A AB') + expect(fallback.body).toContain('3. Bolag C AB') + expect(fallback.body).toContain('Svara med en siffra') + // The question stays armed: exactly one conversation write, no rollback. + expect(findCalls('whatsapp_conversations', 'update')).toHaveLength(1) + }) + + it('falls back to the numbered text question when the list send is rejected', async () => { + sendListMock.mockResolvedValueOnce({ ok: false, wamid: null, errorDetail: 'Send failed (HTTP 400)' }) + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: memberships(5) }) + enqueue({ data: companies(5) }) + enqueue({ data: [{ id: 'conv-1' }] }) + + const asked = await askCompanyQuestion(supabase as unknown as SupabaseClient, { + conversation: makeConversation(), + link: makeLink(), + to: '46701234567', + replyBase, + stagedCount: 2, + }) + + expect(asked).toBe('asked') + expect(sendListMock).toHaveBeenCalledTimes(1) + expect(sendTextMock).toHaveBeenCalledTimes(1) + const body = sendTextMock.mock.calls[0][1].body + expect(body).toContain('kvittona') + expect(body).toContain('5. Bolag E AB') + expect(findCalls('whatsapp_conversations', 'update')).toHaveLength(1) + }) + + it('rolls back only when the numbered fallback also fails', async () => { + sendButtonsMock.mockResolvedValueOnce({ ok: false, wamid: null, errorDetail: 'Send failed (HTTP 400)' }) + sendTextMock.mockResolvedValueOnce({ ok: false, wamid: null, errorDetail: 'Send failed (HTTP 500)' }) + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: memberships(3) }) + enqueue({ data: companies(3) }) + enqueue({ data: [{ id: 'conv-1' }] }) // guarded transition won + enqueue({ + data: [ + { + ...(makeConversation() as Record), + state: 'awaiting_company', + context: { company_options: [{ id: 'company-1', name: 'Bolag A AB' }] }, + }, + ], + }) // rollback echo + + const asked = await askCompanyQuestion(supabase as unknown as SupabaseClient, { + conversation: makeConversation(), + link: makeLink(), + to: '46701234567', + replyBase, + stagedCount: 1, + }) + + expect(asked).toBe('not_asked') + expect(sendButtonsMock).toHaveBeenCalledTimes(1) + expect(sendTextMock).toHaveBeenCalledTimes(1) + const updates = findCalls('whatsapp_conversations', 'update').map( + (args) => args[0] as { state?: string; context?: Record }, + ) + expect(updates).toHaveLength(2) + expect(updates[0].state).toBe('awaiting_company') + expect(updates[1].state).toBe('idle') + expect(updates[1].context?.company_options).toBeUndefined() + expect(updates[1].context?.pending_question).toBeUndefined() + }) + it('uses a list message for 4-10 companies', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: memberships(5) }) @@ -299,6 +407,11 @@ describe('applyCompanyChoice', () => { stagedMessageIds: ['stg-1', 'stg-2'], }) + // The membership check is a LIVE-membership check: a tap or digit for a + // company archived since the question was asked is rejected (#1589). + expect(findCalls('company_members', 'select')[0][0]).toContain('companies!inner(archived_at)') + expect(findCalls('company_members', 'is')).toContainEqual(['companies.archived_at', null]) + const patch = findCalls('whatsapp_conversations', 'update')[0][0] as { state: string company_id: string @@ -427,3 +540,40 @@ describe('truncateTitle', () => { expect(cut).not.toMatch(/\s…$/) // no dangling space before the ellipsis }) }) + +describe('uniqueTitles', () => { + it('passes distinct short names through untouched', () => { + expect(uniqueTitles(['Bolag A AB', 'Bolag B AB', 'Bolag C AB'], 20)).toEqual([ + 'Bolag A AB', + 'Bolag B AB', + 'Bolag C AB', + ]) + }) + + it('disambiguates identical names with their 1-based position (Meta #131009)', () => { + const titles = uniqueTitles(['Capelix AB', 'Capelix AB'], 20) + expect(new Set(titles).size).toBe(2) + expect(titles[0]).toBe('Capelix AB 1') + expect(titles[1]).toBe('Capelix AB 2') + for (const t of titles) expect(t.length).toBeLessThanOrEqual(20) + }) + + it('disambiguates names that only collide after truncation, within the limit', () => { + const titles = uniqueTitles( + ['Wennberg Fastighetsförvaltning AB', 'Wennberg Fastighetsförvaltning Holding AB'], + 20, + ) + expect(new Set(titles.map((t) => t.toLowerCase())).size).toBe(2) + for (const t of titles) { + expect(t.length).toBeLessThanOrEqual(20) + expect(t.startsWith('Wennberg')).toBe(true) + } + expect(titles[0].endsWith(' 1')).toBe(true) + expect(titles[1].endsWith(' 2')).toBe(true) + }) + + it('treats a case-only difference as a collision', () => { + const titles = uniqueTitles(['Bolag AB', 'bolag ab', 'Annat AB'], 24) + expect(titles).toEqual(['Bolag AB 1', 'bolag ab 2', 'Annat AB']) + }) +}) diff --git a/extensions/general/whatsapp-inbox/__tests__/graph-api.test.ts b/extensions/general/whatsapp-inbox/__tests__/graph-api.test.ts index 0a2449d8..3884f5b1 100644 --- a/extensions/general/whatsapp-inbox/__tests__/graph-api.test.ts +++ b/extensions/general/whatsapp-inbox/__tests__/graph-api.test.ts @@ -3,6 +3,8 @@ import { createQueuedMockSupabase } from '@/tests/helpers' import { TimeoutError } from '@/lib/http/fetch-with-timeout' import { sendText, + sendReplyButtons, + sendList, sendReaction, RECEIVED_REACTION_EMOJI, downloadMedia, @@ -98,6 +100,69 @@ describe('graph-api', () => { }) }) + describe('interactive titles are unique (#1589, Meta #131009 "Duplicate button title")', () => { + it('sendReplyButtons sends distinct titles when two options share a name, ids untouched', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ messages: [{ id: 'wamid.BTN' }] }), { status: 200 }), + ) + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: null }) + + await sendReplyButtons(supabase as unknown as SupabaseClient, { + to: '46701234567', + body: 'Vilket företag gäller kvittot?', + template: TEMPLATE.m6CompanyQuestion, + buttons: [ + { id: 'company-live', title: 'Capelix AB' }, + { id: 'company-archived', title: 'Capelix AB' }, + ], + }) + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit] + const payload = JSON.parse(init.body as string) as { + interactive: { action: { buttons: { reply: { id: string; title: string } }[] } } + } + const [first, second] = payload.interactive.action.buttons + expect(first.reply.title).not.toBe(second.reply.title) + expect(first.reply.title.length).toBeLessThanOrEqual(20) + expect(second.reply.title.length).toBeLessThanOrEqual(20) + expect(first.reply.id).toBe('company-live') + expect(second.reply.id).toBe('company-archived') + }) + + it('sendList sends distinct row titles when two options share a name, ids untouched', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ messages: [{ id: 'wamid.LIST' }] }), { status: 200 }), + ) + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: null }) + + await sendList(supabase as unknown as SupabaseClient, { + to: '46701234567', + body: 'Vilket företag gäller kvittot?', + template: TEMPLATE.m6CompanyQuestion, + buttonLabel: 'Välj företag', + rows: [ + { id: 'c-1', title: 'Bolag A AB' }, + { id: 'c-2', title: 'Wennberg Fastighetsförvaltning AB' }, + { id: 'c-3', title: 'Wennberg Fastighetsförvaltning Holding AB' }, + { id: 'c-4', title: 'Bolag D AB' }, + ], + }) + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit] + const payload = JSON.parse(init.body as string) as { + interactive: { action: { sections: { rows: { id: string; title: string }[] }[] } } + } + const rows = payload.interactive.action.sections[0].rows + expect(new Set(rows.map((r) => r.title.toLowerCase())).size).toBe(4) + for (const row of rows) expect(row.title.length).toBeLessThanOrEqual(24) + expect(rows.map((r) => r.id)).toEqual(['c-1', 'c-2', 'c-3', 'c-4']) + expect(rows[0].title).toBe('Bolag A AB') + expect(rows[3].title).toBe('Bolag D AB') + }) + }) + describe('sendReaction', () => { it('posts a reaction payload targeting the inbound wamid', async () => { fetchMock.mockResolvedValueOnce( diff --git a/extensions/general/whatsapp-inbox/__tests__/hardening.test.ts b/extensions/general/whatsapp-inbox/__tests__/hardening.test.ts index fcb10aab..e1740594 100644 --- a/extensions/general/whatsapp-inbox/__tests__/hardening.test.ts +++ b/extensions/general/whatsapp-inbox/__tests__/hardening.test.ts @@ -141,6 +141,9 @@ describe('company question is not one-shot when the send fails', () => { it('rolls the question back so the next receipt re-asks', async () => { sendButtonsMock.mockResolvedValue({ ok: false, wamid: null, errorDetail: 'Send failed (HTTP 500)' }) + // The numbered text fallback (#1589) must fail too before anything rolls + // back; with the default ok:true text mock the question would stay armed. + sendTextMock.mockResolvedValue({ ok: false, wamid: null, errorDetail: 'Send failed (HTTP 500)' }) const mock = createQueuedMockSupabase() enqueueAsk(mock) mock.enqueue({ @@ -162,6 +165,11 @@ describe('company question is not one-shot when the send fails', () => { }) expect(asked).toBe('not_asked') + // The numbered fallback was tried once, with the same template id, before + // giving up. + expect(sendTextMock).toHaveBeenCalledTimes(1) + expect(sendTextMock.mock.calls[0][1].template).toBe(TEMPLATE.m6CompanyQuestion) + expect(sendTextMock.mock.calls[0][1].body).toContain('1. Bolag A AB') const updates = mock .findCalls('whatsapp_conversations', 'update') .map((args) => args[0] as { state?: string; context?: Record }) diff --git a/extensions/general/whatsapp-inbox/__tests__/process-inbound.test.ts b/extensions/general/whatsapp-inbox/__tests__/process-inbound.test.ts index f5981078..e1e19e88 100644 --- a/extensions/general/whatsapp-inbox/__tests__/process-inbound.test.ts +++ b/extensions/general/whatsapp-inbox/__tests__/process-inbound.test.ts @@ -44,6 +44,14 @@ vi.mock('@/lib/core/documents/document-service', () => ({ computeSHA256: vi.fn().mockResolvedValue('sha-abc'), })) +// The drain (#1589) re-kicks re-opened rows through the cookieless service +// client; route it to a per-test queued mock so the follow-up run is +// observable instead of hitting a real Supabase URL. +const kickClient = vi.hoisted(() => ({ current: null as unknown })) +vi.mock('@/lib/auth/api-keys', () => ({ + createServiceClientNoCookies: vi.fn(() => kickClient.current), +})) + import { sendText, markReadWithTyping, @@ -177,6 +185,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) // load link enqueue({ data: makeConversation() }) // load conversation enqueue({ data: [{ company_id: 'company-1' }] }) // sole membership + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: null }) // sha256 dup check: none enqueue({ data: null }) // item channel_context load enqueue({ data: null }) // item channel_context update @@ -208,6 +217,11 @@ describe('processInboundMessage (media intake)', () => { .channel_context expect(contextArg.company_selected_via).toBe('single') + // "Sole membership" means sole LIVE membership (#1589): an archived + // same-named company must not turn this sender into a multi-company one. + expect(findCalls('company_members', 'select')[0][0]).toContain('companies!inner(archived_at)') + expect(findCalls('company_members', 'is')).toContainEqual(['companies.archived_at', null]) + const finalUpdate = lastUpdate(findCalls) expect(finalUpdate.processing_status).toBe('done') expect(finalUpdate.inbox_item_id).toBe('item-1') @@ -216,6 +230,164 @@ describe('processInboundMessage (media intake)', () => { expect(sendTextMock).not.toHaveBeenCalled() }) + it('drains receipts parked behind an unaskable company question when the sender resolves as single', async () => { + const kick = createQueuedMockSupabase() + kickClient.current = kick.supabase + kick.enqueue({ data: null }) // follow-up loadRow for the re-opened row (gone: keep it inert) + + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: makeRow() }) // load row + enqueue({ data: { id: 'msg-1' } }) // claim + enqueue({ data: makeLink() }) // load link + enqueue({ data: makeConversation() }) // load conversation + enqueue({ data: [{ company_id: 'company-1' }] }) // sole live membership + enqueue({ data: [{ id: 'stg-1' }, { id: 'stg-2' }] }) // drain: two rows were parked + enqueue({ data: null }) // sha256 dup check: none + enqueue({ data: null }) // item channel_context load + enqueue({ data: null }) // item channel_context update + enqueue({ data: null }) // final markStatus done + + const outcome = await processInboundMessage(supabase as unknown as SupabaseClient, 'msg-1') + expect(outcome).toEqual({ kind: 'media_processed', conversationId: 'conv-1' }) + + // The guarded re-open targets exactly the staged marker on this conversation. + const reopen = findCalls('whatsapp_messages', 'update').find( + (args) => + (args[0] as Record).processing_status === 'received' && + (args[0] as Record).error_message === null, + ) + expect(reopen).toBeTruthy() + const eqs = findCalls('whatsapp_messages', 'eq') + expect(eqs).toContainEqual(['error_message', STAGED_AWAITING_COMPANY]) + expect(eqs).toContainEqual(['conversation_id', 'conv-1']) + + // The re-opened rows were kicked: the follow-up run loaded the first one. + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(kick.findCalls('whatsapp_messages', 'eq')).toContainEqual(['id', 'stg-1']) + kickClient.current = null + }) + + it('does not drain when the company came from a pin or a default (the question is not dead)', async () => { + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: makeRow() }) + enqueue({ data: { id: 'msg-1' } }) + enqueue({ data: makeLink({ default_company_id: 'company-7' }) }) + enqueue({ data: makeConversation() }) + enqueue({ data: { company_id: 'company-7' } }) // membership check for default + enqueue({ data: null }) // dup check + enqueue({ data: null }) // item context load + enqueue({ data: null }) // item context update + enqueue({ data: null }) // markStatus done + + await processInboundMessage(supabase as unknown as SupabaseClient, 'msg-1') + + // The default-membership check is also a LIVE-membership check. + expect(findCalls('company_members', 'is')).toContainEqual(['companies.archived_at', null]) + const reopen = findCalls('whatsapp_messages', 'update').find( + (args) => (args[0] as Record).processing_status === 'received', + ) + expect(reopen).toBeUndefined() + }) + + it('clears the dead company question when the sender resolves as single (state, options, pending)', async () => { + const kick = createQueuedMockSupabase() + kickClient.current = kick.supabase + kick.enqueue({ data: null }) + + const deadQuestion = { + company_options: [ + { id: 'company-1', name: 'Alpha AB' }, + { id: 'company-2', name: 'Alpha AB' }, + ], + pending_question: { type: 'company', inbox_item_id: null, asked_at: '2026-08-01T09:30:00Z' }, + budget: { day_key: '2026-08-01', count: 1 }, + } + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: makeRow() }) // load row + enqueue({ data: { id: 'msg-1' } }) // claim + enqueue({ data: makeLink() }) // load link + enqueue({ data: makeConversation({ state: 'awaiting_company', context: deadQuestion }) }) + enqueue({ data: [{ company_id: 'company-1' }] }) // sole live membership (company-2 archived) + enqueue({ data: [{ id: 'stg-1' }] }) // drain: one row was parked + enqueue({ data: [makeConversation({ state: 'idle', context: { budget: deadQuestion.budget } })] }) // guarded clear won + enqueue({ data: null }) // sha256 dup check: none + enqueue({ data: null }) // item channel_context load + enqueue({ data: null }) // item channel_context update + enqueue({ data: null }) // final markStatus done + + const outcome = await processInboundMessage(supabase as unknown as SupabaseClient, 'msg-1') + expect(outcome).toEqual({ kind: 'media_processed', conversationId: 'conv-1' }) + + // The conversation is released: idle, no options to re-offer (one of + // them is the archived company), no company pending_question, and the + // rest of the context (budget) untouched. + const clears = findCalls('whatsapp_conversations', 'update') + expect(clears).toHaveLength(1) + const patch = clears[0][0] as { state: string; context: Record } + expect(patch.state).toBe('idle') + expect(patch.context).toEqual({ budget: deadQuestion.budget }) + // Guarded on the revision it read, same idiom as every context write. + expect(findCalls('whatsapp_conversations', 'eq')).toContainEqual([ + 'updated_at', + '2026-08-01T09:00:00Z', + ]) + kickClient.current = null + }) + + it('clears company_options kept past the 48h TTL (idle conversation) once the sender resolves as single', async () => { + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: makeRow() }) + enqueue({ data: { id: 'msg-1' } }) + enqueue({ data: makeLink() }) + enqueue({ + data: makeConversation({ + state: 'idle', + context: { company_options: [{ id: 'company-1', name: 'A' }, { id: 'company-2', name: 'B' }] }, + }), + }) + enqueue({ data: [{ company_id: 'company-1' }] }) // sole live membership + enqueue({ data: [] }) // drain: the parked rows already expired + enqueue({ data: [makeConversation()] }) // guarded clear won + enqueue({ data: null }) // dup check + enqueue({ data: null }) // item context load + enqueue({ data: null }) // item context update + enqueue({ data: null }) // markStatus done + + await processInboundMessage(supabase as unknown as SupabaseClient, 'msg-1') + + const clears = findCalls('whatsapp_conversations', 'update') + expect(clears).toHaveLength(1) + const patch = clears[0][0] as { state: string; context: Record } + expect(patch.state).toBe('idle') + expect(patch.context).toEqual({}) + }) + + it('leaves a representation question alone when the sender resolves as single', async () => { + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: makeRow() }) + enqueue({ data: { id: 'msg-1' } }) + enqueue({ data: makeLink() }) + enqueue({ + data: makeConversation({ + state: 'awaiting_representation', + context: { + pending_question: { type: 'representation', inbox_item_id: 'item-0', asked_at: '2026-08-01T09:30:00Z' }, + }, + }), + }) + enqueue({ data: [{ company_id: 'company-1' }] }) // sole live membership + enqueue({ data: [] }) // drain: nothing parked + enqueue({ data: null }) // dup check + enqueue({ data: null }) // item context load + enqueue({ data: null }) // item context update + enqueue({ data: null }) // markStatus done + + await processInboundMessage(supabase as unknown as SupabaseClient, 'msg-1') + + // Not a company question: nothing to clear, no conversation write. + expect(findCalls('whatsapp_conversations', 'update')).toHaveLength(0) + }) + it('does nothing when the claim is lost (already processing)', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: makeRow() }) @@ -415,6 +587,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) enqueue({ data: makeConversation() }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: null }) // M17 notice check: none sent yet enqueue({ data: null }) // markStatus skipped @@ -438,6 +611,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) enqueue({ data: makeConversation() }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: { id: 'earlier-m17' } }) // notice already sent enqueue({ data: null }) // markStatus skipped @@ -453,6 +627,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) enqueue({ data: makeConversation() }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: null }) // no inbox item for this message yet enqueue({ data: { id: 'existing-doc' } }) // dup found enqueue({ data: null }) // markStatus skipped @@ -491,6 +666,7 @@ describe('processInboundMessage (media intake)', () => { }), }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: null }) // dup check enqueue({ data: null }) // new item context load enqueue({ data: null }) // new item context update @@ -535,6 +711,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) enqueue({ data: makeConversation() }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: null }) // no inbox item yet enqueue({ data: null }) // markStatus error enqueue({ data: null }) // no M18 sent yet @@ -559,6 +736,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) enqueue({ data: makeConversation() }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: null }) // no inbox item yet enqueue({ data: null }) // markStatus error enqueue({ data: null }) // no M18 sent for this message yet @@ -578,6 +756,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) enqueue({ data: makeConversation() }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: null }) // no inbox item yet enqueue({ data: null }) // markStatus error enqueue({ data: { id: 'out-1' } }) // an M18 for this correlation exists @@ -595,6 +774,7 @@ describe('processInboundMessage (media intake)', () => { enqueue({ data: makeLink() }) enqueue({ data: makeConversation() }) enqueue({ data: [{ company_id: 'company-1' }] }) + enqueue({ data: [] }) // drain: nothing parked behind an old company question enqueue({ data: { id: 'item-winner' } }) // the winner's item enqueue({ data: null }) // markStatus done diff --git a/extensions/general/whatsapp-inbox/index.ts b/extensions/general/whatsapp-inbox/index.ts index e712f6d3..a2756e87 100644 --- a/extensions/general/whatsapp-inbox/index.ts +++ b/extensions/general/whatsapp-inbox/index.ts @@ -268,10 +268,14 @@ async function handleUnknownSender( processing_status: 'done', }) + // Non-archived only: the greeting must describe what the channel will + // then do (name the sole live company, or say it will ask), not count a + // company receipts can never be filed into (#1589). const { data: memberships } = await supabase .from('company_members') - .select('company_id') + .select('company_id, companies!inner(archived_at)') .eq('user_id', consumed.userId) + .is('companies.archived_at', null) const companyIds = [...new Set((memberships ?? []).map((m) => m.company_id as string))] let companyName: string | null = null diff --git a/extensions/general/whatsapp-inbox/lib/company-question.ts b/extensions/general/whatsapp-inbox/lib/company-question.ts index a3d7c6f1..b169cd40 100644 --- a/extensions/general/whatsapp-inbox/lib/company-question.ts +++ b/extensions/general/whatsapp-inbox/lib/company-question.ts @@ -21,6 +21,7 @@ import { MAX_REPLY_BUTTONS, MAX_LIST_ROWS, type SendMessageBase, + type SendTextResult, } from './graph-api' import { botCopy, TEMPLATE } from './messages' import { @@ -51,10 +52,12 @@ export interface CompanyOption { type ReplyBase = Omit -/** All companies the linked user belongs to, alphabetical (stable digits). - * Returns null when a query FAILED: a transient DB error must read as - * "unknown", never as "no memberships", or the caller parks receipts behind - * a question that can never be asked. */ +/** All non-archived companies the linked user belongs to, alphabetical + * (stable digits). An archived company is not a place receipts can go, and + * offering it produced a Meta-rejected payload when it shared its name with + * the live one (#1589). Returns null when a query FAILED: a transient DB + * error must read as "unknown", never as "no memberships", or the caller + * parks receipts behind a question that can never be asked. */ export async function loadCompanyOptions( supabase: SupabaseClient, userId: string, @@ -74,6 +77,7 @@ export async function loadCompanyOptions( .from('companies') .select('id, name') .in('id', companyIds) + .is('archived_at', null) if (companiesError) { log.warn('company options company query failed', { error: companiesError.message }) return null @@ -107,6 +111,15 @@ export type AskCompanyQuestionOutcome = 'asked' | 'not_asked' | 'no_options' | ' * the rollback an expired token or a Graph 5xx left the conversation parked on * a question the user never received, with the guard suppressing every later * ask and the 48h TTL eventually discarding the staged receipts. + * + * Send ladder: reply buttons (<=3) or a list (<=10), then the numbered text + * variant when Meta rejects the interactive payload synchronously (#1589: + * HTTP 400 "Duplicate button title" and similar payload-shape errors are + * final for that payload, but plain text still delivers), and only when the + * text send ALSO fails is the question rolled back. The fallback reuses the + * M6 template id, so the audit/throttle keys are unchanged, and the answer + * path is mode-agnostic: a typed digit is accepted whenever company_options + * are open and is recorded as via='numbered'. */ export async function askCompanyQuestion( supabase: SupabaseClient, @@ -186,16 +199,21 @@ export async function askCompanyQuestion( const copy = botCopy('sv') const body = copy.m6CompanyQuestion({ count: args.stagedCount }) + const numberedBody = copy.m6CompanyQuestionNumbered({ + count: args.stagedCount, + options: options.map((o) => o.name), + }) const base = { to: args.to, template: TEMPLATE.m6CompanyQuestion, ...args.replyBase } - let sent: { ok: boolean } + const interactive = options.length <= MAX_LIST_ROWS + let sent: SendTextResult if (options.length <= MAX_REPLY_BUTTONS) { sent = await sendReplyButtons(supabase, { ...base, body, buttons: options.map((o) => ({ id: o.id, title: o.name })), }) - } else if (options.length <= MAX_LIST_ROWS) { + } else if (interactive) { sent = await sendList(supabase, { ...base, body, @@ -203,13 +221,19 @@ export async function askCompanyQuestion( rows: options.map((o) => ({ id: o.id, title: o.name })), }) } else { - sent = await sendText(supabase, { - ...base, - body: copy.m6CompanyQuestionNumbered({ - count: args.stagedCount, - options: options.map((o) => o.name), - }), + sent = await sendText(supabase, { ...base, body: numberedBody }) + } + + if (!sent.ok && interactive) { + // Meta rejected the interactive payload at send time (no wamid was ever + // issued, so nothing reached the phone): ask the same question as plain + // numbered text instead of going silent. Same template id, same open + // question; only the answer mechanism changes (digit instead of tap). + log.warn('company question interactive send rejected; falling back to numbered text', { + conversationId: args.conversation.id, + errorDetail: sent.errorDetail, }) + sent = await sendText(supabase, { ...base, body: numberedBody }) } if (!sent.ok) { @@ -291,14 +315,17 @@ export async function applyCompanyChoice( } if (!companyId) return { ok: false, reason: 'invalid_option' } - // Defense in depth: the chosen company must be one the sender belongs to, - // whatever the payload claimed. A query ERROR is not a missing membership: - // treating a transient failure as "not a member" silently drops the answer. + // Defense in depth: the chosen company must be one the sender belongs to + // AND still be live, whatever the payload claimed (a stale tap can name a + // company archived since the question was asked). A query ERROR is not a + // missing membership: treating a transient failure as "not a member" + // silently drops the answer. const { data: membership, error: membershipError } = await supabase .from('company_members') - .select('company_id') + .select('company_id, companies!inner(archived_at)') .eq('user_id', args.link.user_id) .eq('company_id', companyId) + .is('companies.archived_at', null) .limit(1) .maybeSingle() if (membershipError) { diff --git a/extensions/general/whatsapp-inbox/lib/graph-api.ts b/extensions/general/whatsapp-inbox/lib/graph-api.ts index 8477aa9a..ced90eb8 100644 --- a/extensions/general/whatsapp-inbox/lib/graph-api.ts +++ b/extensions/general/whatsapp-inbox/lib/graph-api.ts @@ -198,6 +198,30 @@ export function truncateTitle(name: string, max: number): string { return `${cut.trimEnd()}…` } +/** + * Truncate every title to `max` and make the results unique. Meta rejects an + * interactive payload whose buttons or rows share a title (HTTP 400, error + * #131009 "Duplicate button title"), which happens when a sender belongs to + * two same-named companies or when two long names truncate to the same + * prefix. Colliding entries (compared case-insensitively) get their 1-based + * position appended: the same digit the numbered text variant and the + * typed-digit answer path use, so a "Bolag AB 2" button and a "2" reply mean + * the same option. The result never exceeds `max`. + */ +export function uniqueTitles(titles: string[], max: number): string[] { + const truncated = titles.map((t) => truncateTitle(t, max)) + const occurrences = new Map() + for (const title of truncated) { + const key = title.toLowerCase() + occurrences.set(key, (occurrences.get(key) ?? 0) + 1) + } + return truncated.map((title, i) => { + if ((occurrences.get(title.toLowerCase()) ?? 0) < 2) return title + const suffix = ` ${i + 1}` + return `${truncateTitle(titles[i], max - suffix.length)}${suffix}` + }) +} + export interface SendReplyButtonsArgs extends SendMessageBase { body: string /** At most MAX_REPLY_BUTTONS options; extras are dropped defensively. */ @@ -212,6 +236,10 @@ export async function sendReplyButtons( args: SendReplyButtonsArgs, ): Promise { const buttons = args.buttons.slice(0, MAX_REPLY_BUTTONS) + const titles = uniqueTitles( + buttons.map((b) => b.title), + BUTTON_TITLE_MAX, + ) const result = await postToGraph( { messaging_product: 'whatsapp', @@ -222,9 +250,9 @@ export async function sendReplyButtons( type: 'button', body: { text: args.body }, action: { - buttons: buttons.map((b) => ({ + buttons: buttons.map((b, i) => ({ type: 'reply', - reply: { id: b.id, title: truncateTitle(b.title, BUTTON_TITLE_MAX) }, + reply: { id: b.id, title: titles[i] }, })), }, }, @@ -259,6 +287,10 @@ export async function sendList( args: SendListArgs, ): Promise { const rows = args.rows.slice(0, MAX_LIST_ROWS) + const titles = uniqueTitles( + rows.map((r) => r.title), + LIST_ROW_TITLE_MAX, + ) const result = await postToGraph( { messaging_product: 'whatsapp', @@ -272,9 +304,9 @@ export async function sendList( button: truncateTitle(args.buttonLabel, BUTTON_TITLE_MAX), sections: [ { - rows: rows.map((r) => ({ + rows: rows.map((r, i) => ({ id: r.id, - title: truncateTitle(r.title, LIST_ROW_TITLE_MAX), + title: titles[i], })), }, ], diff --git a/extensions/general/whatsapp-inbox/lib/process-inbound.ts b/extensions/general/whatsapp-inbox/lib/process-inbound.ts index 2741ccb7..e2dac5f1 100644 --- a/extensions/general/whatsapp-inbox/lib/process-inbound.ts +++ b/extensions/general/whatsapp-inbox/lib/process-inbound.ts @@ -147,6 +147,8 @@ async function loadLink( return (data as WhatsAppPhoneLink | null) ?? null } +/** Live membership: a pin or default pointing at an ARCHIVED company must not + * receive receipts (same filter shape as lib/supabase/middleware.ts). */ async function isMember( supabase: SupabaseClient, userId: string, @@ -154,9 +156,10 @@ async function isMember( ): Promise { const { data } = await supabase .from('company_members') - .select('company_id') + .select('company_id, companies!inner(archived_at)') .eq('user_id', userId) .eq('company_id', companyId) + .is('companies.archived_at', null) .limit(1) .maybeSingle() return data != null @@ -275,10 +278,31 @@ interface ResolvedCompany { via: NonNullable } +/** + * True when a company question is still open on the conversation, in any of + * the shapes it survives in: the awaiting_company state, the options behind a + * digit answer (kept past the 48h TTL for a late reply), or the company + * pending_question. Once the sender resolves as 'single' none of them can be + * answered any more. + */ +function hasDeadCompanyQuestion(conversation: WhatsAppConversation): boolean { + const context = getContext(conversation) + return ( + conversation.state === 'awaiting_company' || + (context.company_options?.length ?? 0) > 0 || + context.pending_question?.type === 'company' + ) +} + /** * Conversation pin (live + still a member, sliding 8h) -> default company * (still a member) -> sole membership -> null (ask). * + * Every membership here means a NON-ARCHIVED company: an archived one is not + * a place receipts can go, and counting it made a sender with one live + * company look multi-company (#1589), so they were asked a question Meta + * refused to deliver. + * * Returns 'transient_error' when the membership query FAILED: a DB blip must * not read as "no memberships", which used to park the rows behind an * unanswerable company question forever. The caller releases the row so the @@ -318,8 +342,9 @@ async function resolveCompanyTarget( const { data: memberships, error: membershipsError } = await supabase .from('company_members') - .select('company_id') + .select('company_id, companies!inner(archived_at)') .eq('user_id', link.user_id) + .is('companies.archived_at', null) if (membershipsError) { log.warn('membership query failed during company resolution; will retry', { error: membershipsError.message, @@ -512,6 +537,55 @@ async function processMediaMessage( } const companyId = resolved.companyId + if (resolved.via === 'single' && conversation) { + // Receipts parked behind an earlier company question can never be + // answered now: the sender's other memberships are archived (or gone), + // so that question will not be asked again, and the sole live company + // is unambiguous by construction. Re-open them so they file here + // instead of expiring at Meta. Same guarded idiom as applyCompanyChoice; + // the re-run resolves them as 'single' on its own. default/pin + // resolution deliberately does not drain: those choices the user can + // still change, so the open question there is not dead. + const { data: parked } = await supabase + .from('whatsapp_messages') + .update({ processing_status: 'received', error_message: null }) + .eq('conversation_id', conversation.id) + .eq('processing_status', 'skipped') + .eq('error_message', STAGED_AWAITING_COMPANY) + .select('id') + const parkedIds = ((parked ?? []) as { id: string }[]).map((r) => r.id) + if (parkedIds.length > 0) { + log.info('re-opened receipts parked behind an unaskable company question', { + conversationId: conversation.id, + count: parkedIds.length, + }) + kickInboundProcessing(parkedIds) + } + // The question itself is as dead as the rows behind it. Left in place, + // state 'awaiting_company' turns every typed word into a company_retry + // that re-offers the archived company, swallows 'byt', and keeps + // finalizeBurst from asking anything about the drained receipts until + // the 48h TTL sweep. Guarded and re-checked against fresh state, so a + // concurrent worker that already cleared it is a no-op, and only the + // company question is touched: a representation/context question that + // opened in between stays. + if (hasDeadCompanyQuestion(conversation)) { + await updateConversation(supabase, conversation, (current, currentContext) => { + if (!hasDeadCompanyQuestion(current)) return null + const nextContext: ConversationContext = { ...currentContext } + delete nextContext.company_options + if (nextContext.pending_question?.type === 'company') delete nextContext.pending_question + return { + state: current.state === 'awaiting_company' ? 'idle' : current.state, + context: nextContext, + } + }) + log.info('cleared the dead company question on a single-company conversation', { + conversationId: conversation.id, + }) + } + } + // ── Per-company intake quota (ack-and-drop, never retryable) ── const limit = await checkInboxUploadRateLimit(supabase, companyId) if (!limit.ok) {