11 Commits (feature/pgsql-native-backend-protocol)

Author	SHA1	Message	Date
Rene Cannao	b8cfa32c63	fix: address all critical and important code review findings (issue #5729) ... Critical fixes: - Move db_kind to end of ProxySQL_PluginRuntimeView (tail-append = free ABI-3 compat) - Add mutex to protect Anomaly_Detector::user_statistics from concurrent session threads - Replace system(mkdir -p) with pure mkdir() loop (command injection fix) - Replace 77 std::stoi/stoll with non-throwing parse_int_or_zero/parse_ll_or_zero - Fix proxysql.cnf filename: Mysqlx -> MySQLX - Add --whole-archive to non-ClickHouse link path (plugin symbol resolution) Important fixes: - Add default: case with warning to db_kind switch in ProxySQL_PluginManager.cpp - Add config_db dispatch and null-handle tests to plugin_runtime_views_unit-t (6 new tests) - Update CLAUDE.md with sqlite-vec PROXYSQLGENAI exception - Remove stale anomaly_detection-t Makefile target - Replace fixed 1024-byte buffer with dynamic sqlite3_mprintf in Anomaly_Detector Minor: update ABI.md, PLUGIN_API.md, stale comments, unit test plan counts	2 months ago
Rene Cannao	97649b5005	test: update genai plugin load unit test for 3-handle refresh (issue #5729)	2 months ago
Rene Cannao	967ab401ec	genai: fix lifecycle reloads and drop dead MCP stats registration ... Load GenAI and MCP variables before runtime initialization, reinitialize the AI stack on reload, and make the anomaly hook obey the configured enable flags. Restore the missing MCP command aliases and the disk/config reload verbs, fix the MySQLX packaging filename mismatch, and update the unit harness for the plugin ABI. Also remove the stale stats_mcp_* registrations from the plugin surface. There is no plugin-side writer for those tables in this branch, so advertising them as live schema was misleading. Update the unit test and bootstrap comments to match the actual exposed surface.	2 months ago
Rene Cannao	978a6f8d2f	fix(plugins/genai): blocker fixes from PR-#5701 review ... Five concrete fixes uncovered by the four-agent review (architecture, code-safety, build/packaging, test coverage): # Blocker 1: per-row INSERT failure leaves transaction half-done In the six MCP_Threads_Handler methods that follow the BEGIN -> DELETE -> prepare -> loop INSERT -> COMMIT pattern, the SAFE_SQLITE3_STEP2 macro inside the loop didn't check `rc`. If a single INSERT failed (constraint violation, disk full, ...), the loop would continue binding subsequent rows and the COMMIT below would either succeed with partial data or be left dangling. Hoist the per-row binding into three insert_*_row(stmt, row) -> bool helpers in an anonymous namespace, then have every caller check the return value and `ROLLBACK; return false/return;` on failure. Affects save_auth_profiles_to_admin_table, save_target_profiles_to _admin_table, save_query_rules_to_admin_table, project_auth_profiles _to_runtime_view, project_target_profiles_to_runtime_view, project_query_rules_to_runtime_view, plus the new save_profiles_to _admin_table below. # Blocker 2: cross-table install/save atomicity for profiles target.auth_profile_id is an FK to auth.auth_profile_id. The old mcp_load_target_auth_map_from_admindb / mcp_save_target_auth_map_to_ admindb wrappers ran the two installs / two saves as separate operations under separate locks / transactions. A failure of the second step (or a concurrent LOAD/SAVE racing in between) left target_auth_map rebuilt from a (auth_v2, target_v1) mismatch, or left main.mcp_target_profiles referencing auth_profile_ids that didn't exist in main.mcp_auth_profiles. Add atomic combined methods: - install_profiles_from_admin: reads both source tables BEFORE taking the wrlock (so a slow admindb doesn't gate the listener), then under one wrlock swaps both vectors and rebuilds the joined target_auth_map exactly once. - save_profiles_to_admin_table: snapshots both vectors under one rdlock, then deletes target before auth (FK direction-aware) and inserts auth before target inside a single BEGIN ... COMMIT. The plugin_main.cpp wrappers now route through these atomic methods. The per-table install_*_from_admin / save_*_to_admin_table public methods stay (used by future granular admin verbs and unit tests), but each rebuilds target_auth_map after its own swap as before. # Blocker 3: unit-test PROXYSQLGENAI autodetect probe test/tap/tests/unit/Makefile had been probing libproxysql.a for the `MCP_Tool_Handler` symbol to autodetect PROXYSQLGENAI. Step 7 of the carve-out moved that symbol to plugins/genai/, so the probe silently found 0 matches and stopped defining -DPROXYSQLGENAI for unit-test builds. Switch the probe to invoke_register_schemas_phase (the chassis-exclusive symbol that's already used to probe PROXYSQL40) so the macro auto-defines whenever the chassis is available — which matches when genai-plugin unit tests can actually be built and exercised. # Blocker 4: triplet coverage through dispatch surface Extend genai_plugin_load_unit-t to drive the full install/save/project flow through the production dispatch path: - seed mcp_<X> tables in admindb - dispatch "LOAD MCP PROFILES TO RUNTIME" via mgr.dispatch_admin_ command (same path admin SQL takes) - manually invoke refresh_runtime_views_for_query (the chassis hook that fires on every admin SELECT) and assert runtime_mcp_<X> rows match the source - mutate main.<X> directly to simulate an operator stomp, then dispatch "SAVE MCP PROFILES TO MEMORY" and assert the in-memory snapshot was written back over the stomp This is the end-to-end coverage that plugin_runtime_views_unit-t can't provide (it uses synthetic callbacks) and that a standalone genai_mcp_config_store unit test would have provided at the cost of stubbing 9 plugin classes. Plan bumped from 25 -> 36. # Should-fix: atomic fence + GloGATH lifetime genai_anomaly_embed_fn was std::atomic with relaxed ordering; the comment claimed nothing else needed synchronising. Wrong: the embed function dereferences GloGATH, a non-atomic global mutated by genai_init / genai_stop. Pair the load with acquire and the stores with release so a reader that observes the non-null pointer also observes the prior `GloGATH = new ...`, and a reader that observes the post-stop nullptr short-circuits before GloGATH's deletion is visible. Acquire/release is free on x86_64 / arm64 — no perf regression. # Should-fix: harden DEB awk substitution + RPM glob DEB entrypoint: replace the inline awk -v repl="$..." substitution (which mangled paths containing awk metacharacters via gsub) with a file-backed awk getline that reads the replacement block verbatim. Add a fail-fast grep for the placeholder after substitution so a silently-broken substitution can't ship a malformed package. RPM (rhel + suse): the spec gates `/usr/lib/proxysql/*.so` on %if 0%{?with_plugins}, but rpmbuild aborts on "File not found by glob" if the directory is empty (silent plugin build failure + with_plugins=1). Have the entrypoint compgen-check whether any .so actually got staged and only pass --define "with_plugins 1" when true.	2 months ago
Rene Cannao	b840468cd5	test(unit): align genai_plugin_load + register plugin_runtime_views ... Two unit-test-side fixes uncovered by running unit-tests-g1 against the new ABI-3 / Phase-B plugin shape: # genai_plugin_load_unit-t * setup_admindb_schema was using a pre-Step-4.G shape for the mcp_auth_profiles / mcp_target_profiles tables — the new install_*_from_admin reads the full canonical column set (use_ssl, ssl_mode, comment for auth; description, comment for targets) and was failing with `no such column: use_ssl`. Fixture now mirrors the DDL in include/ProxySQL_Admin_Tables_Definitions.h and also creates mcp_query_rules + runtime_mcp_query_rules so the newly-added install_query_rules_from_admin path also has tables to read. * As of Step 4.G the genai plugin publishes its admin/config/stats table set via genai_register_schemas (Phase B), not init. The test was calling load → init_all → start_all and the table-count assertions saw zero rows because Phase B never fired. Add an explicit invoke_register_schemas_phase between load and init_all (matches the order ProxySQL itself uses at startup) and bump the plan from 25 to 26 to cover the new ok(). # Makefile UNIT_TESTS list plugin_runtime_views_unit-t.cpp lives in test/tap/tests/unit/ and exercises the chassis ABI-3 register_runtime_view dispatch surface that landed in PR #5688. It had a build rule but wasn't in UNIT_TESTS, so the runner reported "(not-found)" for it. Wire it in alongside the other plugin-chassis unit tests under PROXYSQL40.	2 months ago
Rene Cannao	9078fd3150	feat(plugins/genai): Step 4.G — register MCP admin tables via plugin ABI ... Moves MCP admin / config / stats SQLite table registration from lib/Admin_Bootstrap.cpp into the plugin via the chassis register_table service. Pure refactor: the same DDL strings produce the same SQLite tables in the same SQLite DBs; only the registration path changes. New plugins/genai/src/plugin_tables.cpp registers 14 tables: admin DB (6) mcp_query_rules, runtime_mcp_query_rules mcp_auth_profiles, runtime_mcp_auth_profiles mcp_target_profiles, runtime_mcp_target_profiles config DB (3) mcp_query_rules, mcp_auth_profiles, mcp_target_profiles (no runtime_* in config; matches the pre-4.G layout) stats DB (5) stats_mcp_query_tools_counters, _reset stats_mcp_query_digest, _reset stats_mcp_query_rules The DDL strings still live in include/ProxySQL_Admin_Tables_Definitions.h for now — plugin_tables.cpp #includes the header. Migrating the macros into a plugin-private header is a follow-up gated on no remaining core caller of the MCP table macros (Step 7 territory). Core changes (lib/Admin_Bootstrap.cpp): - Deleted the 9 admin/config insert_into_tables_defs() entries inside `#ifdef PROXYSQLGENAI` for mcp_*. - Deleted the 5 stats insert_into_tables_defs() entries similarly. - Replaced both blocks with a one-line comment pointing at the new plugin owner. Plugin lifecycle: - genai_init() now calls genai_register_admin_tables() right after genai_register_admin_commands(). Both run during chassis Phase C (init), before admin module bootstrap so the chassis dispatcher merges plugin tables into the admin DB schema before any DML. Test extension (test/tap/tests/unit/genai_plugin_load_unit-t.cpp): - Updated the "skeleton registers no tables (yet)" assertions to expect 6 admin / 3 config / 5 stats — matching plugin_tables.cpp. - 25/25 tests passing. What this changes for operators (no-op): - Same SQL: SHOW TABLES still shows mcp_*, runtime_mcp_*, stats_mcp_*. - Same DDL: column lists, primary keys, defaults — all preserved. - Same admindb / configdb / statsdb attachment.	2 months ago
Rene Cannao	f90307ad75	feat(plugins/genai): Step 4.F — register LOAD/SAVE MCP QUERY RULES TO/FROM RUNTIME ... Adds the last LOAD/SAVE pair the pre-4.C admin SQL surface had: the MCP query rules cache held by Query_Tool_Handler's Discovery_Schema catalog. New helpers in plugins/genai/src/plugin_main.cpp: mcp_load_query_rules_to_runtime(ctx) SELECT * FROM main.mcp_query_rules WHERE active=1 ORDER BY rule_id → catalog->load_mcp_query_rules(rs). Mirrors the pre-4.C ProxySQL_Admin::load_mcp_query_rules_to_runtime. mcp_save_query_rules_from_runtime(ctx, runtime) Pulls catalog->get_mcp_query_rules() and REPLACEs the rows in main.mcp_query_rules (or runtime_mcp_query_rules when runtime=true). Mirrors the pre-4.C ProxySQL_Admin::save_mcp_query_rules_from_runtime. Both helpers fail cleanly (return false, log to stderr) when the MCP listener isn't running — there's no in-memory Query_Tool_Handler cache to read or write. Pre-4.C behaviour was to attempt a load_mcp_server() self-recovery in that case; we no longer have that shortcut from outside the listener lifecycle, so callers get a clear error instead of an opaque retry. New verbs registered in plugin_commands.cpp: LOAD MCP QUERY RULES TO RUNTIME aliases: FROM MEMORY, FROM MEM, TO RUN SAVE MCP QUERY RULES TO MEMORY aliases: TO MEM, FROM RUNTIME, FROM RUN Required new core-side includes in plugin_main.cpp for the Discovery_Schema and Query_Tool_Handler types these helpers traffic in. The .so already linked against them via the chassis --whole-archive trick on libproxysql.a; this is just a header dependency. Test extension (test/tap/tests/unit/genai_plugin_load_unit-t.cpp): - 6 new assertions for the QUERY RULES canonical + 5 aliases. - 25/25 tests passing. Carve-out scoreboard, runtime reconfig surface restored: LOAD MCP VARIABLES TO RUNTIME / FROM MEMORY / FROM MEM / TO RUN LOAD MCP PROFILES TO RUNTIME / FROM MEMORY / FROM MEM / TO RUN LOAD MCP QUERY RULES TO RUNTIME / FROM MEMORY / FROM MEM / TO RUN SAVE MCP VARIABLES TO MEMORY / TO MEM / FROM RUNTIME / FROM RUN SAVE MCP QUERY RULES TO MEMORY / TO MEM / FROM RUNTIME / FROM RUN Still pending: SET mcp-X=Y (chassis ABI gap, separate enhancement).	2 months ago
Rene Cannao	7a4f08416e	feat(plugins/genai): Step 4.F — register SAVE MCP VARIABLES TO MEMORY ... Adds the SAVE direction (runtime → main.global_variables) to the plugin's admin SQL surface, mirroring the LOAD direction wired in the previous commit. New helper plugins/genai/src/plugin_main.cpp::mcp_save_variables_to_admindb Walks ctx.mcp->get_variables_list(), reads each runtime value via ctx.mcp->get_variable, and REPLACEs the corresponding "mcp-<name>" rows in main.global_variables. Owns its own MCP_Threads_Handler wrlock so callers don't have to coordinate. Mirrors the pre-4.C flush_mcp_variables___runtime_to_database with runtime=false (no runtime_global_variables view write). New verb registered in plugins/genai/src/plugin_commands.cpp: SAVE MCP VARIABLES TO MEMORY (canonical) aliases: SAVE MCP VARIABLES TO MEM SAVE MCP VARIABLES FROM RUNTIME SAVE MCP VARIABLES FROM RUN Same alias convention as the LOAD verbs and as plugins/mysqlx. The on-disk write ("SAVE MCP VARIABLES TO DISK") stays in core's admin_handler_command_load_or_save — it's a pure SQL DML that copies main.* to disk.* and doesn't need plugin runtime state. Test extension (test/tap/tests/unit/genai_plugin_load_unit-t.cpp): - 4 new TAP assertions for the SAVE canonical + 3 aliases. - 19/19 tests passing. Carve-out scoreboard, runtime reconfig surface restored: LOAD MCP VARIABLES TO RUNTIME / FROM MEMORY / FROM MEM / TO RUN LOAD MCP PROFILES TO RUNTIME / FROM MEMORY / FROM MEM / TO RUN SAVE MCP VARIABLES TO MEMORY / TO MEM / FROM RUNTIME / FROM RUN Still pending: - SET mcp-X=Y (chassis ABI lacks per-variable namespace registration; would need a new ABI surface). - LOAD/SAVE MCP QUERY RULES TO/FROM RUNTIME (Query_Tool_Handler in plugin owns the runtime cache; helpers come in next commit).	2 months ago
Rene Cannao	ae13958995	feat(plugins/genai): Step 4.F (continued) — register LOAD MCP {VARIABLES,PROFILES} TO RUNTIME ... Adds plugins/genai/src/plugin_commands.cpp registering two admin SQL verbs through the chassis command registry, restoring runtime reconfiguration that 4.C disabled. Verbs registered (each with FROM MEMORY / FROM MEM / TO RUN aliases): LOAD MCP VARIABLES TO RUNTIME -> mcp_load_variables_from_admindb() -> mcp_start_listener_if_enabled() Re-pushes mcp-* values from main.global_variables into the running MCP_Threads_Handler, then re-evaluates the listener state (start if newly enabled). Replaces the FIXME-stubbed ProxySQL_Admin::flush_mcp_variables___database_to_runtime path. LOAD MCP PROFILES TO RUNTIME -> mcp_load_target_auth_map_from_admindb() Refreshes runtime_mcp_auth_profiles / runtime_mcp_target_profiles from main.* and rebuilds the in-memory target_auth_map. Replaces the FIXME-stubbed init_mcp_variables auth-map block. The chassis dispatcher in lib/Admin_Handler.cpp:3987 runs BEFORE admin_handler_command_load_or_save, so any admin SQL matching a registered plugin verb routes to the plugin and the (now no-op) hardcoded core handlers never fire. No core changes needed. What's still stubbed (deferred to follow-up commits): - SAVE MCP VARIABLES TO MEMORY / FROM RUNTIME (needs runtime -> admindb push in plugin) - SAVE MCP PROFILES TO DISK (writes runtime view back to disk.*) - LOAD/SAVE MCP QUERY RULES (Query_Tool_Handler in plugin owns them, but the runtime->admindb push isn't wired through plugin yet) - SET mcp-X=Y (chassis ABI has no register_variable_namespace; would need either a new ABI surface or per-variable register_command — defer to a chassis enhancement) Refactor: moved mcp_load_variables_from_admindb, mcp_load_target_auth_map_from_admindb, mcp_start_listener_if_enabled out of the anonymous namespace in plugin_main.cpp so plugin_commands.cpp can call them. Forward declarations added to genai_plugin.h. Test extension (test/tap/tests/unit/genai_plugin_load_unit-t.cpp): - 7 new TAP assertions verifying canonical SQL and aliases resolve through the plugin manager's resolve_alias_to_canonical, which is the same path admin SQL dispatch uses. - 15/15 tests passing.	2 months ago
Rene Cannao	84216c6670	feat(plugins/genai): Step 4.F (MVP) — restore MCP listener auto-start ... Step 4.C left the MCP listener inert: the admin-SQL surface that used to drive load_mcp_server() was stubbed in core, so no path existed from "mcp_enabled=true in admin DB" to "ProxySQL_MCP_Server is up". This commit restores the startup path entirely from inside the plugin, so the listener auto-starts again whenever the operator has it enabled in admin tables — which is the 80% case. genai_start() (plugins/genai/src/plugin_main.cpp) now does, in order: 1. mcp_load_variables_from_admindb(ctx) SELECT variable_name, variable_value FROM main.global_variables WHERE variable_name LIKE 'mcp-%'. For each row, strip the 'mcp-' prefix and call ctx.mcp->set_variable(name, value). Mirrors the pre-4.C flush_mcp_variables___database_to_runtime in lib/Admin_FlushVariables.cpp. 2. mcp_load_target_auth_map_from_admindb(ctx) Refresh runtime_mcp_auth_profiles / runtime_mcp_target_profiles from main.*, JOIN them, and pass the result set to ctx.mcp->load_target_auth_map(rs). Mirrors the pre-4.C ProxySQL_Admin::init_mcp_variables target-auth-map block. 3. mcp_start_listener_if_enabled(ctx) If ctx.mcp->variables.mcp_enabled, check SSL cert availability (when use_ssl=true), check_port_availability(port), and new ProxySQL_MCP_Server(port, ctx.mcp) + start(). Mirrors the startup branch of the pre-4.C ProxySQL_Admin::load_mcp_server(). What this MVP does NOT yet cover (deferred to 4.F continued / 4.G): - Runtime reconfiguration via admin SQL: "LOAD MCP VARIABLES TO RUNTIME", "LOAD MCP PROFILES …", "SAVE MCP …", and "SET mcp-X=Y". Those still hit the FIXME stubs in core left by 4.C. Future work registers them through the plugin command registry (services->register_command). - The variable-listener-restart branch from the original load_mcp_server (port/SSL change → tear down + restart). Today, operators must restart proxysql to pick up mcp-port / mcp-use-ssl changes. Test fixture update (test/tap/tests/unit/genai_plugin_load_unit-t.cpp) Replaced the fake `char*`-cast-to-SQLite3DB stubs with real in-memory SQLite3DB instances pre-seeded with the table shapes genai_start now reads. Without this, the test segfaults the moment the plugin tries to query the fake "DB". The schema is empty — no rows in global_variables / mcp_*_profiles — which exercises the "fresh install, MCP disabled" path: all queries succeed and return empty result sets, so the listener correctly stays down. All three plugin unit tests pass: - genai_plugin_load_unit-t : 8/8 - genai_plugin_anomaly_unit-t : 6/6 - genai_plugin_backend_client : 27/27	2 months ago
Rene Cannao	9d06ab90f5	feat(plugins/genai): Step 1 — empty plugin skeleton ... Creates the GenAI plugin shell that subsequent steps will fill out (per docs/superpowers/specs/2026-04-16-genai-plugin-carveout-design.md "Migration sequence — Step 1"). plugins/genai/ ├── Makefile (mirrors plugins/mysqlx/Makefile) ├── .gitignore ├── include/genai_plugin.h (shared plugin context, atomic started flag) └── src/plugin_main.cpp (descriptor + empty init/start/stop/status_json) Top-level Makefile: cd plugins/genai for every existing build_src* / clean target that already cd's into plugins/mysqlx — release, debug, legacy, and all test_* flavours (testaurora, testaurora_random, testgalera, testgrouprep, testreadonly, testreplicationlag, testall). Unit test: test/tap/tests/unit/genai_plugin_load_unit-t.cpp drives the real .so through load → init → start → stop and asserts that the skeleton currently registers no admin/config/stats tables. Wired into the unit Makefile with an explicit build dependency on the .so so a clean checkout can produce a passing test in one make invocation. End-to-end smoke verified: launching proxysql with plugins=("…/ProxySQL_GenAI_Plugin.so") in proxysql.cnf maps the .so into /proc/PID/maps, leaves the admin port responsive, and shuts down cleanly on SIGTERM. All 58 unit-test binaries pass (+1 over baseline). Notes on what is intentionally NOT in this commit: - No GenAI code has moved out of core yet — that begins Step 3. - The skeleton has no tables, no commands, no query hook, no status vars — those are Step 2 (ABI extensions) and onwards. - Pre-existing breakage in test/tap/tests/Makefile (the generic %-t pattern can't link plugin tests because it doesn't pull the core globals stub) is unchanged; the genai test sidesteps it by living under test/tap/tests/unit/, which has the proper harness. The mysqlx test in test/tap/tests/ remains broken in the same way and is left for a separate fix.	2 months ago