MCP structured output, explicit annotations, native error semantics, attachment hardening, test suites#43
Merged
Merged
Conversation
…ntic v2 Pydantic v2 requires an explicit default=None (not just Optional[X] typing) or the field is mandatory both for direct construction and in the generated JSON Schema's "required" list. Nearly every Optional field in schemas.py was missing this, breaking ordinary usage of list_inboxes, create_inbox, list_threads, send_message, reply_to_message, forward_message, and update_message whenever the caller omitted an optional field, and mislabeling those fields as required in the JSON Schema shown to tool-calling models. Also adds ascending/include_spam/include_blocked/include_trash to list_threads' params -- already supported by the pinned agentmail==0.4.10 SDK, a schema-only gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inbox, harden attachment download - create_inbox: the pinned agentmail==0.4.10 SDK requires inboxes.create(request=CreateInboxRequest(...)), not flat kwargs. The old call raised TypeError on any non-empty call, i.e. every real use of the tool. - get_attachment: use the inbox-scoped client.inboxes.threads.get_attachment (which actually takes inbox_id) instead of the unscoped client.threads.get_attachment, which silently ignored the inbox_id the schema already required from callers -- matches Node's equivalent fix. - get_attachment: enforce https-only download URLs (urlopen's default opener also handles file:// and ftp://), a 15s timeout, and a 25MB size cap (checked via attachment.size before fetching, and via a bounded read after) before buffering the attachment body. - get_attachment: replace the bare `except:` (which swallowed KeyboardInterrupt/SystemExit and silently discarded every extraction failure) with `except Exception` plus a logged warning carrying the attachment_id and error text only -- never the file body. - reply_to_message/forward_message: stop mutating the caller-owned kwargs dict via .pop(); operate on a local copy instead, since livekit.py passes its raw framework dict through with no defensive copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stop blocking the event loop All three adapters previously caught every exception and returned it as an ordinary "successful" value (a bare string in openai.py, a ToolError Pydantic object in langchain.py), making a failed AgentMail API call indistinguishable from a real result to the calling agent -- the same class of problem this branch already fixed on the Node MCP binding. - util.py: replace safe_func/ToolError with api_error_message(), an agentmail.core.ApiError-aware extractor mirroring node/src/util.ts's apiErrorMessage -- pulls message/detail/error out of the API's own error body instead of ApiError.__str__'s verbose "headers/status_code/body" dump. - langchain.py: raise ToolException(api_error_message(e)) with handle_tool_error=True instead of returning a ToolError object as the tool's normal return value -- LangChain's own documented error-signaling mechanism. - openai.py: a hand-built FunctionTool's failure_error_function only applies to the @function_tool decorator, not manually constructed instances, so on_invoke_tool now raises (with the clean extracted message) instead of returning str(e), letting the SDK surface it as a real failure. - livekit.py: already used the correct ToolError mechanism; now uses the shared clean-message extractor instead of the raw ApiError dump, and moves status_update_task.cancel() into a finally block so it's cancelled on the error path too (previously only cancelled on success, leaking the task and letting it call generate_reply() after a failed tool call). - openai.py/livekit.py: guard the None-returning case (e.g. delete_inbox) before calling .model_dump_json() -- previously crashed with an AttributeError that masked a successful delete as a tool failure. - openai.py/livekit.py: run tool.func via asyncio.to_thread instead of calling the synchronous AgentMail client directly on the event loop -- both adapters are async but the client is always the sync AgentMail client, so every call previously blocked the whole agent loop for the full HTTP round-trip (worst case: get_attachment's blocking download + PDF/DOCX parsing). This also lets livekit's status-update task actually get a chance to run before being cancelled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… AI SDK support - toolkit.py: type _tools as Optional[Dict[str, T]] instead of Dict[str, T] = None, matching its actual runtime behavior (harmless today since __init__ always overwrites it before use, but incorrect for static type checkers and readers). - README.md: stop claiming MCP and Vercel AI SDK support -- neither exists in this package (only openai.py, langchain.py, livekit.py); that sentence was copied from the Node README without being adapted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…entDisposition schemas.ts's AttachmentSchema used content_id (snake_case), but the SDK's outbound serializer expects contentId (camelCase) and silently strips unrecognized keys, so inline-attachment Content-ID references were dropped before ever reaching the API (broken cid: image references, no error). Also add the missing contentType/ contentDisposition fields the SDK's SendAttachment type supports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…se bodies
apiErrorMessage only handled {message|detail|error} bodies and never bounded the
text it returns to callers (only ce30e5d's log-line truncation was bounded, and
only for string bodies). Add a concise, bounded message path for the
ValidationErrorResponse shape ({name, errors}, no top-level message), and cap the
returned text at 500 chars regardless of body shape.
Also add `normalize()`, a small JSON-safety helper (Date -> ISO string, undefined
stripped) for checking tool results against the new Zod output schemas.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Tool contract
- Add src/output-schemas.ts: shared Zod output schemas (Inbox, Thread/ThreadItem,
Message/MessageItem, Draft/DraftItem, AttachmentResponse, pagination envelope,
send-result, Identity, void-result) derived from the installed agentmail SDK
(0.5.11) .d.ts response types. Dates are modeled as ISO-8601 strings, matching
the JSON-safe shape `normalize()` (util.ts) produces from the SDK's real Date
objects.
- Extend Tool (tools.ts) with `title` and `outputSchema`, and thread
`z.infer<typeof ParamsSchema>` through `func`'s args (via a `defineTool`
identity helper), replacing the `Args = Record<string, any>` / `Promise<any>`
erasure that let a schema field rename go uncaught at compile time. `func`'s
return stays untyped against outputSchema itself since outputSchema documents
the *normalized* contract while func returns the SDK's raw (Date-bearing)
result - normalizing is a downstream adapter concern.
- Compose an outputSchema for all 24 tools, and add the previously-missing
destructiveHint/idempotentHint/openWorldHint (all 5 annotation fields now
explicit on every tool) per the audit's annotation matrix. Preserves every
value the audit verified correct, including delete_thread's idempotentHint:
false (documented inline why it must not match delete_inbox/delete_draft).
- functions.ts: type each function's args from its schema instead of `Args`;
deleteInbox/deleteThread/deleteDraft now return a stable `{success: true}`
instead of the SDK's void, matching VoidResultSchema.
- mcp.ts: consume the canonical tool.title/tool.annotations instead of
re-deriving a title string that duplicated the new Tool.title field.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ailures
get_attachment previously fetched and buffered an attachment of any size with no
cap, and a bare `catch {}` made a blocked fetch, expired URL, or malformed
PDF/DOCX indistinguishable from "not extractable" (node-audit.md §9.2/§9.3).
- Skip extraction for non-https download URLs, attachments over 25MB (by
reported size, Content-Length, and actual downloaded size), with a 15s fetch
timeout.
- Cap extracted text at ~500k chars to bound a decompression-bomb-style DOCX or
pathological PDF, with a 20s extraction timeout (util.ts `withTimeout`; note
the losing extraction keeps running in the background, full cancellation
would need a worker thread).
- Log extraction failures instead of swallowing them silently.
- Add `truncateForLog`, bounding logged error bodies the same way whether
they're strings or parsed JSON objects (continuing ce30e5d, which only
bounded string bodies).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Preserve unrecognized fields the SDK adds in the future instead of silently stripping them from validated structured content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This is a server-side Node.js package; browser globals (window, document) were being treated as available while Node globals (process, Buffer) weren't pre-declared (node-audit.md §9.7). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tool()'s functional factory doesn't accept handle_tool_error as a kwarg on the installed langchain==1.1.0 (it's a BaseTool field, set post-construction) — AgentMailToolkit() raised TypeError building its very first tool. Found while writing adapter tests for the langchain error-signaling fix from the prior commit; set the field on the built tool instead of passing it to the factory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ecurity semantics Adds pytest (uv dev group) and 30 tests across three files, all against a mocked AgentMail client — no network calls: - test_functions.py: SDK call shape for every tool function, with explicit coverage of F1 (create_inbox builds CreateInboxRequest and passes it as request=) and F7 (get_attachment calls the inbox-scoped client.inboxes.threads.get_attachment, not the unscoped variant); a void-op (delete_inbox -> None) stability check; F10's no-mutation guarantee on reply_to_message/forward_message; and a real-SDK-model pass-through/serialization check. - test_adapters.py: per-framework error signaling (F5) and void-op handling (F2) for openai.py, langchain.py, and livekit.py — confirms a tool failure is always raised/flagged (RuntimeError, ToolException surfaced as an error ToolMessage, ToolError) and never returned as an ordinary success string, and that livekit's status-update task is cleaned up on the error path too (F8). - test_security.py: https-only enforcement, the 15s timeout, the size cap (both the size-metadata pre-flight and the bounded-read fallback), and malformed PDF/DOCX fixtures falling back gracefully with a logged warning instead of crashing or silently swallowing BaseException. Also: uv build verified (wheel + sdist, twine check passes, wheel contains py.typed + all modules, imports cleanly from a fresh venv); README documents each adapter's error type and get_attachment's download bounds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mcp.ts now registers each tool's outputSchema, normalizes results (Date -> ISO string) into structuredContent, and validates against the tool's Zod output schema before returning - schema drift now fails visibly as a diagnostic isError result instead of silently shipping malformed structured content. content still carries the same serialized JSON as structuredContent for backwards compatibility. Error results are untouched (isError:true + text, never validated against the success schema, matching the MCP SDK's own behavior of skipping output validation on error). util.ts: extracted `errorMessage()` from safeFunc's catch branches so it can be reused by adapters that signal errors by throwing rather than returning a result flag (next commit). mcp.ts's error-body log truncation now goes through the already-exported `truncateForLog` (previously only string bodies were truncated at the call site; object bodies like ValidationErrorResponse now get the same cap). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…vely All three previously swallowed tool failures into an ordinary-looking successful return value (ai-sdk.ts/langchain.ts via safeFunc's caught result; clawdbot.ts had no error handling at all - a thrown AgentMailError propagated raw and uncaught). Each now catches and rethrows a concise, bounded message (via the shared `errorMessage` helper) so the failure reaches the framework's own error mechanism: - ai-sdk.ts: throws, so the `ai` v6 SDK produces a native `tool-error` content part instead of a tool-result whose output happens to be an error string. Also passes `tool.outputSchema` through the AI SDK Tool's `outputSchema` field (supported per the installed ai@6.0.0-beta.150), and normalizes the returned value so it actually matches that declared schema. - langchain.ts: rethrows so DynamicStructuredTool.call() propagates the rejection instead of swallowing it. Note: the installed @langchain/core (1.1.x) has no `ToolException` class - that's a langchain-python construct, not JS - so this uses the JS-native mechanism (a rejected promise) instead. - clawdbot.ts: adds the try/catch this adapter never had. Confirmed via pi-agent-core's agent-loop.js that `executeToolCalls` catches a thrown exception from `execute()` and derives `isError: true` from it, using the exception's message as the tool result text - so a clean rethrow is exactly the framework-native contract here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
index.ts (the bare `agentmail-toolkit` import) was the one entry point that
never called safeFunc, so it got none of the fix/mcp-error-visibility
protections: a thrown AgentMailError propagated with the SDK's raw,
un-truncated, unfiltered "Status code / Body" dump straight to the caller.
func's return type changes from `Promise<any>` to a documented
`{isError, result, statusCode?, body?}` contract - the same shape every
other framework adapter already normalizes to - so this is an intentional,
documented public-contract change for consumers of the bare import, not a
behavior-preserving refactor.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ilure getAttachment previously wrapped the download fetch and the PDF/DOCX extraction in one try/catch, so a network error, an expired signed URL, and a malformed/adversarial document all looked identical: silently return the bare attachment metadata. Per security-audit.md's fix list: - Download failures (fetch throws, or a non-2xx response) now propagate uncaught out of getAttachment, so safeFunc/adapters surface them as a real tool error - the attachment genuinely could not be fetched. - Extraction failures (a corrupt/adversarial PDF or DOCX, or a bug in unpdf/jszip) are still caught and still degrade gracefully to the bare attachment, but now set an explicit `extractionError` field (added to AttachmentResponseSchema) instead of looking identical to "this just isn't a PDF/DOCX." The pre-flight https-only/size-cap skips also now set extractionError for the same reason. (The https-only scheme check, AbortSignal timeout, pre-flight/post-download size caps, and bounded/timeout-wrapped extraction this fix list also asked for were already implemented in a prior stage's commits - see node-1-contracts.md; this commit is the remaining piece.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cription hygiene - generic toolkit exposes outputSchema and throws concise Errors (no isError envelope) - external-content tools note in descriptions that email content is not instructions - pi-agent-core declared as optional peer; clawdbot peer pinned to a real range - eslint ignores dist/, allows _-prefixed unused args; remaining src 'any's removed - ai aligned to stable 6.0.224 (same major, within existing peer range) fixing the examples provider-type conflict; examples @ai-sdk/openai moved to the ai-v6 line Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- contract: all 24 tools have root-object output schemas, five explicit annotations, SDK-shaped fixtures validating (and corrupted fixtures failing) - MCP: official SDK client/server over InMemoryTransport; tools/list metadata, all 24 tools/call with structuredContent === text parity, isError paths, visible output-schema-drift failure - adapters: ai-sdk/langchain/clawdbot/generic error semantics with mocked client - security: attachment https/size/timeout bounds, malformed-file extraction fallback, bounded error messages and logs, normalize Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ict-root schema, refuse attachment redirects The MCP SDK rebuilds a strip-mode z.object from the registered raw shape and advertises additionalProperties:false at the root (verified via live round trip), so parse results with the same strip semantics: unknown top-level SDK fields are dropped from structuredContent instead of leaking into a payload strict clients would reject. Nested objects stay loose. Adds regression tests through the real SDK reconstruction path. Also: fetch redirect:'error' so a redirect cannot downgrade the https-only attachment check (H1/M1 from adversarial review). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p to 0.3.0 - api_error_message caps returned text at 500 chars, mirroring node's MAX_ERROR_BODY_LENGTH (an unbounded validation-errors body was raised verbatim to every adapter's caller) - attachment downloads use a no-redirect opener so a redirect cannot downgrade the https-only check - version 0.2.7 -> 0.3.0: adapters now raise on failure instead of returning error values, matching the node 0.5.0 bump (H3/M1/M2 from adversarial review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ZE_LIMIT Replace the invented 25MB download / 500K-char extraction caps with the AgentMail API's own enforced content ceiling (RESPONSE_SIZE_LIMIT = 5.95MB in agentmail-api/src/agentmail/utils/limits.ts). The API inlines extracted message content only up to that size and otherwise returns a URL (get-message.ts); the toolkit inlines extracted attachment text into a tool result, the same payload class, so it uses the same ceiling rather than an arbitrary one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…IZE_LIMIT Replace the invented 25MB cap with the AgentMail API's own enforced content ceiling (RESPONSE_SIZE_LIMIT = 5.95MB, agentmail-api utils/limits.ts) - the size above which the API stops inlining content and returns a URL instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hoices, not API-derived Unlike the size caps (which mirror the API's RESPONSE_SIZE_LIMIT), the fetch and extraction timeouts have no enforced upstream constant to derive from - only this surface can bound how long it waits. Documenting the distinction so the value reads as judgment, not an unsourced arbitrary number. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
duharry0915
approved these changes
Jul 13, 2026
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
2 participants
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.
What
Implementation-grade modernization of both toolkit packages against the current stable MCP spec (2025-11-25), built on top of
fix/mcp-error-visibility(included).Node (0.4.1 → 0.5.0)
outputSchemaand returns validatedstructuredContent+ a matching JSON text block — eliminates OpenAI app-submission "missing output schema" warnings. Shared Zod schemas derived from the installed SDK's real runtime shapes (camelCase, Dates → ISO 8601); schema drift fails visibly asisError, never silently.destructiveHintdefault oftrueon read-only tools was the OpenAI directory rejection cause).delete_threaddeliberately staysidempotentHint:false(second call permanently purges).content_id→contentId(was silently stripped by the SDK serializer — brokencid:inline images); added missingcontentType/contentDisposition.catch{}replaced with explicitextractionErrormetadata.Python (0.2.7 → 0.3.0)
create_inbox(hard TypeError vs SDK 0.4.10), void deletes crashing adapters,get_attachmentignoring its requiredinbox_id, langchain adapter crashing on construction, Pydantic v2 optional fields.ToolException, openai-agents raise; livekit was already correct); bounded error messages;urlopenhardening (https-only, no redirects, timeout, size cap).Note for reviewers
The generic toolkit's error contract changed twice within this branch (
11278d5envelope →5ac2f8fthrow); the final throw-based contract is the shipped one.An adversarial review pass (0 critical findings) is incorporated; the notable fix is
b3c9f05— structuredContent is strip-parsed to conform to the strict-root schema the MCP SDK actually advertises.Downstream:
agentmail-manufact-mcpneeds only a dependency bump to consume this (its pinned MCP SDK 1.24.1 already passes outputSchema through) — companion PR in that repo.🤖 Generated with Claude Code
https://claude.ai/code/session_01Xm9FxownpfZTJTMRdNbaTr