refactor(module)/codemode improvements combined#212
Draft
Mat4m0 wants to merge 15 commits intonuxt-modules:mainfrom
Draft
refactor(module)/codemode improvements combined#212Mat4m0 wants to merge 15 commits intonuxt-modules:mainfrom
Mat4m0 wants to merge 15 commits intonuxt-modules:mainfrom
Conversation
Tools with outputSchema now emit typed Promise return values in code mode type definitions instead of Promise<unknown>. Small schemas (<=3 primitive fields) are inlined; larger schemas get named interfaces.
…pe generation Added a new function to format TypeScript property keys, ensuring non-safe identifiers are quoted. Introduced a utility to generate schema type information, streamlining the handling of input and output schemas in tools. This improves type safety and clarity in generated type definitions.
…nd concurrency safety - Fix AsyncLocalStorage context loss through singleton RPC server by using per-execution snapshots via AsyncLocalStorage.snapshot() - Fix concurrent execute() calls overwriting shared fns/onReturn by introducing per-execution ExecutionContext keyed by execId - Add bounded RPC body reader (1MB default, HTTP 413) - Add wall-clock execution timeout (60s default, HTTP 408) - Add per-execution RPC call quota (200 default, HTTP 429) - Add per-tool response size limit (1MB default, truncation) - Sanitize error messages to strip file paths and stack traces - Add server-side logging to all catch blocks - Fix exec.returned set before callback completes - Replace void handleRpcRequest with .catch() safety net
When a tool handler returns structuredContent (the MCP spec's typed data channel), buildDispatchFunctions() ignored it and fell through to extracting text from content[].text — silently losing IDs, booleans, and nested objects. This broke operation chaining where a returned ID is needed for follow-up calls.
…edContent handling Updated the codemode tests to utilize a mockMcpExtra function, ensuring that structuredContent is correctly processed in tool handlers. Changed tool definitions from McpToolDefinition to McpToolDefinitionListItem for better type alignment.
Tool errors (isError: true) were returned as plain strings, making them indistinguishable from successful results in sandbox code. try/catch never fires, and structured error details from structuredContent are lost. Return a __toolError sentinel from dispatch so the sandbox proxy can throw a structured Error with .tool, .isToolError, and .details fields.
…example handling - Updated description templates for code execution to be more concise. - Added conditional logic to include or omit example blocks based on the number of available tools. - Improved the handling of description formatting to collapse excessive newlines. - Adjusted tests to verify the new description formats and example inclusion logic.
- Introduced a durationMs field in the execution result to track execution time. - Updated ExecuteResult type to include durationMs for both success and error cases. - Improved error handling in tool dispatch to ensure structured error messages are returned. - Refactored code to maintain clarity and consistency in execution context management.
- Changed error prefix from '__ERROR__' to '__MCP_EXEC_ERR__' for clarity. - Enhanced error handling in RPC session creation and cleanup processes with console warnings for better debugging. - Updated tool error structure to use '__mcp_toolkit_error__' instead of '__toolError__'. - Refactored ExecuteResult and CodeToolEnvelope types for consistency in error representation. - Improved tests to validate new error handling and ensure proper functionality.
- Added support for surfacing MCP tool annotations (`readOnlyHint`, `destructiveHint`, `idempotentHint`) as tags in code mode type signatures and search results.
- Updated the description template to include an `{{example}}` placeholder for usage examples.
- Implemented grouping of search results by tool `group` field, improving organization in output.
- Refactored related functions to accommodate new features and ensure backward compatibility.
- Expanded tests to validate annotation surfacing and grouped search results functionality.
…l name collisions - Added a fallback mechanism for AsyncLocalStorage.snapshot to handle Node.js versions <18.16.0, logging a warning when the snapshot is unavailable. - Updated tool name collision handling to issue a warning instead of throwing an error, preserving the last tool in case of a name conflict. - Adjusted tests to validate the new fallback behavior and ensure proper logging for name collisions.
Contributor
|
@Mat4m0 is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
commit: |
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.
Code Mode Integration — Combined Changeset
This PR combines the earlier PRs (#210 #206 #207 #208 )into one branch, with a few additional improvements. The goal is to make review easier and give us a single discussion baseline; it is not intended to be merge-ready yet.
This document describes every change in this branch, why it was made, and how it was validated.
Table of Contents
1. Per-Session RPC Architecture
Problem: The original executor used a singleton RPC server and a singleton V8 runtime. All concurrent
execute()calls shared the samefnsmap,onReturncallback, and RPC token. This meant:onReturncallback was a single slot — racing executions would lose return valuesSolution: Each
execute()call now creates its own isolated RPC session:createServer()on port 0 (OS-assigned ephemeral port)execId(8 random bytes) andtoken(32 random bytes) per executionExecutionContextstruct holds per-execution state:fns,onReturn,deadlineMs,rpcCallCountfnsmap is frozen withObject.freeze()so sandbox code can't mutate itexecIdin every RPC call; the server validates it matches the sessionActiveSessiontracking with cleanup on completion, error, or timeoutFiles:
executor.ts(complete rewrite of RPC layer)2. AsyncLocalStorage Context Preservation
Problem: Nitro uses
AsyncLocalStorageto carry per-request context (H3 event, auth, etc.). When Code Mode dispatches a tool call via HTTP RPC, the async context is lost — the RPC handler runs in the HTTP server's context, not the original request's.Solution: On entry to
execute(), we capture the current async context viaAsyncLocalStorage.snapshot(). Every tool dispatch is wrapped throughrestoreContext(), which re-enters the original request's async context before calling the tool handler. This means tools called from sandbox code have access to the sameuseEvent(), auth state, etc. as if called directly.When
snapshot()is unavailable (Node.js < 18.16.0), execution degrades gracefully: a passthrough function is used instead, and a one-time warning is logged viaconsole.warn. Tools still work but lose access to request context (useEvent(), auth, etc.) — matching the behavior of main before this branch. This preserves backward compatibility with Node.js 18.0+.Files:
executor.ts3. Resource Limits & Abuse Prevention
Problem: The original executor had no protection against:
Solution: Added configurable limits with sensible defaults:
maxRequestBodyBytesmaxToolCallsExecutionContextwallTimeLimitMssetTimeout→runtime.dispose()maxToolResponseSizemaxResultSize)The wall-clock timeout is separate from the V8 CPU time limit. CPU time only counts isolate execution; wall time caps total elapsed time including host-side tool calls. On timeout, the runtime is forcefully disposed and the sandbox gets a clear error.
Error messages returned to sandbox/client are sanitized: file paths are replaced with
[path], stack traces are stripped, and messages are truncated to 500 chars. Full errors are logged server-side withconsole.error('[nuxt-mcp-toolkit] ...').Files:
executor.ts,types.ts(new options),8.code-mode.md(docs)4. structuredContent Dispatch
Problem: When a tool handler returned
structuredContent(the MCP spec's typed data channel), Code Mode ignored it and fell through to extracting text fromcontent[].text. This silently lost IDs, booleans, nested objects — breaking operation chaining where a returned ID is needed for follow-up calls.Solution:
normalizeDispatchResult()now checksstructuredContentfirst:rawResult.structuredContent != null→ return it directly (preserves typed data)rawResult.isError→ convert toCodeModeToolErrorsentinel (see feat: adddefineMcpHandlerutils #5)rawResult.contenthas text items → return joined text (no JSON.parse — intentional, avoids ambiguity)The function also uses
isCallToolResult()to distinguish MCP results from plain objects returned by handlers. This duck-type check was tightened:isErroralone no longer matches unless it's a boolean (prevents false positives from objects that happen to have anisErrorproperty).Files:
index.ts(normalizeDispatchResult, extractTextContent),results.ts(isCallToolResult)5. Tool Error Surfacing in Sandbox
Problem: Tool errors (
isError: trueresults or thrown exceptions) were returned as plain strings to sandbox code, making them indistinguishable from successful results.try/catchnever fired, and structured error details fromstructuredContentwere lost.Solution: Tool errors are now wrapped as a sentinel object with a namespaced key:
The sandbox proxy code detects this sentinel and throws a structured
Errorwith.tool,.isToolError, and.detailsproperties. This lets sandbox code usetry/catchto handle tool errors with full context.The sentinel key is namespaced (
__mcp_toolkit_error__) rather than generic (__toolError) to avoid collisions with legitimate tool return values. Similarly, the stderr error prefix is__MCP_EXEC_ERR__rather than__ERROR__.Files:
index.ts(toToolError, CodeModeToolError),executor.ts(proxy boilerplate)6. Typed Output Schemas
Problem: All Code Mode tools returned
Promise<unknown>regardless of whether anoutputSchemawas defined. The LLM had no type information about what to expect back.Solution: When a tool definition includes
outputSchema, the type generator now emits typed return values:Promise<{ id: string; ok: boolean }>Promise<GetReportOutput>This reuses the same
generateSchemaTypeInfo()helper used for input schemas, extracted from the duplicated inline logic.Files:
types.ts(generateSchemaTypeInfo, generateToolTypeInfo output handling)7. Type Generation Improvements
Several improvements to the TypeScript type generation for sandbox code:
JSON.stringify()viaformatTsPropertyKey(). Before:my-key?: string(invalid TS). After:"my-key"?: string.JSON.stringify(v)instead of template literals. Before:"he said "hello""(broken). After:"he said \"hello\"".buildToolNameMap()now warns viaconsole.warnif two tools sanitize to the same name (e.g.,get-userandget_userboth becomeget_user), keeping last-wins behavior for backward compatibility. The warning message includes a notice that this will become an error in a future version.Files:
types.ts8. Description Templates & Progressive Mode
Problem: The code tool descriptions were overly verbose with multi-paragraph instructions about combining sequential/parallel/conditional logic. This wastes context window for every tool call.
Solution:
{{example}}placeholder support alongside existing{{types}}and{{count}}Files:
index.ts(templates, applyDescriptionTemplate),types.ts(CodeModeOptions.description docs)9. Structured Envelope Responses
Problem: The code tool returned only text content — no structured data for programmatic consumption by MCP clients.
Solution: The code tool now returns both
structuredContentandcontent:CodeToolEnvelopeis a discriminated union (ok: true | false) preventing impossible states (simultaneous result + error)ExecuteResultis also a discriminated union —resultanderrorare mutually exclusive at the type leveldurationMstracks wall-clock execution timeoutputSchemais declared on the code tool definition so MCP clients can validate responsesFiles:
index.ts(CodeToolEnvelope, createCodeToolEnvelope, formatSuccessContent),types.ts(ExecuteResult)10. Handler Reuse Between Registration and Code Mode
Problem: Code Mode reimplemented tool handler invocation: it manually resolved tool names, manually checked for
inputSchema, and called handlers differently thanregisterToolFromDefinition(). This meant cache wrappers, error normalization, and future middleware would only apply to registered tools, not code mode dispatches.Solution: Extracted shared utilities from
registerToolFromDefinition():resolveToolDefinitionName(tool)— appliesenrichNameTitleto get the canonical namecreateWrappedToolHandler(tool)— applies cache wrappers and returns the handlerinvokeWrappedToolHandler(tool, handler, input, extra)— calls the handler with the correct argument shape. Usestool.inputSchema !== undefined(notObject.keys(...).length > 0) to determine the call shape, correctly handlinginputSchema: {}which is a valid empty schema where the handler still receives(args, extra)Code Mode now uses these same functions, so cache, auth, and any future middleware apply consistently whether a tool is called via MCP protocol or via sandbox code.
The
buildDispatchFunctions()helper is now exported (used by tests) and acceptsMcpRequestExtra, which gets passed through to tool handlers so they have access torequestId,signal,sendNotification, etc.Files:
tools.ts(resolveToolDefinitionName, createWrappedToolHandler, invokeWrappedToolHandler),index.ts(buildDispatchEntries, buildDispatchFunctionsFromEntries)11. Hardening Pass (PR Review Fixes)
After the feature work, a comprehensive PR review identified issues that were fixed:
Error sentinel spoofability
Changed sentinel key from
__toolErrorto__mcp_toolkit_error__and stderr prefix from__ERROR__to__MCP_EXEC_ERR__to avoid collisions with legitimate tool return values or user code logging.Empty catch blocks
Four truly empty
catch {}blocks in cleanup paths (server.close, runtime.dispose, wall-timer dispose) now log warnings withconsole.warn('[nuxt-mcp-toolkit] ...')and context about which operation failed.Unhandled promise rejection
void handleRpcRequest(...)had no catch handler. If the inner try/catch failed ANDsendJsonalso threw, the rejection escaped. Added.catch(() => { if (!res.headersSent) res.destroy() })as a safety net.Missing server-side error logging
The outer
catchinexecute()sanitized errors before returning them but never logged the raw error. Addedconsole.error('[nuxt-mcp-toolkit] Execution error:', error)before sanitization so infrastructure errors are visible in server logs.isCallToolResult false positives
isErroralone was enough to matchisCallToolResult(). Now requirestypeof isError === 'boolean'— objects with incidental string/numberisErrorproperties aren't misrouted.Dead proxy cache
cachedProxyKey/cachedProxyCodemodule-level variables never hit because each execution creates a new port+token. Removed entirely.Impossible type states
ExecuteResultwas{ result: unknown, error?: string }— both could be set. Now a discriminated union whereresultanderrorare mutually exclusive. Same forCodeToolEnvelopewithokas discriminant.disposeCodeMode error swallowing
.catch(() => {})replaced with.catch(error => console.warn(...)).Empty-schema dispatch fix
invokeWrappedToolHandlerwas checkingObject.keys(tool.inputSchema).length > 0which treatedinputSchema: {}as "no schema", callinghandler(extra)instead ofhandler({}, extra). Fixed to checktool.inputSchema !== undefined. This was a pre-existing bug on main that was surfaced during the handler extraction refactor.Files:
executor.ts,index.ts,types.ts,results.ts,tools.ts12. Annotation Surfacing & Grouped Progressive Search
Problem: MCP annotations (
readOnlyHint,destructiveHint,idempotentHint) were defined on tool definitions but invisible in code mode. Progressive mode search results were a flat list with no grouping.Solution: Derive more from existing metadata, configure nothing new.
12a. Annotation surfacing
buildAnnotationTags()converts existing MCP annotations into compact tags prepended to the// commentin type signatures:readOnlyHint: true→[read-only]destructiveHint: true→[destructive]idempotentHint: true→[idempotent]Tags use text (not emoji) for token efficiency. Both annotation tags and description comments are always preserved in the type block — the
// descriptioncomment is the primary way the LLM understands tool semantics in standard mode. When annotations are present, tags are prepended to the description (e.g.,// [read-only] List all items). Tools without annotations retain their plain// descriptioncomment.12b. Grouped progressive search
formatSearchResults()groups results by the existinggroupfield (auto-inferred from directory structure, e.g.,server/mcp/tools/workspace/→'workspace'):tool.grouportool._meta.group13. Test Coverage
codemode.test.ts (~530 lines)
createCodemodeTools: standard mode, progressive mode, example block omission, outputSchemabuildDispatchFunctions: structuredContent preference, plain text preservation, native object passthrough, thrown error → sentinel conversion, MCP extra propagation, empty-schema extra passing, undefined-schema extra passingnormalizeCode: markdown fence stripping, arrow function unwrapping, export default removalisCallToolResult: content arrays, structuredContent, boolean isError, non-boolean isError rejection, plain object rejectionisErrorCallToolResult → tool error sentinel flow (including structuredContent details)[read-only],[destructive],[idempotent]tags; combined tagscodemode-executor.test.ts (new, ~450 lines)
14. Documentation Updates
8.code-mode.md: Added new config options (maxRequestBodyBytes,maxToolResponseSize,wallTimeLimitMs,maxToolCalls) to the configuration reference and resource limits table. Added "Error Sanitization" section. Added Node.js >=18.16.0 callout. Added "Annotation Surfacing" section. Updated progressive mode section with grouped search output example.handlers.ts: Expandedexperimental_codeModeJSDoc with config keys and Node.js requirement.2.installation.md: Minor addition (context for secure-exec requirement).Files Changed Summary
executor.tsindex.tstypes.tsbuildAnnotationTags, groupedformatSearchResultstools.tsresolveToolDefinitionName,createWrappedToolHandler,invokeWrappedToolHandlerresults.tsisCallToolResult, tightenedisErrorcheckhandlers.tsexperimental_codeModeexecutor.cloudflare.tscodemode.test.tscodemode-executor.test.ts8.code-mode.md2.installation.md5.handlers.mdHow Everything Connects
Key design principle: Derive from existing MCP metadata (
annotations,group,outputSchema,description), don't add new per-tool config surface. Every feature is additive and optional — tools without annotations or groups produce identical output to before.