Skip to content

fix(mcp): prevent get_bank from creating banks #2393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sanderhoff-alt wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −0
Open

fix(mcp): prevent get_bank from creating banks #2393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions hindsight-api-slim/hindsight_api/mcp_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3129,7 +3129,10 @@ async def get_bank(
profile = await memory.get_bank_profile(
target_bank,
request_context=_get_request_context(config),
create_if_missing=False,
)
if profile is None:
return json.dumps({"error": f"Bank '{target_bank}' not found"})
if "disposition" in profile and hasattr(profile["disposition"], "model_dump"):
profile["disposition"] = profile["disposition"].model_dump()
return json.dumps(profile, indent=2, default=str)
Expand Down Expand Up @@ -3157,7 +3160,10 @@ async def get_bank() -> dict:
profile = await memory.get_bank_profile(
target_bank,
request_context=_get_request_context(config),
create_if_missing=False,
)
if profile is None:
return {"error": f"Bank '{target_bank}' not found"}
if "disposition" in profile and hasattr(profile["disposition"], "model_dump"):
profile["disposition"] = profile["disposition"].model_dump()
return profile
Expand Down
16 changes: 16 additions & 0 deletions hindsight-api-slim/tests/test_mcp_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1511,6 +1511,14 @@ async def test_get_bank(self, mock_memory):
mcp = _make_mcp_server(mock_memory, {"get_bank"}, include_bank_id=True)
result = await _tools(mcp)["get_bank"].fn()
assert '"test-bank"' in result or "test-bank" in result
assert mock_memory.get_bank_profile.call_args.kwargs["create_if_missing"] is False

async def test_get_bank_missing_does_not_create(self, mock_memory):
mock_memory.get_bank_profile.return_value = None
mcp = _make_mcp_server(mock_memory, {"get_bank"}, include_bank_id=True)
result = await _tools(mcp)["get_bank"].fn(bank_id="missing-bank")
assert json.loads(result)["error"] == "Bank 'missing-bank' not found"
assert mock_memory.get_bank_profile.call_args.kwargs["create_if_missing"] is False

async def test_get_bank_stats(self, mock_memory):
mcp = _make_mcp_server(mock_memory, {"get_bank_stats"}, include_bank_id=True)
Expand Down Expand Up @@ -1556,6 +1564,14 @@ async def test_get_bank_single_bank(self, mock_memory):
mcp = _make_mcp_server(mock_memory, {"get_bank"}, include_bank_id=False)
result = await _tools(mcp)["get_bank"].fn()
assert isinstance(result, dict)
assert mock_memory.get_bank_profile.call_args.kwargs["create_if_missing"] is False

async def test_get_bank_single_bank_missing_does_not_create(self, mock_memory):
mock_memory.get_bank_profile.return_value = None
mcp = _make_mcp_server(mock_memory, {"get_bank"}, include_bank_id=False)
result = await _tools(mcp)["get_bank"].fn()
assert result["error"] == "Bank 'test-bank' not found"
assert mock_memory.get_bank_profile.call_args.kwargs["create_if_missing"] is False

async def test_delete_bank_single_bank(self, mock_memory):
mcp = _make_mcp_server(mock_memory, {"delete_bank"}, include_bank_id=False)
Expand Down