improve(ui): auto-save config edits, flatten config pages, single restart affordance#107458
Conversation
1e030d7 to
44a7d36
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e030d79c4
ℹ️ 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".
| patchForm: (path, value) => { | ||
| mutate(() => updateConfigFormValue(state, path, value)); | ||
| scheduleAutoSave(); | ||
| }, |
There was a problem hiding this comment.
Preserve owner save side effects for autosaves
When patchForm now schedules a config.set for every caller, it also covers non-settings surfaces such as Channels and Agents; those pages still keep their owner refresh/reprobe in their explicit Save handlers (saveChannelConfig calls channels.refresh(true), and saveAgentConfig refreshes the agent list). If a user edits channel or agent config and waits for the 800 ms debounce, the autosave clears configFormDirty, which disables those Save buttons before the page-specific handlers run, so the file is written but the channel/agent runtime state stays stale until a manual reload/refresh. Scope autosave to callers that have the new status/apply flow, or make autosave run the same owner post-save callbacks.
Useful? React with 👍 / 👎.
What Problem This Solves
The schema-driven config pages (AI & Agents, Communications, Automation, MCP, Infrastructure, General Advanced) carried a six-button toolbar — Open, Reload, Clear, Save, Apply, Update — plus a "No changes" status, an in-page search that duplicated the sidebar settings search, pill sub-tabs, and an outer panel wrapping everything in a card-in-card. Editing a setting meant understanding the difference between Save and Apply and remembering to press one of them.
Why This Change Was Made
Edits just apply. Form edits auto-save to
openclaw.json(800 ms debounce, one serialized flight with a single trailing save). Sinceconfig.applyrestarts the gateway, the one action that must stay explicit is a slim inline banner: "Saved to openclaw.json — restart the gateway to apply" with a single Restart & apply button. The Raw editor is the one file surface (Open / Discard / Save). The Update button (app self-update) leaves config entirely — the sidebar update card owns it.Flat and single-search. The outer
.config-layoutpanel is gone — sections render directly on the page column like every other settings page; sub-tabs are a segmented control on one row with the save status inline; the in-page search is removed (the sidebar settings search deep-links into these sections and is the single search surface).Gateway acks the persisted hash (additive).
config.set/config.applyresponses now include the post-write snapshothash(identical derivation toconfig.get). The client synthesizes the authoritative snapshot from the acked bytes, which is what makes autosave correct without inference: reloads after save are purely cosmetic, reverts made mid-flight stay dirty and save, explicit save/apply drain the autosave chain first, base-hash conflicts surface as a distinct "Settings changed elsewhere" status with a Reload action, drafts survive reconnects and SPA teardown, and the restart banner persists across page reloads keyed to the saved hash.App-updater interlock. Autosave introduces background writes, so the app updater now coordinates with them: when an update starts, config writes are suspended (pending autosaves cancelled, new submissions blocked) and
update.runis only issued after a barrier drains any write already in flight — a save,config.patch, or gateway-restarting apply can no longer overlap the install. An unsaved Raw draft also pins the Raw editor on screen (Form toggle disabled with an explanatory tooltip) until it is saved or discarded, instead of hiding it behind a stale form with an apply that would always refuse.Also fixes a pre-existing bug found during review: raw-mode saves previously serialized the stale parsed form instead of the raw text, silently dropping raw edits (and JSON5 comments/formatting on apply).
User Impact
Change a setting → it saves itself; one clearly-labeled restart affordance appears only when the gateway needs it. No more Save-vs-Apply guessing, no duplicate search, no card-in-card. Raw editing keeps explicit control. Quick Settings shares the same status/recovery UI.
Known limitation (documented in code, follow-up filed): the restart banner is tracked per saved-config hash in localStorage; concurrent tabs racing saves/applies can momentarily show/hide it incorrectly. Config writes themselves are always CAS-protected by the gateway. A gateway-reported
appliedConfigHashfollow-up will replace the heuristic.Evidence
node scripts/run-vitest.mjs).gpt-5.6-sol, xhigh) ran seventeen rounds; ~40 accepted findings fixed across the state machine: reconnect/teardown draft loss, raw fidelity, conflict recovery, cross-flight races, the updater interlock + drain barrier (includingconfig.patch), interrupted-write reconciliation after reconnect (with restart-marker recovery and revert restoration), hashless-ack recovery, autosave/manual-write serialization, and an explicit-op queue so two saves/applies/patches queued behind the same flight can't double-dispatch against one base hash. Two late findings were rejected with proof: the "stale-epoch ack overwrites a newer restart marker" scenario is unreachable (the protocol client rejects all pending requests on socket close —packages/gateway-client/src/protocol-client.tsflushRequests), and the "revert destroys JSON5 comments" claim is disproven by a regression test (form dirtiness is canonical-vs-canonical; clean drafts submitsnapshot.rawverbatim). The remaining cross-tab marker observations are the documented advisory-banner limitation above.