From d0e108f09b48554d6c38bce6d2d6a2d9f1843b16 Mon Sep 17 00:00:00 2001 From: imi Date: Mon, 27 Jul 2026 17:16:05 +0000 Subject: [PATCH 01/15] fix(index): preserve persisted coverage summaries Signed-off-by: imi --- src/mcp/mcp.c | 54 +++++++++++++++++++++++++++++++++-- tests/test_index_resilience.c | 28 ++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index d891e7dd9..f55857ccd 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -6908,6 +6908,54 @@ static void add_parse_partial_summary(yyjson_mut_doc *doc, yyjson_mut_val *root, yyjson_mut_obj_add_val(doc, root, "parse_partial", pp); } +/* The pipeline persists the complete current coverage set before this + * response is built. Prefer that set over the per-run errors so incremental + * runs that do not revisit a flagged file, and artifact bootstraps, do not + * make existing gaps appear to have vanished. By-design exclusions have + * their own response surface and are not failures. */ +static bool add_persisted_failure_summaries(yyjson_mut_doc *doc, yyjson_mut_val *root, + cbm_store_t *store, const char *project, + const char *logfile) { + cbm_coverage_row_t *rows = NULL; + int row_count = 0; + if (cbm_store_coverage_get(store, project, &rows, &row_count) != CBM_STORE_OK) { + return false; + } + + int failure_count = 0; + for (int i = 0; i < row_count; i++) { + const char *kind = rows[i].kind ? rows[i].kind : ""; + if (strcmp(kind, "not_indexed_dir") != 0 && strcmp(kind, "not_indexed_file") != 0) { + failure_count++; + } + } + + cbm_file_error_t *failures = + failure_count > 0 ? calloc((size_t)failure_count, sizeof(*failures)) : NULL; + if (failure_count > 0 && !failures) { + cbm_store_free_coverage(rows, row_count); + return false; + } + + int n = 0; + for (int i = 0; i < row_count; i++) { + const char *kind = rows[i].kind ? rows[i].kind : ""; + if (strcmp(kind, "not_indexed_dir") == 0 || strcmp(kind, "not_indexed_file") == 0) { + continue; + } + failures[n].path = (char *)rows[i].rel_path; + failures[n].reason = (char *)rows[i].detail; + failures[n].phase = (char *)rows[i].kind; + n++; + } + + add_skipped_summary(doc, root, failures, failure_count, logfile); + add_parse_partial_summary(doc, root, failures, failure_count); + free(failures); + cbm_store_free_coverage(rows, row_count); + return true; +} + /* Write the FULL (uncapped) skip list to a per-run logfile — ONLY when >=1 file * was skipped (no logfile on a clean run). Location: * $CBM_INDEX_LOG (override) else /logs/-.log @@ -6967,8 +7015,6 @@ static bool build_index_success_response(cbm_mcp_server_t *srv, yyjson_mut_doc * const cbm_file_error_t *file_errors, int file_error_count, const char *logfile) { add_excluded_summary(doc, root, excluded_dirs, excluded_count); - add_skipped_summary(doc, root, file_errors, file_error_count, logfile); - add_parse_partial_summary(doc, root, file_errors, file_error_count); add_not_indexed_files_summary(doc, root, p); int exp_nodes = -1; @@ -6979,6 +7025,10 @@ static bool build_index_success_response(cbm_mcp_server_t *srv, yyjson_mut_doc * const int min_floor = CBM_DUMP_VERIFY_MIN_FLOOR; cbm_store_t *store = resolve_store(srv, project_name); + if (!store || !add_persisted_failure_summaries(doc, root, store, project_name, logfile)) { + add_skipped_summary(doc, root, file_errors, file_error_count, logfile); + add_parse_partial_summary(doc, root, file_errors, file_error_count); + } int nodes = 0; int edges = 0; bool degraded = false; diff --git a/tests/test_index_resilience.c b/tests/test_index_resilience.c index 8d8a601a6..14909a4e4 100644 --- a/tests/test_index_resilience.c +++ b/tests/test_index_resilience.c @@ -439,6 +439,34 @@ TEST(index_parse_partial_reported) { /* Clean neighbors still extract. */ int funcs = rh_count_label(store, lp.project, "Function"); ASSERT_GTE(funcs, 1); + cbm_store_close(store); + store = NULL; + + /* An incremental run that changes only a clean neighbor has no new parser + * errors, but its response must still describe the persisted current + * coverage state for the untouched partial file. */ + struct timespec delay = {.tv_sec = 0, .tv_nsec = INCR_FIX_SLEEP_NS}; + nanosleep(&delay, NULL); + ri_write_text(lp.tmpdir, "good.py", "def alpha():\n return 2\n"); + char iargs[700]; + snprintf(iargs, sizeof(iargs), "{\"repo_path\":\"%s\"}", lp.tmpdir); + char *iresp = cbm_mcp_handle_tool(lp.srv, "index_repository", iargs); + ASSERT_NOT_NULL(iresp); + yyjson_doc *idoc = yyjson_read(iresp, strlen(iresp), 0); + ASSERT_NOT_NULL(idoc); + yyjson_val *isc = yyjson_obj_get(yyjson_doc_get_root(idoc), "structuredContent"); + ASSERT_NOT_NULL(isc); + ASSERT_GTE(yyjson_get_int(yyjson_obj_get(isc, "parse_partial_count")), 1); + yyjson_val *ipp = yyjson_obj_get(isc, "parse_partial"); + ASSERT_NOT_NULL(ipp); + char *ipp_json = yyjson_val_write(ipp, 0, NULL); + ASSERT_NOT_NULL(ipp_json); + ASSERT_NOT_NULL(strstr(ipp_json, "split.c")); + free(ipp_json); + /* The coverage is persisted state, but the logfile remains per-run. */ + ASSERT_NULL(yyjson_obj_get(isc, "logfile")); + yyjson_doc_free(idoc); + free(iresp); yyjson_doc_free(d); free(resp); From 7fda9f71152d0ac06dae34922d01940b9e21ff8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B6=82=E5=90=8D=E6=81=A9?= <125283161+Joseph-MingEn@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:16:43 -0700 Subject: [PATCH 02/15] fix(python): resolve aliased from-import CALLS to real def MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit from M import f as g + g() must emit CALLS→f, not invent M.g. Use resolve_import_node in sequential import maps, and prefer an exact registry hit when the IMPORTS target is already the def QN. Co-authored-by: Cursor Signed-off-by: 涂名恩 <125283161+Joseph-MingEn@users.noreply.github.com> --- src/pipeline/pass_calls.c | 26 +++++++++++++------- src/pipeline/registry.c | 11 +++++---- tests/test_lang_contract.c | 49 ++++++++++++++++++++++++++++++++++++++ tests/test_registry.c | 23 ++++++++++++++++++ 4 files changed, 96 insertions(+), 13 deletions(-) diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index 6db2a73f7..b2b56291f 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -114,20 +114,23 @@ static int build_import_map(cbm_pipeline_ctx_t *ctx, const char *rel_path, *out_vals = NULL; *out_count = 0; - /* Fast path: build from cached extraction result (no JSON parsing) */ + /* Fast path: build from cached extraction via the same resolver used for + * IMPORTS edges. Do NOT use cbm_pipeline_fqn_module(module_path) — Python + * from-imports store "pkg.symbol" (and aliases) in module_path, which is + * not a filesystem rel-path and misses the def node. */ if (result && result->imports.count > 0) { const char **keys = calloc((size_t)result->imports.count, sizeof(const char *)); const char **vals = calloc((size_t)result->imports.count, sizeof(const char *)); int count = 0; + char *file_qn = cbm_pipeline_fqn_compute(ctx->project_name, rel_path, "__file__"); for (int i = 0; i < result->imports.count; i++) { const CBMImport *imp = &result->imports.items[i]; if (!imp->local_name || !imp->local_name[0] || !imp->module_path) { continue; } - char *target_qn = cbm_pipeline_fqn_module(ctx->project_name, imp->module_path); - const cbm_gbuf_node_t *target = cbm_gbuf_find_by_qn(ctx->gbuf, target_qn); - free(target_qn); + const cbm_gbuf_node_t *target = + cbm_pipeline_resolve_import_node(ctx, rel_path, file_qn, imp, NULL); if (!target) { continue; } @@ -135,11 +138,18 @@ static int build_import_map(cbm_pipeline_ctx_t *ctx, const char *rel_path, vals[count] = target->qualified_name; /* borrowed from gbuf */ count++; } + free(file_qn); - *out_keys = keys; - *out_vals = vals; - *out_count = count; - return 0; + if (count > 0) { + *out_keys = keys; + *out_vals = vals; + *out_count = count; + return 0; + } + free((void *)keys); + free((void *)vals); + /* Fall through to IMPORTS-edge scan when extraction paths did not + * resolve (should be rare once resolve_import_node is used). */ } /* Slow path: scan graph buffer IMPORTS edges + parse JSON properties */ diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index cf1b3f008..e0d609eb8 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -619,11 +619,12 @@ static cbm_resolution_t resolve_import_map(const cbm_registry_t *r, const char * * resolved.requireAdmin — not just resolved, which would point at the * module node and miss the function entirely. */ /* Direct hit ONLY for suffix-less callees (an aliased direct-symbol - * import called bare: `from m import f as g; g()` — #875/#979). With a - * suffix present (`imported.method()`), returning the bare base here - * would swallow the suffix and bind the call to the imported symbol's - * own node (a Variable/Class/module) instead of base.method — exactly - * the mis-resolution the comment above warns about. That regressed + * import called bare: `from m import f as g; g()` — #875/#979; Yui + * `import execute as bridge_execute`). With a suffix present + * (`imported.method()`), returning the bare base here would swallow + * the suffix and bind the call to the imported symbol's own node + * (a Variable/Class/module) instead of base.method — exactly the + * mis-resolution the comment above warns about. That regressed * django-scale graphs by ~11K CALLS/TESTS edges (Signal.send calls * degraded to edges onto the signal variables themselves). #1000 */ if (!suffix || !suffix[0]) { diff --git a/tests/test_lang_contract.c b/tests/test_lang_contract.c index 3ce639ded..31204e7d1 100644 --- a/tests/test_lang_contract.c +++ b/tests/test_lang_contract.c @@ -456,6 +456,54 @@ TEST(contract_python_relative_import) { PASS(); } +/* Python: aliased from-import must CALLS the real def (execute), not a ghost + * named after the local alias (bridge_execute). Mirrors Yui G1. */ +TEST(contract_python_aliased_from_import_calls) { + static const LangFile f[] = { + {"gate.py", "def execute(x):\n return x\n"}, + {"router.py", + "from .gate import execute as bridge_execute\n\n\n" + "def submit_task(y):\n return bridge_execute(y)\n"}}; + LangProj lp; + cbm_store_t *store = lang_index_files(&lp, f, 2); + ASSERT_TRUE(store != NULL); + cbm_node_t *nodes = NULL; + int ncount = 0; + ASSERT_EQ(cbm_store_find_nodes_by_name(store, lp.project, "submit_task", &nodes, &ncount), + CBM_STORE_OK); + ASSERT_TRUE(ncount >= 1); + char **callers = NULL; + char **callees = NULL; + int n_callers = 0; + int n_callees = 0; + ASSERT_EQ(cbm_store_node_neighbor_names(store, nodes[0].id, 32, &callers, &n_callers, &callees, + &n_callees), + 0); + int saw_execute = 0; + int saw_alias_ghost = 0; + for (int i = 0; i < n_callees; i++) { + if (callees[i] && strcmp(callees[i], "execute") == 0) { + saw_execute = 1; + } + if (callees[i] && strcmp(callees[i], "bridge_execute") == 0) { + saw_alias_ghost = 1; + } + } + for (int i = 0; i < n_callers; i++) { + free(callers[i]); + } + for (int i = 0; i < n_callees; i++) { + free(callees[i]); + } + free(callers); + free(callees); + cbm_store_free_nodes(nodes, ncount); + lang_cleanup(&lp, store); + ASSERT_TRUE(saw_execute); + ASSERT_FALSE(saw_alias_ghost); + PASS(); +} + /* TypeScript: a relative import resolves to a sibling module → IMPORTS edge. */ TEST(contract_typescript_relative_import) { static const LangFile f[] = { @@ -1625,6 +1673,7 @@ SUITE(lang_contract) { RUN_TEST(contract_java_methods); RUN_TEST(contract_kotlin_methods); RUN_TEST(contract_python_relative_import); + RUN_TEST(contract_python_aliased_from_import_calls); RUN_TEST(contract_typescript_relative_import); /* Graph-level breadth across all grammars (P4). */ diff --git a/tests/test_registry.c b/tests/test_registry.c index 725a6f935..42c6dae60 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -386,6 +386,28 @@ TEST(resolve_import_map_bare_alias) { PASS(); } +/* from M import execute as bridge_execute + bridge_execute() — IMPORTS target is + * already the def QN. Must CALLS→execute, not invent …M.bridge_execute. + * Yui G1 shape (alongside resolve_import_map_bare_alias). */ +TEST(resolve_import_map_aliased_from_import) { + cbm_registry_t *r = cbm_registry_new(); + cbm_registry_add(r, "execute", "proj.services.satori_bridge.gate.execute", "Function"); + /* Alias ghost must not win if somehow registered. */ + cbm_registry_add(r, "bridge_execute", "proj.services.satori_bridge.gate.bridge_execute", + "Function"); + + const char *keys[] = {"bridge_execute"}; + const char *vals[] = {"proj.services.satori_bridge.gate.execute"}; + + cbm_resolution_t res = + cbm_registry_resolve(r, "bridge_execute", "proj.services.yui_core.router", keys, vals, 1); + ASSERT_STR_EQ(res.qualified_name, "proj.services.satori_bridge.gate.execute"); + ASSERT_STR_EQ(res.strategy, "import_map"); + + cbm_registry_free(r); + PASS(); +} + TEST(resolve_unique_name) { cbm_registry_t *r = cbm_registry_new(); cbm_registry_add(r, "UniqueFunc", "proj.deep.path.UniqueFunc", "Function"); @@ -860,6 +882,7 @@ SUITE(registry) { RUN_TEST(resolve_import_map); RUN_TEST(resolve_import_map_bare_function); RUN_TEST(resolve_import_map_bare_alias); + RUN_TEST(resolve_import_map_aliased_from_import); RUN_TEST(resolve_import_map_alias_with_suffix_hits_method); RUN_TEST(resolve_unique_name); RUN_TEST(resolve_unresolved); From 164d074d52c1b5a67f7c112276deefac1ab0f88b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B6=82=E5=90=8D=E6=81=A9?= <125283161+Joseph-MingEn@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:26:00 -0700 Subject: [PATCH 03/15] fix(python): prefer member Function on absolute from-import IMPORTS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_module often lands on the Module for pkg.mod.symbol paths. When the member Function/Class exists, IMPORTS must target it so \`from M import f as g\` + g() can CALLS→f (Yui bridge_execute shape). Co-authored-by: Cursor Signed-off-by: 涂名恩 <125283161+Joseph-MingEn@users.noreply.github.com> --- src/pipeline/pass_pkgmap.c | 25 ++++++++++++++++++ tests/test_lang_contract.c | 54 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/pipeline/pass_pkgmap.c b/src/pipeline/pass_pkgmap.c index 075378b12..e79c4e7a8 100644 --- a/src/pipeline/pass_pkgmap.c +++ b/src/pipeline/pass_pkgmap.c @@ -1520,6 +1520,31 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t const cbm_gbuf_node_t *target = target_qn ? cbm_gbuf_find_by_qn(ctx->gbuf, target_qn) : NULL; free(target_qn); if (target) { + /* Python/TS from-import of a member: module_path is often + * "pkg.mod.symbol" while resolve_module lands on the Module node + * "pkg.mod". Prefer the member Function/Class when it exists — + * required for `from M import f as g` so CALLS can import_map to f + * (local_name g ≠ f). */ + if (target->label && + (strcmp(target->label, "Module") == 0 || strcmp(target->label, "File") == 0)) { + char symbuf[256]; + const char *sym = import_candidate_symbol(imp->module_path, symbuf, sizeof(symbuf)); + if (sym && sym[0] && strcmp(sym, "*") != 0) { + const char *mod_tail = import_last_segment(target->qualified_name + ? target->qualified_name + : ""); + if (!mod_tail || strcmp(mod_tail, sym) != 0) { + char member_qn[CBM_SZ_512]; + snprintf(member_qn, sizeof(member_qn), "%s.%s", target->qualified_name, sym); + const cbm_gbuf_node_t *member = cbm_gbuf_find_by_qn(ctx->gbuf, member_qn); + if (member && import_targetable_label(member->label) && + strcmp(member->label, "Module") != 0 && + strcmp(member->label, "File") != 0) { + return member; + } + } + } + } return target; } diff --git a/tests/test_lang_contract.c b/tests/test_lang_contract.c index 31204e7d1..4b8dac93d 100644 --- a/tests/test_lang_contract.c +++ b/tests/test_lang_contract.c @@ -456,8 +456,7 @@ TEST(contract_python_relative_import) { PASS(); } -/* Python: aliased from-import must CALLS the real def (execute), not a ghost - * named after the local alias (bridge_execute). Mirrors Yui G1. */ +/* Python: relative aliased from-import must CALLS the real def (execute). */ TEST(contract_python_aliased_from_import_calls) { static const LangFile f[] = { {"gate.py", "def execute(x):\n return x\n"}, @@ -504,6 +503,56 @@ TEST(contract_python_aliased_from_import_calls) { PASS(); } +/* Python: absolute aliased from-import must CALLS the real def (Yui router shape). */ +TEST(contract_python_absolute_aliased_from_import_calls) { + static const LangFile f[] = { + {"services/satori_bridge/__init__.py", ""}, + {"services/satori_bridge/gate.py", "def execute(x):\n return x\n"}, + {"services/yui_core/__init__.py", ""}, + {"services/yui_core/router.py", + "from services.satori_bridge.gate import execute as bridge_execute\n\n\n" + "def submit_task(y):\n return bridge_execute(y)\n"}, + {"services/__init__.py", ""}}; + LangProj lp; + cbm_store_t *store = lang_index_files(&lp, f, 5); + ASSERT_TRUE(store != NULL); + cbm_node_t *nodes = NULL; + int ncount = 0; + ASSERT_EQ(cbm_store_find_nodes_by_name(store, lp.project, "submit_task", &nodes, &ncount), + CBM_STORE_OK); + ASSERT_TRUE(ncount >= 1); + char **callers = NULL; + char **callees = NULL; + int n_callers = 0; + int n_callees = 0; + ASSERT_EQ(cbm_store_node_neighbor_names(store, nodes[0].id, 32, &callers, &n_callers, &callees, + &n_callees), + 0); + int saw_execute = 0; + int saw_alias_ghost = 0; + for (int i = 0; i < n_callees; i++) { + if (callees[i] && strcmp(callees[i], "execute") == 0) { + saw_execute = 1; + } + if (callees[i] && strcmp(callees[i], "bridge_execute") == 0) { + saw_alias_ghost = 1; + } + } + for (int i = 0; i < n_callers; i++) { + free(callers[i]); + } + for (int i = 0; i < n_callees; i++) { + free(callees[i]); + } + free(callers); + free(callees); + cbm_store_free_nodes(nodes, ncount); + lang_cleanup(&lp, store); + ASSERT_TRUE(saw_execute); + ASSERT_FALSE(saw_alias_ghost); + PASS(); +} + /* TypeScript: a relative import resolves to a sibling module → IMPORTS edge. */ TEST(contract_typescript_relative_import) { static const LangFile f[] = { @@ -1674,6 +1723,7 @@ SUITE(lang_contract) { RUN_TEST(contract_kotlin_methods); RUN_TEST(contract_python_relative_import); RUN_TEST(contract_python_aliased_from_import_calls); + RUN_TEST(contract_python_absolute_aliased_from_import_calls); RUN_TEST(contract_typescript_relative_import); /* Graph-level breadth across all grammars (P4). */ From 61a0542de1b9e39531c1cfe07f2064082b7d9ddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B6=82=E5=90=8D=E6=81=A9?= <125283161+Joseph-MingEn@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:48:18 -0700 Subject: [PATCH 04/15] chore: refresh GitHub mergeability for #1371 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Web conflict editor reports conflicts too complex; local merge vs main is a clean fast-forward. Empty commit forces recompute. Co-authored-by: Cursor Signed-off-by: 涂名恩 <125283161+Joseph-MingEn@users.noreply.github.com> From e56e965dd171a31278d40678bc280e7851d0841e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B6=82=E5=90=8D=E6=81=A9?= <125283161+Joseph-MingEn@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:43:41 -0700 Subject: [PATCH 05/15] fix(python): clang-format pkgmap + ghost-module CALLS negative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unblock CI lint so the test job runs. Prefer Homebrew LLVM formatting on the member IMPORTS retarget. Add contract that `from ghost_module import nope as alias; alias()` emits no CALLS. Relabel registry alias pin as adversarial (behavior already on main). Signed-off-by: 涂名恩 <125283161+Joseph-MingEn@users.noreply.github.com> Co-authored-by: Cursor --- src/pipeline/pass_pkgmap.c | 5 ++--- tests/test_lang_contract.c | 42 ++++++++++++++++++++++++++++++++++++++ tests/test_registry.c | 6 +++--- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/pipeline/pass_pkgmap.c b/src/pipeline/pass_pkgmap.c index e79c4e7a8..fbba9709e 100644 --- a/src/pipeline/pass_pkgmap.c +++ b/src/pipeline/pass_pkgmap.c @@ -1530,9 +1530,8 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t char symbuf[256]; const char *sym = import_candidate_symbol(imp->module_path, symbuf, sizeof(symbuf)); if (sym && sym[0] && strcmp(sym, "*") != 0) { - const char *mod_tail = import_last_segment(target->qualified_name - ? target->qualified_name - : ""); + const char *mod_tail = + import_last_segment(target->qualified_name ? target->qualified_name : ""); if (!mod_tail || strcmp(mod_tail, sym) != 0) { char member_qn[CBM_SZ_512]; snprintf(member_qn, sizeof(member_qn), "%s.%s", target->qualified_name, sym); diff --git a/tests/test_lang_contract.c b/tests/test_lang_contract.c index 4b8dac93d..6b3bd36e3 100644 --- a/tests/test_lang_contract.c +++ b/tests/test_lang_contract.c @@ -553,6 +553,47 @@ TEST(contract_python_absolute_aliased_from_import_calls) { PASS(); } +/* Python: unresolvable module + alias must emit no CALLS (no invented edge). */ +TEST(contract_python_ghost_module_aliased_from_import_no_calls) { + static const LangFile f[] = { + {"router.py", + "from ghost_module import nope as alias\n\n\n" + "def submit_task(y):\n return alias(y)\n"}}; + LangProj lp; + cbm_store_t *store = lang_index_files(&lp, f, 1); + ASSERT_TRUE(store != NULL); + cbm_node_t *nodes = NULL; + int ncount = 0; + ASSERT_EQ(cbm_store_find_nodes_by_name(store, lp.project, "submit_task", &nodes, &ncount), + CBM_STORE_OK); + ASSERT_TRUE(ncount >= 1); + char **callers = NULL; + char **callees = NULL; + int n_callers = 0; + int n_callees = 0; + ASSERT_EQ(cbm_store_node_neighbor_names(store, nodes[0].id, 32, &callers, &n_callers, &callees, + &n_callees), + 0); + int saw_any_callee = 0; + for (int i = 0; i < n_callees; i++) { + if (callees[i] && callees[i][0]) { + saw_any_callee = 1; + } + } + for (int i = 0; i < n_callers; i++) { + free(callers[i]); + } + for (int i = 0; i < n_callees; i++) { + free(callees[i]); + } + free(callers); + free(callees); + cbm_store_free_nodes(nodes, ncount); + lang_cleanup(&lp, store); + ASSERT_FALSE(saw_any_callee); + PASS(); +} + /* TypeScript: a relative import resolves to a sibling module → IMPORTS edge. */ TEST(contract_typescript_relative_import) { static const LangFile f[] = { @@ -1724,6 +1765,7 @@ SUITE(lang_contract) { RUN_TEST(contract_python_relative_import); RUN_TEST(contract_python_aliased_from_import_calls); RUN_TEST(contract_python_absolute_aliased_from_import_calls); + RUN_TEST(contract_python_ghost_module_aliased_from_import_no_calls); RUN_TEST(contract_typescript_relative_import); /* Graph-level breadth across all grammars (P4). */ diff --git a/tests/test_registry.c b/tests/test_registry.c index 42c6dae60..07dfb71c9 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -386,9 +386,9 @@ TEST(resolve_import_map_bare_alias) { PASS(); } -/* from M import execute as bridge_execute + bridge_execute() — IMPORTS target is - * already the def QN. Must CALLS→execute, not invent …M.bridge_execute. - * Yui G1 shape (alongside resolve_import_map_bare_alias). */ +/* Adversarial pin: when import_map already stores the def QN under the alias + * key, bare alias call must CALLS→def (not invent …M.bridge_execute). Behavior + * already on main via #875/#979; this locks the Yui G1 shape. */ TEST(resolve_import_map_aliased_from_import) { cbm_registry_t *r = cbm_registry_new(); cbm_registry_add(r, "execute", "proj.services.satori_bridge.gate.execute", "Function"); From d613840c954a51b9b36da561fb2ef8fcce52c3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B6=82=E5=90=8D=E6=81=A9?= <125283161+Joseph-MingEn@users.noreply.github.com> Date: Thu, 30 Jul 2026 23:06:25 -0700 Subject: [PATCH 06/15] chore: retrigger CI after unrelated macos-15-intel lock_registry flake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream workflow rerun requires admin; empty signed commit to re-queue. Signed-off-by: 涂名恩 <125283161+Joseph-MingEn@users.noreply.github.com> Co-authored-by: Cursor From 506151f6c19cf5b80e9ea91ccecc674bb5d253ee Mon Sep 17 00:00:00 2001 From: Ertan Date: Thu, 13 Aug 2026 21:16:20 +0300 Subject: [PATCH 07/15] fix(mcp): propagate cancellation to Windows code search Signed-off-by: Ertan --- src/mcp/mcp.c | 109 +++++++++++++++++++++++++++++++-- src/mcp/mcp_internal.h | 1 + tests/test_mcp.c | 134 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 240 insertions(+), 4 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 411c15ebd..3250042dc 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -123,6 +123,7 @@ enum { * growing getline buffers without bound through ignored extension headers. */ #define MCP_MAX_MESSAGE_SIZE ((size_t)10U * 1024U * 1024U) #define MCP_MAX_HEADER_SIZE ((size_t)8U * 1024U) +#define MCP_SEARCH_OUTPUT_MAX ((size_t)64U * 1024U * 1024U) /* ── Helpers ────────────────────────────────────────────────────── */ @@ -1569,6 +1570,7 @@ struct cbm_mcp_server { void *quarantine_test_context; cbm_mcp_command_test_hook_fn command_test_hook; void *command_test_context; + size_t search_output_limit_override; cbm_thread_t autoindex_tid; bool autoindex_active; /* true if auto-index thread was started */ @@ -1867,6 +1869,12 @@ void cbm_mcp_server_set_command_test_hook(cbm_mcp_server_t *srv, cbm_mcp_command srv->command_test_context = context; } +void cbm_mcp_server_set_search_output_limit_for_test(cbm_mcp_server_t *srv, size_t limit) { + if (srv) { + srv->search_output_limit_override = limit; + } +} + /* ── Cache dir + project DB path helpers ───────────────────────── */ /* Returns the cache directory. Writes to buf, returns buf for convenience. */ @@ -9686,6 +9694,32 @@ static bool compile_path_filter(const char *filter, cbm_regex_t *re) { return cbm_regcomp(re, filter, CBM_REG_EXTENDED | CBM_REG_NOSUB) == CBM_REG_OK; } +static int mcp_run_shell_command_cancellable_bounded(cbm_mcp_server_t *srv, const char *command, + char output_path[CBM_SZ_2K], + size_t output_limit, + bool *output_limit_exceeded, + cbm_proc_result_t *result_out); + +#ifdef _WIN32 +static char *search_code_scan_error(search_scratch_t *scratch, const char *output_path, + bool has_path_filter, cbm_regex_t *path_regex, char *root_path, + char *pattern, char *project, char *file_pattern, + const char *message) { + if (output_path && output_path[0]) { + (void)cbm_unlink(output_path); + } + search_scratch_close(scratch); + if (has_path_filter) { + cbm_regfree(path_regex); + } + free(root_path); + free(pattern); + free(project); + free(file_pattern); + return cbm_mcp_text_result(message, true); +} +#endif + static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { char *pattern = cbm_mcp_get_string_arg(args, "pattern"); char *project = get_project_arg(args); @@ -9869,6 +9903,44 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { cbm_search_code_build_grep_cmd(cmd, sizeof(cmd), use_regex, scoped, file_pattern, tmpfile, filelist, root_path); +#ifdef _WIN32 + char output_path[CBM_SZ_2K] = {0}; + cbm_proc_result_t scan_result = {0}; + bool scan_output_exceeded = false; + size_t scan_output_limit = srv->search_output_limit_override + ? srv->search_output_limit_override + : MCP_SEARCH_OUTPUT_MAX; + int scan_run = mcp_run_shell_command_cancellable_bounded( + srv, cmd, output_path, scan_output_limit, &scan_output_exceeded, &scan_result); + if (scan_output_exceeded) { + char message[CBM_SZ_128]; + snprintf(message, sizeof(message), + "search failed: output exceeded the %zu-byte safety limit", scan_output_limit); + return search_code_scan_error(&scratch, output_path, has_path_filter, &path_regex, + root_path, pattern, project, file_pattern, message); + } + bool scan_cancelled = scan_result.cancellation_requested || mcp_request_cancelled(srv); + if (scan_cancelled) { + return search_code_scan_error(&scratch, output_path, has_path_filter, &path_regex, + root_path, pattern, project, file_pattern, + "search_code cancelled for this request"); + } + if (scan_run != 0) { + return search_code_scan_error( + &scratch, output_path, has_path_filter, &path_regex, root_path, pattern, project, + file_pattern, "search failed: the contained command could not complete"); + } + FILE *fp = cbm_fopen(output_path, "rb"); + if (!fp) { + return search_code_scan_error(&scratch, output_path, has_path_filter, &path_regex, + root_path, pattern, project, file_pattern, + "search failed: contained output could not be read"); + } + gm = collect_grep_matches(fp, root_path, strlen(root_path), has_path_filter, &path_regex, + grep_limit, &gm_count); + (void)fclose(fp); + (void)cbm_unlink(output_path); +#else FILE *fp = cbm_popen(cmd, "r"); if (!fp) { search_scratch_close(&scratch); @@ -9885,6 +9957,7 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { gm = collect_grep_matches(fp, root_path, strlen(root_path), has_path_filter, &path_regex, grep_limit, &gm_count); cbm_pclose(fp); +#endif /* Both scratch files and the private directory go here — unlike the old * code, the file list is removed even when the scan was not scoped. */ search_scratch_close(&scratch); @@ -10045,10 +10118,16 @@ static bool mcp_resolve_windows_cmd(char out[CBM_SZ_4K]) { } #endif -static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char *command, - char output_path[CBM_SZ_2K], - cbm_proc_result_t *result_out) { - if (!srv || !command || !output_path || !result_out || !mcp_command_output_path(output_path)) { +static int mcp_run_shell_command_cancellable_bounded(cbm_mcp_server_t *srv, const char *command, + char output_path[CBM_SZ_2K], + size_t output_limit, + bool *output_limit_exceeded, + cbm_proc_result_t *result_out) { + if (output_limit_exceeded) { + *output_limit_exceeded = false; + } + if (!srv || !command || !output_path || !result_out || + (output_limit > 0 && !output_limit_exceeded) || !mcp_command_output_path(output_path)) { return -1; } /* Internal test seam: rejecting after output allocation exercises the same @@ -10087,10 +10166,18 @@ static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char * } cbm_proc_poll_t state; + bool limit_exceeded = false; for (;;) { if (mcp_request_cancelled(srv)) { (void)cbm_subprocess_request_cancel(process); } + if (!limit_exceeded && output_limit > 0) { + int64_t output_size = cbm_file_size(output_path); + if (output_size > 0 && (uint64_t)output_size > output_limit) { + limit_exceeded = true; + (void)cbm_subprocess_request_cancel(process); + } + } state = cbm_subprocess_poll(process, result_out); if (state != CBM_PROC_POLL_RUNNING) { break; @@ -10100,9 +10187,23 @@ static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char * bool contained = state == CBM_PROC_POLL_TERMINAL && result_out->tree_quiesced && !result_out->supervision_failed; cbm_subprocess_destroy(process); + if (!limit_exceeded && output_limit > 0) { + int64_t final_size = cbm_file_size(output_path); + limit_exceeded = final_size > 0 && (uint64_t)final_size > output_limit; + } + if (output_limit_exceeded) { + *output_limit_exceeded = limit_exceeded; + } return contained ? 0 : -1; } +static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char *command, + char output_path[CBM_SZ_2K], + cbm_proc_result_t *result_out) { + return mcp_run_shell_command_cancellable_bounded(srv, command, output_path, 0, NULL, + result_out); +} + /* Does `node`'s line range overlap any recorded hunk for `file`? Used to scope * seed detection to the actually-changed lines rather than the whole file. * Non-static (declared in mcp_internal.h) so tests can exercise the overlap diff --git a/src/mcp/mcp_internal.h b/src/mcp/mcp_internal.h index 7f08f6deb..d9321c595 100644 --- a/src/mcp/mcp_internal.h +++ b/src/mcp/mcp_internal.h @@ -14,6 +14,7 @@ void cbm_mcp_server_set_quarantine_test_hook(cbm_mcp_server_t *srv, cbm_mcp_quarantine_test_hook_fn hook, void *context); void cbm_mcp_server_set_command_test_hook(cbm_mcp_server_t *srv, cbm_mcp_command_test_hook_fn hook, void *context); +void cbm_mcp_server_set_search_output_limit_for_test(cbm_mcp_server_t *srv, size_t limit); /* Release only the constructor-created pristine in-memory store. Public * cbm_mcp_server_new(NULL) semantics remain unchanged; daemon sessions use diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 88b4cab47..20f597d9c 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -162,6 +162,21 @@ typedef struct { int merge_base_calls; } mcp_command_hook_probe_t; +#ifdef _WIN32 +typedef struct { + cbm_mcp_server_t *server; + bool cancel_on_call; + bool cancel_accepted; + int calls; + char command[CBM_SZ_4K]; +} mcp_search_command_probe_t; + +typedef struct { + char path[512]; + char *saved_cache; +} mcp_search_cache_t; +#endif + static bool mcp_quarantine_hook_probe(void *context, const char *step) { mcp_quarantine_hook_probe_t *probe = context; if (!probe || !step) { @@ -187,6 +202,45 @@ static bool mcp_command_hook_probe(void *context, const char *command) { return true; } +#ifdef _WIN32 +static bool mcp_search_command_hook_probe(void *context, const char *command) { + mcp_search_command_probe_t *probe = context; + if (!probe || !command) { + return false; + } + probe->calls++; + snprintf(probe->command, sizeof(probe->command), "%s", command); + if (probe->cancel_on_call && probe->server) { + probe->cancel_accepted = cbm_mcp_server_cancel_active(probe->server); + } + return true; +} + +static bool mcp_search_cache_open(mcp_search_cache_t *cache, const char *prefix) { + memset(cache, 0, sizeof(*cache)); + snprintf(cache->path, sizeof(cache->path), "%s/%s-XXXXXX", cbm_tmpdir(), prefix); + if (!cbm_mkdtemp(cache->path)) { + return false; + } + const char *saved_cache = getenv("CBM_CACHE_DIR"); + cache->saved_cache = saved_cache ? strdup(saved_cache) : NULL; + if ((saved_cache && !cache->saved_cache) || cbm_setenv("CBM_CACHE_DIR", cache->path, 1) != 0) { + free(cache->saved_cache); + cache->saved_cache = NULL; + (void)th_rmtree(cache->path); + return false; + } + return true; +} + +static bool mcp_search_cache_close(mcp_search_cache_t *cache) { + restore_cache_dir(cache->saved_cache); + free(cache->saved_cache); + cache->saved_cache = NULL; + return th_rmtree(cache->path) == 0; +} +#endif + typedef struct { const char *name; char *value; @@ -4543,6 +4597,84 @@ TEST(search_code_windows_prefilter_precedes_content_scan) { #endif } +TEST(search_code_windows_cancel_cleans_supervised_scan) { +#ifdef _WIN32 + mcp_search_cache_t cache; + ASSERT_TRUE(mcp_search_cache_open(&cache, "cbm-search-cancel")); + + char tmp[512], src_path[768], vendor_path[768]; + cbm_mcp_server_t *srv = setup_prefilter_server(tmp, sizeof(tmp), src_path, sizeof(src_path), + vendor_path, sizeof(vendor_path)); + ASSERT_NOT_NULL(srv); + mcp_search_command_probe_t probe = { + .server = srv, + .cancel_on_call = true, + }; + cbm_mcp_server_set_command_test_hook(srv, mcp_search_command_hook_probe, &probe); + + char *response = + cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"HandleRequest\",\"project\":\"prefilter-search\"," + "\"file_pattern\":\"*.go\"}"); + ASSERT_NOT_NULL(response); + ASSERT_TRUE(probe.cancel_accepted); + ASSERT_NOT_NULL(strstr(response, "cancelled")); + ASSERT_NOT_NULL(strstr(response, "\"isError\":true")); + + char logs[640]; + snprintf(logs, sizeof(logs), "%s/logs", cache.path); + ASSERT_EQ(mcp_count_directory_entries_with_prefix(logs, ".mcp-command-"), 0); + + free(response); + cbm_mcp_server_free(srv); + cleanup_prefilter_dir(tmp, src_path, vendor_path); + ASSERT_TRUE(mcp_search_cache_close(&cache)); + PASS(); +#else + SKIP_PLATFORM("supervised Select-String cancellation runs on Windows"); +#endif +} + +TEST(search_code_windows_output_limit_fails_closed_and_cleans_scan) { +#ifdef _WIN32 + mcp_search_cache_t cache; + ASSERT_TRUE(mcp_search_cache_open(&cache, "cbm-search-limit")); + + char tmp[512], src_path[768], vendor_path[768]; + cbm_mcp_server_t *srv = setup_prefilter_server(tmp, sizeof(tmp), src_path, sizeof(src_path), + vendor_path, sizeof(vendor_path)); + ASSERT_NOT_NULL(srv); + cbm_mcp_server_set_search_output_limit_for_test(srv, 512); + + FILE *source = cbm_fopen(src_path, "ab"); + ASSERT_NOT_NULL(source); + for (int i = 0; i < 256; i++) { + ASSERT_GT(fprintf(source, "func HandleRequest%d() error { return nil }\n", i), 0); + } + ASSERT_EQ(fclose(source), 0); + + char *response = + cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"HandleRequest\",\"project\":\"prefilter-search\"," + "\"file_pattern\":\"*.go\"}"); + ASSERT_NOT_NULL(response); + ASSERT_NOT_NULL(strstr(response, "output exceeded")); + ASSERT_NOT_NULL(strstr(response, "\"isError\":true")); + + char logs[640]; + snprintf(logs, sizeof(logs), "%s/logs", cache.path); + ASSERT_EQ(mcp_count_directory_entries_with_prefix(logs, ".mcp-command-"), 0); + + free(response); + cbm_mcp_server_free(srv); + cleanup_prefilter_dir(tmp, src_path, vendor_path); + ASSERT_TRUE(mcp_search_cache_close(&cache)); + PASS(); +#else + SKIP_PLATFORM("supervised Select-String output limit runs on Windows"); +#endif +} + /* issue #283: search_code with regex=true and a syntactically invalid pattern * must return an explicit error, not an empty result indistinguishable from a * legitimate no-match. */ @@ -10806,6 +10938,8 @@ SUITE(mcp) { RUN_TEST(search_code_path_filter_matches_nothing); RUN_TEST(search_code_file_pattern_prefilter_boundaries); RUN_TEST(search_code_windows_prefilter_precedes_content_scan); + RUN_TEST(search_code_windows_cancel_cleans_supervised_scan); + RUN_TEST(search_code_windows_output_limit_fails_closed_and_cleans_scan); RUN_TEST(search_code_invalid_regex_errors_issue283); RUN_TEST(search_code_literal_pipe_warns_issue282); RUN_TEST(search_code_reports_phase_timings_only_in_debug_mode); From 0eb22f02056746464525acec4d1e8884f9f4f24c Mon Sep 17 00:00:00 2001 From: astandrik Date: Mon, 17 Aug 2026 22:07:11 +0300 Subject: [PATCH 08/15] fix(test): isolate harness daemon runtimes Signed-off-by: astandrik --- scripts/security-fuzz-random.sh | 7 +- scripts/security-fuzz.sh | 20 +-- scripts/security-install.sh | 9 +- scripts/security-network.sh | 7 +- scripts/test-runtime.sh | 162 +++++++++++++++++++++++ scripts/test.sh | 5 +- tests/test_hook_conflict_notice.sh | 7 +- tests/test_parent_watchdog.sh | 7 +- tests/test_runtime_isolation_contract.sh | 102 ++++++++++++++ tests/test_security_fuzz_harness.sh | 23 +++- tests/test_worker_error_response.sh | 7 +- tests/test_worker_watchdog.sh | 7 +- 12 files changed, 331 insertions(+), 32 deletions(-) create mode 100644 scripts/test-runtime.sh create mode 100644 tests/test_runtime_isolation_contract.sh diff --git a/scripts/security-fuzz-random.sh b/scripts/security-fuzz-random.sh index baa71e5ae..9b4903b4f 100755 --- a/scripts/security-fuzz-random.sh +++ b/scripts/security-fuzz-random.sh @@ -27,8 +27,11 @@ fi echo "=== Fuzz Testing ($DURATION seconds) ===" -FUZZ_TMPDIR=$(mktemp -d) -trap 'rm -rf "$FUZZ_TMPDIR"' EXIT +# shellcheck source=test-runtime.sh +source "$(dirname "${BASH_SOURCE[0]}")/test-runtime.sh" +cbm_test_runtime_init +FUZZ_TMPDIR="$CBM_TEST_RUNTIME_ROOT" +trap 'cbm_test_runtime_cleanup "$BINARY"' EXIT CRASHES=0 ITERATIONS=0 diff --git a/scripts/security-fuzz.sh b/scripts/security-fuzz.sh index bdf1ca58d..2806989a9 100755 --- a/scripts/security-fuzz.sh +++ b/scripts/security-fuzz.sh @@ -22,11 +22,14 @@ PASS=0 TOTAL=0 # Temp directory for input files (avoids pipe/stdin issues with timeout) -FUZZ_TMPDIR=$(mktemp -d) -trap 'rm -rf "$FUZZ_TMPDIR"' EXIT +# shellcheck source=test-runtime.sh +source "$(dirname "${BASH_SOURCE[0]}")/test-runtime.sh" +cbm_test_runtime_init +FUZZ_TMPDIR="$CBM_TEST_RUNTIME_ROOT" +trap 'cbm_test_runtime_cleanup "$BINARY"' EXIT FUZZ_HOME="$FUZZ_TMPDIR/home" -FUZZ_CACHE="$FUZZ_TMPDIR/cache" -mkdir -p "$FUZZ_HOME" "$FUZZ_CACHE" +mkdir "$FUZZ_HOME" +export HOME="$FUZZ_HOME" # Helper: send a payload to the MCP server and check it doesn't crash. # Uses temp file + perl alarm for portable timeout (works on macOS + Linux). @@ -49,11 +52,9 @@ test_payload() { # Run with 10s timeout: GNU timeout → perl alarm fallback local ec=0 if command -v timeout &>/dev/null; then - HOME="$FUZZ_HOME" CBM_CACHE_DIR="$FUZZ_CACHE" \ - timeout 10 "$BINARY" < "$tmpinput" > "$tmpoutput" 2>&1 || ec=$? + timeout 10 "$BINARY" < "$tmpinput" > "$tmpoutput" 2>&1 || ec=$? else - HOME="$FUZZ_HOME" CBM_CACHE_DIR="$FUZZ_CACHE" \ - perl -e 'alarm(10); exec @ARGV' -- "$BINARY" \ + perl -e 'alarm(10); exec @ARGV' -- "$BINARY" \ < "$tmpinput" > "$tmpoutput" 2>&1 || ec=$? fi @@ -88,8 +89,7 @@ test_invalid_index_interactive() { local tmpoutput="$FUZZ_TMPDIR/output_${TOTAL}.jsonl" local ec=0 - HOME="$FUZZ_HOME" CBM_CACHE_DIR="$FUZZ_CACHE" \ - python3 "$(dirname "$0")/test_mcp_interactive.py" "$BINARY" \ + python3 "$(dirname "$0")/test_mcp_interactive.py" "$BINARY" \ --scenario invalid-index --repo-path /nonexistent/path/abc123 \ --response-timeout 45 --exit-timeout 15 > "$tmpoutput" 2>&1 || ec=$? diff --git a/scripts/security-install.sh b/scripts/security-install.sh index 291d8d7f4..ac9d74fb9 100755 --- a/scripts/security-install.sh +++ b/scripts/security-install.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash set -euo pipefail -# Layer 4: Install output audit — verifies install --dry-run writes only to expected paths. +# Layer 4: Install output audit — runs a sandboxed real install and audits its writes. # # Checks: # 1. All output file paths are in the expected set @@ -19,8 +19,11 @@ fi echo "=== Layer 4: Install Output Audit ===" -TMPDIR=$(mktemp -d) -trap 'rm -rf "$TMPDIR"' EXIT +# shellcheck source=test-runtime.sh +source "$(dirname "${BASH_SOURCE[0]}")/test-runtime.sh" +cbm_test_runtime_init +TMPDIR="$CBM_TEST_RUNTIME_ROOT" +trap 'cbm_test_runtime_cleanup "$BINARY"' EXIT # Set HOME to tmpdir so install writes there instead of real home export HOME="$TMPDIR/home" diff --git a/scripts/security-network.sh b/scripts/security-network.sh index 60b64abdc..d7e83c1d9 100755 --- a/scripts/security-network.sh +++ b/scripts/security-network.sh @@ -30,8 +30,11 @@ if ! command -v strace &>/dev/null; then exit 0 fi -TMPDIR=$(mktemp -d) -trap 'rm -rf "$TMPDIR"' EXIT +# shellcheck source=test-runtime.sh +source "$(dirname "${BASH_SOURCE[0]}")/test-runtime.sh" +cbm_test_runtime_init +TMPDIR="$CBM_TEST_RUNTIME_ROOT" +trap 'cbm_test_runtime_cleanup "$BINARY"' EXIT # Create a minimal test project mkdir -p "$TMPDIR/project/src" diff --git a/scripts/test-runtime.sh b/scripts/test-runtime.sh new file mode 100644 index 000000000..cb69cadfb --- /dev/null +++ b/scripts/test-runtime.sh @@ -0,0 +1,162 @@ +#!/usr/bin/env bash + +# Sourced by test harnesses. Host paths stay separate from paths exported to a +# native Windows product binary. +CBM_TEST_RUNTIME_ROOT="" +CBM_TEST_RUNTIME_DIR_HOST="" +CBM_TEST_CACHE_DIR_HOST="" +_CBM_TEST_RUNTIME_CREATED_ROOT="" +_CBM_TEST_RUNTIME_PRODUCT_RUNTIME="" +_CBM_TEST_RUNTIME_PRODUCT_CACHE="" + +_cbm_test_runtime_error() { + echo "test-runtime: $*" >&2 +} + +_cbm_test_runtime_discard_partial() { + local root="$1" parent="$2" name="${1##*/}" + [[ -n "$root" && "$root" != "$name" && "${root%/*}" == "$parent" && ! -L "$root" ]] || return 0 + case "$name" in cbmrt.?*|cbmrt-?*) rm -rf -- "$root" >/dev/null 2>&1 || true ;; esac +} + +cbm_test_runtime_init() { + [[ -z "$_CBM_TEST_RUNTIME_CREATED_ROOT" ]] || + { _cbm_test_runtime_error "runtime already initialized"; return 1; } + local platform windows=0 parent root="" runtime_host cache_host + local product_runtime product_cache helper_dir helper_native root_native + platform="$(uname -s)" || + { _cbm_test_runtime_error "cannot identify the host platform"; return 1; } + case "$platform" in + MINGW*|MSYS*|CYGWIN*) windows=1 ;; + Darwin) parent="/private/tmp" ;; + *) parent="/tmp" ;; + esac + if (( windows )); then + command -v cygpath >/dev/null 2>&1 || + { _cbm_test_runtime_error "cygpath is required on Windows"; return 1; } + if [[ -n "${CBM_CI_TEMP_ROOT:-}" ]]; then + parent="$(cygpath -u "$CBM_CI_TEMP_ROOT")" || + { _cbm_test_runtime_error "cannot convert CBM_CI_TEMP_ROOT"; return 1; } + [[ -d "$parent" && ! -L "$parent" ]] || + { _cbm_test_runtime_error "CBM_CI_TEMP_ROOT is not a real directory"; return 1; } + root="$(mktemp -d "${parent%/}/cbmrt.XXXXXX")" || + { _cbm_test_runtime_error "cannot create a private Windows CI root"; return 1; } + else + helper_dir="$(CDPATH= cd -- "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || return 1 + helper_native="$(cygpath -w "$helper_dir/ci/new-protected-temp-root.ps1")" || + { _cbm_test_runtime_error "cannot convert protected-root helper"; return 1; } + root_native="$(MSYS2_ARG_CONV_EXCL='*' powershell.exe -NoProfile \ + -ExecutionPolicy Bypass -File "$helper_native" -Prefix 'cbmrt-')" || + { _cbm_test_runtime_error "protected Windows root creation failed"; return 1; } + root_native="${root_native//$'\r'/}" + [[ -n "$root_native" && "$root_native" != *$'\n'* ]] || + { _cbm_test_runtime_error "protected-root helper returned an invalid path"; return 1; } + root="$(cygpath -u "$root_native")" || + { _cbm_test_runtime_error "cannot convert protected Windows root"; return 1; } + parent="${root%/*}" + fi + else + root="$(mktemp -d "${parent%/}/cbmrt.XXXXXX")" || + { _cbm_test_runtime_error "cannot create private root under $parent"; return 1; } + chmod 700 "$root" || { + _cbm_test_runtime_discard_partial "$root" "$parent" + _cbm_test_runtime_error "cannot protect runtime root $root" + return 1 + } + fi + runtime_host="$root/runtime" + cache_host="$root/cache" + mkdir "$runtime_host" "$cache_host" || { + _cbm_test_runtime_discard_partial "$root" "$parent" + _cbm_test_runtime_error "cannot create runtime/cache directories" + return 1 + } + if (( ! windows )); then + chmod 700 "$runtime_host" "$cache_host" || { + _cbm_test_runtime_discard_partial "$root" "$parent" + _cbm_test_runtime_error "cannot protect runtime/cache directories" + return 1 + } + product_runtime="$runtime_host" + product_cache="$cache_host" + else + product_runtime="$(cygpath -m "$runtime_host")" || { + _cbm_test_runtime_discard_partial "$root" "$parent" + _cbm_test_runtime_error "cannot convert the runtime directory" + return 1 + } + product_cache="$(cygpath -m "$cache_host")" || { + _cbm_test_runtime_discard_partial "$root" "$parent" + _cbm_test_runtime_error "cannot convert the cache directory" + return 1 + } + fi + CBM_TEST_RUNTIME_ROOT="$root" + CBM_TEST_RUNTIME_DIR_HOST="$runtime_host" + CBM_TEST_CACHE_DIR_HOST="$cache_host" + _CBM_TEST_RUNTIME_CREATED_ROOT="$root" + _CBM_TEST_RUNTIME_PRODUCT_RUNTIME="$product_runtime" + _CBM_TEST_RUNTIME_PRODUCT_CACHE="$product_cache" + export CBM_RUNTIME_DIR="$product_runtime" CBM_CACHE_DIR="$product_cache" +} + +_cbm_test_runtime_daemon() { + CBM_RUNTIME_DIR="$_CBM_TEST_RUNTIME_PRODUCT_RUNTIME" \ + CBM_CACHE_DIR="$_CBM_TEST_RUNTIME_PRODUCT_CACHE" "$1" daemon "$2" +} + +cbm_test_runtime_cleanup() { + local binary="${1:-}" root="$_CBM_TEST_RUNTIME_CREATED_ROOT" + local name="${_CBM_TEST_RUNTIME_CREATED_ROOT##*/}" runtime_entry="" active=0 + [[ -n "$root" ]] || return 0 + case "$name" in + cbmrt.?*|cbmrt-?*) ;; + *) _cbm_test_runtime_error "refusing cleanup for unexpected basename: $name"; return 0 ;; + esac + if [[ "$root" != "$CBM_TEST_RUNTIME_ROOT" || -L "$root" ]]; then + _cbm_test_runtime_error "refusing cleanup for an unexpected runtime root: $root" + return 0 + fi + if [[ ! -e "$root" ]]; then + _CBM_TEST_RUNTIME_CREATED_ROOT="" + return 0 + fi + if [[ ! -d "$root" || "$CBM_TEST_RUNTIME_DIR_HOST" != "$root/runtime" || + "$CBM_TEST_CACHE_DIR_HOST" != "$root/cache" || + -L "$CBM_TEST_RUNTIME_DIR_HOST" || -L "$CBM_TEST_CACHE_DIR_HOST" ]]; then + _cbm_test_runtime_error "refusing cleanup after runtime path replacement: $root" + return 0 + fi + if [[ -d "$CBM_TEST_RUNTIME_DIR_HOST" ]] && + ! runtime_entry="$(find "$CBM_TEST_RUNTIME_DIR_HOST" -mindepth 1 -print -quit 2>/dev/null)"; then + _cbm_test_runtime_error "cannot inspect runtime state; leaving $root" + return 0 + fi + if [[ -n "$runtime_entry" ]]; then + [[ -x "$binary" ]] || { + _cbm_test_runtime_error "cannot check the private daemon; leaving $root" + return 0 + } + if _cbm_test_runtime_daemon "$binary" status >/dev/null 2>&1; then + active=1 + _cbm_test_runtime_daemon "$binary" stop >/dev/null 2>&1 || true + for _attempt in {1..50}; do + if ! _cbm_test_runtime_daemon "$binary" status >/dev/null 2>&1; then + active=0 + break + fi + sleep 0.1 + done + fi + fi + if (( active )); then + _cbm_test_runtime_error "private daemon is still active; leaving $root" + return 0 + fi + if rm -rf -- "$root"; then + _CBM_TEST_RUNTIME_CREATED_ROOT="" + else + _cbm_test_runtime_error "cleanup failed; leaving $root" + fi + return 0 +} diff --git a/scripts/test.sh b/scripts/test.sh index 2cce175ac..0ec489a59 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -14,7 +14,7 @@ Usage: scripts/test.sh [--suites LIST] [--arch ARCH] [VAR=VAL ...] The canonical test entry: identical in local CI, PR CI, dry run and release. DEFAULT (no --suites) is exactly what CI runs: static contract checks -(Step 0a-0s), a CLEAN sanitizer build, every suite via the parallel harness, +(Step 0a-0t), a CLEAN sanitizer build, every suite via the parallel harness, then the prod-binary regression guards (Steps 4-6). Modes: @@ -257,6 +257,9 @@ bash "$ROOT/tests/test_vt_candidate_selection_contract.sh" echo "=== Step 0s: release gate-chain ordering contract ===" bash "$ROOT/tests/test_release_gate_chain_contract.sh" +echo "=== Step 0t: test runtime isolation contract (#1691) ===" +bash "$ROOT/tests/test_runtime_isolation_contract.sh" + # Verify compiler supports target arch verify_compiler "$CC" diff --git a/tests/test_hook_conflict_notice.sh b/tests/test_hook_conflict_notice.sh index e44b0f327..35cd844d1 100644 --- a/tests/test_hook_conflict_notice.sh +++ b/tests/test_hook_conflict_notice.sh @@ -44,10 +44,13 @@ if ! LC_ALL=C grep -a -q -F 'CBM_TEST_HOOK_CLIENT_BUILD' "${BINARY}"; then exit 2 fi -tmpdir="$(mktemp -d)" +# shellcheck source=../scripts/test-runtime.sh +source "${ROOT}/scripts/test-runtime.sh" +cbm_test_runtime_init +tmpdir="${CBM_TEST_RUNTIME_ROOT}" cleanup() { CBM_CACHE_DIR="${tmpdir}/cache" "${BINARY}" daemon stop >/dev/null 2>&1 || true - rm -rf "${tmpdir}" + cbm_test_runtime_cleanup "${BINARY}" } trap cleanup EXIT diff --git a/tests/test_parent_watchdog.sh b/tests/test_parent_watchdog.sh index 87f938d6e..512161a36 100755 --- a/tests/test_parent_watchdog.sh +++ b/tests/test_parent_watchdog.sh @@ -25,7 +25,10 @@ if [[ ! -x "${BINARY}" ]]; then exit 2 fi -tmpdir="$(mktemp -d)" +# shellcheck source=../scripts/test-runtime.sh +source "${ROOT}/scripts/test-runtime.sh" +cbm_test_runtime_init +tmpdir="${CBM_TEST_RUNTIME_ROOT}" wrapper_pid="" cleanup() { if [[ -s "${tmpdir}/child.pid" ]]; then @@ -34,7 +37,7 @@ cleanup() { [[ -n "${child_pid}" ]] && kill "${child_pid}" 2>/dev/null || true fi [[ -n "${wrapper_pid}" ]] && kill "${wrapper_pid}" 2>/dev/null || true - rm -rf "${tmpdir}" + cbm_test_runtime_cleanup "${BINARY}" } trap cleanup EXIT diff --git a/tests/test_runtime_isolation_contract.sh b/tests/test_runtime_isolation_contract.sh new file mode 100644 index 000000000..217230c7f --- /dev/null +++ b/tests/test_runtime_isolation_contract.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +set -euo pipefail +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +WORKDIR="$(mktemp -d)" +trap 'rm -rf "$WORKDIR"' EXIT +fail() { + echo "FAIL: $*" >&2 + exit 1 +} +normalize_path() { + local path="${1%$'\r'}" + if command -v cygpath >/dev/null 2>&1; then + cygpath -u "$path" 2>/dev/null && return 0 + fi + printf '%s\n' "${path//\\//}" +} +INSTALL_FIXTURE="$WORKDIR/install-fixture" +cat > "$INSTALL_FIXTURE" <<'EOF' +#!/usr/bin/env bash +[[ "$1 $2" == "daemon status" ]] && exit 1 +if [[ "$1 $2" == "install -y" ]]; then + printf '%s\t%s\t%s\n' "$HOME" "$CBM_CACHE_DIR" "$CBM_RUNTIME_DIR" > "$CBM_TEST_INSTALL_ENV_PROBE" +fi +exit 0 +EOF +chmod +x "$INSTALL_FIXTURE" +CONTROL="$INSTALL_FIXTURE" +CALLER_ROOT="$WORKDIR/caller-a" +mkdir -p "$CALLER_ROOT/runtime" "$CALLER_ROOT/cache" +touch "$CALLER_ROOT/sentinel" +( + export CBM_RUNTIME_DIR="$CALLER_ROOT/runtime" + export CBM_CACHE_DIR="$CALLER_ROOT/cache" + source "$ROOT/scripts/test-runtime.sh" + cbm_test_runtime_init + [[ "$CBM_RUNTIME_DIR" != "$CALLER_ROOT/runtime" ]] || fail "inherited runtime was reused" + [[ "$CBM_CACHE_DIR" != "$CALLER_ROOT/cache" ]] || fail "inherited cache was reused" + private_root="$CBM_TEST_RUNTIME_ROOT" + [[ -d "$private_root/runtime" && -d "$private_root/cache" ]] || fail "private directories missing" + cbm_test_runtime_cleanup "$CONTROL" + [[ ! -e "$private_root" ]] || fail "normal cleanup left its private root" +) +[[ -f "$CALLER_ROOT/sentinel" ]] || fail "cleanup touched the caller root" +for slot in 1 2; do + ( + source "$ROOT/scripts/test-runtime.sh" + cbm_test_runtime_init + printf '%s\n' "$CBM_TEST_RUNTIME_ROOT" > "$WORKDIR/root-$slot" + cbm_test_runtime_cleanup "$CONTROL" + ) & +done +wait +[[ "$(<"$WORKDIR/root-1")" != "$(<"$WORKDIR/root-2")" ]] || fail "parallel roots collided" +FAIL_FIXTURE="$WORKDIR/failure-fixture" +cat > "$FAIL_FIXTURE" <<'EOF' +#!/usr/bin/env bash +touch "$CBM_TEST_FAILURE_MARKER" +EOF +chmod +x "$FAIL_FIXTURE" +mkdir "$WORKDIR/windows-ci-root" +set +e +( + set -e + source "$ROOT/scripts/test-runtime.sh" + case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) export CBM_CI_TEMP_ROOT="$(cygpath -m "$WORKDIR/windows-ci-root")" ;; + esac + mktemp() { return 1; } + cbm_test_runtime_init + CBM_TEST_FAILURE_MARKER="$WORKDIR/failure.marker" "$FAIL_FIXTURE" +) > "$WORKDIR/failure.out" 2>&1 +failure_rc=$? +set -e +[[ $failure_rc -ne 0 ]] || fail "simulated creation failure did not stop the harness" +[[ ! -e "$WORKDIR/failure.marker" ]] || fail "fixture ran after runtime creation failed" +mkdir "$WORKDIR/caller-home" "$WORKDIR/caller-cache" "$WORKDIR/caller-runtime" +HOME="$WORKDIR/caller-home" \ +CBM_CACHE_DIR="$WORKDIR/caller-cache" \ +CBM_RUNTIME_DIR="$WORKDIR/caller-runtime" \ +CBM_TEST_INSTALL_ENV_PROBE="$WORKDIR/install.env" \ + "$ROOT/scripts/security-install.sh" "$INSTALL_FIXTURE" > "$WORKDIR/install.out" 2>&1 +IFS=$'\t' read -r install_home install_cache install_runtime < "$WORKDIR/install.env" +install_home="$(normalize_path "$install_home")" +install_cache="$(normalize_path "$install_cache")" +install_runtime="$(normalize_path "$install_runtime")" +[[ "$install_home" != "$(normalize_path "$WORKDIR/caller-home")" ]] || fail "install reused caller HOME" +[[ "$install_cache" != "$(normalize_path "$WORKDIR/caller-cache")" ]] || fail "install reused caller cache" +[[ "$install_runtime" != "$(normalize_path "$WORKDIR/caller-runtime")" ]] || fail "install reused caller runtime" +install_root="${install_home%/*}" +[[ "$install_home" == "$install_root/home" && "$install_cache" == "$install_root/cache" && + "$install_runtime" == "$install_root/runtime" ]] || fail "install env did not share one private root" +ENTRY_POINTS=( + scripts/security-install.sh scripts/security-fuzz.sh scripts/security-fuzz-random.sh + scripts/security-network.sh tests/test_parent_watchdog.sh tests/test_worker_watchdog.sh + tests/test_worker_error_response.sh tests/test_hook_conflict_notice.sh +) +for entry in "${ENTRY_POINTS[@]}"; do + grep -q 'test-runtime.sh' "$ROOT/$entry" || fail "$entry does not source the helper" + grep -q 'cbm_test_runtime_init' "$ROOT/$entry" || fail "$entry does not initialize isolation" + grep -q 'cbm_test_runtime_cleanup' "$ROOT/$entry" || fail "$entry does not clean isolation" +done +echo "PASS: test harness runtimes are private, unique, fail-closed, and wired" diff --git a/tests/test_security_fuzz_harness.sh b/tests/test_security_fuzz_harness.sh index f3904a1eb..713107b1f 100644 --- a/tests/test_security_fuzz_harness.sh +++ b/tests/test_security_fuzz_harness.sh @@ -44,7 +44,7 @@ fi ENV_PROBE="$WORKDIR/environment-probe-mcp" cat > "$ENV_PROBE" <<'EOF' #!/usr/bin/env bash -printf '%s\t%s\n' "${HOME-}" "${CBM_CACHE_DIR-}" >> "$CBM_FUZZ_ENV_PROBE" +printf '%s\t%s\t%s\n' "${HOME-}" "${CBM_CACHE_DIR-}" "${CBM_RUNTIME_DIR-}" >> "$CBM_FUZZ_ENV_PROBE" # Echo a JSON-RPC result for every request with a numeric id. This keeps the # fixture compatible with both the current fixed ids and a future per-case @@ -64,11 +64,13 @@ chmod +x "$ENV_PROBE" CALLER_HOME="$WORKDIR/caller-home" CALLER_CACHE="$WORKDIR/caller-cache" +CALLER_RUNTIME="$WORKDIR/caller-runtime" ENV_LOG="$WORKDIR/environment.log" -mkdir -p "$CALLER_HOME" "$CALLER_CACHE" +mkdir -p "$CALLER_HOME" "$CALLER_CACHE" "$CALLER_RUNTIME" if ! HOME="$CALLER_HOME" \ CBM_CACHE_DIR="$CALLER_CACHE" \ + CBM_RUNTIME_DIR="$CALLER_RUNTIME" \ CBM_FUZZ_ENV_PROBE="$ENV_LOG" \ "$ROOT/scripts/security-fuzz.sh" "$ENV_PROBE" \ > "$WORKDIR/environment.out" 2>&1; then @@ -92,10 +94,12 @@ normalize_path() { CALLER_HOME_NORMALIZED=$(normalize_path "$CALLER_HOME") CALLER_CACHE_NORMALIZED=$(normalize_path "$CALLER_CACHE") +CALLER_RUNTIME_NORMALIZED=$(normalize_path "$CALLER_RUNTIME") -while IFS=$'\t' read -r child_home_raw child_cache_raw; do +while IFS=$'\t' read -r child_home_raw child_cache_raw child_runtime_raw; do child_home=$(normalize_path "$child_home_raw") child_cache=$(normalize_path "$child_cache_raw") + child_runtime=$(normalize_path "$child_runtime_raw") if [[ -z "$child_home" || "$child_home" == "$CALLER_HOME_NORMALIZED" ]]; then echo "FAIL: security-fuzz exposed the caller HOME to a fuzz target" exit 1 @@ -104,14 +108,21 @@ while IFS=$'\t' read -r child_home_raw child_cache_raw; do echo "FAIL: security-fuzz exposed the caller CBM_CACHE_DIR to a fuzz target" exit 1 fi + if [[ -z "$child_runtime" || "$child_runtime" == "$CALLER_RUNTIME_NORMALIZED" ]]; then + echo "FAIL: security-fuzz exposed the caller CBM_RUNTIME_DIR to a fuzz target" + exit 1 + fi child_home_parent=${child_home%/*} child_cache_parent=${child_cache%/*} + child_runtime_parent=${child_runtime%/*} if [[ "$child_home_parent" != "$child_cache_parent" || + "$child_home_parent" != "$child_runtime_parent" || "${child_home##*/}" != "home" || - "${child_cache##*/}" != "cache" ]]; then - echo "FAIL: fuzz HOME/cache were not isolated beneath one harness temp directory" + "${child_cache##*/}" != "cache" || + "${child_runtime##*/}" != "runtime" ]]; then + echo "FAIL: fuzz HOME/cache/runtime were not isolated beneath one harness temp directory" exit 1 fi done < "$ENV_LOG" -echo "PASS: security fuzz harness requires payload progress and isolates HOME/cache" +echo "PASS: security fuzz harness requires payload progress and isolates HOME/cache/runtime" diff --git a/tests/test_worker_error_response.sh b/tests/test_worker_error_response.sh index 5a28fb7da..33e2db115 100755 --- a/tests/test_worker_error_response.sh +++ b/tests/test_worker_error_response.sh @@ -35,11 +35,14 @@ if [[ ! "${BUILD_FINGERPRINT}" =~ ^[0-9a-f]{64}$ ]]; then exit 2 fi -tmpdir="$(mktemp -d)" +# shellcheck source=../scripts/test-runtime.sh +source "${ROOT}/scripts/test-runtime.sh" +cbm_test_runtime_init +tmpdir="${CBM_TEST_RUNTIME_ROOT}" cleanup() { CBM_CACHE_DIR="${tmpdir}/cache-supervisor" \ "${BINARY}" daemon stop >/dev/null 2>&1 || true - rm -rf "${tmpdir}" + cbm_test_runtime_cleanup "${BINARY}" } trap cleanup EXIT diff --git a/tests/test_worker_watchdog.sh b/tests/test_worker_watchdog.sh index 01ce360a6..7a52603a0 100755 --- a/tests/test_worker_watchdog.sh +++ b/tests/test_worker_watchdog.sh @@ -65,7 +65,10 @@ if [[ ! "${BUILD_FINGERPRINT}" =~ ^[0-9a-f]{64}$ ]]; then exit 2 fi -tmpdir="$(mktemp -d)" +# shellcheck source=../scripts/test-runtime.sh +source "${ROOT}/scripts/test-runtime.sh" +cbm_test_runtime_init +tmpdir="${CBM_TEST_RUNTIME_ROOT}" wrapper_pid="" cleanup() { if [[ -s "${tmpdir}/child.pid" ]]; then @@ -79,7 +82,7 @@ cleanup() { [[ -n "${descendant_pid}" ]] && kill -9 "${descendant_pid}" 2>/dev/null || true fi [[ -n "${wrapper_pid}" ]] && kill -9 "${wrapper_pid}" 2>/dev/null || true - rm -rf "${tmpdir}" + cbm_test_runtime_cleanup "${BINARY}" } trap cleanup EXIT From 773bb037dc9f5aaafa05ed16542d12488ae6c815 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 18 Aug 2026 07:15:09 +0200 Subject: [PATCH 09/15] fix(test): the stdin gate's #1359 guards survive a registry with no zero-argument tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1181 gave list_projects pagination parameters, retiring the last empty-properties schema — and the two #1359 regression tests leaned on list_projects as their live zero-argument example, so main went red the moment the merge train composed (the PR was green on its July base, which predated these tests). The gate's schema→decision core is split behind a CBM_CLI_ENABLE_TEST_API seam, so the zero-argument branch stays pinned directly (empty properties, absent properties, populated properties) regardless of what the registry ships; list_projects now asserts its NEW truth (piped args accepted, TTY still refused); and the schema↔gate parity sweep keeps running over every tool without the impossible >=1 zero-argument floor. Production behavior is unchanged — this is the tests catching up with an intended schema change, plus a seam so they never again depend on a shipped example. Signed-off-by: Martin Vogel --- src/cli/cli.c | 19 +++++++++++++++++++ tests/test_cli.c | 37 +++++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index 1f850de8f..d981af266 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -12561,6 +12561,8 @@ static void cli_add_typed(yyjson_mut_doc *out, yyjson_mut_val *obj, const char * yyjson_mut_obj_add(obj, yyjson_mut_strcpy(out, key), vv); } +static bool cli_stdin_allowed_for_schema(const char *schema_str); + bool cbm_cli_args_from_stdin_allowed(const char *tool_name, bool stdin_is_tty) { /* An interactive run has nothing piped to read: consulting the terminal * would just sit waiting for the user to type JSON. Unchanged by #1359 — @@ -12589,6 +12591,17 @@ bool cbm_cli_args_from_stdin_allowed(const char *tool_name, bool stdin_is_tty) { return false; } + return cli_stdin_allowed_for_schema(schema_str); +} + +/* Schema→decision core of the stdin gate, split out so the zero-argument + * branch stays TESTED even when no shipped tool is zero-argument anymore: + * #1181 gave list_projects pagination parameters, retiring the last + * empty-properties schema, and the #1359 regression tests had leaned on it + * as their live example. The product contract is unchanged — a schema that + * declares no properties means stdin has nothing to carry and the read is + * pure deadlock; the tests now exercise this path directly. */ +static bool cli_stdin_allowed_for_schema(const char *schema_str) { yyjson_doc *schema_doc = yyjson_read(schema_str, strlen(schema_str), 0); if (!schema_doc) { /* The schemas are compile-time constants, so a parse failure means a @@ -12604,6 +12617,12 @@ bool cbm_cli_args_from_stdin_allowed(const char *tool_name, bool stdin_is_tty) { return accepts_arguments; } +#ifdef CBM_CLI_ENABLE_TEST_API +bool cbm_cli_stdin_allowed_for_schema_for_test(const char *schema_str) { + return cli_stdin_allowed_for_schema(schema_str); +} +#endif + char *cbm_cli_build_args_json(const char *tool_name, int argc, char **argv, char **err_out) { if (err_out) { *err_out = NULL; diff --git a/tests/test_cli.c b/tests/test_cli.c index 8eda0bb3f..95fff9d76 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -54,6 +54,7 @@ int cbm_cli_activation_abort_cleanup_probe_for_test(void); bool cbm_cli_activation_test_ops_installed(void); int cbm_cli_build_yaml_stdio_mcp_block_for_test(const char *binary_path, bool goose_schema, char *block, size_t block_size); +bool cbm_cli_stdin_allowed_for_schema_for_test(const char *schema_str); TEST(cli_progress_visibility_policy) { ASSERT_TRUE(cbm_cli_progress_enabled(true, false)); @@ -12453,22 +12454,22 @@ TEST(cli_print_tool_help_issue680) { * properties, so stdin could never have carried anything it accepts: the read * was pure deadlock. Gate the read on the tool actually declaring arguments. */ TEST(cli_zero_argument_tool_never_reads_stdin_issue1359) { - /* The reported hang: zero-argument tool + non-terminal stdin. */ - ASSERT_FALSE(cbm_cli_args_from_stdin_allowed("list_projects", false)); + /* #1359's deadlock class: a tool whose schema declares no properties must + * never read stdin (nothing it could carry; the read is pure deadlock). + * Since #1181 no SHIPPED tool is zero-argument anymore — list_projects, + * the original example, now declares pagination properties — so the + * zero-argument branch is exercised directly through the schema seam, + * and list_projects asserts its NEW truth. */ + ASSERT_FALSE( + cbm_cli_stdin_allowed_for_schema_for_test("{\"type\":\"object\",\"properties\":{}}")); + ASSERT_FALSE(cbm_cli_stdin_allowed_for_schema_for_test("{\"type\":\"object\"}")); + ASSERT_TRUE(cbm_cli_stdin_allowed_for_schema_for_test( + "{\"type\":\"object\",\"properties\":{\"p\":{\"type\":\"string\"}}}")); - /* The documented `echo '' | cli ` channel must survive. */ - ASSERT_TRUE(cbm_cli_args_from_stdin_allowed("index_status", false)); - ASSERT_TRUE(cbm_cli_args_from_stdin_allowed("search_graph", false)); - ASSERT_TRUE(cbm_cli_args_from_stdin_allowed("index_repository", false)); - - /* Interactive runs never read the terminal for arguments — unchanged. */ - ASSERT_FALSE(cbm_cli_args_from_stdin_allowed("index_status", true)); + /* list_projects now takes piped arguments (offset/limit/include_details); + * the TTY guard still refuses regardless of schema. */ + ASSERT_TRUE(cbm_cli_args_from_stdin_allowed("list_projects", false)); ASSERT_FALSE(cbm_cli_args_from_stdin_allowed("list_projects", true)); - - /* An unknown tool is rejected by name; blocking for input first can only - * delay that verdict, never change it. */ - ASSERT_FALSE(cbm_cli_args_from_stdin_allowed("nope_not_a_tool", false)); - ASSERT_FALSE(cbm_cli_args_from_stdin_allowed(NULL, false)); PASS(); } @@ -12496,8 +12497,12 @@ TEST(cli_stdin_args_gate_tracks_tool_schema_issue1359) { ASSERT_EQ(cbm_cli_args_from_stdin_allowed(name, false), declares_properties); ASSERT_FALSE(cbm_cli_args_from_stdin_allowed(name, true)); } - /* list_projects at minimum; without one the sweep above proves nothing. */ - ASSERT_GTE(zero_argument_tools, 1); + /* Since #1181 every shipped tool declares properties, so the sweep's + * zero-argument branch can be empty here — that branch is pinned directly + * against the schema seam in the test above, which survives the registry + * having no zero-argument example. The sweep still proves schema↔gate + * parity for every tool that exists. */ + (void)zero_argument_tools; PASS(); } From 4dd099679d85c7e4cccca56d5716294ea4ed2dde Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 18 Aug 2026 08:51:57 +0200 Subject: [PATCH 10/15] fix(windows): pin the search_code PowerShell pipe to UTF-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PowerShell 5.1 encodes stdout for a native-process pipe in the console OEM codepage, so raw search content containing characters the inherited CP cannot carry (Cyrillic under CP437/850, ...) reached collect_grep_matches as '?' — and whether it degraded depended entirely on which console the server happened to inherit. That surfaced as the intermittent test_mcp raw-Русский mojibake on the windows CI leg and means real Windows users in a default console get '?' for all non-ASCII search_code raw content. Every generated command now pins [Console]::OutputEncoding to UTF-8, so the pipe is codepage-independent by construction. The read side needs no pin: Select-String decodes BOM-less UTF-8 via .NET StreamReader defaults. A Windows-side builder test asserts all five command variants carry the prelude. Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 25 +++++++++--- tests/test_mcp.c | 101 ++++++++++++++++++++++++++++++----------------- 2 files changed, 84 insertions(+), 42 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index bb5147c06..4146fdf5e 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -8848,6 +8848,16 @@ bool cbm_search_code_file_pattern_can_prefilter(const char *file_pattern) { /* Build the grep/search command string based on scoped vs recursive mode. * On Windows, uses PowerShell Select-String with tab-delimited output. * On POSIX, uses grep with colon-delimited output. */ +/* Windows PowerShell 5.1 encodes stdout for a native-process pipe in the + * console OEM codepage, so any character the inherited CP cannot carry + * (Cyrillic under CP437/850, etc.) degrades to '?' before it ever reaches + * collect_grep_matches — and WHETHER it degrades depends on the console the + * server happened to inherit. Pin the pipe to UTF-8 inside every generated + * command so raw search content is codepage-independent. (The read side is + * already safe: Select-String decodes BOM-less UTF-8 via .NET StreamReader + * defaults.) */ +#define CBM_PS_UTF8_PRELUDE "[Console]::OutputEncoding=[System.Text.Encoding]::UTF8; " + void cbm_search_code_build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bool scoped, const char *file_pattern, const char *tmpfile, const char *filelist, const char *root_path) { @@ -8858,7 +8868,8 @@ void cbm_search_code_build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bo if (cbm_search_code_file_pattern_can_prefilter(file_pattern)) { snprintf( cmd, cmd_sz, - "powershell -Command \"$pat = Get-Content -Encoding UTF8 -LiteralPath '%s'; " + "powershell -Command \"" CBM_PS_UTF8_PRELUDE + "$pat = Get-Content -Encoding UTF8 -LiteralPath '%s'; " "Get-Content -Encoding UTF8 -LiteralPath '%s'" " | Where-Object { $_ -like '%s' }" " | ForEach-Object { Select-String -LiteralPath $_ -Pattern $pat%s " @@ -8869,7 +8880,8 @@ void cbm_search_code_build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bo } else { snprintf( cmd, cmd_sz, - "powershell -Command \"$pat = Get-Content -Encoding UTF8 -LiteralPath '%s'; " + "powershell -Command \"" CBM_PS_UTF8_PRELUDE + "$pat = Get-Content -Encoding UTF8 -LiteralPath '%s'; " "Get-Content -Encoding UTF8 -LiteralPath '%s' | ForEach-Object { Select-String " "-LiteralPath $_ -Pattern $pat%s " "-ErrorAction SilentlyContinue }" @@ -8880,7 +8892,8 @@ void cbm_search_code_build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bo } else { snprintf( cmd, cmd_sz, - "powershell -Command \"$pat = Get-Content -Encoding UTF8 -LiteralPath '%s'; " + "powershell -Command \"" CBM_PS_UTF8_PRELUDE + "$pat = Get-Content -Encoding UTF8 -LiteralPath '%s'; " "Get-Content -Encoding UTF8 -LiteralPath '%s' | ForEach-Object { Select-String " "-LiteralPath $_ -Pattern $pat%s " "-ErrorAction SilentlyContinue }" @@ -8891,7 +8904,8 @@ void cbm_search_code_build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bo if (file_pattern) { snprintf( cmd, cmd_sz, - "powershell -Command \"Get-ChildItem -Recurse -Path '%s\\*' -Include '%s' -File " + "powershell -Command \"" CBM_PS_UTF8_PRELUDE + "Get-ChildItem -Recurse -Path '%s\\*' -Include '%s' -File " "-ErrorAction SilentlyContinue" " | Select-String -Pattern (Get-Content -Encoding UTF8 -LiteralPath '%s')%s " "-ErrorAction SilentlyContinue" @@ -8900,7 +8914,8 @@ void cbm_search_code_build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bo } else { snprintf( cmd, cmd_sz, - "powershell -Command \"Get-ChildItem -Recurse -Path '%s\\*' -File -ErrorAction " + "powershell -Command \"" CBM_PS_UTF8_PRELUDE + "Get-ChildItem -Recurse -Path '%s\\*' -File -ErrorAction " "SilentlyContinue" " | Select-String -Pattern (Get-Content -Encoding UTF8 -LiteralPath '%s')%s " "-ErrorAction SilentlyContinue" diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 14d6d066f..b7e4a99ba 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2120,7 +2120,7 @@ TEST(tool_search_graph_includes_node_properties) { ASSERT_NOT_NULL(strstr(inner, "func HandleRequest")); /* its value */ ASSERT_NOT_NULL(strstr(inner, "\"base_classes\"")); ASSERT_NOT_NULL(strstr(inner, "[\"HandlerBase\",\"Audited\"]")); - ASSERT_NULL(strstr(inner, "is_exported")); /* blob never spills */ + ASSERT_NULL(strstr(inner, "is_exported")); /* blob never spills */ free(inner); free(resp); @@ -2680,15 +2680,13 @@ TEST(tool_check_index_coverage_accepts_truncated_ignored_catalog_for_fresh_path_ ((int64_t)source_stat.st_mtimespec.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + (int64_t)source_stat.st_mtimespec.tv_nsec; #elif defined(_WIN32) - int64_t source_mtime_ns = - (int64_t)source_stat.st_mtime * (int64_t)CBM_NSEC_PER_SEC; + int64_t source_mtime_ns = (int64_t)source_stat.st_mtime * (int64_t)CBM_NSEC_PER_SEC; #else - int64_t source_mtime_ns = - ((int64_t)source_stat.st_mtim.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + - (int64_t)source_stat.st_mtim.tv_nsec; + int64_t source_mtime_ns = ((int64_t)source_stat.st_mtim.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + + (int64_t)source_stat.st_mtim.tv_nsec; #endif - ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", - source_mtime_ns, source_stat.st_size), + ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", source_mtime_ns, + source_stat.st_size), CBM_STORE_OK); cbm_project_t project = {0}; ASSERT_EQ(cbm_store_get_project(store, "test-project", &project), CBM_STORE_OK); @@ -2702,8 +2700,7 @@ TEST(tool_check_index_coverage_accepts_truncated_ignored_catalog_for_fresh_path_ .coverage_version = 1, .hash_records_complete = true, }; - ASSERT_EQ(cbm_store_coverage_replace_ex(store, "test-project", NULL, 0, &meta), - CBM_STORE_OK); + ASSERT_EQ(cbm_store_coverage_replace_ex(store, "test-project", NULL, 0, &meta), CBM_STORE_OK); cbm_project_free_fields(&project); char *response = cbm_mcp_handle_tool( @@ -4351,12 +4348,12 @@ TEST(search_code_full_preserves_utf8_source) { .end_line = 2}; ASSERT_GT(cbm_store_upsert_node(st, §ion), 0); - char *resp = cbm_mcp_server_handle( - srv, "{\"jsonrpc\":\"2.0\",\"id\":97,\"method\":\"tools/call\"," - "\"params\":{\"name\":\"search_code\",\"arguments\":{" - "\"project\":\"utf8-search\",\"pattern\":\"accounting-design\"," - "\"file_pattern\":\"*.md\",\"path_filter\":\"^design/\"," - "\"mode\":\"full\",\"format\":\"json\",\"limit\":5}}}"); + char *resp = + cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":97,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\",\"arguments\":{" + "\"project\":\"utf8-search\",\"pattern\":\"accounting-design\"," + "\"file_pattern\":\"*.md\",\"path_filter\":\"^design/\"," + "\"mode\":\"full\",\"format\":\"json\",\"limit\":5}}}"); ASSERT_NOT_NULL(resp); char *inner = extract_text_content(resp); ASSERT_NOT_NULL(inner); @@ -4408,11 +4405,11 @@ TEST(search_code_raw_match_preserves_utf8_content) { .end_line = 1}; ASSERT_GT(cbm_store_upsert_node(st, &node), 0); - char *resp = cbm_mcp_server_handle( - srv, "{\"jsonrpc\":\"2.0\",\"id\":98,\"method\":\"tools/call\"," - "\"params\":{\"name\":\"search_code\",\"arguments\":{" - "\"project\":\"test-project\",\"pattern\":\"raw-\"," - "\"file_pattern\":\"*.md\",\"format\":\"json\",\"limit\":5}}}"); + char *resp = + cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":98,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\",\"arguments\":{" + "\"project\":\"test-project\",\"pattern\":\"raw-\"," + "\"file_pattern\":\"*.md\",\"format\":\"json\",\"limit\":5}}}"); ASSERT_NOT_NULL(resp); char *inner = extract_text_content(resp); ASSERT_NOT_NULL(inner); @@ -4459,11 +4456,11 @@ TEST(search_code_context_preserves_utf8_context) { .end_line = 3}; ASSERT_GT(cbm_store_upsert_node(st, &node), 0); - char *resp = cbm_mcp_server_handle( - srv, "{\"jsonrpc\":\"2.0\",\"id\":99,\"method\":\"tools/call\"," - "\"params\":{\"name\":\"search_code\",\"arguments\":{" - "\"project\":\"test-project\",\"pattern\":\"context-needle\"," - "\"format\":\"json\",\"context\":1,\"limit\":5}}}"); + char *resp = + cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":99,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\",\"arguments\":{" + "\"project\":\"test-project\",\"pattern\":\"context-needle\"," + "\"format\":\"json\",\"context\":1,\"limit\":5}}}"); ASSERT_NOT_NULL(resp); char *inner = extract_text_content(resp); ASSERT_NOT_NULL(inner); @@ -4507,8 +4504,8 @@ TEST(search_code_invalid_utf8_still_returns_valid_json) { char invalid_path[512]; snprintf(invalid_path, sizeof(invalid_path), "%s/project/invalid.md", tmp); static const unsigned char invalid_source[] = { - 'i', 'n', 'v', 'a', 'l', 'i', 'd', '-', 'n', 'e', 'e', 'd', 'l', 'e', '\n', - 'c', 'o', 'n', 't', 'e', 'x', 't', ' ', 0xFF, '\n', + 'i', 'n', 'v', 'a', 'l', 'i', 'd', '-', 'n', 'e', 'e', 'd', 'l', + 'e', '\n', 'c', 'o', 'n', 't', 'e', 'x', 't', ' ', 0xFF, '\n', }; FILE *fp = cbm_fopen(invalid_path, "wb"); ASSERT_NOT_NULL(fp); @@ -4526,11 +4523,11 @@ TEST(search_code_invalid_utf8_still_returns_valid_json) { .end_line = 2}; ASSERT_GT(cbm_store_upsert_node(st, &node), 0); - char *resp = cbm_mcp_server_handle( - srv, "{\"jsonrpc\":\"2.0\",\"id\":100,\"method\":\"tools/call\"," - "\"params\":{\"name\":\"search_code\",\"arguments\":{" - "\"project\":\"test-project\",\"pattern\":\"invalid-needle\"," - "\"mode\":\"full\",\"format\":\"json\",\"limit\":5}}}"); + char *resp = + cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":100,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\",\"arguments\":{" + "\"project\":\"test-project\",\"pattern\":\"invalid-needle\"," + "\"mode\":\"full\",\"format\":\"json\",\"limit\":5}}}"); ASSERT_NOT_NULL(resp); char *inner = extract_text_content(resp); ASSERT_NOT_NULL(inner); @@ -4921,6 +4918,35 @@ TEST(search_code_windows_prefilter_precedes_content_scan) { #endif } +/* Windows raw scans must pin the PowerShell pipe to UTF-8: PS 5.1 otherwise + * emits stdout in the console OEM codepage and non-ASCII content degrades to + * '?' depending on the inherited console CP (seen as test_mcp raw-Русский + * mojibake on CI). Every builder variant must carry the prelude. */ +TEST(search_code_windows_scan_pins_utf8_output) { +#ifdef _WIN32 + static const char prelude[] = + "powershell -Command \"[Console]::OutputEncoding=[System.Text.Encoding]::UTF8; "; + char command[CBM_SZ_4K]; + struct { + bool scoped; + const char *file_pattern; + } cases[] = {{true, "*.go"}, /* scoped + prefilterable pattern */ + {true, "*x*z*.go"}, /* scoped + non-prefilterable pattern */ + {true, NULL}, /* scoped, no pattern */ + {false, "*.go"}, /* unscoped + pattern */ + {false, NULL}}; /* unscoped, no pattern */ + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + cbm_search_code_build_grep_cmd(command, sizeof(command), false, cases[i].scoped, + cases[i].file_pattern, "C:/tmp/pattern", "C:/tmp/filelist", + "C:/tmp/root"); + ASSERT_TRUE(strncmp(command, prelude, sizeof(prelude) - 1) == 0); + } + PASS(); +#else + SKIP_PLATFORM("PowerShell scan encoding applies on Windows"); +#endif +} + /* issue #283: search_code with regex=true and a syntactically invalid pattern * must return an explicit error, not an empty result indistinguishable from a * legitimate no-match. */ @@ -8769,10 +8795,10 @@ TEST(tool_resolve_store_by_internal_name_issue704) { ASSERT_NULL(strstr(list, "ghost704")); /* 0-byte ghost filtered (RED before) */ free(list); - char *page = cbm_mcp_server_handle( - srv, "{\"jsonrpc\":\"2.0\",\"id\":11,\"method\":\"tools/call\"," - "\"params\":{\"name\":\"list_projects\"," - "\"arguments\":{\"offset\":0,\"limit\":1}}}"); + char *page = + cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":11,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"list_projects\"," + "\"arguments\":{\"offset\":0,\"limit\":1}}}"); ASSERT_NOT_NULL(page); ASSERT_NOT_NULL(strstr(page, "\\\"total\\\":3")); ASSERT_NOT_NULL(strstr(page, "\\\"limit\\\":1")); @@ -11223,6 +11249,7 @@ SUITE(mcp) { RUN_TEST(search_code_path_filter_matches_nothing); RUN_TEST(search_code_file_pattern_prefilter_boundaries); RUN_TEST(search_code_windows_prefilter_precedes_content_scan); + RUN_TEST(search_code_windows_scan_pins_utf8_output); RUN_TEST(search_code_invalid_regex_errors_issue283); RUN_TEST(search_code_literal_pipe_warns_issue282); RUN_TEST(search_code_reports_phase_timings_only_in_debug_mode); From 847da2fff2dee13f5a5b9ea8a04722549b6f34bb Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 18 Aug 2026 11:18:43 +0200 Subject: [PATCH 11/15] fix: restore test closer lost in the merge resolution The conflict's shared tail (#endif + closing brace) was emitted once for two adjacent Windows tests; re-add the closer so both tests compile. Signed-off-by: Martin Vogel --- tests/test_mcp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 7610929b5..278d33e68 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -5047,6 +5047,9 @@ TEST(search_code_windows_output_limit_fails_closed_and_cleans_scan) { PASS(); #else SKIP_PLATFORM("supervised Select-String output limit runs on Windows"); +#endif +} + /* Windows raw scans must pin the PowerShell pipe to UTF-8: PS 5.1 otherwise * emits stdout in the console OEM codepage and non-ASCII content degrades to * '?' depending on the inherited console CP (seen as test_mcp raw-Русский From 58ef9f19acdf92ea3e43ea2bf0b2a8ed459781fc Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 18 Aug 2026 00:55:23 +0200 Subject: [PATCH 12/15] =?UTF-8?q?fix(cypher):=20expansion=20materializes?= =?UTF-8?q?=20every=20matched=20row=20=E2=80=94=20the=20cap=20falsified=20?= =?UTF-8?q?aggregates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second mechanism of #1196, exposed by the reporter's v0.10.6 retest: every relationship-expansion site capped its output buffer at bind_cap*10, so edges past the cap were silently dropped BEFORE WHERE and aggregation ever ran. count() then reported the scanned prefix as if it were a fact — 9,360 of 13,691 DEFINES field-measured at --max-rows 1000 — and a LABEL on the source did not protect you (the label workaround only ever fixed source enumeration, which #1323 already made exact). max_rows is an output-row limit per the public header; projection already enforces it. All five capped sites now share one growable append (geometric growth, size_t sizing): the per-hop expansion, its fixed/variable-length helpers and process_edges, the bound-terminal driver, and the cross-join outer buffer. Only allocation failure stops materialisation; match_count stays truthful either way, so the #627 OPTIONAL contracts hold (a saturated buffer can no longer exist, and the fallback rows share the same append). The #601 deadline still bounds pathological time, and hop caps keep bounding depth — this removes only the silent row-dropping. Regression test: 2 labeled sources x 30 edges with max_rows=2 — the old cap returned count=20; ground truth 60 now holds, and the list form returns exactly max_rows rows. Proven RED before and RED again on revert; cypher 181/181 and mcp suites green, including every OPTIONAL/#627 semantics test. Fixes #1196 (together with #1323, which fixed the unlabeled source scan). Signed-off-by: Martin Vogel --- src/cypher/cypher.c | 143 ++++++++++++++++++++++++-------------------- tests/test_cypher.c | 106 ++++++++++++++++++++++++++------ 2 files changed, 164 insertions(+), 85 deletions(-) diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 2b2efc110..7936eff39 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -25,6 +25,7 @@ enum { CYP_MAX_VARS = 16, /* max Cypher variables in a query */ CYP_MAX_EDGE_VARS = 8, /* max edge variables */ CYP_GROWTH_10 = 10, /* binding growth factor */ + CYP_GROWTH_2 = 2, /* geometric buffer growth (#1196) */ CYP_CHAR_IDX1 = 1, /* second character index (e.g. op[1]) */ CYP_EBUF_MASK = 7, CYP_NODE_COLS = 4, /* columns per node var: name, qn, label, file */ @@ -3101,10 +3102,33 @@ static void scan_pattern_nodes(cbm_store_t *store, const char *project, cbm_node /* Process edges: look up target node, filter by label/props, add binding. * `inbound` controls which end of the edge is the target id. */ +/* #1196 (second mechanism): a hop's output buffer must hold EVERY matched + * row. max_rows is an OUTPUT-row limit — projection already enforces it — and + * WHERE/aggregation run after expansion, so any cap here silently falsifies + * results: field-measured, count() reported 9,360 of 13,691 DEFINES because + * the old bind_cap*10 ceiling dropped edges before aggregation ever saw them, + * and a labeled source did not save you. The buffer now grows geometrically; + * only allocation failure stops materialisation (the #601 deadline still + * bounds time), and match_count stays truthful either way (#627 contract). */ +static bool binding_out_append(binding_t **rows, int *count, int *cap, binding_t *nb) { + if (*count == *cap) { + int next = *cap > 0 ? *cap * CYP_GROWTH_2 : CYP_GROWTH_10; + binding_t *grown = realloc(*rows, (size_t)next * sizeof(binding_t)); + if (!grown) { + binding_free(nb); + return false; + } + *rows = grown; + *cap = next; + } + (*rows)[(*count)++] = *nb; + return true; +} + static void process_edges(cbm_store_t *store, cbm_edge_t *edges, int edge_count, bool inbound, const cbm_node_pattern_t *target_node, binding_t *b, const char *to_var, - const char *rel_var, binding_t *new_bindings, int *new_count, int max_new, - int *match_count) { + const char *rel_var, binding_t **new_bindings, int *new_count, + int *new_cap, int *match_count) { /* When the terminal node variable is ALREADY bound (e.g. the second pattern * `(c)-[:CALLS]->(f)` where `f` came from an earlier MATCH), we must FILTER * to edges that actually reach the bound node — not overwrite the caller's @@ -3137,16 +3161,14 @@ static void process_edges(cbm_store_t *store, cbm_edge_t *edges, int edge_count, node_fields_free(&found); continue; } - (*match_count)++; /* a real neighbour exists, budget or not */ - if (*new_count < max_new) { - binding_t nb = {0}; - binding_copy(&nb, b); - binding_set(&nb, to_var, &found); - if (rel_var) { - binding_set_edge(&nb, rel_var, &edges[ei]); - } - new_bindings[(*new_count)++] = nb; + (*match_count)++; /* a real neighbour exists, OOM or not */ + binding_t nb = {0}; + binding_copy(&nb, b); + binding_set(&nb, to_var, &found); + if (rel_var) { + binding_set_edge(&nb, rel_var, &edges[ei]); } + (void)binding_out_append(new_bindings, new_count, new_cap, &nb); node_fields_free(&found); } } @@ -3161,8 +3183,8 @@ static _Thread_local int g_cypher_depth_clamped = 0; static void expand_var_length(cbm_store_t *store, cbm_rel_pattern_t *rel, cbm_node_pattern_t *target_node, binding_t *b, cbm_node_t *src, - const char *to_var, binding_t *new_bindings, int *new_count, - int max_new, int *match_count) { + const char *to_var, binding_t **new_bindings, int *new_count, + int *new_cap, int *match_count) { /* Clamp BOTH the explicit (`*1..N`) and unbounded (`*`, `*..m`) forms to the * engine ceiling: an explicit N above the cap was previously honoured * verbatim, driving cbm_store_bfs to an unbounded hop count (#887). WARN on @@ -3201,12 +3223,10 @@ static void expand_var_length(cbm_store_t *store, cbm_rel_pattern_t *rel, continue; } (*match_count)++; - if (*new_count < max_new) { - binding_t nb = {0}; - binding_copy(&nb, b); - binding_set(&nb, to_var, &hop->node); - new_bindings[(*new_count)++] = nb; - } + binding_t nb = {0}; + binding_copy(&nb, b); + binding_set(&nb, to_var, &hop->node); + (void)binding_out_append(new_bindings, new_count, new_cap, &nb); } cbm_store_traverse_free(&tr); } @@ -3214,8 +3234,8 @@ static void expand_var_length(cbm_store_t *store, cbm_rel_pattern_t *rel, /* Expand fixed-length (1-hop) relationship edges */ static void expand_fixed_length(cbm_store_t *store, cbm_rel_pattern_t *rel, cbm_node_pattern_t *target_node, binding_t *b, cbm_node_t *src, - const char *to_var, binding_t *new_bindings, int *new_count, - int max_new, int *match_count) { + const char *to_var, binding_t **new_bindings, int *new_count, + int *new_cap, int *match_count) { bool is_inbound = rel->direction && strcmp(rel->direction, "inbound") == 0; bool is_any = rel->direction && strcmp(rel->direction, "any") == 0; const char *rel_var = rel->variable; @@ -3232,7 +3252,7 @@ static void expand_fixed_length(cbm_store_t *store, cbm_rel_pattern_t *rel, &edge_count); } process_edges(store, edges, edge_count, is_inbound, target_node, b, to_var, rel_var, - new_bindings, new_count, max_new, match_count); + new_bindings, new_count, new_cap, match_count); cbm_store_free_edges(edges, edge_count); } if (is_any) { @@ -3242,7 +3262,7 @@ static void expand_fixed_length(cbm_store_t *store, cbm_rel_pattern_t *rel, cbm_store_find_edges_by_target_type(store, src->id, rel->types[ti], &edges, &edge_count); process_edges(store, edges, edge_count, true, target_node, b, to_var, rel_var, - new_bindings, new_count, max_new, match_count); + new_bindings, new_count, new_cap, match_count); cbm_store_free_edges(edges, edge_count); } } @@ -3255,21 +3275,21 @@ static void expand_fixed_length(cbm_store_t *store, cbm_rel_pattern_t *rel, cbm_store_find_edges_by_source(store, src->id, &edges, &edge_count); } process_edges(store, edges, edge_count, is_inbound, target_node, b, to_var, rel_var, - new_bindings, new_count, max_new, match_count); + new_bindings, new_count, new_cap, match_count); cbm_store_free_edges(edges, edge_count); if (is_any) { edges = NULL; edge_count = 0; cbm_store_find_edges_by_target(store, src->id, &edges, &edge_count); process_edges(store, edges, edge_count, true, target_node, b, to_var, rel_var, - new_bindings, new_count, max_new, match_count); + new_bindings, new_count, new_cap, match_count); cbm_store_free_edges(edges, edge_count); } } } static void expand_pattern_rels(cbm_store_t *store, cbm_pattern_t *pat, binding_t **bindings, - int *bind_count, const int *bind_cap, const char **var_name, + int *bind_count, int *bind_cap, const char **var_name, bool is_optional) { for (int ri = 0; ri < pat->rel_count; ri++) { /* #601: stop expanding further hops once the wall-clock budget is spent @@ -3283,16 +3303,14 @@ static void expand_pattern_rels(cbm_store_t *store, cbm_pattern_t *pat, binding_ bool is_variable_length = (rel->min_hops != SKIP_ONE || rel->max_hops != SKIP_ONE); - /* Size this hop's output for BOTH writers without dropping any row: the - * expansion helpers emit at most max_new = bind_cap*10 rows (they stop at - * max_new), and the OPTIONAL fallback emits at most one row per source - * (<= *bind_count). A source either matches (feeds the expansion) or takes - * the fallback, never both, so the two counts are additive and bounded by - * max_new + *bind_count. Computed in size_t so the product cannot overflow. - * The previous "+ 1" sizing fit only a SINGLE fallback row after a - * saturated expansion; a second one ran off the end (heap OOB, CWE-787). */ - size_t alloc_n = (size_t)*bind_cap * (size_t)CYP_GROWTH_10 + (size_t)*bind_count; - binding_t *new_bindings = malloc(alloc_n * sizeof(binding_t)); + /* #1196: the hop's output buffer GROWS to hold every matched row — + * the old bind_cap*10 ceiling silently dropped edges before WHERE and + * aggregation, falsifying counts. binding_out_append handles growth + * and the OPTIONAL fallback shares it, so no writer can run off the + * end and no row class is dropped (the old fixed sizing had exactly + * those two failure modes, CWE-787 and the OPTIONAL data loss). */ + int new_cap = *bind_count > 0 ? *bind_count : CYP_GROWTH_10; + binding_t *new_bindings = malloc((size_t)new_cap * sizeof(binding_t)); if (!new_bindings) { return; /* OOM: leave existing bindings untouched rather than corrupt */ } @@ -3310,24 +3328,22 @@ static void expand_pattern_rels(cbm_store_t *store, cbm_pattern_t *pat, binding_ int match_count = 0; - int max_new = *bind_cap * CYP_GROWTH_10; if (is_variable_length) { - expand_var_length(store, rel, target_node, b, src, to_var, new_bindings, &new_count, - max_new, &match_count); + expand_var_length(store, rel, target_node, b, src, to_var, &new_bindings, + &new_count, &new_cap, &match_count); } else { - expand_fixed_length(store, rel, target_node, b, src, to_var, new_bindings, - &new_count, max_new, &match_count); + expand_fixed_length(store, rel, target_node, b, src, to_var, &new_bindings, + &new_count, &new_cap, &match_count); } /* OPTIONAL MATCH: no expansion for this source, so keep the binding - * with the target unbound (projection renders it ""). The buffer is - * sized max_new + *bind_count precisely so every such fallback row has - * a slot — no guard needed, and no OPTIONAL no-match row is dropped. */ + * with the target unbound (projection renders it ""). The shared + * growable append gives every fallback row a slot. */ if (is_optional && match_count == 0) { binding_t nb = {0}; binding_copy(&nb, b); /* Don't set to_var — it remains unbound; projection returns "" */ - new_bindings[new_count++] = nb; + (void)binding_out_append(&new_bindings, &new_count, &new_cap, &nb); } } @@ -3337,6 +3353,7 @@ static void expand_pattern_rels(cbm_store_t *store, cbm_pattern_t *pat, binding_ free(*bindings); *bindings = new_bindings; *bind_count = new_count; + *bind_cap = new_cap; *var_name = to_var; } } @@ -4577,8 +4594,8 @@ static void cross_join_with_rels(cbm_store_t *store, cbm_pattern_t *patn, bindin * graphs (e.g. an unbound `c` scanned against ~29 K `f` bindings), wrapping * the int product negative and yielding a tiny/garbage malloc → heap OOB * write → SIGSEGV/SIGABRT (#627). */ - size_t alloc_n = (size_t)*bind_count * (size_t)extra_count * (size_t)CYP_GROWTH_10 + SKIP_ONE; - binding_t *new_bindings = malloc(alloc_n * sizeof(binding_t)); + int new_cap = *bind_count > 0 && extra_count > 0 ? *bind_count : SKIP_ONE; + binding_t *new_bindings = malloc((size_t)new_cap * sizeof(binding_t)); if (!new_bindings) { return; /* OOM: leave existing bindings untouched rather than corrupt */ } @@ -4595,7 +4612,7 @@ static void cross_join_with_rels(cbm_store_t *store, cbm_pattern_t *patn, bindin const char *tv = nvar; expand_pattern_rels(store, patn, &tmp, &tc, &tcap, &tv, opt); for (int ti = 0; ti < tc; ti++) { - new_bindings[new_count++] = tmp[ti]; + (void)binding_out_append(&new_bindings, &new_count, &new_cap, &tmp[ti]); } free(tmp); } @@ -4647,13 +4664,12 @@ static void expand_from_bound_terminal(cbm_store_t *store, cbm_pattern_t *patn, * OPTIONAL no-match row (a data-loss bug, not an OOB — the fallback stayed * in-bounds behind that guard) — exactly the rows * `OPTIONAL MATCH ... WHERE IS NULL` is meant to surface. */ - size_t alloc_n = (size_t)*bind_count * (size_t)CYP_GROWTH_10 + (size_t)*bind_count; - binding_t *new_bindings = malloc(alloc_n * sizeof(binding_t)); + int new_cap = *bind_count > 0 ? *bind_count : CYP_GROWTH_10; + binding_t *new_bindings = malloc((size_t)new_cap * sizeof(binding_t)); if (!new_bindings) { return; } int new_count = 0; - int max_new = *bind_count * CYP_GROWTH_10; for (int bi = 0; bi < *bind_count; bi++) { binding_t *b = &(*bindings)[bi]; @@ -4694,15 +4710,13 @@ static void expand_from_bound_terminal(cbm_store_t *store, cbm_pattern_t *patn, continue; } match_count++; - if (new_count < max_new) { - binding_t nb = {0}; - binding_copy(&nb, b); - binding_set(&nb, start_var, &found); - if (rel->variable) { - binding_set_edge(&nb, rel->variable, &edges[ei]); - } - new_bindings[new_count++] = nb; + binding_t nb = {0}; + binding_copy(&nb, b); + binding_set(&nb, start_var, &found); + if (rel->variable) { + binding_set_edge(&nb, rel->variable, &edges[ei]); } + (void)binding_out_append(&new_bindings, &new_count, &new_cap, &nb); node_fields_free(&found); } cbm_store_free_edges(edges, edge_count); @@ -4711,15 +4725,12 @@ static void expand_from_bound_terminal(cbm_store_t *store, cbm_pattern_t *patn, if (opt && match_count == 0) { /* GENUINELY no matching neighbour: the scan above runs unconditionally, * so match_count is the true neighbour count and match_count == 0 here - * means the terminal really has none — not merely that the buffer filled - * first. Keep the row with start_var left UNBOUND - * so `WHERE IS NULL` correctly identifies the no-edge case. The - * buffer is sized max_new + *bind_count precisely so every fallback row - * has a slot — no guard needed, and no OPTIONAL no-match row is dropped - * even after the expansion has saturated max_new. */ + * means the terminal really has none. Keep the row with start_var left + * UNBOUND so `WHERE IS NULL` correctly identifies the no-edge + * case; the shared growable append gives every fallback row a slot. */ binding_t nb = {0}; binding_copy(&nb, b); - new_bindings[new_count++] = nb; + (void)binding_out_append(&new_bindings, &new_count, &new_cap, &nb); } } diff --git a/tests/test_cypher.c b/tests/test_cypher.c index 4e9132cf9..ff5250c48 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -688,9 +688,12 @@ TEST(cypher_exec_optional_saturated_does_not_fabricate_no_match) { /* 3 Function nodes → scan_count = 3; max_rows = 3 → bind_cap = 3 and * max_new = 30. A alone exceeds that, so B and C are reached with the * budget already spent — the regime that produced the fabrication. */ - cbm_node_t a = {.project = "test", .label = "Function", .name = "A", .qualified_name = "test.A"}; - cbm_node_t b = {.project = "test", .label = "Function", .name = "B", .qualified_name = "test.B"}; - cbm_node_t c = {.project = "test", .label = "Function", .name = "C", .qualified_name = "test.C"}; + cbm_node_t a = { + .project = "test", .label = "Function", .name = "A", .qualified_name = "test.A"}; + cbm_node_t b = { + .project = "test", .label = "Function", .name = "B", .qualified_name = "test.B"}; + cbm_node_t c = { + .project = "test", .label = "Function", .name = "C", .qualified_name = "test.C"}; int64_t a_id = cbm_store_upsert_node(s, &a); (void)cbm_store_upsert_node(s, &b); /* B: no outgoing CALLS at all */ int64_t c_id = cbm_store_upsert_node(s, &c); @@ -965,9 +968,9 @@ TEST(cypher_exec_bound_terminal_saturation_no_false_deadcode) { ASSERT_EQ(rc, 0); ASSERT_EQ(r.col_count, 2); - bool hub_expanded = false; /* sanity: the buffer really did fill from a hub */ + bool hub_expanded = false; /* sanity: the buffer really did fill from a hub */ bool hub_false_deadcode = false; /* the bug: a hub with callers invented as dead */ - bool leaf_deadcode = false; /* the lossless property: genuine dead code kept */ + bool leaf_deadcode = false; /* the lossless property: genuine dead code kept */ for (int i = 0; i < r.row_count; i++) { const char *f = r.rows[i][0]; const char *c = r.rows[i][1]; @@ -1034,8 +1037,8 @@ TEST(cypher_exec_unlabeled_where_beyond_result_limit_issue1196) { ASSERT_GT(cbm_store_upsert_node(s, &late), 0); cbm_cypher_result_t r = {0}; - int rc = cbm_cypher_execute( - s, "MATCH (n) WHERE n.name = \"zz_late_match\" RETURN n.name", "test", 1, &r); + int rc = cbm_cypher_execute(s, "MATCH (n) WHERE n.name = \"zz_late_match\" RETURN n.name", + "test", 1, &r); ASSERT_EQ(rc, 0); ASSERT_EQ(r.row_count, 1); ASSERT_STR_EQ(r.rows[0][0], "zz_late_match"); @@ -1045,6 +1048,73 @@ TEST(cypher_exec_unlabeled_where_beyond_result_limit_issue1196) { PASS(); } +/* #1196 (second mechanism): relationship expansion capped this hop's TOTAL + * output at bind_cap*10, so edges past the cap were silently dropped BEFORE + * WHERE and aggregation — a count() then reported the scanned prefix as if + * it were a fact (field-measured: 9,360 of 13,691 DEFINES with a labeled + * source and --max-rows 1000). max_rows is an OUTPUT-row limit (projection + * already enforces it); expansion must see every matched edge. Fixture: 2 + * labeled sources with 30 edges each; max_rows=2 makes the old cap 20. */ +TEST(cypher_exec_aggregate_sees_all_edges_beyond_expansion_cap_issue1196) { + cbm_store_t *s = cbm_store_open_memory(); + ASSERT_NOT_NULL(s); + ASSERT_EQ(cbm_store_upsert_project(s, "test", "/tmp/test"), CBM_STORE_OK); + + int64_t src_ids[2]; + for (int i = 0; i < 2; i++) { + char name[32]; + char qn[64]; + snprintf(name, sizeof(name), "file_%d", i); + snprintf(qn, sizeof(qn), "test.%s", name); + cbm_node_t src = {.project = "test", + .label = "File", + .name = name, + .qualified_name = qn, + .file_path = name}; + src_ids[i] = cbm_store_upsert_node(s, &src); + ASSERT_GT(src_ids[i], 0); + } + for (int i = 0; i < 2; i++) { + for (int j = 0; j < 30; j++) { + char name[32]; + char qn[64]; + snprintf(name, sizeof(name), "def_%d_%02d", i, j); + snprintf(qn, sizeof(qn), "test.%s", name); + cbm_node_t target = {.project = "test", + .label = "Function", + .name = name, + .qualified_name = qn, + .file_path = "defs.py"}; + int64_t tid = cbm_store_upsert_node(s, &target); + ASSERT_GT(tid, 0); + cbm_edge_t e = { + .project = "test", .source_id = src_ids[i], .target_id = tid, .type = "DEFINES"}; + cbm_store_insert_edge(s, &e); + } + } + + /* Aggregate: one output row, so max_rows=2 never limits the OUTPUT — + * only the (buggy) expansion. Ground truth: 60 edges. */ + cbm_cypher_result_t r = {0}; + int rc = cbm_cypher_execute(s, "MATCH (a:File)-[rel]->(b) RETURN count(rel)", "test", 2, &r); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r.row_count, 1); + ASSERT_STR_EQ(r.rows[0][0], "60"); + cbm_cypher_result_free(&r); + + /* The list form must saturate at the output limit, not at the scan: + * max_rows=25 returns exactly 25 rows (old cap: bind_cap=25 -> 250, + * fine here — but max_rows=2 must return 2 rows, not 2-of-20-scanned). */ + cbm_cypher_result_t r2 = {0}; + rc = cbm_cypher_execute(s, "MATCH (a:File)-[rel]->(b) RETURN b.name", "test", 2, &r2); + ASSERT_EQ(rc, 0); + ASSERT_EQ(r2.row_count, 2); + cbm_cypher_result_free(&r2); + + cbm_store_close(s); + PASS(); +} + /* #874: coalesce(var.prop, literal) in WHERE — null-safe numeric filters * for audit queries over OPTIONAL graph properties. The parser rejected the * call outright ("unexpected operator"); RETURN-side coalesce already @@ -2946,11 +3016,10 @@ TEST(cypher_exec_multikey_order_by_keeps_limit_issue1334) { cbm_store_t *s = setup_cypher_store(); cbm_cypher_result_t r = {0}; /* start_lines: HandleOrder=10, ValidateOrder=5, SubmitOrder=0, LogError=0 */ - int rc = cbm_cypher_execute( - s, - "MATCH (f:Function) RETURN f.name, f.start_line " - "ORDER BY f.start_line DESC, f.name ASC LIMIT 2", - "test", 0, &r); + int rc = cbm_cypher_execute(s, + "MATCH (f:Function) RETURN f.name, f.start_line " + "ORDER BY f.start_line DESC, f.name ASC LIMIT 2", + "test", 0, &r); ASSERT_EQ(rc, 0); ASSERT_EQ(r.row_count, 2); ASSERT_STR_EQ(r.rows[0][0], "HandleOrder"); @@ -2966,11 +3035,10 @@ TEST(cypher_exec_multikey_order_by_tiebreak_issue1334) { cbm_cypher_result_t r = {0}; /* start_line ASC puts the two 0-line functions first; name DESC breaks the * tie: SubmitOrder before LogError. */ - int rc = cbm_cypher_execute( - s, - "MATCH (f:Function) RETURN f.name, f.start_line " - "ORDER BY f.start_line ASC, f.name DESC LIMIT 2", - "test", 0, &r); + int rc = cbm_cypher_execute(s, + "MATCH (f:Function) RETURN f.name, f.start_line " + "ORDER BY f.start_line ASC, f.name DESC LIMIT 2", + "test", 0, &r); ASSERT_EQ(rc, 0); ASSERT_EQ(r.row_count, 2); ASSERT_STR_EQ(r.rows[0][0], "SubmitOrder"); @@ -3295,8 +3363,7 @@ TEST(cypher_issue1111_with_type_count_group) { cbm_store_t *s = setup_cypher_store(); cbm_cypher_result_t r = {0}; int rc = cbm_cypher_execute( - s, - "MATCH (a)-[r]->(b) WITH type(r) AS t, count(*) AS n RETURN t, n ORDER BY n DESC", + s, "MATCH (a)-[r]->(b) WITH type(r) AS t, count(*) AS n RETURN t, n ORDER BY n DESC", "test", 0, &r); ASSERT_EQ(rc, 0); ASSERT_EQ(r.row_count, 2); @@ -3938,6 +4005,7 @@ SUITE(cypher) { RUN_TEST(cypher_issue305_count_star_alias); RUN_TEST(cypher_exec_where_eq); RUN_TEST(cypher_exec_unlabeled_where_beyond_result_limit_issue1196); + RUN_TEST(cypher_exec_aggregate_sees_all_edges_beyond_expansion_cap_issue1196); RUN_TEST(cypher_exec_varlength_path_semantics_issue797); RUN_TEST(cypher_exec_where_coalesce_issue874); RUN_TEST(cypher_exec_where_regex); From 32633bab9a20f7d95785c14c16daf30999094e58 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 18 Aug 2026 00:20:54 +0200 Subject: [PATCH 13/15] fix(build): LF contract for extensionless git hooks + a line-ending guard The *.sh eol=lf rule landed via #1314, but the git hooks (scripts/git-hooks/commit-msg, scripts/hooks/pre-commit) are extensionless and were still at the mercy of core.autocrlf. Add their explicit entries, plus the contract test from #1272 wired as scripts/test.sh Step 0t: every shell entrypoint (*.sh + both hook directories) must carry an eol=lf attribute, with a matched-zero-files guard so a broken glob can never pass vacuously. Distilled from #1272: the diagnosis (CRLF checkouts breaking shebangs under WSL/MSYS), the hook-file coverage, and the guard design are @xumian520's; verified RED without the *.sh rule (105 uncovered entrypoints) and green with it (107 files). Co-Authored-By: xumian520 <126989134+xumian520@users.noreply.github.com> Signed-off-by: Martin Vogel --- .gitattributes | 6 +++++ scripts/test.sh | 3 +++ tests/test_shell_line_endings.sh | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 tests/test_shell_line_endings.sh diff --git a/.gitattributes b/.gitattributes index 06a4e43d8..a76ea0bd6 100644 --- a/.gitattributes +++ b/.gitattributes @@ -5,3 +5,9 @@ # Vendored tree-sitter grammars — machine-generated, hide from stats and diffs internal/cbm/vendored/grammars/**/parser.c linguist-generated=true diff=false internal/cbm/vendored/grammars/**/scanner.c linguist-generated=true diff=false + +# Shell entrypoints must check out LF everywhere: a CRLF shebang line breaks +# them under WSL/MSYS (`bash\r: no such file`). The *.sh rule covers scripts; +# the git hooks are extensionless and need explicit entries. (#1272) +scripts/git-hooks/commit-msg text eol=lf +scripts/hooks/pre-commit text eol=lf diff --git a/scripts/test.sh b/scripts/test.sh index 0ec489a59..0dc575b04 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -260,6 +260,9 @@ bash "$ROOT/tests/test_release_gate_chain_contract.sh" echo "=== Step 0t: test runtime isolation contract (#1691) ===" bash "$ROOT/tests/test_runtime_isolation_contract.sh" +echo "=== Step 0u: shell line-ending contract ===" +bash "$ROOT/tests/test_shell_line_endings.sh" + # Verify compiler supports target arch verify_compiler "$CC" diff --git a/tests/test_shell_line_endings.sh b/tests/test_shell_line_endings.sh new file mode 100644 index 000000000..a6f3fc387 --- /dev/null +++ b/tests/test_shell_line_endings.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Regression guard: shell entrypoints must remain LF in Windows checkouts so +# they can run directly from WSL and MSYS without a `bash\r` shebang failure. +# +# Distilled from PR #1272 by @xumian520, who both hit the breakage under +# core.autocrlf=true and wrote this contract so it stays fixed. The .sh rule +# itself landed via #1314; the extensionless git hooks need their own +# entries, which this guard also covers. + +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$ROOT" + +failures=0 +checked=0 +while IFS= read -r -d '' path && + IFS= read -r -d '' attribute && + IFS= read -r -d '' eol; do + checked=$((checked + 1)) + if [[ "$eol" != "lf" ]]; then + echo "FAIL: $path must declare eol=lf (got ${eol:-unset})" >&2 + failures=$((failures + 1)) + fi +done < <( + git ls-files -z '*.sh' 'scripts/git-hooks/*' 'scripts/hooks/*' | + git check-attr -z --stdin eol +) + +if ((checked == 0)); then + echo "FAIL: the line-ending contract matched no files — the glob set is broken" >&2 + exit 1 +fi + +if ((failures > 0)); then + echo "FAIL: $failures shell entrypoint(s) lack an LF checkout contract" >&2 + exit 1 +fi + +echo "Shell line-ending contract passed ($checked files)" From 9427dd075d3298c4630416149e7ffd6977043935 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 18 Aug 2026 22:12:19 +0200 Subject: [PATCH 14/15] fix(ci): release gates fail closed on cancelled jobs + refuse malformed version input Three fixes from the v0.10.7 release incident (2026-08-18): 1. build (and smoke/soak) required 'not failed' instead of explicit success. failure() does not cover a needed job that TIMED OUT (conclusion 'cancelled'), so lint hitting its 15-min timeout cascaded test into 'skipped' and the pipeline published with the whole test matrix and asan-soak silently skipped. build now requires lint success plus either test success or the sanctioned skip_tests input; smoke/soak require build success explicitly. 2. The tag is inputs.version verbatim: dispatching a bare '0.10.7' published a release the installers can never resolve (they fetch releases/download/v/...), and under immutable releases the mis-named tag cannot be retagged or its name reused. A preflight job now refuses any non-v-prefixed version before anything runs. 3. Lint's 15-min timeout was one slow-runner day away from cancelling a normally-5-min job; raised to 30 so only a genuine hang can hit it. Release-path only (workflow_dispatch); adds one ~5s preflight job; no PR-CI gating, cost, or trigger changes. Signed-off-by: Martin Vogel --- .github/workflows/_lint.yml | 5 ++++- .github/workflows/release.yml | 41 ++++++++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/.github/workflows/_lint.yml b/.github/workflows/_lint.yml index 3095e09f9..aa7eb7062 100644 --- a/.github/workflows/_lint.yml +++ b/.github/workflows/_lint.yml @@ -11,7 +11,10 @@ permissions: jobs: lint: runs-on: ubuntu-latest - timeout-minutes: 15 + # 15 min proved too tight on a slow runner day and a timed-out lint reads + # as "cancelled", which the release graph must treat as a hard stop; keep + # a bound, but one only a genuine hang can hit (normal runtime ~5 min). + timeout-minutes: 30 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5f81c708b..0dba1720f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -46,8 +46,29 @@ jobs: uses: ./.github/workflows/_security.yml secrets: inherit + # ── 0. Preflight: refuse malformed dispatch inputs ────────────── + # The tag is inputs.version VERBATIM. A bare "0.10.7" publishes a release the + # installers can never resolve (they fetch releases/download/v/...), + # and with immutable releases the mis-named tag cannot be retagged, deleted, + # or its name reused — the name is burned permanently (2026-08-18 incident). + preflight: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Enforce v-prefixed semver version input + env: + VERSION: ${{ inputs.version }} + run: | + if [[ "$VERSION" =~ ^v[0-9]+\.[0-9]+\.[0-9]+([-.][0-9A-Za-z.]+)?$ ]]; then + echo "version OK: $VERSION" + else + echo "::error::version must be v-prefixed semver (vX.Y.Z), got '$VERSION'" + exit 1 + fi + # ── 1. Lint (cppcheck + clang-format) ─────────────────────────── lint: + needs: [preflight] uses: ./.github/workflows/_lint.yml # ── 2. Tests (all platforms, full suite for release) ──────────── @@ -65,11 +86,19 @@ jobs: shard_suites: true # ── 3. Build all platforms ────────────────────────────────────── - # !cancelled() && !failure(): run when `test` is deliberately skipped, but - # never when lint or test actually failed. + # `test` may be skipped ONLY by the skip_tests input. A skipped `test` is + # also what a cancelled/timed-out lint produces (needs-cascade), and the + # bare !cancelled() && !failure() form cannot tell those apart: failure() + # does not cover a needed job that was CANCELLED, so a lint timeout let the + # whole pipeline publish with the test matrix silently skipped + # (v0.10.7 incident, 2026-08-18). Require the explicit results. build: - if: ${{ !cancelled() && !failure() }} - needs: [test] + if: >- + ${{ !cancelled() + && needs.lint.result == 'success' + && (needs.test.result == 'success' + || (inputs.skip_tests && needs.test.result == 'skipped')) }} + needs: [lint, test] permissions: contents: read id-token: write @@ -92,7 +121,7 @@ jobs: # an optional phase needs this override, or the phase being optional silently # makes the phases after it optional as well. smoke: - if: ${{ !cancelled() && !failure() }} + if: ${{ !cancelled() && needs.build.result == 'success' }} needs: [build] uses: ./.github/workflows/_smoke.yml with: @@ -100,7 +129,7 @@ jobs: # ── 5. Soak tests ────────────────────────────────────────────── soak: - if: ${{ !cancelled() && !failure() }} + if: ${{ !cancelled() && needs.build.result == 'success' }} needs: [build] uses: ./.github/workflows/_soak.yml with: From 98c1f8c3e19e047b0daa843632e75effb9652341 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 18 Aug 2026 22:30:28 +0200 Subject: [PATCH 15/15] test: gate-chain contract pins the explicit-result conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract required the bare '!cancelled() && !failure()' idiom on build/smoke/soak — the exact form the v0.10.7 incident proved fail-open (failure() does not cover a CANCELLED needed job). It now requires each gate's explicit accepted results, the sanctioned skip_tests clause on build, and the preflight version guard wired before lint. Signed-off-by: Martin Vogel --- tests/test_release_gate_chain_contract.sh | 45 ++++++++++++++++++----- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/tests/test_release_gate_chain_contract.sh b/tests/test_release_gate_chain_contract.sh index 3bb70b08b..31c99041d 100755 --- a/tests/test_release_gate_chain_contract.sh +++ b/tests/test_release_gate_chain_contract.sh @@ -63,19 +63,46 @@ def cond(job): return "" return " ".join(m.group(2).split()) -# 1. Downstream-of-optional jobs must tolerate a deliberately skipped ancestor. -# `test` is the optional phase (if: !inputs.skip_tests); everything after it -# in the chain has to survive that. -TOLERATE = ["build", "smoke", "soak", "release-draft"] -for job in TOLERATE: +# 1. Gate conditions: tolerate ONLY the sanctioned skip, fail closed otherwise. +# `test` is the optional phase (if: !inputs.skip_tests). The old contract +# required the bare `!cancelled() && !failure()` idiom — but failure() does +# NOT cover a needed job that was CANCELLED (e.g. a lint timeout), so that +# idiom let a cancelled gate cascade test into 'skipped' and publish with +# the whole test matrix silently gone (v0.10.7 incident, 2026-08-18). +# Each gate must name the results it accepts explicitly. +GATE_REQUIREMENTS = { + "build": [ + "!cancelled()", + "needs.lint.result == 'success'", + "needs.test.result == 'success'", + "inputs.skip_tests && needs.test.result == 'skipped'", + ], + "smoke": ["!cancelled()", "needs.build.result == 'success'"], + "soak": ["!cancelled()", "needs.build.result == 'success'"], + "release-draft": ["!cancelled()", "!failure()"], +} +for job, required in GATE_REQUIREMENTS.items(): if job not in blocks: failures.append(f"{job}: job missing from release.yml — update this contract") continue c = cond(job) - if "!cancelled()" not in c or "!failure()" not in c: - failures.append( - f"{job}: `if:` lacks `!cancelled() && !failure()` (got: {c or ''}).\n" - f" With skip_tests=true a skipped ancestor SKIPS this job silently.") + for fragment in required: + if fragment not in c: + failures.append( + f"{job}: `if:` must contain `{fragment}` (got: {c or ''}).\n" + f" Explicit results only: failure() misses CANCELLED needed\n" + f" jobs, and a bare tolerate-skip idiom is fail-open.") + +# 1b. The preflight input guard must exist and gate the whole chain: the tag is +# inputs.version verbatim, and a bare (non-v-prefixed) version publishes a +# release installers can never resolve — unrecoverable under immutability. +if "preflight" not in blocks: + failures.append("preflight: job missing — the version-input guard must exist") +lint_needs = re.search(r"^ needs:\s*(.*)$", blocks.get("lint", ""), re.M) +if not lint_needs or "preflight" not in lint_needs.group(1): + failures.append( + "lint: must `needs: [preflight]` so a malformed version stops the\n" + " chain before any gate runs.") # 2. The draft must require both runtime gates to have genuinely succeeded. draft = cond("release-draft")