//! Concrete tool handlers that wire MCP tool calls to FURNACE agents.
//!
//! Each handler captures `Arc` references to the agents/databases it needs
//! and implements `ToolHandler` so it can be registered with the `McpServer`.

use std::path::PathBuf;
use std::sync::Arc;

use anyhow::Result;
use serde_json::{json, Value};

use crate::agents::aragorn::Aragorn;
use crate::agents::elrond::{Elrond, ScanStats};
use crate::agents::gandalf::Gandalf;
use crate::agents::legolas::Legolas;
use crate::agents::morpheus::Morpheus;
use crate::agents::trinity::Trinity;
use crate::mcp::prefetch::{PrefetchCompletion, build_prefetch_response};
use crate::db::SqliteDb;
use crate::knowledge::KnowledgeManager;
use crate::scroll_export::ScrollExporter;
use crate::scroll_import::ScrollImporter;
use crate::search::metrics::SearchMetrics;
use crate::search::orchestrator::SearchOrchestrator;
use crate::recipes::manager::RecipeManager;
use crate::sync::bridge::SyncBridge;
use crate::sync::credentials::CredentialStore;
use crate::sync::types::{
    ExternalRef, ExternalSystem, McpServerConfig, SyncDirection,
};
use crate::telemetry::tracker::{
    ScrollCreatedEvent, ScrollExecutedEvent, SearchEvent, TelemetryTracker,
};
use crate::types::{
    ExecutionMode, Scroll, ScrollEventListener, ScrollMetadata, ScrollStatus, SearchRequest,
};

use super::server::ToolHandler;

/// Validate that a required string field is present and non-empty.
fn require_non_empty(input: &Value, field: &str) -> Result<String> {
    let val = input[field].as_str().unwrap_or("");
    if val.trim().is_empty() {
        anyhow::bail!("'{field}' is required and must not be empty");
    }
    Ok(val.to_string())
}

// ---------------------------------------------------------------------------
// search_code
// ---------------------------------------------------------------------------

pub struct SearchCodeHandler {
    pub orchestrator: Arc<SearchOrchestrator>,
    pub telemetry: Arc<TelemetryTracker>,
}

#[async_trait::async_trait]
impl ToolHandler for SearchCodeHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let query = require_non_empty(&input, "query")?;
        let max_results = input["max_results"].as_u64().unwrap_or(10) as usize;
        let file_types: Vec<String> = input["file_types"]
            .as_array()
            .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect())
            .unwrap_or_default();
        let exclude_paths: Vec<String> = input["exclude_paths"]
            .as_array()
            .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect())
            .unwrap_or_else(|| vec!["node_modules".into(), "target".into(), ".git".into()]);

        let request = SearchRequest {
            query,
            max_results,
            file_types,
            exclude_paths,
        };

        let start = std::time::Instant::now();
        let response = self.orchestrator.search(&request).await?;
        let latency_ms = start.elapsed().as_millis() as u64;

        if let Err(e) = self.telemetry.track_search(SearchEvent {
            query_type: "code".into(),
            strategy: format!("{:?}", response.source),
            latency_ms,
            result_count: response.results.len(),
            tier_used: format!("{:?}", response.source),
        }) {
            tracing::debug!(error = %e, "Failed to track search telemetry");
        }

        Ok(serde_json::to_value(&response)?)
    }
}

// ---------------------------------------------------------------------------
// create_scroll
// ---------------------------------------------------------------------------

pub struct CreateScrollHandler {
    pub sqlite: Arc<SqliteDb>,
    pub gandalf: Arc<Gandalf>,
    pub telemetry: Arc<TelemetryTracker>,
    pub recipe_manager: Arc<RecipeManager>,
    pub scroll_exporter: Arc<ScrollExporter>,
}

#[async_trait::async_trait]
impl ToolHandler for CreateScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let intent = require_non_empty(&input, "intent")?;

        let recipe_name = input
            .get("recipe")
            .and_then(|v| v.as_str())
            .map(String::from);

        // Validate recipe exists if specified.
        if let Some(ref name) = recipe_name {
            self.recipe_manager
                .load_recipe(name)
                .await
                .map_err(|e| anyhow::anyhow!("Recipe '{}' not found: {}", name, e))?;
        }

        let now = chrono::Utc::now();
        let scroll_id = format!("SCROLL-{}", now.timestamp_millis());

        // Create a draft scroll from the intent.
        let scroll = Scroll {
            id: scroll_id.clone(),
            title: intent.chars().take(80).collect::<String>(),
            intent: intent.clone(),
            constraints: vec![],
            tasks: vec![],
            metadata: ScrollMetadata {
                execution_mode: ExecutionMode::AutoForge,
                auto_forge_config: Some(crate::types::AutoForgeConfig::default()),
                recipe_used: recipe_name,
                search_context: vec![],
            },
            status: ScrollStatus::Draft,
            created_at: now,
            updated_at: now,
            cleanup_after: None,
        };

        self.sqlite.create_scroll(&scroll).await?;

        // Extract constraints from the knowledge graph.
        let constraint_count = match self.gandalf.extract_constraints(&scroll_id).await {
            Ok(constraints) => {
                tracing::info!(
                    scroll_id = %scroll_id,
                    constraints = constraints.len(),
                    "Constraints extracted for new scroll"
                );
                constraints.len()
            }
            Err(e) => {
                tracing::warn!(
                    scroll_id = %scroll_id,
                    error = %e,
                    "Failed to extract constraints (scroll created without them)"
                );
                0
            }
        };

        if let Err(e) = self.telemetry.track_scroll_created(ScrollCreatedEvent {
            scroll_id: scroll_id.clone(),
            execution_mode: format!("{:?}", ExecutionMode::AutoForge),
            task_count: 0,
        }) {
            tracing::debug!(error = %e, "Failed to track scroll creation telemetry");
        }

        // Export .scroll file to disk for IDE extensions
        self.scroll_exporter.export_scroll_quiet(&scroll_id).await;

        Ok(json!({
            "scroll_id": scroll_id,
            "status": "Draft",
            "constraints_extracted": constraint_count,
            "message": format!("Scroll created from intent: {}", intent),
            "next_step": "Call `add_tasks` with the scroll_id and an array of file-level tasks (each with description, target_file, task_type, and generated_code).",
        }))
    }
}

// ---------------------------------------------------------------------------
// add_tasks
// ---------------------------------------------------------------------------

pub struct AddTasksHandler {
    pub sqlite: Arc<SqliteDb>,
    pub scroll_exporter: Arc<ScrollExporter>,
}

#[async_trait::async_trait]
impl ToolHandler for AddTasksHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        // Verify scroll exists
        let _scroll = self.sqlite.get_scroll(&scroll_id).await
            .map_err(|e| anyhow::anyhow!("Scroll '{}' not found: {}", scroll_id, e))?;

        let tasks_input = input["tasks"]
            .as_array()
            .ok_or_else(|| anyhow::anyhow!("'tasks' must be an array"))?;

        if tasks_input.is_empty() {
            return Err(anyhow::anyhow!("'tasks' array must not be empty"));
        }

        // Count existing tasks to avoid ID collisions on repeated add_tasks calls.
        let existing_tasks = self.sqlite.get_tasks_for_scroll(&scroll_id).await
            .unwrap_or_default();
        let offset = existing_tasks.len();

        let now = chrono::Utc::now();
        let mut task_ids = Vec::new();

        for (idx, task_input) in tasks_input.iter().enumerate() {
            let task_id = format!("{}-T{}", scroll_id, offset + idx + 1);

            let description = require_non_empty(task_input, "description")
                .map_err(|_| anyhow::anyhow!("Task {} requires a non-empty 'description'", idx + 1))?;
            let target_file = require_non_empty(task_input, "target_file")
                .map_err(|_| anyhow::anyhow!("Task {} requires a non-empty 'target_file'", idx + 1))?;
            let task_type_str = task_input["task_type"]
                .as_str()
                .unwrap_or("Create");
            let task_type = match task_type_str {
                "Modify" => crate::types::TaskType::Modify,
                "Delete" => crate::types::TaskType::Delete,
                "Test" => crate::types::TaskType::Test,
                _ => crate::types::TaskType::Create,
            };
            let generated_code = task_input
                .get("generated_code")
                .and_then(|v| v.as_str())
                .map(String::from);
            let dependencies: Vec<String> = task_input
                .get("dependencies")
                .and_then(|v| v.as_array())
                .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect())
                .unwrap_or_default();
            let group_label = task_input
                .get("group")
                .and_then(|v| v.as_str())
                .map(String::from);

            let task = crate::types::Task {
                id: task_id.clone(),
                scroll_id: scroll_id.clone(),
                description,
                target_file,
                task_type,
                status: crate::types::TaskStatus::Pending,
                dependencies,
                generated_code,
                diff: None,
                created_at: now,
                group_label,
            };

            self.sqlite.create_task(&task).await
                .map_err(|e| anyhow::anyhow!("Failed to create task {}: {}", task_id, e))?;

            task_ids.push(task_id);
        }

        // Re-export .scroll file with tasks
        self.scroll_exporter.export_scroll_quiet(&scroll_id).await;

        Ok(json!({
            "scroll_id": scroll_id,
            "task_ids": task_ids,
            "task_count": task_ids.len(),
            "next_step": "Present this scroll to the user for review: call `get_scroll` to see full details. \
                          The user can request changes via `update_task`, add more tasks via `add_tasks`, or \
                          remove tasks via `remove_tasks`. When the user is satisfied, call `execute_scroll` \
                          to apply changes (optionally `validate_scroll` first).",
        }))
    }
}

// ---------------------------------------------------------------------------
// update_scroll
// ---------------------------------------------------------------------------

pub struct UpdateScrollHandler {
    pub sqlite: Arc<SqliteDb>,
    pub sync_bridge: Arc<SyncBridge>,
    pub scroll_exporter: Arc<ScrollExporter>,
}

#[async_trait::async_trait]
impl ToolHandler for UpdateScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        // Verify scroll exists and get current status for sync
        let scroll = self.sqlite.get_scroll(&scroll_id).await
            .map_err(|e| anyhow::anyhow!("Scroll '{}' not found: {}", scroll_id, e))?;

        let title = input.get("title").and_then(|v| v.as_str());
        let intent = input.get("intent").and_then(|v| v.as_str());

        let execution_mode = input.get("execution_mode")
            .and_then(|v| v.as_str())
            .map(|s| match s {
                "AutoForge" => ExecutionMode::AutoForge,
                _ => ExecutionMode::Forge,
            });

        let auto_forge_config = input.get("auto_forge_config")
            .and_then(|v| v.as_object())
            .map(|obj| {
                let mut config = crate::types::AutoForgeConfig::default();
                if let Some(v) = obj.get("auto_apply").and_then(|v| v.as_bool()) {
                    config.auto_apply = v;
                }
                if let Some(v) = obj.get("auto_test").and_then(|v| v.as_bool()) {
                    config.auto_test = v;
                }
                if let Some(v) = obj.get("auto_verify").and_then(|v| v.as_bool()) {
                    config.auto_verify = v;
                }
                if let Some(v) = obj.get("max_iterations").and_then(|v| v.as_u64()) {
                    config.max_iterations = v as usize;
                }
                if let Some(v) = obj.get("test_command").and_then(|v| v.as_str()) {
                    config.test_command = Some(v.to_string());
                }
                config
            });

        self.sqlite.update_scroll_fields(
            &scroll_id,
            title,
            intent,
            execution_mode.as_ref(),
            auto_forge_config.as_ref().map(Some),
        ).await?;

        // Trigger sync to external systems
        use crate::types::ScrollEventListener;
        self.sync_bridge.on_scroll_status_changed(&scroll_id, &scroll.status).await;

        // Re-export .scroll file
        self.scroll_exporter.export_scroll_quiet(&scroll_id).await;

        Ok(json!({
            "scroll_id": scroll_id,
            "status": "updated",
            "message": "Scroll updated. Changes synced to linked external systems.",
        }))
    }
}

// ---------------------------------------------------------------------------
// update_task
// ---------------------------------------------------------------------------

pub struct UpdateTaskHandler {
    pub sqlite: Arc<SqliteDb>,
    pub sync_bridge: Arc<SyncBridge>,
    pub scroll_exporter: Arc<ScrollExporter>,
}

#[async_trait::async_trait]
impl ToolHandler for UpdateTaskHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;
        let task_id = require_non_empty(&input, "task_id")?;

        // Verify scroll and task exist
        let scroll = self.sqlite.get_scroll(&scroll_id).await
            .map_err(|e| anyhow::anyhow!("Scroll '{}' not found: {}", scroll_id, e))?;
        let _task = self.sqlite.get_task_by_id(&task_id).await
            .map_err(|e| anyhow::anyhow!("Task '{}' not found: {}", task_id, e))?;

        let description = input.get("description").and_then(|v| v.as_str());
        let target_file = input.get("target_file").and_then(|v| v.as_str());
        let task_type = input.get("task_type")
            .and_then(|v| v.as_str())
            .map(|s| match s {
                "Modify" => crate::types::TaskType::Modify,
                "Delete" => crate::types::TaskType::Delete,
                "Test" => crate::types::TaskType::Test,
                _ => crate::types::TaskType::Create,
            });
        let generated_code: Option<Option<&str>> = if input.get("generated_code").is_some() {
            Some(input["generated_code"].as_str())
        } else {
            None
        };
        let dependencies: Option<Vec<String>> = input.get("dependencies")
            .and_then(|v| v.as_array())
            .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect());

        self.sqlite.update_task_fields(
            &task_id,
            description,
            target_file,
            task_type.as_ref(),
            generated_code,
            dependencies.as_deref(),
        ).await?;

        // Trigger sync to external systems
        use crate::types::ScrollEventListener;
        self.sync_bridge.on_scroll_status_changed(&scroll_id, &scroll.status).await;

        // Re-export .scroll file
        self.scroll_exporter.export_scroll_quiet(&scroll_id).await;

        Ok(json!({
            "scroll_id": scroll_id,
            "task_id": task_id,
            "status": "updated",
            "message": "Task updated. Changes synced to linked external systems.",
        }))
    }
}

// ---------------------------------------------------------------------------
// remove_tasks
// ---------------------------------------------------------------------------

pub struct RemoveTasksHandler {
    pub sqlite: Arc<SqliteDb>,
    pub sync_bridge: Arc<SyncBridge>,
    pub scroll_exporter: Arc<ScrollExporter>,
}

#[async_trait::async_trait]
impl ToolHandler for RemoveTasksHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        let task_ids: Vec<String> = input["task_ids"]
            .as_array()
            .ok_or_else(|| anyhow::anyhow!("'task_ids' must be an array"))?
            .iter()
            .filter_map(|v| v.as_str().map(String::from))
            .collect();

        if task_ids.is_empty() {
            return Err(anyhow::anyhow!("'task_ids' must not be empty"));
        }

        // Verify scroll exists
        let scroll = self.sqlite.get_scroll(&scroll_id).await
            .map_err(|e| anyhow::anyhow!("Scroll '{}' not found: {}", scroll_id, e))?;

        let deleted = self.sqlite.delete_tasks(&task_ids).await?;

        // Trigger sync to external systems
        use crate::types::ScrollEventListener;
        self.sync_bridge.on_scroll_status_changed(&scroll_id, &scroll.status).await;

        // Re-export .scroll file
        self.scroll_exporter.export_scroll_quiet(&scroll_id).await;

        Ok(json!({
            "scroll_id": scroll_id,
            "removed": deleted,
            "task_ids": task_ids,
            "message": format!("Removed {} task(s). Changes synced to linked external systems.", deleted),
        }))
    }
}

// ---------------------------------------------------------------------------
// reload_scroll
// ---------------------------------------------------------------------------

pub struct ReloadScrollHandler {
    pub scroll_importer: Arc<ScrollImporter>,
    pub sqlite: Arc<SqliteDb>,
    pub sync_bridge: Arc<SyncBridge>,
}

#[async_trait::async_trait]
impl ToolHandler for ReloadScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        let result = self.scroll_importer.import_scroll(&scroll_id).await?;

        // If anything changed, push to linked external systems
        if result.scroll_updated || result.tasks_created > 0
            || result.tasks_updated > 0 || result.tasks_deleted > 0
        {
            let scroll = self.sqlite.get_scroll(&scroll_id).await?;
            self.sync_bridge.on_scroll_status_changed(&scroll_id, &scroll.status).await;
        }

        Ok(json!({
            "scroll_id": scroll_id,
            "scroll_updated": result.scroll_updated,
            "tasks_created": result.tasks_created,
            "tasks_updated": result.tasks_updated,
            "tasks_deleted": result.tasks_deleted,
            "message": if result.scroll_updated || result.tasks_created > 0
                || result.tasks_updated > 0 || result.tasks_deleted > 0 {
                "Scroll reloaded from disk — changes imported into SQLite."
            } else {
                "Scroll file matches SQLite — no changes needed."
            },
        }))
    }
}

// ---------------------------------------------------------------------------
// execute_scroll
// ---------------------------------------------------------------------------

pub struct ExecuteScrollHandler {
    pub aragorn: Arc<Aragorn>,
    pub autoforge: Arc<crate::autoforge::workflow::AutoForge>,
    pub sqlite: Arc<SqliteDb>,
    pub telemetry: Arc<TelemetryTracker>,
    pub scroll_exporter: Arc<ScrollExporter>,
    pub scroll_importer: Arc<ScrollImporter>,
    pub gandalf: Arc<Gandalf>,
    pub elrond: Arc<Elrond>,
    pub scan_stats: Arc<tokio::sync::RwLock<ScanStats>>,
    pub workspace_root: PathBuf,
    pub gimli: Arc<crate::agents::gimli::Gimli>,
    pub morpheus: Arc<Morpheus>,
    pub trinity: Arc<Trinity>,
    pub legolas: Arc<Legolas>,
}

#[async_trait::async_trait]
impl ToolHandler for ExecuteScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        // Parse optional prefetch protocol fields.
        let completions: Vec<PrefetchCompletion> = match input.get("completions") {
            Some(v) => serde_json::from_value(v.clone()).unwrap_or_else(|e| {
                tracing::warn!(error = %e, "Failed to parse prefetch completions, treating as empty");
                vec![]
            }),
            None => vec![],
        };
        let prefetch_round: Option<u8> = input
            .get("prefetch_round")
            .and_then(|v| v.as_u64())
            .map(|v| v as u8);

        // Auto-reload from disk if the .scroll file has been edited
        match self.scroll_importer.sync_if_needed(&scroll_id).await {
            Ok(true) => tracing::info!(scroll_id = %scroll_id, "Auto-reloaded scroll from disk before execution"),
            Ok(false) => {}
            Err(e) => tracing::warn!(scroll_id = %scroll_id, error = %e, "Failed to auto-reload scroll (continuing with DB state)"),
        }

        // Guard: refuse to execute a scroll with no tasks.
        let tasks = self.sqlite.get_tasks_for_scroll(&scroll_id).await?;
        if tasks.is_empty() {
            return Err(anyhow::anyhow!(
                "Scroll '{}' has no tasks. Call `add_tasks` first to decompose \
                 your intent into file-level tasks with generated code.",
                scroll_id
            ));
        }

        // FAST PATH: sampling available → existing flow unchanged.
        if self.gimli.is_llm_available() {
            return self.execute_with_sampling(&scroll_id, &tasks).await;
        }

        // PREFETCH PATH: LLM unavailable — multi-round protocol.
        match (prefetch_round, completions.is_empty()) {
            // Initial call — no completions yet → determine first needed round.
            (None, true) => self.prefetch_determine_round(&scroll_id).await,

            // Round 1 completions received → process Gandalf, then advance.
            (Some(1), false) => {
                self.process_round1(&scroll_id, &completions).await?;
                self.prefetch_determine_round(&scroll_id).await
            }

            // Round 2 completions received → process Gimli, run server-side, advance.
            (Some(2), false) => {
                self.process_round2(&scroll_id, &completions).await?;
                self.execute_server_side(&scroll_id).await?;
                self.prefetch_determine_round(&scroll_id).await
            }

            // Round 3 completions received → process Trinity+Legolas, finalize.
            (Some(3), false) => {
                self.process_round3(&scroll_id, &completions).await?;
                self.finalize(&scroll_id).await
            }

            // Fallback: determine next round from DB state.
            _ => self.prefetch_determine_round(&scroll_id).await,
        }
    }
}

impl ExecuteScrollHandler {
    // -----------------------------------------------------------------------
    // Fast path: LLM available (existing flow, zero behavior change)
    // -----------------------------------------------------------------------

    async fn execute_with_sampling(
        &self,
        scroll_id: &str,
        tasks: &[crate::types::Task],
    ) -> Result<Value> {
        // Gandalf LLM task augmentation.
        let scroll = self.sqlite.get_scroll(scroll_id).await?;
        let augmentation = match self.gandalf.verify_and_augment_tasks(&scroll, tasks).await {
            Ok(aug) => {
                self.apply_augmentation(scroll_id, &aug).await;
                Some(aug)
            }
            Err(e) => {
                tracing::warn!(error = %e, "Gandalf augmentation failed (continuing without)");
                None
            }
        };

        // Phase detection: check if tasks need code and LLM is unavailable.
        // (After Gandalf, LLM status is definitive.)
        let fresh_tasks = self.sqlite.get_tasks_for_scroll(scroll_id).await?;
        let tasks_without_code: Vec<&crate::types::Task> = fresh_tasks.iter()
            .filter(|t| t.generated_code.is_none() && t.task_type != crate::types::TaskType::Delete)
            .collect();

        if !tasks_without_code.is_empty() && !self.gimli.is_llm_available() {
            // LLM became unavailable after Gandalf probe — fall back to prefetch.
            return self.prefetch_determine_round(scroll_id).await;
        }

        // Morpheus pre-execution validation (types + breaking changes, 30s timeout).
        let morpheus_result = tokio::time::timeout(
            std::time::Duration::from_secs(30),
            self.morpheus.validate_scroll(scroll_id),
        ).await;
        if let Ok(Ok(report)) = morpheus_result {
            if !report.is_valid {
                let has_errors = report.type_errors.iter()
                    .chain(report.breaking_changes.iter())
                    .any(|r| !r.passed && r.severity == crate::types::Severity::Error);

                if has_errors {
                    self.scroll_exporter.export_scroll_quiet(scroll_id).await;
                    return Ok(json!({
                        "scroll_id": scroll_id,
                        "status": "validation_failed",
                        "validation_report": serde_json::to_value(&report)?,
                        "next_step": "Fix validation errors, then call execute_scroll again.",
                    }));
                }
            }
        }

        let start = std::time::Instant::now();
        let result = self.aragorn.execute_scroll(scroll_id).await?;
        let duration_secs = start.elapsed().as_secs();

        if let Err(e) = self.telemetry.track_scroll_executed(ScrollExecutedEvent {
            scroll_id: scroll_id.to_string(),
            duration_secs,
            tasks_completed: result.completed_tasks.len(),
            tasks_failed: result.failed_tasks.len(),
        }) {
            tracing::debug!(error = %e, "Failed to track scroll execution telemetry");
        }

        // Run AutoForge batch workflow if configured on this scroll.
        let scroll = self.sqlite.get_scroll(scroll_id).await?;
        let batch_report = if scroll.metadata.auto_forge_config.is_some() {
            match self.autoforge.batch_execute(scroll_id).await {
                Ok(report) => {
                    tracing::info!(
                        scroll_id = %scroll_id,
                        status = ?report.overall_status,
                        "AutoForge batch workflow completed"
                    );
                    Some(report)
                }
                Err(e) => {
                    tracing::warn!(
                        scroll_id = %scroll_id,
                        error = %e,
                        "AutoForge batch workflow failed (Aragorn results still available)"
                    );
                    None
                }
            }
        } else {
            None
        };

        // Post-execute re-indexing.
        self.reindex_affected_files(scroll_id).await;

        // Re-export .scroll file with updated status and agent activities
        self.scroll_exporter.export_scroll_quiet(scroll_id).await;

        let mut response = serde_json::to_value(&result)?;
        if let Some(report) = &batch_report {
            response["autoforge_report"] = serde_json::to_value(report)?;
        }
        if let Some(ref aug) = augmentation {
            if !aug.warnings.is_empty() || !aug.suggestions.is_empty() {
                response["gandalf_augmentation"] = serde_json::to_value(aug)?;
            }
        }

        // Code review errors from batch analysis → pending_code_review
        if let Some(ref report) = batch_report {
            let error_issues: Vec<&crate::types::AnalysisIssue> = report.analysis.issues.iter()
                .filter(|i| i.severity == crate::types::Severity::Error)
                .collect();
            if !error_issues.is_empty() {
                let issue_json: Vec<serde_json::Value> = error_issues.iter().map(|i| {
                    json!({
                        "file_path": i.file_path,
                        "line": i.line,
                        "severity": "Error",
                        "message": i.message,
                    })
                }).collect();
                response["pending_code_review"] = json!({
                    "message": format!(
                        "Code review found {} error(s). Fix via update_task, then execute_scroll again.",
                        error_issues.len()
                    ),
                    "issues": issue_json,
                    "error_count": error_issues.len(),
                });
            }
        }

        // Completion signal — next_step guidance based on outcome.
        let has_failures = !result.failed_tasks.is_empty();
        let has_autoforge_issues = batch_report.as_ref()
            .map(|r| r.overall_status != crate::types::AutoForgeStatus::AllPassed)
            .unwrap_or(false);
        let has_code_review_errors = batch_report.as_ref()
            .map(|r| r.analysis.issues.iter()
                .any(|i| i.severity == crate::types::Severity::Error))
            .unwrap_or(false);

        let next_step = if has_failures {
            "Execution completed with failures. Review failed_tasks for details. \
             Fix the issues and re-execute, or use update_task to adjust individual tasks."
        } else if has_code_review_errors {
            "Code applied but code review found errors. Review pending_code_review for details. \
             Fix via update_task, then execute_scroll again."
        } else if has_autoforge_issues {
            "Code applied but AutoForge verification flagged issues. \
             Review the autoforge_report for details."
        } else {
            "Scroll execution completed successfully. All tasks are done. \
             Call `commit_scroll` to commit changes to git."
        };
        response["next_step"] = json!(next_step);

        // Suggested follow-ups from Gandalf augmentation.
        let mut follow_ups = Vec::new();
        if let Some(ref aug) = augmentation {
            for s in &aug.suggestions {
                follow_ups.push(json!({
                    "title": s.description,
                    "target_file": s.target_file,
                    "task_type": s.task_type,
                }));
            }
        }
        response["suggested_follow_ups"] = json!(follow_ups);

        // Knowledge context summary from agent activities.
        let activities = self.sqlite.get_agent_activities_for_scroll(scroll_id).await
            .unwrap_or_default();
        let gandalf_activities: Vec<&crate::types::AgentActivity> = activities.iter()
            .filter(|a| a.agent_name == "Gandalf" && !a.detail.is_empty())
            .collect();
        if !gandalf_activities.is_empty() {
            let summary: Vec<String> = gandalf_activities.iter()
                .map(|a| a.detail.clone())
                .collect();
            response["knowledge_context"] = json!(summary.join("; "));
        }

        Ok(response)
    }

    // -----------------------------------------------------------------------
    // Prefetch path: multi-round protocol
    // -----------------------------------------------------------------------

    /// Inspect DB state and return the first needed round's prompts,
    /// or the final result if all rounds are done.
    async fn prefetch_determine_round(&self, scroll_id: &str) -> Result<Value> {
        let scroll = self.sqlite.get_scroll(scroll_id).await?;
        let tasks = self.sqlite.get_tasks_for_scroll(scroll_id).await?;

        // Check if Gandalf augmentation has already run for this scroll.
        let activities = self.sqlite.get_agent_activities_for_scroll(scroll_id).await
            .unwrap_or_default();
        let gandalf_done = activities.iter().any(|a| {
            a.agent_name == "Gandalf"
                && a.action == "verify_and_augment"
                && a.status == crate::types::AgentActivityStatus::Completed
        });

        // Round 1: Gandalf augmentation needed?
        if !gandalf_done {
            let prompts = self.gandalf.prepare_augmentation_prompts(&scroll, &tasks);
            if !prompts.is_empty() {
                self.scroll_exporter.export_scroll_quiet(scroll_id).await;
                return Ok(build_prefetch_response(scroll_id, 1, prompts, vec![]));
            }
        }

        // Round 2: tasks needing code generation?
        let tasks_without_code: Vec<&crate::types::Task> = tasks.iter()
            .filter(|t| t.generated_code.is_none() && t.task_type != crate::types::TaskType::Delete)
            .collect();

        if !tasks_without_code.is_empty() {
            let prompts = self.prefetch_round2(scroll_id, &tasks_without_code).await?;
            if !prompts.is_empty() {
                self.scroll_exporter.export_scroll_quiet(scroll_id).await;
                return Ok(build_prefetch_response(
                    scroll_id,
                    2,
                    prompts,
                    if gandalf_done {
                        vec!["Gandalf augmentation already completed".into()]
                    } else {
                        vec![]
                    },
                ));
            }
        }

        // Check if Aragorn has already executed (i.e. server-side work done).
        let aragorn_done = activities.iter().any(|a| {
            a.agent_name == "Aragorn" && a.action == "execute_scroll"
        });

        // If server-side work hasn't been done yet and all tasks have code, do it now.
        if !aragorn_done && tasks_without_code.is_empty() {
            self.execute_server_side(scroll_id).await?;
        }

        // Re-read activities after possible server-side execution (Aragorn
        // records a new activity that the stale `activities` vec doesn't have).
        let activities = self.sqlite.get_agent_activities_for_scroll(scroll_id).await
            .unwrap_or_default();

        // Round 3: Trinity test gen + Legolas review needed?
        // Only if there are files with generated code to test/review.
        let tasks_with_code: Vec<&crate::types::Task> = tasks.iter()
            .filter(|t| t.generated_code.is_some())
            .collect();

        if !tasks_with_code.is_empty() {
            // Check if Trinity/Legolas have completed (not just started).
            // Activities are recorded as Running when prompts are sent, and
            // Completed after completions are processed.  If we only find
            // Running activities, the host may have crashed — re-send prompts.
            let trinity_done = activities.iter().any(|a| {
                a.agent_name == "Trinity"
                    && a.action == "prefetch_tests"
                    && a.status == crate::types::AgentActivityStatus::Completed
            });
            let legolas_done = activities.iter().any(|a| {
                a.agent_name == "Legolas"
                    && a.action == "prefetch_review"
                    && a.status == crate::types::AgentActivityStatus::Completed
            });

            if !trinity_done || !legolas_done {
                let prompts = self.prefetch_round3(scroll_id, &tasks_with_code).await?;
                if !prompts.is_empty() {
                    let mut server_work = vec![];
                    if aragorn_done || tasks_without_code.is_empty() {
                        server_work.push("Aragorn applied code changes".into());
                        server_work.push("Morpheus validation completed".into());
                    }
                    self.scroll_exporter.export_scroll_quiet(scroll_id).await;
                    return Ok(build_prefetch_response(scroll_id, 3, prompts, server_work));
                }
            }
        }

        // All rounds done — finalize.
        self.finalize(scroll_id).await
    }

    /// Build Gimli code-generation prompts for tasks without code.
    ///
    /// Grouped tasks are combined into a single prompt per group to give the
    /// LLM cross-file awareness. Ungrouped tasks keep individual prompts.
    async fn prefetch_round2(
        &self,
        scroll_id: &str,
        tasks_without_code: &[&crate::types::Task],
    ) -> Result<Vec<crate::mcp::prefetch::PrefetchPrompt>> {
        let _ = scroll_id;

        // Partition tasks by group_label (BTreeMap for deterministic ordering).
        let mut grouped: std::collections::BTreeMap<String, Vec<&crate::types::Task>> =
            std::collections::BTreeMap::new();
        let mut ungrouped: Vec<&crate::types::Task> = Vec::new();

        for task in tasks_without_code {
            if let Some(ref label) = task.group_label {
                grouped.entry(label.clone()).or_default().push(task);
            } else {
                ungrouped.push(task);
            }
        }

        let mut prompts = Vec::new();

        // Build combined prompts for grouped tasks.
        for (label, group_tasks) in &grouped {
            if group_tasks.len() == 1 {
                // Single-task group — treat as ungrouped.
                ungrouped.push(group_tasks[0]);
                continue;
            }

            // Build a combined prompt with group context for all tasks in the group.
            let combined_system = format!(
                "You are an expert programmer working inside the FURNACE code generation system.\n\
                 You are generating code for {} tasks in group \"{}\" simultaneously.\n\
                 For EACH task, output the complete file content inside its own fenced code block.\n\
                 Output exactly {} fenced code blocks, one per task, in the same order as given.\n\
                 Ensure consistency across all files in the group (imports, types, interfaces).\n\
                 Do NOT include explanations outside the code blocks.\n\
                 IMPORTANT: Respond with code directly. Do not ask questions or seek clarification.\n",
                group_tasks.len(), label, group_tasks.len()
            );

            let mut combined_user = String::new();
            for (i, task) in group_tasks.iter().enumerate() {
                let context = match self.gandalf.build_constraint_context(task).await {
                    Ok(ctx) => ctx,
                    Err(_) => crate::agents::gandalf::ConstraintContext {
                        direct_constraints: vec![],
                        inherited_constraints: vec![],
                        pattern_examples: vec![],
                        related_types: vec![],
                        dependency_context: vec![],
                        summary: String::new(),
                        recipe_context: None,
                    },
                };
                let siblings: Vec<&crate::types::Task> = group_tasks.iter()
                    .filter(|t| t.id != task.id)
                    .copied()
                    .collect();
                let task_prompt = self.gimli.build_prompt_with_group_context(task, &context, &siblings);
                combined_user.push_str(&format!(
                    "\n---\n## Task {} of {} (ID: {})\nTarget: {}\n\n{}\n",
                    i + 1, group_tasks.len(), task.id, task.target_file, task_prompt,
                ));
            }

            // Use first task's ID for the prompt ID so process_round2 can map it back.
            let prompt_id = format!("gimli-group-{}", label);
            prompts.push(crate::mcp::prefetch::PrefetchPrompt {
                id: prompt_id,
                agent: "Gimli".into(),
                task_id: None,
                system_prompt: combined_system,
                user_prompt: combined_user,
                description: format!(
                    "Generate code for {} tasks in group \"{}\"",
                    group_tasks.len(), label
                ),
            });
        }

        // Build individual prompts for ungrouped tasks.
        for task in ungrouped {
            let context = match self.gandalf.build_constraint_context(task).await {
                Ok(ctx) => ctx,
                Err(_) => crate::agents::gandalf::ConstraintContext {
                    direct_constraints: vec![],
                    inherited_constraints: vec![],
                    pattern_examples: vec![],
                    related_types: vec![],
                    dependency_context: vec![],
                    summary: String::new(),
                    recipe_context: None,
                },
            };
            match self.gimli.prepare_generation_spec(task, &context).await {
                Ok(spec) => {
                    prompts.push(crate::agents::gimli::Gimli::generation_spec_to_prompt(&spec));
                }
                Err(e) => {
                    tracing::warn!(task_id = %task.id, error = %e, "Failed to prepare generation spec");
                }
            }
        }

        Ok(prompts)
    }

    /// Build Trinity test-gen + Legolas review prompts for tasks with code.
    async fn prefetch_round3(
        &self,
        scroll_id: &str,
        tasks_with_code: &[&crate::types::Task],
    ) -> Result<Vec<crate::mcp::prefetch::PrefetchPrompt>> {
        let mut prompts = Vec::new();

        // Trinity: one prompt per task with code.
        let tasks_and_code: Vec<(&crate::types::Task, &str)> = tasks_with_code
            .iter()
            .filter_map(|t| {
                t.generated_code.as_deref().map(|code| (*t, code))
            })
            .collect();

        if !tasks_and_code.is_empty() {
            let refs: Vec<(&crate::types::Task, &str)> = tasks_and_code.iter()
                .map(|(t, c)| (*t, *c))
                .collect();
            prompts.extend(self.trinity.prepare_test_prompts(&refs));
        }

        // Legolas: one prompt per file with generated code.
        let files: Vec<(&str, &str)> = tasks_with_code
            .iter()
            .filter_map(|t| {
                t.generated_code.as_deref().map(|code| (t.target_file.as_str(), code))
            })
            .collect();

        if !files.is_empty() {
            prompts.extend(self.legolas.prepare_review_prompts(&files));
        }

        // Record that we prepared round 3 prompts.
        if !prompts.is_empty() {
            use crate::types::{AgentActivity, AgentActivityStatus};
            let ts = chrono::Utc::now();
            let _ = self.sqlite.record_agent_activity(&AgentActivity {
                id: format!("act-trinity-prefetch-{}-{}", scroll_id, ts.timestamp_millis()),
                scroll_id: scroll_id.to_string(),
                task_id: String::new(),
                agent_name: "Trinity".into(),
                action: "prefetch_tests".into(),
                detail: format!("{} test prompts", tasks_and_code.len()),
                started_at: ts,
                completed_at: None,
                status: AgentActivityStatus::Running,
            }).await;
            let _ = self.sqlite.record_agent_activity(&AgentActivity {
                id: format!("act-legolas-prefetch-{}-{}", scroll_id, ts.timestamp_millis()),
                scroll_id: scroll_id.to_string(),
                task_id: String::new(),
                agent_name: "Legolas".into(),
                action: "prefetch_review".into(),
                detail: format!("{} review prompts", files.len()),
                started_at: ts,
                completed_at: None,
                status: AgentActivityStatus::Running,
            }).await;
        }

        Ok(prompts)
    }

    // -----------------------------------------------------------------------
    // Process completions
    // -----------------------------------------------------------------------

    /// Process Round 1 (Gandalf augmentation) completions.
    async fn process_round1(
        &self,
        scroll_id: &str,
        completions: &[PrefetchCompletion],
    ) -> Result<()> {
        for completion in completions {
            if !completion.id.starts_with("gandalf-aug-") {
                continue;
            }
            let aug = self.gandalf.parse_augmentation_response(&completion.response);
            self.apply_augmentation(scroll_id, &aug).await;

            // Record activity completion.
            use crate::types::{AgentActivity, AgentActivityStatus};
            let activity_id = format!(
                "act-gandalf-aug-{}-{}", scroll_id, chrono::Utc::now().timestamp_millis()
            );
            let _ = self.sqlite.record_agent_activity(&AgentActivity {
                id: activity_id.clone(),
                scroll_id: scroll_id.to_string(),
                task_id: String::new(),
                agent_name: "Gandalf".into(),
                action: "verify_and_augment".into(),
                detail: format!(
                    "verified={}, {} suggestions, {} warnings (prefetch)",
                    aug.verified, aug.suggestions.len(), aug.warnings.len(),
                ),
                started_at: chrono::Utc::now(),
                completed_at: Some(chrono::Utc::now()),
                status: AgentActivityStatus::Completed,
            }).await;
        }
        Ok(())
    }

    /// Process Round 2 (Gimli code generation) completions.
    async fn process_round2(
        &self,
        scroll_id: &str,
        completions: &[PrefetchCompletion],
    ) -> Result<()> {
        let tasks = self.sqlite.get_tasks_for_scroll(scroll_id).await?;

        for completion in completions {
            if completion.id.starts_with("gimli-gen-") {
                // Individual task prompt: "gimli-gen-TASK-001" → "TASK-001"
                let task_id = completion.id.trim_start_matches("gimli-gen-");
                if let Some(task) = tasks.iter().find(|t| t.id == task_id) {
                    let code = crate::agents::gimli::Gimli::extract_code_block(&completion.response);
                    if let Err(e) = self.sqlite.update_task_code(
                        &task.id,
                        &code,
                        "", // diff will be built by Aragorn at execution time
                    ).await {
                        tracing::warn!(task_id = %task.id, error = %e, "Failed to store generated code");
                    } else {
                        tracing::info!(task_id = %task.id, lines = code.lines().count(), "Gimli prefetch: code stored");
                    }
                }
            } else if completion.id.starts_with("gimli-group-") {
                // Group prompt: parse multiple code blocks, one per task.
                let group_label = completion.id.trim_start_matches("gimli-group-");
                // Only match tasks WITHOUT code — prefetch_round2 only sent codeless tasks.
                let group_tasks: Vec<&crate::types::Task> = tasks.iter()
                    .filter(|t| t.group_label.as_deref() == Some(group_label) && t.generated_code.is_none())
                    .collect();

                let owned_tasks: Vec<crate::types::Task> = group_tasks.iter().map(|t| (*t).clone()).collect();
                let parsed = crate::agents::gimli::Gimli::parse_batch_response(
                    &completion.response,
                    &owned_tasks,
                );

                if let Some(code_map) = parsed {
                    for task in &group_tasks {
                        if let Some(code) = code_map.get(&task.id) {
                            if let Err(e) = self.sqlite.update_task_code(
                                &task.id,
                                code,
                                "",
                            ).await {
                                tracing::warn!(task_id = %task.id, error = %e, "Failed to store group-generated code");
                            } else {
                                tracing::info!(task_id = %task.id, lines = code.lines().count(), "Gimli prefetch (group): code stored");
                            }
                        }
                    }
                } else {
                    tracing::warn!(group = %group_label, "Failed to parse group code generation response");
                }
            }
        }
        Ok(())
    }

    /// Process Round 3 (Trinity tests + Legolas review) completions.
    async fn process_round3(
        &self,
        scroll_id: &str,
        completions: &[PrefetchCompletion],
    ) -> Result<()> {
        use crate::types::AgentActivityStatus;

        for completion in completions {
            if completion.id.starts_with("trinity-test-") {
                let task_id = completion.id.trim_start_matches("trinity-test-");
                let test_code = Trinity::strip_markdown_fences(&completion.response);
                tracing::info!(
                    task_id = %task_id,
                    lines = test_code.lines().count(),
                    "Trinity prefetch: test code received"
                );
                // Store test code as an agent activity detail for downstream use.
                let activity_id = format!(
                    "act-trinity-result-{}-{}", task_id, chrono::Utc::now().timestamp_millis()
                );
                let _ = self.sqlite.record_agent_activity(&crate::types::AgentActivity {
                    id: activity_id,
                    scroll_id: scroll_id.to_string(),
                    task_id: task_id.to_string(),
                    agent_name: "Trinity".into(),
                    action: "generate_tests".into(),
                    detail: format!("{} lines of test code generated (prefetch)", test_code.lines().count()),
                    started_at: chrono::Utc::now(),
                    completed_at: Some(chrono::Utc::now()),
                    status: AgentActivityStatus::Completed,
                }).await;
            } else if completion.id.starts_with("legolas-review-") {
                // Decode file path: '::' → '/' (reverse of prepare_review_prompts encoding).
                let encoded = completion.id.trim_start_matches("legolas-review-");
                let file_path = encoded.replace("::", "/");
                match Legolas::parse_llm_review_issues(&completion.response, &file_path) {
                    Ok(issues) => {
                        tracing::info!(
                            file = %file_path,
                            issues = issues.len(),
                            "Legolas prefetch: review complete"
                        );
                        let activity_id = format!(
                            "act-legolas-result-{}-{}", encoded, chrono::Utc::now().timestamp_millis()
                        );
                        let _ = self.sqlite.record_agent_activity(&crate::types::AgentActivity {
                            id: activity_id,
                            scroll_id: scroll_id.to_string(),
                            task_id: String::new(),
                            agent_name: "Legolas".into(),
                            action: "review_code".into(),
                            detail: format!("{} issues found in {} (prefetch)", issues.len(), file_path),
                            started_at: chrono::Utc::now(),
                            completed_at: Some(chrono::Utc::now()),
                            status: AgentActivityStatus::Completed,
                        }).await;
                    }
                    Err(e) => {
                        tracing::warn!(file = %file_path, error = %e, "Legolas prefetch: failed to parse review");
                    }
                }
            }
        }

        // Mark the prefetch activities as completed.
        let activities = self.sqlite.get_agent_activities_for_scroll(scroll_id).await
            .unwrap_or_default();
        for act in &activities {
            if (act.agent_name == "Trinity" && act.action == "prefetch_tests")
                || (act.agent_name == "Legolas" && act.action == "prefetch_review")
            {
                if act.status == crate::types::AgentActivityStatus::Running {
                    let _ = self.sqlite.complete_agent_activity(
                        &act.id,
                        &AgentActivityStatus::Completed,
                        "prefetch completions processed",
                    ).await;
                }
            }
        }

        Ok(())
    }

    // -----------------------------------------------------------------------
    // Server-side execution (no LLM needed)
    // -----------------------------------------------------------------------

    /// Run Aragorn (apply code) + Morpheus (validation) + Elrond (re-index).
    async fn execute_server_side(&self, scroll_id: &str) -> Result<()> {
        // Morpheus pre-execution validation.
        let morpheus_result = tokio::time::timeout(
            std::time::Duration::from_secs(30),
            self.morpheus.validate_scroll(scroll_id),
        ).await;
        if let Ok(Ok(report)) = morpheus_result {
            if !report.is_valid {
                let has_errors = report.type_errors.iter()
                    .chain(report.breaking_changes.iter())
                    .any(|r| !r.passed && r.severity == crate::types::Severity::Error);
                if has_errors {
                    tracing::warn!(
                        scroll_id = %scroll_id,
                        "Morpheus validation found errors during prefetch server-side execution"
                    );
                    // Continue anyway — errors will surface in the final response.
                }
            }
        }

        // Aragorn: apply code to filesystem.
        let start = std::time::Instant::now();
        let result = self.aragorn.execute_scroll(scroll_id).await?;
        let duration_secs = start.elapsed().as_secs();

        if let Err(e) = self.telemetry.track_scroll_executed(ScrollExecutedEvent {
            scroll_id: scroll_id.to_string(),
            duration_secs,
            tasks_completed: result.completed_tasks.len(),
            tasks_failed: result.failed_tasks.len(),
        }) {
            tracing::debug!(error = %e, "Failed to track scroll execution telemetry");
        }

        // Record Aragorn activity so prefetch_determine_round can detect completion.
        use crate::types::{AgentActivity, AgentActivityStatus};
        let activity_id = format!(
            "act-aragorn-exec-{}-{}", scroll_id, chrono::Utc::now().timestamp_millis()
        );
        let _ = self.sqlite.record_agent_activity(&AgentActivity {
            id: activity_id.clone(),
            scroll_id: scroll_id.to_string(),
            task_id: String::new(),
            agent_name: "Aragorn".into(),
            action: "execute_scroll".into(),
            detail: format!(
                "{} completed, {} failed",
                result.completed_tasks.len(),
                result.failed_tasks.len(),
            ),
            started_at: chrono::Utc::now(),
            completed_at: Some(chrono::Utc::now()),
            status: AgentActivityStatus::Completed,
        }).await;

        // Elrond: re-index affected files.
        self.reindex_affected_files(scroll_id).await;

        // Export scroll file to persist task status changes.
        self.scroll_exporter.export_scroll_quiet(scroll_id).await;

        Ok(())
    }

    /// Finalize: build the final execution report.
    async fn finalize(&self, scroll_id: &str) -> Result<Value> {
        // Re-export .scroll file with updated status and agent activities.
        self.scroll_exporter.export_scroll_quiet(scroll_id).await;

        // Gather execution result from Aragorn (may have already run).
        let tasks = self.sqlite.get_tasks_for_scroll(scroll_id).await?;
        let completed: Vec<String> = tasks.iter()
            .filter(|t| t.status == crate::types::TaskStatus::Completed)
            .map(|t| t.id.clone())
            .collect();
        let failed: Vec<String> = tasks.iter()
            .filter(|t| t.status == crate::types::TaskStatus::Failed)
            .map(|t| t.id.clone())
            .collect();

        // Knowledge context from agent activities.
        let activities = self.sqlite.get_agent_activities_for_scroll(scroll_id).await
            .unwrap_or_default();
        let gandalf_summary: Vec<String> = activities.iter()
            .filter(|a| a.agent_name == "Gandalf" && !a.detail.is_empty())
            .map(|a| a.detail.clone())
            .collect();
        let trinity_summary: Vec<String> = activities.iter()
            .filter(|a| a.agent_name == "Trinity" && a.action == "generate_tests" && !a.detail.is_empty())
            .map(|a| a.detail.clone())
            .collect();
        let legolas_summary: Vec<String> = activities.iter()
            .filter(|a| a.agent_name == "Legolas" && a.action == "review_code" && !a.detail.is_empty())
            .map(|a| a.detail.clone())
            .collect();

        let has_failures = !failed.is_empty();
        let next_step = if has_failures {
            "Execution completed with failures. Review failed_tasks for details. \
             Fix the issues and re-execute, or use update_task to adjust individual tasks."
        } else {
            "Scroll execution completed successfully via prefetch protocol. All tasks are done. \
             Call `commit_scroll` to commit changes to git."
        };

        let mut response = json!({
            "scroll_id": scroll_id,
            "status": "completed",
            "completed_tasks": completed,
            "failed_tasks": failed,
            "prefetch_protocol": true,
            "next_step": next_step,
        });

        if !gandalf_summary.is_empty() {
            response["knowledge_context"] = json!(gandalf_summary.join("; "));
        }
        if !trinity_summary.is_empty() {
            response["test_generation"] = json!(trinity_summary);
        }
        if !legolas_summary.is_empty() {
            response["code_review"] = json!(legolas_summary);
        }

        Ok(response)
    }

    // -----------------------------------------------------------------------
    // Shared helpers
    // -----------------------------------------------------------------------

    /// Create suggested tasks from a Gandalf augmentation result.
    async fn apply_augmentation(
        &self,
        scroll_id: &str,
        aug: &crate::agents::gandalf::TaskAugmentation,
    ) {
        static AUG_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);

        for suggestion in &aug.suggestions {
            let task_type = match suggestion.task_type.as_str() {
                "Create" => crate::types::TaskType::Create,
                "Modify" => crate::types::TaskType::Modify,
                "Test" => crate::types::TaskType::Test,
                _ => crate::types::TaskType::Create,
            };
            let counter = AUG_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
            let new_task = crate::types::Task {
                id: format!("TASK-aug-{}-{}", chrono::Utc::now().timestamp_millis(), counter),
                scroll_id: scroll_id.to_string(),
                description: suggestion.description.clone(),
                target_file: suggestion.target_file.clone(),
                task_type,
                status: crate::types::TaskStatus::Pending,
                dependencies: vec![],
                generated_code: None,
                diff: None,
                created_at: chrono::Utc::now(),
                group_label: suggestion.group.clone(),
            };
            if let Err(e) = self.sqlite.create_task(&new_task).await {
                tracing::warn!(error = %e, "Failed to create augmented task");
            } else {
                tracing::info!(task_id = %new_task.id, "Gandalf augmented task created");
            }
        }

        // Apply groupings to existing tasks.
        for grouping in &aug.groupings {
            for task_id in &grouping.task_ids {
                if let Err(e) = self.sqlite.update_task_group_label(task_id, &grouping.group_label).await {
                    tracing::warn!(task_id = %task_id, error = %e, "Failed to set group label");
                }
            }
        }
    }

    /// Re-index affected files after execution.
    async fn reindex_affected_files(&self, scroll_id: &str) {
        let reindex_tasks = self.sqlite.get_tasks_for_scroll(scroll_id).await.unwrap_or_default();
        let affected_paths: Vec<std::path::PathBuf> = reindex_tasks
            .iter()
            .map(|t| self.workspace_root.join(&t.target_file))
            .collect();

        if affected_paths.is_empty() {
            return;
        }

        use crate::types::{AgentActivity, AgentActivityStatus};
        let elrond_activity_id = format!(
            "act-elrond-reindex-{}-{}", scroll_id, chrono::Utc::now().timestamp_millis()
        );
        let _ = self.sqlite.record_agent_activity(&AgentActivity {
            id: elrond_activity_id.clone(),
            scroll_id: scroll_id.to_string(),
            task_id: String::new(),
            agent_name: "Elrond".into(),
            action: "reindex_files".into(),
            detail: format!("{} files", affected_paths.len()),
            started_at: chrono::Utc::now(),
            completed_at: None,
            status: AgentActivityStatus::Running,
        }).await;

        match self.elrond.index_files(&affected_paths).await {
            Ok(reindex_stats) => {
                tracing::info!(
                    scroll_id = %scroll_id,
                    indexed = reindex_stats.files_indexed,
                    "Post-execute re-indexing complete"
                );
                let _ = self.sqlite.complete_agent_activity(
                    &elrond_activity_id,
                    &AgentActivityStatus::Completed,
                    &format!("{} files re-indexed", reindex_stats.files_indexed),
                ).await;
                let mut stats = self.scan_stats.write().await;
                stats.files_indexed += reindex_stats.files_indexed;
            }
            Err(e) => {
                tracing::warn!(
                    scroll_id = %scroll_id,
                    error = %e,
                    "Post-execute re-indexing failed (non-fatal)"
                );
                let _ = self.sqlite.complete_agent_activity(
                    &elrond_activity_id,
                    &AgentActivityStatus::Failed,
                    &format!("{}", e),
                ).await;
            }
        }
    }
}

// ---------------------------------------------------------------------------
// validate_scroll
// ---------------------------------------------------------------------------

pub struct ValidateScrollHandler {
    pub morpheus: Arc<Morpheus>,
    pub sqlite: Arc<SqliteDb>,
    pub scroll_importer: Arc<ScrollImporter>,
}

#[async_trait::async_trait]
impl ToolHandler for ValidateScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        use crate::types::{AgentActivity, AgentActivityStatus};

        let scroll_id = require_non_empty(&input, "scroll_id")?;

        // Auto-reload from disk if the .scroll file has been edited
        match self.scroll_importer.sync_if_needed(&scroll_id).await {
            Ok(true) => tracing::info!(scroll_id = %scroll_id, "Auto-reloaded scroll from disk before validation"),
            Ok(false) => {}
            Err(e) => tracing::warn!(scroll_id = %scroll_id, error = %e, "Failed to auto-reload scroll (continuing with DB state)"),
        }

        let activity_id = format!("act-morpheus-{}-{}", scroll_id, chrono::Utc::now().timestamp_millis());
        let _ = self.sqlite.record_agent_activity(&AgentActivity {
            id: activity_id.clone(),
            scroll_id: scroll_id.clone(),
            task_id: String::new(),
            agent_name: "Morpheus".into(),
            action: "validate_scroll".into(),
            detail: String::new(),
            started_at: chrono::Utc::now(),
            completed_at: None,
            status: AgentActivityStatus::Running,
        }).await;

        match self.morpheus.validate_scroll(&scroll_id).await {
            Ok(report) => {
                let _ = self.sqlite.complete_agent_activity(
                    &activity_id,
                    &AgentActivityStatus::Completed,
                    &format!("valid={}", report.is_valid),
                ).await;
                Ok(serde_json::to_value(&report)?)
            }
            Err(e) => {
                let _ = self.sqlite.complete_agent_activity(
                    &activity_id,
                    &AgentActivityStatus::Failed,
                    &format!("{}", e),
                ).await;
                Err(e)
            }
        }
    }
}

// ---------------------------------------------------------------------------
// list_scrolls
// ---------------------------------------------------------------------------

pub struct ListScrollsHandler {
    pub sqlite: Arc<SqliteDb>,
    pub sync_bridge: Arc<SyncBridge>,
    pub knowledge_manager: Option<Arc<KnowledgeManager>>,
}

#[async_trait::async_trait]
impl ToolHandler for ListScrollsHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        // Parse optional status filter and pass it to the DB query.
        let status_filter = input["status_filter"]
            .as_str()
            .and_then(|s| match s {
                "Draft" => Some(ScrollStatus::Draft),
                "Validated" => Some(ScrollStatus::Validated),
                "InProgress" => Some(ScrollStatus::InProgress),
                "Completed" => Some(ScrollStatus::Completed),
                "Failed" => Some(ScrollStatus::Failed),
                "Archived" => Some(ScrollStatus::Archived),
                _ => None,
            });
        let scrolls = self.sqlite.list_scrolls(status_filter.as_ref()).await?;
        let filtered: Vec<&Scroll> = scrolls.iter().collect();

        // Fetch task counts and KB source statuses in one pass.
        let task_counts = self.sqlite.get_task_counts().await.unwrap_or_default();
        let kb_statuses = if let Some(ref km) = self.knowledge_manager {
            km.source_statuses().await
        } else {
            Vec::new()
        };

        let mut scroll_values = Vec::new();
        for s in &filtered {
            let external_links = self.sync_bridge
                .get_all_links(Some(&s.id))
                .await
                .unwrap_or_default()
                .into_iter()
                .map(|(_sid, ext_ref)| format_external_link(&ext_ref))
                .collect::<Vec<_>>();

            let knowledge_sources = extract_kb_sources_for_scroll(s, &kb_statuses);

            let mut scroll_json = json!({
                "id": s.id,
                "title": s.title,
                "status": format!("{:?}", s.status),
                "created_at": s.created_at.to_rfc3339(),
                "task_count": task_counts.get(&s.id).copied().unwrap_or(0),
                "external_links": external_links,
                "knowledge_sources": knowledge_sources,
            });
            if let Some(ref cleanup) = s.cleanup_after {
                scroll_json["cleanup_after"] = json!(cleanup.to_rfc3339());
            }
            scroll_values.push(scroll_json);
        }

        Ok(json!({
            "scrolls": scroll_values,
            "total": filtered.len(),
        }))
    }
}

// ---------------------------------------------------------------------------
// get_scroll
// ---------------------------------------------------------------------------

pub struct GetScrollHandler {
    pub sqlite: Arc<SqliteDb>,
    pub sync_bridge: Arc<SyncBridge>,
    pub knowledge_manager: Option<Arc<KnowledgeManager>>,
}

#[async_trait::async_trait]
impl ToolHandler for GetScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        let scroll = self.sqlite.get_scroll(&scroll_id).await?;
        let tasks = self.sqlite.get_tasks_for_scroll(&scroll_id).await?;
        let agent_activities = self.sqlite.get_agent_activities_for_scroll(&scroll_id).await
            .unwrap_or_default();

        let external_links: Vec<Value> = self.sync_bridge
            .get_all_links(Some(&scroll_id))
            .await
            .unwrap_or_default()
            .into_iter()
            .map(|(_sid, ext_ref)| format_external_link(&ext_ref))
            .collect();

        let kb_statuses = if let Some(ref km) = self.knowledge_manager {
            km.source_statuses().await
        } else {
            Vec::new()
        };
        let knowledge_sources = extract_kb_sources_for_scroll(&scroll, &kb_statuses);

        Ok(json!({
            "scroll": serde_json::to_value(&scroll)?,
            "tasks": serde_json::to_value(&tasks)?,
            "agent_activities": serde_json::to_value(&agent_activities)?,
            "external_links": external_links,
            "knowledge_sources": knowledge_sources,
        }))
    }
}

// ---------------------------------------------------------------------------
// analyze_file
// ---------------------------------------------------------------------------

pub struct AnalyzeFileHandler {
    pub legolas: Arc<Legolas>,
    pub sqlite: Arc<SqliteDb>,
    pub workspace_root: PathBuf,
}

#[async_trait::async_trait]
impl ToolHandler for AnalyzeFileHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        use crate::types::{AgentActivity, AgentActivityStatus};

        let file_path = require_non_empty(&input, "file_path")?;

        // Resolve relative paths against workspace root.
        let full_path = if file_path.starts_with('/') {
            PathBuf::from(&file_path)
        } else {
            self.workspace_root.join(&file_path)
        };

        let activity_id = format!("act-legolas-{}", chrono::Utc::now().timestamp_millis());
        let _ = self.sqlite.record_agent_activity(&AgentActivity {
            id: activity_id.clone(),
            scroll_id: String::new(),
            task_id: String::new(),
            agent_name: "Legolas".into(),
            action: "analyze_file".into(),
            detail: file_path.clone(),
            started_at: chrono::Utc::now(),
            completed_at: None,
            status: AgentActivityStatus::Running,
        }).await;

        let content = tokio::fs::read_to_string(&full_path).await?;
        match self.legolas.analyze_file(&file_path, &content).await {
            Ok(analysis) => {
                let _ = self.sqlite.complete_agent_activity(
                    &activity_id,
                    &AgentActivityStatus::Completed,
                    &format!("analyzed {}", file_path),
                ).await;
                Ok(serde_json::to_value(&analysis)?)
            }
            Err(e) => {
                let _ = self.sqlite.complete_agent_activity(
                    &activity_id,
                    &AgentActivityStatus::Failed,
                    &format!("{}", e),
                ).await;
                Err(e)
            }
        }
    }
}

// ---------------------------------------------------------------------------
// search_feedback
// ---------------------------------------------------------------------------

pub struct SearchFeedbackHandler {
    pub sqlite: Arc<SqliteDb>,
    pub metrics: SearchMetrics,
}

#[async_trait::async_trait]
impl ToolHandler for SearchFeedbackHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let query_id = require_non_empty(&input, "query_id")?;
        let satisfied = input["satisfied"]
            .as_bool()
            .unwrap_or(true);

        self.sqlite
            .update_search_satisfaction(&query_id, satisfied)
            .await?;

        // Trigger re-learning for the affected query type so the adaptive
        // strategy selector benefits from this feedback immediately.
        if let Ok(Some(query_type)) = self.sqlite.get_search_query_type(&query_id).await {
            if let Err(e) = self.metrics.maybe_update_learned_strategy(&query_type).await {
                tracing::warn!(error = %e, "Failed to update learned strategy after feedback");
            }
        }

        Ok(json!({
            "status": "ok",
            "query_id": query_id,
            "satisfied": satisfied,
        }))
    }
}

// ---------------------------------------------------------------------------
// list_recipes
// ---------------------------------------------------------------------------

pub struct ListRecipesHandler {
    pub recipe_manager: Arc<RecipeManager>,
}

#[async_trait::async_trait]
impl ToolHandler for ListRecipesHandler {
    async fn handle(&self, _input: Value) -> Result<Value> {
        let recipes = self.recipe_manager.list_recipes_with_descriptions().await?;
        let entries: Vec<Value> = recipes
            .iter()
            .map(|(name, desc)| json!({"name": name, "description": desc}))
            .collect();
        Ok(json!({
            "recipes": entries,
            "total": entries.len(),
        }))
    }
}

// ---------------------------------------------------------------------------
// render_recipe
// ---------------------------------------------------------------------------

pub struct RenderRecipeHandler {
    pub recipe_manager: Arc<RecipeManager>,
}

#[async_trait::async_trait]
impl ToolHandler for RenderRecipeHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let name = require_non_empty(&input, "name")?;

        let variables: std::collections::HashMap<String, String> = input
            .get("variables")
            .and_then(|v| v.as_object())
            .map(|obj| {
                obj.iter()
                    .map(|(k, v)| (k.clone(), v.as_str().unwrap_or("").to_string()))
                    .collect()
            })
            .unwrap_or_default();

        let rendered = self.recipe_manager.render_recipe(&name, &variables).await?;
        let recipe = self.recipe_manager.load_recipe(&name).await?;

        let variables_used: Vec<&str> = recipe
            .variables
            .iter()
            .map(|v| v.name.as_str())
            .collect();

        Ok(json!({
            "recipe_name": name,
            "rendered": rendered,
            "variables_used": variables_used,
        }))
    }
}

// ---------------------------------------------------------------------------
// configure_sync
// ---------------------------------------------------------------------------

pub struct ConfigureSyncHandler {
    pub sync_bridge: Arc<SyncBridge>,
}

#[async_trait::async_trait]
impl ToolHandler for ConfigureSyncHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let system: ExternalSystem = input["system"]
            .as_str()
            .unwrap_or("")
            .parse()
            .map_err(|e: String| anyhow::anyhow!(e))?;

        let api_token = require_non_empty(&input, "api_token")?;

        let command = input["command"]
            .as_str()
            .unwrap_or("npx")
            .to_string();

        let args: Vec<String> = input["args"]
            .as_array()
            .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect())
            .unwrap_or_default();

        // Store the API token in the OS keyring.
        self.sync_bridge.credentials().store_token(&system, &api_token)?;

        // Build env var for the MCP server (pass the token as an env var).
        let env_key = match &system {
            ExternalSystem::Linear => "LINEAR_API_KEY",
            ExternalSystem::Jira => "JIRA_API_TOKEN",
            ExternalSystem::Notion => "NOTION_API_KEY",
        };

        let config = McpServerConfig {
            system: system.clone(),
            command,
            args,
            env_vars: vec![(env_key.to_string(), api_token)],
        };

        // Register the MCP server connection.
        self.sync_bridge.mcp_client().register(config).await;

        let token_ref = CredentialStore::token_ref_key(&system);
        Ok(json!({
            "status": "configured",
            "system": system.to_string(),
            "token_ref": token_ref,
            "message": format!("External sync configured for {}. Token stored in OS keyring.", system),
        }))
    }
}

// ---------------------------------------------------------------------------
// link_issue
// ---------------------------------------------------------------------------

pub struct LinkIssueHandler {
    pub sync_bridge: Arc<SyncBridge>,
}

#[async_trait::async_trait]
impl ToolHandler for LinkIssueHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        let system: ExternalSystem = input["system"]
            .as_str()
            .unwrap_or("")
            .parse()
            .map_err(|e: String| anyhow::anyhow!(e))?;

        let external_id = require_non_empty(&input, "external_id")?;

        let url = input["url"]
            .as_str()
            .unwrap_or("")
            .to_string();

        let token_ref = CredentialStore::token_ref_key(&system);

        let ext_ref = ExternalRef {
            system: system.clone(),
            external_id: external_id.clone(),
            url: url.clone(),
            last_synced: chrono::Utc::now().timestamp(),
            sync_direction: SyncDirection::default(),
            token_ref: Some(token_ref),
        };

        self.sync_bridge.link_scroll(&scroll_id, &ext_ref).await?;

        Ok(json!({
            "status": "linked",
            "scroll_id": scroll_id,
            "system": system.to_string(),
            "external_id": external_id,
            "url": url,
        }))
    }
}

// ---------------------------------------------------------------------------
// push_scroll
// ---------------------------------------------------------------------------

pub struct PushScrollHandler {
    pub sync_bridge: Arc<SyncBridge>,
}

#[async_trait::async_trait]
impl ToolHandler for PushScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        let system: ExternalSystem = input["system"]
            .as_str()
            .unwrap_or("")
            .parse()
            .map_err(|e: String| anyhow::anyhow!(e))?;

        let result = self.sync_bridge
            .push_to_external(&scroll_id, &system)
            .await?;

        Ok(serde_json::to_value(&result)?)
    }
}

// ---------------------------------------------------------------------------
// sync_status
// ---------------------------------------------------------------------------

pub struct SyncStatusHandler {
    pub sync_bridge: Arc<SyncBridge>,
}

#[async_trait::async_trait]
impl ToolHandler for SyncStatusHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;

        let system: ExternalSystem = input["system"]
            .as_str()
            .unwrap_or("")
            .parse()
            .map_err(|e: String| anyhow::anyhow!(e))?;

        let result = self.sync_bridge
            .sync_status(&scroll_id, &system)
            .await?;

        Ok(serde_json::to_value(&result)?)
    }
}

// ---------------------------------------------------------------------------
// list_connections
// ---------------------------------------------------------------------------

pub struct ListConnectionsHandler {
    pub sync_bridge: Arc<SyncBridge>,
}

#[async_trait::async_trait]
impl ToolHandler for ListConnectionsHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = input.get("scroll_id")
            .and_then(|v| v.as_str())
            .map(|s| s.to_string());

        let links = self.sync_bridge
            .get_all_links(scroll_id.as_deref())
            .await?;

        let connections: Vec<Value> = links
            .iter()
            .map(|(sid, ext_ref)| {
                let last_synced_iso = chrono::DateTime::from_timestamp(ext_ref.last_synced, 0)
                    .map(|dt| dt.to_rfc3339())
                    .unwrap_or_else(|| ext_ref.last_synced.to_string());

                let sync_direction_label = match ext_ref.sync_direction {
                    SyncDirection::Push => "Push only",
                    SyncDirection::Pull => "Pull only",
                    SyncDirection::Bidirectional => "Two-way sync",
                };

                json!({
                    "scroll_id": sid,
                    "system": ext_ref.system.to_string(),
                    "external_id": ext_ref.external_id,
                    "url": ext_ref.url,
                    "last_synced": last_synced_iso,
                    "sync_direction": ext_ref.sync_direction.to_string(),
                    "sync_direction_label": sync_direction_label,
                })
            })
            .collect();

        Ok(json!({
            "connections": connections,
            "total": connections.len(),
        }))
    }
}

// ---------------------------------------------------------------------------
// get_metrics
// ---------------------------------------------------------------------------

pub struct GetMetricsHandler {
    pub telemetry: Arc<TelemetryTracker>,
}

#[async_trait::async_trait]
impl ToolHandler for GetMetricsHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let stats = if let Some(since) = input.get("since_timestamp").and_then(|v| v.as_i64()) {
            // Filter events, then compute stats from the filtered set.
            let events = self.telemetry.get_events_since(since)?;
            let total_events = events.len();
            let mut scrolls_created: usize = 0;
            let mut scrolls_executed: usize = 0;
            let mut searches_performed: usize = 0;
            let mut auto_forge_runs: usize = 0;
            let mut search_latency_sum: u64 = 0;
            let mut search_latency_count: usize = 0;
            let mut earliest_event: Option<i64> = None;
            let mut latest_event: Option<i64> = None;

            for event in &events {
                match earliest_event {
                    None => earliest_event = Some(event.timestamp),
                    Some(e) if event.timestamp < e => earliest_event = Some(event.timestamp),
                    _ => {}
                }
                match latest_event {
                    None => latest_event = Some(event.timestamp),
                    Some(l) if event.timestamp > l => latest_event = Some(event.timestamp),
                    _ => {}
                }
                match event.event_type.as_str() {
                    "scroll_created" => scrolls_created += 1,
                    "scroll_executed" => scrolls_executed += 1,
                    "search" => {
                        searches_performed += 1;
                        if let Some(latency) = event.data.get("latency_ms").and_then(|v| v.as_u64()) {
                            search_latency_sum += latency;
                            search_latency_count += 1;
                        }
                    }
                    "auto_forge" => auto_forge_runs += 1,
                    _ => {}
                }
            }

            let avg_search_latency_ms = if search_latency_count > 0 {
                search_latency_sum as f64 / search_latency_count as f64
            } else {
                0.0
            };

            crate::telemetry::tracker::TelemetryStats {
                total_events,
                scrolls_created,
                scrolls_executed,
                searches_performed,
                auto_forge_runs,
                avg_search_latency_ms,
                earliest_event,
                latest_event,
            }
        } else {
            self.telemetry.get_statistics()?
        };

        Ok(serde_json::to_value(&stats)?)
    }
}

// ---------------------------------------------------------------------------
// configure_kb
// ---------------------------------------------------------------------------

pub struct ConfigureKBHandler {
    pub knowledge_manager: Arc<KnowledgeManager>,
}

#[async_trait::async_trait]
impl ToolHandler for ConfigureKBHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let source_str = require_non_empty(&input, "source")?;
        let command = require_non_empty(&input, "command")?;
        let args: Vec<String> = input["args"]
            .as_array()
            .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect())
            .unwrap_or_default();
        let collection = input.get("collection")
            .and_then(|v| v.as_str())
            .map(String::from);
        let collection_pattern = input.get("collection_pattern")
            .and_then(|v| v.as_str())
            .map(String::from);
        let env_vars: Vec<(String, String)> = input.get("env_vars")
            .and_then(|v| v.as_object())
            .map(|obj| {
                obj.iter()
                    .map(|(k, v)| (k.clone(), v.as_str().unwrap_or("").to_string()))
                    .collect()
            })
            .unwrap_or_default();

        let source: crate::knowledge::KBSource = source_str
            .parse()
            .unwrap_or(crate::knowledge::KBSource::Custom(source_str.clone()));

        let name = source_str.clone();

        let config = crate::knowledge::KBServerConfig {
            source: source.clone(),
            name: name.clone(),
            command,
            args,
            env_vars,
            collection,
            collection_pattern,
        };

        self.knowledge_manager.configure_kb(config).await;

        Ok(json!({
            "status": "configured",
            "source": source.to_string(),
            "name": name,
            "message": format!("Knowledge base '{}' configured successfully.", source),
        }))
    }
}

// ---------------------------------------------------------------------------
// query_kb
// ---------------------------------------------------------------------------

pub struct QueryKBHandler {
    pub knowledge_manager: Arc<KnowledgeManager>,
}

#[async_trait::async_trait]
impl ToolHandler for QueryKBHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let source = require_non_empty(&input, "source")?;
        let query = require_non_empty(&input, "query")?;
        let max_results = input["max_results"].as_u64().unwrap_or(5) as usize;
        let collection = input.get("collection").and_then(|v| v.as_str());
        let collection_pattern = input.get("collection_pattern").and_then(|v| v.as_str());

        let results = self
            .knowledge_manager
            .query_kb(&source, &query, max_results, collection, collection_pattern)
            .await?;

        Ok(serde_json::to_value(&results)?)
    }
}

// ---------------------------------------------------------------------------
// sync_kb
// ---------------------------------------------------------------------------

pub struct SyncKBHandler {
    pub knowledge_manager: Arc<KnowledgeManager>,
}

#[async_trait::async_trait]
impl ToolHandler for SyncKBHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let source = require_non_empty(&input, "source")?;
        let query = require_non_empty(&input, "query")?;
        let max_results = input["max_results"].as_u64().unwrap_or(10) as usize;
        let collection = input.get("collection").and_then(|v| v.as_str());
        let collection_pattern = input.get("collection_pattern").and_then(|v| v.as_str());

        let result = self
            .knowledge_manager
            .sync_kb(&source, &query, max_results, collection, collection_pattern)
            .await?;

        Ok(serde_json::to_value(&result)?)
    }
}

// ---------------------------------------------------------------------------
// ingest_kb
// ---------------------------------------------------------------------------

pub struct IngestKBHandler {
    pub knowledge_manager: Arc<KnowledgeManager>,
}

#[async_trait::async_trait]
impl ToolHandler for IngestKBHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let source = require_non_empty(&input, "source")?;
        let title = require_non_empty(&input, "title")?;
        let content = require_non_empty(&input, "content")?;
        let collection = input.get("collection").and_then(|v| v.as_str());
        let metadata: std::collections::HashMap<String, String> = input
            .get("metadata")
            .and_then(|v| v.as_object())
            .map(|obj| {
                obj.iter()
                    .map(|(k, v)| (k.clone(), v.as_str().unwrap_or("").to_string()))
                    .collect()
            })
            .unwrap_or_default();

        let result = self
            .knowledge_manager
            .ingest_to_kb(&source, &title, &content, &metadata, collection)
            .await?;

        Ok(json!({
            "status": "ingested",
            "source": source,
            "title": title,
            "result": result,
        }))
    }
}

// ---------------------------------------------------------------------------
// kb_status
// ---------------------------------------------------------------------------

pub struct KBStatusHandler {
    pub knowledge_manager: Arc<KnowledgeManager>,
}

#[async_trait::async_trait]
impl ToolHandler for KBStatusHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let source = input.get("source").and_then(|v| v.as_str());
        self.knowledge_manager.kb_status(source).await
    }
}

// ---------------------------------------------------------------------------
// furnace_status
// ---------------------------------------------------------------------------

pub struct FurnaceStatusHandler {
    pub workspace_root: PathBuf,
    pub project_dir: PathBuf,
    pub user_dir: PathBuf,
    pub is_first_run: bool,
    pub telemetry: Arc<TelemetryTracker>,
    pub recipe_manager: Arc<RecipeManager>,
    pub knowledge_manager: Arc<KnowledgeManager>,
    pub sync_bridge: Arc<SyncBridge>,
    pub sqlite: Arc<SqliteDb>,
    pub scan_stats: Arc<tokio::sync::RwLock<ScanStats>>,
}

#[async_trait::async_trait]
impl ToolHandler for FurnaceStatusHandler {
    async fn handle(&self, _input: Value) -> Result<Value> {
        // Workspace scan info
        let stats = self.scan_stats.read().await;
        let (scan_complete, files_indexed) = if stats.files_scanned > 0 {
            (true, stats.files_indexed)
        } else {
            (false, 0)
        };
        drop(stats);

        // Recipes
        let recipes = self
            .recipe_manager
            .list_recipes_with_descriptions()
            .await
            .unwrap_or_default();
        let recipe_entries: Vec<Value> = recipes
            .iter()
            .map(|(name, desc)| json!({"name": name, "description": desc}))
            .collect();

        // Knowledge bases
        let kb_statuses = self.knowledge_manager.source_statuses().await;
        let kb_entries: Vec<Value> = kb_statuses
            .iter()
            .map(|s| {
                json!({
                    "source": s.name,
                    "synced": s.synced,
                    "knowledge_count": s.knowledge_count,
                })
            })
            .collect();

        // External systems
        let connections = self
            .sync_bridge
            .get_all_links(None)
            .await
            .unwrap_or_default();
        let mut systems: Vec<String> = connections
            .iter()
            .map(|(_, ext)| ext.system.to_string())
            .collect();
        systems.sort();
        systems.dedup();

        // Telemetry
        let telemetry_stats = self.telemetry.get_statistics().ok();

        // Scrolls count
        let scrolls = self
            .sqlite
            .list_scrolls(None)
            .await
            .unwrap_or_default();
        let scroll_count = scrolls.len();

        // Check if .furnace is in .gitignore
        let gitignore_path = self.workspace_root.join(".gitignore");
        let furnace_gitignored = if gitignore_path.exists() {
            std::fs::read_to_string(&gitignore_path)
                .map(|content| {
                    content.lines().any(|line| {
                        let trimmed = line.trim();
                        trimmed == ".furnace" || trimmed == ".furnace/" || trimmed == "/.furnace"
                    })
                })
                .unwrap_or(false)
        } else {
            false
        };

        // Build suggestions
        let mut suggestions: Vec<Value> = Vec::new();

        if self.is_first_run {
            suggestions.push(json!({
                "action": "First-run setup complete",
                "detail": format!(
                    "FURNACE has been initialized for this workspace. \
                     Project data is stored in {}",
                    self.project_dir.display()
                ),
            }));
        }

        if !furnace_gitignored {
            suggestions.push(json!({
                "action": "Add .furnace to .gitignore",
                "detail": "The .furnace/ directory contains local databases and should not be committed. Add '.furnace/' to your .gitignore file.",
            }));
        }

        if recipes.is_empty() {
            suggestions.push(json!({
                "action": "Create prompt templates",
                "detail": "No recipes installed. Recipes are portable prompt templates that standardize common workflows. Use list_recipes to see available templates.",
            }));
        }

        if kb_statuses.is_empty() {
            suggestions.push(json!({
                "action": "Connect a knowledge base",
                "detail": "No knowledge bases configured. Use configure_kb to connect ChromaDB, Pinecone, or another vector store for enriched search.",
            }));
        }

        if systems.is_empty() {
            suggestions.push(json!({
                "action": "Connect an external system",
                "detail": "No external sync configured. Use configure_sync to link Linear, Jira, or Notion for bidirectional issue tracking.",
            }));
        }

        if scroll_count == 0 {
            suggestions.push(json!({
                "action": "Create your first scroll",
                "detail": "No scrolls found. Use create_scroll with a natural-language intent to create a code change specification.",
            }));
        }

        Ok(json!({
            "workflow": {
                "steps": [
                    "1. search_code — find relevant code and understand the codebase",
                    "2. create_scroll — create a scroll with your intent",
                    "3. add_tasks — decompose into file-level tasks with generated_code",
                    "4. execute_scroll — FURNACE handles ordering, code gen, tests, and verification"
                ],
                "important": "Always use scrolls for code changes. execute_scroll runs the full pipeline automatically."
            },
            "workspace": {
                "root": self.workspace_root.display().to_string(),
                "files_indexed": files_indexed,
                "scan_complete": scan_complete,
            },
            "directories": {
                "project": self.project_dir.display().to_string(),
                "user": self.user_dir.display().to_string(),
                "first_run": self.is_first_run,
                "gitignored": furnace_gitignored,
            },
            "recipes": {
                "count": recipe_entries.len(),
                "list": recipe_entries,
            },
            "knowledge_bases": kb_entries,
            "external_systems": systems,
            "telemetry": telemetry_stats,
            "scrolls": {
                "total": scroll_count,
            },
            "suggestions": suggestions,
        }))
    }
}

// ---------------------------------------------------------------------------
// commit_scroll
// ---------------------------------------------------------------------------

pub struct CommitScrollHandler {
    pub sqlite: Arc<SqliteDb>,
    pub workspace_root: PathBuf,
}

#[async_trait::async_trait]
impl ToolHandler for CommitScrollHandler {
    async fn handle(&self, input: Value) -> Result<Value> {
        let scroll_id = require_non_empty(&input, "scroll_id")?;
        let commit_type_override = input
            .get("commit_type")
            .and_then(|v| v.as_str())
            .map(String::from);
        let custom_message = input
            .get("message")
            .and_then(|v| v.as_str())
            .map(String::from);

        // 1. Validate scroll exists and is completed.
        let scroll = self.sqlite.get_scroll(&scroll_id).await
            .map_err(|e| anyhow::anyhow!("Scroll '{}' not found: {}", scroll_id, e))?;

        if scroll.status != crate::types::ScrollStatus::Completed {
            return Err(anyhow::anyhow!(
                "Scroll '{}' has status {:?}, expected Completed. Only completed scrolls can be committed.",
                scroll_id, scroll.status
            ));
        }

        // 2. Check workspace is a git repo.
        let git = crate::git::GitOps::new(self.workspace_root.clone());
        if !git.is_git_repo().await {
            return Err(anyhow::anyhow!(
                "Workspace '{}' is not a git repository. Initialize with `git init` first.",
                self.workspace_root.display()
            ));
        }

        // 3. Collect target files from completed tasks.
        let tasks = self.sqlite.get_tasks_for_scroll(&scroll_id).await?;
        let completed_tasks: Vec<&crate::types::Task> = tasks.iter()
            .filter(|t| t.status == crate::types::TaskStatus::Completed)
            .collect();

        if completed_tasks.is_empty() {
            return Err(anyhow::anyhow!(
                "Scroll '{}' has no completed tasks to commit.",
                scroll_id
            ));
        }

        let files: Vec<String> = completed_tasks.iter()
            .map(|t| t.target_file.clone())
            .collect();

        // 4. Stage the files.
        git.stage_files(&files).await
            .map_err(|e| anyhow::anyhow!("Failed to stage files: {}", e))?;

        // 5. Guard against no-op (nothing actually staged).
        if !git.has_staged_changes().await {
            return Ok(json!({
                "scroll_id": scroll_id,
                "status": "no_changes",
                "message": "No changes to commit. Files may already be committed or unchanged.",
            }));
        }

        // 6. Build commit message.
        let owned_tasks: Vec<crate::types::Task> = completed_tasks.into_iter().cloned().collect();
        let commit_message = if let Some(ref custom) = custom_message {
            custom.clone()
        } else {
            crate::git::build_commit_message(
                &scroll_id,
                &scroll.intent,
                &owned_tasks,
                commit_type_override.as_deref(),
            )
        };

        // 7. Commit.
        let commit_hash = git.commit(&commit_message).await
            .map_err(|e| anyhow::anyhow!("Failed to commit: {}", e))?;

        tracing::info!(
            scroll_id = %scroll_id,
            commit_hash = %commit_hash,
            files = files.len(),
            "Scroll changes committed to git"
        );

        Ok(json!({
            "scroll_id": scroll_id,
            "status": "committed",
            "commit_hash": commit_hash,
            "files_committed": files,
            "commit_message": commit_message,
        }))
    }
}

// ---------------------------------------------------------------------------
// Helper: format an ExternalRef into a human-readable JSON value
// ---------------------------------------------------------------------------

fn format_external_link(ext_ref: &ExternalRef) -> Value {
    let last_synced_iso = chrono::DateTime::from_timestamp(ext_ref.last_synced, 0)
        .map(|dt| dt.to_rfc3339())
        .unwrap_or_else(|| ext_ref.last_synced.to_string());

    let sync_direction_label = match ext_ref.sync_direction {
        SyncDirection::Push => "Push only",
        SyncDirection::Pull => "Pull only",
        SyncDirection::Bidirectional => "Two-way sync",
    };

    json!({
        "system": ext_ref.system.to_string(),
        "external_id": ext_ref.external_id,
        "url": ext_ref.url,
        "sync_direction": ext_ref.sync_direction.to_string(),
        "sync_direction_label": sync_direction_label,
        "last_synced": last_synced_iso,
    })
}

// ---------------------------------------------------------------------------
// Helper: extract KB sources that contributed constraints to a scroll
// ---------------------------------------------------------------------------

fn extract_kb_sources_for_scroll(
    scroll: &crate::types::Scroll,
    kb_statuses: &[crate::knowledge::KBSourceStatus],
) -> Vec<Value> {
    // Constraints with ids starting with "kb-" were contributed by KB enrichment.
    // Format: "kb-{source_name}-{entry_id}"
    let mut seen_sources = std::collections::HashSet::new();
    let mut result = Vec::new();

    for constraint in &scroll.constraints {
        if let Some(rest) = constraint.id.strip_prefix("kb-") {
            // Extract source name (everything before the second '-' segment).
            // The description also contains [KB:source_name] for cross-reference.
            let source_name = rest.split('-').next().unwrap_or(rest);
            if seen_sources.insert(source_name.to_string()) {
                let status = kb_statuses
                    .iter()
                    .find(|s| s.name.eq_ignore_ascii_case(source_name));

                result.push(json!({
                    "source": source_name,
                    "synced": status.map(|s| s.synced).unwrap_or(false),
                    "knowledge_count": status.map(|s| s.knowledge_count).unwrap_or(0),
                }));
            }
        }
    }

    // Also include configured-but-unused sources so the UI shows all registered KBs.
    for status in kb_statuses {
        if !seen_sources.contains(&status.name) {
            result.push(json!({
                "source": status.name,
                "synced": status.synced,
                "knowledge_count": status.knowledge_count,
                "contributed": false,
            }));
        }
    }

    result
}

// ===========================================================================
// Tests
// ===========================================================================

#[cfg(test)]
mod tests {
    use super::*;
    use crate::sync::types::{ExternalRef, ExternalSystem, SyncDirection};
    use crate::telemetry::tracker::{
        AutoForgeEvent, ScrollCreatedEvent, ScrollExecutedEvent, SearchEvent, TelemetryTracker,
    };
    use crate::types::{ExecutionMode, Scroll, ScrollMetadata, ScrollStatus};
    use chrono::Utc;
    use serde_json::json;
    use tempfile::{NamedTempFile, TempDir};

    // -- helpers ----------------------------------------------------------------

    async fn make_sqlite() -> (Arc<SqliteDb>, NamedTempFile) {
        let tmp = NamedTempFile::new().expect("temp file");
        let db = Arc::new(SqliteDb::new(tmp.path()).expect("sqlite"));
        (db, tmp)
    }

    fn make_scroll(id: &str) -> Scroll {
        Scroll {
            id: id.to_string(),
            title: format!("Test scroll {}", id),
            intent: "Testing".to_string(),
            constraints: vec![],
            tasks: vec![],
            metadata: ScrollMetadata {
                execution_mode: ExecutionMode::Forge,
                auto_forge_config: None,
                recipe_used: None,
                search_context: vec![],
            },
            status: ScrollStatus::Draft,
            created_at: Utc::now(),
            updated_at: Utc::now(),
            cleanup_after: None,
        }
    }

    fn make_ext_ref(system: ExternalSystem, direction: SyncDirection) -> ExternalRef {
        ExternalRef {
            system: system.clone(),
            external_id: format!("{}-123", system),
            url: format!("https://example.com/{}-123", system),
            last_synced: 1707300600, // 2024-02-07T10:30:00Z
            sync_direction: direction,
            token_ref: None,
        }
    }

    // -- format_external_link ---------------------------------------------------

    #[test]
    fn test_format_external_link_bidirectional() {
        let ext_ref = make_ext_ref(ExternalSystem::Linear, SyncDirection::Bidirectional);
        let result = format_external_link(&ext_ref);

        assert_eq!(result["system"], "Linear");
        assert_eq!(result["external_id"], "Linear-123");
        assert_eq!(result["url"], "https://example.com/Linear-123");
        assert_eq!(result["sync_direction"], "Bidirectional");
        assert_eq!(result["sync_direction_label"], "Two-way sync");
        // last_synced should be ISO 8601, not a raw integer
        let ls = result["last_synced"].as_str().unwrap();
        assert!(ls.contains("T"), "Expected ISO 8601 format, got: {}", ls);
        assert!(ls.contains("2024-02-07"), "Expected 2024-02-07 in: {}", ls);
    }

    #[test]
    fn test_format_external_link_push_only() {
        let ext_ref = make_ext_ref(ExternalSystem::Jira, SyncDirection::Push);
        let result = format_external_link(&ext_ref);

        assert_eq!(result["sync_direction_label"], "Push only");
    }

    #[test]
    fn test_format_external_link_pull_only() {
        let ext_ref = make_ext_ref(ExternalSystem::Notion, SyncDirection::Pull);
        let result = format_external_link(&ext_ref);

        assert_eq!(result["sync_direction_label"], "Pull only");
    }

    // -- ListScrollsHandler -----------------------------------------------------

    #[tokio::test]
    async fn test_list_scrolls_without_external_links() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        sqlite.create_scroll(&make_scroll("scroll-1")).await.unwrap();

        let handler = ListScrollsHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            knowledge_manager: None,
        };

        let result = handler.handle(json!({})).await.unwrap();
        let scrolls = result["scrolls"].as_array().unwrap();
        assert_eq!(scrolls.len(), 1);
        assert_eq!(scrolls[0]["id"], "scroll-1");
        // external_links should be present but empty
        let links = scrolls[0]["external_links"].as_array().unwrap();
        assert!(links.is_empty());
    }

    #[tokio::test]
    async fn test_list_scrolls_with_external_links() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        sqlite.create_scroll(&make_scroll("scroll-linked")).await.unwrap();

        let ext_ref = make_ext_ref(ExternalSystem::Linear, SyncDirection::Bidirectional);
        sync_bridge.link_scroll("scroll-linked", &ext_ref).await.unwrap();

        let handler = ListScrollsHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            knowledge_manager: None,
        };

        let result = handler.handle(json!({})).await.unwrap();
        let scrolls = result["scrolls"].as_array().unwrap();
        assert_eq!(scrolls.len(), 1);

        let links = scrolls[0]["external_links"].as_array().unwrap();
        assert_eq!(links.len(), 1);
        assert_eq!(links[0]["system"], "Linear");
        assert_eq!(links[0]["external_id"], "Linear-123");
        assert_eq!(links[0]["sync_direction_label"], "Two-way sync");
        // Verify ISO 8601 timestamp
        let ls = links[0]["last_synced"].as_str().unwrap();
        assert!(ls.contains("T"));
    }

    #[tokio::test]
    async fn test_list_scrolls_multiple_links_per_scroll() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        sqlite.create_scroll(&make_scroll("multi-link")).await.unwrap();

        let linear_ref = make_ext_ref(ExternalSystem::Linear, SyncDirection::Push);
        let jira_ref = make_ext_ref(ExternalSystem::Jira, SyncDirection::Bidirectional);
        sync_bridge.link_scroll("multi-link", &linear_ref).await.unwrap();
        sync_bridge.link_scroll("multi-link", &jira_ref).await.unwrap();

        let handler = ListScrollsHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            knowledge_manager: None,
        };

        let result = handler.handle(json!({})).await.unwrap();
        let links = result["scrolls"][0]["external_links"].as_array().unwrap();
        assert_eq!(links.len(), 2);

        let systems: Vec<&str> = links.iter()
            .map(|l| l["system"].as_str().unwrap())
            .collect();
        assert!(systems.contains(&"Linear"));
        assert!(systems.contains(&"Jira"));
    }

    // -- GetScrollHandler -------------------------------------------------------

    #[tokio::test]
    async fn test_get_scroll_with_external_links() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        sqlite.create_scroll(&make_scroll("get-test")).await.unwrap();

        let ext_ref = make_ext_ref(ExternalSystem::Notion, SyncDirection::Pull);
        sync_bridge.link_scroll("get-test", &ext_ref).await.unwrap();

        let handler = GetScrollHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            knowledge_manager: None,
        };

        let result = handler.handle(json!({"scroll_id": "get-test"})).await.unwrap();
        assert!(result["scroll"].is_object());
        assert!(result["tasks"].is_array());

        let links = result["external_links"].as_array().unwrap();
        assert_eq!(links.len(), 1);
        assert_eq!(links[0]["system"], "Notion");
        assert_eq!(links[0]["sync_direction_label"], "Pull only");
    }

    #[tokio::test]
    async fn test_get_scroll_without_external_links() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        sqlite.create_scroll(&make_scroll("no-links")).await.unwrap();

        let handler = GetScrollHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            knowledge_manager: None,
        };

        let result = handler.handle(json!({"scroll_id": "no-links"})).await.unwrap();
        let links = result["external_links"].as_array().unwrap();
        assert!(links.is_empty());
    }

    // -- ListConnectionsHandler -------------------------------------------------

    #[tokio::test]
    async fn test_list_connections_iso8601_and_labels() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        sqlite.create_scroll(&make_scroll("conn-test")).await.unwrap();

        let push_ref = make_ext_ref(ExternalSystem::Linear, SyncDirection::Push);
        let pull_ref = make_ext_ref(ExternalSystem::Jira, SyncDirection::Pull);
        let bidir_ref = make_ext_ref(ExternalSystem::Notion, SyncDirection::Bidirectional);
        sync_bridge.link_scroll("conn-test", &push_ref).await.unwrap();
        sync_bridge.link_scroll("conn-test", &pull_ref).await.unwrap();
        sync_bridge.link_scroll("conn-test", &bidir_ref).await.unwrap();

        let handler = ListConnectionsHandler {
            sync_bridge,
        };

        let result = handler.handle(json!({"scroll_id": "conn-test"})).await.unwrap();
        let conns = result["connections"].as_array().unwrap();
        assert_eq!(conns.len(), 3);
        assert_eq!(result["total"], 3);

        for conn in conns {
            let system = conn["system"].as_str().unwrap();
            let last_synced = conn["last_synced"].as_str().unwrap();
            let label = conn["sync_direction_label"].as_str().unwrap();

            // All should have ISO 8601 timestamps
            assert!(
                last_synced.contains("T"),
                "Expected ISO 8601 for {}, got: {}",
                system, last_synced
            );

            // Verify labels match directions
            match system {
                "Linear" => assert_eq!(label, "Push only"),
                "Jira" => assert_eq!(label, "Pull only"),
                "Notion" => assert_eq!(label, "Two-way sync"),
                _ => panic!("Unexpected system: {}", system),
            }
        }
    }

    // -- GetMetricsHandler ------------------------------------------------------

    #[tokio::test]
    async fn test_get_metrics_basic() {
        let dir = TempDir::new().unwrap();
        let telemetry = Arc::new(TelemetryTracker::new(dir.path()).unwrap());

        // Seed some events
        telemetry.track_scroll_created(ScrollCreatedEvent {
            scroll_id: "s1".into(),
            execution_mode: "Forge".into(),
            task_count: 3,
        }).unwrap();
        telemetry.track_scroll_created(ScrollCreatedEvent {
            scroll_id: "s2".into(),
            execution_mode: "Forge".into(),
            task_count: 1,
        }).unwrap();
        telemetry.track_scroll_executed(ScrollExecutedEvent {
            scroll_id: "s1".into(),
            duration_secs: 10,
            tasks_completed: 3,
            tasks_failed: 0,
        }).unwrap();
        telemetry.track_search(SearchEvent {
            query_type: "code".into(),
            strategy: "semantic".into(),
            latency_ms: 50,
            result_count: 5,
            tier_used: "tier1".into(),
        }).unwrap();
        telemetry.track_search(SearchEvent {
            query_type: "code".into(),
            strategy: "lexical".into(),
            latency_ms: 30,
            result_count: 10,
            tier_used: "tier1".into(),
        }).unwrap();
        telemetry.track_auto_forge(AutoForgeEvent {
            scroll_id: "s2".into(),
            duration_secs: 60,
            test_count: 5,
            test_pass_rate: 1.0,
            user_decision: "accept".into(),
        }).unwrap();

        let handler = GetMetricsHandler { telemetry };
        let result = handler.handle(json!({})).await.unwrap();

        assert_eq!(result["total_events"], 6);
        assert_eq!(result["scrolls_created"], 2);
        assert_eq!(result["scrolls_executed"], 1);
        assert_eq!(result["searches_performed"], 2);
        assert_eq!(result["auto_forge_runs"], 1);
        // avg latency = (50 + 30) / 2 = 40.0
        let avg = result["avg_search_latency_ms"].as_f64().unwrap();
        assert!((avg - 40.0).abs() < f64::EPSILON, "Expected 40.0, got {}", avg);
        assert!(result["earliest_event"].is_number());
        assert!(result["latest_event"].is_number());
    }

    #[tokio::test]
    async fn test_get_metrics_empty() {
        let dir = TempDir::new().unwrap();
        let telemetry = Arc::new(TelemetryTracker::new(dir.path()).unwrap());

        let handler = GetMetricsHandler { telemetry };
        let result = handler.handle(json!({})).await.unwrap();

        assert_eq!(result["total_events"], 0);
        assert_eq!(result["scrolls_created"], 0);
        assert_eq!(result["searches_performed"], 0);
        assert_eq!(result["avg_search_latency_ms"], 0.0);
        assert!(result["earliest_event"].is_null());
    }

    #[tokio::test]
    async fn test_get_metrics_with_since_timestamp() {
        let dir = TempDir::new().unwrap();
        let telemetry = Arc::new(TelemetryTracker::new(dir.path()).unwrap());

        // Record events — they all get "now" timestamp
        telemetry.track_scroll_created(ScrollCreatedEvent {
            scroll_id: "s1".into(),
            execution_mode: "Forge".into(),
            task_count: 1,
        }).unwrap();
        telemetry.track_search(SearchEvent {
            query_type: "code".into(),
            strategy: "semantic".into(),
            latency_ms: 100,
            result_count: 5,
            tier_used: "tier1".into(),
        }).unwrap();

        let now = Utc::now().timestamp();

        // With since_timestamp far in the past, should include all events
        let handler = GetMetricsHandler {
            telemetry: Arc::clone(&telemetry),
        };
        let result = handler.handle(json!({"since_timestamp": 0})).await.unwrap();
        assert_eq!(result["total_events"], 2);

        // With since_timestamp in the far future, should include no events
        let result = handler.handle(json!({"since_timestamp": now + 10000})).await.unwrap();
        assert_eq!(result["total_events"], 0);
        assert_eq!(result["scrolls_created"], 0);
        assert_eq!(result["searches_performed"], 0);
    }

    // -- E2E: create → link → list roundtrip -----------------------------------

    #[tokio::test]
    async fn test_e2e_create_link_list_roundtrip() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        // Step 1: Create a scroll via DB (simulating CreateScrollHandler)
        let scroll = make_scroll("e2e-scroll-1");
        sqlite.create_scroll(&scroll).await.unwrap();

        // Step 2: Link it via LinkIssueHandler
        let link_handler = LinkIssueHandler {
            sync_bridge: Arc::clone(&sync_bridge),
        };
        let link_result = link_handler.handle(json!({
            "scroll_id": "e2e-scroll-1",
            "system": "Linear",
            "external_id": "LIN-456",
            "url": "https://linear.app/team/LIN-456"
        })).await.unwrap();
        assert_eq!(link_result["status"], "linked");
        assert_eq!(link_result["scroll_id"], "e2e-scroll-1");
        assert_eq!(link_result["external_id"], "LIN-456");

        // Step 3: list_scrolls should show the external_links
        let list_handler = ListScrollsHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge: Arc::clone(&sync_bridge),
            knowledge_manager: None,
        };
        let list_result = list_handler.handle(json!({})).await.unwrap();
        let scrolls = list_result["scrolls"].as_array().unwrap();
        assert_eq!(scrolls.len(), 1);

        let links = scrolls[0]["external_links"].as_array().unwrap();
        assert_eq!(links.len(), 1);
        assert_eq!(links[0]["system"], "Linear");
        assert_eq!(links[0]["external_id"], "LIN-456");
        assert_eq!(links[0]["url"], "https://linear.app/team/LIN-456");
        assert_eq!(links[0]["sync_direction_label"], "Two-way sync");

        // Step 4: get_scroll should also show external_links
        let get_handler = GetScrollHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge: Arc::clone(&sync_bridge),
            knowledge_manager: None,
        };
        let get_result = get_handler.handle(json!({
            "scroll_id": "e2e-scroll-1"
        })).await.unwrap();
        let get_links = get_result["external_links"].as_array().unwrap();
        assert_eq!(get_links.len(), 1);
        assert_eq!(get_links[0]["external_id"], "LIN-456");

        // Step 5: list_connections should also show it with enriched fields
        let conn_handler = ListConnectionsHandler {
            sync_bridge: Arc::clone(&sync_bridge),
        };
        let conn_result = conn_handler.handle(json!({
            "scroll_id": "e2e-scroll-1"
        })).await.unwrap();
        let conns = conn_result["connections"].as_array().unwrap();
        assert_eq!(conns.len(), 1);
        assert_eq!(conns[0]["system"], "Linear");
        assert_eq!(conns[0]["sync_direction_label"], "Two-way sync");
        // Verify ISO 8601
        assert!(conns[0]["last_synced"].as_str().unwrap().contains("T"));
    }

    // -- E2E: track events → get_metrics → verify correctness ------------------

    #[tokio::test]
    async fn test_e2e_metrics_correctness() {
        let dir = TempDir::new().unwrap();
        let telemetry = Arc::new(TelemetryTracker::new(dir.path()).unwrap());

        // Simulate a realistic session: 3 searches, 2 scroll creates,
        // 1 execution, 1 auto-forge
        for i in 0..3 {
            telemetry.track_search(SearchEvent {
                query_type: "code".into(),
                strategy: if i % 2 == 0 { "semantic" } else { "lexical" }.into(),
                latency_ms: (i + 1) * 20, // 20, 40, 60
                result_count: 5,
                tier_used: "tier1".into(),
            }).unwrap();
        }

        telemetry.track_scroll_created(ScrollCreatedEvent {
            scroll_id: "s1".into(),
            execution_mode: "Forge".into(),
            task_count: 3,
        }).unwrap();
        telemetry.track_scroll_created(ScrollCreatedEvent {
            scroll_id: "s2".into(),
            execution_mode: "Forge".into(),
            task_count: 5,
        }).unwrap();

        telemetry.track_scroll_executed(ScrollExecutedEvent {
            scroll_id: "s1".into(),
            duration_secs: 45,
            tasks_completed: 3,
            tasks_failed: 0,
        }).unwrap();

        telemetry.track_auto_forge(AutoForgeEvent {
            scroll_id: "s2".into(),
            duration_secs: 120,
            test_count: 10,
            test_pass_rate: 0.9,
            user_decision: "accept".into(),
        }).unwrap();

        let handler = GetMetricsHandler { telemetry };
        let result = handler.handle(json!({})).await.unwrap();

        // 3 searches + 2 creates + 1 execute + 1 autoforge = 7
        assert_eq!(result["total_events"], 7);
        assert_eq!(result["searches_performed"], 3);
        assert_eq!(result["scrolls_created"], 2);
        assert_eq!(result["scrolls_executed"], 1);
        assert_eq!(result["auto_forge_runs"], 1);

        // avg latency = (20 + 40 + 60) / 3 = 40.0
        let avg = result["avg_search_latency_ms"].as_f64().unwrap();
        assert!(
            (avg - 40.0).abs() < f64::EPSILON,
            "Expected avg latency 40.0, got {}",
            avg
        );
    }

    // -- Bidirectional sync verification ----------------------------------------

    #[tokio::test]
    async fn test_bidirectional_link_is_retrievable_after_creation() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        // Create a scroll
        let mut scroll = make_scroll("bidir-test");
        scroll.status = ScrollStatus::InProgress;
        sqlite.create_scroll(&scroll).await.unwrap();

        // Link with Bidirectional direction
        let ext_ref = ExternalRef {
            system: ExternalSystem::Linear,
            external_id: "LIN-789".to_string(),
            url: "https://linear.app/team/LIN-789".to_string(),
            last_synced: Utc::now().timestamp(),
            sync_direction: SyncDirection::Bidirectional,
            token_ref: None,
        };
        sync_bridge.link_scroll("bidir-test", &ext_ref).await.unwrap();

        // Verify the link exists and has correct direction
        let link = sync_bridge
            .get_link("bidir-test", &ExternalSystem::Linear)
            .await
            .unwrap();
        assert!(link.is_some());
        let link = link.unwrap();
        assert_eq!(link.external_id, "LIN-789");
        assert_eq!(link.sync_direction, SyncDirection::Bidirectional);

        // Verify get_all_links also returns it
        let all = sync_bridge
            .get_all_links(Some("bidir-test"))
            .await
            .unwrap();
        assert_eq!(all.len(), 1);
        assert_eq!(all[0].0, "bidir-test");
        assert_eq!(all[0].1.sync_direction, SyncDirection::Bidirectional);
    }

    #[tokio::test]
    async fn test_pull_only_sync_skips_push() {
        use crate::types::ScrollEventListener;

        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        let mut scroll = make_scroll("pull-only-test");
        scroll.status = ScrollStatus::Draft;
        sqlite.create_scroll(&scroll).await.unwrap();

        // Link with Pull-only direction
        let ext_ref = ExternalRef {
            system: ExternalSystem::Jira,
            external_id: "JIRA-100".to_string(),
            url: "https://jira.example.com/JIRA-100".to_string(),
            last_synced: Utc::now().timestamp(),
            sync_direction: SyncDirection::Pull,
            token_ref: None,
        };
        sync_bridge.link_scroll("pull-only-test", &ext_ref).await.unwrap();

        // The event listener should skip push for Pull-only links
        // (no panic, no push attempt)
        sync_bridge
            .on_scroll_status_changed("pull-only-test", &ScrollStatus::InProgress)
            .await;

        // Link still intact
        let link = sync_bridge
            .get_link("pull-only-test", &ExternalSystem::Jira)
            .await
            .unwrap();
        assert!(link.is_some());
    }

    #[tokio::test]
    async fn test_push_direction_is_not_skipped_by_listener() {
        // Verify that Push direction links are NOT skipped by the event
        // listener (unlike Pull-only links). We test this by checking the
        // direction matching logic rather than calling push_to_external
        // (which would hit the OS keyring and may hang in CI).
        let push = SyncDirection::Push;
        let bidir = SyncDirection::Bidirectional;
        let pull = SyncDirection::Pull;

        // Push and Bidirectional should proceed to push
        assert!(matches!(push, SyncDirection::Push | SyncDirection::Bidirectional));
        assert!(matches!(bidir, SyncDirection::Push | SyncDirection::Bidirectional));

        // Pull should be skipped
        assert!(!matches!(pull, SyncDirection::Push | SyncDirection::Bidirectional));
    }

    // -- Edge cases -------------------------------------------------------------

    #[tokio::test]
    async fn test_list_scrolls_with_status_filter() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        let mut s1 = make_scroll("draft-1");
        s1.status = ScrollStatus::Draft;
        let mut s2 = make_scroll("completed-1");
        s2.status = ScrollStatus::Completed;
        sqlite.create_scroll(&s1).await.unwrap();
        sqlite.create_scroll(&s2).await.unwrap();

        let handler = ListScrollsHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            knowledge_manager: None,
        };

        // Filter by Draft
        let result = handler.handle(json!({"status_filter": "Draft"})).await.unwrap();
        let scrolls = result["scrolls"].as_array().unwrap();
        assert_eq!(scrolls.len(), 1);
        assert_eq!(scrolls[0]["id"], "draft-1");
        assert_eq!(scrolls[0]["status"], "Draft");
    }

    #[tokio::test]
    async fn test_e2e_multiple_scrolls_with_mixed_links() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));

        // Create 3 scrolls: one linked, one multi-linked, one unlinked
        sqlite.create_scroll(&make_scroll("linked-1")).await.unwrap();
        sqlite.create_scroll(&make_scroll("multi-linked")).await.unwrap();
        sqlite.create_scroll(&make_scroll("unlinked")).await.unwrap();

        let linear_ref = make_ext_ref(ExternalSystem::Linear, SyncDirection::Bidirectional);
        sync_bridge.link_scroll("linked-1", &linear_ref).await.unwrap();

        let jira_ref = make_ext_ref(ExternalSystem::Jira, SyncDirection::Push);
        let notion_ref = make_ext_ref(ExternalSystem::Notion, SyncDirection::Pull);
        sync_bridge.link_scroll("multi-linked", &jira_ref).await.unwrap();
        sync_bridge.link_scroll("multi-linked", &notion_ref).await.unwrap();

        let handler = ListScrollsHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            knowledge_manager: None,
        };

        let result = handler.handle(json!({})).await.unwrap();
        let scrolls = result["scrolls"].as_array().unwrap();
        assert_eq!(scrolls.len(), 3);

        for s in scrolls {
            let id = s["id"].as_str().unwrap();
            let links = s["external_links"].as_array().unwrap();
            match id {
                "linked-1" => assert_eq!(links.len(), 1),
                "multi-linked" => assert_eq!(links.len(), 2),
                "unlinked" => assert_eq!(links.len(), 0),
                other => panic!("Unexpected scroll id: {}", other),
            }
        }
    }

    // -- AddTasksHandler --------------------------------------------------------

    #[tokio::test]
    async fn test_add_tasks_handler_basic() {
        let (sqlite, _tmp) = make_sqlite().await;
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("add-tasks-test")).await.unwrap();

        let handler = AddTasksHandler {
            sqlite: Arc::clone(&sqlite),
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "add-tasks-test",
            "tasks": [
                {
                    "description": "Create main.dart",
                    "target_file": "lib/main.dart",
                    "task_type": "Create"
                },
                {
                    "description": "Create pubspec.yaml",
                    "target_file": "pubspec.yaml",
                    "task_type": "Create"
                },
                {
                    "description": "Create test file",
                    "target_file": "test/main_test.dart",
                    "task_type": "Test"
                }
            ]
        })).await.unwrap();

        assert_eq!(result["scroll_id"], "add-tasks-test");
        assert_eq!(result["task_count"], 3);
        let task_ids = result["task_ids"].as_array().unwrap();
        assert_eq!(task_ids.len(), 3);
        assert_eq!(task_ids[0], "add-tasks-test-T1");
        assert_eq!(task_ids[1], "add-tasks-test-T2");
        assert_eq!(task_ids[2], "add-tasks-test-T3");

        // Verify tasks are in the DB
        let tasks = sqlite.get_tasks_for_scroll("add-tasks-test").await.unwrap();
        assert_eq!(tasks.len(), 3);
        assert_eq!(tasks[0].target_file, "lib/main.dart");
        assert_eq!(tasks[2].target_file, "test/main_test.dart");
    }

    #[tokio::test]
    async fn test_add_tasks_with_generated_code() {
        let (sqlite, _tmp) = make_sqlite().await;
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("code-tasks")).await.unwrap();

        let handler = AddTasksHandler {
            sqlite: Arc::clone(&sqlite),
            scroll_exporter,
        };

        let code = "void main() { print('hello'); }";
        let result = handler.handle(json!({
            "scroll_id": "code-tasks",
            "tasks": [
                {
                    "description": "Create main.dart with hello world",
                    "target_file": "lib/main.dart",
                    "task_type": "Create",
                    "generated_code": code
                }
            ]
        })).await.unwrap();

        assert_eq!(result["task_count"], 1);

        // Verify generated_code is stored
        let tasks = sqlite.get_tasks_for_scroll("code-tasks").await.unwrap();
        assert_eq!(tasks.len(), 1);
        assert_eq!(tasks[0].generated_code.as_deref(), Some(code));
    }

    #[tokio::test]
    async fn test_add_tasks_empty_array_fails() {
        let (sqlite, _tmp) = make_sqlite().await;
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("empty-tasks")).await.unwrap();

        let handler = AddTasksHandler {
            sqlite: Arc::clone(&sqlite),
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "empty-tasks",
            "tasks": []
        })).await;

        assert!(result.is_err());
        assert!(result.unwrap_err().to_string().contains("must not be empty"));
    }

    #[tokio::test]
    async fn test_add_tasks_nonexistent_scroll_fails() {
        let (sqlite, _tmp) = make_sqlite().await;
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        let handler = AddTasksHandler {
            sqlite: Arc::clone(&sqlite),
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "nonexistent",
            "tasks": [{"description": "x", "target_file": "f", "task_type": "Create"}]
        })).await;

        assert!(result.is_err());
        assert!(result.unwrap_err().to_string().contains("not found"));
    }

    #[tokio::test]
    async fn test_create_scroll_autoforge_defaults() {
        // Verify the make_scroll helper uses the old defaults but the real
        // CreateScrollHandler would set AutoForge mode.  We test the structural
        // expectation: AutoForgeConfig::default() should have auto_apply=true.
        let config = crate::types::AutoForgeConfig::default();
        assert!(config.auto_apply);
        assert!(config.auto_test);
        assert!(config.auto_verify);
        assert_eq!(config.max_iterations, 3);
    }

    // -- UpdateScrollHandler ----------------------------------------------------

    #[tokio::test]
    async fn test_update_scroll_handler_title_and_intent() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("update-test")).await.unwrap();

        let handler = UpdateScrollHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "update-test",
            "title": "Updated Title",
            "intent": "Updated intent description"
        })).await.unwrap();

        assert_eq!(result["status"], "updated");

        // Verify DB was updated
        let scroll = sqlite.get_scroll("update-test").await.unwrap();
        assert_eq!(scroll.title, "Updated Title");
        assert_eq!(scroll.intent, "Updated intent description");
    }

    #[tokio::test]
    async fn test_update_scroll_execution_mode() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("mode-test")).await.unwrap();

        let handler = UpdateScrollHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "mode-test",
            "execution_mode": "AutoForge",
            "auto_forge_config": {
                "auto_test": false,
                "max_iterations": 5
            }
        })).await.unwrap();

        assert_eq!(result["status"], "updated");

        let scroll = sqlite.get_scroll("mode-test").await.unwrap();
        assert!(matches!(scroll.metadata.execution_mode, ExecutionMode::AutoForge));
        let afc = scroll.metadata.auto_forge_config.unwrap();
        assert!(!afc.auto_test);
        assert_eq!(afc.max_iterations, 5);
        assert!(afc.auto_apply); // default preserved
    }

    // -- UpdateTaskHandler ------------------------------------------------------

    #[tokio::test]
    async fn test_update_task_handler() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("task-update")).await.unwrap();

        // Create a task first
        let task = crate::types::Task {
            id: "task-update-T1".into(),
            scroll_id: "task-update".into(),
            description: "Original description".into(),
            target_file: "original.dart".into(),
            task_type: crate::types::TaskType::Create,
            status: crate::types::TaskStatus::Pending,
            dependencies: vec![],
            generated_code: None,
            diff: None,
            created_at: Utc::now(),
            group_label: None,
        };
        sqlite.create_task(&task).await.unwrap();

        let handler = UpdateTaskHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "task-update",
            "task_id": "task-update-T1",
            "description": "Updated description",
            "target_file": "updated.dart",
            "generated_code": "void main() {}"
        })).await.unwrap();

        assert_eq!(result["status"], "updated");

        // Verify DB
        let updated = sqlite.get_task_by_id("task-update-T1").await.unwrap();
        assert_eq!(updated.description, "Updated description");
        assert_eq!(updated.target_file, "updated.dart");
        assert_eq!(updated.generated_code.as_deref(), Some("void main() {}"));
    }

    // -- RemoveTasksHandler -----------------------------------------------------

    #[tokio::test]
    async fn test_remove_tasks_handler() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("rm-tasks")).await.unwrap();

        for i in 1..=3 {
            let task = crate::types::Task {
                id: format!("rm-tasks-T{}", i),
                scroll_id: "rm-tasks".into(),
                description: format!("Task {}", i),
                target_file: format!("file{}.rs", i),
                task_type: crate::types::TaskType::Create,
                status: crate::types::TaskStatus::Pending,
                dependencies: vec![],
                generated_code: None,
                diff: None,
                created_at: Utc::now(),
                group_label: None,
            };
            sqlite.create_task(&task).await.unwrap();
        }

        let handler = RemoveTasksHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "rm-tasks",
            "task_ids": ["rm-tasks-T1", "rm-tasks-T3"]
        })).await.unwrap();

        assert_eq!(result["removed"], 2);

        // Only task T2 should remain
        let remaining = sqlite.get_tasks_for_scroll("rm-tasks").await.unwrap();
        assert_eq!(remaining.len(), 1);
        assert_eq!(remaining[0].id, "rm-tasks-T2");
    }

    #[tokio::test]
    async fn test_remove_tasks_empty_fails() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        sqlite.create_scroll(&make_scroll("rm-empty")).await.unwrap();

        let handler = RemoveTasksHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge,
            scroll_exporter,
        };

        let result = handler.handle(json!({
            "scroll_id": "rm-empty",
            "task_ids": []
        })).await;

        assert!(result.is_err());
        assert!(result.unwrap_err().to_string().contains("must not be empty"));
    }

    // -- E2E: create → add_tasks → update → remove → verify --------------------

    #[tokio::test]
    async fn test_e2e_scroll_edit_workflow() {
        let (sqlite, _tmp) = make_sqlite().await;
        let sync_bridge = Arc::new(SyncBridge::new(Arc::clone(&sqlite)));
        let dir = TempDir::new().unwrap();
        let scroll_exporter = Arc::new(
            crate::scroll_export::ScrollExporter::new(dir.path().to_path_buf(), Arc::clone(&sqlite)),
        );

        // 1. Create scroll
        sqlite.create_scroll(&make_scroll("e2e-edit")).await.unwrap();

        // 2. Add tasks
        let add_handler = AddTasksHandler {
            sqlite: Arc::clone(&sqlite),
            scroll_exporter: Arc::clone(&scroll_exporter),
        };
        add_handler.handle(json!({
            "scroll_id": "e2e-edit",
            "tasks": [
                {"description": "File A", "target_file": "a.rs", "task_type": "Create", "generated_code": "// A"},
                {"description": "File B", "target_file": "b.rs", "task_type": "Create", "generated_code": "// B"},
                {"description": "File C", "target_file": "c.rs", "task_type": "Create", "generated_code": "// C"}
            ]
        })).await.unwrap();

        assert_eq!(sqlite.get_tasks_for_scroll("e2e-edit").await.unwrap().len(), 3);

        // 3. Update task (user edits code)
        let update_handler = UpdateTaskHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge: Arc::clone(&sync_bridge),
            scroll_exporter: Arc::clone(&scroll_exporter),
        };
        update_handler.handle(json!({
            "scroll_id": "e2e-edit",
            "task_id": "e2e-edit-T2",
            "generated_code": "// B updated by user"
        })).await.unwrap();

        let t2 = sqlite.get_task_by_id("e2e-edit-T2").await.unwrap();
        assert_eq!(t2.generated_code.as_deref(), Some("// B updated by user"));

        // 4. Remove task (user removes File C)
        let remove_handler = RemoveTasksHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge: Arc::clone(&sync_bridge),
            scroll_exporter: Arc::clone(&scroll_exporter),
        };
        remove_handler.handle(json!({
            "scroll_id": "e2e-edit",
            "task_ids": ["e2e-edit-T3"]
        })).await.unwrap();

        let remaining = sqlite.get_tasks_for_scroll("e2e-edit").await.unwrap();
        assert_eq!(remaining.len(), 2);

        // 5. Update scroll metadata
        let scroll_handler = UpdateScrollHandler {
            sqlite: Arc::clone(&sqlite),
            sync_bridge: Arc::clone(&sync_bridge),
            scroll_exporter: Arc::clone(&scroll_exporter),
        };
        scroll_handler.handle(json!({
            "scroll_id": "e2e-edit",
            "title": "User-reviewed scroll"
        })).await.unwrap();

        let scroll = sqlite.get_scroll("e2e-edit").await.unwrap();
        assert_eq!(scroll.title, "User-reviewed scroll");
    }
}
