inboundMessageIds was only ever cleared whole (on disconnect), so it grew
without bound proportional to total inbound message volume over a
connection's lifetime. Now capped at 5000 entries, evicting the oldest
(Set preserves insertion order) once exceeded.
withTimeout's internal setTimeout wasn't unref'd, unlike every other timer
in this file (qrTimer, reconnectTimer, the lease timer) — a pending
lookup/send timeout could keep the event loop alive if the process ever
tried to exit gracefully without an explicit process.exit().
Found via a full-project audit.
attemptsToday/failureCount joined outreach_sends to outreach_campaign_queue
by lead_id + campaign_id instead of the actual queue-to-send link
(outreach_send_id). campaignMetrics() already used outreach_send_id for
its own equivalent queries (see the existing F1 regression test) after a
prior fix; evaluateCampaignHealth's separate queries had not received the
same fix. Currently unreachable in practice since the campaign catalog
uses mutually-exclusive segments per lead, but a latent bug: if a lead
ever had automated-send history in two campaigns, one campaign's failures
could count toward another's high_failure_rate auto-pause threshold.
Fixed two existing test fixtures that happened to pass without actually
exercising the attempt/failure-rate path they were named for (their sends
weren't linked via outreach_send_id, so attemptsToday silently came out as
0 pre-fix). Added a regression test (F1b) mirroring the existing F1 test's
cross-campaign-contamination scenario for campaignMetrics, applied to
evaluateCampaignHealth instead; verified it fails against the pre-fix
query and passes against the fix.
Found via a full-project audit.
buildLeadFilter used Number(filtro.prioridade) || 4, so an explicit
prioridade=0 or any non-numeric value (both falsy after Number()) matched
P4 leads instead of the requested (invalid) priority. Not reachable via
the UI (its dropdown only offers 1-4) but reachable through a direct API
call. Drops the fallback: an invalid value becomes NaN, which node:sqlite
accepts as a bound parameter and never compares equal to anything, so it
now correctly matches zero rows instead of silently redirecting to P4.
Found via a full-project audit.
pg-prev/pg-next deliberately skip comCarregando (it would fight the
page-boundary disabled-state logic), so nothing stopped two rapid clicks
from firing overlapping GET /api/leads requests. If the earlier page's
response resolved after the later one, it silently overwrote the table,
pager text and prev/next disabled state with a page the operator was no
longer viewing. Adds a per-call sequence number in carregarTabela(); a
response is only applied if no newer call has started since, covering
every caller (pager clicks, filter changes, tab activation) with one guard.
Found via a full-project audit; verified the regression test fails
against the pre-fix code and passes against the fix.
carregarMetrics()/carregarMetricsData() swallowed fetch errors with a bare
catch, leaving the metrics card blank with no indication anything went
wrong (same class of issue already fixed for pending replies). Now shows
an explicit error message and toasts.
Found via a full-project audit.
search() accumulated results across up to 3 pages in a local array, but a
failure on page 2 or 3 (e.g. Google's next_page_token not active yet
despite the 2.5s wait, or a transient error) propagated out and lost the
whole accumulated array, not just the failing page. The caller's catch
only logs and continues, discarding leads that were already successfully
fetched. Now only rethrows (preserving existing behavior) when nothing has
been accumulated yet; a failure after at least one successful page logs
and returns what was already collected.
Found via a full-project audit.
exportExcel's CSV/XLS write streams had no 'error' listener, so a write
failure (disk full, export directory becomes inaccessible mid-run — both
realistic for a large CNPJ export) surfaced as an uncaught exception that
crashed the whole export-job.mjs child process, bypassing every
await/try-catch in the caller. The exports row was then never marked
'error' and stayed stuck, and a half-written file could sit at the final
path (no temp+rename, unlike the same pattern already used in
rfb-extractor.mjs/rfb-downloader.mjs).
Streams now track their error via a listener instead of throwing, writes
and stream-end are rejected through the normal promise chain so
export-job.mjs's existing catch/finally handles it, and both files are
written to .tmp paths and only renamed to their final names once both
streams close successfully.
Found via a full-project audit.
The 'line' handler applied a t:'error' event immediately and unconditionally,
even when a t:'done' had already arrived (deferred as pendingDone until
close). A job whose real work finished successfully (io.done() called,
e.g. leads already committed by the child) could still end up persisted
and reported as 'error' if something in the child's finally-block cleanup
(closing the lease/db) threw afterward, since main().catch() calls
io.fail() + process.exit(1) in that case — discarding a real success
summary for a cosmetic cleanup failure.
Fix distinguishes this from a genuinely unexplained nonzero exit (process
killed/crashed with no chance to report anything), which two existing
tests deliberately require to still override a done: a new
lateErrorAfterDone flag is only set when an explicit error line follows a
done, and close only trusts the done over a nonzero exit code when that
flag is set.
Found via a full-project audit.
carregarReplies() swallowed any fetch error, leaving whatever was already
in #pending-replies — on first load, the static "Nenhuma resposta
pendente." placeholder. A failed load was visually indistinguishable from
a genuine empty state, so a customer reply could go unnoticed with no
error surfaced (every other action in this file already toasts on
failure). Now shows an explicit error state and toasts, and the fetch
mock harness in campaign-ui.test.mjs gained toast-call tracking to prove it.
Found via a full-project audit.
isOptedOut/addOptOut/removeOptOut used equivalentPhoneNumbers, which is not
symmetric for a 12-digit local outside the 6-9 range vs its 13-digit
counterpart (same root cause already fixed for shared-number detection).
An opt-out registered against one form of such a number silently failed to
block sends to a lead whose number was stored in the other form, on both
the manual send path (reserveOutreachSend) and, independently, in two
inlined checks that bypassed isOptedOut entirely: the last-mile recheck in
campaign-worker.mjs's reserveCampaignQueueSend, and the opt-out NOT EXISTS
clause in campaign-queue.mjs's candidate query. All three now use the
symmetric canonicalPhoneVariants key already introduced for shared-number
overrides, and the worker's last-mile check now calls isOptedOut directly
instead of re-deriving its own number list.
Found via a full-project audit; reproduced and verified before fixing.
Drop any row whose id disappeared between the id-pagination query and the
hydrate query (e.g. a lead deleted by a base reset mid-request) instead of
leaking an undefined hole into the response. Document that SORTS may only
reference leads columns, since the id-pagination query no longer joins
segments.
Re-review of afa350d found that the correlated-subquery rewrite in
listLeads regressed any page landing in a large sort-tie block (e.g. the
default prioridade:asc sort's prioridade=4/score=0 bucket): SQLite's block
sort materializes the whole tied group before LIMIT/OFFSET apply, so the
shared-number subqueries got evaluated for the entire block instead of just
the page (measured 8-9x slower on page 2+ of a 60k-row fixture).
Split listLeads into two queries: a cheap id-only pass that does the
sorting/pagination (same cost as before the shared-number feature existed),
then a hydrate pass scoped to WHERE leads.id IN (...) for just that page's
ids, which is where the shared-number subqueries now run. getLead was
already a single-id lookup and didn't need this.
Also fixed the accompanying test, which only checked page 1 (a small,
already-sorted block) and so couldn't have caught the regression it was
meant to guard against; it now also checks a page landing inside the large
tied block.
The last-mile validator in campaign-worker.mjs revalidated opt-out/inbound/
dedupe right before sending but never rechecked shared-number state, so
re-blocking a released number after scheduling didn't stop the send.
equivalentPhoneNumbers isn't symmetric for all 8/9-digit pairs, which made
isNumberSharedAcrossLeads and listSharedNumberGroups disagree for a 12-digit
local outside the 6-9 range vs its 13-digit counterpart: one side of the pair
could be enforcement-blocked while invisible and unreleasable in the review
screen. A new canonicalPhoneKey/canonicalPhoneVariants pair in phone.mjs
(plus SQL mirrors) makes the matching relation symmetric by construction.
listSharedNumberGroups did a full leads-table scan on every listLeads/getLead
call, the app's primary paginated screen. Replaced with per-row correlated
subqueries against idx_leads_whatsapp so cost tracks the page, not the table.
normalizedSql/equivalentSql (SQL) duplicated equivalentPhoneNumbers (JS) with
no test proving they agreed; moved into phone.mjs alongside their canonical
counterparts and added tests/phone.test.mjs asserting agreement across a
shared number set, including the asymmetric case.
Adds an admin-only "Numeros compartilhados entre leads" section to the
settings modal, wired to the Task 4 shared-numbers routes: lists
detected groups, lets an admin liberar (with an inline motivo input)
or bloquear de novo. Updates the two frontend VM-based test files'
default API mocks to answer GET /api/outreach/shared-numbers so
existing settings-modal tests keep passing, and adds a new
interaction test covering the liberar round trip.
Review found the comment claimed a per-row precomputation optimization
that SQLite does not actually perform: the reviewer counted replace()
opcodes in the compiled bytecode and got an identical count (305) for
both the inline and derived-subquery forms of the query, since SQLite
flattens the subquery and re-substitutes the expression text at each
usage site either way. Rewrote the comment to describe the derived
subquery as what it actually is (a readability device), and to state
plainly that the perf investigation did not conclusively isolate the
rare timing outliers to this SQL's shape.
The candidate query's lead_number/lead_equivalent forms were being
re-embedded as raw SQL expression text at every usage site (verification
join, opt-out check, shared-number check, inbound-replies check). Each
embedding re-evaluates the same deeply nested replace()/substr()/length()
chain per row; adding the shared-number check as a 5th usage site made
this measurably worse, especially in the worst case where a segment has
zero pre-verified numbers and the scan can't terminate early via LIMIT
(confirmed via reconstructing the pre-Task-3 query on an identical
fixture: that scenario already cost ~3.4s before this feature, and the
5th usage site pushed it to 4.1-9.5s with real variance, including an
outright failure over 5 consecutive runs).
Restructured candidateQuery to compute lead_number/lead_equivalent once
per row in a derived subquery (aliased `candidates`), referenced by
column everywhere else instead of re-embedding the expression text.
SQLite still flattens/uses idx_leads_campaign_order for the segmento
scan and sort (verified via EXPLAIN QUERY PLAN), so the common fast path
is unaffected (~2ms), while the worst-case path dropped to a consistent
~3-5s across 5 consecutive runs of the 1.3M-row perf test.
candidateQuery now blocks a lead whose WhatsApp number is shared by 2+
leads, unless a shared-number override exists. The inner correlated
subquery compares the bare other.whatsapp column (not normalizedSql())
against the outer row's two computed number forms so SQLite can use
idx_leads_whatsapp instead of a full table scan, keeping the 1.3M-row
perf test within budget.
Also updates the pre-existing 'persiste erro temporario e nao repete
equivalente' test: its two leads use 8/9-digit equivalent numbers,
which the new filter now blocks by default, so an override is added
to keep exercising the intended equivalent-number dedup behavior.
Implement shared-number detection in reserveOutreachSend: reject manual WhatsApp sends with HTTP 409 when the destination number is shared across 2+ leads and hasn't been explicitly released via override.
- Add test: reserveOutreachSend bloqueia numero compartilhado sem override e libera com override
- Add check in reserveOutreachSend right after isOptedOut check
- Uses isNumberSharedAcrossLeads and isSharedNumberOverridden from Task 1
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The v15 test pattern (delete migration records >= 15, drop schema, reopen to replay)
was failing because outreach_shared_number_overrides table from v19 migration was not
dropped before replaying. Add DROP TABLE IF EXISTS to the schema cleanup block to
match the same fix applied to v17 test.
This resolves the 'table already exists' error during v19 migration replay.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Add migration v19: creates outreach_shared_number_overrides table
- Implement 7 new data-layer functions:
- leadsSharingNumber: find leads sharing a phone number variant
- isNumberSharedAcrossLeads: detect if number is shared by 2+ leads
- getSharedNumberOverride: retrieve override entry for a number
- isSharedNumberOverridden: check if number has an override
- addSharedNumberOverride: create or update override with reason
- removeSharedNumberOverride: delete override record
- listSharedNumberGroups: group leads by shared numbers, reflect override state
- Add 4 comprehensive tests covering migration, detection, override crud, and grouping
- Update version assertions from 18 to 19
- Fix v17 migration test to properly clean up v19 table for schema reset
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Tracks the app that was already in place (capture, treatment, outreach,
WhatsApp integration, server, frontend, tests, docs) so future feature
work can branch and use isolated worktrees.
Detects when a WhatsApp number is shared by multiple leads (e.g. an
accountant's phone reused across client CNPJs) and blocks automatic
and manual outreach to all of them until an operator reviews and
releases the number.
Gustavo
