feat(libxce-failover): endpoint rotator — hint-first target selection + master rotation; keep api_ctx_t decls private (task 2.2)

.kiro/specs/libxce-failover/tasks.md

@@ -22,7 +22,7 @@
   - Done: classification unit tests pass for every status and for both duplicate-replay cases
   - _Requirements: 2.5, 4.1, 4.2, 4.3, 4.5, 5.2_
   - _Boundary: StatusClassifier_
-- [ ] 2.2 Implement endpoint reconnect and master rotation
+- [x] 2.2 Implement endpoint reconnect and master rotation
   - On rotate, try the captured redirect-hint address first when one is set (then clear it); otherwise advance the master cursor to the next configured master and reconnect
   - Close the stale connection first and reuse the same endpoint object so reply routing stays intact; a failed hint simply falls through to the next master
   - Done: rotation unit tests show hint-first then next-master fallback, reusing a single endpoint object, with correctness independent of the hint
@@ -102,3 +102,4 @@
 ## Implementation Notes
 - 1.1: design said `XCE_MAXHOST_LEN`; real constant is `XCE_MAXHOSTNAME_LEN` (config.h, =256) — design corrected. `api_ctx_t` now in private `client_internal.h`; `xcecli_reconnect_cb` typedef lives there until task 4.1 adds the public setter. Build: `cd src && nix develop --command bash tools/xce-build.sh lib`.
 - 1.2 (ADAPTED): RED-against-undefined-symbols would break linking of the shared lib test binary across tasks, so 1.2 builds a GREEN reusable harness (`src/lib/testsuite/failover_harness.h`): fake multi-master config+cursor, an 11-row controller-status taxonomy table (resolves to real errnox.h codes), and an N-pthread runner. Component RED→GREEN tests live in 2.1/2.2/2.3/3.1/3.2. The harness defines `fo_expected_action_t` (FO_ACT_*) mirroring the design `xce_fo_action_t` — 2.1 asserts the real classifier against these rows.
+- 2.2: EndpointRotator `xce_failover_next_target` (pure, socket-free: hint-first/consume, else cursor advance+wrap over conf->masters[]) + thin `xce_failover_rotate` (close-old/reconnect same endpoint). Live connect path is E2E (6.2). LAYERING: functions taking the private `api_ctx_t` are declared in `src/lib/client_internal.h`, NOT the public `xce/client_failover.h` (which keeps only the pure int-typed classifier) — per src/lib/CLAUDE.md (public headers in src/include/xce/ must not pull private state).

src/include/xce/client_failover.h

@@ -46,4 +46,10 @@ typedef enum {
  */
 xce_fo_action_t xce_failover_classify(int status, bool is_write, int attempt);
 
+/*
+ * EndpointRotator (xce_failover_next_target / xce_failover_rotate) operates on
+ * the private api_ctx_t and is therefore declared in the internal header
+ * src/lib/client_internal.h, not in this public surface.
+ */
+
 #endif /* XCE_CLIENT_FAILOVER_H */

src/lib/client_failover.c

@@ -1,15 +1,26 @@
 /*
- * client_failover.c — StatusClassifier for the libxce-failover spec (task 2.1).
+ * client_failover.c — StatusClassifier (task 2.1) + EndpointRotator (task 2.2)
+ * for the libxce-failover spec.
  *
- * Pure mapping from a reply status / transport rc to a failover action. No I/O,
+ * StatusClassifier: pure mapping from a reply status / transport rc to a
- * no globals. See design.md "Components and Interfaces → StatusClassifier" and
+ * failover action. No I/O, no globals (requirements 2.5, 4.1-4.3, 4.5, 5.2).
- * requirements 2.5, 4.1, 4.2, 4.3, 4.5, 5.2.
+ *
+ * EndpointRotator: choose and connect the next failover target. The target
+ * SELECTION (xce_failover_next_target) is a pure, socket-free helper so it is
+ * unit-testable; xce_failover_rotate() is a thin wrapper that closes the stale
+ * connection and reconnects the same endpoint object (requirements 2.1, 2.2,
+ * 2.3, 4.4, 8.4). See design.md "Components and Interfaces → EndpointRotator".
  */
 #define _GNU_SOURCE
 #include "xce/client_failover.h"
 
 #include <errno.h>
+#include <string.h>
+#include <sys/socket.h>
 
+#include "client_internal.h" /* api_ctx_t + EndpointRotator declarations */
+#include "config.h"          /* XCE_MAXHOSTNAME_LEN */
+#include "netio.h"           /* endpoint_init / endpoint_connect / endpoint_fini */
 #include "xce/errnox.h"
 
 /*
@@ -58,3 +69,89 @@ xce_fo_action_t xce_failover_classify(int status, bool is_write, int attempt)
      * status — is non-retriable and surfaced to the caller (4.5). */
     return XCE_FO_TERMINAL;
 }
+
+/* ════════════════════════════════════════════════════════════════════════
+ * EndpointRotator — target selection (pure) + connect wrapper (thin)
+ * ════════════════════════════════════════════════════════════════════════ */
+
+/* True if a redirect-address hint is currently set on the ctx. The hint is a
+ * NUL-terminated host; an empty host ("") means "no hint" (xcectx.h). */
+static bool fo_has_hint(const api_ctx_t *ac)
+{
+    return ac->ctx.redirect_hint_host[0] != '\0';
+}
+
+/* Clear (consume) the redirect-address hint on the ctx. One-shot + advisory. */
+static void fo_clear_hint(api_ctx_t *ac)
+{
+    ac->ctx.redirect_hint_host[0] = '\0';
+    ac->ctx.redirect_hint_port    = 0;
+}
+
+int xce_failover_next_target(api_ctx_t *ac, char *host_out, size_t host_sz,
+                             uint16_t *port_out)
+{
+    if (ac == NULL || host_out == NULL || host_sz == 0 || port_out == NULL)
+        return -EINVAL;
+
+    /* Hint-first (2.2, 8.3): if the last reply carried a redirect address, try
+     * it first and CONSUME it. The cursor is intentionally left untouched so a
+     * failed hint falls through to normal rotation on the next call (4.4) —
+     * correctness never depends on the hint. */
+    if (fo_has_hint(ac)) {
+        strncpy(host_out, ac->ctx.redirect_hint_host, host_sz - 1);
+        host_out[host_sz - 1] = '\0';
+        *port_out             = ac->ctx.redirect_hint_port;
+        fo_clear_hint(ac);
+        return 0;
+    }
+
+    /* No hint: advance the sticky master cursor to the NEXT configured master,
+     * wrapping around (2.1, 8.4). */
+    if (ac->conf == NULL || ac->conf->nmasters <= 0)
+        return -ENOENT;
+
+    int next = (ac->failover.cur_master + 1) % ac->conf->nmasters;
+    ac->failover.cur_master = next;
+
+    const char *host = ac->conf->masters[next];
+    if (host == NULL)
+        return -ENOENT;
+
+    strncpy(host_out, host, host_sz - 1);
+    host_out[host_sz - 1] = '\0';
+    *port_out             = (uint16_t)ac->conf->master_ports[next];
+    return 0;
+}
+
+int xce_failover_rotate(api_ctx_t *ac)
+{
+    if (ac == NULL)
+        return -EINVAL;
+
+    char     host[XCE_MAXHOSTNAME_LEN] = { 0 };
+    uint16_t port                      = 0;
+
+    int rc = xce_failover_next_target(ac, host, sizeof(host), &port);
+    if (rc < 0)
+        return rc;
+
+    /* Close the stale connection FIRST, then reconnect the SAME endpoint
+     * object so reply routing to ctx->wbuf (via ctx->ep == &ac->ep) is
+     * preserved. endpoint_fini() closes the fd, frees the SSL session, and
+     * clears handlers/peer; endpoint_init() recreates the socket fd and
+     * re-arms the object before endpoint_connect() drives the handshake.
+     *
+     * The live socket/handshake path is exercised by the E2E task (6.2); this
+     * wrapper keeps the I/O surface thin around the pure selector above. */
+    endpoint_fini(&ac->ep);
+
+    rc = endpoint_init(&ac->ep, AF_INET, SOCK_STREAM, 0, 0);
+    if (rc < 0)
+        return rc;
+
+    endpoint_set_peer(&ac->ep, &ac->ctx.myinfo);
+
+    rc = endpoint_connect(&ac->ep, host, (int)port);
+    return rc < 0 ? rc : 0;
+}

src/lib/client_internal.h

@@ -2,6 +2,7 @@
 
 #define _CLIENT_INTERNAL_H_ 1
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include "client.h"
@@ -60,4 +61,23 @@ typedef struct {
     xce_failover_state_t failover;
 } api_ctx_t;
 
+/*
+ * EndpointRotator (task 2.2) — these operate on the private api_ctx_t, so they
+ * live in this internal header rather than the public xce/client_failover.h.
+ *
+ * xce_failover_next_target(): PURE target-selection (no I/O). If a redirect-
+ * address hint is set on ac->ctx, write it out and CONSUME it (one-shot,
+ * advisory). Otherwise advance the sticky master cursor (ac->failover.cur_master)
+ * to the next configured master in ac->conf, wrapping around. Returns 0,
+ * -EINVAL on a NULL/zero argument, or -ENOENT if no masters and no hint.
+ * Correctness never depends on the hint.
+ *
+ * xce_failover_rotate(): close the stale connection on ac->ep and reconnect the
+ * SAME endpoint object to the chosen target. Thin wrapper over the selector +
+ * endpoint_connect(); the live socket path is exercised by the E2E task.
+ */
+int xce_failover_next_target(api_ctx_t* ac, char* host_out, size_t host_sz,
+                             uint16_t* port_out);
+int xce_failover_rotate(api_ctx_t* ac);
+
 #endif

src/lib/testsuite/test_libxce_retry.c

@@ -26,8 +26,11 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <stdint.h>
+
 #include <unity/unity.h>
 
+#include "client_internal.h"  /* api_ctx_t, xce_failover_state_t */
 #include "failover_harness.h"
 #include "request.h"
 #include "xce/client_failover.h"
@@ -450,6 +453,170 @@ static void test_classify_unknown_is_terminal(void)
     printf("test_classify_unknown_is_terminal: PASS\n");
 }
 
+/* ════════════════════════════════════════════════════════════════════════
+ * Task 2.2 — EndpointRotator target selection (xce_failover_next_target)
+ *            RED→GREEN tests. Pure, socket-free target-selection logic; the
+ *            live connect path is covered by E2E task 6.2.
+ * ════════════════════════════════════════════════════════════════════════ */
+
+/* Build a 3-master client config (distinct host:port) the rotator selects over.
+ * Strings are static-duration so the api_ctx may borrow them by pointer. */
+static void fo_make_client_masters(xce_client_config_t *conf)
+{
+    static char *hosts[N_MAX_MASTERS] = { "10.30.0.1", "10.30.0.2",
+                                          "10.30.0.3" };
+    static int   ports[N_MAX_MASTERS] = { 8001, 8002, 8003 };
+
+    memset(conf, 0, sizeof(*conf));
+    conf->nmasters = N_MAX_MASTERS;
+    for (int i = 0; i < N_MAX_MASTERS; i++) {
+        conf->masters[i]      = hosts[i];
+        conf->master_ports[i] = ports[i];
+    }
+}
+
+/* Minimal api_ctx for target-selection tests: only conf, the failover cursor,
+ * and the ctx redirect-hint fields are exercised. No endpoint/socket touched. */
+static void fo_make_api_ctx(api_ctx_t *ac, xce_client_config_t *conf)
+{
+    memset(ac, 0, sizeof(*ac));
+    ac->conf                = conf;
+    ac->failover.cur_master = 0;
+}
+
+static void fo_set_hint(api_ctx_t *ac, const char *host, uint16_t port)
+{
+    strncpy(ac->ctx.redirect_hint_host, host, XCE_MAXHOSTNAME_LEN - 1);
+    ac->ctx.redirect_hint_host[XCE_MAXHOSTNAME_LEN - 1] = '\0';
+    ac->ctx.redirect_hint_port                          = port;
+}
+
+/* Req 2.2, 8.4: with a redirect hint set, next_target returns the hint
+ * host/port and CONSUMES (clears) it; the following call ignores the stale
+ * hint and advances the master cursor instead. */
+static void test_next_target_hint_first_then_consumed(void)
+{
+    xce_client_config_t conf;
+    api_ctx_t           ac;
+    fo_make_client_masters(&conf);
+    fo_make_api_ctx(&ac, &conf);
+    fo_set_hint(&ac, "10.99.0.7", 9100);
+
+    char     host[XCE_MAXHOSTNAME_LEN] = { 0 };
+    uint16_t port                      = 0;
+
+    int rc = xce_failover_next_target(&ac, host, sizeof(host), &port);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+    TEST_ASSERT_EQUAL_STRING("10.99.0.7", host);
+    TEST_ASSERT_EQUAL_INT(9100, port);
+
+    /* hint consumed: cleared on the ctx, cursor untouched */
+    TEST_ASSERT_EQUAL_STRING("", ac.ctx.redirect_hint_host);
+    TEST_ASSERT_EQUAL_INT(0, ac.failover.cur_master);
+
+    /* next call has no hint → advances cursor to master 1 */
+    rc = xce_failover_next_target(&ac, host, sizeof(host), &port);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+    TEST_ASSERT_EQUAL_STRING("10.30.0.2", host);
+    TEST_ASSERT_EQUAL_INT(8002, port);
+    TEST_ASSERT_EQUAL_INT(1, ac.failover.cur_master);
+
+    printf("test_next_target_hint_first_then_consumed: PASS\n");
+}
+
+/* Req 2.1, 8.4: with no hint, successive next_target calls advance through the
+ * configured masters and WRAP around; the cursor lives on failover.cur_master. */
+static void test_next_target_rotates_and_wraps(void)
+{
+    xce_client_config_t conf;
+    api_ctx_t           ac;
+    fo_make_client_masters(&conf);
+    fo_make_api_ctx(&ac, &conf);
+
+    char     host[XCE_MAXHOSTNAME_LEN] = { 0 };
+    uint16_t port                      = 0;
+
+    /* from cur_master=0, first rotation advances to master 1 */
+    TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host),
+                                                      &port));
+    TEST_ASSERT_EQUAL_STRING("10.30.0.2", host);
+    TEST_ASSERT_EQUAL_INT(8002, port);
+    TEST_ASSERT_EQUAL_INT(1, ac.failover.cur_master);
+
+    TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host),
+                                                      &port));
+    TEST_ASSERT_EQUAL_STRING("10.30.0.3", host);
+    TEST_ASSERT_EQUAL_INT(8003, port);
+    TEST_ASSERT_EQUAL_INT(2, ac.failover.cur_master);
+
+    /* wrap back to master 0 */
+    TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host),
+                                                      &port));
+    TEST_ASSERT_EQUAL_STRING("10.30.0.1", host);
+    TEST_ASSERT_EQUAL_INT(8001, port);
+    TEST_ASSERT_EQUAL_INT(0, ac.failover.cur_master);
+
+    printf("test_next_target_rotates_and_wraps: PASS\n");
+}
+
+/* Req 4.4, 8.4: after a hint is consumed, the very next call advances to the
+ * next master — correctness is independent of the (one-shot) hint. */
+static void test_next_target_hint_then_fallback_rotation(void)
+{
+    xce_client_config_t conf;
+    api_ctx_t           ac;
+    fo_make_client_masters(&conf);
+    fo_make_api_ctx(&ac, &conf);
+    /* start the cursor at master 1 to prove fallback advances from current */
+    ac.failover.cur_master = 1;
+    fo_set_hint(&ac, "10.99.0.9", 9200);
+
+    char     host[XCE_MAXHOSTNAME_LEN] = { 0 };
+    uint16_t port                      = 0;
+
+    /* hint first */
+    TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host),
+                                                      &port));
+    TEST_ASSERT_EQUAL_STRING("10.99.0.9", host);
+    TEST_ASSERT_EQUAL_INT(9200, port);
+    TEST_ASSERT_EQUAL_INT(1, ac.failover.cur_master); /* cursor untouched */
+
+    /* fallback: advances from master 1 → master 2 */
+    TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host),
+                                                      &port));
+    TEST_ASSERT_EQUAL_STRING("10.30.0.3", host);
+    TEST_ASSERT_EQUAL_INT(8003, port);
+    TEST_ASSERT_EQUAL_INT(2, ac.failover.cur_master);
+
+    printf("test_next_target_hint_then_fallback_rotation: PASS\n");
+}
+
+/* Guard: NULL/invalid arguments are rejected without touching state. */
+static void test_next_target_rejects_bad_args(void)
+{
+    xce_client_config_t conf;
+    api_ctx_t           ac;
+    fo_make_client_masters(&conf);
+    fo_make_api_ctx(&ac, &conf);
+
+    char     host[XCE_MAXHOSTNAME_LEN] = { 0 };
+    uint16_t port                      = 0;
+
+    TEST_ASSERT_EQUAL_INT(-EINVAL,
+                          xce_failover_next_target(NULL, host, sizeof(host),
+                                                   &port));
+    TEST_ASSERT_EQUAL_INT(-EINVAL,
+                          xce_failover_next_target(&ac, NULL, sizeof(host),
+                                                   &port));
+    TEST_ASSERT_EQUAL_INT(-EINVAL,
+                          xce_failover_next_target(&ac, host, 0, &port));
+    TEST_ASSERT_EQUAL_INT(-EINVAL,
+                          xce_failover_next_target(&ac, host, sizeof(host),
+                                                   NULL));
+
+    printf("test_next_target_rejects_bad_args: PASS\n");
+}
+
 /* ── Suite entry point ──────────────────────────────────────────────── */
 
 void test_libxce_retry(void)
@@ -467,5 +634,9 @@ void test_libxce_retry(void)
     RUN_TEST(test_classify_dup_replay_rule);
     RUN_TEST(test_classify_zero_is_success);
     RUN_TEST(test_classify_unknown_is_terminal);
+    RUN_TEST(test_next_target_hint_first_then_consumed);
+    RUN_TEST(test_next_target_rotates_and_wraps);
+    RUN_TEST(test_next_target_hint_then_fallback_rotation);
+    RUN_TEST(test_next_target_rejects_bad_args);
     UNITY_END();
 }