use crate::graph::{Node, Task, WorkGraph};
use std::collections::HashMap;
use std::fs::{File, OpenOptions};
use std::io::{BufRead, BufReader, Write};
use std::path::{Path, PathBuf};
use thiserror::Error;

#[derive(Error, Debug)]
pub enum ParseError {
    #[error("IO error: {0}")]
    Io(#[from] std::io::Error),
    #[error("JSON error on line {line}: {source}")]
    Json {
        line: usize,
        source: serde_json::Error,
    },
    #[error("Lock error: {0}")]
    Lock(String),
}

/// RAII guard for file locks - automatically releases lock on drop
struct FileLock {
    #[cfg(unix)]
    file: File,
}

impl FileLock {
    /// Acquire an exclusive lock on a lock file (blocks until available)
    #[cfg(unix)]
    fn acquire<P: AsRef<Path>>(lock_path: P) -> Result<Self, ParseError> {
        Self::flock_impl(&lock_path, libc::LOCK_EX)
    }

    #[cfg(not(unix))]
    fn acquire<P: AsRef<Path>>(_lock_path: P) -> Result<Self, ParseError> {
        // On non-Unix systems, we can't use flock - return a no-op lock
        // This is a limitation but WG is primarily for Unix systems
        Ok(FileLock {})
    }

    /// Try to acquire a shared (read) lock, non-blocking.
    /// Returns `Ok(Some(lock))` on success, `Ok(None)` if the lock is held
    /// exclusively by another process (EWOULDBLOCK).
    ///
    /// Transient `EIO` / `EINTR` (notably MooseFS surfacing flock contention
    /// as `EIO`) are retried with bounded exponential backoff via
    /// [`crate::lock::retry_acquire`]. `EWOULDBLOCK` is intentionally NOT
    /// retried here — it is the documented non-error signal that another
    /// process holds the exclusive lock, and the caller proceeds without
    /// taking the shared lock (atomic-rename writes make readers safe).
    #[cfg(unix)]
    fn try_acquire_shared<P: AsRef<Path>>(lock_path: P) -> Result<Option<Self>, ParseError> {
        use std::os::unix::io::AsRawFd;

        if let Some(parent) = lock_path.as_ref().parent() {
            std::fs::create_dir_all(parent)?;
        }

        let file = OpenOptions::new()
            .read(true)
            .write(true)
            .create(true)
            .truncate(false)
            .open(&lock_path)?;

        let fd = file.as_raw_fd();
        let policy = crate::lock::RetryPolicy::default();
        let mut wouldblock = false;
        let res =
            crate::lock::retry_acquire(&policy, crate::lock::is_transient_nonblocking, || {
                let ret = unsafe { libc::flock(fd, libc::LOCK_SH | libc::LOCK_NB) };
                if ret == 0 {
                    return Ok(());
                }
                let err = std::io::Error::last_os_error();
                if err.kind() == std::io::ErrorKind::WouldBlock {
                    wouldblock = true;
                    return Ok(()); // Stop retrying; caller proceeds lockless.
                }
                Err(err)
            });

        match res {
            Ok(()) if wouldblock => Ok(None),
            Ok(()) => Ok(Some(FileLock { file })),
            Err(err) => Err(ParseError::Lock(format!(
                "Failed to acquire shared lock on {:?}: {}",
                lock_path.as_ref(),
                err
            ))),
        }
    }

    #[cfg(not(unix))]
    fn try_acquire_shared<P: AsRef<Path>>(_lock_path: P) -> Result<Option<Self>, ParseError> {
        Ok(Some(FileLock {}))
    }

    #[cfg(unix)]
    fn flock_impl<P: AsRef<Path>>(
        lock_path: P,
        operation: libc::c_int,
    ) -> Result<Self, ParseError> {
        use std::os::unix::io::AsRawFd;

        if let Some(parent) = lock_path.as_ref().parent() {
            std::fs::create_dir_all(parent)?;
        }

        let file = OpenOptions::new()
            .read(true)
            .write(true)
            .create(true)
            .truncate(false)
            .open(&lock_path)?;

        let fd = file.as_raw_fd();
        let policy = crate::lock::RetryPolicy::default();
        crate::lock::retry_acquire(&policy, crate::lock::is_transient_blocking, || {
            let ret = unsafe { libc::flock(fd, operation) };
            if ret == 0 {
                Ok(())
            } else {
                Err(std::io::Error::last_os_error())
            }
        })
        .map_err(|err| {
            ParseError::Lock(format!(
                "Failed to acquire lock on {:?}: {}",
                lock_path.as_ref(),
                err
            ))
        })?;

        Ok(FileLock { file })
    }
}

impl Drop for FileLock {
    fn drop(&mut self) {
        #[cfg(unix)]
        {
            use std::os::unix::io::AsRawFd;
            // Release the lock (LOCK_UN) - best effort, ignore errors on drop
            let fd = self.file.as_raw_fd();
            unsafe {
                libc::flock(fd, libc::LOCK_UN);
            }
        }
    }
}

/// Get the lock file path for a given graph file
fn get_lock_path<P: AsRef<Path>>(graph_path: P) -> PathBuf {
    let graph_path = graph_path.as_ref();
    if let Some(parent) = graph_path.parent() {
        parent.join("graph.lock")
    } else {
        PathBuf::from("graph.lock")
    }
}

/// Load a WG task graph from a JSONL file (internal, no locking).
///
/// Callers must hold the flock themselves or use [`load_graph`] which
/// acquires it automatically.
fn load_graph_inner<P: AsRef<Path>>(path: P) -> Result<WorkGraph, ParseError> {
    let file = File::open(path)?;
    let reader = BufReader::new(file);
    let mut graph = WorkGraph::new();

    for (line_num, line) in reader.lines().enumerate() {
        let line = line?;
        let trimmed = line.trim();
        if trimmed.is_empty() || trimmed.starts_with('#') {
            continue;
        }
        // Skip legacy Actor nodes (removed in actor-system cleanup)
        if let Ok(v) = serde_json::from_str::<serde_json::Value>(trimmed)
            && v.get("kind").and_then(|k| k.as_str()) == Some("actor")
        {
            continue;
        }
        let node: Node = serde_json::from_str(trimmed).map_err(|e| ParseError::Json {
            line: line_num + 1,
            source: e,
        })?;
        let node_id = node.id().to_string();
        if graph.get_node(&node_id).is_some() {
            eprintln!(
                "Warning: duplicate node ID '{}' at line {} (overwriting previous definition)",
                node_id,
                line_num + 1
            );
        }
        graph.add_node(node);
    }

    Ok(graph)
}

/// Load a WG task graph from a JSONL file.
///
/// Uses a non-blocking shared lock (`LOCK_SH | LOCK_NB`). If another process
/// holds an exclusive lock (e.g. `modify_graph` in the coordinator), the read
/// proceeds without the lock. This is safe because `save_graph_inner` uses
/// atomic writes (temp file + rename), so readers always see a consistent
/// snapshot — either the pre- or post-write state, never a partial write.
///
/// This prevents the TUI (and other read-only callers) from blocking
/// indefinitely while the coordinator holds an exclusive lock during
/// long-running modify_graph closures.
pub fn load_graph<P: AsRef<Path>>(path: P) -> Result<WorkGraph, ParseError> {
    let lock_path = get_lock_path(&path);
    // Try a non-blocking shared lock; proceed regardless.
    let _lock = FileLock::try_acquire_shared(&lock_path)?;
    load_graph_inner(path)
    // Lock (if acquired) is automatically released when _lock goes out of scope
}

/// Save a WG task graph to a JSONL file (internal, no locking).
///
/// Callers must hold the flock themselves or use [`save_graph`] which
/// acquires it automatically.
fn save_graph_inner<P: AsRef<Path>>(graph: &WorkGraph, path: P) -> Result<(), ParseError> {
    let path = path.as_ref();

    // Write to a temporary file in the same directory, then atomically rename.
    // This ensures a crash mid-write leaves the original file intact.
    let parent = path.parent().unwrap_or(Path::new("."));
    let tmp_path = parent.join(format!(".graph.tmp.{}", std::process::id()));

    let result = (|| -> Result<(), ParseError> {
        let mut file = OpenOptions::new()
            .write(true)
            .create(true)
            .truncate(true)
            .open(&tmp_path)?;

        for node in graph.nodes() {
            let json =
                serde_json::to_string(node).map_err(|e| ParseError::Json { line: 0, source: e })?;
            writeln!(file, "{}", json)?;
        }

        file.flush()?;
        #[cfg(unix)]
        {
            use std::os::unix::io::AsRawFd;
            // fsync to ensure data is on disk before rename
            let rc = unsafe { libc::fsync(file.as_raw_fd()) };
            if rc != 0 {
                return Err(ParseError::Io(std::io::Error::last_os_error()));
            }
        }

        Ok(())
    })();

    if result.is_ok() {
        std::fs::rename(&tmp_path, path)?;
    } else {
        // Clean up temp file on failure
        let _ = std::fs::remove_file(&tmp_path);
    }

    result
}

/// Save a WG task graph to a JSONL file
/// Uses advisory file locking and atomic write (temp file + rename) to
/// prevent data loss on crash.
pub fn save_graph<P: AsRef<Path>>(graph: &WorkGraph, path: P) -> Result<(), ParseError> {
    let path = path.as_ref();
    let lock_path = get_lock_path(path);
    let _lock = FileLock::acquire(&lock_path)?;
    save_graph_inner(graph, path)
    // Lock is automatically released when _lock goes out of scope
}

/// Atomically load, modify, and save a graph file.
///
/// The file lock is held for the entire load-modify-save cycle, preventing
/// concurrent read-modify-write operations from clobbering each other's
/// changes (the "lost update" problem).
///
/// The closure receives a mutable reference to the freshly-loaded graph.
/// It should return `true` if it modified the graph (triggering a save),
/// or `false` to skip saving.
///
/// Returns the final graph state (after any modifications and save).
pub fn modify_graph<P, F>(path: P, f: F) -> Result<WorkGraph, ParseError>
where
    P: AsRef<Path>,
    F: FnOnce(&mut WorkGraph) -> bool,
{
    let path = path.as_ref();
    let lock_path = get_lock_path(path);
    let _lock = FileLock::acquire(&lock_path)?;

    let mut graph = load_graph_inner(path)?;
    // Snapshot every task before the closure runs so we can detect which
    // tasks changed substantively (anything other than `last_interaction_at`)
    // and bump their interaction timestamp. This is the single helper that
    // wraps mutation + timestamp-bump for every modify_graph caller.
    let before: HashMap<String, Task> = graph.tasks().map(|t| (t.id.clone(), t.clone())).collect();
    let modified = f(&mut graph);
    if modified {
        bump_interaction_timestamps(&mut graph, &before);
        save_graph_inner(&graph, path)?;
    }
    Ok(graph)
    // Lock is automatically released when _lock goes out of scope
}

/// For every task whose persistent fields changed (other than
/// `last_interaction_at` itself), set `last_interaction_at` to now.
/// New tasks created in the closure are bumped if they don't already have
/// an interaction timestamp set.
fn bump_interaction_timestamps(graph: &mut WorkGraph, before: &HashMap<String, Task>) {
    let now = chrono::Utc::now().to_rfc3339();
    for task in graph.tasks_mut() {
        match before.get(&task.id) {
            None => {
                if task.last_interaction_at.is_none() {
                    task.last_interaction_at = Some(now.clone());
                }
            }
            Some(prev) => {
                if !task.substantively_eq(prev) {
                    task.last_interaction_at = Some(now.clone());
                }
            }
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::graph::Task;
    use std::io::Write;
    use tempfile::NamedTempFile;

    fn make_task(id: &str, title: &str) -> Task {
        Task {
            id: id.to_string(),
            title: title.to_string(),
            ..Task::default()
        }
    }

    #[test]
    fn test_load_empty_file() {
        let file = NamedTempFile::new().unwrap();
        let graph = load_graph(file.path()).unwrap();
        assert!(graph.is_empty());
    }

    #[test]
    fn test_load_single_task() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Test","status":"open"}}"#
        )
        .unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 1);
        assert!(graph.get_task("t1").is_some());
    }

    #[test]
    fn test_load_multiple_nodes() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Task 1","status":"open"}}"#
        )
        .unwrap();
        writeln!(
            file,
            r#"{{"id":"t2","kind":"task","title":"Task 2","status":"done"}}"#
        )
        .unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 2);
    }

    #[test]
    fn test_load_skips_legacy_actor_nodes() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Task 1","status":"open"}}"#
        )
        .unwrap();
        writeln!(file, r#"{{"id":"erik","kind":"actor","name":"Erik"}}"#).unwrap();
        writeln!(
            file,
            r#"{{"id":"t2","kind":"task","title":"Task 2","status":"done"}}"#
        )
        .unwrap();

        let graph = load_graph(file.path()).unwrap();
        // Actor nodes should be silently skipped
        assert_eq!(graph.len(), 2);
        assert!(graph.get_task("t1").is_some());
        assert!(graph.get_task("t2").is_some());
    }

    #[test]
    fn test_load_skips_empty_lines_and_comments() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, "# This is a comment").unwrap();
        writeln!(file).unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Test","status":"open"}}"#
        )
        .unwrap();
        writeln!(file, "   ").unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 1);
    }

    #[test]
    fn test_load_invalid_json_returns_error() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, "not valid json").unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        let err = result.unwrap_err();
        assert!(matches!(err, ParseError::Json { line: 1, .. }));
    }

    #[test]
    fn test_save_and_load_roundtrip() {
        let mut graph = WorkGraph::new();
        graph.add_node(Node::Task(make_task("t1", "Task 1")));
        graph.add_node(Node::Task(make_task("t2", "Task 2")));

        let file = NamedTempFile::new().unwrap();
        save_graph(&graph, file.path()).unwrap();

        let loaded = load_graph(file.path()).unwrap();
        assert_eq!(loaded.len(), 2);
        assert!(loaded.get_task("t1").is_some());
        assert!(loaded.get_task("t2").is_some());
    }

    #[test]
    fn test_load_nonexistent_file_returns_error() {
        let result = load_graph("/nonexistent/path/graph.jsonl");
        assert!(result.is_err());
        assert!(matches!(result.unwrap_err(), ParseError::Io(_)));
    }

    // ---- Edge case tests for error handling ----

    #[test]
    fn test_load_mid_record_corruption() {
        // Valid line, then a corrupted line (truncated JSON), then another valid line
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Good","status":"open"}}"#
        )
        .unwrap();
        writeln!(file, r#"{{"id":"t2","kind":"task","title":"Trun"#).unwrap(); // truncated
        writeln!(
            file,
            r#"{{"id":"t3","kind":"task","title":"Also Good","status":"open"}}"#
        )
        .unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        match result.unwrap_err() {
            ParseError::Json { line, .. } => assert_eq!(line, 2),
            other => panic!("Expected ParseError::Json, got: {:?}", other),
        }
    }

    #[test]
    fn test_load_truncated_single_line() {
        let mut file = NamedTempFile::new().unwrap();
        write!(file, r#"{{"id":"t1","kind":"task""#).unwrap(); // no newline, truncated
        file.flush().unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(
            result.unwrap_err(),
            ParseError::Json { line: 1, .. }
        ));
    }

    #[test]
    fn test_load_invalid_json_on_specific_lines() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"OK","status":"open"}}"#
        )
        .unwrap();
        writeln!(
            file,
            r#"{{"id":"t2","kind":"task","title":"OK","status":"open"}}"#
        )
        .unwrap();
        writeln!(
            file,
            r#"{{"id":"t3","kind":"task","title":"OK","status":"open"}}"#
        )
        .unwrap();
        writeln!(file, "this is not json at all").unwrap(); // line 4
        writeln!(
            file,
            r#"{{"id":"t5","kind":"task","title":"OK","status":"open"}}"#
        )
        .unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        match result.unwrap_err() {
            ParseError::Json { line, .. } => assert_eq!(line, 4),
            other => panic!("Expected ParseError::Json on line 4, got: {:?}", other),
        }
    }

    #[test]
    fn test_load_bare_opening_brace() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, "{{").unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(
            result.unwrap_err(),
            ParseError::Json { line: 1, .. }
        ));
    }

    #[test]
    fn test_load_json_array_instead_of_object() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"[{{"id":"t1","kind":"task","title":"T","status":"open"}}]"#
        )
        .unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(
            result.unwrap_err(),
            ParseError::Json { line: 1, .. }
        ));
    }

    #[test]
    fn test_load_mixed_line_endings_crlf() {
        let mut file = NamedTempFile::new().unwrap();
        // Write with \r\n line endings
        write!(
            file,
            "{{\"id\":\"t1\",\"kind\":\"task\",\"title\":\"CRLF Task 1\",\"status\":\"open\"}}\r\n{{\"id\":\"t2\",\"kind\":\"task\",\"title\":\"CRLF Task 2\",\"status\":\"done\"}}\r\n",
        )
        .unwrap();
        file.flush().unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 2);
        assert!(graph.get_task("t1").is_some());
        assert!(graph.get_task("t2").is_some());
    }

    #[test]
    fn test_load_mixed_line_endings_mixed() {
        let mut file = NamedTempFile::new().unwrap();
        // Mix \n and \r\n in the same file
        write!(
            file,
            "{{\"id\":\"t1\",\"kind\":\"task\",\"title\":\"LF\",\"status\":\"open\"}}\n{{\"id\":\"t2\",\"kind\":\"task\",\"title\":\"CRLF\",\"status\":\"open\"}}\r\n{{\"id\":\"t3\",\"kind\":\"task\",\"title\":\"LF again\",\"status\":\"open\"}}\n",
        )
        .unwrap();
        file.flush().unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 3);
    }

    #[test]
    fn test_load_whitespace_only_file() {
        let mut file = NamedTempFile::new().unwrap();
        write!(file, "   \n  \t  \n\n   \n").unwrap();
        file.flush().unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert!(graph.is_empty());
    }

    #[test]
    fn test_load_file_with_trailing_newlines() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Test","status":"open"}}"#
        )
        .unwrap();
        write!(file, "\n\n\n").unwrap();
        file.flush().unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 1);
    }

    #[test]
    fn test_load_non_utf8_file() {
        let mut file = NamedTempFile::new().unwrap();
        // Write valid JSON followed by invalid UTF-8 bytes
        let valid_start = br#"{"id":"t1","kind":"task","title":""#;
        let invalid_utf8: &[u8] = &[0xFF, 0xFE, 0x80, 0x81];
        let valid_end = br#"","status":"open"}"#;
        file.write_all(valid_start).unwrap();
        file.write_all(invalid_utf8).unwrap();
        file.write_all(valid_end).unwrap();
        writeln!(file).unwrap();
        file.flush().unwrap();

        // BufReader::lines() returns Err for non-UTF8 lines, which becomes ParseError::Io
        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(result.unwrap_err(), ParseError::Io(_)));
    }

    #[test]
    fn test_load_large_graph_1000_nodes() {
        let mut file = NamedTempFile::new().unwrap();
        for i in 0..1000 {
            writeln!(
                file,
                r#"{{"id":"t{i}","kind":"task","title":"Task {i}","status":"open"}}"#
            )
            .unwrap();
        }
        file.flush().unwrap();

        let start = std::time::Instant::now();
        let graph = load_graph(file.path()).unwrap();
        let elapsed = start.elapsed();

        assert_eq!(graph.len(), 1000);
        // Sanity: loading 1000 tasks should be well under 5 seconds
        assert!(
            elapsed.as_secs() < 5,
            "Loading 1000 nodes took {:?}, which is too slow",
            elapsed
        );
    }

    #[test]
    fn test_save_and_load_large_graph_roundtrip() {
        let mut graph = WorkGraph::new();
        for i in 0..1000 {
            graph.add_node(Node::Task(make_task(
                &format!("t{i}"),
                &format!("Task {i}"),
            )));
        }

        let file = NamedTempFile::new().unwrap();
        save_graph(&graph, file.path()).unwrap();

        let loaded = load_graph(file.path()).unwrap();
        assert_eq!(loaded.len(), 1000);
    }

    #[test]
    fn test_special_characters_in_task_id() {
        let ids = &[
            "task-with-dashes",
            "task_with_underscores",
            "task.with.dots",
            "task/with/slashes",
            "task:with:colons",
            "UPPER_CASE",
            "MiXeD-CaSe_123",
        ];

        for id in ids {
            let mut graph = WorkGraph::new();
            graph.add_node(Node::Task(make_task(id, "Test")));

            let file = NamedTempFile::new().unwrap();
            save_graph(&graph, file.path()).unwrap();
            let loaded = load_graph(file.path()).unwrap();
            assert!(
                loaded.get_task(id).is_some(),
                "Failed roundtrip for id: {}",
                id
            );
        }
    }

    #[test]
    fn test_special_characters_in_task_title() {
        let titles = &[
            "Title with spaces",
            "Title with \"quotes\"",
            "Title with 'apostrophes'",
            "Title with <angle> & brackets",
            "Title with unicode: café résumé naïve",
            "Title with emoji: \u{1F680}\u{1F525}",
            "Title with newline\\nescaped",
            "Title with tab\\tescaped",
            "日本語のタイトル",
            "Título en español",
        ];

        for title in titles {
            let mut graph = WorkGraph::new();
            graph.add_node(Node::Task(make_task("test-id", title)));

            let file = NamedTempFile::new().unwrap();
            save_graph(&graph, file.path()).unwrap();
            let loaded = load_graph(file.path()).unwrap();
            let task = loaded.get_task("test-id").unwrap();
            assert_eq!(&task.title, title, "Failed roundtrip for title: {}", title);
        }
    }

    #[test]
    fn test_load_valid_json_but_wrong_schema() {
        let mut file = NamedTempFile::new().unwrap();
        // Valid JSON object but missing required fields (no "kind" tag)
        writeln!(file, r#"{{"foo":"bar","baz":42}}"#).unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(
            result.unwrap_err(),
            ParseError::Json { line: 1, .. }
        ));
    }

    #[test]
    fn test_load_task_missing_title() {
        let mut file = NamedTempFile::new().unwrap();
        // Has kind but missing required "title" field
        writeln!(file, r#"{{"id":"t1","kind":"task","status":"open"}}"#).unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(
            result.unwrap_err(),
            ParseError::Json { line: 1, .. }
        ));
    }

    #[test]
    fn test_load_task_missing_id() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, r#"{{"kind":"task","title":"No ID","status":"open"}}"#).unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(
            result.unwrap_err(),
            ParseError::Json { line: 1, .. }
        ));
    }

    #[test]
    fn test_load_resource_node() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, r#"{{"id":"r1","kind":"resource","name":"Budget","type":"currency","available":1000.0,"unit":"USD"}}"#).unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 1);
        assert!(graph.get_node("r1").is_some());
    }

    #[test]
    fn test_load_unknown_kind() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"x1","kind":"unknown_type","title":"Mystery"}}"#
        )
        .unwrap();

        let result = load_graph(file.path());
        assert!(result.is_err());
        assert!(matches!(
            result.unwrap_err(),
            ParseError::Json { line: 1, .. }
        ));
    }

    #[test]
    fn test_save_to_readonly_path() {
        use std::os::unix::fs::PermissionsExt;

        let dir = tempfile::tempdir().unwrap();
        let file_path = dir.path().join("readonly.jsonl");

        // Create the file, then make the directory read-only so the temp
        // file cannot be created (atomic write uses a temp file + rename).
        File::create(&file_path).unwrap();
        std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o555)).unwrap();

        let graph = WorkGraph::new();
        let result = save_graph(&graph, &file_path);
        assert!(result.is_err());
        // Clean up: restore write permission so tempdir can be deleted
        std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755)).unwrap();
    }

    #[test]
    fn test_save_to_nonexistent_directory() {
        let graph = WorkGraph::new();
        let result = save_graph(&graph, "/nonexistent/deep/path/graph.jsonl");
        assert!(result.is_err());
    }

    #[test]
    fn test_save_empty_graph() {
        let graph = WorkGraph::new();
        let file = NamedTempFile::new().unwrap();
        save_graph(&graph, file.path()).unwrap();

        let loaded = load_graph(file.path()).unwrap();
        assert!(loaded.is_empty());
    }

    #[test]
    fn test_save_overwrites_existing_content() {
        let file = NamedTempFile::new().unwrap();

        // Save a graph with 3 tasks
        let mut graph1 = WorkGraph::new();
        graph1.add_node(Node::Task(make_task("t1", "Task 1")));
        graph1.add_node(Node::Task(make_task("t2", "Task 2")));
        graph1.add_node(Node::Task(make_task("t3", "Task 3")));
        save_graph(&graph1, file.path()).unwrap();

        // Save a graph with 1 task - should overwrite, not append
        let mut graph2 = WorkGraph::new();
        graph2.add_node(Node::Task(make_task("t4", "Task 4")));
        save_graph(&graph2, file.path()).unwrap();

        let loaded = load_graph(file.path()).unwrap();
        assert_eq!(loaded.len(), 1);
        assert!(loaded.get_task("t4").is_some());
        assert!(loaded.get_task("t1").is_none());
    }

    #[test]
    fn test_load_duplicate_ids_last_wins() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"First","status":"open"}}"#
        )
        .unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Second","status":"done"}}"#
        )
        .unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 1);
        let task = graph.get_task("t1").unwrap();
        assert_eq!(task.title, "Second");
    }

    #[test]
    fn test_load_extra_json_fields_are_ignored() {
        let mut file = NamedTempFile::new().unwrap();
        // JSON with extra unknown fields - serde should ignore them with default config
        writeln!(file, r#"{{"id":"t1","kind":"task","title":"Test","status":"open","unknown_field":"value","another":42}}"#).unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 1);
        assert!(graph.get_task("t1").is_some());
    }

    #[test]
    fn test_load_comments_only_file() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, "# comment 1").unwrap();
        writeln!(file, "# comment 2").unwrap();
        writeln!(file, "# comment 3").unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert!(graph.is_empty());
    }

    #[test]
    fn test_load_interleaved_comments_and_tasks() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, "# Header comment").unwrap();
        writeln!(
            file,
            r#"{{"id":"t1","kind":"task","title":"Task 1","status":"open"}}"#
        )
        .unwrap();
        writeln!(file, "# Mid-file comment").unwrap();
        writeln!(file).unwrap();
        writeln!(
            file,
            r#"{{"id":"t2","kind":"task","title":"Task 2","status":"open"}}"#
        )
        .unwrap();
        writeln!(file, "# Trailing comment").unwrap();

        let graph = load_graph(file.path()).unwrap();
        assert_eq!(graph.len(), 2);
    }

    #[test]
    fn test_load_legacy_identity_migration() {
        let mut file = NamedTempFile::new().unwrap();
        writeln!(file, r#"{{"id":"t1","kind":"task","title":"Legacy","status":"open","identity":{{"role_id":"r1","motivation_id":"m1"}}}}"#).unwrap();

        let graph = load_graph(file.path()).unwrap();
        let task = graph.get_task("t1").unwrap();
        // The legacy identity should be migrated to an agent hash
        assert!(task.agent.is_some());
    }

    #[test]
    fn test_concurrent_access_with_locking() {
        use std::sync::Arc;
        use std::sync::atomic::{AtomicUsize, Ordering};
        use std::thread;

        // Create a temporary file
        let file = NamedTempFile::new().unwrap();
        let path = Arc::new(file.path().to_path_buf());

        // Initialize with a task
        let mut graph = WorkGraph::new();
        graph.add_node(Node::Task(make_task("t1", "Initial Task")));
        save_graph(&graph, path.as_ref()).unwrap();

        let success_count = Arc::new(AtomicUsize::new(0));
        let mut handles = vec![];

        // Spawn multiple threads that try to read and write concurrently
        for i in 0..10 {
            let path = Arc::clone(&path);
            let success_count = Arc::clone(&success_count);

            let handle = thread::spawn(move || {
                // Each thread loads the graph, adds a task, and saves it back
                if let Ok(mut graph) = load_graph(path.as_ref()) {
                    graph.add_node(Node::Task(make_task(
                        &format!("t{}", i + 2),
                        &format!("Task {}", i + 2),
                    )));
                    if save_graph(&graph, path.as_ref()).is_ok() {
                        success_count.fetch_add(1, Ordering::SeqCst);
                    }
                }
            });

            handles.push(handle);
        }

        // Wait for all threads to complete
        for handle in handles {
            handle.join().unwrap();
        }

        // Verify we can still load the graph without corruption
        let final_graph = load_graph(path.as_ref()).unwrap();

        // At least some operations should have succeeded
        assert!(success_count.load(Ordering::SeqCst) > 0);

        // The graph should be parseable (no EOF errors or corruption)
        assert!(!final_graph.is_empty());
    }

    #[test]
    fn test_load_graph_duplicate_ids_last_wins() {
        let mut file = NamedTempFile::new().unwrap();

        let task1 = Node::Task(make_task("dup", "First version"));
        let task2 = Node::Task(make_task("dup", "Second version"));
        writeln!(file, "{}", serde_json::to_string(&task1).unwrap()).unwrap();
        writeln!(file, "{}", serde_json::to_string(&task2).unwrap()).unwrap();
        file.flush().unwrap();

        let graph = load_graph(file.path()).unwrap();
        // Last-wins semantics: the second definition should be kept
        let task = graph.get_task("dup").unwrap();
        assert_eq!(task.title, "Second version");
    }

    // ---- modify_graph tests ----

    #[test]
    fn test_modify_graph_saves_when_modified() {
        let file = NamedTempFile::new().unwrap();
        let mut graph = WorkGraph::new();
        graph.add_node(Node::Task(make_task("t1", "Task 1")));
        save_graph(&graph, file.path()).unwrap();

        let result = modify_graph(file.path(), |g| {
            g.add_node(Node::Task(make_task("t2", "Task 2")));
            true
        })
        .unwrap();

        assert_eq!(result.len(), 2);

        // Verify it was persisted to disk
        let loaded = load_graph(file.path()).unwrap();
        assert_eq!(loaded.len(), 2);
        assert!(loaded.get_task("t2").is_some());
    }

    #[test]
    fn test_modify_graph_skips_save_when_not_modified() {
        let file = NamedTempFile::new().unwrap();
        let mut graph = WorkGraph::new();
        graph.add_node(Node::Task(make_task("t1", "Task 1")));
        save_graph(&graph, file.path()).unwrap();

        let result = modify_graph(file.path(), |_g| false).unwrap();
        assert_eq!(result.len(), 1);

        // Still the original graph on disk
        let loaded = load_graph(file.path()).unwrap();
        assert_eq!(loaded.len(), 1);
    }

    #[test]
    fn test_modify_graph_concurrent_no_lost_updates() {
        use std::sync::Arc;
        use std::sync::atomic::{AtomicUsize, Ordering};
        use std::thread;

        let file = NamedTempFile::new().unwrap();
        let mut graph = WorkGraph::new();
        graph.add_node(Node::Task(make_task("t0", "Initial")));
        save_graph(&graph, file.path()).unwrap();

        let path = Arc::new(file.path().to_path_buf());
        let success_count = Arc::new(AtomicUsize::new(0));
        let mut handles = vec![];

        // Spawn 10 threads, each atomically adding a task via modify_graph
        for i in 0..10 {
            let path = Arc::clone(&path);
            let success_count = Arc::clone(&success_count);

            let handle = thread::spawn(move || {
                let result = modify_graph(path.as_ref(), |g| {
                    g.add_node(Node::Task(make_task(
                        &format!("t{}", i + 1),
                        &format!("Task {}", i + 1),
                    )));
                    true
                });
                if result.is_ok() {
                    success_count.fetch_add(1, Ordering::SeqCst);
                }
            });
            handles.push(handle);
        }

        for handle in handles {
            handle.join().unwrap();
        }

        let final_graph = load_graph(path.as_ref()).unwrap();

        // All 10 threads should have succeeded
        assert_eq!(success_count.load(Ordering::SeqCst), 10);

        // All 11 tasks should be present (no lost updates)
        assert_eq!(
            final_graph.len(),
            11,
            "modify_graph should prevent lost updates: expected 11 tasks (1 initial + 10 added), got {}",
            final_graph.len()
        );
    }
}
