fix(extension): per-workspace idle timeout for browser sessions#1064
Merged
fix(extension): per-workspace idle timeout for browser sessions#1064
Conversation
The global 30s WINDOW_IDLE_TIMEOUT was too aggressive for interactive `opencli browser` commands where users type manually between invocations. - browser:*/operate:* workspaces now default to 10 min idle timeout - Adapter workspaces keep the existing 30s timeout - Support custom timeout via OPENCLI_BROWSER_TIMEOUT env var (seconds) or command-level idleTimeout parameter - Surface sessionExpired warning when a new window is created after the previous session timed out - Fix stale comment (said 120s, actual was 30s) Closes #1058
…ifecycle Addresses @codex-coder review blockers: 1. sessionExpired flag was never set because getAutomationWindow() consumed expiredWorkspaces before handleCommand() could check it. Fix: use .has() in getAutomationWindow, only .delete() in handleCommand. 2. workspaceTimeoutOverrides was never cleaned up — once set, it persisted until extension restart. Fix: clear override on idle timeout expiry, explicit close-window, and borrowed-session detach. Adds 5 tests covering: - browser:* uses 10min timeout (not 30s) - sessionExpired flag is set and consumed correctly - workspaceTimeoutOverrides cleared on idle expiry - workspaceTimeoutOverrides cleared on explicit close - idleTimeout from command applies to workspace override
@WAWQAQ decided session-expired warning is not needed. Remove expiredWorkspaces tracking, sessionExpired flag from protocol, and related CLI-side warning code. Keep per-workspace timeout and override lifecycle cleanup.
832e165 to
c595460
Compare
The windows.onRemoved listener was missing workspaceTimeoutOverrides cleanup, causing stale overrides to persist across sessions when users manually close the automation window.
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 #1058 —
opencli browsersessions intermittently reset toabout:blankbecause the global 30s idle timeout is too aggressive for interactive use.browser:*/operate:*workspaces now default to 10 minutes; adapter workspaces keep the existing 30sOPENCLI_BROWSER_TIMEOUT=<seconds>env var, or passidleTimeoutin the command protocol to overrideabout:blankChanges
extension/src/background.tsgetIdleTimeout(),expiredWorkspacestracking,workspaceTimeoutOverridesfor custom timeoutextension/src/protocol.tsidleTimeoutto Command,sessionExpiredto Resultsrc/browser/page.tsidleTimeoutthrough_cmdOpts()/_wsOpt()src/browser/daemon-client.tssessionExpiredwarning vialog.warn()src/browser/bridge.tsidleTimeoutin connect optionssrc/cli.tsOPENCLI_BROWSER_TIMEOUTenv varTest plan
npm test— 208 passed (1 pre-existing bilibili network timeout, unrelated)npm run build— cleanvite build— cleanOPENCLI_BROWSER_TIMEOUT=60→ verify 60s timeout override works