fix(browser): return JSON-RPC errors for malformed CDP client frames in relay bridge by krissding · Pull Request #102071 · openclaw/openclaw

krissding · 2026-07-08T06:39:09Z

What Problem This Solves

The browser extension relay bridge's attachCdpClientSocket silently
drops malformed JSON and invalid CDP request frames without sending any
response to the client. This leaves automation clients (e.g. Playwright
connectOverCDP) waiting for their own timeout instead of receiving an
immediate protocol error.

Fixes #102057.

Why This Change Was Made

The onMessage handler in attachCdpClientSocket had two silent-return
paths:

catch block after JSON.parse — malformed JSON
Missing id (number) or method (string) guard — invalid request

Both returned without writing anything to the client socket. The fix
sends JSON-RPC 2.0 error responses:

Parse error (-32700): malformed JSON, id: null
Invalid Request (-32600): missing required fields, preserves id
from the parsed object when available

This matches the existing JSON-RPC error precedent in the gateway MCP
HTTP path (src/gateway/mcp-http.ts), which already returns -32700
and -32600 at protocol boundaries.

toErrorPayload was loosened from id: number to id: number | null
so parse errors can use a null id as specified by JSON-RPC 2.0.

Evidence

Test results (Vitest)

$ npx vitest run extensions/browser/src/browser/extension-relay/relay-bridge.test.ts --no-coverage

 Test Files  1 passed (1)
      Tests  13 passed (13)
   Duration  10.99s

3 regression tests added (all existing 10 tests continue to pass):

Malformed JSON ({) → parse error -32700 with id: null
Invalid request (no id or method) ({params:{}}) → invalid
request -32600 with id: null
Has id but no method ({id:7}) → invalid request -32600 with
id: 7

Pre-commit checks pass

$ git diff --cached --stat
 extensions/browser/.../relay-bridge.test.ts | 40 ++++++++++++++++++++++
 extensions/browser/.../relay-bridge.ts      | 16 ++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)

No lockfile or unrelated file changes.

Merge Risk

Low. The change is scoped to two guarded early-return paths in a single
CDP client message handler. The only behavioral difference is that
clients now receive an error response instead of silence. Normal CDP
routing through handleCdpRequest is unchanged.

…in relay bridge Silently dropping malformed JSON or invalid CDP request objects leaves automation clients waiting for their own timeout. Return standard JSON-RPC parse error (-32700) and invalid-request error (-32600) at the CDP client socket boundary, matching the precedent in gateway MCP HTTP. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clawsweeper · 2026-07-08T07:06:43Z

Codex review: needs real behavior proof before merge. Reviewed July 8, 2026, 3:14 AM ET / 07:14 UTC.

Summary
The branch changes the browser extension relay bridge to send parse-error and invalid-request CDP error frames for malformed client messages and adds relay-bridge regression tests.

PR surface: Source +14, Tests +40. Total +54 across 2 files.

Reproducibility: yes. Current main has a high-confidence source-level reproduction: attach a CDP client through ExtensionRelayBridge.attachCdpClientSocket(...) and send malformed JSON or an object without numeric id plus string method; the handler returns without sending a response.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: extensions/browser/src/browser/extension-relay/relay-bridge.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #102057
Summary: The canonical remaining work is the linked browser relay malformed-frame bug; this PR and #102070 are separate candidate fixes for that same issue.

Members:

canonical: [Bug]: browser extension relay leaves CDP clients waiting when malformed frames arrive #102057 - The PR body uses closing syntax for this issue, and the issue describes the same malformed CDP client frame behavior.
fixed_by_candidate: fix(browser): report malformed relay CDP frames #102070 - This sibling open PR also closes the same issue and changes the same relay bridge files, but it is not merged or proof-positive enough to supersede this PR automatically.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Preserve parsed string sessionId on invalid-request error responses and add a regression test for that case.
[P1] Add redacted real behavior proof from a relay/CDP client run, not only unit test output.
[P1] Coordinate with the sibling candidate PR so maintainers land one proven fix path.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR provides Vitest/precommit output only; before merge, add redacted live CDP relay proof such as terminal output, logs, or a transcript showing malformed frames receive errors, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

[P1] As submitted, invalid-request errors are sent without sessionId, so Playwright-style session-scoped clients may not receive the rejection on the session that sent the malformed frame.
[P1] Real behavior proof is absent; the PR body provides Vitest/precommit output only, not a live relay/client transcript, logs, terminal output, or artifact showing after-fix behavior.
[P1] Another open candidate PR targets the same linked issue, so maintainers should land one proven path and close or supersede the other rather than merging both.

Maintainer options:

Preserve session routing before merge (recommended)
Copy a string sessionId from the parsed invalid CDP frame into the -32600 error response and add a regression test for a session-scoped malformed request.

Next step before merge

[P1] No ClawSweeper repair job: this external PR needs contributor-supplied real behavior proof and a small PR-branch fix before normal merge review.

Security
Cleared: The diff is limited to authenticated relay message validation and tests, with no dependency, credential, permission, or supply-chain changes found.

Review findings

[P2] Preserve sessionId on invalid CDP request errors — extensions/browser/src/browser/extension-relay/relay-bridge.ts:513

Review details

Best possible solution:

Preserve a string sessionId when returning -32600 for parsed invalid CDP frames, cover that session-scoped case in relay-bridge tests, add redacted real CDP relay proof, then land one candidate fix for the linked issue.

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

Yes. Current main has a high-confidence source-level reproduction: attach a CDP client through ExtensionRelayBridge.attachCdpClientSocket(...) and send malformed JSON or an object without numeric id plus string method; the handler returns without sending a response.

Is this the best way to solve the issue?

No as submitted. The fix belongs at the CDP client boundary, but the invalid-request response should preserve a parsed string sessionId so session-scoped clients receive the error on the correct session.

Full review comments:

[P2] Preserve sessionId on invalid CDP request errors — extensions/browser/src/browser/extension-relay/relay-bridge.ts:513
The invalid-request branch always passes undefined for sessionId. OpenClaw pins Playwright 1.61.1, whose Chromium connection routes incoming messages by message.sessionId || '' before matching callbacks, and this bridge already echoes request.sessionId for normal errors. A malformed session-scoped frame with a numeric id can therefore be delivered to the root session instead of rejecting the session callback; preserve a string sessionId from the parsed frame when sending -32600.
Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7e0324263b86.

Label changes

Label changes:

add P2: This is a focused browser relay bugfix with limited blast radius, but the submitted patch still has a session-scoped CDP error-routing defect before merge.
add merge-risk: 🚨 message-delivery: The diff changes CDP error delivery and currently omits the sessionId Playwright uses to route responses to the correct CDP session.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides Vitest/precommit output only; before merge, add redacted live CDP relay proof such as terminal output, logs, or a transcript showing malformed frames receive errors, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review.

Label justifications:

P2: This is a focused browser relay bugfix with limited blast radius, but the submitted patch still has a session-scoped CDP error-routing defect before merge.
merge-risk: 🚨 message-delivery: The diff changes CDP error delivery and currently omits the sessionId Playwright uses to route responses to the correct CDP session.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides Vitest/precommit output only; before merge, add redacted live CDP relay proof such as terminal output, logs, or a transcript showing malformed frames receive errors, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +14, Tests +40. Total +54 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	15	1	+14
Tests	1	40	0	+40
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	55	1	+54

What I checked:

Current main silently drops malformed CDP client frames: On current main, attachCdpClientSocket catches JSON.parse failures and returns, then separately returns when the parsed frame lacks numeric id or string method, with no socket.send on either path. (extensions/browser/src/browser/extension-relay/relay-bridge.ts:499, 7e0324263b86)
CDP entry point uses this bridge: The loopback relay server binds /cdp WebSocket upgrades directly to bridge.attachCdpClientSocket(...), so the changed handler is the client protocol boundary. (extensions/browser/src/browser/extension-relay/relay-server.ts:192, 7e0324263b86)
Patch omits session routing on invalid requests: The PR's invalid-request branch preserves a numeric id but passes undefined as the error sessionId, unlike normal session-scoped CDP responses and errors. (extensions/browser/src/browser/extension-relay/relay-bridge.ts:513, bfe6dd375130)
Existing relay errors preserve sessionId: The existing respondError helper sends toErrorPayload(request.id, request.sessionId, ...), so valid session-scoped error responses already keep the session routing field. (extensions/browser/src/browser/extension-relay/relay-bridge.ts:559, 7e0324263b86)
Playwright dependency routes responses by sessionId: OpenClaw pins playwright-core 1.61.1, and that version's Chromium connection receives a protocol message, selects _sessions.get(message.sessionId || ''), then matches callbacks inside that session; a missing sessionId routes a session response to the root session. (package.json:2023, 7e0324263b86)
Linked issue and sibling candidate: The linked bug report remains open and is referenced by this PR and by another open candidate PR, but the sibling PR is unmerged and does not provide positive real behavior proof, so it is not a safe superseding close target for this PR.

Likely related people:

vincentkoc: Blame for attachCdpClientSocket points to commit 4969c44, and live PR metadata maps that browser plugin refactor to refactor(deadcode): localize browser plugin declarations #101898 authored and merged by vincentkoc. (role: introduced behavior and recent area contributor; confidence: high; commits: 4969c4422694, e260600c1066; files: extensions/browser/src/browser/extension-relay/relay-bridge.ts, extensions/browser/src/browser/extension-relay/relay-bridge.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-09T11:02:09Z

Closing as a duplicate of #102070, now landed in 56096eb8590c7998534504b64745efa2494b0925.

The landed fix covers both malformed JSON (-32700) and structurally invalid CDP requests (-32600), preserves flat-session routing ids, and passed focused Testbox, real authenticated relay/WebSocket, and exact-head CI proof. Thank you for independently identifying and fixing the silent-drop hang.

openclaw-barnacle Bot added the size: S label Jul 8, 2026

clawsweeper Bot mentioned this pull request Jul 8, 2026

fix(browser): report malformed relay CDP frames #102070

Merged

chenxiaoyu209 mentioned this pull request Jul 8, 2026

fix(browser): return JSON-RPC errors for malformed CDP client frames in relay bridge #102115

Closed

steipete closed this Jul 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(browser): return JSON-RPC errors for malformed CDP client frames in relay bridge#102071

fix(browser): return JSON-RPC errors for malformed CDP client frames in relay bridge#102071
krissding wants to merge 1 commit into
openclaw:mainfrom
krissding:fix/cdp-relay-malformed-json-error

krissding commented Jul 8, 2026

Uh oh!

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

krissding commented Jul 8, 2026

What Problem This Solves

Why This Change Was Made

Evidence

Test results (Vitest)

Pre-commit checks pass

Merge Risk

Uh oh!

clawsweeper Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading