Review and merge pull request 112 #114
Merged
Conversation
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
- Extract callGradioTool utility for SSE connection, MCP call, and progress relay - Extract applyResultPostProcessing for image filtering and OpenAI transforms - Refactor proxied gr_* tools to use shared utilities - Move stripImageContentFromResult and extractUrlFromContent to gradio-result-processor.ts - Ensure both proxied tools and space tool can use identical logic
- Import space tool files from PR #112 - Modify invoke.ts to return raw MCP result instead of formatted text - Add InvokeResult type to support both raw content and warnings - Export space tool from packages/mcp/src/index.ts - Add SPACE_TOOL_CONFIG and SPACE_TOOL_ID to tool-ids.ts - Space tool invoke operation now returns same content blocks as proxied gr_* tools - This ensures consistent behavior across both tool invocation paths
- Add SPACE_TOOL_CONFIG, SpaceTool, SpaceArgs, and InvokeResult imports - Import applyResultPostProcessing from gradio-tool-caller - Register space tool in mcp-server.ts with proper InvokeResult handling - Apply unified post-processing (image filtering + OpenAI transforms) - Handle warnings from invoke operation - Return raw MCP content blocks for invoke, formatted text for view_parameters - Respect NO_GRADIO_IMAGE_CONTENT flag for image filtering - Space tool now behaves identically to proxied gr_* tools
- Remove unused RequestOptions import - Remove unused stripImageContentFromResult and extractUrlFromContent imports - Remove unused createLazyConnection function (replaced by callGradioTool) - Fixes ESLint errors
…://github.com/huggingface/hf-mcp-server into claude/pr-112-review-011CUzcMKz4PvkUz7XBnErAJ
- Make InvokeResult.result.isError optional to match CallToolResultSchema - Add type cast for invokeResult.result when calling applyResultPostProcessing - Add type cast for ToolResult in view_parameters path - Remove unused SSEClientTransport imports from gradio-endpoint-connector - Import CallToolResultSchema in mcp-server for type assertions This fixes type compatibility issues between MCP schema types and the InvokeResult interface, allowing both invoke and view_parameters operations to work correctly with proper type safety.
- Import ToolResult type from @llmindset/hf-mcp - Replace inline import() type annotation with imported type - Fix @typescript-eslint/consistent-type-imports violation The linter also updated CallToolResultSchema to use type import for better tree-shaking.
1. Switch space tool invoke operation to use logGradioEvent:
- Track timing metrics (durationMs) like proxied gr_* tools
- Log success/error status and response size
- Add isDynamic: true flag to distinguish dynamic invocations from proxied tools
- Keep logSearchQuery for view_parameters operation
2. Disable invoke operation when gradio=none:
- Check for gradio=none header before allowing invoke operation
- Return helpful error message if invoke is attempted with gradio=none
- Keep view_parameters operation available regardless of gradio setting
3. Add isDynamic parameter to logGradioEvent:
- New boolean column in Gradio dataset to differentiate:
* isDynamic: true = space tool invoke (dynamic invocation)
* isDynamic: false = proxied gr_* tool (static endpoint)
- Defaults to false for backward compatibility
This provides consistent logging across all Gradio interactions while
clearly distinguishing between proxied tools and dynamic space tool
invocations in the dataset.
When a 404 error occurs (space not found), add a helpful note that: - The space MUST be an MCP enabled space - Users should use the space_search tool to find MCP enabled spaces This provides better guidance when users try to view parameters for spaces that don't exist or aren't MCP enabled. Example error message: Error fetching parameters for space 'silveroxides/Chroma-Extra': HTTP 404: Not Found Note: The space MUST be an MCP enabled space. Use the `space_search` tool to find MCP enabled spaces.
The space tool (invoke and view-parameters) imports from @modelcontextprotocol/sdk but the package didn't declare it as a dependency. This caused CI/Docker builds to fail with: error TS2307: Cannot find module '@modelcontextprotocol/sdk/types.js' While it worked locally due to pnpm workspace hoisting, proper dependency declaration is required for isolated package builds. Added @modelcontextprotocol/sdk ^1.20.0 to match the version used in packages/app.
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.
No description provided.