feat(cli): implement comprehensive command-line interface

[mjacobs]

Summary

• Implement complete CLI argument parsing with comprehensive flag support
• Add help system with detailed usage examples and environment variable documentation
• Support configuration hierarchy: CLI flags > environment variables > config files > defaults
• Add TOML configuration file support with structured sections (server, pool, logging, timeouts, tls)
• Implement robust configuration validation with clear error messages
• Add operational modes: verbose, quiet, daemon, dry-run

Key Features

CLI Commands & Flags

• Commands: start (default), config, version, help
• Connection: --listen-port, --listen-host, --server-host, --server-port
• Pool: --pool-mode, --max-client-conn, --pool-size
• Config: --config, --env, --dry-run
• Logging: --verbose, --quiet, --log-level, --log-connections
• Operational: --daemon, --pid-file

Configuration Management

• Environment variables with PGBUN_* prefix
• TOML configuration file support
• Configuration validation and clear error reporting
• Configuration display command

Operational Features

• Comprehensive help system with examples
• Graceful shutdown with PID file cleanup
• Daemon mode support
• Dry-run configuration validation

Test Plan

• Help and version commands work correctly
• CLI flag overrides work (tested with --listen-port, --pool-mode)
• Environment variable support works (PGBUN_LISTEN_PORT, PGBUN_POOL_MODE)
• TOML configuration file loading works
• Configuration validation with proper error messages
• Configuration hierarchy respected (CLI > env > file > defaults)
• Dry-run mode validates without starting server
• Error handling for invalid options and missing files

Examples

# Basic usage
pgbun --help                           # Shows comprehensive help
pgbun config                          # Shows current configuration

# Configuration overrides
pgbun -p 6433 --pool-mode transaction # CLI flags
PGBUN_LISTEN_PORT=7000 pgbun config   # Environment variables
pgbun --config example.conf           # TOML config file

# Operational modes
pgbun --dry-run --verbose             # Validate configuration
pgbun --daemon --pid-file /var/run/pgbun.pid  # Daemon mode

🤖 Generated with Claude Code

[claude]

Claude finished @mjacobs's task —— View job

---

PR Review: CLI Implementation

Comprehensive review of the CLI interface implementation focusing on code quality, security, performance, and best practices.

Review Progress

• Analyze CLI argument parsing implementation (src/cli.ts)
• Review configuration management changes (src/config.ts)
• Examine main entry point modifications (src/index.ts)
• Check TOML configuration file structure (example.conf)
• Evaluate package dependencies and security
• Assess error handling and validation
• Review performance implications
• Check for potential bugs and edge cases
• Evaluate test coverage gaps
• Provide comprehensive feedback and recommendations

---

Overall Assessment ⭐

This is a well-structured CLI implementation that adds significant functionality to pgbun. The code demonstrates good TypeScript practices, comprehensive validation, and thoughtful design. However, there are several areas that need attention for production readiness.

✅ Strengths

1. Comprehensive CLI Design: Excellent coverage of essential options with clear, consistent naming conventions
2. Configuration Hierarchy: Well-implemented precedence (CLI > env > file > defaults) matches industry standards
3. Robust Validation: Thorough input validation with clear error messages
4. TypeScript Usage: Good type safety throughout with proper interfaces
5. Help System: Comprehensive help documentation with examples

⚠️ Critical Issues

Security Concerns

1. Path Traversal Vulnerability (src/config.ts:218-222)

if (!fs.existsSync(configPath)) {
  throw new Error(`Configuration file not found: ${configPath}`);
}
const content = fs.readFileSync(configPath, 'utf-8');

Risk: No validation of config file paths allows potential directory traversal attacks
Recommendation: Validate and sanitize file paths, restrict to allowed directories

2. PID File Race Condition (src/index.ts:60-63, 73-80)

fs.writeFileSync(cliOptions.pidFile, process.pid.toString());
// ... later
fs.unlinkSync(cliOptions.pidFile);

Risk: No atomic operations or proper file locking for PID files
Recommendation: Use proper file locking mechanisms and validate PID file paths

Code Quality Issues

3. Inconsistent Error Handling (src/cli.ts:207-214)

try {
  const packageJson = require('../package.json');
  return packageJson.version || '0.1.0';
} catch {
  return '0.1.0';
}

Issue: Silent failure and hardcoded fallback version
Recommendation: Use proper error logging and consider build-time version injection

4. Type Safety Gaps (src/config.ts:223)

const parsed = TOML.parse(content) as any;

Issue: Using any type defeats TypeScript's type checking
Recommendation: Define proper TOML schema interfaces

5. Missing Bounds Checking (src/cli.ts:155-160)

if (!nextArg || isNaN(Number(nextArg))) {
  throw new Error(`Invalid stats period value: ${nextArg}`);
}
options.statsPeriod = Number(nextArg);

Issue: No validation for reasonable time period bounds
Recommendation: Add min/max validation for time-based options

Performance Considerations

6. Synchronous File Operations (src/config.ts:218-222)

const content = fs.readFileSync(configPath, 'utf-8');

Issue: Blocking file I/O during startup
Recommendation: Consider async file operations for better startup performance

7. Redundant Validation (Config constructor calls validate(), then load() may validate again)
   Recommendation: Optimize validation flow to avoid duplicate checks

Potential Bugs

8. Port Range Validation Duplication (CLI parser and Config class both validate ports)

// src/cli.ts:197-202
if (options.listenPort && (options.listenPort < 1 || options.listenPort > 65535)) {
// src/config.ts:92-98 (same validation)

Issue: Validation logic scattered across modules
Recommendation: Centralize validation logic

9. Incomplete CLI Mapping (src/config.ts:277-317)
   Missing mappings for some CLI options like logLevel, verbose, quiet

10. Environment Variable Type Coercion (src/config.ts:169-212)

const port = parseInt(process.env.PGBUN_LISTEN_PORT);
if (!isNaN(port)) config.listen_port = port;

Issue: No validation for environment variable ranges
Recommendation: Apply same validation as CLI arguments

🔧 Recommendations

High Priority Fixes

1. Secure file path handling: Implement path validation and sanitization
2. Fix type safety: Remove any types and add proper TOML schema interfaces
3. Centralize validation: Create a shared validation module
4. Add bounds checking: Validate all numeric inputs for reasonable ranges

Medium Priority Improvements

1. Add unit tests: Critical for CLI parsing and configuration loading
2. Improve error messages: More contextual error information
3. Add configuration validation: Validate TOML structure matches expected schema
4. Implement async file operations: For better startup performance

Low Priority Enhancements

1. Add shell completion: Bash/zsh completion scripts
2. Improve daemon mode: Proper daemonization with process management
3. Add configuration file discovery: Search common locations for config files

📋 Missing Test Coverage

The implementation lacks comprehensive test coverage for:

• CLI argument parsing edge cases
• Configuration hierarchy merging
• Error handling scenarios
• File I/O error conditions
• Environment variable parsing
• TOML configuration validation

🚀 Performance Impact

The CLI additions should have minimal runtime performance impact as most operations occur during startup. The main considerations are:

• Synchronous file I/O during config loading
• TOML parsing overhead
• Validation overhead (acceptable for startup)

Summary

This is a solid foundation for pgbun's CLI interface with good architectural decisions. The main concerns are security-related (path handling) and type safety. With the recommended fixes, this would be production-ready.

Estimated effort to address critical issues: 1-2 days
Overall recommendation: Approve with required security fixes

---