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":"<inner json>"}]}, 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 <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel committed 2026-06-28 01:23:16 +02:00
1 parent c718d68390
commit b351f76843
3 files changed
+34 -28

No files matched your search

+19 -19
View File
@@ -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":"<inner trace_path json>"}]}
* 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);
+10 -6
View File
@@ -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:
+5 -3
View File
@@ -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 ─────
*