From 2f8e63cef26673187f9da5bb208c5f8d2a4767e9 Mon Sep 17 00:00:00 2001
From: Rene Cannao <rene@proxysql.com>
Date: Fri, 20 Mar 2026 11:42:43 +0000
Subject: [PATCH] Address code review feedback for AI TAP group migration

Fixes from PR #5461 review:

1. Fix BASE_GROUP derivation inconsistency in run-tests-isolated.bash
   - Changed from bash pattern matching `${TAP_GROUP%%-g[0-9]*}` to sed
   - Now uses same pattern as ensure-infras.bash: `sed -E "s/[-_]g[0-9]+.*//"`
   - Removed redundant BASE_GROUP reassignment

2. Update outdated comments in seed files
   - seed-mysql.sql and seed-pgsql.sql now correctly reference setup-infras.bash

3. Change INSERT OR IGNORE to INSERT OR REPLACE in mcp-config.sql
   - Ensures credentials are updated on reruns for deterministic state

4. Add shellcheck directives to env.sh files
   - Added `# shellcheck shell=bash` to ai/env.sh and ai-g1/env.sh

5. Add explicit validation for derived infrastructure names
   - setup-infras.bash now validates DEFAULT_MYSQL_INFRA and DEFAULT_PGSQL_INFRA
   - Provides clear error messages if infras.lst is misconfigured

6. Add WORKSPACE export to README examples
   - All runnable examples now include `export WORKSPACE=$(pwd)`

Test verification:
- ai-g1 group infrastructure setup works correctly
- legacy-g2 group tests still pass (backward compatibility verified)
---
 test/infra/control/run-tests-isolated.bash |  3 +--
 test/tap/groups/ai-g1/env.sh               |  1 +
 test/tap/groups/ai/README.md               |  3 +++
 test/tap/groups/ai/env.sh                  |  1 +
 test/tap/groups/ai/mcp-config.sql          |  4 ++--
 test/tap/groups/ai/seed-mysql.sql          |  2 +-
 test/tap/groups/ai/seed-pgsql.sql          |  2 +-
 test/tap/groups/ai/setup-infras.bash       | 11 +++++++++++
 8 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/test/infra/control/run-tests-isolated.bash b/test/infra/control/run-tests-isolated.bash
index ded42a880..689df1854 100755
--- a/test/infra/control/run-tests-isolated.bash
+++ b/test/infra/control/run-tests-isolated.bash
@@ -12,7 +12,7 @@ export INFRA_ID="${INFRA_ID:-dev-$USER}"
 
 # 1. Determine Required Infras
 INFRAS_TO_CHECK=""
-BASE_GROUP="${TAP_GROUP%%-g[0-9]*}" # Strip -g1, -g2 etc.
+BASE_GROUP=$(echo "${TAP_GROUP}" | sed -E "s/[-_]g[0-9]+.*//") # Strip -g1, -g2, _g1, _g2 etc.
 
 if [ -n "${TAP_GROUP}" ]; then
     if [ -f "${WORKSPACE}/test/tap/groups/${TAP_GROUP}/infras.lst" ]; then
@@ -142,7 +142,6 @@ docker run \
 # This runs before the test runner container is removed, allowing cleanup
 # of ProxySQL-specific configuration while admin is still accessible
 if [ -n "${TAP_GROUP}" ]; then
-    BASE_GROUP="${TAP_GROUP%%-g[0-9]*}"
     PRE_CLEANUP_HOOK="${WORKSPACE}/test/tap/groups/${TAP_GROUP}/pre-cleanup.bash"
     if [ ! -f "${PRE_CLEANUP_HOOK}" ]; then
         PRE_CLEANUP_HOOK="${WORKSPACE}/test/tap/groups/${BASE_GROUP}/pre-cleanup.bash"
diff --git a/test/tap/groups/ai-g1/env.sh b/test/tap/groups/ai-g1/env.sh
index 1ea85268a..04635ab7a 100644
--- a/test/tap/groups/ai-g1/env.sh
+++ b/test/tap/groups/ai-g1/env.sh
@@ -1,3 +1,4 @@
+# shellcheck shell=bash
 # AI-g1 Subgroup Environment Configuration
 # Sources the parent ai group configuration
 
diff --git a/test/tap/groups/ai/README.md b/test/tap/groups/ai/README.md
index ec54af526..a5f8dc8cd 100644
--- a/test/tap/groups/ai/README.md
+++ b/test/tap/groups/ai/README.md
@@ -93,6 +93,7 @@ source test/infra/common/env.sh
 export INFRA_ID="ai-harvest-$(date +%s)"
 export TAP_GROUP="ai-g1"
 export TEST_PY_TAP_INCL="test_mcp_static_harvest-t"
+export WORKSPACE=$(pwd)
 
 source test/infra/common/env.sh
 ./test/infra/control/ensure-infras.bash
@@ -106,6 +107,7 @@ source test/infra/common/env.sh
 export INFRA_ID="ai-discovery-$(date +%s)"
 export TAP_GROUP="ai-g1"
 export TEST_PY_TAP_INCL="test_mcp_llm_discovery_phaseb-t"
+export WORKSPACE=$(pwd)
 
 source test/infra/common/env.sh
 ./test/infra/control/ensure-infras.bash
@@ -119,6 +121,7 @@ source test/infra/common/env.sh
 export INFRA_ID="ai-claude-$(date +%s)"
 export TAP_GROUP="ai-g1"
 export TEST_PY_TAP_INCL="test_mcp_claude_headless_flow-t"
+export WORKSPACE=$(pwd)
 
 source test/infra/common/env.sh
 ./test/infra/control/ensure-infras.bash
diff --git a/test/tap/groups/ai/env.sh b/test/tap/groups/ai/env.sh
index de39e7265..52b6152e9 100644
--- a/test/tap/groups/ai/env.sh
+++ b/test/tap/groups/ai/env.sh
@@ -1,3 +1,4 @@
+# shellcheck shell=bash
 # AI TAP Group Environment Configuration
 # Defines the primary targets for AI/MCP tests using standard test/infra/ pattern
 
diff --git a/test/tap/groups/ai/mcp-config.sql b/test/tap/groups/ai/mcp-config.sql
index 87c651a19..f106b302e 100644
--- a/test/tap/groups/ai/mcp-config.sql
+++ b/test/tap/groups/ai/mcp-config.sql
@@ -26,9 +26,9 @@ LOAD PGSQL SERVERS TO RUNTIME;
 SAVE PGSQL SERVERS TO DISK;
 
 -- Configure frontend users for TAP tests
-INSERT OR IGNORE INTO mysql_users (username, password, active, default_hostgroup, comment)
+INSERT OR REPLACE INTO mysql_users (username, password, active, default_hostgroup, comment)
     VALUES ('${TAP_MYSQLUSERNAME}', '${TAP_MYSQLPASSWORD}', 1, 0, 'ai local');
-INSERT OR IGNORE INTO mysql_users (username, password, active, default_hostgroup, comment)
+INSERT OR REPLACE INTO mysql_users (username, password, active, default_hostgroup, comment)
     VALUES ('testuser', 'testuser', 1, 0, 'ai local');
 LOAD MYSQL USERS TO RUNTIME;
 SAVE MYSQL USERS TO DISK;
diff --git a/test/tap/groups/ai/seed-mysql.sql b/test/tap/groups/ai/seed-mysql.sql
index 6c2e1791e..342563197 100644
--- a/test/tap/groups/ai/seed-mysql.sql
+++ b/test/tap/groups/ai/seed-mysql.sql
@@ -1,6 +1,6 @@
 -- AI Group MySQL Test Data Seeding
 -- Creates tables needed by AI/MCP tests
--- This is executed by docker-mysql-post.bash when TAP_GROUP starts with 'ai'
+-- This is executed by setup-infras.bash as part of AI group infrastructure setup
 
 CREATE DATABASE IF NOT EXISTS test;
 USE test;
diff --git a/test/tap/groups/ai/seed-pgsql.sql b/test/tap/groups/ai/seed-pgsql.sql
index 7208c9a47..cf14fedad 100644
--- a/test/tap/groups/ai/seed-pgsql.sql
+++ b/test/tap/groups/ai/seed-pgsql.sql
@@ -1,6 +1,6 @@
 -- AI Group PostgreSQL Test Data Seeding
 -- Creates tables needed by AI/MCP tests
--- This is executed by docker-pgsql-post.bash when TAP_GROUP starts with 'ai'
+-- This is executed by setup-infras.bash as part of AI group infrastructure setup
 
 CREATE TABLE IF NOT EXISTS public.tap_pgsql_static_accounts (
   account_id INT PRIMARY KEY,
diff --git a/test/tap/groups/ai/setup-infras.bash b/test/tap/groups/ai/setup-infras.bash
index 1daa6aecb..4bcf43096 100755
--- a/test/tap/groups/ai/setup-infras.bash
+++ b/test/tap/groups/ai/setup-infras.bash
@@ -34,6 +34,17 @@ if [ -z "${DEFAULT_MYSQL_INFRA:-}" ] || [ -z "${DEFAULT_PGSQL_INFRA:-}" ]; then
     fi
 fi
 
+# Validate that required infrastructure names were resolved
+if [ -z "${DEFAULT_MYSQL_INFRA:-}" ]; then
+    echo "ERROR: Could not resolve DEFAULT_MYSQL_INFRA (check ${SCRIPT_DIR}/infras.lst)"
+    exit 1
+fi
+
+if [ -z "${DEFAULT_PGSQL_INFRA:-}" ]; then
+    echo "ERROR: Could not resolve DEFAULT_PGSQL_INFRA (check ${SCRIPT_DIR}/infras.lst)"
+    exit 1
+fi
+
 export ROOT_PASSWORD=$(echo -n "${INFRA_ID}" | sha256sum | head -c 10)
 PROXY_CONTAINER="proxysql.${INFRA_ID}"