zbobr_fix-52-update-commands

zbobr-dispatcher/src/cli.rs

@@ -91,9 +91,9 @@ pub fn resolve_config_location(
 /// This includes only dispatcher and executor config, not backend-specific settings.
 #[derive(Args, Clone)]
 pub struct GlobalArgs {
-    /// Enable log output to stderr
-    #[arg(long)]
-    pub logs: bool,
+    /// Enable log output to stderr (accepts optional value: --logs, --logs=true, --logs=false)
+    #[arg(long, num_args = 0..=1, default_missing_value = "true", require_equals = true, action = clap::ArgAction::Append)]
+    pub logs: Vec<bool>,
 
     #[command(
         flatten,
@@ -131,18 +131,32 @@ pub fn parse_cli<C: Parser + clap::CommandFactory>(
         .map(|s| s.get_name().to_owned())
         .collect();
 
+    // Classify each global arg: Flag (no value), RequiredValue, or OptionalValue.
+    // OptionalValue means `num_args = 0..=1` — the next token may or may not be a value.
+    #[derive(Clone, Copy)]
+    enum ArgValence {
+        Flag,
+        RequiredValue,
+        OptionalValue,
+    }
     let global_tmp = GlobalArgs::augment_args(clap::Command::new(""));
-    let global_flags: std::collections::HashMap<String, bool> = global_tmp
+    let global_flags: std::collections::HashMap<String, ArgValence> = global_tmp
         .get_arguments()
         .flat_map(|a| {
-            let takes_value = !matches!(
+            let valence = if matches!(
                 a.get_action(),
                 clap::ArgAction::SetTrue | clap::ArgAction::SetFalse | clap::ArgAction::Count
-            );
-            let long_entry = a.get_long().map(|long| (format!("--{long}"), takes_value));
+            ) {
+                ArgValence::Flag
+            } else if a.get_num_args().is_some_and(|r| r.min_values() == 0) {
+                ArgValence::OptionalValue
+            } else {
+                ArgValence::RequiredValue
+            };
+            let long_entry = a.get_long().map(|long| (format!("--{long}"), valence));
             let short_entry = a
                 .get_short()
-                .map(|short| (format!("-{short}"), takes_value));
+                .map(|short| (format!("-{short}"), valence));
             long_entry.into_iter().chain(short_entry)
         })
         .collect();
@@ -153,6 +167,11 @@ pub fn parse_cli<C: Parser + clap::CommandFactory>(
         return C::from_arg_matches(&m).unwrap_or_else(|e| e.exit());
     }
 
+    // Helper: returns true if the token is a valid boolean literal for optional-value args.
+    fn is_bool_literal(s: &str) -> bool {
+        s.eq_ignore_ascii_case("true") || s.eq_ignore_ascii_case("false")
+    }
+
     let mut before_sub = vec![raw_args[0].clone()];
     let mut sub_and_after: Vec<String> = Vec::new();
     let mut found_sub = false;
@@ -165,10 +184,28 @@ pub fn parse_cli<C: Parser + clap::CommandFactory>(
             if subcommands.contains(arg.as_str()) {
                 found_sub = true;
                 sub_and_after.push(arg.clone());
+                i += 1;
+                continue;
+            }
+            // Normalize optional-value globals before the subcommand.
+            // For `--logs false`, join into `--logs=false` so clap (with
+            // require_equals) sees an attached value.
+            let base = arg.split('=').next().unwrap_or(arg);
+            if let Some(ArgValence::OptionalValue) = global_flags.get(base).copied() {
+                if !arg.contains('=')
+                    && i + 1 < raw_args.len()
+                    && is_bool_literal(&raw_args[i + 1])
+                {
+                    before_sub.push(format!("{}={}", arg, raw_args[i + 1]));
+                    i += 2;
+                } else {
+                    before_sub.push(arg.clone());
+                    i += 1;
+                }
             } else {
                 before_sub.push(arg.clone());
+                i += 1;
             }
-            i += 1;
             continue;
         }
 
@@ -184,22 +221,31 @@ pub fn parse_cli<C: Parser + clap::CommandFactory>(
                 global_flags
                     .get(short_key)
                     .copied()
-                    .filter(|&takes_value| takes_value)
+                    .filter(|v| matches!(v, ArgValence::RequiredValue | ArgValence::OptionalValue))
             } else {
                 None
             }
         });
-        if let Some(takes_value) = matched {
+        if let Some(valence) = matched {
             if arg.contains('=')
                 || (arg.starts_with('-') && !arg.starts_with("--") && arg.len() > 2)
             {
                 // Attached value: --config=val or -cval
                 before_sub.push(arg.clone());
                 i += 1;
-            } else if takes_value && i + 1 < raw_args.len() {
+            } else if matches!(valence, ArgValence::RequiredValue) && i + 1 < raw_args.len() {
                 before_sub.push(arg.clone());
                 before_sub.push(raw_args[i + 1].clone());
                 i += 2;
+            } else if matches!(valence, ArgValence::OptionalValue)
+                && i + 1 < raw_args.len()
+                && is_bool_literal(&raw_args[i + 1])
+            {
+                // Optional value: consume the next token only if it is a
+                // valid boolean literal. Join with '=' so clap sees it as
+                // an attached value (required by require_equals = true).
+                before_sub.push(format!("{}={}", arg, raw_args[i + 1]));
+                i += 2;
             } else {
                 before_sub.push(arg.clone());
                 i += 1;
@@ -1153,7 +1199,7 @@ impl ZbobrDispatcher {
 
             // Phase 2: use select_runnable_task to pick the highest-priority RunStage candidate
             // and run its stage.  This shares the exact same ready-task selection logic as the
-            // `task list --select` CLI flag.
+            // `task select` CLI subcommand.
             if let Some(task) = select_runnable_task(workflow, &all_tasks) {
                 let action = match workflow.resolve_next_action(task) {
                     Ok(a) => a,
@@ -2297,8 +2343,8 @@ mod tests {
         );
         let action = logs_arg.unwrap().get_action();
         assert!(
-            matches!(action, clap::ArgAction::SetTrue),
-            "--logs should be a boolean flag (SetTrue action)"
+            matches!(action, clap::ArgAction::Append),
+            "--logs should use Append action for last-value-wins semantics"
         );
     }
 }

zbobr/src/commands.rs

@@ -87,10 +87,9 @@ pub enum TaskSubcommand {
         /// Output as JSON array
         #[arg(long)]
         json: bool,
-        /// Print the ID of the highest-priority ready task and exit; exits with code 1 if none
-        #[arg(long)]
-        select: bool,
     },
+    /// Print the ID of the highest-priority ready task and exit; exits with code 1 if none
+    Select,
     /// Show a task by ID (or list all tasks if no ID given)
     Show {
         /// Task ID
@@ -326,11 +325,7 @@ async fn run_task_subcommand(
             }
             println!("Created task #{}", id);
         }
-        TaskSubcommand::List {
-            state,
-            json,
-            select,
-        } => {
+        TaskSubcommand::List { state, json } => {
             let state_filter = state
                 .as_deref()
                 .map(str::parse::<zbobr_api::State>)
@@ -348,29 +343,6 @@ async fn run_task_subcommand(
             }
             tasks.sort_by_key(|t| t.id);
 
-            if select {
-                let eligible = eligible_runnable_tasks(zbobr.workflow(), &tasks);
-                tracing::info!(
-                    "Select requested: {} eligible runnable task(s)",
-                    eligible.len()
-                );
-                if eligible.is_empty() {
-                    tracing::info!("No runnable tasks available for selection");
-                    std::process::exit(1);
-                }
-                match select_runnable_task(zbobr.workflow(), &tasks) {
-                    Some(task) => {
-                        tracing::info!("Selected task #{} for task list select", task.id);
-                        println!("{}", task.id)
-                    }
-                    None => {
-                        tracing::info!("No runnable tasks available for selection");
-                        std::process::exit(1)
-                    }
-                }
-                return Ok(());
-            }
-
             if json {
                 let entries: Vec<TaskListEntry> = tasks.iter().map(TaskListEntry::from).collect();
                 println!("{}", serde_json::to_string_pretty(&entries)?);
@@ -385,6 +357,34 @@ async fn run_task_subcommand(
                 }
             }
         }
+        TaskSubcommand::Select => {
+            let weak_tasks = task_backend.list_tasks().await?;
+            let mut tasks = Vec::new();
+            for w in &weak_tasks {
+                tasks.push(w.snapshot(false).await?);
+            }
+            tasks.sort_by_key(|t| t.id);
+
+            let eligible = eligible_runnable_tasks(zbobr.workflow(), &tasks);
+            tracing::info!(
+                "Select requested: {} eligible runnable task(s)",
+                eligible.len()
+            );
+            if eligible.is_empty() {
+                tracing::info!("No runnable tasks available for selection");
+                std::process::exit(1);
+            }
+            match select_runnable_task(zbobr.workflow(), &tasks) {
+                Some(task) => {
+                    tracing::info!("Selected task #{} for task select", task.id);
+                    println!("{}", task.id)
+                }
+                None => {
+                    tracing::info!("No runnable tasks available for selection");
+                    std::process::exit(1)
+                }
+            }
+        }
         TaskSubcommand::Show { id, json } => {
             if let Some(id) = id {
                 let weak = task_backend.get_task(id).await?;

zbobr/src/main.rs

@@ -39,9 +39,9 @@ struct RootConfig {
 
 #[derive(Parser)]
 struct Cli {
-    /// Enable log output to stderr
-    #[arg(long)]
-    logs: bool,
+    /// Enable log output to stderr (accepts optional value: --logs, --logs=true, --logs=false)
+    #[arg(long, num_args = 0..=1, default_missing_value = "true", require_equals = true, action = clap::ArgAction::Append)]
+    logs: Vec<bool>,
 
     #[command(
         flatten,
@@ -56,6 +56,11 @@ struct Cli {
     command: Command,
 }
 
+/// Resolve the final logs boolean from collected `--logs` values (last value wins).
+fn resolve_logs(values: &[bool]) -> bool {
+    values.last().copied().unwrap_or(false)
+}
+
 #[tokio::main]
 async fn main() -> anyhow::Result<()> {
     let _ = rustls::crypto::ring::default_provider().install_default();
@@ -69,7 +74,8 @@ async fn main() -> anyhow::Result<()> {
         Easiest way: export GH_TOKEN=$(gh auth token)",
     );
 
-    let filter = if cli.logs {
+    let logs_enabled = resolve_logs(&cli.logs);
+    let filter = if logs_enabled {
         tracing_subscriber::EnvFilter::try_from_default_env().unwrap_or_else(|_| "info".into())
     } else {
         "off".into()
@@ -189,15 +195,97 @@ mod tests {
         }
     }
 
+    #[test]
+    fn task_select_parses_without_arguments() {
+        let cli = TestCli::try_parse_from(["zbobr", "task", "select"]).unwrap();
+        match cli.command {
+            commands::Command::Task { subcommand } => match subcommand {
+                commands::TaskSubcommand::Select => {}
+                _ => panic!("expected Select subcommand"),
+            },
+            _ => panic!("expected Task command"),
+        }
+    }
+
+    #[test]
+    fn task_list_no_longer_accepts_select_flag() {
+        let result = TestCli::try_parse_from(["zbobr", "task", "list", "--select"]);
+        assert!(
+            result.is_err(),
+            "task list should no longer accept --select flag"
+        );
+    }
+
     #[test]
     fn logs_flag_defaults_to_false() {
         let cli = Cli::try_parse_from(["zbobr", "task", "process", "--select"]).unwrap();
-        assert!(!cli.logs, "logs flag should default to false");
+        assert!(!resolve_logs(&cli.logs), "logs should default to false");
     }
 
     #[test]
     fn logs_flag_parses_when_present() {
         let cli = Cli::try_parse_from(["zbobr", "--logs", "task", "process", "--select"]).unwrap();
-        assert!(cli.logs, "--logs flag should set logs to true");
+        assert!(resolve_logs(&cli.logs), "--logs should set logs to true");
+    }
+
+    #[test]
+    fn logs_flag_explicit_true() {
+        let cli =
+            Cli::try_parse_from(["zbobr", "--logs=true", "task", "process", "--select"]).unwrap();
+        assert!(resolve_logs(&cli.logs), "--logs=true should be true");
+    }
+
+    #[test]
+    fn logs_flag_explicit_false() {
+        let cli =
+            Cli::try_parse_from(["zbobr", "--logs=false", "task", "process", "--select"]).unwrap();
+        assert!(
+            !resolve_logs(&cli.logs),
+            "--logs=false should be false"
+        );
+    }
+
+    #[test]
+    fn logs_flag_last_value_wins() {
+        let cli = Cli::try_parse_from([
+            "zbobr",
+            "--logs",
+            "--logs=false",
+            "task",
+            "process",
+            "--select",
+        ])
+        .unwrap();
+        assert!(
+            !resolve_logs(&cli.logs),
+            "--logs --logs=false: last value wins = false"
+        );
+    }
+
+    #[test]
+    fn logs_flag_last_value_wins_true() {
+        let cli = Cli::try_parse_from([
+            "zbobr",
+            "--logs=false",
+            "--logs",
+            "task",
+            "process",
+            "--select",
+        ])
+        .unwrap();
+        assert!(
+            resolve_logs(&cli.logs),
+            "--logs=false --logs: last value wins = true"
+        );
+    }
+
+    #[test]
+    fn logs_invalid_value_rejected() {
+        let result =
+            Cli::try_parse_from(["zbobr", "--logs=maybe", "task", "process", "--select"]);
+        assert!(
+            result.is_err(),
+            "--logs=maybe should be rejected as an invalid boolean"
+        );
     }
 }