fix(cli): honor --connect on browse env remote#2059
Conversation
`browse env remote --connect <session-id>` ignores the connect flag. The env command writes the mode override and starts the daemon, but never writes the connect file. As a result, the daemon initializes a fresh Browserbase session instead of resuming the requested one — even though the next command will write the connect file too late, after the daemon has already created its own session. Two changes: 1. The env handler now writes (or removes) the connect file alongside the mode override, so a fresh daemon starts with the right session ID. 2. `needsRestart` now also fires when the desired connect ID differs from the current one. Without this, switching `--connect` while already in remote mode would short-circuit the restart and keep the old session. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 3/5
- There is a concrete regression risk in
packages/cli/src/index.ts: when--connectis omitted but a connect file already exists,opts.connectbecomesundefined→desiredConnectbecomesnull, which can incorrectly look like a config change and trigger a daemon restart. - Given the high confidence and user-facing behavior impact (unexpected restarts), this is more than a minor housekeeping issue and adds moderate merge risk.
- Pay close attention to
packages/cli/src/index.ts- connect-state comparison logic may treat an omitted flag as a change and restart the daemon unnecessarily.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/index.ts">
<violation number="1" location="packages/cli/src/index.ts:1992">
P1: Spurious daemon restart when `--connect` is not passed but a connect file exists. Since `opts.connect` is `undefined`, `desiredConnect` becomes `null`, which differs from the existing file content, triggering a restart. But the file-management logic intentionally leaves the connect file untouched when `--connect` isn't passed—so the daemon restarts only to reconnect to the same session.
Align the restart condition with the file-management semantics: only treat connect as "changed" when the user explicitly passes `--connect`.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User
participant CLI as CLI (env handler)
participant FS as Local Filesystem
participant Daemon as Browserbase Daemon
participant API as Browserbase API
Note over User,API: Mode Selection: bb browse env remote --connect [id]
User->>CLI: browse env remote --connect "abc-123"
CLI->>FS: Read current mode & connect files
FS-->>CLI: currentMode, currentConnectID
alt CHANGED: Mode changed OR (remote AND Connect ID changed)
CLI->>Daemon: Stop running daemon
Daemon-->>CLI: Stopped
end
CLI->>FS: Write mode override ("browserbase")
alt NEW: Remote mode with --connect
CLI->>FS: NEW: Write Connect ID to file
else NEW: Switching to local mode
CLI->>FS: NEW: Unlink Connect ID file (cleanup)
end
CLI->>Daemon: ensureDaemon()
Note over Daemon,API: Daemon Initialization
Daemon->>FS: Read mode & Connect ID file
FS-->>Daemon: mode="remote", connectID="abc-123"
alt Connect ID exists
Daemon->>API: Attach to existing session "abc-123"
else No Connect ID
Daemon->>API: Create new session (browse_cli: true)
end
API-->>Daemon: Session Ready
CLI-->>User: Success (remote mode active)
Note over User,API: Command Execution (Subsequent call)
User->>CLI: browse open "https://example.com"
CLI->>Daemon: Proxy request to active session
Daemon->>API: Navigate session to URL
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // no connect file | ||
| } | ||
| const desiredConnect = (opts.connect as string | undefined) ?? null; | ||
| const connectChanged = desiredConnect !== currentConnect; |
There was a problem hiding this comment.
P1: Spurious daemon restart when --connect is not passed but a connect file exists. Since opts.connect is undefined, desiredConnect becomes null, which differs from the existing file content, triggering a restart. But the file-management logic intentionally leaves the connect file untouched when --connect isn't passed—so the daemon restarts only to reconnect to the same session.
Align the restart condition with the file-management semantics: only treat connect as "changed" when the user explicitly passes --connect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/index.ts, line 1992:
<comment>Spurious daemon restart when `--connect` is not passed but a connect file exists. Since `opts.connect` is `undefined`, `desiredConnect` becomes `null`, which differs from the existing file content, triggering a restart. But the file-management logic intentionally leaves the connect file untouched when `--connect` isn't passed—so the daemon restarts only to reconnect to the same session.
Align the restart condition with the file-management semantics: only treat connect as "changed" when the user explicitly passes `--connect`.</comment>
<file context>
@@ -1980,7 +1980,18 @@ envCommand.action(
+ // no connect file
+ }
+ const desiredConnect = (opts.connect as string | undefined) ?? null;
+ const connectChanged = desiredConnect !== currentConnect;
+ const needsRestart =
+ currentMode !== mapped || mapped === "local" || connectChanged; // local always restarts to pick up strategy change
</file context>
Summary
bb browse env remote --connect <session-id>(orbrowse env remote --connect ...) silently ignores the--connectflag.The env handler currently writes only the mode override and starts the daemon. The connect file (which the daemon reads at init time to set
browserbaseSessionID) is only managed by the generic command dispatcher. So in practice:browse env remote --connect <id>→ mode file written, daemon spawnedbrowse open <url>→ also writes connect file via the generic dispatcher, but the daemon has already created its own fresh sessionNet effect: a brand-new companion session is created (tagged
browse_cli: true), and the user's session ID is never resumed. Logs and recordings live on the companion session — not the one the user passed in.Fix
Two small changes to the env action:
mapped === "browserbase". Switching to local removes any stale connect file.needsRestartonly checks mode; if you're already in remote mode and switch--connect, the daemon would otherwise stay running with the old session.Reproduction (before)
After
The user's
abc-123session is the one that gets driven. No companion session is created.Test plan
browse env remote --connect <id>followed bybrowse open <url>drives the supplied session (verify in dashboard)browse env remote --connect <id1>thenbrowse env remote --connect <id2>restarts the daemon and switches sessionsbrowse env localafterbrowse env remote --connect <id>cleans up the connect filebrowse env remote(no--connect) still works and creates a fresh session🤖 Generated with Claude Code