Skip to content

fix(mcp): _temporal_request must raise on non-2xx#47

Merged
amitpaz1 merged 1 commit into
mainfrom
fix-temporal-request-404-handling
May 8, 2026
Merged

fix(mcp): _temporal_request must raise on non-2xx#47
amitpaz1 merged 1 commit into
mainfrom
fix-temporal-request-404-handling

Conversation

@amitpaz1
Copy link
Copy Markdown
Collaborator

@amitpaz1 amitpaz1 commented May 8, 2026

Summary

  • Smoke-testing PR fix(audit): provenance + fallback gaps in recall and consolidation #46's new `consolidate_memories` MCP tool revealed a pre-existing bug: `HttpStore._request` returns 404 responses without raising, and the temporal MCP wrapper (`_temporal_request`) didn't check status codes — so 404s came back as `{"status": "ok", "id": null, ...}` to the agent.
  • Fix: `_temporal_request` now raises `RuntimeError` with the server's `message` / `detail` / `error` field on any non-2xx. Each tool's existing `except Exception as e: return f"Failed to ..."` clause turns it into a clear "Failed to ..." surface for the agent.
  • Affects every Phase 6F MCP tool that goes through `_temporal_request`: `supersede`, `list_at_time`, `supersession_chain`, plus the new `consolidate_memories` and `provenance` from PR fix(audit): provenance + fallback gaps in recall and consolidation #46.

Test plan

  • `ruff check src/ tests/` — clean
  • `pytest tests/test_mcp_provenance_and_recall.py` — 11 passed
  • 4 new tests covering: 2xx pass-through, 404 with `message` field, 422 with FastAPI's `detail` field, end-to-end consolidate_memories surfacing the route's 404 message
  • CI green

🤖 Generated with Claude Code

HttpStore._request returns the raw response on 4xx/404 instead of
raising, so the MCP temporal wrapper was formatting error bodies as
``{"status": "ok", "id": null, "superseded_count": null}``. Smoke
test on a freshly merged PR #46 surfaced this — calling
consolidate_memories with bogus source ids reported success despite
the route returning 404 "Source memory not found: nope".

Fix: _temporal_request now inspects status_code and raises
RuntimeError with the server's message / detail / error field on any
non-2xx. The existing ``except Exception as e: return f"Failed to ..."``
clauses on supersede / consolidate_memories / provenance /
supersession_chain / list_at_time turn this into a clear Failed-to
message at the MCP boundary.

Test plan: 4 new tests in test_mcp_provenance_and_recall.py covering
2xx pass-through, 404 with message, 422 with FastAPI's "detail"
field, and the end-to-end consolidate_memories surface that proved
the bug live.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@amitpaz1 amitpaz1 merged commit 885bccf into main May 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant