From 34c12fd41133789e1aa7a95005d50c18f9cfc02d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Feb 2026 01:05:48 +0000 Subject: [PATCH 1/2] fix(builtins): fix jq argument parsing for combined flags, --arg, and --argjson The jq builtin's argument parser had several issues: - Combined short flags (e.g., -rn, -sc) were silently ignored because the parser only matched exact single-flag strings like "-r" or "-n" - --arg name value and --argjson name value were not handled, causing the name argument to be misidentified as the jq filter and the actual filter to be treated as a file argument ("Could not open file" errors) - --indent N was not handled, causing N to be misidentified as the filter - -- (end of options separator) was not supported Fixes: - Switch from iterator to index-based arg loop to consume multi-arg flags - Parse combined short flags by iterating over characters (e.g., -rn -> -r + -n) - Handle --arg/--argjson by consuming 2 extra args and passing variable bindings to jaq Compiler::with_global_vars and Ctx::new - Handle --indent N, --args, --jsonargs, and -- separator - Skip unknown long flags gracefully https://claude.ai/code/session_01JKnPuxK6nH19jUhfc2UVst --- crates/bashkit/src/builtins/jq.rs | 191 +++++++++++++++--- crates/bashkit/tests/spec_cases/jq/jq.test.sh | 28 +++ 2 files changed, 194 insertions(+), 25 deletions(-) diff --git a/crates/bashkit/src/builtins/jq.rs b/crates/bashkit/src/builtins/jq.rs index 8c9181d6..11d08e36 100644 --- a/crates/bashkit/src/builtins/jq.rs +++ b/crates/bashkit/src/builtins/jq.rs @@ -129,7 +129,8 @@ impl Builtin for Jq { } } - // Parse arguments for flags + // Parse arguments for flags using index-based loop to support + // multi-arg flags like --arg name value and --argjson name value. let mut raw_output = false; let mut compact_output = false; let mut null_input = false; @@ -140,32 +141,103 @@ impl Builtin for Jq { let mut join_output = false; let mut filter = "."; let mut file_args: Vec<&str> = Vec::new(); + // Store variable bindings as (name, serde_json::Value) to avoid + // holding non-Send jaq Val across await points. + let mut var_bindings: Vec<(String, serde_json::Value)> = Vec::new(); let mut found_filter = false; - for arg in ctx.args { + let mut i = 0; + while i < ctx.args.len() { + let arg = &ctx.args[i]; if found_filter { // Everything after the filter is a file argument file_args.push(arg); - } else if arg == "-r" || arg == "--raw-output" { + i += 1; + continue; + } + + if arg == "--" { + // End of options: next arg is filter, rest are files + i += 1; + if i < ctx.args.len() { + filter = &ctx.args[i]; + found_filter = true; + } + i += 1; + continue; + } + + if arg == "--raw-output" { raw_output = true; - } else if arg == "-c" || arg == "--compact-output" { + } else if arg == "--compact-output" { compact_output = true; - } else if arg == "-n" || arg == "--null-input" { + } else if arg == "--null-input" { null_input = true; - } else if arg == "-S" || arg == "--sort-keys" { + } else if arg == "--sort-keys" { sort_keys = true; - } else if arg == "-s" || arg == "--slurp" { + } else if arg == "--slurp" { slurp = true; - } else if arg == "-e" || arg == "--exit-status" { + } else if arg == "--exit-status" { exit_status = true; } else if arg == "--tab" { tab_indent = true; - } else if arg == "-j" || arg == "--join-output" { + } else if arg == "--join-output" { join_output = true; - } else if !arg.starts_with('-') { + } else if arg == "--arg" { + // --arg name value: bind $name to string value + if i + 2 < ctx.args.len() { + let name = format!("${}", &ctx.args[i + 1]); + let value = serde_json::Value::String(ctx.args[i + 2].to_string()); + var_bindings.push((name, value)); + i += 3; + continue; + } + i += 1; + continue; + } else if arg == "--argjson" { + // --argjson name value: bind $name to parsed JSON value + if i + 2 < ctx.args.len() { + let name = format!("${}", &ctx.args[i + 1]); + let json_val: serde_json::Value = serde_json::from_str(&ctx.args[i + 2]) + .map_err(|e| { + Error::Execution(format!("jq: invalid JSON for --argjson: {}", e)) + })?; + var_bindings.push((name, json_val)); + i += 3; + continue; + } + i += 1; + continue; + } else if arg == "--indent" { + // --indent N: skip the numeric argument (use default formatting) + i += 2; + continue; + } else if arg == "--args" || arg == "--jsonargs" { + // Remaining args are positional, not files; skip for now + i += 1; + continue; + } else if arg.starts_with("--") { + // Unknown long flag: skip + } else if arg.starts_with('-') && arg.len() > 1 { + // Short flag(s): may be combined like -rn, -sc, -snr + for ch in arg[1..].chars() { + match ch { + 'r' => raw_output = true, + 'c' => compact_output = true, + 'n' => null_input = true, + 'S' => sort_keys = true, + 's' => slurp = true, + 'e' => exit_status = true, + 'j' => join_output = true, + _ => {} // ignore unknown short flags + } + } + } else { + // Non-flag argument: this is the filter filter = arg; found_filter = true; } + i += 1; } // Build input: read from file arguments if provided, otherwise stdin @@ -221,19 +293,23 @@ impl Builtin for Jq { )) })?; - // Compile the filter - let filter = Compiler::default() - .with_funs(jaq_std::funs().chain(jaq_json::funs())) - .compile(modules) - .map_err(|errs| { - Error::Execution(format!( - "jq: compile error: {}", - errs.into_iter() - .map(|e| format!("{:?}", e)) - .collect::>() - .join(", ") - )) - })?; + // Compile the filter, registering any --arg/--argjson variable names + let var_names: Vec<&str> = var_bindings.iter().map(|(n, _)| n.as_str()).collect(); + let compiler = Compiler::default().with_funs(jaq_std::funs().chain(jaq_json::funs())); + let compiler = if var_names.is_empty() { + compiler + } else { + compiler.with_global_vars(var_names.iter().copied()) + }; + let filter = compiler.compile(modules).map_err(|errs| { + Error::Execution(format!( + "jq: compile error: {}", + errs.into_iter() + .map(|e| format!("{:?}", e)) + .collect::>() + .join(", ") + )) + })?; // Process input as JSON let mut output = String::new(); @@ -260,8 +336,12 @@ impl Builtin for Jq { // Create empty inputs iterator let inputs = RcIter::new(core::iter::empty()); - // Run the filter - let ctx = Ctx::new([], &inputs); + // Run the filter, passing any --arg/--argjson variable values + let var_vals: Vec = var_bindings + .iter() + .map(|(_, v)| Val::from(v.clone())) + .collect(); + let ctx = Ctx::new(var_vals, &inputs); for result in filter.run((ctx, jaq_input)) { match result { Ok(val) => { @@ -706,4 +786,65 @@ mod tests { assert_eq!(arr[0]["id"], 1); assert_eq!(arr[1]["id"], 2); } + + // --- Argument parsing bug regression tests --- + + #[tokio::test] + async fn test_jq_combined_short_flags() { + // -rn should be parsed as -r + -n + let result = run_jq_with_args(&["-rn", "1+1"], "").await.unwrap(); + assert_eq!(result.trim(), "2"); + } + + #[tokio::test] + async fn test_jq_combined_flags_sc() { + // -sc should be parsed as -s + -c + let result = run_jq_with_args(&["-sc", "add"], "1\n2\n3\n") + .await + .unwrap(); + assert_eq!(result.trim(), "6"); + } + + #[tokio::test] + async fn test_jq_arg_flag() { + // --arg name value: $name should resolve to "value" in the filter + let result = run_jq_with_args(&["--arg", "name", "world", "-n", r#""hello \($name)""#], "") + .await + .unwrap(); + assert_eq!(result.trim(), r#""hello world""#); + } + + #[tokio::test] + async fn test_jq_argjson_flag() { + // --argjson count 5: $count should resolve to 5 (number, not string) + let result = run_jq_with_args(&["--argjson", "count", "5", "-n", "$count + 1"], "") + .await + .unwrap(); + assert_eq!(result.trim(), "6"); + } + + #[tokio::test] + async fn test_jq_arg_does_not_eat_filter() { + // --arg name value '.' should NOT treat '.' as a file + let result = run_jq_with_args(&["--arg", "x", "hello", "."], r#"{"a":1}"#) + .await + .unwrap(); + assert!(result.contains("\"a\": 1")); + } + + #[tokio::test] + async fn test_jq_double_dash_separator() { + // -- ends option parsing; next arg is filter + let result = run_jq_with_args(&["-n", "--", "1+1"], "").await.unwrap(); + assert_eq!(result.trim(), "2"); + } + + #[tokio::test] + async fn test_jq_indent_flag() { + // --indent 4 should not eat the filter + let result = run_jq_with_args(&["--indent", "4", "."], r#"{"a":1}"#) + .await + .unwrap(); + assert!(result.contains("\"a\"")); + } } diff --git a/crates/bashkit/tests/spec_cases/jq/jq.test.sh b/crates/bashkit/tests/spec_cases/jq/jq.test.sh index d46aa526..9e457a17 100644 --- a/crates/bashkit/tests/spec_cases/jq/jq.test.sh +++ b/crates/bashkit/tests/spec_cases/jq/jq.test.sh @@ -839,3 +839,31 @@ jq -s '.' /tmp/jqtest2/a.json /tmp/jqtest2/b.json } ] ### end + +### jq_combined_flags_rn +# Combined short flags -rn should work like -r -n +jq -rn '"hello"' +### expect +hello +### end + +### jq_combined_flags_sc +# Combined short flags -sc should work like -s -c +printf '1\n2\n3\n' | jq -sc 'add' +### expect +6 +### end + +### jq_arg_binding +# --arg binds a string variable accessible as $name in filter +jq -n --arg greeting hello '"say \($greeting)"' +### expect +"say hello" +### end + +### jq_argjson_binding +# --argjson binds a parsed JSON value accessible as $name in filter +jq -n --argjson count 5 '$count + 1' +### expect +6 +### end From 5409beb395daac07e4e5fca46c6e00b5523a4c72 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Feb 2026 03:03:25 +0000 Subject: [PATCH 2/2] test(jq): add negative tests, unskip passing tests, update specs and docs - Unskip jq_try and jq_debug spec tests (both pass with jaq) - Add 8 new spec tests: empty input, NDJSON, multiple --arg, --argjson with objects, combined -snr flags, exit status for false/truthy - Add 10 new unit tests: invalid JSON, invalid filter, invalid argjson, empty/whitespace input, NDJSON, exit status, multiple bindings, combined flags - Update skip reasons to be precise about jaq limitations - Update 009-implementation-status: 108 tests (100 pass, 8 skip) - Update 005-builtins: document --arg, --argjson, -V, combined flags - Update compatibility.md: list all supported jq flags https://claude.ai/code/session_01JKnPuxK6nH19jUhfc2UVst --- crates/bashkit/docs/compatibility.md | 2 +- crates/bashkit/src/builtins/jq.rs | 118 ++++++++++++++++++ crates/bashkit/tests/spec_cases/jq/jq.test.sh | 71 +++++++++-- specs/005-builtins.md | 2 +- specs/009-implementation-status.md | 24 ++-- 5 files changed, 196 insertions(+), 21 deletions(-) diff --git a/crates/bashkit/docs/compatibility.md b/crates/bashkit/docs/compatibility.md index b67391dd..5b856097 100644 --- a/crates/bashkit/docs/compatibility.md +++ b/crates/bashkit/docs/compatibility.md @@ -72,7 +72,7 @@ for sandbox security reasons. See the compliance spec for details. | `grep` | `-i`, `-v`, `-c`, `-n`, `-E`, `-q` | Pattern matching | | `sed` | `s///[g]`, `d`, `p`, `q`, `a`, `i`, `c`, `h/H/g/G/x`, `-E`, `-n`, `!` | Stream editing | | `awk` | `'{print}'`, `-F`, `-v`, loops, arrays, increment, ternary | Text processing | -| `jq` | `.field`, `.[n]`, pipes, file args | JSON processing | +| `jq` | `.field`, `.[n]`, pipes, file args, `-r`, `-c`, `-n`, `-s`, `-S`, `-e`, `-j`, `--tab`, `--arg`, `--argjson`, `-V`, combined flags | JSON processing | | `sleep` | `N`, `N.N` | Pause execution (max 60s) | | `head` | `-n N`, `-N` | First N lines (default 10) | | `tail` | `-n N`, `-N` | Last N lines (default 10) | diff --git a/crates/bashkit/src/builtins/jq.rs b/crates/bashkit/src/builtins/jq.rs index 11d08e36..2444460c 100644 --- a/crates/bashkit/src/builtins/jq.rs +++ b/crates/bashkit/src/builtins/jq.rs @@ -847,4 +847,122 @@ mod tests { .unwrap(); assert!(result.contains("\"a\"")); } + + // --- Negative tests --- + + #[tokio::test] + async fn test_jq_invalid_json_input() { + let result = run_jq(".", "not valid json"); + assert!(result.await.is_err(), "invalid JSON input should error"); + } + + #[tokio::test] + async fn test_jq_invalid_filter_syntax() { + let result = run_jq(".[", r#"{"a":1}"#); + assert!(result.await.is_err(), "invalid filter should error"); + } + + #[tokio::test] + async fn test_jq_invalid_argjson_value() { + let result = run_jq_with_args(&["--argjson", "x", "not json", "-n", "$x"], ""); + assert!( + result.await.is_err(), + "invalid JSON for --argjson should error" + ); + } + + #[tokio::test] + async fn test_jq_empty_input_no_null() { + // Empty stdin without -n should produce empty output, not error + let result = run_jq(".", "").await.unwrap(); + assert_eq!(result, ""); + } + + #[tokio::test] + async fn test_jq_whitespace_only_input() { + let result = run_jq(".", " \n\t ").await.unwrap(); + assert_eq!(result, ""); + } + + #[tokio::test] + async fn test_jq_ndjson_multiple_values() { + // Multiple JSON values on separate lines (NDJSON) + let result = run_jq(".a", "{\"a\":1}\n{\"a\":2}\n").await.unwrap(); + assert_eq!(result.trim(), "1\n2"); + } + + #[tokio::test] + async fn test_jq_exit_status_false() { + // -e flag: false output -> exit code 1 + let jq = Jq; + let fs = Arc::new(InMemoryFs::new()); + let mut vars = HashMap::new(); + let mut cwd = PathBuf::from("/"); + let args = vec!["-e".to_string(), ".".to_string()]; + let ctx = Context { + args: &args, + env: &HashMap::new(), + variables: &mut vars, + cwd: &mut cwd, + fs, + stdin: Some("false"), + #[cfg(feature = "http_client")] + http_client: None, + #[cfg(feature = "git")] + git_client: None, + }; + let result = jq.execute(ctx).await.unwrap(); + assert_eq!(result.exit_code, 1, "-e with false should exit 1"); + } + + #[tokio::test] + async fn test_jq_exit_status_truthy() { + // -e flag: truthy output -> exit code 0 + let jq = Jq; + let fs = Arc::new(InMemoryFs::new()); + let mut vars = HashMap::new(); + let mut cwd = PathBuf::from("/"); + let args = vec!["-e".to_string(), ".".to_string()]; + let ctx = Context { + args: &args, + env: &HashMap::new(), + variables: &mut vars, + cwd: &mut cwd, + fs, + stdin: Some("42"), + #[cfg(feature = "http_client")] + http_client: None, + #[cfg(feature = "git")] + git_client: None, + }; + let result = jq.execute(ctx).await.unwrap(); + assert_eq!(result.exit_code, 0, "-e with truthy value should exit 0"); + } + + #[tokio::test] + async fn test_jq_multiple_arg_bindings() { + let result = run_jq_with_args( + &[ + "--arg", + "x", + "hello", + "--arg", + "y", + "world", + "-n", + r#""[\($x)] [\($y)]""#, + ], + "", + ) + .await + .unwrap(); + assert_eq!(result.trim(), r#""[hello] [world]""#); + } + + #[tokio::test] + async fn test_jq_combined_flags_snr() { + // -snr: slurp + null-input + raw-output + let result = run_jq_with_args(&["-snr", r#""hello""#], "").await.unwrap(); + assert_eq!(result.trim(), "hello"); + } } diff --git a/crates/bashkit/tests/spec_cases/jq/jq.test.sh b/crates/bashkit/tests/spec_cases/jq/jq.test.sh index 9e457a17..7364c5d4 100644 --- a/crates/bashkit/tests/spec_cases/jq/jq.test.sh +++ b/crates/bashkit/tests/spec_cases/jq/jq.test.sh @@ -482,14 +482,14 @@ echo '5' | jq 'if . > 3 then "big" else "small" end' ### end ### jq_alternative -### skip: alternative operator (//) not implemented +### skip: jaq errors on .foo applied to null instead of returning null for // echo 'null' | jq '.foo // "default"' ### expect "default" ### end ### jq_try -### skip: try operator not implemented +# Try-catch handles runtime errors gracefully echo 'null' | jq 'try .foo catch "error"' ### expect "error" @@ -512,7 +512,7 @@ echo '{"a":{"b":1}}' | jq 'getpath(["a","b"])' ### end ### jq_setpath -### skip: setpath not implemented +### skip: setpath not available in jaq standard library echo '{"a":1}' | jq 'setpath(["b"];2)' ### expect {"a":1,"b":2} @@ -577,7 +577,7 @@ echo '{"a":{"b":1}}' | jq '[paths]' ### end ### jq_leaf_paths -### skip: leaf_paths not implemented +### skip: leaf_paths not available in jaq standard library echo '{"a":{"b":1}}' | jq '[leaf_paths]' ### expect [["a","b"]] @@ -628,28 +628,28 @@ echo '1' | jq '[while(. < 5; . + 1)]' ### end ### jq_input -### skip: input not implemented +### skip: inputs iterator not wired to remaining stdin values printf '1\n2\n' | jq 'input' ### expect 2 ### end ### jq_inputs -### skip: inputs not implemented +### skip: inputs iterator not wired to remaining stdin values printf '1\n2\n3\n' | jq '[inputs]' ### expect [2,3] ### end ### jq_debug -### skip: debug not implemented +# Debug passes value through (stderr output not captured) echo '1' | jq 'debug' ### expect 1 ### end ### jq_env -### skip: env not implemented +### skip: shell env vars not propagated to jaq runtime FOO=bar jq -n 'env.FOO' ### expect "bar" @@ -730,14 +730,14 @@ true ### end ### jq_match -### skip: match not implemented +### skip: jaq omits capture name field (real jq includes "name":null) echo '"hello"' | jq 'match("e(ll)o")' ### expect {"offset":1,"length":4,"string":"ello","captures":[{"offset":2,"length":2,"string":"ll","name":null}]} ### end ### jq_scan -### skip: scan not implemented +### skip: jaq scan requires explicit "g" flag for global match echo '"hello hello"' | jq '[scan("hel")]' ### expect ["hel","hel"] @@ -867,3 +867,54 @@ jq -n --argjson count 5 '$count + 1' ### expect 6 ### end + +### jq_empty_input +# Empty input with no flags produces no output +echo '' | jq '.' +### expect +### end + +### jq_ndjson +# Multiple JSON values (NDJSON) are each processed separately +printf '{"a":1}\n{"a":2}\n' | jq '.a' +### expect +1 +2 +### end + +### jq_multiple_arg_bindings +# Multiple --arg flags each bind a separate variable +jq -n --arg x hello --arg y world '"[\($x)] [\($y)]"' +### expect +"[hello] [world]" +### end + +### jq_argjson_object +# --argjson with a JSON object value +jq -n --argjson obj '{"a":1}' '$obj.a' +### expect +1 +### end + +### jq_combined_flags_snr +# Three combined short flags -snr +jq -snr '"hello"' +### expect +hello +### end + +### jq_exit_status_false +# Exit status mode sets exit code 1 for false output +echo 'false' | jq -e '.' +### exit_code: 1 +### expect +false +### end + +### jq_exit_status_truthy +# Exit status mode sets exit code 0 for truthy output +echo '42' | jq -e '.' +### exit_code: 0 +### expect +42 +### end diff --git a/specs/005-builtins.md b/specs/005-builtins.md index 4e2d2af7..4b4195e7 100644 --- a/specs/005-builtins.md +++ b/specs/005-builtins.md @@ -68,7 +68,7 @@ in a virtual environment. All builtins operate on the virtual filesystem. - `grep` - Pattern matching (`-i`, `-v`, `-c`, `-n`, `-o`, `-l`, `-w`, `-E`, `-F`, `-P`, `-q`, `-m`, `-x`, `-A`, `-B`, `-C`, `-e`, `-f`, `-H`, `-h`, `-b`, `-a`, `-z`, `-r`) - `sed` - Stream editing (s/pat/repl/, d, p, a, i; `-E`, `-e`, `-i`, `-n`; nth occurrence, `!` negation) - `awk` - Text processing (print, -F, variables) -- `jq` - JSON processing (file arguments, `-s`, `-r`, `-c`, `-n`, `-S`, `-e`, `--tab`, `-j`) +- `jq` - JSON processing (file arguments, `-s`, `-r`, `-c`, `-n`, `-S`, `-e`, `--tab`, `-j`, `--arg`, `--argjson`, `-V`/`--version`, combined short flags) - `sort` - Sort lines (`-r`, `-n`, `-u`) - `uniq` - Filter duplicates (`-c`, `-d`, `-u`) - `cut` - Extract fields (`-d`, `-f`) diff --git a/specs/009-implementation-status.md b/specs/009-implementation-status.md index 505f575d..d8f420c4 100644 --- a/specs/009-implementation-status.md +++ b/specs/009-implementation-status.md @@ -107,7 +107,7 @@ Bashkit implements IEEE 1003.1-2024 Shell Command Language. See ## Spec Test Coverage -**Total spec test cases:** 1030 +**Total spec test cases:** 1038 | Category | Cases | In CI | Pass | Skip | Notes | |----------|-------|-------|------|------|-------| @@ -115,9 +115,9 @@ Bashkit implements IEEE 1003.1-2024 Shell Command Language. See | AWK | 90 | Yes | 73 | 17 | loops, arrays, -v, ternary, field assign | | Grep | 81 | Yes | 76 | 5 | now with -z, -r, -a, -b, -H, -h, -f, -P | | Sed | 65 | Yes | 53 | 12 | hold space, change, regex ranges, -E | -| JQ | 100 | Yes | 90 | 10 | reduce, walk, regex funcs | +| JQ | 108 | Yes | 100 | 8 | reduce, walk, regex funcs, --arg/--argjson, combined flags | | Python | 58 | Yes | 50 | 8 | **Experimental.** VFS bridging, pathlib, env vars | -| **Total** | **1030** | **Yes** | **918** | **112** | | +| **Total** | **1038** | **Yes** | **928** | **110** | | ### Bash Spec Tests Breakdown @@ -310,15 +310,21 @@ Features that may be added in the future (not intentionally excluded): ### JQ Limitations -**Skipped Tests (10):** +**Skipped Tests (8):** | Feature | Count | Notes | |---------|-------|-------| -| Alternative `//` | 1 | Null coalescing operator | -| Try-catch | 1 | `try` expression | -| Path functions | 2 | `setpath`, `leaf_paths` | -| I/O functions | 4 | `input`, `inputs`, `debug`, `env` | -| Regex functions | 2 | `match`, `scan` | +| Alternative `//` | 1 | jaq errors on `.foo` applied to null instead of returning null | +| Path functions | 2 | `setpath`, `leaf_paths` not in jaq standard library | +| I/O functions | 3 | `input`, `inputs` (iterator not wired), `env` (shell vars not propagated) | +| Regex functions | 2 | `match` (jaq omits capture `name` field), `scan` (jaq needs explicit `"g"` flag) | + +**Recently Fixed:** +- `try`/`catch` expressions now work (jaq handles runtime errors) +- `debug` passes through values correctly (stderr not captured) +- Combined short flags (`-rn`, `-sc`, `-snr`) +- `--arg name value` and `--argjson name value` variable bindings +- `--indent N` flag no longer eats the filter argument ### Curl Limitations