Skip to content

fix(ui): discard stale config state on explicit reload#40443

Closed
MikeChongCan wants to merge 6 commits intoopenclaw:mainfrom
MikeChongCan:codex/fix-control-ui-config-reload
Closed

fix(ui): discard stale config state on explicit reload#40443
MikeChongCan wants to merge 6 commits intoopenclaw:mainfrom
MikeChongCan:codex/fix-control-ui-config-reload

Conversation

@MikeChongCan
Copy link
Copy Markdown

Summary

  • Problem: Control UI config reload paths could leave stale local config state in place, so the Agents model dropdown could diverge from the actual gateway config.
  • Why it matters: clicking Reload Config should replace stale local edits with the current gateway snapshot, while passive refreshes should still preserve unsaved work.
  • What changed: split passive refresh vs explicit reload semantics in the config controller, wired explicit reload buttons to discard pending edits, preserved unsaved raw-mode edits during passive refresh, and added regression coverage plus a browser-level agent view test.
  • What did NOT change (scope boundary): no runtime model resolution behavior, no gateway config schema changes, and no speculative refactor of the agent dropdown rendering.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • Reload Config in Control UI now discards pending local config edits and reloads the live gateway snapshot.
  • Passive refreshes continue to preserve unsaved edits.
  • Raw config edits are no longer overwritten by passive refreshes.

Security Impact (required)

  • New permissions/capabilities? (No)
  • Secrets/tokens handling changed? (No)
  • New/changed network calls? (No)
  • Command/tool execution surface changed? (No)
  • Data access scope changed? (No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: local checkout
  • Model/provider: N/A
  • Integration/channel (if any): Control UI / Agents / Config / Channels views
  • Relevant config (redacted): agents.defaults.model changed between anthropic/claude-opus-4-6 and ollama/qwen3-coder:30b-64k

Steps

  1. Open Agents -> Overview and observe the current primary model.
  2. Change the saved config so the gateway snapshot points at a different model.
  3. Click Reload Config and verify the overview and dropdown both update.
  4. Make unsaved edits in both form mode and raw mode.
  5. Verify passive refreshes preserve those edits, but explicit reload discards them.

Expected

  • Explicit reload replaces stale local config state with the current gateway snapshot.
  • Passive refresh does not wipe unsaved edits.

Actual

  • Before this change, stale local config state could survive reload flows and raw-mode passive refreshes could overwrite pending edits.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: targeted UI tests for config reload semantics and a browser-level rerender test for the agents model dropdown; UI and root builds succeeded.
  • Edge cases checked: form-mode dirty edits, raw-mode dirty edits, explicit reload, passive refresh, Nostr profile async state handling.
  • What you did not verify: manual click-through in a live browser session against a running gateway.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Migration needed? (No)
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: revert commit 2a467c2.
  • Files/config to restore: ui/src/ui/controllers/config.ts, ui/src/ui/app-render.ts, ui/src/ui/app-channels.ts.
  • Known bad symptoms reviewers should watch for: explicit reload unexpectedly preserving stale edits, or passive refresh unexpectedly overwriting unsaved edits.

Risks and Mitigations

  • Risk: reload semantics differ between passive refresh and explicit reload, and a missed call site could preserve/discard edits unexpectedly.
    • Mitigation: explicit reload call sites were updated for Agents, Config, and Channels, and regression tests cover both form and raw modes.

AI-assisted: yes. Codex was used to implement the change, run checks, and apply follow-up fixes from Gemini/Claude review.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 9, 2026

Greptile Summary

This PR correctly splits passive-refresh from explicit-reload semantics in the config controller so that clicking "Reload Config" discards stale local edits while background refreshes preserve unsaved work. The core implementation is sound:

  • applyConfigSnapshot now accepts a discardPendingEdits option to control whether pending form and raw edits are preserved or discarded
  • All explicit "Reload Config" button callbacks in Agents, Config, and Channels views correctly pass { discardPendingEdits: true }
  • saveConfig and applyConfig call loadConfig with discardPendingEdits: true after success, correctly clearing the dirty flag
  • The removed redundant loadConfig call from handleChannelConfigSave is safe since saveConfig now handles it internally on success
  • Test coverage for all four passive/explicit × form/raw combinations is present but has a minor gap: the discard-edits snapshot test is missing an assertion for configRawOriginal

Confidence Score: 4/5

  • Safe to merge — the core reload/preserve semantics are correctly implemented and well-tested, with only a minor test assertion gap that does not affect correctness.
  • The logic across all four passive/explicit × form/raw paths is sound, call sites are consistently updated, and no behavioral regressions are present. The score is 4 rather than 5 due to the incomplete configRawOriginal assertion in the discard-edits test, which should be added for complete regression coverage.
  • ui/src/ui/controllers/config.test.ts (missing configRawOriginal assertion)

Last reviewed commit: 2a467c2

Comment thread ui/src/ui/controllers/config.test.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a467c20ed

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/src/ui/app-channels.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b982a39d7a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/src/ui/app-render.ts
@MikeChongCan MikeChongCan force-pushed the codex/fix-control-ui-config-reload branch from b982a39 to 2d2db75 Compare March 9, 2026 13:57
@openclaw-barnacle openclaw-barnacle Bot removed the cli CLI command changes label Mar 9, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d2db75ac7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/src/ui/controllers/config.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1cd3a73ba

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/src/ui/app-channels.ts
@MikeChongCan MikeChongCan force-pushed the codex/fix-control-ui-config-reload branch from b1cd3a7 to 60efccd Compare March 10, 2026 14:07
@MikeChongCan MikeChongCan force-pushed the codex/fix-control-ui-config-reload branch from 60efccd to 07b9555 Compare March 11, 2026 15:17
@MikeChongCan
Copy link
Copy Markdown
Author

Rebased onto the latest main to pick up the upstream CI fixes that landed after the previous push, including the pi-ai OAuth import alignment and the CI gate restoration. No new review threads were open; this update is just the rebase so the PR can rerun against the current base cleanly.

@openclaw-barnacle
Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 26, 2026

Codex automated review: keeping this open.

Keep open. Current main still lacks this PR's explicit reload versus passive refresh split: loadConfig has no discard option, applyConfigSnapshot preserves dirty form state, and the Agents, Config, and Channels reload buttons still call the same passive load path. The linked bug #40352 remains open, and the competing #40441 was closed unmerged rather than superseding this work.

Best possible solution:

Keep this PR open for maintainer review or land an equivalent fix. The best path is to rebase it onto current main, preserve the explicit reload versus passive refresh split in ui/src/ui/controllers/config.ts, wire Agents/Config/Channels user-clicked reload actions to discard pending edits, keep passive refreshes and hash-conflict recovery non-destructive, and retain focused regression coverage for form mode, raw mode, hash conflicts, and the Agents model dropdown rerender.

What I checked:

  • Current main checked: The checkout is on current main SHA 91e835e, with no local modifications reported before inspection. (91e835ebe0ab)
  • No explicit reload option on main: loadConfig accepts only state, fetches config.get, and calls applyConfigSnapshot(state, res) with no discardPendingEdits or equivalent option. (ui/src/ui/controllers/config.ts:40, 91e835ebe0ab)
  • Dirty form state still survives reload on main: applyConfigSnapshot only replaces configForm, configFormOriginal, and configRawOriginal when !state.configFormDirty, so a dirty local form can keep stale config state after a reload through the current path. (ui/src/ui/controllers/config.ts:80, 91e835ebe0ab)
  • Explicit reload call sites still use passive load semantics: The Config tab onReload, Agents overview onConfigReload, and Channels reload handler all call loadConfig(state) / loadConfig(host as ConfigState) without a discard mode. (ui/src/ui/app-render.ts:858, 91e835ebe0ab)
  • Agents dropdown depends on config form state: The Agents overview resolves model display and select values from resolveAgentConfig(configForm, agent.id) and effectivePrimary, so stale configForm can still keep the Model Selection dropdown divergent from the gateway snapshot. (ui/src/ui/views/agents-panels-overview.ts:52, 91e835ebe0ab)
  • Main tests do not cover discard reload semantics: Current tests cover preserving dirty edits during snapshot application, but there is no loadConfig option or explicit passive-versus-discard reload coverage on main. (ui/src/ui/controllers/config.test.ts:53, 91e835ebe0ab)

Remaining risk / open question:

  • The branch is based on an older main and needs a rebase plus current UI changed-lane validation before merge.
  • The PR includes related Nostr profile async-state fixes in addition to the config reload fix; maintainers may want to keep that bundled or split it during review.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against 91e835ebe0ab.

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Apr 27, 2026
@vincentkoc
Copy link
Copy Markdown
Member

ProjectClownfish could not safely update this branch, so it opened a narrow replacement PR instead.

Replacement PR: #72624
Source PR: #40443
Contributor credit is preserved in the replacement PR body and changelog plan.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Control UI Model Selection dropdown shows stale model after config reload

3 participants