From e8dcb28ba719e4272f605f3b961422bb1f3c3fad Mon Sep 17 00:00:00 2001 From: Brian McNaboe Date: Thu, 20 Aug 2026 12:26:03 -0400 Subject: [PATCH 01/10] test(hook-augment): pin worktree payload-cwd resolution The hook resolves a payload cwd inside a linked git worktree to that worktree own indexed project (and a subdir of it), never a sibling checkout or nothing, and an unindexed worktree gets a deliberate "no indexed graph project matched" notice rather than a silent 0 bytes. Main-checkout resolution is unchanged. These regression tests use a real linked worktree and lock the payload-cwd -> project invariant. Fixes #1752 Signed-off-by: Brian McNaboe --- tests/test_cli.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) diff --git a/tests/test_cli.c b/tests/test_cli.c index 9762808680..c13e27b996 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -9342,6 +9342,175 @@ TEST(cli_hook_session_exhausts_project_registry_pages) { PASS(); } +#ifndef _WIN32 +/* A linked git worktree is indexed as its OWN project, keyed by its checkout + * path. The SessionStart hook must return that project for a payload cwd inside + * the worktree (or a subdir of it) — never the main checkout's — and the main + * checkout's own resolution must be unchanged. Pins the payload-cwd → project + * resolution so a worktree can never regress to resolving a sibling checkout or + * to nothing (the historical silent-0-bytes worktree bug). */ +TEST(cli_hook_worktree_cwd_resolves_own_indexed_project) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-hook-worktree-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + char cache[512]; + char maindir[512]; + char wtdir[512]; + char subdir[600]; + snprintf(cache, sizeof(cache), "%s/cache", tmpdir); + snprintf(maindir, sizeof(maindir), "%s/checkout-main", tmpdir); + snprintf(wtdir, sizeof(wtdir), "%s/checkout-side", tmpdir); + snprintf(subdir, sizeof(subdir), "%s/nested", wtdir); + test_mkdirp(cache); + test_mkdirp(maindir); + + /* A real linked worktree so is_worktree/git_common_dir/canonical_root are + * exactly what the hook resolves against in the field. */ + char cmd[2048]; + snprintf(cmd, sizeof(cmd), + "cd \"%s\" && git init -q && git config user.email t@t && git config user.name t && " + ": > f.txt && git add f.txt && git commit -qm init && " + "git worktree add -q \"%s\" -b agl7branch", + maindir, wtdir); + if (system(cmd) != 0) { + test_rmdir_r(tmpdir); + FAIL("git worktree setup failed"); + } + test_mkdirp(subdir); + + /* Register each checkout as its own indexed project under the exact + * path-derived name the indexer uses (the db filename == that name). */ + char *main_name = cbm_project_name_from_path(maindir); + char *wt_name = cbm_project_name_from_path(wtdir); + ASSERT_NOT_NULL(main_name); + ASSERT_NOT_NULL(wt_name); + ASSERT_STR_NEQ(main_name, wt_name); + char db_main[900]; + char db_wt[900]; + snprintf(db_main, sizeof(db_main), "%s/%s.db", cache, main_name); + snprintf(db_wt, sizeof(db_wt), "%s/%s.db", cache, wt_name); + cbm_store_t *sm = cbm_store_open_path(db_main); + ASSERT_NOT_NULL(sm); + ASSERT_EQ(cbm_store_upsert_project(sm, main_name, maindir), CBM_STORE_OK); + cbm_store_close(sm); + cbm_store_t *sw = cbm_store_open_path(db_wt); + ASSERT_NOT_NULL(sw); + ASSERT_EQ(cbm_store_upsert_project(sw, wt_name, wtdir), CBM_STORE_OK); + cbm_store_close(sw); + + char *saved_cache = save_test_env("CBM_CACHE_DIR"); + cbm_setenv("CBM_CACHE_DIR", cache, 1); + + /* The two names share a prefix but diverge (checkout-main vs checkout-side), + * so a bare-name strstr for one never matches the other's context. */ + char input[1024]; + snprintf(input, sizeof(input), "{\"hook_event_name\":\"SessionStart\",\"cwd\":\"%s\"}", wtdir); + char *wt_out = cbm_hook_augment_lifecycle_json(input); + bool wt_ok = wt_out && strstr(wt_out, wt_name) && strstr(wt_out, "is indexed") && + !strstr(wt_out, main_name); + free(wt_out); + + snprintf(input, sizeof(input), "{\"hook_event_name\":\"SessionStart\",\"cwd\":\"%s\"}", subdir); + char *sub_out = cbm_hook_augment_lifecycle_json(input); + bool sub_ok = sub_out && strstr(sub_out, wt_name) && strstr(sub_out, "is indexed"); + free(sub_out); + + snprintf(input, sizeof(input), "{\"hook_event_name\":\"SessionStart\",\"cwd\":\"%s\"}", + maindir); + char *main_out = cbm_hook_augment_lifecycle_json(input); + bool main_ok = main_out && strstr(main_out, main_name) && strstr(main_out, "is indexed") && + !strstr(main_out, wt_name); + free(main_out); + + restore_test_env("CBM_CACHE_DIR", saved_cache); + free(main_name); + free(wt_name); + snprintf(cmd, sizeof(cmd), "cd \"%s\" && git worktree remove --force \"%s\" >/dev/null 2>&1", + maindir, wtdir); + (void)system(cmd); + test_rmdir_r(tmpdir); + + if (!wt_ok) + FAIL("worktree cwd must resolve the worktree's own indexed project"); + if (!sub_ok) + FAIL("a subdir of the worktree must resolve the worktree's own project"); + if (!main_ok) + FAIL("main-checkout resolution must be unchanged and not leak the worktree project"); + PASS(); +} + +/* Completes the payload-cwd matrix (with the indexed-worktree test above and the + * deadline/logging contract in cli_hook_augment_deadline_breadcrumb_issue858): a + * linked worktree that was never indexed must yield a deliberate, non-empty "no + * project matched — run index_repository" notice, never a silent 0-byte reply + * that an agent cannot distinguish from "no matches". */ +TEST(cli_hook_unindexed_worktree_reports_no_match_not_silent) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-hook-unindexed-wt-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + char cache[512]; + char maindir[512]; + char wtdir[512]; + snprintf(cache, sizeof(cache), "%s/cache", tmpdir); + snprintf(maindir, sizeof(maindir), "%s/checkout-main", tmpdir); + snprintf(wtdir, sizeof(wtdir), "%s/checkout-side", tmpdir); + test_mkdirp(cache); + test_mkdirp(maindir); + + char cmd[2048]; + snprintf(cmd, sizeof(cmd), + "cd \"%s\" && git init -q && git config user.email t@t && git config user.name t && " + ": > f.txt && git add f.txt && git commit -qm init && " + "git worktree add -q \"%s\" -b agl7unindexed", + maindir, wtdir); + if (system(cmd) != 0) { + test_rmdir_r(tmpdir); + FAIL("git worktree setup failed"); + } + + /* Index ONLY the main checkout; the worktree is deliberately left out. */ + char *main_name = cbm_project_name_from_path(maindir); + ASSERT_NOT_NULL(main_name); + char db_main[900]; + snprintf(db_main, sizeof(db_main), "%s/%s.db", cache, main_name); + cbm_store_t *sm = cbm_store_open_path(db_main); + ASSERT_NOT_NULL(sm); + ASSERT_EQ(cbm_store_upsert_project(sm, main_name, maindir), CBM_STORE_OK); + cbm_store_close(sm); + + char *saved_cache = save_test_env("CBM_CACHE_DIR"); + cbm_setenv("CBM_CACHE_DIR", cache, 1); + + char input[1024]; + snprintf(input, sizeof(input), "{\"hook_event_name\":\"SessionStart\",\"cwd\":\"%s\"}", wtdir); + char *wt_out = cbm_hook_augment_lifecycle_json(input); + bool deliberate = wt_out && wt_out[0] && strstr(wt_out, "no indexed graph project matched") && + !strstr(wt_out, "is indexed"); + free(wt_out); + + snprintf(input, sizeof(input), "{\"hook_event_name\":\"SessionStart\",\"cwd\":\"%s\"}", + maindir); + char *main_out = cbm_hook_augment_lifecycle_json(input); + bool main_ok = main_out && strstr(main_out, main_name) && strstr(main_out, "is indexed"); + free(main_out); + + restore_test_env("CBM_CACHE_DIR", saved_cache); + free(main_name); + snprintf(cmd, sizeof(cmd), "cd \"%s\" && git worktree remove --force \"%s\" >/dev/null 2>&1", + maindir, wtdir); + (void)system(cmd); + test_rmdir_r(tmpdir); + + if (!deliberate) + FAIL("unindexed worktree must get a deliberate no-match notice, never a silent 0 bytes"); + if (!main_ok) + FAIL("the indexed main checkout must still resolve in the same matrix"); + PASS(); +} +#endif + TEST(cli_hook_session_sanitizes_untrusted_project_metadata) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-hook-untrusted-project-XXXXXX"); @@ -14984,6 +15153,10 @@ SUITE(cli) { RUN_TEST(cli_augment_session_uses_workspace_roots); RUN_TEST(cli_hook_session_resolves_custom_named_index_by_root_path); RUN_TEST(cli_hook_session_exhausts_project_registry_pages); +#ifndef _WIN32 + RUN_TEST(cli_hook_worktree_cwd_resolves_own_indexed_project); + RUN_TEST(cli_hook_unindexed_worktree_reports_no_match_not_silent); +#endif RUN_TEST(cli_hook_session_sanitizes_untrusted_project_metadata); RUN_TEST(cli_hook_ownership_requires_exact_command_identity); RUN_TEST(cli_gemini_hook_upgrade_migrates_released_exact_commands); From dadc57dbfa8615ce897c338f6af8d6c6eb1674b1 Mon Sep 17 00:00:00 2001 From: Tamer Date: Sun, 20 Sep 2026 21:37:41 +0200 Subject: [PATCH 02/10] fix(smoke): keep MCP tool inventory exact Signed-off-by: Tamer --- scripts/smoke-invariants.sh | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/scripts/smoke-invariants.sh b/scripts/smoke-invariants.sh index 3a4f9ce09f..175a599039 100755 --- a/scripts/smoke-invariants.sh +++ b/scripts/smoke-invariants.sh @@ -520,10 +520,10 @@ inv_mcp_initialize() { return 0 } -# ── Invariant 4: tools/list returns all expected tools ───────────────────── -# Cross-check against the canonical 14-tool list (TOOLS[] in src/mcp/mcp.c). -EXPECTED_TOOLS="index_repository search_graph query_graph trace_path get_code_snippet get_graph_schema get_architecture search_code list_projects delete_project index_status detect_changes manage_adr ingest_traces" -EXPECTED_TOOL_COUNT=14 +# ── Invariant 4: tools/list returns exactly the expected tools ───────────── +# Cross-check against the canonical registry (TOOLS[] in src/mcp/mcp.c). +EXPECTED_TOOLS="index_repository search_graph query_graph trace_path get_code_snippet get_file_outline get_graph_schema compare_graphs get_architecture search_code list_projects delete_project index_status check_index_coverage detect_changes manage_adr ingest_traces" +EXPECTED_TOOL_COUNT="$(printf '%s\n' $EXPECTED_TOOLS | grep -c .)" inv_tools_list() { if ! mcp_alive; then fail "tools-list" "server not alive" @@ -573,7 +573,7 @@ print(r.get("nextCursor") or "")' 2>/dev/null)" fail "tools-list" "got $got_count tools across $page page(s), expected $EXPECTED_TOOL_COUNT; names=[$got_names]" return fi - local missing="" + local missing="" extra="" duplicates="" local t for t in $EXPECTED_TOOLS; do case " $got_names " in @@ -581,10 +581,21 @@ print(r.get("nextCursor") or "")' 2>/dev/null)" *) missing="$missing $t" ;; esac done + for t in $got_names; do + case " $EXPECTED_TOOLS " in + *" $t "*) ;; + *) extra="$extra $t" ;; + esac + done + duplicates="$(printf '%s\n' $got_names | sort | uniq -d | tr '\n' ' ')" if [ -n "$missing" ]; then fail "tools-list" "missing tools:$missing" + elif [ -n "$extra" ]; then + fail "tools-list" "unexpected tools:$extra" + elif [ -n "$duplicates" ]; then + fail "tools-list" "duplicate tools: $duplicates" else - pass "tools-list (all $EXPECTED_TOOL_COUNT tools present)" + pass "tools-list (exactly $EXPECTED_TOOL_COUNT unique tools present)" fi } @@ -626,7 +637,7 @@ inv_every_tool() { return fi - # name|minimal-args (JSON object) for the remaining 13 tools. + # name|minimal-args (JSON object) for the remaining 16 tools. # Args chosen to be minimally valid per TOOLS[] required fields. local p="$PROJ_NAME" local -a CALLS @@ -635,11 +646,14 @@ inv_every_tool() { "query_graph|{\"project\":\"$p\",\"query\":\"MATCH (n) RETURN n.name LIMIT 5\"}" "trace_path|{\"project\":\"$p\",\"function_name\":\"compute\",\"direction\":\"both\"}" "get_code_snippet|{\"project\":\"$p\",\"qualified_name\":\"compute\"}" + "get_file_outline|{\"project\":\"$p\",\"file_path\":\"src/main.py\"}" "get_graph_schema|{\"project\":\"$p\"}" + "compare_graphs|{\"base_project\":\"$p\",\"target_project\":\"$p\"}" "get_architecture|{\"project\":\"$p\"}" "search_code|{\"project\":\"$p\",\"pattern\":\"def \"}" "list_projects|{}" "index_status|{\"project\":\"$p\"}" + "check_index_coverage|{\"project\":\"$p\",\"paths\":[\"src/main.py\"]}" "detect_changes|{\"project\":\"$p\"}" "manage_adr|{\"project\":\"$p\",\"mode\":\"get\"}" "ingest_traces|{\"project\":\"$p\",\"traces\":[]}" From cda5e3c9e5c4d2251467ced00ff305205c368d17 Mon Sep 17 00:00:00 2001 From: Tamer Date: Sun, 20 Sep 2026 21:37:43 +0200 Subject: [PATCH 03/10] docs(ci): avoid hard-coded MCP tool count Signed-off-by: Tamer --- .github/workflows/smoke.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index aa91f91c3d..e9b4f93575 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -1,7 +1,7 @@ # Smoke invariants across the WIDEST set of GitHub-hosted runners. Builds the # production source path with explicit fault-injection test seams and runs # scripts/smoke-invariants.sh -# (version/help, MCP initialize handshake [#513], all 14 tools invocable, malformed- +# (version/help, MCP initialize handshake [#513], all registered tools invocable, malformed- # input resilience, clean EOF exit, shared-lib resolution, install dry-run). # # Maximizing platforms is the point: ubuntu-22.04 (older glibc → AlmaLinux/#182 From 183cd727be5968fec3ce43176ebb1fa255f8976b Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Sun, 13 Sep 2026 13:53:45 +0300 Subject: [PATCH 04/10] fix(test): isolate smoke harness daemon runtime scripts/smoke-test.sh retires the account daemon from seven call sites, but its wrappers sandbox only HOME/TMPDIR/CBM_CACHE_DIR and only CBM_RUNTIME_DIR moves the daemon rendezvous, so every retirement reached the operator's live daemon. Source scripts/test-runtime.sh so every product process runs under a harness-owned runtime and cache, and clean that root up from the EXIT trap. Add tests/test_smoke_runtime_isolation_contract.sh, which fails before this change, and wire it into scripts/test.sh. Part of #1696. Signed-off-by: Anton Standrik --- scripts/smoke-test.sh | 17 +++- scripts/test.sh | 3 + .../test_smoke_runtime_isolation_contract.sh | 82 +++++++++++++++++++ 3 files changed, 99 insertions(+), 3 deletions(-) create mode 100755 tests/test_smoke_runtime_isolation_contract.sh diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 3371fc424a..e996da8475 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -22,6 +22,8 @@ they stage the release fixture, start the fixture server, and sandbox HOME/TEMP/agent-config destinations. Called bare, the download/checksum/ install-script phases (12-13) SKIP for lack of a fixture server, and the run mutates the REAL profile — the venue-parity contract forbids that in any venue. +The daemon runtime and cache are private to the run either way: every product +process is started under a CBM_RUNTIME_DIR/CBM_CACHE_DIR this harness owns. Arguments: product binary to smoke @@ -41,6 +43,15 @@ if [ -n "$SMOKE_MODE" ] && [ "$SMOKE_MODE" != "--agent-config-only" ]; then fi REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")/.." && pwd)" +# Every product process below — the phases, the install/update E2E and the +# daemon retirements — must reach a daemon rendezvous this run owns. Only +# CBM_RUNTIME_DIR moves that rendezvous; the wrappers' HOME/TMPDIR/CBM_CACHE_DIR +# sandbox does not, so without this the retirements land on the operator's live +# account daemon (#1691, #1696). +# shellcheck source=test-runtime.sh +source "$REPO_ROOT/scripts/test-runtime.sh" +cbm_test_runtime_init + smoke_mktemp_file() { if [ -n "${SMOKE_TEMP_ROOT:-}" ]; then mktemp "$SMOKE_TEMP_ROOT/cbm-smoke.XXXXXX" @@ -100,8 +111,8 @@ copy_smoke_binary() { cp "$BINARY" "$destination" } -# Retire the shared account daemon (if one is running) and wait until it -# reports not-running. Install/uninstall flows leave an ephemeral daemon +# Retire this run's private account daemon (if one is running) and wait until +# it reports not-running. Install/uninstall flows leave an ephemeral daemon # draining asynchronously whose mapped generation backing and open logs # block rm on Windows (POSIX rm doesn't care) — so every cleanup of a # fixture HOME that received an install, and the final cache removal, must @@ -149,7 +160,7 @@ CODEX_LIFECYCLE_HOME="" if command -v cygpath &>/dev/null; then TMPDIR=$(cygpath -m "$TMPDIR") fi -trap 'smoke_rmtree "$TMPDIR" "${DRYRUN_HOME:-}" "${CODEX_LIFECYCLE_HOME:-}"' EXIT +trap 'smoke_rmtree "$TMPDIR" "${DRYRUN_HOME:-}" "${CODEX_LIFECYCLE_HOME:-}"; cbm_test_runtime_cleanup "$BINARY"' EXIT CLI_STDERR=$(smoke_mktemp_file) # 10 of the cli call sites assign directly (VAR=$(cli ...)). Under diff --git a/scripts/test.sh b/scripts/test.sh index 50d65643f9..0845a2e77a 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -272,6 +272,9 @@ bash "$ROOT/tests/test_release_gate_chain_contract.sh" echo "=== Step 0t: test runtime isolation contract (#1691) ===" bash "$ROOT/tests/test_runtime_isolation_contract.sh" +echo "=== Step 0t2: smoke harness runtime isolation contract (#1696) ===" +bash "$ROOT/tests/test_smoke_runtime_isolation_contract.sh" + echo "=== Step 0u: shell line-ending contract ===" bash "$ROOT/tests/test_shell_line_endings.sh" diff --git a/tests/test_smoke_runtime_isolation_contract.sh b/tests/test_smoke_runtime_isolation_contract.sh new file mode 100755 index 0000000000..e91e1d38df --- /dev/null +++ b/tests/test_smoke_runtime_isolation_contract.sh @@ -0,0 +1,82 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Runtime-isolation contract for the smoke harness (#1696, follow-up to #1691). +# +# scripts/smoke-test.sh is the process that actually starts the product during +# a smoke run, and it retires "the account daemon" seven times through +# `daemon stop`. Its wrappers sandbox HOME/XDG/TMPDIR and CBM_CACHE_DIR, but +# only CBM_RUNTIME_DIR moves the daemon rendezvous (docs/CONFIGURATION.md), so +# without a private runtime every one of those stops lands on the operator's +# live daemon. Drive the harness with an environment-probe fixture and require +# that no product process ever receives the caller's runtime or cache. + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +WORKDIR="$(mktemp -d)" +trap 'rm -rf "$WORKDIR"' EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +normalize_path() { + local path=${1%$'\r'} + if command -v cygpath >/dev/null 2>&1; then + cygpath -u "$path" 2>/dev/null && return 0 + fi + printf '%s\n' "${path//\\//}" +} + +ENV_PROBE="$WORKDIR/environment-probe" +cat > "$ENV_PROBE" <<'EOF' +#!/usr/bin/env bash +printf '%s\t%s\n' "${CBM_CACHE_DIR-}" "${CBM_RUNTIME_DIR-}" >> "$CBM_SMOKE_ENV_PROBE" +case "${1-}" in + --version) echo "v0.0.0-probe"; exit 0 ;; + daemon) [[ "${2-}" == status ]] && exit 1; exit 0 ;; +esac +echo '{}' +exit 0 +EOF +chmod +x "$ENV_PROBE" + +CALLER_CACHE="$WORKDIR/caller-cache" +CALLER_RUNTIME="$WORKDIR/caller-runtime" +ENV_LOG="$WORKDIR/environment.log" +mkdir -p "$CALLER_CACHE" "$CALLER_RUNTIME" + +# The fixture answers nothing beyond --version, so the smoke fails early; only +# the environment it handed to the product is under test here. +CBM_CACHE_DIR="$CALLER_CACHE" \ +CBM_RUNTIME_DIR="$CALLER_RUNTIME" \ +CBM_SMOKE_ENV_PROBE="$ENV_LOG" \ + "$ROOT/scripts/smoke-test.sh" "$ENV_PROBE" > "$WORKDIR/smoke.out" 2>&1 || true + +[[ -s "$ENV_LOG" ]] || fail "smoke-test did not execute the environment-probe fixture" + +CALLER_CACHE_NORMALIZED=$(normalize_path "$CALLER_CACHE") +CALLER_RUNTIME_NORMALIZED=$(normalize_path "$CALLER_RUNTIME") +private_root="" +while IFS=$'\t' read -r child_cache_raw child_runtime_raw; do + child_cache=$(normalize_path "$child_cache_raw") + child_runtime=$(normalize_path "$child_runtime_raw") + if [[ -z "$child_runtime" || "$child_runtime" == "$CALLER_RUNTIME_NORMALIZED" ]]; then + fail "smoke-test exposed the caller CBM_RUNTIME_DIR to a product process" + fi + if [[ -z "$child_cache" || "$child_cache" == "$CALLER_CACHE_NORMALIZED" ]]; then + fail "smoke-test exposed the caller CBM_CACHE_DIR to a product process" + fi + if [[ "${child_runtime%/*}" != "${child_cache%/*}" || + "${child_runtime##*/}" != "runtime" || "${child_cache##*/}" != "cache" ]]; then + fail "smoke runtime/cache were not isolated beneath one private root" + fi + if [[ -n "$private_root" && "$private_root" != "${child_runtime%/*}" ]]; then + fail "smoke-test switched private roots mid-run" + fi + private_root="${child_runtime%/*}" +done < "$ENV_LOG" + +[[ ! -e "$private_root" ]] || fail "smoke-test left its private root behind: $private_root" + +echo "PASS: smoke harness isolates its daemon runtime and cache from the caller" From fa138acf44525ef94cfd6e16772d601da5623aef Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Mon, 21 Sep 2026 11:29:04 +0300 Subject: [PATCH 05/10] fix(test): arm smoke runtime cleanup at init, retire the daemon first Review on #2199: cbm_test_runtime_init ran more than a hundred lines before the only EXIT trap, so under `set -e` a failure in the fixture mktemp or its cygpath conversion left the private root behind, and the fixture trap ran smoke_rmtree ahead of the runtime cleanup. Arm `trap 'cbm_test_runtime_cleanup "$BINARY"' EXIT` immediately after a successful init, as soak-test.sh and memlab.sh do. The fixture trap that replaces it now retires the private daemon first and removes the fixtures second, so no earlier cleanup step stands between the exit and the runtime cleanup; on Windows the retirement is also what unblocks the fixture rm. tests/test_smoke_runtime_isolation_contract.sh pins the early trap to the window between the init call and the first fixture; the pin fails against the previous head. Part of #1696. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Anton Standrik --- scripts/smoke-test.sh | 11 +++++++++- .../test_smoke_runtime_isolation_contract.sh | 21 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index e996da8475..d1c64c6b75 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -51,6 +51,11 @@ REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")/.." && pwd)" # shellcheck source=test-runtime.sh source "$REPO_ROOT/scripts/test-runtime.sh" cbm_test_runtime_init +# Armed here rather than only with the fixture trap below: the fixture mktemp +# and its cygpath conversion sit between the two, and under `set -e` a failure +# there would otherwise leave the private root behind. The fixture trap +# replaces this one and keeps the same cleanup as its first step. +trap 'cbm_test_runtime_cleanup "$BINARY"' EXIT smoke_mktemp_file() { if [ -n "${SMOKE_TEMP_ROOT:-}" ]; then @@ -160,7 +165,11 @@ CODEX_LIFECYCLE_HOME="" if command -v cygpath &>/dev/null; then TMPDIR=$(cygpath -m "$TMPDIR") fi -trap 'smoke_rmtree "$TMPDIR" "${DRYRUN_HOME:-}" "${CODEX_LIFECYCLE_HOME:-}"; cbm_test_runtime_cleanup "$BINARY"' EXIT +# Runtime cleanup first, so no earlier cleanup step stands between the exit +# and the private daemon's retirement; on Windows that retirement is also what +# unblocks the fixture rm (mapped binary, open logs). smoke_rmtree never fails, +# so the fixture removal still runs after it. +trap 'cbm_test_runtime_cleanup "$BINARY"; smoke_rmtree "$TMPDIR" "${DRYRUN_HOME:-}" "${CODEX_LIFECYCLE_HOME:-}"' EXIT CLI_STDERR=$(smoke_mktemp_file) # 10 of the cli call sites assign directly (VAR=$(cli ...)). Under diff --git a/tests/test_smoke_runtime_isolation_contract.sh b/tests/test_smoke_runtime_isolation_contract.sh index e91e1d38df..31ba7a7b63 100755 --- a/tests/test_smoke_runtime_isolation_contract.sh +++ b/tests/test_smoke_runtime_isolation_contract.sh @@ -79,4 +79,25 @@ done < "$ENV_LOG" [[ ! -e "$private_root" ]] || fail "smoke-test left its private root behind: $private_root" +# The run above exits through the fixture trap, so it cannot show what happens +# to the private root when the harness dies before that trap exists — the +# fixture mktemp and its cygpath conversion are in that window, and under +# `set -e` either can end the run. Reproducing that failure would mean scanning +# the shared /tmp parent for orphaned roots, which races every other harness +# test in the same suite, so pin the ordering instead: the cleanup trap is +# armed between the init call and the first fixture work. +smoke="$ROOT/scripts/smoke-test.sh" +smoke_line_of() { + # A missing pattern is the failure this check reports, not a reason to end + # the test silently under `set -e`. + grep -n "$1" "$smoke" | head -1 | cut -d: -f1 || true +} +init_line=$(smoke_line_of '^cbm_test_runtime_init$') +early_trap_line=$(smoke_line_of "^trap 'cbm_test_runtime_cleanup \"\$BINARY\"' EXIT\$") +fixture_line=$(smoke_line_of '^TMPDIR=\$(smoke_mktemp_dir)$') +if [[ -z "$init_line" || -z "$early_trap_line" || -z "$fixture_line" ]] || + ((early_trap_line < init_line || early_trap_line > fixture_line)); then + fail "smoke-test must arm the runtime cleanup trap between cbm_test_runtime_init and its first fixture" +fi + echo "PASS: smoke harness isolates its daemon runtime and cache from the caller" From da4bd25a01b612517512ee628bf0ec652000e80a Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 21 Sep 2026 22:46:04 +0200 Subject: [PATCH 06/10] fix(test): the ENOSPC bootstrap test no longer lets a deadline decide its verdict daemon_bootstrap_fails_fast_when_daemon_dies_at_publication went red on the MSan lane of three PRs that touch no daemon code at all (#2245, #2158, #2140) with tests/test_daemon_bootstrap.c:1116: ASSERT(daemon_named_cause) and green on a rerun of the identical commit. Cause, attributed 2026-09-20. The test forks a real daemon host whose record publication fails with ENOSPC, and asserts that the client names that cause instead of waiting out its 30 s startup deadline. But before it listens, the host SHA-256s its own executable image, and the test runner is ~450 MB: ~3 s natively, 20-32 s under MSan (48 CI samples, median 25.8 s). The spawn override returned as soon as it had forked, so the client's 30 s clock raced the host's hash. On a slow run the deadline fired first, the reaper SIGKILLed a host that had not failed yet, and the log had no listen_failed line to find. Nothing was wrong with the product: with a 28 s artificial delay the client does learn the recorded ENOSPC cause. The spawn override now reaps the forked host before it returns. The host writes its start-failure record and only then releases its lifetime reservation and exits, so "record on disk, reservation released" is a stable state pinned by construction when the client takes its first look -- and that first look is guaranteed: the client's post-spawn wait loop runs at least once even past the deadline, and its first failure check after a spawn is unthrottled. How long the host takes no longer matters. No production code changes; the assertions are unchanged. A regression to the pre-#1828 behaviour (a client that ignores the record) still fails the unchanged assertions: its message no longer names the failure, the errno and the path. Proof, one build with temporary env-switched instrumentation that is not part of this commit (a sleep in the forked host before it starts, and a switch back to the old fire-and-forget spawn): spawn forked host delayed daemon_bootstrap suite synchronous no 28 passed old (async) 31 s 27 passed, 1 failed: ASSERT(log_read) synchronous 31 s 28 passed The old spawn fails under the delay one assertion earlier than CI did (log_read, not daemon_named_cause): the injected sleep sits before the host opens its log, whereas in CI the host was killed mid-hash with the log already open. Same mechanism -- the client's deadline beating the host -- and the 2026-09-20 attribution reproduced the CI message itself with a delay placed after the log is opened. The final tree, without the instrumentation: daemon_bootstrap 28 passed, three runs in a row. Signed-off-by: Martin Vogel --- tests/test_daemon_bootstrap.c | 37 ++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/tests/test_daemon_bootstrap.c b/tests/test_daemon_bootstrap.c index 0114d63290..c2243cbeaa 100644 --- a/tests/test_daemon_bootstrap.c +++ b/tests/test_daemon_bootstrap.c @@ -893,6 +893,8 @@ typedef struct { char parent[BOOTSTRAP_TEST_PATH_CAP]; cbm_daemon_build_identity_t identity; pid_t children[BOOTSTRAP_ENOSPC_MAX_CHILDREN]; + int exit_status[BOOTSTRAP_ENOSPC_MAX_CHILDREN]; + bool reaped[BOOTSTRAP_ENOSPC_MAX_CHILDREN]; size_t child_count; size_t spawn_calls; } bootstrap_enospc_host_t; @@ -928,19 +930,40 @@ static bool bootstrap_enospc_host_spawn(void *opaque, int run_result = endpoint ? cbm_daemon_host_run(&config) : 0; _exit(run_result == -1 ? 0 : 50); } - state->children[state->child_count++] = child; + /* SYNCHRONOUS on purpose. The daemon host writes its start-failure record + * and only then lets go of its lifetime reservation and exits, so once it + * has been reaped "record on disk, reservation released" is a STABLE state + * pinned by construction. The client's first look after the spawn (the + * wait loop always runs once and its first failure check is unthrottled) + * therefore finds the record however long the host took to get there. + * + * Returning while the host was still running made the verdict ride on the + * client's 30 s startup deadline: before it listens the host SHA-256s its + * own image, which for the ~450 MB test runner is ~3 s natively and ~26 s + * under MSan (48 CI samples: 20-32 s). The deadline won that race on slow + * runs and the reaper then SIGKILLed a host that had not failed yet -- + * `ASSERT(daemon_named_cause)` red on PRs that touch no C at all (#2245, + * #2158, #2140). A deadline must never decide a test (O9). */ + size_t slot = state->child_count++; + state->children[slot] = child; + int status = 0; + pid_t waited; + do { + waited = waitpid(child, &status, 0); + } while (waited < 0 && errno == EINTR); + state->reaped[slot] = waited == child; + state->exit_status[slot] = status; return true; } static void bootstrap_enospc_reap(bootstrap_enospc_host_t *state, int *nonzero_exits) { *nonzero_exits = 0; for (size_t i = 0; i < state->child_count; i++) { - int status = 0; - pid_t waited; - do { - waited = waitpid(state->children[i], &status, WNOHANG); - } while (waited < 0 && errno == EINTR); - if (waited == 0) { + int status = state->exit_status[i]; + if (!state->reaped[i]) { + /* Only reachable when the synchronous wait in the spawn itself + * failed; never leave a host behind. */ + pid_t waited; (void)kill(state->children[i], SIGKILL); do { waited = waitpid(state->children[i], &status, 0); From f3bdf11a039b8ed8d685b04ddecedaac1e409167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Deust?= Date: Tue, 25 Aug 2026 10:53:40 +0000 Subject: [PATCH 07/10] feat(pipeline): link markdown file references into the graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Markdown docs reference other repo files constantly (a coding-standards doc links the modules it governs, a README points at entry points) but none of that surfaced as edges, so fan-in queries were blind to documentation hubs. In an A/B retrieval benchmark I ran in August on a docs-heavy repo, the graph's top fan-in answer was off by roughly 17x: the most-referenced file was a standards doc with 173 references and zero inbound edges. New pre-dump pass, pass_doclinks.c, modeled on pass_configlink.c: three strategies emit REFERENCES_FILE edges between existing File nodes only (unresolvable targets are dropped, the pass never invents nodes): inline links, backtick paths, bare path mentions. Targets resolve against the referencing file's directory and the repo root; repeated references collapse into one edge carrying strategy, confidence and count. Registered in the pre-dump sequence after configlink and in the incremental post-passes; REFERENCES_FILE added to the skill's edge-type list and the structural/language contract tests. Tests mirror test_configlink.c (real files in a tmpdir, File nodes in a gbuf, run the pass, assert edges): inline link, backtick, bare mention, http/anchor ignored, anchor-suffixed file link, dedupe with count, relative-vs-root resolution, unresolvable-target guard, NULL repo_path skip. A companion change adds the same linking for shell files (source lines and script invocations); split out to keep each change reviewable. Validation: scripts/build.sh clean; focused serial runner (doclinks, configlink, pipeline, edge_structural, lang_contract) 340 passed under ASan/UBSan; cppcheck clean on the new files with the repo's flags. Signed-off-by: Clément Deust Co-Authored-By: Claude Fable 5 --- Makefile.cbm | 3 +- src/cli/cli.c | 4 +- src/pipeline/pass_doclinks.c | 495 ++++++++++++++++++++++++++++ src/pipeline/pipeline.c | 4 + src/pipeline/pipeline_incremental.c | 8 + src/pipeline/pipeline_internal.h | 3 + tests/test_doclinks.c | 312 ++++++++++++++++++ tests/test_edge_structural.c | 1 + tests/test_lang_contract.c | 1 + tests/test_main.c | 4 + 10 files changed, 832 insertions(+), 3 deletions(-) create mode 100644 src/pipeline/pass_doclinks.c create mode 100644 tests/test_doclinks.c diff --git a/Makefile.cbm b/Makefile.cbm index 305cb69372..7a3cd31e16 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -449,6 +449,7 @@ PIPELINE_SRCS = \ src/pipeline/pass_gitdiff.c \ src/pipeline/pass_configures.c \ src/pipeline/pass_configlink.c \ + src/pipeline/pass_doclinks.c \ src/pipeline/pass_route_nodes.c \ src/pipeline/pass_enrichment.c \ src/pipeline/pass_envscan.c \ @@ -675,7 +676,7 @@ TEST_DISCOVER_SRCS = \ TEST_GRAPH_BUFFER_SRCS = tests/test_graph_buffer.c -TEST_PIPELINE_SRCS = tests/test_registry.c tests/test_pipeline.c tests/test_importance.c tests/test_cross_repo.c tests/test_fqn.c tests/test_route_canon.c tests/test_path_alias.c tests/test_configlink.c tests/test_infrascan.c tests/test_worker_pool.c tests/test_parallel.c tests/test_index_resilience.c tests/test_index_format.c tests/test_call_reference_contract.c tests/repro/repro_call_scope_usages.c tests/repro/repro_call_argument_usages.c tests/repro/repro_reference_precision.c tests/repro/repro_lexical_binding_precision.c tests/repro/repro_call_argument_matrix_a.c tests/repro/repro_call_argument_matrix_b.c tests/repro/repro_call_node_behaviors.c tests/repro/repro_language_registry.c tests/repro/repro_call_node_manifest.c tests/repro/repro_lsp_ordered_signatures.c tests/repro/repro_lsp_ordered_local.c tests/repro/repro_ts_overload_return_chains.c tests/repro/repro_harness_cleanup.c tests/repro/repro_runner_filter.c +TEST_PIPELINE_SRCS = tests/test_registry.c tests/test_pipeline.c tests/test_importance.c tests/test_cross_repo.c tests/test_fqn.c tests/test_route_canon.c tests/test_path_alias.c tests/test_configlink.c tests/test_doclinks.c tests/test_infrascan.c tests/test_worker_pool.c tests/test_parallel.c tests/test_index_resilience.c tests/test_index_format.c tests/test_call_reference_contract.c tests/repro/repro_call_scope_usages.c tests/repro/repro_call_argument_usages.c tests/repro/repro_reference_precision.c tests/repro/repro_lexical_binding_precision.c tests/repro/repro_call_argument_matrix_a.c tests/repro/repro_call_argument_matrix_b.c tests/repro/repro_call_node_behaviors.c tests/repro/repro_language_registry.c tests/repro/repro_call_node_manifest.c tests/repro/repro_lsp_ordered_signatures.c tests/repro/repro_lsp_ordered_local.c tests/repro/repro_ts_overload_return_chains.c tests/repro/repro_harness_cleanup.c tests/repro/repro_runner_filter.c TEST_WATCHER_SRCS = tests/test_watcher.c diff --git a/src/cli/cli.c b/src/cli/cli.c index b59a894a6a..b868fdca25 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -1513,8 +1513,8 @@ static const char skill_content[] = "\n" "## Edge Types\n" "CALLS, HTTP_CALLS, ASYNC_CALLS, DATA_FLOWS, IMPORTS, DEFINES, DEFINES_METHOD,\n" - "HANDLES, IMPLEMENTS, OVERRIDE, USAGE, CALL_REFERENCE, CONFIGURES, FILE_CHANGES_WITH,\n" - "SIMILAR_TO, SEMANTICALLY_RELATED, CONTAINS_FILE, CONTAINS_FOLDER,\n" + "HANDLES, IMPLEMENTS, OVERRIDE, USAGE, CALL_REFERENCE, CONFIGURES, REFERENCES_FILE,\n" + "FILE_CHANGES_WITH, SIMILAR_TO, SEMANTICALLY_RELATED, CONTAINS_FILE, CONTAINS_FOLDER,\n" "CONTAINS_PACKAGE\n" "\n" "## Cypher Examples (for query_graph)\n" diff --git a/src/pipeline/pass_doclinks.c b/src/pipeline/pass_doclinks.c new file mode 100644 index 0000000000..d27a22ce0a --- /dev/null +++ b/src/pipeline/pass_doclinks.c @@ -0,0 +1,495 @@ +/* + * pass_doclinks.c — Documentation → file reference linking (pre-dump pass). + * + * Markdown docs reference other repo files constantly — a coding-standards + * doc links to the modules it governs, a README points at entry points — + * but none of that surfaced as graph edges, so fan-in queries were blind to + * documentation hubs: on a docs-heavy repo the top fan-in answer was off by + * an order of magnitude because the most-referenced doc had zero inbound + * edges. + * + * Three strategies emit REFERENCES_FILE edges between EXISTING File nodes + * (targets that don't resolve to an indexed file are dropped — the pass + * never invents nodes): + * MD 1. Inline link: [text](relative/path.ext) (not http/mailto/#anchor) + * MD 2. Backtick path: `path/with/slash.ext` or `file.ext` + * MD 3. Bare mention: relative/path.ext (slash + extension) + * + * Targets resolve relative to the referencing file's directory AND the repo + * root (docs are written both ways). Repeated references between the same + * file pair are collapsed to one edge carrying a "count" property; the edge + * keeps the highest-confidence strategy that matched. + * + * Operates on the graph buffer before dump to .db file. + */ +#include "pipeline/pipeline.h" +#include "pipeline/pipeline_internal.h" +#include "graph_buffer/graph_buffer.h" +#include "foundation/constants.h" +#include "foundation/hash_table.h" +#include "foundation/log.h" +#include "foundation/compat_fs.h" +#include "foundation/limits.h" + +#include +#include +#include +#include +#include +#include + +#define SLEN(s) (sizeof(s) - SKIP_ONE) + +/* ── Doc link confidence scores ──────────────────────────────────── */ +/* Markdown strategies */ +#define DOCLINK_MD_INLINE 0.95 +#define DOCLINK_MD_BACKTICK 0.85 +#define DOCLINK_MD_BARE 0.70 + +/* Edge type emitted by this pass. */ +#define DOCLINK_EDGE_TYPE "REFERENCES_FILE" + +enum { + DOCLINK_MAX_REFS = CBM_SZ_256, /* distinct targets per referencing file */ + DOCLINK_MAX_SEGS = CBM_SZ_64, /* path segments during normalization */ +}; + +/* ── Path classification ─────────────────────────────────────────── */ + +/* Extension of the path's basename (including the dot), or NULL. */ +static const char *doclink_path_ext(const char *path) { + if (!path) { + return NULL; + } + const char *base = strrchr(path, '/'); + base = base ? base + SKIP_ONE : path; + return strrchr(base, '.'); +} + +static bool doclink_is_markdown_path(const char *path) { + const char *ext = doclink_path_ext(path); + return ext && (strcmp(ext, ".md") == 0 || strcmp(ext, ".mdx") == 0); +} + +/* ── File reading (mirrors pass_semantic.c read_file, minus TS pad) ── */ + +static char *doclink_read_file(const char *path) { + FILE *f = cbm_fopen(path, "rb"); + if (!f) { + return NULL; + } + (void)fseek(f, 0, SEEK_END); + long size = ftell(f); + (void)fseek(f, 0, SEEK_SET); + if (size <= 0 || size > cbm_max_file_bytes()) { + (void)fclose(f); + return NULL; + } + char *buf = malloc((size_t)size + SKIP_ONE); + if (!buf) { + (void)fclose(f); + return NULL; + } + size_t nread = fread(buf, SKIP_ONE, (size_t)size, f); + (void)fclose(f); + if (nread > (size_t)size) { + nread = (size_t)size; + } + buf[nread] = '\0'; + return buf; +} + +/* ── Path normalization + resolution ─────────────────────────────── */ + +/* Normalize "a/./b/../c" into "a/c". Rejects paths that escape the repo + * root (leading ".."), empty results, and over-long/over-deep inputs. */ +static bool doclink_normalize(const char *in, char *out, size_t out_sz) { + size_t seg_starts[DOCLINK_MAX_SEGS]; + int depth = 0; + size_t out_len = 0; + const char *p = in; + out[0] = '\0'; + while (*p) { + const char *seg = p; + const char *slash = strchr(p, '/'); + size_t seg_len = slash ? (size_t)(slash - p) : strlen(p); + p = slash ? slash + SKIP_ONE : p + seg_len; + if (seg_len == 0 || (seg_len == SKIP_ONE && seg[0] == '.')) { + continue; + } + if (seg_len == PAIR_LEN && seg[0] == '.' && seg[SKIP_ONE] == '.') { + if (depth == 0) { + return false; /* escapes the repo root */ + } + depth--; + out_len = seg_starts[depth]; + out[out_len] = '\0'; + continue; + } + if (depth >= DOCLINK_MAX_SEGS || out_len + seg_len + PAIR_LEN >= out_sz) { + return false; + } + seg_starts[depth++] = out_len; + if (out_len > 0) { + out[out_len++] = '/'; + } + memcpy(out + out_len, seg, seg_len); + out_len += seg_len; + out[out_len] = '\0'; + } + return out_len > 0; +} + +/* Per-file accumulator: dedupes repeated references to the same target. */ +typedef struct { + int64_t target_id; + int count; + double confidence; + const char *strategy; /* static string literal */ +} doclink_ref_t; + +typedef struct { + cbm_gbuf_t *gb; + CBMHashTable *files_by_path; /* rel_path → cbm_gbuf_node_t* (borrowed) */ + const cbm_gbuf_node_t *src; /* referencing File node */ + char src_dir[CBM_SZ_512]; /* its directory ("" at repo root) */ + doclink_ref_t refs[DOCLINK_MAX_REFS]; + int ref_count; + bool truncated; +} doclink_ctx_t; + +/* Resolve a reference against the referencing file's directory, then the + * repo root, mirroring how humans write doc links. Returns the already- + * indexed File node or NULL — unresolvable references are dropped. */ +static const cbm_gbuf_node_t *doclink_resolve(doclink_ctx_t *dc, const char *ref) { + const char *r = ref; + while (r[0] == '.' && r[SKIP_ONE] == '/') { + r += PAIR_LEN; + } + if (r[0] == '/') { + r++; /* "/docs/x.md" is repo-root-relative by doc convention */ + } + if (r[0] == '\0') { + return NULL; + } + + char norm[CBM_SZ_512]; + if (dc->src_dir[0] != '\0') { + char joined[CBM_SZ_512]; + int n = snprintf(joined, sizeof(joined), "%s/%s", dc->src_dir, r); + if (n > 0 && (size_t)n < sizeof(joined) && doclink_normalize(joined, norm, sizeof(norm))) { + const cbm_gbuf_node_t *node = cbm_ht_get(dc->files_by_path, norm); + if (node) { + return node; + } + } + } + if (doclink_normalize(r, norm, sizeof(norm))) { + return cbm_ht_get(dc->files_by_path, norm); + } + return NULL; +} + +/* Record one match. Same-pair repeats bump the count; a higher-confidence + * strategy upgrades the edge's confidence + strategy label. */ +static void doclink_record(doclink_ctx_t *dc, const cbm_gbuf_node_t *target, double confidence, + const char *strategy) { + if (!target || target->id == dc->src->id) { + return; /* never self-reference */ + } + for (int i = 0; i < dc->ref_count; i++) { + if (dc->refs[i].target_id == target->id) { + dc->refs[i].count++; + if (confidence > dc->refs[i].confidence) { + dc->refs[i].confidence = confidence; + dc->refs[i].strategy = strategy; + } + return; + } + } + if (dc->ref_count >= DOCLINK_MAX_REFS) { + dc->truncated = true; + return; + } + dc->refs[dc->ref_count].target_id = target->id; + dc->refs[dc->ref_count].count = SKIP_ONE; + dc->refs[dc->ref_count].confidence = confidence; + dc->refs[dc->ref_count].strategy = strategy; + dc->ref_count++; +} + +/* Emit accumulated references as REFERENCES_FILE edges. Returns edge count. */ +static int doclink_flush(doclink_ctx_t *dc) { + int emitted = 0; + for (int i = 0; i < dc->ref_count; i++) { + char props[CBM_SZ_256]; + (void)snprintf(props, sizeof(props), + "{\"strategy\":\"%s\",\"confidence\":%.2f,\"count\":%d}", + dc->refs[i].strategy, dc->refs[i].confidence, dc->refs[i].count); + if (cbm_gbuf_insert_edge(dc->gb, dc->src->id, dc->refs[i].target_id, DOCLINK_EDGE_TYPE, + props) > 0) { + emitted++; + } + } + if (dc->truncated) { + char cap_buf[CBM_SZ_16]; + (void)snprintf(cap_buf, sizeof(cap_buf), "%d", DOCLINK_MAX_REFS); + cbm_log_info("doclinks.truncated", "file", dc->src->file_path ? dc->src->file_path : "", + "cap", cap_buf); + } + dc->ref_count = 0; + dc->truncated = false; + return emitted; +} + +/* ── Markdown scanning ───────────────────────────────────────────── */ + +/* Characters allowed in a path-shaped token (backtick / bare mention). */ +static bool doclink_token_pathlike(const char *tok) { + bool last_seg_has_dot = false; + for (const char *p = tok; *p; p++) { + unsigned char c = (unsigned char)*p; + if (c == '/') { + last_seg_has_dot = false; + continue; + } + if (c == '.') { + last_seg_has_dot = true; + continue; + } + if (!isalnum(c) && c != '_' && c != '-' && c != '+' && c != '@' && c != '~') { + return false; + } + } + /* the basename must carry an extension — bare words are not paths */ + return last_seg_has_dot; +} + +/* A markdown link target worth resolving: not a URL, mailto, or pure anchor. */ +static bool doclink_md_target_ok(const char *target) { + if (target[0] == '\0' || target[0] == '#') { + return false; + } + if (strstr(target, "://") != NULL || strncmp(target, "mailto:", SLEN("mailto:")) == 0) { + return false; + } + return true; +} + +/* MD 1: inline links [text](target). Consumed spans are blanked so the + * backtick / bare-mention scans below cannot re-match the same path. */ +static void doclink_scan_md_links(doclink_ctx_t *dc, char *line) { + char *p = line; + while ((p = strstr(p, "](")) != NULL) { + char *close = strchr(p + PAIR_LEN, ')'); + if (!close) { + return; + } + char target[CBM_SZ_512]; + size_t tlen = (size_t)(close - (p + PAIR_LEN)); + if (tlen < sizeof(target)) { + memcpy(target, p + PAIR_LEN, tlen); + target[tlen] = '\0'; + char *cut = strchr(target, ' '); /* [t](path "title") */ + if (cut) { + *cut = '\0'; + } + cut = strchr(target, '#'); /* [t](path#anchor) */ + if (cut) { + *cut = '\0'; + } + if (doclink_md_target_ok(target)) { + doclink_record(dc, doclink_resolve(dc, target), DOCLINK_MD_INLINE, + "md_inline_link"); + } + } + /* blank the whole [text](target) span, link text included, so a + * path-shaped link text is not re-counted as a bare mention */ + char *open = p; + while (open > line && *open != '[') { + open--; + } + if (*open != '[') { + open = p; + } + memset(open, ' ', (size_t)(close - open) + SKIP_ONE); + p = close + SKIP_ONE; + } +} + +/* MD 2: backtick-quoted paths `src/foo.c` / `build.sh`. */ +static void doclink_scan_md_backticks(doclink_ctx_t *dc, char *line) { + char *p = line; + while ((p = strchr(p, '`')) != NULL) { + char *end = strchr(p + SKIP_ONE, '`'); + if (!end) { + return; + } + char tok[CBM_SZ_512]; + size_t tlen = (size_t)(end - (p + SKIP_ONE)); + if (tlen > 0 && tlen < sizeof(tok)) { + memcpy(tok, p + SKIP_ONE, tlen); + tok[tlen] = '\0'; + if (doclink_token_pathlike(tok)) { + doclink_record(dc, doclink_resolve(dc, tok), DOCLINK_MD_BACKTICK, + "md_backtick_path"); + } + } + memset(p, ' ', (size_t)(end - p) + SKIP_ONE); + p = end + SKIP_ONE; + } +} + +static bool doclink_md_delim(char c) { + return isspace((unsigned char)c) || strchr("()[]{}<>\"',;:`*|", c) != NULL; +} + +/* MD 3: bare relative path mentions — must contain a slash AND an extension + * (and, via doclink_resolve, an indexed file) to count. */ +static void doclink_scan_md_bare(doclink_ctx_t *dc, const char *line) { + const char *p = line; + while (*p) { + while (*p && doclink_md_delim(*p)) { + p++; + } + const char *start = p; + while (*p && !doclink_md_delim(*p)) { + p++; + } + size_t tlen = (size_t)(p - start); + char tok[CBM_SZ_512]; + if (tlen == 0 || tlen >= sizeof(tok)) { + continue; + } + memcpy(tok, start, tlen); + tok[tlen] = '\0'; + while (tlen > 0 && tok[tlen - SKIP_ONE] == '.') { + tok[--tlen] = '\0'; /* sentence-ending period */ + } + if (strchr(tok, '/') != NULL && doclink_token_pathlike(tok)) { + doclink_record(dc, doclink_resolve(dc, tok), DOCLINK_MD_BARE, "md_bare_mention"); + } + } +} + +static void doclink_scan_md_line(doclink_ctx_t *dc, char *line) { + doclink_scan_md_links(dc, line); + doclink_scan_md_backticks(dc, line); + doclink_scan_md_bare(dc, line); +} + +/* ── Per-file driver ─────────────────────────────────────────────── */ + +/* Scan one referencing file's content line by line and emit its edges. */ +static int doclink_scan_file(doclink_ctx_t *dc, const cbm_gbuf_node_t *node, const char *source) { + dc->src = node; + dc->ref_count = 0; + dc->truncated = false; + dc->src_dir[0] = '\0'; + const char *slash = strrchr(node->file_path, '/'); + if (slash) { + size_t dlen = (size_t)(slash - node->file_path); + if (dlen >= sizeof(dc->src_dir)) { + return 0; + } + memcpy(dc->src_dir, node->file_path, dlen); + dc->src_dir[dlen] = '\0'; + } + + const char *p = source; + char line[CBM_SZ_4K]; + while (*p) { + const char *eol = strchr(p, '\n'); + size_t line_len = eol ? (size_t)(eol - p) : strlen(p); + if (line_len >= sizeof(line)) { + line_len = sizeof(line) - SKIP_ONE; + } + memcpy(line, p, line_len); + line[line_len] = '\0'; + p = eol ? eol + SKIP_ONE : p + line_len; + + doclink_scan_md_line(dc, line); + } + return doclink_flush(dc); +} + +/* ── Pass entry point ────────────────────────────────────────────── */ + +/* True when at least one File node is a markdown file. */ +static bool doclink_has_doc_files(const cbm_gbuf_node_t *const *files, int file_count) { + for (int i = 0; i < file_count; i++) { + if (doclink_is_markdown_path(files[i]->file_path)) { + return true; + } + } + return false; +} + +/* Scan every markdown File node's on-disk content, emitting edges. + * md_edges receives the emitted edge count. */ +static void doclink_scan_repo(doclink_ctx_t *dc, const char *repo_path, + const cbm_gbuf_node_t *const *files, int file_count, int *md_edges) { + for (int i = 0; i < file_count; i++) { + if (!files[i]->file_path || !doclink_is_markdown_path(files[i]->file_path)) { + continue; + } + + char abs_path[CBM_PATH_MAX]; + int n = snprintf(abs_path, sizeof(abs_path), "%s/%s", repo_path, files[i]->file_path); + if (n <= 0 || (size_t)n >= sizeof(abs_path)) { + continue; + } + char *source = doclink_read_file(abs_path); + if (!source) { + continue; + } + int emitted = doclink_scan_file(dc, files[i], source); + free(source); + *md_edges += emitted; + } +} + +int cbm_pipeline_pass_doclinks(cbm_pipeline_ctx_t *ctx) { + cbm_gbuf_t *gb = ctx->gbuf; + + const cbm_gbuf_node_t **files = NULL; + int file_count = 0; + if (cbm_gbuf_find_by_label(gb, "File", &files, &file_count) != 0 || file_count == 0) { + return 0; + } + + /* Early exit: no markdown/shell files means nothing to scan. */ + if (!doclink_has_doc_files(files, file_count)) { + cbm_log_info("doclinks.skip", "reason", "no_doc_files"); + return 0; + } + if (!ctx->repo_path) { + cbm_log_info("doclinks.skip", "reason", "no_repo_path"); + return 0; + } + + doclink_ctx_t dc; + memset(&dc, 0, sizeof(dc)); + dc.gb = gb; + dc.files_by_path = cbm_ht_create((uint32_t)file_count); + if (!dc.files_by_path) { + return 0; + } + for (int i = 0; i < file_count; i++) { + if (files[i]->file_path) { + /* key borrowed from the node (owned by gbuf, outlives the pass) */ + cbm_ht_set(dc.files_by_path, files[i]->file_path, (void *)files[i]); + } + } + + int md_edges = 0; + doclink_scan_repo(&dc, ctx->repo_path, files, file_count, &md_edges); + cbm_ht_free(dc.files_by_path); + + char buf1[CBM_SZ_16]; + (void)snprintf(buf1, sizeof(buf1), "%d", md_edges); + cbm_log_info("doclinks.strategy", "name", "markdown", "edges", buf1); + cbm_log_info("doclinks.done", "total", buf1); + + return md_edges; +} diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index 6bc0310dd5..fa62441a11 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -1054,6 +1054,9 @@ static void predump_sem(cbm_pipeline_ctx_t *ctx) { static void predump_cfg(cbm_pipeline_ctx_t *ctx) { cbm_pipeline_pass_configlink(ctx); } +static void predump_doclinks(cbm_pipeline_ctx_t *ctx) { + cbm_pipeline_pass_doclinks(ctx); +} static void predump_complexity(cbm_pipeline_ctx_t *ctx) { cbm_pipeline_pass_complexity(ctx); } @@ -1288,6 +1291,7 @@ static void run_predump_passes(cbm_pipeline_t *p, cbm_pipeline_ctx_t *ctx) { } passes[] = { {predump_deco, "decorator_tags", false}, {predump_cfg, "configlink", false}, + {predump_doclinks, "doclinks", false}, {predump_route, "route_match", false}, {predump_ensemble, "ensemble_routing", false}, {predump_sim, "similarity", true}, diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 894b732572..154518658d 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -1446,6 +1446,14 @@ static int run_postpasses(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed_file return rc < 0 ? rc : CBM_NOT_FOUND; } + cbm_clock_gettime(CLOCK_MONOTONIC, &t); + rc = cbm_pipeline_pass_doclinks(ctx); + cbm_log_info("pass.timing", "pass", "incr_doclinks", "elapsed_ms", + itoa_buf((int)elapsed_ms(t))); + if (rc < 0 || cbm_pipeline_check_cancel(ctx)) { + return rc < 0 ? rc : CBM_NOT_FOUND; + } + /* SIMILAR_TO + SEMANTICALLY_RELATED edges only in moderate/full modes */ if (ctx->mode <= CBM_MODE_MODERATE) { cbm_clock_gettime(CLOCK_MONOTONIC, &t); diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 78ce486a0f..2750947317 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -665,6 +665,9 @@ int cbm_pipeline_pass_decorator_tags(cbm_gbuf_t *gbuf, const char *project); /* Pre-dump pass: config ↔ code linking. */ int cbm_pipeline_pass_configlink(cbm_pipeline_ctx_t *ctx); +/* Pre-dump pass: markdown/shell → file REFERENCES_FILE linking. */ +int cbm_pipeline_pass_doclinks(cbm_pipeline_ctx_t *ctx); + /* Pre-dump pass: SIMILAR_TO edges via MinHash fingerprinting. */ int cbm_pipeline_pass_similarity(cbm_pipeline_ctx_t *ctx); diff --git a/tests/test_doclinks.c b/tests/test_doclinks.c new file mode 100644 index 0000000000..a03542d9b4 --- /dev/null +++ b/tests/test_doclinks.c @@ -0,0 +1,312 @@ +/* + * test_doclinks.c — Tests for markdown/shell → file reference linking. + * + * Unit-test approach mirrors test_configlink.c: create real files in a + * tmpdir (the pass reads content from disk), set up File nodes in a gbuf, + * run the pass, check REFERENCES_FILE edges. + */ +#include "../src/foundation/compat.h" +#include "test_framework.h" +#include "test_helpers.h" +#include "pipeline/pipeline.h" +#include "pipeline/pipeline_internal.h" +#include "graph_buffer/graph_buffer.h" + +#include +#include +#include +#include + +/* ── Helpers ─────────────────────────────────────────────────────── */ + +/* Run the pass with a minimal ctx (same shape as run_configlink). */ +static int run_doclinks(cbm_gbuf_t *gb, const char *project, const char *repo_path) { + atomic_int cancelled; + atomic_init(&cancelled, 0); + cbm_pipeline_ctx_t ctx = { + .project_name = project, + .repo_path = repo_path, + .gbuf = gb, + .cancelled = &cancelled, + }; + return cbm_pipeline_pass_doclinks(&ctx); +} + +/* Create a File node the way pass_structure does: QN via fqn_compute with + * "__file__", file_path = repo-relative path. Returns the node id. */ +static int64_t add_file_node(cbm_gbuf_t *gb, const char *project, const char *rel) { + char *qn = cbm_pipeline_fqn_compute(project, rel, "__file__"); + const char *slash = strrchr(rel, '/'); + const char *basename = slash ? slash + 1 : rel; + int64_t id = cbm_gbuf_upsert_node(gb, "File", basename, qn, rel, 0, 0, NULL); + free(qn); + return id; +} + +/* Find the REFERENCES_FILE edge between two node ids. NULL if absent. */ +static const cbm_gbuf_edge_t *find_ref_edge(cbm_gbuf_t *gb, int64_t src, int64_t tgt) { + const cbm_gbuf_edge_t **edges = NULL; + int count = 0; + cbm_gbuf_find_edges_by_type(gb, "REFERENCES_FILE", &edges, &count); + for (int i = 0; i < count; i++) { + if (edges[i]->source_id == src && edges[i]->target_id == tgt) { + return edges[i]; + } + } + return NULL; +} + +/* True when the edge's props JSON carries the given strategy. */ +static bool edge_has_strategy(const cbm_gbuf_edge_t *e, const char *strategy) { + char needle[64]; + snprintf(needle, sizeof(needle), "\"strategy\":\"%s\"", strategy); + return e && e->properties_json && strstr(e->properties_json, needle) != NULL; +} + +/* Total REFERENCES_FILE edge count. */ +static int ref_edge_count(cbm_gbuf_t *gb) { + return cbm_gbuf_edge_count_by_type(gb, "REFERENCES_FILE"); +} + +/* Fixture: tmpdir + project + gbuf. */ +typedef struct { + char tmpdir[256]; + char *project; + cbm_gbuf_t *gb; +} dl_fix_t; + +static bool dl_fix_init(dl_fix_t *fx) { + snprintf(fx->tmpdir, sizeof(fx->tmpdir), "/tmp/cbm_doclinks_XXXXXX"); + if (!cbm_mkdtemp(fx->tmpdir)) { + return false; + } + fx->project = cbm_project_name_from_path(fx->tmpdir); + fx->gb = cbm_gbuf_new(fx->project, fx->tmpdir); + return fx->gb != NULL; +} + +static void dl_fix_free(dl_fix_t *fx) { + cbm_gbuf_free(fx->gb); + free(fx->project); + th_rmtree(fx->tmpdir); +} + +/* ── Markdown: inline link ───────────────────────────────────────── */ + +TEST(doclinks_md_inline_link) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "docs/guide.md"), + "# Guide\n\nSee [the build script](../scripts/build.sh) for details.\n"); + th_write_file(TH_PATH(fx.tmpdir, "scripts/build.sh"), "#!/bin/sh\necho build\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "docs/guide.md"); + int64_t script_id = add_file_node(fx.gb, fx.project, "scripts/build.sh"); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_GT(n, 0); + + const cbm_gbuf_edge_t *e = find_ref_edge(fx.gb, doc_id, script_id); + ASSERT_NOT_NULL(e); + ASSERT_TRUE(edge_has_strategy(e, "md_inline_link")); + ASSERT_NOT_NULL(strstr(e->properties_json, "\"confidence\":0.95")); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Markdown: backtick path ─────────────────────────────────────── */ + +TEST(doclinks_md_backtick_path) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), + "Run `scripts/build.sh` before pushing. `not_a_file.xyz` is unknown.\n"); + th_write_file(TH_PATH(fx.tmpdir, "scripts/build.sh"), "#!/bin/sh\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "README.md"); + int64_t script_id = add_file_node(fx.gb, fx.project, "scripts/build.sh"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + + const cbm_gbuf_edge_t *e = find_ref_edge(fx.gb, doc_id, script_id); + ASSERT_NOT_NULL(e); + ASSERT_TRUE(edge_has_strategy(e, "md_backtick_path")); + /* `not_a_file.xyz` has no File node → no invented target */ + ASSERT_EQ(ref_edge_count(fx.gb), 1); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Markdown: bare mention ──────────────────────────────────────── */ + +TEST(doclinks_md_bare_mention) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "STANDARDS.md"), + "All handlers live in src/handlers.c and follow the pattern there.\n"); + th_write_file(TH_PATH(fx.tmpdir, "src/handlers.c"), "int h(void) { return 0; }\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "STANDARDS.md"); + int64_t code_id = add_file_node(fx.gb, fx.project, "src/handlers.c"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + + const cbm_gbuf_edge_t *e = find_ref_edge(fx.gb, doc_id, code_id); + ASSERT_NOT_NULL(e); + ASSERT_TRUE(edge_has_strategy(e, "md_bare_mention")); + ASSERT_NOT_NULL(strstr(e->properties_json, "\"confidence\":0.70")); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Markdown: non-file links ignored (http / mailto / #anchor) ──── */ + +TEST(doclinks_md_non_file_link_ignored) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), + "See [the site](https://example.com/scripts/build.sh) or\n" + "[mail us](mailto:dev@example.com) or [below](#usage).\n"); + th_write_file(TH_PATH(fx.tmpdir, "scripts/build.sh"), "#!/bin/sh\n"); + + add_file_node(fx.gb, fx.project, "README.md"); + add_file_node(fx.gb, fx.project, "scripts/build.sh"); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_EQ(n, 0); + ASSERT_EQ(ref_edge_count(fx.gb), 0); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Markdown: anchor suffix on a file link still resolves ───────── */ + +TEST(doclinks_md_link_with_anchor_resolves_file) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "docs/a.md"), "See [setup](../INSTALL.md#quick-start).\n"); + th_write_file(TH_PATH(fx.tmpdir, "INSTALL.md"), "# Install\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "docs/a.md"); + int64_t tgt_id = add_file_node(fx.gb, fx.project, "INSTALL.md"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_NOT_NULL(find_ref_edge(fx.gb, doc_id, tgt_id)); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Dedupe: repeated references collapse to one counted edge ────── */ + +TEST(doclinks_dedupe_keeps_count) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), + "Use [build](scripts/build.sh) daily.\n" + "Run `scripts/build.sh` first, then scripts/build.sh again.\n"); + th_write_file(TH_PATH(fx.tmpdir, "scripts/build.sh"), "#!/bin/sh\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "README.md"); + int64_t script_id = add_file_node(fx.gb, fx.project, "scripts/build.sh"); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + /* three matches, ONE edge */ + ASSERT_EQ(n, 1); + ASSERT_EQ(ref_edge_count(fx.gb), 1); + + const cbm_gbuf_edge_t *e = find_ref_edge(fx.gb, doc_id, script_id); + ASSERT_NOT_NULL(e); + ASSERT_NOT_NULL(strstr(e->properties_json, "\"count\":3")); + /* highest-confidence match kind wins the edge label */ + ASSERT_TRUE(edge_has_strategy(e, "md_inline_link")); + ASSERT_NOT_NULL(strstr(e->properties_json, "\"confidence\":0.95")); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Path resolution: relative to the referencing file's directory ── */ + +TEST(doclinks_resolves_relative_to_file_dir) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + /* docs/deep/page.md links an upward "../../src/main.c" path and a + * sibling-relative "notes.md" path, neither repo-root-relative. */ + th_write_file(TH_PATH(fx.tmpdir, "docs/deep/page.md"), + "See [main](../../src/main.c) and [notes](notes.md).\n"); + th_write_file(TH_PATH(fx.tmpdir, "docs/deep/notes.md"), "# notes\n"); + th_write_file(TH_PATH(fx.tmpdir, "src/main.c"), "int main(void) { return 0; }\n"); + + int64_t page_id = add_file_node(fx.gb, fx.project, "docs/deep/page.md"); + int64_t notes_id = add_file_node(fx.gb, fx.project, "docs/deep/notes.md"); + int64_t main_id = add_file_node(fx.gb, fx.project, "src/main.c"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + + ASSERT_NOT_NULL(find_ref_edge(fx.gb, page_id, main_id)); + ASSERT_NOT_NULL(find_ref_edge(fx.gb, page_id, notes_id)); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Never invent nodes: unresolvable targets produce nothing ────── */ + +TEST(doclinks_unresolvable_target_no_edge) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), + "See [gone](docs/removed.md) and `also/missing.sh`.\n"); + + add_file_node(fx.gb, fx.project, "README.md"); + + int node_count_before = cbm_gbuf_node_count(fx.gb); + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_EQ(n, 0); + ASSERT_EQ(ref_edge_count(fx.gb), 0); + ASSERT_EQ(cbm_gbuf_node_count(fx.gb), node_count_before); + + dl_fix_free(&fx); + PASS(); +} + +/* ── NULL repo_path (configlink-style unit setups) is a clean skip ── */ + +TEST(doclinks_null_repo_path_skips) { + cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp/test"); + add_file_node(gb, "test", "README.md"); + int n = run_doclinks(gb, "test", NULL); + ASSERT_EQ(n, 0); + cbm_gbuf_free(gb); + PASS(); +} + +/* ── Suite ───────────────────────────────────────────────────────── */ + +SUITE(doclinks) { + /* Markdown strategies */ + RUN_TEST(doclinks_md_inline_link); + RUN_TEST(doclinks_md_backtick_path); + RUN_TEST(doclinks_md_bare_mention); + RUN_TEST(doclinks_md_non_file_link_ignored); + RUN_TEST(doclinks_md_link_with_anchor_resolves_file); + + /* Dedupe + resolution + guards */ + RUN_TEST(doclinks_dedupe_keeps_count); + RUN_TEST(doclinks_resolves_relative_to_file_dir); + RUN_TEST(doclinks_unresolvable_target_no_edge); + RUN_TEST(doclinks_null_repo_path_skips); +} diff --git a/tests/test_edge_structural.c b/tests/test_edge_structural.c index 3f43628fa9..798fef3d08 100644 --- a/tests/test_edge_structural.c +++ b/tests/test_edge_structural.c @@ -258,6 +258,7 @@ static const char *ES_ALL_EDGE_TYPES[] = {"CALLS", "INHERITS", "INFRA_MAPS", "OVERRIDE", + "REFERENCES_FILE", "SEMANTICALLY_RELATED", "SIMILAR_TO", "TESTS_FILE", diff --git a/tests/test_lang_contract.c b/tests/test_lang_contract.c index 5a32bcda05..6eb993564e 100644 --- a/tests/test_lang_contract.c +++ b/tests/test_lang_contract.c @@ -1070,6 +1070,7 @@ static const char *ALL_EDGE_TYPES[] = {"CALLS", "INHERITS", "INFRA_MAPS", "OVERRIDE", + "REFERENCES_FILE", "SEMANTICALLY_RELATED", "SIMILAR_TO", "TESTS_FILE", diff --git a/tests/test_main.c b/tests/test_main.c index 31cceac608..1278c1c70b 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -852,6 +852,7 @@ extern void suite_store_pragmas(void); extern void suite_store_checkpoint(void); extern void suite_traces(void); extern void suite_configlink(void); +extern void suite_doclinks(void); extern void suite_infrascan(void); extern void suite_cli(void); extern void suite_agent_clients(void); @@ -1181,6 +1182,9 @@ int main(int argc, char **argv) { /* Config link */ RUN_SELECTED_SUITE(configlink); + /* Doc/shell file reference link */ + RUN_SELECTED_SUITE(doclinks); + /* Infrastructure scanning */ RUN_SELECTED_SUITE(infrascan); From 6ae6cd4c3f280cb8e3ffec2858269055e390ffec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Deust?= Date: Tue, 22 Sep 2026 08:48:33 +0200 Subject: [PATCH 08/10] fix(pipeline): address doclinks review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace raw malloc/free with cbm_alloc/cbm_free (CBM_MEM_CLASS_EXTRACT) and a growable CBM_DYN_ARRAY for matches. Remove the 4KiB line-copy cap; lines are split in place instead of copied. Remove the DOCLINK_MAX_REFS(256) cap; the ref vector now grows, is sorted by target id, then collapsed. Track reference kind (bare, relative, rooted) so /x never attempts a directory join and ./x never falls back to root. Report per-file skips via cbm_pipeline_add_file_error and cbm_log_warn, and check cbm_pipeline_check_cancel in the per-file loop. Add negative-control tests for all of the above, plus CRLF handling, fenced-code-block pinning, bare URLs, backtick-basename exactness, and ".." escaping. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Clément Deust --- src/pipeline/pass_doclinks.c | 231 +++++++++++++++++++---------- src/pipeline/pipeline_internal.h | 2 +- tests/test_doclinks.c | 246 ++++++++++++++++++++++++++++++- tests/test_main.c | 2 +- tests/test_pipeline.c | 31 ++++ 5 files changed, 432 insertions(+), 80 deletions(-) diff --git a/src/pipeline/pass_doclinks.c b/src/pipeline/pass_doclinks.c index d27a22ce0a..4b75357e50 100644 --- a/src/pipeline/pass_doclinks.c +++ b/src/pipeline/pass_doclinks.c @@ -30,6 +30,8 @@ #include "foundation/log.h" #include "foundation/compat_fs.h" #include "foundation/limits.h" +#include "foundation/mem_core.h" +#include "foundation/dyn_array.h" #include #include @@ -50,8 +52,7 @@ #define DOCLINK_EDGE_TYPE "REFERENCES_FILE" enum { - DOCLINK_MAX_REFS = CBM_SZ_256, /* distinct targets per referencing file */ - DOCLINK_MAX_SEGS = CBM_SZ_64, /* path segments during normalization */ + DOCLINK_MAX_SEGS = CBM_SZ_64, /* path segments during normalization */ }; /* ── Path classification ─────────────────────────────────────────── */ @@ -71,23 +72,51 @@ static bool doclink_is_markdown_path(const char *path) { return ext && (strcmp(ext, ".md") == 0 || strcmp(ext, ".mdx") == 0); } -/* ── File reading (mirrors pass_semantic.c read_file, minus TS pad) ── */ - -static char *doclink_read_file(const char *path) { +/* ── File reading (mirrors pass_definitions.c read_file, minus TS pad) ── + * + * Reports *out_status so the caller can attribute a skip to the right + * reason (open failure vs. oversized vs. OOM) instead of a silent drop — + * hitting a limit here degrades to a REPORTED skip, per limits.h. */ +static char *doclink_read_file(const char *path, long *out_size, cbm_read_status_t *out_status) { + if (out_size) { + *out_size = 0; + } + if (out_status) { + *out_status = CBM_READ_OK; + } FILE *f = cbm_fopen(path, "rb"); if (!f) { + if (out_status) { + *out_status = CBM_READ_OPEN_FAIL; + } return NULL; } (void)fseek(f, 0, SEEK_END); long size = ftell(f); (void)fseek(f, 0, SEEK_SET); - if (size <= 0 || size > cbm_max_file_bytes()) { + if (out_size) { + *out_size = size; + } + if (size <= 0) { + (void)fclose(f); + if (out_status) { + *out_status = CBM_READ_EMPTY; + } + return NULL; + } + if (size > cbm_max_file_bytes()) { (void)fclose(f); + if (out_status) { + *out_status = CBM_READ_OVERSIZED; + } return NULL; } - char *buf = malloc((size_t)size + SKIP_ONE); + char *buf = cbm_alloc(CBM_MEM_CLASS_EXTRACT, (size_t)size + SKIP_ONE); if (!buf) { (void)fclose(f); + if (out_status) { + *out_status = CBM_READ_OOM; + } return NULL; } size_t nread = fread(buf, SKIP_ONE, (size_t)size, f); @@ -140,33 +169,45 @@ static bool doclink_normalize(const char *in, char *out, size_t out_sz) { return out_len > 0; } -/* Per-file accumulator: dedupes repeated references to the same target. */ +/* One raw match before dedup. Collected in a growable vector (no per-file + * cap) and collapsed by doclink_flush(). */ typedef struct { int64_t target_id; - int count; double confidence; const char *strategy; /* static string literal */ -} doclink_ref_t; +} doclink_match_t; typedef struct { cbm_gbuf_t *gb; CBMHashTable *files_by_path; /* rel_path → cbm_gbuf_node_t* (borrowed) */ const cbm_gbuf_node_t *src; /* referencing File node */ char src_dir[CBM_SZ_512]; /* its directory ("" at repo root) */ - doclink_ref_t refs[DOCLINK_MAX_REFS]; - int ref_count; - bool truncated; + CBM_DYN_ARRAY(doclink_match_t) matches; } doclink_ctx_t; -/* Resolve a reference against the referencing file's directory, then the - * repo root, mirroring how humans write doc links. Returns the already- - * indexed File node or NULL — unresolvable references are dropped. */ +/* How a reference was written, which decides where it may resolve. Mixing + * these up let a rooted "/x" still match a same-named file in the + * referencing directory, and let an explicit "./x" fall back to an + * unrelated same-named file at the repo root. */ +typedef enum { + DOCLINK_REF_BARE = 0, /* "x": referencing dir, then repo root */ + DOCLINK_REF_RELATIVE, /* "./x": referencing dir only */ + DOCLINK_REF_ROOTED, /* "/x": repo root only */ +} doclink_ref_kind_t; + +/* Resolve a reference against the referencing file's directory and/or the + * repo root, per its kind. Returns the already-indexed File node or NULL — + * unresolvable references are dropped. */ static const cbm_gbuf_node_t *doclink_resolve(doclink_ctx_t *dc, const char *ref) { const char *r = ref; - while (r[0] == '.' && r[SKIP_ONE] == '/') { - r += PAIR_LEN; - } - if (r[0] == '/') { + doclink_ref_kind_t kind = DOCLINK_REF_BARE; + if (r[0] == '.' && r[SKIP_ONE] == '/') { + kind = DOCLINK_REF_RELATIVE; + while (r[0] == '.' && r[SKIP_ONE] == '/') { + r += PAIR_LEN; + } + } else if (r[0] == '/') { + kind = DOCLINK_REF_ROOTED; r++; /* "/docs/x.md" is repo-root-relative by doc convention */ } if (r[0] == '\0') { @@ -174,7 +215,7 @@ static const cbm_gbuf_node_t *doclink_resolve(doclink_ctx_t *dc, const char *ref } char norm[CBM_SZ_512]; - if (dc->src_dir[0] != '\0') { + if (kind != DOCLINK_REF_ROOTED && dc->src_dir[0] != '\0') { char joined[CBM_SZ_512]; int n = snprintf(joined, sizeof(joined), "%s/%s", dc->src_dir, r); if (n > 0 && (size_t)n < sizeof(joined) && doclink_normalize(joined, norm, sizeof(norm))) { @@ -184,61 +225,67 @@ static const cbm_gbuf_node_t *doclink_resolve(doclink_ctx_t *dc, const char *ref } } } - if (doclink_normalize(r, norm, sizeof(norm))) { + if (kind != DOCLINK_REF_RELATIVE && doclink_normalize(r, norm, sizeof(norm))) { return cbm_ht_get(dc->files_by_path, norm); } return NULL; } -/* Record one match. Same-pair repeats bump the count; a higher-confidence - * strategy upgrades the edge's confidence + strategy label. */ +/* Record one match. No cap, no dedup here — dc->matches is a plain append + * log; doclink_flush() sorts and collapses same-target matches so a + * generated file with hundreds of distinct targets loses nothing. */ static void doclink_record(doclink_ctx_t *dc, const cbm_gbuf_node_t *target, double confidence, const char *strategy) { if (!target || target->id == dc->src->id) { return; /* never self-reference */ } - for (int i = 0; i < dc->ref_count; i++) { - if (dc->refs[i].target_id == target->id) { - dc->refs[i].count++; - if (confidence > dc->refs[i].confidence) { - dc->refs[i].confidence = confidence; - dc->refs[i].strategy = strategy; - } - return; - } - } - if (dc->ref_count >= DOCLINK_MAX_REFS) { - dc->truncated = true; - return; - } - dc->refs[dc->ref_count].target_id = target->id; - dc->refs[dc->ref_count].count = SKIP_ONE; - dc->refs[dc->ref_count].confidence = confidence; - dc->refs[dc->ref_count].strategy = strategy; - dc->ref_count++; + doclink_match_t m = {.target_id = target->id, .confidence = confidence, .strategy = strategy}; + cbm_da_push(&dc->matches, m); +} + +static int doclink_match_cmp(const void *a, const void *b) { + int64_t ta = ((const doclink_match_t *)a)->target_id; + int64_t tb = ((const doclink_match_t *)b)->target_id; + return (ta > tb) - (ta < tb); } -/* Emit accumulated references as REFERENCES_FILE edges. Returns edge count. */ +/* Emit accumulated references as REFERENCES_FILE edges. Sorts by target id + * (O(M log M)) and collapses same-target runs: count = occurrences, the + * highest-confidence match in the run wins strategy + confidence. Returns + * edge count. */ static int doclink_flush(doclink_ctx_t *dc) { + if (dc->matches.count > 1) { + qsort(dc->matches.items, (size_t)dc->matches.count, sizeof(dc->matches.items[0]), + doclink_match_cmp); + } + int emitted = 0; - for (int i = 0; i < dc->ref_count; i++) { + int i = 0; + while (i < dc->matches.count) { + int64_t target_id = dc->matches.items[i].target_id; + double confidence = dc->matches.items[i].confidence; + const char *strategy = dc->matches.items[i].strategy; + int count = SKIP_ONE; + int j = i + SKIP_ONE; + while (j < dc->matches.count && dc->matches.items[j].target_id == target_id) { + count++; + if (dc->matches.items[j].confidence > confidence) { + confidence = dc->matches.items[j].confidence; + strategy = dc->matches.items[j].strategy; + } + j++; + } + char props[CBM_SZ_256]; (void)snprintf(props, sizeof(props), - "{\"strategy\":\"%s\",\"confidence\":%.2f,\"count\":%d}", - dc->refs[i].strategy, dc->refs[i].confidence, dc->refs[i].count); - if (cbm_gbuf_insert_edge(dc->gb, dc->src->id, dc->refs[i].target_id, DOCLINK_EDGE_TYPE, - props) > 0) { + "{\"strategy\":\"%s\",\"confidence\":%.2f,\"count\":%d}", strategy, + confidence, count); + if (cbm_gbuf_insert_edge(dc->gb, dc->src->id, target_id, DOCLINK_EDGE_TYPE, props) > 0) { emitted++; } + i = j; } - if (dc->truncated) { - char cap_buf[CBM_SZ_16]; - (void)snprintf(cap_buf, sizeof(cap_buf), "%d", DOCLINK_MAX_REFS); - cbm_log_info("doclinks.truncated", "file", dc->src->file_path ? dc->src->file_path : "", - "cap", cap_buf); - } - dc->ref_count = 0; - dc->truncated = false; + cbm_da_clear(&dc->matches); return emitted; } @@ -380,11 +427,15 @@ static void doclink_scan_md_line(doclink_ctx_t *dc, char *line) { /* ── Per-file driver ─────────────────────────────────────────────── */ -/* Scan one referencing file's content line by line and emit its edges. */ -static int doclink_scan_file(doclink_ctx_t *dc, const cbm_gbuf_node_t *node, const char *source) { +/* Scan one referencing file's content line by line and emit its edges. + * `source` is the caller's private mutable buffer (freshly read for this + * file, never reused): lines are terminated in place ('\n' -> '\0') and + * scanned by pointer, so there is no fixed-size copy and no line-length + * cap to straddle a token across (a prior CBM_SZ_4K copy buffer could cut + * "src/foo.cpp" to "src/foo.c" at the boundary and bind to the wrong + * file). A trailing '\r' (CRLF) is trimmed the same way. */ +static int doclink_scan_file(doclink_ctx_t *dc, const cbm_gbuf_node_t *node, char *source) { dc->src = node; - dc->ref_count = 0; - dc->truncated = false; dc->src_dir[0] = '\0'; const char *slash = strrchr(node->file_path, '/'); if (slash) { @@ -396,18 +447,20 @@ static int doclink_scan_file(doclink_ctx_t *dc, const cbm_gbuf_node_t *node, con dc->src_dir[dlen] = '\0'; } - const char *p = source; - char line[CBM_SZ_4K]; + char *p = source; while (*p) { - const char *eol = strchr(p, '\n'); - size_t line_len = eol ? (size_t)(eol - p) : strlen(p); - if (line_len >= sizeof(line)) { - line_len = sizeof(line) - SKIP_ONE; + char *line = p; + char *eol = strchr(p, '\n'); + if (eol) { + *eol = '\0'; + p = eol + SKIP_ONE; + } else { + p += strlen(p); + } + size_t line_len = strlen(line); + if (line_len > 0 && line[line_len - SKIP_ONE] == '\r') { + line[line_len - SKIP_ONE] = '\0'; } - memcpy(line, p, line_len); - line[line_len] = '\0'; - p = eol ? eol + SKIP_ONE : p + line_len; - doclink_scan_md_line(dc, line); } return doclink_flush(dc); @@ -426,10 +479,16 @@ static bool doclink_has_doc_files(const cbm_gbuf_node_t *const *files, int file_ } /* Scan every markdown File node's on-disk content, emitting edges. - * md_edges receives the emitted edge count. */ -static void doclink_scan_repo(doclink_ctx_t *dc, const char *repo_path, + * md_edges receives the emitted edge count. Checks cancellation once per + * file — on a docs-heavy repo this is the whole pass's cancel latency — + * and every skip is reported via cbm_pipeline_add_file_error, never + * silent. */ +static void doclink_scan_repo(cbm_pipeline_ctx_t *ctx, doclink_ctx_t *dc, const char *repo_path, const cbm_gbuf_node_t *const *files, int file_count, int *md_edges) { for (int i = 0; i < file_count; i++) { + if (cbm_pipeline_check_cancel(ctx)) { + return; + } if (!files[i]->file_path || !doclink_is_markdown_path(files[i]->file_path)) { continue; } @@ -439,12 +498,29 @@ static void doclink_scan_repo(doclink_ctx_t *dc, const char *repo_path, if (n <= 0 || (size_t)n >= sizeof(abs_path)) { continue; } - char *source = doclink_read_file(abs_path); + long file_size = 0; + cbm_read_status_t rst = CBM_READ_OK; + char *source = doclink_read_file(abs_path, &file_size, &rst); if (!source) { + if (rst == CBM_READ_OVERSIZED) { + long cap = cbm_max_file_bytes(); + char reason[96]; + (void)snprintf(reason, sizeof(reason), "oversized (%lld MB > %lld MB)", + (long long)(file_size / (CBM_SZ_1K * CBM_SZ_1K)), + (long long)(cap / (CBM_SZ_1K * CBM_SZ_1K))); + cbm_pipeline_add_file_error(ctx->pipeline, files[i]->file_path, reason, + "oversized"); + cbm_log_warn("doclinks.file_oversized", "path", files[i]->file_path); + } else if (rst == CBM_READ_OPEN_FAIL || rst == CBM_READ_OOM) { + cbm_pipeline_add_file_error(ctx->pipeline, files[i]->file_path, "read failed", + "read"); + cbm_log_warn("doclinks.file_unreadable", "path", files[i]->file_path); + } + /* CBM_READ_EMPTY: benign 0-byte file, nothing to index, not reported. */ continue; } int emitted = doclink_scan_file(dc, files[i], source); - free(source); + cbm_free(CBM_MEM_CLASS_EXTRACT, source); *md_edges += emitted; } } @@ -458,7 +534,7 @@ int cbm_pipeline_pass_doclinks(cbm_pipeline_ctx_t *ctx) { return 0; } - /* Early exit: no markdown/shell files means nothing to scan. */ + /* Early exit: no markdown files means nothing to scan. */ if (!doclink_has_doc_files(files, file_count)) { cbm_log_info("doclinks.skip", "reason", "no_doc_files"); return 0; @@ -483,7 +559,8 @@ int cbm_pipeline_pass_doclinks(cbm_pipeline_ctx_t *ctx) { } int md_edges = 0; - doclink_scan_repo(&dc, ctx->repo_path, files, file_count, &md_edges); + doclink_scan_repo(ctx, &dc, ctx->repo_path, files, file_count, &md_edges); + cbm_da_free(&dc.matches); cbm_ht_free(dc.files_by_path); char buf1[CBM_SZ_16]; diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 2750947317..e7782adeff 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -665,7 +665,7 @@ int cbm_pipeline_pass_decorator_tags(cbm_gbuf_t *gbuf, const char *project); /* Pre-dump pass: config ↔ code linking. */ int cbm_pipeline_pass_configlink(cbm_pipeline_ctx_t *ctx); -/* Pre-dump pass: markdown/shell → file REFERENCES_FILE linking. */ +/* Pre-dump pass: markdown → file REFERENCES_FILE linking. */ int cbm_pipeline_pass_doclinks(cbm_pipeline_ctx_t *ctx); /* Pre-dump pass: SIMILAR_TO edges via MinHash fingerprinting. */ diff --git a/tests/test_doclinks.c b/tests/test_doclinks.c index a03542d9b4..af24c2447d 100644 --- a/tests/test_doclinks.c +++ b/tests/test_doclinks.c @@ -1,5 +1,5 @@ /* - * test_doclinks.c — Tests for markdown/shell → file reference linking. + * test_doclinks.c — Tests for markdown → file reference linking. * * Unit-test approach mirrors test_configlink.c: create real files in a * tmpdir (the pass reads content from disk), set up File nodes in a gbuf, @@ -294,6 +294,237 @@ TEST(doclinks_null_repo_path_skips) { PASS(); } +/* ── Rooted "/x" resolves against the repo root ONLY ─────────────── */ + +TEST(doclinks_rooted_reference_skips_directory_join) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + /* docs/guide.md links "/README.md" (repo-root-relative by doc + * convention). Both docs/README.md and the root README.md exist: a + * rooted reference must resolve to the root file, never to a + * same-named file in the referencing directory. */ + th_write_file(TH_PATH(fx.tmpdir, "docs/guide.md"), "See [home](/README.md).\n"); + th_write_file(TH_PATH(fx.tmpdir, "docs/README.md"), "# Wrong target\n"); + th_write_file(TH_PATH(fx.tmpdir, "README.md"), "# Root\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "docs/guide.md"); + int64_t docs_readme_id = add_file_node(fx.gb, fx.project, "docs/README.md"); + int64_t root_readme_id = add_file_node(fx.gb, fx.project, "README.md"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + + ASSERT_NOT_NULL(find_ref_edge(fx.gb, doc_id, root_readme_id)); + ASSERT_NULL(find_ref_edge(fx.gb, doc_id, docs_readme_id)); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Explicit "./x" resolves against the referencing dir ONLY ────── */ + +TEST(doclinks_explicit_relative_no_root_fallback) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + /* docs/a.md links "./config.md". docs/config.md does not exist, but a + * root config.md does — an explicit "./" reference must not fall back + * to an unrelated same-named file at the repo root. */ + th_write_file(TH_PATH(fx.tmpdir, "docs/a.md"), "See [cfg](./config.md).\n"); + th_write_file(TH_PATH(fx.tmpdir, "config.md"), "# Root config\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "docs/a.md"); + add_file_node(fx.gb, fx.project, "config.md"); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_EQ(n, 0); + ASSERT_EQ(ref_edge_count(fx.gb), 0); + (void)doc_id; + + dl_fix_free(&fx); + PASS(); +} + +/* ── A line past the old 4 KiB copy cap is scanned in full ───────── */ + +TEST(doclinks_long_line_no_truncation) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + /* Padding + "src/foo.cpp" (no trailing newline) straddles the old + * CBM_SZ_4K line-copy cap so that, under the old bug, the copy cut + * exactly after "src/foo.c" and dropped "pp": a bare mention of + * src/foo.cpp silently became a reference to the wrong file. Both + * files exist so a wrong match is observable. */ + enum { FILLER_WORDS = 681 }; /* 681 * strlen("lorem ") == 4086 */ + char content[CBM_SZ_4K * 2]; + size_t pos = 0; + for (int i = 0; i < FILLER_WORDS; i++) { + memcpy(content + pos, "lorem ", 6); + pos += 6; + } + ASSERT_EQ((int)pos, 4086); + memcpy(content + pos, "src/foo.cpp", 11); + pos += 11; + content[pos] = '\0'; + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), content); + th_write_file(TH_PATH(fx.tmpdir, "src/foo.c"), "// wrong target\n"); + th_write_file(TH_PATH(fx.tmpdir, "src/foo.cpp"), "// right target\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "README.md"); + int64_t short_id = add_file_node(fx.gb, fx.project, "src/foo.c"); + int64_t long_id = add_file_node(fx.gb, fx.project, "src/foo.cpp"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + + ASSERT_NOT_NULL(find_ref_edge(fx.gb, doc_id, long_id)); + ASSERT_NULL(find_ref_edge(fx.gb, doc_id, short_id)); + + dl_fix_free(&fx); + PASS(); +} + +/* ── More than the old 256-target cap: nothing is dropped ────────── */ + +TEST(doclinks_many_distinct_targets_no_cap) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + enum { TARGETS = 300 }; /* > the old DOCLINK_MAX_REFS (256) */ + char content[TARGETS * 24]; + size_t pos = 0; + int64_t doc_id = add_file_node(fx.gb, fx.project, "MANY.md"); + for (int i = 0; i < TARGETS; i++) { + char rel[32]; + char line[40]; + snprintf(rel, sizeof(rel), "gen/f%03d.txt", i); + th_write_file(TH_PATH(fx.tmpdir, rel), "x\n"); + add_file_node(fx.gb, fx.project, rel); + int n = snprintf(line, sizeof(line), "%s\n", rel); + memcpy(content + pos, line, (size_t)n); + pos += (size_t)n; + } + content[pos] = '\0'; + th_write_file(TH_PATH(fx.tmpdir, "MANY.md"), content); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_EQ(n, TARGETS); + ASSERT_EQ(ref_edge_count(fx.gb), TARGETS); + (void)doc_id; + + dl_fix_free(&fx); + PASS(); +} + +/* ── ".." past the repo root is dropped, not resolved ────────────── */ + +TEST(doclinks_dotdot_escaping_root_dropped) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + /* README.md sits at the repo root (depth 0): "../outside.md" has + * nowhere to go up from and must be dropped, not resolved. */ + th_write_file(TH_PATH(fx.tmpdir, "README.md"), "See [x](../outside.md).\n"); + + add_file_node(fx.gb, fx.project, "README.md"); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_EQ(n, 0); + ASSERT_EQ(ref_edge_count(fx.gb), 0); + + dl_fix_free(&fx); + PASS(); +} + +/* ── A bare URL in prose is not mistaken for a repo file path ────── */ + +TEST(doclinks_bare_url_in_prose_no_edge) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), + "Fetch it from http://host/path/file.c directly.\n"); + th_write_file(TH_PATH(fx.tmpdir, "path/file.c"), "int x;\n"); + + add_file_node(fx.gb, fx.project, "README.md"); + add_file_node(fx.gb, fx.project, "path/file.c"); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_EQ(n, 0); + ASSERT_EQ(ref_edge_count(fx.gb), 0); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Exact-path matching: a bare basename never fuzzy-binds ──────── */ + +TEST(doclinks_backtick_basename_does_not_fuzzy_match) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + /* `main.c` in docs/ must resolve against docs/main.c or the repo-root + * main.c ONLY (exact path match) — never against src/main.c just + * because the basename matches. */ + th_write_file(TH_PATH(fx.tmpdir, "docs/a.md"), "See `main.c` for the entry point.\n"); + th_write_file(TH_PATH(fx.tmpdir, "src/main.c"), "int main(void) { return 0; }\n"); + + add_file_node(fx.gb, fx.project, "docs/a.md"); + add_file_node(fx.gb, fx.project, "src/main.c"); + + int n = run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_EQ(n, 0); + ASSERT_EQ(ref_edge_count(fx.gb), 0); + + dl_fix_free(&fx); + PASS(); +} + +/* ── CRLF line endings do not corrupt a matched target ───────────── */ + +TEST(doclinks_crlf_line_endings) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), + "# Title\r\nSee [build](scripts/build.sh) for details.\r\n"); + th_write_file(TH_PATH(fx.tmpdir, "scripts/build.sh"), "#!/bin/sh\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "README.md"); + int64_t script_id = add_file_node(fx.gb, fx.project, "scripts/build.sh"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + ASSERT_NOT_NULL(find_ref_edge(fx.gb, doc_id, script_id)); + + dl_fix_free(&fx); + PASS(); +} + +/* ── Fenced code blocks are scanned like prose (pinned, not fixed) ── */ + +TEST(doclinks_fenced_code_block_bare_mention_pinned) { + dl_fix_t fx; + ASSERT_TRUE(dl_fix_init(&fx)); + + th_write_file(TH_PATH(fx.tmpdir, "README.md"), + "Build it:\n\n```\ngcc src/main.c -o out\n```\n"); + th_write_file(TH_PATH(fx.tmpdir, "src/main.c"), "int main(void) { return 0; }\n"); + + int64_t doc_id = add_file_node(fx.gb, fx.project, "README.md"); + int64_t code_id = add_file_node(fx.gb, fx.project, "src/main.c"); + + run_doclinks(fx.gb, fx.project, fx.tmpdir); + + const cbm_gbuf_edge_t *e = find_ref_edge(fx.gb, doc_id, code_id); + ASSERT_NOT_NULL(e); + ASSERT_TRUE(edge_has_strategy(e, "md_bare_mention")); + ASSERT_NOT_NULL(strstr(e->properties_json, "\"confidence\":0.70")); + + dl_fix_free(&fx); + PASS(); +} + /* ── Suite ───────────────────────────────────────────────────────── */ SUITE(doclinks) { @@ -309,4 +540,17 @@ SUITE(doclinks) { RUN_TEST(doclinks_resolves_relative_to_file_dir); RUN_TEST(doclinks_unresolvable_target_no_edge); RUN_TEST(doclinks_null_repo_path_skips); + + /* Reference-kind precedence (rooted / explicit-relative / bare) */ + RUN_TEST(doclinks_rooted_reference_skips_directory_join); + RUN_TEST(doclinks_explicit_relative_no_root_fallback); + + /* Negative controls / regression guards */ + RUN_TEST(doclinks_long_line_no_truncation); + RUN_TEST(doclinks_many_distinct_targets_no_cap); + RUN_TEST(doclinks_dotdot_escaping_root_dropped); + RUN_TEST(doclinks_bare_url_in_prose_no_edge); + RUN_TEST(doclinks_backtick_basename_does_not_fuzzy_match); + RUN_TEST(doclinks_crlf_line_endings); + RUN_TEST(doclinks_fenced_code_block_bare_mention_pinned); } diff --git a/tests/test_main.c b/tests/test_main.c index 1278c1c70b..db37482886 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -1182,7 +1182,7 @@ int main(int argc, char **argv) { /* Config link */ RUN_SELECTED_SUITE(configlink); - /* Doc/shell file reference link */ + /* Markdown file reference link */ RUN_SELECTED_SUITE(doclinks); /* Infrastructure scanning */ diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index d32ab3538b..6ef16a89be 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -314,6 +314,36 @@ TEST(pipeline_grpc_routes_cover_every_service_past_the_old_cap) { PASS(); } +/* Pipeline-level: the doclinks pass runs as part of a real index and its + * REFERENCES_FILE edges land in the store, not just in a unit test that + * calls cbm_pipeline_pass_doclinks() directly. Registration-list gap: every + * doclinks unit test calls the pass function by hand, so deleting its + * {predump_doclinks, "doclinks", false} entry in pipeline.c would leave + * every one of them green. */ +TEST(pipeline_doclinks_edge_lands_in_store) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_doclinks_pipeline_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + + write_temp_file(tmp, "README.md", "See [main](src/main.go) for the entry point.\n"); + write_temp_file(tmp, "src/main.go", "package main\n\nfunc main() {}\n"); + + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/doclinks.db", tmp); + cbm_pipeline_t *p = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + const char *project = cbm_pipeline_project_name(p); + ASSERT_GT(cbm_store_count_edges_by_type(s, project, "REFERENCES_FILE"), 0); + cbm_store_close(s); + cbm_pipeline_free(p); + th_rmtree(tmp); + PASS(); +} + /* Spilling must be invisible in the OUTPUT: the same repository indexed with * results parked on disk must produce the same graph as one indexed entirely in * memory. It did not. The namespace map that `use`/`using`/package imports @@ -15079,6 +15109,7 @@ SUITE(pipeline) { RUN_TEST(store_bulk_persistence); /* Integration: structure pass */ RUN_TEST(pipeline_grpc_routes_cover_every_service_past_the_old_cap); + RUN_TEST(pipeline_doclinks_edge_lands_in_store); RUN_TEST(pipeline_spill_resolves_namespace_imports_like_memory); RUN_TEST(pipeline_structure_nodes); RUN_TEST(pipeline_committed_counts_match_persisted); From 9a9534cbc45d40afa4e8bf070030a806acea1f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Deust?= Date: Tue, 22 Sep 2026 08:54:40 +0200 Subject: [PATCH 09/10] docs: list REFERENCES_FILE in the README edge types table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The full Edge Types list near the graph schema section still missed the new REFERENCES_FILE type added by the doclinks pass. The "selected" list earlier in the file is explicitly a subset and is left as is. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Clément Deust --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 66b50fe082..9c82c18aed 100644 --- a/README.md +++ b/README.md @@ -704,7 +704,7 @@ JSON arguments can also be piped on stdin, for tools that take arguments. A tool ### Edge Types -`CONTAINS_PACKAGE`, `CONTAINS_FOLDER`, `CONTAINS_FILE`, `DEFINES`, `DEFINES_METHOD`, `IMPORTS`, `CALLS`, `CALL_REFERENCE`, `HTTP_CALLS`, `ASYNC_CALLS`, `IMPLEMENTS`, `HANDLES`, `USAGE`, `CONFIGURES`, `WRITES`, `MEMBER_OF`, `TESTS`, `USES_TYPE`, `FILE_CHANGES_WITH` +`CONTAINS_PACKAGE`, `CONTAINS_FOLDER`, `CONTAINS_FILE`, `DEFINES`, `DEFINES_METHOD`, `IMPORTS`, `CALLS`, `CALL_REFERENCE`, `HTTP_CALLS`, `ASYNC_CALLS`, `IMPLEMENTS`, `HANDLES`, `USAGE`, `CONFIGURES`, `REFERENCES_FILE`, `WRITES`, `MEMBER_OF`, `TESTS`, `USES_TYPE`, `FILE_CHANGES_WITH` ### Qualified Names From 1d2bbc425988eb78fcd966dc8c9cfbdb814f0685 Mon Sep 17 00:00:00 2001 From: News Date: Tue, 22 Sep 2026 22:33:51 +0900 Subject: [PATCH 10/10] fix(windows): preserve worker recovery errors Signed-off-by: News --- src/foundation/compat.c | 52 +++++++++++++++++++++++++------------- src/mcp/index_supervisor.c | 6 +++++ src/mcp/mcp.c | 5 +++- tests/test_platform.c | 41 +++++++++++++++++++++++++++--- 4 files changed, 82 insertions(+), 22 deletions(-) diff --git a/src/foundation/compat.c b/src/foundation/compat.c index 94e7020bd7..fb3caca871 100644 --- a/src/foundation/compat.c +++ b/src/foundation/compat.c @@ -269,36 +269,54 @@ int cbm_mkstemp(char *tmpl) { errno = ENAMETOOLONG; return CBM_NOT_FOUND; } - /* Wide-API expansion and open: worker staging files land inside - * CBM_CACHE_DIR, which users may place at non-ASCII paths; the ANSI CRT - * (_mktemp/_open) mangles those bytes in the local codepage. */ + /* Keep the six-character mkstemp contract, but do not use _wmktemp: + * that CRT helper has a tiny name space on Windows and retained worker + * logs can exhaust it during recovery. The exclusive open closes races + * with other processes; collisions simply draw another random name. */ wchar_t *wide_template = cbm_utf8_to_wide(buf); - if (!wide_template || !_wmktemp(wide_template)) { - free(wide_template); + if (!wide_template) { + errno = EINVAL; return CBM_NOT_FOUND; } - char *expanded_for_open = cbm_wide_to_utf8(wide_template); - wchar_t *wide_open = expanded_for_open ? cbm_path_to_wide(expanded_for_open) : NULL; - free(expanded_for_open); - if (!wide_open) { + size_t wide_len = wcslen(wide_template); + if (wide_len < 6 || wcscmp(wide_template + wide_len - 6, L"XXXXXX") != 0) { free(wide_template); + errno = EINVAL; return CBM_NOT_FOUND; } - int fd = _wopen(wide_open, _O_CREAT | _O_EXCL | _O_RDWR | _O_BINARY, _S_IREAD | _S_IWRITE); - free(wide_open); - if (fd >= 0) { + static const wchar_t hex[] = L"0123456789abcdef"; + for (int attempt = 0; attempt < 128; attempt++) { + unsigned int random_bits = 0; + if (!cbm_secure_random(&random_bits, sizeof(random_bits))) { + errno = EIO; + break; + } + for (int digit = 0; digit < 6; digit++) { + wide_template[wide_len - 6 + digit] = hex[(random_bits >> (digit * 4)) & 0xf]; + } char *expanded = cbm_wide_to_utf8(wide_template); - if (!expanded || strlen(expanded) >= sizeof(buf)) { + wchar_t *wide_open = expanded ? cbm_path_to_wide(expanded) : NULL; + if (!expanded || !wide_open || strlen(expanded) >= sizeof(buf)) { + free(expanded); + free(wide_open); + errno = ENAMETOOLONG; + break; + } + int fd = _wopen(wide_open, _O_CREAT | _O_EXCL | _O_RDWR | _O_BINARY, _S_IREAD | _S_IWRITE); + free(wide_open); + if (fd >= 0) { + strcpy(tmpl, expanded); free(expanded); free(wide_template); - (void)_close(fd); - return CBM_NOT_FOUND; + return fd; } - strcpy(tmpl, expanded); free(expanded); + if (errno != EEXIST) { + break; + } } free(wide_template); - return fd; + return CBM_NOT_FOUND; } #endif diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c index 4cd69f6426..5e15a7eed0 100644 --- a/src/mcp/index_supervisor.c +++ b/src/mcp/index_supervisor.c @@ -13,6 +13,7 @@ #include "ui/http_server.h" /* cbm_http_server_resolve_binary_path */ #include +#include #include #include #include @@ -705,6 +706,11 @@ int cbm_index_worker_start_with_log(const char *args_json, size_t memory_budget_ worker_result_init(&handle->result); if (!worker_unique_file(handle->response_path, sizeof(handle->response_path), "response") || !worker_unique_file(handle->log_path, sizeof(handle->log_path), "log")) { + int saved_errno = errno; + char error_text[CBM_SZ_32]; + (void)snprintf(error_text, sizeof(error_text), "%d", saved_errno); + cbm_log_error("index.supervisor.artifact_create_failed", "artifact", + handle->response_path[0] ? "log" : "response", "errno", error_text); (void)cbm_unlink(handle->response_path); (void)cbm_unlink(handle->log_path); free(handle); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 75ff8e396c..d6c7bd4686 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -10808,7 +10808,10 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { cbm_mcp_supervised_result_disposition_t recovery_disposition = cbm_mcp_supervised_result_disposition(rc2, &wr2); if (recovery_disposition == CBM_MCP_SUPERVISED_RESULT_FALLBACK) { - last_outcome = wr2.outcome; + /* Keep the original worker failure if recovery setup fails. */ + cbm_log_error("index.supervisor.recovery_start_failed", "original_outcome", + cbm_proc_outcome_str(last_outcome), "recovery_outcome", + cbm_proc_outcome_str(wr2.outcome)); cbm_index_worker_result_free(&wr2); break; /* spawn failed mid-recovery — give up */ } diff --git a/tests/test_platform.c b/tests/test_platform.c index 5995a4b017..16cc3b5d31 100644 --- a/tests/test_platform.c +++ b/tests/test_platform.c @@ -46,8 +46,7 @@ TEST(platform_file_apis_survive_max_path_overflow) { ASSERT_NOT_NULL(cbm_mkdtemp(base)); enum { LONG_SEGMENTS = 5 }; - static const char segment[] = - "segment-abcdefghijklmnopqrstuvwxyz0123456789-abcdefghijklmnop"; + static const char segment[] = "segment-abcdefghijklmnopqrstuvwxyz0123456789-abcdefghijklmnop"; char deep[CBM_SZ_1K]; written = snprintf(deep, sizeof(deep), "%s", base); ASSERT_TRUE(written > 0 && written < (int)sizeof(deep)); @@ -475,6 +474,28 @@ TEST(platform_mkstemp_and_mkdtemp_survive_non_ascii_directory) { PASS(); } +#ifdef _WIN32 +TEST(platform_mkstemp_retained_files_exceed_crt_namespace) { + char base[CBM_SZ_256] = "/tmp/cbm-many-temp-XXXXXX"; + ASSERT_NOT_NULL(cbm_mkdtemp(base)); + char paths[64][CBM_SZ_512] = {{0}}; + int created = 0; + for (; created < 64; created++) { + int written = + snprintf(paths[created], sizeof(paths[created]), "%s/.worker-log-XXXXXX", base); + ASSERT_TRUE(written > 0 && written < (int)sizeof(paths[created])); + int descriptor = cbm_mkstemp(paths[created]); + ASSERT_TRUE(descriptor >= 0); + ASSERT_EQ(_close(descriptor), 0); + } + for (int index = 0; index < created; index++) { + ASSERT_EQ(cbm_unlink(paths[index]), 0); + } + ASSERT_EQ(cbm_rmdir(base), 0); + PASS(); +} +#endif + typedef struct { atomic_int *ready; atomic_bool *go; @@ -832,8 +853,17 @@ TEST(platform_env_long_refuses_what_it_cannot_read) { /* Every one of these used to answer 0 through atol. */ const char *unreadable[] = { - "abc", "30s", " 30", "30 ", "", "1e3", - "0x10", "+ 30", "--3", "3.5", "99999999999999999999999999", + "abc", + "30s", + " 30", + "30 ", + "", + "1e3", + "0x10", + "+ 30", + "--3", + "3.5", + "99999999999999999999999999", }; for (size_t i = 0; i < sizeof(unreadable) / sizeof(unreadable[0]); i++) { ASSERT_EQ(cbm_setenv(name, unreadable[i], 1), 0); @@ -1135,6 +1165,9 @@ SUITE(platform) { RUN_TEST(platform_mkdir_p_follow_owned_is_per_call_site); RUN_TEST(platform_mkdir_p_resolves_link_text_from_the_link_directory); RUN_TEST(platform_mkstemp_and_mkdtemp_survive_non_ascii_directory); +#ifdef _WIN32 + RUN_TEST(platform_mkstemp_retained_files_exceed_crt_namespace); +#endif RUN_TEST(platform_mkdtemp_is_thread_safe); RUN_TEST(platform_counter_scaling_avoids_intermediate_overflow); RUN_TEST(platform_counter_scaling_preserves_monotonic_deadlines);