From b351f768434e6f59e51c37d9fcbc632fd2fa1987 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 28 Jun 2026 01:23:16 +0200 Subject: [PATCH] test(repro): match MCP-envelope escaping in trace_path repros (#480/#514/#546) trace_path returns the MCP tool-result envelope {"content":[{"type":"text","text":""}]}, so the inner json is embedded as an escaped string value: the "callers"/"callees" keys appear as backslash-escaped \"callers\" / \"callees\". These reproduce-first tests hand-rolled strstr(resp, "\"callers\"") (unescaped), which can never match a correctly escaped envelope, so they stayed RED even though the production traversal is correct (verified by inspecting the inner json: callers/callees populated, data_flow arg expressions surfaced, both import styles unioned). Correct the assertions to the escaped form (the project's passing trace_path tests in test_incremental.c use the same via resp_has_key) and assert populated entries by qualified-name tail, keeping the empty-array guards so the tests still RED on a genuinely-empty traversal. Production code unchanged. Signed-off-by: Martin Vogel --- tests/repro/repro_issue480.c | 38 ++++++++++++++++++------------------ tests/repro/repro_issue514.c | 16 +++++++++------ tests/repro/repro_issue546.c | 8 +++++--- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/tests/repro/repro_issue480.c b/tests/repro/repro_issue480.c index 3ba9a1492..83dcc07c5 100644 --- a/tests/repro/repro_issue480.c +++ b/tests/repro/repro_issue480.c @@ -141,26 +141,26 @@ TEST(repro_issue480_trace_path_nonempty_with_calls) { * start-node lookup as the breakage site. */ ASSERT_NULL(strstr(resp, "function not found")); - /* The "callers" key must appear — it is always emitted when direction - * includes "inbound" (even as an empty array), confirming we reached - * the BFS step. */ - ASSERT_NOT_NULL(strstr(resp, "\"callers\"")); + /* The response is the MCP tool-result envelope + * {"content":[{"type":"text","text":""}]} + * so the inner json is embedded as a STRING value and its quotes are + * backslash-escaped: the "callers" key appears as \"callers\" in the + * serialized response. Match the escaped form — the project's own + * passing trace_path tests (test_incremental.c, via resp_has_key) do the + * same. (The earlier unescaped strstr could never match a correctly + * escaped MCP envelope, which is why this repro was mis-targeted.) + * + * The "callers" key must appear (always emitted for inbound). */ + ASSERT_NOT_NULL(strstr(resp, "\\\"callers\\\"")); - /* The "callers" array must be NON-EMPTY: the string "caller" must - * appear inside the response as the name of the calling function. - * - * WHY RED on buggy code: - * cbm_store_bfs() starts from nodes[sel].id but that id does not - * resolve to any edge endpoint in the graph, so visited_count == 0. - * bfs_to_json_array() serialises an empty JSON array []. - * strstr(resp, "\"caller\"") returns NULL → ASSERT_NOT_NULL fails. - * - * The two-part check below distinguishes the two bug symptoms: - * 1. empty array → "callers\":[]" present, no "\"caller\"" entry - * 2. name missing → "callers" array populated with wrong names - * Either way the final assertion is the one that must turn RED. */ - ASSERT_NULL(strstr(resp, "\"callers\":[]")); /* empty array = traversal bug */ - ASSERT_NOT_NULL(strstr(resp, "\"caller\"")); /* caller name in results */ + /* The "callers" array must be NON-EMPTY. WHY RED on the #480 bug: + * cbm_store_bfs() returning 0 hops serialises \"callers\":[] (no caller + * QN in the response), so BOTH the empty-array guard and the caller-QN + * assertion fire RED. We assert the caller's qualified-name tail + * "main.caller" (unambiguous vs the callee "main.callee", and immune to + * escaping) so a populated, correctly-named caller hop is required. */ + ASSERT_NULL(strstr(resp, "\\\"callers\\\":[]")); /* empty array = traversal bug */ + ASSERT_NOT_NULL(strstr(resp, "main.caller")); /* caller QN in results */ free(resp); rh_cleanup(&lp, store); diff --git a/tests/repro/repro_issue514.c b/tests/repro/repro_issue514.c index 738033c10..96f255045 100644 --- a/tests/repro/repro_issue514.c +++ b/tests/repro/repro_issue514.c @@ -164,13 +164,17 @@ TEST(repro_issue514_data_flow_surfaces_arg_expr) { /* The response must not be an error -- the node must be found. */ ASSERT_NULL(strstr(resp, "function not found")); - /* The "callees" key must appear (direction=outbound always emits it). */ - ASSERT_NOT_NULL(strstr(resp, "\"callees\"")); + /* The response is the MCP tool-result envelope (inner json embedded as an + * escaped string value), so the "callees" key appears as \"callees\". + * Match the escaped form (see repro_issue480 / test_incremental's + * resp_has_key idiom). */ + ASSERT_NOT_NULL(strstr(resp, "\\\"callees\\\"")); - /* The callees array must be non-empty: "callee" must appear as a hop. - * If this fires RED, the CALLS traversal is broken (separate from #514). */ - ASSERT_NULL(strstr(resp, "\"callees\":[]")); - ASSERT_NOT_NULL(strstr(resp, "\"callee\"")); + /* The callees array must be non-empty: the callee's QN tail "service.callee" + * must appear as a hop (unambiguous + escaping-proof). RED if the CALLS + * traversal is broken (separate from #514). */ + ASSERT_NULL(strstr(resp, "\\\"callees\\\":[]")); + ASSERT_NOT_NULL(strstr(resp, "service.callee")); /* * THE CORE ASSERTION FOR BUG #514: diff --git a/tests/repro/repro_issue546.c b/tests/repro/repro_issue546.c index b88daa4fc..546535c0c 100644 --- a/tests/repro/repro_issue546.c +++ b/tests/repro/repro_issue546.c @@ -219,8 +219,10 @@ TEST(repro_issue546_dts_split_caller_set) { * itself has a problem unrelated to #546. */ ASSERT_NULL(strstr(resp, "function not found")); - /* "callers" key must appear (always emitted when direction is inbound). */ - ASSERT_NOT_NULL(strstr(resp, "\"callers\"")); + /* "callers" key must appear (always emitted when direction is inbound). + * The response is the MCP envelope (inner json embedded as an escaped + * string), so the key appears as \"callers\" — match the escaped form. */ + ASSERT_NOT_NULL(strstr(resp, "\\\"callers\\\"")); /* The callers array must not be empty — at least the internalConsumer * (whose relative-path import is reliably resolved) must appear. @@ -230,7 +232,7 @@ TEST(repro_issue546_dts_split_caller_set) { * externalConsumer is there; internalConsumer's edge is on the impl * node, so this check fires RED immediately (callers:[]) or wrong name. */ - ASSERT_NULL(strstr(resp, "\"callers\":[]")); /* empty = traversal totally wrong */ + ASSERT_NULL(strstr(resp, "\\\"callers\\\":[]")); /* empty = traversal totally wrong */ /* ── PRIMARY ASSERTION: BOTH callers must appear in the response ───── *