fix: address CLI panics and missing help in benchmark tools

[strawgate]

Fixed several CLI argument parsing issues that caused panics instead of user-friendly errors:

1. In logfwd-competitive-bench, added bounds checking for options like --lines, --mode, etc., and replaced .expect() with proper error messages.
2. In logfwd-bench, added --help handling to stop it from interpreting the flag as a criterion directory path.
3. In udp-bench, updated the argument parsing to be more robust against missing values and invalid numeric input.
   All changes were verified by re-running the failing commands and confirming they now output helpful errors.

Fixes #459

---

PR created automatically by Jules for task 5500191673198945353 started by @strawgate

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4701d997-0335-429b-86a1-e94d087759c5

📥 Commits

Reviewing files that changed from the base of the PR and between d942b7c and d4b5646.

📒 Files selected for processing (1)

• crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/main.rs

---

Walkthrough

The PR hardens CLI parsing across multiple bench binaries by adding --help/-h handling and usage output, switching to indexed argument iteration, and adding explicit bounds checks for flags that require a following value. Numeric parsing sites that previously used .expect() were changed to controlled unwrap_or_else handlers that print an error and exit with status 1. Several formatting-only refactors (multi-line eprintln!/argument lists and comment whitespace) were applied. No public API or exported declarations were changed.

Possibly related PRs

• strawgate/memagent PR 552: Applies near-identical CLI parsing changes (help handling, bounds checks, replacing expect() with controlled exits) across the same bench binaries.
• strawgate/memagent PR 310: Modifies argument parsing in the competitive-bench binary and overlaps on flag handling/error checks.
• strawgate/memagent PR 68: Introduces --help handling and replaces panic/expect() failures with explicit error messages and exit codes in CLI parsing.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	PR implements all key objectives from issue #459: bounds checking for options requiring values, --help/-h handling, and graceful error exits replacing panics.
Out of Scope Changes check	✅ Passed	All changes are scoped to CLI argument parsing improvements in three benchmark tools; no unrelated modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/logfwd-competitive-bench/src/main.rs (1)

1011-1151: ⚠️ Potential issue | 🟠 Major

Value-required flags should reject flag-like next tokens.

Current checks only verify bounds. If the next token is another flag, it is consumed as a value (or fails later with a misleading parse error). This still misses the intended “requires a value” behavior.

Proposed pattern (apply to all value-taking flags)

+let require_value = |flag: &str, idx: usize| -> &str {
+    match args.get(idx) {
+        Some(v) if !v.starts_with('-') => v.as_str(),
+        _ => {
+            eprintln!("ERROR: {flag} requires a value");
+            process::exit(1);
+        }
+    }
+};

 "--lines" => {
     i += 1;
-    if i >= args.len() {
-        eprintln!("ERROR: --lines requires a value");
-        process::exit(1);
-    }
-    result.lines = args[i].parse().unwrap_or_else(|e| {
-        eprintln!("ERROR: invalid --lines value '{}': {e}", args[i]);
+    let value = require_value("--lines", i);
+    result.lines = value.parse().unwrap_or_else(|e| {
+        eprintln!("ERROR: invalid --lines value '{}': {e}", value);
         process::exit(1);
     });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/logfwd-competitive-bench/src/main.rs` around lines 1011 - 1151, The
argument-parsing loop accepts the next token as a value even if it’s another
flag (starting with '-'), causing misleading errors; update every value-taking
branch (e.g., the handlers that set result.lines, result.agents,
result.scenarios, result.json_file, result.gh_bench_file, result.mode,
result.docker_limits.cpus, result.docker_limits.memory, result.profile_dir,
result.dhat_binary, result.iterations, result.results_file, result.results_dir,
result.dashboard_file) to, immediately after incrementing i, check both bounds
and that args[i] does not start with '-' (e.g., if i >= args.len() ||
args[i].starts_with('-') { eprintln!("ERROR: --<flag> requires a value");
process::exit(1); }), and only then proceed to parse or convert the value; apply
this pattern consistently to all value-required branches and the
Scenario::from_name unwrap_or_else path.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/main.rs`:
- Around line 419-430: The argument-parsing block (the get_arg closure that
iterates args and returns args[i+1]) only checks bounds and fails to treat a
next token that is another flag as a missing value; update the logic inside that
block (where it currently does "if i + 1 < args.len() { return args[i +
1].clone(); } else { eprintln!(...); std::process::exit(1); }") to also check
whether args[i + 1].starts_with('-') and, if so, treat it like a missing value
by printing the same error and exiting; this ensures cases like "udp-bench
receive --port --severity 4" report a missing --port value instead of a later
parse error.

---

Outside diff comments:
In `@crates/logfwd-competitive-bench/src/main.rs`:
- Around line 1011-1151: The argument-parsing loop accepts the next token as a
value even if it’s another flag (starting with '-'), causing misleading errors;
update every value-taking branch (e.g., the handlers that set result.lines,
result.agents, result.scenarios, result.json_file, result.gh_bench_file,
result.mode, result.docker_limits.cpus, result.docker_limits.memory,
result.profile_dir, result.dhat_binary, result.iterations, result.results_file,
result.results_dir, result.dashboard_file) to, immediately after incrementing i,
check both bounds and that args[i] does not start with '-' (e.g., if i >=
args.len() || args[i].starts_with('-') { eprintln!("ERROR: --<flag> requires a
value"); process::exit(1); }), and only then proceed to parse or convert the
value; apply this pattern consistently to all value-required branches and the
Scenario::from_name unwrap_or_else path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cc51d2f1-bd83-4cc5-aef4-a8266d661bc4

📥 Commits

Reviewing files that changed from the base of the PR and between a5ceb56 and f952a4e.

📒 Files selected for processing (3)

• crates/logfwd-bench/src/main.rs
• crates/logfwd-competitive-bench/src/main.rs
• crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/main.rs

[github-actions]

Addressed the remaining unresolved review feedback with a targeted code change.

• Thread resolved with code change: PRRT_kwDORzg8fM54G09a (missing value detection in udp-bench arg parsing)
• Threads replied without code change: none

Validation results

• RUSTC_WRAPPER= cargo check -q --manifest-path crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/Cargo.toml ✅
• RUSTC_WRAPPER= cargo run -q --manifest-path crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/Cargo.toml -- receive --port --severity 4 ✅ (now prints ERROR: --port requires a value)
• just ci ⚠️ could not run in this environment (just: command not found)
• RUSTC_WRAPPER= cargo test -q ⚠️ started but was stopped due to runtime duration in this environment

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:fix: address CLI panics and missing help in benchmark tools #552 (pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)

---

What is this? | From workflow: AI: Address PR Review Feedback

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/logfwd-competitive-bench/src/main.rs (1)

1008-1149: ⚠️ Potential issue | 🟠 Major

Treat a following flag token as a missing value (not as the value).

i >= args.len() prevents OOB, but cases like --json-file --markdown still consume --markdown as a value. This leaves misleading behavior and misses the intended requires a value error path.

Suggested fix pattern (apply to all value-taking options)

+let require_value = |idx: usize, opt: &str| -> &str {
+    match args.get(idx + 1) {
+        Some(v) if !v.starts_with('-') => v.as_str(),
+        _ => {
+            eprintln!("ERROR: {opt} requires a value");
+            process::exit(1);
+        }
+    }
+};
+
 match arg.as_str() {
     "--lines" => {
-        i += 1;
-        if i >= args.len() {
-            eprintln!("ERROR: --lines requires a value");
-            process::exit(1);
-        }
-        result.lines = args[i].parse().unwrap_or_else(|e| {
-            eprintln!("ERROR: invalid --lines value '{}': {e}", args[i]);
+        let v = require_value(i, "--lines");
+        i += 1;
+        result.lines = v.parse().unwrap_or_else(|e| {
+            eprintln!("ERROR: invalid --lines value '{}': {e}", v);
             process::exit(1);
         });
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/logfwd-competitive-bench/src/main.rs` around lines 1008 - 1149, The
parser currently treats any next token as a value (e.g., args[i]) even if it is
another flag (so "--json-file --markdown" is accepted as a value); update the
value-taking branches to validate the next token is not a flag by checking
args[i].starts_with('-') (or starts_with("--")) and if so print the same
"requires a value" error and exit. Apply this check in all places that increment
i and then consume args[i] (examples: the "--json-file" / result.json_file,
"--gh-bench-file" / result.gh_bench_file, "--mode" / result.mode, "--cpus" /
result.docker_limits.cpus, "--memory" / result.docker_limits.memory, "--profile"
/ result.profile_dir, "--dhat-binary" / result.dhat_binary, "--iterations" /
result.iterations, "--results-file" / result.results_file, "--results-dir" /
result.results_dir, "--dashboard-file" / result.dashboard_file, and any similar
branches) so flags are rejected as missing values consistently.

♻️ Duplicate comments (1)

crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/main.rs (1)

456-467: ⚠️ Potential issue | 🟠 Major

Still missing “next token is a flag” handling in get_arg.

udp-bench receive --port --severity 4 still treats --severity as the --port value, then fails later as “invalid port” instead of “--port requires a value”.

Targeted fix

 let get_arg = |name: &str, default: &str| -> String {
     for i in 0..args.len() {
         if args[i] == format!("--{}", name) {
-            if i + 1 < args.len() {
-                return args[i + 1].clone();
-            } else {
-                eprintln!("ERROR: --{} requires a value", name);
-                std::process::exit(1);
-            }
+            match args.get(i + 1) {
+                Some(v) if !v.starts_with('-') => return v.clone(),
+                _ => {
+                    eprintln!("ERROR: --{} requires a value", name);
+                    std::process::exit(1);
+                }
+            }
         }
     }
     default.to_string()
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/main.rs` around lines 456 -
467, The get_arg function currently treats the next argv token as a value even
if it's another flag; modify get_arg (in main.rs) so when you find "--{name}"
and i+1 < args.len() you verify that args[i+1] is not a flag (e.g., does not
start with "--"); if it is a flag, print the same error ("ERROR: --{name}
requires a value") and exit, otherwise return args[i+1].clone(); keep the
existing branch for i+1 out of range unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@crates/logfwd-competitive-bench/src/main.rs`:
- Around line 1008-1149: The parser currently treats any next token as a value
(e.g., args[i]) even if it is another flag (so "--json-file --markdown" is
accepted as a value); update the value-taking branches to validate the next
token is not a flag by checking args[i].starts_with('-') (or starts_with("--"))
and if so print the same "requires a value" error and exit. Apply this check in
all places that increment i and then consume args[i] (examples: the
"--json-file" / result.json_file, "--gh-bench-file" / result.gh_bench_file,
"--mode" / result.mode, "--cpus" / result.docker_limits.cpus, "--memory" /
result.docker_limits.memory, "--profile" / result.profile_dir, "--dhat-binary" /
result.dhat_binary, "--iterations" / result.iterations, "--results-file" /
result.results_file, "--results-dir" / result.results_dir, "--dashboard-file" /
result.dashboard_file, and any similar branches) so flags are rejected as
missing values consistently.

---

Duplicate comments:
In `@crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/main.rs`:
- Around line 456-467: The get_arg function currently treats the next argv token
as a value even if it's another flag; modify get_arg (in main.rs) so when you
find "--{name}" and i+1 < args.len() you verify that args[i+1] is not a flag
(e.g., does not start with "--"); if it is a flag, print the same error ("ERROR:
--{name} requires a value") and exit, otherwise return args[i+1].clone(); keep
the existing branch for i+1 out of range unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a0538f72-3290-4e79-8b74-102239bf35b3

📥 Commits

Reviewing files that changed from the base of the PR and between f952a4e and e956f0c.

📒 Files selected for processing (3)

• crates/logfwd-competitive-bench/src/main.rs
• crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/main.rs
• crates/logfwd-ebpf-proto/xdp-syslog/udp-bench/src/syslog_parser.rs