Skip to content

Commit 0cc090e

Browse files
cdeustclaude
andcommitted
fix(handlers): remember/recall/get_telemetry return dict not str (issue #17)
PSGSupport reported strict MCP clients (Claude Code tool runtime) rejecting these three handlers with "structured_content must be a dict or None. Got str". The DB writes happened but FastMCP rejected the response before delivery to the client. Root cause: mcp_server/tool_error_handler.py:safe_handler used to wrap every handler return value in json.dumps(...) and return the JSON string. FastMCP 2.x validates structured content against the declared output_schema and rejects non-dict returns. Three handlers (remember, recall, get_telemetry) declare output_schema and so failed; others without output_schema worked because FastMCP auto-wraps strings when no schema is declared. The fix returns the dict directly from safe_handler (with a defensive {} fallback for None). All tool registry signatures updated from -> str to -> dict to match. Liskov: every MCP handler now uniformly returns dict (or None) per the output_schema contract. Added a contract-enforcement test in tests_py/server/test_handler_contract.py that asserts the invariant across all registered tools, so future handlers can't silently regress this. Also pinned the dict-not-str contract in: - tests_py/handlers/test_get_telemetry.py (new file) - tests_py/handlers/test_remember.py (TestRememberReturnsDict class) - tests_py/handlers/test_recall.py (TestRecallReturnsDict class) - tests_py/integration/test_cold_start.py (TestToolErrorHandler updates) Verification: 16 new/updated tests pass; full handlers + server + cold_start sweep stays green (355 passed). Closes #17. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4260a99 commit 0cc090e

13 files changed

Lines changed: 310 additions & 89 deletions

mcp_server/tool_error_handler.py

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,25 @@
99
DB methods) run on a worker thread instead of blocking the event
1010
loop
1111
12+
Issue #17 (PSGSupport): handlers that declare ``output_schema`` were
13+
rejected by FastMCP with ``structured_content must be a dict or None.
14+
Got str: '{...}'`` because this wrapper used to ``json.dumps`` the
15+
result before returning. FastMCP 2.x validates the return shape
16+
against the declared schema and rejects strings. Fix: return the
17+
dict directly. The handler contract IS dict-or-None (Liskov: every
18+
``mcp__cortex__*`` handler now uniformly satisfies the same interface).
19+
1220
Usage in tool registries:
1321
from mcp_server.tool_error_handler import safe_handler
1422
15-
async def tool_remember(...) -> str:
23+
async def tool_remember(...) -> dict:
1624
result = await safe_handler(remember.handler, {...}, tool_name="remember")
1725
return result
1826
"""
1927

2028
from __future__ import annotations
2129

2230
import asyncio
23-
import json
2431
from typing import Any, Callable, Coroutine
2532

2633
_DB_SETUP_GUIDE = (
@@ -107,8 +114,8 @@ async def safe_handler(
107114
handler_fn: Callable[..., Coroutine[Any, Any, dict]],
108115
args: dict[str, Any],
109116
tool_name: str | None = None,
110-
) -> str:
111-
"""Call a handler and return JSON, catching errors gracefully.
117+
) -> dict[str, Any]:
118+
"""Call a handler and return its dict, catching errors gracefully.
112119
113120
When ``tool_name`` is provided:
114121
* The call is gated by the per-tool admission semaphore (Phase 5
@@ -124,9 +131,15 @@ async def safe_handler(
124131
event loop without admission (backward-compat for code paths not
125132
yet migrated).
126133
127-
On success: returns json.dumps(result).
128-
On DB errors: returns a friendly setup guide.
129-
On other errors: returns error type + message (no traceback).
134+
Contract (issue #17 — Liskov enforcement across all MCP handlers):
135+
precondition: ``handler_fn`` is an async callable returning a dict.
136+
postcondition: returns a ``dict[str, Any]``. Never a JSON string.
137+
FastMCP 2.x validates structured content against
138+
the declared ``output_schema`` and rejects strings.
139+
140+
On success: returns the handler's dict verbatim.
141+
On DB errors: returns a friendly setup-guide dict.
142+
On other errors: returns an error-type/message dict (no traceback).
130143
"""
131144
try:
132145
if tool_name:
@@ -147,7 +160,13 @@ async def safe_handler(
147160
)
148161
else:
149162
result = await handler_fn(args)
150-
return json.dumps(result, indent=2, default=str)
163+
# Defensive: every handler must already return a dict per its
164+
# ``output_schema``. If a handler regresses to None we surface
165+
# an empty dict so FastMCP's structured-content validator does
166+
# not reject the response.
167+
if result is None:
168+
return {}
169+
return result
151170
except Exception as exc:
152171
error_type, message = _classify_error(exc)
153172
if tool_name:
@@ -160,17 +179,13 @@ async def safe_handler(
160179
)
161180
except Exception:
162181
pass
163-
return json.dumps(
164-
{
165-
"error": error_type,
166-
"message": message,
167-
"hint": (
168-
"If this persists, check that PostgreSQL is running "
169-
"and DATABASE_URL is set correctly."
170-
)
171-
if error_type not in ("missing_extension", "database_not_connected")
172-
else None,
173-
},
174-
indent=2,
175-
default=str,
176-
)
182+
return {
183+
"error": error_type,
184+
"message": message,
185+
"hint": (
186+
"If this persists, check that PostgreSQL is running "
187+
"and DATABASE_URL is set correctly."
188+
)
189+
if error_type not in ("missing_extension", "database_not_connected")
190+
else None,
191+
}

mcp_server/tool_registry_advanced.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ async def tool_sync_instructions(
3939
max_insights: int = 10,
4040
min_heat: float = 0.3,
4141
dry_run: bool = False,
42-
) -> str:
42+
) -> dict:
4343
"""Push top memory insights into CLAUDE.md."""
4444
return await safe_handler(
4545
sync_instructions.handler,
@@ -63,7 +63,7 @@ async def tool_create_trigger(
6363
trigger_condition: str,
6464
trigger_type: str = "keyword",
6565
target_directory: str | None = None,
66-
) -> str:
66+
) -> dict:
6767
"""Create a prospective memory trigger."""
6868
return await safe_handler(
6969
create_trigger.handler,
@@ -89,7 +89,7 @@ async def tool_add_rule(
8989
scope: str = "global",
9090
scope_value: str | None = None,
9191
priority: int = 0,
92-
) -> str:
92+
) -> dict:
9393
"""Add a neuro-symbolic rule to the memory store."""
9494
return await safe_handler(
9595
add_rule.handler,
@@ -114,7 +114,7 @@ async def tool_get_rules(
114114
scope: str | None = None,
115115
rule_type: str | None = None,
116116
include_inactive: bool = False,
117-
) -> str:
117+
) -> dict:
118118
"""List active neuro-symbolic rules."""
119119
return await safe_handler(
120120
get_rules.handler,
@@ -137,7 +137,7 @@ async def tool_get_project_story(
137137
domain: str | None = None,
138138
period: str = "week",
139139
max_chapters: int = 5,
140-
) -> str:
140+
) -> dict:
141141
"""Generate a period-based autobiographical narrative."""
142142
return await safe_handler(
143143
get_project_story.handler,
@@ -160,7 +160,7 @@ async def tool_assess_coverage(
160160
directory: str | None = None,
161161
domain: str | None = None,
162162
stale_days: int = 14,
163-
) -> str:
163+
) -> dict:
164164
"""Evaluate knowledge coverage completeness."""
165165
return await safe_handler(
166166
assess_coverage.handler,

mcp_server/tool_registry_core.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ async def tool_query_methodology(
4646
cwd: str | None = None,
4747
project: str | None = None,
4848
first_message: str | None = None,
49-
) -> str:
49+
) -> dict:
5050
"""Returns cognitive profile for the current domain."""
5151
return await safe_handler(
5252
query_methodology.handler,
@@ -68,7 +68,7 @@ async def tool_detect_domain(
6868
cwd: str | None = None,
6969
project: str | None = None,
7070
first_message: str | None = None,
71-
) -> str:
71+
) -> dict:
7272
"""Lightweight domain classification."""
7373
return await safe_handler(
7474
detect_domain_handler.handler,
@@ -89,7 +89,7 @@ def _register_rebuild_profiles(mcp: FastMCP) -> None:
8989
async def tool_rebuild_profiles(
9090
domain: str | None = None,
9191
force: bool = False,
92-
) -> str:
92+
) -> dict:
9393
"""Full rescan of all session data to rebuild methodology profiles."""
9494
return await safe_handler(
9595
rebuild_profiles.handler,
@@ -106,7 +106,7 @@ def _register_list_domains(mcp: FastMCP) -> None:
106106
name="list_domains",
107107
**tool_kwargs(list_domains.schema),
108108
)
109-
async def tool_list_domains() -> str:
109+
async def tool_list_domains() -> dict:
110110
"""Overview of all detected cognitive domains."""
111111
return await safe_handler(list_domains.handler, {}, tool_name="list_domains")
112112

@@ -125,7 +125,7 @@ async def tool_record_session_end(
125125
keywords: list[str] | None = None,
126126
cwd: str | None = None,
127127
project: str | None = None,
128-
) -> str:
128+
) -> dict:
129129
"""Incremental profile update after a session ends."""
130130
return await safe_handler(
131131
record_session_end.handler,
@@ -150,7 +150,7 @@ def _register_get_methodology_graph(mcp: FastMCP) -> None:
150150
)
151151
async def tool_get_methodology_graph(
152152
domain: str | None = None,
153-
) -> str:
153+
) -> dict:
154154
"""Returns methodology map as graph data for 3D visualization."""
155155
return await safe_handler(
156156
get_methodology_graph.handler,
@@ -175,7 +175,7 @@ async def tool_query_workflow_graph(
175175
depth: int | None = None,
176176
domain: str | None = None,
177177
limit_nodes: int | None = None,
178-
) -> str:
178+
) -> dict:
179179
"""Return a typed subgraph of the unified workflow graph."""
180180
return await safe_handler(
181181
query_workflow_graph.handler,
@@ -198,7 +198,7 @@ def _register_open_visualization(mcp: FastMCP) -> None:
198198
)
199199
async def tool_open_visualization(
200200
domain: str | None = None,
201-
) -> str:
201+
) -> dict:
202202
"""Launch the 3D methodology constellation map in the browser."""
203203
return await safe_handler(
204204
open_visualization.handler,
@@ -216,7 +216,7 @@ async def tool_explore_features(
216216
mode: str,
217217
domain: str | None = None,
218218
compare_domain: str | None = None,
219-
) -> str:
219+
) -> dict:
220220
"""Explore interpretability features."""
221221
return await safe_handler(
222222
explore_features.handler,

mcp_server/tool_registry_ingest.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ async def tool_ingest_codebase(
3131
output_dir: str | None = None,
3232
language: str = "auto",
3333
force_reindex: bool = False,
34-
) -> str:
34+
) -> dict:
3535
"""Ingest upstream codebase analysis into Cortex.
3636
3737
No caps. Pulls every Function/Method/Struct/process the upstream
@@ -61,7 +61,7 @@ async def tool_change_impact(
6161
head: str = "HEAD",
6262
expand_impact: bool = False,
6363
apply_heat_bump: bool = False,
64-
) -> str:
64+
) -> dict:
6565
"""Report memories affected by a commit's code changes (ADR-0046 P4)."""
6666
return await safe_handler(
6767
change_impact.handler,
@@ -87,7 +87,7 @@ async def tool_ingest_prd(
8787
title: str | None = None,
8888
validate: bool = False,
8989
domain: str | None = None,
90-
) -> str:
90+
) -> dict:
9191
"""Ingest a PRD document into Cortex."""
9292
return await safe_handler(
9393
ingest_prd.handler,

mcp_server/tool_registry_manage.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ async def tool_forget(
4040
memory_id: int,
4141
soft: bool = False,
4242
force: bool = False,
43-
) -> str:
43+
) -> dict:
4444
"""Delete or soft-delete a memory by ID."""
4545
return await safe_handler(
4646
forget.handler,
@@ -65,7 +65,7 @@ async def tool_validate_memory(
6565
base_dir: str | None = None,
6666
staleness_threshold: float = 0.5,
6767
dry_run: bool = False,
68-
) -> str:
68+
) -> dict:
6969
"""Validate memories against current filesystem state."""
7070
return await safe_handler(
7171
validate_memory.handler,
@@ -89,7 +89,7 @@ def _register_rate_memory(mcp: FastMCP) -> None:
8989
async def tool_rate_memory(
9090
memory_id: int,
9191
useful: bool,
92-
) -> str:
92+
) -> dict:
9393
"""Rate a memory as useful or not to update metamemory confidence."""
9494
return await safe_handler(
9595
rate_memory.handler,
@@ -111,7 +111,7 @@ async def tool_seed_project(
111111
domain: str | None = None,
112112
max_file_size_kb: int = 64,
113113
dry_run: bool = False,
114-
) -> str:
114+
) -> dict:
115115
"""Bootstrap memory from an existing codebase."""
116116
return await safe_handler(
117117
seed_project.handler,
@@ -133,7 +133,7 @@ def _register_anchor(mcp: FastMCP) -> None:
133133
async def tool_anchor(
134134
memory_id: int,
135135
reason: str | None = None,
136-
) -> str:
136+
) -> dict:
137137
"""Mark a memory as compaction-resistant (heat=1.0)."""
138138
return await safe_handler(
139139
anchor.handler,
@@ -156,7 +156,7 @@ async def tool_backfill_memories(
156156
min_importance: float = 0.35,
157157
dry_run: bool = False,
158158
force_reprocess: bool = False,
159-
) -> str:
159+
) -> dict:
160160
"""Auto-import prior Claude Code conversations into memory."""
161161
return await safe_handler(
162162
backfill_memories.handler,
@@ -184,7 +184,7 @@ async def tool_codebase_analyze(
184184
incremental: bool = True,
185185
dry_run: bool = False,
186186
domain: str | None = None,
187-
) -> str:
187+
) -> dict:
188188
"""Analyze codebase and store structure as memories."""
189189
return await safe_handler(
190190
codebase_analyze.handler,

0 commit comments

Comments
 (0)