Add JsonReadHelpers and WslShellQuoting canonical helpers (additive)

docs/ARCHITECTURE.md

@@ -52,8 +52,8 @@ These are the canonical homes. Do not reintroduce private copies elsewhere.
 | Loopback MCP server for tests | `OpenClaw.TestSupport.FakeMcpServer` | authoritative |
 | Gateway record test data | `OpenClaw.Connection.Tests.GatewayRecordBuilder` | authoritative |
 | Settings test data | `OpenClaw.TestSupport.SettingsDataBuilder` | authoritative |
-| JSON `JsonElement` coercion | `JsonReadHelpers` (planned) | planned |
-| WSL/POSIX shell quoting | dedicated POSIX quote APIs (planned) | planned |
+| JSON `JsonElement` coercion (non-nullable fallback family) | `JsonReadHelpers` | authoritative |
+| WSL/POSIX shell quoting | `WslShellQuoting` | authoritative |
 | Capability UI metadata | `NodeCapabilityUiCatalog` (planned) | planned |
 | Capability registration/gating | `NodeCapabilityRegistrationPolicy` (planned) | planned |
 | Local MCP exposure policy | `McpCapabilityPolicy` (planned) | planned |
@@ -74,7 +74,7 @@ These are the canonical homes. Do not reintroduce private copies elsewhere.
 | `src/OpenClaw.Shared/Models.cs` | per-domain model files + `*Mapper` classes |
 | `src/OpenClaw.Shared/Capabilities/SystemCapability.cs` | `ExecApprovalService` |
 | `src/OpenClaw.Connection/GatewayConnectionManager.cs` | `NodeConnectionCoordinator`, `BootstrapTokenLifecycle`, `DevicePairApprovalCoordinator` |
-| `src/OpenClaw.SetupEngine/SetupSteps.cs` | one file per step; `WslShellClient`, `GatewayConfigScriptBuilder`, `KeepaliveProcessManager` |
+| `src/OpenClaw.SetupEngine/SetupSteps.cs` | one file per step; `WslShellClient`, `WslShellQuoting`, `GatewayConfigScriptBuilder`, `KeepaliveProcessManager` |
 | Any test hand-rolling a temp dir / env save-restore / CLI capture | `OpenClaw.TestSupport` fixtures |
 
 ## Ledger
@@ -107,8 +107,8 @@ leading and trailing pipe. Columns, in order:
 | test-fake-mcp | authoritative | OpenClaw.WinNode.Cli.Tests | private internal FakeMcpServer copy | OpenClaw.TestSupport.FakeMcpServer | - | one loopback MCP server captures method/body/auth and returns canned/timeout responses | TestSupportFixtureTests.FakeMcpServer_CapturesRequest | behavioral | when all MCP-round-trip tests share it |
 | test-gateway-builder | authoritative | OpenClaw.Connection.Tests | per-file MakeRecord(id,url) helpers | OpenClaw.Connection.Tests.GatewayRecordBuilder | pre-existing MakeRecord until migrated | gateway record test data has one builder | TestSupportFixtureTests.GatewayRecordBuilder_BuildsRecord | behavioral | when MakeRecord helpers are removed |
 | test-settings-builder | authoritative | scattered test files | ad hoc SettingsData construction in migrated tests | OpenClaw.TestSupport.SettingsDataBuilder | pre-existing un-migrated tests until adopted | settings test data starts from production defaults | TestSupportFixtureTests.SettingsDataBuilder_StartsFromDefaults | behavioral | when settings tests adopt the builder |
-| json-read-helpers | planned | OpenClaw.Shared (multiple files) | duplicate private GetString/TryGetString/FirstNonEmpty helpers | JsonReadHelpers | - | one JSON coercion helper with defined null/wrong-kind behavior | none | review-only | extracted in PR 2 |
-| wsl-posix-quoting | planned | OpenClaw.SetupEngine/SetupSteps.cs | ad hoc ShellEscape with divergent wrap semantics | dedicated POSIX quote APIs | - | WSL scripts use POSIX quoting, never cmd/PowerShell quoting | none | review-only | extracted in PR 2-3 |
+| json-read-helpers | authoritative | OpenClaw.Shared (multiple files) | duplicate non-nullable fallback-returning JsonElement getters | JsonReadHelpers | null-sentinel / non-negative / whitespace-absent / trimming variants stay separate | canonical non-nullable fallback JSON coercion; divergent-contract helpers are not blindly routed here | JsonReadHelpersTests.GetString_ReturnsNull_WhenPropertyMissing | behavioral | when the non-nullable fallback getters are all routed here |
+| wsl-posix-quoting | authoritative | OpenClaw.SetupEngine/SetupSteps.cs | ad hoc ShellEscape with divergent wrap semantics | WslShellQuoting | SetupSteps ShellEscape until PR 3 migration | WSL scripts use POSIX quoting not cmd/PowerShell quoting | WslShellQuotingTests.QuotePosixSingleQuote_WrapsAndEscapesEmbeddedQuote | behavioral | when SetupSteps call sites use WslShellQuoting |
 | app-window-manager | planned | src/OpenClaw.Tray.WinUI/App.xaml.cs | window creation/show/hide/shutdown | IWindowManager | composition/delegation only | startup/shutdown ordering deterministic; disposed once | none | review-only | extracted in Phase 3 |
 | app-tray-controller | planned | src/OpenClaw.Tray.WinUI/App.xaml.cs | tray icon/menu/action routing | ITrayController | composition/delegation only | tray actions route unchanged | none | review-only | extracted in Phase 3 |
 | app-activation-router | planned | src/OpenClaw.Tray.WinUI/App.xaml.cs | deep-link/toast/single-instance activation | IActivationRouter | composition/delegation only | activation routes land on the same UI/actions; current-user pipe security preserved | none | review-only | extracted in Phase 3 |

src/OpenClaw.Shared/JsonReadHelpers.cs

@@ -0,0 +1,188 @@
+using System.Diagnostics.CodeAnalysis;
+using System.Text.Json;
+
+namespace OpenClaw.Shared;
+
+/// <summary>
+/// Canonical home for the <em>non-nullable, fallback-returning</em> family of
+/// <see cref="JsonElement"/> coercion helpers: the gateway-client-style getters
+/// (<c>GetString</c> returning <c>null</c> when absent, numeric getters returning
+/// a caller-supplied fallback, <c>GetArrayLength</c>, and a verbatim
+/// <c>FirstNonEmpty</c>) plus the defensive non-object guard that the
+/// <c>GetStringSafe</c> variant added. See docs/ARCHITECTURE.md →
+/// <c>json-read-helpers</c>.
+///
+/// All accessors are <em>total</em> (they never throw): a missing property, a
+/// JSON null, a value of the wrong <see cref="JsonValueKind"/>, or a non-object
+/// <paramref name="parent"/> yields the documented fallback (<c>null</c> / 0 /
+/// the caller-supplied fallback) rather than an exception — including a non-object
+/// element, which <see cref="JsonElement.TryGetProperty(string, out JsonElement)"/>
+/// would otherwise throw on.
+///
+/// <para><b>Scope — do not blindly migrate.</b> This is deliberately NOT a
+/// drop-in for every private JSON helper in the repo. Several existing helpers
+/// have different contracts; routing them through these methods changes behavior
+/// and needs a call-site guard or a dedicated variant:</para>
+/// <list type="bullet">
+/// <item><c>Models.GetInt</c> / <c>Models.GetLong</c> return <c>int?</c> /
+/// <c>long?</c> (a null sentinel means "missing"), and <c>Models.GetLong</c> has
+/// no double-to-long fallback — the canonical <see cref="GetLong"/> instead
+/// truncates a fractional number.</item>
+/// <item><c>WorkspaceFilesModel.GetLong</c> rejects negatives (requires
+/// <c>&gt;= 0</c>); the canonical getters accept negative numbers.</item>
+/// <item><c>WindowsNodeClient.TryGetString</c> treats empty/whitespace as
+/// <em>absent</em>; the canonical <see cref="TryGetString"/> returns <c>true</c>
+/// with an empty string ("" is present).</item>
+/// <item><c>Mxc.MxcAvailability.FirstNonEmpty</c> trims its result, and
+/// <c>SetupSteps.FirstNonEmpty</c> trims and falls back to <c>"no output"</c>;
+/// the canonical <see cref="FirstNonEmpty"/> returns the first value verbatim, or
+/// <c>null</c>.</item>
+/// <item>Some older getters throw on a non-object <paramref name="parent"/>
+/// (failing the parse); the canonical getters are total and return the fallback.</item>
+/// </list>
+/// </summary>
+internal static class JsonReadHelpers
+{
+    /// <summary>
+    /// Returns the string value of <paramref name="property"/>, or <c>null</c>
+    /// when <paramref name="parent"/> is not an object, the property is absent, or
+    /// the value is not a JSON string. The returned string may be empty; only
+    /// missing / non-string values collapse to <c>null</c>.
+    /// </summary>
+    internal static string? GetString(JsonElement parent, string property)
+    {
+        if (parent.ValueKind != JsonValueKind.Object
+            || !parent.TryGetProperty(property, out var value)
+            || value.ValueKind != JsonValueKind.String)
+        {
+            return null;
+        }
+
+        return value.GetString();
+    }
+
+    /// <summary>
+    /// Try-pattern companion to <see cref="GetString"/>. Returns <c>true</c> and
+    /// sets <paramref name="value"/> when the property exists and is a JSON string
+    /// (the string may be empty); otherwise returns <c>false</c> with
+    /// <paramref name="value"/> set to <c>null</c>.
+    /// </summary>
+    internal static bool TryGetString(JsonElement parent, string property, [NotNullWhen(true)] out string? value)
+    {
+        value = GetString(parent, property);
+        return value is not null;
+    }
+
+    /// <summary>
+    /// Returns the <see cref="int"/> value of <paramref name="property"/>, or
+    /// <paramref name="fallback"/> when it is missing or not a JSON number. A
+    /// number within <see cref="long"/> range that exceeds the <see cref="int"/>
+    /// range is clamped to the <see cref="int"/> range; a number beyond
+    /// <see cref="long"/> range returns <paramref name="fallback"/> (matching the
+    /// prior private helper).
+    /// </summary>
+    internal static int GetInt(JsonElement parent, string property, int fallback = 0)
+    {
+        if (parent.ValueKind != JsonValueKind.Object
+            || !parent.TryGetProperty(property, out var value)
+            || value.ValueKind != JsonValueKind.Number)
+        {
+            return fallback;
+        }
+
+        if (value.TryGetInt32(out var intValue))
+        {
+            return intValue;
+        }
+
+        if (value.TryGetInt64(out var longValue))
+        {
+            return (int)Math.Clamp(longValue, int.MinValue, int.MaxValue);
+        }
+
+        return fallback;
+    }
+
+    /// <summary>
+    /// Returns the <see cref="long"/> value of <paramref name="property"/>, or
+    /// <paramref name="fallback"/> when it is missing or not a JSON number. A
+    /// fractional number is truncated toward zero.
+    /// </summary>
+    internal static long GetLong(JsonElement parent, string property, long fallback = 0)
+    {
+        if (parent.ValueKind != JsonValueKind.Object
+            || !parent.TryGetProperty(property, out var value)
+            || value.ValueKind != JsonValueKind.Number)
+        {
+            return fallback;
+        }
+
+        if (value.TryGetInt64(out var longValue))
+        {
+            return longValue;
+        }
+
+        if (value.TryGetDouble(out var doubleValue))
+        {
+            return (long)doubleValue;
+        }
+
+        return fallback;
+    }
+
+    /// <summary>
+    /// Returns the <see cref="double"/> value of <paramref name="property"/>, or
+    /// <paramref name="fallback"/> when it is missing or not a JSON number.
+    /// </summary>
+    internal static double GetDouble(JsonElement parent, string property, double fallback = 0)
+    {
+        if (parent.ValueKind != JsonValueKind.Object
+            || !parent.TryGetProperty(property, out var value)
+            || value.ValueKind != JsonValueKind.Number)
+        {
+            return fallback;
+        }
+
+        return value.TryGetDouble(out var doubleValue) ? doubleValue : fallback;
+    }
+
+    /// <summary>
+    /// Returns the number of elements in the array at <paramref name="property"/>,
+    /// or 0 when it is missing or not a JSON array.
+    /// </summary>
+    internal static int GetArrayLength(JsonElement parent, string property)
+    {
+        if (parent.ValueKind != JsonValueKind.Object
+            || !parent.TryGetProperty(property, out var value)
+            || value.ValueKind != JsonValueKind.Array)
+        {
+            return 0;
+        }
+
+        return value.GetArrayLength();
+    }
+
+    /// <summary>
+    /// Returns the first value that is not <c>null</c>, empty, or whitespace, or
+    /// <c>null</c> when none qualify. Values are returned verbatim (not trimmed).
+    /// </summary>
+    internal static string? FirstNonEmpty(params string?[] values)
+    {
+        // A bare `null` argument binds to the params array itself rather than a
+        // one-element array; guard so the canonical helper never throws.
+        if (values is null)
+        {
+            return null;
+        }
+
+        foreach (var value in values)
+        {
+            if (!string.IsNullOrWhiteSpace(value))
+            {
+                return value;
+            }
+        }
+
+        return null;
+    }
+}

src/OpenClaw.Shared/OpenClawGatewayClient.cs

@@ -3837,7 +3837,7 @@ private void ParseUsageCost(JsonElement payload)
             var summary = new GatewayCostUsageInfo
             {
                 UpdatedAt = ParseUnixTimestampMs(payload, "updatedAt") ?? DateTime.UtcNow,
-                Days = GetInt(payload, "days")
+                Days = JsonReadHelpers.GetInt(payload, "days")
             };
 
             if (payload.TryGetProperty("totals", out var totals) && totals.ValueKind == JsonValueKind.Object)
@@ -3850,7 +3850,7 @@ private void ParseUsageCost(JsonElement payload)
                     CacheWrite = GetLong(totals, "cacheWrite"),
                     TotalTokens = GetLong(totals, "totalTokens"),
                     TotalCost = GetDouble(totals, "totalCost"),
-                    MissingCostEntries = GetInt(totals, "missingCostEntries")
+                    MissingCostEntries = JsonReadHelpers.GetInt(totals, "missingCostEntries")
                 };
             }
 
@@ -3867,7 +3867,7 @@ private void ParseUsageCost(JsonElement payload)
                         CacheWrite = GetLong(day, "cacheWrite"),
                         TotalTokens = GetLong(day, "totalTokens"),
                         TotalCost = GetDouble(day, "totalCost"),
-                        MissingCostEntries = GetInt(day, "missingCostEntries")
+                        MissingCostEntries = JsonReadHelpers.GetInt(day, "missingCostEntries")
                     });
                 }
             }
@@ -4038,15 +4038,6 @@ private static string BuildProviderSummary(GatewayUsageStatusInfo status)
         };
     }
 
-    private static int GetInt(JsonElement parent, string property)
-    {
-        if (!parent.TryGetProperty(property, out var value) || value.ValueKind != JsonValueKind.Number)
-            return 0;
-        if (value.TryGetInt32(out var intVal)) return intVal;
-        if (value.TryGetInt64(out var longVal)) return (int)Math.Clamp(longVal, int.MinValue, int.MaxValue);
-        return 0;
-    }
-
     private static long GetLong(JsonElement parent, string property)
     {
         if (!parent.TryGetProperty(property, out var value) || value.ValueKind != JsonValueKind.Number)

src/OpenClaw.Shared/WslShellQuoting.cs

@@ -0,0 +1,56 @@
+namespace OpenClaw.Shared;
+
+/// <summary>
+/// POSIX / WSL single-quote quoting. This is a deliberately separate contract
+/// from <see cref="ShellQuoting"/>, which targets Windows cmd.exe / PowerShell
+/// (double quotes, or single quotes doubled as <c>''</c>). Those rules are
+/// <em>wrong and injection-unsafe</em> for the bash/sh command lines OpenClaw
+/// builds to run inside WSL.
+///
+/// A POSIX shell performs no processing whatsoever inside single quotes — there
+/// is no backslash escape for a single quote. The only way to place a literal
+/// single quote in a single-quoted string is to close the quote, emit an escaped
+/// literal quote, then reopen: the canonical <c>'\''</c> idiom. Every embedded
+/// single quote is replaced by that four-character sequence.
+///
+/// The two operations are kept distinct on purpose (see docs/ARCHITECTURE.md →
+/// <c>wsl-posix-quoting</c>); conflating "escape the inner text" with "produce a
+/// wrapped token" is exactly the divergent-semantics bug this type retires:
+/// <list type="bullet">
+/// <item><see cref="EscapePosixSingleQuoteInner"/> escapes embedded quotes for a
+/// value the caller is placing between its <em>own</em> outer single quotes, and
+/// adds no quotes of its own.</item>
+/// <item><see cref="QuotePosixSingleQuote"/> returns a complete, standalone
+/// single-quoted token (<c>''</c> for an empty value).</item>
+/// </list>
+/// Neither operation trims, collapses, or otherwise interprets the input: the
+/// bytes between the quotes are preserved verbatim, which is what makes the
+/// result injection-safe for arbitrary values (URLs, JSON, paths, newlines).
+/// </summary>
+internal static class WslShellQuoting
+{
+    // Close quote ('), an escaped literal quote (\'), then reopen quote (').
+    private const string EscapedSingleQuote = "'\\''";
+
+    /// <summary>
+    /// Escapes embedded single quotes using the POSIX <c>'\''</c> idiom, for a
+    /// value the caller will wrap in its own outer single quotes. Does NOT add
+    /// outer quotes. An empty input yields an empty string.
+    /// </summary>
+    internal static string EscapePosixSingleQuoteInner(string value)
+    {
+        ArgumentNullException.ThrowIfNull(value);
+        return value.Replace("'", EscapedSingleQuote);
+    }
+
+    /// <summary>
+    /// Wraps <paramref name="value"/> in single quotes, escaping any embedded
+    /// single quotes, so the result is exactly one POSIX-shell token. An empty
+    /// input yields <c>''</c> (an empty argument, not an omitted one).
+    /// </summary>
+    internal static string QuotePosixSingleQuote(string value)
+    {
+        ArgumentNullException.ThrowIfNull(value);
+        return string.Concat("'", EscapePosixSingleQuoteInner(value), "'");
+    }
+}