Control UI: reduce unnecessary gateway churn and prevent stale index caching#69454
Control UI: reduce unnecessary gateway churn and prevent stale index caching#69454cmaldonado80 wants to merge 6 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR introduces targeted performance fixes to reduce unnecessary gateway churn in the Control UI. The changes bound chat/session refresh calls, scope node polling to the Nodes tab, debounce Confidence Score: 5/5Safe to merge — all changes are opt-in debouncing / scoping improvements with no correctness regressions and solid test coverage. No P0 or P1 issues found. Every changed behavior (sessions debounce, tab-scoped polling, bounded session list, no-store cache header) is unit-tested and the logic is straightforward. No files require special attention. Reviews (1): Last reviewed commit: "Prevent control UI index caching" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62738540dd
ℹ️ 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".
| host.sessionsChangedReloadTimer = window.setTimeout(() => { | ||
| host.sessionsChangedReloadTimer = null; | ||
| void loadSessions(host as unknown as SessionsState); |
There was a problem hiding this comment.
Guard deferred sessions reload against tab switches
The timeout callback always calls loadSessions even if the user has left overview/sessions before the 750ms delay elapses. In practice, if a sessions.changed event arrives on Overview and the user quickly navigates to Chat, the scheduled callback still reloads sessions on Chat, which reintroduces the gateway churn this change is meant to avoid. Re-check shouldReloadSessionsForEvent(host) (or clear the pending timer on tab change) inside the timeout callback before issuing the request.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. What this changes: The PR bounds Control UI chat/session session-list refreshes, scopes node and device loading and polling to the Nodes tab, debounces session-change reloads away from Chat, serves Control UI index HTML with no-store caching, and adds focused tests. Required change before merge: The remaining blockers are narrow and mechanical on the PR branch: guard or cancel the deferred session reload after tab changes, add a changelog entry, resolve the dirty branch, and rerun focused UI/gateway validation. Security review: Security review cleared: The diff does not add dependencies, workflows, package-resolution changes, secret handling, or new code-execution paths; the index no-store change is defensive for the admin UI surface. Review findings:
Review detailsBest possible solution: Update or replace this PR with the same narrow Control UI churn/cache improvements after adding a tab recheck or timer cancellation for deferred session reloads, adding the required Unreleased changelog credit, rebasing cleanly on current main, and keeping the broader polling-interval proposal separate in #45930. Do we have a high-confidence way to reproduce the issue? Yes. A focused fake-timer test can schedule sessions.changed from Overview, switch host.tab to Chat before 750 ms, advance the timer, and assert loadSessions is not called; the PR head currently fails that behavior by calling loadSessions unconditionally inside the timeout. Is this the best way to solve the issue? No, not as submitted. The overall scoping/debouncing/no-store direction is narrow and maintainable, but the deferred callback needs to honor the current tab at execution time and the branch needs the required changelog/rebase cleanup before merge. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 4981ec7061ab. |
|
Maintainer overlap triage after opening #75986: This PR is complementary, not duplicate-closed or superseded by #75986. Scope split:
I would keep this PR deferred/needs-changes, not duplicate-closed. The existing review blockers still look like the right path: guard/cancel deferred session reloads after tab changes, add the changelog entry, rebase cleanly, then rerun the focused UI/gateway validation. |
Summary
This PR packages the local Control UI slowness fixes that materially reduced gateway churn during investigation, plus one small server-side hardening for stale bundle rollouts.
Included fixes
activeMinutes: 120limit: 200sessions.changedreloadssessions.changedwhile on Chatnode.list/device.pair.listpreloads outside the Nodes flowindex.htmlwithCache-Control: no-storeso browsers fetch fresh bundle references after rolloutsWhy
In local runtime evidence, the biggest proven slowness driver was Control UI itself:
sessions.listrefreshes from chat/session UI statenode.listpolling even when Nodes was not opensessions.changedpushThese changes are intentionally small and reversible. They reduce control-plane load without changing the overall product model.
Validation
Run on an isolated branch based on
origin/main:pnpm test -- --run src/gateway/control-ui.http.test.tspnpm --dir ui test -- src/ui/app-gateway.node.test.ts src/ui/app-gateway.sessions.node.test.ts src/ui/app-lifecycle.node.test.ts src/ui/app-lifecycle-connect.node.test.ts src/ui/app-settings.test.ts src/ui/app-render.helpers.node.test.ts src/ui/app-chat.test.tspnpm --dir ui buildObserved locally after rollout:
Notes
origin/main.ui/src/ui/views/chat.ts) was left out on purpose.