fix: honour --reconnection-grace-time when the browser closes - #7956
Open
hugodeco wants to merge 1 commit into
Open
fix: honour --reconnection-grace-time when the browser closes#7956hugodeco wants to merge 1 commit into
hugodeco wants to merge 1 commit into
Conversation
--reconnection-grace-time lets an operator say how long a disconnected session should be kept, but two behaviours inherited from Code stop it from delivering that. Closing the tab runs the browser workbench's shutdown, which disposes the remote connection gracefully. The server reads a graceful dispose as "the client is finished" and cleans up at once, so the grace time is never consulted -- the session dies with the tab however the flag is set. Separately, any new connection shortens every disconnected session to ProtocolConstants.ReconnectionShortGraceTime (5 minutes), so opening a second tab cuts a deliberately long grace time back down. Add patches/session-preservation.diff, which makes the configured grace time authoritative: pagehide persists UI state instead of unloading, a browser-driven unload no longer tears the workbench down (beforeunload vetoes are still honoured, and an explicit in-product shutdown still unloads), and the grace time is only shortened when it was left at or below the default, so installations that never set the flag keep Code's stock behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
|
Thank you for the PR! This feels like something we should commit directly to VS Code rather than try to maintain as a patch here. Could we try submitting it upstream? |
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
2 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.
Fixes #7955
Summary
--reconnection-grace-time(#7678) cannot currently deliver what it documents, for two independent reasons:Closing the tab bypasses the grace time.
BrowserLifecycleServiceruns the workbench shutdown onpagehide/beforeunload, disposing the remote connection gracefully.ManagementConnectiontreats a graceful dispose as a finished client and calls_cleanResources()immediately — the grace time is never consulted, so the session dies with the tab however the flag is set.A second connection cuts it to 5 minutes. Every new connection calls
shortenReconnectionGraceTimeIfNecessary()on all disconnected connections, schedulingProtocolConstants.ReconnectionShortGraceTime. Opening a second tab is enough to shorten a deliberately long grace time.This adds
patches/session-preservation.diff, which makes the configured grace time authoritative.Changes
lib/vscode/src/vs/workbench/services/lifecycle/browser/lifecycleService.tspagehidepersists UI state (storageService.flush(WillSaveStateReason.SHUTDOWN)) instead of unloading.beforeunloadveto is still honoured, and an explicit in-product shutdown (shutdown(), which passes no veto handler) still unloads normally.lib/vscode/src/vs/server/node/remoteExtensionHostAgentServer.tsProtocolConstants.ReconnectionGraceTime. Installations that never set the flag keep Code's stock behaviour; only an operator who deliberately raised it opts into the longer wait.Also updates the unreleased changelog section.
Test plan
quilt pop -a && quilt push -a— full series applies cleanly with the new patchnpm run typecheck-clientinlib/vscode— passesnpm run lint:ts— passesnpm run test:unit— 331/332; the one failure (should unlink a socket before listening on the socket, expectingEACCES) is environmental, from running as root, and exercisessrc/node/app.ts, which this PR does not touchnpm run valid-layers-checkfails identically with and without this patch (pre-existing, innetwork.ts/extensionResourceLoader.ts)--reconnection-grace-time 2592000, sessions survive closing the browser and reconnect on returnI was not able to run a full product build on the machine I had available (disk), so a CI build is the remaining check.
🤖 Generated with Claude Code