Per-product message templates, official/não-oficial channel selection,
manual per-number sending, and a 20-minute internal scheduler for the
existing automatic outreach batch — all pre-existing local work being
committed as-is.
- Wrap the "Marcar" stage button's onclick body in try/catch (toast on
error), matching the pattern already applied to the 5 campaign action
buttons.
- carregarSegmentos' error handler (from the produto <select> change
handler) can fire while #modal-nova-campanha is open, where a toast
alone is hidden behind the open <dialog> (documented in api.js next to
the toast() function) -- now also surfaces the error in the existing
inline #campanha-erro paragraph.
- addVarianteRow now caps at 3 rows (A/B/C, matching the convention used
elsewhere in the UI/backend) and disables "+ Variante" once the cap is
reached.
Two issues in the campaign worker path:
1. runCampaignQueue had none of the safety rails run.mjs's automatic sender
has: no outreach.ativo gate, no business-hours window, no jitter between
sends. A campaign left "rodando" would send around the clock even with
outreach disabled or outside business hours, and would hammer the same
WhatsApp number back-to-back. Added the same outreach.ativo/horaInicio/
horaFim checks (duplicated, not imported, to keep the modules
independent) plus a randomized 3-8s jitter between real send attempts.
Sleep delay is injectable via an optional sleepMs param so tests can
pass sleepMs: 0.
2. evaluateCampaignHealth mutated state (via applyPause) on every call,
including from the read-only GET /api/campaigns/:id/saude route -- so
simply opening the campaign detail panel in the UI could silently pause
a healthy-looking campaign. Split it into a pure evaluateCampaignHealth
(computes {pause, reason}, never writes) and a new applyCampaignHealth
(evaluates, then persists the pause if warranted). The worker now calls
applyCampaignHealth; the GET /saude route keeps calling the pure
evaluateCampaignHealth.
Tests: worker sends nothing with outreach.ativo=false or outside business
hours; evaluateCampaignHealth confirmed not to mutate status even when it
would return pause:true; existing mutation assertions moved onto
applyCampaignHealth.
resumeCampaign only updated status/pause_reason, so a campaign auto-paused
for many days and then resumed could immediately satisfy
escalateWeeklyLimits' 7-day check and double its daily limit right after
coming back from an unhealthy state -- the opposite of the intended
safety behavior. Now resumeCampaign also stamps semana_iniciada_em to
datetime('now'), resetting the weekly clock on every resume.
The real manual-send route (routes/outreach.mjs) always inserts with
produto='' and lead_id=NULL, so the old single NOT EXISTS predicate
(produto = ? AND (lead_id = leads.id OR numero = leads.whatsapp)) could
never match a real manual send's row -- the numero branch was unreachable
in production even though a prior test fabricated a produto value manual
sends never write. Split into two NOT EXISTS clauses: one scoped to
produto+lead_id (automatic/campaign sends), one scoped to numero+
origem='manual' with no produto requirement (manual sends). Updated the
existing regression test to insert the row exactly like the real route
does (produto: '', lead_id: NULL, origem: 'manual').
Marking a lead opt_out via /estagio only wrote outreach_lead_states/
outreach_stage_events and never called addOptOut, so the lead stayed fully
eligible for future campaign sends. Also normalize numero with onlyDigits
in POST/DELETE opt-out routes and in the estagio lookup, matching how every
other number in the app (leads.whatsapp, queue.numero) is stored, so opt-out
comparisons never silently miss due to formatting differences.
Relabel misleading "Empresa" header to "Lead ID" in fila table (queue
has no join to leads, cell showed lead_id). Wrap iniciar/pausar/retomar
and aprovar/cancelar selecionados handlers in try/catch with toast on
failure, matching btn-gerar-fila's existing pattern.
The new-campaign segment checkboxes reused .chip/.chips, which is
reserved codebase-wide for click-to-remove selected tags (adds a
trailing " x" and removable-tag hover styling). Give the segment
picker its own segmento-opcao/segmento-lista classes with a minimal
non-removable style instead.
generateCampaignQueue computed candidatosEncontrados after slicing to
quantidade, so it could never differ from adicionadosNaFila. An operator
generating a queue with quantidade smaller than the eligible pool saw no
signal that more eligible leads existed. Now candidatosEncontrados is read
before the slice, while adicionadosNaFila still reflects rows actually
inserted.
Previously, setCampaignStatus did not set semana_iniciada_em, causing
escalateWeeklyLimits to bail early (if (!campaign.semana_iniciada_em) continue).
Now when a campaign transitions to 'rodando' for the first time, semana_iniciada_em
is set to datetime('now'). Subsequent rodando transitions leave it unchanged
(using COALESCE to preserve first-run timestamp).
Regression tests confirm:
- First transition to 'rodando' stamps semana_iniciada_em
- Subsequent transitions do not reset the timestamp
- resumeCampaign continues to work correctly (pause/resume cycle)
- All 105 tests pass
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This work already existed uncommitted in the main working tree (not
authored by this branch) and is required as a runtime dependency for the
campaign outreach port (zpro-client, templates, outreach routes/UI). Copied
in as-is, untouched, so this feature branch has a working baseline; the
original uncommitted state in the main checkout is left exactly as it was.
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.
GustavoandClaude Sonnet 5