From 7c9d058ce62a7305a310dbbd25816354194d5ecc Mon Sep 17 00:00:00 2001 From: Elijah Voigt Date: Mon, 2 Mar 2026 10:48:33 -0800 Subject: [PATCH] feat(nbd): scope nbd next/ready by dependency subtree (#818598) Add an optional positional `` argument to `nbd next` and `nbd ready` that restricts results to the dependency subtree of the given ticket. The scoping ticket itself is excluded from results. - CLI: add `id: Option` to `Commands::Next` and `Commands::Ready` - Logic: build a `TicketGraph`, call `subtree()`, restrict candidate pool - Tests: 4 new integration tests covering scoped next/ready and --filter Co-Authored-By: Claude Sonnet 4.6 --- nbd/.nbd/tickets/818598.md | 2 +- nbd/src/main.rs | 99 +++++++++++++++++- nbd/tests/integration.rs | 202 +++++++++++++++++++++++++++++++++++++ 3 files changed, 298 insertions(+), 5 deletions(-) diff --git a/nbd/.nbd/tickets/818598.md b/nbd/.nbd/tickets/818598.md index 266fb3d..b1a9e83 100644 --- a/nbd/.nbd/tickets/818598.md +++ b/nbd/.nbd/tickets/818598.md @@ -1,7 +1,7 @@ +++ title = "Scope nbd next and nbd ready by dependency subtree" priority = 5 -status = "todo" +status = "done" ticket_type = "feature" dependencies = [] +++ diff --git a/nbd/src/main.rs b/nbd/src/main.rs index 50353df..331bf7f 100644 --- a/nbd/src/main.rs +++ b/nbd/src/main.rs @@ -132,7 +132,18 @@ enum Commands { /// A ticket is ready when its status is not `done` and every ticket it /// depends on has status `done`. Tickets with no dependencies and status /// `todo` or `in_progress` are always ready. + /// + /// With an optional ``, restricts results to the dependency subtree of + /// that ticket — only ready tickets that `` depends on (directly or + /// transitively) are returned. The scoping ticket itself is excluded. Ready { + /// Optional ticket ID or unique prefix to scope results to its dependency subtree. + /// + /// When provided, only ready tickets within the subtree that `` + /// depends on (directly or transitively) are returned. The ticket + /// identified by `` itself is never included in the results. + id: Option, + /// Filter ready tickets by field: repeatable `key=value` pairs. /// /// Applied after the ready check — narrows within already-ready tickets. @@ -147,8 +158,20 @@ enum Commands { /// depends on has status `done`. Returns the single highest-priority /// ready ticket, optionally narrowed by `--filter KEY=VALUE`. /// + /// With an optional ``, restricts results to the dependency subtree of + /// that ticket — only the highest-priority ready ticket that `` depends + /// on (directly or transitively) is returned. The scoping ticket itself is + /// excluded. + /// /// Exits 0 even when no ready ticket exists. Next { + /// Optional ticket ID or unique prefix to scope results to its dependency subtree. + /// + /// When provided, only the highest-priority ready ticket within the + /// subtree that `` depends on (directly or transitively) is + /// returned. The ticket identified by `` itself is never returned. + id: Option, + /// Filter ready tickets: key=value pairs (repeatable). /// AND between different keys, OR within same key. #[arg(long = "filter", value_name = "KEY=VALUE")] @@ -305,9 +328,9 @@ async fn dispatch(cli: Cli) -> store::Result<()> { Commands::Init => cmd_init(cli.json).await, - Commands::Next { filter } => cmd_next(filter, cli.json).await, + Commands::Next { id, filter } => cmd_next(id, filter, cli.json).await, - Commands::Ready { filter } => cmd_ready(filter, cli.json).await, + Commands::Ready { id, filter } => cmd_ready(id, filter, cli.json).await, Commands::Migrate { dry_run, filter } => cmd_migrate(filter, dry_run, cli.json).await, @@ -469,8 +492,17 @@ async fn cmd_init(json: bool) -> store::Result<()> { /// Missing dependency IDs are treated conservatively — the ticket is **not** /// ready if any dep cannot be resolved. /// +/// When `scope_id` is `Some`, results are restricted to the dependency subtree +/// of the identified ticket — only ready tickets that the scoping ticket depends +/// on (directly or transitively) are returned. The scoping ticket itself is +/// excluded from the results. +/// /// `filter_args` are applied after the ready check, narrowing the results. -async fn cmd_ready(filter_args: Vec, json: bool) -> store::Result<()> { +async fn cmd_ready( + scope_id: Option, + filter_args: Vec, + json: bool, +) -> store::Result<()> { let filter = crate::filter::parse_filters(&filter_args)?; let root = find_nbd_root()?; let all = list_tickets_cached(&root).await?; @@ -487,9 +519,34 @@ async fn cmd_ready(filter_args: Vec, json: bool) -> store::Result<()> { .map(|t| t.id.as_str()) .collect(); + // If a scope ID was provided, resolve it and build the dependency subtree. + // The candidate pool is restricted to tickets within that subtree (excluding + // the scoping ticket itself). + let scope_subtree: Option> = match scope_id { + Some(raw) => { + let resolved = resolve_id(&root, &raw).await?; + let graph = TicketGraph::build(&all); + // subtree() includes the root itself; exclude it from candidates. + let ids: std::collections::HashSet = graph + .subtree(&resolved) + .into_iter() + .filter(|&id| id != resolved.as_str()) + .map(|id| id.to_string()) + .collect(); + Some(ids) + } + None => None, + }; + let ready: Vec<&crate::ticket::Ticket> = all .iter() .filter(|t| { + // If a subtree scope was set, only include tickets in that scope. + if let Some(ref subtree) = scope_subtree { + if !subtree.contains(&t.id) { + return false; + } + } t.status != crate::ticket::Status::Done && t.status != crate::ticket::Status::Closed && t.status != crate::ticket::Status::Archived @@ -516,10 +573,19 @@ async fn cmd_ready(filter_args: Vec, json: bool) -> store::Result<()> { /// every dependency has status `done`. Missing dependency IDs make a ticket /// **not** ready. /// +/// When `scope_id` is `Some`, results are restricted to the dependency subtree +/// of the identified ticket — only the highest-priority ready ticket that the +/// scoping ticket depends on (directly or transitively) is returned. The scoping +/// ticket itself is excluded from the results. +/// /// With `--json`, outputs `{"next": {...ticket...}}` when a ticket is found or /// `{"next": null}` when none are ready, so callers always receive an object /// with a `"next"` key. -async fn cmd_next(filter_args: Vec, json: bool) -> store::Result<()> { +async fn cmd_next( + scope_id: Option, + filter_args: Vec, + json: bool, +) -> store::Result<()> { let filter = crate::filter::parse_filters(&filter_args)?; let root = find_nbd_root()?; let all = list_tickets_cached(&root).await?; // sorted by priority desc @@ -534,7 +600,32 @@ async fn cmd_next(filter_args: Vec, json: bool) -> store::Result<()> { .map(|t| t.id.as_str()) .collect(); + // If a scope ID was provided, resolve it and build the dependency subtree. + // The candidate pool is restricted to tickets within that subtree (excluding + // the scoping ticket itself). + let scope_subtree: Option> = match scope_id { + Some(raw) => { + let resolved = resolve_id(&root, &raw).await?; + let graph = TicketGraph::build(&all); + // subtree() includes the root itself; exclude it from candidates. + let ids: std::collections::HashSet = graph + .subtree(&resolved) + .into_iter() + .filter(|&id| id != resolved.as_str()) + .map(|id| id.to_string()) + .collect(); + Some(ids) + } + None => None, + }; + let next = all.iter().find(|t| { + // If a subtree scope was set, only include tickets in that scope. + if let Some(ref subtree) = scope_subtree { + if !subtree.contains(&t.id) { + return false; + } + } t.status != crate::ticket::Status::Done && t.status != crate::ticket::Status::Closed && t.status != crate::ticket::Status::Archived diff --git a/nbd/tests/integration.rs b/nbd/tests/integration.rs index 7be819e..8859742 100644 --- a/nbd/tests/integration.rs +++ b/nbd/tests/integration.rs @@ -2197,3 +2197,205 @@ fn version_flag_exits_zero_with_semver() { "--version should contain '+' separator: {stdout}" ); } + +// ── Scoped next / ready tests ───────────────────────────────────────────────── + +/// `nbd next ` returns the highest-priority ready dep within the subtree, +/// not the scoping ticket itself and not any unrelated ticket. +/// +/// Graph: P → [A, B], A → [C]. C is done. So: +/// - A is ready (C is done) +/// - B is ready (no deps) +/// - P is blocked (A and B not done) +/// `nbd next P` with A at priority 8 and B at priority 3 should return A. +#[test] +fn test_next_scoped_by_id() { + let env = TestEnv::new(); + + // Create leaf ticket C (done). + let c_id = env.create(&["--title", "C", "--priority", "5", "--type", "task"]); + env.run(&["update", &c_id, "--status", "done"]); + + // Create A (depends on C, priority 8). + let a_id = env.create(&[ + "--title", + "A", + "--priority", + "8", + "--type", + "task", + "--deps", + &c_id, + ]); + + // Create B (no deps, priority 3). + let b_id = env.create(&["--title", "B", "--priority", "3", "--type", "task"]); + + // Create unrelated ticket U that should never appear. + env.create(&["--title", "Unrelated", "--priority", "10", "--type", "task"]); + + // Create P (project, depends on A and B). + let deps = format!("{a_id},{b_id}"); + let p_id = env.create(&[ + "--title", + "P", + "--priority", + "5", + "--type", + "project", + "--deps", + &deps, + ]); + + // `nbd next P --json` should return A (highest-priority ready dep of P). + let output = env.run(&["next", &p_id, "--json"]); + assert!( + output.status.success(), + "next scoped failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + 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"); + + let next_id = parsed["next"]["id"] + .as_str() + .expect("next should be non-null"); + assert_eq!( + next_id, a_id, + "next scoped to P should return A, got {next_id}" + ); + + // P itself and Unrelated must not appear. + assert_ne!(next_id, p_id, "scoping ticket must not be returned"); +} + +/// `nbd ready ` returns all ready deps within the subtree of ``. +/// +/// Graph: P → [A, B], A → [C]. C is done. B has no deps. +/// - A is ready (C done) +/// - B is ready (no deps) +/// `nbd ready P` should return exactly [A, B]. +#[test] +fn test_ready_scoped_by_id() { + let env = TestEnv::new(); + + let c_id = env.create(&["--title", "C", "--priority", "5"]); + env.run(&["update", &c_id, "--status", "done"]); + + let a_id = env.create(&["--title", "A", "--priority", "7", "--deps", &c_id]); + let b_id = env.create(&["--title", "B", "--priority", "4"]); + + // Unrelated ticket with high priority — must not appear. + env.create(&["--title", "Unrelated", "--priority", "10"]); + + let deps = format!("{a_id},{b_id}"); + let p_id = env.create(&["--title", "P", "--type", "project", "--deps", &deps]); + + let output = env.run(&["ready", &p_id, "--json"]); + assert!( + output.status.success(), + "ready scoped failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + 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"); + + let arr = parsed + .as_array() + .expect("ready --json should return an array"); + let ids: Vec<&str> = arr.iter().map(|v| v["id"].as_str().unwrap()).collect(); + + assert!( + ids.contains(&a_id.as_str()), + "A should be in scoped ready list" + ); + assert!( + ids.contains(&b_id.as_str()), + "B should be in scoped ready list" + ); + assert!( + !ids.contains(&p_id.as_str()), + "scoping ticket P must not appear in results" + ); + // Unrelated should not appear. + assert_eq!( + ids.len(), + 2, + "exactly A and B should be ready in subtree of P, got: {ids:?}" + ); +} + +/// `nbd next ` returns `null` when all deps of the scoping ticket are done. +#[test] +fn test_next_scoped_no_ready() { + let env = TestEnv::new(); + + let a_id = env.create(&["--title", "A", "--priority", "8"]); + let b_id = env.create(&["--title", "B", "--priority", "5"]); + env.run(&["update", &a_id, "--status", "done"]); + env.run(&["update", &b_id, "--status", "done"]); + + let deps = format!("{a_id},{b_id}"); + let p_id = env.create(&["--title", "P", "--type", "project", "--deps", &deps]); + + let output = env.run(&["next", &p_id, "--json"]); + assert!( + output.status.success(), + "next scoped (no ready) failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + 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!( + parsed["next"].is_null(), + "next should be null when all deps are done, got: {}", + parsed["next"] + ); +} + +/// `nbd ready --filter` narrows within the scoped subtree. +/// +/// Graph: P → [A (bug), B (task)]. Both ready. +/// `nbd ready P --filter type=bug` should return only A. +#[test] +fn test_ready_scoped_with_filter() { + let env = TestEnv::new(); + + let a_id = env.create(&["--title", "A", "--priority", "6", "--type", "bug"]); + let b_id = env.create(&["--title", "B", "--priority", "6", "--type", "task"]); + + // Unrelated bug — must not appear even though it matches the filter. + env.create(&[ + "--title", + "Unrelated bug", + "--priority", + "9", + "--type", + "bug", + ]); + + let deps = format!("{a_id},{b_id}"); + let p_id = env.create(&["--title", "P", "--type", "project", "--deps", &deps]); + + let output = env.run(&["ready", &p_id, "--filter", "type=bug", "--json"]); + assert!( + output.status.success(), + "ready scoped+filtered failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + 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"); + + let arr = parsed + .as_array() + .expect("ready --json should return an array"); + let ids: Vec<&str> = arr.iter().map(|v| v["id"].as_str().unwrap()).collect(); + + assert_eq!(ids.len(), 1, "only A (bug) should match; got: {ids:?}"); + assert_eq!(ids[0], a_id, "matched ticket should be A"); +}