Skip to content

fix(cmux): inject cookies via a hidden background tab (no popup, no sidebar workspace) - #97

Merged
mvanhorn merged 4 commits into
mainfrom
fix/cmux-stale-workspace-ref
Jun 7, 2026
Merged

fix(cmux): inject cookies via a hidden background tab (no popup, no sidebar workspace)#97
mvanhorn merged 4 commits into
mainfrom
fix/cmux-stale-workspace-ref

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Problem

The cmux cookie-sync popped an about:blank browser pane into the user's focused workspace every ~30s, and left an agentcookie workspace in the left sidebar.

Two layered causes:

  1. Ref aliasing: the injection surface was parked in a dedicated agentcookie workspace cached by short ref (workspace:N). Short refs renumber when cmux restarts, and the cmux-sync LaunchAgent outlives cmux by days, so a cached ref could silently alias a live user workspace and drop the pane into focus.
  2. The workspace itself: even fixed, the dedicated workspace showed in the sidebar, and workspace create auto-spawns a Claude Code terminal, so the parking spot carried a phantom agent session.

Why a workspace was never needed

browser.cookies.set needs a surface_id, but WebKit cookies live at the WKWebsiteDataStore (profile) level, shared across every browser surface and surviving pane close. Verified empirically:

  • a cookies.set on one surface is visible to cookies.get on any other (profile-level), and
  • a non-selected (hidden) browser tab still accepts cookies.set (set: 1 on an unfocused tab, the user's selected surface undisturbed).

Fix

Add one unfocused about:blank browser tab to the first existing workspace cmux lists, and cache it. No workspace is created, nothing enters the sidebar, no phantom terminal, focus never moves. The only artifact is a small background tab chip. The host workspace is resolved fresh on every (re)add (never cached, since refs renumber across restarts); a stale-surface error re-adds once.

Deletes the workspace create/find/recreate machinery in favor of hostWorkspaceLocked + firstWorkspaceRef. Net -134 lines.

Verification

Live against a running cmux: restarted the cmux-sync daemon on this build. No agentcookie workspace appears; the injection tab lands unfocused in the first workspace; 7,324 cookies read back from it. 321 tests pass, go vet clean.

🤖 Generated with Claude Code

The #91 fix parked the injection surface in a dedicated background
"agentcookie" workspace, but cached the workspace by short ref
(workspace:N). Short refs renumber when cmux restarts, and the
cmux-sync LaunchAgent outlives cmux by days -- so after a restart the
cached ref can silently alias a live USER workspace. `browser open
--workspace` then succeeds with no error, the workspace-not-found
recreate path never fires, and the about:blank pane lands right in the
user's focused workspace again (observed 2026-06-06: the surface opened
inside an active email-task workspace whose post-restart ref collided
with the pre-restart agentcookie ref).

ensureWorkspaceLocked now re-resolves the background workspace by NAME
on every surface (re)open instead of trusting the cached ref. Name
lookup is the only identity stable across restarts; resolution only
happens when a surface must be (re)opened, so the extra
`workspace list` is rare. The cached field remains as last-resolved
state only.

Two regression tests cover the incident: a stale cached ref aliasing a
user workspace on fresh open, and the exact reopen-after-surface-error
path.

Verified live: killed the stale daemon, restarted on this build against
a running cmux -- it created the agentcookie workspace and parked the
about:blank surface there; the user workspace stayed clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jun 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the dedicated agentcookie background workspace with a single unfocused about:blank browser tab added to the first workspace cmux reports. The previous design had two layered defects: short workspace refs renumber across cmux restarts (causing the cached ref to alias a live user workspace), and the dedicated workspace itself appeared in the sidebar with a phantom agent terminal.

  • Workspace machinery removed: ensureWorkspaceLocked, findWorkspaceRef, isWorkspaceError, workspaceRefRE, and the workspaceRef field are all deleted. hostWorkspaceLocked replaces them — it calls workspace list --json fresh on every (re)add, intentionally never caching the ref.
  • Surface command updated: browser opencmux new-surface --type browser --workspace <first> --url about:blank --focus false.
  • Zero-workspace case: Push now hard-fails (soft, logged) instead of creating a workspace; TestCmuxPush_NoWorkspaceToHostIsAnError covers this path.

Confidence Score: 5/5

Safe to merge — focused deletion of workspace-management code replaced by a simpler, well-tested path.

Workspace refs are resolved fresh on every add preventing the stale-ref aliasing that was the root cause. Surface caching and re-add on stale-surface error work correctly. The zero-workspace hard-fail is the right behaviour. isSurfaceError only runs against setCookiesLocked errors so the retry loop is safe. No credential path reads or new outbound network sinks introduced.

No files require special attention.

Important Files Changed

Filename Overview
internal/sinkpush/adapter_cmux.go Replaces dedicated agentcookie workspace + browser open machinery with a single unfocused about:blank tab in the first existing workspace; removes workspaceRef field, isWorkspaceError, findWorkspaceRef, ensureWorkspaceLocked, and workspaceRefRE (-134 lines).
internal/sinkpush/adapter_cmux_test.go Test suite fully updated to the new hidden-tab model; adds TestCmuxPush_NoWorkspaceToHostIsAnError, TestCmuxPush_ReAddResolvesHostFreshNotCached, and TestFirstWorkspaceRef; removes TestIsWorkspaceError_OnlyMissingWorkspace and TestFindWorkspaceRef.

Reviews (3): Last reviewed commit: "style: gofmt adapter_cmux struct alignme..." | Re-trigger Greptile

Comment thread internal/sinkpush/adapter_cmux.go Outdated
mvanhorn and others added 2 commits June 6, 2026 21:38
With ensureWorkspaceLocked re-resolving by name every call, the cached
a.workspaceRef field was assigned but never read -- dead state that a
future maintainer could wire back into a new early-return without
realizing refs renumber across cmux restarts (the exact bug this PR
fixes). Removed the field entirely; the name-resolution invariant is now
enforced structurally rather than by comment. Tests assert the behavior
(every open preceded by `workspace list`, never targets a user
workspace) instead of poking the removed field.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…kspace

The dedicated "agentcookie" workspace approach had two user-visible
problems: it showed up as an entry in the left sidebar, and cmux
auto-spawns a Claude Code terminal on `workspace create`, so the parking
spot also carried a phantom agent session. (It also had the ref-aliasing
bug the earlier commits on this branch fixed -- short refs renumber
across cmux restarts, so a cached workspace ref could alias a live user
workspace and pop the pane into focus.)

Root cause of why a whole workspace was ever created: the belief that
browser.cookies.set needs a dedicated, persistent surface. It does need
*a* surface_id -- but WebKit cookies live at the WKWebsiteDataStore
(profile) level, shared across every browser surface (verified: a set on
one surface is visible to cookies.get on any other). And a non-selected
browser tab is a hidden webview that still accepts cookies.set (verified
live: set=1 on an unfocused tab, user's selected surface undisturbed).

So the adapter now adds ONE unfocused about:blank browser tab to the
first existing workspace cmux lists, and caches it. No workspace is
created, nothing enters the sidebar, no phantom terminal, and the user's
focus never moves. The only artifact is a small background tab chip. The
host workspace is resolved fresh on every (re)add -- never cached --
since refs renumber across restarts; a stale-surface error re-adds once.

This deletes the workspace create/find/recreate machinery
(ensureWorkspaceLocked, findWorkspaceRef, isWorkspaceError,
cmuxWorkspaceName) in favor of hostWorkspaceLocked + firstWorkspaceRef.

Verified live against a running cmux: restarted the cmux-sync daemon on
this build; no agentcookie workspace appears, the injection tab lands
unfocused in the first workspace, and 7324 cookies read back from it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mvanhorn mvanhorn changed the title fix(cmux): never trust the cached workspace ref across cmux restarts fix(cmux): inject cookies via a hidden background tab (no popup, no sidebar workspace) Jun 7, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mvanhorn
mvanhorn merged commit 6b12c61 into main Jun 7, 2026
5 checks passed
@mvanhorn
mvanhorn deleted the fix/cmux-stale-workspace-ref branch June 7, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant