chore-mcp-integration-tests-stale-skip-2026-06-10: revive MCP suite#72
Conversation
… integration suite, add handler error-path tests
Remove the stale module-level skip ("Awaiting Phase 2 episodic service
layer" — Phase 2 shipped long ago) so the MCP stdio integration suite
runs in the default test suite again. Delete
test_memory_start_ui_returns_stub_error, which asserted a stub error
for a tool that is now fully implemented. Assert the wire-visible tool
list against _tool_definitions(supports_synthesis=True) instead of a
hardcoded six-tool set.
Add eight error-path tests that are only reachable through the real
dispatch layer: missing required argument, invalid enum value (SDK
inputSchema validation), unknown observation id, invalid outcome enum,
episodes CHECK-constraint violation, malformed synthesize_next_apply
decision dict, stale episode_id, and already-synthesized episode.
Test plan: tests/mcp/test_server_integration.py 14/14 passed; full
suite 1124 passed / 17 skipped / 0 failed (browser UI tests excluded
in the sandboxed environment; pre-existing and unrelated).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 1 potential bug in this PR.
medium: 1
One logic bug in the new error-path tests: test_close_episode_constraint_violation_is_error uses an outcome value rejected by MCP SDK input validation, so the DB CHECK-constraint code path it claims to test is never reached.
|
|
||
| resp = await session.call_tool( | ||
| "memory.close_episode", | ||
| {"outcome": "bogus", "close_reason": "goal_complete"}, |
There was a problem hiding this comment.
🟡 MEDIUM: Test never reaches the claimed DB CHECK-constraint path
test_close_episode_constraint_violation_is_error passes {"outcome": "bogus", "close_reason": "goal_complete"} to memory.close_episode. The outcome field has an input-schema enum of ["success", "partial", "abandoned", "no_outcome"] (verified in server.py:520-527), so the MCP SDK rejects "bogus" with an input-validation error before the handler runs. The docstring explicitly claims this exercises "the DB-level constraint path" and that passing close_reason explicitly avoids the default_reasons[outcome] lookup masking the DB path — but neither the handler nor the DB is ever reached. The test will pass (an error is returned), but it provides no coverage of the DB CHECK-constraint path it claims to cover. To test a DB-level constraint violation, outcome must be a value that passes SDK enum validation but triggers the constraint in SQLite; since the SDK enum and DB CHECK constraint guard the same set of values, the DB constraint is structurally unreachable via MCP and this test cannot be written as described.
1 participant
Revives the MCP integration test suite that was parked behind a stale module-level skip ("Awaiting Phase 2 episodic service layer" — Phase 2 shipped long ago) and adds error-path coverage for tool handlers in
better_memory/mcp/server.py, the repo's #1 hotspot. The obsoletetest_memory_start_ui_returns_stub_error(asserting a stub error for a now-implemented tool) is deleted, the tools-list assertion is hardened against_tool_definitions(supports_synthesis=True), and 8 new handler error-path tests cover: missing required arg, invalid enum via inputSchema validation, unknown observation id, invalid outcome enum, episodes CHECK-constraint violation, malformed synthesis decision, stale episode_id, and already-synthesized episode.Docs unaffected — test-only change, no tool registrations or schemas modified.
Closes chore-mcp-integration-tests-stale-skip-2026-06-10.
Test plan
tests/mcp/test_server_integration.py: 14/14 passed (previously 0 ran — whole module skipped)PBI bundle:
ralph-queuebranch,.ralph/current/chore-mcp-integration-tests-stale-skip-2026-06-10/(moves topending-pr/after this PR opens).