-
Notifications
You must be signed in to change notification settings - Fork 3
fix(tools): add serde aliases to Glob handler for parameter compatibility #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Greptile OverviewGreptile SummaryThis PR adds serde aliases to tool handlers for better client compatibility and includes several code quality improvements. Main Changes:
Note: The PR title mentions only the Glob handler fix, but this PR contains 7 commits spanning multiple improvements (tool compatibility, timeout refactoring, dead code cleanup, auth fixes, and documentation). While all changes appear sound individually, bundling unrelated changes makes it harder to track which commit addresses which issue and complicates potential rollbacks. Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| src/cortex-engine/src/tools/handlers/glob.rs | Added serde aliases for exclude_patterns (alias: excludePatterns) and directory (alias: folder) to support both snake_case and camelCase parameter names |
| src/cortex-engine/src/tools/handlers/grep.rs | Added serde alias head_limit for max_results field to support alternative parameter naming |
| src/cortex-app-server/src/auth.rs | Replaced .unwrap() with .unwrap_or_default() in three SystemTime operations to handle potential time errors gracefully |
| src/cortex-tui/src/runner/event_loop/streaming.rs | Extracted magic timeout numbers into named constants STREAMING_CONNECTION_TIMEOUT (60s) and STREAMING_CHUNK_TIMEOUT (30s) |
| src/cortex-common/src/http_client.rs | Added comprehensive timeout configuration documentation table explaining the timeout hierarchy across all Cortex services |
Sequence Diagram
sequenceDiagram
participant Client
participant ToolHandler
participant GlobHandler
participant GrepHandler
participant FileSystem
Note over Client,FileSystem: Tool Parameter Compatibility Enhancement
Client->>ToolHandler: Request with camelCase params<br/>(excludePatterns, folder, head_limit)
alt Glob Tool
ToolHandler->>GlobHandler: deserialize with serde aliases
Note over GlobHandler: alias "excludePatterns" → exclude_patterns<br/>alias "folder" → directory
GlobHandler->>GlobHandler: Parse parameters (snake_case or camelCase)
GlobHandler->>FileSystem: Execute glob search
FileSystem-->>GlobHandler: Return matching files
GlobHandler-->>ToolHandler: ToolResult
else Grep Tool
ToolHandler->>GrepHandler: deserialize with serde aliases
Note over GrepHandler: alias "head_limit" → max_results
GrepHandler->>GrepHandler: Parse parameters (snake_case or camelCase)
GrepHandler->>FileSystem: Execute grep search
FileSystem-->>GrepHandler: Return matching content
GrepHandler-->>ToolHandler: ToolResult
end
ToolHandler-->>Client: Success response
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 files reviewed, no comments
|
Changes already included in #75 - fix(tools): consolidated tools parameter aliases and handler name fixes |
Summary
Adds serde aliases to the Glob tool handler to support both snake_case and camelCase parameter names for better client compatibility.
Problem
The Glob tool only accepts snake_case parameter names (exclude_patterns, directory), but some clients may send camelCase variants. This creates inconsistency with other tools that support both conventions.
Changes
#[serde(alias = "excludePatterns")]to exclude_patterns field#[serde(alias = "folder")]to directory field for consistency with tool definitionTesting
Verification