From 7883507db6779a20bcfad398c8d6594f4206aa9b Mon Sep 17 00:00:00 2001 From: Brian McNaboe Date: Wed, 19 Aug 2026 10:14:08 -0400 Subject: [PATCH 01/14] fix(hooks): clear repository git environment for tests Signed-off-by: Brian McNaboe --- scripts/hooks/pre-commit | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/hooks/pre-commit b/scripts/hooks/pre-commit index fa42d92ae..7b5e1a4ff 100755 --- a/scripts/hooks/pre-commit +++ b/scripts/hooks/pre-commit @@ -17,4 +17,13 @@ echo "pre-commit: security audit (source-level)..." scripts/security-audit.sh echo "pre-commit: building and running tests..." -make -j$(sysctl -n hw.ncpu 2>/dev/null || nproc) -f Makefile.cbm test +( + # Git exports repository-local variables to hooks. The test suite creates + # independent repositories and linked worktrees, so keep the commit's + # index and object paths out of those fixtures. + while IFS= read -r variable; do + unset "$variable" + done < <(git rev-parse --local-env-vars) + + make -j"$(sysctl -n hw.ncpu 2>/dev/null || nproc)" -f Makefile.cbm test +) From 3604d6cf66c3d3099cec0b0b9a2f6da7f15e9e88 Mon Sep 17 00:00:00 2001 From: Brian McNaboe Date: Wed, 19 Aug 2026 10:35:16 -0400 Subject: [PATCH 02/14] fix(tests): scale daemon runtime sanitizer timeout Signed-off-by: Brian McNaboe --- tests/test_daemon_runtime.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_daemon_runtime.c b/tests/test_daemon_runtime.c index 5f41d914e..b650b5efb 100644 --- a/tests/test_daemon_runtime.c +++ b/tests/test_daemon_runtime.c @@ -21,6 +21,7 @@ #include "foundation/compat_thread.h" #include "foundation/log.h" #include "foundation/platform.h" +#include "foundation/sanitized.h" #include "pipeline/pipeline.h" #include "store/store.h" @@ -3550,10 +3551,18 @@ TEST(daemon_runtime_disconnect_cancels_blocked_non_index_child_and_preserves_oth SKIP_PLATFORM("requires a queryable copied process image"); #else enum { +#if CBM_SANITIZED + /* This readiness wait is a liveness backstop, not the behavior under + * test. The copied instrumented runner can take several seconds to + * reach main on macOS, especially while the parallel gate is busy. */ + CHILD_READY_BOUND_MS = 60000, + REQUEST_TIMEOUT_MS = 90000, +#else CHILD_READY_BOUND_MS = 5000, + REQUEST_TIMEOUT_MS = 15000, +#endif CHILD_CANCEL_BOUND_MS = 3000, CHILD_CLEANUP_BOUND_MS = 5000, - REQUEST_TIMEOUT_MS = 15000, }; const char *old_cache = getenv("CBM_CACHE_DIR"); const char *old_path = getenv("PATH"); From 5cb90f64adaf9c5a38b591971aa7c09e3d9e958c Mon Sep 17 00:00:00 2001 From: Ilya Brykau Date: Sat, 29 Aug 2026 19:02:12 +0200 Subject: [PATCH 03/14] fix(extract): drop cgo's `import "C"` pseudo-package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `import "C"` is not an import: "C" is reserved by go/build and no package with that path can exist — it only tells the toolchain to compile the preceding comment as C. cbm emitted it as an ordinary import, so the import resolver fell through to its symbol-name fallback (cbm_pipeline_resolve_import_node Strategy 3, pass_pkgmap.c), matched the literal name "C" against every project node called C, and picked the lexicographically smallest one. Measured on a real Go+C repo: all 27 files carrying `import "C"` pointed their IMPORTS edge at the same unrelated `C` member of a test helper — 27 false edges, and 27 files whose import map named a package that does not exist. Skip the spec in parse_go_import_spec. Node counts are unchanged and the file's real imports are untouched; only the false edge disappears (a 3-file fixture goes 22 edges -> 21, 14 nodes -> 14; the real repo goes 27 -> 0 such edges). Reproduce-first: go_cgo_pseudo_import_dropped is RED without the extractor change (the "C" import is still in imports[]) and GREEN with it, while still asserting the file's real `fmt` import survives. Signed-off-by: Ilya Brykau --- internal/cbm/extract_imports.c | 13 +++++++++++++ tests/test_extraction.c | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/internal/cbm/extract_imports.c b/internal/cbm/extract_imports.c index 9de2fd007..37c9f5f63 100644 --- a/internal/cbm/extract_imports.c +++ b/internal/cbm/extract_imports.c @@ -133,6 +133,19 @@ static void parse_go_import_spec(CBMExtractCtx *ctx, TSNode spec) { return; } + /* cgo: `import "C"` names the cgo PSEUDO-package, not a real one — "C" is + * reserved by go/build and no package with that import path can exist. + * Emitting it as an ordinary import makes the import resolver fall through + * to its symbol-name fallback (cbm_pipeline_resolve_import_node Strategy 3, + * pass_pkgmap.c), which matches the literal name "C" against every project + * node called C and picks the lexicographically smallest: on a real Go repo + * all 27 cgo files pointed their IMPORTS edge at the same unrelated `C` + * member of a test helper. Drop it — the C side is not a graph package, and + * the file's remaining imports are unaffected. */ + if (strcmp(path, "C") == 0) { + return; + } + TSNode name_node = ts_node_child_by_field_name(spec, TS_FIELD("name")); const char *local_name = !ts_node_is_null(name_node) ? cbm_node_text(a, name_node, ctx->source) : path_last(a, path); diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 5b8d16f61..9375adc2f 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -3015,6 +3015,25 @@ TEST(go_imports) { PASS(); } +/* cgo's `import "C"` is a pseudo-package, not a real import: keeping it lets the + * import resolver name-match "C" onto an arbitrary project symbol called C. The + * real imports of the same file must survive. */ +TEST(go_cgo_pseudo_import_dropped) { + CBMFileResult *r = extract("package m\n\n/*\nstatic int helper(void) { return 1; }\n*/\n" + "import \"C\"\n\nimport \"fmt\"\n\n" + "func Run() { fmt.Println(C.helper()) }\n", + CBM_LANG_GO, "t", "cgo.go"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT(has_import(r, "fmt")); + for (int i = 0; i < r->imports.count; i++) { + ASSERT_NOT_NULL(r->imports.items[i].module_path); + ASSERT_TRUE(strcmp(r->imports.items[i].module_path, "C") != 0); + } + cbm_free_result(r); + PASS(); +} + /* #1935: Go struct fields were never extracted — find_class_body() returns the * struct_type node, whose only named child is a field_declaration_list, so the * member loop matched nothing and every field was silently skipped (0 Field @@ -6752,6 +6771,7 @@ SUITE(extraction) { RUN_TEST(python_imports); RUN_TEST(js_imports); RUN_TEST(go_imports); + RUN_TEST(go_cgo_pseudo_import_dropped); RUN_TEST(extract_go_struct_fields_have_nodes); RUN_TEST(java_imports); RUN_TEST(rust_imports); From eb80a4ecff04d8bfacef20e1aa2ee29445a782ff Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sat, 29 Aug 2026 13:53:42 -0400 Subject: [PATCH 04/14] fix(extraction): reach a Swift URL built by a constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Swift has no URL literal, so almost no real code passes a bare string to a request. It writes URL(string: "https://…")! instead, and the literal then sits two levels below the argument list: past the trailing "!", which the grammar models as a postfix_expression, and inside the constructor's own value_arguments. extract_url_or_topic_arg saw only the outer node and gave up, so the URL never reached the service-pattern table and no Route node formed. That is the shape issue #1892 reported from a real project — reaching a bare string argument was only the layer underneath it. swift_unwrap_url_constructor() steps past both wrappers. It unwraps only URL, URLComponents and URLRequest, so any other constructor keeps its own meaning and the outer call does not borrow the inner call's string. A non-literal argument such as URL(string: base + path) falls through to the ordinary handling unchanged. The value_argument unwrap added for the bare-string case is now swift_argument_value(), because the nested argument list needs the same step and the code was identical. Refs #1892 Signed-off-by: Joshua Richter --- internal/cbm/extract_calls.c | 70 +++++++++++++++++++++++++++++++----- tests/test_extraction.c | 49 +++++++++++++++++++++++++ tests/test_pipeline.c | 50 ++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 9 deletions(-) diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index c22a11e01..1821cb692 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -2257,6 +2257,61 @@ static const char *extract_binary_concat_suffix(CBMExtractCtx *ctx, TSNode node) } // Try to extract URL/topic from a positional argument (string or constant). +/* Swift names its call arguments, so each one is a value_argument that may lead + * with a value_argument_label — the `from:` in `data(from: url)`. Return the + * value itself, and leave any other node exactly as it came in. */ +static TSNode swift_argument_value(TSNode arg) { + if (strcmp(ts_node_type(arg), "value_argument") != 0 || ts_node_named_child_count(arg) == 0) { + return arg; + } + TSNode val = ts_node_named_child(arg, 0); + if (strcmp(ts_node_type(val), "value_argument_label") == 0 && + ts_node_named_child_count(arg) > 1) { + val = ts_node_named_child(arg, 1); + } + return val; +} + +/* Swift has no URL literal, so almost no real code passes a bare string to a + * request. It writes `URL(string: "https://…")!` instead, and the literal then + * sits two levels down: past the trailing `!`, which the grammar models as a + * postfix_expression, and inside the constructor's own argument list. + * + * Unwrap both so that literal is as reachable as a bare one. Only the three + * Foundation types that take a URL string are unwrapped — any other call keeps + * its own meaning, and a non-literal argument such as `URL(string: base + path)` + * falls through to the ordinary handling unchanged. */ +static TSNode swift_unwrap_url_constructor(CBMExtractCtx *ctx, TSNode arg) { + /* Step past a trailing "!" or "?". */ + if (strcmp(ts_node_type(arg), "postfix_expression") == 0) { + TSNode target = ts_node_child_by_field_name(arg, TS_FIELD("target")); + if (!ts_node_is_null(target)) { + arg = target; + } + } + if (strcmp(ts_node_type(arg), "call_expression") != 0) { + return arg; + } + TSNode callee = ts_node_named_child(arg, 0); + if (ts_node_is_null(callee) || strcmp(ts_node_type(callee), "simple_identifier") != 0) { + return arg; + } + const char *name = cbm_node_text(ctx->arena, callee, ctx->source); + if (!name || (strcmp(name, "URL") != 0 && strcmp(name, "URLComponents") != 0 && + strcmp(name, "URLRequest") != 0)) { + return arg; + } + TSNode suffix = cbm_find_child_by_kind(arg, "call_suffix"); + if (ts_node_is_null(suffix)) { + return arg; + } + TSNode inner = cbm_find_child_by_kind(suffix, "value_arguments"); + if (ts_node_is_null(inner) || ts_node_named_child_count(inner) == 0) { + return arg; + } + return swift_argument_value(ts_node_named_child(inner, 0)); +} + static const char *extract_positional_url(CBMExtractCtx *ctx, TSNode arg, const char *ak) { /* JS/TS template literals: `/things/${id}` normalizes to "/things/{}" so the * client URL joins the server route's canonical placeholder (issue #1006). */ @@ -2300,15 +2355,12 @@ static const char *extract_url_or_topic_arg(CBMExtractCtx *ctx, TSNode args) { } /* Swift wraps each argument in a value_argument that may lead with its * label, so `data(from: url)` would otherwise yield the label `from` - * rather than the value. Step past a leading value_argument_label. */ - if (strcmp(ts_node_type(arg), "value_argument") == 0 && - ts_node_named_child_count(arg) > 0) { - TSNode val = ts_node_named_child(arg, 0); - if (strcmp(ts_node_type(val), "value_argument_label") == 0 && - ts_node_named_child_count(arg) > 1) { - val = ts_node_named_child(arg, 1); - } - arg = val; + * rather than the value. */ + arg = swift_argument_value(arg); + /* A Swift URL is usually built rather than written bare, and the + * literal then sits inside that constructor. */ + if (ctx->language == CBM_LANG_SWIFT) { + arg = swift_unwrap_url_constructor(ctx, arg); } const char *ak = ts_node_type(arg); diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 5c63c98aa..cb7a1d1df 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -4186,6 +4186,52 @@ TEST(swift_labeled_call_string_arg_issue1892) { PASS(); } +/* Swift has no URL literal, so real code builds one and force-unwraps it. The + * string then sits two levels below the argument list. */ +TEST(swift_nested_url_constructor_issue1892) { + CBMFileResult *r = extract("func fetch() { URLSession.shared.dataTask(with: URL(string: " + "\"https://example.com/api/v1/widgets\")!) }\n", + CBM_LANG_SWIFT, "t", "Fetch.swift"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + const CBMCall *c = find_call_by_callee(r, "URLSession.shared.dataTask"); + ASSERT_NOT_NULL(c); + ASSERT_NOT_NULL(c->first_string_arg); + ASSERT_STR_EQ(c->first_string_arg, "https://example.com/api/v1/widgets"); + cbm_free_result(r); + PASS(); +} + +/* Without the trailing "!" the constructor is not wrapped in a + * postfix_expression, so this covers the other shape. */ +TEST(swift_nested_url_no_bang_issue1892) { + CBMFileResult *r = + extract("func fetch() { client.send(to: URLRequest(url: \"/api/v1/widgets/1\")) }\n", + CBM_LANG_SWIFT, "t", "Send.swift"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + const CBMCall *c = find_call_by_callee(r, "client.send"); + ASSERT_NOT_NULL(c); + ASSERT_NOT_NULL(c->first_string_arg); + ASSERT_STR_EQ(c->first_string_arg, "/api/v1/widgets/1"); + cbm_free_result(r); + PASS(); +} + +/* A constructor that is not one of the three URL types keeps its own meaning: + * the outer call must not borrow the inner call's string. */ +TEST(swift_non_url_constructor_untouched_issue1892) { + CBMFileResult *r = extract("func f() { log.write(to: Formatter(pattern: \"%s-%d\")) }\n", + CBM_LANG_SWIFT, "t", "Log.swift"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + const CBMCall *c = find_call_by_callee(r, "log.write"); + ASSERT_NOT_NULL(c); + ASSERT_NULL(c->first_string_arg); + cbm_free_result(r); + PASS(); +} + /* Issue #1009: URL-builder helper pattern — a function returning a URL-shaped * literal, consumed as client(buildPath(id)). The builder's URL is recorded in * the per-file constant map and resolved at the call site, for both return @@ -7131,6 +7177,9 @@ SUITE(extraction) { RUN_TEST(swift_chained_call); RUN_TEST(swift_force_unwrap_scanner_shift); RUN_TEST(swift_call_string_arg_issue1892); + RUN_TEST(swift_nested_url_constructor_issue1892); + RUN_TEST(swift_nested_url_no_bang_issue1892); + RUN_TEST(swift_non_url_constructor_untouched_issue1892); RUN_TEST(swift_labeled_call_string_arg_issue1892); RUN_TEST(objc_interface); RUN_TEST(objc_implementation); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index cf40dd57a..192e09a94 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -5690,6 +5690,55 @@ TEST(pipeline_native_fetch_classified_as_http_calls) { * no call arguments at all. Alamofire/URLSession were already in the service * pattern table; the URL simply never reached it. This is the Swift twin of * the TypeScript fetch case above. */ +/* The shape real Swift actually writes: the URL is built by a constructor and + * force-unwrapped, so the literal is two levels below the argument list. This + * is what issue #1892 reported from a real project. */ +TEST(pipeline_swift_nested_url_makes_route_issue1892) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_swiftnested_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + + write_temp_file(tmp, "Sources/Client.swift", + "import Foundation\n" + "final class Client {\n" + " func listWidgets() {\n" + " URLSession.shared.dataTask(with: " + "URL(string: \"/api/v1/widgets\")!)\n" + " }\n" + "}\n"); + + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/swiftnested.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); + const char *project = cbm_pipeline_project_name(p); + + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + + ASSERT_GTE(cbm_store_count_edges_by_type(s, project, "HTTP_CALLS"), 1); + + cbm_node_t *routes = NULL; + int route_count = 0; + cbm_store_find_nodes_by_label(s, project, "Route", &routes, &route_count); + int widget_routes = 0; + for (int i = 0; i < route_count; i++) { + if (routes[i].qualified_name && strstr(routes[i].qualified_name, "/api/v1/widgets")) { + widget_routes++; + } + } + cbm_store_free_nodes(routes, route_count); + ASSERT_GTE(widget_routes, 1); + + cbm_store_close(s); + cbm_pipeline_free(p); + th_rmtree(tmp); + PASS(); +} + TEST(pipeline_swift_http_call_makes_route_issue1892) { char tmp[256]; snprintf(tmp, sizeof(tmp), "/tmp/cbm_swifthttp_XXXXXX"); @@ -13475,6 +13524,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_parallel_rust_cross_only_macro_hidden_gets_synthetic_carrier); RUN_TEST(pipeline_arg_url_rejects_non_http_slash_arguments); RUN_TEST(pipeline_native_fetch_classified_as_http_calls); + RUN_TEST(pipeline_swift_nested_url_makes_route_issue1892); RUN_TEST(pipeline_swift_http_call_makes_route_issue1892); RUN_TEST(pipeline_native_fetch_parallel_classified_as_http_calls); RUN_TEST(pipeline_local_fetch_shadow_not_classified_as_http); From 58c1c0caa7061ae868305dbe59a6c5e67004849d Mon Sep 17 00:00:00 2001 From: Anton Dziatkovskii <194927794+tonydzi@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:38:31 -0700 Subject: [PATCH 05/14] docs: MCP tool count is 17; document the three missing tools The TOOLS[] registry in src/mcp/mcp.c and the v0.11.0 --help output list 17 tools, but README, the npm README, CONTRIBUTING and the Pages site still say 15 (or 14), and the README tool table has no row for get_file_outline, compare_graphs or check_index_coverage. Refs #1297 Assisted-by: Claude Code / claude-opus-5 Machine: MacBook-Anton Account: a@gmail Operator: robot:git-s3-docs-fix-lane Signed-off-by: Anton Dziatkovskii <194927794+tonydzi@users.noreply.github.com> --- CONTRIBUTING.md | 2 +- README.md | 11 +++++++---- docs/index.html | 4 ++-- pkg/npm/README.md | 4 ++-- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 96ce8f861..12567ac82 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,7 +55,7 @@ src/ foundation/ Arena allocator, hash table, string utils, platform compat store/ SQLite graph storage (WAL mode, FTS5) cypher/ Cypher query → SQL translation - mcp/ MCP server (JSON-RPC 2.0 over stdio, 14 tools) + mcp/ MCP server (JSON-RPC 2.0 over stdio, 17 tools) pipeline/ Multi-pass indexing pipeline pass_*.c Individual pipeline passes (definitions, calls, usages, etc.) httplink.c HTTP route extraction (Go/Express/Laravel/Ktor/Python) diff --git a/README.md b/README.md index 3911bacf8..cc94d5194 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ **The fastest and most efficient code intelligence engine for AI coding agents.** Full-indexes an average repository in milliseconds, the Linux kernel (28M LOC, 75K files) in 3 minutes. Answers structural queries in under 1ms. Ships as a native executable with a small verified runtime-asset set for macOS, Linux, and Windows — download, run `install`, done. -High-quality parsing through [tree-sitter](https://tree-sitter.github.io/tree-sitter/) AST analysis across all 162 languages, enhanced with [**Hybrid LSP** semantic type resolution](#hybrid-lsp) for Python, TypeScript / JavaScript / JSX / TSX, PHP, C#, Go, C, C++, Java, Kotlin, Rust, and Perl — producing a persistent knowledge graph of functions, classes, call chains, HTTP routes, and cross-service links. 15 MCP tools. No language runtime, hosted service, or API key. Plug and play across 45 supported automatic/conditional client surfaces. +High-quality parsing through [tree-sitter](https://tree-sitter.github.io/tree-sitter/) AST analysis across all 162 languages, enhanced with [**Hybrid LSP** semantic type resolution](#hybrid-lsp) for Python, TypeScript / JavaScript / JSX / TSX, PHP, C#, Go, C, C++, Java, Kotlin, Rust, and Perl — producing a persistent knowledge graph of functions, classes, call chains, HTTP routes, and cross-service links. 17 MCP tools. No language runtime, hosted service, or API key. Plug and play across 45 supported automatic/conditional client surfaces. > **Research** — The design and benchmarks behind this project are described in the preprint [*Codebase-Memory: Tree-Sitter-Based Knowledge Graphs for LLM Code Exploration via MCP*](https://arxiv.org/abs/2603.27277) (arXiv:2603.27277). Evaluated across 31 real-world repositories: 83% answer quality, 10× fewer tokens, 2.1× fewer tool calls vs. file-by-file exploration. @@ -37,7 +37,7 @@ High-quality parsing through [tree-sitter](https://tree-sitter.github.io/tree-si - **45 supported automatic/conditional client surfaces** — `install` configures detected clients and safely activates conditional clients only when their documented platform, marker, or explicit existing config path is present. See [Multi-Agent Support](#multi-agent-support) for the complete matrix and manual/UI-only boundaries. - **Built-in graph visualization** — 3D interactive UI at `localhost:9749`, served from the binary itself. - **Infrastructure-as-code indexing** — Dockerfiles, Kubernetes manifests, and Kustomize overlays indexed as graph nodes with cross-references. `Resource` nodes for K8s kinds, `Module` nodes for Kustomize overlays with `IMPORTS` edges to referenced resources. -- **15 MCP tools** — search, trace, architecture, impact analysis, targeted index-coverage checks, Cypher queries, dead code detection, cross-service HTTP linking, ADR management, and more. +- **17 MCP tools** — search, trace, architecture, impact analysis, targeted index-coverage checks, Cypher queries, dead code detection, cross-service HTTP linking, ADR management, and more. ## Quick Start @@ -461,7 +461,7 @@ Add to `~/.claude.json` (user scope) or project `.mcp.json`: } ``` -Restart your agent. Verify with `/mcp` — you should see `codebase-memory-mcp` with 15 tools. +Restart your agent. Verify with `/mcp` — you should see `codebase-memory-mcp` with 17 tools. @@ -673,6 +673,7 @@ JSON arguments can also be piped on stdin, for tools that take arguments. A tool | `list_projects` | List all indexed projects with node/edge counts. | | `delete_project` | Remove a project and all its graph data. | | `index_status` | Check indexing status of a project. | +| `check_index_coverage` | Check whether exact paths or a scope are indexed and fresh. A clean result means no recorded gap, not proof of completeness. | ### Querying @@ -683,7 +684,9 @@ JSON arguments can also be piped on stdin, for tools that take arguments. A tool | `detect_changes` | Map git diff to affected symbols + blast radius with risk classification. | | `query_graph` | Execute Cypher-like graph queries (read-only). | | `get_graph_schema` | Node/edge counts, relationship patterns, property definitions per label. Run this first. | +| `compare_graphs` | Compare two indexed snapshots: node/edge additions and removals between a base and a target. | | `get_code_snippet` | Read source code for a function by qualified name. | +| `get_file_outline` | Declaration outline of one repository-relative file in source order, with optional label filter and paging. | | `get_architecture` | Codebase overview: languages, packages, routes, hotspots, clusters, ADR. | | `search_code` | Grep-like text search within indexed project files. | | `manage_adr` | CRUD for Architecture Decision Records (`get` reads, `update` replaces the whole document, `set_sections` rewrites only the named sections and leaves every other byte untouched, `sections` lists headings). Query modes do not wait behind a same-project reindex; writes remain serialized. | @@ -843,7 +846,7 @@ Also supported (not yet benchmarked): Ada, Agda, Apex, Assembly (NASM), Astro, A src/ main.c Entry point (MCP stdio server + CLI + install/update/config) daemon/ Per-account session coordination, IPC, lifecycle, shared jobs/watchers - mcp/ MCP server (15 tools, JSON-RPC 2.0, session detection, auto-index) + mcp/ MCP server (17 tools, JSON-RPC 2.0, session detection, auto-index) cli/ Install/uninstall/update/config (45 client surfaces, hooks, instructions) store/ SQLite graph storage (nodes, edges, traversal, search, Louvain) pipeline/ Multi-pass indexing (structure → definitions → calls → HTTP links → config → tests) diff --git a/docs/index.html b/docs/index.html index 56496d5d8..fdc1501a5 100644 --- a/docs/index.html +++ b/docs/index.html @@ -56,7 +56,7 @@ "featureList": [ "Indexes 158 programming languages via vendored tree-sitter grammars", "Hybrid LSP semantic type resolution for Python, TypeScript/JavaScript, PHP, C#, Go, C/C++, Java, Kotlin, and Rust", - "15 MCP tools for structural search, call-path tracing, targeted coverage checks, and Cypher graph queries", + "17 MCP tools for structural search, call-path tracing, targeted coverage checks, and Cypher graph queries", "Semantic vector code search via bundled nomic-embed-code embeddings (no API key, fully local)", "Semantic graph edges (SEMANTICALLY_RELATED) and near-clone detection (SIMILAR_TO, MinHash + LSH)", "Cross-service linking for HTTP, gRPC, GraphQL, tRPC, and pub/sub channels with confidence scoring", @@ -739,7 +739,7 @@

The optional UI variant adds one verified, content-addressed frontend pack and serves an interactive 3D graph at localhost:9749 to explore nodes, edges, and clusters visually.

-

15 MCP tools

+

17 MCP tools

search_graph, trace_path, detect_changes, query_graph (Cypher), get_architecture, get_code_snippet, check_index_coverage, manage_adr, and 7 more.

diff --git a/pkg/npm/README.md b/pkg/npm/README.md index 614b81b1c..e1e5abe3e 100644 --- a/pkg/npm/README.md +++ b/pkg/npm/README.md @@ -7,7 +7,7 @@ **The fastest and most efficient code intelligence engine for AI coding agents.** Full-indexes an average repository in milliseconds, the Linux kernel (28M LOC, 75K files) in 3 minutes. Answers structural queries in under 1ms. This npm wrapper downloads, verifies, and caches the selected native runtime set: the executable, its authenticated integration asset, and—when requested—the content-addressed UI pack. -High-quality parsing through [tree-sitter](https://tree-sitter.github.io/tree-sitter/) AST analysis across 162 languages — producing a persistent knowledge graph of functions, classes, call chains, HTTP routes, and cross-service links. 15 MCP tools. No hosted service or API key; this package requires Node.js to manage and launch the cached native runtime set. Plug and play across 45 automatic/conditional client surfaces. +High-quality parsing through [tree-sitter](https://tree-sitter.github.io/tree-sitter/) AST analysis across 162 languages — producing a persistent knowledge graph of functions, classes, call chains, HTTP routes, and cross-service links. 17 MCP tools. No hosted service or API key; this package requires Node.js to manage and launch the cached native runtime set. Plug and play across 45 automatic/conditional client surfaces. ## Installation @@ -38,7 +38,7 @@ Restart your agent. Say **"Index this project"** — done. - **Lifecycle hooks stay conservative** — Kimi uses `UserPromptSubmit`; on macOS/Linux, GitLab Duo gets a fail-open user `SessionStart`, while Devin gets `UserPromptSubmit`, `PostCompaction`, and a deduplicated `SessionStart` when Claude does not already provide it. Qoder, GitLab Duo, Devin, and Factory hooks are withheld on Windows without a documented shell/executor contract. Cline's auto-activating file hooks are withheld because their context output is not reliably consumed, CodeBuddy beta hooks are not auto-installed, and Cursor context hooks remain withheld. - **Subagent access is explicit** — Claude, Gemini, Kiro, Qwen, CodeBuddy, KiloCode, Mistral Vibe, Grok Build, Qoder, Junie, and Factory get documented graph profiles with the narrowest tool/server filters their schemas support. KiloCode and Vibe enumerate read-only query tools rather than using server wildcards. Cursor, Rovo, Pochi, and Cline use explicit parent handoff where child MCP is unavailable or unsafe; IBM Bob receives no invented hook or agent. - **Manual, UI, cloud, or repository-managed (not counted)** — Qodo, Warp MCP, JetBrains AI/ACP, GitHub Copilot coding agent, Jules, CodeRabbit, Replit, BLACKBOX AI, Plandex, and SWE-agent. Warp is counted above for its detected skill installation; its MCP connection remains manual. -- **15 MCP tools** — search, trace, architecture, impact analysis, Cypher queries, dead code detection, cross-service HTTP linking, ADR management, and more. +- **17 MCP tools** — search, trace, architecture, impact analysis, Cypher queries, dead code detection, cross-service HTTP linking, ADR management, and more. ## Supported Platforms From c067c771b9f93d08a3e614e68aee7d710d2f256c Mon Sep 17 00:00:00 2001 From: Anton Dziatkovskii <194927794+tonydzi@users.noreply.github.com> Date: Sat, 19 Sep 2026 15:38:36 -0700 Subject: [PATCH 06/14] docs: Pages card says "and 9 more" so the list sums to 17 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card headline already reads "17 MCP tools" but the body named eight tools and then said "and 7 more" — 8 + 7 = 15, the old count. TOOL_ANNOTATIONS[] in src/mcp/mcp.c has 17 entries, so the tail is 9. Assisted-by: Claude Code / claude-opus-5 Machine: MacBook-Anton Account: a@gmail Operator: robot:pr-reply-daily Signed-off-by: Anton Dziatkovskii <194927794+tonydzi@users.noreply.github.com> --- docs/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.html b/docs/index.html index fdc1501a5..fdfde941e 100644 --- a/docs/index.html +++ b/docs/index.html @@ -740,7 +740,7 @@

17 MCP tools

-

search_graph, trace_path, detect_changes, query_graph (Cypher), get_architecture, get_code_snippet, check_index_coverage, manage_adr, and 7 more.

+

search_graph, trace_path, detect_changes, query_graph (Cypher), get_architecture, get_code_snippet, check_index_coverage, manage_adr, and 9 more.

Cypher graph queries

From f428d583008ce87281bac138c02bd4375c56d735 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 19:56:36 +0200 Subject: [PATCH 07/14] docs(contributing): state the terms for AI-assisted and AI-authored contributions CONTRIBUTING.md said nothing about contributions written by or with an AI agent, and the project now receives both kinds: PRs from accounts that openly identify as an agent acting for a person (#2225 is one, and a good one), and, in September, an account that posted approvals and change requests styled as maintainer decisions across nineteen threads. The first deserves a clear welcome; the second needs a rule that was until now only enforced, never written down. The new section says: disclosed AI authorship is welcome and never held against a change; the DCO sign-off is a legal certification only a person can make, so a human is accountable for every commit; the evidence bar is the same for everyone -- a claim is something a reviewer can check. Co-authored-by trailers naming a tool are fine, links to private agent sessions are not. High-volume low-effort submissions are closed whoever produced them. A second subsection states who speaks for the project -- the maintainer account only -- and that impersonating a maintainer, by a person or an agent, gets the reviews dismissed, the comments hidden, a public correction in the affected threads, and the account reported and blocked. That is what was done in September; it is now written where a contributor can read it first. Signed-off-by: Martin Vogel --- CONTRIBUTING.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 96ce8f861..ff969c483 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -149,6 +149,25 @@ If in doubt, open an issue and ask. - Run `scripts/test.sh` and `scripts/lint.sh` before submitting - Keep PRs focused — avoid unrelated reformatting or refactoring +## AI-assisted and AI-authored contributions + +Contributions written with the help of an AI tool, or written by an AI agent acting for a person, are welcome here on the same terms as any other. What we review is the change, not who or what typed it. Three things make that work: + +- **Say so.** If an agent wrote the PR or is answering in the thread, disclose it — a line in the PR description or at the top of a comment is enough ("written with X", "this account is operated by an AI agent on behalf of Y"). Disclosure is never held against a contribution. Finding out later is. +- **A person is accountable.** The [DCO sign-off](#license-and-sign-off-dco--required-on-every-commit) is a legal certification, and only a person can make it. The human behind the account certifies every commit, answers for it, and is who we talk to if something goes wrong. An agent may carry the conversation; it cannot carry the responsibility. +- **The bar does not move.** Every claim in a PR must be something a reviewer can check: the failing test, the command that was run, the numbers and the machine they came from. "The model says it works" is not evidence, and a confident description does not replace a test that fails without the fix. We read AI-written PRs exactly as carefully as human-written ones — no more suspiciously, and no less. + +Two practical notes: + +- `Co-authored-by:` trailers naming a tool are fine. Links to private chat or agent sessions are not — they are dead to everyone else and do not belong in permanent history; we will ask you to remove them. +- High-volume, low-effort submissions — many near-identical PRs, changes nobody ran, replies that do not engage with the review — will be closed, whoever or whatever produced them. One careful PR is worth more than twenty generated ones. + +### Who speaks for the project + +Only the maintainer account ([@DeusData](https://github.com/DeusData)) speaks for this project. A review, an approval, a "this will be merged" or a "this is a duplicate" from any other account is that person's opinion, however official it sounds — helpful reviews from the community are very welcome, and that is what they are. + +**Impersonating a maintainer is not acceptable**, by a person or by an agent: presenting yourself as part of the maintainer team, posting approvals or change requests styled as maintainer decisions, or telling other contributors what the project has decided. We dismiss such reviews, hide the comments, say publicly in the affected threads that they were not ours, and report and block the account. If you are unsure whether a response you received is from the project, check the account name — and ask. + ## Security We take security seriously. All PRs go through: From a262a35724b3246353f07b5de96ec4731681dd4b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 19:16:19 +0200 Subject: [PATCH 08/14] ci(pr): classify changed files from the merge ref when the files API 422s The `changes` job decides whether a PR touches product code, and ci-ok requires it. It asks GitHub for the PR's file list. An earlier fix moved it off the .diff endpoint, which rejects PRs over 20k lines, onto the paginated pulls/files endpoint -- but that endpoint renders the diff server-side as well, and for a PR that regenerates a vendored parser it answers HTTP 422: Sorry, this diff is taking too long to generate. Observed on #2246 (a 42 MB sql/parser.c): `changes` red, therefore ci-ok red, for a reason that has nothing to do with the contribution. The compare endpoint fails the same way, so there is no API route to the file list for such a PR. Every grammar refresh would hit this. When the API call fails, the job now takes the list from git instead. The PR merge ref's first parent is the base, so a name-only diff across that one merge commit is exactly the PR's changed files. The fetch is depth 2 with --filter=blob:none into a bare repository under RUNNER_TEMP: commits and trees only, no blobs, nothing checked out and nothing from the PR executed. No new action, no new permission, and the API path and the classification regex are untouched, so ordinary PRs behave as before. A ::notice:: line records when the fallback was used. If the fetch fails too, the step exits non-zero and writes no output -- the gate fails closed, as it did before. Proof, by executing the workflow's own run block under `bash -eo pipefail` with a stub gh: API path, docs-only list rc 0 product=false fallback, real #2246 rc 0 product=true, the same five files `git diff --numstat` reports for the PR; 0.85 s, 188 KB fetched fallback, nonexistent merge ref rc 128 no output written Contract tests that read pr.yml -- security_gate_fail_closed, smoke_fixture_contract, windows_bundle_contract, venue_parity_contract -- pass. Not addressed here, noted for a follow-up: the classification pipes the list into `grep -q` under pipefail, which can report a match as a failure once the list outgrows the pipe buffer. Signed-off-by: Martin Vogel --- .github/workflows/pr.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0cfc796be..6e5691423 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -64,7 +64,23 @@ jobs: run: | # The full .diff endpoint rejects large-but-valid PRs at 20k lines. # The paginated files endpoint remains filename-only for this gate. - FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename') + if FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename'); then + echo "file list: pulls/files API" + else + # The files endpoint renders the diff server-side too, and answers + # 422 "this diff is taking too long to generate" for a PR that + # regenerates a vendored parser (#2246: a 42 MB sql/parser.c) -- + # so does the compare endpoint. The PR merge ref's first parent is + # the base, so a name-only diff across it is the same file list. + # depth 2 + blob:none fetches commits and trees only (well under + # 1 MB, under a second); nothing from the PR is checked out or run. + echo "::notice::pulls/files API failed; file list taken from refs/pull/$PR/merge" + CHANGES_GIT="$RUNNER_TEMP/changes.git" + git init -q --bare "$CHANGES_GIT" + git --git-dir="$CHANGES_GIT" fetch -q --depth=2 --filter=blob:none \ + "https://github.com/$REPO" "refs/pull/$PR/merge" + FILES=$(git --git-dir="$CHANGES_GIT" diff --name-only --no-renames FETCH_HEAD^1 FETCH_HEAD) + fi printf '%s\n' "$FILES" if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then echo "product=true" >> "$GITHUB_OUTPUT" From 65d60e72f5b86f5982b15f0d88826ba79a085aa2 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 19:34:37 +0200 Subject: [PATCH 09/14] ci(pr): the changes classifier no longer fails open on a large file list Found while proving the previous commit, in the same step, and the worse of the two defects because it fails OPEN. The classification was if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|...)'; then under the runner's `bash -eo pipefail`. grep -q exits on its first match; once the list is larger than the pipe buffer printf is still writing, dies of SIGPIPE, and pipefail turns the MATCH into a failed pipeline. The else branch then writes product=false, and pr-smoke and memwaste are skipped -- on exactly the PRs that change the most files. The previous commit's message flagged this as a follow-up; it is fixed here instead. The list now reaches grep through a here-string, so there is no pipe to break. The regex is unchanged. Proof, by executing the workflow's own run block under `bash -eo pipefail` with a stub gh, ten runs each: 849 KB list, first line src/a.c before: product=false 10/10 after: product=true 10/10 849 KB list, docs only product=false before and after small docs-only list product=false 422 fallback on the real #2246 product=true fallback, nonexistent merge ref rc 128, no output written It takes roughly 1,500 changed files to cross the buffer, which is why ordinary PRs never showed it. Signed-off-by: Martin Vogel --- .github/workflows/pr.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 6e5691423..8cb464b19 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -82,7 +82,11 @@ jobs: FILES=$(git --git-dir="$CHANGES_GIT" diff --name-only --no-renames FETCH_HEAD^1 FETCH_HEAD) fi printf '%s\n' "$FILES" - if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then + # A here-string, not a pipe: under this shell's pipefail, grep -q + # exits on its first match, printf dies of SIGPIPE once the list + # outgrows the pipe buffer, and the MATCH is then reported as a + # failure -- product=false, smoke skipped, on the largest PRs. + if grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)' <<<"$FILES"; then echo "product=true" >> "$GITHUB_OUTPUT" else echo "product=false" >> "$GITHUB_OUTPUT" From c314dea4c1409a3a68fb33706b49e272809b5079 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 19:29:00 +0200 Subject: [PATCH 10/14] fix(service-patterns): match library ids on identifier boundaries Distilled from #1245 by Andrew Hundt (d15071c9, the match_qn half). match_qn was a raw strstr(qn, library_id). Short ids fired inside unrelated words, and a service kind REPLACES the plain CALLS edge, so a false match both invents a service edge and deletes a real call: "gin." in plugin. -> route registration; any "/"-prefixed first argument then minted a Route node "nconf" in encryptionconfig -> CONFIGURES for every call into the package "tonic" in Monotonic -> gRPC; unparseable, so NO edge at all "express" in expression -> route registration "get_env" in get_envelope, get_environ -> CONFIGURES "dio" "surf" "curl" "resty" "phin" "hyper" "treq" "rocket" in studio, surface, curly, restyle, dolphin, hyperbolic, streq, rocketmq The rule (qn_hit_on_boundary; every occurrence of an id is tried, not only the first). A separator is any character that is not an ASCII letter or digit. before: start of string | separator | the id itself starts with a separator ("@trpc/server") | the id starts with an uppercase letter -- a capital opens a new CamelCase word whatever precedes it (AsyncHttpClient, IHttpClientFactory, NSURLSession) after: end of string | separator | the id itself ends in a separator ("gin.") | uppercase letter (GuzzleHttp, FeignClient, KafkaProducer) | digit (urllib2, Mint.HTTP2, amqp091-go) Rejected: a lowercase- or digit-initial id glued to a preceding letter or digit, and any id continued by a lowercase letter. Two deliberate differences from the upstream hunk: upstream's before-rule needed a GuzzleHttp table entry and would have dropped IHttpClientFactory and NSURLSession, and without the digit after-boundary urllib2 and amqp091-go stop matching. All three match on main today. No recall is traded for this. No boundary rule can tell "grequests" from "myrequests", so a library whose own name glues a prefix or suffix onto another id gets an explicit entry with the kind (and broker) it had on main -- 47 of them, found mechanically by running main's matcher and the new one over 173 candidate names and listing every one that went from a kind to NONE: HTTP grequests txrequests redaxios gaxios libcurl curlpp curlcpp hyperlocal guzzlehttp ASYNC aiokafka pykafka rskafka librdkafka rdkafkacpp aioamqp amqpstorm pamqp pyamqp amqprs amqpcpp pynats jnats aiomqtt amqtt hbmqtt umqtt mqttools emqtt rumqtt gomqtt libmosquitto mosquittopp pubsublite gocelery CONFIG wgetenv getenvb qgetenv dotenvx phpdotenv ROUTE_REG apiflask honox fasthttprouter GRAPHQL gqlparser gqlgenc aiogqlc Left at NONE on purpose: names that are not clients of the id they contain and were misclassified on main -- hypercorn, hyperlink, hyperopt, openresty, gqlalchemy, celeryconfig, kafkacat, gnatsd, pypubsub. One visible reclassification: hyperium/tonic was HTTP through "hyper" inside "hyperium" and is now gRPC through "tonic". RED, production reverted and the final tests kept: infrascan_service_pattern_match_rejects_ids_inside_words FAIL tests/test_infrascan.c:105: svc_case_mismatches(cases) == 28, expected 0 == 0 4 passed, 1 failed All 28 negatives mismatch on main. The 47 glued-library positives are RED the other way round: against the boundary rule without the entries. Each positive uses a name containing no second id, so it binds to its own entry -- "aiokafka.AIOKafkaProducer" would have passed through "KafkaProducer" and proven nothing. GREEN: infrascan 5, pipeline 281, parallel 74, extraction 350, edge_types_probe 59, route_canon 11, lang_contract 41, cross_repo 8, registry 64, mcp 318 (4 Windows-only skips). 0 failed. Effect on real code -- production binaries, main 92abefa3 against this change. main against main differs by 0 edges and 0 nodes on all three corpora, so every delta below is the change: django CONFIGURES 76 -> 74; both calls return as CALLS (geom.get_envelope, WSGIRequestHandler.get_environ) typescript CONFIGURES 12,026 -> 12,024; one bogus Route "/*type*/" gone with its CALLS and HANDLES; 3 calls return as CALLS kubernetes CONFIGURES 7,962 -> 7,609: all 353 are "nconf" inside encryptionconfig / authorizationconfig / authenticationconfig, and all 353 return as CALLS. 8 bogus Routes gone -- os.ReadFile of the serviceaccount namespace file, t.Run("/TwoWay"), "/tmp/test" -- with 26 CALLS, 2 HANDLES and 21 TESTS edges that pointed at them. 374 plain CALLS restored; 8 of them (validate.Monotonic and friends) had NO edge at all on main. GRPC_CALLS 60 -> 60. HTTP_CALLS 398 -> 407: nine calls main swallowed as route registrations now reach the existing arg_url heuristic like every other package. 88 of the 408 removed service-derived edges were read at the call site -- all of django and typescript, 82 of 402 in kubernetes: 88 false positives, 0 true positives lost. None of the three corpora uses a glued-name library, so the 47 entries are covered by the unit test only. Known limits: "_" is a separator, so optical_fiber.len still matches "fiber." exactly as requests_get matches "requests" by design; a lowercase id followed by a capital matches (kafkaProducer), which also admits a dioXide-style name; a user wrapper glued without a separator (mygetenv) is no longer classified; a glued-name library missing from the list is NONE until someone adds it. Co-authored-by: Andrew Hundt Signed-off-by: Martin Vogel --- internal/cbm/service_patterns.c | 124 ++++++++++++++++++++-- tests/test_infrascan.c | 183 +++++++++++++++++++++++++++++++- 2 files changed, 295 insertions(+), 12 deletions(-) diff --git a/internal/cbm/service_patterns.c b/internal/cbm/service_patterns.c index 0dd404877..c2425e09e 100644 --- a/internal/cbm/service_patterns.c +++ b/internal/cbm/service_patterns.c @@ -140,6 +140,20 @@ static const lib_pattern_t http_libraries[] = { {"socket.http", CBM_SVC_HTTP, NULL}, {"resty.http", CBM_SVC_HTTP, NULL}, + /* Glued-name libraries. match_qn needs an identifier boundary around an + * id, so a library whose own name glues a lowercase prefix/suffix onto + * another id ("grequests", "curlpp") is listed explicitly: no boundary + * rule can tell "grequests" from "myrequests". */ + {"grequests", CBM_SVC_HTTP, NULL}, /* Python: gevent + requests */ + {"txrequests", CBM_SVC_HTTP, NULL}, /* Python: Twisted + requests */ + {"redaxios", CBM_SVC_HTTP, NULL}, /* JS: axios-compatible fetch client */ + {"gaxios", CBM_SVC_HTTP, NULL}, /* JS: Google's axios-style client */ + {"libcurl", CBM_SVC_HTTP, NULL}, /* C, node-libcurl */ + {"curlpp", CBM_SVC_HTTP, NULL}, /* C++ libcurl wrapper */ + {"curlcpp", CBM_SVC_HTTP, NULL}, /* C++ libcurl wrapper */ + {"hyperlocal", CBM_SVC_HTTP, NULL}, /* Rust: hyper over unix sockets */ + {"guzzlehttp", CBM_SVC_HTTP, NULL}, /* PHP: Guzzle's composer vendor */ + {NULL, CBM_SVC_NONE, NULL}, }; @@ -263,6 +277,33 @@ static const lib_pattern_t async_libraries[] = { {"dapr.clients.grpc", CBM_SVC_ASYNC, "dapr"}, {"DaprClient", CBM_SVC_ASYNC, "dapr"}, + /* Glued-name libraries (see the note in http_libraries). */ + {"aiokafka", CBM_SVC_ASYNC, "kafka"}, + {"pykafka", CBM_SVC_ASYNC, "kafka"}, + {"librdkafka", CBM_SVC_ASYNC, "kafka"}, + {"rdkafkacpp", CBM_SVC_ASYNC, "kafka"}, + {"rskafka", CBM_SVC_ASYNC, "kafka"}, + {"aioamqp", CBM_SVC_ASYNC, "rabbitmq"}, + {"amqpstorm", CBM_SVC_ASYNC, "rabbitmq"}, + {"pamqp", CBM_SVC_ASYNC, "rabbitmq"}, + {"pyamqp", CBM_SVC_ASYNC, "rabbitmq"}, + {"amqprs", CBM_SVC_ASYNC, "rabbitmq"}, + {"amqpcpp", CBM_SVC_ASYNC, "rabbitmq"}, + {"pynats", CBM_SVC_ASYNC, "nats"}, + {"jnats", CBM_SVC_ASYNC, "nats"}, + {"aiomqtt", CBM_SVC_ASYNC, "mqtt"}, + {"amqtt", CBM_SVC_ASYNC, "mqtt"}, + {"hbmqtt", CBM_SVC_ASYNC, "mqtt"}, + {"umqtt", CBM_SVC_ASYNC, "mqtt"}, + {"mqttools", CBM_SVC_ASYNC, "mqtt"}, + {"emqtt", CBM_SVC_ASYNC, "mqtt"}, + {"rumqtt", CBM_SVC_ASYNC, "mqtt"}, + {"gomqtt", CBM_SVC_ASYNC, "mqtt"}, + {"libmosquitto", CBM_SVC_ASYNC, "mqtt"}, + {"mosquittopp", CBM_SVC_ASYNC, "mqtt"}, + {"pubsublite", CBM_SVC_ASYNC, "pubsub"}, + {"gocelery", CBM_SVC_ASYNC, "celery"}, + {NULL, CBM_SVC_NONE, NULL}, }; @@ -308,6 +349,13 @@ static const lib_pattern_t config_libraries[] = { {"Application.get_env", CBM_SVC_CONFIG, NULL}, {"Application.fetch_env", CBM_SVC_CONFIG, NULL}, + /* Glued-name accessors (see the note in http_libraries). */ + {"wgetenv", CBM_SVC_CONFIG, NULL}, /* Windows CRT: _wgetenv, _wgetenv_s */ + {"getenvb", CBM_SVC_CONFIG, NULL}, /* Python: os.getenvb */ + {"qgetenv", CBM_SVC_CONFIG, NULL}, /* Qt */ + {"dotenvx", CBM_SVC_CONFIG, NULL}, /* JS: @dotenvx/dotenvx */ + {"phpdotenv", CBM_SVC_CONFIG, NULL}, /* PHP: vlucas/phpdotenv */ + {NULL, CBM_SVC_NONE, NULL}, }; @@ -377,6 +425,11 @@ static const lib_pattern_t route_reg_libraries[] = { {"akka.http.scaladsl.server", CBM_SVC_ROUTE_REG, NULL}, {"play.api.routing", CBM_SVC_ROUTE_REG, NULL}, + /* Glued-name frameworks (see the note in http_libraries). */ + {"apiflask", CBM_SVC_ROUTE_REG, NULL}, /* Python: Flask-based */ + {"honox", CBM_SVC_ROUTE_REG, NULL}, /* JS: Hono meta-framework */ + {"fasthttprouter", CBM_SVC_ROUTE_REG, NULL}, /* Go: httprouter for fasthttp */ + {NULL, CBM_SVC_NONE, NULL}, }; @@ -448,6 +501,11 @@ static const lib_pattern_t graphql_libraries[] = { {"async-graphql", CBM_SVC_GRAPHQL, NULL}, {"juniper", CBM_SVC_GRAPHQL, NULL}, + /* Glued-name libraries (see the note in http_libraries). */ + {"gqlparser", CBM_SVC_GRAPHQL, NULL}, /* Go: vektah/gqlparser */ + {"gqlgenc", CBM_SVC_GRAPHQL, NULL}, /* Go: gqlgen client generator */ + {"aiogqlc", CBM_SVC_GRAPHQL, NULL}, /* Python: asyncio GraphQL client */ + {NULL, CBM_SVC_NONE, NULL}, }; @@ -534,17 +592,67 @@ static const method_suffix_t method_suffixes[] = { /* ── Matching implementation ───────────────────────────────────── */ -/* Check if any library identifier appears as a substring in the QN. - * Case-sensitive: "requests" matches "project.venv.requests.api.get" - * but not "Requests". Library names are specific enough to avoid - * false positives even with substring matching. */ +static bool qn_is_lower(char ch) { + return ch >= 'a' && ch <= 'z'; +} + +static bool is_digit_char(char ch) { + return ch >= '0' && ch <= '9'; +} + +static bool qn_is_alnum(char ch) { + return qn_is_lower(ch) || is_digit_char(ch) || (ch >= 'A' && ch <= 'Z'); +} + +/* True when the occurrence of `id` at `hit` sits on identifier boundaries. + * + * A raw substring match let short ids fire inside unrelated words: "gin." + * in "plugin.", "dio" in "studio", "surf" in "surface", "express" in + * "expression". Every character that is not an ASCII letter or digit is a + * separator ('.', '/', '\\', ':', '_', '-', '$', '@', ...). + * + * BEFORE: start of string, a separator, an id that itself starts with a + * separator ("@trpc/server"), or an id that starts with an + * uppercase letter — a capital opens a new CamelCase word whatever + * precedes it ("AsyncHttpClient", "IHttpClientFactory", + * "NSURLSession"). Rejected: a lowercase/digit-initial id glued to + * a letter or digit ("plugin." / "studio" / "myrequests"). + * AFTER: end of string, a separator, an id that itself ends in a + * separator ("gin."), an uppercase letter (CamelCase glue: + * "GuzzleHttp", "FeignClient", "KafkaProducer", "kafkaProducer") + * or a digit (version suffix: "urllib2", "amqp091-go"). + * Rejected: a lowercase letter continuing the word ("surface", + * "curly", "expression"). */ +static bool qn_hit_on_boundary(const char *qn, const char *hit, const char *id, size_t id_len) { + char first = id[0]; + char last = id[id_len - 1]; + if (hit > qn && qn_is_alnum(hit[-1]) && (qn_is_lower(first) || is_digit_char(first))) { + return false; + } + if (qn_is_alnum(last) && qn_is_lower(hit[id_len])) { + return false; + } + return true; +} + +/* Check if any library identifier appears in the QN on identifier + * boundaries (see qn_hit_on_boundary). Case-sensitive: "requests" matches + * "project.venv.requests.api.get" and "svc.requests_get" but neither + * "Requests" nor "myrequests". */ static const lib_pattern_t *match_qn(const char *qn, const lib_pattern_t *patterns) { if (!qn || !qn[0]) { return NULL; } for (int i = 0; patterns[i].library_id != NULL; i++) { - if (strstr(qn, patterns[i].library_id) != NULL) { - return &patterns[i]; + const char *id = patterns[i].library_id; + size_t id_len = strlen(id); + if (id_len == 0) { + continue; + } + for (const char *hit = strstr(qn, id); hit != NULL; hit = strstr(hit + 1, id)) { + if (qn_hit_on_boundary(qn, hit, id, id_len)) { + return &patterns[i]; + } } } return NULL; @@ -574,10 +682,6 @@ static bool contains_segment(const char *path, const char *segment) { return false; } -static bool is_digit_char(char ch) { - return ch >= '0' && ch <= '9'; -} - static bool has_http_route_marker(const char *path) { if (starts_with_segment(path, "api") || starts_with_segment(path, "apis") || starts_with_segment(path, "graphql") || starts_with_segment(path, "health") || diff --git a/tests/test_infrascan.c b/tests/test_infrascan.c index cd7df945d..a9bf1aef9 100644 --- a/tests/test_infrascan.c +++ b/tests/test_infrascan.c @@ -1,11 +1,11 @@ #include "test_framework.h" #include "graph_buffer/graph_buffer.h" #include "pipeline/pipeline_internal.h" +#include "service_patterns.h" #include #include - -bool cbm_service_pattern_is_http_route_literal(const char *literal, const char *callee_name); +#include static int has_data_flow(cbm_gbuf_t *gb, int64_t source_id, int64_t target_id) { const cbm_gbuf_edge_t **edges = NULL; @@ -48,6 +48,183 @@ TEST(infrascan_http_route_literal_guard_rejects_filesystem_paths) { PASS(); } +/* ── Service-pattern QN boundaries (distilled from PR #1245) ───────── + * A library id must sit on an identifier boundary inside the QN. A raw + * substring match classified "proj.plugin.loader" as gin ("gin.") and + * "proj.studio.x" as Dio ("dio"), which minted bogus Route / HTTP_CALLS + * edges. The rule must stay open for CamelCase glue ("GuzzleHttp", + * "FeignClient", "KafkaProducer", "IHttpClientFactory") and for version + * suffixes ("urllib2", "amqp091-go"). */ +typedef struct { + const char *qn; + cbm_svc_kind_t want; +} svc_case_t; + +static int svc_case_mismatches(const svc_case_t *cases) { + int bad = 0; + for (int i = 0; cases[i].qn != NULL; i++) { + cbm_svc_kind_t got = cbm_service_pattern_match(cases[i].qn); + if (got != cases[i].want) { + printf(" service-pattern mismatch: \"%s\" -> %d, expected %d\n", cases[i].qn, (int)got, + (int)cases[i].want); + bad++; + } + } + return bad; +} + +TEST(infrascan_service_pattern_match_rejects_ids_inside_words) { + static const svc_case_t cases[] = { + /* "gin." inside plugin. */ + {"proj.plugins.autorun.tests.test_plugin._dispatch", CBM_SVC_NONE}, + {"proj.plugin.loader", CBM_SVC_NONE}, + /* "chi." / "echo." / "fiber." */ + {"proj.hibachi.grill", CBM_SVC_NONE}, + {"sound.reecho.apply", CBM_SVC_NONE}, + {"textile.microfiber.weave", CBM_SVC_NONE}, + /* "dio" inside studio / radio */ + {"proj.studio.x", CBM_SVC_NONE}, + {"radio.tune", CBM_SVC_NONE}, + /* "surf" / "resty" / "curl" / "phin" / "hyper" / "sling" / "needle" / "treq" */ + {"app.surface.draw", CBM_SVC_NONE}, + {"ui.restyle.apply", CBM_SVC_NONE}, + /* control: "forestry" spells "restr", never contained "resty" */ + {"pkg.forestry.trees", CBM_SVC_NONE}, + {"util.curly.brace", CBM_SVC_NONE}, + {"sea.dolphin.swim", CBM_SVC_NONE}, + {"math.hyperbolic.tanh", CBM_SVC_NONE}, + {"farm.gosling.walk", CBM_SVC_NONE}, + {"sewing.needles.sort", CBM_SVC_NONE}, + {"util.streq", CBM_SVC_NONE}, + /* "requests" glued to a lowercase prefix (upstream's case) */ + {"proj.myrequests.client.get", CBM_SVC_NONE}, + /* "express" / "rocket" / "tonic" / "nconf" */ + {"ast.expression.eval", CBM_SVC_NONE}, + {"mq.rocketmq.send", CBM_SVC_NONE}, + {"geo.tectonic.plates", CBM_SVC_NONE}, + {"app.unconfigured.warn", CBM_SVC_NONE}, + {"cli.runconfig.load", CBM_SVC_NONE}, + {"aws.s3express.session", CBM_SVC_NONE}, + /* not clients of the id they contain: ASGI server, URL library, + * nginx platform, Memgraph OGM, Celery settings module, a word */ + {"hypercorn.run.serve", CBM_SVC_NONE}, + {"hyperlink.URL.from_text", CBM_SVC_NONE}, + {"openresty.core.request", CBM_SVC_NONE}, + {"gqlalchemy.Memgraph.execute", CBM_SVC_NONE}, + {"celeryconfig.broker_url", CBM_SVC_NONE}, + {"kafkaesque.story.tell", CBM_SVC_NONE}, + {NULL, CBM_SVC_NONE}, + }; + ASSERT_EQ(svc_case_mismatches(cases), 0); + PASS(); +} + +TEST(infrascan_service_pattern_match_keeps_real_library_qns) { + static const svc_case_t cases[] = { + /* Route registration: ids that end in a separator, own segment */ + {"github.com/gin-gonic/gin.Default", CBM_SVC_ROUTE_REG}, + {"proj.gin.router.GET", CBM_SVC_ROUTE_REG}, + {"gin.GET", CBM_SVC_ROUTE_REG}, + {"chi.NewRouter", CBM_SVC_ROUTE_REG}, + {"github.com/labstack/echo.New", CBM_SVC_ROUTE_REG}, + {"fiber.New", CBM_SVC_ROUTE_REG}, + {"proj.express.router.get", CBM_SVC_ROUTE_REG}, + {"@hapi/hapi.server.route", CBM_SVC_ROUTE_REG}, + /* HTTP: own segment / separator-joined wrapper */ + {"proj.venv.requests.api.get", CBM_SVC_HTTP}, + {"proj.service.requests_get", CBM_SVC_HTTP}, + {"package:dio/dio.dart", CBM_SVC_HTTP}, + {"dio.Dio.get", CBM_SVC_HTTP}, + {"pkg.net.curl.get", CBM_SVC_HTTP}, + {"curl_exec", CBM_SVC_HTTP}, + {"surf.get", CBM_SVC_HTTP}, + {"hyper.Client.request", CBM_SVC_HTTP}, + {"phin.promisified", CBM_SVC_HTTP}, + {"needle.get", CBM_SVC_HTTP}, + {"github.com/go-resty/resty.New", CBM_SVC_HTTP}, + {"github.com/dghubble/sling.New", CBM_SVC_HTTP}, + /* HTTP: CamelCase glue after the id */ + {"GuzzleHttp\\Client", CBM_SVC_HTTP}, + {"proj.GuzzleHttp.Client.get", CBM_SVC_HTTP}, + {"org.springframework.cloud.openfeign.FeignClientBuilder.build", CBM_SVC_HTTP}, + {"com.acme.net.HttpClientFactory.create", CBM_SVC_HTTP}, + /* HTTP: CamelCase glue before the id */ + {"System.Net.Http.IHttpClientFactory.CreateClient", CBM_SVC_HTTP}, + {"org.asynchttpclient.AsyncHttpClient.prepareGet", CBM_SVC_HTTP}, + {"Foundation.NSURLSession.dataTask", CBM_SVC_HTTP}, + /* HTTP: version suffix after the id */ + {"urllib2.urlopen", CBM_SVC_HTTP}, + {"Mint.HTTP2.request", CBM_SVC_HTTP}, + /* Async */ + {"com.acme.events.KafkaProducer.send", CBM_SVC_ASYNC}, + {"this.kafkaProducer.send", CBM_SVC_ASYNC}, + {"org.apache.kafka.clients.producer.KafkaProducer.send", CBM_SVC_ASYNC}, + {"github.com/rabbitmq/amqp091-go.Channel.Publish", CBM_SVC_ASYNC}, + {"github.com/nats-io/nats.go.Conn.Publish", CBM_SVC_ASYNC}, + /* Config */ + {"os.getenv", CBM_SVC_CONFIG}, + {"os.Getenv", CBM_SVC_CONFIG}, + /* Glued-name libraries: the library's own name glues a lowercase + * prefix/suffix onto another id, so each has an explicit table entry. + * Every QN below contains NO other id — only the entry can match. */ + {"grequests.map", CBM_SVC_HTTP}, + {"txrequests.Session.send", CBM_SVC_HTTP}, + {"redaxios.create", CBM_SVC_HTTP}, + {"gaxios.request", CBM_SVC_HTTP}, + {"libcurl.version", CBM_SVC_HTTP}, + {"node-libcurl.Easy.perform", CBM_SVC_HTTP}, + {"curlpp.Easy.perform", CBM_SVC_HTTP}, + {"curlcpp.easy.perform", CBM_SVC_HTTP}, + {"hyperlocal.UnixConnector.call", CBM_SVC_HTTP}, + {"guzzlehttp/psr7.Utils.streamFor", CBM_SVC_HTTP}, + {"aiokafka.helpers.create_ssl_context", CBM_SVC_ASYNC}, + {"pykafka.topic.Topic.get_producer", CBM_SVC_ASYNC}, + {"librdkafka.rd_version", CBM_SVC_ASYNC}, + {"rdkafkacpp.version", CBM_SVC_ASYNC}, + {"rskafka.client.partition.write", CBM_SVC_ASYNC}, + {"aioamqp.connect", CBM_SVC_ASYNC}, + {"amqpstorm.Connection.channel", CBM_SVC_ASYNC}, + {"pamqp.frame.marshal", CBM_SVC_ASYNC}, + {"kombu.transport.pyamqp.Transport.establish_connection", CBM_SVC_ASYNC}, + {"amqprs.channel.basic_publish", CBM_SVC_ASYNC}, + {"amqpcpp.TcpChannel.publish", CBM_SVC_ASYNC}, + {"pynats.client.send", CBM_SVC_ASYNC}, + {"jnats.options.build", CBM_SVC_ASYNC}, + {"aiomqtt.client.publish", CBM_SVC_ASYNC}, + {"amqtt.session.deliver", CBM_SVC_ASYNC}, + {"hbmqtt.session.deliver", CBM_SVC_ASYNC}, + {"umqtt.simple.publish", CBM_SVC_ASYNC}, + {"mqttools.client.publish", CBM_SVC_ASYNC}, + {"emqtt.publish", CBM_SVC_ASYNC}, + {"rumqtt.client.publish", CBM_SVC_ASYNC}, + {"github.com/256dpi/gomqtt/client.Publish", CBM_SVC_ASYNC}, + {"libmosquitto.publish", CBM_SVC_ASYNC}, + {"mosquittopp.publish", CBM_SVC_ASYNC}, + {"google.cloud.pubsublite.admin.create_topic", CBM_SVC_ASYNC}, + {"gocelery.client.delay", CBM_SVC_ASYNC}, + {"_wgetenv", CBM_SVC_CONFIG}, + {"_wgetenv_s", CBM_SVC_CONFIG}, + {"os.getenvb", CBM_SVC_CONFIG}, + {"qgetenv", CBM_SVC_CONFIG}, + {"@dotenvx/dotenvx.config", CBM_SVC_CONFIG}, + {"vlucas/phpdotenv.load", CBM_SVC_CONFIG}, + {"apiflask.scaffold.get", CBM_SVC_ROUTE_REG}, + {"honox.factory.createRoute", CBM_SVC_ROUTE_REG}, + {"fasthttprouter.Router.GET", CBM_SVC_ROUTE_REG}, + {"github.com/vektah/gqlparser/v2.LoadSchema", CBM_SVC_GRAPHQL}, + {"github.com/Yamashou/gqlgenc.client.Post", CBM_SVC_GRAPHQL}, + {"aiogqlc.client.execute", CBM_SVC_GRAPHQL}, + {NULL, CBM_SVC_NONE}, + }; + ASSERT_EQ(svc_case_mismatches(cases), 0); + ASSERT_STR_EQ(cbm_service_pattern_broker("aiokafka.helpers.create_ssl_context"), "kafka"); + ASSERT_STR_EQ(cbm_service_pattern_broker("umqtt.simple.publish"), "mqtt"); + ASSERT_STR_EQ(cbm_service_pattern_broker("com.acme.events.KafkaProducer.send"), "kafka"); + ASSERT_STR_EQ(cbm_service_pattern_broker("github.com/rabbitmq/amqp091-go.Channel.Publish"), + "rabbitmq"); + PASS(); +} + TEST(infrascan_route_nodes_skip_bad_http_url_paths) { cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp/cbm_infrascan_route_guard"); ASSERT_NOT_NULL(gb); @@ -128,6 +305,8 @@ TEST(infrascan_http_calls_join_matching_handler_route) { SUITE(infrascan) { RUN_TEST(infrascan_http_route_literal_guard_rejects_filesystem_paths); + RUN_TEST(infrascan_service_pattern_match_rejects_ids_inside_words); + RUN_TEST(infrascan_service_pattern_match_keeps_real_library_qns); RUN_TEST(infrascan_route_nodes_skip_bad_http_url_paths); RUN_TEST(infrascan_http_calls_join_matching_handler_route); } From 343d7dd5e47b817999098605370f0eab706675f0 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 19:45:31 +0200 Subject: [PATCH 11/14] fix(extract): a C-family function's return type keeps its pointer and qualifiers Distilled from #1245 by Andrew Hundt (c51e5cb0, the extract_defs.c half; the mcp.c and token-reduction parts are not taken). For C-family functions return_type was the text of the declaration's `type` node alone. In these grammars the `*`, `&` and `&&` live on the declarator and cv-qualifiers are sibling nodes, so const char *get_name(void) published char robj **lookupKeys(...) published robj Text &operator=(const Text &) published Text and no C or C++ function in any index had a pointer return type at all. The C/C++ resolver's return-type parser already strips a leading const/volatile and handles trailing `*`, `&`, `&&` -- branches that were unreachable for functions, so every `Foo *f()` was registered as returning Foo by value. c_declared_return_type() renders the declared type at both sites (functions and methods) for C, C++, CUDA, GLSL, HLSL, ISPC, Slang and Objective-C -- the list resolve_func_name_c_family uses: so `const char *`, `char **`, `Text &`, `Text &&`, `Text *&`, `volatile unsigned long *`; a pointer-level qualifier follows its star (`char *const *`); east-const `char const *` normalises to `const char *`. With no qualifier and no marker the old text is returned unchanged. It measures, then writes into one exact-size arena allocation; the declarator walk is a plain child chain, no recursion. def.return_types is untouched. Two deliberate differences from the upstream hunk. Upstream copied every type_qualifier node, which in these grammars includes constexpr, _Noreturn, mutable and __extension__ -- `constexpr int f()` would have become `constexpr int`. Only const, volatile, restrict, __restrict, __restrict__ and _Atomic are kept, and both cases are asserted unchanged. And the walk stops at the first declarator that is neither pointer nor reference, so `int (*f(void))(int)` stays `int` rather than becoming a wrong `int *`. RED, production reverted and the final tests kept: c_function_return_type_preserves_pointer_and_qualifier FAIL tests/test_extraction.c:1005: "char" != "const char *" cpp_method_return_type_preserves_pointer_and_qualifier FAIL tests/test_extraction.c:1072: "char" != "const char *" 351 passed, 2 failed c_function_return_type_plain_unchanged is the control: it passes before and after. GREEN: extraction 353, c_lsp 762, lang_contract 41, pipeline 281. Effect on real code -- Redis, production binaries, old against new. 39,132 nodes in both. 1,442 Function nodes change, in return_type only: void -> void * (270), char -> const char * (101), unsigned char -> unsigned char * (91), char -> char * (81), robj -> robj * (57), ... Functions with a `*` in return_type: 0 -> 1,440. With one worker the complete edge set apart from SEMANTICALLY_RELATED is byte-identical old against new across six runs (three each) -- CALLS, USAGE, WRITES, IMPORTS and the rest do not move. SEMANTICALLY_RELATED goes 350 -> 325 because that pass tokenises the return-type string into its type vector. One thing measured rather than assumed. With several workers, which `dict.h` Redis' src/dict.c imports (src/ or deps/hiredis/) depends on worker merge order. That is main's defect, not this change's: unmodified main picked src/dict.h in 2 of 10 runs and the hiredis header in 8; with one worker both binaries always pick src/dict.h. This change picked src/dict.h in 6 of 10. At ten runs a side that gap is not separable from chance (Fisher exact, two-sided p = 0.17), and a shifted rate would be unsurprising for a timing-dependent pick; it is recorded here rather than explained. The include-target fix is a separate change (#2227). Known limits, all pre-existing and out of scope: struct and class FIELDS have the same defect (`const char *data;` publishes char); C prototypes and declaration-only C++ methods are not extracted as definitions, so they are not reached; where a macro is misparsed as the type (`LUA_API const char *f()`), the base was already wrong and now reads `const LUA_API *`; the resolver's parser does not yet understand a pointer-level qualifier or a leading _Atomic/restrict and falls back to a named-type lookup for those, as it effectively did for every pointer return before. Co-authored-by: Andrew Hundt Signed-off-by: Martin Vogel --- internal/cbm/extract_defs.c | 157 +++++++++++++++++++++++++++++++++++- tests/test_extraction.c | 98 ++++++++++++++++++++++ 2 files changed, 253 insertions(+), 2 deletions(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 93da084be..916f1136f 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -3477,6 +3477,159 @@ static TSNode find_function_params(TSNode func_node, CBMLanguage lang) { return params; } +/* ── C-family declared return type ────────────────────────────────── + * The C-family grammars split a declared return type three ways: the `type` + * field (`char`), sibling type_qualifier nodes (`const`), and the + * pointer/reference declarators wrapping the function declarator (`*`). Taking + * only the `type` field published `const char *get_name(void)` as "char". + * + * Canonical spelling: leading cv-qualifiers in source order, the base type text + * verbatim, then one space and the declarator markers outermost-first with no + * space between them — `const char *`, `char **`, `Text &`, `Text *&`. A + * qualifier on a pointer level follows its `*` and is separated from the next + * marker by a space: `char *const *`. A qualifier written after the base type + * (`char const *`) is normalized to the leading position. */ + +/* Output sink: measures when buf is NULL, writes otherwise. Rendering twice + * sizes the arena allocation exactly without a second copy of the logic. */ +typedef struct { + char *buf; + size_t len; +} c_rt_out_t; + +static void c_rt_put(c_rt_out_t *out, const char *text, size_t n) { + if (out->buf) { + memcpy(out->buf + out->len, text, n); + } + out->len += n; +} + +static void c_rt_put_str(c_rt_out_t *out, const char *text) { + c_rt_put(out, text, strlen(text)); +} + +static void c_rt_put_node(c_rt_out_t *out, TSNode node, const char *source) { + uint32_t start = ts_node_start_byte(node); + uint32_t end = ts_node_end_byte(node); + if (end > start) { + c_rt_put(out, source + start, (size_t)(end - start)); + } +} + +/* type_qualifier also covers keywords that are not part of the type + * (`constexpr`, `_Noreturn`, `mutable`, `__extension__`, …). Keep only the ones + * that are, so `constexpr int f()` still returns "int". */ +static bool is_c_return_cv_qualifier(TSNode node, const char *source) { + static const char *const kept[] = {"const", "volatile", "restrict", "__restrict", + "__restrict__", "_Atomic", NULL}; + if (strcmp(ts_node_type(node), "type_qualifier") != 0) { + return false; + } + uint32_t start = ts_node_start_byte(node); + uint32_t end = ts_node_end_byte(node); + size_t len = end > start ? (size_t)(end - start) : 0; + for (const char *const *k = kept; *k; k++) { + if (strlen(*k) == len && memcmp(source + start, *k, len) == 0) { + return true; + } + } + return false; +} + +static bool is_c_declarator_lang(CBMLanguage lang) { + return lang == CBM_LANG_C || lang == CBM_LANG_CPP || lang == CBM_LANG_CUDA || + lang == CBM_LANG_GLSL || lang == CBM_LANG_HLSL || lang == CBM_LANG_ISPC || + lang == CBM_LANG_SLANG || lang == CBM_LANG_OBJC; +} + +/* Render the canonical return type into `out`; returns how many qualifiers and + * markers were added around the base type (0 = the base text alone is already + * the whole type). The declarator walk is one strict child chain, so it is + * O(depth) with no recursion and needs no depth cap. It stops at the first node + * that is neither a pointer nor a reference declarator: for a function returning + * a function pointer (`int (*f(void))(int)`) that is the outer + * function_declarator, which leaves the base type as it was. */ +static size_t c_rt_render(c_rt_out_t *out, TSNode func_node, TSNode type_node, TSNode declarator, + const char *source) { + size_t added = 0; + uint32_t decl_start = ts_node_start_byte(declarator); + uint32_t nc = ts_node_named_child_count(func_node); + for (uint32_t i = 0; i < nc; i++) { + TSNode ch = ts_node_named_child(func_node, i); + if (ts_node_start_byte(ch) >= decl_start) { + break; + } + if (is_c_return_cv_qualifier(ch, source)) { + c_rt_put_node(out, ch, source); + c_rt_put_str(out, " "); + added++; + } + } + c_rt_put_node(out, type_node, source); + + bool need_space = true; + TSNode decl = declarator; + while (!ts_node_is_null(decl)) { + const char *dk = ts_node_type(decl); + bool is_ref = strcmp(dk, "reference_declarator") == 0; + if (!is_ref && strcmp(dk, "pointer_declarator") != 0) { + break; + } + if (need_space) { + c_rt_put_str(out, " "); + need_space = false; + } + /* A reference_declarator opens with its `&` / `&&` token. */ + TSNode marker = ts_node_child(decl, 0); + if (is_ref && !ts_node_is_null(marker) && !ts_node_is_named(marker)) { + c_rt_put_node(out, marker, source); + } else { + c_rt_put_str(out, is_ref ? "&" : "*"); + } + added++; + uint32_t dn = ts_node_named_child_count(decl); + for (uint32_t i = 0; i < dn; i++) { + TSNode q = ts_node_named_child(decl, i); + if (is_c_return_cv_qualifier(q, source)) { + c_rt_put_node(out, q, source); + need_space = true; + added++; + } + } + /* tree-sitter-cpp/-cuda give a reference_declarator's inner declarator no + * `declarator` field (see find_c_params); it is the one named child. */ + TSNode inner = ts_node_child_by_field_name(decl, TS_FIELD("declarator")); + if (ts_node_is_null(inner) && is_ref && dn > 0) { + inner = ts_node_named_child(decl, 0); + } + decl = inner; + } + return added; +} + +/* Declared return type of a C-family function/method node whose `type` field is + * `type_node`. Any other language, and any type with nothing around its base + * type, gets the base type text exactly as before. */ +static char *c_declared_return_type(CBMExtractCtx *ctx, TSNode func_node, TSNode type_node) { + CBMArena *a = ctx->arena; + TSNode declarator = ts_node_child_by_field_name(func_node, TS_FIELD("declarator")); + if (!is_c_declarator_lang(ctx->language) || ts_node_is_null(declarator)) { + return cbm_node_text(a, type_node, ctx->source); + } + c_rt_out_t out = {NULL, 0}; + if (c_rt_render(&out, func_node, type_node, declarator, ctx->source) == 0) { + return cbm_node_text(a, type_node, ctx->source); + } + out.buf = (char *)cbm_arena_alloc(a, out.len + NULL_TERM); + if (!out.buf) { + return cbm_node_text(a, type_node, ctx->source); + } + out.len = 0; + (void)c_rt_render(&out, func_node, type_node, declarator, ctx->source); + out.buf[out.len] = '\0'; + return out.buf; +} + // C++: resolve trailing return type (auto f() -> Type) on a declarator node. // Updates def->return_type and def->return_types if trailing type found. static void resolve_cpp_trailing_return(CBMArena *a, TSNode func_node, const char *source, @@ -3712,7 +3865,7 @@ static void extract_func_def(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec for (const char **f = rt_fields; *f; f++) { TSNode rt = ts_node_child_by_field_name(func_node, *f, (uint32_t)strlen(*f)); if (!ts_node_is_null(rt)) { - def.return_type = cbm_node_text(a, rt, ctx->source); + def.return_type = c_declared_return_type(ctx, func_node, rt); def.return_types = extract_return_types(a, rt, ctx->source, ctx->language); break; } @@ -4964,7 +5117,7 @@ static void push_method_def(CBMExtractCtx *ctx, TSNode child, TSNode class_node, for (const char **f = rt_fields; *f; f++) { TSNode rt = ts_node_child_by_field_name(child, *f, (uint32_t)strlen(*f)); if (!ts_node_is_null(rt)) { - def.return_type = cbm_node_text(a, rt, ctx->source); + def.return_type = c_declared_return_type(ctx, child, rt); break; } } diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 8d9bf742b..331aa54f3 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -977,6 +977,65 @@ TEST(c_struct) { PASS(); } +/* return_type of the first definition named `name`; NULL when there is no such + * definition or it carries no return type. */ +static const char *def_return_type(CBMFileResult *r, const char *name) { + for (int i = 0; i < r->defs.count; i++) { + if (strcmp(r->defs.items[i].name, name) == 0) { + return r->defs.items[i].return_type; + } + } + return NULL; +} + +/* PR #1245: the C grammar splits a declared return type across the `type` node, + * sibling type_qualifier nodes and the pointer_declarator chain wrapping the + * function declarator. Taking only the `type` node published `const char *` as + * "char". Canonical spelling: qualifiers, base type, one space, then the + * declarator markers unspaced (`const char *`, `char **`). */ +TEST(c_function_return_type_preserves_pointer_and_qualifier) { + CBMFileResult *r = extract("static const char *text_end(const char *text) { return text; }\n" + "char **table(void) { return 0; }\n" + "char const *east_const(void) { return 0; }\n" + "const struct Point *find_point(void) { return 0; }\n" + "volatile unsigned long *reg(void) { return 0; }\n" + "char *const *frozen(void) { return 0; }\n", + CBM_LANG_C, "t", "returns.c"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT_STR_EQ(def_return_type(r, "text_end"), "const char *"); + ASSERT_STR_EQ(def_return_type(r, "table"), "char **"); + ASSERT_STR_EQ(def_return_type(r, "east_const"), "const char *"); + ASSERT_STR_EQ(def_return_type(r, "find_point"), "const struct Point *"); + ASSERT_STR_EQ(def_return_type(r, "reg"), "volatile unsigned long *"); + ASSERT_STR_EQ(def_return_type(r, "frozen"), "char *const *"); + cbm_free_result(r); + PASS(); +} + +/* Guard for the fix above: a return type with no qualifier and no declarator + * marker is already correct and must come out byte-identical. */ +TEST(c_function_return_type_plain_unchanged) { + CBMFileResult *r = extract("int scalar(void) { return 0; }\n" + "void nothing(void) {}\n" + "unsigned long wide(void) { return 0; }\n" + "struct Point make_point(void) { struct Point p; return p; }\n" + "static inline size_t count(void) { return 0; }\n" + "_Noreturn void die(void) { for (;;) {} }\n", + CBM_LANG_C, "t", "plain.c"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT_STR_EQ(def_return_type(r, "scalar"), "int"); + ASSERT_STR_EQ(def_return_type(r, "nothing"), "void"); + ASSERT_STR_EQ(def_return_type(r, "wide"), "unsigned long"); + ASSERT_STR_EQ(def_return_type(r, "make_point"), "struct Point"); + ASSERT_STR_EQ(def_return_type(r, "count"), "size_t"); + /* _Noreturn parses as a type_qualifier but is not part of the type. */ + ASSERT_STR_EQ(def_return_type(r, "die"), "void"); + cbm_free_result(r); + PASS(); +} + /* --- C++ --- */ TEST(cpp_class) { CBMFileResult *r = extract( @@ -990,6 +1049,42 @@ TEST(cpp_class) { PASS(); } +/* PR #1245, C++ side: in-class methods go through a separate extraction path + * from free functions, and C++ adds reference markers (`&`, `&&`) whose + * reference_declarator carries no `declarator` field. */ +TEST(cpp_method_return_type_preserves_pointer_and_qualifier) { + CBMFileResult *r = extract("class Text {\n" + "public:\n" + " const char *end() { return nullptr; }\n" + " char **table() { return nullptr; }\n" + " Text &self() { return *this; }\n" + " const Text &cself() const { return *this; }\n" + " Text *&slot() { return next_; }\n" + " Text *next_;\n" + " int width() const { return 0; }\n" + " constexpr int square(int x) const { return x * x; }\n" + "};\n" + "const Text &shared() { static Text t; return t; }\n" + "Text &&moved(Text &t) { return static_cast(t); }\n" + "const char *Text::c_str() const { return nullptr; }\n", + CBM_LANG_CPP, "t", "text.cpp"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT_STR_EQ(def_return_type(r, "end"), "const char *"); + ASSERT_STR_EQ(def_return_type(r, "table"), "char **"); + ASSERT_STR_EQ(def_return_type(r, "self"), "Text &"); + ASSERT_STR_EQ(def_return_type(r, "cself"), "const Text &"); + ASSERT_STR_EQ(def_return_type(r, "slot"), "Text *&"); + ASSERT_STR_EQ(def_return_type(r, "width"), "int"); + /* constexpr parses as a type_qualifier but is not part of the type. */ + ASSERT_STR_EQ(def_return_type(r, "square"), "int"); + ASSERT_STR_EQ(def_return_type(r, "shared"), "const Text &"); + ASSERT_STR_EQ(def_return_type(r, "moved"), "Text &&"); + ASSERT_STR_EQ(def_return_type(r, "c_str"), "const char *"); + cbm_free_result(r); + PASS(); +} + /* ═══════════════════════════════════════════════════════════════════ * Group C: Scripting / Dynamic Languages * ═══════════════════════════════════════════════════════════════════ */ @@ -8206,8 +8301,11 @@ SUITE(extraction) { RUN_TEST(go_interface); RUN_TEST(zig_function); RUN_TEST(c_function); + RUN_TEST(c_function_return_type_preserves_pointer_and_qualifier); + RUN_TEST(c_function_return_type_plain_unchanged); RUN_TEST(c_struct); RUN_TEST(cpp_class); + RUN_TEST(cpp_method_return_type_preserves_pointer_and_qualifier); /* Scripting */ RUN_TEST(python_function); From a041794f8e51ddf42342899be7b2ac26f951d777 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 22 Sep 2026 01:12:51 +0200 Subject: [PATCH 12/14] fix(daemon): losing the runtime-directory creation race is not a failure on Windows Several processes first-starting together against a runtime directory that does not exist yet -- test-windows-guards' section_cold_storm, or a host that launches more than one MCP server on its very first run -- all observe the final path component absent. One CreateDirectoryW wins; the others get ERROR_ALREADY_EXISTS. win_private_directory_tree_secure() treated that as a failure and recorded no validation detail, so every loser exited with a bare secure CLI coordination could not be created (endpoint) The function called right after the walk, win_runtime_directory_secure(), has always tolerated ERROR_ALREADY_EXISTS for the same directory, and the POSIX walk tolerates EEXIST at the same point; only the Windows walk in front of them did not. Seen four times on 2026-09-21 on unrelated PRs (#1768 three attempts in a row, #2140, #1999, #808). The racy lines date from July; the guard began exercising them on 2026-09-03, when each guard section was given its own empty CBM_RUNTIME_DIR, so the final component is now absent at storm time on every run. The walk now treats ERROR_ALREADY_EXISTS from its own CreateDirectoryW as "the directory I wanted exists". Nothing is trusted because of that: an ancestor still goes through win_directory_component_secure(), and the final component through win_runtime_directory_secure(), which refuses a non-directory or reparse point and enforces owner and DACL. Every refusal on this path now names its component and its rule. The walk reports the Windows error when it can neither create nor inspect a component, and win_runtime_directory_secure() says whether the path could not be created, cannot be inspected, exists but is not a directory, or is a reparse point. One helper owns the wide-to-UTF-8 conversion for these messages and the existing ancestor message now uses it too, so src/daemon/ipc.c stays at its memory-core baseline. Deterministic reproduction, no threads and no timing: a test seam fires in the walk between "component observed absent" and CreateDirectoryW, and the test plays the process that wins the creation. daemon_ipc_windows_private_directory_survives_lost_creation_race daemon_ipc_windows_private_directory_refuses_and_names_a_planted_file Verification. macOS arm64 (ASan+UBSan): build clean, daemon_ipc 53 passed (2 Windows-only skips), daemon_bootstrap 28 passed, lint-memory-core unchanged (ipc.c stays at 146 raw sites). The two new tests are Windows-only and have NOT been run locally: the Windows VM was down when this was written, so the RED run on the seam-only tree and the GREEN run with this fix are delegated to CI's windows-latest legs (test-windows shards, test-windows-guards) by an explicit maintainer decision. If the guard suite's cold storm still fails with this in place, the attribution above is wrong and this reverts. Signed-off-by: Martin Vogel --- src/daemon/ipc.c | 88 ++++++++++++++++++++++++--- src/daemon/ipc_internal.h | 11 ++++ tests/test_daemon_ipc.c | 125 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 7 deletions(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index a92befb08..3917a7a05 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -214,6 +214,31 @@ static void ipc_startup_gate_run(void) { } #endif +#ifdef _WIN32 +static cbm_daemon_ipc_win_directory_create_hook_fn g_win_directory_create_hook_for_test; +static void *g_win_directory_create_hook_context_for_test; +#endif + +void cbm_daemon_ipc_win_directory_create_hook_set_for_test( + cbm_daemon_ipc_win_directory_create_hook_fn hook, void *context) { +#ifdef _WIN32 + g_win_directory_create_hook_context_for_test = context; + g_win_directory_create_hook_for_test = hook; +#else + (void)hook; + (void)context; +#endif +} + +#ifdef _WIN32 +static void ipc_win_directory_create_hook_run(const wchar_t *path) { + cbm_daemon_ipc_win_directory_create_hook_fn hook = g_win_directory_create_hook_for_test; + if (hook) { + hook(path, g_win_directory_create_hook_context_for_test); + } +} +#endif + bool cbm_daemon_ipc_windows_legacy_names(const char *canonical_runtime_parent, const char *instance_key, char pipe_out[CBM_DAEMON_IPC_WINDOWS_NAME_CAP], @@ -3994,6 +4019,16 @@ static char *wide_to_utf8(const wchar_t *value) { return utf8; } +/* Record ": " as the validation detail, with the wide path + * rendered as UTF-8. One place owns the conversion buffer, so every refusal on + * the directory walk can name its component without each call site carrying + * its own allocation. */ +static void ipc_validation_detail_set_for_path(const wchar_t *path, const char *message) { + char *path_utf8 = wide_to_utf8(path); + ipc_validation_detail_set("%s: %s", path_utf8 ? path_utf8 : "", message); + free(path_utf8); +} + static wchar_t *wide_copy(const wchar_t *value) { if (!value) { return NULL; @@ -4861,13 +4896,28 @@ static bool win_runtime_directory_secure(const wchar_t *runtime_dir) { return false; } bool created = CreateDirectoryW(runtime_dir, &security.directory_attributes) != 0; - if (!created && GetLastError() != ERROR_ALREADY_EXISTS) { + DWORD create_error = created ? ERROR_SUCCESS : GetLastError(); + if (!created && create_error != ERROR_ALREADY_EXISTS) { + char message[96]; + (void)snprintf(message, sizeof(message), + "could not create the directory (Windows error %lu)", + (unsigned long)create_error); + ipc_validation_detail_set_for_path(runtime_dir, message); win_security_destroy(&security); return false; } DWORD attributes = GetFileAttributesW(runtime_dir); if (attributes == INVALID_FILE_ATTRIBUTES || (attributes & FILE_ATTRIBUTE_DIRECTORY) == 0 || (attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) { + /* What already exists at this path is adopted only if it is a plain + * directory. Say which rule refused it: the walk in front of this + * function tolerates losing a creation race, so this check is what + * stands between that tolerance and a planted file or junction. */ + const char *rule = attributes == INVALID_FILE_ATTRIBUTES ? "cannot be inspected" + : (attributes & FILE_ATTRIBUTE_DIRECTORY) == 0 + ? "exists but is not a directory" + : "is a reparse point (junction or symlink)"; + ipc_validation_detail_set_for_path(runtime_dir, rule); win_security_destroy(&security); return false; } @@ -5021,8 +5071,35 @@ static bool win_private_directory_tree_secure(const wchar_t *directory_path) { DWORD attributes = GetFileAttributesW(path); if (attributes == INVALID_FILE_ATTRIBUTES) { DWORD error = GetLastError(); - ok = (error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND) && - CreateDirectoryW(path, &security.directory_attributes) != 0; + bool absent = error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND; + if (absent) { + ipc_win_directory_create_hook_run(path); + /* Several processes first-starting together all observe + * this component absent; one CreateDirectoryW wins and + * the rest get ERROR_ALREADY_EXISTS. Losing that race is + * not a failure -- the directory this process wanted now + * exists, exactly as if it had been there all along. It + * is NOT trusted for that: an ancestor still goes through + * win_directory_component_secure() below, and the final + * component through win_runtime_directory_secure(), which + * refuses a non-directory or reparse point and enforces + * owner and DACL. The POSIX walk tolerates EEXIST at the + * same point for the same reason. */ + if (CreateDirectoryW(path, &security.directory_attributes) == 0) { + error = GetLastError(); + absent = error == ERROR_ALREADY_EXISTS; + } + } + ok = absent; + if (!ok) { + /* Never a bare "(endpoint)": name the component and the + * Windows error that refused it. */ + char message[96]; + (void)snprintf(message, sizeof(message), + "could not create or inspect the directory (Windows error %lu)", + (unsigned long)error); + ipc_validation_detail_set_for_path(path, message); + } } /* Ancestors are observe-only and must already be secure. The * final current-user directory is intentionally handled below by @@ -5034,10 +5111,7 @@ static bool win_private_directory_tree_secure(const wchar_t *directory_path) { * at this walk position; the helper set the inner rule. */ char inner[384]; (void)snprintf(inner, sizeof(inner), "%s", ipc_validation_detail_buffer); - char *component_utf8 = wide_to_utf8(path); - ipc_validation_detail_set( - "%s: %s", component_utf8 ? component_utf8 : "", inner); - free(component_utf8); + ipc_validation_detail_set_for_path(path, inner); } } } diff --git a/src/daemon/ipc_internal.h b/src/daemon/ipc_internal.h index 4b7cf1fd5..f481b7378 100644 --- a/src/daemon/ipc_internal.h +++ b/src/daemon/ipc_internal.h @@ -110,4 +110,15 @@ void cbm_daemon_ipc_posix_record_write_failure_set_for_test(int errno_value); typedef void (*cbm_daemon_ipc_startup_gate_fn)(void *context); void cbm_daemon_ipc_startup_gate_set_for_test(cbm_daemon_ipc_startup_gate_fn gate, void *context); +/* Deterministic-interleaving seam for the Windows private-directory walk: + * fires after a path component was observed ABSENT and before this process + * tries to create it. A test plays the concurrent process that wins that + * creation, which pins the cold-start race (N processes first-starting + * against a runtime directory that does not exist yet) by construction + * instead of by thread timing. `path` is the NUL-terminated component path at + * the walk position. No-op off Windows. */ +typedef void (*cbm_daemon_ipc_win_directory_create_hook_fn)(const wchar_t *path, void *context); +void cbm_daemon_ipc_win_directory_create_hook_set_for_test( + cbm_daemon_ipc_win_directory_create_hook_fn hook, void *context); + #endif /* CBM_DAEMON_IPC_INTERNAL_H */ diff --git a/tests/test_daemon_ipc.c b/tests/test_daemon_ipc.c index b451f9bb3..815f630a6 100644 --- a/tests/test_daemon_ipc.c +++ b/tests/test_daemon_ipc.c @@ -882,6 +882,129 @@ TEST(daemon_ipc_windows_private_directory_ace_is_inheritable) { PASS(); } +typedef struct { + int calls; + bool plant_file; /* false: play a racer that creates the DIRECTORY first */ + bool planted; +} ipc_test_win_create_racer_t; + +/* Plays the concurrent process that wins the creation of a path component the + * walk has just observed absent. Runs inside the walk, at the exact point the + * race lives, so the interleaving is pinned by construction: no threads, no + * timing. */ +static void ipc_test_win_create_racer(const wchar_t *path, void *context) { + ipc_test_win_create_racer_t *racer = context; + racer->calls++; + if (racer->calls != 1) { + return; + } + if (racer->plant_file) { + HANDLE file = + CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL); + racer->planted = file != INVALID_HANDLE_VALUE; + if (racer->planted) { + (void)CloseHandle(file); + } + } else { + racer->planted = CreateDirectoryW(path, NULL) != 0; + } +} + +/* Cold-start race (test-windows-guards section_cold_storm, 2026-09-21; the + * same thing a user gets when a host launches several MCP servers on its very + * first run): N processes first-start against a runtime directory that does + * not exist yet. All of them observe the final component absent, one + * CreateDirectoryW wins, and every other process gets ERROR_ALREADY_EXISTS. + * The walk treated that as a failure and recorded no detail, so the losers + * exited with a bare "secure CLI coordination could not be created + * (endpoint)". Losing the creation race is not a failure: the directory this + * process wanted now exists, and the owner / DACL / not-a-reparse-point checks + * that follow are exactly the ones a pre-existing directory gets. The POSIX + * walk has always tolerated EEXIST here. */ +TEST(daemon_ipc_windows_private_directory_survives_lost_creation_race) { + char parent[TEST_PATH_CAP] = {0}; + char cache[TEST_PATH_CAP] = {0}; + bool paths_ok = false; + bool secured = false; + bool owner_only = false; + ipc_test_win_create_racer_t racer = {0}; + + if (ipc_test_parent_new(parent, "win-create-race")) { + int written = snprintf(cache, sizeof(cache), "%s/cache", parent); + paths_ok = written > 0 && written < (int)sizeof(cache); + } + if (paths_ok) { + cbm_daemon_ipc_win_directory_create_hook_set_for_test(ipc_test_win_create_racer, &racer); + secured = cbm_daemon_ipc_private_directory_secure(cache); + cbm_daemon_ipc_win_directory_create_hook_set_for_test(NULL, NULL); + } + if (secured) { + /* The racer created the directory with an inherited DACL; the winner's + * directory must still end up owner-only, like any adopted one. */ + PACL dacl = NULL; + PSECURITY_DESCRIPTOR descriptor = NULL; + ACL_SIZE_INFORMATION information; + memset(&information, 0, sizeof(information)); + if (GetNamedSecurityInfoA(cache, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, + &dacl, NULL, &descriptor) == ERROR_SUCCESS && + dacl && + GetAclInformation(dacl, &information, sizeof(information), AclSizeInformation)) { + owner_only = information.AceCount == 1; + } + if (descriptor) { + (void)LocalFree(descriptor); + } + } + + (void)RemoveDirectoryA(cache); + ipc_test_remove_flat_dir(parent); + + ASSERT_TRUE(paths_ok); + /* The seam sat on the path and the racer really won: without these two the + * test could pass without ever exercising the race. */ + ASSERT_EQ(racer.calls, 1); + ASSERT_TRUE(racer.planted); + ASSERT_TRUE(secured); + ASSERT_TRUE(owner_only); + PASS(); +} + +/* The tolerance above must not admit anything: when what appeared at the path + * is NOT a directory, the walk still refuses -- and it says why. A bare + * "(endpoint)" is what four reporters in #1533/#1574 were left with; every + * refusal on this path names the component and the rule. */ +TEST(daemon_ipc_windows_private_directory_refuses_and_names_a_planted_file) { + char parent[TEST_PATH_CAP] = {0}; + char cache[TEST_PATH_CAP] = {0}; + char detail[512] = {0}; + bool paths_ok = false; + bool refused = false; + ipc_test_win_create_racer_t racer = {.plant_file = true}; + + if (ipc_test_parent_new(parent, "win-create-race-file")) { + int written = snprintf(cache, sizeof(cache), "%s/cache", parent); + paths_ok = written > 0 && written < (int)sizeof(cache); + } + if (paths_ok) { + cbm_daemon_ipc_win_directory_create_hook_set_for_test(ipc_test_win_create_racer, &racer); + refused = !cbm_daemon_ipc_private_directory_secure(cache); + cbm_daemon_ipc_win_directory_create_hook_set_for_test(NULL, NULL); + const char *why = cbm_daemon_ipc_validation_detail(); + (void)snprintf(detail, sizeof(detail), "%s", why ? why : ""); + } + + (void)DeleteFileA(cache); + ipc_test_remove_flat_dir(parent); + + ASSERT_TRUE(paths_ok); + ASSERT_EQ(racer.calls, 1); + ASSERT_TRUE(racer.planted); + ASSERT_TRUE(refused); + ASSERT_NOT_NULL(strstr(detail, "cache")); + ASSERT_NOT_NULL(strstr(detail, "not a directory")); + PASS(); +} + TEST(daemon_ipc_windows_private_directory_allows_add_subdirectory_only_ancestor) { char parent[TEST_PATH_CAP] = {0}; char add_only[TEST_PATH_CAP] = {0}; @@ -5361,6 +5484,8 @@ SUITE(daemon_ipc) { RUN_TEST(daemon_ipc_windows_default_endpoint_ignores_temp_environment); RUN_TEST(daemon_ipc_windows_private_directory_rejects_untrusted_ancestor_acl); RUN_TEST(daemon_ipc_windows_private_directory_ace_is_inheritable); + RUN_TEST(daemon_ipc_windows_private_directory_survives_lost_creation_race); + RUN_TEST(daemon_ipc_windows_private_directory_refuses_and_names_a_planted_file); RUN_TEST(daemon_ipc_windows_private_directory_allows_add_subdirectory_only_ancestor); RUN_TEST(daemon_ipc_windows_sid_trust_accepts_local_admin_rejects_foreign_500); RUN_TEST(daemon_ipc_windows_legacy_bridge_covers_handoff_and_lifetime); From aaa72fbe60ad341325e9c944e32cf67d98eca387 Mon Sep 17 00:00:00 2001 From: Lowpower <3133734+Lowpower@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:06:03 +0000 Subject: [PATCH 13/14] feat(cli): point top-level cli --help at --format json CLI_USAGE described outer --json but did not mention tool-level --format json. Add a help-only pointer that tools which accept format support --format tree|json, and distinguish payload JSON from the full MCP envelope. No new flags, env vars, or output-default changes. Fixes #2102 Signed-off-by: Lowpower <3133734+Lowpower@users.noreply.github.com> --- src/cli/cli.h | 12 ++++++++++++ src/main.c | 8 +------- tests/test_cli.c | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/cli/cli.h b/src/cli/cli.h index cf7f6fa44..a1b749bb0 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -27,6 +27,18 @@ const char *cbm_cli_get_version(void); /* ── CLI tool arguments (flags / --args-file / --help) ────────── */ +/* Top-level `cli --help` text printed by run_cli() in src/main.c. + * Documents tool-level --format without adding a session-wide flag (#2102). */ +#define CBM_CLI_USAGE \ + "Usage: codebase-memory-mcp cli [--quiet] [--progress] [--verbose] [--json] " \ + "[json_args]\n" \ + " --quiet Show errors only; cannot combine with --progress or outer --verbose\n" \ + " --progress Show lifecycle progress even when stderr is redirected\n" \ + " --verbose Include informational logs (preserves CBM_LOG_LEVEL=debug)\n" \ + " --json Print the raw MCP result envelope\n" \ + " Tools that accept format support --format tree|json (default: tree).\n" \ + " --format json prints payload JSON; outer --json prints the full MCP envelope.\n" + /* Convert `--flag value` / `--flag=value` / bare-boolean `--flag` arguments for * a tool into a JSON arguments object string, using the tool's input_schema to * type values (string/integer/boolean) and to collect repeated flags into diff --git a/src/main.c b/src/main.c index 9b91f205c..65e041217 100644 --- a/src/main.c +++ b/src/main.c @@ -675,13 +675,7 @@ static bool client_start_parent_watchdog(DWORD initial_ppid) { /* ── CLI mode ───────────────────────────────────────────────────── */ -#define CLI_USAGE \ - "Usage: codebase-memory-mcp cli [--quiet] [--progress] [--verbose] [--json] " \ - "[json_args]\n" \ - " --quiet Show errors only; cannot combine with --progress or outer --verbose\n" \ - " --progress Show lifecycle progress even when stderr is redirected\n" \ - " --verbose Include informational logs (preserves CBM_LOG_LEVEL=debug)\n" \ - " --json Print the raw MCP result envelope\n" +#define CLI_USAGE CBM_CLI_USAGE /* `cli --help`; wording lives in cli.h (#2102) */ /* Extract text content from MCP tool result envelope and print it. * MCP results: {"content":[{"type":"text","text":"..."}],"isError":...} diff --git a/tests/test_cli.c b/tests/test_cli.c index 211ec6355..2ca51ab2f 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -15336,6 +15336,21 @@ TEST(cli_print_tool_help_issue680) { PASS(); } +/* #2102: top-level `cli --help` (CLI_USAGE) must point at tool-level + * `--format json` and distinguish it from outer `--json`. This is help-only: + * the usage synopsis still has no session-wide --format / CBM_CLI_FORMAT. */ +TEST(cli_usage_points_to_tool_format_json_issue2102) { + ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "--format tree|json")); + ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "payload JSON")); + ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "full MCP envelope")); + ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "--json Print the raw MCP result envelope")); + ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, + "cli [--quiet] [--progress] [--verbose] [--json] ")); + ASSERT_NULL(strstr(CBM_CLI_USAGE, "[--format")); + ASSERT_NULL(strstr(CBM_CLI_USAGE, "CBM_CLI_FORMAT")); + PASS(); +} + /* #1359: `cli ` with no argument-bearing token used to slurp stdin to EOF * for ANY non-terminal stdin. The ordinary automation caller never sends that * EOF — Node's child_process.spawn defaults to stdio:['pipe','pipe','pipe'] and @@ -16376,6 +16391,7 @@ SUITE(cli) { RUN_TEST(cli_build_args_json_key_equals_value_issue680); RUN_TEST(cli_build_args_json_bad_positional_errors_issue680); RUN_TEST(cli_print_tool_help_issue680); + RUN_TEST(cli_usage_points_to_tool_format_json_issue2102); /* Stdin argument gate (#1359) */ RUN_TEST(cli_zero_argument_tool_never_reads_stdin_issue1359); From 2f557f53986976b2df1eb2a1a540c9d48cf91a8e Mon Sep 17 00:00:00 2001 From: Lowpower <3133734+Lowpower@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:17:17 +0000 Subject: [PATCH 14/14] style(cli): clang-format CBM_CLI_USAGE and help-text test Signed-off-by: Lowpower <3133734+Lowpower@users.noreply.github.com> --- src/cli/cli.h | 14 +++++++------- tests/test_cli.c | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cli/cli.h b/src/cli/cli.h index a1b749bb0..5522edef0 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -29,14 +29,14 @@ const char *cbm_cli_get_version(void); /* Top-level `cli --help` text printed by run_cli() in src/main.c. * Documents tool-level --format without adding a session-wide flag (#2102). */ -#define CBM_CLI_USAGE \ +#define CBM_CLI_USAGE \ "Usage: codebase-memory-mcp cli [--quiet] [--progress] [--verbose] [--json] " \ - "[json_args]\n" \ - " --quiet Show errors only; cannot combine with --progress or outer --verbose\n" \ - " --progress Show lifecycle progress even when stderr is redirected\n" \ - " --verbose Include informational logs (preserves CBM_LOG_LEVEL=debug)\n" \ - " --json Print the raw MCP result envelope\n" \ - " Tools that accept format support --format tree|json (default: tree).\n" \ + "[json_args]\n" \ + " --quiet Show errors only; cannot combine with --progress or outer --verbose\n" \ + " --progress Show lifecycle progress even when stderr is redirected\n" \ + " --verbose Include informational logs (preserves CBM_LOG_LEVEL=debug)\n" \ + " --json Print the raw MCP result envelope\n" \ + " Tools that accept format support --format tree|json (default: tree).\n" \ " --format json prints payload JSON; outer --json prints the full MCP envelope.\n" /* Convert `--flag value` / `--flag=value` / bare-boolean `--flag` arguments for diff --git a/tests/test_cli.c b/tests/test_cli.c index 2ca51ab2f..01fb79ec8 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -15344,8 +15344,8 @@ TEST(cli_usage_points_to_tool_format_json_issue2102) { ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "payload JSON")); ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "full MCP envelope")); ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "--json Print the raw MCP result envelope")); - ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, - "cli [--quiet] [--progress] [--verbose] [--json] ")); + ASSERT_NOT_NULL(strstr(CBM_CLI_USAGE, "cli [--quiet] [--progress] [--verbose] [--json] " + "")); ASSERT_NULL(strstr(CBM_CLI_USAGE, "[--format")); ASSERT_NULL(strstr(CBM_CLI_USAGE, "CBM_CLI_FORMAT")); PASS();