fix(gateway): log last-completed websocket handshake phase#79654
fix(gateway): log last-completed websocket handshake phase#79654dmytrovk wants to merge 3 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: yes. by source inspection: current main reaches pre-connect close and handshake-timeout paths that log coarse handshake state without a last-completed phase. I did not run a live Gateway failure scenario. Real behavior proof Next step before merge Security Review detailsBest possible solution: Land the narrow Gateway diagnostics change after contributor-provided real Gateway proof shows the new phase in an actual failure log and maintainers verify the focused Gateway checks. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: current main reaches pre-connect close and handshake-timeout paths that log coarse handshake state without a last-completed phase. I did not run a live Gateway failure scenario. Is this the best way to solve the issue? Mostly yes: the PR keeps the diagnostic state inside the Gateway WebSocket connection path, uses an ordered phase list, and adds focused tests. Merge should still wait for real behavior proof from the contributor’s setup. What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against ab070054a3d1. |
Drop-in script that wires the production gateway ws-connection handler against a fresh HTTP + WebSocketServer, opens an idle WebSocket, and lets the preauth handshake timer fire so maintainers can independently reproduce the new "phase=ws_upgrade_started" diagnostic line. Refs: openclaw#79603 Co-authored-by: Cursor <[email protected]>
SummaryResolves #79603. Gateway WebSocket handshake failure logs do not currently If this PR fixes a plugin beta-release blocker, title it
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)External contributors must show after-fix evidence from a real OpenClaw setup. Unit tests, mocks, lint, typechecks, snapshots, and CI are supplemental only. Screenshots are encouraged even for CLI, console, text, or log changes; terminal screenshots and copied live output count. Be mindful of private information like IP addresses, API keys, phone numbers, non-public endpoints, or other private details when providing evidence.
Root Cause (if applicable)For bug fixes or regressions, explain why this happened, not just what changed.
Regression Test Plan (if applicable)
User-visible / Behavior ChangesUser-visible runtime log content gains a Diagram (if applicable)Security Impact (required)
Repro + VerificationEnvironment
Steps
ExpectedTwo warn lines appear: a ActualMatches expected. See terminal capture above. Evidence
Human Verification (required)
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers. Compatibility / Migration
Risks and Mitigations
|
|
Summary
Describe the problem and fix in 2–5 bullets:
If this PR fixes a plugin beta-release blocker, title it
fix(<plugin-id>): beta blocker - <summary>and link the matchingBeta blocker: <plugin-name> - <summary>issue labeledbeta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)
External contributors must show after-fix evidence from a real OpenClaw setup. Unit tests, mocks, lint, typechecks, snapshots, and CI are supplemental only. Screenshots are encouraged even for CLI, console, text, or log changes; terminal screenshots and copied live output count. Be mindful of private information like IP addresses, API keys, phone numbers, non-public endpoints, or other private details when providing evidence.
Root Cause (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.User-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Security Impact (required)
Yes/No)Yes/No)Yes/No)Yes/No)Yes/No)Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No)Yes/No)Yes/No)Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.