From 5abd77f5d9580d75f8600914b46495be21e9a4a5 Mon Sep 17 00:00:00 2001 From: astandrik Date: Sun, 6 Sep 2026 16:46:18 +0300 Subject: [PATCH 01/10] fix(tests): create nested pipeline fixtures reliably Create missing parent directories recursively and require fixture nodes before negative graph assertions. Cover exact bytes, deep paths and blocked parents. Fixes #2034 Signed-off-by: astandrik --- tests/test_pipeline.c | 324 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 291 insertions(+), 33 deletions(-) diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index cf40dd57a3..353ce0c70c 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -1173,6 +1173,26 @@ static int named_node_count(cbm_store_t *s, const char *project, const char *nam return count; } +static int fixture_node_count(cbm_store_t *store, const char *project, const char *path, + const char *name, const char *label) { + cbm_node_t *nodes = NULL; + int count = 0; + if (cbm_store_find_nodes_by_name(store, project, name, &nodes, &count) != CBM_STORE_OK) { + return -1; + } + int found = 0; + for (int i = 0; i < count; i++) { + if (nodes[i].file_path && nodes[i].label && strcmp(nodes[i].file_path, path) == 0 && + strcmp(nodes[i].label, label) == 0) { + found++; + } + } + if (nodes) { + cbm_store_free_nodes(nodes, count); + } + return found; +} + typedef struct { int run_rc; bool store_opened; @@ -4697,6 +4717,8 @@ TEST(pipeline_tsjs_receiver_suppresses_weak_method_edge) { ASSERT_NOT_NULL(s); /* (1) The false edge is suppressed (reproduce-first: RED before the fix). */ + ASSERT_GTE(fixture_node_count(s, project, "src/caller.ts", "checkFormat", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "src/client.ts", "test", "Method"), 1); ASSERT_FALSE(cross_file_call_exists(s, project, "checkFormat", "test")); /* (2) The type-resolved receiver call survives (LSP wins before the guard). */ ASSERT_TRUE(cross_file_call_exists(s, project, "runTyped", "test")); @@ -4758,6 +4780,8 @@ TEST(pipeline_python_receiver_suppresses_weak_method_edge) { ASSERT_NOT_NULL(s); /* NEGATIVE: `accelerator` is a parameter — Worker.backward must not bind. */ + ASSERT_GTE(fixture_node_count(s, project, "caller.py", "external_call", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "worker.py", "backward", "Method"), 1); ASSERT_FALSE(cross_file_call_exists(s, project, "external_call", "backward")); /* POSITIVE: import-bound receiver and bare local call both survive. */ ASSERT_TRUE(cross_file_call_exists(s, project, "imported_call", "compute")); @@ -4778,13 +4802,14 @@ TEST(pipeline_python_receiver_suppresses_weak_method_edge) { * path and both must consult the guard. */ static void write_go_c_ref_guard_fixture(const char *tmp, int pad_files) { write_temp_file(tmp, "go.mod", "module example.com/fxguard\n\ngo 1.22\n"); - /* The C probe: a local named `event` and a file-scope function `handle`, - * the two shapes Go identifiers collide with. */ + /* Both C targets must be indexed: block-local C variables are not emitted, + * so keep event at file scope for the cross-language reference guard. */ write_temp_file(tmp, "probe/probe.c", "static int total_events = 0;\n" + "static int event = 0;\n" "\n" "static int handle(void) {\n" - " int event = 0;\n" + " event = 0;\n" " total_events += event;\n" " return event;\n" "}\n"); @@ -4856,6 +4881,12 @@ TEST(pipeline_go_rw_usage_never_cross_into_c) { /* Reproduce-first: RED before the fix — the Go local write binds the C * probe's global, and the Go value use binds the C `handle`. */ + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "TrackEvent", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "UsesHandle", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "WriteTotal", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "probe/probe.c", "handle", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "probe/probe.c", "event", "Variable"), 1); + ASSERT_GTE(fixture_node_count(s, project, "probe/probe.c", "total_events", "Variable"), 1); ASSERT_FALSE(cross_file_edge_exists(s, project, "TrackEvent", "event", "WRITES")); ASSERT_FALSE(cross_file_edge_exists(s, project, "TrackEvent", "event", "READS")); ASSERT_FALSE(cross_file_edge_exists(s, project, "UsesHandle", "handle", "WRITES")); @@ -4896,6 +4927,12 @@ TEST(pipeline_go_rw_usage_never_cross_into_c_parallel) { cbm_store_t *s = cbm_store_open_path(db_path); ASSERT_NOT_NULL(s); + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "TrackEvent", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "UsesHandle", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "WriteTotal", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "probe/probe.c", "handle", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "probe/probe.c", "event", "Variable"), 1); + ASSERT_GTE(fixture_node_count(s, project, "probe/probe.c", "total_events", "Variable"), 1); ASSERT_FALSE(cross_file_edge_exists(s, project, "TrackEvent", "event", "WRITES")); ASSERT_FALSE(cross_file_edge_exists(s, project, "TrackEvent", "event", "READS")); ASSERT_FALSE(cross_file_edge_exists(s, project, "UsesHandle", "handle", "WRITES")); @@ -4981,7 +5018,8 @@ TEST(pipeline_go_bare_ref_never_binds_field) { ASSERT_NOT_NULL(s); /* The field must exist for the probe to mean anything (#1935's fix). */ - ASSERT_TRUE(count_nodes_named(s, project, "err") >= 1); + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "Run", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "state/state.go", "err", "Field"), 1); /* Reproduce-first: RED before the fix — the bare local binds the field. */ ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "WRITES")); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "READS")); @@ -5018,7 +5056,8 @@ TEST(pipeline_go_bare_ref_never_binds_field_parallel) { cbm_store_t *s = cbm_store_open_path(db_path); ASSERT_NOT_NULL(s); - ASSERT_TRUE(count_nodes_named(s, project, "err") >= 1); + ASSERT_GTE(fixture_node_count(s, project, "app/app.go", "Run", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "state/state.go", "err", "Field"), 1); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "WRITES")); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "READS")); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "USAGE")); @@ -5166,6 +5205,10 @@ TEST(pipeline_tsjs_receiver_parallel_keeps_service_edges) { ASSERT_GTE(count_nodes_named(s, project, "/users"), 1); /* (3) The false plain-CALLS edges are suppressed in parallel: the regex * receiver and the dev.load weak match to ApiThing.load. */ + ASSERT_GTE(fixture_node_count(s, project, "src/re.ts", "checkFormat", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "src/thing.ts", "test", "Method"), 1); + ASSERT_GTE(fixture_node_count(s, project, "src/load.ts", "callLoad", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "src/thing.ts", "load", "Method"), 1); ASSERT_FALSE(cross_file_call_exists(s, project, "checkFormat", "test")); ASSERT_FALSE(cross_file_call_exists(s, project, "callLoad", "load")); @@ -5313,6 +5356,10 @@ TEST(pipeline_python_receiver_parallel_suppresses_weak_method_edges) { ASSERT_NOT_NULL(s); /* NEGATIVE: every receiver here is a parameter or an attribute of one. */ + ASSERT_GTE(fixture_node_count(s, project, "caller.py", "train", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "targets.py", "print", "Method"), 2); + ASSERT_GTE(fixture_node_count(s, project, "targets.py", "backward", "Method"), 2); + ASSERT_GTE(fixture_node_count(s, project, "targets.py", "step", "Method"), 2); ASSERT_FALSE(cross_file_call_exists(s, project, "train", "print")); ASSERT_FALSE(cross_file_call_exists(s, project, "train", "backward")); ASSERT_FALSE(cross_file_call_exists(s, project, "train", "step")); @@ -5624,6 +5671,9 @@ TEST(pipeline_arg_url_rejects_non_http_slash_arguments) { cbm_store_t *s = cbm_store_open_path(db_path); ASSERT_NOT_NULL(s); + ASSERT_GTE(fixture_node_count(s, project, "src/args.py", "write_fixture", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "src/args.py", "load_api", "Function"), 1); + ASSERT_GTE(fixture_node_count(s, project, "src/regex.js", "sanitize", "Function"), 1); ASSERT_EQ(count_nodes_named(s, project, "/html/g"), 0); ASSERT_EQ(count_nodes_named(s, project, "/ expected_length - offset || memcmp(actual, expected + offset, bytes) != 0) { + matched = false; + break; + } + offset += bytes; + } + matched = matched && !ferror(file) && feof(file) && offset == expected_length; + if (fclose(file) != 0) { + matched = false; + } + return matched; +} + +/* #2034: no manual mkdir may make an absent deep fixture look indexed. */ +TEST(pipeline_nested_fixture_files_are_written) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_nested_fixture_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + char long_name[480]; + size_t long_length = 480 - strlen(tmp) - 1; + memset(long_name, 'a', long_length); + for (size_t i = 89; i < long_length - 4; i += 90) { + long_name[i] = '/'; + } + memcpy(long_name + long_length - 4, ".txt", 4); + long_name[long_length] = '\0'; + char boundary[3][1026]; + for (size_t i = 0; i < 3; i++) { + size_t length = 1023 + i; + memset(boundary[i], 'a', length); + boundary[i][length] = '\0'; + } + const struct { + const char *name; + const char *content; + } cases[] = { + {"root.go", "package fixtures\n"}, + {"pkg/one.go", "package fixtures\n\nfunc Marker() {}\n"}, + {"registry/batch/two.go", "package fixtures\n"}, + {"registry/batch/cronjob/strategy.go", "package fixtures\n"}, + {"a/b/c/d/e/f/g/h/deep.txt", "deep\n"}, + {"space dir/кириллица/中文/data.txt", "строка\nline\r\n行\n"}, + {"empty/a/file.txt", ""}, + {"newline/a/file.txt", "\n"}, + {"siblings/one/file.txt", "one\n"}, + {"siblings/two/file.txt", "two\n"}, + {"partial/existing/new/branch/file.txt", "partial\n"}, + {long_name, "long path\n"}, + {"boundary/deep/1023.txt", boundary[0]}, + {"boundary/deep/1024.txt", boundary[1]}, + {"boundary/deep/1025.txt", boundary[2]}, + }; + bool partial_parent = cbm_mkdir_p(TH_PATH(tmp, "partial/existing"), 0755); + bool matched[sizeof(cases) / sizeof(cases[0])] = {false}; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + write_temp_file(tmp, cases[i].name, cases[i].content); + } + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + matched[i] = fixture_file_matches(TH_PATH(tmp, cases[i].name), cases[i].content); + if (!matched[i]) { + fprintf(stderr, "missing or mismatched fixture: %s\n", cases[i].name); + } + } + const char *replacement[] = {"longer replacement\n", "x", "", "again\n"}; + bool replaced[sizeof(replacement) / sizeof(replacement[0])] = {false}; + for (size_t i = 0; i < sizeof(replacement) / sizeof(replacement[0]); i++) { + write_temp_file(tmp, "registry/batch/cronjob/strategy.go", replacement[i]); + replaced[i] = fixture_file_matches(TH_PATH(tmp, "registry/batch/cronjob/strategy.go"), + replacement[i]); + } + bool sibling_preserved = fixture_file_matches(TH_PATH(tmp, "siblings/one/file.txt"), "one\n"); + bool extra_tail_matches = + fixture_file_matches(TH_PATH(tmp, "boundary/deep/1025.txt"), boundary[1]); + bool truncated_matches = + fixture_file_matches(TH_PATH(tmp, "boundary/deep/1023.txt"), boundary[1]); + char changed[sizeof(boundary[2])]; + memcpy(changed, boundary[2], sizeof(changed)); + changed[1024] = 'b'; + bool changed_byte_matches = + fixture_file_matches(TH_PATH(tmp, "boundary/deep/1025.txt"), changed); + int cleanup_rc = th_rmtree(tmp); + ASSERT_TRUE(partial_parent); + ASSERT_FALSE(extra_tail_matches); + ASSERT_FALSE(truncated_matches); + ASSERT_FALSE(changed_byte_matches); + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + ASSERT_TRUE(matched[i]); + } + for (size_t i = 0; i < sizeof(replacement) / sizeof(replacement[0]); i++) { + ASSERT_TRUE(replaced[i]); + } + ASSERT_TRUE(sibling_preserved); + ASSERT_EQ(cleanup_rc, 0); + PASS(); +} + +TEST(pipeline_fixture_file_parent_is_preserved) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_fixture_parent_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + write_temp_file(tmp, "blocked", "existing bytes\n"); + write_temp_file(tmp, "blocked/child/file.txt", "must not be written\n"); + bool parent_preserved = fixture_file_matches(TH_PATH(tmp, "blocked"), "existing bytes\n"); + bool child_present = cbm_file_exists(TH_PATH(tmp, "blocked/child/file.txt")); + bool precondition_satisfied = + fixture_file_matches(TH_PATH(tmp, "blocked/child/file.txt"), "must not be written\n"); + int cleanup_rc = th_rmtree(tmp); + ASSERT_TRUE(parent_preserved); + ASSERT_FALSE(child_present); + ASSERT_FALSE(precondition_satisfied); + ASSERT_EQ(cleanup_rc, 0); + PASS(); +} + +TEST(pipeline_fixture_node_count_requires_exact_source_nodes) { + cbm_store_t *store = cbm_store_open_memory(); + ASSERT_NOT_NULL(store); + ASSERT_EQ(cbm_store_upsert_project(store, "fixture", "/tmp"), CBM_STORE_OK); + cbm_node_t nodes[] = { + {.project = "fixture", + .label = "Method", + .name = "work", + .qualified_name = "fixture.Service.work", + .file_path = "nested/service.py"}, + {.project = "fixture", + .label = "Method", + .name = "work", + .qualified_name = "fixture.OtherService.work", + .file_path = "nested/service.py"}, + {.project = "fixture", + .label = "Method", + .name = "work", + .qualified_name = "fixture.OtherFile.work", + .file_path = "other/service.py"}, + {.project = "fixture", + .label = "Function", + .name = "work", + .qualified_name = "fixture.free_work", + .file_path = "nested/service.py"}, + }; + bool inserted = true; + for (size_t i = 0; i < sizeof(nodes) / sizeof(nodes[0]); i++) { + if (cbm_store_upsert_node(store, &nodes[i]) <= 0) { + inserted = false; + } + } + int methods = fixture_node_count(store, "fixture", "nested/service.py", "work", "Method"); + int other_file = fixture_node_count(store, "fixture", "other/service.py", "work", "Method"); + int wrong_file = fixture_node_count(store, "fixture", "absent/service.py", "work", "Method"); + int wrong_label = fixture_node_count(store, "fixture", "nested/service.py", "work", "Class"); + int missing_name = + fixture_node_count(store, "fixture", "nested/service.py", "absent", "Method"); + int unavailable = fixture_node_count(NULL, "fixture", "nested/service.py", "work", "Method"); + cbm_store_close(store); + ASSERT_TRUE(inserted); + ASSERT_EQ(methods, 2); + ASSERT_EQ(other_file, 1); + ASSERT_EQ(wrong_file, 0); + ASSERT_EQ(wrong_label, 0); + ASSERT_EQ(missing_name, 0); + ASSERT_EQ(unavailable, -1); + PASS(); +} + +TEST(pipeline_nested_fixture_graph_has_endpoints) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_nested_graph_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + write_temp_file(tmp, "main.go", "package main\n\nfunc main() {}\n"); + write_temp_file(tmp, "registry/batch/cronjob/strategy.go", + "package cronjob\n\n" + "func FixtureTarget() {}\n" + "func FixtureCaller() { FixtureTarget() }\n" + "func FixtureDecoy() {}\n"); + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/fixture.db", tmp); + cbm_pipeline_t *pipeline = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + int run_rc = pipeline ? cbm_pipeline_run(pipeline) : -1; + cbm_store_t *store = run_rc == 0 ? cbm_store_open_path(db_path) : NULL; + const char *names[] = {"FixtureCaller", "FixtureTarget", "FixtureDecoy"}; + int counts[3] = {-1, -1, -1}; + bool expected_call = false; + bool forbidden_call = false; + if (store) { + const char *project = cbm_pipeline_project_name(pipeline); + for (size_t i = 0; i < 3; i++) { + counts[i] = fixture_node_count(store, project, "registry/batch/cronjob/strategy.go", + names[i], "Function"); + } + expected_call = cross_file_call_exists(store, project, "FixtureCaller", "FixtureTarget"); + forbidden_call = cross_file_call_exists(store, project, "FixtureCaller", "FixtureDecoy"); + cbm_store_close(store); + } + cbm_pipeline_free(pipeline); + int cleanup_rc = th_rmtree(tmp); + ASSERT_EQ(run_rc, 0); + for (size_t i = 0; i < 3; i++) { + ASSERT_EQ(counts[i], 1); + } + ASSERT_TRUE(expected_call); + ASSERT_FALSE(forbidden_call); + ASSERT_EQ(cleanup_rc, 0); + PASS(); +} + /* Helper: find binding by key in results */ static const cbm_env_binding_t *find_binding_by_key(const cbm_env_binding_t *bindings, int count, const char *key) { @@ -10220,22 +10491,13 @@ TEST(envscan_skips_ignored_dirs) { if (!cbm_mkdtemp(tmpdir)) FAIL("tmpdir"); - /* File inside .git should be skipped */ - char gitdir[512]; - snprintf(gitdir, sizeof(gitdir), "%s/.git", tmpdir); - cbm_mkdir(gitdir); - write_temp_file(tmpdir, ".git/config.sh", - "#!/bin/bash\nexport API_URL=\"https://api.example.com/v1\"\n"); - - /* File inside node_modules should be skipped */ - char nmdir[512]; - snprintf(nmdir, sizeof(nmdir), "%s/node_modules", tmpdir); - cbm_mkdir(nmdir); - char nmpkg[512]; - snprintf(nmpkg, sizeof(nmpkg), "%s/node_modules/pkg", tmpdir); - cbm_mkdir(nmpkg); - write_temp_file(tmpdir, "node_modules/pkg/config.sh", - "#!/bin/bash\nexport API_URL=\"https://api.example.com/v1\"\n"); + static const char ignored_content[] = + "#!/bin/bash\nexport API_URL=\"https://api.example.com/v1\"\n"; + write_temp_file(tmpdir, ".git/config.sh", ignored_content); + write_temp_file(tmpdir, "node_modules/pkg/config.sh", ignored_content); + bool git_written = fixture_file_matches(TH_PATH(tmpdir, ".git/config.sh"), ignored_content); + bool module_written = + fixture_file_matches(TH_PATH(tmpdir, "node_modules/pkg/config.sh"), ignored_content); /* File at root level should be scanned */ write_temp_file(tmpdir, "deploy.sh", @@ -10253,6 +10515,8 @@ TEST(envscan_skips_ignored_dirs) { if (strcmp(bindings[i].file_path, "deploy.sh") == 0) from_root = 1; } + ASSERT_TRUE(git_written); + ASSERT_TRUE(module_written); ASSERT_EQ(from_git, 0); ASSERT_EQ(from_nm, 0); ASSERT_EQ(from_root, 1); @@ -10604,16 +10868,6 @@ TEST(pkgmap_scan_repo_honors_discovery_exclusions) { if (!cbm_mkdtemp(tmpdir)) FAIL("tmpdir"); - char dir[512]; - snprintf(dir, sizeof(dir), "%s/packages", tmpdir); - cbm_mkdir(dir); - snprintf(dir, sizeof(dir), "%s/packages/app", tmpdir); - cbm_mkdir(dir); - snprintf(dir, sizeof(dir), "%s/vendor_big", tmpdir); - cbm_mkdir(dir); - snprintf(dir, sizeof(dir), "%s/vendor_big/lib", tmpdir); - cbm_mkdir(dir); - write_temp_file(tmpdir, "packages/app/package.json", "{\"name\":\"@org/app\",\"main\":\"index.js\"}\n"); write_temp_file(tmpdir, "vendor_big/lib/package.json", @@ -13410,6 +13664,10 @@ TEST(pipeline_markdown_and_config_prose_reaches_fts_body) { } SUITE(pipeline) { + RUN_TEST(pipeline_nested_fixture_files_are_written); + RUN_TEST(pipeline_fixture_file_parent_is_preserved); + RUN_TEST(pipeline_fixture_node_count_requires_exact_source_nodes); + RUN_TEST(pipeline_nested_fixture_graph_has_endpoints); RUN_TEST(pipeline_lsp_surface_persisted_and_body_edit_invariant); /* Index lock */ RUN_TEST(pipeline_lock_try_acquire); From aec9bfe3751d8c1f2eb2cb48d8fe584b9d3fd973 Mon Sep 17 00:00:00 2001 From: astandrik Date: Mon, 7 Sep 2026 17:07:37 +0300 Subject: [PATCH 02/10] test(pipeline): clarify cross-language fixture comments Signed-off-by: astandrik --- tests/test_pipeline.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index cbb9d00cf5..5db774c29b 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -4802,7 +4802,7 @@ TEST(pipeline_python_receiver_suppresses_weak_method_edge) { * path and both must consult the guard. */ static void write_go_c_ref_guard_fixture(const char *tmp, int pad_files) { write_temp_file(tmp, "go.mod", "module example.com/fxguard\n\ngo 1.22\n"); - /* Both C targets must be indexed: block-local C variables are not emitted, + /* C targets must be indexed: block-local C variables are not emitted, * so keep event at file scope for the cross-language reference guard. */ write_temp_file(tmp, "probe/probe.c", "static int total_events = 0;\n" @@ -4813,8 +4813,8 @@ static void write_go_c_ref_guard_fixture(const char *tmp, int pad_files) { " total_events += event;\n" " return event;\n" "}\n"); - /* Go: a local write named like the C local, and a value use named like - * the C function. Neither can touch anything in a C translation unit. */ + /* Go local writes and value uses must not bind to the C variables or + * functions in this fixture. */ write_temp_file(tmp, "app/app.go", "package app\n" "\n" From e4780c208632b3c42488a3c9e315014ef4da157e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 25 Sep 2026 18:49:37 +0200 Subject: [PATCH 03/10] fix(coverage): stop walking the source per error region in the #1071 check (#1735) cbm_subtract_macro_invocation_regions asked "is this region a call to a file-defined macro?" before "is it inside a function?". The first question (cbm_span_is_macro_invocation) walks the source from byte 0 to find the region's lines, so every file with parse-error regions paid regions x file bytes, whatever its language and whether or not it has a single function. On a multi-MB SQL dump that is up to 256 walks over the whole file. Both predicates are pure, so asking the cheap one first changes no answer: a region no function encloses never touches the source, and the regions that do share one cbm_line_offsets table built on first use. The table gives exactly the span the walk found; the walk stays as the fallback when the table cannot be allocated. #1071 behaviour is unchanged: same regions, same verdicts. Indexing this repository's src/ before and after gives identical graphs (7,339 nodes, 28,924 edges) and the same 46 parse_partial files with byte-identical ranges. Tests count the bytes read, never time them (test-seam counter cbm_test_macro_line_scan_bytes): - 40 top-level junk regions: 39,870 bytes read -> 0, still 40 regions - 60 in-body real errors: 85,725 bytes read -> 2,801 (one table = file size), still 60 regions Refs #1735 Signed-off-by: Martin Vogel --- internal/cbm/cbm.c | 63 +++++++++++++++++++++++++++++++++++++---- internal/cbm/cbm.h | 6 ++++ tests/test_extraction.c | 54 +++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 5 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index b54fb54a7a..a6fa8fd320 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -1773,6 +1773,19 @@ static void cbm_subtract_recovered_regions(cbm_error_regions_t *regs, const CBMD *regs = out; } +#ifdef CBM_ENABLE_TEST_SEAMS +/* Source bytes this thread has read to locate lines for the #1071 macro check + * (walks and line-table builds). Lets a test pin the check's cost as a count + * instead of a clock (#1735). Compiled only into seam-enabled test artifacts. */ +static CBM_TLS uint64_t tl_macro_line_scan_bytes = 0; +uint64_t cbm_test_macro_line_scan_bytes(void) { + return tl_macro_line_scan_bytes; +} +#define CBM_MACRO_LINE_SCAN(n) (tl_macro_line_scan_bytes += (uint64_t)(n)) +#else +#define CBM_MACRO_LINE_SCAN(n) ((void)0) +#endif + /* #1071: a function-like macro invocation whose argument is a type token * (e.g. ALLOC(int, n)) makes tree-sitter's C/C++ grammar emit an ERROR node — it * parses `int` in expression position — which would be recorded as a parse_partial @@ -1862,6 +1875,7 @@ static bool cbm_span_is_macro_invocation(const char *src, int src_len, uint32_t } } if (line != start_line) { + CBM_MACRO_LINE_SCAN(span_start); return false; } int span_end = span_start; @@ -1870,6 +1884,7 @@ static bool cbm_span_is_macro_invocation(const char *src, int src_len, uint32_t line++; } } + CBM_MACRO_LINE_SCAN(span_end); return cbm_byte_span_is_macro_invocation(src, src_len, span_start, span_end, defs); } @@ -1896,21 +1911,59 @@ static bool cbm_region_inside_callable(uint32_t rs, uint32_t re, const CBMDefArr return false; } +/* cbm_span_is_macro_invocation over a cbm_line_offsets table: offs[k] is where + * 1-based line k+1 starts and offs[nlines] is the end of the source, so a span + * costs two reads instead of a walk from byte 0. Same answer as the walk; falls + * back to it when there is no table. */ +static bool cbm_lines_are_macro_invocation(const char *src, int src_len, const uint32_t *offs, + uint32_t nlines, uint32_t start_line, uint32_t end_line, + const CBMDefArray *defs) { + if (!offs) { + return cbm_span_is_macro_invocation(src, src_len, start_line, end_line, defs); + } + if (!src || src_len <= 0 || !defs || start_line == 0 || end_line < start_line || + start_line > nlines) { + return false; + } + uint32_t last = end_line < nlines ? end_line : nlines; + return cbm_byte_span_is_macro_invocation(src, src_len, (int)offs[start_line - 1], + (int)offs[last], defs); +} + +/* #1071 subtraction. Both questions are pure, so asking the cheap one first + * changes no answer: "is the region inside a function?" reads only the + * definition list, while "is it a macro call?" has to find the region's lines + * in the source. A region no function encloses — every region of a file with + * no functions, such as a SQL data dump — never touches the source, and the + * ones that do share one line table built on first use. The old order walked + * the source from byte 0 for every region: regions x file bytes (#1735). */ static void cbm_subtract_macro_invocation_regions(cbm_error_regions_t *regs, const CBMDefArray *defs, const char *src, int src_len) { + uint32_t *offs = NULL; + uint32_t nlines = 0; + bool offs_built = false; int kept = 0; for (int i = 0; i < regs->count; i++) { - bool benign = - cbm_span_is_macro_invocation(src, src_len, regs->starts[i], regs->ends[i], defs) && - cbm_region_inside_callable(regs->starts[i], regs->ends[i], defs); + uint32_t rs = regs->starts[i]; + uint32_t re = regs->ends[i]; + bool benign = false; + if (cbm_region_inside_callable(rs, re, defs)) { + if (!offs_built) { + offs = cbm_line_offsets(src, src_len, &nlines); + offs_built = true; + CBM_MACRO_LINE_SCAN(src_len); + } + benign = cbm_lines_are_macro_invocation(src, src_len, offs, nlines, rs, re, defs); + } if (!benign) { - regs->starts[kept] = regs->starts[i]; - regs->ends[kept] = regs->ends[i]; + regs->starts[kept] = rs; + regs->ends[kept] = re; kept++; } } regs->count = kept; + cbm_free(CBM_MEM_CLASS_EXTRACT, offs); } /* Push [start, end] after trimming no-code lines off both ends. A run made diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index a00192a188..1b5d3f3ef9 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -800,6 +800,12 @@ void cbm_free_tree(CBMFileResult *result); // Free a standalone TSTree pointer (for Go layer cleanup). void cbm_free_tree_ptr(TSTree *tree); +#ifdef CBM_ENABLE_TEST_SEAMS +// Test-only: source bytes this thread has read to locate lines for the #1071 +// macro-invocation check, cumulative (#1735). +uint64_t cbm_test_macro_line_scan_bytes(void); +#endif + // Reset the thread-local parser's internal state, releasing slab-allocated // subtrees. Must be called BEFORE cbm_slab_reset_thread() so the slab rebuild // doesn't corrupt live parser state. diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 1e6506ed1a..77c6b47561 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -302,6 +302,58 @@ TEST(extract_cpp_real_in_body_error_still_flagged_issue1071) { PASS(); } +/* #1735: the #1071 check's cost. cbm_subtract_macro_invocation_regions asked + * "is this region a macro call?" before "is it inside a function?", and the + * first question walked the source from byte 0 for every region: regions x + * file bytes on any file with many error regions, whatever its language. + * Counted (test seam), never timed. */ +enum { MACRO_SCAN_SRC_CAP = 8192 }; + +TEST(macro_check_reads_no_source_for_regions_outside_functions_issue1735) { + /* 40 functions, each followed by a top-level junk line: 40 regions, none + * of them inside a function. */ + char src[MACRO_SCAN_SRC_CAP]; + int len = snprintf(src, sizeof(src), "#define ALLOC(T, n) ((T *)malloc(sizeof(T) * (n)))\n"); + for (int i = 0; i < 40; i++) { + len += snprintf(src + len, sizeof(src) - (size_t)len, + "int ok%d(void) {\n return %d;\n}\n} ] junk ( {\n", i, i); + } + ASSERT_LT(len, MACRO_SCAN_SRC_CAP); + uint64_t before = cbm_test_macro_line_scan_bytes(); + CBMFileResult *r = extract(src, CBM_LANG_C, "t", "junk.c"); + uint64_t scanned = cbm_test_macro_line_scan_bytes() - before; + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); /* top-level junk stays reported... */ + ASSERT_EQ(r->error_region_count, 40); /* ...one range per junk line, as before */ + ASSERT_EQ(scanned, 0u); + cbm_free_result(r); + PASS(); +} + +TEST(macro_check_locates_lines_with_one_table_issue1735) { + /* 60 functions whose bodies hold a real syntax error: every region sits + * inside a function, so the check has to look at each one's lines. */ + char src[MACRO_SCAN_SRC_CAP]; + int len = snprintf(src, sizeof(src), "#define ALLOC(T, n) ((T *)malloc(sizeof(T) * (n)))\n"); + for (int i = 0; i < 60; i++) { + len += snprintf(src + len, sizeof(src) - (size_t)len, + "int f%d(void) {\n int x = ;\n return x;\n}\n", i); + } + ASSERT_LT(len, MACRO_SCAN_SRC_CAP); + uint64_t before = cbm_test_macro_line_scan_bytes(); + CBMFileResult *r = extract(src, CBM_LANG_C, "t", "inbody.c"); + uint64_t scanned = cbm_test_macro_line_scan_bytes() - before; + ASSERT_NOT_NULL(r); + /* #1071 unchanged: a real in-body error is not a macro call and stays + * reported, one range per function. */ + ASSERT_TRUE(r->parse_incomplete); + ASSERT_EQ(r->error_region_count, 60); + /* One line table for the file, not one walk per region. */ + ASSERT_LTE(scanned, (uint64_t)len); + cbm_free_result(r); + PASS(); +} + /* --- GDScript: AST -> graph visitor (Godot, #186) --- */ TEST(extract_gdscript_issue186) { CBMFileResult *r = extract("extends Node\n" @@ -8325,6 +8377,8 @@ SUITE(extraction) { RUN_TEST(extract_cpp_macros_issue375); RUN_TEST(extract_cpp_functionlike_macro_type_arg_no_false_parse_partial_issue1071); RUN_TEST(extract_cpp_real_in_body_error_still_flagged_issue1071); + RUN_TEST(macro_check_reads_no_source_for_regions_outside_functions_issue1735); + RUN_TEST(macro_check_locates_lines_with_one_table_issue1735); RUN_TEST(extract_gdscript_issue186); RUN_TEST(extract_powershell_issue35); RUN_TEST(extract_luau_issue39); From c0a06b3d0a90513e0d80f79514b869f91cb958f1 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 25 Sep 2026 18:49:37 +0200 Subject: [PATCH 04/10] fix(sql): keep literal INSERT rows out of the SQL parse (#1735) A mysqldump-style .sql file is a few CREATE TABLEs followed by megabytes of `INSERT ... VALUES (..),(..),...` rows. Tree-sitter built a full tree for every row (~70 bytes of tree per source byte; 0.26 s/MB with MySQL \' escapes, which the grammar does not know and error-recovers from) until the 5 s parse budget ran out, and the whole file, tables included, was skipped as "parse timeout". The reporter's database/sql/world.sql was dropped this way. The rows contribute nothing to the graph. A linear, quote- and comment-aware pre-scan (internal/cbm/sql_values.c) finds INSERT/REPLACE ... VALUES lists and excludes every tuple after the first that consists only of literals: strings (standard '' and MySQL \' escapes, X'..'/B'..'/N'.., _charset introducers), numbers, NULL, TRUE, FALSE, DEFAULT. A tuple holding a subquery, a function call, an identifier or a quoted identifier stays in the parse, and so does one whose string spans a raw newline. The excluded bytes go to ts_parser_set_included_ranges as the complement, so byte offsets and line/column positions of all kept text are unchanged. The ranges are cleared right after the parse because the parser is thread-local and reused. This is a syntax rule, not a work cap: the same file always gives the same ranges, and the parse budget is unchanged. Measured on generated mysqldump-shaped dumps, release build, same machine, index worker's own numbers (base = origin/main): dump extraction pipeline peak RSS graph 8 MB std '' 1310 -> 23 ms 1431 -> 139 ms 625 -> 37 MB identical 8 MB MySQL \' 2712 -> 24 ms 2827 -> 146 ms 474 -> 37 MB identical 32 MB MySQL \' 5145 -> 82 ms 5467 -> 396 ms 1132 -> 62 MB timeout -> indexed 59 MB MySQL \' 5130 -> 138 ms 5667 -> 674 ms 1157 -> 89 MB timeout -> indexed "identical" = every stored node and edge, Module end line included. For the 32/59 MB dumps the base run skipped the file as "parse timeout" (Module + 3 Tables + their DEFINES/USAGE missing). The fixed graph matches the 8 MB graph except for the Module's end line. With standard escapes the extraction and the stored graph are identical to the full parse. With MySQL \' escapes the full parse's error recovery swallows rows next to a misread escape, a subquery row among them, and the cut parse keeps those rows. So it loses nothing the full parse found outside the dropped rows and recovers what the full parse lost. The only things the full parse extracted INSIDE the dropped rows were junk usages (DEFAULT and _binary read as identifiers, fragments of misread strings) that name nothing. Tests (parse_coverage, pipeline; a CBM_TEST_SQL_FULL_PARSE_ON seam turns the exclusion off for one file so the same source can be compared both ways): - scanner cases: comments, strings, dollar quotes, ON DUPLICATE KEY UPDATE, VALUES outside INSERT, unclosed tuples, exact kept positions - extraction equals the full parse on a small dump (standard escapes); nothing lost outside dropped rows (MySQL escapes) - the subquery and function-call rows are still parsed - the tree does not grow when the row count doubles (a count, not a clock) - a 25 MB MySQL dump under the production budget is indexed, not "parse timeout" (asserts the outcome, not the time) - stored nodes and edges of a pipeline index equal the full parse's Refs #1735 Signed-off-by: Martin Vogel --- Makefile.cbm | 1 + internal/cbm/cbm.c | 28 +++ internal/cbm/sql_values.c | 474 ++++++++++++++++++++++++++++++++++++ internal/cbm/sql_values.h | 40 +++ tests/test_parse_coverage.c | 448 ++++++++++++++++++++++++++++++++++ tests/test_pipeline.c | 160 ++++++++++++ 6 files changed, 1151 insertions(+) create mode 100644 internal/cbm/sql_values.c create mode 100644 internal/cbm/sql_values.h diff --git a/Makefile.cbm b/Makefile.cbm index 7a3cd31e16..368fce973e 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -348,6 +348,7 @@ EXTRACTION_SRCS = \ $(CBM_DIR)/extract_channels.c \ $(CBM_DIR)/extract_k8s.c \ $(CBM_DIR)/extract_dbt.c \ + $(CBM_DIR)/sql_values.c \ $(CBM_DIR)/helpers.c \ $(CBM_DIR)/result_compact.c \ $(CBM_DIR)/result_spill.c \ diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index a6fa8fd320..082bca1d6d 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -21,6 +21,7 @@ #include "lsp/kotlin_lsp.h" #include "lsp/rust_lsp.h" #include "preprocessor.h" +#include "sql_values.h" // #1735: literal INSERT rows kept out of the SQL parse #include "foundation/compat.h" #include "foundation/compat_fs.h" // cbm_fopen — crash-supervisor per-file marker write #include "foundation/hash_table.h" // CBMHashTable — crash-supervisor quarantine set @@ -2125,6 +2126,21 @@ CBMFileResult *cbm_extract_file(const char *source, int source_len, CBMLanguage enum { CBM_EXTRACT_SCRATCH_BLOCK = CBM_SZ_512 * CBM_SZ_1K }; enum { CBM_EXTRACT_SCRATCH_KEEP_BYTES = 4 * CBM_SZ_1K * CBM_SZ_1K }; +/* The #1735 value-row exclusion is always on. Test builds can turn it off for + * one file (CBM_TEST_SQL_FULL_PARSE_ON=) so a test can + * compare the excluded parse against the full one on the same source. */ +static bool cbm_sql_values_exclusion_on(const char *rel_path) { +#ifdef CBM_ENABLE_TEST_SEAMS + const char *full_on = getenv("CBM_TEST_SQL_FULL_PARSE_ON"); + if (full_on && full_on[0] && rel_path && strstr(rel_path, full_on)) { + return false; + } +#else + (void)rel_path; +#endif + return true; +} + static CBMFileResult *extract_file_ex_body(const char *source, int source_len, CBMLanguage language, const char *project, const char *rel_path, int64_t timeout_micros, const char **extra_defines, @@ -2233,7 +2249,19 @@ static CBMFileResult *extract_file_ex_body(const char *source, int source_len, C #endif } + /* #1735: a SQL data dump's literal-only INSERT rows carry no graph content + * but dominate its parse. Keep them out through included ranges; offsets + * and positions of everything kept are unchanged. The parser is + * thread-local and reused, so the ranges are cleared right after. */ + CBMSqlKeptRanges sql_kept = {NULL, 0}; + bool sql_ranged = language == CBM_LANG_SQL && cbm_sql_values_exclusion_on(rel_path) && + cbm_sql_values_kept_ranges(source, (uint32_t)source_len, &sql_kept) && + ts_parser_set_included_ranges(parser, sql_kept.items, sql_kept.count); TSTree *tree = ts_parser_parse_with_options(parser, NULL, ts_input, opts); + if (sql_ranged) { + (void)ts_parser_set_included_ranges(parser, NULL, 0); + } + cbm_sql_kept_ranges_free(&sql_kept); uint64_t t1 = now_ns(); #ifdef CBM_ENABLE_TEST_SEAMS t1 += tl_parse_wall_seam_offset_ns; /* the stall seam inflates every wall reading */ diff --git a/internal/cbm/sql_values.c b/internal/cbm/sql_values.c new file mode 100644 index 0000000000..21d2866896 --- /dev/null +++ b/internal/cbm/sql_values.c @@ -0,0 +1,474 @@ +// sql_values.c — exclude literal-only INSERT value tuples from the SQL parse (#1735). +// +// See sql_values.h for why. The scanner is one forward pass over the file. It +// tracks enough of SQL's lexical structure to never mistake the inside of a +// string, quoted identifier or comment for syntax: +// - '...' strings with both the standard '' escape and MySQL's backslash escape +// - "..." and `...` quoted identifiers, $tag$...$tag$ dollar-quoted bodies +// - -- and # line comments, /* */ block comments +// and it knows only one statement shape: INSERT/REPLACE ... VALUES (..), (..). +// +// Everything it is unsure about stays in the parse. A tuple is excluded only +// when every value in it is a plain literal and the tuple closes cleanly; a +// string holding a raw newline also keeps its tuple, because a dump writes line +// breaks as \n and a raw one means the scan's idea of where strings end may not +// match the grammar's. + +#include "sql_values.h" +#include "foundation/mem_core.h" +#include +#include +#include +#include + +typedef struct { + const char *s; + uint32_t len; + uint32_t i; + uint32_t row; + uint32_t bol; /* byte offset where the current row starts */ +} sqlv_scan_t; + +typedef struct { + TSRange *items; + uint32_t count; + uint32_t cap; + bool failed; +} sqlv_ranges_t; + +enum { SQLV_INITIAL_CAP = 64, SQLV_GROWTH = 2 }; + +static char sqlv_at(const sqlv_scan_t *sc, uint32_t off) { + return sc->i + off < sc->len ? sc->s[sc->i + off] : '\0'; +} + +static void sqlv_step(sqlv_scan_t *sc) { + if (sc->s[sc->i] == '\n') { + sc->row++; + sc->bol = sc->i + 1; + } + sc->i++; +} + +static TSPoint sqlv_point(const sqlv_scan_t *sc) { + return (TSPoint){sc->row, sc->i - sc->bol}; +} + +static bool sqlv_word_char(char c) { + return isalnum((unsigned char)c) || c == '_' || c == '$' || (unsigned char)c >= 0x80; +} + +static bool sqlv_word_is(const char *w, uint32_t n, const char *kw) { + return strlen(kw) == n && strncasecmp(w, kw, n) == 0; +} + +/* Consume a '..', "..", or `..` run starting at the opening quote. Doubled + * quotes escape everywhere; a backslash escapes in strings and "..". Sets + * *raw_newline when the run crosses a line break. False if it never closes. */ +static bool sqlv_skip_quoted(sqlv_scan_t *sc, bool *raw_newline) { + char q = sc->s[sc->i]; + sqlv_step(sc); + while (sc->i < sc->len) { + char c = sc->s[sc->i]; + if (c == '\n') { + *raw_newline = true; + } + if (c == '\\' && q != '`' && sc->i + 1 < sc->len) { + sqlv_step(sc); + sqlv_step(sc); + continue; + } + sqlv_step(sc); + if (c == q) { + if (sqlv_at(sc, 0) != q) { + return true; + } + sqlv_step(sc); /* doubled quote: an escaped quote character */ + } + } + return false; +} + +/* $tag$ ... $tag$ (PostgreSQL). Returns false, consuming nothing, when the `$` + * does not open a dollar quote ($1 parameters, a lone $). */ +static bool sqlv_skip_dollar(sqlv_scan_t *sc) { + uint32_t t = sc->i + 1; + if (t < sc->len && isdigit((unsigned char)sc->s[t])) { + return false; + } + while (t < sc->len && sc->s[t] != '$' && sqlv_word_char(sc->s[t])) { + t++; + } + if (t >= sc->len || sc->s[t] != '$') { + return false; + } + uint32_t tag_len = t - sc->i + 1; /* the opener, both dollars included */ + const char *tag = sc->s + sc->i; + for (uint32_t k = 0; k < tag_len; k++) { + sqlv_step(sc); + } + while (sc->i < sc->len) { + if (sc->s[sc->i] == '$' && sc->len - sc->i >= tag_len && + memcmp(sc->s + sc->i, tag, tag_len) == 0) { + for (uint32_t k = 0; k < tag_len; k++) { + sqlv_step(sc); + } + return true; + } + sqlv_step(sc); + } + return true; +} + +/* Consume one comment starting at the cursor; false if there is none. */ +static bool sqlv_skip_comment(sqlv_scan_t *sc) { + char c = sqlv_at(sc, 0); + char n = sqlv_at(sc, 1); + if ((c == '-' && n == '-') || c == '#') { + while (sc->i < sc->len && sc->s[sc->i] != '\n') { + sqlv_step(sc); + } + return true; + } + if (c == '/' && n == '*') { + sqlv_step(sc); + sqlv_step(sc); + while (sc->i < sc->len && !(sc->s[sc->i] == '*' && sqlv_at(sc, 1) == '/')) { + sqlv_step(sc); + } + if (sc->i < sc->len) { + sqlv_step(sc); + sqlv_step(sc); + } + return true; + } + return false; +} + +/* Skip whitespace and comments. *ws_only (optional) is cleared if a comment was + * among them. */ +static void sqlv_skip_trivia(sqlv_scan_t *sc, bool *ws_only) { + while (sc->i < sc->len) { + if (isspace((unsigned char)sc->s[sc->i])) { + sqlv_step(sc); + } else if (sqlv_skip_comment(sc)) { + if (ws_only) { + *ws_only = false; + } + } else { + return; + } + } +} + +/* Consume one quoted or dollar-quoted run if one starts here. */ +static bool sqlv_skip_any_quoted(sqlv_scan_t *sc) { + char c = sc->s[sc->i]; + bool nl = false; + if (c == '\'' || c == '"' || c == '`') { + (void)sqlv_skip_quoted(sc, &nl); + return true; + } + return c == '$' && sqlv_skip_dollar(sc); +} + +/* From inside an open parenthesis, consume through its matching ')'. */ +static bool sqlv_skip_balanced(sqlv_scan_t *sc) { + int depth = 1; + while (sc->i < sc->len) { + if (sqlv_skip_comment(sc) || sqlv_skip_any_quoted(sc)) { + continue; + } + char c = sc->s[sc->i]; + sqlv_step(sc); + if (c == '(') { + depth++; + } else if (c == ')' && --depth == 0) { + return true; + } + } + return false; +} + +static bool sqlv_read_string(sqlv_scan_t *sc) { + bool nl = false; + return sqlv_skip_quoted(sc, &nl) && !nl; +} + +/* 12, -3.5, 1e-9, .5, 0x1F, 0b101 — followed by a non-word character. */ +static bool sqlv_read_number(sqlv_scan_t *sc) { + char n = sqlv_at(sc, 1); + if (sqlv_at(sc, 0) == '0' && (n == 'x' || n == 'X') && + isxdigit((unsigned char)sqlv_at(sc, 2))) { + sqlv_step(sc); + sqlv_step(sc); + while (isxdigit((unsigned char)sqlv_at(sc, 0))) { + sqlv_step(sc); + } + } else if (sqlv_at(sc, 0) == '0' && (n == 'b' || n == 'B') && + (sqlv_at(sc, 2) == '0' || sqlv_at(sc, 2) == '1')) { + sqlv_step(sc); + sqlv_step(sc); + while (sqlv_at(sc, 0) == '0' || sqlv_at(sc, 0) == '1') { + sqlv_step(sc); + } + } else { + while (isdigit((unsigned char)sqlv_at(sc, 0)) || sqlv_at(sc, 0) == '.') { + sqlv_step(sc); + } + char e = sqlv_at(sc, 0); + char s1 = sqlv_at(sc, 1); + if ((e == 'e' || e == 'E') && + (isdigit((unsigned char)s1) || + ((s1 == '+' || s1 == '-') && isdigit((unsigned char)sqlv_at(sc, 2))))) { + sqlv_step(sc); + sqlv_step(sc); + while (isdigit((unsigned char)sqlv_at(sc, 0))) { + sqlv_step(sc); + } + } + } + return !sqlv_word_char(sqlv_at(sc, 0)) && sqlv_at(sc, 0) != '.'; +} + +/* A word in value position: NULL/TRUE/FALSE/DEFAULT, an X'..' / B'..' / N'..' + * prefixed string, or a _charset introducer before a string or number. */ +static bool sqlv_read_word_literal(sqlv_scan_t *sc) { + const char *w = sc->s + sc->i; + uint32_t n = 0; + while (sqlv_word_char(sqlv_at(sc, 0))) { + sqlv_step(sc); + n++; + } + if (sqlv_word_is(w, n, "NULL") || sqlv_word_is(w, n, "TRUE") || sqlv_word_is(w, n, "FALSE") || + sqlv_word_is(w, n, "DEFAULT")) { + return true; + } + if (n == 1 && strchr("xXbBnN", w[0]) && sqlv_at(sc, 0) == '\'') { + return sqlv_read_string(sc); + } + if (n > 1 && w[0] == '_') { + sqlv_skip_trivia(sc, NULL); + if (sqlv_at(sc, 0) == '\'') { + return sqlv_read_string(sc); + } + return isdigit((unsigned char)sqlv_at(sc, 0)) && sqlv_read_number(sc); + } + return false; +} + +/* Consume one value if it is a plain literal. Never consumes a parenthesis, so + * on false the caller can still find the tuple's end from the cursor. */ +static bool sqlv_read_literal(sqlv_scan_t *sc) { + char c = sqlv_at(sc, 0); + char n = sqlv_at(sc, 1); + if (c == '\'') { + return sqlv_read_string(sc); + } + if ((c == '-' || c == '+') && + (isdigit((unsigned char)n) || (n == '.' && isdigit((unsigned char)sqlv_at(sc, 2))))) { + sqlv_step(sc); + return sqlv_read_number(sc); + } + if (isdigit((unsigned char)c) || (c == '.' && isdigit((unsigned char)n))) { + return sqlv_read_number(sc); + } + if (isalpha((unsigned char)c) || c == '_') { + return sqlv_read_word_literal(sc); + } + return false; +} + +/* Consume one tuple starting at its '('. *literal says whether every value in + * it was a plain literal. Returns false if the tuple never closes. */ +static bool sqlv_scan_tuple(sqlv_scan_t *sc, bool *literal) { + sqlv_step(sc); + sqlv_skip_trivia(sc, NULL); + if (sqlv_at(sc, 0) == ')') { + sqlv_step(sc); + *literal = true; + return true; + } + while (sqlv_read_literal(sc)) { + sqlv_skip_trivia(sc, NULL); + char c = sqlv_at(sc, 0); + if (c == ')') { + sqlv_step(sc); + *literal = true; + return true; + } + if (c != ',') { + break; + } + sqlv_step(sc); + sqlv_skip_trivia(sc, NULL); + } + *literal = false; + return sqlv_skip_balanced(sc); +} + +static void sqlv_push(sqlv_ranges_t *r, TSRange range) { + if (r->failed) { + return; + } + if (r->count == r->cap) { + uint32_t cap = r->cap ? r->cap * SQLV_GROWTH : SQLV_INITIAL_CAP; + TSRange *grown = + (TSRange *)cbm_realloc(CBM_MEM_CLASS_EXTRACT, r->items, (size_t)cap * sizeof(TSRange)); + if (!grown) { + r->failed = true; + return; + } + r->items = grown; + r->cap = cap; + } + r->items[r->count++] = range; +} + +/* The cursor sits just past VALUES. Keep the first tuple; exclude each later + * literal-only tuple together with the comma before it. Runs of excluded tuples + * separated only by whitespace become one exclusion. */ +static void sqlv_handle_values(sqlv_scan_t *sc, sqlv_ranges_t *ex) { + sqlv_skip_trivia(sc, NULL); + bool literal = false; + if (sqlv_at(sc, 0) != '(' || !sqlv_scan_tuple(sc, &literal)) { + return; + } + bool pending = false; + TSRange cur = {0}; + for (;;) { + bool ws_only = true; + sqlv_skip_trivia(sc, &ws_only); + if (sqlv_at(sc, 0) != ',') { + break; + } + uint32_t comma = sc->i; + TSPoint comma_pt = sqlv_point(sc); + sqlv_step(sc); + sqlv_skip_trivia(sc, NULL); + if (sqlv_at(sc, 0) != '(' || !sqlv_scan_tuple(sc, &literal)) { + break; + } + if (!literal) { + if (pending) { + sqlv_push(ex, cur); + } + pending = false; + continue; + } + if (!pending || !ws_only) { + if (pending) { + sqlv_push(ex, cur); + } + cur.start_byte = comma; + cur.start_point = comma_pt; + pending = true; + } + cur.end_byte = sc->i; + cur.end_point = sqlv_point(sc); + } + if (pending) { + sqlv_push(ex, cur); + } +} + +typedef struct { + bool started; /* a token of the current statement has been seen */ + bool insert; /* the statement began with INSERT or REPLACE */ + int depth; +} sqlv_stmt_t; + +static void sqlv_on_word(sqlv_scan_t *sc, sqlv_stmt_t *st, sqlv_ranges_t *ex) { + const char *w = sc->s + sc->i; + uint32_t n = 0; + while (sc->i < sc->len && sqlv_word_char(sc->s[sc->i])) { + sqlv_step(sc); + n++; + } + if (!st->started) { + st->started = true; + st->insert = sqlv_word_is(w, n, "INSERT") || sqlv_word_is(w, n, "REPLACE"); + } else if (st->insert && st->depth == 0 && + (sqlv_word_is(w, n, "VALUES") || sqlv_word_is(w, n, "VALUE"))) { + sqlv_handle_values(sc, ex); + } +} + +static void sqlv_scan_file(sqlv_scan_t *sc, sqlv_ranges_t *ex) { + sqlv_stmt_t st = {false, false, 0}; + while (sc->i < sc->len && !ex->failed) { + if (sqlv_skip_comment(sc)) { + continue; + } + char c = sc->s[sc->i]; + if (sqlv_skip_any_quoted(sc)) { + st.started = true; + continue; + } + if (sqlv_word_char(c)) { + sqlv_on_word(sc, &st, ex); + continue; + } + if (c == ';') { + st = (sqlv_stmt_t){false, false, 0}; + } else if (c == '(') { + st.depth++; + } else if (c == ')' && st.depth > 0) { + st.depth--; + } + if (!isspace((unsigned char)c) && c != ';') { + st.started = true; + } + sqlv_step(sc); + } +} + +bool cbm_sql_values_kept_ranges(const char *src, uint32_t len, CBMSqlKeptRanges *out) { + out->items = NULL; + out->count = 0; + if (!src || len == 0) { + return false; + } + sqlv_scan_t sc = {src, len, 0, 0, 0}; + sqlv_ranges_t ex = {NULL, 0, 0, false}; + sqlv_scan_file(&sc, &ex); + if (ex.failed || ex.count == 0) { + cbm_free(CBM_MEM_CLASS_EXTRACT, ex.items); + return false; + } + /* Kept = the complement of the exclusions. The scan ended at EOF, so its + * cursor point is the end of the file. */ + TSRange *kept = + (TSRange *)cbm_alloc(CBM_MEM_CLASS_EXTRACT, ((size_t)ex.count + 1) * sizeof(TSRange)); + if (!kept) { + cbm_free(CBM_MEM_CLASS_EXTRACT, ex.items); + return false; + } + uint32_t n = 0; + uint32_t from = 0; + TSPoint from_pt = {0, 0}; + for (uint32_t k = 0; k <= ex.count; k++) { + uint32_t to = k < ex.count ? ex.items[k].start_byte : len; + TSPoint to_pt = k < ex.count ? ex.items[k].start_point : sqlv_point(&sc); + if (to > from) { + kept[n++] = (TSRange){from_pt, to_pt, from, to}; + } + if (k < ex.count) { + from = ex.items[k].end_byte; + from_pt = ex.items[k].end_point; + } + } + cbm_free(CBM_MEM_CLASS_EXTRACT, ex.items); + out->items = kept; + out->count = n; + return true; +} + +void cbm_sql_kept_ranges_free(CBMSqlKeptRanges *ranges) { + if (!ranges) { + return; + } + cbm_free(CBM_MEM_CLASS_EXTRACT, ranges->items); + ranges->items = NULL; + ranges->count = 0; +} diff --git a/internal/cbm/sql_values.h b/internal/cbm/sql_values.h new file mode 100644 index 0000000000..5960ec3201 --- /dev/null +++ b/internal/cbm/sql_values.h @@ -0,0 +1,40 @@ +/* sql_values.h — keep a SQL data dump's literal rows out of the parse (#1735). + * + * A mysqldump-style file is almost entirely `INSERT ... VALUES (..),(..),...` + * rows of plain literals. Those rows contribute nothing to the graph (no + * definition, call or usage lives in a literal), yet tree-sitter builds a full + * tree for every one of them: ~70 bytes of tree per source byte and seconds of + * parse per ten megabytes, until the per-file budget gives up and the whole file + * — its CREATE TABLE statements included — is skipped as "parse timeout". + * + * cbm_sql_values_kept_ranges() finds, in one linear quote- and comment-aware + * scan, every value tuple AFTER THE FIRST of an INSERT/REPLACE ... VALUES list + * that consists only of literals (strings, numbers, NULL, TRUE, FALSE, DEFAULT), + * and returns the complement as tree-sitter included ranges. The parser then + * sees `INSERT INTO t VALUES (first row);` — a complete statement — while every + * byte offset and line/column of the kept text is unchanged, so every node the + * extractors read still points at the real source. + * + * A syntax rule, not a work cap: the same file always yields the same ranges, + * whatever its size. A tuple holding anything that could reference code — a + * subquery, a function call, an identifier, a quoted identifier — is kept. */ +#ifndef CBM_SQL_VALUES_H +#define CBM_SQL_VALUES_H + +#include "tree_sitter/api.h" +#include +#include + +typedef struct { + TSRange *items; /* ascending, non-overlapping; owned (cbm_sql_kept_ranges_free) */ + uint32_t count; +} CBMSqlKeptRanges; + +/* Compute the included ranges for `src`. Returns true and fills `out` only when + * at least one literal tuple was excluded; returns false (out zeroed) when the + * whole file must be parsed as-is or on allocation failure. */ +bool cbm_sql_values_kept_ranges(const char *src, uint32_t len, CBMSqlKeptRanges *out); + +void cbm_sql_kept_ranges_free(CBMSqlKeptRanges *ranges); + +#endif /* CBM_SQL_VALUES_H */ diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index fd0e64e478..91f99f95d3 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -30,6 +30,9 @@ #include "test_framework.h" #include "cbm.h" +#include "sql_values.h" /* #1735 value-row scanner */ +#include "pipeline/pipeline_internal.h" /* CBM_EXTRACT_BUDGET */ +#include #include #include #include @@ -983,6 +986,445 @@ TEST(coverage_gap_of_only_comments_is_not_a_miss) { PASS(); } +/* ── #1735: SQL data dumps ─────────────────────────────────────────────────── + * A mysqldump file is a few CREATE TABLEs followed by megabytes of literal + * INSERT rows. Tree-sitter built a full tree for every row until the parse + * budget ran out, and the whole file — tables included — was skipped as + * "parse timeout". Literal-only rows after the first of each VALUES list are + * now kept out of the parse (sql_values.c). These tests pin that nothing the + * full parse extracted outside those rows is lost (and, for standard '' escapes + * the grammar reads correctly, that nothing changes at all), that a row which + * can reference something is still parsed, and that the parse no longer grows + * with the row count. */ + +typedef struct { + char *s; + size_t len; + size_t cap; +} cov_buf_t; + +static void cov_put(cov_buf_t *b, const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + va_list ap2; + va_copy(ap2, ap); + int n = vsnprintf(NULL, 0, fmt, ap); + va_end(ap); + if (n < 0) { + va_end(ap2); + return; + } + if (b->len + (size_t)n + 1 > b->cap) { + size_t cap = b->cap ? b->cap : 4096; + while (b->len + (size_t)n + 1 > cap) { + cap *= 2; + } + char *grown = realloc(b->s, cap); + if (!grown) { + va_end(ap2); + return; + } + b->s = grown; + b->cap = cap; + } + vsnprintf(b->s + b->len, (size_t)n + 1, fmt, ap2); + va_end(ap2); + b->len += (size_t)n; +} + +static const char *const DUMP_WORDS[] = {"Kabul", "Herat", "Amsterdam", "O'Brien", + "Haag", "Rotterdam", "Zuid-Holland", "Utrecht"}; +enum { DUMP_WORD_COUNT = sizeof(DUMP_WORDS) / sizeof(DUMP_WORDS[0]) }; +static const char *const DUMP_ODD[] = {"NULL", "-12.5", "0x1F", "_binary 'ab'", "DEFAULT", + "TRUE", "1e-3", "X'0A'", "''", "'a''b'"}; +enum { DUMP_ODD_COUNT = sizeof(DUMP_ODD) / sizeof(DUMP_ODD[0]) }; + +/* Deterministic mysqldump-shaped SQL: two tables and a view, then `stmts` + * extended INSERTs of `rows` tuples. `mysql_esc` writes a quote inside a string + * as \' (MySQL) instead of '' (standard). `mixed` puts one tuple holding a + * subquery and one holding a function call in the middle of every INSERT. The + * first two INSERTs separate their tuples with a newline and with a comment, the + * way pretty-printed dumps do. */ +static char *sql_dump(int stmts, int rows, bool mysql_esc, bool mixed) { + cov_buf_t b = {NULL, 0, 0}; + cov_put(&b, "-- MySQL dump 10.13\n" + "/*!40101 SET NAMES utf8mb4 */;\n" + "CREATE TABLE `city` (\n" + " `ID` int NOT NULL,\n" + " `Name` char(35) NOT NULL DEFAULT '',\n" + " `CountryCode` char(3),\n" + " `Note` text,\n" + " `Population` int\n" + ");\n" + "CREATE TABLE country (Code char(3), Name char(52));\n" + "CREATE VIEW big_cities AS SELECT Name FROM city WHERE Population > 1000000;\n" + "LOCK TABLES `city` WRITE;\n"); + uint32_t seed = 1735; + int id = 1; + for (int s = 0; s < stmts; s++) { + cov_put(&b, "INSERT INTO `city` VALUES "); + const char *sep = s == 0 ? ",\n" : (s == 1 ? ", /* split */ " : ","); + for (int r = 0; r < rows; r++) { + if (r > 0) { + cov_put(&b, "%s", sep); + } + if (mixed && r == rows / 2) { + cov_put(&b, "((SELECT MAX(Code) FROM country),'x','AAA',NULL,1)"); + continue; + } + if (mixed && r == rows / 2 + 1) { + cov_put(&b, "(%d,UPPER('y'),'BBB',NULL,2)", id++); + continue; + } + seed = seed * 1103515245u + 12345u; + const char *w = DUMP_WORDS[(seed >> 8) % DUMP_WORD_COUNT]; + cov_put(&b, "(%d,'", id++); + for (const char *p = w; *p; p++) { + cov_put(&b, *p == '\'' ? (mysql_esc ? "\\'" : "''") : "%c", *p); + } + cov_put(&b, "','%c%c%c',%s,%u)", 'A' + (int)(seed % 26), 'A' + (int)((seed >> 5) % 26), + 'A' + (int)((seed >> 10) % 26), DUMP_ODD[(seed >> 12) % DUMP_ODD_COUNT], + (seed >> 3) % 9000000u); + } + cov_put(&b, ";\n"); + } + cov_put(&b, "UNLOCK TABLES;\n"); + return b.s; +} + +static void fp_s(cov_buf_t *b, const char *s) { + cov_put(b, "%s|", s ? s : "~"); +} + +/* Everything the graph is built from except calls and usages, in order (those + * two are compared site by site, sites_agree). */ +static char *result_fingerprint(const CBMFileResult *r) { + cov_buf_t b = {NULL, 0, 0}; + for (int i = 0; i < r->defs.count; i++) { + const CBMDefinition *d = &r->defs.items[i]; + fp_s(&b, d->label); + fp_s(&b, d->name); + fp_s(&b, d->qualified_name); + fp_s(&b, d->signature); + fp_s(&b, d->return_type); + fp_s(&b, d->structural_profile); + fp_s(&b, d->body_tokens); + cov_put(&b, "%u-%u c%d l%d\n", d->start_line, d->end_line, d->complexity, d->lines); + } + for (int i = 0; i < r->imports.count; i++) { + fp_s(&b, r->imports.items[i].local_name); + fp_s(&b, r->imports.items[i].module_path); + cov_put(&b, "imp\n"); + } + for (int i = 0; i < r->rw.count; i++) { + fp_s(&b, r->rw.items[i].var_name); + cov_put(&b, "rw%d\n", (int)r->rw.items[i].is_write); + } + for (int i = 0; i < r->type_refs.count; i++) { + fp_s(&b, r->type_refs.items[i].type_name); + cov_put(&b, "tref\n"); + } + for (int i = 0; i < r->env_accesses.count; i++) { + fp_s(&b, r->env_accesses.items[i].env_key); + cov_put(&b, "env\n"); + } + for (int i = 0; i < r->throws.count; i++) { + fp_s(&b, r->throws.items[i].exception_name); + cov_put(&b, "throw\n"); + } + for (int i = 0; i < r->string_refs.count; i++) { + fp_s(&b, r->string_refs.items[i].value); + cov_put(&b, "sref%d\n", (int)r->string_refs.items[i].kind); + } + cov_put(&b, "ta%d it%d rc%d ib%d ch%d\n", r->type_assigns.count, r->impl_traits.count, + r->resolved_calls.count, r->infra_bindings.count, r->channels.count); + return b.s; +} + +static int has_usage_named(const CBMFileResult *r, const char *name) { + for (int i = 0; i < r->usages.count; i++) { + if (r->usages.items[i].ref_name && strstr(r->usages.items[i].ref_name, name)) { + return 1; + } + } + return 0; +} + +static int has_call_named(const CBMFileResult *r, const char *name) { + for (int i = 0; i < r->calls.count; i++) { + if (r->calls.items[i].callee_name && strstr(r->calls.items[i].callee_name, name)) { + return 1; + } + } + return 0; +} + +typedef struct { + const char *name; + uint32_t start; + uint32_t end; +} cov_site_t; + +static int collect_sites(const CBMFileResult *r, bool calls, cov_site_t **out) { + int n = calls ? r->calls.count : r->usages.count; + *out = calloc((size_t)(n > 0 ? n : 1), sizeof(cov_site_t)); + for (int i = 0; i < n && *out; i++) { + (*out)[i] = + calls ? (cov_site_t){r->calls.items[i].callee_name, r->calls.items[i].site_start_byte, + r->calls.items[i].site_end_byte} + : (cov_site_t){r->usages.items[i].ref_name, r->usages.items[i].site_start_byte, + r->usages.items[i].site_end_byte}; + } + return *out ? n : 0; +} + +static bool site_in(cov_site_t s, const cov_site_t *arr, int n) { + for (int i = 0; i < n; i++) { + if (arr[i].start == s.start && arr[i].end == s.end && s.name && arr[i].name && + strcmp(arr[i].name, s.name) == 0) { + return true; + } + } + return false; +} + +/* Calls (calls=true) or usages, cut parse against full parse: + * - nothing the full parse found outside the dropped rows may be missing; + * - with `exact`, the cut parse may find nothing the full parse did not. + * The full parse does find things INSIDE dropped rows: the SQL grammar reads + * DEFAULT and a _binary introducer as identifiers, and a MySQL \' escape it + * does not know turns the rest of a string into one. None of those names + * anything. Without `exact` (MySQL escapes) the cut parse may find more: the + * full parse's error recovery around a misread escape swallows neighbouring + * rows, a subquery row among them, and the cut parse no longer misreads them. */ +static bool sites_agree(const char *src, const CBMFileResult *cut, const CBMFileResult *full, + bool calls, bool exact) { + cov_site_t *cs = NULL; + cov_site_t *fs = NULL; + int cn = collect_sites(cut, calls, &cs); + int fn = collect_sites(full, calls, &fs); + CBMSqlKeptRanges k = {NULL, 0}; + (void)cbm_sql_values_kept_ranges(src, (uint32_t)strlen(src), &k); + bool ok = cs && fs; + for (int i = 0; ok && exact && i < cn; i++) { + if (!site_in(cs[i], fs, fn)) { + fprintf(stderr, " %s only in the cut parse\n", cs[i].name); + ok = false; + } + } + for (int i = 0; ok && i < fn; i++) { + if (site_in(fs[i], cs, cn)) { + continue; + } + for (uint32_t j = 0; j < k.count; j++) { + if (fs[i].start < k.items[j].end_byte && fs[i].end > k.items[j].start_byte) { + fprintf(stderr, " %s lost outside the dropped rows\n", fs[i].name); + ok = false; + } + } + } + cbm_sql_kept_ranges_free(&k); + free(cs); + free(fs); + return ok; +} + +static int count_calls_named(const CBMFileResult *r, const char *name) { + int n = 0; + for (int i = 0; i < r->calls.count; i++) { + if (r->calls.items[i].callee_name && strcmp(r->calls.items[i].callee_name, name) == 0) { + n++; + } + } + return n; +} + +/* The same source parsed with the row exclusion and without it (test seam). */ +static CBMFileResult *extract_sql_full(const char *src, const char *path) { + setenv("CBM_TEST_SQL_FULL_PARSE_ON", path, 1); + CBMFileResult *r = do_extract(src, CBM_LANG_SQL, path); + unsetenv("CBM_TEST_SQL_FULL_PARSE_ON"); + return r; +} + +TEST(sql_dump_literal_rows_leave_the_graph_unchanged_issue1735) { + for (int esc = 0; esc < 2; esc++) { + char *src = sql_dump(4, 60, esc == 1, true); + ASSERT_NOT_NULL(src); + CBMFileResult *cut = do_extract(src, CBM_LANG_SQL, "dump.sql"); + CBMFileResult *full = extract_sql_full(src, "dump.sql"); + ASSERT_NOT_NULL(cut); + ASSERT_NOT_NULL(full); + ASSERT_FALSE(cut->has_error); + ASSERT_TRUE(has_def(cut, "big_cities")); + char *fp_cut = result_fingerprint(cut); + char *fp_full = result_fingerprint(full); + ASSERT_NOT_NULL(fp_cut); + ASSERT_NOT_NULL(fp_full); + ASSERT_STR_EQ(fp_cut, fp_full); + ASSERT_TRUE(sites_agree(src, cut, full, true, esc == 0)); + ASSERT_TRUE(sites_agree(src, cut, full, false, esc == 0)); + /* Every INSERT's subquery row is parsed. */ + ASSERT_EQ(count_calls_named(cut, "MAX"), 4); + /* ...and the rows really were left out: under a quarter of the file + * is parsed. */ + CBMSqlKeptRanges k = {NULL, 0}; + ASSERT_TRUE(cbm_sql_values_kept_ranges(src, (uint32_t)strlen(src), &k)); + size_t kept = 0; + for (uint32_t j = 0; j < k.count; j++) { + kept += k.items[j].end_byte - k.items[j].start_byte; + } + cbm_sql_kept_ranges_free(&k); + ASSERT_LT(kept * 4, strlen(src)); + free(fp_cut); + free(fp_full); + cbm_free_result(cut); + cbm_free_result(full); + free(src); + } + PASS(); +} + +TEST(sql_dump_tuple_with_subquery_or_call_is_still_parsed_issue1735) { + char *src = sql_dump(1, 20, true, true); + ASSERT_NOT_NULL(src); + CBMFileResult *r = do_extract(src, CBM_LANG_SQL, "dump.sql"); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(has_usage_named(r, "country")); /* from the subquery row */ + ASSERT_TRUE(has_call_named(r, "UPPER")); /* from the function-call row */ + cbm_free_result(r); + free(src); + PASS(); +} + +TEST(sql_dump_parse_does_not_grow_with_the_row_count_issue1735) { + /* A count, not a clock: with literal rows kept out, doubling them must not + * add a single tree node. */ + char *small = sql_dump(1, 500, true, false); + char *big = sql_dump(1, 1000, true, false); + ASSERT_NOT_NULL(small); + ASSERT_NOT_NULL(big); + CBMFileResult *rs = do_extract(small, CBM_LANG_SQL, "small.sql"); + CBMFileResult *rb = do_extract(big, CBM_LANG_SQL, "big.sql"); + ASSERT_NOT_NULL(rs); + ASSERT_NOT_NULL(rb); + ASSERT_EQ(rs->tree_nodes, rb->tree_nodes); + ASSERT_EQ(rs->defs.count, rb->defs.count); + cbm_free_result(rs); + cbm_free_result(rb); + free(small); + free(big); + PASS(); +} + +TEST(sql_dump_of_many_megabytes_is_indexed_not_timed_out_issue1735) { + /* ~26 MB with MySQL escapes, extracted under the production parse budget. + * The outcome is asserted — the file is indexed and its tables are in the + * graph — never how long it took. */ + char *src = sql_dump(330, 2000, true, true); + ASSERT_NOT_NULL(src); + size_t len = strlen(src); + ASSERT_GT(len, (size_t)24 * 1024 * 1024); + CBMFileResult *r = cbm_extract_file(src, (int)len, CBM_LANG_SQL, "covproj", "world.sql", + CBM_EXTRACT_BUDGET, NULL, NULL); + ASSERT_NOT_NULL(r); + if (r->has_error) { + FAIL(r->error_msg ? r->error_msg : "extraction failed"); + } + ASSERT_TRUE(has_def(r, "big_cities")); + ASSERT_TRUE(has_usage_named(r, "country")); + cbm_free_result(r); + free(src); + PASS(); +} + +/* The text the parser sees: every kept range, concatenated. */ +static char *kept_text(const char *src, const CBMSqlKeptRanges *k) { + cov_buf_t b = {NULL, 0, 0}; + cov_put(&b, "%s", ""); + for (uint32_t i = 0; i < k->count; i++) { + cov_put(&b, "%.*s", (int)(k->items[i].end_byte - k->items[i].start_byte), + src + k->items[i].start_byte); + } + return b.s; +} + +TEST(sql_values_scanner_excludes_only_literal_rows_issue1735) { + /* {source, what the parser sees} — NULL means nothing is excluded. */ + static const char *const cases[][2] = { + {"INSERT INTO t VALUES (1,'a'),(2,'b\\'c'),(3,NULL);", "INSERT INTO t VALUES (1,'a');"}, + {"INSERT INTO t VALUES (1),(f(2)),(3);", "INSERT INTO t VALUES (1),(f(2));"}, + {"insert into t values (1),(-2.5e3),(0x1F),(_binary 'x'),(DEFAULT),(true),(X'0A')," + "('it''s'),(.5),();", + "insert into t values (1);"}, + {"REPLACE INTO t VALUES (1), /* c */ (2), (3);", "REPLACE INTO t VALUES (1);"}, + {"INSERT INTO t VALUES (1),(2) -- c\n,(3);", "INSERT INTO t VALUES (1) -- c\n;"}, + {"INSERT INTO t VALUES (1),((SELECT id FROM u)),(x),(\"q\"),(`b`),(a.b);", NULL}, + {"INSERT INTO t VALUES (1),('a\nb'),(2);", "INSERT INTO t VALUES (1),('a\nb');"}, + {"INSERT INTO t VALUES (1),(2) ON DUPLICATE KEY UPDATE a=VALUES(a);", + "INSERT INTO t VALUES (1) ON DUPLICATE KEY UPDATE a=VALUES(a);"}, + {"INSERT INTO t (a, b) VALUES (1, 2), (3, 4);", "INSERT INTO t (a, b) VALUES (1, 2);"}, + {"SELECT 'INSERT INTO t VALUES (1),(2)';", NULL}, + {"-- INSERT INTO t VALUES (1),(2)\nSELECT 1;", NULL}, + {"/* INSERT INTO t VALUES (1),(2) */ SELECT 1;", NULL}, + {"SELECT * FROM (VALUES (1),(2)) AS v(x);", NULL}, + {"CREATE FUNCTION f() AS $$ SELECT 1; INSERT INTO t VALUES (1),(2); $$;", NULL}, + {"INSERT INTO t VALUES (1),(2", NULL}, + {"INSERT INTO t VALUES (1),(1abc),(2);", "INSERT INTO t VALUES (1),(1abc);"}, + }; + for (size_t c = 0; c < sizeof(cases) / sizeof(cases[0]); c++) { + const char *src = cases[c][0]; + CBMSqlKeptRanges k = {NULL, 0}; + bool cut = cbm_sql_values_kept_ranges(src, (uint32_t)strlen(src), &k); + if (!cases[c][1]) { + if (cut) { + fprintf(stderr, " case %zu excluded rows unexpectedly: %s\n", c, src); + } + ASSERT_FALSE(cut); + continue; + } + ASSERT_TRUE(cut); + char *seen = kept_text(src, &k); + ASSERT_NOT_NULL(seen); + ASSERT_STR_EQ(seen, cases[c][1]); + free(seen); + cbm_sql_kept_ranges_free(&k); + } + PASS(); +} + +TEST(sql_values_scanner_keeps_positions_of_kept_text_issue1735) { + const char *src = "INSERT INTO t VALUES\n(1),\n(2);\nCREATE TABLE u (id int);\n"; + CBMSqlKeptRanges k = {NULL, 0}; + ASSERT_TRUE(cbm_sql_values_kept_ranges(src, (uint32_t)strlen(src), &k)); + ASSERT_EQ(k.count, 2u); + /* excluded: ",\n(2)" from byte 24 (row 1, col 3) to byte 29 (row 2, col 3) */ + ASSERT_EQ(k.items[0].start_byte, 0u); + ASSERT_EQ(k.items[0].end_byte, 24u); + ASSERT_EQ(k.items[0].end_point.row, 1u); + ASSERT_EQ(k.items[0].end_point.column, 3u); + ASSERT_EQ(k.items[1].start_byte, 29u); + ASSERT_EQ(k.items[1].start_point.row, 2u); + ASSERT_EQ(k.items[1].start_point.column, 3u); + ASSERT_EQ(k.items[1].end_byte, (uint32_t)strlen(src)); + ASSERT_EQ(k.items[1].end_point.row, 4u); + ASSERT_EQ(k.items[1].end_point.column, 0u); + cbm_sql_kept_ranges_free(&k); + /* The table after the cut keeps its real line. */ + CBMFileResult *r = do_extract(src, CBM_LANG_SQL, "pos.sql"); + ASSERT_NOT_NULL(r); + bool found = false; + for (int i = 0; i < r->defs.count; i++) { + if (r->defs.items[i].name && strcmp(r->defs.items[i].name, "u") == 0) { + ASSERT_EQ(r->defs.items[i].start_line, 4u); + found = true; + } + } + ASSERT_TRUE(found); + cbm_free_result(r); + PASS(); +} + SUITE(parse_coverage) { RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); @@ -1023,4 +1465,10 @@ SUITE(parse_coverage) { RUN_TEST(coverage_range_never_ends_past_the_last_line_issue963); RUN_TEST(coverage_range_never_covers_an_extracted_definition); RUN_TEST(coverage_gap_of_only_comments_is_not_a_miss); + RUN_TEST(sql_values_scanner_excludes_only_literal_rows_issue1735); + RUN_TEST(sql_values_scanner_keeps_positions_of_kept_text_issue1735); + RUN_TEST(sql_dump_literal_rows_leave_the_graph_unchanged_issue1735); + RUN_TEST(sql_dump_tuple_with_subquery_or_call_is_still_parsed_issue1735); + RUN_TEST(sql_dump_parse_does_not_grow_with_the_row_count_issue1735); + RUN_TEST(sql_dump_of_many_megabytes_is_indexed_not_timed_out_issue1735); } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 6ef16a89be..6618a4a5db 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -344,6 +344,165 @@ TEST(pipeline_doclinks_edge_lands_in_store) { PASS(); } +/* #1735: every node and edge of a stored index, one sorted line each. */ +static void graph_listing_append(sqlite3 *db, const char *sql, const char *project, char **buf, + size_t *len) { + sqlite3_stmt *stmt = NULL; + if (sqlite3_prepare_v2(db, sql, -1, &stmt, NULL) != SQLITE_OK) { + return; + } + sqlite3_bind_text(stmt, 1, project, -1, SQLITE_TRANSIENT); + while (sqlite3_step(stmt) == SQLITE_ROW) { + const char *row = (const char *)sqlite3_column_text(stmt, 0); + size_t n = row ? strlen(row) : 0; + char *grown = realloc(*buf, *len + n + 2); + if (!grown) { + break; + } + *buf = grown; + memcpy(*buf + *len, row ? row : "", n); + (*buf)[*len + n] = '\n'; + *len += n + 1; + (*buf)[*len] = '\0'; + } + sqlite3_finalize(stmt); +} + +static char *graph_listing(const char *db_path, const char *project) { + sqlite3 *db = NULL; + if (sqlite3_open_v2(db_path, &db, SQLITE_OPEN_READONLY, NULL) != SQLITE_OK) { + sqlite3_close(db); + return NULL; + } + char *buf = calloc(1, 1); + size_t len = 0; + graph_listing_append(db, + "SELECT label||'|'||name||'|'||qualified_name||'|'||file_path||'|'||" + "start_line||'|'||end_line||'|'||properties FROM nodes " + "WHERE project=?1 ORDER BY 1", + project, &buf, &len); + graph_listing_append(db, + "SELECT e.type||'|'||s.qualified_name||'|'||t.qualified_name||'|'||" + "e.properties FROM edges e JOIN nodes s ON s.id=e.source_id " + "JOIN nodes t ON t.id=e.target_id WHERE e.project=?1 ORDER BY 1", + project, &buf, &len); + sqlite3_close(db); + return buf; +} + +static char *index_and_list(const char *repo, const char *db_name) { + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/%s", repo, db_name); + cbm_pipeline_t *p = cbm_pipeline_new(repo, db_path, CBM_MODE_FULL); + if (!p) { + return NULL; + } + char *listing = + cbm_pipeline_run(p) == 0 ? graph_listing(db_path, cbm_pipeline_project_name(p)) : NULL; + cbm_pipeline_free(p); + return listing; +} + +/* True if every line of `sub` is a line of `super`. */ +static bool listing_lines_within(const char *sub, const char *super) { + for (const char *line = sub; *line;) { + const char *nl = strchr(line, '\n'); + size_t n = nl ? (size_t)(nl - line) : strlen(line); + bool found = false; + for (const char *p = super; *p && !found;) { + const char *pnl = strchr(p, '\n'); + size_t pn = pnl ? (size_t)(pnl - p) : strlen(p); + found = pn == n && memcmp(p, line, n) == 0; + p = pnl ? pnl + 1 : p + pn; + } + if (!found) { + fprintf(stderr, " missing from the cut index: %.*s\n", (int)n, line); + return false; + } + line = nl ? nl + 1 : line + n; + } + return true; +} + +/* A small dump: two tables, a view, three INSERTs of 200 rows, each with one + * subquery row and one function-call row among literal rows. */ +static char *sql_dump_small(bool mysql_esc) { + size_t cap = 256 * 1024; + char *src = malloc(cap); + if (!src) { + return NULL; + } + const char *quoted = mysql_esc ? "(%d,'O\\'Brien',DEFAULT)" : "(%d,'O''Brien',DEFAULT)"; + size_t len = (size_t)snprintf(src, cap, + "CREATE TABLE city (ID int, Name char(35), Population int);\n" + "CREATE TABLE country (Code char(3), Name char(52));\n" + "CREATE VIEW big_cities AS SELECT Name FROM city " + "WHERE Population > 1000000;\n"); + for (int s = 0; s < 3; s++) { + len += (size_t)snprintf(src + len, cap - len, "INSERT INTO `city` VALUES "); + for (int r = 0; r < 200; r++) { + if (r) { + src[len++] = ','; + } + int id = s * 1000 + r; + if (r == 100) { + len += (size_t)snprintf(src + len, cap - len, + "((SELECT MAX(Code) FROM country),'x',1)"); + } else if (r == 101) { + len += (size_t)snprintf(src + len, cap - len, "(%d,UPPER('y'),2)", id); + } else if (r % 3) { + len += (size_t)snprintf(src + len, cap - len, quoted, id); + } else { + len += (size_t)snprintf(src + len, cap - len, "(%d,_binary 'ab',NULL)", id); + } + } + len += (size_t)snprintf(src + len, cap - len, ";\n"); + } + return src; +} + +/* #1735: leaving a dump's literal INSERT rows out of the parse must not change + * the graph. The same repository is indexed twice — once as shipped and once + * with the full parse (test seam) — and the stored nodes and edges compared. + * Standard '' escapes, which the SQL grammar reads correctly: identical. + * MySQL \' escapes, which it misreads: the full parse's error recovery swallows + * neighbouring rows, so the cut index must hold everything the full one has + * (and may hold more — the rows the full parse lost). */ +TEST(pipeline_sql_dump_graph_matches_the_full_parse_issue1735) { + for (int esc = 0; esc < 2; esc++) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_sql_dump_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + char *src = sql_dump_small(esc == 1); + ASSERT_NOT_NULL(src); + write_temp_file(tmp, "dump.sql", src); + free(src); + + char *cut = index_and_list(tmp, "cut.db"); + setenv("CBM_TEST_SQL_FULL_PARSE_ON", "dump.sql", 1); + char *full = index_and_list(tmp, "full.db"); + unsetenv("CBM_TEST_SQL_FULL_PARSE_ON"); + th_rmtree(tmp); + ASSERT_NOT_NULL(cut); + ASSERT_NOT_NULL(full); + /* Not vacuous: the tables, the view and its lineage are there. */ + ASSERT_NOT_NULL(strstr(cut, "Table|city|")); + ASSERT_NOT_NULL(strstr(cut, "View|big_cities|")); + ASSERT_NOT_NULL(strstr(cut, "USAGE|")); + if (esc == 0 && strcmp(cut, full) != 0) { + fprintf(stderr, "--- cut ---\n%s--- full ---\n%s", cut, full); + } + if (esc == 0) { + ASSERT_STR_EQ(cut, full); + } else { + ASSERT_TRUE(listing_lines_within(full, cut)); + } + free(cut); + free(full); + } + PASS(); +} + /* Spilling must be invisible in the OUTPUT: the same repository indexed with * results parked on disk must produce the same graph as one indexed entirely in * memory. It did not. The namespace map that `use`/`using`/package imports @@ -15110,6 +15269,7 @@ SUITE(pipeline) { /* Integration: structure pass */ RUN_TEST(pipeline_grpc_routes_cover_every_service_past_the_old_cap); RUN_TEST(pipeline_doclinks_edge_lands_in_store); + RUN_TEST(pipeline_sql_dump_graph_matches_the_full_parse_issue1735); RUN_TEST(pipeline_spill_resolves_namespace_imports_like_memory); RUN_TEST(pipeline_structure_nodes); RUN_TEST(pipeline_committed_counts_match_persisted); From 8093d10b812d9a7840ae99d8f9d24a04cb0f9559 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 25 Sep 2026 22:24:01 +0200 Subject: [PATCH 05/10] fix(rescript): stop binary .res resources hanging the indexer (#2176) A 4.7 MB binary Godot resource (piko_walk_mesh.res) never finished indexing: `.res` maps to ReScript, so the resource was parsed as ReScript source and the definitions pass spun. Three defects, each fixed: 1. Quadratic usage walk (the hang). The ReScript `let` binding check (is_first_named_part_of, shared with Julia, Typst, Elm, PureScript and Nickel) climbed from every named leaf with ts_node_parent, which descends from the root and scans each level's children. A binary file parses into a flat error tree (~170k root children per MB), so each climb cost O(root children) and the file O(n^2): 1 MB spent 46 s in cbm_extract_unified, 4.7 MB never returned. The check now climbs the unified walk's own cursor (O(1) per hop), like the other occurrence classifiers; the node-based climb stays only for callers outside the walk. 1 MB: 46 s -> 0.03 s; 4.7 MB: 2.2 s end to end. 2. Quadratic lexing in error recovery. tree-sitter retries the lexer at every byte of an unparseable stretch with every external token valid; the ReScript scanner's template-string loop then ran to the next backtick/$/backslash/NUL (end of file if none) and discarded the result. That cost O(n^2) inside lexing, where the parse budget's progress callback never runs, so such files were dropped by the 5 s CPU budget instead of parsed -- plain text too (100 KB of non-ReScript text: 15 s -> 0.18 s). Local scanner patch: skip the template branch when NEWLINE is also valid, which only the ERROR state allows. Valid files parse to identical trees. Recorded in vendored/grammars/MANIFEST.md. 3. Binary content parsed as source. `.res` is also Godot's binary resource format and the Windows compiled-resource format. Following the .m/.cls/.frm/.inc/.cfc precedent, cbm_disambiguate_res reads the first 4 KB and reports a file holding a NUL byte as unsupported; ReScript text never contains one. Tests (deterministic work counters, no clocks): - extraction::extract_rescript_let_bindings_use_the_walk_cursor -- zero root-descending parent hops at 128 and 1024 statements (before: 515 / 4099). - complexity::complexity_rescript_error_recovery_lexing_is_linear -- lexer bytes pulled through a chunked TSInput, 4 KB vs 8 KB, ratio must be linear (before 3.99, after 2.00). - language::lang_res_binary_resource_unsupported and lang_res_rescript_stays_rescript. Fixes #2176 Signed-off-by: Martin Vogel --- internal/cbm/extract_usages.c | 54 +++++++++---- internal/cbm/vendored/grammars/MANIFEST.md | 1 + .../cbm/vendored/grammars/rescript/scanner.c | 7 +- src/discover/discover.c | 4 + src/discover/discover.h | 6 ++ src/discover/language.c | 21 +++++ tests/test_complexity.c | 77 +++++++++++++++++++ tests/test_extraction.c | 68 ++++++++++++++++ tests/test_language.c | 32 ++++++++ 9 files changed, 254 insertions(+), 16 deletions(-) diff --git a/internal/cbm/extract_usages.c b/internal/cbm/extract_usages.c index f6b3913151..bdad1c8c7c 100644 --- a/internal/cbm/extract_usages.c +++ b/internal/cbm/extract_usages.c @@ -622,14 +622,38 @@ static bool is_elixir_def_binding(CBMExtractCtx *ctx, TSNode node) { return false; } -static bool is_first_named_part_of(TSNode node, const char *container_kind) { - for (TSNode parent = ts_node_parent(node); !ts_node_is_null(parent); - parent = ts_node_parent(parent)) { +/* Climbs from `node` to its nearest `container_kind` ancestor and reports + * whether `node` sits in that ancestor's first named child. It runs for every + * named leaf of ReScript, Julia, Typst, Elm, PureScript and Nickel files, so it + * climbs the unified walk's cursor (O(1) per hop) whenever the walk is on + * `node`. ts_node_parent answers by descending from the ROOT and scanning the + * children at every level: on an error-heavy parse (a binary Godot `.res` + * resource yields a root with ~170k children per MB) each call is + * O(root children), and one climb per leaf made the file quadratic (#2176: + * 1 MB spent 46 s in this walk, 4.7 MB never finished). The node-based climb + * remains only for callers outside the walk. */ +static bool is_first_named_part_of(TSNode node, const char *container_kind, WalkState *state) { + TSTreeCursor *cursor = reset_occurrence_cursor(state, node); + TSNode current = node; + for (;;) { + TSNode parent; + if (cursor) { + if (!ts_tree_cursor_goto_parent(cursor)) { + return false; + } + parent = ts_tree_cursor_current_node(cursor); + } else { + usage_slow_parent_fallback_test_note(); /* one root descent per hop */ + parent = ts_node_parent(current); + if (ts_node_is_null(parent)) { + return false; + } + } if (strcmp(ts_node_type(parent), container_kind) == 0) { return named_child_contains(parent, 0, node); } + current = parent; } - return false; } static bool is_wolfram_lhs(TSNode node) { @@ -1032,8 +1056,8 @@ static bool is_pkl_declaration_binding(TSNode node) { return false; } -static bool is_policy_binding(CBMExtractCtx *ctx, TSNode node, - const CBMOccurrenceSpec *occurrence) { +static bool is_policy_binding(CBMExtractCtx *ctx, TSNode node, const CBMOccurrenceSpec *occurrence, + WalkState *state) { switch (occurrence->policy) { case CBM_OCCURRENCE_LISP_DEF: return is_lisp_def_binding(ctx, node); @@ -1044,11 +1068,11 @@ static bool is_policy_binding(CBMExtractCtx *ctx, TSNode node, case CBM_OCCURRENCE_ELIXIR_DEF: return is_elixir_def_binding(ctx, node); case CBM_OCCURRENCE_JULIA_FUNCTION: - return is_first_named_part_of(node, "function_definition"); + return is_first_named_part_of(node, "function_definition", state); case CBM_OCCURRENCE_WOLFRAM_SET: return is_wolfram_lhs(node); case CBM_OCCURRENCE_TYPST_LET: - return is_first_named_part_of(node, "let"); + return is_first_named_part_of(node, "let", state); case CBM_OCCURRENCE_AGDA_FUNCTION: for (TSNode parent = ts_node_parent(node); !ts_node_is_null(parent); parent = ts_node_parent(parent)) { @@ -1064,14 +1088,14 @@ static bool is_policy_binding(CBMExtractCtx *ctx, TSNode node, case CBM_OCCURRENCE_HCL_ATTRIBUTE: return is_hcl_attribute_binding(node); case CBM_OCCURRENCE_ELM_VALUE: - return is_first_named_part_of(node, "value_declaration"); + return is_first_named_part_of(node, "value_declaration", state); case CBM_OCCURRENCE_RESCRIPT_LET: - return is_first_named_part_of(node, "let_binding"); + return is_first_named_part_of(node, "let_binding", state); case CBM_OCCURRENCE_PURESCRIPT_LHS: - return is_first_named_part_of(node, "function"); + return is_first_named_part_of(node, "function", state); case CBM_OCCURRENCE_NICKEL_LET: - return is_first_named_part_of(node, "let_binding") || - is_first_named_part_of(node, "pattern_fun"); + return is_first_named_part_of(node, "let_binding", state) || + is_first_named_part_of(node, "pattern_fun", state); case CBM_OCCURRENCE_ERLANG_CLAUSE: return is_erlang_clause_binding(node); case CBM_OCCURRENCE_NIX_FUNCTION: @@ -1119,7 +1143,7 @@ static bool is_binding_occurrence(CBMExtractCtx *ctx, TSNode node, const CBMLang if (is_exact_language_binding(ctx, node, state)) { return true; } - if (is_policy_binding(ctx, node, occurrence)) { + if (is_policy_binding(ctx, node, occurrence, state)) { return true; } @@ -2629,7 +2653,7 @@ void handle_usages(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec *spec, Wal bool possible_binding_leaf = ts_node_is_named(node) && ts_node_named_child_count(node) == 0 && (state->inside_import || is_exact_language_binding(ctx, node, state) || - is_policy_binding(ctx, node, occurrence)); + is_policy_binding(ctx, node, occurrence, state)); if (!reference_node && !possible_binding_leaf) { return; } diff --git a/internal/cbm/vendored/grammars/MANIFEST.md b/internal/cbm/vendored/grammars/MANIFEST.md index 74322763e8..9bedf192e2 100644 --- a/internal/cbm/vendored/grammars/MANIFEST.md +++ b/internal/cbm/vendored/grammars/MANIFEST.md @@ -85,6 +85,7 @@ row instead. |---|---|---|---| | crystal | `crystal/scanner.c`, serialize | guard `memcpy(&buffer[offset], state->literals.contents, literal_content_size)` with `if (literal_content_size > 0)` | UBSan: zero-length `memcpy` with a NULL/0-size source on the empty-state serialize round-trip (formal UB, harmless) | | rescript | `rescript/scanner.c`, deserialize | guard `memcpy(state, buffer, n_bytes)` with `if (n_bytes > 0)` | UBSan: zero-length `memcpy` with a NULL `buffer` / `n_bytes == 0` on empty-state deserialize (formal UB, harmless). The sibling serialize copies a fixed `sizeof(ScannerState)` (always > 0, non-NULL src) and needs no guard. | +| rescript | `rescript/scanner.c`, scan, `TEMPLATE_CHARS` branch | `if (valid_symbols[TEMPLATE_CHARS])` → `if (valid_symbols[TEMPLATE_CHARS] && !valid_symbols[NEWLINE])` | **Quadratic lexing in error recovery (#2176).** The only parse state whose external lex state enables both tokens is tree-sitter's ERROR state (`ts_lex_modes[0]`, external state 1; the in-template state 7 has no `_newline`), where every external token is marked valid and the lexer retries at each byte. From every byte the template loop ran to the next `` ` ``, `$`, `\` or NUL — the end of file when there is none — and returned false, so an unparseable stretch cost O(n²) inside lexing, where the parse budget's progress callback never runs. 50 KB of `0xFF` took 5.3 s, 100 KB 30 s; after: 3 ms, linear (lexer bytes pulled for 4 KB → 8 KB of `0xFF`: 8.8 M → 35.3 M before, 8.7 K → 17.4 K after, `complexity_rescript_error_recovery_lexing_is_linear`). Valid files parse to byte-identical trees; broken files recover more (a `let` after an unterminated expression is kept instead of swallowed). Upstream pin `43c2f1f35024` carries the unguarded branch | | purescript | `purescript/scanner.c`, serialize | guard `memcpy(buffer, indents->data, to_copy)` with `if (to_copy > 0)` | UBSan: zero-length `memcpy` with a NULL/0-size source when the indent vector is empty (formal UB, harmless) | | plsql | `plsql/parser.c`, include | `#include ` → `#include "tree_sitter/parser.h"` | The older ABI-14 generator emits angle brackets; every other vendored grammar uses the quoted form, which resolves the per-grammar `tree_sitter/` header from the including file's directory | | properties | `properties/scanner.c`, whole file | move the `reached_eof` flag out of a file-scope `static bool` and into a per-parser payload allocated by `..._external_scanner_create()` | **Data race, and the only one of its kind in 103 vendored scanners.** That flag is how the scanner refuses to emit a second end-of-input `FAKE_EOL`, and upstream keeps it in ONE process-wide object. With several worker threads indexing `.properties` files at once, whichever reached EOF first set the flag, so another thread's `!reached_eof` was false and its parse never received the `FAKE_EOL` it needed to finish — it sat at end-of-input asking for a token that would never arrive. Measured 2026-09-19: ~106 M parse operations on a 253-byte file before the per-file budget stopped it, after which the file was dropped from the graph and java/kotlin indexes differed run to run. Interleaved A/B under identical load, 40 runs each: unpatched 6 stalls, patched 0. The serialize/deserialize wire format is unchanged (the state is still the returned length). Upstream `6310671b24d4` still carries the static, so a re-vendor must re-apply this | diff --git a/internal/cbm/vendored/grammars/rescript/scanner.c b/internal/cbm/vendored/grammars/rescript/scanner.c index 171d4b6285..fb7aecfae6 100644 --- a/internal/cbm/vendored/grammars/rescript/scanner.c +++ b/internal/cbm/vendored/grammars/rescript/scanner.c @@ -159,7 +159,12 @@ bool tree_sitter_rescript_external_scanner_scan( skip(lexer); } - if (valid_symbols[TEMPLATE_CHARS]) { + // cbm local patch (#2176, see MANIFEST.md): NEWLINE is never valid inside + // a template string, so both valid at once means error recovery, where + // tree-sitter marks every external token valid and retries at each byte. + // Scanning template chars there ran to the next '`', '$', '\\' or NUL (the + // end of file when there is none) from every byte: O(n^2) per file. + if (valid_symbols[TEMPLATE_CHARS] && !valid_symbols[NEWLINE]) { lexer->result_symbol = TEMPLATE_CHARS; for (bool has_content = false;; has_content = true) { lexer->mark_end(lexer); diff --git a/src/discover/discover.c b/src/discover/discover.c index fd4cea2720..0e5ae5e4f9 100644 --- a/src/discover/discover.c +++ b/src/discover/discover.c @@ -799,6 +799,10 @@ static CBMLanguage detect_file_language(const char *entry_name, const char *abs_ if (dot && strcmp(dot, ".frm") == 0) { lang = cbm_disambiguate_frm(abs_path); } + /* Special: .res is also a binary Godot / Windows resource (#2176) */ + if (dot && strcmp(dot, ".res") == 0) { + lang = cbm_disambiguate_res(abs_path); + } /* Special: ObjectScript Studio Export XML () is * detected by content; otherwise .xml stays XML. */ if (lang == CBM_LANG_XML) { diff --git a/src/discover/discover.h b/src/discover/discover.h index fb8d7b5e6a..89ccca4f36 100644 --- a/src/discover/discover.h +++ b/src/discover/discover.h @@ -51,6 +51,12 @@ CBMLanguage cbm_disambiguate_cls(const char *path); * CBM_LANG_FORM. On read failure, defaults to CBM_LANG_FORM. */ CBMLanguage cbm_disambiguate_frm(const char *path); +/* Disambiguate .res files by reading first 4KB of content (#2176). + * Returns CBM_LANG_COUNT (not indexed) for binary content -- a Godot resource + * or a Windows compiled resource file, recognised by a NUL byte -- otherwise + * CBM_LANG_RESCRIPT. On read failure, defaults to CBM_LANG_RESCRIPT. */ +CBMLanguage cbm_disambiguate_res(const char *path); + /* Disambiguate .inc files by reading first 4KB of content. * Returns CBM_LANG_OBJECTSCRIPT_ROUTINE if it looks like an ObjectScript * include (a "ROUTINE " header), otherwise CBM_LANG_BITBAKE. diff --git a/src/discover/language.c b/src/discover/language.c index c9d92719aa..dcc41dafd0 100644 --- a/src/discover/language.c +++ b/src/discover/language.c @@ -1309,6 +1309,27 @@ CBMLanguage cbm_disambiguate_frm(const char *path) { return has_vb6_markers(buf) ? CBM_LANG_COUNT : CBM_LANG_FORM; } +/* Disambiguate .res files (#2176): ReScript source shares the extension with + * two binary formats -- Godot resources (RSRC/RSCC magic, the default save + * format for imported meshes) and Windows compiled resource files. Binary + * content has no ReScript meaning, and a NUL byte never occurs in ReScript + * text while both binary formats carry NULs in their first bytes. */ +CBMLanguage cbm_disambiguate_res(const char *path) { + if (!path) { + return CBM_LANG_RESCRIPT; + } + + FILE *f = cbm_fopen(path, "rb"); + if (!f) { + return CBM_LANG_RESCRIPT; + } + + char buf[CBM_SZ_4K]; + size_t n = fread(buf, SKIP_ONE, sizeof(buf), f); + (void)fclose(f); + return memchr(buf, '\0', n) ? CBM_LANG_COUNT : CBM_LANG_RESCRIPT; +} + /* Disambiguate .cls files: shared by InterSystems ObjectScript UDL, Salesforce * Apex and Visual Basic 6 class modules (#721). ObjectScript class files begin * with a line of the form "Class ..."; VB6 class modules diff --git a/tests/test_complexity.c b/tests/test_complexity.c index d9c651ff9e..e510f52eb7 100644 --- a/tests/test_complexity.c +++ b/tests/test_complexity.c @@ -49,6 +49,8 @@ #include "../src/foundation/profile.h" #include "cbm.h" #include "discover/discover.h" +#include "lang_specs.h" +#include "tree_sitter/api.h" #include "pipeline/pass_lsp_cross.h" #include "pipeline/pipeline.h" #include "pipeline/pipeline_internal.h" @@ -760,7 +762,82 @@ TEST(complexity_importance_scoring_is_linear) { PASS(); } +/* ── Lexer work in error recovery (#2176) ────────────────────────────── + * tree-sitter lexes an unparseable stretch by retrying at every byte with + * every external token marked valid. The ReScript scanner then ran its + * template-string loop from each byte to the next '`', '$', '\\' or NUL — to + * the end of the file when there is none — and threw the result away. That + * is O(stretch) per byte, O(n^2) per file, all inside lexing where the parse + * budget's progress callback never runs; a binary Godot `.res` of high bytes + * (or plain text such as a run of '~') was dropped by the clock instead of + * parsed. Work is counted as the bytes the lexer pulls through a chunked + * TSInput: a pure function of (grammar, input), independent of speed. */ +enum { CX_LEX_CHUNK = 64, CX_LEX_BASE_BYTES = 4096 }; + +typedef struct { + const char *src; + uint32_t len; + uint64_t bytes_pulled; +} CxLexInput; + +static const char *cx_lex_read(void *payload, uint32_t byte_index, TSPoint position, + uint32_t *bytes_read) { + (void)position; + CxLexInput *in = (CxLexInput *)payload; + if (byte_index >= in->len) { + *bytes_read = 0; + return ""; + } + uint32_t n = in->len - byte_index; + if (n > CX_LEX_CHUNK) { + n = CX_LEX_CHUNK; + } + in->bytes_pulled += n; + *bytes_read = n; + return in->src + byte_index; +} + +/* Bytes pulled while parsing `len` copies of `fill` as ReScript; 0 on failure. */ +static uint64_t cx_rescript_lex_work(unsigned char fill, uint32_t len) { + char *src = malloc(len); + TSParser *parser = ts_parser_new(); + uint64_t work = 0; + if (src && parser && ts_parser_set_language(parser, cbm_ts_language(CBM_LANG_RESCRIPT))) { + memset(src, fill, len); + CxLexInput in = {src, len, 0}; + TSInput input = {&in, cx_lex_read, TSInputEncodingUTF8, NULL}; + TSTree *tree = ts_parser_parse(parser, NULL, input); + if (tree) { + work = in.bytes_pulled; + ts_tree_delete(tree); + } + } + if (parser) { + ts_parser_delete(parser); + } + free(src); + return work; +} + +TEST(complexity_rescript_error_recovery_lexing_is_linear) { + /* 0xFF: the reporter's binary bytes (invalid UTF-8). '~': plain ASCII text + * that ReScript cannot parse either — the defect is not binary-only. */ + static const unsigned char fills[] = {0xFF, '~'}; + for (size_t i = 0; i < sizeof(fills); i++) { + uint64_t base = cx_rescript_lex_work(fills[i], CX_LEX_BASE_BYTES); + uint64_t doubled = cx_rescript_lex_work(fills[i], 2 * CX_LEX_BASE_BYTES); + double r = cx_ratio((double)doubled, (double)base); + printf(" fill 0x%02x: lexer bytes %llu -> %llu ratio %.2f (linear ~2, quadratic ~4)\n", + fills[i], (unsigned long long)base, (unsigned long long)doubled, r); + /* Non-vacuous: the lexer must at least have read the input once. */ + ASSERT_GTE(base, (uint64_t)CX_LEX_BASE_BYTES); + ASSERT_TRUE(r >= CX_RATIO_LO && r <= CX_RATIO_HI); + } + PASS(); +} + SUITE(complexity) { + RUN_TEST(complexity_rescript_error_recovery_lexing_is_linear); RUN_TEST(complexity_replicated_modules_scale_linearly); RUN_TEST(complexity_perfile_registry_work_is_linear); RUN_TEST(complexity_importance_scoring_is_linear); diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 1e6506ed1a..c235910ac1 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -6920,6 +6920,73 @@ TEST(extract_csharp_argument_values_use_the_walk_cursor) { } PASS(); } + +/* #2176: a binary Godot `.res` resource (4.7 MB) never finished indexing. It + * parses as ReScript into an error-heavy tree whose root has ~170k children + * per MB, and the ReScript `let` binding check climbed from every named leaf + * with ts_node_parent — a descent from the root that scans the children at + * every level, so O(root children) per leaf and quadratic per file (1 MB spent + * 46 s in the unified walk). Valid ReScript with many top-level statements has + * the same flat root, so plain text drives it too. The walk now climbs its own + * cursor. The counter records every root-descending hop that check makes; the + * assertion is that the walk makes none, at any size. */ +static int extract_rescript_let_leaf_fallbacks(int statement_count, int *out_usages, + uint64_t *out_slow_parent_fallbacks) { + static const char prefix[] = "let target = 1\n"; + static const char statement[] = "let v = target\n"; + size_t capacity = sizeof(prefix) + (size_t)statement_count * sizeof(statement); + char *source = malloc(capacity); + if (!source) { + return -1; + } + size_t offset = 0; + memcpy(source + offset, prefix, sizeof(prefix) - 1U); + offset += sizeof(prefix) - 1U; + for (int i = 0; i < statement_count; i++) { + memcpy(source + offset, statement, sizeof(statement) - 1U); + offset += sizeof(statement) - 1U; + } + source[offset] = '\0'; + + cbm_usage_field_lookup_test_reset(); + CBMFileResult *result = + cbm_extract_file(source, (int)offset, CBM_LANG_RESCRIPT, "proj", "Flat.res", 0, NULL, NULL); + free(source); + if (!result) { + return -1; + } + int usages = 0; + for (int i = 0; i < result->usages.count; i++) { + if (result->usages.items[i].ref_name && + strcmp(result->usages.items[i].ref_name, "target") == 0) { + usages++; + } + } + *out_slow_parent_fallbacks = cbm_usage_slow_parent_fallback_test_count(); + cbm_free_result(result); + *out_usages = usages; + return 0; +} + +TEST(extract_rescript_let_bindings_use_the_walk_cursor) { + enum { SMALL = 128, BIG = 1024 }; + int small_usages = 0; + int big_usages = 0; + uint64_t small_fallbacks = 0; + uint64_t big_fallbacks = 0; + ASSERT_EQ(extract_rescript_let_leaf_fallbacks(SMALL, &small_usages, &small_fallbacks), 0); + ASSERT_EQ(extract_rescript_let_leaf_fallbacks(BIG, &big_usages, &big_fallbacks), 0); + fprintf(stderr, " [rescript-let-bindings] fallbacks(%d)=%llu fallbacks(%d)=%llu\n", SMALL, + (unsigned long long)small_fallbacks, BIG, (unsigned long long)big_fallbacks); + /* Anti-vacuous: every leaf really was classified (each `target` read is a + * usage, each `v` a binding), so zero fallbacks means "took the cursor", + * not "never looked". */ + ASSERT_EQ(small_usages, SMALL); + ASSERT_EQ(big_usages, BIG); + ASSERT_EQ(small_fallbacks, 0); + ASSERT_EQ(big_fallbacks, 0); + PASS(); +} #endif /* =================================================================== @@ -8266,6 +8333,7 @@ SUITE(extraction) { #if defined(CBM_CALL_REFERENCE_LOOKUP_TEST_API) && CBM_CALL_REFERENCE_LOOKUP_TEST_API RUN_TEST(extract_wide_flat_reference_fields_are_linear); RUN_TEST(extract_csharp_argument_values_use_the_walk_cursor); + RUN_TEST(extract_rescript_let_bindings_use_the_walk_cursor); #endif /* Perl call-graph noise (#459 follow-up) */ diff --git a/tests/test_language.c b/tests/test_language.c index 18ad096008..dc2bcf7d36 100644 --- a/tests/test_language.c +++ b/tests/test_language.c @@ -4,6 +4,7 @@ * RED phase: These tests define the expected behavior for registered languages. */ #include "../src/foundation/compat.h" +#include "../src/foundation/compat_fs.h" #include "test_framework.h" #include "discover/discover.h" @@ -736,6 +737,35 @@ TEST(lang_frm_form_stays_form) { PASS(); } +/* #2176: a Godot binary resource shares the .res extension with ReScript. + * Parsed as ReScript it produced an error tree that never finished indexing; + * binary content is not ReScript source and must not be indexed as such. */ +TEST(lang_res_binary_resource_unsupported) { + static const unsigned char godot_head[] = {'R', 'S', 'R', 'C', 0x00, 0x00, 0x00, 0x00, + 0x04, 0x00, 0x00, 0x00, 'A', 'r', 'r', 'a', + 'y', 'M', 'e', 's', 'h', 0x00, 0xff, 0x80}; + char path[256]; + snprintf(path, sizeof(path), "%s/test_lang_godot.res", cbm_tmpdir()); + FILE *f = cbm_fopen(path, "wb"); + ASSERT_NOT_NULL(f); + ASSERT_EQ(fwrite(godot_head, 1, sizeof(godot_head), f), sizeof(godot_head)); + fclose(f); + ASSERT_EQ(cbm_disambiguate_res(path), CBM_LANG_COUNT); + remove(path); + PASS(); +} + +TEST(lang_res_rescript_stays_rescript) { + char path[256]; + snprintf(path, sizeof(path), "%s/test_lang_rescript.res", cbm_tmpdir()); + ASSERT_TRUE(write_probe_file(path, "let greet = name => `Hello ${name}`\n" + "module M = {\n let x = 1\n}\n")); + ASSERT_EQ(cbm_disambiguate_res(path), CBM_LANG_RESCRIPT); + remove(path); + ASSERT_EQ(cbm_disambiguate_res("/tmp/nonexistent_file_12345.res"), CBM_LANG_RESCRIPT); + PASS(); +} + TEST(lang_cfc_tag_component) { /* wrapper ⇒ tag dialect. */ ASSERT_EQ(disambiguate_cfc_content("test_cfc_tag.cfc", @@ -1456,6 +1486,8 @@ SUITE(language) { RUN_TEST(lang_cls_objectscript_stays_objectscript); RUN_TEST(lang_frm_vb6_form_unsupported); RUN_TEST(lang_frm_form_stays_form); + RUN_TEST(lang_res_binary_resource_unsupported); + RUN_TEST(lang_res_rescript_stays_rescript); /* Go test ports */ /* New languages */ From e9058e11db9ed94e401ad711b2016249970a9040 Mon Sep 17 00:00:00 2001 From: Chad <4307099+BobbieBarker@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:26:22 -0700 Subject: [PATCH 06/10] fix(registry): reach a cfg-fenced function from its bare name cbm_registry_add takes the symbol's `name`, discards it, and derives the bare-name lookup key from the QN's last dot segment. That makes the QN's tail load-bearing for the by-name index every language shares, and for one grammar the tail is not a name anybody writes. rust_cfg_qualified_name mints a `#[cfg(test)]` twin as "proj.lib.add#cfg(test)"; simple_name() has no '#' handling, so that function is filed under the whole string "add#cfg(test)", which no bare `add` callee can reach, leaving a test-only twin of a resolved function absent from the by-name index entirely. The caller always knows the name, because it is the argument. So the index takes a second key from it, only where the derived key carries a fence: const char *derived = simple_name(qualified_name); index_under_name(r, derived, owned_qn); if (name && name[0] && strchr(derived, '#') && strcmp(name, derived) != 0) { index_under_name(r, name, owned_qn); } The gate keys on the fence rather than on the file's language, and that is the part worth reviewing. It makes "unchanged for every other grammar" checkable: the derived key is written unconditionally exactly as before, and every by-name lookup keys through simple_name(), which splits on '.' and "::" only. A passed name that differs from the derived key by anything other than a fence is therefore a key no lookup can reach, so adding it would only widen the bucket the scorer walks. A grammar that mints no '#' sees no new key at all. Two of the rows a language gate would have had to cover turn out not to reach this function. Registry membership is decided by cbm_label_is_registry_symbol at all three production callers (pass_definitions.c, pass_parallel.c, pipeline_incremental.c), and it admits neither "Module" nor "Section". So a file's Module node never registers, and neither does a Markdown heading. What does register, under label "Class", is the dotted-name direction: grammar def->name QN tail HCL resource.aws_instance.web web TOML tool.poetry.dependencies dependencies INI tool.isort isort These are why the derived key stays unconditional. `instance = aws_instance.web.id` reaches the block through the tail `web`, so re-keying the index on the passed name would drop that edge outright. The fence gate leaves all three byte-identical. `name` is NULL or empty only for callers that have no symbol name to give; those have the derived key and nothing else. tests/test_registry.c: registry_indexes_by_passed_name_not_qn_tail resolves a bare `add` against a cfg-fenced QN and is the reproducer, failing without this commit. registry_indexes_a_dotted_name_under_its_tail_too pins the unconditional derived key for the HCL and Module shapes. tests/test_pipeline.c: pipeline_hcl_block_reference_resolves_to_its_block indexes a two-resource Terraform file end to end and asserts the surviving USAGE edge. The latter two pass without this commit and are guards: what would break them is re-keying the by-name index, and an HCL block name is dotted where its QN tail is not, so they pin the shape that regression would take. A throwaway probe walked tests/grammar_cases.h (one fixture per grammar, all 162) comparing def->name with simple_name(def->qualified_name), then targeted fixtures for the dotted shapes above. HCL is the only fixture-level mismatch among registry-eligible labels; TOML and INI mismatch on targeted fixtures. XML namespaced elements (`ns:root`) do not mismatch, because ':' is not a QN separator. Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com> --- src/pipeline/registry.c | 86 +++++++++++++++++++++++++++++------------ tests/test_pipeline.c | 55 ++++++++++++++++++++++++++ tests/test_registry.c | 52 +++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 25 deletions(-) diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index c99c72d8d5..bdf6f5944d 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -858,9 +858,38 @@ void cbm_registry_free(cbm_registry_t *r) { /* ── Registration ────────────────────────────────────────────────── */ +/* Record `owned_qn` in the by-name bucket for `key`, keeping the bucket's + * is_test flags in step with its entries. + * + * No array dedup needed: cbm_registry_add's exact-map check guarantees the QN + * is new, and it calls this at most once per distinct key. */ +static void index_under_name(cbm_registry_t *r, const char *key, const char *owned_qn) { + qn_array_t *arr = cbm_ht_get(r->by_name, key); + if (!arr) { + arr = calloc(CBM_ALLOC_ONE, sizeof(qn_array_t)); + cbm_ht_set(r->by_name, strdup(key), arr); + } + int before = arr->count; + cbm_da_push(arr, (char *)owned_qn); + if (arr->count == before) { + return; /* the name could not be recorded: no verdict to cache */ + } + if (arr->count > arr->is_test_cap) { + int want = arr->cap > 0 ? arr->cap : arr->count; + uint8_t *grown = + cbm_realloc(CBM_MEM_CLASS_DYN_ARRAY, arr->is_test, (size_t)want * sizeof(uint8_t)); + if (grown) { + arr->is_test = grown; + arr->is_test_cap = want; + } + } + if (arr->count <= arr->is_test_cap) { + arr->is_test[arr->count - SKIP_ONE] = is_test_qn(owned_qn) ? 1 : 0; + } +} + void cbm_registry_add(cbm_registry_t *r, const char *name, const char *qualified_name, const char *label) { - (void)name; if (!r || !qualified_name || !label) { return; } @@ -893,30 +922,37 @@ void cbm_registry_add(cbm_registry_t *r, const char *name, const char *qualified cbm_ht_set(r->exact, strdup(qualified_name), (void *)interned); const char *owned_qn = cbm_ht_get_key(r->exact, qualified_name); - /* Index by simple name. - * No array dedup needed: exact-map check above guarantees uniqueness. */ - const char *simple = simple_name(qualified_name); - qn_array_t *arr = cbm_ht_get(r->by_name, simple); - if (!arr) { - arr = calloc(CBM_ALLOC_ONE, sizeof(qn_array_t)); - cbm_ht_set(r->by_name, strdup(simple), arr); - } - int before = arr->count; - cbm_da_push(arr, (char *)owned_qn); - if (arr->count == before) { - return; /* the name could not be recorded: no verdict to cache */ - } - if (arr->count > arr->is_test_cap) { - int want = arr->cap > 0 ? arr->cap : arr->count; - uint8_t *grown = - cbm_realloc(CBM_MEM_CLASS_DYN_ARRAY, arr->is_test, (size_t)want * sizeof(uint8_t)); - if (grown) { - arr->is_test = grown; - arr->is_test_cap = want; - } - } - if (arr->count <= arr->is_test_cap) { - arr->is_test[arr->count - SKIP_ONE] = is_test_qn(owned_qn) ? 1 : 0; + /* Index the symbol under the QN's last dot segment, and additionally under + * the name its caller passed when that segment carries a '#' fence. + * + * The derived key is the one every language has always used and it stays + * unconditional, so a grammar whose QNs carry no '#' is byte-identical to + * before this commit. The second key exists for one shape: a fenced tail is + * a literal string no bare callee is ever written as. rust_cfg_qualified_name + * mints a `#[cfg(test)]` twin as "proj.lib.add#cfg(test)", simple_name() has + * no '#' handling, so the derived key filed that function under the whole + * string "add#cfg(test)" where no bare `add` callee could reach it. + * + * Gating on the fence rather than on the file's language is what makes + * "unchanged for every other grammar" checkable instead of asserted. Every + * by-name lookup keys through simple_name(), which splits on '.' and "::" + * only, so a passed name that differs from the derived key by anything + * OTHER than a fence is a key no lookup can reach: an HCL block's + * "resource.aws_instance.web" and a TOML table's "tool.poetry.dependencies" + * are both already indexed under the tail a reference spells, and adding + * their dotted form would only widen the bucket the scorer walks. + * + * `name` is NULL or empty only for callers that have no symbol name to + * give; those have the derived key and nothing else. */ + const char *derived = simple_name(qualified_name); + index_under_name(r, derived, owned_qn); + /* '#' is a QN fence, and extract_defs.c's rust_cfg_qualified_name is the + * only thing in the tree that mints one today. A grammar that starts + * minting a '#' opts into this second key by doing so, whatever it means by + * the fence: its symbols become reachable under the passed name as well, + * and they share that name's bucket with everything else filed under it. */ + if (name && name[0] && strchr(derived, '#') && strcmp(name, derived) != 0) { + index_under_name(r, name, owned_qn); } } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 798c6556ad..4ed39ba205 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -1147,6 +1147,60 @@ TEST(pipeline_nix_scoped_binding_calls_resolve) { PASS(); } +/* Terraform reference resolution, end to end. + * + * An HCL block names itself with its labels appended -- find_hcl_block_name + * mints "resource.aws_instance.web" -- while its QN's last dot segment is + * bare "web", and a reference written `aws_instance.web.id` reaches the + * by-name index through that tail. HCL is therefore a language where the + * definition name and the QN tail are different strings, and an index keyed on + * either one alone drops every cross-resource reference in the file. + * + * This has to be a pipeline test: the registry is shared by every language + * and nothing in the HCL extractor mentions the index key, so the two halves + * can disagree with every extraction-level assertion still green. + */ +TEST(pipeline_hcl_block_reference_resolves_to_its_block) { + if (setup_test_repo() != 0) { + FAIL("failed to create temp dir"); + } + + char tf_path[512]; + snprintf(tf_path, sizeof(tf_path), "%s/main.tf", g_tmpdir); + FILE *tf = fopen(tf_path, "w"); + if (!tf) { + teardown_test_repo(); + FAIL("failed to write terraform fixture"); + } + fprintf(tf, "resource \"aws_instance\" \"web\" {\n" + " ami = \"ami-0c55b159cbfafe1f0\"\n" + " instance_type = \"t2.micro\"\n" + "}\n" + "\n" + "resource \"aws_eip\" \"ip\" {\n" + " instance = aws_instance.web.id\n" + "}\n"); + fclose(tf); + + char tf_db[512]; + snprintf(tf_db, sizeof(tf_db), "%s/test_hcl_refs.db", g_tmpdir); + + cbm_pipeline_t *tp = cbm_pipeline_new(g_tmpdir, tf_db, CBM_MODE_FULL); + ASSERT_NOT_NULL(tp); + ASSERT_EQ(cbm_pipeline_run(tp), 0); + + cbm_store_t *ts = cbm_store_open_path(tf_db); + ASSERT_NOT_NULL(ts); + const char *tf_project = cbm_pipeline_project_name(tp); + + ASSERT(cross_file_edge_exists(ts, tf_project, "main", "resource.aws_instance.web", "USAGE")); + + cbm_store_close(ts); + cbm_pipeline_free(tp); + teardown_test_repo(); + PASS(); +} + /* Regression: incremental re-index of an edited file must NOT drop inbound * cross-file CALLS edges whose source lives in an UNCHANGED file. * @@ -15166,6 +15220,7 @@ SUITE(pipeline) { /* Calls pass */ RUN_TEST(pipeline_calls_resolution); RUN_TEST(pipeline_nix_scoped_binding_calls_resolve); + RUN_TEST(pipeline_hcl_block_reference_resolves_to_its_block); RUN_TEST(pipeline_incremental_preserves_cross_file_calls); RUN_TEST(pipeline_objectscript_export_preserves_calls_sequential_parallel); RUN_TEST(pipeline_objectscript_export_incremental_matches_full_relationships); diff --git a/tests/test_registry.c b/tests/test_registry.c index 5f751a8993..f77b1460bb 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -325,6 +325,56 @@ TEST(resolve_qualified_ambiguous_tail_falls_through) { PASS(); } +/* cbm_registry_add used to discard its `name` argument and re-derive the + * lookup key from the QN's last dot segment. That made the QN's tail load + * bearing for the bare-name index every language shares: a Rust cfg twin, + * minted as "add#cfg(test)", was indexed under that literal string and no bare + * `add` callee could ever reach it. */ +TEST(registry_indexes_by_passed_name_not_qn_tail) { + cbm_registry_t *r = cbm_registry_new(); + cbm_registry_add(r, "add", "proj.lib.add#cfg(test)", "Function"); + + cbm_resolution_t res = cbm_registry_resolve(r, "add", "proj.lib.caller", NULL, NULL, 0); + ASSERT_STR_EQ(res.qualified_name, "proj.lib.add#cfg(test)"); + + cbm_registry_free(r); + PASS(); +} + +/* The derived key stays unconditional, and this pins that. + * + * A name may carry segments the QN's tail drops, and then the tail is the key + * callers actually spell. An HCL block is named "resource.aws_instance.web" by + * find_hcl_block_name while its QN tail is bare "web", which is how an + * `aws_instance.web.id` reference reaches it. Re-keying the index on the passed + * name would lose that lookup outright, so the fence gate above leaves these + * shapes exactly as they were. + * + * The Module row registers a shape production cannot produce: every caller of + * cbm_registry_add gates on cbm_label_is_registry_symbol, which does not admit + * "Module". It is here because the registry's own contract is per-label-string, + * not per-caller, and a future label change should not silently drop the stem + * lookup a bare module reference needs. */ +TEST(registry_indexes_a_dotted_name_under_its_tail_too) { + cbm_registry_t *r = cbm_registry_new(); + cbm_registry_add(r, "resource.aws_instance.web", "proj.main.resource.aws_instance.web", + "Class"); + cbm_registry_add(r, "helper.py", "proj.pkg.helper", "Module"); + + cbm_resolution_t tail = cbm_registry_resolve(r, "web", "proj.main", NULL, NULL, 0); + ASSERT_STR_EQ(tail.qualified_name, "proj.main.resource.aws_instance.web"); + + cbm_resolution_t whole = + cbm_registry_resolve(r, "resource.aws_instance.web", "proj.main", NULL, NULL, 0); + ASSERT_STR_EQ(whole.qualified_name, "proj.main.resource.aws_instance.web"); + + cbm_resolution_t stem = cbm_registry_resolve(r, "helper", "proj.pkg.caller", NULL, NULL, 0); + ASSERT_STR_EQ(stem.qualified_name, "proj.pkg.helper"); + + cbm_registry_free(r); + PASS(); +} + TEST(resolve_import_map) { cbm_registry_t *r = cbm_registry_new(); cbm_registry_add(r, "Process", "proj.pkg.worker.Process", "Function"); @@ -1201,6 +1251,8 @@ SUITE(registry) { RUN_TEST(resolve_same_module); RUN_TEST(resolve_qualified_disambiguates_same_name); RUN_TEST(resolve_qualified_ambiguous_tail_falls_through); + RUN_TEST(registry_indexes_by_passed_name_not_qn_tail); + RUN_TEST(registry_indexes_a_dotted_name_under_its_tail_too); RUN_TEST(resolve_import_map); RUN_TEST(resolve_import_map_bare_function); RUN_TEST(resolve_import_map_bare_alias); From 19d144477b9558dad148d3f555ec6ede16caa3f5 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 26 Sep 2026 01:07:11 +0200 Subject: [PATCH 07/10] fix(daemon): name the conflicting daemon and the way out on a build conflict (#2277) A client refused because another CBM build owns the account daemon printed only "Close all CBM sessions and commands, then retry." The reporter had a 0.10.3 daemon kept alive by a running Codex session while starting 0.11.0 from a terminal, and nothing in the message said which process that was, that an agent session held it, or that `daemon stop` exists. The refusal itself is correct and stays unchanged: session-managed daemons serving live sessions must never be killed by a newer client, and cbm_daemon_conflict_format() is validated byte-for-byte across generations on the HELLO wire, so its text cannot change without breaking old/new daemon pairs. The fix adds a separate, client-side remedy sentence built from the cross-build STATUS probe (cbm_daemon_conflict_remedy_format): active daemon pid, version, permanent vs session-managed, "already stopping", the pids of the CBM sessions holding it, the likely cause (another editor or agent launched a different install; compare `which -a codebase-memory-mcp` with each client's MCP config) and `daemon stop` once no session uses it. When STATUS does not answer, the remedy points at `daemon status`. It is appended on every user-facing conflict exit: the MCP/CLI bootstrap path (after the older-permanent auto-drain policy declines), the one-shot CLI exact-build cohort refusal and the MCP client cohort refusal (the path the reporter hit). Hooks skip the extra probe to keep their fail-open budget. The probe uses the `daemon status` budget (3 s): the daemon hashes the ~300 MB peer image before answering, which overran the 1 s connect budget in the real-binary proof. No identity or admission check changes; STATUS is already the authenticated, cross-build observability op behind `daemon status`. Signed-off-by: Martin Vogel --- src/daemon/bootstrap.h | 3 ++ src/daemon/runtime.c | 67 ++++++++++++++++++++++++++++ src/daemon/runtime.h | 11 +++++ src/main.c | 82 ++++++++++++++++++++++++++++------ tests/test_daemon_version.c | 87 +++++++++++++++++++++++++++++++++++++ 5 files changed, 237 insertions(+), 13 deletions(-) diff --git a/src/daemon/bootstrap.h b/src/daemon/bootstrap.h index 8b1e797dd7..9cc322c953 100644 --- a/src/daemon/bootstrap.h +++ b/src/daemon/bootstrap.h @@ -93,6 +93,9 @@ typedef struct { cbm_daemon_runtime_connect_result_t connect_result; bool daemon_spawned; char message[CBM_DAEMON_CONFLICT_MESSAGE_SIZE]; + /* #2277: filled by the client on a CONFLICT only — who holds the + * endpoint and how to clear it (cbm_daemon_conflict_remedy_format). */ + char remedy[CBM_DAEMON_CONFLICT_REMEDY_SIZE]; } cbm_daemon_bootstrap_result_t; /* A probe distinguishes an absent endpoint from a reserved endpoint whose diff --git a/src/daemon/runtime.c b/src/daemon/runtime.c index efa41a0204..8d1d8af949 100644 --- a/src/daemon/runtime.c +++ b/src/daemon/runtime.c @@ -2857,6 +2857,73 @@ bool cbm_daemon_runtime_request_status(const cbm_daemon_ipc_endpoint_t *endpoint return true; } +/* "; CBM sessions using it: pids 1, 2 and 3 more" — empty when none are known. */ +static bool remedy_sessions_format(const cbm_daemon_runtime_status_t *active, char *out, + size_t out_size) { + out[0] = '\0'; + size_t listed = active->client_count; + if (listed > CBM_DAEMON_CONTROL_CLIENT_CAP) { + listed = CBM_DAEMON_CONTROL_CLIENT_CAP; + } + size_t used = 0; + for (size_t i = 0; i < listed; i++) { + int written = snprintf(out + used, out_size - used, "%s%lu", + i == 0 ? "; CBM sessions using it: pids " : ", ", + (unsigned long)active->client_pids[i]); + if (written < 0 || (size_t)written >= out_size - used) { + return false; + } + used += (size_t)written; + } + if (listed > 0 && active->committed_clients > listed) { + int written = snprintf(out + used, out_size - used, " and %lu more", + (unsigned long)(active->committed_clients - listed)); + if (written < 0 || (size_t)written >= out_size - used) { + return false; + } + } + return true; +} + +bool cbm_daemon_conflict_remedy_format(const cbm_daemon_runtime_status_t *active, char *out, + size_t out_size) { + if (!out || out_size == 0) { + return false; + } + out[0] = '\0'; + static const char install_hint[] = + "Usually another editor or agent session (for example Codex or Claude Code) " + "launched a different codebase-memory-mcp install: close it, or point every client " + "at one binary (compare `which -a codebase-memory-mcp` with each client's MCP " + "config)."; + int written; + if (!active) { + written = snprintf(out, out_size, + "Run `codebase-memory-mcp daemon status` to see the active daemon and " + "the sessions using it. %s `codebase-memory-mcp daemon stop` retires " + "the daemon once no session uses it.", + install_hint); + } else { + char sessions[160]; + if (!remedy_sessions_format(active, sessions, sizeof(sessions))) { + return false; + } + const char *lifetime = + active->permanent ? "permanent" : "session-managed, exits when its last session closes"; + written = snprintf(out, out_size, + "Active daemon: pid %lu, version %s, %s%s%s. %s " + "`codebase-memory-mcp daemon stop` retires the daemon once no session " + "uses it.", + (unsigned long)active->daemon_pid, active->semantic_version, lifetime, + active->stopping ? ", already stopping" : "", sessions, install_hint); + } + if (written < 0 || (size_t)written >= out_size) { + out[0] = '\0'; + return false; + } + return true; +} + bool cbm_daemon_runtime_request_stop(const cbm_daemon_ipc_endpoint_t *endpoint, const cbm_daemon_build_identity_t *identity, uint32_t timeout_ms, diff --git a/src/daemon/runtime.h b/src/daemon/runtime.h index 7a31dab615..d9c3db8e98 100644 --- a/src/daemon/runtime.h +++ b/src/daemon/runtime.h @@ -301,6 +301,17 @@ bool cbm_daemon_runtime_request_stop(const cbm_daemon_ipc_endpoint_t *endpoint, uint32_t timeout_ms, cbm_daemon_runtime_stop_result_t *result_out); +/* #2277: the actionable half of a version/build conflict refusal. The + * conflict text itself (cbm_daemon_conflict_format) is wire-validated + * byte-for-byte across generations and must not change, so the remedy is a + * separate, client-side sentence built from the cross-build STATUS probe: + * which daemon holds the endpoint (pid, version, lifetime), which CBM + * sessions keep it alive, and how to clear it. active may be NULL when the + * status probe did not answer. Returns false (out empty) on truncation. */ +#define CBM_DAEMON_CONFLICT_REMEDY_SIZE 640U +bool cbm_daemon_conflict_remedy_format(const cbm_daemon_runtime_status_t *active, char *out, + size_t out_size); + /* Performs the complete guarded first-participant handoff, starts listening * synchronously, then owns both that participant claim and its * accept/connection threads. All config scalar/text data is copied. endpoint diff --git a/src/main.c b/src/main.c index 65e041217b..fd3370c044 100644 --- a/src/main.c +++ b/src/main.c @@ -45,6 +45,10 @@ enum { MAIN_MAX_PORT = 65536, MAIN_PATH_CAP = 4096, MAIN_CONNECT_TIMEOUT_MS = 1000, + /* #2277: the conflict-remedy STATUS probe runs only after a refusal, so it + * gets the same budget as `daemon status`: the daemon hashes the peer's + * (~300 MB) image before answering, which can exceed a 1 s connect budget. */ + MAIN_CONFLICT_STATUS_PROBE_TIMEOUT_MS = 3000, MAIN_STARTUP_TIMEOUT_MS = 10000, /* Backstop for waiting out a held startup transition — see * main_local_transition_acquire. Not a budget for healthy contention: a busy @@ -1698,6 +1702,10 @@ static bool main_semver_newer(const char *candidate, const char *active) { return false; } +/* A conflict refusal plus its remedy (#2277), joined by one space. */ +#define MAIN_CLIENT_FAILURE_DETAIL_CAP \ + (CBM_DAEMON_CONFLICT_MESSAGE_SIZE + CBM_DAEMON_CONFLICT_REMEDY_SIZE + 1U) + /* A client that cannot reach the daemon must SAY SO, in the caller's own * protocol. An MCP client speaks JSON-RPC over stdout, and the old path * returned EXIT_FAILURE having written nothing at all: agents saw a transport @@ -1721,7 +1729,7 @@ static bool main_semver_newer(const char *candidate, const char *active) { * just the one that happened to be fixed first. */ static void main_report_client_failure(cbm_daemon_process_role_t role, const char *detail) { if (cbm_daemon_process_role_requires_client(role)) { - char escaped[CBM_DAEMON_CONFLICT_MESSAGE_SIZE * 2]; + char escaped[MAIN_CLIENT_FAILURE_DETAIL_CAP * 2]; size_t out = 0; for (size_t i = 0; detail[i] && out + 2 < sizeof(escaped); i++) { unsigned char c = (unsigned char)detail[i]; @@ -1746,10 +1754,35 @@ static void main_report_client_failure(cbm_daemon_process_role_t role, const cha static void main_report_client_bootstrap_failure(cbm_daemon_process_role_t role, const cbm_daemon_bootstrap_result_t *result) { - main_report_client_failure(role, (result && result->message[0]) - ? result->message - : "CBM daemon connection failed before the session was " - "established"); + const char *message = (result && result->message[0]) + ? result->message + : "CBM daemon connection failed before the session was " + "established"; + char detail[MAIN_CLIENT_FAILURE_DETAIL_CAP]; + (void)snprintf(detail, sizeof(detail), "%s%s%s", message, + (result && result->remedy[0]) ? " " : "", result ? result->remedy : ""); + main_report_client_failure(role, detail); +} + +/* #2277: a conflict is a legitimate refusal (another build owns the account + * daemon), but it must name that daemon and the way out. known_active is a + * STATUS answer the caller already holds; without one this probes STATUS + * (cross-build by design) itself. When nothing answers, the remedy falls + * back to pointing at `daemon status`. */ +static void main_conflict_remedy_probe(const cbm_daemon_ipc_endpoint_t *endpoint, + const cbm_daemon_build_identity_t *identity, + const cbm_daemon_runtime_status_t *known_active, char *out, + size_t out_size) { + cbm_daemon_runtime_status_t probed; + const cbm_daemon_runtime_status_t *active = known_active; + if (!active && endpoint && identity && + cbm_daemon_runtime_request_status(endpoint, identity, MAIN_CONFLICT_STATUS_PROBE_TIMEOUT_MS, + &probed)) { + active = &probed; + } + if (!cbm_daemon_conflict_remedy_format(active, out, out_size)) { + out[0] = '\0'; + } } /* Client bootstrap with the upgrade policy: a CONFLICT against a PERMANENT @@ -1766,10 +1799,13 @@ static cbm_daemon_bootstrap_status_t main_client_bootstrap_with_upgrade( return status; } cbm_daemon_runtime_status_t active; - if (!cbm_daemon_runtime_request_status(config->endpoint, config->identity, - MAIN_CONNECT_TIMEOUT_MS, &active) || - !active.permanent || + bool active_known = cbm_daemon_runtime_request_status(config->endpoint, config->identity, + MAIN_CONNECT_TIMEOUT_MS, &active); + if (!active_known || !active.permanent || !main_semver_newer(config->identity->semantic_version, active.semantic_version)) { + main_conflict_remedy_probe(config->endpoint, config->identity, + active_known ? &active : NULL, result->remedy, + sizeof(result->remedy)); return status; } if (report_lifecycle) { @@ -1786,9 +1822,16 @@ static cbm_daemon_bootstrap_status_t main_client_bootstrap_with_upgrade( !drain.accepted) { (void)fprintf(stderr, "codebase-memory-mcp: the active daemon did not accept the " "upgrade drain; run `codebase-memory-mcp daemon stop`\n"); + main_conflict_remedy_probe(config->endpoint, config->identity, &active, result->remedy, + sizeof(result->remedy)); return status; } - return cbm_daemon_bootstrap_execute(config, result); + status = cbm_daemon_bootstrap_execute(config, result); + if (status == CBM_DAEMON_BOOTSTRAP_CONFLICT) { + main_conflict_remedy_probe(config->endpoint, config->identity, NULL, result->remedy, + sizeof(result->remedy)); + } + return status; } /* One-shot CLI commands execute through the shared daemon, exactly like MCP @@ -1825,6 +1868,9 @@ static char *main_local_cli_daemon_execute(const char *tool_name, const char *ar (void)fprintf(stderr, "error: %s\n", bootstrap.message[0] ? bootstrap.message : "no CBM daemon connection for CLI execution"); + if (bootstrap.remedy[0]) { + (void)fprintf(stderr, "hint: %s\n", bootstrap.remedy); + } return NULL; } if (bootstrap.daemon_spawned && cbm_log_get_level() <= CBM_LOG_INFO) { @@ -2870,13 +2916,17 @@ int main(int argc, char **argv) { char message[CBM_DAEMON_CONFLICT_MESSAGE_SIZE]; bool formatted = cohort_status == CBM_VERSION_COHORT_CONFLICT && cbm_daemon_conflict_format(&cohort_conflict, message, sizeof(message)); + char remedy[CBM_DAEMON_CONFLICT_REMEDY_SIZE] = ""; if (cohort_status == CBM_VERSION_COHORT_CONFLICT) { (void)cbm_version_cohort_log_conflict(&cohort_conflict); + main_conflict_remedy_probe(local_endpoint, &local_identity, NULL, remedy, + sizeof(remedy)); } - (void)fprintf(stderr, "codebase-memory-mcp: %s\n", + (void)fprintf(stderr, "codebase-memory-mcp: %s%s%s\n", formatted ? message : "CLI exact-build admission could not be verified; retry " - "after active CBM operations exit"); + "after active CBM operations exit", + remedy[0] ? " " : "", remedy); goto local_cli_cleanup; } main_local_maintenance_context_init(&maintenance_context); @@ -3198,11 +3248,17 @@ int main(int argc, char **argv) { bool formatted = client_cohort_status == CBM_VERSION_COHORT_CONFLICT && cbm_daemon_conflict_format(&client_cohort_conflict, message, sizeof(message)); + char remedy[CBM_DAEMON_CONFLICT_REMEDY_SIZE] = ""; if (client_cohort_status == CBM_VERSION_COHORT_CONFLICT) { (void)cbm_version_cohort_log_conflict(&client_cohort_conflict); + /* Hooks fail open on a tight budget; they skip the extra probe. */ + if (role != CBM_DAEMON_PROCESS_HOOK_CLIENT) { + main_conflict_remedy_probe(endpoint, &identity, NULL, remedy, sizeof(remedy)); + } } - (void)fprintf(stderr, "codebase-memory-mcp: %s\n", - formatted ? message : "client exact-build admission failed"); + (void)fprintf(stderr, "codebase-memory-mcp: %s%s%s\n", + formatted ? message : "client exact-build admission failed", + remedy[0] ? " " : "", remedy); if (role == CBM_DAEMON_PROCESS_HOOK_CLIENT && client_cohort_status == CBM_VERSION_COHORT_CONFLICT) { main_hook_report_conflicted_daemon(hook_dialect); diff --git a/tests/test_daemon_version.c b/tests/test_daemon_version.c index 89982546a0..4f368d3577 100644 --- a/tests/test_daemon_version.c +++ b/tests/test_daemon_version.c @@ -9,6 +9,7 @@ #include "test_framework.h" #include "daemon/daemon.h" +#include "daemon/runtime.h" #include "daemon/service.h" #include "daemon/service_internal.h" #include "foundation/compat.h" @@ -277,6 +278,87 @@ TEST(daemon_hello_version_conflict_exposes_active_and_requested_builds) { PASS(); } +/* #2277: a version/build conflict refusal must say WHICH daemon holds the + * endpoint and how to clear it. The reporter saw only "close all CBM sessions + * and commands", with no pid, no hint that a live agent session (Codex) had + * launched an older install, and no mention of `daemon stop`. */ +static cbm_daemon_runtime_status_t version_test_active_status(const char *version, bool permanent, + uint32_t pid) { + cbm_daemon_runtime_status_t status; + memset(&status, 0, sizeof(status)); + status.permanent = permanent; + status.daemon_pid = pid; + (void)snprintf(status.semantic_version, sizeof(status.semantic_version), "%s", version); + (void)snprintf(status.build_fingerprint, sizeof(status.build_fingerprint), "%s", BUILD_A); + return status; +} + +TEST(daemon_conflict_remedy_names_session_managed_daemon_and_its_sessions) { + cbm_daemon_runtime_status_t active = version_test_active_status("0.10.3", false, 4242); + active.committed_clients = 2; + active.client_count = 2; + active.client_pids[0] = 5151; + active.client_pids[1] = 6262; + char remedy[CBM_DAEMON_CONFLICT_REMEDY_SIZE]; + + ASSERT_TRUE(cbm_daemon_conflict_remedy_format(&active, remedy, sizeof(remedy))); + ASSERT_NOT_NULL(strstr(remedy, "pid 4242")); + ASSERT_NOT_NULL(strstr(remedy, "version 0.10.3")); + ASSERT_NOT_NULL(strstr(remedy, "session-managed")); + ASSERT_NOT_NULL(strstr(remedy, "5151, 6262")); + ASSERT_NOT_NULL(strstr(remedy, "which -a codebase-memory-mcp")); + ASSERT_NOT_NULL(strstr(remedy, "codebase-memory-mcp daemon stop")); + PASS(); +} + +TEST(daemon_conflict_remedy_names_permanent_daemon_and_truncated_session_list) { + cbm_daemon_runtime_status_t active = version_test_active_status("0.12.0", true, 77); + active.committed_clients = CBM_DAEMON_CONTROL_CLIENT_CAP + 3U; + active.client_count = CBM_DAEMON_CONTROL_CLIENT_CAP; + for (uint32_t i = 0; i < CBM_DAEMON_CONTROL_CLIENT_CAP; i++) { + active.client_pids[i] = 1000U + i; + } + char remedy[CBM_DAEMON_CONFLICT_REMEDY_SIZE]; + + ASSERT_TRUE(cbm_daemon_conflict_remedy_format(&active, remedy, sizeof(remedy))); + ASSERT_NOT_NULL(strstr(remedy, "pid 77")); + ASSERT_NOT_NULL(strstr(remedy, "permanent")); + ASSERT_NOT_NULL(strstr(remedy, "1007 and 3 more")); + ASSERT_NOT_NULL(strstr(remedy, "codebase-memory-mcp daemon stop")); + PASS(); +} + +TEST(daemon_conflict_remedy_reports_a_stopping_daemon) { + cbm_daemon_runtime_status_t active = version_test_active_status("0.10.3", false, 99); + active.stopping = true; + char remedy[CBM_DAEMON_CONFLICT_REMEDY_SIZE]; + + ASSERT_TRUE(cbm_daemon_conflict_remedy_format(&active, remedy, sizeof(remedy))); + ASSERT_NOT_NULL(strstr(remedy, "pid 99")); + ASSERT_NOT_NULL(strstr(remedy, "already stopping")); + PASS(); +} + +TEST(daemon_conflict_remedy_without_status_points_at_daemon_status) { + char remedy[CBM_DAEMON_CONFLICT_REMEDY_SIZE]; + + ASSERT_TRUE(cbm_daemon_conflict_remedy_format(NULL, remedy, sizeof(remedy))); + ASSERT_NOT_NULL(strstr(remedy, "codebase-memory-mcp daemon status")); + ASSERT_NOT_NULL(strstr(remedy, "which -a codebase-memory-mcp")); + ASSERT_NOT_NULL(strstr(remedy, "codebase-memory-mcp daemon stop")); + PASS(); +} + +TEST(daemon_conflict_remedy_fails_closed_when_truncated) { + cbm_daemon_runtime_status_t active = version_test_active_status("0.10.3", false, 4242); + char tiny[16]; + + ASSERT_FALSE(cbm_daemon_conflict_remedy_format(&active, tiny, sizeof(tiny))); + ASSERT_STR_EQ(tiny, ""); + ASSERT_FALSE(cbm_daemon_conflict_remedy_format(&active, NULL, 0)); + PASS(); +} + TEST(daemon_hello_rejects_each_abi_mismatch) { cbm_daemon_build_identity_t active = version_test_identity("2.4.0", BUILD_A); cbm_daemon_build_identity_t requested = active; @@ -613,6 +695,11 @@ SUITE(daemon_version) { RUN_TEST(daemon_build_fingerprint_hashes_exact_executable_bytes); RUN_TEST(daemon_hello_accepts_only_the_exact_active_build_identity); RUN_TEST(daemon_hello_version_conflict_exposes_active_and_requested_builds); + RUN_TEST(daemon_conflict_remedy_names_session_managed_daemon_and_its_sessions); + RUN_TEST(daemon_conflict_remedy_names_permanent_daemon_and_truncated_session_list); + RUN_TEST(daemon_conflict_remedy_reports_a_stopping_daemon); + RUN_TEST(daemon_conflict_remedy_without_status_points_at_daemon_status); + RUN_TEST(daemon_conflict_remedy_fails_closed_when_truncated); RUN_TEST(daemon_hello_rejects_each_abi_mismatch); RUN_TEST(daemon_hello_fails_closed_without_an_exact_build_fingerprint); RUN_TEST(daemon_conflict_log_is_durable_private_and_rotates); From dc735c01fbbb4eddbbb73ff4df6007ab68413a4d Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 26 Sep 2026 01:07:11 +0200 Subject: [PATCH 08/10] fix(install): migrate v0.10.8 OpenCode agent profiles on upgrade (#2264) Upgrading from v0.10.8 to v0.11.0 on a machine with OpenCode reported all three OpenCode subagent profiles as "preserved modified profile" plus an op=agent_install error each, and the activation stopped ("one or more agent configurations failed; the published/current executable was kept"), although nobody had edited those files. Root cause: #1933 (v0.11.0) added the tool_search / tool_search_regex permissions to the rendered OpenCode profile. Install and uninstall recognise a profile as ours only when it matches the current rendering or one of the known released renderings (the other access mode, the v0.9.1-rc.1 Codex shape, the pre-tier Verify file). The v0.10.8 OpenCode shape was not in that set, so cbm_text_migrate_owned_document classified the untouched v0.10.8 files as user-modified. Reproduced end to end: install with the v0.10.8 release binary, then install with v0.11.0/main, into an isolated HOME; the three profiles are 1729/1880/1910 bytes, the same sizes as in the report. cbm_render_graph_profile_opencode_v0108() renders the pre-#1933 shape (both access modes), and install/uninstall now list it as a released rendering, so those files are upgraded in place and uninstall removes them as owned. A profile the user actually edited is still preserved. The released-list assembly that install and uninstall duplicated moves into one helper, and the two legacy renderers share one implementation (cli.c drops three raw free sites; the memory-core baseline is tightened 173 -> 170). The Codex and OpenCode op=mcp_install errors in the same report do not reproduce from a pristine v0.10.8 config and depend on the reporter's own config.toml / opencode.json; they are not addressed here. Signed-off-by: Martin Vogel --- scripts/memory-core-baseline.txt | 2 +- src/cli/agent_profiles.c | 50 ++++++++++----- src/cli/agent_profiles.h | 4 ++ src/cli/cli.c | 106 +++++++++++++++++-------------- tests/test_cli.c | 104 ++++++++++++++++++++++++++++++ 5 files changed, 204 insertions(+), 62 deletions(-) diff --git a/scripts/memory-core-baseline.txt b/scripts/memory-core-baseline.txt index fe1cb426cb..602fe59585 100644 --- a/scripts/memory-core-baseline.txt +++ b/scripts/memory-core-baseline.txt @@ -17,7 +17,7 @@ internal/cbm/sqlite_writer.c 74 src/cli/activation_transaction.c 80 src/cli/agent_clients.c 73 src/cli/agent_profiles.c 7 -src/cli/cli.c 173 +src/cli/cli.c 170 src/cli/client_adapter.c 3 src/cli/config_edit_path.c 2 src/cli/config_json_like.c 142 diff --git a/src/cli/agent_profiles.c b/src/cli/agent_profiles.c index 7ae55b82e7..7e72fc5608 100644 --- a/src/cli/agent_profiles.c +++ b/src/cli/agent_profiles.c @@ -498,6 +498,23 @@ static bool append_codex_profile(profile_buffer_t *buffer, cbm_graph_tier_t tier profile_buffer_append(buffer, "]\n"); } +/* before_tool_search reproduces the v0.10.8 rendering: #1933 (v0.11.0) added + * the tool_search / tool_search_regex permissions, and install/uninstall + * must still recognise the older bytes as ours (#2264). */ +static bool append_opencode_profile(profile_buffer_t *buffer, cbm_graph_tier_t tier, + cbm_graph_access_t access, const char *prompt, + bool before_tool_search) { + return profile_buffer_append(buffer, "---\ndescription: ") && + profile_buffer_append(buffer, profile_description(tier, access)) && + profile_buffer_append(buffer, "\nmode: subagent\npermission:\n \"*\": deny\n read: " + "allow\n grep: allow\n glob: allow\n") && + (before_tool_search || + profile_buffer_append(buffer, " tool_search: allow\n tool_search_regex: allow\n")) && + (access != CBM_GRAPH_ACCESS_DIRECT || + append_permission_mcp_tools(buffer, CBM_GRAPH_DIALECT_OPENCODE, tier)) && + profile_buffer_append(buffer, "---\n") && profile_buffer_append(buffer, prompt); +} + static bool render_profile_text(profile_buffer_t *buffer, cbm_graph_profile_dialect_t dialect, cbm_graph_tier_t tier, cbm_graph_access_t access, const char *binary_path, const char *prompt) { @@ -547,17 +564,7 @@ static bool render_profile_text(profile_buffer_t *buffer, cbm_graph_profile_dial } return true; case CBM_GRAPH_DIALECT_OPENCODE: - if (!profile_buffer_append(buffer, "---\ndescription: ") || - !profile_buffer_append(buffer, description) || - !profile_buffer_append( - buffer, "\nmode: subagent\npermission:\n \"*\": deny\n read: allow\n grep: " - "allow\n glob: allow\n tool_search: allow\n tool_search_regex: " - "allow\n") || - (direct && !append_permission_mcp_tools(buffer, dialect, tier)) || - !profile_buffer_append(buffer, "---\n") || !profile_buffer_append(buffer, prompt)) { - return false; - } - return true; + return append_opencode_profile(buffer, tier, access, prompt, false); case CBM_GRAPH_DIALECT_KILO: if (!profile_buffer_append(buffer, "---\ndescription: ") || !profile_buffer_append(buffer, description) || @@ -711,17 +718,22 @@ char *cbm_render_graph_profile(cbm_graph_profile_dialect_t dialect, cbm_graph_ti return profile_buffer_finish(&buffer); } -char *cbm_render_graph_profile_codex_rc1(cbm_graph_tier_t tier) { - if (!tier_valid(tier)) { +/* A released-but-superseded rendering, kept only so install/uninstall can + * recognise and migrate files an older release wrote. */ +static char *render_released_profile(cbm_graph_profile_dialect_t dialect, cbm_graph_tier_t tier, + cbm_graph_access_t access) { + if (!tier_valid(tier) || !access_valid(access)) { return NULL; } - char *prompt = cbm_render_graph_prompt(tier, CBM_GRAPH_ACCESS_DIRECT); + char *prompt = cbm_render_graph_prompt(tier, access); if (!prompt) { return NULL; } profile_buffer_t buffer; profile_buffer_init(&buffer); - bool ok = append_codex_profile(&buffer, tier, CBM_GRAPH_ACCESS_DIRECT, NULL, prompt, true); + bool ok = dialect == CBM_GRAPH_DIALECT_OPENCODE + ? append_opencode_profile(&buffer, tier, access, prompt, true) + : append_codex_profile(&buffer, tier, access, NULL, prompt, true); free(prompt); if (!ok) { profile_buffer_discard(&buffer); @@ -729,3 +741,11 @@ char *cbm_render_graph_profile_codex_rc1(cbm_graph_tier_t tier) { } return profile_buffer_finish(&buffer); } + +char *cbm_render_graph_profile_opencode_v0108(cbm_graph_tier_t tier, cbm_graph_access_t access) { + return render_released_profile(CBM_GRAPH_DIALECT_OPENCODE, tier, access); +} + +char *cbm_render_graph_profile_codex_rc1(cbm_graph_tier_t tier) { + return render_released_profile(CBM_GRAPH_DIALECT_CODEX, tier, CBM_GRAPH_ACCESS_DIRECT); +} diff --git a/src/cli/agent_profiles.h b/src/cli/agent_profiles.h index 184b03bb19..aa85c665f1 100644 --- a/src/cli/agent_profiles.h +++ b/src/cli/agent_profiles.h @@ -60,6 +60,10 @@ char *cbm_render_graph_profile(cbm_graph_profile_dialect_t dialect, cbm_graph_ti * so install/uninstall can recognize and migrate those files. */ char *cbm_render_graph_profile_codex_rc1(cbm_graph_tier_t tier); +/* v0.10.8 OpenCode rendering (before the tool_search permissions of #1933), + * kept so install/uninstall can recognize and migrate those files (#2264). */ +char *cbm_render_graph_profile_opencode_v0108(cbm_graph_tier_t tier, cbm_graph_access_t access); + /* Vibe stores the behavioral prompt separately from its TOML agent definition. * Other integrations may also use this as the canonical contract text. */ char *cbm_render_graph_prompt(cbm_graph_tier_t tier, cbm_graph_access_t access); diff --git a/src/cli/cli.c b/src/cli/cli.c index b868fdca25..6e33759e4a 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -8717,6 +8717,54 @@ static cbm_graph_access_t cbm_tiered_profile_set_access(cbm_tiered_profile_set_t : CBM_GRAPH_ACCESS_HANDOFF; } +/* Every earlier rendering of one tier's profile that install/uninstall must + * still recognise as ours: the other access mode, the v0.9.1-rc.1 Codex + * shape, the pre-tier Verify file, and the v0.10.8 OpenCode shape (#2264). */ +typedef struct { + char *owned[4]; + size_t owned_count; + const char *content[5]; + size_t count; +} tiered_released_profiles_t; + +static void tiered_released_profiles_keep(tiered_released_profiles_t *released, char *rendered) { + if (rendered) { + released->owned[released->owned_count++] = rendered; + released->content[released->count++] = rendered; + } +} + +static void tiered_released_profiles_collect(cbm_tiered_profile_set_t profiles, + cbm_graph_tier_t tier, cbm_graph_access_t access, + tiered_released_profiles_t *released) { + memset(released, 0, sizeof(*released)); + cbm_graph_access_t alternate_access = + access == CBM_GRAPH_ACCESS_DIRECT ? CBM_GRAPH_ACCESS_HANDOFF : CBM_GRAPH_ACCESS_DIRECT; + tiered_released_profiles_keep( + released, + cbm_render_graph_profile(profiles.dialect, tier, alternate_access, profiles.binary_path)); + if (profiles.dialect == CBM_GRAPH_DIALECT_CODEX && access == CBM_GRAPH_ACCESS_DIRECT) { + tiered_released_profiles_keep(released, cbm_render_graph_profile_codex_rc1(tier)); + } + if (profiles.dialect == CBM_GRAPH_DIALECT_OPENCODE) { + tiered_released_profiles_keep(released, + cbm_render_graph_profile_opencode_v0108(tier, access)); + tiered_released_profiles_keep( + released, cbm_render_graph_profile_opencode_v0108(tier, alternate_access)); + } + if (tier == CBM_GRAPH_TIER_VERIFY && profiles.legacy_verify_content) { + released->content[released->count++] = profiles.legacy_verify_content; + } +} + +static void tiered_released_profiles_free(tiered_released_profiles_t *released) { + for (size_t i = 0U; i < released->owned_count; i++) { + free(released->owned[i]); + } + released->owned_count = 0U; + released->count = 0U; +} + static void install_tiered_agent_profiles(cbm_tiered_profile_set_t profiles, bool dry_run) { cbm_graph_access_t access = cbm_tiered_profile_set_access(profiles); for (int value = 0; value < (int)CBM_GRAPH_TIER_COUNT; value++) { @@ -8740,30 +8788,13 @@ static void install_tiered_agent_profiles(cbm_tiered_profile_set_t profiles, boo record_agent_config_error(false, profiles.label, "agent_render", path); continue; } - cbm_graph_access_t alternate_access = - access == CBM_GRAPH_ACCESS_DIRECT ? CBM_GRAPH_ACCESS_HANDOFF : CBM_GRAPH_ACCESS_DIRECT; - char *alternate = cbm_render_graph_profile(profiles.dialect, tier, alternate_access, - profiles.binary_path); - char *codex_rc1 = - profiles.dialect == CBM_GRAPH_DIALECT_CODEX && access == CBM_GRAPH_ACCESS_DIRECT - ? cbm_render_graph_profile_codex_rc1(tier) - : NULL; - const char *released[3]; - size_t released_count = 0U; - if (alternate) { - released[released_count++] = alternate; - } - if (codex_rc1) { - released[released_count++] = codex_rc1; - } - if (tier == CBM_GRAPH_TIER_VERIFY && profiles.legacy_verify_content) { - released[released_count++] = profiles.legacy_verify_content; - } - int result = prepare_config_parent(path) - ? cbm_text_migrate_owned_document(path, current, released, released_count) - : CLI_ERR; - free(codex_rc1); - free(alternate); + tiered_released_profiles_t released; + tiered_released_profiles_collect(profiles, tier, access, &released); + int result = + prepare_config_parent(path) + ? cbm_text_migrate_owned_document(path, current, released.content, released.count) + : CLI_ERR; + tiered_released_profiles_free(&released); free(current); if (result != CLI_OK) { if (result > CLI_OK) { @@ -8795,28 +8826,11 @@ static void uninstall_tiered_agent_profiles(cbm_tiered_profile_set_t profiles, b record_agent_config_error(true, profiles.label, "agent_render", path); continue; } - cbm_graph_access_t alternate_access = - access == CBM_GRAPH_ACCESS_DIRECT ? CBM_GRAPH_ACCESS_HANDOFF : CBM_GRAPH_ACCESS_DIRECT; - char *alternate = cbm_render_graph_profile(profiles.dialect, tier, alternate_access, - profiles.binary_path); - char *codex_rc1 = - profiles.dialect == CBM_GRAPH_DIALECT_CODEX && access == CBM_GRAPH_ACCESS_DIRECT - ? cbm_render_graph_profile_codex_rc1(tier) - : NULL; - const char *released[3]; - size_t released_count = 0U; - if (alternate) { - released[released_count++] = alternate; - } - if (codex_rc1) { - released[released_count++] = codex_rc1; - } - if (tier == CBM_GRAPH_TIER_VERIFY && profiles.legacy_verify_content) { - released[released_count++] = profiles.legacy_verify_content; - } - int result = cbm_text_remove_owned_document_any(path, current, released, released_count); - free(codex_rc1); - free(alternate); + tiered_released_profiles_t released; + tiered_released_profiles_collect(profiles, tier, access, &released); + int result = + cbm_text_remove_owned_document_any(path, current, released.content, released.count); + tiered_released_profiles_free(&released); free(current); if (result < CLI_OK) { record_agent_config_error(true, profiles.label, "agent_uninstall", path); diff --git a/tests/test_cli.c b/tests/test_cli.c index 4f95e9c43a..6306504aa5 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -7412,6 +7412,109 @@ TEST(cli_tiered_codex_profiles_migrate_preserve_and_uninstall) { PASS(); } +/* The OpenCode subagent profile as v0.10.8 wrote it: today's rendering minus + * the tool_search / tool_search_regex permission lines #1933 added in + * v0.11.0. Derived by deleting exactly those lines, so the test states the + * released bytes without going through any production legacy renderer. */ +static char *opencode_profile_before_tool_search(cbm_graph_tier_t tier) { + char *current = + cbm_render_graph_profile(CBM_GRAPH_DIALECT_OPENCODE, tier, CBM_GRAPH_ACCESS_DIRECT, NULL); + if (!current) { + return NULL; + } + const char *const added[] = {" tool_search: allow\n", " tool_search_regex: allow\n"}; + for (size_t i = 0U; i < sizeof(added) / sizeof(added[0]); i++) { + char *at = strstr(current, added[i]); + if (!at) { + free(current); + return NULL; + } + size_t len = strlen(added[i]); + memmove(at, at + len, strlen(at + len) + 1U); + } + return current; +} + +/* #2264: upgrading v0.10.8 -> v0.11.0 reported all three OpenCode profiles + * as "preserved modified profile" + op=agent_install errors and stopped the + * activation, although nobody had edited them: the release render changed + * and the previous render was not recognised as ours. */ +TEST(cli_tiered_opencode_profiles_migrate_v0108_render) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-tiered-opencode-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *const env_names[] = { + "HOME", "PATH", "CODEX_HOME", "QWEN_HOME", "COPILOT_HOME", + "CLINE_DATA_DIR", "KIRO_HOME", "OPENCODE_CONFIG", "OPENCODE_CONFIG_DIR"}; + char *saved_env[sizeof(env_names) / sizeof(env_names[0])]; + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + saved_env[i] = save_test_env(env_names[i]); + cbm_unsetenv(env_names[i]); + } + cbm_setenv("HOME", tmpdir, 1); + cbm_setenv("PATH", tmpdir, 1); + + char agents_dir[512]; + snprintf(agents_dir, sizeof(agents_dir), "%s/.config/opencode/agents", tmpdir); + test_mkdirp(agents_dir); + char paths[CBM_GRAPH_TIER_COUNT][640]; + bool seeded = true; + for (int t = 0; t < (int)CBM_GRAPH_TIER_COUNT; t++) { + cbm_graph_tier_t tier = (cbm_graph_tier_t)t; + snprintf(paths[t], sizeof(paths[t]), "%s/%s.md", agents_dir, cbm_graph_tier_slug(tier)); + char *old = opencode_profile_before_tool_search(tier); + seeded = seeded && old != NULL; + if (old) { + write_test_file(paths[t], old); + free(old); + } + } + + char installed_binary[640]; + snprintf(installed_binary, sizeof(installed_binary), "%s/.local/bin/codebase-memory-mcp", + tmpdir); + int install_rc = cbm_install_agent_configs(tmpdir, installed_binary, false, false); + bool migrated = true; + for (int t = 0; t < (int)CBM_GRAPH_TIER_COUNT; t++) { + char *after = read_test_file_alloc(paths[t]); + migrated = migrated && after && strstr(after, " tool_search: allow\n") && + strstr(after, " tool_search_regex: allow\n"); + free(after); + } + + /* Uninstall must also recognise the v0.10.8 bytes as ours. */ + for (int t = 0; t < (int)CBM_GRAPH_TIER_COUNT; t++) { + char *old = opencode_profile_before_tool_search((cbm_graph_tier_t)t); + if (old) { + write_test_file(paths[t], old); + free(old); + } + } + char *argv[] = {"uninstall", "--yes"}; + int uninstall_rc = cli_test_cmd_uninstall(2, argv); + bool removed = true; + struct stat state; + for (int t = 0; t < (int)CBM_GRAPH_TIER_COUNT; t++) { + removed = removed && stat(paths[t], &state) != 0; + } + + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + restore_test_env(env_names[i], saved_env[i]); + } + test_rmdir_r(tmpdir); + if (!seeded) + FAIL("v0.10.8 OpenCode profile bytes could not be derived"); + ASSERT_EQ(install_rc, 0); + if (!migrated) + FAIL("v0.10.8 OpenCode profiles must be upgraded in place, not preserved as modified"); + ASSERT_EQ(uninstall_rc, 0); + if (!removed) + FAIL("uninstall must remove v0.10.8 OpenCode profiles as owned"); + PASS(); +} + TEST(cli_tiered_vibe_installs_matching_agent_prompt_sets) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-tiered-vibe-XXXXXX"); @@ -16428,6 +16531,7 @@ SUITE(cli) { RUN_TEST(cli_warp_installs_shared_skill_without_mcp_or_permissions); RUN_TEST(cli_owned_durable_profiles_preserve_user_files); RUN_TEST(cli_tiered_codex_profiles_migrate_preserve_and_uninstall); + RUN_TEST(cli_tiered_opencode_profiles_migrate_v0108_render); RUN_TEST(cli_tiered_vibe_installs_matching_agent_prompt_sets); RUN_TEST(cli_tiered_grok_installs_profiles_and_withholds_hooks); RUN_TEST(cli_grok_mcp_preserves_foreign_table); From ea6fa49fc2c209de69c1b0f393d9d2cc25b61419 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 28 Sep 2026 00:42:58 +0200 Subject: [PATCH 09/10] chore(security): refresh vendored checksums for the #2176 ReScript scanner patch security-static (scripts/security-vendored.sh) failed on this branch: the #2176 fix patches internal/cbm/vendored/grammars/rescript/scanner.c (skip the template-string branch in the error-recovery state) and records that patch in vendored/grammars/MANIFEST.md, but the checked-in digests in scripts/vendored-checksums.txt still described the unpatched files. Both changes are intentional and reviewed in the parent commit; this records their SHA-256 via `scripts/security-vendored.sh --update`. Only the two affected lines change, and the integrity check passes again. Signed-off-by: Martin Vogel --- scripts/vendored-checksums.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/vendored-checksums.txt b/scripts/vendored-checksums.txt index 176f96d1e4..b561de94e2 100644 --- a/scripts/vendored-checksums.txt +++ b/scripts/vendored-checksums.txt @@ -5,7 +5,7 @@ c5cfb43042b6b72045f4ba997834d0a7786d2793d91680868b5815b39f14fc78 internal/cbm/v b29c1c9fb7cc82f58c84b376df1297d6e2737a1d655fd356db0859e3c29c2fea internal/cbm/vendored/common/tree_sitter/alloc.h 31e60a1bff6f715afacce03b5b70efe42b58371b4f9595dd4af52a577ff9608c internal/cbm/vendored/common/tree_sitter/array.h 180b893c8734778fd32f372dfbc27bd6ad1cd2221f26150b31256ff6716320d2 internal/cbm/vendored/common/tree_sitter/parser.h -a66a04ed735fdb3d4a90ee4353b8f0490dc0360da5965b0d13ec5d6766f146fa internal/cbm/vendored/grammars/MANIFEST.md +9fe439eff24b2a474618f2cb75cbeb06074840f0396fbeb1d2503e668a5d056a internal/cbm/vendored/grammars/MANIFEST.md ad8425038de519f8c4e3e9339feebf99dfad8a6002dcf79227d91402780a32cc internal/cbm/vendored/grammars/ada/LICENSE 02805ec13939b749c891567be36bf024b09034e04c80683a1ae667272458d549 internal/cbm/vendored/grammars/ada/parser.c 115a75d000bef9c70c4de7dfbb7f2a80b90cbb14ba265056dc7abb1ef3b9b6db internal/cbm/vendored/grammars/ada/tree_sitter/alloc.h @@ -667,7 +667,7 @@ c4b9482069f61a2a26a590181baa34f08eb6dcc57a138def578e6f9720c35ea5 internal/cbm/v c4b9482069f61a2a26a590181baa34f08eb6dcc57a138def578e6f9720c35ea5 internal/cbm/vendored/grammars/requirements/tree_sitter/parser.h 21da9b4e463af4b4501cbd4e0c5886a1f321a740b584002a9c9a5fe7b2d46a8a internal/cbm/vendored/grammars/rescript/LICENSE d75538517e1024056c206f6d1b10cb4ed83670a5d6fb338dbad7b2bb4269403a internal/cbm/vendored/grammars/rescript/parser.c -4bf614fc0b599bf75414c1512e92c971d4bb04d0334e5f562ff34a0dbd745f8a internal/cbm/vendored/grammars/rescript/scanner.c +d550ab7d815666f14b938e805f106a2ce4d99f6f86377d7a301dcf076db5f138 internal/cbm/vendored/grammars/rescript/scanner.c 115a75d000bef9c70c4de7dfbb7f2a80b90cbb14ba265056dc7abb1ef3b9b6db internal/cbm/vendored/grammars/rescript/tree_sitter/alloc.h 02d139d44303b0b0b85a899839dbc1346c478e7318d1f2971fad2b38fd57cb73 internal/cbm/vendored/grammars/rescript/tree_sitter/array.h c4b9482069f61a2a26a590181baa34f08eb6dcc57a138def578e6f9720c35ea5 internal/cbm/vendored/grammars/rescript/tree_sitter/parser.h From 259dbb67bee1d3547ae61a98c9d0c1cf4e491d44 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 28 Sep 2026 00:42:58 +0200 Subject: [PATCH 10/10] test(sql): make the #1735 tests portable and clock-free Two CI failures, both in the tests this branch added: 1. Windows (MinGW clang64) did not compile the test runner: test_parse_coverage.c and test_pipeline.c called POSIX setenv/unsetenv, which the Windows CRT does not declare. They now use the repo's cbm_setenv/cbm_unsetenv from foundation/compat.h, as the other suites do. 2. sql_dump_of_many_megabytes_is_indexed_not_timed_out_issue1735 failed on the ubuntu-24.04-arm ASan leg with "parse timeout". It extracted a 26 MB dump under CBM_EXTRACT_BUDGET, which is CPU time, so the verdict depended on runner speed. It passed on the fast legs and tripped on the slow one, and the early FAIL return leaked the result (the LSan report in that log). The property behind "indexed, not timed out" is a count: parse work is set by the statements, not the rows. The test now extracts unbudgeted, asserts the outcome (the view and the subquery usage are in the graph), and bounds the tree by the same 330 statements with 4 rows each. Results are freed before any assertion. Binds to the fix: forcing the full parse for the big dump through the CBM_TEST_SQL_FULL_PARSE_ON seam fails the bound (5,158,222 tree nodes vs a bound of 52,872); with the row exclusion it passes. Signed-off-by: Martin Vogel --- tests/test_parse_coverage.c | 42 ++++++++++++++++++++++++------------- tests/test_pipeline.c | 4 ++-- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 91f99f95d3..c8ed901140 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -30,8 +30,8 @@ #include "test_framework.h" #include "cbm.h" -#include "sql_values.h" /* #1735 value-row scanner */ -#include "pipeline/pipeline_internal.h" /* CBM_EXTRACT_BUDGET */ +#include "sql_values.h" /* #1735 value-row scanner */ +#include "foundation/compat.h" /* cbm_setenv / cbm_unsetenv */ #include #include #include @@ -1241,9 +1241,9 @@ static int count_calls_named(const CBMFileResult *r, const char *name) { /* The same source parsed with the row exclusion and without it (test seam). */ static CBMFileResult *extract_sql_full(const char *src, const char *path) { - setenv("CBM_TEST_SQL_FULL_PARSE_ON", path, 1); + cbm_setenv("CBM_TEST_SQL_FULL_PARSE_ON", path, 1); CBMFileResult *r = do_extract(src, CBM_LANG_SQL, path); - unsetenv("CBM_TEST_SQL_FULL_PARSE_ON"); + cbm_unsetenv("CBM_TEST_SQL_FULL_PARSE_ON"); return r; } @@ -1318,23 +1318,35 @@ TEST(sql_dump_parse_does_not_grow_with_the_row_count_issue1735) { } TEST(sql_dump_of_many_megabytes_is_indexed_not_timed_out_issue1735) { - /* ~26 MB with MySQL escapes, extracted under the production parse budget. - * The outcome is asserted — the file is indexed and its tables are in the - * graph — never how long it took. */ + /* ~26 MB with MySQL escapes. The production parse budget is CPU time, so + * extracting under it made the verdict a function of runner speed (an + * ASan arm leg tripped it). The property behind "indexed, not timed out" + * is a count: the parse work is set by the statements, not by the rows. + * So: extract unbudgeted, assert the outcome, and bound the tree by the + * same 330 statements carrying only a handful of rows each (the full + * parse built a tree node for every row token). */ char *src = sql_dump(330, 2000, true, true); + char *few = sql_dump(330, 4, true, true); ASSERT_NOT_NULL(src); + ASSERT_NOT_NULL(few); size_t len = strlen(src); ASSERT_GT(len, (size_t)24 * 1024 * 1024); - CBMFileResult *r = cbm_extract_file(src, (int)len, CBM_LANG_SQL, "covproj", "world.sql", - CBM_EXTRACT_BUDGET, NULL, NULL); + CBMFileResult *r = + cbm_extract_file(src, (int)len, CBM_LANG_SQL, "covproj", "world.sql", 0, NULL, NULL); + CBMFileResult *rf = + cbm_extract_file(few, (int)strlen(few), CBM_LANG_SQL, "covproj", "few.sql", 0, NULL, NULL); + free(src); + free(few); ASSERT_NOT_NULL(r); - if (r->has_error) { - FAIL(r->error_msg ? r->error_msg : "extraction failed"); - } - ASSERT_TRUE(has_def(r, "big_cities")); - ASSERT_TRUE(has_usage_named(r, "country")); + ASSERT_NOT_NULL(rf); + bool indexed = !r->has_error && has_def(r, "big_cities") && has_usage_named(r, "country"); + uint32_t nodes = r->tree_nodes; + uint32_t few_nodes = rf->tree_nodes; cbm_free_result(r); - free(src); + cbm_free_result(rf); + ASSERT_TRUE(indexed); + ASSERT_GT(few_nodes, 0); + ASSERT_LTE(nodes, 2 * few_nodes); PASS(); } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 6618a4a5db..eb83f50034 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -479,9 +479,9 @@ TEST(pipeline_sql_dump_graph_matches_the_full_parse_issue1735) { free(src); char *cut = index_and_list(tmp, "cut.db"); - setenv("CBM_TEST_SQL_FULL_PARSE_ON", "dump.sql", 1); + cbm_setenv("CBM_TEST_SQL_FULL_PARSE_ON", "dump.sql", 1); char *full = index_and_list(tmp, "full.db"); - unsetenv("CBM_TEST_SQL_FULL_PARSE_ON"); + cbm_unsetenv("CBM_TEST_SQL_FULL_PARSE_ON"); th_rmtree(tmp); ASSERT_NOT_NULL(cut); ASSERT_NOT_NULL(full);