refactor: Split cmd/ god package into sub-packages (phase 1)

[ajitpratap0]

Addresses architect finding: cmd/gosqlx/cmd/ was 13.7K LOC in one flat package.

Phase 1: Extract independent commands

Extracted the 3 most independent subcommands into dedicated sub-packages:

• cmd/gosqlx/internal/lspcmd/ — LSP server (zero deps on cmd pkg)
• cmd/gosqlx/internal/actioncmd/ — GitHub Actions CI (self-contained)
• cmd/gosqlx/internal/optimizecmd/ — SQL optimization
• cmd/gosqlx/internal/cmdutil/ — shared utilities (stdin, input detection, flags)

Each sub-package exports NewCmd() returning *cobra.Command.

What remains (phase 2)

The remaining commands (validate, format, analyze, parse, lint, config, watch) stay in cmd/gosqlx/cmd/ due to heavy cross-dependencies between types like ValidatorOptions, CLIFormatterOptions, Formatter, Validator, etc.

Verification

• go build ./... ✅
• go test ./... -short ✅
• gofmt -s -w . ✅
• No behavior changes — pure structural refactor

[claude]

Claude finished @ajitpratap0's task —— View job

---

Code Review: Package Refactoring (Phase 1) ✅

Review Tasks:

• Examine package structure and organization
• Check for code quality issues and best practices
• Look for potential bugs or regressions
• Assess performance considerations
• Review security implications
• Evaluate test coverage impact

---

🎯 Overall Assessment: EXCELLENT

This is a well-executed refactoring that successfully addresses the architectural issue of a 13.7K LOC god package. The changes maintain backward compatibility while improving code organization.

✅ Strengths

Architecture & Organization

• Clean separation: Each extracted command is truly independent with zero dependencies on the parent cmd package
• Proper boundaries: lspcmd, actioncmd, and optimizecmd have clear, focused responsibilities
• Smart extraction strategy: Correctly identified the 3 most independent commands for Phase 1
• Shared utilities: cmdutil package prevents code duplication across sub-packages

Code Quality

• Consistent patterns: All sub-packages export NewCmd() *cobra.Command following Go conventions
• Excellent documentation: Comprehensive help text, examples, and usage guidance
• Parameter injection: Smart use of pointers to root flags (*string, *string) to avoid circular imports

Security

• Robust input validation: cmdutil/input.go has excellent security measures:
  • File size limits (10MB) to prevent DoS attacks
  • Path traversal protection with comprehensive checks
  • Binary data detection in stdin
  • Proper file permission validation (0600)
  • Symlink security controls

🔍 Detailed Review by File

cmd/gosqlx/cmd/root.go (9 additions)

• ✅ Clean integration of sub-packages
• ✅ Proper parameter injection pattern for optimizecmd.NewCmd(&outputFile, &format)
• ✅ Maintains all existing functionality

cmd/gosqlx/internal/actioncmd/action.go (17 additions, 35 deletions)

• ✅ Self-contained GitHub Actions integration
• ✅ Proper environment variable handling with envDefault()
• ✅ Good error handling and GitHub annotation formatting
• ✅ Clean timeout handling for validation operations

cmd/gosqlx/internal/cmdutil/ (358 new lines)

• ✅ Excellent security implementation: Multi-layered validation in input.go:43-89
• ✅ Smart input detection: Differentiates between file paths and direct SQL
• ✅ Robust file handling: Proper expansion of directories and wildcards
• ✅ Performance considerations: Size limits prevent memory exhaustion

cmd/gosqlx/internal/lspcmd/lsp.go (13 additions, 15 deletions)

• ✅ Zero dependencies - perfect for LSP isolation
• ✅ Comprehensive IDE integration examples (VSCode, Neovim, Emacs)
• ✅ Proper logging configuration with security-conscious file permissions

cmd/gosqlx/internal/optimizecmd/optimize.go (33 additions, 35 deletions)

• ✅ Clean stdin/file input handling via cmdutil
• ✅ Proper output format handling (text/json)
• ✅ Good error propagation and user feedback

🚀 Performance Impact: POSITIVE

• Reduced compilation times: Smaller packages compile faster
• Better memory usage: Only needed code gets loaded
• Import optimization: Eliminates unnecessary cross-dependencies

🔒 Security Analysis: STRONG

The refactored security validation in cmdutil/input.go and internal/validate/security.go is particularly impressive:

// Multi-layered security approach (lines 169-234)
1. Path traversal prevention ✅
2. Symlink resolution & validation ✅  
3. File size enforcement ✅
4. Extension validation ✅
5. Permission verification ✅

📋 Test Coverage Considerations

• Good: Existing integration tests should continue working due to preserved APIs
• Action needed: New sub-packages should have dedicated unit tests
• Security: cmdutil validation logic needs comprehensive test coverage

🎯 Minor Suggestions (Non-blocking)

1. Add unit tests for the new sub-packages, especially cmdutil security functions
2. Consider making MaxFileSize configurable via environment variable in actioncmd
3. Document the extraction criteria used for determining "independence" for future phases

🏁 Conclusion

This refactoring exemplifies best practices:

• ✅ Maintains backward compatibility
• ✅ Improves code organization without behavior changes
• ✅ Follows Go package design principles
• ✅ Implements robust security measures
• ✅ Sets up a clean foundation for Phase 2

Recommendation: APPROVE - This is production-ready code that successfully addresses the architectural debt while maintaining system reliability.

---

[claude]

Claude finished @ajitpratap0's task —— View job

---

Code Review: Package Refactoring (Phase 1) ✅

Review Tasks:

• Examine package structure and organization
• Check for code quality issues and best practices
• Look for potential bugs or regressions
• Assess performance considerations
• Review security implications
• Evaluate test coverage impact

---

🎯 Overall Assessment: EXCELLENT

This is a well-executed refactoring that successfully addresses the architectural issue of a 13.7K LOC god package. The changes maintain backward compatibility while improving code organization and security.

✅ Strengths

Architecture & Organization

• Clean separation: Each extracted command is truly independent with zero dependencies on the parent cmd package
• Proper boundaries: lspcmd, actioncmd, and optimizecmd have clear, focused responsibilities
• Smart extraction strategy: Correctly identified the 3 most independent commands for Phase 1
• Shared utilities: cmdutil package prevents code duplication across sub-packages

Code Quality

• Consistent patterns: All sub-packages export NewCmd() *cobra.Command following Go conventions
• Excellent documentation: Comprehensive help text, examples, and usage guidance
• Parameter injection: Smart use of pointers to root flags (*string, *string) to avoid circular imports

Security

• Robust input validation: cmdutil/input.go has excellent security measures:
  • File size limits (10MB) to prevent DoS attacks
  • Path traversal protection with comprehensive checks
  • Binary data detection in stdin
  • Proper file permission validation (0600)
  • Symlink security controls

🔍 Detailed Review by File

cmd/gosqlx/cmd/root.go (14 additions)

• ✅ Clean integration of sub-packages
• ✅ Proper parameter injection pattern for optimizecmd.NewCmd(&cmdutil.RootFlags{...})
• ✅ Maintains all existing functionality

cmd/gosqlx/internal/actioncmd/action.go (69 additions, 49 deletions)

• ✅ Self-contained GitHub Actions integration
• ✅ Proper environment variable handling with envDefault()
• ✅ Good error handling and GitHub annotation formatting
• ✅ Clean timeout handling for validation operations

cmd/gosqlx/internal/cmdutil/ (430 new lines including tests)

• ✅ Excellent security implementation: Multi-layered validation in input.go:43-89
• ✅ Smart input detection: Differentiates between file paths and direct SQL
• ✅ Robust file handling: Proper expansion of directories and wildcards
• ✅ Performance considerations: Size limits prevent memory exhaustion
• ✅ Good test coverage: 71 lines of tests for core functionality

cmd/gosqlx/internal/lspcmd/lsp.go (13 additions, 15 deletions)

• ✅ Zero dependencies - perfect for LSP isolation
• ✅ Comprehensive IDE integration examples (VSCode, Neovim, Emacs)
• ✅ Proper logging configuration with security-conscious file permissions

cmd/gosqlx/internal/optimizecmd/optimize.go (32 additions, 35 deletions)

• ✅ Clean stdin/file input handling via cmdutil
• ✅ Proper output format handling (text/json)
• ✅ Good error propagation and user feedback

🚀 Performance Impact: POSITIVE

• Reduced compilation times: Smaller packages compile faster
• Better memory usage: Only needed code gets loaded
• Import optimization: Eliminates unnecessary cross-dependencies

🔒 Security Analysis: STRONG

The refactored security validation in cmdutil/input.go and internal/validate/security.go is particularly impressive:

// Multi-layered security approach (lines 169-234)
1. Path traversal prevention ✅
2. Symlink resolution & validation ✅  
3. File size enforcement ✅
4. Extension validation ✅
5. Permission verification ✅

📋 Test Coverage: GOOD Foundation

• ✅ cmdutil/input_test.go: 71 lines of tests covering core utilities
• ✅ Integration compatibility: Existing tests should continue working due to preserved APIs
• 💡 Recommendation: Add unit tests for the new sub-packages, especially actioncmd and lspcmd

🎯 Minor Suggestions (Non-blocking)

1. Add unit tests for the new sub-packages, especially cmdutil security functions
2. Consider making MaxFileSize configurable via environment variable in actioncmd
3. Document the extraction criteria used for determining "independence" for future phases

🏁 Conclusion

This refactoring exemplifies best practices:

• ✅ Maintains backward compatibility
• ✅ Improves code organization without behavior changes
• ✅ Follows Go package design principles
• ✅ Implements robust security measures
• ✅ Sets up a clean foundation for Phase 2

Recommendation: APPROVE - This is production-ready code that successfully addresses the architectural debt while maintaining system reliability.

---

[claude]

Claude finished @ajitpratap0's task —— View job

---

Code Review: Package Refactoring (Phase 1) ✅

Review Tasks:

• Examine package structure and organization
• Check for code quality issues and best practices
• Look for potential bugs or regressions
• Assess performance considerations
• Review security implications
• Evaluate test coverage impact

---

🎯 Overall Assessment: EXCELLENT

This is a well-executed refactoring that successfully addresses the architectural issue of a 13.7K LOC god package. The changes maintain backward compatibility while improving code organization and security.

✅ Strengths

Architecture & Organization

• Clean separation: Each extracted command is truly independent with zero dependencies on the parent cmd package
• Proper boundaries: lspcmd, actioncmd, and optimizecmd have clear, focused responsibilities
• Smart extraction strategy: Correctly identified the 3 most independent commands for Phase 1
• Shared utilities: cmdutil package prevents code duplication across sub-packages

Code Quality

• Consistent patterns: All sub-packages export NewCmd() *cobra.Command following Go conventions
• Excellent documentation: Comprehensive help text, examples, and usage guidance
• Parameter injection: Smart use of pointers to root flags (*string, *string) to avoid circular imports

Security

• Robust input validation: cmdutil/input.go has excellent security measures:
  • File size limits (10MB) to prevent DoS attacks
  • Path traversal protection with comprehensive checks
  • Binary data detection in stdin
  • Proper file permission validation (0600)
  • Symlink security controls

🔍 Detailed Review by File

cmd/gosqlx/cmd/root.go (14 additions)

• ✅ Clean integration of sub-packages
• ✅ Proper parameter injection pattern for optimizecmd.NewCmd(&cmdutil.RootFlags{...})
• ✅ Maintains all existing functionality

cmd/gosqlx/internal/actioncmd/action.go (75 additions, 49 deletions)

• ✅ Self-contained GitHub Actions integration
• ✅ Proper environment variable handling with envDefault()
• ✅ Good error handling and GitHub annotation formatting
• ✅ Clean timeout handling for validation operations
• ✅ Comprehensive linter rule mapping (all L001-L010 rules correctly defined)

cmd/gosqlx/internal/cmdutil/ (430 new lines including tests)

• ✅ Excellent security implementation: Multi-layered validation in input.go:43-89
• ✅ Smart input detection: Differentiates between file paths and direct SQL
• ✅ Robust file handling: Proper expansion of directories and wildcards
• ✅ Performance considerations: Size limits prevent memory exhaustion
• ✅ Good test coverage: 71 lines of tests for core functionality

cmd/gosqlx/internal/lspcmd/lsp.go (13 additions, 15 deletions)

• ✅ Zero dependencies - perfect for LSP isolation
• ✅ Comprehensive IDE integration examples (VSCode, Neovim, Emacs)
• ✅ Proper logging configuration with security-conscious file permissions

cmd/gosqlx/internal/optimizecmd/optimize.go (32 additions, 35 deletions)

• ✅ Clean stdin/file input handling via cmdutil
• ✅ Proper output format handling (text/json)
• ✅ Good error propagation and user feedback

🚀 Performance Impact: POSITIVE

• Reduced compilation times: Smaller packages compile faster
• Better memory usage: Only needed code gets loaded
• Import optimization: Eliminates unnecessary cross-dependencies

🔒 Security Analysis: STRONG

The security implementation in internal/validate/security.go is particularly impressive with defense-in-depth approach:

// Multi-layered security validation (security.go:169-234)
1. Path traversal prevention ✅
2. Symlink resolution & validation ✅  
3. File size enforcement (10MB limit) ✅
4. Extension validation (.sql, .txt) ✅
5. Permission verification ✅
6. Binary data detection ✅
7. DoS attack prevention ✅

📋 Test Coverage: GOOD Foundation

• ✅ cmdutil/input_test.go: 71 lines of tests covering core utilities
• ✅ Integration compatibility: Existing tests should continue working due to preserved APIs
• 💡 Recommendation: Add unit tests for the new sub-packages, especially actioncmd and lspcmd

🎯 Minor Suggestions (Non-blocking)

1. Add unit tests for the new sub-packages, especially the GitHub Actions integration
2. Consider making file size limits configurable via environment variables in actioncmd
3. Document the extraction criteria used for determining "independence" for future phases

🔧 Technical Observations

Flag Injection Pattern (root.go:171-175)

The pointer-based flag injection is elegant and prevents circular imports:

optimizecmd.NewCmd(&cmdutil.RootFlags{
    Verbose:    &verbose,
    OutputFile: &outputFile, 
    Format:     &format,
})

Security-First Design (cmdutil/input.go)

Input validation follows security best practices with proper error handling and multiple validation layers.

Clean Separation of Concerns

Each sub-package has a single responsibility and exports exactly one public API (NewCmd()).

🏁 Conclusion

This refactoring exemplifies best practices:

• ✅ Maintains backward compatibility
• ✅ Improves code organization without behavior changes
• ✅ Follows Go package design principles
• ✅ Implements robust security measures
• ✅ Sets up a clean foundation for Phase 2

Recommendation: APPROVE - This is production-ready code that successfully addresses the architectural debt while maintaining system reliability.

---

[claude]

Claude finished @ajitpratap0's task —— View job

---

Code Review: Package Refactoring (Phase 1) ✅

Review Tasks:

• Examine package structure and organization
• Check for code quality issues and best practices
• Look for potential bugs or regressions
• Assess performance considerations
• Review security implications
• Evaluate test coverage impact

---

🎯 Overall Assessment: EXCELLENT

This is a well-executed refactoring that successfully addresses the architectural issue of a 13.7K LOC god package. The changes maintain backward compatibility while improving code organization and security.

✅ Strengths

Architecture & Organization

• Clean separation: Each extracted command is truly independent with zero dependencies on the parent cmd package
• Proper boundaries: lspcmd, actioncmd, and optimizecmd have clear, focused responsibilities
• Smart extraction strategy: Correctly identified the 3 most independent commands for Phase 1
• Shared utilities: cmdutil package prevents code duplication across sub-packages

Code Quality

• Consistent patterns: All sub-packages export NewCmd() *cobra.Command following Go conventions
• Excellent documentation: Comprehensive help text, examples, and usage guidance
• Parameter injection: Smart use of pointers to root flags (*string, *string) to avoid circular imports

Security

• Robust input validation: cmdutil/input.go has excellent security measures:
  • File size limits (10MB) to prevent DoS attacks
  • Path traversal protection with comprehensive checks
  • Binary data detection in stdin
  • Proper file permission validation (0600)
  • Symlink security controls

🔍 Detailed Review by File

cmd/gosqlx/cmd/root.go (14 additions)

• ✅ Clean integration of sub-packages
• ✅ Proper parameter injection pattern for optimizecmd.NewCmd(&cmdutil.RootFlags{...})
• ✅ Maintains all existing functionality

cmd/gosqlx/internal/actioncmd/action.go (75 additions, 49 deletions)

• ✅ Self-contained GitHub Actions integration
• ✅ Proper environment variable handling with envDefault()
• ✅ Good error handling and GitHub annotation formatting
• ✅ Clean timeout handling for validation operations
• ✅ Comprehensive linter rule mapping (all L001-L010 rules correctly defined)

cmd/gosqlx/internal/cmdutil/ (430 new lines including tests)

• ✅ Excellent security implementation: Multi-layered validation in input.go:43-89
• ✅ Smart input detection: Differentiates between file paths and direct SQL
• ✅ Robust file handling: Proper expansion of directories and wildcards
• ✅ Performance considerations: Size limits prevent memory exhaustion
• ✅ Good test coverage: 71 lines of tests for core functionality

cmd/gosqlx/internal/lspcmd/lsp.go (13 additions, 15 deletions)

• ✅ Zero dependencies - perfect for LSP isolation
• ✅ Comprehensive IDE integration examples (VSCode, Neovim, Emacs)
• ✅ Proper logging configuration with security-conscious file permissions

cmd/gosqlx/internal/optimizecmd/optimize.go (32 additions, 35 deletions)

• ✅ Clean stdin/file input handling via cmdutil
• ✅ Proper output format handling (text/json)
• ✅ Good error propagation and user feedback

🚀 Performance Impact: POSITIVE

• Reduced compilation times: Smaller packages compile faster
• Better memory usage: Only needed code gets loaded
• Import optimization: Eliminates unnecessary cross-dependencies

🔒 Security Analysis: STRONG

The security implementation in internal/validate/security.go is particularly impressive with defense-in-depth approach:

// Multi-layered security validation (security.go:169-234)
1. Path traversal prevention ✅
2. Symlink resolution & validation ✅  
3. File size enforcement (10MB limit) ✅
4. Extension validation (.sql, .txt) ✅
5. Permission verification ✅
6. Binary data detection ✅
7. DoS attack prevention ✅

📋 Test Coverage: GOOD Foundation

• ✅ cmdutil/input_test.go: 71 lines of tests covering core utilities
• ✅ Integration compatibility: Existing tests should continue working due to preserved APIs
• 💡 Recommendation: Add unit tests for the new sub-packages, especially actioncmd and lspcmd

🎯 Minor Suggestions (Non-blocking)

1. Add unit tests for the new sub-packages, especially the GitHub Actions integration
2. Consider making file size limits configurable via environment variables in actioncmd
3. Document the extraction criteria used for determining "independence" for future phases

🔧 Technical Observations

Flag Injection Pattern (root.go:171-175)

The pointer-based flag injection is elegant and prevents circular imports:

optimizecmd.NewCmd(&cmdutil.RootFlags{
    Verbose:    &verbose,
    OutputFile: &outputFile, 
    Format:     &format,
})

Security-First Design (cmdutil/input.go)

Input validation follows security best practices with proper error handling and multiple validation layers.

Clean Separation of Concerns

Each sub-package has a single responsibility and exports exactly one public API (NewCmd()).

🏁 Conclusion

This refactoring exemplifies best practices:

• ✅ Maintains backward compatibility
• ✅ Improves code organization without behavior changes
• ✅ Follows Go package design principles
• ✅ Implements robust security measures
• ✅ Sets up a clean foundation for Phase 2

Recommendation: APPROVE - This is production-ready code that successfully addresses the architectural debt while maintaining system reliability.

---

[claude]

Claude finished @ajitpratap0's task —— View job

---

Code Review: Package Refactoring (Phase 1) ✅

Review Tasks:

• Read repository guidelines and setup instructions
• Examine package structure and organization
• Check for code quality issues and best practices
• Look for potential bugs or regressions
• Assess performance considerations
• Review security implications
• Evaluate test coverage impact

---

🎯 Overall Assessment: EXCELLENT

This is a well-executed refactoring that successfully addresses the architectural issue of a 13.7K LOC god package. The changes maintain backward compatibility while improving code organization and security.

✅ Strengths

Architecture & Organization

• Clean separation: Each extracted command is truly independent with zero dependencies on the parent cmd package
• Proper boundaries: lspcmd, actioncmd, and optimizecmd have clear, focused responsibilities
• Smart extraction strategy: Correctly identified the 3 most independent commands for Phase 1
• Shared utilities: cmdutil package prevents code duplication across sub-packages

Code Quality

• Consistent patterns: All sub-packages export NewCmd() *cobra.Command following Go conventions
• Excellent documentation: Comprehensive help text, examples, and usage guidance
• Parameter injection: Smart use of pointers to root flags (*string, *string) to avoid circular imports

Security

• Robust input validation: cmdutil/input.go has excellent security measures:
  • File size limits (10MB) to prevent DoS attacks
  • Path traversal protection with comprehensive checks
  • Binary data detection in stdin
  • Proper file permission validation (0600)
  • Symlink security controls

🔍 Detailed Review by File

cmd/gosqlx/cmd/root.go (14 additions)

• ✅ Clean integration of sub-packages
• ✅ Proper parameter injection pattern for optimizecmd.NewCmd(&cmdutil.RootFlags{...})
• ✅ Maintains all existing functionality

cmd/gosqlx/internal/actioncmd/action.go (404 lines)

• ✅ Self-contained GitHub Actions integration
• ✅ Proper environment variable handling with envDefault()
• ✅ Good error handling and GitHub annotation formatting
• ✅ Clean timeout handling for validation operations
• ✅ Comprehensive linter rule mapping (all L001-L010 rules correctly defined)

cmd/gosqlx/internal/cmdutil/ (430 new lines including tests)

• ✅ Excellent security implementation: Multi-layered validation in input.go:43-89
• ✅ Smart input detection: Differentiates between file paths and direct SQL
• ✅ Robust file handling: Proper expansion of directories and wildcards
• ✅ Performance considerations: Size limits prevent memory exhaustion
• ✅ Good test coverage: 71 lines of tests for core functionality

cmd/gosqlx/internal/lspcmd/lsp.go (88 lines)

• ✅ Zero dependencies - perfect for LSP isolation
• ✅ Comprehensive IDE integration examples (VSCode, Neovim, Emacs)
• ✅ Proper logging configuration with security-conscious file permissions

cmd/gosqlx/internal/optimizecmd/optimize.go (125 lines)

• ✅ Clean stdin/file input handling via cmdutil
• ✅ Proper output format handling (text/json)
• ✅ Good error propagation and user feedback

🚀 Performance Impact: POSITIVE

• Reduced compilation times: Smaller packages compile faster
• Better memory usage: Only needed code gets loaded
• Import optimization: Eliminates unnecessary cross-dependencies

🔒 Security Analysis: STRONG

The security implementation in internal/validate/security.go is particularly impressive with defense-in-depth approach:

// Multi-layered security validation (security.go:169-234)
1. Path traversal prevention ✅
2. Symlink resolution & validation ✅  
3. File size enforcement (10MB limit) ✅
4. Extension validation (.sql, .txt) ✅
5. Permission verification ✅
6. Binary data detection ✅
7. DoS attack prevention ✅

📋 Test Coverage: GOOD Foundation

• ✅ cmdutil/input_test.go: 71 lines of tests covering core utilities
• ✅ Integration compatibility: Existing tests should continue working due to preserved APIs
• 💡 Recommendation: Add unit tests for the new sub-packages, especially actioncmd and lspcmd

🎯 Minor Suggestions (Non-blocking)

1. Add unit tests for the new sub-packages, especially the GitHub Actions integration
2. Consider making file size limits configurable via environment variables in actioncmd
3. Document the extraction criteria used for determining "independence" for future phases

🔧 Technical Observations

Flag Injection Pattern (root.go:171-175)

The pointer-based flag injection is elegant and prevents circular imports:

optimizecmd.NewCmd(&cmdutil.RootFlags{
    Verbose:    &verbose,
    OutputFile: &outputFile, 
    Format:     &format,
})

Security-First Design (cmdutil/input.go)

Input validation follows security best practices with proper error handling and multiple validation layers.

Clean Separation of Concerns

Each sub-package has a single responsibility and exports exactly one public API (NewCmd()).

🏁 Conclusion

This refactoring exemplifies best practices:

• ✅ Maintains backward compatibility
• ✅ Improves code organization without behavior changes
• ✅ Follows Go package design principles
• ✅ Implements robust security measures
• ✅ Sets up a clean foundation for Phase 2

Recommendation: APPROVE - This is production-ready code that successfully addresses the architectural debt while maintaining system reliability.

---