docs(cortex-cli): add comprehensive analysis and improvement recommendations

This analysis document covers:
- Critical issues: error handling inconsistencies, input validation gaps
- Important points: code duplication, logging improvements, test coverage
- Optimization opportunities: allocations, async usage
- Functional improvements: McpServer implementation, feature flags

Includes actionable recommendations organized by priority and a quality
assessment with score of 7.5/10.

Author: factorydroid

cortex-cli/ANALYSIS_IMPROVEMENTS.md

@@ -0,0 +1,353 @@
+# Analyse et Points d'Amélioration - Cortex CLI
+
+Date: 2026-01-27  
+Version analysée: 0.0.4
+
+## Résumé Exécutif
+
+La CLI Cortex est une application bien structurée et fonctionnelle avec une architecture modulaire. Cette analyse identifie les points d'amélioration potentiels organisés par catégorie de priorité.
+
+---
+
+## 🔴 Points Critiques (Haute Priorité)
+
+### 1. **Gestion des Erreurs Inconsistante**
+
+**Fichiers concernés:** `login.rs`, `main.rs`
+
+**Problème:** Les fonctions de login utilisent `std::process::exit()` directement au lieu de propager les erreurs, ce qui empêche une gestion correcte des ressources et les tests.
+
+```rust
+// Actuel (login.rs)
+pub async fn run_login_with_api_key(...) -> ! {
+    // ...
+    std::process::exit(0);
+}
+```
+
+**Recommandation:** Retourner `Result<()>` et laisser le caller gérer l'exit code.
+
+```rust
+pub async fn run_login_with_api_key(...) -> Result<()> {
+    // ...
+    Ok(())
+}
+```
+
+---
+
+### 2. **Validation des Entrées Utilisateur**
+
+**Fichiers concernés:** `agent_cmd.rs`, `mcp_cmd.rs`, `run_cmd.rs`
+
+**Points positifs:** De bonnes validations existent déjà (ex: `validate_server_name`, `validate_url`).
+
+**Points à améliorer:**
+
+- **agent_cmd.rs:**: Les noms d'agents vides sont bien validés, mais la validation est dispersée dans `run_create` et `run_generate`.
+
+```rust
+// Suggestion: Centraliser la validation
+fn validate_agent_name(name: &str) -> Result<()> {
+    if name.trim().is_empty() {
+        bail!("Agent name cannot be empty");
+    }
+    if !name.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') {
+        bail!("Agent name must contain only alphanumeric characters, hyphens, and underscores");
+    }
+    Ok(())
+}
+```
+
+---
+
+### 3. **Constantes et Timeouts Non Configurables**
+
+**Fichiers concernés:** `mcp_cmd.rs`, `scrape_cmd.rs`, `debug_cmd.rs`
+
+**Problème:** Plusieurs constantes sont hardcodées sans possibilité de configuration.
+
+```rust
+// mcp_cmd.rs
+const MAX_SERVER_NAME_LENGTH: usize = 64;
+const MAX_URL_LENGTH: usize = 2048;
+const MAX_ENV_VARS: usize = 50;
+
+// scrape_cmd.rs
+const MAX_FILE_SIZE: u64 = 10 * 1024 * 1024; // Dans run_cmd.rs
+
+// debug_cmd.rs - interval minimum hardcodé
+let interval_ms = if args.interval == 0 { 500 } else if args.interval < 100 { 100 } else { args.interval };
+```
+
+**Recommandation:** Centraliser ces constantes dans un module de configuration ou les rendre configurables via `config.toml`.
+
+---
+
+## 🟠 Points Importants (Priorité Moyenne)
+
+### 4. **Duplication de Code pour la Résolution des Session IDs**
+
+**Fichiers concernés:** `main.rs`, `export_cmd.rs`, `import_cmd.rs`, `run_cmd.rs`, `debug_cmd.rs`
+
+**Problème:** La logique de résolution des session IDs (full UUID ou prefix 8 chars) est dupliquée dans plusieurs fichiers.
+
+```rust
+// Pattern répété dans 5+ fichiers:
+let conversation_id: ConversationId = match session_id.parse() {
+    Ok(id) => id,
+    Err(_) => {
+        if session_id.len() == 8 {
+            // Try to find a session with matching prefix...
+        }
+    }
+};
+```
+
+**Recommandation:** Extraire dans un module commun:
+
+```rust
+// Dans un nouveau fichier: src/session_utils.rs
+pub fn resolve_session_id(
+    session_id: &str, 
+    fabric_home: &PathBuf
+) -> Result<ConversationId> {
+    // Logique centralisée
+}
+```
+
+---
+
+### 5. **Absence de Logging Structuré**
+
+**Fichiers concernés:** Tous
+
+**Problème:** Le logging utilise `eprintln!` et `println!` de manière inconsistante. Pas d'utilisation systématique de `tracing`.
+
+```rust
+// Actuel
+eprintln!("Warning: Failed to start mDNS advertising: {e}");
+println!("✓ Added stdio MCP server '{name}'");
+
+// Recommandé avec tracing
+tracing::warn!("Failed to start mDNS advertising: {}", e);
+tracing::info!(server_name = %name, "Added stdio MCP server");
+```
+
+**Recommandation:** Standardiser avec `tracing` pour:
+- Logging JSON structuré
+- Filtrage par niveau
+- Contexte de spans
+
+---
+
+### 6. **Tests Manquants ou Incomplets**
+
+**Fichiers concernés:** Plusieurs modules
+
+| Module | Couverture Tests |
+|--------|------------------|
+| `agent_cmd.rs` | ✅ Bonne (frontmatter, mode parsing) |
+| `mcp_cmd.rs` | ⚠️ Absente (pas de tests) |
+| `run_cmd.rs` | ✅ Bonne (model spec, mime types) |
+| `login.rs` | ⚠️ Absente (difficile avec exit) |
+| `scrape_cmd.rs` | ✅ Bonne (HTML to markdown) |
+| `github_cmd.rs` | ⚠️ Nécessite mocking |
+| `stats_cmd.rs` | ✅ Bonne |
+
+**Recommandation:** Ajouter des tests d'intégration et améliorer la couverture des modules critiques.
+
+---
+
+### 7. **Documentation API Incomplète**
+
+**Fichiers concernés:** `lib.rs`, tous les modules
+
+**Problème:** Les modules publics manquent de documentation rustdoc complète.
+
+```rust
+// Actuel (lib.rs)
+pub mod acp_cmd;
+pub mod agent_cmd;
+// etc.
+
+// Recommandé
+/// ACP (Agent Communication Protocol) server commands for IDE integration.
+/// 
+/// This module provides commands for running the ACP server which enables
+/// real-time communication with IDEs like Zed.
+pub mod acp_cmd;
+```
+
+---
+
+## 🟡 Points d'Optimisation (Priorité Basse)
+
+### 8. **Allocations String Évitables**
+
+**Fichiers concernés:** `scrape_cmd.rs`, `stats_cmd.rs`
+
+```rust
+// scrape_cmd.rs - Allocation répétée dans la boucle
+for line in content.lines() {
+    let trimmed = line.trim().to_string(); // Allocation évitable
+}
+
+// Mieux
+for line in content.lines() {
+    let trimmed = line.trim(); // &str suffit souvent
+}
+```
+
+---
+
+### 9. **Async/Await Non Utilisé Où Approprié**
+
+**Fichiers concernés:** `debug_cmd.rs`, certaines fonctions sync inutilement async
+
+```rust
+// Fonction qui n'utilise pas vraiment async
+async fn run_config(args: ConfigArgs) -> Result<()> {
+    // Pas d'opérations async dans le corps
+}
+```
+
+**Recommandation:** Marquer comme sync ou utiliser `spawn_blocking` pour les opérations CPU-bound.
+
+---
+
+### 10. **Gestion de la Progression des Downloads**
+
+**Fichiers concernés:** `upgrade_cmd.rs`
+
+```rust
+// Actuel - flush stdout non géré dans la closure
+.download_update(info, |progress| {
+    let pct = (progress.downloaded as f64 / progress.total as f64 * 100.0) as u32;
+    print!("\r  Downloading... {}% ({}/{})", pct, progress.downloaded, progress.total);
+    let _ = stdout().flush(); // Silently ignore errors
+})
+```
+
+**Recommandation:** Utiliser un writer dédié avec meilleure gestion des erreurs.
+
+---
+
+## 🔵 Améliorations Fonctionnelles Suggérées
+
+### 11. **Support Manquant pour `McpServer`**
+
+**Fichier:** `main.rs`
+
+```rust
+Some(Commands::McpServer) => {
+    eprintln!("MCP server mode not yet implemented");
+    Ok(())
+}
+```
+
+**Recommandation:** Implémenter ou supprimer de l'interface CLI.
+
+---
+
+### 12. **Feature Flags Hardcodés**
+
+**Fichier:** `main.rs`
+
+```rust
+async fn list_features() -> Result<()> {
+    let features = [
+        ("unified_exec", "stable", true),
+        ("web_search", "beta", false),
+        // ... hardcodé
+    ];
+}
+```
+
+**Recommandation:** Charger depuis la configuration ou le runtime.
+
+---
+
+### 13. **Amélioration du Shell Completion**
+
+**Fichier:** `main.rs`
+
+```rust
+// Actuel - supporte seulement les shells de base
+#[arg(value_enum, default_value_t = Shell::Bash)]
+shell: Shell,
+```
+
+**Recommandation:** 
+- Ajouter support Nushell, Elvish
+- Auto-détection du shell courant
+
+---
+
+## 📊 Métriques de Qualité
+
+### Complexité Cyclomatique Élevée
+
+| Fonction | Fichier | Complexité |
+|----------|---------|------------|
+| `process_node_to_markdown` | scrape_cmd.rs | ~40+ |
+| `run_debug` | mcp_cmd.rs | ~25+ |
+| `run_create` | agent_cmd.rs | ~20+ |
+| `run_local` | run_cmd.rs | ~20+ |
+
+**Recommandation:** Refactoriser en sous-fonctions plus petites.
+
+---
+
+### Dépendances Non Utilisées ou Optionnelles
+
+```toml
+# Cargo.toml - À vérifier si vraiment utilisées partout
+serde_yaml = "0.9"  # Utilisé uniquement pour agent parsing
+scraper = "0.22"    # Uniquement pour scrape_cmd
+hostname = { workspace = true }  # Uniquement pour mDNS
+```
+
+**Recommandation:** Rendre ces dépendances optionnelles avec feature flags.
+
+---
+
+## ✅ Points Positifs Notables
+
+1. **Excellente validation de sécurité** dans `mcp_cmd.rs` (URL validation, blocked patterns)
+2. **Bonne gestion des erreurs contextuelles** avec `anyhow::Context`
+3. **Architecture modulaire** claire avec séparation des commandes
+4. **Support multi-plateforme** bien pensé (Windows, macOS, Linux)
+5. **Tests unitaires** présents dans les modules critiques
+6. **Documentation inline** pour les commandes principales
+7. **Validation d'entrée** robuste dans `pr_cmd.rs` (branch names, refspecs)
+8. **Sécurité** bien pensée dans `uninstall_cmd.rs` (validate_path_safety)
+
+---
+
+## 🛠️ Plan d'Action Recommandé
+
+### Phase 1 (Court terme - 1-2 semaines)
+1. ✅ Refactoriser les fonctions login pour retourner `Result`
+2. ✅ Extraire la logique session ID dans un module commun
+3. ✅ Ajouter tests pour `mcp_cmd.rs`
+
+### Phase 2 (Moyen terme - 1 mois)
+1. Migrer vers `tracing` pour le logging
+2. Réduire la complexité cyclomatique des grandes fonctions
+3. Compléter la documentation rustdoc
+
+### Phase 3 (Long terme)
+1. Rendre les constantes configurables
+2. Implémenter `McpServer` ou le supprimer
+3. Ajouter feature flags pour les dépendances optionnelles
+
+---
+
+## Conclusion
+
+La CLI Cortex est une base solide avec une architecture bien pensée. Les améliorations identifiées sont principalement des optimisations et des standardisations qui amélioreront la maintenabilité et la testabilité du code sans nécessiter de refonte majeure.
+
+**Score global de qualité:** 7.5/10
+
+Les points forts sont la sécurité, la modularité et la couverture fonctionnelle. Les points à améliorer sont la gestion des erreurs, la duplication de code et la couverture de tests.