merge(upstream): retain managed spawn with bounded EAGAIN retries

Merge upstream/main at 0c64de93 while retaining cbm_darwin_spawn_managed's separate stderr descriptor, CLOEXEC_DEFAULT descriptor hygiene, child process group, signal reset, and post-spawn lifecycle seam.

Use six bounded exponential backoffs for EAGAIN/ENOMEM on both posix_spawnp and fork, with deterministic success and exhaustion tests. Prefer an existing OpenCode opencode.jsonc target, and classify JSON/XML release objects as structured text while retaining credential scans.

Verified: subprocess suite 38 passed; focused OpenCode tests 3 passed; security-strings allow-list tests 4 passed.
Signed-off-by: Andrew Hundt <ATHundt@gmail.com>

scripts/security-strings.sh

@@ -31,10 +31,20 @@ FAIL=0
 # audits which are designed for compiled artifacts. Script content is
 # reviewed in PRs and scanned end-to-end by VirusTotal in the same pipeline.
 IS_SCRIPT=false
-if command -v file &>/dev/null; then
+# Structured-text artifacts by EXTENSION first: `file` reports JSON as
+# "JSON data" / "JSON text data", which matched none of the text patterns
+# below, so an .mcpb bundle manifest was audited as if it were a compiled
+# binary and its own homepage/documentation URLs were BLOCKED as unauthorized.
+# The extension check also covers hosts where file(1) is absent, where the
+# `command -v` guard below silently leaves every artifact classified as binary.
+case "$BINARY" in
+    *.json|*.jsonc|*.yaml|*.yml|*.toml|*.md|*.txt) IS_SCRIPT=true ;;
+esac
+if [ "$IS_SCRIPT" = false ] && command -v file &>/dev/null; then
     FILE_TYPE=$(file -b "$BINARY" 2>/dev/null || echo "")
     case "$FILE_TYPE" in
-        *"shell script"*|*"ASCII text"*|*"UTF-8 Unicode text"*|*"Unicode text"*|*"a /usr/bin/env"*)
+        *"shell script"*|*"ASCII text"*|*"UTF-8 Unicode text"*|*"Unicode text"*|*"a /usr/bin/env"*|\
+        *"JSON"*|*"XML"*)
             IS_SCRIPT=true
             ;;
     esac
@@ -72,6 +82,10 @@ echo "--- URL audit ---"
 ALLOWED_URLS=(
     "https://api.github.com/repos/DeusData/codebase-memory-mcp"
     "https://github.com/DeusData/codebase-memory-mcp"
+    # Our own org root and documentation site: the .mcpb bundle manifest carries
+    # them as homepage/documentation fields, and the graph UI links the docs.
+    "https://github.com/DeusData"
+    "https://deusdata.github.io/codebase-memory-mcp"
     "http://127.0.0.1"
     "http://localhost"
     # SQLite internal URLs (part of vendored sqlite3 strings)

src/cli/cli.c

@@ -2493,6 +2493,20 @@ static void cbm_opencode_config_path(const char *home_dir, char *out, size_t out
         snprintf(out, out_sz, "%s", custom);
         return;
     }
+    /* OpenCode reads either opencode.json or opencode.jsonc. We always wrote
+     * the .json name, so a user whose real config is .jsonc got a SECOND file
+     * that OpenCode ignores — their MCP server silently never appeared, and the
+     * install looked like it had succeeded (discussion #1560). Prefer whichever
+     * file already exists, .jsonc first since it is the one we used to miss;
+     * with neither present, create .json as before. Other clients in this file
+     * (pochi, kilo) already carry .jsonc paths — OpenCode was the gap. */
+    char candidate[CLI_BUF_1K];
+    int written = snprintf(candidate, sizeof(candidate), "%s/.config/opencode/opencode.jsonc",
+                           home_dir ? home_dir : "");
+    if (written > 0 && (size_t)written < sizeof(candidate) && cbm_file_exists(candidate)) {
+        snprintf(out, out_sz, "%s", candidate);
+        return;
+    }
     snprintf(out, out_sz, "%s/.config/opencode/opencode.json", home_dir);
 }
 

src/foundation/subprocess.c

@@ -948,26 +948,76 @@ static cbm_proc_poll_t cbm_subprocess_poll_win(cbm_subprocess_t *process, cbm_pr
 
 #else /* POSIX */
 
-/* Transient spawn-failure retry (see the EAGAIN note in cbm_darwin_spawn_managed).
- * Three attempts over ~20ms total: long enough to ride out a burst of process
- * creation, short enough that a genuinely exhausted system still fails fast. */
-enum { CBM_SPAWN_RETRY = 2, CBM_SPAWN_RETRY_ATTEMPTS = 3 };
-static const struct timespec CBM_SPAWN_RETRY_DELAY = {0, 10L * 1000L * 1000L};
-#define CBM_SPAWN_RETRY_DELAY_NS (&CBM_SPAWN_RETRY_DELAY)
+/* Transient spawn-failure retry (see the EAGAIN note in cbm_darwin_spawn_managed). */
+enum { CBM_SPAWN_RETRY = 2, CBM_SPAWN_RETRY_ATTEMPTS = 6 };
+
+/* Exponential backoff: 10, 20, 40, 80, 160, 320ms — ~630ms of total patience.
+ *
+ * The first version used only flat 10ms intervals. That was enough for a
+ * momentary dip and not for the real thing: a CI runner building and testing in
+ * parallel stays process-starved for hundreds of milliseconds at a stretch, and
+ * `subprocess_run_spawn_failure` kept failing with the retry shipped (macos-15-
+ * intel on a release matrix, macos-14 under ThreadSanitizer, which is itself
+ * slow enough to create the pressure). A fixed short delay samples the same
+ * congested instant repeatedly; doubling walks out of it.
+ *
+ * The ceiling is deliberate. ~0.6s is invisible next to spawning a process that
+ * does real work, and a machine still refusing after that is genuinely out of
+ * capacity — at which point failing IS the correct answer, and failing fast
+ * beats hanging. */
+#ifdef CBM_ENABLE_TEST_SEAMS
+/* Deterministic EAGAIN injection: see the header. Counts DOWN, so a test asks
+ * for N simulated refusals and the (N+1)th attempt proceeds for real. */
+static int g_force_spawn_eagain = 0;
+void cbm_subprocess_force_spawn_eagain_for_testing(int attempts) {
+    g_force_spawn_eagain = attempts > 0 ? attempts : 0;
+}
+int cbm_subprocess_pending_spawn_eagain_for_testing(void) {
+    return g_force_spawn_eagain;
+}
+static bool cbm_spawn_eagain_injected(void) {
+    if (g_force_spawn_eagain > 0) {
+        g_force_spawn_eagain--;
+        return true;
+    }
+    return false;
+}
+#endif
+
+static void cbm_spawn_backoff(int attempt) {
+    long ms = 10L << (attempt < 6 ? attempt : 6);
+    struct timespec delay = {ms / 1000L, (ms % 1000L) * 1000L * 1000L};
+    (void)cbm_nanosleep(&delay, NULL);
+}
 
 /* fork() fails with EAGAIN under the same pressure posix_spawn does, and the
  * fallback path must not be less robust than the primary one. */
 static pid_t cbm_fork_with_retry(void) {
-    for (int attempt = 0; attempt < CBM_SPAWN_RETRY_ATTEMPTS; attempt++) {
+    /* CBM_SPAWN_RETRY_ATTEMPTS backoffs means ATTEMPTS+1 tries. Every try goes
+     * through the same branch — including the last — so the injection seam
+     * models production exactly rather than leaving a final unguarded fork() the
+     * tests could never reach. */
+    for (int attempt = 0;; attempt++) {
+#ifdef CBM_ENABLE_TEST_SEAMS
+        if (cbm_spawn_eagain_injected()) {
+            if (attempt >= CBM_SPAWN_RETRY_ATTEMPTS) {
+                errno = EAGAIN;
+                return -1;
+            }
+            cbm_spawn_backoff(attempt);
+            continue;
+        }
+#endif
         pid_t pid = fork();
         if (pid >= 0 || (errno != EAGAIN && errno != ENOMEM)) {
             return pid;
         }
-        if (attempt + 1 < CBM_SPAWN_RETRY_ATTEMPTS) {
-            (void)cbm_nanosleep(CBM_SPAWN_RETRY_DELAY_NS, NULL);
+        if (attempt >= CBM_SPAWN_RETRY_ATTEMPTS) {
+            errno = EAGAIN;
+            return -1;
         }
+        cbm_spawn_backoff(attempt);
     }
-    return -1;
 }
 
 /* Used by the fork+exec child. posix_spawn performs the same reset
@@ -1087,6 +1137,11 @@ static int cbm_darwin_spawn_managed(cbm_subprocess_t *process, int input, int ou
      * Setup and exec-class errors fall back to fork/exec below. Transient
      * EAGAIN/ENOMEM results are retried by the caller with a fixed bound.
      */
+#ifdef CBM_ENABLE_TEST_SEAMS
+    if (cbm_spawn_eagain_injected()) {
+        return CBM_SPAWN_RETRY;
+    }
+#endif
     posix_spawn_file_actions_t actions;
     if (posix_spawn_file_actions_init(&actions) != 0) {
         return 1;
@@ -1205,9 +1260,9 @@ static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) {
     pid_t pid = -1;
 #ifdef __APPLE__
     int spawn_rc = cbm_darwin_spawn_managed(process, input, output, error_output, &pid);
-    for (int attempt = 1; spawn_rc == CBM_SPAWN_RETRY && attempt < CBM_SPAWN_RETRY_ATTEMPTS;
+    for (int attempt = 0; spawn_rc == CBM_SPAWN_RETRY && attempt < CBM_SPAWN_RETRY_ATTEMPTS;
          attempt++) {
-        (void)cbm_nanosleep(CBM_SPAWN_RETRY_DELAY_NS, NULL);
+        cbm_spawn_backoff(attempt);
         spawn_rc = cbm_darwin_spawn_managed(process, input, output, error_output, &pid);
     }
     bool spawned = spawn_rc == 0;

src/foundation/subprocess.h

@@ -225,4 +225,12 @@ bool cbm_build_win_cmdline(char *buf, size_t cap, const char *const *argv);
 bool cbm_build_win_cmd_payload(char *buf, size_t cap, const char *cmd_executable,
                                const char *payload);
 
+#ifdef CBM_ENABLE_TEST_SEAMS
+/* Force the next N spawn attempts to behave as if the kernel returned EAGAIN
+ * ("try again"), so the retry path can be exercised deterministically instead
+ * of hoping a loaded machine reproduces it. Test builds only. */
+void cbm_subprocess_force_spawn_eagain_for_testing(int attempts);
+int cbm_subprocess_pending_spawn_eagain_for_testing(void);
+#endif
+
 #endif /* CBM_SUBPROCESS_H */

tests/test_cli.c

@@ -7560,6 +7560,40 @@ TEST(cli_opencode_honors_custom_config) {
     PASS();
 }
 
+/* Discussion #1560: OpenCode reads either opencode.json or opencode.jsonc, but
+ * we always wrote the .json name. A user whose real config is .jsonc got a
+ * SECOND file that OpenCode ignores — the MCP server silently never appeared
+ * while the install reported success. Prefer an existing .jsonc; with neither
+ * present, .json is still created, so fresh installs are unchanged. */
+TEST(cli_opencode_prefers_existing_jsonc_config_discussion1560) {
+    char tmpdir[256];
+    snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-opencode-jsonc-XXXXXX");
+    if (!cbm_mkdtemp(tmpdir))
+        FAIL("cbm_mkdtemp failed");
+    char config_dir[512];
+    snprintf(config_dir, sizeof(config_dir), "%s/.config/opencode", tmpdir);
+    test_mkdirp(config_dir);
+    char jsonc_path[640];
+    snprintf(jsonc_path, sizeof(jsonc_path), "%s/opencode.jsonc", config_dir);
+    write_test_file(jsonc_path, "{\n  // user's hand-written config\n  \"theme\": \"dark\"\n}\n");
+
+    char *saved_path = save_test_env("PATH");
+    char *saved_file = save_test_env("OPENCODE_CONFIG");
+    cbm_setenv("PATH", tmpdir, 1);
+    cbm_unsetenv("OPENCODE_CONFIG");
+
+    char *json = cbm_build_install_plan_json(tmpdir, "/usr/local/bin/codebase-memory-mcp");
+    bool targets_jsonc = json && strstr(json, "/.config/opencode/opencode.jsonc") != NULL;
+
+    free(json);
+    restore_test_env("PATH", saved_path);
+    restore_test_env("OPENCODE_CONFIG", saved_file);
+    test_rmdir_r(tmpdir);
+    if (!targets_jsonc)
+        FAIL("an existing opencode.jsonc must be the install target, not a new opencode.json");
+    PASS();
+}
+
 TEST(cli_opencode_config_dir_detects_without_retargeting_global_json) {
     char tmpdir[256];
     snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-opencode-dir-XXXXXX");
@@ -14924,6 +14958,7 @@ SUITE(cli) {
     RUN_TEST(cli_antigravity_does_not_imply_gemini);
     RUN_TEST(cli_antigravity_plan_uses_documented_global_files);
     RUN_TEST(cli_opencode_honors_custom_config);
+    RUN_TEST(cli_opencode_prefers_existing_jsonc_config_discussion1560);
     RUN_TEST(cli_opencode_config_dir_detects_without_retargeting_global_json);
     RUN_TEST(cli_kiro_and_hermes_homes_are_honored);
     RUN_TEST(cli_detect_agents_finds_official_kiro_cli_executable);

tests/test_security_strings_allowlist.sh

@@ -63,5 +63,47 @@ else
     PASS=$((PASS + 1))
 fi
 
+# ── Case 3: an .mcpb bundle manifest must not be audited as a compiled binary ──
+# `file` reports JSON as "JSON data", which matched none of the text patterns, so
+# the manifest was run through the URL audit and its own homepage/documentation
+# fields were BLOCKED as unauthorized. This blocked the v0.10.3 release — the
+# first release to ship .mcpb bundles, hence the first time a manifest reached
+# the scanned object set.
+MANIFEST="$TMP/manifest.json"
+cat > "$MANIFEST" <<'JSON'
+{
+  "name": "codebase-memory-mcp",
+  "homepage": "https://github.com/DeusData/codebase-memory-mcp",
+  "documentation": "https://deusdata.github.io/codebase-memory-mcp/",
+  "server": { "command": "codebase-memory-mcp", "args": [] }
+}
+JSON
+if bash "$SCRIPT" "$MANIFEST" >/dev/null 2>&1; then
+    echo "PASS: .mcpb manifest.json audited as structured text, not as a binary"
+    PASS=$((PASS + 1))
+else
+    echo "FAIL: manifest.json still audited as a compiled binary (regression)"
+    bash "$SCRIPT" "$MANIFEST" 2>&1 | grep -i "BLOCKED" || true
+    FAIL=$((FAIL + 1))
+fi
+
+# ── Case 4 (negative control for case 3): the exemption is by FILE TYPE, not a
+# blanket pass. A binary carrying an unauthorized URL must still be blocked even
+# though a .json carrying one would not be — that asymmetry is the design, and
+# case 2 above proves the binary half still holds. Here we prove the credential
+# audit still runs on structured text, so the exemption is narrow. ──
+CREDS="$TMP/creds.json"
+# The credential audit matches assignment syntax (api_key=, secret=, ...), which
+# is what leaks look like in embedded strings — so the fixture uses that form
+# inside the JSON value rather than a JSON "key": "value" pair.
+printf '{"connection":"host=db user=admin password=hunter2-not-a-real-secret"}' > "$CREDS"
+if bash "$SCRIPT" "$CREDS" >/dev/null 2>&1; then
+    echo "FAIL: credential pattern in JSON was NOT flagged (exemption too broad)"
+    FAIL=$((FAIL + 1))
+else
+    echo "PASS: credential audit still runs on structured text"
+    PASS=$((PASS + 1))
+fi
+
 echo "=== security-strings allow-list test: $PASS passed, $FAIL failed ==="
 [ "$FAIL" -eq 0 ]

tests/test_subprocess.c

@@ -275,7 +275,45 @@ TEST(subprocess_run_cancel_reaps_child) {
 #endif
 }
 
-/* A spawn of a non-existent binary fails cleanly (no child), not a crash. */
+/* The kernel refusing a spawn with EAGAIN means "not right now", not "never" —
+ * a momentarily full process table on a busy machine. We used to treat it as a
+ * permanent failure, so a git probe or LSP server refused to start for a reason
+ * the user could neither see nor act on, and `subprocess_run_spawn_failure`
+ * failed on loaded CI runners with the contract intact.
+ *
+ * These pin the retry itself rather than the constant. Injecting refusals is
+ * deterministic, so this proves the loop retries on EVERY machine instead of
+ * only on one that happens to be starved. */
+TEST(subprocess_retries_transient_spawn_refusal) {
+#ifdef _WIN32
+    SKIP_PLATFORM("POSIX fork/EAGAIN path");
+#else
+    /* Fewer refusals than the budget: the spawn must still succeed. */
+    cbm_subprocess_force_spawn_eagain_for_testing(3);
+    cbm_proc_result_t r = run_sh("exit 0", 0);
+    ASSERT_EQ(cbm_subprocess_pending_spawn_eagain_for_testing(), 0);
+    ASSERT_EQ(r.outcome, CBM_PROC_CLEAN);
+    ASSERT_EQ(r.exit_code, 0);
+    PASS();
+#endif
+}
+
+TEST(subprocess_gives_up_after_the_retry_budget) {
+#ifdef _WIN32
+    SKIP_PLATFORM("POSIX fork/EAGAIN path");
+#else
+    /* More refusals than the budget: it must fail rather than retry forever.
+     * A machine still refusing after ~0.6s is genuinely out of capacity, and
+     * failing fast beats hanging. */
+    cbm_subprocess_force_spawn_eagain_for_testing(50);
+    cbm_proc_result_t r = run_sh("exit 0", 0);
+    bool refused = r.outcome == CBM_PROC_SPAWN_FAILED;
+    cbm_subprocess_force_spawn_eagain_for_testing(0); /* never leak into later tests */
+    ASSERT_TRUE(refused);
+    PASS();
+#endif
+}
+
 TEST(subprocess_run_spawn_failure) {
 #ifdef _WIN32
     SKIP_PLATFORM("POSIX exec semantics");
@@ -1398,6 +1436,8 @@ SUITE(subprocess) {
     RUN_TEST(subprocess_run_crash_is_crash);
     RUN_TEST(subprocess_run_hang_is_hang);
     RUN_TEST(subprocess_run_cancel_reaps_child);
+    RUN_TEST(subprocess_retries_transient_spawn_refusal);
+    RUN_TEST(subprocess_gives_up_after_the_retry_budget);
     RUN_TEST(subprocess_run_spawn_failure);
     RUN_TEST(subprocess_run_null_bin_rejected);
     RUN_TEST(subprocess_spawn_returns_while_child_is_running);