test(handler): drive tools/call through oneshot, killing two reds-nothing #39

Merged

jlxq0 merged 1 commit from feat/handler-harness into main

2026-09-01 18:03:51 +00:00

jlxq0 commented

2026-09-01 18:00:46 +00:00

Owner

Closes #31, and closes the residual from #33 with the same harness.

The two mutations that redded nothing now red

mutation	red
handler discards `build_email_object`'s `Result` (`?` → `unwrap_or_else`)	`send_email_with_html_and_empty_text_is_an_error_to_the_caller`
handler hard-codes `body_html: None`	`read_email_returns_the_html_the_backend_served`

Neither was reachable by a unit test on either function, because the fault is at the call site. The assertion is on what the caller receives, isError and structuredContent, not on what a function returns.

The harness

initialize, then one tools/call, against a mock JMAP backend that answers every method in one methodResponses body (each caller filters by name, so one body serves Mailbox/get, Identity/get, Email/get and both sets).

Two details, and the second is the one that will bite the next person.

Request::builder() sets no Host on a relative URI, so every call is 400 Bad Request: missing Host header until one is set.

Clone the router; never build a second one. A fresh router(cfg) is identical in construction and shares no session manager, so initialize returns 200 with a session id and the second call answers 404 Session not found — which reads as a session bug rather than as two routers. Router being Clone is what makes the wrong version look correct, because rebuilding is exactly what a careful person does to avoid shared state. Same shape as a fixed name in a shared namespace, except the namespace is one process and the two actors are two calls in one test.

Mutation-checked: rebuilding instead of cloning reds both handler tests on assertion left == right failed: tools/call transport.

Why this was affordable

Auth in the harness is a Stalwart app password against the mock. Before the Basic path existed, an authenticated handler test needed a Logto-signed JWT: mock the JWKS, generate a key, sign a token. Two days of being careful about widening the auth surface produced the thing that makes an untested error path testable, and nobody planned it. AGENTS.md records it, because the next person weighing a similar widening will see the cost and not this.

Also in the diff

The index-versus-single-manifest count widens from five servers to six, adding m365-mcp to the single-manifest side, and now says plainly that this repository is the trap rather than the exception: someone who learns "our MCP servers are single-manifest" from the other five and applies it here gets a confident mismatch on a correctly deployed image.

Gate

Toolchain read from ci.yml: 1.93.0, through build-slot.sh.

cargo +1.93.0 fmt --all --check                                          rc=0
cargo +1.93.0 clippy --all-targets --all-features --locked -- -D warnings rc=0
cargo +1.93.0 test --all-features --locked                               rc=0
running 197 tests
test result: ok. 197 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

The first clippy run was rc=101 on clippy::panic in the SSE parser. Restructured into a helper carrying the allow, rather than allowing panics across the whole test module.

No version bump: tests only, plus AGENTS.md.

Closes #31, and closes the residual from #33 with the same harness. ## The two mutations that redded nothing now red | mutation | red | |---|---| | handler discards `build_email_object`'s `Result` (`?` → `unwrap_or_else`) | `send_email_with_html_and_empty_text_is_an_error_to_the_caller` | | handler hard-codes `body_html: None` | `read_email_returns_the_html_the_backend_served` | Neither was reachable by a unit test on either function, because **the fault is at the call site**. The assertion is on what the caller receives, `isError` and `structuredContent`, not on what a function returns. ## The harness `initialize`, then one `tools/call`, against a mock JMAP backend that answers every method in one `methodResponses` body (each caller filters by name, so one body serves `Mailbox/get`, `Identity/get`, `Email/get` and both `set`s). **Two details, and the second is the one that will bite the next person.** `Request::builder()` sets no `Host` on a relative URI, so every call is `400 Bad Request: missing Host header` until one is set. **Clone the router; never build a second one.** A fresh `router(cfg)` is identical in construction and shares **no** session manager, so `initialize` returns `200` with a session id and the second call answers `404 Session not found` — which reads as a session bug rather than as two routers. **`Router` being `Clone` is what makes the wrong version look correct**, because rebuilding is exactly what a careful person does to avoid shared state. Same shape as a fixed name in a shared namespace, except the namespace is one process and the two actors are two calls in one test. Mutation-checked: rebuilding instead of cloning reds **both** handler tests on `assertion left == right failed: tools/call transport`. ## Why this was affordable Auth in the harness is a Stalwart app password against the mock. **Before the Basic path existed, an authenticated handler test needed a Logto-signed JWT**: mock the JWKS, generate a key, sign a token. Two days of being careful about widening the auth surface produced the thing that makes an untested error path testable, and nobody planned it. `AGENTS.md` records it, because the next person weighing a similar widening will see the cost and not this. ## Also in the diff The index-versus-single-manifest count widens from five servers to six, adding `m365-mcp` to the single-manifest side, and now says plainly that **this repository is the trap rather than the exception**: someone who learns "our MCP servers are single-manifest" from the other five and applies it here gets a confident mismatch on a correctly deployed image. ## Gate Toolchain read from `ci.yml`: `1.93.0`, through `build-slot.sh`. ``` cargo +1.93.0 fmt --all --check rc=0 cargo +1.93.0 clippy --all-targets --all-features --locked -- -D warnings rc=0 cargo +1.93.0 test --all-features --locked rc=0 running 197 tests test result: ok. 197 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ``` The first clippy run was `rc=101` on `clippy::panic` in the SSE parser. Restructured into a helper carrying the allow, rather than allowing panics across the whole test module. No version bump: tests only, plus `AGENTS.md`.

jlxq0 added 1 commit

2026-09-01 18:00:46 +00:00

test(handler): drive tools/call through oneshot, killing two reds-nothing

CI / tag-ancestry (pull_request) Successful in 3s

Details

CI / cargo (pull_request) Successful in 58s

Details

CI / docker (pull_request) Successful in 45s

Details

c99a7343db

Both mutations reported as redding nothing now red, each on its own test.

  handler discards build_email_object's Result
    -> send_email_with_html_and_empty_text_is_an_error_to_the_caller
  handler hard-codes body_html: None
    -> read_email_returns_the_html_the_backend_served

Neither was reachable by a unit test on either function, because the fault is
at the call site: a discarded ? is invisible to a test on the callee and
visible to a request that gets a result where it should get an error. The
assertion is on what the caller receives, isError and structuredContent.

The harness does initialize then one tools/call against a mock JMAP backend.
Two details in it:

Request::builder sets no Host on a relative URI, so every call is 400 Bad
Request: missing Host header until one is set.

The router must be CLONED, never rebuilt. A fresh router(cfg) is identical in
construction and shares no session manager, so initialize returns 200 with a
session id and the second call answers 404 Session not found, which reads as a
session bug rather than as two routers. Router being Clone is what makes the
wrong version look correct, because rebuilding is what a careful person does to
avoid shared state. Mutation-checked: rebuilding reds both handler tests on
"assertion left == right failed: tools/call transport".

Auth in the harness is a Stalwart app password against the mock. That is only
affordable because of the Basic path added for a second identity: before it, an
authenticated handler test needed a Logto-signed JWT, so mocking the JWKS,
generating a key and signing a token. AGENTS.md records that, because the next
person weighing a similar widening will see the cost and not this.

Also widens the index-versus-single-manifest count from five servers to six,
adding m365-mcp to the single-manifest side, and says plainly that this
repository is the trap rather than the exception.

Gate on 1.93.0 from ci.yml through build-slot.sh: fmt rc=0, clippy -D warnings
rc=0, test rc=0, 197 passed. The first clippy run was rc=101 on clippy::panic
in the SSE parser; restructured rather than allowed across the module.

Closes #31

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

jlxq0 merged commit 65691a9447 into main

2026-09-01 18:03:51 +00:00

jlxq0 referenced this pull request from a commit

2026-09-01 18:03:51 +00:00

Merge pull request 'test(handler): drive tools/call through oneshot, killing two reds-nothing' (#39) from feat/handler-harness into main

jlxq0 deleted branch feat/handler-harness

2026-09-01 18:03:51 +00:00