From ed78ff056e89da814ff5ae05da6fb32b7e10c0db Mon Sep 17 00:00:00 2001 From: lorenzozanee Date: Sun, 20 Sep 2026 16:44:10 +0800 Subject: [PATCH 01/15] fix(install): reject Android portable binary Signed-off-by: lorenzozanee --- install.sh | 11 ++++++++ tests/test-install-android-guard.sh | 41 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100755 tests/test-install-android-guard.sh diff --git a/install.sh b/install.sh index 9f4b4ec99..01ab6c378 100755 --- a/install.sh +++ b/install.sh @@ -148,8 +148,19 @@ detect_arch() { esac } +is_android() { + [ "$(uname -o 2>/dev/null || true)" = "Android" ] \ + || [ -n "${TERMUX_VERSION:-}" ] \ + || [[ "${PREFIX:-}" == /data/data/com.termux/* ]] +} + OS=$(detect_os) ARCH=$(detect_arch) +if [ "$OS" = "linux" ] && is_android; then + echo "error: Android/Termux is not supported by the Linux portable binary." >&2 + echo "Build from source with scripts/build.sh in Termux or use a supported Linux host." >&2 + exit 1 +fi echo "codebase-memory-mcp installer" echo " os: $OS" diff --git a/tests/test-install-android-guard.sh b/tests/test-install-android-guard.sh new file mode 100755 index 000000000..6b3a383a2 --- /dev/null +++ b/tests/test-install-android-guard.sh @@ -0,0 +1,41 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT + +mkdir -p "$WORK/bin" "$WORK/home" +cat >"$WORK/bin/uname" <<'EOF' +case "$1" in + -s) printf '%s\n' Linux ;; + -m) printf '%s\n' aarch64 ;; + -o) printf '%s\n' Android ;; + *) exit 1 ;; +esac +EOF +cat >"$WORK/bin/curl" <<'EOF' +printf 'curl must not run for Android/Termux\n' >&2 +exit 99 +EOF +chmod +x "$WORK/bin/uname" "$WORK/bin/curl" + +set +e +output=$(PATH="$WORK/bin:$PATH" HOME="$WORK/home" bash "$ROOT/install.sh" --skip-config 2>&1) +rc=$? +set -e + +if [ "$rc" -eq 0 ]; then + printf 'FAIL: installer accepted Android/Termux\n' >&2 + exit 1 +fi +if ! grep -Fq 'Android/Termux' <<<"$output"; then + printf 'FAIL: missing Android/Termux guidance\n%s\n' "$output" >&2 + exit 1 +fi +if grep -Fq 'curl must not run' <<<"$output"; then + printf 'FAIL: installer downloaded before rejecting Android/Termux\n' >&2 + exit 1 +fi + +printf 'PASS: Android/Termux is rejected before download\n' From 42474dd7f062d924ab1944c15471cc13c89e96cd Mon Sep 17 00:00:00 2001 From: Tamer Date: Sun, 20 Sep 2026 20:47:20 +0200 Subject: [PATCH 02/15] docs: document contributor check prerequisites Signed-off-by: Tamer --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 96ce8f861..07a96ee97 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,7 +6,7 @@ Contributions are welcome. This guide covers setup, testing, and PR guidelines. ## Build from Source -**Prerequisites**: C compiler (gcc or clang), make, zlib, Git. Optional: Node.js 22+ (for graph UI). +**Build prerequisites**: C compiler (gcc or clang), make, zlib, Git. Optional: Node.js 22+ (for graph UI).\n\n**Contributor check prerequisites**: `scripts/test.sh` also requires `zip`, `python3`, and `curl`. `scripts/lint.sh` requires `cppcheck` and `clang-format`; full lint mode also requires `clang-tidy`. ```bash git clone https://github.com/DeusData/codebase-memory-mcp.git From 28d62001fd1e5d0a18085a8a662a1b806747815f Mon Sep 17 00:00:00 2001 From: Tamer Date: Sun, 20 Sep 2026 20:47:41 +0200 Subject: [PATCH 03/15] docs: clarify test and lint prerequisites Signed-off-by: Tamer --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 9d9e4ee09..9a74d8f4a 100644 --- a/README.md +++ b/README.md @@ -415,6 +415,8 @@ You: "Install this MCP server: https://github.com/DeusData/codebase-memory-mcp" +> **Contributor checks need a few extra tools.** `scripts/test.sh` requires `zip`, `python3`, and `curl`. `scripts/lint.sh` requires `cppcheck` and `clang-format`; full lint mode also requires `clang-tidy`. + ```bash git clone https://github.com/DeusData/codebase-memory-mcp.git cd codebase-memory-mcp From 3aec07ea3705700a1eba71321f1eb76c435a4d1d Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Mon, 14 Sep 2026 14:08:49 +0000 Subject: [PATCH 04/15] fix(daemon): stop the lifetime-lock probe from dropping a held lock posix_lifetime_lock_probe opened a throwaway fd on the lock file to confirm an already-claimed lock, then closed it. fcntl(2) record locks are scoped to (process, inode), so that close silently released the real lock too, even though the reservation's own fd stayed open. Check the in-process claim before opening anything and return early; that branch only ever needed to trust the registry. Signed-off-by: Amir Fathi --- src/daemon/ipc.c | 13 +++-- tests/test_daemon_ipc.c | 104 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index 3917a7a05..e3dbe0aff 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -1205,6 +1205,15 @@ static int posix_lifetime_lock_probe(const cbm_daemon_ipc_endpoint_t *endpoint, if (process_claimed < 0) { return -1; } + if (process_claimed == 1) { + /* Already held by this process per the in-process registry: trust + * that alone. Opening a throwaway fd on the lock file here and then + * closing it would release the real fcntl(2) record lock, which is + * scoped to (process, inode) rather than (fd, inode): closing any + * fd on this inode drops every lock this process holds on it, even + * one held via a different, still-open fd. */ + return endpoint_runtime_still_valid(endpoint) ? 1 : -1; + } int fd = openat(endpoint->dir_fd, lock_name, O_RDWR | O_CLOEXEC | O_NOFOLLOW); if (fd < 0) { @@ -1216,10 +1225,6 @@ static int posix_lifetime_lock_probe(const cbm_daemon_ipc_endpoint_t *endpoint, (void)close(fd); return -1; } - if (process_claimed == 1) { - bool still_private = endpoint_runtime_still_valid(endpoint); - return close(fd) == 0 && still_private ? 1 : -1; - } struct flock record_lock = { .l_type = F_WRLCK, .l_whence = SEEK_SET, diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index 815f630a6..ce034dd11 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -3365,6 +3365,109 @@ TEST(daemon_ipc_posix_lifetime_reservation_rejects_fork_inheritance) { PASS(); } +static bool ipc_test_lifetime_lock_path(char out[TEST_PATH_CAP], const char *runtime_dir, + const char *key) { + int written = runtime_dir && key + ? snprintf(out, TEST_PATH_CAP, "%s/cbm-%s.lifetime.lock", runtime_dir, key) + : -1; + return written > 0 && written < TEST_PATH_CAP; +} + +TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock) { + static const char key[] = "c1c1d2d2e3e3f4f4"; + char parent[TEST_PATH_CAP] = {0}; + char runtime_dir[TEST_PATH_CAP] = {0}; + char lock_path[TEST_PATH_CAP] = {0}; + cbm_daemon_ipc_endpoint_t *endpoint = NULL; + cbm_daemon_ipc_lifetime_reservation_t *reservation = NULL; + int result_pipe[2] = {-1, -1}; + pid_t child = -1; + uint8_t child_result = 0; + int child_status = -1; + int acquired = -1; + int held_after_first_probe = -1; + int held_after_second_probe = -1; + int free_after_release = -1; + bool lock_path_ok = false; + + if (ipc_test_parent_new(parent, "probe-holds-lock")) { + endpoint = cbm_daemon_ipc_endpoint_new(key, parent); + } + if (endpoint) { + ipc_test_copy_path(runtime_dir, cbm_daemon_ipc_endpoint_runtime_dir(endpoint)); + lock_path_ok = ipc_test_lifetime_lock_path(lock_path, runtime_dir, key); + acquired = cbm_daemon_ipc_lifetime_reservation_try_acquire(endpoint, &reservation); + } + if (reservation) { + /* Two probes, matching how a real daemon polls its own reservation + * repeatedly while running. Each probe of an already-held lock used + * to open a throwaway fd on the lock file and close it. Since + * fcntl(2) record locks are scoped to (process, inode), that close + * silently released the real lock every time, regardless of the + * still-open fd the reservation itself is holding. */ + held_after_first_probe = cbm_daemon_ipc_lifetime_reservation_probe(endpoint); + held_after_second_probe = cbm_daemon_ipc_lifetime_reservation_probe(endpoint); + } + if (reservation && lock_path_ok && pipe(result_pipe) == 0) { + child = fork(); + } + if (child == 0) { + /* A genuinely independent process, not going through any of this + * codebase's APIs so it cannot inherit the parent's in-process + * claim registry: try to take the real OS-level lock directly. If + * the parent's fcntl lock survived the probes above, this fails. */ + (void)close(result_pipe[0]); + int fd = open(lock_path, O_RDWR); + struct flock record_lock = { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 0, + }; + int lock_result = fd >= 0 ? fcntl(fd, F_SETLK, &record_lock) : -1; + child_result = lock_result == 0 ? 1 : 0; + if (fd >= 0) { + (void)close(fd); + } + bool reported = ipc_test_fd_write_all(result_pipe[1], &child_result, sizeof(child_result)); + (void)close(result_pipe[1]); + _exit(reported ? 0 : 1); + } + if (child > 0) { + (void)close(result_pipe[1]); + result_pipe[1] = -1; + bool received = ipc_test_fd_read_all(result_pipe[0], &child_result, sizeof(child_result)); + (void)close(result_pipe[0]); + result_pipe[0] = -1; + if (!received) { + child_result = 2; + } + while (waitpid(child, &child_status, 0) < 0 && errno == EINTR) {} + } + cbm_daemon_ipc_lifetime_reservation_release(reservation); + if (endpoint) { + free_after_release = cbm_daemon_ipc_lifetime_reservation_probe(endpoint); + } + for (size_t index = 0; index < 2; index++) { + if (result_pipe[index] >= 0) { + (void)close(result_pipe[index]); + } + } + cbm_daemon_ipc_endpoint_free(endpoint); + ipc_test_remove_tree(runtime_dir, parent); + + ASSERT_EQ(acquired, 1); + ASSERT_TRUE(lock_path_ok); + ASSERT_EQ(held_after_first_probe, 1); + ASSERT_EQ(held_after_second_probe, 1); + ASSERT_GT(child, 0); + ASSERT_TRUE(WIFEXITED(child_status)); + ASSERT_EQ(WEXITSTATUS(child_status), 0); + ASSERT_EQ(child_result, 0); + ASSERT_EQ(free_after_release, 0); + PASS(); +} + TEST(daemon_ipc_posix_child_participant_handoff_retains_legacy_bridge) { #ifdef _WIN32 PASS(); @@ -5524,6 +5627,7 @@ SUITE(daemon_ipc) { #endif RUN_TEST(daemon_ipc_posix_startup_lock_is_cross_process); RUN_TEST(daemon_ipc_posix_lifetime_reservation_rejects_fork_inheritance); + RUN_TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock); RUN_TEST(daemon_ipc_posix_child_participant_handoff_retains_legacy_bridge); RUN_TEST(daemon_ipc_posix_publication_boundaries_recover_from_crash); RUN_TEST(daemon_ipc_posix_record_publication_windows_recover_from_crash); From 8bd9d37b156641ecf1c82b16b0ec0a8246e8c201 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Sun, 27 Sep 2026 06:12:08 +0000 Subject: [PATCH 05/15] fix(daemon): stop the lifetime-lock probe from dropping a held lock posix_lifetime_lock_try_acquire opened a throwaway fd on the lock file to confirm an already-claimed lock, then closed it, in both the probe path and the try-acquire-again path. fcntl(2) record locks are scoped to (process, inode), so that close silently released the real lock too, even though the reservation's own fd stayed open. Check the in-process claim before opening anything and return early in both places; neither branch ever needed more than the registry. Also hardens the child-process verdict in the regression test: it now reports held only when open() succeeded and F_SETLK failed with EACCES or EAGAIN, so a wrong path or an unrelated errno can no longer read as a pass. Signed-off-by: Amir Fathi --- src/daemon/ipc.c | 19 ++++-- tests/test_daemon_ipc.c | 140 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 141 insertions(+), 18 deletions(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index e3dbe0aff..46ed2d514 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -1182,6 +1182,13 @@ static int posix_named_shared_lock_try_acquire(const cbm_daemon_ipc_endpoint_t * return posix_named_lock_try_acquire_mode(endpoint, lock_name, true, fd_out, process_entry_out); } +/* Never open and close the lifetime lock file in a process that may already + * hold it: fcntl(2) record locks are scoped to (process, inode), so that + * close would silently release every lock this process holds on the file, + * regardless of which fd took it. This lock is fcntl rather than flock for + * exactly this reason in reverse: flock locks survive a fork, fcntl locks + * do not, and a forked child must never appear to hold its parent's + * reservation. */ static int posix_record_lock_set(int fd, short lock_type) { struct flock record_lock = { .l_type = lock_type, @@ -1262,11 +1269,13 @@ static int posix_lifetime_lock_try_acquire(const cbm_daemon_ipc_endpoint_t *endp process_lock_entry_t *process_entry = NULL; int process_result = process_lock_claim(endpoint, lock_name, &process_entry); if (process_result != 1) { - return process_result == 0 && - private_regular_file_at_is_safe(endpoint->dir_fd, lock_name, 1) && - endpoint_runtime_still_valid(endpoint) - ? 0 - : -1; + /* Already held by this process per the in-process registry: trust + * that alone, the same as the probe's early return above. Calling + * private_regular_file_at_is_safe() here would open a throwaway fd + * on the lock file and close it, releasing the real fcntl(2) record + * lock this process holds, because that lock is scoped to (process, + * inode) rather than (fd, inode). */ + return process_result == 0 && endpoint_runtime_still_valid(endpoint) ? 0 : -1; } int fd = openat(endpoint->dir_fd, lock_name, O_RDWR | O_CREAT | O_CLOEXEC | O_NOFOLLOW, 0600); if (fd < 0 || !fd_set_cloexec(fd)) { diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index ce034dd11..b5e6c006c 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -3373,6 +3373,46 @@ static bool ipc_test_lifetime_lock_path(char out[TEST_PATH_CAP], const char *run return written > 0 && written < TEST_PATH_CAP; } +/* Child-process verdict on whether the lifetime lock at lock_path is held by + * someone else, going straight through open()/fcntl() rather than any of + * this codebase's APIs, so a genuinely separate process cannot inherit the + * parent's in-process claim registry. Distinguishes three outcomes instead + * of collapsing "held" and "could not even check" into the same value: + * IPC_TEST_LOCK_HELD only when open() succeeded and F_SETLK failed with + * EACCES or EAGAIN (the lock is held by another process, as expected); + * IPC_TEST_LOCK_TAKEN when F_SETLK succeeded (the lock was free, a bug); + * IPC_TEST_LOCK_ERROR for anything else (open() failed, e.g. a wrong path, + * or F_SETLK failed with an errno that says nothing about the lock's + * state), which must never be mistaken for IPC_TEST_LOCK_HELD. */ +enum { + IPC_TEST_LOCK_HELD = 0, + IPC_TEST_LOCK_TAKEN = 1, + IPC_TEST_LOCK_ERROR = 3, +}; + +static uint8_t ipc_test_lock_child_verdict(const char *lock_path) { + int fd = open(lock_path, O_RDWR); + if (fd < 0) { + return IPC_TEST_LOCK_ERROR; + } + struct flock record_lock = { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 0, + }; + int lock_result; + do { + lock_result = fcntl(fd, F_SETLK, &record_lock); + } while (lock_result != 0 && errno == EINTR); + int lock_errno = errno; + (void)close(fd); + if (lock_result == 0) { + return IPC_TEST_LOCK_TAKEN; + } + return lock_errno == EACCES || lock_errno == EAGAIN ? IPC_TEST_LOCK_HELD : IPC_TEST_LOCK_ERROR; +} + TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock) { static const char key[] = "c1c1d2d2e3e3f4f4"; char parent[TEST_PATH_CAP] = {0}; @@ -3417,18 +3457,7 @@ TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock) { * claim registry: try to take the real OS-level lock directly. If * the parent's fcntl lock survived the probes above, this fails. */ (void)close(result_pipe[0]); - int fd = open(lock_path, O_RDWR); - struct flock record_lock = { - .l_type = F_WRLCK, - .l_whence = SEEK_SET, - .l_start = 0, - .l_len = 0, - }; - int lock_result = fd >= 0 ? fcntl(fd, F_SETLK, &record_lock) : -1; - child_result = lock_result == 0 ? 1 : 0; - if (fd >= 0) { - (void)close(fd); - } + child_result = ipc_test_lock_child_verdict(lock_path); bool reported = ipc_test_fd_write_all(result_pipe[1], &child_result, sizeof(child_result)); (void)close(result_pipe[1]); _exit(reported ? 0 : 1); @@ -3463,7 +3492,91 @@ TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock) { ASSERT_GT(child, 0); ASSERT_TRUE(WIFEXITED(child_status)); ASSERT_EQ(WEXITSTATUS(child_status), 0); - ASSERT_EQ(child_result, 0); + ASSERT_EQ(child_result, IPC_TEST_LOCK_HELD); + ASSERT_EQ(free_after_release, 0); + PASS(); +} + +TEST(daemon_ipc_posix_lifetime_reservation_try_acquire_again_does_not_drop_held_lock) { + static const char key[] = "a1a1b2b2c3c3d4d4"; + char parent[TEST_PATH_CAP] = {0}; + char runtime_dir[TEST_PATH_CAP] = {0}; + char lock_path[TEST_PATH_CAP] = {0}; + cbm_daemon_ipc_endpoint_t *endpoint = NULL; + cbm_daemon_ipc_lifetime_reservation_t *reservation = NULL; + cbm_daemon_ipc_lifetime_reservation_t *second_reservation = + (cbm_daemon_ipc_lifetime_reservation_t *)(uintptr_t)1; + int result_pipe[2] = {-1, -1}; + pid_t child = -1; + uint8_t child_result = 0; + int child_status = -1; + int acquired = -1; + int reacquired = -1; + int free_after_release = -1; + bool lock_path_ok = false; + + if (ipc_test_parent_new(parent, "try-acquire-again-holds-lock")) { + endpoint = cbm_daemon_ipc_endpoint_new(key, parent); + } + if (endpoint) { + ipc_test_copy_path(runtime_dir, cbm_daemon_ipc_endpoint_runtime_dir(endpoint)); + lock_path_ok = ipc_test_lifetime_lock_path(lock_path, runtime_dir, key); + acquired = cbm_daemon_ipc_lifetime_reservation_try_acquire(endpoint, &reservation); + } + if (reservation) { + /* This process already holds the lifetime lock. Calling try_acquire + * again, still in this process, must trust the in-process registry + * alone and return 0 (already held, nothing new to hand back) + * without touching the lock file: opening and closing a second fd + * on it would drop the fcntl(2) record lock this process already + * holds, exactly the way the probe's own fast path used to. */ + reacquired = cbm_daemon_ipc_lifetime_reservation_try_acquire(endpoint, &second_reservation); + } + if (reservation && lock_path_ok && pipe(result_pipe) == 0) { + child = fork(); + } + if (child == 0) { + /* A genuinely independent process, not going through any of this + * codebase's APIs so it cannot inherit the parent's in-process + * claim registry: try to take the real OS-level lock directly. If + * the repeated try_acquire above survived, this fails. */ + (void)close(result_pipe[0]); + child_result = ipc_test_lock_child_verdict(lock_path); + bool reported = ipc_test_fd_write_all(result_pipe[1], &child_result, sizeof(child_result)); + (void)close(result_pipe[1]); + _exit(reported ? 0 : 1); + } + if (child > 0) { + (void)close(result_pipe[1]); + result_pipe[1] = -1; + bool received = ipc_test_fd_read_all(result_pipe[0], &child_result, sizeof(child_result)); + (void)close(result_pipe[0]); + result_pipe[0] = -1; + if (!received) { + child_result = 2; + } + while (waitpid(child, &child_status, 0) < 0 && errno == EINTR) {} + } + cbm_daemon_ipc_lifetime_reservation_release(reservation); + if (endpoint) { + free_after_release = cbm_daemon_ipc_lifetime_reservation_probe(endpoint); + } + for (size_t index = 0; index < 2; index++) { + if (result_pipe[index] >= 0) { + (void)close(result_pipe[index]); + } + } + cbm_daemon_ipc_endpoint_free(endpoint); + ipc_test_remove_tree(runtime_dir, parent); + + ASSERT_EQ(acquired, 1); + ASSERT_TRUE(lock_path_ok); + ASSERT_EQ(reacquired, 0); + ASSERT_TRUE(second_reservation == NULL); + ASSERT_GT(child, 0); + ASSERT_TRUE(WIFEXITED(child_status)); + ASSERT_EQ(WEXITSTATUS(child_status), 0); + ASSERT_EQ(child_result, IPC_TEST_LOCK_HELD); ASSERT_EQ(free_after_release, 0); PASS(); } @@ -5628,6 +5741,7 @@ SUITE(daemon_ipc) { RUN_TEST(daemon_ipc_posix_startup_lock_is_cross_process); RUN_TEST(daemon_ipc_posix_lifetime_reservation_rejects_fork_inheritance); RUN_TEST(daemon_ipc_posix_lifetime_reservation_probe_does_not_drop_held_lock); + RUN_TEST(daemon_ipc_posix_lifetime_reservation_try_acquire_again_does_not_drop_held_lock); RUN_TEST(daemon_ipc_posix_child_participant_handoff_retains_legacy_bridge); RUN_TEST(daemon_ipc_posix_publication_boundaries_recover_from_crash); RUN_TEST(daemon_ipc_posix_record_publication_windows_recover_from_crash); From 1c840df7495e3b71924b8efd7e3bbf53c9076abc Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Mon, 28 Sep 2026 15:05:48 +0000 Subject: [PATCH 06/15] ci: retrigger after macOS smoke runner cancellation pr-smoke (macos-14) was cancelled mid-run (operation was canceled) at an unrelated UI-server check, with all 30+ other legs green; ci-ok failed only as a consequence. Empty commit to get a fresh run. Signed-off-by: Amir Fathi From 2a7da82a044c8ac49d09d9192775b25ba00d9f7b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 19:31:31 +0200 Subject: [PATCH 07/15] docs: fix contributor prerequisites paragraph break and lint wording Replace the literal "\n\n" in CONTRIBUTING.md with a real paragraph break; Markdown rendered it as text in the middle of the sentence. A plain scripts/lint.sh run is the full mode and needs clang-tidy as well as cppcheck and clang-format; scripts/lint.sh --ci, which is what CI runs, skips clang-tidy. Say it that way round in README.md and CONTRIBUTING.md. The curl entry stays: since 6c00f9631152 (#2480) Step 0y3 of scripts/test.sh runs the setup-script contract, which downloads a local fixture release through scripts/setup.sh with curl. Signed-off-by: Martin Vogel --- CONTRIBUTING.md | 4 +++- README.md | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 07a96ee97..a67bd460e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,7 +6,9 @@ Contributions are welcome. This guide covers setup, testing, and PR guidelines. ## Build from Source -**Build prerequisites**: C compiler (gcc or clang), make, zlib, Git. Optional: Node.js 22+ (for graph UI).\n\n**Contributor check prerequisites**: `scripts/test.sh` also requires `zip`, `python3`, and `curl`. `scripts/lint.sh` requires `cppcheck` and `clang-format`; full lint mode also requires `clang-tidy`. +**Build prerequisites**: C compiler (gcc or clang), make, zlib, Git. Optional: Node.js 22+ (for graph UI). + +**Contributor check prerequisites**: `scripts/test.sh` also requires `zip`, `python3`, and `curl`. `scripts/lint.sh` requires `cppcheck`, `clang-format`, and `clang-tidy`; `scripts/lint.sh --ci`, which is what CI runs, skips `clang-tidy`. ```bash git clone https://github.com/DeusData/codebase-memory-mcp.git diff --git a/README.md b/README.md index 9a74d8f4a..f5a71b0a6 100644 --- a/README.md +++ b/README.md @@ -415,7 +415,7 @@ You: "Install this MCP server: https://github.com/DeusData/codebase-memory-mcp" -> **Contributor checks need a few extra tools.** `scripts/test.sh` requires `zip`, `python3`, and `curl`. `scripts/lint.sh` requires `cppcheck` and `clang-format`; full lint mode also requires `clang-tidy`. +> **Contributor checks need a few extra tools.** `scripts/test.sh` requires `zip`, `python3`, and `curl`. `scripts/lint.sh` requires `cppcheck`, `clang-format`, and `clang-tidy`; `scripts/lint.sh --ci`, which is what CI runs, skips `clang-tidy`. ```bash git clone https://github.com/DeusData/codebase-memory-mcp.git From fa331ed5e6b4294a0ed9bbd6b6f2bb6c98302d85 Mon Sep 17 00:00:00 2001 From: Petteri Ponsimaa Date: Wed, 21 Oct 2026 10:00:23 +0300 Subject: [PATCH 08/15] feat(extraction): route Next.js App Router handlers app/**/route.{ts,js} files produced 0 Route nodes, so cross-repo intelligence found no edges into a Next.js backend (#759). cbm_extract_nextjs_routes fills route_path/route_method on the existing Function definition for each exported HTTP-verb handler. It adds no pass and no synthetic definitions; the route-node pass builds the Route and HANDLES edge as it does for other frameworks. CBM_SEMANTIC_INDEX_VERSION goes from 9 to 10 so existing indexes re-extract once. Fixes #759 Signed-off-by: Petteri Ponsimaa --- Makefile.cbm | 1 + internal/cbm/cbm.c | 3 + internal/cbm/cbm.h | 5 + internal/cbm/extract_nextjs.c | 272 ++++++++++++++++++++ src/pipeline/pipeline_internal.h | 9 +- tests/test_extraction.c | 173 +++++++++++++ tests/test_pipeline.c | 422 +++++++++++++++++++++++++++++++ 7 files changed, 883 insertions(+), 2 deletions(-) create mode 100644 internal/cbm/extract_nextjs.c diff --git a/Makefile.cbm b/Makefile.cbm index 3b93146f1..631290d69 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -397,6 +397,7 @@ EXTRACTION_SRCS = \ $(CBM_DIR)/diagram.c \ $(CBM_DIR)/route_path.c \ $(CBM_DIR)/openapi.c \ + $(CBM_DIR)/extract_nextjs.c \ $(CBM_DIR)/sql_values.c \ $(CBM_DIR)/helpers.c \ $(CBM_DIR)/cpp_branch_views.c \ diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index 23b871dbc..50dd97568 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -3902,6 +3902,9 @@ static CBMFileResult *extract_file_ex_body(const char *source, int source_len, C // OpenAPI operations as Routes (self-gated: API description documents). cbm_extract_openapi(&ctx); + // Next.js App Router handlers' routes (self-gated: app/**/route.ts and route.js). + cbm_extract_nextjs_routes(&ctx); + // K8s / Kustomize semantic pass (additional structured extraction for YAML-based infra files). if (ctx.language == CBM_LANG_KUSTOMIZE || ctx.language == CBM_LANG_K8S) { cbm_extract_k8s(&ctx); diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index ade41fea5..12b8c1acd 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -1389,6 +1389,11 @@ void cbm_extract_diagram_file(CBMExtractCtx *ctx); * or JSON files whose root says `openapi` or `swagger`. */ void cbm_extract_openapi(CBMExtractCtx *ctx); +/* Next.js App Router route handlers (extract_nextjs.c): the HTTP verb Functions + * a route file exports carry the route path its directories name. Self-gated: + * TS / JS route.ts and route.js under app/ or src/app/. */ +void cbm_extract_nextjs_routes(CBMExtractCtx *ctx); + /* Canonicalize route-path parameter placeholders (":id", "{id}", "", * "${...}") to a single "{}" token so that client call sites, server handlers * and OpenAPI operations rendezvous on the same Route QN regardless of diff --git a/internal/cbm/extract_nextjs.c b/internal/cbm/extract_nextjs.c new file mode 100644 index 000000000..eea270fa7 --- /dev/null +++ b/internal/cbm/extract_nextjs.c @@ -0,0 +1,272 @@ +/* + * extract_nextjs.c -- Next.js App Router route handlers (#759): an HTTP verb a + * route file (`app/.../route.ts` or `route.js`, the app directory at the root + * or under src/) exports serves the path its directories name. + * + * The handler is the Function definition extraction already made for the + * export: this fills its route_path / route_method, so its Route node and + * HANDLES edge come from the route pass exactly as a decorator route's do, on + * a full and on an incremental index alike. The path is spelled as SvelteKit's + * (sveltekit_route_path, pass_route_nodes.c): `[id]` is `:id`, `[...slug]` is + * `*slug` and a `(group)` directory adds no segment. + * + * A directory this cannot spell makes the file no route at all: an optional + * catch-all (`[[...slug]]`), a private folder (`_lib`), a parallel slot + * (`@modal`), an intercepting route (`(.)photo`), a malformed bracket, or a + * path that does not fit route_path. A handler is a Function of this file: + * `export [async] function GET`, `export const GET = [async] (...) => ...`, + * or `export { GET }` of such a declaration. A renamed + * (`export { handler as GET }`), re-exported, destructured or wrapped + * (`export const GET = auth(handler)`) handler, or a `function` expression + * bound to a const, has no Function of its own here and is not routed. Each + * file is judged alone: a src/app/ route file is routed even where a root + * app/ directory makes Next.js ignore src/app/. + */ +#include "cbm.h" +#include "foundation/arena.h" +#include "helpers.h" +#include "tree_sitter/api.h" +#include "foundation/constants.h" +#include +#include +#include + +enum { + NX_PATH_MAX = CBM_SZ_256, /* the route pass rebuilds a Route from this much */ + NX_REST_LEN = 3, /* "..." */ +}; + +/* The path of a route file below its app directory, NULL for any other file. */ +static const char *nx_app_relative(const char *rel_path) { + const char *base = rel_path ? strrchr(rel_path, '/') : NULL; + if (!base || (strcmp(base, "/route.ts") != 0 && strcmp(base, "/route.js") != 0)) { + return NULL; + } + if (strncmp(rel_path, "app/", strlen("app/")) == 0) { + return rel_path + strlen("app/"); + } + if (strncmp(rel_path, "src/app/", strlen("src/app/")) == 0) { + return rel_path + strlen("src/app/"); + } + return NULL; +} + +static bool nx_has_any(const char *s, size_t n, const char *chars) { + for (size_t i = 0; i < n; i++) { + if (strchr(chars, s[i])) { + return true; + } + } + return false; +} + +/* Append directory segment seg (n bytes) to the route path out[*len]. False + * when the segment cannot be spelled or the path would not fit. */ +static bool nx_append_segment(char *out, size_t *len, const char *seg, size_t n) { + if (n == 0 || seg[0] == '_' || seg[0] == '@') { + return false; + } + if (seg[0] == '(') { + /* A route group adds nothing; `(.)x`, `(..)x` and `(...)x` intercept. */ + return n > PAIR_LEN && seg[n - SKIP_ONE] == ')' && seg[SKIP_ONE] != '.' && + !nx_has_any(seg + SKIP_ONE, n - PAIR_LEN, "()"); + } + char marker = '\0'; + if (seg[0] == '[') { + if (seg[n - SKIP_ONE] != ']') { + return false; + } + marker = ':'; + seg += SKIP_ONE; + n -= PAIR_LEN; + if (n >= NX_REST_LEN && strncmp(seg, "...", NX_REST_LEN) == 0) { + marker = '*'; + seg += NX_REST_LEN; + n -= NX_REST_LEN; + } + if (n == 0 || nx_has_any(seg, n, "[].")) { + return false; + } + } else if (nx_has_any(seg, n, "[]")) { + return false; + } + size_t need = SKIP_ONE + (marker ? SKIP_ONE : 0) + n; + if (*len + need >= NX_PATH_MAX) { + return false; + } + out[(*len)++] = '/'; + if (marker) { + out[(*len)++] = marker; + } + memcpy(out + *len, seg, n); + *len += n; + out[*len] = '\0'; + return true; +} + +/* The route path of the file being extracted, in the arena; NULL for none. */ +static const char *nx_route_path(CBMExtractCtx *ctx) { + const char *seg = nx_app_relative(ctx->rel_path); + if (!seg) { + return NULL; + } + const char *base = strrchr(ctx->rel_path, '/'); + char path[NX_PATH_MAX] = "/"; + size_t len = 0; + while (seg < base) { + const char *end = strchr(seg, '/'); + if (!nx_append_segment(path, &len, seg, (size_t)(end - seg))) { + return NULL; + } + seg = end + SKIP_ONE; + } + return cbm_arena_strdup(ctx->arena, path); +} + +static bool nx_is_method(const char *name) { + static const char *const methods[] = {"GET", "POST", "PUT", "PATCH", + "DELETE", "HEAD", "OPTIONS"}; + for (size_t i = 0; i < sizeof(methods) / sizeof(methods[0]); i++) { + if (strcmp(name, methods[i]) == 0) { + return true; + } + } + return false; +} + +static bool nx_is(TSNode n, const char *type) { + return !ts_node_is_null(n) && strcmp(ts_node_type(n), type) == 0; +} + +/* Does a node carry the anonymous keyword child kw (`default`, `type`)? */ +static bool nx_has_keyword(TSNode n, const char *kw) { + uint32_t count = ts_node_child_count(n); + for (uint32_t i = 0; i < count; i++) { + TSNode c = ts_node_child(n, i); + if (!ts_node_is_named(c) && strcmp(ts_node_type(c), kw) == 0) { + return true; + } + } + return false; +} + +/* The name of a declarator whose value is an arrow function, the Function + * extraction names after the binding (`GET = () => ...`); a null node for any + * other declarator. */ +static TSNode nx_arrow_binding(TSNode declarator) { + TSNode value = ts_node_child_by_field_name(declarator, TS_FIELD("value")); + TSNode name = ts_node_child_by_field_name(declarator, TS_FIELD("name")); + if (!nx_is(name, "identifier") || !nx_is(value, "arrow_function")) { + return (TSNode){0}; + } + return name; +} + +/* Route the Function definition named by name_node, declared inside decl. */ +static void nx_route_handler(CBMExtractCtx *ctx, TSNode decl, TSNode name_node, const char *path) { + const char *name = cbm_node_text(ctx->arena, name_node, ctx->source); + if (!name || !nx_is_method(name)) { + return; + } + const char *qn = + cbm_fqn_compute_source_lang(ctx->arena, ctx->project, ctx->rel_path, name, ctx->language); + uint32_t first = ts_node_start_point(decl).row + TS_LINE_OFFSET; + uint32_t last = ts_node_end_point(decl).row + TS_LINE_OFFSET; + for (int i = 0; qn && i < ctx->result->defs.count; i++) { + CBMDefinition *def = &ctx->result->defs.items[i]; + if (!def->route_path && def->label && def->qualified_name && + strcmp(def->label, "Function") == 0 && strcmp(def->qualified_name, qn) == 0 && + def->start_line >= first && def->end_line <= last) { + def->route_path = path; + def->route_method = name; + return; + } + } +} + +/* Does the name node, if any, spell want? Any name does when want is NULL. */ +static bool nx_wanted(CBMExtractCtx *ctx, TSNode name, const char *want) { + if (ts_node_is_null(name)) { + return false; + } + const char *text = want ? cbm_node_text(ctx->arena, name, ctx->source) : NULL; + return !want || (text && strcmp(text, want) == 0); +} + +/* Route the handlers a top-level declaration (function or const) defines. + * Only names equal to want are considered when want is set. */ +static void nx_route_declaration(CBMExtractCtx *ctx, TSNode decl, const char *path, + const char *want) { + if (nx_is(decl, "function_declaration")) { + TSNode name = ts_node_child_by_field_name(decl, TS_FIELD("name")); + if (nx_wanted(ctx, name, want)) { + nx_route_handler(ctx, decl, name, path); + } + return; + } + if (!nx_is(decl, "lexical_declaration") && !nx_is(decl, "variable_declaration")) { + return; + } + uint32_t count = ts_node_named_child_count(decl); + for (uint32_t i = 0; i < count; i++) { + TSNode declarator = ts_node_named_child(decl, i); + TSNode name = + nx_is(declarator, "variable_declarator") ? nx_arrow_binding(declarator) : (TSNode){0}; + if (nx_wanted(ctx, name, want)) { + nx_route_handler(ctx, declarator, name, path); + } + } +} + +/* `export { GET }`: route the file's own top-level declaration of each + * unrenamed, non-type specifier. */ +static void nx_route_export_clause(CBMExtractCtx *ctx, TSNode clause, const char *path) { + uint32_t count = ts_node_named_child_count(clause); + uint32_t top = ts_node_named_child_count(ctx->root); + for (uint32_t i = 0; i < count; i++) { + TSNode spec = ts_node_named_child(clause, i); + TSNode name = ts_node_child_by_field_name(spec, TS_FIELD("name")); + if (!nx_is(spec, "export_specifier") || !nx_is(name, "identifier") || + !ts_node_is_null(ts_node_child_by_field_name(spec, TS_FIELD("alias"))) || + nx_has_keyword(spec, "type") || nx_has_keyword(spec, "typeof")) { + continue; + } + const char *want = cbm_node_text(ctx->arena, name, ctx->source); + for (uint32_t j = 0; want && j < top; j++) { + nx_route_declaration(ctx, ts_node_named_child(ctx->root, j), path, want); + } + } +} + +void cbm_extract_nextjs_routes(CBMExtractCtx *ctx) { + if (ctx->language != CBM_LANG_TYPESCRIPT && ctx->language != CBM_LANG_JAVASCRIPT) { + return; + } + const char *path = nx_route_path(ctx); + if (!path) { + return; + } + uint32_t count = ts_node_named_child_count(ctx->root); + for (uint32_t i = 0; i < count; i++) { + TSNode node = ts_node_named_child(ctx->root, i); + /* A runtime export of this file: not `export default`, not + * `export type { ... }`, not a re-export `... from './x'`. */ + if (!nx_is(node, "export_statement") || nx_has_keyword(node, "default") || + nx_has_keyword(node, "type") || + !ts_node_is_null(ts_node_child_by_field_name(node, TS_FIELD("source")))) { + continue; + } + TSNode decl = ts_node_child_by_field_name(node, TS_FIELD("declaration")); + if (!ts_node_is_null(decl)) { + nx_route_declaration(ctx, decl, path, NULL); + continue; + } + uint32_t parts = ts_node_named_child_count(node); + for (uint32_t j = 0; j < parts; j++) { + TSNode part = ts_node_named_child(node, j); + if (nx_is(part, "export_clause")) { + nx_route_export_clause(ctx, part, path); + } + } + } +} diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index fb5b724d6..bf72989a0 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -1645,8 +1645,13 @@ int cbm_pipeline_build_fresh_semantic_manifest(cbm_pipeline_t *p, const char *pr * a kube / k8s task DEPLOYS {via:ansible} the template it applies * (pass_ansible.c). An external Chart is keyed by its publisher URL. * An index built before holds none of them, so it rebuilds once on - * upgrade. */ -enum { CBM_SEMANTIC_INDEX_VERSION = 9 }; + * upgrade. + * 10: a Next.js App Router route handler, the HTTP verb Function a route.ts + * / route.js under app/ or src/app/ exports, carries the route_path its + * directories name and HANDLES its Route (extract_nextjs.c). An index + * built before has none of these Routes for its unchanged route files, + * so it rebuilds once on upgrade. */ +enum { CBM_SEMANTIC_INDEX_VERSION = 10 }; typedef struct { cbm_gbuf_t *gbuf; diff --git a/tests/test_extraction.c b/tests/test_extraction.c index e1c1e4491..15f03463a 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -5622,6 +5622,177 @@ TEST(extract_ts_nestjs_controller_routes_issue1428) { PASS(); } +/* #759: the one definition named `name`; NULL for none or more than one. */ +static const CBMDefinition *nextjs_only_def(CBMFileResult *r, const char *name) { + const CBMDefinition *found = NULL; + for (int i = 0; i < r->defs.count; i++) { + if (r->defs.items[i].name && strcmp(r->defs.items[i].name, name) == 0) { + if (found) { + return NULL; + } + found = &r->defs.items[i]; + } + } + return found; +} + +/* #759: a Next.js App Router route file's exported HTTP verb handlers are the + * Functions extraction already makes for them, now carrying the route path of + * the file's directories -- `[id]` as `:id`, `[...slug]` as `*slug`, a + * `(group)` adding nothing -- and the verb as their method. A local + * declaration exported by name (`export { GET }`) is that same Function; no + * second definition appears. The 255-byte path is the longest that fits. */ +TEST(extract_nextjs_route_metadata_contract_issue759) { + char long_file[CBM_SZ_512]; + char long_path[CBM_SZ_256]; + char segment[CBM_SZ_256]; + memset(segment, 'x', CBM_SZ_256 - PAIR_LEN); + segment[CBM_SZ_256 - PAIR_LEN] = '\0'; + snprintf(long_file, sizeof(long_file), "app/%s/route.ts", segment); + snprintf(long_path, sizeof(long_path), "/%s", segment); + static const char verbs_src[] = "export async function GET() {}\n" + "export async function POST() {}\n" + "export function PUT() {}\n" + "export function PATCH() {}\n" + "export function DELETE() {}\n" + "export function HEAD() {}\n" + "export function OPTIONS() {}\n"; + const struct { + const char *file; + const char *src; + const char *path; + const char *names[CBM_SZ_8]; + } cases[] = { + {"src/app/api/users/[id]/route.ts", + "export function GET() { return 1; }\nexport const POST = async () => 2;\n", + "/api/users/:id", + {"GET", "POST"}}, + {"app/api/verbs/route.ts", + verbs_src, + "/api/verbs", + {"GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS"}}, + {"src/app/(public)/api/[org]/users/[id]/route.ts", + "export async function GET() { return 1; }\n", + "/api/:org/users/:id", + {"GET"}}, + {"app/api/[...slug]/route.ts", "export function GET() {}\n", "/api/*slug", {"GET"}}, + {"app/api/typed/route.ts", + "type Handler = () => Promise;\nexport const GET: Handler = async () => 1;\n", + "/api/typed", + {"GET"}}, + {"app/route.js", "export function GET() { return 1; }\n", "/", {"GET"}}, + {"app/api/local/route.ts", + "function GET() { return 1; }\nconst POST = async () => 2;\nexport { GET, POST };\n", + "/api/local", + {"GET", "POST"}}, + {long_file, "export function GET() {}\n", long_path, {"GET"}}, + }; + for (size_t c = 0; c < sizeof(cases) / sizeof(cases[0]); c++) { + CBMLanguage lang = strstr(cases[c].file, ".js") ? CBM_LANG_JAVASCRIPT : CBM_LANG_TYPESCRIPT; + CBMFileResult *r = extract(cases[c].src, lang, "t", cases[c].file); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + int routed = 0; + for (int i = 0; i < r->defs.count; i++) { + routed += r->defs.items[i].route_path != NULL; + } + int want = 0; + for (; want < CBM_SZ_8 && cases[c].names[want]; want++) { + const CBMDefinition *d = nextjs_only_def(r, cases[c].names[want]); + ASSERT_NOT_NULL(d); + ASSERT_STR_EQ(d->label, "Function"); + ASSERT_STR_EQ(d->file_path, cases[c].file); + ASSERT_NOT_NULL(d->route_path); + ASSERT_STR_EQ(d->route_path, cases[c].path); + ASSERT_STR_EQ(d->route_method, cases[c].names[want]); + } + ASSERT_EQ(routed, want); + cbm_free_result(r); + } + PASS(); +} + +/* #759: nothing else routes. Not a route file, or a directory the route path + * cannot spell -- an optional catch-all, a private folder, a parallel slot, + * an intercepting route, a malformed bracket, a workspace package root, a path + * past 255 bytes. Not a runtime export of an HTTP verb Function of this file: + * an unexported, default, type-only or nested GET, a class method, a + * constant, a lower-case verb; and no handler is made up for a renamed, + * re-exported, wrapped or destructured export, or a `function` expression + * bound to a const: none of them has a Function of its own here. */ +TEST(extract_nextjs_rejects_nonroutes_issue759) { + char long_file[CBM_SZ_512]; + char segment[CBM_SZ_256]; + memset(segment, 'x', CBM_SZ_256 - SKIP_ONE); + segment[CBM_SZ_256 - SKIP_ONE] = '\0'; + snprintf(long_file, sizeof(long_file), "app/%s/route.ts", segment); + const char *files[] = { + "pages/api/route.ts", + "app/api/page.ts", + "app/api/route.test.ts", + "app/api/view/route.tsx", + "app/api/view/route.jsx", + "app/api/module/route.mjs", + "app/_private/route.ts", + "src/app/api/_lib/route.ts", + "app/@modal/route.ts", + "app/(.)photo/route.ts", + "app/(..)(..)photo/route.ts", + "app/api/[[...opt]]/route.ts", + "app/api/[broken/route.ts", + "app/api/broken]/route.ts", + "app/api/[]/route.ts", + "app/api/[...]/route.ts", + "app/api/a[id]/route.ts", + "app/()/route.ts", + "apps/web/app/api/route.ts", + long_file, + }; + const struct { + const char *file; + const char *src; + } sources[] = { + {"app/api/nonexports/route.ts", "function GET() {}\n" + "export default function POST() {}\n" + "function PUT() {}\n" + "export type { PUT };\n" + "function OPTIONS() {}\n" + "export { type OPTIONS };\n" + "export class Handlers { PATCH() {} }\n" + "export const DELETE = 42;\n" + "export function get() {}\n" + "export function helper() { function HEAD() {} }\n"}, + {"app/api/bindings/route.ts", "const handler = async () => 1;\n" + "export { handler as GET };\n" + "export { POST } from './impl';\n" + "export const PUT = wrap(handler);\n" + "export const DELETE = function () {};\n" + "export const { PATCH } = handlers;\n" + "export const HEAD = handler as Handler;\n" + "export const OPTIONS = (async () => 1) satisfies Handler;\n" + "export * from './all';\n"}, + }; + const int file_count = (int)(sizeof(files) / sizeof(files[0])); + const int source_count = (int)(sizeof(sources) / sizeof(sources[0])); + for (int c = 0; c < file_count + source_count; c++) { + bool by_file = c < file_count; + const char *file = by_file ? files[c] : sources[c - file_count].file; + CBMFileResult *r = + extract(by_file ? "export function GET() {}\n" : sources[c - file_count].src, + CBM_LANG_TYPESCRIPT, "t", file); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + if (by_file) { + ASSERT_NOT_NULL(nextjs_only_def(r, "GET")); + } + for (int i = 0; i < r->defs.count; i++) { + ASSERT_NULL(r->defs.items[i].route_path); + } + cbm_free_result(r); + } + PASS(); +} + /* Return the file's Module definition (extraction pushes it first), or NULL. */ static const CBMDefinition *find_module_def(CBMFileResult *r) { for (int i = 0; i < r->defs.count; i++) { @@ -13434,6 +13605,8 @@ SUITE(extraction) { RUN_TEST(extract_java_jaxrs_empty_path_means_class_path); RUN_TEST(extract_java_spring_relative_string_not_route_path); RUN_TEST(extract_ts_nestjs_controller_routes_issue1428); + RUN_TEST(extract_nextjs_route_metadata_contract_issue759); + RUN_TEST(extract_nextjs_rejects_nonroutes_issue759); RUN_TEST(extract_blazor_page_directive_routes_component); RUN_TEST(extract_blazor_component_without_page_has_no_route); RUN_TEST(extract_razor_page_directive_routes_cshtml_view); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index bd76775f3..060e87022 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -18874,6 +18874,423 @@ TEST(pipeline_semantic_version_3_index_is_rebuilt_in_full_c1) { PASS(); } +/* #759: index a Next.js App Router tree as project "nextjs" (a full index, or + * the incremental route a stored index of the same tree takes). */ +static int nextjs_index(const char *repo, const char *db_path) { + cbm_pipeline_t *p = cbm_pipeline_new(repo, db_path, CBM_MODE_FULL); + if (!p) { + return -1; + } + (void)cbm_pipeline_set_project_name(p, "nextjs"); + int rc = cbm_pipeline_run(p); + cbm_pipeline_free(p); + return rc; +} + +/* Every Route of a stored index with each handler that HANDLES it, one sorted + * line each: "METHOD path route_qn file_path handler_qn". A Route no handler + * reaches ends in "-". The properties are parsed (json_extract): a Route not + * from route metadata is marked "!source", a handler declared in another file + * "!file", a HANDLES edge whose handler property is not its handler's QN + * "!handler"; malformed JSON fails the query. */ +static bool nextjs_route_tuples(const char *db_path, char *out, size_t sz) { + sqlite3 *db = NULL; + sqlite3_stmt *st = NULL; + bool ok = false; + if (sqlite3_open_v2(db_path, &db, SQLITE_OPEN_READONLY, NULL) == SQLITE_OK && + sqlite3_prepare_v2( + db, + "SELECT coalesce(group_concat(t, char(10)), '') FROM (SELECT " + "json_extract(r.properties,'$.method') || ' ' || r.name || ' ' || r.qualified_name || " + "' ' || r.file_path || ' ' || coalesce(h.qualified_name, '-') || CASE WHEN " + "json_extract(r.properties,'$.source') = 'decorator' THEN '' ELSE ' !source' END || " + "CASE WHEN h.id IS NULL OR h.file_path = r.file_path THEN '' ELSE ' !file' END || " + "CASE WHEN e.id IS NULL OR json_extract(e.properties,'$.handler') = h.qualified_name " + "THEN '' ELSE ' !handler' END AS t FROM nodes r LEFT JOIN edges e ON " + "e.target_id=r.id AND e.type='HANDLES' LEFT JOIN nodes h ON h.id=e.source_id WHERE " + "r.label='Route' ORDER BY t)", + -1, &st, NULL) == SQLITE_OK && + sqlite3_step(st) == SQLITE_ROW) { + snprintf(out, sz, "%s", (const char *)sqlite3_column_text(st, 0)); + ok = true; + } + sqlite3_finalize(st); + sqlite3_close(db); + return ok; +} + +static int nextjs_by_qn(const void *a, const void *b) { + return strcmp(((const cbm_search_result_t *)a)->node.qualified_name, + ((const cbm_search_result_t *)b)->node.qualified_name); +} + +/* The Routes a search by label Route and declaring-file glob finds, one + * "qualified_name file_path" line each in QN order. */ +static bool nextjs_found_routes(const char *db_path, const char *glob, char *out, size_t sz) { + cbm_store_t *s = cbm_store_open_path_existing(db_path); + if (!s) { + return false; + } + cbm_search_params_t params = {.project = "nextjs", + .label = "Route", + .file_pattern = glob, + .min_degree = -1, + .max_degree = -1, + .limit = 100}; + cbm_search_output_t res = {0}; + bool ok = cbm_store_search(s, ¶ms, &res) == CBM_STORE_OK; + if (ok && res.count > 1) { + qsort(res.results, (size_t)res.count, sizeof(res.results[0]), nextjs_by_qn); + } + size_t len = 0; + out[0] = '\0'; + for (int i = 0; ok && i < res.count; i++) { + int n = snprintf(out + len, sz - len, "%s%s %s", i ? "\n" : "", + res.results[i].node.qualified_name, res.results[i].node.file_path); + ok = n >= 0 && (size_t)n < sz - len; + len += ok ? (size_t)n : 0; + } + cbm_store_search_free(&res); + cbm_store_close(s); + return ok; +} + +/* One index of a Next.js tree: its result, the route it took and the stored + * Routes after it (nextjs_route_tuples). */ +typedef struct { + int rc; + cbm_incremental_route_t route; + bool read; + char tuples[1024]; +} nextjs_run_t; + +static void nextjs_run(const char *repo, const char *db_path, nextjs_run_t *run) { + run->rc = nextjs_index(repo, db_path); + run->route = cbm_pipeline_incremental_test_last_route(); + run->read = nextjs_route_tuples(db_path, run->tuples, sizeof(run->tuples)); +} + +/* #759: the HTTP verb Functions a Next.js route file exports HANDLE the Routes + * its directories name, in the stored graph: a dynamic segment, a group, a + * catch-all and the root, under src/app/ and app/. Each Route is there once, + * with its file, from its own handler, whose HANDLES property is its QN. A + * search by label Route and the declaring file's glob finds them. `fillers` + * more files put the index on the parallel extraction path. */ +static int nextjs_persisted_contract(int fillers) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_nextjs_routes_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + for (int i = 0; i < fillers; i++) { + char name[64]; + char body[128]; + snprintf(name, sizeof(name), "lib/filler%d.ts", i); + snprintf(body, sizeof(body), "export function filler%d(): number {\n return %d;\n}\n", i, + i); + write_temp_file(tmp, name, body); + } + write_temp_file(tmp, "src/app/api/users/[id]/route.ts", + "export function GET() {\n return 1;\n}\n" + "export const POST = async () => 1;\n"); + write_temp_file(tmp, "src/app/(public)/api/[org]/users/[id]/route.ts", + "export async function GET() {\n return 2;\n}\n"); + write_temp_file(tmp, "app/api/[...slug]/route.ts", "export function GET() {\n return 3;\n}\n"); + write_temp_file(tmp, "app/route.js", "export function GET() {\n return 4;\n}\n"); + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/nextjs.db", tmp); + + char *old_workers = getenv("CBM_WORKERS"); + char *saved = old_workers ? strdup(old_workers) : NULL; + if (fillers > 0) { + cbm_setenv("CBM_WORKERS", "4", 1); + } + nextjs_run_t full; + nextjs_run(tmp, db_path, &full); + if (saved) { + cbm_setenv("CBM_WORKERS", saved, 1); + free(saved); + } else { + cbm_unsetenv("CBM_WORKERS"); + } + char found[1024]; + bool searched = + nextjs_found_routes(db_path, "src/app/api/users/*/route.ts", found, sizeof(found)); + th_rmtree(tmp); + + ASSERT_EQ(full.rc, 0); + ASSERT_TRUE(full.read); + ASSERT_STR_EQ( + full.tuples, + "GET / __route__GET__/ app/route.js nextjs.app.route.GET\n" + "GET /api/*slug __route__GET__/api/*slug app/api/[...slug]/route.ts " + "nextjs.app.api.[...slug].route.GET\n" + "GET /api/:org/users/:id __route__GET__/api/{}/users/{} " + "src/app/(public)/api/[org]/users/[id]/route.ts " + "nextjs.src.app.(public).api.[org].users.[id].route.GET\n" + "GET /api/users/:id __route__GET__/api/users/{} src/app/api/users/[id]/route.ts " + "nextjs.src.app.api.users.[id].route.GET\n" + "POST /api/users/:id __route__POST__/api/users/{} src/app/api/users/[id]/route.ts " + "nextjs.src.app.api.users.[id].route.POST"); + ASSERT_TRUE(searched); + ASSERT_STR_EQ(found, "__route__GET__/api/users/{} src/app/api/users/[id]/route.ts\n" + "__route__POST__/api/users/{} src/app/api/users/[id]/route.ts"); + PASS(); +} + +TEST(pipeline_nextjs_persisted_route_contract_issue759) { + return nextjs_persisted_contract(0); +} + +TEST(pipeline_nextjs_persisted_route_contract_parallel_issue759) { + return nextjs_persisted_contract(52); +} + +/* #759 over the life of a route file. Each step re-indexes the stored graph; + * after each, the file's current handlers are its only Routes, each once, and + * an unrelated route file keeps its Route. An unchanged tree is a no-op, and a + * change to another file rebuilds the unchanged route files' Routes from their + * stored handlers (pass_route_nodes.c), which a search still finds. */ +TEST(pipeline_nextjs_incremental_lifecycle_issue759) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_nextjs_life_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + write_temp_file(tmp, "app/api/items/route.ts", + "export function GET() {\n return 1;\n}\n" + "export async function POST() {\n return 2;\n}\n"); + write_temp_file(tmp, "app/api/health/route.ts", "export const GET = () => 'ok';\n"); + write_temp_file(tmp, "lib/helper.ts", "export function helper(): number {\n return 1;\n}\n"); + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/nextjs.db", tmp); + + nextjs_run_t full; + nextjs_run(tmp, db_path, &full); + nextjs_run_t same; + nextjs_run(tmp, db_path, &same); + char found_same[1024]; + bool searched_same = + nextjs_found_routes(db_path, "app/api/*/route.ts", found_same, sizeof(found_same)); + + write_temp_file(tmp, "lib/helper.ts", "export function helper(): number {\n return 2;\n}\n"); + nextjs_run_t helper_only; + nextjs_run(tmp, db_path, &helper_only); + char found_repair[1024]; + bool searched_repair = + nextjs_found_routes(db_path, "app/api/*/route.ts", found_repair, sizeof(found_repair)); + + write_temp_file(tmp, "app/api/items/route.ts", "export function GET() {\n return 1;\n}\n"); + nextjs_run_t no_post; + nextjs_run(tmp, db_path, &no_post); + + write_temp_file(tmp, "app/api/items/route.ts", + "export function GET() {\n const n = 10;\n return n;\n}\n"); + nextjs_run_t new_body; + nextjs_run(tmp, db_path, &new_body); + + write_temp_file(tmp, "app/api/items/route.ts", "export function PATCH() {\n return 3;\n}\n"); + nextjs_run_t patch; + nextjs_run(tmp, db_path, &patch); + + write_temp_file(tmp, "app/api/products/route.ts", + "export function PATCH() {\n return 3;\n}\n"); + stem_unlink(tmp, "app/api/items/route.ts"); + nextjs_run_t moved; + nextjs_run(tmp, db_path, &moved); + + write_temp_file(tmp, "app/api/products/[id]/route.ts", + "export function PATCH() {\n return 3;\n}\n"); + stem_unlink(tmp, "app/api/products/route.ts"); + nextjs_run_t dynamic; + nextjs_run(tmp, db_path, &dynamic); + + stem_unlink(tmp, "app/api/products/[id]/route.ts"); + nextjs_run_t deleted; + nextjs_run(tmp, db_path, &deleted); + th_rmtree(tmp); + + static const char health[] = "GET /api/health __route__GET__/api/health " + "app/api/health/route.ts nextjs.app.api.health.route.GET"; + static const char routes[] = "GET /api/health __route__GET__/api/health " + "app/api/health/route.ts nextjs.app.api.health.route.GET\n" + "GET /api/items __route__GET__/api/items app/api/items/route.ts " + "nextjs.app.api.items.route.GET\n" + "POST /api/items __route__POST__/api/items app/api/items/route.ts " + "nextjs.app.api.items.route.POST"; + static const char get_only[] = "GET /api/health __route__GET__/api/health " + "app/api/health/route.ts nextjs.app.api.health.route.GET\n" + "GET /api/items __route__GET__/api/items app/api/items/route.ts " + "nextjs.app.api.items.route.GET"; + ASSERT_EQ(full.rc, 0); + ASSERT_TRUE(full.read); + ASSERT_STR_EQ(full.tuples, routes); + ASSERT_EQ(same.rc, 0); + ASSERT_EQ(same.route, CBM_INCREMENTAL_ROUTE_NOOP); + ASSERT_STR_EQ(same.tuples, routes); + ASSERT_TRUE(searched_same); + ASSERT_STR_EQ(found_same, "__route__GET__/api/health app/api/health/route.ts\n" + "__route__GET__/api/items app/api/items/route.ts\n" + "__route__POST__/api/items app/api/items/route.ts"); + ASSERT_EQ(helper_only.rc, 0); + ASSERT_EQ(helper_only.route, CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR); + ASSERT_STR_EQ(helper_only.tuples, routes); + ASSERT_TRUE(searched_repair); + ASSERT_STR_EQ(found_repair, found_same); + ASSERT_EQ(no_post.rc, 0); + ASSERT_EQ(no_post.route, CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR); + ASSERT_STR_EQ(no_post.tuples, get_only); + ASSERT_EQ(new_body.rc, 0); + ASSERT_EQ(new_body.route, CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR); + ASSERT_STR_EQ(new_body.tuples, get_only); + ASSERT_EQ(patch.rc, 0); + ASSERT_STR_EQ(patch.tuples, "GET /api/health __route__GET__/api/health " + "app/api/health/route.ts nextjs.app.api.health.route.GET\n" + "PATCH /api/items __route__PATCH__/api/items " + "app/api/items/route.ts nextjs.app.api.items.route.PATCH"); + ASSERT_EQ(moved.rc, 0); + ASSERT_STR_EQ(moved.tuples, "GET /api/health __route__GET__/api/health " + "app/api/health/route.ts nextjs.app.api.health.route.GET\n" + "PATCH /api/products __route__PATCH__/api/products " + "app/api/products/route.ts nextjs.app.api.products.route.PATCH"); + ASSERT_EQ(dynamic.rc, 0); + ASSERT_STR_EQ(dynamic.tuples, + "GET /api/health __route__GET__/api/health " + "app/api/health/route.ts nextjs.app.api.health.route.GET\n" + "PATCH /api/products/:id __route__PATCH__/api/products/{} " + "app/api/products/[id]/route.ts nextjs.app.api.products.[id].route.PATCH"); + ASSERT_EQ(deleted.rc, 0); + ASSERT_STR_EQ(deleted.tuples, health); + PASS(); +} + +/* CBM_SEMANTIC_INDEX_VERSION 10 (#759): an index version 9 wrote has no route + * metadata, Route or HANDLES for a Next.js route file, and an unchanged tree + * is otherwise a no-op, so the version is what makes the new binary rebuild + * it. The stored index is put back to the version-9 state by hand (metadata, + * the handlers' route properties, the Routes and their edges); the run after + * that must replace it in full, the next one is a no-op. */ +TEST(pipeline_nextjs_semantic_upgrade_issue759) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_nextjs_upgrade_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + write_temp_file(tmp, "app/api/items/route.ts", + "export function GET() {\n return 1;\n}\n" + "export const POST = async () => 2;\n"); + write_temp_file(tmp, "lib/helper.ts", "export function helper(): number {\n return 1;\n}\n"); + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/nextjs.db", tmp); + static const char all_nodes[] = "SELECT COUNT(*) FROM nodes"; + static const char all_edges[] = "SELECT COUNT(*) FROM edges"; + static const char routed[] = + "SELECT COUNT(*) FROM nodes WHERE json_extract(properties,'$.route_path') IS NOT NULL"; + + nextjs_run_t full; + nextjs_run(tmp, db_path, &full); + int nodes_before = k8s_inc_count(db_path, all_nodes); + int edges_before = k8s_inc_count(db_path, all_edges); + + /* Put the published index back to what version 9 wrote. */ + cbm_store_t *store = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(store); + cbm_coverage_row_t *coverage_rows = NULL; + int coverage_count = 0; + ASSERT_EQ(cbm_store_coverage_get(store, "nextjs", &coverage_rows, &coverage_count), + CBM_STORE_OK); + cbm_coverage_meta_t meta = {0}; + ASSERT_EQ(cbm_store_coverage_meta_get(store, "nextjs", &meta), CBM_STORE_OK); + cbm_coverage_meta_t old_meta = meta; + old_meta.coverage_version = 9; + ASSERT_EQ( + cbm_store_coverage_replace_ex(store, "nextjs", coverage_rows, coverage_count, &old_meta), + CBM_STORE_OK); + cbm_store_free_coverage(coverage_rows, coverage_count); + cbm_store_coverage_meta_clear(&meta); + ASSERT_EQ(cbm_store_exec(store, + "DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE " + "label = 'Route') OR target_id IN (SELECT id FROM nodes WHERE " + "label = 'Route');" + "DELETE FROM nodes WHERE label = 'Route';" + "UPDATE nodes SET properties = json_remove(properties, " + "'$.route_path', '$.route_method') " + "WHERE json_extract(properties, '$.route_path') IS NOT NULL;"), + CBM_STORE_OK); + cbm_store_close(store); + nextjs_run_t version_9 = {0}; + version_9.read = nextjs_route_tuples(db_path, version_9.tuples, sizeof(version_9.tuples)); + int routed_9 = k8s_inc_count(db_path, routed); + + /* Nothing in the tree changed: only the version says the index is stale. */ + nextjs_run_t upgrade; + nextjs_run(tmp, db_path, &upgrade); + int nodes_after = k8s_inc_count(db_path, all_nodes); + int edges_after = k8s_inc_count(db_path, all_edges); + int routed_after = k8s_inc_count(db_path, routed); + store = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(store); + cbm_coverage_meta_t new_meta = {0}; + ASSERT_EQ(cbm_store_coverage_meta_get(store, "nextjs", &new_meta), CBM_STORE_OK); + int stored_version = new_meta.coverage_version; + cbm_store_coverage_meta_clear(&new_meta); + cbm_store_close(store); + nextjs_run_t again; + nextjs_run(tmp, db_path, &again); + th_rmtree(tmp); + + ASSERT_EQ(full.rc, 0); + ASSERT_STR_EQ(full.tuples, "GET /api/items __route__GET__/api/items app/api/items/route.ts " + "nextjs.app.api.items.route.GET\n" + "POST /api/items __route__POST__/api/items app/api/items/route.ts " + "nextjs.app.api.items.route.POST"); + ASSERT_TRUE(version_9.read); + ASSERT_STR_EQ(version_9.tuples, ""); + ASSERT_EQ(routed_9, 0); + ASSERT_EQ(upgrade.rc, 0); + ASSERT_EQ(upgrade.route, CBM_INCREMENTAL_ROUTE_FORCED_FULL); + ASSERT_STR_EQ(upgrade.tuples, full.tuples); + ASSERT_EQ(routed_after, 2); + ASSERT_EQ(nodes_after, nodes_before); + ASSERT_EQ(edges_after, edges_before); + ASSERT_EQ(stored_version, CBM_SEMANTIC_INDEX_VERSION); + ASSERT_EQ(again.rc, 0); + ASSERT_EQ(again.route, CBM_INCREMENTAL_ROUTE_NOOP); + ASSERT_STR_EQ(again.tuples, full.tuples); + ASSERT_GTE(CBM_SEMANTIC_INDEX_VERSION, 10); + PASS(); +} + +/* #759: a quote is legal in a POSIX directory name, so a route path, its file + * and its handler's QN can hold one, escaped in the stored JSON. The route pass + * builds the Route from its handler's stored properties on every index + * (pass_route_nodes.c), so the full index and an incremental one after an + * unrelated edit must both store it intact. */ +TEST(pipeline_nextjs_quoted_route_round_trip_issue759) { +#ifdef _WIN32 + SKIP_PLATFORM("a quote is not a legal Windows path character"); +#else + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_nextjs_quote_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + write_temp_file(tmp, "app/api/a\"b/route.ts", "export function GET() {\n return 1;\n}\n"); + write_temp_file(tmp, "lib/helper.ts", "export function helper(): number {\n return 1;\n}\n"); + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/nextjs.db", tmp); + + nextjs_run_t full; + nextjs_run(tmp, db_path, &full); + write_temp_file(tmp, "lib/helper.ts", "export function helper(): number {\n return 2;\n}\n"); + nextjs_run_t helper_only; + nextjs_run(tmp, db_path, &helper_only); + th_rmtree(tmp); + + ASSERT_EQ(full.rc, 0); + ASSERT_TRUE(full.read); + ASSERT_STR_EQ(full.tuples, "GET /api/a\"b __route__GET__/api/a\"b app/api/a\"b/route.ts " + "nextjs.app.api.a\"b.route.GET"); + ASSERT_EQ(helper_only.rc, 0); + ASSERT_EQ(helper_only.route, CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR); + ASSERT_TRUE(helper_only.read); + ASSERT_STR_EQ(helper_only.tuples, full.tuples); + PASS(); +#endif +} + /* `#include ` from arch/x/bugs.c: two headers end with the * include path (include/linux/device.h, tools/virtio/linux/device.h) and each * declares `struct device`. The exact-file lookup returned the first matching @@ -27771,6 +28188,11 @@ SUITE(pipeline) { RUN_TEST(pipeline_c_variants_on_sequential_and_parallel_paths_c1); RUN_TEST(pipeline_cpp_overloads_are_listed_as_variants_c1); RUN_TEST(pipeline_semantic_version_3_index_is_rebuilt_in_full_c1); + RUN_TEST(pipeline_nextjs_persisted_route_contract_issue759); + RUN_TEST(pipeline_nextjs_persisted_route_contract_parallel_issue759); + RUN_TEST(pipeline_nextjs_incremental_lifecycle_issue759); + RUN_TEST(pipeline_nextjs_semantic_upgrade_issue759); + RUN_TEST(pipeline_nextjs_quoted_route_round_trip_issue759); RUN_TEST(pipeline_python_app_factory_routes_issue1548); RUN_TEST(pipeline_python_nested_def_is_function_local_issue1548); RUN_TEST(pipeline_python_local_classes_stay_in_their_def_issue1548); From 349d80c64361df8d0aea8c695cf1549445088ddb Mon Sep 17 00:00:00 2001 From: Shane McCarron Date: Thu, 29 Oct 2026 11:46:19 -0500 Subject: [PATCH 09/15] perf(index): run supervised index workers at below-normal CPU priority A full index saturates every core in the parallel extraction pass, and interactive CLI/MCP queries (CPU-bound at start-up) were measured taking twice their wall time behind it with flat CPU time: pure run-queue contention. The index-worker role now lowers its own priority once, before any indexing thread is created: nice 10 on POSIX, BELOW_NORMAL_PRIORITY_CLASS on Windows. It never raises priority: a process already nicer than 10 (or in the IDLE / BELOW_NORMAL class) is left alone. Plain nice only, not macOS background QoS, which would pin indexing to efficiency cores and throttle its I/O. Failure is logged and ignored. The test runs the helper in a forked child and checks both legs: a normal-priority child ends at nice >= 10, and a child started nicer than that is left where it was. Refs #2834 Signed-off-by: Shane McCarron --- src/foundation/platform.c | 25 +++++++++++++++++++++ src/foundation/platform.h | 13 +++++++++++ src/main.c | 6 +++++ tests/test_platform.c | 47 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+) diff --git a/src/foundation/platform.c b/src/foundation/platform.c index d11df686e..3a4ac1642 100644 --- a/src/foundation/platform.c +++ b/src/foundation/platform.c @@ -729,3 +729,28 @@ const char *cbm_resolve_cache_dir(void) { } return buf; } + +/* ── CPU scheduling priority ────────────────── */ + +#ifndef _WIN32 +#include +#endif + +bool cbm_lower_cpu_priority(void) { +#ifdef _WIN32 + HANDLE self = GetCurrentProcess(); + DWORD current = GetPriorityClass(self); + if (current == IDLE_PRIORITY_CLASS || current == BELOW_NORMAL_PRIORITY_CLASS) { + return true; /* already at least this low: never raise priority */ + } + return SetPriorityClass(self, BELOW_NORMAL_PRIORITY_CLASS) != 0; +#else + /* getpriority can legitimately return -1, so errno is the only error signal. */ + errno = 0; + int current = getpriority(PRIO_PROCESS, 0); + if (errno == 0 && current >= CBM_LOW_CPU_PRIORITY_NICE) { + return true; /* already at least this nice: never raise priority */ + } + return setpriority(PRIO_PROCESS, 0, CBM_LOW_CPU_PRIORITY_NICE) == 0; +#endif +} diff --git a/src/foundation/platform.h b/src/foundation/platform.h index 40b020222..8eb462778 100644 --- a/src/foundation/platform.h +++ b/src/foundation/platform.h @@ -215,4 +215,17 @@ int64_t cbm_file_size(const char *path); * On POSIX, this is a no-op. Returns the input pointer. */ char *cbm_normalize_path_sep(char *path); +/* ── CPU scheduling priority ───────────────────────────────────── */ + +enum { CBM_LOW_CPU_PRIORITY_NICE = 10 }; + +/* Lower this process's CPU scheduling priority so background work yields the + * cores to interactive processes. POSIX: nice CBM_LOW_CPU_PRIORITY_NICE (never + * raises an already-nicer process). Plain nice only, deliberately not macOS + * background QoS: that pins work to efficiency cores and throttles I/O. + * Windows: BELOW_NORMAL_PRIORITY_CLASS. On Linux nice is per thread, so call + * this before creating threads; later threads inherit it. + * Returns false when the priority could not be changed. */ +bool cbm_lower_cpu_priority(void); + #endif /* CBM_PLATFORM_H */ diff --git a/src/main.c b/src/main.c index c2f22ada9..3f5e80e22 100644 --- a/src/main.c +++ b/src/main.c @@ -902,6 +902,12 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo * own here, and only here (cbm_sqlite_dedicated_heap). */ cbm_sqlite_dedicated_heap(true); cbm_memev_process_role("worker"); + /* A full index saturates every core; run it below normal priority so + * interactive CLI/MCP queries are not starved. Before any indexing + * thread exists, so they all inherit it. Failure is harmless. */ + if (!cbm_lower_cpu_priority()) { + cbm_log_info("index.worker.priority", "lowered", "false"); + } } cbm_index_set_worker_role_options(index_worker, response_out, worker_single_thread, worker_marker, worker_quarantine, diff --git a/tests/test_platform.c b/tests/test_platform.c index 2c383c025..d83f90dab 100644 --- a/tests/test_platform.c +++ b/tests/test_platform.c @@ -16,6 +16,9 @@ #include #ifndef _WIN32 #include /* chmod for the symlink-policy mode legs */ +#include +#include /* getpriority for the worker-priority leg */ +#include #endif #ifdef __linux__ @@ -1442,6 +1445,49 @@ TEST(regex_estimate_counts_brackets_and_escapes_literally) { PASS(); } +/* An index worker lowers its own priority (#2834). A non-root process cannot + * raise it back, so the helper runs in a forked child that reports its nice. */ +TEST(platform_lower_cpu_priority_nices_the_process) { +#ifdef _WIN32 + SKIP_PLATFORM("POSIX nice contract"); +#else + int fds[2]; + ASSERT_EQ(pipe(fds), 0); + pid_t child = fork(); + if (child == 0) { + (void)close(fds[0]); + /* report: lowered ok, nice after; then, from a nicer start, ok, target, nice after */ + int report[5] = {cbm_lower_cpu_priority() ? 1 : 0, 0, 0, 0, 0}; + errno = 0; + report[1] = getpriority(PRIO_PROCESS, 0); + if (errno != 0) { + report[0] = 0; + } + /* Never-raise leg: start nicer than the helper's level; it must leave it. */ + report[3] = report[1] >= 15 ? 19 : 15; + if (setpriority(PRIO_PROCESS, 0, report[3]) == 0) { + report[2] = cbm_lower_cpu_priority() ? 1 : 0; + errno = 0; + report[4] = getpriority(PRIO_PROCESS, 0); + } + _exit(write(fds[1], report, sizeof(report)) == (ssize_t)sizeof(report) ? 0 : 1); + } + ASSERT_TRUE(child > 0); + (void)close(fds[1]); + int report[5] = {0, 0, 0, 0, 0}; + ssize_t got = read(fds[0], report, sizeof(report)); + (void)close(fds[0]); + int status = 0; + (void)waitpid(child, &status, 0); + ASSERT_EQ(got, (ssize_t)sizeof(report)); + ASSERT_EQ(report[0], 1); + ASSERT_TRUE(report[1] >= CBM_LOW_CPU_PRIORITY_NICE); + ASSERT_EQ(report[2], 1); + ASSERT_EQ(report[4], report[3]); + PASS(); +#endif +} + SUITE(platform) { RUN_TEST(platform_file_apis_survive_max_path_overflow); RUN_TEST(platform_mkdir_p_follows_own_symlink_only_when_opted_in); @@ -1477,6 +1523,7 @@ SUITE(platform) { RUN_TEST(platform_env_long_refuses_what_it_cannot_read); RUN_TEST(platform_env_dup_honours_the_settings_resolver); RUN_TEST(platform_system_info); + RUN_TEST(platform_lower_cpu_priority_nices_the_process); RUN_TEST(regex_compile_refuses_oversized_expansion); RUN_TEST(regex_compile_accepts_expansion_under_budget); RUN_TEST(regex_compile_refuses_overlong_pattern); From 11f20c578bf142a0a896f04f319491eec449c685 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:34:44 +0000 Subject: [PATCH 10/15] build(deps): bump actions/setup-python from 7.0.0 to 7.1.0 Bumps [actions/setup-python](https://github.com/actions/setup-python) from 7.0.0 to 7.1.0. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](https://github.com/actions/setup-python/compare/5fda3b95a4ea91299a34e894583c3862153e4b97...6d4fd22b20465ccc0f7354476588e8df7c9ca0e0) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: 7.1.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/_test.yml | 2 +- .github/workflows/release.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 501a82b20..ba967f283 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -318,7 +318,7 @@ jobs: with: node-version: "22" - - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 with: python-version: "3.12" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e4926260d..020ea3f0f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -535,7 +535,7 @@ jobs: # ── PyPI ───────────────────────────────────────────────── - name: Setup Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 with: python-version: '3.12' From 083bc94d5e37776ed53479adb2ba36d0a6ee13a0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:34:52 +0000 Subject: [PATCH 11/15] build(deps): bump ossf/scorecard-action from 2.4.4 to 2.5.0 Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.4.4 to 2.5.0. - [Release notes](https://github.com/ossf/scorecard-action/releases) - [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md) - [Commits](https://github.com/ossf/scorecard-action/compare/2d1146689b8cda280b9bc96326124645441f03bc...fa7117db83c52d787ae5a89f350999701505d78e) --- updated-dependencies: - dependency-name: ossf/scorecard-action dependency-version: 2.5.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/scorecard.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index dfb8ae59d..d5a794173 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -26,7 +26,7 @@ jobs: persist-credentials: false - name: Run OpenSSF Scorecard - uses: ossf/scorecard-action@2d1146689b8cda280b9bc96326124645441f03bc # v2.4.4 + uses: ossf/scorecard-action@fa7117db83c52d787ae5a89f350999701505d78e # v2.5.0 with: results_file: results.sarif results_format: sarif From 17415f6a3806a8529c0fc217b04f0cd9a056ce9f Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 1 Nov 2026 22:43:58 +0100 Subject: [PATCH 12/15] test(install): rename the Android guard test to the test_*.sh convention Every shell contract test under tests/ is named test_*.sh with underscores. A pure rename, so history and blame follow the file. Refs #1202 Signed-off-by: Martin Vogel --- ...est-install-android-guard.sh => test_install_android_guard.sh} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test-install-android-guard.sh => test_install_android_guard.sh} (100%) diff --git a/tests/test-install-android-guard.sh b/tests/test_install_android_guard.sh similarity index 100% rename from tests/test-install-android-guard.sh rename to tests/test_install_android_guard.sh From 1fc70c2024ed6ff7e4fb5c735a342143c8572e02 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 1 Nov 2026 22:43:58 +0100 Subject: [PATCH 13/15] test(install): run the Android guard test as Step 0x2 of scripts/test.sh scripts/test.sh lists every shell contract test explicitly and does not glob tests/, so an unlisted test never ran on any leg. Register tests/test_install_android_guard.sh as Step 0x2, right after Step 0x, in the same two-line shape as its neighbours. Give the two PATH stubs (uname, curl) the same shebang every other stub under tests/ uses, so they are proper executables on every leg instead of relying on the shell fallback for files without one. Refs #1202 Signed-off-by: Martin Vogel --- scripts/test.sh | 3 +++ tests/test_install_android_guard.sh | 2 ++ 2 files changed, 5 insertions(+) diff --git a/scripts/test.sh b/scripts/test.sh index 992aea837..baf41a935 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -278,6 +278,9 @@ bash "$ROOT/tests/test_language_count_contract.sh" echo "=== Step 0x: packaging version-metadata contract ===" bash "$ROOT/tests/test_version_metadata_contract.sh" +echo "=== Step 0x2: installer Android/Termux guard contract (#1202) ===" +bash "$ROOT/tests/test_install_android_guard.sh" + # Step 0y: the Windows leg must not decide its verdict from an exit status that # the ssh/msys2_shell chain can mangle — a channel that turns 0 into 1 can turn # 1 into 0, and that direction reports a RED Windows leg as green. Runs diff --git a/tests/test_install_android_guard.sh b/tests/test_install_android_guard.sh index 6b3a383a2..59503fc7d 100755 --- a/tests/test_install_android_guard.sh +++ b/tests/test_install_android_guard.sh @@ -7,6 +7,7 @@ trap 'rm -rf "$WORK"' EXIT mkdir -p "$WORK/bin" "$WORK/home" cat >"$WORK/bin/uname" <<'EOF' +#!/usr/bin/env bash case "$1" in -s) printf '%s\n' Linux ;; -m) printf '%s\n' aarch64 ;; @@ -15,6 +16,7 @@ case "$1" in esac EOF cat >"$WORK/bin/curl" <<'EOF' +#!/usr/bin/env bash printf 'curl must not run for Android/Termux\n' >&2 exit 99 EOF From 8b2b0d769f00dcfb994fd250ecbd9f58fd395973 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 1 Nov 2026 22:43:59 +0100 Subject: [PATCH 14/15] fix(install): say that Termux is not a supported platform yet The guard told users to build from source with scripts/build.sh in Termux, which reads as a supported path. Source builds under Termux still have open problems tracked in #1202, so the message now says why the portable binary cannot run there and that a source build may work but is not supported yet, pointing at #1202. The test still matches Android/Termux in the new message. Refs #1202 Signed-off-by: Martin Vogel --- install.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/install.sh b/install.sh index 01ab6c378..f3666e5d4 100755 --- a/install.sh +++ b/install.sh @@ -157,8 +157,8 @@ is_android() { OS=$(detect_os) ARCH=$(detect_arch) if [ "$OS" = "linux" ] && is_android; then - echo "error: Android/Termux is not supported by the Linux portable binary." >&2 - echo "Build from source with scripts/build.sh in Termux or use a supported Linux host." >&2 + echo 'error: Android/Termux cannot run the Linux portable binary (static glibc; Android kills it with "Bad system call").' >&2 + echo "Building from source in Termux may work but is not a supported platform yet - see issue #1202." >&2 exit 1 fi From 7a7ecb7356c152e0f6d22ac4d208b5919a21c862 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 1 Nov 2026 23:02:46 +0100 Subject: [PATCH 15/15] test(install): keep the Android guard test off the network Point CBM_DOWNLOAD_URL, the documented download-base override in install.sh, at a closed loopback port. If a curl stub were ever missed on some leg, the download would then fail locally instead of fetching a real release. Check that the uname stub is the one PATH resolves before running install.sh. Otherwise the test would quietly exercise the real host instead of an Android one and prove nothing. Refs #1202 Signed-off-by: Martin Vogel --- tests/test_install_android_guard.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_install_android_guard.sh b/tests/test_install_android_guard.sh index 59503fc7d..b4bd6a704 100755 --- a/tests/test_install_android_guard.sh +++ b/tests/test_install_android_guard.sh @@ -22,8 +22,16 @@ exit 99 EOF chmod +x "$WORK/bin/uname" "$WORK/bin/curl" +# The stubs must be what install.sh sees, or this test proves nothing. +if [ "$(PATH="$WORK/bin:$PATH" uname -o)" != "Android" ]; then + printf 'FAIL: the uname stub is not picked up from PATH\n' >&2 + exit 1 +fi + set +e -output=$(PATH="$WORK/bin:$PATH" HOME="$WORK/home" bash "$ROOT/install.sh" --skip-config 2>&1) +output=$(PATH="$WORK/bin:$PATH" HOME="$WORK/home" \ + CBM_DOWNLOAD_URL="http://127.0.0.1:9/cbm-android-guard-test" \ + bash "$ROOT/install.sh" --skip-config 2>&1) rc=$? set -e