Skip to content

Stop default openmemory_query from dropping the HTTP session - #212

Open
vincenzopalazzo wants to merge 1 commit into
CaviraOSS:mainfrom
vincenzopalazzo:fix/mcp-query-hsg-fail-soft
Open

Stop default openmemory_query from dropping the HTTP session#212
vincenzopalazzo wants to merge 1 commit into
CaviraOSS:mainfrom
vincenzopalazzo:fix/mcp-query-hsg-fail-soft

Conversation

@vincenzopalazzo

Copy link
Copy Markdown
Contributor

Summary

  • Default openmemory_query (type=contextual) ran hsg_query with no try/catch or timeout. A hung embed, rate-limit throw, or long vector writeback dropped the per-request Streamable HTTP POST (Streamable HTTP error: Error POSTing to endpoint, empty status), so the first query looked like a total outage.
  • Wrap HSG and factual search in a 12s timeout (OM_MCP_QUERY_TIMEOUT_MS) and return a normal MCP tool error (isError + readable message) instead of letting the transport die. Unified queries fail-soft on HSG and still return facts.
  • Unpatterned factual queries dumped the whole temporal graph (1MB+). Cap them at k (patterned queries still cap at 32). Same fail-soft + cap on the Python MCP path.

Test plan

  • npx vitest run tests/mcp_query_failsoft.test.ts (7 tests)
  • npm test in packages/openmemory-js (65 tests)
  • npm run typecheck
  • CI passes
  • Default openmemory_query with no type returns a tool result or isError, never a transport drop
  • list / get / store / delete / type=factual with fact_pattern still work

Default openmemory_query runs HSG with no timeout, so a hung embed
or vector writeback dropped the Streamable HTTP POST. Catch HSG
failures as MCP tool errors, time out after 12s, and cap unpatterned
factual dumps at k.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70a92566a7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +208 to +209
facts = await asyncio.wait_for(
query_facts_at_time(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Run the Python fact query off the event-loop thread

When a factual query encounters a slow or locked SQLite database, this timeout cannot fire: query_facts_at_time() contains no suspension point and calls the synchronous db.fetchall() directly, so it blocks the same event loop that must schedule asyncio.wait_for's timer. Consequently OM_MCP_QUERY_TIMEOUT_MS remains ineffective for factual searches and the Python MCP session can still hang beyond the configured deadline; offload the database operation to a thread or make the query genuinely asynchronous.

Useful? React with 👍 / 👎.

Comment on lines +187 to +190
except asyncio.TimeoutError:
msg = f"HSG contextual search timed out after {int(timeout_s * 1000)}ms"
if qtype == "contextual":
return [TextContent(type="text", text=f"openmemory_query failed: {msg}")]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mark Python timeout results as MCP tool errors

When the Python contextual query times out, returning a plain list of TextContent is wrapped by the low-level MCP server as a successful tool result (isError remains false). Clients therefore cannot distinguish this failure from valid output and may proceed instead of retrying or reporting a tool failure, unlike the JavaScript path added here; return an error-bearing CallToolResult or propagate an exception that the MCP server converts to isError.

Useful? React with 👍 / 👎.

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