feat: unify param rendering between tools and benchmarks

[k-rister]

Summary

• dump_params() rendered benchmark params as unquoted --arg=val strings, so a value containing a space (e.g. bench-fio's exec_prerun/exec_postrun) silently corrupted the shell invocation via word-splitting.
• Fixing it surfaced a deeper issue: build_tool_cmd() built tool commands via a bash-syntax hack (declare -a ARGS=(...)) inherited from when the engine was Perl+Bash, giving tools and benchmarks two incompatible param_regex grammars for the same ON/OFF placeholder-value convention (bench-trafficgen/bench-fio vs. the still-draft tool-kernel PR#64).
• Both now share one render_param() helper (quotes a value only when the shell would otherwise misinterpret it), one param_regex grammar, and one JSON-argv transport: tool/benchmark commands ship as plain argv lists rather than pre-rendered shell strings, with shlex.join() applied exactly once, immediately before each run_command() call.
• This also closes a related hazard in process_bench_roadblocks()'s unbounded-timeout path, which used to wrap an already-rendered command string in a second layer of shell quoting for a wait_for re-invocation — it now hands roadblock a literal argv list instead (roadblock already supports this natively).
• Fixes two other consumers of the old flat-text bench-cmds format that would otherwise have broken: endpoints.py's own reader (shared by kube/remotehosts) and endpoints/base's equivalent for the osp endpoint.
• fio's and trafficgen's already-shipped param_regex patterns are unaffected — verified byte-for-byte identical output against their real values.

Fixes #867

Test plan

• python3 -m unittest discover -s tests -p "test_*.py" — 58 tests pass, including new coverage for render_param()/dump_params(), build_tool_cmd()'s new argv grammar, engine_lib.py's argv-handling (including the wait_for list-construction), and endpoints.py's bench-cmds JSON read.
• Manual empirical verification: a space-containing value survives render → param_regex → shlex.split() → JSON round-trip → shlex.join() intact.
• Live end-to-end crucible run against a real kube endpoint (fio + procstat), with exec_prerun set to "echo starting test" — completed successfully; result summary shows the value preserved as one coherent parameter, real fio results captured (18014 IOPS), and procstat metrics correctly indexed (interrupts-sec, processed-sec, time-squeeze-sec all non-zero).
• Code-review pass caught and fixed 2 additional value-coercion bugs (falsy 0/False values and JSON null mishandled) plus the endpoints/base gap above; all fixes have dedicated regression tests.

Follow-up (not part of this PR)

tool-kernel PR#64 is held in draft pending this change — its param_regex needs updating to the converged bare-text grammar once this merges (tracked separately, different repo).

[k-rister]

PR Review: rickshaw#868 — feat-unify-param-rendering

Summary: This PR unifies parameter rendering and execution command transport between benchmarks and tools by transitioning them to a single render_param() helper, standardizing on a converged param_regex grammar, and passing commands as structured JSON-argv lists rather than pre-rendered flat-text strings.
Changed files: 10
Review dimensions: Correctness, API & Contracts, Build & Deploy, Documentation, Style, Completeness

---

File Coverage

• endpoints/base — No issues found. Effectively modernized to iterate and parse structured start.json.xz utilizing xzcat and jq.
• endpoints/endpoints.py — No issues found. Safely reads from compressed JSON representation via load_json_file(..., uselzma=True).
• engine/engine_lib.py — No issues found. Command boundaries and list indexes are cleanly guarded to prevent out-of-bounds access. Defers shell quoting correctly using shlex.join().
• rickshaw-run.py — No issues found. Safely replaces client-id placeholders while keeping rendering consistent.
• schema/benchmark.json — No issues found. Updated descriptions clearly specify the modern converged substitution patterns.
• schema/tool.json — No issues found. Correctly introduces the unified param_regex descriptor block.
• tests/test_build_tool_cmd.py — No issues found. Thoroughly tests option edge-cases such as 0 values, ON/OFF rewrites, and unbalanced quotes.
• tests/test_dump_params.py — No issues found. Properly covers the unified parameter rendering output.
• tests/test_endpoints_process_bench_roadblocks.py — No issues found. Validates correct modern roadblock processing loops.
• tests/test_engine_lib.py — No issues found. Robust regression testing for the wait-for double-quoting hazards.

---

Limitations

• Runtime Environment Verification: The review is based on static analysis, verification of caller/callee contracts, and local unit test execution. We cannot verify execution behavior on active external targets (such as live Kubernetes clusters, OpenStack endpoints, or physical target remote hosts).

---

Verdict

Approve — Excellent and exceptionally robust refactoring of the command and parameter rendering subsystem. The implementation successfully eliminates shell word-splitting hazards, resolves the roadblock double-quoting issue, and maintains full style/convention compatibility. The thorough test coverage ensures high confidence for merging this PR 868 is functional and completely safe.

---

Review compiled and generated by Antigravity (AI Coding Assistant)
Powered by Gemini 1.5 Pro — Advanced Agentic Coding Team, Google DeepMind

[atheurer]

Approved after thorough code review and local test suite verification (all 58 tests passed).