fix: prevent state commands from starting daemon without session_name (#677)#964
Open
jin-2-kakaoent wants to merge 2 commits intovercel-labs:mainfrom
Open
fix: prevent state commands from starting daemon without session_name (#677)#964jin-2-kakaoent wants to merge 2 commits intovercel-labs:mainfrom
jin-2-kakaoent wants to merge 2 commits intovercel-labs:mainfrom
Conversation
(vercel-labs#677) State management commands (state_list, state_show, state_clear, state_clean, state_rename) are pure file operations that don't need a running daemon. Previously, these commands would trigger daemon startup via ensure_daemon(), and if AGENT_BROWSER_SESSION_NAME was exported after the first command (e.g. `state clear --all`), the daemon would start without session_name. Subsequent open/close commands would reuse that daemon, causing close to skip state persistence entirely. Fix: execute state management commands locally in the CLI process before ensure_daemon() is called. This is done via a new dispatch_state_command() function in state.rs that centralizes the command routing, used by both the CLI (local path) and the daemon (batch/IPC path). Also: - Add OutputOptions::from_flags() helper to deduplicate construction - Add unit tests for dispatch_state_command routing and error handling
Contributor
|
@hyunjinee is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
- Remove redundant closure in dispatch_state_command (clippy::redundant_closure) - Remove needless borrow in run_batch (clippy::needless_borrow) - Fix trailing blank lines (rustfmt) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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.
Summary
Fixes #677
--session-name/AGENT_BROWSER_SESSION_NAMEdoes not persist cookies or localStorage across restarts when a state management command (e.g.state clear --all) starts the daemon before the session name is set.Root Cause
AS-IS (Before)
session_nameis only passed to the daemon via environment variable at daemon spawn time. Once the daemon is running, there is no way to update itssession_name.sequenceDiagram participant Shell participant CLI participant Daemon participant Browser Note over Shell: SESSION_NAME not yet exported Shell->>CLI: agent-browser state clear --all CLI->>Daemon: ensure_daemon() → spawns daemon Note over Daemon: session_name = None ❌<br/>(env var not set at spawn time) Daemon-->>CLI: Done Note over Shell: export SESSION_NAME=my-session Shell->>CLI: agent-browser open https://example.com CLI->>Daemon: reuse running daemon (already_running=true) Note over Daemon: session_name = still None ❌ Daemon->>Browser: launch + navigate Shell->>CLI: agent-browser eval 'localStorage.setItem(...)' CLI->>Daemon: evaluate Daemon->>Browser: set cookie + localStorage ✅ Shell->>CLI: agent-browser close CLI->>Daemon: close Note over Daemon: handle_close():<br/>if session_name.is_some() → FALSE<br/>→ state save SKIPPED ❌ Daemon->>Browser: Browser.close Shell->>CLI: agent-browser open https://example.com CLI->>Daemon: ensure_daemon() → new daemon Note over Daemon: session_name = "my-session" ✅<br/>(env var now set) Note over Daemon: looks for state file → NOT FOUND ❌ Daemon->>Browser: launch (no restoration) Shell->>CLI: agent-browser eval 'localStorage.getItem(...)' Daemon->>Browser: evaluate Browser-->>CLI: null ❌The commands
state_list,state_show,state_clear,state_clean, andstate_renameare pure file operations that don't need a browser or daemon. Yet they were routed throughensure_daemon(), which could start a daemon withoutsession_name.TO-BE (After)
State management commands are now executed locally in the CLI process before
ensure_daemon()is called.sequenceDiagram participant Shell participant CLI participant Daemon participant Browser Note over Shell: SESSION_NAME not yet exported Shell->>CLI: agent-browser state clear --all Note over CLI: dispatch_state_command()<br/>→ local file operation<br/>→ daemon NOT started ✅ CLI-->>Shell: Done Note over Shell: export SESSION_NAME=my-session Shell->>CLI: agent-browser open https://example.com CLI->>Daemon: ensure_daemon() → spawns daemon Note over Daemon: session_name = "my-session" ✅<br/>(env var set before first daemon start) Daemon->>Browser: launch + navigate Shell->>CLI: agent-browser eval 'localStorage.setItem(...)' CLI->>Daemon: evaluate Daemon->>Browser: set cookie + localStorage ✅ Shell->>CLI: agent-browser close CLI->>Daemon: close Note over Daemon: handle_close():<br/>session_name = Some("my-session") ✅<br/>→ saves cookies + localStorage Daemon->>Browser: Browser.close Shell->>CLI: agent-browser open https://example.com CLI->>Daemon: ensure_daemon() → new daemon Note over Daemon: finds state file → restores data ✅ Daemon->>Browser: launch + restore Shell->>CLI: agent-browser eval 'localStorage.getItem(...)' Daemon->>Browser: evaluate Browser-->>CLI: "ok" ✅Test Plan
cargo test— 503 passed, 0 faileddispatch_state_command: routing, None for unknown, missing paramsstate clearno longer starts a daemon process🤖 Generated with Claude Code