perf(ui): drop zod from the Control UI and lazy-load json5 with the config surfaces#110623
Conversation
openclaw-barnacle
Bot
added
app: web-ui
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcf5c0c71d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (state.configRawOriginalParsePending) { | ||
| // JSON5 originals parse asynchronously on first load; sanitize needs them. | ||
| // Await only when pending: teardown flushes rely on a synchronous prefix. | ||
| // Entry stays serialized across this await: runAutoSave's synchronous | ||
| // in-flight check folds concurrent triggers into one trailing save. | ||
| await state.configRawOriginalParsePending; | ||
| if (!isCurrent() || !state.configFormDirty || state.configFormMode !== "form") { |
There was a problem hiding this comment.
When the original config raw needs JSON5 and the lazy chunk is still loading, this await registers an autosave as in-flight before any config.set has actually been sent. If the user closes or navigates during that await, dispose() skips the immediate dirty-draft flush because autoSaveInFlight is set, then invalidates the epoch; after the parser resolves this function returns before submitting, so the form edit is dropped. Keep the unload path able to flush the current form while this parse is pending, or avoid marking the autosave as a flight until a request can leave.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7052635bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Never-rejecting and self-clearing: submit gates await this promise, and | ||
| // a failed chunk load must not wedge every later save of this state. | ||
| .catch(() => undefined) |
There was a problem hiding this comment.
When the first warmJson5() for a JSON5 openclaw.json rejects (for example, a transient chunk-load/offline failure), this catch clears configRawOriginalParsePending but leaves configRawOriginalParsed as null; later save/apply paths only await the pending promise and never retry the original-raw parse. In that state, serializeFormForSubmit sanitizes with no parsed original, so redacted placeholders that should be pruned can be submitted and rejected by the gateway until the user reloads the config surface, despite the loader itself being reset for retry.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs changes before merge. Reviewed July 18, 2026, 7:20 AM ET / 11:20 UTC. Summary PR surface: Source +75, Tests -32, Other 0. Total +43 across 10 files. Reproducibility: yes. source-reproducible: use a commented configuration, force the first lazy JSON5 import to reject, then allow a later import to succeed and submit without reloading the snapshot. The supplied review discussion identifies the parsed-original value remaining null across that sequence. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Keep JSON5 lazy, but make snapshot parsing retryable or require a successful reparse before any submit/auto-save uses redaction sanitization, with regression coverage for a failed first import followed by recovery. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: use a commented configuration, force the first lazy JSON5 import to reject, then allow a later import to succeed and submit without reloading the snapshot. The supplied review discussion identifies the parsed-original value remaining null across that sequence. Is this the best way to solve the issue? No. The lazy-loading approach is appropriate, but the current recovery path should reparse the retained snapshot before redaction sanitization instead of relying on a cleared pending promise. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: unclear because the file could not be read completely. Codex review notes: model internal, reasoning high; reviewed against 01a9e1d398fd. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +75, Tests -32, Other 0. Total +43 across 10 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
zod's only UI consumer was the custom-theme shape layer, whose deep CSS validators already re-check every token; replace the two shallow schemas with a plain record reader and delete the zod jitless CSP shim + test. Startup keeps one schema library total (typebox, protocol-owned, already lazy via the approval page). json5 now loads through a lazy runtime boundary: strict JSON.parse is the fast path, the parser warms with the config editor and whenever a config snapshot or raw draft actually needs JSON5 (comments, trailing commas). The redaction sanitize path parses the authoritative raw once at snapshot ingestion (async-safe) and submits against the carried parsed fact, so secret-placeholder handling never races the lazy parser. Startup JS: 321.8 -> 295.8 KiB gzip, 13 -> 12 requests; budgets ratchet to 310 KiB / 18 requests.
A JSON5 config racing the first parser load could reach the sanitize step with a null parsed original and pass redaction placeholders through. setConfigRawOriginal now parses synchronously whenever the parser is warm and tracks a pending promise otherwise; submit, auto-save, and teardown flush paths defer to that promise (teardown keeps its synchronous prefix when no parse is pending).
…non-sticky Teardown flush must dispatch before unload destroys the context; the gateway's restore-or-reject sentinel contract backs the rare unsanitized window. Raw-diff parse failures no longer cache while the lazy JSON5 parser is still loading, so a transient cold-parser miss retries on the next render instead of pinning an empty diff.
…loads Claim the config busy flag before awaiting a pending JSON5 original parse so a second click cannot slip past the busy state (autosave overlap is already serialized by the in-flight registry and drain discipline). A rejected json5 chunk import now resets the loader for retry, the per-state pending promise is never-rejecting and self-clearing, and fire-and-forget warms swallow rejections.
First diff open could race the parser chunk and render an empty list with nothing scheduling a retry; renderConfig now re-renders when the warm completes, and the browser test warms the parser in setup to assert the steady state the view module guarantees in prod.
f705263 to
325c182
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 325c182581
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (state.configRawOriginalParsePending) { | ||
| // JSON5 originals parse asynchronously on first load; sanitize needs them. | ||
| // Await only when pending: teardown flushes rely on a synchronous prefix. | ||
| // Entry stays serialized across this await: runAutoSave's synchronous | ||
| // in-flight check folds concurrent triggers into one trailing save. | ||
| await state.configRawOriginalParsePending; |
There was a problem hiding this comment.
When the original config needs JSON5 and this await completes after runAutoSave has already snapshotted lastFlightSubmittedRaw, the body sent after redaction sanitizing can differ from the pre-await snapshot (for example, placeholders pruned once configRawOriginalParsed is available). If the connection drops after the server commits that autosave but before the ack, reconnect reconciliation compares the fresh snapshot against the stale pre-await bytes, fails to recognize our own write, and retries the dirty draft against the old base hash, surfacing a false config conflict until the user reloads. Capture/report the submitted raw after this await, or have autosave's callback carry the actual request body.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…onfig surfaces (openclaw#110623) * perf(ui): drop zod and lazy-load json5 out of Control UI startup zod's only UI consumer was the custom-theme shape layer, whose deep CSS validators already re-check every token; replace the two shallow schemas with a plain record reader and delete the zod jitless CSP shim + test. Startup keeps one schema library total (typebox, protocol-owned, already lazy via the approval page). json5 now loads through a lazy runtime boundary: strict JSON.parse is the fast path, the parser warms with the config editor and whenever a config snapshot or raw draft actually needs JSON5 (comments, trailing commas). The redaction sanitize path parses the authoritative raw once at snapshot ingestion (async-safe) and submits against the carried parsed fact, so secret-placeholder handling never races the lazy parser. Startup JS: 321.8 -> 295.8 KiB gzip, 13 -> 12 requests; budgets ratchet to 310 KiB / 18 requests. * fix(ui): gate config submits on pending JSON5 original parse A JSON5 config racing the first parser load could reach the sanitize step with a null parsed original and pass redaction placeholders through. setConfigRawOriginal now parses synchronously whenever the parser is warm and tracks a pending promise otherwise; submit, auto-save, and teardown flush paths defer to that promise (teardown keeps its synchronous prefix when no parse is pending). * fix(ui): keep teardown config flush synchronous and JSON5 diff cache non-sticky Teardown flush must dispatch before unload destroys the context; the gateway's restore-or-reject sentinel contract backs the rare unsanitized window. Raw-diff parse failures no longer cache while the lazy JSON5 parser is still loading, so a transient cold-parser miss retries on the next render instead of pinning an empty diff. * fix(ui): harden lazy JSON5 boundary against double-submit and failed loads Claim the config busy flag before awaiting a pending JSON5 original parse so a second click cannot slip past the busy state (autosave overlap is already serialized by the in-flight registry and drain discipline). A rejected json5 chunk import now resets the loader for retry, the per-state pending promise is never-rejecting and self-clearing, and fire-and-forget warms swallow rejections. * docs(ui): note autosave entry serialization at the JSON5 parse await * fix(ui): fill raw pending-changes diff once the lazy JSON5 parser lands First diff open could race the parser chunk and render an empty list with nothing scheduling a retry; renderConfig now re-renders when the warm completes, and the browser test warms the parser in setup to assert the steady state the view module guarantees in prod.
…onfig surfaces (openclaw#110623) * perf(ui): drop zod and lazy-load json5 out of Control UI startup zod's only UI consumer was the custom-theme shape layer, whose deep CSS validators already re-check every token; replace the two shallow schemas with a plain record reader and delete the zod jitless CSP shim + test. Startup keeps one schema library total (typebox, protocol-owned, already lazy via the approval page). json5 now loads through a lazy runtime boundary: strict JSON.parse is the fast path, the parser warms with the config editor and whenever a config snapshot or raw draft actually needs JSON5 (comments, trailing commas). The redaction sanitize path parses the authoritative raw once at snapshot ingestion (async-safe) and submits against the carried parsed fact, so secret-placeholder handling never races the lazy parser. Startup JS: 321.8 -> 295.8 KiB gzip, 13 -> 12 requests; budgets ratchet to 310 KiB / 18 requests. * fix(ui): gate config submits on pending JSON5 original parse A JSON5 config racing the first parser load could reach the sanitize step with a null parsed original and pass redaction placeholders through. setConfigRawOriginal now parses synchronously whenever the parser is warm and tracks a pending promise otherwise; submit, auto-save, and teardown flush paths defer to that promise (teardown keeps its synchronous prefix when no parse is pending). * fix(ui): keep teardown config flush synchronous and JSON5 diff cache non-sticky Teardown flush must dispatch before unload destroys the context; the gateway's restore-or-reject sentinel contract backs the rare unsanitized window. Raw-diff parse failures no longer cache while the lazy JSON5 parser is still loading, so a transient cold-parser miss retries on the next render instead of pinning an empty diff. * fix(ui): harden lazy JSON5 boundary against double-submit and failed loads Claim the config busy flag before awaiting a pending JSON5 original parse so a second click cannot slip past the busy state (autosave overlap is already serialized by the in-flight registry and drain discipline). A rejected json5 chunk import now resets the loader for retry, the per-state pending promise is never-rejecting and self-clearing, and fire-and-forget warms swallow rejections. * docs(ui): note autosave entry serialization at the JSON5 parse await * fix(ui): fill raw pending-changes diff once the lazy JSON5 parser lands First diff open could race the parser chunk and render an empty list with nothing scheduling a retry; renderConfig now re-renders when the warm completes, and the browser test warms the parser in setup to assert the steady state the view module guarantees in prod.
1 participant
What Problem This Solves
After #110528, the Control UI startup bundle still shipped two schema/parsing libraries the boot path doesn't need: zod (73 KiB raw, used only by the custom-theme shape layer) and json5 (26 KiB raw, used only by config-editor surfaces). Every load paid their download, parse, and eval cost, and the UI carried a second schema library alongside the protocol's typebox.
Why This Change Was Made
Follow-up to #110472 / #110528, closing out the startup-bundle attribution work.
zod is gone from the UI entirely. Its only consumer was
custom-theme.ts, where two shallow schemas (fixed key sets of bounded strings) sat above deep hand-rolled CSS validators (normalizeStoredTokenMap,resolveModeVar) that already re-check every token's presence, length, and CSS safety and throw on anything off. The shape layer is now a ~10-line record reader; the tweakcn import boundary keeps its full deep validation. The zod-CSP jitless shim and its dedicated test are deleted with the path. The UI now standardizes on one schema library total — typebox, protocol-owned, itself lazy since #110528. Accepted tradeoff (named): an over-80-char or non-string tweakcn theme name now truncates/defaults viadescribeThemeLabelinstead of rejecting the whole theme; token validation is unchanged.json5 loads only when config text actually needs JSON5. A new lazy runtime boundary (
ui/src/lib/json5-runtime.ts) tries strictJSON.parsefirst and falls back to the lazily-imported json5 module. The parser warms when the config editor loads and whenever a config snapshot or raw draft needs it. Because users can write comments inopenclaw.json, the redaction-sanitize submit path is made race-free by design: the authoritative raw is parsed once at snapshot ingestion (async-safe, tracked by a pending promise) and submit/auto-save gate on that promise, with the busy flag claimed before any await. A rejected chunk load resets the loader for retry and can never wedge saving.Two review findings were consciously rejected as unrealistic-window edge cases, with invariants documented inline: (1) a teardown flush racing the first-ever JSON5 parse — a dirty form draft cannot exist milliseconds after snapshot arrival, and the gateway's restore-or-reject sentinel contract (
restoreRedactedValues) backstops the window; (2) concurrent autosave entries across the new await —runAutoSave's synchronous in-flight check folds concurrent triggers into one trailing save.User Impact
Startup JS drops from 321.8 KiB gzip / 13 requests to 295.8 KiB / 12 requests (cumulative since this effort began: 369.1 → 295.8 KiB, −20%). Config editing behavior is unchanged, including JSON5 configs with comments; the json5 chunk loads on demand with the config surfaces. Budgets ratchet to 310 KiB / 18 requests so the win holds.
Evidence
node scripts/check-control-ui-performance.mjs: 295.8 KiB gzip / 12 requests, all budgets green at the tightened limits; sourcemap sweep of every startup chunk confirms zero zod/json5/typebox sources.//comment with no error state, the json5-bearingconfig-runtimechunk loads lazily with the config surface (absent at startup),/approvestill lazy-defines the approval page.ui/srcsuite: exit 0 (310 files). Focused: config lib (66), config-form-utils (13, fixtures updated to the new parsed-original signature), json5-runtime (new), custom-theme, config pages, settings, perf-script tests — all pass.node scripts/check-changed.mjs(delegated Blacksmith Testbox): green after rebase (one earlier failure was a then-brokenmaintypecheck, fixed upstream by fix(security): restore fs import dropped by bounded manifest-read change #110585/fix(security): bound skill-file audit read (restore removed fs dependency) #110589).src/config/redact-snapshot.tsrestoreRedactedValuesrestores sentinels on config.set/apply/patch.