Skip to content

Commit 2791d0d

Browse files
fix(mcp,store): address QA round 1
- Update stale comment in verify_project_indexed: resolve_store now uses cbm_store_open_path_query (no SQLITE_OPEN_CREATE), so store is NULL for missing files; the helper catches the empty-but-present .db case - Guard state updates in resolve_store behind successful open check: only set owns_store=true and update current_project when store is non-NULL, preventing misleading state when an unknown project is queried - Expand smoke_guard.sh to test all 5 guarded handlers (search_graph, query_graph, get_graph_schema, trace_call_path, get_code_snippet) instead of search_graph only; each checks both the guard error and no-ghost-file invariant Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e671745 commit 2791d0d

2 files changed

Lines changed: 54 additions & 28 deletions

File tree

src/mcp/mcp.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -642,9 +642,14 @@ static cbm_store_t *resolve_store(cbm_mcp_server_t *srv, const char *project) {
642642
char path[1024];
643643
project_db_path(project, path, sizeof(path));
644644
srv->store = cbm_store_open_path_query(path);
645-
srv->owns_store = true;
646-
free(srv->current_project);
647-
srv->current_project = heap_strdup(project);
645+
if (srv->store) {
646+
/* Only update ownership and cached project name on successful open.
647+
* When the file is absent, store is NULL and current_project retains
648+
* its previous value so the next call correctly retries the open. */
649+
srv->owns_store = true;
650+
free(srv->current_project);
651+
srv->current_project = heap_strdup(project);
652+
}
648653

649654
return srv->store;
650655
}
@@ -748,8 +753,10 @@ static char *handle_list_projects(cbm_mcp_server_t *srv, const char *args) {
748753

749754
/* verify_project_indexed — returns a heap-allocated error JSON string when the
750755
* named project has not been indexed yet, or NULL when the project exists.
751-
* resolve_store uses SQLITE_OPEN_CREATE so store is always non-NULL even for
752-
* unindexed projects; this check catches that silent-empty case.
756+
* resolve_store uses cbm_store_open_path_query (no SQLITE_OPEN_CREATE), so
757+
* store is NULL for missing .db files (REQUIRE_STORE fires first). This
758+
* function catches the remaining case: a .db file exists but has no indexed
759+
* nodes (e.g., an empty or half-initialised project).
753760
* Callers that receive a non-NULL return value must free(project) themselves
754761
* before returning the error string. */
755762
static char *verify_project_indexed(cbm_store_t *store, const char *project) {

tests/smoke_guard.sh

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#!/usr/bin/env bash
22
# smoke_guard.sh — Smoke test for guard and ghost-file invariants.
33
#
4-
# Verifies two properties:
5-
# 1. Query handlers return "project not indexed" for unknown projects.
4+
# Verifies two properties across all 5 guarded query handlers:
5+
# 1. Each handler returns a guard error for unknown/unindexed projects.
66
# 2. No ghost .db file is created for the unknown project name.
77
#
88
# Usage: bash tests/smoke_guard.sh
@@ -15,6 +15,7 @@ BINARY="$PROJECT_ROOT/build/c/codebase-memory-mcp"
1515
FAKE_PROJECT="nonexistent_smoke_test_xyz"
1616
CACHE_DIR="${HOME}/.cache/codebase-memory-mcp"
1717
GHOST_FILE="$CACHE_DIR/${FAKE_PROJECT}.db"
18+
FAILURES=0
1819

1920
# ── Step 1: Build ─────────────────────────────────────────────────
2021
echo "[smoke_guard] Building project..."
@@ -31,29 +32,47 @@ if [ -f "$GHOST_FILE" ]; then
3132
rm -f "$GHOST_FILE"
3233
fi
3334

34-
# ── Step 3: Invoke query tool with unknown project ────────────────
35-
echo "[smoke_guard] Invoking search_graph with project='$FAKE_PROJECT'..."
36-
RESPONSE="$("$BINARY" cli search_graph "{\"project\":\"$FAKE_PROJECT\",\"name_pattern\":\".*\"}" 2>/dev/null)"
37-
echo "[smoke_guard] Response: $RESPONSE"
38-
39-
# ── Step 4: Assert error message present ─────────────────────────
40-
# For a truly absent project (no .db file), cbm_store_open_path_query returns
41-
# NULL, so REQUIRE_STORE fires with "no project loaded" before
42-
# verify_project_indexed is reached. Both messages confirm the guard is active.
43-
if ! echo "$RESPONSE" | grep -qE "no project loaded|not indexed"; then
44-
echo "[smoke_guard] FAIL: response does not contain guard error ('no project loaded' or 'not indexed')" >&2
45-
echo "[smoke_guard] Got: $RESPONSE" >&2
46-
exit 1
47-
fi
48-
echo "[smoke_guard] PASS: guard error message present"
35+
# ── Helper: assert guard error and no ghost file ──────────────────
36+
check_handler() {
37+
local handler="$1"
38+
local args="$2"
39+
echo "[smoke_guard] Invoking $handler with project='$FAKE_PROJECT'..."
40+
local response
41+
response="$("$BINARY" cli "$handler" "$args" 2>/dev/null)"
42+
echo "[smoke_guard] Response: $response"
4943

50-
# ── Step 5: Assert no ghost .db file was created ─────────────────
51-
if [ -f "$GHOST_FILE" ]; then
52-
echo "[smoke_guard] FAIL: ghost file was created at $GHOST_FILE" >&2
53-
rm -f "$GHOST_FILE"
44+
# For a missing .db file, cbm_store_open_path_query returns NULL so
45+
# REQUIRE_STORE fires ("no project loaded"). For an empty .db,
46+
# verify_project_indexed fires ("project not indexed"). Both are valid.
47+
if ! echo "$response" | grep -qE "no project loaded|not indexed"; then
48+
echo "[smoke_guard] FAIL [$handler]: response does not contain guard error" >&2
49+
echo "[smoke_guard] Got: $response" >&2
50+
FAILURES=$((FAILURES + 1))
51+
else
52+
echo "[smoke_guard] PASS [$handler]: guard error present"
53+
fi
54+
55+
if [ -f "$GHOST_FILE" ]; then
56+
echo "[smoke_guard] FAIL [$handler]: ghost file created at $GHOST_FILE" >&2
57+
rm -f "$GHOST_FILE"
58+
FAILURES=$((FAILURES + 1))
59+
else
60+
echo "[smoke_guard] PASS [$handler]: no ghost .db file"
61+
fi
62+
}
63+
64+
# ── Step 3: Test all 5 guarded handlers ───────────────────────────
65+
check_handler "search_graph" "{\"project\":\"$FAKE_PROJECT\",\"name_pattern\":\".*\"}"
66+
check_handler "query_graph" "{\"project\":\"$FAKE_PROJECT\",\"cypher\":\"MATCH (n) RETURN n LIMIT 1\"}"
67+
check_handler "get_graph_schema" "{\"project\":\"$FAKE_PROJECT\"}"
68+
check_handler "trace_call_path" "{\"project\":\"$FAKE_PROJECT\",\"function_name\":\"main\",\"direction\":\"both\",\"depth\":1}"
69+
check_handler "get_code_snippet" "{\"project\":\"$FAKE_PROJECT\",\"qualified_name\":\"main\"}"
70+
71+
# ── Step 4: Final result ──────────────────────────────────────────
72+
if [ "$FAILURES" -gt 0 ]; then
73+
echo "[smoke_guard] FAILED: $FAILURES check(s) failed." >&2
5474
exit 1
5575
fi
56-
echo "[smoke_guard] PASS: no ghost .db file created"
5776

58-
echo "[smoke_guard] All checks passed."
77+
echo "[smoke_guard] All checks passed (5 handlers, guard + ghost-file invariants)."
5978
exit 0

0 commit comments

Comments
 (0)