From 78874df7a6845686fd6df9c2c98e83342ca283d8 Mon Sep 17 00:00:00 2001 From: Elijah Voigt Date: Sat, 21 Feb 2026 21:36:48 -0800 Subject: [PATCH] feat(nbd): implement CLI commands and integration tests (Phase 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire up clap subcommands to storage and display layers: - Cli struct with global --json flag and Create/Read/List/Update subcommands - cmd_create: generates ID, validates priority and deps, writes and prints ticket - cmd_read: looks up ticket by ID and prints it - cmd_list: lists all tickets sorted by priority - cmd_update: reads existing ticket, merges only provided flags, writes and prints - parse_status, parse_ticket_type, parse_deps, validate_deps helpers - 8 integration tests using process::Command against a tempdir - Fix clippy: map_or(false, …) → is_some_and(…) in store.rs Co-Authored-By: Claude Sonnet 4.6 --- nbd/PLAN.md | 14 +- nbd/src/main.rs | 345 ++++++++++++++++++++++++++++++++++++++- nbd/src/store.rs | 2 +- nbd/tests/integration.rs | 272 +++++++++++++++++++++++++++++- 4 files changed, 618 insertions(+), 15 deletions(-) diff --git a/nbd/PLAN.md b/nbd/PLAN.md index 4457dcc..a2d53c0 100644 --- a/nbd/PLAN.md +++ b/nbd/PLAN.md @@ -101,17 +101,19 @@ Output formatting. Wire up `clap` subcommands to storage and display. -- [ ] Define CLI structure with `clap` derive: +- [x] Define CLI structure with `clap` derive: - Global flag: `--json` (all commands) - Subcommand `create`: `--title` (required), `--body`, `--priority`, `--status`, `--type`, `--deps` (comma-separated IDs) - Subcommand `read`: positional `` - Subcommand `list`: no args - Subcommand `update`: positional ``, same optional flags as `create` -- [ ] Implement `cmd_create`: generate ID, validate deps exist, write ticket, print -- [ ] Implement `cmd_read`: find ticket by ID, print -- [ ] Implement `cmd_list`: list all tickets, print -- [ ] Implement `cmd_update`: read existing ticket, merge only provided flags, write, print -- [ ] Integration tests (tempdir): create → read roundtrip, list shows created tickets, update merges correctly, traversal test (run from subdir), error on unknown ID +- [x] Implement `cmd_create`: generate ID, validate deps exist, write ticket, print +- [x] Implement `cmd_read`: find ticket by ID, print +- [x] Implement `cmd_list`: list all tickets, print +- [x] Implement `cmd_update`: read existing ticket, merge only provided flags, write, print +- [x] Added `parse_status`, `parse_ticket_type`, `parse_deps`, `validate_deps` helpers +- [x] Fixed clippy warning in `store.rs`: `map_or(false, ...)` → `is_some_and(...)` +- [x] Integration tests (tempdir): create → read roundtrip, list shows created tickets, update merges correctly, traversal test (run from subdir), error on unknown ID, JSON flag tests, dep replacement test --- diff --git a/nbd/src/main.rs b/nbd/src/main.rs index 381e73a..ffb3efc 100644 --- a/nbd/src/main.rs +++ b/nbd/src/main.rs @@ -1,8 +1,8 @@ -//! `nbd` — CLI tool for managing work tickets, targeted at agent workflows. +//! `nbd` — CLI entry point. //! -//! Tickets are stored as JSON files in `.nbd/tickets/` within the project root, -//! which is located by traversing up from the current working directory (similar -//! to how `git` finds `.git/`). +//! Parses subcommands with `clap` and dispatches to the appropriate command +//! handler. All file I/O is delegated to [`store`] and output is rendered by +//! [`display`]. mod display; mod store; @@ -11,6 +11,339 @@ mod ticket; #[cfg(test)] mod tests; -fn main() { - println!("nbd — ticket manager (not yet implemented)"); +use clap::{Parser, Subcommand}; + +use crate::store::{ensure_tickets_dir, find_nbd_root, list_tickets, read_ticket, write_ticket}; +use crate::ticket::{generate_id, validate_priority, Status, Ticket, TicketType}; + +// ── CLI definition ──────────────────────────────────────────────────────────── + +/// CLI for managing work tickets targeted at agent workflows. +/// +/// Tickets are stored as JSON files in `.nbd/tickets/` inside the nearest +/// ancestor directory that contains a `.nbd/` folder, discovered by traversing +/// upward from the current working directory (like `git` finds `.git/`). +#[derive(Parser)] +#[command(name = "nbd", about = "Manage work tickets for agent workflows")] +struct Cli { + /// Output machine-readable JSON instead of a human-readable table. + #[arg(long, global = true)] + json: bool, + + #[command(subcommand)] + command: Commands, +} + +/// Available `nbd` subcommands. +#[derive(Subcommand)] +enum Commands { + /// Create a new ticket and print it. + Create { + /// Short summary of the work to be done. + #[arg(long)] + title: String, + + /// Long-form description (optional, defaults to empty). + #[arg(long, default_value = "")] + body: String, + + /// Priority on a scale of 0–10 (default: 5). + #[arg(long, default_value_t = 5)] + priority: u8, + + /// Lifecycle status: `todo`, `in_progress`, or `done` (default: `todo`). + #[arg(long, default_value = "todo")] + status: String, + + /// Ticket category: `project`, `feature`, `task`, or `bug` (default: `task`). + #[arg(long = "type", default_value = "task")] + ticket_type: String, + + /// Comma-separated list of dependency ticket IDs (e.g. `a3f9c2,b7d41e`). + #[arg(long)] + deps: Option, + }, + + /// Print a single ticket by ID. + Read { + /// The 6-character hex ticket ID to look up. + id: String, + }, + + /// List all tickets sorted by priority (highest first). + List, + + /// Update fields of an existing ticket and print the result. + /// + /// Only the flags you supply are changed; all other fields retain their + /// current values. + Update { + /// The 6-character hex ticket ID to modify. + id: String, + + /// New title. + #[arg(long)] + title: Option, + + /// New body. + #[arg(long)] + body: Option, + + /// New priority (0–10). + #[arg(long)] + priority: Option, + + /// New status: `todo`, `in_progress`, or `done`. + #[arg(long)] + status: Option, + + /// New ticket type: `project`, `feature`, `task`, or `bug`. + #[arg(long = "type")] + ticket_type: Option, + + /// New comma-separated dependency IDs (replaces the existing list). + #[arg(long)] + deps: Option, + }, +} + +// ── Entry point ─────────────────────────────────────────────────────────────── + +#[async_std::main] +async fn main() { + let cli = Cli::parse(); + if let Err(e) = dispatch(cli).await { + eprintln!("error: {e}"); + std::process::exit(1); + } +} + +/// Route the parsed CLI arguments to the appropriate command handler. +async fn dispatch(cli: Cli) -> store::Result<()> { + match cli.command { + Commands::Create { + title, + body, + priority, + status, + ticket_type, + deps, + } => cmd_create(title, body, priority, status, ticket_type, deps, cli.json).await, + + Commands::Read { id } => cmd_read(id, cli.json).await, + + Commands::List => cmd_list(cli.json).await, + + Commands::Update { + id, + title, + body, + priority, + status, + ticket_type, + deps, + } => { + cmd_update( + id, + title, + body, + priority, + status, + ticket_type, + deps, + cli.json, + ) + .await + } + } +} + +// ── Parsing helpers ─────────────────────────────────────────────────────────── + +/// Parse a [`Status`] from its lowercase string representation. +/// +/// Accepts `"todo"`, `"in_progress"`, and `"done"`. +/// +/// # Errors +/// +/// Returns an error if `s` does not match a known variant. +fn parse_status(s: &str) -> store::Result { + match s { + "todo" => Ok(Status::Todo), + "in_progress" => Ok(Status::InProgress), + "done" => Ok(Status::Done), + other => Err(format!( + "unknown status '{other}'; expected 'todo', 'in_progress', or 'done'" + ) + .into()), + } +} + +/// Parse a [`TicketType`] from its lowercase string representation. +/// +/// Accepts `"project"`, `"feature"`, `"task"`, and `"bug"`. +/// +/// # Errors +/// +/// Returns an error if `s` does not match a known variant. +fn parse_ticket_type(s: &str) -> store::Result { + match s { + "project" => Ok(TicketType::Project), + "feature" => Ok(TicketType::Feature), + "task" => Ok(TicketType::Task), + "bug" => Ok(TicketType::Bug), + other => Err(format!( + "unknown ticket type '{other}'; expected 'project', 'feature', 'task', or 'bug'" + ) + .into()), + } +} + +/// Split a comma-separated dependency string into a `Vec`. +/// +/// Returns an empty `Vec` when `deps` is `None` or an empty string. +fn parse_deps(deps: Option<&str>) -> Vec { + match deps { + None | Some("") => Vec::new(), + Some(s) => s.split(',').map(|id| id.trim().to_string()).collect(), + } +} + +/// Verify that every ID in `deps` refers to an existing ticket. +/// +/// # Errors +/// +/// Returns an error that names the first missing dependency. +async fn validate_deps(root: &std::path::Path, deps: &[String]) -> store::Result<()> { + for dep_id in deps { + read_ticket(root, dep_id).await.map_err( + |_| -> Box { + format!("dependency '{dep_id}' not found").into() + }, + )?; + } + Ok(()) +} + +// ── Command handlers ────────────────────────────────────────────────────────── + +/// Create a new ticket, persist it, and print it. +/// +/// Generates a fresh ID, validates `priority` and all dependency IDs, then +/// writes the ticket to `.nbd/tickets/{id}.json`. +async fn cmd_create( + title: String, + body: String, + priority: u8, + status: String, + ticket_type: String, + deps: Option, + json: bool, +) -> store::Result<()> { + validate_priority(priority) + .map_err(|e| -> Box { e.into() })?; + + let root = find_nbd_root()?; + ensure_tickets_dir(&root).await?; + + let dependencies = parse_deps(deps.as_deref()); + validate_deps(&root, &dependencies).await?; + + let id = generate_id(); + let mut ticket = Ticket::new(id, title); + ticket.body = body; + ticket.priority = priority; + ticket.status = parse_status(&status)?; + ticket.ticket_type = parse_ticket_type(&ticket_type)?; + ticket.dependencies = dependencies; + + write_ticket(&root, &ticket).await?; + + if json { + display::print_ticket_json(&ticket); + } else { + display::print_ticket(&ticket); + } + + Ok(()) +} + +/// Read a ticket by ID and print it. +async fn cmd_read(id: String, json: bool) -> store::Result<()> { + let root = find_nbd_root()?; + let ticket = read_ticket(&root, &id).await?; + + if json { + display::print_ticket_json(&ticket); + } else { + display::print_ticket(&ticket); + } + + Ok(()) +} + +/// List all tickets sorted by priority and print them. +async fn cmd_list(json: bool) -> store::Result<()> { + let root = find_nbd_root()?; + let tickets = list_tickets(&root).await?; + + if json { + display::print_list_json(&tickets); + } else { + display::print_list(&tickets); + } + + Ok(()) +} + +/// Update the specified fields of an existing ticket, persist it, and print it. +/// +/// Only the flags explicitly passed on the command line are applied; all other +/// fields keep their current values. +#[allow(clippy::too_many_arguments)] +async fn cmd_update( + id: String, + title: Option, + body: Option, + priority: Option, + status: Option, + ticket_type: Option, + deps: Option, + json: bool, +) -> store::Result<()> { + let root = find_nbd_root()?; + let mut ticket = read_ticket(&root, &id).await?; + + if let Some(t) = title { + ticket.title = t; + } + if let Some(b) = body { + ticket.body = b; + } + if let Some(p) = priority { + validate_priority(p) + .map_err(|e| -> Box { e.into() })?; + ticket.priority = p; + } + if let Some(s) = status { + ticket.status = parse_status(&s)?; + } + if let Some(tt) = ticket_type { + ticket.ticket_type = parse_ticket_type(&tt)?; + } + if deps.is_some() { + let dependencies = parse_deps(deps.as_deref()); + validate_deps(&root, &dependencies).await?; + ticket.dependencies = dependencies; + } + + write_ticket(&root, &ticket).await?; + + if json { + display::print_ticket_json(&ticket); + } else { + display::print_ticket(&ticket); + } + + Ok(()) } diff --git a/nbd/src/store.rs b/nbd/src/store.rs index 8542dcc..e13f665 100644 --- a/nbd/src/store.rs +++ b/nbd/src/store.rs @@ -154,7 +154,7 @@ pub async fn list_tickets(root: &Path) -> Result> { while let Some(entry) = entries.next().await { let entry = entry?; let path = entry.path(); - if path.extension().map_or(false, |ext| ext == "json") { + if path.extension().is_some_and(|ext| ext == "json") { let bytes = fs::read(&path).await?; let ticket: Ticket = serde_json::from_slice(&bytes)?; tickets.push(ticket); diff --git a/nbd/tests/integration.rs b/nbd/tests/integration.rs index 12056c8..92dcf7a 100644 --- a/nbd/tests/integration.rs +++ b/nbd/tests/integration.rs @@ -1,5 +1,273 @@ //! Integration tests for `nbd`. //! //! Tests full command flows (create → read → list → update) against a real -//! temporary directory, and verifies that directory traversal correctly locates -//! `.nbd/` from a subdirectory. +//! temporary directory and verifies that directory traversal correctly locates +//! `.nbd/` when the binary is run from a nested subdirectory. + +use std::fs; +use std::path::{Path, PathBuf}; +use tempfile::TempDir; + +// ── Test environment helper ─────────────────────────────────────────────────── + +/// A temporary project environment with `.nbd/tickets/` already initialised. +struct TestEnv { + /// Keep `TempDir` alive — dropping it would delete the directory. + _tmp: TempDir, + /// The project root (the directory that contains `.nbd/`). + pub root: PathBuf, +} + +impl TestEnv { + /// Create a fresh temporary environment with `.nbd/tickets/` ready. + fn new() -> Self { + let tmp = tempfile::tempdir().expect("failed to create tempdir"); + let root = tmp.path().to_path_buf(); + fs::create_dir_all(root.join(".nbd").join("tickets")) + .expect("failed to create .nbd/tickets/"); + TestEnv { _tmp: tmp, root } + } + + /// Spawn `nbd` with `args`, using `dir` as the working directory. + fn run_from(&self, dir: &Path, args: &[&str]) -> std::process::Output { + std::process::Command::new(env!("CARGO_BIN_EXE_nbd")) + .args(args) + .current_dir(dir) + .output() + .expect("failed to spawn nbd") + } + + /// Spawn `nbd` with `args` from the project root. + fn run(&self, args: &[&str]) -> std::process::Output { + self.run_from(&self.root, args) + } + + /// Run `nbd create` with the given extra arguments and return the printed + /// ticket ID extracted from stdout. + fn create(&self, extra_args: &[&str]) -> String { + let mut args = vec!["create"]; + args.extend_from_slice(extra_args); + let output = self.run(&args); + assert!( + output.status.success(), + "create failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8(output.stdout).unwrap(); + stdout + .lines() + .find(|l| l.trim_start().starts_with("ID:")) + .and_then(|l| l.split_whitespace().last()) + .expect("could not extract ID from create output") + .to_string() + } +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +/// `create` then `read` produces a ticket with the expected field values. +#[test] +fn create_and_read_roundtrip() { + let env = TestEnv::new(); + + let id = env.create(&[ + "--title", + "Fix login bug", + "--body", + "Users cannot log in with email addresses containing +", + "--priority", + "8", + "--type", + "bug", + ]); + + let read = env.run(&["read", &id]); + assert!( + read.status.success(), + "read failed: {}", + String::from_utf8_lossy(&read.stderr) + ); + let stdout = String::from_utf8(read.stdout).unwrap(); + assert!(stdout.contains(&id), "output should contain the ID"); + assert!( + stdout.contains("Fix login bug"), + "output should contain the title" + ); + assert!( + stdout.contains("Users cannot log in"), + "output should contain the body" + ); + assert!(stdout.contains('8'), "output should contain the priority"); + assert!(stdout.contains("bug"), "output should contain the type"); +} + +/// `list` displays all previously created tickets. +#[test] +fn list_shows_created_tickets() { + let env = TestEnv::new(); + + env.create(&["--title", "First ticket"]); + env.create(&["--title", "Second ticket"]); + + let list = env.run(&["list"]); + assert!(list.status.success()); + let stdout = String::from_utf8(list.stdout).unwrap(); + assert!( + stdout.contains("First ticket"), + "list should contain first ticket" + ); + assert!( + stdout.contains("Second ticket"), + "list should contain second ticket" + ); +} + +/// `update` changes only the specified fields; others are preserved. +#[test] +fn update_merges_correctly() { + let env = TestEnv::new(); + + let id = env.create(&[ + "--title", + "Original title", + "--priority", + "5", + "--type", + "task", + ]); + + // Update only the status; title and priority should be unchanged. + let update = env.run(&["update", &id, "--status", "in_progress"]); + assert!( + update.status.success(), + "update failed: {}", + String::from_utf8_lossy(&update.stderr) + ); + + let read = env.run(&["read", &id]); + let stdout = String::from_utf8(read.stdout).unwrap(); + assert!( + stdout.contains("in_progress"), + "status should be updated to in_progress" + ); + assert!( + stdout.contains("Original title"), + "title should be unchanged" + ); + assert!(stdout.contains('5'), "priority should be unchanged"); +} + +/// `read` works when executed from a nested subdirectory (traversal test). +#[test] +fn traversal_from_subdir() { + let env = TestEnv::new(); + + let id = env.create(&["--title", "Traversal test"]); + + // Create a nested subdirectory two levels deep. + let subdir = env.root.join("sub").join("dir"); + fs::create_dir_all(&subdir).unwrap(); + + let read = env.run_from(&subdir, &["read", &id]); + assert!( + read.status.success(), + "traversal failed: {}", + String::from_utf8_lossy(&read.stderr) + ); + let stdout = String::from_utf8(read.stdout).unwrap(); + assert!( + stdout.contains("Traversal test"), + "output should contain the title" + ); +} + +/// `read` with an unknown ID exits non-zero and mentions the ID in stderr. +#[test] +fn error_on_unknown_id() { + let env = TestEnv::new(); + + let result = env.run(&["read", "ffffff"]); + assert!( + !result.status.success(), + "reading an unknown ID should exit non-zero" + ); + let stderr = String::from_utf8(result.stderr).unwrap(); + assert!( + stderr.contains("ffffff"), + "error message should mention the ticket ID, got: {stderr}" + ); +} + +/// `create --json` outputs valid JSON containing the correct field values. +#[test] +fn create_with_json_flag() { + let env = TestEnv::new(); + + let output = env.run(&[ + "create", + "--title", + "JSON test", + "--priority", + "7", + "--json", + ]); + assert!(output.status.success()); + + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = + serde_json::from_str(&stdout).expect("--json output should be valid JSON"); + assert_eq!(parsed["title"], "JSON test"); + assert_eq!(parsed["priority"], 7); +} + +/// `list --json` outputs a valid JSON array with one object per ticket. +#[test] +fn list_with_json_flag() { + let env = TestEnv::new(); + + env.create(&["--title", "First"]); + env.create(&["--title", "Second"]); + + let output = env.run(&["list", "--json"]); + assert!(output.status.success()); + + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = + serde_json::from_str(&stdout).expect("--json list output should be valid JSON"); + assert!(parsed.is_array(), "output should be a JSON array"); + assert_eq!( + parsed.as_array().unwrap().len(), + 2, + "array should contain exactly two tickets" + ); +} + +/// `update --deps` replaces the dependency list. +#[test] +fn update_deps_replaces_list() { + let env = TestEnv::new(); + + // Create two tickets to use as deps. + let dep1 = env.create(&["--title", "Dep one"]); + let dep2 = env.create(&["--title", "Dep two"]); + let main_id = env.create(&["--title", "Main ticket"]); + + // Add dep1 as a dependency. + let update = env.run(&["update", &main_id, "--deps", &dep1]); + assert!( + update.status.success(), + "update with deps failed: {}", + String::from_utf8_lossy(&update.stderr) + ); + + // Replace with dep2. + let update2 = env.run(&["update", &main_id, "--deps", &dep2]); + assert!(update2.status.success()); + + let read = env.run(&["read", &main_id, "--json"]); + let stdout = String::from_utf8(read.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let deps = parsed["dependencies"].as_array().unwrap(); + assert_eq!(deps.len(), 1, "should have exactly one dependency"); + assert_eq!(deps[0], dep2, "dependency should be dep2"); +}