feat(browser): side panel copilot with per-tab agent sessions#93680
Closed
anagnorisis2peripeteia wants to merge 18 commits into
Closed
feat(browser): side panel copilot with per-tab agent sessions#93680anagnorisis2peripeteia wants to merge 18 commits into
anagnorisis2peripeteia wants to merge 18 commits into
Conversation
openclaw-barnacle
Bot
added
app: macos
Contributor
|
Codex review: found issues before merge. Reviewed July 16, 2026, 12:14 AM ET / 04:14 UTC. Summary PR surface: Source +1383, Tests +351, Docs +48. Total +1782 across 11 files. Reproducibility: yes. for the review blocker: the required documentation command sets the complete allowed-origin array, so following it without manually reconstructing existing entries demonstrably replaces those settings. The feature itself is not a bug reproduction and has convincing positive Chromium proof. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Keep the per-tab copilot only with browser-extension owner approval, add a guided setup or CLI/Doctor path that safely appends the exact extension origin without replacing existing settings, and document and test the approved operator-credential lifecycle and reconnect behavior. Do we have a high-confidence way to reproduce the issue? Yes for the review blocker: the required documentation command sets the complete allowed-origin array, so following it without manually reconstructing existing entries demonstrably replaces those settings. The feature itself is not a bug reproduction and has convincing positive Chromium proof. Is this the best way to solve the issue? No, not yet. The side-panel and per-tab session design is plausible, but requiring manual whole-list security configuration is not the safest setup path, and bundled chat ownership plus credential persistence still requires explicit product and security approval. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4b7f193c9d57. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1383, Tests +351, Docs +48. Total +1782 across 11 files. View PR surface stats
Security concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (2 earlier review cycles)
|
Contributor
Author
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Contributor
Author
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Contributor
Author
|
@clawsweeper re-review |
anagnorisis2peripeteia
marked this pull request as ready for review
June 17, 2026 05:36
Contributor
Author
|
@clawsweeper re-review |
github-actions
Bot
added
the
dependencies-changed
Contributor
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
This was referenced Jun 18, 2026
openclaw-barnacle
Bot
added
scripts
clawsweeper
Bot
added
proof: sufficient
The panel recorded subscribedKey before the subscribe round trip and only cleared it when settings changed, so a dropped socket or a failed subscribe left a stale key that made subscribeSession() early-return and silently never resubscribe. Record the key only once the gateway confirms, and clear it whenever the socket goes away. The NOT_PAIRED retry also closed the socket before calling connect(), which awaits storage before reassigning ws - so the close listener still saw the current socket and scheduled a competing reconnect. Route both teardown sites through dropSocket(), which nulls ws before close() the way the settings path already did.
Mutation testing surfaced a real gap, not an equivalent mutant: replacing the "ws:" literal in gatewayUrlFromRelayUrl() with "" left every test passing, because wss:// misses that literal, http:// fails both branches identically, and the loopback case returns null via the pathname check regardless. Nothing exercised a ws:// relay that should succeed, even though the protocol check deliberately accepts it.
…s thread Two silent-failure paths the review surfaced. The handshake sets 'Connected' before handleHelloOk() binds the tab session, and the rejection was dropped, so a bind failure left the panel looking healthy over a session that never attached; handleHelloOk() now owns that failure and reports it. The tabs.onRemoved handler cleared pinnedTabId but left sessionKey and the input live, and sendMessage() only guards on the socket - so a turn could still be sent into the thread of a tab that no longer exists. Release the session and close input when the pinned tab goes away.
…ad session sendMessage() disables input and the only path that re-enables it is the run's terminal chat event, which handleChatEvent() drops when payload.sessionKey no longer matches. Two deterministic lockouts followed. Clicking New chat mid-run swaps sessionKey, so the in-flight run's terminal event is filtered and the composer never comes back; a socket drop mid-run is worse, because no timeout covers a chat event that will never arrive. Abandon the turn with the socket - fail waiting requests instead of letting them sit until the 30s timeout, finalize the bubble, and hand the composer back - and re-enable input when a fresh thread starts. Also report a send attempted while reconnecting instead of silently dropping it.
…acing itself Three lifecycle gaps the review found. bindTabSession() returned early when the derived key already matched sessionKey, which is exactly the reconnect case, so a reconnected panel never resubscribed and silently received no chat events - clearing subscribedKey on close was not enough on its own. dropSocket() left a reconnect timer scheduled by an earlier close running, so it fired alongside the connect() that follows the drop and left two sockets. And abandonInFlightTurn() re-enabled the composer unconditionally, undoing the deliberate lockout for a tab that had been closed.
Four gaps the review found. A reconnect mid-run can deliver only the terminal chat event - the gateway dedupes its pre-terminal flush against what it already broadcast, so the reply reaches the new socket once, in that payload's snapshot - and the terminal branch discarded it whenever no streamed bubble existed, so the reply silently never rendered. It dropped errorMessage the same way when a run failed before any delta. Render both. The NOT_PAIRED retry timer was untracked while the gateway also closes the socket, so a stale timer could tear down an already-healthy connection after the device was approved. onNewChat() bumped the persisted tab generation before any gateway round trip and threw into a void handler, stranding the pane on the old thread while sessionKey pointed at the new one. And Ed25519 WebCrypto only ships enabled from Chrome 137 while the manifest supports 125, where the panel died as a bare 'Auth failed' - handleChallenge now reports the real cause.
…panel origin The review found the panel's own trust boundary was the one being crossed. buildTabPreamble interpolated document.title and the URL straight into every turn, so a hostile page title carrying ']' and a newline could break out of the preamble and issue instructions with the user's authority - while this plugin already fences browser-originated text as untrusted before agents see it (browser-tool.actions.ts). Sanitize both values and fence them the same way. The documented setup was also wrong. The panel is an extension page, so its WebSocket always sends an Origin header, which forces the gateway's origin check; allowedOrigins is empty by default, so the 'pairs silently on loopback' flow actually failed closed and surfaced only a bare status code. Make the allowlist step unconditional in the docs and body, and report what the gateway said instead of a lone code. Also clear the pairing retry timer on hello-ok - the close-driven backoff beats it, so an approval landing in that window was torn down at +5s - and dedupe the auth-failure notice, which otherwise repeats forever on pre-137 Chrome.
…oad's absence Mutation surfaced that the injection tests only checked the hostile payload was absent, so a sanitizer that swapped the delimiters for other junk, or dropped newlines instead of spacing them, still passed while mangling honest titles. Pin the exact fenced text, and pin the instructions the preamble must still carry.
…und trips startFreshThread() persisted the generation bump and swapped sessionKey, then awaited the gateway before touching the pane. If either call threw, the pane kept showing the old thread while every later send landed in the new one, and the error claimed the fresh conversation had not started when the swap was already permanent. Show the new thread first, so a failure reports only what is actually missing - the gateway session - against a pane that matches where sends go. bindTabSession() also subscribed and hydrated with its locally derived key while ensureSession() adopts the gateway's canonical echo into sessionKey, which is what handleChatEvent filters on. Identity today, but a canonicalizing gateway would leave that path listening to a key no event carries; both paths now use the canonical key.
…ng the reply Dropping the socket mid-run freezes the partial bubble and resets the stream, so a run that survived the reconnect came back with an unmatched runId and rendered its whole cumulative snapshot underneath the partial - the same reply twice. No data was lost, but it is a display bug in exactly the lifecycle this panel is meant to get right. chat.history carries the in-flight run, so redraw from it on a same-key reconnect. The docs also handed users a foot-gun: 'openclaw config set gateway.controlUi.allowedOrigins' sets the whole list, so a copy-paste would revoke any origins already allowed, the Control UI's own included. Say so.
The previous commit redrew from chat.history on a same-key reconnect to stop a surviving run repeating its text. Driving it proved that trade backwards: the redraw wiped the pane mid-run and rebuilt from a history that did not yet carry the just-sent user message, so the turn vanished from the pane while the agent worked on it. A duplicated reply is cosmetic; a lost message is not. Keep the resubscribe, drop the rehydrate, and record why at the call site.
…a dead branch The terminal chat event's snapshot was only applied when no bubble existed. The gateway broadcasts the pre-terminal flush drop-if-slow but not the terminal event, so a slow connection could miss the flush and keep stale partial text as the final answer. applyChatDelta is snapshot-idempotent, so applying it every time costs nothing and closes that gap. The session.tool branch could never fire: that mirror goes to sessions.subscribe subscribers and the panel subscribes to messages, not sessions. Worse, the comment claimed the opposite. Drop the branch and say what actually reaches the panel - a run it did not start shows no step lines.
…hole URL isLoopbackUrl decides whether the gateway token is optional, and it matched //localhost anywhere in the string - including in a path, query or fragment. So https://evil.example/x//localhost/ read as loopback and the panel would have connected to a remote gateway with no token at all. The existing test only covered a loopback-looking host label, never a loopback-looking path. Parse the URL and compare the hostname. Tests pin the path, query, fragment and userinfo variants, and fail against the old regex.
Tab ids are only unique within a browser session - Chrome reissues them from a low counter every launch - but gateway sessions persist. Keying a thread on the tab id alone therefore handed a brand-new tab the previous launch's conversation for that id: hydrateHistory rendered a stranger's thread and new turns continued it. That breaks the panel's central promise that two tabs never mix context, and it hits low-numbered tabs after every restart. Fold a per-launch nonce (kept in storage.session, alongside the generation map it already scopes) into the derived key, and refuse to derive a key without it rather than falling back to a colliding one. This trades resume-across-restart, which tab-id reuse had already made meaningless, for correctness.
Contributor
Author
|
@clawsweeper re-review Force-pushed to Since the last push, driven review found and fixed real defects rather than cosmetics — each one
The recording is driven on the gateway config the docs prescribe — Review history, with every finding, disposition and the evidence behind it: |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
Contributor
Author
|
@clawsweeper re-review |
Contributor
|
🦞👀 Command router queued. I will update this comment with the next step. |
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
Contributor
Author
|
superseded by #109817 |
Contributor
|
AI-assisted (approved by Peter) Thanks @2CB2 — your implementation established the side-panel copilot direction and served as the basis for the secure rebuild in #109817. The landed version keeps that product model while adding dedicated device pairing and token custody, Gateway-owned targeted session delivery, typed tab enforcement, per-tab Side Panel instances, durable archive-on-tab-close retention, denial UX, and Chromium isolation/lifecycle proof. Closing this prototype in favor of #109817. Thank you for the substantial foundation and for pushing this forward. |
Contributor
Author
@steipete thanks! That's my handle on a different app if you wouldn't mind using: @anagnorisis2peripeteia |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
#100619 gave OpenClaw a way to drive your signed-in Chrome: the bundled extension
attaches with
chrome.debugger, and the OpenClaw tab group is the consent boundary. What itdoes not give you is a way to talk to the agent about the tab you are looking at.
Today, asking anything about the page in front of you means leaving the browser — Telegram, the
CLI, or the Control UI — and then telling the agent, in words, which tab you meant. Two things
fall out of that:
are on lands in the same thread as everything else, and the agent has to guess which of the
shared tabs you mean.
questionnaire is exactly the task you want to hand to an agent while looking at it — and
exactly the task that is painful to drive from a chat app on your phone.
Why This Change Was Made
The extension already owns the hard parts — the relay,
chrome.debugger, and the tab-groupconsent model. The missing piece is a conversation surface pinned to a tab, so this PR adds
a side panel on top of what #100619 merged rather than building anything new underneath it:
WebSocket as an ordinary operator client (protocol v4, Ed25519 device identity), so the relay
stays a pure CDP/tab-consent plane and the panel is just another gateway client.
(
<agent-main>:thread:tab-<id>), so two tabs never mix context and reopening the panel on atab resumes its thread. The key is the identity — no client-side history storage.
shared into the OpenClaw tab group; its "Stop sharing" button routes through the extension's
existing
toggleShareTabhandler. Dragging a tab out (or dismissing Chrome's banner) revokesthe agent exactly as before.
operator.read+operator.write. Twoconsequences, both deliberate: "New chat" mints a fresh per-tab key generation instead of
calling the admin-only
sessions.reset, and tab targeting rides a message preamble ratherthan the admin-only
browser.request. No admin scope, no new privileged surface.Scope boundary. This supersedes this PR's previous node-anchored design, which stacked on
#93411 (
gateway.tools.byNode). #100619 made that unnecessary — it proxies browser actions to anode host natively — so #93411 is closed and all of the node-anchored routing, the
extension-bridgedriver, and the second bridge/relay are dropped. What remains is the sidepanel and per-tab sessions, layered on the merged extension. No core, gateway, or protocol
changes.
User Impact
Open the popup, click Open copilot panel, and ask for things in plain language about the tab
you are on — "fill this form with my details", "summarize this thread". The agent drives that
tab through the normal
browsertool.sends
Origin: chrome-extension://<id>, and the Gateway checks any origin againstgateway.controlUi.allowedOrigins— which is empty by default. The extension's origin must beadded there once, on the same machine too; loopback does not exempt it. Without it the panel now
reports
Gateway refused the connection: origin not allowedrather than looping silently. Afterthat it pairs silently locally; a remote gateway needs a one-time
openclaw devicesapproval.Gateway URL/token live in the panel's ⚙ settings, and a gateway-hosted relay pairing already
names the gateway, so remote setups need no second URL. The panel needs Chrome 137+ (Ed25519
device keys); the rest of the extension keeps working below that and says so.
chromeprofilebehave exactly as they do today; the panel is additive.
require one entry in the existing
gateway.controlUi.allowedOriginslist (see Setup). Nothingchanges for existing users who never open the panel.
Docs:
docs/tools/chrome-extension.mdgains a "Side panel copilot" section.Evidence
Real behavior, paired before/after, same surface, real Chromium 152 (not headless
Chrome-for-Testing). Identical scenario both sides: load the extension, pair, share the
example.org tab, open the popup from the toolbar, and use the panel to ask the agent to navigate
that shared tab to wikipedia.org.
56eda48f20f)c688f94de6c)Stop sharing this tab,UnpairOpen copilot panelsidepanel.htmlRecording: the whole thing in one take (32s, real Chromium 152)
One continuous take, no cuts: fresh per-tab thread, request typed, agent drives the shared tab.
Driven at this PR's head on the gateway config this page documents —
allowedOriginsholdingonly the extension's own origin, nothing permissive.
The stills below are frames of this same recording.
https://raw.githubusercontent.com/anagnorisis2peripeteia/openclaw/d1f3da6b98fb22f18e679ba2abb92881e9657b8b/05-panel-demo.mp4
Visual before/after (same surface, real Chromium 152)
Before — base
56eda48f20f. Same extension popup, same clicks. Only "Stop sharing this tab"and "Unpair": there is no copilot surface to open, so the scenario cannot start. (This capture was
taken at
c94da0df6f3before the branch was rebased; no commit between that revision and thecurrent base touches
extensions/browser/, so the base extension is byte-identical and the popupis unchanged. Re-verified on the current base:
sidepanel.htmlstill does not exist andpopup.jsstill contains zero panel references.)After — this PR. The popup gains "Open copilot panel"; clicking it docks the side panel,
which shows green Connected, pins the shared tab, and reports "This tab has its own
conversation".
After — asking in natural language. Typing the request into the panel:
After — the agent drives the shared tab. The panel streams the reply and two
browsertoolsteps, and the same tab moves example.org → wikipedia.org (note the tab is still inside the
orange OpenClaw tab group — the consent boundary held):
sidepanel.htmlis absent andpopup.html/popup.jscontain zero panel references. The popup renders only "Stop sharingthis tab" and "Unpair", so the scenario cannot proceed: there is nothing to open.
Connected, pins the shared tab ("This tab has its own conversation"), and typing
"Navigate this tab to wikipedia.org" + Send streamed the assistant reply plus two
browsertool steps and drove the same tab (
E1A50B8F1A849579F7B316844119F354) from example.org tohttps://www.wikipedia.org/. Independently confirmed:What driving this for real, and reviewing it hard, actually caught — none of it visible to unit
tests or static checks:
client.id, which the gateway enums —the handshake failed with
invalid connect params: at /client/id. Only a real gateway handshakesurfaces that.
sends an
Originheader, which forces the gateway's origin check;allowedOriginsis empty bydefault, so the "pairs silently on loopback" flow this PR originally claimed actually failed
closed, and the panel showed only a bare status code. The first proof runs missed it because the
rig had
allowedOrigins: ["*"]. The recording above is now driven with only this extension'sown origin allowlisted, which is what the docs now tell you to do, and the panel reports what
the gateway said instead of looping silently.
buildTabPreambleinterpolated the page-controlleddocument.titleand URL straight into every turn, so a hostile title carrying]and a newlinecould break out and issue instructions with the user's own authority — bypassing the boundary
this plugin already enforces, where browser-originated text is fenced as untrusted before agents
see it (
browser-tool.actions.ts). Both values are now sanitized and fenced the same way, withtests that fail against the unfenced version.
deliver only the terminal chat event (the gateway dedupes its pre-terminal flush), and the panel
discarded that snapshot, so the reply never rendered. The composer only re-enables on a run's
terminal event, so New chat mid-run — or a dropped socket — stranded it permanently; the
lockout was reproduced and then fixed red-green on the live rig. Subscriptions were recorded
before the gateway confirmed them and never re-established after a reconnect. Teardown could race
its own retry timers.
Tests / checks
node scripts/run-vitest.mjs extensions/browser/chrome-extension/modules/panel-core.test.ts— 44 passing.Covers the per-tab key derivation (incl. the generation suffix), the protocol-v4 delta
segmenter (incremental
deltaText, cumulative-snapshot idempotency,replace=true,runId reset, tool-boundary segmentation, buffer-reset rebase), markdown escaping, and the
gateway-URL derivation.
marmorkrebs --tool strykeronmodules/panel-core.js, tests scoped topanel-core.test.ts): 187 mutants, score 0.95. It earned its place twice. It first showed the"ws:"literal ingatewayUrlFromRelayUrl()could be replaced with""and leave every testgreen — nothing exercised a
ws://relay that should succeed, though the check deliberatelyaccepts it. Later it showed the new preamble-sanitizer tests only asserted the hostile payload was
absent, so a sanitizer that swapped the delimiters for other junk, or dropped newlines instead of
spacing them, still passed while mangling honest titles. Both gaps now have tests, each red-green
verified against the unfixed code.
Two caveats stated rather than hidden. The score moves between runs of identical source because the
command runner starts a fresh vitest per mutant, so most mutants land as
Timeoutrather thanKilledand that split races. And the survivor list is not trustworthy at face value — each wasre-applied by hand, and at least one (
if (!payload || typeof payload !== "object")->false) isa false survivor: applying it fails the suite. The ones that do reproduce are equivalent mutants
(a scratch array whose seeded element is never referenced;
String(x ?? "")fallbacks where anyreplacement yields the identical result, since
new URL("…")throws exactly asnew URL("")does).Scope: the pure module is the unit-tested surface, mirroring this extension's existing boundary
(
modules/relay-core.jshasrelay-core.test.ts;background.js/popup.jshave no unittests). The chrome/DOM glue (
sidepanel.js,device-identity.js,popup.js) is covered by thebefore/after proof above rather than mocked unit tests.
node scripts/run-oxlint.mjs extensions/browser/chrome-extension— clean.oxfmtclean;node scripts/format-docs.mjs --checkclean;docs/docs_map.mdregenerated.Size (this PR only)
The extension ships unbundled, so
sidepanel.*andmodules/panel-core.*ride the existingcopy-chrome-extension.mjsstatic copy with no build change, and the colocatedpanel-core.test.tsandpanel-core.d.tsare excluded from the shipped artifact by thatscript's existing filter (matching
relay-core.js/.d.ts/.test.ts).Known limitation.
chrome.sidePanel.open()requires a real user gesture, so the panel opensfrom the popup button rather than the toolbar icon directly (the action already owns the popup
for pairing).
Review history: every review round on this branch (autoreview + local ClawSweeper), with the complete reviews and per-finding dispositions: https://gist.github.com/871e4fb8db67e9016c570197ccb5f099