From 141536d34f9855e5ad7e0494c4bfd199bb13ef9c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 20 Mar 2026 01:00:05 +0000 Subject: [PATCH] refactor: extract TryParseArgv helper in SystemCapability, fix operator precedence Two related improvements to SystemCapability: 1. Extract TryParseArgv helper: HandleRunPrepare and HandleRunAsync both contained near-identical JSON argv-array parsing logic (~20 lines each). Extract into a private static TryParseArgv method. Both callers are simplified and the parsing contract is documented in one place. 2. Fix operator precedence in HandleExecApprovalsSet: the 'enabled' bool check was written as: TryGetProperty(...) && kind == True || kind == False which C# parses as: (TryGetProperty(...) && kind == True) || (kind == False) rather than the intended: TryGetProperty(...) && (kind == True || kind == False) Behaviour happened to be correct in all cases (default JsonElement has ValueKind == Undefined, not False), but the unparenthesised form is misleading. Added parentheses to make intent explicit. 503 tests pass, 18 skipped (integration/Windows-only). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Capabilities/SystemCapability.cs | 133 +++++++++--------- 1 file changed, 63 insertions(+), 70 deletions(-) diff --git a/src/OpenClaw.Shared/Capabilities/SystemCapability.cs b/src/OpenClaw.Shared/Capabilities/SystemCapability.cs index 986f3fb..4c89ae7 100644 --- a/src/OpenClaw.Shared/Capabilities/SystemCapability.cs +++ b/src/OpenClaw.Shared/Capabilities/SystemCapability.cs @@ -161,46 +161,60 @@ private NodeInvokeResponse HandleWhich(NodeInvokeRequest request) } private static string FormatExecCommand(string[] argv) => ShellQuoting.FormatExecCommand(argv); - + /// - /// Pre-flight for system.run: echoes back the execution plan without running anything. - /// The gateway uses this to build its approval context before the actual run. + /// Parses the "command" field from a request's JSON args into an argv array. + /// Accepts either a JSON string array or a single string. + /// Returns true when at least one non-empty element was extracted. /// - private NodeInvokeResponse HandleRunPrepare(NodeInvokeRequest request) + private static bool TryParseArgv(System.Text.Json.JsonElement args, out string[] argv) { - string? command = null; - string[]? argv = null; - string? rawCommand = null; - string? cwd = null; - - if (request.Args.ValueKind != System.Text.Json.JsonValueKind.Undefined && - request.Args.TryGetProperty("command", out var cmdEl)) + argv = []; + if (args.ValueKind == System.Text.Json.JsonValueKind.Undefined || + !args.TryGetProperty("command", out var cmdEl)) { - if (cmdEl.ValueKind == System.Text.Json.JsonValueKind.Array) + return false; + } + + if (cmdEl.ValueKind == System.Text.Json.JsonValueKind.Array) + { + var list = new List(); + foreach (var item in cmdEl.EnumerateArray()) { - var list = new List(); - foreach (var item in cmdEl.EnumerateArray()) - { - if (item.ValueKind == System.Text.Json.JsonValueKind.String) - list.Add(item.GetString() ?? ""); - } - argv = list.ToArray(); - command = argv.Length > 0 ? argv[0] : null; + if (item.ValueKind == System.Text.Json.JsonValueKind.String) + list.Add(item.GetString() ?? ""); } - else if (cmdEl.ValueKind == System.Text.Json.JsonValueKind.String) + argv = list.ToArray(); + return argv.Length > 0; + } + + if (cmdEl.ValueKind == System.Text.Json.JsonValueKind.String) + { + var cmd = cmdEl.GetString(); + if (!string.IsNullOrWhiteSpace(cmd)) { - command = cmdEl.GetString(); - argv = command != null ? new[] { command } : null; + argv = [cmd]; + return true; } } - - if (string.IsNullOrWhiteSpace(command) || argv == null || argv.Length == 0) + + return false; + } + + /// + /// Pre-flight for system.run: echoes back the execution plan without running anything. + /// The gateway uses this to build its approval context before the actual run. + /// + private NodeInvokeResponse HandleRunPrepare(NodeInvokeRequest request) + { + if (!TryParseArgv(request.Args, out var argv) || string.IsNullOrWhiteSpace(argv[0])) { return Error("Missing command parameter"); } - - rawCommand = GetStringArg(request.Args, "rawCommand"); - cwd = GetStringArg(request.Args, "cwd"); + + var command = argv[0]; + var rawCommand = GetStringArg(request.Args, "rawCommand"); + var cwd = GetStringArg(request.Args, "cwd"); var agentId = GetStringArg(request.Args, "agentId"); var sessionKey = GetStringArg(request.Args, "sessionKey"); @@ -226,52 +240,31 @@ private async Task HandleRunAsync(NodeInvokeRequest request) { return Error("Command execution not available"); } - + // Per OpenClaw spec, "command" is an argv array (e.g. ["echo","Hello"]). // Also accept a plain string for backward compatibility. - string? command = null; - string[]? args = null; - - if (request.Args.ValueKind != System.Text.Json.JsonValueKind.Undefined && - request.Args.TryGetProperty("command", out var cmdEl)) + if (!TryParseArgv(request.Args, out var argv) || string.IsNullOrWhiteSpace(argv[0])) { - if (cmdEl.ValueKind == System.Text.Json.JsonValueKind.Array) - { - var argv = new List(); - foreach (var item in cmdEl.EnumerateArray()) - { - if (item.ValueKind == System.Text.Json.JsonValueKind.String) - argv.Add(item.GetString() ?? ""); - } - if (argv.Count > 0) - { - command = argv[0]; - args = argv.Count > 1 ? argv.Skip(1).ToArray() : null; - } - } - else if (cmdEl.ValueKind == System.Text.Json.JsonValueKind.String) - { - command = cmdEl.GetString(); - - // When command is a string, also check for separate "args" array - if (request.Args.TryGetProperty("args", out var argsEl) && - argsEl.ValueKind == System.Text.Json.JsonValueKind.Array) - { - var list = new List(); - foreach (var item in argsEl.EnumerateArray()) - { - if (item.ValueKind == System.Text.Json.JsonValueKind.String) - list.Add(item.GetString() ?? ""); - } - if (list.Count > 0) - args = list.ToArray(); - } - } + return Error("Missing command parameter"); } - - if (string.IsNullOrWhiteSpace(command)) + + var command = argv[0]; + string[]? args = argv.Length > 1 ? argv[1..] : null; + + // When command is a plain string (not an array), also check for a separate "args" array. + if (argv.Length == 1 && + request.Args.ValueKind != System.Text.Json.JsonValueKind.Undefined && + request.Args.TryGetProperty("args", out var argsEl) && + argsEl.ValueKind == System.Text.Json.JsonValueKind.Array) { - return Error("Missing command parameter"); + var list = new List(); + foreach (var item in argsEl.EnumerateArray()) + { + if (item.ValueKind == System.Text.Json.JsonValueKind.String) + list.Add(item.GetString() ?? ""); + } + if (list.Count > 0) + args = list.ToArray(); } var shell = GetStringArg(request.Args, "shell"); @@ -402,7 +395,7 @@ private NodeInvokeResponse HandleExecApprovalsSet(NodeInvokeRequest request) if (ruleEl.TryGetProperty("description", out var descEl) && descEl.ValueKind == System.Text.Json.JsonValueKind.String) rule.Description = descEl.GetString(); - if (ruleEl.TryGetProperty("enabled", out var enEl) && enEl.ValueKind == System.Text.Json.JsonValueKind.True || enEl.ValueKind == System.Text.Json.JsonValueKind.False) + if (ruleEl.TryGetProperty("enabled", out var enEl) && (enEl.ValueKind == System.Text.Json.JsonValueKind.True || enEl.ValueKind == System.Text.Json.JsonValueKind.False)) rule.Enabled = enEl.GetBoolean(); if (ruleEl.TryGetProperty("shells", out var shellsEl) && shellsEl.ValueKind == System.Text.Json.JsonValueKind.Array)