feat(control-ui): add gateway restart confirmation dialog and related…#63807
feat(control-ui): add gateway restart confirmation dialog and related…#63807BunsDev merged 4 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR adds a confirmation dialog before applying Dreaming mode changes that trigger a gateway restart, replacing the previous immediate-action toggle. The implementation wires three new reactive state fields (
Confidence Score: 4/5Safe to merge with one state-cleanup fix recommended before landing. One P1 defect identified (stale error after cancel) that's distinct from the two prior thread findings. The core confirmation gate is correctly implemented and test-covered. The P1 is a one-line fix with no architectural impact. ui/src/ui/app-render.ts — cancelDreamingRestart needs state.dreamingStatusError = null Prompt To Fix All With AIThis is a comment left during a code review.
Path: ui/src/ui/app-render.ts
Line: 390-396
Comment:
**Stale error shown on dreaming view after cancel**
`cancelDreamingRestart` clears `dreamingRestartConfirmOpen` and `dreamingPendingEnabled`, but does not clear `dreamingStatusError`. If the user clicked Confirm, got a patch failure (which sets `dreamingStatusError`), and then clicks Cancel to dismiss the dialog, the error string remains in state and is immediately rendered in the dreaming view's `dreams__controls-error` section — showing a stale failure message for an operation the user just abandoned.
```suggestion
const cancelDreamingRestart = () => {
if (state.dreamingRestartConfirmLoading) {
return;
}
state.dreamingRestartConfirmOpen = false;
state.dreamingPendingEnabled = null;
state.dreamingStatusError = null;
};
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: ui/src/ui/views/dreaming-restart-confirmation.ts
Line: 31-38
Comment:
**Cancel/Confirm button order differs from sibling dialogs**
`gateway-url-confirmation.ts` and `exec-approval.ts` both render the primary action first, secondary (Cancel) last. This dialog inverts that — Cancel appears first, Confirm Restart second — which is inconsistent with the existing pattern in the codebase.
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "feat(control-ui): add gateway restart co..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: e2f4655eb1
ℹ️ 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".
bbddbb1
force-pushed
the
feature/gateway-restart-ui-fix
branch
from
e2f4655 to
d53ca72
Compare
|
Hi @BunsDev, I’ve implemented a confirmation modal for the Dreaming mode toggle to prevent accidental gateway restarts. I've also added integration tests to cover the new flow. Ready for review! |
bbddbb1
force-pushed
the
feature/gateway-restart-ui-fix
branch
from
917dd58 to
d4cd733
Compare
|
Absolutely, this is an intelligent and thoughtful addition to ensure trust and transparency. Thanks for your contribution @bbddbb1 |
Thanks, appreciate it! Happy to iterate further if needed. |
|
Mind removing anything out of scope from this PR and opening the MiniMax addition, for example, etc in another PR, please? @bbddbb1 that'll get us to main much faster with this request (which is a valuable addition). |
Got it. Just to confirm — is the i18n part what you had in mind as out of scope, or are there other pieces you'd like split out as well? @BunsDev |
bbddbb1
force-pushed
the
feature/gateway-restart-ui-fix
branch
from
d4cd733 to
8c94b48
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c94b48e5f
ℹ️ 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".
bbddbb1
force-pushed
the
feature/gateway-restart-ui-fix
branch
from
dadabeb to
f27aa1a
Compare
|
Codex automated review: keeping this open. Keep open. Current main still applies the Dreaming toggle immediately through Best possible solution: Keep this PR open and finish the narrow Control UI change: review the latest scoped diff, settle the generated locale handling, keep the confirmation-flow browser coverage, and merge this PR or an equivalent main-branch implementation. If maintainers decide not to ship the UX change, close it as a product decision rather than as already implemented or obsolete. What I checked:
Remaining risk / open question:
Codex Review notes: model gpt-5.5, reasoning high; reviewed against bfdee5fa72cc. |
BunsDev
force-pushed
the
feature/gateway-restart-ui-fix
branch
from
73c8dec to
51bcf31
Compare
BunsDev
left a comment
BunsDev
left a comment
There was a problem hiding this comment.
Approved on code review for the scoped Control UI change.
What this fixes: #63804 / the Dreaming page toggle previously sent the restart-impacting config patch immediately. This PR now opens an explicit confirmation dialog first, keeps Cancel as a true no-op, shows Restarting... while the confirmed patch is in flight, and refreshes config/status after success.
Why this is the right shape: the behavior stays in the Control UI layer, reuses the existing Dreaming config patch controller, keeps restart semantics out of core, and includes generated locale sync from the English source copy. The prior review issues are addressed: cancel clears stale error state and the button order now matches sibling confirmation dialogs.
Local validation on head 51bcf318c16d27a5e60c7882dad5822c07f19b78:
pnpm ui:i18n:checkOPENCLAW_TEST_HEAVY_CHECK_LOCK_HELD=1 pnpm test ui/src/ui/navigation.browser.test.tspnpm exec oxfmt --check $(git diff --name-only origin/main...HEAD)OPENCLAW_TSGO_HEAVY_CHECK_LOCK_HELD=1 pnpm tsgo:coreOPENCLAW_TSGO_HEAVY_CHECK_LOCK_HELD=1 pnpm tsgo:core:testOPENCLAW_OXLINT_SKIP_LOCK=1 pnpm lint:corepnpm check:no-conflict-markerspnpm check:import-cycles
Exact-head CI is still red, but the failures I checked are outside this PR's touched files: docs formatting in docs/plugins/sdk-subpaths.md, tmpdir guard in extensions/migrate-hermes/test/provider-helpers.ts, an existing agent/plugin import-cycle report, and migrate-hermes model expectation failures. This is review-approved, not merge-ready until CI is cleaned up or the unrelated failures are separately handled.
BunsDev
force-pushed
the
feature/gateway-restart-ui-fix
branch
from
51bcf31 to
fcf4a46
Compare
|
Post-rebase update: the PR is now on I reran the focused local gates after rebasing on latest
Exact-head CI completed red, but the remaining failures are outside this PR's touched files:
No merge action taken. |
2 participants
Summary
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
Regression Test Plan (if applicable)
User-visible / Behavior Changes
Diagram (if applicable)
Security Impact (required)
Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Human Verification (required)
Review Conversations
Compatibility / Migration
Risks and Mitigations
… state management