[Project Review] Architectural and Security Improvements

[albatrosary]

This issue tracks the systemic improvements and critical bug fixes identified during a comprehensive code review by Claude. The goal is to move from a functional prototype to a production-quality CLI tool.

🔴 Critical Issues (Must Fix First)

• Security: Symlink bypass in secure_resolve_path (utils.py)
  • os.path.abspath does not resolve symlinks. Use os.path.realpath to prevent directory traversal via malicious symlinks.
• Concurrency: Thread-safety in parallel tasks (handlers.py)
  • Parallel tasks in @sequence targeting the same engine concurrently mutate engine.history. Implement per-engine threading.Lock.
• Architecture: Conflicting initialize_engines() (config.py vs engines.py)
  • Remove duplicate logic in config.py and unify the signature to avoid runtime crashes.
• Functional: Gemini system prompt implementation (engines.py)
  • Use the native system_instruction parameter instead of injecting it as a "model" role message.
• Reliability: Code fence parser logic (utils.py)
  • Replace line-by-line parsing with a robust regex to handle nested fences and language identifiers correctly.

🟠 High & Medium Priority

• UX: Implement CLI Arguments & History (main.py)
  • Add argparse for --config, --no-log, and --version.
  • Add readline support for command history navigation.
• Refactoring: Eliminate Duplicated Splitters (parsers.py)
  • Extract _smart_split logic to handle both -> and || delimiters.
• Refactoring: Dynamic Engine Registry (parsers.py)
  • Derive VALID_COMMANDS from the active engine registry instead of hardcoding.
• UX: Handle Ctrl+C and Ctrl+D gracefully (main.py)
  • Implement double Ctrl+C to exit and handle EOFError.

🟡 Systemic Cleanups

• Reduce Comment Noise
  • Remove redundant "restating" comments across all modules (~20% line count reduction).
• Standardize Data Classes (parsers.py)
  • Use field(default_factory=list) instead of __post_init__ for mutable defaults.
• Unify Stream Output (handlers.py)
  • Create a helper for truncated stdout/stderr display.

📂 Module-Specific Notes

• config.py: Fix setup_logger ignoring no_log. Validate INI file existence.
• engines.py: Add timeouts to all API calls. Wrap Gemini errors in AIError.
• parsers.py: Consolidate indices_to_skip and bare_tokens patterns.
• utils.py: Use ANSI escape codes for clearing the thinking line.

Generated by Claude Review Pipeline