feat(compose): optional body_html on send_email #30

Merged

jlxq0 merged 2 commits from html-body into main 2026-08-29 14:27:15 +00:00

Conversation 1 Commits 2 Files changed 2 +204 -23

jlxq0 commented 2026-08-29 14:19:16 +00:00

Owner

Copy link

Closes #29. send_email built a text-only email and the schema had no way to say otherwise, so an agent composing a styled briefing had no route at all.

Nothing outside this repo moves

This is the one part of the change that can break something elsewhere, so it is measured rather than asserted:

git diff origin/main -- src/mcp.rs | grep -E '^[+-].*(tool\(|description =|annotations|read_only_hint|async fn send_email)'
  (no output)

Read back from src/mcp.rs as it stands, immediately above async fn send_email:

title = "Send email",
read_only_hint = false,
destructive_hint = false,
idempotent_hint = false

The tool is still send_email, the description is unchanged, and read_only_hint is still false. tool-scope.sh generates the same deny lines and no mount's scope moves. One file changed.

Behaviour

• body_html absent: the Email object is byte-identical to what this tool sent before the parameter existed. Asserted as a whole object, not spot checks, because an extra key passes every assertion about the keys you named.
• body_html present: both bodies go out with distinct part ids, textBody naming the plain one and htmlBody the HTML one. Sharing an id would make them the same part and the client would render whichever it found first.
• HTML with empty or whitespace body_text: invalid_params. A briefing that arrives blank on a client that will not render HTML is worse than an unstyled one.

No sanitiser on the outbound HTML. It is composed by the calling agent rather than received from anyone, which is the distinction read_email's wrapping already draws. A test pins the pass-through, including a style attribute and an & in a query string, so nobody adds one later by reflex.

Four mutations, each red on exactly one test

what I broke	what it said	red
build_email_object, the if let Some(html) line: emit htmlBody when None	left had "htmlBody": Array [], right did not	without_html_the_object_is_unchanged
htmlBody part id "h" → "b", shared with the text part	assertion left == right failed: one bodyValue per part	with_html_both_bodies_go_out_with_distinct_part_ids
the bodyValues["h"] line: strip a style attribute first	value no longer equalled the composed html	outbound_html_is_passed_through_unaltered
validate_send_bodies, the guard condition made unreachable	must be refused: ()	html_without_text_is_invalid_params

Each left the other three green.

And the mutation that tells you nothing, run deliberately for contrast: build_email_object returning Value::Null reds three at once. That is the precondition mutation, and it is why the four above are targeted at one line each.

Gate

Toolchain read from ci.yml rather than assumed: toolchain: 1.93.0. Run through build-slot.sh.

fmt    rc=0
clippy rc=0
test   rc=0

clippy, tail:

        Checking jmap-mcp v0.2.18 (/Users/jl/Forge/jmap-mcp/wt/html-body/src)
        Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.00s

test:

    running 187 tests
    test result: ok. 187 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.23s

The first gate run was rc=101: needless_pass_by_value on from_header: Value at src/mcp.rs:935, -D clippy::needless-pass-by-value implied by -D warnings. Taken by reference as clippy suggested, and all four mutations re-verified afterwards because the signature changed under them.

Closes #29. `send_email` built a text-only email and the schema had no way to say otherwise, so an agent composing a styled briefing had no route at all. ## Nothing outside this repo moves This is the one part of the change that can break something elsewhere, so it is measured rather than asserted: ``` git diff origin/main -- src/mcp.rs | grep -E '^[+-].*(tool\(|description =|annotations|read_only_hint|async fn send_email)' (no output) ``` Read back from `src/mcp.rs` as it stands, immediately above `async fn send_email`: ``` title = "Send email", read_only_hint = false, destructive_hint = false, idempotent_hint = false ``` The tool is still `send_email`, the description is unchanged, and `read_only_hint` is still `false`. `tool-scope.sh` generates the same deny lines and no mount's scope moves. One file changed. ## Behaviour - **`body_html` absent**: the `Email` object is byte-identical to what this tool sent before the parameter existed. Asserted as a **whole object**, not spot checks, because an extra key passes every assertion about the keys you named. - **`body_html` present**: both bodies go out with **distinct part ids**, `textBody` naming the plain one and `htmlBody` the HTML one. Sharing an id would make them the same part and the client would render whichever it found first. - **HTML with empty or whitespace `body_text`**: `invalid_params`. A briefing that arrives blank on a client that will not render HTML is worse than an unstyled one. **No sanitiser on the outbound HTML.** It is composed by the calling agent rather than received from anyone, which is the distinction `read_email`'s wrapping already draws. A test pins the pass-through, including a `style` attribute and an `&` in a query string, so nobody adds one later by reflex. ## Four mutations, each red on exactly one test | what I broke | what it said | red | |---|---|---| | `build_email_object`, the `if let Some(html)` line: emit `htmlBody` when `None` | `left` had `"htmlBody": Array []`, `right` did not | `without_html_the_object_is_unchanged` | | `htmlBody` part id `"h"` → `"b"`, shared with the text part | `assertion left == right failed: one bodyValue per part` | `with_html_both_bodies_go_out_with_distinct_part_ids` | | the `bodyValues["h"]` line: strip a `style` attribute first | value no longer equalled the composed html | `outbound_html_is_passed_through_unaltered` | | `validate_send_bodies`, the guard condition made unreachable | `must be refused: ()` | `html_without_text_is_invalid_params` | Each left the other three green. **And the mutation that tells you nothing**, run deliberately for contrast: `build_email_object` returning `Value::Null` reds **three at once**. That is the precondition mutation, and it is why the four above are targeted at one line each. ## Gate Toolchain read from `ci.yml` rather than assumed: `toolchain: 1.93.0`. Run through `build-slot.sh`. ``` fmt rc=0 clippy rc=0 test rc=0 ``` clippy, tail: ``` Checking jmap-mcp v0.2.18 (/Users/jl/Forge/jmap-mcp/wt/html-body/src) Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.00s ``` test: ``` running 187 tests test result: ok. 187 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.23s ``` The first gate run was **rc=101**: `needless_pass_by_value` on `from_header: Value` at `src/mcp.rs:935`, `-D clippy::needless-pass-by-value implied by -D warnings`. Taken by reference as clippy suggested, and all four mutations re-verified afterwards because the signature changed under them.

jlxq0 added 1 commit 2026-08-29 14:19:16 +00:00

feat(compose): optional body_html on send_email ... 6de1fd1d0d

send_email built a text-only email and the schema had no way to say otherwise,
so an agent composing a styled briefing had no route at all.

body_html is optional and defaults to None. Absent, the Email object is
byte-identical to what this tool sent before the parameter existed, which is
asserted as a whole object rather than as spot checks: an extra key passes
every assertion about the keys you named.

Present, both bodies go out with distinct part ids, textBody naming the plain
one and htmlBody the HTML one. Sharing an id would make them the same part and
the client would render whichever it found first.

HTML with an empty or whitespace body_text is invalid_params. A briefing that
arrives blank on a client that will not render HTML is worse than an unstyled
one, and the caller composing the HTML is the caller who forgets the fallback.
Structural rather than left to whoever composes.

The outbound HTML is not sanitised. It is composed by the calling agent rather
than received from anyone, which is the distinction read_email's wrapping
already draws; rewriting it would corrupt legitimate markup for no threat
model. A test pins the pass-through so nobody adds one later by reflex.

Extended send_email rather than adding a tool. The tool name, description and
annotations are untouched, read_only_hint stays false, so tool-scope.sh
generates the same deny lines and no mount's scope moves.

The Email construction moved into build_email_object so the wire shape can be
asserted without a mail server.

Four mutations, each red on exactly one test and green on the other three:
  emit htmlBody when body_html is None -> without_html_the_object_is_unchanged
  share one partId between the bodies  -> with_html_both_bodies_..._part_ids
  rewrite the outbound html            -> outbound_html_is_passed_through_unaltered
  remove the empty-text guard          -> html_without_text_is_invalid_params

For contrast, returning Value::Null from build_email_object reds three at once
and discriminates nothing, which is the precondition mutation to avoid.

Gate on 1.93.0, the toolchain read from ci.yml, through build-slot.sh:
fmt rc=0, clippy -D warnings rc=0, test rc=0, 187 passed.

Closes #29

jlxq0 added 1 commit 2026-08-29 14:25:32 +00:00

fix(compose): validate inside the builder, not in a guard the handler calls ... a0a1a9ff18

Alan found the hole in review: validate_send_bodies was tested by testing the
function, and nothing held that it was reached. Deleting its single call site
in send_email left all 187 tests green while HTML-with-empty-text sent in
production. Reproduced here before changing anything.

All four of my mutations targeted function bodies and none targeted
reachability, so the set looked thorough with one whole class outside it.

The check now lives inside build_email_object, which returns Result. There is
no call to omit, and html_without_text_is_invalid_params reaches it through the
one path the handler uses, so removing the guard reds that test. An object
builder that cannot construct an invalid object beats a guard somebody has to
remember to call.

Mutations, each red on exactly one test and green on the other three:
  emit htmlBody when body_html is None -> without_html_the_object_is_unchanged
  share one partId between the bodies  -> with_html_both_bodies_..._part_ids
  rewrite the outbound html            -> outbound_html_is_passed_through_unaltered
  remove the guard from the builder    -> html_without_text_is_invalid_params

And one that reds NOTHING, reported rather than dropped: replacing the
handler's `?` with unwrap_or_else(|_| json!({})) discards the error and no test
notices. Closing that needs a test driving the handler itself; no unit test on
either function reaches it.

AGENTS.md records the class: a mutation set that only edits function bodies
cannot see whether the function is called, and mutate the call site as well as
the callee.

Gate on 1.93.0 from ci.yml, through build-slot.sh: fmt rc=0, clippy rc=0,
test rc=0, 187 passed.

Closes #29

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

jlxq0 commented 2026-08-29 14:27:14 +00:00

Author

Owner

Copy link

Merging. Gate run here on 1.93.0 through build-slot.sh, and I reproduced both the fix and the hole you reported rather than taking either.

fmt rc=0   clippy rc=0   test rc=0   187 passed; 0 failed
declaration diff for tool(/description/annotations/read_only_hint: empty

The residual is real and I confirmed it, same mutation, same result:

build_email_object(...)?  ->  .unwrap_or_else(|_| json!({}))
187 passed; 0 failed

Merging with it open, and the reason is that the two failures are not the same size. Deleting a validation call is something a person does while tidying and leaves no trace in review. Writing unwrap_or_else(|_| json!({})) over a ? is a deliberate act that a reader sees. The guard can no longer be omitted by accident, which was the whole of what I held for.

Reporting a mutation that reds nothing, rather than dropping it, is what made this decision available. A set of four all-red mutations and a sentence saying the wiring is covered would have been accepted and would have been wrong, and I would not have found it twice.

Filed as jlxq0/jmap-mcp#31: a handler-level harness that drives send_email far enough to hold its error path. Its own PR, not this one, and not tonight.

What goes in AGENTS.md from this

A mutation set that only edits function bodies cannot see whether the function is called. All four of the first set were thorough, targeted, each red on exactly one test, and every one pointed a layer below the fault. Coverage of a guard reads as coverage of its use, which is the same shape as caldav-mcp's 120 tests over a reverted default.

Delete the call, not just the function.

**Merging. Gate run here on 1.93.0 through `build-slot.sh`, and I reproduced both the fix and the hole you reported rather than taking either.** fmt rc=0 clippy rc=0 test rc=0 187 passed; 0 failed declaration diff for tool(/description/annotations/read_only_hint: empty **The residual is real and I confirmed it**, same mutation, same result: build_email_object(...)? -> .unwrap_or_else(|_| json!({})) 187 passed; 0 failed **Merging with it open, and the reason is that the two failures are not the same size.** Deleting a validation call is something a person does while tidying and leaves no trace in review. Writing `unwrap_or_else(|_| json!({}))` over a `?` is a deliberate act that a reader sees. The guard can no longer be omitted by accident, which was the whole of what I held for. **Reporting a mutation that reds nothing, rather than dropping it, is what made this decision available.** A set of four all-red mutations and a sentence saying the wiring is covered would have been accepted and would have been wrong, and I would not have found it twice. Filed as `jlxq0/jmap-mcp#31`: a handler-level harness that drives `send_email` far enough to hold its error path. Its own PR, not this one, and not tonight. ## What goes in `AGENTS.md` from this **A mutation set that only edits function bodies cannot see whether the function is called.** All four of the first set were thorough, targeted, each red on exactly one test, and every one pointed a layer below the fault. **Coverage of a guard reads as coverage of its use**, which is the same shape as `caldav-mcp`'s 120 tests over a reverted default. **Delete the call, not just the function.**

jlxq0 merged commit 32fa6e7dc6 into main 2026-08-29 14:27:15 +00:00

jlxq0 referenced this pull request from a commit 2026-08-29 14:27:15 +00:00

Merge pull request 'feat(compose): optional body_html on send_email' (#30) from html-body into main

jlxq0 referenced this pull request from a commit 2026-08-29 14:28:05 +00:00

chore(release): v0.2.19

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

waiting-on-julian

Blocked on Julian. The ask is in the issue.

No labels

waiting-on-julian

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jlxq0/jmap-mcp!30

No description provided.