Skip to content

[Repo Assist] improve: remove debug scaffolding and double-serialization from WindowsNodeClient#150

Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-remove-debug-scaffolding-20260406-5b3f0f30cbd22f9a
Draft

[Repo Assist] improve: remove debug scaffolding and double-serialization from WindowsNodeClient#150
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-remove-debug-scaffolding-20260406-5b3f0f30cbd22f9a

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

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

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

Summary

Removes leftover development-time debug scaffolding from WindowsNodeClient.SendNodeConnectAsync and HandleResponse, and eliminates a redundant JSON serialization.


1. Remove sensitive debug logging block in SendNodeConnectAsync

Problem: A === Debug Info === block logged on every connect attempt included:

  • First 16 chars of the auth token
  • The Ed25519 signature (in full)
  • The constructed signing payload (contains auth token + nonce)
  • The full indented JSON of the connect request (includes auth.token)

These messages fired at Debug level in all builds (no #if DEBUG guard), via NullLogger which writes to Console.WriteLine and AppLogger which writes to a log file. While each message truncates aggressively, the pattern poses an unnecessary exposure risk.

Additionally, BuildDebugPayload() was called explicitly just for the log, then called again internally by SignPayload() — two allocations of the same string per connect.

Fix: Keep only the SignPayload() call; remove the logging block and the redundant BuildDebugPayload() call.


2. Eliminate double JSON serialization in SendNodeConnectAsync

Problem: The connect message object was serialized twice:

var json = JsonSerializer.Serialize(msg, s_indentedOptions);   // for debug log
_logger.Debug($"[NODE TX FULL JSON]:\n{json}");
await SendRawAsync(JsonSerializer.Serialize(msg));              // actually sent

The debug log logged a pretty-printed version; the actual send used a separate compact serialization. Besides the unnecessary CPU/memory cost, the log and the wire payload were technically different strings (whitespace).

Fix: Serialize once, send it, remove the s_indentedOptions field (now unused).


3. Remove HandleResponse debug log

Problem: HandleResponse allocated a full JSON string from payload.ToString() on every response, then truncated to 200 chars just to emit a Debug-level message. The // DEBUG: comment confirmed this was scaffolding.

Fix: Remove the comment, the payloadText allocation, and the truncated log. The existing _logger.Warn for missing payload is retained.


Trade-offs

  • No functional change — connection, authentication, and registration behaviour are identical.
  • The BuildDebugPayload() public method on DeviceIdentity is still present (used by SignPayload internally and by tests) — no public API changes.

Test Status

Suite Result
OpenClaw.Shared.Tests ✅ 525 passed, 20 skipped, 0 failed
OpenClaw.Tray.Tests ✅ 99 passed, 0 failed

Generated by Repo Assist ·

To install this agentic workflow, run

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

…wsNodeClient

- Remove verbose debug block in SendNodeConnectAsync that logged sensitive
  data (auth token prefix, Ed25519 signature, full connect payload) on every
  connect attempt; this ran in all builds, not just debug.
- Eliminate redundant BuildDebugPayload() call (was called twice: once for
  logging, once inside SignPayload) — only the SignPayload call remains.
- Remove duplicate JSON serialization: msg was serialized twice (once with
  WriteIndented for the debug log, once compact to actually send); collapse
  to a single compact serialization and remove the now-unused s_indentedOptions
  field.
- Remove HandleResponse debug log that allocated a full payload string just
  to truncate it for a Debug()-level message.

No functional change; connection, signing, and registration behaviour are
identical. Test status: 525 Shared passed, 99 Tray passed.

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