fix(stats): get-mail-stats failed on every path — undeclared schema key + sequential fan-out (#135) - #136
Merged
Merged
Conversation
Scoped calls died client-side with -32602 "data must NOT have additional properties"; unscoped calls timed out with -32001. Two separate faults. Schema: the CLIENT validates structuredContent against the ADVERTISED JSON Schema, and a bare zod raw shape renders as additionalProperties:false, so any undeclared field is fatal. get-mail-stats' IMAP branch spreads an ImapStats, which carries perMailbox. The server never noticed because zod's own parse strips unknown keys rather than failing — which is why the v2.3.0 migration's "all fields optional, no .strict()" read as permissive: it covered optionality, not undeclared keys. All 50 tools on the shipped 2.10.5 bundle advertised additionalProperties:false; get-mail-stats was just the one whose payload tripped it. Every tool now registers through a wrapper that applies .passthrough(), and the contract test fails any tool that regresses (50 offenders against 2.10.5, 0 against this). Timeout: the all-accounts path counted accounts sequentially, so wall clock was the sum over every account, each costing one IMAP STATUS per mailbox (Gmail lists every label). The pool is per-account and doesn't contend, so accounts are now counted concurrently — 16.9s -> 9.3s on three real accounts — and each is bounded by APPLE_MAIL_MCP_STATS_BUDGET_MS (default 25s). Also: a failed account no longer folds in as a silent zero. It sets partial:true + failedAccounts, the same treatment #130 gave get-unread-count and never applied here; a scoped call errors instead, since a partial result for the one account you named is no result. Fixes #135.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #135. Both faults @ismyemailaddress reported, plus a third the investigation turned up.
1. Schema rejection (scoped) — affected all 50 tools, latently
The client validates
structuredContentagainst the JSON Schema the server advertised, not against the server's own zod object. A bare zod raw shape renders asadditionalProperties: false, so any field the schema doesn't enumerate is a hard-32602.get-mail-stats's IMAP branch spreads anImapStats, which carriesperMailbox— never declared. The server never saw a problem because zod's own parse strips unknown keys rather than failing. That is precisely why the v2.3.0 migration's "all fields optional, no.strict()" was believed permissive: it covered optionality, not undeclared keys.Measured against the shipped 2.10.5 bundle: all 50 tools advertise
additionalProperties: false.get-mail-statswas simply the only one whose payload tripped it. So this is fixed for the whole surface, not one tool: every tool now registers through a wrapper applying.passthrough().Exact reproduction against a real account, shipped bundle vs this branch:
(The repro must call
listTools()beforecallTool()— the client only builds a validator from a cached tool list, which is why a naive script appears to pass.)2. Unscoped timeout
The all-accounts path counted accounts sequentially, so wall clock was the sum over every account, each costing one IMAP
STATUSper mailbox — and Gmail lists every label as a mailbox. Four accounts overran the request timeout and the call died as-32001with nothing returned.The pool is per-account and doesn't contend, so accounts are now counted concurrently (wall clock = slowest account, not the sum). Measured on three real accounts: 16.9s → 9.3s. Each account is additionally bounded by
APPLE_MAIL_MCP_STATS_BUDGET_MS(default 25s, min 1s), so one wedged account degrades to a partial result instead of taking the rest down.3. Silent zeros (found while fixing the above)
The IMAP fan-out's
catchlogged to stderr and continued, so an unreadable account contributed 0 to the totals with nothing in the result saying so — the same defect #130 fixed inget-unread-count, left behind here. Unreadable accounts now setpartial: true+failedAccounts. A scoped call errors instead, naming the budget and the remedy, since a partial result for the one account you asked about is no result.Guard
The contract test now fails any tool advertising
additionalProperties: false. The existing checks couldn't see this class — they inspect the advertised schema and round-trip onlyhealth-check/doctor, so a tool with an undeclared key passes CI and fails in the user's client, exactly as @ismyemailaddress predicted. Verified a real guard, not a tautology: 50 offenders against 2.10.5, 0 against this branch.Verification
lint(0 errors, 10 pre-existing warnings — same count asmain),typecheck,format:checkcleanDocs: README tool reference + env table,
docs/IMAP-SETUP.mdenv table,CLAUDE.mdagent guidance (apartialtotal is a floor, not an answer),skills/apple-mail/SKILL.mdviasync:skills. Version bumped to 2.10.6 with CHANGELOG under a real heading.