fix(mcp,ci): harden get_architecture serialization + CodeQL gate

Follow-up to #281 (handle_get_architecture) plus a CodeQL workflow
upgrade developed in parallel; bundling into one commit because the
test suite had to land alongside both.

mcp.c — handle_get_architecture
- NULL-coerce every const-char* field in the architecture sections
  via `x ? x : ""`, matching the rest of mcp.c (search_graph, etc.).
  Without this, a NULL field becomes a missing JSON key instead of
  an empty string; yyjson_mut_obj_add_str returns false on NULL and
  silently no-ops, so an inconsistent omission could surprise callers.
- Serialize two more architecture aspects that #281 left on the
  floor: services (cbm_service_link_t: from/to/type/count) and
  clusters (cbm_cluster_info_t: id/label/members/cohesion plus the
  top_nodes / packages / edge_types string arrays). The store-side
  computation populates these for aspects=["all"] / explicit names,
  so dropping them in the serializer was data loss.

tests/test_mcp.c
- New tool_get_architecture_emits_populated_sections regression test.
  Uses a minimal inline fixture (single Function node tagged with
  "is_entry_point": true) since arch_entry_points reads that flag
  out of properties_json. Asserts the response contains both an
  "entry_points" array and the function name — neither would appear
  before #281 because handle_get_architecture never called
  cbm_store_get_architecture.
- extract_text_content drilled too shallow: it pulled "content" only
  from the JSON root, so it worked for cbm_mcp_handle_tool but
  silently fell through to the raw response for cbm_mcp_server_handle
  (where content lives under .result.content). Added a fallback that
  checks .result.content; both unwrappers tested by existing fixtures.

ci(codeql)
- Run on pull_request to main, not just push; surfaces findings on
  the PR instead of after merge.
- Pull custom queries from ./codeql via `queries: +./codeql`.
- Capture SARIF output and fail the job on any error-level finding,
  using jq to enumerate rule id, file:line, and message text in the
  GitHub Actions error annotation. Warnings are still reported as
  before; only errors block.

Full suite: 2842 passed, 0 failed.
This commit is contained in:
Martin Vogel
2026-05-09 16:37:53 +02:00
parent ca5e309062
commit 27e640b3b0
3 changed files with 164 additions and 18 deletions
+23 -1
View File
@@ -15,18 +15,40 @@ jobs:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: Install build dependencies
run: sudo apt-get update && sudo apt-get install -y zlib1g-dev
run: sudo apt-get update && sudo apt-get install -y zlib1g-dev jq
- name: Initialize CodeQL
uses: github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4
with:
languages: c-cpp
build-mode: manual
queries: +./codeql
- name: Build for CodeQL analysis
run: scripts/build.sh
- name: Perform CodeQL Analysis
id: analyze
uses: github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4
with:
category: "/language:c-cpp"
output: codeql-results
- name: Fail on CodeQL error-level findings
run: |
sarif=$(find codeql-results -name '*.sarif' | head -1)
if [[ -z "$sarif" ]]; then
echo "::error::No SARIF output found"; exit 1
fi
err=$(jq '[.runs[].results[]? | select((.level // "warning") == "error")] | length' "$sarif")
warn=$(jq '[.runs[].results[]? | select((.level // "warning") == "warning")] | length' "$sarif")
echo "CodeQL findings: $err error(s), $warn warning(s)"
if [[ "$err" -gt 0 ]]; then
jq -r '
.runs[].results[]?
| select((.level // "warning") == "error")
| "\(.locations[0].physicalLocation.artifactLocation.uri):\(.locations[0].physicalLocation.region.startLine) [\(.ruleId)] \(.message.text)"
' "$sarif"
echo "::error::$err CodeQL error-level finding(s) — failing build."
exit 1
fi
+83 -17
View File
@@ -1860,7 +1860,8 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) {
yyjson_mut_val *langs = yyjson_mut_arr(doc);
for (int i = 0; i < arch.language_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "language", arch.languages[i].language);
yyjson_mut_obj_add_str(doc, item, "language",
arch.languages[i].language ? arch.languages[i].language : "");
yyjson_mut_obj_add_int(doc, item, "file_count", arch.languages[i].file_count);
yyjson_mut_arr_add_val(langs, item);
}
@@ -1872,7 +1873,8 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) {
yyjson_mut_val *pkgs = yyjson_mut_arr(doc);
for (int i = 0; i < arch.package_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "name", arch.packages[i].name);
yyjson_mut_obj_add_str(doc, item, "name",
arch.packages[i].name ? arch.packages[i].name : "");
yyjson_mut_obj_add_int(doc, item, "node_count", arch.packages[i].node_count);
yyjson_mut_obj_add_int(doc, item, "fan_in", arch.packages[i].fan_in);
yyjson_mut_obj_add_int(doc, item, "fan_out", arch.packages[i].fan_out);
@@ -1886,10 +1888,14 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) {
yyjson_mut_val *eps = yyjson_mut_arr(doc);
for (int i = 0; i < arch.entry_point_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "name", arch.entry_points[i].name);
yyjson_mut_obj_add_str(doc, item, "name",
arch.entry_points[i].name ? arch.entry_points[i].name : "");
yyjson_mut_obj_add_str(doc, item, "qualified_name",
arch.entry_points[i].qualified_name);
yyjson_mut_obj_add_str(doc, item, "file", arch.entry_points[i].file);
arch.entry_points[i].qualified_name
? arch.entry_points[i].qualified_name
: "");
yyjson_mut_obj_add_str(doc, item, "file",
arch.entry_points[i].file ? arch.entry_points[i].file : "");
yyjson_mut_arr_add_val(eps, item);
}
yyjson_mut_obj_add_val(doc, root, "entry_points", eps);
@@ -1900,9 +1906,12 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) {
yyjson_mut_val *routes = yyjson_mut_arr(doc);
for (int i = 0; i < arch.route_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "method", arch.routes[i].method);
yyjson_mut_obj_add_str(doc, item, "path", arch.routes[i].path);
yyjson_mut_obj_add_str(doc, item, "handler", arch.routes[i].handler);
yyjson_mut_obj_add_str(doc, item, "method",
arch.routes[i].method ? arch.routes[i].method : "");
yyjson_mut_obj_add_str(doc, item, "path",
arch.routes[i].path ? arch.routes[i].path : "");
yyjson_mut_obj_add_str(doc, item, "handler",
arch.routes[i].handler ? arch.routes[i].handler : "");
yyjson_mut_arr_add_val(routes, item);
}
yyjson_mut_obj_add_val(doc, root, "routes", routes);
@@ -1913,9 +1922,12 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) {
yyjson_mut_val *hotspots = yyjson_mut_arr(doc);
for (int i = 0; i < arch.hotspot_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "name", arch.hotspots[i].name);
yyjson_mut_obj_add_str(doc, item, "name",
arch.hotspots[i].name ? arch.hotspots[i].name : "");
yyjson_mut_obj_add_str(doc, item, "qualified_name",
arch.hotspots[i].qualified_name);
arch.hotspots[i].qualified_name
? arch.hotspots[i].qualified_name
: "");
yyjson_mut_obj_add_int(doc, item, "fan_in", arch.hotspots[i].fan_in);
yyjson_mut_arr_add_val(hotspots, item);
}
@@ -1927,34 +1939,88 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) {
yyjson_mut_val *boundaries = yyjson_mut_arr(doc);
for (int i = 0; i < arch.boundary_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "from", arch.boundaries[i].from);
yyjson_mut_obj_add_str(doc, item, "to", arch.boundaries[i].to);
yyjson_mut_obj_add_str(doc, item, "from",
arch.boundaries[i].from ? arch.boundaries[i].from : "");
yyjson_mut_obj_add_str(doc, item, "to",
arch.boundaries[i].to ? arch.boundaries[i].to : "");
yyjson_mut_obj_add_int(doc, item, "call_count", arch.boundaries[i].call_count);
yyjson_mut_arr_add_val(boundaries, item);
}
yyjson_mut_obj_add_val(doc, root, "boundaries", boundaries);
}
/* Cross-service links (HTTP/async between services) */
if (arch.service_count > 0) {
yyjson_mut_val *services = yyjson_mut_arr(doc);
for (int i = 0; i < arch.service_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "from",
arch.services[i].from ? arch.services[i].from : "");
yyjson_mut_obj_add_str(doc, item, "to",
arch.services[i].to ? arch.services[i].to : "");
yyjson_mut_obj_add_str(doc, item, "type",
arch.services[i].type ? arch.services[i].type : "");
yyjson_mut_obj_add_int(doc, item, "count", arch.services[i].count);
yyjson_mut_arr_add_val(services, item);
}
yyjson_mut_obj_add_val(doc, root, "services", services);
}
/* Package layers */
if (arch.layer_count > 0) {
yyjson_mut_val *layers = yyjson_mut_arr(doc);
for (int i = 0; i < arch.layer_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "name", arch.layers[i].name);
yyjson_mut_obj_add_str(doc, item, "layer", arch.layers[i].layer);
yyjson_mut_obj_add_str(doc, item, "reason", arch.layers[i].reason);
yyjson_mut_obj_add_str(doc, item, "name",
arch.layers[i].name ? arch.layers[i].name : "");
yyjson_mut_obj_add_str(doc, item, "layer",
arch.layers[i].layer ? arch.layers[i].layer : "");
yyjson_mut_obj_add_str(doc, item, "reason",
arch.layers[i].reason ? arch.layers[i].reason : "");
yyjson_mut_arr_add_val(layers, item);
}
yyjson_mut_obj_add_val(doc, root, "layers", layers);
}
/* Clusters (community detection) */
if (arch.cluster_count > 0) {
yyjson_mut_val *clusters = yyjson_mut_arr(doc);
for (int i = 0; i < arch.cluster_count; i++) {
const cbm_cluster_info_t *c = &arch.clusters[i];
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_int(doc, item, "id", c->id);
yyjson_mut_obj_add_str(doc, item, "label", c->label ? c->label : "");
yyjson_mut_obj_add_int(doc, item, "members", c->members);
yyjson_mut_obj_add_real(doc, item, "cohesion", c->cohesion);
yyjson_mut_val *top = yyjson_mut_arr(doc);
for (int j = 0; j < c->top_node_count; j++) {
yyjson_mut_arr_add_str(doc, top, c->top_nodes[j] ? c->top_nodes[j] : "");
}
yyjson_mut_obj_add_val(doc, item, "top_nodes", top);
yyjson_mut_val *pkgs = yyjson_mut_arr(doc);
for (int j = 0; j < c->package_count; j++) {
yyjson_mut_arr_add_str(doc, pkgs, c->packages[j] ? c->packages[j] : "");
}
yyjson_mut_obj_add_val(doc, item, "packages", pkgs);
yyjson_mut_val *etypes = yyjson_mut_arr(doc);
for (int j = 0; j < c->edge_type_count; j++) {
yyjson_mut_arr_add_str(doc, etypes, c->edge_types[j] ? c->edge_types[j] : "");
}
yyjson_mut_obj_add_val(doc, item, "edge_types", etypes);
yyjson_mut_arr_add_val(clusters, item);
}
yyjson_mut_obj_add_val(doc, root, "clusters", clusters);
}
/* File tree */
if (arch.file_tree_count > 0) {
yyjson_mut_val *file_tree = yyjson_mut_arr(doc);
for (int i = 0; i < arch.file_tree_count; i++) {
yyjson_mut_val *item = yyjson_mut_obj(doc);
yyjson_mut_obj_add_str(doc, item, "path", arch.file_tree[i].path);
yyjson_mut_obj_add_str(doc, item, "type", arch.file_tree[i].type);
yyjson_mut_obj_add_str(doc, item, "path",
arch.file_tree[i].path ? arch.file_tree[i].path : "");
yyjson_mut_obj_add_str(doc, item, "type",
arch.file_tree[i].type ? arch.file_tree[i].type : "");
yyjson_mut_obj_add_int(doc, item, "children", arch.file_tree[i].children);
yyjson_mut_arr_add_val(file_tree, item);
}
+58
View File
@@ -525,6 +525,56 @@ TEST(tool_get_architecture_empty) {
PASS();
}
/* Regression for #281: handle_get_architecture must actually call
* cbm_store_get_architecture and surface its sections. Before the fix
* only label/edge histograms were emitted regardless of which aspects
* were requested. The store-side arch_entry_points query reads
* properties.is_entry_point on Function nodes, so we tag one node and
* assert the resulting JSON surfaces an "entry_points" array containing
* the tagged function — which is impossible without the wiring. */
TEST(tool_get_architecture_emits_populated_sections) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
ASSERT_NOT_NULL(srv);
cbm_store_t *st = cbm_mcp_server_store(srv);
ASSERT_NOT_NULL(st);
const char *proj = "arch-test";
cbm_mcp_server_set_project(srv, proj);
cbm_store_upsert_project(st, proj, "/tmp/arch-test");
cbm_node_t main_fn = {0};
main_fn.project = proj;
main_fn.label = "Function";
main_fn.name = "main";
main_fn.qualified_name = "arch-test.cmd.main";
main_fn.file_path = "cmd/main.go";
main_fn.start_line = 1;
main_fn.end_line = 3;
main_fn.properties_json = "{\"is_entry_point\":true}";
ASSERT_GT(cbm_store_upsert_node(st, &main_fn), 0);
char *resp = cbm_mcp_server_handle(
srv, "{\"jsonrpc\":\"2.0\",\"id\":91,\"method\":\"tools/call\","
"\"params\":{\"name\":\"get_architecture\","
"\"arguments\":{\"project\":\"arch-test\",\"aspects\":[\"all\"]}}}");
ASSERT_NOT_NULL(resp);
char *inner = extract_text_content(resp);
ASSERT_NOT_NULL(inner);
/* The handler always emits node/edge counts and schema histograms;
* those existed before #281. The "entry_points" array only appears
* when cbm_store_get_architecture is actually called and its result
* is serialized — which is exactly what #281 wires up. */
ASSERT_NOT_NULL(strstr(inner, "\"entry_points\""));
ASSERT_NOT_NULL(strstr(inner, "main"));
free(inner);
free(resp);
cbm_mcp_server_free(srv);
PASS();
}
TEST(tool_query_graph_missing_query) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
@@ -1001,6 +1051,13 @@ static char *extract_text_content(const char *mcp_result) {
return strdup(mcp_result); /* fallback */
yyjson_val *root = yyjson_doc_get_root(doc);
yyjson_val *content = yyjson_obj_get(root, "content");
if (!content) {
/* Handle JSON-RPC wrapper: {"jsonrpc":...,"result":{"content":[...]}} */
yyjson_val *rpc_result = yyjson_obj_get(root, "result");
if (rpc_result) {
content = yyjson_obj_get(rpc_result, "content");
}
}
if (!content || !yyjson_is_arr(content)) {
yyjson_doc_free(doc);
return strdup(mcp_result);
@@ -1739,6 +1796,7 @@ SUITE(mcp) {
RUN_TEST(tool_trace_missing_function_name);
RUN_TEST(tool_delete_project_not_found);
RUN_TEST(tool_get_architecture_empty);
RUN_TEST(tool_get_architecture_emits_populated_sections);
RUN_TEST(tool_query_graph_missing_query);
/* Pipeline-dependent tool handlers */