fix(engine,db,template,settings): reliability + correctness hardening#364
Merged
Conversation
…ck, read-only settings, cross-DB helper docs - baileys: tear down the previous socket (detach listeners + end) before an internal reconnect overwrites it, preventing a per-drop WebSocket/listener leak - engine plugins: also receive the engine config blob at construction so a session still gets operator config if a plugin fails to enable before onLoad runs - settings: PUT is read-only (env-derived); return 501 instead of a false-success 200 - cross-DB column-type/date helpers: document data-connection-only intent + lock behavior with characterization tests
- composite unique index (sessionId, name) via entity decorator + a hand-authored
migration that losslessly de-duplicates pre-existing collisions (keeps the earliest,
renames the rest to <name>-dup-<id>) before creating the index
- resolve-by-name is now deterministic (order by createdAt ASC)
- create/update translate a duplicate-name constraint violation into 409 Conflict
Note: the dual {{var}}/{var} placeholder syntax is a separate, breaking change and stays
tracked in #69; only the deterministic-uniqueness half is addressed here.
The app runs the main connection as a separate always-SQLite connection, but the default CLI DataSource only managed the data connection's migrations — so the main-owned migrations (migrations-main) could not be run/generated from the CLI, which matters when boot auto-migration is disabled (MAIN_DATABASE_SYNCHRONIZE=false). Adds src/database/data-source-main.ts + migration:*:main npm scripts (purely additive; data-source.ts unchanged). Documents the command where MAIN_DATABASE_SYNCHRONIZE is set.
message_create fires for both phone-composed and API-originated sends; the latter are already persisted by the REST send path, so mirroring here would double-persist. Documents the intentional omission and adds a contract-lock test so it can't change silently without the unique-index + dedup work that safe persistence would require.
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
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
A batch of small, self-contained reliability and correctness fixes across the engine, template, settings, and database layers. No functional change for a correctly-configured deployment except one honest contract correction (see Behavior change below). Each change is source-traced, TDD'd, and adversarially reviewed; the full backend + dashboard gate is green (903 tests).
Changes
Engine
connection.updatelistener is detached beforeend()(Baileys'end()emits a synthetic close that would otherwise schedule a spurious second reconnect).createEnginestill appliessessionDataPath/executablePath/authDirifenablePluginfails beforeonLoadruns (previously those silently dropped to defaults). The healthy path still prefers theonLoad-set context (which carries any persisted-override merge).Templates
(sessionId, name)is added via the entity and a hand-authored migration that losslessly de-duplicates any pre-existing collisions first (keeps the earliest, renames the rest to<name>-dup-<id>— never deletes a row). Resolve-by-name is now deterministic (ORDER BY createdAt ASC), and a duplicate name on create/update returns 409 Conflict. (The dual{{var}}/{var}placeholder syntax is a separate, breaking change and remains tracked in message template support #69.)Database
src/database/data-source-main.ts+migration:*:mainnpm scripts (purely additive;data-source.tsis unchanged). Matters whenMAIN_DATABASE_SYNCHRONIZE=false.Settings (behavior change)
PUT /settingsis now honest. Settings are environment-derived and consumed at boot, andConfigServiceis immutable at runtime, so the old handler mutated an in-memory copy and returned200 'updated'while persisting/applying nothing — a false success. It now returns 501 Not Implemented with a clear message. The ADMIN guard andGET /settingsare unchanged; the dashboard has no caller of this endpoint.Hardening / docs
message_createalso fires for API sends, which the REST path already persists, so saving here would double-persist; safe persistence needs a unique-index + dedup and is deferred.Behavior change
PUT /settingsreturns 501 instead of200. The endpoint never actually applied changes, and nothing in the dashboard calls it, so no working flow regresses — but it is a response-status change worth a changelog note.Testing