feat(libxce-failover): capture redirect hint in handle_reply (before free, before status return); verify endpoint-error wake (task 2.3)

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

@@ -29,7 +29,7 @@
   - _Requirements: 2.1, 2.2, 2.3, 4.4, 8.4_
   - _Boundary: EndpointRotator_
   - _Depends: 1.1_
-- [ ] 2.3 (P) Capture redirect on the reply path and wake the wait on endpoint error
+- [x] 2.3 (P) Capture redirect on the reply path and wake the wait on endpoint error
   - In the reply handler, copy the redirect-hint host/port into the client reply context when a redirect reply carries one, before the reply is freed; leave empty otherwise (advisory hint, no sequence correlation)
   - On endpoint error or peer reset, release any pending reply wait with a transport error so the in-flight call returns instead of hanging
   - Keep the status flowing through the existing wait-channel path so the public API layer stays unchanged
@@ -103,3 +103,4 @@
 - 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).
+- 2.3: RedirectCapture via non-static seam `xce_ctx_capture_redirect_hint()` called in handle_reply BEFORE the capnp free AND before the `status<0` early return (so negative E_XCE_REDIRECT still captures). Bounded strncpy into redirect_hint_host. ReplyWaitWake needed NO production change — existing `handle_disconnect` (err_handler, xcectx.c) already injects -ECONNRESET into wbuf + sets XCE_CTX_ERR; full err_handler path is covered by E2E (6.2).

src/include/xcectx.h

@@ -59,4 +59,19 @@ extern int  xce_ctx_set_endpoint(xce_ctx_t*, endpoint_t*);
 extern void xce_block_tasks(xce_ctx_t*);
 extern void xce_unblock_tasks(xce_ctx_t*);
 
+/*
+ * xce_ctx_capture_redirect_hint: copy a redirect address carried on a reply
+ * into the ctx's redirect_hint_host/port fields as a best-effort failover hint
+ * (Req 2.2, 8.3). Called by handle_reply() BEFORE the capnp-owned reply (and
+ * its redirect_host string) is freed.
+ *
+ * When `host` is NULL/empty or `port` is 0 there is no usable address: the hint
+ * is left untouched (it stays empty). Otherwise the host is bounded-copied into
+ * the fixed-size field (truncated + NUL-terminated, never overflowing) and the
+ * port is stored. A NULL ctx is a safe no-op. Advisory only — correctness never
+ * depends on the hint.
+ */
+extern void xce_ctx_capture_redirect_hint(xce_ctx_t*, const char* host,
+                                          uint16_t port);
+
 #endif

src/lib/testsuite/test_libxce_retry.c

@@ -617,6 +617,150 @@ static void test_next_target_rejects_bad_args(void)
     printf("test_next_target_rejects_bad_args: PASS\n");
 }
 
+/* ════════════════════════════════════════════════════════════════════════
+ * Task 2.3 — RedirectCapture (xce_ctx_capture_redirect_hint) RED→GREEN tests.
+ *
+ * handle_reply() copies the redirect address carried on a redirect reply into
+ * the ctx's redirect_hint_host/port BEFORE the capnp-owned reply is freed. The
+ * copy logic is factored into a non-static seam, xce_ctx_capture_redirect_hint,
+ * so it can be unit-tested without a live server.
+ *
+ * Requirements: 2.2 (capture redirect addr as a best-effort hint), 8.3 (try the
+ * hinted address first — capture is the prerequisite).
+ * ════════════════════════════════════════════════════════════════════════ */
+
+/* Build a bare xce_ctx_t with an empty redirect hint, without touching any
+ * iomgr/ssl/endpoint state (capture is pure ctx-field manipulation). */
+static void fo_ctx_clear_hint(xce_ctx_t *ctx)
+{
+    memset(ctx, 0, sizeof(*ctx));
+    ctx->redirect_hint_host[0] = '\0';
+    ctx->redirect_hint_port    = 0;
+}
+
+/* Req 2.2, 8.3: a present host+port is copied into the ctx hint fields. */
+static void test_capture_redirect_hint_present(void)
+{
+    xce_ctx_t ctx;
+    fo_ctx_clear_hint(&ctx);
+
+    xce_ctx_capture_redirect_hint(&ctx, "10.40.0.5", 9300);
+
+    TEST_ASSERT_EQUAL_STRING("10.40.0.5", ctx.redirect_hint_host);
+    TEST_ASSERT_EQUAL_INT(9300, ctx.redirect_hint_port);
+
+    printf("test_capture_redirect_hint_present: PASS\n");
+}
+
+/* Req 2.2: when no address is present (host NULL/empty or port 0) the hint is
+ * left empty — capture must not invent an address. */
+static void test_capture_redirect_hint_absent_stays_empty(void)
+{
+    xce_ctx_t ctx;
+
+    /* NULL host → hint untouched */
+    fo_ctx_clear_hint(&ctx);
+    xce_ctx_capture_redirect_hint(&ctx, NULL, 9300);
+    TEST_ASSERT_EQUAL_STRING("", ctx.redirect_hint_host);
+    TEST_ASSERT_EQUAL_INT(0, ctx.redirect_hint_port);
+
+    /* empty host → hint untouched */
+    fo_ctx_clear_hint(&ctx);
+    xce_ctx_capture_redirect_hint(&ctx, "", 9300);
+    TEST_ASSERT_EQUAL_STRING("", ctx.redirect_hint_host);
+    TEST_ASSERT_EQUAL_INT(0, ctx.redirect_hint_port);
+
+    /* port 0 → hint untouched (no usable target) */
+    fo_ctx_clear_hint(&ctx);
+    xce_ctx_capture_redirect_hint(&ctx, "10.40.0.5", 0);
+    TEST_ASSERT_EQUAL_STRING("", ctx.redirect_hint_host);
+    TEST_ASSERT_EQUAL_INT(0, ctx.redirect_hint_port);
+
+    printf("test_capture_redirect_hint_absent_stays_empty: PASS\n");
+}
+
+/* Req 2.2: an over-long host is bounded-copied (NUL-terminated, no overflow);
+ * the capnp-owned string the controller may send is not length-checked here. */
+static void test_capture_redirect_hint_truncates_long_host(void)
+{
+    xce_ctx_t ctx;
+    fo_ctx_clear_hint(&ctx);
+
+    char longhost[XCE_MAXHOSTNAME_LEN * 2];
+    memset(longhost, 'h', sizeof(longhost) - 1);
+    longhost[sizeof(longhost) - 1] = '\0';
+
+    xce_ctx_capture_redirect_hint(&ctx, longhost, 9400);
+
+    /* copied, NUL-terminated, and never longer than the field bound */
+    TEST_ASSERT_EQUAL_INT(XCE_MAXHOSTNAME_LEN - 1,
+                          (int)strlen(ctx.redirect_hint_host));
+    TEST_ASSERT_EQUAL_INT('\0',
+                          ctx.redirect_hint_host[XCE_MAXHOSTNAME_LEN - 1]);
+    TEST_ASSERT_EQUAL_INT(9400, ctx.redirect_hint_port);
+
+    printf("test_capture_redirect_hint_truncates_long_host: PASS\n");
+}
+
+/* Guard: a NULL ctx is rejected without crashing. */
+static void test_capture_redirect_hint_null_ctx_safe(void)
+{
+    /* must not crash */
+    xce_ctx_capture_redirect_hint(NULL, "10.40.0.5", 9300);
+    printf("test_capture_redirect_hint_null_ctx_safe: PASS\n");
+}
+
+/* ════════════════════════════════════════════════════════════════════════
+ * Task 2.3 — ReplyWaitWake RED→GREEN test.
+ *
+ * On an endpoint error/peer reset, the existing disconnect path injects a
+ * transport error into ctx->wbuf so a blocked cbuf_get returns instead of
+ * hanging. This test proves the wake mechanism end-to-end on a real wbuf: a
+ * thread blocks in cbuf_get, the "disconnect" injects -ECONNRESET, and the
+ * blocked waiter wakes with exactly that transport error (Req 4.4).
+ * ════════════════════════════════════════════════════════════════════════ */
+
+typedef struct {
+    cbuf_t wbuf;
+    int    woke_status; /* what the blocked cbuf_get observed */
+    int    got;         /* cbuf_get return code               */
+} fo_wake_ctx_t;
+
+static void fo_wake_waiter(void *p, int idx)
+{
+    fo_wake_ctx_t *w = (fo_wake_ctx_t *)p;
+    if (idx == 0) {
+        /* waiter: block until the disconnect injects an error */
+        w->got = cbuf_get(&w->wbuf, &w->woke_status);
+    } else {
+        /* injector: mirror handle_disconnect's wbuf error injection. A short
+         * spin lets the waiter reach cbuf_get first so we exercise the wake,
+         * not a pre-filled buffer. */
+        for (volatile int i = 0; i < 2000000; i++) {
+        }
+        int err = -ECONNRESET;
+        cbuf_tryput(&w->wbuf, &err);
+    }
+}
+
+/* Req 4.4: a pending cbuf_get is released with the injected transport error so
+ * an in-flight xce_* call returns instead of hanging across an endpoint reset. */
+static void test_reply_wait_wake_releases_blocked_get(void)
+{
+    fo_wake_ctx_t w;
+    memset(&w, 0, sizeof(w));
+    TEST_ASSERT_EQUAL_INT(0, cbuf_init(&w.wbuf, 1, sizeof(int)));
+
+    int rc = fo_run_threads(2, fo_wake_waiter, &w);
+    cbuf_fini(&w.wbuf);
+
+    TEST_ASSERT_EQUAL_INT(0, rc);
+    TEST_ASSERT_TRUE(w.got >= 0);            /* cbuf_get returned, did not hang */
+    TEST_ASSERT_EQUAL_INT(-ECONNRESET, w.woke_status);
+
+    printf("test_reply_wait_wake_releases_blocked_get: PASS\n");
+}
+
 /* ── Suite entry point ──────────────────────────────────────────────── */
 
 void test_libxce_retry(void)
@@ -638,5 +782,10 @@ void test_libxce_retry(void)
     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);
+    RUN_TEST(test_capture_redirect_hint_present);
+    RUN_TEST(test_capture_redirect_hint_absent_stays_empty);
+    RUN_TEST(test_capture_redirect_hint_truncates_long_host);
+    RUN_TEST(test_capture_redirect_hint_null_ctx_safe);
+    RUN_TEST(test_reply_wait_wake_releases_blocked_get);
     UNITY_END();
 }

src/lib/xcectx.c

@@ -27,10 +27,36 @@ static int handle_sub_reply(xce_ctx_t* ctx, xce_reply_t* reply)
     return 1;
 }
 
+void xce_ctx_capture_redirect_hint(xce_ctx_t* ctx, const char* host,
+                                   uint16_t port)
+{
+    if (ctx == NULL) {
+        return;
+    }
+
+    /* No usable redirect address → leave the hint untouched (empty). The
+     * capnp-owned host pointer may be NULL; an empty host or a zero port is
+     * likewise "no address". Advisory only — correctness never depends on it. */
+    if ((host == NULL) || (host[0] == '\0') || (port == 0)) {
+        return;
+    }
+
+    strncpy(ctx->redirect_hint_host, host, XCE_MAXHOSTNAME_LEN - 1);
+    ctx->redirect_hint_host[XCE_MAXHOSTNAME_LEN - 1] = '\0';
+    ctx->redirect_hint_port                          = port;
+}
+
 static int handle_reply(xce_ctx_t* ctx, xce_reply_t* reply)
 {
     int rc = 0;
 
+    /* Capture the redirect address (if any) as a best-effort failover hint
+     * BEFORE the capnp-owned reply — including reply->redirect_host — is freed
+     * by the caller (Req 2.2, 8.3). The status still flows through the existing
+     * cbuf_tryput(&ctx->wbuf,&status) path unchanged, so xceapi.c is untouched. */
+    xce_ctx_capture_redirect_hint(ctx, reply->redirect_host,
+                                  reply->redirect_port);
+
     if (reply->status < 0) {
         return reply->status;
     }