fix(browser): tolerate brief extension relay disconnects on attached tabs#30232
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42391c3c4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Allow CDP clients to connect even during brief extension worker drops. | ||
| // Individual commands already wait briefly for extension reconnect. | ||
| wssCdp.handleUpgrade(req, socket, head, (ws) => { | ||
| wssCdp.emit("connection", ws, req); |
There was a problem hiding this comment.
Reject /cdp upgrades when relay has no active tab state
Allowing wssCdp.handleUpgrade unconditionally here means /cdp now accepts WebSocket sessions even when the extension has never connected (or after target cache cleanup), so clients that reconnect directly to /cdp can hold sockets open indefinitely while every command fails with "Extension not connected"; this is a behavioral regression from the previous immediate 503 gate and can cause hanging/retry loops plus unnecessary socket/resource retention in disconnected states.
Useful? React with 👍 / 👎.
Greptile SummaryExtended browser extension relay tolerance for brief MV3 extension worker disconnects by maintaining CDP endpoint availability while targets are cached. The relay now keeps Key changes:
Security preserved:
Scope: Confidence Score: 4/5
Last reviewed commit: 42391c3 |
…tabs Keep extension relay tab metadata available across short extension worker drops and allow CDP clients to connect while waiting for reconnect. This prevents false "no tab connected" failures in environments where the extension worker disconnects transiently (e.g. WSLg/MV3).
42391c3 to
a30cb0d
Compare
|
Landed via temp rebase onto main.
Checking formatting... All matched files use the correct format.
Found 0 warnings and 0 errors.
OK apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift
A2UI bundle up to date; skipping.
[copy-hook-metadata] Copied boot-md/HOOK.md
RUN v4.0.18 /Users/steipete/Projects/clawdbot3 RUN v4.0.18 /Users/steipete/Projects/clawdbot3 ✓ src/cli/program/preaction.test.ts (10 tests) 523ms Test Files 17 passed (17) ✓ src/shared/text/reasoning-tags.test.ts (12 tests) 6ms Test Files 718 passed | 1 skipped (719)
Thanks @Sid-Qin! |
Summary
Describe the problem and fix in 2–5 bullets:
/json/versionwebsocket advertisement while attached targets are cached, allows/cdpsocket connection during brief extension disconnects, and extends default reconnect grace to tolerate transient worker drops.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
The browser tool is more tolerant of brief extension worker disconnects and less likely to fail with false "no tab connected" diagnostics right after users attach a tab.
Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation:Repro + Verification
Environment
chrome)cdpUrlSteps
/json/versionand open/cdpwebsocket during disconnect window.Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm test -- --run src/browser/extension-relay.test.ts src/browser/server-context.ensure-tab-available.prefers-last-target.test.ts.Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoFailure Recovery (if this breaks)
src/browser/extension-relay.ts,src/browser/extension-relay.test.ts.Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None./cdpsocket pre-connect during extension disconnect could keep idle sockets open longer.