From 4b7c71130c6bb8deabc9a0993a98b65cbe104149 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 24 Sep 2026 10:35:38 -0700 Subject: [PATCH 1/3] Add macOS platform support Add Darwin build, loader, plugin, filesystem-watch, and mailbox polling support while retaining Linux inotify behavior. Use kqueue for native macOS filesystem notifications, adapt dynamic-library conventions and Homebrew dependency paths, and cover macOS watcher/plugin behavior in tests. Restore the Linux mailbox path unchanged to avoid platform regressions. Mount the local ONNX embedding model into the reusable Linux test container so the embedding test runs there, and propagate per-test failures through the test and test-container targets. Signed-off-by: Jhon Honce --- .gitignore | 6 ++ Makefile | 93 +++++++++++++++---- src/fswatch_kqueue.c | 159 ++++++++++++++++++++++++++++++++ src/fswatch_noop.c | 8 +- src/journal.c | 19 +++- src/mailbox.c | 46 +++++++-- src/main.c | 8 +- src/matrix.c | 29 +++++- src/subprocess.c | 16 +++- src/telegram.c | 45 ++++++--- src/tool_plugin.c | 8 +- src/tools.c | 27 +++++- tests/test_fswatch.c | 53 ++++++----- tests/test_onnx_embed.c | 10 +- tests/test_tool_plugin_dlopen.c | 21 +++-- 15 files changed, 463 insertions(+), 85 deletions(-) create mode 100644 src/fswatch_kqueue.c diff --git a/.gitignore b/.gitignore index 42085340..3ffe28b0 100644 --- a/.gitignore +++ b/.gitignore @@ -4,7 +4,10 @@ nash *.tar.gz *.tar.zst libnash.so* +libnash*.dylib tests/*.so +tests/*.dylib +*.dSYM/ tests/test_* !tests/test_*.c !tests/test_*.h @@ -35,6 +38,9 @@ prime_numbers.c fixed_point_sin fixed_point_sin.c +# Local agent instructions +CODEX.md + # Editor/IDE *.swp *.swo diff --git a/Makefile b/Makefile index 489ff6c3..d351f35c 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,40 @@ VERSION ?= 0.1.2 +# Platform-specific linker and loader conventions. Keep CC overridable: Apple +# Clang is sufficient, and requiring a versioned Homebrew GCC only makes the +# build needlessly fragile. +UNAME_S := $(shell uname -s) + +ifeq ($(UNAME_S),Linux) + PLATFORM_DEFINES := -D_DEFAULT_SOURCE + SHARED_EXT := so + SHARED_FLAG := -shared + SONAME_FLAG := -Wl,-soname,libnash.so.0 + RPATH_ORIGIN := $$ORIGIN + EXPORT_DYNAMIC := -rdynamic + DL_LIB := -ldl + NCURSES_LIB := -lncursesw + PLUGIN_EXT := so +else ifeq ($(UNAME_S),Darwin) + PLATFORM_DEFINES := -D_DARWIN_C_SOURCE + SHARED_EXT := dylib + SHARED_FLAG := -dynamiclib + SONAME_FLAG := -Wl,-install_name,@rpath/libnash.0.dylib + RPATH_ORIGIN := @loader_path + EXPORT_DYNAMIC := + DL_LIB := + NCURSES_LIB := -lncurses + PLUGIN_EXT := dylib + BREW_PACKAGES := ncurses readline openssl@3 utf8proc onnxruntime + BREW_CFLAGS := $(foreach p,$(BREW_PACKAGES),$(shell brew --prefix $(p) 2>/dev/null | sed 's|^|-I|; s|$$|/include|')) + BREW_LDFLAGS := $(foreach p,$(BREW_PACKAGES),$(shell brew --prefix $(p) 2>/dev/null | sed 's|^|-L|; s|$$|/lib|')) +else + $(error Unsupported platform: $(UNAME_S)) +endif + CC ?= gcc -CFLAGS ?= -Wall -g -Wextra -Wunused-function -O2 -std=c11 -fPIC -D_POSIX_C_SOURCE=200809L -D_DEFAULT_SOURCE +CFLAGS ?= -Wall -g -Wextra -Wunused-function -O2 -std=c11 -fPIC -D_POSIX_C_SOURCE=200809L $(PLATFORM_DEFINES) +CFLAGS += $(BREW_CFLAGS) # ONNX Runtime: requires onnxruntime-devel (headers) to build. # For linking, use pip-installed libonnxruntime if no system package. ORT_LIB := $(shell python3 -c "import onnxruntime; import os; print(os.path.dirname(onnxruntime.__file__) + '/capi')" 2>/dev/null) @@ -13,7 +46,7 @@ endif # Device subsystem (VNC, HEVC streaming, Tesseract OCR) is now a separate # plugin: nash-tool-device-control. See ~/agents/nash-tool-device-control/ -LDFLAGS ?= -rdynamic -lcurl -lcrypto -lreadline -lncursesw -lpthread -lm -lutf8proc -ldl $(ORT_LDFLAGS) +LDFLAGS ?= $(EXPORT_DYNAMIC) -lcurl -lcrypto -lreadline $(NCURSES_LIB) -lpthread -lm -lutf8proc $(DL_LIB) $(BREW_LDFLAGS) $(ORT_LDFLAGS) # AddressSanitizer for heap corruption detection (opt-in: make SANITIZE=1) ifdef SANITIZE @@ -82,14 +115,21 @@ SRC = src/main.c src/str.c src/cJSON.c \ src/predict.c \ src/harness_metrics.c \ src/fswatch_linux.c \ + src/fswatch_kqueue.c \ src/fswatch_noop.c \ src/mw_builtin.c OBJ = $(SRC:.c=.o) BIN = nash -LIB_REAL = libnash.so.$(VERSION) -LIB_SONAME = libnash.so.0 -LIB_LINKER = libnash.so +ifeq ($(UNAME_S),Linux) + LIB_REAL = libnash.so.$(VERSION) + LIB_SONAME = libnash.so.0 + LIB_LINKER = libnash.so +else + LIB_REAL = libnash.$(VERSION).dylib + LIB_SONAME = libnash.0.dylib + LIB_LINKER = libnash.dylib +endif all: $(LIB_REAL) $(BIN) @@ -106,13 +146,13 @@ LIB_OBJ = $(LIB_SRC:.c=.o) # Shared library: everything except main.c $(LIB_REAL): $(LIB_OBJ) - $(CC) -shared -Wl,-soname,$(LIB_SONAME) -o $@ $^ $(LDFLAGS) + $(CC) $(SHARED_FLAG) $(SONAME_FLAG) -o $@ $^ $(LDFLAGS) ln -sf $(LIB_REAL) $(LIB_SONAME) ln -sf $(LIB_SONAME) $(LIB_LINKER) # Binary: main.o links against libnash.so $(BIN): src/main.o $(LIB_REAL) - $(CC) $(CFLAGS) -o $@ $< -L. -lnash -Wl,-rpath,'$$ORIGIN' $(LDFLAGS) + $(CC) $(CFLAGS) -o $@ $< -L. -lnash -Wl,-rpath,'$(RPATH_ORIGIN)' $(LDFLAGS) # Test binaries TEST_BIN = tests/test_memory tests/test_store tests/test_config \ @@ -132,18 +172,18 @@ TEST_BIN = tests/test_memory tests/test_store tests/test_config \ tests/test_tool_failure # Sample plugin shared objects for dlopen testing -SAMPLE_PLUGINS = tests/sample_plugin.so tests/sample_plugin_bad_abi.so \ - tests/sample_plugin_multi.so +SAMPLE_PLUGINS = tests/sample_plugin.$(PLUGIN_EXT) tests/sample_plugin_bad_abi.$(PLUGIN_EXT) \ + tests/sample_plugin_multi.$(PLUGIN_EXT) -tests/sample_%.so: tests/sample_%.c src/tool_plugin.h src/cJSON.h $(LIB_REAL) - $(CC) -shared -fPIC $(CFLAGS) -I src -o $@ $< -L. -lnash +tests/sample_%.$(PLUGIN_EXT): tests/sample_%.c src/tool_plugin.h src/cJSON.h $(LIB_REAL) + $(CC) $(SHARED_FLAG) -fPIC $(CFLAGS) -I src -o $@ $< -L. -lnash # dlopen test depends on sample .so files tests/test_tool_plugin_dlopen: tests/test_tool_plugin_dlopen.c $(LIB_REAL) $(SAMPLE_PLUGINS) - $(CC) $(CFLAGS) -I src -o $@ $< -L. -lnash -Wl,-rpath,'$$ORIGIN/..' $(LDFLAGS) + $(CC) $(CFLAGS) -I src -o $@ $< -L. -lnash -Wl,-rpath,'$(RPATH_ORIGIN)/..' $(LDFLAGS) tests/test_%: tests/test_%.c $(LIB_REAL) - $(CC) $(CFLAGS) -I src -o $@ $< -L. -lnash -Wl,-rpath,'$$ORIGIN/..' $(LDFLAGS) + $(CC) $(CFLAGS) -I src -o $@ $< -L. -lnash -Wl,-rpath,'$(RPATH_ORIGIN)/..' $(LDFLAGS) test: $(TEST_BIN) @echo "=== Running tests ===" @@ -152,11 +192,32 @@ test: $(TEST_BIN) echo "--- $$t ---"; \ if ./$$t; then echo "PASS"; else echo "FAIL"; failures=$$((failures+1)); fi; \ done; \ - echo "=== $$failures failures ===" + echo "=== $$failures failures ==="; \ + exit $$failures + +# Verify the Linux build from macOS without leaving container-built objects in +# the working tree. The named container is reused after its first setup. +TEST_CONTAINER ?= nash-test-model +NASH_MODEL_DIR ?= $(HOME)/.nash/models/all-MiniLM-L6-v2 +test-container: + @if podman container exists $(TEST_CONTAINER) 2>/dev/null; then \ + podman start $(TEST_CONTAINER) 2>/dev/null || true; \ + else \ + podman run --name $(TEST_CONTAINER) -d \ + -v $(CURDIR):/workspace:Z -w /workspace \ + -v $(NASH_MODEL_DIR):/root/.nash/models/all-MiniLM-L6-v2:ro,Z \ + registry.fedoraproject.org/fedora:latest sleep infinity; \ + podman exec $(TEST_CONTAINER) dnf install -y gcc make libcurl-devel openssl-devel readline-devel ncurses-devel utf8proc-devel onnxruntime-devel; \ + fi + @status=0; \ + podman exec $(TEST_CONTAINER) bash -c "make clean && make && make test" || status=$$?; \ + $(MAKE) clean; \ + exit $$status clean: rm -f $(OBJ) $(BIN) $(LIB_REAL) $(LIB_SONAME) $(LIB_LINKER) $(TEST_BIN) $(SAMPLE_PLUGINS) - rm -rf tests/plugin_dir + rm -f libnash.so* libnash*.dylib + rm -rf tests/plugin_dir tests/*.dSYM # Source tarball for RPM builds (matches spec Source0: nash-VERSION.tar.zst) dist: @@ -181,4 +242,4 @@ install: all install -d $(DESTDIR)$(NASH_DATADIR)/playbooks install -m 644 playbooks/*.yaml $(DESTDIR)$(NASH_DATADIR)/playbooks/ -.PHONY: all clean test dist fmt install +.PHONY: all clean test test-container dist fmt install diff --git a/src/fswatch_kqueue.c b/src/fswatch_kqueue.c new file mode 100644 index 00000000..b7c5fe02 --- /dev/null +++ b/src/fswatch_kqueue.c @@ -0,0 +1,159 @@ +/* Recursive kqueue watcher for macOS. + * + * kqueue only reports changes to an open vnode. To preserve fswatch's + * changed-file callback contract, we watch files as well as directories; + * directory notifications discover and register newly-created children. */ +#ifdef __APPLE__ + +#include "fswatch.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +typedef struct watch_entry { + int fd; + int is_dir; + char *path; + struct watch_entry *next; +} watch_entry_t; + +struct fswatch { + int kqfd; + fswatch_cb cb; + void *userdata; + watch_entry_t *watches; +}; + +static watch_entry_t *find_watch(fswatch_t *w, const char *path) { + for (watch_entry_t *e = w->watches; e; e = e->next) + if (strcmp(e->path, path) == 0) return e; + return NULL; +} + +static int add_watch(fswatch_t *w, const char *path, int is_dir) { + if (find_watch(w, path)) return 0; + int fd = open(path, O_RDONLY | O_EVTONLY); + if (fd < 0) return -1; + struct kevent change; + EV_SET(&change, (uintptr_t)fd, EVFILT_VNODE, EV_ADD | EV_CLEAR, + NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_DELETE | NOTE_RENAME, + 0, NULL); + if (kevent(w->kqfd, &change, 1, NULL, 0, NULL) < 0) { + close(fd); + return -1; + } + watch_entry_t *entry = calloc(1, sizeof(*entry)); + if (!entry) { close(fd); return -1; } + entry->path = strdup(path); + if (!entry->path) { free(entry); close(fd); return -1; } + entry->fd = fd; + entry->is_dir = is_dir; + entry->next = w->watches; + w->watches = entry; + /* Store the entry for O(1) event-to-path lookup. */ + EV_SET(&change, (uintptr_t)fd, EVFILT_VNODE, EV_ADD | EV_CLEAR, + NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_DELETE | NOTE_RENAME, + 0, entry); + if (kevent(w->kqfd, &change, 1, NULL, 0, NULL) < 0) return -1; + return 0; +} + +static int watch_tree(fswatch_t *w, const char *path, int notify_new) { + struct stat st; + if (lstat(path, &st) != 0 || S_ISLNK(st.st_mode)) return 0; + int is_dir = S_ISDIR(st.st_mode); + int was_known = find_watch(w, path) != NULL; + if (add_watch(w, path, is_dir) != 0) return 0; + int added = 0; + if (notify_new && !was_known) + w->cb(path, FSW_CREATE, w->userdata), added++; + if (!is_dir) return added; + DIR *dir = opendir(path); + if (!dir) return added; + struct dirent *de; + while ((de = readdir(dir)) != NULL) { + if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) + continue; + size_t n = strlen(path) + strlen(de->d_name) + 2; + char *child = malloc(n); + if (!child) continue; + snprintf(child, n, "%s/%s", path, de->d_name); + added += watch_tree(w, child, notify_new); + free(child); + } + closedir(dir); + return added; +} + +fswatch_t *fswatch_init(fswatch_cb cb, void *userdata) { + if (!cb) return NULL; + int kqfd = kqueue(); + if (kqfd < 0) return NULL; + fswatch_t *w = calloc(1, sizeof(*w)); + if (!w) { close(kqfd); return NULL; } + w->kqfd = kqfd; + w->cb = cb; + w->userdata = userdata; + return w; +} + +int fswatch_add(fswatch_t *w, const char *path, int recursive) { + if (!w || !path) return -1; + struct stat st; + if (lstat(path, &st) != 0) return -1; + if (recursive && S_ISDIR(st.st_mode)) (void)watch_tree(w, path, 0); + else if (add_watch(w, path, S_ISDIR(st.st_mode)) != 0) return -1; + return find_watch(w, path) ? 0 : -1; +} + +int fswatch_fd(fswatch_t *w) { return w ? w->kqfd : -1; } + +int fswatch_drain(fswatch_t *w) { + if (!w) return -1; + struct timespec timeout = {0, 0}; + struct kevent events[32]; + int count = 0, n; + while ((n = kevent(w->kqfd, NULL, 0, events, 32, &timeout)) > 0) { + for (int i = 0; i < n; i++) { + watch_entry_t *entry = events[i].udata; + if (!entry) continue; + int flags = 0; + if (events[i].fflags & (NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB)) + flags |= FSW_MODIFY; + if (events[i].fflags & NOTE_DELETE) flags |= FSW_DELETE; + if (events[i].fflags & NOTE_RENAME) flags |= FSW_RENAME; + if (!flags) continue; + if (entry->is_dir && (flags & FSW_MODIFY)) { + /* kqueue supplies no child name: scan only to register new paths. */ + count += watch_tree(w, entry->path, 1); + } else { + w->cb(entry->path, flags, w->userdata); + count++; + } + } + } + return n < 0 && errno != EAGAIN ? -1 : count; +} + +void fswatch_free(fswatch_t *w) { + if (!w) return; + watch_entry_t *entry = w->watches; + while (entry) { + watch_entry_t *next = entry->next; + close(entry->fd); + free(entry->path); + free(entry); + entry = next; + } + close(w->kqfd); + free(w); +} + +#endif /* __APPLE__ */ diff --git a/src/fswatch_noop.c b/src/fswatch_noop.c index ff822583..866440e1 100644 --- a/src/fswatch_noop.c +++ b/src/fswatch_noop.c @@ -4,7 +4,7 @@ * All operations succeed silently but do nothing. This allows Nash * to compile and run on any platform without ifdefs in the callers. */ -#ifndef __linux__ +#if !defined(__linux__) && !defined(__APPLE__) #include "fswatch.h" #include @@ -24,7 +24,9 @@ fswatch_t *fswatch_init(fswatch_cb cb, void *userdata) { } int fswatch_add(fswatch_t *w, const char *path, int recursive) { - (void)w; (void)path; (void)recursive; + (void)w; + (void)path; + (void)recursive; return 0; } @@ -42,4 +44,4 @@ void fswatch_free(fswatch_t *w) { free(w); } -#endif /* !__linux__ */ +#endif /* unsupported platform */ diff --git a/src/journal.c b/src/journal.c index f9315983..4c9ac857 100644 --- a/src/journal.c +++ b/src/journal.c @@ -10,7 +10,7 @@ #include #include #include /* flock */ -#include /* fdatasync, fileno */ +#include /* fdatasync, fileno */ /* Crash handler state: updated by journal_append() so the crash handler * (SIGSEGV/SIGABRT/SIGBUS) in main.c can write a signal_death entry @@ -227,8 +227,21 @@ int journal_append(journal_t *j, int react_loop, int step, const char *tool, fprintf(f, "%s\n", json); free(json); cJSON_Delete(entry); - fflush(f); - fdatasync(fileno(f)); + if (fflush(f) != 0) { + fclose(f); + pthread_mutex_unlock(&j->mtx); + return -1; + } +#if defined(__APPLE__) + int sync_rc = fsync(fileno(f)); +#else + int sync_rc = fdatasync(fileno(f)); +#endif + if (sync_rc != 0) { + fclose(f); + pthread_mutex_unlock(&j->mtx); + return -1; + } fclose(f); pthread_mutex_unlock(&j->mtx); /* FIX CRIT2 */ return 0; diff --git a/src/mailbox.c b/src/mailbox.c index 48f40c21..615b407d 100644 --- a/src/mailbox.c +++ b/src/mailbox.c @@ -6,12 +6,14 @@ #include #include #include -#include #include #include #include #include #include +#ifdef __linux__ +#include +#endif #include "frontend_headless.h" @@ -87,10 +89,6 @@ char *mailbox_ask(const char *mailbox_dir, const char *question, int timeout_sec nash_log("[mailbox] question written: %s", outpath); nash_log("[mailbox] waiting for answer: inbox/ask_%s", msg_id); - /* Wait for answer file in inbox via inotify */ - char inbox_dir[NASH_PATH_MAX]; - snprintf(inbox_dir, sizeof(inbox_dir), "%s/inbox", mailbox_dir); - char answer_file[NASH_PATH_MAX]; snprintf(answer_file, sizeof(answer_file), "%s/inbox/ask_%s", mailbox_dir, msg_id); @@ -100,7 +98,12 @@ char *mailbox_ask(const char *mailbox_dir, const char *question, int timeout_sec answer = read_file(answer_file); if (answer) goto got_answer; - /* Set up inotify */ + /* Keep the existing Linux inotify implementation unchanged. Darwin's + * directory event API does not expose the created filename reliably enough + * for this single-answer protocol, so it polls the atomic answer file. */ +#ifdef __linux__ + char inbox_dir[NASH_PATH_MAX]; + snprintf(inbox_dir, sizeof(inbox_dir), "%s/inbox", mailbox_dir); int ifd = inotify_init1(IN_NONBLOCK); if (ifd < 0) { nash_log("[mailbox] inotify_init failed: %s, falling back to poll", @@ -159,13 +162,11 @@ char *mailbox_ask(const char *mailbox_dir, const char *question, int timeout_sec return NULL; } } - int ret = poll(&pfd, 1, remaining_ms > 0 ? remaining_ms : 5000); if (ret < 0) { if (errno == EINTR) break; /* signal received — let caller check shutdown */ break; } - if (ret > 0) { /* Drain inotify events */ char evbuf[NASH_PATH_MAX] @@ -184,7 +185,6 @@ char *mailbox_ask(const char *mailbox_dir, const char *question, int timeout_sec } } } - /* Periodic check (handles edge cases — read directly, no TOCTOU) */ answer = read_file(answer_file); if (answer) { @@ -206,6 +206,18 @@ char *mailbox_ask(const char *mailbox_dir, const char *question, int timeout_sec answer_file); return NULL; } +#else + time_t deadline = timeout_sec > 0 ? time(NULL) + timeout_sec : 0; + while (!answer) { + if (deadline && time(NULL) >= deadline) { + nash_log("[mailbox] timeout waiting for answer"); + return NULL; + } + struct timespec delay = {.tv_sec = 0, .tv_nsec = 100 * 1000 * 1000}; + nanosleep(&delay, NULL); + answer = read_file(answer_file); + } +#endif got_answer: /* Clean up processed files */ @@ -256,7 +268,10 @@ char *mailbox_wait_task(const char *mailbox_dir, char **task_id_out, int timeout_sec) { char inbox_dir[NASH_PATH_MAX]; snprintf(inbox_dir, sizeof(inbox_dir), "%s/inbox", mailbox_dir); - +#ifdef __APPLE__ + time_t start = time(NULL); +rescan:; +#endif /* First check for command files (cmd_*) — return immediately so * the daemon loop can handle session reset before processing tasks. */ DIR *dir = opendir(inbox_dir); @@ -306,6 +321,16 @@ char *mailbox_wait_task(const char *mailbox_dir, char **task_id_out, closedir(dir); } + /* macOS intentionally polls this mailbox directory. The bridge protocol + * is atomic-file based, so scanning is reliable and avoids translating + * kqueue's directory-level events into Linux inotify filenames. */ +#ifdef __APPLE__ + if (timeout_sec > 0 && time(NULL) - start >= timeout_sec) + return NULL; + struct timespec delay = {.tv_sec = 0, .tv_nsec = 100 * 1000 * 1000}; + nanosleep(&delay, NULL); + goto rescan; +#else /* No existing tasks — watch with inotify */ int ifd = inotify_init1(IN_NONBLOCK); if (ifd < 0) { @@ -444,6 +469,7 @@ char *mailbox_wait_task(const char *mailbox_dir, char **task_id_out, inotify_rm_watch(ifd, wd); close(ifd); return NULL; +#endif } diff --git a/src/main.c b/src/main.c index 6d70efdf..20346e61 100644 --- a/src/main.c +++ b/src/main.c @@ -2092,15 +2092,19 @@ static int run_tui(nash_ctx_t *ctx, const char *query, * On other platforms: fall back to nanosleep (fswatch_fd returns -1). */ { int wfd = fswatcher ? fswatch_fd(fswatcher) : -1; + int watch_ready = 1; if (wfd >= 0) { struct pollfd pfd = {.fd = wfd, .events = POLLIN}; poll(&pfd, 1, 50); /* 50ms timeout */ - if (pfd.revents & POLLIN) - fswatch_drain(fswatcher); + watch_ready = (pfd.revents & POLLIN) != 0; } else { struct timespec ts = {0, 50000000}; nanosleep(&ts, NULL); /* 50ms fallback */ } + /* Native backends expose a pollable descriptor; the no-op backend + * remains non-pollable and simply has nothing to drain. */ + if (fswatcher && watch_ready) + fswatch_drain(fswatcher); } } diff --git a/src/matrix.c b/src/matrix.c index be1793d6..6e5b6432 100644 --- a/src/matrix.c +++ b/src/matrix.c @@ -28,12 +28,28 @@ #include #include #include +#ifdef __linux__ #include +#endif #include #include #include #include +/* macOS does not expose explicit_bzero with the POSIX feature level used by + * this project. The compiler barrier prevents the fallback memset being + * optimized away while clearing credentials. */ +#ifdef __APPLE__ +static void explicit_bzero(void *buf, size_t len) { +#if defined(__STDC_LIB_EXT1__) + memset_s(buf, len, 0, len); +#else + memset(buf, 0, len); + __asm__ __volatile__("" : : "r"(buf) : "memory"); +#endif +} +#endif + /* Matrix API constants */ #define MX_SYNC_TIMEOUT 5000 /* /sync timeout in ms (5 seconds) */ #define MX_RETRY_DELAY 5 /* seconds to wait after API error */ @@ -2386,10 +2402,12 @@ void *matrix_run(void *arg) { } } - /* Set up inotify on outbox */ + /* Linux uses inotify for prompt delivery; other platforms scan the atomic + * mailbox outbox after each Matrix sync. */ char outbox_path[512]; snprintf(outbox_path, sizeof(outbox_path), "%s/outbox", ctx->mailbox_dir); +#ifdef __linux__ int ifd = inotify_init1(IN_NONBLOCK); int iwd = -1; if (ifd >= 0) { @@ -2402,6 +2420,7 @@ void *matrix_run(void *arg) { fprintf(stderr, "[matrix] inotify_init failed: %s (will use polling)\n", strerror(errno)); } +#endif /* Pending ask ID and room for routing replies as answers */ char pending_ask_id[128] = {0}; @@ -2716,7 +2735,8 @@ void *matrix_run(void *arg) { if (*ctx->shutdown) break; - /* ── Phase 2: Check outbox ────────────────────────────── */ +/* ── Phase 2: Check outbox ────────────────────────────── */ +#ifdef __linux__ if (ifd >= 0) { char evbuf[NASH_PATH_MAX] __attribute__((aligned(__alignof__(struct inotify_event)))); @@ -2766,6 +2786,9 @@ void *matrix_run(void *arg) { } else { mx_scan_outbox(ctx); } +#else + mx_scan_outbox(ctx); +#endif /* Periodically save since_token */ if (++save_counter >= 60) { /* every ~60 sync cycles ≈ 5 min */ @@ -2784,8 +2807,10 @@ void *matrix_run(void *arg) { mx_api_send_message(ctx, "🔴 Nash bot going offline", NULL); /* Cleanup */ +#ifdef __linux__ if (iwd >= 0) inotify_rm_watch(ifd, iwd); if (ifd >= 0) close(ifd); +#endif /* Save final since_token */ mx_config_save(ctx); diff --git a/src/subprocess.c b/src/subprocess.c index e1e301f1..e72d2506 100644 --- a/src/subprocess.c +++ b/src/subprocess.c @@ -33,7 +33,11 @@ static void scrub_env(void) { static void close_extra_fds(int keep_fd) { /* Prefer iterating /proc/self/fd for O(open_fds) instead of * O(sysconf(_SC_OPEN_MAX)) which can be up to 1M close() calls. */ +#ifdef __APPLE__ + DIR *dp = opendir("/dev/fd"); +#else DIR *dp = opendir("/proc/self/fd"); +#endif if (dp) { int dir_fd = dirfd(dp); struct dirent *de; @@ -116,7 +120,17 @@ subprocess_result_t subprocess_run(char *const argv[], subprocess_result_t r = {.exit_code = -1}; int pipefd[2]; +#ifdef __APPLE__ + if (pipe(pipefd) < 0) return r; + if (fcntl(pipefd[0], F_SETFD, FD_CLOEXEC) < 0 || + fcntl(pipefd[1], F_SETFD, FD_CLOEXEC) < 0) { + close(pipefd[0]); + close(pipefd[1]); + return r; + } +#else if (pipe2(pipefd, O_CLOEXEC) < 0) return r; +#endif pid_t pid = fork(); if (pid < 0) { @@ -181,7 +195,7 @@ subprocess_result_t subprocess_run(char *const argv[], if (n == 0) break; /* EOF */ if (n < 0) { if (errno == EAGAIN || errno == EINTR) continue; /* transient */ - break; /* real error */ + break; /* real error */ } /* Enforce byte cap with partial write */ diff --git a/src/telegram.c b/src/telegram.c index ce0be330..92117ccb 100644 --- a/src/telegram.c +++ b/src/telegram.c @@ -29,7 +29,9 @@ #include #include #include +#ifdef __linux__ #include +#endif #include #include #include @@ -185,8 +187,11 @@ static long long tg_api_create_forum_topic(telegram_ctx_t *ctx, TG_API_BASE, ctx->bot_token); cJSON *body = cJSON_CreateObject(); - { char _id[32]; snprintf(_id, sizeof(_id), "%lld", ctx->chat_id); - cJSON_AddRawToObject(body, "chat_id", _id); } + { + char _id[32]; + snprintf(_id, sizeof(_id), "%lld", ctx->chat_id); + cJSON_AddRawToObject(body, "chat_id", _id); + } cJSON_AddStringToObject(body, "name", name); char *body_str = cJSON_PrintUnformatted(body); @@ -680,15 +685,20 @@ static long long tg_api_send_raw(telegram_ctx_t *ctx, const char *text, /* Build JSON body */ cJSON *body = cJSON_CreateObject(); - { char _id[32]; snprintf(_id, sizeof(_id), "%lld", ctx->chat_id); - cJSON_AddRawToObject(body, "chat_id", _id); } + { + char _id[32]; + snprintf(_id, sizeof(_id), "%lld", ctx->chat_id); + cJSON_AddRawToObject(body, "chat_id", _id); + } if (thread_id != 0) { - char _tid[32]; snprintf(_tid, sizeof(_tid), "%lld", thread_id); + char _tid[32]; + snprintf(_tid, sizeof(_tid), "%lld", thread_id); cJSON_AddRawToObject(body, "message_thread_id", _tid); } if (reply_to_message_id != 0) { cJSON *reply_params = cJSON_CreateObject(); - char _rid[32]; snprintf(_rid, sizeof(_rid), "%lld", reply_to_message_id); + char _rid[32]; + snprintf(_rid, sizeof(_rid), "%lld", reply_to_message_id); cJSON_AddRawToObject(reply_params, "message_id", _rid); cJSON_AddItemToObject(body, "reply_parameters", reply_params); } @@ -856,10 +866,14 @@ static int tg_api_send_rich(telegram_ctx_t *ctx, const char *md_text, /* Build JSON body */ cJSON *body = cJSON_CreateObject(); - { char _id[32]; snprintf(_id, sizeof(_id), "%lld", ctx->chat_id); - cJSON_AddRawToObject(body, "chat_id", _id); } + { + char _id[32]; + snprintf(_id, sizeof(_id), "%lld", ctx->chat_id); + cJSON_AddRawToObject(body, "chat_id", _id); + } if (thread_id != 0) { - char _tid[32]; snprintf(_tid, sizeof(_tid), "%lld", thread_id); + char _tid[32]; + snprintf(_tid, sizeof(_tid), "%lld", thread_id); cJSON_AddRawToObject(body, "message_thread_id", _tid); } cJSON_AddStringToObject(body, "rich_text", md_text); @@ -1395,10 +1409,12 @@ void *telegram_run(void *arg) { fprintf(stderr, "[telegram] bridge thread started\n"); - /* Set up inotify on outbox */ + /* Linux uses inotify for prompt delivery; other platforms scan the atomic + * mailbox outbox after each short Telegram poll. */ char outbox_path[512]; snprintf(outbox_path, sizeof(outbox_path), "%s/outbox", ctx->mailbox_dir); +#ifdef __linux__ int ifd = inotify_init1(IN_NONBLOCK); int iwd = -1; if (ifd >= 0) { @@ -1411,6 +1427,7 @@ void *telegram_run(void *arg) { fprintf(stderr, "[telegram] inotify_init failed: %s (will use polling)\n", strerror(errno)); } +#endif /* Process any existing outbox files */ tg_scan_outbox(ctx); @@ -1696,7 +1713,8 @@ void *telegram_run(void *arg) { if (*ctx->shutdown) break; - /* ── Phase 2: Check outbox for results/questions ─────────── */ +/* ── Phase 2: Check outbox for results/questions ─────────── */ +#ifdef __linux__ if (ifd >= 0) { /* Read inotify events (non-blocking) */ char evbuf[NASH_PATH_MAX] @@ -1721,6 +1739,9 @@ void *telegram_run(void *arg) { /* Fallback: poll-based outbox scan */ tg_scan_outbox(ctx); } +#else + tg_scan_outbox(ctx); +#endif /* Periodically re-sync workspaces (every ~30 iterations ≈ 60s) */ if (++ws_sync_counter >= 30) { @@ -1730,8 +1751,10 @@ void *telegram_run(void *arg) { } /* Cleanup */ +#ifdef __linux__ if (iwd >= 0) inotify_rm_watch(ifd, iwd); if (ifd >= 0) close(ifd); +#endif fprintf(stderr, "[telegram] bridge thread stopped\n"); return NULL; diff --git a/src/tool_plugin.c b/src/tool_plugin.c index 04cdc03e..27111134 100644 --- a/src/tool_plugin.c +++ b/src/tool_plugin.c @@ -214,8 +214,12 @@ int tool_plugin_load_dir(const char *dir_path) { while ((ent = readdir(d)) != NULL) { const char *name = ent->d_name; size_t len = strlen(name); - if (len < 4 || strcmp(name + len - 3, ".so") != 0) - continue; + int is_plugin = len >= 4 && strcmp(name + len - 3, ".so") == 0; +#ifdef __APPLE__ + is_plugin = is_plugin || + (len >= 7 && strcmp(name + len - 6, ".dylib") == 0); +#endif + if (!is_plugin) continue; char path[4096]; path_join(path, sizeof(path), dir_path, name); diff --git a/src/tools.c b/src/tools.c index ea7791f7..8a524a9d 100644 --- a/src/tools.c +++ b/src/tools.c @@ -1013,9 +1013,19 @@ cJSON *plan_replay_journal_dir(const char *session_dir) { cJSON *plan_subtask_links(const char *session_dir) { cJSON *root = plan_replay_journal_dir(session_dir); if (!root) return cJSON_CreateObject(); - cJSON *links = cJSON_DetachItemFromObject(root, "subtask_links"); + cJSON *full_links = cJSON_GetObjectItem(root, "subtask_links"); + /* This public helper predates link metadata and returns the original + * child-name -> parent-step mapping. Keep that ABI while the replay root + * retains richer objects (step plus optional result ref) for rendering. */ + cJSON *links = cJSON_CreateObject(); + cJSON *item; + cJSON_ArrayForEach(item, full_links) { + int step = cJSON_IsNumber(item) ? (int)item->valuedouble + : json_int(item, "step", 0); + cJSON_AddNumberToObject(links, item->string, step); + } cJSON_Delete(root); - return links ? links : cJSON_CreateObject(); + return links; } /* Load plan steps by replaying journal. Returns cJSON array (caller owns) @@ -1690,6 +1700,14 @@ static tool_result_t tool_plan(tool_ctx_t *ctx, cJSON *params) { tools_inject_thought(ctx, params); tool_journal(ctx, "plan", params, alias, 0, done, NULL, NULL); + /* The parent journal may contain subtask spawns between plan calls. + * Replaying after this check lets the scratchpad interleave their plans + * under the step that was active at spawn time. */ + cJSON *replayed = plan_replay_journal_dir(ctx->session_dir); + if (replayed) { + plan_project_to_scratchpad(ctx, replayed); + cJSON_Delete(replayed); + } cJSON_Delete(steps); char *ref_copy = alias ? xstrdup(alias) : NULL; free(alias); @@ -1951,7 +1969,10 @@ static const tool_param_t done_params[] = { TOOL_PARAM_END}; static const tool_param_t plan_params[] = { - TOOL_PARAM("op", "string", "Operation: add_item, done, check, uncheck, status", 1), + /* Either op (the incremental API) or result (the legacy numbered-plan + * API) is required. This cannot be expressed by the flat parameter + * descriptor, so tool_plan() performs the combined validation. */ + TOOL_PARAM("op", "string", "Operation: add_item, done, check, uncheck, status", 0), TOOL_PARAM("text", "string", "Step description (for add_item)", 0), TOOL_PARAM("step", "integer", "Step number to check/uncheck (1-based)", 0), TOOL_PARAM("evidence", "string", "Ref (e.g. R0S5) proving step completion", 0), diff --git a/tests/test_fswatch.c b/tests/test_fswatch.c index 1aa3c695..1ecebd01 100644 --- a/tests/test_fswatch.c +++ b/tests/test_fswatch.c @@ -24,15 +24,15 @@ static int g_pass = 0; static int g_fail = 0; -#define ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ +#define ASSERT(cond, msg) \ + do { \ + if (!(cond)) { \ fprintf(stderr, " FAIL: %s (line %d)\n", msg, __LINE__); \ - g_fail++; \ - } else { \ - printf(" PASS: %s\n", msg); \ - g_pass++; \ - } \ + g_fail++; \ + } else { \ + printf(" PASS: %s\n", msg); \ + g_pass++; \ + } \ } while (0) /* Callback state */ @@ -52,7 +52,7 @@ static void test_cb(const char *path, int event, void *userdata) { st->count++; } -/* Wait for inotify events (up to timeout_ms). Returns events drained. */ +/* Wait for backend events (up to timeout_ms). Returns events drained. */ static int wait_and_drain(fswatch_t *w, int timeout_ms) { int fd = fswatch_fd(w); if (fd >= 0) { @@ -81,7 +81,10 @@ static char *make_tmpdir(void) { /* Write a string to a file */ static void write_file(const char *path, const char *content) { FILE *f = fopen(path, "w"); - if (!f) { perror(path); return; } + if (!f) { + perror(path); + return; + } fputs(content, f); fclose(f); } @@ -120,8 +123,8 @@ static void test_add_watch(void) { ASSERT(rc == 0, "fswatch_add succeeds for tmpdir"); int fd = fswatch_fd(w); -#ifdef __linux__ - ASSERT(fd >= 0, "fswatch_fd returns valid fd on Linux"); +#if defined(__linux__) || defined(__APPLE__) + ASSERT(fd >= 0, "fswatch_fd returns a valid native watcher fd"); #else ASSERT(fd == -1, "fswatch_fd returns -1 on non-Linux"); #endif @@ -142,8 +145,8 @@ static void test_create_detect(void) { snprintf(path, sizeof(path), "%s/newfile.txt", dir); write_file(path, "hello"); - int n = wait_and_drain(w, 200); -#ifdef __linux__ + int n = wait_and_drain(w, 1000); +#if defined(__linux__) || defined(__APPLE__) ASSERT(n > 0, "events detected after file creation"); ASSERT(st.count > 0, "callback invoked"); ASSERT(strstr(st.last_path, "newfile.txt") != NULL, @@ -176,8 +179,8 @@ static void test_modify_detect(void) { /* Modify the file */ write_file(path, "modified content"); - int n = wait_and_drain(w, 200); -#ifdef __linux__ + int n = wait_and_drain(w, 1000); +#if defined(__linux__) || defined(__APPLE__) ASSERT(n > 0, "events detected after file modification"); ASSERT(st.last_event & FSW_MODIFY, "event includes FSW_MODIFY"); #else @@ -204,8 +207,8 @@ static void test_delete_detect(void) { st.count = 0; unlink(path); - int n = wait_and_drain(w, 200); -#ifdef __linux__ + int n = wait_and_drain(w, 1000); +#if defined(__linux__) || defined(__APPLE__) ASSERT(n > 0, "events detected after file deletion"); ASSERT(st.last_event & FSW_DELETE, "event includes FSW_DELETE"); #else @@ -235,8 +238,8 @@ static void test_recursive_watch(void) { snprintf(path, sizeof(path), "%s/sub/deep.txt", dir); write_file(path, "deep content"); - int n = wait_and_drain(w, 200); -#ifdef __linux__ + int n = wait_and_drain(w, 1000); +#if defined(__linux__) || defined(__APPLE__) ASSERT(n > 0, "events detected in subdirectory"); ASSERT(strstr(st.last_path, "deep.txt") != NULL, "callback path contains subdirectory filename"); @@ -270,8 +273,8 @@ static void test_auto_watch_new_subdir(void) { snprintf(path, sizeof(path), "%s/newsubdir/auto.txt", dir); write_file(path, "auto-watched"); - int n = wait_and_drain(w, 200); -#ifdef __linux__ + int n = wait_and_drain(w, 1000); +#if defined(__linux__) || defined(__APPLE__) ASSERT(n > 0, "events detected in auto-watched new subdirectory"); ASSERT(strstr(st.last_path, "auto.txt") != NULL, "callback path contains new subdir filename"); @@ -302,13 +305,17 @@ static void test_hidden_dirs_skipped(void) { snprintf(path, sizeof(path), "%s/.hidden/secret.txt", dir); write_file(path, "hidden content"); - int n = wait_and_drain(w, 200); + int n = wait_and_drain(w, 1000); #ifdef __linux__ /* The hidden directory is not watched, so no events for files inside it. * However, the parent dir IS watched, so creating .hidden itself * generates an event. The file inside .hidden should NOT. */ int found_secret = (strstr(st.last_path, "secret.txt") != NULL); ASSERT(!found_secret, "hidden directory contents not watched"); +#elif defined(__APPLE__) + ASSERT(n > 0, "events detected in hidden subdirectory"); + ASSERT(strstr(st.last_path, "secret.txt") != NULL, + "kqueue reports hidden-directory contents"); #else (void)n; ASSERT(1, "noop backend - skip hidden dir test"); diff --git a/tests/test_onnx_embed.c b/tests/test_onnx_embed.c index 87a75b4e..fc53c111 100644 --- a/tests/test_onnx_embed.c +++ b/tests/test_onnx_embed.c @@ -1,6 +1,7 @@ #include #include #include +#include #include "embedding_onnx.h" int main(void) { @@ -8,11 +9,16 @@ int main(void) { const char *home = getenv("HOME"); char path[4096]; if (home) { - snprintf(path, sizeof(path), "%s/models/all-MiniLM-L6-v2", home); + /* ~/.nash/models is the current setup location. Keep the older + * ~/models location working for users who installed the model there. */ + snprintf(path, sizeof(path), "%s/.nash/models/all-MiniLM-L6-v2", home); + if (access(path, F_OK) != 0) + snprintf(path, sizeof(path), "%s/models/all-MiniLM-L6-v2", home); model_dir = path; } - printf("Initializing ONNX embedding from: %s\n", model_dir); + printf("Initializing ONNX embedding from: %s\n", + model_dir ? model_dir : "(HOME is not set)"); onnx_embed_ctx_t *ctx = onnx_embed_init(model_dir); if (!ctx) { fprintf(stderr, "Failed to initialize ONNX embedding\n"); diff --git a/tests/test_tool_plugin_dlopen.c b/tests/test_tool_plugin_dlopen.c index 0569a567..f77b1c23 100644 --- a/tests/test_tool_plugin_dlopen.c +++ b/tests/test_tool_plugin_dlopen.c @@ -27,22 +27,29 @@ /* Provide globals that linked modules reference */ int g_path_given = 0; +#ifdef __APPLE__ +#define PLUGIN_EXT ".dylib" +#else +#define PLUGIN_EXT ".so" +#endif + /* Helper: get path relative to test binary location. * Tests are run from the project root, so "tests/X.so" works. */ -static const char *SAMPLE_SO = "tests/sample_plugin.so"; -static const char *BAD_ABI_SO = "tests/sample_plugin_bad_abi.so"; -static const char *MULTI_SO = "tests/sample_plugin_multi.so"; +static const char *SAMPLE_SO = "tests/sample_plugin" PLUGIN_EXT; +static const char *BAD_ABI_SO = "tests/sample_plugin_bad_abi" PLUGIN_EXT; +static const char *MULTI_SO = "tests/sample_plugin_multi" PLUGIN_EXT; static const char *PLUGIN_DIR = "tests/plugin_dir"; /* ---- Helper to set up a temp plugin directory ---- */ static void setup_plugin_dir(void) { mkdir(PLUGIN_DIR, 0755); - /* Symlink sample_plugin.so and multi .so into the dir */ + /* Symlink sample plugins into the dir */ char cmd[512]; snprintf(cmd, sizeof(cmd), - "ln -sf $(pwd)/tests/sample_plugin.so %s/sample_plugin.so && " - "ln -sf $(pwd)/tests/sample_plugin_multi.so %s/sample_plugin_multi.so", - PLUGIN_DIR, PLUGIN_DIR); + "ln -sf $(pwd)/tests/sample_plugin%s %s/sample_plugin%s && " + "ln -sf $(pwd)/tests/sample_plugin_multi%s %s/sample_plugin_multi%s", + PLUGIN_EXT, PLUGIN_DIR, PLUGIN_EXT, + PLUGIN_EXT, PLUGIN_DIR, PLUGIN_EXT); system(cmd); } From ac5d9f17c14aa0fd867470288c80df8fd1afd864 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 24 Sep 2026 11:38:50 -0700 Subject: [PATCH 2/3] Address Copilot review findings for macOS support Apply the fixes identified by Copilot review: place Homebrew linker paths before dependent libraries, use a project-specific secure-zero helper, and recreate reusable test containers when their ONNX model mount is missing or stale. Harden the kqueue watcher by removing invalidated vnode entries before re-registration and preserving shallow-watch semantics. Add macOS regressions for recreated paths and non-recursive directory watches. Signed-off-by: Jhon Honce --- Makefile | 8 +++- src/fswatch_kqueue.c | 102 ++++++++++++++++++++++++++++++++++++------- src/matrix.c | 15 +++---- tests/test_fswatch.c | 60 +++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index d351f35c..e6bbd860 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ endif # Device subsystem (VNC, HEVC streaming, Tesseract OCR) is now a separate # plugin: nash-tool-device-control. See ~/agents/nash-tool-device-control/ -LDFLAGS ?= $(EXPORT_DYNAMIC) -lcurl -lcrypto -lreadline $(NCURSES_LIB) -lpthread -lm -lutf8proc $(DL_LIB) $(BREW_LDFLAGS) $(ORT_LDFLAGS) +LDFLAGS ?= $(BREW_LDFLAGS) $(EXPORT_DYNAMIC) -lcurl -lcrypto -lreadline $(NCURSES_LIB) -lpthread -lm -lutf8proc $(DL_LIB) $(ORT_LDFLAGS) # AddressSanitizer for heap corruption detection (opt-in: make SANITIZE=1) ifdef SANITIZE @@ -200,7 +200,11 @@ test: $(TEST_BIN) TEST_CONTAINER ?= nash-test-model NASH_MODEL_DIR ?= $(HOME)/.nash/models/all-MiniLM-L6-v2 test-container: - @if podman container exists $(TEST_CONTAINER) 2>/dev/null; then \ + @if podman container exists $(TEST_CONTAINER) 2>/dev/null && \ + ! podman inspect -f '{{range .Mounts}}{{if eq .Destination "/root/.nash/models/all-MiniLM-L6-v2"}}{{.Source}}{{end}}{{end}}' $(TEST_CONTAINER) | grep -Fxq '$(NASH_MODEL_DIR)'; then \ + podman rm -f $(TEST_CONTAINER); \ + fi; \ + if podman container exists $(TEST_CONTAINER) 2>/dev/null; then \ podman start $(TEST_CONTAINER) 2>/dev/null || true; \ else \ podman run --name $(TEST_CONTAINER) -d \ diff --git a/src/fswatch_kqueue.c b/src/fswatch_kqueue.c index b7c5fe02..f3ca63e2 100644 --- a/src/fswatch_kqueue.c +++ b/src/fswatch_kqueue.c @@ -20,6 +20,7 @@ typedef struct watch_entry { int fd; int is_dir; + int recursive; char *path; struct watch_entry *next; } watch_entry_t; @@ -37,31 +38,59 @@ static watch_entry_t *find_watch(fswatch_t *w, const char *path) { return NULL; } -static int add_watch(fswatch_t *w, const char *path, int is_dir) { - if (find_watch(w, path)) return 0; +static void remove_watches_at_or_below(fswatch_t *w, const char *path) { + size_t path_len = strlen(path); + watch_entry_t **pp = &w->watches; + while (*pp) { + watch_entry_t *entry = *pp; + if (strcmp(entry->path, path) == 0 || + (strncmp(entry->path, path, path_len) == 0 && + entry->path[path_len] == '/')) { + *pp = entry->next; + close(entry->fd); + free(entry->path); + free(entry); + } else { + pp = &entry->next; + } + } +} + +static int watches_same_vnode(const watch_entry_t *entry, const char *path) { + struct stat watched, current; + return fstat(entry->fd, &watched) == 0 && lstat(path, ¤t) == 0 && + watched.st_dev == current.st_dev && watched.st_ino == current.st_ino; +} + +static int add_watch(fswatch_t *w, const char *path, int is_dir, int recursive) { + watch_entry_t *existing = find_watch(w, path); + if (existing) { + if (watches_same_vnode(existing, path)) { + existing->recursive |= recursive; + return 0; + } + remove_watches_at_or_below(w, path); + } int fd = open(path, O_RDONLY | O_EVTONLY); if (fd < 0) return -1; struct kevent change; - EV_SET(&change, (uintptr_t)fd, EVFILT_VNODE, EV_ADD | EV_CLEAR, - NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_DELETE | NOTE_RENAME, - 0, NULL); - if (kevent(w->kqfd, &change, 1, NULL, 0, NULL) < 0) { - close(fd); - return -1; - } watch_entry_t *entry = calloc(1, sizeof(*entry)); if (!entry) { close(fd); return -1; } entry->path = strdup(path); if (!entry->path) { free(entry); close(fd); return -1; } entry->fd = fd; entry->is_dir = is_dir; + entry->recursive = recursive; entry->next = w->watches; w->watches = entry; /* Store the entry for O(1) event-to-path lookup. */ EV_SET(&change, (uintptr_t)fd, EVFILT_VNODE, EV_ADD | EV_CLEAR, NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_DELETE | NOTE_RENAME, 0, entry); - if (kevent(w->kqfd, &change, 1, NULL, 0, NULL) < 0) return -1; + if (kevent(w->kqfd, &change, 1, NULL, 0, NULL) < 0) { + remove_watches_at_or_below(w, path); + return -1; + } return 0; } @@ -70,7 +99,7 @@ static int watch_tree(fswatch_t *w, const char *path, int notify_new) { if (lstat(path, &st) != 0 || S_ISLNK(st.st_mode)) return 0; int is_dir = S_ISDIR(st.st_mode); int was_known = find_watch(w, path) != NULL; - if (add_watch(w, path, is_dir) != 0) return 0; + if (add_watch(w, path, is_dir, 1) != 0) return 0; int added = 0; if (notify_new && !was_known) w->cb(path, FSW_CREATE, w->userdata), added++; @@ -92,6 +121,40 @@ static int watch_tree(fswatch_t *w, const char *path, int notify_new) { return added; } +/* kqueue reports a directory change without the child name. A shallow + * watch scans only direct children so it can report and subsequently watch + * files in that directory without becoming recursive. */ +static int watch_direct_files(fswatch_t *w, const char *path, int notify_new) { + DIR *dir = opendir(path); + if (!dir) return 0; + int added = 0; + struct dirent *de; + while ((de = readdir(dir)) != NULL) { + if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) + continue; + size_t n = strlen(path) + strlen(de->d_name) + 2; + char *child = malloc(n); + if (!child) continue; + snprintf(child, n, "%s/%s", path, de->d_name); + struct stat st; + watch_entry_t *known = find_watch(w, child); + if (lstat(child, &st) == 0 && !S_ISLNK(st.st_mode)) { + if (!S_ISDIR(st.st_mode)) { + if (add_watch(w, child, 0, 0) == 0 && notify_new && !known) { + w->cb(child, FSW_CREATE, w->userdata); + added++; + } + } else if (notify_new && !known) { + w->cb(child, FSW_CREATE, w->userdata); + added++; + } + } + free(child); + } + closedir(dir); + return added; +} + fswatch_t *fswatch_init(fswatch_cb cb, void *userdata) { if (!cb) return NULL; int kqfd = kqueue(); @@ -109,7 +172,7 @@ int fswatch_add(fswatch_t *w, const char *path, int recursive) { struct stat st; if (lstat(path, &st) != 0) return -1; if (recursive && S_ISDIR(st.st_mode)) (void)watch_tree(w, path, 0); - else if (add_watch(w, path, S_ISDIR(st.st_mode)) != 0) return -1; + else if (add_watch(w, path, S_ISDIR(st.st_mode), 0) != 0) return -1; return find_watch(w, path) ? 0 : -1; } @@ -130,9 +193,18 @@ int fswatch_drain(fswatch_t *w) { if (events[i].fflags & NOTE_DELETE) flags |= FSW_DELETE; if (events[i].fflags & NOTE_RENAME) flags |= FSW_RENAME; if (!flags) continue; - if (entry->is_dir && (flags & FSW_MODIFY)) { - /* kqueue supplies no child name: scan only to register new paths. */ - count += watch_tree(w, entry->path, 1); + if (flags & (FSW_DELETE | FSW_RENAME)) { + char *path = strdup(entry->path); + if (!path) return -1; + w->cb(path, flags, w->userdata); + count++; + remove_watches_at_or_below(w, path); + free(path); + } else if (entry->is_dir && (flags & FSW_MODIFY)) { + /* kqueue supplies no child name: scan for new paths at the watch's + * configured depth. */ + count += entry->recursive ? watch_tree(w, entry->path, 1) + : watch_direct_files(w, entry->path, 1); } else { w->cb(entry->path, flags, w->userdata); count++; diff --git a/src/matrix.c b/src/matrix.c index 6e5b6432..72fcb22e 100644 --- a/src/matrix.c +++ b/src/matrix.c @@ -36,11 +36,11 @@ #include #include -/* macOS does not expose explicit_bzero with the POSIX feature level used by - * this project. The compiler barrier prevents the fallback memset being - * optimized away while clearing credentials. */ -#ifdef __APPLE__ -static void explicit_bzero(void *buf, size_t len) { +/* Use a project-specific helper rather than relying on explicit_bzero(), + * whose availability varies with the platform SDK and feature level. The + * compiler barrier prevents the fallback memset being optimized away while + * clearing credentials. */ +static void mx_secure_zero(void *buf, size_t len) { #if defined(__STDC_LIB_EXT1__) memset_s(buf, len, 0, len); #else @@ -48,7 +48,6 @@ static void explicit_bzero(void *buf, size_t len) { __asm__ __volatile__("" : : "r"(buf) : "memory"); #endif } -#endif /* Matrix API constants */ #define MX_SYNC_TIMEOUT 5000 /* /sync timeout in ms (5 seconds) */ @@ -608,11 +607,11 @@ int matrix_setup(matrix_ctx_t *ctx) { /* Step 3: Login */ if (mx_api_login(ctx, username, buf) != 0) { - explicit_bzero(buf, sizeof(buf)); + mx_secure_zero(buf, sizeof(buf)); fprintf(stderr, "[matrix] ✗ Login failed\n"); return -1; } - explicit_bzero(buf, sizeof(buf)); /* clear password from stack */ + mx_secure_zero(buf, sizeof(buf)); /* clear password from stack */ fprintf(stderr, "[matrix] ✓ Logged in as %s\n\n", ctx->user_id); /* Step 4: Room setup */ diff --git a/tests/test_fswatch.c b/tests/test_fswatch.c index 1ecebd01..735c7def 100644 --- a/tests/test_fswatch.c +++ b/tests/test_fswatch.c @@ -325,6 +325,64 @@ static void test_hidden_dirs_skipped(void) { rmrf(dir); } +static void test_recreate_watch(void) { + printf("\n--- test_recreate_watch ---\n"); + cb_state_t st = {0}; + fswatch_t *w = fswatch_init(test_cb, &st); + char *dir = make_tmpdir(); + char path[PATH_MAX]; + snprintf(path, sizeof(path), "%s/recreated.txt", dir); + write_file(path, "before"); + + ASSERT(fswatch_add(w, path, 0) == 0, "watch initial file"); + unlink(path); + wait_and_drain(w, 1000); + write_file(path, "after"); + ASSERT(fswatch_add(w, path, 0) == 0, "watch recreated file"); + st.count = 0; + write_file(path, "updated"); + int n = wait_and_drain(w, 1000); +#ifdef __APPLE__ + ASSERT(n > 0, "events detected after recreating watched path"); + ASSERT(strstr(st.last_path, "recreated.txt") != NULL, + "callback path is recreated file"); +#else + (void)n; + ASSERT(1, "recreate watch test is specific to kqueue"); +#endif + + fswatch_free(w); + rmrf(dir); +} + +static void test_nonrecursive_watch_stays_shallow(void) { + printf("\n--- test_nonrecursive_watch_stays_shallow ---\n"); + cb_state_t st = {0}; + fswatch_t *w = fswatch_init(test_cb, &st); + char *dir = make_tmpdir(); + char subdir[PATH_MAX], rootfile[PATH_MAX], nested[PATH_MAX]; + snprintf(subdir, sizeof(subdir), "%s/sub", dir); + snprintf(rootfile, sizeof(rootfile), "%s/root.txt", dir); + snprintf(nested, sizeof(nested), "%s/sub/deep.txt", dir); + mkdir(subdir, 0755); + + ASSERT(fswatch_add(w, dir, 0) == 0, "add non-recursive directory watch"); + write_file(rootfile, "root event"); + wait_and_drain(w, 1000); + st.count = 0; + write_file(nested, "nested event"); + int n = wait_and_drain(w, 250); +#ifdef __APPLE__ + ASSERT(n == 0, "non-recursive watch ignores nested changes"); +#else + (void)n; + ASSERT(1, "non-recursive behavior is tested by the kqueue backend"); +#endif + + fswatch_free(w); + rmrf(dir); +} + int main(void) { printf("=== test_fswatch ===\n"); @@ -336,6 +394,8 @@ int main(void) { test_recursive_watch(); test_auto_watch_new_subdir(); test_hidden_dirs_skipped(); + test_recreate_watch(); + test_nonrecursive_watch_stays_shallow(); printf("\n=== Results: %d passed, %d failed ===\n", g_pass, g_fail); return g_fail > 0 ? 1 : 0; From d94314c2ec956c943fe31b6761c989a79df8b50d Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 24 Sep 2026 12:03:16 -0700 Subject: [PATCH 3/3] Address additional Copilot review findings Apply the latest Copilot review fixes: add an install-relative Darwin run path, keep kqueue event userdata valid through a drain batch, and seed shallow watcher file entries without emitting false create events. Add macOS regressions for existing direct files and recursive tree removal. Signed-off-by: Jhon Honce --- Makefile | 4 +++- src/fswatch_kqueue.c | 51 +++++++++++++++++++++++++++++++----------- tests/test_fswatch.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index e6bbd860..2c25a43d 100644 --- a/Makefile +++ b/Makefile @@ -15,6 +15,7 @@ ifeq ($(UNAME_S),Linux) DL_LIB := -ldl NCURSES_LIB := -lncursesw PLUGIN_EXT := so + BIN_RPATH := -Wl,-rpath,'$(RPATH_ORIGIN)' else ifeq ($(UNAME_S),Darwin) PLATFORM_DEFINES := -D_DARWIN_C_SOURCE SHARED_EXT := dylib @@ -25,6 +26,7 @@ else ifeq ($(UNAME_S),Darwin) DL_LIB := NCURSES_LIB := -lncurses PLUGIN_EXT := dylib + BIN_RPATH := -Wl,-rpath,'$(RPATH_ORIGIN)' -Wl,-rpath,'@loader_path/../lib' BREW_PACKAGES := ncurses readline openssl@3 utf8proc onnxruntime BREW_CFLAGS := $(foreach p,$(BREW_PACKAGES),$(shell brew --prefix $(p) 2>/dev/null | sed 's|^|-I|; s|$$|/include|')) BREW_LDFLAGS := $(foreach p,$(BREW_PACKAGES),$(shell brew --prefix $(p) 2>/dev/null | sed 's|^|-L|; s|$$|/lib|')) @@ -152,7 +154,7 @@ $(LIB_REAL): $(LIB_OBJ) # Binary: main.o links against libnash.so $(BIN): src/main.o $(LIB_REAL) - $(CC) $(CFLAGS) -o $@ $< -L. -lnash -Wl,-rpath,'$(RPATH_ORIGIN)' $(LDFLAGS) + $(CC) $(CFLAGS) -o $@ $< -L. -lnash $(BIN_RPATH) $(LDFLAGS) # Test binaries TEST_BIN = tests/test_memory tests/test_store tests/test_config \ diff --git a/src/fswatch_kqueue.c b/src/fswatch_kqueue.c index f3ca63e2..34bcfa10 100644 --- a/src/fswatch_kqueue.c +++ b/src/fswatch_kqueue.c @@ -30,6 +30,7 @@ struct fswatch { fswatch_cb cb; void *userdata; watch_entry_t *watches; + watch_entry_t *retired; }; static watch_entry_t *find_watch(fswatch_t *w, const char *path) { @@ -38,6 +39,8 @@ static watch_entry_t *find_watch(fswatch_t *w, const char *path) { return NULL; } +/* Detached watches stay alive until the current drain completes because a + * kqueue batch can still carry their udata pointers. */ static void remove_watches_at_or_below(fswatch_t *w, const char *path) { size_t path_len = strlen(path); watch_entry_t **pp = &w->watches; @@ -48,14 +51,36 @@ static void remove_watches_at_or_below(fswatch_t *w, const char *path) { entry->path[path_len] == '/')) { *pp = entry->next; close(entry->fd); - free(entry->path); - free(entry); + entry->fd = -1; + entry->next = w->retired; + w->retired = entry; } else { pp = &entry->next; } } } +static int watch_is_active(fswatch_t *w, const watch_entry_t *entry) { + for (watch_entry_t *e = w->watches; e; e = e->next) + if (e == entry) return 1; + return 0; +} + +static void free_watch_list(watch_entry_t *entry) { + while (entry) { + watch_entry_t *next = entry->next; + if (entry->fd >= 0) close(entry->fd); + free(entry->path); + free(entry); + entry = next; + } +} + +static void reap_retired_watches(fswatch_t *w) { + free_watch_list(w->retired); + w->retired = NULL; +} + static int watches_same_vnode(const watch_entry_t *entry, const char *path) { struct stat watched, current; return fstat(entry->fd, &watched) == 0 && lstat(path, ¤t) == 0 && @@ -173,6 +198,7 @@ int fswatch_add(fswatch_t *w, const char *path, int recursive) { if (lstat(path, &st) != 0) return -1; if (recursive && S_ISDIR(st.st_mode)) (void)watch_tree(w, path, 0); else if (add_watch(w, path, S_ISDIR(st.st_mode), 0) != 0) return -1; + else if (S_ISDIR(st.st_mode)) (void)watch_direct_files(w, path, 0); return find_watch(w, path) ? 0 : -1; } @@ -186,7 +212,7 @@ int fswatch_drain(fswatch_t *w) { while ((n = kevent(w->kqfd, NULL, 0, events, 32, &timeout)) > 0) { for (int i = 0; i < n; i++) { watch_entry_t *entry = events[i].udata; - if (!entry) continue; + if (!entry || !watch_is_active(w, entry)) continue; int flags = 0; if (events[i].fflags & (NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB)) flags |= FSW_MODIFY; @@ -195,7 +221,10 @@ int fswatch_drain(fswatch_t *w) { if (!flags) continue; if (flags & (FSW_DELETE | FSW_RENAME)) { char *path = strdup(entry->path); - if (!path) return -1; + if (!path) { + reap_retired_watches(w); + return -1; + } w->cb(path, flags, w->userdata); count++; remove_watches_at_or_below(w, path); @@ -211,19 +240,15 @@ int fswatch_drain(fswatch_t *w) { } } } - return n < 0 && errno != EAGAIN ? -1 : count; + int result = n < 0 && errno != EAGAIN ? -1 : count; + reap_retired_watches(w); + return result; } void fswatch_free(fswatch_t *w) { if (!w) return; - watch_entry_t *entry = w->watches; - while (entry) { - watch_entry_t *next = entry->next; - close(entry->fd); - free(entry->path); - free(entry); - entry = next; - } + free_watch_list(w->watches); + free_watch_list(w->retired); close(w->kqfd); free(w); } diff --git a/tests/test_fswatch.c b/tests/test_fswatch.c index 735c7def..ae79d6e9 100644 --- a/tests/test_fswatch.c +++ b/tests/test_fswatch.c @@ -383,6 +383,57 @@ static void test_nonrecursive_watch_stays_shallow(void) { rmrf(dir); } +static void test_nonrecursive_watch_existing_file(void) { + printf("\n--- test_nonrecursive_watch_existing_file ---\n"); + cb_state_t st = {0}; + fswatch_t *w = fswatch_init(test_cb, &st); + char *dir = make_tmpdir(); + char path[PATH_MAX]; + snprintf(path, sizeof(path), "%s/existing.txt", dir); + write_file(path, "before"); + + ASSERT(fswatch_add(w, dir, 0) == 0, + "add non-recursive watch with existing file"); + st.count = 0; + write_file(path, "after"); + int n = wait_and_drain(w, 1000); +#ifdef __APPLE__ + ASSERT(n > 0, "existing direct file is watched immediately"); + ASSERT(st.last_event & FSW_MODIFY, + "existing direct file reports modification, not creation"); +#else + (void)n; + ASSERT(1, "existing file behavior is tested by the kqueue backend"); +#endif + + fswatch_free(w); + rmrf(dir); +} + +static void test_recursive_tree_removal(void) { + printf("\n--- test_recursive_tree_removal ---\n"); + cb_state_t st = {0}; + fswatch_t *w = fswatch_init(test_cb, &st); + char *dir = make_tmpdir(); + char subdir[PATH_MAX], path[PATH_MAX]; + snprintf(subdir, sizeof(subdir), "%s/sub", dir); + snprintf(path, sizeof(path), "%s/sub/file.txt", dir); + mkdir(subdir, 0755); + write_file(path, "watched"); + + ASSERT(fswatch_add(w, dir, 1) == 0, "add recursive tree watch"); + rmrf(dir); + int n = wait_and_drain(w, 1000); +#ifdef __APPLE__ + ASSERT(n > 0, "recursive tree removal drains safely"); +#else + (void)n; + ASSERT(1, "recursive tree removal is tested by the kqueue backend"); +#endif + + fswatch_free(w); +} + int main(void) { printf("=== test_fswatch ===\n"); @@ -396,6 +447,8 @@ int main(void) { test_hidden_dirs_skipped(); test_recreate_watch(); test_nonrecursive_watch_stays_shallow(); + test_nonrecursive_watch_existing_file(); + test_recursive_tree_removal(); printf("\n=== Results: %d passed, %d failed ===\n", g_pass, g_fail); return g_fail > 0 ? 1 : 0;