From 0870c49dcdee9aca692179b08021242514b3415e Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Wed, 25 Feb 2026 01:50:08 +0900 Subject: [PATCH] feat(cli): add `--output-format` option and rename `--format` to `--input-format` Add `--output-format json|text` option to `check` subcommand with `text` as the default, making output human-readable. Rename `--format` to `--input-format` for symmetry between input/output format options. Co-Authored-By: Claude Opus 4.6 --- src/adapter/check_adapter.rs | 61 ++++++++++++++++++++++++--- src/cli/mod.rs | 30 +++++++++---- src/cli/route.rs | 53 ++++++++++++++--------- src/main.rs | 15 ++++--- tests/e2e/check_claude_code_hook.rs | 14 +++--- tests/e2e/check_format_auto_detect.rs | 2 +- tests/e2e/check_generic.rs | 45 ++++++++++++++++---- tests/e2e/error_handling.rs | 12 ++++-- 8 files changed, 176 insertions(+), 56 deletions(-) diff --git a/src/adapter/check_adapter.rs b/src/adapter/check_adapter.rs index 75bf25c..9b7f859 100644 --- a/src/adapter/check_adapter.rs +++ b/src/adapter/check_adapter.rs @@ -1,3 +1,5 @@ +use std::fmt; + use serde::{Deserialize, Serialize}; use crate::config::{ActionKind, Defaults, MergedSandboxPolicy}; @@ -5,6 +7,14 @@ use crate::rules::rule_engine::Action; use super::{ActionResult, Endpoint, SandboxInfo}; +/// Output format for `runok check` (generic mode). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum OutputFormat { + Json, + #[default] + Text, +} + /// stdin JSON input for `runok check`. #[derive(Debug, Deserialize)] pub struct CheckInput { @@ -36,20 +46,31 @@ pub struct CheckSandboxInfo { /// Generic check endpoint implementing `Endpoint`. pub struct CheckAdapter { command: String, + output_format: OutputFormat, } impl CheckAdapter { /// Build from the `--command` CLI argument. pub fn from_command(command: String) -> Self { - Self { command } + Self { + command, + output_format: OutputFormat::default(), + } } /// Build from stdin JSON input. pub fn from_stdin(input: CheckInput) -> Self { Self { command: input.command, + output_format: OutputFormat::default(), } } + + /// Set the output format. + pub fn with_output_format(mut self, output_format: OutputFormat) -> Self { + self.output_format = output_format; + self + } } /// Build a `CheckOutput` from an `ActionResult`. @@ -91,6 +112,23 @@ fn build_no_match_output(defaults: &Defaults) -> CheckOutput { } } +/// Format a `CheckOutput` as human-readable text. +impl fmt::Display for CheckOutput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.decision)?; + if let Some(ref reason) = self.reason { + write!(f, ": {reason}")?; + } + if let Some(ref suggestion) = self.fix_suggestion { + write!(f, " (suggestion: {suggestion})")?; + } + if let Some(ref sandbox) = self.sandbox { + write!(f, " [sandbox: {}]", sandbox.preset)?; + } + Ok(()) + } +} + impl Endpoint for CheckAdapter { fn extract_command(&self) -> Result, anyhow::Error> { Ok(Some(self.command.clone())) @@ -98,15 +136,13 @@ impl Endpoint for CheckAdapter { fn handle_action(&self, result: ActionResult) -> Result { let output = build_check_output(&result); - let json = serde_json::to_string(&output)?; - println!("{json}"); + self.print_output(&output)?; Ok(0) } fn handle_no_match(&self, defaults: &Defaults) -> Result { let output = build_no_match_output(defaults); - let json = serde_json::to_string(&output)?; - println!("{json}"); + self.print_output(&output)?; Ok(0) } @@ -116,6 +152,21 @@ impl Endpoint for CheckAdapter { } } +impl CheckAdapter { + fn print_output(&self, output: &CheckOutput) -> Result<(), anyhow::Error> { + match self.output_format { + OutputFormat::Json => { + let json = serde_json::to_string(output)?; + println!("{json}"); + } + OutputFormat::Text => { + println!("{output}"); + } + } + Ok(()) + } +} + /// Convert `SandboxInfo` into the informational `CheckSandboxInfo` for the response. fn build_sandbox_info(info: &SandboxInfo) -> Option { match info { diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 163d108..a7fba7d 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -52,13 +52,23 @@ pub struct CheckArgs { /// Input format: "claude-code-hook" or omit for auto-detection #[arg(long)] - pub format: Option, + pub input_format: Option, + + /// Output format + #[arg(long, value_enum, default_value_t = OutputFormat::Text)] + pub output_format: OutputFormat, /// Output detailed rule matching information to stderr #[arg(long)] pub verbose: bool, } +#[derive(clap::ValueEnum, Clone, Debug, PartialEq)] +pub enum OutputFormat { + Json, + Text, +} + #[cfg(test)] mod tests { use super::*; @@ -87,19 +97,23 @@ mod tests { )] #[case::check_with_command( &["runok", "check", "--command", "git status"], - Commands::Check(CheckArgs { command: Some("git status".into()), format: None, verbose: false }), + Commands::Check(CheckArgs { command: Some("git status".into()), input_format: None, output_format: OutputFormat::Text, verbose: false }), + )] + #[case::check_with_input_format( + &["runok", "check", "--input-format", "claude-code-hook"], + Commands::Check(CheckArgs { command: None, input_format: Some("claude-code-hook".into()), output_format: OutputFormat::Text, verbose: false }), )] - #[case::check_with_format( - &["runok", "check", "--format", "claude-code-hook"], - Commands::Check(CheckArgs { command: None, format: Some("claude-code-hook".into()), verbose: false }), + #[case::check_with_output_format_json( + &["runok", "check", "--output-format", "json", "--command", "ls"], + Commands::Check(CheckArgs { command: Some("ls".into()), input_format: None, output_format: OutputFormat::Json, verbose: false }), )] #[case::check_with_both( - &["runok", "check", "--command", "ls", "--format", "claude-code-hook"], - Commands::Check(CheckArgs { command: Some("ls".into()), format: Some("claude-code-hook".into()), verbose: false }), + &["runok", "check", "--command", "ls", "--input-format", "claude-code-hook"], + Commands::Check(CheckArgs { command: Some("ls".into()), input_format: Some("claude-code-hook".into()), output_format: OutputFormat::Text, verbose: false }), )] #[case::check_with_verbose( &["runok", "check", "--verbose", "--command", "git status"], - Commands::Check(CheckArgs { command: Some("git status".into()), format: None, verbose: true }), + Commands::Check(CheckArgs { command: Some("git status".into()), input_format: None, output_format: OutputFormat::Text, verbose: true }), )] fn cli_parsing(#[case] argv: &[&str], #[case] expected: Commands) { let cli = Cli::parse_from(argv); diff --git a/src/cli/route.rs b/src/cli/route.rs index 7e4d0bb..d733b70 100644 --- a/src/cli/route.rs +++ b/src/cli/route.rs @@ -1,9 +1,17 @@ use crate::adapter::Endpoint; -use crate::adapter::check_adapter::{CheckAdapter, CheckInput}; +use crate::adapter::check_adapter::{CheckAdapter, CheckInput, OutputFormat}; use crate::adapter::hook_adapter::{ClaudeCodeHookAdapter, HookInput}; use super::CheckArgs; +/// Convert the CLI output format enum to the adapter output format enum. +fn to_adapter_output_format(cli_format: &super::OutputFormat) -> OutputFormat { + match cli_format { + super::OutputFormat::Json => OutputFormat::Json, + super::OutputFormat::Text => OutputFormat::Text, + } +} + /// Result of routing `runok check`: either a single endpoint or multiple commands. pub enum CheckRoute { /// Single endpoint (--command, JSON stdin, or single-line plaintext). @@ -17,11 +25,13 @@ pub fn route_check( args: &CheckArgs, mut stdin: impl std::io::Read, ) -> Result { + let output_format = to_adapter_output_format(&args.output_format); + // 1. --command CLI argument → always generic mode (no stdin) if let Some(command) = &args.command { - return Ok(CheckRoute::Single(Box::new(CheckAdapter::from_command( - command.clone(), - )))); + return Ok(CheckRoute::Single(Box::new( + CheckAdapter::from_command(command.clone()).with_output_format(output_format), + ))); } // 2. Read stdin once @@ -38,9 +48,9 @@ pub fn route_check( } // 4. --format requires JSON; plaintext fallback is not allowed when --format is specified - if let Some(format) = &args.format { + if let Some(format) = &args.input_format { return Err(anyhow::anyhow!( - "JSON parse error: input must be valid JSON when --format '{format}' is specified" + "JSON parse error: input must be valid JSON when --input-format '{format}' is specified" )); } @@ -57,15 +67,16 @@ pub fn route_check( } if commands.len() == 1 { - return Ok(CheckRoute::Single(Box::new(CheckAdapter::from_command( - commands.into_iter().next().unwrap_or_default(), - )))); + return Ok(CheckRoute::Single(Box::new( + CheckAdapter::from_command(commands.into_iter().next().unwrap_or_default()) + .with_output_format(output_format), + ))); } Ok(CheckRoute::Multi( commands .into_iter() - .map(CheckAdapter::from_command) + .map(|cmd| CheckAdapter::from_command(cmd).with_output_format(output_format)) .collect(), )) } @@ -76,7 +87,7 @@ fn route_json( json_value: serde_json::Value, ) -> Result { // --format is explicitly specified → use that format - if let Some(format) = &args.format { + if let Some(format) = &args.input_format { return match format.as_str() { "claude-code-hook" => { let hook_input: HookInput = serde_json::from_value(json_value)?; @@ -85,7 +96,7 @@ fn route_json( )))) } unknown => Err(anyhow::anyhow!( - "Unknown format: '{unknown}'. Valid formats: claude-code-hook" + "Unknown input format: '{unknown}'. Valid formats: claude-code-hook" )), }; } @@ -98,10 +109,11 @@ fn route_json( hook_input, )))) } else if json_value.get("command").is_some() { + let output_format = to_adapter_output_format(&args.output_format); let check_input: CheckInput = serde_json::from_value(json_value)?; - Ok(CheckRoute::Single(Box::new(CheckAdapter::from_stdin( - check_input, - )))) + Ok(CheckRoute::Single(Box::new( + CheckAdapter::from_stdin(check_input).with_output_format(output_format), + ))) } else { Err(anyhow::anyhow!( "Unknown input format: expected 'tool_name' (Claude Code hook) or 'command' (generic) field" @@ -116,10 +128,11 @@ mod tests { use rstest::rstest; /// Helper: build CheckArgs for testing - fn check_args(command: Option<&str>, format: Option<&str>) -> CheckArgs { + fn check_args(command: Option<&str>, input_format: Option<&str>) -> CheckArgs { CheckArgs { command: command.map(String::from), - format: format.map(String::from), + input_format: input_format.map(String::from), + output_format: crate::cli::OutputFormat::Text, verbose: false, } } @@ -212,7 +225,8 @@ mod tests { let result = route_check(&args, "not valid json".as_bytes()); match result { Err(e) => assert!( - e.to_string().contains("JSON parse error") && e.to_string().contains("--format"), + e.to_string().contains("JSON parse error") + && e.to_string().contains("--input-format"), "error was: {e}" ), Ok(_) => panic!("expected an error"), @@ -383,7 +397,8 @@ mod tests { let result = route_check(&args, r#"{"command": "ls"}"#.as_bytes()); match result { Err(e) => assert!( - e.to_string().contains("Unknown format: 'invalid-format'"), + e.to_string() + .contains("Unknown input format: 'invalid-format'"), "error was: {e}" ), Ok(_) => panic!("expected an error"), diff --git a/src/main.rs b/src/main.rs index 26ad37e..45c08dd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -141,7 +141,8 @@ mod tests { fn run_command_check_with_command_returns_zero() { let cmd = Commands::Check(CheckArgs { command: Some("echo hello".into()), - format: None, + input_format: None, + output_format: cli::OutputFormat::Text, verbose: false, }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); @@ -153,7 +154,8 @@ mod tests { fn run_command_check_with_empty_stdin_returns_two() { let cmd = Commands::Check(CheckArgs { command: None, - format: None, + input_format: None, + output_format: cli::OutputFormat::Text, verbose: false, }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); @@ -165,7 +167,8 @@ mod tests { fn run_command_check_with_stdin_json_returns_zero() { let cmd = Commands::Check(CheckArgs { command: None, - format: None, + input_format: None, + output_format: cli::OutputFormat::Text, verbose: false, }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); @@ -177,7 +180,8 @@ mod tests { fn run_command_check_with_plaintext_stdin_returns_zero() { let cmd = Commands::Check(CheckArgs { command: None, - format: None, + input_format: None, + output_format: cli::OutputFormat::Text, verbose: false, }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); @@ -209,7 +213,8 @@ mod tests { fn run_command_check_with_multiline_plaintext_stdin_returns_zero() { let cmd = Commands::Check(CheckArgs { command: None, - format: None, + input_format: None, + output_format: cli::OutputFormat::Text, verbose: false, }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); diff --git a/tests/e2e/check_claude_code_hook.rs b/tests/e2e/check_claude_code_hook.rs index 97c0585..6da86eb 100644 --- a/tests/e2e/check_claude_code_hook.rs +++ b/tests/e2e/check_claude_code_hook.rs @@ -56,7 +56,7 @@ fn non_bash_hook_json(tool_name: &str) -> String { fn hook_bash_deny(hook_env: TestEnv) { let assert = hook_env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin(bash_hook_json("rm -rf /")) .assert(); let output = assert.code(0).get_output().stdout.clone(); @@ -76,7 +76,7 @@ fn hook_bash_deny(hook_env: TestEnv) { fn hook_bash_allow(hook_env: TestEnv) { let assert = hook_env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin(bash_hook_json("git status")) .assert(); let output = assert.code(0).get_output().stdout.clone(); @@ -96,7 +96,7 @@ fn hook_bash_allow(hook_env: TestEnv) { fn hook_non_bash_tool_no_output(hook_env: TestEnv, #[case] tool_name: &str) { let assert = hook_env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin(non_bash_hook_json(tool_name)) .assert(); assert.code(0).stdout(predicates::str::is_empty()); @@ -108,7 +108,7 @@ fn hook_non_bash_tool_no_output(hook_env: TestEnv, #[case] tool_name: &str) { fn hook_invalid_json_exits_2(hook_env: TestEnv) { let assert = hook_env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin("invalid json") .assert(); assert.code(2); @@ -120,7 +120,7 @@ fn hook_invalid_json_exits_2(hook_env: TestEnv) { fn hook_sandbox_allow_rewrites_command(hook_env: TestEnv) { let assert = hook_env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin(bash_hook_json("echo hello")) .assert(); let output = assert.code(0).get_output().stdout.clone(); @@ -147,7 +147,7 @@ fn hook_sandbox_allow_rewrites_command(hook_env: TestEnv) { fn hook_bash_no_match_returns_ask(hook_env: TestEnv) { let assert = hook_env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin(bash_hook_json("unknown-command --flag")) .assert(); let output = assert.code(0).get_output().stdout.clone(); @@ -162,7 +162,7 @@ fn hook_bash_no_match_returns_ask(hook_env: TestEnv) { fn hook_output_contains_event_name(hook_env: TestEnv) { let assert = hook_env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin(bash_hook_json("git status")) .assert(); let output = assert.code(0).get_output().stdout.clone(); diff --git a/tests/e2e/check_format_auto_detect.rs b/tests/e2e/check_format_auto_detect.rs index 9faaed2..00e6cad 100644 --- a/tests/e2e/check_format_auto_detect.rs +++ b/tests/e2e/check_format_auto_detect.rs @@ -52,7 +52,7 @@ fn auto_detect_claude_code_hook_format(auto_detect_env: TestEnv) { fn auto_detect_generic_format(auto_detect_env: TestEnv) { let assert = auto_detect_env .command() - .arg("check") + .args(["check", "--output-format", "json"]) .write_stdin(r#"{"command":"rm -rf /"}"#) .assert(); let output = assert.code(0).get_output().stdout.clone(); diff --git a/tests/e2e/check_generic.rs b/tests/e2e/check_generic.rs index 216b16d..daf758c 100644 --- a/tests/e2e/check_generic.rs +++ b/tests/e2e/check_generic.rs @@ -13,14 +13,14 @@ fn check_env() -> TestEnv { "}) } -// --- CLI argument mode --- +// --- CLI argument mode (JSON output) --- #[rstest] #[case::deny_rm("rm -rf /", 0, "deny")] #[case::allow_git_status("git status", 0, "allow")] #[case::comment_before_command("# description\ngit status", 0, "allow")] #[case::comment_only("# just a comment", 0, "ask")] -fn check_command_arg( +fn check_command_arg_json( check_env: TestEnv, #[case] command: &str, #[case] expected_exit: i32, @@ -28,7 +28,7 @@ fn check_command_arg( ) { let assert = check_env .command() - .args(["check", "--command", command]) + .args(["check", "--output-format", "json", "--command", command]) .assert(); let output = assert.code(expected_exit).get_output().stdout.clone(); let json: serde_json::Value = @@ -36,13 +36,36 @@ fn check_command_arg( assert_eq!(json["decision"], expected_decision); } +// --- CLI argument mode (text output, default) --- + +#[rstest] +#[case::deny_rm("rm -rf /", "deny")] +#[case::allow_git_status("git status", "allow")] +#[case::comment_only("# just a comment", "ask")] +fn check_command_arg_text( + check_env: TestEnv, + #[case] command: &str, + #[case] expected_decision: &str, +) { + let assert = check_env + .command() + .args(["check", "--command", command]) + .assert(); + let output = assert.code(0).get_output().stdout.clone(); + let stdout = String::from_utf8_lossy(&output); + assert!( + stdout.starts_with(expected_decision), + "expected stdout to start with '{expected_decision}', got: {stdout}" + ); +} + // --- stdin JSON mode --- #[rstest] fn check_stdin_json_deny(check_env: TestEnv) { let assert = check_env .command() - .arg("check") + .args(["check", "--output-format", "json"]) .write_stdin(r#"{"command":"rm -rf /"}"#) .assert(); let output = assert.code(0).get_output().stdout.clone(); @@ -55,7 +78,7 @@ fn check_stdin_json_deny(check_env: TestEnv) { fn check_stdin_json_allow(check_env: TestEnv) { let assert = check_env .command() - .arg("check") + .args(["check", "--output-format", "json"]) .write_stdin(r#"{"command":"git status"}"#) .assert(); let output = assert.code(0).get_output().stdout.clone(); @@ -78,7 +101,7 @@ fn check_no_input_exits_2(check_env: TestEnv) { fn check_plaintext_stdin_single_line(check_env: TestEnv) { let assert = check_env .command() - .arg("check") + .args(["check", "--output-format", "json"]) .write_stdin("git status\n") .assert(); let output = assert.code(0).get_output().stdout.clone(); @@ -93,7 +116,7 @@ fn check_plaintext_stdin_single_line(check_env: TestEnv) { fn check_deny_includes_reason(check_env: TestEnv) { let assert = check_env .command() - .args(["check", "--command", "rm -rf /"]) + .args(["check", "--output-format", "json", "--command", "rm -rf /"]) .assert(); let output = assert.code(0).get_output().stdout.clone(); let json: serde_json::Value = @@ -120,7 +143,13 @@ fn check_allow_with_sandbox_info() { "}); let assert = env .command() - .args(["check", "--command", "python3 script.py"]) + .args([ + "check", + "--output-format", + "json", + "--command", + "python3 script.py", + ]) .assert(); let output = assert.code(0).get_output().stdout.clone(); let json: serde_json::Value = diff --git a/tests/e2e/error_handling.rs b/tests/e2e/error_handling.rs index 96ca18e..04ba68f 100644 --- a/tests/e2e/error_handling.rs +++ b/tests/e2e/error_handling.rs @@ -86,7 +86,13 @@ fn no_config_check_returns_default() { let env = TestEnv::new("{}"); let assert = env .command() - .args(["check", "--command", "git status"]) + .args([ + "check", + "--output-format", + "json", + "--command", + "git status", + ]) .assert(); let output = assert.code(0).get_output().stdout.clone(); let json: serde_json::Value = @@ -102,7 +108,7 @@ fn unknown_format_flag_exits_2() { let env = TestEnv::new("{}"); let assert = env .command() - .args(["check", "--format", "unknown-format"]) + .args(["check", "--input-format", "unknown-format"]) .write_stdin(r#"{"command":"ls"}"#) .assert(); assert.code(2); @@ -115,7 +121,7 @@ fn format_with_non_json_stdin_exits_2() { let env = TestEnv::new("{}"); let assert = env .command() - .args(["check", "--format", "claude-code-hook"]) + .args(["check", "--input-format", "claude-code-hook"]) .write_stdin("not valid json") .assert(); assert.code(2);