Skip to content

Conversation

@echobt
Copy link
Contributor

@echobt echobt commented Feb 4, 2026

Summary

This PR consolidates 3 tools fixes into a single, cohesive change.

Included PRs:

Key Changes:

  • Added serde aliases for backward compatibility with different parameter naming conventions
  • Fixed mismatched handler names to match tool registration names
  • Improved overall tool parameter handling consistency

Files Modified:

  • src/cortex-engine/src/tools/handlers/glob.rs
  • src/cortex-engine/src/tools/handlers/file_ops.rs
  • src/cortex-engine/src/tools/handlers/grep.rs

Closes #53, #62, #63

This PR consolidates the following tools fixes:
- #53: Add serde alias for Glob tool folder parameter
- #62: Align tool handler names with registration names
- #63: Add serde alias for Grep tool head_limit parameter

Key changes:
- Added serde aliases for backward compatibility
- Fixed mismatched handler names to match tool registration
- Improved tool parameter handling consistency
@greptile-apps
Copy link

greptile-apps bot commented Feb 4, 2026

Greptile Overview

Greptile Summary

This PR consolidates three important fixes to the tool system, improving consistency and backward compatibility.

Key Changes:

  • Handler Name Fixes: Corrected WriteFileHandler to return "Create" and SearchFilesHandler to return "SearchFiles" to match their registration names in ToolRouter. This fixes potential tool lookup failures.
  • Parameter Aliases: Added serde aliases for backward compatibility:
    • GlobHandler: Added #[serde(alias = "folder")] for the directory parameter
    • GrepHandler: Added #[serde(alias = "head_limit")] for the max_results parameter
  • Code Cleanup: Removed unused code (reconnect methods, dead fields, unused imports) and enhanced documentation for timeout constants.
  • Safety Improvements: Changed unwrap() to unwrap_or_default() for UNIX_EPOCH duration calculations in auth.rs.

Additional Changes:
The PR includes broader cleanup work:

  • Comprehensive timeout documentation in cortex_common::http_client with a reference table
  • Removal of unused base_dir field from SessionStorage
  • Removal of unused replace_all field from SearchReplace
  • Dead code cleanup in transport layer (unused reconnect methods)

All changes are safe, well-tested patterns (serde aliases, handler name corrections), and improve code quality without breaking existing functionality.

Confidence Score: 5/5

  • This PR is safe to merge - all changes are non-breaking improvements and fixes
  • The changes are straightforward bug fixes and backward-compatible improvements: handler names now match registrations (preventing lookup failures), serde aliases maintain backward compatibility, and code cleanup removes dead code. No breaking changes or risky logic modifications.
  • No files require special attention - all changes follow established patterns

Important Files Changed

Filename Overview
src/cortex-engine/src/tools/handlers/file_ops.rs Fixed handler names to match registration: WriteFileHandler returns "Create", SearchFilesHandler returns "SearchFiles"
src/cortex-engine/src/tools/handlers/glob.rs Added serde alias "folder" for backward compatibility with directory parameter
src/cortex-engine/src/tools/handlers/grep.rs Added serde alias "head_limit" for backward compatibility with max_results parameter

Sequence Diagram

sequenceDiagram
    participant Client as Client/LLM
    participant Router as ToolRouter
    participant Handler as Tool Handler
    participant Serde as Serde Deserializer

    Note over Client,Serde: Tool Parameter Backward Compatibility Flow

    Client->>Router: execute("Glob", {"folder": "/path"})
    Note over Client: Uses legacy "folder" parameter
    Router->>Handler: GlobHandler.execute(args)
    Handler->>Serde: Deserialize GlobArgs
    Note over Serde: #[serde(alias = "folder")]<br/>maps to "directory"
    Serde-->>Handler: GlobArgs { directory: Some("/path") }
    Handler->>Handler: Process with directory field
    Handler-->>Router: ToolResult
    Router-->>Client: Success

    Client->>Router: execute("Grep", {"head_limit": 10})
    Note over Client: Uses legacy "head_limit" parameter
    Router->>Handler: GrepHandler.execute(args)
    Handler->>Serde: Deserialize GrepArgs
    Note over Serde: #[serde(alias = "head_limit")]<br/>maps to "max_results"
    Serde-->>Handler: GrepArgs { max_results: Some(10) }
    Handler->>Handler: Apply max_results limit
    Handler-->>Router: ToolResult
    Router-->>Client: Success

    Client->>Router: execute("Create", args)
    Note over Client: Uses registered name "Create"
    Router->>Handler: WriteFileHandler.execute(args)
    Note over Handler: name() returns "Create"<br/>(matches registration)
    Handler->>Handler: Write file
    Handler-->>Router: ToolResult
    Router-->>Client: Success
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@echobt
Copy link
Contributor Author

echobt commented Feb 4, 2026

Closing this PR to consolidate into a single mega-PR combining all bug fixes. The changes will be included in a new consolidated PR.

@echobt echobt closed this Feb 4, 2026
echobt added a commit that referenced this pull request Feb 4, 2026
This PR consolidates all bug fixes and security improvements from PRs #69-88 into a single cohesive change.

## Categories

### Security Fixes
- Path traversal prevention in MCP and session storage
- Shell injection prevention in restore scripts
- Secure random temp files for external editor
- TOCTOU race condition fixes

### TUI Improvements
- Overflow prevention for u16 conversions
- Cursor positioning fixes in selection lists
- Unicode width handling for popups
- Empty section handling in help browser

### Error Handling
- Graceful semaphore and init failure handling
- Improved error propagation in middleware
- Better client access error handling
- SystemTime operation safety

### Memory and Storage
- Cache size limits to prevent unbounded growth
- File lock cleanup for memory leak prevention
- fsync after critical writes for durability
- Bounded ToolResponseStore with automatic cleanup

### Protocol Robustness
- Buffer size limits for StreamProcessor
- ToolState transition validation
- State machine documentation

### Numeric Safety
- Saturating operations to prevent overflow/underflow
- Safe UTF-8 string slicing throughout codebase

### Tools
- Parameter alias support for backward compatibility
- Handler name consistency fixes

## Files Modified
Multiple files across cortex-tui, cortex-engine, cortex-exec, cortex-common,
cortex-protocol, cortex-storage, cortex-mcp-server, and other crates.

Closes #69, #70, #71, #73, #75, #80, #82, #87, #88
echobt added a commit that referenced this pull request Feb 4, 2026
This PR consolidates all bug fixes and security improvements from PRs #69-88 into a single cohesive change.

## Categories

### Security Fixes
- Path traversal prevention in MCP and session storage
- Shell injection prevention in restore scripts
- Secure random temp files for external editor
- TOCTOU race condition fixes

### TUI Improvements
- Overflow prevention for u16 conversions
- Cursor positioning fixes in selection lists
- Unicode width handling for popups
- Empty section handling in help browser

### Error Handling
- Graceful semaphore and init failure handling
- Improved error propagation in middleware
- Better client access error handling
- SystemTime operation safety

### Memory and Storage
- Cache size limits to prevent unbounded growth
- File lock cleanup for memory leak prevention
- fsync after critical writes for durability
- Bounded ToolResponseStore with automatic cleanup

### Protocol Robustness
- Buffer size limits for StreamProcessor
- ToolState transition validation
- State machine documentation

### Numeric Safety
- Saturating operations to prevent overflow/underflow
- Safe UTF-8 string slicing throughout codebase

### Tools
- Parameter alias support for backward compatibility
- Handler name consistency fixes

## Files Modified
Multiple files across cortex-tui, cortex-engine, cortex-exec, cortex-common,
cortex-protocol, cortex-storage, cortex-mcp-server, and other crates.

Closes #69, #70, #71, #73, #75, #80, #82, #87, #88
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant