4 Commits (6cdd4e697eb86982e3dfd2ceab009ead5da0efcb)

Author	SHA1	Message	Date
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	482ce386cd	chore(genai-carveout): strip dead PROXYSQLGENAI blocks + plugin hardening ... Two threads of cleanup that had to land together because they touch both core (where the dead code lives) and the plugin (where the new module-owned state lives): # Strip dead #ifdef PROXYSQLGENAI from lib/, include/, src/ (Spec Q9) Step 7 of the carve-out left PSQLGA empty in lib/Makefile, so every #ifdef PROXYSQLGENAI block in the core build became permanently false. The blocks remained as dead text (visible to readers, irrelevant to the binary). Strip ~600 lines spanning: src/main.cpp 2 sites lib/Admin_Bootstrap.cpp 3 sites (mcpdb open + attach + flush_*_variables hooks) lib/Admin_FlushVariables.cpp 2 sites (~250 lines of stub + #if 0 _ORIGINAL bodies) lib/Admin_Handler.cpp 5 sites (LOAD/SAVE GENAI VARIABLES dispatch + LOAD/SAVE MCP VARIABLES dispatch + MCP profiles handler) lib/ProxySQL_Admin.cpp ~10 sites (init_*_variables + load_mcp_server + load/save_mcp_query_rules + GenericRefreshStatistics MCP probes) lib/ProxySQL_Admin_Stats.cpp 3 stats___mcp_* function bodies include/proxysql_admin.h 7 method declarations include/ProxySQL_Admin_Tables_Definitions.h 4 unused DDL macros The plugin now owns LOAD/SAVE MCP VARIABLES, LOAD/SAVE GENAI VARIABLES, LOAD/SAVE MCP PROFILES, LOAD/SAVE MCP QUERY RULES via its admin command registry; the chassis dispatcher catches them before any of the dead core branches would have fired. Caveat — known break: the `stats_mcp_*` tables are still registered (empty schema) but the populator path that fed them is gone. SELECTs return empty result sets until a follow-up wires the populator into the plugin (likely via register_runtime_view or a parallel stats-side ABI). Tracked separately. # Plugin hardening (review-feedback items 4-7) * plugins/genai/Makefile: add -fvisibility=hidden, -fvisibility-inlines-hidden, -fstack-protector-strong (mirrors plugins/mysqlx). Tag proxysql_plugin_descriptor_v1 with __attribute__((visibility("default"))) so the loader's dlsym still resolves; without the explicit annotation the hidden-default flag would also hide the entry point. * plugins/genai/src/Anomaly_Detector.cpp: convert genai_anomaly_embed_fn to std::atomic<…> with relaxed loads/stores. The detector hot-path read races with init/stop on the lifecycle thread; atomics give us a portable no-torn-pointer guarantee without paying for acquire/release fences we don't need (no other state synchronises with the pointer). Replaces the hand-wavy "no mutex needed" comment with the actual reasoning. * plugins/genai/src/{plugin_main,plugin_commands,plugin_tables}.cpp: route the 17 fprintf(stderr, "genai plugin: ...") log lines through a new genai_log() helper that calls services->log_message when the chassis is wired up and falls back to stderr for early-init / unit-test paths. Lines now land in proxysql.log alongside the rest of the runtime. * plugins/genai/include/MCP_Thread.h: add get_variable_string(name, std::string&) returning a std::string, and switch mcp_save_variables_to_admindb (plugin_main.cpp) to it. The legacy get_variable(name, char*) had an unbounded sprintf into a stack buffer and the SAVE path was passing char val[256] — a real overflow risk for *_endpoint_auth bearer tokens. * plugins/genai/src/plugin_main.cpp: rewrite the genai_init / genai_stop comment block with the actual teardown ordering rationale (consumers before producers; atomic-clear-then-delete for the embed hook; why Prometheus counters stay registered).	2 months ago
Rene Cannao	16253a1c99	feat(plugins/genai): adopt ABI-3 separation-of-duties for runtime_mcp_* ... Wire the genai plugin into the plugin-chassis ABI-3 contract added in PR #5688: Admin owns the editable mcp_<X> tables, the module owns the in-memory snapshot under its own mutex, and runtime_mcp_<X> is a chassis-projected view repopulated lazily by a refresh callback before each admin SELECT. No more triple-storage of profile / query-rule data. # MCP_Threads_Handler: per-table install / save / project triplets Replaces the monolithic load_target_auth_map(joined-resultset) with three independent triplets, mirroring the mysqlx config_store rework: install_<X>_from_admin reads main.mcp_<X>, replaces the in-memory snapshot vector under wrlock, rebuilds the joined target_auth_map (for profiles) save_<X>_to_admin_table REPLACEs main.mcp_<X> from the snapshot project_<X>_to_runtime_view DELETE+INSERT runtime_mcp_<X> from the snapshot — invoked only by the chassis runtime-view refresh dispatcher Two new vectors (auth_profiles_, target_profiles_, query_rules_) hold the authoritative per-table rows; target_auth_map remains as the joined view consumed by the listener and is rebuilt whenever either profile vector changes. # Plugin descriptor + table registration genai_register_admin_tables now also calls services->register_runtime_view for runtime_mcp_auth_profiles, runtime_mcp_target_profiles, and runtime_mcp_query_rules, pointing each at a project_*_to_runtime_view trampoline. The runtime_mcp_<X> tables stay registered for their CREATE TABLE schema only — their rows are entirely owned by the per-SELECT projection callbacks from this point on. # Admin SQL surface LOAD MCP PROFILES TO RUNTIME / LOAD MCP QUERY RULES TO RUNTIME now go through install_<X>_from_admin and never touch runtime_mcp_<X>. SAVE MCP PROFILES TO MEMORY is added (was missing pre-ABI-3) and joins SAVE MCP QUERY RULES TO MEMORY in pulling from the module snapshot instead of the runtime view. Query_Tool_Handler::refresh_target_registry no longer re-reads runtime_mcp_<X> via SQLite on every refresh; it consumes the in-memory snapshot directly through GloMCPH->get_all_target_auth_contexts(). # Drop stale GENAI:-prefix tests test/tap/tests/genai_{async,embedding_rerank,live_validation}-t.cpp were left over from before Step 4.A removed the GENAI: query-prefix escape hatch — they would all have failed under ai-g1. Delete them and their groups.json / ai/README.md references; equivalent coverage now flows through the MCP /mcp/ai endpoint and the genai_module-t suite that already drives the plugin surface.	2 months ago
Rene Cannao	5fe286e972	feat(plugins/genai): Step 4.C — move MCP subsystem into the plugin ... Per docs/superpowers/plans/2026-04-19-step4-mcp-subsystem-move.md "4.C / 4.D / 4.E merge" revision. This is the largest single carve-out commit so far: ~13 K LOC physically relocated, plus surgical edits across the core admin SQL surface to neutralize references to the moved code. What moved (git mv preserves history) Tool handlers (lib/<name>.cpp -> plugins/genai/src/tool_handlers/<name>.cpp; include/<name>.h -> plugins/genai/include/<name>.h) Admin_Tool_Handler, Cache_Tool_Handler, Config_Tool_Handler, MySQL_Tool_Handler, Observe_Tool_Handler, Query_Tool_Handler, Stats_Tool_Handler. MCP listener stack (lib/<name>.cpp -> plugins/genai/src/<name>.cpp; include/<name>.h(pp) -> plugins/genai/include/...) MCP_Endpoint, MCP_Thread, ProxySQL_MCP_Server. Promoted from Step 6 (bidirectional dep with MySQL_Tool_Handler): MySQL_FTS. MCP_Tool_Handler base stays in core because AI_Tool_Handler and RAG_Tool_Handler (Steps 5/6 scope) inherit from it; moving the base would force pulling those in too. Plugin lifecycle takeover plugins/genai/include/genai_plugin.h: GenAIPluginContext now holds the MCP_Threads_Handler*. plugins/genai/src/plugin_main.cpp: genai_init constructs the MCP_Threads_Handler and assigns to a plugin-local GloMCPH symbol (the legacy global name, kept for the moved tool handlers that still reference it; the symbol no longer exists in core). genai_stop tears down on shutdown. genai_start is a no-op for now; the MCP listener auto-start is gated on the admin-SQL surface that 4.F restores. plugins/genai/src/ProxySQL_MCP_Server.cpp: AI_Tool_Handler and RAG_Tool_Handler endpoint registration is disabled via stub assignments (the headers stay included so member-pointer types resolve). Steps 5/6 re-enable when those classes move into the plugin. Core neutralization (FIXME stubs awaiting 4.F/4.G) src/main.cpp: GloMCPH global removed; ProxySQL_Main_init_MCP_module removed; shutdown block deleted. lib/ProxySQL_Admin.cpp: init_mcp_variables, load_mcp_server, load_mcp_query_rules_to_runtime, save_mcp_query_rules_from_runtime stubbed to no-ops; original bodies preserved inside `#if 0` for 4.F reference. lib/Admin_FlushVariables.cpp: flush_mcp_variables___database_to_runtime and flush_mcp_variables___runtime_to_database stubbed similarly. lib/ProxySQL_Admin_Stats.cpp: stats___mcp_query_tools_counters, stats___mcp_query_digest, stats___mcp_query_rules stubbed. lib/Admin_Handler.cpp: has_variable("mcp-…") and the load_target_auth_map call site in LOAD MCP PROFILES … stubbed. Result: the MCP listener does NOT start automatically between this commit and 4.F (mcp-* admin variables are not pushed into the running plugin). Documented in the plan's "4.C/4.D/4.E merge" section. src link change: --whole-archive on libproxysql.a src/Makefile: the proxysql binary now links libproxysql.a with -Wl,--whole-archive so all symbols are exported via -Wl,--export-dynamic, allowing the genai plugin .so to dlopen cleanly even when core code paths don't reference moved-but-still- in-core classes (LLM_Bridge, Discovery_Schema, MySQL_Catalog, Static_Harvester, …) directly. test/tap/tests/unit/Makefile gets the same change so unit tests can dlopen the plugin. Without this, ld dead-code-eliminates the unreferenced symbols and dlopen fails with "undefined symbol". Tests passing - genai_plugin_anomaly_unit-t : 6/6 - genai_plugin_load_unit-t : 8/8 (lifecycle init/start/stop + admin table registration) - genai_plugin_backend_client_unit-t : 27/27 Carve-out scoreboard Step 0-2 : 0 (infrastructure only) Step 3 : ~1.1 K (Anomaly_Detector) Step 4.B : ~0.5 K (backend_client helper, no callers yet) Step 4.C : ~13 K (this commit; MCP subsystem) Pending : Steps 4.F, 4.G, 5, 6, 7	2 months ago