Skip to content

[Repo Assist] refactor: extract TryParseArgv helper in SystemCapability, fix operator precedence#96

Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-system-capability-argv-task5-8b58b2e10649e089-8b435228792c34cf
Draft

[Repo Assist] refactor: extract TryParseArgv helper in SystemCapability, fix operator precedence#96
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-system-capability-argv-task5-8b58b2e10649e089-8b435228792c34cf

Conversation

@github-actions
Copy link
Contributor

🤖 This is an automated contribution from Repo Assist.

Two related improvements to SystemCapability in OpenClaw.Shared.

Changes

1. Extract TryParseArgv helper

HandleRunPrepare and HandleRunAsync both contained near-identical ~20-line JSON argv-parsing blocks. Both accept command as either a JSON string array or a single string. The logic is now consolidated into a single private static bool TryParseArgv(JsonElement, out string[]) method, with the parsing contract documented in one place.

Before: ~40 lines of duplicated argv parsing across two methods.
After: Two calls to TryParseArgv; each method focuses on its own concern.

2. Fix operator precedence in HandleExecApprovalsSet

The enabled boolean check was written as:

if (ruleEl.TryGetProperty("enabled", out var enEl) && enEl.ValueKind == JsonValueKind.True || enEl.ValueKind == JsonValueKind.False)

&& binds tighter than ||, so this evaluates as (A && B) || C rather than the intended A && (B || C). Added parentheses to make the intent unambiguous.

Test Status

503 tests pass, 18 skipped (integration/Windows-only). The refactor is a pure structural extraction with no logic changes — the observable behaviour of system.run, system.run.prepare, and system.execApprovals.set is unchanged.

Closes #91

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@cbb46ab386962aa371045839fc9998ee4e97ca64

…or 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Repo Assist] refactor: extract TryParseArgv helper in SystemCapability, fix operator precedence

0 participants