Skip to content

[Repo Assist] improve: SettingsData.FromJson null safety + GatewayUrlHelper static array#148

Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-settings-fromjson-null-safety-2026-04-05-38c88e68c788941a
Draft

[Repo Assist] improve: SettingsData.FromJson null safety + GatewayUrlHelper static array#148
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-settings-fromjson-null-safety-2026-04-05-38c88e68c788941a

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Apr 5, 2026

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Two small, independent coding improvements to SettingsData and GatewayUrlHelper.


1. SettingsData.FromJson — null/empty input safety

Problem: FromJson declared its parameter as string json (non-nullable). If a caller passes null (e.g. from NRT-unaware code or a test), JsonSerializer.Deserialize throws ArgumentNullException, which is not caught by the existing JsonException handler — the exception would propagate unexpectedly.

Fix: Change the parameter to string? json and return null immediately for null or empty input, consistent with the method's existing contract of returning null for invalid input.

// Before
public static SettingsData? FromJson(string json) { ... }

// After
public static SettingsData? FromJson(string? json)
{
    if (string.IsNullOrEmpty(json))
        return null;
    ...
}

Two regression tests added (null and "" both return null).


2. GatewayUrlHelper.RemoveUserInfo — eliminate per-call char array allocation

Problem: RemoveUserInfo contained an inline array literal:

var authorityEnd = url.IndexOfAny(new[] { '/', '?', '#' }, authorityStart);

This allocates a new char[] on the heap every time the method is called. RemoveUserInfo is called from both TryNormalizeWebSocketUrl and SanitizeForDisplay, which are invoked on every connection attempt and every URL-related UI update.

Fix: Extract to a private static readonly field:

private static readonly char[] s_authorityTerminators = { '/', '?', '#' };
// ...
var authorityEnd = url.IndexOfAny(s_authorityTerminators, authorityStart);

The allocation is now paid once at class initialisation rather than once per call.

Test Status

Suite Before After
OpenClaw.Shared.Tests 525 passed, 20 skipped 525 passed, 20 skipped (no change)
OpenClaw.Tray.Tests 99 passed 101 passed (+2 new null-safety tests)

All tests pass. No production logic was changed — only defensive input handling and a minor allocation improvement.

Generated by Repo Assist ·

To install this agentic workflow, run

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

…array

Two small coding improvements:

1. SettingsData.FromJson(string? json) — null/empty guard
   - Change parameter from non-nullable 'string' to 'string?'
   - Return null immediately for null or empty input rather than
     forwarding to JsonSerializer.Deserialize which would throw
     ArgumentNullException (not caught by the existing JsonException
     handler)
   - Adds two regression tests: null and empty string both return null

2. GatewayUrlHelper.RemoveUserInfo — static readonly char array
   - Replace the inline 'new[] { '/', '?', '#' }' array literal
     in RemoveUserInfo with a 'private static readonly' field
   - The method is called on every URL normalisation and display
     sanitisation; the previous form allocated a new char array on
     each call, which is now eliminated

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.

0 participants