Merge pull request DeusData#1369 from WarGloom/agent/fix-worker-error-transport

fix(worker): preserve delivered MCP errors

Author: DeusData

scripts/test.sh

@@ -276,6 +276,12 @@ CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_parent_watchdog.sh"
 echo "=== Step 5b: worker-mode watchdog regression (#845) ==="
 CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_worker_watchdog.sh"
 
+# Step 5c: a worker-delivered MCP error is transport success. The outer CLI
+# still exits nonzero for the user-facing tool error, but the supervisor must
+# preserve that response instead of misreporting exit_nonzero as a file crash.
+echo "=== Step 5c: worker error-response transport regression ==="
+bash "$ROOT/tests/test_worker_error_response.sh"
+
 # Step 6: security-strings URL allow-list regression. The MSYS2 CLANG64 toolchain
 # bakes its package-tracker URL into the static Windows .exe; the binary string
 # audit must allow-list it (Windows-only — Linux smoke never saw it).

src/main.c

@@ -829,11 +829,13 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo
         /* Supervised worker: hand the full result string to the parent via the
          * response file before printing (parent reads it back on a clean exit). */
         const char *ro = cbm_index_worker_response_out();
+        bool worker_response_written = false;
         if (ro) {
             FILE *rf = cbm_fopen(ro, "wb");
             if (rf) {
-                (void)fputs(result, rf);
-                (void)fclose(rf);
+                int write_rc = fputs(result, rf);
+                int close_rc = fclose(rf);
+                worker_response_written = write_rc >= 0 && close_rc == 0;
             }
         }
         if (raw_json) {
@@ -847,14 +849,15 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo
         }
         exit_code = cbm_cli_exit_status_after_maintenance(exit_code, maintenance_cancelled);
         if (cbm_index_worker_active()) {
-            /* Supervised worker: the response is delivered (file + stdout).
-             * Skip the multi-GB teardown (server/store frees) — the process
-             * dies now and the OS reclaims everything wholesale; piecemeal
-             * free() of a kernel-scale graph costs minutes. _Exit skips
-             * atexit/LSan by design for this prod worker path. */
+            /* The supervisor protocol classifies the PROCESS, not the tool
+             * result: a valid MCP error response is a healthy worker outcome.
+             * Propagating cli_print_mcp_result's isError exit code made the
+             * parent discard that response and falsely report exit_nonzero as
+             * "crashed on a file". Fail only when the response transport itself
+             * failed. Skip multi-GB teardown; the OS reclaims it at exit. */
             cbm_log_info("index.worker.fast_exit", "action", "_Exit");
             fflush(NULL);
-            _Exit(exit_code);
+            _Exit(worker_response_written ? 0 : SKIP_ONE);
         }
         free(result);
     }

tests/test_worker_error_response.sh

@@ -0,0 +1,84 @@
+#!/usr/bin/env bash
+# A supervised worker that produces a valid MCP error response is healthy.
+# The CLI may exit nonzero when presenting that error to a human, but the worker
+# transport must exit zero so its parent reads the response instead of inventing
+# an exit_nonzero "crashed on a file" diagnosis.
+set -euo pipefail
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+BINARY="${ROOT}/build/c/codebase-memory-mcp"
+if [[ ! -x "${BINARY}" && -x "${BINARY}.exe" ]]; then
+  BINARY="${BINARY}.exe"
+fi
+
+if [[ ! -x "${BINARY}" ]]; then
+  echo "missing binary: ${BINARY}" >&2
+  exit 2
+fi
+
+if command -v shasum >/dev/null 2>&1; then
+  BUILD_FINGERPRINT="$(shasum -a 256 "${BINARY}" | awk '{print $1}')"
+elif command -v sha256sum >/dev/null 2>&1; then
+  BUILD_FINGERPRINT="$(sha256sum "${BINARY}" | awk '{print $1}')"
+elif command -v openssl >/dev/null 2>&1; then
+  BUILD_FINGERPRINT="$(openssl dgst -sha256 "${BINARY}" | awk '{print $NF}')"
+else
+  echo "no SHA-256 command available for worker build binding" >&2
+  exit 2
+fi
+if [[ ! "${BUILD_FINGERPRINT}" =~ ^[0-9a-f]{64}$ ]]; then
+  echo "invalid worker build fingerprint: ${BUILD_FINGERPRINT}" >&2
+  exit 2
+fi
+
+tmpdir="$(mktemp -d)"
+cleanup() {
+  CBM_CACHE_DIR="${tmpdir}/cache-supervisor" \
+    "${BINARY}" daemon stop >/dev/null 2>&1 || true
+  rm -rf "${tmpdir}"
+}
+trap cleanup EXIT
+
+missing="${tmpdir}/repository-does-not-exist"
+response="${tmpdir}/worker.response"
+args="{\"repo_path\":\"${missing}\",\"mode\":\"fast\"}"
+
+if ! CBM_CACHE_DIR="${tmpdir}/cache-worker" \
+  "${BINARY}" cli --index-worker \
+  --index-worker-build "${BUILD_FINGERPRINT}" \
+  index_repository "${args}" \
+  --response-out "${response}" >"${tmpdir}/worker.out" 2>"${tmpdir}/worker.err"; then
+  echo "worker treated a delivered MCP error as a process failure" >&2
+  cat "${tmpdir}/worker.err" >&2
+  exit 1
+fi
+
+if [[ ! -s "${response}" ]] || ! grep -q 'Pipeline failed' "${response}"; then
+  echo "worker did not deliver the underlying pipeline error" >&2
+  cat "${response}" 2>/dev/null >&2 || true
+  exit 1
+fi
+
+set +e
+CBM_CACHE_DIR="${tmpdir}/cache-supervisor" \
+  "${BINARY}" cli index_repository --repo-path "${missing}" --mode fast \
+  >"${tmpdir}/supervisor.out" 2>"${tmpdir}/supervisor.err"
+cli_rc=$?
+set -e
+
+if [[ ${cli_rc} -eq 0 ]]; then
+  echo "human-facing CLI unexpectedly accepted an indexing error" >&2
+  exit 1
+fi
+if grep -q 'crashed on a file' "${tmpdir}/supervisor.out" "${tmpdir}/supervisor.err"; then
+  echo "supervisor replaced a valid tool error with a false crash diagnosis" >&2
+  exit 1
+fi
+if ! grep -q 'Pipeline failed' "${tmpdir}/supervisor.out" "${tmpdir}/supervisor.err"; then
+  echo "supervisor did not preserve the worker's pipeline error" >&2
+  cat "${tmpdir}/supervisor.out" >&2
+  cat "${tmpdir}/supervisor.err" >&2
+  exit 1
+fi
+
+echo "ok: worker MCP errors remain tool errors, not process crashes"