-
Notifications
You must be signed in to change notification settings - Fork 1
Closed
Labels
priority:highHigh priority issueHigh priority issuestatus:doneCompletedCompletedtype:enhancementNew feature or requestNew feature or requesttype:refactorCode refactoringCode refactoring
Description
Problem Description
The bssh codebase currently has 19 files that exceed 500 lines, with the largest file (ssh/client.rs) containing 1,394 lines. This violates the single responsibility principle and makes the codebase difficult to maintain, test, and understand. Large modules create cognitive overhead for developers and increase the likelihood of merge conflicts.
Oversized Files (>500 lines)
🔴 Critical Priority (>1000 lines)
src/ssh/client.rs- 1,394 lines⚠️ CRITICALsrc/commands/interactive.rs- 1,383 lines⚠️ CRITICALsrc/jump/chain.rs- 1,113 lines⚠️ CRITICALsrc/ssh/tokio_client/client.rs- 1,079 lines⚠️ CRITICAL
🟠 High Priority (800-1000 lines)
src/main.rs- 976 lines⚠️ Highsrc/config.rs- 926 lines⚠️ Highsrc/forwarding/dynamic.rs- 830 lines⚠️ Highsrc/executor.rs- 823 lines⚠️ High
🟡 Medium Priority (600-800 lines)
src/ssh/config_cache.rs- 757 lines 🔸 Mediumsrc/pty/session.rs- 717 lines 🔸 Mediumsrc/ssh/ssh_config/env_cache.rs- 656 lines 🔸 Mediumsrc/ssh/ssh_config/security.rs- 653 lines 🔸 Mediumsrc/jump/parser.rs- 613 lines 🔸 Medium
🟢 Lower Priority (500-600 lines)
src/ssh/auth.rs- 564 linessrc/forwarding/local.rs- 543 linessrc/forwarding/remote.rs- 542 linessrc/cli.rs- 538 linessrc/forwarding/manager.rs- 537 linessrc/ssh/ssh_config/parser.rs- 535 lines
Impact
- Maintainability: Large files are harder to navigate and modify
- Testing: Difficult to write focused unit tests for oversized modules
- Code Review: Hard to review large files effectively
- Collaboration: Increased merge conflicts due to many developers working on the same large files
- Architecture: Single responsibility principle violations
Proposed Modular Architecture
1. src/ssh/client.rs (1,394 lines) → Feature-based modules
Split into:
src/ssh/
├── mod.rs # Public exports
├── high_level_client.rs # High-level SSH client wrapper (~300 lines)
├── session_manager.rs # Session lifecycle management (~250 lines)
├── command_executor.rs # Remote command execution (~300 lines)
├── file_operations.rs # File transfer operations (~250 lines)
├── connection_pool.rs # Connection pooling logic (~200 lines)
└── error_handling.rs # Client-specific error handling (~100 lines)
2. src/commands/interactive.rs (1,383 lines) → UI/Logic separation
Split into:
src/commands/interactive/
├── mod.rs # Public API and module coordination (~150 lines)
├── ui.rs # User interface components and rendering (~300 lines)
├── input_handler.rs # Input processing and event handling (~250 lines)
├── state_manager.rs # Session state and data management (~300 lines)
├── terminal_controller.rs # Terminal operations and control sequences (~250 lines)
└── command_executor.rs # Command execution and result processing (~150 lines)
3. src/jump/chain.rs (1,113 lines) → Chain management modules
Split into:
src/jump/
├── mod.rs # Public exports (~50 lines)
├── chain_builder.rs # Chain construction and validation (~300 lines)
├── hop_manager.rs # Individual hop management (~250 lines)
├── connection_pool.rs # Connection pooling for jump hosts (~300 lines)
├── error_recovery.rs # Failure handling and retry logic (~200 lines)
└── parser.rs # Existing parser module (613 lines)
4. src/ssh/tokio_client/client.rs (1,079 lines) → Protocol layers
Split into:
src/ssh/tokio_client/
├── mod.rs # Public API exports (~100 lines)
├── connection.rs # Connection establishment and management (~300 lines)
├── authentication.rs # Auth methods (agent, key, password) (~250 lines)
├── channel_manager.rs # SSH channel operations (~250 lines)
├── file_transfer.rs # SFTP operations (upload/download) (~200 lines)
└── error.rs # Existing error module
5. src/main.rs (976 lines) → Application layers
Split into:
src/
├── main.rs # Entry point (~100 lines)
├── app.rs # Application structure and coordination (~300 lines)
├── command_dispatcher.rs # Command routing and execution (~300 lines)
├── initialization.rs # App initialization and setup (~200 lines)
└── signal_handler.rs # Signal handling logic (~100 lines)
6. src/config.rs (926 lines) → Configuration domains
Split into:
src/config/
├── mod.rs # Main config struct and loading (~150 lines)
├── parser.rs # YAML/file parsing logic (~200 lines)
├── validation.rs # Configuration validation (~200 lines)
├── defaults.rs # Default values and constants (~100 lines)
├── env_expansion.rs # Environment variable expansion (~150 lines)
└── cluster_config.rs # Cluster-specific configuration (~150 lines)
7. src/ssh/config_cache.rs (757 lines) → Cache components
Split into:
src/ssh/config_cache/
├── mod.rs # Public API (~100 lines)
├── cache_manager.rs # Cache management logic (~300 lines)
├── storage.rs # Cache storage backend (~200 lines)
└── invalidation.rs # Cache invalidation strategies (~150 lines)
8. src/pty/session.rs (717 lines) → PTY concerns
Split into:
src/pty/
├── mod.rs # Module exports (~50 lines)
├── session_manager.rs # PTY session management (~300 lines)
├── terminal_control.rs # Terminal control operations (~250 lines)
└── mode_handler.rs # Terminal mode handling (~150 lines)
Implementation Strategy
Phase 1: Critical Files (>1000 lines)
- ssh/client.rs (1,394 lines) - Largest file, highest impact
- commands/interactive.rs (1,383 lines) - User-facing, high complexity
- jump/chain.rs (1,113 lines) - Complex connection logic
- ssh/tokio_client/client.rs (1,079 lines) - Core SSH functionality
Phase 2: High Priority (800-1000 lines)
- main.rs (976 lines) - Application entry point
- config.rs (926 lines) - Foundation component
- forwarding/dynamic.rs (830 lines) - Advanced feature
- executor.rs (823 lines) - Core execution logic
Phase 3: Medium Priority (600-800 lines)
- ssh/config_cache.rs (757 lines)
- pty/session.rs (717 lines)
- ssh/ssh_config/env_cache.rs (656 lines)
- ssh/ssh_config/security.rs (653 lines)
- jump/parser.rs (613 lines)
Phase 4: Lower Priority (500-600 lines)
Files 14-19: Monitor for now, split if they continue to grow
Acceptance Criteria
- All files should be under 500 lines (target: 300-400 lines max)
- Each module should have a single, clear responsibility
- Public APIs should remain unchanged (no breaking changes)
- All existing tests should continue to pass
- New module boundaries should be logical and cohesive
- Documentation should be updated to reflect new structure (ARCHITECTURE.md)
- Each split should be done in separate PRs for easier review
Implementation Guidelines
- Preserve API compatibility: Public interfaces should remain unchanged
- Maintain test coverage: All existing functionality must remain tested
- Use Rust module system: Leverage
mod.rsfiles for clean organization - Document decisions: Update ARCHITECTURE.md with rationale for splits
- Incremental approach: Split one file at a time to minimize risk
- Run checks: Ensure
cargo clippy,cargo fmt, andcargo testpass after each split
Benefits
- Improved maintainability: Easier to understand and modify individual modules
- Better testability: Focused unit tests for specific responsibilities
- Enhanced collaboration: Reduced merge conflicts through better separation
- Clearer architecture: More obvious module boundaries and dependencies
- Future scalability: Easier to add new features without creating more oversized files
- Reduced cognitive load: Developers can focus on smaller, more manageable units
Priority Order for Implementation
ssh/client.rs(1,394 lines) - Largest file, most criticalcommands/interactive.rs(1,383 lines) - User-facing complexityjump/chain.rs(1,113 lines) - Complex connection managementssh/tokio_client/client.rs(1,079 lines) - Core SSH operationsmain.rs(976 lines) - Application structureconfig.rs(926 lines) - Configuration foundationforwarding/dynamic.rs(830 lines) - Advanced forwardingexecutor.rs(823 lines) - Parallel execution logic
Each file split should be implemented as a separate GitHub issue and pull request to ensure thorough review and testing.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
priority:highHigh priority issueHigh priority issuestatus:doneCompletedCompletedtype:enhancementNew feature or requestNew feature or requesttype:refactorCode refactoringCode refactoring