From 5ca5f3291fa51ca2f820242d087cdbe19f4f92dd Mon Sep 17 00:00:00 2001
From: Shane McCarron <shane.mccarron@corvexconnect.com>
Date: Sat, 21 Mar 2026 12:37:28 -0500
Subject: [PATCH] fix(mcp): address QA round 1 findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Use O_NONBLOCK + clearerr() in Phase 2 fgetc probe to preserve the
  60s idle eviction timeout when both kernel fd and FILE* buffer are
  empty (fgetc on a blocking fd would otherwise block indefinitely,
  bypassing Phase 3 poll timeout and preventing cbm_mcp_server_evict_idle)
- Add #include <fcntl.h> for fcntl()/O_NONBLOCK
- Fix comment: "two-phase" → "three-phase" (implementation has 3 phases)
- Improve Python integration test: verify id:1 (initialize) and id:2
  (tools/list) response IDs are both present, not just "tools" substring

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 scripts/test_mcp_rapid_init.py | 22 +++++++++++++++++++++-
 src/mcp/mcp.c                  | 25 +++++++++++++++++++++----
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/scripts/test_mcp_rapid_init.py b/scripts/test_mcp_rapid_init.py
index d2c5f1434..fe4f65d5f 100755
--- a/scripts/test_mcp_rapid_init.py
+++ b/scripts/test_mcp_rapid_init.py
@@ -65,8 +65,28 @@ def main():
 
     output = stdout_data.decode("utf-8", errors="replace")
 
+    # Expect exactly 2 JSON responses: id:1 (initialize) and id:2 (tools/list).
+    # notifications/initialized has no id and produces no response.
+    lines = [ln.strip() for ln in output.splitlines() if ln.strip()]
+    import json as _json
+    json_lines = []
+    for ln in lines:
+        try:
+            json_lines.append(_json.loads(ln))
+        except _json.JSONDecodeError:
+            pass
+
+    ids = {obj.get("id") for obj in json_lines if "id" in obj}
+    if 1 not in ids:
+        print("FAIL: missing initialize response (id:1) in server output")
+        print(f"Server output was:\n{output!r}")
+        sys.exit(1)
+    if 2 not in ids:
+        print("FAIL: missing tools/list response (id:2) in server output")
+        print(f"Server output was:\n{output!r}")
+        sys.exit(1)
     if "tools" not in output:
-        print("FAIL: tools/list response not found in server output")
+        print("FAIL: tools/list response body missing 'tools' key")
         print(f"Server output was:\n{output!r}")
         sys.exit(1)
 
diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c
index 7757deabf..0729d0fa5 100644
--- a/src/mcp/mcp.c
+++ b/src/mcp/mcp.c
@@ -28,6 +28,7 @@
 #include <sys/unistd.h>
 #include <sys/poll.h>
 #include <poll.h>
+#include <fcntl.h>
 #endif
 #include <yyjson/yyjson.h>
 #include <stdint.h> // int64_t
@@ -2383,10 +2384,14 @@ int cbm_mcp_server_run(cbm_mcp_server_t *srv, FILE *in, FILE *out) {
          * empty kernel fd and block for STORE_IDLE_TIMEOUT_S seconds even
          * though the next messages are already in the FILE* buffer.
          *
-         * Fix (Unix): use a two-phase approach —
+         * Fix (Unix): use a three-phase approach —
          *   Phase 1: non-blocking poll (timeout=0) to check the kernel fd.
          *   Phase 2: if Phase 1 returns 0, peek the FILE* buffer via fgetc/
          *            ungetc to detect data buffered by a prior getline() call.
+         *            The fd is temporarily set O_NONBLOCK so fgetc() returns
+         *            immediately (EAGAIN → EOF + ferror) instead of blocking
+         *            when the FILE* buffer is empty, which would otherwise
+         *            bypass the Phase 3 idle eviction timeout.
          *   Phase 3: only if both phases confirm no data, do blocking poll. */
 #ifdef _WIN32
         /* Windows: WaitForSingleObject on stdin handle */
@@ -2414,14 +2419,26 @@ int cbm_mcp_server_run(cbm_mcp_server_t *srv, FILE *in, FILE *out) {
         if (pr == 0) {
             /* Raw fd appears empty. Check whether libc has already buffered
              * data from a previous over-read by peeking one byte via fgetc.
-             * If successful, push it back and proceed to getline without
-             * blocking. If not (EOF or EAGAIN), do the blocking poll. */
+             * IMPORTANT: temporarily set O_NONBLOCK so fgetc() returns
+             * immediately when the FILE* buffer AND kernel fd are both empty
+             * (EAGAIN → EOF + ferror). Without this, fgetc() on a blocking fd
+             * would block indefinitely, preventing the Phase 3 idle eviction
+             * timeout from ever firing. */
+            int saved_flags = fcntl(fd, F_GETFL);
+            if (saved_flags >= 0) {
+                (void)fcntl(fd, F_SETFL, saved_flags | O_NONBLOCK);
+            }
             int c = fgetc(in);
+            if (saved_flags >= 0) {
+                (void)fcntl(fd, F_SETFL, saved_flags); /* restore blocking */
+            }
             if (c == EOF) {
                 if (feof(in)) {
                     break; /* true EOF */
                 }
-                /* No buffered data and fd is empty — do blocking poll */
+                /* No buffered data (EAGAIN from non-blocking read) — clear the
+                 * ferror indicator set by EAGAIN, then do the blocking poll. */
+                clearerr(in);
                 pr = poll(&pfd, 1, STORE_IDLE_TIMEOUT_S * 1000);
                 if (pr < 0) {
                     break;