fix(control-ui): preserve Stop after reconnect#76111
Conversation
|
Codex review: needs changes before merge. Summary Reproducibility: yes. The PR diff itself gives a high-confidence reproduction: invoke Next step before merge Security Review findings
Review detailsBest possible solution: Keep the PR direction, but repair the active-run lookup to use the request context registry and stored session keys, then land with focused gateway/UI tests and the existing changelog entry. Do we have a high-confidence way to reproduce the issue? Yes. The PR diff itself gives a high-confidence reproduction: invoke Is this the best way to solve the issue? No. The backend/UI model is the right narrow approach, but this implementation uses the wrong object for active abort state; a context-scoped lookup is the safer maintainable fix. 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 15cb06430e8e. |
There was a problem hiding this comment.
Pull request overview
This PR fixes Control UI’s lost “Stop” affordance after reconnect/reload by recovering abortability from gateway session state, and improves long-running dashboard connection stability by adding WebSocket protocol-level keepalive pings.
Changes:
- Add a server-side WebSocket
ping()keepalive loop (25s) for authenticated gateway WS clients. - Expose
hasActiveRunonsessions.listandsessions.changed, derived from the gateway’s active chat abort-controller registry. - Update Control UI abort logic to keep Stop available after reconnect and to queue/send session-scoped
chat.abort { sessionKey }whenrunIdis no longer known.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/ui/types.ts | Adds hasActiveRun?: boolean to GatewaySessionRow for UI consumption. |
| ui/src/ui/controllers/sessions.ts | Ensures hasActiveRun is requested/handled in session event rows. |
| ui/src/ui/app-render.ts | Uses hasAbortableSessionRun(state) instead of Boolean(chatRunId) to keep Stop visible after reconnect. |
| ui/src/ui/app-gateway.ts | Allows queued aborts without runId and sends session-scoped chat.abort after reconnect. |
| ui/src/ui/app-gateway.node.test.ts | Adds coverage for queued session-scoped abort sending after reconnect. |
| ui/src/ui/app-chat.ts | Introduces hasAbortableSessionRun() and queues aborts while disconnected even if runId was lost. |
| ui/src/ui/app-chat.test.ts | Adds test for disconnected session-scoped abort queuing after recovering active-run state. |
| src/gateway/session-utils.types.ts | Adds hasActiveRun?: boolean to gateway session row typing. |
| src/gateway/server/ws-connection.ts | Adds 25s protocol ping loop and clears it during connection teardown. |
| src/gateway/server/ws-connection.test.ts | Adds timer-based test verifying pings stop after socket close; restores real timers after tests. |
| src/gateway/server.sessions.list-changed.test.ts | Adds test asserting sessions.list marks sessions with active abortable runs. |
| src/gateway/server-methods/sessions.ts | Populates hasActiveRun in sessions.list responses and sessions.changed events. |
| CHANGELOG.md | Documents the user-visible fix for #70991. |
|
@copilot apply changes based on the comments in this thread |
Both review comments are now addressed:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…-connection Agent-Logs-Url: https://github.com/openclaw/openclaw/sessions/30df0b08-4389-40c2-91e9-6f9189914f7b Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com>
6f76ed3 to
1f4c872
Compare
Summary
hasActiveRunonsessions.listandsessions.changedfrom the Gateway's active chat abort controller registrychat.abortwhen the localrunIdwas lostCloses #70991.
Related: #56668, #74783, #70230.
Root cause
The backend already supports
chat.abortwith{ sessionKey }and norunId, but the Control UI only rendered Stop from localchatRunId. A reconnect/reload clears that local value, so the user loses the Stop affordance even when the Gateway still has an abortable active run. The gateway WebSocket path also still lacked protocol-level ping frames, leaving long tool-call windows dependent on JSON tick delivery.User-visible behavior
sessions.list/sessions.changedreports an active run for the current session.chat.abort { sessionKey }after reconnect.Security review
The ping loop skips unauthenticated pre-handshake sockets and is cleared during the existing connection teardown path.
Validation
pnpm test ui/src/ui/app-chat.test.ts ui/src/ui/app-gateway.node.test.ts src/gateway/server.sessions.list-changed.test.ts src/gateway/server/ws-connection.test.tspnpm test src/gateway/server.sessions.list-changed.test.tspnpm test src/gateway/server/ws-connection.test.tspnpm exec oxfmt --check --threads=1 CHANGELOG.md src/gateway/server-methods/sessions.ts src/gateway/session-utils.types.ts src/gateway/server/ws-connection.ts src/gateway/server/ws-connection.test.ts src/gateway/server.sessions.list-changed.test.ts ui/src/ui/app-chat.ts ui/src/ui/app-chat.test.ts ui/src/ui/app-gateway.ts ui/src/ui/app-gateway.node.test.ts ui/src/ui/app-render.ts ui/src/ui/controllers/sessions.ts ui/src/ui/types.tsOPENCLAW_LOCAL_CHECK=1 OPENCLAW_LOCAL_CHECK_MODE=throttled pnpm check:changedNotes
I attempted the repo-preferred Blacksmith Testbox changed gate, but the Testbox sync deleted tracked sparse/large files and removed
node_modules, causing unrelated all-lane failures. I stopped that Testbox and used the local changed gate with dependencies installed.