feat(api): add GET /health endpoint with WAHA status & Docker HEALTHCHECK (#22) - #23
Closed
bdmohammed wants to merge 38 commits into
Closed
bdmohammed wants to merge 38 commits into
bdmohammed wants to merge 38 commits into
Conversation
…y-specific brains SARA is a self-hostable AI agent for WhatsApp that understands your industry. 20 verticals, 30+ function-calling tools, RAG engine, multi-provider AI chain, PII anonymization, prompt injection guardrails, multi-language (IT/EN/ES/PT). Stack: Node.js 20, TypeScript, Fastify, PostgreSQL+pgvector, Docker. License: AGPL-3.0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…UTING.md - Remove NON-AVVIARE-SARA-QUI.md (contained production server IP) - Remove ale.code-workspace, README_HARDENED.md, waha-qr-static.png - Remove benchmark/test JSON files (internal artifacts) - Add CONTRIBUTING.md (was linked from README but missing) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove FREE (€0) plan references (dead since Jul 17) - Fix line 455: SARA WhatsApp is SCALE-only, not Growth - Add add-on pricing (WhatsApp Connect €19, Voice €29, WA Business API €39) - Update all 4 languages (IT/EN/ES/PT) in vertical-prompts.ts - Remove "no add-ons exist" claim (add-ons DO exist) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…), add CONTRIBUTING.md - FREE plan removed from ai.ts static FAQ (demo + crm keys in all 4 languages) - SARA WhatsApp correctly marked as SCALE-only (was incorrectly listed as GROWTH) - Fixed pricing in all 4 languages (IT/EN/ES/PT) - Added branch naming, code style, and pricing constraints to CONTRIBUTING.md - AGPL-3.0 license notice explicit in CONTRIBUTING.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- populate_rag.mjs / populate_rag_v2.mjs: remove hardcoded fallback connection string with plaintext password, require DATABASE_URL env var - Untrack .wwebjs_cache/, .crash_history.json, .reconnect_state.json, .session_fingerprint.json (runtime state, never should have been committed) - Update .gitignore to prevent future commits of runtime files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Delete populate_rag.mjs + populate_rag_v2.mjs (legacy, wrong pricing, dead scalaai.it URLs, no open-source value) - Delete disabled deploy workflow (leaked /home/ale paths, dead PM2 refs) - Remove non-existent FREE tier from waha-bridge.cjs system prompt - Sanitize tests/sara-selftest.sh: replace hardcoded paths with $SARA_HOME Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace static demo.png with animated GIF showing a real conversation: user books a table → SARA calls check_availability → check_allergens → book_table with function calling visible. Shows multi-language switch. Add star-history chart and stronger star CTA. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8-frame animation showing DineOS vertical: table booking with function calling (check_availability, check_allergens, book_table), 400x700px WhatsApp dark mode style. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The LICENSE file held a truncated stub of roughly 1KB against the ~34KB of the actual licence, so GitHub reported the repository as NOASSERTION: it could see a licence file but not identify it. To a legal team an unidentifiable licence reads the same as none at all, which blocks adoption exactly where the project wants it. Text taken verbatim from https://www.gnu.org/licenses/agpl-3.0.txt
* Make the test suite pass on a clean clone, and add CI
`npm test` reported 6 passed / 2 failed for anyone who cloned the repo:
- conversation-memory.test.ts imported vitest, which is not a dependency,
so the file threw on load. It also re-declared its own copies of the
classification regexes and asserted on those, which is why it never
noticed they had drifted from the real ones (missing bravo|stupendo|wow).
Rewritten against node:assert — the runner the rest of the suite uses —
and pointed at the patterns the bot actually runs.
- guardrails.test.ts failed a real assertion: validateOutput() let internal
source paths such as `src/ai.ts` through to the user. Added a LEAK_MARKER
for them; the bot talks to business owners and has no reason to name a
file in this repo.
Those regexes were duplicated verbatim between conversation-memory.ts and
handlers/text.ts, and the two copies had already diverged. Both now import
from lib/text-patterns.ts. Role detection was also running the same 15-branch
regex twice in a row, once to test and once to match.
config.ts no longer falls back to a hardcoded localhost connection string
when DATABASE_URL is unset; index.ts checks it at startup and exits with a
readable message instead. The check cannot live in config.ts, because ESM
evaluates imports before any body statement and the unit tests import it
transitively.
Adds a GitHub Actions workflow running install, typecheck and tests on
Node 20 and 22.
Result: 8 passed, 0 failed. tsc --noEmit clean.
* Fix npm ci on a clean clone
CI caught what local verification had missed. `npm ci` runs a postinstall
that shells out to patch-package, and patches/ still held a patch for
@whiskeysockets/baileys — a package dropped from dependencies in May 2026.
patch-package exits 1 on an orphan patch, so install failed for everyone
who cloned the repository:
Error: Patch file found for package baileys which is not present
at node_modules/@whiskeysockets/baileys
The patch existed for src/wa-hardened-adapter.ts, which index.ts:27 already
labels dead code and which nothing imports. It kept a live import of the
removed package; tsconfig hid that by excluding the file from typechecking
rather than deleting it. Removed both, 583 lines.
Also dropped the postinstall hook, which had nothing left to patch and was
npx-downloading patch-package on every install, and stopped excluding
src/__tests__ from typechecking — the missing vitest dependency fixed in
the previous commit slipped through precisely because the tests were never
type-checked.
Verified on a clean clone with node_modules removed and install scripts
enabled: npm ci exit 0, tsc --noEmit exit 0 (tests now included),
8 passed / 0 failed.
* Add a disposable staging environment, and fix what it found
The staging job builds a whole environment from nothing on every run and
throws it away with the runner: a pgvector service container created empty,
the schema applied from scratch, the API booted against it, and real HTTP
requests made to it. Nothing in it can reach production — that is the point.
It also refuses to run if DATABASE_URL looks like a production host.
It paid for itself before it was even pushed. Two failures on a clean
database, neither visible on a machine where the schema already existed:
- initDB() never created the pgvector extension, while the knowledge table
declares embedding vector(1024). The whole DDL batch failed, and the catch
logged "Table initialization skipped (tables may already exist)" and carried
on. Every statement in that batch is CREATE ... IF NOT EXISTS, so an
existing schema can never be the cause: the message was wrong and the
recovery was worse, leaving the bot running with no tables at all. This is
what anyone cloning the repo and running docker compose up was hitting.
The extension is now created first, and a schema failure throws.
- sara_contact_profiles was missing, so /api/sara/health reported "degraded"
on a fresh install. It is created by ensureMemorySchema(), which only
index.ts called — anything reaching the database through initDB() alone came
up with a partial schema. initDB now calls it too.
crm_contacts stays absent by design and the smoke test treats it as optional:
crm-sync.ts writes into it, but it belongs to the SCALA backend schema and a
self-hosted install without SCALA will not have it. Worth noting that the
health endpoint still calls that "degraded", which reads as broken to someone
evaluating the project — a separate fix.
Verified end to end against a throwaway pgvector container: schema from zero,
migration applied, API listening, health 200 with both required tables ok,
four read endpoints answering 401 rather than 5xx. tsc clean, 8 tests passing.
Il "20" del README non era sbagliato: 21 definizioni in scala-agent-definitions meno `general` fa esattamente 20, ed e il perimetro del PRODOTTO. Sbagliato era il perimetro implicito — chi clona questo repo ottiene il MOTORE, che include 13 insiemi di prompt. Dire 13 sarebbe stato fuorviante nell'altro verso. Il README ora dice entrambe le cose: "20 verticals defined, 13 brains included". Corretti anche i prompt, che dichiaravano 20 verticali e ne elencavano 19: mancava TenderOS (gare d'appalto), presente nelle definizioni pubbliche. Aggiunto in 16 elenchi su 3 file, in tutte e quattro le lingue. Ora ogni elenco che dichiara 20 ne contiene 20 — verificato contandoli. FacilityOS rinominato ServiceOS: era il vecchio nome (vedi il commento in sara-tools.ts:644) e le definizioni pubbliche lo chiamano `service`. Il nome rivolto al cliente ora coincide con quello dello schema aperto. Conteggio strumenti: 79 -> 87, contati nei definitions/*.json. Test invariati: 6 file falliti prima e dopo.
… e ometteva TenderOS Erano 19 verticali reali piu un doppione, presentati come 20. Ora sono 20 distinti.
…ilazione Nel commit precedente avevo inserito "gare d'appalto" nella descrizione di TenderOS, dentro stringhe TypeScript delimitate da apici singoli. L'apostrofo chiudeva la stringa: 40+ errori TS1005/TS1002 in ai.ts e sectors.ts. Sostituito con "gare di appalto", che e corretto in italiano e non dipende dal tipo di virgolette che delimita la stringa. Nota su come mi era sfuggito: avevo dichiarato "test invariati" eseguendo npm test SENZA node_modules installati (il repo non li aveva, 0 pacchetti). Quel confronto non misurava niente. Rifatto con le dipendenze installate: 2 file falliti prima, 2 dopo, gli stessi.
Le otto costanti di prompt (VERTICAL_SYSTEM_PROMPTS + EN/ES/PT,
SECTOR_PROMPTS + EN/ES/PT) erano 1.450 righe di lavoro di dominio dentro il
motore. Erano la parte difficile, mentre il codice intorno — adattatore
WhatsApp, function calling, failover — e la parte che chiunque riscrive.
Ora sono file JSON caricati da pack-loader.ts. Il repo include `general`
(fallback, sempre presente) e `dine` (esempio completo in quattro lingue).
Verificato ESEGUENDO, non leggendo:
pacchetti caricati dine, general + settori general, ristorante
ristorante usa il suo pack 2.295 caratteri, diverso da general
inglese diverso da italiano si
dermatologia (rimosso) ricade su general, nessun errore
con SARA_VERTICAL_PACKS 13 verticali e 12 settori, dermatologia
torna a usare il proprio pacchetto
test 2 file falliti prima e dopo, gli stessi
SECTOR_PROMPTS resta esportata con la stessa forma (ai.ts e dream-cycle.ts
la leggono come oggetto): e un Proxy sui pacchetti caricati.
packs/README.md documenta il formato: e parte del prodotto, non un dettaglio
interno — il valore della proposta e "scrivi il tuo".
Deciso dall'utente. Il vincolo di sequenza era che i prompt verticali fossero ancora nel repo: cambiare licenza prima dell'estrazione li avrebbe regalati. L'estrazione e fatta (cab37fe), quindi il vincolo e sciolto. Non e solo il file LICENSE. Il README diceva 'se distribuisci una versione modificata devi rilasciare il sorgente sotto la stessa licenza' e offriva una licenza commerciale 'per usarlo senza gli obblighi AGPL': sotto Apache sono due affermazioni FALSE, e lasciarle sarebbe stato peggio che non cambiare licenza affatto - chi legge si regola su quelle. Riscritto per dire cosa Apache concede davvero: nessun copyleft, uso in prodotti chiusi, obbligo di conservare LICENSE e NOTICE e di dichiarare le modifiche, concessione di brevetto inclusa. Aggiunto NOTICE, che Apache si aspetta e che il README ora nomina. Aggiunto il campo license in package.json, che non c'era affatto: senza, npm e gli strumenti di analisi non sanno sotto cosa sta il pacchetto. Testo della licenza copiato verbatim da scala-agent-definitions, non riscritto: 193 righe, verificate integre.
I NUMERI. Il README diceva 87 strumenti in tre punti. Il motore ne definisce 83 (contati da SARA_TOOLS: 20 verticali). E il terzo punto, la tabella dei repo in fondo, attribuiva quegli 87 a scala-agent-definitions, che ne ha 80. Tre numeri sbagliati, tutti scritti in buona fede e invecchiati insieme al codice. In un repo open source con poche stelle un numero gonfiato non e un dettaglio estetico: chi valuta il progetto lo conta, e quando non torna smette di fidarsi anche del resto. Il test ora conta gli strumenti veri e confronta ogni occorrenza nel README, escludendo quella che parla dell'altro repo. LA PATCH. patches/@WhiskeySockets+baileys+7.0.0-rc.9.patch faceva questo: - if (Buffer.compare(hmac, advSign) !== 0) { + if (false && Buffer.compare(hmac, advSign) !== 0) { throw new Boom('Invalid identity key signature'); cioe disabilitava la verifica della firma della chiave identita, in un repo PUBBLICO. Ed era orfana: baileys non e fra le dipendenze da maggio 2026 - lo dice il codice stesso in index.ts, 'Baileys adapter REMOVED (May 2026)'. Quindi non proteggeva niente e non serviva a niente: falliva soltanto a ogni installazione ('Patch file found for package baileys which is not present'), e nel frattempo mostrava a chiunque legga il repo come aggirare quel controllo. Rimossa insieme al postinstall che la applicava. npm install ora e pulito: 291 pacchetti, nessun errore. RESTA DA DECIDERE: src/wa-hardened-adapter.ts, 583 righe di codice morto che importano baileys. tsconfig.json lo esclude a mano dalla compilazione, il che e il motivo per cui la build passa. Non l'ho cancellato io - e una scelta, non una correzione.
Unisce apertura/estrazione-verticali in main. L'ordine conta ed e rispettato: i prompt escono e la licenza cambia NELLO STESSO commit, quindi main non si trova mai con Apache addosso mentre contiene ancora le 1.213 righe di vertical-prompts.ts. Se fossero due passaggi separati, in mezzo ci sarebbe una finestra in cui il lavoro di dominio in quattro lingue e regalato. CONFLITTO RISOLTO: LICENSE. main aveva il testo AGPL completo (da8c378), il ramo Apache 2.0. Vince Apache, che e la decisione presa. VERIFICATO SULL'ALBERO FUSO, non sui due rami separatamente: LICENSE Apache 2.0 vertical-prompts.ts 93 righe (erano 1.213) packs/ 4 file — general e dine, piu due settori guardia source_path_leak presente (viene da main) text-patterns.ts presente (viene da main) wa-hardened-adapter.ts assente patches/ vuota NOTICE presente suite 9 file, uscita 0 build compila Le tre cose che sembravano regressioni del ramo non lo erano: guardrails, text-patterns e conversation-memory.test li ha cambiati main, non il ramo, che e semplicemente piu vecchio. Nel merge vince main e infatti restano quelli nuovi. L'ho verificato con git diff dalla base comune invece di confrontare le due punte, che e cio che mi aveva tratto in inganno.
…iorno
E il modulo che decide QUALE strumento eseguire quando il modello lo chiede.
Non aveva un test. Un errore qui non solleva un'eccezione: fa fare a SARA la
cosa sbagliata col cliente.
IL DIFETTO, trovato dal primo test scritto. normalizeDate convertiva con
toISOString(), che e UTC. Su una macchina a fuso positivo, fra mezzanotte e
l'offset, 'oggi' restituiva IERI e 'domani' restituiva OGGI: ogni data gestita
dall'agente slittava di un giorno per un paio d'ore ogni notte. Un tavolo
prenotato all'una per domani finiva sul giorno sbagliato, in silenzio.
Misurato mentre accadeva, non dedotto:
ora locale 00:50, fuso +2
data locale 2026-08-25
normalizeDate('oggi') 2026-08-24 <- ieri
In produzione il server e su Etc/UTC e il difetto non si manifesta. Ma questo
repository e PUBBLICO e auto-ospitabile: chi lo installa in Italia lo prende in
pieno ogni notte. E far dipendere la correttezza dal fuso della macchina, senza
scriverlo da nessuna parte, resta una trappola anche dove per caso funziona.
Le cinque conversioni dentro normalizeDate ora usano i componenti locali della
data. Il resto del file non e stato toccato.
13 PROVE, fra cui due che coprono decisioni di sicurezza:
- uno strumento NON classificato deve valere 'medium', mai 'low'. Il livello
di rischio governa l'autonomia: se un domani qualcuno aggiunge uno strumento
e dimentica di classificarlo, non deve diventare eseguibile in automatico
per omissione.
- notes, category e gli altri sei campi facoltativi non devono mai finire fra
i 'required' dello schema mandato all'LLM: se ci finissero, il modello
inventerebbe un valore pur di riempirli.
878 righe senza un test, e due promesse scritte nel README che nessuno aveva mai verificato. IL FAILOVER. 'Multi-provider LLM chain with automatic failover — zero downtime when one provider is rate-limited'. Se non scatta, SARA si spegne appena Groq risponde 429, che e cio che fa il piano gratuito sotto carico. Verificato con uno strato di rete finto: una sola chiamata quando il primo risponde, passaggio al successivo dopo un 429, attraversamento di tre provider caduti fino al quarto, e con tutti e quattro giu un errore che li NOMINA tutti — senza i nomi, chi guarda i log non distingue un guasto da un rate limit. L'ANONIMIZZAZIONE. 'PII anonymization before every external LLM call'. Se salta, i numeri di telefono dei clienti finiscono nei log di Groq. Il caso che sfugge a occhio e il secondo tentativo: se l'anonimizzazione vivesse dentro il ramo del primo provider, dopo il failover la chiamata partirebbe in chiaro. C'e un test apposta per quello, e regge. Verificato anche che getProviderStatus non esponga le chiavi: finisce in un endpoint di diagnostica, e una chiave in un log e una chiave bruciata. UN MIO ERRORE, lasciato scritto nel test perche e istruttivo. La prima versione cablava il segnaposto '[PHONE_1]', che mi ero inventato: il test falliva accusando il codice di non deanonimizzare. Il formato vero e '[TEL_1]' e il codice funzionava. Ora il segnaposto si CHIEDE all'anonimizzatore invece di scriverlo a mano — un test che cabla il formato di un altro modulo mente appena quel formato cambia.
Questo progetto non ha vitest: i test sono node:assert eseguiti con tsx. Un file che importa vitest non fallisce in modo leggibile — muore con ERR_MODULE_NOT_FOUND prima di eseguire una sola asserzione, e in mezzo all'output di undici file ci si accorge a malapena. Peggio: chi lo scrive lo prova con 'npx vitest', che se lo prende dalla cache di npm, lo vede verde, e conclude che funziona. Passa con uno strumento che nel progetto non esiste. L'ho fatto io ieri, e il file sarebbe finito su main senza girare mai. Ora il runner lo intercetta prima di partire e spiega anche PERCHE sembrava passare, che e la meta del messaggio che mi sarebbe servita. Verificato in entrambi i versi: con un file in stile vitest esce 1, senza esce 0 e gli undici file passano.
1.248 righe senza test. Coperte extractLeadInfo e evaluateRetrieval, che sono le due funzioni pure del modulo e anche quelle in cui un errore e invisibile: nessuna delle due solleva. extractLeadInfo sbaglia in due direzioni opposte e costano entrambe. Se non riconosce un nome si perde l'attribuzione del lead; se lo riconosce dove non c'e, il CRM si riempie di contatti che si chiamano 'Avvocato' o 'Nel'. Cinque prove difendono una DECISIONE, non un comportamento: 'sono' e escluso dal pattern apposta - il commento nel codice dice che causava falsi positivi su 'sono avvocato' e 'sono nel team di Marco'. Senza un test, il primo che vede 'Sono Mario Rossi' non riconosciuto lo aggiunge al pattern e ricrea il problema. Ora quel test spiega perche non va fatto. evaluateRetrieval decide se il RAG ha trovato qualcosa di buono. Il caso che conta e la somiglianza alta su un argomento diverso: 0.9 senza una parola in comune NON deve valere 'correct', altrimenti SARA risponde con sicurezza usando un documento che non c'entra - che e il modo peggiore di sbagliare per un assistente. Verificato che finisca in 'ambiguous'. E verificato che ogni verdetto porti una motivazione: finisce nei log, e senza un verdetto sbagliato non si riesce a spiegare a posteriori.
788 righe senza test. Due funzioni decidono quando iniettare nel contesto del
modello il listino e la documentazione della piattaforma SCALA, e cercavano i
segnali come SOTTOSTRINGA. Con termini corti questo produce falsi positivi che
il cliente vede.
isPricingQuery aveva nell'elenco i numeri nudi '49', '149', '298'. Un numero di
telefono contiene quasi sempre '49': 'il mio numero e +39 349 1234567' faceva
iniettare 26 righe di listino. Erano anche OBSOLETI — i prezzi veri sono 97,
197, 970 e 1970. Aveva anche 'quanto' ('quanto tempo ci vuole'), 'piano'
('siamo al piano terra', 'vai piano') e il simbolo '€', che scattava quando il
ristorante citava i PROPRI prezzi.
isScalaPlatformQuery aveva 'free', 'cost' e 'trial', che vivono dentro parole
comuni: freelance, costume, indus-TRIAL-e.
Misurato, non dedotto: su nove messaggi da ristorante, sei attivavano il
listino e cinque erano falsi positivi.
Il danno non e teorico. Il modello riceveva il listino degli abbonamenti nel
contesto mentre un cliente prenotava un tavolo, e veniva invitato a parlargli
di 97 euro al mese. Piu 26 righe di contesto sprecate a ogni messaggio.
Ora le frasi si cercano come sottostringa, che e giusto, e le parole singole
con i confini di parola. \p{L} e \p{N} invece di \w perche i messaggi sono
anche in spagnolo e portoghese e con \w 'però' spezzerebbe sull'accento.
11 prove, fra cui una che confronta gli importi nei quattro listini: se una
traduzione resta indietro, SARA dice un prezzo diverso a seconda della lingua
del cliente, ed e il difetto piu difficile da vedere perche ognuno dei quattro
testi letto da solo sembra giusto. Oggi i 7 importi coincidono.
618 righe senza test. Coperte buildProfileContext e buildKBContext, che sono le uniche funzioni pure del modulo e anche le uniche il cui risultato viene consegnato TESTUALMENTE al modello: chi e il cliente, e cosa sa l'azienda. Un errore qui non solleva. Produce un blocco leggermente sbagliato che il modello legge come verita, e la conversazione va storta senza che in nessun log compaia niente. 17 prove. Quelle che valgono piu delle altre: - nessun 'null' o 'undefined' nel testo. Un profilo quasi vuoto non deve produrre 'Name: null' davanti al modello: e rumore che il modello puo ripetere al cliente. - il numero di telefono NON e nel profilo. ContactProfile non lo porta, ed e giusto: quel blocco finisce nel prompt e da li puo arrivare a un provider esterno. Il test impedisce che qualcuno lo aggiunga senza accorgersi. - mai 'History: 0 days'. Primo e ultimo contatto coincidenti danno 1 giorno: zero, al modello, suona come 'non l'ho mai sentito'. - gli argomenti sono gli ULTIMI dieci, non i primi. Senza il limite un cliente con centinaia di argomenti riempirebbe il contesto da solo. - il blocco della knowledge base chiude dicendo al modello di ammettere quando non sa. Senza quella riga il modello riempie il vuoto inventando, che per un bot che risponde ai clienti e il difetto peggiore possibile. Le soglie del sentimento sono verificate sui valori ESATTI di confine, 0.7 e 0.3, non solo in mezzo agli intervalli.
Modificando scripts/run-tests.sh da Windows sono finiti dentro i fine riga
CRLF. Il kernel Linux legge lo shebang come 'bash\r', non trova nessun
interprete con quel nome e il processo muore con exit 127.
Nel log appare cosi:
/usr/bin/env: 'bash\r': No such file or directory
che sembra un problema con bash, non con lo script, e manda fuori strada. Il
passo 'Test' falliva prima di eseguire una sola asserzione, e la suite era
verde in locale.
Nessuna riga di logica era cambiata: solo i fine riga.
.gitattributes impedisce che si ripeta, per gli script e per i file che la CI
esegue o legge riga per riga.
Prima non si misurava affatto. In mancanza del numero si citava il rapporto
fra righe di test e righe di codice — 888 su 23.152, cioe 3,8% — come se fosse
copertura. Sono due misure diverse, e quella grezza faceva sembrare il progetto
messo molto peggio di com'e:
Statements 56,63% (5.581/9.854)
Branches 80,04% (361/451)
Functions 45,02% (95/211)
L'80% sui rami dice una cosa precisa: i percorsi decisionali del codice
eseguito sono coperti bene. Il 45% sulle funzioni dice l'altra: ci sono moduli
interi che nessun test tocca ancora.
c8 e non un runner nuovo: la suite resta node:assert con tsx, che e la forma di
questo repo. c8 la osserva da fuori senza cambiarle niente.
La copertura era al 56,63% e il grosso di cio che mancava parlava al database: db.ts al 6,7%, persistent-memory al 27%. Non perche sia codice trascurato — in prova non c'era mai stato un database a cui parlare. L'alternativa era rattoppare il modulo pg con dei finti, ma quei test verificherebbero i finti. Qui il job monta un Postgres vuoto che dura quanto la corsa, e ci gira sopra il codice VERO. ensureMemorySchema() si crea le tabelle da solo, quindi non serve nemmeno una migrazione. 12 prove sul percorso della memoria. Due meritano di essere nominate: - ISOLAMENTO FRA CONTATTI. E il difetto piu grave possibile qui: la conversazione di un cliente che finisce nel contesto di un altro. Il test scrive un messaggio riconoscibile per un secondo numero e verifica che non compaia nella storia del primo. - L'ORDINE CRONOLOGICO. La storia va al modello come contesto: invertita racconta una conversazione che non e mai avvenuta, e il modello risponde di conseguenza. E una che protegge un comportamento facile da rompere: un aggiornamento parziale del profilo non deve cancellare i campi che non nomina. Senza DATABASE_URL il file si SALTA invece di fallire. Un test rosso per assenza di ambiente addestra a ignorare i test rossi, ed e cosi che una suite smette di servire a qualcosa.
…ssandro114#13) Il commit precedente ha lasciato main rossa su due workflow (CI e Staging), entrambi caduti sullo stesso `tsc`: 15 errori TS2554/TS2345 nel file di test che avevo appena aggiunto. La causa non e stata una svista di battitura. Ho scritto le chiamate INDOVINANDO le firme invece di aprire persistent-memory.ts e leggerle. Ogni funzione di quel modulo prende `userId` come PRIMO parametro — e multi-tenant, il telefono da solo non identifica nulla — e io passavo il telefono per primo omettendo del tutto l'utente. upsertContactProfile poi non riceve un profilo gia fatto: riceve il testo del messaggio e ne ricava lingua, sentiment, argomenti e nome. I due test che gli passavano {name, language} non verificavano una funzione diversa da quella che credevo: verificavano una funzione che non esiste. Oltre alle firme, ho dovuto rifare le asserzioni. OGNI funzione di persistent-memory cattura le proprie eccezioni e restituisce un ripiego — [] in lettura, niente in scrittura — perche la memoria non deve mai far cadere un messaggio di WhatsApp. E una scelta giusta per la produzione e una trappola per i test: `assert.ok(Array.isArray(r))` passa identico con la query sana e con la query rotta. Tre test su sedici erano esattamente cosi, verde garantito a prescindere. Ora ognuno controlla un dato che puo esistere solo se la query ha funzionato: la ricerca semina una voce e pretende di ritrovarla, il messaggio lungo viene riletto dal database, lo schema viene contato in pg_tables invece di fidarsi che ensureMemorySchema non abbia solo loggato un warning. Due test guardano la cosa piu grave che possa succedere qui, la conversazione di un cliente che finisce nel contesto di un altro: uno cambia telefono a parita di utente, l'altro cambia utente a parita di telefono. Il secondo e quello che vedrebbe una WHERE a cui manca user_id. I riempitivi sono saliti da 6 a 25 perche getConversationSummary sotto i 20 messaggi esce subito: con 6 si copriva il return, non la funzione. Verificato: tsc --noEmit pulito, 15/15 file della suite passano in locale. I sedici test del database in locale si SALTANO (nessun Postgres su questa macchina, ne docker), quindi non li ho ancora visti passare: e il motivo per cui questo va su un ramo e non su main. Si fonde a CI verde, non prima.
* db.ts era al 6,7%: 25 test contro un Postgres vero
Il file meno coperto del repo, e non perche sia trascurato — ci passa ogni
messaggio che il bot riceve — ma perche in prova non c'e mai stato un database
a cui parlare. Ora in CI c'e, quindi si prova.
Coperto: initDB (anche la seconda chiamata, che il bot fa a ogni avvio),
sessioni in creazione e in aggiornamento, i rami booleani che il codice scrive
a parte (cta_shown, lead_score, opted_out), lo storico con la traduzione
in→user / out→model e l'ordine cronologico, il filtro sui media, gli ultimi
argomenti col taglio a 100 caratteri, le tre soglie del punteggio, i solleciti
programmati/annullati/inviati con la JOIN su wa_sessions.
Tre test guardano cose che, se si rompessero, si romperebbero in silenzio:
- opted_out deve scrivere anche opted_out_at. E il momento in cui una persona
chiede di non essere piu contattata, e la data serve a dimostrare quando.
- un contatto gia 'converted' non deve tornare 'engaged' perche il punteggio
si muove: sarebbe un cliente che riceve i richiami da lead da coltivare.
- lo storico di un numero non deve contenere i messaggi di un altro.
A differenza di persistent-memory, db.ts NON inghiotte gli errori: solleva. E
la scelta giusta — una sessione che non si salva e un guasto, non un dettaglio
— e rende questi test onesti senza sforzo: se una query e rotta il test
esplode, invece di ricevere un [] silenzioso.
─────────────────────────────────────
Due correzioni trovate scrivendoli:
L'immagine del Postgres in CI passa da postgres:15 a pgvector/pgvector:pg15.
initDB() apre con CREATE EXTENSION vector, perche la tabella della conoscenza
tiene gli embedding come vector(1024): con l'immagine semplice fallisce alla
prima riga. E' esattamente il caso che il messaggio d'errore di db.ts spiega
("Use the pgvector/pgvector image"), e nessun test lo aveva mai raggiunto.
E c'era un console.log dopo il `throw err` in initDB: irraggiungibile, e
diceva "Bot will continue with existing schema", cioe il contrario di quello
che il throw fa. Rimasuglio della versione che inghiottiva l'errore, quando il
messaggio era anche vero. Rimosso.
* Lo stadio del lead si calcolava sul doppio del punteggio
Trovato dal test appena scritto, al primo giro in CI: "10 punti non devono
bastare" e invece bastavano.
await pool.query('UPDATE wa_sessions SET lead_score = GREATEST(0, lead_score + $1) ...');
const session = await getSession(phone);
const score = (session.lead_score || 0) + delta; // <— il delta due volte
getSession gira DOPO l'UPDATE, quindi session.lead_score contiene gia il
delta. Sommarlo di nuovo faceva calcolare lo stadio sul doppio del punteggio
vero, e le due soglie scattavano a meta: 'engaged' a 10 punti invece di 20,
'qualified' a 25 invece di 50.
Il punteggio salvato in tabella era giusto. Sbagliato era solo lo stadio — che
pero e il campo su cui si decide chi contattare, e da cui parte
syncLeadToSCALA quando un contatto arriva a 'qualified'. In pratica meta dei
lead venivano spinti al CRM come qualificati senza esserlo, e chi guardava il
punteggio accanto allo stadio vedeva due numeri che non tornavano.
Difetto invisibile senza database: e in una query che nessun test aveva mai
eseguito.
Aggiunto testPunteggioSoglieAlBordo, che pianta i bordi esatti — 19 → new,
20 → engaged, 49 → engaged, 50 → qualified. Le soglie sono `>=` e un
fuori-di-uno qui sposta chi viene contattato. Con il difetto in piedi
falliscono entrambi i bordi, quindi funziona anche da rete contro il ritorno.
…ro114#15) Rischio introdotto da me stamattina. memoria-db.test.ts e db-sessioni.test.ts si connettono a DATABASE_URL e fanno INSERT, DELETE e — attraverso initDB() — CREATE TABLE e ALTER TABLE. Finche i test erano tutti in memoria non c'era niente da proteggere; da quando parlano a un database, lanciarli nell'ambiente sbagliato scrive nell'ambiente sbagliato. Su 89 DATABASE_URL e esportato nell'ambiente della shell e punta alla PRODUZIONE. Un `bash scripts/run-tests.sh` dato per abitudine prima di un commit ci eseguirebbe sopra della DDL. Il guard sta nella SHELL e non in un file di setup come su scala-backend, perche questo repo non usa vitest: ogni test e un processo tsx a se, e non esiste un punto comune dentro Node in cui mettersi. La shell e quel punto. E il motivo per cui la protezione di scala-backend non si trasferisce qui copiandola. Due regole: 1. Un DSN che PUZZA di produzione (scalacore, scala-postgres, gli IP di 89 e 65) fa fallire subito, anche in CI, e non c'e opzione per aggirarlo. Se serve provare contro quei dati si copia il database, non si toglie il controllo. 2. Fuori dalla CI un DSN qualunque viene IGNORATO se non lo si e chiesto esplicitamente con SARA_TEST_DB=1. I test del database si saltano da soli e lo dicono. Meglio saltarli che scoprire dopo dove hanno scritto. Verificato eseguendolo, non ragionandoci: - DSN con l'IP di 89 -> RIFIUTO, uscita 1 - DSN con host scala-postgres -> RIFIUTO, uscita 1 - DSN locale sconosciuto -> avviso, variabile tolta, test saltati - nessun DSN -> 16 file passati, come prima Segnalato da ssh-manager-ea, che aveva visto il rischio su whatsapp-bot e sara. Su whatsapp-bot non intervengo io: e privato e non e in carico a me.
…ro114#16) Elencava `mxbai-embed-large` e `jina-embeddings-v3` come modelli supportati a 1024 dimensioni, e ometteva `bge-m3` — che e quello su cui la produzione e migrata il 25/08. Chi seguiva il README ne sceglieva uno dei due, e nessuno dei due e quello che usiamo. Aggiunto, ma la parte che vale di piu e l'avvertimento che non c'era. Il README metteva in guardia dal caso SBAGLIATO. Diceva di non usare `nomic-embed-text` perche e a 768 dimensioni — ed e vero, ma quello e il caso innocuo: pgvector rifiuta l'inserimento al primo chunk e te ne accorgi subito. Il caso pericoloso e l'opposto: due modelli DIVERSI che producono la STESSA dimensione. Tutti e tre i modelli supportati emettono 1024d, quindi il database accetta vettori di uno e dell'altro nella stessa colonna senza un solo errore. Ma sono spazi vettoriali diversi, e la distanza fra un vettore dell'uno e uno dell'altro non significa niente. Un indice mescolato non da nessun sintomo: restituisce risultati plausibili e casuali. Con un indice HNSW e peggio che impreciso, perche i vettori del modello in minoranza diventano IRRAGGIUNGIBILI, non solo mal ordinati. Non e teoria: nel nostro impianto un indice da ~50.000 chunk `jina-embeddings-v3` si e ritrovato dentro 11 chunk `mxbai-embed-large`, scritti da una seconda pipeline che usava un altro modello. Non e fallito niente. Il README ora dice anche cosa fare: ri-vettorializzare TUTTO il corpus a ogni cambio di modello, mai in modo incrementale, e tenere il nome del modello su ogni riga — senza quella colonna un indice mescolato e indistinguibile da uno sano, ed e la query che ha rivelato il nostro. Vale per chiunque usi questo repo, non solo per noi: e la trappola in cui finisce chiunque cambi modello di embedding senza saperlo.
Alessandro114#17) * Avevo messo dettagli della nostra infrastruttura in un README pubblico Nel commit precedente ho scritto, nel README di sara: "In our own deployment an index of ~50k jina-embeddings-v3 chunks silently acquired 11 mxbai-embed-large ones, written by a second pipeline." E un repo PUBBLICO, e quella frase dice quanto e grande il nostro indice, quali modelli usavamo e che abbiamo due pipeline che scrivono nella stessa tabella. Sono dettagli operativi nostri, e per giunta riguardano internal-rag — un sistema che non e nemmeno quello che questo README documenta. L'ha notato l'utente, non io. Stavo cercando di rendere l'avvertimento credibile citando un caso vero, e il caso vero era nostro. Tolto. L'avvertimento resta e non perde niente: che due modelli con la stessa dimensione siano mescolabili in silenzio discende da come funziona pgvector, e non ha bisogno di un aneddoto per essere vero. Al suo posto c'e la descrizione generica dello scenario — un secondo processo che scrive nella stessa tabella con un'altra impostazione di modello — che e piu utile a chi legge, perche gli dice dove guardare invece di raccontargli cosa e successo a noi. Regola: un README pubblico documenta il progetto, non il nostro impianto. Se un esempio ha bisogno di dati nostri per stare in piedi, non e un esempio: e una divulgazione. * Il guard pubblicava gli IP dei server di produzione. Riscritto al contrario Il controllo che ho aggiunto stamattina conteneva questa riga, in un repo PUBBLICO: PRODUZIONE='scalacore|scala-postgres|<IP di 89>|<IP di 65>' Cioe: un file scritto per proteggere i server di produzione ne pubblicava gli indirizzi. Ed erano indirizzi che NON erano scopribili: il DNS di get-scala.com e app.get-scala.com risolve su Cloudflare, quindi le origini sono nascoste apposta. Li ho esposti io. ─── La riparazione, e perche e anche piu robusta ─── Il rimedio non e nascondere meglio l'elenco: e che quel controllo non ha bisogno di nominarli. Ora invece di VIETARE gli host noti, AMMETTE solo quelli locali — localhost, 127.0.0.1, ::1, e i nomi di servizio tipici di un compose. Qualunque host remoto viene rifiutato. E piu stretto, non solo piu discreto: - prima proteggeva solo da cio che qualcuno si era ricordato di elencare. Un database di produzione nuovo, quello di un cliente, o un indirizzo che nessuno aveva previsto passavano lisci. - adesso passa solo il locale. Verificato: un IP a caso mai elencato (203.0.113.9) viene rifiutato, e `scala-postgres` pure, pur non essendo piu scritto da nessuna parte. Nessun indirizzo da tenere aggiornato, nessun indirizzo da divulgare. ─── Sull'esposizione ─── Circa 90 minuti fra la pubblicazione e questa correzione. Il repo e pubblico e ha un fork creato prima del commit: le reti di fork condividono gli oggetti git, quindi riscrivere la storia NON ritirerebbe niente — e la stessa cosa che ho detto oggi a un collega a proposito di un force-push su un altro repo, e vale identica quando l'errore e mio. Verificato dopo la correzione: nessun riscontro per gli IP, per `scalacore` o per `scala-postgres` in nessun file di questo repo.
staging.yml passa da Node 20 a 22, come gli altri repo. ci.yml resta com'e, e non e una svista. Usa una matrice ['20','22'], quindi prova gia entrambe le versioni. Toglierne il 20 non sarebbe un dettaglio di cutover: sarebbe dichiarare che sara non supporta piu Node 20 — e sara e un repo PUBBLICO, quindi quella riga e una promessa a chi lo usa, non una nostra impostazione interna. Chi ha installato sara su Node 20 continua a vedere il proprio ambiente provato a ogni giro. Se e ora di smettere, e una decisione di prodotto e la prende l'utente. Nel frattempo la matrice fa una cosa utile: e la sola CI che dimostra che il codice regge su ENTRAMBE, mentre altrove il cutover e una sostituzione secca.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a standard
GET /healthendpoint to S.A.R.A. and integrates healthchecks into Dockerfile and Docker Compose. This allows container orchestrators (Docker Compose, Kubernetes, ECS) and monitoring tools to monitor S.A.R.A.'s uptime, database connectivity, and WAHA WhatsApp bridge status.Changes
GET /health):status(ok|degraded|down),uptimein seconds,bot,version,wahaconnection status, individual table checks (checks), andtimestamp.X-SARA-API-KEY.checkWahaHealth()insrc/lib/multi-session.tsandsrc/lib/multi-session-waha.tswith a 3s timeout to safely inspect WAHA availability without crashing or blocking.DockerfileHEALTHCHECKcommand to probehttp://127.0.0.1:3006/health.healthcheckdefinition todocker-compose.ymlfor thesaraservice.src/__tests__/health.test.tscovering response contracts and offline fallback handling.README.mdandstaging-smoke.mjsto target/health.Verification
checkWahaHealth()returns{ status: 'disconnected', reachable: false }gracefully when WAHA is offline./health.