fix(control-ui): keep channel status refreshes responsive#78803
fix(control-ui): keep channel status refreshes responsive#78803BunsDev wants to merge 3 commits into
Conversation
|
Codex review: found issues before merge. Summary 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 Next step before merge Security Review findings
Review detailsBest 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:
Overall correctness: patch is incorrect What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 945fcc10fdf1. |
|
Maintainer proof update for the responsiveness churn reconciliation now pushed in
Verification run on the canonical branch:
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. |
|
Maintainer review result: request changes before this draft is marked ready. Required:
Verified on
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. Re-review progress:
|
60dc080 to
0c4ead2
Compare
|
Recreated this draft from current
Verification on the recreated head
|
|
Closing this draft in favor of the signed replacement PR #80657. The patch content was recreated from current |
Summary
Verification
pnpm docs:listnode scripts/run-vitest.mjs run --config test/vitest/vitest.gateway.config.ts src/gateway/server-methods/channels.status.test.tspnpm --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.tspnpm 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.tsgit diff --checkpnpm check:changedgit diff --check origin/main...HEAD, focused gateway test, focused UI testsNotes
pnpm --dir ui test -- controllers/channels.test.ts app-settings.refresh-active-tab.node.test.ts views/config.browser.test.tsstill expands into unrelated browser suites and fails before running them cleanly becausesrc/ui/chat/chat-responsive.browser.test.tsandsrc/ui/views/sessions.browser.test.tsimportplaywright-corein a browser context whereprocessis not defined. In that run, the selected tests themselves passed (1237 passed).