Restore ggsql sessions after an extension host restart or window reload - #507
Open
juliasilge wants to merge 8 commits into
Open
Restore ggsql sessions after an extension host restart or window reload#507juliasilge wants to merge 8 commits into
juliasilge wants to merge 8 commits into
Conversation
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.
Closes #506.
ggsql console sessions in Positron did not come back after an extension host restart or a window reload. R and Python sessions in the same window reconnected normally. Both of these are pretty important use cases:
Cause
Positron decides which extension owns a Positron API object from the file path of the caller of
require('positron'). The extension reached the API throughtryAcquirePositronApi()from@posit-dev/positron. That helper calls a global function that Positron defines in its own bootstrap file. The path of that file is inside no extension folder. Positron therefore recorded every ggsql runtime undernullExtensionDescription.Positron uses this identity to activate the owning extension before it restores a session. For ggsql the activation failed, the error was logged at debug level, and the session was dropped.
R and Python use
import * as positron from 'positron'inside their own source files, which Positron attributes correctly.The fix
src/positronApi.tsnow callsrequire('positron')directly, andesbuild.jsmarkspositronas external. The call therefore stays inout/extension.js, which is inside the extension folder. Positron attributes it toggsql.ggsqland the sessions can come back:session-comes-back.mov
Five smaller changes are included as well:
restoreSessionaccepts thesessionNamethat Positron passes, so a renamed session keeps its name across a restore.getSupervisorApi()replaces three copies of the Positron Supervisor lookup.GgsqlRuntimeManager.alwaysRediscoveristrue. ggsql runtimes are never markedcacheable, so Positron must run discovery on every window open.ggsql-vscode/CLAUDE.mdrecords why the API is acquired this way. It also documents the F5 dev workflow, and its guidance no longer points attryAcquirePositronApi().One behavior change
validateSessionrejects when the Positron Supervisor extension is absent. It returnedfalsebefore. Positron treats both as a failed validation. The rejection shows a restore-failure notice, wherefalsediscarded the session without a message. A user who disables the Supervisor now sees why the session did not come back.Testing
The current suites run in stock VS Code, where no Positron API exists, so they cannot cover the restore path.
src/test/bundle.test.tsasserts thatrequire("positron")survives bundling, which is the half of the mechanism a unit test can currently reach. 29 tests pass.We will get going on a set of extension tests for Positron specifically and will cover functionality like this then.