Skip to content

Commit 8fc04c0

Browse files
committed
fix(worker): preserve delivered MCP errors
Signed-off-by: wargloom <wargloom@gmail.com>
1 parent a65faeb commit 8fc04c0

3 files changed

Lines changed: 101 additions & 8 deletions

File tree

‎scripts/test.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,12 @@ CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_parent_watchdog.sh"
276276
echo "=== Step 5b: worker-mode watchdog regression (#845) ==="
277277
CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_worker_watchdog.sh"
278278

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

‎src/main.c‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -812,11 +812,13 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo
812812
/* Supervised worker: hand the full result string to the parent via the
813813
* response file before printing (parent reads it back on a clean exit). */
814814
const char *ro = cbm_index_worker_response_out();
815+
bool worker_response_written = false;
815816
if (ro) {
816817
FILE *rf = cbm_fopen(ro, "wb");
817818
if (rf) {
818-
(void)fputs(result, rf);
819-
(void)fclose(rf);
819+
int write_rc = fputs(result, rf);
820+
int close_rc = fclose(rf);
821+
worker_response_written = write_rc >= 0 && close_rc == 0;
820822
}
821823
}
822824
if (raw_json) {
@@ -830,14 +832,15 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo
830832
}
831833
exit_code = cbm_cli_exit_status_after_maintenance(exit_code, maintenance_cancelled);
832834
if (cbm_index_worker_active()) {
833-
/* Supervised worker: the response is delivered (file + stdout).
834-
* Skip the multi-GB teardown (server/store frees) — the process
835-
* dies now and the OS reclaims everything wholesale; piecemeal
836-
* free() of a kernel-scale graph costs minutes. _Exit skips
837-
* atexit/LSan by design for this prod worker path. */
835+
/* The supervisor protocol classifies the PROCESS, not the tool
836+
* result: a valid MCP error response is a healthy worker outcome.
837+
* Propagating cli_print_mcp_result's isError exit code made the
838+
* parent discard that response and falsely report exit_nonzero as
839+
* "crashed on a file". Fail only when the response transport itself
840+
* failed. Skip multi-GB teardown; the OS reclaims it at exit. */
838841
cbm_log_info("index.worker.fast_exit", "action", "_Exit");
839842
fflush(NULL);
840-
_Exit(exit_code);
843+
_Exit(worker_response_written ? 0 : SKIP_ONE);
841844
}
842845
free(result);
843846
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
#!/usr/bin/env bash
2+
# A supervised worker that produces a valid MCP error response is healthy.
3+
# The CLI may exit nonzero when presenting that error to a human, but the worker
4+
# transport must exit zero so its parent reads the response instead of inventing
5+
# an exit_nonzero "crashed on a file" diagnosis.
6+
set -euo pipefail
7+
8+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
9+
BINARY="${ROOT}/build/c/codebase-memory-mcp"
10+
if [[ ! -x "${BINARY}" && -x "${BINARY}.exe" ]]; then
11+
BINARY="${BINARY}.exe"
12+
fi
13+
14+
if [[ ! -x "${BINARY}" ]]; then
15+
echo "missing binary: ${BINARY}" >&2
16+
exit 2
17+
fi
18+
19+
if command -v shasum >/dev/null 2>&1; then
20+
BUILD_FINGERPRINT="$(shasum -a 256 "${BINARY}" | awk '{print $1}')"
21+
elif command -v sha256sum >/dev/null 2>&1; then
22+
BUILD_FINGERPRINT="$(sha256sum "${BINARY}" | awk '{print $1}')"
23+
elif command -v openssl >/dev/null 2>&1; then
24+
BUILD_FINGERPRINT="$(openssl dgst -sha256 "${BINARY}" | awk '{print $NF}')"
25+
else
26+
echo "no SHA-256 command available for worker build binding" >&2
27+
exit 2
28+
fi
29+
if [[ ! "${BUILD_FINGERPRINT}" =~ ^[0-9a-f]{64}$ ]]; then
30+
echo "invalid worker build fingerprint: ${BUILD_FINGERPRINT}" >&2
31+
exit 2
32+
fi
33+
34+
tmpdir="$(mktemp -d)"
35+
cleanup() {
36+
CBM_CACHE_DIR="${tmpdir}/cache-supervisor" \
37+
"${BINARY}" daemon stop >/dev/null 2>&1 || true
38+
rm -rf "${tmpdir}"
39+
}
40+
trap cleanup EXIT
41+
42+
missing="${tmpdir}/repository-does-not-exist"
43+
response="${tmpdir}/worker.response"
44+
args="{\"repo_path\":\"${missing}\",\"mode\":\"fast\"}"
45+
46+
if ! CBM_CACHE_DIR="${tmpdir}/cache-worker" \
47+
"${BINARY}" cli --index-worker \
48+
--index-worker-build "${BUILD_FINGERPRINT}" \
49+
index_repository "${args}" \
50+
--response-out "${response}" >"${tmpdir}/worker.out" 2>"${tmpdir}/worker.err"; then
51+
echo "worker treated a delivered MCP error as a process failure" >&2
52+
cat "${tmpdir}/worker.err" >&2
53+
exit 1
54+
fi
55+
56+
if [[ ! -s "${response}" ]] || ! grep -q 'Pipeline failed' "${response}"; then
57+
echo "worker did not deliver the underlying pipeline error" >&2
58+
cat "${response}" 2>/dev/null >&2 || true
59+
exit 1
60+
fi
61+
62+
set +e
63+
CBM_CACHE_DIR="${tmpdir}/cache-supervisor" \
64+
"${BINARY}" cli index_repository --repo-path "${missing}" --mode fast \
65+
>"${tmpdir}/supervisor.out" 2>"${tmpdir}/supervisor.err"
66+
cli_rc=$?
67+
set -e
68+
69+
if [[ ${cli_rc} -eq 0 ]]; then
70+
echo "human-facing CLI unexpectedly accepted an indexing error" >&2
71+
exit 1
72+
fi
73+
if grep -q 'crashed on a file' "${tmpdir}/supervisor.out" "${tmpdir}/supervisor.err"; then
74+
echo "supervisor replaced a valid tool error with a false crash diagnosis" >&2
75+
exit 1
76+
fi
77+
if ! grep -q 'Pipeline failed' "${tmpdir}/supervisor.out" "${tmpdir}/supervisor.err"; then
78+
echo "supervisor did not preserve the worker's pipeline error" >&2
79+
cat "${tmpdir}/supervisor.out" >&2
80+
cat "${tmpdir}/supervisor.err" >&2
81+
exit 1
82+
fi
83+
84+
echo "ok: worker MCP errors remain tool errors, not process crashes"

0 commit comments

Comments
 (0)