Skip to content

Commit 73cb6bf

Browse files
QUSETIONSclaude
andcommitted
Harden system-prompt builder + fix memory search concurrency race
prompt.py (runs every turn): malformed MCP server entries (missing toolCount/name/status, or non-dict), None elements in permission_summary, and malformed skill dicts each crashed build_system_prompt_bundle — guarded all section builders with .get() defaults + per-entry isinstance checks so one bad entry can't take down the whole prompt build. memory.py MemoryFile.search: snapshot entries once at the start instead of iterating self.entries twice. Previously a concurrent add_entry could append between the entry_tokens-building loop and the scoring loop, making entry_tokens[i] index out of range (test_concurrent_add_and_search crashed deterministically). The two loops now iterate the same snapshot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e1f1faa commit 73cb6bf

3 files changed

Lines changed: 91 additions & 19 deletions

File tree

minicode/memory.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,12 @@ def search(self, query: str, active_domains: list[str] | None = None) -> list[Me
821821
if not self.entries:
822822
return []
823823

824+
# Snapshot entries so concurrent add_entry/_enforce_limits can't shift
825+
# indices between the two loops below (was: "list index out of range"
826+
# when another thread appended between building entry_tokens and the
827+
# scoring loop).
828+
entries = list(self.entries)
829+
824830
query_tokens = _tokenize(query)
825831
query_tokens = _expand_query_terms(query_tokens, active_domains=active_domains)
826832
if not query_tokens:
@@ -830,15 +836,15 @@ def search(self, query: str, active_domains: list[str] | None = None) -> list[Me
830836
query_terms = query_lower.split()
831837

832838
entry_tokens = []
833-
for entry in self.entries:
839+
for entry in entries:
834840
text = f"{entry.content} {entry.category} {' '.join(entry.tags)}"
835841
entry_tokens.append(_tokenize(text))
836842

837843
idf = _compute_idf(entry_tokens)
838844
avgdl = _compute_avgdl(entry_tokens)
839845

840846
scored: list[tuple[float, MemoryEntry]] = []
841-
for i, entry in enumerate(self.entries):
847+
for i, entry in enumerate(entries):
842848
bm25 = _bm25_score(query_tokens, entry_tokens[i], idf, avgdl)
843849

844850
substring_score = 0.0

minicode/prompt.py

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,10 @@ def build_system_prompt_bundle(
156156
# --- Dynamic Suffix (Per-turn) ---
157157
# Permission context
158158
if permission_summary:
159-
perm_text = "Permission context:\n" + "\n".join(permission_summary)
159+
# Coerce/filter so a None element in the summary can't crash join().
160+
perm_text = "Permission context:\n" + "\n".join(
161+
str(p) for p in permission_summary if p is not None
162+
)
160163
pipeline.register_dynamic("permissions", lambda: perm_text)
161164

162165
# Skills section with conditional injection
@@ -165,7 +168,10 @@ def build_system_prompt_bundle(
165168
def _build_skills():
166169
lines = ["Available skills:"]
167170
lines.extend(
168-
f"- {skill['name']}: {skill['description']}" for skill in skills
171+
f"- {skill.get('name', '?')}: {skill.get('description', '')}"
172+
if isinstance(skill, dict)
173+
else f"- {skill}"
174+
for skill in skills
169175
)
170176
lines.extend([
171177
"",
@@ -194,31 +200,44 @@ def _build_skills():
194200
# MCP servers section
195201
mcp_servers = extras.get("mcpServers", [])
196202
if mcp_servers:
203+
def _server_line(server: Any) -> str:
204+
# Guard each entry so one malformed server (missing keys / non-dict)
205+
# can't crash the whole system-prompt build.
206+
if not isinstance(server, dict):
207+
return f"- (malformed MCP server entry: {server!r})"
208+
parts = [
209+
f"- {server.get('name', '(unnamed)')}: "
210+
f"{server.get('status', 'unknown')}, tools={server.get('toolCount', 0)}"
211+
]
212+
if server.get("resourceCount") is not None:
213+
parts.append(f", resources={server['resourceCount']}")
214+
if server.get("promptCount") is not None:
215+
parts.append(f", prompts={server['promptCount']}")
216+
if server.get("protocol"):
217+
parts.append(f", protocol={server['protocol']}")
218+
if server.get("error"):
219+
parts.append(f" ({server['error']})")
220+
return "".join(parts)
221+
197222
def _build_mcp():
198223
lines = ["Configured MCP servers:"]
199-
lines.extend(
200-
"- "
201-
+ server["name"]
202-
+ f": {server['status']}, tools={server['toolCount']}"
203-
+ (f", resources={server['resourceCount']}" if server.get("resourceCount") is not None else "")
204-
+ (f", prompts={server['promptCount']}" if server.get("promptCount") is not None else "")
205-
+ (f", protocol={server['protocol']}" if server.get("protocol") else "")
206-
+ (f" ({server['error']})" if server.get("error") else "")
207-
for server in mcp_servers
208-
)
209-
if any(server.get("status") == "connected" for server in mcp_servers):
224+
lines.extend(_server_line(server) for server in mcp_servers)
225+
if any((server.get("status") if isinstance(server, dict) else None) == "connected" for server in mcp_servers):
210226
lines.append(
211227
"Connected MCP tools are already exposed in the tool list with names prefixed like mcp__server__tool. "
212228
"Use list_mcp_resources/read_mcp_resource and list_mcp_prompts/get_mcp_prompt when a server exposes those capabilities."
213229
)
214230
# Sequential thinking server detection
215231
sequential_servers = [
216232
server for server in mcp_servers
217-
if "sequential" in server.get("name", "").lower()
218-
or "branch-thinking" in server.get("name", "").lower()
219-
or "think" in server.get("name", "").lower()
233+
if isinstance(server, dict)
234+
and (
235+
"sequential" in server.get("name", "").lower()
236+
or "branch-thinking" in server.get("name", "").lower()
237+
or "think" in server.get("name", "").lower()
238+
)
220239
]
221-
if any(server.get("status") == "connected" for server in sequential_servers):
240+
if any(isinstance(s, dict) and s.get("status") == "connected" for s in sequential_servers):
222241
lines.extend([
223242
"",
224243
"SEQUENTIAL THINKING MCP SERVER IS CONNECTED!",

tests/test_prompt.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,50 @@ def test_build_system_prompt_includes_memory_context(tmp_path: Path) -> None:
4343

4444
assert "Project Memory & Context" in prompt
4545
assert "Always run pytest before release." in prompt
46+
47+
48+
# ---------------------------------------------------------------------------
49+
# Robustness: the system-prompt builder runs every turn; malformed MCP/skill/
50+
# permission inputs must not crash it.
51+
# ---------------------------------------------------------------------------
52+
53+
54+
def test_build_system_prompt_bundle_handles_malformed_mcp_entry():
55+
"""A partial MCP server dict (missing toolCount/name/status) or a non-dict
56+
entry must not KeyError/AttributeError the prompt build."""
57+
from minicode.prompt import build_system_prompt_bundle
58+
59+
extras = {
60+
"mcpServers": [
61+
{"name": "broken", "status": "error"}, # missing toolCount
62+
"not-a-dict", # wholly malformed
63+
{"name": "sequential-thinking", "status": "connected", "toolCount": 2},
64+
],
65+
"skills": [],
66+
"memory_context": "",
67+
"runtime": {},
68+
}
69+
bundle = build_system_prompt_bundle(".", ["cwd: ."], extras)
70+
assert isinstance(bundle.prompt, str) and bundle.prompt
71+
assert "sequential-thinking" in bundle.prompt
72+
73+
74+
def test_build_system_prompt_bundle_handles_none_in_permission_summary():
75+
"""A None element in permission_summary must not crash join()."""
76+
from minicode.prompt import build_system_prompt_bundle
77+
78+
bundle = build_system_prompt_bundle(
79+
".", ["ok", None, "x"], {"mcpServers": [], "skills": [], "memory_context": "", "runtime": {}}
80+
)
81+
assert "Permission context" in bundle.prompt
82+
assert "ok" in bundle.prompt and "x" in bundle.prompt
83+
84+
85+
def test_build_system_prompt_bundle_handles_malformed_skill():
86+
"""A skill dict missing name/description must not KeyError the build."""
87+
from minicode.prompt import build_system_prompt_bundle
88+
89+
bundle = build_system_prompt_bundle(
90+
".", [], {"mcpServers": [], "skills": [{"name": "s"}], "memory_context": "", "runtime": {}}
91+
)
92+
assert isinstance(bundle.prompt, str) and bundle.prompt

0 commit comments

Comments
 (0)