fix: close controller-peers-env-fallback validation gaps (bug-1909..1912)

- bug-1912: xce_parse_peers() ignores whitespace around entries and skips
  blank ones, matching xceadmin's resolve_controllers(), so "a=h:1, b=h:2"
  names the second candidate "b" (5 libxce tests + regression case).
- bug-1911: xcecd frees the -l log dir before the -h / --version returns
  (regression case: no LeakSanitizer report, exit 0).
- bug-1910: both version scripts build the apiserver with -mod=vendor
  (as the meson build does) instead of -mod=readonly, so they no longer
  need a Go module cache.
- bug-1909: test-version-startup-consistency.sh accepts exactly the
  dirty-content-fingerprint pairing ('<sha>-<8 hex>' in --version with
  '<sha>-dirty' in the startup log), with self-test cases (e)/(f).

Both version scripts now exit 0 on a dirty tree with XCE_CONTROLLER_PEERS
set; regression script 35/35; xce-build all: C 127/0, scheduler 16/0,
xceadmin 364 passed, no ASAN reports.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/include/xce/cluster.h

@@ -129,7 +129,10 @@ void xce_reconnect_fini(xce_reconnect_ctx_t* rctx);
 /**
  * xce_parse_peers() — parse "--peers name=host:port,..." into cfg->candidates[].
  *
- * Each token may be "name=host:port" or bare "host:port".
+ * Each token may be "name=host:port" or bare "host:port". Whitespace around
+ * each comma-separated token is ignored and tokens that are blank after
+ * trimming are skipped (they do not use a MAX_HA_CANDIDATES slot), matching
+ * xceadmin's resolve_controllers() (bug-1912).
  * The name field is stored when present; it allows multiple daemons on the
  * same host to identify themselves by logical name rather than IP address.
  *

src/lib/cluster.c

@@ -11,6 +11,7 @@
  * Backoff sleep is skipped when s_connect_fn is set (i.e., in unit tests)
  * so tests do not need to wait for real timeouts.
  */
+#include <ctype.h>
 #include <errno.h>
 #include <stddef.h>
 #include <stdio.h>
@@ -35,6 +36,19 @@ int xce_parse_peers(const char* peers_str, xce_cluster_config_t* cfg)
     char* saveptr;
     char* tok = strtok_r(buf, ",", &saveptr);
     while (tok && cfg->candidates_count < MAX_HA_CANDIDATES) {
+        /* bug-1912: ignore whitespace around each entry and skip entries
+         * that are blank after trimming, as xceadmin's
+         * resolve_controllers() does, so "a=h:1, b=h:2" names "b". */
+        while (isspace((unsigned char)*tok))
+            tok++;
+        char* end = tok + strlen(tok);
+        while (end > tok && isspace((unsigned char)end[-1]))
+            *--end = '\0';
+        if (*tok == '\0') {
+            tok = strtok_r(NULL, ",", &saveptr);
+            continue;
+        }
+
         /* Format: name=host:port  OR  host:port */
         char* eq    = strchr(tok, '=');
         char* addr  = eq ? eq + 1 : tok;

src/lib/testsuite/test_cluster.c

@@ -420,6 +420,63 @@ static void test_find_identity_empty_name_matches_empty_hostname(void)
     printf("test_find_identity_empty_name_matches_empty_hostname: PASS\n");
 }
 
+/* bug-1912: xce_parse_peers() trims whitespace around each comma-separated
+ * entry and skips entries that are blank after trimming, matching xceadmin's
+ * resolve_controllers(), so "a=h:1, b=h:2" names the second candidate "b". */
+static void test_parse_peers_trims_spaces_around_entries(void)
+{
+    xce_cluster_config_t cfg = {0};
+    TEST_ASSERT_EQUAL_INT(0, xce_parse_peers("a=h1:1, b=h2:2 ,\tc=h3:3 ", &cfg));
+    TEST_ASSERT_EQUAL_INT(3, cfg.candidates_count);
+    TEST_ASSERT_EQUAL_STRING("a", cfg.candidates[0].name);
+    TEST_ASSERT_EQUAL_STRING("b", cfg.candidates[1].name);
+    TEST_ASSERT_EQUAL_STRING("c", cfg.candidates[2].name);
+    TEST_ASSERT_EQUAL_STRING("h1", cfg.candidates[0].hostname);
+    TEST_ASSERT_EQUAL_STRING("h2", cfg.candidates[1].hostname);
+    TEST_ASSERT_EQUAL_STRING("h3", cfg.candidates[2].hostname);
+    TEST_ASSERT_EQUAL_INT(1, cfg.candidates[0].port);
+    TEST_ASSERT_EQUAL_INT(2, cfg.candidates[1].port);
+    TEST_ASSERT_EQUAL_INT(3, cfg.candidates[2].port);
+    printf("test_parse_peers_trims_spaces_around_entries: PASS\n");
+}
+
+static void test_parse_peers_trims_unnamed_entry(void)
+{
+    xce_cluster_config_t cfg = {0};
+    TEST_ASSERT_EQUAL_INT(0, xce_parse_peers("  localhost:9991  ", &cfg));
+    TEST_ASSERT_EQUAL_INT(1, cfg.candidates_count);
+    TEST_ASSERT_EQUAL_STRING("", cfg.candidates[0].name);
+    TEST_ASSERT_EQUAL_STRING("localhost", cfg.candidates[0].hostname);
+    TEST_ASSERT_EQUAL_INT(9991, cfg.candidates[0].port);
+    printf("test_parse_peers_trims_unnamed_entry: PASS\n");
+}
+
+static void test_parse_peers_skips_blank_entries(void)
+{
+    xce_cluster_config_t cfg = {0};
+    TEST_ASSERT_EQUAL_INT(0, xce_parse_peers("a=h1:1, ,b=h2:2", &cfg));
+    TEST_ASSERT_EQUAL_INT(2, cfg.candidates_count);
+    TEST_ASSERT_EQUAL_STRING("a", cfg.candidates[0].name);
+    TEST_ASSERT_EQUAL_STRING("b", cfg.candidates[1].name);
+    printf("test_parse_peers_skips_blank_entries: PASS\n");
+}
+
+static void test_parse_peers_all_blank_rejected(void)
+{
+    xce_cluster_config_t cfg = {0};
+    TEST_ASSERT_EQUAL_INT(-1, xce_parse_peers(" , \t ", &cfg));
+    TEST_ASSERT_EQUAL_INT(0, cfg.candidates_count);
+    printf("test_parse_peers_all_blank_rejected: PASS\n");
+}
+
+/* Unchanged behavior: an entry without a port is still a parse error. */
+static void test_parse_peers_entry_without_port_rejected(void)
+{
+    xce_cluster_config_t cfg = {0};
+    TEST_ASSERT_EQUAL_INT(-1, xce_parse_peers("a=h1:1, b=h2", &cfg));
+    printf("test_parse_peers_entry_without_port_rejected: PASS\n");
+}
+
 /* ── Test group entry point ─────────────────────────────────────── */
 void test_cluster(void)
 {
@@ -450,4 +507,9 @@ void test_cluster(void)
     RUN_TEST(test_find_identity_first_duplicate_wins);
     RUN_TEST(test_find_identity_null_name);
     RUN_TEST(test_find_identity_empty_name_matches_empty_hostname);
+    RUN_TEST(test_parse_peers_trims_spaces_around_entries);
+    RUN_TEST(test_parse_peers_trims_unnamed_entry);
+    RUN_TEST(test_parse_peers_skips_blank_entries);
+    RUN_TEST(test_parse_peers_all_blank_rejected);
+    RUN_TEST(test_parse_peers_entry_without_port_rejected);
 }

src/tools/test-dirty-fingerprint-startup-log-unchanged.sh

@@ -517,13 +517,15 @@ fi
 yellow "=== [apiserver] go build (direct invocation, see header comment re: bug-1597 / go_build's goproxy.cn hang in this sandbox) -- commit_id=$API_COMMIT_ID build_time=$API_BUILD_TIME dirty_fingerprint=$API_DIRTY_FINGERPRINT ==="
 APISERVER_BIN="$APILIB_DIR/frontend/apiserver/xce_apiserver"
 mkdir -p "$(dirname "$APISERVER_BIN")"
+# bug-1910: -mod=vendor builds from the committed vendor/ tree (as the
+# canonical meson build does), so no Go module cache is needed.
 nix develop "$ISO_SRC" --command bash -c "
 set -e
 cd '$ISO_SRC/frontend/apiserver'
 CGO_CFLAGS=\"-g \$(pkg-config --cflags librtti c-capnproto openssl tinylog) -I$ISO_SRC/include -I$APILIB_DIR/lib\"
 CGO_LDFLAGS=\"-g \$(pkg-config --libs librtti c-capnproto openssl tinylog) -lm -lrt -L$APILIB_DIR/lib -lxce\"
 GOPROXY=off CGO_ENABLED=1 CGO_CFLAGS=\"\$CGO_CFLAGS\" CGO_LDFLAGS=\"\$CGO_LDFLAGS\" \
-  go build -mod=readonly -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME -X main.DirtyFingerprint=$API_DIRTY_FINGERPRINT\" \
+  go build -mod=vendor -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME -X main.DirtyFingerprint=$API_DIRTY_FINGERPRINT\" \
   -o $APISERVER_BIN ./cmd/main.go
 "
 

src/tools/test-version-startup-consistency.sh

@@ -39,7 +39,10 @@
 # so this script extracts just those two field values from each capture
 # and asserts the extracted VALUES are byte-identical between --version
 # and the startup log, for each binary. Surrounding text differing is
-# expected and not a failure.
+# expected and not a failure. One sanctioned exception (bug-1909): on a
+# dirty tree the dirty-content-fingerprint feature shows
+# '<sha>-<8-hex fingerprint>' in --version while the startup log keeps
+# '<sha>-dirty'; check_consistency() accepts exactly that pairing.
 #
 # Build path notes:
 #   - xce_scheduler is built via the exact same real `/xce-build`-equivalent
@@ -63,11 +66,12 @@
 #     `go build` directly with the same -ldflags shape go_build/Makefile
 #     use and the same CGO_CFLAGS/CGO_LDFLAGS pkg-config derivation
 #     test-backend/apiserver's meson.build already use, against the
-#     default (already-populated) module cache -- functionally the same
+#     committed vendor/ tree -- functionally the same
 #     real build (same source, same compiler, same ldflags injection
 #     mechanism), just not routed through the specific wrapper script that
 #     hangs in this sandbox. Neither go_build nor the Makefile is modified.
-#     `go build` is invoked with `-mod=readonly` explicitly (confirmed live:
+#     `go build` was originally invoked with `-mod=readonly` (superseded by
+#     `-mod=vendor`, see the bug-1910 note below; confirmed live at the time:
 #     without it, this build silently ADDED ~65 unrelated indirect-
 #     dependency /go.mod checksum lines to the committed
 #     src/frontend/apiserver/go.sum as a side effect of resolving the full
@@ -78,6 +82,13 @@
 #     check" constraint requires. The build still succeeds under
 #     -mod=readonly against this repo's current go.sum, so nothing was
 #     actually missing -- the extra entries were superfluous).
+#     bug-1910: `-mod=readonly` made Go ignore the committed
+#     src/frontend/apiserver/vendor/ tree and need a populated module
+#     cache, so with GOPROXY=off the build failed on any machine without
+#     one. The build now uses `-mod=vendor`, as the canonical meson build
+#     does (GOFLAGS=-mod=vendor in src/frontend/apiserver/meson.build);
+#     vendor mode never touches go.mod or go.sum either, so the bug-1607
+#     no-repo-mutation property still holds.
 #
 # Usage:
 #   bash src/tools/test-version-startup-consistency.sh [--keep-artifacts]
@@ -216,7 +227,17 @@ extract_zap_fields() {
 # behaviour.
 check_consistency() {
     local label="$1" v_commit="$2" v_build="$3" l_commit="$4" l_build="$5"
-    local ok=1
+    local ok=1 v_shown="$2"
+
+    # bug-1909: dirty-content-fingerprint (requirements.md, scope note and
+    # 2.2) deliberately shows '<sha>-<8-hex fingerprint>' in --version on a
+    # dirty tree while the startup log keeps '<sha>-dirty'. Treat exactly
+    # that pairing -- same base commit, '-dirty' in the log -- as agreement.
+    if [[ "$v_commit" =~ ^(.+)-[0-9a-f]{8}$ ]] \
+        && [ "$l_commit" = "${BASH_REMATCH[1]}-dirty" ]; then
+        yellow "[$label] --version fingerprint '$v_commit' pairs with startup log '$l_commit' (dirty-content-fingerprint)"
+        v_commit="$l_commit"
+    fi
 
     if [ -z "$v_commit" ] || [ -z "$l_commit" ]; then
         red "FAIL [$label]: commit id missing from one or both captures (--version='$v_commit' startup-log='$l_commit')"
@@ -235,7 +256,7 @@ check_consistency() {
     fi
 
     if [ "$ok" = 1 ]; then
-        green "PASS [$label]: --version and startup log agree -- commit='$v_commit' build_time='$v_build'"
+        green "PASS [$label]: --version and startup log agree -- commit='$v_shown' (startup log '$l_commit') build_time='$v_build'"
         return 0
     fi
     return 1
@@ -275,6 +296,24 @@ demonstrate_detection_capability() {
     fi
     green "Confirmed: an empty field on one side is correctly rejected."
 
+    yellow "--- (e) dirty tree: --version '<sha>-<8-hex fingerprint>' with startup log '<sha>-dirty' is the documented dirty-content-fingerprint pairing and must PASS ---"
+    if ! check_consistency "DEMO-dirty-fingerprint-pairing" "a1b2c3d4-0f1e2d3c" "$real_build" "a1b2c3d4-dirty" "$real_build"; then
+        red "DEMONSTRATION FAILED: incorrectly FAILED the documented dirty-fingerprint pairing."
+        exit 1
+    fi
+    green "Confirmed: the documented dirty-fingerprint pairing is accepted."
+
+    yellow "--- (f) the pairing must not hide a real mismatch: different base commit, or a fingerprint with a clean log ---"
+    if check_consistency "DEMO-dirty-pairing-other-sha" "a1b2c3d4-0f1e2d3c" "$real_build" "deadbeef-dirty" "$real_build"; then
+        red "DEMONSTRATION FAILED: incorrectly PASSED a fingerprint against a different commit's -dirty."
+        exit 1
+    fi
+    if check_consistency "DEMO-fingerprint-clean-log" "a1b2c3d4-0f1e2d3c" "$real_build" "a1b2c3d4" "$real_build"; then
+        red "DEMONSTRATION FAILED: incorrectly PASSED a fingerprint against a clean startup-log commit."
+        exit 1
+    fi
+    green "Confirmed: the pairing is accepted only for the same base commit with '-dirty' in the log."
+
     yellow "--- (d) sanity: genuinely matching real values must PASS ---"
     if ! check_consistency "DEMO-sanity-real-match" "$real_commit" "$real_build" "$real_commit" "$real_build"; then
         red "DEMONSTRATION FAILED: incorrectly FAILED genuinely matching real values."
@@ -386,7 +425,7 @@ cd '$SRC_DIR/frontend/apiserver'
 CGO_CFLAGS=\"-g \$(pkg-config --cflags librtti c-capnproto openssl tinylog) -I$SRC_DIR/include -I$APILIB_DIR/lib\"
 CGO_LDFLAGS=\"-g \$(pkg-config --libs librtti c-capnproto openssl tinylog) -lm -lrt -L$APILIB_DIR/lib -lxce\"
 GOPROXY=off CGO_ENABLED=1 CGO_CFLAGS=\"\$CGO_CFLAGS\" CGO_LDFLAGS=\"\$CGO_LDFLAGS\" \
-  go build -mod=readonly -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME\" \
+  go build -mod=vendor -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME\" \
   -o $APISERVER_BIN ./cmd/main.go
 "
 
@@ -502,7 +541,7 @@ else
 fi
 
 if [ "$overall_pass" = 1 ]; then
-    green "=== ALL CHECKS PASS: xce_scheduler and xce_apiserver each report byte-identical commit id + build time between --version/-version and their startup log ==="
+    green "=== ALL CHECKS PASS: xce_scheduler and xce_apiserver each report the same commit id (or the sanctioned dirty-fingerprint pairing) and byte-identical build time between --version/-version and their startup log ==="
     exit 0
 else
     red "=== INCONSISTENCY DETECTED: see FAIL line(s) above ==="

src/tools/verify-controller-peers-env.sh

@@ -498,6 +498,20 @@ case_controller_env_own_name_present() {
 add_case case_controller_env_own_name_present \
     "controller: env list containing the own name is accepted (1.6)"
 
+# bug-1912: spaces after the commas (natural in a shell variable) must not
+# become part of a candidate's name, so --myname b still matches " b=...".
+case_controller_env_spaced_list() {
+    local d; d="$(new_scratch controller-env-spaced)"
+    run_controller "$d" \
+        "XCE_CONTROLLER_PEERS=a=localhost:1, b=localhost:2, c=localhost:3" \
+        -- --myname b
+    _assert_stopped_early "$d" || return 1
+    assert_grep "$d/controller.log" 'HA mode: 3 controller candidates, identity=b' || return 1
+    assert_no_grep "$d/stderr" "$OWN_NAME_RE" "stderr" || return 1
+}
+add_case case_controller_env_spaced_list \
+    "controller: spaces around env entries are ignored (bug-1912)"
+
 case_controller_flag_own_name_unchecked() {
     local d; d="$(new_scratch controller-own-flag)"
     run_controller "$d" -u XCE_CONTROLLER_PEERS -- --myname zz --peers "$ENV3"
@@ -798,6 +812,20 @@ case_worker_help_names_variable() {
 add_case case_worker_help_names_variable \
     "worker: -h documents XCE_CONTROLLER_PEERS, the opt-out and that sudo drops it (1.5, 1.7)"
 
+# bug-1911: -h and --version after -l must exit 0 without a LeakSanitizer
+# report (the -l string used to leak on those early returns).
+case_worker_help_version_after_logdir_no_leak() {
+    local d opt
+    for opt in -h --version; do
+        d="$(new_scratch "worker-early-exit${opt}")"
+        run_worker "$d" -u XCE_CONTROLLER_PEERS -- "$opt"
+        assert_rc 0 || return 1
+        assert_no_grep "$d/stderr" 'LeakSanitizer' "stderr ($opt after -l)" || return 1
+    done
+}
+add_case case_worker_help_version_after_logdir_no_leak \
+    "worker: -h / --version after -l exit 0 without a leak (bug-1911)"
+
 # ── Task 3.3 (Req 1.1-1.5, 1.7): the harness (xce_test) takes its `masters`
 # default from XCE_CONTROLLER_PEERS; an explicit `set masters` wins. Always
 # run with --norc so the developer's own rc file cannot change the result,

src/worker/src/main.c

@@ -438,6 +438,7 @@ int main(int argc, char* argv[])
              * version_buf -- the startup INFO log lines further down keep
              * reading the original version_buf, untouched. */
             printf("%s\n", version_display_buf);
+            FREEUP(logdir); /* bug-1911: -l may have allocated it */
             return 0;
         case 'h':
         default:
@@ -460,6 +461,7 @@ int main(int argc, char* argv[])
                    "                preserved, e.g. "
                    "sudo --preserve-env=XCE_CONTROLLER_PEERS xcecd ...\n",
                    argv[0]);
+            FREEUP(logdir); /* bug-1911: -l may have allocated it */
             return 0;
         }
     }