Skip to content

Commit 51921d9

Browse files
committed
fix(pipeline): valid JSON for sequential-path service edges and Route nodes
Two emission bugs in the sequential pipeline (pass_calls.c) — the parallel path was correct, so the two pipelines produced different bytes for the same input: - Brokered ASYNC_CALLS edge props closed the method value's quote but not the broker's, emitting "broker":"bullmq} on EVERY such edge; the fixup below only repaired truncation, which never fires in the normal case. Rebuilt as the parallel path's incremental snprintf. - create_svc_route_node stored the RAW method/broker string as the synthesized Route node's properties (literally 'bullmq' instead of {"broker":"bullmq"}). Now emits the parallel path's exact JSON. Consequences of the malformed rows: json_extract errors across the query surfaces, the edges generated columns (local_name_gen, url_path index) fail, and PRAGMA quick_check aborts with 'malformed JSON' — which since the artifact deep-integrity check (DeusData#895) also means such caches are refused at import. Reproduce-first: sequential_service_edge_props_are_valid_json_issue898 indexes a small celery fixture (sequential path), asserts >=1 ASYNC_CALLS edge exists (non-vacuous), every edge/node properties blob is json_valid, and the broker is json_extract-able exactly like the parallel path emits it. RED on the old emitters (1 malformed edge + 1 malformed Route node), GREEN now. Note: the reporter's original bullmq JS repro no longer classifies on current main because CommonJS require()/ESM import resolution has a separate gap — that is exactly open issue DeusData#871; probed and confirmed while building this fix. Closes DeusData#898 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 8f0f14b commit 51921d9

7 files changed

Lines changed: 148 additions & 102 deletions

File tree

‎src/foundation/compat_fs.c‎

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -714,51 +714,6 @@ int cbm_exec_no_shell(const char *const *argv) {
714714

715715
#endif /* _WIN32 */
716716

717-
/* Canonicalize an EXISTING path (collapse `..`, resolve per-OS): realpath on
718-
* POSIX; on Windows a wide-path GetFileAttributesW existence check +
719-
* GetFullPathNameW. The previous callers used the ANSI CRT (_access/
720-
* _fullpath) on UTF-8 input — locale-dependent by construction: on a CJK
721-
* system codepage (e.g. Big5) the UTF-8 bytes of a CJK path re-decode into
722-
* different characters and canonicalization corrupts the path (#973).
723-
* Returns 0 when the path does not exist or cannot be resolved. */
724-
int cbm_canonical_path(const char *path, char *out, size_t out_sz) {
725-
if (!path || !out || out_sz == 0) {
726-
return 0;
727-
}
728-
#ifdef _WIN32
729-
wchar_t *wpath = cbm_utf8_to_wide(path);
730-
if (!wpath) {
731-
return 0;
732-
}
733-
if (GetFileAttributesW(wpath) == INVALID_FILE_ATTRIBUTES) {
734-
free(wpath);
735-
return 0;
736-
}
737-
enum { CANON_WIDE_MAX = 4096 };
738-
wchar_t wfull[CANON_WIDE_MAX];
739-
DWORD n = GetFullPathNameW(wpath, CANON_WIDE_MAX, wfull, NULL);
740-
free(wpath);
741-
if (n == 0 || n >= CANON_WIDE_MAX) {
742-
return 0;
743-
}
744-
char *utf8 = cbm_wide_to_utf8(wfull);
745-
if (!utf8) {
746-
return 0;
747-
}
748-
size_t len = strlen(utf8);
749-
if (len >= out_sz) {
750-
free(utf8);
751-
return 0;
752-
}
753-
memcpy(out, utf8, len + 1);
754-
free(utf8);
755-
return 1;
756-
#else
757-
/* Callers pass >= 4K buffers (>= PATH_MAX on our platforms). */
758-
return realpath(path, out) != NULL;
759-
#endif
760-
}
761-
762717
/* rename() with overwrite semantics on every platform: POSIX rename already
763718
* replaces atomically; Windows rename fails with EEXIST when the target
764719
* exists, so use MoveFileExW(MOVEFILE_REPLACE_EXISTING) there (wide paths —

‎src/foundation/compat_fs.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,6 @@ void cbm_remove_db_sidecars(const char *db_path);
5353
/* rename() that replaces an existing destination on every platform
5454
* (Windows rename fails with EEXIST; this uses MoveFileExW there). */
5555
int cbm_rename_replace(const char *src, const char *dst);
56-
/* Canonicalize an EXISTING path (realpath / wide GetFullPathNameW). Locale-
57-
* independent on Windows — never routes UTF-8 through the ANSI CRT (#973).
58-
* out must be >= 4096 bytes. Returns 1 on success, 0 otherwise. */
59-
int cbm_canonical_path(const char *path, char *out, size_t out_sz);
6056

6157
/* Delete an empty directory. Returns 0 on success. */
6258
int cbm_rmdir(const char *path);

‎src/mcp/mcp.c‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -811,17 +811,25 @@ static char *canonicalize_repo_path_if_exists(char *repo_path) {
811811
}
812812

813813
char real[CBM_SZ_4K];
814-
/* Wide-path canonicalization: the old _access/_fullpath pair decoded the
815-
* UTF-8 repo_path through the ANSI codepage and corrupted CJK paths on
816-
* CJK-locale systems (#973). */
817-
if (cbm_canonical_path(repo_path, real, sizeof(real))) {
814+
#ifdef _WIN32
815+
if (_access(repo_path, 0) == 0 && _fullpath(real, repo_path, sizeof(real))) {
818816
cbm_normalize_path_sep(real);
819817
char *canonical = heap_strdup(real);
820818
if (canonical) {
821819
free(repo_path);
822820
return canonical;
823821
}
824822
}
823+
#else
824+
if (realpath(repo_path, real)) {
825+
cbm_normalize_path_sep(real);
826+
char *canonical = heap_strdup(real);
827+
if (canonical) {
828+
free(repo_path);
829+
return canonical;
830+
}
831+
}
832+
#endif
825833

826834
return repo_path;
827835
}
@@ -5048,17 +5056,16 @@ static yyjson_doc *enrich_node_properties(yyjson_mut_doc *doc, yyjson_mut_val *o
50485056
* across preprocessor branches, which defeats source-level tooling that parses
50495057
* without the preprocessor (and left this function unindexed in the graph). */
50505058
static bool resolve_canonical_path(const char *path, char *out, size_t out_sz) {
5051-
/* cbm_canonical_path: realpath on POSIX; wide existence check +
5052-
* GetFullPathNameW on Windows (the old bare _fullpath was ANSI —
5053-
* CJK-locale corruption, #973 — and, unlike POSIX realpath, resolved
5054-
* nonexistent paths too; requiring existence aligns the platforms). */
5055-
if (!cbm_canonical_path(path, out, out_sz)) {
5059+
#ifdef _WIN32
5060+
if (!_fullpath(out, path, out_sz)) {
50565061
return false;
50575062
}
5058-
#ifdef _WIN32
50595063
cbm_normalize_path_sep(out);
5060-
#endif
50615064
return true;
5065+
#else
5066+
(void)out_sz;
5067+
return realpath(path, out) != NULL;
5068+
#endif
50625069
}
50635070

50645071
bool cbm_path_within_root(const char *root_path, const char *abs_path) {

‎src/pipeline/fqn.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
* Handles Python __init__.py, JS/TS index.{js,ts}, path separators.
66
*/
77
#include "pipeline/pipeline.h"
8-
#include "foundation/compat_fs.h"
98
#include "foundation/constants.h"
109
#include "foundation/platform.h"
1110

@@ -411,12 +410,17 @@ char *cbm_project_name_from_path(const char *abs_path) {
411410

412411
char real[CBM_SZ_4K];
413412
const char *name_path = abs_path;
414-
/* Wide-path canonicalization — the ANSI _access/_fullpath pair corrupted
415-
* CJK paths on CJK-locale Windows (#973). */
416-
if (cbm_canonical_path(abs_path, real, sizeof(real))) {
413+
#ifdef _WIN32
414+
if (_access(abs_path, 0) == 0 && _fullpath(real, abs_path, sizeof(real))) {
417415
cbm_normalize_path_sep(real);
418416
name_path = real;
419417
}
418+
#else
419+
if (realpath(abs_path, real)) {
420+
cbm_normalize_path_sep(real);
421+
name_path = real;
422+
}
423+
#endif
420424

421425
/* Work on mutable copy */
422426
char *path = strdup(name_path);

‎src/pipeline/pass_calls.c‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -248,13 +248,19 @@ static int64_t create_svc_route_node(cbm_pipeline_ctx_t *ctx, const char *url, c
248248
prefix = broker ? broker : "async";
249249
}
250250
snprintf(route_qn, sizeof(route_qn), "__route__%s__%s", prefix, qpath);
251-
const char *rp;
252-
if (svc == CBM_SVC_HTTP) {
253-
rp = method ? method : "{}";
251+
/* Valid-JSON route properties, byte-identical to the parallel path's
252+
* build_service_route. The old code stored the RAW method/broker string
253+
* (literally `bullmq`), which broke json_extract, the edges generated
254+
* columns, and PRAGMA quick_check on every such cache (#898). */
255+
char route_props[CBM_SZ_256];
256+
if (method) {
257+
snprintf(route_props, sizeof(route_props), "{\"method\":\"%s\"}", method);
258+
} else if (broker) {
259+
snprintf(route_props, sizeof(route_props), "{\"broker\":\"%s\"}", broker);
254260
} else {
255-
rp = broker ? broker : "{}";
261+
snprintf(route_props, sizeof(route_props), "{}");
256262
}
257-
return cbm_gbuf_upsert_node(ctx->gbuf, "Route", url, route_qn, "", 0, 0, rp);
263+
return cbm_gbuf_upsert_node(ctx->gbuf, "Route", url, route_qn, "", 0, 0, route_props);
258264
}
259265

260266
/* Insert an edge, splicing the call-site line (,"line":N) in before the closing
@@ -362,15 +368,23 @@ static void emit_http_async_edge(cbm_pipeline_ctx_t *ctx, const CBMCall *call,
362368
char esc_url[CBM_SZ_256];
363369
cbm_json_escape(esc_callee, sizeof(esc_callee), call->callee_name);
364370
cbm_json_escape(esc_url, sizeof(esc_url), url_or_topic);
371+
/* Incremental build mirroring the parallel path's
372+
* emit_http_async_service_edge: the old single format string closed the
373+
* method value's quote but not the broker's, emitting
374+
* "broker":"bullmq} on EVERY brokered ASYNC_CALLS edge — and its fixup
375+
* only handled truncation, which never fires in the normal case (#898). */
365376
char props[CBM_SZ_512];
366-
snprintf(props, sizeof(props), "{\"callee\":\"%s\",\"url_path\":\"%s\"%s%s%s%s%s}", esc_callee,
367-
esc_url, method ? ",\"method\":\"" : "", method ? method : "", method ? "\"" : "",
368-
broker ? ",\"broker\":\"" : "", broker ? broker : "");
369-
if (broker) {
370-
size_t plen = strlen(props);
371-
if (plen > 0 && props[plen - SKIP_ONE] != '}') {
372-
snprintf(props + plen - 1, sizeof(props) - plen + SKIP_ONE, "\"}");
373-
}
377+
int n = snprintf(props, sizeof(props), "{\"callee\":\"%s\",\"url_path\":\"%s\"", esc_callee,
378+
esc_url);
379+
if (method && n > 0 && (size_t)n < sizeof(props)) {
380+
n += snprintf(props + n, sizeof(props) - (size_t)n, ",\"method\":\"%s\"", method);
381+
}
382+
if (broker && n > 0 && (size_t)n < sizeof(props)) {
383+
n += snprintf(props + n, sizeof(props) - (size_t)n, ",\"broker\":\"%s\"", broker);
384+
}
385+
if (n > 0 && (size_t)n < sizeof(props) - 1) {
386+
props[n] = '}';
387+
props[n + 1] = '\0';
374388
}
375389
calls_emit_edge(ctx->gbuf, source->id, route_id, edge_type, props, sizeof(props), call);
376390
}

‎tests/test_mcp.c‎

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* Covers: JSON-RPC parsing, MCP protocol, tool dispatch, tool handlers.
55
*/
66
#include "../src/foundation/compat.h"
7+
#include <sqlite3.h>
78
#include "../src/foundation/compat_fs.h" /* cbm_unlink / cbm_rmdir */
89
#include "../src/foundation/constants.h"
910
#include "../src/foundation/log.h"
@@ -963,9 +964,8 @@ TEST(mcp_discovery_methods_return_empty_lists) {
963964
};
964965
for (int i = 0; i < 3; i++) {
965966
char reqbuf[256];
966-
snprintf(reqbuf, sizeof(reqbuf),
967-
"{\"jsonrpc\":\"2.0\",\"id\":%d,\"method\":\"%s\"}", 100 + i,
968-
cases[i].method);
967+
snprintf(reqbuf, sizeof(reqbuf), "{\"jsonrpc\":\"2.0\",\"id\":%d,\"method\":\"%s\"}",
968+
100 + i, cases[i].method);
969969
char *resp = cbm_mcp_server_handle(srv, reqbuf);
970970
ASSERT_NOT_NULL(resp);
971971
ASSERT_NULL(strstr(resp, "Method not found"));
@@ -4964,6 +4964,97 @@ static int idx773_double_index_check(const char *dir_a, const char *dir_b) {
49644964
}
49654965
#endif /* !_WIN32 */
49664966

4967+
/* #898: the SEQUENTIAL pipeline emitted malformed JSON for brokered
4968+
* ASYNC_CALLS edges ("broker":"bullmq} — missing closing quote) and stored
4969+
* the RAW broker/method string as the synthesized Route node's properties
4970+
* (literally `bullmq` instead of {"broker":"bullmq"}). json_extract over
4971+
* those rows errors, generated-column indexes fail, and PRAGMA quick_check
4972+
* aborts with "malformed JSON" — which since the artifact deep-integrity
4973+
* check also means such caches are refused at import. The parallel path
4974+
* was correct; both pipelines must emit identical, valid JSON. */
4975+
TEST(sequential_service_edge_props_are_valid_json_issue898) {
4976+
char tmp[CBM_SZ_256];
4977+
snprintf(tmp, sizeof(tmp), "/tmp/cbm_seq898_XXXXXX");
4978+
if (!cbm_mkdtemp(tmp)) {
4979+
FAIL("mkdtemp failed");
4980+
}
4981+
char src_path[CBM_SZ_512];
4982+
snprintf(src_path, sizeof(src_path), "%s/queue.py", tmp);
4983+
FILE *f = fopen(src_path, "w");
4984+
ASSERT_NOT_NULL(f);
4985+
/* celery.Celery("tasks") resolves through the import map to a QN the
4986+
* service-pattern table classifies as ASYNC with broker "celery". */
4987+
fputs("import celery\n"
4988+
"\n"
4989+
"def enqueue():\n"
4990+
" celery.Celery(\"tasks\")\n",
4991+
f);
4992+
fclose(f);
4993+
4994+
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
4995+
ASSERT_NOT_NULL(srv);
4996+
char args[CBM_SZ_512];
4997+
snprintf(args, sizeof(args), "{\"repo_path\":\"%s\"}", tmp);
4998+
char *resp = cbm_mcp_handle_tool(srv, "index_repository", args);
4999+
ASSERT_NOT_NULL(resp);
5000+
ASSERT_NOT_NULL(strstr(resp, "indexed"));
5001+
free(resp);
5002+
5003+
cbm_store_t *store = cbm_mcp_server_store(srv);
5004+
ASSERT_NOT_NULL(store);
5005+
struct sqlite3 *db = cbm_store_get_db(store);
5006+
ASSERT_NOT_NULL(db);
5007+
5008+
/* Non-vacuous: the fixture must actually produce a brokered edge. */
5009+
sqlite3_stmt *stmt = NULL;
5010+
ASSERT_EQ(sqlite3_prepare_v2(db, "SELECT count(*) FROM edges WHERE type='ASYNC_CALLS';", -1,
5011+
&stmt, NULL),
5012+
SQLITE_OK);
5013+
ASSERT_EQ(sqlite3_step(stmt), SQLITE_ROW);
5014+
int async_edges = sqlite3_column_int(stmt, 0);
5015+
sqlite3_finalize(stmt);
5016+
ASSERT_TRUE(async_edges >= 1);
5017+
5018+
/* THE BUG: malformed properties on edges (broker quote) and Route nodes
5019+
* (raw string). Every properties blob must be valid JSON. */
5020+
ASSERT_EQ(sqlite3_prepare_v2(db,
5021+
"SELECT count(*) FROM edges WHERE properties IS NOT NULL "
5022+
"AND properties != '' AND json_valid(properties)=0;",
5023+
-1, &stmt, NULL),
5024+
SQLITE_OK);
5025+
ASSERT_EQ(sqlite3_step(stmt), SQLITE_ROW);
5026+
int bad_edges = sqlite3_column_int(stmt, 0);
5027+
sqlite3_finalize(stmt);
5028+
ASSERT_EQ(bad_edges, 0);
5029+
5030+
ASSERT_EQ(sqlite3_prepare_v2(db,
5031+
"SELECT count(*) FROM nodes WHERE properties IS NOT NULL "
5032+
"AND properties != '' AND json_valid(properties)=0;",
5033+
-1, &stmt, NULL),
5034+
SQLITE_OK);
5035+
ASSERT_EQ(sqlite3_step(stmt), SQLITE_ROW);
5036+
int bad_nodes = sqlite3_column_int(stmt, 0);
5037+
sqlite3_finalize(stmt);
5038+
ASSERT_EQ(bad_nodes, 0);
5039+
5040+
/* Pipeline parity: the broker must be extractable exactly like the
5041+
* parallel path emits it. */
5042+
ASSERT_EQ(sqlite3_prepare_v2(db,
5043+
"SELECT count(*) FROM edges WHERE type='ASYNC_CALLS' AND "
5044+
"json_extract(properties,'$.broker')='celery';",
5045+
-1, &stmt, NULL),
5046+
SQLITE_OK);
5047+
ASSERT_EQ(sqlite3_step(stmt), SQLITE_ROW);
5048+
int brokered = sqlite3_column_int(stmt, 0);
5049+
sqlite3_finalize(stmt);
5050+
ASSERT_TRUE(brokered >= 1);
5051+
5052+
cbm_mcp_server_free(srv);
5053+
unlink(src_path);
5054+
cbm_rmdir(tmp);
5055+
PASS();
5056+
}
5057+
49675058
TEST(index_second_inprocess_run_survives_issue773) {
49685059
#ifdef _WIN32
49695060
SKIP_PLATFORM("fork-isolated crash guard (POSIX-only)");
@@ -5615,6 +5706,7 @@ SUITE(mcp) {
56155706
RUN_TEST(index_repository_cli_name_override_issue823);
56165707
RUN_TEST(index_supervisor_gate_requires_marked_host_issue845);
56175708
RUN_TEST(index_bg_paths_route_through_supervisor_issue832);
5709+
RUN_TEST(sequential_service_edge_props_are_valid_json_issue898);
56185710
RUN_TEST(index_second_inprocess_run_survives_issue773);
56195711
RUN_TEST(index_recovery_parallel_quarantines_crasher);
56205712
RUN_TEST(tool_manage_adr_not_found_rich_error);

‎tests/windows/test_cli_non_ascii_arg.py‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,6 @@ def main():
9898
print("non-ASCII argv (supervised): rc=%d" % p.returncode)
9999
print(" stdout: %s" % out[:200].replace("\n", " "))
100100
print(" stderr: %s" % err[-200:].replace("\n", " "))
101-
# #973 variant: the reporter's exact shape — Traditional-Chinese dir,
102-
# FLAG-form --repo-path + --mode fast, supervised. This adds coverage
103-
# of the flag->JSON converter and the repo-path canonicalization,
104-
# which used ANSI _access/_fullpath (locale-dependent — corrupted CJK
105-
# paths on CJK-codepage systems) before the cbm_canonical_path fix.
106-
cjk_repo = os.path.join(work, "\u96f7\u9054\u6e2c\u8a66")
107-
make_fixture(cjk_repo)
108-
env3 = dict(os.environ)
109-
env3["CBM_CACHE_DIR"] = os.path.join(work, "cache_cjk")
110-
env3.pop("CBM_INDEX_SUPERVISOR", None)
111-
p2 = subprocess.run([binary, "cli", "index_repository",
112-
"--repo-path", cjk_repo, "--mode", "fast"],
113-
capture_output=True, timeout=120, env=env3)
114-
out2 = (p2.stdout or b"").decode("utf-8", "replace")
115-
cjk_ok = '"nodes"' in out2 and '"nodes":0' not in out2.replace(" ", "")
116-
print("CJK flag-form (--repo-path, supervised, fast): rc=%d" % p2.returncode)
117-
print(" stdout: %s" % out2[:200].replace("\n", " "))
118-
if not cjk_ok:
119-
print("\nRED: CJK flag-form repo_path was not indexed (#973 — "
120-
"canonicalization or spawn boundary mangled the path).")
121-
return 1
122-
123101
if honored:
124102
print("\nGREEN: CLI honored the non-ASCII repo_path (argv + worker spawn).")
125103
return 0

0 commit comments

Comments
 (0)