test: make omitted cluster comments optional

docs/superpowers/plans/2026-08-10-cluster-simulator-optional-comment.md

@@ -0,0 +1,71 @@
+# Cluster Simulator Optional Comment Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Make omitted expected cluster-state comments behave as wildcards while preserving exact matching for explicitly supplied comments and improving mismatch diagnostics.
+
+**Architecture:** Add a presence bit to the shared `server_status` tuple, populate it while parsing expected and runtime JSON, and consume it in comparison and JSON projection. Exercise the shared functions with a focused TAP unit binary and then validate the existing simulator end to end.
+
+**Tech Stack:** C++11-compatible tuple-based state model, nlohmann JSON, ProxySQL TAP test framework, GNU Make, isolated cluster-simulator CI infrastructure.
+
+## Global Constraints
+
+- Base the dedicated branch on the latest `origin/v3.0`.
+- Omitted comments are wildcards; explicit comments, including empty strings, remain exact.
+- Do not change checksum inputs or ProxySQL runtime behavior.
+- Do not modify or include PR #6017 changes.
+
+---
+
+### Task 1: Add the focused failing regression
+
+**Files:**
+- Create: `test/deps/cluster_simulator/tests/common_utils_unit-t.cpp`
+- Modify: `test/deps/cluster_simulator/Makefile`
+- Modify: `test/infra/control/cluster-simulator-ci.bash`
+
+**Interfaces:**
+- Consumes: `server_status`, `matching_server_status()`, and `cluster_status_to_json()` from `lib/common_utils.h`.
+- Produces: `common_utils_unit-t`, a focused TAP binary runnable through `make check` and executed by the cluster-simulator CI build.
+
+- [ ] **Step 1: Add assertions for omitted, explicit, different, and explicitly empty comments plus JSON projection.**
+- [ ] **Step 2: Add `make check` to the cluster-simulator CI build path.**
+- [ ] **Step 3: Build and run the focused binary.**
+
+  Run: `make -C test/deps/cluster_simulator check -j"$(nproc)"`
+
+  Expected: FAIL because the current tuple cannot distinguish omitted from explicitly empty comments and diagnostics omit comments.
+
+### Task 2: Preserve comment presence and apply the semantic contract
+
+**Files:**
+- Modify: `test/deps/cluster_simulator/lib/common_utils.h`
+- Modify: `test/deps/cluster_simulator/lib/common_utils.cpp`
+
+**Interfaces:**
+- Produces: `MYSQL_SERVER_STATUS_T::COMMENT_IS_SET` and a nine-element `server_status` whose final boolean records comment presence.
+- Preserves: all existing public helper names and checksum behavior.
+
+- [ ] **Step 1: Extend `server_status` and its index enum with the presence bit.**
+- [ ] **Step 2: Set the bit in `extract_cluster_status()` using `m_mysql_server.contains("comment")`.**
+- [ ] **Step 3: Compare comments only when the expected presence bit is true.**
+- [ ] **Step 4: Serialize comments only when their presence bit is true.**
+- [ ] **Step 5: Rebuild and rerun `common_utils_unit-t`.**
+
+  Expected: all focused TAP assertions pass.
+
+### Task 3: Document and verify the shared behavior
+
+**Files:**
+- Modify: `test/deps/cluster_simulator/README.md`
+
+**Interfaces:**
+- Documents: optional expected-state fields and omission-as-wildcard semantics.
+
+- [ ] **Step 1: Update the payload-format documentation with the exact optional-field contract.**
+- [ ] **Step 2: Build the cluster simulator with bounded parallelism.**
+- [ ] **Step 3: Run the Galera simulator group repeatedly in a unique isolated infrastructure.**
+- [ ] **Step 4: Run at least one additional shared-comparator simulator family.**
+- [ ] **Step 5: Run `git diff --check`, inspect the full diff, and verify the worktree contains only this fix.**
+- [ ] **Step 6: Commit the implementation with a focused message.**
+- [ ] **Step 7: Push the dedicated branch and open a draft PR against `v3.0` describing the CI evidence and independence from PR #6017.**

docs/superpowers/specs/2026-08-10-cluster-simulator-optional-comment-design.md

@@ -0,0 +1,65 @@
+# Cluster Simulator Optional Comment Design
+
+## Problem
+
+The cluster simulator accepts `comment` in `proxysql_init_state` and
+`proxysql_final_state`, but it currently turns an omitted comment into an empty
+string and compares that string exactly. This differs from the documented
+four-field expected-state shape and from the wildcard behavior already used
+for omitted `weight`, `max_connections`, and `use_ssl` values.
+
+The `cluster_sim_galera-g1` failure in GitHub Actions exposed the mismatch.
+The initial runtime transition retained non-empty comments for only some
+servers. All visible expected and actual fields matched, but the hidden comment
+values caused `check_cluster_status()` to fail. The diagnostic JSON also
+discarded comments, so the failure did not identify the differing field.
+
+PR #6017 does not modify the cluster simulator or its Galera wrapper. The fix
+therefore belongs in a dedicated PR based on `v3.0`.
+
+## Contract
+
+- `hostgroup_id`, `hostname`, `port`, and `status` remain required expected
+  state fields.
+- Omitted `weight`, `max_connections`, `use_ssl`, and `comment` fields are
+  wildcards.
+- An explicitly supplied comment, including `""`, is compared exactly.
+- Runtime rows always have a specified comment because the value is selected
+  from `runtime_mysql_servers`.
+- Diagnostic and simulation JSON include a comment whenever the corresponding
+  state carries an explicitly specified comment.
+- Checksums remain diagnostic. Their existing inputs and meaning do not change.
+
+## Representation
+
+Extend `server_status` with a boolean recording whether `comment` was present
+in the source JSON. This preserves the distinction between an omitted comment
+and an explicitly empty comment without requiring C++17 `std::optional`; the
+project can still compile in its C++11 fallback mode.
+
+`extract_cluster_status()` sets the flag from `contains("comment")`.
+`matching_server_status()` compares comment values only when the expected
+flag is true. `cluster_status_to_json()` emits the comment only when the flag
+is true.
+
+## Testing
+
+Add focused TAP unit coverage for the shared comparator and JSON projection:
+
+- omitted expected comment matches a non-empty actual comment;
+- explicit matching comments pass;
+- explicit different comments fail;
+- explicitly empty expected comment does not match a non-empty actual comment;
+- JSON omits an unspecified comment and emits a specified one.
+
+Run the focused test before implementation to capture the intended failure,
+then after implementation for GREEN. Build the cluster simulator and run the
+Galera simulator group in isolated infrastructure. Also run shared-comparator
+coverage for another simulator family to guard against cross-family regressions.
+
+## Non-goals
+
+- Do not change ProxySQL runtime comment propagation.
+- Do not add sleeps or hardcode infrastructure-specific comments.
+- Do not change PR #6017 or its pending RSA authentication work.
+- Do not redefine the cluster-state checksum as semantic equality.

test/deps/cluster_simulator/Makefile

@@ -90,7 +90,7 @@ debug: cluster_simulator
 
 .PHONY: clean
 clean:
-	rm -f cluster_simulator
+	rm -f cluster_simulator common_utils_unit-t
 	rm -rf $(UTILSLDIR)
 
 
@@ -130,3 +130,10 @@ $(TAP_LDIR)/libtap.so:
 
 cluster_simulator: cluster_simulator.cpp $(UTILSLDIR)/cluster_simulator_lib.a $(TAP_LDIR)/libtap.so
 	$(CXX) -o $@ $(OBJ) cluster_simulator.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) $(MYLIBSJEMALLOC) $(MYLIBS) $(STATIC_LIBS) $(UTILSLDIR)/cluster_simulator_lib.a
+
+common_utils_unit-t: tests/common_utils_unit-t.cpp $(UTILSLDIR)/common_utils.o $(TAP_LDIR)/libtap.so
+	$(CXX) -o $@ $(OBJ) tests/common_utils_unit-t.cpp $(UTILSLDIR)/common_utils.o $(INCLUDEDIRS) $(LDIRS) $(OPT) $(MYLIBSJEMALLOC) $(MYLIBS) $(STATIC_LIBS)
+
+.PHONY: check
+check: common_utils_unit-t
+	LD_LIBRARY_PATH="$(TAP_LDIR):$${LD_LIBRARY_PATH:-}" ./common_utils_unit-t

test/deps/cluster_simulator/README.md

@@ -135,7 +135,9 @@ Simulator payload is a JSON object holding the following members:
   the initial configuration provided.
 
   Objects in the array are required to be JSON objects with the following members: `{hostgroup_id,
-  hostname, port, status}`.
+  hostname, port, status}`. They can also include `weight`, `max_connections`, `use_ssl`, and
+  `comment`. An omitted optional member is not compared; an explicitly supplied member, including
+  an empty `comment`, must match exactly.
 
   Status member should be an `string` of one of the following values:
   "SHUNNED|ONLINE|OFFLINE_SOFT|OFFLINE_HARD". NOTE: Since 'OFFLINE_HARD' status is a transitory

test/deps/cluster_simulator/lib/common_utils.cpp

@@ -353,36 +353,40 @@ bool check_present_and_type(const json& j, const std::vector<std::string>& path,
 	return res;
 }
 
-bool matching_server_status(const server_status& srv_st1, const server_status& srv_st2) {
+bool matching_server_status(const server_status& exp_srv_st, const server_status& act_srv_st) {
 	bool res = false;
 	bool same_hid_host =
-		std::get<0>(srv_st1) == std::get<0>(srv_st2) &&
-		std::get<1>(srv_st1) == std::get<1>(srv_st2) &&
-		std::get<2>(srv_st1) == std::get<2>(srv_st2);
+		std::get<0>(exp_srv_st) == std::get<0>(act_srv_st) &&
+		std::get<1>(exp_srv_st) == std::get<1>(act_srv_st) &&
+		std::get<2>(exp_srv_st) == std::get<2>(act_srv_st);
 
 	if (same_hid_host) {
 		std::vector<std::string> allowed_sts {
-			str_split(std::get<3>(srv_st1), '|')
+			str_split(std::get<3>(exp_srv_st), '|')
 		};
 
-		if (std::find(allowed_sts.begin(), allowed_sts.end(), std::get<3>(srv_st2)) != allowed_sts.end()) {
-			const int64_t exp_weight = std::get<MYSQL_SERVER_STATUS_T::WEIGHT>(srv_st1);
-			const int64_t exp_max_conns = std::get<MYSQL_SERVER_STATUS_T::MAX_CONNS>(srv_st1);
-			const int32_t exp_use_ssl = std::get<MYSQL_SERVER_STATUS_T::USE_SSL>(srv_st1);
-			const std::string exp_comment = std::get<MYSQL_SERVER_STATUS_T::COMMENT>(srv_st1);
+		if (std::find(allowed_sts.begin(), allowed_sts.end(), std::get<3>(act_srv_st)) != allowed_sts.end()) {
+			const int64_t exp_weight = std::get<MYSQL_SERVER_STATUS_T::WEIGHT>(exp_srv_st);
+			const int64_t exp_max_conns = std::get<MYSQL_SERVER_STATUS_T::MAX_CONNS>(exp_srv_st);
+			const int32_t exp_use_ssl = std::get<MYSQL_SERVER_STATUS_T::USE_SSL>(exp_srv_st);
+			const std::string& exp_comment = std::get<MYSQL_SERVER_STATUS_T::COMMENT>(exp_srv_st);
+			const bool exp_comment_is_set =
+				std::get<MYSQL_SERVER_STATUS_T::COMMENT_IS_SET>(exp_srv_st);
 
 			bool match = true;
 
 			if (exp_weight != -1) {
-				match = match && exp_weight == std::get<MYSQL_SERVER_STATUS_T::WEIGHT>(srv_st2);
+				match = match && exp_weight == std::get<MYSQL_SERVER_STATUS_T::WEIGHT>(act_srv_st);
 			}
 			if (exp_max_conns != -1) {
-				match = match && exp_max_conns == std::get<MYSQL_SERVER_STATUS_T::MAX_CONNS>(srv_st2);
+				match = match && exp_max_conns == std::get<MYSQL_SERVER_STATUS_T::MAX_CONNS>(act_srv_st);
 			}
 			if (exp_use_ssl != -1) {
-				match = match && exp_use_ssl == std::get<MYSQL_SERVER_STATUS_T::USE_SSL>(srv_st2);
+				match = match && exp_use_ssl == std::get<MYSQL_SERVER_STATUS_T::USE_SSL>(act_srv_st);
+			}
+			if (exp_comment_is_set) {
+				match = match && exp_comment == std::get<MYSQL_SERVER_STATUS_T::COMMENT>(act_srv_st);
 			}
-			match = match && exp_comment == std::get<MYSQL_SERVER_STATUS_T::COMMENT>(srv_st2);
 
 			res = match;
 		}
@@ -990,6 +994,7 @@ std::pair<int, std::string> extract_cluster_status(
 		int64_t max_conns = -1;
 		int32_t use_ssl = -1;
 		std::string comment {};
+		bool comment_is_set = false;
 
 		try {
 			hostgroup_id = m_mysql_server.at("hostgroup_id");
@@ -1008,13 +1013,24 @@ std::pair<int, std::string> extract_cluster_status(
 			}
 			if (m_mysql_server.contains("comment")) {
 				comment = m_mysql_server.at("comment");
+				comment_is_set = true;
 			}
 		} catch(const std::exception& e) {
 			return { EXIT_FAILURE, e.what() };
 		}
 
 		cluster_status.push_back(
-			std::make_tuple(hostgroup_id, hostname, port, status, weight, max_conns, use_ssl, comment)
+			std::make_tuple(
+				hostgroup_id,
+				hostname,
+				port,
+				status,
+				weight,
+				max_conns,
+				use_ssl,
+				comment,
+				comment_is_set
+			)
 		);
 	}
 
@@ -1088,10 +1104,11 @@ ordered_json cluster_status_to_json(const std::vector<server_status>& cluster_st
 			{ "status", std::get<3>(server_status) },
 		};
 
-		int64_t weight = std::get<MYSQL_SERVER_STATUS_T::WEIGHT>(server_status);
-		int64_t max_conns = std::get<MYSQL_SERVER_STATUS_T::MAX_CONNS>(server_status);
-		int64_t use_ssl = std::get<MYSQL_SERVER_STATUS_T::USE_SSL>(server_status);
-		string comment = std::get<MYSQL_SERVER_STATUS_T::COMMENT>(server_status);
+		const int64_t weight = std::get<MYSQL_SERVER_STATUS_T::WEIGHT>(server_status);
+		const int64_t max_conns = std::get<MYSQL_SERVER_STATUS_T::MAX_CONNS>(server_status);
+		const int64_t use_ssl = std::get<MYSQL_SERVER_STATUS_T::USE_SSL>(server_status);
+		const string& comment = std::get<MYSQL_SERVER_STATUS_T::COMMENT>(server_status);
+		const bool comment_is_set = std::get<MYSQL_SERVER_STATUS_T::COMMENT_IS_SET>(server_status);
 
 		if (weight != -1) {
 			srv_result["weight"] = weight;
@@ -1102,6 +1119,9 @@ ordered_json cluster_status_to_json(const std::vector<server_status>& cluster_st
 		if (use_ssl != -1) {
 			srv_result["use_ssl"] = use_ssl;
 		}
+		if (comment_is_set) {
+			srv_result["comment"] = comment;
+		}
 
 		result.push_back(srv_result);
 	}

test/deps/cluster_simulator/lib/common_utils.h

@@ -20,7 +20,8 @@ using status   = std::string;
 using comment  = std::string;
 
 using mysql_server_def = std::tuple<int, hostname, port, status, int, comment>;
-using server_status    = std::tuple<hg_id,hostname,port,status,int64_t,int64_t,int32_t,comment>;
+using server_status =
+	std::tuple<hg_id, hostname, port, status, int64_t, int64_t, int32_t, comment, bool>;
 struct MYSQL_SERVER_STATUS_T {
 	enum {
 		HG_ID,
@@ -30,7 +31,8 @@ struct MYSQL_SERVER_STATUS_T {
 		WEIGHT,
 		MAX_CONNS,
 		USE_SSL,
-		COMMENT
+		COMMENT,
+		COMMENT_IS_SET
 	};
 };
 
@@ -246,14 +248,17 @@ std::pair<int,std::string> gen_invalid_keys_err(
  */
 bool check_present_and_type(const json& j, const std::vector<std::string>& path, const json::value_t& type);
 /**
- * @brief Check that the supplied 'server_status' are both equal.
+ * @brief Check whether an actual server status satisfies an expected status.
  *
- * @param srv_st1 The first server status to check.
- * @param srv_st2 The second server status to check.
+ * Optional fields omitted from the expected status are wildcards. Fields that
+ * are present, including an explicitly empty comment, are compared exactly.
  *
- * @return 'true' if both server status are equal, 'false' otherwise.
+ * @param exp_srv_st The expected server status.
+ * @param act_srv_st The actual server status.
+ *
+ * @return 'true' if the actual status satisfies every expectation.
  */
-bool matching_server_status(const server_status& srv_st1, const server_status& srv_st2);
+bool matching_server_status(const server_status& exp_srv_st, const server_status& act_srv_st);
 /**
  * @brief Check that the both supplied cluster states are equal, or equivalent.
  *