[mertcankaratass]

Fixes #77

Problem

On macOS and Linux, every child process that codebase-memory-mcp starts (index workers, search_code grep scans, git commands, and CLI update helpers) inherited the server's open lock-file descriptors (e.g. cbm.zova.ws-*.lock, cbm.zova.prepare.lock) because lock files were opened without close-on-exec and spawn paths did not close open descriptors.

Solution

1. macOS: Kernel-enforced closure with posix_spawn & POSIX_SPAWN_CLOEXEC_DEFAULT

• src/foundation/subprocess.c (cbm_run_posix): On macOS, spawns children via posix_spawnp with POSIX_SPAWN_CLOEXEC_DEFAULT instead of fork + bounded loop. The kernel automatically closes all descriptors except those explicitly set up (/dev/null on stdin, target on stdout/stderr, and POSIX_SPAWN_SETPGROUP for process groups). When posix_spawnp fails (e.g. ENOENT/EACCES), if opts->delete_log_on_exit is set, opts->log_file is unlinked immediately so target file is never left behind.
• src/foundation/compat_fs.c (cbm_exec_no_shell): On macOS, spawns via posix_spawnp with POSIX_SPAWN_CLOEXEC_DEFAULT (inheriting stdin/stdout/stderr via posix_spawn_file_actions_addinherit_np), keeping PATH lookup and closing all stray fds without limit.
• src/foundation/compat_fs.c (cbm_popen_spawn_sh): Uses posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, closing all descriptors other than stdin (/dev/null), stdout (pipe write-end), and stderr (addinherit_np).
• Linux (cbm_fd_close_above_stderr): Uses SYS_close_range on Linux 5.9+, and keeps a bounded loop as fallback for older Linux kernels.

2. Open-Site Close-on-Exec Audit (Ported with #163 Zig Migration)

• src/foundation/file_lock.c: Opens lock files with O_CLOEXEC on POSIX and applies cbm_fd_set_cloexec(fd).
• src/pipeline/result_spill.c: Changed sp->file = fopen(...) to cbm_fopen(...) so the long-lived spill file open during indexing has FD_CLOEXEC.
• src/foundation/compat.h: cbm_mkstemp now sets FD_CLOEXEC on the created descriptor.
• src/mcp/search_tools.zig: writeScopedFilelist, writePatternFile, and runGrepScan (former src/mcp/mcp.c) now use c.cbm_fopen.
• src/cli/cli.zig: cbm_replace_binary (former src/cli/cli.c) now uses c.O_CLOEXEC in c.open flags on POSIX.
• src/foundation/diagnostics.c & src/main.c & src/foundation/subprocess.c: Switched diagnostics, slurp, and log-tailing fopen calls to cbm_fopen.
• src/foundation/platform.c: Added O_CLOEXEC to raw open() calls.
• Left as raw: Short-lived atomic file operations (e.g. cli_copy_file, cbm_zova_operations manifest/sha256 reads, mem.c statm reader) that close immediately before returning and before any child is spawned.

3. Acceptance Tests (tests/test_subprocess.c) & Verification

• posix_child_does_not_inherit_lock_fd: Tests that holding an exclusive file lock does not leak descriptors to cbm_subprocess_run or cbm_popen. Uses cbm_tmpdir() and cleans up resources before asserting.
• posix_child_backstop_closes_raw_descriptors: Opens a raw descriptor without close-on-exec (open("/dev/null", O_RDONLY)), and asserts that neither cbm_subprocess_run nor cbm_exec_no_shell leaks it to the child.
• macOS Verification: Verified that the subprocess test suite (make -f Makefile.cbm test) passes on macOS, with descriptor closure kernel-enforced without limit.
• DCO: All commits are signed off (Signed-off-by).

[ata-sesli]

Thanks for this. The lock-file fix is correct: with O_CLOEXEC on the lock files, children no longer inherit them, which is the leak #77 reported. CI is green.

Three things to change before merging, each with an inline comment:

1. Test the close-everything backstop. The new test passes even without cbm_fd_close_above_stderr(), because the lock descriptor is already close-on-exec. Nothing covers cbm_exec_no_shell.
2. Use posix_spawn on macOS instead of the close() loop. The loop stops at descriptor 4096, so higher descriptors still reach the child.
3. Finish the open-site audit from #77 (step 2). Only cbm_fopen gained close-on-exec. Files opened with raw fopen()/open() can still leak, and the git spawn paths (cbm_popen_git, cbm_popen) have no backstop, so close-on-exec is their only protection. Since the PR says Fixes #77, it should cover this part too.

[ata-sesli]

The backstop has no test. The lock descriptor in this test is already close-on-exec because of the file_lock.c change, so the test still passes if cbm_fd_close_above_stderr() is removed from cbm_run_posix and cbm_exec_no_shell. That step is most of this PR, and cbm_exec_no_shell has no test at all.

Please add a case that uses a descriptor without close-on-exec, for example a plain open("/dev/null", O_RDONLY). Spawn through cbm_subprocess_run and through cbm_exec_no_shell, and assert the child can't see it. For cbm_exec_no_shell, the check's exit code is its return value.

I checked the helper on macOS in a small standalone program. A non-close-on-exec descriptor reaches the child without the helper and is closed with it, so this test would catch a regression.

Smaller points in this test:

• The lock path is a hard-coded /tmp/.... Most of this file uses cbm_tmpdir().
• If an assertion fails before cleanup, the lock and its file are left behind.

[ata-sesli]

Descriptors above 4096 still leak here. This fallback runs on macOS and on Linux kernels older than 5.9. With the open-file limit raised to 8192 on macOS, descriptor 5000 was still open in the child, while 3 and 3000 were closed. Separately, getrlimit and syscall are not on POSIX's list of functions safe to call between fork and exec.

On macOS, please spawn with posix_spawn and POSIX_SPAWN_CLOEXEC_DEFAULT instead of fork plus this loop. The kernel then closes every descriptor except the ones the file actions set up, with no limit and no loop.

• cbm_run_posix:
  • redirect stdout/stderr with posix_spawn_file_actions_addopen and posix_spawn_file_actions_adddup2;
  • replace the setpgid call with POSIX_SPAWN_SETPGROUP and a pgroup of 0;
  • keep stdin explicitly with posix_spawn_file_actions_addinherit_np(STDIN_FILENO), or open /dev/null there. Under CLOEXEC_DEFAULT, stdin is closed otherwise.
• cbm_exec_no_shell: use posix_spawnp, which keeps the PATH lookup that execvp does now.
• cbm_popen_spawn_sh already uses posix_spawn, so it can take the same flag.

POSIX_SPAWN_CLOEXEC_DEFAULT is Apple-only. Keep close_range on Linux, and keep the loop only as the fallback for old Linux kernels.

[ata-sesli]

The audit from #77 (step 2) is only partly done. cbm_fopen now sets close-on-exec, but many descriptors are opened with raw fopen()/open(). Some of them stay open while children run. One example is the result spill file (sp->file = fopen(sp->path, "w+b") in src/pipeline/result_spill.c), which is open for the whole indexing run.

The backstop only covers cbm_run_posix and cbm_exec_no_shell. Children started through cbm_popen_git (posix_spawn, used for git) and cbm_popen (plain popen) still inherit every descriptor that lacks close-on-exec. For those paths, close-on-exec at the open site is the only protection.

Since this PR says Fixes #77, please go through the raw fopen()/open()/mkstemp()/fdopen() sites under src/ and give close-on-exec to the ones that can be open while a child is spawned. Use O_CLOEXEC for open(), and cbm_fd_set_cloexec(fileno(f)) or cbm_fopen for fopen(). Short-lived reads that close before returning can stay as they are. A short list in the PR description of which sites you changed and which you left (and why) would make this easy to review.

[ata-sesli]

Thanks for the follow-up. All three points from the last review are handled:

• Backstop test: posix_child_backstop_closes_raw_descriptors uses a descriptor without close-on-exec and covers both cbm_subprocess_run and cbm_exec_no_shell. The lock test now uses cbm_tmpdir() and cleans up before asserting.
• macOS spawn: cbm_run_posix, cbm_exec_no_shell and cbm_popen_spawn_sh now spawn with POSIX_SPAWN_CLOEXEC_DEFAULT. Linux keeps close_range and falls back to the loop.
• Open-site audit: the long-lived descriptors now get close-on-exec: the spill file, cbm_mkstemp, the search-scan files, diagnostics, log tailing, cbm_replace_binary and cbm_mmap_read. The raw sites still left are short-lived reads and writes that close before returning, which is fine.

Two things block merging now.

1. Conflict with main: src/cli/cli.c and src/mcp/mcp.c no longer exist

#163 was merged. It ports src/cli and src/mcp to Zig and deletes both C files, so this branch now has two modify/delete conflicts. After rebasing onto main, please move these four changes into the Zig files:

Was (C)	Now on main
write_scoped_filelist: fopen(filelist, "wb")	src/mcp/search_tools.zig:649, c.fopen(filelist, "wb")
write_pattern_file: fopen(tmpfile, "w")	src/mcp/search_tools.zig:709, c.fopen(tmpfile, "w")
handle_search_code: fopen(outfile, "r")	src/mcp/search_tools.zig:794, c.fopen(&outfile, "r")
cbm_replace_binary: open(..., O_CLOEXEC)	src/cli/cli.zig:308, c.open(path, c.O_WRONLY | c.O_CREAT | c.O_TRUNC, ...)

For the first three, use c.cbm_fopen, which the Zig code already calls elsewhere (e.g. search_tools.zig:500). For the last one, add c.O_CLOEXEC to the flags. That branch only compiles off Windows, since is_windows is comptime. The other c.fopen calls in cli.zig match the short-lived C sites you already left alone.

2. Commits are not signed off

Neither 6b87d508 nor 42b6fc18 has a Signed-off-by: line. CONTRIBUTING.md requires it. The CI here doesn't check it, so green checks don't mean it passed. git rebase --signoff main covers both, and the rebase for point 1 is a good time to do it. While you're there, please drop the leftover # Conflicts: src/mcp/mcp.c lines from the 42b6fc18 message.

Smaller points

• Log file left behind (inline comment). Nothing calls this path today, so it's not urgent.
• The macOS paths are untested in CI. Every test job in ci.yml runs on ubuntu-latest, and macOS is only compiled. Please run the subprocess suite on a Mac (e.g. scripts/zova-run-tests.sh or make -f Makefile.cbm test) and say in the PR description that it passed.