fix(whatsapp): unknown-sender quota RPC fails open to the throttled greeting path (#1991)
* fix(whatsapp): unknown-sender quota RPC fails open to the throttled greeting path (#1599) When check_and_increment_whatsapp_sender_quota errored, handleUnknownSender logged, wrote a fail-closed trace row and returned: a transient DB hiccup silenced a first-time sender at the exact moment they were trying to link. The limiter being unavailable now falls through to the existing greeting path, whose own throttle (1 M1 per hour for text, 10-minute media burst, 3 per day, fail-closed on its own read error) and the single-use link-code claim already bound outbound volume. A thrown RPC (network) is treated the same as a PostgREST error. Over-quota (ok: false) is untouched: silent by design, decline trace kept. In degraded mode a valid code still binds and gets M3; a bad code gets the throttled M1 instead of M2, because only the quota bounds M2. The greeting dispositions carry a ' (quota limiter unavailable)' suffix so support can tell the two modes apart; suffix rather than prefix because last-event.ts matches dispositions with startsWith. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkUfWtuFCUkNtRAgMQCse2 * fix(whatsapp): answer a bad link code with a throttled M2 in degraded mode Review finding on #1991: withholding M2 while the quota RPC is down left the worst sub-path of the linking moment silent. A sender greeted with M1 inside the last hour who then sends an expired or mistyped code fell through to the greeting path, where the 1/hour rule declined M1, so they heard nothing at all; exactly the silence issue #1599 targets. M2 now gets its own small bound instead of being withheld: a new badCodeThrottled read in lib/conversation.ts (mirrors greetingThrottled, keyed on raw_payload->>template = m2_bad_code: 1 per 10 minutes, 3 per day per phone hash, fail-closed on read error). In degraded mode a bad code sends M2 when that throttle allows and otherwise falls through to the existing M1/silence path. The normal path is untouched: the short-circuit only does the extra read when the quota RPC was unavailable. The M2 trace disposition carries the degraded suffix. Tests: the replaced "withholds M2" case now asserts M2 goes out once under its own throttle with the degraded trace suffix; new cases cover a repeated bad code inside the 10 min window (silent skipped trace via the greeting throttle), the 3/day cap, and an unreadable M2 window failing closed to the M1 path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
Jakob Wennberg
parent
f0af4ad4ee
commit
57ff2eda96
@@ -1298,6 +1298,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. 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_<ts>_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<string, unknown>[] 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 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.
|
||||
[2026-08-27] reverseEntry also deletes the reversed entry's transaction_voucher_links rows and releases (is_business, category, reconciliation_method to null) only the rows left with no anchor: bulk-booked samlingsverifikat anchor N>1 bank rows through the junction alone, so the journal_entry_id-scoped reset (#1950) matched nothing there and the rows stayed out of Att bokföra against a reversed entry. The release is guarded by a remaining-links read plus journal_entry_id IS NULL because a residual booking (lib/reconciliation/residual.ts) keeps the main verifikat in journal_entry_id while its junction row points at the residual verifikat; stornoing the residual must not put a still-booked row back in the list.
|
||||
|
||||
Reference in New Issue
Block a user