Skip to content

MCP: windowed get_message body reading#421

Open
endolith wants to merge 4 commits into
kenn-io:mainfrom
endolith:split/mcp1-get-message-paging
Open

MCP: windowed get_message body reading#421
endolith wants to merge 4 commits into
kenn-io:mainfrom
endolith:split/mcp1-get-message-paging

Conversation

@endolith

@endolith endolith commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Return body_text as a paginated slice instead of the full message body. Adds offset, center_at, max_chars (default 2000, max 4000), body_length, body_returned, and has_more so agents can page through long messages or jump to a match via center_at (byte offset from search_in_message, which doesn't exist yet, see endolith@d819d71).

Values above maxBodyChars clamp to 4000; zero or negative max_chars use the default.

Followup to #388

(Many of my emails are very long and contain entire conversations, so the LLM retrieving a bunch of them can kill the 1M token context window in a single response. These changes are meant to make the MCP tools return more narrowly-focused content to avoid wasting money and tokens and overwhelming the LLM.)

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (098a7fe)

High-level verdict: changes need fixes before merge due to a panic path and incorrect UTF-8 pagination metadata.

High

  • internal/mcp/handlers.go:718
    query.Engine.GetMessage implementations return (nil, nil) for not-found messages, but the new code dereferences msg after only checking err. A missing ID in the real MCP path can panic instead of returning a tool error.
    Fix: Check msg == nil after the error check and return a message not found tool error. Update the mock/test to cover the real engine contract.

Medium

  • internal/mcp/handlers.go:748
    bodyByteSlice adjusts UTF-8 boundaries inward, but offset, body_returned, and has_more are computed from the unadjusted start/end. Sequential paging with offset += body_returned can silently skip multibyte characters split at a chunk boundary.
    Fix: Return the adjusted start/end from the slicing helper and populate paging metadata from those values. Also ensure very small max_chars values still make progress or are rejected/clamped.

Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 1m43s), codex_security (codex/security, done, 12s) | Total: 2m3s

@cursor cursor Bot force-pushed the split/mcp1-get-message-paging branch 2 times, most recently from f25f1d1 to c1805ce Compare June 26, 2026 14:06
@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (c1805ce)

Verdict: No medium-or-higher severity findings to report.

Security review found no issues. The only reported finding was Low severity and is omitted per instructions.


Panel: ci_default_security | Synthesis: codex, 5s | Members: codex_default (codex/default, done, 1m49s), codex_security (codex/security, done, 16s) | Total: 2m10s

@wesm wesm marked this pull request as ready for review June 27, 2026 12:39
@wesm

wesm commented Jun 27, 2026

Copy link
Copy Markdown
Member

Written by Codex on Wes's behalf:

I’m worried this improves MCP response size but may hurt the actual “read this email” workflow.

get_message still calls h.engine.GetMessage(ctx, id) before slicing the body, and the SQLite/API paths still hydrate the full body from message_bodies. So max_chars only trims the JSON returned to the MCP client; it does not reduce DB/API read cost. For a long email, paging through the full body now means repeated tool calls, and each call reloads the full body before returning the next 2KB/4KB slice.

That makes the current defaults feel too small:

  • default max_chars = 2000
  • max max_chars = 4000

Those are good context-budget limits, but not good read-performance defaults unless body windowing happens at the storage/API layer.

I’d suggest one of these before merging:

  1. Raise the default and cap substantially so normal email reading does not require many repeated full-body loads.
  2. Make body paging opt-in, preserving full-body behavior by default.
  3. Better: add a backend/API method that fetches only (body_length, body_slice) for the requested window, then these paging defaults become a real performance win.

Two smaller concerns:

  • BodyHTML is always blanked, so HTML-only messages may become unreadable via MCP.
  • The tool description mentions search_in_message, but I don’t see that MCP tool in this branch.

@endolith

Copy link
Copy Markdown
Contributor Author

Raise the default and cap substantially so normal email reading does not require many repeated full-body loads.

I'll change it if you really want me to, but the size is already sufficient for most emails, no? I checked my latest 32 work emails and the biggest is 4232 characters. The rest are all under 4k and most are under 2k. (This is considering only the plain text main body of the email, and ignoring HTML and the dozens of previous emails that are quoted at the bottom.)

In practice, these are some typical sizes the agent requests when I'm using it:

  • 500×7, 600, 800, 2000×6, 3000×4, 4000×5

Make body paging opt-in, preserving full-body behavior by default.

Well the whole point of this PR is to make it impossible for the agent to load the full body at once, since even when I told it not to do so in the system message, it still ended up doing it and destroying its context window very quickly. In one case it couldn't even finish writing its first response to my first question and had an API error because it used up the entire (1M token!) context window in one response. (Good thing I'm using a cheap model.) In most cases, the agent is looking for facts within the email, not needing to read through the complete email sequentially, and in most cases the complete email fits within the first response anyway, and in the rare cases that these aren't true, it can page through it in multiple calls.

So max_chars only trims the JSON returned to the MCP client; it does not reduce DB/API read cost. For a long email, paging through the full body now means repeated tool calls, and each call reloads the full body before returning the next 2KB/4KB slice.

Better: add a backend/API method that fetches only (body_length, body_slice) for the requested window, then these paging defaults become a real performance win.

Well, the performance or implementation details aren't a problem for me, but if you want me to have Cursor work on a better implementation, or have your bots work on it, I am fine with either. But I've been using this for a few weeks and it works fine; the LLM is the speed bottleneck anyway. Could that be a separate Issue/PR though?

BodyHTML is always blanked, so HTML-only messages may become unreadable via MCP.

Well keyword search only looks in the plain text part anyway, right? In the case that an email has HTML but body text is empty, that email won't be returned from a search unless it happens to contain the keyword in the subject line or other metadata?

The tool description mentions search_in_message, but I don’t see that MCP tool in this branch.

Yes that will be the next PR. The changes I've been using in my fork are all lined up here: endolith#15

Return body_text as a paginated slice instead of the full message body.
Adds offset, center_at, max_chars (default 2000, max 4000), body_length,
body_returned, and has_more so agents can page through long messages or
jump to a match via center_at (byte offset from search_in_message).

Values above maxBodyChars clamp to 4000; zero or negative max_chars use
the default.

Guard against Engine.GetMessage returning (nil, nil) for not-found.
Return UTF-8-adjusted offset, body_returned, and has_more so sequential
paging (offset += body_returned) does not skip multibyte characters.

Co-authored-by: endolith <endolith@gmail.com>
@endolith endolith force-pushed the split/mcp1-get-message-paging branch from c1805ce to 4f5da6a Compare June 28, 2026 00:35
@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (4f5da6a)

Summary verdict: PR has two Medium issues to address; no Critical or High findings were reported.

Medium

  • Location: internal/mcp/handlers.go:778
    Problem: get_message now pages only msg.BodyText and always returns body_html as empty. Messages that have only BodyHTML stored will report body_length: 0 with no readable content via MCP, even though the query/API layers support HTML-only fallback.
    Fix: When BodyText is empty, derive text from BodyHTML or page/return BodyHTML explicitly, and add an HTML-only message test.

  • Location: internal/mcp/handlers.go:763
    Problem: offset and max_chars are applied only after h.engine.GetMessage loads the full message body. Reading beyond the first small window requires repeated calls that each reload the full body and metadata, which can make large-message reads slower than the previous single full-body response.
    Fix: Add a windowed body fetch path in the engine/store/API layer, or avoid forcing small paged reads until backend windowing exists.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 3m31s), codex_security (codex/security, done, 14s) | Total: 3m53s

@wesm

wesm commented Jun 28, 2026

Copy link
Copy Markdown
Member

I looked at my own msgvault and I have a lot of large emails, but I think this change makes sense. I'm working on a couple small refinements and then i'll try to get this merged

Windowed get_message responses are meant to protect MCP context, but blanking HTML made messages without a plain-text body appear unreadable. Preserve the context guard while falling back to a windowed body_html slice for HTML-only messages and advertise the selected body format so clients can interpret the response correctly.

The get_message tool description also referred to a follow-up search_in_message tool that is not part of this branch, so keep the current schema self-contained until that tool lands.

Generated with Codex (GPT-5)
Co-authored-by: Codex <codex@openai.com>
@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (52ed0ea)

Summary verdict: One medium regression found; no high or critical issues.

Medium

  • Location: internal/mcp/handlers.go:799
    • Problem: get_message now clears body_html whenever body_text is present, so mixed text/html messages lose access to their HTML representation even though the field still exists and was previously returned.
    • Fix: Preserve a way to request the HTML body, such as a body_format/format argument that selects text or HTML and applies the same windowing metadata to the selected body.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 3m35s), codex_security (codex/security, done, 50s) | Total: 4m32s

Windowed get_message should default to plain text for context safety, but mixed text/html messages still need a discoverable way to read the HTML representation. Add an explicit body_format selector so clients can page either representation without restoring unbounded full-body responses.

Generated with Codex (GPT-5)
Co-authored-by: Codex <codex@openai.com>
@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (74b8a60)

Summary verdict: One Medium issue remains; no High or Critical findings were reported.

Medium

  • internal/mcp/handlers.go:767: get_message still hydrates the full MessageDetail body before applying offset/max_chars, so paging through a large email reloads the entire body on every call. This reduces MCP response size but can make the “read the whole message” workflow significantly more expensive than the previous single full-body response.
    • Fix: Add an engine/API path that fetches metadata plus only the requested body window and total length, or provide an explicit full-body mode until backend windowed reads are available.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 3m21s), codex_security (codex/security, done, 1m6s) | Total: 4m33s

Windowed get_message defaults protect MCP context, but reading a whole long message through repeated pages reloads the same hydrated body each time. Add an explicit full_body escape hatch so clients can intentionally trade context budget for one full selected-body response until backend-side body slicing exists.

Generated with Codex (GPT-5)
Co-authored-by: Codex <codex@openai.com>
@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (3a4f0a3)

No issues found.


Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 4m38s), codex_security (codex/security, done, 13s) | Total: 4m51s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants