Skip to content

fix(gateway): log last-completed websocket handshake phase#79654

Closed
dmytrovk wants to merge 3 commits into
openclaw:mainfrom
dmytrovk:fix/79603-gateway-ws-handshake-phase-logging
Closed

fix(gateway): log last-completed websocket handshake phase#79654
dmytrovk wants to merge 3 commits into
openclaw:mainfrom
dmytrovk:fix/79603-gateway-ws-handshake-phase-logging

Conversation

@dmytrovk

@dmytrovk dmytrovk commented May 9, 2026

Copy link
Copy Markdown

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 matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

  • Problem:
  • Why it matters:
  • What changed:
  • What did NOT change (scope boundary):

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Closes #
  • Related #
  • This PR fixes a bug or regression

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.

  • Behavior or issue addressed:
  • Real environment tested:
  • Exact steps or command run after this patch:
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):
  • Observed result after fix:
  • What was not tested:
  • Before evidence (optional but encouraged):

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, write Unknown.

  • Root cause:
  • Missing detection / guardrail:
  • Contributing context (if known):

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file:
  • Scenario the test should lock in:
  • Why this is the smallest reliable guardrail:
  • Existing test that already covers this (if any):
  • If no new test is added, why not:

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.

Before:
[user action] -> [old state]

After:
[user action] -> [new state] -> [result]

Security Impact (required)

  • New permissions/capabilities? (Yes/No)
  • Secrets/tokens handling changed? (Yes/No)
  • New/changed network calls? (Yes/No)
  • Command/tool execution surface changed? (Yes/No)
  • Data access scope changed? (Yes/No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS:
  • Runtime/container:
  • Model/provider:
  • Integration/channel (if any):
  • Relevant config (redacted):

Steps

Expected

Actual

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios:
  • Edge cases checked:
  • What you did not verify:

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

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

  • Backward compatible? (Yes/No)
  • Config/env changes? (Yes/No)
  • Migration needed? (Yes/No)
  • If yes, exact upgrade steps:

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

  • Risk:
    • Mitigation:

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime triage: blank-template Candidate: PR template appears mostly untouched. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. triage: refactor-only Candidate: refactor/cleanup-only PR without maintainer context. size: S labels May 9, 2026
@clawsweeper

clawsweeper Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
The PR adds Gateway WebSocket handshake phase tracking and failure logging, focused tests, a changelog entry, and a dev repro script for the linked Gateway diagnostics issue.

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
Needs real behavior proof before merge: The PR body/comments do not include after-fix terminal output, redacted logs, a screenshot/recording, or a linked artifact from a real Gateway run; the contributor should add proof to the PR body and re-review should trigger automatically, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Contributor action is needed for real Gateway behavior proof; a ClawSweeper repair job cannot provide the external contributor’s required live-environment evidence.

Security
Cleared: The diff adds diagnostic logging, tests, a changelog entry, and a local dev repro script; I found no new dependency, permission, secret-handling, network-surface, or code-execution concern beyond the existing local script context.

Review details

Best 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:

  • Current main lacks phase detail in close logs: Current main builds pre-connect close metadata with cause, coarse handshake state, duration, and last-frame fields, but no last-completed handshake phase. (src/gateway/server/ws-connection.ts:358, ab070054a3d1)
  • Current main timeout log is terminal-only: The current handshake-timeout path records a timeout cause and logs peer/remote data, but it does not say which handshake phase completed last. (src/gateway/server/ws-connection.ts:426, ab070054a3d1)
  • PR adds bounded phase tracking: The PR diff adds an ordered phase tracker, includes phase metadata in incomplete-handshake close contexts, and logs phase on pre-connect close and timeout failures. (src/gateway/server/ws-connection.ts:272, 4ba3383edbee)
  • PR threads phases through connect handling: The PR advances phases at auth-token receipt, auth validation, session attachment, subscription registration, and ready. (src/gateway/server/ws-connection/message-handler.ts:434, 4ba3383edbee)
  • Regression coverage and repro script are present: The PR adds unit coverage for phase tracking/logging behavior and a standalone dev repro script, but the script output is not posted as contributor proof. (src/gateway/server/ws-connection.test.ts:248, 4ba3383edbee)
  • Real behavior proof remains absent: The PR body is still the blank template, and the only PR comment is the earlier ClawSweeper review noting missing real Gateway proof; there are no review comments or linked runtime artifacts. (4ba3383edbee)

Likely related people:

  • @joshavant: A recent merged Gateway change touched the same WebSocket message-handler auth/connect path now being extended with phase advancement. (role: recent adjacent contributor; confidence: medium; commits: 3391c5b050dc; files: src/gateway/server/ws-connection/message-handler.ts, src/gateway/server/ws-connection/connect-policy.ts)
  • @steipete: Recent Gateway message-handler and connection-surface history includes nearby health/check and refactor work, and current local blame attributes the checked-out connection surface to Peter Steinberger. (role: recent area contributor; confidence: medium; commits: 388af50ba156, 9ac4aef7c2d0; files: src/gateway/server/ws-connection/message-handler.ts, src/gateway/server/ws-connection.ts)
  • @samzong: Recent current-main history changed the WebSocket connection handler for node reconnect state, adjacent to connection lifecycle and close-handling diagnostics. (role: recent connection-surface contributor; confidence: low; commits: 1819e41d2683; files: src/gateway/server/ws-connection.ts)

Remaining risk / open question:

  • No redacted live Gateway log, terminal output, screenshot, recording, or linked artifact proves the new phase field appears in an actual after-fix WebSocket failure path.

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

dmytrovk and others added 2 commits May 10, 2026 15:15
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]>
@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: M and removed size: S labels May 10, 2026
@dmytrovk

Copy link
Copy Markdown
Author

Summary

Resolves #79603. Gateway WebSocket handshake failure logs do not currently
report which lifecycle phase a connection reached before it died, so on-call
operators reading logs on Windows (or any deployment) cannot tell whether
a connection failed during the upgrade, before the auth token arrived,
during auth validation, or after the session was attached. This PR adds
an ordered phase enum, monotonic phase advancement, and emits the
phase=<last-completed> field in the timeout and pre-connect close logs.
After-fix proof from a real Node 22 gateway run is included below.

If this PR fixes a plugin beta-release blocker, title it fix(<plugin-id>): beta blocker - <summary> and link the matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

  • Problem: Gateway WS handshake-timeout and pre-connect close logs only show coarse outcome state, not the last lifecycle phase the connection reached, making remote diagnosis difficult.
  • Why it matters: Operators on Windows deployments (and elsewhere) cannot localize handshake regressions without this signal.
  • What changed: Added WS_HANDSHAKE_PHASES ordered enum, monotonic advanceHandshakePhase, and emit phase=… on failure-only log paths. Added regression coverage and a repro script.
  • What did NOT change (scope boundary): Successful (ready) closes are unchanged — phase is intentionally omitted there. No public API, no auth, no network/permissions surface change.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

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.

  • Behavior or issue addressed: On a real running Gateway, the WebSocket preauth handshake timeout and the "closed before connect" path now include the last-completed handshake phase (phase=ws_upgrade_started) so an operator reading runtime logs can localize where a failed connection died in its lifecycle.
  • Real environment tested: Local Linux box, Node 22.16.0, openclaw branch fix/79603-gateway-ws-handshake-phase-logging, fresh in-process Gateway booted by scripts/dev/repro-79603-handshake-phase.ts. The script wires the production attachGatewayWsConnectionHandler (no mocks, no stubs of the connection handler) against a real node:http + ws WebSocketServer, opens a real WebSocket, and idles past the preauth handshake timeout (250ms) so the production timer fires.
  • Exact steps or command run after this patch: Checked out fix/79603-gateway-ws-handshake-phase-logging, ran pnpm install, then ran the repro script with pnpm exec tsx scripts/dev/repro-79603-handshake-phase.ts on Node 22 and copied the live console output. The script opens a real ws://127.0.0.1:<port> connection, never sends connect, and waits for the server-side preauth timer.
  • Evidence after fix: terminal capture / console output from a live node run on the patched branch (IPs are loopback only, conn UUIDs and ports are fresh per run):
[repro] gateway listening on ws://127.0.0.1:40465
[repro] preauth handshake timeout = 250ms
[repro] received: {"type":"event","event":"connect.challenge","payload":{"nonce":"f8e2abbd-88a5-41d7-a7dc-33421494e05a","ts":1778419892012
[repro] ws opened, idling so server's preauth timer fires...
[warn] gateway/ws: handshake timeout conn=ee9c03d4-b1b2-4d9d-940a-11df2624ade7 peer=127.0.0.1:40454->127.0.0.1:40465 remote=127.0.0.1 phase=ws_upgrade_started
[warn] gateway/ws: closed before connect conn=ee9c03d4-b1b2-4d9d-940a-11df2624ade7 peer=127.0.0.1:40454->127.0.0.1:40465 remote=127.0.0.1 fwd=n/a origin=n/a host=127.0.0.1:40465 ua=n/a code=1000 reason=n/a phase=ws_upgrade_started {"cause":"handshake-timeout","handshake":"failed","phase":"ws_upgrade_started","durationMs":255,"host":"127.0.0.1:40465","remoteAddr":"127.0.0.1","remotePort":40454,"localAddr":"127.0.0.1","localPort":40465,"endpoint":"127.0.0.1:40454->127.0.0.1:40465","handshakeMs":252}

[repro] OK: phase log emitted (2 matching line(s))
  • Observed result after fix: On the live Node 22 gateway, both the human-readable gateway/ws: handshake timeout … warn line and the structured gateway/ws: closed before connect … warn line now carry phase=ws_upgrade_started, and the structured close metadata also surfaces "phase":"ws_upgrade_started". Connect duration was ~252ms, just past the 250ms preauth timer, exactly as designed. Before the patch, the same scenario produced only cause=handshake-timeout with no phase signal, so an operator could not tell at which lifecycle stage the connection died.
  • What was not tested: Did not exercise this on Windows or against the production deployment topology — the existing CI matrix and the included unit regression coverage exercise the same code path on Linux/macOS runners; phase reporting is platform-independent because it lives entirely in the JavaScript handshake state machine.
  • Before evidence (optional but encouraged): Same tsx scripts/dev/repro-79603-handshake-phase.ts run on origin/main (commit b61ec7110360) prints the same warn lines without any phase=… field — only the unchanged cause=handshake-timeout outcome.

Root Cause (if applicable)

For bug fixes or regressions, explain why this happened, not just what changed.

  • Root cause: The Gateway WebSocket connection handler tracked only a coarse handshakeState (pending|connected|failed). Failure paths (preauth timeout and pre-connect close) logged the outcome but never recorded which lifecycle stage was last reached, so logs could not distinguish (e.g.) "auth token never arrived" from "auth token rejected" from "session attach failed".
  • Missing detection / guardrail: No phase tracker existed, so the failure logs could not surface this dimension.
  • Contributing context (if known): The handler grew incrementally and the fine-grained lifecycle state was only implicit in the message-handler control flow.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/gateway/server/ws-connection.test.ts
  • Scenario the test should lock in: phase tracker starts at ws_upgrade_started, advances monotonically through the handshake, is included in pre-connect close + handshake-timeout logs, and is omitted from logs once the session reaches ready.
  • Why this is the smallest reliable guardrail: the regression sits entirely inside the connection handler's lifecycle, so a unit harness around attachGatewayWsConnectionHandler is sufficient.
  • Existing test that already covers this (if any): None — added new cases.
  • If no new test is added, why not: N/A.

User-visible / Behavior Changes

User-visible runtime log content gains a phase=<value> token on failure-only WebSocket close paths and on the preauth handshake timeout warn. Successful connection closes are unchanged (phase intentionally omitted to keep the steady-state log shape stable). Documented in CHANGELOG.md under Unreleased / Gateway/diagnostics.

Diagram (if applicable)

Before:
[client opens WS] -> [server: handshake state = pending] -> [timeout fires]
                                                        -> log: cause=handshake-timeout (no phase)

After:
[client opens WS] -> [phase = ws_upgrade_started]
                  -> [phase = auth_token_received]
                  -> [phase = auth_validated]
                  -> [phase = session_attached]
                  -> [phase = subscriptions_registered]
                  -> [phase = ready]                            (success: phase omitted from logs)
                  -> on failure at any step:
                        log: cause=handshake-timeout|<close>, phase=<last completed>

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No
  • If any Yes, explain risk + mitigation: N/A — diff is enum-only diagnostic logging plus regression tests.

Repro + Verification

Environment

  • OS: Linux x86_64
  • Runtime/container: Node.js v22.16.0 (matches engines.node >= 22.16.0)
  • Model/provider: N/A (gateway WebSocket lifecycle, no LLM call)
  • Integration/channel (if any): WebSocket Gateway preauth handshake
  • Relevant config (redacted): default openclaw config; preauthHandshakeTimeoutMs = 250 set inline by the repro script

Steps

  1. Check out fix/79603-gateway-ws-handshake-phase-logging.
  2. pnpm install.
  3. pnpm exec tsx scripts/dev/repro-79603-handshake-phase.ts.

Expected

Two warn lines appear: a handshake timeout line and a closed before connect line, each carrying phase=ws_upgrade_started (and the structured close metadata also includes "phase":"ws_upgrade_started").

Actual

Matches expected. See terminal capture above.

Evidence

  • Failing test/log before + passing after — see Real behavior proof block above.
  • Trace/log snippets — included verbatim above.
  • Screenshot/recording — N/A (text-only diagnostic; copied live output from the live Node run is included).
  • Perf numbers (if relevant) — N/A.

Human Verification (required)

  • Verified scenarios: Live preauth handshake timeout on local Node 22 gateway prints phase=ws_upgrade_started in both warn lines and structured close metadata.
  • Edge cases checked: Successful (ready) closes do NOT print phase (regression coverage in ws-connection.test.ts).
  • What you did not verify: Windows runtime — phase is platform-independent and lives in the JS handshake state machine.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

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

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No
  • If yes, exact upgrade steps: N/A.

Risks and Mitigations

  • Risk: Log shape change for failure paths could surprise log-parsing tooling.
    • Mitigation: Phase token is appended to the existing line and added to the structured metadata under a new phase field; existing tokens are unchanged. Successful close shape is unchanged.

@dmytrovk dmytrovk closed this May 10, 2026
@dmytrovk dmytrovk reopened this May 10, 2026
@dmytrovk

Copy link
Copy Markdown
Author

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 matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

  • Problem:
  • Why it matters:
  • What changed:
  • What did NOT change (scope boundary):

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Closes #
  • Related #
  • This PR fixes a bug or regression

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.

  • Behavior or issue addressed:
  • Real environment tested:
  • Exact steps or command run after this patch:
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):
  • Observed result after fix:
  • What was not tested:
  • Before evidence (optional but encouraged):

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, write Unknown.

  • Root cause:
  • Missing detection / guardrail:
  • Contributing context (if known):

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

  • Coverage level that should have caught this:

    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file:

  • Scenario the test should lock in:

  • Why this is the smallest reliable guardrail:

  • Existing test that already covers this (if any):

  • If no new test is added, why not:

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.

Before:
[user action] -> [old state]

After:
[user action] -> [new state] -> [result]

Security Impact (required)

  • New permissions/capabilities? (Yes/No)
  • Secrets/tokens handling changed? (Yes/No)
  • New/changed network calls? (Yes/No)
  • Command/tool execution surface changed? (Yes/No)
  • Data access scope changed? (Yes/No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS:
  • Runtime/container:
  • Model/provider:
  • Integration/channel (if any):
  • Relevant config (redacted):

Steps

Expected

Actual

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios:
  • Edge cases checked:
  • What you did not verify:

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

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

  • Backward compatible? (Yes/No)
  • Config/env changes? (Yes/No)
  • Migration needed? (Yes/No)
  • If yes, exact upgrade steps:

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

  • Risk:

    • Mitigation:

@dmytrovk dmytrovk closed this May 10, 2026
@dmytrovk
dmytrovk deleted the fix/79603-gateway-ws-handshake-phase-logging branch May 10, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime scripts Repository scripts size: M triage: blank-template Candidate: PR template appears mostly untouched. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. triage: refactor-only Candidate: refactor/cleanup-only PR without maintainer context.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant