Skip to content

[Repo Assist] refactor: eliminate inline JsonSerializerOptions allocations in Shared and SetupEngine#566

Draft
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/improve-json-options-cache-2026-05-28-390f40d2ad6ac5f9
Draft

[Repo Assist] refactor: eliminate inline JsonSerializerOptions allocations in Shared and SetupEngine#566
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/improve-json-options-cache-2026-05-28-390f40d2ad6ac5f9

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

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

Summary

Replaces 9 repeated new JsonSerializerOptions { WriteIndented = true } inline allocations scattered across OpenClaw.Shared and OpenClaw.SetupEngine with shared static singletons.

Motivation

Creating a new JsonSerializerOptions instance at every call site:

  • Allocates a new heap object on every invocation (including hot paths like DataJson in Models.cs, which is a cached property)
  • Creates risk of settings drift if future options are added to some sites but not others
  • Produces noise when reading call sites: the intent (pretty-print JSON) is buried in a repeated constructor expression

Changes

OpenClaw.Shared

  • New file JsonSerializerOptionsCache.cs: internal static class JsonSerializerOptionsCache with a WriteIndented singleton for pretty-print serialization
  • Fix 3 inline allocations: ChannelConfigPatchBuilder.cs, DeviceIdentity.cs, Models.cs

OpenClaw.SetupEngine

  • SetupConfig.JsonWriteOptions added alongside the existing SetupConfig.JsonOptions (which is for reading, with comment/trailing-comma tolerance)
  • Fix 6 inline allocations: Program.cs, SetupContext.cs, SetupSteps.cs (×2), SetupWizardRunner.cs, TrayArtifactCleanup.cs

No behavior change

The replacement singletons have identical settings (WriteIndented = true only). Output JSON is byte-for-byte identical.

Test Status

  • ./build.ps1 — skipped (Linux CI; requires Windows)
  • dotnet build OpenClaw.Shared — ✅ Build succeeded (1 pre-existing warning, unrelated)
  • dotnet build OpenClaw.SetupEngine — ✅ Build succeeded (1 pre-existing warning, unrelated)
  • dotnet test OpenClaw.Shared.Tests — ✅ 2015 passed, 29 skipped, 8 pre-existing failures (Windows-specific path tests)
  • dotnet test OpenClaw.Tray.Tests — ✅ 858 passed, 2 skipped, 0 failures

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

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

…d and SetupEngine

Add JsonSerializerOptionsCache.WriteIndented in OpenClaw.Shared and
SetupConfig.JsonWriteOptions in OpenClaw.SetupEngine to replace
10 inline `new JsonSerializerOptions { WriteIndented = true }`
allocations scattered across both projects.

- Create src/OpenClaw.Shared/JsonSerializerOptionsCache.cs with a
  shared WriteIndented singleton for pretty-print serialization
- Fix 3 inline allocations in ChannelConfigPatchBuilder, DeviceIdentity,
  and Models
- Add SetupConfig.JsonWriteOptions alongside the existing JsonOptions
- Fix 6 inline allocations across Program, SetupContext, SetupSteps,
  SetupWizardRunner, and TrayArtifactCleanup

No behavior change: the replacement instances have identical settings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 28, 2026

Codex review: needs maintainer review before merge. Reviewed May 27, 2026, 9:28 PM ET / 01:28 UTC.

Summary
This PR replaces inline WriteIndented JsonSerializerOptions allocations in OpenClaw.Shared and OpenClaw.SetupEngine with shared static option instances.

Reproducibility: not applicable. this is a cleanup PR, not a bug report. Source inspection confirms the edited call sites currently allocate inline WriteIndented JSON options on main.

Review metrics: 2 noteworthy metrics.

  • Touched surface: 9 files, +38/-10. The refactor crosses OpenClaw.Shared and OpenClaw.SetupEngine, so both affected assemblies need validation before merge.
  • Reported validation gap: ./build.ps1 skipped; Shared tests report 8 failures. Repository policy requires the full build plus Shared and Tray test validation after code changes.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Run or require the repository-required ./build.ps1 on Windows before merge.

Risk before merge

  • The PR body says ./build.ps1 was skipped and reports 8 Shared test failures as pre-existing, so maintainers should rely on required Windows validation or equivalent CI before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land the narrow static-options refactor after the required Windows build and Shared/Tray test validation are green or explicitly accepted by maintainers.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No automated repair is needed; the remaining action is maintainer or CI validation of a small cleanup PR.

Security
Cleared: No security or supply-chain concern found; the diff only reuses local System.Text.Json options in C# source.

Review details

Best possible solution:

Land the narrow static-options refactor after the required Windows build and Shared/Tray test validation are green or explicitly accepted by maintainers.

Do we have a high-confidence way to reproduce the issue?

Not applicable; this is a cleanup PR, not a bug report. Source inspection confirms the edited call sites currently allocate inline WriteIndented JSON options on main.

Is this the best way to solve the issue?

Yes; centralizing write-only options per assembly is a narrow maintainable solution, and the proposed settings match the existing inline serializer options.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6a5921883c86.

Label changes

Label changes:

  • add P3: This is a low-risk cleanup/refactor with no observed user-facing behavior change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The real-behavior proof gate is not applied because this is an automated bot cleanup PR with no user-visible runtime behavior change.

Label justifications:

  • P3: This is a low-risk cleanup/refactor with no observed user-facing behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The real-behavior proof gate is not applied because this is an automated bot cleanup PR with no user-visible runtime behavior change.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its required validation guidance is relevant because the PR body says ./build.ps1 was skipped and reports shared-test failures in the test status. (AGENTS.md:1, 6a5921883c86)
  • Current source has the targeted inline allocations: Current main still has inline new JsonSerializerOptions { WriteIndented = true } allocations across the Shared and SetupEngine files targeted by the PR. (src/OpenClaw.SetupEngine/Program.cs:194, 6a5921883c86)
  • Proposed behavior is mechanically narrow: The provided PR diff adds one Shared cache and one SetupEngine write-options singleton, then changes the targeted serializers to use those WriteIndented-only options. (src/OpenClaw.Shared/JsonSerializerOptionsCache.cs:1, 52f4e9877895)
  • Existing SetupEngine read options are separate: SetupConfig.JsonOptions includes read-tolerance and null-ignore settings, while the PR adds a separate write-only option matching the previous inline write settings. (src/OpenClaw.SetupEngine/SetupContext.cs:66, 6a5921883c86)
  • Feature history: Blame shows the Shared inline JSON write sites were introduced with the connection work in ed126f2, and SetupEngine write sites were introduced with the out-of-process SetupEngine redesign in cefce395. (src/OpenClaw.Shared/ChannelConfigPatchBuilder.cs:134, ed126f2aac6d)
  • Maintainer notes check: No .agents/maintainer-notes/ directory exists in this checkout, so there were no matching internal maintainer notes to apply.

Likely related people:

  • Ranjesh: Git blame ties the Shared JSON write sites to ed126f2 and the SetupEngine JSON write sites to cefce395, both authored by Ranjesh shortly before this PR. (role: introduced behavior and recent area contributor; confidence: high; commits: ed126f2aac6d, cefce3952ab1; files: src/OpenClaw.Shared/ChannelConfigPatchBuilder.cs, src/OpenClaw.Shared/DeviceIdentity.cs, src/OpenClaw.Shared/Models.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels May 28, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 28, 2026

ClawSweeper PR egg

✨ Hatched: 💎 rare Mossy Lint Imp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 💎 rare.
Trait: stacks clean commits.
Image traits: location merge queue dock; accessory CI status badge; palette seafoam, black, and opal; mood patient; pose curling around a status light; shell starlit enamel shell; lighting soft underwater shimmer; background small review tokens.
Share on X: post this hatch
Copy: My PR egg hatched a 💎 rare Mossy Lint Imp in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. repo-assist status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants