Skip to content

fix(control-ui): keep channel status refreshes responsive#78803

Closed
BunsDev wants to merge 3 commits into
mainfrom
meow/control-ui-responsiveness-followup
Closed

fix(control-ui): keep channel status refreshes responsive#78803
BunsDev wants to merge 3 commits into
mainfrom
meow/control-ui-responsiveness-followup

Conversation

@BunsDev

@BunsDev BunsDev commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Pre-scope scoped Control UI config schemas before analysis so off-tab sections are not normalized during Communications/Appearance-style tab paints.
  • Keep Channels tab navigation on the fast runtime/cached status path, while manual Probe remains the live network check path.
  • Preserve channel account rows when probe/summary status hooks fail or exceed the status budget, and keep stale slow probe completions from replacing newer non-probe snapshots.

Verification

  • pnpm docs:list
  • node scripts/run-vitest.mjs run --config test/vitest/vitest.gateway.config.ts src/gateway/server-methods/channels.status.test.ts
  • pnpm --dir ui test src/ui/controllers/channels.test.ts src/ui/app-settings.refresh-active-tab.node.test.ts src/ui/views/config.browser.test.ts
  • pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/gateway/server-methods/channels.ts src/gateway/server-methods/channels.status.test.ts ui/src/ui/controllers/channels.ts ui/src/ui/controllers/channels.types.ts ui/src/ui/controllers/channels.test.ts ui/src/ui/app-settings.ts ui/src/ui/app-settings.refresh-active-tab.node.test.ts ui/src/ui/views/config.ts ui/src/ui/views/config.browser.test.ts
  • git diff --check
  • pnpm check:changed
  • Post-rebase sanity: git diff --check origin/main...HEAD, focused gateway test, focused UI tests

Notes

  • The exact requested UI command pnpm --dir ui test -- controllers/channels.test.ts app-settings.refresh-active-tab.node.test.ts views/config.browser.test.ts still expands into unrelated browser suites and fails before running them cleanly because src/ui/chat/chat-responsive.browser.test.ts and src/ui/views/sessions.browser.test.ts import playwright-core in a browser context where process is not defined. In that run, the selected tests themselves passed (1237 passed).

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime size: M maintainer Maintainer-authored PR labels May 7, 2026
@clawsweeper

clawsweeper Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge.

Summary
The branch changes Control UI and gateway channel status refreshes, scoped config-schema analysis, session reload debouncing/bounds, Nodes polling, focused tests, and the changelog.

Reproducibility: yes. The original responsiveness problem and both review findings are source-reproducible from current main and the PR head: slow channel refreshes are serialized on main, while the PR removes the current background probe and can filter out the selected session row.

Real behavior proof
Not applicable: The external contributor proof gate does not apply because this is a MEMBER PR with a protected maintainer label; the posted command output remains useful review context.

Next step before merge
Draft protected maintainer PR with blocking review findings needs author or maintainer changes rather than cleanup closure or an autonomous repair lane.

Security
Cleared: No concrete security or supply-chain concern found; the diff is limited to TypeScript Control UI/gateway logic, focused tests, and changelog text.

Review findings

  • [P2] Keep the Channels tab background probe — ui/src/ui/app-settings.ts:856
  • [P2] Preserve the active session when bounding session refreshes — ui/src/ui/app-render.helpers.ts:681-683
Review details

Best possible solution:

Keep the responsiveness hardening, but update the branch so Channels tab entry preserves current-main cached status plus background live probe behavior and bounded session refreshes explicitly preserve or hydrate the selected session row.

Do we have a high-confidence way to reproduce the issue?

Yes. The original responsiveness problem and both review findings are source-reproducible from current main and the PR head: slow channel refreshes are serialized on main, while the PR removes the current background probe and can filter out the selected session row.

Is this the best way to solve the issue?

No. The timeout/stale-guard direction is useful, but the current patch should not regress live Channels status refresh or replace session options with a bounded list that can omit the active session.

Full review comments:

  • [P2] Keep the Channels tab background probe — ui/src/ui/app-settings.ts:856
    Current main renders the cached/runtime Channels state first and then schedules loadChannels(app, true) so provider/account status is refreshed live without blocking the tab. This patch deletes that background probe, so opening Channels can leave stale status until the user manually probes, which regresses the current main behavior the rebase review asked to preserve.
    Confidence: 0.9
  • [P2] Preserve the active session when bounding session refreshes — ui/src/ui/app-render.helpers.ts:681-683
    The new bounded loadSessions call replaces sessionsResult with only recent/limited rows. If the active session is an older non-main session, resolveSessionOptionGroups does not re-add it, so switching or refreshing chat can drop the selected session from the selector/model context; keep or hydrate the selected row when applying these bounds.
    Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.87

What I checked:

  • Protected PR state: Provided GitHub context reports this PR is open, draft, authored by a MEMBER, and labeled maintainer, so it is not eligible for cleanup close. (0c4ead2373b1)
  • Current main keeps a background channel probe: Current main loads cached/runtime channel status and config, then schedules loadChannels(app, true) in the background for live probe refresh on Channels tab entry. (ui/src/ui/app-settings.ts:842, 945fcc10fdf1)
  • PR head removes that probe: The PR diff deletes the post-render loadChannels(app, true) call from loadChannelsTab, leaving Channels tab entry without the current-main live probe path. (ui/src/ui/app-settings.ts:856, 0c4ead2373b1)
  • Maintainer review context: A maintainer review comment required the branch to preserve current-main Channels tab behavior after rebase; the recreated head still removes the background probe. (ui/src/ui/app-settings.ts:856, 0c4ead2373b1)
  • Bounded session refresh source: The PR changes chat/session option refreshes to request only CHAT_SESSIONS_ACTIVE_MINUTES and CHAT_SESSIONS_REFRESH_LIMIT, replacing the prior unbounded session option refresh. (ui/src/ui/app-render.helpers.ts:681, 0c4ead2373b1)
  • Session list contract in current code: loadSessionsOnce forwards positive activeMinutes and limit to sessions.list and replaces state.sessionsResult with the returned rows. (ui/src/ui/controllers/sessions.ts:442, 945fcc10fdf1)

Likely related people:

  • BunsDev: Related merged Control UI responsiveness PRs in the provided context and local commits cover the same app-settings, channel-status, chat, and session refresh areas; this PR continues that line of work. (role: recent area contributor; confidence: high; commits: 64fcc8a1aadb, 5ae385b2f0f1, 0c4ead2373b1; files: ui/src/ui/app-settings.ts, ui/src/ui/controllers/channels.ts, ui/src/ui/app-chat.ts)
  • pashpashpash: Local current-main blame for the Channels tab and channel/session controller lines inspected points to commit 2846d9e, so this is a weak but concrete recent routing signal for the checked-out snapshot. (role: recent current-main committer; confidence: low; commits: 2846d9ea56c6; files: ui/src/ui/app-settings.ts, ui/src/ui/controllers/channels.ts, ui/src/ui/controllers/sessions.ts)

Remaining risk / open question:

  • The PR body intentionally describes manual Probe as the live network path, but that conflicts with current main and the maintainer review request to preserve the background probe.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 945fcc10fdf1.

@BunsDev

BunsDev commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer proof update for the responsiveness churn reconciliation now pushed in 60dc080613:

  • Slow channels.status / Channels tab: fix(control-ui): keep channel status refreshes responsive #78803 keeps tab entry on cached/runtime status, keeps manual Probe as the live network path, preserves stale channel rows on slow/failed probes, and has regression coverage for stale slow probes not overwriting newer snapshots.
  • Full config-schema analysis on scoped pages: scoped Communications / AI & Agents / Automation-style renders now pre-scope schema work before expensive analysis while preserving unsupported-path and unsafe raw-edit warnings.
  • Background Control UI churn: the follow-up commit scopes Nodes polling to the active Nodes tab, removes eager node/device preloads from non-Nodes startup, debounces unapplied sessions.changed full reconciliation with tab/disconnect cleanup, and bounds chat-side session refreshes.

Verification run on the canonical branch:

  • pnpm test ui/src/ui/app-polling.node.test.ts ui/src/ui/app-gateway.sessions.node.test.ts ui/src/ui/app-settings.refresh-active-tab.node.test.ts ui/src/ui/app-lifecycle.node.test.ts ui/src/ui/app-lifecycle-connect.node.test.ts ui/src/ui/app-gateway-chat-load.node.test.ts ui/src/ui/app-gateway.node.test.ts ui/src/ui/app-chat.test.ts ui/src/ui/app-render.helpers.node.test.ts
  • pnpm test ui/src/ui/app-channels.test.ts ui/src/ui/controllers/channels.test.ts ui/src/ui/app-settings.refresh-active-tab.node.test.ts ui/src/ui/app-gateway.sessions.node.test.ts ui/src/ui/app-lifecycle-connect.node.test.ts ui/src/ui/controllers/sessions.test.ts ui/src/ui/views/config.browser.test.ts src/gateway/server-methods/channels.status.test.ts src/gateway/control-ui.http.test.ts ui/src/ui/app-polling.node.test.ts
  • pnpm exec oxfmt --check --threads=1 ...changed TS files...
  • git diff --check
  • pnpm changed:lanes --json -> core, coreTests, docs
  • pnpm check:changed

This should be the canonical branch for #45698/#77060 follow-up review. I am not closing those issues from this PR comment; they should wait for the merged patch plus any remaining live/provider proof the maintainer wants.

@BunsDev

BunsDev commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer review result: request changes before this draft is marked ready.

Required:

  • Rebase onto current origin/main. This head is 2 commits ahead / 4711 behind after fetch (merge-base a8d8d49, head 60dc080). The stale base reverts current main Channels tab behavior in ui/src/ui/app-settings.ts:853-856: current main loads config schema off the critical path and schedules loadChannels(app, true) in the background; this branch waits on schema/config and never starts the live probe. Please preserve the current main shape when rebasing.
  • Restore a green changed gate. OPENCLAW_LOCAL_CHECK=1 pnpm check:changed currently fails in lint:core with 139 no-unused-vars errors on the submitted head. These look like stale-branch drift rather than the 26-file PR delta, but the branch cannot be merged while the gate is red.

Verified on 60dc080:

  • pnpm docs:list
  • node scripts/run-vitest.mjs run --config test/vitest/vitest.gateway.config.ts src/gateway/server-methods/channels.status.test.ts => 6 passed
  • pnpm --dir ui test src/ui/controllers/channels.test.ts src/ui/app-settings.refresh-active-tab.node.test.ts src/ui/views/config.browser.test.ts src/ui/app-chat.test.ts src/ui/app-gateway-chat-load.node.test.ts src/ui/app-gateway.node.test.ts src/ui/app-gateway.sessions.node.test.ts src/ui/app-lifecycle-connect.node.test.ts src/ui/app-lifecycle.node.test.ts src/ui/app-polling.node.test.ts src/ui/app-render.helpers.node.test.ts => 204 passed
  • pnpm exec oxfmt --check --threads=1 $(git diff --name-only $(git merge-base HEAD origin/main)...HEAD) => clean
  • git diff --check $(git merge-base HEAD origin/main)...HEAD => clean

Duplicate check: not a clean duplicate. #79126 and #75986 are related/canonical responsiveness work, and #45698 remains related, but this PR still has distinct gateway/channel-row behavior if rebased. prtags was unavailable with backend 502s, so no duplicate tags were written.

Re-review progress:

@BunsDev
BunsDev force-pushed the meow/control-ui-responsiveness-followup branch from 60dc080 to 0c4ead2 Compare May 11, 2026 12:10
@BunsDev

BunsDev commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Recreated this draft from current origin/main (8f1e6ab13c) and force-updated the branch with verified commits only:

  • d5d8d07c87 fix(control-ui): keep channel status refreshes responsive
  • b68693c7e2 fix(control-ui): reduce responsiveness churn
  • 0c4ead2373 fix(control-ui): reconcile channel tab refresh replay

Verification on the recreated head 0c4ead2373:

  • pnpm test ui/src/ui/app-polling.node.test.ts ui/src/ui/app-gateway.sessions.node.test.ts ui/src/ui/app-settings.refresh-active-tab.node.test.ts ui/src/ui/app-lifecycle.node.test.ts ui/src/ui/app-lifecycle-connect.node.test.ts ui/src/ui/app-gateway-chat-load.node.test.ts ui/src/ui/app-gateway.node.test.ts ui/src/ui/app-chat.test.ts ui/src/ui/app-render.helpers.node.test.ts
  • pnpm test ui/src/ui/app-channels.test.ts ui/src/ui/controllers/channels.test.ts ui/src/ui/app-settings.refresh-active-tab.node.test.ts ui/src/ui/app-gateway.sessions.node.test.ts ui/src/ui/app-lifecycle-connect.node.test.ts ui/src/ui/controllers/sessions.test.ts ui/src/ui/views/config.browser.test.ts src/gateway/server-methods/channels.status.test.ts src/gateway/control-ui.http.test.ts ui/src/ui/app-polling.node.test.ts
  • pnpm exec oxfmt --check --threads=1 <changed TS files>
  • git diff --check origin/main...HEAD
  • pnpm changed:lanes --json reported core, coreTests, docs
  • pnpm check:changed

pnpm check:changed initially failed because local @earendil-works/*/pi-tui workspace dependencies were missing; I ran pnpm install, restored lockfile churn, and the retry passed.

@BunsDev

BunsDev commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing this draft in favor of the signed replacement PR #80657.

The patch content was recreated from current origin/main, pushed on meow/control-ui-responsiveness-signed, and GitHub reports all three replacement commits as verified (verified=true, reason=valid). The prior draft had two unsigned replay commits, so keeping the signed PR as canonical is cleaner for review and merge history.

@BunsDev BunsDev closed this May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui gateway Gateway runtime maintainer Maintainer-authored PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant